| Message ID | 20170418212212.10190.73484.stgit@tlendack-t1.amdoffice.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/18/2017 02:22 PM, Tom Lendacky wrote: > Add sysfs support for SME so that user-space utilities (kdump, etc.) can > determine if SME is active. > > A new directory will be created: > /sys/kernel/mm/sme/ > > And two entries within the new directory: > /sys/kernel/mm/sme/active > /sys/kernel/mm/sme/encryption_mask Why do they care, and what will they be doing with this information?
On 04/21/17 at 02:55pm, Dave Hansen wrote: > On 04/18/2017 02:22 PM, Tom Lendacky wrote: > > Add sysfs support for SME so that user-space utilities (kdump, etc.) can > > determine if SME is active. > > > > A new directory will be created: > > /sys/kernel/mm/sme/ > > > > And two entries within the new directory: > > /sys/kernel/mm/sme/active > > /sys/kernel/mm/sme/encryption_mask > > Why do they care, and what will they be doing with this information? Since kdump will copy old memory but need this to know if the old memory was encrypted or not. With this sysfs file we can know the previous SME status and pass to kdump kernel as like a kernel param. Tom, have you got chance to try if it works or not? Thanks Dave
On 04/27/2017 12:25 AM, Dave Young wrote: > On 04/21/17 at 02:55pm, Dave Hansen wrote: >> On 04/18/2017 02:22 PM, Tom Lendacky wrote: >>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can >>> determine if SME is active. >>> >>> A new directory will be created: >>> /sys/kernel/mm/sme/ >>> >>> And two entries within the new directory: >>> /sys/kernel/mm/sme/active >>> /sys/kernel/mm/sme/encryption_mask >> >> Why do they care, and what will they be doing with this information? > > Since kdump will copy old memory but need this to know if the old memory > was encrypted or not. With this sysfs file we can know the previous SME > status and pass to kdump kernel as like a kernel param. > > Tom, have you got chance to try if it works or not? What will the kdump kernel do with it though? We kexec() into that kernel so the SME keys will all be the same, right? So, will the kdump kernel be just setting the encryption bit in the PTE so it can copy the old plaintext out? Why do we need both 'active' and 'encryption_mask'? How could it be that the hardware-enumerated 'encryption_mask' changes across a kexec()?
On 04/27/17 at 08:52am, Dave Hansen wrote: > On 04/27/2017 12:25 AM, Dave Young wrote: > > On 04/21/17 at 02:55pm, Dave Hansen wrote: > >> On 04/18/2017 02:22 PM, Tom Lendacky wrote: > >>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can > >>> determine if SME is active. > >>> > >>> A new directory will be created: > >>> /sys/kernel/mm/sme/ > >>> > >>> And two entries within the new directory: > >>> /sys/kernel/mm/sme/active > >>> /sys/kernel/mm/sme/encryption_mask > >> > >> Why do they care, and what will they be doing with this information? > > > > Since kdump will copy old memory but need this to know if the old memory > > was encrypted or not. With this sysfs file we can know the previous SME > > status and pass to kdump kernel as like a kernel param. > > > > Tom, have you got chance to try if it works or not? > > What will the kdump kernel do with it though? We kexec() into that > kernel so the SME keys will all be the same, right? So, will the kdump > kernel be just setting the encryption bit in the PTE so it can copy the > old plaintext out? I assume it is for active -> non active case, the new boot need to know the old memory is encrypted. But I think I did not read all the patches I may miss things. > > Why do we need both 'active' and 'encryption_mask'? How could it be > that the hardware-enumerated 'encryption_mask' changes across a kexec()? Leave this question to Tom.. Thanks Dave
On 4/27/2017 2:25 AM, Dave Young wrote: > On 04/21/17 at 02:55pm, Dave Hansen wrote: >> On 04/18/2017 02:22 PM, Tom Lendacky wrote: >>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can >>> determine if SME is active. >>> >>> A new directory will be created: >>> /sys/kernel/mm/sme/ >>> >>> And two entries within the new directory: >>> /sys/kernel/mm/sme/active >>> /sys/kernel/mm/sme/encryption_mask >> >> Why do they care, and what will they be doing with this information? > > Since kdump will copy old memory but need this to know if the old memory > was encrypted or not. With this sysfs file we can know the previous SME > status and pass to kdump kernel as like a kernel param. > > Tom, have you got chance to try if it works or not? Sorry, I haven't had a chance to test this yet. Thanks, Tom > > Thanks > Dave >
On 4/27/2017 10:52 AM, Dave Hansen wrote: > On 04/27/2017 12:25 AM, Dave Young wrote: >> On 04/21/17 at 02:55pm, Dave Hansen wrote: >>> On 04/18/2017 02:22 PM, Tom Lendacky wrote: >>>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can >>>> determine if SME is active. >>>> >>>> A new directory will be created: >>>> /sys/kernel/mm/sme/ >>>> >>>> And two entries within the new directory: >>>> /sys/kernel/mm/sme/active >>>> /sys/kernel/mm/sme/encryption_mask >>> >>> Why do they care, and what will they be doing with this information? >> >> Since kdump will copy old memory but need this to know if the old memory >> was encrypted or not. With this sysfs file we can know the previous SME >> status and pass to kdump kernel as like a kernel param. >> >> Tom, have you got chance to try if it works or not? > > What will the kdump kernel do with it though? We kexec() into that > kernel so the SME keys will all be the same, right? So, will the kdump Yes, the SME key will be same after a kexec. > kernel be just setting the encryption bit in the PTE so it can copy the > old plaintext out? Yes, the idea would be to set the encryption bit in the PTE when mapping and copying encrypted pages and not set it for unencrypted pages. > > Why do we need both 'active' and 'encryption_mask'? How could it be > that the hardware-enumerated 'encryption_mask' changes across a kexec()? > We don't need both, I just added the 'encryption mask' entry for information. It won't change across a kexec, I can remove it. Thanks, Tom
On Tue, Apr 18, 2017 at 04:22:12PM -0500, Tom Lendacky wrote: > Add sysfs support for SME so that user-space utilities (kdump, etc.) can > determine if SME is active. But why do user-space tools need to know that? I mean, when we load the kdump kernel, we do it with the first kernel, with the kexec_load() syscall, AFAICT. And that code does a lot of things during that init, like machine_kexec_prepare()->init_pgtable() to prepare the ident mapping of the second kernel, for example. What I'm aiming at is that the first kernel knows *exactly* whether SME is enabled or not and doesn't need to tell the second one through some sysfs entries - it can do that during loading. So I don't think we need any userspace things at all... Or?
Ccing Xunlei he is reading the patches see what need to be done for kdump. There should still be several places to handle to make kdump work. On 05/18/17 at 07:01pm, Borislav Petkov wrote: > On Tue, Apr 18, 2017 at 04:22:12PM -0500, Tom Lendacky wrote: > > Add sysfs support for SME so that user-space utilities (kdump, etc.) can > > determine if SME is active. > > But why do user-space tools need to know that? > > I mean, when we load the kdump kernel, we do it with the first kernel, > with the kexec_load() syscall, AFAICT. And that code does a lot of > things during that init, like machine_kexec_prepare()->init_pgtable() to > prepare the ident mapping of the second kernel, for example. > > What I'm aiming at is that the first kernel knows *exactly* whether SME > is enabled or not and doesn't need to tell the second one through some > sysfs entries - it can do that during loading. > > So I don't think we need any userspace things at all... If kdump kernel can get the SME status from hardware register then this should be not necessary and this patch can be dropped. Thanks Dave
On 05/26/2017 at 10:49 AM, Dave Young wrote: > Ccing Xunlei he is reading the patches see what need to be done for > kdump. There should still be several places to handle to make kdump work. > > On 05/18/17 at 07:01pm, Borislav Petkov wrote: >> On Tue, Apr 18, 2017 at 04:22:12PM -0500, Tom Lendacky wrote: >>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can >>> determine if SME is active. >> But why do user-space tools need to know that? >> >> I mean, when we load the kdump kernel, we do it with the first kernel, >> with the kexec_load() syscall, AFAICT. And that code does a lot of >> things during that init, like machine_kexec_prepare()->init_pgtable() to >> prepare the ident mapping of the second kernel, for example. >> >> What I'm aiming at is that the first kernel knows *exactly* whether SME >> is enabled or not and doesn't need to tell the second one through some >> sysfs entries - it can do that during loading. >> >> So I don't think we need any userspace things at all... > If kdump kernel can get the SME status from hardware register then this > should be not necessary and this patch can be dropped. Yes, I also agree with dropping this one. Regards, Xunlei
On 5/26/2017 12:04 AM, Xunlei Pang wrote: > On 05/26/2017 at 10:49 AM, Dave Young wrote: >> Ccing Xunlei he is reading the patches see what need to be done for >> kdump. There should still be several places to handle to make kdump work. >> >> On 05/18/17 at 07:01pm, Borislav Petkov wrote: >>> On Tue, Apr 18, 2017 at 04:22:12PM -0500, Tom Lendacky wrote: >>>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can >>>> determine if SME is active. >>> But why do user-space tools need to know that? >>> >>> I mean, when we load the kdump kernel, we do it with the first kernel, >>> with the kexec_load() syscall, AFAICT. And that code does a lot of >>> things during that init, like machine_kexec_prepare()->init_pgtable() to >>> prepare the ident mapping of the second kernel, for example. >>> >>> What I'm aiming at is that the first kernel knows *exactly* whether SME >>> is enabled or not and doesn't need to tell the second one through some >>> sysfs entries - it can do that during loading. >>> >>> So I don't think we need any userspace things at all... >> If kdump kernel can get the SME status from hardware register then this >> should be not necessary and this patch can be dropped. > > Yes, I also agree with dropping this one. Consensus is to drop, so it will be. Thanks, Tom > > Regards, > Xunlei >
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index 0ff41a4..7dc4e98 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -18,6 +18,8 @@ #include <linux/mm.h> #include <linux/dma-mapping.h> #include <linux/swiotlb.h> +#include <linux/kobject.h> +#include <linux/sysfs.h> #include <asm/tlbflush.h> #include <asm/fixmap.h> @@ -25,6 +27,7 @@ #include <asm/bootparam.h> #include <asm/cacheflush.h> #include <asm/sections.h> +#include <asm/mem_encrypt.h> /* * Since SME related variables are set early in the boot process they must @@ -38,6 +41,52 @@ static char sme_early_buffer[PAGE_SIZE] __aligned(PAGE_SIZE); /* + * Sysfs support for SME. + * Create an sme directory under /sys/kernel/mm + * Create two sme entries under /sys/kernel/mm/sme: + * active - returns 0 if not active, 1 if active + * encryption_mask - returns the encryption mask in use + */ +static ssize_t active_show(struct kobject *kobj, struct kobj_attribute *attr, + char *buf) +{ + return sprintf(buf, "%u\n", sme_active()); +} +static struct kobj_attribute active_attr = __ATTR_RO(active); + +static ssize_t encryption_mask_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sprintf(buf, "0x%016lx\n", sme_me_mask); +} +static struct kobj_attribute encryption_mask_attr = __ATTR_RO(encryption_mask); + +static struct attribute *sme_attrs[] = { + &active_attr.attr, + &encryption_mask_attr.attr, + NULL +}; + +static struct attribute_group sme_attr_group = { + .attrs = sme_attrs, + .name = "sme", +}; + +static int __init sme_sysfs_init(void) +{ + int ret; + + ret = sysfs_create_group(mm_kobj, &sme_attr_group); + if (ret) { + pr_err("SME sysfs initialization failed\n"); + return ret; + } + + return 0; +} +subsys_initcall(sme_sysfs_init); + +/* * This routine does not change the underlying encryption setting of the * page(s) that map this memory. It assumes that eventually the memory is * meant to be accessed as either encrypted or decrypted but the contents
Add sysfs support for SME so that user-space utilities (kdump, etc.) can determine if SME is active. A new directory will be created: /sys/kernel/mm/sme/ And two entries within the new directory: /sys/kernel/mm/sme/active /sys/kernel/mm/sme/encryption_mask Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- arch/x86/mm/mem_encrypt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)