On Mon, Jan 26, 2015 at 10:56:51AM -0600, Michael Roth wrote:
> Quoting David Gibson (2015-01-18 22:31:23)
> > On Tue, Dec 23, 2014 at 06:30:22AM -0600, Michael Roth wrote:
> > > From: Nathan Fontenot <nf...@linux.vnet.ibm.com>
[snip]
> > > +static void spapr_hotplug_req_event(sPAPRDRConnector *drc, uint8_t
> > > hp_action)
> > > +{
> > > + 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 = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > > + sPAPRDRConnectorType drc_type = drck->get_type(drc);
> > > +
> > > + new_hp = g_malloc0(sizeof(struct hp_log_full));
> > > + hdr = &new_hp->hdr;
> > > + v6hdr = &new_hp->v6hdr;
> > > + maina = &new_hp->maina;
> > > + mainb = &new_hp->mainb;
> > > + hp = &new_hp->hp;
> > > +
> > > + hdr->summary = 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 = 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 = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINB);
> > > + mainb->hdr.section_length = cpu_to_be16(sizeof(*mainb));
> > > + mainb->subsystem_id = 0x80; /* External environment */
> > > + mainb->event_severity = 0x00; /* Informational / non-error */
> > > + mainb->event_subtype = 0x00; /* Normal shutdown */
> > > +
> > > + hp->hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_HOTPLUG);
> > > + hp->hdr.section_length = cpu_to_be16(sizeof(*hp));
> > > + hp->hdr.section_version = 1; /* includes extended modifier */
> > > + hp->hotplug_action = hp_action;
> > > +
> > > +
> > > + switch (drc_type) {
> > > + case SPAPR_DR_CONNECTOR_TYPE_PCI:
> > > + hp->drc.index = cpu_to_be32(drck->get_index(drc));
> > > + hp->hotplug_identifier = RTAS_LOG_V6_HP_ID_DRC_INDEX;
> > > + hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PCI;
> > > + break;
> > > + default:
> > > + /* skip notification for unknown connector types */
> > > + g_free(new_hp);
> > > + return;
> > > + }
> > > +
> > > + if (pending_hp) {
> > > + /* Just toss any pending hotplug events for now, this will
> > > + * need to be fixed later on.
> > > + */
> >
> > So, we can get away with a 1-element queue for EPOW, because they're
> > just triggering a shutdown - so once the first one's processed, any
> > others aren't going to matter. For hotplug you really do need a
> > proper queue.
>
> Yah, this was discussed in the past, but until now I didn't notice how
> easy it would be to trigger this when hotplugging multiple devices from
> a script or management harness of some sort. Should be simple enough to
> fix for v5 though.
Ok, good.
> > > + g_free(pending_hp);
> > > + }
> > > + pending_hp = 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);
> > > }
> > >
> > > static void check_exception(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> > > @@ -298,15 +420,26 @@ static void check_exception(PowerPCCPU *cpu,
> > > sPAPREnvironment *spapr,
> > > xinfo |= (uint64_t)rtas_ld(args, 6) << 32;
> > > }
> > >
> > > - if ((mask & EVENT_MASK_EPOW) && pending_epow) {
> > > - if (sizeof(*pending_epow) < len) {
> > > - len = sizeof(*pending_epow);
> > > - }
> > > + if (mask & EVENT_MASK_EPOW) {
> > > + if (pending_epow) {
> > > + if (sizeof(*pending_epow) < len) {
> > > + len = sizeof(*pending_epow);
> > > + }
> > >
> > > - cpu_physical_memory_write(buf, pending_epow, len);
> > > - g_free(pending_epow);
> > > - pending_epow = NULL;
> > > - rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > > + cpu_physical_memory_write(buf, pending_epow, len);
> > > + g_free(pending_epow);
> > > + pending_epow = NULL;
> > > + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > > + } else if (pending_hp) {
> >
> > So.. the hotplug messages are a different type from EPOW, but are
> > still selected by EVENT_MASK_EPOW? Seems a bit odd.
>
> It's a little odd, but it's mainly just due to the way we surface the
> hotplug event. Rather than requiring patched guest kernels, we opted
> to re-use and generalize the EPOW IRQ to also surface hotplug-related
> RTAS events, which is why we still expect the EVENT_MASK_EPOW when
> returning an HP event via check-exception.
>
> EPOW events have well-defined behavior in how they're exposed to
> userspace via rtas_errd, which allows us to add hotplug support for
> older guests via patched userspace tools.
Ok. Kind of a hack, but for good reasons. I kind of think it needs a
comment, but I'm not sure where it would belong.
--
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
pgpseTbhil8y9.pgp
Description: PGP signature
