From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0Sqa-0007la-LV for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:53:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0SqR-0002zu-JL for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:53:32 -0400 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:44707) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0SqQ-0002y2-Ed for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:53:23 -0400 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Jun 2014 15:23:18 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 37B601258059 for ; Fri, 27 Jun 2014 15:22:46 +0530 (IST) Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s5R9rRwj59768954 for ; Fri, 27 Jun 2014 15:23:27 +0530 Received: from d28av04.in.ibm.com (localhost [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s5R9rAC2019872 for ; Fri, 27 Jun 2014 15:23:11 +0530 Date: Fri, 27 Jun 2014 19:53:02 +1000 From: Gavin Shan Message-ID: <20140627095301.GA6819@shangw> References: <1403746538-12487-1-git-send-email-gwshan@linux.vnet.ibm.com> <1403746538-12487-2-git-send-email-gwshan@linux.vnet.ibm.com> <53ABF638.7090208@suse.de> <20140626104338.GA4806@shangw> <53ABFA1A.10105@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53ABFA1A.10105@suse.de> Subject: Re: [Qemu-devel] [PATCH v11 1/3] sPAPR: Implement EEH RTAS calls Reply-To: Gavin Shan List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: aik@ozlabs.ru, alex.williamson@redhat.com, Gavin Shan , qemu-devel@nongnu.org On Thu, Jun 26, 2014 at 12:46:50PM +0200, Alexander Graf wrote: > >On 26.06.14 12:43, Gavin Shan wrote: >>On Thu, Jun 26, 2014 at 12:30:16PM +0200, Alexander Graf wrote: >>>On 26.06.14 03:35, 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 | 240 ++++++++++++++++++++++++++++++++++++++++++++ >>>> include/hw/pci-host/spapr.h | 7 ++ >>>> include/hw/ppc/spapr.h | 33 ++++++ >>>> 3 files changed, 280 insertions(+) >>>> >>>>diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>>>index 131434b..8712051 100644 >>>>--- a/hw/ppc/spapr_pci.c >>>>+++ b/hw/ppc/spapr_pci.c >>>>@@ -422,6 +422,233 @@ 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(sPAPREnvironment *spapr, >>>>+ uint64_t buid, uint32_t req, uint32_t opt) >>>>+{ >>>>+ sPAPRPHBState *sphb = spapr_find_phb(spapr, buid); >>>>+ sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >>>>+ >>>>+ if (!sphb || !info->eeh_handler) { >>>>+ return -ENOENT; >>>>+ } >>>>+ >>>>+ return info->eeh_handler(sphb, req, opt); >>>>+} >>>>+ >>>>+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); >>>>+ int ret; >>>>+ >>>>+ 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; >>>>+ } >>>>+ >>>>+ ret = rtas_handle_eeh_request(spapr, buid, >>>>+ RTAS_EEH_REQ_SET_OPTION, option); >>>>+ if (ret >= 0) { >>>>+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>>+ 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); >>>>+ int ret; >>>>+ >>>>+ if ((nargs != 3) || (nret != 4 && nret != 5)) { >>>>+ goto param_error_exit; >>>>+ } >>>>+ >>>>+ ret = rtas_handle_eeh_request(spapr, buid, RTAS_EEH_REQ_GET_STATE, 0); >>>>+ if (ret >= 0) { >>>>+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>>+ 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); >>>>+ } >>>>+ >>>>+ 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); >>>>+ int ret; >>>>+ >>>>+ if ((nargs != 4) || (nret != 1)) { >>>>+ goto param_error_exit; >>>>+ } >>>>+ >>>>+ option = rtas_ld(args, 3); >>>>+ switch (option) { >>>>+ case RTAS_SLOT_RESET_DEACTIVATE: >>>>+ case RTAS_SLOT_RESET_HOT: >>>>+ case RTAS_SLOT_RESET_FUNDAMENTAL: >>>>+ break; >>>>+ default: >>>>+ goto param_error_exit; >>>>+ } >>>>+ >>>>+ ret = rtas_handle_eeh_request(spapr, buid, RTAS_EEH_REQ_RESET, option); >>>>+ if (ret >= 0) { >>>>+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>>+ 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); >>>>+ int ret; >>>>+ >>>>+ if ((nargs != 3) || (nret != 1)) { >>>>+ goto param_error_exit; >>>>+ } >>>>+ >>>>+ ret = rtas_handle_eeh_request(spapr, buid, RTAS_EEH_REQ_CONFIGURE, 0); >>>>+ if (ret >= 0) { >>>>+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>>+ 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); >>>>+ switch (option) { >>>>+ case RTAS_SLOT_TEMP_ERR_LOG: >>>>+ case RTAS_SLOT_PERM_ERR_LOG: >>>>+ break; >>>>+ default: >>>>+ goto param_error_exit; >>>>+ } >>>>+ >>>>+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>We don't return an error log, so shouldn't this be >>>RTAS_OUT_NO_ERRORS_FOUND (or a new name you come up with that fits >>>better for 1)? >>> >>Well, RTAS_OUT_NO_ERRORS_FOUND is correct constant for the return value >>according to PAPR spec: >> >>1: No Error Log Returned >> >>Apart from this, anything else I need change? :-) > >I didn't spot anything :) > Ok. Alex, should I change the code to return RTAS_OUT_NO_ERRORS_FOUND and then send v12 (next revision) for your further review, or you have more comments on v11 (current revision)? :-) rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); Thanks, Gavin