KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Andre Przywara <andre.przywara@arm.com>,
	Andrew Jones <drjones@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH 12/17] arm: gic: Change gic_read_iar() to take group parameter
Date: Tue, 12 Nov 2019 17:19:36 +0000
Message-ID: <68cd4ae5-0d85-4300-2851-adb3f5af6243@arm.com> (raw)
In-Reply-To: <20191108144240.204202-13-andre.przywara@arm.com>

Hi,

On 11/8/19 2:42 PM, Andre Przywara wrote:
> Acknowledging a GIC group 0 interrupt requires us to use a different
> system register on GICv3. To allow us to differentiate the two groups
> later, add a group parameter to gic_read_iar(). For GICv2 we can use the
> same CPU interface register to acknowledge group 0 as well, so we ignore
> the parameter here.
>
> For now this is still using group 1 on every caller.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arm/gic.c                  |  4 ++--
>  arm/micro-bench.c          |  2 +-
>  arm/pl031.c                |  2 +-
>  arm/timer.c                |  2 +-
>  lib/arm/asm/arch_gicv3.h   | 11 +++++++++--
>  lib/arm/asm/gic-v2.h       |  2 +-
>  lib/arm/asm/gic-v3.h       |  2 +-
>  lib/arm/asm/gic.h          |  2 +-
>  lib/arm/gic-v2.c           |  3 ++-
>  lib/arm/gic.c              |  6 +++---
>  lib/arm64/asm/arch_gicv3.h | 10 ++++++++--
>  11 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/arm/gic.c b/arm/gic.c
> index a0511e5..7be13a6 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -156,7 +156,7 @@ static void check_irqnr(u32 irqnr, int expected)
>  
>  static void irq_handler(struct pt_regs *regs __unused)
>  {
> -	u32 irqstat = gic_read_iar();
> +	u32 irqstat = gic_read_iar(1);
>  	u32 irqnr = gic_iar_irqnr(irqstat);
>  
>  	if (irqnr == GICC_INT_SPURIOUS) {
> @@ -288,7 +288,7 @@ static struct gic gicv3 = {
>  
>  static void ipi_clear_active_handler(struct pt_regs *regs __unused)
>  {
> -	u32 irqstat = gic_read_iar();
> +	u32 irqstat = gic_read_iar(1);
>  	u32 irqnr = gic_iar_irqnr(irqstat);
>  
>  	if (irqnr != GICC_INT_SPURIOUS) {
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 4612f41..2bfee68 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -33,7 +33,7 @@ static void ipi_irq_handler(struct pt_regs *regs)
>  {
>  	ipi_ready = false;
>  	ipi_received = true;
> -	gic_write_eoir(gic_read_iar());
> +	gic_write_eoir(gic_read_iar(1));
>  	ipi_ready = true;
>  }
>  
> diff --git a/arm/pl031.c b/arm/pl031.c
> index 5672f36..5be3d76 100644
> --- a/arm/pl031.c
> +++ b/arm/pl031.c
> @@ -134,7 +134,7 @@ static void gic_irq_unmask(void)
>  
>  static void irq_handler(struct pt_regs *regs)
>  {
> -	u32 irqstat = gic_read_iar();
> +	u32 irqstat = gic_read_iar(1);
>  	u32 irqnr = gic_iar_irqnr(irqstat);
>  
>  	gic_write_eoir(irqstat);
> diff --git a/arm/timer.c b/arm/timer.c
> index 0b808d5..e5cc3b4 100644
> --- a/arm/timer.c
> +++ b/arm/timer.c
> @@ -150,7 +150,7 @@ static void set_timer_irq_enabled(struct timer_info *info, bool enabled)
>  static void irq_handler(struct pt_regs *regs)
>  {
>  	struct timer_info *info;
> -	u32 irqstat = gic_read_iar();
> +	u32 irqstat = gic_read_iar(1);
>  	u32 irqnr = gic_iar_irqnr(irqstat);
>  
>  	if (irqnr != GICC_INT_SPURIOUS)
> diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
> index 45b6096..52e7bba 100644
> --- a/lib/arm/asm/arch_gicv3.h
> +++ b/lib/arm/asm/arch_gicv3.h
> @@ -16,6 +16,7 @@
>  
>  #define ICC_PMR				__ACCESS_CP15(c4, 0, c6, 0)
>  #define ICC_SGI1R			__ACCESS_CP15_64(0, c12)
> +#define ICC_IAR0			__ACCESS_CP15(c12, 0,  c8, 0)
>  #define ICC_IAR1			__ACCESS_CP15(c12, 0, c12, 0)
>  #define ICC_EOIR1			__ACCESS_CP15(c12, 0, c12, 1)
>  #define ICC_IGRPEN1			__ACCESS_CP15(c12, 0, c12, 7)
> @@ -30,9 +31,15 @@ static inline void gicv3_write_sgi1r(u64 val)
>  	write_sysreg(val, ICC_SGI1R);
>  }
>  
> -static inline u32 gicv3_read_iar(void)
> +static inline u32 gicv3_read_iar(int group)
>  {
> -	u32 irqstat = read_sysreg(ICC_IAR1);
> +	u32 irqstat;
> +
> +	if (group == 0)
> +		irqstat = read_sysreg(ICC_IAR0);
> +	else
> +		irqstat = read_sysreg(ICC_IAR1);
> +
>  	dsb(sy);
>  	return irqstat;
>  }
> diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
> index 1fcfd43..d50c610 100644
> --- a/lib/arm/asm/gic-v2.h
> +++ b/lib/arm/asm/gic-v2.h
> @@ -32,7 +32,7 @@ extern struct gicv2_data gicv2_data;
>  
>  extern int gicv2_init(void);
>  extern void gicv2_enable_defaults(void);
> -extern u32 gicv2_read_iar(void);
> +extern u32 gicv2_read_iar(int group);
>  extern u32 gicv2_iar_irqnr(u32 iar);
>  extern void gicv2_write_eoir(u32 irqstat);
>  extern void gicv2_ipi_send_single(int irq, int cpu);
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> index 0a29610..ca19110 100644
> --- a/lib/arm/asm/gic-v3.h
> +++ b/lib/arm/asm/gic-v3.h
> @@ -69,7 +69,7 @@ extern struct gicv3_data gicv3_data;
>  
>  extern int gicv3_init(void);
>  extern void gicv3_enable_defaults(void);
> -extern u32 gicv3_read_iar(void);
> +extern u32 gicv3_read_iar(int group);
>  extern u32 gicv3_iar_irqnr(u32 iar);
>  extern void gicv3_write_eoir(u32 irqstat);
>  extern void gicv3_ipi_send_single(int irq, int cpu);
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> index 21cdb58..09663e7 100644
> --- a/lib/arm/asm/gic.h
> +++ b/lib/arm/asm/gic.h
> @@ -68,7 +68,7 @@ extern void gic_enable_defaults(void);
>   * below will work with any supported gic version.
>   */
>  extern int gic_version(void);
> -extern u32 gic_read_iar(void);
> +extern u32 gic_read_iar(int group);
>  extern u32 gic_iar_irqnr(u32 iar);
>  extern void gic_write_eoir(u32 irqstat);
>  extern void gic_ipi_send_single(int irq, int cpu);
> diff --git a/lib/arm/gic-v2.c b/lib/arm/gic-v2.c
> index dc6a97c..b60967e 100644
> --- a/lib/arm/gic-v2.c
> +++ b/lib/arm/gic-v2.c
> @@ -26,8 +26,9 @@ void gicv2_enable_defaults(void)
>  	writel(GICC_ENABLE, cpu_base + GICC_CTLR);
>  }
>  
> -u32 gicv2_read_iar(void)
> +u32 gicv2_read_iar(int group)
>  {
> +	/* GICv2 acks both group0 and group1 IRQs with the same register. */
>  	return readl(gicv2_cpu_base() + GICC_IAR);
>  }
>  
> diff --git a/lib/arm/gic.c b/lib/arm/gic.c
> index cf4e811..b51eff5 100644
> --- a/lib/arm/gic.c
> +++ b/lib/arm/gic.c
> @@ -12,7 +12,7 @@ struct gicv3_data gicv3_data;
>  
>  struct gic_common_ops {
>  	void (*enable_defaults)(void);
> -	u32 (*read_iar)(void);
> +	u32 (*read_iar)(int group);
>  	u32 (*iar_irqnr)(u32 iar);
>  	void (*write_eoir)(u32 irqstat);
>  	void (*ipi_send_single)(int irq, int cpu);
> @@ -117,10 +117,10 @@ void gic_enable_defaults(void)
>  	gic_common_ops->enable_defaults();
>  }
>  
> -u32 gic_read_iar(void)
> +u32 gic_read_iar(int group)
>  {
>  	assert(gic_common_ops && gic_common_ops->read_iar);
> -	return gic_common_ops->read_iar();
> +	return gic_common_ops->read_iar(group);
>  }
>  
>  u32 gic_iar_irqnr(u32 iar)
> diff --git a/lib/arm64/asm/arch_gicv3.h b/lib/arm64/asm/arch_gicv3.h
> index a7994ec..876e1fc 100644
> --- a/lib/arm64/asm/arch_gicv3.h
> +++ b/lib/arm64/asm/arch_gicv3.h
> @@ -11,6 +11,7 @@
>  #include <asm/sysreg.h>
>  
>  #define ICC_PMR_EL1			sys_reg(3, 0, 4, 6, 0)
> +#define ICC_IAR0_EL1			sys_reg(3, 0, 12, 8, 0)
>  #define ICC_SGI1R_EL1			sys_reg(3, 0, 12, 11, 5)
>  #define ICC_IAR1_EL1			sys_reg(3, 0, 12, 12, 0)
>  #define ICC_EOIR1_EL1			sys_reg(3, 0, 12, 12, 1)
> @@ -38,10 +39,15 @@ static inline void gicv3_write_sgi1r(u64 val)
>  	asm volatile("msr_s " xstr(ICC_SGI1R_EL1) ", %0" : : "r" (val));
>  }
>  
> -static inline u32 gicv3_read_iar(void)
> +static inline u32 gicv3_read_iar(int group)
>  {
>  	u64 irqstat;
> -	asm volatile("mrs_s %0, " xstr(ICC_IAR1_EL1) : "=r" (irqstat));
> +
> +	if (group == 0)
> +		asm volatile("mrs_s %0, " xstr(ICC_IAR0_EL1) : "=r" (irqstat));
> +	else
> +		asm volatile("mrs_s %0, " xstr(ICC_IAR1_EL1) : "=r" (irqstat));
> +
>  	dsb(sy);
>  	return (u64)irqstat;
>  }

I'm not sure this is the best approach. Now every test that happens to use the gic
has to know about interrupt groups. Have you considered implementing the functions
that you need for the test in arm/gic.c? Andrew, what do you think?

Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 14:42 [kvm-unit-tests PATCH 00/17] arm: gic: Test SPIs and interrupt groups Andre Przywara
2019-11-08 14:42 ` [kvm-unit-tests PATCH 01/17] arm: gic: Enable GIC MMIO tests for GICv3 as well Andre Przywara
2019-11-08 17:28   ` Alexandru Elisei
2019-11-12 12:49   ` Auger Eric
2019-11-08 14:42 ` [kvm-unit-tests PATCH 02/17] arm: gic: Generalise function names Andre Przywara
2019-11-12 11:11   ` Alexandru Elisei
2019-11-12 12:49   ` Auger Eric
2019-11-08 14:42 ` [kvm-unit-tests PATCH 03/17] arm: gic: Provide per-IRQ helper functions Andre Przywara
2019-11-12 12:51   ` Alexandru Elisei
2019-11-12 15:53     ` Auger Eric
2019-11-12 16:53       ` Alexandru Elisei
2019-11-12 13:49   ` Auger Eric
2019-11-08 14:42 ` [kvm-unit-tests PATCH 04/17] arm: gic: Support no IRQs test case Andre Przywara
2019-11-12 13:26   ` Alexandru Elisei
2019-11-12 21:14     ` Auger Eric
2019-11-08 14:42 ` [kvm-unit-tests PATCH 05/17] arm: gic: Prepare IRQ handler for handling SPIs Andre Przywara
2019-11-12 13:36   ` Alexandru Elisei
2019-11-12 20:56   ` Auger Eric
2019-11-08 14:42 ` [kvm-unit-tests PATCH 06/17] arm: gic: Add simple shared IRQ test Andre Przywara
2019-11-12 13:54   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 07/17] arm: gic: Extend check_acked() to allow silent call Andre Przywara
2019-11-12 15:23   ` Alexandru Elisei
2019-11-14 12:32     ` Andrew Jones
2019-11-15 11:32       ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 08/17] arm: gic: Add simple SPI MP test Andre Przywara
2019-11-12 15:41   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 09/17] arm: gic: Add test for flipping GICD_CTLR.DS Andre Przywara
2019-11-12 16:42   ` Alexandru Elisei
2019-11-14 13:39     ` Vladimir Murzin
2019-11-14 14:17       ` Andre Przywara
2019-11-14 14:50         ` Vladimir Murzin
2019-11-14 15:21           ` Alexandru Elisei
2019-11-14 15:27             ` Peter Maydell
2019-11-14 15:47               ` Alexandru Elisei
2019-11-14 15:56                 ` Peter Maydell
2019-11-08 14:42 ` [kvm-unit-tests PATCH 10/17] arm: gic: Check for writable IGROUPR registers Andre Przywara
2019-11-12 16:51   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 11/17] arm: gic: Check for validity of both group enable bits Andre Przywara
2019-11-12 16:58   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 12/17] arm: gic: Change gic_read_iar() to take group parameter Andre Przywara
2019-11-12 17:19   ` Alexandru Elisei [this message]
2019-11-14 12:50     ` Andrew Jones
2019-11-08 14:42 ` [kvm-unit-tests PATCH 13/17] arm: gic: Change write_eoir() " Andre Przywara
2019-11-08 14:42 ` [kvm-unit-tests PATCH 14/17] arm: gic: Prepare for receiving GIC group 0 interrupts via FIQs Andre Przywara
2019-11-12 17:30   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 15/17] arm: gic: Provide FIQ handler Andre Przywara
2019-11-13 10:14   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 16/17] arm: gic: Prepare interrupt statistics for both groups Andre Przywara
2019-11-13 10:44   ` Alexandru Elisei
2019-11-08 14:42 ` [kvm-unit-tests PATCH 17/17] arm: gic: Test Group0 SPIs Andre Przywara
2019-11-13 11:26   ` Alexandru Elisei

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68cd4ae5-0d85-4300-2851-adb3f5af6243@arm.com \
    --to=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git