kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vipin Sharma <vipinsh@google.com>
To: Tejun Heo <tj@kernel.org>
Cc: thomas.lendacky@amd.com, brijesh.singh@amd.com,
	jon.grimm@amd.com, eric.vantassell@amd.com, pbonzini@redhat.com,
	seanjc@google.com, lizefan@huawei.com, hannes@cmpxchg.org,
	frankja@linux.ibm.com, borntraeger@de.ibm.com, corbet@lwn.net,
	joro@8bytes.org, vkuznets@redhat.com, wanpengli@tencent.com,
	jmattson@google.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, hpa@zytor.com, gingell@google.com,
	rientjes@google.com, dionnaglaze@google.com, kvm@vger.kernel.org,
	x86@kernel.org, cgroups@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Patch v4 1/2] cgroup: svm: Add Encryption ID controller
Date: Fri, 15 Jan 2021 20:32:19 -0800	[thread overview]
Message-ID: <YAJsUyH2zspZxF2S@google.com> (raw)
In-Reply-To: <YAJg5MB/Qn5dRqmu@mtj.duckdns.org>

On Fri, Jan 15, 2021 at 10:43:32PM -0500, Tejun Heo wrote:
> On Fri, Jan 15, 2021 at 02:18:40PM -0800, Vipin Sharma wrote:
> > > * Why is .sev a separate namespace? Isn't the controller supposed to cover
> > >   encryption ids across different implementations? It's not like multiple
> > >   types of IDs can be in use on the same machine, right?
> > > 
> > 
> > On AMD platform we have two types SEV and SEV-ES which can exists
> > simultaneously and they have their own quota.
> 
> Can you please give a brief explanation of the two and lay out a scenario
> where the two are being used / allocated disjointly?
> 

SEV-ES has stronger memory encryption gurantees compared to SEV, apart
from encrypting the application memory it also encrypts register state
among other things. In a single host ASIDs can be distributed between
these two types by BIOS settings.

Currently, Google Cloud has Confidential VM machines offering using SEV.
ASIDs are not compatible between SEV and SEV-ES, so a VM running on SEV
cannot run on SEV-ES and vice versa

There are use cases for both types of VMs getting used in future.

> > > > Other ID types can be easily added in the controller in the same way.
> > > 
> > > I'm not sure this is necessarily a good thing.
> > 
> > This is to just say that when Intel and PowerPC changes are ready it
> > won't be difficult for them to add their controller.
> 
> I'm not really enthused about having per-hardware-type control knobs. None
> of other controllers behave that way. Unless it can be abstracted into
> something common, I'm likely to object.

There was a discussion in Patch v1 and consensus was to have individual
files because it makes kernel implementation extremely simple.

https://lore.kernel.org/lkml/alpine.DEB.2.23.453.2011131615510.333518@chino.kir.corp.google.com/#t

> 
> > > > +static int enc_id_cg_stat_show(struct seq_file *sf, void *v)
> > > > +{
> > > > +	unsigned long flags;
> > > > +	enum encryption_id_type type = seq_cft(sf)->private;
> > > > +
> > > > +	spin_lock_irqsave(&enc_id_cg_lock, flags);
> > > > +
> > > > +	seq_printf(sf, "total %u\n", enc_id_capacity[type]);
> > > > +	seq_printf(sf, "used %u\n", root_cg.res[type].usage);
> > > 
> > > Dup with .current and no need to show total on every cgroup, right?
> > 
> > This is for the stat file which will only be seen in the root cgroup
> > directory.  It is to know overall picture for the resource, what is the
> > total capacity and what is the current usage. ".current" file is not
> > shown on the root cgroup.
> 
> Ah, missed the flags. It's odd for the usage to be presented in two
> different ways tho. I think it'd make more sense w/ cgroup.current at root
> level. Is the total number available somewhere else in the system?

This information is not available anywhere else in the system. Only
other way to get this value is to use CPUID instruction (0x8000001F) of
the processor. Which also has disdvantage if sev module in kernel
doesn't use all of the available ASIDs for its work (right now it uses
all) then there will be a mismatch between what user get through their
code and what is actually getting used in the kernel by sev.

In cgroup v2, I didn't see current files for other cgroups in root
folder that is why I didn't show that file in root folder.

Will you be fine if I show two files in the root, something like:

encids.sev.capacity
encids.sev.current

In non root folder, it will be:
encids.sev.max
encids.sev.current

I still prefer encids.sev.stat, as it won't repeat same information in
each cgroup but let me know what you think.

Thanks

  reply	other threads:[~2021-01-16  4:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-08  1:28 [Patch v4 0/2] cgroup: KVM: New Encryption IDs cgroup controller Vipin Sharma
2021-01-08  1:28 ` [Patch v4 1/2] cgroup: svm: Add Encryption ID controller Vipin Sharma
2021-01-13 15:19   ` Brijesh Singh
2021-01-15 20:59   ` Tejun Heo
2021-01-15 22:18     ` Vipin Sharma
2021-01-16  3:43       ` Tejun Heo
2021-01-16  4:32         ` Vipin Sharma [this message]
2021-01-19 15:51           ` Tejun Heo
2021-01-20  7:13             ` Vipin Sharma
2021-01-20 16:40               ` Tejun Heo
2021-01-20 23:18                 ` Vipin Sharma
2021-01-20 23:32                   ` Tejun Heo
2021-01-22  0:09                     ` Vipin Sharma
2021-01-21 14:55                 ` Tom Lendacky
2021-01-21 15:55                   ` Tejun Heo
2021-01-21 23:12                     ` Tom Lendacky
2021-01-22  1:25                       ` Sean Christopherson
2021-01-26 20:49                         ` David Rientjes
2021-01-26 22:01                           ` Tejun Heo
2021-01-26 22:02                             ` Tejun Heo
2021-01-27  1:11                             ` Vipin Sharma
2021-01-27 14:10                               ` Tejun Heo
2021-01-08  1:28 ` [Patch v4 2/2] cgroup: svm: Encryption IDs cgroup documentation Vipin Sharma
2021-01-15 21:00   ` Tejun Heo
2021-01-15 21:41     ` Vipin Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YAJsUyH2zspZxF2S@google.com \
    --to=vipinsh@google.com \
    --cc=borntraeger@de.ibm.com \
    --cc=bp@alien8.de \
    --cc=brijesh.singh@amd.com \
    --cc=cgroups@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=dionnaglaze@google.com \
    --cc=eric.vantassell@amd.com \
    --cc=frankja@linux.ibm.com \
    --cc=gingell@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=jon.grimm@amd.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=rientjes@google.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=tj@kernel.org \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).