Re: [Qemu-devel] [PATCH 04/12] spapr_pci: add set-indicator RTAS interface

[Nathan Fontenot <nfont@linux.vnet.ibm.com>]

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
>