Re: [Qemu-devel] qemu patch for adding functionality to rtas_ibm_get_system_parameter [Version 2]

[Mike Day]

Tomohiro B Berry <address@hidden> writes:

> Hi again,
>
> I believe I have added the appropriate format changes and made a couple 
> changes to the code.  This patch should add functionality to the function 
> rtas_ibm_get_system_parameter to return a string containing the values for 
> partition_max_entitled_capacity and system_potential_processors. 

The text before the diff goes into the git commit log. The patch
guidelines say the commit log entry should describe the patch and
anything else should go below "--" which will keep it out of the commit
log. "I believe I have..." is an example of the type of text that
shouldn't be in the commit log.

There is still a line wrap - If you are not doing so consider running
your patch through scripts/chechpatch.pl before submitting it.

Also consider using git-format-patch to generate the patch you will
send if you're not doing so already.

Mike

> Regards, 
> Tomo Berry
>
> Signed-off-by: Tomo Berry <address@hidden>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 1cb276d..931ba06 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -225,6 +225,9 @@ static void rtas_stop_self(PowerPCCPU *cpu, 
> sPAPREnvironment *spapr,
>  }
>  
>  #define DIAGNOSTICS_RUN_MODE        42
> +#define SPLPAR_CHARACTERISTICS_TOKEN 20
> +#define CMO_CHARACTERISTICS_TOKEN 44
> +#define CEDE_LATENCY_TOKEN 45
>  
>  static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>                                            sPAPREnvironment *spapr,
> @@ -236,6 +239,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
> *cpu,
>      target_ulong buffer = rtas_ld(args, 1);
>      target_ulong length = rtas_ld(args, 2);
>      target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
> +    char *local_buffer;
>  
>      switch (parameter) {
>      case DIAGNOSTICS_RUN_MODE:
> @@ -244,6 +248,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
> *cpu,
>              ret = RTAS_OUT_SUCCESS;
>          }
>          break;
> +    case SPLPAR_CHARACTERISTICS_TOKEN:
> +
> +        /*  Create a string of length bytes locally to copy to buffer  */
> +
> +        local_buffer = calloc(length, 1);
> +
> +        /*  These are the current system parameters supported.  The first
> +         *  16 bits of the buffer must contain the length of the string.
> +         *  These 16 bits are not included in the length of the string. 
> */
> +
> +        ((uint16_t *)local_buffer)[0] = cpu_to_be16(snprintf(local_buffer 
> + 2,
> +             length - 2, "MaxEntCap=%d,MaxPlatProcs=%d", max_cpus, 
> smp_cpus));
> +
> +        /*  Copy the string into buffer using rtas_st_buffer to
> +         *  convert the addresses. */
> +
> +        rtas_st_buffer(buffer, length, (uint8_t *)local_buffer);
> +
> +        free(local_buffer);
> +        ret = RTAS_OUT_SUCCESS;
> +        break;
> +    case CMO_CHARACTERISTICS_TOKEN:
> +        ret = RTAS_OUT_NOT_SUPPORTED;
> +        break;
> +    case CEDE_LATENCY_TOKEN:
> +        ret = RTAS_OUT_NOT_SUPPORTED;
> +        break;
> +    default:
> +        ret = RTAS_OUT_NOT_SUPPORTED;
> +        break;
>      }
>  
>      rtas_st(rets, 0, ret);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index b2f11e9..ee6ed2d 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -356,6 +356,18 @@ static inline void rtas_st(target_ulong phys, int n, 
> uint32_t val)
>      stl_be_phys(ppc64_phys_to_real(phys + 4*n), val);
>  }
>  
> +/*  This function will store a buffer 1 byte at a time into the
> + *  address at phys up to a maximum of phys_len bytes.*/
> +
> +static inline void rtas_st_buffer(target_ulong phys,
> +                                  target_ulong phys_len,
> +                                  uint8_t *buffer) {
> +    uint32_t i;
> +    for (i = 0; i < phys_len; i++) {
> +        stb_phys(ppc64_phys_to_real(phys + i), buffer[i]);
> +    }
> +}
> +
>  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                                uint32_t token,
>                                uint32_t nargs, target_ulong args,
>
>
>
> From:   Mike Day <address@hidden>
> To:     Tomohiro B Berry/Austin/address@hidden, address@hidden, 
> Date:   03/13/2014 03:24 PM
> Subject:        Re: [Qemu-devel] qemu patch for adding functionality to 
> rtas_ibm_get_system_parameter
>
>
>
>
> Tomohiro, 
>
> Please follow the guidelines for submitting a patch to Qemu that are
> found in:
>
> http://wiki.qemu.org/Contribute/SubmitAPatch
>
> This patch has an inappropriate commit log, is missing a signed-off-by:
> tag, and some of the lines wrapped in my reader. These are explained in
> the document above. You should be able to fix these issues quickly and
> resubmit this patch. 
>
> Mike
>
> Tomohiro B Berry <address@hidden> writes:
>
>> Hi all,
>>
>> rtas_ibm_get_system_parameter did not previously have the functionality 
> to 
>> return the appropriate string when called with the 
>> SPLPAR_CHARACTERISTICS_TOKEN.  I am proposing the following patch to add 
>
>> that functionality.  I am including the cases for 
>> CMO_CHARACTERISTICS_TOKEN and CEDE_LATENCY_TOKEN because the function 
> gets 
>> called with those tokens during the boot process, but I understand that 
>> they are currently redundant with the default case.  I just figured they 
>
>> would be useful for future development, but if we don't want them in 
> there 
>> right now I think that would be okay, too. 
>>
>> Regards,
>> Tomo Berry
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 1cb276d..318fdcd 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -225,6 +225,9 @@ static void rtas_stop_self(PowerPCCPU *cpu, 
>> sPAPREnvironment *spapr,
>>  }
>> 
>>  #define DIAGNOSTICS_RUN_MODE        42
>> +#define SPLPAR_CHARACTERISTICS_TOKEN 20
>> +#define CMO_CHARACTERISTICS_TOKEN 44
>> +#define CEDE_LATENCY_TOKEN 45
>> 
>>  static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>>                                            sPAPREnvironment *spapr,
>> @@ -236,6 +239,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
>
>> *cpu,
>>      target_ulong buffer = rtas_ld(args, 1);
>>      target_ulong length = rtas_ld(args, 2);
>>      target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
>> +    char *local_buffer;
>> 
>>      switch (parameter) {
>>      case DIAGNOSTICS_RUN_MODE:
>> @@ -244,6 +248,42 @@ static void 
> rtas_ibm_get_system_parameter(PowerPCCPU 
>> *cpu,
>>              ret = RTAS_OUT_SUCCESS;
>>          }
>>          break;
>> +    case SPLPAR_CHARACTERISTICS_TOKEN:
>> +
>> +       /*  Create a string locally to copy to buffer  */
>> + 
>> +       local_buffer=(char*)malloc(length*sizeof(char));
>> +       memset(local_buffer,0,length);
>> +
>> +       /*  These are the current system parameters supported.  The 
> spaces 
>> at the 
>> +        *  start of the string are place holders for the string length. 
>
>> */
>> +
>> +       snprintf(local_buffer,length," 
>> MaxEntCap=%d,MaxPlatProcs=%d",max_cpus,smp_cpus);
>> +
>> +       /*  The first 16 bits of the buffer must contain the length of 
> the 
>> string.
>> +        *  These 16 bits are not included in the length of the string. 
> */
>> + 
>> + 
>> 
> ((uint16_t*)local_buffer)[0]=cpu_to_be16((uint16_t)strlen(&local_buffer[2]));
>> + 
>> +       /*  Copy the string into buffer using rtas_st_buffer to 
>> +        *  convert the addresses. */
>> + 
>> +       rtas_st_buffer(buffer,length,(uint8_t*)local_buffer);
>> +
>> +       /*  Free the local buffer and return success. */ 
>> + 
>> +       free(local_buffer);
>> +       ret = RTAS_OUT_SUCCESS;
>> +       break;
>> +    case CMO_CHARACTERISTICS_TOKEN:
>> +       ret = RTAS_OUT_NOT_SUPPORTED;
>> +       break;
>> +    case CEDE_LATENCY_TOKEN:
>> +       ret = RTAS_OUT_NOT_SUPPORTED;
>> +       break;
>> +    default:
>> +       ret = RTAS_OUT_NOT_SUPPORTED;
>> +       break;
>>      }
>> 
>>      rtas_st(rets, 0, ret);
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index b2f11e9..87c039c 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -356,6 +356,16 @@ static inline void rtas_st(target_ulong phys, int 
> n, 
>> uint32_t val)
>>      stl_be_phys(ppc64_phys_to_real(phys + 4*n), val);
>>  }
>> 
>> +/*  This function will store a buffer 1 byte at a time into the 
>> + *  address at phys up to a maximum of phys_len bytes. */
>> +
>> +static inline void rtas_st_buffer(target_ulong phys, target_ulong 
>> phys_len, uint8_t* buffer){
>> +    uint32_t i; 
>> +    for(i=0;i<phys_len;i++){
>> +        stb_phys(ppc64_phys_to_real(phys + i),buffer[i]);
>> +    }
>> +}
>> +
>>  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>                                uint32_t token,
>>                                uint32_t nargs, target_ulong args,
>> diff --git a/pixman b/pixman
>> --- a/pixman
>> +++ b/pixman
>> @@ -1 +1 @@
>> -Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3
>> +Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3-dirty
> -- 
> Mike Day | "Endurance is a Virtue"
>
>

-- 
Mike Day | "Endurance is a Virtue"