Re: [Qemu-devel] [PATCH v2 12/14] spapr_events: re-use EPOW event infrastructure for hotplug events

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Alexey Kardashevskiy (2013-12-15 23:05:55)
> On 12/06/2013 09:33 AM, Michael Roth wrote:
> > From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > 
> > This extends the data structures currently used to report EPOW events to
> > gets via the check-exception RTAS interfaces to also include event types
> > for hotplug/unplug events.
> > 
> > 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).
> > 
> > Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c         |    2 +-
> >  hw/ppc/spapr_events.c  |  219 +++++++++++++++++++++++++++++++++++++++---------
> >  include/hw/ppc/spapr.h |    4 +-
> >  3 files changed, 184 insertions(+), 41 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 2250ee1..7079e4e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1522,7 +1522,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> >      spapr->fdt_skel = 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 != NULL);
> >  }
> >  
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index 16fa49e..9dfdbcf 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -32,6 +32,8 @@
> >  
> >  #include "hw/ppc/spapr.h"
> >  #include "hw/ppc/spapr_vio.h"
> > +#include "hw/pci/pci.h"
> > +#include "hw/pci-host/spapr.h"
> >  
> >  #include <libfdt.h>
> >  
> > @@ -77,6 +79,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;
> >  
> > @@ -166,6 +169,38 @@ struct epow_log_full {
> >      struct rtas_event_log_v6_epow epow;
> >  } QEMU_PACKED;
> >  
> > +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,29 +216,61 @@ struct epow_log_full {
> >          }                                                          \
> >      } while (0)
> >  
> > -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[] = {cpu_to_be32(epow_irq), cpu_to_be32(1)};
> > -    uint32_t epow_interrupts[] = {cpu_to_be32(epow_irq), 0};
> > +    uint32_t irq_ranges[] = {cpu_to_be32(check_exception_irq), cpu_to_be32(1)};
> > +    uint32_t interrupts[] = {cpu_to_be32(check_exception_irq), 0};
> >  
> >      _FDT((fdt_begin_node(fdt, "event-sources")));
> >  
> >      _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))));
> >  
> >      _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)));
> >  
> >      _FDT((fdt_end_node(fdt)));
> >  }
> >  
> >  static struct epow_log_full *pending_epow;
> > +static struct hp_log_full *pending_hp;
> >  static uint32_t next_plid;
> >  
> > +static void spapr_init_v6hdr(struct rtas_event_log_v6 *v6hdr)
> > +{
> > +    v6hdr->b0 = RTAS_LOG_V6_B0_VALID | RTAS_LOG_V6_B0_NEW_LOG
> > +        | RTAS_LOG_V6_B0_BIGENDIAN;
> > +    v6hdr->b2 = RTAS_LOG_V6_B2_POWERPC_FORMAT
> > +        | RTAS_LOG_V6_B2_LOG_FORMAT_PLATFORM_EVENT;
> > +    v6hdr->company = 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;
> > +
> > +    maina->hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINA);
> > +    maina->hdr.section_length = cpu_to_be16(sizeof(*maina));
> > +    /* FIXME: section version, subtype and creator id? */
> > +    qemu_get_timedate(&tm, spapr->rtc_offset);
> > +    year = tm.tm_year + 1900;
> > +    maina->creation_date = cpu_to_be32((to_bcd(year / 100) << 24)
> > +                                       | (to_bcd(year % 100) << 16)
> > +                                       | (to_bcd(tm.tm_mon + 1) << 8)
> > +                                       | to_bcd(tm.tm_mday));
> > +    maina->creation_time = cpu_to_be32((to_bcd(tm.tm_hour) << 24)
> > +                                       | (to_bcd(tm.tm_min) << 16)
> > +                                       | (to_bcd(tm.tm_sec) << 8));
> > +    maina->creator_id = 'H'; /* Hypervisor */
> > +    maina->section_count = section_count;
> > +    maina->plid = next_plid++;
> > +}
> > +
> >  static void spapr_powerdown_req(Notifier *n, void *opaque)
> >  {
> >      sPAPREnvironment *spapr = container_of(n, sPAPREnvironment, epow_notifier);
> > @@ -212,8 +279,6 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
> >      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;
> >  
> >      if (pending_epow) {
> >          /* For now, we just throw away earlier events if two come
> > @@ -237,27 +302,8 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
> >      hdr->extended_length = cpu_to_be32(sizeof(*pending_epow)
> >                                         - sizeof(pending_epow->hdr));
> >  
> > -    v6hdr->b0 = RTAS_LOG_V6_B0_VALID | RTAS_LOG_V6_B0_NEW_LOG
> > -        | RTAS_LOG_V6_B0_BIGENDIAN;
> > -    v6hdr->b2 = RTAS_LOG_V6_B2_POWERPC_FORMAT
> > -        | RTAS_LOG_V6_B2_LOG_FORMAT_PLATFORM_EVENT;
> > -    v6hdr->company = cpu_to_be32(RTAS_LOG_V6_COMPANY_IBM);
> > -
> > -    maina->hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINA);
> > -    maina->hdr.section_length = cpu_to_be16(sizeof(*maina));
> > -    /* FIXME: section version, subtype and creator id? */
> > -    qemu_get_timedate(&tm, spapr->rtc_offset);
> > -    year = tm.tm_year + 1900;
> > -    maina->creation_date = cpu_to_be32((to_bcd(year / 100) << 24)
> > -                                       | (to_bcd(year % 100) << 16)
> > -                                       | (to_bcd(tm.tm_mon + 1) << 8)
> > -                                       | to_bcd(tm.tm_mday));
> > -    maina->creation_time = cpu_to_be32((to_bcd(tm.tm_hour) << 24)
> > -                                       | (to_bcd(tm.tm_min) << 16)
> > -                                       | (to_bcd(tm.tm_sec) << 8));
> > -    maina->creator_id = 'H'; /* Hypervisor */
> > -    maina->section_count = 3; /* Main-A, Main-B and EPOW */
> > -    maina->plid = next_plid++;
> > +    spapr_init_v6hdr(v6hdr);
> > +    spapr_init_maina(maina, 3 /* Main-A, Main-B and EPOW */);
> >  
> >      mainb->hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MAINB);
> >      mainb->hdr.section_length = cpu_to_be16(sizeof(*mainb));
> > @@ -274,9 +320,93 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
> >      epow->event_modifier = RTAS_LOG_V6_EPOW_MODIFIER_NORMAL;
> >      epow->extended_modifier = RTAS_LOG_V6_EPOW_XMODIFIER_PARTITION_SPECIFIC;
> >  
> > -    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->epow_irq));
> > +    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
> > +}
> > +
> > +static void spapr_hotplug_req_event(uint8_t hp_type, uint8_t hp_action,
> > +                                    sPAPRPHBState *phb, int slot)
> 
> 
> This only uses a @buid from sPAPRPHBState, what is the point in passing the
> while struct? Any plans to use other fields there?

