[PATCH] x86/Dom0: account for shadow/HAP allocation

[PATCH] x86/Dom0: account for shadow/HAP allocation

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 4370 bytes --]

... when calculating how many pages to allocate fopr Dom0. This is
basically equivalent to the already present IOMMU related adjustment.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -132,6 +132,8 @@ struct vcpu *__init alloc_dom0_vcpu0(str
 #ifdef CONFIG_SHADOW_PAGING
 static bool_t __initdata opt_dom0_shadow;
 boolean_param("dom0_shadow", opt_dom0_shadow);
+#else
+#define opt_dom0_shadow 0
 #endif
 
 static char __initdata opt_dom0_ioports_disable[200] = "";
@@ -203,13 +205,23 @@ static struct page_info * __init alloc_c
     return page;
 }
 
+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));
+
+    return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
+}
+
 static unsigned long __init compute_dom0_nr_pages(
     struct domain *d, struct elf_dom_parms *parms, unsigned long initrd_len)
 {
     unsigned long avail = avail_domheap_pages() + initial_images_nrpages();
-    unsigned long nr_pages = dom0_nrpages;
-    unsigned long min_pages = dom0_min_nrpages;
-    unsigned long max_pages = dom0_max_nrpages;
+    unsigned long nr_pages, min_pages, max_pages;
+    bool_t need_paging;
 
     /* Reserve memory for further dom0 vcpu-struct allocations... */
     avail -= (d->max_vcpus - 1UL)
@@ -227,23 +239,37 @@ static unsigned long __init compute_dom0
             avail -= max_pdx >> s;
     }
 
-    /*
-     * If domain 0 allocation isn't specified, reserve 1/16th of available
-     * memory for things like DMA buffers. This reservation is clamped to 
-     * a maximum of 128MB.
-     */
-    if ( nr_pages == 0 )
-        nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
+    need_paging = opt_dom0_shadow || (is_pvh_domain(d) && !iommu_hap_pt_share);
+    for ( ; ; need_paging = 0 )
+    {
+        nr_pages = dom0_nrpages;
+        min_pages = dom0_min_nrpages;
+        max_pages = dom0_max_nrpages;
+
+        /*
+         * If allocation isn't specified, reserve 1/16th of available memory
+         * for things like DMA buffers. This reservation is clamped to a
+         * maximum of 128MB.
+         */
+        if ( nr_pages == 0 )
+            nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
 
-    /* Negative memory specification means "all memory - specified amount". */
-    if ( (long)nr_pages  < 0 ) nr_pages  += avail;
-    if ( (long)min_pages < 0 ) min_pages += avail;
-    if ( (long)max_pages < 0 ) max_pages += avail;
-
-    /* Clamp dom0 memory according to min/max limits and available memory. */
-    nr_pages = max(nr_pages, min_pages);
-    nr_pages = min(nr_pages, max_pages);
-    nr_pages = min(nr_pages, avail);
+        /* Negative specification means "all memory - specified amount". */
+        if ( (long)nr_pages  < 0 ) nr_pages  += avail;
+        if ( (long)min_pages < 0 ) min_pages += avail;
+        if ( (long)max_pages < 0 ) max_pages += avail;
+
+        /* Clamp according to min/max limits and available memory. */
+        nr_pages = max(nr_pages, min_pages);
+        nr_pages = min(nr_pages, max_pages);
+        nr_pages = min(nr_pages, avail);
+
+        if ( !need_paging )
+            break;
+
+        /* Reserve memory for shadow or HAP. */
+        avail -= dom0_paging_pages(d, nr_pages);
+    }
 
     if ( (parms->p2m_base == UNSET_ADDR) && (dom0_nrpages <= 0) &&
          ((dom0_min_nrpages <= 0) || (nr_pages > min_pages)) )
@@ -1287,14 +1313,7 @@ int __init construct_dom0(
     }
 
     if ( is_pvh_domain(d) )
-    {
-        unsigned long hap_pages, memkb = nr_pages * (PAGE_SIZE / 1024);
-
-        /* Copied from: libxl_get_required_shadow_memory() */
-        memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
-        hap_pages = ( (memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
-        hap_set_alloc_for_pvh_dom0(d, hap_pages);
-    }
+        hap_set_alloc_for_pvh_dom0(d, dom0_paging_pages(d, nr_pages));
 
     /*
      * We enable paging mode again so guest_physmap_add_page will do the



[-- Attachment #2: x86-Dom0-nr-pages.patch --]
[-- Type: text/plain, Size: 4413 bytes --]

x86/Dom0: account for shadow/HAP allocation

... when calculating how many pages to allocate fopr Dom0. This is
basically equivalent to the already present IOMMU related adjustment.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -132,6 +132,8 @@ struct vcpu *__init alloc_dom0_vcpu0(str
 #ifdef CONFIG_SHADOW_PAGING
 static bool_t __initdata opt_dom0_shadow;
 boolean_param("dom0_shadow", opt_dom0_shadow);
+#else
+#define opt_dom0_shadow 0
 #endif
 
 static char __initdata opt_dom0_ioports_disable[200] = "";
@@ -203,13 +205,23 @@ static struct page_info * __init alloc_c
     return page;
 }
 
+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));
+
+    return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
+}
+
 static unsigned long __init compute_dom0_nr_pages(
     struct domain *d, struct elf_dom_parms *parms, unsigned long initrd_len)
 {
     unsigned long avail = avail_domheap_pages() + initial_images_nrpages();
-    unsigned long nr_pages = dom0_nrpages;
-    unsigned long min_pages = dom0_min_nrpages;
-    unsigned long max_pages = dom0_max_nrpages;
+    unsigned long nr_pages, min_pages, max_pages;
+    bool_t need_paging;
 
     /* Reserve memory for further dom0 vcpu-struct allocations... */
     avail -= (d->max_vcpus - 1UL)
@@ -227,23 +239,37 @@ static unsigned long __init compute_dom0
             avail -= max_pdx >> s;
     }
 
-    /*
-     * If domain 0 allocation isn't specified, reserve 1/16th of available
-     * memory for things like DMA buffers. This reservation is clamped to 
-     * a maximum of 128MB.
-     */
-    if ( nr_pages == 0 )
-        nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
+    need_paging = opt_dom0_shadow || (is_pvh_domain(d) && !iommu_hap_pt_share);
+    for ( ; ; need_paging = 0 )
+    {
+        nr_pages = dom0_nrpages;
+        min_pages = dom0_min_nrpages;
+        max_pages = dom0_max_nrpages;
+
+        /*
+         * If allocation isn't specified, reserve 1/16th of available memory
+         * for things like DMA buffers. This reservation is clamped to a
+         * maximum of 128MB.
+         */
+        if ( nr_pages == 0 )
+            nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
 
-    /* Negative memory specification means "all memory - specified amount". */
-    if ( (long)nr_pages  < 0 ) nr_pages  += avail;
-    if ( (long)min_pages < 0 ) min_pages += avail;
-    if ( (long)max_pages < 0 ) max_pages += avail;
-
-    /* Clamp dom0 memory according to min/max limits and available memory. */
-    nr_pages = max(nr_pages, min_pages);
-    nr_pages = min(nr_pages, max_pages);
-    nr_pages = min(nr_pages, avail);
+        /* Negative specification means "all memory - specified amount". */
+        if ( (long)nr_pages  < 0 ) nr_pages  += avail;
+        if ( (long)min_pages < 0 ) min_pages += avail;
+        if ( (long)max_pages < 0 ) max_pages += avail;
+
+        /* Clamp according to min/max limits and available memory. */
+        nr_pages = max(nr_pages, min_pages);
+        nr_pages = min(nr_pages, max_pages);
+        nr_pages = min(nr_pages, avail);
+
+        if ( !need_paging )
+            break;
+
+        /* Reserve memory for shadow or HAP. */
+        avail -= dom0_paging_pages(d, nr_pages);
+    }
 
     if ( (parms->p2m_base == UNSET_ADDR) && (dom0_nrpages <= 0) &&
          ((dom0_min_nrpages <= 0) || (nr_pages > min_pages)) )
@@ -1287,14 +1313,7 @@ int __init construct_dom0(
     }
 
     if ( is_pvh_domain(d) )
-    {
-        unsigned long hap_pages, memkb = nr_pages * (PAGE_SIZE / 1024);
-
-        /* Copied from: libxl_get_required_shadow_memory() */
-        memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
-        hap_pages = ( (memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
-        hap_set_alloc_for_pvh_dom0(d, hap_pages);
-    }
+        hap_set_alloc_for_pvh_dom0(d, dom0_paging_pages(d, nr_pages));
 
     /*
      * We enable paging mode again so guest_physmap_add_page will do the

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

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

[Andrew Cooper]

On 25/02/15 14:45, Jan Beulich wrote:
> ... when calculating how many pages to allocate fopr Dom0. This is

"fopr" => "for" ?

> basically equivalent to the already present IOMMU related adjustment.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -132,6 +132,8 @@ struct vcpu *__init alloc_dom0_vcpu0(str
>  #ifdef CONFIG_SHADOW_PAGING
>  static bool_t __initdata opt_dom0_shadow;
>  boolean_param("dom0_shadow", opt_dom0_shadow);
> +#else
> +#define opt_dom0_shadow 0
>  #endif
>  
>  static char __initdata opt_dom0_ioports_disable[200] = "";
> @@ -203,13 +205,23 @@ static struct page_info * __init alloc_c
>      return page;
>  }
>  
> +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.

~Andrew

> +
> +    return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
> +}
> +
>  static unsigned long __init compute_dom0_nr_pages(
>      struct domain *d, struct elf_dom_parms *parms, unsigned long initrd_len)
>  {
>      unsigned long avail = avail_domheap_pages() + initial_images_nrpages();
> -    unsigned long nr_pages = dom0_nrpages;
> -    unsigned long min_pages = dom0_min_nrpages;
> -    unsigned long max_pages = dom0_max_nrpages;
> +    unsigned long nr_pages, min_pages, max_pages;
> +    bool_t need_paging;
>  
>      /* Reserve memory for further dom0 vcpu-struct allocations... */
>      avail -= (d->max_vcpus - 1UL)
> @@ -227,23 +239,37 @@ static unsigned long __init compute_dom0
>              avail -= max_pdx >> s;
>      }
>  
> -    /*
> -     * If domain 0 allocation isn't specified, reserve 1/16th of available
> -     * memory for things like DMA buffers. This reservation is clamped to 
> -     * a maximum of 128MB.
> -     */
> -    if ( nr_pages == 0 )
> -        nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
> +    need_paging = opt_dom0_shadow || (is_pvh_domain(d) && !iommu_hap_pt_share);
> +    for ( ; ; need_paging = 0 )
> +    {
> +        nr_pages = dom0_nrpages;
> +        min_pages = dom0_min_nrpages;
> +        max_pages = dom0_max_nrpages;
> +
> +        /*
> +         * If allocation isn't specified, reserve 1/16th of available memory
> +         * for things like DMA buffers. This reservation is clamped to a
> +         * maximum of 128MB.
> +         */
> +        if ( nr_pages == 0 )
> +            nr_pages = -min(avail / 16, 128UL << (20 - PAGE_SHIFT));
>  
> -    /* Negative memory specification means "all memory - specified amount". */
> -    if ( (long)nr_pages  < 0 ) nr_pages  += avail;
> -    if ( (long)min_pages < 0 ) min_pages += avail;
> -    if ( (long)max_pages < 0 ) max_pages += avail;
> -
> -    /* Clamp dom0 memory according to min/max limits and available memory. */
> -    nr_pages = max(nr_pages, min_pages);
> -    nr_pages = min(nr_pages, max_pages);
> -    nr_pages = min(nr_pages, avail);
> +        /* Negative specification means "all memory - specified amount". */
> +        if ( (long)nr_pages  < 0 ) nr_pages  += avail;
> +        if ( (long)min_pages < 0 ) min_pages += avail;
> +        if ( (long)max_pages < 0 ) max_pages += avail;
> +
> +        /* Clamp according to min/max limits and available memory. */
> +        nr_pages = max(nr_pages, min_pages);
> +        nr_pages = min(nr_pages, max_pages);
> +        nr_pages = min(nr_pages, avail);
> +
> +        if ( !need_paging )
> +            break;
> +
> +        /* Reserve memory for shadow or HAP. */
> +        avail -= dom0_paging_pages(d, nr_pages);
> +    }
>  
>      if ( (parms->p2m_base == UNSET_ADDR) && (dom0_nrpages <= 0) &&
>           ((dom0_min_nrpages <= 0) || (nr_pages > min_pages)) )
> @@ -1287,14 +1313,7 @@ int __init construct_dom0(
>      }
>  
>      if ( is_pvh_domain(d) )
> -    {
> -        unsigned long hap_pages, memkb = nr_pages * (PAGE_SIZE / 1024);
> -
> -        /* Copied from: libxl_get_required_shadow_memory() */
> -        memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
> -        hap_pages = ( (memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
> -        hap_set_alloc_for_pvh_dom0(d, hap_pages);
> -    }
> +        hap_set_alloc_for_pvh_dom0(d, dom0_paging_pages(d, nr_pages));
>  
>      /*
>       * We enable paging mode again so guest_physmap_add_page will do the
>
>

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

[Jan Beulich]

>>> On 25.02.15 at 18:06, <andrew.cooper3@citrix.com> 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. 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.

Jan

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

[Andrew Cooper]

On 26/02/15 07:43, Jan Beulich wrote:
>>>> On 25.02.15 at 18:06, <andrew.cooper3@citrix.com> 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.

~Andrew

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

[Jan Beulich]

>>> On 27.02.15 at 13:02, <andrew.cooper3@citrix.com> wrote:
> On 26/02/15 07:43, Jan Beulich wrote:
>>>>> On 25.02.15 at 18:06, <andrew.cooper3@citrix.com> 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

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

[Andrew Cooper]

On 27/02/15 13:21, Jan Beulich wrote:
>>>> On 27.02.15 at 13:02, <andrew.cooper3@citrix.com> wrote:
>> On 26/02/15 07:43, Jan Beulich wrote:
>>>>>> On 25.02.15 at 18:06, <andrew.cooper3@citrix.com> 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@citrix.com>

>
> Jan
>