Re: [Qemu-devel] [PATCH v1 2/2] s390x/cpumodel: improve defintion search without an IBC

["Jason J. Herne" <jjherne@linux.vnet.ibm.com>]

On 05/31/2017 03:34 PM, David Hildenbrand wrote:
> Currently, under z/VM on a 0x2827, QEMU will detect a 0x2828 if no
> IBC value is provided. QEMU will simply take the last model of that HW
> generation, which happens to be the BC version.
>
> Let's improve our search for that case by selecting the latest CPU
> definition that matches the CPU type. This might still detect e.g. a
> GA2 version on a GA1 system, but as we don't have further information at
> hand, there isn't too much we can do about it.
>

Isn't it better to choose the earliest machine possible in this case? 
Systems
are backwards compatible far more often than they are forwards compatible.

Said in a more verbose way, I'd rather pretend to be a GA1 on a GA2 
(ignoring
some new features) than pretend to be a GA2 on a GA1 and attempt to use
something that does not exist.

I'm sure you've thought of all of this though, so what am I missing?


> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu_models.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 9e23535..b6220c8 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -184,6 +184,7 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
>                                      S390FeatBitmap features)
>  {
>      const S390CPUDef *last_compatible = NULL;
> +    const S390CPUDef *matching_cpu_type = NULL;
>      int i;
>
>      if (!gen) {
> @@ -218,8 +219,16 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
>          if (def->type == type && def->ec_ga == ec_ga) {
>              return def;
>          }
> +        /* remember if we've at least seen one with the same cpu type */
> +        if (def->type == type) {
> +            matching_cpu_type = def;
> +        }
>          last_compatible = def;
>      }
> +    /* prefer the model with the same cpu type, esp. don't take the BC for EC */
> +    if (matching_cpu_type) {
> +        return matching_cpu_type;
> +    }
>      return last_compatible;
>  }
>

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)