Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník wrote: > On 7/21/22 10:06, Daniel P. Berrangé wrote: >> On Wed, Jul 20, 2022 at 11:12:56PM +0000, Yang, Lin A wrote: >>>> This version is a bit better than the previous one. But we're at version >>>> 13 and I am still unable to even start a guest. Please, make sure that this >>>> basic functionality works in v14, because this is plain waste of precious >>>> review bandwidth. >>>> >>>> Anyway, as usual, I've uploaded my suggested fixes here: >>>> >>>> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/sgx/ >>> >>> Sorry to hear it didn't work in your environment. We definitely tested it >>> several times and it works well for both QEMU 6.2.0 and 7.0.0. > > Alright, I finally made it work. The problem was with memfd backend. > I'll post patch for that soon. > >>> >>> Let me try to reproduce it with the domain xml you shared before. >>> >>> By my best guess, if you see "qemu-system-x86_64:***: >>> invalid object type: memory-backend-epc" error, it means QEMU didn't >>> get enough permission to launch SGX VM. >>> >>> Pls add /dev/sgx_vepc, /dev/sgx_enclave and /dev/sgx_provision to the >>> "cgroup_device_acl" list in /etc/libvirt/qemu.conf. QEMU requires those access >>> to assign EPC, but it was denied by libvirt’s cgroup controllers by default. >>> >>> cgroup_device_acl = [ >>> "/dev/null", "/dev/full", "/dev/zero", >>> ... >>> "/dev/sgx_vepc", >>> "/dev/sgx_enclave”, >>> "/dev/sgx_provision”

>>> ] >>> >>> Also in /etc/libvirt/qemu.conf, set the runtime user to uid 0, since QEMU needs to >>> read and write to those sgx devices, like /dev/sgx_vepc. Unfortunately, it is owned >>> by root with file mode 600, so QEMU has to launch as root. >>> >>> user = "+0" >>> >>> It would be really helpful if you can use these steps to see whether it resolve >>> the issue. I will add a doc somewhere to include all steps are required for use to >>> use sgx in libvirt. >> >> The need to customize qemu.conf to change cgroups ACLs and set uid==0 makes >> this patch series unusal in the real world deployments. It cannot be merged >> with such problems existing. >> > > Agreed. While libvirt can allow /dev/sgx* in CGroups (we do that for > other devices, including NVDIMM and virtio-pmem types of <memory/>), > it's more tricky with relabelling. > > By default, when available, libvirt creates a separate mount namespace > for each QEMU process and creates a very small /dev there, with only > those nodes that QEMU needs. Now, if libvirt is fixed (I have follow up > patches on top of this series) the /dev/sgx* nodes are created there AND > I have another patch that sets DAC/SELinux label on them so that uid=0 > is no longer needed. What I worry about though, is the case when this > namespace feature is disabled. Then libvirt should not touch /dev/sgx* > because that might compromise security in the system. That might in turn require the ability to pass in pre-opened FDs for the devices to QEMU.