| Message ID | 1431893048-5214-40-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
+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 --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);
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(+)