diff mbox

[v2,5/8] xen, gfx passthrough: create intel isa bridge

Message ID 1400237624-8505-6-git-send-email-tiejun.chen@intel.com
State New
Headers show

Commit Message

Tiejun Chen May 16, 2014, 10:53 a.m. UTC 
ISA bridge is needed since Intel gfx drive will probe it instead
of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
only need to expose ISA bridge to let driver know the real hardware underneath.

The original patch is from Allen Kay [allen.m.kay@intel.com]

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Cc: Allen Kay <allen.m.kay@intel.com>
---
v2:

* Nothing is changed.

 hw/xen/xen_pt_graphics.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

Konrad Rzeszutek Wilk May 16, 2014, 2:11 p.m. UTC | #1 
On Fri, May 16, 2014 at 06:53:41PM +0800, Tiejun Chen wrote:
> ISA bridge is needed since Intel gfx drive will probe it instead
> of Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
> only need to expose ISA bridge to let driver know the real hardware underneath.
> 
> The original patch is from Allen Kay [allen.m.kay@intel.com]
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Cc: Allen Kay <allen.m.kay@intel.com>
> ---
> v2:
> 
> * Nothing is changed.
> 
>  hw/xen/xen_pt_graphics.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index e1f0724..6b86293 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -175,3 +175,74 @@ out:
>      g_free(bios);
>      return rc;
>  }
> +
> +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
> +{
> +    uint32_t v;
> +
> +    v = pci_default_read_config(d, addr, len);
> +
> +    return v;

return pci_default_read_config...

> +}
> +
> +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
> +                                    int len)
> +{
> +    pci_default_write_config(d, addr, v, len);
> +
> +    return;
> +}
> +
> +static void isa_bridge_class_init(ObjectClass *klass, void *data)
> +{
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->config_read = isa_bridge_read_config;
> +    k->config_write = isa_bridge_write_config;
> +
> +    return;
> +};
> +
> +typedef struct {
> +    PCIDevice dev;
> +} ISABridgeState;
> +
> +static TypeInfo isa_bridge_info = {
> +    .name          = "inte-pch-isa-bridge",

s/inte/intel/

> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(ISABridgeState),
> +    .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 int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
> +{
> +    struct PCIDevice *dev;
> +
> +    char rid;
> +
> +    dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "inte-pch-isa-bridge");

s/inte/intel/?

> +    if (!dev) {
> +        XEN_PT_LOG(dev, "fail to create PCH ISA bridge.\n");

failed

> +        return -1;

Could we return a better error code to help troubleshoot why we 
failed?

> +    }
> +
> +    qdev_init_nofail(&dev->qdev);
> +
> +    pci_config_set_vendor_id(dev->config, hdev->vendor_id);
> +    pci_config_set_device_id(dev->config, hdev->device_id);
> +
> +    xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
> +
> +    pci_config_set_revision(dev->config, rid);
> +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
> +
> +    XEN_PT_LOG(dev, "Intel PCH ISA bridge is created.\n");

s/is//

> +    return 0;
> +}
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Tiejun Chen May 19, 2014, 9:59 a.m. UTC | #2 
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, May 16, 2014 10:12 PM
> To: Chen, Tiejun
> Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; weidong.han@intel.com; Kay, Allen M;
> qemu-devel@nongnu.org; jean.guyader@eu.citrix.com;
> anthony@codemonkey.ws; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v2][PATCH 5/8] xen, gfx passthrough: create intel isa
> bridge
> 
> On Fri, May 16, 2014 at 06:53:41PM +0800, Tiejun Chen wrote:
> > ISA bridge is needed since Intel gfx drive will probe it instead of
> > Dev31:Fun0 to make graphics device passthrough work easy for VMM, that
> > only need to expose ISA bridge to let driver know the real hardware
> underneath.
> >
> > The original patch is from Allen Kay [allen.m.kay@intel.com]
> >
> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> > Cc: Allen Kay <allen.m.kay@intel.com>
> > ---
> > v2:
> >
> > * Nothing is changed.
> >
> >  hw/xen/xen_pt_graphics.c | 71
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> >
> > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index
> > e1f0724..6b86293 100644
> > --- a/hw/xen/xen_pt_graphics.c
> > +++ b/hw/xen/xen_pt_graphics.c
> > @@ -175,3 +175,74 @@ out:
> >      g_free(bios);
> >      return rc;
> >  }
> > +
> > +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr,
> > +int len) {
> > +    uint32_t v;
> > +
> > +    v = pci_default_read_config(d, addr, len);
> > +
> > +    return v;
> 
> return pci_default_read_config...

Fixed.

> 
> > +}
> > +
> > +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
> > +                                    int len) {
> > +    pci_default_write_config(d, addr, v, len);
> > +
> > +    return;
> > +}
> > +
> > +static void isa_bridge_class_init(ObjectClass *klass, void *data) {
> > +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +
> > +    k->config_read = isa_bridge_read_config;
> > +    k->config_write = isa_bridge_write_config;
> > +
> > +    return;
> > +};
> > +
> > +typedef struct {
> > +    PCIDevice dev;
> > +} ISABridgeState;
> > +
> > +static TypeInfo isa_bridge_info = {
> > +    .name          = "inte-pch-isa-bridge",
> 
> s/inte/intel/

Fixed.

> 
> > +    .parent        = TYPE_PCI_DEVICE,
> > +    .instance_size = sizeof(ISABridgeState),
> > +    .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 int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
> > +{
> > +    struct PCIDevice *dev;
> > +
> > +    char rid;
> > +
> > +    dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "inte-pch-isa-bridge");
> 
> s/inte/intel/?

Fixed.

> 
> > +    if (!dev) {
> > +        XEN_PT_LOG(dev, "fail to create PCH ISA bridge.\n");
> 
> failed

Fixed.

> 
> > +        return -1;
> 
> Could we return a better error code to help troubleshoot why we failed?

I take a look at this then I think we can remove these lines,

pci_create()
	|
	+ qdev_create()

And 

DeviceState *qdev_create(BusState *bus, const char *name)
{
    DeviceState *dev; 
 
    dev = qdev_try_create(bus, name); 
    if (!dev) {
        if (bus) {
            error_report("Unknown device '%s' for bus '%s'", name, 
                         object_get_typename(OBJECT(bus)));
        } else { 
            error_report("Unknown device '%s' for default sysbus", name);
        }
        abort(); 
    }

> 
> > +    }
> > +
> > +    qdev_init_nofail(&dev->qdev);
> > +
> > +    pci_config_set_vendor_id(dev->config, hdev->vendor_id);
> > +    pci_config_set_device_id(dev->config, hdev->device_id);
> > +
> > +    xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid,
> > + 1);
> > +
> > +    pci_config_set_revision(dev->config, rid);
> > +    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
> > +
> > +    XEN_PT_LOG(dev, "Intel PCH ISA bridge is created.\n");
> 
> s/is//

Fixed.

Thanks
Tiejun
diff mbox

Patch

diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index e1f0724..6b86293 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -175,3 +175,74 @@  out:
     g_free(bios);
     return rc;
 }
+
+static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+    uint32_t v;
+
+    v = pci_default_read_config(d, addr, len);
+
+    return v;
+}
+
+static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
+                                    int len)
+{
+    pci_default_write_config(d, addr, v, len);
+
+    return;
+}
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->config_read = isa_bridge_read_config;
+    k->config_write = isa_bridge_write_config;
+
+    return;
+};
+
+typedef struct {
+    PCIDevice dev;
+} ISABridgeState;
+
+static TypeInfo isa_bridge_info = {
+    .name          = "inte-pch-isa-bridge",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(ISABridgeState),
+    .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 int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+    struct PCIDevice *dev;
+
+    char rid;
+
+    dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "inte-pch-isa-bridge");
+    if (!dev) {
+        XEN_PT_LOG(dev, "fail to create PCH ISA bridge.\n");
+        return -1;
+    }
+
+    qdev_init_nofail(&dev->qdev);
+
+    pci_config_set_vendor_id(dev->config, hdev->vendor_id);
+    pci_config_set_device_id(dev->config, hdev->device_id);
+
+    xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)&rid, 1);
+
+    pci_config_set_revision(dev->config, rid);
+    pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_ISA);
+
+    XEN_PT_LOG(dev, "Intel PCH ISA bridge is created.\n");
+    return 0;
+}