Re: [PATCH v4 23/26] iommu/arm-smmu-v3: Add stall support for platform devices

Xu Zaibo
2020-02-26T08:45:08Z

Hi,

On 2020/2/25 2:23, Jean-Philippe Brucker wrote:
> From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>
> The SMMU provides a Stall model for handling page faults in platform
> devices. It is similar to PCI PRI, but doesn't require devices to have
> their own translation cache. Instead, faulting transactions are parked and
> the OS is given a chance to fix the page tables and retry the transaction.
>
> Enable stall for devices that support it (opt-in by firmware). When an
> event corresponds to a translation error, call the IOMMU fault handler. If
> the fault is recoverable, it will call us back to terminate or continue
> the stall.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/arm-smmu-v3.c | 271 ++++++++++++++++++++++++++++++++++--
> drivers/iommu/of_iommu.c | 5 +-
> include/linux/iommu.h | 2 +
> 3 files changed, 269 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 6a5987cce03f..da5dda5ba26a 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -374,6 +374,13 @@
> #define CMDQ_PRI_1_GRPID GENMASK_ULL(8, 0)
> #define CMDQ_PRI_1_RESP GENMASK_ULL(13, 12)
>
[...]
> +static int arm_smmu_page_response(struct device *dev,
> + struct iommu_fault_event *unused,
> + struct iommu_page_response *resp)
> +{
> + struct arm_smmu_cmdq_ent cmd = {0};
> + struct arm_smmu_master *master = dev_iommu_fwspec_get(dev)->iommu_priv;
Here can use 'dev_to_master' ?

Cheers,
Zaibo

.
> + int sid = master->streams[0].id;
> +
> + if (master->stall_enabled) {
> + cmd.opcode = CMDQ_OP_RESUME;
> + cmd.resume.sid = sid;
> + cmd.resume.stag = resp->grpid;
> + switch (resp->code) {
> + case IOMMU_PAGE_RESP_INVALID:
> + case IOMMU_PAGE_RESP_FAILURE:
> + cmd.resume.resp = CMDQ_RESUME_0_RESP_ABORT;
> + break;
> + case IOMMU_PAGE_RESP_SUCCESS:
> + cmd.resume.resp = CMDQ_RESUME_0_RESP_RETRY;
> + break;
> + default:
> + return -EINVAL;
> + }
> + } else {
> + /* TODO: insert PRI response here */
> + return -ENODEV;
> + }
> +
> + arm_smmu_cmdq_issue_cmd(master->smmu, &cmd);
> + /*
> + * Don't send a SYNC, it doesn't do anything for RESUME or PRI_RESP.
> + * RESUME consumption guarantees that the stalled transaction will be
> + * terminated... at some point in the future. PRI_RESP is fire and
> + * forget.
> + */
> +
> + return 0;
> +}
> + …