On Thu, Nov 22, 2018 at 10:58:56PM +0100, Cédric Le Goater wrote: > 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? > > yes, it should probably be under the XiveRouter you are right because > there is a direct link with the ENDT which is in the XiverRouter. > > But if I remove the chip_id field from the XiveRouter, it becomes a QOM > interface. something to ponder.
Huh? I really don't understand what you're saying here. What does
chip_id have to do with anything?
>
> >> +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)) {
> >> + 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.
>
> yes clearly. The idea was to introduce a XiveESB model handling only the
> MMIO aspects and rely on an interface to query/modify the underlying PQ bits.
> These state bits are related to a device and the ESB pages are the XIVE way
> to expose them.
>
> I left that for later. I didn't want to complexify more the XiveSource
> with a feature not used today.
>
> Thanks,
>
> C.
>
> >
> >> +
> >> + 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)
> >
>
--
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
