Re: [PATCH v3 23/27] KVM: arm64: nv: Add SVC trap forwarding

From: Eric Auger <eric.auger@redhat.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Brown <broonie@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Andre Przywara <andre.przywara@arm.com>,
	Chase Conklin <chase.conklin@arm.com>,
	Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>,
	Darren Hart <darren@os.amperecomputing.com>,
	Miguel Luis <miguel.luis@oracle.com>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: [PATCH v3 23/27] KVM: arm64: nv: Add SVC trap forwarding
Date: Thu, 10 Aug 2023 19:30:25 +0200	[thread overview]
Message-ID: <527eddd0-b069-3b58-d82e-97b758c128ab@redhat.com> (raw)
In-Reply-To: <87wmy3p4ac.wl-maz@kernel.org>

Hi Marc,
On 8/10/23 12:42, Marc Zyngier wrote:
> Hi Eric,
>
> On Thu, 10 Aug 2023 09:35:41 +0100,
> Eric Auger <eric.auger@redhat.com> wrote:
>> Hi Marc,
>>
>> On 8/8/23 13:47, Marc Zyngier wrote:
>>> HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these
>>> traps to be forwarded. Take this opportunity to deny any 32bit activity
>>> when NV is enabled.
>> I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are
>> handled. Please could you explain.
> - SVC: KVM itself never traps it, so any trap of SVC must be the
>   result of a guest trap -- we don't need to do any demultiplexing. We
>   thus directly inject the trap back. This is what the comment in
>   handle_svc() tries to capture, but obviously fails to convey the
>   point.
Thank you for the explanation. Now I get it and this helps.
>
> - ERET: This is already handled since 6898a55ce38c ("KVM: arm64: nv:
>   Handle trapped ERET from virtual EL2"). Similarly to SVC, KVM never
>   traps it unless we run NV.
OK
>
> Now, looking into it, I think I'm missing the additional case where
> the L2 guest runs at vEL1. I'm about to add the following patchlet:
>
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 3b86d534b995..617ae6dea5d5 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -222,7 +222,22 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu)
>  	if (kvm_vcpu_get_esr(vcpu) & ESR_ELx_ERET_ISS_ERET)
>  		return kvm_handle_ptrauth(vcpu);
>  
> -	kvm_emulate_nested_eret(vcpu);
> +	/*
> +	 * If we got here, two possibilities:
> +	 *
> +	 * - the guest is in EL2, and we need to fully emulate ERET
> +	 *
> +	 * - the guest is in EL1, and we need to reinject the
> +         *   exception into the L1 hypervisor.
but in the case the guest was running in vEL1 are we supposed to trap
and end up here? in kvm_emulate_nested_eret I can see
"the current EL is always the vEL2 since we set the HCR_EL2.NV bit only
when entering the vEL2". But I am still catching up on the already landed

[PATCH 00/18] KVM: arm64: Prefix patches for NV support <https://lore.kernel.org/all/20230209175820.1939006-1-maz@kernel.org/>  so please forgive me my confusion ;-)

Thanks

Eric

> +	 *
> +	 * If KVM ever traps ERET for its own use, we'll have to
> +	 * revisit this.
> +	 */
> +	if (is_hyp_ctxt(vcpu))
> +		kvm_emulate_nested_eret(vcpu);
> +	else
> +		kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu));
> +
>  	return 1;
>  }
>  
>
> Does the above help?
>
> Thanks,
>
> 	M.
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel