On Mon, 2022-05-16 at 16:40 +0800, Zhiquan Li wrote: > On 2022/5/16 10:29, Kai Huang wrote: > > On Sat, 2022-05-14 at 13:39 +0800, Zhiquan Li wrote: > > > On 2022/5/14 00:35, Luck, Tony wrote: > > > > > > Do you think the processes sharing the same enclave need to be killed, > > > > > > even they had not touched the EPC page with hardware error? > > > > > > Any ideas are welcome. > > > > > I do not think the patch set is going to wrong direction. This discussion > > > > > was just missing from the cover letter. > > > OK, I will add this point into v2 of cover letter and patch 03. > > > > > > > > I don't think you should add to patch 03. The same enclave can be shared by > > multiple processes is only true for host enclaves, but not virtual EPC isntance. > > Virtual EPC instance is just a raw EPC resource which is accessible to guest, > > but how enclaves are created on this EPC resource is completely upto guest. > > Therefore, one virtual EPC cannot be shared by two guests. > > > > Thanks for your clarification, Kai. > Patch 04 is for the host case, we can put it there. > > May I add your comments in patch 03 and add you as "Acked-by" in v2? > I suppose it is a good answer if someone has the same question. > Yes you can add the comments to the patch. One problem is currently we don't explicitly prevent vepc being shared via fork(). For instance, an application can open /dev/sgx_vepc, mmap(), and fork(). Then if the child does mmap() against the fd opened by the parent, the child will share the same vepc with the parent. We can either explicitly enforce that only one process can mmap() to one vepc in mmap() of vepc, or we can put into comment saying something like below: Unlike host encalves, virtual EPC instance cannot be shared by multiple VMs. It is because how enclaves are created is totally upto the guest. Sharing virtual EPC instance will very likely to unexpectedly break enclaves in all VMs. SGX virtual EPC driver doesn't explicitly prevent virtual EPC instance being shared by multiple VMs via fork(). However KVM doesn't support running a VM across multiple mm structures, and the de facto userspace hypervisor (Qemu) doesn't use fork() to create a new VM, so in practice this should not happen. I'd like to hear from others whether simply adding some comment is fine. -- Thanks, -Kai
