[Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Michael Shavit]

Hi Jean-Philippe, all

We’d like to use the new iommu_attach_device_pasid API to control
mappings on a per SSID basis. To that end, we’ve started investigating
how to implement the set_dev_pasid callback in the arm-smmu-v3.c
driver for domain types other than SVA.

As noted in your smmuv3 aux-domain patch, the arm_smmu_domain
represents not only “the non-PASID address space but also (at least
for SMMUv3) the whole device context, PASID tables etc.” Multiple SVA
iommu_domains reach into a shared arm_smmu_domain to write their CDs
into a shared CD table.
Is moving to a model where each arm_smmu_master holds this previously
shared device context instead of the arm_smmu_domain a bad idea?

This would allow a pretty straightforward implementation of
set_dev_pasid for other domain types:
    * Attaching a domain with PASID to a device doesn’t have to
piggyback on any other domain’s CD table. The domain’s CD can directly
be written into the master’s CD table.
    * A domain attached with PASID is no different than one attached
without PASID, attach_dev becomes attach_dev_with_pasid(0).

At a high-level:
   1. The arm_smmu_domain would still own the arm_smmu_ctx_desc struct
that describes that domain (translation table, etc)
   2. Each arm_smmu_master would now own its own s1_cfg struct (and CD
table). Attaching a domain to a device would write the CD into that
device’s CD table instead of the previously shared CD table.

From an implementation POV there’s a few other changes and
book-keeping that become required to handle edge-cases (e.g. We need
to find all the CD entries that a domain is written to when the SVA
driver tries to update a domain’s ASID value). Right now I’m more
concerned about whether this idea is sane at a design level.

As a first order consequence, when multiple devices share a domain:
    1. We’ll use more memory than we would have before.
    2. The SMMU will perform dma to distinct CD tables to find the
page table; however the page table itself would still be shared.

Is that acceptable considering some users might not need the
set_dev_pasid support this is geared towards?
Are there other consequences we've overlooked?

Thanks,
Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Jean-Philippe Brucker]

Hi Michael,

On Fri, Mar 24, 2023 at 05:18:36PM +0800, Michael Shavit wrote:
> Hi Jean-Philippe, all
> 
> We’d like to use the new iommu_attach_device_pasid API to control
> mappings on a per SSID basis. To that end, we’ve started investigating
> how to implement the set_dev_pasid callback in the arm-smmu-v3.c
> driver for domain types other than SVA.
> 
> As noted in your smmuv3 aux-domain patch, the arm_smmu_domain
> represents not only “the non-PASID address space but also (at least
> for SMMUv3) the whole device context, PASID tables etc.” Multiple SVA
> iommu_domains reach into a shared arm_smmu_domain to write their CDs
> into a shared CD table.
> Is moving to a model where each arm_smmu_master holds this previously
> shared device context instead of the arm_smmu_domain a bad idea?
> 
> This would allow a pretty straightforward implementation of
> set_dev_pasid for other domain types:
>     * Attaching a domain with PASID to a device doesn’t have to
> piggyback on any other domain’s CD table. The domain’s CD can directly
> be written into the master’s CD table.
>     * A domain attached with PASID is no different than one attached
> without PASID, attach_dev becomes attach_dev_with_pasid(0).
> 
> At a high-level:
>    1. The arm_smmu_domain would still own the arm_smmu_ctx_desc struct
> that describes that domain (translation table, etc)
>    2. Each arm_smmu_master would now own its own s1_cfg struct (and CD
> table). Attaching a domain to a device would write the CD into that
> device’s CD table instead of the previously shared CD table.
> 
> From an implementation POV there’s a few other changes and
> book-keeping that become required to handle edge-cases (e.g. We need
> to find all the CD entries that a domain is written to when the SVA
> driver tries to update a domain’s ASID value). Right now I’m more
> concerned about whether this idea is sane at a design level.
> 
> As a first order consequence, when multiple devices share a domain:
>     1. We’ll use more memory than we would have before.

I wonder if we could save some memory by reducing the allocation size for
single-entry context tables: it looks like dma_direct_alloc() gives us a
full page at the moment even though we only need 64 bytes per CD
(and S1ContextPtr alignment is 64B).

>     2. The SMMU will perform dma to distinct CD tables to find the
> page table; however the page table itself would still be shared.

Yes, I'd expect this to be negligible since an SMMU would cache the CD

> 
> Is that acceptable considering some users might not need the
> set_dev_pasid support this is geared towards?

That does sound like a nice cleanup. When I last looked at this years ago,
there were two cases that shared a CD table between devices:

(a) IOMMU groups, where devices cannot be isolated from each others by the
    IOMMU. Those are probably not going away anytime soon, but they aren't
    desirable. The normal case is one device per group.

(b) Devices assigned to the same VM without a viommu, using the same VFIO
    container. QEMU used stage-1 even though stage-2, which doesn't
    require a CD table, was available with VFIO_TYPE1_NESTING_IOMMU.

    I don't know what the plan is for IOMMUFD, maybe using stage-2 would
    be the right direction for new users.

