qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Chen, Tiejun
Subject: Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough
Date: Fri, 05 Jun 2015 16:36:39 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 2015/6/2 17:17, Michael S. Tsirkin wrote:
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

It should be, but in this revision, I just received this sole comment until now so I mean its not worth resending a new review just with this fix :)

Anyway, its not big deal, just let me send this out as you expect.

Thanks
Tiejun

fixed in the next revision of the patchset"?




reply via email to

[Prev in Thread] Current Thread [Next in Thread]