New CPUID/MSR driver; virtualization hooks

New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

I have finally gotten off the pot and finished writing up my new 
CPUID/MSR driver, which contains support for registers that need 
arbitrary GPRs touched.  For i386 vs x86-64 compatibility, both use an 
x86-64 register image (16 64-bit register fields); this allows 32-bit 
userspace to access the full 64-bit image if the kernel is 64 bits.

Anyway, this presumably requires new paravirtualization hooks.  The 
patch is at:

http://www.kernel.org/pub/linux/kernel/people/hpa/new-cpuid-msr.patch

... and a git tree is at ...

http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-cpuidmsr.git;a=summary

I'm posting this here to give the paravirt maintainers an opportunity to 
comment.  Presumably the functions that need to be paravirtualized are 
the ones represented by the functions do_cpuid(), do_rdmsr() and 
do_wrmsr(): they take a cpu number, an input register image, and an 
output register image, and return either 0 or -EIO (in case of a trap.)

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Chris Wright]

* H. Peter Anvin (hpa@zytor.com) wrote:
> I have finally gotten off the pot and finished writing up my new 
> CPUID/MSR driver, which contains support for registers that need 
> arbitrary GPRs touched.  For i386 vs x86-64 compatibility, both use an 
> x86-64 register image (16 64-bit register fields); this allows 32-bit 
> userspace to access the full 64-bit image if the kernel is 64 bits.
> 
> Anyway, this presumably requires new paravirtualization hooks.  The 
> patch is at:
> 
> http://www.kernel.org/pub/linux/kernel/people/hpa/new-cpuid-msr.patch

Not mirrored out yet
> 
> ... and a git tree is at ...
> 
> http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-cpuidmsr.git;a=summary

Bleah, and gitweb is unhappy ATM too.

> I'm posting this here to give the paravirt maintainers an opportunity to 
> comment.  Presumably the functions that need to be paravirtualized are 
> the ones represented by the functions do_cpuid(), do_rdmsr() and 
> do_wrmsr(): they take a cpu number, an input register image, and an 
> output register image, and return either 0 or -EIO (in case of a trap.)

Yes, so currently cpuid, for example, is like this:

do_cpuid
  cpuid
   __cpuid

Where __cpuid is
native_cpuid() on !CONFIG_PARAVIRT (include/asm-i386/processor.h)
(and this is real asm("cpuid"))
and
paravirt_ops.cpuid() on CONFIG_PARAVIRT (

Without having seen the patch yet, you'll need to make sure
that the final point which is issuing asm("cpuid") is wrapped
and split to CONFIG_PARAVIRT and non CONFIG_PARAVIRT modes.

Similar for rdmsr:

do_rdmsr
  rdmsr_eio
    rdmsr_safe

Where rdmsr is paravirtualized
rdmsr is asm("rdmsr") on !CONFIG_PARAVIRT (include/asm-i386/msr.h)
and
paravirt_ops.read_msr() on CONFIG_PARAVIRT (include/asm-i386/paravirt.h)

Similar for do_wrmsr.

Does that answer your question?

thanks,
-chris

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Chris Wright wrote:
>>
>> http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-cpuidmsr.git;a=summary
> 
> Bleah, and gitweb is unhappy ATM too.
> 

??? Works for me?

> Without having seen the patch yet, you'll need to make sure
> that the final point which is issuing asm("cpuid") is wrapped
> and split to CONFIG_PARAVIRT and non CONFIG_PARAVIRT modes.

It's not *quite* that easy.  The assembly code around this is pretty 
extensive, because it has to stand on its head in order to present the 
proper register image.

Pretty much as far as I can see it, there are two possible points where 
one can break out CONFIG_PARAVIRT:

a) int do_foo(int cpu, const u64 ireg[16], u64 oreg[16]);

b) int foo_everything(const u64 ireg[16], u64 oreg[16]);

The difference, of course, is that the former is invoked on the 
originating CPU and the latter on the target CPU at interrupt level. 
Those are pretty much the choices.

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
> It's not *quite* that easy.  The assembly code around this is pretty 
> extensive, because it has to stand on its head in order to present the 
> proper register image.
>   

Having just stood on my head for 55 breaths, might I suggest we 
implement a binary equivalent CPUID paravirt-ops wrapper; then the 
assembly code can just call CPUID and we can redefine it to call to a 
stub, which makes the pv-ops CPUID call, the puts the outputs back in 
the proper registers.

The VMI ROM CPUID is binary identical in register format with the native 
instruction, so it need not do a headstand.  For the others,

