Re: [Qemu-devel] [PATCH] kvm: make one_reg helpers available for everyone

[Alexander Graf]

On 09.05.14 10:06, Cornelia Huck wrote:
> s390x introduced helper functions for getting/setting one_regs with
> commit 860643bc. However, nothing about these is s390-specific.
>
> Alexey Kardashevskiy had already posted a general version, so let's
> merge the two patches and massage the code a bit.
>
> CC: Alexey Kardashevskiy <address@hidden>
> Signed-off-by: Cornelia Huck <address@hidden>

Thanks a lot for the patch Conny. Unfortunately Paolo seems to have 
missed this and queued Alexey's patch instead.

Does s390x even compile still with Alexey's patch applied? If it does, 
please post a simple follow-up patch removing your own s390 
implementation. If it doesn't compile, NACK on the current KVM pull request.


Alex

> ---
>   include/sysemu/kvm.h | 20 ++++++++++++++++++++
>   kvm-all.c            | 28 ++++++++++++++++++++++++++++
>   target-s390x/kvm.c   | 29 -----------------------------
>   trace-events         |  6 ++----
>   4 files changed, 50 insertions(+), 33 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 5ad4e0e..a6c2823 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -383,4 +383,24 @@ void kvm_init_irq_routing(KVMState *s);
>    *          > 0: irq chip was created
>    */
>   int kvm_arch_irqchip_create(KVMState *s);
> +
> +/**
> + * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
> + * @id: The register ID
> + * @source: The pointer to the value to be set. It must point to a variable
> + *          of the correct type/size for the register being accessed.
> + *
> + * Returns: 0 on success, or a negative errno on failure.
> + */
> +int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
> +
> +/**
> + * kvm_get_one_reg - get a register value from KVM via KVM_GET_ONE_REG ioctl
> + * @id: The register ID
> + * @target: The pointer where the value is to be stored. It must point to a
> + *          variable of the correct type/size for the register being accessed.
> + *
> + * Returns: 0 on success, or a negative errno on failure.
> + */
> +int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
>   #endif
> diff --git a/kvm-all.c b/kvm-all.c
> index 5cb7f26..94520e5 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2114,3 +2114,31 @@ int kvm_create_device(KVMState *s, uint64_t type, bool 
> test)
>   
>       return test ? 0 : create_dev.fd;
>   }
> +
> +int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
> +{
> +    struct kvm_one_reg reg;
> +    int r;
> +
> +    reg.id = id;
> +    reg.addr = (uintptr_t) source;
> +    r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> +    if (r) {
> +        trace_kvm_failed_reg_set(id, strerror(r));
> +    }
> +    return r;
> +}
> +
> +int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
> +{
> +    struct kvm_one_reg reg;
> +    int r;
> +
> +    reg.id = id;
> +    reg.addr = (uintptr_t) target;
> +    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> +    if (r) {
> +        trace_kvm_failed_reg_get(id, strerror(r));
> +    }
> +    return r;
> +}
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b7b0edc..ba2dffe 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -129,35 +129,6 @@ void kvm_arch_reset_vcpu(CPUState *cpu)
>       }
>   }
>   
> -static int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
> -{
> -    struct kvm_one_reg reg;
> -    int r;
> -
> -    reg.id = id;
> -    reg.addr = (uint64_t) source;
> -    r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> -    if (r) {
> -        trace_kvm_failed_reg_set(id, strerror(errno));
> -    }
> -    return r;
> -}
> -
> -static int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
> -{
> -    struct kvm_one_reg reg;
> -    int r;
> -
> -    reg.id = id;
> -    reg.addr = (uint64_t) target;
> -    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> -    if (r) {
> -        trace_kvm_failed_reg_get(id, strerror(errno));
> -    }
> -    return r;
> -}
> -
> -
>   int kvm_arch_put_registers(CPUState *cs, int level)
>   {
>       S390CPU *cpu = S390_CPU(cs);
> diff --git a/trace-events b/trace-events
> index af4449d..2c5b307 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1230,6 +1230,8 @@ kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, 
> reason %d"
>   kvm_device_ioctl(int fd, int type, void *arg) "dev fd %d, type 0x%x, arg %p"
>   kvm_failed_spr_set(int str, const char *msg) "Warning: Unable to set SPR %d to 
> KVM: %s"
>   kvm_failed_spr_get(int str, const char *msg) "Warning: Unable to retrieve SPR %d 
> from KVM: %s"
> +kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve ONEREG %" 
> PRIu64 " from KVM: %s"
> +kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set ONEREG %" 
> PRIu64 " to KVM: %s"
>   
>   # memory.c
>   memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr 
> %#"PRIx64" value %#"PRIx64" size %u"
> @@ -1246,7 +1248,3 @@ xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen 
> PV Device MMIO space (ad
>   # hw/pci/pci_host.c
>   pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) 
> "%s %02u:%u @0x%x -> 0x%x"
>   pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) 
> "%s %02u:%u @0x%x <- 0x%x"
> -
> -# target-s390/kvm.c
> -kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve ONEREG %" 
> PRIu64 " from KVM: %s"
> -kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set ONEREG %" 
> PRIu64 " to KVM: %s"