Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

On 1/6/23 9:31 AM, Chuck Zmudzinski wrote:
> On 1/6/23 9:10 AM, Chuck Zmudzinski wrote:
>> On 1/6/23 9:03 AM, Anthony PERARD wrote:
>>> On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote:
>>>> Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus,
>>>> as noted in docs/igd-assign.txt in the Qemu source code.
>>>> 
>>>> Currently, when the xl toolstack is used to configure a Xen HVM guest with
>>>> Intel IGD passthrough to the guest with the Qemu upstream device model,
>>>> a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy
>>>> a different slot. This problem often prevents the guest from booting.
>>>> 
>>>> The only available workaround is not good: Configure Xen HVM guests to use
>>>> the old and no longer maintained Qemu traditional device model available
>>>> from xenbits.xen.org which does reserve slot 2 for the Intel IGD.
>>>> 
>>>> To implement this feature in the Qemu upstream device model for Xen HVM
>>>> guests, introduce the following new functions, types, and macros:
>>>> 
>>>> * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE
>>>> * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS
>>>> * typedef XenPTQdevRealize function pointer
>>>> * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2
>>>> * xen_igd_reserve_slot and xen_igd_clear_slot functions
>>>> 
>>>> The new xen_igd_reserve_slot function uses the existing slot_reserved_mask
>>>> member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using
>>>> the xl toolstack with the gfx_passthru option enabled, which sets the
>>>> igd-passthru=on option to Qemu for the Xen HVM machine type.
>>>> 
>>>> The new xen_igd_reserve_slot function also needs to be implemented in
>>>> hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case
>>>> when Qemu is configured with --enable-xen and 
>>>> --disable-xen-pci-passthrough,
>>>> in which case it does nothing.
>>>> 
>>>> The new xen_igd_clear_slot function overrides qdev->realize of the parent
>>>> PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus
>>>> since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was
>>>> created in hw/i386/pc_piix.c for the case when igd-passthru=on.
>>>> 
>>>> Move the call to xen_host_pci_device_get, and the associated error
>>>> handling, from xen_pt_realize to the new xen_igd_clear_slot function to
>>>> initialize the device class and vendor values which enables the checks for
>>>> the Intel IGD to succeed. The verification that the host device is an
>>>> Intel IGD to be passed through is done by checking the domain, bus, slot,
>>>> and function values as well as by checking that gfx_passthru is enabled,
>>>> the device class is VGA, and the device vendor in Intel.
>>>> 
>>>> Signed-off-by: Chuck Zmudzinski <brchuckz@xxxxxxx>
>>> 
>>> 
>>> This patch looks good enough. It only changes the "xenfv" machine so it
>>> doesn't prevent a proper fix to be done in the toolstack libxl.
>>> 
>>> The change in xen_pci_passthrough_class_init() to try to run some code
>>> before pci_qdev_realize() could potentially break in the future due to
>>> been uncommon but hopefully that will be ok.
>>> 
>>> So if no work to fix libxl appear soon, I'm ok with this patch:
> 
> Well, I can tell you and others who use qemu are more comfortable
> fixing this in libxl, so hold off for a week or so. I should have
> a patch to fix this in libxl written and tested by then. If for
> some reason that does not work out, then we can fix it in qemu.

One last thought: the only donwnside to fixing this in libxl is that
other toolstacks that configure qemu to use the xenfv machine will not
benefit from the fix in qemu that would simplify configuring the
guest correctly for the igd. Other toolstacks would still need to
override the default behavior of adding the xen platform device at
slot 2. I think no matter what, we should at least patch qemu to have
the xen-platform device use slot 3 instead of being automatically assigned
to slot 2 when igd-passthru=on. The rest of the fix could then be
implemented in libxl so that other pci devices such as emulated network
devices, other passed through pci devices, etc., do not take slot 2 when
gfx_passthru in xl.cfg is set.

So, unless I hear any objection, my plan is to patch qemu to use slot
3 for the xen platform device when igd-passthru is on, and implement the
rest of the fix in libxl. I should have it ready within a week.

Thanks for your help,

Chuck