On 12/05/2017 10:13 AM, Cornelia Huck wrote:
> On Tue, 5 Dec 2017 09:33:21 +0100
> Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
>
>> From: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>>
>> All skey functions call skey_check_enable at their start, which checks
>> if we are in the PSTATE and injects a privileged operation exception
>> if we are.
>>
>> Unfortunately they continue processing afterwards and perform the
>> operation anyhow as skey_check_enable does not deliver an error if the
>> exception injection was successful.
>>
>> Let's move the PSTATE check into the skey functions and exit them on
>> such an occasion, also we now do not enable skey handling anymore in
>> such a case.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> Fixes: a7e19ab ("KVM: s390: handle missing storage-key facility")
>> Cc: <stable@xxxxxxxxxxxxxxx> # v4.8+
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>> arch/s390/kvm/priv.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
>
> This reminds me of something I stumbled upon the other day:
>
> handle_ri() and handle_gs() (both implemented in priv.c) don't seem to
> have a check for PSTATE, yet they enable ri/gs before retrying the
> instruction. Is that correct?
The guarded storage ops (e3 49 and e3 4d) are problem state.
Most of the ri instruction are as well, so problem state can enable RI
interpretion.
We could do some optimization to only enable RI if the
instruction would enable in for the guest (e.g. an inspection of the
control block could leave RI disabled). On the other hand that would
require to implement these instruction in KVM, which I would like
to avoid. Right now we enable RI and re-drive the instruction.
