Re: [Xen-devel] [PATCH] x86/pvh: fix memory accounting for Dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.17 at 17:55, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Sep 28, 2017 at 03:39:08PM +0000, Roger Pau Monné wrote:
>> On Thu, Sep 28, 2017 at 01:18:55PM +0000, Jan Beulich wrote:
>> > >>> On 28.09.17 at 12:16, <roger.pau@xxxxxxxxxx> wrote:
>> > > Make sure that the memory for the paging structures in case of a HVM
>> > > Dom0 is subtracted from the total amount of memory available for Dom0
>> > > to use. Also take into account whether the IOMMU is sharing the
>> > > page tables with HAP, or else also reserve some memory for the IOMMU
>> > > page tables.
>> > > 
>> > > While there re-organize the code slightly so that the for loop and the
>> > > need_paging local variable can be removed.
>> > 
>> > These two things very definitely should not be merged into a single
>> > patch; I'm not convinced the reorg is correct in the first place. Note
>> > how avail, which is being changed in the first iteration of the loop,
>> > feeds back into the second iteration.
>> 
>> I'm afraid I don't understand why this is done. Also, the second loop
>> is only going to happen when need_paging is true, which only happens
>> for HVM guests using shadow or without shared pt with the IOMMU.
> 
> OK I think I'm starting to understand this. The need_paging thing it's
> only done if the page tables are not shared because the iommu_enabled
> has already reserved some memory for the page tables, that can be
> shared with EPT. I think this is all very confusing, and the
> calculations done for the iommu_enabled case are wrong. Xen should use
> dom0_paging_pages instead.

You need to look at the history - the IOMMU part was there before
the paging portion, I think. And yes, as indicated in an earlier reply,
removing the IOMMU part in favor of using dom0_paging_pages()
(and doubling the value when not sharing page tables) would seem
preferable.

> I still don't understand the need for the 'for' loop.

The second iteration of the loop uses a shrunk "avail" resulting
from the first iteration, to make sure the remaining amount of
memory comes reasonably close to what was requested on the
command line. This matters specifically when quite a bit of
memory is needed for the page tables.

Jan

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