On Mon, Aug 26, 2019 at 10:11:34AM +0200, Thomas Huth wrote: > On 26/08/2019 09.57, Peter Xu wrote: > > The dirty_log_test is failing on some old machines like Xeon E3-1220 > > with tripple faults when writting to the tracked memory region: > > > > Test iterations: 32, interval: 10 (ms) > > Testing guest mode: PA-bits:52, VA-bits:48, 4K pages > > guest physical test memory offset: 0x7fbffef000 > > ==== Test Assertion Failure ==== > > dirty_log_test.c:138: false > > pid=6137 tid=6139 - Success > > 1 0x0000000000401ca1: vcpu_worker at dirty_log_test.c:138 > > 2 0x00007f3dd9e392dd: ?? ??:0 > > 3 0x00007f3dd9b6a132: ?? ??:0 > > Invalid guest sync status: exit_reason=SHUTDOWN > > > > It's because previously we moved the testing memory region from a > > static place (1G) to the top of the system's physical address space, > > meanwhile we stick to 39 bits PA for all the x86_64 machines. That's > > not true for machines like Xeon E3-1220 where it only supports 36. > > > > Let's unbreak this test by dynamically detect PA width from CPUID > > 0x80000008. Meanwhile, even allow kvm_get_supported_cpuid_index() to > > fail. I don't know whether that could be useful because I think > > 0x80000008 should be there for all x86_64 hosts, but I also think it's > > not really helpful to assert in the kvm_get_supported_cpuid_index(). > [...] > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > index 6cb34a0fa200..9de2fd310ac8 100644 > > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > @@ -760,9 +760,6 @@ kvm_get_supported_cpuid_index(uint32_t function, uint32_t index) > > break; > > } > > } > > - > > - TEST_ASSERT(entry, "Guest CPUID entry not found: (EAX=%x, ECX=%x).", > > - function, index); > > return entry; > > } > > You should also adjust the comment of the function. It currently says > "Never returns NULL". Not it can return NULL. Yeh that's better. > > And maybe add a TEST_ASSERT() to the other callers instead, which do not > expect a NULL to be returned? I think it's fine because it's the same as moving the assert from here to the callers because when the caller uses entry->xxx it'll assert. :) Thanks, -- Peter Xu
