[PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[wu.fei9]

Currently dirty_log_test hardcodes usleep 1ms in each interval, which
could be too short for guest to write and fault in enough pages, then
there is less chance to test the write protection mechanism, especially
in the case of (log_mode != LOG_MODE_DIRTY_RING).

Unit is introduced to replace the default 1ms if specified in command
line. The following test can't trigger failure on my riscv vm:

  # ./dirty_log_test -m 21 -M clear-log

By enlarging unit, it fails every time:

  # ./dirty_log_test -u 100 -m 21 -M clear-log
  Random seed: 0x6b8b4567
  Setting log mode to: 'clear-log'
  Test iterations: 32, interval: 10, unit: 100 (ms)
  Testing guest mode: PA-bits:50,  VA-bits:48,  4K pages
  guest physical test memory offset: 0x3ffffbfffc000
  Iteration  1: dirty: 262147 clean: 0      writes: 8454
  ==== Test Assertion Failure ====
    dirty_log_test.c:565: val < iteration
    pid=295 tid=295 errno=0 - Success
  sh: addr2line: not found
    Clear page 197160 value (2) >= iteration (2) (last = 18446744073709551615, prev_last = 18446744073709551615)

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 tools/testing/selftests/kvm/dirty_log_test.c | 24 ++++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..06234a77f1c5 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -34,9 +34,12 @@
 /* How many host loops to run (one KVM_GET_DIRTY_LOG for each loop) */
 #define TEST_HOST_LOOP_N               32UL

-/* Interval for each host loop (ms) */
+/* Interval for each host loop (number of units) */
 #define TEST_HOST_LOOP_INTERVAL                10UL

+/* ms per unit, one iteration takes (interval * unit) ms */
+#define TEST_HOST_LOOP_UNIT            1UL
+
 /*
  * Ensure the vCPU is able to perform a reasonable number of writes in each
  * iteration to provide a lower bound on coverage.
@@ -592,6 +595,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
 struct test_params {
        unsigned long iterations;
        unsigned long interval;
+       unsigned long unit;
        u64 phys_offset;
 };

@@ -718,7 +722,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * and so that the test doesn't run too far beyond the
                         * specified interval.
                         */
-                       usleep(1000);
+                       usleep(1000 * p->unit);

                        sync_global_from_guest(vm, nr_writes);

@@ -807,7 +811,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 static void help(char *name)
 {
        puts("");
-       printf("usage: %s [-h] [-i iterations] [-I interval] "
+       printf("usage: %s [-h] [-i iterations] [-I interval] [-u unit] "
               "[-p offset] [-m mode]\n", name);
        puts("");
        printf(" -c: hint to dirty ring size, in number of entries\n");
@@ -815,8 +819,9 @@ static void help(char *name)
               TEST_DIRTY_RING_COUNT);
        printf(" -i: specify iteration counts (default: %"PRIu64")\n",
               TEST_HOST_LOOP_N);
-       printf(" -I: specify interval in ms (default: %"PRIu64" ms)\n",
+       printf(" -I: specify interval in unit (default: %"PRIu64")\n",
               TEST_HOST_LOOP_INTERVAL);
+       printf(" -u: specify unit in ms (default: 1)\n");
        printf(" -p: specify guest physical test memory offset\n"
               "     Warning: a low offset can conflict with the loaded test code.\n");
        printf(" -M: specify the host logging mode "
@@ -832,6 +837,7 @@ int main(int argc, char *argv[])
        struct test_params p = {
                .iterations = TEST_HOST_LOOP_N,
                .interval = TEST_HOST_LOOP_INTERVAL,
+               .unit = TEST_HOST_LOOP_UNIT,
        };
        int opt, i;

@@ -840,7 +846,7 @@ int main(int argc, char *argv[])

        guest_modes_append_default();

-       while ((opt = getopt(argc, argv, "c:hi:I:p:m:M:")) != -1) {
+       while ((opt = getopt(argc, argv, "c:hi:I:u:p:m:M:")) != -1) {
                switch (opt) {
                case 'c':
                        test_dirty_ring_count = strtol(optarg, NULL, 10);
@@ -851,6 +857,9 @@ int main(int argc, char *argv[])
                case 'I':
                        p.interval = strtol(optarg, NULL, 10);
                        break;
+               case 'u':
+                       p.unit = strtol(optarg, NULL, 10);
+                       break;
                case 'p':
                        p.phys_offset = strtoull(optarg, NULL, 0);
                        break;
@@ -886,9 +895,10 @@ int main(int argc, char *argv[])

        TEST_ASSERT(p.iterations > 0, "Iterations must be greater than zero");
        TEST_ASSERT(p.interval > 0, "Interval must be greater than zero");
+       TEST_ASSERT(p.unit > 0, "Unit must be greater than zero");

-       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
-               p.iterations, p.interval);
+       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64", unit: %"PRIu64" (ms)\n",
+               p.iterations, p.interval, p.unit);

        if (host_log_mode_option == LOG_MODE_ALL) {
                /* Run each log mode */
--
2.43.0

[PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[wu.fei9]

Currently dirty_log_test hardcodes usleep 1ms in each interval, which
could be too short for guest to write and fault in enough pages, then
there is less chance to test the write protection mechanism, especially
in the case of (log_mode != LOG_MODE_DIRTY_RING).

Unit is introduced to replace the default 1ms if specified in command
line. The following test can't trigger failure on my riscv vm:

  # ./dirty_log_test -m 21 -M clear-log

By enlarging unit, it fails every time:

  # ./dirty_log_test -u 100 -m 21 -M clear-log
  Random seed: 0x6b8b4567
  Setting log mode to: 'clear-log'
  Test iterations: 32, interval: 10, unit: 100 (ms)
  Testing guest mode: PA-bits:50,  VA-bits:48,  4K pages
  guest physical test memory offset: 0x3ffffbfffc000
  Iteration  1: dirty: 262147 clean: 0      writes: 8454
  ==== Test Assertion Failure ====
    dirty_log_test.c:565: val < iteration
    pid=295 tid=295 errno=0 - Success
  sh: addr2line: not found
    Clear page 197160 value (2) >= iteration (2) (last = 18446744073709551615, prev_last = 18446744073709551615)

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 tools/testing/selftests/kvm/dirty_log_test.c | 24 ++++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..06234a77f1c5 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -34,9 +34,12 @@
 /* How many host loops to run (one KVM_GET_DIRTY_LOG for each loop) */
 #define TEST_HOST_LOOP_N               32UL

-/* Interval for each host loop (ms) */
+/* Interval for each host loop (number of units) */
 #define TEST_HOST_LOOP_INTERVAL                10UL

+/* ms per unit, one iteration takes (interval * unit) ms */
+#define TEST_HOST_LOOP_UNIT            1UL
+
 /*
  * Ensure the vCPU is able to perform a reasonable number of writes in each
  * iteration to provide a lower bound on coverage.
@@ -592,6 +595,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
 struct test_params {
        unsigned long iterations;
        unsigned long interval;
+       unsigned long unit;
        u64 phys_offset;
 };

@@ -718,7 +722,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * and so that the test doesn't run too far beyond the
                         * specified interval.
                         */
-                       usleep(1000);
+                       usleep(1000 * p->unit);

                        sync_global_from_guest(vm, nr_writes);

@@ -807,7 +811,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 static void help(char *name)
 {
        puts("");
-       printf("usage: %s [-h] [-i iterations] [-I interval] "
+       printf("usage: %s [-h] [-i iterations] [-I interval] [-u unit] "
               "[-p offset] [-m mode]\n", name);
        puts("");
        printf(" -c: hint to dirty ring size, in number of entries\n");
@@ -815,8 +819,9 @@ static void help(char *name)
               TEST_DIRTY_RING_COUNT);
        printf(" -i: specify iteration counts (default: %"PRIu64")\n",
               TEST_HOST_LOOP_N);
-       printf(" -I: specify interval in ms (default: %"PRIu64" ms)\n",
+       printf(" -I: specify interval in unit (default: %"PRIu64")\n",
               TEST_HOST_LOOP_INTERVAL);
+       printf(" -u: specify unit in ms (default: 1)\n");
        printf(" -p: specify guest physical test memory offset\n"
               "     Warning: a low offset can conflict with the loaded test code.\n");
        printf(" -M: specify the host logging mode "
@@ -832,6 +837,7 @@ int main(int argc, char *argv[])
        struct test_params p = {
                .iterations = TEST_HOST_LOOP_N,
                .interval = TEST_HOST_LOOP_INTERVAL,
+               .unit = TEST_HOST_LOOP_UNIT,
        };
        int opt, i;

@@ -840,7 +846,7 @@ int main(int argc, char *argv[])

        guest_modes_append_default();

-       while ((opt = getopt(argc, argv, "c:hi:I:p:m:M:")) != -1) {
+       while ((opt = getopt(argc, argv, "c:hi:I:u:p:m:M:")) != -1) {
                switch (opt) {
                case 'c':
                        test_dirty_ring_count = strtol(optarg, NULL, 10);
@@ -851,6 +857,9 @@ int main(int argc, char *argv[])
                case 'I':
                        p.interval = strtol(optarg, NULL, 10);
                        break;
+               case 'u':
+                       p.unit = strtol(optarg, NULL, 10);
+                       break;
                case 'p':
                        p.phys_offset = strtoull(optarg, NULL, 0);
                        break;
@@ -886,9 +895,10 @@ int main(int argc, char *argv[])

        TEST_ASSERT(p.iterations > 0, "Iterations must be greater than zero");
        TEST_ASSERT(p.interval > 0, "Interval must be greater than zero");
+       TEST_ASSERT(p.unit > 0, "Unit must be greater than zero");

-       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
-               p.iterations, p.interval);
+       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64", unit: %"PRIu64" (ms)\n",
+               p.iterations, p.interval, p.unit);

        if (host_log_mode_option == LOG_MODE_ALL) {
                /* Run each log mode */
--
2.43.0

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[wu.fei9]

Currently dirty_log_test hardcodes usleep 1ms in each interval, which
could be too short for guest to write and fault in enough pages, then
there is less chance to test the write protection mechanism, especially
in the case of (log_mode != LOG_MODE_DIRTY_RING).

Unit is introduced to replace the default 1ms if specified in command
line. The following test can't trigger failure on my riscv vm:

  # ./dirty_log_test -m 21 -M clear-log

By enlarging unit, it fails every time:

  # ./dirty_log_test -u 100 -m 21 -M clear-log
  Random seed: 0x6b8b4567
  Setting log mode to: 'clear-log'
  Test iterations: 32, interval: 10, unit: 100 (ms)
  Testing guest mode: PA-bits:50,  VA-bits:48,  4K pages
  guest physical test memory offset: 0x3ffffbfffc000
  Iteration  1: dirty: 262147 clean: 0      writes: 8454
  ==== Test Assertion Failure ====
    dirty_log_test.c:565: val < iteration
    pid=295 tid=295 errno=0 - Success
  sh: addr2line: not found
    Clear page 197160 value (2) >= iteration (2) (last = 18446744073709551615, prev_last = 18446744073709551615)

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 tools/testing/selftests/kvm/dirty_log_test.c | 24 ++++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..06234a77f1c5 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -34,9 +34,12 @@
 /* How many host loops to run (one KVM_GET_DIRTY_LOG for each loop) */
 #define TEST_HOST_LOOP_N               32UL

-/* Interval for each host loop (ms) */
+/* Interval for each host loop (number of units) */
 #define TEST_HOST_LOOP_INTERVAL                10UL

+/* ms per unit, one iteration takes (interval * unit) ms */
+#define TEST_HOST_LOOP_UNIT            1UL
+
 /*
  * Ensure the vCPU is able to perform a reasonable number of writes in each
  * iteration to provide a lower bound on coverage.
@@ -592,6 +595,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
 struct test_params {
        unsigned long iterations;
        unsigned long interval;
+       unsigned long unit;
        u64 phys_offset;
 };

@@ -718,7 +722,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * and so that the test doesn't run too far beyond the
                         * specified interval.
                         */
-                       usleep(1000);
+                       usleep(1000 * p->unit);

                        sync_global_from_guest(vm, nr_writes);

@@ -807,7 +811,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 static void help(char *name)
 {
        puts("");
-       printf("usage: %s [-h] [-i iterations] [-I interval] "
+       printf("usage: %s [-h] [-i iterations] [-I interval] [-u unit] "
               "[-p offset] [-m mode]\n", name);
        puts("");
        printf(" -c: hint to dirty ring size, in number of entries\n");
@@ -815,8 +819,9 @@ static void help(char *name)
               TEST_DIRTY_RING_COUNT);
        printf(" -i: specify iteration counts (default: %"PRIu64")\n",
               TEST_HOST_LOOP_N);
-       printf(" -I: specify interval in ms (default: %"PRIu64" ms)\n",
+       printf(" -I: specify interval in unit (default: %"PRIu64")\n",
               TEST_HOST_LOOP_INTERVAL);
+       printf(" -u: specify unit in ms (default: 1)\n");
        printf(" -p: specify guest physical test memory offset\n"
               "     Warning: a low offset can conflict with the loaded test code.\n");
        printf(" -M: specify the host logging mode "
@@ -832,6 +837,7 @@ int main(int argc, char *argv[])
        struct test_params p = {
                .iterations = TEST_HOST_LOOP_N,
                .interval = TEST_HOST_LOOP_INTERVAL,
+               .unit = TEST_HOST_LOOP_UNIT,
        };
        int opt, i;

@@ -840,7 +846,7 @@ int main(int argc, char *argv[])

        guest_modes_append_default();

-       while ((opt = getopt(argc, argv, "c:hi:I:p:m:M:")) != -1) {
+       while ((opt = getopt(argc, argv, "c:hi:I:u:p:m:M:")) != -1) {
                switch (opt) {
                case 'c':
                        test_dirty_ring_count = strtol(optarg, NULL, 10);
@@ -851,6 +857,9 @@ int main(int argc, char *argv[])
                case 'I':
                        p.interval = strtol(optarg, NULL, 10);
                        break;
+               case 'u':
+                       p.unit = strtol(optarg, NULL, 10);
+                       break;
                case 'p':
                        p.phys_offset = strtoull(optarg, NULL, 0);
                        break;
@@ -886,9 +895,10 @@ int main(int argc, char *argv[])

        TEST_ASSERT(p.iterations > 0, "Iterations must be greater than zero");
        TEST_ASSERT(p.interval > 0, "Interval must be greater than zero");
+       TEST_ASSERT(p.unit > 0, "Unit must be greater than zero");

-       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
-               p.iterations, p.interval);
+       pr_info("Test iterations: %"PRIu64", interval: %"PRIu64", unit: %"PRIu64" (ms)\n",
+               p.iterations, p.interval, p.unit);

        if (host_log_mode_option == LOG_MODE_ALL) {
                /* Run each log mode */
--
2.43.0

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> could be too short for guest to write and fault in enough pages, then
> there is less chance to test the write protection mechanism, especially
> in the case of (log_mode != LOG_MODE_DIRTY_RING).

But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
meaningless, because the test won't reap the bitmaps for iterations > 0.

			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
				continue;

>
> Unit is introduced to replace the default 1ms if specified in command
> line. The following test can't trigger failure on my riscv vm:

Failure of what?  And does the failure really not reproduce with a higher interval?

> 
>   # ./dirty_log_test -m 21 -M clear-log
> 
> By enlarging unit, it fails every time:

"unit" is way too vague.   Even looking at the code, it's not clear to me what
you intended a "unit" to be.  I'm not entirely opposed to providing the user with
more control over the innards of the test, but the interface needs to be somewhat
intuitive.

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> could be too short for guest to write and fault in enough pages, then
> there is less chance to test the write protection mechanism, especially
> in the case of (log_mode != LOG_MODE_DIRTY_RING).

But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
meaningless, because the test won't reap the bitmaps for iterations > 0.

			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
				continue;

>
> Unit is introduced to replace the default 1ms if specified in command
> line. The following test can't trigger failure on my riscv vm:

Failure of what?  And does the failure really not reproduce with a higher interval?

> 
>   # ./dirty_log_test -m 21 -M clear-log
> 
> By enlarging unit, it fails every time:

"unit" is way too vague.   Even looking at the code, it's not clear to me what
you intended a "unit" to be.  I'm not entirely opposed to providing the user with
more control over the innards of the test, but the interface needs to be somewhat
intuitive.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> could be too short for guest to write and fault in enough pages, then
> there is less chance to test the write protection mechanism, especially
> in the case of (log_mode != LOG_MODE_DIRTY_RING).

But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
meaningless, because the test won't reap the bitmaps for iterations > 0.

			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
				continue;

>
> Unit is introduced to replace the default 1ms if specified in command
> line. The following test can't trigger failure on my riscv vm:

Failure of what?  And does the failure really not reproduce with a higher interval?

> 
>   # ./dirty_log_test -m 21 -M clear-log
> 
> By enlarging unit, it fails every time:

"unit" is way too vague.   Even looking at the code, it's not clear to me what
you intended a "unit" to be.  I'm not entirely opposed to providing the user with
more control over the innards of the test, but the interface needs to be somewhat
intuitive.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/13/26 08:03, Sean Christopherson wrote:
> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>> could be too short for guest to write and fault in enough pages, then
>> there is less chance to test the write protection mechanism, especially
>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
> 
> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> meaningless, because the test won't reap the bitmaps for iterations > 0.
> 
> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> 				continue;
> 
The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. 
The dirty bitmap is not precise in the first get_dirty_log, all pages 
are marked as dirty but most of them are not populated in page table, 
this creates the situation I mentioned in the cover letter.

"when the range to wp is
[0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
returns false and page_size is 2MB, it skips the whole range, but it's
possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."

>>
>> Unit is introduced to replace the default 1ms if specified in command
>> line. The following test can't trigger failure on my riscv vm:
> 
> Failure of what?  And does the failure really not reproduce with a higher interval?

On riscv, it fails to write protect some pages with valid page table 
entry then loses track of dirty pages. Higher interval doesn't help 
because only the first usleep matters, after the first 
collect_dirty_pages, all dirty pages are tracked precisely then there is 
no such problem.

> 
>>
>>    # ./dirty_log_test -m 21 -M clear-log
>>
>> By enlarging unit, it fails every time:
> 
> "unit" is way too vague.   Even looking at the code, it's not clear to me what
> you intended a "unit" to be.  I'm not entirely opposed to providing the user with
> more control over the innards of the test, but the interface needs to be somewhat
> intuitive.

Basically, a unit is the time of one usleep(), any suggestions?

Thanks,
Fei.

> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/13/26 08:03, Sean Christopherson wrote:
> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>> could be too short for guest to write and fault in enough pages, then
>> there is less chance to test the write protection mechanism, especially
>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
> 
> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> meaningless, because the test won't reap the bitmaps for iterations > 0.
> 
> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> 				continue;
> 
The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. 
The dirty bitmap is not precise in the first get_dirty_log, all pages 
are marked as dirty but most of them are not populated in page table, 
this creates the situation I mentioned in the cover letter.

"when the range to wp is
[0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
returns false and page_size is 2MB, it skips the whole range, but it's
possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."

>>
>> Unit is introduced to replace the default 1ms if specified in command
>> line. The following test can't trigger failure on my riscv vm:
> 
> Failure of what?  And does the failure really not reproduce with a higher interval?

On riscv, it fails to write protect some pages with valid page table 
entry then loses track of dirty pages. Higher interval doesn't help 
because only the first usleep matters, after the first 
collect_dirty_pages, all dirty pages are tracked precisely then there is 
no such problem.

> 
>>
>>    # ./dirty_log_test -m 21 -M clear-log
>>
>> By enlarging unit, it fails every time:
> 
> "unit" is way too vague.   Even looking at the code, it's not clear to me what
> you intended a "unit" to be.  I'm not entirely opposed to providing the user with
> more control over the innards of the test, but the interface needs to be somewhat
> intuitive.

Basically, a unit is the time of one usleep(), any suggestions?

Thanks,
Fei.

> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/13/26 08:03, Sean Christopherson wrote:
> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>> could be too short for guest to write and fault in enough pages, then
>> there is less chance to test the write protection mechanism, especially
>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
> 
> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> meaningless, because the test won't reap the bitmaps for iterations > 0.
> 
> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> 				continue;
> 
The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. 
The dirty bitmap is not precise in the first get_dirty_log, all pages 
are marked as dirty but most of them are not populated in page table, 
this creates the situation I mentioned in the cover letter.

"when the range to wp is
[0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
returns false and page_size is 2MB, it skips the whole range, but it's
possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."

>>
>> Unit is introduced to replace the default 1ms if specified in command
>> line. The following test can't trigger failure on my riscv vm:
> 
> Failure of what?  And does the failure really not reproduce with a higher interval?

On riscv, it fails to write protect some pages with valid page table 
entry then loses track of dirty pages. Higher interval doesn't help 
because only the first usleep matters, after the first 
collect_dirty_pages, all dirty pages are tracked precisely then there is 
no such problem.

> 
>>
>>    # ./dirty_log_test -m 21 -M clear-log
>>
>> By enlarging unit, it fails every time:
> 
> "unit" is way too vague.   Even looking at the code, it's not clear to me what
> you intended a "unit" to be.  I'm not entirely opposed to providing the user with
> more control over the innards of the test, but the interface needs to be somewhat
> intuitive.

Basically, a unit is the time of one usleep(), any suggestions?

Thanks,
Fei.

> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Wed, May 13, 2026, Wu Fei wrote:
> On 5/13/26 08:03, Sean Christopherson wrote:
> > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> > > could be too short for guest to write and fault in enough pages, then
> > > there is less chance to test the write protection mechanism, especially
> > > in the case of (log_mode != LOG_MODE_DIRTY_RING).
> > 
> > But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> > meaningless, because the test won't reap the bitmaps for iterations > 0.
> > 
> > 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> > 				continue;
> > 
> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
> as dirty but most of them are not populated in page table, this creates the
> situation I mentioned in the cover letter.

I suspect something is messed up in your workflow, because the actual patches
aren't properly threaded with respect to the cover letter.  E.g. patch 1 has

  In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>

but the cover letter has:

  Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>

Copy+pasting the entirety of the cover letter for reference:

 : The current gstage range walker unconditionally advances by 'page_size'
 : when a leaf PTE is not found, e.g. when the range to wp is
 : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
 : returns false and page_size is 2MB, it skips the whole range, but it's
 : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
 : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
 : wp/unamp have the same pattern.
 : 
 : dirty_log_test intentionally sets up the unaligned guest physical
 : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
 : to trigger this bug if there is a larger window for guest to write more
 : pages before first collect_dirty_pages.

> "when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
> returns false and page_size is 2MB, it skips the whole range, but it's
> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
> 
> > > 
> > > Unit is introduced to replace the default 1ms if specified in command
> > > line. The following test can't trigger failure on my riscv vm:
> > 
> > Failure of what?  And does the failure really not reproduce with a higher interval?
> 
> On riscv, it fails to write protect some pages with valid page table entry
> then loses track of dirty pages. Higher interval doesn't help because only
> the first usleep matters, after the first collect_dirty_pages, all dirty
> pages are tracked precisely then there is no such problem.

Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
what if we instead randomize the delay before the initial reaping of the dirty
bitmap/ring?  That should provide a good balance between coverage, complexity and
user-friendliness.

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..74ca096bf976 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
        pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
 
        for (iteration = 1; iteration <= p->iterations; iteration++) {
-               unsigned long i;
+               unsigned long i, reap_i;
+
+               /*
+                * Select a random point in the time interval to reap the dirty
+                * bitmap/ring while the guest is running, i.e. randomize how
+                * long the guest gets to initially run and thus how many pages
+                * it can dirty, before collecting the dirty bitmap/ring.  See
+                * the loop below for details.
+                */
+               reap_i = random() % p->interval;
+               printf("Reaping after a %lu ms delay\n", reap_i);
 
                sync_global_to_guest(vm, iteration);
 
@@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * that's effectively blocked.  Collecting while the
                         * guest is running also verifies KVM doesn't lose any
                         * state.
-                        *
+                        */
+                       if (i < reap_i)
+                               continue;
+
+                       /*
                         * For bitmap modes, KVM overwrites the entire bitmap,
                         * i.e. collecting the bitmaps is destructive.  Collect
-                        * the bitmap only on the first pass, otherwise this
-                        * test would lose track of dirty pages.
+                        * the bitmap while the guest is running only once,
+                        * otherwise this test would lose track of dirty pages.
                         */
-                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
+                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
                                continue;
 
                        /*
@@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * the ring on every pass would make it unlikely the
                         * vCPU would ever fill the fing).
                         */
-                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
+                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
                                continue;
 
                        log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Wed, May 13, 2026, Wu Fei wrote:
> On 5/13/26 08:03, Sean Christopherson wrote:
> > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> > > could be too short for guest to write and fault in enough pages, then
> > > there is less chance to test the write protection mechanism, especially
> > > in the case of (log_mode != LOG_MODE_DIRTY_RING).
> > 
> > But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> > meaningless, because the test won't reap the bitmaps for iterations > 0.
> > 
> > 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> > 				continue;
> > 
> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
> as dirty but most of them are not populated in page table, this creates the
> situation I mentioned in the cover letter.

I suspect something is messed up in your workflow, because the actual patches
aren't properly threaded with respect to the cover letter.  E.g. patch 1 has

  In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>

but the cover letter has:

  Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>

Copy+pasting the entirety of the cover letter for reference:

 : The current gstage range walker unconditionally advances by 'page_size'
 : when a leaf PTE is not found, e.g. when the range to wp is
 : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
 : returns false and page_size is 2MB, it skips the whole range, but it's
 : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
 : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
 : wp/unamp have the same pattern.
 : 
 : dirty_log_test intentionally sets up the unaligned guest physical
 : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
 : to trigger this bug if there is a larger window for guest to write more
 : pages before first collect_dirty_pages.

> "when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
> returns false and page_size is 2MB, it skips the whole range, but it's
> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
> 
> > > 
> > > Unit is introduced to replace the default 1ms if specified in command
> > > line. The following test can't trigger failure on my riscv vm:
> > 
> > Failure of what?  And does the failure really not reproduce with a higher interval?
> 
> On riscv, it fails to write protect some pages with valid page table entry
> then loses track of dirty pages. Higher interval doesn't help because only
> the first usleep matters, after the first collect_dirty_pages, all dirty
> pages are tracked precisely then there is no such problem.

Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
what if we instead randomize the delay before the initial reaping of the dirty
bitmap/ring?  That should provide a good balance between coverage, complexity and
user-friendliness.

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..74ca096bf976 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
        pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
 
        for (iteration = 1; iteration <= p->iterations; iteration++) {
-               unsigned long i;
+               unsigned long i, reap_i;
+
+               /*
+                * Select a random point in the time interval to reap the dirty
+                * bitmap/ring while the guest is running, i.e. randomize how
+                * long the guest gets to initially run and thus how many pages
+                * it can dirty, before collecting the dirty bitmap/ring.  See
+                * the loop below for details.
+                */
+               reap_i = random() % p->interval;
+               printf("Reaping after a %lu ms delay\n", reap_i);
 
                sync_global_to_guest(vm, iteration);
 
@@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * that's effectively blocked.  Collecting while the
                         * guest is running also verifies KVM doesn't lose any
                         * state.
-                        *
+                        */
+                       if (i < reap_i)
+                               continue;
+
+                       /*
                         * For bitmap modes, KVM overwrites the entire bitmap,
                         * i.e. collecting the bitmaps is destructive.  Collect
-                        * the bitmap only on the first pass, otherwise this
-                        * test would lose track of dirty pages.
+                        * the bitmap while the guest is running only once,
+                        * otherwise this test would lose track of dirty pages.
                         */
-                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
+                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
                                continue;
 
                        /*
@@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * the ring on every pass would make it unlikely the
                         * vCPU would ever fill the fing).
                         */
-                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
+                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
                                continue;
 
                        log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Wed, May 13, 2026, Wu Fei wrote:
> On 5/13/26 08:03, Sean Christopherson wrote:
> > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > Currently dirty_log_test hardcodes usleep 1ms in each interval, which
> > > could be too short for guest to write and fault in enough pages, then
> > > there is less chance to test the write protection mechanism, especially
> > > in the case of (log_mode != LOG_MODE_DIRTY_RING).
> > 
> > But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
> > meaningless, because the test won't reap the bitmaps for iterations > 0.
> > 
> > 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> > 				continue;
> > 
> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
> as dirty but most of them are not populated in page table, this creates the
> situation I mentioned in the cover letter.

I suspect something is messed up in your workflow, because the actual patches
aren't properly threaded with respect to the cover letter.  E.g. patch 1 has

  In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>

but the cover letter has:

  Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>

Copy+pasting the entirety of the cover letter for reference:

 : The current gstage range walker unconditionally advances by 'page_size'
 : when a leaf PTE is not found, e.g. when the range to wp is
 : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
 : returns false and page_size is 2MB, it skips the whole range, but it's
 : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
 : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
 : wp/unamp have the same pattern.
 : 
 : dirty_log_test intentionally sets up the unaligned guest physical
 : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
 : to trigger this bug if there is a larger window for guest to write more
 : pages before first collect_dirty_pages.

> "when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
> returns false and page_size is 2MB, it skips the whole range, but it's
> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
> 
> > > 
> > > Unit is introduced to replace the default 1ms if specified in command
> > > line. The following test can't trigger failure on my riscv vm:
> > 
> > Failure of what?  And does the failure really not reproduce with a higher interval?
> 
> On riscv, it fails to write protect some pages with valid page table entry
> then loses track of dirty pages. Higher interval doesn't help because only
> the first usleep matters, after the first collect_dirty_pages, all dirty
> pages are tracked precisely then there is no such problem.

Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
what if we instead randomize the delay before the initial reaping of the dirty
bitmap/ring?  That should provide a good balance between coverage, complexity and
user-friendliness.

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12446a4b6e8d..74ca096bf976 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
        pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
 
        for (iteration = 1; iteration <= p->iterations; iteration++) {
-               unsigned long i;
+               unsigned long i, reap_i;
+
+               /*
+                * Select a random point in the time interval to reap the dirty
+                * bitmap/ring while the guest is running, i.e. randomize how
+                * long the guest gets to initially run and thus how many pages
+                * it can dirty, before collecting the dirty bitmap/ring.  See
+                * the loop below for details.
+                */
+               reap_i = random() % p->interval;
+               printf("Reaping after a %lu ms delay\n", reap_i);
 
                sync_global_to_guest(vm, iteration);
 
@@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * that's effectively blocked.  Collecting while the
                         * guest is running also verifies KVM doesn't lose any
                         * state.
-                        *
+                        */
+                       if (i < reap_i)
+                               continue;
+
+                       /*
                         * For bitmap modes, KVM overwrites the entire bitmap,
                         * i.e. collecting the bitmaps is destructive.  Collect
-                        * the bitmap only on the first pass, otherwise this
-                        * test would lose track of dirty pages.
+                        * the bitmap while the guest is running only once,
+                        * otherwise this test would lose track of dirty pages.
                         */
-                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
+                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
                                continue;
 
                        /*
@@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                         * the ring on every pass would make it unlikely the
                         * vCPU would ever fill the fing).
                         */
-                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
+                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
                                continue;
 
                        log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/15/26 21:51, Sean Christopherson wrote:
> On Wed, May 13, 2026, Wu Fei wrote:
>> On 5/13/26 08:03, Sean Christopherson wrote:
>>> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>>>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>>>> could be too short for guest to write and fault in enough pages, then
>>>> there is less chance to test the write protection mechanism, especially
>>>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
>>>
>>> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
>>> meaningless, because the test won't reap the bitmaps for iterations > 0.
>>>
>>> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
>>> 				continue;
>>>
>> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
>> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
>> as dirty but most of them are not populated in page table, this creates the
>> situation I mentioned in the cover letter.
> 
> I suspect something is messed up in your workflow, because the actual patches
> aren't properly threaded with respect to the cover letter.  E.g. patch 1 has
> 
>    In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>
> 
> but the cover letter has:
> 
>    Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>
> 
> Copy+pasting the entirety of the cover letter for reference:
> 
>   : The current gstage range walker unconditionally advances by 'page_size'
>   : when a leaf PTE is not found, e.g. when the range to wp is
>   : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>   : returns false and page_size is 2MB, it skips the whole range, but it's
>   : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>   : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
>   : wp/unamp have the same pattern.
>   :
>   : dirty_log_test intentionally sets up the unaligned guest physical
>   : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
>   : to trigger this bug if there is a larger window for guest to write more
>   : pages before first collect_dirty_pages.
> 
>> "when the range to wp is
>> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>> returns false and page_size is 2MB, it skips the whole range, but it's
>> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
>>
>>>>
>>>> Unit is introduced to replace the default 1ms if specified in command
>>>> line. The following test can't trigger failure on my riscv vm:
>>>
>>> Failure of what?  And does the failure really not reproduce with a higher interval?
>>
>> On riscv, it fails to write protect some pages with valid page table entry
>> then loses track of dirty pages. Higher interval doesn't help because only
>> the first usleep matters, after the first collect_dirty_pages, all dirty
>> pages are tracked precisely then there is no such problem.
> 
> Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> what if we instead randomize the delay before the initial reaping of the dirty
> bitmap/ring?  That should provide a good balance between coverage, complexity and
> user-friendliness.

I'm fine with your solution, I applied it and it did trigger the same issue.

Thanks,
Fei.

> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 12446a4b6e8d..74ca096bf976 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>          pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
>   
>          for (iteration = 1; iteration <= p->iterations; iteration++) {
> -               unsigned long i;
> +               unsigned long i, reap_i;
> +
> +               /*
> +                * Select a random point in the time interval to reap the dirty
> +                * bitmap/ring while the guest is running, i.e. randomize how
> +                * long the guest gets to initially run and thus how many pages
> +                * it can dirty, before collecting the dirty bitmap/ring.  See
> +                * the loop below for details.
> +                */
> +               reap_i = random() % p->interval;
> +               printf("Reaping after a %lu ms delay\n", reap_i);
>   
>                  sync_global_to_guest(vm, iteration);
>   
> @@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * that's effectively blocked.  Collecting while the
>                           * guest is running also verifies KVM doesn't lose any
>                           * state.
> -                        *
> +                        */
> +                       if (i < reap_i)
> +                               continue;
> +
> +                       /*
>                           * For bitmap modes, KVM overwrites the entire bitmap,
>                           * i.e. collecting the bitmaps is destructive.  Collect
> -                        * the bitmap only on the first pass, otherwise this
> -                        * test would lose track of dirty pages.
> +                        * the bitmap while the guest is running only once,
> +                        * otherwise this test would lose track of dirty pages.
>                           */
> -                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> +                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
>                                  continue;
>   
>                          /*
> @@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * the ring on every pass would make it unlikely the
>                           * vCPU would ever fill the fing).
>                           */
> -                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
> +                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
>                                  continue;
>   
>                          log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/15/26 21:51, Sean Christopherson wrote:
> On Wed, May 13, 2026, Wu Fei wrote:
>> On 5/13/26 08:03, Sean Christopherson wrote:
>>> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>>>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>>>> could be too short for guest to write and fault in enough pages, then
>>>> there is less chance to test the write protection mechanism, especially
>>>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
>>>
>>> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
>>> meaningless, because the test won't reap the bitmaps for iterations > 0.
>>>
>>> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
>>> 				continue;
>>>
>> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
>> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
>> as dirty but most of them are not populated in page table, this creates the
>> situation I mentioned in the cover letter.
> 
> I suspect something is messed up in your workflow, because the actual patches
> aren't properly threaded with respect to the cover letter.  E.g. patch 1 has
> 
>    In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>
> 
> but the cover letter has:
> 
>    Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>
> 
> Copy+pasting the entirety of the cover letter for reference:
> 
>   : The current gstage range walker unconditionally advances by 'page_size'
>   : when a leaf PTE is not found, e.g. when the range to wp is
>   : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>   : returns false and page_size is 2MB, it skips the whole range, but it's
>   : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>   : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
>   : wp/unamp have the same pattern.
>   :
>   : dirty_log_test intentionally sets up the unaligned guest physical
>   : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
>   : to trigger this bug if there is a larger window for guest to write more
>   : pages before first collect_dirty_pages.
> 
>> "when the range to wp is
>> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>> returns false and page_size is 2MB, it skips the whole range, but it's
>> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
>>
>>>>
>>>> Unit is introduced to replace the default 1ms if specified in command
>>>> line. The following test can't trigger failure on my riscv vm:
>>>
>>> Failure of what?  And does the failure really not reproduce with a higher interval?
>>
>> On riscv, it fails to write protect some pages with valid page table entry
>> then loses track of dirty pages. Higher interval doesn't help because only
>> the first usleep matters, after the first collect_dirty_pages, all dirty
>> pages are tracked precisely then there is no such problem.
> 
> Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> what if we instead randomize the delay before the initial reaping of the dirty
> bitmap/ring?  That should provide a good balance between coverage, complexity and
> user-friendliness.

I'm fine with your solution, I applied it and it did trigger the same issue.

Thanks,
Fei.

> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 12446a4b6e8d..74ca096bf976 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>          pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
>   
>          for (iteration = 1; iteration <= p->iterations; iteration++) {
> -               unsigned long i;
> +               unsigned long i, reap_i;
> +
> +               /*
> +                * Select a random point in the time interval to reap the dirty
> +                * bitmap/ring while the guest is running, i.e. randomize how
> +                * long the guest gets to initially run and thus how many pages
> +                * it can dirty, before collecting the dirty bitmap/ring.  See
> +                * the loop below for details.
> +                */
> +               reap_i = random() % p->interval;
> +               printf("Reaping after a %lu ms delay\n", reap_i);
>   
>                  sync_global_to_guest(vm, iteration);
>   
> @@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * that's effectively blocked.  Collecting while the
>                           * guest is running also verifies KVM doesn't lose any
>                           * state.
> -                        *
> +                        */
> +                       if (i < reap_i)
> +                               continue;
> +
> +                       /*
>                           * For bitmap modes, KVM overwrites the entire bitmap,
>                           * i.e. collecting the bitmaps is destructive.  Collect
> -                        * the bitmap only on the first pass, otherwise this
> -                        * test would lose track of dirty pages.
> +                        * the bitmap while the guest is running only once,
> +                        * otherwise this test would lose track of dirty pages.
>                           */
> -                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> +                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
>                                  continue;
>   
>                          /*
> @@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * the ring on every pass would make it unlikely the
>                           * vCPU would ever fill the fing).
>                           */
> -                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
> +                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
>                                  continue;
>   
>                          log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Wu Fei]

On 5/15/26 21:51, Sean Christopherson wrote:
> On Wed, May 13, 2026, Wu Fei wrote:
>> On 5/13/26 08:03, Sean Christopherson wrote:
>>> On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
>>>> Currently dirty_log_test hardcodes usleep 1ms in each interval, which
>>>> could be too short for guest to write and fault in enough pages, then
>>>> there is less chance to test the write protection mechanism, especially
>>>> in the case of (log_mode != LOG_MODE_DIRTY_RING).
>>>
>>> But when log_mode != LOG_MODE_DIRTY_RING, the individual sleep time is largely
>>> meaningless, because the test won't reap the bitmaps for iterations > 0.
>>>
>>> 			if (i && host_log_mode != LOG_MODE_DIRTY_RING)
>>> 				continue;
>>>
>> The first usleep  matters in the case of KVM_DIRTY_LOG_INITIALLY_SET. The
>> dirty bitmap is not precise in the first get_dirty_log, all pages are marked
>> as dirty but most of them are not populated in page table, this creates the
>> situation I mentioned in the cover letter.
> 
> I suspect something is messed up in your workflow, because the actual patches
> aren't properly threaded with respect to the cover letter.  E.g. patch 1 has
> 
>    In-Reply-To: <202605111849442561v1a0B_7W1L2Z-ENusLaP@zte.com.cn>
> 
> but the cover letter has:
> 
>    Message-Id: <202605111108.64BB8RFR010522@mse-db.zte.com.cn>
> 
> Copy+pasting the entirety of the cover letter for reference:
> 
>   : The current gstage range walker unconditionally advances by 'page_size'
>   : when a leaf PTE is not found, e.g. when the range to wp is
>   : [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>   : returns false and page_size is 2MB, it skips the whole range, but it's
>   : possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>   : only [0xfffff01fc000, 0xfffff0200000) can be skipped safely. Both
>   : wp/unamp have the same pattern.
>   :
>   : dirty_log_test intentionally sets up the unaligned guest physical
>   : address, after riscv kvm enabling KVM_DIRTY_LOG_INITIALLY_SET, it's easy
>   : to trigger this bug if there is a larger window for guest to write more
>   : pages before first collect_dirty_pages.
> 
>> "when the range to wp is
>> [0xfffff01fc000, 0xfffff023c000) , if found_leaf of 0xfffff01fc000
>> returns false and page_size is 2MB, it skips the whole range, but it's
>> possible to have valid entries in [0xfffff0200000, 0xfffff023c000), so
>> only [0xfffff01fc000, 0xfffff0200000) can be skipped safely."
>>
>>>>
>>>> Unit is introduced to replace the default 1ms if specified in command
>>>> line. The following test can't trigger failure on my riscv vm:
>>>
>>> Failure of what?  And does the failure really not reproduce with a higher interval?
>>
>> On riscv, it fails to write protect some pages with valid page table entry
>> then loses track of dirty pages. Higher interval doesn't help because only
>> the first usleep matters, after the first collect_dirty_pages, all dirty
>> pages are tracked precisely then there is no such problem.
> 
> Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> what if we instead randomize the delay before the initial reaping of the dirty
> bitmap/ring?  That should provide a good balance between coverage, complexity and
> user-friendliness.

I'm fine with your solution, I applied it and it did trigger the same issue.

Thanks,
Fei.

> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 12446a4b6e8d..74ca096bf976 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -694,7 +694,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>          pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
>   
>          for (iteration = 1; iteration <= p->iterations; iteration++) {
> -               unsigned long i;
> +               unsigned long i, reap_i;
> +
> +               /*
> +                * Select a random point in the time interval to reap the dirty
> +                * bitmap/ring while the guest is running, i.e. randomize how
> +                * long the guest gets to initially run and thus how many pages
> +                * it can dirty, before collecting the dirty bitmap/ring.  See
> +                * the loop below for details.
> +                */
> +               reap_i = random() % p->interval;
> +               printf("Reaping after a %lu ms delay\n", reap_i);
>   
>                  sync_global_to_guest(vm, iteration);
>   
> @@ -729,13 +739,17 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * that's effectively blocked.  Collecting while the
>                           * guest is running also verifies KVM doesn't lose any
>                           * state.
> -                        *
> +                        */
> +                       if (i < reap_i)
> +                               continue;
> +
> +                       /*
>                           * For bitmap modes, KVM overwrites the entire bitmap,
>                           * i.e. collecting the bitmaps is destructive.  Collect
> -                        * the bitmap only on the first pass, otherwise this
> -                        * test would lose track of dirty pages.
> +                        * the bitmap while the guest is running only once,
> +                        * otherwise this test would lose track of dirty pages.
>                           */
> -                       if (i && host_log_mode != LOG_MODE_DIRTY_RING)
> +                       if (i > reap_i && host_log_mode != LOG_MODE_DIRTY_RING)
>                                  continue;
>   
>                          /*
> @@ -745,7 +759,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                           * the ring on every pass would make it unlikely the
>                           * vCPU would ever fill the fing).
>                           */
> -                       if (i && !READ_ONCE(dirty_ring_vcpu_ring_full))
> +                       if (i > reap_i && !READ_ONCE(dirty_ring_vcpu_ring_full))
>                                  continue;
>   
>                          log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX,

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 18, 2026, Wu Fei wrote:
> On 5/15/26 21:51, Sean Christopherson wrote:
> > On Wed, May 13, 2026, Wu Fei wrote:
> > > On 5/13/26 08:03, Sean Christopherson wrote:
> > > > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > > > 
> > > > > Unit is introduced to replace the default 1ms if specified in command
> > > > > line. The following test can't trigger failure on my riscv vm:
> > > > 
> > > > Failure of what?  And does the failure really not reproduce with a higher interval?
> > > 
> > > On riscv, it fails to write protect some pages with valid page table entry
> > > then loses track of dirty pages. Higher interval doesn't help because only
> > > the first usleep matters, after the first collect_dirty_pages, all dirty
> > > pages are tracked precisely then there is no such problem.
> > 
> > Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> > what if we instead randomize the delay before the initial reaping of the dirty
> > bitmap/ring?  That should provide a good balance between coverage, complexity and
> > user-friendliness.
> 
> I'm fine with your solution, I applied it and it did trigger the same issue.

Awesome!  I'll post a patch today.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 18, 2026, Wu Fei wrote:
> On 5/15/26 21:51, Sean Christopherson wrote:
> > On Wed, May 13, 2026, Wu Fei wrote:
> > > On 5/13/26 08:03, Sean Christopherson wrote:
> > > > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > > > 
> > > > > Unit is introduced to replace the default 1ms if specified in command
> > > > > line. The following test can't trigger failure on my riscv vm:
> > > > 
> > > > Failure of what?  And does the failure really not reproduce with a higher interval?
> > > 
> > > On riscv, it fails to write protect some pages with valid page table entry
> > > then loses track of dirty pages. Higher interval doesn't help because only
> > > the first usleep matters, after the first collect_dirty_pages, all dirty
> > > pages are tracked precisely then there is no such problem.
> > 
> > Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> > what if we instead randomize the delay before the initial reaping of the dirty
> > bitmap/ring?  That should provide a good balance between coverage, complexity and
> > user-friendliness.
> 
> I'm fine with your solution, I applied it and it did trigger the same issue.

Awesome!  I'll post a patch today.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/3] KVM: selftests: Add unit to dirty_log_test

[Sean Christopherson]

On Mon, May 18, 2026, Wu Fei wrote:
> On 5/15/26 21:51, Sean Christopherson wrote:
> > On Wed, May 13, 2026, Wu Fei wrote:
> > > On 5/13/26 08:03, Sean Christopherson wrote:
> > > > On Mon, May 11, 2026, wu.fei9@sanechips.com.cn wrote:
> > > > > 
> > > > > Unit is introduced to replace the default 1ms if specified in command
> > > > > line. The following test can't trigger failure on my riscv vm:
> > > > 
> > > > Failure of what?  And does the failure really not reproduce with a higher interval?
> > > 
> > > On riscv, it fails to write protect some pages with valid page table entry
> > > then loses track of dirty pages. Higher interval doesn't help because only
> > > the first usleep matters, after the first collect_dirty_pages, all dirty
> > > pages are tracked precisely then there is no such problem.
> > 
> > Ah, gotcha.  Rather than let (and force) the user to provide a larger sleep time,
> > what if we instead randomize the delay before the initial reaping of the dirty
> > bitmap/ring?  That should provide a good balance between coverage, complexity and
> > user-friendliness.
> 
> I'm fine with your solution, I applied it and it did trigger the same issue.

Awesome!  I'll post a patch today.

[PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[wu.fei9]

The current gstage range walker unconditionally advances by 'page_size'
when a leaf PTE is not found, e.g. when the range to wp is
[0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
0xfffff01fc000 returns false, it skip the whole range, but it's possible
to have valid entries in [0xfffff0200000, 0xfffff023c000).

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index d9fe8be2a151..2b141a78ecac 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               addr = ALIGN_DOWN(addr, page_size);
-               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                       ptep_level, GSTAGE_OP_WP);
-next:
-               addr += page_size;
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       addr = ALIGN_DOWN(addr, page_size);
+                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                               ptep_level, GSTAGE_OP_WP);
+                       addr += page_size;
+               }
        }
 }

--
2.43.0

[PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[wu.fei9]

The current gstage range walker unconditionally advances by 'page_size'
when a leaf PTE is not found, e.g. when the range to wp is
[0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
0xfffff01fc000 returns false, it skip the whole range, but it's possible
to have valid entries in [0xfffff0200000, 0xfffff023c000).

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index d9fe8be2a151..2b141a78ecac 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               addr = ALIGN_DOWN(addr, page_size);
-               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                       ptep_level, GSTAGE_OP_WP);
-next:
-               addr += page_size;
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       addr = ALIGN_DOWN(addr, page_size);
+                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                               ptep_level, GSTAGE_OP_WP);
+                       addr += page_size;
+               }
        }
 }

--
2.43.0

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[wu.fei9]

The current gstage range walker unconditionally advances by 'page_size'
when a leaf PTE is not found, e.g. when the range to wp is
[0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
0xfffff01fc000 returns false, it skip the whole range, but it's possible
to have valid entries in [0xfffff0200000, 0xfffff023c000).

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index d9fe8be2a151..2b141a78ecac 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               addr = ALIGN_DOWN(addr, page_size);
-               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                       ptep_level, GSTAGE_OP_WP);
-next:
-               addr += page_size;
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       addr = ALIGN_DOWN(addr, page_size);
+                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                               ptep_level, GSTAGE_OP_WP);
+                       addr += page_size;
+               }
        }
 }

--
2.43.0

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[Anup Patel]

On Mon, May 11, 2026 at 5:00 PM <wu.fei9@sanechips.com.cn> wrote:
>
> The current gstage range walker unconditionally advances by 'page_size'
> when a leaf PTE is not found, e.g. when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
> 0xfffff01fc000 returns false, it skip the whole range, but it's possible
> to have valid entries in [0xfffff0200000, 0xfffff023c000).
>
> Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>

I see all tabs converted to spaces. Looks like this patch was
not send using git send-email.

Please rebase and send updated series using git send-email.

Regards,
Anup

> ---
>  arch/riscv/kvm/gstage.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index d9fe8be2a151..2b141a78ecac 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>                 if (ret)
>                         break;
>
> -               if (!found_leaf)
> -                       goto next;
> -
> -               addr = ALIGN_DOWN(addr, page_size);
> -               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> -                                       ptep_level, GSTAGE_OP_WP);
> -next:
> -               addr += page_size;
> +               if (!found_leaf) {
> +                       addr = ALIGN(addr + 1, page_size);
> +               } else {
> +                       addr = ALIGN_DOWN(addr, page_size);
> +                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> +                                               ptep_level, GSTAGE_OP_WP);
> +                       addr += page_size;
> +               }
>         }
>  }
>
> --
> 2.43.0

Re: [PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[Anup Patel]

On Mon, May 11, 2026 at 5:00 PM <wu.fei9@sanechips.com.cn> wrote:
>
> The current gstage range walker unconditionally advances by 'page_size'
> when a leaf PTE is not found, e.g. when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
> 0xfffff01fc000 returns false, it skip the whole range, but it's possible
> to have valid entries in [0xfffff0200000, 0xfffff023c000).
>
> Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>

I see all tabs converted to spaces. Looks like this patch was
not send using git send-email.

Please rebase and send updated series using git send-email.

Regards,
Anup

> ---
>  arch/riscv/kvm/gstage.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index d9fe8be2a151..2b141a78ecac 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>                 if (ret)
>                         break;
>
> -               if (!found_leaf)
> -                       goto next;
> -
> -               addr = ALIGN_DOWN(addr, page_size);
> -               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> -                                       ptep_level, GSTAGE_OP_WP);
> -next:
> -               addr += page_size;
> +               if (!found_leaf) {
> +                       addr = ALIGN(addr + 1, page_size);
> +               } else {
> +                       addr = ALIGN_DOWN(addr, page_size);
> +                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> +                                               ptep_level, GSTAGE_OP_WP);
> +                       addr += page_size;
> +               }
>         }
>  }
>
> --
> 2.43.0

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_wp_range

[Anup Patel]

On Mon, May 11, 2026 at 5:00 PM <wu.fei9@sanechips.com.cn> wrote:
>
> The current gstage range walker unconditionally advances by 'page_size'
> when a leaf PTE is not found, e.g. when the range to wp is
> [0xfffff01fc000, 0xfffff023c000) and page_size is 2MB, if found_leaf of
> 0xfffff01fc000 returns false, it skip the whole range, but it's possible
> to have valid entries in [0xfffff0200000, 0xfffff023c000).
>
> Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>

I see all tabs converted to spaces. Looks like this patch was
not send using git send-email.

Please rebase and send updated series using git send-email.

Regards,
Anup

> ---
>  arch/riscv/kvm/gstage.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index d9fe8be2a151..2b141a78ecac 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -429,14 +429,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>                 if (ret)
>                         break;
>
> -               if (!found_leaf)
> -                       goto next;
> -
> -               addr = ALIGN_DOWN(addr, page_size);
> -               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> -                                       ptep_level, GSTAGE_OP_WP);
> -next:
> -               addr += page_size;
> +               if (!found_leaf) {
> +                       addr = ALIGN(addr + 1, page_size);
> +               } else {
> +                       addr = ALIGN_DOWN(addr, page_size);
> +                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
> +                                               ptep_level, GSTAGE_OP_WP);
> +                       addr += page_size;
> +               }
>         }
>  }
>
> --
> 2.43.0

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

[PATCH 3/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_unmap_range

[wu.fei9]

Same as kvm_riscv_gstage_wp_range, the possible valid pages should not
be skipped if !found_leaf. Different from wp case, which can
write-protect more than asked, unmap can't do that, no splitting is
added right now but a warning is logged instead.

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index 2b141a78ecac..d78f5aeb8b40 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -395,15 +395,19 @@ void kvm_riscv_gstage_unmap_range(struct kvm_gstage *gstage,
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
-                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                               ptep_level, GSTAGE_OP_CLEAR);
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
+                               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                                       ptep_level, GSTAGE_OP_CLEAR);
+                       else {
+                               WARN_ONCE(1, "Skip unmap range addr: %#llx, end: %#llx, page_size: %#lx\n",
+                                               addr, end, page_size);
+                       }

-next:
-               addr += page_size;
+                       addr += page_size;
+               }

                /*
                 * If the range is too large, release the kvm->mmu_lock
--
2.43.0

[PATCH 3/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_unmap_range

[wu.fei9]

Same as kvm_riscv_gstage_wp_range, the possible valid pages should not
be skipped if !found_leaf. Different from wp case, which can
write-protect more than asked, unmap can't do that, no splitting is
added right now but a warning is logged instead.

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index 2b141a78ecac..d78f5aeb8b40 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -395,15 +395,19 @@ void kvm_riscv_gstage_unmap_range(struct kvm_gstage *gstage,
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
-                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                               ptep_level, GSTAGE_OP_CLEAR);
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
+                               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                                       ptep_level, GSTAGE_OP_CLEAR);
+                       else {
+                               WARN_ONCE(1, "Skip unmap range addr: %#llx, end: %#llx, page_size: %#lx\n",
+                                               addr, end, page_size);
+                       }

-next:
-               addr += page_size;
+                       addr += page_size;
+               }

                /*
                 * If the range is too large, release the kvm->mmu_lock
--
2.43.0

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/3] RISC-V: KVM: Fix skip of valid pages in kvm_riscv_gstage_unmap_range

[wu.fei9]

Same as kvm_riscv_gstage_wp_range, the possible valid pages should not
be skipped if !found_leaf. Different from wp case, which can
write-protect more than asked, unmap can't do that, no splitting is
added right now but a warning is logged instead.

Signed-off-by: Wu Fei <wu.fei9@sanechips.com.cn>
---
 arch/riscv/kvm/gstage.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index 2b141a78ecac..d78f5aeb8b40 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -395,15 +395,19 @@ void kvm_riscv_gstage_unmap_range(struct kvm_gstage *gstage,
                if (ret)
                        break;

-               if (!found_leaf)
-                       goto next;
-
-               if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
-                       kvm_riscv_gstage_op_pte(gstage, addr, ptep,
-                                               ptep_level, GSTAGE_OP_CLEAR);
+               if (!found_leaf) {
+                       addr = ALIGN(addr + 1, page_size);
+               } else {
+                       if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
+                               kvm_riscv_gstage_op_pte(gstage, addr, ptep,
+                                                       ptep_level, GSTAGE_OP_CLEAR);
+                       else {
+                               WARN_ONCE(1, "Skip unmap range addr: %#llx, end: %#llx, page_size: %#lx\n",
+                                               addr, end, page_size);
+                       }

-next:
-               addr += page_size;
+                       addr += page_size;
+               }

                /*
                 * If the range is too large, release the kvm->mmu_lock
--
2.43.0

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv