Re: [Freedreno] [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG
On Thu, Mar 4, 2021 at 7:48 AM Robin Murphy wrote:
>
> On 2021-03-01 08:42, Christoph Hellwig wrote:
> > Signed-off-by: Christoph Hellwig
>
> Moreso than the previous patch, where the feature is at least relatively
> generic (note that there's a bunch of in-flight development around
> DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to
> bloat the generic iommu_ops structure with private driver-specific
> interfaces. The attribute interface is a great compromise for these
> kinds of things, and you can easily add type-checked wrappers around it
> for external callers (maybe even make the actual attributes internal
> between the IOMMU core and drivers) if that's your concern.
I suppose if this is *just* for the GPU we could move it into adreno_smmu_priv..
But one thing I'm not sure about is whether
IO_PGTABLE_QUIRK_ARM_OUTER_WBWA is something that other devices
*should* be using as well, but just haven't gotten around to yet.
BR,
-R
> Robin.
>
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
> > drivers/iommu/arm/arm-smmu/arm-smmu.c | 40 +++--
> > drivers/iommu/iommu.c | 9 ++
> > include/linux/iommu.h | 9 +-
> > 4 files changed, 29 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain
> > *iommu)
> > struct io_pgtable_domain_attr pgtbl_cfg;
> >
> > pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
> > - iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, _cfg);
> > + iommu_domain_set_pgtable_attr(iommu, _cfg);
> > }
> >
> > struct msm_gem_address_space *
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index 2e17d990d04481..2858999c86dfd1 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct
> > iommu_domain *domain)
> > return ret;
> > }
> >
> > -static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
> > - enum iommu_attr attr, void *data)
> > +static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_cfg)
> > {
> > - int ret = 0;
> > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > + int ret = -EPERM;
> >
> > - mutex_lock(_domain->init_mutex);
> > -
> > - switch(domain->type) {
> > - case IOMMU_DOMAIN_UNMANAGED:
> > - switch (attr) {
> > - case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> > - struct io_pgtable_domain_attr *pgtbl_cfg = data;
> > -
> > - if (smmu_domain->smmu) {
> > - ret = -EPERM;
> > - goto out_unlock;
> > - }
> > + if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> > + return -EINVAL;
> >
> > - smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > - break;
> > - }
> > - default:
> > - ret = -ENODEV;
> > - }
> > - break;
> > - case IOMMU_DOMAIN_DMA:
> > - ret = -ENODEV;
> > - break;
> > - default:
> > - ret = -EINVAL;
> > + mutex_lock(_domain->init_mutex);
> > + if (!smmu_domain->smmu) {
> > + smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > + ret = 0;
> > }
> > -out_unlock:
> > mutex_unlock(_domain->init_mutex);
> > +
> > return ret;
> > }
> >
> > @@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {
> > .device_group = arm_smmu_device_group,
> > .dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
> > .dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
> > - .domain_set_attr= arm_smmu_domain_set_attr,
> > + .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
> > .domain_enable_nesting = arm_smmu_domain_enable_nesting,
> > .of_xlate = arm_smmu_of_xlate,
> > .get_resv_regions = arm_smmu_get_resv_regions,
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2e9e058501a953..8490aefd4b41f8 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain
> > *domain)
> > }
> > EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
> >
> > +int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_cfg)
> > +{
> > + if
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
Hi Jason, On Thu, 4 Mar 2021 15:02:53 -0400, Jason Gunthorpe wrote: > On Thu, Mar 04, 2021 at 11:01:44AM -0800, Jacob Pan wrote: > > > > For something like qemu I'd expect to put the qemu process in a cgroup > > > with 1 PASID. Who cares what qemu uses the PASID for, or how it was > > > allocated? > > > > For vSVA, we will need one PASID per guest process. But that is up to > > the admin based on whether or how many SVA capable devices are directly > > assigned. > > I hope the virtual IOMMU driver can communicate the PASID limit and > the cgroup machinery in the guest can know what the actual limit is. > For VT-d, emulated vIOMMU can communicate with the guest IOMMU driver on how many PASID bits are supported (extended cap reg PASID size fields). But it cannot communicate how many PASIDs are in the pool(host cgroup capacity). The QEMU process may not be the only one in a cgroup so it cannot give hard guarantees. I don't see a good way to communicate accurately at runtime as the process migrates or limit changes. We were thinking to adopt the "Limits" model as defined in the cgroup-v2 doc. " Limits -- A child can only consume upto the configured amount of the resource. Limits can be over-committed - the sum of the limits of children can exceed the amount of resource available to the parent. " So the guest cgroup would still think it has full 20 bits of PASID at its disposal. But PASID allocation may fail before reaching the full 20 bits (2M). Similar on the host side, we only enforce the limit set by the cgroup but not guarantee it. > I was thinking of a case where qemu is using a single PASID to setup > the guest kVA or similar > got it. > Jason Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
On Thu, Mar 04, 2021 at 11:01:44AM -0800, Jacob Pan wrote: > > For something like qemu I'd expect to put the qemu process in a cgroup > > with 1 PASID. Who cares what qemu uses the PASID for, or how it was > > allocated? > > For vSVA, we will need one PASID per guest process. But that is up to the > admin based on whether or how many SVA capable devices are directly > assigned. I hope the virtual IOMMU driver can communicate the PASID limit and the cgroup machinery in the guest can know what the actual limit is. I was thinking of a case where qemu is using a single PASID to setup the guest kVA or similar Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
Hi Jason, On Thu, 4 Mar 2021 13:54:02 -0400, Jason Gunthorpe wrote: > On Thu, Mar 04, 2021 at 09:46:03AM -0800, Jacob Pan wrote: > > > Right, I was assuming have three use cases of IOASIDs: > > 1. host supervisor SVA (not a concern, just one init_mm to bind) > > 2. host user SVA, either one IOASID per process or perhaps some private > > IOASID for private address space > > 3. VM use for guest SVA, each IOASID is bound to a guest process > > > > My current cgroup proposal applies to #3 with IOASID_SET_TYPE_MM, which > > is allocated by the new /dev/ioasid interface. > > > > For #2, I was thinking you can limit the host process via PIDs cgroup? > > i.e. limit fork. So the host IOASIDs are currently allocated from the > > system pool with quota of chosen by iommu_sva_init() in my patch, 0 > > means unlimited use whatever is available. > > https://lkml.org/lkml/2021/2/28/18 > > Why do we need two pools? > > If PASID's are limited then why does it matter how the PASID was > allocated? Either the thing requesting it is below the limit, or it > isn't. > you are right. it should be tracked based on the process regardless it is allocated by the user (/dev/ioasid) or indirectly by kernel drivers during iommu_sva_bind_device(). Need to consolidate both 2 and 3 and decouple cgroup and IOASID set. > For something like qemu I'd expect to put the qemu process in a cgroup > with 1 PASID. Who cares what qemu uses the PASID for, or how it was > allocated? > For vSVA, we will need one PASID per guest process. But that is up to the admin based on whether or how many SVA capable devices are directly assigned. > Jason Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
On Thu, Mar 04, 2021 at 09:46:03AM -0800, Jacob Pan wrote: > Right, I was assuming have three use cases of IOASIDs: > 1. host supervisor SVA (not a concern, just one init_mm to bind) > 2. host user SVA, either one IOASID per process or perhaps some private > IOASID for private address space > 3. VM use for guest SVA, each IOASID is bound to a guest process > > My current cgroup proposal applies to #3 with IOASID_SET_TYPE_MM, which is > allocated by the new /dev/ioasid interface. > > For #2, I was thinking you can limit the host process via PIDs cgroup? i.e. > limit fork. So the host IOASIDs are currently allocated from the system pool > with quota of chosen by iommu_sva_init() in my patch, 0 means unlimited use > whatever is available. https://lkml.org/lkml/2021/2/28/18 Why do we need two pools? If PASID's are limited then why does it matter how the PASID was allocated? Either the thing requesting it is below the limit, or it isn't. For something like qemu I'd expect to put the qemu process in a cgroup with 1 PASID. Who cares what qemu uses the PASID for, or how it was allocated? Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
Hi Jean-Philippe, On Thu, 4 Mar 2021 10:49:37 +0100, Jean-Philippe Brucker wrote: > On Wed, Mar 03, 2021 at 04:02:05PM -0800, Jacob Pan wrote: > > Hi Jacob, > > > > On Wed, 3 Mar 2021 13:17:26 -0800, Jacob Pan > > wrote: > > > > > Hi Tejun, > > > > > > On Wed, 3 Mar 2021 10:44:28 -0500, Tejun Heo wrote: > > > > > > > On Sat, Feb 27, 2021 at 02:01:23PM -0800, Jacob Pan wrote: > > > > > IOASIDs are used to associate DMA requests with virtual address > > > > > spaces. They are a system-wide limited resource made available to > > > > > the userspace applications. Let it be VMs or user-space device > > > > > drivers. > > > > > > > > > > This RFC patch introduces a cgroup controller to address the > > > > > following problems: > > > > > 1. Some user applications exhaust all the available IOASIDs thus > > > > > depriving others of the same host. > > > > > 2. System admins need to provision VMs based on their needs for > > > > > IOASIDs, e.g. the number of VMs with assigned devices that perform > > > > > DMA requests with PASID. > > > > > > > > Please take a look at the proposed misc controller: > > > > > > > > http://lkml.kernel.org/r/20210302081705.1990283-2-vipi...@google.com > > > > > > > > Would that fit your bill? > > > The interface definitely can be reused. But IOASID has a different > > > behavior in terms of migration and ownership checking. I guess SEV key > > > IDs are not tied to a process whereas IOASIDs are. Perhaps this can be > > > solved by adding > > > + .can_attach = ioasids_can_attach, > > > + .cancel_attach = ioasids_cancel_attach, > > > Let me give it a try and come back. > > > > > While I am trying to fit the IOASIDs cgroup in to the misc cgroup > > proposal. I'd like to have a direction check on whether this idea of > > using cgroup for IOASID/PASID resource management is viable. > > Yes, even for host SVA it would be good to have a cgroup. Currently the > number of shared address spaces is naturally limited by number of > processes, which can be controlled with rlimit and cgroup. But on Arm the > hardware limit on shared address spaces is 64k (number of ASIDs), easily > exhausted with the default PASID and PID limits. So a cgroup for managing > this resource is more than welcome. > > It looks like your current implementation is very dependent on > IOASID_SET_TYPE_MM? I'll need to do more reading about cgroup to see how > easily it can be adapted to host SVA which uses IOASID_SET_TYPE_NULL. > Right, I was assuming have three use cases of IOASIDs: 1. host supervisor SVA (not a concern, just one init_mm to bind) 2. host user SVA, either one IOASID per process or perhaps some private IOASID for private address space 3. VM use for guest SVA, each IOASID is bound to a guest process My current cgroup proposal applies to #3 with IOASID_SET_TYPE_MM, which is allocated by the new /dev/ioasid interface. For #2, I was thinking you can limit the host process via PIDs cgroup? i.e. limit fork. So the host IOASIDs are currently allocated from the system pool with quota of chosen by iommu_sva_init() in my patch, 0 means unlimited use whatever is available. https://lkml.org/lkml/2021/2/28/18 > Thanks, > Jean Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/5] iommu/arm-smmu-v3: Use pinned VMID for NESTED stage with BTM
Hi Shameer,
On Mon, Feb 22, 2021 at 03:53:37PM +, Shameer Kolothum wrote:
> If the SMMU supports BTM and the device belongs to NESTED domain
> with shared pasid table, we need to use the VMID allocated by the
> KVM for the s2 configuration. Hence, request a pinned VMID from KVM.
>
> Signed-off-by: Shameer Kolothum
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 49 -
> 1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 26bf7da1bcd0..04f83f7c8319 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -2195,6 +2196,33 @@ static void arm_smmu_bitmap_free(unsigned long *map,
> int idx)
> clear_bit(idx, map);
> }
>
> +static int arm_smmu_pinned_vmid_get(struct arm_smmu_domain *smmu_domain)
> +{
> + struct arm_smmu_master *master;
> +
> + master = list_first_entry_or_null(_domain->devices,
> + struct arm_smmu_master, domain_head);
This probably needs to hold devices_lock while using master.
> + if (!master)
> + return -EINVAL;
> +
> + return kvm_pinned_vmid_get(master->dev);
> +}
> +
> +static int arm_smmu_pinned_vmid_put(struct arm_smmu_domain *smmu_domain)
> +{
> + struct arm_smmu_master *master;
> +
> + master = list_first_entry_or_null(_domain->devices,
> + struct arm_smmu_master, domain_head);
> + if (!master)
> + return -EINVAL;
> +
> + if (smmu_domain->s2_cfg.vmid)
> + return kvm_pinned_vmid_put(master->dev);
> +
> + return 0;
> +}
> +
> static void arm_smmu_domain_free(struct iommu_domain *domain)
> {
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> @@ -2215,8 +2243,11 @@ static void arm_smmu_domain_free(struct iommu_domain
> *domain)
> mutex_unlock(_smmu_asid_lock);
> }
> if (s2_cfg->set) {
> - if (s2_cfg->vmid)
> - arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
> + if (s2_cfg->vmid) {
> + if (!(smmu->features & ARM_SMMU_FEAT_BTM) &&
> + smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> + arm_smmu_bitmap_free(smmu->vmid_map,
> s2_cfg->vmid);
> + }
> }
>
> kfree(smmu_domain);
> @@ -3199,6 +3230,17 @@ static int arm_smmu_attach_pasid_table(struct
> iommu_domain *domain,
> !(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> goto out;
>
> + if (smmu->features & ARM_SMMU_FEAT_BTM) {
> + ret = arm_smmu_pinned_vmid_get(smmu_domain);
> + if (ret < 0)
> + goto out;
> +
> + if (smmu_domain->s2_cfg.vmid)
> + arm_smmu_bitmap_free(smmu->vmid_map,
> smmu_domain->s2_cfg.vmid);
> +
> + smmu_domain->s2_cfg.vmid = (u16)ret;
That will require a TLB invalidation on the old VMID, once the STE is
rewritten.
More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without PASID
table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no? They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}
It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and might break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.
Besides we probably want to restrict this feature to systems supporting
VMID16 on both SMMU and CPUs, or at least check that they are compatible.
> + }
> +
> smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
> smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
> smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;
> @@ -3221,6 +3263,7 @@ static int arm_smmu_attach_pasid_table(struct
> iommu_domain *domain,
> static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
> {
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_master *master;
> unsigned long flags;
>
> @@ -3237,6 +3280,8 @@ static void arm_smmu_detach_pasid_table(struct
> iommu_domain *domain)
> arm_smmu_install_ste_for_dev(master);
>
Re: CAAM: kernel BUG at drivers/crypto/caam/jr.c:230! (and dma-coherent query)
On 2021-03-03 16:40, Horia Geantă wrote: On 3/3/2021 4:57 PM, Sascha Hauer wrote: On Wed, Mar 03, 2021 at 12:26:32PM +0200, Horia Geantă wrote: Adding some people in the loop, maybe they could help in understanding why lack of "dma-coherent" property for a HW-coherent device could lead to unexpected / strange side effects. On 3/1/2021 5:22 PM, Sascha Hauer wrote: Hi All, I am on a Layerscape LS1046a using Linux-5.11. The CAAM driver sometimes crashes during the run-time self tests with: kernel BUG at drivers/crypto/caam/jr.c:247! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-20210225-3-00039-g434215968816-dirty #12 Hardware name: TQ TQMLS1046A SoM on Arkona AT1130 (C300) board (DT) pstate: 6005 (nZCv daif -PAN -UAO -TCO BTYPE=--) pc : caam_jr_dequeue+0x98/0x57c lr : caam_jr_dequeue+0x98/0x57c sp : 800010003d50 x29: 800010003d50 x28: 8000118d4000 x27: 8000118d4328 x26: 01f0 x25: 0008022be480 x24: 0008022c6410 x23: 01f1 x22: 8000118d4329 x21: 4d80 x20: 01f1 x19: 0001 x18: 0020 x17: x16: 0015 x15: 800011690230 x14: 2e2e2e2e2e2e2e2e x13: 2e2e2e2e2e2e2020 x12: 3030303030303030 x11: 800011700a38 x10: f000 x9 : 8000100ada30 x8 : 8000116a8a38 x7 : 0001 x6 : x5 : x4 : x3 : x2 : x1 : x0 : 1800 Call trace: caam_jr_dequeue+0x98/0x57c tasklet_action_common.constprop.0+0x164/0x18c tasklet_action+0x44/0x54 __do_softirq+0x160/0x454 __irq_exit_rcu+0x164/0x16c irq_exit+0x1c/0x30 __handle_domain_irq+0xc0/0x13c gic_handle_irq+0x5c/0xf0 el1_irq+0xb4/0x180 arch_cpu_idle+0x18/0x30 default_idle_call+0x3c/0x1c0 do_idle+0x23c/0x274 cpu_startup_entry+0x34/0x70 rest_init+0xdc/0xec arch_call_rest_init+0x1c/0x28 start_kernel+0x4ac/0x4e4 Code: 91392021 912c2000 d377d8c6 97f24d96 (d421) The driver iterates over the descriptors in the output ring and matches them with the ones it has previously queued. If it doesn't find a matching descriptor it complains with the BUG_ON() seen above. What I see sometimes is that the address in the output ring is 0x0, the job status in this case is 0x4006 (meaning DECO Invalid KEY command). It seems that the CAAM doesn't write the descriptor address to the output ring at least in some error cases. When we don't have the descriptor address of the failed descriptor we have no way to find it in the list of queued descriptors, thus we also can't find the callback for that descriptor. This looks very unfortunate, anyone else seen this or has an idea what to do about it? I haven't investigated yet which job actually fails and why. Of course that would be my ultimate goal to find that out. This looks very similar to an earlier report from Greg. He confirmed that adding "dma-coherent" property to the "crypto" DT node fixes the issue: https://lore.kernel.org/linux-crypto/74f664f5-5433-d322-4789-3c78bdb81...@kernel.org Patch rebased on v5.11 is at the bottom. Does it work for you too? Indeed this seems to solve it for me as well, you can add my Tested-by: Sascha Hauer Thanks! I'll append the tag to the formally submitted patch. However, there seem to be two problems: First that "DECO Invalid KEY command" actually occurs and second that the deqeueue code currently can't handle a NULL pointer in the output ring. Currently the dequeue code BUGs not only for "NULL pointer", but for any IOVA in the output ring that is not matched with an entry in the "shadow" (SW) ring. Here the BUG_ON() should be replaced with WARN_ON since not finding a match means driver can't go to the "SW context" and eventually call complete() to wake up the crypto API user. In many cases the user relies on crypto_wait_req(), which does not time out and is not killable. Do you think that the occurence of a NULL pointer is also a coherency issue? I strongly believe there's a single problem because the issue goes away when the patch is applied, even though I haven't figured out what is the exact place / data structure that gets corrupted. One theory is that corruption occurs in the input ring: -CPU sets up correctly the input ring entry -device doesn't see the "fresh" data, reading 0x0 for the descriptor address -device reads the descriptor commands from address 0x0 and issues "DECO invalid KEY command" (note that KEY command opcode is b'0, so reading all zeros from address 0x0 would lead to this error) But then the input & output rings are allocated using dma_alloc_coherent(), so I'll need to check if lack of "dma-coherent" DT property has the same effect on consistent DMA mappings as on streaming DMA mappings. It certainly can, at least on arm64 where coherent buffers are remapped in vmalloc rather than changing the
Re: [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG
On 2021-03-01 08:42, Christoph Hellwig wrote:
Signed-off-by: Christoph Hellwig
Moreso than the previous patch, where the feature is at least relatively
generic (note that there's a bunch of in-flight development around
DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to
bloat the generic iommu_ops structure with private driver-specific
interfaces. The attribute interface is a great compromise for these
kinds of things, and you can easily add type-checked wrappers around it
for external callers (maybe even make the actual attributes internal
between the IOMMU core and drivers) if that's your concern.
Robin.
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 40 +++--
drivers/iommu/iommu.c | 9 ++
include/linux/iommu.h | 9 +-
4 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain *iommu)
struct io_pgtable_domain_attr pgtbl_cfg;
pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
- iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, _cfg);
+ iommu_domain_set_pgtable_attr(iommu, _cfg);
}
struct msm_gem_address_space *
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2e17d990d04481..2858999c86dfd1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct
iommu_domain *domain)
return ret;
}
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
+static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
+ struct io_pgtable_domain_attr *pgtbl_cfg)
{
- int ret = 0;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = -EPERM;
- mutex_lock(_domain->init_mutex);
-
- switch(domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_IO_PGTABLE_CFG: {
- struct io_pgtable_domain_attr *pgtbl_cfg = data;
-
- if (smmu_domain->smmu) {
- ret = -EPERM;
- goto out_unlock;
- }
+ if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+ return -EINVAL;
- smmu_domain->pgtbl_cfg = *pgtbl_cfg;
- break;
- }
- default:
- ret = -ENODEV;
- }
- break;
- case IOMMU_DOMAIN_DMA:
- ret = -ENODEV;
- break;
- default:
- ret = -EINVAL;
+ mutex_lock(_domain->init_mutex);
+ if (!smmu_domain->smmu) {
+ smmu_domain->pgtbl_cfg = *pgtbl_cfg;
+ ret = 0;
}
-out_unlock:
mutex_unlock(_domain->init_mutex);
+
return ret;
}
@@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
.dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
- .domain_set_attr= arm_smmu_domain_set_attr,
+ .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
.domain_enable_nesting = arm_smmu_domain_enable_nesting,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2e9e058501a953..8490aefd4b41f8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain
*domain)
}
EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
+int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,
+ struct io_pgtable_domain_attr *pgtbl_cfg)
+{
+ if (!domain->ops->domain_set_pgtable_attr)
+ return -EINVAL;
+ return domain->ops->domain_set_pgtable_attr(domain, pgtbl_cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_domain_set_pgtable_attr);
+
void iommu_get_resv_regions(struct device *dev, struct list_head *list)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index aed88aa3bd3edf..39d3ed4d2700ac 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
struct notifier_block;
struct iommu_sva;
struct iommu_fault_event;
+struct io_pgtable_domain_attr;
Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
On 2021-03-01 08:42, Christoph Hellwig wrote:
Use explicit methods for setting and querying the information instead.
Now that everyone's using iommu-dma, is there any point in bouncing this
through the drivers at all? Seems like it would make more sense for the
x86 drivers to reflect their private options back to iommu_dma_strict
(and allow Intel's caching mode to override it as well), then have
iommu_dma_init_domain just test !iommu_dma_strict &&
domain->ops->flush_iotlb_all.
Robin.
Also remove the now unused iommu_domain_get_attr functionality.
Signed-off-by: Christoph Hellwig
---
drivers/iommu/amd/iommu.c | 23 ++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 47 ++---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 56 +
drivers/iommu/dma-iommu.c | 8 ++-
drivers/iommu/intel/iommu.c | 27 ++
drivers/iommu/iommu.c | 19 +++
include/linux/iommu.h | 17 ++-
7 files changed, 51 insertions(+), 146 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40d0..37a8e51db17656 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1771,24 +1771,11 @@ static struct iommu_group
*amd_iommu_device_group(struct device *dev)
return acpihid_device_group(dev);
}
-static int amd_iommu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
+static bool amd_iommu_dma_use_flush_queue(struct iommu_domain *domain)
{
- switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- return -ENODEV;
- case IOMMU_DOMAIN_DMA:
- switch (attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = !amd_iommu_unmap_flush;
- return 0;
- default:
- return -ENODEV;
- }
- break;
- default:
- return -EINVAL;
- }
+ if (domain->type != IOMMU_DOMAIN_DMA)
+ return false;
+ return !amd_iommu_unmap_flush;
}
/*
@@ -2257,7 +2244,7 @@ const struct iommu_ops amd_iommu_ops = {
.release_device = amd_iommu_release_device,
.probe_finalize = amd_iommu_probe_finalize,
.device_group = amd_iommu_device_group,
- .domain_get_attr = amd_iommu_domain_get_attr,
+ .dma_use_flush_queue = amd_iommu_dma_use_flush_queue,
.get_resv_regions = amd_iommu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.is_attach_deferred = amd_iommu_is_attach_deferred,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8594b4a8304375..bf96172e8c1f71 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2449,33 +2449,21 @@ static struct iommu_group *arm_smmu_device_group(struct
device *dev)
return group;
}
-static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
+static bool arm_smmu_dma_use_flush_queue(struct iommu_domain *domain)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
- switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_NESTING:
- *(int *)data = (smmu_domain->stage ==
ARM_SMMU_DOMAIN_NESTED);
- return 0;
- default:
- return -ENODEV;
- }
- break;
- case IOMMU_DOMAIN_DMA:
- switch (attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = smmu_domain->non_strict;
- return 0;
- default:
- return -ENODEV;
- }
- break;
- default:
- return -EINVAL;
- }
+ if (domain->type != IOMMU_DOMAIN_DMA)
+ return false;
+ return smmu_domain->non_strict;
+}
+
+
+static void arm_smmu_dma_enable_flush_queue(struct iommu_domain *domain)
+{
+ if (domain->type != IOMMU_DOMAIN_DMA)
+ return;
+ to_smmu_domain(domain)->non_strict = true;
}
static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
@@ -2505,13 +2493,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
}
break;
case IOMMU_DOMAIN_DMA:
- switch(attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- smmu_domain->non_strict = *(int *)data;
- break;
- default:
- ret =
Re: [Patch v8 04/10] vfio/type1: Support binding guest page tables to PASID
On Thu, Mar 04, 2021 at 07:20:22AM +, Liu, Yi L wrote: > > > However, IOMMU is a system device which has little value to be exposed > > to > > > the userspace. Not to mention the device-IOMMU affinity/topology. VFIO > > > nicely abstracts IOMMU from the userspace, why do we want to reverse > > that? > > > > The other patch was talking about a /dev/ioasid - why can't this stuff > > be run over that? > > The stuff in this patch are actually iommu domain operations, which are > finally supported by iommu domain ops. While /dev/ioasid in another patch > is created for IOASID allocation/free to fit the PASID allocation requirement > from both vSVA and vDPA. It has no idea about iommu domain and neither the > device information. Without such info, /dev/ioasid is unable to run this > stuff. Why can't it know? My point was that VFIO should interact with /dev/ioasid to exchange the information both sides need and once exchanged the control over IOMMU should be done through /dev/ioasid, not inside VFIO. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Fix status code for Allocate/Free PASID command
On Sat, Feb 27, 2021 at 03:39:09PM +0800, Zenghui Yu wrote:
> As per Intel vt-d spec, Rev 3.0 (section 10.4.45 "Virtual Command Response
> Register"), the status code of "No PASID available" error in response to
> the Allocate PASID command is 2, not 1. The same for "Invalid PASID" error
> in response to the Free PASID command.
>
> We will otherwise see confusing kernel log under the command failure from
> guest side. Fix it.
>
> Fixes: 24f27d32ab6b ("iommu/vt-d: Enlightened PASID allocation")
> Signed-off-by: Zenghui Yu
Applied for v5.12, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Thu, Feb 18, 2021 at 02:07:02PM -0800, Nicolin Chen wrote: > drivers/iommu/tegra-smmu.c | 72 +- > 1 file changed, 71 insertions(+), 1 deletion(-) Applied for v5.12, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries
On Thu, Feb 25, 2021 at 02:26:51PM +0800, Lu Baolu wrote: > When the first level page table is used for IOVA translation, it only > supports Read-Only and Read-Write permissions. The Write-Only permission > is not supported as the PRESENT bit (implying Read permission) should > always set. When using second level, we still give separate permissions > that allows WriteOnly which seems inconsistent and awkward. There is no > use case we can think off, hence remove that configuration to make it > consistent. No use-case for WriteOnly mappings? How about DMA_FROM_DEVICE mappings? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix sleeping in atomic in increase_address_space()
On Wed, Feb 17, 2021 at 06:10:02PM +, Will Deacon wrote:
> > drivers/iommu/amd/iommu.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
>
> Acked-by: Will Deacon
Applied for v5.12, thanks.
There were some conflicts which I resolved, can you please check the
result, Andrey? The updated patch is attached.
>From 140456f994195b568ecd7fc2287a34eadffef3ca Mon Sep 17 00:00:00 2001
From: Andrey Ryabinin
Date: Wed, 17 Feb 2021 17:30:04 +0300
Subject: [PATCH] iommu/amd: Fix sleeping in atomic in increase_address_space()
increase_address_space() calls get_zeroed_page(gfp) under spin_lock with
disabled interrupts. gfp flags passed to increase_address_space() may allow
sleeping, so it comes to this:
BUG: sleeping function called from invalid context at mm/page_alloc.c:4342
in_atomic(): 1, irqs_disabled(): 1, pid: 21555, name: epdcbbf1qnhbsd8
Call Trace:
dump_stack+0x66/0x8b
___might_sleep+0xec/0x110
__alloc_pages_nodemask+0x104/0x300
get_zeroed_page+0x15/0x40
iommu_map_page+0xdd/0x3e0
amd_iommu_map+0x50/0x70
iommu_map+0x106/0x220
vfio_iommu_type1_ioctl+0x76e/0x950 [vfio_iommu_type1]
do_vfs_ioctl+0xa3/0x6f0
ksys_ioctl+0x66/0x70
__x64_sys_ioctl+0x16/0x20
do_syscall_64+0x4e/0x100
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fix this by moving get_zeroed_page() out of spin_lock/unlock section.
Fixes: 754265bcab ("iommu/amd: Fix race in increase_address_space()")
Signed-off-by: Andrey Ryabinin
Acked-by: Will Deacon
Cc:
Link: https://lore.kernel.org/r/20210217143004.19165-1-a...@yandex-team.com
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd/io_pgtable.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index 1c4961e05c12..bb0ee5c9fde7 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -182,6 +182,10 @@ static bool increase_address_space(struct
protection_domain *domain,
bool ret = true;
u64 *pte;
+ pte = (void *)get_zeroed_page(gfp);
+ if (!pte)
+ return false;
+
spin_lock_irqsave(>lock, flags);
if (address <= PM_LEVEL_SIZE(domain->iop.mode))
@@ -191,10 +195,6 @@ static bool increase_address_space(struct
protection_domain *domain,
if (WARN_ON_ONCE(domain->iop.mode == PAGE_MODE_6_LEVEL))
goto out;
- pte = (void *)get_zeroed_page(gfp);
- if (!pte)
- goto out;
-
*pte = PM_LEVEL_PDE(domain->iop.mode,
iommu_virt_to_phys(domain->iop.root));
domain->iop.root = pte;
@@ -208,10 +208,12 @@ static bool increase_address_space(struct
protection_domain *domain,
*/
amd_iommu_domain_set_pgtable(domain, pte, domain->iop.mode);
+ pte = NULL;
ret = true;
out:
spin_unlock_irqrestore(>lock, flags);
+ free_page((unsigned long)pte);
return ret;
}
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: cleanup unused or almost unused IOMMU APIs and the FSL PAMU driver
On Mon, Mar 01, 2021 at 09:42:40AM +0100, Christoph Hellwig wrote: > Diffstat: > arch/powerpc/include/asm/fsl_pamu_stash.h | 12 > drivers/gpu/drm/msm/adreno/adreno_gpu.c |2 > drivers/iommu/amd/iommu.c | 23 > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 85 --- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 122 +--- > drivers/iommu/dma-iommu.c |8 > drivers/iommu/fsl_pamu.c| 264 -- > drivers/iommu/fsl_pamu.h| 10 > drivers/iommu/fsl_pamu_domain.c | 694 > ++-- > drivers/iommu/fsl_pamu_domain.h | 46 - > drivers/iommu/intel/iommu.c | 55 -- > drivers/iommu/iommu.c | 75 --- > drivers/soc/fsl/qbman/qman_portal.c | 56 -- > drivers/vfio/vfio_iommu_type1.c | 31 - > drivers/vhost/vdpa.c| 10 > include/linux/iommu.h | 81 --- > 16 files changed, 214 insertions(+), 1360 deletions(-) Nice cleanup, thanks. The fsl_pamu driver and interface has always been a little bit of an alien compared to other IOMMU drivers. I am inclined to merge this after -rc3 is out, given some reviews. Can you also please add changelogs to the last three patches? Thanks, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 15/18] cgroup: Introduce ioasids controller
On Wed, Mar 03, 2021 at 04:02:05PM -0800, Jacob Pan wrote: > Hi Jacob, > > On Wed, 3 Mar 2021 13:17:26 -0800, Jacob Pan > wrote: > > > Hi Tejun, > > > > On Wed, 3 Mar 2021 10:44:28 -0500, Tejun Heo wrote: > > > > > On Sat, Feb 27, 2021 at 02:01:23PM -0800, Jacob Pan wrote: > > > > IOASIDs are used to associate DMA requests with virtual address > > > > spaces. They are a system-wide limited resource made available to the > > > > userspace applications. Let it be VMs or user-space device drivers. > > > > > > > > This RFC patch introduces a cgroup controller to address the following > > > > problems: > > > > 1. Some user applications exhaust all the available IOASIDs thus > > > > depriving others of the same host. > > > > 2. System admins need to provision VMs based on their needs for > > > > IOASIDs, e.g. the number of VMs with assigned devices that perform > > > > DMA requests with PASID. > > > > > > Please take a look at the proposed misc controller: > > > > > > http://lkml.kernel.org/r/20210302081705.1990283-2-vipi...@google.com > > > > > > Would that fit your bill? > > The interface definitely can be reused. But IOASID has a different > > behavior in terms of migration and ownership checking. I guess SEV key > > IDs are not tied to a process whereas IOASIDs are. Perhaps this can be > > solved by adding > > + .can_attach = ioasids_can_attach, > > + .cancel_attach = ioasids_cancel_attach, > > Let me give it a try and come back. > > > While I am trying to fit the IOASIDs cgroup in to the misc cgroup proposal. > I'd like to have a direction check on whether this idea of using cgroup for > IOASID/PASID resource management is viable. Yes, even for host SVA it would be good to have a cgroup. Currently the number of shared address spaces is naturally limited by number of processes, which can be controlled with rlimit and cgroup. But on Arm the hardware limit on shared address spaces is 64k (number of ASIDs), easily exhausted with the default PASID and PID limits. So a cgroup for managing this resource is more than welcome. It looks like your current implementation is very dependent on IOASID_SET_TYPE_MM? I'll need to do more reading about cgroup to see how easily it can be adapted to host SVA which uses IOASID_SET_TYPE_NULL. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
Hi Robin, On Tue, 16 Feb 2021 at 23:10, Robin Murphy wrote: > > >>> > >>> On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote: > From: Chunyan Zhang > > This iommu module can be used by Unisoc's multimedia devices, such as > display, Image codec(jpeg) and a few signal processors, including > VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), > etc. > > Signed-off-by: Chunyan Zhang > --- > .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++ > 1 file changed, 72 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > new file mode 100644 > index ..4fc99e81fa66 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright 2020 Unisoc Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Unisoc IOMMU and Multi-media MMU > + > +maintainers: > + - Chunyan Zhang > + > +properties: > + compatible: > +enum: > + - sprd,iommu-v1 > + > + "#iommu-cells": > +const: 0 > +description: > + Unisoc IOMMUs are all single-master IOMMU devices, therefore no > + additional information needs to associate with its master device. > + Please refer to the generic bindings document for more details, > + Documentation/devicetree/bindings/iommu/iommu.txt > + > + reg: > +maxItems: 1 > +description: > + Not required if 'sprd,iommu-regs' is defined. > + > + clocks: > +description: > + Reference to a gate clock phandle, since access to some of IOMMUs > are > + controlled by gate clock, but this is not required. > + > + sprd,iommu-regs: > +$ref: /schemas/types.yaml#/definitions/phandle-array > +description: > + Reference to a syscon phandle plus 1 cell, the syscon defines the > + register range used by the iommu and the media device, the cell > + defines the offset for iommu registers. Since iommu module shares > + the same register range with the media device which uses it. > + > +required: > + - compatible > + - "#iommu-cells" > > OK, so apparently the hardware is not quite as trivial as my initial > impression, and you should have interrupts as well. I've checked with my colleagues for this issue. And like I explained before, one sprd IOMMU serves one master device only, so interrupts are handled by master devices rather than IOMMUs. Chunyan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] iommu/vt-d: Enable write protect for supervisor SVM
Hi Jacob,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on linux/master linus/master v5.12-rc1 next-20210303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jacob-Pan/Misc-vSVA-fixes-for-VT-d/20210219-141141
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-randconfig-r023-20210304 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/4c1de3403ecb6f91cc1bdc5e3ca81f16a2ffc0b5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jacob-Pan/Misc-vSVA-fixes-for-VT-d/20210219-141141
git checkout 4c1de3403ecb6f91cc1bdc5e3ca81f16a2ffc0b5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/pgtable.h:154,
from include/linux/pgtable.h:6,
from include/linux/mm.h:33,
from include/linux/scatterlist.h:8,
from include/linux/dma-mapping.h:10,
from include/linux/iova.h:16,
from include/linux/intel-iommu.h:14,
from drivers/iommu/intel/pasid.c:15:
arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
arch/ia64/include/asm/mmu_context.h:127:41: warning: variable 'old_rr4' set
but not used [-Wunused-but-set-variable]
127 | unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
| ^~~
drivers/iommu/intel/pasid.c: In function 'pasid_enable_wpe':
>> drivers/iommu/intel/pasid.c:536:22: error: implicit declaration of function
>> 'read_cr0' [-Werror=implicit-function-declaration]
536 | unsigned long cr0 = read_cr0();
| ^~~~
In file included from include/linux/build_bug.h:5,
from include/linux/bits.h:22,
from include/linux/bitops.h:6,
from drivers/iommu/intel/pasid.c:12:
>> drivers/iommu/intel/pasid.c:539:23: error: 'X86_CR0_WP' undeclared (first
>> use in this function)
539 | if (unlikely(!(cr0 & X86_CR0_WP))) {
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
drivers/iommu/intel/pasid.c:539:23: note: each undeclared identifier is
reported only once for each function it appears in
539 | if (unlikely(!(cr0 & X86_CR0_WP))) {
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
cc1: some warnings being treated as errors
vim +/read_cr0 +536 drivers/iommu/intel/pasid.c
533
534 static inline int pasid_enable_wpe(struct pasid_entry *pte)
535 {
> 536 unsigned long cr0 = read_cr0();
537
538 /* CR0.WP is normally set but just to be sure */
> 539 if (unlikely(!(cr0 & X86_CR0_WP))) {
540 pr_err_ratelimited("No CPU write protect!\n");
541 return -EINVAL;
542 }
543 pasid_set_wpe(pte);
544
545 return 0;
546 };
547
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
