* [PATCH v2 0/2] x86/dom0: improve PVH paging memory calculation @ 2018-12-05 14:54 Roger Pau Monne 2018-12-05 14:54 ` [PATCH v2 1/2] x86/dom0: rename paging function Roger Pau Monne 2018-12-05 14:55 ` [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations Roger Pau Monne 0 siblings, 2 replies; 33+ messages in thread From: Roger Pau Monne @ 2018-12-05 14:54 UTC (permalink / raw) To: xen-devel; +Cc: Roger Pau Monne Hello, There have been several reports of PVH Dom0 builder running out of memory due to bad paging memory approximation done in dom0_compute_nr_pages. The most recent reports is: https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg03103.html This series attempts to improve the situation by introducing a new function to approximate the memory usage of HAP and IOMMU page tables. It also takes into account whether the IOMMU page tables are shared with HAP or not when calculating the amount of available memory. It can be found on the following git repo: git://xenbits.xen.org/people/royger/xen.git dom0-paging-v2 Thanks, Roger. Roger Pau Monne (2): x86/dom0: rename paging function x86/dom0: improve paging memory usage calculations xen/arch/x86/dom0_build.c | 33 +++++++++++++++++++++++++++----- xen/arch/x86/hvm/dom0_build.c | 6 ++++-- xen/include/asm-x86/dom0_build.h | 4 +++- 3 files changed, 35 insertions(+), 8 deletions(-) -- 2.19.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-05 14:54 [PATCH v2 0/2] x86/dom0: improve PVH paging memory calculation Roger Pau Monne @ 2018-12-05 14:54 ` Roger Pau Monne 2018-12-06 12:31 ` Wei Liu 2018-12-11 15:08 ` Jan Beulich 2018-12-05 14:55 ` [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations Roger Pau Monne 1 sibling, 2 replies; 33+ messages in thread From: Roger Pau Monne @ 2018-12-05 14:54 UTC (permalink / raw) To: xen-devel Cc: Andrew Cooper, andrei.semenov, Wei Liu, Jan Beulich, Roger Pau Monne To note it's calculating the approximate amount of memory required by shadow paging. No functional change. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: andrei.semenov@bertin.fr --- xen/arch/x86/dom0_build.c | 4 ++-- xen/arch/x86/hvm/dom0_build.c | 2 +- xen/include/asm-x86/dom0_build.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c index 5e2ad4bd56..ba9aa85611 100644 --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -256,7 +256,7 @@ boolean_param("ro-hpet", ro_hpet); unsigned int __initdata dom0_memflags = MEMF_no_dma|MEMF_exact_node; -unsigned long __init dom0_paging_pages(const struct domain *d, +unsigned long __init dom0_shadow_pages(const struct domain *d, unsigned long nr_pages) { /* Copied from: libxl_get_required_shadow_memory() */ @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( break; /* Reserve memory for shadow or HAP. */ - avail -= dom0_paging_pages(d, nr_pages); + avail -= dom0_shadow_pages(d, nr_pages); } if ( is_pv_domain(d) && diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index 12c20a4b66..2af2bd8c3d 100644 --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -366,7 +366,7 @@ static int __init pvh_setup_p2m(struct domain *d) pvh_setup_e820(d, nr_pages); do { preempted = false; - paging_set_allocation(d, dom0_paging_pages(d, nr_pages), + paging_set_allocation(d, dom0_shadow_pages(d, nr_pages), &preempted); process_pending_softirqs(); } while ( preempted ); diff --git a/xen/include/asm-x86/dom0_build.h b/xen/include/asm-x86/dom0_build.h index 33a5483739..22f960b8b0 100644 --- a/xen/include/asm-x86/dom0_build.h +++ b/xen/include/asm-x86/dom0_build.h @@ -25,7 +25,7 @@ int dom0_construct_pvh(struct domain *d, const module_t *image, module_t *initrd, char *cmdline); -unsigned long dom0_paging_pages(const struct domain *d, +unsigned long dom0_shadow_pages(const struct domain *d, unsigned long nr_pages); void dom0_update_physmap(struct domain *d, unsigned long pfn, -- 2.19.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-05 14:54 ` [PATCH v2 1/2] x86/dom0: rename paging function Roger Pau Monne @ 2018-12-06 12:31 ` Wei Liu 2018-12-11 15:08 ` Jan Beulich 1 sibling, 0 replies; 33+ messages in thread From: Wei Liu @ 2018-12-06 12:31 UTC (permalink / raw) To: Roger Pau Monne Cc: xen-devel, andrei.semenov, Wei Liu, Jan Beulich, Andrew Cooper On Wed, Dec 05, 2018 at 03:54:59PM +0100, Roger Pau Monne wrote: > To note it's calculating the approximate amount of memory required by > shadow paging. > > No functional change. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-05 14:54 ` [PATCH v2 1/2] x86/dom0: rename paging function Roger Pau Monne 2018-12-06 12:31 ` Wei Liu @ 2018-12-11 15:08 ` Jan Beulich 2018-12-11 15:19 ` Roger Pau Monné 1 sibling, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-11 15:08 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: > To note it's calculating the approximate amount of memory required by > shadow paging. I don't understand this logic, and ... > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( > break; > > /* Reserve memory for shadow or HAP. */ > - avail -= dom0_paging_pages(d, nr_pages); > + avail -= dom0_shadow_pages(d, nr_pages); > } ... the comment here (and lack of conditional restricting the code to shadow mode) appear to support me: Have you been mislead by the function having a comment referring to libxl_get_required_shadow_memory()? I think if anything that libxl function would want to be renamed (to replace "shadow" by something more generic in its name). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-11 15:08 ` Jan Beulich @ 2018-12-11 15:19 ` Roger Pau Monné 2018-12-11 15:33 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-11 15:19 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: > >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: > > To note it's calculating the approximate amount of memory required by > > shadow paging. > > I don't understand this logic, and ... > > > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( > > break; > > > > /* Reserve memory for shadow or HAP. */ > > - avail -= dom0_paging_pages(d, nr_pages); > > + avail -= dom0_shadow_pages(d, nr_pages); > > } > > ... the comment here (and lack of conditional restricting the > code to shadow mode) appear to support me: Have you > been mislead by the function having a comment referring > to libxl_get_required_shadow_memory()? I think if anything > that libxl function would want to be renamed (to replace > "shadow" by something more generic in its name). But the logic in dom0_shadow_pages to calculate the size of the paging memory pool is specifically for shadow AFAICT, I don't think HAP needs to take the number of vCPUs into account, since there's only a single p2m for the whole domain. OTOH shadow needs to take the number of vCPUs into account because each one will have a different shadow. Note that patch 2 in this series adds a function to calculate the size of the paging memory pool for HAP, and a conditional is added to the expression above that takes into account whether shadow or HAP is in use when subtracting from the amount of available memory. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-11 15:19 ` Roger Pau Monné @ 2018-12-11 15:33 ` Jan Beulich 2018-12-12 9:14 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-11 15:33 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: >> > To note it's calculating the approximate amount of memory required by >> > shadow paging. >> >> I don't understand this logic, and ... >> >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( >> > break; >> > >> > /* Reserve memory for shadow or HAP. */ >> > - avail -= dom0_paging_pages(d, nr_pages); >> > + avail -= dom0_shadow_pages(d, nr_pages); >> > } >> >> ... the comment here (and lack of conditional restricting the >> code to shadow mode) appear to support me: Have you >> been mislead by the function having a comment referring >> to libxl_get_required_shadow_memory()? I think if anything >> that libxl function would want to be renamed (to replace >> "shadow" by something more generic in its name). > > But the logic in dom0_shadow_pages to calculate the size of the paging > memory pool is specifically for shadow AFAICT, I don't think HAP needs > to take the number of vCPUs into account, since there's only a > single p2m for the whole domain. OTOH shadow needs to take the number > of vCPUs into account because each one will have a different shadow. Yes, the vCPU count aspect is indeed shadow specific. However, as said in reply to the other patch, the calculation here was at least supposed to also take into account the P2M part of the needed allocations. Yet the P2M part ought to be similar between both modes. > Note that patch 2 in this series adds a function to calculate the size > of the paging memory pool for HAP, and a conditional is added to the > expression above that takes into account whether shadow or HAP is in > use when subtracting from the amount of available memory. Well, assuming we can settle on what shape patch 2 should take I can see the point in doing the rename here, but then with an adjusted description: Especially in light of the code comment still visible above you'll want to point out that the rename is in preparation of splitting the calculations. Since I question the split, though, the rename (in a separate patch) is questionable to me too. If we used uniform P2M calculations and added just shadow's per-vCPU extra on top, no rename in a separate patch would seem warranted. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-11 15:33 ` Jan Beulich @ 2018-12-12 9:14 ` Roger Pau Monné 2018-12-12 9:53 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 9:14 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Tue, Dec 11, 2018 at 08:33:08AM -0700, Jan Beulich wrote: > >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: > > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: > >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: > >> > To note it's calculating the approximate amount of memory required by > >> > shadow paging. > >> > >> I don't understand this logic, and ... > >> > >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( > >> > break; > >> > > >> > /* Reserve memory for shadow or HAP. */ > >> > - avail -= dom0_paging_pages(d, nr_pages); > >> > + avail -= dom0_shadow_pages(d, nr_pages); > >> > } > >> > >> ... the comment here (and lack of conditional restricting the > >> code to shadow mode) appear to support me: Have you > >> been mislead by the function having a comment referring > >> to libxl_get_required_shadow_memory()? I think if anything > >> that libxl function would want to be renamed (to replace > >> "shadow" by something more generic in its name). > > > > But the logic in dom0_shadow_pages to calculate the size of the paging > > memory pool is specifically for shadow AFAICT, I don't think HAP needs > > to take the number of vCPUs into account, since there's only a > > single p2m for the whole domain. OTOH shadow needs to take the number > > of vCPUs into account because each one will have a different shadow. > > Yes, the vCPU count aspect is indeed shadow specific. However, > as said in reply to the other patch, the calculation here was at > least supposed to also take into account the P2M part of the > needed allocations. Yet the P2M part ought to be similar between > both modes. > > > Note that patch 2 in this series adds a function to calculate the size > > of the paging memory pool for HAP, and a conditional is added to the > > expression above that takes into account whether shadow or HAP is in > > use when subtracting from the amount of available memory. > > Well, assuming we can settle on what shape patch 2 should take > I can see the point in doing the rename here, but then with an > adjusted description: Especially in light of the code comment still > visible above you'll want to point out that the rename is in > preparation of splitting the calculations. Since I question the split, > though, the rename (in a separate patch) is questionable to me > too. If we used uniform P2M calculations and added just shadow's > per-vCPU extra on top, no rename in a separate patch would > seem warranted. The current calculations in dom0_paging_pages assume 1 page is needed for each 1MB of guest memory for the p2m, do you think this is OK? (and suitable to be used for HAP/IOMMU page tables also) Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 9:14 ` Roger Pau Monné @ 2018-12-12 9:53 ` Jan Beulich 2018-12-12 10:04 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-12 9:53 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 10:14, <roger.pau@citrix.com> wrote: > On Tue, Dec 11, 2018 at 08:33:08AM -0700, Jan Beulich wrote: >> >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: >> > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: >> >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: >> >> > To note it's calculating the approximate amount of memory required by >> >> > shadow paging. >> >> >> >> I don't understand this logic, and ... >> >> >> >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( >> >> > break; >> >> > >> >> > /* Reserve memory for shadow or HAP. */ >> >> > - avail -= dom0_paging_pages(d, nr_pages); >> >> > + avail -= dom0_shadow_pages(d, nr_pages); >> >> > } >> >> >> >> ... the comment here (and lack of conditional restricting the >> >> code to shadow mode) appear to support me: Have you >> >> been mislead by the function having a comment referring >> >> to libxl_get_required_shadow_memory()? I think if anything >> >> that libxl function would want to be renamed (to replace >> >> "shadow" by something more generic in its name). >> > >> > But the logic in dom0_shadow_pages to calculate the size of the paging >> > memory pool is specifically for shadow AFAICT, I don't think HAP needs >> > to take the number of vCPUs into account, since there's only a >> > single p2m for the whole domain. OTOH shadow needs to take the number >> > of vCPUs into account because each one will have a different shadow. >> >> Yes, the vCPU count aspect is indeed shadow specific. However, >> as said in reply to the other patch, the calculation here was at >> least supposed to also take into account the P2M part of the >> needed allocations. Yet the P2M part ought to be similar between >> both modes. >> >> > Note that patch 2 in this series adds a function to calculate the size >> > of the paging memory pool for HAP, and a conditional is added to the >> > expression above that takes into account whether shadow or HAP is in >> > use when subtracting from the amount of available memory. >> >> Well, assuming we can settle on what shape patch 2 should take >> I can see the point in doing the rename here, but then with an >> adjusted description: Especially in light of the code comment still >> visible above you'll want to point out that the rename is in >> preparation of splitting the calculations. Since I question the split, >> though, the rename (in a separate patch) is questionable to me >> too. If we used uniform P2M calculations and added just shadow's >> per-vCPU extra on top, no rename in a separate patch would >> seem warranted. > > The current calculations in dom0_paging_pages assume 1 page is needed > for each 1MB of guest memory for the p2m, do you think this is OK? > (and suitable to be used for HAP/IOMMU page tables also) Well, 1 page per 1Mb means the same as your current 8 bytes per page times 2 (for separate P2M and IOMMU tables), afaict. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 9:53 ` Jan Beulich @ 2018-12-12 10:04 ` Roger Pau Monné 2018-12-12 10:32 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 10:04 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Wed, Dec 12, 2018 at 02:53:26AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 10:14, <roger.pau@citrix.com> wrote: > > On Tue, Dec 11, 2018 at 08:33:08AM -0700, Jan Beulich wrote: > >> >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: > >> > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: > >> >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: > >> >> > To note it's calculating the approximate amount of memory required by > >> >> > shadow paging. > >> >> > >> >> I don't understand this logic, and ... > >> >> > >> >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( > >> >> > break; > >> >> > > >> >> > /* Reserve memory for shadow or HAP. */ > >> >> > - avail -= dom0_paging_pages(d, nr_pages); > >> >> > + avail -= dom0_shadow_pages(d, nr_pages); > >> >> > } > >> >> > >> >> ... the comment here (and lack of conditional restricting the > >> >> code to shadow mode) appear to support me: Have you > >> >> been mislead by the function having a comment referring > >> >> to libxl_get_required_shadow_memory()? I think if anything > >> >> that libxl function would want to be renamed (to replace > >> >> "shadow" by something more generic in its name). > >> > > >> > But the logic in dom0_shadow_pages to calculate the size of the paging > >> > memory pool is specifically for shadow AFAICT, I don't think HAP needs > >> > to take the number of vCPUs into account, since there's only a > >> > single p2m for the whole domain. OTOH shadow needs to take the number > >> > of vCPUs into account because each one will have a different shadow. > >> > >> Yes, the vCPU count aspect is indeed shadow specific. However, > >> as said in reply to the other patch, the calculation here was at > >> least supposed to also take into account the P2M part of the > >> needed allocations. Yet the P2M part ought to be similar between > >> both modes. > >> > >> > Note that patch 2 in this series adds a function to calculate the size > >> > of the paging memory pool for HAP, and a conditional is added to the > >> > expression above that takes into account whether shadow or HAP is in > >> > use when subtracting from the amount of available memory. > >> > >> Well, assuming we can settle on what shape patch 2 should take > >> I can see the point in doing the rename here, but then with an > >> adjusted description: Especially in light of the code comment still > >> visible above you'll want to point out that the rename is in > >> preparation of splitting the calculations. Since I question the split, > >> though, the rename (in a separate patch) is questionable to me > >> too. If we used uniform P2M calculations and added just shadow's > >> per-vCPU extra on top, no rename in a separate patch would > >> seem warranted. > > > > The current calculations in dom0_paging_pages assume 1 page is needed > > for each 1MB of guest memory for the p2m, do you think this is OK? > > (and suitable to be used for HAP/IOMMU page tables also) > > Well, 1 page per 1Mb means the same as your current 8 bytes > per page times 2 (for separate P2M and IOMMU tables), afaict. I was planning to use 1 page per 1Mb for the p2m, and then 1 page per 1Mb for the IOMMU, so 16 bytes per page. You mentioned there's some code (for PV?) to calculate the size of the page tables but I'm having trouble finding it (mainly because I'm not that familiar with PV), could you point me to it? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 10:04 ` Roger Pau Monné @ 2018-12-12 10:32 ` Jan Beulich 2018-12-12 15:56 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-12 10:32 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 11:04, <roger.pau@citrix.com> wrote: > On Wed, Dec 12, 2018 at 02:53:26AM -0700, Jan Beulich wrote: >> >>> On 12.12.18 at 10:14, <roger.pau@citrix.com> wrote: >> > On Tue, Dec 11, 2018 at 08:33:08AM -0700, Jan Beulich wrote: >> >> >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: >> >> > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: >> >> >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: >> >> >> > To note it's calculating the approximate amount of memory required by >> >> >> > shadow paging. >> >> >> >> >> >> I don't understand this logic, and ... >> >> >> >> >> >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( >> >> >> > break; >> >> >> > >> >> >> > /* Reserve memory for shadow or HAP. */ >> >> >> > - avail -= dom0_paging_pages(d, nr_pages); >> >> >> > + avail -= dom0_shadow_pages(d, nr_pages); >> >> >> > } >> >> >> >> >> >> ... the comment here (and lack of conditional restricting the >> >> >> code to shadow mode) appear to support me: Have you >> >> >> been mislead by the function having a comment referring >> >> >> to libxl_get_required_shadow_memory()? I think if anything >> >> >> that libxl function would want to be renamed (to replace >> >> >> "shadow" by something more generic in its name). >> >> > >> >> > But the logic in dom0_shadow_pages to calculate the size of the paging >> >> > memory pool is specifically for shadow AFAICT, I don't think HAP needs >> >> > to take the number of vCPUs into account, since there's only a >> >> > single p2m for the whole domain. OTOH shadow needs to take the number >> >> > of vCPUs into account because each one will have a different shadow. >> >> >> >> Yes, the vCPU count aspect is indeed shadow specific. However, >> >> as said in reply to the other patch, the calculation here was at >> >> least supposed to also take into account the P2M part of the >> >> needed allocations. Yet the P2M part ought to be similar between >> >> both modes. >> >> >> >> > Note that patch 2 in this series adds a function to calculate the size >> >> > of the paging memory pool for HAP, and a conditional is added to the >> >> > expression above that takes into account whether shadow or HAP is in >> >> > use when subtracting from the amount of available memory. >> >> >> >> Well, assuming we can settle on what shape patch 2 should take >> >> I can see the point in doing the rename here, but then with an >> >> adjusted description: Especially in light of the code comment still >> >> visible above you'll want to point out that the rename is in >> >> preparation of splitting the calculations. Since I question the split, >> >> though, the rename (in a separate patch) is questionable to me >> >> too. If we used uniform P2M calculations and added just shadow's >> >> per-vCPU extra on top, no rename in a separate patch would >> >> seem warranted. >> > >> > The current calculations in dom0_paging_pages assume 1 page is needed >> > for each 1MB of guest memory for the p2m, do you think this is OK? >> > (and suitable to be used for HAP/IOMMU page tables also) >> >> Well, 1 page per 1Mb means the same as your current 8 bytes >> per page times 2 (for separate P2M and IOMMU tables), afaict. > > I was planning to use 1 page per 1Mb for the p2m, and then 1 page per > 1Mb for the IOMMU, so 16 bytes per page. Well, that's (as said for patch 2) quite a bit of an over-estimate, but then again reserving a little too much is perhaps better than reserving too little. > You mentioned there's some code (for PV?) to calculate the size of the > page tables but I'm having trouble finding it (mainly because I'm not > that familiar with PV), could you point me to it? In dom0_construct_pv() you'll find a loop starting with "for ( nr_pt_pages = 2; ; nr_pt_pages++ )". It's not the neatest, but at least we've never had reports of failure. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 10:32 ` Jan Beulich @ 2018-12-12 15:56 ` Roger Pau Monné 2018-12-12 16:15 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 15:56 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Wed, Dec 12, 2018 at 03:32:53AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 11:04, <roger.pau@citrix.com> wrote: > > On Wed, Dec 12, 2018 at 02:53:26AM -0700, Jan Beulich wrote: > >> >>> On 12.12.18 at 10:14, <roger.pau@citrix.com> wrote: > >> > On Tue, Dec 11, 2018 at 08:33:08AM -0700, Jan Beulich wrote: > >> >> >>> On 11.12.18 at 16:19, <roger.pau@citrix.com> wrote: > >> >> > On Tue, Dec 11, 2018 at 08:08:51AM -0700, Jan Beulich wrote: > >> >> >> >>> On 05.12.18 at 15:54, <roger.pau@citrix.com> wrote: > >> >> >> > To note it's calculating the approximate amount of memory required by > >> >> >> > shadow paging. > >> >> >> > >> >> >> I don't understand this logic, and ... > >> >> >> > >> >> >> > @@ -325,7 +325,7 @@ unsigned long __init dom0_compute_nr_pages( > >> >> >> > break; > >> >> >> > > >> >> >> > /* Reserve memory for shadow or HAP. */ > >> >> >> > - avail -= dom0_paging_pages(d, nr_pages); > >> >> >> > + avail -= dom0_shadow_pages(d, nr_pages); > >> >> >> > } > >> >> >> > >> >> >> ... the comment here (and lack of conditional restricting the > >> >> >> code to shadow mode) appear to support me: Have you > >> >> >> been mislead by the function having a comment referring > >> >> >> to libxl_get_required_shadow_memory()? I think if anything > >> >> >> that libxl function would want to be renamed (to replace > >> >> >> "shadow" by something more generic in its name). > >> >> > > >> >> > But the logic in dom0_shadow_pages to calculate the size of the paging > >> >> > memory pool is specifically for shadow AFAICT, I don't think HAP needs > >> >> > to take the number of vCPUs into account, since there's only a > >> >> > single p2m for the whole domain. OTOH shadow needs to take the number > >> >> > of vCPUs into account because each one will have a different shadow. > >> >> > >> >> Yes, the vCPU count aspect is indeed shadow specific. However, > >> >> as said in reply to the other patch, the calculation here was at > >> >> least supposed to also take into account the P2M part of the > >> >> needed allocations. Yet the P2M part ought to be similar between > >> >> both modes. > >> >> > >> >> > Note that patch 2 in this series adds a function to calculate the size > >> >> > of the paging memory pool for HAP, and a conditional is added to the > >> >> > expression above that takes into account whether shadow or HAP is in > >> >> > use when subtracting from the amount of available memory. > >> >> > >> >> Well, assuming we can settle on what shape patch 2 should take > >> >> I can see the point in doing the rename here, but then with an > >> >> adjusted description: Especially in light of the code comment still > >> >> visible above you'll want to point out that the rename is in > >> >> preparation of splitting the calculations. Since I question the split, > >> >> though, the rename (in a separate patch) is questionable to me > >> >> too. If we used uniform P2M calculations and added just shadow's > >> >> per-vCPU extra on top, no rename in a separate patch would > >> >> seem warranted. > >> > > >> > The current calculations in dom0_paging_pages assume 1 page is needed > >> > for each 1MB of guest memory for the p2m, do you think this is OK? > >> > (and suitable to be used for HAP/IOMMU page tables also) > >> > >> Well, 1 page per 1Mb means the same as your current 8 bytes > >> per page times 2 (for separate P2M and IOMMU tables), afaict. > > > > I was planning to use 1 page per 1Mb for the p2m, and then 1 page per > > 1Mb for the IOMMU, so 16 bytes per page. > > Well, that's (as said for patch 2) quite a bit of an over-estimate, > but then again reserving a little too much is perhaps better than > reserving too little. > > > You mentioned there's some code (for PV?) to calculate the size of the > > page tables but I'm having trouble finding it (mainly because I'm not > > that familiar with PV), could you point me to it? > > In dom0_construct_pv() you'll find a loop starting with > "for ( nr_pt_pages = 2; ; nr_pt_pages++ )". It's not the neatest, > but at least we've never had reports of failure. That seems quite complicated, what about using the formula below: /* * Approximate the memory required for the HAP/IOMMU page tables by * pessimistically assuming every guest page will use a p2m page table * entry. */ return DIV_ROUND_UP(( /* Account for one entry in the L1 per page. */ nr_pages + /* Account for one entry in the L2 per 512 pages. */ DIV_ROUND_UP(nr_pages, 512) + /* Account for one entry in the L3 per 512^2 pages. */ DIV_ROUND_UP(nr_pages, 512 * 512) + /* Account for one entry in the L4 per 512^3 pages. */ DIV_ROUND_UP(nr_pages, 512 * 512 * 512) + ) * 8, PAGE_SIZE << PAGE_ORDER_4K); That takes into account higher level page table structures. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 15:56 ` Roger Pau Monné @ 2018-12-12 16:15 ` Jan Beulich 2018-12-12 17:05 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-12 16:15 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 16:56, <roger.pau@citrix.com> wrote: > On Wed, Dec 12, 2018 at 03:32:53AM -0700, Jan Beulich wrote: >> >>> On 12.12.18 at 11:04, <roger.pau@citrix.com> wrote: >> > You mentioned there's some code (for PV?) to calculate the size of the >> > page tables but I'm having trouble finding it (mainly because I'm not >> > that familiar with PV), could you point me to it? >> >> In dom0_construct_pv() you'll find a loop starting with >> "for ( nr_pt_pages = 2; ; nr_pt_pages++ )". It's not the neatest, >> but at least we've never had reports of failure. > > That seems quite complicated, what about using the formula below: > > /* > * Approximate the memory required for the HAP/IOMMU page tables by > * pessimistically assuming every guest page will use a p2m page table > * entry. > */ > return DIV_ROUND_UP(( > /* Account for one entry in the L1 per page. */ > nr_pages + > /* Account for one entry in the L2 per 512 pages. */ > DIV_ROUND_UP(nr_pages, 512) + > /* Account for one entry in the L3 per 512^2 pages. */ > DIV_ROUND_UP(nr_pages, 512 * 512) + > /* Account for one entry in the L4 per 512^3 pages. */ > DIV_ROUND_UP(nr_pages, 512 * 512 * 512) + > ) * 8, PAGE_SIZE << PAGE_ORDER_4K); > > That takes into account higher level page table structures. That's a fair approximation without 2M and 1G pages available. I'm unconvinced we want to over-estimate this heavily in the more common case of large page mappings being available. Otoh this provides enough resources to later also deal with shattering of large pages. The MMIO side of things of course still remains unclear. What I don't understand in any case though is "PAGE_SIZE << PAGE_ORDER_4K". This is x86 code - why not just PAGE_SIZE? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-12 16:15 ` Jan Beulich @ 2018-12-12 17:05 ` Roger Pau Monné [not found] ` <3F7E1F6E020000A10063616D@prv1-mh.provo.novell.com> 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 17:05 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 16:56, <roger.pau@citrix.com> wrote: > > On Wed, Dec 12, 2018 at 03:32:53AM -0700, Jan Beulich wrote: > >> >>> On 12.12.18 at 11:04, <roger.pau@citrix.com> wrote: > >> > You mentioned there's some code (for PV?) to calculate the size of the > >> > page tables but I'm having trouble finding it (mainly because I'm not > >> > that familiar with PV), could you point me to it? > >> > >> In dom0_construct_pv() you'll find a loop starting with > >> "for ( nr_pt_pages = 2; ; nr_pt_pages++ )". It's not the neatest, > >> but at least we've never had reports of failure. > > > > That seems quite complicated, what about using the formula below: > > > > /* > > * Approximate the memory required for the HAP/IOMMU page tables by > > * pessimistically assuming every guest page will use a p2m page table > > * entry. > > */ > > return DIV_ROUND_UP(( > > /* Account for one entry in the L1 per page. */ > > nr_pages + > > /* Account for one entry in the L2 per 512 pages. */ > > DIV_ROUND_UP(nr_pages, 512) + > > /* Account for one entry in the L3 per 512^2 pages. */ > > DIV_ROUND_UP(nr_pages, 512 * 512) + > > /* Account for one entry in the L4 per 512^3 pages. */ > > DIV_ROUND_UP(nr_pages, 512 * 512 * 512) + > > ) * 8, PAGE_SIZE << PAGE_ORDER_4K); > > > > That takes into account higher level page table structures. > > That's a fair approximation without 2M and 1G pages available. I'm > unconvinced we want to over-estimate this heavily in the more > common case of large page mappings being available. Otoh this > provides enough resources to later also deal with shattering of > large pages. > > The MMIO side of things of course still remains unclear. Right, for the MMIO and the handling of grant and foreign mappings it's not clear how we want to proceed. Maybe account for all host RAM (total_pages) plus MMIO BARs? > What I don't understand in any case though is > "PAGE_SIZE << PAGE_ORDER_4K". This is x86 code - why not > just PAGE_SIZE? Oh, I've done it like that because this is related to p2m code, which uses this way to get the page size. IIRC you told me to use this for things like pvh_setup_e820. I don't mind switching to just PAGE_SIZE. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
[parent not found: <3F7E1F6E020000A10063616D@prv1-mh.provo.novell.com>]
* Re: [PATCH v2 1/2] x86/dom0: rename paging function [not found] ` <3F7E1F6E020000A10063616D@prv1-mh.provo.novell.com> @ 2018-12-13 7:45 ` Jan Beulich 2018-12-13 9:14 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-13 7:45 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: >> The MMIO side of things of course still remains unclear. > > Right, for the MMIO and the handling of grant and foreign mappings it's > not clear how we want to proceed. > > Maybe account for all host RAM (total_pages) plus MMIO BARs? Well, I thought we've already settled on it being impossible to account for all MMIO BARs at this point. >> What I don't understand in any case though is >> "PAGE_SIZE << PAGE_ORDER_4K". This is x86 code - why not >> just PAGE_SIZE? > > Oh, I've done it like that because this is related to p2m code, which > uses this way to get the page size. IIRC you told me to use this for > things like pvh_setup_e820. I don't mind switching to just PAGE_SIZE. Oh, I see. It's fine either way then. My general way of thinking here is that outside of x86 code we better use these PAGE_ORDER_* values, while in x86 specific code I don't see the point. But indeed the p2m code is littered with them. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-13 7:45 ` Jan Beulich @ 2018-12-13 9:14 ` Roger Pau Monné [not found] ` <12305AED020000300063616D@prv1-mh.provo.novell.com> 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-13 9:14 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: > > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: > >> The MMIO side of things of course still remains unclear. > > > > Right, for the MMIO and the handling of grant and foreign mappings it's > > not clear how we want to proceed. > > > > Maybe account for all host RAM (total_pages) plus MMIO BARs? > > Well, I thought we've already settled on it being impossible to > account for all MMIO BARs at this point. Well, I could iterate over all the registered PCI devices and size the BARs (without VF BARs at least initially). This is quite cumbersome, my other option would be using max_page and hope that there are enough holes to make up for BAR MMIO regions. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
[parent not found: <12305AED020000300063616D@prv1-mh.provo.novell.com>]
* Re: [PATCH v2 1/2] x86/dom0: rename paging function [not found] ` <12305AED020000300063616D@prv1-mh.provo.novell.com> @ 2018-12-13 10:17 ` Jan Beulich 2018-12-13 14:20 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-13 10:17 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 13.12.18 at 10:14, <roger.pau@citrix.com> wrote: > On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: >> >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: >> > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: >> >> The MMIO side of things of course still remains unclear. >> > >> > Right, for the MMIO and the handling of grant and foreign mappings it's >> > not clear how we want to proceed. >> > >> > Maybe account for all host RAM (total_pages) plus MMIO BARs? >> >> Well, I thought we've already settled on it being impossible to >> account for all MMIO BARs at this point. > > Well, I could iterate over all the registered PCI devices and size > the BARs (without VF BARs at least initially). This is quite > cumbersome, my other option would be using max_page and hope that > there are enough holes to make up for BAR MMIO regions. Well, maybe we could live with this for now. I certainly would prefer to have a 3rd opinion though, as I continue to feel uneasy with this rather imprecise estimation (i.e. I'd much prefer a more dynamic / on-demand approach). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-13 10:17 ` Jan Beulich @ 2018-12-13 14:20 ` Roger Pau Monné [not found] ` <7320EEF8020000C00063616D@prv1-mh.provo.novell.com> 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-13 14:20 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Thu, Dec 13, 2018 at 03:17:05AM -0700, Jan Beulich wrote: > >>> On 13.12.18 at 10:14, <roger.pau@citrix.com> wrote: > > On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: > >> >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: > >> > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: > >> >> The MMIO side of things of course still remains unclear. > >> > > >> > Right, for the MMIO and the handling of grant and foreign mappings it's > >> > not clear how we want to proceed. > >> > > >> > Maybe account for all host RAM (total_pages) plus MMIO BARs? > >> > >> Well, I thought we've already settled on it being impossible to > >> account for all MMIO BARs at this point. > > > > Well, I could iterate over all the registered PCI devices and size > > the BARs (without VF BARs at least initially). This is quite > > cumbersome, my other option would be using max_page and hope that > > there are enough holes to make up for BAR MMIO regions. > > Well, maybe we could live with this for now. I certainly would > prefer to have a 3rd opinion though, as I continue to feel uneasy > with this rather imprecise estimation (i.e. I'd much prefer a more > dynamic / on-demand approach). I agree it's not a perfect solution, but I think what's currently done is even worse, and we already had bug reports of users seeing Xen panic at PVH Dom0 build time if no dom0_mem parameter is specified. Would you be OK with using max_page then? This is the less complex option to implement ATM, and BAR sizing can be added later together with a more dynamic p2m memory management. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
[parent not found: <7320EEF8020000C00063616D@prv1-mh.provo.novell.com>]
* Re: [PATCH v2 1/2] x86/dom0: rename paging function [not found] ` <7320EEF8020000C00063616D@prv1-mh.provo.novell.com> @ 2018-12-13 14:47 ` Jan Beulich 2019-01-29 15:02 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-13 14:47 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 13.12.18 at 15:20, <roger.pau@citrix.com> wrote: > On Thu, Dec 13, 2018 at 03:17:05AM -0700, Jan Beulich wrote: >> >>> On 13.12.18 at 10:14, <roger.pau@citrix.com> wrote: >> > On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: >> >> >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: >> >> > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: >> >> >> The MMIO side of things of course still remains unclear. >> >> > >> >> > Right, for the MMIO and the handling of grant and foreign mappings it's >> >> > not clear how we want to proceed. >> >> > >> >> > Maybe account for all host RAM (total_pages) plus MMIO BARs? >> >> >> >> Well, I thought we've already settled on it being impossible to >> >> account for all MMIO BARs at this point. >> > >> > Well, I could iterate over all the registered PCI devices and size >> > the BARs (without VF BARs at least initially). This is quite >> > cumbersome, my other option would be using max_page and hope that >> > there are enough holes to make up for BAR MMIO regions. >> >> Well, maybe we could live with this for now. I certainly would >> prefer to have a 3rd opinion though, as I continue to feel uneasy >> with this rather imprecise estimation (i.e. I'd much prefer a more >> dynamic / on-demand approach). > > I agree it's not a perfect solution, but I think what's currently done > is even worse, and we already had bug reports of users seeing Xen > panic at PVH Dom0 build time if no dom0_mem parameter is specified. > > Would you be OK with using max_page then? I'm not going to say yes or no here without having seen a (qualified) 3rd opinion. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] x86/dom0: rename paging function 2018-12-13 14:47 ` Jan Beulich @ 2019-01-29 15:02 ` Roger Pau Monné [not found] ` <812B19D1020000B00063616D@prv1-mh.provo.novell.com> 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2019-01-29 15:02 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Thu, Dec 13, 2018 at 07:47:59AM -0700, Jan Beulich wrote: > >>> On 13.12.18 at 15:20, <roger.pau@citrix.com> wrote: > > On Thu, Dec 13, 2018 at 03:17:05AM -0700, Jan Beulich wrote: > >> >>> On 13.12.18 at 10:14, <roger.pau@citrix.com> wrote: > >> > On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: > >> >> >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: > >> >> > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: > >> >> >> The MMIO side of things of course still remains unclear. > >> >> > > >> >> > Right, for the MMIO and the handling of grant and foreign mappings it's > >> >> > not clear how we want to proceed. > >> >> > > >> >> > Maybe account for all host RAM (total_pages) plus MMIO BARs? > >> >> > >> >> Well, I thought we've already settled on it being impossible to > >> >> account for all MMIO BARs at this point. > >> > > >> > Well, I could iterate over all the registered PCI devices and size > >> > the BARs (without VF BARs at least initially). This is quite > >> > cumbersome, my other option would be using max_page and hope that > >> > there are enough holes to make up for BAR MMIO regions. > >> > >> Well, maybe we could live with this for now. I certainly would > >> prefer to have a 3rd opinion though, as I continue to feel uneasy > >> with this rather imprecise estimation (i.e. I'd much prefer a more > >> dynamic / on-demand approach). > > > > I agree it's not a perfect solution, but I think what's currently done > > is even worse, and we already had bug reports of users seeing Xen > > panic at PVH Dom0 build time if no dom0_mem parameter is specified. > > > > Would you be OK with using max_page then? > > I'm not going to say yes or no here without having seen a (qualified) > 3rd opinion. I would like to get this fixed. The current code for accounting the memory required for the paging structures is wrong, I've received several reports (most of them privately) of the dom0 builder running out of memory, thus leading to a panic. I'm open to suggestions. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
[parent not found: <812B19D1020000B00063616D@prv1-mh.provo.novell.com>]
* Re: [PATCH v2 1/2] x86/dom0: rename paging function [not found] ` <812B19D1020000B00063616D@prv1-mh.provo.novell.com> @ 2019-01-29 15:38 ` Jan Beulich 0 siblings, 0 replies; 33+ messages in thread From: Jan Beulich @ 2019-01-29 15:38 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 29.01.19 at 16:02, <roger.pau@citrix.com> wrote: > On Thu, Dec 13, 2018 at 07:47:59AM -0700, Jan Beulich wrote: >> >>> On 13.12.18 at 15:20, <roger.pau@citrix.com> wrote: >> > On Thu, Dec 13, 2018 at 03:17:05AM -0700, Jan Beulich wrote: >> >> >>> On 13.12.18 at 10:14, <roger.pau@citrix.com> wrote: >> >> > On Thu, Dec 13, 2018 at 12:45:07AM -0700, Jan Beulich wrote: >> >> >> >>> On 12.12.18 at 18:05, <roger.pau@citrix.com> wrote: >> >> >> > On Wed, Dec 12, 2018 at 09:15:09AM -0700, Jan Beulich wrote: >> >> >> >> The MMIO side of things of course still remains unclear. >> >> >> > >> >> >> > Right, for the MMIO and the handling of grant and foreign mappings it's >> >> >> > not clear how we want to proceed. >> >> >> > >> >> >> > Maybe account for all host RAM (total_pages) plus MMIO BARs? >> >> >> >> >> >> Well, I thought we've already settled on it being impossible to >> >> >> account for all MMIO BARs at this point. >> >> > >> >> > Well, I could iterate over all the registered PCI devices and size >> >> > the BARs (without VF BARs at least initially). This is quite >> >> > cumbersome, my other option would be using max_page and hope that >> >> > there are enough holes to make up for BAR MMIO regions. >> >> >> >> Well, maybe we could live with this for now. I certainly would >> >> prefer to have a 3rd opinion though, as I continue to feel uneasy >> >> with this rather imprecise estimation (i.e. I'd much prefer a more >> >> dynamic / on-demand approach). >> > >> > I agree it's not a perfect solution, but I think what's currently done >> > is even worse, and we already had bug reports of users seeing Xen >> > panic at PVH Dom0 build time if no dom0_mem parameter is specified. >> > >> > Would you be OK with using max_page then? >> >> I'm not going to say yes or no here without having seen a (qualified) >> 3rd opinion. > > I would like to get this fixed. The current code for accounting the > memory required for the paging structures is wrong, I've received > several reports (most of them privately) of the dom0 builder running > out of memory, thus leading to a panic. > > I'm open to suggestions. I'm afraid I'm the wrong addressee: I can't provide the asked for 3rd opinion. And if others think going from one often breaking solution to another, less often (but still) breaking one is a good idea, I won't block a change along those lines from going in. But it is kind of hard for me to understand why we would do such, instead of eliminating the problem altogether. Granted this may involve more intrusive a change. In any event PVH Dom0 is still experimental, and hence requiring people to use dom0_mem= for the time being does not seem overly unfriendly to me. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-05 14:54 [PATCH v2 0/2] x86/dom0: improve PVH paging memory calculation Roger Pau Monne 2018-12-05 14:54 ` [PATCH v2 1/2] x86/dom0: rename paging function Roger Pau Monne @ 2018-12-05 14:55 ` Roger Pau Monne 2018-12-06 12:42 ` Wei Liu 2018-12-11 15:19 ` Jan Beulich 1 sibling, 2 replies; 33+ messages in thread From: Roger Pau Monne @ 2018-12-05 14:55 UTC (permalink / raw) To: xen-devel Cc: Andrew Cooper, andrei.semenov, Wei Liu, Jan Beulich, Roger Pau Monne Current approximation of paging memory usage is based on the required amount when running in shadow mode and doesn't take into account the memory required by the IOMMU page tables. Fix this by introducing a function to calculate the amount of memory required by HAP/IOMMU page tables. The formula used to calculate such approximation is based on the pessimistic approach that each 4KB memory chunk will use 8 bytes of page table memory. Note that this approximation might need further tuning based on testing on different systems. Also fix the calculation of the required paging related memory in dom0_compute_nr_pages to take into account the paging implementation (shadow or HAP) and whether the IOMMU pages tables are shared with the HAP page tables. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: andrei.semenov@bertin.fr --- xen/arch/x86/dom0_build.c | 31 +++++++++++++++++++++++++++---- xen/arch/x86/hvm/dom0_build.c | 6 ++++-- xen/include/asm-x86/dom0_build.h | 2 ++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c index ba9aa85611..3a8e138f23 100644 --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -267,6 +267,25 @@ unsigned long __init dom0_shadow_pages(const struct domain *d, return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT); } +unsigned long __init dom0_hap_pages(const struct domain *d, + unsigned long nr_pages) +{ + /* + * Attempt to account for at least some of the MMIO regions by adding the + * size of the holes in the memory map to the amount of pages to map. Note + * this will obviously not account for MMIO regions that are past the last + * RAM range in the memory map. + */ + nr_pages += max_page - total_pages; + /* + * Approximate the memory required for the HAP/IOMMU page tables by + * pessimistically assuming each page will consume a 8 byte page table + * entry. + */ + return DIV_ROUND_UP(nr_pages * 8, PAGE_SIZE << PAGE_ORDER_4K); +} + + unsigned long __init dom0_compute_nr_pages( struct domain *d, struct elf_dom_parms *parms, unsigned long initrd_len) { @@ -294,8 +313,7 @@ unsigned long __init dom0_compute_nr_pages( avail -= max_pdx >> s; } - need_paging = is_hvm_domain(d) && - (!iommu_hap_pt_share || !paging_mode_hap(d)); + need_paging = is_hvm_domain(d); for ( ; ; need_paging = false ) { nr_pages = dom0_nrpages; @@ -324,8 +342,13 @@ unsigned long __init dom0_compute_nr_pages( if ( !need_paging ) break; - /* Reserve memory for shadow or HAP. */ - avail -= dom0_shadow_pages(d, nr_pages); + /* Reserve memory for CPU and IOMMU page tables. */ + if ( paging_mode_hap(d) ) + avail -= dom0_hap_pages(d, nr_pages) * + (iommu_hap_pt_share ? 1 : 2); + else + avail -= dom0_shadow_pages(d, nr_pages) + + dom0_hap_pages(d, nr_pages); } if ( is_pv_domain(d) && diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index 2af2bd8c3d..051e999957 100644 --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -358,6 +358,9 @@ static int __init pvh_setup_p2m(struct domain *d) { struct vcpu *v = d->vcpu[0]; unsigned long nr_pages = dom0_compute_nr_pages(d, NULL, 0); + unsigned long paging_pages = + paging_mode_hap(d) ? dom0_hap_pages(d, nr_pages) + : dom0_shadow_pages(d, nr_pages); unsigned int i; int rc; bool preempted; @@ -366,8 +369,7 @@ static int __init pvh_setup_p2m(struct domain *d) pvh_setup_e820(d, nr_pages); do { preempted = false; - paging_set_allocation(d, dom0_shadow_pages(d, nr_pages), - &preempted); + paging_set_allocation(d, paging_pages, &preempted); process_pending_softirqs(); } while ( preempted ); diff --git a/xen/include/asm-x86/dom0_build.h b/xen/include/asm-x86/dom0_build.h index 22f960b8b0..e1309c25e8 100644 --- a/xen/include/asm-x86/dom0_build.h +++ b/xen/include/asm-x86/dom0_build.h @@ -27,6 +27,8 @@ int dom0_construct_pvh(struct domain *d, const module_t *image, unsigned long dom0_shadow_pages(const struct domain *d, unsigned long nr_pages); +unsigned long dom0_hap_pages(const struct domain *d, + unsigned long nr_pages); void dom0_update_physmap(struct domain *d, unsigned long pfn, unsigned long mfn, unsigned long vphysmap_s); -- 2.19.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-05 14:55 ` [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations Roger Pau Monne @ 2018-12-06 12:42 ` Wei Liu 2018-12-10 10:33 ` Roger Pau Monné 2018-12-11 15:19 ` Jan Beulich 1 sibling, 1 reply; 33+ messages in thread From: Wei Liu @ 2018-12-06 12:42 UTC (permalink / raw) To: Roger Pau Monne Cc: xen-devel, andrei.semenov, Wei Liu, Jan Beulich, Andrew Cooper On Wed, Dec 05, 2018 at 03:55:00PM +0100, Roger Pau Monne wrote: > Current approximation of paging memory usage is based on the required > amount when running in shadow mode and doesn't take into account the > memory required by the IOMMU page tables. > > Fix this by introducing a function to calculate the amount of memory > required by HAP/IOMMU page tables. The formula used to calculate such > approximation is based on the pessimistic approach that each 4KB > memory chunk will use 8 bytes of page table memory. Note that this > approximation might need further tuning based on testing on different > systems. > > Also fix the calculation of the required paging related memory in > dom0_compute_nr_pages to take into account the paging implementation > (shadow or HAP) and whether the IOMMU pages tables are shared with the > HAP page tables. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Cc: andrei.semenov@bertin.fr > --- > xen/arch/x86/dom0_build.c | 31 +++++++++++++++++++++++++++---- > xen/arch/x86/hvm/dom0_build.c | 6 ++++-- > xen/include/asm-x86/dom0_build.h | 2 ++ > 3 files changed, 33 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c > index ba9aa85611..3a8e138f23 100644 > --- a/xen/arch/x86/dom0_build.c > +++ b/xen/arch/x86/dom0_build.c > @@ -267,6 +267,25 @@ unsigned long __init dom0_shadow_pages(const struct domain *d, > return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT); > } > > +unsigned long __init dom0_hap_pages(const struct domain *d, > + unsigned long nr_pages) > +{ > + /* > + * Attempt to account for at least some of the MMIO regions by adding the > + * size of the holes in the memory map to the amount of pages to map. Note > + * this will obviously not account for MMIO regions that are past the last > + * RAM range in the memory map. > + */ > + nr_pages += max_page - total_pages; Do those regions past end of RAM range show up in E820 map? If so, why not just sum up all reserved regions? Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-06 12:42 ` Wei Liu @ 2018-12-10 10:33 ` Roger Pau Monné 2018-12-11 12:17 ` Wei Liu 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-10 10:33 UTC (permalink / raw) To: Wei Liu; +Cc: xen-devel, andrei.semenov, Jan Beulich, Andrew Cooper On Thu, Dec 06, 2018 at 12:42:15PM +0000, Wei Liu wrote: > On Wed, Dec 05, 2018 at 03:55:00PM +0100, Roger Pau Monne wrote: > > Current approximation of paging memory usage is based on the required > > amount when running in shadow mode and doesn't take into account the > > memory required by the IOMMU page tables. > > > > Fix this by introducing a function to calculate the amount of memory > > required by HAP/IOMMU page tables. The formula used to calculate such > > approximation is based on the pessimistic approach that each 4KB > > memory chunk will use 8 bytes of page table memory. Note that this > > approximation might need further tuning based on testing on different > > systems. > > > > Also fix the calculation of the required paging related memory in > > dom0_compute_nr_pages to take into account the paging implementation > > (shadow or HAP) and whether the IOMMU pages tables are shared with the > > HAP page tables. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Cc: Jan Beulich <jbeulich@suse.com> > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > Cc: andrei.semenov@bertin.fr > > --- > > xen/arch/x86/dom0_build.c | 31 +++++++++++++++++++++++++++---- > > xen/arch/x86/hvm/dom0_build.c | 6 ++++-- > > xen/include/asm-x86/dom0_build.h | 2 ++ > > 3 files changed, 33 insertions(+), 6 deletions(-) > > > > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c > > index ba9aa85611..3a8e138f23 100644 > > --- a/xen/arch/x86/dom0_build.c > > +++ b/xen/arch/x86/dom0_build.c > > @@ -267,6 +267,25 @@ unsigned long __init dom0_shadow_pages(const struct domain *d, > > return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT); > > } > > > > +unsigned long __init dom0_hap_pages(const struct domain *d, > > + unsigned long nr_pages) > > +{ > > + /* > > + * Attempt to account for at least some of the MMIO regions by adding the > > + * size of the holes in the memory map to the amount of pages to map. Note > > + * this will obviously not account for MMIO regions that are past the last > > + * RAM range in the memory map. > > + */ > > + nr_pages += max_page - total_pages; > > Do those regions past end of RAM range show up in E820 map? No, BARs for example don't need to be in reserved regions. I've got one box with a 16GB Tesla card that has the 16GB BAR placed way past the last entry in the memory map, without any reserved region. So while this approach is not perfect, it's better than what we currently do, and we can always improve from there if it's clear what limitations we currently have. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-10 10:33 ` Roger Pau Monné @ 2018-12-11 12:17 ` Wei Liu 0 siblings, 0 replies; 33+ messages in thread From: Wei Liu @ 2018-12-11 12:17 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, andrei.semenov, Wei Liu, Jan Beulich, Andrew Cooper On Mon, Dec 10, 2018 at 11:33:28AM +0100, Roger Pau Monné wrote: > On Thu, Dec 06, 2018 at 12:42:15PM +0000, Wei Liu wrote: > > On Wed, Dec 05, 2018 at 03:55:00PM +0100, Roger Pau Monne wrote: > > > Current approximation of paging memory usage is based on the required > > > amount when running in shadow mode and doesn't take into account the > > > memory required by the IOMMU page tables. > > > > > > Fix this by introducing a function to calculate the amount of memory > > > required by HAP/IOMMU page tables. The formula used to calculate such > > > approximation is based on the pessimistic approach that each 4KB > > > memory chunk will use 8 bytes of page table memory. Note that this > > > approximation might need further tuning based on testing on different > > > systems. > > > > > > Also fix the calculation of the required paging related memory in > > > dom0_compute_nr_pages to take into account the paging implementation > > > (shadow or HAP) and whether the IOMMU pages tables are shared with the > > > HAP page tables. > > > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > --- > > > Cc: Jan Beulich <jbeulich@suse.com> > > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > > Cc: Wei Liu <wei.liu2@citrix.com> > > > Cc: andrei.semenov@bertin.fr > > > --- > > > xen/arch/x86/dom0_build.c | 31 +++++++++++++++++++++++++++---- > > > xen/arch/x86/hvm/dom0_build.c | 6 ++++-- > > > xen/include/asm-x86/dom0_build.h | 2 ++ > > > 3 files changed, 33 insertions(+), 6 deletions(-) > > > > > > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c > > > index ba9aa85611..3a8e138f23 100644 > > > --- a/xen/arch/x86/dom0_build.c > > > +++ b/xen/arch/x86/dom0_build.c > > > @@ -267,6 +267,25 @@ unsigned long __init dom0_shadow_pages(const struct domain *d, > > > return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT); > > > } > > > > > > +unsigned long __init dom0_hap_pages(const struct domain *d, > > > + unsigned long nr_pages) > > > +{ > > > + /* > > > + * Attempt to account for at least some of the MMIO regions by adding the > > > + * size of the holes in the memory map to the amount of pages to map. Note > > > + * this will obviously not account for MMIO regions that are past the last > > > + * RAM range in the memory map. > > > + */ > > > + nr_pages += max_page - total_pages; > > > > Do those regions past end of RAM range show up in E820 map? > > No, BARs for example don't need to be in reserved regions. I've got > one box with a 16GB Tesla card that has the 16GB BAR placed way past > the last entry in the memory map, without any reserved region. > > So while this approach is not perfect, it's better than what we > currently do, and we can always improve from there if it's clear what > limitations we currently have. Fair enough. Reviewed-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-05 14:55 ` [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations Roger Pau Monne 2018-12-06 12:42 ` Wei Liu @ 2018-12-11 15:19 ` Jan Beulich 2018-12-11 15:36 ` Roger Pau Monné 1 sibling, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-11 15:19 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 05.12.18 at 15:55, <roger.pau@citrix.com> wrote: > +unsigned long __init dom0_hap_pages(const struct domain *d, > + unsigned long nr_pages) > +{ > + /* > + * Attempt to account for at least some of the MMIO regions by adding the > + * size of the holes in the memory map to the amount of pages to map. Note > + * this will obviously not account for MMIO regions that are past the last > + * RAM range in the memory map. > + */ > + nr_pages += max_page - total_pages; > + /* > + * Approximate the memory required for the HAP/IOMMU page tables by > + * pessimistically assuming each page will consume a 8 byte page table > + * entry. > + */ > + return DIV_ROUND_UP(nr_pages * 8, PAGE_SIZE << PAGE_ORDER_4K); With enough memory handed to Dom0 the memory needed for L2 and higher page tables will matter as well. I'm anyway having difficulty seeing why HAP and shadow would have to use different calculations, the more that shadow relies on the same P2M code that shadow uses in the AMD/SVM case. Plus, as iirc was said by someone else already, I don't think we can (continue to) neglect the MMIO space needs for MMCFG and PCI devices, especially with devices having multi-Gb BARs. > +} > + > + No double blank lines please. > @@ -324,8 +342,13 @@ unsigned long __init dom0_compute_nr_pages( > if ( !need_paging ) > break; > > - /* Reserve memory for shadow or HAP. */ > - avail -= dom0_shadow_pages(d, nr_pages); > + /* Reserve memory for CPU and IOMMU page tables. */ > + if ( paging_mode_hap(d) ) > + avail -= dom0_hap_pages(d, nr_pages) * > + (iommu_hap_pt_share ? 1 : 2); Use "<< !iommu_hap_pt_share" instead? > + else > + avail -= dom0_shadow_pages(d, nr_pages) + > + dom0_hap_pages(d, nr_pages); > } Doesn't dom0_shadow_pages() (mean to) already include the amount needed for the P2M? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-11 15:19 ` Jan Beulich @ 2018-12-11 15:36 ` Roger Pau Monné 2018-12-11 16:21 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-11 15:36 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Tue, Dec 11, 2018 at 08:19:34AM -0700, Jan Beulich wrote: > >>> On 05.12.18 at 15:55, <roger.pau@citrix.com> wrote: > > +unsigned long __init dom0_hap_pages(const struct domain *d, > > + unsigned long nr_pages) > > +{ > > + /* > > + * Attempt to account for at least some of the MMIO regions by adding the > > + * size of the holes in the memory map to the amount of pages to map. Note > > + * this will obviously not account for MMIO regions that are past the last > > + * RAM range in the memory map. > > + */ > > + nr_pages += max_page - total_pages; > > + /* > > + * Approximate the memory required for the HAP/IOMMU page tables by > > + * pessimistically assuming each page will consume a 8 byte page table > > + * entry. > > + */ > > + return DIV_ROUND_UP(nr_pages * 8, PAGE_SIZE << PAGE_ORDER_4K); > > With enough memory handed to Dom0 the memory needed for > L2 and higher page tables will matter as well. The above calculation assumes all chunks will be mapped as 4KB entries, but this is very unlikely, so there's some room for higher page tables. If that doesn't seem enough I can add some extra space here, maybe a +5% or +10%? > I'm anyway having difficulty seeing why HAP and shadow would > have to use different calculations, the more that shadow relies > on the same P2M code that shadow uses in the AMD/SVM case. For once shadow needs to take the number of vCPUs into account while HAP doesn't. > Plus, as iirc was said by someone else already, I don't think we > can (continue to) neglect the MMIO space needs for MMCFG > and PCI devices, especially with devices having multi-Gb BARs. Well, there's the comment above that notes this approach only takes into account the holes in the memory map as regions to be mapped. This can be improved later on, but I think the important point here is to know where this numbers come from in order to tweak it in the future. > > + else > > + avail -= dom0_shadow_pages(d, nr_pages) + > > + dom0_hap_pages(d, nr_pages); > > } > > Doesn't dom0_shadow_pages() (mean to) already include the > amount needed for the P2M? libxl code mentions: "plus 1 page per MiB of RAM for the P2M map," so I guess the shadow calculation takes into account the memory used by the IOMMU page tables? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-11 15:36 ` Roger Pau Monné @ 2018-12-11 16:21 ` Jan Beulich 2018-12-12 9:37 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-11 16:21 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 11.12.18 at 16:36, <roger.pau@citrix.com> wrote: > On Tue, Dec 11, 2018 at 08:19:34AM -0700, Jan Beulich wrote: >> >>> On 05.12.18 at 15:55, <roger.pau@citrix.com> wrote: >> > +unsigned long __init dom0_hap_pages(const struct domain *d, >> > + unsigned long nr_pages) >> > +{ >> > + /* >> > + * Attempt to account for at least some of the MMIO regions by adding the >> > + * size of the holes in the memory map to the amount of pages to map. Note >> > + * this will obviously not account for MMIO regions that are past the last >> > + * RAM range in the memory map. >> > + */ >> > + nr_pages += max_page - total_pages; >> > + /* >> > + * Approximate the memory required for the HAP/IOMMU page tables by >> > + * pessimistically assuming each page will consume a 8 byte page table >> > + * entry. >> > + */ >> > + return DIV_ROUND_UP(nr_pages * 8, PAGE_SIZE << PAGE_ORDER_4K); >> >> With enough memory handed to Dom0 the memory needed for >> L2 and higher page tables will matter as well. > > The above calculation assumes all chunks will be mapped as 4KB > entries, but this is very unlikely, so there's some room for higher > page tables. Right, but there's no dependency on 2M and/or 1G pages being available, nor does the comment give any hint towards that implication. > If that doesn't seem enough I can add some extra space > here, maybe a +5% or +10%? A percentage doesn't do imo. From the memory map it should be clear how many L2, L3, and L4 tables are going to be needed. We do such a calculation in the PV case as well, after all. >> I'm anyway having difficulty seeing why HAP and shadow would >> have to use different calculations, the more that shadow relies >> on the same P2M code that shadow uses in the AMD/SVM case. > > For once shadow needs to take the number of vCPUs into account while > HAP doesn't. Yes, and as said - adding that shadow-specific amount on top of the generic calculation would seem better to me. >> Plus, as iirc was said by someone else already, I don't think we >> can (continue to) neglect the MMIO space needs for MMCFG >> and PCI devices, especially with devices having multi-Gb BARs. > > Well, there's the comment above that notes this approach only takes > into account the holes in the memory map as regions to be mapped. This > can be improved later on, but I think the important point here is to > know where this numbers come from in order to tweak it in the future. You've given this same argument to Wei before. I agree the calculation adjustments are an improvement even without taking that other aspect into consideration, but I'm not happy to see an important portion left out. What if the sum of all BARs exceeds the amount of RAM? What if enough BARs are so undesirably placed that every one of them needs a full separate chain of L4, L3, L2, and L1 entries? >> > + else >> > + avail -= dom0_shadow_pages(d, nr_pages) + >> > + dom0_hap_pages(d, nr_pages); >> > } >> >> Doesn't dom0_shadow_pages() (mean to) already include the >> amount needed for the P2M? > > libxl code mentions: "plus 1 page per MiB of RAM for the P2M map," so > I guess the shadow calculation takes into account the memory used by > the IOMMU page tables? I think that comment refers to the P2M needs, not the IOMMU ones. Iirc in shadow mode the IOMMU uses separate page tables, albeit I don't recall why that is when the P2M is really only used by software in that case. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-11 16:21 ` Jan Beulich @ 2018-12-12 9:37 ` Roger Pau Monné 2018-12-12 9:59 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 9:37 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Tue, Dec 11, 2018 at 09:21:29AM -0700, Jan Beulich wrote: > >>> On 11.12.18 at 16:36, <roger.pau@citrix.com> wrote: > > On Tue, Dec 11, 2018 at 08:19:34AM -0700, Jan Beulich wrote: > >> >>> On 05.12.18 at 15:55, <roger.pau@citrix.com> wrote: > >> > +unsigned long __init dom0_hap_pages(const struct domain *d, > >> > + unsigned long nr_pages) > >> > +{ > >> > + /* > >> > + * Attempt to account for at least some of the MMIO regions by adding the > >> > + * size of the holes in the memory map to the amount of pages to map. Note > >> > + * this will obviously not account for MMIO regions that are past the last > >> > + * RAM range in the memory map. > >> > + */ > >> > + nr_pages += max_page - total_pages; > >> > + /* > >> > + * Approximate the memory required for the HAP/IOMMU page tables by > >> > + * pessimistically assuming each page will consume a 8 byte page table > >> > + * entry. > >> > + */ > >> > + return DIV_ROUND_UP(nr_pages * 8, PAGE_SIZE << PAGE_ORDER_4K); > >> > >> With enough memory handed to Dom0 the memory needed for > >> L2 and higher page tables will matter as well. > > > > The above calculation assumes all chunks will be mapped as 4KB > > entries, but this is very unlikely, so there's some room for higher > > page tables. > > Right, but there's no dependency on 2M and/or 1G pages being > available, nor does the comment give any hint towards that > implication. > > > If that doesn't seem enough I can add some extra space > > here, maybe a +5% or +10%? > > A percentage doesn't do imo. From the memory map it should > be clear how many L2, L3, and L4 tables are going to be needed. > We do such a calculation in the PV case as well, after all. As replied on patch 1, I'm planning to use the same calculations used by shadow, which assume 1 page is needed for the p2m for each 1MB guest memory. > >> I'm anyway having difficulty seeing why HAP and shadow would > >> have to use different calculations, the more that shadow relies > >> on the same P2M code that shadow uses in the AMD/SVM case. > > > > For once shadow needs to take the number of vCPUs into account while > > HAP doesn't. > > Yes, and as said - adding that shadow-specific amount on top of > the generic calculation would seem better to me. > > >> Plus, as iirc was said by someone else already, I don't think we > >> can (continue to) neglect the MMIO space needs for MMCFG > >> and PCI devices, especially with devices having multi-Gb BARs. > > > > Well, there's the comment above that notes this approach only takes > > into account the holes in the memory map as regions to be mapped. This > > can be improved later on, but I think the important point here is to > > know where this numbers come from in order to tweak it in the future. > > You've given this same argument to Wei before. I agree the > calculation adjustments are an improvement even without > taking that other aspect into consideration, but I'm not happy > to see an important portion left out. What if the sum of all > BARs exceeds the amount of RAM? What if enough BARs are > so undesirably placed that every one of them needs a full > separate chain of L4, L3, L2, and L1 entries? OK, I will iterate over all the devices in order to size the BARs, and then add the sum of BARs MMIO regions to the amount of guest memory, so that each 1MB of BAR MMIO will require 1 page for the p2m. Note that ATM I will not account for VF BARs. > >> > + else > >> > + avail -= dom0_shadow_pages(d, nr_pages) + > >> > + dom0_hap_pages(d, nr_pages); > >> > } > >> > >> Doesn't dom0_shadow_pages() (mean to) already include the > >> amount needed for the P2M? > > > > libxl code mentions: "plus 1 page per MiB of RAM for the P2M map," so > > I guess the shadow calculation takes into account the memory used by > > the IOMMU page tables? > > I think that comment refers to the P2M needs, not the IOMMU ones. > Iirc in shadow mode the IOMMU uses separate page tables, albeit I > don't recall why that is when the P2M is really only used by software > in that case. OK, so for shadow we also need to account for the IOMMU page table size, which is not done now AFAICT. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-12 9:37 ` Roger Pau Monné @ 2018-12-12 9:59 ` Jan Beulich 2018-12-12 10:16 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-12 9:59 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 10:37, <roger.pau@citrix.com> wrote: > OK, I will iterate over all the devices in order to size the BARs, and > then add the sum of BARs MMIO regions to the amount of guest memory, > so that each 1MB of BAR MMIO will require 1 page for the p2m. Don't you construct a rangeset for all of the BARs already anyway? For ones which have an address assigned and are enabled, also taking the address into account would seem desirable, as then you could do with less than double over- estimating (in the common case) the amount of space needed. > Note that ATM I will not account for VF BARs. Hmm, yes, this is perhaps indeed too much to ask for at this point. But ultimately we need a clean approach there too. > OK, so for shadow we also need to account for the IOMMU page table > size, which is not done now AFAICT. As said (or perhaps implied) in the other reply, the calculation there may imply half of the space for the IOMMU, but it may also mean the other half to be for higher level tables on the CPU side. It's all guesswork without suitable comments. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-12 9:59 ` Jan Beulich @ 2018-12-12 10:16 ` Roger Pau Monné 2018-12-12 10:57 ` Jan Beulich 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 10:16 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Wed, Dec 12, 2018 at 02:59:42AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 10:37, <roger.pau@citrix.com> wrote: > > OK, I will iterate over all the devices in order to size the BARs, and > > then add the sum of BARs MMIO regions to the amount of guest memory, > > so that each 1MB of BAR MMIO will require 1 page for the p2m. > > Don't you construct a rangeset for all of the BARs already > anyway? For ones which have an address assigned and are > enabled, also taking the address into account would seem > desirable, as then you could do with less than double over- > estimating (in the common case) the amount of space needed. That's a chicken and egg problem, the current accounting of BAR sizes is done in init_bars, which will also setup the identity p2m mappings, so the call to paging_set_allocation (and thus the amount of memory required by the p2m) needs to happen before mapping any BARs. > > Note that ATM I will not account for VF BARs. > > Hmm, yes, this is perhaps indeed too much to ask for at this > point. But ultimately we need a clean approach there too. Well, since there's no support for SR-IOV for PVH Dom0 ATM I guess I could add this once it's supported. It's not overly complicated, at the end is just BAR sizings at a different position in the config space. > > OK, so for shadow we also need to account for the IOMMU page table > > size, which is not done now AFAICT. > > As said (or perhaps implied) in the other reply, the calculation > there may imply half of the space for the IOMMU, but it may > also mean the other half to be for higher level tables on the > CPU side. It's all guesswork without suitable comments. My guess was that the other half of the space was supposed to be used by higher level tables, and that the accounting for IOMMU page tables is completely missing. Note that on PV there's no accounting for IOMMU page tables either AFAICT. There are also further issues that I wanted to discuss in a separate thread, what about foreign mappings? Dom0 will likely map a non trivial amount of grants and foreign mappings, which will also require p2m/IOMMU page table entries. Should we maybe size Dom0 p2m/iommu internal paging structures to be able to map up to max_page at least? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-12 10:16 ` Roger Pau Monné @ 2018-12-12 10:57 ` Jan Beulich 2018-12-12 11:14 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Jan Beulich @ 2018-12-12 10:57 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel >>> On 12.12.18 at 11:16, <roger.pau@citrix.com> wrote: > There are also further issues that I wanted to discuss in a separate > thread, what about foreign mappings? Dom0 will likely map a non > trivial amount of grants and foreign mappings, which will also require > p2m/IOMMU page table entries. Hmm, good point. Then again this is a runtime requirement, whereas here we want to get the boot time estimate right. At runtime lack of memory for P2M tables will simply result in -ENOMEM. > Should we maybe size Dom0 p2m/iommu internal paging structures to be > able to map up to max_page at least? Well, max_page is a gross over-estimate of RAM (especially with dom0_mem= in effect) and doesn't help at all with MMIO or the foreign/grant maps you mention. I wonder whether for Dom0 we don't need to change the entire approach of how we set it up in PVH mode: Instead of a single paging_set_allocation(), why don't we call the function repeatedly whenever we run out of space, shrinking what we actually give to Dom0 accordingly (and incrementally). For the PCI BAR mappings this would require doing so when the Dom0 kernel is already running, but I think that's acceptable. PCI device add would fail with -ENOMEM when the allocation pools can't be suitably grown. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-12 10:57 ` Jan Beulich @ 2018-12-12 11:14 ` Roger Pau Monné 2018-12-12 11:19 ` Roger Pau Monné 0 siblings, 1 reply; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 11:14 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, andrei.semenov, Wei Liu, xen-devel On Wed, Dec 12, 2018 at 03:57:41AM -0700, Jan Beulich wrote: > >>> On 12.12.18 at 11:16, <roger.pau@citrix.com> wrote: > > There are also further issues that I wanted to discuss in a separate > > thread, what about foreign mappings? Dom0 will likely map a non > > trivial amount of grants and foreign mappings, which will also require > > p2m/IOMMU page table entries. > > Hmm, good point. Then again this is a runtime requirement, > whereas here we want to get the boot time estimate right. At > runtime lack of memory for P2M tables will simply result in > -ENOMEM. But Xen runtime memory is also tied to the boot estimates if there's no dom0_mem parameter specified on the command line. I would expect Dom0 to balloon down memory when it attempts to map BARs, even at runtime. > > Should we maybe size Dom0 p2m/iommu internal paging structures to be > > able to map up to max_page at least? > > Well, max_page is a gross over-estimate of RAM (especially with > dom0_mem= in effect) and doesn't help at all with MMIO or the > foreign/grant maps you mention. > > I wonder whether for Dom0 we don't need to change the entire > approach of how we set it up in PVH mode: Instead of a single > paging_set_allocation(), why don't we call the function > repeatedly whenever we run out of space, shrinking what we > actually give to Dom0 accordingly (and incrementally). This could work given a suitable dom0_mem value is specified at the command line. Without the Dom0 amount of memory being assigned by the admin, Xen still needs to estimate how much memory is needed for it's internal structures (p2m, IOMMU page tables) and we are back to the same scenario. > For the > PCI BAR mappings this would require doing so when the Dom0 > kernel is already running, but I think that's acceptable. PCI > device add would fail with -ENOMEM when the allocation pools > can't be suitably grown. The default Xen free slack memory is 128MB, which I'm afraid would be consumed quite easily. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations 2018-12-12 11:14 ` Roger Pau Monné @ 2018-12-12 11:19 ` Roger Pau Monné 0 siblings, 0 replies; 33+ messages in thread From: Roger Pau Monné @ 2018-12-12 11:19 UTC (permalink / raw) To: Roger Pau Monné Cc: Andrew Cooper, andrei.semenov, Wei Liu, Jan Beulich, xen-devel On Wed, Dec 12, 2018 at 12:14:53PM +0100, Roger Pau Monné wrote: > On Wed, Dec 12, 2018 at 03:57:41AM -0700, Jan Beulich wrote: > > >>> On 12.12.18 at 11:16, <roger.pau@citrix.com> wrote: > > > There are also further issues that I wanted to discuss in a separate > > > thread, what about foreign mappings? Dom0 will likely map a non > > > trivial amount of grants and foreign mappings, which will also require > > > p2m/IOMMU page table entries. > > > > Hmm, good point. Then again this is a runtime requirement, > > whereas here we want to get the boot time estimate right. At > > runtime lack of memory for P2M tables will simply result in > > -ENOMEM. > > But Xen runtime memory is also tied to the boot estimates if there's > no dom0_mem parameter specified on the command line. I would expect ^ wouldn't > Dom0 to balloon down memory when it attempts to map BARs, even at > runtime. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2019-01-29 15:38 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-05 14:54 [PATCH v2 0/2] x86/dom0: improve PVH paging memory calculation Roger Pau Monne 2018-12-05 14:54 ` [PATCH v2 1/2] x86/dom0: rename paging function Roger Pau Monne 2018-12-06 12:31 ` Wei Liu 2018-12-11 15:08 ` Jan Beulich 2018-12-11 15:19 ` Roger Pau Monné 2018-12-11 15:33 ` Jan Beulich 2018-12-12 9:14 ` Roger Pau Monné 2018-12-12 9:53 ` Jan Beulich 2018-12-12 10:04 ` Roger Pau Monné 2018-12-12 10:32 ` Jan Beulich 2018-12-12 15:56 ` Roger Pau Monné 2018-12-12 16:15 ` Jan Beulich 2018-12-12 17:05 ` Roger Pau Monné [not found] ` <3F7E1F6E020000A10063616D@prv1-mh.provo.novell.com> 2018-12-13 7:45 ` Jan Beulich 2018-12-13 9:14 ` Roger Pau Monné [not found] ` <12305AED020000300063616D@prv1-mh.provo.novell.com> 2018-12-13 10:17 ` Jan Beulich 2018-12-13 14:20 ` Roger Pau Monné [not found] ` <7320EEF8020000C00063616D@prv1-mh.provo.novell.com> 2018-12-13 14:47 ` Jan Beulich 2019-01-29 15:02 ` Roger Pau Monné [not found] ` <812B19D1020000B00063616D@prv1-mh.provo.novell.com> 2019-01-29 15:38 ` Jan Beulich 2018-12-05 14:55 ` [PATCH v2 2/2] x86/dom0: improve paging memory usage calculations Roger Pau Monne 2018-12-06 12:42 ` Wei Liu 2018-12-10 10:33 ` Roger Pau Monné 2018-12-11 12:17 ` Wei Liu 2018-12-11 15:19 ` Jan Beulich 2018-12-11 15:36 ` Roger Pau Monné 2018-12-11 16:21 ` Jan Beulich 2018-12-12 9:37 ` Roger Pau Monné 2018-12-12 9:59 ` Jan Beulich 2018-12-12 10:16 ` Roger Pau Monné 2018-12-12 10:57 ` Jan Beulich 2018-12-12 11:14 ` Roger Pau Monné 2018-12-12 11:19 ` Roger Pau Monné
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.