mbox series

[0/3] Add CpuidUserDis support

Message ID 20230505175705.18098-1-alejandro.vallejo@cloud.com (mailing list archive)
Headers show
Series Add CpuidUserDis support | expand

Message

Alejandro Vallejo May 5, 2023, 5:57 p.m. UTC
Nowadays AMD supports trapping the CPUID instruction from ring3 to ring0,
(CpuidUserDis) 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(-)

Comments

Jan Beulich May 8, 2023, 9:06 a.m. UTC | #1
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(-)
>
Alejandro Vallejo May 10, 2023, 11:28 a.m. UTC | #2
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