Re: [PATCH 0/4] KVM: arm64: nv: HAF fixes

[Marc Zyngier <maz@kernel.org>]

On Sun, 30 Nov 2025 13:11:57 +0000,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Fri, 28 Nov 2025 10:09:42 +0000,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> > 
> > Based on kvmarm's next branch.
> > 
> > HAF support for the software translation table walker was merged while I
> > was in the process of reading the patches, so instead of comments I have
> > these few fixes.
> > 
> > One thing I didn't touch is this sequence in hyp_set_prot_attr():
> > 
> > 	if (prot & KVM_PGTABLE_PROT_X) {
> > 		/* don't set the XN bit */
> > 	} else {
> > 		attr |= KVM_PTE_LEAF_ATTR_HI_S1_XN;
> > 	}
> > 
> > If the caller is executing in nVHE mode, the translation regime is EL2,
> > which has only PrivExecute permission. Since KVM_PGTABLE_PROT_X is now the
> > union of PrivExecute and UnprivExecute, if the caller requests only the
> > UnprivExecute permission, but no PrivExecute permission, the function does
> > not return an error code and sets the PrivExecute permission.
> 
> I don't think this is a huge problem *right now*, as long as we don't
> have anything that looks like "hvhe hypervisor userspace" (yes, I
> proposed that a while ago, and haven't completely dropped the
> idea). But at the same time, the page-table code should probably be
> built to the architecture and not to the use cases.
> 
> But it also outlines a rather bad bug in the hVHE case, where we set
> the UXN bit instead of the PXN bit...
> 
> What I have in mind is something like this, untested. Thoughts?
> 
> 	M.
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index d57c12f074a40..48305118ba3c5 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -88,6 +88,8 @@ typedef u64 kvm_pte_t;
>  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
>  
>  #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> +#define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> +#define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
>  
>  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
>  
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index e0bd6a0172729..cbf9b6b58e284 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -330,6 +330,11 @@ struct hyp_map_data {
>  	kvm_pte_t			attr;
>  };
>  
> +static bool el2_nvhe(void)
> +{
> +	return !has_vhe() && !cpus_have_final_cap(ARM64_KVM_PROTECTED_MODE);

Obviously, this should read ARM64_KVM_HVHE, not PROTECTED_MODE...

	M.

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