Re: [PATCH v4 4/4] hw/i386/sgx: Attach SGX-EPC objects to machine

[Daniel P . Berrangé]

On Mon, Feb 14, 2022 at 09:21:07AM +0100, Igor Mammedov wrote:
> On Mon, 14 Feb 2022 14:58:57 +0800
> Yang Zhong <yang.zhong@intel.com> wrote:
> 
> > On Mon, Feb 07, 2022 at 09:37:52AM +0100, Igor Mammedov wrote:
> > > On Sat,  5 Feb 2022 13:45:26 +0100
> > > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >   
> > > > Previously SGX-EPC objects were exposed in the QOM tree at a path
> > > > 
> > > >   /machine/unattached/device[nn]
> > > > 
> > > > where the 'nn' varies depending on what devices were already created.
> > > > 
> > > > With this change the SGX-EPC objects are now at
> > > > 
> > > >   /machine/sgx-epc[nn]
> > > > 
> > > > where the 'nn' of the first SGX-EPC object is always zero.  
> > > 
> > > yet again, why it's necessary?  
> > 
> > 
> >   Igor, Sorry for delay feedback because of Chinese New Year holiday.
> > 
> >   This series patches are to fix below issues I reported before,
> >   https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg05670.html
> > 
> >   Since the /machine/unattached/device[0] is used by vcpu and Libvirt
> >   use this interface to get unavailable-features list. But in the SGX
> >   VM, the device[0] will be occupied by virtual sgx epc device, Libvirt
> >   can't get unavailable-features from this device[0].
> > 
> >   Although patch 2 in this series already fixed "unavailable-features" 
> > issue,
> 
> I've seen patches on libvirt fixing "unavailable-features" in another way
> without dependence on  /machine/unattached/device[0].
> see:
>  https://www.mail-archive.com/libvir-list@redhat.com/msg226244.html
> 
> >   this patch can move sgx virtual device from /machine/unattached/device[nn]
> >   to /machine/sgx-epc[nn], which seems more clear. Thanks!
> 
> with those patches device[0] becomes non issue, and this patch also becomes
> unnecessary.
> I don't mind putting sgx-epc under machine, but that shall be justified
> somehow. A drawback I noticed in this case is an extra manual
> plumbing/wiring without apparent need for it.

This is effectively questioning why we have a QOM hierarchy with
named devices at all. IMHO we don't need to justify giving explicitly
named nodes under QOM beyond  "this is normal QOM modelling", and
anything under '/unattached' is subject to being fixed in this way.

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 :|