Re: [PATCH 00/20] iommu/amd: Use generic IOVA allocator

[Wan Zongshun]

On 2016年07月12日 18:55, Joerg Roedel wrote:

Hey Vincent,

On Tue, Jul 12, 2016 at 05:03:08PM +0800, Wan Zongshun wrote:

Currently, those patches can not work at my eCarrizo board.
When I merged your patches, boot failed, and no any info print to me.
I set iommu=pt, it also does not work; set iommu=soft, boot ok.

When I removed those patches, kernel boot ok.

This eCarrizo board uart doesnot work, so I can not get useful
information from kernel by uart console, I also set 'text' in boot
option, but still cannot print any log.


Thanks for your testing. The issue turned out to be an older bug, which
just got uncovered by these patches. I updated the branch at

git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git 
amd-iommu-iova

This branch boots now on my Kaveri and Carrizo system. Can you please
give it a test too?


Joerg, I have tested the patches, it works now.



Thanks,

Joerg

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



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

Re: [PATCH] iommu/amd: Fix unity mapping initialization race

[Wan ZongShun]

>
>> Sorry, why you still say this 'init_device_table_dma' can block DMA?
>> I just think this function will enable DMA transfer, since  we set
>> the V and TV bits, right? or I misunderstand what "block DMA" mean?
>
> When the V and TV bits are not set, it means that all DMA from that
> device-id is forwared untranslated by the IOMMU. But if we set V and TV
> it means that there is translation information, and the IOMMU translates
> the requests using the rest of the DTE information. As all other bits
> are 0, this means that page-table-level is 0 (== no page-table) and that
> the global IW and IR bits are 0 too (== no read and write permissions).
> So all requests are blocked.
>

Clearly.

Thanks.

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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/amd: Fix unity mapping initialization race

[Wan Zongshun]

On 2016年07月11日 15:19, Joerg Roedel wrote:

On Sun, Jul 10, 2016 at 07:40:53PM +0800, Wan Zongshun wrote:

Do you mean we need enable the V and TV bits to DTE entry after all
DTEs tables were initialized completely?


Yes, this is what my patch does and what fixes the bug that was
reported on machines which have unity-mapping entries.


Okay, this patch should also better to general case not only unity-mapping.

How about the interrupt remap function? Do we need same considering for 
IV bit enable for interrupt remap?





I checked this function 'init_device_table_dma', and find it just set
V and TV bit, to set translation info valid and DTE bits127:1 valid.


Right, if no other bits are set this blocks all DMA from the gives
device-id.


Sorry, why you still say this 'init_device_table_dma' can block DMA?
I just think this function will enable DMA transfer, since  we set the V 
and TV bits, right? or I misunderstand what "block DMA" mean?





So I just think all things it should to do are to allow DMA access,
GPA-to-SPA translation should be active, why you add function
comments below is to not allow DMA access and suppress all page
faults?

/*
  * Init the device table to not allow DMA access for devices and
  * suppress all page faults
  */


Yeah, that comment needs to be updated. Not all DMA is blocked and
page-faults are not suppressed at all. Thanks for noticing.



Joerg



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

Re: [PATCH] iommu/amd: Fix unity mapping initialization race

[Wan Zongshun]

On 2016年07月07日 00:00, Joerg Roedel wrote:

From: Joerg Roedel 

There is a race condition in the AMD IOMMU init code that
causes requested unity mappings to be blocked by the IOMMU
for a short period of time. This results on boot failures
and IO_PAGE_FAULTs on some machines.

Fix this by making sure the unity mappings are installed
before all other DMA is blocked.

Fixes: aafd8ba0ca74 ('iommu/amd: Implement add_device and remove_device')
Cc: sta...@vger.kernel.org # v4.2+
Signed-off-by: Joerg Roedel 
---
  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 d091def..59741ea 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1568,13 +1568,23 @@ static int __init amd_iommu_init_pci(void)
break;
}

+   /*
+* Order is important here to make sure any unity map requirements are
+* fulfilled. The unity mappings are created and written to the device
+* table during the amd_iommu_init_api() call.
+*
+* After that we call init_device_table_dma() to make sure any
+* uninitialized DTE will block DMA, and in the end we flush the caches
+* of all IOMMUs to make sure the changes to the device table are
+* active.
+*/


Joerg,

Do you mean we need enable the V and TV bits to DTE entry after all DTEs 
tables were initialized completely?


I checked this function 'init_device_table_dma', and find it just set
V and TV bit, to set translation info valid and DTE bits127:1 valid.

So I just think all things it should to do are to allow DMA access,
GPA-to-SPA translation should be active, why you add function comments 
below is to not allow DMA access and suppress all page faults?


/*
 * Init the device table to not allow DMA access for devices and
 * suppress all page faults
 */
static void init_device_table_dma(void)


+   ret = amd_iommu_init_api();
+
init_device_table_dma();

for_each_iommu(iommu)
iommu_flush_all_caches(iommu);

-   ret = amd_iommu_init_api();
-
if (!ret)
print_iommu_info();



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

Re: [Regression] Amd-Vi + ivrs_ioapic cause kernel oops (4.4, 4.7 fail 3.9 works)

[Wan Zongshun]

On 2016年07月05日 09:56, sunnydrake wrote:


On 04.07.16 16:51, Wan Zongshun wrote:



在 7/4/2016 4:48 AM, sunnydrake 写道:

Thanks for reply.
On 03.07.16 17:26, Wan Zongshun wrote:



在 7/3/2016 8:59 AM, sunnydrake 写道:

[description]
working in kernel 3.9
Oops in current 4.4.0-28,4.7.0-040700rc5
kernel options ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1
workaround to fix ivrs table
cause kernel Oops on boot

Do you mean "ivrs_ioapic[7]=00:14.0  ivrs_ioapic[8]=00:00.1" are
workable at kernel-3.9 but failed in kernel-4.4?

1)yes kernel 3.9 boots ok with ivrs_ioapic[7]=00:14.0
ivrs_ioapic[8]=00:00.1
kernels 4.4 and 4.7 fall to Oops




[bug]
oops:
short oops text
AMD-Vi: Completion_wait loop timed Out
BUG: unable to handle kernel NULL pointer dereference at 000..03e
... irq_pm_install_action+0x1c/0xd0
full oops image text
http://img.ctrlv.in/img/16/07/03/577863055370c.jpg

[additional info]
dmesg|grep AMD-Vi without ivrs_ioapic[8]=00:00.1

This log is from the kernel print without ivrs_ioapic[8]=00:00.1?
Why not provide your kernel log with "ivrs_ioapic[7]=00:14.0
ivrs_ioapic[8]=00:00.1" ?
Full kernel log is better.



2) yes,  because with ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1
kernels are not bootable. Screen of Oops
http://img.ctrlv.in/img/16/07/03/577863055370c.jpg (this with params
ivrs_ioapic[7]=00:14.0  ivrs_ioapic[8]=00:00.1 ). if you need
something another like kdump, i can provide.


If you can provide a full kernel log with ivrs_ioapic[7]=00:14.0
ivrs_ioapic[8]=00:00.1, that is better.
I checked your crash log, and find some things related to i8042 maybe
wrong, it is ps2 relation driver, is it necessary in your system? can
you disable this i8042 firsty to check if your issue is reasoned from it?

i have serial port disabled in bios and booting with i8042.no_acpi=1
does not fix problem. I don't think i8042 related, because
i8042_panic_blink  is caps lock blinking when kernel crash (std behavior)

here is more detailed image of crash
http://img.ctrlv.in/img/16/07/05/577b0ec96746e.jpg


This is not enough to check this issue, I just see "AMD-vi CW loop 
timoutout...", but I can not see that more info ahead of this timeout.


I guess some pci device dead, and it leads to iommu send command timeout 
or else...




Unable to handle null pointer reference at irq_pm_install_action...
ok i will setup linux-crashdump and report logs


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

Re: [Regression] Amd-Vi + ivrs_ioapic cause kernel oops (4.4, 4.7 fail 3.9 works)

[Wan Zongshun]

在 7/4/2016 4:48 AM, sunnydrake 写道:

Thanks for reply.
On 03.07.16 17:26, Wan Zongshun wrote:



在 7/3/2016 8:59 AM, sunnydrake 写道:

[description]
working in kernel 3.9
Oops in current 4.4.0-28,4.7.0-040700rc5
kernel options ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1 
workaround to fix ivrs table

cause kernel Oops on boot
Do you mean "ivrs_ioapic[7]=00:14.0  ivrs_ioapic[8]=00:00.1" are 
workable at kernel-3.9 but failed in kernel-4.4?
1)yes kernel 3.9 boots ok with ivrs_ioapic[7]=00:14.0 
ivrs_ioapic[8]=00:00.1

kernels 4.4 and 4.7 fall to Oops




[bug]
oops:
short oops text
AMD-Vi: Completion_wait loop timed Out
BUG: unable to handle kernel NULL pointer dereference at 000..03e
... irq_pm_install_action+0x1c/0xd0
full oops image text
http://img.ctrlv.in/img/16/07/03/577863055370c.jpg

[additional info]
dmesg|grep AMD-Vi without ivrs_ioapic[8]=00:00.1

This log is from the kernel print without ivrs_ioapic[8]=00:00.1?
Why not provide your kernel log with "ivrs_ioapic[7]=00:14.0 
ivrs_ioapic[8]=00:00.1" ?

Full kernel log is better.



2) yes,  because with ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1 
kernels are not bootable. Screen of Oops 
http://img.ctrlv.in/img/16/07/03/577863055370c.jpg (this with params 
ivrs_ioapic[7]=00:14.0  ivrs_ioapic[8]=00:00.1 ). if you need 
something another like kdump, i can provide.


If you can provide a full kernel log with ivrs_ioapic[7]=00:14.0 
ivrs_ioapic[8]=00:00.1, that is better.
I checked your crash log, and find some things related to i8042 maybe 
wrong, it is ps2 relation driver, is it necessary in your system? can 
you disable this i8042 firsty to check if your issue is reasoned from it?



[ 0.108566] AMD-Vi: Using IVHD type 0x10
[0.108630] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: 3e 
info 1300

[0.108671] AMD-Vi:mmio-addr: feb2
[0.108750] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:00.0 
flags: 00

[0.108792] AMD-Vi:   DEV_RANGE_END devid: 00:00.2
[0.108834] AMD-Vi:   DEV_SELECT devid: 00:02.0 
flags: 00
[0.108875] AMD-Vi:   DEV_SELECT_RANGE_START devid: 01:00.0 
flags: 00

[0.108917] AMD-Vi:   DEV_RANGE_END devid: 01:00.1
[0.108958] AMD-Vi:   DEV_SELECT devid: 00:04.0 
flags: 00
[0.108999] AMD-Vi:   DEV_SELECT devid: 02:00.0 
flags: 00
[0.109041] AMD-Vi:   DEV_SELECT devid: 00:07.0 
flags: 00
[0.109082] AMD-Vi:   DEV_SELECT devid: 03:00.0 
flags: 00
[0.109124] AMD-Vi:   DEV_SELECT devid: 00:11.0 
flags: 00
[0.109166] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:12.0 
flags: 00

[0.109207] AMD-Vi:   DEV_RANGE_END devid: 00:12.2
[0.109249] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:13.0 
flags: 00

[0.114267] AMD-Vi:   DEV_RANGE_END devid: 00:13.2
[0.114308] AMD-Vi:   DEV_SELECT devid: 00:14.0 
flags: d7
[0.114348] AMD-Vi:   DEV_SELECT devid: 00:14.2 
flags: 00
[0.114389] AMD-Vi:   DEV_SELECT devid: 00:14.3 
flags: 00
[0.114429] AMD-Vi:   DEV_SELECT devid: 00:14.4 
flags: 00
[0.114471] AMD-Vi:   DEV_ALIAS_RANGE devid: 04:00.0 
flags: 00 devid_to: 00:14.4

[0.114519] AMD-Vi:   DEV_RANGE_END devid: 04:1f.7
[0.114564] AMD-Vi:   DEV_SELECT devid: 00:14.5 
flags: 00
[0.114605] AMD-Vi:   DEV_SELECT devid: 00:15.0 
flags: 00
[0.114645] AMD-Vi:   DEV_SELECT_RANGE_START devid: 05:00.0 
flags: 00

[0.114687] AMD-Vi:   DEV_RANGE_END devid: 05:00.1
[0.114728] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:16.0 
flags: 00

[0.114768] AMD-Vi:   DEV_RANGE_END devid: 00:16.2
[0.114840] AMD-Vi:   DEV_SPECIAL(IOAPIC[0])devid: 00:14.0
[0.114885] AMD-Vi:   DEV_SPECIAL(HPET[0])devid: 00:14.0
[0.114927] AMD-Vi:   DEV_SPECIAL(IOAPIC[255]) devid: 00:00.1
[0.114969] AMD-Vi: IOAPIC[8] not in IVRS table
Here, it is abnormal, you should add ivrs_ioapic[8]=00:00.1 in boot 
option, or else if the devices rely on

ivrs_ioapic[8], they will not work.

3) yes this why i used ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1 
in 3.9 kernel to enable interrupt remapping, but on 4.4 and 4.7 this 
wil cause Oops.

[ 0.115010] AMD-Vi: Disabling interrupt remapping
[2.994584] AMD-Vi: Completion-Wait loop timed out
[3.103059] AMD-Vi: Completion-Wait loop timed out
[3.103145] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
[3.103279] AMD-Vi: Lazy IO/TLB flushing enabled
[3.993134] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT 
device=05:00.0 address=0x00044c6464a0]
[3.993213] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT 
device=05:00.0 address=0x00044c6464c0]
[5.100622] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT 
device=05:00.0 address=0x00044c646500]

05:00.0 device should be dead now.


4) this is second graphics card(ATI R9 380) on second PCI-E i use 
vfio-

Re: [Regression] Amd-Vi + ivrs_ioapic cause kernel oops (4.4, 4.7 fail 3.9 works)

[Wan Zongshun]

在 7/3/2016 8:59 AM, sunnydrake 写道:

[description]
working in kernel 3.9
Oops in current 4.4.0-28,4.7.0-040700rc5
kernel options ivrs_ioapic[7]=00:14.0 ivrs_ioapic[8]=00:00.1 
workaround to fix ivrs table

cause kernel Oops on boot
Do you mean "ivrs_ioapic[7]=00:14.0  ivrs_ioapic[8]=00:00.1" are 
workable at kernel-3.9 but failed in kernel-4.4?




[bug]
oops:
short oops text
AMD-Vi: Completion_wait loop timed Out
BUG: unable to handle kernel NULL pointer dereference at 000..03e
... irq_pm_install_action+0x1c/0xd0
full oops image text
http://img.ctrlv.in/img/16/07/03/577863055370c.jpg

[additional info]
dmesg|grep AMD-Vi without ivrs_ioapic[8]=00:00.1

This log is from the kernel print without ivrs_ioapic[8]=00:00.1?
Why not provide your kernel log with "ivrs_ioapic[7]=00:14.0 
ivrs_ioapic[8]=00:00.1" ?

Full kernel log is better.


[ 0.108566] AMD-Vi: Using IVHD type 0x10
[0.108630] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: 3e info 
1300

[0.108671] AMD-Vi:mmio-addr: feb2
[0.108750] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:00.0 
flags: 00

[0.108792] AMD-Vi:   DEV_RANGE_END devid: 00:00.2
[0.108834] AMD-Vi:   DEV_SELECT devid: 00:02.0 flags: 00
[0.108875] AMD-Vi:   DEV_SELECT_RANGE_START devid: 01:00.0 
flags: 00

[0.108917] AMD-Vi:   DEV_RANGE_END devid: 01:00.1
[0.108958] AMD-Vi:   DEV_SELECT devid: 00:04.0 flags: 00
[0.108999] AMD-Vi:   DEV_SELECT devid: 02:00.0 flags: 00
[0.109041] AMD-Vi:   DEV_SELECT devid: 00:07.0 flags: 00
[0.109082] AMD-Vi:   DEV_SELECT devid: 03:00.0 flags: 00
[0.109124] AMD-Vi:   DEV_SELECT devid: 00:11.0 flags: 00
[0.109166] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:12.0 
flags: 00

[0.109207] AMD-Vi:   DEV_RANGE_END devid: 00:12.2
[0.109249] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:13.0 
flags: 00

[0.114267] AMD-Vi:   DEV_RANGE_END devid: 00:13.2
[0.114308] AMD-Vi:   DEV_SELECT devid: 00:14.0 flags: d7
[0.114348] AMD-Vi:   DEV_SELECT devid: 00:14.2 flags: 00
[0.114389] AMD-Vi:   DEV_SELECT devid: 00:14.3 flags: 00
[0.114429] AMD-Vi:   DEV_SELECT devid: 00:14.4 flags: 00
[0.114471] AMD-Vi:   DEV_ALIAS_RANGE devid: 04:00.0 flags: 
00 devid_to: 00:14.4

[0.114519] AMD-Vi:   DEV_RANGE_END devid: 04:1f.7
[0.114564] AMD-Vi:   DEV_SELECT devid: 00:14.5 flags: 00
[0.114605] AMD-Vi:   DEV_SELECT devid: 00:15.0 flags: 00
[0.114645] AMD-Vi:   DEV_SELECT_RANGE_START devid: 05:00.0 
flags: 00

[0.114687] AMD-Vi:   DEV_RANGE_END devid: 05:00.1
[0.114728] AMD-Vi:   DEV_SELECT_RANGE_START devid: 00:16.0 
flags: 00

[0.114768] AMD-Vi:   DEV_RANGE_END devid: 00:16.2
[0.114840] AMD-Vi:   DEV_SPECIAL(IOAPIC[0])devid: 00:14.0
[0.114885] AMD-Vi:   DEV_SPECIAL(HPET[0])devid: 00:14.0
[0.114927] AMD-Vi:   DEV_SPECIAL(IOAPIC[255])devid: 00:00.1
[0.114969] AMD-Vi: IOAPIC[8] not in IVRS table
Here, it is abnormal, you should add ivrs_ioapic[8]=00:00.1 in boot 
option, or else if the devices rely on

ivrs_ioapic[8], they will not work.


[ 0.115010] AMD-Vi: Disabling interrupt remapping
[2.994584] AMD-Vi: Completion-Wait loop timed out
[3.103059] AMD-Vi: Completion-Wait loop timed out
[3.103145] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
[3.103279] AMD-Vi: Lazy IO/TLB flushing enabled
[3.993134] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT device=05:00.0 
address=0x00044c6464a0]
[3.993213] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT device=05:00.0 
address=0x00044c6464c0]
[5.100622] AMD-Vi: Event logged [IOTLB_INV_TIMEOUT device=05:00.0 
address=0x00044c646500]

05:00.0 device should be dead now.


[software]
Kernel Version:
Linux version 4.7.0-040700rc5-generic (kernel@tangerine) (gcc version 
5.4.0 20160609 (Ubuntu 5.4.0-4ubuntu1) )

#201606262232 SMP Mon Jun 27 02:34:07 UTC 2016
Environment:
Description:Ubuntu 16.04 LTS
Release:16.04
Software :
Linux main 4.7.0-040700rc5-generic #201606262232 SMP Mon Jun 27 
02:34:07 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux


GNU C5.3.1
GNU Make4.1
Binutils2.26
Util-linux2.27.1
Mount2.27.1
Module-init-tools22
E2fsprogs1.42.13
Pcmciautils018
PPP2.4.7
Linux C Library2.23
Dynamic linker (ldd)2.23
Linux C++ Library6.0.21
Procps3.3.10
Net-tools1.60
Kbd1.15.5
Console-tools1.15.5
Sh-utils8.25
Udev229
Wireless-tools30
Modules Loadedablk_helper acpi_cpufreq aesni_intel aes_x86_64 
ahci ansi_cprng asus_wmi autofs4 binfmt_misc bridge button crc16 
crc32c_intel crc32_pclmul crct10dif_pclmul cryptd dm_mod drbg drm 
drm_kms_helper ebtable_filter ebtables 

Re: [PATCH 1/1] iommu/amd: initialize devid variable before using it

[Wan ZongShun]

2016-06-26 16:33 GMT+08:00 Nicolas Iooss <nicolas.iooss_li...@m4x.org>:
> Commit 2a0cb4e2d423 ("iommu/amd: Add new map for storing IVHD dev entry
> type HID") added a call to DUMP_printk in init_iommu_from_acpi() which
> used the value of devid before this variable was initialized.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
> ---
>  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 9e0034196e10..d091defc3426 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1107,13 +1107,13 @@ static int __init init_iommu_from_acpi(struct 
> amd_iommu *iommu,
> break;
> }
>
> +   devid = e->devid;
> DUMP_printk("  DEV_ACPI_HID(%s[%s])\t\tdevid: 
> %02x:%02x.%x\n",
> hid, uid,
> PCI_BUS_NUM(devid),
> PCI_SLOT(devid),
> PCI_FUNC(devid));
>
> -   devid  = e->devid;
> flags = e->flags;
>

Sure, thanks for your patch.
This is my fault.

> ret = add_acpi_hid_device(hid, uid, , false);
> --
> 2.9.0
>



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2] iommu/amd: Add proper error check in two functions

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch is to do the following:

1. Add error check for caller of iommu_device_create.

2. Add error check for caller of iommu_device_link and
move 'iommu = amd_iommu_rlookup_table[dev_data->devid]' out of
iommuv2 capability condition that make iommu_device_link also
use the 'iommu' to make code more clear and no more than 80
characters.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
Changes from v1:

Don't fail the iommu_init_device() function when the iommu_device_link returned
an error,just print a warning message.
---
 drivers/iommu/amd_iommu.c  | 11 +--
 drivers/iommu/amd_iommu_init.c |  2 ++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 634f636..dbdcaeb 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -476,6 +476,7 @@ out:
 static int iommu_init_device(struct device *dev)
 {
struct iommu_dev_data *dev_data;
+   struct amd_iommu *iommu;
int devid;
 
if (dev->archdata.iommu)
@@ -491,17 +492,15 @@ static int iommu_init_device(struct device *dev)
 
dev_data->alias = get_alias(dev);
 
-   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
-   struct amd_iommu *iommu;
+   iommu = amd_iommu_rlookup_table[dev_data->devid];
 
-   iommu = amd_iommu_rlookup_table[dev_data->devid];
+   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev)))
dev_data->iommu_v2 = iommu->is_iommu_v2;
-   }
 
dev->archdata.iommu = dev_data;
 
-   iommu_device_link(amd_iommu_rlookup_table[dev_data->devid]->iommu_dev,
- dev);
+   if (iommu_device_link(iommu->iommu_dev, dev))
+   dev_warn(dev, "Creating iommu device link failed in sysfs.\n");
 
return 0;
 }
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 9e00341..73fa986 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1526,6 +1526,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
iommu->iommu_dev = iommu_device_create(>dev->dev, iommu,
   amd_iommu_groups, "ivhd%d",
   iommu->index);
+   if (IS_ERR(iommu->iommu_dev))
+   return PTR_ERR(iommu->iommu_dev);
 
return pci_enable_device(iommu->dev);
 }
-- 
1.9.1

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

Re: [PATCH v2] iommu/amd: Set AMD iommu callbacks for platform bus driver

[Wan ZongShun]

2016-05-10 21:21 GMT+08:00 Wan Zongshun <vincent@amd.com>:
> From: Wan Zongshun <vincent@amd.com>
>
> AMD has more drivers will use ACPI to platform bus driver later,
> all those devices need iommu support, for example: eMMC driver.
>
> For latest AMD eMMC controller, it will utilize sdhci-acpi.c driver,
> which will rely on platform bus to match device and driver, where we
> will set 'dev' of struct platform_device as map_sg parameter passing
> to iommu driver for DMA request, so the iommu-ops are needed on the
> platform bus.

Joerg,

How about this patch? it will impact our new eMMC controller driver bringup.


Thanks.

>
> Signed-off-by: Wan Zongshun <vincent@amd.com>
>
> ---
> changes from v1: Add comment why the iommu-ops are needed on platform bus.
> ---
>  drivers/iommu/amd_iommu.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index c430c10..547cdd4 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2980,6 +2981,9 @@ int __init amd_iommu_init_api(void)
> if (err)
> return err;
>  #endif
> +   err = bus_set_iommu(_bus_type, _iommu_ops);
> +   if (err)
> +   return err;
> return 0;
>  }
>
> --
> 1.9.1
>



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [E1000-devel] AMD-Vi: Event logged IO_PAGE_FAULT - ixgbe Detected Tx Unit Hang - Reset adapter - master disable timed out

[Wan ZongShun]

2016-06-14 1:40 GMT+08:00 Lutz Vieweg <l...@5t9.de>:
> On 06/13/2016 04:46 AM, Wan ZongShun wrote:
>>>
>>> With "iommu=pt":
>>>>
>>>>
>>>> [4.832580] iommu: Adding device :04:00.0 to group 13
>>>> [4.832838] iommu: Using direct mapping for device :04:00.0
>>>
>>>
>>
>> That is right, you will pass through AMD IOMMU when you set iommu=pt.
>>
>>> ...
>>>>
>>>>
>>>> [4.837074] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
>>>> [4.837305] AMD-Vi: Found IOMMU at :40:00.2 cap 0x40
>>>> [4.837535] AMD-Vi: Interrupt remapping enabled
>>>> [4.838062] AMD-Vi: Lazy IO/TLB flushing enabled
>>>> [4.838291] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
>>>> [4.838533] software IO TLB [mem 0xd3e8-0xd7e8] (64MB) mapped
>>>> at [8800d3e8-8800d7e7]
>>>
>>>
>>>
>>> I hope that doesn't mean all my network data is now passing through
>>> an additional copy-by-CPU... that would be kind of the opposite of what
>>> "iommu=pt" seemed to promise :-)
>>
>>
>> It depends.
>>
>> Firstly, I need to know if your ethernet card works well now or not
>> after you set iommu=pt.
>
>
> Too early to tell - the NIC worked for the last 4 days now without
> failing, however, that is only about the same time as it took after
> the upgrade to linux-4.6.1 before the bug was encountered, first.
>
> I'd say celebration of "works with iommu=pt" has to wait for at least
> two weeks or so before it is reasonably probable it works for this reason.
>
>> If your ethernet card with 64bit(not 32bit) DMA addressable cap, that
>> is ok, you will not be impacted by bounce buffer.
>
>
>> But iommu=pt is a terrible option, that make all devices bypass the iommu.
>
>
> Why is that terrible? The documentation I found on what iommu=pt actually
> means were pretty scarce, but I noticed how many places recommended to use
> this option for 10G NICs.

I supposed it will work well for your card after set iommu=pt, but it
is not rootcause for your issue.
The iommu=pt just let your all system devices bypassed the iommu, if
there are some device with 32bit DMA addressable cap in your system,
they will be impacted by bounce buffer, it is bad for performance.


Wan Zongshun.

>
>> If you want to get further help, Please try:
>>
>> (1)Please add 'amd_iommu_dump' option in your kernel boot option, and
>> send your full kernel logs, lspci info, don't add iommu=pt.
>> (2) Add amd_iommu=fullflush option to kernel boot option, just try it.
>
>
> Will try that when the NIC becomes unavailable again.
>
>>> One more thing I find curious, but this didn't change with "iommu=pt":
>>>>
>>>>
>>>> [0.00] AGP: Checking aperture...
>>>> [0.00] AGP: No AGP bridge found
>>>> [0.00] AGP: Node 0: aperture [bus addr 0x-0x01ff]
>>>> (32MB)
>>>> [0.00] AGP: Your BIOS doesn't leave an aperture memory hole
>>>> [0.00] AGP: Please enable the IOMMU option in the BIOS setup
>>>> [0.00] AGP: This costs you 64MB of RAM
>>>> [0.00] AGP: Mapping aperture over RAM [mem
>>>> 0xcc00-0xcfff]
>>>> (65536KB)
>>>
>>>
>>> I checked and the IOMMU-option is definitely enabled in the BIOS setup.
>>> So I assume right that these message are irrelevant (since AGP as a whole
>>> is irrelevant on this server)?
>>
>>
>> Please cat /proc/iomem, send the information.
>

This AGP should be used for old GPU, so I don't think it will impact
your this issue.

>
> Here it is:
>>
>> -0fff : reserved
>> 1000-00097bff : System RAM
>> 00097c00-0009 : reserved
>> 000a-000b : PCI Bus :00
>> 000c-000c7fff : Video ROM
>> 000ce800-000d43ff : Adapter ROM
>> 000d4800-000d57ff : Adapter ROM
>> 000e6000-000f : reserved
>>   000f-000f : System ROM
>> 0010-d7e7 : System RAM
>>   0100-01688c05 : Kernel code
>>   01688c06-01d4f53f : Kernel data
>>   01eea000-02174fff : Kernel bss
>> d7e8-d7e8dfff : RAM buffer
>> d7e8e000-d7e8 : reserved
>> d7e9-d7eb3fff : ACPI Tables
>> d7eb4000-d7ed : ACPI Non-volatile Storage
>> d7ee-d7ff : reserved
>> d900-daff : PCI Bus :40
>>   d900-d90003ff 

Re: [E1000-devel] AMD-Vi: Event logged IO_PAGE_FAULT - ixgbe Detected Tx Unit Hang - Reset adapter - master disable timed out

[Wan ZongShun]

2016-06-10 0:57 GMT+08:00 Lutz Vieweg <l...@5t9.de>:
> On 06/09/2016 06:03 PM, Alexander Duyck wrote:
>>>
>>> This time I'll reboot the machine, and also try "iommu=pt" as suggested
>>> in different places for use with 10G NICs.
>>
>>
>> That might be a good place to start.
>>
>> I'm adding, or at least attempting to, the mailing list and maintainer
>> for the IOMMU code.  You might want to check with the AMD-Vi IOMMU
>> maintainers to see if they have any other advice as this seems like
>> something that may have been introduced with changes to the IOMMU as
>> the ixgbe driver hasn't had any updates to the DMA mapping/unmapping
>> code in some time and it was working in the 4.4 kernel series and
>> still works on my system which runs an Intel IOMMU so I am wondering
>> if this may be something specifically related to changes in the AMD
>> IOMMU code.
>
>
> After having rebooted the system with "iommu=pt", the following change
> of dmesg-output looks curious to me:
>
> Without "iommu=pt":
>>
>> [4.869591] iommu: Adding device :04:00.0 to group 13
>
> ...
>>
>> [4.873105] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
>> [4.873347] AMD-Vi: Found IOMMU at :40:00.2 cap 0x40
>> [4.873586] AMD-Vi: Interrupt remapping enabled
>> [4.874108] AMD-Vi: Lazy IO/TLB flushing enabled
>

Ok, so there are two iommus controller in your system.

>
> With "iommu=pt":
>>
>> [4.832580] iommu: Adding device :04:00.0 to group 13
>> [4.832838] iommu: Using direct mapping for device :04:00.0
>

That is right, you will pass through AMD IOMMU when you set iommu=pt.

> ...
>>
>> [4.837074] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
>> [4.837305] AMD-Vi: Found IOMMU at :40:00.2 cap 0x40
>> [4.837535] AMD-Vi: Interrupt remapping enabled
>> [4.838062] AMD-Vi: Lazy IO/TLB flushing enabled
>> [4.838291] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
>> [4.838533] software IO TLB [mem 0xd3e8-0xd7e8] (64MB) mapped
>> at [8800d3e8-8800d7e7]
>
>
> I hope that doesn't mean all my network data is now passing through
> an additional copy-by-CPU... that would be kind of the opposite of what
> "iommu=pt" seemed to promise :-)

It depends.

Firstly, I need to know if your ethernet card works well now or not
after you set iommu=pt.

If your ethernet card with 64bit(not 32bit) DMA addressable cap, that
is ok, you will not be impacted by bounce buffer. But iommu=pt is a
terrible option, that make all devices bypass the iommu.

If you want to get further help, Please try:

(1)Please add 'amd_iommu_dump' option in your kernel boot option, and
send your full kernel logs, lspci info, don't add iommu=pt.
(2) Add amd_iommu=fullflush option to kernel boot option, just try it.


>
> One more thing I find curious, but this didn't change with "iommu=pt":
>>
>> [0.00] AGP: Checking aperture...
>> [0.00] AGP: No AGP bridge found
>> [0.00] AGP: Node 0: aperture [bus addr 0x-0x01ff]
>> (32MB)
>> [0.00] AGP: Your BIOS doesn't leave an aperture memory hole
>> [0.00] AGP: Please enable the IOMMU option in the BIOS setup
>> [0.00] AGP: This costs you 64MB of RAM
>> [0.00] AGP: Mapping aperture over RAM [mem 0xcc00-0xcfff]
>> (65536KB)
>
> I checked and the IOMMU-option is definitely enabled in the BIOS setup.
> So I assume right that these message are irrelevant (since AGP as a whole
> is irrelevant on this server)?

Please cat /proc/iomem, send the information.

>
> Regards,
>
> Lutz Vieweg
>
>
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [Patch v4 9/9] iommu/amd: Check the validation of irq table and domain id

[Wan Zongshun]

 Original Message 

From: Baoquan HE 

If not valid just skip reserving the old domain id.

Signed-off-by: Baoquan He 
---
 drivers/iommu/amd_iommu.c   | 4 
 drivers/iommu/amd_iommu_init.c  | 5 +++--
 drivers/iommu/amd_iommu_types.h | 5 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index f3bd7fd..40c4a05 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3697,10 +3697,6 @@ struct amd_ir_data {

 static struct irq_chip amd_ir_chip;

-#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
-#define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
-#define DTE_IRQ_TABLE_LEN   (8ULL << 1)
-#define DTE_IRQ_REMAP_ENABLE1ULL


At least, you should give reason comments to why you want move it.

Any drivers files you want to use them as well?



 static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
 {
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 47e5972..263704a 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -670,7 +670,7 @@ static int copy_dev_tables(void)
u32 lo, hi, devid;
phys_addr_t old_devtb_phys;
struct dev_table_entry *old_devtb;
-   u16 dom_id, dte_v;
+   u16 dom_id, dte_v, irq_v;
struct amd_iommu *iommu;
static int copied;

@@ -692,7 +692,8 @@ static int copy_dev_tables(void)
 amd_iommu_dev_table[devid] = old_devtb[devid];
 dom_id = amd_iommu_dev_table[devid].data[1] & 
DEV_DOMID_MASK;
dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
-   if (!dte_v)
+   irq_v = amd_iommu_dev_table[devid].data[2] & 
DTE_IRQ_REMAP_ENABLE;
+   if (!dte_v || !irq_v || !dom_id)
continue;
 __set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
 }
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 34acd73..08340f5 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -224,6 +224,11 @@

 #define PPR_REQ_FAULT  0x01

+#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
+#define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
+#define DTE_IRQ_TABLE_LEN   (8ULL << 1)
+#define DTE_IRQ_REMAP_ENABLE1ULL
+
 #define PAGE_MODE_NONE0x00
 #define PAGE_MODE_1_LEVEL 0x01
 #define PAGE_MODE_2_LEVEL 0x02


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

Re: [Patch v4 7/9] iommu/amd: copy old trans table from old kernel

[Wan Zongshun]

 Original Message 

From: Baoquan HE 

Here several things need be done:
1) Initialize amd_iommu_dev_table because it was set several times
   since kdump kernel reboot. We don't need the set because we will
   copy the content from old kernel.
2) Re-enable event/cmd buffer
3) Install the DTE table to reg
4) Flush all caches

Signed-off-by: Baoquan He 
---
 drivers/iommu/amd_iommu_init.c | 47 +-
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 71c7ac9..66a1fa5 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
 #include "irq_remapping.h"
@@ -675,7 +675,7 @@ static int copy_dev_tables(void)
static int copied;

 for_each_iommu(iommu) {
-   if (!translation_pre_enabled()) {
+   if (!translation_pre_enabled(iommu)) {
pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
return -1;
}
@@ -1160,8 +1160,13 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)
iommu->int_enabled = false;

init_translation_status(iommu);
+   if (translation_pre_enabled(iommu) && !is_kdump_kernel()) {
+clear_translation_pre_enabled(iommu);
+pr_warn("Translation was enabled for IOMMU:%d but we are 
not in kdump mode\n",
+iommu->index);
+}

-   if (translation_pre_enabled())
+   if (translation_pre_enabled(iommu))
pr_warn("Translation is already enabled - trying to copy translation 
structures\n");

ret = init_iommu_from_acpi(iommu, h);
@@ -1730,9 +1735,41 @@ static void early_enable_iommu(struct amd_iommu *iommu)
 static void early_enable_iommus(void)
 {
struct amd_iommu *iommu;
+   bool is_pre_enabled=false;

-   for_each_iommu(iommu)
-   early_enable_iommu(iommu);
+   for_each_iommu(iommu) {
+   if ( translation_pre_enabled(iommu) ) {
+   is_pre_enabled = true;
+   break;
+   }
+   }


I wonder if you consider multi-iommus condition for your those series 
patches?



+
+   if ( !is_pre_enabled) {


Why not use translation_pre_enabled(iommu) to judge pre-enable directly?


+   for_each_iommu(iommu)
+   early_enable_iommu(iommu);
+   } else {
+   if (copy_dev_tables()) {
+   pr_err("Failed to copy translation tables from previous 
kernel.\n");
+   /*
+* If failed to copy dev tables from old kernel, 
continue to proceed
+* as it does in normal kernel.
+*/
+   for_each_iommu(iommu) {
+   clear_translation_pre_enabled(iommu);
+   early_enable_iommu(iommu);
+   }
+   } else {
+   pr_info("Copied translation tables from previous 
kernel.\n");
+   for_each_iommu(iommu) {
+   iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
+   iommu_feature_disable(iommu, 
CONTROL_EVT_LOG_EN);
+   iommu_enable_command_buffer(iommu);
+   iommu_enable_event_buffer(iommu);
+   iommu_set_device_table(iommu);
+   iommu_flush_all_caches(iommu);
+   }
+   }
+   }
 }

 static void enable_iommus_v2(void)


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

Re: [Patch v4 6/9] iommu/amd: Add function copy_dev_tables

[Wan Zongshun]

 Original Message 

Add function copy_dev_tables to copy old DTE of the 1st kernel to
the new DTE table. Since all iommu share the same DTE table the
copy only need be done once as long as the physical address of
old DTE table is retrieved from iommu reg. Besides the old domain
id occupied in 1st kernel need be reserved in order to avoid touch
the old translation tables.

Signed-off-by: Baoquan He 
---
 drivers/iommu/amd_iommu.c   |  2 +-
 drivers/iommu/amd_iommu_init.c  | 38 ++
 drivers/iommu/amd_iommu_types.h |  1 +
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1c916cc..f3bd7fd 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2081,7 +2081,7 @@ static void set_dte_entry(u16 devid, struct 
protection_domain *domain, bool ats)
flags|= tmp;
}

-   flags &= ~(0xUL);
+   flags &= ~DEV_DOMID_MASK;
flags |= domain->id;

amd_iommu_dev_table[devid].data[1]  = flags;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 9c1aa54..71c7ac9 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -664,6 +664,44 @@ static int get_dev_entry_bit(u16 devid, u8 bit)
 }


+static int copy_dev_tables(void)
+{
+   u64 entry;
+   u32 lo, hi, devid;
+   phys_addr_t old_devtb_phys;
+   struct dev_table_entry *old_devtb;
+   u16 dom_id, dte_v;
+   struct amd_iommu *iommu;
+   static int copied;
+
+for_each_iommu(iommu) {
+   if (!translation_pre_enabled()) {
+   pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
+   return -1;
+   }


If one iommu is not pre-enabled, all iommus will be exit the copy.


+
+   if (copied)
+   continue;
+
+lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
+hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
+entry = (((u64) hi) << 32) + lo;
+old_devtb_phys = entry & PAGE_MASK;
+old_devtb = memremap(old_devtb_phys, dev_table_size, 
MEMREMAP_WB);
+for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
+amd_iommu_dev_table[devid] = old_devtb[devid];
+dom_id = amd_iommu_dev_table[devid].data[1] & 
DEV_DOMID_MASK;
+   dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
+   if (!dte_v)
+   continue;
+__set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
+}
+   memunmap(old_devtb);
+   copied = 1;
+}
+   return 0;
+}
+
 void amd_iommu_apply_erratum_63(u16 devid)
 {
int sysmgt;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 7796edf..34acd73 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -311,6 +311,7 @@
 #define DTE_FLAG_MASK  (0x3ffULL << 32)
 #define DTE_GLX_SHIFT  (56)
 #define DTE_GLX_MASK   (3)
+#define DEV_DOMID_MASK 0xULL

 #define DTE_GCR3_VAL_A(x)  (((x) >> 12) & 0x7ULL)
 #define DTE_GCR3_VAL_B(x)  (((x) >> 15) & 0x0ULL)


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

Re: [Patch v4 3/9] iommu/amd: Detect pre enabled translation

[Wan Zongshun]

 Original Message 

Add functions to check whether translation is already enabled in IOMMU.

Signed-off-by: Baoquan He 
---
 drivers/iommu/amd_iommu_init.c  | 25 +
 drivers/iommu/amd_iommu_types.h |  4 
 2 files changed, 29 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 8361367d..9e1dfcb 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -229,6 +229,26 @@ static int amd_iommu_enable_interrupts(void);
 static int __init iommu_go_to_state(enum iommu_init_state state);
 static void init_device_table_dma(void);

+
+static bool translation_pre_enabled(struct amd_iommu *iommu)
+{
+   return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
+}
+
+static void clear_translation_pre_enabled(struct amd_iommu *iommu)
+{
+iommu->flags &= ~AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
+static void init_translation_status(struct amd_iommu *iommu)
+{
+   u32 ctrl;
+
+   ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
+   if (ctrl & (1<flags |= AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
 static int iommu_pc_get_set_reg_val(struct amd_iommu *iommu,
u8 bank, u8 cntr, u8 fxn,
u64 *value, bool is_write);
@@ -1101,6 +1121,11 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)

iommu->int_enabled = false;

+   init_translation_status(iommu);
+
+   if (translation_pre_enabled())
+   pr_warn("Translation is already enabled - trying to copy translation 
structures\n");
+


You missed this 'iommu' parameter here, even I saw you fixed it in 
another patch, but please keep each patch to be meaningful.



ret = init_iommu_from_acpi(iommu, h);
if (ret)
return ret;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 9d32b20..01783cc 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -384,6 +384,7 @@ extern struct kmem_cache *amd_iommu_irq_cache;
 #define APERTURE_PAGE_INDEX(a) (((a) >> 21) & 0x3fULL)


+
 /*
  * This struct is used to pass information about
  * incoming PPR faults around.
@@ -401,6 +402,8 @@ struct amd_iommu_fault {
 struct iommu_domain;
 struct irq_domain;

+#define AMD_IOMMU_FLAG_TRANS_PRE_ENABLED  (1 << 0)
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
@@ -525,6 +528,7 @@ struct amd_iommu {
struct irq_domain *ir_domain;
struct irq_domain *msi_domain;
 #endif
+   u32 flags;
 };

 struct devid_map {


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

[PATCH] iommu/amd: Add proper error check in two functions

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch is to do the following:

1. Add error check for caller of iommu_device_create.

2. Add error check for caller of iommu_device_link and
move 'iommu = amd_iommu_rlookup_table[dev_data->devid]' out of
iommuv2 capability condition that make iommu_device_link also
use the 'iommu' to make code more clear and no more than 80
characters.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c  | 12 
 drivers/iommu/amd_iommu_init.c |  2 ++
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 547cdd4..232a3b9 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -414,6 +414,7 @@ out:
 static int iommu_init_device(struct device *dev)
 {
struct iommu_dev_data *dev_data;
+   struct amd_iommu *iommu;
int devid;
 
if (dev->archdata.iommu)
@@ -427,19 +428,14 @@ static int iommu_init_device(struct device *dev)
if (!dev_data)
return -ENOMEM;
 
-   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
-   struct amd_iommu *iommu;
+   iommu = amd_iommu_rlookup_table[dev_data->devid];
 
-   iommu = amd_iommu_rlookup_table[dev_data->devid];
+   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev)))
dev_data->iommu_v2 = iommu->is_iommu_v2;
-   }
 
dev->archdata.iommu = dev_data;
 
-   iommu_device_link(amd_iommu_rlookup_table[dev_data->devid]->iommu_dev,
- dev);
-
-   return 0;
+   return iommu_device_link(iommu->iommu_dev, dev);
 }
 
 static void iommu_ignore_device(struct device *dev)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 9e00341..73fa986 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1526,6 +1526,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
iommu->iommu_dev = iommu_device_create(>dev->dev, iommu,
   amd_iommu_groups, "ivhd%d",
   iommu->index);
+   if (IS_ERR(iommu->iommu_dev))
+   return PTR_ERR(iommu->iommu_dev);
 
return pci_enable_device(iommu->dev);
 }
-- 
1.9.1

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

[PATCH v2] iommu/amd: Set AMD iommu callbacks for platform bus driver

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

AMD has more drivers will use ACPI to platform bus driver later,
all those devices need iommu support, for example: eMMC driver.

For latest AMD eMMC controller, it will utilize sdhci-acpi.c driver,
which will rely on platform bus to match device and driver, where we
will set 'dev' of struct platform_device as map_sg parameter passing
to iommu driver for DMA request, so the iommu-ops are needed on the
platform bus.

Signed-off-by: Wan Zongshun <vincent@amd.com>

---
changes from v1: Add comment why the iommu-ops are needed on platform bus.
---
 drivers/iommu/amd_iommu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index c430c10..547cdd4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2980,6 +2981,9 @@ int __init amd_iommu_init_api(void)
if (err)
return err;
 #endif
+   err = bus_set_iommu(_bus_type, _iommu_ops);
+   if (err)
+   return err;
return 0;
 }
 
-- 
1.9.1

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

[PATCH] iommu/amd: Set AMD iommu callbacks for platform bus driver

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

AMD has more drivers will use ACPI to platform bus driver later,
all those devices need iommu support, such as eMMC acpi driver.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index c430c10..547cdd4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2980,6 +2981,9 @@ int __init amd_iommu_init_api(void)
if (err)
return err;
 #endif
+   err = bus_set_iommu(_bus_type, _iommu_ops);
+   if (err)
+   return err;
return 0;
 }
 
-- 
1.9.1

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

[PATCH V3 9/9] iommu/amd: Set AMD iommu callbacks for amba bus

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

AMD Uart DMA belongs to ACPI HID type device, and its driver
is basing on AMBA Bus, need also IOMMU support.

This patch is just to set the AMD iommu callbacks for amba bus.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 713e7ea..c430c10 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2969,7 +2970,17 @@ static struct dma_map_ops amd_iommu_dma_ops = {
 
 int __init amd_iommu_init_api(void)
 {
-   return bus_set_iommu(_bus_type, _iommu_ops);
+   int err = 0;
+
+   err = bus_set_iommu(_bus_type, _iommu_ops);
+   if (err)
+   return err;
+#ifdef CONFIG_ARM_AMBA
+   err = bus_set_iommu(_bustype, _iommu_ops);
+   if (err)
+   return err;
+#endif
+   return 0;
 }
 
 int __init amd_iommu_init_dma_ops(void)
-- 
1.9.1

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

[PATCH V3 8/9] iommu/amd: Manage iommu_group for ACPI HID devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch creates a new function for finding or creating an IOMMU
group for acpihid(ACPI Hardware ID) device.

The acpihid devices with the same devid will be put into same group and
there will have the same domain id and share the same page table.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 0df651a3..713e7ea 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -276,6 +276,29 @@ static struct iommu_dev_data *get_dev_data(struct device 
*dev)
return dev->archdata.iommu;
 }
 
+/*
+* Find or create an IOMMU group for a acpihid device.
+*/
+static struct iommu_group *acpihid_device_group(struct device *dev)
+{
+   struct acpihid_map_entry *p, *entry = NULL;
+   u16 devid;
+
+   devid = get_acpihid_device_id(dev, );
+   if (devid < 0)
+   return ERR_PTR(devid);
+
+   list_for_each_entry(p, _map, list) {
+   if ((devid == p->devid) && p->group)
+   entry->group = p->group;
+   }
+
+   if (!entry->group)
+   entry->group = generic_device_group(dev);
+
+   return entry->group;
+}
+
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
static const int caps[] = {
@@ -2445,6 +2468,14 @@ static void amd_iommu_remove_device(struct device *dev)
iommu_completion_wait(iommu);
 }
 
+static struct iommu_group *amd_iommu_device_group(struct device *dev)
+{
+   if (dev_is_pci(dev))
+   return pci_device_group(dev);
+
+   return acpihid_device_group(dev);
+}
+
 /*
  *
  * The next functions belong to the dma_ops mapping/unmapping code.
@@ -3286,7 +3317,7 @@ static const struct iommu_ops amd_iommu_ops = {
.iova_to_phys = amd_iommu_iova_to_phys,
.add_device = amd_iommu_add_device,
.remove_device = amd_iommu_remove_device,
-   .device_group = pci_device_group,
+   .device_group = amd_iommu_device_group,
.get_dm_regions = amd_iommu_get_dm_regions,
.put_dm_regions = amd_iommu_put_dm_regions,
.pgsize_bitmap  = AMD_IOMMU_PGSIZES,
-- 
1.9.1

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

[PATCH V3 7/9] iommu/amd: Add iommu support for ACPI HID devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

Current IOMMU driver make assumption that the downstream devices are PCI.
With the newly added ACPI-HID IVHD device entry support, this is no
longer true. This patch is to add dev type check and to distinguish the
pci and acpihid device code path.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c | 69 ---
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 400867f..0df651a3 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -216,13 +217,60 @@ static struct iommu_dev_data *find_dev_data(u16 devid)
return dev_data;
 }
 
-static inline u16 get_device_id(struct device *dev)
+static inline int match_hid_uid(struct device *dev,
+   struct acpihid_map_entry *entry)
+{
+   const char *hid, *uid;
+
+   hid = acpi_device_hid(ACPI_COMPANION(dev));
+   uid = acpi_device_uid(ACPI_COMPANION(dev));
+
+   if (!hid || !(*hid))
+   return -ENODEV;
+
+   if (!uid || !(*uid))
+   return strcmp(hid, entry->hid);
+
+   if (!(*entry->uid))
+   return strcmp(hid, entry->hid);
+
+   return (strcmp(hid, entry->hid) || strcmp(uid, entry->uid));
+}
+
+static inline u16 get_pci_device_id(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
 
return PCI_DEVID(pdev->bus->number, pdev->devfn);
 }
 
+static inline int get_acpihid_device_id(struct device *dev,
+   struct acpihid_map_entry **entry)
+{
+   struct acpihid_map_entry *p;
+
+   list_for_each_entry(p, _map, list) {
+   if (!match_hid_uid(dev, p)) {
+   if (entry)
+   *entry = p;
+   return p->devid;
+   }
+   }
+   return -EINVAL;
+}
+
+static inline int get_device_id(struct device *dev)
+{
+   int devid;
+
+   if (dev_is_pci(dev))
+   devid = get_pci_device_id(dev);
+   else
+   devid = get_acpihid_device_id(dev, NULL);
+
+   return devid;
+}
+
 static struct iommu_dev_data *get_dev_data(struct device *dev)
 {
return dev->archdata.iommu;
@@ -303,10 +351,6 @@ static bool check_device(struct device *dev)
if (!dev || !dev->dma_mask)
return false;
 
-   /* No PCI device */
-   if (!dev_is_pci(dev))
-   return false;
-
devid = get_device_id(dev);
if (IS_ERR_VALUE(devid))
return false;
@@ -344,7 +388,6 @@ out:
 
 static int iommu_init_device(struct device *dev)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
int devid;
 
@@ -359,10 +402,10 @@ static int iommu_init_device(struct device *dev)
if (!dev_data)
return -ENOMEM;
 
-   if (pci_iommuv2_capable(pdev)) {
+   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
 
-   iommu  = amd_iommu_rlookup_table[dev_data->devid];
+   iommu = amd_iommu_rlookup_table[dev_data->devid];
dev_data->iommu_v2 = iommu->is_iommu_v2;
}
 
@@ -2239,13 +2282,17 @@ static bool pci_pri_tlp_required(struct pci_dev *pdev)
 static int attach_device(struct device *dev,
 struct protection_domain *domain)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
+   struct pci_dev *pdev;
struct iommu_dev_data *dev_data;
unsigned long flags;
int ret;
 
dev_data = get_dev_data(dev);
 
+   if (!dev_is_pci(dev))
+   goto skip_ats_check;
+
+   pdev = to_pci_dev(dev);
if (domain->flags & PD_IOMMUV2_MASK) {
if (!dev_data->passthrough)
return -EINVAL;
@@ -2264,6 +2311,7 @@ static int attach_device(struct device *dev,
dev_data->ats.qdep= pci_ats_queue_depth(pdev);
}
 
+skip_ats_check:
write_lock_irqsave(_iommu_devtable_lock, flags);
ret = __attach_device(dev_data, domain);
write_unlock_irqrestore(_iommu_devtable_lock, flags);
@@ -2320,6 +2368,9 @@ static void detach_device(struct device *dev)
__detach_device(dev_data);
write_unlock_irqrestore(_iommu_devtable_lock, flags);
 
+   if (!dev_is_pci(dev))
+   return;
+
if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
pdev_iommuv2_disable(to_pci_dev(dev));
else if (dev_data->ats.enabled)
-- 
1.9.1

_

[PATCH V3 6/9] iommu/amd: Make call-sites of get_device_id aware of its return value

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch is to make the call-sites of get_device_id aware of its
return value.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 51 +--
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index d8e59a8..400867f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -279,9 +279,11 @@ static void init_unity_mappings_for_device(struct device 
*dev,
   struct dma_ops_domain *dma_dom)
 {
struct unity_map_entry *e;
-   u16 devid;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
list_for_each_entry(e, _iommu_unity_map, list) {
if (!(devid >= e->devid_start && devid <= e->devid_end))
@@ -296,7 +298,7 @@ static void init_unity_mappings_for_device(struct device 
*dev,
  */
 static bool check_device(struct device *dev)
 {
-   u16 devid;
+   int devid;
 
if (!dev || !dev->dma_mask)
return false;
@@ -306,6 +308,8 @@ static bool check_device(struct device *dev)
return false;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return false;
 
/* Out of our scope? */
if (devid > amd_iommu_last_bdf)
@@ -342,11 +346,16 @@ static int iommu_init_device(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
+   int devid;
 
if (dev->archdata.iommu)
return 0;
 
-   dev_data = find_dev_data(get_device_id(dev));
+   devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return devid;
+
+   dev_data = find_dev_data(devid);
if (!dev_data)
return -ENOMEM;
 
@@ -367,9 +376,13 @@ static int iommu_init_device(struct device *dev)
 
 static void iommu_ignore_device(struct device *dev)
 {
-   u16 devid, alias;
+   u16 alias;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
+
alias = amd_iommu_alias_table[devid];
 
memset(_iommu_dev_table[devid], 0, sizeof(struct dev_table_entry));
@@ -381,8 +394,14 @@ static void iommu_ignore_device(struct device *dev)
 
 static void iommu_uninit_device(struct device *dev)
 {
-   struct iommu_dev_data *dev_data = search_dev_data(get_device_id(dev));
+   int devid;
+   struct iommu_dev_data *dev_data;
+
+   devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
+   dev_data = search_dev_data(devid);
if (!dev_data)
return;
 
@@ -2314,13 +2333,15 @@ static int amd_iommu_add_device(struct device *dev)
struct iommu_dev_data *dev_data;
struct iommu_domain *domain;
struct amd_iommu *iommu;
-   u16 devid;
-   int ret;
+   int ret, devid;
 
if (!check_device(dev) || get_dev_data(dev))
return 0;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return devid;
+
iommu = amd_iommu_rlookup_table[devid];
 
ret = iommu_init_device(dev);
@@ -2358,12 +2379,15 @@ out:
 static void amd_iommu_remove_device(struct device *dev)
 {
struct amd_iommu *iommu;
-   u16 devid;
+   int devid;
 
if (!check_device(dev))
return;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
+
iommu = amd_iommu_rlookup_table[devid];
 
iommu_uninit_device(dev);
@@ -3035,12 +3059,14 @@ static void amd_iommu_detach_device(struct iommu_domain 
*dom,
 {
struct iommu_dev_data *dev_data = dev->archdata.iommu;
struct amd_iommu *iommu;
-   u16 devid;
+   int devid;
 
if (!check_device(dev))
return;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
if (dev_data->domain != NULL)
detach_device(dev);
@@ -3158,9 +3184,11 @@ static void amd_iommu_get_dm_regions(struct device *dev,
 struct list_head *head)
 {
struct unity_map_entry *entry;
-   u16 devid;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
list_for_each_entry(entry, _iommu_unity_map, list) {
struct iommu_dm_region *region;
@@ -3862,6 +3890,9 @@ static struct irq_domain *get_irq_domain(struct 
irq_alloc_info *info)
case X86_IRQ_ALLOC_TYPE_MSI:
case X86_IRQ_ALLOC_TYPE_MSIX:
devid = get_device_id(>msi_dev->dev);
+   if (IS_ERR_VALUE(devid))
+   return NULL;
+

[PATCH V3 0/9] iommu/amd: enable ACPI hardware ID device support

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

There are some devices indentified using ACPI HID format in AMD chip.
This patch series enable iommu support for those ACPI HID device, 
since the existing AMD iommu only supports PCI bus based device.

The latest public version of AMD IOMMU specification that describes
the support for IVHD type 40h and ACPI HID IVHD device type, 
implemented by this patch series, is available here:

http://support.amd.com/TechDocs/48882_IOMMU.pdf

The V2 added two new patches: patch 5 and patch 8 according to Joerg's
comments, there is a little modification in patch 6 to distinguish pci
and none pci.

The V3 added the fix for the booting issue Joerg experienced on the CZ
platform due to the IOMMU performance counter init code.

Suravee Suthikulpanit (4):
  iommu/amd: Adding Extended Feature Register check for PC support
  iommu/amd: Modify ivhd_header structure to support type 11h and 40h
  iommu/amd: Use the most comprehensive IVHD type that the driver can
support
  iommu/amd: Introduces ivrs_acpihid kernel parameter

Wan Zongshun (5):
  iommu/amd: Add new map for storing IVHD dev entry type HID
  iommu/amd: Make call-sites of get_device_id aware of its return value
  iommu/amd: Add iommu support for ACPI HID devices
  iommu/amd: Manage iommu_group for ACPI HID devices
  iommu/amd: Set AMD iommu callbacks for amba bus

 Documentation/kernel-parameters.txt |   7 +
 drivers/iommu/amd_iommu.c   | 167 +++---
 drivers/iommu/amd_iommu_init.c  | 329 +++-
 drivers/iommu/amd_iommu_types.h |  14 ++
 4 files changed, 457 insertions(+), 60 deletions(-)

-- 
1.9.1

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

[PATCH V3 2/9] iommu/amd: Modify ivhd_header structure to support type 11h and 40h

[Wan Zongshun]

From: Suravee Suthikulpanit 

This patch modifies the existing struct ivhd_header,
which currently only support IVHD type 0x10, to add
new fields from IVHD type 11h and 40h.

It also modifies the pointer calculation to allow
support for IVHD type 11h and 40h

Signed-off-by: Suravee Suthikulpanit 
---
 drivers/iommu/amd_iommu_init.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index dff1e01..22e078b 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -398,6 +398,22 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu 
*iommu)
release_mem_region(iommu->mmio_phys, iommu->mmio_phys_end);
 }
 
+static inline u32 get_ivhd_header_size(struct ivhd_header *h)
+{
+   u32 size = 0;
+
+   switch (h->type) {
+   case 0x10:
+   size = 24;
+   break;
+   case 0x11:
+   case 0x40:
+   size = 40;
+   break;
+   }
+   return size;
+}
+
 /
  *
  * The functions below belong to the first pass of AMD IOMMU ACPI table
@@ -424,7 +440,14 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
u8 *p = (void *)h, *end = (void *)h;
struct ivhd_entry *dev;
 
-   p += sizeof(*h);
+   u32 ivhd_size = get_ivhd_header_size(h);
+
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
+
+   p += ivhd_size;
end += h->length;
 
while (p < end) {
@@ -789,6 +812,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
u32 dev_i, ext_flags = 0;
bool alias = false;
struct ivhd_entry *e;
+   u32 ivhd_size;
int ret;
 
 
@@ -804,7 +828,14 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
/*
 * Done. Now parse the device entries
 */
-   p += sizeof(struct ivhd_header);
+   ivhd_size = get_ivhd_header_size(h);
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
+
+   p += ivhd_size;
+
end += h->length;
 
 
-- 
1.9.1

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

[PATCH V3 3/9] iommu/amd: Use the most comprehensive IVHD type that the driver can support

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

The IVRS in more recent AMD system usually contains multiple
IVHD block types (e.g. 0x10, 0x11, and 0x40) for each IOMMU.
The newer IVHD types provide more information (e.g. new features
specified in the IOMMU spec), while maintain compatibility with
the older IVHD type.

Having multiple IVHD type allows older IOMMU drivers to still function
(e.g. using the older IVHD type 0x10) while the newer IOMMU driver can use
the newer IVHD types (e.g. 0x11 and 0x40). Therefore, the IOMMU driver
should only make use of the newest IVHD type that it can support.

This patch adds new logic to determine the highest level of IVHD type
it can support, and use it throughout the to initialize the driver.
This requires adding another pass to the IVRS parsing to determine
appropriate IVHD type (see function get_highest_supported_ivhd_type())
before parsing the contents.

[Vincent: fix the build error of IVHD_DEV_ACPI_HID flag not found]

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 107 ++---
 1 file changed, 78 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 22e078b..8f49612 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -44,7 +44,7 @@
  */
 #define IVRS_HEADER_LENGTH 48
 
-#define ACPI_IVHD_TYPE  0x10
+#define ACPI_IVHD_TYPE_MAX_SUPPORTED   0x40
 #define ACPI_IVMD_TYPE_ALL  0x20
 #define ACPI_IVMD_TYPE  0x21
 #define ACPI_IVMD_TYPE_RANGE0x22
@@ -58,6 +58,7 @@
 #define IVHD_DEV_EXT_SELECT 0x46
 #define IVHD_DEV_EXT_SELECT_RANGE   0x47
 #define IVHD_DEV_SPECIAL   0x48
+#define IVHD_DEV_ACPI_HID  0xf0
 
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
@@ -137,6 +138,7 @@ bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
+static int amd_iommu_target_ivhd_type;
 
 u16 amd_iommu_last_bdf;/* largest PCI device id we have
   to handle */
@@ -428,7 +430,15 @@ static inline u32 get_ivhd_header_size(struct ivhd_header 
*h)
  */
 static inline int ivhd_entry_length(u8 *ivhd)
 {
-   return 0x04 << (*ivhd >> 6);
+   u32 type = ((struct ivhd_entry *)ivhd)->type;
+
+   if (type < 0x80) {
+   return 0x04 << (*ivhd >> 6);
+   } else if (type == IVHD_DEV_ACPI_HID) {
+   /* For ACPI_HID, offset 21 is uid len */
+   return *((u8 *)ivhd + 21) + 22;
+   }
+   return 0;
 }
 
 /*
@@ -475,6 +485,22 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
return 0;
 }
 
+static int __init check_ivrs_checksum(struct acpi_table_header *table)
+{
+   int i;
+   u8 checksum = 0, *p = (u8 *)table;
+
+   for (i = 0; i < table->length; ++i)
+   checksum += p[i];
+   if (checksum != 0) {
+   /* ACPI table corrupt */
+   pr_err(FW_BUG "AMD-Vi: IVRS invalid checksum\n");
+   return -ENODEV;
+   }
+
+   return 0;
+}
+
 /*
  * Iterate over all IVHD entries in the ACPI table and find the highest device
  * id which we need to handle. This is the first of three functions which parse
@@ -482,31 +508,19 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
  */
 static int __init find_last_devid_acpi(struct acpi_table_header *table)
 {
-   int i;
-   u8 checksum = 0, *p = (u8 *)table, *end = (u8 *)table;
+   u8 *p = (u8 *)table, *end = (u8 *)table;
struct ivhd_header *h;
 
-   /*
-* Validate checksum here so we don't need to do it when
-* we actually parse the table
-*/
-   for (i = 0; i < table->length; ++i)
-   checksum += p[i];
-   if (checksum != 0)
-   /* ACPI table corrupt */
-   return -ENODEV;
-
p += IVRS_HEADER_LENGTH;
 
end += table->length;
while (p < end) {
h = (struct ivhd_header *)p;
-   switch (h->type) {
-   case ACPI_IVHD_TYPE:
-   find_last_devid_from_ivhd(h);
-   break;
-   default:
-   break;
+   if (h->type == amd_iommu_target_ivhd_type) {
+   int ret = find_last_devid_from_ivhd(h);
+
+   if (ret)
+   return ret;
}
p += h->length;
}
@@ -1164,6 +1178,32 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)
return 0;
 }
 
+/**
+ * get_highest_supported

[PATCH V3 5/9] iommu/amd: Introduces ivrs_acpihid kernel parameter

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

This patch introduces a new kernel parameter, ivrs_acpihid.
This is used to override existing ACPI-HID IVHD device entry,
or add an entry in case it is missing in the IVHD.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 Documentation/kernel-parameters.txt |  7 +++
 drivers/iommu/amd_iommu_init.c  | 33 +
 2 files changed, 40 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index ecc74fa..8c881a5 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1767,6 +1767,13 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
PCI device 00:14.0 write the parameter as:
ivrs_hpet[0]=00:14.0
 
+   ivrs_acpihid[HW,X86_64]
+   Provide an override to the ACPI-HID:UID<->DEVICE-ID
+   mapping provided in the IVRS ACPI table. For
+   example, to map UART-HID:UID AMD0020:0 to
+   PCI device 00:14.5 write the parameter as:
+   ivrs_acpihid[00:14.5]=AMD0020:0
+
js= [HW,JOY] Analog joystick
See Documentation/input/joystick.txt.
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e7ebfa2..9e00341 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2477,10 +2477,43 @@ static int __init parse_ivrs_hpet(char *str)
return 1;
 }
 
+static int __init parse_ivrs_acpihid(char *str)
+{
+   u32 bus, dev, fn;
+   char *hid, *uid, *p;
+   char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0};
+   int ret, i;
+
+   ret = sscanf(str, "[%x:%x.%x]=%s", , , , acpiid);
+   if (ret != 4) {
+   pr_err("AMD-Vi: Invalid command line: ivrs_acpihid(%s)\n", str);
+   return 1;
+   }
+
+   p = acpiid;
+   hid = strsep(, ":");
+   uid = p;
+
+   if (!hid || !(*hid) || !uid) {
+   pr_err("AMD-Vi: Invalid command line: hid or uid\n");
+   return 1;
+   }
+
+   i = early_acpihid_map_size++;
+   memcpy(early_acpihid_map[i].hid, hid, strlen(hid));
+   memcpy(early_acpihid_map[i].uid, uid, strlen(uid));
+   early_acpihid_map[i].devid =
+   ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+   early_acpihid_map[i].cmd_line   = true;
+
+   return 1;
+}
+
 __setup("amd_iommu_dump",  parse_amd_iommu_dump);
 __setup("amd_iommu=",  parse_amd_iommu_options);
 __setup("ivrs_ioapic", parse_ivrs_ioapic);
 __setup("ivrs_hpet",   parse_ivrs_hpet);
+__setup("ivrs_acpihid",parse_ivrs_acpihid);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
  gart_iommu_hole_init,
-- 
1.9.1

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

[PATCH V3 1/9] iommu/amd: Adding Extended Feature Register check for PC support

[Wan Zongshun]

From: Suravee Suthikulpanit 

The IVHD header type 11h and 40h introduce the PCSup bit in
the EFR Register Image bit fileds. This should be used to
determine the IOMMU performance support instead of relying
on the PNCounters and PNBanks.

Note also that the PNCouters and PNBanks bits in the IOMMU
attributes field of IVHD headers type 11h are incorrectly
programmed on some systems.

So, we should not rely on it to determine the performance
counter/banks size. Instead, these values should be read
from the MMIO Offset 0030h IOMMU Extended Feature Register.

Signed-off-by: Suravee Suthikulpanit 
---
 drivers/iommu/amd_iommu_init.c | 32 
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index bf4959f..dff1e01 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -99,7 +99,11 @@ struct ivhd_header {
u64 mmio_phys;
u16 pci_seg;
u16 info;
-   u32 efr;
+   u32 efr_attr;
+
+   /* Following only valid on IVHD type 11h and 40h */
+   u64 efr_reg; /* Exact copy of MMIO_EXT_FEATURES */
+   u64 res;
 } __attribute__((packed));
 
 /*
@@ -1078,13 +1082,25 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)
iommu->pci_seg = h->pci_seg;
iommu->mmio_phys = h->mmio_phys;
 
-   /* Check if IVHD EFR contains proper max banks/counters */
-   if ((h->efr != 0) &&
-   ((h->efr & (0xF << 13)) != 0) &&
-   ((h->efr & (0x3F << 17)) != 0)) {
-   iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
-   } else {
-   iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
+   switch (h->type) {
+   case 0x10:
+   /* Check if IVHD EFR contains proper max banks/counters */
+   if ((h->efr_attr != 0) &&
+   ((h->efr_attr & (0xF << 13)) != 0) &&
+   ((h->efr_attr & (0x3F << 17)) != 0))
+   iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
+   else
+   iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
+   break;
+   case 0x11:
+   case 0x40:
+   if (h->efr_reg & (1 << 9))
+   iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
+   else
+   iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
+   break;
+   default:
+   return -EINVAL;
}
 
iommu->mmio_base = iommu_map_mmio_space(iommu->mmio_phys,
-- 
1.9.1

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

[PATCH V3 4/9] iommu/amd: Add new map for storing IVHD dev entry type HID

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch introduces acpihid_map, which is used to store
the new IVHD device entry extracted from BIOS IVRS table.

It also provides a utility function add_acpi_hid_device(),
to add this types of devices to the map.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c   |   1 +
 drivers/iommu/amd_iommu_init.c  | 122 
 drivers/iommu/amd_iommu_types.h |  14 +
 3 files changed, 137 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 374c129..d8e59a8 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,7 @@ static DEFINE_SPINLOCK(dev_data_list_lock);
 
 LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
+LIST_HEAD(acpihid_map);
 
 /*
  * Domain for untranslated devices - only allocated
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 8f49612..e7ebfa2 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -60,6 +60,10 @@
 #define IVHD_DEV_SPECIAL   0x48
 #define IVHD_DEV_ACPI_HID  0xf0
 
+#define UID_NOT_PRESENT 0
+#define UID_IS_INTEGER  1
+#define UID_IS_CHARACTER2
+
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
 
@@ -116,6 +120,11 @@ struct ivhd_entry {
u16 devid;
u8 flags;
u32 ext;
+   u32 hidh;
+   u64 cid;
+   u8 uidf;
+   u8 uidl;
+   u8 uid;
 } __attribute__((packed));
 
 /*
@@ -224,8 +233,12 @@ enum iommu_init_state {
 #define EARLY_MAP_SIZE 4
 static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static struct acpihid_map_entry __initdata early_acpihid_map[EARLY_MAP_SIZE];
+
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static int __initdata early_acpihid_map_size;
+
 static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
@@ -765,6 +778,42 @@ static int __init add_special_device(u8 type, u8 id, u16 
*devid, bool cmd_line)
return 0;
 }
 
+static int __init add_acpi_hid_device(u8 *hid, u8 *uid, u16 *devid,
+ bool cmd_line)
+{
+   struct acpihid_map_entry *entry;
+   struct list_head *list = _map;
+
+   list_for_each_entry(entry, list, list) {
+   if (strcmp(entry->hid, hid) ||
+   (*uid && *entry->uid && strcmp(entry->uid, uid)) ||
+   !entry->cmd_line)
+   continue;
+
+   pr_info("AMD-Vi: Command-line override for hid:%s uid:%s\n",
+   hid, uid);
+   *devid = entry->devid;
+   return 0;
+   }
+
+   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+   if (!entry)
+   return -ENOMEM;
+
+   memcpy(entry->uid, uid, strlen(uid));
+   memcpy(entry->hid, hid, strlen(hid));
+   entry->devid = *devid;
+   entry->cmd_line = cmd_line;
+   entry->root_devid = (entry->devid & (~0x7));
+
+   pr_info("AMD-Vi:%s, add hid:%s, uid:%s, rdevid:%d\n",
+   entry->cmd_line ? "cmd" : "ivrs",
+   entry->hid, entry->uid, entry->root_devid);
+
+   list_add_tail(>list, list);
+   return 0;
+}
+
 static int __init add_early_maps(void)
 {
int i, ret;
@@ -787,6 +836,15 @@ static int __init add_early_maps(void)
return ret;
}
 
+   for (i = 0; i < early_acpihid_map_size; ++i) {
+   ret = add_acpi_hid_device(early_acpihid_map[i].hid,
+ early_acpihid_map[i].uid,
+ _acpihid_map[i].devid,
+ early_acpihid_map[i].cmd_line);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -1007,6 +1065,70 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
 
break;
}
+   case IVHD_DEV_ACPI_HID: {
+   u16 devid;
+   u8 hid[ACPIHID_HID_LEN] = {0};
+   u8 uid[ACPIHID_UID_LEN] = {0};
+   int ret;
+
+   if (h->type != 0x40) {
+   pr_err(FW_BUG "Invalid IVHD device type %#x\n",
+  e->type);
+   break;
+   }
+
+   memcpy(hid, (u8 *)(>ext), ACPIHID_HID_LEN - 1);
+   hid[ACPIHID_HID_LEN - 1] = '

Re: [Patch v3 11/12] iommu/amd: No need to wait iommu completion if no dte irq entry change

[Wan Zongshun]

 Original Message 

On 01/27/16 at 07:03pm, Wan Zongshun wrote:



 Original Message 


alias = amd_iommu_alias_table[devid];
table = irq_lookup_table[alias];
@@ -3688,7 +3688,7 @@ static struct irq_remap_table *get_irq_table(u16 devid, 
bool ioapic)
/* Nothing there yet, allocate new irq remapping table */
table = kzalloc(sizeof(*table), GFP_ATOMIC);
if (!table)
-   goto out;
+   goto out_unlock;

/* Initialize table spin-lock */
spin_lock_init(>lock);
@@ -3701,7 +3701,7 @@ static struct irq_remap_table *get_irq_table(u16 devid, 
bool ioapic)
if (!table->table) {
kfree(table);
table = NULL;
-   goto out;
+   goto out_unlock;
}

memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));



So here the 'out:' label text should also remove, since no err need
goto it, right?

out:
iommu_completion_wait(iommu);


Seems we still need out lable here since it need jump to out label if
alias irte table has been set ite irte table of alias will be set in DTE.



It is okay.






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

Re: [Patch v3 05/12] iommu/amd: change IOMMU_PTE_P to IOMMU_PTE_V

[Wan Zongshun]

 Original Message 

On 01/27/16 at 06:18pm, Wan Zongshun wrote:



 Original Message 

In amd-vi spec the name of bit0 in DTE is V. But in code it's defined
as IOMMU_PTE_P. Here change it to IOMMU_PTE_V to make it be consistent
with spec.


Hi, Baoquan

This should be PR bit which means present, So maybe you got
confusion between DTE and PTE table, right?


Well, I got it. In fact the MACRO definition and current code confused
me. IOMMU_PTE_P is not only used for PTE table, but used for DTE entry.

You can check functions set_dte_entry()/clear_dte_entry() where
IOMMU_PTE_P and IOMMU_PTE_TV are set in DTE entry to indicate the DTE
entry and translation is valid. Meanwhile I saw in iommu_map_page
IOMMU_PTE_P is used to incidate pte is present. This makes us confused. I
will post a patch to clean up this, defining new specific MACRO for DTE
use only. Do you agree on this?



Baoquan,

It is fine to me, is it better to add a lot of explanation for those 
bits MACRO? like:


/* PTE bit value definition*/
#define IOMMU_PTE_P  (1ULL << 0)
#define IOMMU_PTE_TV (1ULL << 1)

/* DTE bit value definition*/
#define IOMMU_DTE_TV (1ULL << 1)



So please don't change the IOMMU_PTE_P to IOMMU_PTE_V firstly, maybe
you should rename IOMMU_PTE_TV to IOMMU_DTE_TV, or just keep those
codes here, does make sense?


As you said, IOMMU_PTE_P need be kept, also IOMMU_DTE_V/IOMMU_DTE_TV are
also necessary to re-define separately.


Vincent.



Signed-off-by: Baoquan He <b...@redhat.com>
---
  drivers/iommu/amd_iommu.c   | 10 +-
  drivers/iommu/amd_iommu_types.h |  6 +++---
  2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 63f4c6b..f02d4b1 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1331,9 +1331,9 @@ static int iommu_map_page(struct protection_domain *dom,

if (count > 1) {
__pte = PAGE_SIZE_PTE(phys_addr, page_size);
-   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_V | IOMMU_PTE_FC;
} else
-   __pte = phys_addr | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte = phys_addr | IOMMU_PTE_V | IOMMU_PTE_FC;

if (prot & IOMMU_PROT_IR)
__pte |= IOMMU_PTE_IR;
@@ -1978,7 +1978,7 @@ static void set_dte_entry(u16 devid, struct 
protection_domain *domain, bool ats)

pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
<< DEV_ENTRY_MODE_SHIFT;
-   pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
+   pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_V | IOMMU_PTE_TV;

flags = amd_iommu_dev_table[devid].data[1];

@@ -2021,7 +2021,7 @@ static void set_dte_entry(u16 devid, struct 
protection_domain *domain, bool ats)
  static void clear_dte_entry(u16 devid)
  {
/* remove entry from the device table seen by the hardware */
-   amd_iommu_dev_table[devid].data[0]  = IOMMU_PTE_P | IOMMU_PTE_TV;
+   amd_iommu_dev_table[devid].data[0]  = IOMMU_PTE_V | IOMMU_PTE_TV;
amd_iommu_dev_table[devid].data[1] &= DTE_FLAG_MASK;

amd_iommu_apply_erratum_63(devid);
@@ -2463,7 +2463,7 @@ static dma_addr_t dma_ops_domain_map(struct 
dma_ops_domain *dom,
if (!pte)
return DMA_ERROR_CODE;

-   __pte = paddr | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte = paddr | IOMMU_PTE_V | IOMMU_PTE_FC;

if (direction == DMA_TO_DEVICE)
__pte |= IOMMU_PTE_IR;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 9b8ace4..65f7988 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -244,7 +244,7 @@
  #define PM_LEVEL_INDEX(x, a)  (((a) >> PM_LEVEL_SHIFT((x))) & 0x1ffULL)
  #define PM_LEVEL_ENC(x)   (((x) << 9) & 0xe00ULL)
  #define PM_LEVEL_PDE(x, a)((a) | PM_LEVEL_ENC((x)) | \
-IOMMU_PTE_P | IOMMU_PTE_IR | IOMMU_PTE_IW)
+IOMMU_PTE_V | IOMMU_PTE_IR | IOMMU_PTE_IW)
  #define PM_PTE_LEVEL(pte) (((pte) >> 9) & 0x7ULL)

  #define PM_MAP_4k 0
@@ -293,7 +293,7 @@
  #define PTE_LEVEL_PAGE_SIZE(level)\
(1ULL << (12 + (9 * (level

-#define IOMMU_PTE_P  (1ULL << 0)
+#define IOMMU_PTE_V  (1ULL << 0)
  #define IOMMU_PTE_TV (1ULL << 1)
  #define IOMMU_PTE_U  (1ULL << 59)
  #define IOMMU_PTE_FC (1ULL << 60)
@@ -321,7 +321,7 @@
  #define GCR3_VALID0x01ULL

  #define IOMMU_PAGE_MASK (((1ULL << 52) - 1) & ~0xfffULL)
-#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_P)
+#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_V)
  #define IOMMU_PTE_PAGE(pte) (phys_to_virt((pte) & IOMMU_PAGE_MASK))
  #define IOMMU_PTE_MODE(p

Re: [Patch v3 04/12] iommu/amd: add copy_irq_table function

[Wan Zongshun]

 Original Message 

If irq table exists in old kernel create a new one and copy the content
of old irq table to the newly created.

Signed-off-by: Baoquan He 
---
  drivers/iommu/amd_iommu.c   |  2 +-
  drivers/iommu/amd_iommu_init.c  | 43 +
  drivers/iommu/amd_iommu_proto.h |  1 +
  3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index aeae07a..63f4c6b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3628,7 +3628,7 @@ struct amd_ir_data {
  static struct irq_chip amd_ir_chip;


-static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
+void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
  {
u64 dte;

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 91659d8..450adad 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -660,6 +660,49 @@ static int get_dev_entry_bit(u16 devid, u8 bit)
return (amd_iommu_dev_table[devid].data[i] & (1UL << _bit)) >> _bit;
  }

+static void copy_irq_table(u16 devid)
+{
+   struct irq_remap_table *table = NULL;
+   u16 alias;
+   u64 dte;
+   u64 *old_intr_virt;
+
+   alias = amd_iommu_alias_table[devid];
+   table = irq_lookup_table[alias];
+   if (table) {
+   irq_lookup_table[devid] = table;
+   set_dte_irq_entry(devid, table);
+   return;
+   }
+   dte = amd_iommu_dev_table[devid].data[2];
+   dte &= DTE_IRQ_PHYS_ADDR_MASK;
+   if( (!dte_IRQ_REMAP_ENABLE) || ( dte == 0) )
+   return;
+


here: dte_IRQ_REMAP_ENABLE

style?
dte & DTE_IRQ_REMAP_ENABLE


+   table = kzalloc(sizeof(*table), GFP_ATOMIC);
+   if (!table){
+   pr_warn("AMD-Vi: amd irq table allocation failed\n");
+   return;
+   }
+
+   table->table = kmem_cache_alloc(amd_iommu_irq_cache, GFP_ATOMIC);
+if (!table->table) {
+kfree(table);
+table = NULL;
+   return;
+}
+   memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));
+
+   old_intr_virt = memremap(dte, MAX_IRQS_PER_TABLE * sizeof(u32), 
MEMREMAP_WB);


Here if rename the 'dte' to 'irte',  better to read?



+   memcpy(table->table, old_intr_virt, MAX_IRQS_PER_TABLE * sizeof(u32));
+   irq_lookup_table[devid] = table;
+   set_dte_irq_entry(devid, table);
+   if (devid != alias) {
+irq_lookup_table[alias] = table;
+   set_dte_irq_entry(alias, table);
+}
+   memunmap(old_intr_virt);
+}

  void amd_iommu_apply_erratum_63(u16 devid)
  {
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 743e209..ab562e0 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -101,5 +101,6 @@ static inline bool iommu_feature(struct amd_iommu *iommu, 
u64 f)
  /* kdump checking  */
  extern bool translation_pre_enabled(void);
  extern void clear_translation_pre_enabled(void);
+extern void set_dte_irq_entry(u16 devid, struct irq_remap_table *table);

  #endif /* _ASM_X86_AMD_IOMMU_PROTO_H  */


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

Re: [Patch v3 05/12] iommu/amd: change IOMMU_PTE_P to IOMMU_PTE_V

[Wan Zongshun]

 Original Message 

In amd-vi spec the name of bit0 in DTE is V. But in code it's defined
as IOMMU_PTE_P. Here change it to IOMMU_PTE_V to make it be consistent
with spec.


Hi, Baoquan

This should be PR bit which means present, So maybe you got confusion 
between DTE and PTE table, right?


So please don't change the IOMMU_PTE_P to IOMMU_PTE_V firstly, maybe you 
should rename IOMMU_PTE_TV to IOMMU_DTE_TV, or just keep those codes 
here, does make sense?


Vincent.



Signed-off-by: Baoquan He 
---
  drivers/iommu/amd_iommu.c   | 10 +-
  drivers/iommu/amd_iommu_types.h |  6 +++---
  2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 63f4c6b..f02d4b1 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1331,9 +1331,9 @@ static int iommu_map_page(struct protection_domain *dom,

if (count > 1) {
__pte = PAGE_SIZE_PTE(phys_addr, page_size);
-   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_V | IOMMU_PTE_FC;
} else
-   __pte = phys_addr | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte = phys_addr | IOMMU_PTE_V | IOMMU_PTE_FC;

if (prot & IOMMU_PROT_IR)
__pte |= IOMMU_PTE_IR;
@@ -1978,7 +1978,7 @@ static void set_dte_entry(u16 devid, struct 
protection_domain *domain, bool ats)

pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
<< DEV_ENTRY_MODE_SHIFT;
-   pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
+   pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_V | IOMMU_PTE_TV;

flags = amd_iommu_dev_table[devid].data[1];

@@ -2021,7 +2021,7 @@ static void set_dte_entry(u16 devid, struct 
protection_domain *domain, bool ats)
  static void clear_dte_entry(u16 devid)
  {
/* remove entry from the device table seen by the hardware */
-   amd_iommu_dev_table[devid].data[0]  = IOMMU_PTE_P | IOMMU_PTE_TV;
+   amd_iommu_dev_table[devid].data[0]  = IOMMU_PTE_V | IOMMU_PTE_TV;
amd_iommu_dev_table[devid].data[1] &= DTE_FLAG_MASK;

amd_iommu_apply_erratum_63(devid);
@@ -2463,7 +2463,7 @@ static dma_addr_t dma_ops_domain_map(struct 
dma_ops_domain *dom,
if (!pte)
return DMA_ERROR_CODE;

-   __pte = paddr | IOMMU_PTE_P | IOMMU_PTE_FC;
+   __pte = paddr | IOMMU_PTE_V | IOMMU_PTE_FC;

if (direction == DMA_TO_DEVICE)
__pte |= IOMMU_PTE_IR;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 9b8ace4..65f7988 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -244,7 +244,7 @@
  #define PM_LEVEL_INDEX(x, a)  (((a) >> PM_LEVEL_SHIFT((x))) & 0x1ffULL)
  #define PM_LEVEL_ENC(x)   (((x) << 9) & 0xe00ULL)
  #define PM_LEVEL_PDE(x, a)((a) | PM_LEVEL_ENC((x)) | \
-IOMMU_PTE_P | IOMMU_PTE_IR | IOMMU_PTE_IW)
+IOMMU_PTE_V | IOMMU_PTE_IR | IOMMU_PTE_IW)
  #define PM_PTE_LEVEL(pte) (((pte) >> 9) & 0x7ULL)

  #define PM_MAP_4k 0
@@ -293,7 +293,7 @@
  #define PTE_LEVEL_PAGE_SIZE(level)\
(1ULL << (12 + (9 * (level

-#define IOMMU_PTE_P  (1ULL << 0)
+#define IOMMU_PTE_V  (1ULL << 0)
  #define IOMMU_PTE_TV (1ULL << 1)
  #define IOMMU_PTE_U  (1ULL << 59)
  #define IOMMU_PTE_FC (1ULL << 60)
@@ -321,7 +321,7 @@
  #define GCR3_VALID0x01ULL

  #define IOMMU_PAGE_MASK (((1ULL << 52) - 1) & ~0xfffULL)
-#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_P)
+#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_V)
  #define IOMMU_PTE_PAGE(pte) (phys_to_virt((pte) & IOMMU_PAGE_MASK))
  #define IOMMU_PTE_MODE(pte) (((pte) >> 9) & 0x07)



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

Re: [Patch v3 06/12] iommu/amd: Clean up the useless IOMMU_PTE_U/IOMMU_PTE_FC

[Wan Zongshun]

 Original Message 

In amd-vi spec bit[60:58] are only used to store the bit[14:12] of GCR3.
No any other useage is found in several versions of amd-vi spec. So remove
them in this patch.


Also,this patch also made me confusion, please keep FC bit here, bit[60] 
should be PTE's FC bit.


Vincent.



Signed-off-by: Baoquan He 
---
  drivers/iommu/amd_iommu.c   | 6 +++---
  drivers/iommu/amd_iommu_types.h | 2 --
  2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index f02d4b1..93bc690 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1331,9 +1331,9 @@ static int iommu_map_page(struct protection_domain *dom,

if (count > 1) {
__pte = PAGE_SIZE_PTE(phys_addr, page_size);
-   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_V | IOMMU_PTE_FC;
+   __pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_V;
} else
-   __pte = phys_addr | IOMMU_PTE_V | IOMMU_PTE_FC;
+   __pte = phys_addr | IOMMU_PTE_V;

if (prot & IOMMU_PROT_IR)
__pte |= IOMMU_PTE_IR;
@@ -2463,7 +2463,7 @@ static dma_addr_t dma_ops_domain_map(struct 
dma_ops_domain *dom,
if (!pte)
return DMA_ERROR_CODE;

-   __pte = paddr | IOMMU_PTE_V | IOMMU_PTE_FC;
+   __pte = paddr | IOMMU_PTE_V;

if (direction == DMA_TO_DEVICE)
__pte |= IOMMU_PTE_IR;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 65f7988..42cd3d5 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -295,8 +295,6 @@

  #define IOMMU_PTE_V  (1ULL << 0)
  #define IOMMU_PTE_TV (1ULL << 1)
-#define IOMMU_PTE_U  (1ULL << 59)
-#define IOMMU_PTE_FC (1ULL << 60)
  #define IOMMU_PTE_IR (1ULL << 61)
  #define IOMMU_PTE_IW (1ULL << 62)



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

Re: [Patch v3 03/12] iommu/amd: move dte irq macro defitions to amd_iommu_types.h

[Wan Zongshun]

 Original Message 

These macro definitions are also needed by irq table copy function
later, so move them to amd_iommu_types.h.


Typo for your subject (defitions?).



Signed-off-by: Baoquan He 
---
  drivers/iommu/amd_iommu.c   | 4 
  drivers/iommu/amd_iommu_types.h | 5 +
  2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index dd869fe..aeae07a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3627,10 +3627,6 @@ struct amd_ir_data {

  static struct irq_chip amd_ir_chip;

-#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
-#define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
-#define DTE_IRQ_TABLE_LEN   (8ULL << 1)
-#define DTE_IRQ_REMAP_ENABLE1ULL

  static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
  {
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 9d32b20..9b8ace4 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -224,6 +224,11 @@

  #define PPR_REQ_FAULT 0x01

+#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
+#define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
+#define DTE_IRQ_TABLE_LEN   (8ULL << 1)
+#define DTE_IRQ_REMAP_ENABLE1ULL
+
  #define PAGE_MODE_NONE0x00
  #define PAGE_MODE_1_LEVEL 0x01
  #define PAGE_MODE_2_LEVEL 0x02


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

Re: [Patch v3 11/12] iommu/amd: No need to wait iommu completion if no dte irq entry change

[Wan Zongshun]

 Original Message 

This is a clean up. In get_irq_table() only if DTE entry is changed
iommu_completion_wait() need be called. Otherwise no need to do it.

Signed-off-by: Baoquan He 
---
  drivers/iommu/amd_iommu.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 0880500..dda703b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3674,7 +3674,7 @@ static struct irq_remap_table *get_irq_table(u16 devid, 
bool ioapic)

table = irq_lookup_table[devid];
if (table)
-   goto out;
+   goto out_unlock;

alias = amd_iommu_alias_table[devid];
table = irq_lookup_table[alias];
@@ -3688,7 +3688,7 @@ static struct irq_remap_table *get_irq_table(u16 devid, 
bool ioapic)
/* Nothing there yet, allocate new irq remapping table */
table = kzalloc(sizeof(*table), GFP_ATOMIC);
if (!table)
-   goto out;
+   goto out_unlock;

/* Initialize table spin-lock */
spin_lock_init(>lock);
@@ -3701,7 +3701,7 @@ static struct irq_remap_table *get_irq_table(u16 devid, 
bool ioapic)
if (!table->table) {
kfree(table);
table = NULL;
-   goto out;
+   goto out_unlock;
}

memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));



So here the 'out:' label text should also remove, since no err need goto 
it, right?


out:
iommu_completion_wait(iommu);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH V2 0/8] iommu/amd: enable ACPI hardware ID device support

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

There are some devices indentified using ACPI HID format in AMD chip.
This patch series enable iommu support for those ACPI HID device, 
since the existing AMD iommu only supports PCI bus based device.

The latest public version of AMD IOMMU specification that describes
the support for IVHD type 40h and ACPI HID IVHD device type, 
implemented by this patch series, is available here:

http://support.amd.com/TechDocs/48882_IOMMU.pdf


The V2 adds two new patches: patch 5 and patch 8 according to Joerg's
comments.

There is a little modification in patch 6 to distinguish pci and none pci.


Suravee Suthikulpanit (3):
  iommu/amd: Modify ivhd_header structure to support type 11h and 40h
  iommu/amd: Use the most comprehensive IVHD type that the driver can
support
  iommu/amd: Introduces ivrs_acpihid kernel parameter

Wan Zongshun (5):
  iommu/amd: Add new map for storing IVHD dev entry type HID
  iommu/amd: Make call-sites of get_device_id aware of its return value
  iommu/amd: Add iommu support for ACPI HID devices
  iommu/amd: Manage iommu_group for ACPI HID devices
  iommu/amd: Set AMD iommu callbacks for amba bus

 Documentation/kernel-parameters.txt |   7 +
 drivers/iommu/amd_iommu.c   | 167 ---
 drivers/iommu/amd_iommu_init.c  | 309 +++-
 drivers/iommu/amd_iommu_types.h |  14 ++
 4 files changed, 440 insertions(+), 57 deletions(-)

-- 
1.9.1

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

[PATCH V2 1/8] iommu/amd: Modify ivhd_header structure to support type 11h and 40h

[Wan Zongshun]

From: Suravee Suthikulpanit 

This patch modifies the existing struct ivhd_header, which currently
only support IVHD type 0x10, to add new fields from IVHD type 11h and 40h.
It also modifies the pointer calculation to allow support for IVHD type
11h and 40h

Signed-off-by: Suravee Suthikulpanit 
---
 drivers/iommu/amd_iommu_init.c | 47 +++---
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..2ff7000 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -88,7 +88,7 @@
 
 /*
  * structure describing one IOMMU in the ACPI table. Typically followed by one
- * or more ivhd_entrys.
+ * or more ivhd_entrys. This struct supports both IVHD type 10h, 11h and 40h.
  */
 struct ivhd_header {
u8 type;
@@ -99,7 +99,11 @@ struct ivhd_header {
u64 mmio_phys;
u16 pci_seg;
u16 info;
-   u32 efr;
+   u32 efr_attr;
+
+   /* Following only valid on IVHD type 11h and 40h */
+   u64 efr_reg_img; /* Exact copy of MMIO_EXT_FEATURES */
+   u64 res;
 } __attribute__((packed));
 
 /*
@@ -399,6 +403,22 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu 
*iommu)
  *
  /
 
+static inline u32 get_ivhd_header_size(struct ivhd_header *h)
+{
+   u32 size = 0;
+
+   switch (h->type) {
+   case 0x10:
+   size = 24;
+   break;
+   case 0x11:
+   case 0x40:
+   size = 40;
+   break;
+   }
+   return size;
+}
+
 /*
  * This function calculates the length of a given IVHD entry
  */
@@ -415,8 +435,14 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
 {
u8 *p = (void *)h, *end = (void *)h;
struct ivhd_entry *dev;
+   u32 ivhd_size = get_ivhd_header_size(h);
+
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
 
-   p += sizeof(*h);
+   p += ivhd_size;
end += h->length;
 
while (p < end) {
@@ -781,6 +807,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
u32 dev_i, ext_flags = 0;
bool alias = false;
struct ivhd_entry *e;
+   u32 ivhd_size;
int ret;
 
 
@@ -796,7 +823,13 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
/*
 * Done. Now parse the device entries
 */
-   p += sizeof(struct ivhd_header);
+   ivhd_size = get_ivhd_header_size(h);
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
+
+   p += ivhd_size;
end += h->length;
 
 
@@ -1047,9 +1080,9 @@ static int __init init_iommu_one(struct amd_iommu *iommu, 
struct ivhd_header *h)
iommu->mmio_phys = h->mmio_phys;
 
/* Check if IVHD EFR contains proper max banks/counters */
-   if ((h->efr != 0) &&
-   ((h->efr & (0xF << 13)) != 0) &&
-   ((h->efr & (0x3F << 17)) != 0)) {
+   if ((h->efr_attr != 0) &&
+   ((h->efr_attr & (0xF << 13)) != 0) &&
+   ((h->efr_attr & (0x3F << 17)) != 0)) {
iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
} else {
iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
-- 
1.9.1

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

[PATCH V2 4/8] iommu/amd: Introduces ivrs_acpihid kernel parameter

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

This patch introduces a new kernel parameter, ivrs_acpihid.
This is used to override existing ACPI-HID IVHD device entry,
or add an entry in case it is missing in the IVHD.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 Documentation/kernel-parameters.txt |  7 +++
 drivers/iommu/amd_iommu_init.c  | 33 +
 2 files changed, 40 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 742f69d..5c364c6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1680,6 +1680,13 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
PCI device 00:14.0 write the parameter as:
ivrs_hpet[0]=00:14.0
 
+   ivrs_acpihid[HW,X86_64]
+   Provide an override to the ACPI-HID:UID<->DEVICE-ID
+   mapping provided in the IVRS ACPI table. For
+   example, to map UART-HID:UID AMD0020:0 to
+   PCI device 00:14.5 write the parameter as:
+   ivrs_acpihid[00:14.5]=AMD0020:0
+
js= [HW,JOY] Analog joystick
See Documentation/input/joystick.txt.
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 0a8e033..ad05614 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2430,10 +2430,43 @@ static int __init parse_ivrs_hpet(char *str)
return 1;
 }
 
+static int __init parse_ivrs_acpihid(char *str)
+{
+   u32 bus, dev, fn;
+   char *hid, *uid, *p;
+   char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0};
+   int ret, i;
+
+   ret = sscanf(str, "[%x:%x.%x]=%s", , , , acpiid);
+   if (ret != 4) {
+   pr_err("AMD-Vi: Invalid command line: ivrs_acpihid(%s)\n", str);
+   return 1;
+   }
+
+   p = acpiid;
+   hid = strsep(, ":");
+   uid = p;
+
+   if (!hid || !(*hid) || !uid) {
+   pr_err("AMD-Vi: Invalid command line: hid or uid\n");
+   return 1;
+   }
+
+   i = early_acpihid_map_size++;
+   memcpy(early_acpihid_map[i].hid, hid, strlen(hid));
+   memcpy(early_acpihid_map[i].uid, uid, strlen(uid));
+   early_acpihid_map[i].devid =
+   ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+   early_acpihid_map[i].cmd_line   = true;
+
+   return 1;
+}
+
 __setup("amd_iommu_dump",  parse_amd_iommu_dump);
 __setup("amd_iommu=",  parse_amd_iommu_options);
 __setup("ivrs_ioapic", parse_ivrs_ioapic);
 __setup("ivrs_hpet",   parse_ivrs_hpet);
+__setup("ivrs_acpihid",parse_ivrs_acpihid);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
  gart_iommu_hole_init,
-- 
1.9.1

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

[PATCH V2 3/8] iommu/amd: Add new map for storing IVHD dev entry type HID

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch introduces acpihid_map, which is used to store
the new IVHD device entry extracted from BIOS IVRS table.

It also provides a utility function add_acpi_hid_device(),
to add this types of devices to the map.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c   |   1 +
 drivers/iommu/amd_iommu_init.c  | 122 
 drivers/iommu/amd_iommu_types.h |  14 +
 3 files changed, 137 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 539b0de..8063ab2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,7 @@ static DEFINE_SPINLOCK(dev_data_list_lock);
 
 LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
+LIST_HEAD(acpihid_map);
 
 /*
  * Domain for untranslated devices - only allocated
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 146f3ee..0a8e033 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -60,6 +60,10 @@
 #define IVHD_DEV_SPECIAL   0x48
 #define IVHD_DEV_ACPI_HID  0xf0
 
+#define UID_NOT_PRESENT 0
+#define UID_IS_INTEGER  1
+#define UID_IS_CHARACTER2
+
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
 
@@ -116,6 +120,11 @@ struct ivhd_entry {
u16 devid;
u8 flags;
u32 ext;
+   u32 hidh;
+   u64 cid;
+   u8 uidf;
+   u8 uidl;
+   u8 uid;
 } __attribute__((packed));
 
 /*
@@ -224,8 +233,12 @@ enum iommu_init_state {
 #define EARLY_MAP_SIZE 4
 static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static struct acpihid_map_entry __initdata early_acpihid_map[EARLY_MAP_SIZE];
+
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static int __initdata early_acpihid_map_size;
+
 static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
@@ -760,6 +773,42 @@ static int __init add_special_device(u8 type, u8 id, u16 
*devid, bool cmd_line)
return 0;
 }
 
+static int __init add_acpi_hid_device(u8 *hid, u8 *uid, u16 *devid,
+ bool cmd_line)
+{
+   struct acpihid_map_entry *entry;
+   struct list_head *list = _map;
+
+   list_for_each_entry(entry, list, list) {
+   if (strcmp(entry->hid, hid) ||
+   (*uid && *entry->uid && strcmp(entry->uid, uid)) ||
+   !entry->cmd_line)
+   continue;
+
+   pr_info("AMD-Vi: Command-line override for hid:%s uid:%s\n",
+   hid, uid);
+   *devid = entry->devid;
+   return 0;
+   }
+
+   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+   if (!entry)
+   return -ENOMEM;
+
+   memcpy(entry->uid, uid, strlen(uid));
+   memcpy(entry->hid, hid, strlen(hid));
+   entry->devid = *devid;
+   entry->cmd_line = cmd_line;
+   entry->root_devid = (entry->devid & (~0x7));
+
+   pr_info("AMD-Vi:%s, add hid:%s, uid:%s, rdevid:%d\n",
+   entry->cmd_line ? "cmd" : "ivrs",
+   entry->hid, entry->uid, entry->root_devid);
+
+   list_add_tail(>list, list);
+   return 0;
+}
+
 static int __init add_early_maps(void)
 {
int i, ret;
@@ -782,6 +831,15 @@ static int __init add_early_maps(void)
return ret;
}
 
+   for (i = 0; i < early_acpihid_map_size; ++i) {
+   ret = add_acpi_hid_device(early_acpihid_map[i].hid,
+ early_acpihid_map[i].uid,
+ _acpihid_map[i].devid,
+ early_acpihid_map[i].cmd_line);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -1001,6 +1059,70 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
 
break;
}
+   case IVHD_DEV_ACPI_HID: {
+   u16 devid;
+   u8 hid[ACPIHID_HID_LEN] = {0};
+   u8 uid[ACPIHID_UID_LEN] = {0};
+   int ret;
+
+   if (h->type != 0x40) {
+   pr_err(FW_BUG "Invalid IVHD device type %#x\n",
+  e->type);
+   break;
+   }
+
+   memcpy(hid, (u8 *)(>ext), ACPIHID_HID_LEN - 1);
+   hid[ACPIHID_HID_LEN - 1] = '

[PATCH V2 2/8] iommu/amd: Use the most comprehensive IVHD type that the driver can support

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

The IVRS in more recent AMD system usually contains multiple
IVHD block types (e.g. 0x10, 0x11, and 0x40) for each IOMMU.
The newer IVHD types provide more information (e.g. new features
specified in the IOMMU spec), while maintain compatibility with
the older IVHD type.

Having multiple IVHD type allows older IOMMU drivers to still function
(e.g. using the older IVHD type 0x10) while the newer IOMMU driver can use
the newer IVHD types (e.g. 0x11 and 0x40). Therefore, the IOMMU driver
should only make use of the newest IVHD type that it can support.

This patch adds new logic to determine the highest level of IVHD type
it can support, and use it throughout the to initialize the driver.
This requires adding another pass to the IVRS parsing to determine
appropriate IVHD type (see function get_highest_supported_ivhd_type())
before parsing the contents.

[Vincent: fix the build error of IVHD_DEV_ACPI_HID flag not found]

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 107 ++---
 1 file changed, 78 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2ff7000..146f3ee 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -44,7 +44,7 @@
  */
 #define IVRS_HEADER_LENGTH 48
 
-#define ACPI_IVHD_TYPE  0x10
+#define ACPI_IVHD_TYPE_MAX_SUPPORTED   0x40
 #define ACPI_IVMD_TYPE_ALL  0x20
 #define ACPI_IVMD_TYPE  0x21
 #define ACPI_IVMD_TYPE_RANGE0x22
@@ -58,6 +58,7 @@
 #define IVHD_DEV_EXT_SELECT 0x46
 #define IVHD_DEV_EXT_SELECT_RANGE   0x47
 #define IVHD_DEV_SPECIAL   0x48
+#define IVHD_DEV_ACPI_HID  0xf0
 
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
@@ -137,6 +138,7 @@ bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
+static int amd_iommu_target_ivhd_type;
 
 u16 amd_iommu_last_bdf;/* largest PCI device id we have
   to handle */
@@ -424,7 +426,15 @@ static inline u32 get_ivhd_header_size(struct ivhd_header 
*h)
  */
 static inline int ivhd_entry_length(u8 *ivhd)
 {
-   return 0x04 << (*ivhd >> 6);
+   u32 type = ((struct ivhd_entry *)ivhd)->type;
+
+   if (type < 0x80) {
+   return 0x04 << (*ivhd >> 6);
+   } else if (type == IVHD_DEV_ACPI_HID) {
+   /* For ACPI_HID, offset 21 is uid len */
+   return *((u8 *)ivhd + 21) + 22;
+   }
+   return 0;
 }
 
 /*
@@ -470,6 +480,22 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
return 0;
 }
 
+static int __init check_ivrs_checksum(struct acpi_table_header *table)
+{
+   int i;
+   u8 checksum = 0, *p = (u8 *)table;
+
+   for (i = 0; i < table->length; ++i)
+   checksum += p[i];
+   if (checksum != 0) {
+   /* ACPI table corrupt */
+   pr_err(FW_BUG "AMD-Vi: IVRS invalid checksum\n");
+   return -ENODEV;
+   }
+
+   return 0;
+}
+
 /*
  * Iterate over all IVHD entries in the ACPI table and find the highest device
  * id which we need to handle. This is the first of three functions which parse
@@ -477,31 +503,19 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
  */
 static int __init find_last_devid_acpi(struct acpi_table_header *table)
 {
-   int i;
-   u8 checksum = 0, *p = (u8 *)table, *end = (u8 *)table;
+   u8 *p = (u8 *)table, *end = (u8 *)table;
struct ivhd_header *h;
 
-   /*
-* Validate checksum here so we don't need to do it when
-* we actually parse the table
-*/
-   for (i = 0; i < table->length; ++i)
-   checksum += p[i];
-   if (checksum != 0)
-   /* ACPI table corrupt */
-   return -ENODEV;
-
p += IVRS_HEADER_LENGTH;
 
end += table->length;
while (p < end) {
h = (struct ivhd_header *)p;
-   switch (h->type) {
-   case ACPI_IVHD_TYPE:
-   find_last_devid_from_ivhd(h);
-   break;
-   default:
-   break;
+   if (h->type == amd_iommu_target_ivhd_type) {
+   int ret = find_last_devid_from_ivhd(h);
+
+   if (ret)
+   return ret;
}
p += h->length;
}
@@ -1118,6 +1132,32 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)
return 0;
 }
 
+/**
+ * get_highest_supported

[PATCH V2 6/8] iommu/amd: Add iommu support for ACPI HID devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

Current IOMMU driver make assumption that the downstream devices are PCI.
With the newly added ACPI-HID IVHD device entry support, this is no
longer true. This patch is to add dev type check and to distinguish the
pci and acpihid device code path.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c | 69 ---
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 10d6623..4857d66 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -215,13 +216,60 @@ static struct iommu_dev_data *find_dev_data(u16 devid)
return dev_data;
 }
 
-static inline u16 get_device_id(struct device *dev)
+static inline int match_hid_uid(struct device *dev,
+   struct acpihid_map_entry *entry)
+{
+   const char *hid, *uid;
+
+   hid = acpi_device_hid(ACPI_COMPANION(dev));
+   uid = acpi_device_uid(ACPI_COMPANION(dev));
+
+   if (!hid || !(*hid))
+   return -ENODEV;
+
+   if (!uid || !(*uid))
+   return strcmp(hid, entry->hid);
+
+   if (!(*entry->uid))
+   return strcmp(hid, entry->hid);
+
+   return (strcmp(hid, entry->hid) || strcmp(uid, entry->uid));
+}
+
+static inline u16 get_pci_device_id(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
 
return PCI_DEVID(pdev->bus->number, pdev->devfn);
 }
 
+static inline int get_acpihid_device_id(struct device *dev,
+   struct acpihid_map_entry **entry)
+{
+   struct acpihid_map_entry *p;
+
+   list_for_each_entry(p, _map, list) {
+   if (!match_hid_uid(dev, p)) {
+   if (entry)
+   *entry = p;
+   return p->devid;
+   }
+   }
+   return -EINVAL;
+}
+
+static inline int get_device_id(struct device *dev)
+{
+   int devid;
+
+   if (dev_is_pci(dev))
+   devid = get_pci_device_id(dev);
+   else
+   devid = get_acpihid_device_id(dev, NULL);
+
+   return devid;
+}
+
 static struct iommu_dev_data *get_dev_data(struct device *dev)
 {
return dev->archdata.iommu;
@@ -302,10 +350,6 @@ static bool check_device(struct device *dev)
if (!dev || !dev->dma_mask)
return false;
 
-   /* No PCI device */
-   if (!dev_is_pci(dev))
-   return false;
-
devid = get_device_id(dev);
if (IS_ERR_VALUE(devid))
return false;
@@ -343,7 +387,6 @@ out:
 
 static int iommu_init_device(struct device *dev)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
int devid;
 
@@ -358,10 +401,10 @@ static int iommu_init_device(struct device *dev)
if (!dev_data)
return -ENOMEM;
 
-   if (pci_iommuv2_capable(pdev)) {
+   if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
 
-   iommu  = amd_iommu_rlookup_table[dev_data->devid];
+   iommu = amd_iommu_rlookup_table[dev_data->devid];
dev_data->iommu_v2 = iommu->is_iommu_v2;
}
 
@@ -2235,13 +2278,17 @@ static bool pci_pri_tlp_required(struct pci_dev *pdev)
 static int attach_device(struct device *dev,
 struct protection_domain *domain)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
+   struct pci_dev *pdev;
struct iommu_dev_data *dev_data;
unsigned long flags;
int ret;
 
dev_data = get_dev_data(dev);
 
+   if (!dev_is_pci(dev))
+   goto skip_ats_check;
+
+   pdev = to_pci_dev(dev);
if (domain->flags & PD_IOMMUV2_MASK) {
if (!dev_data->passthrough)
return -EINVAL;
@@ -2260,6 +2307,7 @@ static int attach_device(struct device *dev,
dev_data->ats.qdep= pci_ats_queue_depth(pdev);
}
 
+skip_ats_check:
write_lock_irqsave(_iommu_devtable_lock, flags);
ret = __attach_device(dev_data, domain);
write_unlock_irqrestore(_iommu_devtable_lock, flags);
@@ -2316,6 +2364,9 @@ static void detach_device(struct device *dev)
__detach_device(dev_data);
write_unlock_irqrestore(_iommu_devtable_lock, flags);
 
+   if (!dev_is_pci(dev))
+   return;
+
if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
pdev_iommuv2_disable(to_pci_dev(dev));
else if (dev_data->ats.enabled)
-- 
1.9.1

_

[PATCH V2 7/8] iommu/amd: Manage iommu_group for ACPI HID devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch creates a new function for finding or creating an IOMMU
group for acpihid(ACPI Hardware ID) device.

The acpihid devices with the same devid will be put into same group and
there will have the same domain id and share the same page table.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 4857d66..59398f9 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -275,6 +275,29 @@ static struct iommu_dev_data *get_dev_data(struct device 
*dev)
return dev->archdata.iommu;
 }
 
+/*
+* Find or create an IOMMU group for a acpihid device.
+*/
+static struct iommu_group *acpihid_device_group(struct device *dev)
+{
+   struct acpihid_map_entry *p, *entry = NULL;
+   u16 devid;
+
+   devid = get_acpihid_device_id(dev, );
+   if (devid < 0)
+   return ERR_PTR(devid);
+
+   list_for_each_entry(p, _map, list) {
+   if ((devid == p->devid) && p->group)
+   entry->group = p->group;
+   }
+
+   if (!entry->group)
+   entry->group = generic_device_group(dev);
+
+   return entry->group;
+}
+
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
static const int caps[] = {
@@ -2441,6 +2464,14 @@ static void amd_iommu_remove_device(struct device *dev)
iommu_completion_wait(iommu);
 }
 
+static struct iommu_group *amd_iommu_device_group(struct device *dev)
+{
+   if (dev_is_pci(dev))
+   return pci_device_group(dev);
+
+   return acpihid_device_group(dev);
+}
+
 /*
  *
  * The next functions belong to the dma_ops mapping/unmapping code.
@@ -3282,7 +3313,7 @@ static const struct iommu_ops amd_iommu_ops = {
.iova_to_phys = amd_iommu_iova_to_phys,
.add_device = amd_iommu_add_device,
.remove_device = amd_iommu_remove_device,
-   .device_group = pci_device_group,
+   .device_group = amd_iommu_device_group,
.get_dm_regions = amd_iommu_get_dm_regions,
.put_dm_regions = amd_iommu_put_dm_regions,
.pgsize_bitmap  = AMD_IOMMU_PGSIZES,
-- 
1.9.1

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

[PATCH V2 5/8] iommu/amd: Make call-sites of get_device_id aware of its return value

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch is to make the call-sites of get_device_id aware of its
return value.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 51 +--
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8063ab2..10d6623 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -278,9 +278,11 @@ static void init_unity_mappings_for_device(struct device 
*dev,
   struct dma_ops_domain *dma_dom)
 {
struct unity_map_entry *e;
-   u16 devid;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
list_for_each_entry(e, _iommu_unity_map, list) {
if (!(devid >= e->devid_start && devid <= e->devid_end))
@@ -295,7 +297,7 @@ static void init_unity_mappings_for_device(struct device 
*dev,
  */
 static bool check_device(struct device *dev)
 {
-   u16 devid;
+   int devid;
 
if (!dev || !dev->dma_mask)
return false;
@@ -305,6 +307,8 @@ static bool check_device(struct device *dev)
return false;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return false;
 
/* Out of our scope? */
if (devid > amd_iommu_last_bdf)
@@ -341,11 +345,16 @@ static int iommu_init_device(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
+   int devid;
 
if (dev->archdata.iommu)
return 0;
 
-   dev_data = find_dev_data(get_device_id(dev));
+   devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return devid;
+
+   dev_data = find_dev_data(devid);
if (!dev_data)
return -ENOMEM;
 
@@ -366,9 +375,13 @@ static int iommu_init_device(struct device *dev)
 
 static void iommu_ignore_device(struct device *dev)
 {
-   u16 devid, alias;
+   u16 alias;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
+
alias = amd_iommu_alias_table[devid];
 
memset(_iommu_dev_table[devid], 0, sizeof(struct dev_table_entry));
@@ -380,8 +393,14 @@ static void iommu_ignore_device(struct device *dev)
 
 static void iommu_uninit_device(struct device *dev)
 {
-   struct iommu_dev_data *dev_data = search_dev_data(get_device_id(dev));
+   int devid;
+   struct iommu_dev_data *dev_data;
+
+   devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
+   dev_data = search_dev_data(devid);
if (!dev_data)
return;
 
@@ -2310,13 +2329,15 @@ static int amd_iommu_add_device(struct device *dev)
struct iommu_dev_data *dev_data;
struct iommu_domain *domain;
struct amd_iommu *iommu;
-   u16 devid;
-   int ret;
+   int ret, devid;
 
if (!check_device(dev) || get_dev_data(dev))
return 0;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return devid;
+
iommu = amd_iommu_rlookup_table[devid];
 
ret = iommu_init_device(dev);
@@ -2354,12 +2375,15 @@ out:
 static void amd_iommu_remove_device(struct device *dev)
 {
struct amd_iommu *iommu;
-   u16 devid;
+   int devid;
 
if (!check_device(dev))
return;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
+
iommu = amd_iommu_rlookup_table[devid];
 
iommu_uninit_device(dev);
@@ -3031,12 +3055,14 @@ static void amd_iommu_detach_device(struct iommu_domain 
*dom,
 {
struct iommu_dev_data *dev_data = dev->archdata.iommu;
struct amd_iommu *iommu;
-   u16 devid;
+   int devid;
 
if (!check_device(dev))
return;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
if (dev_data->domain != NULL)
detach_device(dev);
@@ -3154,9 +3180,11 @@ static void amd_iommu_get_dm_regions(struct device *dev,
 struct list_head *head)
 {
struct unity_map_entry *entry;
-   u16 devid;
+   int devid;
 
devid = get_device_id(dev);
+   if (IS_ERR_VALUE(devid))
+   return;
 
list_for_each_entry(entry, _iommu_unity_map, list) {
struct iommu_dm_region *region;
@@ -3858,6 +3886,9 @@ static struct irq_domain *get_irq_domain(struct 
irq_alloc_info *info)
case X86_IRQ_ALLOC_TYPE_MSI:
case X86_IRQ_ALLOC_TYPE_MSIX:
devid = get_device_id(>msi_dev->dev);
+   if (IS_ERR_VALUE(devid))
+   return NULL;
+

[PATCH V2 8/8] iommu/amd: Set AMD iommu callbacks for amba bus

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

AMD Uart DMA belongs to ACPI HID type device, and its driver
is basing on AMBA Bus, need also IOMMU support.

This patch is just to set the AMD iommu callbacks for amba bus.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 59398f9..9194161 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2965,7 +2966,17 @@ static struct dma_map_ops amd_iommu_dma_ops = {
 
 int __init amd_iommu_init_api(void)
 {
-   return bus_set_iommu(_bus_type, _iommu_ops);
+   int err = 0;
+
+   err = bus_set_iommu(_bus_type, _iommu_ops);
+   if (err)
+   return err;
+#ifdef CONFIG_ARM_AMBA
+   err = bus_set_iommu(_bustype, _iommu_ops);
+   if (err)
+   return err;
+#endif
+   return 0;
 }
 
 int __init amd_iommu_init_dma_ops(void)
-- 
1.9.1

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

Re: [PATCH] iommu/amd: set AMD iommu-callbacks for the amba bus

[Wan ZongShun]

2015-12-30 0:54 GMT+08:00 Wan Zongshun <vincent@amd.com>:
> From: Wan Zongshun <vincent@amd.com>
>
> Since uart dma is using AMD iommu, and it bases on amba bus.
> So we need set callbacks for amba bus type firstly.
>
> Signed-off-by: Wan Zongshun <vincent@amd.com>

Joerg,
How about this patch?
It is a separating patch to our acpihid device support patches.

> ---
>  drivers/iommu/amd_iommu.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 8b2be1e..9097b11 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2758,7 +2759,17 @@ static struct dma_map_ops amd_iommu_dma_ops = {
>
>  int __init amd_iommu_init_api(void)
>  {
> -   return bus_set_iommu(_bus_type, _iommu_ops);
> +   int err = 0;
> +
> +   err = bus_set_iommu(_bus_type, _iommu_ops);
> +   if (err)
> +   return err;
> +#ifdef CONFIG_ARM_AMBA
> +   err = bus_set_iommu(_bustype, _iommu_ops);
> +   if (err)
> +   return err;
> +#endif
> +   return 0;
>  }
>
>  int __init amd_iommu_init_dma_ops(void)
> --
> 1.9.1
>
> _______
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices

[Wan Zongshun]

static bool check_device(struct device *dev)
{
 u16 devid;
..

 /* No PCI device */
 if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
 return false;

 devid = get_device_id(dev);


That is true for this case, but the other call-sites of get_device_id()
still have to care about a potential negative return value, right?



Actually I am supposing the '.add_device' will be the first called in 
iommu initializing stage, so I think as long as having no error of check 
device here, any call-sites of get_device_id() will be fine, because 
adding device successfully should be the pre-condition of any iommu 
function can be performed, please correct me.


static int amd_iommu_add_device(struct device *dev)
{
struct iommu_dev_data *dev_data;
struct iommu_domain *domain;
struct amd_iommu *iommu;
u16 devid;
int ret;

if (!check_device(dev) || get_dev_data(dev))
return 0;

devid = get_device_id(dev);
iommu = amd_iommu_rlookup_table[devid];


Vincent.


Joerg



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

Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices

[Wan ZongShun]

2016-01-07 20:04 GMT+08:00 Joerg Roedel <j...@8bytes.org>:
> On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
>> -static inline u16 get_device_id(struct device *dev)
>> +static inline int match_hid_uid(struct device *dev,
>> + struct acpihid_map_entry *entry)
>> +{
>> + const char *hid, *uid;
>> +
>> + hid = acpi_device_hid(ACPI_COMPANION(dev));
>> + uid = acpi_device_uid(ACPI_COMPANION(dev));
>> +
>> + if (!hid || !(*hid))
>> + return -ENODEV;
>> +
>> + if (!uid || !(*uid))
>> + return strcmp(hid, entry->hid);
>> +
>> + if (!(*entry->uid))
>> + return strcmp(hid, entry->hid);
>> +
>> + return -ENODEV;
>> +}
>> +
>> +static inline u16 get_pci_device_id(struct device *dev)
>>  {
>>   struct pci_dev *pdev = to_pci_dev(dev);
>>
>>   return PCI_DEVID(pdev->bus->number, pdev->devfn);
>>  }
>>
>> +static inline int get_acpihid_device_id(struct device *dev,
>> + struct acpihid_map_entry **entry)
>> +{
>> + struct acpihid_map_entry *p;
>> +
>> + list_for_each_entry(p, _map, list) {
>> + if (!match_hid_uid(dev, p)) {
>> + if (entry)
>> + *entry = p;
>> + return p->devid;
>> + }
>> + }
>> + return -EINVAL;
>> +}
>> +
>> +static inline u16 get_device_id(struct device *dev)
>> +{
>> + if (dev_is_pci(dev))
>> + return get_pci_device_id(dev);
>> + else
>> + return get_acpihid_device_id(dev, NULL);
>> +}
>
> This is not robust, get_acpihid_device_id() returns int and can return a
> negative value. This gets lost when converting it to u16 here. So either
> you add error handling for get_acpihid_device_id() in get_device_id() or
> you change get_device_id() to return int too and handle the error at the
> callers of get_device_id().

Joerg,

Please see the following function, since I judge this
'get_acpihid_device_id(dev, NULL) < 0'  in the front of
'get_device_id', so your concern should not exist. I have already
filtered the negative situation in check_device firstly, do you think
it is ok?


static bool check_device(struct device *dev)
{
u16 devid;
..

/* No PCI device */
if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
return false;

devid = get_device_id(dev);

.

return true;
}


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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices

[Wan ZongShun]

2016-01-07 20:06 GMT+08:00 Joerg Roedel <j...@8bytes.org>:
> On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
>> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
>> +{
>> + if (dev_is_pci(dev))
>> + return pci_device_group(dev);
>> +
>> + return acpihid_device_group(dev);
>> +}
>> +
>>  
>> /*
>>   *
>>   * The next functions belong to the dma_ops mapping/unmapping code.
>> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>>   .iova_to_phys = amd_iommu_iova_to_phys,
>>   .add_device = amd_iommu_add_device,
>>   .remove_device = amd_iommu_remove_device,
>> - .device_group = pci_device_group,
>> + .device_group = amd_iommu_device_group,
>
> Does this work? Which bus do the ACPIHID devices belong to (what does
> dev->bus point to)? If it is not _bus_type, then the iommu core code
> will not create an iommu group for the devices.

Yes, it works, we have already done validation on AMD platform.
Please refer to my previous patch :
[PATCH] iommu/amd: set AMD iommu-callbacks for the amba bus.

Currently, The UART DMA is the use case of ACPIHID device, the
dev->bus is amba_bus, so I add bus_set_iommu for this bus type.

We will do create iommu group in acpihid_device_group which will call
generic_device_group to alloc group.

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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/6] iommu/amd: Modify ivhd_header structure to support type 11h and 40h

[Wan Zongshun]

From: Suravee Suthikulpanit 

This patch modifies the existing struct ivhd_header, which currently
only support IVHD type 0x10, to add new fields from IVHD type 11h and 40h.
It also modifies the pointer calculation to allow support for IVHD type
11h and 40h

Signed-off-by: Suravee Suthikulpanit 
---
 drivers/iommu/amd_iommu_init.c | 47 +++---
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..2ff7000 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -88,7 +88,7 @@
 
 /*
  * structure describing one IOMMU in the ACPI table. Typically followed by one
- * or more ivhd_entrys.
+ * or more ivhd_entrys. This struct supports both IVHD type 10h, 11h and 40h.
  */
 struct ivhd_header {
u8 type;
@@ -99,7 +99,11 @@ struct ivhd_header {
u64 mmio_phys;
u16 pci_seg;
u16 info;
-   u32 efr;
+   u32 efr_attr;
+
+   /* Following only valid on IVHD type 11h and 40h */
+   u64 efr_reg_img; /* Exact copy of MMIO_EXT_FEATURES */
+   u64 res;
 } __attribute__((packed));
 
 /*
@@ -399,6 +403,22 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu 
*iommu)
  *
  /
 
+static inline u32 get_ivhd_header_size(struct ivhd_header *h)
+{
+   u32 size = 0;
+
+   switch (h->type) {
+   case 0x10:
+   size = 24;
+   break;
+   case 0x11:
+   case 0x40:
+   size = 40;
+   break;
+   }
+   return size;
+}
+
 /*
  * This function calculates the length of a given IVHD entry
  */
@@ -415,8 +435,14 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
 {
u8 *p = (void *)h, *end = (void *)h;
struct ivhd_entry *dev;
+   u32 ivhd_size = get_ivhd_header_size(h);
+
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
 
-   p += sizeof(*h);
+   p += ivhd_size;
end += h->length;
 
while (p < end) {
@@ -781,6 +807,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
u32 dev_i, ext_flags = 0;
bool alias = false;
struct ivhd_entry *e;
+   u32 ivhd_size;
int ret;
 
 
@@ -796,7 +823,13 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
/*
 * Done. Now parse the device entries
 */
-   p += sizeof(struct ivhd_header);
+   ivhd_size = get_ivhd_header_size(h);
+   if (!ivhd_size) {
+   pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+   return -EINVAL;
+   }
+
+   p += ivhd_size;
end += h->length;
 
 
@@ -1047,9 +1080,9 @@ static int __init init_iommu_one(struct amd_iommu *iommu, 
struct ivhd_header *h)
iommu->mmio_phys = h->mmio_phys;
 
/* Check if IVHD EFR contains proper max banks/counters */
-   if ((h->efr != 0) &&
-   ((h->efr & (0xF << 13)) != 0) &&
-   ((h->efr & (0x3F << 17)) != 0)) {
+   if ((h->efr_attr != 0) &&
+   ((h->efr_attr & (0xF << 13)) != 0) &&
+   ((h->efr_attr & (0x3F << 17)) != 0)) {
iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
} else {
iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
-- 
1.9.1

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

[PATCH 2/6] iommu/amd: Use the most comprehensive IVHD type that the driver can support

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

The IVRS in more recent AMD system usually contains multiple
IVHD block types (e.g. 0x10, 0x11, and 0x40) for each IOMMU.
The newer IVHD types provide more information (e.g. new features
specified in the IOMMU spec), while maintain compatibility with
the older IVHD type.

Having multiple IVHD type allows older IOMMU drivers to still function
(e.g. using the older IVHD type 0x10) while the newer IOMMU driver can use
the newer IVHD types (e.g. 0x11 and 0x40). Therefore, the IOMMU driver
should only make use of the newest IVHD type that it can support.

This patch adds new logic to determine the highest level of IVHD type
it can support, and use it throughout the to initialize the driver.
This requires adding another pass to the IVRS parsing to determine
appropriate IVHD type (see function get_highest_supported_ivhd_type())
before parsing the contents.

[Vincent: fix the build error of IVHD_DEV_ACPI_HID flag not found]

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 107 ++---
 1 file changed, 78 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2ff7000..146f3ee 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -44,7 +44,7 @@
  */
 #define IVRS_HEADER_LENGTH 48
 
-#define ACPI_IVHD_TYPE  0x10
+#define ACPI_IVHD_TYPE_MAX_SUPPORTED   0x40
 #define ACPI_IVMD_TYPE_ALL  0x20
 #define ACPI_IVMD_TYPE  0x21
 #define ACPI_IVMD_TYPE_RANGE0x22
@@ -58,6 +58,7 @@
 #define IVHD_DEV_EXT_SELECT 0x46
 #define IVHD_DEV_EXT_SELECT_RANGE   0x47
 #define IVHD_DEV_SPECIAL   0x48
+#define IVHD_DEV_ACPI_HID  0xf0
 
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
@@ -137,6 +138,7 @@ bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
+static int amd_iommu_target_ivhd_type;
 
 u16 amd_iommu_last_bdf;/* largest PCI device id we have
   to handle */
@@ -424,7 +426,15 @@ static inline u32 get_ivhd_header_size(struct ivhd_header 
*h)
  */
 static inline int ivhd_entry_length(u8 *ivhd)
 {
-   return 0x04 << (*ivhd >> 6);
+   u32 type = ((struct ivhd_entry *)ivhd)->type;
+
+   if (type < 0x80) {
+   return 0x04 << (*ivhd >> 6);
+   } else if (type == IVHD_DEV_ACPI_HID) {
+   /* For ACPI_HID, offset 21 is uid len */
+   return *((u8 *)ivhd + 21) + 22;
+   }
+   return 0;
 }
 
 /*
@@ -470,6 +480,22 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
return 0;
 }
 
+static int __init check_ivrs_checksum(struct acpi_table_header *table)
+{
+   int i;
+   u8 checksum = 0, *p = (u8 *)table;
+
+   for (i = 0; i < table->length; ++i)
+   checksum += p[i];
+   if (checksum != 0) {
+   /* ACPI table corrupt */
+   pr_err(FW_BUG "AMD-Vi: IVRS invalid checksum\n");
+   return -ENODEV;
+   }
+
+   return 0;
+}
+
 /*
  * Iterate over all IVHD entries in the ACPI table and find the highest device
  * id which we need to handle. This is the first of three functions which parse
@@ -477,31 +503,19 @@ static int __init find_last_devid_from_ivhd(struct 
ivhd_header *h)
  */
 static int __init find_last_devid_acpi(struct acpi_table_header *table)
 {
-   int i;
-   u8 checksum = 0, *p = (u8 *)table, *end = (u8 *)table;
+   u8 *p = (u8 *)table, *end = (u8 *)table;
struct ivhd_header *h;
 
-   /*
-* Validate checksum here so we don't need to do it when
-* we actually parse the table
-*/
-   for (i = 0; i < table->length; ++i)
-   checksum += p[i];
-   if (checksum != 0)
-   /* ACPI table corrupt */
-   return -ENODEV;
-
p += IVRS_HEADER_LENGTH;
 
end += table->length;
while (p < end) {
h = (struct ivhd_header *)p;
-   switch (h->type) {
-   case ACPI_IVHD_TYPE:
-   find_last_devid_from_ivhd(h);
-   break;
-   default:
-   break;
+   if (h->type == amd_iommu_target_ivhd_type) {
+   int ret = find_last_devid_from_ivhd(h);
+
+   if (ret)
+   return ret;
}
p += h->length;
}
@@ -1118,6 +1132,32 @@ static int __init init_iommu_one(struct amd_iommu 
*iommu, struct ivhd_header *h)
return 0;
 }
 
+/**
+ * get_highest_supported

[PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch creates a new function for finding or creating an IOMMU
group for acpihid(ACPI Hardware ID) device.

The acpihid devices with the same devid will be put into same group and
there will have the same domain id and share the same page table.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1902d01..e1083ce 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -232,6 +232,29 @@ static struct iommu_dev_data *get_dev_data(struct device 
*dev)
return dev->archdata.iommu;
 }
 
+/*
+* Find or create an IOMMU group for a acpihid device.
+*/
+static struct iommu_group *acpihid_device_group(struct device *dev)
+{
+   struct acpihid_map_entry *p, *entry = NULL;
+   u16 devid;
+
+   devid = get_acpihid_device_id(dev, );
+   if (devid < 0)
+   return ERR_PTR(devid);
+
+   list_for_each_entry(p, _map, list) {
+   if ((devid == p->devid) && p->group)
+   entry->group = p->group;
+   }
+
+   if (!entry->group)
+   entry->group = generic_device_group(dev);
+
+   return entry->group;
+}
+
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
static const int caps[] = {
@@ -2311,6 +2334,14 @@ static void amd_iommu_remove_device(struct device *dev)
iommu_completion_wait(iommu);
 }
 
+static struct iommu_group *amd_iommu_device_group(struct device *dev)
+{
+   if (dev_is_pci(dev))
+   return pci_device_group(dev);
+
+   return acpihid_device_group(dev);
+}
+
 /*
  *
  * The next functions belong to the dma_ops mapping/unmapping code.
@@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
.iova_to_phys = amd_iommu_iova_to_phys,
.add_device = amd_iommu_add_device,
.remove_device = amd_iommu_remove_device,
-   .device_group = pci_device_group,
+   .device_group = amd_iommu_device_group,
.get_dm_regions = amd_iommu_get_dm_regions,
.put_dm_regions = amd_iommu_put_dm_regions,
.pgsize_bitmap  = AMD_IOMMU_PGSIZES,
-- 
1.9.1

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

[PATCH 3/6] iommu/amd: Add new map for storing IVHD dev entry type HID

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch introduces acpihid_map, which is used to store
the new IVHD device entry extracted from BIOS IVRS table.

It also provides a utility function add_acpi_hid_device(),
to add this types of devices to the map.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c   |   1 +
 drivers/iommu/amd_iommu_init.c  | 122 
 drivers/iommu/amd_iommu_types.h |  14 +
 3 files changed, 137 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9097b11..98acb89 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,7 @@ static DEFINE_SPINLOCK(dev_data_list_lock);
 
 LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
+LIST_HEAD(acpihid_map);
 
 /*
  * Domain for untranslated devices - only allocated
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 146f3ee..0a8e033 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -60,6 +60,10 @@
 #define IVHD_DEV_SPECIAL   0x48
 #define IVHD_DEV_ACPI_HID  0xf0
 
+#define UID_NOT_PRESENT 0
+#define UID_IS_INTEGER  1
+#define UID_IS_CHARACTER2
+
 #define IVHD_SPECIAL_IOAPIC1
 #define IVHD_SPECIAL_HPET  2
 
@@ -116,6 +120,11 @@ struct ivhd_entry {
u16 devid;
u8 flags;
u32 ext;
+   u32 hidh;
+   u64 cid;
+   u8 uidf;
+   u8 uidl;
+   u8 uid;
 } __attribute__((packed));
 
 /*
@@ -224,8 +233,12 @@ enum iommu_init_state {
 #define EARLY_MAP_SIZE 4
 static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static struct acpihid_map_entry __initdata early_acpihid_map[EARLY_MAP_SIZE];
+
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static int __initdata early_acpihid_map_size;
+
 static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
@@ -760,6 +773,42 @@ static int __init add_special_device(u8 type, u8 id, u16 
*devid, bool cmd_line)
return 0;
 }
 
+static int __init add_acpi_hid_device(u8 *hid, u8 *uid, u16 *devid,
+ bool cmd_line)
+{
+   struct acpihid_map_entry *entry;
+   struct list_head *list = _map;
+
+   list_for_each_entry(entry, list, list) {
+   if (strcmp(entry->hid, hid) ||
+   (*uid && *entry->uid && strcmp(entry->uid, uid)) ||
+   !entry->cmd_line)
+   continue;
+
+   pr_info("AMD-Vi: Command-line override for hid:%s uid:%s\n",
+   hid, uid);
+   *devid = entry->devid;
+   return 0;
+   }
+
+   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+   if (!entry)
+   return -ENOMEM;
+
+   memcpy(entry->uid, uid, strlen(uid));
+   memcpy(entry->hid, hid, strlen(hid));
+   entry->devid = *devid;
+   entry->cmd_line = cmd_line;
+   entry->root_devid = (entry->devid & (~0x7));
+
+   pr_info("AMD-Vi:%s, add hid:%s, uid:%s, rdevid:%d\n",
+   entry->cmd_line ? "cmd" : "ivrs",
+   entry->hid, entry->uid, entry->root_devid);
+
+   list_add_tail(>list, list);
+   return 0;
+}
+
 static int __init add_early_maps(void)
 {
int i, ret;
@@ -782,6 +831,15 @@ static int __init add_early_maps(void)
return ret;
}
 
+   for (i = 0; i < early_acpihid_map_size; ++i) {
+   ret = add_acpi_hid_device(early_acpihid_map[i].hid,
+ early_acpihid_map[i].uid,
+ _acpihid_map[i].devid,
+ early_acpihid_map[i].cmd_line);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -1001,6 +1059,70 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
 
break;
}
+   case IVHD_DEV_ACPI_HID: {
+   u16 devid;
+   u8 hid[ACPIHID_HID_LEN] = {0};
+   u8 uid[ACPIHID_UID_LEN] = {0};
+   int ret;
+
+   if (h->type != 0x40) {
+   pr_err(FW_BUG "Invalid IVHD device type %#x\n",
+  e->type);
+   break;
+   }
+
+   memcpy(hid, (u8 *)(>ext), ACPIHID_HID_LEN - 1);
+   hid[ACPIHID_HID_LEN - 1] = '

[PATCH 0/6] iommu/amd: enable ACPI hardware ID device support

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

This patch series enable ACPI hardware ID device support, 

There are some devices indentified using ACPI HID format in AMD chip.
This patch series enable iommu support for those ACPI HID device,
since the existing AMD iommu only supports PCI bus based device.

Suravee Suthikulpanit (3):
  iommu/amd: Modify ivhd_header structure to support type 11h and 40h
  iommu/amd: Use the most comprehensive IVHD type that the driver can
support
  iommu/amd: Introduces ivrs_acpihid kernel parameter

Wan Zongshun (3):
  iommu/amd: Add new map for storing IVHD dev entry type HID
  iommu/amd: Add support for non-pci devices
  iommu/amd: Manage iommu_group for non-pci devices

 Documentation/kernel-parameters.txt |   7 +
 drivers/iommu/amd_iommu.c   | 158 +++---
 drivers/iommu/amd_iommu_init.c  | 309 +++-
 drivers/iommu/amd_iommu_types.h |  14 ++
 4 files changed, 433 insertions(+), 55 deletions(-)

-- 
1.9.1

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

[PATCH 4/6] iommu/amd: Introduces ivrs_acpihid kernel parameter

[Wan Zongshun]

From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>

This patch introduces a new kernel parameter, ivrs_acpihid.
This is used to override existing ACPI-HID IVHD device entry,
or add an entry in case it is missing in the IVHD.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 Documentation/kernel-parameters.txt |  7 +++
 drivers/iommu/amd_iommu_init.c  | 33 +
 2 files changed, 40 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 742f69d..5c364c6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1680,6 +1680,13 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
PCI device 00:14.0 write the parameter as:
ivrs_hpet[0]=00:14.0
 
+   ivrs_acpihid[HW,X86_64]
+   Provide an override to the ACPI-HID:UID<->DEVICE-ID
+   mapping provided in the IVRS ACPI table. For
+   example, to map UART-HID:UID AMD0020:0 to
+   PCI device 00:14.5 write the parameter as:
+   ivrs_acpihid[00:14.5]=AMD0020:0
+
js= [HW,JOY] Analog joystick
See Documentation/input/joystick.txt.
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 0a8e033..ad05614 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2430,10 +2430,43 @@ static int __init parse_ivrs_hpet(char *str)
return 1;
 }
 
+static int __init parse_ivrs_acpihid(char *str)
+{
+   u32 bus, dev, fn;
+   char *hid, *uid, *p;
+   char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0};
+   int ret, i;
+
+   ret = sscanf(str, "[%x:%x.%x]=%s", , , , acpiid);
+   if (ret != 4) {
+   pr_err("AMD-Vi: Invalid command line: ivrs_acpihid(%s)\n", str);
+   return 1;
+   }
+
+   p = acpiid;
+   hid = strsep(, ":");
+   uid = p;
+
+   if (!hid || !(*hid) || !uid) {
+   pr_err("AMD-Vi: Invalid command line: hid or uid\n");
+   return 1;
+   }
+
+   i = early_acpihid_map_size++;
+   memcpy(early_acpihid_map[i].hid, hid, strlen(hid));
+   memcpy(early_acpihid_map[i].uid, uid, strlen(uid));
+   early_acpihid_map[i].devid =
+   ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+   early_acpihid_map[i].cmd_line   = true;
+
+   return 1;
+}
+
 __setup("amd_iommu_dump",  parse_amd_iommu_dump);
 __setup("amd_iommu=",  parse_amd_iommu_options);
 __setup("ivrs_ioapic", parse_ivrs_ioapic);
 __setup("ivrs_hpet",   parse_ivrs_hpet);
+__setup("ivrs_acpihid",parse_ivrs_acpihid);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
  gart_iommu_hole_init,
-- 
1.9.1

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

[PATCH 5/6] iommu/amd: Add support for non-pci devices

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

Current IOMMU driver make assumption that the downstream devices are PCI.
With the newly added ACPI-HID IVHD device entry support, this is no
longer true. This patch is to add dev type check and to distinguish the
pci and acpihid device code path.

Signed-off-by: Wan Zongshun <vincent@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
 drivers/iommu/amd_iommu.c | 124 +++---
 1 file changed, 106 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 98acb89..1902d01 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -176,13 +177,56 @@ static struct iommu_dev_data *find_dev_data(u16 devid)
return dev_data;
 }
 
-static inline u16 get_device_id(struct device *dev)
+static inline int match_hid_uid(struct device *dev,
+   struct acpihid_map_entry *entry)
+{
+   const char *hid, *uid;
+
+   hid = acpi_device_hid(ACPI_COMPANION(dev));
+   uid = acpi_device_uid(ACPI_COMPANION(dev));
+
+   if (!hid || !(*hid))
+   return -ENODEV;
+
+   if (!uid || !(*uid))
+   return strcmp(hid, entry->hid);
+
+   if (!(*entry->uid))
+   return strcmp(hid, entry->hid);
+
+   return -ENODEV;
+}
+
+static inline u16 get_pci_device_id(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
 
return PCI_DEVID(pdev->bus->number, pdev->devfn);
 }
 
+static inline int get_acpihid_device_id(struct device *dev,
+   struct acpihid_map_entry **entry)
+{
+   struct acpihid_map_entry *p;
+
+   list_for_each_entry(p, _map, list) {
+   if (!match_hid_uid(dev, p)) {
+   if (entry)
+   *entry = p;
+   return p->devid;
+   }
+   }
+   return -EINVAL;
+}
+
+static inline u16 get_device_id(struct device *dev)
+{
+   if (dev_is_pci(dev))
+   return get_pci_device_id(dev);
+   else
+   return get_acpihid_device_id(dev, NULL);
+}
+
 static struct iommu_dev_data *get_dev_data(struct device *dev)
 {
return dev->archdata.iommu;
@@ -262,7 +306,7 @@ static bool check_device(struct device *dev)
return false;
 
/* No PCI device */
-   if (!dev_is_pci(dev))
+   if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
return false;
 
devid = get_device_id(dev);
@@ -298,7 +342,7 @@ out:
iommu_group_put(group);
 }
 
-static int iommu_init_device(struct device *dev)
+static int iommu_init_pci_device(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
@@ -325,6 +369,32 @@ static int iommu_init_device(struct device *dev)
return 0;
 }
 
+static int iommu_init_acpihid_device(struct device *dev)
+{
+   struct iommu_dev_data *dev_data;
+
+   if (dev->archdata.iommu)
+   return 0;
+
+   dev_data = find_dev_data(get_device_id(dev));
+   if (!dev_data)
+   return -ENOMEM;
+
+   dev->archdata.iommu = dev_data;
+
+   iommu_device_link(amd_iommu_rlookup_table[dev_data->devid]->iommu_dev,
+ dev);
+
+   return 0;
+}
+
+static int iommu_init_device(struct device *dev)
+{
+   if (dev_is_pci(dev))
+   return iommu_init_pci_device(dev);
+   return iommu_init_acpihid_device(dev);
+}
+
 static void iommu_ignore_device(struct device *dev)
 {
u16 devid, alias;
@@ -2066,19 +2136,16 @@ static bool pci_pri_tlp_required(struct pci_dev *pdev)
return (status & PCI_PRI_TLP_OFF) ? true : false;
 }
 
-/*
- * If a device is not yet associated with a domain, this function
- * assigns it visible for the hardware
- */
-static int attach_device(struct device *dev,
-struct protection_domain *domain)
+static int setup_ats_device(struct device *dev,
+   struct protection_domain *domain)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
-   struct iommu_dev_data *dev_data;
-   unsigned long flags;
-   int ret;
+   struct pci_dev *pdev;
+   struct iommu_dev_data *dev_data = get_dev_data(dev);
 
-   dev_data = get_dev_data(dev);
+   if (!dev_is_pci(dev))
+   return 0;
+
+   pdev = to_pci_dev(dev);
 
if (domain->flags & PD_IOMMUV2_MASK) {
if (!dev_data->passthrough)
@@ -2098,6 +2165,25 @@ static int attach_device(struct device *dev,
dev_data->ats.qdep= pci_ats_queue_depth(pdev);
}
 
+   return 0;
+}
+
+/*
+ * If a device is not yet associated with a domain, th

[PATCH] iommu/amd: set AMD iommu-callbacks for the amba bus

[Wan Zongshun]

From: Wan Zongshun <vincent@amd.com>

Since uart dma is using AMD iommu, and it bases on amba bus.
So we need set callbacks for amba bus type firstly.

Signed-off-by: Wan Zongshun <vincent@amd.com>
---
 drivers/iommu/amd_iommu.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8b2be1e..9097b11 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2758,7 +2759,17 @@ static struct dma_map_ops amd_iommu_dma_ops = {
 
 int __init amd_iommu_init_api(void)
 {
-   return bus_set_iommu(_bus_type, _iommu_ops);
+   int err = 0;
+
+   err = bus_set_iommu(_bus_type, _iommu_ops);
+   if (err)
+   return err;
+#ifdef CONFIG_ARM_AMBA
+   err = bus_set_iommu(_bustype, _iommu_ops);
+   if (err)
+   return err;
+#endif
+   return 0;
 }
 
 int __init amd_iommu_init_dma_ops(void)
-- 
1.9.1

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

Re: [PATCH] iommu:Check that iommu_device_create has completed successfully in alloc_iommu

[Wan ZongShun]

2015-12-29 13:10 GMT+08:00 Nicholas Krause <xerofo...@gmail.com>:
> This adds the proper check to alloc_iommu to make sure that the call
> to iommu_device_create has completed successfully and if not return
> to the caller the error code returned after freeing up resources
> allocated previously by alloc_iommu.
>
> Signed-off-by: Nicholas Krause <xerofo...@gmail.com>
> ---
>  drivers/iommu/dmar.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 80e3c17..27333b6 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1069,9 +1069,12 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
> iommu->iommu_dev = iommu_device_create(NULL, iommu,
>intel_iommu_groups,
>"%s", iommu->name);
> +   if (IS_ERR(iommu->iommu_dev)) {
> +   err = PTR_ERR(iommu->iommu_dev);
> +   goto err_unmap;
> +   }

If so, will this bad 'iommu->iommu_dev' break your iommu work?  It
seems not necessary.

>
> return 0;
> -
>  err_unmap:
> unmap_iommu(iommu);
>  error_free_seq_id:
> --
> 2.5.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu