Re: [PATCH 4/4] hw/pci: Ensure pci_add_capability() is called before device is realized

[Philippe Mathieu-Daudé]

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"
> 20230313153031.86107-1-philmd@linaro.org/">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.