| Message ID | 4b3e2ead4f392d1a47a7528da119d57918e5d806.1664392886.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | ACPI/IORT: Update SMMUv3 DeviceID support | expand |
On Wed, Sep 28, 2022 at 08:21:26PM +0100, Robin Murphy wrote: > External email: Use caution opening links or attachments > > > IORT E.e now allows SMMUv3 nodes to describe the DeviceID for MSIs > independently of wired GSIVs, where the previous oddly-restrictive > definition meant that an SMMU without PRI support had to provide a > DeviceID even if it didn't support MSIs either. Support this, with > the usual temporary flag definition while the real one is making > its way through ACPICA. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> All the indentations in this patch are using white spaces vs. tabs, so it fails at git-apply. I manually fixed them and tested the PATCH by applying a small revision hack to the IORT binaries: --------- diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 3269a888fb7a..5a4eef7b937c 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -333,8 +333,20 @@ void __iomem __ref return NULL; } - if (!acpi_permanent_mmap) - return __acpi_map_table((unsigned long)phys, size); + if (!acpi_permanent_mmap) { + virt = __acpi_map_table((unsigned long)phys, size); + if (!strncmp((char *)virt, "IORT", 4)) { + u8 *tmp = virt; + int i = 0x30; + while (i < size) { + if (tmp[i] == 0x4) /* SMMUv3 */ + tmp[i + 3] = 0x5; /* Revision */ + i += tmp[i + 1]; /* next node */ + continue; + } + } + return virt; + } mutex_lock(&acpi_ioremap_lock); /* Check if there's a suitable mapping already. */ --------- Once the indentations are fixed, Tested-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Thanks! Nicolin > --- > drivers/acpi/arm64/iort.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index ca2aed86b540..51bc3c1d8d42 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -402,6 +402,10 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node, > return NULL; > } > > +#ifndef ACPI_IORT_SMMU_V3_DEVICEID_VALID > +#define ACPI_IORT_SMMU_V3_DEVICEID_VALID (1 << 4) > +#endif > + > static int iort_get_id_mapping_index(struct acpi_iort_node *node) > { > struct acpi_iort_smmu_v3 *smmu; > @@ -418,12 +422,16 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node) > > smmu = (struct acpi_iort_smmu_v3 *)node->node_data; > /* > - * ID mapping index is only ignored if all interrupts are > - * GSIV based > + * Until IORT E.e (node rev. 5), the ID mapping index was > + * defined to be valid unless all interrupts are GSIV-based. > */ > - if (smmu->event_gsiv && smmu->pri_gsiv && smmu->gerr_gsiv > - && smmu->sync_gsiv) > + if (node->revision < 5) { > + if (smmu->event_gsiv && smmu->pri_gsiv && > + smmu->gerr_gsiv && smmu->sync_gsiv) > + return -EINVAL; > + } else if (!(smmu->flags & ACPI_IORT_SMMU_V3_DEVICEID_VALID)) { > return -EINVAL; > + } > > if (smmu->id_mapping_index >= node->mapping_count) { > pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n", > -- > 2.36.1.dirty >
On 2022-09-29 00:55, Nicolin Chen wrote: > On Wed, Sep 28, 2022 at 08:21:26PM +0100, Robin Murphy wrote: >> External email: Use caution opening links or attachments >> >> >> IORT E.e now allows SMMUv3 nodes to describe the DeviceID for MSIs >> independently of wired GSIVs, where the previous oddly-restrictive >> definition meant that an SMMU without PRI support had to provide a >> DeviceID even if it didn't support MSIs either. Support this, with >> the usual temporary flag definition while the real one is making >> its way through ACPICA. >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> > > All the indentations in this patch are using white spaces vs. tabs, That must be something at your end - they're definitely tabs here, and the copy in the lore archives looks right too. > so it fails at git-apply. I manually fixed them and tested the PATCH > by applying a small revision hack to the IORT binaries: > > --------- > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index 3269a888fb7a..5a4eef7b937c 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -333,8 +333,20 @@ void __iomem __ref > return NULL; > } > > - if (!acpi_permanent_mmap) > - return __acpi_map_table((unsigned long)phys, size); > + if (!acpi_permanent_mmap) { > + virt = __acpi_map_table((unsigned long)phys, size); > + if (!strncmp((char *)virt, "IORT", 4)) { > + u8 *tmp = virt; > + int i = 0x30; > + while (i < size) { > + if (tmp[i] == 0x4) /* SMMUv3 */ > + tmp[i + 3] = 0x5; /* Revision */ > + i += tmp[i + 1]; /* next node */ > + continue; > + } > + } > + return virt; > + } > > mutex_lock(&acpi_ioremap_lock); > /* Check if there's a suitable mapping already. */ > --------- > > Once the indentations are fixed, > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Thanks for confirming! Robin. > > Thanks! > Nicolin > >> --- >> drivers/acpi/arm64/iort.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >> index ca2aed86b540..51bc3c1d8d42 100644 >> --- a/drivers/acpi/arm64/iort.c >> +++ b/drivers/acpi/arm64/iort.c >> @@ -402,6 +402,10 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node, >> return NULL; >> } >> >> +#ifndef ACPI_IORT_SMMU_V3_DEVICEID_VALID >> +#define ACPI_IORT_SMMU_V3_DEVICEID_VALID (1 << 4) >> +#endif >> + >> static int iort_get_id_mapping_index(struct acpi_iort_node *node) >> { >> struct acpi_iort_smmu_v3 *smmu; >> @@ -418,12 +422,16 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node) >> >> smmu = (struct acpi_iort_smmu_v3 *)node->node_data; >> /* >> - * ID mapping index is only ignored if all interrupts are >> - * GSIV based >> + * Until IORT E.e (node rev. 5), the ID mapping index was >> + * defined to be valid unless all interrupts are GSIV-based. >> */ >> - if (smmu->event_gsiv && smmu->pri_gsiv && smmu->gerr_gsiv >> - && smmu->sync_gsiv) >> + if (node->revision < 5) { >> + if (smmu->event_gsiv && smmu->pri_gsiv && >> + smmu->gerr_gsiv && smmu->sync_gsiv) >> + return -EINVAL; >> + } else if (!(smmu->flags & ACPI_IORT_SMMU_V3_DEVICEID_VALID)) { >> return -EINVAL; >> + } >> >> if (smmu->id_mapping_index >= node->mapping_count) { >> pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n", >> -- >> 2.36.1.dirty >>
On Thu, Sep 29, 2022 at 11:22:13AM +0100, Robin Murphy wrote: > External email: Use caution opening links or attachments > > > On 2022-09-29 00:55, Nicolin Chen wrote: > > On Wed, Sep 28, 2022 at 08:21:26PM +0100, Robin Murphy wrote: > > > External email: Use caution opening links or attachments > > > > > > > > > IORT E.e now allows SMMUv3 nodes to describe the DeviceID for MSIs > > > independently of wired GSIVs, where the previous oddly-restrictive > > > definition meant that an SMMU without PRI support had to provide a > > > DeviceID even if it didn't support MSIs either. Support this, with > > > the usual temporary flag definition while the real one is making > > > its way through ACPICA. > > > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > > > > All the indentations in this patch are using white spaces vs. tabs, > > That must be something at your end - they're definitely tabs here, and > the copy in the lore archives looks right too. Oh...it is something wrong on my side. The patch should be fine. Sorry for the confusion.
On Wed, 28 Sep 2022 20:21:26 +0100, Robin Murphy wrote: > IORT E.e now allows SMMUv3 nodes to describe the DeviceID for MSIs > independently of wired GSIVs, where the previous oddly-restrictive > definition meant that an SMMU without PRI support had to provide a > DeviceID even if it didn't support MSIs either. Support this, with > the usual temporary flag definition while the real one is making > its way through ACPICA. > > [...] Applied to arm64 (for-next/acpi), thanks! [1/1] ACPI/IORT: Update SMMUv3 DeviceID support https://git.kernel.org/arm64/c/05da178ce0aa Cheers,
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index ca2aed86b540..51bc3c1d8d42 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -402,6 +402,10 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node, return NULL; } +#ifndef ACPI_IORT_SMMU_V3_DEVICEID_VALID +#define ACPI_IORT_SMMU_V3_DEVICEID_VALID (1 << 4) +#endif + static int iort_get_id_mapping_index(struct acpi_iort_node *node) { struct acpi_iort_smmu_v3 *smmu; @@ -418,12 +422,16 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node) smmu = (struct acpi_iort_smmu_v3 *)node->node_data; /* - * ID mapping index is only ignored if all interrupts are - * GSIV based + * Until IORT E.e (node rev. 5), the ID mapping index was + * defined to be valid unless all interrupts are GSIV-based. */ - if (smmu->event_gsiv && smmu->pri_gsiv && smmu->gerr_gsiv - && smmu->sync_gsiv) + if (node->revision < 5) { + if (smmu->event_gsiv && smmu->pri_gsiv && + smmu->gerr_gsiv && smmu->sync_gsiv) + return -EINVAL; + } else if (!(smmu->flags & ACPI_IORT_SMMU_V3_DEVICEID_VALID)) { return -EINVAL; + } if (smmu->id_mapping_index >= node->mapping_count) { pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
IORT E.e now allows SMMUv3 nodes to describe the DeviceID for MSIs independently of wired GSIVs, where the previous oddly-restrictive definition meant that an SMMU without PRI support had to provide a DeviceID even if it didn't support MSIs either. Support this, with the usual temporary flag definition while the real one is making its way through ACPICA. Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- drivers/acpi/arm64/iort.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)