svn commit: r267651 - in head: share/man/man4 sys/dev/cpuctl sys/sys usr.sbin/cpucontrol

[Konstantin Belousov]

On Fri, Jun 20, 2014 at 08:12:59AM +0200, Attilio Rao wrote:
> On Fri, Jun 20, 2014 at 6:08 AM, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
> > On Thu, Jun 19, 2014 at 09:54:41PM +0000, Attilio Rao wrote:
> >> Author: attilio
> >> Date: Thu Jun 19 21:54:41 2014
> >> New Revision: 267651
> >> URL: http://svnweb.freebsd.org/changeset/base/267651
> >>
> >> Log:
> >>   Following comments in r242565 add the possibility to specify ecx when
> >>   performing cpuid calls.
> >>   Add also a new way to specify the level type to cpucontrol(8) as
> >>   reported in the manpage.
> >>
> >>   Sponsored by:       EMC / Isilon storage division
> >>   Reviewed by:        bdrewery, gcooper
> >>   Testerd by: bdrewery
> >> Modified: head/sys/sys/cpuctl.h
> >> ==============================================================================
> >> --- head/sys/sys/cpuctl.h     Thu Jun 19 21:05:07 2014        (r267650)
> >> +++ head/sys/sys/cpuctl.h     Thu Jun 19 21:54:41 2014        (r267651)
> >> @@ -35,7 +35,8 @@ typedef struct {
> >>  } cpuctl_msr_args_t;
> >>
> >>  typedef struct {
> >> -     int             level;  /* CPUID level */
> >> +     int             level;          /* CPUID level */
> >> +     int             level_type;     /* CPUID level type */
> >>       uint32_t        data[4];
> >>  } cpuctl_cpuid_args_t;
> >>
> >> @@ -50,5 +51,6 @@ typedef struct {
> >>  #define      CPUCTL_UPDATE   _IOWR('c', 4, cpuctl_update_args_t)
> >>  #define      CPUCTL_MSRSBIT  _IOWR('c', 5, cpuctl_msr_args_t)
> >>  #define      CPUCTL_MSRCBIT  _IOWR('c', 6, cpuctl_msr_args_t)
> >> +#define      CPUCTL_CPUID_COUNT _IOWR('c', 7, cpuctl_cpuid_args_t)
> >>
> >>  #endif /* _CPUCTL_H_ */
> >
> > The cpuctl(4) is used by third-party code, and this change breaks its
> > ABI. The numeric value for CPUCTL_CPUID is changed, which means that
> > old binaries call non-existing ioctl now. This is at least a visible
> > breakage, since the argument for the ioctl changed the layout as well.
> >
> > The following patch restored the CPUCTL_CPUID for me.  I considered
> > naming its argument differently, instead of renaming the argument
> > of CPUCTL_CPUID_COUNT (which you tried to do ?), but decided not,
> > to preserve the API as well.
> 
> No, breaking the ABI is fine for -CURRENT so I don't see why we need the bloat.

No, breaking ABI is not fine at all, be it CURRENT or not. We try to
keep the ABI stable, doing costly measures like symbol versioning, where
applicable. Since this is a change to ABI for kernel interface, it
cannot be covered by symver tricks and must be done in kernel.

> I don't plan on MFC this patch. If I need to (or any user requests
> that) I will do with the appropriate ABI-compliant way (ie. adding a
> new argument like this one).

Besides the same-world ABI issue, people run jails with lower world
version on higher-versioned kernels.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20140620/75ea79a7/attachment.sig>