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

[Chuck Zmudzinski]

On 1/9/23 6:35 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 09, 2023 at 06:28:44PM -0500, Chuck Zmudzinski wrote:
>> On 1/9/23 5:33 PM, Michael S. Tsirkin wrote:
>> > On Mon, Jan 09, 2023 at 04:55:42PM -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.
>> > 
>> > I don't like how slot_reserved_mask is set initially then cleared on
>> > device realize.
>> > To me this looks like a fragile hack. I suggest one of the following
>> > 1. adding a new mask
>> > "slot-manual-mask" or some such blocking auto-allocation of a given
>> > slot without blocking its use if address is specified on command line.
>> > 2. adding a special property that overrides slot_reserved_mask
>> > for a given device.
>> > 
>> > both need changes in pci core but look like something generally
>> > useful.
>> 
>> I was hoping to not need to touch pci core but I understand it would be
>> better for this patch to not affect machines that are manually configured
>> on the command line.
>> 
>> However, keep in mind that this patch will only actually reserve the slot
>> initially for xen hvm machines (machine type "xenfv") that also are 
>> configured
>> with the qemu igd-passthru=on option which, AFAIK, only applies to machines
>> witn accel=xen. It will not affect kvm users at all. So I don't think this 
>> patch
>> will break many machines out there that manually specify the pci slots. The
>> only machines it could affect are machines configured for igd-passthru on 
>> xen.
>> This patch also does *not* reserve the slot initially for "xenfv" machines 
>> that
>> are not configured with igd passthrough which I am sure is the vast majority
>> of all the xen virtual machines out in the wild.
> 
> I'm just saying that adding a capability that is generally useful
> as opposed to xen specific means less technical debt.
> 

I agree with that.