Re: [PATCH for-4.6 4/4] xen/arm: Find automatically a PPI for the DOM0 event channel interrupt

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2014-12-15 at 16:09 +0000, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/12/14 15:35, Stefano Stabellini wrote:
> > On Fri, 12 Dec 2014, Julien Grall wrote:
> >> Use the new vgic interface to know which virtual PPI is free and use it
> >> for the event channel code.
> >>
> >> At the DOM0 creation time, Xen still don't know which vIRQ will be free.
> >> All the vIRQ will be reserved when we parse the device tree. So allocate
> >> when the hypervisor node is created.
> >>
> >> It's safe to defer the allocation because no vIRQ can be injected as
> >> long as the vCPU is not online.
> >>
> >> Also correct the check in arch_domain_create to use is_hardware_domain.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> ---
> >>  xen/arch/arm/domain.c                | 13 ++++++++++---
> >>  xen/arch/arm/domain_build.c          | 10 ++++++++++
> >>  xen/arch/arm/platform.c              |  7 -------
> >>  xen/arch/arm/platforms/xgene-storm.c |  1 -
> >>  xen/include/asm-arm/platform.h       |  4 ----
> >>  5 files changed, 20 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> >> index 7221bc8..7d14377 100644
> >> --- a/xen/arch/arm/domain.c
> >> +++ b/xen/arch/arm/domain.c
> >> @@ -543,10 +543,17 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
> >>      if ( (rc = domain_vtimer_init(d)) != 0 )
> >>          goto fail;
> >>  
> >> -    if ( d->domain_id )
> >> +    /*
> >> +     * The hardware domain will get a PPI later in
> >> +     * arch/arm/domain_build.c  depending on the
> >> +     * interrupt map of the hardware.
> >> +     */
> >> +    if ( !is_hardware_domain(d) )
> >> +    {
> >>          d->arch.evtchn_irq = GUEST_EVTCHN_PPI;
> >> -    else
> >> -        d->arch.evtchn_irq = platform_dom0_evtchn_ppi();
> >> +        /* At this stage vgic_reserve_virq should never fail */
> >> +        BUG_ON(vgic_reserve_virq(d, GUEST_EVTCHN_PPI));
> >> +    }
> > 
> > Why do we still need this, if we have another vgic_allocate_virq call in
> > make_hypervisor_node? Wouldn't that work for DomUs too?
> 
> Because make_hypervisor_node is only used for DOM0.

Yeah.

> Futhermore, DOMUs
> are using a specific hardcoded layout (see xen/include/public/arch-arm.h).

They don't actually have to be, but exposing vgic_allocate_virq to the
tools would be overkill, so hardcoding is the pragmatic choice.

We could e.g. randomise the PPI in the tools, to stop people making
assumptions. (If we were feeling mean of course...)

Ian.