On Thu, Apr 19, 2018 at 02:42:58PM +0200, Cédric Le Goater wrote: > The 'sent' status of the LSI interrupt source is modeled with the 'P' > bit of the ESB and the assertion status of the source is maintained in > an array under the main sPAPRXive object. The type of the source is > stored in the same array for practical reasons. > > Signed-off-by: Cédric Le Goater <c...@kaod.org> > --- > hw/intc/xive.c | 54 > +++++++++++++++++++++++++++++++++++++++++++++++---- > include/hw/ppc/xive.h | 16 +++++++++++++++ > 2 files changed, 66 insertions(+), 4 deletions(-) > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index c70578759d02..060976077dd7 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -104,6 +104,21 @@ static void xive_source_notify(XiveSource *xsrc, int > srcno) > > } > > +/* > + * LSI interrupt sources use the P bit and a custom assertion flag > + */ > +static bool xive_source_lsi_trigger(XiveSource *xsrc, uint32_t srcno) > +{ > + uint8_t old_pq = xive_source_pq_get(xsrc, srcno); > + > + if (old_pq == XIVE_ESB_RESET && > + xsrc->status[srcno] & XIVE_STATUS_ASSERTED) { > + xive_source_pq_set(xsrc, srcno, XIVE_ESB_PENDING); > + return true; > + } > + return false; > +} > + > /* In a two pages ESB MMIO setting, even page is the trigger page, odd > * page is for management */ > static inline bool xive_source_is_trigger_page(hwaddr addr) > @@ -133,6 +148,13 @@ static uint64_t xive_source_esb_read(void *opaque, > hwaddr addr, unsigned size) > */ > ret = xive_source_pq_eoi(xsrc, srcno); > > + /* If the LSI source is still asserted, forward a new source > + * event notification */ > + if (xive_source_irq_is_lsi(xsrc, srcno)) { > + if (xive_source_lsi_trigger(xsrc, srcno)) { > + xive_source_notify(xsrc, srcno); > + } > + } > break; > > case XIVE_ESB_GET: > @@ -183,6 +205,14 @@ static void xive_source_esb_write(void *opaque, hwaddr > addr, > * notification > */ > notify = xive_source_pq_eoi(xsrc, srcno); > + > + /* LSI sources do not set the Q bit but they can still be > + * asserted, in which case we should forward a new source > + * event notification > + */ > + if (xive_source_irq_is_lsi(xsrc, srcno)) { > + notify = xive_source_lsi_trigger(xsrc, srcno); > + } > break; > > default: > @@ -216,8 +246,17 @@ static void xive_source_set_irq(void *opaque, int srcno, > int val) > XiveSource *xsrc = XIVE_SOURCE(opaque); > bool notify = false; > > - if (val) { > - notify = xive_source_pq_trigger(xsrc, srcno); > + if (xive_source_irq_is_lsi(xsrc, srcno)) { > + if (val) { > + xsrc->status[srcno] |= XIVE_STATUS_ASSERTED; > + } else { > + xsrc->status[srcno] &= ~XIVE_STATUS_ASSERTED; > + } > + notify = xive_source_lsi_trigger(xsrc, srcno); > + } else { > + if (val) { > + notify = xive_source_pq_trigger(xsrc, srcno); > + } > } > > /* Forward the source event notification for routing */ > @@ -234,13 +273,13 @@ void xive_source_pic_print_info(XiveSource *xsrc, > Monitor *mon) > xsrc->offset, xsrc->offset + xsrc->nr_irqs - 1); > for (i = 0; i < xsrc->nr_irqs; i++) { > uint8_t pq = xive_source_pq_get(xsrc, i); > - uint32_t lisn = i + xsrc->offset; > > if (pq == XIVE_ESB_OFF) { > continue; > } > > - monitor_printf(mon, " %4x %c%c\n", lisn, > + monitor_printf(mon, " %4x %s %c%c\n", i + xsrc->offset, > + xive_source_irq_is_lsi(xsrc, i) ? "LSI" : "MSI", > pq & XIVE_ESB_VAL_P ? 'P' : '-', > pq & XIVE_ESB_VAL_Q ? 'Q' : '-'); > } > @@ -249,6 +288,12 @@ void xive_source_pic_print_info(XiveSource *xsrc, > Monitor *mon) > static void xive_source_reset(DeviceState *dev) > { > XiveSource *xsrc = XIVE_SOURCE(dev); > + int i; > + > + /* Keep the IRQ type */ > + for (i = 0; i < xsrc->nr_irqs; i++) { > + xsrc->status[i] &= ~XIVE_STATUS_ASSERTED; > + } > > /* SBEs are initialized to 0b01 which corresponds to "ints off" */ > memset(xsrc->sbe, 0x55, xsrc->sbe_size); > @@ -273,6 +318,7 @@ static void xive_source_realize(DeviceState *dev, Error > **errp) > > xsrc->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, > xsrc->nr_irqs); > + xsrc->status = g_malloc0(xsrc->nr_irqs); > > /* Allocate the SBEs (State Bit Entry). 2 bits, so 4 entries per byte */ > xsrc->sbe_size = DIV_ROUND_UP(xsrc->nr_irqs, 4); > diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h > index d92a50519edf..0b76dd278d9b 100644 > --- a/include/hw/ppc/xive.h > +++ b/include/hw/ppc/xive.h > @@ -33,6 +33,9 @@ typedef struct XiveSource { > uint32_t nr_irqs; > uint32_t offset; > qemu_irq *qirqs; > +#define XIVE_STATUS_LSI 0x1 > +#define XIVE_STATUS_ASSERTED 0x2 > + uint8_t *status;
I don't love the idea of mixing configuration information (STATUS_LSI) with runtime state information (ASSERTED) in the same field. Any reason not to have these as parallel bitmaps. Come to that.. is there a compelling reason to allow any individual irq to be marked LSI or MSI, rather than using separate XiveSource objects for MSIs and LSIs? > /* PQ bits */ > uint8_t *sbe; .. and come to that is there a reason to keep the ASSERTED bit in a separate array from sbe? AFAICT the actual 2-bit-per-irq layout is never exposed to the guests. Or, even re-use the Q bit for asserted in LSIs (but report it as always 0 in the register read/write path). > @@ -127,4 +130,17 @@ uint8_t xive_source_pq_set(XiveSource *xsrc, uint32_t > srcno, uint8_t pq); > > void xive_source_pic_print_info(XiveSource *xsrc, Monitor *mon); > > +static inline bool xive_source_irq_is_lsi(XiveSource *xsrc, uint32_t srcno) > +{ > + assert(srcno < xsrc->nr_irqs); > + return xsrc->status[srcno] & XIVE_STATUS_LSI; > +} > + > +static inline void xive_source_irq_set(XiveSource *xsrc, uint32_t srcno, > + bool lsi) > +{ > + assert(srcno < xsrc->nr_irqs); > + xsrc->status[srcno] |= lsi ? XIVE_STATUS_LSI : 0; > +} > + > #endif /* PPC_XIVE_H */ -- 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
signature.asc
Description: PGP signature