Messages in this thread | | | Date | Wed, 1 Apr 2020 16:03:01 +0200 | From | Jean-Philippe Brucker <> | Subject | Re: [PATCH 00/10] IOASID extensions for guest SVA |
| |
Hi Jacob,
On Wed, Mar 25, 2020 at 10:55:21AM -0700, Jacob Pan wrote: > IOASID was introduced in v5.5 as a generic kernel allocator service for > both PCIe Process Address Space ID (PASID) and ARM SMMU's Sub Stream > ID. In addition to basic ID allocation, ioasid_set was introduced as a > token that is shared by a group of IOASIDs. This set token can be used > for permission checking but lack of some features needed by guest Shared > Virtual Address (SVA). In addition, IOASID support for life cycle > management is needed among multiple users. > > This patchset introduces two extensions to the IOASID code, > 1. IOASID set operations > 2. Notifications for IOASID state synchronization
My main concern with this series is patch 7 changing the spinlock to a mutex, which prevents SVA from calling ioasid_free() from the RCU callback of MMU notifiers. Could we use atomic notifiers, or do the FREE notification another way?
Most of my other comments are just confusion on my part, I think, as I haven't yet properly looked through the VFIO and VT-d changes. I'd rather avoid the change to ioasid_find() if possible, because it adds a seemingly unnecessary indirection to the fast path, but it's probably insignificant.
Thanks, Jean
> > Part #1: > IOASIDs used by each VM fits naturally into an ioasid_set. The usage > for per set management requires the following features: > > - Quota enforcement - This is to prevent one VM from abusing the > allocator to take all the system IOASIDs. Though VFIO layer can also > enforce the quota, but it cannot cover the usage with both guest and > host SVA on the same system. > > - Stores guest IOASID-Host IOASID mapping within the set. To > support live migration, IOASID namespace should be owned by the > guest. This requires per IOASID set look up between guest and host > IOASIDs. This patchset does not introduce non-identity guest-host > IOASID lookup, we merely introduce the infrastructure in per set data. > > - Set level operations, e.g. when a guest terminates, it is likely to > free the entire set. Having a single place to manage the set where the > IOASIDs are stored makes iteration much easier. > > > New APIs are: > - void ioasid_install_capacity(ioasid_t total); > Set the system capacity prior to any allocations. On x86, VT-d driver > calls this function to set max number of PASIDs, typically 1 million > (20 bits). > > - int ioasid_alloc_system_set(int quota); > Host system has a default ioasid_set, during boot it is expected that > this default set is allocated with a reasonable quota, e.g. PID_MAX. > This default/system set is used for baremetal SVA. > > - int ioasid_alloc_set(struct ioasid_set *token, ioasid_t quota, int > *sid); > Allocate a new set with a token, returned sid (set ID) will be used > to allocate IOASIDs within the set. Allocation of IOASIDs cannot > exceed the quota. > > - void ioasid_free_set(int sid, bool destroy_set); > Free the entire set and notify all users with an option to destroy > the set. Set ID can be used for allocation again if not destroyed. > > - int ioasid_find_sid(ioasid_t ioasid); > Look up the set ID from an ioasid. There is no reference held, > assuming set has a single owner. > > - int ioasid_adjust_set(int sid, int quota); > Change the quota of the set, new quota cannot be less than the number > of IOASIDs already allocated within the set. This is useful when > IOASID resource needs to be balanced among VMs. > > Part #2 > Notification service. Since IOASIDs are used by many consumers that > follow publisher-subscriber pattern, notification is a natural choice > to keep states synchronized. For example, on x86 system, guest PASID > allocation and bind call results in VFIO IOCTL that can add and change > guest-host PASID states. At the same time, IOMMU driver and KVM need to > maintain its own PASID contexts. In this case, VFIO is the publisher > within the kernel, IOMMU driver and KVM are the subscribers. > > This patchset introduces a global blocking notifier chain and APIs to > operate on. Not all events nor all IOASIDs are of interests to all > subscribers. e.g. KVM is only interested in the IOASIDs within its set. > IOMMU driver is not ioasid_set aware. A further optimization could be > having both global and per set notifier. But consider the infrequent > nature of bind/unbind and relatively long process life cycle, this > optimization may not be needed at this time. > > To register/unregister notification blocks, use these two APIs: > - int ioasid_add_notifier(struct notifier_block *nb); > - void ioasid_remove_notifier(struct notifier_block *nb) > > To send notification on an IOASID with one of the commands (FREE, > BIND/UNBIND, etc.), use: > - int ioasid_notify(ioasid_t id, enum ioasid_notify_val cmd); > > This work is a result of collaboration with many people: > Liu, Yi L <yi.l.liu@intel.com> > Wu Hao <hao.wu@intel.com> > Ashok Raj <ashok.raj@intel.com> > Kevin Tian <kevin.tian@intel.com> > > Thanks, > > Jacob > > Jacob Pan (10): > iommu/ioasid: Introduce system-wide capacity > iommu/vt-d: Set IOASID capacity when SVM is enabled > iommu/ioasid: Introduce per set allocation APIs > iommu/ioasid: Rename ioasid_set_data to avoid confusion with > ioasid_set > iommu/ioasid: Create an IOASID set for host SVA use > iommu/ioasid: Convert to set aware allocations > iommu/ioasid: Use mutex instead of spinlock > iommu/ioasid: Introduce notifier APIs > iommu/ioasid: Support ioasid_set quota adjustment > iommu/vt-d: Register PASID notifier for status change > > drivers/iommu/intel-iommu.c | 20 ++- > drivers/iommu/intel-svm.c | 89 ++++++++-- > drivers/iommu/ioasid.c | 387 +++++++++++++++++++++++++++++++++++++++----- > include/linux/intel-iommu.h | 1 + > include/linux/ioasid.h | 86 +++++++++- > 5 files changed, 522 insertions(+), 61 deletions(-) > > -- > 2.7.4 >
| |