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 _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization
