From: FelixCui-oc <FelixCui-oc@zhaoxin.com>
To: Lu Baolu <baolu.lu@linux.intel.com>,
Joerg Roedel <joro@8bytes.org>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"David Woodhouse" <dwmw2@infradead.org>,
Dan Carpenter <dan.carpenter@oracle.com>,
"kbuild@lists.01.org" <kbuild@lists.01.org>
Cc: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com>,
CobeChen-oc <CobeChen-oc@zhaoxin.com>
Subject: 答复: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
Date: Wed, 2 Sep 2020 01:48:28 +0000 [thread overview]
Message-ID: <09f1d01c5066455a98dcb3dd22ff28d0@zhaoxin.com> (raw)
In-Reply-To: <1295ce42af404700ac89f177442bb27f@zhaoxin.com>
[-- Attachment #1.1: Type: text/plain, Size: 12963 bytes --]
hi baolu ,
Add pr_info(), there will be a problem.
dmar_acpi_bus_add_dev() will call dmar_acpi_insert_dev_scope() twice. So it will print two identical logs. This is similar to dmar_pci_bus_add_dev(). What do you think?
Add pr_warn(), No problem.
Best regards
Felixcui-oc
________________________________
发件人: FelixCui-oc
发送时间: 2020年9月1日 15:53:47
收件人: Lu Baolu; Joerg Roedel; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse; Dan Carpenter; kbuild@lists.01.org
抄送: CobeChen-oc; RaymondPang-oc; Tony W Wang-oc
主题: 答复: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
hi baolu,
The function dmar_rmrr_add_acpi_dev() is defined in intel/iommu.c because struct dmar_rmrr_unit {} is defined in intel/iommu.c.
And dmar_parse_one_rmrr() is also defined here, so we think it should be defined in iommu.c.
Best regards
Felixcui-oc
________________________________
发件人: Lu Baolu <baolu.lu@linux.intel.com>
发送时间: 2020年9月1日 14:05
收件人: FelixCui-oc; Joerg Roedel; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse; Dan Carpenter; kbuild@lists.01.org
抄送: baolu.lu@linux.intel.com; CobeChen-oc; RaymondPang-oc; Tony W Wang-oc
主题: Re: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
Hi Felix,
On 8/27/20 6:02 PM, FelixCuioc wrote:
> Some ACPI devices need to issue dma requests to access
> the reserved memory area.BIOS uses the device scope type
> ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
> This patch add support for detecting ACPI devices in RMRR.
>
> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
> ---
> drivers/iommu/intel/dmar.c | 76 +++++++++++++++++++++----------------
> drivers/iommu/intel/iommu.c | 23 ++++++++++-
> include/linux/dmar.h | 12 +++++-
> 3 files changed, 76 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 93e6345f3414..f6691c36bd3f 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -215,7 +215,7 @@ static bool dmar_match_pci_path(struct dmar_pci_notify_info *info, int bus,
> }
>
> /* Return: > 0 if match found, 0 if no match found, < 0 if error happens */
> -int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
> +int dmar_pci_insert_dev_scope(struct dmar_pci_notify_info *info,
> void *start, void*end, u16 segment,
> struct dmar_dev_scope *devices,
> int devices_cnt)
> @@ -304,7 +304,7 @@ static int dmar_pci_bus_add_dev(struct dmar_pci_notify_info *info)
>
> drhd = container_of(dmaru->hdr,
> struct acpi_dmar_hardware_unit, header);
> - ret = dmar_insert_dev_scope(info, (void *)(drhd + 1),
> + ret = dmar_pci_insert_dev_scope(info, (void *)(drhd + 1),
> ((void *)drhd) + drhd->header.length,
> dmaru->segment,
> dmaru->devices, dmaru->devices_cnt);
> @@ -697,47 +697,59 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
> return dmaru;
> }
>
> -static void __init dmar_acpi_insert_dev_scope(u8 device_number,
> - struct acpi_device *adev)
> +/* Return: > 0 if match found, 0 if no match found */
> +bool dmar_acpi_insert_dev_scope(u8 device_number,
> + struct acpi_device *adev,
> + void *start, void *end,
> + struct dmar_dev_scope *devices,
> + int devices_cnt)
> {
> - struct dmar_drhd_unit *dmaru;
> - struct acpi_dmar_hardware_unit *drhd;
> struct acpi_dmar_device_scope *scope;
> struct device *tmp;
> int i;
> struct acpi_dmar_pci_path *path;
>
> + for (; start < end; start += scope->length) {
> + scope = start;
> + if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
> + continue;
> + if (scope->enumeration_id != device_number)
> + continue;
> + path = (void *)(scope + 1);
> + for_each_dev_scope(devices, devices_cnt, i, tmp)
> + if (tmp == NULL) {
> + devices[i].bus = scope->bus;
> + devices[i].devfn = PCI_DEVFN(path->device, path->function);
> + rcu_assign_pointer(devices[i].dev,
> + get_device(&adev->dev));
> + return true;
> + }
> + WARN_ON(i >= devices_cnt);
> + }
> + return false;
> +}
> +
> +static int dmar_acpi_bus_add_dev(u8 device_number, struct acpi_device *adev)
> +{
> + struct dmar_drhd_unit *dmaru;
> + struct acpi_dmar_hardware_unit *drhd;
> + int ret;
> +
> for_each_drhd_unit(dmaru) {
> drhd = container_of(dmaru->hdr,
> struct acpi_dmar_hardware_unit,
> header);
> + ret = dmar_acpi_insert_dev_scope(device_number, adev, (void *)(drhd+1),
> + ((void *)drhd)+drhd->header.length,
> + dmaru->devices, dmaru->devices_cnt);
> + if (ret)
> + break;
> + }
> + if (ret > 0)
> + ret = dmar_rmrr_add_acpi_dev(device_number, adev);
>
> - for (scope = (void *)(drhd + 1);
> - (unsigned long)scope < ((unsigned long)drhd) + drhd->header.length;
> - scope = ((void *)scope) + scope->length) {
> - if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
> - continue;
> - if (scope->enumeration_id != device_number)
> - continue;
> + return ret;
>
> - path = (void *)(scope + 1);
> - pr_info("ACPI device \"%s\" under DMAR at %llx as %02x:%02x.%d\n",
> - dev_name(&adev->dev), dmaru->reg_base_addr,
> - scope->bus, path->device, path->function);
Please keep such information. People are used to use them as a method to
know the hardware configuration.
> - for_each_dev_scope(dmaru->devices, dmaru->devices_cnt, i, tmp)
> - if (tmp == NULL) {
> - dmaru->devices[i].bus = scope->bus;
> - dmaru->devices[i].devfn = PCI_DEVFN(path->device,
> - path->function);
> - rcu_assign_pointer(dmaru->devices[i].dev,
> - get_device(&adev->dev));
> - return;
> - }
> - BUG_ON(i >= dmaru->devices_cnt);
> - }
> - }
> - pr_warn("No IOMMU scope found for ANDD enumeration ID %d (%s)\n",
> - device_number, dev_name(&adev->dev));
Ditto.
> }
>
> static int __init dmar_acpi_dev_scope_init(void)
> @@ -766,7 +778,7 @@ static int __init dmar_acpi_dev_scope_init(void)
> andd->device_name);
> continue;
> }
> - dmar_acpi_insert_dev_scope(andd->device_number, adev);
> + dmar_acpi_bus_add_dev(andd->device_number, adev);
> }
> }
> return 0;
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index ca557d351518..208a91605288 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4508,6 +4508,25 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
> return ret;
> }
>
> +int dmar_rmrr_add_acpi_dev(u8 device_number, struct acpi_device *adev)
> +{
> + int ret;
> + struct dmar_rmrr_unit *rmrru;
> + struct acpi_dmar_reserved_memory *rmrr;
> +
> + list_for_each_entry(rmrru, &dmar_rmrr_units, list) {
> + rmrr = container_of(rmrru->hdr,
> + struct acpi_dmar_reserved_memory,
> + header);
> + ret = dmar_acpi_insert_dev_scope(device_number, adev, (void *)(rmrr + 1),
> + ((void *)rmrr) + rmrr->header.length,
> + rmrru->devices, rmrru->devices_cnt);
> + if (ret)
> + break;
> + }
> + return 0;
> +}
This is only used in dmar.c. Why do you want to define it here and use
it in another file?
> +
> int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
> {
> int ret;
> @@ -4523,7 +4542,7 @@ int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
> rmrr = container_of(rmrru->hdr,
> struct acpi_dmar_reserved_memory, header);
> if (info->event == BUS_NOTIFY_ADD_DEVICE) {
> - ret = dmar_insert_dev_scope(info, (void *)(rmrr + 1),
> + ret = dmar_pci_insert_dev_scope(info, (void *)(rmrr + 1),
> ((void *)rmrr) + rmrr->header.length,
> rmrr->segment, rmrru->devices,
> rmrru->devices_cnt);
> @@ -4541,7 +4560,7 @@ int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
>
> atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
> if (info->event == BUS_NOTIFY_ADD_DEVICE) {
> - ret = dmar_insert_dev_scope(info, (void *)(atsr + 1),
> + ret = dmar_pci_insert_dev_scope(info, (void *)(atsr + 1),
> (void *)atsr + atsr->header.length,
> atsr->segment, atsru->devices,
> atsru->devices_cnt);
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 65565820328a..d0981d35d3c9 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -113,10 +113,14 @@ extern int dmar_parse_dev_scope(void *start, void *end, int *cnt,
> struct dmar_dev_scope **devices, u16 segment);
> extern void *dmar_alloc_dev_scope(void *start, void *end, int *cnt);
> extern void dmar_free_dev_scope(struct dmar_dev_scope **devices, int *cnt);
> -extern int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
> +extern int dmar_pci_insert_dev_scope(struct dmar_pci_notify_info *info,
> void *start, void*end, u16 segment,
> struct dmar_dev_scope *devices,
> int devices_cnt);
> +extern bool dmar_acpi_insert_dev_scope(u8 device_number,
> + struct acpi_device *adev, void *start, void *end,
> + struct dmar_dev_scope *devices, int devices_cnt);
> +
> extern int dmar_remove_dev_scope(struct dmar_pci_notify_info *info,
> u16 segment, struct dmar_dev_scope *devices,
> int count);
> @@ -140,6 +144,7 @@ extern int dmar_parse_one_atsr(struct acpi_dmar_header *header, void *arg);
> extern int dmar_check_one_atsr(struct acpi_dmar_header *hdr, void *arg);
> extern int dmar_release_one_atsr(struct acpi_dmar_header *hdr, void *arg);
> extern int dmar_iommu_hotplug(struct dmar_drhd_unit *dmaru, bool insert);
> +extern int dmar_rmrr_add_acpi_dev(u8 device_number, struct acpi_device *adev);
> extern int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info);
> #else /* !CONFIG_INTEL_IOMMU: */
> static inline int intel_iommu_init(void) { return -ENODEV; }
> @@ -150,6 +155,11 @@ static inline void intel_iommu_shutdown(void) { }
> #define dmar_check_one_atsr dmar_res_noop
> #define dmar_release_one_atsr dmar_res_noop
>
> +static inline int dmar_rmrr_add_acpi_dev(u8 device_number, struct acpi_device *adev)
> +{
> + return 0;
> +}
> +
> static inline int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
> {
> return 0;
>
Best regards,
baolu
[-- Attachment #1.2: Type: text/html, Size: 32225 bytes --]
[-- Attachment #2: Type: text/plain, Size: 156 bytes --]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-09-02 1:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-27 10:02 [PATCH v3 0/2] Add support for ACPI device in RMRR FelixCuioc
2020-08-27 10:02 ` [PATCH v3 1/2] iommu/vt-d:Add support for detecting " FelixCuioc
2020-09-01 6:05 ` Lu Baolu
2020-09-01 7:53 ` 答复: " FelixCui-oc
2020-09-02 1:48 ` FelixCui-oc [this message]
2020-08-27 10:02 ` [PATCH v3 2/2] iommu/vt-d:Add support for probing " FelixCuioc
2020-09-01 6:12 ` Lu Baolu
2020-09-01 9:13 ` 答复: " FelixCui-oc
2020-09-02 2:32 ` Lu Baolu
2020-09-02 3:24 ` 答复: " FelixCui-oc
2020-09-03 7:22 ` Lu Baolu
2020-09-04 5:42 ` 答复: " FelixCui-oc
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=09f1d01c5066455a98dcb3dd22ff28d0@zhaoxin.com \
--to=felixcui-oc@zhaoxin.com \
--cc=CobeChen-oc@zhaoxin.com \
--cc=TonyWWang-oc@zhaoxin.com \
--cc=baolu.lu@linux.intel.com \
--cc=dan.carpenter@oracle.com \
--cc=dwmw2@infradead.org \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=kbuild@lists.01.org \
--cc=linux-kernel@vger.kernel.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).