From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50771) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtQLo-0008P8-GQ for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:48:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WtQLe-0001BA-0H for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:48:40 -0400 Received: from mail-pb0-f42.google.com ([209.85.160.42]:34267) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtQLd-0001Ao-Od for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:48:29 -0400 Received: by mail-pb0-f42.google.com with SMTP id md12so3945252pbc.15 for ; Sat, 07 Jun 2014 16:48:27 -0700 (PDT) Message-ID: <5393A4C5.70304@ozlabs.ru> Date: Sun, 08 Jun 2014 09:48:21 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1402025693-25096-1-git-send-email-aik@ozlabs.ru> <1402025693-25096-3-git-send-email-aik@ozlabs.ru> <1402073323.14174.5.camel@ul30vt.home> <5391F250.9080802@ozlabs.ru> <1402076106.14174.19.camel@ul30vt.home> <539245CC.9020205@ozlabs.ru> <1402095312.14174.32.camel@ul30vt.home> In-Reply-To: <1402095312.14174.32.camel@ul30vt.home> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl() 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, Gavin Shan On 06/07/2014 08:55 AM, Alex Williamson wrote: > On Sat, 2014-06-07 at 08:50 +1000, Alexey Kardashevskiy wrote: >> On 06/07/2014 03:35 AM, Alex Williamson wrote: >>> On Sat, 2014-06-07 at 02:54 +1000, Alexey Kardashevskiy wrote: >>>> On 06/07/2014 02:48 AM, Alex Williamson wrote: >>>>> On Fri, 2014-06-06 at 13:34 +1000, Alexey Kardashevskiy wrote: >>>>>> While most operations with VFIO IOMMU driver are generic and used inside >>>>>> vfio.c, there are still some operations which only specific VFIO IOMMU >>>>>> drivers implement. The first example of it will be reading a DMA window >>>>>> start from the host. >>>>>> >>>>>> This adds a helper which passes an ioctl request to the container's fd. >>>>>> >>>>>> Signed-off-by: Alexey Kardashevskiy >>>>>> --- >>>>>> Changes: >>>>>> v8: >>>>>> * s/vfio_container_spapr_get_info/vfio_container_ioctl/ - now it is >>>>>> generalized >>>>>> >>>>>> v7: >>>>>> * do not return a group fd from the helper >>>>>> >>>>>> v6: >>>>>> * added dup() to protect group_fd from accidental disposal >>>>>> >>>>>> v5: >>>>>> * reworked to reflect change in vfio_get_group() from one >>>>>> of previous patches change >>>>>> >>>>>> v4: >>>>>> * fixed possible leaks on error paths >>>>>> --- >>>>>> hw/misc/vfio.c | 28 ++++++++++++++++++++++++++++ >>>>>> include/hw/misc/vfio.h | 9 +++++++++ >>>>>> 2 files changed, 37 insertions(+) >>>>>> create mode 100644 include/hw/misc/vfio.h >>>>>> >>>>>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >>>>>> index 7437c2e..bb77934 100644 >>>>>> --- a/hw/misc/vfio.c >>>>>> +++ b/hw/misc/vfio.c >>>>>> @@ -39,6 +39,7 @@ >>>>>> #include "qemu/range.h" >>>>>> #include "sysemu/kvm.h" >>>>>> #include "sysemu/sysemu.h" >>>>>> +#include "hw/misc/vfio.h" >>>>>> >>>>>> /* #define DEBUG_VFIO */ >>>>>> #ifdef DEBUG_VFIO >>>>>> @@ -4318,3 +4319,30 @@ static void register_vfio_pci_dev_type(void) >>>>>> } >>>>>> >>>>>> type_init(register_vfio_pci_dev_type) >>>>>> + >>>>>> +int vfio_container_ioctl(AddressSpace *as, int32_t groupid, >>>>>> + int req, void *param) >>>>>> +{ >>>>>> + VFIOGroup *group; >>>>>> + VFIOContainer *container; >>>>>> + int ret = -1; >>>>>> + >>>>>> + group = vfio_get_group(groupid, as); >>>>>> + if (!group) { >>>>>> + error_report("vfio: group %d not registered", groupid); >>>>>> + return ret; >>>>>> + } >> >> >>>>> >>>>> Gavin's version of this walked existing groups and returned an error if >>>>> not found. vfio_get_group() will go out and create a group and connect >>>>> it to a container. I'm not sure we really want to expose that sort of >>>>> capability out to the reset of QEMU. Thanks, >> >>>> >>>> >>>> If it is not created here, then it can only be created from vfio_initfn() >>>> which is too late as I need group to get DMA window properties for PHB when >>>> PHB is being realized. >>>> >>>> Add another helper? >>> >>> Urgh. No, let's not make another helper. Can we at least keep the >>> filtering Gavin had so we don't just allow any ioctl through here >>> though? At least we can make vfio.c involved if someone what's to do >>> something crazy here. Thanks, >> >> >> if (req != CHECK_EXTENSION) && >> (req != ENABLE) && >> (reg != EEH_OP) >> return -1; >> >> Like that (roughly)? > > A switch statement seems a lot cleaner. Like that (#2 adds empty switch{}, #4 enables two ioctls, below is #4)? int vfio_container_ioctl(AddressSpace *as, int32_t groupid, int req, void *param) { VFIOGroup *group; VFIOContainer *container; int ret = -1; switch (req) { + case VFIO_CHECK_EXTENSION: + case VFIO_IOMMU_SPAPR_TCE_GET_INFO: + break; default: /* Return an error on unknown requests */ return ret; } group = vfio_get_group(groupid, as); if (!group) { >> >>>>>> + >>>>>> + container = group->container; >>>>>> + if (group->container) { >>>>>> + ret = ioctl(container->fd, req, param); >>>>>> + if (ret < 0) { >>>>>> + error_report("vfio: failed to ioctl container: ret=%d, %s", >>>>>> + ret, strerror(errno)); >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + vfio_put_group(group); >>>>>> + >>>>>> + return ret; >>>>>> +} >>>>>> diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h >>>>>> new file mode 100644 >>>>>> index 0000000..0b26cd8 >>>>>> --- /dev/null >>>>>> +++ b/include/hw/misc/vfio.h >>>>>> @@ -0,0 +1,9 @@ >>>>>> +#ifndef VFIO_API_H >>>>>> +#define VFIO_API_H >>>>>> + >>>>>> +#include "qemu/typedefs.h" >>>>>> + >>>>>> +extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid, >>>>>> + int req, void *param); >>>>>> + >>>>>> +#endif >>>>> >>>>> >>>>> >>>> >>>> >>> >>> >>> >> >> > > > -- Alexey