On 10/27/2026 10:41 AM, Shameer Kolothum Thodi wrote:
>
>
>> -----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?
Sure.
>
>> +
>> +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?
We shouldn't have to. I will amend this in V2.
>
>> + 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?
Yes.
Thanks.
-Tushar
>
> Thanks,
> Shameer
