All of lore.kernel.org
 help / color / mirror / Atom feed
* PVH dom0 memory setup
@ 2017-09-08 14:56 Boris Ostrovsky
  2017-09-08 17:11 ` Roger Pau Monné
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2017-09-08 14:56 UTC (permalink / raw)
  To: Roger Pau Monné, Andrew Cooper, Jan Beulich; +Cc: xen-devel

I am slightly confused by the use of 'need_paging' variable in
dom0_compute_nr_pages().

Because paging_mode_hap() and iommu_hap_pt_share are (almost?) always
true, we are not reducing available memory for PVH dom0 by page tables
size. But then in pvh_setup_p2m() we do use this memory by
paging_set_allocation(). And from what I've seen we then may run our of
heap pages when populating memory map (in the 'for' loop below).

Am I not reading this correctly?

Thanks.
-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PVH dom0 memory setup
  2017-09-08 14:56 PVH dom0 memory setup Boris Ostrovsky
@ 2017-09-08 17:11 ` Roger Pau Monné
  2017-09-08 19:04   ` Boris Ostrovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Roger Pau Monné @ 2017-09-08 17:11 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: Andrew Cooper, Jan Beulich, xen-devel

On Fri, Sep 08, 2017 at 10:56:33AM -0400, Boris Ostrovsky wrote:
> I am slightly confused by the use of 'need_paging' variable in
> dom0_compute_nr_pages().
> 
> Because paging_mode_hap() and iommu_hap_pt_share are (almost?) always
> true, we are not reducing available memory for PVH dom0 by page tables
> size. But then in pvh_setup_p2m() we do use this memory by
> paging_set_allocation(). And from what I've seen we then may run our of
> heap pages when populating memory map (in the 'for' loop below).
> 
> Am I not reading this correctly?

Yes, I think you are reading this correctly. dom0_compute_nr_pages
should set need_paging if the domain type is hvm AFAICT, because hap
also consumes memory for it's page tables. Do you have a reliable way
to trigger this?

I was thinking of a fix along the lines of:

---8<---
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index f616b99ddc..424192a7c4 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -263,8 +263,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;


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: PVH dom0 memory setup
  2017-09-08 17:11 ` Roger Pau Monné
@ 2017-09-08 19:04   ` Boris Ostrovsky
  2017-09-11  8:49     ` Roger Pau Monné
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2017-09-08 19:04 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: Andrew Cooper, Jan Beulich, xen-devel

On 09/08/2017 01:11 PM, Roger Pau Monné wrote:
> On Fri, Sep 08, 2017 at 10:56:33AM -0400, Boris Ostrovsky wrote:
>> I am slightly confused by the use of 'need_paging' variable in
>> dom0_compute_nr_pages().
>>
>> Because paging_mode_hap() and iommu_hap_pt_share are (almost?) always
>> true, we are not reducing available memory for PVH dom0 by page tables
>> size. But then in pvh_setup_p2m() we do use this memory by
>> paging_set_allocation(). And from what I've seen we then may run our of
>> heap pages when populating memory map (in the 'for' loop below).
>>
>> Am I not reading this correctly?
> Yes, I think you are reading this correctly. dom0_compute_nr_pages
> should set need_paging if the domain type is hvm AFAICT, because hap
> also consumes memory for it's page tables. Do you have a reliable way
> to trigger this?
>
> I was thinking of a fix along the lines of:

Yes, this is essentially what I ended up doing and yes it does fix this
problem.

I wasn't sure whether this would work for !HAP case (which we still
support).

-boris


>
> ---8<---
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index f616b99ddc..424192a7c4 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -263,8 +263,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;
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PVH dom0 memory setup
  2017-09-08 19:04   ` Boris Ostrovsky
@ 2017-09-11  8:49     ` Roger Pau Monné
  2017-09-11  9:35       ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Roger Pau Monné @ 2017-09-11  8:49 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: Andrew Cooper, Jan Beulich, xen-devel

On Fri, Sep 08, 2017 at 03:04:30PM -0400, Boris Ostrovsky wrote:
> On 09/08/2017 01:11 PM, Roger Pau Monné wrote:
> > On Fri, Sep 08, 2017 at 10:56:33AM -0400, Boris Ostrovsky wrote:
> >> I am slightly confused by the use of 'need_paging' variable in
> >> dom0_compute_nr_pages().
> >>
> >> Because paging_mode_hap() and iommu_hap_pt_share are (almost?) always
> >> true, we are not reducing available memory for PVH dom0 by page tables
> >> size. But then in pvh_setup_p2m() we do use this memory by
> >> paging_set_allocation(). And from what I've seen we then may run our of
> >> heap pages when populating memory map (in the 'for' loop below).
> >>
> >> Am I not reading this correctly?
> > Yes, I think you are reading this correctly. dom0_compute_nr_pages
> > should set need_paging if the domain type is hvm AFAICT, because hap
> > also consumes memory for it's page tables. Do you have a reliable way
> > to trigger this?
> >
> > I was thinking of a fix along the lines of:
> 
> Yes, this is essentially what I ended up doing and yes it does fix this
> problem.
> 
> I wasn't sure whether this would work for !HAP case (which we still
> support).

It should work fine in both cases, but we should check whether the
calculation of the reservation is done in the same way as libxl.

AFAICT even with the change proposed I don't think
dom0_compute_nr_pages will handle the !iommu_hap_pt_share case
properly (ie: I don't see specific memory for the IOMMU pages tables
being reserved anywhere).

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PVH dom0 memory setup
  2017-09-11  8:49     ` Roger Pau Monné
@ 2017-09-11  9:35       ` Jan Beulich
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2017-09-11  9:35 UTC (permalink / raw)
  To: Roger Pau Monné, Boris Ostrovsky; +Cc: Andrew Cooper, xen-devel

>>> On 11.09.17 at 10:49, <roger.pau@citrix.com> wrote:
> AFAICT even with the change proposed I don't think
> dom0_compute_nr_pages will handle the !iommu_hap_pt_share case
> properly (ie: I don't see specific memory for the IOMMU pages tables
> being reserved anywhere).

Which (sadly) is in line with what the tool stack does (or really fails
to do), despite me having pointed this out a few times before.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-09-11  9:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08 14:56 PVH dom0 memory setup Boris Ostrovsky
2017-09-08 17:11 ` Roger Pau Monné
2017-09-08 19:04   ` Boris Ostrovsky
2017-09-11  8:49     ` Roger Pau Monné
2017-09-11  9:35       ` Jan Beulich

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.