Re: [PATCH v2 25/42] KVM: s390: protvirt: disallow one_reg

[David Hildenbrand <david@xxxxxxxxxx>]

On 14.02.20 23:26, Christian Borntraeger wrote:
> From: Janosch Frank <frankja@xxxxxxxxxxxxx>

"KVM: s390: protvirt: disallow KVM_GET_ONE_REG/KVM_SET_ONE_REG"

> 
> A lot of the registers are controlled by the Ultravisor and never
> visible to KVM. Some fields in the sie control block are overlayed, like
> gbea. As no known userspace uses the ONE_REG interface on s390 if sync
> regs are available, no functionality is lost if it is disabled for
> protected guests.
> 
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> [borntraeger@xxxxxxxxxx: patch merging, splitting, fixing]
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> ---
>  Documentation/virt/kvm/api.rst | 6 ++++--
>  arch/s390/kvm/kvm-s390.c       | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index cb58714fe60d..a82166e5f7d9 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -2117,7 +2117,8 @@ Errors:
>  
>    ======   ============================================================
>    ENOENT   no such register
> -  EINVAL   invalid register ID, or no such register
> +  EINVAL   invalid register ID, or no such register, ONE_REG forbidden
> +           for protected guests (s390)

"invalid register ID, no such register, or used with VMs in protected
virtualization mode on s390" ?

>    EPERM    (arm64) register access not allowed before vcpu finalization
>    ======   ============================================================
>  
> @@ -2552,7 +2553,8 @@ Errors include:
>  
>    ======== ============================================================
>    ENOENT   no such register
> -  EINVAL   invalid register ID, or no such register
> +  EINVAL   invalid register ID, or no such register, ONE_REG forbidden
> +           for protected guests (s390)

dito

>    EPERM    (arm64) register access not allowed before vcpu finalization
>    ======== ============================================================
>  
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 8db82aaf1275..d20a7fa9d480 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4638,6 +4638,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  	case KVM_SET_ONE_REG:
>  	case KVM_GET_ONE_REG: {
>  		struct kvm_one_reg reg;
> +		r = -EINVAL;
> +		if (kvm_s390_pv_is_protected(vcpu->kvm))
> +			break;

I assume races will be dealt with in your next series.

>  		r = -EFAULT;
>  		if (copy_from_user(&reg, argp, sizeof(reg)))
>  			break;
> 

With the two nits fixed

Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>

-- 
Thanks,

David / dhildenb