iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
@ 2020-08-02 10:07 FelixCuioc
  2020-08-03  2:31 ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCuioc @ 2020-08-02 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu, linux-kernel, David Woodhouse, Lu Baolu
  Cc: CobeChen, RaymondPang

Some ACPI devices require access to the specified reserved memory
region.BIOS report the specified reserved memory region through
RMRR structures.Add analysis of ACPI device in RMRR and establish
identity mapping for ACPI device.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
---
 drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
 drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
 drivers/iommu/iommu.c       |  6 +++
 include/linux/dmar.h        | 12 +++++-
 include/linux/iommu.h       |  3 ++
 5 files changed, 105 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 93e6345f3414..024ca38dba12 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);
@@ -696,48 +696,56 @@ 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)
+int 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 1;
+			}
+		WARN_ON(i >= devices_cnt);
+	}
+	return 0;
+}
+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 = 0;
+
 	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;
+	}
+	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 +774,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..be1793415326 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4507,6 +4507,24 @@ 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)
 {
@@ -4523,7 +4541,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 +4559,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);
@@ -4779,6 +4797,26 @@ 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)
+{
+	int ret;
+	struct iommu_group *group;
+
+	if (pn_dev == NULL) {
+		acpi_device->bus->iommu_ops = &intel_iommu_ops;
+		ret = iommu_probe_device(acpi_device);
+		if (ret) {
+			pr_err("acpi_device probe fail! ret:%d\n", ret);
+			return ret;
+		}
+		return 0;
+	}
+	acpi_device->bus->iommu_ops = &intel_iommu_ops;
+	group = iommu_group_get(pn_dev);
+	__acpi_device_create_direct_mappings(group, acpi_device);
+
+	return 0;
+}
 
 static int __init probe_acpi_namespace_devices(void)
 {
@@ -4794,6 +4832,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;
@@ -4804,6 +4843,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;
 				}
@@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
 				ret = iommu_probe_device(pn->dev);
 				if (ret)
 					break;
+				pn_dev = pn->dev;
 			}
+			ret = acpi_device_create_direct_mappings(pn_dev, dev);
 			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/dmar.h b/include/linux/dmar.h
index 65565820328a..881ac61a4336 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 int 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;
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] 18+ messages in thread

* Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-02 10:07 [PATCH] iommu/vt-d:Add support for ACPI device in RMRR FelixCuioc
@ 2020-08-03  2:31 ` Lu Baolu
  2020-08-03  4:40   ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-03  2:31 UTC (permalink / raw)
  To: FelixCuioc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen, RaymondPang

Hi,

On 8/2/20 6:07 PM, FelixCuioc wrote:
> Some ACPI devices require access to the specified reserved memory
> region.BIOS report the specified reserved memory region through
> RMRR structures.Add analysis of ACPI device in RMRR and establish
> identity mapping for ACPI device.

Can you please add more words about the problem you want to solve? Do
you mean some RMRRs are not enumerated correctly? Or, enumerated, but
not identity mapped?

Nit: add version and change log once you refreshed your patch.

> 
> Reported-by: kernel test robot <lkp@intel.com>

No need to add this. The problem you want to solve through this patch is
not reported by lkp.

Best regards,
baolu

> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
> ---
>   drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>   drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>   drivers/iommu/iommu.c       |  6 +++
>   include/linux/dmar.h        | 12 +++++-
>   include/linux/iommu.h       |  3 ++
>   5 files changed, 105 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 93e6345f3414..024ca38dba12 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);
> @@ -696,48 +696,56 @@ 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)
> +int 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 1;
> +			}
> +		WARN_ON(i >= devices_cnt);
> +	}
> +	return 0;
> +}
> +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 = 0;
> +
>   	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;
> +	}
> +	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 +774,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..be1793415326 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4507,6 +4507,24 @@ 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)
>   {
> @@ -4523,7 +4541,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 +4559,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);
> @@ -4779,6 +4797,26 @@ 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)
> +{
> +	int ret;
> +	struct iommu_group *group;
> +
> +	if (pn_dev == NULL) {
> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +		ret = iommu_probe_device(acpi_device);
> +		if (ret) {
> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
> +			return ret;
> +		}
> +		return 0;
> +	}
> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +	group = iommu_group_get(pn_dev);
> +	__acpi_device_create_direct_mappings(group, acpi_device);
> +
> +	return 0;
> +}
>   
>   static int __init probe_acpi_namespace_devices(void)
>   {
> @@ -4794,6 +4832,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;
> @@ -4804,6 +4843,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;
>   				}
> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>   				ret = iommu_probe_device(pn->dev);
>   				if (ret)
>   					break;
> +				pn_dev = pn->dev;
>   			}
> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>   			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/dmar.h b/include/linux/dmar.h
> index 65565820328a..881ac61a4336 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 int 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;
> 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] 18+ messages in thread

* 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  2:31 ` Lu Baolu
@ 2020-08-03  4:40   ` FelixCui-oc
  2020-08-03  6:18     ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-03  4:40 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi baolu:
		Some ACPI devices need to issue dma requests to access the reserved memory area.
		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for these ACPI devices. In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
		You can refer to the description of type in 8.3.1 device scope in vt-d spec.

Best regards
FelixCui-oc



-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月3日 10:32
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi,

On 8/2/20 6:07 PM, FelixCuioc wrote:
> Some ACPI devices require access to the specified reserved memory 
> region.BIOS report the specified reserved memory region through RMRR 
> structures.Add analysis of ACPI device in RMRR and establish identity 
> mapping for ACPI device.

Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?

Nit: add version and change log once you refreshed your patch.

> 
> Reported-by: kernel test robot <lkp@intel.com>

No need to add this. The problem you want to solve through this patch is not reported by lkp.

Best regards,
baolu

> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
> ---
>   drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>   drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>   drivers/iommu/iommu.c       |  6 +++
>   include/linux/dmar.h        | 12 +++++-
>   include/linux/iommu.h       |  3 ++
>   5 files changed, 105 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c 
> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@ 
> 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)
> +int 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 1;
> +			}
> +		WARN_ON(i >= devices_cnt);
> +	}
> +	return 0;
> +}
> +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 = 0;
> +
>   	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;
> +	}
> +	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 +774,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..be1793415326 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4507,6 +4507,24 @@ 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)
>   {
> @@ -4523,7 +4541,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 +4559,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);
> @@ -4779,6 +4797,26 @@ 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) {
> +	int ret;
> +	struct iommu_group *group;
> +
> +	if (pn_dev == NULL) {
> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +		ret = iommu_probe_device(acpi_device);
> +		if (ret) {
> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
> +			return ret;
> +		}
> +		return 0;
> +	}
> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
> +	group = iommu_group_get(pn_dev);
> +	__acpi_device_create_direct_mappings(group, acpi_device);
> +
> +	return 0;
> +}
>   
>   static int __init probe_acpi_namespace_devices(void)
>   {
> @@ -4794,6 +4832,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;
> @@ -4804,6 +4843,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;
>   				}
> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>   				ret = iommu_probe_device(pn->dev);
>   				if (ret)
>   					break;
> +				pn_dev = pn->dev;
>   			}
> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>   			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/dmar.h b/include/linux/dmar.h 
> index 65565820328a..881ac61a4336 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 int 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;
> 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] 18+ messages in thread

* Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  4:40   ` 答复: " FelixCui-oc
@ 2020-08-03  6:18     ` Lu Baolu
  2020-08-03  6:52       ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-03  6:18 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi,

On 2020/8/3 12:40, FelixCui-oc wrote:
> Hi baolu:
> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.

So the problem is "although namespace devices in RMRR have been parsed,
but the identity map for those devices aren't created. As the result,
the namespace devices fail to access the RMRR region."

Do I understand it right?

Best regards,
baolu

> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
> 
> Best regards
> FelixCui-oc
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 10:32
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi,
> 
> On 8/2/20 6:07 PM, FelixCuioc wrote:
>> Some ACPI devices require access to the specified reserved memory
>> region.BIOS report the specified reserved memory region through RMRR
>> structures.Add analysis of ACPI device in RMRR and establish identity
>> mapping for ACPI device.
> 
> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
> 
> Nit: add version and change log once you refreshed your patch.
> 
>>
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> No need to add this. The problem you want to solve through this patch is not reported by lkp.
> 
> Best regards,
> baolu
> 
>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>> ---
>>    drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>    drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>    drivers/iommu/iommu.c       |  6 +++
>>    include/linux/dmar.h        | 12 +++++-
>>    include/linux/iommu.h       |  3 ++
>>    5 files changed, 105 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@
>> 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)
>> +int 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 1;
>> +			}
>> +		WARN_ON(i >= devices_cnt);
>> +	}
>> +	return 0;
>> +}
>> +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 = 0;
>> +
>>    	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;
>> +	}
>> +	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 +774,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..be1793415326 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4507,6 +4507,24 @@ 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)
>>    {
>> @@ -4523,7 +4541,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 +4559,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);
>> @@ -4779,6 +4797,26 @@ 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) {
>> +	int ret;
>> +	struct iommu_group *group;
>> +
>> +	if (pn_dev == NULL) {
>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +		ret = iommu_probe_device(acpi_device);
>> +		if (ret) {
>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>> +			return ret;
>> +		}
>> +		return 0;
>> +	}
>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +	group = iommu_group_get(pn_dev);
>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>> +
>> +	return 0;
>> +}
>>    
>>    static int __init probe_acpi_namespace_devices(void)
>>    {
>> @@ -4794,6 +4832,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;
>> @@ -4804,6 +4843,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;
>>    				}
>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>    				ret = iommu_probe_device(pn->dev);
>>    				if (ret)
>>    					break;
>> +				pn_dev = pn->dev;
>>    			}
>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>    			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/dmar.h b/include/linux/dmar.h
>> index 65565820328a..881ac61a4336 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 int 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;
>> 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] 18+ messages in thread

* 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  6:18     ` Lu Baolu
@ 2020-08-03  6:52       ` FelixCui-oc
  2020-08-03  8:25         ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-03  6:52 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi  baolu ,
		Yes ,that's right.
		This patch is to achieve acpi namespace devices to access the RMRR region.

Best regards
Felix cui




-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月3日 14:19
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi,

On 2020/8/3 12:40, FelixCui-oc wrote:
> Hi baolu:
> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.

So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."

Do I understand it right?

Best regards,
baolu

> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
> 
> Best regards
> FelixCui-oc
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 10:32
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) <CobeChen@zhaoxin.com>; 
> Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi,
> 
> On 8/2/20 6:07 PM, FelixCuioc wrote:
>> Some ACPI devices require access to the specified reserved memory 
>> region.BIOS report the specified reserved memory region through RMRR 
>> structures.Add analysis of ACPI device in RMRR and establish identity 
>> mapping for ACPI device.
> 
> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
> 
> Nit: add version and change log once you refreshed your patch.
> 
>>
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> No need to add this. The problem you want to solve through this patch is not reported by lkp.
> 
> Best regards,
> baolu
> 
>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>> ---
>>    drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>    drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>    drivers/iommu/iommu.c       |  6 +++
>>    include/linux/dmar.h        | 12 +++++-
>>    include/linux/iommu.h       |  3 ++
>>    5 files changed, 105 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c 
>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@ 
>> 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)
>> +int 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 1;
>> +			}
>> +		WARN_ON(i >= devices_cnt);
>> +	}
>> +	return 0;
>> +}
>> +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 = 0;
>> +
>>    	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;
>> +	}
>> +	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 +774,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..be1793415326 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4507,6 +4507,24 @@ 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)
>>    {
>> @@ -4523,7 +4541,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 +4559,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);
>> @@ -4779,6 +4797,26 @@ 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) {
>> +	int ret;
>> +	struct iommu_group *group;
>> +
>> +	if (pn_dev == NULL) {
>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +		ret = iommu_probe_device(acpi_device);
>> +		if (ret) {
>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>> +			return ret;
>> +		}
>> +		return 0;
>> +	}
>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>> +	group = iommu_group_get(pn_dev);
>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>> +
>> +	return 0;
>> +}
>>    
>>    static int __init probe_acpi_namespace_devices(void)
>>    {
>> @@ -4794,6 +4832,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;
>> @@ -4804,6 +4843,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;
>>    				}
>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>    				ret = iommu_probe_device(pn->dev);
>>    				if (ret)
>>    					break;
>> +				pn_dev = pn->dev;
>>    			}
>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>    			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/dmar.h 
>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>> 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] 18+ messages in thread

* Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  6:52       ` 答复: " FelixCui-oc
@ 2020-08-03  8:25         ` Lu Baolu
  2020-08-03  9:01           ` 答复: " FelixCui-oc
  2020-08-03  9:41           ` FelixCui-oc
  0 siblings, 2 replies; 18+ messages in thread
From: Lu Baolu @ 2020-08-03  8:25 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

On 2020/8/3 14:52, FelixCui-oc wrote:
> Hi  baolu ,
> 		Yes ,that's right.
> 		This patch is to achieve acpi namespace devices to access the RMRR region.

The namespace devices are enumerated in probe_acpi_namespace_devices().
It calls iommu_probe_device() to process the enumeration and setup the
identity mappings. Can you please check why that code doesn't work for
you?

Best regards,
baolu

> 
> Best regards
> Felix cui
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 14:19
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi,
> 
> On 2020/8/3 12:40, FelixCui-oc wrote:
>> Hi baolu:
>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
> 
> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
> 
> Do I understand it right?
> 
> Best regards,
> baolu
> 
>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>
>> Best regards
>> FelixCui-oc
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 10:32
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) <CobeChen@zhaoxin.com>;
>> Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi,
>>
>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>> Some ACPI devices require access to the specified reserved memory
>>> region.BIOS report the specified reserved memory region through RMRR
>>> structures.Add analysis of ACPI device in RMRR and establish identity
>>> mapping for ACPI device.
>>
>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>
>> Nit: add version and change log once you refreshed your patch.
>>
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>
>> Best regards,
>> baolu
>>
>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>> ---
>>>     drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>     drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>     drivers/iommu/iommu.c       |  6 +++
>>>     include/linux/dmar.h        | 12 +++++-
>>>     include/linux/iommu.h       |  3 ++
>>>     5 files changed, 105 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@
>>> 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)
>>> +int 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 1;
>>> +			}
>>> +		WARN_ON(i >= devices_cnt);
>>> +	}
>>> +	return 0;
>>> +}
>>> +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 = 0;
>>> +
>>>     	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;
>>> +	}
>>> +	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 +774,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..be1793415326 100644
>>> --- a/drivers/iommu/intel/iommu.c
>>> +++ b/drivers/iommu/intel/iommu.c
>>> @@ -4507,6 +4507,24 @@ 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)
>>>     {
>>> @@ -4523,7 +4541,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 +4559,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);
>>> @@ -4779,6 +4797,26 @@ 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) {
>>> +	int ret;
>>> +	struct iommu_group *group;
>>> +
>>> +	if (pn_dev == NULL) {
>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +		ret = iommu_probe_device(acpi_device);
>>> +		if (ret) {
>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>> +			return ret;
>>> +		}
>>> +		return 0;
>>> +	}
>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +	group = iommu_group_get(pn_dev);
>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>> +
>>> +	return 0;
>>> +}
>>>     
>>>     static int __init probe_acpi_namespace_devices(void)
>>>     {
>>> @@ -4794,6 +4832,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;
>>> @@ -4804,6 +4843,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;
>>>     				}
>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>     				ret = iommu_probe_device(pn->dev);
>>>     				if (ret)
>>>     					break;
>>> +				pn_dev = pn->dev;
>>>     			}
>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>     			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/dmar.h
>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>> 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] 18+ messages in thread

* 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  8:25         ` Lu Baolu
@ 2020-08-03  9:01           ` FelixCui-oc
  2020-08-03  9:41           ` FelixCui-oc
  1 sibling, 0 replies; 18+ messages in thread
From: FelixCui-oc @ 2020-08-03  9:01 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi  baolu:
		"The namespace devices are enumerated in probe_acpi_namespace_devices().
		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
		
		This situation only applies to the physical node of the namespaces device as the pci device.
		In fact, the physical node of the namespaces device can be a platform device or NULL.
		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.

Best regards
Felix cui




-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月3日 16:26
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

On 2020/8/3 14:52, FelixCui-oc wrote:
> Hi  baolu ,
> 		Yes ,that's right.
> 		This patch is to achieve acpi namespace devices to access the RMRR region.

The namespace devices are enumerated in probe_acpi_namespace_devices().
It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?

Best regards,
baolu

