> -----Original Message-----
> From: Tushar Dave <[email protected]>
> Sent: 16 September 2026 19:56
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Alex Williamson
> <[email protected]>; Jason Gunthorpe <[email protected]>; Shameer
> Kolothum Thodi <[email protected]>
> Subject: [PATCH 2/5] pci: add validation for fixed BAR configuration
>
> Validate the fixed BAR configuration specified through the
> fixed-bar-N properties on PCI endpoint devices.
>
> Configuring any fixed-bar-N property on an endpoint device requires
> every memory BAR on that device to have a fixed address. Validate
> that each assigned address:
>
> - is aligned to its BAR size;
> - is within the appropriate PCIe MMIO aperture; and
> - does not overlap any other fixed BAR.
>
> Under a PCIe topology with a root port, fixed BAR placement is
> all-or-nothing within that root port's subtree. If any one endpoint
> device below the root port has a fixed BAR, every other endpoint
> device below that same root port with a memory BAR must have all of
> its memory BARs fixed too.
>
> Endpoint devices with no root port in their ancestry, such as those
> attached directly to the host bridge, are not required to match
> their sibling devices. One such device may use fixed-bar-N for all
> of its memory BARs while another sibling device on the same host
> bridge uses none at all. Per-device completeness still applies
> regardless of ancestry. Once any fixed-bar-N property is set on a
> device, every memory BAR on that device must be fixed.
>
> Abort QEMU if any validation fails.
>
> Signed-off-by: Tushar Dave <[email protected]>
> ---
> hw/pci/meson.build | 1 +
> hw/pci/pci-fixed-bar-validate.c | 326
> ++++++++++++++++++++++++++++++++
> hw/pci/pci-fixed-bar-validate.h | 21 ++
> 3 files changed, 348 insertions(+)
> create mode 100644 hw/pci/pci-fixed-bar-validate.c
> create mode 100644 hw/pci/pci-fixed-bar-validate.h
>
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> index 984c7327e5..150085c997 100644
> --- a/hw/pci/meson.build
> +++ b/hw/pci/meson.build
> @@ -19,6 +19,7 @@ endif
> pci_ss.add(files('pcie.c', 'pcie_aer.c'))
> pci_ss.add(files('pcie_doe.c'))
> system_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pcie_port.c',
> 'pcie_host.c'))
> +system_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pci-fixed-bar-
> validate.c'))
> system_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>
> stub_ss.add(files('pci-stub.c'))
> diff --git a/hw/pci/pci-fixed-bar-validate.c b/hw/pci/pci-fixed-bar-validate.c
> new file mode 100644
> index 0000000000..0b5baad8a0
> --- /dev/null
> +++ b/hw/pci/pci-fixed-bar-validate.c
> @@ -0,0 +1,326 @@
> +/*
> + * Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights
> reserved
> + * BAR address validation for fixed-BAR placement.
> + *
> + * Written by Tushar Dave
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_port.h"
> +#include "qemu/error-report.h"
> +#include "qemu/range.h"
> +#include "qemu/units.h"
> +#include "pci-internal.h"
> +#include "pci-fixed-bar-validate.h"
> +
> +/*
> + * Claimed BAR ranges — detect inter-device and inter-hierarchy overlaps
> + * across all fixed BARs system-wide.
> + */
> +
> +typedef struct {
> + uint64_t start;
> + uint64_t end;
> + const char *owner; /* device name, for error messages */
> + int bar;
> +} FixedClaim;
> +
> +static GArray *fixed_claims;
Can this live in the FixedBarsInfo instead? IIUC, the info context is
already passed through the walk, and the claims only need to exist
for that validation pass?
> +
> +static void fixed_claims_init(void)
> +{
> + if (fixed_claims) {
> + g_array_free(fixed_claims, true);
> + }
> + fixed_claims = g_array_new(false, true, sizeof(FixedClaim));
> +}
> +
> +static void fixed_claims_free(void)
> +{
> + g_array_free(fixed_claims, true);
> + fixed_claims = NULL;
> +}
> +
> +static bool fixed_claims_overlap(uint64_t start, uint64_t end,
> + const char **owner_out, int *bar_out,
> + uint64_t *start_out, uint64_t *end_out)
> +{
> + FixedClaim *c;
> + guint i;
> +
> + for (i = 0; i < fixed_claims->len; i++) {
> + c = &g_array_index(fixed_claims, FixedClaim, i);
> + if (ranges_overlap(start, end - start + 1,
> + c->start, c->end - c->start + 1)) {
> + *owner_out = c->owner;
> + *bar_out = c->bar;
> + *start_out = c->start;
> + *end_out = c->end;
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static void fixed_claims_add(uint64_t start, uint64_t end,
> + const char *owner, int bar)
> +{
> + FixedClaim cl;
> +
> + cl.start = start;
> + cl.end = end;
> + cl.owner = owner;
> + cl.bar = bar;
> + g_array_append_val(fixed_claims, cl);
> +}
> +
> +static bool validate_bars(PCIDevice *pdev, FixedBarsInfo *info,
> + bool is_fixed_subtree)
> +{
> + const char *devname = DEVICE(pdev)->id ? DEVICE(pdev)->id : pdev-
> >name;
> + const char *overlap_owner;
> + const char *wname;
> + bool has_fixed_bars = false;
> + bool is_64bit;
> + uint64_t addr, end, wbase, wlim, overlap_start, overlap_end;
> + PCIIORegion *r;
> + int overlap_bar;
> + int i;
> +
> + for (i = 0; i < PCI_NUM_REGIONS - 1; i++) {
> + if (pdev->fixed_bar_addr[i] != PCI_BAR_UNMAPPED) {
> + has_fixed_bars = true;
> + break;
> + }
> + }
> +
> + if (is_fixed_subtree && !has_fixed_bars) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] under fixed-bar root "
> + "port has memory BARs but no fixed-bar-N= specified",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> + exit(1);
> + }
> +
> + if (!has_fixed_bars) {
> + return false;
> + }
> +
> + /* Completeness: every memory BAR must have an address. */
> + for (i = 0; i < PCI_NUM_REGIONS - 1; i++) {
> + r = &pdev->io_regions[i];
> + if (!r->size || (r->type & PCI_BASE_ADDRESS_SPACE_IO)) {
> + continue;
> + }
> + if (pdev->fixed_bar_addr[i] == PCI_BAR_UNMAPPED) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d "
> + "missing a fixed-bar-N= value",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), i);
> + exit(1);
> + }
> + }
> +
> + /* Per-BAR checks: alignment, MMIO window, overlap. */
> + for (i = 0; i < PCI_NUM_REGIONS - 1; i++) {
> + r = &pdev->io_regions[i];
> + if (!r->size || (r->type & PCI_BASE_ADDRESS_SPACE_IO)) {
> + continue;
> + }
> +
> + is_64bit = !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
> + addr = (uint64_t)pdev->fixed_bar_addr[i];
> +
> + if (r->size - 1 > UINT64_MAX - addr) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d "
> + "addr=0x%"PRIx64" + size=0x%"PRIx64" overflows",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> + i, addr, r->size);
> + exit(1);
> + }
> + end = addr + r->size - 1;
> +
> + if (is_64bit && !(r->type & PCI_BASE_ADDRESS_MEM_PREFETCH) &&
> + !pci_bus_is_root(pci_get_bus(pdev)) && addr >= 4 * GiB) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d is a "
> + "non-prefetchable 64-bit BAR behind a PCI bridge "
> + "and cannot be placed above 4GB",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), i);
> + exit(1);
> + }
> +
> + /*
> + * wbase/wlim are both inclusive bounds (mmio*_limit = base+size-1,
> + * set by the caller alongside mmio*_base), matching end's own
> + * inclusive computation above.
> + */
> + if (is_64bit) {
> + wbase = info->mmio64_base;
> + wlim = info->mmio64_limit;
> + wname = "64-bit MMIO";
> + } else {
> + wbase = info->mmio32_base;
> + wlim = info->mmio32_limit;
> + wname = "32-bit MMIO";
> + }
> +
> + if (addr & (r->size - 1)) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d "
> + "addr=0x%"PRIx64" not aligned to size=0x%"PRIx64,
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> + i, addr, r->size);
> + exit(1);
> + }
> +
> + if (addr < wbase || end > wlim) {
This will reject a 64-bit BAR whose requested address falls within
the low MMIO aperture. That is a valid placement, right?
Is restricting 64-bit BARs to the high aperture intentional?
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d "
> + "[0x%"PRIx64"..0x%"PRIx64"] outside %s window "
> + "[0x%"PRIx64"..0x%"PRIx64"]",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> + i, addr, end, wname, wbase, wlim);
> + exit(1);
> + }
> +
> + if (fixed_claims_overlap(addr, end, &overlap_owner, &overlap_bar,
> + &overlap_start, &overlap_end)) {
> + error_report("fixed-bar: %s [%02x:%02x.%x] BAR%d "
> + "[0x%"PRIx64"..0x%"PRIx64"] overlaps %s BAR%d "
> + "[0x%"PRIx64"..0x%"PRIx64"]",
> + devname, pci_dev_bus_num(pdev),
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> + i, addr, end, overlap_owner, overlap_bar,
> + overlap_start, overlap_end);
> + exit(1);
May be better if we could pass an Error **errp through the helpers instead
of calling exit(1) here and let the machine code handle the error?
Thanks,
Shameer
