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

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

On 1/13/23 4:33 AM, Igor Mammedov wrote:
> On Thu, 12 Jan 2023 23:14:26 -0500
> Chuck Zmudzinski <brchuckz@xxxxxxx> wrote:
> 
>> On 1/12/23 6:03 PM, Michael S. Tsirkin wrote:
>> > On Thu, Jan 12, 2023 at 10:55:25PM +0000, Bernhard Beschow wrote:  
>> >> I think the change Michael suggests is very minimalistic: Move the if
>> >> condition around xen_igd_reserve_slot() into the function itself and
>> >> always call it there unconditionally -- basically turning three lines
>> >> into one. Since xen_igd_reserve_slot() seems very problem specific,
>> >> Michael further suggests to rename it to something more general. All
>> >> in all no big changes required.  
>> > 
>> > yes, exactly.
>> >   
>> 
>> OK, got it. I can do that along with the other suggestions.
> 
> have you considered instead of reservation, putting a slot check in device 
> model
> and if it's intel igd being passed through, fail at realize time  if it can't 
> take
> required slot (with a error directing user to fix command line)?

Yes, but the core pci code currently already fails at realize time
with a useful error message if the user tries to use slot 2 for the
igd, because of the xen platform device which has slot 2. The user
can fix this without patching qemu, but having the user fix it on
the command line is not the best way to solve the problem, primarily
because the user would need to hotplug the xen platform device via a
command line option instead of having the xen platform device added by
pc_xen_hvm_init functions almost immediately after creating the pci
bus, and that delay in adding the xen platform device degrades
startup performance of the guest.

> That could be less complicated than dealing with slot reservations at the 
> cost of
> being less convenient.

And also a cost of reduced startup performance.

However, the performance hit can be prevented by assigning slot
3 instead of slot 2 for the xen platform device if igd passthrough
is configured on the command line instead of doing slot reservation,
but there would still be less convenience and, for libxl users, an
inability to easily configure the command line so that the igd can
still have slot 2 without a hacky and error-prone patch to libxl to
deal with this problem.

I did post a patch on xen-devel to fix this using libxl, but so far
it has not yet been reviewed and I mentioned in that patch that the
approach of patching qemu so qemu reserves slot 2 for the igd is less
prone to coding errors and is easier to maintain than the patch that
would be required to implement the fix in libxl.