From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52677) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaCrz-00045M-Ke for qemu-devel@nongnu.org; Sun, 28 Feb 2016 20:43:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaCrv-0003Fb-Bn for qemu-devel@nongnu.org; Sun, 28 Feb 2016 20:43:31 -0500 Received: from mail-pf0-x242.google.com ([2607:f8b0:400e:c00::242]:33938) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaCrv-0003F8-0V for qemu-devel@nongnu.org; Sun, 28 Feb 2016 20:43:27 -0500 Received: by mail-pf0-x242.google.com with SMTP id 184so2354754pff.1 for ; Sun, 28 Feb 2016 17:43:26 -0800 (PST) References: <1456486323-8047-1-git-send-email-david@gibson.dropbear.id.au> <1456486323-8047-4-git-send-email-david@gibson.dropbear.id.au> From: Alexey Kardashevskiy Message-ID: <56D3A238.2000806@ozlabs.ru> Date: Mon, 29 Feb 2016 12:43:20 +1100 MIME-Version: 1.0 In-Reply-To: <1456486323-8047-4-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , benh@kernel.crashing.org Cc: agraf@suse.de, qemu-devel@nongnu.org, gwshan@au1.ibm.com, mdroth@linux.vnet.ibm.com, alex.williamson@redhat.com, qemu-ppc@nongnu.org On 02/26/2016 10:31 PM, David Gibson wrote: > The EEH operations in the spapr-vfio-pci-host-bridge no longer rely on the > special groupid field in sPAPRPHBVFIOState. So we can simplify, removing > the class specific callbacks with direct calls based on a simple > spapr_phb_eeh_enabled() helper. For now we implement that in terms of > a boolean in the class, but we'll continue to clean that up later. > > On its own this is a rather strange way of doing things, but it's a useful > intermediate step to further cleanups. Reviewed-by: Alexey Kardashevskiy > > Signed-off-by: David Gibson > --- > hw/ppc/spapr_pci.c | 44 ++++++++++++++++++++++---------------------- > hw/ppc/spapr_pci_vfio.c | 18 +++++++----------- > include/hw/pci-host/spapr.h | 13 +++++++++---- > 3 files changed, 38 insertions(+), 37 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 9b2b546..d1e5222 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -92,6 +92,13 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid, > return pci_find_device(phb->bus, bus_num, devfn); > } > > +static bool spapr_phb_eeh_available(sPAPRPHBState *sphb) > +{ > + sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + return spc->eeh_available; > +} > + > static uint32_t rtas_pci_cfgaddr(uint32_t arg) > { > /* This handles the encoding of extended config space addresses */ > @@ -438,7 +445,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > uint32_t addr, option; > uint64_t buid; > int ret; > @@ -456,12 +462,11 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_set_option) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > - ret = spc->eeh_set_option(sphb, addr, option); > + ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option); > rtas_st(rets, 0, ret); > return; > > @@ -476,7 +481,6 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > PCIDevice *pdev; > uint32_t addr, option; > uint64_t buid; > @@ -491,8 +495,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_set_option) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > @@ -532,7 +535,6 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > uint64_t buid; > int state, ret; > > @@ -546,12 +548,11 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_get_state) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > - ret = spc->eeh_get_state(sphb, &state); > + ret = spapr_phb_vfio_eeh_get_state(sphb, &state); > rtas_st(rets, 0, ret); > if (ret != RTAS_OUT_SUCCESS) { > return; > @@ -576,7 +577,6 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > uint32_t option; > uint64_t buid; > int ret; > @@ -592,12 +592,11 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_reset) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > - ret = spc->eeh_reset(sphb, option); > + ret = spapr_phb_vfio_eeh_reset(sphb, option); > rtas_st(rets, 0, ret); > return; > > @@ -612,7 +611,6 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > uint64_t buid; > int ret; > > @@ -626,12 +624,11 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_configure) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > - ret = spc->eeh_configure(sphb); > + ret = spapr_phb_vfio_eeh_configure(sphb); > rtas_st(rets, 0, ret); > return; > > @@ -647,7 +644,6 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu, > target_ulong rets) > { > sPAPRPHBState *sphb; > - sPAPRPHBClass *spc; > int option; > uint64_t buid; > > @@ -661,8 +657,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu, > goto param_error_exit; > } > > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > - if (!spc->eeh_set_option) { > + if (!spapr_phb_eeh_available(sphb)) { > goto param_error_exit; > } > > @@ -1430,6 +1425,10 @@ static void spapr_phb_reset(DeviceState *qdev) > { > /* Reset the IOMMU state */ > object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL); > + > + if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) { > + spapr_phb_vfio_reset(qdev); > + } > } > > static Property spapr_phb_properties[] = { > @@ -1560,6 +1559,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data) > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > dc->cannot_instantiate_with_device_add_yet = false; > spc->finish_realize = spapr_phb_finish_realize; > + spc->eeh_available = false; > hp->plug = spapr_phb_hot_plug_child; > hp->unplug = spapr_phb_hot_unplug_child; > } > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index b1e8e8e..10fa88a 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -78,7 +78,7 @@ static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb) > vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE); > } > > -static void spapr_phb_vfio_reset(DeviceState *qdev) > +void spapr_phb_vfio_reset(DeviceState *qdev) > { > /* > * The PE might be in frozen state. To reenable the EEH > @@ -89,8 +89,8 @@ static void spapr_phb_vfio_reset(DeviceState *qdev) > spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev)); > } > > -static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, > - unsigned int addr, int option) > +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, > + unsigned int addr, int option) > { > uint32_t op; > int ret; > @@ -136,7 +136,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, > return RTAS_OUT_SUCCESS; > } > > -static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state) > +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state) > { > int ret; > > @@ -192,7 +192,7 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb) > pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL); > } > > -static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) > +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) > { > uint32_t op; > int ret; > @@ -221,7 +221,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) > return RTAS_OUT_SUCCESS; > } > > -static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) > +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) > { > int ret; > > @@ -239,12 +239,8 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) > sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass); > > dc->props = spapr_phb_vfio_properties; > - dc->reset = spapr_phb_vfio_reset; > spc->finish_realize = spapr_phb_vfio_finish_realize; > - spc->eeh_set_option = spapr_phb_vfio_eeh_set_option; > - spc->eeh_get_state = spapr_phb_vfio_eeh_get_state; > - spc->eeh_reset = spapr_phb_vfio_eeh_reset; > - spc->eeh_configure = spapr_phb_vfio_eeh_configure; > + spc->eeh_available = true; > } > > static const TypeInfo spapr_phb_vfio_info = { > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index 7de5e02..cc1b82c 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -49,10 +49,7 @@ struct sPAPRPHBClass { > PCIHostBridgeClass parent_class; > > void (*finish_realize)(sPAPRPHBState *sphb, Error **errp); > - int (*eeh_set_option)(sPAPRPHBState *sphb, unsigned int addr, int option); > - int (*eeh_get_state)(sPAPRPHBState *sphb, int *state); > - int (*eeh_reset)(sPAPRPHBState *sphb, int option); > - int (*eeh_configure)(sPAPRPHBState *sphb); > + bool eeh_available; > }; > > typedef struct spapr_pci_msi { > @@ -136,5 +133,13 @@ void spapr_pci_rtas_init(void); > sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t buid); > PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid, > uint32_t config_addr); > +void spapr_phb_vfio_reset(DeviceState *qdev); > + > +/* VFIO EEH hooks */ > +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, > + unsigned int addr, int option); > +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state); > +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option); > +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb); > > #endif /* __HW_SPAPR_PCI_H__ */ > -- Alexey