linux-kernel - Re: [PATCH v2 05/10] MIPS: Refactor mips_cps_core

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]

Message-ID: <87o7g46wcp.fsf@BL-laptop>
Date:   Wed, 08 Nov 2023 17:30:46 +0100
From:   Gregory CLEMENT <gregory.clement@...tlin.com>
To:     Jiaxun Yang <jiaxun.yang@...goat.com>, linux-mips@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, tsbogend@...ha.franken.de,
        vladimir.kondratiev@...el.com,
        Jiaxun Yang <jiaxun.yang@...goat.com>
Subject: Re: [PATCH v2 05/10] MIPS: Refactor mips_cps_core_entry implementation

Hello Jiaxun,

> Now the exception vector for CPS systems are allocated on-fly
> with memblock as well.
>
> It will try to allocate from KSEG1 first, and then try to allocate
> in low 4G if possible.
>
> The main reset vector is now generated by uasm, to avoid tons
> of patches to the code. Other vectors are copied to the location
> later.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@...goat.com>
> ---

> +
> +static int __init setup_cps_vecs(void)
> +{
[...]
> +
> +	/* We want to ensure cache is clean before writing uncached mem */
> +	blast_dcache_range(TO_CAC(cps_vec_pa), TO_CAC(cps_vec_pa) +
> BEV_VEC_SIZE);

In my case this call failed because when setup_cps_vecs is called, the
cache information are not initialized yet!

As a workaround I moved the cpu_cache_init() call before
plat_smp_setup() in the /arch/mips/kernel/setup.c file.

Obviously it is not the right thing to do, but it shows that the cache
related function are called too early. For example, in
blast_dcache_range, the value returned by cpu_dcache_line_size was 0
instead of 64, because the value cpu_data[0].dcache.linesz was not set
yet.

So I wonder who it managed to work in your setup. What is the machine
running in QEMU ?

Does it use someting like the following line ?
#define cpu_dcache_line_size()       32


> +	bc_wback_inv(TO_CAC(cps_vec_pa), BEV_VEC_SIZE);
> +	__sync();
> +
> +	cps_vec = (void *)TO_UNCAC(cps_vec_pa);
> +	mips_cps_build_core_entry(cps_vec);
> +
> +	memcpy(cps_vec + 0x200, &excep_tlbfill, 0x80);
> +	memcpy(cps_vec + 0x280, &excep_xtlbfill, 0x80);
> +	memcpy(cps_vec + 0x300, &excep_cache, 0x80);
> +	memcpy(cps_vec + 0x380, &excep_genex, 0x80);
> +	memcpy(cps_vec + 0x400, &excep_intex, 0x80);
> +	memcpy(cps_vec + 0x480, &excep_ejtag, 0x80);
> +
> +	/* Make sure no prefetched data in cache */
> +	blast_inv_dcache_range(TO_CAC(cps_vec_pa), TO_CAC(cps_vec_pa) + BEV_VEC_SIZE);
> +	bc_inv(TO_CAC(cps_vec_pa), BEV_VEC_SIZE);
> +	__sync();
> +
> +	return 0;
> +}

[...]

>  	/* If we have an FPU, enroll ourselves in the FPU-full mask */
> @@ -110,10 +241,14 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
>  {
>  	unsigned ncores, core_vpes, c, cca;
>  	bool cca_unsuitable, cores_limited;
> -	u32 *entry_code;
>  
>  	mips_mt_set_cpuoptions();
>  
> +	if (!core_entry_reg) {
> +		pr_err("core_entry address unsuitable, disabling smp-cps\n");
> +		goto err_out;
> +	}
> +
>  	/* Detect whether the CCA is unsuited to multi-core SMP */
>  	cca = read_c0_config() & CONF_CM_CMASK;
>  	switch (cca) {
> @@ -145,20 +280,6 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
>  			(cca_unsuitable && cpu_has_dc_aliases) ? " & " : "",
>  			cpu_has_dc_aliases ? "dcache aliasing" : "");
>  
> -	/*
> -	 * Patch the start of mips_cps_core_entry to provide:
> -	 *
> -	 * s0 = kseg0 CCA
> -	 */
> -	entry_code = (u32 *)&mips_cps_core_entry;
> -	uasm_i_addiu(&entry_code, 16, 0, cca);
> -	UASM_i_LA(&entry_code, 17, (long)mips_gcr_base);
> -	BUG_ON((void *)entry_code > (void *)&mips_cps_core_entry_patch_end);
> -	blast_dcache_range((unsigned long)&mips_cps_core_entry,
> -			   (unsigned long)entry_code);
> -	bc_wback_inv((unsigned long)&mips_cps_core_entry,
> -		     (void *)entry_code - (void *)&mips_cps_core_entry);
> -	__sync();

The original code here was called later during boot from
kernel_init_freeable() which is called by kernel_init() after all the
calls in start_kernel. That's why there were no issue before the move.

Gregory

>  
>  	/* Allocate core boot configuration structs */
>  	ncores = mips_cps_numcores(0);
> @@ -213,7 +334,7 @@ static void boot_core(unsigned int core, unsigned int vpe_id)
>  	mips_cm_lock_other(0, core, 0, CM_GCR_Cx_OTHER_BLOCK_LOCAL);
>  
>  	/* Set its reset vector */
> -	write_gcr_co_reset_base(CKSEG1ADDR((unsigned long)mips_cps_core_entry));
> +	write_gcr_co_reset_base(core_entry_reg);
>  
>  	/* Ensure its coherency is disabled */
>  	write_gcr_co_coherence(0);
> @@ -290,7 +411,6 @@ static int cps_boot_secondary(int cpu, struct task_struct *idle)
>  	unsigned vpe_id = cpu_vpe_id(&cpu_data[cpu]);
>  	struct core_boot_config *core_cfg = &mips_cps_core_bootcfg[core];
>  	struct vpe_boot_config *vpe_cfg = &core_cfg->vpe_config[vpe_id];
> -	unsigned long core_entry;
>  	unsigned int remote;
>  	int err;
> 
-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com