Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend

[Philippe Mathieu-Daudé]

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.