Re: [PATCH] powerpc/prom_init: Check display props exist before enabling btext

[Michael Ellerman <mpe@ellerman.id.au>]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 21/08/2020 20:34, Michael Ellerman wrote:
>> It's possible to enable CONFIG_PPC_EARLY_DEBUG_BOOTX for a pseries
>> kernel (maybe it shouldn't be), which is then booted with qemu/slof.
>
>
> CONFIG_BOOTX_TEXT=y
> CONFIG_PPC_EARLY_DEBUG=y
> CONFIG_PPC_EARLY_DEBUG_BOOTX=y
>
> this does not crash my VM. The changed chunk is sitting under "if
> (prom_getprop(node, "linux,boot-display", NULL, 0)" and I cannot find
> what creates this property - it is neither slof/grub/qemu, unlikely that
> it is phyp so it must be this one:
>
> arch/powerpc/platforms/powermac/bootx_init.c|244|
> bootx_dt_add_string("linux,boot-display", mem_end);

It's in prom_init.c:

static void __init prom_init_stdout(void)
{
...
	stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
	if (stdout_node != PROM_ERROR) {
		val = cpu_to_be32(stdout_node);

		/* If it's a display, note it */
		memset(type, 0, sizeof(type));
		prom_getprop(stdout_node, "device_type", type, sizeof(type));
		if (prom_strcmp(type, "display") == 0)
			prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
	}
}

> which is powermac and not pseries. Or may be that pmac firmware.
>
> Where did you see this crash?

Qemu pseries either TCG or KVM with eg:

$ qemu-system-ppc64 -M pseries -cpu POWER8 -m 1G -kernel build~/vmlinux


>> But if you do that the kernel crashes in draw_byte(), with a DAR
>> pointing somewhere near INT_MAX.
>> 
>> Adding some debug to prom_init we see that we're not able to read the
>> "address" property from OF, so we're just using whatever junk value
>> was on the stack.
>> 
>> So check the properties can be read properly from OF, if not we bail
>> out before initialising btext, which avoids the crash.
>
> This is a right thing any way, just the commit log is confusing.
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Thanks.

cheers