Re: [Xen-devel] [Qemu-devel] [RFC][PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/11/17 18:13, Michael S. Tsirkin wrote:
> On Mon, Nov 17, 2014 at 05:42:12PM +0800, Chen, Tiejun wrote:
> > On 2014/11/17 17:25, Michael S. Tsirkin wrote:
> > > On Mon, Nov 17, 2014 at 04:48:32PM +0800, Chen, Tiejun wrote:
> > > > On 2014/11/17 14:10, Michael S. Tsirkin wrote:
> > > > > On Mon, Nov 17, 2014 at 10:47:56AM +0800, Chen, Tiejun wrote:
> > > > > > On 2014/11/5 22:09, Michael S. Tsirkin wrote:
> > > > > > > On Wed, Nov 05, 2014 at 03:22:59PM +0800, Tiejun Chen wrote:
> > > > > > > > Currently IGD drivers always need to access PCH by 1f.0, and
> > > > > > > > PCH vendor/device id is used to identify the card.
> > > > > > > >
> > > > > > > > Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> > > > > > > > ---
> > [snip]
> >
> > > Cleaner:
> > >          if (!pci_dev) {
> > >                 fprintf
> > >                 return;
> > >         }
> > >           pci_config_set_device_id(pci_dev->config, pch_id);
> > I will address all comments and thanks.
> >
> > > > +    }
> > > > +}
> > > > +
> > > >   /* init */
> > > >
> > > >   static int xen_pt_initfn(PCIDevice *d)
> > > > @@ -682,6 +770,9 @@ static int xen_pt_initfn(PCIDevice *d)
> > > >           return -1;
> > > >       }
> > > >
> > > > +    /* Register ISA bridge for passthrough GFX. */
> > > > +    xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
> > > > +
> > > >       /* reinitialize each config register to be emulated */
> > > >       if (xen_pt_config_init(s)) {
> > > >           XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> > > >
> > > > Note I will introduce a inline function in another patch,
> > > >
> > > > +static inline int is_vga_passthrough(XenHostPCIDevice *dev)
> > > > +{
> > > > +    return (xen_has_gfx_passthru && (dev->vendor_id == PCI_VENDOR_ID_INTEL)
> > > > +            && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
> > > > +}
> > > >
> > > > Thanks
> > > > Tiejun
> > > Why bother with all these conditions?
> > > Won't it be enough to check dev->vendor_id == PCI_VENDOR_ID_INTEL?
> > If this is just used for IGD, its always fine without checking vendor_id.
> You need to match device ID to *know* it's IGD.
>
> > So after remove that check, I guess I need to rename that as
> > is_igd_vga_passthrough() to make sense.
> >
> > Thanks
> > Tiejun
> There is no need to check class code or xen_has_gfx_passthru flag.
> Device ID + vendor ID identifies each device uniquely.
Yeah.

Here I assume vendor ID is always PCI_VENDOR_ID_INTEL so looks you means 
I also need to check that table to do something like,
is_igd_vga_passthugh(dev)
{       
        int i;
        int num = ARRAY_SIZE(xen_igd_combo_id_infos);
        for (i = 0; i < num; i++) {
                if (dev->device_id == xen_igd_combo_id_infos[i].gpu_device_id)
                        return 1;
        return 0;
}

Then we can simplify the subsequent codes based on this, right?

Thanks
Tiejun


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