linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: jean-philippe.brucker@arm.com (Jean-Philippe Brucker)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC] iommu: arm-smmu: stall support
Date: Wed, 27 Sep 2017 14:49:00 +0100	[thread overview]
Message-ID: <4e2fc08e-3e7e-3355-17e5-72106196c732@arm.com> (raw)
In-Reply-To: <20170927121540.GL8398@8bytes.org>

Hi Joerg,

On 27/09/17 13:15, Joerg Roedel wrote:
> Hi Rob, Jean,
> 
> On Fri, Sep 22, 2017 at 02:42:44PM -0400, Rob Clark wrote:
>> I'm in favour if splitting the reporting *somehow*.. the two
>> approaches that seemed sane are:
>> 
>> 1) call fault handler from irq and having separate domain->resume()
>> called by the driver, potentially from a wq
>> 2) or having two fault callbacks, first called before wq and then
>> based on returned value, optionally 2nd callback called from wq
>> 
>> The first seemed less intrusive to me, but I'm flexible.
> 
> How about adding a flag to the fault-handler call-back that tells us
> whether it wants to sleep or not. If it wants, we call it from a wq, if
> not we call call it directly like we do today in the
> report_iommu_fault() function.
> 
> In any case we call iommu_ops->resume() when set on completion of the
> fault-handler either from the workqueue or report_iommu_fault itself.

I like this approach. When the device driver registers a fault handler,
it also tells when it would like to be called (either in atomic context,
blocking context, or both).

Then the handler itself receives a flag that says which context it's
being called from. It returns a value telling the IOMMU how to proceed.
Depending on this value we either resume/abort immediately, or add the
fault to the workqueue if necessary.

How about using the following return values:

/**
 * enum iommu_fault_status - Return status of fault handlers, telling the IOMMU
 *      driver how to proceed with the fault.
 *
 * @IOMMU_FAULT_STATUS_NONE: Fault was not handled. Call the next handler, or
 *      terminate.
 * @IOMMU_FAULT_STATUS_FAILURE: General error. Drop all subsequent faults from
 *      this device if possible. This is "Response Failure" in PCI PRI.
 * @IOMMU_FAULT_STATUS_INVALID: Could not handle this fault, don't retry the
 *      access. This is "Invalid Request" in PCI PRI.
 * @IOMMU_FAULT_STATUS_HANDLED: Fault has been handled and the page tables
 *      populated, retry the access.
 * @IOMMU_FAULT_STATUS_IGNORE: Stop processing the fault, and do not send a
 *      reply to the device.
 *
 * For unrecoverable faults, the only valid status is IOMMU_FAULT_STATUS_NONE
 * For a recoverable fault, if no one handled the fault, treat as
 * IOMMU_FAULT_STATUS_INVALID.
 */
enum iommu_fault_status {
        IOMMU_FAULT_STATUS_NONE = 0,
        IOMMU_FAULT_STATUS_FAILURE,
        IOMMU_FAULT_STATUS_INVALID,
        IOMMU_FAULT_STATUS_HANDLED,
        IOMMU_FAULT_STATUS_IGNORE,
};

This would probably cover the two use-cases of reporting faults to
device drivers, and injecting them into the guest with VFIO, as well as
handling PPRs internally. I'm also working on providing more details
(pasid for instance) in the fault callback.

We could also use the fault handler for invalid PRI Page Requests
(currently specialized by amd_iommu_set_invalid_ppr_cb). It's just a
matter of adding a registration flag to iommu_set_fault_handler.

Thanks,
Jean

  reply	other threads:[~2017-09-27 13:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-14 19:44 [RFC] iommu: arm-smmu: stall support Rob Clark
2017-09-18 11:13 ` Jean-Philippe Brucker
2017-09-18 12:11   ` Rob Clark
2017-09-18 17:33     ` Will Deacon
2017-09-19 12:30 ` Joerg Roedel
2017-09-19 14:23   ` Rob Clark
2017-09-22  9:02     ` Joerg Roedel
2017-09-22 10:02       ` Jean-Philippe Brucker
2017-09-22 18:42         ` Rob Clark
2017-09-27 12:15           ` Joerg Roedel
2017-09-27 13:49             ` Jean-Philippe Brucker [this message]
2017-09-27 14:35               ` Joerg Roedel
2017-09-27 16:14               ` Rob Clark
2019-05-10 18:23         ` Rob Clark
2019-05-13 18:37           ` Jean-Philippe Brucker
2019-05-14  1:54             ` Rob Clark
2019-05-14 10:24               ` Robin Murphy
2019-05-14 17:17                 ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4e2fc08e-3e7e-3355-17e5-72106196c732@arm.com \
    --to=jean-philippe.brucker@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).