Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
> On 2020-09-04 16:55, Bjorn Andersson wrote:
> > Add a new operation to allow platform implementations to inherit any
> > stream mappings from the boot loader.
>
> Is there a reason we need an explicit step for this? The aim of the
> cfg_probe hook is that the SMMU software state should all be set up by then,
> and you can mess about with it however you like before arm_smmu_reset()
> actually commits anything to hardware. I would have thought you could
> permanently steal a context bank, configure it as your bypass hole, read out
> the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> appropriately all together "invisibly" at that point.
I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
configuration impl hook before consuming features") we no longer have
setup pgsize_bitmap as we hit cfg_probe, which means that I need to
replicate this logic to set up the iommu_domain.
If I avoid setting up an iommu_domain for the identity context, as you
request in patch 8, this shouldn't be needed anymore.
> If that can't work, I'm very curious as to what I've overlooked.
>
I believe that will work, I will rework the patches and try it out.
Thanks,
Bjorn
> Robin.
>
> > Signed-off-by: Bjorn Andersson
> > ---
> >
> > Changes since v2:
> > - New patch/interface
> >
> > drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 ++-
> > drivers/iommu/arm/arm-smmu/arm-smmu.h | 6 ++
> > 2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index eb5c6ca5c138..4c4d302cd747 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -85,11 +85,6 @@ static inline void arm_smmu_rpm_put(struct
> > arm_smmu_device *smmu)
> > pm_runtime_put_autosuspend(smmu->dev);
> > }
> > -static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> > -{
> > - return container_of(dom, struct arm_smmu_domain, domain);
> > -}
> > -
> > static struct platform_driver arm_smmu_driver;
> > static struct iommu_ops arm_smmu_ops;
> > @@ -2188,6 +2183,12 @@ static int arm_smmu_device_probe(struct
> > platform_device *pdev)
> > if (err)
> > return err;
> > + if (smmu->impl->inherit_mappings) {
> > + err = smmu->impl->inherit_mappings(smmu);
> > + if (err)
> > + return err;
> > + }
> > +
> > if (smmu->version == ARM_SMMU_V2) {
> > if (smmu->num_context_banks > smmu->num_context_irqs) {
> > dev_err(dev,
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > index 235d9a3a6ab6..f58164976e74 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -378,6 +378,11 @@ struct arm_smmu_domain {
> > struct iommu_domain domain;
> > };
> > +static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain
> > *dom)
> > +{
> > + return container_of(dom, struct arm_smmu_domain, domain);
> > +}
> > +
> > struct arm_smmu_master_cfg {
> > struct arm_smmu_device *smmu;
> > s16 smendx[];
> > @@ -442,6 +447,7 @@ struct arm_smmu_impl {
> > int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
> > struct arm_smmu_device *smmu,
> > struct device *dev, int start);
> > + int (*inherit_mappings)(struct arm_smmu_device *smmu);
> > };
> > #define INVALID_SMENDX-1
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v7 03/16] vfio/type1: Report iommu nesting info to userspace
Hi Alex,
> From: Alex Williamson
> Sent: Saturday, September 12, 2020 4:17 AM
>
> On Thu, 10 Sep 2020 03:45:20 -0700
> Liu Yi L wrote:
>
> > This patch exports iommu nesting capability info to user space through
> > VFIO. Userspace is expected to check this info for supported uAPIs (e.g.
> > PASID alloc/free, bind page table, and cache invalidation) and the
> > vendor specific format information for first level/stage page table
> > that will be bound to.
> >
> > The nesting info is available only after container set to be NESTED type.
> > Current implementation imposes one limitation - one nesting container
> > should include at most one iommu group. The philosophy of vfio
> > container is having all groups/devices within the container share the
> > same IOMMU context. When vSVA is enabled, one IOMMU context could
> > include one 2nd- level address space and multiple 1st-level address
> > spaces. While the 2nd-level address space is reasonably sharable by
> > multiple groups, blindly sharing 1st-level address spaces across all
> > groups within the container might instead break the guest expectation.
> > In the future sub/super container concept might be introduced to allow
> > partial address space sharing within an IOMMU context. But for now
> > let's go with this restriction by requiring singleton container for
> > using nesting iommu features. Below link has the related discussion about
> > this
> decision.
> >
> > https://lore.kernel.org/kvm/20200515115924.37e69...@w520.home/
> >
> > This patch also changes the NESTING type container behaviour.
> > Something that would have succeeded before will now fail: Before this
> > series, if user asked for a VFIO_IOMMU_TYPE1_NESTING, it would have
> > succeeded even if the SMMU didn't support stage-2, as the driver would
> > have silently fallen back on stage-1 mappings (which work exactly the
> > same as stage-2 only since there was no nesting supported). After the
> > series, we do check for DOMAIN_ATTR_NESTING so if user asks for
> > VFIO_IOMMU_TYPE1_NESTING and the SMMU doesn't support stage-2, the
> > ioctl fails. But it should be a good fix and completely harmless. Detail
> > can be found
> in below link as well.
> >
> > https://lore.kernel.org/kvm/20200717090900.GC4850@myrica/
> >
> > Cc: Kevin Tian
> > CC: Jacob Pan
> > Cc: Alex Williamson
> > Cc: Eric Auger
> > Cc: Jean-Philippe Brucker
> > Cc: Joerg Roedel
> > Cc: Lu Baolu
> > Signed-off-by: Liu Yi L
> > ---
> > v6 -> v7:
> > *) using vfio_info_add_capability() for adding nesting cap per suggestion
> >from Eric.
> >
> > v5 -> v6:
> > *) address comments against v5 from Eric Auger.
> > *) don't report nesting cap to userspace if the nesting_info->format is
> >invalid.
> >
> > v4 -> v5:
> > *) address comments from Eric Auger.
> > *) return struct iommu_nesting_info for
> VFIO_IOMMU_TYPE1_INFO_CAP_NESTING as
> >cap is much "cheap", if needs extension in future, just define another
> > cap.
> >https://lore.kernel.org/kvm/20200708132947.5b7ee...@x1.home/
> >
> > v3 -> v4:
> > *) address comments against v3.
> >
> > v1 -> v2:
> > *) added in v2
> > ---
> > drivers/vfio/vfio_iommu_type1.c | 92 +++-
> -
> > include/uapi/linux/vfio.h | 19 +
> > 2 files changed, 99 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > b/drivers/vfio/vfio_iommu_type1.c index c992973..3c0048b 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -62,18 +62,20 @@ MODULE_PARM_DESC(dma_entry_limit,
> > "Maximum number of user DMA mappings per container (65535).");
> >
> > struct vfio_iommu {
> > - struct list_headdomain_list;
> > - struct list_headiova_list;
> > - struct vfio_domain *external_domain; /* domain for external user */
> > - struct mutexlock;
> > - struct rb_root dma_list;
> > - struct blocking_notifier_head notifier;
> > - unsigned intdma_avail;
> > - uint64_tpgsize_bitmap;
> > - boolv2;
> > - boolnesting;
> > - booldirty_page_tracking;
> > - boolpinned_page_dirty_scope;
> > + struct list_headdomain_list;
> > + struct list_headiova_list;
> > + /* domain for external user */
> > + struct vfio_domain *external_domain;
> > + struct mutexlock;
> > + struct rb_root dma_list;
> > + struct blocking_notifier_head notifier;
> > + unsigned intdma_avail;
> > + uint64_tpgsize_bitmap;
> > + boolv2;
> > + boolnesting;
> > + booldirty_page_tracking;
> > + boolpinned_page_dirty_scope;
> > + struct iommu_nesting_info *nest
RE: [PATCH v7 13/16] vfio/pci: Expose PCIe PASID capability to guest
Hi Alex,
> From: Alex Williamson
> Sent: Saturday, September 12, 2020 6:13 AM
>
> On Thu, 10 Sep 2020 03:45:30 -0700
> Liu Yi L wrote:
>
> > This patch exposes PCIe PASID capability to guest for assigned devices.
> > Existing vfio_pci driver hides it from guest by setting the capability
> > length as 0 in pci_ext_cap_length[].
>
> This exposes the PASID capability, but it's still read-only, so this largely
> just helps
> userspace know where to emulate the capability, right? Thanks,
oh, yes. This path only makes it visible to userspace. perhaps, I should
refine the commit message and the patch name. right?
Regards,
Yi Liu
> Alex
>
> > And this patch only exposes PASID capability for devices which has
> > PCIe PASID extended struture in its configuration space. VFs will not
> > expose the PASID capability as they do not implement the PASID
> > extended structure in their config space. It is a TODO in future.
> > Related discussion can be found in below link:
> >
> > https://lore.kernel.org/kvm/20200407095801.648b1...@w520.home/
> >
> > Cc: Kevin Tian
> > CC: Jacob Pan
> > Cc: Alex Williamson
> > Cc: Eric Auger
> > Cc: Jean-Philippe Brucker
> > Cc: Joerg Roedel
> > Cc: Lu Baolu
> > Signed-off-by: Liu Yi L
> > Reviewed-by: Eric Auger
> > ---
> > v5 -> v6:
> > *) add review-by from Eric Auger.
> >
> > v1 -> v2:
> > *) added in v2, but it was sent in a separate patchseries before
> > ---
> > drivers/vfio/pci/vfio_pci_config.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_config.c
> > b/drivers/vfio/pci/vfio_pci_config.c
> > index d98843f..07ff2e6 100644
> > --- a/drivers/vfio/pci/vfio_pci_config.c
> > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > @@ -95,7 +95,7 @@ static const u16 pci_ext_cap_length[PCI_EXT_CAP_ID_MAX +
> 1] = {
> > [PCI_EXT_CAP_ID_LTR]= PCI_EXT_CAP_LTR_SIZEOF,
> > [PCI_EXT_CAP_ID_SECPCI] = 0, /* not yet */
> > [PCI_EXT_CAP_ID_PMUX] = 0, /* not yet */
> > - [PCI_EXT_CAP_ID_PASID] = 0, /* not yet */
> > + [PCI_EXT_CAP_ID_PASID] = PCI_EXT_CAP_PASID_SIZEOF,
> > };
> >
> > /*
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
