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

[Kiryl Shutsemau <kas@kernel.org>]

On Thu, Sep 03, 2026 at 08:09:55AM -0700, Nicolin Chen wrote:
> 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.

cmdq_max_entries it is. Renamed for the next version.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov