Re: [Qemu-devel] [PATCH v4 08/17] spapr_events: re-use EPOW event infrastructure for hotplug events

[Bharata B Rao]

On Mon, Jan 26, 2015 at 10:26 PM, Michael Roth
<address@hidden> wrote:
> Quoting David Gibson (2015-01-18 22:31:23)
>> On Tue, Dec 23, 2014 at 06:30:22AM -0600, Michael Roth wrote:
>> > +
>> > +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.

In case of memory hotplug, each LMB is associated with a drc. Hence a
request to hotplug a chunk of memory will be broken down into multiple
requests (each of min LMB size). So supporting a queue will be needed
and can be tested with memory hotplug code.

Regards,
Bharata.