On 2022/5/17 08:43, Kai Huang wrote: > 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. > Very excellent explanation!! I will put them into patch 03. Thanks a lot, Kai. Best Regards, Zhiquan > I'd like to hear from others whether simply adding some comment is fine. > > -- Thanks, -Kai
