iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add support for ACPI device in RMRR
@ 2020-08-27 10:02 FelixCuioc
  2020-08-27 10:02 ` [PATCH v3 1/2] iommu/vt-d:Add support for detecting " FelixCuioc
  2020-08-27 10:02 ` [PATCH v3 2/2] iommu/vt-d:Add support for probing " FelixCuioc
  0 siblings, 2 replies; 12+ messages in thread
From: FelixCuioc @ 2020-08-27 10:02 UTC (permalink / raw)
  To: Joerg Roedel, iommu, linux-kernel, David Woodhouse, Lu Baolu,
	Dan Carpenter, kbuild
  Cc: TonyWWang-oc, CobeChen-oc

BIOS allocate reserved memory ranges that may be DMA targets.
BIOS may report each such reserved memory region through the
RMRR structures,along with the devices that requires access to
the specified reserved memory region.

The purpose of this series is to achieve ACPI device in RMRR
access reserved memory.Therefore,it is necessary to increase
the analysis of acpi device in RMRR and establish a mapping
for this device.

The first patch adds interfaces for detecting ACPI device
in RMRR and in order to distinguish it from pci device,
some interface functions are modified.

The second patch adds support for probing ACPI device in RMRR.
In probe_acpi_namespace_devices(),add support for direct mapping
of ACPI device and add support for physical node of acpi device
to be NULL.

v2->v3:
   - Add the blank line between functions.
   - Make dmar_acpi_insert_dev_scope() bool,change the 1/0 to true/false
     and add a comment explaining.
   - Delete unused initialization.
   - if dmar_acpi_insert_dev_scope() always returns zero,will not
     call dmar_rmrr_add_acpi_dev().
   - Use a proper error code.
   - Use if(!pdev).
   - Use goto unlock instead of mutex_unlock().


FelixCuioc (2):
  iommu/vt-d:Add support for detecting ACPI device in RMRR
  iommu/vt-d:Add support for probing ACPI device in RMRR

 drivers/iommu/intel/dmar.c  | 76 +++++++++++++++++++++----------------
 drivers/iommu/intel/iommu.c | 52 ++++++++++++++++++++++++-
 drivers/iommu/iommu.c       |  6 +++
 include/linux/dmar.h        | 12 +++++-
 include/linux/iommu.h       |  3 ++
 5 files changed, 114 insertions(+), 35 deletions(-)

-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
  2020-08-27 10:02 [PATCH v3 0/2] Add support for ACPI device in RMRR FelixCuioc
@ 2020-08-27 10:02 ` FelixCuioc
  2020-09-01  6:05   ` Lu Baolu
  2020-08-27 10:02 ` [PATCH v3 2/2] iommu/vt-d:Add support for probing " FelixCuioc
  1 sibling, 1 reply; 12+ messages in thread
From: FelixCuioc @ 2020-08-27 10:02 UTC (permalink / raw)
  To: Joerg Roedel, iommu, linux-kernel, David Woodhouse, Lu Baolu,
	Dan Carpenter, kbuild
  Cc: TonyWWang-oc, CobeChen-oc

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);
-			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));
 }
 
 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;
+}
+
 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;
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  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-08-27 10:02 ` FelixCuioc
  2020-09-01  6:12   ` Lu Baolu
  1 sibling, 1 reply; 12+ messages in thread
From: FelixCuioc @ 2020-08-27 10:02 UTC (permalink / raw)
  To: Joerg Roedel, iommu, linux-kernel, David Woodhouse, Lu Baolu,
	Dan Carpenter, kbuild
  Cc: TonyWWang-oc, CobeChen-oc

After acpi device in RMRR is detected,it is necessary
to establish a mapping for these devices.
In acpi_device_create_direct_mappings(),create a mapping
for the acpi device in RMRR.
Add a helper to achieve the acpi namespace device can
access the RMRR region.

Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
---
 drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
 drivers/iommu/iommu.c       |  6 ++++++
 include/linux/iommu.h       |  3 +++
 3 files changed, 38 insertions(+)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 208a91605288..51d7a5b18f41 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
 	return 1;
 }
 
+static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
+{
+	struct iommu_group *group;
+
+	acpi_device->bus->iommu_ops = &intel_iommu_ops;
+	group = iommu_group_get(pn_dev);
+	if (!group) {
+		pr_warn("ACPI name space devices create direct mappings wrong!\n");
+		return -EINVAL;
+	}
+	__acpi_device_create_direct_mappings(group, acpi_device);
+
+	return 0;
+}
+
 static int __init probe_acpi_namespace_devices(void)
 {
 	struct dmar_drhd_unit *drhd;
@@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
 			struct acpi_device_physical_node *pn;
 			struct iommu_group *group;
 			struct acpi_device *adev;
+			struct device *pn_dev = NULL;
 
 			if (dev->bus != &acpi_bus_type)
 				continue;
@@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
 					    &adev->physical_node_list, node) {
 				group = iommu_group_get(pn->dev);
 				if (group) {
+					pn_dev = pn->dev;
 					iommu_group_put(group);
 					continue;
 				}
@@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
 				ret = iommu_probe_device(pn->dev);
 				if (ret)
 					break;
+				pn_dev = pn->dev;
+			}
+			if (!pn_dev) {
+				dev->bus->iommu_ops = &intel_iommu_ops;
+				ret = iommu_probe_device(dev);
+				if (ret) {
+					pr_err("acpi_device probe fail! ret:%d\n", ret);
+					goto unlock;
+				}
+				goto unlock;
 			}
+			ret = acpi_device_create_direct_mappings(pn_dev, dev);
+unlock:
 			mutex_unlock(&adev->physical_node_lock);
 
 			if (ret)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 609bd25bf154..4f714a2d5ef7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain *domain,
 	return false;
 }
 
+void  __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
+{
+	iommu_create_device_direct_mappings(group, acpi_device);
+}
+EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
+
 /**
  * iommu_group_add_device - add a device to an iommu group
  * @group: the group into which to add the device (reference should be held)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fee209efb756..9be134775886 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
 extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
 			      unsigned long iova, int flags);
 
+extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
+						struct device *acpi_device);
+
 static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
 {
 	if (domain->ops->flush_iotlb_all)
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Lu Baolu @ 2020-09-01  6:05 UTC (permalink / raw)
  To: FelixCuioc, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: TonyWWang-oc, CobeChen-oc

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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Lu Baolu @ 2020-09-01  6:12 UTC (permalink / raw)
  To: FelixCuioc, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: TonyWWang-oc, CobeChen-oc

Hi Felix,

On 8/27/20 6:02 PM, FelixCuioc wrote:
> After acpi device in RMRR is detected,it is necessary
> to establish a mapping for these devices.
> In acpi_device_create_direct_mappings(),create a mapping
> for the acpi device in RMRR.
> Add a helper to achieve the acpi namespace device can
> access the RMRR region.

Are those ACPI devices visible to kernel? If so, any device driver bound
for it?

Best regards,
baolu

> 
> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
> ---
>   drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
>   drivers/iommu/iommu.c       |  6 ++++++
>   include/linux/iommu.h       |  3 +++
>   3 files changed, 38 insertions(+)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 208a91605288..51d7a5b18f41 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
>   	return 1;
>   }
>   
> +static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
> +{
> +	struct iommu_group *group;
> +
> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +	group = iommu_group_get(pn_dev);
> +	if (!group) {
> +		pr_warn("ACPI name space devices create direct mappings wrong!\n");
> +		return -EINVAL;
> +	}
> +	__acpi_device_create_direct_mappings(group, acpi_device);
> +
> +	return 0;
> +}
> +
>   static int __init probe_acpi_namespace_devices(void)
>   {
>   	struct dmar_drhd_unit *drhd;
> @@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
>   			struct acpi_device_physical_node *pn;
>   			struct iommu_group *group;
>   			struct acpi_device *adev;
> +			struct device *pn_dev = NULL;
>   
>   			if (dev->bus != &acpi_bus_type)
>   				continue;
> @@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
>   					    &adev->physical_node_list, node) {
>   				group = iommu_group_get(pn->dev);
>   				if (group) {
> +					pn_dev = pn->dev;
>   					iommu_group_put(group);
>   					continue;
>   				}
> @@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
>   				ret = iommu_probe_device(pn->dev);
>   				if (ret)
>   					break;
> +				pn_dev = pn->dev;
> +			}
> +			if (!pn_dev) {
> +				dev->bus->iommu_ops = &intel_iommu_ops;
> +				ret = iommu_probe_device(dev);
> +				if (ret) {
> +					pr_err("acpi_device probe fail! ret:%d\n", ret);
> +					goto unlock;
> +				}
> +				goto unlock;
>   			}
> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
> +unlock:
>   			mutex_unlock(&adev->physical_node_lock);
>   
>   			if (ret)
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 609bd25bf154..4f714a2d5ef7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain *domain,
>   	return false;
>   }
>   
> +void  __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
> +{
> +	iommu_create_device_direct_mappings(group, acpi_device);
> +}
> +EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
> +
>   /**
>    * iommu_group_add_device - add a device to an iommu group
>    * @group: the group into which to add the device (reference should be held)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index fee209efb756..9be134775886 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
>   extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
>   			      unsigned long iova, int flags);
>   
> +extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
> +						struct device *acpi_device);
> +
>   static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
>   {
>   	if (domain->ops->flush_iotlb_all)
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* 答复: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
  2020-09-01  6:05   ` Lu Baolu
@ 2020-09-01  7:53     ` FelixCui-oc
  2020-09-02  1:48       ` FelixCui-oc
  0 siblings, 1 reply; 12+ messages in thread
From: FelixCui-oc @ 2020-09-01  7:53 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Tony W Wang-oc, CobeChen-oc


[-- Attachment #1.1: Type: text/plain, Size: 12215 bytes --]

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: 29654 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  2020-09-01  6:12   ` Lu Baolu
@ 2020-09-01  9:13     ` FelixCui-oc
  2020-09-02  2:32       ` Lu Baolu
  0 siblings, 1 reply; 12+ messages in thread
From: FelixCui-oc @ 2020-09-01  9:13 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Tony W Wang-oc, CobeChen-oc


[-- Attachment #1.1: Type: text/plain, Size: 5477 bytes --]

hi  baolu ,

                   These ACPI devices can be retrieved from the kernel and there is no bound device driver .


Best regards

Felixcui-oc

________________________________
发件人: Lu Baolu <baolu.lu@linux.intel.com>
发送时间: 2020年9月1日 14:12:34
收件人: 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 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR

Hi Felix,

On 8/27/20 6:02 PM, FelixCuioc wrote:
> After acpi device in RMRR is detected,it is necessary
> to establish a mapping for these devices.
> In acpi_device_create_direct_mappings(),create a mapping
> for the acpi device in RMRR.
> Add a helper to achieve the acpi namespace device can
> access the RMRR region.

Are those ACPI devices visible to kernel? If so, any device driver bound
for it?

Best regards,
baolu

>
> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
> ---
>   drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
>   drivers/iommu/iommu.c       |  6 ++++++
>   include/linux/iommu.h       |  3 +++
>   3 files changed, 38 insertions(+)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 208a91605288..51d7a5b18f41 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
>        return 1;
>   }
>
> +static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
> +{
> +     struct iommu_group *group;
> +
> +     acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +     group = iommu_group_get(pn_dev);
> +     if (!group) {
> +             pr_warn("ACPI name space devices create direct mappings wrong!\n");
> +             return -EINVAL;
> +     }
> +     __acpi_device_create_direct_mappings(group, acpi_device);
> +
> +     return 0;
> +}
> +
>   static int __init probe_acpi_namespace_devices(void)
>   {
>        struct dmar_drhd_unit *drhd;
> @@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
>                        struct acpi_device_physical_node *pn;
>                        struct iommu_group *group;
>                        struct acpi_device *adev;
> +                     struct device *pn_dev = NULL;
>
>                        if (dev->bus != &acpi_bus_type)
>                                continue;
> @@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
>                                            &adev->physical_node_list, node) {
>                                group = iommu_group_get(pn->dev);
>                                if (group) {
> +                                     pn_dev = pn->dev;
>                                        iommu_group_put(group);
>                                        continue;
>                                }
> @@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
>                                ret = iommu_probe_device(pn->dev);
>                                if (ret)
>                                        break;
> +                             pn_dev = pn->dev;
> +                     }
> +                     if (!pn_dev) {
> +                             dev->bus->iommu_ops = &intel_iommu_ops;
> +                             ret = iommu_probe_device(dev);
> +                             if (ret) {
> +                                     pr_err("acpi_device probe fail! ret:%d\n", ret);
> +                                     goto unlock;
> +                             }
> +                             goto unlock;
>                        }
> +                     ret = acpi_device_create_direct_mappings(pn_dev, dev);
> +unlock:
>                        mutex_unlock(&adev->physical_node_lock);
>
>                        if (ret)
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 609bd25bf154..4f714a2d5ef7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain *domain,
>        return false;
>   }
>
> +void  __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
> +{
> +     iommu_create_device_direct_mappings(group, acpi_device);
> +}
> +EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
> +
>   /**
>    * iommu_group_add_device - add a device to an iommu group
>    * @group: the group into which to add the device (reference should be held)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index fee209efb756..9be134775886 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
>   extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
>                              unsigned long iova, int flags);
>
> +extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
> +                                             struct device *acpi_device);
> +
>   static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
>   {
>        if (domain->ops->flush_iotlb_all)
>

[-- Attachment #1.2: Type: text/html, Size: 13586 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* 答复: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
  2020-09-01  7:53     ` 答复: " FelixCui-oc
@ 2020-09-02  1:48       ` FelixCui-oc
  0 siblings, 0 replies; 12+ messages in thread
From: FelixCui-oc @ 2020-09-02  1:48 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Tony W Wang-oc, CobeChen-oc


[-- 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  2020-09-01  9:13     ` 答复: " FelixCui-oc
@ 2020-09-02  2:32       ` Lu Baolu
  2020-09-02  3:24         ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 12+ messages in thread
From: Lu Baolu @ 2020-09-02  2:32 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Ashok Raj, Tony W Wang-oc, CobeChen-oc

On 9/1/20 5:13 PM, FelixCui-oc wrote:
> hi  baolu ,
> 
>                     These ACPI devices can be retrieved from the kernel 
> and there is no bound device driver .

So you have a hidden device (invisible to host kernel). But you need to
setup some identity mappings for this device, so that the firmware
could keep working, right?

The platform designs this by putting that range in the RMRR table and
expecting the OS kernel to setup identity mappings during boot.

Do I understand it right?

Best regards,
baolu

> 
> 
> Best regards
> 
> Felixcui-oc
> 
> ------------------------------------------------------------------------
> *发件人:* Lu Baolu <baolu.lu@linux.intel.com>
> *发送时间:* 2020年9月1日 14:12:34
> *收件人:* 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 2/2] iommu/vt-d:Add support for probing ACPI 
> device in RMRR
> Hi Felix,
> 
> On 8/27/20 6:02 PM, FelixCuioc wrote:
>> After acpi device in RMRR is detected,it is necessary
>> to establish a mapping for these devices.
>> In acpi_device_create_direct_mappings(),create a mapping
>> for the acpi device in RMRR.
>> Add a helper to achieve the acpi namespace device can
>> access the RMRR region.
> 
> Are those ACPI devices visible to kernel? If so, any device driver bound
> for it?
> 
> Best regards,
> baolu
> 
>> 
>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
>>   drivers/iommu/iommu.c       |  6 ++++++
>>   include/linux/iommu.h       |  3 +++
>>   3 files changed, 38 insertions(+)
>> 
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 208a91605288..51d7a5b18f41 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
>>        return 1;
>>   }
>>   
>> +static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
>> +{
>> +     struct iommu_group *group;
>> +
>> +     acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +     group = iommu_group_get(pn_dev);
>> +     if (!group) {
>> +             pr_warn("ACPI name space devices create direct mappings wrong!\n");
>> +             return -EINVAL;
>> +     }
>> +     __acpi_device_create_direct_mappings(group, acpi_device);
>> +
>> +     return 0;
>> +}
>> +
>>   static int __init probe_acpi_namespace_devices(void)
>>   {
>>        struct dmar_drhd_unit *drhd;
>> @@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
>>                        struct acpi_device_physical_node *pn;
>>                        struct iommu_group *group;
>>                        struct acpi_device *adev;
>> +                     struct device *pn_dev = NULL;
>>   
>>                        if (dev->bus != &acpi_bus_type)
>>                                continue;
>> @@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
>>                                            &adev->physical_node_list, node) {
>>                                group = iommu_group_get(pn->dev);
>>                                if (group) {
>> +                                     pn_dev = pn->dev;
>>                                        iommu_group_put(group);
>>                                        continue;
>>                                }
>> @@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
>>                                ret = iommu_probe_device(pn->dev);
>>                                if (ret)
>>                                        break;
>> +                             pn_dev = pn->dev;
>> +                     }
>> +                     if (!pn_dev) {
>> +                             dev->bus->iommu_ops = &intel_iommu_ops;
>> +                             ret = iommu_probe_device(dev);
>> +                             if (ret) {
>> +                                     pr_err("acpi_device probe fail! ret:%d\n", ret);
>> +                                     goto unlock;
>> +                             }
>> +                             goto unlock;
>>                        }
>> +                     ret = acpi_device_create_direct_mappings(pn_dev, dev);
>> +unlock:
>>                        mutex_unlock(&adev->physical_node_lock);
>>   
>>                        if (ret)
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 609bd25bf154..4f714a2d5ef7 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain *domain,
>>        return false;
>>   }
>>   
>> +void  __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
>> +{
>> +     iommu_create_device_direct_mappings(group, acpi_device);
>> +}
>> +EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
>> +
>>   /**
>>    * iommu_group_add_device - add a device to an iommu group
>>    * @group: the group into which to add the device (reference should be held)
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index fee209efb756..9be134775886 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
>>   extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
>>                              unsigned long iova, int flags);
>>   
>> +extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
>> +                                             struct device *acpi_device);
>> +
>>   static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
>>   {
>>        if (domain->ops->flush_iotlb_all)
>> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* 答复: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  2020-09-02  2:32       ` Lu Baolu
@ 2020-09-02  3:24         ` FelixCui-oc
  2020-09-03  7:22           ` Lu Baolu
  0 siblings, 1 reply; 12+ messages in thread
From: FelixCui-oc @ 2020-09-02  3:24 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Tony W Wang-oc, CobeChen-oc, Ashok Raj


[-- Attachment #1.1: Type: text/plain, Size: 6969 bytes --]

hi baolu,

> So you have a hidden device (invisible to host kernel). But you need to

>setup some identity mappings for this device, so that the firmware
>could keep working, right?

>The platform designs this by putting that range in the RMRR table and
>expecting the OS kernel to setup identity mappings during boot.

>Do I understand it right?


Yes. What you understand is correct.


Best regards

Felixcui-oc

________________________________
发件人: Lu Baolu <baolu.lu@linux.intel.com>
发送时间: 2020年9月2日 10:32:36
收件人: 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; Ashok Raj
主题: Re: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR

On 9/1/20 5:13 PM, FelixCui-oc wrote:
> hi  baolu ,
>
>                     These ACPI devices can be retrieved from the kernel
> and there is no bound device driver .

So you have a hidden device (invisible to host kernel). But you need to
setup some identity mappings for this device, so that the firmware
could keep working, right?

The platform designs this by putting that range in the RMRR table and
expecting the OS kernel to setup identity mappings during boot.

Do I understand it right?

Best regards,
baolu

>
>
> Best regards
>
> Felixcui-oc
>
> ------------------------------------------------------------------------
> *发件人:* Lu Baolu <baolu.lu@linux.intel.com>
> *发送时间:* 2020年9月1日 14:12:34
> *收件人:* 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 2/2] iommu/vt-d:Add support for probing ACPI
> device in RMRR
> Hi Felix,
>
> On 8/27/20 6:02 PM, FelixCuioc wrote:
>> After acpi device in RMRR is detected,it is necessary
>> to establish a mapping for these devices.
>> In acpi_device_create_direct_mappings(),create a mapping
>> for the acpi device in RMRR.
>> Add a helper to achieve the acpi namespace device can
>> access the RMRR region.
>
> Are those ACPI devices visible to kernel? If so, any device driver bound
> for it?
>
> Best regards,
> baolu
>
>>
>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
>>   drivers/iommu/iommu.c       |  6 ++++++
>>   include/linux/iommu.h       |  3 +++
>>   3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 208a91605288..51d7a5b18f41 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
>>        return 1;
>>   }
>>
>> +static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
>> +{
>> +     struct iommu_group *group;
>> +
>> +     acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +     group = iommu_group_get(pn_dev);
>> +     if (!group) {
>> +             pr_warn("ACPI name space devices create direct mappings wrong!\n");
>> +             return -EINVAL;
>> +     }
>> +     __acpi_device_create_direct_mappings(group, acpi_device);
>> +
>> +     return 0;
>> +}
>> +
>>   static int __init probe_acpi_namespace_devices(void)
>>   {
>>        struct dmar_drhd_unit *drhd;
>> @@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
>>                        struct acpi_device_physical_node *pn;
>>                        struct iommu_group *group;
>>                        struct acpi_device *adev;
>> +                     struct device *pn_dev = NULL;
>>
>>                        if (dev->bus != &acpi_bus_type)
>>                                continue;
>> @@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
>>                                            &adev->physical_node_list, node) {
>>                                group = iommu_group_get(pn->dev);
>>                                if (group) {
>> +                                     pn_dev = pn->dev;
>>                                        iommu_group_put(group);
>>                                        continue;
>>                                }
>> @@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
>>                                ret = iommu_probe_device(pn->dev);
>>                                if (ret)
>>                                        break;
>> +                             pn_dev = pn->dev;
>> +                     }
>> +                     if (!pn_dev) {
>> +                             dev->bus->iommu_ops = &intel_iommu_ops;
>> +                             ret = iommu_probe_device(dev);
>> +                             if (ret) {
>> +                                     pr_err("acpi_device probe fail! ret:%d\n", ret);
>> +                                     goto unlock;
>> +                             }
>> +                             goto unlock;
>>                        }
>> +                     ret = acpi_device_create_direct_mappings(pn_dev, dev);
>> +unlock:
>>                        mutex_unlock(&adev->physical_node_lock);
>>
>>                        if (ret)
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 609bd25bf154..4f714a2d5ef7 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain *domain,
>>        return false;
>>   }
>>
>> +void  __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
>> +{
>> +     iommu_create_device_direct_mappings(group, acpi_device);
>> +}
>> +EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
>> +
>>   /**
>>    * iommu_group_add_device - add a device to an iommu group
>>    * @group: the group into which to add the device (reference should be held)
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index fee209efb756..9be134775886 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
>>   extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
>>                              unsigned long iova, int flags);
>>
>> +extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
>> +                                             struct device *acpi_device);
>> +
>>   static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
>>   {
>>        if (domain->ops->flush_iotlb_all)
>>

[-- Attachment #1.2: Type: text/html, Size: 18315 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 答复: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  2020-09-02  3:24         ` 答复: " FelixCui-oc
@ 2020-09-03  7:22           ` Lu Baolu
  2020-09-04  5:42             ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 12+ messages in thread
From: Lu Baolu @ 2020-09-03  7:22 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Ashok Raj, Tony W Wang-oc, CobeChen-oc

Hi Felix,

On 9/2/20 11:24 AM, FelixCui-oc wrote:
> hi baolu,
> 
>> So you have a hidden device (invisible to host kernel). But you need to
> 
>>setup some identity mappings for this device, so that the firmware
>>could keep working, right?
> 
>>The platform designs this by putting that range in the RMRR table and
>>expecting the OS kernel to setup identity mappings during boot.
> 
>>Do I understand it right?
> 
> 
> Yes. What you understand is correct.

This appears to be a new usage model of RMRR. I need to discuss this
with the VT-d spec maintainer. Do you mind telling which platform are
you going to run this on? What is the motivation of creating such hidden
device?

Basically, RMRRs were added as work around for certain legacy device and
we have been working hard to fix those legacy devices so that RMRR are
no longer needed.

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* 答复: 答复: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
  2020-09-03  7:22           ` Lu Baolu
@ 2020-09-04  5:42             ` FelixCui-oc
  0 siblings, 0 replies; 12+ messages in thread
From: FelixCui-oc @ 2020-09-04  5:42 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse,
	Dan Carpenter, kbuild
  Cc: Tony W Wang-oc, CobeChen-oc, Ashok Raj


[-- Attachment #1.1: Type: text/plain, Size: 1668 bytes --]

hi baolu,

               We run this on zhaoxin X86 paltform.

               There are some MCUS on our platforms that read and write data to the system memory.

During this process, the MCU is invisible to the host kernel. And the MCU needs to report through ACPI_NAMESPACE_DEVICE in RMRR.


Best regards

Felixcui-oc

________________________________
发件人: Lu Baolu <baolu.lu@linux.intel.com>
发送时间: 2020年9月3日 15:22:14
收件人: 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; Ashok Raj
主题: Re: 答复: 答复: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR

Hi Felix,

On 9/2/20 11:24 AM, FelixCui-oc wrote:
> hi baolu,
>
>> So you have a hidden device (invisible to host kernel). But you need to
>
>>setup some identity mappings for this device, so that the firmware
>>could keep working, right?
>
>>The platform designs this by putting that range in the RMRR table and
>>expecting the OS kernel to setup identity mappings during boot.
>
>>Do I understand it right?
>
>
> Yes. What you understand is correct.

This appears to be a new usage model of RMRR. I need to discuss this
with the VT-d spec maintainer. Do you mind telling which platform are
you going to run this on? What is the motivation of creating such hidden
device?

Basically, RMRRs were added as work around for certain legacy device and
we have been working hard to fix those legacy devices so that RMRR are
no longer needed.

Best regards,
baolu

[-- Attachment #1.2: Type: text/html, Size: 2910 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-09-04  5:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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).