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

[Chen, Tiejun]

Michael and Gerd,

I don't see any more comments for this revision, so what's next that I 
should do?

Thanks
Tiejun

On 2015/3/19 9:01, 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.
>
> Thanks
> Tiejun