[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl()
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl() |
Date: |
Sun, 08 Jun 2014 09:48:21 +1000 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
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 <address@hidden>
>>>>>> ---
>>>>>> 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
- [Qemu-devel] [PATCH v8 0/4] vfio on spapr-ppc64, Alexey Kardashevskiy, 2014/06/05
- [Qemu-devel] [PATCH v8 1/4] spapr_iommu: Make in-kernel TCE table optional, Alexey Kardashevskiy, 2014/06/05
- [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alexey Kardashevskiy, 2014/06/05
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alex Williamson, 2014/06/06
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alexey Kardashevskiy, 2014/06/06
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alex Williamson, 2014/06/06
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alexey Kardashevskiy, 2014/06/06
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alex Williamson, 2014/06/06
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(),
Alexey Kardashevskiy <=
- Re: [Qemu-devel] [PATCH v8 2/4] vfio: Add vfio_container_ioctl(), Alex Williamson, 2014/06/09
[Qemu-devel] [PATCH v8 4/4] vfio: Enable for SPAPR, Alexey Kardashevskiy, 2014/06/05
[Qemu-devel] [PATCH v8 3/4] spapr_pci_vfio: Add spapr-pci-vfio-host-bridge to support vfio, Alexey Kardashevskiy, 2014/06/05