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

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Sep 03, 2026 at 03:15:27PM +0100, Kiryl Shutsemau wrote:
> 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.

Well, maybe "cmdq_max_n_shift" is a better one over "depth".

Given CMDQ_BATCH_ENTRIES is used, I think cmdq_max_entries works.

Nicolin