* [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues
@ 2023-12-12 6:27 Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 1/3] x86: Add a common header asyncpf.h Dan Wu
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Dan Wu @ 2023-12-12 6:27 UTC (permalink / raw)
To: seanjc, pbonzini, kvm; +Cc: xiaoyao.li, dan1.wu
When running asyncpf test, it gets skipped without a clear reason:
./asyncpf
enabling apic
smp: waiting for 0 APs
paging enabled
cr0 = 80010011
cr3 = 107f000
cr4 = 20
install handler
enable async pf
alloc memory
start loop
end loop
start loop
end loop
SUMMARY: 0 tests
SKIP asyncpf (0 tests)
The reason is that KVM changed to use interrupt-based 'page-ready' notification
and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
This series tries to fix the problem by separating two testcases for different mechanisms.
- For old #PF-based notification, changes current asyncpf.c to add CPUID check
at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
otherwise it gets skipped.
- For new interrupt-based notification, add a new test, asyncpf-int.c, to check
(KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
'page-ready' handler.
Dan Wu (3):
x86: Add a common header asyncpf.h
x86: Add async page fault int test
x86/asyncpf: Add CPUID feature bits check to ensure feature is
available
ci/cirrus-ci-fedora.yml | 1 +
lib/x86/processor.h | 6 ++
x86/Makefile.common | 3 +-
x86/asyncpf.c | 31 ++++++----
x86/asyncpf.h | 23 ++++++++
x86/asyncpf_int.c | 127 ++++++++++++++++++++++++++++++++++++++++
x86/unittests.cfg | 6 +-
7 files changed, 185 insertions(+), 12 deletions(-)
create mode 100644 x86/asyncpf.h
create mode 100644 x86/asyncpf_int.c
--
2.39.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [kvm-unit-tests PATCH v1 1/3] x86: Add a common header asyncpf.h
2023-12-12 6:27 [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Dan Wu
@ 2023-12-12 6:27 ` Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 2/3] x86: Add async page fault int test Dan Wu
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Dan Wu @ 2023-12-12 6:27 UTC (permalink / raw)
To: seanjc, pbonzini, kvm; +Cc: xiaoyao.li, dan1.wu
Grab the ASYNC PF related bit definitions from asyncpf.c and put them
into asyncpf.h. Add some new definitions and they are going to be used
by asyncpf-int test which will be added later.
Signed-off-by: Dan Wu <dan1.wu@intel.com>
---
x86/asyncpf.c | 9 +--------
x86/asyncpf.h | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 8 deletions(-)
create mode 100644 x86/asyncpf.h
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index bc515be9..a0bdefcf 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -27,14 +27,7 @@
#include "libcflat.h"
#include "vmalloc.h"
#include <stdint.h>
-
-#define KVM_PV_REASON_PAGE_NOT_PRESENT 1
-#define KVM_PV_REASON_PAGE_READY 2
-
-#define MSR_KVM_ASYNC_PF_EN 0x4b564d02
-
-#define KVM_ASYNC_PF_ENABLED (1 << 0)
-#define KVM_ASYNC_PF_SEND_ALWAYS (1 << 1)
+#include "asyncpf.h"
volatile uint32_t apf_reason __attribute__((aligned(64)));
char *buf;
diff --git a/x86/asyncpf.h b/x86/asyncpf.h
new file mode 100644
index 00000000..8e4a133e
--- /dev/null
+++ b/x86/asyncpf.h
@@ -0,0 +1,23 @@
+#define KVM_PV_REASON_PAGE_NOT_PRESENT 1
+#define KVM_PV_REASON_PAGE_READY 2
+
+#define MSR_KVM_ASYNC_PF_EN 0x4b564d02
+#define MSR_KVM_ASYNC_PF_INT 0x4b564d06
+#define MSR_KVM_ASYNC_PF_ACK 0x4b564d07
+
+#define KVM_ASYNC_PF_ENABLED (1 << 0)
+#define KVM_ASYNC_PF_SEND_ALWAYS (1 << 1)
+#define KVM_ASYNC_PF_DELIVERY_AS_INT (1 << 3)
+
+#define HYPERVISOR_CALLBACK_VECTOR 0xf3
+
+struct kvm_vcpu_pv_apf_data {
+ /* Used for 'page not present' events delivered via #PF */
+ uint32_t flags;
+
+ /* Used for 'page ready' events delivered via interrupt notification */
+ uint32_t token;
+
+ uint8_t pad[56];
+ uint32_t enabled;
+} __attribute__((aligned(64)));
--
2.39.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [kvm-unit-tests PATCH v1 2/3] x86: Add async page fault int test
2023-12-12 6:27 [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 1/3] x86: Add a common header asyncpf.h Dan Wu
@ 2023-12-12 6:27 ` Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 3/3] x86/asyncpf: Add CPUID feature bits check to ensure feature is available Dan Wu
2023-12-12 15:17 ` [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Sean Christopherson
3 siblings, 0 replies; 8+ messages in thread
From: Dan Wu @ 2023-12-12 6:27 UTC (permalink / raw)
To: seanjc, pbonzini, kvm; +Cc: xiaoyao.li, dan1.wu
KVM switched to use interrupt for 'page ready' APF event since Linux v5.10 and
the legacy mechanism using #PF was deprecated. Add a new test for interrupt
based 'page ready' APF event delivery.
Signed-off-by: Dan Wu <dan1.wu@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
The test is based on asyncpf.c and simplifies implementation.
---
ci/cirrus-ci-fedora.yml | 1 +
lib/x86/processor.h | 6 ++
x86/Makefile.common | 3 +-
x86/asyncpf_int.c | 127 ++++++++++++++++++++++++++++++++++++++++
x86/unittests.cfg | 4 ++
5 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 x86/asyncpf_int.c
diff --git a/ci/cirrus-ci-fedora.yml b/ci/cirrus-ci-fedora.yml
index 918c9a36..52cb10c6 100644
--- a/ci/cirrus-ci-fedora.yml
+++ b/ci/cirrus-ci-fedora.yml
@@ -22,6 +22,7 @@ fedora_task:
- ./run_tests.sh
access
asyncpf
+ asyncpf_int
debug
emulator
ept
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 44f4fd1e..1a0f1243 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -263,6 +263,12 @@ static inline bool is_intel(void)
#define X86_FEATURE_ARCH_CAPABILITIES (CPUID(0x7, 0, EDX, 29))
#define X86_FEATURE_PKS (CPUID(0x7, 0, ECX, 31))
+/*
+ * KVM defined leafs
+ */
+#define KVM_FEATURE_ASYNC_PF (CPUID(0x40000001, 0, EAX, 4))
+#define KVM_FEATURE_ASYNC_PF_INT (CPUID(0x40000001, 0, EAX, 14))
+
/*
* Extended Leafs, a.k.a. AMD defined
*/
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 4ae9a557..c4b309e3 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -90,7 +90,8 @@ tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
$(TEST_DIR)/emulator.$(exe) \
$(TEST_DIR)/eventinj.$(exe) \
$(TEST_DIR)/smap.$(exe) \
- $(TEST_DIR)/umip.$(exe)
+ $(TEST_DIR)/umip.$(exe) \
+ $(TEST_DIR)/asyncpf_int.$(exe)
# The following test cases are disabled when building EFI tests because they
# use absolute addresses in their inline assembly code, which cannot compile
diff --git a/x86/asyncpf_int.c b/x86/asyncpf_int.c
new file mode 100644
index 00000000..84268f6b
--- /dev/null
+++ b/x86/asyncpf_int.c
@@ -0,0 +1,127 @@
+/*
+ * Async PF Int test. For the test to actually do anything it needs to be started
+ * in memory cgroup with 512M of memory and with more than 1G memory provided
+ * to the guest.
+ *
+ * To identify the cgroup version on Linux:
+ * stat -fc %T /sys/fs/cgroup/
+ *
+ * If the output is tmpfs, your system is using cgroup v1:
+ * To create cgroup do as root:
+ * mkdir /dev/cgroup
+ * mount -t cgroup none -omemory /dev/cgroup
+ * chmod a+rxw /dev/cgroup/
+ * From a shell you will start qemu from:
+ * mkdir /dev/cgroup/1
+ * echo $$ > /dev/cgroup/1/tasks
+ * echo 512M > /dev/cgroup/1/memory.limit_in_bytes
+ *
+ * If the output is cgroup2fs, your system is using cgroup v2:
+ * mkdir /sys/fs/cgroup/cg1
+ * echo $$ > /sys/fs/cgroup/cg1/cgroup.procs
+ * echo 512M > /sys/fs/cgroup/cg1/memory.max
+ *
+ */
+#include "x86/processor.h"
+#include "x86/apic.h"
+#include "x86/isr.h"
+#include "x86/vm.h"
+#include "alloc.h"
+#include "vmalloc.h"
+#include "asyncpf.h"
+
+struct kvm_vcpu_pv_apf_data apf_reason;
+
+char *buf;
+void* virt;
+volatile uint64_t i;
+volatile uint64_t phys;
+volatile uint32_t saved_token;
+volatile uint32_t asyncpf_num;
+
+static inline uint32_t get_and_clear_apf_reason(void)
+{
+ uint32_t r = apf_reason.flags;
+ apf_reason.flags = 0;
+ return r;
+}
+
+static void handle_interrupt(isr_regs_t *regs)
+{
+ uint32_t apf_token = apf_reason.token;
+
+ apf_reason.token = 0;
+ wrmsr(MSR_KVM_ASYNC_PF_ACK, 1);
+
+ if (apf_token == 0xffffffff) {
+ report_pass("Wakeup all, got token 0x%x", apf_token);
+ } else if (apf_token == saved_token) {
+ asyncpf_num++;
+ install_pte(phys_to_virt(read_cr3()), 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
+ phys = 0;
+ } else {
+ report_fail("unexpected async pf int token 0x%x", apf_token);
+ }
+
+ eoi();
+}
+
+static void handle_pf(struct ex_regs *r)
+{
+ virt = (void*)((ulong)(buf+i) & ~(PAGE_SIZE-1));
+ uint32_t reason = get_and_clear_apf_reason();
+ switch (reason) {
+ case 0:
+ report_fail("unexpected #PF at %#lx", read_cr2());
+ exit(report_summary());
+ case KVM_PV_REASON_PAGE_NOT_PRESENT:
+ phys = virt_to_pte_phys(phys_to_virt(read_cr3()), virt);
+ install_pte(phys_to_virt(read_cr3()), 1, virt, phys, 0);
+ write_cr3(read_cr3());
+ saved_token = read_cr2();
+ while (phys) {
+ safe_halt(); /* enables irq */
+ }
+ break;
+ default:
+ report_fail("unexpected async pf with reason 0x%x", reason);
+ exit(report_summary());
+ }
+}
+
+#define MEM (1ull*1024*1024*1024)
+
+int main(int ac, char **av)
+{
+ if (!this_cpu_has(KVM_FEATURE_ASYNC_PF)) {
+ report_skip("KVM_FEATURE_ASYNC_PF is not supported\n");
+ return report_summary();
+ }
+
+ if (!this_cpu_has(KVM_FEATURE_ASYNC_PF_INT)) {
+ report_skip("KVM_FEATURE_ASYNC_PF_INT is not supported\n");
+ return report_summary();
+ }
+
+ setup_vm();
+
+ handle_exception(PF_VECTOR, handle_pf);
+ handle_irq(HYPERVISOR_CALLBACK_VECTOR, handle_interrupt);
+ memset(&apf_reason, 0, sizeof(apf_reason));
+
+ wrmsr(MSR_KVM_ASYNC_PF_INT, HYPERVISOR_CALLBACK_VECTOR);
+ wrmsr(MSR_KVM_ASYNC_PF_EN, virt_to_phys((void*)&apf_reason) |
+ KVM_ASYNC_PF_SEND_ALWAYS | KVM_ASYNC_PF_ENABLED | KVM_ASYNC_PF_DELIVERY_AS_INT);
+
+ buf = malloc(MEM);
+ sti();
+
+ /* access a lot of memory to make host swap it out */
+ for (i = 0; i < MEM; i += 4096)
+ buf[i] = 1;
+
+ cli();
+ report(asyncpf_num > 0, "get %d async pf events ('page not present' #PF event with matched "
+ "'page ready' interrupt event )", asyncpf_num);
+ return report_summary();
+}
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 3fe59449..8735ba34 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -174,6 +174,10 @@ extra_params = -cpu max
file = asyncpf.flat
extra_params = -m 2048
+[asyncpf_int]
+file = asyncpf_int.flat
+extra_params = -cpu host -m 2048
+
[emulator]
file = emulator.flat
--
2.39.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [kvm-unit-tests PATCH v1 3/3] x86/asyncpf: Add CPUID feature bits check to ensure feature is available
2023-12-12 6:27 [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 1/3] x86: Add a common header asyncpf.h Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 2/3] x86: Add async page fault int test Dan Wu
@ 2023-12-12 6:27 ` Dan Wu
2023-12-12 15:17 ` [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Sean Christopherson
3 siblings, 0 replies; 8+ messages in thread
From: Dan Wu @ 2023-12-12 6:27 UTC (permalink / raw)
To: seanjc, pbonzini, kvm; +Cc: xiaoyao.li, dan1.wu
Asyncpf test only works for #PF-based 'page-ready' notification. This case
becomes invalid for KVM that starts to enumerate interrupt-based 'page-ready'
notification.
Add CPUID feature check at the beginning, and skip the test when KVM_FEATURE_ASYNC_PF
is not available or it enumerates KVM_FEATURE_ASYNC_PF_INT.
To run this test, add the QEMU option "-cpu host" to check CPUID, since
KVM_FEATURE_ASYNC_PF_INT can't be detected without "-cpu host".
Update the usage of how to setup cgroup for different cgroup versions.
Signed-off-by: Dan Wu <dan1.wu@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Unlike patch #2, this patch retains the original test without reducing the memory
access round from 2 to 1. If anyone thinks it worthes a go, please call out.
---
x86/asyncpf.c | 22 ++++++++++++++++++++--
x86/unittests.cfg | 2 +-
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index a0bdefcf..e2cf9934 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -1,18 +1,26 @@
/*
* Async PF test. For the test to actually do anything it needs to be started
- * in memory cgroup with 512M of memory and with more then 1G memory provided
+ * in memory cgroup with 512M of memory and with more than 1G memory provided
* to the guest.
*
+ * To identify the cgroup version on Linux:
+ * stat -fc %T /sys/fs/cgroup/
+ *
+ * If the output is tmpfs, your system is using cgroup v1:
* To create cgroup do as root:
* mkdir /dev/cgroup
* mount -t cgroup none -omemory /dev/cgroup
* chmod a+rxw /dev/cgroup/
- *
* From a shell you will start qemu from:
* mkdir /dev/cgroup/1
* echo $$ > /dev/cgroup/1/tasks
* echo 512M > /dev/cgroup/1/memory.limit_in_bytes
*
+ * If the output is cgroup2fs, your system is using cgroup v2:
+ * mkdir /sys/fs/cgroup/cg1
+ * echo $$ > /sys/fs/cgroup/cg1/cgroup.procs
+ * echo 512M > /sys/fs/cgroup/cg1/memory.max
+ *
*/
#include "x86/msr.h"
#include "x86/processor.h"
@@ -79,6 +87,16 @@ static void pf_isr(struct ex_regs *r)
int main(int ac, char **av)
{
+ if (!this_cpu_has(KVM_FEATURE_ASYNC_PF)) {
+ report_skip("KVM_FEATURE_ASYNC_PF is not supported\n");
+ return report_summary();
+ }
+
+ if (this_cpu_has(KVM_FEATURE_ASYNC_PF_INT)) {
+ report_skip("interrupt-based page-ready event is enumerated, use asyncpf-int instead.\n");
+ return report_summary();
+ }
+
int loop = 2;
setup_vm();
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 8735ba34..94a5c7c7 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -172,7 +172,7 @@ extra_params = -cpu max
[asyncpf]
file = asyncpf.flat
-extra_params = -m 2048
+extra_params = -cpu host -m 2048
[asyncpf_int]
file = asyncpf_int.flat
--
2.39.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues
2023-12-12 6:27 [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Dan Wu
` (2 preceding siblings ...)
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 3/3] x86/asyncpf: Add CPUID feature bits check to ensure feature is available Dan Wu
@ 2023-12-12 15:17 ` Sean Christopherson
2023-12-13 1:36 ` Wu, Dan1
3 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2023-12-12 15:17 UTC (permalink / raw)
To: Dan Wu; +Cc: pbonzini, kvm, xiaoyao.li
On Tue, Dec 12, 2023, Dan Wu wrote:
> When running asyncpf test, it gets skipped without a clear reason:
>
> ./asyncpf
>
> enabling apic
> smp: waiting for 0 APs
> paging enabled
> cr0 = 80010011
> cr3 = 107f000
> cr4 = 20
> install handler
> enable async pf
> alloc memory
> start loop
> end loop
> start loop
> end loop
> SUMMARY: 0 tests
> SKIP asyncpf (0 tests)
>
> The reason is that KVM changed to use interrupt-based 'page-ready' notification
> and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
> 'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
> in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
>
> This series tries to fix the problem by separating two testcases for different mechanisms.
>
> - For old #PF-based notification, changes current asyncpf.c to add CPUID check
> at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
> otherwise it gets skipped.
>
> - For new interrupt-based notification, add a new test, asyncpf-int.c, to check
> (KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
> 'page-ready' handler.
Using #PF to deliver page-ready is completely dead, no? Unless I'm mistaken, let's
just drop the existing support and replace it with the interrupted-based mechanism.
I see no reason to continue maintaining the old crud. If someone wants to verify
an old, broken KVM, then they can use the old version of KUT.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues
2023-12-12 15:17 ` [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Sean Christopherson
@ 2023-12-13 1:36 ` Wu, Dan1
2023-12-13 18:20 ` Sean Christopherson
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Dan1 @ 2023-12-13 1:36 UTC (permalink / raw)
To: Sean Christopherson; +Cc: pbonzini, kvm, xiaoyao.li
On 12/12/2023 11:17 PM, Sean Christopherson wrote:
> On Tue, Dec 12, 2023, Dan Wu wrote:
>> When running asyncpf test, it gets skipped without a clear reason:
>>
>> ./asyncpf
>>
>> enabling apic
>> smp: waiting for 0 APs
>> paging enabled
>> cr0 = 80010011
>> cr3 = 107f000
>> cr4 = 20
>> install handler
>> enable async pf
>> alloc memory
>> start loop
>> end loop
>> start loop
>> end loop
>> SUMMARY: 0 tests
>> SKIP asyncpf (0 tests)
>>
>> The reason is that KVM changed to use interrupt-based 'page-ready' notification
>> and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
>> 'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
>> in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
>>
>> This series tries to fix the problem by separating two testcases for different mechanisms.
>>
>> - For old #PF-based notification, changes current asyncpf.c to add CPUID check
>> at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
>> otherwise it gets skipped.
>>
>> - For new interrupt-based notification, add a new test, asyncpf-int.c, to check
>> (KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
>> 'page-ready' handler.
> Using #PF to deliver page-ready is completely dead, no? Unless I'm mistaken, let's
> just drop the existing support and replace it with the interrupted-based mechanism.
> I see no reason to continue maintaining the old crud. If someone wants to verify
> an old, broken KVM, then they can use the old version of KUT.
Yes, since Linux v5.10 the feature asyncpf is deprecated.
So, just drop asyncpf.c and add asyncpf_int.c is enough, right?
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues
2023-12-13 1:36 ` Wu, Dan1
@ 2023-12-13 18:20 ` Sean Christopherson
2023-12-14 2:32 ` Wu, Dan1
0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2023-12-13 18:20 UTC (permalink / raw)
To: Dan1 Wu; +Cc: pbonzini, kvm, xiaoyao.li
On Wed, Dec 13, 2023, Dan1 Wu wrote:
>
> On 12/12/2023 11:17 PM, Sean Christopherson wrote:
> > On Tue, Dec 12, 2023, Dan Wu wrote:
> > > When running asyncpf test, it gets skipped without a clear reason:
> > >
> > > ./asyncpf
> > >
> > > enabling apic
> > > smp: waiting for 0 APs
> > > paging enabled
> > > cr0 = 80010011
> > > cr3 = 107f000
> > > cr4 = 20
> > > install handler
> > > enable async pf
> > > alloc memory
> > > start loop
> > > end loop
> > > start loop
> > > end loop
> > > SUMMARY: 0 tests
> > > SKIP asyncpf (0 tests)
> > >
> > > The reason is that KVM changed to use interrupt-based 'page-ready' notification
> > > and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
> > > 'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
> > > in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
> > >
> > > This series tries to fix the problem by separating two testcases for different mechanisms.
> > >
> > > - For old #PF-based notification, changes current asyncpf.c to add CPUID check
> > > at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
> > > otherwise it gets skipped.
> > >
> > > - For new interrupt-based notification, add a new test, asyncpf-int.c, to check
> > > (KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
> > > 'page-ready' handler.
> > Using #PF to deliver page-ready is completely dead, no? Unless I'm mistaken, let's
> > just drop the existing support and replace it with the interrupted-based mechanism.
> > I see no reason to continue maintaining the old crud. If someone wants to verify
> > an old, broken KVM, then they can use the old version of KUT.
>
> Yes, since Linux v5.10 the feature asyncpf is deprecated.
>
> So, just drop asyncpf.c and add asyncpf_int.c is enough, right?
I would rather not add asyncpf_int.c, and instead keep asyncpf.c and modify it to
use ASYNC_PF_INT. It _might_ be a bit more churn, but modifying the existing code
instead of dropping in a new file will better preserve the history, and may also
allow for finer grained patches (not sure on that one).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues
2023-12-13 18:20 ` Sean Christopherson
@ 2023-12-14 2:32 ` Wu, Dan1
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Dan1 @ 2023-12-14 2:32 UTC (permalink / raw)
To: Sean Christopherson; +Cc: pbonzini, kvm, xiaoyao.li
On 12/14/2023 2:20 AM, Sean Christopherson wrote:
> On Wed, Dec 13, 2023, Dan1 Wu wrote:
>> On 12/12/2023 11:17 PM, Sean Christopherson wrote:
>>> On Tue, Dec 12, 2023, Dan Wu wrote:
>>>> When running asyncpf test, it gets skipped without a clear reason:
>>>>
>>>> ./asyncpf
>>>>
>>>> enabling apic
>>>> smp: waiting for 0 APs
>>>> paging enabled
>>>> cr0 = 80010011
>>>> cr3 = 107f000
>>>> cr4 = 20
>>>> install handler
>>>> enable async pf
>>>> alloc memory
>>>> start loop
>>>> end loop
>>>> start loop
>>>> end loop
>>>> SUMMARY: 0 tests
>>>> SKIP asyncpf (0 tests)
>>>>
>>>> The reason is that KVM changed to use interrupt-based 'page-ready' notification
>>>> and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
>>>> 'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
>>>> in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
>>>>
>>>> This series tries to fix the problem by separating two testcases for different mechanisms.
>>>>
>>>> - For old #PF-based notification, changes current asyncpf.c to add CPUID check
>>>> at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
>>>> otherwise it gets skipped.
>>>>
>>>> - For new interrupt-based notification, add a new test, asyncpf-int.c, to check
>>>> (KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
>>>> 'page-ready' handler.
>>> Using #PF to deliver page-ready is completely dead, no? Unless I'm mistaken, let's
>>> just drop the existing support and replace it with the interrupted-based mechanism.
>>> I see no reason to continue maintaining the old crud. If someone wants to verify
>>> an old, broken KVM, then they can use the old version of KUT.
>> Yes, since Linux v5.10 the feature asyncpf is deprecated.
>>
>> So, just drop asyncpf.c and add asyncpf_int.c is enough, right?
> I would rather not add asyncpf_int.c, and instead keep asyncpf.c and modify it to
> use ASYNC_PF_INT. It _might_ be a bit more churn, but modifying the existing code
> instead of dropping in a new file will better preserve the history, and may also
> allow for finer grained patches (not sure on that one).
ok, I will modify it in the next version. Thanks for your review.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-14 2:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-12 6:27 [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 1/3] x86: Add a common header asyncpf.h Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 2/3] x86: Add async page fault int test Dan Wu
2023-12-12 6:27 ` [kvm-unit-tests PATCH v1 3/3] x86/asyncpf: Add CPUID feature bits check to ensure feature is available Dan Wu
2023-12-12 15:17 ` [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues Sean Christopherson
2023-12-13 1:36 ` Wu, Dan1
2023-12-13 18:20 ` Sean Christopherson
2023-12-14 2:32 ` Wu, Dan1
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox