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

[Jason J. Herne]

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 <address@hidden>
> ---
>  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 (address@hidden)