* [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
@ 2014-07-04 14:39 Stefano Stabellini
2014-07-04 14:44 ` Julien Grall
2014-07-09 12:41 ` Ian Campbell
0 siblings, 2 replies; 12+ messages in thread
From: Stefano Stabellini @ 2014-07-04 14:39 UTC (permalink / raw)
To: xen-devel
Cc: julien.grall, apatel, Ian.Campbell, psawargaonkar, stefano.stabellini
GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
interrupts and perform EOIs in the hypervisor for hardware interrupts as
a workaround. Trigger this behaviour with a per platform option.
This patch assumes that GICC_DIR can be written on any pcpu for a given
SGI, not matter where GICC_IAR has been read before.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: psawargaonkar@apm.com
CC: apatel@apm.com
---
Changes in v3:
- do not request maintenance interrupts for virtual interrupts.
Changes in v2:
- s/PLATFORM_QUIRK_NEED_EOI/PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI/g;
- improved commit message.
---
xen/arch/arm/gic-v2.c | 9 +++++++--
xen/arch/arm/gic.c | 4 ++++
xen/arch/arm/platforms/xgene-storm.c | 2 +-
xen/include/asm-arm/platform.h | 5 +++++
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index c3d2853..66ad069 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -361,8 +361,13 @@ static void gicv2_update_lr(int lr, const struct pending_irq *p,
((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
if ( p->desc != NULL )
- lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
- << GICH_V2_LR_PHYSICAL_SHIFT);
+ {
+ if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
+ lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
+ else
+ lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
+ << GICH_V2_LR_PHYSICAL_SHIFT);
+ }
writel_relaxed(lr_reg, GICH + GICH_LR + lr * 4);
}
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 37b08c2..7fa8a0f 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -369,7 +369,11 @@ static void gic_update_one_lr(struct vcpu *v, int i)
clear_bit(i, &this_cpu(lr_mask));
if ( p->desc != NULL )
+ {
p->desc->status &= ~IRQ_INPROGRESS;
+ if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
+ gic_hw_ops->deactivate_irq(p->desc);
+ }
clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
p->lr = GIC_INVALID_LR;
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index c9dd63c..837d8e6 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -37,7 +37,7 @@ static bool reset_vals_valid = false;
static uint32_t xgene_storm_quirks(void)
{
- return PLATFORM_QUIRK_GIC_64K_STRIDE;
+ return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
}
static int map_one_mmio(struct domain *d, const char *what,
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index bcd2097..eefaca6 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -55,6 +55,11 @@ struct platform_desc {
*/
#define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0)
+/*
+ * Quirk for platforms where GICH_LR_HW does not work as expected.
+ */
+#define PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI (1 << 1)
+
void __init platform_init(void);
int __init platform_init_time(void);
int __init platform_specific_mapping(struct domain *d);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-04 14:39 [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI Stefano Stabellini
@ 2014-07-04 14:44 ` Julien Grall
2014-07-09 12:41 ` Ian Campbell
1 sibling, 0 replies; 12+ messages in thread
From: Julien Grall @ 2014-07-04 14:44 UTC (permalink / raw)
To: Stefano Stabellini, xen-devel; +Cc: apatel, Ian.Campbell, psawargaonkar
Hi Stefano,
On 04/07/14 15:39, Stefano Stabellini wrote:
> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> interrupts and perform EOIs in the hypervisor for hardware interrupts as
> a workaround. Trigger this behaviour with a per platform option.
>
> This patch assumes that GICC_DIR can be written on any pcpu for a given
> SGI, not matter where GICC_IAR has been read before.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-04 14:39 [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI Stefano Stabellini
2014-07-04 14:44 ` Julien Grall
@ 2014-07-09 12:41 ` Ian Campbell
2014-07-09 13:43 ` Pranavkumar Sawargaonkar
1 sibling, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2014-07-09 12:41 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, apatel, psawargaonkar, julien.grall
On Fri, 2014-07-04 at 15:39 +0100, Stefano Stabellini wrote:
> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> interrupts and perform EOIs in the hypervisor for hardware interrupts as
> a workaround. Trigger this behaviour with a per platform option.
>
> This patch assumes that GICC_DIR can be written on any pcpu for a given
> SGI, not matter where GICC_IAR has been read before.
Did you really mean SGI here? Those are per-cpu, I suspect you meant
SPI?
Ack to the actual patch though.
Ian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 12:41 ` Ian Campbell
@ 2014-07-09 13:43 ` Pranavkumar Sawargaonkar
2014-07-09 14:19 ` Stefano Stabellini
0 siblings, 1 reply; 12+ messages in thread
From: Pranavkumar Sawargaonkar @ 2014-07-09 13:43 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Anup Patel, Ian Campbell, Julien Grall
Hi Stefano/ Ian,
On Wed, Jul 9, 2014 at 6:11 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2014-07-04 at 15:39 +0100, Stefano Stabellini wrote:
>> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
>> interrupts and perform EOIs in the hypervisor for hardware interrupts as
>> a workaround. Trigger this behaviour with a per platform option.
>>
>> This patch assumes that GICC_DIR can be written on any pcpu for a given
>> SGI, not matter where GICC_IAR has been read before.
>
> Did you really mean SGI here? Those are per-cpu, I suspect you meant
> SPI?
>
> Ack to the actual patch though.
>
> Ian.
>
We have found clean fix for this issue in u-boot.
The issue is that X-Gene does not implement security extensions
but the GIC-400 present in X-Gene has security extensions. To take
care of this situation, APM HW designers have provided two sets
of GIC register addresses: one for accessing GIC secured registers,
and another for accessing GIC non-secured registers. Currently, we
are only accessing GIC secured register for Linux, Xen, and KVM.
This works fine in most cases but does not work for GICH_LRn.HW
bit because we can only auto-deactivate non-secured interrupts using
GICH_LRn.HW bit.
To fix this issue, we have updated u-boot to initialize GIC secured
register to make all interrupts as non-secured and we will need to
access GIC non-secured registers from Linux, Xen, KVM, and
everywhere else.
For now, you can go ahead with this patch but once we have updated
u-boot released by APM then we will need to disable the quirk for
X-Gene Mustang. We would be also having a patch for Linux to fix
the GIC addresses in X-Gene Storm DTS file.
Thanks,
Pranav
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 13:43 ` Pranavkumar Sawargaonkar
@ 2014-07-09 14:19 ` Stefano Stabellini
2014-07-09 14:24 ` Anup Patel
0 siblings, 1 reply; 12+ messages in thread
From: Stefano Stabellini @ 2014-07-09 14:19 UTC (permalink / raw)
To: Pranavkumar Sawargaonkar
Cc: xen-devel, Anup Patel, Ian Campbell, Julien Grall, Stefano Stabellini
On Wed, 9 Jul 2014, Pranavkumar Sawargaonkar wrote:
> Hi Stefano/ Ian,
>
> On Wed, Jul 9, 2014 at 6:11 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-07-04 at 15:39 +0100, Stefano Stabellini wrote:
> >> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> >> interrupts and perform EOIs in the hypervisor for hardware interrupts as
> >> a workaround. Trigger this behaviour with a per platform option.
> >>
> >> This patch assumes that GICC_DIR can be written on any pcpu for a given
> >> SGI, not matter where GICC_IAR has been read before.
> >
> > Did you really mean SGI here? Those are per-cpu, I suspect you meant
> > SPI?
> >
> > Ack to the actual patch though.
> >
> > Ian.
> >
>
> We have found clean fix for this issue in u-boot.
>
> The issue is that X-Gene does not implement security extensions
> but the GIC-400 present in X-Gene has security extensions. To take
> care of this situation, APM HW designers have provided two sets
> of GIC register addresses: one for accessing GIC secured registers,
> and another for accessing GIC non-secured registers. Currently, we
> are only accessing GIC secured register for Linux, Xen, and KVM.
> This works fine in most cases but does not work for GICH_LRn.HW
> bit because we can only auto-deactivate non-secured interrupts using
> GICH_LRn.HW bit.
>
> To fix this issue, we have updated u-boot to initialize GIC secured
> register to make all interrupts as non-secured and we will need to
> access GIC non-secured registers from Linux, Xen, KVM, and
> everywhere else.
>
> For now, you can go ahead with this patch but once we have updated
> u-boot released by APM then we will need to disable the quirk for
> X-Gene Mustang. We would be also having a patch for Linux to fix
> the GIC addresses in X-Gene Storm DTS file.
It's great that you managed to fix the problem!
How can we detect whether the Mustang board we are running on has an
up-to-date uboot? Would it be possible for u-boot to add a flag
somewhere to tell that the GICH_LRn.HW is safe to use? Then we could
check that flag and only enable PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI if
the flag is missing.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 14:19 ` Stefano Stabellini
@ 2014-07-09 14:24 ` Anup Patel
2014-07-09 14:40 ` Ian Campbell
0 siblings, 1 reply; 12+ messages in thread
From: Anup Patel @ 2014-07-09 14:24 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Pranavkumar Sawargaonkar, Julien Grall, Ian Campbell
On Wed, Jul 9, 2014 at 7:49 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 9 Jul 2014, Pranavkumar Sawargaonkar wrote:
>> Hi Stefano/ Ian,
>>
>> On Wed, Jul 9, 2014 at 6:11 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Fri, 2014-07-04 at 15:39 +0100, Stefano Stabellini wrote:
>> >> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
>> >> interrupts and perform EOIs in the hypervisor for hardware interrupts as
>> >> a workaround. Trigger this behaviour with a per platform option.
>> >>
>> >> This patch assumes that GICC_DIR can be written on any pcpu for a given
>> >> SGI, not matter where GICC_IAR has been read before.
>> >
>> > Did you really mean SGI here? Those are per-cpu, I suspect you meant
>> > SPI?
>> >
>> > Ack to the actual patch though.
>> >
>> > Ian.
>> >
>>
>> We have found clean fix for this issue in u-boot.
>>
>> The issue is that X-Gene does not implement security extensions
>> but the GIC-400 present in X-Gene has security extensions. To take
>> care of this situation, APM HW designers have provided two sets
>> of GIC register addresses: one for accessing GIC secured registers,
>> and another for accessing GIC non-secured registers. Currently, we
>> are only accessing GIC secured register for Linux, Xen, and KVM.
>> This works fine in most cases but does not work for GICH_LRn.HW
>> bit because we can only auto-deactivate non-secured interrupts using
>> GICH_LRn.HW bit.
>>
>> To fix this issue, we have updated u-boot to initialize GIC secured
>> register to make all interrupts as non-secured and we will need to
>> access GIC non-secured registers from Linux, Xen, KVM, and
>> everywhere else.
>>
>> For now, you can go ahead with this patch but once we have updated
>> u-boot released by APM then we will need to disable the quirk for
>> X-Gene Mustang. We would be also having a patch for Linux to fix
>> the GIC addresses in X-Gene Storm DTS file.
>
> It's great that you managed to fix the problem!
>
> How can we detect whether the Mustang board we are running on has an
> up-to-date uboot? Would it be possible for u-boot to add a flag
> somewhere to tell that the GICH_LRn.HW is safe to use? Then we could
> check that flag and only enable PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI if
> the flag is missing.
I think we should check GIC Dist address passed in DTS by
u-boot to Xen. If GIC Dist address is secured address then
enable the quirk otherwise disable the qurik.
Regards,
Anup
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 14:24 ` Anup Patel
@ 2014-07-09 14:40 ` Ian Campbell
2014-07-09 14:43 ` Julien Grall
0 siblings, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2014-07-09 14:40 UTC (permalink / raw)
To: Anup Patel
Cc: xen-devel, Pranavkumar Sawargaonkar, Julien Grall, Stefano Stabellini
On Wed, 2014-07-09 at 19:54 +0530, Anup Patel wrote:
> On Wed, Jul 9, 2014 at 7:49 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Wed, 9 Jul 2014, Pranavkumar Sawargaonkar wrote:
> >> Hi Stefano/ Ian,
> >>
> >> On Wed, Jul 9, 2014 at 6:11 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Fri, 2014-07-04 at 15:39 +0100, Stefano Stabellini wrote:
> >> >> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> >> >> interrupts and perform EOIs in the hypervisor for hardware interrupts as
> >> >> a workaround. Trigger this behaviour with a per platform option.
> >> >>
> >> >> This patch assumes that GICC_DIR can be written on any pcpu for a given
> >> >> SGI, not matter where GICC_IAR has been read before.
> >> >
> >> > Did you really mean SGI here? Those are per-cpu, I suspect you meant
> >> > SPI?
> >> >
> >> > Ack to the actual patch though.
> >> >
> >> > Ian.
> >> >
> >>
> >> We have found clean fix for this issue in u-boot.
> >>
> >> The issue is that X-Gene does not implement security extensions
> >> but the GIC-400 present in X-Gene has security extensions. To take
> >> care of this situation, APM HW designers have provided two sets
> >> of GIC register addresses: one for accessing GIC secured registers,
> >> and another for accessing GIC non-secured registers. Currently, we
> >> are only accessing GIC secured register for Linux, Xen, and KVM.
> >> This works fine in most cases but does not work for GICH_LRn.HW
> >> bit because we can only auto-deactivate non-secured interrupts using
> >> GICH_LRn.HW bit.
> >>
> >> To fix this issue, we have updated u-boot to initialize GIC secured
> >> register to make all interrupts as non-secured and we will need to
> >> access GIC non-secured registers from Linux, Xen, KVM, and
> >> everywhere else.
> >>
> >> For now, you can go ahead with this patch but once we have updated
> >> u-boot released by APM then we will need to disable the quirk for
> >> X-Gene Mustang. We would be also having a patch for Linux to fix
> >> the GIC addresses in X-Gene Storm DTS file.
> >
> > It's great that you managed to fix the problem!
> >
> > How can we detect whether the Mustang board we are running on has an
> > up-to-date uboot? Would it be possible for u-boot to add a flag
> > somewhere to tell that the GICH_LRn.HW is safe to use? Then we could
> > check that flag and only enable PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI if
> > the flag is missing.
>
> I think we should check GIC Dist address passed in DTS by
> u-boot to Xen. If GIC Dist address is secured address then
> enable the quirk otherwise disable the qurik.
Isn't IGROUPR readable from NS? In which case we should be able to spot
the difference, I think?
Stefano, you might want to arrange in your patch to cache the value of
the quirk in the gicv2 struct -- otherwise all the calls to
platform_has_quirk are going to add up.
Ian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 14:40 ` Ian Campbell
@ 2014-07-09 14:43 ` Julien Grall
2014-07-09 15:11 ` Stefano Stabellini
0 siblings, 1 reply; 12+ messages in thread
From: Julien Grall @ 2014-07-09 14:43 UTC (permalink / raw)
To: Ian Campbell, Anup Patel
Cc: xen-devel, Pranavkumar Sawargaonkar, Stefano Stabellini
On 07/09/2014 03:40 PM, Ian Campbell wrote:
>> I think we should check GIC Dist address passed in DTS by
>> u-boot to Xen. If GIC Dist address is secured address then
>> enable the quirk otherwise disable the qurik.
>
> Isn't IGROUPR readable from NS? In which case we should be able to spot
> the difference, I think?
>
> Stefano, you might want to arrange in your patch to cache the value of
> the quirk in the gicv2 struct -- otherwise all the calls to
> platform_has_quirk are going to add up.
Can't we cache the value in the platform code? I don't think it's too
expensive to call the platform_has_quirk in this case.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 14:43 ` Julien Grall
@ 2014-07-09 15:11 ` Stefano Stabellini
2014-07-10 10:33 ` Ian Campbell
2014-07-10 10:51 ` Anup Patel
0 siblings, 2 replies; 12+ messages in thread
From: Stefano Stabellini @ 2014-07-09 15:11 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Anup Patel, Ian Campbell, Pranavkumar Sawargaonkar,
Stefano Stabellini
On Wed, 9 Jul 2014, Julien Grall wrote:
> On 07/09/2014 03:40 PM, Ian Campbell wrote:
> >> I think we should check GIC Dist address passed in DTS by
> >> u-boot to Xen. If GIC Dist address is secured address then
> >> enable the quirk otherwise disable the qurik.
> >
> > Isn't IGROUPR readable from NS? In which case we should be able to spot
> > the difference, I think?
Anup, can you confirm that we can use IGROUPR to spot the difference?
Otherwise is there another register we can use?
In any case given that only you can test it, I think that it should be
one of you guys that come up with a way to detect the change and submit
a patch to conditionally disable the workaround.
> > Stefano, you might want to arrange in your patch to cache the value of
> > the quirk in the gicv2 struct -- otherwise all the calls to
> > platform_has_quirk are going to add up.
>
> Can't we cache the value in the platform code? I don't think it's too
> expensive to call the platform_has_quirk in this case.
Caching the value in gic-v2.c seems a bit overkill. But we could turn
platform->quirks() into a bitfield, then we can significantly reduce
memory accesses when checking for quirks, from 3 to just 1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 15:11 ` Stefano Stabellini
@ 2014-07-10 10:33 ` Ian Campbell
2014-07-10 10:51 ` Anup Patel
1 sibling, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2014-07-10 10:33 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Julien Grall, Anup Patel, Pranavkumar Sawargaonkar
On Wed, 2014-07-09 at 16:11 +0100, Stefano Stabellini wrote:
> > > Stefano, you might want to arrange in your patch to cache the value of
> > > the quirk in the gicv2 struct -- otherwise all the calls to
> > > platform_has_quirk are going to add up.
> >
> > Can't we cache the value in the platform code? I don't think it's too
> > expensive to call the platform_has_quirk in this case.
>
> Caching the value in gic-v2.c seems a bit overkill. But we could turn
> platform->quirks() into a bitfield, then we can significantly reduce
> memory accesses when checking for quirks, from 3 to just 1.
Hrm, yes.
That does have the disadvantage of potentially un-consting the platform
ops.
Anyway, this patch is correct for now and the detection of the fixed
platform can be done later. So applied with s/SGI/SPI in the commit log
as discussed IRL.
Ian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-09 15:11 ` Stefano Stabellini
2014-07-10 10:33 ` Ian Campbell
@ 2014-07-10 10:51 ` Anup Patel
2015-04-02 13:45 ` Stefano Stabellini
1 sibling, 1 reply; 12+ messages in thread
From: Anup Patel @ 2014-07-10 10:51 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Julien Grall, Ian Campbell, Pranavkumar Sawargaonkar
On Wed, Jul 9, 2014 at 8:41 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 9 Jul 2014, Julien Grall wrote:
>> On 07/09/2014 03:40 PM, Ian Campbell wrote:
>> >> I think we should check GIC Dist address passed in DTS by
>> >> u-boot to Xen. If GIC Dist address is secured address then
>> >> enable the quirk otherwise disable the qurik.
>> >
>> > Isn't IGROUPR readable from NS? In which case we should be able to spot
>> > the difference, I think?
>
> Anup, can you confirm that we can use IGROUPR to spot the difference?
> Otherwise is there another register we can use?
Yes, IGROUPR of the secured GIC registers would be correct register
to spot the difference.
More precisely, if IGROUPR0 == 0xFFFFFFFF in GIC secured registers then
don't use the quirk for X-Gene.
>
> In any case given that only you can test it, I think that it should be
> one of you guys that come up with a way to detect the change and submit
> a patch to conditionally disable the workaround.
Sure, most likely Pranav will send a patch for Xen once APM releases
updated u-boot binary.
Thanks,
Anup
>
>
>> > Stefano, you might want to arrange in your patch to cache the value of
>> > the quirk in the gicv2 struct -- otherwise all the calls to
>> > platform_has_quirk are going to add up.
>>
>> Can't we cache the value in the platform code? I don't think it's too
>> expensive to call the platform_has_quirk in this case.
>
> Caching the value in gic-v2.c seems a bit overkill. But we could turn
> platform->quirks() into a bitfield, then we can significantly reduce
> memory accesses when checking for quirks, from 3 to just 1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI
2014-07-10 10:51 ` Anup Patel
@ 2015-04-02 13:45 ` Stefano Stabellini
0 siblings, 0 replies; 12+ messages in thread
From: Stefano Stabellini @ 2015-04-02 13:45 UTC (permalink / raw)
To: Anup Patel
Cc: xen-devel, Julien Grall, Ian Campbell, Pranavkumar Sawargaonkar,
Stefano Stabellini
On Thu, 10 Jul 2014, Anup Patel wrote:
> On Wed, Jul 9, 2014 at 8:41 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Wed, 9 Jul 2014, Julien Grall wrote:
> >> On 07/09/2014 03:40 PM, Ian Campbell wrote:
> >> >> I think we should check GIC Dist address passed in DTS by
> >> >> u-boot to Xen. If GIC Dist address is secured address then
> >> >> enable the quirk otherwise disable the qurik.
> >> >
> >> > Isn't IGROUPR readable from NS? In which case we should be able to spot
> >> > the difference, I think?
> >
> > Anup, can you confirm that we can use IGROUPR to spot the difference?
> > Otherwise is there another register we can use?
>
> Yes, IGROUPR of the secured GIC registers would be correct register
> to spot the difference.
>
> More precisely, if IGROUPR0 == 0xFFFFFFFF in GIC secured registers then
> don't use the quirk for X-Gene.
>
> >
> > In any case given that only you can test it, I think that it should be
> > one of you guys that come up with a way to detect the change and submit
> > a patch to conditionally disable the workaround.
>
> Sure, most likely Pranav will send a patch for Xen once APM releases
> updated u-boot binary.
This slept through the cracks.
Pranav, do you have a patch for this? It is important we fix this
because as you know, not only PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI is
slower but also not entirely safe.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-04-02 13:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-04 14:39 [PATCH v3] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI Stefano Stabellini
2014-07-04 14:44 ` Julien Grall
2014-07-09 12:41 ` Ian Campbell
2014-07-09 13:43 ` Pranavkumar Sawargaonkar
2014-07-09 14:19 ` Stefano Stabellini
2014-07-09 14:24 ` Anup Patel
2014-07-09 14:40 ` Ian Campbell
2014-07-09 14:43 ` Julien Grall
2014-07-09 15:11 ` Stefano Stabellini
2014-07-10 10:33 ` Ian Campbell
2014-07-10 10:51 ` Anup Patel
2015-04-02 13:45 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.