public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: David Matlack <dmatlack@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Ben Gardon <bgardon@google.com>,
	Sean Christopherson <seanjc@google.com>,
	Oliver Upton <oupton@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Andrew Jones <drjones@redhat.com>,
	"open list:KERNEL VIRTUAL MACHINE (KVM)" <kvm@vger.kernel.org>
Subject: Re: [PATCH v2 01/10] KVM: selftests: Replace x86_page_size with PG_LEVEL_XX
Date: Tue, 17 May 2022 16:26:54 -0400	[thread overview]
Message-ID: <YoQFDgS5mCDlIUoS@xz-m1.local> (raw)
In-Reply-To: <20220517190524.2202762-2-dmatlack@google.com>

On Tue, May 17, 2022 at 07:05:15PM +0000, David Matlack wrote:
> x86_page_size is an enum used to communicate the desired page size with
> which to map a range of memory. Under the hood they just encode the
> desired level at which to map the page. This ends up being clunky in a
> few ways:
> 
>  - The name suggests it encodes the size of the page rather than the
>    level.
>  - In other places in x86_64/processor.c we just use a raw int to encode
>    the level.
> 
> Simplify this by adopting the kernel style of PG_LEVEL_XX enums and pass
> around raw ints when referring to the level. This makes the code easier
> to understand since these macros are very common in KVM MMU code.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  .../selftests/kvm/include/x86_64/processor.h  | 18 ++++++----
>  .../selftests/kvm/lib/x86_64/processor.c      | 33 ++++++++++---------
>  .../selftests/kvm/max_guest_memory_test.c     |  2 +-
>  .../selftests/kvm/x86_64/mmu_role_test.c      |  2 +-
>  4 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 37db341d4cc5..434a4f60f4d9 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -465,13 +465,19 @@ void vcpu_set_hv_cpuid(struct kvm_vm *vm, uint32_t vcpuid);
>  struct kvm_cpuid2 *vcpu_get_supported_hv_cpuid(struct kvm_vm *vm, uint32_t vcpuid);
>  void vm_xsave_req_perm(int bit);
>  
> -enum x86_page_size {
> -	X86_PAGE_SIZE_4K = 0,
> -	X86_PAGE_SIZE_2M,
> -	X86_PAGE_SIZE_1G,
> +enum pg_level {
> +	PG_LEVEL_NONE,
> +	PG_LEVEL_4K,
> +	PG_LEVEL_2M,
> +	PG_LEVEL_1G,
> +	PG_LEVEL_512G,
> +	PG_LEVEL_NUM
>  };

I still prefer PTE/PMD/PUD/... as I suggested, as that's how the kernel mm
handles these levels with arch-independent way across the kernel.  But
well.. I never fight hard on namings, because I know that's the major
complexity. :-)

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu


  reply	other threads:[~2022-05-17 20:27 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 19:05 [PATCH v2 00/10] KVM: selftests: Add nested support to dirty_log_perf_test David Matlack
2022-05-17 19:05 ` [PATCH v2 01/10] KVM: selftests: Replace x86_page_size with PG_LEVEL_XX David Matlack
2022-05-17 20:26   ` Peter Xu [this message]
2022-05-17 19:05 ` [PATCH v2 02/10] KVM: selftests: Add option to create 2M and 1G EPT mappings David Matlack
2022-05-17 20:27   ` Peter Xu
2022-05-17 19:05 ` [PATCH v2 03/10] KVM: selftests: Drop stale function parameter comment for nested_map() David Matlack
2022-05-17 19:05 ` [PATCH v2 04/10] KVM: selftests: Refactor nested_map() to specify target level David Matlack
2022-05-17 19:05 ` [PATCH v2 05/10] KVM: selftests: Move VMX_EPT_VPID_CAP_AD_BITS to vmx.h David Matlack
2022-05-17 19:05 ` [PATCH v2 06/10] KVM: selftests: Add a helper to check EPT/VPID capabilities David Matlack
2022-05-17 19:05 ` [PATCH v2 07/10] KVM: selftests: Link selftests directly with lib object files David Matlack
2022-05-17 19:05 ` [PATCH v2 08/10] KVM: selftests: Drop unnecessary rule for $(LIBKVM_OBJS) David Matlack
2022-05-17 20:21   ` Peter Xu
2022-05-18 17:18     ` David Matlack
2022-05-17 19:05 ` [PATCH v2 09/10] KVM: selftests: Clean up LIBKVM files in Makefile David Matlack
2022-05-17 19:05 ` [PATCH v2 10/10] KVM: selftests: Add option to run dirty_log_perf_test vCPUs in L2 David Matlack
2022-05-17 20:20   ` Peter Xu
2022-05-18 13:51     ` Peter Xu
2022-05-18 15:24       ` Sean Christopherson
2022-05-18 16:12         ` David Matlack
2022-05-18 16:37           ` Sean Christopherson
2022-05-20 22:01             ` David Matlack
2022-05-20 22:49               ` David Matlack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YoQFDgS5mCDlIUoS@xz-m1.local \
    --to=peterx@redhat.com \
    --cc=bgardon@google.com \
    --cc=dmatlack@google.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=oupton@google.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox