From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH V6 08/10] xen: Add arch_domain_preinit to initialise vGIC before evtchn_init Date: Thu, 11 Jun 2015 07:47:11 -0400 Message-ID: <5579753F.2010006@citrix.com> References: <1433163388-16970-1-git-send-email-cbz@baozis.org> <1433163388-16970-9-git-send-email-cbz@baozis.org> <1433521376.7108.347.camel@citrix.com> <20150611092057.GA16415@cbz-thinkpad> <1434015425.30003.134.camel@citrix.com> <20150611111620.GA18855@cbz-thinkpad> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z30xG-0004Uo-Vj for xen-devel@lists.xenproject.org; Thu, 11 Jun 2015 11:47:31 +0000 In-Reply-To: <20150611111620.GA18855@cbz-thinkpad> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chen Baozi , Ian Campbell Cc: Keir Fraser , Andrew Cooper , Tim Deegan , Stefano Stabellini , Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org Hi Chen, On 11/06/2015 07:16, Chen Baozi wrote: > On Thu, Jun 11, 2015 at 10:37:05AM +0100, Ian Campbell wrote: >> On Thu, 2015-06-11 at 17:20 +0800, Chen Baozi wrote: >>> On Fri, Jun 05, 2015 at 05:22:56PM +0100, Ian Campbell wrote: >>>> On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote: >>>>> From: Chen Baozi >>>>> >>>>> evtchn_init will call domain_max_vcpus to allocate poll_mask. On >>>>> arm/arm64 platform, this number is determined by the vGIC the guest >>>>> is going to use, which won't be initialised until arch_domain_create >>>>> is called in current implementation. However, moving arch_domain_create >>>>> means that we will allocate memory before checking the XSM policy, >>>>> which seems not to be acceptable because if the domain is not allowed >>>>> to boot by XSM policy the expensive execution of arch_domain_create >>>>> is wasteful. Thus, we create the arch_domain_preinit to make vgic_ops >>>>> initialisation be done earlier. >>>> >>>> I don't have a fundamental objection to this refactoring, but I'm >>>> curious under what circumstances something would belong in preinit >>>> rather than create, i.e. what is the expected semantics of this new hook >>>> vs the old one. >>> >>> Hmmm, I didn't think about it at this level, :P. Instead, I just brought >>> the code which must be executed before evtchn_init in advance without >>> further consideration on semantics etc... >>> >>> In fact, the current arch_domain_preinit on arm is all about vgic. I >>> was about to use the name vgic_preinit, but that would be an arm-specific >>> name which I thought was not suitable to add in the common codes... >>> >>> Or will it be clearer to call a subfunction called vgic_preinit in the >>> arch_domain_preinit (for arm) and put the current main contents of >>> arch_domain_preinit to vgic_preinit? >> >> The main question which needs answering is: given some new bit of >> functionality which needs initialising when should it be done in preinit >> and when should it be in init? >> > > The resource that would be used (either directly or indirectly) by > xsm_domain_create, evtchn_init or grant_table_create, should be initialised > in preinit? otherwise, it should be put into init? > > I am not sure, for it doesn't look like a(n) good/exact semantic > definition... Rather than splitting arch_domain_init, I was thinking to handle domain_max_vcpus differently: domain_max_vcpus(struct domain *d) { if ( !d->arch.vgic.ops ) return MAX_VIRT_CPUS; else return (min(MAX_VIRT_CPUS, d->arch.vgic.ops.max_vcpus)); } On ARM32, event channel doesn't need to allocate the poll_mask (MAX_VIRT_CPUS < BITS_PER_LONG). So no concern. On ARM64, we have to always allocate the vCPU mask. This wouldn't be so bad to allocate more memory (2 unsigned long rather than 1 for GICv2). Regards, -- Julien Grall