| Message ID | 20211129200150.351436-6-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: ARMv8.3/8.4 Nested Virtualization support | expand |
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@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@arm.com> Thanks, Alex > + > ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level); > if (!ret) > kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level); > -- > 2.30.2 >
Hi Alex, On Thu, 13 Jan 2022 12:12:04 +0000, Alexandru Elisei <alexandru.elisei@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@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@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.
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 I think the opening clause needs to be rewritten. > 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@kernel.org> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
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); + set |= prot & KVM_PTE_LEAF_ATTR_HI_SW; + ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level); if (!ret) kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, pgt->mmu, addr, level);
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@kernel.org> --- arch/arm64/kvm/hyp/pgtable.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)