RE: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet

[Su, Friendy]

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@xxxxxxxxxx]
> Sent: Friday, September 05, 2014 9:53 PM
> To: Su, Friendy
> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/