ENTRY(paravirt_raw_cpuid)
    pushl %edx
    pushl %ecx
    pushl %ebx
    pushl %eax
    mov %esp, %eax    /* pointer for eax in/out */
    leal 4(%esp), %edx /* pointer for ebx in/out */
    leal 8(%esp), %ecx /* pointer for ecx in/out */
    leal 12(%esp), %ebx /* pointer for edx in/out */
    pushl %ebx  /* arg 4 passed on stack */
    call *(paravirt_ops+pv_offset_CPUID)
    addl $4, %esp
    popl %eax
    popl %ebx
    popl %ecx
    popl %edx
    ret

Should do the right thing.  In any case, thanks for the heads up.

Zach

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Zachary Amsden wrote:
 > H. Peter Anvin wrote:
 >> http://www.kernel.org/pub/linux/kernel/people/hpa/new-cpuid-msr.patch
 >
 > The requested URL /pub/linux/kernel/people/hpa/new-cpuid-msr.patch 
was > not found on this server.

Fixed.

>> It's not *quite* that easy.  The assembly code around this is pretty 
>> extensive, because it has to stand on its head in order to present the 
>> proper register image.
> 
> Having just stood on my head for 55 breaths, might I suggest we 
> implement a binary equivalent CPUID paravirt-ops wrapper; then the 
> assembly code can just call CPUID and we can redefine it to call to a 
> stub, which makes the pv-ops CPUID call, the puts the outputs back in 
> the proper registers.
> 
> The VMI ROM CPUID is binary identical in register format with the native 
> instruction, so it need not do a headstand.  For the others,
> 
[snip]

This code is almost entirely identical to the setgpr_wrapper in the 
patch (except for the fact that setgpr_wrapper sets and captures *ALL* 
the GPRs), and it seems rather pointless to use another wrapper.  It 
takes a pointer to an entrypoint (default to "cpuid; ret" in the CPUID 
case), so it should do what you need.

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
>
> This code is almost entirely identical to the setgpr_wrapper in the 
> patch (except for the fact that setgpr_wrapper sets and captures *ALL* 
> the GPRs), and it seems rather pointless to use another wrapper.  It 
> takes a pointer to an entrypoint (default to "cpuid; ret" in the CPUID 
> case), so it should do what you need.

 void (*cpuid)(unsigned int *eax, unsigned int *ebx, ...)

Not quite.  The paravirt_ops CPUID function is a C function which takes 
pointers to each GPR as arguments, and returns the values through those 
pointers.  This doesn't allow for an entrypoint compatible with 
setgpr_wrapper, which expects the same output in both the cpuid; ret 
case and the paravirt-ops case.

One could argue that the paravirt-ops CPUID should in fact emulate the 
native instruction semantics, which would make it a non-C function.  
However, until that bridge is crossed, we would need another wrapper 
after setgpr to convert the paravirt-ops CPUID back into the same format 
as native so that setgpr_wrapper can properly store the output fields 
into the common  i386/x86_64 structure.  What definitely should be done 
is hide this secondary wrapper in the paravirt-ops code so that your 
setgpr wrapper doesn't have to deal with weird issues like this because 
of paravirt-ops changes.

Zach

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Zachary Amsden wrote:
> H. Peter Anvin wrote:
>>
>> This code is almost entirely identical to the setgpr_wrapper in the 
>> patch (except for the fact that setgpr_wrapper sets and captures *ALL* 
>> the GPRs), and it seems rather pointless to use another wrapper.  It 
>> takes a pointer to an entrypoint (default to "cpuid; ret" in the CPUID 
>> case), so it should do what you need.
> 
> void (*cpuid)(unsigned int *eax, unsigned int *ebx, ...)
> 
> Not quite.  The paravirt_ops CPUID function is a C function which takes 
> pointers to each GPR as arguments, and returns the values through those 
> pointers.  This doesn't allow for an entrypoint compatible with 
> setgpr_wrapper, which expects the same output in both the cpuid; ret 
> case and the paravirt-ops case.
> 
> One could argue that the paravirt-ops CPUID should in fact emulate the 
> native instruction semantics, which would make it a non-C function.  
> However, until that bridge is crossed, we would need another wrapper 
> after setgpr to convert the paravirt-ops CPUID back into the same format 
> as native so that setgpr_wrapper can properly store the output fields 
> into the common  i386/x86_64 structure.  What definitely should be done 
> is hide this secondary wrapper in the paravirt-ops code so that your 
> setgpr wrapper doesn't have to deal with weird issues like this because 
> of paravirt-ops changes.

