Re: [PATCH v4 1/2] iommu/arm-smmu-v3: Add a cmdq_entries module parameter

[Kiryl Shutsemau <kas@kernel.org>]

On Wed, Sep 02, 2026 at 10:22:31AM -0700, Nicolin Chen wrote:
> On Wed, Sep 02, 2026 at 01:17:23PM +0100, Kiryl Shutsemau (Meta) wrote:
> > +/**
> > + * arm_smmu_queue_max_n_shift() - pick the log2 depth of a queue
> > + * @hw_shift: log2 depth the hardware allows, capped for natural alignment
> > + * @ent_sz_shift: log2 of the queue entry size in bytes
> > + * @want: number of entries asked for, or zero to use @hw_shift
> > + *
> > + * @want is rounded down to a power of two. It never sizes a queue below one
> > + * page, because coherent DMA is page granular: a shallower queue occupies the
> > + * same memory as one that fills the page, and arm_smmu_init_one_queue() stops
> > + * shrinking at a page too.
> 
> Well, since we want the cmdq depth, why not name the parameter
> "max_cmdq_depth"? Then, no rounding and more straightforward.

The max_ prefix is better, thanks. cmdq_entries does not say it is a
ceiling.

On the units, the page floor has to stay whatever we call the parameter.

Ask for 8 entries and max_n_shift comes out as 3, which the
CMDQ_BATCH_ENTRIES check in arm_smmu_device_hw_probe() rejects with
-ENXIO. The floor gives you 256 entries on a 4K kernel instead of a failed
probe.

So log2 removes the rounding but not the flooring, and the number asked for
still is not always the number you get. I would rather keep entries than
make people convert.

The rounding is not silent, either: arm_smmu_init_one_queue() already
prints "allocated %u entries for cmdq", so the effective depth lands in
dmesg whichever unit the parameter takes.

On the name itself, "depth" reads to me as a number of entries rather than
its log2, so max_cmdq_depth=8 meaning 256 seems more surprising than the
rounding it replaces.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov