From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35536) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0ekp-0002Of-Rl for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:08:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0ekg-0008Gr-NE for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:08:39 -0500 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:57202) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0ekg-0008GY-2z for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:08:30 -0500 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 16 Dec 2014 04:38:26 +0530 Date: Tue, 16 Dec 2014 10:08:20 +1100 From: Gavin Shan Message-ID: <20141215230820.GA1113@shangw> References: <1418602508-30845-1-git-send-email-gwshan@linux.vnet.ibm.com> <1418602508-30845-3-git-send-email-gwshan@linux.vnet.ibm.com> <548EF5A1.2040105@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <548EF5A1.2040105@suse.de> Subject: Re: [Qemu-devel] [PATCH v13 2/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, Gavin Shan , qemu-devel@nongnu.org, alex.williamson@redhat.com, qemu-ppc@nongnu.org On Mon, Dec 15, 2014 at 03:52:17PM +0100, Alexander Graf wrote: >On 15.12.14 01:15, 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: >> >> * RTAS calls are received in spapr_pci.c, sanity check is done >> there. >> * RTAS handlers handle what they can. If there is something it >> cannot handle and sPAPRPHBClass::eeh_handler callback is defined, >> it is called. >> * 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. >> >> [aik: defined RTAS tokens for EEH RTAS calls] >> Signed-off-by: Gavin Shan >> --- >> hw/ppc/spapr_pci.c | 246 ++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/pci-host/spapr.h | 7 ++ >> include/hw/ppc/spapr.h | 43 +++++++- >> 3 files changed, 294 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index 3d70efe..3bb1971 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -406,6 +406,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_pci_find_phb(spapr, buid); >> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > >What happens when you try to cast NULL? Could a guest process invoke a >host assert() through this and abort the whole VM? > Yes, it would cause core dump. I had one experiment to force assigning NULL to "sphb" before doing the cast, the whole VM is aborted. So I guess you're happy to have something as follows. If you're not suggesting something else, I'll update the code as follows in next version: sPAPRPHBState *sphb = spapr_pci_find_phb(spapr, buid); sPAPRPHBClass *info; if (!sphb) { return -ENODEV; } info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); if (!info->eeh_handler) { return -ENOENT; } return info->eeh_handler(sphb, req, opt); >> + >> + 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 (!spapr_pci_find_dev(spapr, buid, addr)) { >> + goto param_error_exit; >> + } >> + break; >> + case RTAS_EEH_DISABLE: >> + case RTAS_EEH_THAW_IO: >> + case RTAS_EEH_THAW_DMA: > >So these don't use the addr hint? > No, there're no address as argument of this RTAS call "ibm,set-eeh-option". The RTAS call has 4 arguments, all of them are 32-bits: BUID high part, BUID low part, PE address, option. The option could be one of: enable/disable EEH functionality, enable IO path, enable DMA path. Thanks, Gavin > >Alex >