Re: [PATCH 4/4] iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist()

From: John Garry
Date: Wed Jun 24 2020 - 04:16:50 EST



I'd say that GENMASK_INPUT_CHECK() should be able to handle a l=0 and
h=unsigned value, so I doubt this warn.

Using GENMASK((int)cmdq->q.llq.max_n_shift, 0) resolves it, but it looks
like GENMASK_INPUT_CHECK() could be improved.

That said, I think this particular case might be even better off dodging
GENMASK() entirely, by doing something like this first. Untested...

Robin.

----->8-----
Subject: [PATCH] iommu/arm-smmu-v3: Streamline queue calculations

Beyond the initial queue setup based on the log2 values from ID
registers, the log2 queue size is only ever used in the form of
(1 << max_n_shift) to repeatedly recalculate the number of queue
elements. Simply storing it in that form leads to slightly more
efficient code, particularly in the low-level queue accessors
where it counts most:

add/remove: 0/0 grow/shrink: 1/7 up/down: 4/-120 (-116)
Function old new delta
arm_smmu_init_one_queue 360 364 +4
arm_smmu_priq_thread 512 508 -4
arm_smmu_evtq_thread 300 292 -8
__arm_smmu_cmdq_poll_set_valid_map.isra 296 288 -8
queue_remove_raw 180 164 -16
arm_smmu_gerror_handler 732 716 -16
arm_smmu_device_probe 4312 4284 -28
arm_smmu_cmdq_issue_cmdlist 1892 1852 -40
Total: Before=20135, After=20019, chg -0.58%

Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---

[...]


}
- smmu->evtq.q.llq.max_n_shift = min_t(u32, EVTQ_MAX_SZ_SHIFT,
- FIELD_GET(IDR1_EVTQS, reg));
- smmu->priq.q.llq.max_n_shift = min_t(u32, PRIQ_MAX_SZ_SHIFT,
- FIELD_GET(IDR1_PRIQS, reg));
+ max_n_shift = min_t(u32, EVTQ_MAX_SZ_SHIFT, FIELD_GET(IDR1_EVTQS, reg));
+ smmu->evtq.q.llq.max_n = 1 << max_n_shift;

So I require the bitmask of this for the prod, which would be (max_n << 1) - 1.

I don't feel too strongly either way, and the other big changes in this series need to be considered first...

Thanks,
John

+
+ max_n_shift = min_t(u32, PRIQ_MAX_SZ_SHIFT, FIELD_GET(IDR1_PRIQS, reg));
+ smmu->priq.q.llq.max_n = 1 << max_n_shift;
/* SID/SSID sizes */
smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);