[Xenomai] [I-PIPE] [Patch] IRQ pipelining support for GICv3

[Xenomai] [I-PIPE] [Patch] IRQ pipelining support for GICv3

[Christoph Müllner]

Hi Xenomai/I-pipe devs,

we have I-pipe/Cobalt/Xenomai running here on a Rockchip RK3399, which is a six core ARMv8/aarch64 SoC.
Our current kernel is based on vanilla Linux 4.12.10 and we've applied the I-pipe patches for 4.11-arm64.
Of course there were some conflicts, but we managed to resolve them quickly.
On top of that we are running Xenomai 3.1 (next) in dual-kernel configuration (Cobalt).
xeno-test did not show any errors/fails and we've successfully run latency for more than 72 hours in a row.

The RK3399 has an ARM GICv3 interrupt controller. Unfortunately the I-pipe patches don't include
support for that controller. Therefore I'd like to contribute a patch for that (see attachment).

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ipipe-gicv3-Enable-interrupt-pipelining.patch
Type: application/octet-stream
Size: 1890 bytes
Desc: not available
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171009/d69b8f01/attachment.obj>
-------------- next part --------------


Thanks,
Christoph

--
Christoph M?llner
Theobroma Systems Design und Consulting GmbH
Seestadtstra?e 27 (Aspern IQ), 1220 Wien, Austria
Phone: +43 1 236 98 93-409, Fax: +43 1 236 98 93-9
http://www.theobroma-systems.com




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171009/d69b8f01/attachment.sig>

Re: [Xenomai] [I-PIPE] [Patch] IRQ pipelining support for GICv3

[Dmitriy Cherkasov]

On 10/09/2017 02:22 AM, Christoph Müllner wrote:
> Hi Xenomai/I-pipe devs,
> 
> we have I-pipe/Cobalt/Xenomai running here on a Rockchip RK3399, which is a six core ARMv8/aarch64 SoC.
> Our current kernel is based on vanilla Linux 4.12.10 and we've applied the I-pipe patches for 4.11-arm64.
> Of course there were some conflicts, but we managed to resolve them quickly.
> On top of that we are running Xenomai 3.1 (next) in dual-kernel configuration (Cobalt).
> xeno-test did not show any errors/fails and we've successfully run latency for more than 72 hours in a row.
> 
> The RK3399 has an ARM GICv3 interrupt controller. Unfortunately the I-pipe patches don't include
> support for that controller. Therefore I'd like to contribute a patch for that (see attachment).
> 
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: 0001-ipipe-gicv3-Enable-interrupt-pipelining.patch
> Type: application/octet-stream
> Size: 1890 bytes
> Desc: not available
> URL: <http://xenomai.org/pipermail/xenomai/attachments/20171009/d69b8f01/attachment.obj>


Shouldn't ipipe_lock/unlock_irq() be added to the mask/unmask functions as well? 
Otherwise hold/release don't do anything special.

Re: [Xenomai] [I-PIPE] [Patch] IRQ pipelining support for GICv3

[Christoph Müllner]

Hi Dmitry,

thanks for the hint.
I've attached a new version of the patch including the I-pipe locking and
the forwarding calls for locking/unlocking the IRQ in case of masking/unmasking.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ipipe-gicv3-Enable-interrupt-pipelining.patch
Type: application/octet-stream
Size: 3285 bytes
Desc: not available
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171010/3cc04ce6/attachment.obj>
-------------- next part --------------


Thanks,
Christoph

--
Christoph M?llner
Theobroma Systems Design und Consulting GmbH
Seestadtstra?e 27 (Aspern IQ), 1220 Wien, Austria
Phone: +43 1 236 98 93-409, Fax: +43 1 236 98 93-9
http://www.theobroma-systems.com




> On 10 Oct 2017, at 01:10, Dmitriy Cherkasov <dmitriy@oss-tech.org> wrote:
> 
> On 10/09/2017 02:22 AM, Christoph M?llner wrote:
>> Hi Xenomai/I-pipe devs,
>> we have I-pipe/Cobalt/Xenomai running here on a Rockchip RK3399, which is a six core ARMv8/aarch64 SoC.
>> Our current kernel is based on vanilla Linux 4.12.10 and we've applied the I-pipe patches for 4.11-arm64.
>> Of course there were some conflicts, but we managed to resolve them quickly.
>> On top of that we are running Xenomai 3.1 (next) in dual-kernel configuration (Cobalt).
>> xeno-test did not show any errors/fails and we've successfully run latency for more than 72 hours in a row.
>> The RK3399 has an ARM GICv3 interrupt controller. Unfortunately the I-pipe patches don't include
>> support for that controller. Therefore I'd like to contribute a patch for that (see attachment).
>> -------------- next part --------------
>> A non-text attachment was scrubbed...
>> Name: 0001-ipipe-gicv3-Enable-interrupt-pipelining.patch
>> Type: application/octet-stream
>> Size: 1890 bytes
>> Desc: not available
>> URL: <http://xenomai.org/pipermail/xenomai/attachments/20171009/d69b8f01/attachment.obj>
> 
> 
> Shouldn't ipipe_lock/unlock_irq() be added to the mask/unmask functions as well? Otherwise hold/release don't do anything special.
> 
> _______________________________________________
> Xenomai mailing list
> Xenomai@xenomai.org
> https://xenomai.org/mailman/listinfo/xenomai

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171010/3cc04ce6/attachment.sig>

Re: [Xenomai] [I-PIPE] [Patch] IRQ pipelining support for GICv3

[Jan Kiszka]

Hi Christoph,

On 2017-10-10 12:59, Christoph Müllner wrote:
> Hi Dmitry,
> 
> thanks for the hint.
> I've attached a new version of the patch including the I-pipe locking and
> the forwarding calls for locking/unlocking the IRQ in case of masking/unmasking.
> 

Is it possible with your mail client to inline patches? That would
simplify the reviews.

[...]
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 9e14388403b9..e68a0589889c 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -59,6 +59,14 @@ struct gic_chip_data {
>  	struct partition_desc	*ppi_descs[16];
>  };
>  
> +#ifdef CONFIG_IPIPE
> +#define pipeline_lock(__flags)		do { (__flags) = hard_local_irq_save(); } while (0)
> +#define pipeline_unlock(__flags)	hard_local_irq_restore(__flags)
> +#else
> +#define pipeline_lock(__flags)		do { (void)__flags; } while (0)
> +#define pipeline_unlock(__flags)	do { (void)__flags; } while (0)
> +#endif
> +

That are hard_cond_* wrappers that should be usable instead of a custom
solution.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

[Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Christoph Muellner]

This patch enables interrupt pipelining for ARM/ARM64 SoC with a
GICv3 interrupt controller.

The patch was tested on a Rockchip RK3399 (ARM64 SoC) with
Linux 4.12.14, I-pipe 4.11-arm64 and Xenomai/Cobalt 3.1 (next).
xeno-test did not show any errors/fails and latency ran
72 hours in a row.

[v2]: Add I-pipe locking and the forwarding calls for locking/unlocking
the IRQ in case of masking/unmasking.

[v3]: Using hard_cond_* instead of #ifdef'd hard_local_*.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/irqchip/irq-gic-v3.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 9e14388403b9..381a35eb3158 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -200,7 +200,12 @@ static void gic_poke_irq(struct irq_data *d, u32 offset)
 
 static void gic_mask_irq(struct irq_data *d)
 {
+	unsigned long flags;
+
+	flags = hard_cond_local_irq_save();
+	ipipe_lock_irq(d->irq);
 	gic_poke_irq(d, GICD_ICENABLER);
+	hard_cond_local_irq_restore(flags);
 }
 
 static void gic_eoimode1_mask_irq(struct irq_data *d)
@@ -220,7 +225,12 @@ static void gic_eoimode1_mask_irq(struct irq_data *d)
 
 static void gic_unmask_irq(struct irq_data *d)
 {
+	unsigned long flags;
+
+	flags = hard_cond_local_irq_save();
 	gic_poke_irq(d, GICD_ISENABLER);
+	ipipe_unlock_irq(d->irq);
+	hard_cond_local_irq_restore(flags);
 }
 
 static int gic_irq_set_irqchip_state(struct irq_data *d,
@@ -294,6 +304,27 @@ static void gic_eoimode1_eoi_irq(struct irq_data *d)
 	gic_write_dir(gic_irq(d));
 }
 
+#ifdef CONFIG_IPIPE
+static void gic_hold_irq(struct irq_data *d)
+{
+	struct irq_chip *chip = irq_data_get_irq_chip(d);
+
+	gic_poke_irq(d, GICD_ICENABLER);
+
+	if (chip->irq_eoi == gic_eoimode1_eoi_irq) {
+		if (irqd_is_forwarded_to_vcpu(d))
+			gic_poke_irq(d, GICD_ICACTIVER);
+		gic_eoimode1_eoi_irq(d);
+	} else
+		gic_eoi_irq(d);
+}
+
+static void gic_release_irq(struct irq_data *d)
+{
+	gic_poke_irq(d, GICD_ISENABLER);
+}
+#endif /* CONFIG_IPIPE */
+
 static int gic_set_type(struct irq_data *d, unsigned int type)
 {
 	unsigned int irq = gic_irq(d);
@@ -717,6 +748,10 @@ static struct irq_chip gic_chip = {
 	.irq_unmask		= gic_unmask_irq,
 	.irq_eoi		= gic_eoi_irq,
 	.irq_set_type		= gic_set_type,
+#ifdef CONFIG_IPIPE
+	.irq_hold		= gic_hold_irq,
+	.irq_release		= gic_release_irq,
+#endif
 	.irq_set_affinity	= gic_set_affinity,
 	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
 	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
@@ -729,6 +764,10 @@ static struct irq_chip gic_eoimode1_chip = {
 	.irq_unmask		= gic_unmask_irq,
 	.irq_eoi		= gic_eoimode1_eoi_irq,
 	.irq_set_type		= gic_set_type,
+#ifdef CONFIG_IPIPE
+	.irq_hold		= gic_hold_irq,
+	.irq_release		= gic_release_irq,
+#endif
 	.irq_set_affinity	= gic_set_affinity,
 	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
 	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
-- 
2.11.0

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Dmitriy Cherkasov]

On 10/12/2017 05:49 AM, Christoph Muellner wrote:
> This patch enables interrupt pipelining for ARM/ARM64 SoC with a
> GICv3 interrupt controller.
> 
> The patch was tested on a Rockchip RK3399 (ARM64 SoC) with
> Linux 4.12.14, I-pipe 4.11-arm64 and Xenomai/Cobalt 3.1 (next).
> xeno-test did not show any errors/fails and latency ran
> 72 hours in a row.
> 
> [v2]: Add I-pipe locking and the forwarding calls for locking/unlocking
> the IRQ in case of masking/unmasking.
> 
> [v3]: Using hard_cond_* instead of #ifdef'd hard_local_*.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---


Thanks Christoph, this is much easier to review.

>   
> +#ifdef CONFIG_IPIPE
> +static void gic_hold_irq(struct irq_data *d)
> +{
> +	struct irq_chip *chip = irq_data_get_irq_chip(d);
> +
> +	gic_poke_irq(d, GICD_ICENABLER);
> +
> +	if (chip->irq_eoi == gic_eoimode1_eoi_irq) {
> +		if (irqd_is_forwarded_to_vcpu(d))
> +			gic_poke_irq(d, GICD_ICACTIVER);
> +		gic_eoimode1_eoi_irq(d);
> +	} else
> +		gic_eoi_irq(d);
> +}
> +
> +static void gic_release_irq(struct irq_data *d)
> +{
> +	gic_poke_irq(d, GICD_ISENABLER);
> +}
> +#endif /* CONFIG_IPIPE */
> +

For these, I'm curious why not just do what you had in your first patch:

+#ifdef CONFIG_IPIPE
+static void gic_hold_irq(struct irq_data *d)
+{
+	struct irq_chip *chip = irq_data_get_irq_chip(d);
+	chip->irq_mask(d);
+	chip->irq_eoi(d);
+}
+
+static void gic_release_irq(struct irq_data *d)
+{
+	gic_unmask_irq(d);
+}
+#endif /* CONFIG_IPIPE */
+

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Christoph Müllner]

> On 12 Oct 2017, at 20:02, Dmitriy Cherkasov <dmitriy@oss-tech.org> wrote:
> 
> On 10/12/2017 05:49 AM, Christoph Muellner wrote:
>> This patch enables interrupt pipelining for ARM/ARM64 SoC with a
>> GICv3 interrupt controller.
>> The patch was tested on a Rockchip RK3399 (ARM64 SoC) with
>> Linux 4.12.14, I-pipe 4.11-arm64 and Xenomai/Cobalt 3.1 (next).
>> xeno-test did not show any errors/fails and latency ran
>> 72 hours in a row.
>> [v2]: Add I-pipe locking and the forwarding calls for locking/unlocking
>> the IRQ in case of masking/unmasking.
>> [v3]: Using hard_cond_* instead of #ifdef'd hard_local_*.
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> ---
> 
> 
> Thanks Christoph, this is much easier to review.
> 
>>  +#ifdef CONFIG_IPIPE
>> +static void gic_hold_irq(struct irq_data *d)
>> +{
>> +	struct irq_chip *chip = irq_data_get_irq_chip(d);
>> +
>> +	gic_poke_irq(d, GICD_ICENABLER);
>> +
>> +	if (chip->irq_eoi == gic_eoimode1_eoi_irq) {
>> +		if (irqd_is_forwarded_to_vcpu(d))
>> +			gic_poke_irq(d, GICD_ICACTIVER);
>> +		gic_eoimode1_eoi_irq(d);
>> +	} else
>> +		gic_eoi_irq(d);
>> +}
>> +
>> +static void gic_release_irq(struct irq_data *d)
>> +{
>> +	gic_poke_irq(d, GICD_ISENABLER);
>> +}
>> +#endif /* CONFIG_IPIPE */
>> +
> 
> For these, I'm curious why not just do what you had in your first patch:
> 
> +#ifdef CONFIG_IPIPE
> +static void gic_hold_irq(struct irq_data *d)
> +{
> +	struct irq_chip *chip = irq_data_get_irq_chip(d);
> +	chip->irq_mask(d);
> +	chip->irq_eoi(d);
> +}
> +
> +static void gic_release_irq(struct irq_data *d)
> +{
> +	gic_unmask_irq(d);
> +}
> +#endif /* CONFIG_IPIPE */
> +

That was my first attempt, but that ends up in a hang during bootup.
I suspect that the .irq_hold() callback must not call ipipe_lock_irq().
And since I added that call to gic_mask_irq(), I cannot call .irq_mask().

Note, that this code equals also the GICv2 implementation in drivers/irqchip/irq-gic.c
(where I got the idea for the pipeline_lock() macros in [v2] from).

Thanks,
Christoph


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171012/1c0ca94a/attachment.sig>

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Dmitriy Cherkasov]

On 10/12/2017 11:16 AM, Christoph Müllner wrote:

> That was my first attempt, but that ends up in a hang during bootup.
> I suspect that the .irq_hold() callback must not call ipipe_lock_irq().
> And since I added that call to gic_mask_irq(), I cannot call .irq_mask().
> 
> Note, that this code equals also the GICv2 implementation in drivers/irqchip/irq-gic.c
> (where I got the idea for the pipeline_lock() macros in [v2] from).
> 
> Thanks,
> Christoph
> 
> 

Yes, that's correct - lock/unlock definitely should not be called from hold/release.

This looks ok to me. Thanks!

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Christoph Müllner]

Is there anything else I can do to get this merged?

Thanks,
Christoph


> On 12 Oct 2017, at 20:59, Dmitriy Cherkasov <dmitriy@oss-tech.org> wrote:
> 
> On 10/12/2017 11:16 AM, Christoph Müllner wrote:
> 
>> That was my first attempt, but that ends up in a hang during bootup.
>> I suspect that the .irq_hold() callback must not call ipipe_lock_irq().
>> And since I added that call to gic_mask_irq(), I cannot call .irq_mask().
>> Note, that this code equals also the GICv2 implementation in drivers/irqchip/irq-gic.c
>> (where I got the idea for the pipeline_lock() macros in [v2] from).
>> Thanks,
>> Christoph
> 
> Yes, that's correct - lock/unlock definitely should not be called from hold/release.
> 
> This looks ok to me. Thanks!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP
URL: <http://xenomai.org/pipermail/xenomai/attachments/20171019/dd80e626/attachment.sig>

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Philippe Gerum]

On 10/12/2017 02:49 PM, Christoph Muellner wrote:
> This patch enables interrupt pipelining for ARM/ARM64 SoC with a
> GICv3 interrupt controller.
> 
> The patch was tested on a Rockchip RK3399 (ARM64 SoC) with
> Linux 4.12.14, I-pipe 4.11-arm64 and Xenomai/Cobalt 3.1 (next).
> xeno-test did not show any errors/fails and latency ran
> 72 hours in a row.
> 
> [v2]: Add I-pipe locking and the forwarding calls for locking/unlocking
> the IRQ in case of masking/unmasking.
> 
> [v3]: Using hard_cond_* instead of #ifdef'd hard_local_*.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>

Merged, thanks.

-- 
Philippe.

Re: [Xenomai] [PATCH] ipipe: gicv3: [v3] Enable interrupt pipelining.

[Philippe Gerum]

On 10/19/2017 10:43 AM, Christoph Müllner wrote:
> Is there anything else I can do to get this merged?

All good. Merged, thanks.

-- 
Philippe.