From: Lu Baolu <baolu.lu@linux.intel.com> To: "Tian, Kevin" <kevin.tian@intel.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org> Cc: baolu.lu@linux.intel.com, Joerg Roedel <joro@8bytes.org>, Jacob Pan <jacob.jun.pan@linux.intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, "Liu, Yi L" <yi.l.liu@intel.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v3 4/4] iommu/vt-d: Add page response ops support Date: Fri, 10 Jul 2020 13:36:52 +0800 [thread overview] Message-ID: <cbf470fe-933a-54d5-e215-afb32d40165f@linux.intel.com> (raw) In-Reply-To: <MWHPR11MB164546581C5F6B6B77AE28C88C650@MWHPR11MB1645.namprd11.prod.outlook.com> Hi Kevin, On 2020/7/10 10:42, Tian, Kevin wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> Sent: Thursday, July 9, 2020 3:06 PM >> >> After page requests are handled, software must respond to the device >> which raised the page request with the result. This is done through >> the iommu ops.page_response if the request was reported to outside of >> vendor iommu driver through iommu_report_device_fault(). This adds the >> VT-d implementation of page_response ops. >> >> Co-developed-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Co-developed-by: Liu Yi L <yi.l.liu@intel.com> >> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> --- >> drivers/iommu/intel/iommu.c | 1 + >> drivers/iommu/intel/svm.c | 100 >> ++++++++++++++++++++++++++++++++++++ >> include/linux/intel-iommu.h | 3 ++ >> 3 files changed, 104 insertions(+) >> >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 4a6b6960fc32..98390a6d8113 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -6057,6 +6057,7 @@ const struct iommu_ops intel_iommu_ops = { >> .sva_bind = intel_svm_bind, >> .sva_unbind = intel_svm_unbind, >> .sva_get_pasid = intel_svm_get_pasid, >> + .page_response = intel_svm_page_response, >> #endif >> }; >> >> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c >> index d24e71bac8db..839d2af377b6 100644 >> --- a/drivers/iommu/intel/svm.c >> +++ b/drivers/iommu/intel/svm.c >> @@ -1082,3 +1082,103 @@ int intel_svm_get_pasid(struct iommu_sva *sva) >> >> return pasid; >> } >> + >> +int intel_svm_page_response(struct device *dev, >> + struct iommu_fault_event *evt, >> + struct iommu_page_response *msg) >> +{ >> + struct iommu_fault_page_request *prm; >> + struct intel_svm_dev *sdev = NULL; >> + struct intel_svm *svm = NULL; >> + struct intel_iommu *iommu; >> + bool private_present; >> + bool pasid_present; >> + bool last_page; >> + u8 bus, devfn; >> + int ret = 0; >> + u16 sid; >> + >> + if (!dev || !dev_is_pci(dev)) >> + return -ENODEV; >> + >> + iommu = device_to_iommu(dev, &bus, &devfn); >> + if (!iommu) >> + return -ENODEV; >> + >> + if (!msg || !evt) >> + return -EINVAL; >> + >> + mutex_lock(&pasid_mutex); >> + >> + prm = &evt->fault.prm; >> + sid = PCI_DEVID(bus, devfn); >> + pasid_present = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; >> + private_present = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA; >> + last_page = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE; >> + >> + if (pasid_present) { >> + if (prm->pasid == 0 || prm->pasid >= PASID_MAX) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + ret = pasid_to_svm_sdev(dev, prm->pasid, &svm, &sdev); >> + if (ret || !sdev) { >> + ret = -ENODEV; >> + goto out; >> + } >> + >> + /* >> + * For responses from userspace, need to make sure that the >> + * pasid has been bound to its mm. >> + */ >> + if (svm->flags & SVM_FLAG_GUEST_MODE) { >> + struct mm_struct *mm; >> + >> + mm = get_task_mm(current); >> + if (!mm) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + if (mm != svm->mm) { >> + ret = -ENODEV; >> + mmput(mm); >> + goto out; >> + } >> + >> + mmput(mm); >> + } >> + } else { >> + pr_err_ratelimited("Invalid page response: no pasid\n"); >> + ret = -EINVAL; >> + goto out; > > check pasid=0 first, then no need to indent so many lines above. Yes. > >> + } >> + >> + /* >> + * Per VT-d spec. v3.0 ch7.7, system software must respond >> + * with page group response if private data is present (PDP) >> + * or last page in group (LPIG) bit is set. This is an >> + * additional VT-d requirement beyond PCI ATS spec. >> + */ > > What is the behavior if system software doesn't follow the requirement? > en... maybe the question is really about whether the information in prm > comes from userspace or from internally-recorded info in iommu core. > The former cannot be trusted. The latter one is OK. We require a page response when reporting such event. The upper layer (IOMMU core or VFIO) will be implemented with a timer, if userspace doesn't respond in time, the timer will get expired and a FAILURE response will be sent to device. Best regards, baolu > > Thanks > Kevin > >> + if (last_page || private_present) { >> + struct qi_desc desc; >> + >> + desc.qw0 = QI_PGRP_PASID(prm->pasid) | QI_PGRP_DID(sid) >> | >> + QI_PGRP_PASID_P(pasid_present) | >> + QI_PGRP_PDP(private_present) | >> + QI_PGRP_RESP_CODE(msg->code) | >> + QI_PGRP_RESP_TYPE; >> + desc.qw1 = QI_PGRP_IDX(prm->grpid) | >> QI_PGRP_LPIG(last_page); >> + desc.qw2 = 0; >> + desc.qw3 = 0; >> + if (private_present) >> + memcpy(&desc.qw2, prm->private_data, >> + sizeof(prm->private_data)); >> + >> + qi_submit_sync(iommu, &desc, 1, 0); >> + } >> +out: >> + mutex_unlock(&pasid_mutex); >> + return ret; >> +} >> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h >> index fc2cfc3db6e1..bf6009a344f5 100644 >> --- a/include/linux/intel-iommu.h >> +++ b/include/linux/intel-iommu.h >> @@ -741,6 +741,9 @@ struct iommu_sva *intel_svm_bind(struct device >> *dev, struct mm_struct *mm, >> void *drvdata); >> void intel_svm_unbind(struct iommu_sva *handle); >> int intel_svm_get_pasid(struct iommu_sva *handle); >> +int intel_svm_page_response(struct device *dev, struct iommu_fault_event >> *evt, >> + struct iommu_page_response *msg); >> + >> struct svm_dev_ops; >> >> struct intel_svm_dev { >> -- >> 2.17.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Lu Baolu <baolu.lu@linux.intel.com> To: "Tian, Kevin" <kevin.tian@intel.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org> Cc: "Raj, Ashok" <ashok.raj@intel.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v3 4/4] iommu/vt-d: Add page response ops support Date: Fri, 10 Jul 2020 13:36:52 +0800 [thread overview] Message-ID: <cbf470fe-933a-54d5-e215-afb32d40165f@linux.intel.com> (raw) In-Reply-To: <MWHPR11MB164546581C5F6B6B77AE28C88C650@MWHPR11MB1645.namprd11.prod.outlook.com> Hi Kevin, On 2020/7/10 10:42, Tian, Kevin wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> Sent: Thursday, July 9, 2020 3:06 PM >> >> After page requests are handled, software must respond to the device >> which raised the page request with the result. This is done through >> the iommu ops.page_response if the request was reported to outside of >> vendor iommu driver through iommu_report_device_fault(). This adds the >> VT-d implementation of page_response ops. >> >> Co-developed-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Co-developed-by: Liu Yi L <yi.l.liu@intel.com> >> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> --- >> drivers/iommu/intel/iommu.c | 1 + >> drivers/iommu/intel/svm.c | 100 >> ++++++++++++++++++++++++++++++++++++ >> include/linux/intel-iommu.h | 3 ++ >> 3 files changed, 104 insertions(+) >> >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 4a6b6960fc32..98390a6d8113 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -6057,6 +6057,7 @@ const struct iommu_ops intel_iommu_ops = { >> .sva_bind = intel_svm_bind, >> .sva_unbind = intel_svm_unbind, >> .sva_get_pasid = intel_svm_get_pasid, >> + .page_response = intel_svm_page_response, >> #endif >> }; >> >> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c >> index d24e71bac8db..839d2af377b6 100644 >> --- a/drivers/iommu/intel/svm.c >> +++ b/drivers/iommu/intel/svm.c >> @@ -1082,3 +1082,103 @@ int intel_svm_get_pasid(struct iommu_sva *sva) >> >> return pasid; >> } >> + >> +int intel_svm_page_response(struct device *dev, >> + struct iommu_fault_event *evt, >> + struct iommu_page_response *msg) >> +{ >> + struct iommu_fault_page_request *prm; >> + struct intel_svm_dev *sdev = NULL; >> + struct intel_svm *svm = NULL; >> + struct intel_iommu *iommu; >> + bool private_present; >> + bool pasid_present; >> + bool last_page; >> + u8 bus, devfn; >> + int ret = 0; >> + u16 sid; >> + >> + if (!dev || !dev_is_pci(dev)) >> + return -ENODEV; >> + >> + iommu = device_to_iommu(dev, &bus, &devfn); >> + if (!iommu) >> + return -ENODEV; >> + >> + if (!msg || !evt) >> + return -EINVAL; >> + >> + mutex_lock(&pasid_mutex); >> + >> + prm = &evt->fault.prm; >> + sid = PCI_DEVID(bus, devfn); >> + pasid_present = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; >> + private_present = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA; >> + last_page = prm->flags & >> IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE; >> + >> + if (pasid_present) { >> + if (prm->pasid == 0 || prm->pasid >= PASID_MAX) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + ret = pasid_to_svm_sdev(dev, prm->pasid, &svm, &sdev); >> + if (ret || !sdev) { >> + ret = -ENODEV; >> + goto out; >> + } >> + >> + /* >> + * For responses from userspace, need to make sure that the >> + * pasid has been bound to its mm. >> + */ >> + if (svm->flags & SVM_FLAG_GUEST_MODE) { >> + struct mm_struct *mm; >> + >> + mm = get_task_mm(current); >> + if (!mm) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + if (mm != svm->mm) { >> + ret = -ENODEV; >> + mmput(mm); >> + goto out; >> + } >> + >> + mmput(mm); >> + } >> + } else { >> + pr_err_ratelimited("Invalid page response: no pasid\n"); >> + ret = -EINVAL; >> + goto out; > > check pasid=0 first, then no need to indent so many lines above. Yes. > >> + } >> + >> + /* >> + * Per VT-d spec. v3.0 ch7.7, system software must respond >> + * with page group response if private data is present (PDP) >> + * or last page in group (LPIG) bit is set. This is an >> + * additional VT-d requirement beyond PCI ATS spec. >> + */ > > What is the behavior if system software doesn't follow the requirement? > en... maybe the question is really about whether the information in prm > comes from userspace or from internally-recorded info in iommu core. > The former cannot be trusted. The latter one is OK. We require a page response when reporting such event. The upper layer (IOMMU core or VFIO) will be implemented with a timer, if userspace doesn't respond in time, the timer will get expired and a FAILURE response will be sent to device. Best regards, baolu > > Thanks > Kevin > >> + if (last_page || private_present) { >> + struct qi_desc desc; >> + >> + desc.qw0 = QI_PGRP_PASID(prm->pasid) | QI_PGRP_DID(sid) >> | >> + QI_PGRP_PASID_P(pasid_present) | >> + QI_PGRP_PDP(private_present) | >> + QI_PGRP_RESP_CODE(msg->code) | >> + QI_PGRP_RESP_TYPE; >> + desc.qw1 = QI_PGRP_IDX(prm->grpid) | >> QI_PGRP_LPIG(last_page); >> + desc.qw2 = 0; >> + desc.qw3 = 0; >> + if (private_present) >> + memcpy(&desc.qw2, prm->private_data, >> + sizeof(prm->private_data)); >> + >> + qi_submit_sync(iommu, &desc, 1, 0); >> + } >> +out: >> + mutex_unlock(&pasid_mutex); >> + return ret; >> +} >> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h >> index fc2cfc3db6e1..bf6009a344f5 100644 >> --- a/include/linux/intel-iommu.h >> +++ b/include/linux/intel-iommu.h >> @@ -741,6 +741,9 @@ struct iommu_sva *intel_svm_bind(struct device >> *dev, struct mm_struct *mm, >> void *drvdata); >> void intel_svm_unbind(struct iommu_sva *handle); >> int intel_svm_get_pasid(struct iommu_sva *handle); >> +int intel_svm_page_response(struct device *dev, struct iommu_fault_event >> *evt, >> + struct iommu_page_response *msg); >> + >> struct svm_dev_ops; >> >> struct intel_svm_dev { >> -- >> 2.17.1 > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-07-10 5:37 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-09 7:05 [PATCH v3 0/4] iommu/vt-d: Add prq report and response support Lu Baolu 2020-07-09 7:05 ` Lu Baolu 2020-07-09 7:05 ` [PATCH v3 1/4] iommu/vt-d: Refactor device_to_iommu() helper Lu Baolu 2020-07-09 7:05 ` Lu Baolu 2020-07-09 7:05 ` [PATCH v3 2/4] iommu/vt-d: Add a helper to get svm and sdev for pasid Lu Baolu 2020-07-09 7:05 ` Lu Baolu 2020-07-09 7:05 ` [PATCH v3 3/4] iommu/vt-d: Report page request faults for guest SVA Lu Baolu 2020-07-09 7:05 ` Lu Baolu 2020-07-10 2:24 ` Tian, Kevin 2020-07-10 2:24 ` Tian, Kevin 2020-07-10 5:25 ` Lu Baolu 2020-07-10 5:25 ` Lu Baolu 2020-07-09 7:05 ` [PATCH v3 4/4] iommu/vt-d: Add page response ops support Lu Baolu 2020-07-09 7:05 ` Lu Baolu 2020-07-10 2:42 ` Tian, Kevin 2020-07-10 2:42 ` Tian, Kevin 2020-07-10 5:36 ` Lu Baolu [this message] 2020-07-10 5:36 ` Lu Baolu 2020-07-10 5:49 ` Tian, Kevin 2020-07-10 5:49 ` Tian, Kevin 2020-07-10 8:17 ` Lu Baolu 2020-07-10 8:17 ` Lu Baolu
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=cbf470fe-933a-54d5-e215-afb32d40165f@linux.intel.com \ --to=baolu.lu@linux.intel.com \ --cc=ashok.raj@intel.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-kernel@vger.kernel.org \ --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.