From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> To: Jacob Pan <jacob.jun.pan@linux.intel.com> Cc: "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "joro@8bytes.org" <joro@8bytes.org>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>, "jcrouse@codeaurora.org" <jcrouse@codeaurora.org>, "alex.williamson@redhat.com" <alex.williamson@redhat.com>, "Jonathan.Cameron@huawei.com" <Jonathan.Cameron@huawei.com>, "christian.koenig@amd.com" <christian.koenig@amd.com>, "eric.auger@redhat.com" <eric.auger@redhat.com>, "kevin.tian@intel.com" <kevin.tian@intel.com>, "yi.l.liu@intel.com" <yi.l.liu@intel.com>, Andrew Murray <Andrew.Murray@arm.com>, Will Deacon <Will.Deacon@arm.com>, Robin Murphy <Robin.Murphy@arm.com>, "ashok.raj@intel.com" <ashok.raj@intel.com>, "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>, "xuzaibo@huawei.com" <xuzaibo@huawei.com>, "liguozhu@hisilicon.com" <liguozhu@hisilicon.com>, "okaya@codeaurora.org" <okaya@codeaurora.org>, "bharatku@xilinx.com" <bharatku@xilinx.com>, "ilias.apalodimas@linaro.org" <ilias.apalodimas@linaro.org>, "shunyong.yang@hxt-semitech.com" <shunyong.yang@hxt-semitech.com> Subject: Re: [PATCH v3 07/10] iommu: Add a page fault handler Date: Wed, 3 Oct 2018 18:46:11 +0100 [thread overview] Message-ID: <f873cc65-ec0c-9273-eb14-9e07f13d3206@arm.com> (raw) In-Reply-To: <20180927133746.319e19b8@jacob-builder> On 27/09/2018 21:37, Jacob Pan wrote: > On Thu, 20 Sep 2018 18:00:43 +0100 > Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote: > >> + /* >> + * When removing a PASID, the device driver tells the device >> to stop >> + * using it, and flush any pending fault to the IOMMU. In >> this flush >> + * callback, the IOMMU driver makes sure that there are no >> such faults >> + * left in the low-level queue. >> + */ >> + queue->flush(queue->flush_arg, dev, pasid); >> + >> + /* >> + * If at some point the low-level fault queue overflowed and >> the IOMMU >> + * device had to auto-respond to a 'last' page fault, other >> faults from >> + * the same Page Request Group may still be stuck in the >> partial list. >> + * We need to make sure that the next address space using >> the PASID >> + * doesn't receive them. >> + */ > Trying to understand the intended use case under queue full condition. > 1 model specific iommu driver register a flush callback to handle > internal PRQ drain > > 2 IOMMU HW detects queue full and auto respond with 'SUCCESS' code to > all device and PASID, raise interrupt > > 3 model specific iommu driver detects queue full and call > iopf_queue_flush_dev() I didn't intent for iopf_queue_flush_dev to be called by the IOMMU driver in this situation, at the moment it's only intended for the SVA code to clean up before removing a PASID (in which case we have to wipe partial faults). This version doesn't provide anything to the IOMMU driver for handling overflow condition cleanly, partial faults are kept until the PASID is unbound or SVA is disabled. > 4 call queue->flush() callback to drain PRQ in-flight inside IOMMU HW Could we avoid this step in this scenario? If it's the PRI IRQ thread that detects queue full in step 3, then it could drain the HW queue before calling iopf_flush_partial() (or something like that). I'm a bit worried about possible locking problems if we go back to the IOMMU driver here while it is calling us. > 5.Shoot down partial list for all PASIDs > > If the above understanding is correct, don't we need to shoot down all > partial groups? instead of just one PASID. At least for VT-d, we need > to do that. Passing IOMMU_PASID_INVALID will do that. But It also needs to be done for all devices that use this IOPF queue, and we don't need to flush the workqueue, so iopf_queue_flush_dev isn't really adapted. Thanks, Jean > > >> + mutex_lock(¶m->lock); >> + list_for_each_entry_safe(fault, next, >> ¶m->iopf_param->partial, head) { >> + if (fault->evt.pasid == pasid || pasid == >> IOMMU_PASID_INVALID) { >> + list_del(&fault->head); >> + kfree(fault); >> + } >> + } >> + mutex_unlock(¶m->lock); >> + >> + flush_workqueue(queue->wq); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(iopf_queue_flush_dev); >> + > [Jacob Pan] >
WARNING: multiple messages have this Message-ID (diff)
From: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> To: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> Cc: "kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org" <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, "ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org" <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, "linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" <ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Will Deacon <Will.Deacon-5wv7dgnIgG8@public.gmane.org>, "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, "okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, "liguozhu-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org" <liguozhu-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>, "christian.koenig-5C7GfCeVMHo@public.gmane.org" <christian.koenig-5C7GfCeVMHo@public.gmane.org>, Robin Murphy <Robin.Murphy-5wv7dgnIgG8@public.gmane.org> Subject: Re: [PATCH v3 07/10] iommu: Add a page fault handler Date: Wed, 3 Oct 2018 18:46:11 +0100 [thread overview] Message-ID: <f873cc65-ec0c-9273-eb14-9e07f13d3206@arm.com> (raw) In-Reply-To: <20180927133746.319e19b8@jacob-builder> On 27/09/2018 21:37, Jacob Pan wrote: > On Thu, 20 Sep 2018 18:00:43 +0100 > Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote: > >> + /* >> + * When removing a PASID, the device driver tells the device >> to stop >> + * using it, and flush any pending fault to the IOMMU. In >> this flush >> + * callback, the IOMMU driver makes sure that there are no >> such faults >> + * left in the low-level queue. >> + */ >> + queue->flush(queue->flush_arg, dev, pasid); >> + >> + /* >> + * If at some point the low-level fault queue overflowed and >> the IOMMU >> + * device had to auto-respond to a 'last' page fault, other >> faults from >> + * the same Page Request Group may still be stuck in the >> partial list. >> + * We need to make sure that the next address space using >> the PASID >> + * doesn't receive them. >> + */ > Trying to understand the intended use case under queue full condition. > 1 model specific iommu driver register a flush callback to handle > internal PRQ drain > > 2 IOMMU HW detects queue full and auto respond with 'SUCCESS' code to > all device and PASID, raise interrupt > > 3 model specific iommu driver detects queue full and call > iopf_queue_flush_dev() I didn't intent for iopf_queue_flush_dev to be called by the IOMMU driver in this situation, at the moment it's only intended for the SVA code to clean up before removing a PASID (in which case we have to wipe partial faults). This version doesn't provide anything to the IOMMU driver for handling overflow condition cleanly, partial faults are kept until the PASID is unbound or SVA is disabled. > 4 call queue->flush() callback to drain PRQ in-flight inside IOMMU HW Could we avoid this step in this scenario? If it's the PRI IRQ thread that detects queue full in step 3, then it could drain the HW queue before calling iopf_flush_partial() (or something like that). I'm a bit worried about possible locking problems if we go back to the IOMMU driver here while it is calling us. > 5.Shoot down partial list for all PASIDs > > If the above understanding is correct, don't we need to shoot down all > partial groups? instead of just one PASID. At least for VT-d, we need > to do that. Passing IOMMU_PASID_INVALID will do that. But It also needs to be done for all devices that use this IOPF queue, and we don't need to flush the workqueue, so iopf_queue_flush_dev isn't really adapted. Thanks, Jean > > >> + mutex_lock(¶m->lock); >> + list_for_each_entry_safe(fault, next, >> ¶m->iopf_param->partial, head) { >> + if (fault->evt.pasid == pasid || pasid == >> IOMMU_PASID_INVALID) { >> + list_del(&fault->head); >> + kfree(fault); >> + } >> + } >> + mutex_unlock(¶m->lock); >> + >> + flush_workqueue(queue->wq); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(iopf_queue_flush_dev); >> + > [Jacob Pan] >
next prev parent reply other threads:[~2018-10-03 17:46 UTC|newest] Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-20 17:00 [PATCH v3 00/10] Shared Virtual Addressing for the IOMMU Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [PATCH v3 01/10] iommu: Introduce Shared Virtual Addressing API Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker [not found] ` <20180920170046.20154-2-jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> 2018-09-23 2:39 ` Lu Baolu 2018-09-24 12:07 ` Jean-Philippe Brucker 2018-09-24 12:07 ` Jean-Philippe Brucker 2018-09-25 13:16 ` Joerg Roedel 2018-09-25 13:16 ` Joerg Roedel 2018-09-25 22:46 ` Jacob Pan 2018-09-25 22:46 ` Jacob Pan 2018-09-26 10:14 ` Jean-Philippe Brucker 2018-09-26 10:14 ` Jean-Philippe Brucker 2018-09-26 12:48 ` Joerg Roedel 2018-09-26 12:48 ` Joerg Roedel 2018-09-20 17:00 ` [PATCH v3 02/10] iommu/sva: Bind process address spaces to devices Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-23 3:05 ` Lu Baolu 2018-09-23 3:05 ` Lu Baolu 2018-09-24 12:07 ` Jean-Philippe Brucker 2018-09-24 12:07 ` Jean-Philippe Brucker 2018-09-26 18:01 ` Jacob Pan 2018-09-26 18:01 ` Jacob Pan 2018-09-27 15:06 ` Jean-Philippe Brucker 2018-09-27 15:06 ` Jean-Philippe Brucker 2018-09-28 1:14 ` Tian, Kevin 2018-09-28 1:14 ` Tian, Kevin 2018-09-20 17:00 ` [PATCH v3 03/10] iommu/sva: Manage process address spaces Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-25 3:15 ` Lu Baolu 2018-09-25 3:15 ` Lu Baolu 2018-09-25 10:32 ` Jean-Philippe Brucker 2018-09-25 10:32 ` Jean-Philippe Brucker 2018-09-26 3:12 ` Lu Baolu 2018-09-26 3:12 ` Lu Baolu 2018-09-25 13:26 ` Joerg Roedel 2018-09-25 13:26 ` Joerg Roedel 2018-09-25 23:33 ` Lu Baolu 2018-09-25 23:33 ` Lu Baolu 2018-09-26 10:20 ` Jean-Philippe Brucker 2018-09-26 10:20 ` Jean-Philippe Brucker 2018-09-26 12:45 ` Joerg Roedel 2018-09-26 12:45 ` Joerg Roedel 2018-09-26 13:50 ` Jean-Philippe Brucker 2018-09-26 13:50 ` Jean-Philippe Brucker 2018-09-27 3:22 ` Liu, Yi L 2018-09-27 3:22 ` Liu, Yi L 2018-09-27 13:37 ` Jean-Philippe Brucker 2018-09-27 13:37 ` Jean-Philippe Brucker 2018-10-08 8:29 ` Liu, Yi L 2018-10-08 8:29 ` Liu, Yi L 2018-09-26 22:58 ` Jacob Pan 2018-09-26 22:58 ` Jacob Pan 2018-09-26 22:35 ` Jacob Pan 2018-09-26 22:35 ` Jacob Pan 2018-10-03 17:52 ` Jean-Philippe Brucker 2018-10-03 17:52 ` Jean-Philippe Brucker 2018-10-15 20:53 ` Jacob Pan 2018-10-15 20:53 ` Jacob Pan 2018-09-20 17:00 ` [PATCH v3 04/10] iommu/sva: Add a mm_exit callback for device drivers Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [PATCH v3 05/10] iommu/sva: Track mm changes with an MMU notifier Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [PATCH v3 06/10] iommu/sva: Search mm by PASID Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-25 4:59 ` Lu Baolu 2018-09-25 4:59 ` Lu Baolu 2018-09-20 17:00 ` [PATCH v3 07/10] iommu: Add a page fault handler Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-27 20:37 ` Jacob Pan 2018-09-27 20:37 ` Jacob Pan 2018-10-03 17:46 ` Jean-Philippe Brucker [this message] 2018-10-03 17:46 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [PATCH v3 08/10] iommu/iopf: Handle mm faults Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [PATCH v3 09/10] iommu/sva: Register page fault handler Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-09-20 17:00 ` [RFC PATCH v3 10/10] iommu/sva: Add support for private PASIDs Jean-Philippe Brucker 2018-09-20 17:00 ` Jean-Philippe Brucker 2018-10-12 14:32 ` Jordan Crouse 2018-10-12 14:32 ` Jordan Crouse 2018-10-17 14:21 ` Jean-Philippe Brucker 2018-10-17 14:21 ` Jean-Philippe Brucker 2018-10-17 14:24 ` Jean-Philippe Brucker 2018-10-17 14:24 ` Jean-Philippe Brucker 2018-10-17 15:07 ` Jordan Crouse 2018-10-17 15:07 ` Jordan Crouse
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=f873cc65-ec0c-9273-eb14-9e07f13d3206@arm.com \ --to=jean-philippe.brucker@arm.com \ --cc=Andrew.Murray@arm.com \ --cc=Jonathan.Cameron@huawei.com \ --cc=Robin.Murphy@arm.com \ --cc=Will.Deacon@arm.com \ --cc=alex.williamson@redhat.com \ --cc=ashok.raj@intel.com \ --cc=baolu.lu@linux.intel.com \ --cc=bharatku@xilinx.com \ --cc=christian.koenig@amd.com \ --cc=eric.auger@redhat.com \ --cc=ilias.apalodimas@linaro.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@linux.intel.com \ --cc=jcrouse@codeaurora.org \ --cc=joro@8bytes.org \ --cc=kevin.tian@intel.com \ --cc=liguozhu@hisilicon.com \ --cc=linux-pci@vger.kernel.org \ --cc=okaya@codeaurora.org \ --cc=shunyong.yang@hxt-semitech.com \ --cc=xuzaibo@huawei.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.