Re: [Xen-devel] [PATCH V6 08/10] xen: Add arch_domain_preinit to initialise vGIC before evtchn_init

[Chen Baozi <cbz@xxxxxxxxxx>]

Hi Julien,

On Thu, Jun 11, 2015 at 07:47:11AM -0400, Julien Grall wrote:
> 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 <baozich@xxxxxxxxx>
> >>>>>
> >>>>>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).

That looks like the second suggestion in Ian's mail. I'll take it in the
next version.

Thanks.

Baozi.

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