On Wed, Dec 11, 2024 at 10:40:02AM +0000, Marc Zyngier wrote:
> On Wed, 11 Dec 2024 00:37:34 +0000,
> Pavan Kondeti <quic_pkond...@quicinc.com> wrote:
> >
> > On Tue, Dec 10, 2024 at 09:24:03PM +0000, Marc Zyngier wrote:
> > > > +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.
> > >
> >
> > Thanks Marc. Any suggestions on how we can make is_hyp_mode_available()
> > available for modules? Do you prefer exporting
> > kvm_protected_mode_initialized and __boot_cpu_mode symbols directly or
> > try something like [1]?
>
> Ideally, neither. These were bad ideas nine years ago, and they still
> are. The least ugly hack I can come up with is the patch below, and
> you'd write something like:
>
> if (cpus_have_cap(ARM64_HAS_EL2_OWNERSHIP))
> blah();
>
> This is obviously completely untested.
>
I have tested your patch. It works as intended. Thanks Marc.
> It also doesn't solve the problem of the kernel booted on bare-metal
> at EL1, or with a hypervisor that doesn't change the programming
> interface of the device under the guest's feet. Eventually, someone
> will have to address these cases.
>
Noted, Thanks.
~Pavan
