| Message ID | 20241203103735.2267589-2-qperret@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: arm64: Non-protected guest stage-2 support for pKVM | expand |
Hi Quentin, On Tue, 3 Dec 2024 at 10:37, Quentin Perret <qperret@google.com> wrote: > > The 'concrete' (a.k.a non-meta) page states are currently encoded using > software bits in PTEs. For performance reasons, the abstract > pkvm_page_state enum uses the same bits to encode these states as that > makes conversions from and to PTEs easy. > > In order to prepare the ground for moving the 'concrete' state storage > to the hyp vmemmap, re-arrange the enum to use bits 0 and 1 for this > purpose. > > No functional changes intended. > > Signed-off-by: Quentin Perret <qperret@google.com> > --- > arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > index 0972faccc2af..ca3177481b78 100644 > --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > @@ -24,25 +24,28 @@ > */ > enum pkvm_page_state { > PKVM_PAGE_OWNED = 0ULL, > - PKVM_PAGE_SHARED_OWNED = KVM_PGTABLE_PROT_SW0, > - PKVM_PAGE_SHARED_BORROWED = KVM_PGTABLE_PROT_SW1, > - __PKVM_PAGE_RESERVED = KVM_PGTABLE_PROT_SW0 | > - KVM_PGTABLE_PROT_SW1, > + 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, > + PKVM_NOPAGE = BIT(2), > }; > +#define PKVM_PAGE_META_STATES_MASK (~(BIT(0) | BIT(1))) > > #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, > enum pkvm_page_state state) > { > - return (prot & ~PKVM_PAGE_STATE_PROT_MASK) | state; > + BUG_ON(state & PKVM_PAGE_META_STATES_MASK); This is a slight change in functionality, having a BUG_ON instead of just masking out illegal states. Is it necessary? Cheers, /fuad > + prot &= ~PKVM_PAGE_STATE_PROT_MASK; > + prot |= FIELD_PREP(PKVM_PAGE_STATE_PROT_MASK, state); > + return prot; > } > > static inline enum pkvm_page_state pkvm_getstate(enum kvm_pgtable_prot prot) > { > - return prot & PKVM_PAGE_STATE_PROT_MASK; > + return FIELD_GET(PKVM_PAGE_STATE_PROT_MASK, prot); > } > > struct host_mmu { > -- > 2.47.0.338.g60cca15819-goog >
Hey Fuad, On Tuesday 10 Dec 2024 at 12:59:44 (+0000), Fuad Tabba wrote: > Hi Quentin, > > On Tue, 3 Dec 2024 at 10:37, Quentin Perret <qperret@google.com> wrote: > > > > The 'concrete' (a.k.a non-meta) page states are currently encoded using > > software bits in PTEs. For performance reasons, the abstract > > pkvm_page_state enum uses the same bits to encode these states as that > > makes conversions from and to PTEs easy. > > > > In order to prepare the ground for moving the 'concrete' state storage > > to the hyp vmemmap, re-arrange the enum to use bits 0 and 1 for this > > purpose. > > > > No functional changes intended. > > > > Signed-off-by: Quentin Perret <qperret@google.com> > > --- > > arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 17 ++++++++++------- > > 1 file changed, 10 insertions(+), 7 deletions(-) > > > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > index 0972faccc2af..ca3177481b78 100644 > > --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > @@ -24,25 +24,28 @@ > > */ > > enum pkvm_page_state { > > PKVM_PAGE_OWNED = 0ULL, > > - PKVM_PAGE_SHARED_OWNED = KVM_PGTABLE_PROT_SW0, > > - PKVM_PAGE_SHARED_BORROWED = KVM_PGTABLE_PROT_SW1, > > - __PKVM_PAGE_RESERVED = KVM_PGTABLE_PROT_SW0 | > > - KVM_PGTABLE_PROT_SW1, > > + 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, > > + PKVM_NOPAGE = BIT(2), > > }; > > +#define PKVM_PAGE_META_STATES_MASK (~(BIT(0) | BIT(1))) > > > > #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, > > enum pkvm_page_state state) > > { > > - return (prot & ~PKVM_PAGE_STATE_PROT_MASK) | state; > > + BUG_ON(state & PKVM_PAGE_META_STATES_MASK); > > This is a slight change in functionality, having a BUG_ON instead of > just masking out illegal states. Is it necessary? Yep, this is arguably a bit zealous. Passing e.g. PKVM_NOPAGE to pkvm_mkstate() would be properly bogus, so having a WARN_ON() or BUG_ON() in there is still a good thing, but it should be done in a separate patch. I'll rework in v3. > > + prot &= ~PKVM_PAGE_STATE_PROT_MASK; > > + prot |= FIELD_PREP(PKVM_PAGE_STATE_PROT_MASK, state); > > + return prot; > > } > > > > static inline enum pkvm_page_state pkvm_getstate(enum kvm_pgtable_prot prot) > > { > > - return prot & PKVM_PAGE_STATE_PROT_MASK; > > + return FIELD_GET(PKVM_PAGE_STATE_PROT_MASK, prot); > > } > > > > struct host_mmu { > > -- > > 2.47.0.338.g60cca15819-goog > >
diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h index 0972faccc2af..ca3177481b78 100644 --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h @@ -24,25 +24,28 @@ */ enum pkvm_page_state { PKVM_PAGE_OWNED = 0ULL, - PKVM_PAGE_SHARED_OWNED = KVM_PGTABLE_PROT_SW0, - PKVM_PAGE_SHARED_BORROWED = KVM_PGTABLE_PROT_SW1, - __PKVM_PAGE_RESERVED = KVM_PGTABLE_PROT_SW0 | - KVM_PGTABLE_PROT_SW1, + 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, + PKVM_NOPAGE = BIT(2), }; +#define PKVM_PAGE_META_STATES_MASK (~(BIT(0) | BIT(1))) #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, enum pkvm_page_state state) { - return (prot & ~PKVM_PAGE_STATE_PROT_MASK) | state; + BUG_ON(state & PKVM_PAGE_META_STATES_MASK); + prot &= ~PKVM_PAGE_STATE_PROT_MASK; + prot |= FIELD_PREP(PKVM_PAGE_STATE_PROT_MASK, state); + return prot; } static inline enum pkvm_page_state pkvm_getstate(enum kvm_pgtable_prot prot) { - return prot & PKVM_PAGE_STATE_PROT_MASK; + return FIELD_GET(PKVM_PAGE_STATE_PROT_MASK, prot); } struct host_mmu {
The 'concrete' (a.k.a non-meta) page states are currently encoded using software bits in PTEs. For performance reasons, the abstract pkvm_page_state enum uses the same bits to encode these states as that makes conversions from and to PTEs easy. In order to prepare the ground for moving the 'concrete' state storage to the hyp vmemmap, re-arrange the enum to use bits 0 and 1 for this purpose. No functional changes intended. Signed-off-by: Quentin Perret <qperret@google.com> --- arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)