Message ID | 20230505175705.18098-1-alejandro.vallejo@cloud.com (mailing list archive) |
---|---|
Headers | show |
Series | Add CpuidUserDis support | expand |
On 05.05.2023 19:57, Alejandro Vallejo wrote: > Nowadays AMD supports trapping the CPUID instruction from ring3 to ring0, Since it's relevant for PV32: Their doc talks about CPL > 0, i.e. not just ring 3. Therefore I wonder whether ... > (CpuidUserDis) ... we shouldn't deviate from the PM and avoid the misleading use of "user" in our internal naming. Jan > akin to Intel's "CPUID faulting". There is a difference in > that the toggle bit is in a different MSR and the support bit is in CPUID > itself rather than yet another MSR. This patch enables AMD hosts to use it > when supported in order to provide correct CPUID contents to PV guests. Also > allows HVM guests to use CpuidUserDis via emulated "CPUID faulting". > > Patch 1 merely adds definitions to various places in CPUID and MSR > > Patch 2 adds support for CpuidUserDis, hooking it in the probing path and > the context switching path. > > Patch 3 enables HVM guests to use CpuidUserDis as if it was CPUID faulting, > saving an avoidable roundtrip through the hypervisor at fault handling. > > Alejandro Vallejo (3): > x86: Add AMD's CpuidUserDis bit definitions > x86: Add support for CpuidUserDis > x86: Use CpuidUserDis if an AMD HVM guest toggles CPUID faulting > > tools/libs/light/libxl_cpuid.c | 1 + > tools/misc/xen-cpuid.c | 2 + > xen/arch/x86/cpu/amd.c | 29 +++++++++++- > xen/arch/x86/cpu/common.c | 51 +++++++++++---------- > xen/arch/x86/cpu/intel.c | 11 ++++- > xen/arch/x86/include/asm/amd.h | 1 + > xen/arch/x86/include/asm/msr-index.h | 1 + > xen/arch/x86/msr.c | 9 +++- > xen/include/public/arch-x86/cpufeatureset.h | 1 + > 9 files changed, 79 insertions(+), 27 deletions(-) >
On Mon, May 08, 2023 at 11:06:31AM +0200, Jan Beulich wrote: > On 05.05.2023 19:57, Alejandro Vallejo wrote: > > Nowadays AMD supports trapping the CPUID instruction from ring3 to ring0, > > Since it's relevant for PV32: Their doc talks about CPL > 0, i.e. not just > ring 3. Therefore I wonder whether ... Very true. It's CPL>0, not just ring3. Noted and changed on v2. > > > (CpuidUserDis) > > ... we shouldn't deviate from the PM and avoid the misleading use of "user" > in our internal naming. > > Jan > IMO it's going to be confusing enough as is. We'll eventually have virtualized versions of both Intel and AMD that may or may not be cross-hooked with each other (e.g: virtualized Intel interface on an AMD host) due to backward compatibility. That means we'll probably want: 1. A name for the Intel mechanism, currently "CPUID faulting" 2. A name for the AMD mechanism, currently "CpuidUserDis" 3. A generic name for the cpuid-can-be-trapped behaviour, currently overloaded with the Intel name (but could do with a Xen-specific one). It doesn't matter a lot now, but it will once the AMD interface is virtualized. Sure, we could give it an alternative name on AMD, but we still need yet another one to disambiguate the trapping behaviour from the specific mechanism that does it. Using the AMD manual name does have the upside that it's easier to check the manual without having to remember the AMD-specific feature that corresponds to a Xen-specific name. That said, if you have a good suggestion I'm happy to change it. So long as in the end result is (1), (2) and (3) have non-ambiguous names. Alejandro