> 
> Best regards
> Felix cui
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 14:19
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi,
> 
> On 2020/8/3 12:40, FelixCui-oc wrote:
>> Hi baolu:
>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
> 
> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
> 
> Do I understand it right?
> 
> Best regards,
> baolu
> 
>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>
>> Best regards
>> FelixCui-oc
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 10:32
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) 
>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi,
>>
>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>> Some ACPI devices require access to the specified reserved memory 
>>> region.BIOS report the specified reserved memory region through RMRR 
>>> structures.Add analysis of ACPI device in RMRR and establish 
>>> identity mapping for ACPI device.
>>
>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>
>> Nit: add version and change log once you refreshed your patch.
>>
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>
>> Best regards,
>> baolu
>>
>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>> ---
>>>     drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>     drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>     drivers/iommu/iommu.c       |  6 +++
>>>     include/linux/dmar.h        | 12 +++++-
>>>     include/linux/iommu.h       |  3 ++
>>>     5 files changed, 105 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c 
>>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@ 
>>> 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)
>>> +int 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 1;
>>> +			}
>>> +		WARN_ON(i >= devices_cnt);
>>> +	}
>>> +	return 0;
>>> +}
>>> +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 = 0;
>>> +
>>>     	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;
>>> +	}
>>> +	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 
>>> +774,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..be1793415326 
>>> 100644
>>> --- a/drivers/iommu/intel/iommu.c
>>> +++ b/drivers/iommu/intel/iommu.c
>>> @@ -4507,6 +4507,24 @@ 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)
>>>     {
>>> @@ -4523,7 +4541,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 +4559,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);
>>> @@ -4779,6 +4797,26 @@ 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) {
>>> +	int ret;
>>> +	struct iommu_group *group;
>>> +
>>> +	if (pn_dev == NULL) {
>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +		ret = iommu_probe_device(acpi_device);
>>> +		if (ret) {
>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>> +			return ret;
>>> +		}
>>> +		return 0;
>>> +	}
>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +	group = iommu_group_get(pn_dev);
>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>> +
>>> +	return 0;
>>> +}
>>>     
>>>     static int __init probe_acpi_namespace_devices(void)
>>>     {
>>> @@ -4794,6 +4832,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;
>>> @@ -4804,6 +4843,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;
>>>     				}
>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>     				ret = iommu_probe_device(pn->dev);
>>>     				if (ret)
>>>     					break;
>>> +				pn_dev = pn->dev;
>>>     			}
>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>     			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/dmar.h 
>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>> 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] 18+ messages in thread

* 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  8:25         ` Lu Baolu
  2020-08-03  9:01           ` 答复: " FelixCui-oc
@ 2020-08-03  9:41           ` FelixCui-oc
  2020-08-04  1:11             ` Lu Baolu
  1 sibling, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-03  9:41 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi baolu,
		 dmar_acpi_dev_scope_init() parse ANDD structure and enumerated namespaces device in DRHD.
		 But the namespace device in RMRR is not enumerated, right?

Best regards
Felix cui-oc




-----邮件原件-----
发件人: FelixCui-oc 
发送时间: 2020年8月3日 17:02
收件人: 'Lu Baolu' <baolu.lu@linux.intel.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi  baolu:
		"The namespace devices are enumerated in probe_acpi_namespace_devices().
		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
		
		This situation only applies to the physical node of the namespaces device as the pci device.
		In fact, the physical node of the namespaces device can be a platform device or NULL.
		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.

Best regards
Felix cui




-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com>
发送时间: 2020年8月3日 16:26
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

On 2020/8/3 14:52, FelixCui-oc wrote:
> Hi  baolu ,
> 		Yes ,that's right.
> 		This patch is to achieve acpi namespace devices to access the RMRR region.

The namespace devices are enumerated in probe_acpi_namespace_devices().
It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?

Best regards,
baolu

> 
> Best regards
> Felix cui
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 14:19
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi,
> 
> On 2020/8/3 12:40, FelixCui-oc wrote:
>> Hi baolu:
>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
> 
> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
> 
> Do I understand it right?
> 
> Best regards,
> baolu
> 
>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>
>> Best regards
>> FelixCui-oc
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 10:32
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) 
>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi,
>>
>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>> Some ACPI devices require access to the specified reserved memory 
>>> region.BIOS report the specified reserved memory region through RMRR 
>>> structures.Add analysis of ACPI device in RMRR and establish 
>>> identity mapping for ACPI device.
>>
>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>
>> Nit: add version and change log once you refreshed your patch.
>>
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>
>> Best regards,
>> baolu
>>
>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>> ---
>>>     drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>     drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>     drivers/iommu/iommu.c       |  6 +++
>>>     include/linux/dmar.h        | 12 +++++-
>>>     include/linux/iommu.h       |  3 ++
>>>     5 files changed, 105 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c 
>>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@ 
>>> 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)
>>> +int 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 1;
>>> +			}
>>> +		WARN_ON(i >= devices_cnt);
>>> +	}
>>> +	return 0;
>>> +}
>>> +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 = 0;
>>> +
>>>     	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;
>>> +	}
>>> +	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
>>> +774,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..be1793415326
>>> 100644
>>> --- a/drivers/iommu/intel/iommu.c
>>> +++ b/drivers/iommu/intel/iommu.c
>>> @@ -4507,6 +4507,24 @@ 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)
>>>     {
>>> @@ -4523,7 +4541,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 +4559,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);
>>> @@ -4779,6 +4797,26 @@ 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) {
>>> +	int ret;
>>> +	struct iommu_group *group;
>>> +
>>> +	if (pn_dev == NULL) {
>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +		ret = iommu_probe_device(acpi_device);
>>> +		if (ret) {
>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>> +			return ret;
>>> +		}
>>> +		return 0;
>>> +	}
>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>> +	group = iommu_group_get(pn_dev);
>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>> +
>>> +	return 0;
>>> +}
>>>     
>>>     static int __init probe_acpi_namespace_devices(void)
>>>     {
>>> @@ -4794,6 +4832,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;
>>> @@ -4804,6 +4843,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;
>>>     				}
>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>     				ret = iommu_probe_device(pn->dev);
>>>     				if (ret)
>>>     					break;
>>> +				pn_dev = pn->dev;
>>>     			}
>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>     			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/dmar.h 
>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>> 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] 18+ messages in thread

* Re: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-03  9:41           ` FelixCui-oc
@ 2020-08-04  1:11             ` Lu Baolu
  2020-08-04  3:11               ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-04  1:11 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi Felix,

On 2020/8/3 17:41, FelixCui-oc wrote:
> Hi baolu,
> 		 dmar_acpi_dev_scope_init() parse ANDD structure and enumerated namespaces device in DRHD.

Yes.

> 		 But the namespace device in RMRR is not enumerated, right?

It should be probed in probe_acpi_namespace_devices().

Best regards,
baolu

> 
> Best regards
> Felix cui-oc
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: FelixCui-oc
> 发送时间: 2020年8月3日 17:02
> 收件人: 'Lu Baolu' <baolu.lu@linux.intel.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi  baolu:
> 		"The namespace devices are enumerated in probe_acpi_namespace_devices().
> 		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
> 		
> 		This situation only applies to the physical node of the namespaces device as the pci device.
> 		In fact, the physical node of the namespaces device can be a platform device or NULL.
> 		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
> 		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.
> 
> Best regards
> Felix cui
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 16:26
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> On 2020/8/3 14:52, FelixCui-oc wrote:
>> Hi  baolu ,
>> 		Yes ,that's right.
>> 		This patch is to achieve acpi namespace devices to access the RMRR region.
> 
> The namespace devices are enumerated in probe_acpi_namespace_devices().
> It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?
> 
> Best regards,
> baolu
> 
>>
>> Best regards
>> Felix cui
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 14:19
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi,
>>
>> On 2020/8/3 12:40, FelixCui-oc wrote:
>>> Hi baolu:
>>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
>>
>> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
>>
>> Do I understand it right?
>>
>> Best regards,
>> baolu
>>
>>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>>
>>> Best regards
>>> FelixCui-oc
>>>
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>> 发送时间: 2020年8月3日 10:32
>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD)
>>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) <RaymondPang@zhaoxin.com>
>>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>
>>> Hi,
>>>
>>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>>> Some ACPI devices require access to the specified reserved memory
>>>> region.BIOS report the specified reserved memory region through RMRR
>>>> structures.Add analysis of ACPI device in RMRR and establish
>>>> identity mapping for ACPI device.
>>>
>>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>>
>>> Nit: add version and change log once you refreshed your patch.
>>>
>>>>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>>
>>> Best regards,
>>> baolu
>>>
>>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>>> ---
>>>>      drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>>      drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>>      drivers/iommu/iommu.c       |  6 +++
>>>>      include/linux/dmar.h        | 12 +++++-
>>>>      include/linux/iommu.h       |  3 ++
>>>>      5 files changed, 105 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>>>> index 93e6345f3414..024ca38dba12 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); @@ -696,48 +696,56 @@
>>>> 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)
>>>> +int 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 1;
>>>> +			}
>>>> +		WARN_ON(i >= devices_cnt);
>>>> +	}
>>>> +	return 0;
>>>> +}
>>>> +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 = 0;
>>>> +
>>>>      	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;
>>>> +	}
>>>> +	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
>>>> +774,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..be1793415326
>>>> 100644
>>>> --- a/drivers/iommu/intel/iommu.c
>>>> +++ b/drivers/iommu/intel/iommu.c
>>>> @@ -4507,6 +4507,24 @@ 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)
>>>>      {
>>>> @@ -4523,7 +4541,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 +4559,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);
>>>> @@ -4779,6 +4797,26 @@ 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) {
>>>> +	int ret;
>>>> +	struct iommu_group *group;
>>>> +
>>>> +	if (pn_dev == NULL) {
>>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>> +		ret = iommu_probe_device(acpi_device);
>>>> +		if (ret) {
>>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>>> +			return ret;
>>>> +		}
>>>> +		return 0;
>>>> +	}
>>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>> +	group = iommu_group_get(pn_dev);
>>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>>> +
>>>> +	return 0;
>>>> +}
>>>>      
>>>>      static int __init probe_acpi_namespace_devices(void)
>>>>      {
>>>> @@ -4794,6 +4832,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;
>>>> @@ -4804,6 +4843,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;
>>>>      				}
>>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>>      				ret = iommu_probe_device(pn->dev);
>>>>      				if (ret)
>>>>      					break;
>>>> +				pn_dev = pn->dev;
>>>>      			}
>>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>>      			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/dmar.h
>>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>>> 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] 18+ messages in thread

* 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-04  1:11             ` Lu Baolu
@ 2020-08-04  3:11               ` FelixCui-oc
  2020-08-05  2:56                 ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-04  3:11 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi  baolu ,
		When creating a identity mapping for a namespace device in RMRR, you need to add the namespace device to the rmrr->device[] , right?
		The dmar_acpi_bus_add_dev() in patch adds the enumeration of the namespace device in RMRR. This is similar to 
enumerating pci devices. Do you think this is unreasonable? If it is unreasonable, please tell me why it is unreasonable.
			
Best regards
Felix cui-oc



-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月4日 9:12
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi Felix,

On 2020/8/3 17:41, FelixCui-oc wrote:
> Hi baolu,
> 		 dmar_acpi_dev_scope_init() parse ANDD structure and enumerated namespaces device in DRHD.

Yes.

> 		 But the namespace device in RMRR is not enumerated, right?

It should be probed in probe_acpi_namespace_devices().

Best regards,
baolu

> 
> Best regards
> Felix cui-oc
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: FelixCui-oc
> 发送时间: 2020年8月3日 17:02
> 收件人: 'Lu Baolu' <baolu.lu@linux.intel.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc 
> <CobeChen-oc@zhaoxin.com>
> 主题: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi  baolu:
> 		"The namespace devices are enumerated in probe_acpi_namespace_devices().
> 		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
> 		
> 		This situation only applies to the physical node of the namespaces device as the pci device.
> 		In fact, the physical node of the namespaces device can be a platform device or NULL.
> 		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
> 		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.
> 
> Best regards
> Felix cui
> 
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月3日 16:26
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> On 2020/8/3 14:52, FelixCui-oc wrote:
>> Hi  baolu ,
>> 		Yes ,that's right.
>> 		This patch is to achieve acpi namespace devices to access the RMRR region.
> 
> The namespace devices are enumerated in probe_acpi_namespace_devices().
> It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?
> 
> Best regards,
> baolu
> 
>>
>> Best regards
>> Felix cui
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 14:19
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi,
>>
>> On 2020/8/3 12:40, FelixCui-oc wrote:
>>> Hi baolu:
>>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
>>
>> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
>>
>> Do I understand it right?
>>
>> Best regards,
>> baolu
>>
>>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>>
>>> Best regards
>>> FelixCui-oc
>>>
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>> 发送时间: 2020年8月3日 10:32
>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) 
>>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) 
>>> <RaymondPang@zhaoxin.com>
>>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>
>>> Hi,
>>>
>>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>>> Some ACPI devices require access to the specified reserved memory 
>>>> region.BIOS report the specified reserved memory region through 
>>>> RMRR structures.Add analysis of ACPI device in RMRR and establish 
>>>> identity mapping for ACPI device.
>>>
>>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>>
>>> Nit: add version and change log once you refreshed your patch.
>>>
>>>>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>>
>>> Best regards,
>>> baolu
>>>
>>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>>> ---
>>>>      drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>>      drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>>      drivers/iommu/iommu.c       |  6 +++
>>>>      include/linux/dmar.h        | 12 +++++-
>>>>      include/linux/iommu.h       |  3 ++
>>>>      5 files changed, 105 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/intel/dmar.c 
>>>> b/drivers/iommu/intel/dmar.c index 93e6345f3414..024ca38dba12 
>>>> 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); @@ -696,48 +696,56 @@ 
>>>> 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)
>>>> +int 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 1;
>>>> +			}
>>>> +		WARN_ON(i >= devices_cnt);
>>>> +	}
>>>> +	return 0;
>>>> +}
>>>> +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 = 0;
>>>> +
>>>>      	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;
>>>> +	}
>>>> +	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
>>>> +774,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..be1793415326
>>>> 100644
>>>> --- a/drivers/iommu/intel/iommu.c
>>>> +++ b/drivers/iommu/intel/iommu.c
>>>> @@ -4507,6 +4507,24 @@ 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)
>>>>      {
>>>> @@ -4523,7 +4541,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 +4559,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);
>>>> @@ -4779,6 +4797,26 @@ 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) {
>>>> +	int ret;
>>>> +	struct iommu_group *group;
>>>> +
>>>> +	if (pn_dev == NULL) {
>>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>> +		ret = iommu_probe_device(acpi_device);
>>>> +		if (ret) {
>>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>>> +			return ret;
>>>> +		}
>>>> +		return 0;
>>>> +	}
>>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>> +	group = iommu_group_get(pn_dev);
>>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>>> +
>>>> +	return 0;
>>>> +}
>>>>      
>>>>      static int __init probe_acpi_namespace_devices(void)
>>>>      {
>>>> @@ -4794,6 +4832,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;
>>>> @@ -4804,6 +4843,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;
>>>>      				}
>>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>>      				ret = iommu_probe_device(pn->dev);
>>>>      				if (ret)
>>>>      					break;
>>>> +				pn_dev = pn->dev;
>>>>      			}
>>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>>      			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/dmar.h 
>>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>>> 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] 18+ messages in thread

* Re: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-04  3:11               ` 答复: " FelixCui-oc
@ 2020-08-05  2:56                 ` Lu Baolu
  2020-08-05  7:37                   ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-05  2:56 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi,

On 8/4/20 11:11 AM, FelixCui-oc wrote:
> Hi  baolu ,
> 		When creating a identity mapping for a namespace device in RMRR, you need to add the namespace device to the rmrr->device[] , right?

Yes. You are right.

> 		The dmar_acpi_bus_add_dev() in patch adds the enumeration of the namespace device in RMRR. This is similar to > enumerating pci devices. Do you think this is unreasonable? If it is 
unreasonable, please tell me why it is unreasonable.

It looks reasonable. Thanks for the explanation.

But I don't think we need to add acpi_device_create_direct_mappings()
since the rmrr identity mapping is already done in the iommu core.

Best regards,
baolu

> 			
> Best regards
> Felix cui-oc
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月4日 9:12
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi Felix,
> 
> On 2020/8/3 17:41, FelixCui-oc wrote:
>> Hi baolu,
>> 		 dmar_acpi_dev_scope_init() parse ANDD structure and enumerated namespaces device in DRHD.
> 
> Yes.
> 
>> 		 But the namespace device in RMRR is not enumerated, right?
> 
> It should be probed in probe_acpi_namespace_devices().
> 
> Best regards,
> baolu
> 
>>
>> Best regards
>> Felix cui-oc
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: FelixCui-oc
>> 发送时间: 2020年8月3日 17:02
>> 收件人: 'Lu Baolu' <baolu.lu@linux.intel.com>; Joerg Roedel
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc
>> <CobeChen-oc@zhaoxin.com>
>> 主题: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> Hi  baolu:
>> 		"The namespace devices are enumerated in probe_acpi_namespace_devices().
>> 		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
>> 		
>> 		This situation only applies to the physical node of the namespaces device as the pci device.
>> 		In fact, the physical node of the namespaces device can be a platform device or NULL.
>> 		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
>> 		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.
>>
>> Best regards
>> Felix cui
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 16:26
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>
>> On 2020/8/3 14:52, FelixCui-oc wrote:
>>> Hi  baolu ,
>>> 		Yes ,that's right.
>>> 		This patch is to achieve acpi namespace devices to access the RMRR region.
>>
>> The namespace devices are enumerated in probe_acpi_namespace_devices().
>> It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?
>>
>> Best regards,
>> baolu
>>
>>>
>>> Best regards
>>> Felix cui
>>>
>>>
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>> 发送时间: 2020年8月3日 14:19
>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc
>>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>>> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>
>>> Hi,
>>>
>>> On 2020/8/3 12:40, FelixCui-oc wrote:
>>>> Hi baolu:
>>>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>>>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>>>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>>>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
>>>
>>> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
>>>
>>> Do I understand it right?
>>>
>>> Best regards,
>>> baolu
>>>
>>>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>>>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>>>
>>>> Best regards
>>>> FelixCui-oc
>>>>
>>>>
>>>>
>>>> -----邮件原件-----
>>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>>> 发送时间: 2020年8月3日 10:32
>>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD)
>>>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD)
>>>> <RaymondPang@zhaoxin.com>
>>>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>>
>>>> Hi,
>>>>
>>>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>>>> Some ACPI devices require access to the specified reserved memory
>>>>> region.BIOS report the specified reserved memory region through
>>>>> RMRR structures.Add analysis of ACPI device in RMRR and establish
>>>>> identity mapping for ACPI device.
>>>>
>>>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>>>
>>>> Nit: add version and change log once you refreshed your patch.
>>>>
>>>>>
>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>>>
>>>> Best regards,
>>>> baolu
>>>>
>>>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>>>> ---
>>>>>       drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>>>       drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>>>       drivers/iommu/iommu.c       |  6 +++
>>>>>       include/linux/dmar.h        | 12 +++++-
>>>>>       include/linux/iommu.h       |  3 ++
>>>>>       5 files changed, 105 insertions(+), 36 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iommu/intel/dmar.c
>>>>> b/drivers/iommu/intel/dmar.c index 93e6345f3414..024ca38dba12
>>>>> 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); @@ -696,48 +696,56 @@
>>>>> 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)
>>>>> +int 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 1;
>>>>> +			}
>>>>> +		WARN_ON(i >= devices_cnt);
>>>>> +	}
>>>>> +	return 0;
>>>>> +}
>>>>> +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 = 0;
>>>>> +
>>>>>       	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;
>>>>> +	}
>>>>> +	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
>>>>> +774,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..be1793415326
>>>>> 100644
>>>>> --- a/drivers/iommu/intel/iommu.c
>>>>> +++ b/drivers/iommu/intel/iommu.c
>>>>> @@ -4507,6 +4507,24 @@ 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)
>>>>>       {
>>>>> @@ -4523,7 +4541,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 +4559,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);
>>>>> @@ -4779,6 +4797,26 @@ 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) {
>>>>> +	int ret;
>>>>> +	struct iommu_group *group;
>>>>> +
>>>>> +	if (pn_dev == NULL) {
>>>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>>> +		ret = iommu_probe_device(acpi_device);
>>>>> +		if (ret) {
>>>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>>>> +			return ret;
>>>>> +		}
>>>>> +		return 0;
>>>>> +	}
>>>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>>> +	group = iommu_group_get(pn_dev);
>>>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>>       
>>>>>       static int __init probe_acpi_namespace_devices(void)
>>>>>       {
>>>>> @@ -4794,6 +4832,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;
>>>>> @@ -4804,6 +4843,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;
>>>>>       				}
>>>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>>>       				ret = iommu_probe_device(pn->dev);
>>>>>       				if (ret)
>>>>>       					break;
>>>>> +				pn_dev = pn->dev;
>>>>>       			}
>>>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>>>       			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/dmar.h
>>>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>>>> 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] 18+ messages in thread

* 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-05  2:56                 ` Lu Baolu
@ 2020-08-05  7:37                   ` FelixCui-oc
  2020-08-06  2:36                     ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-05  7:37 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi baolu,
		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.

		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.
		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices(). 
		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.

Best regards
Felix cui-oc




-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月5日 10:57
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi,

On 8/4/20 11:11 AM, FelixCui-oc wrote:
> Hi  baolu ,
> 		When creating a identity mapping for a namespace device in RMRR, you need to add the namespace device to the rmrr->device[] , right?

Yes. You are right.

> 		The dmar_acpi_bus_add_dev() in patch adds the enumeration of the 
> namespace device in RMRR. This is similar to > enumerating pci 
> devices. Do you think this is unreasonable? If it is
unreasonable, please tell me why it is unreasonable.

It looks reasonable. Thanks for the explanation.

But I don't think we need to add acpi_device_create_direct_mappings()
since the rmrr identity mapping is already done in the iommu core.

Best regards,
baolu

> 			
> Best regards
> Felix cui-oc
> 
> 
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月4日 9:12
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in 
> RMRR
> 
> Hi Felix,
> 
> On 2020/8/3 17:41, FelixCui-oc wrote:
>> Hi baolu,
>> 		 dmar_acpi_dev_scope_init() parse ANDD structure and enumerated namespaces device in DRHD.
> 
> Yes.
> 
>> 		 But the namespace device in RMRR is not enumerated, right?
> 
> It should be probed in probe_acpi_namespace_devices().
> 
> Best regards,
> baolu
> 
>>
>> Best regards
>> Felix cui-oc
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: FelixCui-oc
>> 发送时间: 2020年8月3日 17:02
>> 收件人: 'Lu Baolu' <baolu.lu@linux.intel.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc 
>> <CobeChen-oc@zhaoxin.com>
>> 主题: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in 
>> RMRR
>>
>> Hi  baolu:
>> 		"The namespace devices are enumerated in probe_acpi_namespace_devices().
>> 		It calls iommu_probe_device() to process the enumeration and setup the identity mappings."
>> 		
>> 		This situation only applies to the physical node of the namespaces device as the pci device.
>> 		In fact, the physical node of the namespaces device can be a platform device or NULL.
>> 		If the physical node of the namespaces is a platform device or NULL, it has not actually been enumerated.
>> 		So it is necessary to increase the analysis of the namespaces device in RMRR and establish an identity mapping.
>>
>> Best regards
>> Felix cui
>>
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月3日 16:26
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in 
>> RMRR
>>
>> On 2020/8/3 14:52, FelixCui-oc wrote:
>>> Hi  baolu ,
>>> 		Yes ,that's right.
>>> 		This patch is to achieve acpi namespace devices to access the RMRR region.
>>
>> The namespace devices are enumerated in probe_acpi_namespace_devices().
>> It calls iommu_probe_device() to process the enumeration and setup the identity mappings. Can you please check why that code doesn't work for you?
>>
>> Best regards,
>> baolu
>>
>>>
>>> Best regards
>>> Felix cui
>>>
>>>
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>> 发送时间: 2020年8月3日 14:19
>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
>>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>>> 主题: Re: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>
>>> Hi,
>>>
>>> On 2020/8/3 12:40, FelixCui-oc wrote:
>>>> Hi baolu:
>>>> 		Some ACPI devices need to issue dma requests to access the reserved memory area.
>>>> 		So bios uses the device scope type ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
>>>> 		At present, there is no analysis in the kernel that the device scope type in RMRR is ACPI_NAMESPACE_DEVICE.
>>>> 		This patch is mainly to add the analysis of the device scope type ACPI_NAMESPACE_DEVICE in RMRR structure and establish identity mapping for > these ACPI devices.
>>>
>>> So the problem is "although namespace devices in RMRR have been parsed, but the identity map for those devices aren't created. As the result, the namespace devices fail to access the RMRR region."
>>>
>>> Do I understand it right?
>>>
>>> Best regards,
>>> baolu
>>>
>>>> In addition, some naming changes have been made in patch in order to distinguish acpi device from pci device.
>>>> 		You can refer to the description of type in 8.3.1 device scope in vt-d spec.
>>>>
>>>> Best regards
>>>> FelixCui-oc
>>>>
>>>>
>>>>
>>>> -----邮件原件-----
>>>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>>>> 发送时间: 2020年8月3日 10:32
>>>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>>>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>>>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>>>> 抄送: baolu.lu@linux.intel.com; Cobe Chen(BJ-RD) 
>>>> <CobeChen@zhaoxin.com>; Raymond Pang(BJ-RD) 
>>>> <RaymondPang@zhaoxin.com>
>>>> 主题: Re: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
>>>>
>>>> Hi,
>>>>
>>>> On 8/2/20 6:07 PM, FelixCuioc wrote:
>>>>> Some ACPI devices require access to the specified reserved memory 
>>>>> region.BIOS report the specified reserved memory region through 
>>>>> RMRR structures.Add analysis of ACPI device in RMRR and establish 
>>>>> identity mapping for ACPI device.
>>>>
>>>> Can you please add more words about the problem you want to solve? Do you mean some RMRRs are not enumerated correctly? Or, enumerated, but not identity mapped?
>>>>
>>>> Nit: add version and change log once you refreshed your patch.
>>>>
>>>>>
>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> No need to add this. The problem you want to solve through this patch is not reported by lkp.
>>>>
>>>> Best regards,
>>>> baolu
>>>>
>>>>> Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
>>>>> ---
>>>>>       drivers/iommu/intel/dmar.c  | 74 ++++++++++++++++++++-----------------
>>>>>       drivers/iommu/intel/iommu.c | 46 ++++++++++++++++++++++-
>>>>>       drivers/iommu/iommu.c       |  6 +++
>>>>>       include/linux/dmar.h        | 12 +++++-
>>>>>       include/linux/iommu.h       |  3 ++
>>>>>       5 files changed, 105 insertions(+), 36 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iommu/intel/dmar.c 
>>>>> b/drivers/iommu/intel/dmar.c index 93e6345f3414..024ca38dba12
>>>>> 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); @@ -696,48 +696,56 
>>>>> @@ 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)
>>>>> +int 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 1;
>>>>> +			}
>>>>> +		WARN_ON(i >= devices_cnt);
>>>>> +	}
>>>>> +	return 0;
>>>>> +}
>>>>> +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 = 0;
>>>>> +
>>>>>       	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;
>>>>> +	}
>>>>> +	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
>>>>> +774,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..be1793415326
>>>>> 100644
>>>>> --- a/drivers/iommu/intel/iommu.c
>>>>> +++ b/drivers/iommu/intel/iommu.c
>>>>> @@ -4507,6 +4507,24 @@ 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)
>>>>>       {
>>>>> @@ -4523,7 +4541,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 +4559,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);
>>>>> @@ -4779,6 +4797,26 @@ 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) {
>>>>> +	int ret;
>>>>> +	struct iommu_group *group;
>>>>> +
>>>>> +	if (pn_dev == NULL) {
>>>>> +		acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>>> +		ret = iommu_probe_device(acpi_device);
>>>>> +		if (ret) {
>>>>> +			pr_err("acpi_device probe fail! ret:%d\n", ret);
>>>>> +			return ret;
>>>>> +		}
>>>>> +		return 0;
>>>>> +	}
>>>>> +	acpi_device->bus->iommu_ops = &intel_iommu_ops;
>>>>> +	group = iommu_group_get(pn_dev);
>>>>> +	__acpi_device_create_direct_mappings(group, acpi_device);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>>       
>>>>>       static int __init probe_acpi_namespace_devices(void)
>>>>>       {
>>>>> @@ -4794,6 +4832,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;
>>>>> @@ -4804,6 +4843,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;
>>>>>       				}
>>>>> @@ -4812,7 +4852,9 @@ static int __init probe_acpi_namespace_devices(void)
>>>>>       				ret = iommu_probe_device(pn->dev);
>>>>>       				if (ret)
>>>>>       					break;
>>>>> +				pn_dev = pn->dev;
>>>>>       			}
>>>>> +			ret = acpi_device_create_direct_mappings(pn_dev, dev);
>>>>>       			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/dmar.h 
>>>>> b/include/linux/dmar.h index 65565820328a..881ac61a4336 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 int 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;
>>>>> 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] 18+ messages in thread

* Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-05  7:37                   ` 答复: " FelixCui-oc
@ 2020-08-06  2:36                     ` Lu Baolu
  2020-08-06  6:51                       ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-06  2:36 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi Felix,

On 8/5/20 3:37 PM, FelixCui-oc wrote:
> Hi baolu,
> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.

Sure. Before that, let me sync my understanding with you. You have an
acpi namespace device in ANDD table, it also shows up in the device
scope of a RMRR. Current code doesn't enumerate that device for the
RMRR, hence iommu_create_device_direct_mappings() doesn't work for this
device.

At the same time, probe_acpi_namespace_devices() doesn't work for this
device, hence you want to add a home-made
acpi_device_create_direct_mappings() helper.

Did I get it right?

> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.

The namespace device has been probed and put in one drhd's device list.
Hence, it should be processed by probe_acpi_namespace_devices(). So the
question is why there are no devices in addev->physical_node_list?

> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.

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

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

* 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-06  2:36                     ` Lu Baolu
@ 2020-08-06  6:51                       ` FelixCui-oc
  2020-08-07  1:07                         ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-06  6:51 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi  baolu,
		>Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. 
		>Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.

		>At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
		>acpi_device_create_direct_mappings() helper.

		Your understanding is right. 
		But there is a problem that even if the namespace device in rmrr is enumerated in the code, probe_acpi_namespace_devices() also doesn't work for this device.
		This is because the dev parameter of the iommu_create_device_direct_mappings() is not the namespace device in RMRR.
		The actual parameter passed in is the namespace device's physical node device.
		In iommu_create_device_direct_mappings(), the physical node device passed in cannot match the namespace device in rmrr->device[],right?
		We need acpi_device_create_direct_mappings() helper ? 

		In addition, adev->physical_node_list is related to the __HID of namespace device reported by the bios.
		For example, if the __HID reported by the bios belongs to acpi_pnp_device_ids[], adev->physical_node_list has no devices.
		So in acpi_device_create_direct_mappings(), I added the case that adev->physical_node_list is empty.


Best regards
Felix cui


 

-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月6日 10:36
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi Felix,

On 8/5/20 3:37 PM, FelixCui-oc wrote:
> Hi baolu,
> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.

Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.

At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
acpi_device_create_direct_mappings() helper.

Did I get it right?

> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.

The namespace device has been probed and put in one drhd's device list.
Hence, it should be processed by probe_acpi_namespace_devices(). So the question is why there are no devices in addev->physical_node_list?

> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.

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

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

* Re: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-06  6:51                       ` 答复: " FelixCui-oc
@ 2020-08-07  1:07                         ` Lu Baolu
  2020-08-07  3:29                           ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-07  1:07 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi Felix,

On 2020/8/6 14:51, FelixCui-oc wrote:
> Hi  baolu,
> 		>Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR.
> 		>Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
> 
> 		>At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
> 		>acpi_device_create_direct_mappings() helper.
> 
> 		Your understanding is right.
> 		But there is a problem that even if the namespace device in rmrr is enumerated in the code, probe_acpi_namespace_devices() also doesn't work for this device.
> 		This is because the dev parameter of the iommu_create_device_direct_mappings() is not the namespace device in RMRR.
> 		The actual parameter passed in is the namespace device's physical node device.
> 		In iommu_create_device_direct_mappings(), the physical node device passed in cannot match the namespace device in rmrr->device[],right?
> 		We need acpi_device_create_direct_mappings() helper ?
> 
> 		In addition, adev->physical_node_list is related to the __HID of namespace device reported by the bios.
> 		For example, if the __HID reported by the bios belongs to acpi_pnp_device_ids[], adev->physical_node_list has no devices.
> 		So in acpi_device_create_direct_mappings(), I added the case that adev->physical_node_list is empty.

