* [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-17 11:33 ` Guanghui Feng
0 siblings, 0 replies; 27+ messages in thread
From: Guanghui Feng @ 2023-07-17 11:33 UTC (permalink / raw)
To: lpieralisi, guohanjun, sudeep.holla, rafael
Cc: linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
According to the ARM IORT specifications DEN 0049 issue E,
the "Number of IDs" field in the ID mapping format reports
the number of IDs in the mapping range minus one.
In iort_node_get_rmr_info(), we erroneously skip ID mappings
whose "Number of IDs" equal to 0, resulting in valid mapping
nodes with a single ID to map being skipped, which is wrong.
Fix iort_node_get_rmr_info() by removing the bogus id_count
check.
Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
---
drivers/acpi/arm64/iort.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 3631230..56d8873 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
for (i = 0; i < node->mapping_count; i++, map++) {
struct acpi_iort_node *parent;
- if (!map->id_count)
- continue;
-
parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
map->output_reference);
if (parent != iommu)
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-17 11:33 ` Guanghui Feng
@ 2023-07-18 7:52 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-18 7:52 UTC (permalink / raw)
To: Guanghui Feng
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
[+Catalin, Will, Shameer]
On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> Fix iort_node_get_rmr_info() by removing the bogus id_count
> check.
>
> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> ---
> drivers/acpi/arm64/iort.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 3631230..56d8873 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> for (i = 0; i < node->mapping_count; i++, map++) {
> struct acpi_iort_node *parent;
>
> - if (!map->id_count)
> - continue;
> -
> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> map->output_reference);
> if (parent != iommu)
Shameer, I know this may look like overkill since the hunk we are
removing is buggy but can you please test this patch on platforms
with RMR to make sure we are not triggering regressions by removing
it (by the specs that's what should be done but current firmware
is always something to reckon with) ?
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-18 7:52 ` Lorenzo Pieralisi
0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-18 7:52 UTC (permalink / raw)
To: Guanghui Feng
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
[+Catalin, Will, Shameer]
On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> Fix iort_node_get_rmr_info() by removing the bogus id_count
> check.
>
> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> ---
> drivers/acpi/arm64/iort.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 3631230..56d8873 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> for (i = 0; i < node->mapping_count; i++, map++) {
> struct acpi_iort_node *parent;
>
> - if (!map->id_count)
> - continue;
> -
> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> map->output_reference);
> if (parent != iommu)
Shameer, I know this may look like overkill since the hunk we are
removing is buggy but can you please test this patch on platforms
with RMR to make sure we are not triggering regressions by removing
it (by the specs that's what should be done but current firmware
is always something to reckon with) ?
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-18 7:52 ` Lorenzo Pieralisi
@ 2023-07-18 8:54 ` guanghui.fgh
-1 siblings, 0 replies; 27+ messages in thread
From: guanghui.fgh @ 2023-07-18 8:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
在 2023/7/18 15:52, Lorenzo Pieralisi 写道:
> [+Catalin, Will, Shameer]
>
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
>> According to the ARM IORT specifications DEN 0049 issue E,
>> the "Number of IDs" field in the ID mapping format reports
>> the number of IDs in the mapping range minus one.
>>
>> In iort_node_get_rmr_info(), we erroneously skip ID mappings
>> whose "Number of IDs" equal to 0, resulting in valid mapping
>> nodes with a single ID to map being skipped, which is wrong.
>>
>> Fix iort_node_get_rmr_info() by removing the bogus id_count
>> check.
>>
>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
>> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>> ---
>> drivers/acpi/arm64/iort.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 3631230..56d8873 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
>> for (i = 0; i < node->mapping_count; i++, map++) {
>> struct acpi_iort_node *parent;
>>
>> - if (!map->id_count)
>> - continue;
>> -
>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> map->output_reference);
>> if (parent != iommu)
>
> Shameer, I know this may look like overkill since the hunk we are
> removing is buggy but can you please test this patch on platforms
> with RMR to make sure we are not triggering regressions by removing
> it (by the specs that's what should be done but current firmware
> is always something to reckon with) ?
>
> Thanks,
> Lorenzo
Thanks for your response.
Firstly, I have tested iort rmr with one ID mapping and works well.
Secondly, When there is more than two id mappings, the buggly id_count
check has no effect(for the id_count is larger than zero).
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-18 8:54 ` guanghui.fgh
0 siblings, 0 replies; 27+ messages in thread
From: guanghui.fgh @ 2023-07-18 8:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
在 2023/7/18 15:52, Lorenzo Pieralisi 写道:
> [+Catalin, Will, Shameer]
>
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
>> According to the ARM IORT specifications DEN 0049 issue E,
>> the "Number of IDs" field in the ID mapping format reports
>> the number of IDs in the mapping range minus one.
>>
>> In iort_node_get_rmr_info(), we erroneously skip ID mappings
>> whose "Number of IDs" equal to 0, resulting in valid mapping
>> nodes with a single ID to map being skipped, which is wrong.
>>
>> Fix iort_node_get_rmr_info() by removing the bogus id_count
>> check.
>>
>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
>> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>> ---
>> drivers/acpi/arm64/iort.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 3631230..56d8873 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
>> for (i = 0; i < node->mapping_count; i++, map++) {
>> struct acpi_iort_node *parent;
>>
>> - if (!map->id_count)
>> - continue;
>> -
>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> map->output_reference);
>> if (parent != iommu)
>
> Shameer, I know this may look like overkill since the hunk we are
> removing is buggy but can you please test this patch on platforms
> with RMR to make sure we are not triggering regressions by removing
> it (by the specs that's what should be done but current firmware
> is always something to reckon with) ?
>
> Thanks,
> Lorenzo
Thanks for your response.
Firstly, I have tested iort rmr with one ID mapping and works well.
Secondly, When there is more than two id mappings, the buggly id_count
check has no effect(for the id_count is larger than zero).
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-18 8:54 ` guanghui.fgh
@ 2023-07-18 9:16 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-18 9:16 UTC (permalink / raw)
To: guanghui.fgh
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
On Tue, Jul 18, 2023 at 04:54:36PM +0800, guanghui.fgh wrote:
>
>
> 在 2023/7/18 15:52, Lorenzo Pieralisi 写道:
> > [+Catalin, Will, Shameer]
> >
> > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > According to the ARM IORT specifications DEN 0049 issue E,
> > > the "Number of IDs" field in the ID mapping format reports
> > > the number of IDs in the mapping range minus one.
> > >
> > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > nodes with a single ID to map being skipped, which is wrong.
> > >
> > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > check.
> > >
> > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > ---
> > > drivers/acpi/arm64/iort.c | 3 ---
> > > 1 file changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index 3631230..56d8873 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> > > for (i = 0; i < node->mapping_count; i++, map++) {
> > > struct acpi_iort_node *parent;
> > > - if (!map->id_count)
> > > - continue;
> > > -
> > > parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> > > map->output_reference);
> > > if (parent != iommu)
> >
> > Shameer, I know this may look like overkill since the hunk we are
> > removing is buggy but can you please test this patch on platforms
> > with RMR to make sure we are not triggering regressions by removing
> > it (by the specs that's what should be done but current firmware
> > is always something to reckon with) ?
> >
> > Thanks,
> > Lorenzo
> Thanks for your response.
>
> Firstly, I have tested iort rmr with one ID mapping and works well.
>
> Secondly, When there is more than two id mappings, the buggly id_count check
> has no effect(for the id_count is larger than zero).
Thank you, I wasn't asking you, it is comforting to know you tested the
patch before sending it though.
I asked Shameer to make sure the id_count check did not mask a bug
in the platforms on which the RMR IORT implementation was initially
tested - that's it, I don't want to merge a fix and revert it the
next day.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-18 9:16 ` Lorenzo Pieralisi
0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-18 9:16 UTC (permalink / raw)
To: guanghui.fgh
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas, shameerali.kolothum.thodi
On Tue, Jul 18, 2023 at 04:54:36PM +0800, guanghui.fgh wrote:
>
>
> 在 2023/7/18 15:52, Lorenzo Pieralisi 写道:
> > [+Catalin, Will, Shameer]
> >
> > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > According to the ARM IORT specifications DEN 0049 issue E,
> > > the "Number of IDs" field in the ID mapping format reports
> > > the number of IDs in the mapping range minus one.
> > >
> > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > nodes with a single ID to map being skipped, which is wrong.
> > >
> > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > check.
> > >
> > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > ---
> > > drivers/acpi/arm64/iort.c | 3 ---
> > > 1 file changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index 3631230..56d8873 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> > > for (i = 0; i < node->mapping_count; i++, map++) {
> > > struct acpi_iort_node *parent;
> > > - if (!map->id_count)
> > > - continue;
> > > -
> > > parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> > > map->output_reference);
> > > if (parent != iommu)
> >
> > Shameer, I know this may look like overkill since the hunk we are
> > removing is buggy but can you please test this patch on platforms
> > with RMR to make sure we are not triggering regressions by removing
> > it (by the specs that's what should be done but current firmware
> > is always something to reckon with) ?
> >
> > Thanks,
> > Lorenzo
> Thanks for your response.
>
> Firstly, I have tested iort rmr with one ID mapping and works well.
>
> Secondly, When there is more than two id mappings, the buggly id_count check
> has no effect(for the id_count is larger than zero).
Thank you, I wasn't asking you, it is comforting to know you tested the
patch before sending it though.
I asked Shameer to make sure the id_count check did not mask a bug
in the platforms on which the RMR IORT implementation was initially
tested - that's it, I don't want to merge a fix and revert it the
next day.
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-18 7:52 ` Lorenzo Pieralisi
@ 2023-07-18 8:56 ` Shameerali Kolothum Thodi
-1 siblings, 0 replies; 27+ messages in thread
From: Shameerali Kolothum Thodi @ 2023-07-18 8:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Guanghui Feng
Cc: Guohanjun (Hanjun Guo), sudeep.holla@arm.com, rafael@kernel.org,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, baolin.wang@linux.alibaba.com,
alikernel-developer@linux.alibaba.com, will@kernel.org,
catalin.marinas@arm.com
> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lpieralisi@kernel.org]
> Sent: 18 July 2023 08:52
> To: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> Cc: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> sudeep.holla@arm.com; rafael@kernel.org; linux-acpi@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> baolin.wang@linux.alibaba.com; alikernel-developer@linux.alibaba.com;
> will@kernel.org; catalin.marinas@arm.com; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in
> iort_node_get_rmr_info()
>
> [+Catalin, Will, Shameer]
>
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > According to the ARM IORT specifications DEN 0049 issue E,
> > the "Number of IDs" field in the ID mapping format reports
> > the number of IDs in the mapping range minus one.
> >
> > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > whose "Number of IDs" equal to 0, resulting in valid mapping
> > nodes with a single ID to map being skipped, which is wrong.
> >
> > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > check.
> >
> > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR
> reserved regions")
> > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > ---
> > drivers/acpi/arm64/iort.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 3631230..56d8873 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct
> acpi_iort_node *node,
> > for (i = 0; i < node->mapping_count; i++, map++) {
> > struct acpi_iort_node *parent;
> >
> > - if (!map->id_count)
> > - continue;
> > -
> > parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> > map->output_reference);
> > if (parent != iommu)
>
> Shameer, I know this may look like overkill since the hunk we are
> removing is buggy but can you please test this patch on platforms
> with RMR to make sure we are not triggering regressions by removing
> it (by the specs that's what should be done but current firmware
> is always something to reckon with) ?
Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested
Hanjun to help with the testing as I don't have a test setup with me now.
Hanjun, please help.
Thanks,
Shameer
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-18 8:56 ` Shameerali Kolothum Thodi
0 siblings, 0 replies; 27+ messages in thread
From: Shameerali Kolothum Thodi @ 2023-07-18 8:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Guanghui Feng
Cc: Guohanjun (Hanjun Guo), sudeep.holla@arm.com, rafael@kernel.org,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, baolin.wang@linux.alibaba.com,
alikernel-developer@linux.alibaba.com, will@kernel.org,
catalin.marinas@arm.com
> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lpieralisi@kernel.org]
> Sent: 18 July 2023 08:52
> To: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> Cc: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> sudeep.holla@arm.com; rafael@kernel.org; linux-acpi@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> baolin.wang@linux.alibaba.com; alikernel-developer@linux.alibaba.com;
> will@kernel.org; catalin.marinas@arm.com; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in
> iort_node_get_rmr_info()
>
> [+Catalin, Will, Shameer]
>
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > According to the ARM IORT specifications DEN 0049 issue E,
> > the "Number of IDs" field in the ID mapping format reports
> > the number of IDs in the mapping range minus one.
> >
> > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > whose "Number of IDs" equal to 0, resulting in valid mapping
> > nodes with a single ID to map being skipped, which is wrong.
> >
> > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > check.
> >
> > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR
> reserved regions")
> > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > ---
> > drivers/acpi/arm64/iort.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 3631230..56d8873 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct
> acpi_iort_node *node,
> > for (i = 0; i < node->mapping_count; i++, map++) {
> > struct acpi_iort_node *parent;
> >
> > - if (!map->id_count)
> > - continue;
> > -
> > parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> > map->output_reference);
> > if (parent != iommu)
>
> Shameer, I know this may look like overkill since the hunk we are
> removing is buggy but can you please test this patch on platforms
> with RMR to make sure we are not triggering regressions by removing
> it (by the specs that's what should be done but current firmware
> is always something to reckon with) ?
Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested
Hanjun to help with the testing as I don't have a test setup with me now.
Hanjun, please help.
Thanks,
Shameer
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-18 8:56 ` Shameerali Kolothum Thodi
@ 2023-07-19 9:13 ` Hanjun Guo
-1 siblings, 0 replies; 27+ messages in thread
From: Hanjun Guo @ 2023-07-19 9:13 UTC (permalink / raw)
To: Shameerali Kolothum Thodi, Lorenzo Pieralisi, Guanghui Feng
Cc: sudeep.holla@arm.com, rafael@kernel.org,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, baolin.wang@linux.alibaba.com,
alikernel-developer@linux.alibaba.com, will@kernel.org,
catalin.marinas@arm.com
On 2023/7/18 16:56, Shameerali Kolothum Thodi wrote:
>> [+Catalin, Will, Shameer]
>>
>> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
>>> According to the ARM IORT specifications DEN 0049 issue E,
>>> the "Number of IDs" field in the ID mapping format reports
>>> the number of IDs in the mapping range minus one.
>>>
>>> In iort_node_get_rmr_info(), we erroneously skip ID mappings
>>> whose "Number of IDs" equal to 0, resulting in valid mapping
>>> nodes with a single ID to map being skipped, which is wrong.
>>>
>>> Fix iort_node_get_rmr_info() by removing the bogus id_count
>>> check.
>>>
>>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR
>> reserved regions")
>>> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
>>> ---
>>> drivers/acpi/arm64/iort.c | 3 ---
>>> 1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index 3631230..56d8873 100644
>>> --- a/drivers/acpi/arm64/iort.c
>>> +++ b/drivers/acpi/arm64/iort.c
>>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct
>> acpi_iort_node *node,
>>> for (i = 0; i < node->mapping_count; i++, map++) {
>>> struct acpi_iort_node *parent;
>>>
>>> - if (!map->id_count)
>>> - continue;
>>> -
>>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>>> map->output_reference);
>>> if (parent != iommu)
>> Shameer, I know this may look like overkill since the hunk we are
>> removing is buggy but can you please test this patch on platforms
>> with RMR to make sure we are not triggering regressions by removing
>> it (by the specs that's what should be done but current firmware
>> is always something to reckon with) ?
> Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested
> Hanjun to help with the testing as I don't have a test setup with me now.
Valid fix for me as well, we had a firmware bug which reported the
numbers of ID as 1 when we only have one ID mapping, so remove the
check is fine for the old firmware, but to make it sure, we need some
test before give it a pass.
>
> Hanjun, please help.
I need some time to get it properly tested on two versions of firmware,
and get the test machine properly setup, please allow me give the
feedback next week.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-19 9:13 ` Hanjun Guo
0 siblings, 0 replies; 27+ messages in thread
From: Hanjun Guo @ 2023-07-19 9:13 UTC (permalink / raw)
To: Shameerali Kolothum Thodi, Lorenzo Pieralisi, Guanghui Feng
Cc: sudeep.holla@arm.com, rafael@kernel.org,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, baolin.wang@linux.alibaba.com,
alikernel-developer@linux.alibaba.com, will@kernel.org,
catalin.marinas@arm.com
On 2023/7/18 16:56, Shameerali Kolothum Thodi wrote:
>> [+Catalin, Will, Shameer]
>>
>> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
>>> According to the ARM IORT specifications DEN 0049 issue E,
>>> the "Number of IDs" field in the ID mapping format reports
>>> the number of IDs in the mapping range minus one.
>>>
>>> In iort_node_get_rmr_info(), we erroneously skip ID mappings
>>> whose "Number of IDs" equal to 0, resulting in valid mapping
>>> nodes with a single ID to map being skipped, which is wrong.
>>>
>>> Fix iort_node_get_rmr_info() by removing the bogus id_count
>>> check.
>>>
>>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR
>> reserved regions")
>>> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
>>> ---
>>> drivers/acpi/arm64/iort.c | 3 ---
>>> 1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index 3631230..56d8873 100644
>>> --- a/drivers/acpi/arm64/iort.c
>>> +++ b/drivers/acpi/arm64/iort.c
>>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct
>> acpi_iort_node *node,
>>> for (i = 0; i < node->mapping_count; i++, map++) {
>>> struct acpi_iort_node *parent;
>>>
>>> - if (!map->id_count)
>>> - continue;
>>> -
>>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>>> map->output_reference);
>>> if (parent != iommu)
>> Shameer, I know this may look like overkill since the hunk we are
>> removing is buggy but can you please test this patch on platforms
>> with RMR to make sure we are not triggering regressions by removing
>> it (by the specs that's what should be done but current firmware
>> is always something to reckon with) ?
> Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested
> Hanjun to help with the testing as I don't have a test setup with me now.
Valid fix for me as well, we had a firmware bug which reported the
numbers of ID as 1 when we only have one ID mapping, so remove the
check is fine for the old firmware, but to make it sure, we need some
test before give it a pass.
>
> Hanjun, please help.
I need some time to get it properly tested on two versions of firmware,
and get the test machine properly setup, please allow me give the
feedback next week.
Thanks
Hanjun
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-19 9:13 ` Hanjun Guo
(?)
@ 2023-07-26 9:07 ` Hanjun Guo
-1 siblings, 0 replies; 27+ messages in thread
From: Hanjun Guo @ 2023-07-26 9:07 UTC (permalink / raw)
To: Shameerali Kolothum Thodi, Lorenzo Pieralisi, Guanghui Feng
Cc: sudeep.holla@arm.com, rafael@kernel.org,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, baolin.wang@linux.alibaba.com,
alikernel-developer@linux.alibaba.com, will@kernel.org,
catalin.marinas@arm.com
On 2023/7/19 17:13, Hanjun Guo wrote:
> On 2023/7/18 16:56, Shameerali Kolothum Thodi wrote:
>>> [+Catalin, Will, Shameer]
[...]
>>> Shameer, I know this may look like overkill since the hunk we are
>>> removing is buggy but can you please test this patch on platforms
>>> with RMR to make sure we are not triggering regressions by removing
>>> it (by the specs that's what should be done but current firmware
>>> is always something to reckon with) ?
>> Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I
>> have requested
>> Hanjun to help with the testing as I don't have a test setup with me now.
>
> Valid fix for me as well, we had a firmware bug which reported the
> numbers of ID as 1 when we only have one ID mapping, so remove the
> check is fine for the old firmware, but to make it sure, we need some
> test before give it a pass.
>
>>
>> Hanjun, please help.
>
> I need some time to get it properly tested on two versions of firmware,
> and get the test machine properly setup, please allow me give the
> feedback next week.
No regressions were found,
Tested-by: Hanjun Guo <guohanjun@huawei.com>
Thanks
Hanjun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-17 11:33 ` Guanghui Feng
@ 2023-07-27 13:39 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 13:39 UTC (permalink / raw)
To: Guanghui Feng
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas
[+Catalin, Will]
On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> Fix iort_node_get_rmr_info() by removing the bogus id_count
> check.
>
> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> ---
> drivers/acpi/arm64/iort.c | 3 ---
> 1 file changed, 3 deletions(-)
Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Catalin/Will,
can you pick this up please ?
Thanks,
Lorenzo
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 3631230..56d8873 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> for (i = 0; i < node->mapping_count; i++, map++) {
> struct acpi_iort_node *parent;
>
> - if (!map->id_count)
> - continue;
> -
> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> map->output_reference);
> if (parent != iommu)
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-27 13:39 ` Lorenzo Pieralisi
0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 13:39 UTC (permalink / raw)
To: Guanghui Feng
Cc: guohanjun, sudeep.holla, rafael, linux-acpi, linux-arm-kernel,
linux-kernel, baolin.wang, alikernel-developer, will,
catalin.marinas
[+Catalin, Will]
On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> Fix iort_node_get_rmr_info() by removing the bogus id_count
> check.
>
> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> ---
> drivers/acpi/arm64/iort.c | 3 ---
> 1 file changed, 3 deletions(-)
Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Catalin/Will,
can you pick this up please ?
Thanks,
Lorenzo
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 3631230..56d8873 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> for (i = 0; i < node->mapping_count; i++, map++) {
> struct acpi_iort_node *parent;
>
> - if (!map->id_count)
> - continue;
> -
> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> map->output_reference);
> if (parent != iommu)
> --
> 1.8.3.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-27 13:39 ` Lorenzo Pieralisi
@ 2023-07-27 13:59 ` Catalin Marinas
-1 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-27 13:59 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guanghui Feng, guohanjun, sudeep.holla, rafael, linux-acpi,
linux-arm-kernel, linux-kernel, baolin.wang, alikernel-developer,
will
On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > According to the ARM IORT specifications DEN 0049 issue E,
> > the "Number of IDs" field in the ID mapping format reports
> > the number of IDs in the mapping range minus one.
> >
> > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > whose "Number of IDs" equal to 0, resulting in valid mapping
> > nodes with a single ID to map being skipped, which is wrong.
> >
> > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > check.
> >
> > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > ---
> > drivers/acpi/arm64/iort.c | 3 ---
> > 1 file changed, 3 deletions(-)
>
> Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
>
> Catalin/Will,
>
> can you pick this up please ?
Would you like it merged in 6.5 or 6.6 is fine?
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-27 13:59 ` Catalin Marinas
0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-27 13:59 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guanghui Feng, guohanjun, sudeep.holla, rafael, linux-acpi,
linux-arm-kernel, linux-kernel, baolin.wang, alikernel-developer,
will
On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > According to the ARM IORT specifications DEN 0049 issue E,
> > the "Number of IDs" field in the ID mapping format reports
> > the number of IDs in the mapping range minus one.
> >
> > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > whose "Number of IDs" equal to 0, resulting in valid mapping
> > nodes with a single ID to map being skipped, which is wrong.
> >
> > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > check.
> >
> > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > ---
> > drivers/acpi/arm64/iort.c | 3 ---
> > 1 file changed, 3 deletions(-)
>
> Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
>
> Catalin/Will,
>
> can you pick this up please ?
Would you like it merged in 6.5 or 6.6 is fine?
Thanks.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-27 13:59 ` Catalin Marinas
@ 2023-07-27 14:17 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 14:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: Guanghui Feng, guohanjun, sudeep.holla, rafael, linux-acpi,
linux-arm-kernel, linux-kernel, baolin.wang, alikernel-developer,
will
On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > According to the ARM IORT specifications DEN 0049 issue E,
> > > the "Number of IDs" field in the ID mapping format reports
> > > the number of IDs in the mapping range minus one.
> > >
> > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > nodes with a single ID to map being skipped, which is wrong.
> > >
> > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > check.
> > >
> > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > ---
> > > drivers/acpi/arm64/iort.c | 3 ---
> > > 1 file changed, 3 deletions(-)
> >
> > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> >
> > Catalin/Will,
> >
> > can you pick this up please ?
>
> Would you like it merged in 6.5 or 6.6 is fine?
It is definitely a fix - I don't think that's super urgent
though, bug has been there since v6.0 so the fix can probably
wait to trickle back from v6.6.
Thanks,
Lorenzo
> Thanks.
>
> --
> Catalin
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-27 14:17 ` Lorenzo Pieralisi
0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 14:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: Guanghui Feng, guohanjun, sudeep.holla, rafael, linux-acpi,
linux-arm-kernel, linux-kernel, baolin.wang, alikernel-developer,
will
On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > According to the ARM IORT specifications DEN 0049 issue E,
> > > the "Number of IDs" field in the ID mapping format reports
> > > the number of IDs in the mapping range minus one.
> > >
> > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > nodes with a single ID to map being skipped, which is wrong.
> > >
> > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > check.
> > >
> > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > ---
> > > drivers/acpi/arm64/iort.c | 3 ---
> > > 1 file changed, 3 deletions(-)
> >
> > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> >
> > Catalin/Will,
> >
> > can you pick this up please ?
>
> Would you like it merged in 6.5 or 6.6 is fine?
It is definitely a fix - I don't think that's super urgent
though, bug has been there since v6.0 so the fix can probably
wait to trickle back from v6.6.
Thanks,
Lorenzo
> Thanks.
>
> --
> Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-27 14:17 ` Lorenzo Pieralisi
@ 2023-07-27 15:00 ` Will Deacon
-1 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2023-07-27 15:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Catalin Marinas, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > the "Number of IDs" field in the ID mapping format reports
> > > > the number of IDs in the mapping range minus one.
> > > >
> > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > nodes with a single ID to map being skipped, which is wrong.
> > > >
> > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > check.
> > > >
> > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > ---
> > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > 1 file changed, 3 deletions(-)
> > >
> > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > >
> > > Catalin/Will,
> > >
> > > can you pick this up please ?
> >
> > Would you like it merged in 6.5 or 6.6 is fine?
>
> It is definitely a fix - I don't think that's super urgent
> though, bug has been there since v6.0 so the fix can probably
> wait to trickle back from v6.6.
On the flip side, it's a fix, Hanjun has tested it and we have a few
weeks in case we need to revert it.
So I'd be inclined to say Catalin should pick it up for 6.5, unless
there's a good reason to wait?
Will
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-27 15:00 ` Will Deacon
0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2023-07-27 15:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Catalin Marinas, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > the "Number of IDs" field in the ID mapping format reports
> > > > the number of IDs in the mapping range minus one.
> > > >
> > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > nodes with a single ID to map being skipped, which is wrong.
> > > >
> > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > check.
> > > >
> > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > ---
> > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > 1 file changed, 3 deletions(-)
> > >
> > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > >
> > > Catalin/Will,
> > >
> > > can you pick this up please ?
> >
> > Would you like it merged in 6.5 or 6.6 is fine?
>
> It is definitely a fix - I don't think that's super urgent
> though, bug has been there since v6.0 so the fix can probably
> wait to trickle back from v6.6.
On the flip side, it's a fix, Hanjun has tested it and we have a few
weeks in case we need to revert it.
So I'd be inclined to say Catalin should pick it up for 6.5, unless
there's a good reason to wait?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-27 15:00 ` Will Deacon
@ 2023-07-27 15:08 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 15:08 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 04:00:43PM +0100, Will Deacon wrote:
> On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> > On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > > the "Number of IDs" field in the ID mapping format reports
> > > > > the number of IDs in the mapping range minus one.
> > > > >
> > > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > > nodes with a single ID to map being skipped, which is wrong.
> > > > >
> > > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > > check.
> > > > >
> > > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > > ---
> > > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > > 1 file changed, 3 deletions(-)
> > > >
> > > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > >
> > > > Catalin/Will,
> > > >
> > > > can you pick this up please ?
> > >
> > > Would you like it merged in 6.5 or 6.6 is fine?
> >
> > It is definitely a fix - I don't think that's super urgent
> > though, bug has been there since v6.0 so the fix can probably
> > wait to trickle back from v6.6.
>
> On the flip side, it's a fix, Hanjun has tested it and we have a few
> weeks in case we need to revert it.
>
> So I'd be inclined to say Catalin should pick it up for 6.5, unless
> there's a good reason to wait?
No reason to wait - I just wanted to make clear it is not urgent,
if it is possible it makes sense for it to get into v6.5.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-27 15:08 ` Lorenzo Pieralisi
0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Pieralisi @ 2023-07-27 15:08 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 04:00:43PM +0100, Will Deacon wrote:
> On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> > On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > > the "Number of IDs" field in the ID mapping format reports
> > > > > the number of IDs in the mapping range minus one.
> > > > >
> > > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > > nodes with a single ID to map being skipped, which is wrong.
> > > > >
> > > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > > check.
> > > > >
> > > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > > ---
> > > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > > 1 file changed, 3 deletions(-)
> > > >
> > > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > >
> > > > Catalin/Will,
> > > >
> > > > can you pick this up please ?
> > >
> > > Would you like it merged in 6.5 or 6.6 is fine?
> >
> > It is definitely a fix - I don't think that's super urgent
> > though, bug has been there since v6.0 so the fix can probably
> > wait to trickle back from v6.6.
>
> On the flip side, it's a fix, Hanjun has tested it and we have a few
> weeks in case we need to revert it.
>
> So I'd be inclined to say Catalin should pick it up for 6.5, unless
> there's a good reason to wait?
No reason to wait - I just wanted to make clear it is not urgent,
if it is possible it makes sense for it to get into v6.5.
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-27 15:08 ` Lorenzo Pieralisi
@ 2023-07-28 12:05 ` Catalin Marinas
-1 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-28 12:05 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Will Deacon, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 05:08:20PM +0200, Lorenzo Pieralisi wrote:
> On Thu, Jul 27, 2023 at 04:00:43PM +0100, Will Deacon wrote:
> > On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> > > On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > > > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > > > the "Number of IDs" field in the ID mapping format reports
> > > > > > the number of IDs in the mapping range minus one.
> > > > > >
> > > > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > > > nodes with a single ID to map being skipped, which is wrong.
> > > > > >
> > > > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > > > check.
> > > > > >
> > > > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > > > ---
> > > > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > > > 1 file changed, 3 deletions(-)
> > > > >
> > > > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > > >
> > > > > Catalin/Will,
> > > > >
> > > > > can you pick this up please ?
> > > >
> > > > Would you like it merged in 6.5 or 6.6 is fine?
> > >
> > > It is definitely a fix - I don't think that's super urgent
> > > though, bug has been there since v6.0 so the fix can probably
> > > wait to trickle back from v6.6.
> >
> > On the flip side, it's a fix, Hanjun has tested it and we have a few
> > weeks in case we need to revert it.
> >
> > So I'd be inclined to say Catalin should pick it up for 6.5, unless
> > there's a good reason to wait?
>
> No reason to wait - I just wanted to make clear it is not urgent,
> if it is possible it makes sense for it to get into v6.5.
Happy to pick it up.
--
Catalin
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-28 12:05 ` Catalin Marinas
0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-28 12:05 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Will Deacon, Guanghui Feng, guohanjun, sudeep.holla, rafael,
linux-acpi, linux-arm-kernel, linux-kernel, baolin.wang,
alikernel-developer
On Thu, Jul 27, 2023 at 05:08:20PM +0200, Lorenzo Pieralisi wrote:
> On Thu, Jul 27, 2023 at 04:00:43PM +0100, Will Deacon wrote:
> > On Thu, Jul 27, 2023 at 04:17:49PM +0200, Lorenzo Pieralisi wrote:
> > > On Thu, Jul 27, 2023 at 02:59:02PM +0100, Catalin Marinas wrote:
> > > > On Thu, Jul 27, 2023 at 03:39:27PM +0200, Lorenzo Pieralisi wrote:
> > > > > On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote:
> > > > > > According to the ARM IORT specifications DEN 0049 issue E,
> > > > > > the "Number of IDs" field in the ID mapping format reports
> > > > > > the number of IDs in the mapping range minus one.
> > > > > >
> > > > > > In iort_node_get_rmr_info(), we erroneously skip ID mappings
> > > > > > whose "Number of IDs" equal to 0, resulting in valid mapping
> > > > > > nodes with a single ID to map being skipped, which is wrong.
> > > > > >
> > > > > > Fix iort_node_get_rmr_info() by removing the bogus id_count
> > > > > > check.
> > > > > >
> > > > > > Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR reserved regions")
> > > > > > Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> > > > > > ---
> > > > > > drivers/acpi/arm64/iort.c | 3 ---
> > > > > > 1 file changed, 3 deletions(-)
> > > > >
> > > > > Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > > >
> > > > > Catalin/Will,
> > > > >
> > > > > can you pick this up please ?
> > > >
> > > > Would you like it merged in 6.5 or 6.6 is fine?
> > >
> > > It is definitely a fix - I don't think that's super urgent
> > > though, bug has been there since v6.0 so the fix can probably
> > > wait to trickle back from v6.6.
> >
> > On the flip side, it's a fix, Hanjun has tested it and we have a few
> > weeks in case we need to revert it.
> >
> > So I'd be inclined to say Catalin should pick it up for 6.5, unless
> > there's a good reason to wait?
>
> No reason to wait - I just wanted to make clear it is not urgent,
> if it is possible it makes sense for it to get into v6.5.
Happy to pick it up.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
2023-07-17 11:33 ` Guanghui Feng
@ 2023-07-28 13:55 ` Catalin Marinas
-1 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-28 13:55 UTC (permalink / raw)
To: lpieralisi, guohanjun, sudeep.holla, rafael, Guanghui Feng
Cc: Will Deacon, linux-acpi, linux-arm-kernel, linux-kernel,
baolin.wang, alikernel-developer
On Mon, 17 Jul 2023 19:33:45 +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
https://git.kernel.org/arm64/c/003e6b56d780
--
Catalin
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
@ 2023-07-28 13:55 ` Catalin Marinas
0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2023-07-28 13:55 UTC (permalink / raw)
To: lpieralisi, guohanjun, sudeep.holla, rafael, Guanghui Feng
Cc: Will Deacon, linux-acpi, linux-arm-kernel, linux-kernel,
baolin.wang, alikernel-developer
On Mon, 17 Jul 2023 19:33:45 +0800, Guanghui Feng wrote:
> According to the ARM IORT specifications DEN 0049 issue E,
> the "Number of IDs" field in the ID mapping format reports
> the number of IDs in the mapping range minus one.
>
> In iort_node_get_rmr_info(), we erroneously skip ID mappings
> whose "Number of IDs" equal to 0, resulting in valid mapping
> nodes with a single ID to map being skipped, which is wrong.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
https://git.kernel.org/arm64/c/003e6b56d780
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread