[Qemu-devel] [v6][PATCH 00/10] xen: add Intel IGD passthrough support

[Qemu-devel] [v6][PATCH 00/10] xen: add Intel IGD passthrough support

[Tiejun Chen]

Sorry for this so long delay to address this series since other things
occupied my most time.

Michael,

You carried some patches previously on your branch for a while, 
    i440fx: make types configurable at run-time
    pc_init1: pass parameters just with types
    xen:hw:pci-host:piix: create host bridge to passthrough
    qom-test: blacklist xenigd
    xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

But now V6 just needs fist two and I also include them again here. So
please drop others, and thanks a lot.

v6:

* Drop introducing a new machine specific to IGD passthrough
* Try to share some codes from KVM stuff in qemu to retrive VGA BIOS
* Currently IGD drivers always need to access PCH by 1f.0. But we
  don't want to poke that directly to get ID, and although in real
  world different GPU should have different PCH. But actually the
  different PCH DIDs likely map to different PCH SKUs. We do the
  same thing for the GPU. For PCH, the different SKUs are going to
  be all the same silicon design and implementation, just different
  features turn on and off with fuses. The SW interfaces should be
  consistent across all SKUs in a given family (eg LPT). But just
  same features may not be supported.
 
  Most of these different PCH features probably don't matter to the
  Gfx driver, but obviously any difference in display port connections
  will so it should be fine with any PCH in case of passthrough.
 
  So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
  scenarios, 0x9cc3 for BDW(Broadwell).
* Drop igd write ops since its fine to emulate that, and we also shrink
  those igd read ops as necessary.
* Rebase and cleanup all patches.

v5:

* Simplify to make sure its really inherited from the standard one in patch #3
* Then drop the original patch #3

v4:

* Rebase on latest tree
* Drop patch #2
* Regenerate patches after Michael introduce patch #1
* We need to use this pci_type as a index to reuse I440FX_PCI_DEVICE()
* Test: boot with a preinstalled winxp
  ./i386-softmmu/qemu-system-i386 -hda winxp-32.img -m 2560 -boot c -machine pc

v3:

* Drop patch #4
* Add one patch #1 from Michael
* Rebase
* In./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

v2:

* Fix some coding style
* New patch to separate i440fx_init
* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
* Based on patch #2 to regenerate
* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3
* Test: boot with a preinstalled ubuntu 14.04
  ./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

As we discussed we need to create a separate machine to support current
IGD passthrough.

----------------------------------------------------------------
Michael S. Tsirkin (1):
      i440fx: make types configurable at run-time
 
Tiejun Chen (9):
      pc_init1: pass parameters just with types
      piix: create host bridge to passthrough
      hw/pci-assign: split pci-assign.c
      xen, gfx passthrough: basic graphics passthrough support
      xen, gfx passthrough: retrieve VGA BIOS to work
      xen, gfx passthrough: register a isa bridge
      xen, gfx passthrough: support Intel IGD passthrough with VT-D
      xen, gfx passthrough: register host bridge specific to passthrough
      xen, gfx passthrough: add opregion mapping
 
 hw/i386/Makefile.objs         |   1 +
 hw/i386/kvm/pci-assign.c      |  82 +++----------------------------
 hw/i386/pc_piix.c             |  36 +++++++++++---
 hw/i386/pci-assign-load-rom.c |  93 +++++++++++++++++++++++++++++++++++
 hw/pci-host/piix.c            |  28 +++++++++--
 hw/xen/Makefile.objs          |   1 +
 hw/xen/xen-host-pci-device.c  |   5 ++
 hw/xen/xen-host-pci-device.h  |   1 +
 hw/xen/xen_pt.c               | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/xen/xen_pt.h               |  22 ++++++++-
 hw/xen/xen_pt_config_init.c   |  52 +++++++++++++++++++-
 hw/xen/xen_pt_graphics.c      | 344 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h          |   8 ++-
 include/hw/pci/pci-assign.h   |  27 +++++++++++
 qemu-options.hx               |   9 ++++
 vl.c                          |  10 ++++
 16 files changed, 767 insertions(+), 92 deletions(-)
 create mode 100644 hw/i386/pci-assign-load-rom.c
 create mode 100644 hw/xen/xen_pt_graphics.c
 create mode 100644 include/hw/pci/pci-assign.h

Thanks
Tiejun

[Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Tiejun Chen]

From: "Michael S. Tsirkin" <mst@redhat.com>

Xen wants to supply a different pci and host devices,
inheriting i440fx devices. Make types configurable.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c    | 4 +++-
 hw/pci-host/piix.c   | 9 ++++-----
 include/hw/i386/pc.h | 6 +++++-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f0a3201..cc10f72 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -200,7 +200,9 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
+        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
+                              TYPE_I440FX_PCI_DEVICE,
+                              &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
                               above_4g_mem_size,
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1530038..adc5025 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,6 @@
  * http://download.intel.com/design/chipsets/datashts/29054901.pdf
  */
 
-#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define I440FX_PCI_HOST_BRIDGE(obj) \
     OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
 
@@ -91,7 +90,6 @@ typedef struct PIIX3State {
     MemoryRegion rcr_mem;
 } PIIX3State;
 
-#define TYPE_I440FX_PCI_DEVICE "i440FX"
 #define I440FX_PCI_DEVICE(obj) \
     OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
 
@@ -305,7 +303,8 @@ static int i440fx_initfn(PCIDevice *dev)
     return 0;
 }
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state,
                     int *piix3_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
                     MemoryRegion *address_space_mem,
@@ -325,7 +324,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
     unsigned i;
     I440FXState *i440fx;
 
-    dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+    dev = qdev_create(NULL, host_type);
     s = PCI_HOST_BRIDGE(dev);
     b = pci_bus_new(dev, NULL, pci_address_space,
                     address_space_io, 0, TYPE_PCI_BUS);
@@ -333,7 +332,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
     object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
-    d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+    d = pci_create_simple(b, 0, pci_type);
     *pi440fx_state = I440FX_PCI_DEVICE(d);
     f = *pi440fx_state;
     f->system_memory = address_space_mem;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 69d9cf8..755d6a7 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -244,7 +244,11 @@ extern int no_hpet;
 struct PCII440FXState;
 typedef struct PCII440FXState PCII440FXState;
 
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
+#define TYPE_I440FX_PCI_DEVICE "i440FX"
+
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
-- 
1.9.1

Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Gerd Hoffmann]

On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> 
> Xen wants to supply a different pci and host devices,
> inheriting i440fx devices. Make types configurable.

Description is misleading, this isn't about xen but about IGD
passthrough.  Guess kvm needs this too once kvmgt lands upstream.

Otherwise looks sane to me.

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Chen, Tiejun]

On 2015/1/19 19:36, Gerd Hoffmann wrote:
> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>
>> Xen wants to supply a different pci and host devices,
>> inheriting i440fx devices. Make types configurable.
>
> Description is misleading, this isn't about xen but about IGD

Its about IGD passthrough in Xen side.

> passthrough.  Guess kvm needs this too once kvmgt lands upstream.
>

But I'm not sure if KvmGT is really going to be this so just CCed Jike 
who are focusing on KvmGT currently, and he also promised to have a 
closer loot at this.

Thanks
Tiejun

> Otherwise looks sane to me.
>
> cheers,
>    Gerd
>
>
>

Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Gerd Hoffmann]

On Di, 2015-01-20 at 10:48 +0800, Chen, Tiejun wrote:
> On 2015/1/19 19:36, Gerd Hoffmann wrote:
> > On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> >> From: "Michael S. Tsirkin" <mst@redhat.com>
> >>
> >> Xen wants to supply a different pci and host devices,
> >> inheriting i440fx devices. Make types configurable.
> >
> > Description is misleading, this isn't about xen but about IGD
> 
> Its about IGD passthrough in Xen side.

As far I can see the only really xen specific stuff here is the pci
pass-through bits, i.e. how we handle the IGD device itself.

The northbridge & isa-bridge emulation extensions needed for IGD should
be pretty much common for IGD passthough on xen, IGD passthrough on kvm
(vfio) and IGD virtualization (xengt+kvmgt).

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Jike Song]

On 01/20/2015 04:25 PM, Gerd Hoffmann wrote:
> On Di, 2015-01-20 at 10:48 +0800, Chen, Tiejun wrote:
>> On 2015/1/19 19:36, Gerd Hoffmann wrote:
>>> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>>>
>>>> Xen wants to supply a different pci and host devices,
>>>> inheriting i440fx devices. Make types configurable.
>>>
>>> Description is misleading, this isn't about xen but about IGD
>>
>> Its about IGD passthrough in Xen side.
>
> As far I can see the only really xen specific stuff here is the pci
> pass-through bits, i.e. how we handle the IGD device itself.
>
> The northbridge & isa-bridge emulation extensions needed for IGD should
> be pretty much common for IGD passthough on xen, IGD passthrough on kvm
> (vfio) and IGD virtualization (xengt+kvmgt).

This is exactly what I proposed in another thread :)

>
> cheers,
>    Gerd
>

--
Thanks,
Jike

Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time

[Chen, Tiejun]

On 2015/1/19 19:36, Gerd Hoffmann wrote:
> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>
>> Xen wants to supply a different pci and host devices,
>> inheriting i440fx devices. Make types configurable.
>
> Description is misleading, this isn't about xen but about IGD
> passthrough.  Guess kvm needs this too once kvmgt lands upstream.
>

So just rephrase this as follows:

i440fx: make types configurable at run-time

IGD passthrough wants to supply a different pci and
host devices, inheriting i440fx devices. Make types
configurable.

Thanks
Tiejun

[Qemu-devel] [v6][PATCH 02/10] pc_init1: pass parameters just with types

[Tiejun Chen]

Pass types to configure pc_init1().

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cc10f72..4148028 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -74,7 +74,9 @@ static bool has_reserved_memory = true;
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      int pci_enabled,
-                     int kvmclock_enabled)
+                     int kvmclock_enabled,
+                     const char *host_type,
+                     const char *pci_type)
 {
     PCMachineState *pc_machine = PC_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
@@ -200,8 +202,8 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
-                              TYPE_I440FX_PCI_DEVICE,
+        pci_bus = i440fx_init(host_type,
+                              pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
@@ -307,7 +309,8 @@ static void pc_init1(MachineState *machine,
 
 static void pc_init_pci(MachineState *machine)
 {
-    pc_init1(machine, 1, 1);
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_compat_2_2(MachineState *machine)
@@ -470,7 +473,8 @@ static void pc_init_pci_1_2(MachineState *machine)
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
     pc_compat_1_2(machine);
-    pc_init1(machine, 1, 0);
+    pc_init1(machine, 1, 0, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_init_isa(MachineState *machine)
@@ -487,7 +491,8 @@ static void pc_init_isa(MachineState *machine)
     }
     x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
-    pc_init1(machine, 0, 1);
+    pc_init1(machine, 0, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 #ifdef CONFIG_XEN
-- 
1.9.1

[Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough

[Tiejun Chen]

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c   | 16 ++++++++++++++++
 include/hw/i386/pc.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index adc5025..1468961 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -729,6 +729,21 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
+                                                  void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "IGD PT XEN Host bridge";
+}
+
+static const TypeInfo xen_igd_passthrough_i440fx_info = {
+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_I440FX_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = xen_igd_passthrough_i440fx_class_init,
+};
+
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
@@ -770,6 +785,7 @@ static const TypeInfo i440fx_pcihost_info = {
 static void i440fx_register_types(void)
 {
     type_register_static(&i440fx_info);
+    type_register_static(&xen_igd_passthrough_i440fx_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
     type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 755d6a7..07b3afc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -247,6 +247,8 @@ typedef struct PCII440FXState PCII440FXState;
 #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define TYPE_I440FX_PCI_DEVICE "i440FX"
 
+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

Re: [Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough

[Gerd Hoffmann]

On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
> +                                                  void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc = "IGD PT XEN Host bridge";
> +}

IMO "xen" naming should go away here too.

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough

[Chen, Tiejun]

On 2015/1/19 19:40, Gerd Hoffmann wrote:
> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
>> +                                                  void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->desc = "IGD PT XEN Host bridge";
>> +}
>
> IMO "xen" naming should go away here too.
>

Its easy to do but we need to wait KvmGT guys' response, so now it makes 
sense to leave "xen" as a prefix since this just work in xen side.

Thanks
Tiejun

Re: [Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough

[Jike Song]

On 01/20/2015 10:52 AM, Chen, Tiejun wrote:
> On 2015/1/19 19:40, Gerd Hoffmann wrote:
>> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
>>> +                                                  void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->desc = "IGD PT XEN Host bridge";
>>> +}
>>
>> IMO "xen" naming should go away here too.
>>

I would agree with this.

In fact, this piece of code could possibly be used by:

  a) IGD passthru for Xen and KVM, and/or:
  b) IGD Mediated passthru for Xen and KVM, i.e. XenGT/KVMGT

So it looks better if have "xen" naming purged :)

>
> Its easy to do but we need to wait KvmGT guys' response, so now it makes
> sense to leave "xen" as a prefix since this just work in xen side.
>
> Thanks
> Tiejun
>

--
Thanks,
Jike

Re: [Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough

[Chen, Tiejun]

On 2015/1/20 12:28, Jike Song wrote:
> On 01/20/2015 10:52 AM, Chen, Tiejun wrote:
>> On 2015/1/19 19:40, Gerd Hoffmann wrote:
>>> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
>>>> +                                                  void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +
>>>> +    dc->desc = "IGD PT XEN Host bridge";
>>>> +}
>>>
>>> IMO "xen" naming should go away here too.
>>>
>
> I would agree with this.
>
> In fact, this piece of code could possibly be used by:
>
>   a) IGD passthru for Xen and KVM, and/or:
>   b) IGD Mediated passthru for Xen and KVM, i.e. XenGT/KVMGT
>
> So it looks better if have "xen" naming purged :)

Okay, I'll do this in next revision.

Thanks
Tiejun

>
>>
>> Its easy to do but we need to wait KvmGT guys' response, so now it makes
>> sense to leave "xen" as a prefix since this just work in xen side.
>>
>> Thanks
>> Tiejun
>>
>
> --
> Thanks,
> Jike
>
>

[Qemu-devel] [v6][PATCH 04/10] hw/pci-assign: split pci-assign.c

[Tiejun Chen]

We will try to reuse assign_dev_load_option_rom in xen side, and
especially its a good beginning to unify pci assign codes both on
kvm and xen in the future.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/Makefile.objs         |  1 +
 hw/i386/kvm/pci-assign.c      | 82 ++++----------------------------------
 hw/i386/pci-assign-load-rom.c | 93 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci-assign.h   | 27 +++++++++++++
 4 files changed, 128 insertions(+), 75 deletions(-)
 create mode 100644 hw/i386/pci-assign-load-rom.c
 create mode 100644 include/hw/pci/pci-assign.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 9d419ad..195afc0 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -8,6 +8,7 @@ obj-$(CONFIG_XEN) += ../xenpv/ xen/
 obj-y += kvmvapic.o
 obj-y += acpi-build.o
 obj-y += bios-linker-loader.o
+obj-y += pci-assign-load-rom.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
 	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
 	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index bb206da..2e715b2 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -37,6 +37,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
 #include "kvm_i386.h"
+#include "hw/pci/pci-assign.h"
 
 #define MSIX_PAGE_SIZE 0x1000
 
@@ -48,17 +49,6 @@
 #define IORESOURCE_PREFETCH 0x00002000  /* No side effects */
 #define IORESOURCE_MEM_64   0x00100000
 
-//#define DEVICE_ASSIGNMENT_DEBUG
-
-#ifdef DEVICE_ASSIGNMENT_DEBUG
-#define DEBUG(fmt, ...)                                       \
-    do {                                                      \
-        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
-    } while (0)
-#else
-#define DEBUG(fmt, ...)
-#endif
-
 typedef struct PCIRegion {
     int type;           /* Memory or port I/O */
     int valid;
@@ -1899,73 +1889,15 @@ static void assign_register_types(void)
 
 type_init(assign_register_types)
 
-/*
- * Scan the assigned devices for the devices that have an option ROM, and then
- * load the corresponding ROM data to RAM. If an error occurs while loading an
- * option ROM, we just ignore that option ROM and continue with the next one.
- */
 static void assigned_dev_load_option_rom(AssignedDevice *dev)
 {
-    char name[32], rom_file[64];
-    FILE *fp;
-    uint8_t val;
-    struct stat st;
-    void *ptr;
-
-    /* If loading ROM from file, pci handles it */
-    if (dev->dev.romfile || !dev->dev.rom_bar) {
-        return;
-    }
+    int size = 0;
 
-    snprintf(rom_file, sizeof(rom_file),
-             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
-             dev->host.domain, dev->host.bus, dev->host.slot,
-             dev->host.function);
+    pci_assign_dev_load_option_rom(&dev->dev, OBJECT(dev), &size,
+                                   dev->host.domain, dev->host.bus,
+                                   dev->host.slot, dev->host.function);
 
-    if (stat(rom_file, &st)) {
-        return;
-    }
-
-    if (access(rom_file, F_OK)) {
-        error_report("pci-assign: Insufficient privileges for %s", rom_file);
-        return;
-    }
-
-    /* Write "1" to the ROM file to enable it */
-    fp = fopen(rom_file, "r+");
-    if (fp == NULL) {
-        return;
+    if (!size) {
+        error_report("pci-assign: Invalid ROM.");
     }
-    val = 1;
-    if (fwrite(&val, 1, 1, fp) != 1) {
-        goto close_rom;
-    }
-    fseek(fp, 0, SEEK_SET);
-
-    snprintf(name, sizeof(name), "%s.rom",
-            object_get_typename(OBJECT(dev)));
-    memory_region_init_ram(&dev->dev.rom, OBJECT(dev), name, st.st_size,
-                           &error_abort);
-    vmstate_register_ram(&dev->dev.rom, &dev->dev.qdev);
-    ptr = memory_region_get_ram_ptr(&dev->dev.rom);
-    memset(ptr, 0xff, st.st_size);
-
-    if (!fread(ptr, 1, st.st_size, fp)) {
-        error_report("pci-assign: Cannot read from host %s", rom_file);
-        error_printf("Device option ROM contents are probably invalid "
-                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
-                     "or load from file with romfile=\n");
-        goto close_rom;
-    }
-
-    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
-    dev->dev.has_rom = true;
-close_rom:
-    /* Write "0" to disable ROM */
-    fseek(fp, 0, SEEK_SET);
-    val = 0;
-    if (!fwrite(&val, 1, 1, fp)) {
-        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
-    }
-    fclose(fp);
 }
diff --git a/hw/i386/pci-assign-load-rom.c b/hw/i386/pci-assign-load-rom.c
new file mode 100644
index 0000000..a04b540
--- /dev/null
+++ b/hw/i386/pci-assign-load-rom.c
@@ -0,0 +1,93 @@
+/*
+ * This is splited from hw/i386/kvm/pci-assign.c
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/io.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include "hw/hw.h"
+#include "hw/i386/pc.h"
+#include "qemu/error-report.h"
+#include "ui/console.h"
+#include "hw/loader.h"
+#include "monitor/monitor.h"
+#include "qemu/range.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci-assign.h"
+
+/*
+ * Scan the assigned devices for the devices that have an option ROM, and then
+ * load the corresponding ROM data to RAM. If an error occurs while loading an
+ * option ROM, we just ignore that option ROM and continue with the next one.
+ */
+void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
+                                     int *size, unsigned int domain,
+                                     unsigned int bus, unsigned int slot,
+                                     unsigned int function)
+{
+    char name[32], rom_file[64];
+    FILE *fp;
+    uint8_t val;
+    struct stat st;
+    void *ptr;
+
+    /* If loading ROM from file, pci handles it */
+    if (dev->romfile || !dev->rom_bar) {
+        return NULL;
+    }
+
+    snprintf(rom_file, sizeof(rom_file),
+             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
+             domain, bus, slot, function);
+
+    if (stat(rom_file, &st)) {
+        return NULL;
+    }
+
+    if (access(rom_file, F_OK)) {
+        error_report("pci-assign: Insufficient privileges for %s", rom_file);
+        return NULL;
+    }
+
+    /* Write "1" to the ROM file to enable it */
+    fp = fopen(rom_file, "r+");
+    if (fp == NULL) {
+        return NULL;
+    }
+    val = 1;
+    if (fwrite(&val, 1, 1, fp) != 1) {
+        goto close_rom;
+    }
+    fseek(fp, 0, SEEK_SET);
+
+    snprintf(name, sizeof(name), "%s.rom", object_get_typename(owner));
+    memory_region_init_ram(&dev->rom, owner, name, st.st_size, &error_abort);
+    vmstate_register_ram(&dev->rom, &dev->qdev);
+    ptr = memory_region_get_ram_ptr(&dev->rom);
+    memset(ptr, 0xff, st.st_size);
+
+    if (!fread(ptr, 1, st.st_size, fp)) {
+        error_report("pci-assign: Cannot read from host %s", rom_file);
+        error_printf("Device option ROM contents are probably invalid "
+                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
+                     "or load from file with romfile=\n");
+        goto close_rom;
+    }
+
+    pci_register_bar(dev, PCI_ROM_SLOT, 0, &dev->rom);
+    dev->has_rom = true;
+    *size = st.st_size;
+close_rom:
+    /* Write "0" to disable ROM */
+    fseek(fp, 0, SEEK_SET);
+    val = 0;
+    if (!fwrite(&val, 1, 1, fp)) {
+        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
+    }
+    fclose(fp);
+
+    return ptr;
+}
diff --git a/include/hw/pci/pci-assign.h b/include/hw/pci/pci-assign.h
new file mode 100644
index 0000000..55f42c5
--- /dev/null
+++ b/include/hw/pci/pci-assign.h
@@ -0,0 +1,27 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Just split from hw/i386/kvm/pci-assign.c.
+ */
+#ifndef PCI_ASSIGN_H
+#define PCI_ASSIGN_H
+
+#include "hw/pci/pci.h"
+
+//#define DEVICE_ASSIGNMENT_DEBUG
+
+#ifdef DEVICE_ASSIGNMENT_DEBUG
+#define DEBUG(fmt, ...)                                       \
+    do {                                                      \
+        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
+    } while (0)
+#else
+#define DEBUG(fmt, ...)
+#endif
+
+void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
+                                     int *size, unsigned int domain,
+                                     unsigned int bus, unsigned int slot,
+                                     unsigned int function);
+#endif /* PCI_ASSIGN_H */
-- 
1.9.1

[Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support

[Tiejun Chen]

basic gfx passthrough support:
- add a vga type for gfx passthrough
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/xen/Makefile.objs         |   1 +
 hw/xen/xen-host-pci-device.c |   5 ++
 hw/xen/xen-host-pci-device.h |   1 +
 hw/xen/xen_pt.c              |   4 ++
 hw/xen/xen_pt.h              |  10 +++-
 hw/xen/xen_pt_graphics.c     | 111 +++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx              |   9 ++++
 vl.c                         |  10 ++++
 8 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 hw/xen/xen_pt_graphics.c

diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index a0ca0aa..a9ad7e7 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -3,3 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
 
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
+obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o xen_pt_graphics.o
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 743b37b..a54b7de 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
         goto error;
     }
     d->irq = v;
+    rc = xen_host_pci_get_hex_value(d, "class", &v);
+    if (rc) {
+        goto error;
+    }
+    d->class_code = v;
     d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
 
     return 0;
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index c2486f0..f1e1c30 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
 
     uint16_t vendor_id;
     uint16_t device_id;
+    uint32_t class_code;
     int irq;
 
     XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index f2893b2..1d78021 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
                    d->rom.size, d->rom.base_addr);
     }
 
+    xen_pt_register_vga_regions(d);
     return 0;
 }
 
@@ -748,6 +749,7 @@ out:
 static void xen_pt_unregister_device(PCIDevice *d)
 {
     XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, d);
+    XenHostPCIDevice *host_dev = &s->real_device;
     uint8_t machine_irq = s->machine_irq;
     uint8_t intx = xen_pt_pci_intx(s);
     int rc;
@@ -791,6 +793,8 @@ static void xen_pt_unregister_device(PCIDevice *d)
     /* delete all emulated config registers */
     xen_pt_config_delete(s);
 
+    xen_pt_unregister_vga_regions(host_dev);
+
     memory_listener_unregister(&s->memory_listener);
     memory_listener_unregister(&s->io_listener);
 
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 942dc60..509a447 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,5 +298,13 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
     return s->msix && s->msix->bar_index == bar;
 }
 
-
+extern int xen_has_gfx_passthru;
+static inline int is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+    return (xen_has_gfx_passthru
+            && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev);
 #endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
new file mode 100644
index 0000000..9b3df81
--- /dev/null
+++ b/hw/xen/xen_pt_graphics.c
@@ -0,0 +1,111 @@
+/*
+ * graphics passthrough
+ */
+#include "xen_pt.h"
+#include "xen-host-pci-device.h"
+#include "hw/xen/xen_backend.h"
+
+typedef struct VGARegion {
+    int type;           /* Memory or port I/O */
+    uint64_t guest_base_addr;
+    uint64_t machine_base_addr;
+    uint64_t size;    /* size of the region */
+    int rc;
+} VGARegion;
+
+#define IORESOURCE_IO           0x00000100
+#define IORESOURCE_MEM          0x00000200
+
+static struct VGARegion vga_args[] = {
+    {
+        .type = IORESOURCE_IO,
+        .guest_base_addr = 0x3B0,
+        .machine_base_addr = 0x3B0,
+        .size = 0xC,
+        .rc = -1,
+    },
+    {
+        .type = IORESOURCE_IO,
+        .guest_base_addr = 0x3C0,
+        .machine_base_addr = 0x3C0,
+        .size = 0x20,
+        .rc = -1,
+    },
+    {
+        .type = IORESOURCE_MEM,
+        .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
+        .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
+        .size = 0x20,
+        .rc = -1,
+    },
+};
+
+/*
+ * register VGA resources for the domain with assigned gfx
+ */
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
+{
+    int i = 0;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return 0;
+    }
+
+    for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
+        if (vga_args[i].type == IORESOURCE_IO) {
+            vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
+                            vga_args[i].guest_base_addr,
+                            vga_args[i].machine_base_addr,
+                            vga_args[i].size, DPCI_ADD_MAPPING);
+        } else {
+            vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
+                            vga_args[i].guest_base_addr,
+                            vga_args[i].machine_base_addr,
+                            vga_args[i].size, DPCI_ADD_MAPPING);
+        }
+
+        if (vga_args[i].rc) {
+            XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
+                    vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
+                    vga_args[i].rc);
+            return vga_args[i].rc;
+        }
+    }
+
+    return 0;
+}
+
+/*
+ * unregister VGA resources for the domain with assigned gfx
+ */
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
+{
+    int i = 0;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return 0;
+    }
+
+    for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
+        if (vga_args[i].type == IORESOURCE_IO) {
+            vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
+                            vga_args[i].guest_base_addr,
+                            vga_args[i].machine_base_addr,
+                            vga_args[i].size, DPCI_REMOVE_MAPPING);
+        } else {
+            vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
+                            vga_args[i].guest_base_addr,
+                            vga_args[i].machine_base_addr,
+                            vga_args[i].size, DPCI_REMOVE_MAPPING);
+        }
+
+        if (vga_args[i].rc) {
+            XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
+                    vga_args[i].type == IORESOURCE_IO ? "ioport" : "memory",
+                    vga_args[i].rc);
+            return vga_args[i].rc;
+        }
+    }
+
+    return 0;
+}
diff --git a/qemu-options.hx b/qemu-options.hx
index 10b9568..bb13f69 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1075,6 +1075,15 @@ STEXI
 Rotate graphical output some deg left (only PXA LCD).
 ETEXI
 
+DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
+    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -gfx_passthru
+@findex -gfx_passthru
+Enable Intel IGD passthrough by XEN
+ETEXI
+
 DEF("vga", HAS_ARG, QEMU_OPTION_vga,
     "-vga [std|cirrus|vmware|qxl|xenfb|tcx|cg3|none]\n"
     "                select video card type\n", QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index fbf4240..42fb3c8 100644
--- a/vl.c
+++ b/vl.c
@@ -1222,6 +1222,13 @@ static void configure_msg(QemuOpts *opts)
     enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
 }
 
+/* We still need this for compatibility with XEN. */
+int xen_has_gfx_passthru;
+static void xen_gfx_passthru(const char *optarg)
+{
+    xen_has_gfx_passthru = 1;
+}
+
 /***********************************************************/
 /* USB devices */
 
@@ -3749,6 +3756,9 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_gfx_passthru:
+                xen_gfx_passthru(optarg);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
-- 
1.9.1

Re: [Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support

[Gerd Hoffmann]

On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
> +    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -gfx_passthru
> +@findex -gfx_passthru
> +Enable Intel IGD passthrough by XEN
> +ETEXI

Make that a machine option, i.e. "-machine pc,igd-passthru=on"?

Again, most of this applies to kvmgt too, so I think the "xen" naming
should go away here.

While talking about machine types: what about q35?

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support

[Chen, Tiejun]

On 2015/1/19 19:45, Gerd Hoffmann wrote:
> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
>> +    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> +@item -gfx_passthru
>> +@findex -gfx_passthru
>> +Enable Intel IGD passthrough by XEN
>> +ETEXI
>
> Make that a machine option, i.e. "-machine pc,igd-passthru=on"?

Yeah but I think we need "-machine xenfv,igd-passthru=on" here.

Thanks
Tiejun

Re: [Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support

[Gerd Hoffmann]

On Di, 2015-01-20 at 11:14 +0800, Chen, Tiejun wrote:
> On 2015/1/19 19:45, Gerd Hoffmann wrote:
> > On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> >> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
> >> +    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
> >> +    QEMU_ARCH_ALL)
> >> +STEXI
> >> +@item -gfx_passthru
> >> +@findex -gfx_passthru
> >> +Enable Intel IGD passthrough by XEN
> >> +ETEXI
> >
> > Make that a machine option, i.e. "-machine pc,igd-passthru=on"?
> 
> Yeah but I think we need "-machine xenfv,igd-passthru=on" here.

IIRC xen decided to stop using xenfv and use pc-$version instead (with
$version being what was current at release time, 1.6 for xen 4.4 I
think, to avoid surprises like the address space layout changes in more
recent machine types).

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support

[Chen, Tiejun]

On 2015/1/20 16:14, Gerd Hoffmann wrote:
> On Di, 2015-01-20 at 11:14 +0800, Chen, Tiejun wrote:
>> On 2015/1/19 19:45, Gerd Hoffmann wrote:
>>> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>>>> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
>>>> +    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
>>>> +    QEMU_ARCH_ALL)
>>>> +STEXI
>>>> +@item -gfx_passthru
>>>> +@findex -gfx_passthru
>>>> +Enable Intel IGD passthrough by XEN
>>>> +ETEXI
>>>
>>> Make that a machine option, i.e. "-machine pc,igd-passthru=on"?
>>
>> Yeah but I think we need "-machine xenfv,igd-passthru=on" here.
>
> IIRC xen decided to stop using xenfv and use pc-$version instead (with
> $version being what was current at release time, 1.6 for xen 4.4 I
> think, to avoid surprises like the address space layout changes in more
> recent machine types).
>

To be more exact, 'xen_platform_pci' should determine this at this point,

         if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
             /* Switching here to the machine "pc" which does not add
              * the xen-platform device instead of the default "xenfv" 
machine.
              */
             machinearg = libxl__sprintf(gc, "pc,accel=xen");
         } else {
             machinearg = libxl__sprintf(gc, "xenfv");
         }

But you may remember, in our case we always set 'xen_platform_pci=0' 
since we need to release slot 2 for IGD. So finally we really go pc case.

Anyway this means something should be changed to pass such a new machine 
property in Xen side. And I'll send a patch to address this firstly, 
then go back here.

Thanks
Tiejun

[Qemu-devel] [v6][PATCH 06/10] xen, gfx passthrough: retrieve VGA BIOS to work

[Tiejun Chen]

Now we retrieve VGA bios like kvm stuff in qemu but we need to
fix Device Identification in case if its not matched with the
real IGD device since Seabios is always trying to compare this
ID to work out VGA BIOS.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/xen/xen_pt.c          | 10 ++++++
 hw/xen/xen_pt.h          |  5 +++
 hw/xen/xen_pt_graphics.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 1d78021..fcc9f1c 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -672,6 +672,16 @@ static int xen_pt_initfn(PCIDevice *d)
     s->memory_listener = xen_pt_memory_listener;
     s->io_listener = xen_pt_io_listener;
 
+    /* Setup VGA bios for passthrough GFX */
+    if ((s->real_device.domain == 0) && (s->real_device.bus == 0) &&
+        (s->real_device.dev == 2) && (s->real_device.func == 0)) {
+        if (xen_pt_setup_vga(s, &s->real_device) < 0) {
+            XEN_PT_ERR(d, "Setup VGA BIOS of passthrough GFX failed!\n");
+            xen_host_pci_device_put(&s->real_device);
+            return -1;
+        }
+    }
+
     /* Handle real device's MMIO/PIO BARs */
     xen_pt_register_regions(s);
 
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 509a447..0aa5a93 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,6 +298,11 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
     return s->msix && s->msix->bar_index == bar;
 }
 
+extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
+                                            struct Object *owner, int *size,
+                                            unsigned int domain,
+                                            unsigned int bus, unsigned int slot,
+                                            unsigned int function);
 extern int xen_has_gfx_passthru;
 static inline int is_igd_vga_passthrough(XenHostPCIDevice *dev)
 {
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 9b3df81..3232296 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -109,3 +109,82 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
 
     return 0;
 }
+
+static void *get_vgabios(XenPCIPassthroughState *s, int *size,
+                       XenHostPCIDevice *dev)
+{
+    return pci_assign_dev_load_option_rom(&s->dev, OBJECT(&s->dev), size,
+                                          dev->domain, dev->bus,
+                                          dev->dev, dev->func);
+}
+
+/* Refer to Seabios. */
+struct rom_header {
+    uint16_t signature;
+    uint8_t size;
+    uint8_t initVector[4];
+    uint8_t reserved[17];
+    uint16_t pcioffset;
+    uint16_t pnpoffset;
+} __attribute__((packed));
+
+struct pci_data {
+    uint32_t signature;
+    uint16_t vendor;
+    uint16_t device;
+    uint16_t vitaldata;
+    uint16_t dlen;
+    uint8_t drevision;
+    uint8_t class_lo;
+    uint16_t class_hi;
+    uint16_t ilen;
+    uint16_t irevision;
+    uint8_t type;
+    uint8_t indicator;
+    uint16_t reserved;
+} __attribute__((packed));
+
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
+{
+    unsigned char *bios = NULL;
+    struct rom_header *rom;
+    int bios_size;
+    char *c = NULL;
+    char checksum = 0;
+    uint32_t len = 0;
+    struct pci_data *pd = NULL;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return -1;
+    }
+
+    bios = get_vgabios(s, &bios_size, dev);
+    if (!bios) {
+        XEN_PT_ERR(&s->dev, "VGA: Can't getting VBIOS!\n");
+        return -1;
+    }
+
+    /* Currently we fixed this address as a primary. */
+    rom = (struct rom_header *)bios;
+    pd = (void *)(bios + (unsigned char)rom->pcioffset);
+
+    /* We may need to fixup Device Identification. */
+    if (pd->device != s->real_device.device_id) {
+        pd->device = s->real_device.device_id;
+
+        len = rom->size * 512;
+        /* Then adjust the bios checksum */
+        for (c = (char *)bios; c < ((char *)bios + len); c++) {
+            checksum += *c;
+        }
+        if (checksum) {
+            bios[len - 1] -= checksum;
+            XEN_PT_LOG(&s->dev, "vga bios checksum is adjusted %x!\n",
+                       checksum);
+        }
+    }
+
+    /* Currently we fixed this address as a primary for legacy BIOS. */
+    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
+    return 0;
+}
-- 
1.9.1

[Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Tiejun Chen]

Currently IGD drivers always need to access PCH by 1f.0. But we
don't want to poke that directly to get ID, and although in real
world different GPU should have different PCH. But actually the
different PCH DIDs likely map to different PCH SKUs. We do the
same thing for the GPU. For PCH, the different SKUs are going to
be all the same silicon design and implementation, just different
features turn on and off with fuses. The SW interfaces should be
consistent across all SKUs in a given family (eg LPT). But just
same features may not be supported.

Most of these different PCH features probably don't matter to the
Gfx driver, but obviously any difference in display port connections
will so it should be fine with any PCH in case of passthrough.

So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
scenarios, 0x9cc3 for BDW(Broadwell).

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/xen/xen_pt.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index fcc9f1c..5532d6f 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
     .priority = 10,
 };
 
+typedef struct {
+    uint16_t gpu_device_id;
+    uint16_t pch_device_id;
+    uint8_t pch_revision_id;
+} XenIGDDeviceIDInfo;
+
+/* In real world different GPU should have different PCH. But actually
+ * the different PCH DIDs likely map to different PCH SKUs. We do the
+ * same thing for the GPU. For PCH, the different SKUs are going to be
+ * all the same silicon design and implementation, just different
+ * features turn on and off with fuses. The SW interfaces should be
+ * consistent across all SKUs in a given family (eg LPT). But just same
+ * features may not be supported.
+ *
+ * Most of these different PCH features probably don't matter to the
+ * Gfx driver, but obviously any difference in display port connections
+ * will so it should be fine with any PCH in case of passthrough.
+ *
+ * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
+ * scenarios, 0x9cc3 for BDW(Broadwell).
+ */
+static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
+    /* HSW Classic */
+    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
+    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
+    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
+    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
+    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
+    /* HSW ULT */
+    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
+    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
+    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
+    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
+    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
+    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
+    /* HSW CRW */
+    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
+    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
+    /* HSW Server */
+    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
+    /* HSW SRVR */
+    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
+    /* BSW */
+    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
+    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
+    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
+    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
+    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
+    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
+    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
+    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
+    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
+    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
+    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    dc->desc        = "ISA bridge faked to support IGD PT";
+    k->vendor_id    = PCI_VENDOR_ID_INTEL;
+    k->class_id     = PCI_CLASS_BRIDGE_ISA;
+};
+
+static TypeInfo isa_bridge_info = {
+    .name          = "xen-igd-passthrough-isa-bridge",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIDevice),
+    .class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+    type_register_static(&isa_bridge_info);
+}
+type_init(xen_pt_graphics_register_types)
+
+static void
+xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+                                      XenHostPCIDevice *dev)
+{
+    struct PCIDevice *pci_dev;
+    int i, num;
+    uint16_t gpu_dev_id, pch_dev_id = 0xffff;
+    uint8_t pch_rev_id;
+    PCIDevice *d = &s->dev;
+
+    if (!is_igd_vga_passthrough(dev)) {
+        return;
+    }
+
+    gpu_dev_id = dev->device_id;
+    num = ARRAY_SIZE(xen_igd_combo_id_infos);
+    for (i = 0; i < num; i++) {
+        if (gpu_dev_id == xen_igd_combo_id_infos[i].gpu_device_id) {
+            pch_dev_id = xen_igd_combo_id_infos[i].pch_device_id;
+            pch_rev_id = xen_igd_combo_id_infos[i].pch_revision_id;
+        }
+    }
+
+    if (pch_dev_id == 0xffff) {
+        fprintf(stderr, "unsupported PCH!\n");
+        return;
+    }
+
+    /* Currently IGD drivers always need to access PCH by 1f.0. */
+    pci_dev = pci_create_simple(d->bus, PCI_DEVFN(0x1f, 0),
+                                "xen-igd-passthrough-isa-bridge");
+
+    /*
+     * Identify PCH card with its own real vendor/device ids.
+     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
+     */
+    if (!pci_dev) {
+        fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed!\n");
+        return;
+    }
+    pci_config_set_device_id(pci_dev->config, pch_dev_id);
+    pci_config_set_revision(pci_dev->config, pch_rev_id);
+}
+
 /* init */
 
 static int xen_pt_initfn(PCIDevice *d)
@@ -680,6 +803,9 @@ static int xen_pt_initfn(PCIDevice *d)
             xen_host_pci_device_put(&s->real_device);
             return -1;
         }
+
+        /* Register ISA bridge for passthrough GFX. */
+        xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
     }
 
     /* Handle real device's MMIO/PIO BARs */
-- 
1.9.1

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Gerd Hoffmann]

On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> Currently IGD drivers always need to access PCH by 1f.0. But we

Obvious question: q35?

q35 already has a isa bridge @ 0x1f.0.  Guess that needs to be extended
for the pass-through then (simliar to the host bridge) instead of adding
a dummy bridge.

Also: again xen naming here.

cheers,
  Gerd

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 12:57:18PM +0100, Gerd Hoffmann wrote:
> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
> > Currently IGD drivers always need to access PCH by 1f.0. But we
> 
> Obvious question: q35?
> 
> q35 already has a isa bridge @ 0x1f.0.  Guess that needs to be extended
> for the pass-through then (simliar to the host bridge) instead of adding
> a dummy bridge.
> 
> Also: again xen naming here.
> 
> cheers,
>   Gerd

This hack probably doesn't have a chance to work well for q35
due to the above conflict.
Happily future cards won't need it.

-- 
MST

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Chen, Tiejun]

On 2015/1/19 21:58, Michael S. Tsirkin wrote:
> On Mon, Jan 19, 2015 at 12:57:18PM +0100, Gerd Hoffmann wrote:
>> On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
>>> Currently IGD drivers always need to access PCH by 1f.0. But we
>>
>> Obvious question: q35?
>>
>> q35 already has a isa bridge @ 0x1f.0.  Guess that needs to be extended
>> for the pass-through then (simliar to the host bridge) instead of adding
>> a dummy bridge.
>>
>> Also: again xen naming here.
>>
>> cheers,
>>    Gerd
>
> This hack probably doesn't have a chance to work well for q35
> due to the above conflict.

Yeah, and I remember as we discussed previously, something else also 
conflict with Xen and Windows currently so Xen doesn't intend to go Q35.

Thanks
Tiejun

> Happily future cards won't need it.
>

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 05:28:40PM +0800, Tiejun Chen wrote:
> Currently IGD drivers always need to access PCH by 1f.0. But we
> don't want to poke that directly to get ID, and although in real
> world different GPU should have different PCH. But actually the
> different PCH DIDs likely map to different PCH SKUs. We do the
> same thing for the GPU. For PCH, the different SKUs are going to
> be all the same silicon design and implementation, just different
> features turn on and off with fuses. The SW interfaces should be
> consistent across all SKUs in a given family (eg LPT). But just
> same features may not be supported.
> 
> Most of these different PCH features probably don't matter to the
> Gfx driver, but obviously any difference in display port connections
> will so it should be fine with any PCH in case of passthrough.
> 
> So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
> scenarios, 0x9cc3 for BDW(Broadwell).
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/xen/xen_pt.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index fcc9f1c..5532d6f 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
>      .priority = 10,
>  };
>  
> +typedef struct {
> +    uint16_t gpu_device_id;
> +    uint16_t pch_device_id;
> +    uint8_t pch_revision_id;
> +} XenIGDDeviceIDInfo;
> +
> +/* In real world different GPU should have different PCH. But actually
> + * the different PCH DIDs likely map to different PCH SKUs. We do the
> + * same thing for the GPU. For PCH, the different SKUs are going to be
> + * all the same silicon design and implementation, just different
> + * features turn on and off with fuses. The SW interfaces should be
> + * consistent across all SKUs in a given family (eg LPT). But just same
> + * features may not be supported.
> + *
> + * Most of these different PCH features probably don't matter to the
> + * Gfx driver, but obviously any difference in display port connections
> + * will so it should be fine with any PCH in case of passthrough.
> + *
> + * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
> + * scenarios, 0x9cc3 for BDW(Broadwell).
> + */
> +static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
> +    /* HSW Classic */
> +    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
> +    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
> +    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
> +    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
> +    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
> +    /* HSW ULT */
> +    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
> +    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
> +    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
> +    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
> +    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
> +    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
> +    /* HSW CRW */
> +    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
> +    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
> +    /* HSW Server */
> +    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
> +    /* HSW SRVR */
> +    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
> +    /* BSW */
> +    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
> +    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
> +    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
> +    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
> +    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
> +    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
> +    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
> +    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
> +    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
> +    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
> +    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
> +};
> +
> +static void isa_bridge_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    dc->desc        = "ISA bridge faked to support IGD PT";
> +    k->vendor_id    = PCI_VENDOR_ID_INTEL;
> +    k->class_id     = PCI_CLASS_BRIDGE_ISA;
> +};
> +
> +static TypeInfo isa_bridge_info = {
> +    .name          = "xen-igd-passthrough-isa-bridge",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCIDevice),
> +    .class_init = isa_bridge_class_init,
> +};
> +
> +static void xen_pt_graphics_register_types(void)
> +{
> +    type_register_static(&isa_bridge_info);
> +}
> +type_init(xen_pt_graphics_register_types)
> +
> +static void
> +xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> +                                      XenHostPCIDevice *dev)
> +{
> +    struct PCIDevice *pci_dev;
> +    int i, num;
> +    uint16_t gpu_dev_id, pch_dev_id = 0xffff;
> +    uint8_t pch_rev_id;
> +    PCIDevice *d = &s->dev;
> +
> +    if (!is_igd_vga_passthrough(dev)) {
> +        return;
> +    }
> +
> +    gpu_dev_id = dev->device_id;
> +    num = ARRAY_SIZE(xen_igd_combo_id_infos);
> +    for (i = 0; i < num; i++) {
> +        if (gpu_dev_id == xen_igd_combo_id_infos[i].gpu_device_id) {
> +            pch_dev_id = xen_igd_combo_id_infos[i].pch_device_id;
> +            pch_rev_id = xen_igd_combo_id_infos[i].pch_revision_id;
> +        }
> +    }
> +
> +    if (pch_dev_id == 0xffff) {
> +        fprintf(stderr, "unsupported PCH!\n");

I would drop this fprintf: this likely means a newer
card, so the bridge is not necessary.

> +        return;
> +    }
> +
> +    /* Currently IGD drivers always need to access PCH by 1f.0. */
> +    pci_dev = pci_create_simple(d->bus, PCI_DEVFN(0x1f, 0),
> +                                "xen-igd-passthrough-isa-bridge");
> +
> +    /*
> +     * Identify PCH card with its own real vendor/device ids.

This no longer holds I think.

> +     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
> +     */
> +    if (!pci_dev) {
> +        fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed!\n");
> +        return;
> +    }
> +    pci_config_set_device_id(pci_dev->config, pch_dev_id);
> +    pci_config_set_revision(pci_dev->config, pch_rev_id);
> +}
> +
>  /* init */
>  
>  static int xen_pt_initfn(PCIDevice *d)
> @@ -680,6 +803,9 @@ static int xen_pt_initfn(PCIDevice *d)
>              xen_host_pci_device_put(&s->real_device);
>              return -1;
>          }
> +
> +        /* Register ISA bridge for passthrough GFX. */
> +        xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
>      }
>  
>      /* Handle real device's MMIO/PIO BARs */
> -- 
> 1.9.1

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Chen, Tiejun]

>> +
>> +    if (pch_dev_id == 0xffff) {
>> +        fprintf(stderr, "unsupported PCH!\n");
>
> I would drop this fprintf: this likely means a newer
> card, so the bridge is not necessary.

Okay.

>
>> +        return;
>> +    }
>> +
>> +    /* Currently IGD drivers always need to access PCH by 1f.0. */
>> +    pci_dev = pci_create_simple(d->bus, PCI_DEVFN(0x1f, 0),
>> +                                "xen-igd-passthrough-isa-bridge");
>> +
>> +    /*
>> +     * Identify PCH card with its own real vendor/device ids.
>
> This no longer holds I think.

You're right.

Thanks
Tiejun

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 05:28:40PM +0800, Tiejun Chen wrote:
> Currently IGD drivers always need to access PCH by 1f.0. But we
> don't want to poke that directly to get ID, and although in real
> world different GPU should have different PCH. But actually the
> different PCH DIDs likely map to different PCH SKUs. We do the
> same thing for the GPU. For PCH, the different SKUs are going to
> be all the same silicon design and implementation, just different
> features turn on and off with fuses. The SW interfaces should be
> consistent across all SKUs in a given family (eg LPT). But just
> same features may not be supported.
> 
> Most of these different PCH features probably don't matter to the
> Gfx driver, but obviously any difference in display port connections
> will so it should be fine with any PCH in case of passthrough.
> 
> So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
> scenarios, 0x9cc3 for BDW(Broadwell).
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/xen/xen_pt.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index fcc9f1c..5532d6f 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
>      .priority = 10,
>  };
>  
> +typedef struct {
> +    uint16_t gpu_device_id;
> +    uint16_t pch_device_id;
> +    uint8_t pch_revision_id;
> +} XenIGDDeviceIDInfo;
> +
> +/* In real world different GPU should have different PCH. But actually
> + * the different PCH DIDs likely map to different PCH SKUs. We do the
> + * same thing for the GPU. For PCH, the different SKUs are going to be
> + * all the same silicon design and implementation, just different
> + * features turn on and off with fuses. The SW interfaces should be
> + * consistent across all SKUs in a given family (eg LPT). But just same
> + * features may not be supported.
> + *
> + * Most of these different PCH features probably don't matter to the
> + * Gfx driver, but obviously any difference in display port connections
> + * will so it should be fine with any PCH in case of passthrough.
> + *
> + * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
> + * scenarios, 0x9cc3 for BDW(Broadwell).
> + */
> +static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
> +    /* HSW Classic */
> +    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
> +    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
> +    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
> +    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
> +    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
> +    /* HSW ULT */
> +    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
> +    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
> +    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
> +    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
> +    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
> +    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
> +    /* HSW CRW */
> +    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
> +    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
> +    /* HSW Server */
> +    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
> +    /* HSW SRVR */
> +    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
> +    /* BSW */
> +    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
> +    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
> +    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
> +    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
> +    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
> +    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
> +    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
> +    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
> +    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
> +    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
> +    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
> +};
> +
> +static void isa_bridge_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    dc->desc        = "ISA bridge faked to support IGD PT";
> +    k->vendor_id    = PCI_VENDOR_ID_INTEL;
> +    k->class_id     = PCI_CLASS_BRIDGE_ISA;
> +};
> +
> +static TypeInfo isa_bridge_info = {
> +    .name          = "xen-igd-passthrough-isa-bridge",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCIDevice),
> +    .class_init = isa_bridge_class_init,
> +};
> +
> +static void xen_pt_graphics_register_types(void)
> +{
> +    type_register_static(&isa_bridge_info);
> +}
> +type_init(xen_pt_graphics_register_types)
> +
> +static void
> +xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> +                                      XenHostPCIDevice *dev)
> +{

I suggest this implementation, and the table, are moved
to the same file where igd-passthrough-isa-bridge
is implemented. The function can get PCIDevice *d, this way
it's not xen specific.



> +    struct PCIDevice *pci_dev;

pls rename bridge_dev;

> +    int i, num;
> +    uint16_t gpu_dev_id, pch_dev_id = 0xffff;
> +    uint8_t pch_rev_id;
> +    PCIDevice *d = &s->dev;
> +
> +    if (!is_igd_vga_passthrough(dev)) {
> +        return;
> +    }
> +
> +    gpu_dev_id = dev->device_id;
> +    num = ARRAY_SIZE(xen_igd_combo_id_infos);
> +    for (i = 0; i < num; i++) {
> +        if (gpu_dev_id == xen_igd_combo_id_infos[i].gpu_device_id) {
> +            pch_dev_id = xen_igd_combo_id_infos[i].pch_device_id;
> +            pch_rev_id = xen_igd_combo_id_infos[i].pch_revision_id;
> +        }
> +    }
> +
> +    if (pch_dev_id == 0xffff) {
> +        fprintf(stderr, "unsupported PCH!\n");
> +        return;
> +    }
> +
> +    /* Currently IGD drivers always need to access PCH by 1f.0. */
> +    pci_dev = pci_create_simple(d->bus, PCI_DEVFN(0x1f, 0),
> +                                "xen-igd-passthrough-isa-bridge");
> +
> +    /*
> +     * Identify PCH card with its own real vendor/device ids.
> +     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
> +     */
> +    if (!pci_dev) {
> +        fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed!\n");
> +        return;
> +    }
> +    pci_config_set_device_id(pci_dev->config, pch_dev_id);
> +    pci_config_set_revision(pci_dev->config, pch_rev_id);
> +}
> +
>  /* init */
>  
>  static int xen_pt_initfn(PCIDevice *d)
> @@ -680,6 +803,9 @@ static int xen_pt_initfn(PCIDevice *d)
>              xen_host_pci_device_put(&s->real_device);
>              return -1;
>          }
> +
> +        /* Register ISA bridge for passthrough GFX. */
> +        xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
>      }
>  
>      /* Handle real device's MMIO/PIO BARs */
> -- 
> 1.9.1

Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

[Chen, Tiejun]

>> +static void
>> +xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
>> +                                      XenHostPCIDevice *dev)
>> +{
>
> I suggest this implementation, and the table, are moved
> to the same file where igd-passthrough-isa-bridge
> is implemented. The function can get PCIDevice *d, this way
> it's not xen specific.

Absolutely, you're right.

Actually I already start to work this way since Gerd said this should 
have a common between Xen and Kvm(KvmGT).

>
>
>
>> +    struct PCIDevice *pci_dev;
>
> pls rename bridge_dev;

Okay.

Thanks
Tiejun

[Qemu-devel] [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D

[Tiejun Chen]

Some registers of Intel IGD are mapped in host bridge, so it needs to
passthrough these registers of physical host bridge to guest because
emulated host bridge in guest doesn't have these mappings.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/pci-host/piix.c       |  3 ++
 hw/xen/xen_pt.h          |  1 +
 hw/xen/xen_pt_graphics.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1468961..0a5a4c7 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -34,6 +34,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/i386/ioapic.h"
 #include "qapi/visitor.h"
+#include "hw/xen/xen_pt.h"
 
 /*
  * I440FX chipset data sheet.
@@ -733,8 +734,10 @@ static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
                                                   void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     dc->desc = "IGD PT XEN Host bridge";
+    k->config_read = xen_igd_pci_read;
 }
 
 static const TypeInfo xen_igd_passthrough_i440fx_info = {
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 0aa5a93..94cde4a 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -5,6 +5,7 @@
 #include "hw/xen/xen_common.h"
 #include "hw/pci/pci.h"
 #include "xen-host-pci-device.h"
+uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
 
 void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3);
 
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 3232296..227089b 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -4,6 +4,7 @@
 #include "xen_pt.h"
 #include "xen-host-pci-device.h"
 #include "hw/xen/xen_backend.h"
+#include "hw/pci/pci_bus.h"
 
 typedef struct VGARegion {
     int type;           /* Memory or port I/O */
@@ -188,3 +189,74 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
     return 0;
 }
+
+/*
+ * Currently we just pass this physical host bridge for IGD, 00:02.0.
+ *
+ * Here pci_dev is just that host bridge, so we have to get that real
+ * passthrough device by that given devfn to avoid other devices access.
+ */
+static int is_igd_passthrough(PCIDevice *pci_dev)
+{
+    PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
+    if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
+        XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, f);
+        return (is_igd_vga_passthrough(&s->real_device)
+                    && (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL));
+    } else {
+        return 0;
+    }
+}
+
+uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
+{
+    XenHostPCIDevice dev;
+    uint32_t val;
+    int r;
+
+    /* IGD read/write is through the host bridge.
+     */
+    assert(pci_dev->devfn == 0x00);
+
+    if (!is_igd_passthrough(pci_dev)) {
+        goto read_default;
+    }
+
+    /* Just work for the i915 driver. */
+    switch (config_addr) {
+    case 0x08:        /* revision id */
+    case 0x2c:        /* sybsystem vendor id */
+    case 0x2e:        /* sybsystem id */
+    case 0x50:        /* SNB: processor graphics control register */
+    case 0x52:        /* processor graphics control register */
+    case 0xa0:        /* top of memory */
+    case 0xa4:        /* SNB: graphics base of stolen memory */
+    case 0xa8:        /* SNB: base of GTT stolen memory */
+        break;
+    default:
+        /* Just gets the emulated values. */
+        goto read_default;
+    }
+
+    /* Host read */
+    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
+    if (r) {
+        goto err_out;
+    }
+
+    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
+    if (r) {
+        goto err_out;
+    }
+
+    xen_host_pci_device_put(&dev);
+
+    return val;
+
+read_default:
+    return pci_default_read_config(pci_dev, config_addr, len);
+
+err_out:
+    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
+    return -1;
+}
-- 
1.9.1

Re: [Qemu-devel] [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 05:28:41PM +0800, Tiejun Chen wrote:
> Some registers of Intel IGD are mapped in host bridge, so it needs to
> passthrough these registers of physical host bridge to guest because
> emulated host bridge in guest doesn't have these mappings.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  hw/pci-host/piix.c       |  3 ++
>  hw/xen/xen_pt.h          |  1 +
>  hw/xen/xen_pt_graphics.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1468961..0a5a4c7 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/ioapic.h"
>  #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -733,8 +734,10 @@ static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
>                                                    void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
>      dc->desc = "IGD PT XEN Host bridge";
> +    k->config_read = xen_igd_pci_read;
>  }
>  
>  static const TypeInfo xen_igd_passthrough_i440fx_info = {
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 0aa5a93..94cde4a 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -5,6 +5,7 @@
>  #include "hw/xen/xen_common.h"
>  #include "hw/pci/pci.h"
>  #include "xen-host-pci-device.h"
> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
>  
>  void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3);
>  
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 3232296..227089b 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -4,6 +4,7 @@
>  #include "xen_pt.h"
>  #include "xen-host-pci-device.h"
>  #include "hw/xen/xen_backend.h"
> +#include "hw/pci/pci_bus.h"
>  
>  typedef struct VGARegion {
>      int type;           /* Memory or port I/O */
> @@ -188,3 +189,74 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
>      cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
>      return 0;
>  }
> +
> +/*
> + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> + *
> + * Here pci_dev is just that host bridge, so we have to get that real
> + * passthrough device by that given devfn to avoid other devices access.
> + */
> +static int is_igd_passthrough(PCIDevice *pci_dev)
> +{
> +    PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> +    if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
> +        XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, f);
> +        return (is_igd_vga_passthrough(&s->real_device)
> +                    && (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL));
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
> +{
> +    XenHostPCIDevice dev;
> +    uint32_t val;
> +    int r;
> +
> +    /* IGD read/write is through the host bridge.
> +     */
> +    assert(pci_dev->devfn == 0x00);
> +
> +    if (!is_igd_passthrough(pci_dev)) {
> +        goto read_default;
> +    }
> +
> +    /* Just work for the i915 driver. */
> +    switch (config_addr) {
> +    case 0x08:        /* revision id */
> +    case 0x2c:        /* sybsystem vendor id */
> +    case 0x2e:        /* sybsystem id */
> +    case 0x50:        /* SNB: processor graphics control register */
> +    case 0x52:        /* processor graphics control register */
> +    case 0xa0:        /* top of memory */

Is this host physical memory? If yes how can using it in guest work?

> +    case 0xa4:        /* SNB: graphics base of stolen memory */
> +    case 0xa8:        /* SNB: base of GTT stolen memory */

Same question for above two.

> +        break;
> +    default:
> +        /* Just gets the emulated values. */
> +        goto read_default;
> +    }
> +
> +    /* Host read */
> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> +    if (r) {
> +        goto err_out;
> +    }
> +
> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
> +    if (r) {
> +        goto err_out;
> +    }
> +
> +    xen_host_pci_device_put(&dev);
> +
> +    return val;
> +
> +read_default:
> +    return pci_default_read_config(pci_dev, config_addr, len);
> +
> +err_out:
> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> +    return -1;
> +}

Do any of the above registers change with time?
Does it work if we just read them when device is created
and put in dev->config?

> -- 
> 1.9.1

Re: [Qemu-devel] [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D

[Chen, Tiejun]

>> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
>> +{
>> +    XenHostPCIDevice dev;
>> +    uint32_t val;
>> +    int r;
>> +
>> +    /* IGD read/write is through the host bridge.
>> +     */
>> +    assert(pci_dev->devfn == 0x00);
>> +
>> +    if (!is_igd_passthrough(pci_dev)) {
>> +        goto read_default;
>> +    }
>> +
>> +    /* Just work for the i915 driver. */
>> +    switch (config_addr) {
>> +    case 0x08:        /* revision id */
>> +    case 0x2c:        /* sybsystem vendor id */
>> +    case 0x2e:        /* sybsystem id */
>> +    case 0x50:        /* SNB: processor graphics control register */
>> +    case 0x52:        /* processor graphics control register */
>> +    case 0xa0:        /* top of memory */
>
> Is this host physical memory? If yes how can using it in guest work?

This is just a threshold value, not a start or end address :)

>
>> +    case 0xa4:        /* SNB: graphics base of stolen memory */
>> +    case 0xa8:        /* SNB: base of GTT stolen memory */
>
> Same question for above two.

I shouldn't matter since I remember we already discussed this previously 
but I can't sort out this from those emails now.

Allen,

Could you reexplain this?

>
>> +        break;
>> +    default:
>> +        /* Just gets the emulated values. */
>> +        goto read_default;
>> +    }
>> +
>> +    /* Host read */
>> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
>> +    if (r) {
>> +        goto err_out;
>> +    }
>> +
>> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
>> +    if (r) {
>> +        goto err_out;
>> +    }
>> +
>> +    xen_host_pci_device_put(&dev);
>> +
>> +    return val;
>> +
>> +read_default:
>> +    return pci_default_read_config(pci_dev, config_addr, len);
>> +
>> +err_out:
>> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
>> +    return -1;
>> +}
>
> Do any of the above registers change with time?

Think about we just provide read ops, so they're not changed based on my 
experiential.

> Does it work if we just read them when device is created
> and put in dev->config?

I think this is a good idea so I will go there and thank you.

Tiejun

Re: [Qemu-devel] [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D

[Kay, Allen M]

> -----Original Message-----
> From: Chen, Tiejun
> Sent: Tuesday, January 20, 2015 7:17 PM
> To: Michael S. Tsirkin; Kay, Allen M
> Cc: pbonzini@redhat.com; aliguori@amazon.com; rth@twiddle.net; Zhang,
> Yang Z; qemu-devel@nongnu.org
> Subject: Re: [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD
> passthrough with VT-D
> 
> >> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr,
> >> +int len) {
> >> +    XenHostPCIDevice dev;
> >> +    uint32_t val;
> >> +    int r;
> >> +
> >> +    /* IGD read/write is through the host bridge.
> >> +     */
> >> +    assert(pci_dev->devfn == 0x00);
> >> +
> >> +    if (!is_igd_passthrough(pci_dev)) {
> >> +        goto read_default;
> >> +    }
> >> +
> >> +    /* Just work for the i915 driver. */
> >> +    switch (config_addr) {
> >> +    case 0x08:        /* revision id */
> >> +    case 0x2c:        /* sybsystem vendor id */
> >> +    case 0x2e:        /* sybsystem id */
> >> +    case 0x50:        /* SNB: processor graphics control register */
> >> +    case 0x52:        /* processor graphics control register */
> >> +    case 0xa0:        /* top of memory */
> >
> > Is this host physical memory? If yes how can using it in guest work?
> 
> This is just a threshold value, not a start or end address :)
> 

"top of memory" is no longer used in new versions of Windows drivers.  Tiejun and I have tried BDW/HSW drivers and found they work without the need to passthrough 0xa0 register in the host bridge.  It can be removed.

> >
> >> +    case 0xa4:        /* SNB: graphics base of stolen memory */
> >> +    case 0xa8:        /* SNB: base of GTT stolen memory */
> >
> > Same question for above two.
> 
> I shouldn't matter since I remember we already discussed this previously but
> I can't sort out this from those emails now.
> 
> Allen,
> 
> Could you reexplain this?
> 

These two "stolen memory" regions are part of the VT-d RMRR region.  They are 1:1 mapped in the guest (GPA == HPA).  Tiejun found they are needed by vBIOS during boot.  Modern Windows driver no longer need to access these two registers in MCH.

> >
> >> +        break;
> >> +    default:
> >> +        /* Just gets the emulated values. */
> >> +        goto read_default;
> >> +    }
> >> +
> >> +    /* Host read */
> >> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> >> +    if (r) {
> >> +        goto err_out;
> >> +    }
> >> +
> >> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
> >> +    if (r) {
> >> +        goto err_out;
> >> +    }
> >> +
> >> +    xen_host_pci_device_put(&dev);
> >> +
> >> +    return val;
> >> +
> >> +read_default:
> >> +    return pci_default_read_config(pci_dev, config_addr, len);
> >> +
> >> +err_out:
> >> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> >> +    return -1;
> >> +}
> >
> > Do any of the above registers change with time?
> 
> Think about we just provide read ops, so they're not changed based on my
> experiential.
> 
> > Does it work if we just read them when device is created and put in
> > dev->config?
> 
> I think this is a good idea so I will go there and thank you.
> 
> Tiejun
> 

Allen

[Qemu-devel] [v6][PATCH 09/10] xen, gfx passthrough: register host bridge specific to passthrough

[Tiejun Chen]

Just register that pci host bridge specific to passthrough.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4148028..f015238 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -50,7 +50,8 @@
 #include "cpu.h"
 #include "qemu/error-report.h"
 #ifdef CONFIG_XEN
-#  include <xen/hvm/hvm_info_table.h>
+#include <xen/hvm/hvm_info_table.h>
+#include "hw/xen/xen_pt.h"
 #endif
 
 #define MAX_IDE_BUS 2
@@ -496,11 +497,25 @@ static void pc_init_isa(MachineState *machine)
 }
 
 #ifdef CONFIG_XEN
+static void xen_igd_passthrough_pc_init_pci(MachineState *machine)
+{
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+}
+
+static void xen_pc_init_pci(MachineState *machine)
+{
+    if (xen_has_gfx_passthru)
+        xen_igd_passthrough_pc_init_pci(machine);
+    else
+        pc_init_pci(machine);
+}
+
 static void pc_xen_hvm_init(MachineState *machine)
 {
     PCIBus *bus;
 
-    pc_init_pci(machine);
+    xen_pc_init_pci(machine);
 
     bus = pci_find_primary_bus();
     if (bus != NULL) {
-- 
1.9.1

[Qemu-devel] [v6][PATCH 10/10] xen, gfx passthrough: add opregion mapping

[Tiejun Chen]

The OpRegion shouldn't be mapped 1:1 because the address in the host
can't be used in the guest directly.

This patch traps read and write access to the opregion of the Intel
GPU config space (offset 0xfc).

The original patch is from Jean Guyader <jean.guyader@eu.citrix.com>

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/xen/xen_pt.h             |  6 +++-
 hw/xen/xen_pt_config_init.c | 52 ++++++++++++++++++++++++++--
 hw/xen/xen_pt_graphics.c    | 82 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 94cde4a..56f6d74 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -37,6 +37,9 @@ typedef struct XenPTReg XenPTReg;
 
 typedef struct XenPCIPassthroughState XenPCIPassthroughState;
 
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+
 /* function type for config reg */
 typedef int (*xen_pt_conf_reg_init)
     (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -63,8 +66,9 @@ typedef int (*xen_pt_conf_byte_read)
 #define XEN_PT_BAR_ALLF 0xFFFFFFFF
 #define XEN_PT_BAR_UNMAPPED (-1)
 
-#define PCI_CAP_MAX 48
+#define XEN_PCI_CAP_MAX 48
 
+#define XEN_PCI_INTEL_OPREGION 0xfc
 
 typedef enum {
     XEN_PT_GRP_TYPE_HARDWIRED = 0,  /* 0 Hardwired reg group */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index de9a20f..e4e301a 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -575,6 +575,22 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
     return 0;
 }
 
+static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s,
+                                      XenPTReg *cfg_entry,
+                                      uint32_t *value, uint32_t valid_mask)
+{
+    *value = igd_read_opregion(s);
+    return 0;
+}
+
+static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s,
+                                       XenPTReg *cfg_entry, uint32_t *value,
+                                       uint32_t dev_value, uint32_t valid_mask)
+{
+    igd_write_opregion(s, *value);
+    return 0;
+}
+
 /* Header Type0 reg static information table */
 static XenPTRegInfo xen_pt_emu_reg_header0[] = {
     /* Vendor ID reg */
@@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
     },
 };
 
+static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = {
+    /* Intel IGFX OpRegion reg */
+    {
+        .offset     = 0x0,
+        .size       = 4,
+        .init_val   = 0,
+        .no_wb      = 1,
+        .u.dw.read   = xen_pt_intel_opregion_read,
+        .u.dw.write  = xen_pt_intel_opregion_write,
+    },
+    {
+        .size = 0,
+    },
+};
 
 /****************************
  * Capabilities
@@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = {
         .size_init   = xen_pt_msix_size_init,
         .emu_regs = xen_pt_emu_reg_msix,
     },
+    /* Intel IGD Opregion group */
+    {
+        .grp_id      = XEN_PCI_INTEL_OPREGION,
+        .grp_type    = XEN_PT_GRP_TYPE_EMU,
+        .grp_size    = 0x4,
+        .size_init   = xen_pt_reg_grp_size_init,
+        .emu_regs    = xen_pt_emu_reg_igd_opregion,
+    },
     {
         .grp_size = 0,
     },
@@ -1727,7 +1765,7 @@ out:
 static uint8_t find_cap_offset(XenPCIPassthroughState *s, uint8_t cap)
 {
     uint8_t id;
-    unsigned max_cap = PCI_CAP_MAX;
+    unsigned max_cap = XEN_PCI_CAP_MAX;
     uint8_t pos = PCI_CAPABILITY_LIST;
     uint8_t status = 0;
 
@@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
         uint32_t reg_grp_offset = 0;
         XenPTRegGroup *reg_grp_entry = NULL;
 
-        if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) {
+        if (xen_pt_emu_reg_grps[i].grp_id != 0xFF
+            && xen_pt_emu_reg_grps[i].grp_id != XEN_PCI_INTEL_OPREGION) {
             if (xen_pt_hide_dev_cap(&s->real_device,
                                     xen_pt_emu_reg_grps[i].grp_id)) {
                 continue;
@@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
             }
         }
 
+        /*
+         * By default we will trap up to 0x40 in the cfg space.
+         * If an intel device is pass through we need to trap 0xfc,
+         * therefore the size should be 0xff.
+         */
+        if (xen_pt_emu_reg_grps[i].grp_id == XEN_PCI_INTEL_OPREGION) {
+            reg_grp_offset = XEN_PCI_INTEL_OPREGION;
+        }
+
         reg_grp_entry = g_new0(XenPTRegGroup, 1);
         QLIST_INIT(&reg_grp_entry->reg_tbl_list);
         QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries);
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 227089b..372badf 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -6,6 +6,11 @@
 #include "hw/xen/xen_backend.h"
 #include "hw/pci/pci_bus.h"
 
+static unsigned long igd_guest_opregion;
+static unsigned long igd_host_opregion;
+
+#define XEN_PCI_INTEL_OPREGION_MASK 0xfff
+
 typedef struct VGARegion {
     int type;           /* Memory or port I/O */
     uint64_t guest_base_addr;
@@ -82,6 +87,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
 int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
 {
     int i = 0;
+    int ret = 0;
 
     if (!is_igd_vga_passthrough(dev)) {
         return 0;
@@ -108,6 +114,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
         }
     }
 
+    if (igd_guest_opregion) {
+        ret = xc_domain_memory_mapping(xen_xc, xen_domid,
+                (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
+                (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+                3,
+                DPCI_REMOVE_MAPPING);
+        if (ret) {
+            return ret;
+        }
+    }
+
     return 0;
 }
 
@@ -260,3 +277,68 @@ err_out:
     XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
     return -1;
 }
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s)
+{
+    uint32_t val = 0;
+
+    if (!igd_guest_opregion) {
+        return val;
+    }
+
+    val = igd_guest_opregion;
+
+    XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val);
+    return val;
+}
+
+#define XEN_PCI_INTEL_OPREGION_PAGES 0x3
+#define XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED 0x1
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
+{
+    int ret;
+
+    if (igd_guest_opregion) {
+        XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring %x\n",
+                   val);
+        return;
+    }
+
+    /* We just work with LE. */
+    xen_host_pci_get_block(&s->real_device, XEN_PCI_INTEL_OPREGION,
+            (uint8_t *)&igd_host_opregion, 4);
+    igd_guest_opregion = (unsigned long)(val & ~XEN_PCI_INTEL_OPREGION_MASK)
+                            | (igd_host_opregion & XEN_PCI_INTEL_OPREGION_MASK);
+
+    ret = xc_domain_iomem_permission(xen_xc, xen_domid,
+            (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+            XEN_PCI_INTEL_OPREGION_PAGES,
+            XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED);
+
+    if (ret) {
+        XEN_PT_ERR(&s->dev, "[%d]:Can't enable to access IGD host opregion:"
+                    " 0x%lx.\n", ret,
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT)),
+        igd_guest_opregion = 0;
+        return;
+    }
+
+    ret = xc_domain_memory_mapping(xen_xc, xen_domid,
+            (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
+            (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+            XEN_PCI_INTEL_OPREGION_PAGES,
+            DPCI_ADD_MAPPING);
+
+    if (ret) {
+        XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to"
+                    " guest opregion:0x%lx.\n", ret,
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+                    (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+        igd_guest_opregion = 0;
+        return;
+    }
+
+    XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n",
+                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
+                    (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT));
+}
-- 
1.9.1