[PATCH v5 05/69] KVM: arm64: Allow preservation of the S2 SW bits

[Marc Zyngier]

Hi Alex,

On Thu, 13 Jan 2022 12:12:04 +0000,
Alexandru Elisei <alexandru.elisei at arm.com> wrote:
> 
> Hi Marc,
> 
> On Mon, Nov 29, 2021 at 08:00:46PM +0000, Marc Zyngier wrote:
> > The S2 page table code has a limited use the SW bits, but we are about
> > to need them to encode some guest Stage-2 information (its mapping size
> > in the form of the TTL encoding).
> > 
> > Propagate the SW bits specified by the caller, and store them into
> > the corresponding entry.
> > 
> > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > ---
> >  arch/arm64/kvm/hyp/pgtable.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index 8cdbc43fa651..d69e400b2de6 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -1064,9 +1064,6 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> >  	u32 level;
> >  	kvm_pte_t set = 0, clr = 0;
> >  
> > -	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
> > -		return -EINVAL;
> > -
> >  	if (prot & KVM_PGTABLE_PROT_R)
> >  		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> >  
> > @@ -1076,6 +1073,10 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> >  	if (prot & KVM_PGTABLE_PROT_X)
> >  		clr |= KVM_PTE_LEAF_ATTR_HI_S2_XN;
> >  
> > +	/* Always propagate the SW bits */
> > +	clr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_SW, 0xf);
> 
> Nitpick: isn't that the same as:
> 
> 	clr |= KVM_PTE_LEAF_ATTR_HI_SW;
> 
> which looks more readable to me.
> 
> > +	set |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> 
> Checked stage2_attr_walker() callbak, first it clears the bits in clr, then
> sets the bits in set, so this looks correct to me:
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei at arm.com>

Thanks. However, I have now dropped this patch since as it turns out,
the PTE update does preserve pre-existing SW bits. I am now carrying
this:

@@ -1212,6 +1218,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
         * kvm_pgtable_stage2_map() should be called to change block size.
         */
        if (fault_status == FSC_PERM && vma_pagesize == fault_granule) {
+               /*
+                * Drop the SW bits in favour of those stored in the
+                * PTE, which will be preserved.
+                */
+               prot &= ~KVM_NV_GUEST_MAP_SZ;
                ret = kvm_pgtable_stage2_relax_perms(pgt, fault_ipa, prot);
        } else {
                ret = kvm_pgtable_stage2_map(pgt, fault_ipa, vma_pagesize,

as part of the patch that tags the shadow S2 with the guest's S2
mapping size (or level, which amounts to the same thing).

Thanks,

	M.

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