| Message ID | 20170927141608.73485-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 27.09.17 at 16:16, <roger.pau@citrix.com> wrote: > --- 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; Still in context above is the calculation for IOMMU page tables When iommu_hap_pt_share, why do we need to reserve extra space? If the IOMMU calculation isn't precise enough, perhaps that's what wants changing? Quite possibly it would suffice to simply double the value dom0_paging_pages() returns in that case. Jan
On Wed, Sep 27, 2017 at 02:26:37PM +0000, Jan Beulich wrote: > >>> On 27.09.17 at 16:16, <roger.pau@citrix.com> wrote: > > --- 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; > > Still in context above is the calculation for IOMMU page tables > When iommu_hap_pt_share, why do we need to reserve extra > space? If the IOMMU calculation isn't precise enough, perhaps > that's what wants changing? Quite possibly it would suffice to > simply double the value dom0_paging_pages() returns in that > case. I have not seen this issue myself, perhaps Boris can provide a little more context on how to trigger this, so that the cause can be narrowed down. Thanks, Roger.
On 09/27/2017 11:10 AM, Roger Pau Monné wrote: > On Wed, Sep 27, 2017 at 02:26:37PM +0000, Jan Beulich wrote: >>>>> On 27.09.17 at 16:16, <roger.pau@citrix.com> wrote: >>> --- 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; >> Still in context above is the calculation for IOMMU page tables >> When iommu_hap_pt_share, why do we need to reserve extra >> space? If the IOMMU calculation isn't precise enough, perhaps >> that's what wants changing? Quite possibly it would suffice to >> simply double the value dom0_paging_pages() returns in that >> case. > I have not seen this issue myself, perhaps Boris can provide a little > more context on how to trigger this, so that the cause can be narrowed > down. I could only trigger this on one of my machines but essentially the problem was that we reserved memory for page tables (pvh_setup_p2m()->paging_set_allocation()) and this reservation was not accounted for when trying to populate dom0's e820. -boris
>>> On 27.09.17 at 19:02, <boris.ostrovsky@oracle.com> wrote: > On 09/27/2017 11:10 AM, Roger Pau Monné wrote: >> On Wed, Sep 27, 2017 at 02:26:37PM +0000, Jan Beulich wrote: >>>>>> On 27.09.17 at 16:16, <roger.pau@citrix.com> wrote: >>>> --- 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; >>> Still in context above is the calculation for IOMMU page tables >>> When iommu_hap_pt_share, why do we need to reserve extra >>> space? If the IOMMU calculation isn't precise enough, perhaps >>> that's what wants changing? Quite possibly it would suffice to >>> simply double the value dom0_paging_pages() returns in that >>> case. >> I have not seen this issue myself, perhaps Boris can provide a little >> more context on how to trigger this, so that the cause can be narrowed >> down. > > I could only trigger this on one of my machines but essentially the > problem was that we reserved memory for page tables > (pvh_setup_p2m()->paging_set_allocation()) and this reservation was not > accounted for when trying to populate dom0's e820. Hmm, but that's a problem which can't be resolved by changing the calculation of how much memory to reserve, I would think. I'm afraid I'm confused. Jan
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;
PVHv2 is always going to require the usage of memory in order to store the p2m page tables, either when using hap or shadow. Fix the condition so memory is reserved unconditionally when trying to build a PVHv2 Dom0. Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/dom0_build.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)