[v1] Move pKVM ownership state to hyp_vmemmap

[PATCH 2/6] KVM: arm64: Use 0b11 for encoding PKVM_NOPAGE

Posted by Quentin Perret 11 months, 2 weeks ago

The page ownership state encoded as 0b11 is currently considered
reserved for future use, and PKVM_NOPAGE uses bit 2. In order to
simplify the relocation of the hyp ownership state into the
vmemmap in later patches, let's use the 'reserved' encoding for
the PKVM_NOPAGE state. The struct hyp_page layout isn't guaranteed
stable at all, so there is no real reason to have 'reserved' encodings.

No functional changes intended.

Signed-off-by: Quentin Perret <qperret@google.com>
---
 arch/arm64/kvm/hyp/include/nvhe/memory.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/memory.h b/arch/arm64/kvm/hyp/include/nvhe/memory.h
index 34233d586060..642b5e05fe77 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/memory.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/memory.h
@@ -13,18 +13,15 @@
  *   01: The page is owned by the page-table owner, but is shared
  *       with another entity.
  *   10: The page is shared with, but not owned by the page-table owner.
- *   11: Reserved for future use (lending).
  */
 enum pkvm_page_state {
 	PKVM_PAGE_OWNED			= 0ULL,
 	PKVM_PAGE_SHARED_OWNED		= BIT(0),
 	PKVM_PAGE_SHARED_BORROWED	= BIT(1),
-	__PKVM_PAGE_RESERVED		= BIT(0) | BIT(1),
 
 	/* Meta-states which aren't encoded directly in the PTE's SW bits */
-	PKVM_NOPAGE			= BIT(2),
+	PKVM_NOPAGE			= BIT(0) | BIT(1),
 };
