From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/Dom0: account for shadow/HAP allocation Date: Fri, 27 Feb 2015 14:45:17 +0000 Message-ID: <54F082FD.9080602@citrix.com> References: <54EDEE080200007800063AA4@mail.emea.novell.com> <54EE0101.4000804@citrix.com> <54EEDCA10200007800063EF2@mail.emea.novell.com> <54F05CD1.6040909@citrix.com> <54F07D4C0200007800064988@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YRMDk-0000GT-9a for xen-devel@lists.xenproject.org; Fri, 27 Feb 2015 14:48:52 +0000 In-Reply-To: <54F07D4C0200007800064988@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 27/02/15 13:21, Jan Beulich wrote: >>>> On 27.02.15 at 13:02, wrote: >> On 26/02/15 07:43, Jan Beulich wrote: >>>>>> On 25.02.15 at 18:06, 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 > > Jan >