Re: [Qemu-ppc] [PATCH v8 14/16] pci: make pci_bar useable outside pci.c

[Michael S. Tsirkin]

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 <address@hidden>
> > Reviewed-by: David Gibson <address@hidden>
> > Cc: address@hidden
> 
> 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".

I'd like a comment added, see below.
With that

Acked-by: Michael S. Tsirkin <address@hidden>

> 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