| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 18 Aug 2017 12:10:20 +0100
From: Marc Zyngier <marc.zyngier@....com>
To: Paul Burton <paul.burton@...tec.com>, linux-mips@...ux-mips.org,
Jason Cooper <jason@...edaemon.net>,
Thomas Gleixner <tglx@...utronix.de>
Cc: Ralf Baechle <ralf@...ux-mips.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/38] MIPS: GIC: Introduce asm/mips-gic.h with accessor
functions
On 13/08/17 05:36, Paul Burton wrote:
> This patch introduces a new header providing accessor functions for the
> MIPS Global Interrupt Controller (GIC) mirroring those provided for the
> other 2 components of the MIPS Coherent Processing System (CPS) - the
> Coherence Manager (CM) & Cluster Power Controller (CPC).
>
> This header makes use of the new standardised CPS accessor macros where
> possible, but does require some custom accessors for cases where we have
> either a bit or a register per interrupt.
>
> A major advantage of this over the existing
> include/linux/irqchip/mips-gic.h definitions is that code performing
> accesses can become much simpler, for example this:
>
> gic_update_bits(GIC_REG(SHARED, GIC_SH_SET_TRIGGER) +
> GIC_INTR_OFS(intr), 1ul << GIC_INTR_BIT(intr),
> (unsigned long)trig << GIC_INTR_BIT(intr));
>
> ...can become simply:
>
> change_gic_trig(intr, trig);
>
> The accessors handle 32 vs 64 bit in the same way as for CM & CPC code,
> which means that GIC code will also not need to worry about the access
> size in most cases. They are also accessible outside of
> drivers/irqchip/irq-mips-gic.c which will allow for simplification in
> the use of the non-interrupt portions of the GIC (eg. counters) which
> currently require the interrupt controller driver to expose helper
> functions for access.
>
> This patch doesn't change any existing code over to use the new
> accessors yet, since a wholesale change would be invasive & difficult to
> review. Instead follow-on patches will convert code piecemeal to use
> this new header. The one change to existing code is to rename gic_base
> to mips_gic_base & make it global, in order to fit in with the naming
> expected by the standardised CPS accessor macros.
>
> Signed-off-by: Paul Burton <paul.burton@...tec.com>
> Cc: Jason Cooper <jason@...edaemon.net>
> Cc: Marc Zyngier <marc.zyngier@....com>
> Cc: Ralf Baechle <ralf@...ux-mips.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-mips@...ux-mips.org
> ---
>
> arch/mips/include/asm/mips-cps.h | 1 +
> arch/mips/include/asm/mips-gic.h | 293 +++++++++++++++++++++++++++++++++++++++
> drivers/irqchip/irq-mips-gic.c | 13 +-
> 3 files changed, 300 insertions(+), 7 deletions(-)
> create mode 100644 arch/mips/include/asm/mips-gic.h
>
> diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
> index 2dd737d803e1..bf02b5070a98 100644
> --- a/arch/mips/include/asm/mips-cps.h
> +++ b/arch/mips/include/asm/mips-cps.h
> @@ -107,6 +107,7 @@ static inline void clear_##unit##_##name(uint##sz##_t val) \
>
> #include <asm/mips-cm.h>
> #include <asm/mips-cpc.h>
> +#include <asm/mips-gic.h>
>
> /**
> * mips_cps_numclusters - return the number of clusters present in the system
> diff --git a/arch/mips/include/asm/mips-gic.h b/arch/mips/include/asm/mips-gic.h
> new file mode 100644
> index 000000000000..8cf4bdc1a059
> --- /dev/null
> +++ b/arch/mips/include/asm/mips-gic.h
> @@ -0,0 +1,293 @@
> +/*
> + * Copyright (C) 2017 Imagination Technologies
> + * Author: Paul Burton <paul.burton@...tec.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#ifndef __MIPS_ASM_MIPS_CPS_H__
> +# error Please include asm/mips-cps.h rather than asm/mips-gic.h
> +#endif
> +
> +#ifndef __MIPS_ASM_MIPS_GIC_H__
> +#define __MIPS_ASM_MIPS_GIC_H__
> +
> +#include <linux/bitops.h>
> +
> +/* The base address of the GIC registers */
> +extern void __iomem *mips_gic_base;
> +
> +/* Offsets from the GIC base address to various control blocks */
> +#define MIPS_GIC_SHARED_OFS 0x00000
> +#define MIPS_GIC_SHARED_SZ 0x08000
> +#define MIPS_GIC_LOCAL_OFS 0x08000
> +#define MIPS_GIC_LOCAL_SZ 0x04000
> +#define MIPS_GIC_REDIR_OFS 0x0c000
> +#define MIPS_GIC_REDIR_SZ 0x04000
> +#define MIPS_GIC_USER_OFS 0x10000
> +#define MIPS_GIC_USER_SZ 0x10000
> +
> +/* For read-only shared registers */
> +#define GIC_ACCESSOR_RO(sz, off, name) \
> + CPS_ACCESSOR_RO(gic, sz, MIPS_GIC_SHARED_OFS + off, name)
> +
> +/* For read-write shared registers */
> +#define GIC_ACCESSOR_RW(sz, off, name) \
> + CPS_ACCESSOR_RW(gic, sz, MIPS_GIC_SHARED_OFS + off, name)
> +
> +/* For read-only local registers */
> +#define GIC_VX_ACCESSOR_RO(sz, off, name) \
> + CPS_ACCESSOR_RO(gic, sz, MIPS_GIC_LOCAL_OFS + off, vl_##name) \
> + CPS_ACCESSOR_RO(gic, sz, MIPS_GIC_REDIR_OFS + off, vo_##name)
> +
> +/* For read-write local registers */
> +#define GIC_VX_ACCESSOR_RW(sz, off, name) \
> + CPS_ACCESSOR_RW(gic, sz, MIPS_GIC_LOCAL_OFS + off, vl_##name) \
> + CPS_ACCESSOR_RW(gic, sz, MIPS_GIC_REDIR_OFS + off, vo_##name)
> +
> +/* For read-only shared per-interrupt registers */
> +#define GIC_ACCESSOR_RO_INTR_REG(sz, off, stride, name) \
> +static inline void __iomem *addr_gic_##name(unsigned int intr) \
> +{ \
> + return mips_gic_base + (off) + (intr * (stride)); \
> +} \
> + \
> +static inline unsigned int read_gic_##name(unsigned int intr) \
> +{ \
> + BUILD_BUG_ON(sz != 32); \
> + return __raw_readl(addr_gic_##name(intr)); \
> +}
> +
> +/* For read-write shared per-interrupt registers */
> +#define GIC_ACCESSOR_RW_INTR_REG(sz, off, stride, name) \
> + GIC_ACCESSOR_RO_INTR_REG(sz, off, stride, name) \
> + \
> +static inline void write_gic_##name(unsigned int intr, \
> + unsigned int val) \
> +{ \
> + BUILD_BUG_ON(sz != 32); \
> + __raw_writel(val, addr_gic_##name(intr)); \
> +}
> +
> +/* For read-only local per-interrupt registers */
> +#define GIC_VX_ACCESSOR_RO_INTR_REG(sz, off, stride, name) \
> + GIC_ACCESSOR_RO_INTR_REG(sz, MIPS_GIC_LOCAL_OFS + off, \
> + stride, vl_##name) \
> + GIC_ACCESSOR_RO_INTR_REG(sz, MIPS_GIC_REDIR_OFS + off, \
> + stride, vo_##name)
> +
> +/* For read-write local per-interrupt registers */
> +#define GIC_VX_ACCESSOR_RW_INTR_REG(sz, off, stride, name) \
> + GIC_ACCESSOR_RW_INTR_REG(sz, MIPS_GIC_LOCAL_OFS + off, \
> + stride, vl_##name) \
> + GIC_ACCESSOR_RW_INTR_REG(sz, MIPS_GIC_REDIR_OFS + off, \
> + stride, vo_##name)
> +
> +/* For read-only shared bit-per-interrupt registers */
> +#define GIC_ACCESSOR_RO_INTR_BIT(off, name) \
> +static inline void __iomem *addr_gic_##name(void) \
> +{ \
> + return mips_gic_base + (off); \
> +} \
> + \
> +static inline unsigned int read_gic_##name(unsigned int intr) \
> +{ \
> + void __iomem *addr = addr_gic_##name(); \
> + unsigned int val; \
> + \
> + if (mips_cm_is64) { \
> + addr += (intr / 64) * sizeof(uint64_t); \
> + val = __raw_readq(addr) >> intr % 64; \
> + } else { \
> + addr += (intr / 32) * sizeof(uint32_t); \
> + val = __raw_readl(addr) >> intr % 32; \
> + } \
> + \
> + return val & 0x1; \
> +}
> +
> +/* For read-write shared bit-per-interrupt registers */
> +#define GIC_ACCESSOR_RW_INTR_BIT(off, name) \
> + GIC_ACCESSOR_RO_INTR_BIT(off, name) \
> + \
> +static inline void write_gic_##name(unsigned int intr) \
> +{ \
> + void __iomem *addr = addr_gic_##name(); \
> + \
> + if (mips_cm_is64) { \
Given that you now refer to this symbol, wouldn't it be best to include
asm/mips-cm.h here? It is probably included via some other path, but
it'd make this file standalone.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Powered by blists - more mailing lists