Re: [Qemu-devel] [Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 22 May 2014 14:25:32 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, May 22, 2014 at 10:50:10AM +0000, Chen, Tiejun wrote:
> > > -----Original Message-----
> > > From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> > > Sent: Thursday, May 22, 2014 2:45 PM
> > > To: Chen, Tiejun
> > > Cc: Anthony PERARD; Daniel P. Berrange; peter.maydell@linaro.org;
> > > xen-devel@lists.xensource.com; mst@redhat.com;
> > > stefano.stabellini@eu.citrix.com; Kay, Allen M; Kelly.Zytaruk@amd.com;
> > > qemu-devel@nongnu.org; Zhang, Yang Z; anthony@codemonkey.ws
> > > Subject: Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve
> > > 00:02.0 for INTEL IGD
> > > 
> > >   Hi,
> > > 
> > > > > Another useful thing would be to not create the xen platform device
> > > > > in case "-nodefaults" was specified on the command line (that switch
> > > > > turns off a bunch of other devices present by default: vga, nic, cdrom, ...).
> > > >
> > > > Currently looks 'xen-platform' itself can't be created, not those devices
> > > existed on that.
> > > 
> > > The error message looks more like libxl tries to hot-unplug the xen platform
> > > device.
> > > 
> > > Attached patch (untested!) hooks up the xen platform device to the default
> > > device code we have in qemu.  Two effects:
> > > 
> > >   (1) As mentioned above the device will not be created in case
> > >       -nodefaults  is specified on the command line.
> > >   (2) Autocreating the device is also turned off in case xen-platform
> > >       is added manually via -device.
> > > 
> > > With the patch applied you should be able to move the xen-platform device to
> > > some other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > > 
> > 
> > Gerd,
> > 
> > Sorry, I may misunderstand what you mean previously then have a wrong test.
> > 
> > So this still doesn't work actually. 
> > 
> > After applied your patch, 'xen-platform' is always disabled by default, right? So 00:02.0 is left naturally to be assigned to IGD as we expect like this,
> > 
> > tchen0@tchen0-HVM-domU:~$ lspci
> > 00:00.0 Host bridge: Intel Corporation 4th Gen Core Processor DRAM Controller (rev 06)
> > 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
> > 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> > 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
> > 00:02.0 VGA compatible controller: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor Integrated Graphics Controller (rev 06)
> > 00:03.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB EHCI #2 (rev 04)
> > 00:1f.0 ISA bridge: Intel Corporation Q87 Express LPC Controller (rev 04)
> > 
> > Then this is fine but if you intend to add a 'qemu -device xen-platform,addr=$slot', this doesn't work well. In my case,
> > 
> > gfx_passthru=1
> > pci=["00:02.0@2", "00:1a.0"]
> > xen_platform_pci=0
> > device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']
> > 
> > tchen0@tchen0-linux:~/workspace$ sudo xl cr domu-cfg
> > Parsing config from domu-cfg
> > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an error message from QMP server: Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set
> > libxl: error: libxl_create.c:1277:domcreate_attach_pci: libxl_device_pci_add failed: -3
> > 
> > Thanks
> > Tiejun
> 
> Weird: this implies use_acpi_pci_hotplug got set somehow.
even if it's not set, and xenfv compat prop says it isn't,
root PCI bus should have property present.

Tiejun,
  What is output of the following debug patch:

---

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 3b143b3..6f8e26d 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -305,6 +305,7 @@ void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus,
     s->root= root_bus;
     s->legacy_piix = !bridges_enabled;
 
+    fprintf(stderr, "acpi_pcihp_init\n");
     if (s->legacy_piix) {
         unsigned *bus_bsel = g_malloc(sizeof *bus_bsel);
 
@@ -313,6 +314,7 @@ void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus,
         *bus_bsel = ACPI_PCIHP_BSEL_DEFAULT;
         object_property_add_uint32_ptr(OBJECT(root_bus), ACPI_PCIHP_PROP_BSEL,
                                        bus_bsel, NULL);
+	fprintf(stderr, "Legacy PCI hotplug\n");
     }
 
     memory_region_init_io(&s->io, NULL, &acpi_pcihp_io_ops, s,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9fac589..a1211dd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -736,6 +736,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
     unsigned *bsel_alloc = opaque;
     unsigned *bus_bsel;
 
+    fprintf(stderr, "acpi_set_bsel bus: %s, hotpluggable: %d\n", bus->qbus.name, bus->qbus.allow_hotplug);
     if (bus->qbus.allow_hotplug) {
         bus_bsel = g_malloc(sizeof *bus_bsel);