Re: [PATCH v5 2/2] s390/kvm: diagnose 318 handling

[Collin Walling <walling@linux.ibm.com>]

On 7/2/19 4:00 PM, Christian Borntraeger wrote:
> 
> 
> On 02.07.19 21:50, Collin Walling wrote:
>> On 6/26/19 10:31 AM, David Hildenbrand wrote:
>>> On 26.06.19 16:30, Collin Walling wrote:
>>>> On 6/26/19 6:28 AM, Christian Borntraeger wrote:
>>>>>
>>>>>
>>>>> On 26.06.19 11:45, David Hildenbrand wrote:
>>>>>
>>>>>>
>>>>>> BTW. there is currently no mechanism to fake absence of diag318. Should
>>>>>> we have one? (in contrast, for CMMA we have, which is also a CPU feature)
>>>>>
>>>>> Yes, we want to be able to disable diag318 via a CPU model feature. That actually
>>>>> means that the kernel must not answer this if we disable it.
>>>>>
>>>> Correct. If the guest specifies diag318=off, then the instruction
>>>> shouldn't be executed (it is fenced off in the kernel by checking the
>>>> Read SCP Info bit).
>>>
>>> But the guest *could* execute it and not get an exception.
>>>
>>
>> IIUC, you're talking about the situation where QEMU supports diag318,
>> but KVM does not. The worst case is the guest specifies diag318=on, and
>> nothing will stop the guest from attempting to execute the instruction.
>>
>> However, this is fenced in my QEMU patches. In (v5), I have this
>> following snippet:
>>
>> @@ -2323,6 +2345,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>>           KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
>>           set_bit(S390_FEAT_AP, model->features);
>>       }
>> +
>> +    /* if KVM supports interception of diag318, then let's provide the bit */
>> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_MISC,
>> +        KVM_S390_VM_MISC_DIAG318)) {
>> +        set_bit(S390_FEAT_DIAG318, model->features);
>> +    }
>> +
>>       /* strip of features that are not part of the maximum model */
>>       bitmap_and(model->features, model->features, model->def->full_feat,
>>                  S390_FEAT_MAX);
>>
>> If the guest specifies diag318=on, and KVM does *not* support emulation,
>> then the following message will be observed:
>>
>> qemu-system-s390x: Some features requested in the CPU model are not
>> available in the configuration: diag318
>>
>> and the guest will fail to start. Does this suffice?
> 
> But what happens if the guest ignores read scp info and executes diag318 anyway.
> It should get a specification exception, but it does not instead diag318 is
> executed.
> 

Hmm... I'm not following the logic here. When or how could diag318 be
executed without checking the feature bit? Are we concerned about other
hypervisors running a linux guest with diag318 enabled? Understanding
the scenario will help me follow along with this issue.