Re: [PATCH v8 2/4] MIPS: Octeon: Automatically provision CVMSEG space.

[James Hogan]

On Thu, Feb 22, 2018 at 03:07:14PM -0800, David Daney wrote:
> diff --git a/arch/mips/cavium-octeon/Kconfig b/arch/mips/cavium-octeon/Kconfig
> index b5eee1a57d6c..a283b73b7fc6 100644
> --- a/arch/mips/cavium-octeon/Kconfig
> +++ b/arch/mips/cavium-octeon/Kconfig
> @@ -11,21 +11,26 @@ config CAVIUM_CN63XXP1
>  	  non-CN63XXP1 hardware, so it is recommended to select "n"
>  	  unless it is known the workarounds are needed.
>  
> -config CAVIUM_OCTEON_CVMSEG_SIZE
> -	int "Number of L1 cache lines reserved for CVMSEG memory"

This is set to 2 in cavium_octeon_defconfig, which can now be removed (I
presume the default of 0 for CAVIUM_OCTEON_EXTRA_CVMSEG is sufficient).

> diff --git a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
> index c38b38ce5a3d..cdcca60978a2 100644
> --- a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
> +++ b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
> @@ -26,11 +26,18 @@
>  	# a3 = address of boot descriptor block
>  	.set push
>  	.set arch=octeon
> +	mfc0	v1, CP0_PRID_REG
> +	andi	v1, 0xff00
> +	li	v0, 0x9500		# cn78XX or later
> +	subu	v1, v1, v0
> +	li	t2, 2 + CONFIG_CAVIUM_OCTEON_EXTRA_CVMSEG
> +	bltz	v1, 1f
> +	addiu	t2, 1			# t2 has cvmseg_size

It'd be nice to clean up this PRID code one day to use the defines in
<asm/mipsregs.h> and <asm/cpu.h>. This is consistent with whats already
here though so it can be done later.

> +1:
>  	# Read the cavium mem control register
>  	dmfc0	v0, CP0_CVMMEMCTL_REG
>  	# Clear the lower 6 bits, the CVMSEG size
> -	dins	v0, $0, 0, 6
> -	ori	v0, CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE
> +	dins	v0, t2, 0, 6
>  	dmtc0	v0, CP0_CVMMEMCTL_REG	# Write the cavium mem control register
>  	dmfc0	v0, CP0_CVMCTL_REG	# Read the cavium control register
>  	# Disable unaligned load/store support but leave HW fixup enabled
> @@ -70,7 +77,7 @@
>  	# Flush dcache after config change
>  	cache	9, 0($0)
>  	# Zero all of CVMSEG to make sure parity is correct
> -	dli	v0, CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE
> +	move	v0, t2
>  	dsll	v0, 7
>  	beqz	v0, 2f
>  1:	dsubu	v0, 8
> @@ -126,12 +133,7 @@
>  	LONG_L	sp, (t0)
>  	# Set the SP global variable to zero so the master knows we've started
>  	LONG_S	zero, (t0)
> -#ifdef __OCTEON__
> -	syncw
> -	syncw
> -#else

Is this directly related? I don't think I saw it mentioned anywhere.

>  	sync
> -#endif
>  	# Jump to the normal Linux SMP entry point
>  	j   smp_bootstrap
>  	nop
> @@ -148,6 +150,8 @@
>  
>  #endif /* CONFIG_SMP */
>  octeon_main_processor:
> +	dla	v0, octeon_cvmseg_lines
> +	sw	t2, 0(v0)

You would get something equivalent (and slightly more efficient but
using $at) with this?
	sw	t2, octeon_cvmseg_lines

I.e.
lui     v0,0x8190	->	lui     at,0x8190
daddiu  v0,v0,-19688	->
sw      t2,0(v0)	->	sw      t2,-19688(at)

> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
> index 858752dac337..4e87e4f5247a 100644
> --- a/arch/mips/include/asm/mipsregs.h
> +++ b/arch/mips/include/asm/mipsregs.h
> @@ -1126,6 +1126,8 @@
>  #define FPU_CSR_RD	0x3	/* towards -Infinity */
>  
>  
> +#define CAVIUM_OCTEON_SCRATCH_OFFSET (2 * 128 - 16 - 32768)

This feels a bit out of place, since its effectively cavium specific
memory mapped scratch register addresses. Would tlbex.h or octeon.h be
more appropriate, or even tlbex.c if it isn't used elsewhere?

Otherwise this patch looks reasonable I think.

Cheers
James

Attachment: signature.asc
Description: Digital signature