Patchwork [3/3] spapr: make irq customizable via qdev

login
register
mail settings
Submitter Paolo Bonzini
Date May 24, 2011, 11:45 a.m.
Message ID <1306237507-19189-4-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/97150/
State New
Headers show

Comments

Paolo Bonzini - May 24, 2011, 11:45 a.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr_vio.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
David Gibson - May 24, 2011, 10:14 p.m.
On Tue, May 24, 2011 at 01:45:07PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/spapr_vio.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
> index be535d6..fee4c46 100644
> --- a/hw/spapr_vio.c
> +++ b/hw/spapr_vio.c
> @@ -52,6 +52,10 @@
>  static struct BusInfo spapr_vio_bus_info = {
>      .name       = "spapr-vio",
>      .size       = sizeof(VIOsPAPRBus),
> +    .props = (Property[]) {
> +        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
> +        DEFINE_PROP_END_OF_LIST(),
> +    },
>  };

I don't see what the point of this is.  Absolute irq numbers have no
special meaning in the XICS context, and the guest kernel will remap
them to virtual irqs anyway.


>  VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
> @@ -604,7 +608,9 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
>      }
>  
>      dev->qdev.id = id;
> -    dev->vio_irq_num = bus->irq++;
> +    if (!dev->vio_irq_num) {
> +        dev->vio_irq_num = bus->irq++;
> +    }
>      dev->qirq = xics_find_qirq(spapr->icp, dev->vio_irq_num);
>  
>      rtce_init(dev);
Paolo Bonzini - May 25, 2011, 7:30 a.m.
On 05/25/2011 12:14 AM, David Gibson wrote:
> I don't see what the point of this is.  Absolute irq numbers have no
> special meaning in the XICS context, and the guest kernel will remap
> them to virtual irqs anyway.

It allows you to see the irq in "info qtree" for example.

Paolo
Markus Armbruster - May 25, 2011, 3:13 p.m.
David Gibson <david@gibson.dropbear.id.au> writes:

> On Tue, May 24, 2011 at 01:45:07PM +0200, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/spapr_vio.c |    8 +++++++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
>> index be535d6..fee4c46 100644
>> --- a/hw/spapr_vio.c
>> +++ b/hw/spapr_vio.c
>> @@ -52,6 +52,10 @@
>>  static struct BusInfo spapr_vio_bus_info = {
>>      .name       = "spapr-vio",
>>      .size       = sizeof(VIOsPAPRBus),
>> +    .props = (Property[]) {
>> +        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
>> +        DEFINE_PROP_END_OF_LIST(),
>> +    },
>>  };
>
> I don't see what the point of this is.  Absolute irq numbers have no
> special meaning in the XICS context, and the guest kernel will remap
> them to virtual irqs anyway.

Are the irq numbers guest-visible?  If yes, a property may be required
to keep them stable across migration.  Especially when hot-plug comes
into play.

[...]
David Gibson - May 30, 2011, 3:16 a.m.
On Wed, May 25, 2011 at 09:30:22AM +0200, Paolo Bonzini wrote:
> On 05/25/2011 12:14 AM, David Gibson wrote:
> >I don't see what the point of this is.  Absolute irq numbers have no
> >special meaning in the XICS context, and the guest kernel will remap
> >them to virtual irqs anyway.
> 
> It allows you to see the irq in "info qtree" for example.

Hm, I see.
David Gibson - May 30, 2011, 3:17 a.m.
On Wed, May 25, 2011 at 05:13:40PM +0200, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Tue, May 24, 2011 at 01:45:07PM +0200, Paolo Bonzini wrote:
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Alexander Graf <agraf@suse.de>
> >> Cc: David Gibson <david@gibson.dropbear.id.au>
> >> ---
> >>  hw/spapr_vio.c |    8 +++++++-
> >>  1 files changed, 7 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
> >> index be535d6..fee4c46 100644
> >> --- a/hw/spapr_vio.c
> >> +++ b/hw/spapr_vio.c
> >> @@ -52,6 +52,10 @@
> >>  static struct BusInfo spapr_vio_bus_info = {
> >>      .name       = "spapr-vio",
> >>      .size       = sizeof(VIOsPAPRBus),
> >> +    .props = (Property[]) {
> >> +        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
> >> +        DEFINE_PROP_END_OF_LIST(),
> >> +    },
> >>  };
> >
> > I don't see what the point of this is.  Absolute irq numbers have no
> > special meaning in the XICS context, and the guest kernel will remap
> > them to virtual irqs anyway.
> 
> Are the irq numbers guest-visible?

Yes.

>  If yes, a property may be required
> to keep them stable across migration.  Especially when hot-plug comes
> into play.

Ah, yes, that's a point.

Patch

diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index be535d6..fee4c46 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -52,6 +52,10 @@ 
 static struct BusInfo spapr_vio_bus_info = {
     .name       = "spapr-vio",
     .size       = sizeof(VIOsPAPRBus),
+    .props = (Property[]) {
+        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
@@ -604,7 +608,9 @@  static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
     }
 
     dev->qdev.id = id;
-    dev->vio_irq_num = bus->irq++;
+    if (!dev->vio_irq_num) {
+        dev->vio_irq_num = bus->irq++;
+    }
     dev->qirq = xics_find_qirq(spapr->icp, dev->vio_irq_num);
 
     rtce_init(dev);