[PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Shawn Guo]

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(+)

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);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Will Deacon]

[+ 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Bjorn Andersson]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Robin Murphy]

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Shawn Guo]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Shawn Guo]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Lorenzo Pieralisi]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

[Bjorn Andersson]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel