| Message ID | 20220203164328.203629-7-martin.fernandez@eclypsium.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Show in sysfs if a memory node is able to do encryption | expand |
On 2/3/2022 10:43, Martin Fernandez wrote: > Show in each node in sysfs if its memory is able to do be encrypted by > the CPU, ie. if all its memory is marked with EFI_MEMORY_CPU_CRYPTO in > the EFI memory map. > > Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> > --- > Documentation/ABI/testing/sysfs-devices-node | 10 ++++++++++ > drivers/base/node.c | 10 ++++++++++ > 2 files changed, 20 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-devices-node > > diff --git a/Documentation/ABI/testing/sysfs-devices-node b/Documentation/ABI/testing/sysfs-devices-node > new file mode 100644 > index 000000000000..0d1fd86c9faf > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-devices-node > @@ -0,0 +1,10 @@ > +What: /sys/devices/system/node/nodeX/crypto_capable > +Date: February 2022 > +Contact: Martin Fernandez <martin.fernandez@eclypsium.com> > +Users: fwupd (https://fwupd.org) > +Description: > + This value is 1 if all system memory in this node is > + marked with EFI_MEMORY_CPU_CRYPTO, indicating that the > + system memory is capable of being protected with the > + CPU’s memory cryptographic capabilities. It is 0 > + otherwise. > \ No newline at end of file > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 87acc47e8951..dabaed997ecd 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -560,11 +560,21 @@ static ssize_t node_read_distance(struct device *dev, > } > static DEVICE_ATTR(distance, 0444, node_read_distance, NULL); > > +static ssize_t crypto_capable_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pglist_data *pgdat = NODE_DATA(dev->id); > + > + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); As there is interest in seeing these capabilities from userspace, it seems like a logical time to also expose a `crypto_active` attribute. Then userspace can make a judgement call if the system supports crypto memory (`crypto_capable`) and then also whether or not it's been turned on (`crypto_active`). `crypto_active` could be detected with some existing support in the kernel of `mem_encrypt_active()`. This will then work for a variety of architectures too that offer `mem_encrypt_active()`. As it stands today the only reliable way to tell from userspace (at least for AMD's x86 implementation) is by grepping the system log for the line "AMD Memory Encryption Features active". > +} > +static DEVICE_ATTR_RO(crypto_capable); > + > static struct attribute *node_dev_attrs[] = { > &dev_attr_meminfo.attr, > &dev_attr_numastat.attr, > &dev_attr_distance.attr, > &dev_attr_vmstat.attr, > + &dev_attr_crypto_capable.attr, > NULL > }; >
On Thu, Feb 03, 2022 at 01:43:28PM -0300, Martin Fernandez wrote: > Show in each node in sysfs if its memory is able to do be encrypted by > the CPU, ie. if all its memory is marked with EFI_MEMORY_CPU_CRYPTO in > the EFI memory map. > > Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> > --- > Documentation/ABI/testing/sysfs-devices-node | 10 ++++++++++ > drivers/base/node.c | 10 ++++++++++ > 2 files changed, 20 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-devices-node > > diff --git a/Documentation/ABI/testing/sysfs-devices-node b/Documentation/ABI/testing/sysfs-devices-node > new file mode 100644 > index 000000000000..0d1fd86c9faf > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-devices-node > @@ -0,0 +1,10 @@ > +What: /sys/devices/system/node/nodeX/crypto_capable > +Date: February 2022 > +Contact: Martin Fernandez <martin.fernandez@eclypsium.com> > +Users: fwupd (https://fwupd.org) > +Description: > + This value is 1 if all system memory in this node is > + marked with EFI_MEMORY_CPU_CRYPTO, indicating that the It didn't jump at me at previous postings, but other architectures won't necessary have EFI_MEMORY_CPU_CRYPTO marking crypto-capable memory. How about This value is 1 if all system memory in this node is capable of being protected with the CPU's memory cryptographic capabilities. It is 0 otherwise. On EFI architectures with value corresponds to EFI_MEMORY_CPU_CRYPTO. > + system memory is capable of being protected with the > + CPU’s memory cryptographic capabilities. It is 0 > + otherwise. > \ No newline at end of file > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 87acc47e8951..dabaed997ecd 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -560,11 +560,21 @@ static ssize_t node_read_distance(struct device *dev, > } > static DEVICE_ATTR(distance, 0444, node_read_distance, NULL); > > +static ssize_t crypto_capable_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pglist_data *pgdat = NODE_DATA(dev->id); > + > + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); > +} > +static DEVICE_ATTR_RO(crypto_capable); > + > static struct attribute *node_dev_attrs[] = { > &dev_attr_meminfo.attr, > &dev_attr_numastat.attr, > &dev_attr_distance.attr, > &dev_attr_vmstat.attr, > + &dev_attr_crypto_capable.attr, > NULL > }; > > -- > 2.30.2 >
On 2/4/22, Mike Rapoport <rppt@kernel.org> wrote: > On Thu, Feb 03, 2022 at 01:43:28PM -0300, Martin Fernandez wrote: >> +Description: >> + This value is 1 if all system memory in this node is >> + marked with EFI_MEMORY_CPU_CRYPTO, indicating that the > > It didn't jump at me at previous postings, but other architectures won't > necessary have EFI_MEMORY_CPU_CRYPTO marking crypto-capable memory. > > How about > > This value is 1 if all system memory in this node is capable of being > protected with the CPU's memory cryptographic capabilities. It is 0 > otherwise. > On EFI architectures with value corresponds to EFI_MEMORY_CPU_CRYPTO. > > Yes, sounds good to me. Is there other architecture with something similar to this? Or are you thinking on the possibility of such architecture?
On 2/4/22, Limonciello, Mario <mario.limonciello@amd.com> wrote: > On 2/3/2022 10:43, Martin Fernandez wrote: >> +static ssize_t crypto_capable_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct pglist_data *pgdat = NODE_DATA(dev->id); >> + >> + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); > > As there is interest in seeing these capabilities from userspace, it > seems like a logical time to also expose a `crypto_active` attribute. I planned to do something similar to this, but to show (or actually hide if inactive) tme in cpuinfo, just as Borislav Petkov suggested a few versions back. https://lore.kernel.org/linux-efi/YXrnkxgdjWbcPlJA@zn.tnic/ > Then userspace can make a judgement call if the system supports crypto > memory (`crypto_capable`) and then also whether or not it's been turned > on (`crypto_active`). > > `crypto_active` could be detected with some existing support in the > kernel of `mem_encrypt_active()`. This will then work for a variety of > architectures too that offer `mem_encrypt_active()`. I need a hand with this, I grepped for mem_encrypt_active and nothing showed up... > As it stands today the only reliable way to tell from userspace (at > least for AMD's x86 implementation) is by grepping the system log for > the line "AMD Memory Encryption Features active". Isn't enough to grep for sme/sev in cpuinfo?
On Fri, Feb 04, 2022 at 09:27:42AM -0300, Martin Fernandez wrote: > On 2/4/22, Mike Rapoport <rppt@kernel.org> wrote: > > On Thu, Feb 03, 2022 at 01:43:28PM -0300, Martin Fernandez wrote: > >> +Description: > >> + This value is 1 if all system memory in this node is > >> + marked with EFI_MEMORY_CPU_CRYPTO, indicating that the > > > > It didn't jump at me at previous postings, but other architectures won't > > necessary have EFI_MEMORY_CPU_CRYPTO marking crypto-capable memory. > > > > How about > > > > This value is 1 if all system memory in this node is capable of being > > protected with the CPU's memory cryptographic capabilities. It is 0 > > otherwise. > > On EFI architectures with value corresponds to EFI_MEMORY_CPU_CRYPTO. > > > > > > Yes, sounds good to me. > > Is there other architecture with something similar to this? Or are you > thinking on the possibility of such architecture? AFAIU, s390 and powerpc have memory encryption capabilities, I don't know the details though.
On 2/4/22 07:21, Martin Fernandez wrote: > On 2/4/22, Limonciello, Mario <mario.limonciello@amd.com> wrote: >> On 2/3/2022 10:43, Martin Fernandez wrote: >>> +static ssize_t crypto_capable_show(struct device *dev, >>> + struct device_attribute *attr, char *buf) >>> +{ >>> + struct pglist_data *pgdat = NODE_DATA(dev->id); >>> + >>> + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); >> >> As there is interest in seeing these capabilities from userspace, it >> seems like a logical time to also expose a `crypto_active` attribute. > > I planned to do something similar to this, but to show (or actually > hide if inactive) tme in cpuinfo, just as Borislav Petkov suggested a > few versions back. > > https://lore.kernel.org/linux-efi/YXrnkxgdjWbcPlJA@zn.tnic/ > >> Then userspace can make a judgement call if the system supports crypto >> memory (`crypto_capable`) and then also whether or not it's been turned >> on (`crypto_active`). >> >> `crypto_active` could be detected with some existing support in the >> kernel of `mem_encrypt_active()`. This will then work for a variety of >> architectures too that offer `mem_encrypt_active()`. > > I need a hand with this, I grepped for mem_encrypt_active and nothing > showed up... The mem_encrypt_active() function has been replaced by cc_platform_has(CC_ATTR_MEM_ENCRYPT). > >> As it stands today the only reliable way to tell from userspace (at >> least for AMD's x86 implementation) is by grepping the system log for >> the line "AMD Memory Encryption Features active". > > Isn't enough to grep for sme/sev in cpuinfo? No, it's not enough. Cpuinfo shows a processors capabilities and not necessarily whether that capability is being used. Thanks, Tom
On 2/4/2022 09:59, Tom Lendacky wrote: > On 2/4/22 07:21, Martin Fernandez wrote: >> On 2/4/22, Limonciello, Mario <mario.limonciello@amd.com> wrote: >>> On 2/3/2022 10:43, Martin Fernandez wrote: >>>> +static ssize_t crypto_capable_show(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + struct pglist_data *pgdat = NODE_DATA(dev->id); >>>> + >>>> + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); >>> >>> As there is interest in seeing these capabilities from userspace, it >>> seems like a logical time to also expose a `crypto_active` attribute. >> >> I planned to do something similar to this, but to show (or actually >> hide if inactive) tme in cpuinfo, just as Borislav Petkov suggested a >> few versions back. >> >> https://lore.kernel.org/linux-efi/YXrnkxgdjWbcPlJA@zn.tnic/ As Tom agreed in previous post, Boris is mistaken here. I just double checked on my side on a workstation that supports SME and comparing /proc/cpuinfo before and after SME is enabled via mem_encrypt=on. I confirmed that nothing changed. >> >>> Then userspace can make a judgement call if the system supports crypto >>> memory (`crypto_capable`) and then also whether or not it's been turned >>> on (`crypto_active`). >>> >>> `crypto_active` could be detected with some existing support in the >>> kernel of `mem_encrypt_active()`. This will then work for a variety of >>> architectures too that offer `mem_encrypt_active()`. >> >> I need a hand with this, I grepped for mem_encrypt_active and nothing >> showed up... > > The mem_encrypt_active() function has been replaced by > cc_platform_has(CC_ATTR_MEM_ENCRYPT). Yes, thanks for correcting it . > >> >>> As it stands today the only reliable way to tell from userspace (at >>> least for AMD's x86 implementation) is by grepping the system log for >>> the line "AMD Memory Encryption Features active". >> >> Isn't enough to grep for sme/sev in cpuinfo? > > No, it's not enough. Cpuinfo shows a processors capabilities and not > necessarily whether that capability is being used. > > Thanks, > Tom Tom, Maybe some sysfs file(s) directly from cc_platform.c makes more sense then?
On 2/4/22 10:28, Borislav Petkov wrote: > On Fri, Feb 04, 2022 at 10:23:22AM -0600, Limonciello, Mario wrote: >>>>> As there is interest in seeing these capabilities from userspace, it > > This needs to be explained in a lot more detail: why, what is going to > use it, how, etc. > > We don't do user-visible APIs just because. > >> As Tom agreed in previous post, Boris is mistaken here. I just double >> checked on my side on a workstation that supports SME and comparing >> /proc/cpuinfo before and after SME is enabled via mem_encrypt=on. I >> confirmed that nothing changed. > > Then we should clear that "sme" flag if memory encryption is not > enabled. Like we do for all other flags. If we do that, then this will have to be re-worked: https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/process.c#L761 Thanks, Tom >
On 2/4/2022 11:12, Tom Lendacky wrote: > On 2/4/22 10:28, Borislav Petkov wrote: >> On Fri, Feb 04, 2022 at 10:23:22AM -0600, Limonciello, Mario wrote: >>>>>> As there is interest in seeing these capabilities from userspace, it >> >> This needs to be explained in a lot more detail: why, what is going to >> use it, how, etc. >> >> We don't do user-visible APIs just because. The fwupd daemon has a feature that measures various security aspects of the system hardware, software and firmware and reflects it out to consumers (fwupd clients) in an easily consumable format, in some cases with actionable notes. In this case the information would be used to make a check about memory encryption support and enablement. If a sysfs file was made then it could be something like this: 1) fwupd checks /sys/security/memory_encryption 1: You're encrypted, here's a gold star. 0: keep checking 2) fwupd checks does /proc/cpuinfo have sme, sev_es, or mktme? No: Your hardware doesn't support encryption, tell the user. Yes: keep going. 3)AMD? Check /proc/cmdline, Did user set mem_encrypt=off on explicitly? That's why. Tell user they can enable it with mem_encrypt=on or CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT mem_encrypt=on/CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT? We've got a kernel or hardware problem. 4) Intel? Document Intel's path to turn it on. >> >>> As Tom agreed in previous post, Boris is mistaken here. I just double >>> checked on my side on a workstation that supports SME and comparing >>> /proc/cpuinfo before and after SME is enabled via mem_encrypt=on. I >>> confirmed that nothing changed. >> >> Then we should clear that "sme" flag if memory encryption is not >> enabled. Like we do for all other flags. > > If we do that, then this will have to be re-worked: > > https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/process.c#L761 > I guess if sme/sev/sev_es "are" torn out of cpuinfo when encryption is turned off then that "could" instead do the MSR read perhaps? > > Thanks, > Tom > >>
On 2/4/22 12:00, Borislav Petkov wrote: > On Fri, Feb 04, 2022 at 11:12:04AM -0600, Tom Lendacky wrote: >> https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/process.c#L761 > > For those who won't open a browser just to see what he means :), that's > this snippet: > > void stop_this_cpu(void *dummy): > /* > * Use wbinvd on processors that support SME. This provides support > * for performing a successful kexec when going from SME inactive > * to SME active (or vice-versa). The cache must be cleared so that > * if there are entries with the same physical address, both with and > * without the encryption bit, they don't race each other when flushed > * and potentially end up with the wrong entry being committed to > * memory. > */ > if (boot_cpu_has(X86_FEATURE_SME)) > native_wbinvd(); > > > Well, we do clear our *representation* of CPUID flags for other features > - see output of > > $ git grep -E "(setup_)?clear_cpu_cap" > > for examples. We do that for SME even: early_detect_mem_encrypt(). > > Which means, since this needs to be "processors that support SME", this > line should change to: > > /* ... test the CPUID bit directly because the machine might've cleared > * X86_FEATURE_SME due to cmdline options. > */ > if (cpuid_eax(0x8000001f) & BIT(0)) > native_wbinvd(); > > I'd say... Yep, and that should be safe. We would have to look at the generated code as there can't be any memory stores after the native_wbinvd() and before the native_halt(). Thanks, Tom >
On Fri, Feb 04, 2022 at 12:49:08PM -0600, Tom Lendacky wrote: > Yep, and that should be safe. We would have to look at the generated code as > there can't be any memory stores after the native_wbinvd() and before the > native_halt(). I don't think anything else changes here besides the CPUID. Rest of the asm is the same.
On Fri, Feb 04, 2022 at 05:28:43PM +0100, Borislav Petkov wrote: > Then we should clear that "sme" flag if memory encryption is not > enabled. Like we do for all other flags. Oh, this seems weird to me, as I'd expect it to show up since the CPU is _capable_ of it, even if it's not in use. (Am I really using avx512vl, e.g.?) But as you point out later, it does work that way for a lot of things and boot params. If this is the way things are supposed to be done, it looks like we should wire up "nx" vs "noexec=off" boot param to do the same (separate from this series), though it would need special care since that bit needs very very early handling both and boot and resume. Maybe kernel/cpu/common.c should check for _PAGE_NX in __supported_pte_mask? (And would that break KVM's NX, etc?) Hmmm.
diff --git a/Documentation/ABI/testing/sysfs-devices-node b/Documentation/ABI/testing/sysfs-devices-node new file mode 100644 index 000000000000..0d1fd86c9faf --- /dev/null +++ b/Documentation/ABI/testing/sysfs-devices-node @@ -0,0 +1,10 @@ +What: /sys/devices/system/node/nodeX/crypto_capable +Date: February 2022 +Contact: Martin Fernandez <martin.fernandez@eclypsium.com> +Users: fwupd (https://fwupd.org) +Description: + This value is 1 if all system memory in this node is + marked with EFI_MEMORY_CPU_CRYPTO, indicating that the + system memory is capable of being protected with the + CPU’s memory cryptographic capabilities. It is 0 + otherwise. \ No newline at end of file diff --git a/drivers/base/node.c b/drivers/base/node.c index 87acc47e8951..dabaed997ecd 100644 --- a/drivers/base/node.c +++ b/drivers/base/node.c @@ -560,11 +560,21 @@ static ssize_t node_read_distance(struct device *dev, } static DEVICE_ATTR(distance, 0444, node_read_distance, NULL); +static ssize_t crypto_capable_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pglist_data *pgdat = NODE_DATA(dev->id); + + return sysfs_emit(buf, "%d\n", pgdat->crypto_capable); +} +static DEVICE_ATTR_RO(crypto_capable); + static struct attribute *node_dev_attrs[] = { &dev_attr_meminfo.attr, &dev_attr_numastat.attr, &dev_attr_distance.attr, &dev_attr_vmstat.attr, + &dev_attr_crypto_capable.attr, NULL };
Show in each node in sysfs if its memory is able to do be encrypted by the CPU, ie. if all its memory is marked with EFI_MEMORY_CPU_CRYPTO in the EFI memory map. Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> --- Documentation/ABI/testing/sysfs-devices-node | 10 ++++++++++ drivers/base/node.c | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-devices-node