[PATCH 0/3] Add CpuidUserDis support

Alejandro Vallejo posted 3 patches 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230505175705.18098-1-alejandro.vallejo@cloud.com
There is a newer version of this series
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(-)
[PATCH 0/3] Add CpuidUserDis support
Posted by Alejandro Vallejo 12 months ago
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(-)

-- 
2.34.1
Re: [PATCH 0/3] Add CpuidUserDis support
Posted by Jan Beulich 12 months ago
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(-)
>
Re: [PATCH 0/3] Add CpuidUserDis support
Posted by Alejandro Vallejo 11 months, 4 weeks ago
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