Messages in this thread | | | Date | Tue, 29 May 2018 09:20:58 -0700 | From | Jacob Pan <> | Subject | Re: [PATCH v5 15/23] iommu: handle page response timeout |
| |
On Mon, 14 May 2018 15:43:54 +0800 Lu Baolu <baolu.lu@linux.intel.com> wrote:
> Hi, > > On 05/12/2018 04:54 AM, Jacob Pan wrote: > > When IO page faults are reported outside IOMMU subsystem, the page > > request handler may fail for various reasons. E.g. a guest received > > page requests but did not have a chance to run for a long time. The > > irresponsive behavior could hold off limited resources on the > > pending device. > > There can be hardware or credit based software solutions as > > suggested in the PCI ATS Ch-4. To provide a basic safty net this > > patch introduces a per device deferrable timer which monitors the > > longest pending page fault that requires a response. Proper action > > such as sending failure response code could be taken when timer > > expires but not included in this patch. We need to consider the > > life cycle of page groupd ID to prevent confusion with reused group > > ID by a device. For now, a warning message provides clue of such > > failure. > > > > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > > Signed-off-by: Ashok Raj <ashok.raj@intel.com> > > --- > > drivers/iommu/iommu.c | 53 > > +++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/iommu.h | 4 ++++ 2 files changed, 57 insertions(+) > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index 02fed3e..1f2f49e 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -827,6 +827,37 @@ int iommu_group_unregister_notifier(struct > > iommu_group *group, } > > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); > > > > +static void iommu_dev_fault_timer_fn(struct timer_list *t) > > +{ > > + struct iommu_fault_param *fparam = from_timer(fparam, t, > > timer); > > + struct iommu_fault_event *evt; > > + > > + u64 now; > > + > > + now = get_jiffies_64(); > > + > > + /* The goal is to ensure driver or guest page fault > > handler(via vfio) > > + * send page response on time. Otherwise, limited queue > > resources > > + * may be occupied by some irresponsive guests or drivers. > > + * When per device pending fault list is not empty, we > > periodically checks > > + * if any anticipated page response time has expired. > > + * > > + * TODO: > > + * We could do the following if response time expires: > > + * 1. send page response code FAILURE to all pending PRQ > > + * 2. inform device driver or vfio > > + * 3. drain in-flight page requests and responses for this > > device > > + * 4. clear pending fault list such that driver can > > unregister fault > > + * handler(otherwise blocked when pending faults are > > present). > > + */ > > + list_for_each_entry(evt, &fparam->faults, list) { > > + if (time_after64(now, evt->expire)) > > + pr_err("Page response time expired!, pasid > > %d gid %d exp %llu now %llu\n", > > + evt->pasid, > > evt->page_req_group_id, evt->expire, now); > > + } > > + mod_timer(t, now + prq_timeout); > > +} > > + > > This timer scheme is very rough. > yes, the timer is a rough safety net for misbehaved PRQ handlers such as a guest. > The timer expires every 10 seconds (by default). > > 0 10 20 > 30 40 > +---------------+---------------+---------------+---------------+ ^ > ^ ^ ^ ^ | | | > | | F0 F1 F2 F3 > (F1,F2,F3 will not be handled until here!) > > F0, F1, F2, F3 are four page faults happens during [0, 10s) time > window. F1, F2, F3 timeout won't be handled until the timer expires > again at 20s. That means a fault might be pending there until about > (2 * prq_timeout) seconds later. > correct. it could be 2x for the worst case. I should explain in comments. > Out of curiosity, Why not adding a timer in iommu_fault_event, > starting it in iommu_report_device_fault() and removing it in > iommu_page_response()? > I thought about that also but since we are just trying to have a broad and rough safety net (in addition to potential HW mechanism or credit based solution), my thought was that having a per device timer is more economical than per event. Thanks for the in-depth check!
> Best regards, > Lu Baolu > > > [...] >
[Jacob Pan]
| |