> @@ -4460,6 +4489,10 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>
> switch (mop->op) {
> case KVM_S390_MEMOP_LOGICAL_READ:
> + if (kvm_s390_pv_is_protected(vcpu->kvm)) {
> + r = -EINVAL;
> + break;
> + }
Could we have a possible race with disabling code, especially while
concurrently freeing? (sorry if I ask again, there was just a flood of
emails)
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> mop->size, GACC_FETCH);
> @@ -4472,6 +4505,10 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> }
> break;
> case KVM_S390_MEMOP_LOGICAL_WRITE:
> + if (kvm_s390_pv_is_protected(vcpu->kvm)) {
> + r = -EINVAL;
> + break;
> + }
dito
> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> r = check_gva_range(vcpu, mop->gaddr, mop->ar,
> mop->size, GACC_STORE);
> @@ -4483,6 +4520,11 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> }
> r = write_guest(vcpu, mop->gaddr, mop->ar, tmpbuf, mop->size);
> break;
> + case KVM_S390_MEMOP_SIDA_READ:
> + case KVM_S390_MEMOP_SIDA_WRITE:
> + /* we are locked against sida going away by the vcpu->mutex */
> + r = kvm_s390_guest_sida_op(vcpu, mop);
> + break;
> default:
> r = -EINVAL;
> }
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index 09573e36c329..80169a9b43ec 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -92,6 +92,7 @@ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc)
>
> free_pages(vcpu->arch.pv.stor_base,
> get_order(uv_info.guest_cpu_stor_len));
> + free_page(sida_origin(vcpu->arch.sie_block));
> vcpu->arch.sie_block->pv_handle_cpu = 0;
> vcpu->arch.sie_block->pv_handle_config = 0;
> memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv));
> @@ -121,6 +122,14 @@ int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc)
> uvcb.state_origin = (u64)vcpu->arch.sie_block;
> uvcb.stor_origin = (u64)vcpu->arch.pv.stor_base;
>
> + /* Alloc Secure Instruction Data Area Designation */
> + vcpu->arch.sie_block->sidad = __get_free_page(GFP_KERNEL | __GFP_ZERO);
> + if (!vcpu->arch.sie_block->sidad) {
> + free_pages(vcpu->arch.pv.stor_base,
> + get_order(uv_info.guest_cpu_stor_len));
> + return -ENOMEM;
> + }
> +
> cc = uv_call(0, (u64)&uvcb);
> *rc = uvcb.header.rc;
> *rrc = uvcb.header.rrc;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 207915488502..0fdee1bc3798 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -475,11 +475,15 @@ 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 sida_offset; /* offset into the sida */
> + __u8 reserved28[24]; /* should be set to 0 */
> };
As discussed, I'd prefer an overlaying layout for the sida, as the ar
does not make any sense (correct me if I'm wrong :) )
__u32 op; /* type of operation */
__u64 buf; /* buffer in userspace */
uinon {
__u8 ar; /* the access register number */
__u32 sida_offset; /* offset into the sida */
__u8 reserved[32]; /* should be set to 0 */
};
With something like that
Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
--
Thanks,
David / dhildenb
