| Message ID | 20230203113650.78146-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Use QOM alias properties and few QOM/QDev cleanups | expand |
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/i386/sgx.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c > index db004d17a6..5ddc5d7ea2 100644 > --- a/hw/i386/sgx.c > +++ b/hw/i386/sgx.c > @@ -299,7 +299,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) > &sgx_epc->mr); > > for (list = x86ms->sgx_epc_list; list; list = list->next) { > - obj = object_new("sgx-epc"); > + obj = object_new(TYPE_SGX_EPC); I wonder why this doesn't use qdev_new(). > > /* set the memdev link with memory backend */ > object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev, > @@ -307,8 +307,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) > /* set the numa node property for sgx epc object */ > object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node, > &error_fatal); > - object_property_set_bool(obj, "realized", true, &error_fatal); > - object_unref(obj); > + qdev_realize_and_unref(DEVICE(obj), NULL, &error_fatal); Yes, please! Must have crept in after I converted all realizations. I can see two more: hw/pci/pcie_sriov.c: object_property_set_bool(OBJECT(vf), "realized", false, &local_err); linux-user/syscall.c: object_property_set_bool(OBJECT(cpu), "realized", false, NULL); > } > > if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) { Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 3/2/23 13:32, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/i386/sgx.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c >> index db004d17a6..5ddc5d7ea2 100644 >> --- a/hw/i386/sgx.c >> +++ b/hw/i386/sgx.c >> @@ -299,7 +299,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) >> &sgx_epc->mr); >> >> for (list = x86ms->sgx_epc_list; list; list = list->next) { >> - obj = object_new("sgx-epc"); >> + obj = object_new(TYPE_SGX_EPC); > > I wonder why this doesn't use qdev_new(). OK. >> /* set the memdev link with memory backend */ >> object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev, >> @@ -307,8 +307,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) >> /* set the numa node property for sgx epc object */ >> object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node, >> &error_fatal); >> - object_property_set_bool(obj, "realized", true, &error_fatal); >> - object_unref(obj); >> + qdev_realize_and_unref(DEVICE(obj), NULL, &error_fatal); > > Yes, please! Must have crept in after I converted all realizations. > I can see two more: > > hw/pci/pcie_sriov.c: object_property_set_bool(OBJECT(vf), "realized", false, &local_err); > linux-user/syscall.c: object_property_set_bool(OBJECT(cpu), "realized", false, NULL); No: these would be qdev_UNrealize_and_unref(). Do we want it? Maybe to avoid open-coding it, yes? >> } >> >> if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) { > > Reviewed-by: Markus Armbruster <armbru@redhat.com> Thanks!
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 3/2/23 13:32, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> hw/i386/sgx.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c >>> index db004d17a6..5ddc5d7ea2 100644 >>> --- a/hw/i386/sgx.c >>> +++ b/hw/i386/sgx.c >>> @@ -299,7 +299,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) >>> &sgx_epc->mr); >>> >>> for (list = x86ms->sgx_epc_list; list; list = list->next) { >>> - obj = object_new("sgx-epc"); >>> + obj = object_new(TYPE_SGX_EPC); >> >> I wonder why this doesn't use qdev_new(). > > OK. Observation, not demand! >>> /* set the memdev link with memory backend */ >>> object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev, >>> @@ -307,8 +307,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) >>> /* set the numa node property for sgx epc object */ >>> object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node, >>> &error_fatal); >>> - object_property_set_bool(obj, "realized", true, &error_fatal); >>> - object_unref(obj); >>> + qdev_realize_and_unref(DEVICE(obj), NULL, &error_fatal); >> >> Yes, please! Must have crept in after I converted all realizations. >> I can see two more: >> >> hw/pci/pcie_sriov.c: object_property_set_bool(OBJECT(vf), "realized", false, &local_err); >> linux-user/syscall.c: object_property_set_bool(OBJECT(cpu), "realized", false, NULL); > > No: these would be qdev_UNrealize_and_unref(). You're right. > Do we want it? > Maybe to avoid open-coding it, yes? I think so. See dc3edf8d8a qdev: Convert to qdev_unrealize() manually 981c3dcd94 qdev: Convert to qdev_unrealize() with Coccinelle >>> } >>> >>> if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) { >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Thanks!
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c index db004d17a6..5ddc5d7ea2 100644 --- a/hw/i386/sgx.c +++ b/hw/i386/sgx.c @@ -299,7 +299,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) &sgx_epc->mr); for (list = x86ms->sgx_epc_list; list; list = list->next) { - obj = object_new("sgx-epc"); + obj = object_new(TYPE_SGX_EPC); /* set the memdev link with memory backend */ object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev, @@ -307,8 +307,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms) /* set the numa node property for sgx epc object */ object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node, &error_fatal); - object_property_set_bool(obj, "realized", true, &error_fatal); - object_unref(obj); + qdev_realize_and_unref(DEVICE(obj), NULL, &error_fatal); } if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) {
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/i386/sgx.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)