Download raw body.
acpidmar(4): Store PCI domain in softc for segment mapping
> Date: Mon, 10 Aug 2026 16:13:54 +0200
> From: hshoexer <hshoexer@yerbouti.franken.de>
>
> On Wed, Jul 29, 2026 at 11:12:52PM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2026 15:39:46 +0200
> > > From: hshoexer <hshoexer@yerbouti.franken.de>
> > >
> > > Hi,
> > >
> > > with this change I store the PCI domain in the softc. Later I use
> > > that value when mapping domain to segment. Instead of counting.
> >
> > The way acpipci(4) works we already have a 1:1 mapping between PCI
> > segments and PCI domains. Technically that isn't necessarily correct,
> > but in practice that seems to be the only workable approach.
> >
> > > Also fall back to segment 0, when ACPI did not provide a segment.
> >
> > If APCI doesn't provide a segment, we already set it to 0. This is
> > required by the standard. So...
> >
> > > This actually enforces use of the IOMMU.
> > >
> > > While there:
> > > - tweak printf to use segment instead of PCI domain
> > > - use ACPI provided segment instead of hardcoded 0 in ivhd_showpage()
> > >
> > > ok?
> > >
> > > Take care,
> > > HJ.
> > >
> > > -----------------------------------------------------------------------------
> > > diff --git a/sys/arch/amd64/pci/acpipci.c b/sys/arch/amd64/pci/acpipci.c
> > > index e3121987654..e5e861662d7 100644
> > > --- a/sys/arch/amd64/pci/acpipci.c
> > > +++ b/sys/arch/amd64/pci/acpipci.c
> > > @@ -66,6 +66,7 @@ struct acpipci_softc {
> > > char sc_memex_name[32];
> > > int sc_bus;
> > > uint32_t sc_seg;
> > > + int sc_domain;
> > > };
> > >
> > > int acpipci_match(struct device *, void *, void *);
> > > @@ -97,15 +98,14 @@ int
> > > acpipci_domain_to_seg(int domain)
> > > {
> > > struct acpipci_softc *sc;
> > > - int i, d = 0;
> > > + int i;
> > >
> > > for (i = 0; i < acpipci_cd.cd_ndevs; i++) {
> > > sc = (struct acpipci_softc *)acpipci_cd.cd_devs[i];
> > > if (sc == NULL)
> > > continue;
> > > - if (d == domain)
> > > + if (sc->sc_domain == domain)
> > > return sc->sc_seg;
> > > - d++;
> > > }
> > >
> > > return -1;
> > > @@ -147,6 +147,9 @@ acpipci_attach(struct device *parent, struct device *self, void *aux)
> > > aml_evalinteger(sc->sc_acpi, sc->sc_node, "_SEG", 0, NULL, &seg);
> > > sc->sc_seg = seg;
> > >
> > > + /* Assigned when the PCI bus attaches. */
> > > + sc->sc_domain = -1;
> > > +
> > > if (aml_evalname(sc->sc_acpi, sc->sc_node, "_CRS", 0, NULL, &res)) {
> > > printf(": can't find resources\n");
> > >
> > > @@ -210,6 +213,7 @@ acpipci_attach_bus(struct device *parent, struct acpipci_softc *sc)
> > > pba.pba_pmemex = sc->sc_memex;
> > > pba.pba_domain = pci_ndomains++;
> > > pba.pba_bus = sc->sc_bus;
> > > + sc->sc_domain = pba.pba_domain;
> > >
> > > /* Enable MSI in ACPI 2.0 and above, unless we're told not to. */
> > > if (sc->sc_acpi->sc_fadt->hdr.revision >= 2 &&
> > > diff --git a/sys/dev/acpi/acpidmar.c b/sys/dev/acpi/acpidmar.c
> > > index 57b90ddb193..cf18265fd09 100644
> > > --- a/sys/dev/acpi/acpidmar.c
> > > +++ b/sys/dev/acpi/acpidmar.c
> > > @@ -2581,9 +2581,8 @@ acpidmar_pci_hook(pci_chipset_tag_t pc, struct pci_attach_args *pa)
> > >
> > > segment = acpipci_domain_to_seg(pa->pa_domain);
> > > if (segment < 0) {
> > > - DPRINTF(1, "acpidmar: no ACPI segment for pci domain %d\n",
> > > - pa->pa_domain);
> > > - return;
> > > + /* Fall back to segment 0. */
> > > + segment = 0;
> > > }
> >
> > So it we ever get a -1 here, somethings is really wrong. So I'd
> > probably turn this into:
> >
> > KASSERT(segment >= 0);
>
> good point! Uupdated diff below.
>
> ok?
ok kettenis@
> > Otherwise this looks good.
> >
> > I'm still curious to see any x86 AML that actually uses non-zero _SEG
> > numbers.
> >
> > > /* Record PCI-PCI bridge forwarding windows */
> > > @@ -2608,7 +2607,7 @@ acpidmar_pci_hook(pci_chipset_tag_t pc, struct pci_attach_args *pa)
> > > PCI_SUBCLASS(reg) == PCI_SUBCLASS_BRIDGE_ISA) {
> > > /* For ISA Bridges, map 0-16Mb as 1:1 */
> > > printf("dmar: %.4x:%.2x:%.2x.%x mapping ISA\n",
> > > - pa->pa_domain, bus, dev, fun);
> > > + segment, bus, dev, fun);
> > > domain_map_pthru(dom, 0x00, 16*1024*1024);
> > >
> > > /* Keep the identity mapped IOVA range out of the allocator */
> > > @@ -2917,7 +2916,7 @@ ivhd_showpage(struct iommu_softc *iommu, int sid, paddr_t paddr)
> > > if (show > 10)
> > > return;
> > > show++;
> > > - dom = acpidmar_pci_attach(acpidmar_sc, 0, sid, 0);
> > > + dom = acpidmar_pci_attach(acpidmar_sc, iommu->segment, sid, 0);
> > > if (!dom)
> > > return;
> > > printf("DTE: %.8x %.8x %.8x %.8x %.8x %.8x %.8x %.8x\n",
> > >
> > >
> ------------
> acpidmar(4): Store PCI domain in softc for segment mapping
>
> Ensure we get a valid segement and enforce use of the IOMMU.
>
> While there:
> - tweak printf to use segment instead of PCI domain
> - use ACPI provided segment instead of hardcoded 0 in ivhd_showpage()
> ---
> sys/arch/amd64/pci/acpipci.c | 10 +++++++---
> sys/dev/acpi/acpidmar.c | 10 +++-------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/sys/arch/amd64/pci/acpipci.c b/sys/arch/amd64/pci/acpipci.c
> index e3121987654..e5e861662d7 100644
> --- a/sys/arch/amd64/pci/acpipci.c
> +++ b/sys/arch/amd64/pci/acpipci.c
> @@ -66,6 +66,7 @@ struct acpipci_softc {
> char sc_memex_name[32];
> int sc_bus;
> uint32_t sc_seg;
> + int sc_domain;
> };
>
> int acpipci_match(struct device *, void *, void *);
> @@ -97,15 +98,14 @@ int
> acpipci_domain_to_seg(int domain)
> {
> struct acpipci_softc *sc;
> - int i, d = 0;
> + int i;
>
> for (i = 0; i < acpipci_cd.cd_ndevs; i++) {
> sc = (struct acpipci_softc *)acpipci_cd.cd_devs[i];
> if (sc == NULL)
> continue;
> - if (d == domain)
> + if (sc->sc_domain == domain)
> return sc->sc_seg;
> - d++;
> }
>
> return -1;
> @@ -147,6 +147,9 @@ acpipci_attach(struct device *parent, struct device *self, void *aux)
> aml_evalinteger(sc->sc_acpi, sc->sc_node, "_SEG", 0, NULL, &seg);
> sc->sc_seg = seg;
>
> + /* Assigned when the PCI bus attaches. */
> + sc->sc_domain = -1;
> +
> if (aml_evalname(sc->sc_acpi, sc->sc_node, "_CRS", 0, NULL, &res)) {
> printf(": can't find resources\n");
>
> @@ -210,6 +213,7 @@ acpipci_attach_bus(struct device *parent, struct acpipci_softc *sc)
> pba.pba_pmemex = sc->sc_memex;
> pba.pba_domain = pci_ndomains++;
> pba.pba_bus = sc->sc_bus;
> + sc->sc_domain = pba.pba_domain;
>
> /* Enable MSI in ACPI 2.0 and above, unless we're told not to. */
> if (sc->sc_acpi->sc_fadt->hdr.revision >= 2 &&
> diff --git a/sys/dev/acpi/acpidmar.c b/sys/dev/acpi/acpidmar.c
> index 5771a54ffc9..57581f0e48a 100644
> --- a/sys/dev/acpi/acpidmar.c
> +++ b/sys/dev/acpi/acpidmar.c
> @@ -2556,11 +2556,7 @@ acpidmar_pci_hook(pci_chipset_tag_t pc, struct pci_attach_args *pa)
> reg = pci_conf_read(pc, pa->pa_tag, PCI_CLASS_REG);
>
> segment = acpipci_domain_to_seg(pa->pa_domain);
> - if (segment < 0) {
> - DPRINTF(1, "acpidmar: no ACPI segment for pci domain %d\n",
> - pa->pa_domain);
> - return;
> - }
> + KASSERT(segment >= 0);
>
> /* Record PCI-PCI bridge forwarding windows */
> bhlc = pci_conf_read(pc, pa->pa_tag, PCI_BHLC_REG);
> @@ -2584,7 +2580,7 @@ acpidmar_pci_hook(pci_chipset_tag_t pc, struct pci_attach_args *pa)
> PCI_SUBCLASS(reg) == PCI_SUBCLASS_BRIDGE_ISA) {
> /* For ISA Bridges, map 0-16Mb as 1:1 */
> printf("dmar: %.4x:%.2x:%.2x.%x mapping ISA\n",
> - pa->pa_domain, bus, dev, fun);
> + segment, bus, dev, fun);
> domain_map_pthru(dom, 0x00, 16*1024*1024);
>
> /* Keep the identity mapped IOVA range out of the allocator */
> @@ -2893,7 +2889,7 @@ ivhd_showpage(struct iommu_softc *iommu, int sid, paddr_t paddr)
> if (show > 10)
> return;
> show++;
> - dom = acpidmar_pci_attach(acpidmar_sc, 0, sid, 0);
> + dom = acpidmar_pci_attach(acpidmar_sc, iommu->segment, sid, 0);
> if (!dom)
> return;
> printf("DTE: %.8x %.8x %.8x %.8x %.8x %.8x %.8x %.8x\n",
> --
> 2.55.0
>
>
acpidmar(4): Store PCI domain in softc for segment mapping