Re: [PATCH] drm/msm/a6xx: Skip gpu secure fw load in EL2 mode

[Marc Zyngier <maz@kernel.org>]

On Mon, 09 Dec 2024 08:19:15 +0000,
Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
> 
> When kernel is booted in EL2, SECVID registers are accessible to the
> KMD. So we can use that to switch GPU's secure mode to avoid dependency
> on Zap firmware. Also, we can't load a secure firmware without a
> hypervisor that supports it.
> 
> Tested following configurations on sa8775p chipset (Adreno 663 gpu):
> 
> 1. Gunyah (No KVM) - Loads zap shader based on DT
> 2. KVM in VHE - Skips zap shader load and programs SECVID register
> 3. KVM in nVHE - Loads zap shader based on DT
> 4. Kernel in EL2 with CONFIG_KVM=n - Skips zap shader load and
> 	programs SECVID register
>
> For (1) and (3) configuration, this patch doesn't have any impact.
> Driver loads secure firmware based on other existing hints.

The moment the kernel is entered at EL2, this is a bare metal
situation, and everything is accessible. So your distinction between
VHE and nVHE (which would equally apply to hVHE and pKVM) makes no
sense at all. Same thing for KVM being disabled, which has no bearing
on what can be accessed.

> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 82 +++++++++++++++++++++++------------
>  1 file changed, 54 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 019610341df1..9dcaa8472430 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -14,6 +14,10 @@
>  #include <linux/pm_domain.h>
>  #include <linux/soc/qcom/llcc-qcom.h>
>  
> +#ifdef CONFIG_ARM64
> +#include <asm/virt.h>
> +#endif

How about 32bit ARM?

> +
>  #define GPU_PAS_ID 13
>  
>  static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
> @@ -998,6 +1002,54 @@ static int a6xx_zap_shader_init(struct msm_gpu *gpu)
>  	return ret;
>  }
>  
> +static int a6xx_switch_secure_mode(struct msm_gpu *gpu)
> +{
> +	int ret;
> +
> +#ifdef CONFIG_ARM64
> +	/*
> +	 * We can access SECVID_TRUST_CNTL register when kernel is booted in EL2 mode. So, use it
> +	 * to switch the secure mode to avoid the dependency on zap shader.
> +	 */
> +	if (is_kernel_in_hyp_mode())
> +		goto direct_switch;

No, please. To check whether you are *booted* at EL2, you need to
check for is_hyp_available(). Whether the kernel runs at EL1 or EL2 is
none of the driver's business, really. This is still absolutely
disgusting from an abstraction perspective, but I guess we don't have
much choice here.

In the future, for anything involving KVM, please Cc the maintainers,
reviewers and mailing list listed in the MAINTAINERS file.

Thanks,

	M.

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