Re: [Qemu-devel] [PATCH 1/5] target-ppc: Extend rtas-blob

[Aravinda Prasad <aravinda@linux.vnet.ibm.com>]

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 <aravinda@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.

> 
> 
> 
> 
>>  	mr	4,3
>>  	lis	3,KVMPPC_H_RTAS@h
>>  	ori	3,3,KVMPPC_H_RTAS@l
>>
> 
> 

-- 
Regards,
Aravinda