| Message ID | 1523819244-29954-4-git-send-email-akrowiak@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
> > 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?
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. > >
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.
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?
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?
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
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 >
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 >> >
On 04/18/2018 03:40 AM, 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? I will. >
On 04/18/2018 04:59 AM, David Hildenbrand wrote: > 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? I'll discuss it with the team and get back to you. >
On 04/18/2018 06:55 AM, 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? > >> 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? It's a matter of opinion. I prefer facilities because AP is comprised of not only instructions, but also AP processors. > > > Regards, > Halil
On 04/18/2018 06:55 AM, 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? I changed it by request and I thought it made sense to do so. Maybe we should just take a vote. > > >> 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
On 04/18/2018 07:03 AM, 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 ? It comes from the name of the function Query AP Configuration Information which is identified by the initials 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"), >>> 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 >> >
On 04/22/2018 05:43 PM, Tony Krowiak wrote: >>> + FEAT_INIT_MISC("ap", "AP facilities installed"), >> >> Why plural ('facilities')? Would not s/facilities/instructions be more end-user >> friendly? > > It's a matter of opinion. I prefer facilities because AP is comprised of not only > instructions, but also AP processors. Please elaborate. You mean processors like AP cards? If yes what if the matrix is empty (e.g. the state we decided is the default when no further action is taken (assign queues to the vfio-ap kernel driver, set up an mdev, -device vfio-ap on qemu cmd line))? I just wanted to point out that this plural is very vague. Not speaking about that QCI should be an AP facility too, but is not included in this 'features' and is not covered by this cpu model feature. It has it's own cpu model feature and even a dedicated STFLE bit. Regards, Halil
On 04/22/2018 05:52 PM, Tony Krowiak wrote: >>> + 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? > > I changed it by request and I thought it made sense to do so. Maybe we should just > take a vote. Sorry I've missed that request. Could you give me a link into the mail archive? I would like to examine the discussion. Thanks, Halil
On 04/22/2018 12:01 PM, Halil Pasic wrote: > > > On 04/22/2018 05:43 PM, Tony Krowiak wrote: >>>> + FEAT_INIT_MISC("ap", "AP facilities installed"), >>> >>> Why plural ('facilities')? Would not s/facilities/instructions be >>> more end-user >>> friendly? >> >> It's a matter of opinion. I prefer facilities because AP is comprised >> of not only >> instructions, but also AP processors. > > Please elaborate. You mean processors like AP cards? If yes what if > the matrix is > empty (e.g. the state we decided is the default when no further action > is taken > (assign queues to the vfio-ap kernel driver, set up an mdev, -device > vfio-ap on > qemu cmd line))? I don't want to get into a debate about this. If it means that much to you, I'll go ahead and change it. > > > I just wanted to point out that this plural is very vague. Not > speaking about that > QCI should be an AP facility too, but is not included in this > 'features' and is not > covered by this cpu model feature. It has it's own cpu model feature > and even a > dedicated STFLE bit. Uncle .... I got it. > > > Regards, > Halil
On 04/18/2018 04:59 AM, David Hildenbrand wrote: > 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? This is a business decision based on the following factors: 1. We do not have access to the older cards/systems for testing of the older devices. 2. Pre-CEX2 cards are no longer supported in the kernel with others to follow. 3. Keep the code base as small as possible and ensure it can be adequately tested. 4. Business investment priorities - i.e., $$$$'s. >
On 04/22/2018 12:01 PM, Halil Pasic wrote: > > > On 04/22/2018 05:43 PM, Tony Krowiak wrote: >>>> + FEAT_INIT_MISC("ap", "AP facilities installed"), >>> >>> Why plural ('facilities')? Would not s/facilities/instructions be >>> more end-user >>> friendly? >> >> It's a matter of opinion. I prefer facilities because AP is comprised >> of not only >> instructions, but also AP processors. > > Please elaborate. You mean processors like AP cards? If yes what if > the matrix is > empty (e.g. the state we decided is the default when no further action > is taken > (assign queues to the vfio-ap kernel driver, set up an mdev, -device > vfio-ap on > qemu cmd line))? > > I just wanted to point out that this plural is very vague. Not > speaking about that > QCI should be an AP facility too, but is not included in this > 'features' and is not > covered by this cpu model feature. It has it's own cpu model feature > and even a > dedicated STFLE bit. FYI, I decided to take your suggestion and go with 'AP instructions installed'. > > > Regards, > Halil
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)
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(-)