Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1572856 > unrolled thread
| Started by | Hanjun Guo <hanjun.guo@linaro.org> |
|---|---|
| First post | 2017-02-03 03:30 +0100 |
| Last post | 2017-02-03 10:00 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v2] ACPI/IORT: Fix iort_node_get_id() mapping entries indexing Hanjun Guo <hanjun.guo@linaro.org> - 2017-02-03 03:30 +0100
Re: [PATCH v2] ACPI/IORT: Fix iort_node_get_id() mapping entries indexing Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-02-03 10:00 +0100
| From | Hanjun Guo <hanjun.guo@linaro.org> |
|---|---|
| Date | 2017-02-03 03:30 +0100 |
| Subject | Re: [PATCH v2] ACPI/IORT: Fix iort_node_get_id() mapping entries indexing |
| Message-ID | <t6BNo-3QZ-5@gated-at.bofh.it> |
On 01/10/2017 08:00 PM, Lorenzo Pieralisi wrote:
> Commit 618f535a6062 ("ACPI/IORT: Add single mapping function")
> introduced a function (iort_node_get_id()) to retrieve ids for IORT
> named components.
>
> The iort_node_get_id() takes an index as input to refer to a specific
> mapping entry in the named component IORT node mapping array.
>
> For a mapping entry at a given index, iort_node_get_id() should return
> the id value (through the id_out function parameter) and the IORT node
> output_reference (through function return value) the given mapping entry
> refers to.
>
> Technically output_reference values may differ for different map
> entries, (see diagram below - mapped id values may refer to different eg
> IORT SMMU nodes; the kernel may not be able to handle different
> output_reference values for a given named component but the IORT kernel
> layer should still report the IORT mappings as reported by firmware) but
> current code in iort_node_get_id() fails to use the index function
> parameter to return the correct output_reference value (ie it always
> returns the output_reference value of the first entry in the mapping
> array whilst using the index correctly to retrieve the id value from the
> respective entry).
>
> |----------------------|
> | named component |
> |----------------------|
> | map entry[0] |
> |----------------------|
> | id value |
> | output_reference----------------> eg SMMU 1
> |----------------------|
> | map entry[1] |
> |----------------------|
> | id value |
> | output_reference----------------> eg SMMU 2
> |----------------------|
> .
> .
> .
> |----------------------|
> | map entry[N] |
> |----------------------|
> | id value |
> | output_reference----------------> eg SMMU 1
> |----------------------|
>
> Consequently the iort_node_get_id() function always returns the IORT
> node pointed at by the output_reference value of the first named
> component mapping array entry, irrespective of the index parameter,
> which is a bug.
>
> Update the map array entry pointer computation in iort_node_get_id() to
> take into account the index value, fixing the issue.
>
> Fixes: 618f535a6062 ("ACPI/IORT: Add single mapping function")
> Reported-by: Hanjun Guo <hanjun.guo@linaro.org>
> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sinan Kaya <okaya@codeaurora.org>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Nate Watterson <nwatters@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> v1 -> v2:
> - Updated/improved commit log
> - Added review tags
Forgot to mention that I tested this patch on Hisilicon
D03,
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
BTW, Lorenzo, Rafael, since it's a bugfix, can this be merged into
4.10-rc7?
Thanks
Hanjun
[toc] | [next] | [standalone]
| From | Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> |
|---|---|
| Date | 2017-02-03 10:00 +0100 |
| Message-ID | <t6HSO-7MR-5@gated-at.bofh.it> |
| In reply to | #1572856 |
On Fri, Feb 03, 2017 at 10:20:29AM +0800, Hanjun Guo wrote:
> On 01/10/2017 08:00 PM, Lorenzo Pieralisi wrote:
> >Commit 618f535a6062 ("ACPI/IORT: Add single mapping function")
> >introduced a function (iort_node_get_id()) to retrieve ids for IORT
> >named components.
> >
> >The iort_node_get_id() takes an index as input to refer to a specific
> >mapping entry in the named component IORT node mapping array.
> >
> >For a mapping entry at a given index, iort_node_get_id() should return
> >the id value (through the id_out function parameter) and the IORT node
> >output_reference (through function return value) the given mapping entry
> >refers to.
> >
> >Technically output_reference values may differ for different map
> >entries, (see diagram below - mapped id values may refer to different eg
> >IORT SMMU nodes; the kernel may not be able to handle different
> >output_reference values for a given named component but the IORT kernel
> >layer should still report the IORT mappings as reported by firmware) but
> >current code in iort_node_get_id() fails to use the index function
> >parameter to return the correct output_reference value (ie it always
> >returns the output_reference value of the first entry in the mapping
> >array whilst using the index correctly to retrieve the id value from the
> >respective entry).
> >
> > |----------------------|
> > | named component |
> > |----------------------|
> > | map entry[0] |
> > |----------------------|
> > | id value |
> > | output_reference----------------> eg SMMU 1
> > |----------------------|
> > | map entry[1] |
> > |----------------------|
> > | id value |
> > | output_reference----------------> eg SMMU 2
> > |----------------------|
> > .
> > .
> > .
> > |----------------------|
> > | map entry[N] |
> > |----------------------|
> > | id value |
> > | output_reference----------------> eg SMMU 1
> > |----------------------|
> >
> >Consequently the iort_node_get_id() function always returns the IORT
> >node pointed at by the output_reference value of the first named
> >component mapping array entry, irrespective of the index parameter,
> >which is a bug.
> >
> >Update the map array entry pointer computation in iort_node_get_id() to
> >take into account the index value, fixing the issue.
> >
> >Fixes: 618f535a6062 ("ACPI/IORT: Add single mapping function")
> >Reported-by: Hanjun Guo <hanjun.guo@linaro.org>
> >Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> >Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >Cc: Hanjun Guo <hanjun.guo@linaro.org>
> >Cc: Sinan Kaya <okaya@codeaurora.org>
> >Cc: Tomasz Nowicki <tn@semihalf.com>
> >Cc: Nate Watterson <nwatters@codeaurora.org>
> >Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >---
> >v1 -> v2:
> > - Updated/improved commit log
> > - Added review tags
>
> Forgot to mention that I tested this patch on Hisilicon
> D03,
>
> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
>
> BTW, Lorenzo, Rafael, since it's a bugfix, can this be merged into
> 4.10-rc7?
Yes, I thought Rafael picked it up already (I can't see it in
patchwork any longer).
This one from Dan:
https://patchwork.kernel.org/patch/9521003/
should get in -rc7 too, we should have both fixes in before v4.10
is released.
Rafael can you send them both for -rc7 please ?
Thanks,
Lorenzo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web