On Fri, 2023-05-26 at 13:32 -0700, Sean Christopherson wrote:
> : Aha! And QEMU appears to have Xen emulation support. That means KVM-Unit-Tests
> : is an option. Specifically, extend the "access" test to use this hypercall instead
> : of INVLPG. That'll verify that the flush is actually being performed as expteced.
That works. Metin has a better version that actually sets up the
hypercall page properly and uses it, but that one bails out when Xen
support isn't present, and doesn't show the failure mode quite so
clearly. This is the simple version:
From: Metin Kaya <metikaya@xxxxxxxxxxxx>
Subject: [PATCH] x86/access: Use hvm_op/flush_tlbs hypercall instead of invlpg instruction
Signed-off-by: Metin Kaya <metikaya@xxxxxxxxxxxx>
---
x86/access.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/x86/access.c b/x86/access.c
index 83c8221..6fa08c5 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -253,12 +253,31 @@ static void clear_user_mask(pt_element_t *ptep, int level, unsigned long virt)
*ptep &= ~PT_USER_MASK;
}
+#define KVM_HYPERCALL_INTEL ".byte 0x0f,0x01,0xc1"
+#define __HYPERVISOR_hvm_op 34
+#define HVMOP_flush_tlbs 5
+
+static inline long hvm_op_flush_tlbs(void)
+{
+ long ret;
+ uint64_t nr = __HYPERVISOR_hvm_op;
+ uint64_t op = HVMOP_flush_tlbs;
+ uint64_t arg = 0;
+
+ asm volatile(KVM_HYPERCALL_INTEL
+ : "=a"(ret)
+ : "a"(nr), "D" (op), "S" (arg)
+ : "memory");
+
+ return ret;
+}
+
static void set_user_mask(pt_element_t *ptep, int level, unsigned long virt)
{
*ptep |= PT_USER_MASK;
/* Flush to avoid spurious #PF */
- invlpg((void*)virt);
+ hvm_op_flush_tlbs();
}
static unsigned set_cr4_smep(ac_test_t *at, int smep)
@@ -577,7 +596,7 @@ fault:
static void ac_set_expected_status(ac_test_t *at)
{
- invlpg(at->virt);
+ hvm_op_flush_tlbs();
if (at->ptep)
at->expected_pte = *at->ptep;
--
2.34.1
Run it with Xen support enabled and it works...
$ qemu-system-x86_64 -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -M pc -device pci-testdev --accel kvm,xen-version=0x4000a,kernel-irqchip=split -kernel x86/access_test.flat
enabling apic
starting test
run
CR4.PKE not available, disabling PKE tests
CR4.SMEP not available, disabling SMEP tests
.............................
1916935 tests, 0 failures
To be sure, run it without Xen support, the hypercall silently fails
and the test fails....
$ qemu-system-x86_64 -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -M pc -device pci-testdev --accel kvm -kernel x86/access_test.flat
enabling apic
starting test
run
CR4.PKE not available, disabling PKE tests
CR4.SMEP not available, disabling SMEP tests
test pte.rw pte.p pde.p: FAIL: pte 2000003 expected 2000023
Dump mapping: address: 0xffff923400000000
------L4 I292: 2100027
------L3 I208: 2101027
------L2 I0: 2102061
------L1 I0: 2000003
test pte.user pte.p pde.p: FAIL: pte 2000005 expected 2000025
Dump mapping: address: 0xffff923400000000
------L4 I292: 2100027
------L3 I208: 2101027
------L2 I0: 2102061
------L1 I0: 2000005
…
> Let me be more explicit this time: I am not applying this without a test. I don't
> care how trivial a patch may seem, I'm done taking patches without test coverage
> unless there's a *really* good reason for me to do so.
Understood.
So, we know it *works*, but the above is a one-off and not a
*regression* test.
It would definitely be nice to have regression tests that cover
absolutely everything, but adding Xen guest support to the generic KVM-
Unit-Tests might be considered overkill, because this *particular*
thing is fairly unlikely to regress? It really is just calling
kvm_flush_remote_tlbs(), and if that goes wrong we're probably likely
to notice it anyway.
What do you think?
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
