From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44133) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQBF0-0006DZ-SR for qemu-devel@nongnu.org; Tue, 24 Feb 2015 03:53:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQBEy-0001Je-GK for qemu-devel@nongnu.org; Tue, 24 Feb 2015 03:53:18 -0500 Date: Tue, 24 Feb 2015 17:49:42 +1100 From: David Gibson Message-ID: <20150224064942.GR4536@voom.redhat.com> References: <1424096872-29868-1-git-send-email-mdroth@linux.vnet.ibm.com> <1424096872-29868-9-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="magLDk5D4XGaUXcd" Content-Disposition: inline In-Reply-To: <1424096872-29868-9-git-send-email-mdroth@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v5 08/16] spapr_events: re-use EPOW event infrastructure for hotplug events List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, ncmike@ncultra.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, bharata.rao@gmail.com, nfont@linux.vnet.ibm.com --magLDk5D4XGaUXcd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 16, 2015 at 08:27:44AM -0600, Michael Roth wrote: > From: Nathan Fontenot >=20 > This extends the data structures currently used to report EPOW events to > guests via the check-exception RTAS interfaces to also include event types > for hotplug/unplug events. >=20 > This is currently undocumented and being finalized for inclusion in PAPR > specification, but we implement this here as an extension for guest > userspace tools to implement (existing guest kernels simply log these > events via a sysfs interface that's read by rtas_errd, and current > versions of rtas_errd/powerpc-utils already support the use of this > mechanism for initiating hotplug operations). >=20 > We also add support for queues of pending RTAS events, since in the > case of hotplug there's chance for multiple events being in-flight > at any point in time. >=20 > Signed-off-by: Nathan Fontenot > Signed-off-by: Michael Roth > --- > hw/ppc/spapr.c | 2 +- > hw/ppc/spapr_events.c | 271 ++++++++++++++++++++++++++++++++++++++++---= ------ > include/hw/ppc/spapr.h | 5 +- > 3 files changed, 226 insertions(+), 52 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index bfacc9d..861107e 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1640,7 +1640,7 @@ static void ppc_spapr_init(MachineState *machine) > spapr->fdt_skel =3D spapr_create_fdt_skel(initrd_base, initrd_size, > kernel_size, kernel_le, > boot_device, kernel_cmdline, > - spapr->epow_irq); > + spapr->check_exception_irq); > assert(spapr->fdt_skel !=3D NULL); > } > =20 > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index 283e96b..0eeb1d8 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -32,6 +32,9 @@ > =20 > #include "hw/ppc/spapr.h" > #include "hw/ppc/spapr_vio.h" > +#include "hw/pci/pci.h" > +#include "hw/pci-host/spapr.h" > +#include "hw/ppc/spapr_drc.h" > =20 > #include > =20 > @@ -77,6 +80,7 @@ struct rtas_error_log { > #define RTAS_LOG_TYPE_ECC_UNCORR 0x00000009 > #define RTAS_LOG_TYPE_ECC_CORR 0x0000000a > #define RTAS_LOG_TYPE_EPOW 0x00000040 > +#define RTAS_LOG_TYPE_HOTPLUG 0x000000e5 > uint32_t extended_length; > } QEMU_PACKED; > =20 > @@ -166,6 +170,38 @@ struct epow_log_full { > struct rtas_event_log_v6_epow epow; > } QEMU_PACKED; > =20 > +struct rtas_event_log_v6_hp { > +#define RTAS_LOG_V6_SECTION_ID_HOTPLUG 0x4850 /* HP */ > + struct rtas_event_log_v6_section_header hdr; > + uint8_t hotplug_type; > +#define RTAS_LOG_V6_HP_TYPE_CPU 1 > +#define RTAS_LOG_V6_HP_TYPE_MEMORY 2 > +#define RTAS_LOG_V6_HP_TYPE_SLOT 3 > +#define RTAS_LOG_V6_HP_TYPE_PHB 4 > +#define RTAS_LOG_V6_HP_TYPE_PCI 5 > + uint8_t hotplug_action; > +#define RTAS_LOG_V6_HP_ACTION_ADD 1 > +#define RTAS_LOG_V6_HP_ACTION_REMOVE 2 > + uint8_t hotplug_identifier; > +#define RTAS_LOG_V6_HP_ID_DRC_NAME 1 > +#define RTAS_LOG_V6_HP_ID_DRC_INDEX 2 > +#define RTAS_LOG_V6_HP_ID_DRC_COUNT 3 > + uint8_t reserved; > + union { > + uint32_t index; > + uint32_t count; > + char name[1]; > + } drc; > +} QEMU_PACKED; > + > +struct hp_log_full { > + struct rtas_error_log hdr; > + struct rtas_event_log_v6 v6hdr; > + struct rtas_event_log_v6_maina maina; > + struct rtas_event_log_v6_mainb mainb; > + 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 > @@ -181,67 +217,84 @@ struct epow_log_full { > } \ > } while (0) > =20 > -void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq) > +void spapr_events_fdt_skel(void *fdt, uint32_t check_exception_irq) > { > - uint32_t epow_irq_ranges[] =3D {cpu_to_be32(epow_irq), cpu_to_be32(1= )}; > - uint32_t epow_interrupts[] =3D {cpu_to_be32(epow_irq), 0}; > + uint32_t irq_ranges[] =3D {cpu_to_be32(check_exception_irq), cpu_to_= be32(1)}; > + uint32_t interrupts[] =3D {cpu_to_be32(check_exception_irq), 0}; > =20 > _FDT((fdt_begin_node(fdt, "event-sources"))); > =20 > _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0))); > _FDT((fdt_property_cell(fdt, "#interrupt-cells", 2))); > _FDT((fdt_property(fdt, "interrupt-ranges", > - epow_irq_ranges, sizeof(epow_irq_ranges)))); > + irq_ranges, sizeof(irq_ranges)))); > =20 > _FDT((fdt_begin_node(fdt, "epow-events"))); > - _FDT((fdt_property(fdt, "interrupts", > - epow_interrupts, sizeof(epow_interrupts)))); > + _FDT((fdt_property(fdt, "interrupts", interrupts, sizeof(interrupts)= ))); > _FDT((fdt_end_node(fdt))); > =20 > _FDT((fdt_end_node(fdt))); > } > =20 > -static struct epow_log_full *pending_epow; > -static uint32_t next_plid; > +typedef struct EventLogEntry { > + int log_type; > + void *data; > + QTAILQ_ENTRY(EventLogEntry) next; > +} EventLogEntry; > =20 > -static void spapr_powerdown_req(Notifier *n, void *opaque) > +static struct { > + QTAILQ_HEAD(, EventLogEntry) pending_events; > +} rtas_event_log; Adding new globals is never nice. Couldn't this go into sPAPREnvironment, or better yet the SPAPR_MACHINE structure? (I'd like to merge those two when we get the chance). > +static void rtas_event_log_queue(int log_type, void *data) > { > - sPAPREnvironment *spapr =3D container_of(n, sPAPREnvironment, epow_n= otifier); > - struct rtas_error_log *hdr; > - struct rtas_event_log_v6 *v6hdr; > - struct rtas_event_log_v6_maina *maina; > - struct rtas_event_log_v6_mainb *mainb; > - struct rtas_event_log_v6_epow *epow; > - struct tm tm; > - int year; > + EventLogEntry *entry =3D g_new(EventLogEntry, 1); > + > + entry->log_type =3D log_type; > + entry->data =3D data; > + QTAILQ_INSERT_TAIL(&rtas_event_log.pending_events, entry, next); > +} > =20 > - if (pending_epow) { > - /* For now, we just throw away earlier events if two come > - * along before any are consumed. This is sufficient for our > - * powerdown messages, but we'll need more if we do more > - * general error/event logging */ > - g_free(pending_epow); > +static EventLogEntry *rtas_event_log_dequeue(uint32_t event_mask) > +{ > + EventLogEntry *entry =3D NULL; > + > + /* we only queue EPOW events atm. */ > + if ((event_mask & EVENT_MASK_EPOW) =3D=3D 0) { > + return NULL; > } > - pending_epow =3D g_malloc0(sizeof(*pending_epow)); > - hdr =3D &pending_epow->hdr; > - v6hdr =3D &pending_epow->v6hdr; > - maina =3D &pending_epow->maina; > - mainb =3D &pending_epow->mainb; > - epow =3D &pending_epow->epow; > =20 > - hdr->summary =3D cpu_to_be32(RTAS_LOG_VERSION_6 > - | RTAS_LOG_SEVERITY_EVENT > - | RTAS_LOG_DISPOSITION_NOT_RECOVERED > - | RTAS_LOG_OPTIONAL_PART_PRESENT > - | RTAS_LOG_TYPE_EPOW); > - hdr->extended_length =3D cpu_to_be32(sizeof(*pending_epow) > - - sizeof(pending_epow->hdr)); > + QTAILQ_FOREACH(entry, &rtas_event_log.pending_events, next) { > + /* EPOW and hotplug events are surfaced in the same manner */ > + if (entry->log_type =3D=3D RTAS_LOG_TYPE_EPOW || > + entry->log_type =3D=3D RTAS_LOG_TYPE_HOTPLUG) { > + break; > + } > + } > + > + if (entry) { > + QTAILQ_REMOVE(&rtas_event_log.pending_events, entry, next); > + } > =20 > + return entry; > +} > + > +static uint32_t next_plid; > + > +static void spapr_init_v6hdr(struct rtas_event_log_v6 *v6hdr) > +{ > v6hdr->b0 =3D RTAS_LOG_V6_B0_VALID | RTAS_LOG_V6_B0_NEW_LOG > | RTAS_LOG_V6_B0_BIGENDIAN; > v6hdr->b2 =3D RTAS_LOG_V6_B2_POWERPC_FORMAT > | RTAS_LOG_V6_B2_LOG_FORMAT_PLATFORM_EVENT; > v6hdr->company =3D cpu_to_be32(RTAS_LOG_V6_COMPANY_IBM); > +} > + > +static void spapr_init_maina(struct rtas_event_log_v6_maina *maina, > + int section_count) > +{ > + struct tm tm; > + int year; > =20 > maina->hdr.section_id =3D cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINA); > maina->hdr.section_length =3D cpu_to_be16(sizeof(*maina)); > @@ -256,8 +309,37 @@ static void spapr_powerdown_req(Notifier *n, void *o= paque) > | (to_bcd(tm.tm_min) << 16) > | (to_bcd(tm.tm_sec) << 8)); > maina->creator_id =3D 'H'; /* Hypervisor */ > - maina->section_count =3D 3; /* Main-A, Main-B and EPOW */ > + maina->section_count =3D section_count; > maina->plid =3D next_plid++; > +} > + > +static void spapr_powerdown_req(Notifier *n, void *opaque) > +{ > + sPAPREnvironment *spapr =3D container_of(n, sPAPREnvironment, epow_n= otifier); > + struct rtas_error_log *hdr; > + struct rtas_event_log_v6 *v6hdr; > + struct rtas_event_log_v6_maina *maina; > + struct rtas_event_log_v6_mainb *mainb; > + struct rtas_event_log_v6_epow *epow; > + struct epow_log_full *pending_epow; > + > + pending_epow =3D g_malloc0(sizeof(*pending_epow)); The name's no longer really right as a local. > + hdr =3D &pending_epow->hdr; > + v6hdr =3D &pending_epow->v6hdr; > + maina =3D &pending_epow->maina; > + mainb =3D &pending_epow->mainb; > + epow =3D &pending_epow->epow; > + > + hdr->summary =3D cpu_to_be32(RTAS_LOG_VERSION_6 > + | RTAS_LOG_SEVERITY_EVENT > + | RTAS_LOG_DISPOSITION_NOT_RECOVERED > + | RTAS_LOG_OPTIONAL_PART_PRESENT > + | RTAS_LOG_TYPE_EPOW); > + hdr->extended_length =3D cpu_to_be32(sizeof(*pending_epow) > + - sizeof(pending_epow->hdr)); > + > + spapr_init_v6hdr(v6hdr); > + spapr_init_maina(maina, 3 /* Main-A, Main-B and EPOW */); > =20 > mainb->hdr.section_id =3D cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINB); > mainb->hdr.section_length =3D cpu_to_be16(sizeof(*mainb)); > @@ -274,7 +356,78 @@ static void spapr_powerdown_req(Notifier *n, void *o= paque) > epow->event_modifier =3D RTAS_LOG_V6_EPOW_MODIFIER_NORMAL; > epow->extended_modifier =3D RTAS_LOG_V6_EPOW_XMODIFIER_PARTITION_SPE= CIFIC; > =20 > - qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->epow_irq)); > + rtas_event_log_queue(RTAS_LOG_TYPE_EPOW, pending_epow); > + > + qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq)= ); Shouldn't the irq pulse go into rtas_event_log_queue()? > +} > + > +static void spapr_hotplug_req_event(sPAPRDRConnector *drc, uint8_t hp_ac= tion) > +{ > + struct hp_log_full *new_hp; > + struct rtas_error_log *hdr; > + struct rtas_event_log_v6 *v6hdr; > + struct rtas_event_log_v6_maina *maina; > + struct rtas_event_log_v6_mainb *mainb; > + struct rtas_event_log_v6_hp *hp; > + sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + sPAPRDRConnectorType drc_type =3D drck->get_type(drc); > + > + new_hp =3D g_malloc0(sizeof(struct hp_log_full)); > + hdr =3D &new_hp->hdr; > + v6hdr =3D &new_hp->v6hdr; > + maina =3D &new_hp->maina; > + mainb =3D &new_hp->mainb; > + hp =3D &new_hp->hp; > + > + hdr->summary =3D cpu_to_be32(RTAS_LOG_VERSION_6 > + | RTAS_LOG_SEVERITY_EVENT > + | RTAS_LOG_DISPOSITION_NOT_RECOVERED > + | RTAS_LOG_OPTIONAL_PART_PRESENT > + | RTAS_LOG_INITIATOR_HOTPLUG > + | RTAS_LOG_TYPE_HOTPLUG); > + hdr->extended_length =3D cpu_to_be32(sizeof(*new_hp) > + - sizeof(new_hp->hdr)); > + > + spapr_init_v6hdr(v6hdr); > + spapr_init_maina(maina, 3 /* Main-A, Main-B, HP */); > + > + mainb->hdr.section_id =3D cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINB); > + mainb->hdr.section_length =3D cpu_to_be16(sizeof(*mainb)); > + mainb->subsystem_id =3D 0x80; /* External environment */ > + mainb->event_severity =3D 0x00; /* Informational / non-error */ > + mainb->event_subtype =3D 0x00; /* Normal shutdown */ > + > + hp->hdr.section_id =3D cpu_to_be16(RTAS_LOG_V6_SECTION_ID_HOTPLUG); > + hp->hdr.section_length =3D cpu_to_be16(sizeof(*hp)); > + hp->hdr.section_version =3D 1; /* includes extended modifier */ > + hp->hotplug_action =3D hp_action; > + > + > + switch (drc_type) { > + case SPAPR_DR_CONNECTOR_TYPE_PCI: > + hp->drc.index =3D cpu_to_be32(drck->get_index(drc)); > + hp->hotplug_identifier =3D RTAS_LOG_V6_HP_ID_DRC_INDEX; > + hp->hotplug_type =3D RTAS_LOG_V6_HP_TYPE_PCI; > + break; > + default: > + /* skip notification for unknown connector types */ This should be an assert, shouldn't it? If code is added which can fire this for non-PCI types, without the corresponding non-PCI option here, you want to catch that. > + g_free(new_hp); > + return; > + } > + > + rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp); > + > + qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq)= ); > +} > + > +void spapr_hotplug_req_add_event(sPAPRDRConnector *drc) > +{ > + spapr_hotplug_req_event(drc, RTAS_LOG_V6_HP_ACTION_ADD); > +} > + > +void spapr_hotplug_req_remove_event(sPAPRDRConnector *drc) > +{ > + spapr_hotplug_req_event(drc, RTAS_LOG_V6_HP_ACTION_REMOVE); > } > =20 > static void check_exception(PowerPCCPU *cpu, sPAPREnvironment *spapr, > @@ -282,8 +435,9 @@ static void check_exception(PowerPCCPU *cpu, sPAPREnv= ironment *spapr, > target_ulong args, > uint32_t nret, target_ulong rets) > { > - uint32_t mask, buf, len; > + uint32_t mask, buf, len, event_len; > uint64_t xinfo; > + EventLogEntry *event; > =20 > if ((nargs < 6) || (nargs > 7) || nret !=3D 1) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -298,23 +452,40 @@ static void check_exception(PowerPCCPU *cpu, sPAPRE= nvironment *spapr, > xinfo |=3D (uint64_t)rtas_ld(args, 6) << 32; > } > =20 > - if ((mask & EVENT_MASK_EPOW) && pending_epow) { > - if (sizeof(*pending_epow) < len) { > - len =3D sizeof(*pending_epow); > - } > + event =3D rtas_event_log_dequeue(mask); > + if (!event) { > + goto out_no_events; > + } > =20 > - cpu_physical_memory_write(buf, pending_epow, len); > - g_free(pending_epow); > - pending_epow =3D NULL; > - rtas_st(rets, 0, RTAS_OUT_SUCCESS); > - } else { > - rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); > + switch (event->log_type) { > + case RTAS_LOG_TYPE_EPOW: > + event_len =3D sizeof(struct epow_log_full); > + break; > + case RTAS_LOG_TYPE_HOTPLUG: > + event_len =3D sizeof(struct hp_log_full); > + break; > + default: > + goto out_no_events; Doesn't one of the headers include size information you could use here, avoiding the switch? > } > + > + if (event_len < len) { > + len =3D event_len; > + } > + > + cpu_physical_memory_write(buf, event->data, len); > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + g_free(event->data); > + g_free(event); > + return; > + > +out_no_events: > + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); > } > =20 > void spapr_events_init(sPAPREnvironment *spapr) > { > - spapr->epow_irq =3D xics_alloc(spapr->icp, 0, 0, false); > + QTAILQ_INIT(&rtas_event_log.pending_events); > + spapr->check_exception_irq =3D xics_alloc(spapr->icp, 0, 0, false); > spapr->epow_notifier.notify =3D spapr_powerdown_req; > qemu_register_powerdown_notifier(&spapr->epow_notifier); > spapr_rtas_register(RTAS_CHECK_EXCEPTION, "check-exception", > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 3cc4490..1d27708 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -3,6 +3,7 @@ > =20 > #include "sysemu/dma.h" > #include "hw/ppc/xics.h" > +#include "hw/ppc/spapr_drc.h" > =20 > struct VIOsPAPRBus; > struct sPAPRPHBState; > @@ -31,7 +32,7 @@ typedef struct sPAPREnvironment { > struct PPCTimebase tb; > bool has_graphics; > =20 > - uint32_t epow_irq; > + uint32_t check_exception_irq; > Notifier epow_notifier; > =20 > /* Migration state */ > @@ -498,6 +499,8 @@ int spapr_dma_dt(void *fdt, int node_off, const char = *propname, > uint32_t liobn, uint64_t window, uint32_t size); > int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > sPAPRTCETable *tcet); > +void spapr_hotplug_req_add_event(sPAPRDRConnector *drc); > +void spapr_hotplug_req_remove_event(sPAPRDRConnector *drc); > =20 > #define TYPE_SPAPR_RTC "spapr-rtc" > =20 --=20 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 --magLDk5D4XGaUXcd Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU7B8GAAoJEGw4ysog2bOS2bQQAJRRtjypy5uEkDzXbuz9amVJ cGGqg1Y4l3yCIuda4U+vgo9/FC/jsXdULWKrL2nN9mFz8Q1wdHRqd2ufbEY7w7v+ KKc+ZH4VgAkUkzlYWJkzMz1gSVxJxMKnGRdFxNi9JeWkqM/W8qaQ9pdl/ovq/4Rj PNEJ1Cu6eqQmoErJzRKWJTJSDrIwuf0KQNvG8vr5hwDhhdrkxdFySFOIxR/m2GzR flC8Q2Amkb6/TA4ilDWXdhrxX4GY1vTHhTdoLMhNopfTqLJy8TSqas1hNp+myD8D Z/zP8y9me8CVMWRFbjyWz2pl1PwDXumZB0PoZ2uzuXM1Xz0cKuEHCCMkiON30iza 6INC0S5vyRUDNvW3B4KlAEQ2t7iVWKfFBXxbqd7t+pH+QSErkt8M9HLXGjUYGdAY a3f8KIlI4v+R7uRsknCVZJ9r4UHqowtpqYcMm68y0ZtLX9G+UwpGs3YvmRK2+hQB iCo+1rUYU9WlhLlq5/ykjYG0+zZEZp5mtldVUn0rJDvB6CU1JL/ck6arXsFZdH6/ m8sJNr6EsCsOyxCWmeZYH9nX0Dar9J9ECkCbPmVyDfasK41hJAc/cIsYKlzj62eP 4uoeqM78BfntngMNe/RQzDsCIuOJKjv1U1jST8LnDql3shaPrCClilLlKRMQDxd9 Vg87bvQ6vlL/qAHF+JLl =lK6Z -----END PGP SIGNATURE----- --magLDk5D4XGaUXcd--