| Message ID | 168451347867.3465146.10428399827479313906.stgit@djiang5-mobl3 (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | acpi: numa: Add target support for generic port to HMAT parsing | expand |
On Fri, 19 May 2023 09:24:38 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Add generic port support for the parsing of HMAT system locality sub-table. > The attributes will be added to the third array member of the access > coordinates in order to not mix with the existing memory attributes. It only > provides the system locality attributes from initator to the generic port > targets and is missing the rest of the data to the actual memory device. > > The complete attributes will be updated when a memory device is > attached and the system locality information is calculated end to end. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> Missing --- here As a passing comment, hmat_parse_locality() is awfully deeply nested - maybe worth looking to see if some of the deeply nested stuff can be factored out... That would be a new patch however > > v2: > - Fix commit log runon sentence. (Jonathan) > - Add a check for memory type for skipping other access levels. (Jonathan) > - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan) > --- > drivers/acpi/numa/hmat.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > index e2ab1cce0add..82320c92abed 100644 > --- a/drivers/acpi/numa/hmat.c > +++ b/drivers/acpi/numa/hmat.c > @@ -60,6 +60,7 @@ struct target_cache { > enum { > NODE_ACCESS_CLASS_0 = 0, > NODE_ACCESS_CLASS_1, > + NODE_ACCESS_CLASS_GENPORT_SINK, > NODE_ACCESS_CLASS_MAX, > }; > > @@ -368,6 +369,15 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header, > if (mem_hier == ACPI_HMAT_MEMORY) { > target = find_mem_target(targs[targ]); > if (target && target->processor_pxm == inits[init]) { > + if (*target->device_handle) { > + hmat_update_target_access(target, type, value, > + NODE_ACCESS_CLASS_GENPORT_SINK); > + if ((hmat_loc->flags & > + ACPI_HMAT_MEMORY_HIERARCHY) == > + ACPI_HMAT_MEMORY) > + continue; I'm confused. Isn't this already what was checked for with if (mem_heir == ACPI_HMAT_MEMORY)? > + } > + > hmat_update_target_access(target, type, value, > NODE_ACCESS_CLASS_0); > /* If the node has a CPU, update access 1 */ > >
On 6/1/23 07:38, Jonathan Cameron wrote: > On Fri, 19 May 2023 09:24:38 -0700 > Dave Jiang <dave.jiang@intel.com> wrote: > >> Add generic port support for the parsing of HMAT system locality sub-table. >> The attributes will be added to the third array member of the access >> coordinates in order to not mix with the existing memory attributes. It only >> provides the system locality attributes from initator to the generic port >> targets and is missing the rest of the data to the actual memory device. >> >> The complete attributes will be updated when a memory device is >> attached and the system locality information is calculated end to end. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> > Missing > --- > here > > As a passing comment, hmat_parse_locality() is awfully deeply nested > - maybe worth looking to see if some of the deeply nested stuff can be > factored out... That would be a new patch however I'll take a look > > >> v2: >> - Fix commit log runon sentence. (Jonathan) >> - Add a check for memory type for skipping other access levels. (Jonathan) >> - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan) >> --- >> drivers/acpi/numa/hmat.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c >> index e2ab1cce0add..82320c92abed 100644 >> --- a/drivers/acpi/numa/hmat.c >> +++ b/drivers/acpi/numa/hmat.c >> @@ -60,6 +60,7 @@ struct target_cache { >> enum { >> NODE_ACCESS_CLASS_0 = 0, >> NODE_ACCESS_CLASS_1, >> + NODE_ACCESS_CLASS_GENPORT_SINK, >> NODE_ACCESS_CLASS_MAX, >> }; >> >> @@ -368,6 +369,15 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header, >> if (mem_hier == ACPI_HMAT_MEMORY) { >> target = find_mem_target(targs[targ]); >> if (target && target->processor_pxm == inits[init]) { >> + if (*target->device_handle) { >> + hmat_update_target_access(target, type, value, >> + NODE_ACCESS_CLASS_GENPORT_SINK); >> + if ((hmat_loc->flags & >> + ACPI_HMAT_MEMORY_HIERARCHY) == >> + ACPI_HMAT_MEMORY) >> + continue; > I'm confused. Isn't this already what was checked for with > if (mem_heir == ACPI_HMAT_MEMORY)? Yes. Do gen target show up as not memory? I couldn't tell from the ACPI spec. I wonder if I need to move the setup outside of the (mem_hier == ACPI_HMAT_MEMORY)? >> + } >> + >> hmat_update_target_access(target, type, value, >> NODE_ACCESS_CLASS_0); >> /* If the node has a CPU, update access 1 */ >> >>
On Thu, 1 Jun 2023 09:04:34 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > On 6/1/23 07:38, Jonathan Cameron wrote: > > On Fri, 19 May 2023 09:24:38 -0700 > > Dave Jiang <dave.jiang@intel.com> wrote: > > > >> Add generic port support for the parsing of HMAT system locality sub-table. > >> The attributes will be added to the third array member of the access > >> coordinates in order to not mix with the existing memory attributes. It only > >> provides the system locality attributes from initator to the generic port > >> targets and is missing the rest of the data to the actual memory device. > >> > >> The complete attributes will be updated when a memory device is > >> attached and the system locality information is calculated end to end. > >> > >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > Missing > > --- > > here > > > > As a passing comment, hmat_parse_locality() is awfully deeply nested > > - maybe worth looking to see if some of the deeply nested stuff can be > > factored out... That would be a new patch however > > > I'll take a look > > > > > > >> v2: > >> - Fix commit log runon sentence. (Jonathan) > >> - Add a check for memory type for skipping other access levels. (Jonathan) > >> - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan) > >> --- > >> drivers/acpi/numa/hmat.c | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > >> index e2ab1cce0add..82320c92abed 100644 > >> --- a/drivers/acpi/numa/hmat.c > >> +++ b/drivers/acpi/numa/hmat.c > >> @@ -60,6 +60,7 @@ struct target_cache { > >> enum { > >> NODE_ACCESS_CLASS_0 = 0, > >> NODE_ACCESS_CLASS_1, > >> + NODE_ACCESS_CLASS_GENPORT_SINK, > >> NODE_ACCESS_CLASS_MAX, > >> }; > >> > >> @@ -368,6 +369,15 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header, > >> if (mem_hier == ACPI_HMAT_MEMORY) { > >> target = find_mem_target(targs[targ]); > >> if (target && target->processor_pxm == inits[init]) { > >> + if (*target->device_handle) { > >> + hmat_update_target_access(target, type, value, > >> + NODE_ACCESS_CLASS_GENPORT_SINK); > >> + if ((hmat_loc->flags & > >> + ACPI_HMAT_MEMORY_HIERARCHY) == > >> + ACPI_HMAT_MEMORY) > >> + continue; > > I'm confused. Isn't this already what was checked for with > > if (mem_heir == ACPI_HMAT_MEMORY)? > > Yes. Do gen target show up as not memory? I couldn't tell from the ACPI > spec. I wonder if I need to move the setup outside of the (mem_hier == > ACPI_HMAT_MEMORY)? I think it would be almost meaningless to target non memory. Ultimately I guess someone might do cache stashing or similar and care about targeting a CPU in a memoryless node? Let's ignore that however. > > > >> + } > >> + > >> hmat_update_target_access(target, type, value, > >> NODE_ACCESS_CLASS_0); > >> /* If the node has a CPU, update access 1 */ > >> > >>
diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c index e2ab1cce0add..82320c92abed 100644 --- a/drivers/acpi/numa/hmat.c +++ b/drivers/acpi/numa/hmat.c @@ -60,6 +60,7 @@ struct target_cache { enum { NODE_ACCESS_CLASS_0 = 0, NODE_ACCESS_CLASS_1, + NODE_ACCESS_CLASS_GENPORT_SINK, NODE_ACCESS_CLASS_MAX, }; @@ -368,6 +369,15 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header, if (mem_hier == ACPI_HMAT_MEMORY) { target = find_mem_target(targs[targ]); if (target && target->processor_pxm == inits[init]) { + if (*target->device_handle) { + hmat_update_target_access(target, type, value, + NODE_ACCESS_CLASS_GENPORT_SINK); + if ((hmat_loc->flags & + ACPI_HMAT_MEMORY_HIERARCHY) == + ACPI_HMAT_MEMORY) + continue; + } + hmat_update_target_access(target, type, value, NODE_ACCESS_CLASS_0); /* If the node has a CPU, update access 1 */
Add generic port support for the parsing of HMAT system locality sub-table. The attributes will be added to the third array member of the access coordinates in order to not mix with the existing memory attributes. It only provides the system locality attributes from initator to the generic port targets and is missing the rest of the data to the actual memory device. The complete attributes will be updated when a memory device is attached and the system locality information is calculated end to end. Signed-off-by: Dave Jiang <dave.jiang@intel.com> v2: - Fix commit log runon sentence. (Jonathan) - Add a check for memory type for skipping other access levels. (Jonathan) - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan) --- drivers/acpi/numa/hmat.c | 10 ++++++++++ 1 file changed, 10 insertions(+)