Re: [PATCH v4 18/36] KVM: S390: protvirt: Introduce instruction data area bounce buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 24.02.20 20:13, David Hildenbrand wrote:
> On 24.02.20 12:40, Christian Borntraeger wrote:
>> From: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>
>> Now that we can't access guest memory anymore, we have a dedicated
>> satellite block that's a bounce buffer for instruction data.
>>
>> We re-use the memop interface to copy the instruction data to / from
>> userspace. This lets us re-use a lot of QEMU code which used that
>> interface to make logical guest memory accesses which are not possible
>> anymore in protected mode anyway.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
>> [borntraeger@xxxxxxxxxx: patch merging, splitting, fixing]
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> 
> [...]
> 
>> +
>>  long kvm_arch_vcpu_async_ioctl(struct file *filp,
>>  			       unsigned int ioctl, unsigned long arg)
>>  {
>> @@ -4683,7 +4732,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>>  		struct kvm_s390_mem_op mem_op;
>>  
>>  		if (copy_from_user(&mem_op, argp, sizeof(mem_op)) == 0)
>> -			r = kvm_s390_guest_mem_op(vcpu, &mem_op);
>> +			r = kvm_s390_guest_memsida_op(vcpu, &mem_op);
>>  		else
>>  			r = -EFAULT;
>>  		break;
>> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
>> index 014e53a41ead..cd81a58349a9 100644
>> --- a/arch/s390/kvm/pv.c
>> +++ b/arch/s390/kvm/pv.c
>> @@ -33,10 +33,13 @@ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc)
>>  	if (!cc)
>>  		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));
>>  	vcpu->arch.sie_block->sdf = 0;
>> +	vcpu->arch.sie_block->gbea = 1;
> 
> I am very confused why gbea is set to 1 when destroying the CPU. It's
> otherwise never set (always 0). What's the meaning of this?

This is the guest breaking event address. So a guest (and QEMU) can read it.
It is kind of overlaid sida and gbea. Something like this:

diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index cd81a58349a9..055bf0ec8fbb 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -39,6 +39,11 @@ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc)
        vcpu->arch.sie_block->pv_handle_config = 0;
        memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv));
        vcpu->arch.sie_block->sdf = 0;
+       /*
+        * the sidad field (for sdf == 2) is now the gbea field (for sdf == 0).
+        * Use the reset value of gbea to not leak the kernel pointer of the
+        * just free sida
+        */
        vcpu->arch.sie_block->gbea = 1;
        kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 



> 
> Apart from that, looks good to me.
> 
> 




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux