Skip to content

Commit

Permalink
acpi/pci/vmd: Fix a nit with nested resource mapping requests
Browse files Browse the repository at this point in the history
Some bus drivers use rmans to suballocate resources to child devices.
When the driver for a child device requests a mapping for a
suballocated resource, the bus driver translates this into a mapping
request for a suitable subrange of the original resource the bus
driver allocated from its parent.  This nested mapping request should
look like any other resource mapping request being made by the bus
device (i.e. as if the bus device had called bus_map_resource() or
bus_alloc_resource() with RF_ACTIVE).

I had slightly flubbed this last bit though since the direct use of
bus_generic_map/unmap_resource passed up the original child device
(second argument to the underlying kobj interface).  While this is
currently harmless, it is not strictly correct as the resource being
mapped is owned by the bus device, not the child and can break for
other bus drivers in the future.

Instead, use bus_map/unmap_resource for the nested request where the
requesting device is now the bus device that owns the parent resource.

Reviewed by:	imp
Fixes:		0e1246e acpi: Cleanup handling of suballocated resources
Fixes:		b377ff8 pcib: Refine handling of resources allocated from bridge windows
Fixes:		d79b6b8 pci_host_generic: Don't rewrite resource start address for translation
Fixes:		d714e73 vmd: Use bus_generic_rman_* for PCI bus and memory resources
Differential Revision:	https://reviews.freebsd.org/D45433
  • Loading branch information
bsdjhb committed Jun 4, 2024
1 parent 65a3312 commit 9805612
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 26 deletions.
17 changes: 10 additions & 7 deletions sys/dev/acpica/acpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1651,19 +1651,22 @@ acpi_map_resource(device_t bus, device_t child, struct resource *r,

args.offset = start - rman_get_start(sysres);
args.length = length;
return (bus_generic_map_resource(bus, child, sysres, &args, map));
return (bus_map_resource(bus, sysres, &args, map));
}

static int
acpi_unmap_resource(device_t bus, device_t child, struct resource *r,
struct resource_map *map)
{
if (acpi_is_resource_managed(bus, r)) {
r = acpi_managed_resource(bus, r);
if (r == NULL)
return (ENOENT);
}
return (bus_generic_unmap_resource(bus, child, r, map));
struct resource *sysres;

if (!acpi_is_resource_managed(bus, r))
return (bus_generic_unmap_resource(bus, child, r, map));

sysres = acpi_managed_resource(bus, r);
if (sysres == NULL)
return (ENOENT);
return (bus_unmap_resource(bus, sysres, map));
}

/* Allocate an IO port or memory resource, given its GAS. */
Expand Down
16 changes: 8 additions & 8 deletions sys/dev/pci/pci_host_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ generic_pcie_map_resource(device_t dev, device_t child, struct resource *r,

args.offset = start - range->pci_base;
args.length = length;
return (bus_generic_map_resource(dev, child, range->res, &args, map));
return (bus_map_resource(dev, range->res, &args, map));
}

static int
Expand All @@ -664,16 +664,16 @@ generic_pcie_unmap_resource(device_t dev, device_t child, struct resource *r,
#endif
case SYS_RES_IOPORT:
case SYS_RES_MEMORY:
range = generic_pcie_containing_range(dev, type,
rman_get_start(r), rman_get_end(r));
if (range == NULL || range->res == NULL)
return (ENOENT);
r = range->res;
break;
default:
break;
return (bus_generic_unmap_resource(dev, child, r, argsp, map));
}
return (bus_generic_unmap_resource(dev, child, r, map));

range = generic_pcie_containing_range(dev, type, rman_get_start(r),
rman_get_end(r));
if (range == NULL || range->res == NULL)
return (ENOENT);
return (bus_unmap_resource(dev, range->res, map));
}

static bus_dma_tag_t
Expand Down
16 changes: 9 additions & 7 deletions sys/dev/pci/pci_pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -2549,7 +2549,7 @@ pcib_map_resource(device_t dev, device_t child, struct resource *r,

args.offset = start - rman_get_start(pres);
args.length = length;
return (bus_generic_map_resource(dev, child, pres, &args, map));
return (bus_map_resource(dev, pres, &args, map));
}

static int
Expand All @@ -2558,14 +2558,16 @@ pcib_unmap_resource(device_t dev, device_t child, struct resource *r,
{
struct pcib_softc *sc = device_get_softc(dev);
struct pcib_window *w;
struct resource *pres;

w = pcib_get_resource_window(sc, r);
if (w != NULL) {
r = pcib_find_parent_resource(w, r);
if (r == NULL)
return (ENOENT);
}
return (bus_generic_unmap_resource(dev, child, r, map));
if (w == NULL)
return (bus_generic_unmap_resource(dev, child, r, map));

pres = pcib_find_parent_resource(w, r);
if (pres == NULL)
return (ENOENT);
return (bus_unmap_resource(dev, pres, map));
}
#else
/*
Expand Down
9 changes: 5 additions & 4 deletions sys/dev/vmd/vmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -543,19 +543,20 @@ vmd_map_resource(device_t dev, device_t child, struct resource *r,

args.offset = start - rman_get_start(pres);
args.length = length;
return (bus_generic_map_resource(dev, child, pres, &args, map));
return (bus_map_resource(dev, pres, &args, map));
}

static int
vmd_unmap_resource(device_t dev, device_t child, struct resource *r,
struct resource_map *map)
{
struct vmd_softc *sc = device_get_softc(dev);
struct resource *pres;

r = vmd_find_parent_resource(sc, r);
if (r == NULL)
pres = vmd_find_parent_resource(sc, r);
if (pres == NULL)
return (ENOENT);
return (bus_generic_unmap_resource(dev, child, r, map));
return (bus_unmap_resource(dev, pres, map));
}

static int
Expand Down

0 comments on commit 9805612

Please sign in to comment.