| Message ID | 20210301074021.20059-1-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot | expand |
[+ Lorenzo] On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > laptop. Let's check asl_compiler_id in IORT header to ensure we are > running a QCOM SMMU and create qcom_smmu_impl for it. > > !np is used to check ACPI boot, because fwnode of SMMU device is > a static allocation and thus has_acpi_companion() doesn't work here. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > 1 file changed, 33 insertions(+) I don't know what a "asl_compiler_id" is, but it doesn't sound like it has an awful lot to do with the SMMU. Lorenzo -- any idea what we should be doing here instead? Probably not using ACPI? Will
On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > [+ Lorenzo] > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > > laptop. Let's check asl_compiler_id in IORT header to ensure we are > > running a QCOM SMMU and create qcom_smmu_impl for it. > > > > !np is used to check ACPI boot, because fwnode of SMMU device is > > a static allocation and thus has_acpi_companion() doesn't work here. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > --- > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it > has an awful lot to do with the SMMU. > I would prefer that we somehow relate this to the particular board, rather than all Qualcomm-related ACPI tables. E.g. by relying on the SMMU devices having a _HID of QCOM0409. Shawn, any reason for this wouldn't be possible? > Lorenzo -- any idea what we should be doing here instead? Probably not > using ACPI? > The 8cx (aka sc8180x) platform comes with Qualcomm's usual SMMU stream-mapping quirks and this is one of the patches needed to bring enough ACPI support to run the Debian installer that Shawn has been working on. After the installer we currently only boot this using DT - which already enables the quirk. Regards, Bjorn
On 2021-03-25 17:02, Bjorn Andersson wrote: > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > >> [+ Lorenzo] >> >> On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: >>> Though qcom_adreno_smmu_impl is not used by ACPI boot right now, >>> qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G >>> laptop. Let's check asl_compiler_id in IORT header to ensure we are >>> running a QCOM SMMU and create qcom_smmu_impl for it. >>> >>> !np is used to check ACPI boot, because fwnode of SMMU device is >>> a static allocation and thus has_acpi_companion() doesn't work here. >>> >>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org> >>> --- >>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ >>> 1 file changed, 33 insertions(+) >> >> I don't know what a "asl_compiler_id" is, but it doesn't sound like it >> has an awful lot to do with the SMMU. >> > > I would prefer that we somehow relate this to the particular board, > rather than all Qualcomm-related ACPI tables. E.g. by relying on the > SMMU devices having a _HID of QCOM0409. > > Shawn, any reason for this wouldn't be possible? To do something broadly similar to identify HiSilicon's SMMU PMCG implementation, we use acpi_match_platform_list() - could we do the same for this? Robin. >> Lorenzo -- any idea what we should be doing here instead? Probably not >> using ACPI? >> > > The 8cx (aka sc8180x) platform comes with Qualcomm's usual SMMU > stream-mapping quirks and this is one of the patches needed to bring > enough ACPI support to run the Debian installer that Shawn has been > working on. After the installer we currently only boot this using DT - > which already enables the quirk. > > Regards, > Bjorn >
On Thu, Mar 25, 2021 at 12:02:56PM -0500, Bjorn Andersson wrote: > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > > > [+ Lorenzo] > > > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > > > laptop. Let's check asl_compiler_id in IORT header to ensure we are > > > running a QCOM SMMU and create qcom_smmu_impl for it. > > > > > > !np is used to check ACPI boot, because fwnode of SMMU device is > > > a static allocation and thus has_acpi_companion() doesn't work here. > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > --- > > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > > > 1 file changed, 33 insertions(+) > > > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it > > has an awful lot to do with the SMMU. > > > > I would prefer that we somehow relate this to the particular board, > rather than all Qualcomm-related ACPI tables. E.g. by relying on the > SMMU devices having a _HID of QCOM0409. > > Shawn, any reason for this wouldn't be possible? The arm-smmu driver is probing a device created from IORT table rather DSDT, and there is no info like HID that can be found around the device. Shawn
On Thu, Mar 25, 2021 at 05:07:52PM +0000, Robin Murphy wrote: > On 2021-03-25 17:02, Bjorn Andersson wrote: > > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > > > > > [+ Lorenzo] > > > > > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > > > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > > > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > > > > laptop. Let's check asl_compiler_id in IORT header to ensure we are > > > > running a QCOM SMMU and create qcom_smmu_impl for it. > > > > > > > > !np is used to check ACPI boot, because fwnode of SMMU device is > > > > a static allocation and thus has_acpi_companion() doesn't work here. > > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > --- > > > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > > > > 1 file changed, 33 insertions(+) > > > > > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it > > > has an awful lot to do with the SMMU. > > > > > > > I would prefer that we somehow relate this to the particular board, > > rather than all Qualcomm-related ACPI tables. E.g. by relying on the > > SMMU devices having a _HID of QCOM0409. > > > > Shawn, any reason for this wouldn't be possible? > > To do something broadly similar to identify HiSilicon's SMMU PMCG > implementation, we use acpi_match_platform_list() - could we do the same for > this? Thanks for the suggestion, Robin! Yes, acpi_platform_list is useful for our problem as well. But we will need multiple entries even for a single Snapdragon SoC. For example, both Lenovo Flex 5G and Microsoft Surface Pro X are powered by Snapdragon SC8180X, but their IORT tables have different fields that acpi_match_platform_list() is looking at. If this is not a problem, yeah, acpi_platform_list is a better way out, and I will start updating the patch. Shawn * Lenovo Flex 5G [000h 0000 4] Signature : "IORT" [IO Remapping Table] [004h 0004 4] Table Length : 00001943 [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : DA [00Ah 0010 6] Oem ID : "LENOVO" [010h 0016 8] Oem Table ID : "CB-01 " [018h 0024 4] Oem Revision : 00008180 [01Ch 0028 4] Asl Compiler ID : "QCOM" [020h 0032 4] Asl Compiler Revision : 00000001 * Microsoft Surface Pro X [000h 0000 4] Signature : "IORT" [IO Remapping Table] [004h 0004 4] Table Length : 0000169C [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : 3A [00Ah 0010 6] Oem ID : "QCOM " [010h 0016 8] Oem Table ID : "QCOMEDK2" [018h 0024 4] Oem Revision : 00008180 [01Ch 0028 4] Asl Compiler ID : "QCOM" [020h 0032 4] Asl Compiler Revision : 00000001
On Thu, Mar 25, 2021 at 12:02:56PM -0500, Bjorn Andersson wrote: > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > > > [+ Lorenzo] > > > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > > > laptop. Let's check asl_compiler_id in IORT header to ensure we are > > > running a QCOM SMMU and create qcom_smmu_impl for it. > > > > > > !np is used to check ACPI boot, because fwnode of SMMU device is > > > a static allocation and thus has_acpi_companion() doesn't work here. > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > --- > > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > > > 1 file changed, 33 insertions(+) > > > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it > > has an awful lot to do with the SMMU. > > > > I would prefer that we somehow relate this to the particular board, > rather than all Qualcomm-related ACPI tables. E.g. by relying on the > SMMU devices having a _HID of QCOM0409. > > Shawn, any reason for this wouldn't be possible? > > > Lorenzo -- any idea what we should be doing here instead? Probably not > > using ACPI? > > > > The 8cx (aka sc8180x) platform comes with Qualcomm's usual SMMU > stream-mapping quirks and this is one of the patches needed to bring > enough ACPI support to run the Debian installer that Shawn has been > working on. After the installer we currently only boot this using DT - > which already enables the quirk. I am not sure I follow - can you explain please why this patch (and so the QCOM SMMU) is actually needed ? I don't get why getting the SMMU up and running with ACPI is mandatory to complete the process you describe above (but I am not sure I understood it entirely either - apologies). Thanks, Lorenzo
On Tue 06 Apr 11:23 CDT 2021, Lorenzo Pieralisi wrote: > On Thu, Mar 25, 2021 at 12:02:56PM -0500, Bjorn Andersson wrote: > > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote: > > > > > [+ Lorenzo] > > > > > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote: > > > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now, > > > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G > > > > laptop. Let's check asl_compiler_id in IORT header to ensure we are > > > > running a QCOM SMMU and create qcom_smmu_impl for it. > > > > > > > > !np is used to check ACPI boot, because fwnode of SMMU device is > > > > a static allocation and thus has_acpi_companion() doesn't work here. > > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > --- > > > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ > > > > 1 file changed, 33 insertions(+) > > > > > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it > > > has an awful lot to do with the SMMU. > > > > > > > I would prefer that we somehow relate this to the particular board, > > rather than all Qualcomm-related ACPI tables. E.g. by relying on the > > SMMU devices having a _HID of QCOM0409. > > > > Shawn, any reason for this wouldn't be possible? > > > > > Lorenzo -- any idea what we should be doing here instead? Probably not > > > using ACPI? > > > > > > > The 8cx (aka sc8180x) platform comes with Qualcomm's usual SMMU > > stream-mapping quirks and this is one of the patches needed to bring > > enough ACPI support to run the Debian installer that Shawn has been > > working on. After the installer we currently only boot this using DT - > > which already enables the quirk. > > I am not sure I follow - can you explain please why this patch (and so > the QCOM SMMU) is actually needed ? I don't get why getting the SMMU > up and running with ACPI is mandatory to complete the process you describe > above (but I am not sure I understood it entirely either - apologies). > The bootloader sets up stream mappings for things such as storage and display before jumping to Linux and as things are implemented today the arm-smmu driver is probed. The problem that arises, which is the reason for this patch, is that when the arm-smmu driver resets the SMMU it wipes the stream mappings and the next time the display hardware tries to scan out the EFIFB a fault is triggered - of the type that happens to trap into one of the higher security levels - which results in the device rebooting. The handling of this is implemented by 07a7f2caaa5a ("iommu/arm-smmu-qcom: Read back stream mappings") and f9081b8ff593 ("iommu/arm-smmu-qcom: Implement S2CR quirk"), which are activated based on DT compatibles today. What Shawn is looking for is a way to enable this quirk for the ACPI case as well. Regards, Bjorn
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 98b3a1c2a181..5e8779483367 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -3,6 +3,7 @@ * Copyright (c) 2019, The Linux Foundation. All rights reserved. */ +#include <linux/acpi_iort.h> #include <linux/adreno-smmu-priv.h> #include <linux/of_device.h> #include <linux/qcom_scm.h> @@ -339,10 +340,42 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { { } }; +#ifdef CONFIG_ACPI +static bool is_qcom_iort(struct arm_smmu_device *smmu) +{ + struct acpi_table_header *iort; + acpi_status status; + bool ret = false; + + status = acpi_get_table(ACPI_SIG_IORT, 0, &iort); + if (ACPI_FAILURE(status)) { + dev_err(smmu->dev, "failed to get IORT\n"); + goto done; + } + + if (strncmp(iort->asl_compiler_id, "QCOM", 4) == 0) { + ret = true; + goto done; + } + +done: + acpi_put_table(iort); + return ret; +} +#else +static bool is_qcom_iort(struct arm_smmu_device *smmu) +{ + return false; +} +#endif + struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) { const struct device_node *np = smmu->dev->of_node; + if (!np && is_qcom_iort(smmu)) + return qcom_smmu_create(smmu, &qcom_smmu_impl); + if (of_match_node(qcom_smmu_impl_of_match, np)) return qcom_smmu_create(smmu, &qcom_smmu_impl);
Though qcom_adreno_smmu_impl is not used by ACPI boot right now, qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G laptop. Let's check asl_compiler_id in IORT header to ensure we are running a QCOM SMMU and create qcom_smmu_impl for it. !np is used to check ACPI boot, because fwnode of SMMU device is a static allocation and thus has_acpi_companion() doesn't work here. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) -- 2.17.1