[Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[David Hildenbrand]

STFL bit 4 and 5 are just indications to the guest, which TLB entries an
IDTE call will clear. These are performance indicators for the guest.

STFL bit 4:
    INVALIDATE DAT TABLE ENTRY (IDTE) performs
    the invalidation-and-clearing operation by
    selectively clearing TLB segment-table entries
    when a segment-table entry or entries are
    invalidated. IDTE also performs the clearing-by-
    ASCE operation. Unless bit 4 is one, IDTE simply
    purges all TLBs. Bit 3 is one if bit 4 is one.

We can simply set STFL bit 4 ("idtes") and still purge the complete TLB.
Purging more than advertised is never bad. E.g. Linux doesn't even care
about this bit. We can optimized this later.
This is helpful, as the z9 base model contains this facility.

STFL bit 5 (clearing TLB region-table-entries) was never implemented on
real HW, therefore we can simply ignore it for now.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu_models.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..f056357 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -676,6 +676,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 {
     static const int feats[] = {
         S390_FEAT_DAT_ENH,
+        S390_FEAT_IDTE_SEGMENT,
         S390_FEAT_STFLE,
         S390_FEAT_EXTENDED_IMMEDIATE,
         S390_FEAT_EXTENDED_TRANSLATION_2,
-- 
2.9.4

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Thomas Huth]

On 27.06.2017 18:10, David Hildenbrand wrote:
> STFL bit 4 and 5 are just indications to the guest, which TLB entries an
> IDTE call will clear. These are performance indicators for the guest.
> 
> STFL bit 4:
>     INVALIDATE DAT TABLE ENTRY (IDTE) performs
>     the invalidation-and-clearing operation by
>     selectively clearing TLB segment-table entries
>     when a segment-table entry or entries are
>     invalidated. IDTE also performs the clearing-by-
>     ASCE operation. Unless bit 4 is one, IDTE simply
>     purges all TLBs. Bit 3 is one if bit 4 is one.
> 
> We can simply set STFL bit 4 ("idtes") and still purge the complete TLB.
> Purging more than advertised is never bad. E.g. Linux doesn't even care
> about this bit. We can optimized this later.

Not sure, but why do we need this bit in add_qemu_cpu_model_features()
if Linux does not care about it? We will get it automatically once we
support the z9 in TCG...

 Thomas

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[David Hildenbrand]

On 28.06.2017 16:21, Thomas Huth wrote:
> On 27.06.2017 18:10, David Hildenbrand wrote:
>> STFL bit 4 and 5 are just indications to the guest, which TLB entries an
>> IDTE call will clear. These are performance indicators for the guest.
>>
>> STFL bit 4:
>>     INVALIDATE DAT TABLE ENTRY (IDTE) performs
>>     the invalidation-and-clearing operation by
>>     selectively clearing TLB segment-table entries
>>     when a segment-table entry or entries are
>>     invalidated. IDTE also performs the clearing-by-
>>     ASCE operation. Unless bit 4 is one, IDTE simply
>>     purges all TLBs. Bit 3 is one if bit 4 is one.
>>
>> We can simply set STFL bit 4 ("idtes") and still purge the complete TLB.
>> Purging more than advertised is never bad. E.g. Linux doesn't even care
>> about this bit. We can optimized this later.
> 
> Not sure, but why do we need this bit in add_qemu_cpu_model_features()
> if Linux does not care about it? We will get it automatically once we
> support the z9 in TCG...

The idea is to use this as a list we support in addition to the z900
features. This is later helpful when actually switching to a new model
(z9 might still take some time). Nobody has to do go over all features
again and see if they are implemented.

The commit subject also contains a description then why we can simply
enable it.

> 
>  Thomas
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Thomas Huth]

On 28.06.2017 19:02, David Hildenbrand wrote:
> On 28.06.2017 16:21, Thomas Huth wrote:
>> On 27.06.2017 18:10, David Hildenbrand wrote:
>>> STFL bit 4 and 5 are just indications to the guest, which TLB entries an
>>> IDTE call will clear. These are performance indicators for the guest.
>>>
>>> STFL bit 4:
>>>     INVALIDATE DAT TABLE ENTRY (IDTE) performs
>>>     the invalidation-and-clearing operation by
>>>     selectively clearing TLB segment-table entries
>>>     when a segment-table entry or entries are
>>>     invalidated. IDTE also performs the clearing-by-
>>>     ASCE operation. Unless bit 4 is one, IDTE simply
>>>     purges all TLBs. Bit 3 is one if bit 4 is one.
>>>
>>> We can simply set STFL bit 4 ("idtes") and still purge the complete TLB.
>>> Purging more than advertised is never bad. E.g. Linux doesn't even care
>>> about this bit. We can optimized this later.
>>
>> Not sure, but why do we need this bit in add_qemu_cpu_model_features()
>> if Linux does not care about it? We will get it automatically once we
>> support the z9 in TCG...
> 
> The idea is to use this as a list we support in addition to the z900
> features. This is later helpful when actually switching to a new model
> (z9 might still take some time). Nobody has to do go over all features
> again and see if they are implemented.

OK, I agree, that makes sense.

However, I'm not sure whether you can simply ignore the clearing-by-ASCE
stuff in this case. For example, according to the PoP:

"When the clearing-by-ASCE-option bit (bit 52 of gen-
 eral register R2 is one), the M4 field is ignored."

And the idte helper function currently always takes the M4 field into
account...

 Thomas

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Richard Henderson]

On 06/29/2017 12:05 AM, Thomas Huth wrote:
> However, I'm not sure whether you can simply ignore the clearing-by-ASCE
> stuff in this case. For example, according to the PoP:
> 
> "When the clearing-by-ASCE-option bit (bit 52 of gen-
>   eral register R2 is one), the M4 field is ignored."
> 
> And the idte helper function currently always takes the M4 field into
> account...

I don't see that quote.  I see only

    Bit 2 of the M 4 field is ignored for the
    clearing-by-ASCE operation and when EDAT-2
    does not apply.

We don't actually handle bit 2 at all...


r~

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Thomas Huth]

On 30.06.2017 21:22, Richard Henderson wrote:
> On 06/29/2017 12:05 AM, Thomas Huth wrote:
>> However, I'm not sure whether you can simply ignore the clearing-by-ASCE
>> stuff in this case. For example, according to the PoP:
>>
>> "When the clearing-by-ASCE-option bit (bit 52 of gen-
>>   eral register R2 is one), the M4 field is ignored."
>>
>> And the idte helper function currently always takes the M4 field into
>> account...
> 
> I don't see that quote.

Which version of the Principles of Operation are you using? I just
checked, and that sentence seems to be available in version SA22-7832-10
(but it is not in SA22-7832-09 yet).

 Thomas

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[David Hildenbrand]

On 03.07.2017 13:07, Thomas Huth wrote:
> On 30.06.2017 21:22, Richard Henderson wrote:
>> On 06/29/2017 12:05 AM, Thomas Huth wrote:
>>> However, I'm not sure whether you can simply ignore the clearing-by-ASCE
>>> stuff in this case. For example, according to the PoP:
>>>
>>> "When the clearing-by-ASCE-option bit (bit 52 of gen-
>>>   eral register R2 is one), the M4 field is ignored."
>>>
>>> And the idte helper function currently always takes the M4 field into
>>> account...
>>
>> I don't see that quote.
> 
> Which version of the Principles of Operation are you using? I just
> checked, and that sentence seems to be available in version SA22-7832-10
> (but it is not in SA22-7832-09 yet).
> 
>  Thomas
> 
> 

I think that part of the PoP is quite confusing.


"When the local-TLB-clearing facility is not installed [...]
the term “__specified__ CPU or CPUs” means all of the CPUs in the
configuration."

"When the local-TLB-clearing facility is installed [...]
the term “__specified__ CPU or CPUs” means only the CPU
executing the IDTE instruction (the local CPU)."

"When the clearing-by-ASCE-option bit, bit 52 of gen-
eral register R2, is zero [...] in the
__specified__ CPU or CPUs in the configuration are
cleared of (1) all TLB table entries of the designated [...]"

"When the clearing-by-ASCE-option bit is one [...]
but it does clear, from the TLBs in __all__ CPUs in the [...]"


So one could assume, that local-tlb-clearing does not apply to
clearing-by-ASCE-option. However:

"When bit 52 of general register R2, the clearing-by-
ASCE-option bit, is one, the clearing-by-ASCE oper-
ation is specified. [...] The TLBs of the __specified__ CPU or CPUs in
the configuration [...]"


But:

"When the clearing-by-ASCE-option bit (bit 52 of gen-
eral register R2 is one), the M4 field is ignored."

However also "Common Operation" section rather reads like
local-TLB-clearing applies to any mode.

Also the description of local-TLB-clearing is quite general:

"Local-TLB-Clearing Facility
The local-TLB-clearing facility may be available on a
model implementing z/Architecture. The facility may
provide performance improvements for the INVALI-
DATE DAT TABLE ENTRY and INVALIDATE PAGE
TABLE ENTRY instructions by allowing the program
to specify whether TLB clearing should be done in all
CPUs of the configuration or only in the CPU execut-
ing the instruction."


My interpretation:

local-TLB-clearing was introduced after IDTES but before EDAT2 (M4 bit
2). I think "m4 is ignored" should rather mean "M4 bit 2 is ignored".
Especially as this sentence was added later, I assume it is connected to
EDAT2, not local-TLB-clearing.


For now (!SMP) it should really matter. Opinions?


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG

[Richard Henderson]

On 07/03/2017 04:07 AM, Thomas Huth wrote:
> On 30.06.2017 21:22, Richard Henderson wrote:
>> On 06/29/2017 12:05 AM, Thomas Huth wrote:
>>> However, I'm not sure whether you can simply ignore the clearing-by-ASCE
>>> stuff in this case. For example, according to the PoP:
>>>
>>> "When the clearing-by-ASCE-option bit (bit 52 of gen-
>>>    eral register R2 is one), the M4 field is ignored."
>>>
>>> And the idte helper function currently always takes the M4 field into
>>> account...
>>
>> I don't see that quote.
> 
> Which version of the Principles of Operation are you using? I just
> checked, and that sentence seems to be available in version SA22-7832-10
> (but it is not in SA22-7832-09 yet).

I'm using -10.


r~