vector status when vlen doesn't match

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2289 bytes --]

Yo Andy,

I was wondering if you could clarify something that came up today on
IRC.

When we enable vector we check vlenb and if it doesn't match the kernel
is suppose to not support vector. We do this in a few places, the first
is in riscv_fill_hwcap():

	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
		riscv_v_setup_vsize();
		/*
		 * ISA string in device tree might have 'v' flag, but
		 * CONFIG_RISCV_ISA_V is disabled in kernel.
		 * Clear V flag in elf_hwcap if CONFIG_RISCV_ISA_V is disabled.
		 */
		if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
	}

Why does this riscv_v_setup_vsize() failing have no impact? Because this
is called only once on the boot CPU, right? I feel like it deserves a
comment as to why.

But the main thing I was wondering about was the other time it is called,
in smp_callin():
	if (has_vector()) {
		if (riscv_v_setup_vsize())
			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
	}

If this fails, we clear the HWCAP bit, but I am not immediately grasping
how this affects the rest of the kernel.
has_vector() just checks if the extension is supported by all cores on
the cpu using either an alternative or the extension support bitmap.
There are lots of sites in the kernel that use has_vector() as the
gating check (as far as I can tell), so if vlen does not match between
CPUs should we not be returning an error for has_vector()?

The alternative that has_vector() relies on is patched before the non-boot
CPUs are enabled, so we cannot modify the result once the non-boot CPUs
are in their callin functions and detect a vlen mismatch while at at the
same time using it in smp_calling(), so should this code be changed to
something along the lines of:

	if (riscv_has_extension_unlikely(v)) {
		if (riscv_v_setup_vsize()) {
			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
			riscv_v_vlen_mismatch = true;
		}
	}
and then has_vector() becomes something like
static __always_inline bool has_vector(void)
{
	return riscv_has_extension_unlikely(RISCV_ISA_EXT_v) && unlikely(riscv_v_vlen_mismatch);
}
Probably there's value to be gained in static branches etc here, but I
was just trying to explain what I was getting at. Maybe I am missing
something though? I do remember talking about this back when the vector
patches were still in review.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv