linux-kernel - Re: [PATCH] ARM: bcm281xx: Add L2 support for Rev A2 chips

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 1 May 2013 11:37:18 +0100
From:	Will Deacon <will.deacon@....com>
To:	Christian Daudt <csd@...adcom.com>
Cc:	Russell King <linux@....linux.org.uk>,
	John Stultz <johnstul@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Olof Johansson <olof@...om.net>, Arnd Bergmann <arnd@...db.de>,
	Stephen Warren <swarren@...dotorg.org>,
	"arm@...nel.org" <arm@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"csd_b@...dt.org" <csd_b@...dt.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Josh Cartwright <josh.cartwright@...com>,
	Yehuda Yitschak <yehuday@...vell.com>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	John Stultz <john.stultz@...aro.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: bcm281xx: Add L2 support for Rev A2 chips

Hi Christian,

Thanks for CC'ing me.

On Tue, Apr 30, 2013 at 07:38:09PM +0100, Christian Daudt wrote:
> Rev A2 SoCs have an unorthodox memory re-mapping and this needs
> to be reflected in the cache operations.
> This patch adds new outer cache functions for the l2x0 driver
> to support this SoC revision. It also adds a new compatible
> value for the cache to enable this functionality.

This is a pretty weird thing you've managed to build here...

> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index c465fac..6edba13 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -523,6 +523,162 @@ static void aurora_flush_range(unsigned long start, unsigned long end)
>  	}
>  }
>  
> +/*
> + * For certain Broadcom SoCs, depending on the address range, different offsets
> + * need to be added to the address before passing it to L2 for
> + * invalidation/clean/flush
> + *
> + * Section Address Range              Offset        EMI
> + *   1     0x00000000 - 0x3FFFFFFF    0x80000000    VC
> + *   2     0x40000000 - 0xBFFFFFFF    0x40000000    SYS
> + *   3     0xC0000000 - 0xFFFFFFFF    0x80000000    VC

Hmm, so am I right in thinking that the `Broadcom addresses' for section 1
and 2 overlap? It would also be worth describing which physical addresses
Linux actually wants to use; where is the memory in the physical memory map
for devices with this L2 controller?

> + * When the start and end addresses have crossed two different sections, we
> + * need to break the L2 operation into two, each within its own section.
> + * For example, if we need to invalidate addresses starts at 0xBFFF0000 and
> + * ends at 0xC0001000, we need do invalidate 1) 0xBFFF0000 - 0xBFFFFFFF and 2)
> + * 0xC0000000 - 0xC0001000
> + *
> + * Note 1:
> + * By breaking a single L2 operation into two, we may potentially suffer some
> + * performance hit, but keep in mind the cross section case is very rare
> + *
> + * Note 2:
> + * We do not need to handle the case when the start address is in
> + * Section 1 and the end address is in Section 3, since it is not a valid use
> + * case
> + */
> +
> +#define BCM_VC_EMI_SEC1_START_ADDR    0x00000000UL
> +#define BCM_VC_EMI_SEC1_END_ADDR      0x3FFFFFFFUL
> +#define BCM_SYS_EMI_START_ADDR        0x40000000UL
> +#define BCM_SYS_EMI_END_ADDR          0xBFFFFFFFUL
> +#define BCM_VC_EMI_SEC3_START_ADDR    0xC0000000UL
> +#define BCM_VC_EMI_SEC3_END_ADDR      0xFFFFFFFFUL

Seems a bit odd defining the END_ADDRs here, I'd just use strict '<' against
the start of the next section in your code.

> +#define BCM_SYS_EMI_OFFSET            0x40000000UL
> +#define BCM_VC_EMI_OFFSET             0x80000000UL
> +
> +static inline int bcm_addr_is_sys_emi(unsigned long addr)
> +{
> +	return (addr >= BCM_SYS_EMI_START_ADDR) &&
> +		(addr <= BCM_SYS_EMI_END_ADDR);
> +}
> +
> +static inline unsigned long bcm_l2_phys_addr(unsigned long addr)
> +{
> +	if (bcm_addr_is_sys_emi(addr))
> +		return addr + BCM_SYS_EMI_OFFSET;
> +	else
> +		return addr + BCM_VC_EMI_OFFSET;
> +}
> +
> +static void bcm_inv_range(unsigned long start, unsigned long end)
> +{
> +	unsigned long new_start, new_end;
> +
> +	if (unlikely(end <= start))
> +		return;
> +
> +	new_start = bcm_l2_phys_addr(start);
> +	new_end = bcm_l2_phys_addr(end);
> +
> +	/* normal case, no cross section between start and end */
> +	if (likely((bcm_addr_is_sys_emi(start) && bcm_addr_is_sys_emi(end)) ||
> +		(!bcm_addr_is_sys_emi(start) && !bcm_addr_is_sys_emi(end)))) {

You could avoid evaluating bcm_addr_is_sys_emi twice for each address. In
fact, you know start < end, so you just need to check start >= EMI_START and
end < EMI_END.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/