mbox series

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

Message ID 20230405004520.421768-1-seanjc@google.com
Headers
Series KVM: x86: Fix unpermitted XTILE CPUID reporting |

Message

Sean Christopherson April 5, 2023, 12:45 a.m. UTC 
  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

Comments

Sean Christopherson April 10, 2023, 5:34 p.m. UTC | #1 
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.
Aaron Lewis April 11, 2023, 2:04 p.m. UTC | #2 
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>
Sean Christopherson April 12, 2023, 3:49 p.m. UTC | #3 
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