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

From: Anthony PERARD <anthony.perard@citrix.com>
To: Bernhard Beschow <shentey@gmail.com>
Cc: qemu-devel@nongnu.org,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"David Woodhouse" <dwmw@amazon.co.uk>,
	"Hervé Poussineau" <hpoussin@reactos.org>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Paul Durrant" <paul@xen.org>,
	xen-devel@lists.xenproject.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Chuck Zmudzinski" <brchuckz@aol.com>
Subject: Re: [PATCH v3 2/6] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
Date: Thu, 30 Mar 2023 14:00:25 +0100	[thread overview]
Message-ID: <f52c41f7-e662-4afd-8ac9-ce2c0da2b1be@perard> (raw)
In-Reply-To: <20230312120221.99183-3-shentey@gmail.com>

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?

> Second, pci_bus_set_route_irq_fn() is now also called in Xen mode.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Beside the leak which I think can happen only once, patch is fine:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD