| Message ID | 20230314111435.89796-5-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/pci: Ensure capabilities are added before calling pci_qdev_realize() | expand |
On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote: > PCI capabilities can't appear magically at runtime. > Guests aren't expecting that. Assert all capabilities > are added _before_ a device instance is realized. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/pci/pci.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index ac41fcbf6a..ed60b352e4 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev) > * On success, pci_add_capability() returns a positive value > * that the offset of the pci capability. > * On failure, it sets an error and returns a negative error > - * code. > + * code. @pdev must be unrealized. > */ > int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > uint8_t offset, uint8_t size, > @@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > uint8_t *config; > int i, overlapping_cap; > > + assert(!DEVICE(pdev)->realized); > + > if (!offset) { > offset = pci_find_space(pdev, size); > /* out of PCI config space is programming error */ Fails in CI: https://gitlab.com/mstredhat/qemu/-/jobs/3976974199 qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion `!DEVICE(pdev)->realized' failed. Broken pipe ../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped) TAP parsing error: Too few tests run (expected 49, got 40) (test program exited with status code -6) > -- > 2.38.1
On 22/3/23 03:18, Michael S. Tsirkin wrote: > On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote: >> PCI capabilities can't appear magically at runtime. >> Guests aren't expecting that. Assert all capabilities >> are added _before_ a device instance is realized. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/pci/pci.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index ac41fcbf6a..ed60b352e4 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev) >> * On success, pci_add_capability() returns a positive value >> * that the offset of the pci capability. >> * On failure, it sets an error and returns a negative error >> - * code. >> + * code. @pdev must be unrealized. >> */ >> int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, >> uint8_t offset, uint8_t size, >> @@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, >> uint8_t *config; >> int i, overlapping_cap; >> >> + assert(!DEVICE(pdev)->realized); >> + >> if (!offset) { >> offset = pci_find_space(pdev, size); >> /* out of PCI config space is programming error */ > > Fails in CI: > > https://gitlab.com/mstredhat/qemu/-/jobs/3976974199 > > qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion `!DEVICE(pdev)->realized' failed. > Broken pipe > ../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped) > TAP parsing error: Too few tests run (expected 49, got 40) > (test program exited with status code -6) Thanks for testing! Likely the AMD-Vi device, see on the cover this series is Based-on: <20230313153031.86107-1-philmd@linaro.org> "hw/i386/amd_iommu: Orphanize & QDev cleanups" https://lore.kernel.org/qemu-devel/20230313153031.86107-1-philmd@linaro.org/
On 22/3/23 09:52, Philippe Mathieu-Daudé wrote: > On 22/3/23 03:18, Michael S. Tsirkin wrote: >> On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote: >>> PCI capabilities can't appear magically at runtime. >>> Guests aren't expecting that. Assert all capabilities >>> are added _before_ a device instance is realized. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> hw/pci/pci.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >>> index ac41fcbf6a..ed60b352e4 100644 >>> --- a/hw/pci/pci.c >>> +++ b/hw/pci/pci.c >>> @@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev) >>> * On success, pci_add_capability() returns a positive value >>> * that the offset of the pci capability. >>> * On failure, it sets an error and returns a negative error >>> - * code. >>> + * code. @pdev must be unrealized. >>> */ >>> int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, >>> uint8_t offset, uint8_t size, >>> @@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t >>> cap_id, >>> uint8_t *config; >>> int i, overlapping_cap; >>> + assert(!DEVICE(pdev)->realized); >>> + >>> if (!offset) { >>> offset = pci_find_space(pdev, size); >>> /* out of PCI config space is programming error */ >> >> Fails in CI: >> >> https://gitlab.com/mstredhat/qemu/-/jobs/3976974199 >> >> qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion >> `!DEVICE(pdev)->realized' failed. >> Broken pipe >> ../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from >> signal 6 (Aborted) (core dumped) >> TAP parsing error: Too few tests run (expected 49, got 40) >> (test program exited with status code -6) > > Thanks for testing! > > Likely the AMD-Vi device, see on the cover this series is > Based-on: <20230313153031.86107-1-philmd@linaro.org> > "hw/i386/amd_iommu: Orphanize & QDev cleanups" > https://lore.kernel.org/qemu-devel/20230313153031.86107-1-philmd@linaro.org/ I confirm this is the AMD-Vi device, so you are missing the previous (based-on) series: #1 0x102d4e5b0 in pci_add_capability pci.c:2354 #2 0x102d2ff28 in msi_init msi.c:227 #3 0x10371a340 in amdvi_sysbus_realize amd_iommu.c:1553 #4 0x1037194e8 in x86_iommu_realize x86-iommu.c:124 #5 0x10409db88 in device_set_realized+0x788 (qemu-system-i386:arm64+0x101d91b88) #6 0x1040cb248 in property_set_bool+0x2a0 (qemu-system-i386:arm64+0x101dbf248) #7 0x1040c31f4 in object_property_set+0x4bc (qemu-system-i386:arm64+0x101db71f4) #8 0x1040d9990 in object_property_set_qobject+0x38 (qemu-system-i386:arm64+0x101dcd990) #9 0x1040c40f8 in object_property_set_bool+0xfc (qemu-system-i386:arm64+0x101db80f8) #10 0x10409639c in qdev_realize+0x3bc (qemu-system-i386:arm64+0x101d8a39c) #11 0x10334f8e8 in qdev_device_add_from_qdict qdev-monitor.c:714 #12 0x103352114 in qdev_device_add qdev-monitor.c:733 #13 0x10337906c in device_init_func vl.c:1140 #14 0x104a84200 in qemu_opts_foreach qemu-option.c:1135 #15 0x103364fcc in qemu_create_cli_devices vl.c:2541 #16 0x103364ab8 in qmp_x_exit_preconfig vl.c:2609 #17 0x10336c0dc in qemu_init vl.c:3611 #18 0x1040812e4 in main main.c:47 #19 0x1a025be4c (<unknown module>) Due to the required base series, this is 8.1 material.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index ac41fcbf6a..ed60b352e4 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev) * On success, pci_add_capability() returns a positive value * that the offset of the pci capability. * On failure, it sets an error and returns a negative error - * code. + * code. @pdev must be unrealized. */ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size, @@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t *config; int i, overlapping_cap; + assert(!DEVICE(pdev)->realized); + if (!offset) { offset = pci_find_space(pdev, size); /* out of PCI config space is programming error */
PCI capabilities can't appear magically at runtime. Guests aren't expecting that. Assert all capabilities are added _before_ a device instance is realized. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/pci/pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)