On Fri, Sep 25, 2020 at 03:22:20PM -0700, Vipin Sharma wrote: > On Thu, Sep 24, 2020 at 02:55:18PM -0500, Tom Lendacky wrote: > > On 9/24/20 2:21 PM, Sean Christopherson wrote: > > > On Tue, Sep 22, 2020 at 02:14:04PM -0700, Vipin Sharma wrote: > > > > On Mon, Sep 21, 2020 at 06:48:38PM -0700, Sean Christopherson wrote: > > > > > On Mon, Sep 21, 2020 at 05:40:22PM -0700, Vipin Sharma wrote: > > > > > > Hello, > > > > > > > > > > > > This patch series adds a new SEV controller for tracking and limiting > > > > > > the usage of SEV ASIDs on the AMD SVM platform. > > > > > > > > > > > > SEV ASIDs are used in creating encrypted VM and lightweight sandboxes > > > > > > but this resource is in very limited quantity on a host. > > > > > > > > > > > > This limited quantity creates issues like SEV ASID starvation and > > > > > > unoptimized scheduling in the cloud infrastructure. > > > > > > > > > > > > SEV controller provides SEV ASID tracking and resource control > > > > > > mechanisms. > > > > > > > > > > This should be genericized to not be SEV specific. TDX has a similar > > > > > scarcity issue in the form of key IDs, which IIUC are analogous to SEV ASIDs > > > > > (gave myself a quick crash course on SEV ASIDs). Functionally, I doubt it > > > > > would change anything, I think it'd just be a bunch of renaming. The hardest > > > > > part would probably be figuring out a name :-). > > > > > > > > > > Another idea would be to go even more generic and implement a KVM cgroup > > > > > that accounts the number of VMs of a particular type, e.g. legacy, SEV, > > > > > SEV-ES?, and TDX. That has potential future problems though as it falls > > > > > apart if hardware every supports 1:MANY VMs:KEYS, or if there is a need to > > > > > account keys outside of KVM, e.g. if MKTME for non-KVM cases ever sees the > > > > > light of day. > > > > > > > > I read about the TDX and its use of the KeyID for encrypting VMs. TDX > > > > has two kinds of KeyIDs private and shared. > > > > > > To clarify, "shared" KeyIDs are simply legacy MKTME KeyIDs. This is relevant > > > because those KeyIDs can be used without TDX or KVM in the picture. > > > > > > > On AMD platform there are two types of ASIDs for encryption. > > > > 1. SEV ASID - Normal runtime guest memory encryption. > > > > 2. SEV-ES ASID - Extends SEV ASID by adding register state encryption with > > > > integrity. > > > > > > > > Both types of ASIDs have their own maximum value which is provisioned in > > > > the firmware > > > > > > Ugh, I missed that detail in the SEV-ES RFC. Does SNP add another ASID type, > > > or does it reuse SEV-ES ASIDs? If it does add another type, is that trend > > > expected to continue, i.e. will SEV end up with SEV, SEV-ES, SEV-ES-SNP, > > > SEV-ES-SNP-X, SEV-ES-SNP-X-Y, etc...? > > > > SEV-SNP and SEV-ES share the same ASID range. > > > > Thanks, > > Tom > > > > > > > > > So, we are talking about 4 different types of resources: > > > > 1. AMD SEV ASID (implemented in this patch as sev.* files in SEV cgroup) > > > > 2. AMD SEV-ES ASID (in future, adding files like sev_es.*) > > > > 3. Intel TDX private KeyID > > > > 4. Intel TDX shared KeyID > > > > > > > > TDX private KeyID is similar to SEV and SEV-ES ASID. I think coming up > > > > with the same name which can be used by both platforms will not be easy, > > > > and extensible with the future enhancements. This will get even more > > > > difficult if Arm also comes up with something similar but with different > > > > nuances. > > > > > > Honest question, what's easier for userspace/orchestration layers? Having an > > > abstract but common name, or conrete but different names? My gut reaction is > > > to provide a common interface, but I can see how that could do more harm than > > > good, e.g. some amount of hardware capabilitiy discovery is possible with > > > concrete names. And I'm guessing there's already a fair amount of vendor > > > specific knowledge bleeding into userspace for these features... > > I agree with you that the abstract name is better than the concrete > name, I also feel that we must provide HW extensions. Here is one > approach: > > Cgroup name: cpu_encryption, encryption_slots, or memcrypt (open to > suggestions) > > Control files: slots.{max, current, events} > > Contents of the slot.max: > default max > default: Corresponds to all kinds of encryption capabilities on > a platform. For AMD, it will be SEV and SEV-ES. For > Intel, it will be TDX and MKTME. This can also be used > by other devices not just CPU. > > max: max or any number to denote limit on the cgroup. > > A user who wants the finer control, then they need to know about the > capabilities a platform provides and use them, e.g. on AMD: > > $ echo "sev-es 1000" > slot.max > $ cat slots.max > default max sev-es 1000 > > This means that in the cgroup maximum SEV-ES ASIDs which can be used is > 1000 and SEV ASIDs is max (default, no limit). Each platform can > provide their own extensions which can be overwritten by a user, > otherwise extensions will have the default limit. > > This is kind of similar to the IO and the rdma controller. > > I think it is keeping abstraction for userspace and also providing finer > control for HW specific features. > > What do you think about the above approach? > Hi Sean, Any feedback/concern for the above abstraction approach? Thanks