[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] s390/kvm: Support for get/set of extende
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] s390/kvm: Support for get/set of extended TOD-Clock for guest |
Date: |
Wed, 4 Oct 2017 13:44:31 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 10/04/2017 01:42 PM, Thomas Huth wrote:
> On 04.10.2017 12:57, Christian Borntraeger wrote:
>> From: "Collin L. Walling" <address@hidden>
>>
>> Provides an interface for getting and setting the guest's extended
>> TOD-Clock via a single ioctl to kvm. If the ioctl fails because it
>> is not support by kvm, then we fall back to the old style of
>> retrieving the clock via two ioctls.
>>
>> Signed-off-by: Collin L. Walling <address@hidden>
>> Reviewed-by: Eric Farman <address@hidden>
>> Reviewed-by: Claudio Imbrenda <address@hidden>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> [split failure change from epoch index change]
>> ---
>> target/s390x/cpu.c | 26 +++++++++++++++++++-------
>> target/s390x/kvm-stub.c | 10 ++++++++++
>> target/s390x/kvm.c | 35 ++++++++++++++++++++++++++++++++++-
>> target/s390x/kvm_s390x.h | 2 ++
>> 4 files changed, 65 insertions(+), 8 deletions(-)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 34538c3..c8f1219 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -357,22 +357,34 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state,
>> S390CPU *cpu)
>>
>> int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low)
>> {
>> + int r = 0;
>> +
>> if (kvm_enabled()) {
>> - return kvm_s390_get_clock(tod_high, tod_low);
>> + r = kvm_s390_get_clock_ext(tod_high, tod_low);
>> + if (r == -ENXIO) {
>> + return kvm_s390_get_clock(tod_high, tod_low);
>> + }
>> + } else {
>> + /* Fixme TCG */
>> + *tod_high = 0;
>> + *tod_low = 0;
>> }
>> - /* Fixme TCG */
>> - *tod_high = 0;
>> - *tod_low = 0;
>> - return 0;
>> +
>> + return r;
>> }
>>
>> int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
>> {
>> + int r = 0;
>> +
>> if (kvm_enabled()) {
>> - return kvm_s390_set_clock(tod_high, tod_low);
>> + r = kvm_s390_set_clock_ext(tod_high, tod_low);
>> + if (r == -ENXIO) {
>> + return kvm_s390_set_clock(tod_high, tod_low);
>> + }
>> }
>> /* Fixme TCG */
>> - return 0;
>> + return r;
>> }
>>
>> int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
>> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
>> index 261e1cd..43f02c2 100644
>> --- a/target/s390x/kvm-stub.c
>> +++ b/target/s390x/kvm-stub.c
>> @@ -68,11 +68,21 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t
>> *tod_low)
>> return -ENOSYS;
>> }
>>
>> +int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_low)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
>> {
>> return -ENOSYS;
>> }
>>
>> +int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t *tod_low)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> void kvm_s390_enable_css_support(S390CPU *cpu)
>> {
>> }
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index ebb75ca..4c944a5 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -643,10 +643,27 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t
>> *tod_low)
>> return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
>> }
>>
>> +int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_low)
>> +{
>> + int r;
>> + struct kvm_s390_vm_tod_clock gtod;
>> +
>
> So you've got a blank line here...
Yes, seems that I have forgotten this one.
I will let Conny decide if I should resend or if she can fixup.
>
>> + struct kvm_device_attr attr = {
>> + .group = KVM_S390_VM_TOD,
>> + .attr = KVM_S390_VM_TOD_EXT,
>> + .addr = (uint64_t)(>od),
>> + };
>> +
>> + r = kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
>> + *tod_high = gtod.epoch_idx;
>> + *tod_low = gtod.tod;
>> +
>> + return r;
>> +}
>> +
>> int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
>> {
>> int r;
>> -
>
> ... but removed the blank line here ... it would be more consequent to
> do it the same way everywhere?
>
>> struct kvm_device_attr attr = {
>> .group = KVM_S390_VM_TOD,
>> .attr = KVM_S390_VM_TOD_LOW,
>> @@ -663,6 +680,22 @@ int kvm_s390_set_clock(uint8_t *tod_high, uint64_t
>> *tod_low)
>> return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>> }
>>
>> +int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t *tod_low)
>> +{
>> + struct kvm_s390_vm_tod_clock gtod = {
>> + .epoch_idx = *tod_high,
>> + .tod = *tod_low,
>> + };
>> +
>> + struct kvm_device_attr attr = {
>> + .group = KVM_S390_VM_TOD,
>> + .attr = KVM_S390_VM_TOD_EXT,
>> + .addr = (uint64_t)(>od),
>
> You could remove the parentheses around ">od" (same in the
> get_clock_ext function).
dito.
>
>> + };
>> +
>> + return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>> +}
>> +
>> /**
>> * kvm_s390_mem_op:
>> * @addr: the logical start address in guest memory
>> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
>> index 2d594bd..501fc5a 100644
>> --- a/target/s390x/kvm_s390x.h
>> +++ b/target/s390x/kvm_s390x.h
>> @@ -29,7 +29,9 @@ int kvm_s390_vcpu_interrupt_post_load(S390CPU *cpu);
>> int kvm_s390_get_ri(void);
>> int kvm_s390_get_gs(void);
>> int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>> +int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>> int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_clock);
>> +int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>> void kvm_s390_enable_css_support(S390CPU *cpu);
>> int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>> int vq, bool assign);
>>
>
> Just cosmetic nits... patch looks fine otherwise, so:
>
> Reviewed-by: Thomas Huth <address@hidden>
>