> #ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
> +static long kvm_s390_guest_sida_op(struct kvm_vcpu *vcpu,
> + struct kvm_s390_mem_op *mop)
> +{
> + int r = 0;
> + void __user *uaddr = (void __user *)mop->buf;
Reverse christmas tree :)
> +
> + if (mop->flags || !mop->size)
> + return -EINVAL;
> +
> + if (mop->size > sida_size(vcpu->arch.sie_block))
> + return -E2BIG;
Should be caught by the check below as well (or is this an implicit
overflow check? - see below).
> +
> + if (mop->size + mop->offset > sida_size(vcpu->arch.sie_block))
> + return -E2BIG;
> +
Do we have to care about overflows? (at least the offset is 32-bit,
didn't check the size :))
> + switch (mop->op) {
> + case KVM_S390_MEMOP_SIDA_READ:
> + r = 0;
> + if (copy_to_user(uaddr, (void *)sida_origin(vcpu->arch.sie_block) +
> + mop->offset, mop->size))
> + r = -EFAULT;
> +
> + break;
> + case KVM_S390_MEMOP_SIDA_WRITE:
> + r = 0;
> + if (copy_from_user((void *)vcpu->arch.sie_block->sidad +
> + mop->offset, uaddr, mop->size))
> + r = -EFAULT;
> + break;
> + }
> + return r;
> +}
> +
> static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
> struct kvm_pv_cmd *cmd)
> {
> @@ -4708,6 +4743,20 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> r = kvm_s390_handle_pv_vcpu(vcpu, &args);
> break;
> }
> + case KVM_S390_SIDA_OP: {
> + struct kvm_s390_mem_op mem_op;
> +
> + if (!kvm_s390_pv_is_protected(vcpu->kvm)) {
> + r = -EINVAL;
> + break;
> + }
Could we race against a VM_DESTROY? Should we protect somehow?
[...]
> -/* for KVM_S390_MEM_OP */
> +/* for KVM_S390_MEM_OP and KVM_S390_SIDA_OP */
> struct kvm_s390_mem_op {
> /* in */
> __u64 gaddr; /* the guest address */
> @@ -475,11 +475,17 @@ struct kvm_s390_mem_op {
> __u32 op; /* type of operation */
> __u64 buf; /* buffer in userspace */
> __u8 ar; /* the access register number */
> - __u8 reserved[31]; /* should be set to 0 */
> + __u8 reserved21[3]; /* should be set to 0 */
> + __u32 offset; /* offset into the sida */
maybe "side_offset"? or define a union, overlying the ar (because that
obviously doesn't apply to this memop). So eventually different layout
for different memop.
--
Thanks,
David / dhildenb
