Re: [PATCH 2/2] s390x: pv: Fix diag318 PV fencing

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2148 bytes --]

On 10/21/20 4:14 PM, David Hildenbrand wrote:
> On 21.10.20 15:43, Janosch Frank wrote:
>> Diag318 fencing needs to be determined on the current VM PV state and
>> not on the state that the VM has when we create the CPU model.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Fixes: fabdada935 ("s390: guest support for diagnose 0x318")
>> Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> ---
>>  hw/s390x/sclp.c    | 10 ++++++++++
>>  target/s390x/kvm.c |  3 +--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 0cf2290826..69aba402d3 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -22,6 +22,7 @@
>>  #include "hw/s390x/event-facility.h"
>>  #include "hw/s390x/s390-pci-bus.h"
>>  #include "hw/s390x/ipl.h"
>> +#include "hw/s390x/pv.h"
>>  
>>  static inline SCLPDevice *get_sclp_device(void)
>>  {
>> @@ -142,6 +143,15 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>      if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
>>          s390_get_feat_block(S390_FEAT_TYPE_SCLP_FAC134,
>>                              &read_info->fac134);
>> +        /*
>> +         * Diag318 is not available in protected mode and will result
>> +         * in an operation exception. As we can dynamically move in
>> +         * and out of protected mode, we need to fence the feature
>> +         * here rather than when creating the CPU model.
>> +         */
>> +        if (s390_is_pv()) {
>> +            read_info->fac134 &= ~0x80;
>> +        }
> 
> Hmm, I thought firmware would handle exposing cpu features and similar,
> so we can't temper with it ....

STFLE data is indeed provided by the Ultravisor, but SCLP is still done
by QEMU as that data is often not machine specific as visible in this case

> 
> Can we move that into s390_get_feat_block instead and check against the
> feature bit instead?
> 
You mean fence inside the s390_get_feat_block() function?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]