-#define PKVM_PAGE_META_STATES_MASK	(~__PKVM_PAGE_RESERVED)
 
 #define PKVM_PAGE_STATE_PROT_MASK	(KVM_PGTABLE_PROT_SW0 | KVM_PGTABLE_PROT_SW1)
 static inline enum kvm_pgtable_prot pkvm_mkstate(enum kvm_pgtable_prot prot,
-- 
2.48.1.658.g4767266eb4-goog

Re: [PATCH 2/6] KVM: arm64: Use 0b11 for encoding PKVM_NOPAGE

Posted by Marc Zyngier 11 months ago

On Thu, 27 Feb 2025 00:33:06 +0000,
Quentin Perret <qperret@google.com> wrote:
> 
> The page ownership state encoded as 0b11 is currently considered
> reserved for future use, and PKVM_NOPAGE uses bit 2. In order to
> simplify the relocation of the hyp ownership state into the
> vmemmap in later patches, let's use the 'reserved' encoding for
> the PKVM_NOPAGE state. The struct hyp_page layout isn't guaranteed
> stable at all, so there is no real reason to have 'reserved' encodings.
> 
> No functional changes intended.
> 
> Signed-off-by: Quentin Perret <qperret@google.com>
> ---
>  arch/arm64/kvm/hyp/include/nvhe/memory.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/include/nvhe/memory.h b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> index 34233d586060..642b5e05fe77 100644
> --- a/arch/arm64/kvm/hyp/include/nvhe/memory.h
> +++ b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> @@ -13,18 +13,15 @@
>   *   01: The page is owned by the page-table owner, but is shared
>   *       with another entity.
>   *   10: The page is shared with, but not owned by the page-table owner.
> - *   11: Reserved for future use (lending).
>   */
>  enum pkvm_page_state {
>  	PKVM_PAGE_OWNED			= 0ULL,
>  	PKVM_PAGE_SHARED_OWNED		= BIT(0),
>  	PKVM_PAGE_SHARED_BORROWED	= BIT(1),
> -	__PKVM_PAGE_RESERVED		= BIT(0) | BIT(1),
>  
>  	/* Meta-states which aren't encoded directly in the PTE's SW bits */
> -	PKVM_NOPAGE			= BIT(2),
> +	PKVM_NOPAGE			= BIT(0) | BIT(1),

Isn't this comment stale now?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 2/6] KVM: arm64: Use 0b11 for encoding PKVM_NOPAGE

Posted by Quentin Perret 11 months ago

On Friday 14 Mar 2025 at 11:07:01 (+0000), Marc Zyngier wrote:
> On Thu, 27 Feb 2025 00:33:06 +0000,
> Quentin Perret <qperret@google.com> wrote:
> > 
> > The page ownership state encoded as 0b11 is currently considered
> > reserved for future use, and PKVM_NOPAGE uses bit 2. In order to
> > simplify the relocation of the hyp ownership state into the
> > vmemmap in later patches, let's use the 'reserved' encoding for
> > the PKVM_NOPAGE state. The struct hyp_page layout isn't guaranteed
> > stable at all, so there is no real reason to have 'reserved' encodings.
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Quentin Perret <qperret@google.com>
> > ---
> >  arch/arm64/kvm/hyp/include/nvhe/memory.h | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/include/nvhe/memory.h b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > index 34233d586060..642b5e05fe77 100644
> > --- a/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > +++ b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > @@ -13,18 +13,15 @@
> >   *   01: The page is owned by the page-table owner, but is shared
> >   *       with another entity.
> >   *   10: The page is shared with, but not owned by the page-table owner.
> > - *   11: Reserved for future use (lending).
> >   */
> >  enum pkvm_page_state {
> >  	PKVM_PAGE_OWNED			= 0ULL,
> >  	PKVM_PAGE_SHARED_OWNED		= BIT(0),
> >  	PKVM_PAGE_SHARED_BORROWED	= BIT(1),
> > -	__PKVM_PAGE_RESERVED		= BIT(0) | BIT(1),
> >  
> >  	/* Meta-states which aren't encoded directly in the PTE's SW bits */
> > -	PKVM_NOPAGE			= BIT(2),
> > +	PKVM_NOPAGE			= BIT(0) | BIT(1),
> 
> Isn't this comment stale now?

I believe it still applies to guest stage-2 page-tables as the three
other states above are still stored into PTE SW bits (well, sort of,
only SHARED_BORROWED is at the moment as we don't supported protected
VMs, but OWNED and SHARED_OWNED will be a thing for protected). NOPAGE
is still the only one that is a bit different and doesn't go there.

With that said, the comment could be made more explicit about that and
explain this is now guest s2 only. Happy to spin another version of the
series with that changed if that helps.

Thanks!
Quentin

Re: [PATCH 2/6] KVM: arm64: Use 0b11 for encoding PKVM_NOPAGE

Posted by Marc Zyngier 10 months, 4 weeks ago

On Fri, 14 Mar 2025 14:13:39 +0000,
Quentin Perret <qperret@google.com> wrote:
> 
> On Friday 14 Mar 2025 at 11:07:01 (+0000), Marc Zyngier wrote:
> > On Thu, 27 Feb 2025 00:33:06 +0000,
> > Quentin Perret <qperret@google.com> wrote:
> > > 
> > > The page ownership state encoded as 0b11 is currently considered
> > > reserved for future use, and PKVM_NOPAGE uses bit 2. In order to
> > > simplify the relocation of the hyp ownership state into the
> > > vmemmap in later patches, let's use the 'reserved' encoding for
> > > the PKVM_NOPAGE state. The struct hyp_page layout isn't guaranteed
> > > stable at all, so there is no real reason to have 'reserved' encodings.
> > > 
> > > No functional changes intended.
> > > 
> > > Signed-off-by: Quentin Perret <qperret@google.com>
> > > ---
> > >  arch/arm64/kvm/hyp/include/nvhe/memory.h | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/memory.h b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > > index 34233d586060..642b5e05fe77 100644
> > > --- a/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > > +++ b/arch/arm64/kvm/hyp/include/nvhe/memory.h
> > > @@ -13,18 +13,15 @@
> > >   *   01: The page is owned by the page-table owner, but is shared
> > >   *       with another entity.
> > >   *   10: The page is shared with, but not owned by the page-table owner.
> > > - *   11: Reserved for future use (lending).
> > >   */
> > >  enum pkvm_page_state {
> > >  	PKVM_PAGE_OWNED			= 0ULL,
> > >  	PKVM_PAGE_SHARED_OWNED		= BIT(0),
> > >  	PKVM_PAGE_SHARED_BORROWED	= BIT(1),
> > > -	__PKVM_PAGE_RESERVED		= BIT(0) | BIT(1),
> > >  
> > >  	/* Meta-states which aren't encoded directly in the PTE's SW bits */
> > > -	PKVM_NOPAGE			= BIT(2),
> > > +	PKVM_NOPAGE			= BIT(0) | BIT(1),
> > 
> > Isn't this comment stale now?
> 
> I believe it still applies to guest stage-2 page-tables as the three
> other states above are still stored into PTE SW bits (well, sort of,
> only SHARED_BORROWED is at the moment as we don't supported protected
> VMs, but OWNED and SHARED_OWNED will be a thing for protected). NOPAGE
> is still the only one that is a bit different and doesn't go there.
>
> With that said, the comment could be made more explicit about that and
> explain this is now guest s2 only. Happy to spin another version of the
> series with that changed if that helps.

Right. It is slightly misleading that we end-up with two notions of
state:

- the per-mapping state encoded in the PTs
- the per-page state encoded in the vmemmap

The two are obviously related, but it is all a bit confusing for the
unsuspecting reader. There is no urgency to address this, but maybe
it'd be good to clarify this aspect in the future (no need to respin
on that account).

	M.

-- 
Without deviation from the norm, progress is not possible.