I guess what I was trying to say was that we'd use setgpr_wrapper in the 
case where you have an entrypoint with native (non-C) semantics; in the 
other case we'd use an alternative to setgpr_wrapper.  Either way, it 
sounds like we're talking about implementing paravirtualization *after* 
CPU selection, i.e. we use IPI to get the thing running on the proper 
CPU before invoking the paravirtualization function?

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
>
> I guess what I was trying to say was that we'd use setgpr_wrapper in 
> the case where you have an entrypoint with native (non-C) semantics; 
> in the other case we'd use an alternative to setgpr_wrapper.  Either 
> way, it sounds like we're talking about implementing 
> paravirtualization *after* CPU selection, i.e. we use IPI to get the 
> thing running on the proper CPU before invoking the paravirtualization 
> function?

Yes, re-implementing the IPI support is rather unuseful.

Zach

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Zachary Amsden wrote:
> H. Peter Anvin wrote:
>>
>> This code is almost entirely identical to the setgpr_wrapper in the 
>> patch (except for the fact that setgpr_wrapper sets and captures *ALL* 
>> the GPRs), and it seems rather pointless to use another wrapper.  It 
>> takes a pointer to an entrypoint (default to "cpuid; ret" in the CPUID 
>> case), so it should do what you need.
> 
> void (*cpuid)(unsigned int *eax, unsigned int *ebx, ...)
> 
> Not quite.  The paravirt_ops CPUID function is a C function which takes 
> pointers to each GPR as arguments, and returns the values through those 
> pointers.  This doesn't allow for an entrypoint compatible with 
> setgpr_wrapper, which expects the same output in both the cpuid; ret 
> case and the paravirt-ops case.
> 
> One could argue that the paravirt-ops CPUID should in fact emulate the 
> native instruction semantics, which would make it a non-C function.  

There is also another option, which is to create an entrypoint with the 
semantics of the *_everything() functions, i.e. take a register image in 
and out.  The above definition of a cpuid() wrapper is insufficient to 
handle potential weird cases, so if we go down that route it's 
inherently a limited-functionality hack.

It seems to me that perhaps this is the way to do it:

- Add cpuid_everything, rdmsr_everything and wrmsr_everything
   entrypoints;

- Provide compatibility wrappers that simply invoke the cpuid, rdmsr,
   and wrmsr entrypoints with appropriate parameter marshalling;

- On hardware, point them to the functions which invoke setgpr_wrapper.

What do you think?

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
>
> It seems to me that perhaps this is the way to do it:
>
> - Add cpuid_everything, rdmsr_everything and wrmsr_everything
>   entrypoints;

Yes, although I think we can omit rdmsr / wrmsr.

>
> - Provide compatibility wrappers that simply invoke the cpuid, rdmsr,
>   and wrmsr entrypoints with appropriate parameter marshalling;
>
> - On hardware, point them to the functions which invoke setgpr_wrapper.
>
> What do you think?

Well it is more work than the hack, but it is more maintainable and 
makes for less assembly code - we could do all the parameter juggling in 
C for the paravirt-ops case.

Zach

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Zachary Amsden wrote:
> H. Peter Anvin wrote:
>>
>> It seems to me that perhaps this is the way to do it:
>>
>> - Add cpuid_everything, rdmsr_everything and wrmsr_everything
>>   entrypoints;
> 
> Yes, although I think we can omit rdmsr / wrmsr.

I actually expect rdmsr/wrmsr to have a higher likelihood of weird 
things.  I don't know how many hypervisors can trap and handle those.

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Jeremy Fitzhardinge]

H. Peter Anvin wrote:
> I actually expect rdmsr/wrmsr to have a higher likelihood of weird 
> things.  I don't know how many hypervisors can trap and handle those.

Most, if only to ignore them. 

    J

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
>
> I actually expect rdmsr/wrmsr to have a higher likelihood of weird 
> things.  I don't know how many hypervisors can trap and handle those.

Or implement anything useful at all ... perhaps it might be useful to 
simply wrap them with an -ENOSYS in some cases.

Rusty, Jeremy. Chris - any feedback on MSR support?

Zach

Re: New CPUID/MSR driver; virtualization hooks

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> Or implement anything useful at all ... perhaps it might be useful to
> simply wrap them with an -ENOSYS in some cases.
>
> Rusty, Jeremy. Chris - any feedback on MSR support?

I've looked at hpa's patch a bit, but I don't quite get what the point
is yet.  Is it that rdmsr/wrmsr can use multiple random registers, so
its necessary to haul them all around?  And that cpuid might use more
beyond the current eax-edx?

Xen will trap and emulate any rd/wrmsr instructions, so I don't think it
needs anything beyond native support.

    J

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>> Or implement anything useful at all ... perhaps it might be useful to
>> simply wrap them with an -ENOSYS in some cases.
>>
>> Rusty, Jeremy. Chris - any feedback on MSR support?
> 
> I've looked at hpa's patch a bit, but I don't quite get what the point
> is yet.  Is it that rdmsr/wrmsr can use multiple random registers, so
> its necessary to haul them all around?  And that cpuid might use more
> beyond the current eax-edx?
>

