[v4,3/5] s390x/cpumodel: Set up CPU model for AP device support

Message ID 1523819244-29954-4-git-send-email-akrowiak@linux.vnet.ibm.com
State New
Headers show
Series
  • Untitled series #38948
Related show

Commit Message

Tony Krowiak April 15, 2018, 7:07 p.m.
A new CPU model feature and two new CPU model facilities are
introduced to support AP devices for a KVM guest.

CPU model features:

1. The KVM_S390_VM_CPU_FEAT_AP CPU model feature indicates that
   AP facilities are installed. This feature will be enabled by
   the kernel only if the AP facilities are installed on the linux
   host. This feature must be turned on from userspace to access
   AP devices from the KVM guest. The QEMU command line to turn
   this feature looks something like this:

	qemu-system-s390x ... -cpu xxx,ap=on

CPU model facilities:

1. The S390_FEAT_AP_QUERY_CONFIG_INFO feature indicates the AP Query
   Configuration Information (QCI) facility is installed. This feature
   will be enabled by the kernel only if the QCI is installed on
   the host.

2. The S390_FEAT_AP_FACILITY_TEST feature indicates that the AP
   Facility Test (APFT) facility is installed. This feature will
   be enabled by the kernel only if the APFT facility is installed
   on the host.

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 target/s390x/cpu_features.c     |    3 +++
 target/s390x/cpu_features_def.h |    3 +++
 target/s390x/cpu_models.c       |    2 ++
 target/s390x/gen-features.c     |    3 +++
 target/s390x/kvm.c              |    1 +
 5 files changed, 12 insertions(+), 0 deletions(-)

Comments

David Hildenbrand April 16, 2018, 3:44 p.m. | #1
>  
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 0cdbc15..0d5b0f7 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>      S390_FEAT_EDAT_2,
>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
> +    S390_FEAT_AP_FACILITIES_TEST,
> +    S390_FEAT_AP,
>  };
>  

Now I have to ask a very stupid question:

I heard that the execution controls in the SIE block for AP are one of
the oldest ones we have around. How can it be that the AP feature cannot
be used before zEC12?
Tony Krowiak April 17, 2018, 6:20 p.m. | #2
On 04/16/2018 11:44 AM, David Hildenbrand wrote:
>>   
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 0cdbc15..0d5b0f7 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>       S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>       S390_FEAT_EDAT_2,
>>       S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>> +    S390_FEAT_AP_FACILITIES_TEST,
>> +    S390_FEAT_AP,
>>   };
>>   
> Now I have to ask a very stupid question:
>
> I heard that the execution controls in the SIE block for AP are one of
> the oldest ones we have around. How can it be that the AP feature cannot
> be used before zEC12?

Its not a stupid question, but I don't have an answer because this was
not a design decision I made. I will consult with the crypto team to
see if I can get you an answer.

>
>
Christian Borntraeger April 17, 2018, 6:21 p.m. | #3
On 04/16/2018 05:44 PM, David Hildenbrand wrote:
> 
>>  
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 0cdbc15..0d5b0f7 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>      S390_FEAT_EDAT_2,
>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>> +    S390_FEAT_AP_FACILITIES_TEST,
>> +    S390_FEAT_AP,
>>  };
>>  
> 
> Now I have to ask a very stupid question:
> 
> I heard that the execution controls in the SIE block for AP are one of
> the oldest ones we have around. How can it be that the AP feature cannot
> be used before zEC12?

It was a suggestion from the crypto team due to testability. Nobody has a z196
with the older cards.
Cornelia Huck April 18, 2018, 7:40 a.m. | #4
On Tue, 17 Apr 2018 20:21:31 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
> >   
> >>  
> >> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> >> index 0cdbc15..0d5b0f7 100644
> >> --- a/target/s390x/gen-features.c
> >> +++ b/target/s390x/gen-features.c
> >> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
> >>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
> >>      S390_FEAT_EDAT_2,
> >>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> >> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
> >> +    S390_FEAT_AP_FACILITIES_TEST,
> >> +    S390_FEAT_AP,
> >>  };
> >>    
> > 
> > Now I have to ask a very stupid question:
> > 
> > I heard that the execution controls in the SIE block for AP are one of
> > the oldest ones we have around. How can it be that the AP feature cannot
> > be used before zEC12?  
> 
> It was a suggestion from the crypto team due to testability. Nobody has a z196
> with the older cards.
> 

Might be worth adding a note to that respect?
David Hildenbrand April 18, 2018, 8:59 a.m. | #5
On 18.04.2018 09:40, Cornelia Huck wrote:
> On Tue, 17 Apr 2018 20:21:31 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
>>>   
>>>>  
>>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>>> index 0cdbc15..0d5b0f7 100644
>>>> --- a/target/s390x/gen-features.c
>>>> +++ b/target/s390x/gen-features.c
>>>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>>>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>>>      S390_FEAT_EDAT_2,
>>>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>>>> +    S390_FEAT_AP_FACILITIES_TEST,
>>>> +    S390_FEAT_AP,
>>>>  };
>>>>    
>>>
>>> Now I have to ask a very stupid question:
>>>
>>> I heard that the execution controls in the SIE block for AP are one of
>>> the oldest ones we have around. How can it be that the AP feature cannot
>>> be used before zEC12?  
>>
>> It was a suggestion from the crypto team due to testability. Nobody has a z196
>> with the older cards.
>>
> 
> Might be worth adding a note to that respect?
> 

We used to have the CPU model stick as close as possible to the real CPU
models.

Support statements should cover in specific products what is expected to
work and what not.

So is there any real (!support statement / !testability) reason to not
allow this feature on older CPU models that also had support for it?
Halil Pasic April 18, 2018, 10:55 a.m. | #6
On 04/15/2018 09:07 PM, Tony Krowiak wrote:
> A new CPU model feature and two new CPU model facilities are
> introduced to support AP devices for a KVM guest.
[..]
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 3b9e274..5ee3a2d 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),

Why did you change this form qci to apqci. Too may people found the
qci good?

>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
> 
>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
> +    FEAT_INIT_MISC("ap", "AP facilities installed"),

Why plural ('facilities')? Would not s/facilities/instructions be more end-user
friendly?

Regards,
Halil
David Hildenbrand April 18, 2018, 11:03 a.m. | #7
On 18.04.2018 12:55, Halil Pasic wrote:
> 
> 
> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>> A new CPU model feature and two new CPU model facilities are
>> introduced to support AP devices for a KVM guest.
> [..]
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 3b9e274..5ee3a2d 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
> 
> Why did you change this form qci to apqci. Too may people found the
> qci good?

If the facility is called "Query AP Configuration facility" it should be
qapc

Where does the term "qci" come from ?

> 
>>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>
>>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
> 
> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
> friendly?
> 
> Regards,
> Halil
>
Pierre Morel April 18, 2018, 11:50 a.m. | #8
On 18/04/2018 13:03, David Hildenbrand wrote:
> On 18.04.2018 12:55, Halil Pasic wrote:
>>
>> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>>> A new CPU model feature and two new CPU model facilities are
>>> introduced to support AP devices for a KVM guest.
>> [..]
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 3b9e274..5ee3a2d 100644
>>> --- a/target/s390x/cpu_features.c
>>> +++ b/target/s390x/cpu_features.c
>>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>>        FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>>>        FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>>>        FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
>>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
>> Why did you change this form qci to apqci. Too may people found the
>> qci good?
> If the facility is called "Query AP Configuration facility" it should be
> qapc
>
> Where does the term "qci" come from ?

Query Configuration Information

:)


>
>>>        FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>>>        FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
>>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>>>        FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>>>        FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>>>        FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
>>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>>
>>>        FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>>        FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
>> friendly?
>>
>> Regards,
>> Halil
>>
>

Patch

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 3b9e274..5ee3a2d 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -40,8 +40,10 @@  static const S390FeatDef s390_features[] = {
     FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
     FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
     FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
+    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
     FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
     FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
+    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
     FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
     FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
     FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
@@ -129,6 +131,7 @@  static const S390FeatDef s390_features[] = {
 
     FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
     FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
+    FEAT_INIT_MISC("ap", "AP facilities installed"),
 
     FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit in general registers)"),
     FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 bit in parameter list)"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 7c5915c..8998b65 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -27,8 +27,10 @@  typedef enum {
     S390_FEAT_SENSE_RUNNING_STATUS,
     S390_FEAT_CONDITIONAL_SSKE,
     S390_FEAT_CONFIGURATION_TOPOLOGY,
+    S390_FEAT_AP_QUERY_CONFIG_INFO,
     S390_FEAT_IPTE_RANGE,
     S390_FEAT_NONQ_KEY_SETTING,
+    S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_EXTENDED_TRANSLATION_2,
     S390_FEAT_MSA,
     S390_FEAT_LONG_DISPLACEMENT,
@@ -118,6 +120,7 @@  typedef enum {
     /* Misc */
     S390_FEAT_DAT_ENH_2,
     S390_FEAT_CMM,
+    S390_FEAT_AP,
 
     /* PLO */
     S390_FEAT_PLO_CL,
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2741b68..2ab59cd 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -770,6 +770,8 @@  static void check_consistency(const S390CPUModel *model)
         { S390_FEAT_PRNO_TRNG_QRTCR, S390_FEAT_MSA_EXT_5 },
         { S390_FEAT_PRNO_TRNG, S390_FEAT_MSA_EXT_5 },
         { S390_FEAT_SIE_KSS, S390_FEAT_SIE_F2 },
+        { S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_AP },
+        { S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP },
     };
     int i;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 0cdbc15..0d5b0f7 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -447,6 +447,9 @@  static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_ADAPTER_INT_SUPPRESSION,
     S390_FEAT_EDAT_2,
     S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
+    S390_FEAT_AP_QUERY_CONFIG_INFO,
+    S390_FEAT_AP_FACILITIES_TEST,
+    S390_FEAT_AP,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index fb59d92..8c251b4 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2100,6 +2100,7 @@  static int kvm_to_feat[][2] = {
     { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
     { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
     { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
+    { KVM_S390_VM_CPU_FEAT_AP, S390_FEAT_AP},
 };
 
 static int query_cpu_feat(S390FeatBitmap features)