diff mbox

[v3,30/30] target/s390x: update maximum TCG model to z800

Message ID 20170531220129.27724-31-aurelien@aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno May 31, 2017, 10:01 p.m. UTC
At the same time fix the TCG version of get_max_cpu_model to return the
maximum model like on KVM. Remove the ETF2 and long-displacement
facilities from the additional features as it is included in the z800.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/cpu_models.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

David Hildenbrand June 1, 2017, 8:38 a.m. UTC | #1
On 01.06.2017 00:01, Aurelien Jarno wrote:
> At the same time fix the TCG version of get_max_cpu_model to return the
> maximum model like on KVM. Remove the ETF2 and long-displacement

I don't understand the part
"fix the TCG version of get_max_cpu_model to return the maximum model
like on KVM".

Can you elaborate?

> facilities from the additional features as it is included in the z800.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/cpu_models.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index fc3cb25cc3..c13bbd852c 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>      static const int feats[] = {
>          S390_FEAT_STFLE,
>          S390_FEAT_EXTENDED_IMMEDIATE,
> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> -        S390_FEAT_LONG_DISPLACEMENT,
>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>          S390_FEAT_ETF2_ENH,
>          S390_FEAT_STORE_CLOCK_FAST,
> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>      if (kvm_enabled()) {
>          kvm_s390_get_host_cpu_model(&max_model, errp);
>      } else {
> -        /* TCG emulates a z900 (with some optional additional features) */
> -        max_model.def = &s390_cpu_defs[0];
> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> +        /* TCG emulates a z800 (with some optional additional features) */
> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>                      S390_FEAT_MAX);
>          add_qemu_cpu_model_features(max_model.features);
>      }
> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>      S390CPU *cpu = S390_CPU(obj);
>  
>      cpu->model = g_malloc0(sizeof(*cpu->model));
> -    /* TCG emulates a z900 (with some optional additional features) */
> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> +    /* TCG emulates a z800 (with some optional additional features) */
> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> +           sizeof(s390_qemu_cpu_defs));

No changing the qemu model without compatibility handling.

Thanks!
David Hildenbrand June 1, 2017, 9:04 a.m. UTC | #2
On 01.06.2017 10:38, David Hildenbrand wrote:
> On 01.06.2017 00:01, Aurelien Jarno wrote:
>> At the same time fix the TCG version of get_max_cpu_model to return the
>> maximum model like on KVM. Remove the ETF2 and long-displacement
> 
> I don't understand the part
> "fix the TCG version of get_max_cpu_model to return the maximum model
> like on KVM".
> 
> Can you elaborate?
> 
>> facilities from the additional features as it is included in the z800.
>>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>>  target/s390x/cpu_models.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index fc3cb25cc3..c13bbd852c 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>      static const int feats[] = {
>>          S390_FEAT_STFLE,
>>          S390_FEAT_EXTENDED_IMMEDIATE,
>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>> -        S390_FEAT_LONG_DISPLACEMENT,
>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>          S390_FEAT_ETF2_ENH,
>>          S390_FEAT_STORE_CLOCK_FAST,
>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>      if (kvm_enabled()) {
>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>      } else {
>> -        /* TCG emulates a z900 (with some optional additional features) */
>> -        max_model.def = &s390_cpu_defs[0];
>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>> +        /* TCG emulates a z800 (with some optional additional features) */
>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>                      S390_FEAT_MAX);

This is most likely wrong: you're indicating features here that are not
available on tcg. esp. S390_FEAT_SIE_F2 and friends.

I think should only copy the base features and add whatever else is
available via add_qemu_cpu_model_features() as already done.

>>          add_qemu_cpu_model_features(max_model.features);
>>      }
>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>      S390CPU *cpu = S390_CPU(obj);
>>  
>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>> -    /* TCG emulates a z900 (with some optional additional features) */
>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>> +    /* TCG emulates a z800 (with some optional additional features) */
>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>> +           sizeof(s390_qemu_cpu_defs));
> 
> No changing the qemu model without compatibility handling.
> 
Please have a look at the following mail for a possible solution:

https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html

This could be moved to a separate patch. So this patch really should
just care about the maximum model, not the qemu model.
Aurelien Jarno June 1, 2017, 7:17 p.m. UTC | #3
On 2017-06-01 10:38, David Hildenbrand wrote:
> On 01.06.2017 00:01, Aurelien Jarno wrote:
> > At the same time fix the TCG version of get_max_cpu_model to return the
> > maximum model like on KVM. Remove the ETF2 and long-displacement
> 
> I don't understand the part
> "fix the TCG version of get_max_cpu_model to return the maximum model
> like on KVM".
> 
> Can you elaborate?

