* [PATCH 0/2] hw/i386: QOM-attach CPUs/SGX-EPC objects to their parents
@ 2022-01-16 23:53 Philippe Mathieu-Daudé via
2022-01-16 23:53 ` [PATCH 1/2] hw/i386/x86: Attach CPUs to machine Philippe Mathieu-Daudé via
2022-01-16 23:53 ` [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend Philippe Mathieu-Daudé via
0 siblings, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-16 23:53 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini, Yang Zhong,
Eduardo Habkost, Richard Henderson, Philippe Mathieu-Daudé
Trying to fix the issue reported here:
https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg05670.html
First attach the CPUs to the machine,
then the SGX-EPC to its memory backend.
Philippe Mathieu-Daudé (2):
hw/i386/x86: Attach CPUs to machine
hw/i386/sgx: Attach SGX-EPC to its memory backend
hw/i386/sgx.c | 3 +++
hw/i386/x86.c | 1 +
2 files changed, 4 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] hw/i386/x86: Attach CPUs to machine
2022-01-16 23:53 [PATCH 0/2] hw/i386: QOM-attach CPUs/SGX-EPC objects to their parents Philippe Mathieu-Daudé via
@ 2022-01-16 23:53 ` Philippe Mathieu-Daudé via
2022-01-17 13:48 ` Daniel P. Berrangé
2022-01-16 23:53 ` [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend Philippe Mathieu-Daudé via
1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-16 23:53 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini, Yang Zhong,
Eduardo Habkost, Richard Henderson, Philippe Mathieu-Daudé
Avoid having CPUs objects dangling as unattached QOM ones,
directly attach them to the machine.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/i386/x86.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b84840a1bb9..50bf249c700 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -108,6 +108,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
{
Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
+ object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
goto out;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-16 23:53 [PATCH 0/2] hw/i386: QOM-attach CPUs/SGX-EPC objects to their parents Philippe Mathieu-Daudé via
2022-01-16 23:53 ` [PATCH 1/2] hw/i386/x86: Attach CPUs to machine Philippe Mathieu-Daudé via
@ 2022-01-16 23:53 ` Philippe Mathieu-Daudé via
2022-01-17 11:48 ` Paolo Bonzini
1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-16 23:53 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini, Yang Zhong,
Eduardo Habkost, Richard Henderson, Philippe Mathieu-Daudé
We have one SGX-EPC address/size/node per memory backend,
make it child of the backend in the QOM composition tree.
Cc: Yang Zhong <yang.zhong@intel.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/i386/sgx.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index 5de5dd08936..6362e5e9d02 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
/* set the memdev link with memory backend */
object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
&error_fatal);
+ object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
+ OBJECT(obj));
+
/* set the numa node property for sgx epc object */
object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node,
&error_fatal);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-16 23:53 ` [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend Philippe Mathieu-Daudé via
@ 2022-01-17 11:48 ` Paolo Bonzini
2022-01-17 12:08 ` Philippe Mathieu-Daudé via
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Paolo Bonzini @ 2022-01-17 11:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Richard Henderson, Michael S. Tsirkin
On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
> We have one SGX-EPC address/size/node per memory backend,
> make it child of the backend in the QOM composition tree.
>
> Cc: Yang Zhong <yang.zhong@intel.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/i386/sgx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> index 5de5dd08936..6362e5e9d02 100644
> --- a/hw/i386/sgx.c
> +++ b/hw/i386/sgx.c
> @@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> /* set the memdev link with memory backend */
> object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
> &error_fatal);
> + object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
> + OBJECT(obj));
> +
> /* set the numa node property for sgx epc object */
> object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node,
> &error_fatal);
I don't think this is a good idea; only list->value->memdev should add
something below itself in the tree.
However, I think obj can be added under the machine itself as
/machine/sgx-epc-device[*].
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-17 11:48 ` Paolo Bonzini
@ 2022-01-17 12:08 ` Philippe Mathieu-Daudé via
2022-01-18 2:33 ` Yang Zhong
2022-01-23 12:52 ` Yang Zhong
2 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-17 12:08 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel, Markus Armbruster
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Yang Zhong,
Eduardo Habkost, Richard Henderson
On 1/17/22 12:48, Paolo Bonzini wrote:
> On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
>> We have one SGX-EPC address/size/node per memory backend,
>> make it child of the backend in the QOM composition tree.
>>
>> Cc: Yang Zhong <yang.zhong@intel.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/i386/sgx.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
>> index 5de5dd08936..6362e5e9d02 100644
>> --- a/hw/i386/sgx.c
>> +++ b/hw/i386/sgx.c
>> @@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
>> /* set the memdev link with memory backend */
>> object_property_parse(obj, SGX_EPC_MEMDEV_PROP,
>> list->value->memdev,
>> &error_fatal);
>> + object_property_add_child(OBJECT(list->value->memdev),
>> "sgx-epc",
>> + OBJECT(obj));
>> +
>> /* set the numa node property for sgx epc object */
>> object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP,
>> list->value->node,
>> &error_fatal);
>
> I don't think this is a good idea; only list->value->memdev should add
> something below itself in the tree.
OK, I see.
> However, I think obj can be added under the machine itself as
> /machine/sgx-epc-device[*].
OK. It is hard to understand the difference between /unattached and
/machine.
Thanks for the review,
Phil.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] hw/i386/x86: Attach CPUs to machine
2022-01-16 23:53 ` [PATCH 1/2] hw/i386/x86: Attach CPUs to machine Philippe Mathieu-Daudé via
@ 2022-01-17 13:48 ` Daniel P. Berrangé
2022-01-17 14:57 ` Philippe Mathieu-Daudé via
2022-01-18 2:15 ` Yang Zhong
0 siblings, 2 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-01-17 13:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, qemu-devel, Paolo Bonzini
On Mon, Jan 17, 2022 at 12:53:30AM +0100, Philippe Mathieu-Daudé via wrote:
> Avoid having CPUs objects dangling as unattached QOM ones,
> directly attach them to the machine.
Lets be more explicit here
[quote]
Previously CPUs were exposed in the QOM tree at a path
/machine/unattached/device[nn]
where the 'nn' of the first CPU is usually zero, but can
vary depending on what devices were already created.
With this change the CPUs are now at
/machine/cpu[nn]
where the 'nn' of the first CPU is always zero
[/quote]
to /machine/unattached/device[0->$SMP-COUNT]
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/i386/x86.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index b84840a1bb9..50bf249c700 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -108,6 +108,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> {
> Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>
> + object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
> if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
> goto out;
> }
> --
> 2.34.1
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] hw/i386/x86: Attach CPUs to machine
2022-01-17 13:48 ` Daniel P. Berrangé
@ 2022-01-17 14:57 ` Philippe Mathieu-Daudé via
2022-01-18 2:15 ` Yang Zhong
1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-17 14:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini,
Yang Zhong, Eduardo Habkost, Richard Henderson
On 1/17/22 14:48, Daniel P. Berrangé wrote:
> On Mon, Jan 17, 2022 at 12:53:30AM +0100, Philippe Mathieu-Daudé via wrote:
>> Avoid having CPUs objects dangling as unattached QOM ones,
>> directly attach them to the machine.
>
> Lets be more explicit here
>
> [quote]
> Previously CPUs were exposed in the QOM tree at a path
>
> /machine/unattached/device[nn]
>
> where the 'nn' of the first CPU is usually zero, but can
> vary depending on what devices were already created.
>
> With this change the CPUs are now at
>
> /machine/cpu[nn]
>
> where the 'nn' of the first CPU is always zero
> [/quote]
OK, thanks for the help here!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] hw/i386/x86: Attach CPUs to machine
2022-01-17 13:48 ` Daniel P. Berrangé
2022-01-17 14:57 ` Philippe Mathieu-Daudé via
@ 2022-01-18 2:15 ` Yang Zhong
1 sibling, 0 replies; 11+ messages in thread
From: Yang Zhong @ 2022-01-18 2:15 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Eduardo Habkost, yang.zhong, Michael S. Tsirkin,
Richard Henderson, Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini
On Mon, Jan 17, 2022 at 01:48:46PM +0000, Daniel P. Berrangé wrote:
> On Mon, Jan 17, 2022 at 12:53:30AM +0100, Philippe Mathieu-Daudé via wrote:
> > Avoid having CPUs objects dangling as unattached QOM ones,
> > directly attach them to the machine.
>
> Lets be more explicit here
>
> [quote]
> Previously CPUs were exposed in the QOM tree at a path
>
> /machine/unattached/device[nn]
>
> where the 'nn' of the first CPU is usually zero, but can
> vary depending on what devices were already created.
>
> With this change the CPUs are now at
>
> /machine/cpu[nn]
>
> where the 'nn' of the first CPU is always zero
> [/quote]
>
> to /machine/unattached/device[0->$SMP-COUNT]
>
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > hw/i386/x86.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index b84840a1bb9..50bf249c700 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -108,6 +108,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> > {
> > Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
> >
> > + object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
> > if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
> > goto out;
> > }
> > --
> > 2.34.1
> >
> >
Thanks Philippe, if we change /machine/unattached/device[nn] to /machine/cpu[nn],
the related changes should also be done in the Libvirt side, which still check
/machine/unattached/device[0] to get unvailable-features. thanks!
Yang
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-17 11:48 ` Paolo Bonzini
2022-01-17 12:08 ` Philippe Mathieu-Daudé via
@ 2022-01-18 2:33 ` Yang Zhong
2022-01-23 12:52 ` Yang Zhong
2 siblings, 0 replies; 11+ messages in thread
From: Yang Zhong @ 2022-01-18 2:33 UTC (permalink / raw)
To: Paolo Bonzini, Philippe Mathieu-Daudé
Cc: Eduardo Habkost, yang.zhong, Michael S. Tsirkin,
Richard Henderson, Philippe Mathieu-Daudé,
qemu-devel
On Mon, Jan 17, 2022 at 12:48:10PM +0100, Paolo Bonzini wrote:
> On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
> >We have one SGX-EPC address/size/node per memory backend,
> >make it child of the backend in the QOM composition tree.
> >
> >Cc: Yang Zhong <yang.zhong@intel.com>
> >Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >---
> > hw/i386/sgx.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> >index 5de5dd08936..6362e5e9d02 100644
> >--- a/hw/i386/sgx.c
> >+++ b/hw/i386/sgx.c
> >@@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> > /* set the memdev link with memory backend */
> > object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
> > &error_fatal);
> >+ object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
> >+ OBJECT(obj));
> >+
> > /* set the numa node property for sgx epc object */
> > object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node,
> > &error_fatal);
>
> I don't think this is a good idea; only list->value->memdev should
> add something below itself in the tree.
>
> However, I think obj can be added under the machine itself as
> /machine/sgx-epc-device[*].
>
Thanks Philippe, calling object_property_add_child() in the hw/i386/sgx.c is more
reasonable than in device_set_realized(), thanks!
Yang
> Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-17 11:48 ` Paolo Bonzini
2022-01-17 12:08 ` Philippe Mathieu-Daudé via
2022-01-18 2:33 ` Yang Zhong
@ 2022-01-23 12:52 ` Yang Zhong
2022-01-31 23:41 ` Philippe Mathieu-Daudé via
2 siblings, 1 reply; 11+ messages in thread
From: Yang Zhong @ 2022-01-23 12:52 UTC (permalink / raw)
To: Paolo Bonzini, Philippe Mathieu-Daudé
Cc: Eduardo Habkost, yang.zhong, Michael S. Tsirkin,
Richard Henderson, Philippe Mathieu-Daudé,
qemu-devel
On Mon, Jan 17, 2022 at 12:48:10PM +0100, Paolo Bonzini wrote:
> On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
> >We have one SGX-EPC address/size/node per memory backend,
> >make it child of the backend in the QOM composition tree.
> >
> >Cc: Yang Zhong <yang.zhong@intel.com>
> >Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >---
> > hw/i386/sgx.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> >index 5de5dd08936..6362e5e9d02 100644
> >--- a/hw/i386/sgx.c
> >+++ b/hw/i386/sgx.c
> >@@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> > /* set the memdev link with memory backend */
> > object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
> > &error_fatal);
> >+ object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
> >+ OBJECT(obj));
> >+
> > /* set the numa node property for sgx epc object */
> > object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node,
> > &error_fatal);
>
> I don't think this is a good idea; only list->value->memdev should
> add something below itself in the tree.
>
> However, I think obj can be added under the machine itself as
> /machine/sgx-epc-device[*].
>
Philippe, Sorry I can't receive all Qemu mails from my mutt tool.
https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg03535.html
I verified this patch, and the issue was reported as below:
Unexpected error in object_property_try_add() at ../qom/object.c:1224:
qemu-system-x86_64: attempt to add duplicate property 'sgx-epc' to object (type 'pc-q35-7.0-machine')
Aborted (core dumped)
Even I changed it to another name, which still reported same kind of issue.
I tried below patch as my previous patch, and it can work
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index d60485fc422..66444745b47 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -281,6 +281,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
SGXEPCState *sgx_epc = &pcms->sgx_epc;
X86MachineState *x86ms = X86_MACHINE(pcms);
SgxEPCList *list = NULL;
+ int sgx_count = 0;
Object *obj;
memset(sgx_epc, 0, sizeof(SGXEPCState));
@@ -297,7 +298,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
for (list = x86ms->sgx_epc_list; list; list = list->next) {
obj = object_new("sgx-epc");
- object_property_add_child(OBJECT(pcms), "sgx-epc", OBJECT(obj));
+ gchar *name = g_strdup_printf("device[%d]", sgx_count++);
+ object_property_add_child(container_get(qdev_get_machine(), "/sgx-epc-device"),
+ name, obj);
/* set the memdev link with memory backend */
object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
From the monitor,
(qemu) qom-list /machine/sgx-epc-device
type (string)
device[0] (child<sgx-epc>)
device[1] (child<sgx-epc>)
This can normally show two sgx epc section devices.
If you have new patch, I can help verify, thanks!
Yang
> Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend
2022-01-23 12:52 ` Yang Zhong
@ 2022-01-31 23:41 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-31 23:41 UTC (permalink / raw)
To: Yang Zhong
Cc: qemu-devel, Michael S. Tsirkin, Paolo Bonzini, Marcel Apfelbaum,
Eduardo Habkost, Richard Henderson
On 23/1/22 13:52, Yang Zhong wrote:
> On Mon, Jan 17, 2022 at 12:48:10PM +0100, Paolo Bonzini wrote:
>> On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
>>> We have one SGX-EPC address/size/node per memory backend,
>>> make it child of the backend in the QOM composition tree.
>>>
>>> Cc: Yang Zhong <yang.zhong@intel.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> hw/i386/sgx.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
>>> index 5de5dd08936..6362e5e9d02 100644
>>> --- a/hw/i386/sgx.c
>>> +++ b/hw/i386/sgx.c
>>> @@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
>>> /* set the memdev link with memory backend */
>>> object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
>>> &error_fatal);
>>> + object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
>>> + OBJECT(obj));
>>> +
>>> /* set the numa node property for sgx epc object */
>>> object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, list->value->node,
>>> &error_fatal);
>>
>> I don't think this is a good idea; only list->value->memdev should
>> add something below itself in the tree.
>>
>> However, I think obj can be added under the machine itself as
>> /machine/sgx-epc-device[*].
>>
>
> Philippe, Sorry I can't receive all Qemu mails from my mutt tool.
>
> https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg03535.html
> I verified this patch, and the issue was reported as below:
>
> Unexpected error in object_property_try_add() at ../qom/object.c:1224:
> qemu-system-x86_64: attempt to add duplicate property 'sgx-epc' to object (type 'pc-q35-7.0-machine')
> Aborted (core dumped)
>
> Even I changed it to another name, which still reported same kind of issue.
>
> I tried below patch as my previous patch, and it can work
> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> index d60485fc422..66444745b47 100644
> --- a/hw/i386/sgx.c
> +++ b/hw/i386/sgx.c
> @@ -281,6 +281,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> SGXEPCState *sgx_epc = &pcms->sgx_epc;
> X86MachineState *x86ms = X86_MACHINE(pcms);
> SgxEPCList *list = NULL;
> + int sgx_count = 0;
> Object *obj;
>
> memset(sgx_epc, 0, sizeof(SGXEPCState));
> @@ -297,7 +298,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> for (list = x86ms->sgx_epc_list; list; list = list->next) {
> obj = object_new("sgx-epc");
>
> - object_property_add_child(OBJECT(pcms), "sgx-epc", OBJECT(obj));
> + gchar *name = g_strdup_printf("device[%d]", sgx_count++);
Oops yes you are right... Fixed in v3!
> + object_property_add_child(container_get(qdev_get_machine(), "/sgx-epc-device"),
> + name, obj);
>
> /* set the memdev link with memory backend */
> object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
>
>
> From the monitor,
> (qemu) qom-list /machine/sgx-epc-device
> type (string)
> device[0] (child<sgx-epc>)
> device[1] (child<sgx-epc>)
>
> This can normally show two sgx epc section devices.
>
> If you have new patch, I can help verify, thanks!
Here you go for v3:
https://lore.kernel.org/qemu-devel/20220131233507.334174-1-f4bug@amsat.org/
Regards,
Phil.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-01-31 23:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-16 23:53 [PATCH 0/2] hw/i386: QOM-attach CPUs/SGX-EPC objects to their parents Philippe Mathieu-Daudé via
2022-01-16 23:53 ` [PATCH 1/2] hw/i386/x86: Attach CPUs to machine Philippe Mathieu-Daudé via
2022-01-17 13:48 ` Daniel P. Berrangé
2022-01-17 14:57 ` Philippe Mathieu-Daudé via
2022-01-18 2:15 ` Yang Zhong
2022-01-16 23:53 ` [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend Philippe Mathieu-Daudé via
2022-01-17 11:48 ` Paolo Bonzini
2022-01-17 12:08 ` Philippe Mathieu-Daudé via
2022-01-18 2:33 ` Yang Zhong
2022-01-23 12:52 ` Yang Zhong
2022-01-31 23:41 ` Philippe Mathieu-Daudé via
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.