All of lore.kernel.org
 help / color / mirror / Atom feed
From: "guanghui.fgh" <guanghuifeng@linux.alibaba.com>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: 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@huawei.com
Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
Date: Tue, 18 Jul 2023 16:54:36 +0800	[thread overview]
Message-ID: <8e2cfde3-15fa-4e5a-02bf-0763abf10821@linux.alibaba.com> (raw)
In-Reply-To: <ZLZEq0QBBW4rcxJM@lpieralisi>



在 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).

WARNING: multiple messages have this Message-ID (diff)
From: "guanghui.fgh" <guanghuifeng@linux.alibaba.com>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: 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@huawei.com
Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info()
Date: Tue, 18 Jul 2023 16:54:36 +0800	[thread overview]
Message-ID: <8e2cfde3-15fa-4e5a-02bf-0763abf10821@linux.alibaba.com> (raw)
In-Reply-To: <ZLZEq0QBBW4rcxJM@lpieralisi>



在 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

  reply	other threads:[~2023-07-18  8:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-17 11:33 [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info() Guanghui Feng
2023-07-17 11:33 ` Guanghui Feng
2023-07-18  7:52 ` Lorenzo Pieralisi
2023-07-18  7:52   ` Lorenzo Pieralisi
2023-07-18  8:54   ` guanghui.fgh [this message]
2023-07-18  8:54     ` guanghui.fgh
2023-07-18  9:16     ` Lorenzo Pieralisi
2023-07-18  9:16       ` Lorenzo Pieralisi
2023-07-18  8:56   ` Shameerali Kolothum Thodi
2023-07-18  8:56     ` Shameerali Kolothum Thodi
2023-07-19  9:13     ` Hanjun Guo
2023-07-19  9:13       ` Hanjun Guo
2023-07-26  9:07       ` Hanjun Guo
2023-07-27 13:39 ` Lorenzo Pieralisi
2023-07-27 13:39   ` Lorenzo Pieralisi
2023-07-27 13:59   ` Catalin Marinas
2023-07-27 13:59     ` Catalin Marinas
2023-07-27 14:17     ` Lorenzo Pieralisi
2023-07-27 14:17       ` Lorenzo Pieralisi
2023-07-27 15:00       ` Will Deacon
2023-07-27 15:00         ` Will Deacon
2023-07-27 15:08         ` Lorenzo Pieralisi
2023-07-27 15:08           ` Lorenzo Pieralisi
2023-07-28 12:05           ` Catalin Marinas
2023-07-28 12:05             ` Catalin Marinas
2023-07-28 13:55 ` Catalin Marinas
2023-07-28 13:55   ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e2cfde3-15fa-4e5a-02bf-0763abf10821@linux.alibaba.com \
    --to=guanghuifeng@linux.alibaba.com \
    --cc=alikernel-developer@linux.alibaba.com \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=catalin.marinas@arm.com \
    --cc=guohanjun@huawei.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=rafael@kernel.org \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=sudeep.holla@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.