From 21bde1ecb6cecba1d2f0219a1b79c240bed78749 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Thu, 8 Jul 2021 16:56:25 +1000 Subject: [PATCH] spapr: Fix implementation of Open Firmware client interface This addresses the comments from v22. The functional changes are (the VOF ones need retesting with Pegasos2): (VOF) setprop will start failing if the machine class callback did not handle it; (VOF) unit addresses are lowered in path_offset(); (SPAPR) /chosen/bootargs is initialized from kernel_cmdline if the client did not change it. Fixes: 5c991e5d4378 ("spapr: Implement Open Firmware client interface") Cc: BALATON Zoltan Signed-off-by: Alexey Kardashevskiy Message-Id: <20210708065625.548396-1-aik@ozlabs.ru> Tested-by: BALATON Zoltan Signed-off-by: David Gibson --- MAINTAINERS | 4 ++-- hw/ppc/spapr.c | 10 +--------- hw/ppc/spapr_hcall.c | 5 ++--- hw/ppc/spapr_vof.c | 32 +++++++++++++++++++++++--------- hw/ppc/vof.c | 30 +++++++++++++++++------------- include/hw/ppc/spapr.h | 3 +-- pc-bios/vof.bin | Bin 3784 -> 3456 bytes pc-bios/vof/ci.c | 2 +- pc-bios/vof/libc.c | 26 -------------------------- pc-bios/vof/main.c | 2 +- pc-bios/vof/vof.h | 2 -- 11 files changed, 48 insertions(+), 68 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index ce122eeced..89d71b42b2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1362,8 +1362,8 @@ F: include/hw/pci-host/mv64361.h Virtual Open Firmware (VOF) M: Alexey Kardashevskiy -M: David Gibson -M: Greg Kurz +R: David Gibson +R: Greg Kurz L: qemu-ppc@nongnu.org S: Maintained F: hw/ppc/spapr_vof* diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e9b6d0f587..3808d47053 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1645,15 +1645,7 @@ static void spapr_machine_reset(MachineState *machine) fdt = spapr_build_fdt(spapr, true, FDT_MAX_SIZE); if (spapr->vof) { - target_ulong stack_ptr = 0; - - spapr_vof_reset(spapr, fdt, &stack_ptr, &error_fatal); - - spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, - stack_ptr, spapr->initrd_base, - spapr->initrd_size); - /* VOF is 32bit BE so enforce MSR here */ - first_ppc_cpu->env.msr &= ~((1ULL << MSR_SF) | (1ULL << MSR_LE)); + spapr_vof_reset(spapr, fdt, &error_fatal); /* * Do not pack the FDT as the client may change properties. * VOF client does not expect the FDT so we do not load it to the VM. diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 80ae8eaadd..0e9a5b2e40 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1080,7 +1080,7 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu, SpaprOptionVector *ov1_guest, *ov5_guest; bool guest_radix; bool raw_mode_supported = false; - bool guest_xive, reset_fdt = false; + bool guest_xive; CPUState *cs; void *fdt; uint32_t max_compat = spapr->max_compat_pvr; @@ -1233,8 +1233,7 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu, spapr_setup_hpt(spapr); } - reset_fdt = spapr->vof != NULL; - fdt = spapr_build_fdt(spapr, reset_fdt, fdt_bufsize); + fdt = spapr_build_fdt(spapr, spapr->vof != NULL, fdt_bufsize); g_free(spapr->fdt_blob); spapr->fdt_size = fdt_totalsize(fdt); spapr->fdt_initial_size = spapr->fdt_size; diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c index 131a03fec0..40ce8fe003 100644 --- a/hw/ppc/spapr_vof.c +++ b/hw/ppc/spapr_vof.c @@ -8,6 +8,7 @@ #include "qapi/error.h" #include "hw/ppc/spapr.h" #include "hw/ppc/spapr_vio.h" +#include "hw/ppc/spapr_cpu_core.h" #include "hw/ppc/fdt.h" #include "hw/ppc/vof.h" #include "sysemu/sysemu.h" @@ -29,13 +30,19 @@ target_ulong spapr_h_vof_client(PowerPCCPU *cpu, SpaprMachineState *spapr, void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt) { char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus); - int chosen; vof_build_dt(fdt, spapr->vof); - _FDT(chosen = fdt_path_offset(fdt, "/chosen")); - _FDT(fdt_setprop_string(fdt, chosen, "bootargs", - spapr->vof->bootargs ? : "")); + if (spapr->vof->bootargs) { + int chosen; + + _FDT(chosen = fdt_path_offset(fdt, "/chosen")); + /* + * If the client did not change "bootargs", spapr_dt_chosen() must have + * stored machine->kernel_cmdline in it before getting here. + */ + _FDT(fdt_setprop_string(fdt, chosen, "bootargs", spapr->vof->bootargs)); + } /* * SLOF-less setup requires an open instance of stdout for early @@ -48,20 +55,21 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt) } } -void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, - target_ulong *stack_ptr, Error **errp) +void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp) { + target_ulong stack_ptr; Vof *vof = spapr->vof; + PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); vof_init(vof, spapr->rma_size, errp); - *stack_ptr = vof_claim(vof, 0, VOF_STACK_SIZE, VOF_STACK_SIZE); - if (*stack_ptr == -1) { + stack_ptr = vof_claim(vof, 0, VOF_STACK_SIZE, VOF_STACK_SIZE); + if (stack_ptr == -1) { error_setg(errp, "Memory allocation for stack failed"); return; } /* Stack grows downwards plus reserve space for the minimum stack frame */ - *stack_ptr += VOF_STACK_SIZE - 0x20; + stack_ptr += VOF_STACK_SIZE - 0x20; if (spapr->kernel_size && vof_claim(vof, spapr->kernel_addr, spapr->kernel_size, 0) == -1) { @@ -77,6 +85,12 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, spapr_vof_client_dt_finalize(spapr, fdt); + spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, + stack_ptr, spapr->initrd_base, + spapr->initrd_size); + /* VOF is 32bit BE so enforce MSR here */ + first_ppc_cpu->env.msr &= ~((1ULL << MSR_SF) | (1ULL << MSR_LE)); + /* * At this point the expected allocation map is: * diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c index 47c86e394e..81f6596215 100644 --- a/hw/ppc/vof.c +++ b/hw/ppc/vof.c @@ -144,15 +144,16 @@ static int path_offset(const void *fdt, const char *path) * the lower case forms of the hexadecimal digits in the range a..f, * suppressing leading zeros". */ - at = strchr(path, '@'); - if (!at) { - return fdt_path_offset(fdt, path); + p = g_strdup(path); + for (at = strchr(p, '@'); at && *at; ) { + if (*at == '/') { + at = strchr(at, '@'); + } else { + *at = tolower(*at); + ++at; + } } - p = g_strdup(path); - for (at = at - path + p + 1; *at; ++at) { - *at = tolower(*at); - } return fdt_path_offset(fdt, p); } @@ -300,6 +301,7 @@ static uint32_t vof_setprop(MachineState *ms, void *fdt, Vof *vof, char trval[64] = ""; char nodepath[VOF_MAX_PATH] = ""; Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF); + VofMachineIfClass *vmc; g_autofree char *val = NULL; if (vallen > VOF_MAX_SETPROPLEN) { @@ -322,13 +324,13 @@ static uint32_t vof_setprop(MachineState *ms, void *fdt, Vof *vof, goto trace_exit; } - if (vmo) { - VofMachineIfClass *vmc = VOF_MACHINE_GET_CLASS(vmo); + if (!vmo) { + goto trace_exit; + } - if (vmc->setprop && - !vmc->setprop(ms, nodepath, propname, val, vallen)) { - goto trace_exit; - } + vmc = VOF_MACHINE_GET_CLASS(vmo); + if (!vmc->setprop || !vmc->setprop(ms, nodepath, propname, val, vallen)) { + goto trace_exit; } ret = fdt_setprop(fdt, offset, propname, val, vallen); @@ -919,6 +921,8 @@ static uint32_t vof_client_handle(MachineState *ms, void *fdt, Vof *vof, ret = -1; } +#undef cmpserv + return ret; } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index a25e69fe4c..779f707fb8 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -964,8 +964,7 @@ void spapr_set_all_lpcrs(target_ulong value, target_ulong mask); hwaddr spapr_get_rtas_addr(void); bool spapr_memory_hot_unplug_supported(SpaprMachineState *spapr); -void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, - target_ulong *stack_ptr, Error **errp); +void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp); void spapr_vof_quiesce(MachineState *ms); bool spapr_vof_setprop(MachineState *ms, const char *path, const char *propname, void *val, int vallen); diff --git a/pc-bios/vof.bin b/pc-bios/vof.bin index 1ec670be82134adcb5ae128732aff6e371281360..300cb7c7f9d9d77ffa7cbb7f0f26919246ef2d14 100755 GIT binary patch delta 151 zcmX>h+aSGxjghy9!GnR}jEw^WLxNM!WMRf~rtUM7dl>VWx??8)VQgaRda${HDTtA& zvuE-Z<}9X8h0P8uhZvdKVH uz%*HauS<{T1Oo%l10a6Gz`)A@#2gF^j0r&O17wQ;u?!Gv0I>lOTL1tL)F-q6 delta 369 zcmZpWJ|Vk-jghyH!GnR}jEw^WLxNM^WMRf~re2ZBJ&buwJxeD4VQgaR(b(L;6vW8X zb!GAu<}9YJjLi-#hZvbUmP}@0i(~3=nViCw#?*di@)@=|ruK%-KI~hW>f;$?8toY* zYPdWc92yuV0NDzSK(SgaFA*RO7I$o9r~rvuYX1KZ5(CLiv}fQz5(BFTit$(~FfagV z0iZ(-fNFUxwf_GHi38PgSaJf{@(diEUJMK~JsB8)VgeSHnhcB}4M4>LAOnF8VQ_5t ze*$Pg43IAYlml5L12P2J@X0?olqCgl>7J~^X|b7u>j2Y40hP%oc)IlX1Q;0jG=SIy dh=FGF1u!r$CIGPykR1cWDL`BR#1%l?005cvU&jCd diff --git a/pc-bios/vof/ci.c b/pc-bios/vof/ci.c index 2b56050238..fc4821b3e9 100644 --- a/pc-bios/vof/ci.c +++ b/pc-bios/vof/ci.c @@ -69,7 +69,7 @@ static int call_ci(const char *service, int nargs, int nret, ...) } if (ci_entry((uint32_t)(&args)) < 0) { - return PROM_ERROR; + return -1; } return (nret > 0) ? args.args[nargs] : 0; diff --git a/pc-bios/vof/libc.c b/pc-bios/vof/libc.c index 00c10e6e7d..fdbc30f777 100644 --- a/pc-bios/vof/libc.c +++ b/pc-bios/vof/libc.c @@ -54,32 +54,6 @@ int memcmp(const void *ptr1, const void *ptr2, size_t n) return 0; } -void *memmove(void *dest, const void *src, size_t n) -{ - char *cdest; - const char *csrc; - int i; - - /* Do the buffers overlap in a bad way? */ - if (src < dest && src + n >= dest) { - /* Copy from end to start */ - cdest = dest + n - 1; - csrc = src + n - 1; - for (i = 0; i < n; i++) { - *cdest-- = *csrc--; - } - } else { - /* Normal copy is possible */ - cdest = dest; - csrc = src; - for (i = 0; i < n; i++) { - *cdest++ = *csrc++; - } - } - - return dest; -} - void *memset(void *dest, int c, size_t size) { unsigned char *d = (unsigned char *)dest; diff --git a/pc-bios/vof/main.c b/pc-bios/vof/main.c index 9fc30d2d09..0f0f6b4cb1 100644 --- a/pc-bios/vof/main.c +++ b/pc-bios/vof/main.c @@ -6,7 +6,7 @@ void do_boot(unsigned long addr, unsigned long _r3, unsigned long _r4) register unsigned long r4 __asm__("r4") = _r4; register unsigned long r5 __asm__("r5") = (unsigned long) _prom_entry; - ((client *)(uint32_t)addr)(); + ((void (*)(void))(uint32_t)addr)(); } void entry_c(void) diff --git a/pc-bios/vof/vof.h b/pc-bios/vof/vof.h index 2d89580769..5f12c077f5 100644 --- a/pc-bios/vof/vof.h +++ b/pc-bios/vof/vof.h @@ -10,11 +10,9 @@ typedef unsigned short uint16_t; typedef unsigned long uint32_t; typedef unsigned long long uint64_t; #define NULL (0) -#define PROM_ERROR (-1u) typedef unsigned long ihandle; typedef unsigned long phandle; typedef int size_t; -typedef void client(void); /* globals */ extern void _prom_entry(void); /* OF CI entry point (i.e. this firmware) */