From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57198) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPjh7-0001zn-JQ for qemu-devel@nongnu.org; Thu, 04 Sep 2014 22:56:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XPjgs-0005ov-PS for qemu-devel@nongnu.org; Thu, 04 Sep 2014 22:56:13 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:47859) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPjgs-0005oo-GT for qemu-devel@nongnu.org; Thu, 04 Sep 2014 22:55:58 -0400 Received: from /spool/local by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 4 Sep 2014 20:55:56 -0600 Message-ID: <54092638.4050506@linux.vnet.ibm.com> Date: Thu, 04 Sep 2014 21:55:52 -0500 From: Nathan Fontenot MIME-Version: 1.0 References: <1408407718-10835-1-git-send-email-mdroth@linux.vnet.ibm.com> <1408407718-10835-5-git-send-email-mdroth@linux.vnet.ibm.com> <53FC7159.6040406@suse.de> In-Reply-To: <53FC7159.6040406@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/12] spapr_pci: add set-indicator RTAS interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , Michael Roth , qemu-devel@nongnu.org Cc: aik@ozlabs.ru, ncmike@ncultra.org, qemu-ppc@nongnu.org, tyreld@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 >> >> Signed-off-by: Mike Day >> Signed-off-by: Michael Roth >> --- >> 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 >