On 09/01/2014 09:23 PM, Aravinda Prasad wrote: > > > On Monday 01 September 2014 01:16 PM, Alexey Kardashevskiy wrote: >> On 08/25/2014 11:45 PM, Aravinda Prasad wrote: >>> Extend rtas-blob to accommodate error log. Error log >>> structure is saved in rtas space upon a machine check >>> exception. >>> >>> Signed-off-by: Aravinda Prasad <aravi...@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr.c | 13 ++++++++++--- >>> hw/ppc/spapr_rtas.c | 4 ++-- >>> include/hw/ppc/spapr.h | 2 +- >>> pc-bios/spapr-rtas/spapr-rtas.S | 12 ++++++++++++ >>> 4 files changed, 25 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index d01978f..1120988 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -85,6 +85,12 @@ >>> >>> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) >>> >>> +/* >>> + * The rtas-entry-offset should match the value specified in >>> + * spapr-rtas.S >>> + */ >>> +#define RTAS_ENTRY_OFFSET 0x1000 >>> + >>> typedef struct sPAPRMachineState sPAPRMachineState; >>> >>> #define TYPE_SPAPR_MACHINE "spapr-machine" >>> @@ -670,7 +676,8 @@ static int spapr_populate_memory(sPAPREnvironment >>> *spapr, void *fdt) >>> static void spapr_finalize_fdt(sPAPREnvironment *spapr, >>> hwaddr fdt_addr, >>> hwaddr rtas_addr, >>> - hwaddr rtas_size) >>> + hwaddr rtas_size, >>> + hwaddr rtas_entry) >>> { >>> int ret, i; >>> size_t cb = 0; >>> @@ -705,7 +712,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, >>> } >>> >>> /* RTAS */ >>> - ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size); >>> + ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size, >>> rtas_entry); >>> if (ret < 0) { >>> fprintf(stderr, "Couldn't set up RTAS device tree properties\n"); >>> } >>> @@ -808,7 +815,7 @@ static void ppc_spapr_reset(void) >>> >>> /* Load the fdt */ >>> spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr, >>> - spapr->rtas_size); >>> + spapr->rtas_size, spapr->rtas_addr + >>> RTAS_ENTRY_OFFSET); >>> >>> /* Set up the entry state */ >>> first_ppc_cpu = POWERPC_CPU(first_cpu); >>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c >>> index 9ba1ba6..02ddbf9 100644 >>> --- a/hw/ppc/spapr_rtas.c >>> +++ b/hw/ppc/spapr_rtas.c >>> @@ -328,7 +328,7 @@ void spapr_rtas_register(int token, const char *name, >>> spapr_rtas_fn fn) >>> } >>> >>> int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr, >>> - hwaddr rtas_size) >>> + hwaddr rtas_size, hwaddr rtas_entry) >>> { >>> int ret; >>> int i; >>> @@ -349,7 +349,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr >>> rtas_addr, >>> } >>> >>> ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry", >>> - rtas_addr); >>> + rtas_entry); >>> if (ret < 0) { >>> fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n", >>> fdt_strerror(ret)); >>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >>> index bbba51a..dedfa67 100644 >>> --- a/include/hw/ppc/spapr.h >>> +++ b/include/hw/ppc/spapr.h >>> @@ -436,7 +436,7 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, >>> sPAPREnvironment *spapr, >>> uint32_t token, uint32_t nargs, target_ulong >>> args, >>> uint32_t nret, target_ulong rets); >>> int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr, >>> - hwaddr rtas_size); >>> + hwaddr rtas_size, hwaddr rtas_entry); >>> >>> #define SPAPR_TCE_PAGE_SHIFT 12 >>> #define SPAPR_TCE_PAGE_SIZE (1ULL << SPAPR_TCE_PAGE_SHIFT) >>> diff --git a/pc-bios/spapr-rtas/spapr-rtas.S >>> b/pc-bios/spapr-rtas/spapr-rtas.S >>> index 903bec2..8c9b17e 100644 >>> --- a/pc-bios/spapr-rtas/spapr-rtas.S >>> +++ b/pc-bios/spapr-rtas/spapr-rtas.S >>> @@ -30,6 +30,18 @@ >>> >>> .globl _start >>> _start: >>> + /* >>> + * Reserve space for error log in RTAS blob. >>> + * >>> + * Either we can reserve initial bytes for error log followed by >>> + * rtas-entry or space can be reserved after rtas-entry. I prefer >>> + * former, as we already have rtas-base and rtas-entry (currently >>> + * both pointing to rtas-base) defined in qemu and we can update >>> + * rtas-entry to point to an offset from rtas-base. This avoids >>> + * unnecessary definition of rtas-error-offset while keeping >>> + * rtas-entry redundant. >>> + */ >>> + . = 0x1000 >> >> >> Why not this (and not changing spapr-rtas.S)? >> >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -875,7 +875,8 @@ static void ppc_spapr_reset(void) >> spapr->rtas_size); >> >> /* Copy RTAS over */ >> - cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob, >> + cpu_physical_memory_write(spapr->rtas_addr + RTAS_ENTRY_OFFSET, >> + spapr->rtas_blob, >> spapr->rtas_size); > > This is possible, however requires suitable adjustment to make sure > spapr->rtas_addr has enough space allocated.
How is adding RTAS_ENTRY_OFFSET not enough to make sure that is has enough space? QEMU copies RTAS to guest memory, QEMU makes up rtas_addr/entry properties. > >> >> >> >> >>> mr 4,3 >>> lis 3,KVMPPC_H_RTAS@h >>> ori 3,3,KVMPPC_H_RTAS@l >>> >> >> > -- Alexey Kardashevskiy IBM OzLabs, LTC Team e-mail: a...@au1.ibm.com notes: Alexey Kardashevskiy/Australia/IBM