On Tue, Jun 02, 2015 at 08:50:58AM +0800, Chen, Tiejun wrote:
> On 2015/6/1 2:11, Michael S. Tsirkin wrote:
> >On Thu, Mar 19, 2015 at 09:01:27AM +0800, Chen, Tiejun wrote:
> >>On 2015/3/18 18:21, Gerd Hoffmann wrote:
> >>>On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:
> >>>>Implement a pci host bridge specific to passthrough. Actually
> >>>>this just inherits the standard one. And we also just expose
> >>>>a minimal real host bridge pci configuration subset.
> >>>
> >>>>+/* Here we just expose minimal host bridge offset subset. */
> >>>>+static const IGDHostInfo igd_host_bridge_infos[] = {
> >>>>+ {0x08, 2}, /* revision id */
> >>>>+ {0x2c, 2}, /* sybsystem vendor id */
> >>>>+ {0x2e, 2}, /* sybsystem id */
> >>>>+ {0x50, 2}, /* SNB: processor graphics control register */
> >>>>+ {0x52, 2}, /* processor graphics control register */
> >>>>+ {0xa4, 4}, /* SNB: graphics base of stolen memory */
> >>>>+ {0xa8, 4}, /* SNB: base of GTT stolen memory */
> >>>>+};
> >>>
> >>>Hmm, no vendor/device id here? Will the idg guest drivers happily read
> >>
> >>These two emulated values are already fine.
> >>
> >>And this is a long story. At the beginning, we were initially trying to
> >>expose more,
> >>
> >>+ case 0x00: /* vendor id */
> >>+ case 0x02: /* device id */
> >>+ 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 0xb0: /* ILK: BSM: should read from dev 2 offset 0x5c */
> >>+ case 0x58: /* SNB: PAVPC Offset */
> >>+ case 0xa4: /* SNB: graphics base of stolen memory */
> >>+ case 0xa8: /* SNB: base of GTT stolen memory */
> >>
> >>But this brought some discussions with Paolo and Michael, and then plus our
> >>further experiment, now as everyone expect, this is a minimal real host
> >>bridge pci configuration subset which we need to expose.
> >>
> >>>graphics control registers from i440fx even though this chipset never
> >>>existed in a igd variant?
> >>>
> >>>[ just wondering, if it works that way fine, it certainly makes things
> >>
> >>Yes, it works fine.
> >>
> >>> easier for the firmware which uses host bridge pci id to figure
> >>> whenever it is running @ i440fx or q35 ].
> >>>
> >>>>+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> >>>>+{
> >>>>+ uint32_t val = 0;
> >>>>+ int rc, i, num;
> >>>>+ int pos, len;
> >>>>+
> >>>>+ num = ARRAY_SIZE(igd_host_bridge_infos);
> >>>>+ for (i = 0; i < num; i++) {
> >>>>+ pos = igd_host_bridge_infos[i].offset;
> >>>>+ len = igd_host_bridge_infos[i].len;
> >>>>+ rc = host_pci_config_read(pos, len, val);
> >>>>+ if (rc) {
> >>>>+ return -ENODEV;
> >>>>+ }
> >>>>+ pci_default_write_config(pci_dev, pos, val, len);
> >>>>+ }
> >>>>+
> >>>>+ return 0;
> >>>>+}
> >>>
> >>>Nothing i440fx specific in here, just copying some host bridge pci
> >>>config space bits. I guess we can sub-class the q35 host bridge the
> >>>same way and even reuse the init function?
> >>
> >>This is our another discussion long time ago :)
> >>
> >>Currently Xen don't run with q35. If I remember those discussions correctly,
> >>something is still restricted to Windows.
> >>
> >>>
> >>>>+#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
> >>>>"xen-igd-passthrough-i440FX"
> >>>
> >>>One xen leftover slipped through here.
> >>
> >>Fixed.
> >
> >So should I expect v8 then?
> >
>
> For this revision we just had only this valuable comment, and that is just
> about to rename a macro, so I think its not worth resend this, right?
>
> If I'm wrong let me know :)
>
> Thanks
> Tiejun
I didn't follow closely so I have no idea how valuable was the last
round of feedback, but when you say "Fixed" don't you mean "will be
fixed in the next revision of the patchset"?
--
MST
