[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: |
Liu, Yi L |
Subject: |
RE: [PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks |
Date: |
Tue, 31 Mar 2020 12:54:10 +0000 |
Hi Eric,
> From: Auger Eric <address@hidden>
> Sent: Tuesday, March 31, 2020 7:16 PM
> To: Liu, Yi L <address@hidden>; address@hidden;
> Subject: Re: [PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks
>
> 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.
The three above definition are just as an example. Just want to explain
what model I'm referencing. :-)
> You don't have such kind of inheritance at the moment in your series.
yes, only vfio inherits HostIOMMUContext, no other module inherits.
But I want to show a case in which there are multiple module inherits
one single parent. Just lack a vfio equivalent module to show it. So
I used the iommu_memory_region example. sorry to confuse you.
>
> 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.
I don't know either. But I think it's possible. They can do it per their
need in future.
> On the other hand you have VTDHostIOMMUContext which is not a QOM
> derived object.
Ok, I guess I made you believe both vfio and vIOMMU will inherit the
HostIOMMUContext now. is it?
Actually, it's not. Only vfio inherits HostIOMMUContext in QOM manner.
For the VTDHostIOMMUContext, it's just referencing the HostIOMMUContext
which is initialized by vfio.
Regards,
Yi Liu
- Re: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context callback, (continued)
[PATCH v2 22/22] intel_iommu: modify x-scalable-mode to be string option, Liu Yi L, 2020/03/30
[PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container, Liu Yi L, 2020/03/30
[PATCH v2 02/22] header file update VFIO/IOMMU vSVA APIs, Liu Yi L, 2020/03/30
[PATCH v2 08/22] vfio/common: provide PASID alloc/free hooks, Liu Yi L, 2020/03/30
[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
[PATCH v2 17/22] intel_iommu: do not pass down pasid bind for PASID #0, Liu Yi L, 2020/03/30