Re: [PATCH v6 08/18] s390x: Add SIDA memory ops

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2590 bytes --]

On 3/4/20 6:39 PM, David Hildenbrand wrote:
> On 04.03.20 12:42, Janosch Frank wrote:
>> Protected guests save the instruction control blocks in the SIDA
>> instead of QEMU/KVM directly accessing the guest's memory.
>>
>> Let's introduce new functions to access the SIDA.
> 
> "The new memops are available with KVM_CAP_S390_PROTECTED, so let's
> check for that."

Ack

> 
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  linux-headers/linux/kvm.h |  2 ++
>>  target/s390x/cpu.h        |  7 ++++++-
>>  target/s390x/kvm.c        | 25 +++++++++++++++++++++++++
>>  target/s390x/kvm_s390x.h  |  2 ++
>>  target/s390x/mmu_helper.c | 14 ++++++++++++++
>>  5 files changed, 49 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> index e36f761194..c30344ab00 100644
>> --- a/linux-headers/linux/kvm.h
>> +++ b/linux-headers/linux/kvm.h
>> @@ -483,6 +483,8 @@ struct kvm_s390_mem_op {
>>  /* types for kvm_s390_mem_op->op */
>>  #define KVM_S390_MEMOP_LOGICAL_READ	0
>>  #define KVM_S390_MEMOP_LOGICAL_WRITE	1
>> +#define KVM_S390_MEMOP_SIDA_READ	2
>> +#define KVM_S390_MEMOP_SIDA_WRITE	3
>>  /* flags for kvm_s390_mem_op->flags */
>>  #define KVM_S390_MEMOP_F_CHECK_ONLY		(1ULL << 0)
>>  #define KVM_S390_MEMOP_F_INJECT_EXCEPTION	(1ULL << 1)
> 
> This should go into the header sync.

Ack

> 
> [...]
> 
>> index 7e4d9d267c..2578c838f8 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -824,7 +824,12 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>>  #define s390_cpu_virt_mem_check_write(cpu, laddr, ar, len)   \
>>          s390_cpu_virt_mem_rw(cpu, laddr, ar, NULL, len, true)
>>  void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra);
>> -
>> +int s390_cpu_pv_mem_rw(S390CPU *cpu, unsigned int offset,  void *hostbuf,
> 
> double space after "offset,"

Ack

> 
> [...]
> 
>>  
>> +int s390_cpu_pv_mem_rw(S390CPU *cpu, unsigned int offset, void *hostbuf,
>> +                       int len, bool is_write)
>> +{
>> +    int ret;
>> +
>> +    if (kvm_enabled()) {
>> +        ret = kvm_s390_mem_op_pv(cpu, offset, hostbuf, len, is_write);
>> +    } else {
>> +        /* Protected Virtualization is a KVM/Hardware only feature */
>> +        g_assert_not_reached();
>> +    }
>> +    return ret;
>> +}
> 
> 
> I suspect nobody will ever check the return value, correct?
> 
> With the things changed
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]