Re: [PATCH v3 09/13] iommu/arm-smmu-v3: Support PRI Page Request in arm_smmu_handle_ppr()

[Jonathan Cameron]

On Fri, 4 Sep 2026 22:08:24 -0700
Nicolin Chen <nicolinc@xxxxxxxxxx> wrote:

> On Thu, Sep 03, 2026 at 12:18:33PM -0700, Jonathan Cameron wrote:
> > > For an unrecognised StreamID or a master without master->pri_enabled, fall
> > > through to the existing "unexpected PRI request" log + LAST-page DENY path
> > > to release the credit per the PCIe PRI spec. Merge its two info prints in  
> > 
> > Either don't bother mentioning pcie spec explicitly or expand that
> > to a useful reference - so section number, section title.  
> 
> Dropped:
> 
> For an unrecognised StreamID or a master without master->pri_enabled, fall
> through to the existing "unexpected PRI request" log + LAST-page DENY path.
> Merge its two info prints in the path into one single ratelimited line, as
> a malfunctioning device can spam unrecognised requests that will flood the
> kernel log.
> 
> > > On PRIQ overflow, partial requests stored via report_partial_fault() whose
> > > LAST-page entry was lost stay in iopf_param->partial. Drop them by calling
> > > iopf_queue_discard_partial(), matching intel-iommu's handling. Do it after
> > > the queue is fully drained, since the visible entries all precede the loss
> > > point: a group whose LAST-page entry is still in the queue gets assembled
> > > before the discard, rather than losing its stored partials to it. Nor can
> > > a new arrival race against the discard, since an active overflow inhibits
> > > new entries from being written to the PRI queue (IHI0070 8.1), up until a
> > > final CONS write acknowledges it.  
> > 
> > Kind of a general comment.  I'm guessing AI assisted heavily in these.
> > Whilst I haven't found much that definitely doesn't belong in a patch
> > description, they do end up rather more verbose than ideal.  Maybe
> > poke it to be more terse in English language choices at least?  
> 
> Well, some of the paragraphs in the commit messages turned out to be
> indeed verbose, compared to my original writings. One of the reasons
> was to justify against Sashiko that often nitpicks on a corner case,
> (which surprisingly works well).

*groan*  I understand that works and I guess this is the life we live in
now, but I wish it didn't give me more to read! :)

> 
> I broke it into parts; hope this might ease a bit:
> 
> On PRIQ overflow, partial requests stored via report_partial_fault() whose
> LAST-page entry was lost stay in iopf_param->partial. Drop them by calling
> iopf_queue_discard_partial(), matching intel-iommu's handling.
> 
> Do it only after the queue is fully drained, since the visible entries all
> precede the loss point: a group whose LAST-page entry is still in the queue
> gets assembled before the discard, rather than losing its stored partials
> to it.
> 
> Nothing can arrive in between, as an active overflow inhibits new entries
> from being written to the PRI queue until a final CONS write acknowledges
> it (IHI0070 8.1).
> 
This is fine.
Thanks

J
> Thanks
> Nicolin