Hi Alex,
On Thu, 13 Jan 2022 12:12:04 +0000,
Alexandru Elisei <alexandru.elisei@xxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxx>
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.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
