[PATCH rc] iommu/arm-smmu-v3: Do not use GFP_KERNEL under as spinlock

[Will Deacon]

Hi Jason,

On Tue, Feb 13, 2024 at 08:18:50AM -0400, Jason Gunthorpe wrote:
> On Tue, Feb 13, 2024 at 11:20:15AM +0000, Will Deacon wrote:
> 
> > Damn, all the surrounding rework has really left this poor code in a bit
> > of a state. 
> 
> That's one way to describe it. There is a reason it took me so many
> patches to align this to the new core APIs :(
> 
> > Could you please respin your fix, but with an additional change
> > to arm_smmu_mmu_notifier_get() so that it returns an error if there is
> > more than one device in the domain, with some of the above in a comment?
> 
> Adding a check is not a comprehensive solution, there are still ways
> userspace can attack this code with iommufd's coming PASID support. It
> certainly doesn't belong in this patch which should be backported.

Ok, then how about avoiding the allocation entirely once the lock is held?
Untested diff below, but you'd call arm_smmu_init_cd_table() from
arm_smmu_sva_set_dev_pasid(). I'd just like to make sure we really do
avoid allocating from atomic context if we end up with a multi-master
domain.

> I can summarize some of these details in a comment for this patch.

Alternatively, we can just revert the offending commits if we're not able
to agree on a simple fix for now. I'd prefer to avoid that though.

Will

--->8

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 0ffb1cf17e0b..8ca92639b13a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1019,7 +1019,10 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
        WRITE_ONCE(*dst, cpu_to_le64(val));
 }
 
-static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
+static __le64 *
+__arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid,
+                     int (*cd_alloc)(struct arm_smmu_device *,
+                                     struct arm_smmu_l1_ctx_desc *))
 {
        __le64 *l1ptr;
        unsigned int idx;
@@ -1033,7 +1036,7 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
        idx = ssid >> CTXDESC_SPLIT;
        l1_desc = &cd_table->l1_desc[idx];
        if (!l1_desc->l2ptr) {
-               if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc))
+               if (!cd_alloc || cd_alloc(smmu, l1_desc))
                        return NULL;
 
                l1ptr = cd_table->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
@@ -1045,6 +1048,19 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
        return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
 }
 
+static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
+{
+       return __arm_smmu_get_cd_ptr(master, ssid, NULL);
+}
+
+int arm_smmu_init_cd_table(struct arm_smmu_master *master, u32 ssid)
+{
+       if (!__arm_smmu_get_cd_ptr(master, ssid, arm_smmu_alloc_cd_leaf_table))
+               return -ENOMEM;
+
+       return 0;
+}
+
 int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
                            struct arm_smmu_ctx_desc *cd)
 {