Re: [Xen-devel] [PATCH 2/3] qemu-xen-trad: Correctly expose PCH ISA bridge for IGD passthrough

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.13 at 17:12, Rui Guo <firemeteor@xxxxxxxxxxxxxxxxxxxxx> wrote:
> @@ -40,9 +41,26 @@ void intel_pch_init(PCIBus *bus)
>      did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
>      rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);
>  
> -    if ( vid == PCI_VENDOR_ID_INTEL )
> -        pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
> -                        pch_map_irq, "intel_bridge_1f");
> +    if (vid == PCI_VENDOR_ID_INTEL) {
> +        PCIBridge *s = (PCIBridge *)pci_register_device(bus, 
> "intel_bridge_1f",
> +                sizeof(PCIBridge), PCI_DEVFN(0x1f, 0), NULL, 
> pci_bridge_write_config);
> +
> +        pci_config_set_vendor_id(s->dev.config, vid);
> +        pci_config_set_device_id(s->dev.config, did);
> +
> +        s->dev.config[PCI_COMMAND] = 0x06; // command = bus master, pci mem
> +        s->dev.config[PCI_COMMAND + 1] = 0x00;
> +        s->dev.config[PCI_STATUS] = 0xa0; // status = fast back-to-back, 
> 66MHz, no error
> +        s->dev.config[PCI_STATUS + 1] = 0x00; // status = fast devsel
> +        s->dev.config[PCI_REVISION] = rid;
> +        s->dev.config[PCI_CLASS_PROG] = 0x00; // programming i/f
> +        pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_ISA);
> +        s->dev.config[PCI_LATENCY_TIMER] = 0x10;
> +        s->dev.config[PCI_HEADER_TYPE] = 0x80;
> +        s->dev.config[PCI_SEC_STATUS] = 0xa0;
> +
> +        s->bus = pci_register_secondary_bus(&s->dev, pch_map_irq);
> +    }
>  }
>  
>  uint32_t igd_read_opregion(struct pt_dev *pci_dev)

For one I wonder whether this is really _always_ correct. I.e.
the device at 00:1f.0 always being an ISA bridge. Wouldn't it
be better to mirror whatever the host device says?

And then I don't see why you need to open code all of
pci_bridge_init() instead of just overriding the class value after
you called that function (or, if the order of events for some
reason matters, adding another parameter to the function).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel