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

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

On 1/3/23 10:14 AM, Alex Williamson wrote:

> 
> It's necessary to configure the assigned IGD at slot 2 to make it
> functional, yes, but I don't really understand this notion of
> "reserving" slot 2.  If something occupies address 00:02.0 in the
> config, it's the user's or management tool's responsibility to move it
> to make this configuration functional.  Why does QEMU need to play a
> part in reserving this bus address.  IGD devices are not generally
> hot-pluggable either, so it doesn't seem we need to reserve an address
> in case an IGD device is added dynamically later.

The capability to reserve a bus address for a quirky device need not
be limited to the case of hotplugged or dynamically added devices. The
igd is a quirky device, and its presence in an emulated system like
qemu requires special handling. The slot_reserved_mask member of PCIBus
is also well-suited to the case of quirky device like Intel the igd that
needs to be at slot 2. Just because it is not dynamically added later
does not change the fact that it needs special handling at its initial
configuration when the guest is being created.

>  

Here's the problem that answers Michael's question why this patch is
specific to xen:

---snip---
#ifdef CONFIG_XEN

...

static void pc_xen_hvm_init(MachineState *machine)
{
    PCMachineState *pcms = PC_MACHINE(machine);

    if (!xen_enabled()) {
        error_report("xenfv machine requires the xen accelerator");
        exit(1);
    }

    pc_xen_hvm_init_pci(machine);
    pci_create_simple(pcms->bus, -1, "xen-platform");
}
#endif
---snip---

This code is from hw/i386/pc_piix.c. Note the call to
pci_create_simple to create the xen platform pci device,
which has -1 as the second argument. That -1 tells
pci_create_simple to autoconfigure the pci bdf address.

It is *hard-coded* that way. That means no toolstack or
management tool can change it. And what is hard-coded here
is that the xen platform device will occupy slot 2, preventing
the Intel igd or any other device from occupying slot 2.

So, even if xen developers wanted to create a version of the
libxl that is flexible enough to allow the xen platform device
to be at a different slot, they could not without patching
qemu to at least change that -1 to an initialization variable
that can be read from a qemu command line option that libxl
could configure.

So, why not just accept this patch as the best way to deal
with a xen-specific problem and fix it in a way that uses
the xen/libxl philosophy of autoconfiguring things as much as
possible except in cases of quirky devices like the Intel igd
in which case the existing slot_reserved_mask member of PCIBus
is very useful to accommodate the quirky igd device?

IMHO, trying to impose the kvm/libvirt philosophy of having
a very configurable toolstack on the xen/xenlight philosophy
of autoconfiguring things that can be autoconfigured and
using higher-level configuration options like igd-passthrough=on
to tweak how autoconfiguration is done in a way that is compatible
with quirky devices like the Intel igd is like trying to put
a square peg into a round hole. Actually, qemu with its qom is
able to accommodate both approaches to the design of a toolstack,
and each vendor or project that depends on qemu should be free to
use the approach it prefers.

Just my two cents, FWIW.

Kind regards,

Chuck