From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45304) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wt32r-0004M6-JP for qemu-devel@nongnu.org; Fri, 06 Jun 2014 18:55:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wt32m-0006ed-7A for qemu-devel@nongnu.org; Fri, 06 Jun 2014 18:55:33 -0400 Message-ID: <1402095312.14174.32.camel@ul30vt.home> From: Alex Williamson Date: Fri, 06 Jun 2014 16:55:12 -0600 In-Reply-To: <539245CC.9020205@ozlabs.ru> 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> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 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: Alexey Kardashevskiy Cc: Alexander Graf , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Gavin Shan 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. > > >>>> + > >>>> + 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 > >>> > >>> > >>> > >> > >> > > > > > > > >