[PATCH v4 0/6] KVM: x86: Fix unpermitted XTILE CPUID reporting

Sean Christopherson posted 6 patches 2 years, 10 months ago
arch/x86/kvm/cpuid.c                          |   2 +-
arch/x86/kvm/x86.c                            |   4 +-
arch/x86/kvm/x86.h                            |  29 ++++
tools/testing/selftests/kvm/Makefile          |   1 +
.../selftests/kvm/include/x86_64/processor.h  |  69 +++++++--
.../selftests/kvm/lib/x86_64/processor.c      |  17 ++-
tools/testing/selftests/kvm/x86_64/amx_test.c |  62 +++-----
.../selftests/kvm/x86_64/xcr0_cpuid_test.c    | 132 ++++++++++++++++++
8 files changed, 251 insertions(+), 65 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86_64/xcr0_cpuid_test.c
[PATCH v4 0/6] KVM: x86: Fix unpermitted XTILE CPUID reporting
Posted by Sean Christopherson 2 years, 10 months ago
This is v4 of Aaron's "Clean up the supported xfeatures" series.

Fix a bug where KVM treats/reports XTILE_CFG as supported without
XTILE_DATA being supported if userspace queries the supported CPUID but
doesn't request access to AMX, a.k.a. XTILE_DATA.  If userspace reflects
that CPUID info back into KVM, the resulting VM may use it verbatim and
attempt to shove bad data into XCR0: XTILE_CFG and XTILE_DATA must be
set/cleared as a pair in XCR0, despite being enumerated separately.

This is effectively compile-tested only on my end.

v4:
 - Apply the massaging _only to the XTILE case.
 - Add a build-time assertion to trigger a failure if a new dynamic
   XFeature comes along without updating kvm_get_filtered_xcr0().

v3: https://lore.kernel.org/all/20230224223607.1580880-1-aaronlewis@google.com

Aaron Lewis (4):
  KVM: x86: Add a helper to handle filtering of unpermitted XCR0
    features
  KVM: selftests: Move XGETBV and XSETBV helpers to common code
  KVM: selftests: Add all known XFEATURE masks to common code
  KVM: selftests: Add test to verify KVM's supported XCR0

Sean Christopherson (2):
  KVM: x86: Filter out XTILE_CFG if XTILE_DATA isn't permitted
  KVM: selftests: Rework dynamic XFeature helper to take mask, not bit

 arch/x86/kvm/cpuid.c                          |   2 +-
 arch/x86/kvm/x86.c                            |   4 +-
 arch/x86/kvm/x86.h                            |  29 ++++
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/x86_64/processor.h  |  69 +++++++--
 .../selftests/kvm/lib/x86_64/processor.c      |  17 ++-
 tools/testing/selftests/kvm/x86_64/amx_test.c |  62 +++-----
 .../selftests/kvm/x86_64/xcr0_cpuid_test.c    | 132 ++++++++++++++++++
 8 files changed, 251 insertions(+), 65 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/xcr0_cpuid_test.c


base-commit: 27d6845d258b67f4eb3debe062b7dacc67e0c393
-- 
2.40.0.348.gf938b09366-goog
Re: [PATCH v4 0/6] KVM: x86: Fix unpermitted XTILE CPUID reporting
Posted by Sean Christopherson 2 years, 10 months ago
On Tue, Apr 04, 2023, Sean Christopherson wrote:
> This is v4 of Aaron's "Clean up the supported xfeatures" series.
> 
> Fix a bug where KVM treats/reports XTILE_CFG as supported without
> XTILE_DATA being supported if userspace queries the supported CPUID but
> doesn't request access to AMX, a.k.a. XTILE_DATA.  If userspace reflects
> that CPUID info back into KVM, the resulting VM may use it verbatim and
> attempt to shove bad data into XCR0: XTILE_CFG and XTILE_DATA must be
> set/cleared as a pair in XCR0, despite being enumerated separately.
> 
> This is effectively compile-tested only on my end.

Aaron, can you give this series a quick spin (and review) to make sure it works
as intended?  I'd like to get this into 6.4, but I'd really like it to be tested
on AMX hardware first.
Re: [PATCH v4 0/6] KVM: x86: Fix unpermitted XTILE CPUID reporting
Posted by Aaron Lewis 2 years, 10 months ago
On Mon, Apr 10, 2023 at 5:34 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Apr 04, 2023, Sean Christopherson wrote:
> > This is v4 of Aaron's "Clean up the supported xfeatures" series.
> >
> > Fix a bug where KVM treats/reports XTILE_CFG as supported without
> > XTILE_DATA being supported if userspace queries the supported CPUID but
> > doesn't request access to AMX, a.k.a. XTILE_DATA.  If userspace reflects
> > that CPUID info back into KVM, the resulting VM may use it verbatim and
> > attempt to shove bad data into XCR0: XTILE_CFG and XTILE_DATA must be
> > set/cleared as a pair in XCR0, despite being enumerated separately.
> >
> > This is effectively compile-tested only on my end.
>
> Aaron, can you give this series a quick spin (and review) to make sure it works
> as intended?  I'd like to get this into 6.4, but I'd really like it to be tested
> on AMX hardware first.

LGTM.  I ran the test on SPR and it worked as intended.  I also tried
it with the dynamic feature enabled, i.e. XTILEDATA, and that also
worked as expected.

The first run the guest XCR0 was 0x2e7 and all tests passed.  The
second run the guest XCR0 was 0x602e7 and all tests passed again.

Reviewed-by: Aaron Lewis <aaronlewis@google.com>
Tested-by: Aaron Lewis <aaronlewis@google.com>
Re: [PATCH v4 0/6] KVM: x86: Fix unpermitted XTILE CPUID reporting
Posted by Sean Christopherson 2 years, 10 months ago
On Tue, 04 Apr 2023 17:45:14 -0700, Sean Christopherson wrote:
> This is v4 of Aaron's "Clean up the supported xfeatures" series.
> 
> Fix a bug where KVM treats/reports XTILE_CFG as supported without
> XTILE_DATA being supported if userspace queries the supported CPUID but
> doesn't request access to AMX, a.k.a. XTILE_DATA.  If userspace reflects
> that CPUID info back into KVM, the resulting VM may use it verbatim and
> attempt to shove bad data into XCR0: XTILE_CFG and XTILE_DATA must be
> set/cleared as a pair in XCR0, despite being enumerated separately.
> 
> [...]

Applied to kvm-x86 selftests (due to the dependencies on the earlier AMX
selftests rework).  Thanks!

[1/6] KVM: x86: Add a helper to handle filtering of unpermitted XCR0 features
      https://github.com/kvm-x86/linux/commit/6be3ae45f567
[2/6] KVM: x86: Filter out XTILE_CFG if XTILE_DATA isn't permitted
      https://github.com/kvm-x86/linux/commit/55cd57b596e8
[3/6] KVM: selftests: Move XGETBV and XSETBV helpers to common code
      https://github.com/kvm-x86/linux/commit/b213812d3f4c
[4/6] KVM: selftests: Rework dynamic XFeature helper to take mask, not bit
      https://github.com/kvm-x86/linux/commit/7040e54fddf6
[5/6] KVM: selftests: Add all known XFEATURE masks to common code
      https://github.com/kvm-x86/linux/commit/28f2302584af
[6/6] KVM: selftests: Add test to verify KVM's supported XCR0
      https://github.com/kvm-x86/linux/commit/03a405b7a522

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes