<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em sex., 9 de abr. de 2021 01:36, Alexey Kardashevskiy <<a href="mailto:aik@ozlabs.ru">aik@ozlabs.ru</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
On 08/04/2021 19:04, Michael Ellerman wrote:<br>
> Alexey Kardashevskiy <<a href="mailto:aik@ozlabs.ru" target="_blank" rel="noreferrer">aik@ozlabs.ru</a>> writes:<br>
>> On 08/04/2021 15:37, Michael Ellerman wrote:<br>
>>> Leonardo Bras <<a href="mailto:leobras.c@gmail.com" target="_blank" rel="noreferrer">leobras.c@gmail.com</a>> writes:<br>
>>>> According to LoPAR, ibm,query-pe-dma-window output named "IO Page Sizes"<br>
>>>> will let the OS know all possible pagesizes that can be used for creating a<br>
>>>> new DDW.<br>
>>>><br>
>>>> Currently Linux will only try using 3 of the 8 available options:<br>
>>>> 4K, 64K and 16M. According to LoPAR, Hypervisor may also offer 32M, 64M,<br>
>>>> 128M, 256M and 16G.<br>
>>><br>
>>> Do we know of any hardware & hypervisor combination that will actually<br>
>>> give us bigger pages?<br>
>><br>
>><br>
>> On P8 16MB host pages and 16MB hardware iommu pages worked.<br>
>><br>
>> On P9, VM's 16MB IOMMU pages worked on top of 2MB host pages + 2MB<br>
>> hardware IOMMU pages.<br>
> <br>
> The current code already tries 16MB though.<br>
> <br>
> I'm wondering if we're going to ask for larger sizes that have never<br>
> been tested and possibly expose bugs. But it sounds like this is mainly<br>
> targeted at future platforms.<br>
<br>
<br>
I tried for fun to pass through a PCI device to a guest with this patch as:<br>
<br>
pbuild/qemu-killslof-aiku1904le-ppc64/qemu-system-ppc64 \<br>
-nodefaults \<br>
-chardev stdio,id=STDIO0,signal=off,mux=on \<br>
-device spapr-vty,id=svty0,reg=0x71000110,chardev=STDIO0 \<br>
-mon id=MON0,chardev=STDIO0,mode=readline \<br>
-nographic \<br>
-vga none \<br>
-enable-kvm \<br>
-m 16G \<br>
-kernel ./vmldbg \<br>
-initrd /home/aik/t/le.cpio \<br>
-device vfio-pci,id=vfio0001_01_00_0,host=0001:01:00.0 \<br>
-mem-prealloc \<br>
-mem-path qemu_hp_1G_node0 \<br>
-global spapr-pci-host-bridge.pgsz=0xffffff000 \<br>
-machine cap-cfpc=broken,cap-ccf-assist=off \<br>
-smp 1,threads=1 \<br>
-L /home/aik/t/qemu-ppc64-bios/ \<br>
-trace events=qemu_trace_events \<br>
-d guest_errors,mmu \<br>
-chardev socket,id=SOCKET0,server=on,wait=off,path=qemu.mon.1_1_0_0 \<br>
-mon chardev=SOCKET0,mode=control<br>
<br>
<br>
The guest created a huge window:<br>
<br>
xhci_hcd 0000:00:00.0: ibm,create-pe-dma-window(2027) 0 8000000 20000000 <br>
22 22 returned 0 (liobn = 0x80000001 starting addr = 8000000 0)<br>
<br>
The first "22" is page_shift in hex (16GB), the second "22" is <br>
window_shift (so we have 1 TCE).<br>
<br>
On the host side the window#1 was created with 1GB pages:<br>
pci 0001:01     : [PE# fd] Setting up window#1 <br>
800000000000000..80007ffffffffff pg=40000000<br>
<br>
<br>
The XHCI seems working. Without the patch 16MB was the maximum.<br>
<br>
<br>
> <br>
>>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c<br>
>>>> index 9fc5217f0c8e..6cda1c92597d 100644<br>
>>>> --- a/arch/powerpc/platforms/pseries/iommu.c<br>
>>>> +++ b/arch/powerpc/platforms/pseries/iommu.c<br>
>>>> @@ -53,6 +53,20 @@ enum {<br>
>>>>            DDW_EXT_QUERY_OUT_SIZE = 2<br>
>>>>    };<br>
>>><br>
>>> A comment saying where the values come from would be good.<br>
>>><br>
>>>> +#define QUERY_DDW_PGSIZE_4K       0x01<br>
>>>> +#define QUERY_DDW_PGSIZE_64K      0x02<br>
>>>> +#define QUERY_DDW_PGSIZE_16M      0x04<br>
>>>> +#define QUERY_DDW_PGSIZE_32M      0x08<br>
>>>> +#define QUERY_DDW_PGSIZE_64M      0x10<br>
>>>> +#define QUERY_DDW_PGSIZE_128M     0x20<br>
>>>> +#define QUERY_DDW_PGSIZE_256M     0x40<br>
>>>> +#define QUERY_DDW_PGSIZE_16G      0x80<br>
>>><br>
>>> I'm not sure the #defines really gain us much vs just putting the<br>
>>> literal values in the array below?<br>
>><br>
>> Then someone says "uuuuu magic values" :) I do not mind either way. Thanks,<br>
> <br>
> Yeah that's true. But #defining them doesn't make them less magic, if<br>
> you only use them in one place :)<br>
<br>
Defining them with "QUERY_DDW" in the names kinda tells where they are <br>
from. Can also grep QEMU using these to see how the other side handles <br>
it. Dunno.<br>
<br>
btw the bot complained about __builtin_ctz(SZ_16G) which should be <br>
__builtin_ctzl(SZ_16G) so we have to ask Leonardo to repost anyway :)<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Thanks for testing!</div><div dir="auto"><br></div><div dir="auto"><a href="http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210408201915.174217-1-leobras.c@gmail.com/">http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210408201915.174217-1-leobras.c@gmail.com/</a><br></div><div dir="auto"><br></div><div dir="auto">I sent a v3 a few hours ago, fixing this by using <span style="font-family:sans-serif">__builtin_ctzll() instead of </span><span style="font-family:sans-serif">__builtin_ctz() in all sizes, and it worked like a charm.</span></div><div dir="auto"><span style="font-family:sans-serif"><br></span></div><div dir="auto"><span style="font-family:sans-serif">I also reverted to the previous approach of not having QUERY_DDW defines for masks, as Michael suggested. </span></div><div dir="auto"><span style="font-family:sans-serif"><br></span></div><div dir="auto"><span style="font-family:sans-serif">I can revert back to v2 approach if you guys decide it's better.</span></div><div dir="auto"><span style="font-family:sans-serif"><br></span></div><div dir="auto"><span style="font-family:sans-serif">Best regards,</span></div><div dir="auto"><span style="font-family:sans-serif">Leonardo Bras</span></div></div>