Re: New CPUID/MSR driver; virtualization hooks

["H. Peter Anvin" <hpa@zytor.com>]

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