On 11/22/18 6:13 AM, David Gibson wrote: > On Fri, Nov 16, 2018 at 11:56:59AM +0100, Cédric Le Goater wrote: >> The Event Notification Descriptor also contains two Event State >> Buffers providing further coalescing of interrupts, one for the >> notification event (ESn) and one for the escalation events (ESe). A >> MMIO page is assigned for each to control the EOI through loads >> only. Stores are not allowed. >> >> The END ESBs are modeled through an object resembling the 'XiveSource' >> It is stateless as the END state bits are backed into the XiveEND >> structure under the XiveRouter and the MMIO accesses follow the same >> rules as for the standard source ESBs. >> >> END ESBs are not supported by the Linux drivers neither on OPAL nor on >> sPAPR. Nevetherless, it provides a mean to study the question in the >> future and validates a bit more the XIVE model. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> --- >> include/hw/ppc/xive.h | 20 ++++++ >> hw/intc/xive.c | 160 +++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 178 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h >> index ce62aaf28343..24301bf2076d 100644 >> --- a/include/hw/ppc/xive.h >> +++ b/include/hw/ppc/xive.h >> @@ -208,6 +208,26 @@ int xive_router_get_end(XiveRouter *xrtr, uint8_t >> end_blk, uint32_t end_idx, >> int xive_router_set_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx, >> XiveEND *end); >> >> +/* >> + * XIVE END ESBs >> + */ >> + >> +#define TYPE_XIVE_END_SOURCE "xive-end-source" >> +#define XIVE_END_SOURCE(obj) \ >> + OBJECT_CHECK(XiveENDSource, (obj), TYPE_XIVE_END_SOURCE) > > Is there a particular reason to make this a full QOM object, rather > than just embedding it in the XiveRouter?
Coming back on this question because removing the chip_id from the router is a problem for the END triggering. At least with the current design. See below for the comment. >> +typedef struct XiveENDSource { >> + SysBusDevice parent; >> + >> + uint32_t nr_ends; >> + >> + /* ESB memory region */ >> + uint32_t esb_shift; >> + MemoryRegion esb_mmio; >> + >> + XiveRouter *xrtr; >> +} XiveENDSource; >> + >> /* >> * For legacy compatibility, the exceptions define up to 256 different >> * priorities. P9 implements only 9 levels : 8 active levels [0 - 7] >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >> index 9cb001e7b540..5a8882d47a98 100644 >> --- a/hw/intc/xive.c >> +++ b/hw/intc/xive.c >> @@ -622,8 +622,18 @@ static void xive_router_end_notify(XiveRouter *xrtr, >> uint8_t end_blk, >> * even futher coalescing in the Router >> */ >> if (!(end.w0 & END_W0_UCOND_NOTIFY)) { >> - qemu_log_mask(LOG_UNIMP, "XIVE: !UCOND_NOTIFY not implemented\n"); >> - return; >> + uint8_t pq = GETFIELD(END_W1_ESn, end.w1); >> + bool notify = xive_esb_trigger(&pq); >> + >> + if (pq != GETFIELD(END_W1_ESn, end.w1)) { >> + end.w1 = SETFIELD(END_W1_ESn, end.w1, pq); >> + xive_router_set_end(xrtr, end_blk, end_idx, &end); >> + } >> + >> + /* ESn[Q]=1 : end of notification */ >> + if (!notify) { >> + return; >> + } >> } >> >> /* >> @@ -706,6 +716,151 @@ void xive_eas_pic_print_info(XiveEAS *eas, uint32_t >> lisn, Monitor *mon) >> (uint32_t) GETFIELD(EAS_END_DATA, eas->w)); >> } >> >> +/* >> + * END ESB MMIO loads >> + */ >> +static uint64_t xive_end_source_read(void *opaque, hwaddr addr, unsigned >> size) >> +{ >> + XiveENDSource *xsrc = XIVE_END_SOURCE(opaque); >> + XiveRouter *xrtr = xsrc->xrtr; >> + uint32_t offset = addr & 0xFFF; >> + uint8_t end_blk; >> + uint32_t end_idx; >> + XiveEND end; >> + uint32_t end_esmask; >> + uint8_t pq; >> + uint64_t ret = -1; >> + >> + end_blk = xrtr->chip_id; >> + end_idx = addr >> (xsrc->esb_shift + 1); >> + if (xive_router_get_end(xrtr, end_blk, end_idx, &end)) { The current END accessors require a block identifier, hence xrtr->chip_id, but in this case, we don't really need it because we are using the ENDT local to the router/chip. I don't know how to handle simply this case without keeping chip_id :/ C. >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No END %x/%x\n", end_blk, >> + end_idx); >> + return -1; >> + } >> + >> + if (!(end.w0 & END_W0_VALID)) { >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: END %x/%x is invalid\n", >> + end_blk, end_idx); >> + return -1; >> + } >> + >> + end_esmask = addr_is_even(addr, xsrc->esb_shift) ? END_W1_ESn : >> END_W1_ESe; >> + pq = GETFIELD(end_esmask, end.w1); >> + >> + switch (offset) { >> + case XIVE_ESB_LOAD_EOI ... XIVE_ESB_LOAD_EOI + 0x7FF: >> + ret = xive_esb_eoi(&pq); >> + >> + /* Forward the source event notification for routing ?? */ >> + break; >> + >> + case XIVE_ESB_GET ... XIVE_ESB_GET + 0x3FF: >> + ret = pq; >> + break; >> + >> + case XIVE_ESB_SET_PQ_00 ... XIVE_ESB_SET_PQ_00 + 0x0FF: >> + case XIVE_ESB_SET_PQ_01 ... XIVE_ESB_SET_PQ_01 + 0x0FF: >> + case XIVE_ESB_SET_PQ_10 ... XIVE_ESB_SET_PQ_10 + 0x0FF: >> + case XIVE_ESB_SET_PQ_11 ... XIVE_ESB_SET_PQ_11 + 0x0FF: >> + ret = xive_esb_set(&pq, (offset >> 8) & 0x3); >> + break; >> + default: >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid END ESB load addr >> %d\n", >> + offset); >> + return -1; >> + } >> + >> + if (pq != GETFIELD(end_esmask, end.w1)) { >> + end.w1 = SETFIELD(end_esmask, end.w1, pq); >> + xive_router_set_end(xrtr, end_blk, end_idx, &end); >> + } > > We can probably share some more code with XiveSource here, but that's > something that can be refined later. > >> + >> + return ret; >> +} >> + >> +/* >> + * END ESB MMIO stores are invalid >> + */ >> +static void xive_end_source_write(void *opaque, hwaddr addr, >> + uint64_t value, unsigned size) >> +{ >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid ESB write addr 0x%" >> + HWADDR_PRIx"\n", addr); >> +} >> + >> +static const MemoryRegionOps xive_end_source_ops = { >> + .read = xive_end_source_read, >> + .write = xive_end_source_write, >> + .endianness = DEVICE_BIG_ENDIAN, >> + .valid = { >> + .min_access_size = 8, >> + .max_access_size = 8, >> + }, >> + .impl = { >> + .min_access_size = 8, >> + .max_access_size = 8, >> + }, >> +}; >> + >> +static void xive_end_source_realize(DeviceState *dev, Error **errp) >> +{ >> + XiveENDSource *xsrc = XIVE_END_SOURCE(dev); >> + Object *obj; >> + Error *local_err = NULL; >> + >> + obj = object_property_get_link(OBJECT(dev), "xive", &local_err); >> + if (!obj) { >> + error_propagate(errp, local_err); >> + error_prepend(errp, "required link 'xive' not found: "); >> + return; >> + } >> + >> + xsrc->xrtr = XIVE_ROUTER(obj); >> + >> + if (!xsrc->nr_ends) { >> + error_setg(errp, "Number of interrupt needs to be greater than 0"); >> + return; >> + } >> + >> + if (xsrc->esb_shift != XIVE_ESB_4K && >> + xsrc->esb_shift != XIVE_ESB_64K) { >> + error_setg(errp, "Invalid ESB shift setting"); >> + return; >> + } >> + >> + /* >> + * Each END is assigned an even/odd pair of MMIO pages, the even page >> + * manages the ESn field while the odd page manages the ESe field. >> + */ >> + memory_region_init_io(&xsrc->esb_mmio, OBJECT(xsrc), >> + &xive_end_source_ops, xsrc, "xive.end", >> + (1ull << (xsrc->esb_shift + 1)) * xsrc->nr_ends); >> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &xsrc->esb_mmio); >> +} >> + >> +static Property xive_end_source_properties[] = { >> + DEFINE_PROP_UINT32("nr-ends", XiveENDSource, nr_ends, 0), >> + DEFINE_PROP_UINT32("shift", XiveENDSource, esb_shift, XIVE_ESB_64K), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> +static void xive_end_source_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->desc = "XIVE END Source"; >> + dc->props = xive_end_source_properties; >> + dc->realize = xive_end_source_realize; >> +} >> + >> +static const TypeInfo xive_end_source_info = { >> + .name = TYPE_XIVE_END_SOURCE, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(XiveENDSource), >> + .class_init = xive_end_source_class_init, >> +}; >> + >> /* >> * XIVE Fabric >> */ >> @@ -720,6 +875,7 @@ static void xive_register_types(void) >> type_register_static(&xive_source_info); >> type_register_static(&xive_fabric_info); >> type_register_static(&xive_router_info); >> + type_register_static(&xive_end_source_info); >> } >> >> type_init(xive_register_types) >