From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Vivek Gautam <vivek.gautam@arm.com>
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
iommu@lists.linux-foundation.org,
virtualization@lists.linux-foundation.org, joro@8bytes.org,
will.deacon@arm.com, mst@redhat.com, robin.murphy@arm.com,
eric.auger@redhat.com, kevin.tian@intel.com,
jacob.jun.pan@linux.intel.com, yi.l.liu@intel.com,
Lorenzo.Pieralisi@arm.com, shameerali.kolothum.thodi@huawei.com
Subject: Re: [PATCH RFC v1 03/11] iommu/virtio: Handle incoming page faults
Date: Tue, 21 Sep 2021 17:03:25 +0100 [thread overview]
Message-ID: <YUoCTV6WYxxE10qj@myrica> (raw)
In-Reply-To: <20210423095147.27922-4-vivek.gautam@arm.com>
On Fri, Apr 23, 2021 at 03:21:39PM +0530, Vivek Gautam wrote:
> Redirect the incoming page faults to the registered fault handler
> that can take the fault information such as, pasid, page request
> group-id, address and pasid flags.
>
> Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
> ---
> drivers/iommu/virtio-iommu.c | 80 ++++++++++++++++++++++++++++++-
> include/uapi/linux/virtio_iommu.h | 1 +
> 2 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index c970f386f031..fd237cad1ce5 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -37,6 +37,13 @@
> /* Some architectures need an Address Space ID for each page table */
> DEFINE_XARRAY_ALLOC1(viommu_asid_xa);
>
> +struct viommu_dev_pri_work {
> + struct work_struct work;
> + struct viommu_dev *dev;
> + struct virtio_iommu_fault *vfault;
> + u32 endpoint;
> +};
> +
> struct viommu_dev {
> struct iommu_device iommu;
> struct device *dev;
> @@ -49,6 +56,8 @@ struct viommu_dev {
> struct list_head requests;
> void *evts;
> struct list_head endpoints;
> + struct workqueue_struct *pri_wq;
> + struct viommu_dev_pri_work *pri_work;
IOPF already has a workqueue, so the driver doesn't need one.
iommu_report_device_fault() should be fast enough to be called from the
event handler.
>
> /* Device configuration */
> struct iommu_domain_geometry geometry;
> @@ -666,6 +675,58 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
> return ret;
> }
>
> +static void viommu_handle_ppr(struct work_struct *work)
> +{
> + struct viommu_dev_pri_work *pwork =
> + container_of(work, struct viommu_dev_pri_work, work);
> + struct viommu_dev *viommu = pwork->dev;
> + struct virtio_iommu_fault *vfault = pwork->vfault;
> + struct viommu_endpoint *vdev;
> + struct viommu_ep_entry *ep;
> + struct iommu_fault_event fault_evt = {
> + .fault.type = IOMMU_FAULT_PAGE_REQ,
> + };
> + struct iommu_fault_page_request *prq = &fault_evt.fault.prm;
> +
> + u32 flags = le32_to_cpu(vfault->flags);
> + u32 prq_flags = le32_to_cpu(vfault->pr_evt_flags);
> + u32 endpoint = pwork->endpoint;
> +
> + memset(prq, 0, sizeof(struct iommu_fault_page_request));
The fault_evt struct is already initialized
> + prq->addr = le64_to_cpu(vfault->address);
> +
> + if (prq_flags & VIRTIO_IOMMU_FAULT_PRQ_F_LAST_PAGE)
> + prq->flags |= IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
> + if (prq_flags & VIRTIO_IOMMU_FAULT_PRQ_F_PASID_VALID) {
> + prq->flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> + prq->pasid = le32_to_cpu(vfault->pasid);
> + prq->grpid = le32_to_cpu(vfault->grpid);
> + }
> +
> + if (flags & VIRTIO_IOMMU_FAULT_F_READ)
> + prq->perm |= IOMMU_FAULT_PERM_READ;
> + if (flags & VIRTIO_IOMMU_FAULT_F_WRITE)
> + prq->perm |= IOMMU_FAULT_PERM_WRITE;
> + if (flags & VIRTIO_IOMMU_FAULT_F_EXEC)
> + prq->perm |= IOMMU_FAULT_PERM_EXEC;
> + if (flags & VIRTIO_IOMMU_FAULT_F_PRIV)
> + prq->perm |= IOMMU_FAULT_PERM_PRIV;
> +
> + list_for_each_entry(ep, &viommu->endpoints, list) {
> + if (ep->eid == endpoint) {
> + vdev = ep->vdev;
> + break;
> + }
> + }
> +
> + if ((prq_flags & VIRTIO_IOMMU_FAULT_PRQ_F_PASID_VALID) &&
> + (prq_flags & VIRTIO_IOMMU_FAULT_PRQ_F_NEEDS_PASID))
> + prq->flags |= IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
> +
> + if (iommu_report_device_fault(vdev->dev, &fault_evt))
> + dev_err(vdev->dev, "Couldn't handle page request\n");
An error likely means that nobody registered a fault handler, but we could
display a few more details about the fault that would help debug the
endpoint
> +}
> +
> static int viommu_fault_handler(struct viommu_dev *viommu,
> struct virtio_iommu_fault *fault)
> {
> @@ -679,7 +740,13 @@ static int viommu_fault_handler(struct viommu_dev *viommu,
> u32 pasid = le32_to_cpu(fault->pasid);
>
> if (type == VIRTIO_IOMMU_FAULT_F_PAGE_REQ) {
> - dev_info(viommu->dev, "Page request fault - unhandled\n");
> + dev_info_ratelimited(viommu->dev,
> + "Page request fault from EP %u\n",
> + endpoint);
That's rather for debugging the virtio-iommu driver, so should be
dev_dbg() (or removed entirely)
> +
> + viommu->pri_work->vfault = fault;
> + viommu->pri_work->endpoint = endpoint;
> + queue_work(viommu->pri_wq, &viommu->pri_work->work);
> return 0;
> }
>
> @@ -1683,6 +1750,17 @@ static int viommu_probe(struct virtio_device *vdev)
> goto err_free_vqs;
> }
>
> + viommu->pri_work = kzalloc(sizeof(*viommu->pri_work), GFP_KERNEL);
> + if (!viommu->pri_work)
> + return -ENOMEM;
> +
> + viommu->pri_work->dev = viommu;
> +
> + INIT_WORK(&viommu->pri_work->work, viommu_handle_ppr);
> + viommu->pri_wq = create_singlethread_workqueue("viommu-pri-wq");
> + if (!viommu->pri_wq)
> + return -ENOMEM;
> +
> viommu->map_flags = VIRTIO_IOMMU_MAP_F_READ | VIRTIO_IOMMU_MAP_F_WRITE;
> viommu->last_domain = ~0U;
>
> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
> index accc3318ce46..53aa88e6b077 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -302,6 +302,7 @@ struct virtio_iommu_req_invalidate {
> #define VIRTIO_IOMMU_FAULT_F_READ (1 << 0)
> #define VIRTIO_IOMMU_FAULT_F_WRITE (1 << 1)
> #define VIRTIO_IOMMU_FAULT_F_EXEC (1 << 2)
> +#define VIRTIO_IOMMU_FAULT_F_PRIV (1 << 3)
Should go in the previous patch. (I'd also prefer 'privileged' because in
this context 'priv' is easily read as 'private')
Thanks,
Jean
> #define VIRTIO_IOMMU_FAULT_F_ADDRESS (1 << 8)
>
> #define VIRTIO_IOMMU_FAULT_F_DMA_UNRECOV 1
> --
> 2.17.1
>
next prev parent reply other threads:[~2021-09-21 16:03 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-23 9:51 [PATCH RFC v1 00/11] iommu/virtio: vSVA support with Arm Vivek Gautam
2021-04-23 9:51 ` [PATCH RFC v1 01/11] uapi/virtio-iommu: Add page request grp-id and flags information Vivek Gautam
2021-09-21 15:58 ` Jean-Philippe Brucker
2021-09-30 4:56 ` Vivek Kumar Gautam
2021-09-30 8:49 ` Jean-Philippe Brucker
2021-09-30 10:57 ` Vivek Kumar Gautam
2021-04-23 9:51 ` [PATCH RFC v1 02/11] iommu/virtio: Maintain a list of endpoints served by viommu_dev Vivek Gautam
2021-09-21 15:59 ` Jean-Philippe Brucker
2021-09-30 9:17 ` Vivek Kumar Gautam
2021-04-23 9:51 ` [PATCH RFC v1 03/11] iommu/virtio: Handle incoming page faults Vivek Gautam
2021-09-21 16:03 ` Jean-Philippe Brucker [this message]
2021-10-11 8:11 ` Vivek Gautam
2021-10-11 9:16 ` Jean-Philippe Brucker
2021-10-11 9:20 ` Vivek Kumar Gautam
2021-04-23 9:51 ` [PATCH RFC v1 04/11] iommu/virtio: Add a io page fault queue Vivek Gautam
2021-04-23 9:51 ` [PATCH RFC v1 05/11] iommu/virtio: Add SVA feature and related enable/disable callbacks Vivek Gautam
2021-09-21 16:04 ` Jean-Philippe Brucker
2021-04-23 9:51 ` [PATCH RFC v1 06/11] iommu/pasid-table: Add pasid table ops for shared context management Vivek Gautam
2021-04-23 9:51 ` [PATCH RFC v1 07/11] iommu/arm-smmu-v3: Move shared context descriptor code to cd-lib Vivek Gautam
2021-04-23 9:51 ` [PATCH RFC v1 08/11] iommu/arm-smmu-v3: Implement shared context alloc and free ops Vivek Gautam
2021-09-21 16:07 ` Jean-Philippe Brucker
2021-09-30 9:50 ` Vivek Kumar Gautam
2021-04-23 9:51 ` [PATCH RFC v1 09/11] iommu/virtio: Implement sva bind/unbind calls Vivek Gautam
2021-09-21 16:09 ` Jean-Philippe Brucker
2021-04-23 9:51 ` [PATCH RFC v1 10/11] uapi/virtio-iommu: Add a new request type to send page response Vivek Gautam
2021-09-21 16:16 ` Jean-Philippe Brucker
2021-09-30 9:24 ` Vivek Kumar Gautam
2021-10-06 9:55 ` Jean-Philippe Brucker
2021-04-23 9:51 ` [PATCH RFC v1 11/11] iommu/virtio: Add support " Vivek Gautam
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=YUoCTV6WYxxE10qj@myrica \
--to=jean-philippe@linaro.org \
--cc=Lorenzo.Pieralisi@arm.com \
--cc=eric.auger@redhat.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@linux.intel.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=robin.murphy@arm.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=vivek.gautam@arm.com \
--cc=will.deacon@arm.com \
--cc=yi.l.liu@intel.com \
/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).