For anything else I think we're already allocating separate smmu_domains
for each endpoint, each with their own CD table.

Thanks,
Jean

> Are there other consequences we've overlooked?
> 
> Thanks,
> Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Michael Shavit]

On Fri, Mar 24, 2023 at 8:31 PM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:

> > As a first order consequence, when multiple devices share a domain:
> >     1. We’ll use more memory than we would have before.
>
> I wonder if we could save some memory by reducing the allocation size for
> single-entry context tables: it looks like dma_direct_alloc() gives us a
> full page at the moment even though we only need 64 bytes per CD
> (and S1ContextPtr alignment is 64B).

Oh interesting, I can give that a try. I'm guessing you mean by
allocating from a pool when a master with ssid=1 needs to allocate its
cd table?

> >     2. The SMMU will perform dma to distinct CD tables to find the
> > page table; however the page table itself would still be shared.
>
> Yes, I'd expect this to be negligible since an SMMU would cache the CD
>
> >
> > Is that acceptable considering some users might not need the
> > set_dev_pasid support this is geared towards?
>
> That does sound like a nice cleanup. When I last looked at this years ago,
> there were two cases that shared a CD table between devices:

Awesome, thanks for the feedback Jean. I'll start drafting-up a patch
series over the next few weeks then.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Jean-Philippe Brucker]

On Sat, Mar 25, 2023 at 01:42:22PM +0800, Michael Shavit wrote:
> On Fri, Mar 24, 2023 at 8:31 PM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> 
> > > As a first order consequence, when multiple devices share a domain:
> > >     1. We’ll use more memory than we would have before.
> >
> > I wonder if we could save some memory by reducing the allocation size for
> > single-entry context tables: it looks like dma_direct_alloc() gives us a
> > full page at the moment even though we only need 64 bytes per CD
> > (and S1ContextPtr alignment is 64B).
> 
> Oh interesting, I can give that a try. I'm guessing you mean by
> allocating from a pool when a master with ssid=1 needs to allocate its
> cd table?

Yes, adding a dma_pool for this particular case could work

Thanks,
Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Design] iommu/arm-smmu-v3: Implementing set_dev_pasid for other domain types

[Robin Murphy]

On 2023-03-24 09:18, Michael Shavit wrote:
> Hi Jean-Philippe, all
> 
> We’d like to use the new iommu_attach_device_pasid API to control
> mappings on a per SSID basis. To that end, we’ve started investigating
> how to implement the set_dev_pasid callback in the arm-smmu-v3.c
> driver for domain types other than SVA.
> 
> As noted in your smmuv3 aux-domain patch, the arm_smmu_domain
> represents not only “the non-PASID address space but also (at least
> for SMMUv3) the whole device context, PASID tables etc.” Multiple SVA
> iommu_domains reach into a shared arm_smmu_domain to write their CDs
> into a shared CD table.
> Is moving to a model where each arm_smmu_master holds this previously
> shared device context instead of the arm_smmu_domain a bad idea?

Since the aux_domain concept got knocked on the head, the model should 
really be that a (stage 1) iommu_domain represents the CD + pagetable. 
Having the domain own its containing CD *table* (or not, depending on 
how it's attached) is conceptually a bit backwards, and indeed I think 
it precludes using per-device SSIDs, which might conceivably be a thing 
to consider for non-SVA use-cases.

> This would allow a pretty straightforward implementation of
> set_dev_pasid for other domain types:
>      * Attaching a domain with PASID to a device doesn’t have to
> piggyback on any other domain’s CD table. The domain’s CD can directly
> be written into the master’s CD table.
>      * A domain attached with PASID is no different than one attached
> without PASID, attach_dev becomes attach_dev_with_pasid(0).
> 
> At a high-level:
>     1. The arm_smmu_domain would still own the arm_smmu_ctx_desc struct
> that describes that domain (translation table, etc)
>     2. Each arm_smmu_master would now own its own s1_cfg struct (and CD
> table). Attaching a domain to a device would write the CD into that
> device’s CD table instead of the previously shared CD table.

Short of doing something wacky like trying to make the CD table belong 
to the PASID allocator (not necessarily as mad as it initially sounds, 
though), I'd say conceptually it's probably closest to belonging to the 
STE, as represented by the arm_smmu_master, so this sounds about the 
right shape to me.

Thanks,
Robin.

>  From an implementation POV there’s a few other changes and
> book-keeping that become required to handle edge-cases (e.g. We need
> to find all the CD entries that a domain is written to when the SVA
> driver tries to update a domain’s ASID value). Right now I’m more
> concerned about whether this idea is sane at a design level.
> 
> As a first order consequence, when multiple devices share a domain:
>      1. We’ll use more memory than we would have before.
>      2. The SMMU will perform dma to distinct CD tables to find the
> page table; however the page table itself would still be shared.
> 
> Is that acceptable considering some users might not need the
> set_dev_pasid support this is geared towards?
> Are there other consequences we've overlooked?
> 
> Thanks,
> Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel