On Mon, Jan 11, 2021 at 10:37:05AM -0800, Sean Christopherson wrote: > On Mon, Jan 11, 2021, Jarkko Sakkinen wrote: > > On Wed, 2021-01-06 at 14:55 +1300, Kai Huang wrote: > > > - Does not require changes to KVM's uAPI, e.g. EPC gets handled as > > > just another memory backend for guests. > > > > Why this an advantage? No objection, just a question. > > There are zero KVM changes required to support exposing EPC to a guest. KVM's > MMU is completely ignorant of what physical backing is used for any given host > virtual address. KVM has to be aware of various VM_* flags, e.g. VM_PFNMAP and > VM_IO, but that code is arch agnostic and is quite isolated. Right, thanks for explanation. > > > - EPC management is wholly contained in the SGX subsystem, e.g. SGX > > > does not have to export any symbols, changes to reclaim flows don't > > > need to be routed through KVM, SGX's dirty laundry doesn't have to > > > get aired out for the world to see, and so on and so forth. > > > > No comments to this before understanding code changes better. > > > > > The virtual EPC allocated to guests is currently not reclaimable, due to > > > reclaiming EPC from KVM guests is not currently supported. Due to the > > > complications of handling reclaim conflicts between guest and host, KVM > > > EPC oversubscription, which allows total virtual EPC size greater than > > > physical EPC by being able to reclaiming guests' EPC, is significantly more > > > complex than basic support for SGX virtualization. > > > > I think it should be really in the center of the patch set description that > > this patch set implements segmentation of EPC, not oversubscription. It should > > be clear immediately. It's a core part of knowing "what I'm looking at". > > Technically, it doesn't implement EPC segmentation of EPC. It implements > non-reclaimable EPC allocation. Even that is somewhat untrue as the EPC can be > forcefully reclaimed, but doing so will destroy the guest contents. In SGX case, that isn't actually as a bad as a policy in high stress situations as with "normal" applications. Runtimes must expect dissappearance of the enclave at any point of time anyway... > Userspace can oversubscribe the EPC to KVM guests, but it would need to kill, > migrate, or pause one or more VMs if the pool of physical EPC were exhausted. Right. > > > > - Support SGX virtualization without SGX Launch Control unlocked mode > > > > > > Although SGX driver requires SGX Launch Control unlocked mode to work, SGX > > > virtualization doesn't, since how enclave is created is completely controlled > > > by guest SGX software, which is not necessarily linux. Therefore, this series > > > allows KVM to expose SGX to guest even SGX Launch Control is in locked mode, > > > or is not present at all. The reason is the goal of SGX virtualization, or > > > virtualization in general, is to expose hardware feature to guest, but not to > > > make assumption how guest will use it. Therefore, KVM should support SGX guest > > > as long as hardware is able to, to have chance to support more potential use > > > cases in cloud environment. > > > > AFAIK the convergence point with the FLC was, and is that Linux never enables > > SGX with locked MSRs. > > > > And I don't understand, if it is not fine to allow locked SGX for a *process*, > > why is it fine for a *virtual machine*? They have a lot same. > > Because it's a completely different OS/kernel. If the user has a kernel that > supports locked SGX, then so be it. There's no novel circumvention of the > kernel policy, e.g. the user could simply boot the non-upstream kernel directly, > and running an upstream kernel in the guest will not cause the kernel to support > SGX. > > There are any number of things that are allowed in a KVM guest that are not > allowed in a bare metal process. I buy this. > > I cannot remember out of top of my head, could the Intel SHA256 be read when > > booted with unlocked MSRs. If that is the case, then you can still support > > guests with that configuration. > > No, it's not guaranteed to be readable as firmware could have already changed > the values in the MSRs. Right. > > Context-dependent guidelines tend to also trash code big time. Also, for the > > sake of a sane kernel code base, I would consider only supporting unlocked > > MSRs. > > It's one line of a code to teach the kernel driver not to load if the MSRs are > locked. And IMO, that one line of code is a net positive as it makes it clear > in the driver itself that it chooses not support locked MSRs, even if SGX itself > is fully enabled. Yup, I think this clears my concerns, thank you. /Jarkko
