From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58114) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzRwd-0001L1-DG for qemu-devel@nongnu.org; Tue, 24 Jun 2014 10:43:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzRwT-0006ak-PT for qemu-devel@nongnu.org; Tue, 24 Jun 2014 10:43:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:54567 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzRwT-0006Zk-F4 for qemu-devel@nongnu.org; Tue, 24 Jun 2014 10:43:25 -0400 Message-ID: <53A98E8B.7010302@suse.de> Date: Tue, 24 Jun 2014 16:43:23 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402365785-31620-1-git-send-email-gwshan@linux.vnet.ibm.com> <1402365785-31620-2-git-send-email-gwshan@linux.vnet.ibm.com> In-Reply-To: <1402365785-31620-2-git-send-email-gwshan@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v10 1/3] sPAPR: Implement EEH RTAS calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gavin Shan , qemu-devel@nongnu.org Cc: aik@ozlabs.ru, alex.williamson@redhat.com, qiudayu@linux.vnet.ibm.com On 10.06.14 04:03, Gavin Shan wrote: > The emulation for EEH RTAS requests from guest isn't covered > by QEMU yet and the patch implements them. > > The patch defines constants used by EEH RTAS calls and adds > callback sPAPRPHBClass::eeh_handler, which is going to be used > this way: > > 1. RTAS calls are received in spapr_pci.c, sanity check is done > there. > 2. RTAS handlers handle what they can. If there is something it > cannot handle and sPAPRPHBClass::eeh_handler callback is defined, > it is called. > 3. sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It > does ioctl() to the IOMMU container fd to complete the call. Error > codes from that ioctl() are transferred back to the guest. > > Signed-off-by: Gavin Shan > --- > hw/ppc/spapr_pci.c | 248 ++++++++++++++++++++++++++++++++++++++++++++ > include/hw/pci-host/spapr.h | 7 ++ > include/hw/ppc/spapr.h | 33 ++++++ > 3 files changed, 288 insertions(+) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 131434b..bfea488 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -422,6 +422,241 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ > } > > +static int rtas_handle_eeh_request(sPAPRPHBState *sphb, > + uint32_t req, uint32_t opt, > + target_ulong rets) > +{ > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + int ret; > + > + ret = info->eeh_handler(sphb, req, opt); > + if (ret >= 0) { > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + } else { > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > + } > + > + return ret; I think you're better off failing the eeh_handler() call in here when the function is not implemented. That way all callers don't have to explicitly check for it. I also find it a lot less confusing if the return value doesn't get set inside this helper. > +} > + > +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t addr, option; > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 4) || (nret != 1)) { > + goto param_error_exit; > + } > + > + addr = rtas_ld(args, 0); > + option = rtas_ld(args, 3); > + switch (option) { > + case RTAS_EEH_ENABLE: > + if (!find_dev(spapr, buid, addr)) { > + goto param_error_exit; > + } > + break; > + case RTAS_EEH_DISABLE: > + case RTAS_EEH_THAW_IO: > + case RTAS_EEH_THAW_DMA: > + break; > + default: > + goto param_error_exit; > + } > + > + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_SET_OPTION, option, rets); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t addr, option; > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + PCIDevice *pdev; > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 4) || (nret != 2)) { > + goto param_error_exit; > + } > + > + addr = rtas_ld(args, 0); > + option = rtas_ld(args, 3); > + if (option != RTAS_GET_PE_ADDR && option != RTAS_GET_PE_MODE) { > + goto param_error_exit; > + } > + > + pdev = find_dev(spapr, buid, addr); > + if (!pdev) { > + goto param_error_exit; > + } > + > + /* > + * For now, we always have bus level PE whose address > + * has format "00BBSS00". The guest OS might regard > + * PE address 0 as invalid. We avoid that simply by > + * extending it with one. > + */ > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + if (option == RTAS_GET_PE_ADDR) { > + rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1); > + } else { > + rtas_st(rets, 1, RTAS_PE_MODE_SHARED); > + } > + > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + int32_t ret; > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 3) || (nret != 4 && nret != 5)) { > + goto param_error_exit; > + } > + > + ret = rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_GET_STATE, 0, rets); > + if (ret >= 0) { > + rtas_st(rets, 1, ret); > + rtas_st(rets, 2, RTAS_EEH_SUPPORT); > + rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO); > + if (nret >= 5) { > + rtas_st(rets, 4, RTAS_EEH_PE_RECOVER_INFO); > + } > + } It's really awkward to not see an rtas_st(0) here. > + > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t option; > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 4) || (nret != 1)) { > + goto param_error_exit; > + } > + > + option = rtas_ld(args, 3); > + if (option != RTAS_SLOT_RESET_DEACTIVATE && > + option != RTAS_SLOT_RESET_HOT && > + option != RTAS_SLOT_RESET_FUNDAMENTAL) { > + goto param_error_exit; > + } Please make this a switch(). > + > + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_RESET, option, rets); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 3) || (nret != 1)) { > + goto param_error_exit; > + } > + > + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_CONFIGURE, 0, rets); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +/* To support it later */ > +static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + int option; > + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); > + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + if (!sphb || !info->eeh_handler) { > + goto param_error_exit; > + } > + > + if ((nargs != 8) || (nret != 1)) { > + goto param_error_exit; > + } > + > + option = rtas_ld(args, 7); > + if (option != RTAS_SLOT_TEMP_ERR_LOG && > + option != RTAS_SLOT_PERM_ERR_LOG) { > + goto param_error_exit; > + } switch() again. Alex > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > static int pci_spapr_swizzle(int slot, int pin) > { > return (slot + pin) % PCI_NUM_PINS; > @@ -939,6 +1174,19 @@ void spapr_pci_rtas_init(void) > rtas_ibm_query_interrupt_source_number); > spapr_rtas_register("ibm,change-msi", rtas_ibm_change_msi); > } > + > + spapr_rtas_register("ibm,set-eeh-option", > + rtas_ibm_set_eeh_option); > + spapr_rtas_register("ibm,get-config-addr-info2", > + rtas_ibm_get_config_addr_info2); > + spapr_rtas_register("ibm,read-slot-reset-state2", > + rtas_ibm_read_slot_reset_state2); > + spapr_rtas_register("ibm,set-slot-reset", > + rtas_ibm_set_slot_reset); > + spapr_rtas_register("ibm,configure-pe", > + rtas_ibm_configure_pe); > + spapr_rtas_register("ibm,slot-error-detail", > + rtas_ibm_slot_error_detail); > } > > static void spapr_pci_register_types(void) > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index caeecfa..f487f23 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -51,6 +51,7 @@ struct sPAPRPHBClass { > PCIHostBridgeClass parent_class; > > void (*finish_realize)(sPAPRPHBState *sphb, Error **errp); > + int (*eeh_handler)(sPAPRPHBState *sphb, int req, int opt); > }; > > struct sPAPRPHBState { > @@ -104,6 +105,12 @@ struct sPAPRPHBVFIOState { > > #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL > > +/* EEH related requests */ > +#define RTAS_EEH_REQ_SET_OPTION 0 > +#define RTAS_EEH_REQ_GET_STATE 1 > +#define RTAS_EEH_REQ_RESET 2 > +#define RTAS_EEH_REQ_CONFIGURE 3 > + > static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin) > { > return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 6faae5d..ea84de2 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -354,6 +354,39 @@ static inline int spapr_allocate_lsi(int hint) > return spapr_allocate_irq(hint, true); > } > > +/* ibm,set-eeh-option */ > +#define RTAS_EEH_DISABLE 0 > +#define RTAS_EEH_ENABLE 1 > +#define RTAS_EEH_THAW_IO 2 > +#define RTAS_EEH_THAW_DMA 3 > + > +/* ibm,get-config-addr-info2 */ > +#define RTAS_GET_PE_ADDR 0 > +#define RTAS_GET_PE_MODE 1 > +#define RTAS_PE_MODE_NONE 0 > +#define RTAS_PE_MODE_NOT_SHARED 1 > +#define RTAS_PE_MODE_SHARED 2 > + > +/* ibm,read-slot-reset-state2 */ > +#define RTAS_EEH_PE_STATE_NORMAL 0 > +#define RTAS_EEH_PE_STATE_RESET 1 > +#define RTAS_EEH_PE_STATE_STOPPED_IO_DMA 2 > +#define RTAS_EEH_PE_STATE_STOPPED_DMA 4 > +#define RTAS_EEH_PE_STATE_UNAVAIL 5 > +#define RTAS_EEH_NOT_SUPPORT 0 > +#define RTAS_EEH_SUPPORT 1 > +#define RTAS_EEH_PE_UNAVAIL_INFO 1000 > +#define RTAS_EEH_PE_RECOVER_INFO 0 > + > +/* ibm,set-slot-reset */ > +#define RTAS_SLOT_RESET_DEACTIVATE 0 > +#define RTAS_SLOT_RESET_HOT 1 > +#define RTAS_SLOT_RESET_FUNDAMENTAL 3 > + > +/* ibm,slot-error-detail */ > +#define RTAS_SLOT_TEMP_ERR_LOG 1 > +#define RTAS_SLOT_PERM_ERR_LOG 2 > + > /* RTAS return codes */ > #define RTAS_OUT_SUCCESS 0 > #define RTAS_OUT_NO_ERRORS_FOUND 1