Yes, pretty much.  There are enough evidence that you can't trust CPU 
architecture to stay sane.  Inside Transmeta it was a constant battle, 
and we were a small company.

> Xen will trap and emulate any rd/wrmsr instructions, so I don't think it
> needs anything beyond native support.

If that's true for all hypervisors, then that's obviously the easiest 
all around.

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Jeremy Fitzhardinge]

H. Peter Anvin wrote:
> Yes, pretty much.  There are enough evidence that you can't trust CPU 
> architecture to stay sane.  Inside Transmeta it was a constant battle, 
> and we were a small company.
>   

Is there any indication that the msr or cpuid instructions will change
in this way?  rd/wrmsr is pretty explicitly documented as taking the msr
in %ecx, and the value in %edx:eax; do you think that will change?  And
likewise cpuid?

    J

Re: New CPUID/MSR driver; virtualization hooks

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> H. Peter Anvin wrote:
>> Yes, pretty much.  There are enough evidence that you can't trust CPU 
>> architecture to stay sane.  Inside Transmeta it was a constant battle, 
>> and we were a small company.
>>   
> 
> Is there any indication that the msr or cpuid instructions will change
> in this way?  rd/wrmsr is pretty explicitly documented as taking the msr
> in %ecx, and the value in %edx:eax; do you think that will change?  And
> likewise cpuid?

CPUID already has changed: some CPUID levels are sensitive to %ecx event 
though CPUID was originally documented as being sensitive only to %eax, 
and I know from personal experience that there are undocumented CPUID 
levels in the wild which are sensitive to %ebx.  I can't say for sure 
(at least on the record) if there are MSRs with nonarchitectural 
sensitivities in the wild, but if there aren't, I can guarantee that it 
will happen sooner or later, if only because some junior microcode 
designer thought it was a great idea.

	-hpa

Re: New CPUID/MSR driver; virtualization hooks

[Chris Wright]

* Zachary Amsden (zach@vmware.com) wrote:
> H. Peter Anvin wrote:
> >
> >I actually expect rdmsr/wrmsr to have a higher likelihood of weird 
> >things.  I don't know how many hypervisors can trap and handle those.
> 
> Or implement anything useful at all ... perhaps it might be useful to 
> simply wrap them with an -ENOSYS in some cases.
> 
> Rusty, Jeremy. Chris - any feedback on MSR support?

Drop 'em as far as I'm concerned.  T&E works fine, need to double check
lguest.

Re: New CPUID/MSR driver; virtualization hooks

[Tony Breeds]

On Wed, Apr 04, 2007 at 05:50:58PM -0700, H. Peter Anvin wrote:
> I have finally gotten off the pot and finished writing up my new 
> CPUID/MSR driver, which contains support for registers that need 
> arbitrary GPRs touched.  For i386 vs x86-64 compatibility, both use an 
> x86-64 register image (16 64-bit register fields); this allows 32-bit 
> userspace to access the full 64-bit image if the kernel is 64 bits.
> 
> Anyway, this presumably requires new paravirtualization hooks.  The 
> patch is at:
> 
> http://www.kernel.org/pub/linux/kernel/people/hpa/new-cpuid-msr.patch

I think you mean?

http://www.kernel.org/pub/linux/kernel/people/hpa/new-msr-cpuid.patch

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: New CPUID/MSR driver; virtualization hooks

[Zachary Amsden]

H. Peter Anvin wrote:
> I have finally gotten off the pot and finished writing up my new 
> CPUID/MSR driver, which contains support for registers that need 
> arbitrary GPRs touched.  For i386 vs x86-64 compatibility, both use an 
> x86-64 register image (16 64-bit register fields); this allows 32-bit 
> userspace to access the full 64-bit image if the kernel is 64 bits.
>
> Anyway, this presumably requires new paravirtualization hooks.  The 
> patch is at:
>
> http://www.kernel.org/pub/linux/kernel/people/hpa/new-cpuid-msr.patch
>   
The requested URL /pub/linux/kernel/people/hpa/new-cpuid-msr.patch was 
not found on this server.

> ... and a git tree is at ...
>
> http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-cpuidmsr.git;a=summary
>
> I'm posting this here to give the paravirt maintainers an opportunity to 
> comment.  Presumably the functions that need to be paravirtualized are 
> the ones represented by the functions do_cpuid(), do_rdmsr() and
>   

rdmsr / wrmsr can be dropped from paravirt-ops; at least for us (they 
will trap and emulate just fine, and this driver is not performance 
critical), and I think for the others as well.  CPUID, however, does 
require a hook.

Zach