Re: cpuctl(formely devcpu) patch test request

On Tue, Aug 05, 2008 at 04:15:20PM +0400, Stanislav Sedov wrote:
> On Tue, 5 Aug 2008 14:53:15 +0300
> Kostik Belousov <kostikbel_at_gmail.com> mentioned:
> 
> > On Tue, Aug 05, 2008 at 02:03:24PM +0400, Stanislav Sedov wrote:
> > > On Mon, 16 Jun 2008 14:42:41 -0400
> > > Coleman Kane <cokane_at_FreeBSD.org> mentioned:
> > > 
> > > > 
> > > > Is it potentially "unsafe" to use RDMSR?
> > > >
> > > 
> > > Well, it might disclose some sensitive information,
> > > as well as create covert channels. E.g. some of the
> > > registers contains kernel thread pointers, etc; some
> > > of them undocumented. It won't be very wise to give
> > > access to the rdmsr feature to all users on a
> > > multi-user machine.
> > > 
> > > Sorry for this taking so long. You messages spotted
> > > a bug in my security model for this driver, so I've
> > > redone that. Now, the access to the rdmsr and cpuid
> > > features will be granted only if the caller has
> > > read permissions on the device, and wrmsr/update
> > >  - only if he've opened the device for writing.
> > > This way you can provide fine-grained control to
> > > the driver features.
> > > 
> > > I've also added the cpucontrol utility which provided
> > > userland accesss to the driver, and allows to apply
> > > microcode updates.
> > > 
> > > The latest patch against HEAD is available here:
> > > ftp://ftp.SpringDaemons.com/dustheap/cpuctl.4.diff
> > > 
> > > Thanks!
> > 
> > --- a/sys/amd64/amd64/support.S
> > +++ b/sys/amd64/amd64/support.S
> > _at__at_ -765,6 +765,7 _at__at_ ENTRY(wrmsr_safe)
> >   */
> >    ALIGN_TEXT
> >  msr_onfault:
> > - movq $0,PCB_ONFAULT(%r8)
> > - movl $EFAULT,%eax
> > +  movq PCPU(CURPCB),%r8     /* set fault handler */
> > +  movq $0,PCB_ONFAULT(%r8)
> > +  movq $EFAULT,%rax
> >    ret
> > 
> > movq $EFAULT,%rax is better to be replaced by movl, %eax. Amd64 specifies
> > automatic zeroing of the upper-half of the registers on the 32bit operation.
> > 
> 
> Yeah, it seems that I thought about this initially, but decided
> that this was unsafe lately. Thanks for suggestion!
> 
> There's a fixed version:
> ftp://ftp.SpringDaemons.com/dustheap/cpuctl.5.diff

Ok.

I noted cpucontrol(8) only after trying to import the rev5 patch.
I do not suggest changing it, but what are the reasons for the microcode
patch headers definitions to be private for the cpucontrol, instead of
being put into the machine/<somefile>.h ?