Got you. Thanks!

Have you ever tried to have probe_acpi_namespace_devices() handle the
case of empty adev->physical_node_list at the same time?

Best regards,
baolu

> 
> 
> Best regards
> Felix cui
> 
> 
>   
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月6日 10:36
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi Felix,
> 
> On 8/5/20 3:37 PM, FelixCui-oc wrote:
>> Hi baolu,
>> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.
> 
> Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
> 
> At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
> acpi_device_create_direct_mappings() helper.
> 
> Did I get it right?
> 
>> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
>> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.
> 
> The namespace device has been probed and put in one drhd's device list.
> Hence, it should be processed by probe_acpi_namespace_devices(). So the question is why there are no devices in addev->physical_node_list?
> 
>> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
>> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.
> 
> Best regards,
> baolu
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* 答复: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-07  1:07                         ` Lu Baolu
@ 2020-08-07  3:29                           ` FelixCui-oc
  2020-08-07  8:08                             ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: FelixCui-oc @ 2020-08-07  3:29 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

Hi baolu,
		I understand what you mean is that you want to put the following processing code in the acpi_device_create_direct_mappings() into the probe_acpi_namespace_devices() ,right?
		If you mean it , I think it's OK. 

	if (pn_dev == NULL) {
		acpi_device->bus->iommu_ops = &intel_iommu_ops;
		ret = iommu_probe_device(acpi_device);
		if (ret) {
			pr_err("acpi_device probe fail! ret:%d\n", ret);
			return ret;
		}
		return 0;
	}

Best regards
Felix cui-oc



	

-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月7日 9:08
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi Felix,

On 2020/8/6 14:51, FelixCui-oc wrote:
> Hi  baolu,
> 		>Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR.
> 		>Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
> 
> 		>At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
> 		>acpi_device_create_direct_mappings() helper.
> 
> 		Your understanding is right.
> 		But there is a problem that even if the namespace device in rmrr is enumerated in the code, probe_acpi_namespace_devices() also doesn't work for this device.
> 		This is because the dev parameter of the iommu_create_device_direct_mappings() is not the namespace device in RMRR.
> 		The actual parameter passed in is the namespace device's physical node device.
> 		In iommu_create_device_direct_mappings(), the physical node device passed in cannot match the namespace device in rmrr->device[],right?
> 		We need acpi_device_create_direct_mappings() helper ?
> 
> 		In addition, adev->physical_node_list is related to the __HID of namespace device reported by the bios.
> 		For example, if the __HID reported by the bios belongs to acpi_pnp_device_ids[], adev->physical_node_list has no devices.
> 		So in acpi_device_create_direct_mappings(), I added the case that adev->physical_node_list is empty.

Got you. Thanks!

Have you ever tried to have probe_acpi_namespace_devices() handle the case of empty adev->physical_node_list at the same time?

Best regards,
baolu

> 
> 
> Best regards
> Felix cui
> 
> 
>   
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月6日 10:36
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI 
> device in RMRR
> 
> Hi Felix,
> 
> On 8/5/20 3:37 PM, FelixCui-oc wrote:
>> Hi baolu,
>> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.
> 
> Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
> 
> At the same time, probe_acpi_namespace_devices() doesn't work for this 
> device, hence you want to add a home-made
> acpi_device_create_direct_mappings() helper.
> 
> Did I get it right?
> 
>> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
>> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.
> 
> The namespace device has been probed and put in one drhd's device list.
> Hence, it should be processed by probe_acpi_namespace_devices(). So the question is why there are no devices in addev->physical_node_list?
> 
>> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
>> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.
> 
> Best regards,
> baolu
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: 答复: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-07  3:29                           ` 答复: " FelixCui-oc
@ 2020-08-07  8:08                             ` Lu Baolu
  2020-08-08  7:14                               ` 答复: " FelixCui-oc
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2020-08-07  8:08 UTC (permalink / raw)
  To: FelixCui-oc, Joerg Roedel, iommu, linux-kernel, David Woodhouse
  Cc: CobeChen-oc

Hi,

On 2020/8/7 11:29, FelixCui-oc wrote:
> Hi baolu,
> 		I understand what you mean is that you want to put the following processing code in the acpi_device_create_direct_mappings() into the probe_acpi_namespace_devices() ,right?
> 		If you mean it , I think it's OK.

Yes. If the acpi device has companion physical devices, then probe the
companion devices; otherwise, probe the acpi device instead. Also, let's
put detecting and probing acpi devices in different patches so that they
could be easier for reviewing.

Best regards,
baolu

> 
> 	if (pn_dev == NULL) {
> 		acpi_device->bus->iommu_ops = &intel_iommu_ops;
> 		ret = iommu_probe_device(acpi_device);
> 		if (ret) {
> 			pr_err("acpi_device probe fail! ret:%d\n", ret);
> 			return ret;
> 		}
> 		return 0;
> 	}
> 
> Best regards
> Felix cui-oc
> 
> 
> 
> 	
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月7日 9:08
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
> 
> Hi Felix,
> 
> On 2020/8/6 14:51, FelixCui-oc wrote:
>> Hi  baolu,
>> 		>Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR.
>> 		>Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
>>
>> 		>At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
>> 		>acpi_device_create_direct_mappings() helper.
>>
>> 		Your understanding is right.
>> 		But there is a problem that even if the namespace device in rmrr is enumerated in the code, probe_acpi_namespace_devices() also doesn't work for this device.
>> 		This is because the dev parameter of the iommu_create_device_direct_mappings() is not the namespace device in RMRR.
>> 		The actual parameter passed in is the namespace device's physical node device.
>> 		In iommu_create_device_direct_mappings(), the physical node device passed in cannot match the namespace device in rmrr->device[],right?
>> 		We need acpi_device_create_direct_mappings() helper ?
>>
>> 		In addition, adev->physical_node_list is related to the __HID of namespace device reported by the bios.
>> 		For example, if the __HID reported by the bios belongs to acpi_pnp_device_ids[], adev->physical_node_list has no devices.
>> 		So in acpi_device_create_direct_mappings(), I added the case that adev->physical_node_list is empty.
> 
> Got you. Thanks!
> 
> Have you ever tried to have probe_acpi_namespace_devices() handle the case of empty adev->physical_node_list at the same time?
> 
> Best regards,
> baolu
> 
>>
>>
>> Best regards
>> Felix cui
>>
>>
>>    
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月6日 10:36
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org;
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI
>> device in RMRR
>>
>> Hi Felix,
>>
>> On 8/5/20 3:37 PM, FelixCui-oc wrote:
>>> Hi baolu,
>>> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.
>>
>> Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
>>
>> At the same time, probe_acpi_namespace_devices() doesn't work for this
>> device, hence you want to add a home-made
>> acpi_device_create_direct_mappings() helper.
>>
>> Did I get it right?
>>
>>> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
>>> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.
>>
>> The namespace device has been probed and put in one drhd's device list.
>> Hence, it should be processed by probe_acpi_namespace_devices(). So the question is why there are no devices in addev->physical_node_list?
>>
>>> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
>>> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.
>>
>> Best regards,
>> baolu
>>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* 答复: 答复: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR
  2020-08-07  8:08                             ` Lu Baolu
@ 2020-08-08  7:14                               ` FelixCui-oc
  0 siblings, 0 replies; 18+ messages in thread
From: FelixCui-oc @ 2020-08-08  7:14 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, iommu, linux-kernel, David Woodhouse; +Cc: CobeChen-oc

[-- Attachment #1: Type: text/plain, Size: 5820 bytes --]

Hi baolu,
		The attachment is the new patch.
		If you have any questions, you can contact me at any time.
		Thanks.

Best regards
Felix cui-oc




-----邮件原件-----
发件人: Lu Baolu <baolu.lu@linux.intel.com> 
发送时间: 2020年8月7日 16:08
收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel <joro@8bytes.org>; iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
抄送: baolu.lu@linux.intel.com; RaymondPang-oc <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
主题: Re: 答复: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI device in RMRR

Hi,

On 2020/8/7 11:29, FelixCui-oc wrote:
> Hi baolu,
> 		I understand what you mean is that you want to put the following processing code in the acpi_device_create_direct_mappings() into the probe_acpi_namespace_devices() ,right?
> 		If you mean it , I think it's OK.

Yes. If the acpi device has companion physical devices, then probe the companion devices; otherwise, probe the acpi device instead. Also, let's put detecting and probing acpi devices in different patches so that they could be easier for reviewing.

Best regards,
baolu

> 
> 	if (pn_dev == NULL) {
> 		acpi_device->bus->iommu_ops = &intel_iommu_ops;
> 		ret = iommu_probe_device(acpi_device);
> 		if (ret) {
> 			pr_err("acpi_device probe fail! ret:%d\n", ret);
> 			return ret;
> 		}
> 		return 0;
> 	}
> 
> Best regards
> Felix cui-oc
> 
> 
> 
> 	
> 
> -----邮件原件-----
> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
> 发送时间: 2020年8月7日 9:08
> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
> 主题: Re: 答复: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for 
> ACPI device in RMRR
> 
> Hi Felix,
> 
> On 2020/8/6 14:51, FelixCui-oc wrote:
>> Hi  baolu,
>> 		>Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR.
>> 		>Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
>>
>> 		>At the same time, probe_acpi_namespace_devices() doesn't work for this device, hence you want to add a home-made
>> 		>acpi_device_create_direct_mappings() helper.
>>
>> 		Your understanding is right.
>> 		But there is a problem that even if the namespace device in rmrr is enumerated in the code, probe_acpi_namespace_devices() also doesn't work for this device.
>> 		This is because the dev parameter of the iommu_create_device_direct_mappings() is not the namespace device in RMRR.
>> 		The actual parameter passed in is the namespace device's physical node device.
>> 		In iommu_create_device_direct_mappings(), the physical node device passed in cannot match the namespace device in rmrr->device[],right?
>> 		We need acpi_device_create_direct_mappings() helper ?
>>
>> 		In addition, adev->physical_node_list is related to the __HID of namespace device reported by the bios.
>> 		For example, if the __HID reported by the bios belongs to acpi_pnp_device_ids[], adev->physical_node_list has no devices.
>> 		So in acpi_device_create_direct_mappings(), I added the case that adev->physical_node_list is empty.
> 
> Got you. Thanks!
> 
> Have you ever tried to have probe_acpi_namespace_devices() handle the case of empty adev->physical_node_list at the same time?
> 
> Best regards,
> baolu
> 
>>
>>
>> Best regards
>> Felix cui
>>
>>
>>    
>>
>> -----邮件原件-----
>> 发件人: Lu Baolu <baolu.lu@linux.intel.com>
>> 发送时间: 2020年8月6日 10:36
>> 收件人: FelixCui-oc <FelixCui-oc@zhaoxin.com>; Joerg Roedel 
>> <joro@8bytes.org>; iommu@lists.linux-foundation.org; 
>> linux-kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>
>> 抄送: baolu.lu@linux.intel.com; RaymondPang-oc 
>> <RaymondPang-oc@zhaoxin.com>; CobeChen-oc <CobeChen-oc@zhaoxin.com>
>> 主题: Re: 答复: 答复: 答复: 答复: 答复: [PATCH] iommu/vt-d:Add support for ACPI 
>> device in RMRR
>>
>> Hi Felix,
>>
>> On 8/5/20 3:37 PM, FelixCui-oc wrote:
>>> Hi baolu,
>>> 		Let me talk about why acpi_device_create_direct_mappings() is needed and please tell me if there is an error.
>>
>> Sure. Before that, let me sync my understanding with you. You have an acpi namespace device in ANDD table, it also shows up in the device scope of a RMRR. Current code doesn't enumerate that device for the RMRR, hence iommu_create_device_direct_mappings() doesn't work for this device.
>>
>> At the same time, probe_acpi_namespace_devices() doesn't work for 
>> this device, hence you want to add a home-made
>> acpi_device_create_direct_mappings() helper.
>>
>> Did I get it right?
>>
>>> 		In the probe_acpi_namespace_devices() function, only the device in the addev->physical_node_list is probed,
>>> 		but we need to establish identity mapping for the namespace device in RMRR. These are two different devices.
>>
>> The namespace device has been probed and put in one drhd's device list.
>> Hence, it should be processed by probe_acpi_namespace_devices(). So the question is why there are no devices in addev->physical_node_list?
>>
>>> 		Therefore, the namespace device in RMRR is not mapped in probe_acpi_namespace_devices().
>>> 		acpi_device_create_direct_mappings() is to create direct mappings for namespace devices in RMRR.
>>
>> Best regards,
>> baolu
>>

[-- Attachment #2: 0001-iommu-vt-d-Add-support-for-detecting-ACPI-device-in-.patch --]
[-- Type: application/octet-stream, Size: 8336 bytes --]

From d5bb22f767f6336e1014ff6adb86c7654287b9b4 Mon Sep 17 00:00:00 2001
From: FelixCuioc <FelixCui-oc@zhaoxin.com>
Date: Sat, 8 Aug 2020 00:56:43 -0400
Subject: [PATCH] iommu/vt-d:Add support for detecting ACPI device in RMRR

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  | 74 ++++++++++++++++++++-----------------
 drivers/iommu/intel/iommu.c | 22 ++++++++++-
 include/linux/dmar.h        | 12 +++++-
 3 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 93e6345f3414..024ca38dba12 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);
@@ -696,48 +696,56 @@ 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)
+int 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 1;
+			}
+		WARN_ON(i >= devices_cnt);
+	}
+	return 0;
+}
+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 = 0;
+
 	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;
+	}
+	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 +774,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..f774ef63d473 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4507,6 +4507,24 @@ 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)
 {
@@ -4523,7 +4541,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 +4559,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..881ac61a4336 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 int 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


[-- Attachment #3: 0001-iommu-vt-d-Add-support-for-probing-ACPI-device-in-RM.patch --]
[-- Type: application/octet-stream, Size: 3725 bytes --]

From a43a44fc775ec6fe6a7fe71b1fbe175b1e834e7c Mon Sep 17 00:00:00 2001
From: FelixCuioc <FelixCui-oc@zhaoxin.com>
Date: Sat, 8 Aug 2020 02:43:50 -0400
Subject: [PATCH] iommu/vt-d:Add support for probing ACPI device in RMRR

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 | 27 +++++++++++++++++++++++++++
 drivers/iommu/iommu.c       |  6 ++++++
 include/linux/iommu.h       |  3 +++
 3 files changed, 36 insertions(+)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index f774ef63d473..b31f02f41c96 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4797,6 +4797,20 @@ 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 -1;
+	}
+	__acpi_device_create_direct_mappings(group, acpi_device);
+
+	return 0;
+}
 
 static int __init probe_acpi_namespace_devices(void)
 {
@@ -4812,6 +4826,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;
@@ -4822,6 +4837,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;
 				}
@@ -4830,7 +4846,18 @@ static int __init probe_acpi_namespace_devices(void)
 				ret = iommu_probe_device(pn->dev);
 				if (ret)
 					break;
+				pn_dev = pn->dev;
+			}
+			if (pn_dev == NULL) {
+				dev->bus->iommu_ops = &intel_iommu_ops;
+				ret = iommu_probe_device(dev);
+				if (ret) {
+					pr_err("acpi_device probe fail! ret:%d\n", ret);
+					return ret;
+				}
+				return 0;
 			}
+			ret = acpi_device_create_direct_mappings(pn_dev, dev);
 			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


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

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

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

end of thread, other threads:[~2020-08-08  7:15 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-02 10:07 [PATCH] iommu/vt-d:Add support for ACPI device in RMRR FelixCuioc
2020-08-03  2:31 ` Lu Baolu
2020-08-03  4:40   ` 答复: " FelixCui-oc
2020-08-03  6:18     ` Lu Baolu
2020-08-03  6:52       ` 答复: " FelixCui-oc
2020-08-03  8:25         ` Lu Baolu
2020-08-03  9:01           ` 答复: " FelixCui-oc
2020-08-03  9:41           ` FelixCui-oc
2020-08-04  1:11             ` Lu Baolu
2020-08-04  3:11               ` 答复: " FelixCui-oc
2020-08-05  2:56                 ` Lu Baolu
2020-08-05  7:37                   ` 答复: " FelixCui-oc
2020-08-06  2:36                     ` Lu Baolu
2020-08-06  6:51                       ` 答复: " FelixCui-oc
2020-08-07  1:07                         ` Lu Baolu
2020-08-07  3:29                           ` 答复: " FelixCui-oc
2020-08-07  8:08                             ` Lu Baolu
2020-08-08  7:14                               ` 答复: " 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).