Re: [Xen-devel] [PATCH] qemu: Don't access /proc/bus/pci unless graphics pass-thru is enabled

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 10 Feb 2012, George Dunlap wrote:
> > >      vid = pt_pci_host_read(pci_dev_1f, PCI_VENDOR_ID, 2);
> > > @@ -39,9 +39,9 @@ void intel_pch_init(PCIBus *bus)
> > >  
> > >  void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t 
> > > val, int len)
> > >  {
> > > -    struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0);
> > > +    struct pci_dev *pci_dev_host_bridge;
> > >      assert(pci_dev->devfn == 0x00);
> > > -    if ( !igd_passthru ) {
> > > +    if ( !igd_passthru || !(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 
> > > 0))) {
> > >          pci_default_write_config(pci_dev, config_addr, val, len);
> > >          return;
> > >      }
> > 
> > Why are you adding this test (!(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 
> > 0)) ?
> > 
> > If you are worried that pci_dev_host_bridge could be NULL, shouldn't you
> > also remove the assert?
> 
> The assert is about pci_dev, but the check is about the return value of
> pci_host_bridge() (to which pci_dev_host_bridge is set).  If there's an
> expected relationship between them, it wasn't immediately clear from the
> code.  

I thought you were worried about the BDF being wrong (00:00.0), BDF that
is the same as pci_dev->devfn and already checked by assert.
However now I realize that pt_pci_get_dev could also fail because pcilib
cannot read the host bridge properly so I think that the check makes
sense.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel