Re: [PATCH v3 2/6] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Sat, Apr 01, 2023 at 10:36:45PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 30. März 2023 13:00:25 UTC schrieb Anthony PERARD 
> <anthony.perard@xxxxxxxxxx>:
> >On Sun, Mar 12, 2023 at 01:02:17PM +0100, Bernhard Beschow wrote:
> >> This is a preparational patch for the next one to make the following
> >> more obvious:
> >> 
> >> First, pci_bus_irqs() is now called twice in case of Xen where the
> >> second call overrides the pci_set_irq_fn with the Xen variant.
> >
> >pci_bus_irqs() does allocates pci_bus->irq_count, so the second call in
> >piix3_xen_realize() will leak `pci_bus->irq_count`. Could you look if
> >pci_bus_irqs_cleanup() can be called before the second pci_bus_irqs()
> >call, or maybe some other way to avoid the leak?
> 
> Thanks for catching this! I'll post a v4.
> 
> I think the most fool-proof way to fix this is to free irq_count just before 
> the assignment. pci_bus_irqs_cleanup() would then have to NULL the attribute 
> such that pci_bus_irqs() can be called afterwards.
> 
> BTW: I tried running qemu-system-x86_64 with PIIX4 rather than PIIX3 as Xen 
> guest with my pc-piix4 branch without success. This branch essentially just 
> provides slightly different PCI IDs for PIIX. Does xl or something else in 
> Xen check these? If not then this means I'm still missing something. Under 
> KVM this branch works just fine. Any idea?

Maybe the ACPI tables provided by libxl needs to be updated.
Or maybe something in the firmware (SeaBIOS or OVMF/OvmfXen) check the
id (I know that the PCI id of the root bus is checked, but I don't know
if that's the one that's been changed).

Thanks,

-- 
Anthony PERARD