acpi bus: store children domain in ivar
ClosedPublic
Actions

Authored by kib on Oct 26 2024, 10:30 AM.

Details

Reviewers

jhb
markj

Commits

rG3f0289ea7f66: dmar: set acpi ivar domain on identify()
rG7dd1f0dcd134: acpica: add domain ivar
rG6f423295f18a: acpica: rename acpi_parse_pxm() to acpi_pxm_parse() and make it non-static

Summary

This should allow to properly report domain for DMARs, and also make some more allocations (like sc) to follow affinity.

sysctl dev.: dynamically calculate device' domain

dev/acpica: rename acpi_get_domain() to acpi_get_domain_method()

 acpica: add domain ivar    
 Specialize acpi bus_get_domain method to read ivar, and fall back to
 _PXM if ivar is not set.  If _PXM was used, its result is cached in
 ivar.

 dmar: set acpi ivar domain on identify()

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Oct 26 2024, 10:30 AM

Herald added subscribers: olce, imp. · View Herald TranscriptOct 26 2024, 10:30 AM

kib requested review of this revision.Oct 26 2024, 10:30 AM

emaste added a subscriber: emaste.Oct 27 2024, 4:59 PM

I would suggest parsing _PXM up front when we enumerate devices in the namespace and just set the ivar then.

sys/dev/acpica/acpi_pci.c
111	Hmm, so you are trying to avoid changing this driver to also cache _PXM I guess.
sys/kern/subr_bus.c
269	I don't understand the need for the changes in this file. bus_get_domain() should still work the first time it is called and the value will never change?

kib added inline comments.Oct 28 2024, 2:36 PM

sys/dev/acpica/acpi_pci.c
111	Where would I cache the _PXM value? It is arbitrary pci bus child that ends up calling this method, so I cannot know where to save my bit of data.
sys/kern/subr_bus.c
269	I want to be able to set the domain ivar later then identify. For that, bus_get_domain() might fail during the sysctl_init() but work after e.g. attach. These changes are separate, I can omit them for now if objected.

Call _PXM for acpi children at enumeration time.

jhb added inline comments.Oct 30 2024, 1:41 PM

sys/dev/acpica/acpi.c
1318	I would maybe now move this function to acpi_pci.c and call it `acpi_pci_get_domain` (it would be static then as well) and then the new function added below can just be `acpi_get_domain_method`?
sys/kern/subr_bus.c
269	Hmm, the current devices here set it during identify, so I guess you anticipate needing this for a future driver? It does seem a bit odd for a child device to set this during attach since it's a bus method, that is, the parent bus is responsible for determining the domain and should in theory know it before a child device attaches?

kib marked an inline comment as done.Oct 30 2024, 4:25 PM

kib added inline comments.

sys/kern/subr_bus.c
269	I do not quite understand newbus organization. For me, it is strange that the bus needs to know what is the device' affinity. Esp. for pseudo-bus like ACPI, where devices come from enumerating device namespace, but also from random ACPI tables like DMAR or HPET. Another example of the ACPI child which uses special way to define affinity is NVDIMM, where NFIT table provides the domain number for given physical address range backed by NVDIMM (I believe it is done this way to support also interleave configs). We definitely do not want to teach the acpi0 bus to parse all that tables. Either we introduce the intermediate bus to report affinity, or we need to allow the children to do it.

Use acpi_pci_get_domain() private method.
Rename acpi_parse_pxm() to acpi_pxm_parse() for consistency with other external functions.

Ping?

I would defer the subr_bus.c change until we have a use case that needs it.

sys/kern/subr_bus.c
269	Normally it would be the responsibility of the device_identify() routine that adds the new child device to initialize ivars for a device (e.g. set the ACPI handle if there is one). device_identify() is kind of a special method that should initialize a device_t in all the ways that the bus would initialize devices the bus knows how to enumerate, and specifically setting ivars. So that means that various ACPI device drivers that enumerate devices via other ACPI tables are responsible for initializing the ivars for any devices they add.