linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
@ 2014-09-05 11:25 Su, Friendy
  2014-09-05 13:53 ` Joerg Roedel
  0 siblings, 1 reply; 4+ messages in thread
From: Su, Friendy @ 2014-09-05 11:25 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: iommu, linux-kernel


From: Su Friendy <friendy.su@sony.com.cn>

Subject: iommu/amd: make early mapped ioapic/hpet override IVHD

The early mapped ioapic/hpet specified by kernel boot parameter
ivrs_ioapic[ID]/ivrs_hpet[ID] always override the ioapic/hpet with
same ID reported by ACPI IVHD table.

Current driver still uses devid in IVHD to set dte entry, not devid
in early mapped. This patch fixed to use devid in early mapped.

This issue is found on a mother board whose BIOS reports wrong
IOAPIC devid in IVHD table. Without this fix, the early mapped
does not really override IVHD. So that the wrong reported IOAPIC
does not work.

Signed-off-by: Su Friendy <friendy.su@sony.com.cn>
Signed-off-by: Saeki Shusuke <shusuke.saeki@jp.sony.com>
Signed-off-by: Tamori Masahiro <Masahiro.Tamori@jp.sony.com>
---
 drivers/iommu/amd_iommu_init.c |   40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 3c06183..83cb31b 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -715,7 +715,7 @@ static void __init set_dev_entry_from_acpi(struct amd_iommu *iommu,
 	set_iommu_for_device(iommu, devid);
 }
 
-static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
+static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line, u16 *early_mapped_devid)
 {
 	struct devid_map *entry;
 	struct list_head *list;
@@ -734,6 +734,19 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 		pr_info("AMD-Vi: Command-line override present for %s id %d - ignoring\n",
 			type == IVHD_SPECIAL_IOAPIC ? "IOAPIC" : "HPET", id);
 
+		/*
+		 * if this special device is added from IVHD and
+		 * its devid in IVHD != devid in early mapped, then
+		 * record early mapped devid.
+		 */
+		if (!cmd_line && devid != entry->devid) {
+			if (early_mapped_devid == NULL)
+				return -EINVAL;
+
+			*early_mapped_devid = entry->devid;
+			return -EEXIST;
+		}
+
 		return 0;
 	}
 
@@ -758,7 +771,8 @@ static int __init add_early_maps(void)
 		ret = add_special_device(IVHD_SPECIAL_IOAPIC,
 					 early_ioapic_map[i].id,
 					 early_ioapic_map[i].devid,
-					 early_ioapic_map[i].cmd_line);
+					 early_ioapic_map[i].cmd_line,
+					 NULL);
 		if (ret)
 			return ret;
 	}
@@ -767,7 +781,8 @@ static int __init add_early_maps(void)
 		ret = add_special_device(IVHD_SPECIAL_HPET,
 					 early_hpet_map[i].id,
 					 early_hpet_map[i].devid,
-					 early_hpet_map[i].cmd_line);
+					 early_hpet_map[i].cmd_line,
+					 NULL);
 		if (ret)
 			return ret;
 	}
@@ -961,7 +976,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 		case IVHD_DEV_SPECIAL: {
 			u8 handle, type;
 			const char *var;
-			u16 devid;
+			u16 devid, early_mapped_devid;
 			int ret;
 
 			handle = e->ext & 0xff;
@@ -981,10 +996,21 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 				    PCI_SLOT(devid),
 				    PCI_FUNC(devid));
 
-			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
-			ret = add_special_device(type, handle, devid, false);
-			if (ret)
+			ret = add_special_device(type, handle, devid, false, &early_mapped_devid);
+
+			/*
+			 * if the special device is already early mapped,
+			 * let the early mapped devid override IVHD.
+			 */
+			if (ret == -EEXIST) {
+				devid = early_mapped_devid;
+				ret = 0;
+			}
+
+			if (ret < 0)
 				return ret;
+
+			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
 			break;
 		}
 		default:
-- 
1.7.9.5


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

* Re: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
  2014-09-05 11:25 [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet Su, Friendy
@ 2014-09-05 13:53 ` Joerg Roedel
  2014-09-09  8:34   ` Su, Friendy
  0 siblings, 1 reply; 4+ messages in thread
From: Joerg Roedel @ 2014-09-05 13:53 UTC (permalink / raw)
  To: Su, Friendy; +Cc: iommu, linux-kernel

On Fri, Sep 05, 2014 at 07:25:14PM +0800, Su, Friendy wrote:
> This issue is found on a mother board whose BIOS reports wrong
> IOAPIC devid in IVHD table. Without this fix, the early mapped
> does not really override IVHD. So that the wrong reported IOAPIC
> does not work.

The problem you describe here should also be fixed by this (simpler)
patch. Can you test this please?

Thanks,

	Joerg

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 3783e0b..b0522f1 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -712,7 +712,7 @@ static void __init set_dev_entry_from_acpi(struct amd_iommu *iommu,
 	set_iommu_for_device(iommu, devid);
 }
 
-static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
+static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
 {
 	struct devid_map *entry;
 	struct list_head *list;
@@ -731,6 +731,8 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 		pr_info("AMD-Vi: Command-line override present for %s id %d - ignoring\n",
 			type == IVHD_SPECIAL_IOAPIC ? "IOAPIC" : "HPET", id);
 
+		*devid = entry->devid;
+
 		return 0;
 	}
 
@@ -739,7 +741,7 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 		return -ENOMEM;
 
 	entry->id	= id;
-	entry->devid	= devid;
+	entry->devid	= *devid;
 	entry->cmd_line	= cmd_line;
 
 	list_add_tail(&entry->list, list);
@@ -754,7 +756,7 @@ static int __init add_early_maps(void)
 	for (i = 0; i < early_ioapic_map_size; ++i) {
 		ret = add_special_device(IVHD_SPECIAL_IOAPIC,
 					 early_ioapic_map[i].id,
-					 early_ioapic_map[i].devid,
+					 &early_ioapic_map[i].devid,
 					 early_ioapic_map[i].cmd_line);
 		if (ret)
 			return ret;
@@ -763,7 +765,7 @@ static int __init add_early_maps(void)
 	for (i = 0; i < early_hpet_map_size; ++i) {
 		ret = add_special_device(IVHD_SPECIAL_HPET,
 					 early_hpet_map[i].id,
-					 early_hpet_map[i].devid,
+					 &early_hpet_map[i].devid,
 					 early_hpet_map[i].cmd_line);
 		if (ret)
 			return ret;
@@ -978,10 +980,17 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 				    PCI_SLOT(devid),
 				    PCI_FUNC(devid));
 
-			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
-			ret = add_special_device(type, handle, devid, false);
+			ret = add_special_device(type, handle, &devid, false);
 			if (ret)
 				return ret;
+
+			/*
+			 * add_special_device might update the devid in case a
+			 * command-line override is present. So call
+			 * set_dev_entry_from_acpi after add_special_device.
+			 */
+			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
+
 			break;
 		}
 		default:

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

* RE: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
  2014-09-05 13:53 ` Joerg Roedel
@ 2014-09-09  8:34   ` Su, Friendy
  2014-09-25 17:29     ` Joerg Roedel
  0 siblings, 1 reply; 4+ messages in thread
From: Su, Friendy @ 2014-09-09  8:34 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: iommu, linux-kernel

Hi, Joerg,

> The problem you describe here should also be fixed by this (simpler)
> patch. Can you test this please?

The running result of this patch is correct.

My opinion is we should avoid modifying the original data "early_ioapic_map[i].devid" and "devid from IVHD" since they are original data from user command line and BIOS. At anytime, "early_ioapic_map[i].devid" should reflect the command line setting. So code should not give possibility to modify it. Same to "devid from IVHD" although it is just a local variable. This is why I imported a new argument to add_special_device().

Best Regards
Friendy

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Friday, September 05, 2014 9:53 PM
> To: Su, Friendy
> Cc: iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/1] iommu/amd: set iommu for early mapped
> ioapic/hpet
> 
> On Fri, Sep 05, 2014 at 07:25:14PM +0800, Su, Friendy wrote:
> > This issue is found on a mother board whose BIOS reports wrong
> > IOAPIC devid in IVHD table. Without this fix, the early mapped
> > does not really override IVHD. So that the wrong reported IOAPIC
> > does not work.
> 
> The problem you describe here should also be fixed by this (simpler)
> patch. Can you test this please?
> 
> Thanks,
> 
> 	Joerg
> 
> diff --git a/drivers/iommu/amd_iommu_init.c
> b/drivers/iommu/amd_iommu_init.c
> index 3783e0b..b0522f1 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -712,7 +712,7 @@ static void __init set_dev_entry_from_acpi(struct
> amd_iommu *iommu,
>  	set_iommu_for_device(iommu, devid);
>  }
> 
> -static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
> +static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
>  {
>  	struct devid_map *entry;
>  	struct list_head *list;
> @@ -731,6 +731,8 @@ static int __init add_special_device(u8 type, u8 id, u16
> devid, bool cmd_line)
>  		pr_info("AMD-Vi: Command-line override present for %s id %d -
> ignoring\n",
>  			type == IVHD_SPECIAL_IOAPIC ? "IOAPIC" : "HPET", id);
> 
> +		*devid = entry->devid;
> +
>  		return 0;
>  	}
> 
> @@ -739,7 +741,7 @@ static int __init add_special_device(u8 type, u8 id, u16
> devid, bool cmd_line)
>  		return -ENOMEM;
> 
>  	entry->id	= id;
> -	entry->devid	= devid;
> +	entry->devid	= *devid;
>  	entry->cmd_line	= cmd_line;
> 
>  	list_add_tail(&entry->list, list);
> @@ -754,7 +756,7 @@ static int __init add_early_maps(void)
>  	for (i = 0; i < early_ioapic_map_size; ++i) {
>  		ret = add_special_device(IVHD_SPECIAL_IOAPIC,
>  					 early_ioapic_map[i].id,
> -					 early_ioapic_map[i].devid,
> +					 &early_ioapic_map[i].devid,
>  					 early_ioapic_map[i].cmd_line);
>  		if (ret)
>  			return ret;
> @@ -763,7 +765,7 @@ static int __init add_early_maps(void)
>  	for (i = 0; i < early_hpet_map_size; ++i) {
>  		ret = add_special_device(IVHD_SPECIAL_HPET,
>  					 early_hpet_map[i].id,
> -					 early_hpet_map[i].devid,
> +					 &early_hpet_map[i].devid,
>  					 early_hpet_map[i].cmd_line);
>  		if (ret)
>  			return ret;
> @@ -978,10 +980,17 @@ static int __init init_iommu_from_acpi(struct
> amd_iommu *iommu,
>  				    PCI_SLOT(devid),
>  				    PCI_FUNC(devid));
> 
> -			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
> -			ret = add_special_device(type, handle, devid, false);
> +			ret = add_special_device(type, handle, &devid, false);
>  			if (ret)
>  				return ret;
> +
> +			/*
> +			 * add_special_device might update the devid in case a
> +			 * command-line override is present. So call
> +			 * set_dev_entry_from_acpi after add_special_device.
> +			 */
> +			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
> +
>  			break;
>  		}
>  		default:


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

* Re: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
  2014-09-09  8:34   ` Su, Friendy
@ 2014-09-25 17:29     ` Joerg Roedel
  0 siblings, 0 replies; 4+ messages in thread
From: Joerg Roedel @ 2014-09-25 17:29 UTC (permalink / raw)
  To: Su, Friendy; +Cc: iommu, linux-kernel

Hi,

On Tue, Sep 09, 2014 at 04:34:52PM +0800, Su, Friendy wrote:
> > The problem you describe here should also be fixed by this (simpler)
> > patch. Can you test this please?
> 
> The running result of this patch is correct.

Okay, fine, thanks for testing. I queued this patch up for the next
merge window.


	Joerg


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

end of thread, other threads:[~2014-09-25 17:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-05 11:25 [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet Su, Friendy
2014-09-05 13:53 ` Joerg Roedel
2014-09-09  8:34   ` Su, Friendy
2014-09-25 17:29     ` Joerg Roedel

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