Re: [Qemu-devel] [Patch v1 15/29] s390x/sclp: indicate sclp features

[Thomas Huth <thuth@redhat.com>]

On 02.08.2016 13:59, David Hildenbrand wrote:
> We have three different blocks in the SCLP read-SCP information response
> that indicate sclp features. Let's prepare propagation.
> 
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> ---
>  hw/s390x/sclp.c           |  9 +++++++++
>  target-s390x/cpu_models.c | 14 ++++++++++++++
>  target-s390x/cpu_models.h |  1 +
>  3 files changed, 24 insertions(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 15d7114..3c126ee 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -31,11 +31,14 @@ static inline SCLPDevice *get_sclp_device(void)
>  
>  static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int count)
>  {
> +    uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
>      int i;
>  
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_CPU, features);
>      for (i = 0; i < count; i++) {
>          entry[i].address = i;
>          entry[i].type = 0;
> +        memcpy(entry[i].features, features, sizeof(entry[i].features));
>      }
>  }
>  
> @@ -59,6 +62,12 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
>      read_info->highest_cpu = cpu_to_be16(max_cpus);
>  
> +    /* Configuration Characteristic (Extension) */
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
> +                         read_info->conf_char);
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +                         read_info->conf_char_ext);
> +
>      prepare_cpu_entries(sclp, read_info->entries, cpu_count);
>  
>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
> diff --git a/target-s390x/cpu_models.c b/target-s390x/cpu_models.c
> index 3fe85fa..641aad0 100644
> --- a/target-s390x/cpu_models.c
> +++ b/target-s390x/cpu_models.c
> @@ -74,6 +74,20 @@ static const S390CPUDef s390_cpu_defs[] = {
>      CPUDEF_INIT(0x2965, 13, 2, 47, 0x08000000U, "z13s", "IBM z13s GA1"),
>  };
>  
> +void s390_get_feat_block(S390FeatType type, uint8_t *data)
> +{
> +    static S390CPU *cpu;
> +
> +    if (!cpu) {
> +        cpu = S390_CPU(qemu_get_cpu(0));
> +    }
> +
> +    if (!cpu || !cpu->model) {
> +        return;
> +    }
> +    return s390_fill_feat_block(cpu->model->features, type, data);

IMHO it's somewhat strange to write "return something()" in a function
that has been declared as "void". I know GCC does not reject this, but
anyway, I'd suggest to simply remove the "return" keyword here.

 Thomas