| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4d9bd945-3af7-2e71-29df-8046eb0601ca@gmail.com>
Date: Wed, 2 Mar 2022 15:45:57 +0800
From: Like Xu <like.xu.linux@...il.com>
To: Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>
Cc: David Hildenbrand <david@...hat.com>,
Cornelia Huck <cohuck@...hat.com>,
Claudio Imbrenda <imbrenda@...ux.ibm.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Peter Xu <peterx@...hat.com>,
Peter Shier <pshier@...gle.com>,
Christian Borntraeger <borntraeger@...ibm.com>,
Janosch Frank <frankja@...ux.ibm.com>
Subject: Re: [PATCH 19/19] KVM: sefltests: Add x86-64 test to verify MMU
reacts to CPUID updates
On 23/6/2021 4:05 am, Sean Christopherson wrote:
> Add an x86-only test to verify that x86's MMU reacts to CPUID updates
> that impact the MMU. KVM has had multiple bugs where it fails to
> reconfigure the MMU after the guest's vCPU model changes.
>
> Sadly, this test is effectively limited to shadow paging because the
The x86_64/mmu_role_test fails when "N=/sys/module/kvm_intel/parameters/ept" :
KVM_SET_CPUID2 failed, rc: -1 errno: 22 (due to "63f5a1909f9e")
Does this regression meet your expectations even after "feb627e8d6f6" ?
> hardware page walk handler doesn't support software disabling of GBPAGES
> support, and KVM doesn't manually walk the GVA->GPA on faults for
> performance reasons (doing so would large defeat the benefits of TDP).
>
> Don't require !TDP for the tests as there is still value in running the
> tests with TDP, even though the tests will fail (barring KVM hacks).
In this case, we could assert that the test will fail, rather than throwing the
failure,
which is unfriendly to many CI systems. What do you think ?
> E.g. KVM should not completely explode if MAXPHYADDR results in KVM using
> 4-level vs. 5-level paging for the guest.
>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 1 +
> .../selftests/kvm/include/x86_64/processor.h | 3 +
> .../selftests/kvm/x86_64/mmu_role_test.c | 147 ++++++++++++++++++
> 4 files changed, 152 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/mmu_role_test.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index e0e14150744e..6ead3403eca6 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -15,6 +15,7 @@
> /x86_64/hyperv_cpuid
> /x86_64/hyperv_features
> /x86_64/mmio_warning_test
> +/x86_64/mmu_role_test
> /x86_64/platform_info_test
> /x86_64/set_boot_cpu_id
> /x86_64/set_sregs_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 61e2accd080d..8dc007bac0fe 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -47,6 +47,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
> TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test
> TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test
> +TEST_GEN_PROGS_x86_64 += x86_64/mmu_role_test
> TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test
> TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id
> TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index f21126941f19..914b0d16929c 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -55,6 +55,9 @@
> #define CPUID_PKU (1ul << 3)
> #define CPUID_LA57 (1ul << 16)
>
> +/* CPUID.0x8000_0001.EDX */
> +#define CPUID_GBPAGES (1ul << 26)
> +
> #define UNEXPECTED_VECTOR_PORT 0xfff0u
>
> /* General Registers in 64-Bit Mode */
> diff --git a/tools/testing/selftests/kvm/x86_64/mmu_role_test.c b/tools/testing/selftests/kvm/x86_64/mmu_role_test.c
> new file mode 100644
> index 000000000000..523371cf8e8f
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/mmu_role_test.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +
> +#define VCPU_ID 1
> +
> +#define MMIO_GPA 0x100000000ull
> +
> +static void guest_code(void)
> +{
> + (void)READ_ONCE(*((uint64_t *)MMIO_GPA));
> + (void)READ_ONCE(*((uint64_t *)MMIO_GPA));
> +
> + GUEST_ASSERT(0);
> +}
> +
> +static void guest_pf_handler(struct ex_regs *regs)
> +{
> + /* PFEC == RSVD | PRESENT (read, kernel). */
> + GUEST_ASSERT(regs->error_code == 0x9);
> + GUEST_DONE();
> +}
> +
> +static void mmu_role_test(u32 *cpuid_reg, u32 evil_cpuid_val)
> +{
> + u32 good_cpuid_val = *cpuid_reg;
> + struct kvm_run *run;
> + struct kvm_vm *vm;
> + uint64_t cmd;
> + int r;
> +
> + /* Create VM */
> + vm = vm_create_default(VCPU_ID, 0, guest_code);
> + run = vcpu_state(vm, VCPU_ID);
> +
> + /* Map 1gb page without a backing memlot. */
> + __virt_pg_map(vm, MMIO_GPA, MMIO_GPA, X86_PAGE_SIZE_1G);
> +
> + r = _vcpu_run(vm, VCPU_ID);
> +
> + /* Guest access to the 1gb page should trigger MMIO. */
> + TEST_ASSERT(r == 0, "vcpu_run failed: %d\n", r);
> + TEST_ASSERT(run->exit_reason == KVM_EXIT_MMIO,
> + "Unexpected exit reason: %u (%s), expected MMIO exit (1gb page w/o memslot)\n",
> + run->exit_reason, exit_reason_str(run->exit_reason));
> +
> + TEST_ASSERT(run->mmio.len == 8, "Unexpected exit mmio size = %u", run->mmio.len);
> +
> + TEST_ASSERT(run->mmio.phys_addr == MMIO_GPA,
> + "Unexpected exit mmio address = 0x%llx", run->mmio.phys_addr);
> +
> + /*
> + * Effect the CPUID change for the guest and re-enter the guest. Its
> + * access should now #PF due to the PAGE_SIZE bit being reserved or
> + * the resulting GPA being invalid. Note, kvm_get_supported_cpuid()
> + * returns the struct that contains the entry being modified. Eww.
> + */
> + *cpuid_reg = evil_cpuid_val;
> + vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
> +
> + /*
> + * Add a dummy memslot to coerce KVM into bumping the MMIO generation.
> + * KVM does not "officially" support mucking with CPUID after KVM_RUN,
> + * and will incorrectly reuse MMIO SPTEs. Don't delete the memslot!
> + * KVM x86 zaps all shadow pages on memslot deletion.
> + */
> + vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> + MMIO_GPA << 1, 10, 1, 0);
> +
> + /* Set up a #PF handler to eat the RSVD #PF and signal all done! */
> + vm_init_descriptor_tables(vm);
> + vcpu_init_descriptor_tables(vm, VCPU_ID);
> + vm_handle_exception(vm, PF_VECTOR, guest_pf_handler);
> +
> + r = _vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(r == 0, "vcpu_run failed: %d\n", r);
> +
> + cmd = get_ucall(vm, VCPU_ID, NULL);
> + TEST_ASSERT(cmd == UCALL_DONE,
> + "Unexpected guest exit, exit_reason=%s, ucall.cmd = %lu\n",
> + exit_reason_str(run->exit_reason), cmd);
> +
> + /*
> + * Restore the happy CPUID value for the next test. Yes, changes are
> + * indeed persistent across VM destruction.
> + */
> + *cpuid_reg = good_cpuid_val;
> +
> + kvm_vm_free(vm);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct kvm_cpuid_entry2 *entry;
> + int opt;
> +
> + /*
> + * All tests are opt-in because TDP doesn't play nice with reserved #PF
> + * in the GVA->GPA translation. The hardware page walker doesn't let
> + * software change GBPAGES or MAXPHYADDR, and KVM doesn't manually walk
> + * the GVA on fault for performance reasons.
> + */
> + bool do_gbpages = false;
> + bool do_maxphyaddr = false;
> +
> + setbuf(stdout, NULL);
> +
> + while ((opt = getopt(argc, argv, "gm")) != -1) {
> + switch (opt) {
> + case 'g':
> + do_gbpages = true;
> + break;
> + case 'm':
> + do_maxphyaddr = true;
> + break;
> + case 'h':
> + default:
> + printf("usage: %s [-g (GBPAGES)] [-m (MAXPHYADDR)]\n", argv[0]);
> + break;
> + }
> + }
> +
> + if (!do_gbpages && !do_maxphyaddr) {
> + print_skip("No sub-tests selected");
> + return 0;
> + }
> +
> + entry = kvm_get_supported_cpuid_entry(0x80000001);
> + if (!(entry->edx & CPUID_GBPAGES)) {
> + print_skip("1gb hugepages not supported");
> + return 0;
> + }
> +
> + if (do_gbpages) {
> + pr_info("Test MMIO after toggling CPUID.GBPAGES\n\n");
> + mmu_role_test(&entry->edx, entry->edx & ~CPUID_GBPAGES);
> + }
> +
> + if (do_maxphyaddr) {
> + pr_info("Test MMIO after changing CPUID.MAXPHYADDR\n\n");
> + entry = kvm_get_supported_cpuid_entry(0x80000008);
> + mmu_role_test(&entry->eax, (entry->eax & ~0xff) | 0x20);
> + }
> +
> + return 0;
> +}
Powered by blists - more mailing lists