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

2015-01-14 Thread Ian Campbell
On Wed, 2015-01-14 at 12:24 +, Julien Grall wrote:
> On 13/01/15 15:58, Ian Campbell wrote:
> > 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...)
> 
> It would be nice to have a such think. If not randomize at each boot,
> maybe per Xen version.

Like I say, that would be quite mean of us. (I was 99% joking about this
FWIW, perhaps I needed to ring that bell a bit more though).

> Though it would break mini-os on ARM as the IRQ is hardcoded in the OS.

AIUI there is an eventual desire to have an ARM mini-os binary be
portable across Xen versions, which will require dtb parsing for this
stuff (actually, I thought the latest patches had added a bunch of such
stuff, but maybe not this specific aspect).

Ian.


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


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

2015-01-14 Thread Julien Grall
On 13/01/15 15:58, Ian Campbell wrote:
> 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...)

It would be nice to have a such think. If not randomize at each boot,
maybe per Xen version.

Though it would break mini-os on ARM as the IRQ is hardcoded in the OS.

Regards,

-- 
Julien Grall

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


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

2015-01-13 Thread Ian Campbell
On Mon, 2014-12-15 at 16:09 +, 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 
> >> ---
> >>  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.


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


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

2014-12-15 Thread Julien Grall
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 
>> ---
>>  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. Futhermore, DOMUs
are using a specific hardcoded layout (see xen/include/public/arch-arm.h).

Regards,

-- 
Julien Grall

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


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

2014-12-15 Thread Stefano Stabellini
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 
> ---
>  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?


>  /*
>   * Virtual UART is only used by linux early printk and decompress code.
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c238c8f..8dedc60 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -625,6 +625,16 @@ static int make_hypervisor_node(struct domain *d,
>  return res;
>  
>  /*
> + * The allocation of the event channel IRQ has been deferred until
> + * now. At this time, all PPIs use by DOM0 has been registered
> + */
> +res = vgic_allocate_virq(d, 0);
> +if ( res < 0 )
> +return -FDT_ERR_XEN(ENOSPC);
> +
> +d->arch.evtchn_irq = res;
> +
> +/*
>   * interrupts is evtchn upcall:
>   *  - Active-low level-sensitive
>   *  - All cpus
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index cb4cda8..d016797 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -160,13 +160,6 @@ bool_t platform_device_is_blacklisted(const struct 
> dt_device_node *node)
>  return dt_match_node(blacklist, node);
>  }
>  
> -unsigned int platform_dom0_evtchn_ppi(void)
> -{
> -if ( platform && platform->dom0_evtchn_ppi )
> -return platform->dom0_evtchn_ppi;
> -return GUEST_EVTCHN_PPI;
> -}
> -
>  void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
>  {
>  if ( platform && platform->dom0_gnttab_size )
> diff --git a/xen/arch/arm/platforms/xgene-storm.c 
> b/xen/arch/arm/platforms/xgene-storm.c
> index 416d42c..b0808b8 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -232,7 +232,6 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
>  .quirks = xgene_storm_quirks,
>  .specific_mapping = xgene_storm_specific_mapping,
>  
> -.dom0_evtchn_ppi = 24,
>  .dom0_gnttab_start = 0x1f80,
>  .dom0_gnttab_size = 0x2,
>  PLATFORM_END
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index eefaca6..4eba37b 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -38,10 +38,6 @@ struct platform_desc {
>   */
>  const struct dt_device_match *blacklist_dev;
>  /*
> - * The IRQ (PPI) to use to inject event channels to dom0.
> - */
> -unsigned int dom0_evtchn_ppi;
> -/*
>   * The location of a region of physical address space which dom0
>   * can use for grant table mappings. If size is zero defaults to
>   * 0xb000-0xb002.
> -- 
> 2.1.3
> 

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


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

2014-12-12 Thread Julien Grall
On 12/12/14 14:43, Julien Grall wrote:
> 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));

I forgot the "!" in the BUG_ON. This line should be:

BUG_ON(!..)


-- 
Julien Grall

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


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

2014-12-12 Thread Julien Grall
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 
---
 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));
+}
 
 /*
  * Virtual UART is only used by linux early printk and decompress code.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c238c8f..8dedc60 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -625,6 +625,16 @@ static int make_hypervisor_node(struct domain *d,
 return res;
 
 /*
+ * The allocation of the event channel IRQ has been deferred until
+ * now. At this time, all PPIs use by DOM0 has been registered
+ */
+res = vgic_allocate_virq(d, 0);
+if ( res < 0 )
+return -FDT_ERR_XEN(ENOSPC);
+
+d->arch.evtchn_irq = res;
+
+/*
  * interrupts is evtchn upcall:
  *  - Active-low level-sensitive
  *  - All cpus
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index cb4cda8..d016797 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -160,13 +160,6 @@ bool_t platform_device_is_blacklisted(const struct 
dt_device_node *node)
 return dt_match_node(blacklist, node);
 }
 
-unsigned int platform_dom0_evtchn_ppi(void)
-{
-if ( platform && platform->dom0_evtchn_ppi )
-return platform->dom0_evtchn_ppi;
-return GUEST_EVTCHN_PPI;
-}
-
 void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
 {
 if ( platform && platform->dom0_gnttab_size )
diff --git a/xen/arch/arm/platforms/xgene-storm.c 
b/xen/arch/arm/platforms/xgene-storm.c
index 416d42c..b0808b8 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -232,7 +232,6 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
 .quirks = xgene_storm_quirks,
 .specific_mapping = xgene_storm_specific_mapping,
 
-.dom0_evtchn_ppi = 24,
 .dom0_gnttab_start = 0x1f80,
 .dom0_gnttab_size = 0x2,
 PLATFORM_END
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index eefaca6..4eba37b 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -38,10 +38,6 @@ struct platform_desc {
  */
 const struct dt_device_match *blacklist_dev;
 /*
- * The IRQ (PPI) to use to inject event channels to dom0.
- */
-unsigned int dom0_evtchn_ppi;
-/*
  * The location of a region of physical address space which dom0
  * can use for grant table mappings. If size is zero defaults to
  * 0xb000-0xb002.
-- 
2.1.3


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