Currently get_max_cpu_model returns the features of the base model, so
for example the one of a z900 even on a z800. This makes impossible to
enable the features that are provided by a z800 like etf2 or ldisp.

For what I understand from the KVM code (but I haven't tested), the
function return all the features that are supported by the current CPU,
not all the features that are supported by the base model of the current
CPU.


> > facilities from the additional features as it is included in the z800.
> > 
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  target/s390x/cpu_models.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> > index fc3cb25cc3..c13bbd852c 100644
> > --- a/target/s390x/cpu_models.c
> > +++ b/target/s390x/cpu_models.c
> > @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >      static const int feats[] = {
> >          S390_FEAT_STFLE,
> >          S390_FEAT_EXTENDED_IMMEDIATE,
> > -        S390_FEAT_EXTENDED_TRANSLATION_2,
> > -        S390_FEAT_LONG_DISPLACEMENT,
> >          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >          S390_FEAT_ETF2_ENH,
> >          S390_FEAT_STORE_CLOCK_FAST,
> > @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >      if (kvm_enabled()) {
> >          kvm_s390_get_host_cpu_model(&max_model, errp);
> >      } else {
> > -        /* TCG emulates a z900 (with some optional additional features) */
> > -        max_model.def = &s390_cpu_defs[0];
> > -        bitmap_copy(max_model.features, max_model.def->default_feat,
> > +        /* TCG emulates a z800 (with some optional additional features) */
> > +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> > +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >                      S390_FEAT_MAX);
> >          add_qemu_cpu_model_features(max_model.features);
> >      }
> > @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >      S390CPU *cpu = S390_CPU(obj);
> >  
> >      cpu->model = g_malloc0(sizeof(*cpu->model));
> > -    /* TCG emulates a z900 (with some optional additional features) */
> > -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> > +    /* TCG emulates a z800 (with some optional additional features) */
> > +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> > +           sizeof(s390_qemu_cpu_defs));
> 
> No changing the qemu model without compatibility handling.

This patch series is based on the patch from Thomas Huth. It means the
QEMU model is still based on a z900, but that it is possible to enable
some more features like etf2.

Aurelien
Aurelien Jarno June 1, 2017, 7:17 p.m. UTC | #4
On 2017-06-01 11:04, David Hildenbrand wrote:
> On 01.06.2017 10:38, David Hildenbrand wrote:
> > On 01.06.2017 00:01, Aurelien Jarno wrote:
> >> At the same time fix the TCG version of get_max_cpu_model to return the
> >> maximum model like on KVM. Remove the ETF2 and long-displacement
> > 
> > I don't understand the part
> > "fix the TCG version of get_max_cpu_model to return the maximum model
> > like on KVM".
> > 
> > Can you elaborate?
> > 
> >> facilities from the additional features as it is included in the z800.
> >>
> >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >> ---
> >>  target/s390x/cpu_models.c | 13 ++++++-------
> >>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >> index fc3cb25cc3..c13bbd852c 100644
> >> --- a/target/s390x/cpu_models.c
> >> +++ b/target/s390x/cpu_models.c
> >> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>      static const int feats[] = {
> >>          S390_FEAT_STFLE,
> >>          S390_FEAT_EXTENDED_IMMEDIATE,
> >> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >> -        S390_FEAT_LONG_DISPLACEMENT,
> >>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>          S390_FEAT_ETF2_ENH,
> >>          S390_FEAT_STORE_CLOCK_FAST,
> >> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>      if (kvm_enabled()) {
> >>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>      } else {
> >> -        /* TCG emulates a z900 (with some optional additional features) */
> >> -        max_model.def = &s390_cpu_defs[0];
> >> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >> +        /* TCG emulates a z800 (with some optional additional features) */
> >> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>                      S390_FEAT_MAX);
> 
> This is most likely wrong: you're indicating features here that are not
> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
> 
> I think should only copy the base features and add whatever else is
> available via add_qemu_cpu_model_features() as already done.

The patch series added all the z800 features exposed via STFL/STFLE.
Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
at all so the lack of these features are not exposed to the guest. In that
regard QEMU already wrongly claim to emulate a z900.


> >>          add_qemu_cpu_model_features(max_model.features);
> >>      }
> >> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>      S390CPU *cpu = S390_CPU(obj);
> >>  
> >>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >> -    /* TCG emulates a z900 (with some optional additional features) */
> >> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >> +    /* TCG emulates a z800 (with some optional additional features) */
> >> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >> +           sizeof(s390_qemu_cpu_defs));
> > 
> > No changing the qemu model without compatibility handling.
> > 
> Please have a look at the following mail for a possible solution:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
> 
> This could be moved to a separate patch. So this patch really should
> just care about the maximum model, not the qemu model.

From what I understand from this thread, the patch from Thomas Huth was
finally considered acceptable. I am adding him in Cc: so that he can
comment.

Aurelien
David Hildenbrand June 1, 2017, 7:56 p.m. UTC | #5
On 01.06.2017 21:17, Aurelien Jarno wrote:
> On 2017-06-01 10:38, David Hildenbrand wrote:
>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>
>> I don't understand the part
>> "fix the TCG version of get_max_cpu_model to return the maximum model
>> like on KVM".
>>
>> Can you elaborate?
> 
> Currently get_max_cpu_model returns the features of the base model, so
> for example the one of a z900 even on a z800. This makes impossible to
> enable the features that are provided by a z800 like etf2 or ldisp.
> 

Right, you can always change the max_cpu_model, e.g. bumping up the
version or adding new features, that is just fine.

> For what I understand from the KVM code (but I haven't tested), the
> function return all the features that are supported by the current CPU,
> not all the features that are supported by the base model of the current
> CPU.

Correct, for KVM it is the detected model, that means: Base features +
optional features.


> 
> 
>>> facilities from the additional features as it is included in the z800.
>>>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>> ---
>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index fc3cb25cc3..c13bbd852c 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>      static const int feats[] = {
>>>          S390_FEAT_STFLE,
>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>          S390_FEAT_ETF2_ENH,
>>>          S390_FEAT_STORE_CLOCK_FAST,
>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>      if (kvm_enabled()) {
>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>      } else {
>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>> -        max_model.def = &s390_cpu_defs[0];
>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>                      S390_FEAT_MAX);
>>>          add_qemu_cpu_model_features(max_model.features);
>>>      }
>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>      S390CPU *cpu = S390_CPU(obj);
>>>  
>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>> +           sizeof(s390_qemu_cpu_defs));
>>
>> No changing the qemu model without compatibility handling.
> 
> This patch series is based on the patch from Thomas Huth. It means the
> QEMU model is still based on a z900, but that it is possible to enable
> some more features like etf2.

Thomas' code did neither change features nor the "model definition". It
just allows for some more feature to be set. It is a hack.

I am pretty sure that expanding the "qemu" CPU model now (QMP
query-cpu-model-expansion) will indicate a z800, not a z900.

See cpu_info_from_model(). And that is a problem, because the QEMU CPU
model is a "migration-safe" CPU model, meaning it must remain equal for
every compatibility machine.

Thanks.

> 
> Aurelien
>
Thomas Huth June 2, 2017, 8:09 a.m. UTC | #6
On 01.06.2017 21:17, Aurelien Jarno wrote:
> On 2017-06-01 11:04, David Hildenbrand wrote:
>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>
>>> I don't understand the part
>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>> like on KVM".
>>>
>>> Can you elaborate?
>>>
>>>> facilities from the additional features as it is included in the z800.
>>>>
>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>> ---
>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>> index fc3cb25cc3..c13bbd852c 100644
>>>> --- a/target/s390x/cpu_models.c
>>>> +++ b/target/s390x/cpu_models.c
>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>      static const int feats[] = {
>>>>          S390_FEAT_STFLE,
>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>          S390_FEAT_ETF2_ENH,
>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>      if (kvm_enabled()) {
>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>      } else {
>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>> -        max_model.def = &s390_cpu_defs[0];
>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>                      S390_FEAT_MAX);
>>
>> This is most likely wrong: you're indicating features here that are not
>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>
>> I think should only copy the base features and add whatever else is
>> available via add_qemu_cpu_model_features() as already done.
> 
> The patch series added all the z800 features exposed via STFL/STFLE.
> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
> at all so the lack of these features are not exposed to the guest. In that
> regard QEMU already wrongly claim to emulate a z900.
> 
> 
>>>>          add_qemu_cpu_model_features(max_model.features);
>>>>      }
>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>>      S390CPU *cpu = S390_CPU(obj);
>>>>  
>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>>> +           sizeof(s390_qemu_cpu_defs));
>>>
>>> No changing the qemu model without compatibility handling.
>>>
>> Please have a look at the following mail for a possible solution:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
>>
>> This could be moved to a separate patch. So this patch really should
>> just care about the maximum model, not the qemu model.
> 
> From what I understand from this thread, the patch from Thomas Huth was
> finally considered acceptable. I am adding him in Cc: so that he can
> comment.

In the 2nd version of my patch, I only changed the full_feat of the
model definitions, but not the base_feat and default_feat fields, so
that the CPU stays the same when you run QEMU without "-cpu" parameter
(or simply with "-cpu qemu").

Consider that the following scenario should work: Start a QEMU v2.10
with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
migrate from the v2.10 to the v2.9 instance without problems. This won't
work anymore if you changed the default feature bits unconditionally.

 Thomas
David Hildenbrand June 2, 2017, 11:30 a.m. UTC | #7
On 02.06.2017 10:09, Thomas Huth wrote:
> On 01.06.2017 21:17, Aurelien Jarno wrote:
>> On 2017-06-01 11:04, David Hildenbrand wrote:
>>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>>
>>>> I don't understand the part
>>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>>> like on KVM".
>>>>
>>>> Can you elaborate?
>>>>
>>>>> facilities from the additional features as it is included in the z800.
>>>>>
>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>> ---
>>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>>> index fc3cb25cc3..c13bbd852c 100644
>>>>> --- a/target/s390x/cpu_models.c
>>>>> +++ b/target/s390x/cpu_models.c
>>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>>      static const int feats[] = {
>>>>>          S390_FEAT_STFLE,
>>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>>          S390_FEAT_ETF2_ENH,
>>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>>      if (kvm_enabled()) {
>>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>>      } else {
>>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>>> -        max_model.def = &s390_cpu_defs[0];
>>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>>                      S390_FEAT_MAX);
>>>
>>> This is most likely wrong: you're indicating features here that are not
>>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>>
>>> I think should only copy the base features and add whatever else is
>>> available via add_qemu_cpu_model_features() as already done.
>>
>> The patch series added all the z800 features exposed via STFL/STFLE.
>> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
>> at all so the lack of these features are not exposed to the guest. In that
>> regard QEMU already wrongly claim to emulate a z900.

Please note that:

a) SIE features were never part of the max model. QEMU never claimed
that. With your change one could suddenly do a -cpu z900,sie_f2=on,
which is wrong.

b) The SIE_F2 feature tells the guest that the SIE instruction is
available. E.g. Linux will look at this bit and show SIE support in
/proc/cpuinfo and unlock the KVM module.

Please, just don't add features to the MAX model that we don't implement.

>>
>>
>>>>>          add_qemu_cpu_model_features(max_model.features);
>>>>>      }
>>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>>>      S390CPU *cpu = S390_CPU(obj);
>>>>>  
>>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>>>> +           sizeof(s390_qemu_cpu_defs));
>>>>
>>>> No changing the qemu model without compatibility handling.
>>>>
>>> Please have a look at the following mail for a possible solution:
>>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
>>>
>>> This could be moved to a separate patch. So this patch really should
>>> just care about the maximum model, not the qemu model.
>>
>> From what I understand from this thread, the patch from Thomas Huth was
>> finally considered acceptable. I am adding him in Cc: so that he can
>> comment.
> 
> In the 2nd version of my patch, I only changed the full_feat of the
> model definitions, but not the base_feat and default_feat fields, so
> that the CPU stays the same when you run QEMU without "-cpu" parameter
> (or simply with "-cpu qemu").
> 
> Consider that the following scenario should work: Start a QEMU v2.10
> with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
> s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
> migrate from the v2.10 to the v2.9 instance without problems. This won't
> work anymore if you changed the default feature bits unconditionally.
> 
>  Thomas
> 

The following can be used to make sure the model does not change:

s390x-softmmu/qemu-system-s390x -S -qmp stdio

-> { "execute": "qmp_capabilities" }
-> { "execute": "query-cpu-model-expansion", "arguments": { "type":
"static", "model": { "name": "qemu" } } }

With existing compat machines, this has to give
-> "{"return": {"model": {"name": "z900-base"}}}"


Also, is there any specific reason why to go for a z800 and not a z900.3
? (IBM zSeries 900 GA3).

z900 GA3 is the EC model, z800 GA1 is the BC model. Both have same
features and capabilities. EC always smells more powerful ;) In the end
it doesn't matter.
Aurelien Jarno June 2, 2017, 1:40 p.m. UTC | #8
On 2017-06-01 21:56, David Hildenbrand wrote:
> On 01.06.2017 21:17, Aurelien Jarno wrote:
> > On 2017-06-01 10:38, David Hildenbrand wrote:
> >> On 01.06.2017 00:01, Aurelien Jarno wrote:
> >>> At the same time fix the TCG version of get_max_cpu_model to return the
> >>> maximum model like on KVM. Remove the ETF2 and long-displacement
> >>
> >> I don't understand the part
> >> "fix the TCG version of get_max_cpu_model to return the maximum model
> >> like on KVM".
> >>
> >> Can you elaborate?
> > 
> > Currently get_max_cpu_model returns the features of the base model, so
> > for example the one of a z900 even on a z800. This makes impossible to
> > enable the features that are provided by a z800 like etf2 or ldisp.
> > 
> 
> Right, you can always change the max_cpu_model, e.g. bumping up the
> version or adding new features, that is just fine.
> 
> > For what I understand from the KVM code (but I haven't tested), the
> > function return all the features that are supported by the current CPU,
> > not all the features that are supported by the base model of the current
> > CPU.
> 
> Correct, for KVM it is the detected model, that means: Base features +
> optional features.
> 
> 
> > 
> > 
> >>> facilities from the additional features as it is included in the z800.
> >>>
> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>> ---
> >>>  target/s390x/cpu_models.c | 13 ++++++-------
> >>>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >>> index fc3cb25cc3..c13bbd852c 100644
> >>> --- a/target/s390x/cpu_models.c
> >>> +++ b/target/s390x/cpu_models.c
> >>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>>      static const int feats[] = {
> >>>          S390_FEAT_STFLE,
> >>>          S390_FEAT_EXTENDED_IMMEDIATE,
> >>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >>> -        S390_FEAT_LONG_DISPLACEMENT,
> >>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>>          S390_FEAT_ETF2_ENH,
> >>>          S390_FEAT_STORE_CLOCK_FAST,
> >>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>>      if (kvm_enabled()) {
> >>>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>>      } else {
> >>> -        /* TCG emulates a z900 (with some optional additional features) */
> >>> -        max_model.def = &s390_cpu_defs[0];
> >>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >>> +        /* TCG emulates a z800 (with some optional additional features) */
> >>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>>                      S390_FEAT_MAX);
> >>>          add_qemu_cpu_model_features(max_model.features);
> >>>      }
> >>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>>      S390CPU *cpu = S390_CPU(obj);
> >>>  
> >>>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >>> -    /* TCG emulates a z900 (with some optional additional features) */
> >>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >>> +    /* TCG emulates a z800 (with some optional additional features) */
> >>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >>> +           sizeof(s390_qemu_cpu_defs));
> >>
> >> No changing the qemu model without compatibility handling.
> > 
> > This patch series is based on the patch from Thomas Huth. It means the
> > QEMU model is still based on a z900, but that it is possible to enable
> > some more features like etf2.
> 
> Thomas' code did neither change features nor the "model definition". It
> just allows for some more feature to be set. It is a hack.
> 
> I am pretty sure that expanding the "qemu" CPU model now (QMP
> query-cpu-model-expansion) will indicate a z800, not a z900.
> 
> See cpu_info_from_model(). And that is a problem, because the QEMU CPU
> model is a "migration-safe" CPU model, meaning it must remain equal for
> every compatibility machine.
> 

Ok, I get what you mean now.

Aurelien
Aurelien Jarno June 2, 2017, 2:04 p.m. UTC | #9
On 2017-06-02 13:30, David Hildenbrand wrote:
> On 02.06.2017 10:09, Thomas Huth wrote:
> > On 01.06.2017 21:17, Aurelien Jarno wrote:
> >> On 2017-06-01 11:04, David Hildenbrand wrote:
> >>> On 01.06.2017 10:38, David Hildenbrand wrote:
> >>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
> >>>>> At the same time fix the TCG version of get_max_cpu_model to return the
> >>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
> >>>>
> >>>> I don't understand the part
> >>>> "fix the TCG version of get_max_cpu_model to return the maximum model
> >>>> like on KVM".
> >>>>
> >>>> Can you elaborate?
> >>>>
> >>>>> facilities from the additional features as it is included in the z800.
> >>>>>
> >>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>>>> ---
> >>>>>  target/s390x/cpu_models.c | 13 ++++++-------
> >>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >>>>> index fc3cb25cc3..c13bbd852c 100644
> >>>>> --- a/target/s390x/cpu_models.c
> >>>>> +++ b/target/s390x/cpu_models.c
> >>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>>>>      static const int feats[] = {
> >>>>>          S390_FEAT_STFLE,
> >>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
> >>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >>>>> -        S390_FEAT_LONG_DISPLACEMENT,
> >>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>>>>          S390_FEAT_ETF2_ENH,
> >>>>>          S390_FEAT_STORE_CLOCK_FAST,
> >>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>>>>      if (kvm_enabled()) {
> >>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>>>>      } else {
> >>>>> -        /* TCG emulates a z900 (with some optional additional features) */
> >>>>> -        max_model.def = &s390_cpu_defs[0];
> >>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >>>>> +        /* TCG emulates a z800 (with some optional additional features) */
> >>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>>>>                      S390_FEAT_MAX);
> >>>
> >>> This is most likely wrong: you're indicating features here that are not
> >>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
> >>>
> >>> I think should only copy the base features and add whatever else is
> >>> available via add_qemu_cpu_model_features() as already done.
> >>
> >> The patch series added all the z800 features exposed via STFL/STFLE.
> >> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
> >> at all so the lack of these features are not exposed to the guest. In that
> >> regard QEMU already wrongly claim to emulate a z900.
> 
> Please note that:
> 
> a) SIE features were never part of the max model. QEMU never claimed
> that. With your change one could suddenly do a -cpu z900,sie_f2=on,
> which is wrong.
> 
> b) The SIE_F2 feature tells the guest that the SIE instruction is
> available. E.g. Linux will look at this bit and show SIE support in
> /proc/cpuinfo and unlock the KVM module.

I understand your point. That said I doubt we will support the SIE
instruction soon (it looks quite complicated and I can't find any doc).
As we are going to emulate more facilities to QEMU, it will be more and
more difficult to select a modern CPU. One will have to use -cpu,z900,
etf2=on,ldisp=on,...,eimm=on. I think we have to provide users with an
easier way to do that.

One possibility would be to filter the features that are not emulated by
QEMU (like on the ppc64 target) or to allow the user to specify a higher
model provided the non emulated features are disabled. Something like
-cpu z800,sie_f2=off.

> Please, just don't add features to the MAX model that we don't implement.

Please note that QEMU does not fully implement S390_FEAT_ESAN3 (though
that is addressed in this patchset) and S390_FEAT_ZARCH, despite
claiming it.

> >>>>>          add_qemu_cpu_model_features(max_model.features);
> >>>>>      }
> >>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>>>>      S390CPU *cpu = S390_CPU(obj);
> >>>>>  
> >>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >>>>> -    /* TCG emulates a z900 (with some optional additional features) */
> >>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >>>>> +    /* TCG emulates a z800 (with some optional additional features) */
> >>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >>>>> +           sizeof(s390_qemu_cpu_defs));
> >>>>
> >>>> No changing the qemu model without compatibility handling.
> >>>>
> >>> Please have a look at the following mail for a possible solution:
> >>>
> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
> >>>
> >>> This could be moved to a separate patch. So this patch really should
> >>> just care about the maximum model, not the qemu model.
> >>
> >> From what I understand from this thread, the patch from Thomas Huth was
> >> finally considered acceptable. I am adding him in Cc: so that he can
> >> comment.
> > 
> > In the 2nd version of my patch, I only changed the full_feat of the
> > model definitions, but not the base_feat and default_feat fields, so
> > that the CPU stays the same when you run QEMU without "-cpu" parameter
> > (or simply with "-cpu qemu").
> > 
> > Consider that the following scenario should work: Start a QEMU v2.10
> > with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
> > s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
> > migrate from the v2.10 to the v2.9 instance without problems. This won't
> > work anymore if you changed the default feature bits unconditionally.
> > 
> >  Thomas
> > 
> 
> The following can be used to make sure the model does not change:
> 
> s390x-softmmu/qemu-system-s390x -S -qmp stdio
> 
> -> { "execute": "qmp_capabilities" }
> -> { "execute": "query-cpu-model-expansion", "arguments": { "type":
> "static", "model": { "name": "qemu" } } }
> 
> With existing compat machines, this has to give
> -> "{"return": {"model": {"name": "z900-base"}}}"

Thanks for the hint.
 
> Also, is there any specific reason why to go for a z800 and not a z900.3
> ? (IBM zSeries 900 GA3).
> 
> z900 GA3 is the EC model, z800 GA1 is the BC model. Both have same
> features and capabilities. EC always smells more powerful ;) In the end
> it doesn't matter.

No real reason. It is further in the list, so it looks better to me.

Aurelien
David Hildenbrand June 2, 2017, 2:27 p.m. UTC | #10
On 02.06.2017 16:04, Aurelien Jarno wrote:
> On 2017-06-02 13:30, David Hildenbrand wrote:
>> On 02.06.2017 10:09, Thomas Huth wrote:
>>> On 01.06.2017 21:17, Aurelien Jarno wrote:
>>>> On 2017-06-01 11:04, David Hildenbrand wrote:
>>>>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>>>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>>>>
>>>>>> I don't understand the part
>>>>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>>>>> like on KVM".
>>>>>>
>>>>>> Can you elaborate?
>>>>>>
>>>>>>> facilities from the additional features as it is included in the z800.
>>>>>>>
>>>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>>>> ---
>>>>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>>>>> index fc3cb25cc3..c13bbd852c 100644
>>>>>>> --- a/target/s390x/cpu_models.c
>>>>>>> +++ b/target/s390x/cpu_models.c
>>>>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>>>>      static const int feats[] = {
>>>>>>>          S390_FEAT_STFLE,
>>>>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>>>>          S390_FEAT_ETF2_ENH,
>>>>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>>>>      if (kvm_enabled()) {
>>>>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>>>>      } else {
>>>>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>>>>> -        max_model.def = &s390_cpu_defs[0];
>>>>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>>>>                      S390_FEAT_MAX);
>>>>>
>>>>> This is most likely wrong: you're indicating features here that are not
>>>>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>>>>
>>>>> I think should only copy the base features and add whatever else is
>>>>> available via add_qemu_cpu_model_features() as already done.
>>>>
>>>> The patch series added all the z800 features exposed via STFL/STFLE.
>>>> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
>>>> at all so the lack of these features are not exposed to the guest. In that
>>>> regard QEMU already wrongly claim to emulate a z900.
>>
>> Please note that:
>>
>> a) SIE features were never part of the max model. QEMU never claimed
>> that. With your change one could suddenly do a -cpu z900,sie_f2=on,
>> which is wrong.
>>
>> b) The SIE_F2 feature tells the guest that the SIE instruction is
>> available. E.g. Linux will look at this bit and show SIE support in
>> /proc/cpuinfo and unlock the KVM module.
> 
> I understand your point. That said I doubt we will support the SIE
> instruction soon (it looks quite complicated and I can't find any doc).
> As we are going to emulate more facilities to QEMU, it will be more and
> more difficult to select a modern CPU. One will have to use -cpu,z900,
> etf2=on,ldisp=on,...,eimm=on. I think we have to provide users with an
> easier way to do that.
> 

Okay, I think you got something wrong here (which happens easily with
all these different model types).

CPU definitions:
- Base definition: Minimum features we expect to be around on such a CPU
- Default definition: Features we expect to be around in sane
  environments (== KVM on LPAR)
- Full definition: Maximum features that could be around on such a CPU
  (and there are ususally quite some missing, e.g. when running KVM
   under z/VM)

Only default and full definitions can ever change.

CPU models:
- Base model: maps to base definition == will never change
   -> e.g. "z900-base"
- Default model: maps to default definition == can change (but migration
  safe via compatibility machines!)
  -> e.g. "z900" or "qemu"
- Host model: only for KVM, maps to "max model"
- Max model: (not exposed yet directly for TCG) "maximum supported model
  + features"

Full models do not exist!
So in summary, what you want to do here is:

1. Get the definition of the z800 model. Copy the base features to the
   max model.
2. Add all features that are supported additionally and not in the base.
3. (Maybe add a hack like Thomas implemented to support some further
    features that are exceeding the full model)

So, especially, the CPU model "z800" does not map to the full model, but
the default model. It will never contain SIE features (at least for now)

-cpu z800 can therefore be used just fine, as it won't select SIE
features (especially because SIE features are also only available for
KVM in rare situations as nested virtualization has to be enabled
explcitily).

You can use

{ "execute": "query-cpu-model-expansion", "arguments": { "type": "full",
"model": { "name": "z800" } } }

To see all features that are currently part of the "default" model.

{"return": {"model": {"name": "z800-base", "props": {"pfmfi": false,
"exrl": false, "stfle45": false, "cmma": false, "dateh2": false,
"gen13ptff": false, "dateh": false, "iacc2": false, "parseh": false,
"csst": false, "idter": false, "idtes": false, "msa": false, "aefsi":
false, "csst2": false, "csske": false, "msa5": false, "msa4": false,
"msa3": false, "msa2": false, "msa1": false, "sthyi": false, "stckf":
false, "stfle": false, "etf3": false, "etf2": false, "edat": false,
"hfpm": false, "ri": false, "edat2": false, "hfpue": false, "dfp":
false, "mvcos": false, "sprogp": false, "sigpif": false, "ldisphp":
false, "vx": false, "ipter": false, "emon": false, "cei": false,
"cmpsceh": false, "ginste": false, "dfppc": false, "dfpzc": false,
"dfphp": false, "stfle49": false, "asnlxr": false, "gpereh": false,
"esop": false, "ectg": false, "ib": false, "siif": false, "esan3": true,
"fpe": false, "ibs": false, "zarch": true, "stfle53": false, "sief2":
false, "eimm": false, "srs": false, "cte": false, "fpseh": false,
"ltlbc": false, "ldisp": false, "64bscao": false, "ctop": false,
"etf3eh": false, "etf2eh": false, "nonqks": false, "pfpo": false, "te":
false, "cmm": false, "tods": false, "plo": true, "gsls": false, "skey":
false}}}}

Which is the same as
{ "execute": "query-cpu-model-expansion", "arguments": { "type":
"static", "model": { "name": "z800" } } }
{"return": {"model": {"name": "z800-base"}}}

So no SIE features.


> One possibility would be to filter the features that are not emulated by
> QEMU (like on the ppc64 target) or to allow the user to specify a higher
> model provided the non emulated features are disabled. Something like
> -cpu z800,sie_f2=off.
> 
>> Please, just don't add features to the MAX model that we don't implement.
> 
> Please note that QEMU does not fully implement S390_FEAT_ESAN3 (though
> that is addressed in this patchset) and S390_FEAT_ZARCH, despite
> claiming it.

Yes, there is a lot missing on the TCG side as I noticed already :) But
I think we're going into the right direction. (as long as Linux runs ...)
Thomas Huth June 2, 2017, 2:34 p.m. UTC | #11
On 02.06.2017 16:04, Aurelien Jarno wrote:
[...]
> I understand your point. That said I doubt we will support the SIE
> instruction soon (it looks quite complicated and I can't find any doc).

There is unfortunately no public documentation for the SIE instruction.
The only spec that is available is a very old one from the S/370 age:

http://bitsavers.trailing-edge.com/pdf/ibm/370/princOps/SA22-7095-0_370-XA_Interpretive_Execution_Jan84.pdf

 Thomas
David Hildenbrand June 2, 2017, 2:41 p.m. UTC | #12
On 02.06.2017 16:34, Thomas Huth wrote:
> On 02.06.2017 16:04, Aurelien Jarno wrote:
> [...]
>> I understand your point. That said I doubt we will support the SIE
>> instruction soon (it looks quite complicated and I can't find any doc).
> 
> There is unfortunately no public documentation for the SIE instruction.
> The only spec that is available is a very old one from the S/370 age:
> 
> http://bitsavers.trailing-edge.com/pdf/ibm/370/princOps/SA22-7095-0_370-XA_Interpretive_Execution_Jan84.pdf
> 

And of course the kvm (esp. vsie) implementation is very helpful
understanding how it works and what some bits mean :)

>  Thomas
>
diff mbox

Patch

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index fc3cb25cc3..c13bbd852c 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -668,8 +668,6 @@  static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
     static const int feats[] = {
         S390_FEAT_STFLE,
         S390_FEAT_EXTENDED_IMMEDIATE,
-        S390_FEAT_EXTENDED_TRANSLATION_2,
-        S390_FEAT_LONG_DISPLACEMENT,
         S390_FEAT_LONG_DISPLACEMENT_FAST,
         S390_FEAT_ETF2_ENH,
         S390_FEAT_STORE_CLOCK_FAST,
@@ -696,9 +694,9 @@  static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 (with some optional additional features) */
-        max_model.def = &s390_cpu_defs[0];
-        bitmap_copy(max_model.features, max_model.def->default_feat,
+        /* TCG emulates a z800 (with some optional additional features) */
+        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
+        bitmap_copy(max_model.features, max_model.def->full_feat,
                     S390_FEAT_MAX);
         add_qemu_cpu_model_features(max_model.features);
     }
@@ -956,8 +954,9 @@  static void s390_qemu_cpu_model_initfn(Object *obj)
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));
-    /* TCG emulates a z900 (with some optional additional features) */
-    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    /* TCG emulates a z800 (with some optional additional features) */
+    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
+           sizeof(s390_qemu_cpu_defs));
     add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
     cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,