Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On Tue, Jan 15, 2019 at 04:04:40PM +0800, Chao Gao wrote: [...] > (XEN) Xen version 4.12-unstable (root@) (gcc (Ubuntu 7.3.0-27ubuntu1~18.04) > 7.3.0) debug=y Tue Jan 15 07:25:29 UTC 2019 > (XEN) Latest ChangeSet: Mon Dec 17 09:22:59 2018 + git:a5b0eb3636 [...] > (XEN) *** Building a PVH Dom0 *** > (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 > (XEN) [ Xen-4.12-unstable x86_64 debug=y Tainted: C ] > (XEN) CPU:0 > (XEN) RIP:e008:[] iommu_map+0xba/0x176 > (XEN) RFLAGS: 00010202 CONTEXT: hypervisor > (XEN) rax: rbx: 0007 rcx: 0003 > (XEN) rdx: 0020f081 rsi: 0001 rdi: 830215242000 > (XEN) rbp: 82d080497bb8 rsp: 82d080497b58 r8: > (XEN) r9: 82d080497bd4 r10: 0180 r11: 7fff > (XEN) r12: 830215242000 r13: r14: 0001 > (XEN) r15: 0001 cr0: 8005003b cr4: 001526e0 > (XEN) cr3: dbc8d000 cr2: > (XEN) fsb: gsb: gss: > (XEN) ds: es: fs: gs: ss: cs: e008 > (XEN) Xen code around (iommu_map+0xba/0x176): > (XEN) 41 89 c5 e9 a2 00 00 00 <0f> 0b 0f 0b 41 89 c5 41 80 bc 24 c0 01 00 00 > 00 > (XEN) Xen stack trace from rsp=82d080497b58: > (XEN)0020 82d080497ba8 82d08023d489 > (XEN)0001 82e0041e1000 830215242000 82e0041e1020 > (XEN)830215242000 0001 0001 > (XEN)82d080497c08 82d0804182d8 82d080497c08 0001 > (XEN)009d 0001 82d080444c68 > (XEN)0020b43e 830215242000 82d080497d58 82d08043716c > (XEN)82d080497fff 0001 82d08046cbc0 8309bf40 > (XEN)01e33000 8309bf30 830213525000 0010b43e > (XEN)82db 01f4 0010 001f15242000 > (XEN)00ff82d080497cb8 82d08020a8e4 82d0805cf02c 82d08048f740 > (XEN)0092 82d08023dddb 82d080497fff 82d08048f740 > (XEN)82d080497cc8 82d08023de2e 82d080497ce8 82d08024016b > (XEN)82d080497ce8 82d08023de78 82d080497d08 82d080240205 > (XEN)82d0805a3880 82d0805a3880 82d080497d48 82d08023d489 > (XEN)8302152e1550 8309bf30 01e33000 8309bf30 > (XEN)01e33000 8309bf40 82d08046cbc0 830215242000 > (XEN)82d080497d98 82d08043e53c 82d080497d98 82d08046cbc0 > (XEN)8302152e1550 0001 82d0805d00d0 0008 > (XEN)82d080497ee8 82d08042d8ef 0002 > (XEN)0002 0002 0002 0002 > (XEN) Xen call trace: > (XEN)[] iommu_map+0xba/0x176 > (XEN)[] iommu_hwdom_init+0xef/0x220 > (XEN)[] dom0_construct_pvh+0x189/0x129e > (XEN)[] construct_dom0+0xd4/0xb14 > (XEN)[] __start_xen+0x2710/0x2830 > (XEN)[] __high_start+0x53/0x55 > (XEN) > (XEN) > (XEN) > (XEN) Panic on CPU 0: > (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 > (XEN) Oh, this was added by Paul quite recently. You seem to be using a rather old commit (a5b0eb3636), is there any reason for using such an old baseline? In order to fix this you likely need to update your base changeset to something newer that contains: commit ae7fc10d2ca5c22e04b8a28becbd1fbf8b44e83a Author: Roger Pau Monne Date: Fri Dec 28 12:18:56 2018 +0100 x86/dom0: take alignment into account when populating p2m in PVH mode Current code that allocates memory and populates the p2m for PVH Dom0 doesn't take the address alignment into account, this can lead to high order allocations that start on a non-aligned address to be broken down into lower order entries on the p2m page tables. Fix this by taking into account the p2m page sizes and alignment requirements when allocating the memory and populating the p2m. AFAICT the above commit should fix the issue, just updating to current staging branch should be enough. Paul, I wonder however whether the ASSERT is expected here, this interface used to work correctly without the alignment requirements, and 725bf00a8 changed the requirements of the function, which might make some previously valid calls now trigger the assert. FTR I agree we should use aligned regions, but the addition of the assert can be dangerous given it changes the valid inputs of the function and also affects the p2m functions that ultimately call the iommu code. Thanks, Roger. _
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On Tue, Jan 15, 2019 at 09:18:25AM +0100, Roger Pau Monné wrote: >On Tue, Jan 15, 2019 at 04:04:40PM +0800, Chao Gao wrote: >[...] >> (XEN) Xen version 4.12-unstable (root@) (gcc (Ubuntu 7.3.0-27ubuntu1~18.04) >> 7.3.0) debug=y Tue Jan 15 07:25:29 UTC 2019 >> (XEN) Latest ChangeSet: Mon Dec 17 09:22:59 2018 + git:a5b0eb3636 >[...] >> (XEN) *** Building a PVH Dom0 *** >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at >> iommu.c:323 >> (XEN) [ Xen-4.12-unstable x86_64 debug=y Tainted: C ] >> (XEN) CPU:0 >> (XEN) RIP:e008:[] iommu_map+0xba/0x176 >> (XEN) RFLAGS: 00010202 CONTEXT: hypervisor >> (XEN) rax: rbx: 0007 rcx: 0003 >> (XEN) rdx: 0020f081 rsi: 0001 rdi: 830215242000 >> (XEN) rbp: 82d080497bb8 rsp: 82d080497b58 r8: >> (XEN) r9: 82d080497bd4 r10: 0180 r11: 7fff >> (XEN) r12: 830215242000 r13: r14: 0001 >> (XEN) r15: 0001 cr0: 8005003b cr4: 001526e0 >> (XEN) cr3: dbc8d000 cr2: >> (XEN) fsb: gsb: gss: >> (XEN) ds: es: fs: gs: ss: cs: e008 >> (XEN) Xen code around (iommu_map+0xba/0x176): >> (XEN) 41 89 c5 e9 a2 00 00 00 <0f> 0b 0f 0b 41 89 c5 41 80 bc 24 c0 01 00 >> 00 00 >> (XEN) Xen stack trace from rsp=82d080497b58: >> (XEN)0020 82d080497ba8 82d08023d489 >> (XEN)0001 82e0041e1000 830215242000 82e0041e1020 >> (XEN)830215242000 0001 0001 >> (XEN)82d080497c08 82d0804182d8 82d080497c08 0001 >> (XEN)009d 0001 82d080444c68 >> (XEN)0020b43e 830215242000 82d080497d58 82d08043716c >> (XEN)82d080497fff 0001 82d08046cbc0 8309bf40 >> (XEN)01e33000 8309bf30 830213525000 0010b43e >> (XEN)82db 01f4 0010 001f15242000 >> (XEN)00ff82d080497cb8 82d08020a8e4 82d0805cf02c 82d08048f740 >> (XEN)0092 82d08023dddb 82d080497fff 82d08048f740 >> (XEN)82d080497cc8 82d08023de2e 82d080497ce8 82d08024016b >> (XEN)82d080497ce8 82d08023de78 82d080497d08 82d080240205 >> (XEN)82d0805a3880 82d0805a3880 82d080497d48 82d08023d489 >> (XEN)8302152e1550 8309bf30 01e33000 8309bf30 >> (XEN)01e33000 8309bf40 82d08046cbc0 830215242000 >> (XEN)82d080497d98 82d08043e53c 82d080497d98 82d08046cbc0 >> (XEN)8302152e1550 0001 82d0805d00d0 0008 >> (XEN)82d080497ee8 82d08042d8ef 0002 >> (XEN)0002 0002 0002 0002 >> (XEN) Xen call trace: >> (XEN)[] iommu_map+0xba/0x176 >> (XEN)[] iommu_hwdom_init+0xef/0x220 >> (XEN)[] dom0_construct_pvh+0x189/0x129e >> (XEN)[] construct_dom0+0xd4/0xb14 >> (XEN)[] __start_xen+0x2710/0x2830 >> (XEN)[] __high_start+0x53/0x55 >> (XEN) >> (XEN) >> (XEN) >> (XEN) Panic on CPU 0: >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at >> iommu.c:323 >> (XEN) > >Oh, this was added by Paul quite recently. You seem to be using a >rather old commit (a5b0eb3636), is there any reason for using such an >old baseline? I was using the master branch. Your patch below did fix this issue. Thanks Chao ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
> -Original Message- > From: Roger Pau Monne > Sent: 15 January 2019 08:18 > To: Chao Gao > Cc: xen-devel@lists.xenproject.org; Paul Durrant > Subject: Re: [Xen-devel] an assertion triggered when running Xen on a HSW > desktop > > On Tue, Jan 15, 2019 at 04:04:40PM +0800, Chao Gao wrote: > [...] > > (XEN) Xen version 4.12-unstable (root@) (gcc (Ubuntu 7.3.0- > 27ubuntu1~18.04) 7.3.0) debug=y Tue Jan 15 07:25:29 UTC 2019 > > (XEN) Latest ChangeSet: Mon Dec 17 09:22:59 2018 + git:a5b0eb3636 > [...] > > (XEN) *** Building a PVH Dom0 *** > > (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 > > (XEN) [ Xen-4.12-unstable x86_64 debug=y Tainted: C ] > > (XEN) CPU:0 > > (XEN) RIP:e008:[] iommu_map+0xba/0x176 > > (XEN) RFLAGS: 00010202 CONTEXT: hypervisor > > (XEN) rax: rbx: 0007 rcx: > 0003 > > (XEN) rdx: 0020f081 rsi: 0001 rdi: > 830215242000 > > (XEN) rbp: 82d080497bb8 rsp: 82d080497b58 r8: > > > (XEN) r9: 82d080497bd4 r10: 0180 r11: > 7fff > > (XEN) r12: 830215242000 r13: r14: > 0001 > > (XEN) r15: 0001 cr0: 8005003b cr4: > 001526e0 > > (XEN) cr3: dbc8d000 cr2: > > (XEN) fsb: gsb: gss: > > > (XEN) ds: es: fs: gs: ss: cs: e008 > > (XEN) Xen code around (iommu_map+0xba/0x176): > > (XEN) 41 89 c5 e9 a2 00 00 00 <0f> 0b 0f 0b 41 89 c5 41 80 bc 24 c0 01 > 00 00 00 > > (XEN) Xen stack trace from rsp=82d080497b58: > > (XEN)0020 82d080497ba8 > 82d08023d489 > > (XEN)0001 82e0041e1000 830215242000 > 82e0041e1020 > > (XEN)830215242000 0001 > 0001 > > (XEN)82d080497c08 82d0804182d8 82d080497c08 > 0001 > > (XEN)009d 0001 > 82d080444c68 > > (XEN)0020b43e 830215242000 82d080497d58 > 82d08043716c > > (XEN)82d080497fff 0001 82d08046cbc0 > 8309bf40 > > (XEN)01e33000 8309bf30 830213525000 > 0010b43e > > (XEN)82db 01f4 0010 > 001f15242000 > > (XEN)00ff82d080497cb8 82d08020a8e4 82d0805cf02c > 82d08048f740 > > (XEN)0092 82d08023dddb 82d080497fff > 82d08048f740 > > (XEN)82d080497cc8 82d08023de2e 82d080497ce8 > 82d08024016b > > (XEN)82d080497ce8 82d08023de78 82d080497d08 > 82d080240205 > > (XEN)82d0805a3880 82d0805a3880 82d080497d48 > 82d08023d489 > > (XEN)8302152e1550 8309bf30 01e33000 > 8309bf30 > > (XEN)01e33000 8309bf40 82d08046cbc0 > 830215242000 > > (XEN)82d080497d98 82d08043e53c 82d080497d98 > 82d08046cbc0 > > (XEN)8302152e1550 0001 82d0805d00d0 > 0008 > > (XEN)82d080497ee8 82d08042d8ef > 0002 > > (XEN)0002 0002 0002 > 0002 > > (XEN) Xen call trace: > > (XEN)[] iommu_map+0xba/0x176 > > (XEN)[] iommu_hwdom_init+0xef/0x220 > > (XEN)[] dom0_construct_pvh+0x189/0x129e > > (XEN)[] construct_dom0+0xd4/0xb14 > > (XEN)[] __start_xen+0x2710/0x2830 > > (XEN)[] __high_start+0x53/0x55 > > (XEN) > > (XEN) > > (XEN) > > (XEN) Panic on CPU 0: > > (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 > > (XEN) > > Oh, this was added by Paul quite recently. You seem to be using a > rather old commit (a5b0eb3636), is there any reason for using such an > old baseline? > > In order to fix this you likely need to update your base changeset to > something newer that contains: > > commit ae7fc10d2ca5c22e04b8a28becbd1fbf8b44e83a > Author: Roger Pau Monne > Date: Fri Dec 28 12:18:56 2018 +0100 > > x86/dom0: take alignment into account when populating p2m in PVH mode > > Current code that allocates memory and populates the p2m for PVH Dom0 >
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
> -Original Message- [snip] > >> (XEN) Xen call trace: > >> (XEN)[] iommu_map+0xba/0x176 > >> (XEN)[] iommu_hwdom_init+0xef/0x220 > >> (XEN)[] dom0_construct_pvh+0x189/0x129e > >> (XEN)[] construct_dom0+0xd4/0xb14 > >> (XEN)[] __start_xen+0x2710/0x2830 > >> (XEN)[] __high_start+0x53/0x55 > >> (XEN) > >> (XEN) > >> (XEN) > >> (XEN) Panic on CPU 0: > >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 > >> (XEN) > > > >Oh, this was added by Paul quite recently. You seem to be using a > >rather old commit (a5b0eb3636), is there any reason for using such an > >old baseline? > > I was using the master branch. Your patch below did fix this issue. > Given this failure and the fact that valid orders differ between different architectures, I propose we change the argument to the iommu_map/unmap wrapper functions from an order to a count, thus making it clear that there is no alignment restriction. Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
>>> On 15.01.19 at 10:44, wrote: >> -Original Message- > [snip] >> >> (XEN) Xen call trace: >> >> (XEN)[] iommu_map+0xba/0x176 >> >> (XEN)[] iommu_hwdom_init+0xef/0x220 >> >> (XEN)[] dom0_construct_pvh+0x189/0x129e >> >> (XEN)[] construct_dom0+0xd4/0xb14 >> >> (XEN)[] __start_xen+0x2710/0x2830 >> >> (XEN)[] __high_start+0x53/0x55 >> >> (XEN) >> >> (XEN) >> >> (XEN) >> >> (XEN) Panic on CPU 0: >> >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at >> iommu.c:323 >> >> (XEN) >> > >> >Oh, this was added by Paul quite recently. You seem to be using a >> >rather old commit (a5b0eb3636), is there any reason for using such an >> >old baseline? >> >> I was using the master branch. Your patch below did fix this issue. > > Given this failure and the fact that valid orders differ between different > architectures, I propose we change the argument to the iommu_map/unmap > wrapper functions from an order to a count, thus making it clear that there > is no alignment restriction. But the whole idea is for there to be an alignment restriction, such that it is easy to determine whether large page mappings can be used to satisfy the request. What's the exact case where a caller absolutely has to pass in a mis-aligned (dfn,size) tuple? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: > >>> On 15.01.19 at 10:44, wrote: > >> -Original Message- > > [snip] > >> >> (XEN) Xen call trace: > >> >> (XEN)[] iommu_map+0xba/0x176 > >> >> (XEN)[] iommu_hwdom_init+0xef/0x220 > >> >> (XEN)[] dom0_construct_pvh+0x189/0x129e > >> >> (XEN)[] construct_dom0+0xd4/0xb14 > >> >> (XEN)[] __start_xen+0x2710/0x2830 > >> >> (XEN)[] __high_start+0x53/0x55 > >> >> (XEN) > >> >> (XEN) > >> >> (XEN) > >> >> (XEN) Panic on CPU 0: > >> >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > >> iommu.c:323 > >> >> (XEN) > >> > > >> >Oh, this was added by Paul quite recently. You seem to be using a > >> >rather old commit (a5b0eb3636), is there any reason for using such an > >> >old baseline? > >> > >> I was using the master branch. Your patch below did fix this issue. > > > > Given this failure and the fact that valid orders differ between different > > architectures, I propose we change the argument to the iommu_map/unmap > > wrapper functions from an order to a count, thus making it clear that there > > is no alignment restriction. > > But the whole idea is for there to be an alignment restriction, such > that it is easy to determine whether large page mappings can be > used to satisfy the request. What's the exact case where a caller > absolutely has to pass in a mis-aligned (dfn,size) tuple? Taking PVH Dom0 builder as an example, it's possible to have a RAM region that starts on a 4K only aligned address. The natural operation in that case would be to try to allocate a memory region as big as possible up to the next 2MB boundary. Hence it would be valid to attempt to populate this 4K only aligned address using an order > 0 and < 9 (2MB order). The alternative here if the asserts are not removed would be to open-code a loop in the caller that iterates creating a bunch of order 0 mappings up to the 2MB boundary. The overhead in that case would be quite big, so I don't think we want to go down that route (also we would end up with a bunch of loops in the callers). Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On 15/01/2019 10:27, Roger Pau Monné wrote: > On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: > On 15.01.19 at 10:44, wrote: -Original Message- >>> [snip] >> (XEN) Xen call trace: >> (XEN)[] iommu_map+0xba/0x176 >> (XEN)[] iommu_hwdom_init+0xef/0x220 >> (XEN)[] dom0_construct_pvh+0x189/0x129e >> (XEN)[] construct_dom0+0xd4/0xb14 >> (XEN)[] __start_xen+0x2710/0x2830 >> (XEN)[] __high_start+0x53/0x55 >> (XEN) >> (XEN) >> (XEN) >> (XEN) Panic on CPU 0: >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:323 >> (XEN) > Oh, this was added by Paul quite recently. You seem to be using a > rather old commit (a5b0eb3636), is there any reason for using such an > old baseline? I was using the master branch. Your patch below did fix this issue. >>> Given this failure and the fact that valid orders differ between different >>> architectures, I propose we change the argument to the iommu_map/unmap >>> wrapper functions from an order to a count, thus making it clear that there >>> is no alignment restriction. >> But the whole idea is for there to be an alignment restriction, such >> that it is easy to determine whether large page mappings can be >> used to satisfy the request. What's the exact case where a caller >> absolutely has to pass in a mis-aligned (dfn,size) tuple? > Taking PVH Dom0 builder as an example, it's possible to have a RAM > region that starts on a 4K only aligned address. The natural operation > in that case would be to try to allocate a memory region as big as > possible up to the next 2MB boundary. Hence it would be valid to > attempt to populate this 4K only aligned address using an order > 0 > and < 9 (2MB order). The alternative here if the asserts are not > removed would be to open-code a loop in the caller that iterates > creating a bunch of order 0 mappings up to the 2MB boundary. The > overhead in that case would be quite big, so I don't think we want to > go down that route (also we would end up with a bunch of loops in the > callers). Given the PVH Dom0 building issues which Roger and I worked on over the Christmas period, there is a human-noticeable difference in construction time when the caller is doing a loop over order 0 pages, vs an order 8 allocation, and that was for a total of 4Mb of RAM. A dfn/count interface is actually more flexible than a dfn/order, because it doesn't require the caller to know the superpage orders of the underlying implementation to create efficient mappings. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
>>> On 15.01.19 at 11:27, wrote: > On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: >> >>> On 15.01.19 at 10:44, wrote: >> >> -Original Message- >> > [snip] >> >> >> (XEN) Xen call trace: >> >> >> (XEN)[] iommu_map+0xba/0x176 >> >> >> (XEN)[] iommu_hwdom_init+0xef/0x220 >> >> >> (XEN)[] dom0_construct_pvh+0x189/0x129e >> >> >> (XEN)[] construct_dom0+0xd4/0xb14 >> >> >> (XEN)[] __start_xen+0x2710/0x2830 >> >> >> (XEN)[] __high_start+0x53/0x55 >> >> >> (XEN) >> >> >> (XEN) >> >> >> (XEN) >> >> >> (XEN) Panic on CPU 0: >> >> >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at >> >> iommu.c:323 >> >> >> (XEN) >> >> > >> >> >Oh, this was added by Paul quite recently. You seem to be using a >> >> >rather old commit (a5b0eb3636), is there any reason for using such an >> >> >old baseline? >> >> >> >> I was using the master branch. Your patch below did fix this issue. >> > >> > Given this failure and the fact that valid orders differ between different >> > architectures, I propose we change the argument to the iommu_map/unmap >> > wrapper functions from an order to a count, thus making it clear that >> > there > >> > is no alignment restriction. >> >> But the whole idea is for there to be an alignment restriction, such >> that it is easy to determine whether large page mappings can be >> used to satisfy the request. What's the exact case where a caller >> absolutely has to pass in a mis-aligned (dfn,size) tuple? > > Taking PVH Dom0 builder as an example, it's possible to have a RAM > region that starts on a 4K only aligned address. The natural operation > in that case would be to try to allocate a memory region as big as > possible up to the next 2MB boundary. Hence it would be valid to > attempt to populate this 4K only aligned address using an order > 0 > and < 9 (2MB order). The alternative here if the asserts are not > removed would be to open-code a loop in the caller that iterates > creating a bunch of order 0 mappings up to the 2MB boundary. The > overhead in that case would be quite big, so I don't think we want to > go down that route (also we would end up with a bunch of loops in the > callers). I'm afraid I'm now more confused than before: If there's a RAM region aligned to no better than 4k, how can this possibly be populated with an order-greater-than-zero allocation? And even if I re-phrased your reply to mean an arbitrary alignment / order less than 9, then populating this with such a smaller order is still fine, and requesting the IOMMU mapping with that smaller order is still not going to trip the ASSERT() in question. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
>>> On 15.01.19 at 11:42, wrote: > On 15/01/2019 10:27, Roger Pau Monné wrote: >> On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: >> On 15.01.19 at 10:44, wrote: > -Original Message- [snip] >>> (XEN) Xen call trace: >>> (XEN)[] iommu_map+0xba/0x176 >>> (XEN)[] iommu_hwdom_init+0xef/0x220 >>> (XEN)[] dom0_construct_pvh+0x189/0x129e >>> (XEN)[] construct_dom0+0xd4/0xb14 >>> (XEN)[] __start_xen+0x2710/0x2830 >>> (XEN)[] __high_start+0x53/0x55 >>> (XEN) >>> (XEN) >>> (XEN) >>> (XEN) Panic on CPU 0: >>> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at > iommu.c:323 >>> (XEN) >> Oh, this was added by Paul quite recently. You seem to be using a >> rather old commit (a5b0eb3636), is there any reason for using such an >> old baseline? > I was using the master branch. Your patch below did fix this issue. Given this failure and the fact that valid orders differ between different architectures, I propose we change the argument to the iommu_map/unmap wrapper functions from an order to a count, thus making it clear that there is no alignment restriction. >>> But the whole idea is for there to be an alignment restriction, such >>> that it is easy to determine whether large page mappings can be >>> used to satisfy the request. What's the exact case where a caller >>> absolutely has to pass in a mis-aligned (dfn,size) tuple? >> Taking PVH Dom0 builder as an example, it's possible to have a RAM >> region that starts on a 4K only aligned address. The natural operation >> in that case would be to try to allocate a memory region as big as >> possible up to the next 2MB boundary. Hence it would be valid to >> attempt to populate this 4K only aligned address using an order > 0 >> and < 9 (2MB order). The alternative here if the asserts are not >> removed would be to open-code a loop in the caller that iterates >> creating a bunch of order 0 mappings up to the 2MB boundary. The >> overhead in that case would be quite big, so I don't think we want to >> go down that route (also we would end up with a bunch of loops in the >> callers). > > Given the PVH Dom0 building issues which Roger and I worked on over the > Christmas period, there is a human-noticeable difference in construction > time when the caller is doing a loop over order 0 pages, vs an order 8 > allocation, and that was for a total of 4Mb of RAM. > > A dfn/count interface is actually more flexible than a dfn/order, > because it doesn't require the caller to know the superpage orders of > the underlying implementation to create efficient mappings. The caller not having to know the implementation supported super page orders is pretty desirable indeed. But afaics iommu_map() already allows for this, and even if it didn't this would not require switching from order to count as function parameter. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 15 January 2019 10:53 > To: Andrew Cooper > Cc: Paul Durrant ; Roger Pau Monne > ; Chao Gao ; xen-devel de...@lists.xenproject.org> > Subject: Re: [Xen-devel] an assertion triggered when running Xen on a HSW > desktop > > >>> On 15.01.19 at 11:42, wrote: > > On 15/01/2019 10:27, Roger Pau Monné wrote: > >> On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: > >>>>>> On 15.01.19 at 10:44, wrote: > >>>>> -Original Message- > >>>> [snip] > >>>>>>> (XEN) Xen call trace: > >>>>>>> (XEN)[] iommu_map+0xba/0x176 > >>>>>>> (XEN)[] iommu_hwdom_init+0xef/0x220 > >>>>>>> (XEN)[] dom0_construct_pvh+0x189/0x129e > >>>>>>> (XEN)[] construct_dom0+0xd4/0xb14 > >>>>>>> (XEN)[] __start_xen+0x2710/0x2830 > >>>>>>> (XEN)[] __high_start+0x53/0x55 > >>>>>>> (XEN) > >>>>>>> (XEN) > >>>>>>> (XEN) > >>>>>>> (XEN) Panic on CPU 0: > >>>>>>> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' > failed at > >>>>> iommu.c:323 > >>>>>>> (XEN) > >>>>>> Oh, this was added by Paul quite recently. You seem to be using a > >>>>>> rather old commit (a5b0eb3636), is there any reason for using such > an > >>>>>> old baseline? > >>>>> I was using the master branch. Your patch below did fix this issue. > >>>> Given this failure and the fact that valid orders differ between > different > >>>> architectures, I propose we change the argument to the > iommu_map/unmap > >>>> wrapper functions from an order to a count, thus making it clear that > there > >>>> is no alignment restriction. > >>> But the whole idea is for there to be an alignment restriction, such > >>> that it is easy to determine whether large page mappings can be > >>> used to satisfy the request. What's the exact case where a caller > >>> absolutely has to pass in a mis-aligned (dfn,size) tuple? > >> Taking PVH Dom0 builder as an example, it's possible to have a RAM > >> region that starts on a 4K only aligned address. The natural operation > >> in that case would be to try to allocate a memory region as big as > >> possible up to the next 2MB boundary. Hence it would be valid to > >> attempt to populate this 4K only aligned address using an order > 0 > >> and < 9 (2MB order). The alternative here if the asserts are not > >> removed would be to open-code a loop in the caller that iterates > >> creating a bunch of order 0 mappings up to the 2MB boundary. The > >> overhead in that case would be quite big, so I don't think we want to > >> go down that route (also we would end up with a bunch of loops in the > >> callers). > > > > Given the PVH Dom0 building issues which Roger and I worked on over the > > Christmas period, there is a human-noticeable difference in construction > > time when the caller is doing a loop over order 0 pages, vs an order 8 > > allocation, and that was for a total of 4Mb of RAM. > > > > A dfn/count interface is actually more flexible than a dfn/order, > > because it doesn't require the caller to know the superpage orders of > > the underlying implementation to create efficient mappings. > > The caller not having to know the implementation supported > super page orders is pretty desirable indeed. But afaics > iommu_map() already allows for this, and even if it didn't this > would not require switching from order to count as function > parameter. I guess the question is whether we want to allow arbitrary alignment of the memory passed into iommu_map() or not. If we do then I think a count parameter makes more sense. Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On Tue, Jan 15, 2019 at 03:49:07AM -0700, Jan Beulich wrote: > >>> On 15.01.19 at 11:27, wrote: > > On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: > >> >>> On 15.01.19 at 10:44, wrote: > >> >> -Original Message- > >> > [snip] > >> >> >> (XEN) Xen call trace: > >> >> >> (XEN)[] iommu_map+0xba/0x176 > >> >> >> (XEN)[] iommu_hwdom_init+0xef/0x220 > >> >> >> (XEN)[] dom0_construct_pvh+0x189/0x129e > >> >> >> (XEN)[] construct_dom0+0xd4/0xb14 > >> >> >> (XEN)[] __start_xen+0x2710/0x2830 > >> >> >> (XEN)[] __high_start+0x53/0x55 > >> >> >> (XEN) > >> >> >> (XEN) > >> >> >> (XEN) > >> >> >> (XEN) Panic on CPU 0: > >> >> >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed > >> >> >> at > >> >> iommu.c:323 > >> >> >> (XEN) > >> >> > > >> >> >Oh, this was added by Paul quite recently. You seem to be using a > >> >> >rather old commit (a5b0eb3636), is there any reason for using such an > >> >> >old baseline? > >> >> > >> >> I was using the master branch. Your patch below did fix this issue. > >> > > >> > Given this failure and the fact that valid orders differ between > >> > different > >> > architectures, I propose we change the argument to the iommu_map/unmap > >> > wrapper functions from an order to a count, thus making it clear that > >> > there > > > >> > is no alignment restriction. > >> > >> But the whole idea is for there to be an alignment restriction, such > >> that it is easy to determine whether large page mappings can be > >> used to satisfy the request. What's the exact case where a caller > >> absolutely has to pass in a mis-aligned (dfn,size) tuple? > > > > Taking PVH Dom0 builder as an example, it's possible to have a RAM > > region that starts on a 4K only aligned address. The natural operation > > in that case would be to try to allocate a memory region as big as > > possible up to the next 2MB boundary. Hence it would be valid to > > attempt to populate this 4K only aligned address using an order > 0 > > and < 9 (2MB order). The alternative here if the asserts are not > > removed would be to open-code a loop in the caller that iterates > > creating a bunch of order 0 mappings up to the 2MB boundary. The > > overhead in that case would be quite big, so I don't think we want to > > go down that route (also we would end up with a bunch of loops in the > > callers). > > I'm afraid I'm now more confused than before: If there's a RAM > region aligned to no better than 4k, how can this possibly be > populated with an order-greater-than-zero allocation? Why not? You can request a memory chunk of order 5 from alloc_domheap_pages for example and pass that to guest_physmap_add_page. That would be a perfectly fine operation to do in order to reach a memory address that's aligned to a 2MB boundary. The other option as said above is to force the caller to then have a loop that performs a bunch of order 0 guest_physmap_add_page until it reaches a 2MB aligned address. > And even > if I re-phrased your reply to mean an arbitrary alignment / order > less than 9, then populating this with such a smaller order is still > fine, and requesting the IOMMU mapping with that smaller order > is still not going to trip the ASSERT() in question. But the caller is then forced to iterate over the region and populate it with order 0 calls to guest_physmap_add_page, which introduces a lot of overhead. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
>>> On 15.01.19 at 11:55, wrote: > I guess the question is whether we want to allow arbitrary alignment of the > memory passed into iommu_map() or not. If we do then I think a count > parameter makes more sense. Agreed. Till now I simply don't see the use case. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
>>> On 15.01.19 at 12:07, wrote: > On Tue, Jan 15, 2019 at 03:49:07AM -0700, Jan Beulich wrote: >> >>> On 15.01.19 at 11:27, wrote: >> > On Tue, Jan 15, 2019 at 03:16:01AM -0700, Jan Beulich wrote: >> >> >>> On 15.01.19 at 10:44, wrote: >> >> >> -Original Message- >> >> > [snip] >> >> >> >> (XEN) Xen call trace: >> >> >> >> (XEN)[] iommu_map+0xba/0x176 >> >> >> >> (XEN)[] iommu_hwdom_init+0xef/0x220 >> >> >> >> (XEN)[] dom0_construct_pvh+0x189/0x129e >> >> >> >> (XEN)[] construct_dom0+0xd4/0xb14 >> >> >> >> (XEN)[] __start_xen+0x2710/0x2830 >> >> >> >> (XEN)[] __high_start+0x53/0x55 >> >> >> >> (XEN) >> >> >> >> (XEN) >> >> >> >> (XEN) >> >> >> >> (XEN) Panic on CPU 0: >> >> >> >> (XEN) Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' >> >> >> >> failed at >> >> >> iommu.c:323 >> >> >> >> (XEN) >> >> >> > >> >> >> >Oh, this was added by Paul quite recently. You seem to be using a >> >> >> >rather old commit (a5b0eb3636), is there any reason for using such an >> >> >> >old baseline? >> >> >> >> >> >> I was using the master branch. Your patch below did fix this issue. >> >> > >> >> > Given this failure and the fact that valid orders differ between >> >> > different > >> >> > architectures, I propose we change the argument to the iommu_map/unmap >> >> > wrapper functions from an order to a count, thus making it clear that > there >> > >> >> > is no alignment restriction. >> >> >> >> But the whole idea is for there to be an alignment restriction, such >> >> that it is easy to determine whether large page mappings can be >> >> used to satisfy the request. What's the exact case where a caller >> >> absolutely has to pass in a mis-aligned (dfn,size) tuple? >> > >> > Taking PVH Dom0 builder as an example, it's possible to have a RAM >> > region that starts on a 4K only aligned address. The natural operation >> > in that case would be to try to allocate a memory region as big as >> > possible up to the next 2MB boundary. Hence it would be valid to >> > attempt to populate this 4K only aligned address using an order > 0 >> > and < 9 (2MB order). The alternative here if the asserts are not >> > removed would be to open-code a loop in the caller that iterates >> > creating a bunch of order 0 mappings up to the 2MB boundary. The >> > overhead in that case would be quite big, so I don't think we want to >> > go down that route (also we would end up with a bunch of loops in the >> > callers). >> >> I'm afraid I'm now more confused than before: If there's a RAM >> region aligned to no better than 4k, how can this possibly be >> populated with an order-greater-than-zero allocation? > > Why not? You can request a memory chunk of order 5 from > alloc_domheap_pages for example and pass that to > guest_physmap_add_page. That would be a perfectly fine operation to do > in order to reach a memory address that's aligned to a 2MB boundary. I think it is never a good idea to request chunks of larger alignment (and hence higher order) than is necessary. I still don't understand the second sentence in this context though, so I guess I must still be missing something. >> And even >> if I re-phrased your reply to mean an arbitrary alignment / order >> less than 9, then populating this with such a smaller order is still >> fine, and requesting the IOMMU mapping with that smaller order >> is still not going to trip the ASSERT() in question. > > But the caller is then forced to iterate over the region and populate > it with order 0 calls to guest_physmap_add_page, which introduces a > lot of overhead. How is the overhead going to be any smaller if it's not the caller but iommu_map() to do the looping? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] an assertion triggered when running Xen on a HSW desktop
On 15/01/2019 11:50, Jan Beulich wrote: > >>> And even >>> if I re-phrased your reply to mean an arbitrary alignment / order >>> less than 9, then populating this with such a smaller order is still >>> fine, and requesting the IOMMU mapping with that smaller order >>> is still not going to trip the ASSERT() in question. >> But the caller is then forced to iterate over the region and populate >> it with order 0 calls to guest_physmap_add_page, which introduces a >> lot of overhead. > How is the overhead going to be any smaller if it's not the caller > but iommu_map() to do the looping? Because the calculation of how/when to change superpage level is far simpler when you've got a mapped table in your hand. You're also reducing the quantity of validation logic, and the absolute number of spinlocks which need to taken/released for a single arbitrary sized map. Furthermore, with the legacy mapping interface currently being used, you also reduce the quantity of IO-TLB Flushing. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
