On Mon, Jan 17, 2022 at 04:53:45AM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 30, 2021 at 08:15:36PM +0800, Yang Zhong wrote:
> > On Thu, Nov 25, 2021 at 08:47:22PM +0800, Yang Zhong wrote:
> > > Hello Paolo,
> > >
> > > Our customer used the Libvirt XML to start a SGX VM, but failed.
> > >
> > > libvirt.libvirtError: internal error: unable to execute QEMU command
> > > 'qom-get': Property 'sgx-epc.unavailable-features' not found
> > >
> > > The XML file,
> > > <qemu:commandline>
> > > <qemu:arg value="-cpu"/>
> > > <qemu:arg
> > > value="host,+sgx,+sgx-debug,+sgx-exinfo,+sgx-kss,+sgx-mode64,+sgx-provisionkey,+sgx-tokenkey,+sgx1,+sgx2,+sgxlc"/>
> > > <qemu:arg value="-object"/>
> > > <qemu:arg value="memory-backend-epc,id=mem1,size=16M,prealloc=on"/>
> > > <qemu:arg value="-M"/>
> > > <qemu:arg value="sgx-epc.0.memdev=mem1"/>
> > > </qemu:commandline>
> > >
> > > The new compound property command should be located in /machine path,
> > > which are different with old command '-sgx-epc id=epc1,memdev=mem1'.
> > >
> > > I also tried this from Qemu monitor tool,
> > > (qemu) qom-list /machine
> > > type (string)
> > > kernel (string)
> > > ......
> > > sgx-epc (SgxEPC)
> > > ......
> > > sgx-epc[0] (child<memory-region>)
> > > ......
> > >
> > > We can find sgx-epc from /machine list.
> > >
> >
> > This issue is clear now, which is caused by Libvirt to get the CPU's
> > unavailable-features by below command:
> >
> > {"execute":"qom-get","arguments":{"path":"/machine/unattached/device[0]","property":"unavailable-features"}
> >
> > but in SGX vm, since the sgx is initialized before VCPU because sgx need
> > set the virtual EPC info in the cpuid.
> >
> > So the /machine/unattached/device[0] is occupied by sgx, which fail to
> > get the unvailable-features from
> > /machine/unattached/device[0].
> >
> >
> > We need fix this issue, but this can be done in Qemu or Libvirt side.
> >
> > 1) Libvirt side
> > If the libvirt support SGX EPCs, libvirt can use
> > /machine/unattached/device[n] to check "unavailable-features".
> > n is the next number of sgx's unattached_count.
> >
> > 2) Qemu side
> >
> > One temp patch to create one /sgx in the /machine in the
> > device_set_realized()
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index 84f3019440..4154eef0d8 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -497,7 +497,7 @@ static void device_set_realized(Object *obj, bool
> > value, Error **errp)
> > NamedClockList *ncl;
> > Error *local_err = NULL;
> > bool unattached_parent = false;
> > - static int unattached_count;
> > + static int unattached_count, sgx_count;
> >
> > if (dev->hotplugged && !dc->hotpluggable) {
> > error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
> > @@ -509,7 +509,15 @@ static void device_set_realized(Object *obj, bool
> > value, Error **errp)
> > goto fail;
> > }
> >
> > - if (!obj->parent) {
> > + if (!obj->parent && !strcmp(object_get_typename(obj), "sgx-epc")) {
> > + gchar *name = g_strdup_printf("device[%d]", sgx_count++);
> > +
> > + object_property_add_child(container_get(qdev_get_machine(),
> > + "/sgx"),
> > + name, obj);
> > + unattached_parent = true;
> > + g_free(name);
> > + } else if (!obj->parent) {
> > gchar *name = g_strdup_printf("device[%d]",
> > unattached_count++);
> >
> > object_property_add_child(container_get(qdev_get_machine()
> >
> > This patch can make sure vcpu is still /machine/unattached/device[0].
> >
> >
> > Which solution is best? thanks!
>
> Has either of the fixes reached yet reached upstream or not?
Jarkko, I sent out one patch to fix this issue last week,
https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg02502.html
Daniel regarded this fix is special code for SGX in the generic object code.
So, this fix can be done in Libvirt side. Did you face this issue? or you can
use this patch as TEMP fix. thanks!
Yang
>
> > Yang
>
> BR, Jarkko
