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

[Marc Zyngier <maz@kernel.org>]

On Wed, 11 Dec 2024 00:37:34 +0000,
Pavan Kondeti <quic_pkondeti@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.

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.

Thanks,

	M.

From 4823e7bb868d3ac2b938ecc4c3dbbdd460656af1 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Wed, 11 Dec 2024 10:02:25 +0000
Subject: [PATCH] arm64: Expose kernel ownership of EL2 via a capability

It appears that some drivers have to jump through a lot of hoops
to initialise correctly when running under a particular hypervisor,
while they can directly do it when running bare-metal.

Unfortunately, said hypervisor cannot be directly identified as it
doesn't implement the correct SMCCC interface, leaving the driver
with a certain amount of guesswork.

Being booted at EL2 provides at least an indication that there is no
non-nesting hypervisor, which is good enough to discriminate the
humpy hypervisor.

For this purpose, expose a new system-wide CPU capability aptly named
ARM64_HAS_EL2_OWNERSHIP, which said driver can check.

Note that this doesn't solve the problem of a kernel booted at EL1
without a hypervisor, or with a hypervisor that doesn't break the
device programming interface.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/cpufeature.c | 11 +++++++++++
 arch/arm64/tools/cpucaps       |  1 +
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 36c7b29ddf9e8..8fdc3ef23d9dc 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1868,6 +1868,11 @@ static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)
 		  is_midr_in_range_list(read_cpuid_id(), nv1_ni_list)));
 }
 
+static bool has_el2_ownership(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	return is_hyp_mode_available();
+}
+
 #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
 static bool has_lpa2_at_stage1(u64 mmfr0)
 {
@@ -3012,6 +3017,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
 	},
 #endif
+	{
+		.desc = "Kernel owns EL2",
+		.capability = ARM64_HAS_EL2_OWNERSHIP,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_el2_ownership,
+	},
 	{},
 };
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 1e65f2fb45bd1..94ce3462e6298 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -24,6 +24,7 @@ HAS_DIT
 HAS_E0PD
 HAS_ECV
 HAS_ECV_CNTPOFF
+HAS_EL2_OWNERSHIP
 HAS_EPAN
 HAS_EVT
 HAS_FPMR
-- 
2.39.2


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