On 02.07.19 22:04, Collin Walling wrote:
> 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.
A malicious or broken guest could just ignore the scp info bit.
In the end it is just architectural compliance. If the scp info bit is off, diag318 must
result in a specification exception, which is does not right now.
