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

["Michael S. Tsirkin" <mst@redhat.com>]

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@intel.com>
> >>>>>>---
> 
> [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.

-- 
MST