Re: [PATCH 14/16] kvm: convert "-machine kvm_shadow_mem" to an accelerator property

[Thomas Huth]

On 13/11/2019 15.39, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  accel/kvm/kvm-all.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  hw/core/machine.c   | 39 ---------------------------------------
>  include/hw/boards.h |  2 --
>  qemu-options.hx     |  6 +++---
>  target/i386/kvm.c   |  2 +-
>  vl.c                |  4 ++++
>  6 files changed, 51 insertions(+), 45 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 140b0bd..c016319 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -41,6 +41,7 @@
>  #include "hw/irq.h"
>  #include "sysemu/sev.h"
>  #include "sysemu/balloon.h"
> +#include "qapi/visitor.h"
>  
>  #include "hw/boards.h"
>  
> @@ -92,6 +93,7 @@ struct KVMState
>      int max_nested_state_len;
>      int many_ioeventfds;
>      int intx_set_mask;
> +    int kvm_shadow_mem;

It's an "int" here...

>      bool sync_mmu;
>      bool manual_dirty_log_protect;
>      /* The man page (and posix) say ioctl numbers are signed int, but
> @@ -2922,6 +2924,40 @@ static bool kvm_accel_has_memory(MachineState *ms, 
> AddressSpace *as,
>      return false;
>  }
>  
> +static void kvm_get_kvm_shadow_mem(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    KVMState *s = KVM_STATE(obj);
> +    int64_t value = s->kvm_shadow_mem;
> +
> +    visit_type_int(v, name, &value, errp);
> +}
> +
> +static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    KVMState *s = KVM_STATE(obj);
> +    Error *error = NULL;
> +    int64_t value;
> +
> +    visit_type_int(v, name, &value, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    s->kvm_shadow_mem = value;
> +}

... but the get and set functions are using an int64_t internally? Looks
somewhat weird. Well, it has been like this in the old code already, but
maybe it's now a good point in time to clean this up?

 Thomas