[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks
From: |
Auger Eric |
Subject: |
Re: [PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks |
Date: |
Tue, 31 Mar 2020 13:15:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Yi,
On 3/31/20 12:59 PM, Liu, Yi L wrote:
> Hi Eric,
>
>> From: Auger Eric
>> Sent: Tuesday, March 31, 2020 6:48 PM
>> To: Liu, Yi L <address@hidden>; address@hidden;
>> address@hidden; address@hidden
>> Cc: address@hidden; address@hidden; address@hidden; Tian,
>> Kevin <address@hidden>; Tian, Jun J <address@hidden>; Sun, Yi Y
>> <address@hidden>; address@hidden; Wu, Hao <address@hidden>; jean-
>> address@hidden; Jacob Pan <address@hidden>; Yi Sun
>> <address@hidden>
>> Subject: Re: [PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks
>>
>> Yi,
>>
>> On 3/30/20 6:24 AM, Liu Yi L wrote:
>>> This patch defines vfio_host_iommu_context_info, implements the PASID
>>> alloc/free hooks defined in HostIOMMUContextClass.
>>>
>>> Cc: Kevin Tian <address@hidden>
>>> Cc: Jacob Pan <address@hidden>
>>> Cc: Peter Xu <address@hidden>
>>> Cc: Eric Auger <address@hidden>
>>> Cc: Yi Sun <address@hidden>
>>> Cc: David Gibson <address@hidden>
>>> Cc: Alex Williamson <address@hidden>
>>> Signed-off-by: Liu Yi L <address@hidden>
>>> ---
>>> hw/vfio/common.c | 69
>>> +++++++++++++++++++++++++++++++++++
>>> include/hw/iommu/host_iommu_context.h | 3 ++
>>> include/hw/vfio/vfio-common.h | 4 ++
>>> 3 files changed, 76 insertions(+)
>>>
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
>>> c276732..5f3534d 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -1179,6 +1179,53 @@ static int vfio_get_iommu_type(VFIOContainer
>> *container,
>>> return -EINVAL;
>>> }
>>>
>>> +static int vfio_host_iommu_ctx_pasid_alloc(HostIOMMUContext *iommu_ctx,
>>> + uint32_t min, uint32_t max,
>>> + uint32_t *pasid) {
>>> + VFIOContainer *container = container_of(iommu_ctx,
>>> + VFIOContainer, iommu_ctx);
>>> + struct vfio_iommu_type1_pasid_request req;
>>> + unsigned long argsz;
>> you can easily avoid using argsz variable
>
> oh, right. :-)
>
>>> + int ret;
>>> +
>>> + argsz = sizeof(req);
>>> + req.argsz = argsz;
>>> + req.flags = VFIO_IOMMU_PASID_ALLOC;
>>> + req.alloc_pasid.min = min;
>>> + req.alloc_pasid.max = max;
>>> +
>>> + if (ioctl(container->fd, VFIO_IOMMU_PASID_REQUEST, &req)) {
>>> + ret = -errno;
>>> + error_report("%s: %d, alloc failed", __func__, ret);
>> better use %m directly or strerror(errno) also include vbasedev->name?
>
> or yes, vbasedev->name is also nice to have.
>
>>> + return ret;
>>> + }
>>> + *pasid = req.alloc_pasid.result;
>>> + return 0;
>>> +}
>>> +
>>> +static int vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
>>> + uint32_t pasid) {
>>> + VFIOContainer *container = container_of(iommu_ctx,
>>> + VFIOContainer, iommu_ctx);
>>> + struct vfio_iommu_type1_pasid_request req;
>>> + unsigned long argsz;
>> same
>
> got it.
>
>>> + int ret;
>>> +
>>> + argsz = sizeof(req);
>>> + req.argsz = argsz;
>>> + req.flags = VFIO_IOMMU_PASID_FREE;
>>> + req.free_pasid = pasid;
>>> +
>>> + if (ioctl(container->fd, VFIO_IOMMU_PASID_REQUEST, &req)) {
>>> + ret = -errno;
>>> + error_report("%s: %d, free failed", __func__, ret);
>> same
>
> yep.
>>> + return ret;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>>> Error **errp) { @@ -1791,3 +1838,25
>>> @@ int vfio_eeh_as_op(AddressSpace *as, uint32_t op)
>>> }
>>> return vfio_eeh_container_op(container, op); }
>>> +
>>> +static void vfio_host_iommu_context_class_init(ObjectClass *klass,
>>> + void *data) {
>>> + HostIOMMUContextClass *hicxc = HOST_IOMMU_CONTEXT_CLASS(klass);
>>> +
>>> + hicxc->pasid_alloc = vfio_host_iommu_ctx_pasid_alloc;
>>> + hicxc->pasid_free = vfio_host_iommu_ctx_pasid_free; }
>>> +
>>> +static const TypeInfo vfio_host_iommu_context_info = {
>>> + .parent = TYPE_HOST_IOMMU_CONTEXT,
>>> + .name = TYPE_VFIO_HOST_IOMMU_CONTEXT,
>>> + .class_init = vfio_host_iommu_context_class_init,
>> Ah OK
>>
>> This is the object inheriting from the abstract TYPE_HOST_IOMMU_CONTEXT.
>
> yes. it is. :-)
>
>> I initially thought VTDHostIOMMUContext was, sorry for the misunderstanding.
>
> Ah, my fault, should have got it earlier. so we may have just aligned
> in last Oct.
>
>> Do you expect other HostIOMMUContext backends? Given the name and ops, it
>> looks really related to VFIO?
>
> For other backends, I guess you mean other passthru modules? If yes, I
> think they should have their own type name. Just like vIOMMUs, the below
> vIOMMUs defines their own type name and inherits the same parent.
>
> static const TypeInfo vtd_iommu_memory_region_info = {
> .parent = TYPE_IOMMU_MEMORY_REGION,
> .name = TYPE_INTEL_IOMMU_MEMORY_REGION,
> .class_init = vtd_iommu_memory_region_class_init,
> };
>
> static const TypeInfo smmuv3_iommu_memory_region_info = {
> .parent = TYPE_IOMMU_MEMORY_REGION,
> .name = TYPE_SMMUV3_IOMMU_MEMORY_REGION,
> .class_init = smmuv3_iommu_memory_region_class_init,
> };
>
> static const TypeInfo amdvi_iommu_memory_region_info = {
> .parent = TYPE_IOMMU_MEMORY_REGION,
> .name = TYPE_AMD_IOMMU_MEMORY_REGION,
> .class_init = amdvi_iommu_memory_region_class_init,
> };
Sorry I am confused now.
You don't have such kind of inheritance at the moment in your series.
You have an abstract object (TYPE_HOST_IOMMU_CONTEXT, HostIOMMUContext)
which is derived into TYPE_VFIO_HOST_IOMMU_CONTEXT. Only the class ops
are specialized for VFIO. But I do not foresee any other user than VFIO
(ie. other implementers of the class ops), hence my question. For
instance would virtio/vhost ever implement its TYPE_HOST_IOMMU_CONTEXT.
On the other hand you have VTDHostIOMMUContext which is not a QOM
derived object.
Thanks
Eric
>
> Regards,
> Yi Liu
>
[PATCH v2 05/22] hw/pci: modify pci_setup_iommu() to set PCIIOMMUOps, Liu Yi L, 2020/03/30
[PATCH v2 12/22] intel_iommu: process PASID cache invalidation, Liu Yi L, 2020/03/30
[PATCH v2 11/22] intel_iommu: add virtual command capability support, Liu Yi L, 2020/03/30
[PATCH v2 13/22] intel_iommu: add PASID cache management infrastructure, Liu Yi L, 2020/03/30
[PATCH v2 16/22] intel_iommu: replay pasid binds after context cache invalidation, Liu Yi L, 2020/03/30
[PATCH v2 15/22] intel_iommu: bind/unbind guest page table to host, Liu Yi L, 2020/03/30
[PATCH v2 14/22] vfio: add bind stage-1 page table support, Liu Yi L, 2020/03/30
[PATCH v2 19/22] intel_iommu: process PASID-based iotlb invalidation, Liu Yi L, 2020/03/30