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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/02/15 13:21, Jan Beulich wrote:
>>>> 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?

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

>
> Jan
>


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