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

[Jonathan Cameron]

> Now, arm_smmu_page_response() can issue CMDQ_OP_PRI_RESP for page requests
> from the IOPF infrastructure. Forward PRI requests from the priq into the
> IOPF infrastructure for PRI-enabled masters by building an iopf_fault from
> the priq entry and calling iommu_report_device_fault().
> 
> 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.

> the path into one single ratelimited line, as a malfunctioning device can
> spam unrecognised requests and easily flood the kernel log.
> 
> Discard any PASID Stop Marker (LRW = 0b100) prior to the fault report and
> the DENY fallback both, because a Stop Marker does not expect a response.
> The IOPF infrastructure requires the driver to discard it, as per the doc
> at iommu_report_device_fault(). This also matches the intel-iommu code.
> 
> Note that master->pri_enabled will only be set by a later change, once all
> the PRI paths are ready.
> 
> 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?

> 
> Co-developed-by: Barak Biber <bbiber@xxxxxxxxxx>
> Signed-off-by: Barak Biber <bbiber@xxxxxxxxxx>
> Co-developed-by: Stefan Kaestle <skaestle@xxxxxxxxxx>
> Signed-off-by: Stefan Kaestle <skaestle@xxxxxxxxxx>
> Signed-off-by: Malak Marrid <mmarrid@xxxxxxxxxx>
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>

This looks fine to me, but running low on time today and
I didn't check all the flags etc, so no tag.

-- 
Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>