* [PATCH 2/2] tests/qtest/fuzz: Add generic fuzzer for pcie-pci-bridge
2025-10-08 19:19 [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Navid Emamdoost
@ 2025-10-08 19:19 ` Navid Emamdoost
2025-10-10 15:58 ` [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Alexander Bulekov
2025-11-13 14:02 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Navid Emamdoost @ 2025-10-08 19:19 UTC (permalink / raw)
To: qemu-devel
Cc: navidem, zsm, Alexander Bulekov, Paolo Bonzini, Bandan Das,
Stefan Hajnoczi, Fabiano Rosas, Darren Kenny, Qiuhao Li,
Laurent Vivier
Add a new generic fuzz target for the 'pcie-pci-bridge' device. This
target uses a Q35 machine with a multi-level PCI hierarchy to exercise
the bridge's functionality.
This is made possible by the preceding change to handle unimplemented
BARs during fuzzing.
---
This new target significantly improves code coverage for the pcie-pci-bridge
implementation. The baseline coverage shown below was generated by
running all existing fuzz targets with the oss-fuzz corpus.
=== Component: hw/pci ===
-------------------------------------------------------------------------------
File New Target Baseline Change
-------------------------------------------------------------------------------
shpc.c 359/511 (70.3%) 0/511 (0.0%) +359
pci_bridge.c 255/304 (83.9%) 12/304 (3.9%) +243
pcie.c 390/774 (50.4%) 160/774 (20.7%) +230
pcie_aer.c 119/524 (22.7%) 38/524 (7.3%) +81
pci.c 1154/2069 (55.8%) 1084/2069 (52.4%) +70
pcie_port.c 58/119 (48.7%) 17/119 (14.3%) +41
pci.h 86/132 (65.2%) 81/132 (61.4%) +5
=== Component: hw/pci-bridge ===
-------------------------------------------------------------------------------
File New Target Baseline Change
-------------------------------------------------------------------------------
pcie_root_port.c 86/127 (67.7%) 13/127 (10.2%) +73
pcie_pci_bridge.c 62/94 (66.0%) 20/94 (21.3%) +42
gen_pcie_root_port.c 45/66 (68.2%) 19/66 (28.8%) +26
Signed-off-by: Navid Emamdoost <navidem@google.com>
---
tests/qtest/fuzz/generic_fuzz_configs.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
index ef0ad95712..e025f57a3e 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -247,6 +247,14 @@ const generic_fuzz_config predefined_configs[] = {
.args = "-machine q35 -nodefaults "
"-parallel file:/dev/null",
.objects = "parallel*",
+ },{
+ .name = "pcie-pci-bridge",
+ .args = "-machine q35 -nodefaults "
+ "-device pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=true,addr=0x2 "
+ "-device pcie-pci-bridge,id=pci.2,bus=pci.1,addr=0x0 "
+ "-netdev user,id=net0 "
+ "-device e1000,netdev=net0,id=nic0,bus=pci.2,addr=0x3",
+ .objects = "pci* shpc*"
}
};
--
2.51.0.710.ga91ca5db03-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds
2025-10-08 19:19 [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Navid Emamdoost
2025-10-08 19:19 ` [PATCH 2/2] tests/qtest/fuzz: Add generic fuzzer for pcie-pci-bridge Navid Emamdoost
@ 2025-10-10 15:58 ` Alexander Bulekov
2025-10-14 1:14 ` Navid Emamdoost
2025-11-13 14:02 ` Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Bulekov @ 2025-10-10 15:58 UTC (permalink / raw)
To: Navid Emamdoost
Cc: qemu-devel, zsm, Fabiano Rosas, Laurent Vivier, Paolo Bonzini
On 251008 1919, Navid Emamdoost wrote:
> The qpci_iomap() function fails with a fatal g_assert(addr) if it
> probes a PCI BAR that has a size of zero. This is expected behavior
> for certain devices, like the Q35 PCI Host Bridge, which have valid but
> unimplemented BARs.
> This assertion blocks the creation of fuzz targets for complex machine
> types that include these devices.
> Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> zero-sized BAR is now handled gracefully by returning an empty BAR
> struct, allowing fuzzing to proceed. The original assertion is kept for
> all other builds to maintain strict checking for qtest and production
> environments.
Is there a way to determine whether a BAR is unimplememnted from the
PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?
>
> Signed-off-by: Navid Emamdoost <navidem@google.com>
> ---
> tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b992..df9e2a3993 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> addr &= PCI_BASE_ADDRESS_MEM_MASK;
> }
>
> +#ifdef CONFIG_FUZZ
> + /*
> + * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> + * error. This occurs when probing devices like the Q35 host bridge. We
> + * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> + * we retain the original g_assert() to catch unexpected behavior.
> + */
> + if (!addr) {
> + if (sizeptr) {
> + *sizeptr = 0;
> + }
> + memset(&bar, 0, sizeof(bar));
> + return bar;
> + }
> +#endif
> +
> g_assert(addr); /* Must have *some* size bits */
>
> size = 1U << ctz32(addr);
> --
> 2.51.0.710.ga91ca5db03-goog
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds
2025-10-10 15:58 ` [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Alexander Bulekov
@ 2025-10-14 1:14 ` Navid Emamdoost
2025-11-06 18:41 ` Navid Emamdoost
0 siblings, 1 reply; 6+ messages in thread
From: Navid Emamdoost @ 2025-10-14 1:14 UTC (permalink / raw)
To: Alexander Bulekov
Cc: qemu-devel, zsm, Fabiano Rosas, Laurent Vivier, Paolo Bonzini
Hi Alexander,
On Fri, Oct 10, 2025 at 8:59 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 251008 1919, Navid Emamdoost wrote:
> > The qpci_iomap() function fails with a fatal g_assert(addr) if it
> > probes a PCI BAR that has a size of zero. This is expected behavior
> > for certain devices, like the Q35 PCI Host Bridge, which have valid but
> > unimplemented BARs.
> > This assertion blocks the creation of fuzz targets for complex machine
> > types that include these devices.
> > Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> > zero-sized BAR is now handled gracefully by returning an empty BAR
> > struct, allowing fuzzing to proceed. The original assertion is kept for
> > all other builds to maintain strict checking for qtest and production
> > environments.
>
> Is there a way to determine whether a BAR is unimplememnted from the
> PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?
>
Fair point. I don't think we have a reliable way to determine if a BAR
is truly unimplemented from the PCIDevice model without probing it. If
we moved that hardware probe into pci_enum, it would become
inefficient for all the BARs that are implemented, as they would be
probed twice: once in pci_enum just to check, and then again inside
qpci_iomap to do the actual mapping. That's why I think delegating
this check to qpci_iomap is the cleaner approach.
> >
> > Signed-off-by: Navid Emamdoost <navidem@google.com>
> > ---
> > tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> > index a59197b992..df9e2a3993 100644
> > --- a/tests/qtest/libqos/pci.c
> > +++ b/tests/qtest/libqos/pci.c
> > @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> > addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > }
> >
> > +#ifdef CONFIG_FUZZ
> > + /*
> > + * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> > + * error. This occurs when probing devices like the Q35 host bridge. We
> > + * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> > + * we retain the original g_assert() to catch unexpected behavior.
> > + */
> > + if (!addr) {
> > + if (sizeptr) {
> > + *sizeptr = 0;
> > + }
> > + memset(&bar, 0, sizeof(bar));
> > + return bar;
> > + }
> > +#endif
> > +
> > g_assert(addr); /* Must have *some* size bits */
> >
> > size = 1U << ctz32(addr);
> > --
> > 2.51.0.710.ga91ca5db03-goog
> >
> >
--
Thank you,
Navid.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds
2025-10-14 1:14 ` Navid Emamdoost
@ 2025-11-06 18:41 ` Navid Emamdoost
0 siblings, 0 replies; 6+ messages in thread
From: Navid Emamdoost @ 2025-11-06 18:41 UTC (permalink / raw)
To: Alexander Bulekov
Cc: qemu-devel, zsm, Fabiano Rosas, Laurent Vivier, Paolo Bonzini
On Mon, Oct 13, 2025 at 6:14 PM Navid Emamdoost <navidem@google.com> wrote:
>
> Hi Alexander,
>
> On Fri, Oct 10, 2025 at 8:59 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> >
> > On 251008 1919, Navid Emamdoost wrote:
> > > The qpci_iomap() function fails with a fatal g_assert(addr) if it
> > > probes a PCI BAR that has a size of zero. This is expected behavior
> > > for certain devices, like the Q35 PCI Host Bridge, which have valid but
> > > unimplemented BARs.
> > > This assertion blocks the creation of fuzz targets for complex machine
> > > types that include these devices.
> > > Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> > > zero-sized BAR is now handled gracefully by returning an empty BAR
> > > struct, allowing fuzzing to proceed. The original assertion is kept for
> > > all other builds to maintain strict checking for qtest and production
> > > environments.
> >
> > Is there a way to determine whether a BAR is unimplememnted from the
> > PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?
> >
>
> Fair point. I don't think we have a reliable way to determine if a BAR
> is truly unimplemented from the PCIDevice model without probing it. If
> we moved that hardware probe into pci_enum, it would become
> inefficient for all the BARs that are implemented, as they would be
> probed twice: once in pci_enum just to check, and then again inside
> qpci_iomap to do the actual mapping. That's why I think delegating
> this check to qpci_iomap is the cleaner approach.
>
Friendly ping.
> > >
> > > Signed-off-by: Navid Emamdoost <navidem@google.com>
> > > ---
> > > tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> > >
> > > diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> > > index a59197b992..df9e2a3993 100644
> > > --- a/tests/qtest/libqos/pci.c
> > > +++ b/tests/qtest/libqos/pci.c
> > > @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> > > addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > > }
> > >
> > > +#ifdef CONFIG_FUZZ
> > > + /*
> > > + * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> > > + * error. This occurs when probing devices like the Q35 host bridge. We
> > > + * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> > > + * we retain the original g_assert() to catch unexpected behavior.
> > > + */
> > > + if (!addr) {
> > > + if (sizeptr) {
> > > + *sizeptr = 0;
> > > + }
> > > + memset(&bar, 0, sizeof(bar));
> > > + return bar;
> > > + }
> > > +#endif
> > > +
> > > g_assert(addr); /* Must have *some* size bits */
> > >
> > > size = 1U << ctz32(addr);
> > > --
> > > 2.51.0.710.ga91ca5db03-goog
> > >
> > >
>
>
>
> --
> Thank you,
> Navid.
--
Thank you,
Navid.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds
2025-10-08 19:19 [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Navid Emamdoost
2025-10-08 19:19 ` [PATCH 2/2] tests/qtest/fuzz: Add generic fuzzer for pcie-pci-bridge Navid Emamdoost
2025-10-10 15:58 ` [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds Alexander Bulekov
@ 2025-11-13 14:02 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2025-11-13 14:02 UTC (permalink / raw)
To: Navid Emamdoost
Cc: qemu-devel, zsm, Fabiano Rosas, Laurent Vivier, Paolo Bonzini
On Wed, 8 Oct 2025 at 20:21, Navid Emamdoost <navidem@google.com> wrote:
>
> The qpci_iomap() function fails with a fatal g_assert(addr) if it
> probes a PCI BAR that has a size of zero. This is expected behavior
> for certain devices, like the Q35 PCI Host Bridge, which have valid but
> unimplemented BARs.
> This assertion blocks the creation of fuzz targets for complex machine
> types that include these devices.
> Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> zero-sized BAR is now handled gracefully by returning an empty BAR
> struct, allowing fuzzing to proceed. The original assertion is kept for
> all other builds to maintain strict checking for qtest and production
> environments.
>
> Signed-off-by: Navid Emamdoost <navidem@google.com>
> ---
> tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b992..df9e2a3993 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> addr &= PCI_BASE_ADDRESS_MEM_MASK;
> }
>
> +#ifdef CONFIG_FUZZ
> + /*
> + * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> + * error. This occurs when probing devices like the Q35 host bridge. We
> + * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> + * we retain the original g_assert() to catch unexpected behavior.
> + */
> + if (!addr) {
> + if (sizeptr) {
> + *sizeptr = 0;
> + }
> + memset(&bar, 0, sizeof(bar));
> + return bar;
> + }
> +#endif
Personally I think I'd prefer it if we didn't make this
dependent on CONFIG_FUZZ. If BARs with no size are
a valid thing then the libqos API should handle them
(e.g. in theory one should be able to write a test that the
Q35 host bridge provides the addr=0 BARs it is supposed to).
I think if we added the size to the QPCIBar struct then we
could assert in the accessors like qpci_io_readb() and
friends that the offset provided was in range. That would
catch both the unlikely "we implemented the BAR with no
size" case and the rather more likely "we got the size too
small" case (and also the "bug in the test and it got the
offset too big" case), and would mean that we don't lose
anything by not asserting that we have a non-zero-size BAR here.
What do you think?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread