| Message ID | 20171123132955.1261-12-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: Guest exploitation of the XIVE interrupt controller (POWER9) | expand |
On Thu, Nov 23, 2017 at 02:29:41PM +0100, Cédric Le Goater wrote: > The XIVE interrupt sources can have different characteristics depending > on their nature and the HW level in use. The sPAPR specs provide a set of > flags to describe them : > > - XIVE_SRC_H_INT_ESB the Event State Buffers are controlled with a > specific hcall H_INT_ESB and not with MMIO > - XIVE_SRC_LSI LSI or MSI source (ICSIRQState level) > - XIVE_SRC_TRIGGER the full function page supports trigger > - XIVE_SRC_STORE_EOI EOI can be done with a store. > > Our QEMU emulation of XIVE for the sPAPR machine gathers all sources under > a same model and provides a common source with the XIVE_SRC_TRIGGER type. > So, the above list is mostly informative apart from the XIVE_SRC_LSI flag > which will be deduced from the XIVE_STATUS_LSI flag. > > The OS retrieves this information on the source with the > H_INT_GET_SOURCE_INFO hcall. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/intc/spapr_xive.c | 4 ++++ > include/hw/ppc/spapr_xive.h | 7 +++++++ > 2 files changed, 11 insertions(+) > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > index f45f50fd017e..b1e3f8710cff 100644 > --- a/hw/intc/spapr_xive.c > +++ b/hw/intc/spapr_xive.c > @@ -368,6 +368,10 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) > /* Allocate the IVT (Interrupt Virtualization Table) */ > xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE)); > > + /* All sources are emulated under the XIVE object and share the > + * same characteristic */ > + xive->flags = XIVE_SRC_TRIGGER; You never actually use this field. And since it always has the same value, is there a point to storing it? > + > /* 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); > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h > index 84c910e62e56..7a308fb4db2b 100644 > --- a/include/hw/ppc/spapr_xive.h > +++ b/include/hw/ppc/spapr_xive.h > @@ -40,6 +40,13 @@ struct sPAPRXive { > #define XIVE_STATUS_SENT 0x4 > uint8_t *status; > > + /* Interrupt source flags */ > +#define XIVE_SRC_H_INT_ESB (1ull << (63 - 60)) > +#define XIVE_SRC_LSI (1ull << (63 - 61)) > +#define XIVE_SRC_TRIGGER (1ull << (63 - 62)) > +#define XIVE_SRC_STORE_EOI (1ull << (63 - 63)) > + uint32_t flags; > + > /* XIVE internal tables */ > XiveIVE *ivt; > uint8_t *sbe;
On 11/28/2017 06:40 AM, David Gibson wrote: > On Thu, Nov 23, 2017 at 02:29:41PM +0100, Cédric Le Goater wrote: >> The XIVE interrupt sources can have different characteristics depending >> on their nature and the HW level in use. The sPAPR specs provide a set of >> flags to describe them : >> >> - XIVE_SRC_H_INT_ESB the Event State Buffers are controlled with a >> specific hcall H_INT_ESB and not with MMIO >> - XIVE_SRC_LSI LSI or MSI source (ICSIRQState level) >> - XIVE_SRC_TRIGGER the full function page supports trigger >> - XIVE_SRC_STORE_EOI EOI can be done with a store. >> >> Our QEMU emulation of XIVE for the sPAPR machine gathers all sources under >> a same model and provides a common source with the XIVE_SRC_TRIGGER type. >> So, the above list is mostly informative apart from the XIVE_SRC_LSI flag >> which will be deduced from the XIVE_STATUS_LSI flag. >> >> The OS retrieves this information on the source with the >> H_INT_GET_SOURCE_INFO hcall. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/intc/spapr_xive.c | 4 ++++ >> include/hw/ppc/spapr_xive.h | 7 +++++++ >> 2 files changed, 11 insertions(+) >> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >> index f45f50fd017e..b1e3f8710cff 100644 >> --- a/hw/intc/spapr_xive.c >> +++ b/hw/intc/spapr_xive.c >> @@ -368,6 +368,10 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) >> /* Allocate the IVT (Interrupt Virtualization Table) */ >> xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE)); >> >> + /* All sources are emulated under the XIVE object and share the >> + * same characteristic */ >> + xive->flags = XIVE_SRC_TRIGGER; > > You never actually use this field. And since it always has the same > value, is there a point to storing it? yep. not much. This is a left over. I will keep the defines and move the value to the XIVE hcall layer where it is used. Thanks, C. >> + >> /* 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); >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h >> index 84c910e62e56..7a308fb4db2b 100644 >> --- a/include/hw/ppc/spapr_xive.h >> +++ b/include/hw/ppc/spapr_xive.h >> @@ -40,6 +40,13 @@ struct sPAPRXive { >> #define XIVE_STATUS_SENT 0x4 >> uint8_t *status; >> >> + /* Interrupt source flags */ >> +#define XIVE_SRC_H_INT_ESB (1ull << (63 - 60)) >> +#define XIVE_SRC_LSI (1ull << (63 - 61)) >> +#define XIVE_SRC_TRIGGER (1ull << (63 - 62)) >> +#define XIVE_SRC_STORE_EOI (1ull << (63 - 63)) >> + uint32_t flags; >> + >> /* XIVE internal tables */ >> XiveIVE *ivt; >> uint8_t *sbe; >
On Tue, 2017-11-28 at 17:40 +1100, David Gibson wrote: > > @@ -368,6 +368,10 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) > > /* Allocate the IVT (Interrupt Virtualization Table) */ > > xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE)); > > > > + /* All sources are emulated under the XIVE object and share the > > + * same characteristic */ > > + xive->flags = XIVE_SRC_TRIGGER; > > You never actually use this field. And since it always has the same > value, is there a point to storing it? Some HW sources don't have it, so with pass-through maybe... Cheers, Ben
On 12/02/2017 03:24 PM, Benjamin Herrenschmidt wrote: > On Tue, 2017-11-28 at 17:40 +1100, David Gibson wrote: >>> @@ -368,6 +368,10 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) >>> /* Allocate the IVT (Interrupt Virtualization Table) */ >>> xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE)); >>> >>> + /* All sources are emulated under the XIVE object and share the >>> + * same characteristic */ >>> + xive->flags = XIVE_SRC_TRIGGER; >> >> You never actually use this field. And since it always has the same >> value, is there a point to storing it? > > Some HW sources don't have it, so with pass-through maybe... Hmm, yes. So, the current design for sPAPR handles all sources under the same XIVE object with a global memory region for all the ESBs. The first RFC had a mechanism to register source objects into the XIVE main one, allocating the IRQs per source and mapping the ESBs in the overall region. A bit like OPAL does. I then simplified for the sake of clarity and merged everything under the same XIVE object. Shall I reintroduce multiples sources support ? and provide a default one for IPIs and virtual devices of the machine. Thanks, C.
On Sat, 2017-12-02 at 15:38 +0100, Cédric Le Goater wrote: > Hmm, yes. So, the current design for sPAPR handles all sources > under the same XIVE object with a global memory region for all > the ESBs. > > The first RFC had a mechanism to register source objects into > the XIVE main one, allocating the IRQs per source and mapping > the ESBs in the overall region. A bit like OPAL does. I then > simplified for the sake of clarity and merged everything under > the same XIVE object. > > Shall I reintroduce multiples sources support ? and provide a > default one for IPIs and virtual devices of the machine. That or you need state bits ;-) Cheers, Ben.
On 12/02/2017 03:48 PM, Benjamin Herrenschmidt wrote: > On Sat, 2017-12-02 at 15:38 +0100, Cédric Le Goater wrote: >> Hmm, yes. So, the current design for sPAPR handles all sources >> under the same XIVE object with a global memory region for all >> the ESBs. >> >> The first RFC had a mechanism to register source objects into >> the XIVE main one, allocating the IRQs per source and mapping >> the ESBs in the overall region. A bit like OPAL does. I then >> simplified for the sake of clarity and merged everything under >> the same XIVE object. >> >> Shall I reintroduce multiples sources support ? and provide a >> default one for IPIs and virtual devices of the machine. > > That or you need state bits ;-) yeah. I started with state bits and thought I could hack the PQ ones but that's wrong. Thanks, C.
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index f45f50fd017e..b1e3f8710cff 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -368,6 +368,10 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) /* Allocate the IVT (Interrupt Virtualization Table) */ xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE)); + /* All sources are emulated under the XIVE object and share the + * same characteristic */ + xive->flags = XIVE_SRC_TRIGGER; + /* 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); diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index 84c910e62e56..7a308fb4db2b 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -40,6 +40,13 @@ struct sPAPRXive { #define XIVE_STATUS_SENT 0x4 uint8_t *status; + /* Interrupt source flags */ +#define XIVE_SRC_H_INT_ESB (1ull << (63 - 60)) +#define XIVE_SRC_LSI (1ull << (63 - 61)) +#define XIVE_SRC_TRIGGER (1ull << (63 - 62)) +#define XIVE_SRC_STORE_EOI (1ull << (63 - 63)) + uint32_t flags; + /* XIVE internal tables */ XiveIVE *ivt; uint8_t *sbe;
The XIVE interrupt sources can have different characteristics depending on their nature and the HW level in use. The sPAPR specs provide a set of flags to describe them : - XIVE_SRC_H_INT_ESB the Event State Buffers are controlled with a specific hcall H_INT_ESB and not with MMIO - XIVE_SRC_LSI LSI or MSI source (ICSIRQState level) - XIVE_SRC_TRIGGER the full function page supports trigger - XIVE_SRC_STORE_EOI EOI can be done with a store. Our QEMU emulation of XIVE for the sPAPR machine gathers all sources under a same model and provides a common source with the XIVE_SRC_TRIGGER type. So, the above list is mostly informative apart from the XIVE_SRC_LSI flag which will be deduced from the XIVE_STATUS_LSI flag. The OS retrieves this information on the source with the H_INT_GET_SOURCE_INFO hcall. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/spapr_xive.c | 4 ++++ include/hw/ppc/spapr_xive.h | 7 +++++++ 2 files changed, 11 insertions(+)