[Qemu-devel] [PATCH 0/7] igd passthrough chipset tweaks

[Qemu-devel] [PATCH 0/7] igd passthrough chipset tweaks

[Gerd Hoffmann]

  Hi,

We have some code in our tree to support pci passthrough of intel
graphics devices (igd) on xen, which requires some chipset tweaks
for (a) the host bridge and (b) the lpc/isa-bridge to meat the
expectations of the guest driver.  For kvm we need pretty much
the same, also the requirements for vgpu (xengt/kvmgt) are very
simliar.

This patch series tackles (a) only, (b) will follow later.  It
wires up the igd-passthru machine option for tcg/kvm too, moves
the code to its own file so it is nicely separated, fixes a bunch
of issues and finally adds q35 support.

This patch series has seen very light testing, basically doing
lspci in the guest to check whenever pci config space got updated
correctly.  Trying actual device assignment needs more pieces
being in place.  But I suspect even that is more testing than
the code has seen on xen so far (see patch #6 ...).

cheers,
  Gerd

Gerd Hoffmann (7):
  pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
  pc: move igd support code to igd.c
  igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
  igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
  igd: use defines for standard pci config space offsets
  igd: revamp host config read
  igd: add q35 support

 hw/i386/pc_piix.c         |  11 ++--
 hw/pci-host/Makefile.objs |   3 ++
 hw/pci-host/igd.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/pci-host/piix.c        |  88 -------------------------------
 hw/pci-host/q35.c         |   6 ++-
 5 files changed, 145 insertions(+), 95 deletions(-)
 create mode 100644 hw/pci-host/igd.c

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Gerd Hoffmann]

rename pc_xen_hvm_init_pci to pc_i440fx_init_pci,
use it for both xen and non-xen init.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc_piix.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2e41efe..ce6c3c5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine)
     pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
 }
 
-#ifdef CONFIG_XEN
-static void pc_xen_hvm_init_pci(MachineState *machine)
+static void pc_i440fx_init_pci(MachineState *machine)
 {
-    const char *pci_type = has_igd_gfx_passthru ?
+    const char *pci_type = machine->igd_gfx_passthru ?
                 TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE;
 
     pc_init1(machine,
@@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
              pci_type);
 }
 
+#ifdef CONFIG_XEN
 static void pc_xen_hvm_init(MachineState *machine)
 {
     PCIBus *bus;
@@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine)
         exit(1);
     }
 
-    pc_xen_hvm_init_pci(machine);
+    pc_i440fx_init_pci(machine);
 
     bus = pci_find_primary_bus();
     if (bus != NULL) {
@@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine)
         if (compat) { \
             compat(machine); \
         } \
-        pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
-                 TYPE_I440FX_PCI_DEVICE); \
+        pc_i440fx_init_pci(machine); \
     } \
     DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Marcel Apfelbaum]

On 12/08/2015 04:07 PM, Gerd Hoffmann wrote:
> rename pc_xen_hvm_init_pci to pc_i440fx_init_pci,
> use it for both xen and non-xen init.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/i386/pc_piix.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2e41efe..ce6c3c5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine)
>       pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
>   }
>
> -#ifdef CONFIG_XEN
> -static void pc_xen_hvm_init_pci(MachineState *machine)
> +static void pc_i440fx_init_pci(MachineState *machine)
>   {
> -    const char *pci_type = has_igd_gfx_passthru ?
> +    const char *pci_type = machine->igd_gfx_passthru ?
>                   TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE;
>
>       pc_init1(machine,
> @@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
>                pci_type);
>   }
>
> +#ifdef CONFIG_XEN
>   static void pc_xen_hvm_init(MachineState *machine)
>   {
>       PCIBus *bus;
> @@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine)
>           exit(1);
>       }
>
> -    pc_xen_hvm_init_pci(machine);
> +    pc_i440fx_init_pci(machine);
>
>       bus = pci_find_primary_bus();
>       if (bus != NULL) {
> @@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine)
>           if (compat) { \
>               compat(machine); \
>           } \
> -        pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> -                 TYPE_I440FX_PCI_DEVICE); \
> +        pc_i440fx_init_pci(machine); \

Hi Gerd,

A quick question, does IGD_PASSTHROUGH makes sense for compat machine types?
On the same topic, does machine->igd_gfx_passthru makes sense for all machine types?

Thanks,
Marcel


>       } \
>       DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
>
>

Re: [Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Gerd Hoffmann]

> Hi Gerd,
> 
> A quick question, does IGD_PASSTHROUGH makes sense for compat machine types?

Unlikely to be used in practice, but I don't feel like creating
different initialization code paths because of that ...

> On the same topic, does machine->igd_gfx_passthru makes sense for all machine types?

Same answer ;)

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Eduardo Habkost]

On Tue, Dec 08, 2015 at 03:07:22PM +0100, Gerd Hoffmann wrote:
> rename pc_xen_hvm_init_pci to pc_i440fx_init_pci,
> use it for both xen and non-xen init.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/pc_piix.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2e41efe..ce6c3c5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine)
>      pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
>  }
>  
> -#ifdef CONFIG_XEN
> -static void pc_xen_hvm_init_pci(MachineState *machine)
> +static void pc_i440fx_init_pci(MachineState *machine)
>  {
> -    const char *pci_type = has_igd_gfx_passthru ?
> +    const char *pci_type = machine->igd_gfx_passthru ?
>                  TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE;
>  

Have you considered removing the has_igd_gfx_passthru global
completely?

>      pc_init1(machine,
> @@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
>               pci_type);
>  }
>  
> +#ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(MachineState *machine)
>  {
>      PCIBus *bus;
> @@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    pc_xen_hvm_init_pci(machine);
+    pc_i440fx_init_pci(machine);
>  
>      bus = pci_find_primary_bus();
>      if (bus != NULL) {
> @@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine)
>          if (compat) { \
>              compat(machine); \
>          } \
> -        pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> -                 TYPE_I440FX_PCI_DEVICE); \
> +        pc_i440fx_init_pci(machine); \

machine->igd_gfx_passthru defaults to false, meaning that in the
default case the pc_init1() arguments in pc_i440fx_init_pci()
will be the same as the call being replaced above, keeping
exactly the same behavior.

This change breaks compatibility in the unlikely case somebody is
already using igd-passthru=on in non-xenfv machines. I don't
think it would make sense to keep a broken igd-passthru option in
pc-2.5 and older for compatibility if nobody ever used that
option, but it would be nice to mention that in the commit
message.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Stefano Stabellini]

On Wed, 9 Dec 2015, Eduardo Habkost wrote:
> On Tue, Dec 08, 2015 at 03:07:22PM +0100, Gerd Hoffmann wrote:
> > rename pc_xen_hvm_init_pci to pc_i440fx_init_pci,
> > use it for both xen and non-xen init.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/i386/pc_piix.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 2e41efe..ce6c3c5 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine)
> >      pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
> >  }
> >  
> > -#ifdef CONFIG_XEN
> > -static void pc_xen_hvm_init_pci(MachineState *machine)
> > +static void pc_i440fx_init_pci(MachineState *machine)
> >  {
> > -    const char *pci_type = has_igd_gfx_passthru ?
> > +    const char *pci_type = machine->igd_gfx_passthru ?
> >                  TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE;
> >  
> 
> Have you considered removing the has_igd_gfx_passthru global
> completely?

Indeed. It doesn't make much sense anymore.


> >      pc_init1(machine,
> > @@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
> >               pci_type);
> >  }
> >  
> > +#ifdef CONFIG_XEN
> >  static void pc_xen_hvm_init(MachineState *machine)
> >  {
> >      PCIBus *bus;
> > @@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine)
> >          exit(1);
> >      }
> >  
> > -    pc_xen_hvm_init_pci(machine);
> +    pc_i440fx_init_pci(machine);
> >  
> >      bus = pci_find_primary_bus();
> >      if (bus != NULL) {
> > @@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine)
> >          if (compat) { \
> >              compat(machine); \
> >          } \
> > -        pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> > -                 TYPE_I440FX_PCI_DEVICE); \
> > +        pc_i440fx_init_pci(machine); \
> 
> machine->igd_gfx_passthru defaults to false, meaning that in the
> default case the pc_init1() arguments in pc_i440fx_init_pci()
> will be the same as the call being replaced above, keeping
> exactly the same behavior.
> 
> This change breaks compatibility in the unlikely case somebody is
> already using igd-passthru=on in non-xenfv machines. I don't
> think it would make sense to keep a broken igd-passthru option in
> pc-2.5 and older for compatibility if nobody ever used that
> option, but it would be nice to mention that in the commit
> message.
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I agree

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

[Qemu-devel] [PATCH 2/7] pc: move igd support code to igd.c

[Gerd Hoffmann]

Pure code motion, except for dropping instance_size for
TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE (no need to set,
we can inherit it from TYPE_I440FX_PCI_DEVICE).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/Makefile.objs |  3 ++
 hw/pci-host/igd.c         | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/pci-host/piix.c        | 88 -------------------------------------------
 3 files changed, 99 insertions(+), 88 deletions(-)
 create mode 100644 hw/pci-host/igd.c

diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index 45f1f0e..e341a49 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -11,6 +11,9 @@ common-obj-$(CONFIG_PPCE500_PCI) += ppce500.o
 # ARM devices
 common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o
 
+# igd passthrough support
+common-obj-$(CONFIG_LINUX) += igd.o
+
 common-obj-$(CONFIG_PCI_APB) += apb.o
 common-obj-$(CONFIG_FULONG) += bonito.o
 common-obj-$(CONFIG_PCI_PIIX) += piix.o
diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
new file mode 100644
index 0000000..ef0273b
--- /dev/null
+++ b/hw/pci-host/igd.c
@@ -0,0 +1,96 @@
+#include "qemu-common.h"
+#include "hw/pci/pci.h"
+#include "hw/i386/pc.h"
+
+/* IGD Passthrough Host Bridge. */
+typedef struct {
+    uint8_t offset;
+    uint8_t len;
+} IGDHostInfo;
+
+/* Here we just expose minimal host bridge offset subset. */
+static const IGDHostInfo igd_host_bridge_infos[] = {
+    {0x08, 2},  /* revision id */
+    {0x2c, 2},  /* sybsystem vendor id */
+    {0x2e, 2},  /* sybsystem id */
+    {0x50, 2},  /* SNB: processor graphics control register */
+    {0x52, 2},  /* processor graphics control register */
+    {0xa4, 4},  /* SNB: graphics base of stolen memory */
+    {0xa8, 4},  /* SNB: base of GTT stolen memory */
+};
+
+static int host_pci_config_read(int pos, int len, uint32_t val)
+{
+    char path[PATH_MAX];
+    int config_fd;
+    ssize_t size = sizeof(path);
+    /* Access real host bridge. */
+    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
+                      0, 0, 0, 0, "config");
+    int ret = 0;
+
+    if (rc >= size || rc < 0) {
+        return -ENODEV;
+    }
+
+    config_fd = open(path, O_RDWR);
+    if (config_fd < 0) {
+        return -ENODEV;
+    }
+
+    if (lseek(config_fd, pos, SEEK_SET) != pos) {
+        ret = -errno;
+        goto out;
+    }
+    do {
+        rc = read(config_fd, (uint8_t *)&val, len);
+    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
+    if (rc != len) {
+        ret = -errno;
+    }
+out:
+    close(config_fd);
+    return ret;
+}
+
+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+{
+    uint32_t val = 0;
+    int rc, i, num;
+    int pos, len;
+
+    num = ARRAY_SIZE(igd_host_bridge_infos);
+    for (i = 0; i < num; i++) {
+        pos = igd_host_bridge_infos[i].offset;
+        len = igd_host_bridge_infos[i].len;
+        rc = host_pci_config_read(pos, len, val);
+        if (rc) {
+            return -ENODEV;
+        }
+        pci_default_write_config(pci_dev, pos, val, len);
+    }
+
+    return 0;
+}
+
+static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = igd_pt_i440fx_initfn;
+    dc->desc = "IGD Passthrough Host bridge";
+}
+
+static const TypeInfo igd_passthrough_i440fx_info = {
+    .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_I440FX_PCI_DEVICE,
+    .class_init    = igd_passthrough_i440fx_class_init,
+};
+
+static void igd_register_types(void)
+{
+    type_register_static(&igd_passthrough_i440fx_info);
+}
+
+type_init(igd_register_types)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 715208b..ccacb57 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -744,93 +744,6 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
-/* IGD Passthrough Host Bridge. */
-typedef struct {
-    uint8_t offset;
-    uint8_t len;
-} IGDHostInfo;
-
-/* Here we just expose minimal host bridge offset subset. */
-static const IGDHostInfo igd_host_bridge_infos[] = {
-    {0x08, 2},  /* revision id */
-    {0x2c, 2},  /* sybsystem vendor id */
-    {0x2e, 2},  /* sybsystem id */
-    {0x50, 2},  /* SNB: processor graphics control register */
-    {0x52, 2},  /* processor graphics control register */
-    {0xa4, 4},  /* SNB: graphics base of stolen memory */
-    {0xa8, 4},  /* SNB: base of GTT stolen memory */
-};
-
-static int host_pci_config_read(int pos, int len, uint32_t val)
-{
-    char path[PATH_MAX];
-    int config_fd;
-    ssize_t size = sizeof(path);
-    /* Access real host bridge. */
-    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
-                      0, 0, 0, 0, "config");
-    int ret = 0;
-
-    if (rc >= size || rc < 0) {
-        return -ENODEV;
-    }
-
-    config_fd = open(path, O_RDWR);
-    if (config_fd < 0) {
-        return -ENODEV;
-    }
-
-    if (lseek(config_fd, pos, SEEK_SET) != pos) {
-        ret = -errno;
-        goto out;
-    }
-    do {
-        rc = read(config_fd, (uint8_t *)&val, len);
-    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
-    if (rc != len) {
-        ret = -errno;
-    }
-out:
-    close(config_fd);
-    return ret;
-}
-
-static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
-{
-    uint32_t val = 0;
-    int rc, i, num;
-    int pos, len;
-
-    num = ARRAY_SIZE(igd_host_bridge_infos);
-    for (i = 0; i < num; i++) {
-        pos = igd_host_bridge_infos[i].offset;
-        len = igd_host_bridge_infos[i].len;
-        rc = host_pci_config_read(pos, len, val);
-        if (rc) {
-            return -ENODEV;
-        }
-        pci_default_write_config(pci_dev, pos, val, len);
-    }
-
-    return 0;
-}
-
-static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->init = igd_pt_i440fx_initfn;
-    dc->desc = "IGD Passthrough Host bridge";
-}
-
-static const TypeInfo igd_passthrough_i440fx_info = {
-    .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
-    .parent        = TYPE_I440FX_PCI_DEVICE,
-    .instance_size = sizeof(PCII440FXState),
-    .class_init    = igd_passthrough_i440fx_class_init,
-};
-
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
@@ -872,7 +785,6 @@ static const TypeInfo i440fx_pcihost_info = {
 static void i440fx_register_types(void)
 {
     type_register_static(&i440fx_info);
-    type_register_static(&igd_passthrough_i440fx_info);
     type_register_static(&piix3_pci_type_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/7] pc: move igd support code to igd.c

[Stefano Stabellini]

On Tue, 8 Dec 2015, Gerd Hoffmann wrote:
> Pure code motion, except for dropping instance_size for
> TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE (no need to set,
> we can inherit it from TYPE_I440FX_PCI_DEVICE).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  hw/pci-host/Makefile.objs |  3 ++
>  hw/pci-host/igd.c         | 96 +++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pci-host/piix.c        | 88 -------------------------------------------
>  3 files changed, 99 insertions(+), 88 deletions(-)
>  create mode 100644 hw/pci-host/igd.c
> 
> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
> index 45f1f0e..e341a49 100644
> --- a/hw/pci-host/Makefile.objs
> +++ b/hw/pci-host/Makefile.objs
> @@ -11,6 +11,9 @@ common-obj-$(CONFIG_PPCE500_PCI) += ppce500.o
>  # ARM devices
>  common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o
>  
> +# igd passthrough support
> +common-obj-$(CONFIG_LINUX) += igd.o
> +
>  common-obj-$(CONFIG_PCI_APB) += apb.o
>  common-obj-$(CONFIG_FULONG) += bonito.o
>  common-obj-$(CONFIG_PCI_PIIX) += piix.o
> diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
> new file mode 100644
> index 0000000..ef0273b
> --- /dev/null
> +++ b/hw/pci-host/igd.c
> @@ -0,0 +1,96 @@
> +#include "qemu-common.h"
> +#include "hw/pci/pci.h"
> +#include "hw/i386/pc.h"
> +
> +/* IGD Passthrough Host Bridge. */
> +typedef struct {
> +    uint8_t offset;
> +    uint8_t len;
> +} IGDHostInfo;
> +
> +/* Here we just expose minimal host bridge offset subset. */
> +static const IGDHostInfo igd_host_bridge_infos[] = {
> +    {0x08, 2},  /* revision id */
> +    {0x2c, 2},  /* sybsystem vendor id */
> +    {0x2e, 2},  /* sybsystem id */
> +    {0x50, 2},  /* SNB: processor graphics control register */
> +    {0x52, 2},  /* processor graphics control register */
> +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> +    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> +};
> +
> +static int host_pci_config_read(int pos, int len, uint32_t val)
> +{
> +    char path[PATH_MAX];
> +    int config_fd;
> +    ssize_t size = sizeof(path);
> +    /* Access real host bridge. */
> +    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
> +                      0, 0, 0, 0, "config");
> +    int ret = 0;
> +
> +    if (rc >= size || rc < 0) {
> +        return -ENODEV;
> +    }
> +
> +    config_fd = open(path, O_RDWR);
> +    if (config_fd < 0) {
> +        return -ENODEV;
> +    }
> +
> +    if (lseek(config_fd, pos, SEEK_SET) != pos) {
> +        ret = -errno;
> +        goto out;
> +    }
> +    do {
> +        rc = read(config_fd, (uint8_t *)&val, len);
> +    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> +    if (rc != len) {
> +        ret = -errno;
> +    }
> +out:
> +    close(config_fd);
> +    return ret;
> +}
> +
> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> +{
> +    uint32_t val = 0;
> +    int rc, i, num;
> +    int pos, len;
> +
> +    num = ARRAY_SIZE(igd_host_bridge_infos);
> +    for (i = 0; i < num; i++) {
> +        pos = igd_host_bridge_infos[i].offset;
> +        len = igd_host_bridge_infos[i].len;
> +        rc = host_pci_config_read(pos, len, val);
> +        if (rc) {
> +            return -ENODEV;
> +        }
> +        pci_default_write_config(pci_dev, pos, val, len);
> +    }
> +
> +    return 0;
> +}
> +
> +static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = igd_pt_i440fx_initfn;
> +    dc->desc = "IGD Passthrough Host bridge";
> +}
> +
> +static const TypeInfo igd_passthrough_i440fx_info = {
> +    .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> +    .parent        = TYPE_I440FX_PCI_DEVICE,
> +    .class_init    = igd_passthrough_i440fx_class_init,
> +};
> +
> +static void igd_register_types(void)
> +{
> +    type_register_static(&igd_passthrough_i440fx_info);
> +}
> +
> +type_init(igd_register_types)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 715208b..ccacb57 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -744,93 +744,6 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
>  
> -/* IGD Passthrough Host Bridge. */
> -typedef struct {
> -    uint8_t offset;
> -    uint8_t len;
> -} IGDHostInfo;
> -
> -/* Here we just expose minimal host bridge offset subset. */
> -static const IGDHostInfo igd_host_bridge_infos[] = {
> -    {0x08, 2},  /* revision id */
> -    {0x2c, 2},  /* sybsystem vendor id */
> -    {0x2e, 2},  /* sybsystem id */
> -    {0x50, 2},  /* SNB: processor graphics control register */
> -    {0x52, 2},  /* processor graphics control register */
> -    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> -    {0xa8, 4},  /* SNB: base of GTT stolen memory */
> -};
> -
> -static int host_pci_config_read(int pos, int len, uint32_t val)
> -{
> -    char path[PATH_MAX];
> -    int config_fd;
> -    ssize_t size = sizeof(path);
> -    /* Access real host bridge. */
> -    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
> -                      0, 0, 0, 0, "config");
> -    int ret = 0;
> -
> -    if (rc >= size || rc < 0) {
> -        return -ENODEV;
> -    }
> -
> -    config_fd = open(path, O_RDWR);
> -    if (config_fd < 0) {
> -        return -ENODEV;
> -    }
> -
> -    if (lseek(config_fd, pos, SEEK_SET) != pos) {
> -        ret = -errno;
> -        goto out;
> -    }
> -    do {
> -        rc = read(config_fd, (uint8_t *)&val, len);
> -    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> -    if (rc != len) {
> -        ret = -errno;
> -    }
> -out:
> -    close(config_fd);
> -    return ret;
> -}
> -
> -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> -{
> -    uint32_t val = 0;
> -    int rc, i, num;
> -    int pos, len;
> -
> -    num = ARRAY_SIZE(igd_host_bridge_infos);
> -    for (i = 0; i < num; i++) {
> -        pos = igd_host_bridge_infos[i].offset;
> -        len = igd_host_bridge_infos[i].len;
> -        rc = host_pci_config_read(pos, len, val);
> -        if (rc) {
> -            return -ENODEV;
> -        }
> -        pci_default_write_config(pci_dev, pos, val, len);
> -    }
> -
> -    return 0;
> -}
> -
> -static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -
> -    k->init = igd_pt_i440fx_initfn;
> -    dc->desc = "IGD Passthrough Host bridge";
> -}
> -
> -static const TypeInfo igd_passthrough_i440fx_info = {
> -    .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> -    .parent        = TYPE_I440FX_PCI_DEVICE,
> -    .instance_size = sizeof(PCII440FXState),
> -    .class_init    = igd_passthrough_i440fx_class_init,
> -};
> -
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -872,7 +785,6 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> -    type_register_static(&igd_passthrough_i440fx_info);
>      type_register_static(&piix3_pci_type_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
> -- 
> 1.8.3.1
> 
> 

[Qemu-devel] [PATCH 3/7] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index ef0273b..d1eeafb 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -53,7 +53,7 @@ out:
     return ret;
 }
 
-static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
 {
     uint32_t val = 0;
     int rc, i, num;
@@ -65,12 +65,11 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
         len = igd_host_bridge_infos[i].len;
         rc = host_pci_config_read(pos, len, val);
         if (rc) {
-            return -ENODEV;
+            error_setg(errp, "failed to read host config");
+            return;
         }
         pci_default_write_config(pci_dev, pos, val, len);
     }
-
-    return 0;
 }
 
 static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
@@ -78,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = igd_pt_i440fx_initfn;
+    k->realize = igd_pt_i440fx_realize;
     dc->desc = "IGD Passthrough Host bridge";
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index d1eeafb..6f52ab1 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -53,12 +53,20 @@ out:
     return ret;
 }
 
+static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp);
 static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
 {
+    Error *err = NULL;
     uint32_t val = 0;
     int rc, i, num;
     int pos, len;
 
+    i440fx_realize(pci_dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
     num = ARRAY_SIZE(igd_host_bridge_infos);
     for (i = 0; i < num; i++) {
         pos = igd_host_bridge_infos[i].offset;
@@ -77,6 +85,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+    i440fx_realize = k->realize;
     k->realize = igd_pt_i440fx_realize;
     dc->desc = "IGD Passthrough Host bridge";
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] igd: use defines for standard pci config space offsets

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index 6f52ab1..0784128 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -10,9 +10,9 @@ typedef struct {
 
 /* Here we just expose minimal host bridge offset subset. */
 static const IGDHostInfo igd_host_bridge_infos[] = {
-    {0x08, 2},  /* revision id */
-    {0x2c, 2},  /* sybsystem vendor id */
-    {0x2e, 2},  /* sybsystem id */
+    {PCI_REVISION_ID,         2},
+    {PCI_SUBSYSTEM_VENDOR_ID, 2},
+    {PCI_SUBSYSTEM_ID,        2},
     {0x50, 2},  /* SNB: processor graphics control register */
     {0x52, 2},  /* processor graphics control register */
     {0xa4, 4},  /* SNB: graphics base of stolen memory */
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] igd: revamp host config read

[Gerd Hoffmann]

Move all work to the host_pci_config_copy helper function,
which we can easily reuse when adding q35 support.
Open sysfs file only once for all values.  Use pread.
Proper error handling.  Fix bugs:

 * Don't throw away results (like old host_pci_config_read
   did because val was passed by value not reference).
 * Update config space directly (writing via
   pci_default_write_config only works for registers
   whitelisted in wmask).

Hmm, this code can hardly ever worked before,
/me wonders what test coverage it had.

With this patch in place igd-passthru=on actually
works, although it still requires root priviledges
because linux refuses to allow non-root users access
pci config space above offset 0x50.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c | 65 +++++++++++++++++++++++--------------------------------
 1 file changed, 27 insertions(+), 38 deletions(-)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index 0784128..ec48875 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -19,47 +19,39 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
     {0xa8, 4},  /* SNB: base of GTT stolen memory */
 };
 
-static int host_pci_config_read(int pos, int len, uint32_t val)
+static void host_pci_config_copy(PCIDevice *guest, const char *host,
+                                 const IGDHostInfo *list, int len, Error **errp)
 {
-    char path[PATH_MAX];
-    int config_fd;
-    ssize_t size = sizeof(path);
-    /* Access real host bridge. */
-    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
-                      0, 0, 0, 0, "config");
-    int ret = 0;
+    char *path;
+    int config_fd, rc, i;
 
-    if (rc >= size || rc < 0) {
-        return -ENODEV;
-    }
-
-    config_fd = open(path, O_RDWR);
+    path = g_strdup_printf("/sys/bus/pci/devices/%s/config", host);
+    config_fd = open(path, O_RDONLY);
     if (config_fd < 0) {
-        return -ENODEV;
+        error_setg_file_open(errp, errno, path);
+        goto out_free;
     }
 
-    if (lseek(config_fd, pos, SEEK_SET) != pos) {
-        ret = -errno;
-        goto out;
+    for (i = 0; i < len; i++) {
+        rc = pread(config_fd, guest->config + list[i].offset,
+                   list[i].len, list[i].offset);
+        if (rc != list[i].len) {
+            error_setg_errno(errp, errno, "read %s, offset 0x%x",
+                             path, list[i].offset);
+            goto out_close;
+        }
     }
-    do {
-        rc = read(config_fd, (uint8_t *)&val, len);
-    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
-    if (rc != len) {
-        ret = -errno;
-    }
-out:
+
+out_close:
     close(config_fd);
-    return ret;
+out_free:
+    g_free(path);
 }
 
 static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp);
 static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
 {
     Error *err = NULL;
-    uint32_t val = 0;
-    int rc, i, num;
-    int pos, len;
 
     i440fx_realize(pci_dev, &err);
     if (err != NULL) {
@@ -67,16 +59,13 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    num = ARRAY_SIZE(igd_host_bridge_infos);
-    for (i = 0; i < num; i++) {
-        pos = igd_host_bridge_infos[i].offset;
-        len = igd_host_bridge_infos[i].len;
-        rc = host_pci_config_read(pos, len, val);
-        if (rc) {
-            error_setg(errp, "failed to read host config");
-            return;
-        }
-        pci_default_write_config(pci_dev, pos, val, len);
+    host_pci_config_copy(pci_dev, "0000:00:00.0",
+                         igd_host_bridge_infos,
+                         ARRAY_SIZE(igd_host_bridge_infos),
+                         &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
     }
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [iGVT-g] [PATCH 6/7] igd: revamp host config read

[Jike Song]

On 12/08/2015 10:07 PM, Gerd Hoffmann wrote:
> Move all work to the host_pci_config_copy helper function,
> which we can easily reuse when adding q35 support.
> Open sysfs file only once for all values.  Use pread.
> Proper error handling.  Fix bugs:
>
>   * Don't throw away results (like old host_pci_config_read
>     did because val was passed by value not reference).
>   * Update config space directly (writing via
>     pci_default_write_config only works for registers
>     whitelisted in wmask).
>
> Hmm, this code can hardly ever worked before,
> /me wonders what test coverage it had.
>

WOW, that's really impressive! :)

--
Thanks,
Jike

> With this patch in place igd-passthru=on actually
> works, although it still requires root priviledges
> because linux refuses to allow non-root users access
> pci config space above offset 0x50.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/pci-host/igd.c | 65 +++++++++++++++++++++++--------------------------------
>   1 file changed, 27 insertions(+), 38 deletions(-)
>
> diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
> index 0784128..ec48875 100644
> --- a/hw/pci-host/igd.c
> +++ b/hw/pci-host/igd.c
> @@ -19,47 +19,39 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>       {0xa8, 4},  /* SNB: base of GTT stolen memory */
>   };
>
> -static int host_pci_config_read(int pos, int len, uint32_t val)
> +static void host_pci_config_copy(PCIDevice *guest, const char *host,
> +                                 const IGDHostInfo *list, int len, Error **errp)
>   {
> -    char path[PATH_MAX];
> -    int config_fd;
> -    ssize_t size = sizeof(path);
> -    /* Access real host bridge. */
> -    int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
> -                      0, 0, 0, 0, "config");
> -    int ret = 0;
> +    char *path;
> +    int config_fd, rc, i;
>
> -    if (rc >= size || rc < 0) {
> -        return -ENODEV;
> -    }
> -
> -    config_fd = open(path, O_RDWR);
> +    path = g_strdup_printf("/sys/bus/pci/devices/%s/config", host);
> +    config_fd = open(path, O_RDONLY);
>       if (config_fd < 0) {
> -        return -ENODEV;
> +        error_setg_file_open(errp, errno, path);
> +        goto out_free;
>       }
>
> -    if (lseek(config_fd, pos, SEEK_SET) != pos) {
> -        ret = -errno;
> -        goto out;
> +    for (i = 0; i < len; i++) {
> +        rc = pread(config_fd, guest->config + list[i].offset,
> +                   list[i].len, list[i].offset);
> +        if (rc != list[i].len) {
> +            error_setg_errno(errp, errno, "read %s, offset 0x%x",
> +                             path, list[i].offset);
> +            goto out_close;
> +        }
>       }
> -    do {
> -        rc = read(config_fd, (uint8_t *)&val, len);
> -    } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> -    if (rc != len) {
> -        ret = -errno;
> -    }
> -out:
> +
> +out_close:
>       close(config_fd);
> -    return ret;
> +out_free:
> +    g_free(path);
>   }
>
>   static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp);
>   static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
>   {
>       Error *err = NULL;
> -    uint32_t val = 0;
> -    int rc, i, num;
> -    int pos, len;
>
>       i440fx_realize(pci_dev, &err);
>       if (err != NULL) {
> @@ -67,16 +59,13 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
>           return;
>       }
>
> -    num = ARRAY_SIZE(igd_host_bridge_infos);
> -    for (i = 0; i < num; i++) {
> -        pos = igd_host_bridge_infos[i].offset;
> -        len = igd_host_bridge_infos[i].len;
> -        rc = host_pci_config_read(pos, len, val);
> -        if (rc) {
> -            error_setg(errp, "failed to read host config");
> -            return;
> -        }
> -        pci_default_write_config(pci_dev, pos, val, len);
> +    host_pci_config_copy(pci_dev, "0000:00:00.0",
> +                         igd_host_bridge_infos,
> +                         ARRAY_SIZE(igd_host_bridge_infos),
> +                         &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
>       }
>   }
>
>

[Qemu-devel] [PATCH 7/7] igd: add q35 support

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 hw/pci-host/q35.c |  6 +++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index ec48875..f6e3f7a 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -1,5 +1,6 @@
 #include "qemu-common.h"
 #include "hw/pci/pci.h"
+#include "hw/pci-host/q35.h"
 #include "hw/i386/pc.h"
 
 /* IGD Passthrough Host Bridge. */
@@ -76,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
 
     i440fx_realize = k->realize;
     k->realize = igd_pt_i440fx_realize;
-    dc->desc = "IGD Passthrough Host bridge";
+    dc->desc = "IGD Passthrough Host bridge (i440fx)";
 }
 
 static const TypeInfo igd_passthrough_i440fx_info = {
@@ -85,9 +86,47 @@ static const TypeInfo igd_passthrough_i440fx_info = {
     .class_init    = igd_passthrough_i440fx_class_init,
 };
 
+static void (*q35_realize)(PCIDevice *pci_dev, Error **errp);
+static void igd_pt_q35_realize(PCIDevice *pci_dev, Error **errp)
+{
+    Error *err = NULL;
+
+    q35_realize(pci_dev, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    host_pci_config_copy(pci_dev, "0000:00:00.0",
+                         igd_host_bridge_infos,
+                         ARRAY_SIZE(igd_host_bridge_infos),
+                         &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+}
+
+static void igd_passthrough_q35_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    q35_realize = k->realize;
+    k->realize = igd_pt_q35_realize;
+    dc->desc = "IGD Passthrough Host bridge (q35)";
+}
+
+static const TypeInfo igd_passthrough_q35_info = {
+    .name          = "igd-passthrough-q35-mch",
+    .parent        = TYPE_MCH_PCI_DEVICE,
+    .class_init    = igd_passthrough_q35_class_init,
+};
+
 static void igd_register_types(void)
 {
     type_register_static(&igd_passthrough_i440fx_info);
+    type_register_static(&igd_passthrough_q35_info);
 }
 
 type_init(igd_register_types)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 1fb4707..07dc595 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -151,7 +151,11 @@ static void q35_host_initfn(Object *obj)
     memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb,
                           "pci-conf-data", 4);
 
-    object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE);
+    if (object_property_get_bool(qdev_get_machine(), "igd-passthru", NULL)) {
+        object_initialize(&s->mch, sizeof(s->mch), "igd-passthrough-q35-mch");
+    } else {
+        object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE);
+    }
     object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL);
     qdev_prop_set_uint32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0));
     qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 7/7] igd: add q35 support

[Michael S. Tsirkin]

On Tue, Dec 08, 2015 at 03:07:28PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Generally are you merging this through your tree?


> ---
>  hw/pci-host/igd.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  hw/pci-host/q35.c |  6 +++++-
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
> index ec48875..f6e3f7a 100644
> --- a/hw/pci-host/igd.c
> +++ b/hw/pci-host/igd.c
> @@ -1,5 +1,6 @@
>  #include "qemu-common.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci-host/q35.h"
>  #include "hw/i386/pc.h"
>  
>  /* IGD Passthrough Host Bridge. */
> @@ -76,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>  
>      i440fx_realize = k->realize;
>      k->realize = igd_pt_i440fx_realize;
> -    dc->desc = "IGD Passthrough Host bridge";
> +    dc->desc = "IGD Passthrough Host bridge (i440fx)";
>  }
>  
>  static const TypeInfo igd_passthrough_i440fx_info = {
> @@ -85,9 +86,47 @@ static const TypeInfo igd_passthrough_i440fx_info = {
>      .class_init    = igd_passthrough_i440fx_class_init,
>  };
>  
> +static void (*q35_realize)(PCIDevice *pci_dev, Error **errp);
> +static void igd_pt_q35_realize(PCIDevice *pci_dev, Error **errp)
> +{
> +    Error *err = NULL;
> +
> +    q35_realize(pci_dev, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    host_pci_config_copy(pci_dev, "0000:00:00.0",
> +                         igd_host_bridge_infos,
> +                         ARRAY_SIZE(igd_host_bridge_infos),
> +                         &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +}
> +
> +static void igd_passthrough_q35_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    q35_realize = k->realize;
> +    k->realize = igd_pt_q35_realize;
> +    dc->desc = "IGD Passthrough Host bridge (q35)";
> +}
> +
> +static const TypeInfo igd_passthrough_q35_info = {
> +    .name          = "igd-passthrough-q35-mch",
> +    .parent        = TYPE_MCH_PCI_DEVICE,
> +    .class_init    = igd_passthrough_q35_class_init,
> +};
> +
>  static void igd_register_types(void)
>  {
>      type_register_static(&igd_passthrough_i440fx_info);
> +    type_register_static(&igd_passthrough_q35_info);
>  }
>  
>  type_init(igd_register_types)
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 1fb4707..07dc595 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -151,7 +151,11 @@ static void q35_host_initfn(Object *obj)
>      memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb,
>                            "pci-conf-data", 4);
>  
> -    object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE);
> +    if (object_property_get_bool(qdev_get_machine(), "igd-passthru", NULL)) {
> +        object_initialize(&s->mch, sizeof(s->mch), "igd-passthrough-q35-mch");
> +    } else {
> +        object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE);
> +    }
>      object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL);
>      qdev_prop_set_uint32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0));
>      qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH 7/7] igd: add q35 support

[Gerd Hoffmann]

On Do, 2016-02-04 at 17:22 +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 08, 2015 at 03:07:28PM +0100, Gerd Hoffmann wrote:
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Generally are you merging this through your tree?

First this needs more testing, then we can figure which tree to pick to
merge this.  I can do a pull myself of course ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/7] igd passthrough chipset tweaks

[Stefano Stabellini]

On Tue, 8 Dec 2015, Gerd Hoffmann wrote:
>   Hi,
> 
> We have some code in our tree to support pci passthrough of intel
> graphics devices (igd) on xen, which requires some chipset tweaks
> for (a) the host bridge and (b) the lpc/isa-bridge to meat the
> expectations of the guest driver.  For kvm we need pretty much
> the same, also the requirements for vgpu (xengt/kvmgt) are very
> simliar.
> 
> This patch series tackles (a) only, (b) will follow later.  It
> wires up the igd-passthru machine option for tcg/kvm too, moves
> the code to its own file so it is nicely separated, fixes a bunch
> of issues and finally adds q35 support.
> 
> This patch series has seen very light testing, basically doing
> lspci in the guest to check whenever pci config space got updated
> correctly.  Trying actual device assignment needs more pieces
> being in place.  But I suspect even that is more testing than
> the code has seen on xen so far (see patch #6 ...).

I for one don't have a setup to be able to test this at the moment. But
I would appreciate if this kind of changes were Tested-by Tiejun Chen.


> Gerd Hoffmann (7):
>   pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
>   pc: move igd support code to igd.c
>   igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
>   igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
>   igd: use defines for standard pci config space offsets
>   igd: revamp host config read
>   igd: add q35 support
> 
>  hw/i386/pc_piix.c         |  11 ++--
>  hw/pci-host/Makefile.objs |   3 ++
>  hw/pci-host/igd.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pci-host/piix.c        |  88 -------------------------------
>  hw/pci-host/q35.c         |   6 ++-
>  5 files changed, 145 insertions(+), 95 deletions(-)
>  create mode 100644 hw/pci-host/igd.c
> 
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 0/7] igd passthrough chipset tweaks

[Stefano Stabellini]

Actually CC'ing xen-devel

On Fri, 11 Dec 2015, Stefano Stabellini wrote:
> On Tue, 8 Dec 2015, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > We have some code in our tree to support pci passthrough of intel
> > graphics devices (igd) on xen, which requires some chipset tweaks
> > for (a) the host bridge and (b) the lpc/isa-bridge to meat the
> > expectations of the guest driver.  For kvm we need pretty much
> > the same, also the requirements for vgpu (xengt/kvmgt) are very
> > simliar.
> > 
> > This patch series tackles (a) only, (b) will follow later.  It
> > wires up the igd-passthru machine option for tcg/kvm too, moves
> > the code to its own file so it is nicely separated, fixes a bunch
> > of issues and finally adds q35 support.
> > 
> > This patch series has seen very light testing, basically doing
> > lspci in the guest to check whenever pci config space got updated
> > correctly.  Trying actual device assignment needs more pieces
> > being in place.  But I suspect even that is more testing than
> > the code has seen on xen so far (see patch #6 ...).
> 
> I for one don't have a setup to be able to test this at the moment. But
> I would appreciate if this kind of changes were Tested-by Tiejun Chen.
> 
> 
> > Gerd Hoffmann (7):
> >   pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
> >   pc: move igd support code to igd.c
> >   igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
> >   igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
> >   igd: use defines for standard pci config space offsets
> >   igd: revamp host config read
> >   igd: add q35 support
> > 
> >  hw/i386/pc_piix.c         |  11 ++--
> >  hw/pci-host/Makefile.objs |   3 ++
> >  hw/pci-host/igd.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/pci-host/piix.c        |  88 -------------------------------
> >  hw/pci-host/q35.c         |   6 ++-
> >  5 files changed, 145 insertions(+), 95 deletions(-)
> >  create mode 100644 hw/pci-host/igd.c
> > 
> > -- 
> > 1.8.3.1
> > 
> > 
> 

Re: [PATCH 0/7] igd passthrough chipset tweaks

[Stefano Stabellini]

Actually CC'ing xen-devel

On Fri, 11 Dec 2015, Stefano Stabellini wrote:
> On Tue, 8 Dec 2015, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > We have some code in our tree to support pci passthrough of intel
> > graphics devices (igd) on xen, which requires some chipset tweaks
> > for (a) the host bridge and (b) the lpc/isa-bridge to meat the
> > expectations of the guest driver.  For kvm we need pretty much
> > the same, also the requirements for vgpu (xengt/kvmgt) are very
> > simliar.
> > 
> > This patch series tackles (a) only, (b) will follow later.  It
> > wires up the igd-passthru machine option for tcg/kvm too, moves
> > the code to its own file so it is nicely separated, fixes a bunch
> > of issues and finally adds q35 support.
> > 
> > This patch series has seen very light testing, basically doing
> > lspci in the guest to check whenever pci config space got updated
> > correctly.  Trying actual device assignment needs more pieces
> > being in place.  But I suspect even that is more testing than
> > the code has seen on xen so far (see patch #6 ...).
> 
> I for one don't have a setup to be able to test this at the moment. But
> I would appreciate if this kind of changes were Tested-by Tiejun Chen.
> 
> 
> > Gerd Hoffmann (7):
> >   pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
> >   pc: move igd support code to igd.c
> >   igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
> >   igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
> >   igd: use defines for standard pci config space offsets
> >   igd: revamp host config read
> >   igd: add q35 support
> > 
> >  hw/i386/pc_piix.c         |  11 ++--
> >  hw/pci-host/Makefile.objs |   3 ++
> >  hw/pci-host/igd.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/pci-host/piix.c        |  88 -------------------------------
> >  hw/pci-host/q35.c         |   6 ++-
> >  5 files changed, 145 insertions(+), 95 deletions(-)
> >  create mode 100644 hw/pci-host/igd.c
> > 
> > -- 
> > 1.8.3.1
> > 
> > 
>