lkml.org 
[lkml]   [2022]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
    Date
    > From: Lu Baolu <baolu.lu@linux.intel.com>
    > Sent: Thursday, May 19, 2022 3:21 PM
    >
    > The iommu_sva_domain represents a hardware pagetable that the IOMMU
    > hardware could use for SVA translation. This adds some infrastructure
    > to support SVA domain in the iommu common layer. It includes:
    >
    > - Add a new struct iommu_sva_domain and new IOMMU_DOMAIN_SVA
    > domain
    > type.
    > - Add a new domain ops pointer in iommu_ops. The IOMMU drivers that
    > support SVA should provide the callbacks.
    > - Add helpers to allocate and free an SVA domain.
    > - Add helpers to set an SVA domain to a device and the reverse
    > operation.
    >
    > Some buses, like PCI, route packets without considering the PASID value.
    > Thus a DMA target address with PASID might be treated as P2P if the
    > address falls into the MMIO BAR of other devices in the group. To make
    > things simple, the attach/detach interfaces only apply to devices
    > belonging to the singleton groups, and the singleton is immutable in
    > fabric i.e. not affected by hotplug.
    >
    > The iommu_set/block_device_pasid() can be used for other purposes,
    > such as kernel DMA with pasid, mediation device, etc. Hence, it is put
    > in the iommu.c.

    usually we have 'set/clear' pair or 'allow/block'. Having 'set' paired
    with 'block' doesn't read very clearly.

    > +static bool device_group_immutable_singleton(struct device *dev)
    > +{
    > + struct iommu_group *group = iommu_group_get(dev);

    what about passing group as the parameter since the caller will
    get the group again right after calling this function? In that case
    the function could be renamed as:

    iommu_group_immutable_singleton()

    or be shorter:

    iommu_group_fixed_singleton()

    > + int count;
    > +
    > + if (!group)
    > + return false;
    > +
    > + mutex_lock(&group->mutex);
    > + count = iommu_group_device_count(group);
    > + mutex_unlock(&group->mutex);
    > + iommu_group_put(group);
    > +
    > + if (count != 1)
    > + return false;

    For non-pci devices above doesn't check anything against immutable.
    Please add some comment to explain why doing so is correct.

    > +
    > + /*
    > + * The PCI device could be considered to be fully isolated if all
    > + * devices on the path from the device to the host-PCI bridge are
    > + * protected from peer-to-peer DMA by ACS.
    > + */
    > + if (dev_is_pci(dev))
    > + return pci_acs_path_enabled(to_pci_dev(dev), NULL,
    > + REQ_ACS_FLAGS);
    > +
    > + return true;
    > +}
    > +

    \
     
     \ /
      Last update: 2022-05-24 11:41    [W:4.203 / U:0.744 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site