diff mbox

[Xen-devel,v2,39/41] arm : acpi configure interrupts dynamically

Message ID 1431893048-5214-40-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit May 17, 2015, 8:04 p.m. UTC
Interrupt information is described in DSDT and is not available at
the time of booting. Configure the interrupts dynamically when requested
by Dom0

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
---
 xen/arch/arm/vgic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Parth Dixit July 5, 2015, 1:36 p.m. UTC | #1
+shannon

On 8 June 2015 at 23:09, Julien Grall <julien.grall@citrix.com> wrote:
> Hi Parth,
>
> On 17/05/2015 21:04, Parth Dixit wrote:
>>
>> Interrupt information is described in DSDT and is not available at
>> the time of booting. Configure the interrupts dynamically when requested
>> by Dom0
>
>
> Missing "."
>
> Also, I'm sure we talked about it multiple time. I'd like to keep the ACPI
> changes very contained to Xen boot. Your change is not ACPI specific and
> could be used for DT.
>
>
>>
>> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>> ---
>>   xen/arch/arm/vgic.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>> index 73a6f7e..f63deb4 100644
>> --- a/xen/arch/arm/vgic.c
>> +++ b/xen/arch/arm/vgic.c
>> @@ -25,6 +25,7 @@
>>   #include <xen/irq.h>
>>   #include <xen/sched.h>
>>   #include <xen/perfc.h>
>> +#include <xen/acpi.h>
>>
>>   #include <asm/current.h>
>>
>> @@ -285,6 +286,8 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int
>> n)
>>       }
>>   }
>>
>> +#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
>> +
>>   void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>   {
>>       struct domain *d = v->domain;
>> @@ -296,7 +299,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int
>> n)
>>       struct vcpu *v_target;
>>
>>       while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>> +#ifdef CONFIG_ACPI
>> +        struct vgic_irq_rank *vr = vgic_get_rank(v, n);
>> +        uint32_t tr;
>
>
> New line.
>
>> +        irq = i + (32 * n);
>> +        if( ( !acpi_disabled ) && ( n != 0 ) && is_hardware_domain(d) )
>
>
> You need to add a comment explaining the ( n != 0 ) i.e we don't SGIs and
> PPIs are RO. It's implementation defined for PPI but it's preferable to let
> Xen take care of it.
>
> Also, we should set the type only for IRQ assigned to DOM0 (i.e p->desc !=
> NULL). With your current solution, DOM0 may change the configuration of the
> serial IRQ by mistake and take down Xen because the physical IRQ is enabled
> and the behavior will be unpredictable.
>
> Furthermore, during passthrough, the IRQ may not have been configured by
> DOM0. So we have to let the guest configure the IRQ.
>
>> +        {
>> +            tr = vr->icfg[i >> 4] ;
>> +
>> +            if( ( tr & VGIC_ICFG_MASK(i) ) )
>> +                set_irq_type(irq, ACPI_IRQ_TYPE_EDGE_BOTH);
>> +            else
>> +                set_irq_type(irq, ACPI_IRQ_TYPE_LEVEL_MASK);
>
>
> Given that only SPI can be configured it would have been better to call
> irq_set_type.
>
> Although, those 2 functions don't do what you think. They are setting the
> type internally in Xen but don't change the GIC interrupt configuration
> register.
>
> Lastly, they may fail because the configuration as been set earlier (as you
> did in patch #41.
>
> Regards,
>
> --
> Julien Grall
diff mbox

Patch

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 73a6f7e..f63deb4 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -25,6 +25,7 @@ 
 #include <xen/irq.h>
 #include <xen/sched.h>
 #include <xen/perfc.h>
+#include <xen/acpi.h>
 
 #include <asm/current.h>
 
@@ -285,6 +286,8 @@  void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     }
 }
 
+#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
+
 void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
 {
     struct domain *d = v->domain;
@@ -296,7 +299,22 @@  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     struct vcpu *v_target;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
+#ifdef CONFIG_ACPI
+        struct vgic_irq_rank *vr = vgic_get_rank(v, n);
+        uint32_t tr;
+        irq = i + (32 * n);
+        if( ( !acpi_disabled ) && ( n != 0 ) && is_hardware_domain(d) )
+        {
+            tr = vr->icfg[i >> 4] ;
+
+            if( ( tr & VGIC_ICFG_MASK(i) ) )
+                set_irq_type(irq, ACPI_IRQ_TYPE_EDGE_BOTH);
+            else
+                set_irq_type(irq, ACPI_IRQ_TYPE_LEVEL_MASK);
+        }
+#else
         irq = i + (32 * n);
+#endif
         v_target = d->arch.vgic.handler->get_target_vcpu(v, irq);
         p = irq_to_pending(v_target, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);