| Message ID | 20170911171235.29331-6-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | Guest exploitation of the XIVE interrupt controller (POWER9) | expand |
On Mon, Sep 11, 2017 at 07:12:19PM +0200, Cédric Le Goater wrote: > The number of IPIs is deduced from the max number of CPUs the guest > supports and the IRQ numbers for the IPIs are allocated from the top > of the IRQ number space to reduce conflict with other IRQ numbers > allocated by the devices. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> This is more ick associated with implementing XIVE in terms of XICS. We shouldn't need to "allocate" IRQs for the IPIs - they should just be a fixed set. And we certainly shouldn't need to set the XICS irq type for XIVE irqs. > --- > hw/intc/spapr_xive.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > index 1681affb0848..52c32f588d6d 100644 > --- a/hw/intc/spapr_xive.c > +++ b/hw/intc/spapr_xive.c > @@ -58,6 +58,7 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) > sPAPRXive *xive = SPAPR_XIVE(dev); > Object *obj; > Error *err = NULL; > + int i; > > if (!xive->nr_targets) { > error_setg(errp, "Number of interrupt targets needs to be greater 0"); > @@ -80,6 +81,11 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) > > xive->ics = ICS_BASE(obj); > > + /* Allocate the last IRQ numbers for the IPIs */ > + for (i = xive->nr_irqs - xive->nr_targets; i < xive->nr_irqs; i++) { > + ics_set_irq_type(xive->ics, i, false); > + } > + > /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */ > xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4); > xive->sbe = g_malloc0(xive->sbe_size);
On 09/19/2017 04:45 AM, David Gibson wrote: > On Mon, Sep 11, 2017 at 07:12:19PM +0200, Cédric Le Goater wrote: >> The number of IPIs is deduced from the max number of CPUs the guest >> supports and the IRQ numbers for the IPIs are allocated from the top >> of the IRQ number space to reduce conflict with other IRQ numbers >> allocated by the devices. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > This is more ick associated with implementing XIVE in terms of XICS. > We shouldn't need to "allocate" IRQs for the IPIs - they should just > be a fixed set. They are allocated at the right beginning so we can consider them fixed I suppose. > And we certainly shouldn't need to set the XICS irq type for XIVE irqs. This is because, in this patchset, XIVE and XICS use the same IRQ allocator which happens to be the ICSIRQState array of XICS. yes, this is ugly but we are identifying the different constraints. We should be doing the same with a common interrupt source, that is to allocate some IRQ numbers for the IPIs. C. >> --- >> hw/intc/spapr_xive.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >> index 1681affb0848..52c32f588d6d 100644 >> --- a/hw/intc/spapr_xive.c >> +++ b/hw/intc/spapr_xive.c >> @@ -58,6 +58,7 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) >> sPAPRXive *xive = SPAPR_XIVE(dev); >> Object *obj; >> Error *err = NULL; >> + int i; >> >> if (!xive->nr_targets) { >> error_setg(errp, "Number of interrupt targets needs to be greater 0"); >> @@ -80,6 +81,11 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) >> >> xive->ics = ICS_BASE(obj); >> >> + /* Allocate the last IRQ numbers for the IPIs */ >> + for (i = xive->nr_irqs - xive->nr_targets; i < xive->nr_irqs; i++) { >> + ics_set_irq_type(xive->ics, i, false); >> + } >> + >> /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */ >> xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4); >> xive->sbe = g_malloc0(xive->sbe_size); >
On Tue, Sep 19, 2017 at 04:52:10PM +0200, Cédric Le Goater wrote: > On 09/19/2017 04:45 AM, David Gibson wrote: > > On Mon, Sep 11, 2017 at 07:12:19PM +0200, Cédric Le Goater wrote: > >> The number of IPIs is deduced from the max number of CPUs the guest > >> supports and the IRQ numbers for the IPIs are allocated from the top > >> of the IRQ number space to reduce conflict with other IRQ numbers > >> allocated by the devices. > >> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > This is more ick associated with implementing XIVE in terms of XICS. > > We shouldn't need to "allocate" IRQs for the IPIs - they should just > > be a fixed set. > > They are allocated at the right beginning so we can consider them > fixed I suppose. > > > And we certainly shouldn't need to set the XICS irq type for XIVE irqs. > > This is because, in this patchset, XIVE and XICS use the same IRQ > allocator which happens to be the ICSIRQState array of XICS. yes, > this is ugly but we are identifying the different constraints. Yeah, as I said in the other mail, I think trying to support both immediately is making a mess of the XIVE design. Let's get it working as a machine option first, then worry about CAS and migration.
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 1681affb0848..52c32f588d6d 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -58,6 +58,7 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) sPAPRXive *xive = SPAPR_XIVE(dev); Object *obj; Error *err = NULL; + int i; if (!xive->nr_targets) { error_setg(errp, "Number of interrupt targets needs to be greater 0"); @@ -80,6 +81,11 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) xive->ics = ICS_BASE(obj); + /* Allocate the last IRQ numbers for the IPIs */ + for (i = xive->nr_irqs - xive->nr_targets; i < xive->nr_irqs; i++) { + ics_set_irq_type(xive->ics, i, false); + } + /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */ xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4); xive->sbe = g_malloc0(xive->sbe_size);
The number of IPIs is deduced from the max number of CPUs the guest supports and the IRQ numbers for the IPIs are allocated from the top of the IRQ number space to reduce conflict with other IRQ numbers allocated by the devices. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/spapr_xive.c | 6 ++++++ 1 file changed, 6 insertions(+)