This series enables the MIPS GIC driver to make use of the EIC mode
supported in some MIPS cores. In this mode, the cores 6 interrupt lines
are switched to represent a vector number, 0..63. Currently all GIC
interrupts are routed to a single CPU interrupt pin, but this is
inefficient since we end up checking both local and shared interrupt
flag registers for both local and shared interrupts. This introduces
additional latency into the interrupt paths. With EIC mode this can be
improved by using separate vectors for local and shared interrupts.
This series is based on 4.15-rc6 and has been tested on Boston, Malta &
SEAD3 MIPS platforms implementing a GIC with and without EIC mode
supported in hardware.
Matt Redfearn (6):
MIPS: Move ehb() to barrier.h
MIPS: CPS: Introduce mips_gic_enable_eic
MIPS: Generic: Support GIC in EIC mode
irqchip/mips-gic: Always attempt to enable EIC mode
irqchip/mips-gic: Use separate vector for shared interrupts in EIC
mode
irqchip/mips-gic: Separate local interrupt handling.
arch/mips/generic/irq.c | 18 +++++++++---------
arch/mips/include/asm/barrier.h | 13 +++++++++++++
arch/mips/include/asm/mips-gic.h | 22 ++++++++++++++++++++++
arch/mips/include/asm/mipsmtregs.h | 8 --------
drivers/irqchip/irq-mips-gic.c | 34 +++++++++++++++++++++++-----------
5 files changed, 67 insertions(+), 28 deletions(-)
--
2.7.4
The current location of ehb() in mipsmtregs.h does not make sense, since
it is not strictly related to multi-threading, and may be used in code
which does not include mipsmtregs.h
Signed-off-by: Matt Redfearn <[email protected]>
---
arch/mips/include/asm/barrier.h | 13 +++++++++++++
arch/mips/include/asm/mipsmtregs.h | 8 --------
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index a5eb1bb199a7..a98fd8a2bba2 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -222,6 +222,19 @@
#define __smp_mb__before_atomic() __smp_mb__before_llsc()
#define __smp_mb__after_atomic() smp_llsc_mb()
+/**
+ * ehb() - Execution Hazard Barrier
+ *
+ * Stop instruction execution until execution hazards are cleared
+ */
+static inline void ehb(void)
+{
+ __asm__ __volatile__(
+ " .set mips32r2 \n"
+ " ehb \n"
+ " .set mips0 \n");
+}
+
#include <asm-generic/barrier.h>
#endif /* __ASM_BARRIER_H */
diff --git a/arch/mips/include/asm/mipsmtregs.h b/arch/mips/include/asm/mipsmtregs.h
index 212336b7c0f4..4acfc78bc504 100644
--- a/arch/mips/include/asm/mipsmtregs.h
+++ b/arch/mips/include/asm/mipsmtregs.h
@@ -274,14 +274,6 @@ static inline void emt(int previous)
__raw_emt();
}
-static inline void ehb(void)
-{
- __asm__ __volatile__(
- " .set mips32r2 \n"
- " ehb \n"
- " .set mips0 \n");
-}
-
#define mftc0(rt,sel) \
({ \
unsigned long __res; \
--
2.7.4
The MIPS GIC supports running in External Interrupt Controller (EIC)
mode, in which the GIC can raise up to 64 separate interrupts rather
than the usual 6. This mode is enabled by setting bit GIC_VL_CTL.EIC. If
the bit sticks, then EIC mode is present and becomes enabled. Otherwise
this bit is read-only 0 and setting it will have no effect.
The CP0 register Config3 bit VEIC indicates the status of EIC mode, and
effectively reflects GIC_VL_CTL.EIC. After attempting to enable EIC
mode, read back Config3.VEIC to determine if VEIC mode is present and
has been activated. If so, update the boot CPU flags to reflect that
VEIC mode is now active.
Signed-off-by: Matt Redfearn <[email protected]>
---
arch/mips/include/asm/mips-gic.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/mips/include/asm/mips-gic.h b/arch/mips/include/asm/mips-gic.h
index 558059a8f218..b8345b117224 100644
--- a/arch/mips/include/asm/mips-gic.h
+++ b/arch/mips/include/asm/mips-gic.h
@@ -314,6 +314,28 @@ static inline bool mips_gic_present(void)
return IS_ENABLED(CONFIG_MIPS_GIC) && mips_gic_base;
}
+
+/**
+ * mips_gic_enable_eic() - Enable EIC mode if supported
+ *
+ * Attempt to enable the GICs EIC mode if supported by the hardware.
+ * EIC is enabled via the GIC CTL register. If the bit sticks, then the mode
+ * is supported and active. CP0.Config3.VEIC reflects this state and is read
+ * to determine if the mode has successfully been activated. If it has, update
+ * the boot cpu flags such that cpu_has_veic reflects the new mode.
+ */
+static inline void mips_gic_enable_eic(void)
+{
+ set_gic_vl_ctl(GIC_VX_CTL_EIC);
+ mb(); /* Ensure write to GIC register completes */
+ ehb(); /* Ensure mfc0 does not start early */
+ if (read_c0_config3() & MIPS_CONF3_VEIC) {
+ /* GIC & CPU now in VEIC mode */
+ pr_debug("GIC EIC mode activated\n");
+ boot_cpu_data.options |= MIPS_CPU_VEIC;
+ }
+}
+
/**
* gic_get_c0_compare_int() - Return cp0 count/compare interrupt virq
*
--
2.7.4
The GIC supports running in External Interrupt Controller (EIC) mode,
and will signal this via cpu_has_veic if enabled in hardware. Currently
the generic kernel will panic if cpu_has_veic is set - but the GIC can
legitimately set this flag if either configured to boot in EIC mode, or
if the GIC driver enables this mode. Make the kernel not panic in this
case, and instead just check if the GIC is present. If so, use it's CPU
local interrupt routing functions. If an EIC is present, but it is not
the GIC, then the kernel does not know how to get the VIRQ for the CPU
local interrupts and should panic. Support for alternative EICs being
present is needed here for the generic kernel to support them.
Suggested-by: Paul Burton <[email protected]>
Signed-off-by: Matt Redfearn <[email protected]>
---
arch/mips/generic/irq.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/mips/generic/irq.c b/arch/mips/generic/irq.c
index 394f8161e462..cb7fdaeef426 100644
--- a/arch/mips/generic/irq.c
+++ b/arch/mips/generic/irq.c
@@ -22,10 +22,10 @@ int get_c0_fdc_int(void)
{
int mips_cpu_fdc_irq;
- if (cpu_has_veic)
- panic("Unimplemented!");
- else if (mips_gic_present())
+ if (mips_gic_present())
mips_cpu_fdc_irq = gic_get_c0_fdc_int();
+ else if (cpu_has_veic)
+ panic("Unimplemented!");
else if (cp0_fdc_irq >= 0)
mips_cpu_fdc_irq = MIPS_CPU_IRQ_BASE + cp0_fdc_irq;
else
@@ -38,10 +38,10 @@ int get_c0_perfcount_int(void)
{
int mips_cpu_perf_irq;
- if (cpu_has_veic)
- panic("Unimplemented!");
- else if (mips_gic_present())
+ if (mips_gic_present())
mips_cpu_perf_irq = gic_get_c0_perfcount_int();
+ else if (cpu_has_veic)
+ panic("Unimplemented!");
else if (cp0_perfcount_irq >= 0)
mips_cpu_perf_irq = MIPS_CPU_IRQ_BASE + cp0_perfcount_irq;
else
@@ -54,10 +54,10 @@ unsigned int get_c0_compare_int(void)
{
int mips_cpu_timer_irq;
- if (cpu_has_veic)
- panic("Unimplemented!");
- else if (mips_gic_present())
+ if (mips_gic_present())
mips_cpu_timer_irq = gic_get_c0_compare_int();
+ else if (cpu_has_veic)
+ panic("Unimplemented!");
else
mips_cpu_timer_irq = MIPS_CPU_IRQ_BASE + cp0_compare_irq;
--
2.7.4
The MIPS GIC supports running in External Interrupt Controller (EIC)
mode, in which the GIC can raise up to 64 separate interrupts rather
than the usual 6. This mode is enabled by setting bit GIC_VL_CTL.EIC. If
the bit sticks, then EIC mode is present and becomes enabled. Otherwise
this bit is read-only 0 and setting it will have no effect.
The CP0 register Config3 bit VEIC indicates the status of EIC mode, and
effectively reflects GIC_VL_CTL.EIC. After attempting to enable EIC
mode, read back Config3.VEIC to determine if VEIC mode is present and
has been activated. If so, update the boot CPU flags to reflect that
VEIC mode is now active.
Signed-off-by: Matt Redfearn <[email protected]>
---
drivers/irqchip/irq-mips-gic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index ef92a4d2038e..ee391f42e97d 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -725,6 +725,9 @@ static int __init gic_of_init(struct device_node *node,
gic_shared_intrs >>= __ffs(GIC_CONFIG_NUMINTERRUPTS);
gic_shared_intrs = (gic_shared_intrs + 1) * 8;
+ /* Enable EIC mode if supported. */
+ mips_gic_enable_eic();
+
if (cpu_has_veic) {
/* Always use vector 1 in EIC mode */
gic_cpu_pin = 0;
--
2.7.4
In EIC mode, the GIC can assert 64 distinct interrupt sources in the
CPU. Until now, the GIC has routed all interrupt sources to a single CPU
interrupt source. This is inefficient since we end up checking both local
and shared interrupt flag registers for both local and shared
interrupts. This introduces additional latency into the interrupt paths
which is easy to remove in EIC mode by making use of an additional
vector for shared interrupt sources.
Signed-off-by: Matt Redfearn <[email protected]>
---
drivers/irqchip/irq-mips-gic.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index ee391f42e97d..541eae9a491d 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -52,6 +52,7 @@ static DEFINE_SPINLOCK(gic_lock);
static struct irq_domain *gic_irq_domain;
static struct irq_domain *gic_ipi_domain;
static int gic_shared_intrs;
+static unsigned int shared_cpu_pin;
static unsigned int gic_cpu_pin;
static unsigned int timer_cpu_pin;
static struct irq_chip gic_level_irq_controller, gic_edge_irq_controller;
@@ -406,6 +407,11 @@ static void __gic_irq_dispatch(void)
gic_handle_shared_int(false);
}
+static void __gic_irq_dispatch_shared(void)
+{
+ gic_handle_shared_int(false);
+}
+
static void gic_irq_dispatch(struct irq_desc *desc)
{
gic_handle_local_int(true);
@@ -422,7 +428,7 @@ static int gic_shared_irq_domain_map(struct irq_domain *d, unsigned int virq,
data = irq_get_irq_data(virq);
spin_lock_irqsave(&gic_lock, flags);
- write_gic_map_pin(intr, GIC_MAP_PIN_MAP_TO_PIN | gic_cpu_pin);
+ write_gic_map_pin(intr, GIC_MAP_PIN_MAP_TO_PIN | shared_cpu_pin);
write_gic_map_vp(intr, BIT(mips_cm_vp_id(cpu)));
gic_clear_pcpu_masks(intr);
set_bit(intr, per_cpu_ptr(pcpu_masks, cpu));
@@ -734,8 +740,13 @@ static int __init gic_of_init(struct device_node *node,
timer_cpu_pin = gic_cpu_pin;
set_vi_handler(gic_cpu_pin + GIC_PIN_TO_VEC_OFFSET,
__gic_irq_dispatch);
+
+ /* Route all shared interrupts to pin 1, vector 2 */
+ shared_cpu_pin = 1;
+ set_vi_handler(shared_cpu_pin + GIC_PIN_TO_VEC_OFFSET,
+ __gic_irq_dispatch_shared);
} else {
- gic_cpu_pin = cpu_vec - GIC_CPU_PIN_OFFSET;
+ shared_cpu_pin = gic_cpu_pin = cpu_vec - GIC_CPU_PIN_OFFSET;
irq_set_chained_handler(MIPS_CPU_IRQ_BASE + cpu_vec,
gic_irq_dispatch);
/*
--
2.7.4
The GIC driver now has the concept of multiple vectors for the shared
and local interrupts. The vector number for shared interrupt sources is
now in shared_cpu_pin and this effectively leaves gic_cpu_pin
containing the vector number for local interrupts. Rename it, and
additionally __gic_irq_dispatch to __gic_irq_dispatch_local, which is
only called for local interrupt sources now so no longer needs to handle
shared interrupts.
Signed-off-by: Matt Redfearn <[email protected]>
---
drivers/irqchip/irq-mips-gic.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 541eae9a491d..b2cfc6d66d74 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -53,7 +53,7 @@ static struct irq_domain *gic_irq_domain;
static struct irq_domain *gic_ipi_domain;
static int gic_shared_intrs;
static unsigned int shared_cpu_pin;
-static unsigned int gic_cpu_pin;
+static unsigned int local_cpu_pin;
static unsigned int timer_cpu_pin;
static struct irq_chip gic_level_irq_controller, gic_edge_irq_controller;
static DECLARE_BITMAP(ipi_resrv, GIC_MAX_INTRS);
@@ -401,10 +401,9 @@ static struct irq_chip gic_all_vpes_local_irq_controller = {
.irq_cpu_online = gic_all_vpes_irq_cpu_online,
};
-static void __gic_irq_dispatch(void)
+static void __gic_irq_dispatch_local(void)
{
gic_handle_local_int(false);
- gic_handle_shared_int(false);
}
static void __gic_irq_dispatch_shared(void)
@@ -482,7 +481,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
}
intr = GIC_HWIRQ_TO_LOCAL(hwirq);
- map = GIC_MAP_PIN_MAP_TO_PIN | gic_cpu_pin;
+ map = GIC_MAP_PIN_MAP_TO_PIN | local_cpu_pin;
switch (intr) {
case GIC_LOCAL_INT_TIMER:
@@ -735,18 +734,17 @@ static int __init gic_of_init(struct device_node *node,
mips_gic_enable_eic();
if (cpu_has_veic) {
- /* Always use vector 1 in EIC mode */
- gic_cpu_pin = 0;
- timer_cpu_pin = gic_cpu_pin;
- set_vi_handler(gic_cpu_pin + GIC_PIN_TO_VEC_OFFSET,
- __gic_irq_dispatch);
+ /* Route all local interrupts to pin 0, vector 1 */
+ timer_cpu_pin = local_cpu_pin = 0;
+ set_vi_handler(local_cpu_pin + GIC_PIN_TO_VEC_OFFSET,
+ __gic_irq_dispatch_local);
/* Route all shared interrupts to pin 1, vector 2 */
shared_cpu_pin = 1;
set_vi_handler(shared_cpu_pin + GIC_PIN_TO_VEC_OFFSET,
__gic_irq_dispatch_shared);
} else {
- shared_cpu_pin = gic_cpu_pin = cpu_vec - GIC_CPU_PIN_OFFSET;
+ shared_cpu_pin = local_cpu_pin = cpu_vec - GIC_CPU_PIN_OFFSET;
irq_set_chained_handler(MIPS_CPU_IRQ_BASE + cpu_vec,
gic_irq_dispatch);
/*
@@ -768,7 +766,7 @@ static int __init gic_of_init(struct device_node *node,
timer_cpu_pin,
gic_irq_dispatch);
} else {
- timer_cpu_pin = gic_cpu_pin;
+ timer_cpu_pin = local_cpu_pin;
}
}
--
2.7.4
On Fri, Jan 05, 2018 at 10:31:07AM +0000, Matt Redfearn wrote:
> The GIC supports running in External Interrupt Controller (EIC) mode,
> and will signal this via cpu_has_veic if enabled in hardware. Currently
> the generic kernel will panic if cpu_has_veic is set - but the GIC can
> legitimately set this flag if either configured to boot in EIC mode, or
> if the GIC driver enables this mode. Make the kernel not panic in this
> case, and instead just check if the GIC is present. If so, use it's CPU
> local interrupt routing functions. If an EIC is present, but it is not
> the GIC, then the kernel does not know how to get the VIRQ for the CPU
> local interrupts and should panic. Support for alternative EICs being
> present is needed here for the generic kernel to support them.
>
> Suggested-by: Paul Burton <[email protected]>
> Signed-off-by: Matt Redfearn <[email protected]>
Applied for 4.16,
Thanks
James
> ---
>
> arch/mips/generic/irq.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/mips/generic/irq.c b/arch/mips/generic/irq.c
> index 394f8161e462..cb7fdaeef426 100644
> --- a/arch/mips/generic/irq.c
> +++ b/arch/mips/generic/irq.c
> @@ -22,10 +22,10 @@ int get_c0_fdc_int(void)
> {
> int mips_cpu_fdc_irq;
>
> - if (cpu_has_veic)
> - panic("Unimplemented!");
> - else if (mips_gic_present())
> + if (mips_gic_present())
> mips_cpu_fdc_irq = gic_get_c0_fdc_int();
> + else if (cpu_has_veic)
> + panic("Unimplemented!");
> else if (cp0_fdc_irq >= 0)
> mips_cpu_fdc_irq = MIPS_CPU_IRQ_BASE + cp0_fdc_irq;
> else
> @@ -38,10 +38,10 @@ int get_c0_perfcount_int(void)
> {
> int mips_cpu_perf_irq;
>
> - if (cpu_has_veic)
> - panic("Unimplemented!");
> - else if (mips_gic_present())
> + if (mips_gic_present())
> mips_cpu_perf_irq = gic_get_c0_perfcount_int();
> + else if (cpu_has_veic)
> + panic("Unimplemented!");
> else if (cp0_perfcount_irq >= 0)
> mips_cpu_perf_irq = MIPS_CPU_IRQ_BASE + cp0_perfcount_irq;
> else
> @@ -54,10 +54,10 @@ unsigned int get_c0_compare_int(void)
> {
> int mips_cpu_timer_irq;
>
> - if (cpu_has_veic)
> - panic("Unimplemented!");
> - else if (mips_gic_present())
> + if (mips_gic_present())
> mips_cpu_timer_irq = gic_get_c0_compare_int();
> + else if (cpu_has_veic)
> + panic("Unimplemented!");
> else
> mips_cpu_timer_irq = MIPS_CPU_IRQ_BASE + cp0_compare_irq;
>
> --
> 2.7.4
>
>
Hi Matt,
On 05/01/18 10:31, Matt Redfearn wrote:
>
> This series enables the MIPS GIC driver to make use of the EIC mode
> supported in some MIPS cores. In this mode, the cores 6 interrupt lines
> are switched to represent a vector number, 0..63. Currently all GIC
> interrupts are routed to a single CPU interrupt pin, but this is
> inefficient since we end up checking both local and shared interrupt
> flag registers for both local and shared interrupts. This introduces
> additional latency into the interrupt paths. With EIC mode this can be
> improved by using separate vectors for local and shared interrupts.
>
> This series is based on 4.15-rc6 and has been tested on Boston, Malta &
> SEAD3 MIPS platforms implementing a GIC with and without EIC mode
> supported in hardware.
What the status of this series?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Wed, Mar 14, 2018 at 11:15:47AM +0000, Marc Zyngier wrote:
> Hi Matt,
>
> On 05/01/18 10:31, Matt Redfearn wrote:
> >
> > This series enables the MIPS GIC driver to make use of the EIC mode
> > supported in some MIPS cores. In this mode, the cores 6 interrupt lines
> > are switched to represent a vector number, 0..63. Currently all GIC
> > interrupts are routed to a single CPU interrupt pin, but this is
> > inefficient since we end up checking both local and shared interrupt
> > flag registers for both local and shared interrupts. This introduces
> > additional latency into the interrupt paths. With EIC mode this can be
> > improved by using separate vectors for local and shared interrupts.
> >
> > This series is based on 4.15-rc6 and has been tested on Boston, Malta &
> > SEAD3 MIPS platforms implementing a GIC with and without EIC mode
> > supported in hardware.
>
> What the status of this series?
FYI I've been meaning to test it with KVM, since host EIC I think will
affect KVM & guest stuff.
Cheers
James
On 14/03/18 15:46, James Hogan wrote:
> On Wed, Mar 14, 2018 at 11:15:47AM +0000, Marc Zyngier wrote:
>> Hi Matt,
>>
>> On 05/01/18 10:31, Matt Redfearn wrote:
>>>
>>> This series enables the MIPS GIC driver to make use of the EIC
>>> mode supported in some MIPS cores. In this mode, the cores 6
>>> interrupt lines are switched to represent a vector number,
>>> 0..63. Currently all GIC interrupts are routed to a single CPU
>>> interrupt pin, but this is inefficient since we end up checking
>>> both local and shared interrupt flag registers for both local
>>> and shared interrupts. This introduces additional latency into
>>> the interrupt paths. With EIC mode this can be improved by
>>> using separate vectors for local and shared interrupts.
>>>
>>> This series is based on 4.15-rc6 and has been tested on Boston,
>>> Malta & SEAD3 MIPS platforms implementing a GIC with and
>>> without EIC mode supported in hardware.
>>
>> What the status of this series?
>
> FYI I've been meaning to test it with KVM, since host EIC I think
> will affect KVM & guest stuff.
OK. I'll park that until I hear from you guys.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Fri, Jan 05, 2018 at 10:31:05AM +0000, Matt Redfearn wrote:
> The current location of ehb() in mipsmtregs.h does not make sense, since
> it is not strictly related to multi-threading, and may be used in code
> which does not include mipsmtregs.h
> arch/mips/include/asm/barrier.h | 13 +++++++++++++
> arch/mips/include/asm/mipsmtregs.h | 8 --------
But ehb isn't really a memory barrier like the other barriers in
barrier.h, its an execution hazard barrier, as used when available by
the hazard macros in hazards.h, and in fact there is already an _ehb()
there.
I suspect the intention was that most MIPS arch code using ehb would do
so using the appropriate hazard abstractions, which would do the right
number of NOPs on hardware without the EHB instruction. Code that is
specific to certain arch revisions (like the MIPS MT code and MIPS KVM)
can get away with using _ehb/ehb, but should use the abstractions where
they exist to make intentions clear.
None of the specific hazards in hazards.h really match the case in patch
2, I suppose you could have a new sync_mfc0_hazard() macro, but its so
specific and EHB should be guaranteed to exist there, so perhaps _ehb()
should just be used instead of ehb() there? (with a comment to describe
what operations are being protected from what hazards).
Cheers
James