[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/6] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_real
From: |
Chuck Zmudzinski |
Subject: |
Re: [PATCH v3 2/6] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize() |
Date: |
Wed, 20 Sep 2023 10:44:23 -0400 |
User-agent: |
Mozilla Thunderbird |
On 9/19/2023 4:02 PM, Bernhard Beschow wrote:
>
>
> Am 3. April 2023 12:27:14 UTC schrieb Jason Andryuk <jandryuk@gmail.com>:
>>On Mon, Apr 3, 2023 at 5:33 AM Anthony PERARD <anthony.perard@citrix.com>
>>wrote:
>>>
>>> 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@citrix.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?
>>> >
>>> > 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).
>>
>>Xen also has hvmloader, which runs before SeaBIOS/OVMF. Looking at
>>tools/firmware/hvmloader/pci.c, it has
>> ASSERT((devfn != PCI_ISA_DEVFN) ||
>> ((vendor_id == 0x8086) && (device_id == 0x7000)));
>>
>>From QEMU, it looks like 0x7000 is PCI_DEVICE_ID_INTEL_82371SB_0, but
>>PIIX4 uses 0x7110 (PCI_DEVICE_ID_INTEL_82371AB_0). Maybe try removing
>>that check?
>
> I was finally able to build Xen successfully (without my distribution
> providing too recent dependencies that prevent compilation). With 0x7110
> added in the line above I could indeed run a Xen guest with PIIX4. Yay!
>
> Now I just need to respin my PIIX consolidation series...
Welcome to the world of running guests on Xen! I am the one who tested your
earlier patches with Xen guests, and I just wanted to say thanks for keeping me
in the loop. Please Cc me when you post your respin of the PIIX consolidation
series since I would like to also test it in my Xen environment. I understand I
will also need to patch hvmloader.c on the Xen side to set the correct device
id.
Kind regards,
Chuck
>
> Best regards,
> Bernhard
>
>>
>>Regards,
>>Jason