On Thu, Aug 29, 2019 at 11:45:16AM +0200, Andrew Jones wrote:
> On Thu, Aug 29, 2019 at 10:21:16AM +0800, Peter Xu wrote:
> > The naming VM_MODE_P52V48_4K is explicit but unclear when used on
> > x86_64 machines, because x86_64 machines are having various physical
> > address width rather than some static values. Here's some examples:
> >
> > - Intel Xeon E3-1220: 36 bits
> > - Intel Core i7-8650: 39 bits
> > - AMD EPYC 7251: 48 bits
> >
> > All of them are using 48 bits linear address width but with totally
> > different physical address width (and most of the old machines should
> > be less than 52 bits).
> >
> > Let's create a new guest mode called VM_MODE_PXXV48_4K for current
> > x86_64 tests and make it as the default to replace the old naming of
> > VM_MODE_P52V48_4K because it shows more clearly that the PA width is
> > not really a constant. Meanwhile we also stop assuming all the x86
> > machines are having 52 bits PA width but instead we fetch the real
> > vm->pa_bits from CPUID 0x80000008 during runtime.
> >
> > We currently make this exclusively used by x86_64 but no other arch.
> >
> > As a slight touch up, moving DEBUG macro from dirty_log_test.c to
> > kvm_util.h so lib can use it too.
> >
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/kvm/dirty_log_test.c | 5 ++--
> > .../testing/selftests/kvm/include/kvm_util.h | 9 +++++-
> > .../selftests/kvm/include/x86_64/processor.h | 3 ++
> > .../selftests/kvm/lib/aarch64/processor.c | 3 ++
> > tools/testing/selftests/kvm/lib/kvm_util.c | 29 ++++++++++++++----
> > .../selftests/kvm/lib/x86_64/processor.c | 30 ++++++++++++++++---
> > 6 files changed, 65 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> > index efb7746a7e99..c86f83cb33e5 100644
> > --- a/tools/testing/selftests/kvm/dirty_log_test.c
> > +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> > @@ -19,8 +19,6 @@
> > #include "kvm_util.h"
> > #include "processor.h"
> >
> > -#define DEBUG printf
> > -
> > #define VCPU_ID 1
> >
> > /* The memory slot index to track dirty pages */
> > @@ -256,6 +254,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> >
> > switch (mode) {
> > case VM_MODE_P52V48_4K:
> > + case VM_MODE_PXXV48_4K:
> > guest_pa_bits = 52;
> > guest_page_shift = 12;
> > break;
> > @@ -446,7 +445,7 @@ int main(int argc, char *argv[])
> > #endif
> >
> > #ifdef __x86_64__
> > - vm_guest_mode_params_init(VM_MODE_P52V48_4K, true, true);
> > + vm_guest_mode_params_init(VM_MODE_PXXV48_4K, true, true);
> > #endif
> > #ifdef __aarch64__
> > vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index c78faa2ff7f3..430edbacb9b2 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -24,6 +24,10 @@ struct kvm_vm;
> > typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */
> > typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */
> >
> > +#ifndef DEBUG
> > +#define DEBUG printf
> > +#endif
>
> There's no way to turn this off without modifying code. I suggested
>
> #ifndef NDEBUG
> #define dprintf printf
> #endif
>
> which allows the dprintf(...) statements to be removed by compiling with
> -DNDEBUG added to CFLAGS. And that would also disable all the asserts().
> That's probably not all that useful, but then again, defining printf() as
> DEBUG() isn't useful either if the intention is to always print.
Sorry I misread that...
Though, I'm afraid even if with above it won't compile with -DNDEBUG
because the compiler could start to complain about undefined
"dprintf", or even recognize the dprintf as the libc call, dprintf(3).
So instead, does below looks ok?
#ifdef NDEBUG
#define DEBUG(...)
#else
#define DEBUG(...) printf(__VA_ARGS__);
#endif
Thanks,
--
Peter Xu
