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

Akhil P Oommen quic_akhilpo at quicinc.com
Mon Dec 9 12:52:27 PST 2024


On 12/10/2024 1:24 AM, Rob Clark wrote:
> On Mon, Dec 9, 2024 at 12:20 AM Akhil P Oommen <quic_akhilpo at 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.
> 
> Shouldn't we do this based on whether zap node is in dtb (and not disabled)?

This is better, isn't it? Otherwise, multiple overlays should be
maintained for each soc/board since EL2 can be toggled from bootloader.
And this feature is likely going to be more widely available.

-Akhil.

> 
> slbounce applies some dtb overlays to disable the zap node when
> booting in EL2 (and make some other changes due to kernel being in
> control of the pci smmuv3, or something along those lines).
> 
> BR,
> -R
> 
>>
>> 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.
>>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo at 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
>> +
>>  #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;
>> +#endif
>> +
>> +       /*
>> +        * Try to load a zap shader into the secure world. If successful
>> +        * we can use the CP to switch out of secure mode. If not then we
>> +        * have no resource but to try to switch ourselves out manually. If we
>> +        * guessed wrong then access to the RBBM_SECVID_TRUST_CNTL register will
>> +        * be blocked and a permissions violation will soon follow.
>> +        */
>> +       ret = a6xx_zap_shader_init(gpu);
>> +       if (ret == -ENODEV) {
>> +               /*
>> +                * This device does not use zap shader (but print a warning
>> +                * just in case someone got their dt wrong.. hopefully they
>> +                * have a debug UART to realize the error of their ways...
>> +                * if you mess this up you are about to crash horribly)
>> +                */
>> +               dev_warn_once(gpu->dev->dev,
>> +                       "Zap shader not enabled - using SECVID_TRUST_CNTL instead\n");
>> +               goto direct_switch;
>> +       } else if (ret)
>> +               return ret;
>> +
>> +       OUT_PKT7(gpu->rb[0], CP_SET_SECURE_MODE, 1);
>> +       OUT_RING(gpu->rb[0], 0x00000000);
>> +
>> +       a6xx_flush(gpu, gpu->rb[0]);
>> +       if (!a6xx_idle(gpu, gpu->rb[0]))
>> +               return -EINVAL;
>> +
>> +       return 0;
>> +
>> +direct_switch:
>> +       gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
>> +       return 0;
>> +}
>> +
>>  #define A6XX_INT_MASK (A6XX_RBBM_INT_0_MASK_CP_AHB_ERROR | \
>>                        A6XX_RBBM_INT_0_MASK_RBBM_ATB_ASYNCFIFO_OVERFLOW | \
>>                        A6XX_RBBM_INT_0_MASK_CP_HW_ERROR | \
>> @@ -1341,35 +1393,9 @@ static int hw_init(struct msm_gpu *gpu)
>>         if (ret)
>>                 goto out;
>>
>> -       /*
>> -        * Try to load a zap shader into the secure world. If successful
>> -        * we can use the CP to switch out of secure mode. If not then we
>> -        * have no resource but to try to switch ourselves out manually. If we
>> -        * guessed wrong then access to the RBBM_SECVID_TRUST_CNTL register will
>> -        * be blocked and a permissions violation will soon follow.
>> -        */
>> -       ret = a6xx_zap_shader_init(gpu);
>> -       if (!ret) {
>> -               OUT_PKT7(gpu->rb[0], CP_SET_SECURE_MODE, 1);
>> -               OUT_RING(gpu->rb[0], 0x00000000);
>> -
>> -               a6xx_flush(gpu, gpu->rb[0]);
>> -               if (!a6xx_idle(gpu, gpu->rb[0]))
>> -                       return -EINVAL;
>> -       } else if (ret == -ENODEV) {
>> -               /*
>> -                * This device does not use zap shader (but print a warning
>> -                * just in case someone got their dt wrong.. hopefully they
>> -                * have a debug UART to realize the error of their ways...
>> -                * if you mess this up you are about to crash horribly)
>> -                */
>> -               dev_warn_once(gpu->dev->dev,
>> -                       "Zap shader not enabled - using SECVID_TRUST_CNTL instead\n");
>> -               gpu_write(gpu, REG_A6XX_RBBM_SECVID_TRUST_CNTL, 0x0);
>> -               ret = 0;
>> -       } else {
>> +       ret = a6xx_switch_secure_mode(gpu);
>> +       if (!ret)
>>                 return ret;
>> -       }
>>
>>  out:
>>         if (adreno_has_gmu_wrapper(adreno_gpu))
>>
>> ---
>> base-commit: f4a867a46862c1743501bbe8c813238456ec8699
>> change-id: 20241120-drm-msm-kvm-support-cd6e6744ced6
>>
>> Best regards,
>> --
>> Akhil P Oommen <quic_akhilpo at quicinc.com>
>>




More information about the linux-arm-kernel mailing list