From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60728) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJJoK-0006zp-A5 for qemu-devel@nongnu.org; Tue, 10 Sep 2013 05:00:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJJo8-0001uH-Sn for qemu-devel@nongnu.org; Tue, 10 Sep 2013 05:00:36 -0400 Received: from mail-pb0-f50.google.com ([209.85.160.50]:34365) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJJo8-0001tz-Mr for qemu-devel@nongnu.org; Tue, 10 Sep 2013 05:00:24 -0400 Received: by mail-pb0-f50.google.com with SMTP id uo5so7230472pbc.23 for ; Tue, 10 Sep 2013 02:00:23 -0700 (PDT) Message-ID: <522EDF9F.6040902@ozlabs.ru> Date: Tue, 10 Sep 2013 19:00:15 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1377857738-14789-1-git-send-email-aik@ozlabs.ru> <1377857738-14789-12-git-send-email-aik@ozlabs.ru> <1378407927.3246.263.camel@ul30vt.home> In-Reply-To: <1378407927.3246.263.camel@ul30vt.home> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 11/12] spapr vfio: enable for spapr List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: Alexander Graf , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson On 09/06/2013 05:05 AM, Alex Williamson wrote: > On Fri, 2013-08-30 at 20:15 +1000, Alexey Kardashevskiy wrote: >> This turns the sPAPR support on and enables VFIO container use >> in the kernel. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> Changes: >> v4: >> * fixed format string to use %m which is a glibc extension: >> "Print output of strerror(errno). No argument is required." >> --- >> hw/misc/vfio.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >> index 4210471..882da70 100644 >> --- a/hw/misc/vfio.c >> +++ b/hw/misc/vfio.c >> @@ -2815,6 +2815,36 @@ static int vfio_connect_container(VFIOGroup *group, VFIOAddressSpace *space) >> >> memory_listener_register(&container->iommu_data.listener, >> container->space->as); >> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) { >> + ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd); >> + if (ret) { >> + error_report("vfio: failed to set group container: %m"); >> + g_free(container); >> + close(fd); >> + return -errno; >> + } >> + >> + ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU); >> + if (ret) { >> + error_report("vfio: failed to set iommu for container: %m"); >> + g_free(container); >> + close(fd); >> + return -errno; >> + } >> + >> + ret = ioctl(fd, VFIO_IOMMU_ENABLE); >> + if (ret) { >> + error_report("vfio: failed to enable container: %m"); >> + g_free(container); >> + close(fd); >> + return -errno; > > These (and the copies that already exist in this function) are screaming > for a goto. Heh. So. There should be 2 patches then - one adding gotos to the existing code and another one adding new functionality-with-gotos-already. I can do that, is it what you suggest? What about the rest of the series? Next time I will split "[Qemu-devel] [PATCH v4 05/12] spapr_pci: convert init to realize" but the rest will be still the same. I have understanding that Alex Graf is expecting you to review the whole thing (ack/sob? not sure how this all works) before he pulls it into his tree. And thanks for comments. -- Alexey