From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv7bJ-0001e3-4W for qemu-devel@nongnu.org; Fri, 14 Oct 2016 14:53:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bv7bD-0001na-5o for qemu-devel@nongnu.org; Fri, 14 Oct 2016 14:53:00 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:46854) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bv7bC-0001nP-U3 for qemu-devel@nongnu.org; Fri, 14 Oct 2016 14:52:55 -0400 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9EInPDB091790 for ; Fri, 14 Oct 2016 14:52:54 -0400 Received: from e36.co.us.ibm.com (e36.co.us.ibm.com [32.97.110.154]) by mx0a-001b2d01.pphosted.com with ESMTP id 262ytg6be5-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 14 Oct 2016 14:52:53 -0400 Received: from localhost by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 14 Oct 2016 12:52:53 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20161014084620.GD28693@in.ibm.com> References: <1476314039-9520-1-git-send-email-mdroth@linux.vnet.ibm.com> <1476314039-9520-9-git-send-email-mdroth@linux.vnet.ibm.com> <20161014084620.GD28693@in.ibm.com> Date: Fri, 14 Oct 2016 13:51:19 -0500 Message-Id: <20161014185119.29726.4662@loki> Subject: Re: [Qemu-devel] [PATCH 08/11] spapr_events: add support for dedicated hotplug event source List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: bharata@linux.vnet.ibm.comBharata B Rao Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, david@gibson.dropbear.id.au, nfont@linux.vnet.ibm.com, jallen@linux.vnet.ibm.com Quoting Bharata B Rao (2016-10-14 03:46:20) > On Wed, Oct 12, 2016 at 06:13:56PM -0500, Michael Roth wrote: > > Hotplug events were previously delivered using an EPOW interrupt > > and were queued by linux guests into a circular buffer. For traditional > > EPOW events like shutdown/resets, this isn't an issue, but for hotplug > > events there are cases where this buffer can be exhausted, resulting > > in the loss of hotplug events, resets, etc. > > = > > Newer-style hotplug event are delivered using a dedicated event source. > > We enable this in supported guests by adding standard an additional > > event source in the guest device-tree via /event-sources, and, if > > the guest advertises support for the newer-style hotplug events, > > using the corresponding interrupt to signal the available of > > hotplug/unplug events. > > = > > Signed-off-by: Michael Roth > > --- > > hw/ppc/spapr.c | 10 ++-- > > hw/ppc/spapr_events.c | 148 ++++++++++++++++++++++++++++++++++++++---= -------- > > include/hw/ppc/spapr.h | 3 +- > > 3 files changed, 120 insertions(+), 41 deletions(-) > > = > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index d80a6fa..2037222 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -275,8 +275,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_ba= se, > > hwaddr initrd_size, > > hwaddr kernel_size, > > bool little_endian, > > - const char *kernel_cmdline, > > - uint32_t epow_irq) > > + const char *kernel_cmdline) > > { > > void *fdt; > > uint32_t start_prop =3D cpu_to_be32(initrd_base); > > @@ -437,7 +436,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_ba= se, > > _FDT((fdt_end_node(fdt))); > > = > > /* event-sources */ > > - spapr_events_fdt_skel(fdt, epow_irq); > > + spapr_events_fdt_skel(fdt); > > = > > /* /hypervisor node */ > > if (kvm_enabled()) { > > @@ -1944,7 +1943,7 @@ static void ppc_spapr_init(MachineState *machine) > > } > > g_free(filename); > > = > > - /* Set up EPOW events infrastructure */ > > + /* Set up RTAS event infrastructure */ > > spapr_events_init(spapr); > > = > > /* Set up the RTC RTAS interfaces */ > > @@ -2076,8 +2075,7 @@ static void ppc_spapr_init(MachineState *machine) > > /* Prepare the device tree */ > > spapr->fdt_skel =3D spapr_create_fdt_skel(initrd_base, initrd_size, > > kernel_size, kernel_le, > > - kernel_cmdline, > > - spapr->check_exception_irq= ); > > + kernel_cmdline); > > assert(spapr->fdt_skel !=3D NULL); > > = > > /* used by RTAS */ > > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > > index 4c7b6ae..f8bbec6 100644 > > --- a/hw/ppc/spapr_events.c > > +++ b/hw/ppc/spapr_events.c > > @@ -40,6 +40,7 @@ > > #include "hw/ppc/spapr_drc.h" > > #include "qemu/help_option.h" > > #include "qemu/bcd.h" > > +#include "hw/ppc/spapr_ovec.h" > > #include > > = > > struct rtas_error_log { > > @@ -206,28 +207,104 @@ struct hp_log_full { > > struct rtas_event_log_v6_hp hp; > > } QEMU_PACKED; > > = > > -#define EVENT_MASK_INTERNAL_ERRORS 0x80000000 > > -#define EVENT_MASK_EPOW 0x40000000 > > -#define EVENT_MASK_HOTPLUG 0x10000000 > > -#define EVENT_MASK_IO 0x08000000 > > +typedef enum EventClassIndex { > > + EVENT_CLASS_INTERNAL_ERRORS =3D 0, > > + EVENT_CLASS_EPOW =3D 1, > > + EVENT_CLASS_RESERVED =3D 2, > > + EVENT_CLASS_HOT_PLUG =3D 3, > > + EVENT_CLASS_IO =3D 4, > > + EVENT_CLASS_MAX > > +} EventClassIndex; > > + > > +#define EVENT_CLASS_MASK(index) (1 << (31 - index)) > > + > > +typedef struct EventSource { > > + const char *name; > > + int irq; > > + uint32_t mask; > > + bool enabled; > > +} EventSource; > > + > > +static EventSource event_source[EVENT_CLASS_MAX] =3D { > > + [EVENT_CLASS_INTERNAL_ERRORS] =3D { .name =3D "internal-erro= rs", }, > > + [EVENT_CLASS_EPOW] =3D { .name =3D "epow-events",= }, > > + [EVENT_CLASS_HOT_PLUG] =3D { .name =3D "hot-plug-even= ts", }, > > + [EVENT_CLASS_IO] =3D { .name =3D "ibm,io-events= ", }, > > +}; > > + > > +static void rtas_event_source_register(EventClassIndex index, int irq) > > +{ > > + /* we only support 1 irq per event class at the moment */ > > + g_assert(!event_source[index].enabled); > > + event_source[index].irq =3D irq; > > + event_source[index].mask =3D EVENT_CLASS_MASK(index); > > + event_source[index].enabled =3D true; > > +} > > = > > -void spapr_events_fdt_skel(void *fdt, uint32_t check_exception_irq) > > +void spapr_events_fdt_skel(void *fdt) > > { > > - uint32_t irq_ranges[] =3D {cpu_to_be32(check_exception_irq), cpu_t= o_be32(1)}; > > - uint32_t interrupts[] =3D {cpu_to_be32(check_exception_irq), 0}; > > + uint32_t irq_ranges[EVENT_CLASS_MAX * 2]; > > + int i, count =3D 0; > > = > > _FDT((fdt_begin_node(fdt, "event-sources"))); > > = > > + for (i =3D 0, count =3D 0; i < EVENT_CLASS_MAX; i++) { > > + /* TODO: what does 0 entail? */ > > + uint32_t interrupts[] =3D { cpu_to_be32(event_source[i].irq), = 0 }; > > + > > + if (!event_source[i].enabled) { > > + continue; > > + } > > + > > + _FDT((fdt_begin_node(fdt, event_source[i].name))); > > + _FDT((fdt_property(fdt, "interrupts", interrupts, sizeof(inter= rupts)))); > > + _FDT((fdt_end_node(fdt))); > > + > > + irq_ranges[count++] =3D interrupts[0]; > > + irq_ranges[count++] =3D cpu_to_be32(1); > > + } > > + > > + /* TODO: confirm the count is the last expected element */ > > + irq_ranges[count] =3D cpu_to_be32(count); > > + count++; > > + > > _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0))); > > _FDT((fdt_property_cell(fdt, "#interrupt-cells", 2))); > > _FDT((fdt_property(fdt, "interrupt-ranges", > > - irq_ranges, sizeof(irq_ranges)))); > > + irq_ranges, count * sizeof(uint32_t)))); > > = > > - _FDT((fdt_begin_node(fdt, "epow-events"))); > > - _FDT((fdt_property(fdt, "interrupts", interrupts, sizeof(interrupt= s)))); > > _FDT((fdt_end_node(fdt))); > > +} > > = > > - _FDT((fdt_end_node(fdt))); > > +static const EventSource *rtas_event_log_to_source(int log_type) > > +{ > > + const EventSource *source; > > + > > + switch (log_type) { > > + case RTAS_LOG_TYPE_HOTPLUG: > > + source =3D &event_source[EVENT_CLASS_HOT_PLUG]; > > + if (event_source[EVENT_CLASS_HOT_PLUG].enabled) { > > + break; > > + } > = > In addition to the above .enabled check, shouldn't you be checking if > the guest indeed supports the dedicated hotplug interrupt source before > returning the source ? Yes, I believe you're right. I'd been relying legacy-hotplug-events=3Dtrue to test the old signalling mechanism (which leaves this event source disabled), but haven't tried PCI/CPU since adding the ovec stuff, so didn't notice those were broken with legacy-hotplug-events=3Dfalse. Will fix it up in next submission. > = > This I believe is the reason for the CPU hotplug failures I that mentioned > in reply to your 11/11 thread. I am on 4.7.x kernel which probably doesn't > support hotplug interrupt source, but QEMU ends up registering and raising > such an interrupt. > = > Regards, > Bharata.