| Message ID | 1408407718-10835-5-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 19.08.14 02:21, Michael Roth wrote: > From: Mike Day <ncmike@ncultra.org> > > Signed-off-by: Mike Day <ncmike@ncultra.org> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 3 ++ > 2 files changed, 122 insertions(+) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 924d488..23a3477 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -36,6 +36,16 @@ > > #include "hw/pci/pci_bus.h" > > +/* #define DEBUG_SPAPR */ > + > +#ifdef DEBUG_SPAPR > +#define DPRINTF(fmt, ...) \ > + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) > +#else > +#define DPRINTF(fmt, ...) \ > + do { } while (0) > +#endif > + > /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ > #define RTAS_QUERY_FN 0 > #define RTAS_CHANGE_FN 1 > @@ -47,6 +57,31 @@ > #define RTAS_TYPE_MSI 1 > #define RTAS_TYPE_MSIX 2 > > +/* For set-indicator RTAS interface */ > +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ > +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ > +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ > +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ > +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ > +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ > +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ > +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ > + > +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ > +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ > +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ > +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ > +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ > +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ > +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ > +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ > + > +#define DECODE_DRC_STATE(state, m, s) \ > + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) > + > +#define ENCODE_DRC_STATE(val, m, s) \ > + (((uint32_t)(val) << (s)) & (uint32_t)(m)) > + > static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) > { > sPAPRPHBState *sphb; > @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ > } > > +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t indicator = rtas_ld(args, 0); > + uint32_t drc_index = rtas_ld(args, 1); > + uint32_t indicator_state = rtas_ld(args, 2); > + uint32_t encoded = 0, shift = 0, mask = 0; > + uint32_t *pind; > + sPAPRDrcEntry *drc_entry = NULL; This rtas call does not have any idea what a PHB is. Why does it live in spapr_pci.c? > + > + if (drc_index == 0) { /* platform indicator */ > + pind = &spapr->state; > + } else { > + drc_entry = spapr_find_drc_entry(drc_index); > + if (!drc_entry) { > + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", > + drc_index); > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > + return; > + } > + pind = &drc_entry->state; > + } > + > + switch (indicator) { > + case 9: /* EPOW */ > + shift = INDICATOR_EPOW_SHIFT; > + mask = INDICATOR_EPOW_MASK; > + break; > + case 9001: /* Isolation state */ > + /* encode the new value into the correct bit field */ > + shift = INDICATOR_ISOLATION_SHIFT; > + mask = INDICATOR_ISOLATION_MASK; > + break; > + case 9002: /* DR */ > + shift = INDICATOR_DR_SHIFT; > + mask = INDICATOR_DR_MASK; > + break; > + case 9003: /* Allocation State */ > + shift = INDICATOR_ALLOCATION_SHIFT; > + mask = INDICATOR_ALLOCATION_MASK; > + break; > + case 9005: /* global interrupt */ > + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; > + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; > + break; > + case 9006: /* error log */ > + shift = INDICATOR_ERROR_LOG_SHIFT; > + mask = INDICATOR_ERROR_LOG_MASK; > + break; > + case 9007: /* identify */ > + shift = INDICATOR_IDENTIFY_SHIFT; > + mask = INDICATOR_IDENTIFY_MASK; > + break; > + case 9009: /* reset */ > + shift = INDICATOR_RESET_SHIFT; > + mask = INDICATOR_RESET_MASK; > + break; > + default: > + DPRINTF("rtas_set_indicator: indicator not implemented: %d", > + indicator); > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > + return; > + } > + > + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); > + /* clear the current indicator value */ > + *pind &= ~mask; > + /* set the new value */ > + *pind |= encoded; > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > +} > + > static int pci_spapr_swizzle(int slot, int pin) > { > return (slot + pin) % PCI_NUM_PINS; > @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > sphb->lsi_table[i].irq = irq; > } > > + /* make sure the platform EPOW sensor is initialized - the > + * guest will probe it when there is a hotplug event. > + */ > + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; > + spapr->state |= ENCODE_DRC_STATE(0, > + INDICATOR_EPOW_MASK, > + INDICATOR_EPOW_SHIFT); > + > if (!info->finish_realize) { > error_setg(errp, "finish_realize not defined"); > return; > @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) > spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", > rtas_ibm_change_msi); > } > + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", > + rtas_set_indicator); > } > > static void spapr_pci_register_types(void) > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 0ac1a19..fac85f8 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { > > /* state for Dynamic Reconfiguration Connectors */ > sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; > + > + /* Platform state - sensors and indicators */ > + uint32_t state; Do you think it'd be possible to create a special DRC device that contains all of its tables and global state and also exposes sensors and indicators? That device could then get linked via qom links to the PHBs for their slots. Alex
On 08/26/2014 06:36 AM, Alexander Graf wrote: > > > On 19.08.14 02:21, Michael Roth wrote: >> From: Mike Day <ncmike@ncultra.org> >> >> Signed-off-by: Mike Day <ncmike@ncultra.org> >> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 3 ++ >> 2 files changed, 122 insertions(+) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index 924d488..23a3477 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -36,6 +36,16 @@ >> >> #include "hw/pci/pci_bus.h" >> >> +/* #define DEBUG_SPAPR */ >> + >> +#ifdef DEBUG_SPAPR >> +#define DPRINTF(fmt, ...) \ >> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) >> +#else >> +#define DPRINTF(fmt, ...) \ >> + do { } while (0) >> +#endif >> + >> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >> #define RTAS_QUERY_FN 0 >> #define RTAS_CHANGE_FN 1 >> @@ -47,6 +57,31 @@ >> #define RTAS_TYPE_MSI 1 >> #define RTAS_TYPE_MSIX 2 >> >> +/* For set-indicator RTAS interface */ >> +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ >> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ >> +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ >> +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ >> +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ >> +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ >> +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ >> +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ >> + >> +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ >> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ >> +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ >> +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ >> +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ >> +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ >> +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ >> +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ >> + >> +#define DECODE_DRC_STATE(state, m, s) \ >> + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) >> + >> +#define ENCODE_DRC_STATE(val, m, s) \ >> + (((uint32_t)(val) << (s)) & (uint32_t)(m)) >> + >> static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) >> { >> sPAPRPHBState *sphb; >> @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, >> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ >> } >> >> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> + uint32_t token, uint32_t nargs, >> + target_ulong args, uint32_t nret, >> + target_ulong rets) >> +{ >> + uint32_t indicator = rtas_ld(args, 0); >> + uint32_t drc_index = rtas_ld(args, 1); >> + uint32_t indicator_state = rtas_ld(args, 2); >> + uint32_t encoded = 0, shift = 0, mask = 0; >> + uint32_t *pind; >> + sPAPRDrcEntry *drc_entry = NULL; > > This rtas call does not have any idea what a PHB is. Why does it live in > spapr_pci.c? > It probably shouldn't be there, we will need this call for memory and cpu hotplug later on. -Nathan >> + >> + if (drc_index == 0) { /* platform indicator */ >> + pind = &spapr->state; >> + } else { >> + drc_entry = spapr_find_drc_entry(drc_index); >> + if (!drc_entry) { >> + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", >> + drc_index); >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> + return; >> + } >> + pind = &drc_entry->state; >> + } >> + >> + switch (indicator) { >> + case 9: /* EPOW */ >> + shift = INDICATOR_EPOW_SHIFT; >> + mask = INDICATOR_EPOW_MASK; >> + break; >> + case 9001: /* Isolation state */ >> + /* encode the new value into the correct bit field */ >> + shift = INDICATOR_ISOLATION_SHIFT; >> + mask = INDICATOR_ISOLATION_MASK; >> + break; >> + case 9002: /* DR */ >> + shift = INDICATOR_DR_SHIFT; >> + mask = INDICATOR_DR_MASK; >> + break; >> + case 9003: /* Allocation State */ >> + shift = INDICATOR_ALLOCATION_SHIFT; >> + mask = INDICATOR_ALLOCATION_MASK; >> + break; >> + case 9005: /* global interrupt */ >> + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; >> + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; >> + break; >> + case 9006: /* error log */ >> + shift = INDICATOR_ERROR_LOG_SHIFT; >> + mask = INDICATOR_ERROR_LOG_MASK; >> + break; >> + case 9007: /* identify */ >> + shift = INDICATOR_IDENTIFY_SHIFT; >> + mask = INDICATOR_IDENTIFY_MASK; >> + break; >> + case 9009: /* reset */ >> + shift = INDICATOR_RESET_SHIFT; >> + mask = INDICATOR_RESET_MASK; >> + break; >> + default: >> + DPRINTF("rtas_set_indicator: indicator not implemented: %d", >> + indicator); >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> + return; >> + } >> + >> + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); >> + /* clear the current indicator value */ >> + *pind &= ~mask; >> + /* set the new value */ >> + *pind |= encoded; >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> +} >> + >> static int pci_spapr_swizzle(int slot, int pin) >> { >> return (slot + pin) % PCI_NUM_PINS; >> @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) >> sphb->lsi_table[i].irq = irq; >> } >> >> + /* make sure the platform EPOW sensor is initialized - the >> + * guest will probe it when there is a hotplug event. >> + */ >> + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; >> + spapr->state |= ENCODE_DRC_STATE(0, >> + INDICATOR_EPOW_MASK, >> + INDICATOR_EPOW_SHIFT); >> + >> if (!info->finish_realize) { >> error_setg(errp, "finish_realize not defined"); >> return; >> @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) >> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", >> rtas_ibm_change_msi); >> } >> + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", >> + rtas_set_indicator); >> } >> >> static void spapr_pci_register_types(void) >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> index 0ac1a19..fac85f8 100644 >> --- a/include/hw/ppc/spapr.h >> +++ b/include/hw/ppc/spapr.h >> @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { >> >> /* state for Dynamic Reconfiguration Connectors */ >> sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; >> + >> + /* Platform state - sensors and indicators */ >> + uint32_t state; > > Do you think it'd be possible to create a special DRC device that > contains all of its tables and global state and also exposes sensors and > indicators? That device could then get linked via qom links to the PHBs > for their slots. > > > Alex >
Quoting Alexander Graf (2014-08-26 06:36:57) > On 19.08.14 02:21, Michael Roth wrote: > > From: Mike Day <ncmike@ncultra.org> > > > > Signed-off-by: Mike Day <ncmike@ncultra.org> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr.h | 3 ++ > > 2 files changed, 122 insertions(+) > > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > index 924d488..23a3477 100644 > > --- a/hw/ppc/spapr_pci.c > > +++ b/hw/ppc/spapr_pci.c > > @@ -36,6 +36,16 @@ > > > > #include "hw/pci/pci_bus.h" > > > > +/* #define DEBUG_SPAPR */ > > + > > +#ifdef DEBUG_SPAPR > > +#define DPRINTF(fmt, ...) \ > > + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) > > +#else > > +#define DPRINTF(fmt, ...) \ > > + do { } while (0) > > +#endif > > + > > /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ > > #define RTAS_QUERY_FN 0 > > #define RTAS_CHANGE_FN 1 > > @@ -47,6 +57,31 @@ > > #define RTAS_TYPE_MSI 1 > > #define RTAS_TYPE_MSIX 2 > > > > +/* For set-indicator RTAS interface */ > > +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ > > +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ > > +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ > > +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ > > +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ > > +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ > > +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ > > +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ > > + > > +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ > > +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ > > +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ > > +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ > > +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ > > +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ > > +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ > > +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ > > + > > +#define DECODE_DRC_STATE(state, m, s) \ > > + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) > > + > > +#define ENCODE_DRC_STATE(val, m, s) \ > > + (((uint32_t)(val) << (s)) & (uint32_t)(m)) > > + > > static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) > > { > > sPAPRPHBState *sphb; > > @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > > rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ > > } > > > > +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, > > + uint32_t token, uint32_t nargs, > > + target_ulong args, uint32_t nret, > > + target_ulong rets) > > +{ > > + uint32_t indicator = rtas_ld(args, 0); > > + uint32_t drc_index = rtas_ld(args, 1); > > + uint32_t indicator_state = rtas_ld(args, 2); > > + uint32_t encoded = 0, shift = 0, mask = 0; > > + uint32_t *pind; > > + sPAPRDrcEntry *drc_entry = NULL; > > This rtas call does not have any idea what a PHB is. Why does it live in > spapr_pci.c? spapr_rtas.c does seem like a better home > > > + > > + if (drc_index == 0) { /* platform indicator */ > > + pind = &spapr->state; > > + } else { > > + drc_entry = spapr_find_drc_entry(drc_index); > > + if (!drc_entry) { > > + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", > > + drc_index); > > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > > + return; > > + } > > + pind = &drc_entry->state; > > + } > > + > > + switch (indicator) { > > + case 9: /* EPOW */ > > + shift = INDICATOR_EPOW_SHIFT; > > + mask = INDICATOR_EPOW_MASK; > > + break; > > + case 9001: /* Isolation state */ > > + /* encode the new value into the correct bit field */ > > + shift = INDICATOR_ISOLATION_SHIFT; > > + mask = INDICATOR_ISOLATION_MASK; > > + break; > > + case 9002: /* DR */ > > + shift = INDICATOR_DR_SHIFT; > > + mask = INDICATOR_DR_MASK; > > + break; > > + case 9003: /* Allocation State */ > > + shift = INDICATOR_ALLOCATION_SHIFT; > > + mask = INDICATOR_ALLOCATION_MASK; > > + break; > > + case 9005: /* global interrupt */ > > + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; > > + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; > > + break; > > + case 9006: /* error log */ > > + shift = INDICATOR_ERROR_LOG_SHIFT; > > + mask = INDICATOR_ERROR_LOG_MASK; > > + break; > > + case 9007: /* identify */ > > + shift = INDICATOR_IDENTIFY_SHIFT; > > + mask = INDICATOR_IDENTIFY_MASK; > > + break; > > + case 9009: /* reset */ > > + shift = INDICATOR_RESET_SHIFT; > > + mask = INDICATOR_RESET_MASK; > > + break; > > + default: > > + DPRINTF("rtas_set_indicator: indicator not implemented: %d", > > + indicator); > > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > > + return; > > + } > > + > > + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); > > + /* clear the current indicator value */ > > + *pind &= ~mask; > > + /* set the new value */ > > + *pind |= encoded; > > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > > +} > > + > > static int pci_spapr_swizzle(int slot, int pin) > > { > > return (slot + pin) % PCI_NUM_PINS; > > @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > > sphb->lsi_table[i].irq = irq; > > } > > > > + /* make sure the platform EPOW sensor is initialized - the > > + * guest will probe it when there is a hotplug event. > > + */ > > + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; > > + spapr->state |= ENCODE_DRC_STATE(0, > > + INDICATOR_EPOW_MASK, > > + INDICATOR_EPOW_SHIFT); > > + > > if (!info->finish_realize) { > > error_setg(errp, "finish_realize not defined"); > > return; > > @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) > > spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", > > rtas_ibm_change_msi); > > } > > + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", > > + rtas_set_indicator); > > } > > > > static void spapr_pci_register_types(void) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 0ac1a19..fac85f8 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { > > > > /* state for Dynamic Reconfiguration Connectors */ > > sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; > > + > > + /* Platform state - sensors and indicators */ > > + uint32_t state; > > Do you think it'd be possible to create a special DRC device that > contains all of its tables and global state and also exposes sensors and > indicators? That device could then get linked via qom links to the PHBs > for their slots. Sorry for the delay, I've been going back through the code with this suggestion in mind and there does seem to be a lot of state that can be nicely encapsulated by modeling the DR Connectors as a QOM "device" (though I haven't gone as far as to make them actual DeviceState's since it's more of a firmware abstraction than real hardware) I'm not sure what the best way to plumb things together is, as a first run, since each DRC must have a index drc_index as per spec, I've moved put them under /machine/DRConnector as a flat list, where top-level PHB/CPU/MEMORY DRCs would be allocated statically during sPAPR machine init (since the corresponding DRC indexes/types/etc are hard-coded into the top-level of the boot-time DT anyway, though I guess we could also allocate these on the fly...seems messier though than just plugging new resources into existing DRCs) PHB's in turn will associate themselves with a DRC via an attach/detach method as part of realize (and in the future, hotplug hooks, though that's not part of the series). The PHBs in turn will create a DRC for each hotpluggable PCI slot. Creation is via: sPAPRDRConnector *spapr_dr_connector_new(sPAPRDRConnectorType type, uint32_t id); where the code computes the drc index based on <type> (one of phb, cpu, pci, memory, etc) and <id>, and sticks them under /machine/dr-Connector/<drc_index> Any pci/phb/cpu hotplug hooks can then fetch the DRC via type/id, and hotplug/unplug via attach()/detach() methods. attach() adds the attached/hotplugged DeviceState as a link property of the DRC object, and sets the initial sensor state. rtas calls can fetch DRCs via drc_index, and set/get sensor state via DRC sensor get/set methods. Hotplug event delivery still lives outside of DRC implementation for now. I thought of moving them into DRC, but decisions like whether we should emit events during coldplug/initial boot seemed to require pushing a lot of general machine state into DRCs and making the encapsulation seem superficial. Things end up looking like this (2xxxxxxx are PHBs, 4xxxxxxx are PCI slots): mdroth@loki:~/w/qom/machine/dr-connector$ ls 20000000 40000018 40000038 40000058 40000078 40000098 400000b8 400000d8 400000f8 40000000 40000020 40000040 40000060 40000080 400000a0 400000c0 400000e0 type 40000008 40000028 40000048 40000068 40000088 400000a8 400000c8 400000e8 40000010 40000030 40000050 40000070 40000090 400000b0 400000d0 400000f0 mdroth@loki:~/w/qom/machine/dr-connector$ cd 40000000/ mdroth@loki:~/w/qom/machine/dr-connector/40000000$ ls -l total 0 -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 allocation-state lrwxr-xr-x 2 mdroth mdroth 4096 Dec 31 1969 device -> ../../../machine/peripheral/hp0 -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 drc-index -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 entity-sense -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 indicator-state -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 isolation-state -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 type mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat allocation-state 1 mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat indicator-state 1 mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat ../../../machine/peripheral/hp0/type virtio-net-pci mdroth@loki:~/w/qom/machine/dr-connector/40000000$ Hopefully this is sort of the approach you were thinking of? > > > Alex
On 01.10.14 00:08, Michael Roth wrote: > Quoting Alexander Graf (2014-08-26 06:36:57) >> On 19.08.14 02:21, Michael Roth wrote: >>> From: Mike Day <ncmike@ncultra.org> >>> >>> Signed-off-by: Mike Day <ncmike@ncultra.org> >>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/spapr.h | 3 ++ >>> 2 files changed, 122 insertions(+) >>> >>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>> index 924d488..23a3477 100644 >>> --- a/hw/ppc/spapr_pci.c >>> +++ b/hw/ppc/spapr_pci.c >>> @@ -36,6 +36,16 @@ >>> >>> #include "hw/pci/pci_bus.h" >>> >>> +/* #define DEBUG_SPAPR */ >>> + >>> +#ifdef DEBUG_SPAPR >>> +#define DPRINTF(fmt, ...) \ >>> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) >>> +#else >>> +#define DPRINTF(fmt, ...) \ >>> + do { } while (0) >>> +#endif >>> + >>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >>> #define RTAS_QUERY_FN 0 >>> #define RTAS_CHANGE_FN 1 >>> @@ -47,6 +57,31 @@ >>> #define RTAS_TYPE_MSI 1 >>> #define RTAS_TYPE_MSIX 2 >>> >>> +/* For set-indicator RTAS interface */ >>> +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ >>> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ >>> +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ >>> +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ >>> +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ >>> +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ >>> +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ >>> +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ >>> + >>> +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ >>> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ >>> +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ >>> +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ >>> +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ >>> +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ >>> +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ >>> +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ >>> + >>> +#define DECODE_DRC_STATE(state, m, s) \ >>> + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) >>> + >>> +#define ENCODE_DRC_STATE(val, m, s) \ >>> + (((uint32_t)(val) << (s)) & (uint32_t)(m)) >>> + >>> static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) >>> { >>> sPAPRPHBState *sphb; >>> @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, >>> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ >>> } >>> >>> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, >>> + uint32_t token, uint32_t nargs, >>> + target_ulong args, uint32_t nret, >>> + target_ulong rets) >>> +{ >>> + uint32_t indicator = rtas_ld(args, 0); >>> + uint32_t drc_index = rtas_ld(args, 1); >>> + uint32_t indicator_state = rtas_ld(args, 2); >>> + uint32_t encoded = 0, shift = 0, mask = 0; >>> + uint32_t *pind; >>> + sPAPRDrcEntry *drc_entry = NULL; >> >> This rtas call does not have any idea what a PHB is. Why does it live in >> spapr_pci.c? > > spapr_rtas.c does seem like a better home > >> >>> + >>> + if (drc_index == 0) { /* platform indicator */ >>> + pind = &spapr->state; >>> + } else { >>> + drc_entry = spapr_find_drc_entry(drc_index); >>> + if (!drc_entry) { >>> + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", >>> + drc_index); >>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>> + return; >>> + } >>> + pind = &drc_entry->state; >>> + } >>> + >>> + switch (indicator) { >>> + case 9: /* EPOW */ >>> + shift = INDICATOR_EPOW_SHIFT; >>> + mask = INDICATOR_EPOW_MASK; >>> + break; >>> + case 9001: /* Isolation state */ >>> + /* encode the new value into the correct bit field */ >>> + shift = INDICATOR_ISOLATION_SHIFT; >>> + mask = INDICATOR_ISOLATION_MASK; >>> + break; >>> + case 9002: /* DR */ >>> + shift = INDICATOR_DR_SHIFT; >>> + mask = INDICATOR_DR_MASK; >>> + break; >>> + case 9003: /* Allocation State */ >>> + shift = INDICATOR_ALLOCATION_SHIFT; >>> + mask = INDICATOR_ALLOCATION_MASK; >>> + break; >>> + case 9005: /* global interrupt */ >>> + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; >>> + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; >>> + break; >>> + case 9006: /* error log */ >>> + shift = INDICATOR_ERROR_LOG_SHIFT; >>> + mask = INDICATOR_ERROR_LOG_MASK; >>> + break; >>> + case 9007: /* identify */ >>> + shift = INDICATOR_IDENTIFY_SHIFT; >>> + mask = INDICATOR_IDENTIFY_MASK; >>> + break; >>> + case 9009: /* reset */ >>> + shift = INDICATOR_RESET_SHIFT; >>> + mask = INDICATOR_RESET_MASK; >>> + break; >>> + default: >>> + DPRINTF("rtas_set_indicator: indicator not implemented: %d", >>> + indicator); >>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>> + return; >>> + } >>> + >>> + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); >>> + /* clear the current indicator value */ >>> + *pind &= ~mask; >>> + /* set the new value */ >>> + *pind |= encoded; >>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>> +} >>> + >>> static int pci_spapr_swizzle(int slot, int pin) >>> { >>> return (slot + pin) % PCI_NUM_PINS; >>> @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) >>> sphb->lsi_table[i].irq = irq; >>> } >>> >>> + /* make sure the platform EPOW sensor is initialized - the >>> + * guest will probe it when there is a hotplug event. >>> + */ >>> + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; >>> + spapr->state |= ENCODE_DRC_STATE(0, >>> + INDICATOR_EPOW_MASK, >>> + INDICATOR_EPOW_SHIFT); >>> + >>> if (!info->finish_realize) { >>> error_setg(errp, "finish_realize not defined"); >>> return; >>> @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) >>> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", >>> rtas_ibm_change_msi); >>> } >>> + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", >>> + rtas_set_indicator); >>> } >>> >>> static void spapr_pci_register_types(void) >>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >>> index 0ac1a19..fac85f8 100644 >>> --- a/include/hw/ppc/spapr.h >>> +++ b/include/hw/ppc/spapr.h >>> @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { >>> >>> /* state for Dynamic Reconfiguration Connectors */ >>> sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; >>> + >>> + /* Platform state - sensors and indicators */ >>> + uint32_t state; >> >> Do you think it'd be possible to create a special DRC device that >> contains all of its tables and global state and also exposes sensors and >> indicators? That device could then get linked via qom links to the PHBs >> for their slots. > > Sorry for the delay, I've been going back through the code with this > suggestion in mind and there does seem to be a lot of state that > can be nicely encapsulated by modeling the DR Connectors as a QOM > "device" (though I haven't gone as far as to make them actual > DeviceState's since it's more of a firmware abstraction than real > hardware) > > I'm not sure what the best way to plumb things together is, as a first > run, since each DRC must have a index drc_index as per spec, I've moved > put them under /machine/DRConnector as a flat list, where top-level > PHB/CPU/MEMORY DRCs would be allocated statically during sPAPR machine > init (since the corresponding DRC indexes/types/etc are hard-coded into > the top-level of the boot-time DT anyway, though I guess we could also > allocate these on the fly...seems messier though than just plugging new > resources into existing DRCs) > > PHB's in turn will associate themselves with a DRC via an attach/detach > method as part of realize (and in the future, hotplug hooks, though > that's not part of the series). The PHBs in turn will create a DRC for each > hotpluggable PCI slot. > > Creation is via: > > sPAPRDRConnector *spapr_dr_connector_new(sPAPRDRConnectorType type, > uint32_t id); > > where the code computes the drc index based on <type> (one of phb, cpu, pci, > memory, etc) and <id>, and sticks them under /machine/dr-Connector/<drc_index> > > Any pci/phb/cpu hotplug hooks can then fetch the DRC via type/id, > and hotplug/unplug via attach()/detach() methods. attach() adds > the attached/hotplugged DeviceState as a link property of the > DRC object, and sets the initial sensor state. > > rtas calls can fetch DRCs via drc_index, and set/get sensor state > via DRC sensor get/set methods. > > Hotplug event delivery still lives outside of DRC implementation for now. I > thought of moving them into DRC, but decisions like whether we should > emit events during coldplug/initial boot seemed to require pushing > a lot of general machine state into DRCs and making the encapsulation > seem superficial. > > Things end up looking like this (2xxxxxxx are PHBs, 4xxxxxxx are PCI slots): > > mdroth@loki:~/w/qom/machine/dr-connector$ ls > 20000000 40000018 40000038 40000058 40000078 40000098 400000b8 400000d8 400000f8 > 40000000 40000020 40000040 40000060 40000080 400000a0 400000c0 400000e0 type > 40000008 40000028 40000048 40000068 40000088 400000a8 400000c8 400000e8 > 40000010 40000030 40000050 40000070 40000090 400000b0 400000d0 400000f0 > mdroth@loki:~/w/qom/machine/dr-connector$ cd 40000000/ > mdroth@loki:~/w/qom/machine/dr-connector/40000000$ ls -l > total 0 > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 allocation-state > lrwxr-xr-x 2 mdroth mdroth 4096 Dec 31 1969 device -> ../../../machine/peripheral/hp0 > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 drc-index > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 entity-sense > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 indicator-state > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 isolation-state > -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 type > mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat allocation-state > 1 > mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat indicator-state > 1 > mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat ../../../machine/peripheral/hp0/type > virtio-net-pci > mdroth@loki:~/w/qom/machine/dr-connector/40000000$ > > Hopefully this is sort of the approach you were thinking of? This look quite neat so far, looking forward to the patches :). Alex
On Wed, Oct 1, 2014 at 8:00 PM, Alexander Graf <agraf@suse.de> wrote: > > > On 01.10.14 00:08, Michael Roth wrote: >> Quoting Alexander Graf (2014-08-26 06:36:57) >>> On 19.08.14 02:21, Michael Roth wrote: >>>> From: Mike Day <ncmike@ncultra.org> >>>> >>>> Signed-off-by: Mike Day <ncmike@ncultra.org> >>>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>>> --- >>>> hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/hw/ppc/spapr.h | 3 ++ >>>> 2 files changed, 122 insertions(+) >>>> >>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>>> index 924d488..23a3477 100644 >>>> --- a/hw/ppc/spapr_pci.c >>>> +++ b/hw/ppc/spapr_pci.c >>>> @@ -36,6 +36,16 @@ >>>> >>>> #include "hw/pci/pci_bus.h" >>>> >>>> +/* #define DEBUG_SPAPR */ >>>> + >>>> +#ifdef DEBUG_SPAPR >>>> +#define DPRINTF(fmt, ...) \ >>>> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) >>>> +#else >>>> +#define DPRINTF(fmt, ...) \ >>>> + do { } while (0) >>>> +#endif >>>> + >>>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >>>> #define RTAS_QUERY_FN 0 >>>> #define RTAS_CHANGE_FN 1 >>>> @@ -47,6 +57,31 @@ >>>> #define RTAS_TYPE_MSI 1 >>>> #define RTAS_TYPE_MSIX 2 >>>> >>>> +/* For set-indicator RTAS interface */ >>>> +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ >>>> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ >>>> +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ >>>> +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ >>>> +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ >>>> +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ >>>> +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ >>>> +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ >>>> + >>>> +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ >>>> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ >>>> +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ >>>> +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ >>>> +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ >>>> +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ >>>> +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ >>>> +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ >>>> + >>>> +#define DECODE_DRC_STATE(state, m, s) \ >>>> + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) >>>> + >>>> +#define ENCODE_DRC_STATE(val, m, s) \ >>>> + (((uint32_t)(val) << (s)) & (uint32_t)(m)) >>>> + >>>> static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) >>>> { >>>> sPAPRPHBState *sphb; >>>> @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, >>>> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ >>>> } >>>> >>>> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, >>>> + uint32_t token, uint32_t nargs, >>>> + target_ulong args, uint32_t nret, >>>> + target_ulong rets) >>>> +{ >>>> + uint32_t indicator = rtas_ld(args, 0); >>>> + uint32_t drc_index = rtas_ld(args, 1); >>>> + uint32_t indicator_state = rtas_ld(args, 2); >>>> + uint32_t encoded = 0, shift = 0, mask = 0; >>>> + uint32_t *pind; >>>> + sPAPRDrcEntry *drc_entry = NULL; >>> >>> This rtas call does not have any idea what a PHB is. Why does it live in >>> spapr_pci.c? >> >> spapr_rtas.c does seem like a better home >> >>> >>>> + >>>> + if (drc_index == 0) { /* platform indicator */ >>>> + pind = &spapr->state; >>>> + } else { >>>> + drc_entry = spapr_find_drc_entry(drc_index); >>>> + if (!drc_entry) { >>>> + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", >>>> + drc_index); >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>>> + return; >>>> + } >>>> + pind = &drc_entry->state; >>>> + } >>>> + >>>> + switch (indicator) { >>>> + case 9: /* EPOW */ >>>> + shift = INDICATOR_EPOW_SHIFT; >>>> + mask = INDICATOR_EPOW_MASK; >>>> + break; >>>> + case 9001: /* Isolation state */ >>>> + /* encode the new value into the correct bit field */ >>>> + shift = INDICATOR_ISOLATION_SHIFT; >>>> + mask = INDICATOR_ISOLATION_MASK; >>>> + break; >>>> + case 9002: /* DR */ >>>> + shift = INDICATOR_DR_SHIFT; >>>> + mask = INDICATOR_DR_MASK; >>>> + break; >>>> + case 9003: /* Allocation State */ >>>> + shift = INDICATOR_ALLOCATION_SHIFT; >>>> + mask = INDICATOR_ALLOCATION_MASK; >>>> + break; >>>> + case 9005: /* global interrupt */ >>>> + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; >>>> + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; >>>> + break; >>>> + case 9006: /* error log */ >>>> + shift = INDICATOR_ERROR_LOG_SHIFT; >>>> + mask = INDICATOR_ERROR_LOG_MASK; >>>> + break; >>>> + case 9007: /* identify */ >>>> + shift = INDICATOR_IDENTIFY_SHIFT; >>>> + mask = INDICATOR_IDENTIFY_MASK; >>>> + break; >>>> + case 9009: /* reset */ >>>> + shift = INDICATOR_RESET_SHIFT; >>>> + mask = INDICATOR_RESET_MASK; >>>> + break; >>>> + default: >>>> + DPRINTF("rtas_set_indicator: indicator not implemented: %d", >>>> + indicator); >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>>> + return; >>>> + } >>>> + >>>> + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); >>>> + /* clear the current indicator value */ >>>> + *pind &= ~mask; >>>> + /* set the new value */ >>>> + *pind |= encoded; >>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>> +} >>>> + >>>> static int pci_spapr_swizzle(int slot, int pin) >>>> { >>>> return (slot + pin) % PCI_NUM_PINS; >>>> @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) >>>> sphb->lsi_table[i].irq = irq; >>>> } >>>> >>>> + /* make sure the platform EPOW sensor is initialized - the >>>> + * guest will probe it when there is a hotplug event. >>>> + */ >>>> + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; >>>> + spapr->state |= ENCODE_DRC_STATE(0, >>>> + INDICATOR_EPOW_MASK, >>>> + INDICATOR_EPOW_SHIFT); >>>> + >>>> if (!info->finish_realize) { >>>> error_setg(errp, "finish_realize not defined"); >>>> return; >>>> @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) >>>> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", >>>> rtas_ibm_change_msi); >>>> } >>>> + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", >>>> + rtas_set_indicator); >>>> } >>>> >>>> static void spapr_pci_register_types(void) >>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >>>> index 0ac1a19..fac85f8 100644 >>>> --- a/include/hw/ppc/spapr.h >>>> +++ b/include/hw/ppc/spapr.h >>>> @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { >>>> >>>> /* state for Dynamic Reconfiguration Connectors */ >>>> sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; >>>> + >>>> + /* Platform state - sensors and indicators */ >>>> + uint32_t state; >>> >>> Do you think it'd be possible to create a special DRC device that >>> contains all of its tables and global state and also exposes sensors and >>> indicators? That device could then get linked via qom links to the PHBs >>> for their slots. >> >> Sorry for the delay, I've been going back through the code with this >> suggestion in mind and there does seem to be a lot of state that >> can be nicely encapsulated by modeling the DR Connectors as a QOM >> "device" (though I haven't gone as far as to make them actual >> DeviceState's since it's more of a firmware abstraction than real >> hardware) >> >> I'm not sure what the best way to plumb things together is, as a first >> run, since each DRC must have a index drc_index as per spec, I've moved >> put them under /machine/DRConnector as a flat list, where top-level >> PHB/CPU/MEMORY DRCs would be allocated statically during sPAPR machine >> init (since the corresponding DRC indexes/types/etc are hard-coded into >> the top-level of the boot-time DT anyway, though I guess we could also >> allocate these on the fly...seems messier though than just plugging new >> resources into existing DRCs) >> >> PHB's in turn will associate themselves with a DRC via an attach/detach >> method as part of realize (and in the future, hotplug hooks, though >> that's not part of the series). The PHBs in turn will create a DRC for each >> hotpluggable PCI slot. >> >> Creation is via: >> >> sPAPRDRConnector *spapr_dr_connector_new(sPAPRDRConnectorType type, >> uint32_t id); >> >> where the code computes the drc index based on <type> (one of phb, cpu, pci, >> memory, etc) and <id>, and sticks them under /machine/dr-Connector/<drc_index> >> >> Any pci/phb/cpu hotplug hooks can then fetch the DRC via type/id, >> and hotplug/unplug via attach()/detach() methods. attach() adds >> the attached/hotplugged DeviceState as a link property of the >> DRC object, and sets the initial sensor state. >> >> rtas calls can fetch DRCs via drc_index, and set/get sensor state >> via DRC sensor get/set methods. >> >> Hotplug event delivery still lives outside of DRC implementation for now. I >> thought of moving them into DRC, but decisions like whether we should >> emit events during coldplug/initial boot seemed to require pushing >> a lot of general machine state into DRCs and making the encapsulation >> seem superficial. >> >> Things end up looking like this (2xxxxxxx are PHBs, 4xxxxxxx are PCI slots): >> >> mdroth@loki:~/w/qom/machine/dr-connector$ ls >> 20000000 40000018 40000038 40000058 40000078 40000098 400000b8 400000d8 400000f8 >> 40000000 40000020 40000040 40000060 40000080 400000a0 400000c0 400000e0 type >> 40000008 40000028 40000048 40000068 40000088 400000a8 400000c8 400000e8 >> 40000010 40000030 40000050 40000070 40000090 400000b0 400000d0 400000f0 >> mdroth@loki:~/w/qom/machine/dr-connector$ cd 40000000/ >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ ls -l >> total 0 >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 allocation-state >> lrwxr-xr-x 2 mdroth mdroth 4096 Dec 31 1969 device -> ../../../machine/peripheral/hp0 >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 drc-index >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 entity-sense >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 indicator-state >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 isolation-state >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 type >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat allocation-state >> 1 >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat indicator-state >> 1 >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat ../../../machine/peripheral/hp0/type >> virtio-net-pci >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ >> >> Hopefully this is sort of the approach you were thinking of? > > This look quite neat so far, looking forward to the patches :). Michael, Do you have this code/patches anywhere that I could use ? I have got the initial working versions of both CPU and memory hotplug now for sPAPR guests based on top of your old PCI hotplug patchset and it would be good to rebase them on top of your DR connector device work. Regards, Bharata.
On Wed, Oct 1, 2014 at 8:00 PM, Alexander Graf <agraf@suse.de> wrote: > > > On 01.10.14 00:08, Michael Roth wrote: >> Quoting Alexander Graf (2014-08-26 06:36:57) >>> On 19.08.14 02:21, Michael Roth wrote: >>>> From: Mike Day <ncmike@ncultra.org> >>>> >>>> Signed-off-by: Mike Day <ncmike@ncultra.org> >>>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>>> --- >>>> hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/hw/ppc/spapr.h | 3 ++ >>>> 2 files changed, 122 insertions(+) >>>> >>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>>> index 924d488..23a3477 100644 >>>> --- a/hw/ppc/spapr_pci.c >>>> +++ b/hw/ppc/spapr_pci.c >>>> @@ -36,6 +36,16 @@ >>>> >>>> #include "hw/pci/pci_bus.h" >>>> >>>> +/* #define DEBUG_SPAPR */ >>>> + >>>> +#ifdef DEBUG_SPAPR >>>> +#define DPRINTF(fmt, ...) \ >>>> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) >>>> +#else >>>> +#define DPRINTF(fmt, ...) \ >>>> + do { } while (0) >>>> +#endif >>>> + >>>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >>>> #define RTAS_QUERY_FN 0 >>>> #define RTAS_CHANGE_FN 1 >>>> @@ -47,6 +57,31 @@ >>>> #define RTAS_TYPE_MSI 1 >>>> #define RTAS_TYPE_MSIX 2 >>>> >>>> +/* For set-indicator RTAS interface */ >>>> +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ >>>> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ >>>> +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ >>>> +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ >>>> +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ >>>> +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ >>>> +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ >>>> +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ >>>> + >>>> +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ >>>> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ >>>> +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ >>>> +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ >>>> +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ >>>> +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ >>>> +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ >>>> +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ >>>> + >>>> +#define DECODE_DRC_STATE(state, m, s) \ >>>> + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) >>>> + >>>> +#define ENCODE_DRC_STATE(val, m, s) \ >>>> + (((uint32_t)(val) << (s)) & (uint32_t)(m)) >>>> + >>>> static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) >>>> { >>>> sPAPRPHBState *sphb; >>>> @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, >>>> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ >>>> } >>>> >>>> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, >>>> + uint32_t token, uint32_t nargs, >>>> + target_ulong args, uint32_t nret, >>>> + target_ulong rets) >>>> +{ >>>> + uint32_t indicator = rtas_ld(args, 0); >>>> + uint32_t drc_index = rtas_ld(args, 1); >>>> + uint32_t indicator_state = rtas_ld(args, 2); >>>> + uint32_t encoded = 0, shift = 0, mask = 0; >>>> + uint32_t *pind; >>>> + sPAPRDrcEntry *drc_entry = NULL; >>> >>> This rtas call does not have any idea what a PHB is. Why does it live in >>> spapr_pci.c? >> >> spapr_rtas.c does seem like a better home >> >>> >>>> + >>>> + if (drc_index == 0) { /* platform indicator */ >>>> + pind = &spapr->state; >>>> + } else { >>>> + drc_entry = spapr_find_drc_entry(drc_index); >>>> + if (!drc_entry) { >>>> + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", >>>> + drc_index); >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>>> + return; >>>> + } >>>> + pind = &drc_entry->state; >>>> + } >>>> + >>>> + switch (indicator) { >>>> + case 9: /* EPOW */ >>>> + shift = INDICATOR_EPOW_SHIFT; >>>> + mask = INDICATOR_EPOW_MASK; >>>> + break; >>>> + case 9001: /* Isolation state */ >>>> + /* encode the new value into the correct bit field */ >>>> + shift = INDICATOR_ISOLATION_SHIFT; >>>> + mask = INDICATOR_ISOLATION_MASK; >>>> + break; >>>> + case 9002: /* DR */ >>>> + shift = INDICATOR_DR_SHIFT; >>>> + mask = INDICATOR_DR_MASK; >>>> + break; >>>> + case 9003: /* Allocation State */ >>>> + shift = INDICATOR_ALLOCATION_SHIFT; >>>> + mask = INDICATOR_ALLOCATION_MASK; >>>> + break; >>>> + case 9005: /* global interrupt */ >>>> + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; >>>> + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; >>>> + break; >>>> + case 9006: /* error log */ >>>> + shift = INDICATOR_ERROR_LOG_SHIFT; >>>> + mask = INDICATOR_ERROR_LOG_MASK; >>>> + break; >>>> + case 9007: /* identify */ >>>> + shift = INDICATOR_IDENTIFY_SHIFT; >>>> + mask = INDICATOR_IDENTIFY_MASK; >>>> + break; >>>> + case 9009: /* reset */ >>>> + shift = INDICATOR_RESET_SHIFT; >>>> + mask = INDICATOR_RESET_MASK; >>>> + break; >>>> + default: >>>> + DPRINTF("rtas_set_indicator: indicator not implemented: %d", >>>> + indicator); >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >>>> + return; >>>> + } >>>> + >>>> + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); >>>> + /* clear the current indicator value */ >>>> + *pind &= ~mask; >>>> + /* set the new value */ >>>> + *pind |= encoded; >>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>> +} >>>> + >>>> static int pci_spapr_swizzle(int slot, int pin) >>>> { >>>> return (slot + pin) % PCI_NUM_PINS; >>>> @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) >>>> sphb->lsi_table[i].irq = irq; >>>> } >>>> >>>> + /* make sure the platform EPOW sensor is initialized - the >>>> + * guest will probe it when there is a hotplug event. >>>> + */ >>>> + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; >>>> + spapr->state |= ENCODE_DRC_STATE(0, >>>> + INDICATOR_EPOW_MASK, >>>> + INDICATOR_EPOW_SHIFT); >>>> + >>>> if (!info->finish_realize) { >>>> error_setg(errp, "finish_realize not defined"); >>>> return; >>>> @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) >>>> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", >>>> rtas_ibm_change_msi); >>>> } >>>> + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", >>>> + rtas_set_indicator); >>>> } >>>> >>>> static void spapr_pci_register_types(void) >>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >>>> index 0ac1a19..fac85f8 100644 >>>> --- a/include/hw/ppc/spapr.h >>>> +++ b/include/hw/ppc/spapr.h >>>> @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { >>>> >>>> /* state for Dynamic Reconfiguration Connectors */ >>>> sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; >>>> + >>>> + /* Platform state - sensors and indicators */ >>>> + uint32_t state; >>> >>> Do you think it'd be possible to create a special DRC device that >>> contains all of its tables and global state and also exposes sensors and >>> indicators? That device could then get linked via qom links to the PHBs >>> for their slots. >> >> Sorry for the delay, I've been going back through the code with this >> suggestion in mind and there does seem to be a lot of state that >> can be nicely encapsulated by modeling the DR Connectors as a QOM >> "device" (though I haven't gone as far as to make them actual >> DeviceState's since it's more of a firmware abstraction than real >> hardware) >> >> I'm not sure what the best way to plumb things together is, as a first >> run, since each DRC must have a index drc_index as per spec, I've moved >> put them under /machine/DRConnector as a flat list, where top-level >> PHB/CPU/MEMORY DRCs would be allocated statically during sPAPR machine >> init (since the corresponding DRC indexes/types/etc are hard-coded into >> the top-level of the boot-time DT anyway, though I guess we could also >> allocate these on the fly...seems messier though than just plugging new >> resources into existing DRCs) >> >> PHB's in turn will associate themselves with a DRC via an attach/detach >> method as part of realize (and in the future, hotplug hooks, though >> that's not part of the series). The PHBs in turn will create a DRC for each >> hotpluggable PCI slot. >> >> Creation is via: >> >> sPAPRDRConnector *spapr_dr_connector_new(sPAPRDRConnectorType type, >> uint32_t id); >> >> where the code computes the drc index based on <type> (one of phb, cpu, pci, >> memory, etc) and <id>, and sticks them under /machine/dr-Connector/<drc_index> >> >> Any pci/phb/cpu hotplug hooks can then fetch the DRC via type/id, >> and hotplug/unplug via attach()/detach() methods. attach() adds >> the attached/hotplugged DeviceState as a link property of the >> DRC object, and sets the initial sensor state. >> >> rtas calls can fetch DRCs via drc_index, and set/get sensor state >> via DRC sensor get/set methods. >> >> Hotplug event delivery still lives outside of DRC implementation for now. I >> thought of moving them into DRC, but decisions like whether we should >> emit events during coldplug/initial boot seemed to require pushing >> a lot of general machine state into DRCs and making the encapsulation >> seem superficial. >> >> Things end up looking like this (2xxxxxxx are PHBs, 4xxxxxxx are PCI slots): >> >> mdroth@loki:~/w/qom/machine/dr-connector$ ls >> 20000000 40000018 40000038 40000058 40000078 40000098 400000b8 400000d8 400000f8 >> 40000000 40000020 40000040 40000060 40000080 400000a0 400000c0 400000e0 type >> 40000008 40000028 40000048 40000068 40000088 400000a8 400000c8 400000e8 >> 40000010 40000030 40000050 40000070 40000090 400000b0 400000d0 400000f0 >> mdroth@loki:~/w/qom/machine/dr-connector$ cd 40000000/ >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ ls -l >> total 0 >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 allocation-state >> lrwxr-xr-x 2 mdroth mdroth 4096 Dec 31 1969 device -> ../../../machine/peripheral/hp0 >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 drc-index >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 entity-sense >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 indicator-state >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 isolation-state >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 type >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat allocation-state >> 1 >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat indicator-state >> 1 >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat ../../../machine/peripheral/hp0/type >> virtio-net-pci >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ >> >> Hopefully this is sort of the approach you were thinking of? > > This look quite neat so far, looking forward to the patches :). Michael, Do you have this code/patches anywhere that I could use ? I have got the initial working versions of both CPU and memory hotplug now for sPAPR guests based on top of your old PCI hotplug patchset and it would be good to rebase them on top of your DR connector device work. Regards, Bharata.
Quoting Bharata B Rao (2014-11-25 22:54:12) > On Wed, Oct 1, 2014 at 8:00 PM, Alexander Graf <agraf@suse.de> wrote: > > > > > > On 01.10.14 00:08, Michael Roth wrote: > >> Quoting Alexander Graf (2014-08-26 06:36:57) > >>> On 19.08.14 02:21, Michael Roth wrote: > >>>> From: Mike Day <ncmike@ncultra.org> > >>>> > >>>> Signed-off-by: Mike Day <ncmike@ncultra.org> > >>>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > >>>> --- > >>>> hw/ppc/spapr_pci.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> include/hw/ppc/spapr.h | 3 ++ > >>>> 2 files changed, 122 insertions(+) > >>>> > >>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > >>>> index 924d488..23a3477 100644 > >>>> --- a/hw/ppc/spapr_pci.c > >>>> +++ b/hw/ppc/spapr_pci.c > >>>> @@ -36,6 +36,16 @@ > >>>> > >>>> #include "hw/pci/pci_bus.h" > >>>> > >>>> +/* #define DEBUG_SPAPR */ > >>>> + > >>>> +#ifdef DEBUG_SPAPR > >>>> +#define DPRINTF(fmt, ...) \ > >>>> + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) > >>>> +#else > >>>> +#define DPRINTF(fmt, ...) \ > >>>> + do { } while (0) > >>>> +#endif > >>>> + > >>>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ > >>>> #define RTAS_QUERY_FN 0 > >>>> #define RTAS_CHANGE_FN 1 > >>>> @@ -47,6 +57,31 @@ > >>>> #define RTAS_TYPE_MSI 1 > >>>> #define RTAS_TYPE_MSIX 2 > >>>> > >>>> +/* For set-indicator RTAS interface */ > >>>> +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ > >>>> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ > >>>> +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ > >>>> +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ > >>>> +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ > >>>> +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ > >>>> +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ > >>>> +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ > >>>> + > >>>> +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ > >>>> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ > >>>> +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ > >>>> +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ > >>>> +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ > >>>> +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ > >>>> +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ > >>>> +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ > >>>> + > >>>> +#define DECODE_DRC_STATE(state, m, s) \ > >>>> + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) > >>>> + > >>>> +#define ENCODE_DRC_STATE(val, m, s) \ > >>>> + (((uint32_t)(val) << (s)) & (uint32_t)(m)) > >>>> + > >>>> static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) > >>>> { > >>>> sPAPRPHBState *sphb; > >>>> @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > >>>> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ > >>>> } > >>>> > >>>> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, > >>>> + uint32_t token, uint32_t nargs, > >>>> + target_ulong args, uint32_t nret, > >>>> + target_ulong rets) > >>>> +{ > >>>> + uint32_t indicator = rtas_ld(args, 0); > >>>> + uint32_t drc_index = rtas_ld(args, 1); > >>>> + uint32_t indicator_state = rtas_ld(args, 2); > >>>> + uint32_t encoded = 0, shift = 0, mask = 0; > >>>> + uint32_t *pind; > >>>> + sPAPRDrcEntry *drc_entry = NULL; > >>> > >>> This rtas call does not have any idea what a PHB is. Why does it live in > >>> spapr_pci.c? > >> > >> spapr_rtas.c does seem like a better home > >> > >>> > >>>> + > >>>> + if (drc_index == 0) { /* platform indicator */ > >>>> + pind = &spapr->state; > >>>> + } else { > >>>> + drc_entry = spapr_find_drc_entry(drc_index); > >>>> + if (!drc_entry) { > >>>> + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", > >>>> + drc_index); > >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > >>>> + return; > >>>> + } > >>>> + pind = &drc_entry->state; > >>>> + } > >>>> + > >>>> + switch (indicator) { > >>>> + case 9: /* EPOW */ > >>>> + shift = INDICATOR_EPOW_SHIFT; > >>>> + mask = INDICATOR_EPOW_MASK; > >>>> + break; > >>>> + case 9001: /* Isolation state */ > >>>> + /* encode the new value into the correct bit field */ > >>>> + shift = INDICATOR_ISOLATION_SHIFT; > >>>> + mask = INDICATOR_ISOLATION_MASK; > >>>> + break; > >>>> + case 9002: /* DR */ > >>>> + shift = INDICATOR_DR_SHIFT; > >>>> + mask = INDICATOR_DR_MASK; > >>>> + break; > >>>> + case 9003: /* Allocation State */ > >>>> + shift = INDICATOR_ALLOCATION_SHIFT; > >>>> + mask = INDICATOR_ALLOCATION_MASK; > >>>> + break; > >>>> + case 9005: /* global interrupt */ > >>>> + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; > >>>> + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; > >>>> + break; > >>>> + case 9006: /* error log */ > >>>> + shift = INDICATOR_ERROR_LOG_SHIFT; > >>>> + mask = INDICATOR_ERROR_LOG_MASK; > >>>> + break; > >>>> + case 9007: /* identify */ > >>>> + shift = INDICATOR_IDENTIFY_SHIFT; > >>>> + mask = INDICATOR_IDENTIFY_MASK; > >>>> + break; > >>>> + case 9009: /* reset */ > >>>> + shift = INDICATOR_RESET_SHIFT; > >>>> + mask = INDICATOR_RESET_MASK; > >>>> + break; > >>>> + default: > >>>> + DPRINTF("rtas_set_indicator: indicator not implemented: %d", > >>>> + indicator); > >>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > >>>> + return; > >>>> + } > >>>> + > >>>> + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); > >>>> + /* clear the current indicator value */ > >>>> + *pind &= ~mask; > >>>> + /* set the new value */ > >>>> + *pind |= encoded; > >>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > >>>> +} > >>>> + > >>>> static int pci_spapr_swizzle(int slot, int pin) > >>>> { > >>>> return (slot + pin) % PCI_NUM_PINS; > >>>> @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > >>>> sphb->lsi_table[i].irq = irq; > >>>> } > >>>> > >>>> + /* make sure the platform EPOW sensor is initialized - the > >>>> + * guest will probe it when there is a hotplug event. > >>>> + */ > >>>> + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; > >>>> + spapr->state |= ENCODE_DRC_STATE(0, > >>>> + INDICATOR_EPOW_MASK, > >>>> + INDICATOR_EPOW_SHIFT); > >>>> + > >>>> if (!info->finish_realize) { > >>>> error_setg(errp, "finish_realize not defined"); > >>>> return; > >>>> @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) > >>>> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", > >>>> rtas_ibm_change_msi); > >>>> } > >>>> + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", > >>>> + rtas_set_indicator); > >>>> } > >>>> > >>>> static void spapr_pci_register_types(void) > >>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > >>>> index 0ac1a19..fac85f8 100644 > >>>> --- a/include/hw/ppc/spapr.h > >>>> +++ b/include/hw/ppc/spapr.h > >>>> @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { > >>>> > >>>> /* state for Dynamic Reconfiguration Connectors */ > >>>> sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; > >>>> + > >>>> + /* Platform state - sensors and indicators */ > >>>> + uint32_t state; > >>> > >>> Do you think it'd be possible to create a special DRC device that > >>> contains all of its tables and global state and also exposes sensors and > >>> indicators? That device could then get linked via qom links to the PHBs > >>> for their slots. > >> > >> Sorry for the delay, I've been going back through the code with this > >> suggestion in mind and there does seem to be a lot of state that > >> can be nicely encapsulated by modeling the DR Connectors as a QOM > >> "device" (though I haven't gone as far as to make them actual > >> DeviceState's since it's more of a firmware abstraction than real > >> hardware) > >> > >> I'm not sure what the best way to plumb things together is, as a first > >> run, since each DRC must have a index drc_index as per spec, I've moved > >> put them under /machine/DRConnector as a flat list, where top-level > >> PHB/CPU/MEMORY DRCs would be allocated statically during sPAPR machine > >> init (since the corresponding DRC indexes/types/etc are hard-coded into > >> the top-level of the boot-time DT anyway, though I guess we could also > >> allocate these on the fly...seems messier though than just plugging new > >> resources into existing DRCs) > >> > >> PHB's in turn will associate themselves with a DRC via an attach/detach > >> method as part of realize (and in the future, hotplug hooks, though > >> that's not part of the series). The PHBs in turn will create a DRC for each > >> hotpluggable PCI slot. > >> > >> Creation is via: > >> > >> sPAPRDRConnector *spapr_dr_connector_new(sPAPRDRConnectorType type, > >> uint32_t id); > >> > >> where the code computes the drc index based on <type> (one of phb, cpu, pci, > >> memory, etc) and <id>, and sticks them under /machine/dr-Connector/<drc_index> > >> > >> Any pci/phb/cpu hotplug hooks can then fetch the DRC via type/id, > >> and hotplug/unplug via attach()/detach() methods. attach() adds > >> the attached/hotplugged DeviceState as a link property of the > >> DRC object, and sets the initial sensor state. > >> > >> rtas calls can fetch DRCs via drc_index, and set/get sensor state > >> via DRC sensor get/set methods. > >> > >> Hotplug event delivery still lives outside of DRC implementation for now. I > >> thought of moving them into DRC, but decisions like whether we should > >> emit events during coldplug/initial boot seemed to require pushing > >> a lot of general machine state into DRCs and making the encapsulation > >> seem superficial. > >> > >> Things end up looking like this (2xxxxxxx are PHBs, 4xxxxxxx are PCI slots): > >> > >> mdroth@loki:~/w/qom/machine/dr-connector$ ls > >> 20000000 40000018 40000038 40000058 40000078 40000098 400000b8 400000d8 400000f8 > >> 40000000 40000020 40000040 40000060 40000080 400000a0 400000c0 400000e0 type > >> 40000008 40000028 40000048 40000068 40000088 400000a8 400000c8 400000e8 > >> 40000010 40000030 40000050 40000070 40000090 400000b0 400000d0 400000f0 > >> mdroth@loki:~/w/qom/machine/dr-connector$ cd 40000000/ > >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ ls -l > >> total 0 > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 allocation-state > >> lrwxr-xr-x 2 mdroth mdroth 4096 Dec 31 1969 device -> ../../../machine/peripheral/hp0 > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 drc-index > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 entity-sense > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 indicator-state > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 isolation-state > >> -rw-r--r-- 1 mdroth mdroth 4096 Dec 31 1969 type > >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat allocation-state > >> 1 > >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat indicator-state > >> 1 > >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ cat ../../../machine/peripheral/hp0/type > >> virtio-net-pci > >> mdroth@loki:~/w/qom/machine/dr-connector/40000000$ > >> > >> Hopefully this is sort of the approach you were thinking of? > > > > This look quite neat so far, looking forward to the patches :). > > Michael, > > Do you have this code/patches anywhere that I could use ? I have got > the initial working versions of both CPU and memory hotplug now for > sPAPR guests based on top of your old PCI hotplug patchset and it > would be good to rebase them on top of your DR connector device work. Hi Bharata, Here's the latest branch: https://github.com/mdroth/qemu/commits/spapr-pci-hotplug-ppc-next-cleanup4.2 The sPAPRDREntry stuff is now modeled by the sPAPRDRConnector QOM object in hw/ppc/spapr_drc.c, which manages the device's life-cycle based on rtas-set-sensor-state calls from the guest. As part of qemu-side hotplug/unplug you use the attach/detach methods of the DRC to associate DT bits and callbacks for things like device cleanup or rtas calls to fetch a DT node from the device associated with a particular DRC. I still need to fix endian issues, and am realizing the dr connectors and DT bits for PHBs are not actually a prereq for PCI hotplug, so I may be pulling that out to a separate series specific to enabling PHB hotplug (namely for VFIO hotplug). I realize your CPU/MEM sort of depend on the top-level PHB device tree code so I'm not sure how best to deal with that. Worse case we'd roll the initial code into your series and base a follow-up series on that of that instead. Let me know if you have any questions. > > Regards, > Bharata.
On Wed, Nov 26, 2014 at 11:57 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > https://github.com/mdroth/qemu/commits/spapr-pci-hotplug-ppc-next-cleanup4.2 > > The sPAPRDREntry stuff is now modeled by the sPAPRDRConnector QOM object in > hw/ppc/spapr_drc.c, which manages the device's life-cycle based on > rtas-set-sensor-state calls from the guest. As part of qemu-side hotplug/unplug > you use the attach/detach methods of the DRC to associate DT bits and callbacks > for things like device cleanup or rtas calls to fetch a DT node from the device > associated with a particular DRC. > > I still need to fix endian issues, and am realizing the dr connectors and DT > bits for PHBs are not actually a prereq for PCI hotplug, so I may be pulling > that out to a separate series specific to enabling PHB hotplug (namely for > VFIO hotplug). I realize your CPU/MEM sort of depend on the top-level PHB > device tree code so I'm not sure how best to deal with that. Worse case we'd > roll the initial code into your series and base a follow-up series on that of > that instead. Thanks Michael for pointing me to your git tree. I started rebasing my patchset on top of yours and realized that the generic DT setup code from the below commits of your branch are needed for CPU and memory hotplug too. They all apply in the order I have listed below. 71b32999c4eb spapr_drc: initial implementation 255c50200848 spapr: populate DRC entries for root dt node (don't need code that adds PHB DT entries) 408206fc627e3 spapr_rtas: add set-indicator RTAS interface da7a232fa6a44 spapr_rtas: add get-sensor-state RTAS interface 1c575d5b29688 spapr_rtas: add ibm,configure-connector RTAS interface 0c5d72833666c spapr_events: re-use EPOW event infrastructure for hotplug events 82ee5a9c88155 spapr_events: event-scan RTAS interface If you can make the above set an independent patchset, it will become easy to maintain and post CPU and memory hotplug patchsets. I am facing some endian issues in your patchset and I will send fixes for those separately. Regards, Bharata.
Quoting Bharata B Rao (2014-11-30 22:57:48) > On Wed, Nov 26, 2014 at 11:57 AM, Michael Roth > <mdroth@linux.vnet.ibm.com> wrote: > > https://github.com/mdroth/qemu/commits/spapr-pci-hotplug-ppc-next-cleanup4.2 > > > > The sPAPRDREntry stuff is now modeled by the sPAPRDRConnector QOM object in > > hw/ppc/spapr_drc.c, which manages the device's life-cycle based on > > rtas-set-sensor-state calls from the guest. As part of qemu-side hotplug/unplug > > you use the attach/detach methods of the DRC to associate DT bits and callbacks > > for things like device cleanup or rtas calls to fetch a DT node from the device > > associated with a particular DRC. > > > > I still need to fix endian issues, and am realizing the dr connectors and DT > > bits for PHBs are not actually a prereq for PCI hotplug, so I may be pulling > > that out to a separate series specific to enabling PHB hotplug (namely for > > VFIO hotplug). I realize your CPU/MEM sort of depend on the top-level PHB > > device tree code so I'm not sure how best to deal with that. Worse case we'd > > roll the initial code into your series and base a follow-up series on that of > > that instead. > > Thanks Michael for pointing me to your git tree. > > I started rebasing my patchset on top of yours and realized that the > generic DT setup code from the below commits of your branch are needed > for CPU and memory hotplug too. They all apply in the order I have > listed below. > > 71b32999c4eb spapr_drc: initial implementation > 255c50200848 spapr: populate DRC entries for root dt node (don't need > code that adds PHB DT entries) > 408206fc627e3 spapr_rtas: add set-indicator RTAS interface > da7a232fa6a44 spapr_rtas: add get-sensor-state RTAS interface > 1c575d5b29688 spapr_rtas: add ibm,configure-connector RTAS interface > 0c5d72833666c spapr_events: re-use EPOW event infrastructure for hotplug events > 82ee5a9c88155 spapr_events: event-scan RTAS interface > > If you can make the above set an independent patchset, it will become > easy to maintain and post CPU and memory hotplug patchsets. Hi Bharata, I've submitted v4 of PCI hotplug. The development branch is here: https://github.com/mdroth/qemu/commits/spapr-hotplug-pci and is based on top of a 'core' branch organized similar to what you proposed: https://github.com/mdroth/qemu/commits/spapr-hotplug-core I'll be rolling changes for core/pci code into the branches as we go. The endian fixes you provided are included, and PCI hotplug has been tested on ppc64le. There's a pseries-2.3 in the core patchset to enable/disable dynamic-reconfiguration for individual resources on a machine basis to maintain backward migration compatibility. There's a PHB hotplug patchset based on core that might be a good reference for re-basing CPU/memory: https://github.com/mdroth/qemu/commits/spapr-hotplug-phb > > I am facing some endian issues in your patchset and I will send fixes > for those separately. > > Regards, > Bharata.
On Tue, Dec 23, 2014 at 8:42 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > > Hi Bharata, > > I've submitted v4 of PCI hotplug. The development branch is here: > > https://github.com/mdroth/qemu/commits/spapr-hotplug-pci > > and is based on top of a 'core' branch organized similar to what you proposed: > > https://github.com/mdroth/qemu/commits/spapr-hotplug-core > > I'll be rolling changes for core/pci code into the branches as we go. > The endian fixes you provided are included, and PCI hotplug has been > tested on ppc64le. > > There's a pseries-2.3 in the core patchset to enable/disable > dynamic-reconfiguration for individual resources on a machine basis to > maintain backward migration compatibility. There's a PHB hotplug patchset > based on core that might be a good reference for re-basing CPU/memory: > > https://github.com/mdroth/qemu/commits/spapr-hotplug-phb Thanks Michael for providing the 'core' branch. I shall be posting CPU and memory hotplug patches based on your 'core' branch shortly. Regards, Bharata.
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 924d488..23a3477 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -36,6 +36,16 @@ #include "hw/pci/pci_bus.h" +/* #define DEBUG_SPAPR */ + +#ifdef DEBUG_SPAPR +#define DPRINTF(fmt, ...) \ + do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt, ...) \ + do { } while (0) +#endif + /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ #define RTAS_QUERY_FN 0 #define RTAS_CHANGE_FN 1 @@ -47,6 +57,31 @@ #define RTAS_TYPE_MSI 1 #define RTAS_TYPE_MSIX 2 +/* For set-indicator RTAS interface */ +#define INDICATOR_ISOLATION_MASK 0x0001 /* 9001 one bit */ +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002 /* 9005 one bit */ +#define INDICATOR_ERROR_LOG_MASK 0x0004 /* 9006 one bit */ +#define INDICATOR_IDENTIFY_MASK 0x0008 /* 9007 one bit */ +#define INDICATOR_RESET_MASK 0x0010 /* 9009 one bit */ +#define INDICATOR_DR_MASK 0x00e0 /* 9002 three bits */ +#define INDICATOR_ALLOCATION_MASK 0x0300 /* 9003 two bits */ +#define INDICATOR_EPOW_MASK 0x1c00 /* 9 three bits */ + +#define INDICATOR_ISOLATION_SHIFT 0x00 /* bit 0 */ +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT 0x01 /* bit 1 */ +#define INDICATOR_ERROR_LOG_SHIFT 0x02 /* bit 2 */ +#define INDICATOR_IDENTIFY_SHIFT 0x03 /* bit 3 */ +#define INDICATOR_RESET_SHIFT 0x04 /* bit 4 */ +#define INDICATOR_DR_SHIFT 0x05 /* bits 5-7 */ +#define INDICATOR_ALLOCATION_SHIFT 0x08 /* bits 8-9 */ +#define INDICATOR_EPOW_SHIFT 0x0a /* bits 10-12 */ + +#define DECODE_DRC_STATE(state, m, s) \ + ((((uint32_t)(state) & (uint32_t)(m))) >> (s)) + +#define ENCODE_DRC_STATE(val, m, s) \ + (((uint32_t)(val) << (s)) & (uint32_t)(m)) + static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) { sPAPRPHBState *sphb; @@ -402,6 +437,80 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ } +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + uint32_t indicator = rtas_ld(args, 0); + uint32_t drc_index = rtas_ld(args, 1); + uint32_t indicator_state = rtas_ld(args, 2); + uint32_t encoded = 0, shift = 0, mask = 0; + uint32_t *pind; + sPAPRDrcEntry *drc_entry = NULL; + + if (drc_index == 0) { /* platform indicator */ + pind = &spapr->state; + } else { + drc_entry = spapr_find_drc_entry(drc_index); + if (!drc_entry) { + DPRINTF("rtas_set_indicator: unable to find drc_entry for %x", + drc_index); + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); + return; + } + pind = &drc_entry->state; + } + + switch (indicator) { + case 9: /* EPOW */ + shift = INDICATOR_EPOW_SHIFT; + mask = INDICATOR_EPOW_MASK; + break; + case 9001: /* Isolation state */ + /* encode the new value into the correct bit field */ + shift = INDICATOR_ISOLATION_SHIFT; + mask = INDICATOR_ISOLATION_MASK; + break; + case 9002: /* DR */ + shift = INDICATOR_DR_SHIFT; + mask = INDICATOR_DR_MASK; + break; + case 9003: /* Allocation State */ + shift = INDICATOR_ALLOCATION_SHIFT; + mask = INDICATOR_ALLOCATION_MASK; + break; + case 9005: /* global interrupt */ + shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT; + mask = INDICATOR_GLOBAL_INTERRUPT_MASK; + break; + case 9006: /* error log */ + shift = INDICATOR_ERROR_LOG_SHIFT; + mask = INDICATOR_ERROR_LOG_MASK; + break; + case 9007: /* identify */ + shift = INDICATOR_IDENTIFY_SHIFT; + mask = INDICATOR_IDENTIFY_MASK; + break; + case 9009: /* reset */ + shift = INDICATOR_RESET_SHIFT; + mask = INDICATOR_RESET_MASK; + break; + default: + DPRINTF("rtas_set_indicator: indicator not implemented: %d", + indicator); + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); + return; + } + + encoded = ENCODE_DRC_STATE(indicator_state, mask, shift); + /* clear the current indicator value */ + *pind &= ~mask; + /* set the new value */ + *pind |= encoded; + rtas_st(rets, 0, RTAS_OUT_SUCCESS); +} + static int pci_spapr_swizzle(int slot, int pin) { return (slot + pin) % PCI_NUM_PINS; @@ -624,6 +733,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) sphb->lsi_table[i].irq = irq; } + /* make sure the platform EPOW sensor is initialized - the + * guest will probe it when there is a hotplug event. + */ + spapr->state &= ~(uint32_t)INDICATOR_EPOW_MASK; + spapr->state |= ENCODE_DRC_STATE(0, + INDICATOR_EPOW_MASK, + INDICATOR_EPOW_SHIFT); + if (!info->finish_realize) { error_setg(errp, "finish_realize not defined"); return; @@ -1056,6 +1173,8 @@ void spapr_pci_rtas_init(void) spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", rtas_ibm_change_msi); } + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", + rtas_set_indicator); } static void spapr_pci_register_types(void) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 0ac1a19..fac85f8 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -72,6 +72,9 @@ typedef struct sPAPREnvironment { /* state for Dynamic Reconfiguration Connectors */ sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; + + /* Platform state - sensors and indicators */ + uint32_t state; } sPAPREnvironment; #define H_SUCCESS 0