r205751 - intrin.h: Implement __readmsr, __readcr3, and __writecr3

[Reid Kleckner]

I also realized there were some identifiers that weren't in the
implementer's namespace, which I'll go fix.

On Tue, Apr 8, 2014 at 3:59 AM, PaX Team <pageexec at freemail.hu> wrote:

> On 8 Apr 2014 at 0:28, Reid Kleckner wrote:
>
> > +static __inline__ unsigned __int64 __attribute__((__always_inline__,
> __nodebug__))
> > +__readmsr(unsigned long __register) {
> > +  // Loads the contents of a 64-bit model specific register (MSR)
> specified in
> > +  // the ECX register into registers EDX:EAX. The EDX register is
> loaded with
> > +  // the high-order 32 bits of the MSR and the EAX register is loaded
> with the
> > +  // low-order 32 bits. If less than 64 bits are implemented in the MSR
> being
> > +  // read, the values returned to EDX:EAX in unimplemented bit
> locations are
> > +  // undefined.
> > +  unsigned long __edx;
> > +  unsigned long __eax;
> > +  __asm__ ("rdmsr"
> > +          : "=d"(__edx), "=a"(__eax)
> > +          : "c"(__register)
> > +          : "%ecx", "%edx", "%eax");
> > +  return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
> > +}
>
> i don't think this is correct, input/output registers should not appear on
> the clobbered list. gcc itself doesn't accept this code and complains with:
>
>   error: 'asm' operand has impossible constraints


Yeah, that's wrong. I'll fix it. I suspected it was wrong, but it compiled
fine.

I have *not* done execution tests of this code, because that would involve
far more setup than I'm prepared to do. I've only compiled it and examined
the output.


> > +static __inline__ unsigned long __attribute__((always_inline,
> __nodebug__))
> > +__readcr3(void) {
> > +  unsigned long value;
> > +  __asm__ __volatile__("mov %%cr3, %0" : "=q"(value));
> > +  return value;
> > +}
>
> note that asm volatile won't prevent reordering per se and the solution
> linux uses is that these inline asm stmts have a fake dependency (read
> or write) on a variable used just for this purpose. the alternative would
> be to use a memory clobber as you did for __writecr3 but that's quite
> heavyweight as it prevents reorderig all other loads/stores as well which
> isn't often necessary (e.g., on a context switch the kernel doesn't care
> how loads/stores get reordered around a cr3 reload since only the userland
> part of the address space changes).
>

I'm going to put the memory constraint on to prevent reordering for now.
 This is mostly a compatibility header being used by people writing Windows
drivers.  If they aren't happy with the performance, they can feel free to
pull the inline asm up into their project and fine tune it appropriately.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140408/501304fb/attachment.html>