Re: [PATCH v3 3/6] acpi: numa: Add genport target allocation to the HMAT parsing

[Dave Jiang <dave.jiang@xxxxxxxxx>]

On 6/22/23 06:45, Jonathan Cameron wrote:
> On Thu, 15 Jun 2023 14:24:01 -0700
> Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
>
> > Add SRAT parsing for the HMAT init in order to collect the device handle
> > from the Generic Port Affinity Structure. The device handle will serve as
> > the key to search for target data.
> >
> > Consoliate the common code with alloc_memory_target() in a helper function
> > alloc_target().
> >
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>
> On second thoughts - device_handle is rather cryptic as a name
> and there are no handy comments on what it is.
> Can we call it gen_port_device_handle or something like that?

Sure. I can update and send out a new series.

> > ---
> > v3:
> > - Move ACPI_SRAT_DEVICE_HANDLE_SIZE to separate patch for ACPICA
> > ---
> >   drivers/acpi/numa/hmat.c |   53 +++++++++++++++++++++++++++++++++++++++++++---
> >   1 file changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> > index abed728bf09d..e2ab1cce0add 100644
> > --- a/drivers/acpi/numa/hmat.c
> > +++ b/drivers/acpi/numa/hmat.c
> > @@ -71,6 +71,7 @@ struct memory_target {
> >   	struct access_coordinate coord[NODE_ACCESS_CLASS_MAX];
> >   	struct list_head caches;
> >   	struct node_cache_attrs cache_attrs;
> > +	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
> >   	bool registered;
> >   };
> >   
> > @@ -125,8 +126,7 @@ static __init void alloc_memory_initiator(unsigned int cpu_pxm)
> >   	list_add_tail(&initiator->node, &initiators);
> >   }
> >   
> > -static __init void alloc_memory_target(unsigned int mem_pxm,
> > -		resource_size_t start, resource_size_t len)
> > +static __init struct memory_target *alloc_target(unsigned int mem_pxm)
> >   {
> >   	struct memory_target *target;
> >   
> > @@ -134,7 +134,7 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
> >   	if (!target) {
> >   		target = kzalloc(sizeof(*target), GFP_KERNEL);
> >   		if (!target)
> > -			return;
> > +			return NULL;
> >   		target->memory_pxm = mem_pxm;
> >   		target->processor_pxm = PXM_INVAL;
> >   		target->memregions = (struct resource) {
> > @@ -147,6 +147,19 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
> >   		INIT_LIST_HEAD(&target->caches);
> >   	}
> >   
> > +	return target;
> > +}
> > +
> > +static __init void alloc_memory_target(unsigned int mem_pxm,
> > +				       resource_size_t start,
> > +				       resource_size_t len)
> > +{
> > +	struct memory_target *target;
> > +
> > +	target = alloc_target(mem_pxm);
> > +	if (!target)
> > +		return;
> > +
> >   	/*
> >   	 * There are potentially multiple ranges per PXM, so record each
> >   	 * in the per-target memregions resource tree.
> > @@ -157,6 +170,17 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
> >   				start, start + len, mem_pxm);
> >   }
> >   
> > +static __init void alloc_genport_target(unsigned int mem_pxm, u8 *handle)
> > +{
> > +	struct memory_target *target;
> > +
> > +	target = alloc_target(mem_pxm);
> > +	if (!target)
> > +		return;
> > +
> > +	memcpy(target->device_handle, handle, ACPI_SRAT_DEVICE_HANDLE_SIZE);
> > +}
> > +
> >   static __init const char *hmat_data_type(u8 type)
> >   {
> >   	switch (type) {
> > @@ -498,6 +522,22 @@ static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
> >   	return 0;
> >   }
> >   
> > +static __init int srat_parse_genport_affinity(union acpi_subtable_headers *header,
> > +					      const unsigned long end)
> > +{
> > +	struct acpi_srat_generic_affinity *ga = (void *)header;
> > +
> > +	if (!ga)
> > +		return -EINVAL;
> > +
> > +	if (!(ga->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
> > +		return 0;
> > +
> > +	alloc_genport_target(ga->proximity_domain, (u8 *)ga->device_handle);
> > +
> > +	return 0;
> > +}
> > +
> >   static u32 hmat_initiator_perf(struct memory_target *target,
> >   			       struct memory_initiator *initiator,
> >   			       struct acpi_hmat_locality *hmat_loc)
> > @@ -848,6 +888,13 @@ static __init int hmat_init(void)
> >   				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
> >   				srat_parse_mem_affinity, 0) < 0)
> >   		goto out_put;
> > +
> > +	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
> > +				     sizeof(struct acpi_table_srat),
> > +				     ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY,
> > +				     srat_parse_genport_affinity, 0) < 0)
> > +		goto out_put;
> > +
> >   	acpi_put_table(tbl);
> >   
> >   	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);