Re: [Xen-devel] [PATCH] x86/Dom0: account for shadow/HAP allocation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.02.15 at 13:02, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 26/02/15 07:43, Jan Beulich wrote:
>>>>> On 25.02.15 at 18:06, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 25/02/15 14:45, Jan Beulich wrote:
>>>> +static unsigned long __init dom0_paging_pages(const struct domain *d,
>>>> +                                              unsigned long nr_pages)
>>>> +{
>>>> +    /* Copied from: libxl_get_required_shadow_memory() */
>>>> +    unsigned long memkb = nr_pages * (PAGE_SIZE / 1024);
>>>> +
>>>> +    memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
>>> I have recently raised a bug against Xapi for similar wrong logic when
>>> calculating the size of the shadow pool.
>>>
>>> A per-vcpu reservation of shadow allocation is only needed if shadow
>>> paging is actually in use, and even then should match
>>> shadow_min_acceptable_pages() at 128 pages per vcpu.
>>>
>>> If HAP is in use, the only allocations from the shadow pool are for the
>>> EPT/NPT tables (1% of nr_pages), IOMMU tables (another 1% of nr_pages if
>>> in use), and the logdirty radix tree (substantially less than than 1% of
>>> nr_pages).
>>>
>>> One could argue that structure such as the vmcs/vmcb should have their
>>> allocations accounted against the domain, in which case a small per-vcpu
>>> component would be appropriate.
>>>
>>> However as it currently stands, this calculation wastes 4MB of ram per
>>> vcpu in shadow allocation which is not going to be used.
>> But you realize that the functional change here explicitly only covers
>> the shadow case - the PVH (i.e. HAP) case is effectively unchanged
>> (merely correcting the mistake of not accounting for what gets
>> actually allocated), and I don't intend any functional change for PVH
>> (other than said bug fix) with this patch.
> 
> Ok
> 
>> Hence correcting this (i.e.
>> lowering the accounted for as well as the allocated amount) as well
>> as adding accounting for VMCS/VMCB (just like we account for
>> struct vcpu) should be the subject of a separate patch, presumably
>> by someone actively working on PVH (and then perhaps at once for
>> libxc). I also think that this calculation would better become a paging
>> variant specific hook if calculations differ between shadow and HAP.
> 
> That would be better, in the longrun.

Taking this together, can I read this as an ack then?

Jan


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