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 <gws...@linux.vnet.ibm.com> >> --- >> 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 >