diff mbox

[v8,14/16] pci: make pci_bar useable outside pci.c

Message ID 1429684100-13354-15-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth April 22, 2015, 6:28 a.m. UTC
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
---
 hw/pci/pci.c         | 2 +-
 include/hw/pci/pci.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

David Gibson April 28, 2015, 7:31 a.m. UTC | #1
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 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)
>  {
>      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);
Michael S. Tsirkin April 28, 2015, 8:37 a.m. UTC | #2
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".

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

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> 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
David Gibson April 29, 2015, 1:51 a.m. UTC | #3
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);
> > 
> 
>
diff mbox

Patch

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)
 {
     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);