From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YivUz-0006CA-5u for qemu-devel@nongnu.org; Thu, 16 Apr 2015 21:55:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YivUt-0004le-RV for qemu-devel@nongnu.org; Thu, 16 Apr 2015 21:55:17 -0400 Received: from mail-pd0-f177.google.com ([209.85.192.177]:34750) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YivUt-0004lG-Jd for qemu-devel@nongnu.org; Thu, 16 Apr 2015 21:55:11 -0400 Received: by pdbqa5 with SMTP id qa5so111520728pdb.1 for ; Thu, 16 Apr 2015 18:55:10 -0700 (PDT) Message-ID: <553067F6.2020200@ozlabs.ru> Date: Fri, 17 Apr 2015 11:55:02 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1428679484-15451-1-git-send-email-aik@ozlabs.ru> <1428679484-15451-11-git-send-email-aik@ozlabs.ru> <20150416123139.21d5e778@thh440s> In-Reply-To: <20150416123139.21d5e778@thh440s> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu v6 10/15] spapr_pci_vfio: Remove unnecessary cast to sPAPRPHBVFIOState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Michael Roth , Alexander Graf , qemu-devel@nongnu.org, Alex Williamson , qemu-ppc@nongnu.org, Gavin Shan , David Gibson On 04/16/2015 08:31 PM, Thomas Huth wrote: > Am Sat, 11 Apr 2015 01:24:39 +1000 > schrieb Alexey Kardashevskiy : > >> EEH handlers only need AddressSpace to get to the right VFIO container >> to call ioctl() so remove unnecessary cast of sPAPRPHBState to >> sPAPRPHBVFIOState. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/ppc/spapr_pci_vfio.c | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c >> index a428166..a5b97d0 100644 >> --- a/hw/ppc/spapr_pci_vfio.c >> +++ b/hw/ppc/spapr_pci_vfio.c >> @@ -66,7 +66,6 @@ static void spapr_phb_vfio_reset(DeviceState *qdev) >> static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, >> unsigned int addr, int option) >> { >> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); >> struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; >> int ret; >> >> @@ -103,7 +102,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, >> return RTAS_OUT_PARAM_ERROR; >> } >> >> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, >> + ret = vfio_container_ioctl(&sphb->iommu_as, >> VFIO_EEH_PE_OP, &op); >> if (ret < 0) { >> return RTAS_OUT_HW_ERROR; >> @@ -114,12 +113,11 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, >> >> static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state) >> { >> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); >> struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; >> int ret; >> >> op.op = VFIO_EEH_PE_GET_STATE; >> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, >> + ret = vfio_container_ioctl(&sphb->iommu_as, >> VFIO_EEH_PE_OP, &op); >> if (ret < 0) { >> return RTAS_OUT_PARAM_ERROR; >> @@ -131,7 +129,6 @@ static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state) >> >> static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) >> { >> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); >> struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; >> int ret; >> >> @@ -149,7 +146,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) >> return RTAS_OUT_PARAM_ERROR; >> } >> >> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, >> + ret = vfio_container_ioctl(&sphb->iommu_as, >> VFIO_EEH_PE_OP, &op); >> if (ret < 0) { >> return RTAS_OUT_HW_ERROR; >> @@ -160,12 +157,11 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) >> >> static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) >> { >> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); >> struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; >> int ret; >> >> op.op = VFIO_EEH_PE_CONFIGURE; >> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, >> + ret = vfio_container_ioctl(&sphb->iommu_as, >> VFIO_EEH_PE_OP, &op); >> if (ret < 0) { >> return RTAS_OUT_PARAM_ERROR; > > Patch looks fine to me, so: > > Reviewed-by: Thomas Huth > > But maybe you could even squash this into the patch where you removed the > iommugroupid parameters (patch #5) ... so that you don't have to touch all > these lines twice? May be... Patch #5 enables multiple groups per container, and this one prepares for spapr-vfio-phb merge so they solve different issues in general and I was told multiple times the keeping patches simpler is a good thing (to reasonable extent) :) Anyway in the next respin I'll remove more of EEH stuff, coming soon! -- Alexey