diff mbox

[v2,4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD

Message ID 1400741081.25874.38.camel@nilsson.home.kraxel.org
State New
Headers show

Commit Message

Gerd Hoffmann May 22, 2014, 6:44 a.m. UTC 
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'.

cheers,
  Gerd

Comments

Michael S. Tsirkin May 22, 2014, 6:49 a.m. UTC | #1 
On Thu, May 22, 2014 at 08:44:41AM +0200, Gerd Hoffmann wrote:
>   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'.
> 
> cheers,
>   Gerd
> 

> >From 405349bf38fc05653bfb8c971c61796e981eee98 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Thu, 22 May 2014 08:34:28 +0200
> Subject: [PATCH] xen: make xen-platform a default device
> 
> ---

Looks good to me, though Sob is missing.
Can some Xen folks pls ack this one? I'll queue it up then.

>  hw/i386/pc_piix.c    | 2 +-
>  include/hw/xen/xen.h | 1 +
>  vl.c                 | 3 +++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index eaf3e61..f987d03 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>      pc_init_pci(args);
>  
>      bus = pci_find_primary_bus();
> -    if (bus != NULL) {
> +    if (bus != NULL && default_xenplatform) {
>          pci_create_simple(bus, -1, "xen-platform");
>      }
>  }
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 85fda3d..b350413 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -20,6 +20,7 @@ enum xen_mode {
>  
>  extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
> +extern int default_xenplatform;
>  
>  extern bool xen_allowed;
>  
> diff --git a/vl.c b/vl.c
> index 709d8cd..673148e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -226,6 +226,7 @@ static int default_floppy = 1;
>  static int default_cdrom = 1;
>  static int default_sdcard = 1;
>  static int default_vga = 1;
> +int default_xenplatform = 1;
>  
>  static struct {
>      const char *driver;
> @@ -247,6 +248,7 @@ static struct {
>      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
>      { .driver = "vmware-svga",          .flag = &default_vga       },
>      { .driver = "qxl-vga",              .flag = &default_vga       },
> +    { .driver = "xen-platform",         .flag = &default_xenplatform },
>  };
>  
>  static QemuOptsList qemu_rtc_opts = {
> @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
>          default_monitor = 0;
>          default_net = 0;
>          default_vga = 0;
> +        default_xenplatform = 0;
>      }
>  
>      if (is_daemonized()) {
> -- 
> 1.8.3.1
>
Tiejun Chen May 22, 2014, 7:11 a.m. UTC | #2 
> -----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.


I think this is unnecessary to add this option to support GFX passthrough.

>   (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'.

> 


Yes, this patch works now. So I assume this would be merged into ML soon.

Thanks for your great help, Gerd!

Tiejun
Tiejun Chen May 22, 2014, 10:50 a.m. UTC | #3 
> -----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
Gonglei (Arei) May 22, 2014, 11:03 a.m. UTC | #4 
> -----Original Message-----

> From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org

> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On

> Behalf Of Chen, Tiejun

> Sent: Thursday, May 22, 2014 6:50 PM

> To: Gerd Hoffmann

> Cc: peter.maydell@linaro.org; xen-devel@lists.xensource.com;

> stefano.stabellini@eu.citrix.com; mst@redhat.com; Kay, Allen M;

> Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org; Zhang, Yang Z;

> anthony@codemonkey.ws; Anthony PERARD

> Subject: Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve

> 00:02.0 for INTEL IGD

> 

> > -----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

> 

Maybe you can assign explicitly the bus name, such as:
device_model_args_hvm = ['-device', 'xen-platform,bus=pci.0,addr=0x3']


Best regards,
-Gonglei
Gerd Hoffmann May 22, 2014, 11:22 a.m. UTC | #5 
Hi,

> After applied your patch, 'xen-platform' is always disabled by default, right?

Only in case -nodefaults is passed on the qemu command line (don't know
whenever libxl does that).

> gfx_passthru=1
> pci=["00:02.0@2", "00:1a.0"]
> xen_platform_pci=0
  ^^^^^^^^^^^^^^^^^^
That line isn't needed ...

> device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']

That alone should be enough to move away the platform device.

cheers,
  Gerd
Michael S. Tsirkin May 22, 2014, 11:25 a.m. UTC | #6 
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.
Tiejun Chen May 23, 2014, 1:07 a.m. UTC | #7 
> -----Original Message-----

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]

> Sent: Thursday, May 22, 2014 7:22 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,

> 

> > After applied your patch, 'xen-platform' is always disabled by default, right?

> 

> Only in case -nodefaults is passed on the qemu command line (don't know

> whenever libxl does that).

> 

> > gfx_passthru=1

> > pci=["00:02.0@2", "00:1a.0"]

> > xen_platform_pci=0

>   ^^^^^^^^^^^^^^^^^^

> That line isn't needed ...


Yes, this can work after remove this line:

gfx_passthru=1
pci=["00:02.0@2", "00:1a.0"]
#xen_platform_pci=0
device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']

Then domU has xen-platform again:

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 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
00:04.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)

> 

> > device_model_args_hvm = ['-device', 'xen-platform,addr=0x3']

> 

> That alone should be enough to move away the platform device.

> 


Sure.

Thanks
Tiejun
diff mbox

Patch

From 405349bf38fc05653bfb8c971c61796e981eee98 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 22 May 2014 08:34:28 +0200
Subject: [PATCH] xen: make xen-platform a default device

---
 hw/i386/pc_piix.c    | 2 +-
 include/hw/xen/xen.h | 1 +
 vl.c                 | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eaf3e61..f987d03 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -385,7 +385,7 @@  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
     pc_init_pci(args);
 
     bus = pci_find_primary_bus();
-    if (bus != NULL) {
+    if (bus != NULL && default_xenplatform) {
         pci_create_simple(bus, -1, "xen-platform");
     }
 }
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 85fda3d..b350413 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -20,6 +20,7 @@  enum xen_mode {
 
 extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
+extern int default_xenplatform;
 
 extern bool xen_allowed;
 
diff --git a/vl.c b/vl.c
index 709d8cd..673148e 100644
--- a/vl.c
+++ b/vl.c
@@ -226,6 +226,7 @@  static int default_floppy = 1;
 static int default_cdrom = 1;
 static int default_sdcard = 1;
 static int default_vga = 1;
+int default_xenplatform = 1;
 
 static struct {
     const char *driver;
@@ -247,6 +248,7 @@  static struct {
     { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
     { .driver = "vmware-svga",          .flag = &default_vga       },
     { .driver = "qxl-vga",              .flag = &default_vga       },
+    { .driver = "xen-platform",         .flag = &default_xenplatform },
 };
 
 static QemuOptsList qemu_rtc_opts = {
@@ -4101,6 +4103,7 @@  int main(int argc, char **argv, char **envp)
         default_monitor = 0;
         default_net = 0;
         default_vga = 0;
+        default_xenplatform = 0;
     }
 
     if (is_daemonized()) {
-- 
1.8.3.1