On Tue, Apr 28, 2015 at 10:37:08AM +0200, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2015 at 05:31:03PM +1000, David Gibson wrote:
> > On Wed, Apr 22, 2015 at 01:28:18AM -0500, Michael Roth wrote:
> > > We need to work with PCI BARs to generate OF properties
> > > during PCI hotplug for sPAPR guests.
> > > 
> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > > Cc: mst@redhat.com
> > 
> > Michael Tsirkin,
> > 
> > Interested to see your comment on this.  Are you ok with this?  Can
> > you take this through your tree?  Because it affects general PCI code,
> 
> I'm not too happy but I just chulk it up as
> "yet another spapr weirdness".

Ok.

> I'd like a comment added, see below.
> With that
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Sorry, not entirely clear on your suggested plan here.  Are you saying
resend with the adjusted comment and you'll push it through your tree,
or that I should take it through my tree (with the adjusted comment)?

> 
> > I'm not really comfortable merging this through my spapr-next tree,
> > which is where I plan to put all the rest.
> > 
> > > ---
> > >  hw/pci/pci.c         | 2 +-
> > >  include/hw/pci/pci.h | 1 +
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 6941a82..7d14657 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -123,7 +123,7 @@ static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
> > >  
> > >  static QLIST_HEAD(, PCIHostState) pci_host_bridges;
> > >  
> > > -static int pci_bar(PCIDevice *d, int reg)
> > > +int pci_bar(PCIDevice *d, int reg)
> 
> 
> Please add a comment
> 
> /*
>  * Should not normally be used by devices. For use by
>  * sPAPR target where QEMU emulates firmware.
>  */
> 
> > >  {
> > >      uint8_t type;
> > >  
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index be2d9b8..1a4e0be 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -332,6 +332,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f);
> > >  int pci_device_load(PCIDevice *s, QEMUFile *f);
> > >  MemoryRegion *pci_address_space(PCIDevice *dev);
> > >  MemoryRegion *pci_address_space_io(PCIDevice *dev);
> > > +int pci_bar(PCIDevice *d, int reg);
> > >  
> > >  typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
> > >  typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> > 
> 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson