diff mbox series

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

Message ID 1525782303-16940-4-git-send-email-akrowiak@linux.vnet.ibm.com
State New
Headers show
Series s390x: vfio-ap: guest dedicated crypto adapters | expand

Commit Message

Tony Krowiak May 8, 2018, 12:25 p.m. UTC
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

   This feature will be supported for zEC12 and newer CPU models.
   The feature will not be supported for older models due to
   testability issues.

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

Pierre Morel May 15, 2018, noon UTC | #1
On 08/05/2018 14:25, Tony Krowiak wrote:
> 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
>
>     This feature will be supported for zEC12 and newer CPU models.
>     The feature will not be supported for older models due to
>     testability issues.
>
> 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(-)
>
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 3b9e274..f344323 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"),

Not a big deal, but why forget the I for "Information" in the long 
description for APQCI
Also why not just "QCI" (I think it was already asked)


>       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"),

Not a big deal too, but you always use AP instead of Adjunct Processor, 
why not here too?

>       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 instructions 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 e10035a..5d834b4 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 12b90cf..4d8c344 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2082,6 +2082,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)


LGTM despite the two little things with which I do not agree 100%

Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>
Tony Krowiak May 15, 2018, 3:03 p.m. UTC | #2
On 05/15/2018 08:00 AM, Pierre Morel wrote:
> On 08/05/2018 14:25, Tony Krowiak wrote:
>> 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
>>
>>     This feature will be supported for zEC12 and newer CPU models.
>>     The feature will not be supported for older models due to
>>     testability issues.
>>
>> 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(-)
>>
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 3b9e274..f344323 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"),
>
> Not a big deal, but why forget the I for "Information" in the long 
> description for APQCI

I'll add 'Information'.

>
> Also why not just "QCI" (I think it was already asked)

It was a suggestion from Reinhard with which I agreed. We may know that 
QCI is an AP function,
but most administrators will have no idea. Prepending the 'ap' informs 
that QCI is an
AP function related to the CPU model feature for AP.

>
>
>>       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"),
>
> Not a big deal too, but you always use AP instead of Adjunct 
> Processor, why not here too?

No reason, I can change it if you like.

>
>
>>       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 instructions 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 e10035a..5d834b4 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 12b90cf..4d8c344 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -2082,6 +2082,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)
>
>
> LGTM despite the two little things with which I do not agree 100%

>
>
> Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>
>
Pierre Morel May 16, 2018, 9:05 a.m. UTC | #3
On 15/05/2018 17:03, Tony Krowiak wrote:
> On 05/15/2018 08:00 AM, Pierre Morel wrote:
>> On 08/05/2018 14:25, Tony Krowiak wrote:
>>> 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
>>>
>>>     This feature will be supported for zEC12 and newer CPU models.
>>>     The feature will not be supported for older models due to
>>>     testability issues.
>>>
>>> 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(-)
>>>
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 3b9e274..f344323 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"),
>>
>> Not a big deal, but why forget the I for "Information" in the long 
>> description for APQCI
>
> I'll add 'Information'.
>
>>
>> Also why not just "QCI" (I think it was already asked)
>
> It was a suggestion from Reinhard with which I agreed. We may know 
> that QCI is an AP function,
> but most administrators will have no idea. Prepending the 'ap' informs 
> that QCI is an
> AP function related to the CPU model feature for AP.

QCI is the official name and will be refered as this in the official 
documentation (if it is).
Most admin will use libvirt anyway and the one which will try to use 
qemu will look for
apqci in the official documentation and will not find it.
I do not think it is a good idea, but technically does not change anything.
Keep my RB even you stay by apqci or change for qci.

>
>>
>>
>>>       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"),
>>
>> Not a big deal too, but you always use AP instead of Adjunct 
>> Processor, why not here too?
>
> No reason, I can change it if you like.
>
>>
>>
>>>       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 instructions 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 e10035a..5d834b4 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 12b90cf..4d8c344 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2082,6 +2082,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)
>>
>>
>> LGTM despite the two little things with which I do not agree 100%
>
>>
>>
>> Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>
>>
>
David Hildenbrand May 16, 2018, 9:23 a.m. UTC | #4
On 16.05.2018 11:05, Pierre Morel wrote:
> On 15/05/2018 17:03, Tony Krowiak wrote:
>> On 05/15/2018 08:00 AM, Pierre Morel wrote:
>>> On 08/05/2018 14:25, Tony Krowiak wrote:
>>>> 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
>>>>
>>>>     This feature will be supported for zEC12 and newer CPU models.
>>>>     The feature will not be supported for older models due to
>>>>     testability issues.
>>>>
>>>> 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(-)
>>>>
>>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>>> index 3b9e274..f344323 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"),
>>>
>>> Not a big deal, but why forget the I for "Information" in the long 
>>> description for APQCI
>>
>> I'll add 'Information'.
>>
>>>
>>> Also why not just "QCI" (I think it was already asked)
>>
>> It was a suggestion from Reinhard with which I agreed. We may know 
>> that QCI is an AP function,
>> but most administrators will have no idea. Prepending the 'ap' informs 
>> that QCI is an
>> AP function related to the CPU model feature for AP.
> 
> QCI is the official name and will be refered as this in the official 
> documentation (if it is).
> Most admin will use libvirt anyway and the one which will try to use 
> qemu will look for
> apqci in the official documentation and will not find it.
> I do not think it is a good idea, but technically does not change anything.
> Keep my RB even you stay by apqci or change for qci.
> 

For the SIE features I decided to not name them sie_$feat

So we have e.g. siif instead of sie_siif. I primarily did this to have
shorter feature names and the rational was that the short version (siif)
was sufficient to guess the full name and where it belongs to.

e.g. siif == "Shared IPTE-interlock facility" (we sticked to the f in
there, as siif was a commonly used term if I remember correctly). There
is only one shared ipte-interlock facility.

"qci" (or Query Configuration facility) does _not_ indicate to which
part of the system this belongs. zPCI? sclp? ap?

This should be "apqci" or "qapcf". Or "ap_qci". "qci", on its own is not
sufficient in my opinion.
Tony Krowiak May 16, 2018, 10:41 a.m. UTC | #5
On 05/16/2018 05:23 AM, David Hildenbrand wrote:
> On 16.05.2018 11:05, Pierre Morel wrote:
>> On 15/05/2018 17:03, Tony Krowiak wrote:
>>> On 05/15/2018 08:00 AM, Pierre Morel wrote:
>>>> On 08/05/2018 14:25, Tony Krowiak wrote:
>>>>> 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
>>>>>
>>>>>      This feature will be supported for zEC12 and newer CPU models.
>>>>>      The feature will not be supported for older models due to
>>>>>      testability issues.
>>>>>
>>>>> 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(-)
>>>>>
>>>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>>>> index 3b9e274..f344323 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"),
>>>> Not a big deal, but why forget the I for "Information" in the long
>>>> description for APQCI
>>> I'll add 'Information'.
>>>
>>>> Also why not just "QCI" (I think it was already asked)
>>> It was a suggestion from Reinhard with which I agreed. We may know
>>> that QCI is an AP function,
>>> but most administrators will have no idea. Prepending the 'ap' informs
>>> that QCI is an
>>> AP function related to the CPU model feature for AP.
>> QCI is the official name and will be refered as this in the official
>> documentation (if it is).
>> Most admin will use libvirt anyway and the one which will try to use
>> qemu will look for
>> apqci in the official documentation and will not find it.
>> I do not think it is a good idea, but technically does not change anything.
>> Keep my RB even you stay by apqci or change for qci.
>>
> For the SIE features I decided to not name them sie_$feat
>
> So we have e.g. siif instead of sie_siif. I primarily did this to have
> shorter feature names and the rational was that the short version (siif)
> was sufficient to guess the full name and where it belongs to.
>
> e.g. siif == "Shared IPTE-interlock facility" (we sticked to the f in
> there, as siif was a commonly used term if I remember correctly). There
> is only one shared ipte-interlock facility.
>
> "qci" (or Query Configuration facility) does _not_ indicate to which
> part of the system this belongs. zPCI? sclp? ap?
>
> This should be "apqci" or "qapcf". Or "ap_qci". "qci", on its own is not
> sufficient in my opinion.

It is settled then ... I'll stick with apqci

>
diff mbox series

Patch

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 3b9e274..f344323 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 instructions 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 e10035a..5d834b4 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 12b90cf..4d8c344 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2082,6 +2082,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)