| Message ID | 20240701095505.165383-3-steven.price@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Support for running as a guest in Arm CCA | expand |
On Mon, Jul 01, 2024 at 10:54:52AM +0100, Steven Price wrote: > From: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Add a function to test early if PSCI is present and what conduit it > uses. Because the PSCI conduit corresponds to the SMCCC one, this will > let the kernel know whether it can use SMC instructions to discuss with > the Realm Management Monitor (RMM), early enough to enable RAM and > serial access when running in a Realm. > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Signed-off-by: Steven Price <steven.price@arm.com> > --- > v4: New patch > --- > drivers/firmware/psci/psci.c | 25 +++++++++++++++++++++++++ > include/linux/psci.h | 5 +++++ > 2 files changed, 30 insertions(+) > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c > index d9629ff87861..a40dcaf17822 100644 > --- a/drivers/firmware/psci/psci.c > +++ b/drivers/firmware/psci/psci.c > @@ -13,6 +13,7 @@ > #include <linux/errno.h> > #include <linux/linkage.h> > #include <linux/of.h> > +#include <linux/of_fdt.h> > #include <linux/pm.h> > #include <linux/printk.h> > #include <linux/psci.h> > @@ -767,6 +768,30 @@ int __init psci_dt_init(void) > return ret; > } > > +/* > + * Test early if PSCI is supported, and if its conduit matches @conduit > + */ > +bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit) > +{ > + int len; > + int psci_node; > + const char *method; > + unsigned long dt_root; > + > + /* DT hasn't been unflattened yet, we have to work with the flat blob */ > + dt_root = of_get_flat_dt_root(); > + psci_node = of_get_flat_dt_subnode_by_name(dt_root, "psci"); > + if (psci_node <= 0) > + return false; > + > + method = of_get_flat_dt_prop(psci_node, "method", &len); > + if (!method) > + return false; > + > + return (conduit == SMCCC_CONDUIT_SMC && strncmp(method, "smc", len) == 0) || > + (conduit == SMCCC_CONDUIT_HVC && strncmp(method, "hvc", len) == 0); > +} Hmm, I don't think this is sufficient to check for SMCCC reliably. Instead, I think you need to do something more involved: 1. Check for PSCI in the DT 2. Check that the PSCI major version is >= 1 3. Use PSCI_FEATURES to check that you have SMCCC 4. Use SMCCC_VERSION to find out which version of SMCCC you have That's roughly what the PSCI driver does, so we should avoid duplicating that logic. Will
Hi Will! On 09/07/2024 11:48, Will Deacon wrote: > On Mon, Jul 01, 2024 at 10:54:52AM +0100, Steven Price wrote: >> From: Jean-Philippe Brucker <jean-philippe@linaro.org> >> >> Add a function to test early if PSCI is present and what conduit it >> uses. Because the PSCI conduit corresponds to the SMCCC one, this will >> let the kernel know whether it can use SMC instructions to discuss with >> the Realm Management Monitor (RMM), early enough to enable RAM and >> serial access when running in a Realm. >> >> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> v4: New patch >> --- >> drivers/firmware/psci/psci.c | 25 +++++++++++++++++++++++++ >> include/linux/psci.h | 5 +++++ >> 2 files changed, 30 insertions(+) >> >> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c >> index d9629ff87861..a40dcaf17822 100644 >> --- a/drivers/firmware/psci/psci.c >> +++ b/drivers/firmware/psci/psci.c >> @@ -13,6 +13,7 @@ >> #include <linux/errno.h> >> #include <linux/linkage.h> >> #include <linux/of.h> >> +#include <linux/of_fdt.h> >> #include <linux/pm.h> >> #include <linux/printk.h> >> #include <linux/psci.h> >> @@ -767,6 +768,30 @@ int __init psci_dt_init(void) >> return ret; >> } >> >> +/* >> + * Test early if PSCI is supported, and if its conduit matches @conduit >> + */ >> +bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit) >> +{ >> + int len; >> + int psci_node; >> + const char *method; >> + unsigned long dt_root; >> + >> + /* DT hasn't been unflattened yet, we have to work with the flat blob */ >> + dt_root = of_get_flat_dt_root(); >> + psci_node = of_get_flat_dt_subnode_by_name(dt_root, "psci"); >> + if (psci_node <= 0) >> + return false; >> + >> + method = of_get_flat_dt_prop(psci_node, "method", &len); >> + if (!method) >> + return false; >> + >> + return (conduit == SMCCC_CONDUIT_SMC && strncmp(method, "smc", len) == 0) || >> + (conduit == SMCCC_CONDUIT_HVC && strncmp(method, "hvc", len) == 0); >> +} > > Hmm, I don't think this is sufficient to check for SMCCC reliably. > Instead, I think you need to do something more involved: > > 1. Check for PSCI in the DT > 2. Check that the PSCI major version is >= 1 > 3. Use PSCI_FEATURES to check that you have SMCCC > 4. Use SMCCC_VERSION to find out which version of SMCCC you have > > That's roughly what the PSCI driver does, so we should avoid duplicating > that logic. Hmm, you have a point. This was an improvement over the previous (assume that making an SMC call is going to work well enough to return at error for non-realms), but I guess it's technically possible for a system to have PSCI but not implement enough of SMCCC to reliably get an error code back. Do you have any pointers on how pKVM detects it is a guest? Currently we have a couple of things we have to do very early as a realm guest: * Mark memory as RIPAS_RAM. My preference would be to punt this problem to the VMM (or UEFI) and make it a boot requirement. But it gets in the way for attestation purposes as you can't have the same attestation report for VMs which differ only by available RAM in that case. * earlycon - we need to know if the serial device is in unprotected MMIO as that needs mapping with the top IPA bit set. (I think that's all that's really early - everything else I can think of should be after the usual PSCI probe should have happened) Thanks for taking a look! Steve
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index d9629ff87861..a40dcaf17822 100644 --- a/drivers/firmware/psci/psci.c +++ b/drivers/firmware/psci/psci.c @@ -13,6 +13,7 @@ #include <linux/errno.h> #include <linux/linkage.h> #include <linux/of.h> +#include <linux/of_fdt.h> #include <linux/pm.h> #include <linux/printk.h> #include <linux/psci.h> @@ -767,6 +768,30 @@ int __init psci_dt_init(void) return ret; } +/* + * Test early if PSCI is supported, and if its conduit matches @conduit + */ +bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit) +{ + int len; + int psci_node; + const char *method; + unsigned long dt_root; + + /* DT hasn't been unflattened yet, we have to work with the flat blob */ + dt_root = of_get_flat_dt_root(); + psci_node = of_get_flat_dt_subnode_by_name(dt_root, "psci"); + if (psci_node <= 0) + return false; + + method = of_get_flat_dt_prop(psci_node, "method", &len); + if (!method) + return false; + + return (conduit == SMCCC_CONDUIT_SMC && strncmp(method, "smc", len) == 0) || + (conduit == SMCCC_CONDUIT_HVC && strncmp(method, "hvc", len) == 0); +} + #ifdef CONFIG_ACPI /* * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's diff --git a/include/linux/psci.h b/include/linux/psci.h index 4ca0060a3fc4..a1fc1703ba20 100644 --- a/include/linux/psci.h +++ b/include/linux/psci.h @@ -45,8 +45,13 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void); #if defined(CONFIG_ARM_PSCI_FW) int __init psci_dt_init(void); +bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit); #else static inline int psci_dt_init(void) { return 0; } +static inline bool psci_early_test_conduit(enum arm_smccc_conduit conduit) +{ + return false; +} #endif #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)