Re: [PATCH v3 4/4] qom/object: Use common get/set uint helpers

[Alexey Kardashevskiy]

On 18/12/2019 09:19, Felipe Franciosi wrote:
> Hi Alexey,
> 
> I don't know how, but somehow I didn't receive your reply:
> https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg02127.html
> 
> (I was about to follow up, then I decided to look at the archives to
> make sure your response didn't get lost in my client somehow...)
> 
> Still not sure of what happened, lol, let's move on. :)

dunno, I spent time configuring "spf" on ozlabs.ru and gmail to make my
mails go through all possible checks :)


> 
> I'm top-posting as I couldn't pull your response in for a proper reply.
> 
> You said:
>> The franciozzy/autosetters branch with this on top -
>> https://github.com/aik/qemu/commit/94c33bb7debf
>> - works fine. Thanks,
> 
> Your patch basically reverts a part of my commit and then makes the
> change Marc-Andre recommended (by dropping the (void *) cast).
> 
> Is it ok for me to just drop that part of my patch and send the v4?

Yes. Thanks,


> You can follow-up on the cast change afterwards.
>
> Thanks,
> F.
> 
> 
>> On Dec 10, 2019, at 1:04 PM, Felipe Franciosi <address@hidden> wrote:
>>
>> Hi
>>
>>> On Dec 2, 2019, at 6:31 AM, Alexey Kardashevskiy <address@hidden> wrote:
>>>
>>>
>>>
>>> On 30/11/2019 04:46, Felipe Franciosi wrote:
>>>> Several objects implemented their own uint property getters and setters,
>>>> despite them being straightforward (without any checks/validations on
>>>> the values themselves) and identical across objects. This makes use of
>>>> an enhanced API for object_property_add_uintXX_ptr() which offers
>>>> default setters.
>>>>
>>>> Some of these setters used to update the value even if the type visit
>>>> failed (eg. because the value being set overflowed over the given type).
>>>> The new setter introduces a check for these errors, not updating the
>>>> value if an error occurred. The error is propagated.
>>>>
>>>> Signed-off-by: Felipe Franciosi <address@hidden>
>>>> ---
>>>> hw/acpi/ich9.c       |  95 ++++----------------------------------
>>>> hw/isa/lpc_ich9.c    |  12 +----
>>>> hw/misc/edu.c        |  13 ++----
>>>> hw/pci-host/q35.c    |  14 ++----
>>>> hw/ppc/spapr.c       |  18 ++------
>>>> hw/vfio/pci-quirks.c |  20 +++-----
>>>> memory.c             |  15 +-----
>>>> target/arm/cpu.c     |  22 ++-------
>>>> target/i386/sev.c    | 106 ++++---------------------------------------
>>>> 9 files changed, 40 insertions(+), 275 deletions(-)
>>>>
>>>> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
>>>> index 742fb78226..d9305be891 100644
>>>> --- a/hw/acpi/ich9.c
>>>> +++ b/hw/acpi/ich9.c
>>>> @@ -357,81 +357,6 @@ static void ich9_pm_set_cpu_hotplug_legacy(Object 
>>>> *obj, bool value,
>>>>    s->pm.cpu_hotplug_legacy = value;
>>>> }
>>>>
>>>> -static void ich9_pm_get_disable_s3(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    uint8_t value = pm->disable_s3;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void ich9_pm_set_disable_s3(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    Error *local_err = NULL;
>>>> -    uint8_t value;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, &local_err);
>>>> -    if (local_err) {
>>>> -        goto out;
>>>> -    }
>>>> -    pm->disable_s3 = value;
>>>> -out:
>>>> -    error_propagate(errp, local_err);
>>>> -}
>>>> -
>>>> -static void ich9_pm_get_disable_s4(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    uint8_t value = pm->disable_s4;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void ich9_pm_set_disable_s4(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    Error *local_err = NULL;
>>>> -    uint8_t value;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, &local_err);
>>>> -    if (local_err) {
>>>> -        goto out;
>>>> -    }
>>>> -    pm->disable_s4 = value;
>>>> -out:
>>>> -    error_propagate(errp, local_err);
>>>> -}
>>>> -
>>>> -static void ich9_pm_get_s4_val(Object *obj, Visitor *v, const char *name,
>>>> -                               void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    uint8_t value = pm->s4_val;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void ich9_pm_set_s4_val(Object *obj, Visitor *v, const char *name,
>>>> -                               void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCPMRegs *pm = opaque;
>>>> -    Error *local_err = NULL;
>>>> -    uint8_t value;
>>>> -
>>>> -    visit_type_uint8(v, name, &value, &local_err);
>>>> -    if (local_err) {
>>>> -        goto out;
>>>> -    }
>>>> -    pm->s4_val = value;
>>>> -out:
>>>> -    error_propagate(errp, local_err);
>>>> -}
>>>> -
>>>> static bool ich9_pm_get_enable_tco(Object *obj, Error **errp)
>>>> {
>>>>    ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
>>>> @@ -468,18 +393,14 @@ void ich9_pm_add_properties(Object *obj, 
>>>> ICH9LPCPMRegs *pm, Error **errp)
>>>>                             ich9_pm_get_cpu_hotplug_legacy,
>>>>                             ich9_pm_set_cpu_hotplug_legacy,
>>>>                             NULL);
>>>> -    object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "uint8",
>>>> -                        ich9_pm_get_disable_s3,
>>>> -                        ich9_pm_set_disable_s3,
>>>> -                        NULL, pm, NULL);
>>>> -    object_property_add(obj, ACPI_PM_PROP_S4_DISABLED, "uint8",
>>>> -                        ich9_pm_get_disable_s4,
>>>> -                        ich9_pm_set_disable_s4,
>>>> -                        NULL, pm, NULL);
>>>> -    object_property_add(obj, ACPI_PM_PROP_S4_VAL, "uint8",
>>>> -                        ich9_pm_get_s4_val,
>>>> -                        ich9_pm_set_s4_val,
>>>> -                        NULL, pm, NULL);
>>>> +    object_property_add_uint8_ptr(obj, ACPI_PM_PROP_S3_DISABLED,
>>>> +                                  &pm->disable_s3, 
>>>> OBJ_PROP_FLAG_READWRITE,
>>>> +                                  NULL);
>>>> +    object_property_add_uint8_ptr(obj, ACPI_PM_PROP_S4_DISABLED,
>>>> +                                  &pm->disable_s4, 
>>>> OBJ_PROP_FLAG_READWRITE,
>>>> +                                  NULL);
>>>> +    object_property_add_uint8_ptr(obj, ACPI_PM_PROP_S4_VAL,
>>>> +                                  &pm->s4_val, OBJ_PROP_FLAG_READWRITE, 
>>>> NULL);
>>>>    object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED,
>>>>                             ich9_pm_get_enable_tco,
>>>>                             ich9_pm_set_enable_tco,
>>>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
>>>> index c40bb3c420..b99a613954 100644
>>>> --- a/hw/isa/lpc_ich9.c
>>>> +++ b/hw/isa/lpc_ich9.c
>>>> @@ -627,13 +627,6 @@ static const MemoryRegionOps ich9_rst_cnt_ops = {
>>>>    .endianness = DEVICE_LITTLE_ENDIAN
>>>> };
>>>>
>>>> -static void ich9_lpc_get_sci_int(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                 void *opaque, Error **errp)
>>>> -{
>>>> -    ICH9LPCState *lpc = ICH9_LPC_DEVICE(obj);
>>>> -    visit_type_uint8(v, name, &lpc->sci_gsi, errp);
>>>> -}
>>>> -
>>>> static void ich9_lpc_initfn(Object *obj)
>>>> {
>>>>    ICH9LPCState *lpc = ICH9_LPC_DEVICE(obj);
>>>> @@ -641,9 +634,8 @@ static void ich9_lpc_initfn(Object *obj)
>>>>    static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
>>>>    static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
>>>>
>>>> -    object_property_add(obj, ACPI_PM_PROP_SCI_INT, "uint8",
>>>> -                        ich9_lpc_get_sci_int,
>>>> -                        NULL, NULL, NULL, NULL);
>>>> +    object_property_add_uint8_ptr(obj, ACPI_PM_PROP_SCI_INT,
>>>> +                                  &lpc->sci_gsi, OBJ_PROP_FLAG_READ, 
>>>> NULL);
>>>>    object_property_add_uint8_ptr(OBJECT(lpc), ACPI_PM_PROP_ACPI_ENABLE_CMD,
>>>>                                  &acpi_enable_cmd, OBJ_PROP_FLAG_READ, 
>>>> NULL);
>>>>    object_property_add_uint8_ptr(OBJECT(lpc), 
>>>> ACPI_PM_PROP_ACPI_DISABLE_CMD,
>>>> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
>>>> index d5e2bdbb57..ff10f5b794 100644
>>>> --- a/hw/misc/edu.c
>>>> +++ b/hw/misc/edu.c
>>>> @@ -396,21 +396,14 @@ static void pci_edu_uninit(PCIDevice *pdev)
>>>>    msi_uninit(pdev);
>>>> }
>>>>
>>>> -static void edu_obj_uint64(Object *obj, Visitor *v, const char *name,
>>>> -                           void *opaque, Error **errp)
>>>> -{
>>>> -    uint64_t *val = opaque;
>>>> -
>>>> -    visit_type_uint64(v, name, val, errp);
>>>> -}
>>>> -
>>>> static void edu_instance_init(Object *obj)
>>>> {
>>>>    EduState *edu = EDU(obj);
>>>>
>>>>    edu->dma_mask = (1UL << 28) - 1;
>>>> -    object_property_add(obj, "dma_mask", "uint64", edu_obj_uint64,
>>>> -                    edu_obj_uint64, NULL, &edu->dma_mask, NULL);
>>>> +    object_property_add_uint64_ptr(obj, "dma_mask",
>>>> +                                   &edu->dma_mask, 
>>>> OBJ_PROP_FLAG_READWRITE,
>>>> +                                   NULL);
>>>> }
>>>>
>>>> static void edu_class_init(ObjectClass *class, void *data)
>>>> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
>>>> index 158d270b9f..f384ab95c6 100644
>>>> --- a/hw/pci-host/q35.c
>>>> +++ b/hw/pci-host/q35.c
>>>> @@ -165,14 +165,6 @@ static void q35_host_get_pci_hole64_end(Object *obj, 
>>>> Visitor *v,
>>>>    visit_type_uint64(v, name, &value, errp);
>>>> }
>>>>
>>>> -static void q35_host_get_mmcfg_size(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                    void *opaque, Error **errp)
>>>> -{
>>>> -    PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
>>>> -
>>>> -    visit_type_uint64(v, name, &e->size, errp);
>>>> -}
>>>> -
>>>> /*
>>>> * NOTE: setting defaults for the mch.* fields in this table
>>>> * doesn't work, because mch is a separate QOM object that is
>>>> @@ -213,6 +205,7 @@ static void q35_host_initfn(Object *obj)
>>>> {
>>>>    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
>>>>    PCIHostState *phb = PCI_HOST_BRIDGE(obj);
>>>> +    PCIExpressHost *pehb = PCIE_HOST_BRIDGE(obj);
>>>>
>>>>    memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb,
>>>>                          "pci-conf-idx", 4);
>>>> @@ -242,9 +235,8 @@ static void q35_host_initfn(Object *obj)
>>>>                        q35_host_get_pci_hole64_end,
>>>>                        NULL, NULL, NULL, NULL);
>>>>
>>>> -    object_property_add(obj, PCIE_HOST_MCFG_SIZE, "uint64",
>>>> -                        q35_host_get_mmcfg_size,
>>>> -                        NULL, NULL, NULL, NULL);
>>>> +    object_property_add_uint64_ptr(obj, PCIE_HOST_MCFG_SIZE,
>>>> +                                   &pehb->size, OBJ_PROP_FLAG_READ, NULL);
>>>>
>>>>    object_property_add_link(obj, MCH_HOST_PROP_RAM_MEM, TYPE_MEMORY_REGION,
>>>>                             (Object **) &s->mch.ram_memory,
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index e076f6023c..668f045023 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -3227,18 +3227,6 @@ static void spapr_set_resize_hpt(Object *obj, const 
>>>> char *value, Error **errp)
>>>>    }
>>>> }
>>>>
>>>> -static void spapr_get_vsmt(Object *obj, Visitor *v, const char *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    visit_type_uint32(v, name, (uint32_t *)opaque, errp);
>>>> -}
>>>> -
>>>> -static void spapr_set_vsmt(Object *obj, Visitor *v, const char *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    visit_type_uint32(v, name, (uint32_t *)opaque, errp);
>>>> -}
>>>> -
>>>> static char *spapr_get_ic_mode(Object *obj, Error **errp)
>>>> {
>>>>    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
>>>> @@ -3336,8 +3324,10 @@ static void spapr_instance_init(Object *obj)
>>>>    object_property_set_description(obj, "resize-hpt",
>>>>                                    "Resizing of the Hash Page Table 
>>>> (enabled, disabled, required)",
>>>>                                    NULL);
>>>> -    object_property_add(obj, "vsmt", "uint32", spapr_get_vsmt,
>>>> -                        spapr_set_vsmt, NULL, &spapr->vsmt, &error_abort);
>>>> +    object_property_add_uint32_ptr(obj, "vsmt",
>>>> +                                   &spapr->vsmt, OBJ_PROP_FLAG_READWRITE,
>>>> +                                   &error_abort);
>>>
>>>
>>> Ths looks alright but...
>>
>> Ok.
>>
>>>
>>>
>>>> +
>>>>    object_property_set_description(obj, "vsmt",
>>>>                                    "Virtual SMT: KVM behaves as if this 
>>>> were"
>>>>                                    " the host's SMT mode", &error_abort);
>>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>>>> index 136f3a9ad6..d769c99bde 100644
>>>> --- a/hw/vfio/pci-quirks.c
>>>> +++ b/hw/vfio/pci-quirks.c
>>>> @@ -2187,14 +2187,6 @@ int vfio_add_virt_caps(VFIOPCIDevice *vdev, Error 
>>>> **errp)
>>>>    return 0;
>>>> }
>>>>
>>>> -static void vfio_pci_nvlink2_get_tgt(Object *obj, Visitor *v,
>>>> -                                     const char *name,
>>>> -                                     void *opaque, Error **errp)
>>>> -{
>>>> -    uint64_t tgt = (uintptr_t) opaque;
>>>> -    visit_type_uint64(v, name, &tgt, errp);
>>>> -}
>>>> -
>>>> static void vfio_pci_nvlink2_get_link_speed(Object *obj, Visitor *v,
>>>>                                                 const char *name,
>>>>                                                 void *opaque, Error **errp)
>>>> @@ -2240,9 +2232,9 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice 
>>>> *vdev, Error **errp)
>>>>                               nv2reg->size, p);
>>>>    QLIST_INSERT_HEAD(&vdev->bars[0].quirks, quirk, next);
>>>>
>>>> -    object_property_add(OBJECT(vdev), "nvlink2-tgt", "uint64",
>>>> -                        vfio_pci_nvlink2_get_tgt, NULL, NULL,
>>>> -                        (void *) (uintptr_t) cap->tgt, NULL);
>>>> +    object_property_add_uint64_ptr(OBJECT(vdev), "nvlink2-tgt",
>>>> +                                   (void *)(uintptr_t)cap->tgt,
>>>
>>>
>>> ... this does not seem equalent. The getter will assume cap->tgt is an
>>> userspace address which it is not. &cap->tgt would be QOM-correct but
>>> won't work because of the lifetime of @cap but this is another story.
>>
>> Maybe I just don't understand where this value comes from. It sounds
>> like you know what you are talking about :) so I'll send a v4 and
>> leave this method untouched, unless you have a chance to test this and
>> tell me that it still works.
>>
>>>
>>> btw what is this patchset based on? I tried applying it on top of 3 week
>>> old and today upstream and it failed. Thanks,
>>
>> I'm not sure why there were problems. I just rebased my branch on top
>> of latest master (without issues). I pushed it to Github so you can
>> have a look. Let me know if you want to try it out or if I should just
>> send a v4 straight away dropping the changes immediately above.
>>
>> https://github.com/franciozzy/qemu
>>
>> Thanks!
>> F.
>>
>>>
>>>
>>>
>>>
>>>
>>>> +                                   OBJ_PROP_FLAG_READ, NULL);
>>>>    trace_vfio_pci_nvidia_gpu_setup_quirk(vdev->vbasedev.name, cap->tgt,
>>>>                                          nv2reg->size);
>>>> free_exit:
>>>> @@ -2301,9 +2293,9 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error 
>>>> **errp)
>>>>        QLIST_INSERT_HEAD(&vdev->bars[0].quirks, quirk, next);
>>>>    }
>>>>
>>>> -    object_property_add(OBJECT(vdev), "nvlink2-tgt", "uint64",
>>>> -                        vfio_pci_nvlink2_get_tgt, NULL, NULL,
>>>> -                        (void *) (uintptr_t) captgt->tgt, NULL);
>>>> +    object_property_add_uint64_ptr(OBJECT(vdev), "nvlink2-tgt",
>>>> +                                   (void *)(uintptr_t)captgt->tgt,
>>>> +                                   OBJ_PROP_FLAG_READ, NULL);
>>>>    trace_vfio_pci_nvlink2_setup_quirk_ssatgt(vdev->vbasedev.name, 
>>>> captgt->tgt,
>>>>                                              atsdreg->size);
>>>>
>>>> diff --git a/memory.c b/memory.c
>>>> index 06484c2bff..7dac2aa059 100644
>>>> --- a/memory.c
>>>> +++ b/memory.c
>>>> @@ -1158,15 +1158,6 @@ void memory_region_init(MemoryRegion *mr,
>>>>    memory_region_do_init(mr, owner, name, size);
>>>> }
>>>>
>>>> -static void memory_region_get_addr(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    MemoryRegion *mr = MEMORY_REGION(obj);
>>>> -    uint64_t value = mr->addr;
>>>> -
>>>> -    visit_type_uint64(v, name, &value, errp);
>>>> -}
>>>> -
>>>> static void memory_region_get_container(Object *obj, Visitor *v,
>>>>                                        const char *name, void *opaque,
>>>>                                        Error **errp)
>>>> @@ -1230,10 +1221,8 @@ static void memory_region_initfn(Object *obj)
>>>>                             NULL, NULL, &error_abort);
>>>>    op->resolve = memory_region_resolve_container;
>>>>
>>>> -    object_property_add(OBJECT(mr), "addr", "uint64",
>>>> -                        memory_region_get_addr,
>>>> -                        NULL, /* memory_region_set_addr */
>>>> -                        NULL, NULL, &error_abort);
>>>> +    object_property_add_uint64_ptr(OBJECT(mr), "addr",
>>>> +                                   &mr->addr, OBJ_PROP_FLAG_READ, 
>>>> &error_abort);
>>>>    object_property_add(OBJECT(mr), "priority", "uint32",
>>>>                        memory_region_get_priority,
>>>>                        NULL, /* memory_region_set_priority */
>>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>>> index 7a4ac9339b..bbe25a73c4 100644
>>>> --- a/target/arm/cpu.c
>>>> +++ b/target/arm/cpu.c
>>>> @@ -1039,22 +1039,6 @@ static void arm_set_pmu(Object *obj, bool value, 
>>>> Error **errp)
>>>>    cpu->has_pmu = value;
>>>> }
>>>>
>>>> -static void arm_get_init_svtor(Object *obj, Visitor *v, const char *name,
>>>> -                               void *opaque, Error **errp)
>>>> -{
>>>> -    ARMCPU *cpu = ARM_CPU(obj);
>>>> -
>>>> -    visit_type_uint32(v, name, &cpu->init_svtor, errp);
>>>> -}
>>>> -
>>>> -static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
>>>> -                               void *opaque, Error **errp)
>>>> -{
>>>> -    ARMCPU *cpu = ARM_CPU(obj);
>>>> -
>>>> -    visit_type_uint32(v, name, &cpu->init_svtor, errp);
>>>> -}
>>>> -
>>>> void arm_cpu_post_init(Object *obj)
>>>> {
>>>>    ARMCPU *cpu = ARM_CPU(obj);
>>>> @@ -1165,9 +1149,9 @@ void arm_cpu_post_init(Object *obj)
>>>>         * a simple DEFINE_PROP_UINT32 for this because we want to permit
>>>>         * the property to be set after realize.
>>>>         */
>>>> -        object_property_add(obj, "init-svtor", "uint32",
>>>> -                            arm_get_init_svtor, arm_set_init_svtor,
>>>> -                            NULL, NULL, &error_abort);
>>>> +        object_property_add_uint32_ptr(obj, "init-svtor",
>>>> +                                       &cpu->init_svtor,
>>>> +                                       OBJ_PROP_FLAG_READWRITE, 
>>>> &error_abort);
>>>>    }
>>>>
>>>>    qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
>>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>>> index 024bb24e51..846018a12d 100644
>>>> --- a/target/i386/sev.c
>>>> +++ b/target/i386/sev.c
>>>> @@ -266,94 +266,6 @@ qsev_guest_class_init(ObjectClass *oc, void *data)
>>>>            "guest owners session parameters (encoded with base64)", NULL);
>>>> }
>>>>
>>>> -static void
>>>> -qsev_guest_set_handle(Object *obj, Visitor *v, const char *name,
>>>> -                      void *opaque, Error **errp)
>>>> -{
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -    uint32_t value;
>>>> -
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -    sev->handle = value;
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_set_policy(Object *obj, Visitor *v, const char *name,
>>>> -                      void *opaque, Error **errp)
>>>> -{
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -    uint32_t value;
>>>> -
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -    sev->policy = value;
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_set_cbitpos(Object *obj, Visitor *v, const char *name,
>>>> -                       void *opaque, Error **errp)
>>>> -{
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -    uint32_t value;
>>>> -
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -    sev->cbitpos = value;
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_set_reduced_phys_bits(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -    uint32_t value;
>>>> -
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -    sev->reduced_phys_bits = value;
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_get_policy(Object *obj, Visitor *v, const char *name,
>>>> -                      void *opaque, Error **errp)
>>>> -{
>>>> -    uint32_t value;
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -
>>>> -    value = sev->policy;
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_get_handle(Object *obj, Visitor *v, const char *name,
>>>> -                      void *opaque, Error **errp)
>>>> -{
>>>> -    uint32_t value;
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -
>>>> -    value = sev->handle;
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_get_cbitpos(Object *obj, Visitor *v, const char *name,
>>>> -                       void *opaque, Error **errp)
>>>> -{
>>>> -    uint32_t value;
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -
>>>> -    value = sev->cbitpos;
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -}
>>>> -
>>>> -static void
>>>> -qsev_guest_get_reduced_phys_bits(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> -                                   void *opaque, Error **errp)
>>>> -{
>>>> -    uint32_t value;
>>>> -    QSevGuestInfo *sev = QSEV_GUEST_INFO(obj);
>>>> -
>>>> -    value = sev->reduced_phys_bits;
>>>> -    visit_type_uint32(v, name, &value, errp);
>>>> -}
>>>> -
>>>> static void
>>>> qsev_guest_init(Object *obj)
>>>> {
>>>> @@ -361,15 +273,15 @@ qsev_guest_init(Object *obj)
>>>>
>>>>    sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
>>>>    sev->policy = DEFAULT_GUEST_POLICY;
>>>> -    object_property_add(obj, "policy", "uint32", qsev_guest_get_policy,
>>>> -                        qsev_guest_set_policy, NULL, NULL, NULL);
>>>> -    object_property_add(obj, "handle", "uint32", qsev_guest_get_handle,
>>>> -                        qsev_guest_set_handle, NULL, NULL, NULL);
>>>> -    object_property_add(obj, "cbitpos", "uint32", qsev_guest_get_cbitpos,
>>>> -                        qsev_guest_set_cbitpos, NULL, NULL, NULL);
>>>> -    object_property_add(obj, "reduced-phys-bits", "uint32",
>>>> -                        qsev_guest_get_reduced_phys_bits,
>>>> -                        qsev_guest_set_reduced_phys_bits, NULL, NULL, 
>>>> NULL);
>>>> +    object_property_add_uint32_ptr(obj, "policy", &sev->policy,
>>>> +                                   OBJ_PROP_FLAG_READWRITE, NULL);
>>>> +    object_property_add_uint32_ptr(obj, "handle", &sev->handle,
>>>> +                                   OBJ_PROP_FLAG_READWRITE, NULL);
>>>> +    object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
>>>> +                                   OBJ_PROP_FLAG_READWRITE, NULL);
>>>> +    object_property_add_uint32_ptr(obj, "reduced-phys-bits",
>>>> +                                   &sev->reduced_phys_bits,
>>>> +                                   OBJ_PROP_FLAG_READWRITE, NULL);
>>>> }
>>>>
>>>> /* sev guest info */
>>>>
>>>
>>> -- 
>>> Alexey
>>
> 

-- 
Alexey