None that I can think of, will switch to using only buid. We may need to
expand this in the future for phb/cpu/memory hp event types, but
those will require more parameters anyway.

> 
> 
> > +{
> > +    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;
> > +    DrcEntry *drc_entry;
> > +
> > +    if (pending_hp) {
> > +        /* Just toss any pending hotplug events for now, this will
> > +         * need to be fixed later on.
> > +         */
> > +        g_free(pending_hp);
> > +    }
> > +
> > +    pending_hp = g_malloc0(sizeof(*pending_hp));
> > +    hdr = &pending_hp->hdr;
> > +    v6hdr = &pending_hp->v6hdr;
> > +    maina = &pending_hp->maina;
> > +    mainb = &pending_hp->mainb;
> > +    hp = &pending_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(*pending_hp)
> > +                                       - sizeof(pending_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;
> > +
> > +    hp->hotplug_type = hp_type;
> > +
> > +    drc_entry = spapr_phb_to_drc_entry(phb->buid);
> > +    if (!drc_entry) {
> > +        drc_entry = spapr_add_phb_to_drc_table(phb->buid, 2 /* Unusable */);
> > +    }
> > +
> > +    switch (hp_type) {
> > +    case RTAS_LOG_V6_HP_TYPE_PCI:
> > +        hp->drc.index = drc_entry->child_entries[slot].drc_index;
> > +        hp->hotplug_identifier = RTAS_LOG_V6_HP_ID_DRC_INDEX;
> > +        break;
> > +    }
> > +
> > +    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
> > +}
> > +
> > +void spapr_pci_hotplug_add_event(DeviceState *qdev, int slot)
> > +{
> > +    sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(qdev);
> > +
> > +    return spapr_hotplug_req_event(RTAS_LOG_V6_HP_TYPE_PCI,
> > +                                   RTAS_LOG_V6_HP_ACTION_ADD, phb, slot);
> >  }
> >  
> > +void spapr_pci_hotplug_remove_event(DeviceState *qdev, int slot)
> > +{
> > +    sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(qdev);
> > +
> > +    /* TODO: removal is generally initiated by guest, need to
> > +     * document what exactly the guest is supposed to do with
> > +     * this event. What does ACPI or shpc do?
> > +     */
> > +    return spapr_hotplug_req_event(RTAS_LOG_V6_HP_TYPE_PCI,
> > +                                   RTAS_LOG_V6_HP_ACTION_REMOVE, phb, slot);
> > + }
> > +
> >  static void check_exception(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> >                              uint32_t token, uint32_t nargs,
> >                              target_ulong args,
> > @@ -298,15 +428,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) {
> > +            if (sizeof(*pending_hp) < len) {
> > +                len = sizeof(*pending_hp);
> > +            }
> > +
> > +            cpu_physical_memory_write(buf, pending_hp, len);
> > +            g_free(pending_hp);
> > +            pending_hp = NULL;
> > +            rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > +        }
> >      } else {
> >          rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND);
> >      }
> > @@ -314,7 +455,7 @@ static void check_exception(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> >  
> >  void spapr_events_init(sPAPREnvironment *spapr)
> >  {
> > -    spapr->epow_irq = spapr_allocate_msi(0);
> > +    spapr->check_exception_irq = spapr_allocate_msi(0);
> >      spapr->epow_notifier.notify = spapr_powerdown_req;
> >      qemu_register_powerdown_notifier(&spapr->epow_notifier);
> >      spapr_rtas_register("check-exception", check_exception);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 1c9b725..9eef2ce 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -31,7 +31,7 @@ typedef struct sPAPREnvironment {
> >      uint64_t rtc_offset;
> >      bool has_graphics;
> >  
> > -    uint32_t epow_irq;
> > +    uint32_t check_exception_irq;
> >      Notifier epow_notifier;
> >  
> >      /* Migration state */
> > @@ -473,5 +473,7 @@ 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_pci_hotplug_add_event(DeviceState *qdev, int slot);
> > +void spapr_pci_hotplug_remove_event(DeviceState *qdev, int slot);
> >  
> >  #endif /* !defined (__HW_SPAPR_H__) */
> > 
> 
> 
> -- 
> Alexey