RE: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
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:j...@8bytes.org] Sent: Friday, September 05, 2014 9:53 PM To: Su, Friendy Cc: iommu@lists.linux-foundation.org; linux-ker...@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: ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v1 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
Hi, Joerg, -Original Message- From: j...@8bytes.org [mailto:j...@8bytes.org] Sent: Wednesday, September 03, 2014 11:06 PM To: Su, Friendy Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH v1 1/1] iommu/amd: set iommu for early mapped ioapic/hpet On Mon, Sep 01, 2014 at 02:17:44PM +0800, Su, Friendy wrote: diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index 3783e0b..148ab61 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -747,7 +747,7 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line) return 0; } -static int __init add_early_maps(void) +static int __init add_early_maps(struct amd_iommu *iommu) { int i, ret; @@ -758,6 +758,11 @@ static int __init add_early_maps(void) early_ioapic_map[i].cmd_line); if (ret) return ret; + /* +* early mapped ioapci overrides ACPI IVRS, +* they should be always controlled by iommu. +*/ + set_iommu_for_device(iommu, early_ioapic_map[i].devid); } for (i = 0; i early_hpet_map_size; ++i) { @@ -767,6 +772,11 @@ static int __init add_early_maps(void) early_hpet_map[i].cmd_line); if (ret) return ret; + /* +* early mapped hpet overrides ACPI IVRS, +* they should be always controlled by iommu. +*/ + set_iommu_for_device(iommu, early_hpet_map[i].devid); This doesn't work on machines with multiple IOMMUs in it. Also the problem only exists if there is no IVHD entry in the IVRS table for the device containing IOAPIC and HPET. But if this IVHD entry is not present we can't reliably know which IOMMU is responsible for a given IOAPIC/HPET. Joerg Thanks. You are correct, we should handle early mapped inside IOMMU whose IVHD reports the same ID. I will update. Friendy ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
From: Su Friendy friendy...@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...@sony.com.cn Signed-off-by: Saeki Shusuke shusuke.sa...@jp.sony.com Signed-off-by: Tamori Masahiro masahiro.tam...@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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
From: Su Friendy friendy...@sony.com.cn 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 IVRS table. Therefore, the early mapped should be always controlled by iommu. Current driver did not set iommu for the early mapped. This causes ivrs_ioapic[ID]/ivrs_hpet[ID] actually are not controlled by iommu. This patch fixes this problem by setting iommu for early mapped. Signed-off-by: Su Friendy friendy...@sony.com.cn Signed-off-by: Saeki Shusuke shusuke.sa...@jp.sony.com Signed-off-by: Tamori Masahiro masahiro.tam...@jp.sony.com --- drivers/iommu/amd_iommu_init.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index 3783e0b..148ab61 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -747,7 +747,7 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line) return 0; } -static int __init add_early_maps(void) +static int __init add_early_maps(struct amd_iommu *iommu) { int i, ret; @@ -758,6 +758,11 @@ static int __init add_early_maps(void) early_ioapic_map[i].cmd_line); if (ret) return ret; + /* +* early mapped ioapci overrides ACPI IVRS, +* they should be always controlled by iommu. +*/ + set_iommu_for_device(iommu, early_ioapic_map[i].devid); } for (i = 0; i early_hpet_map_size; ++i) { @@ -767,6 +772,11 @@ static int __init add_early_maps(void) early_hpet_map[i].cmd_line); if (ret) return ret; + /* +* early mapped hpet overrides ACPI IVRS, +* they should be always controlled by iommu. +*/ + set_iommu_for_device(iommu, early_hpet_map[i].devid); } return 0; @@ -811,7 +821,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu, int ret; - ret = add_early_maps(); + ret = add_early_maps(iommu); if (ret) return ret; -- 1.8.2.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 1/1] iommu/amd: fix enabling exclusion range for an exact device
From: Su Friendy friendy...@sony.com.cn set_device_exclusion_range(u16 devid, struct ivmd_header *m) enables exclusion range for ONE device. IOMMU does not translate the access to the exclusion range from the device. The device is specified by input argument 'devid'. But 'devid' is not passed to the actual set function set_dev_entry_bit(), instead 'm-devid' is passed. 'm-devid' does not specify the exact device which needs enable the exclusion range. 'm-devid' represents DeviceID field of IVMD, which has different meaning depends on IVMD type. The caller init_exclusion_range() sets 'devid' for ONE device. When m-type is equal to ACPI_IVMD_TYPE_ALL or ACPI_IVMD_TYPE_RANGE, 'm-devid' is not equal to 'devid'. This patch fixes 'm-devid' to 'devid'. Signed-off-by: Su Friendy friendy...@sony.com.cn Signed-off-by: Tamori Masahiro masahiro.tam...@jp.sony.com --- drivers/iommu/amd_iommu_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index b76c58d..0e08545 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -788,7 +788,7 @@ static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m) * per device. But we can enable the exclusion range per * device. This is done here */ - set_dev_entry_bit(m-devid, DEV_ENTRY_EX); + set_dev_entry_bit(devid, DEV_ENTRY_EX); iommu-exclusion_start = m-range_start; iommu-exclusion_length = m-range_length; } -- 1.8.2.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu