| Message ID | 20240927142108.1156362-5-dave.jiang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | acpi/hmat / cxl: Add exclusive caching enumeration and RAS support | expand |
On Fri, 27 Sep 2024 07:16:56 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Add helper functions to help do address translation for either the address > of the extended linear cache or its alias address. The translation function > attempt to detect an I/O hole in the proximity domain and adjusts the address > if the hole impacts the aliasing of the address. The range of the I/O hole > is retrieved by walking through the associated memory target resources. What does the I/O hole correspond to in the system? > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/acpi/numa/hmat.c | 136 +++++++++++++++++++++++++++++++++++++++ > include/linux/acpi.h | 14 ++++ > 2 files changed, 150 insertions(+) > > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > index d299f8d7af8c..834314582f4c 100644 > --- a/drivers/acpi/numa/hmat.c > +++ b/drivers/acpi/numa/hmat.c > @@ -152,6 +152,142 @@ int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, > } > EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); > > +static int alias_address_find_iohole(struct memory_target *target, > + u64 address, u64 alias, struct range *hole) > +{ > + struct resource *alias_res = NULL; > + struct resource *res, *prev; > + > + *hole = (struct range) { > + .start = 0, > + .end = -1, > + }; > + > + /* First find the resource that the address is in */ > + prev = target->memregions.child; > + for (res = target->memregions.child; res; res = res->sibling) { > + if (alias >= res->start && alias <= res->end) { > + alias_res = res; > + break; > + } > + prev = res; > + } > + if (!alias_res) if (!res) and you can just use res instead of alias_res for the following as you exit the loop with it set to the right value. > + return -EINVAL; > + > + /* No memory hole */ > + if (alias_res == prev) > + return 0; > + > + /* If address is within the current resource, no need to deal with memory hole */ > + if (address >= alias_res->start) > + return 0; > + > + *hole = (struct range) { > + .start = prev->end + 1, > + .end = alias_res->start - 1, > + }; Ordering assumption should be avoided in such a generic sounding function. Can the hole be first? or rename the function to include preceding_hole or something like that. > + > + return 0; > +} > + > +int hmat_extended_linear_cache_alias_xlat(u64 address, u64 *alias, int nid) > +{ > + unsigned int pxm = node_to_pxm(nid); > + struct memory_target *target; > + struct range iohole; > + int rc; > + > + target = find_mem_target(pxm); > + if (!target) > + return -EINVAL; > + > + rc = alias_address_find_iohole(target, address, *alias, &iohole); > + if (rc) > + return rc; > + > + if (!range_len(&iohole)) > + return 0; > + Maybe reformat like this and add comments on each condition. if (address >= iohole.start) return 0; if (*alias <= iohole.start) return 0; *alias += range_len(&iohole); return 0; > + if (address < iohole.start) { > + if (*alias > iohole.start) { > + *alias = *alias + range_len(&iohole); > + return 0; > + } > + } > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_alias_xlat, CXL); > + > +static int target_address_find_iohole(struct memory_target *target, > + u64 address, u64 alias, > + struct range *hole) > +{ > + struct resource *addr_res = NULL; > + struct resource *res, *next; > + > + *hole = (struct range) { > + .start = 0, > + .end = -1, > + }; > + > + /* First find the resource that the address is in */ > + for (res = target->memregions.child; res; res = res->sibling) { > + if (address >= res->start && address <= res->end) { > + addr_res = res; Could just use res as it's scope is outside the loop. > + break; > + } > + } > + if (!addr_res) > + return -EINVAL; > + > + next = res->sibling; > + /* No memory hole after the region */ > + if (!next) > + return 0; > + > + /* If alias is within the current resource, no need to deal with memory hole */ > + if (alias <= addr_res->end) > + return 0; > + > + *hole = (struct range) { > + .start = addr_res->end + 1, > + .end = next->start - 1, > + }; > + > + return 0; > +} > + > +int hmat_extended_linear_cache_address_xlat(u64 *address, u64 alias, int nid) > +{ > + unsigned int pxm = node_to_pxm(nid); > + struct memory_target *target; > + struct range iohole; > + int rc; > + > + target = find_mem_target(pxm); > + if (!target) > + return -EINVAL; > + > + rc = target_address_find_iohole(target, *address, alias, &iohole); > + if (rc) > + return rc; > + > + if (!range_len(&iohole)) > + return 0; > + Similar to above, maybe break into multiple reasons to exit early. > + if (alias > iohole.end) { > + if (*address < iohole.end) { > + *address = *address - range_len(&iohole); > + return 0; > + } > + } > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_address_xlat, CXL); > Jonathan
> What does the I/O hole correspond to in the system?
PCIe mmio mapped space. 32-bit devices must have addresses below 4G
so X86 systems have a physical memory map that looks like:
0 - 2G: RAM
2G-4G: MMIO
4G-end of memory: RAM
end of memory-infinity: 64-bit MMIO
Depending on how much MMIO there is different systems put the
dividing line at other addresses than 2G.
-Tony
On Thu, 17 Oct 2024 16:46:35 +0000 "Luck, Tony" <tony.luck@intel.com> wrote: > > What does the I/O hole correspond to in the system? > > PCIe mmio mapped space. 32-bit devices must have addresses below 4G > so X86 systems have a physical memory map that looks like: > > 0 - 2G: RAM > 2G-4G: MMIO > 4G-end of memory: RAM > end of memory-infinity: 64-bit MMIO > > Depending on how much MMIO there is different systems put the > dividing line at other addresses than 2G. Ah, thanks. So this weird cache setup might be not quite linear module N aliases as described in the ACPI spec (System vs host physical addresses I guess). Had wrong mental model :( Ouch. > > -Tony >
On 10/17/24 9:33 AM, Jonathan Cameron wrote: > On Fri, 27 Sep 2024 07:16:56 -0700 > Dave Jiang <dave.jiang@intel.com> wrote: > >> Add helper functions to help do address translation for either the address >> of the extended linear cache or its alias address. The translation function >> attempt to detect an I/O hole in the proximity domain and adjusts the address >> if the hole impacts the aliasing of the address. The range of the I/O hole >> is retrieved by walking through the associated memory target resources. > > What does the I/O hole correspond to in the system? > >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/acpi/numa/hmat.c | 136 +++++++++++++++++++++++++++++++++++++++ >> include/linux/acpi.h | 14 ++++ >> 2 files changed, 150 insertions(+) >> >> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c >> index d299f8d7af8c..834314582f4c 100644 >> --- a/drivers/acpi/numa/hmat.c >> +++ b/drivers/acpi/numa/hmat.c >> @@ -152,6 +152,142 @@ int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, >> } >> EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); >> >> +static int alias_address_find_iohole(struct memory_target *target, >> + u64 address, u64 alias, struct range *hole) >> +{ >> + struct resource *alias_res = NULL; >> + struct resource *res, *prev; >> + >> + *hole = (struct range) { >> + .start = 0, >> + .end = -1, >> + }; >> + >> + /* First find the resource that the address is in */ >> + prev = target->memregions.child; >> + for (res = target->memregions.child; res; res = res->sibling) { >> + if (alias >= res->start && alias <= res->end) { >> + alias_res = res; >> + break; >> + } >> + prev = res; >> + } >> + if (!alias_res) > > if (!res) and you can just use res instead of alias_res for the following > as you exit the loop with it set to the right value. > Ok will do that > > >> + return -EINVAL; >> + >> + /* No memory hole */ >> + if (alias_res == prev) >> + return 0; >> + >> + /* If address is within the current resource, no need to deal with memory hole */ >> + if (address >= alias_res->start) >> + return 0; >> + >> + *hole = (struct range) { >> + .start = prev->end + 1, >> + .end = alias_res->start - 1, >> + }; > Ordering assumption should be avoided in such a generic > sounding function. Can the hole be first? Do you mean if the address mapping starts out with an MMIO range and then memory range? I'm not sure if such an implementation exists in the x86 world. And if the hole is behind all the ranges, then it shouldn't impact the address calculations. > > or rename the function to include preceding_hole or something like that. >> + >> + return 0; >> +} >> + >> +int hmat_extended_linear_cache_alias_xlat(u64 address, u64 *alias, int nid) >> +{ >> + unsigned int pxm = node_to_pxm(nid); >> + struct memory_target *target; >> + struct range iohole; >> + int rc; >> + >> + target = find_mem_target(pxm); >> + if (!target) >> + return -EINVAL; >> + >> + rc = alias_address_find_iohole(target, address, *alias, &iohole); >> + if (rc) >> + return rc; >> + >> + if (!range_len(&iohole)) >> + return 0; >> + > Maybe reformat like this and add comments on each condition. > > if (address >= iohole.start) > return 0; > > if (*alias <= iohole.start) > return 0; > > *alias += range_len(&iohole); > > return 0; > > Will change to that and add comments. >> + if (address < iohole.start) { >> + if (*alias > iohole.start) { >> + *alias = *alias + range_len(&iohole); >> + return 0; >> + } >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_alias_xlat, CXL); >> + >> +static int target_address_find_iohole(struct memory_target *target, >> + u64 address, u64 alias, >> + struct range *hole) >> +{ >> + struct resource *addr_res = NULL; >> + struct resource *res, *next; >> + >> + *hole = (struct range) { >> + .start = 0, >> + .end = -1, >> + }; >> + >> + /* First find the resource that the address is in */ >> + for (res = target->memregions.child; res; res = res->sibling) { >> + if (address >= res->start && address <= res->end) { >> + addr_res = res; > > Could just use res as it's scope is outside the loop. Will update. > >> + break; >> + } >> + } >> + if (!addr_res) >> + return -EINVAL; >> + >> + next = res->sibling; >> + /* No memory hole after the region */ >> + if (!next) >> + return 0; >> + >> + /* If alias is within the current resource, no need to deal with memory hole */ >> + if (alias <= addr_res->end) >> + return 0; >> + >> + *hole = (struct range) { >> + .start = addr_res->end + 1, >> + .end = next->start - 1, >> + }; >> + >> + return 0; >> +} >> + >> +int hmat_extended_linear_cache_address_xlat(u64 *address, u64 alias, int nid) >> +{ >> + unsigned int pxm = node_to_pxm(nid); >> + struct memory_target *target; >> + struct range iohole; >> + int rc; >> + >> + target = find_mem_target(pxm); >> + if (!target) >> + return -EINVAL; >> + >> + rc = target_address_find_iohole(target, *address, alias, &iohole); >> + if (rc) >> + return rc; >> + >> + if (!range_len(&iohole)) >> + return 0; >> + > > Similar to above, maybe break into multiple reasons to exit early. > Will update. >> + if (alias > iohole.end) { >> + if (*address < iohole.end) { >> + *address = *address - range_len(&iohole); >> + return 0; >> + } >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_address_xlat, CXL); >> > > Jonathan
On Wed, 30 Oct 2024 15:53:20 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > On 10/17/24 9:33 AM, Jonathan Cameron wrote: > > On Fri, 27 Sep 2024 07:16:56 -0700 > > Dave Jiang <dave.jiang@intel.com> wrote: > > > >> Add helper functions to help do address translation for either the address > >> of the extended linear cache or its alias address. The translation function > >> attempt to detect an I/O hole in the proximity domain and adjusts the address > >> if the hole impacts the aliasing of the address. The range of the I/O hole > >> is retrieved by walking through the associated memory target resources. > > > > What does the I/O hole correspond to in the system? > > > >> > >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> > >> --- > >> drivers/acpi/numa/hmat.c | 136 +++++++++++++++++++++++++++++++++++++++ > >> include/linux/acpi.h | 14 ++++ > >> 2 files changed, 150 insertions(+) > >> > >> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > >> index d299f8d7af8c..834314582f4c 100644 > >> --- a/drivers/acpi/numa/hmat.c > >> +++ b/drivers/acpi/numa/hmat.c > >> @@ -152,6 +152,142 @@ int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, > >> } > >> EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); > >> > >> +static int alias_address_find_iohole(struct memory_target *target, > >> + u64 address, u64 alias, struct range *hole) > >> +{ > >> + struct resource *alias_res = NULL; > >> + struct resource *res, *prev; > >> + > >> + *hole = (struct range) { > >> + .start = 0, > >> + .end = -1, > >> + }; > >> + > >> + /* First find the resource that the address is in */ > >> + prev = target->memregions.child; > >> + for (res = target->memregions.child; res; res = res->sibling) { > >> + if (alias >= res->start && alias <= res->end) { > >> + alias_res = res; > >> + break; > >> + } > >> + prev = res; > >> + } > >> + if (!alias_res) > > > > if (!res) and you can just use res instead of alias_res for the following > > as you exit the loop with it set to the right value. > > > Ok will do that > > > > > > >> + return -EINVAL; > >> + > >> + /* No memory hole */ > >> + if (alias_res == prev) > >> + return 0; > >> + > >> + /* If address is within the current resource, no need to deal with memory hole */ > >> + if (address >= alias_res->start) > >> + return 0; > >> + > >> + *hole = (struct range) { > >> + .start = prev->end + 1, > >> + .end = alias_res->start - 1, > >> + }; > > Ordering assumption should be avoided in such a generic > > sounding function. Can the hole be first? > > Do you mean if the address mapping starts out with an MMIO range and then memory range? I'm not sure if such an implementation exists in the x86 world. And if the hole is behind all the ranges, then it shouldn't impact the address calculations. > That was me not really understanding what the hole was. Tony filled in that gap. > > > > or rename the function to include preceding_hole or something like that. > >> + > >> + return 0; > >> +} >
diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c index d299f8d7af8c..834314582f4c 100644 --- a/drivers/acpi/numa/hmat.c +++ b/drivers/acpi/numa/hmat.c @@ -152,6 +152,142 @@ int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, } EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); +static int alias_address_find_iohole(struct memory_target *target, + u64 address, u64 alias, struct range *hole) +{ + struct resource *alias_res = NULL; + struct resource *res, *prev; + + *hole = (struct range) { + .start = 0, + .end = -1, + }; + + /* First find the resource that the address is in */ + prev = target->memregions.child; + for (res = target->memregions.child; res; res = res->sibling) { + if (alias >= res->start && alias <= res->end) { + alias_res = res; + break; + } + prev = res; + } + if (!alias_res) + return -EINVAL; + + /* No memory hole */ + if (alias_res == prev) + return 0; + + /* If address is within the current resource, no need to deal with memory hole */ + if (address >= alias_res->start) + return 0; + + *hole = (struct range) { + .start = prev->end + 1, + .end = alias_res->start - 1, + }; + + return 0; +} + +int hmat_extended_linear_cache_alias_xlat(u64 address, u64 *alias, int nid) +{ + unsigned int pxm = node_to_pxm(nid); + struct memory_target *target; + struct range iohole; + int rc; + + target = find_mem_target(pxm); + if (!target) + return -EINVAL; + + rc = alias_address_find_iohole(target, address, *alias, &iohole); + if (rc) + return rc; + + if (!range_len(&iohole)) + return 0; + + if (address < iohole.start) { + if (*alias > iohole.start) { + *alias = *alias + range_len(&iohole); + return 0; + } + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_alias_xlat, CXL); + +static int target_address_find_iohole(struct memory_target *target, + u64 address, u64 alias, + struct range *hole) +{ + struct resource *addr_res = NULL; + struct resource *res, *next; + + *hole = (struct range) { + .start = 0, + .end = -1, + }; + + /* First find the resource that the address is in */ + for (res = target->memregions.child; res; res = res->sibling) { + if (address >= res->start && address <= res->end) { + addr_res = res; + break; + } + } + if (!addr_res) + return -EINVAL; + + next = res->sibling; + /* No memory hole after the region */ + if (!next) + return 0; + + /* If alias is within the current resource, no need to deal with memory hole */ + if (alias <= addr_res->end) + return 0; + + *hole = (struct range) { + .start = addr_res->end + 1, + .end = next->start - 1, + }; + + return 0; +} + +int hmat_extended_linear_cache_address_xlat(u64 *address, u64 alias, int nid) +{ + unsigned int pxm = node_to_pxm(nid); + struct memory_target *target; + struct range iohole; + int rc; + + target = find_mem_target(pxm); + if (!target) + return -EINVAL; + + rc = target_address_find_iohole(target, *address, alias, &iohole); + if (rc) + return rc; + + if (!range_len(&iohole)) + return 0; + + if (alias > iohole.end) { + if (*address < iohole.end) { + *address = *address - range_len(&iohole); + return 0; + } + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(hmat_extended_linear_cache_address_xlat, CXL); + static struct memory_target *acpi_find_genport_target(u32 uid) { struct memory_target *target; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 8ed72d431dca..704bdfc79f85 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -437,6 +437,8 @@ int thermal_acpi_critical_trip_temp(struct acpi_device *adev, int *ret_temp); int acpi_get_genport_coordinates(u32 uid, struct access_coordinate *coord); int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, resource_size_t *size); +int hmat_extended_linear_cache_alias_xlat(u64 address, u64 *alias, int nid); +int hmat_extended_linear_cache_address_xlat(u64 *address, u64 alias, int nid); #else static inline int acpi_get_genport_coordinates(u32 uid, struct access_coordinate *coord) @@ -449,6 +451,18 @@ static inline int hmat_get_extended_linear_cache_size(struct resource *backing_r { return -EOPNOTSUPP; } + +static inline int hmat_extended_linear_cache_alias_xlat(u64 address, + u64 *alias, int nid) +{ + return -EOPNOTSUPP; +} + +static inline int hmat_extended_linear_cache_address_xlat(u64 *address, + u64 alias, int nid) +{ + return -EOPNOTSUPP; +} #endif #ifdef CONFIG_ACPI_NUMA
Add helper functions to help do address translation for either the address of the extended linear cache or its alias address. The translation function attempt to detect an I/O hole in the proximity domain and adjusts the address if the hole impacts the aliasing of the address. The range of the I/O hole is retrieved by walking through the associated memory target resources. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/acpi/numa/hmat.c | 136 +++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 14 ++++ 2 files changed, 150 insertions(+)