Show for each node if every memory descriptor in that node has the
EFI_MEMORY_CPU_CRYPTO attribute.
fwupd project plans to use it as part of a check to see if the users
have properly configured memory hardware encryption
capabilities. fwupd's people have seen cases where it seems like there
is memory encryption because all the hardware is capable of doing it,
but on a closer look there is not, either because of system firmware
or because some component requires updating to enable the feature.
It's planned to make it part of a specification that can be passed to
people purchasing hardware
These checks will run at every boot. The specification is called Host
Security ID: https://fwupd.github.io/libfwupdplugin/hsi.html.
We choosed to do it a per-node basis because although an ABI that
shows that the whole system memory is capable of encryption would be
useful for the fwupd use case, doing it in a per-node basis gives also
the capability to the user to target allocations from applications to
NUMA nodes which have encryption capabilities.
I did some tests for some of the functions introduced (and modified)
in e820.c. Sadly KUnit is not able to test __init functions and data
so I had some warnings during the linking. There is a KUnit patch
already to fix that [1]. I wanted to wait for it to be merged but it
is taking more time than I expected so I'm sending this without tests
for now.
[1] https://lore.kernel.org/lkml/[email protected]/T/
Changes since v6:
Fixes in __e820__handle_range_update
Const correctness in e820.c
Correct alignment in memblock.h
Rework memblock_overlaps_region
Changes since v5:
Refactor e820__range_{update, remove, set_crypto_capable} in order to
avoid code duplication.
Warn the user when a node has both encryptable and non-encryptable
regions.
Check that e820_table has enough size to store both current e820_table
and EFI memmap.
Changes since v4:
Add enum to represent the cryptographic capabilities in e820:
e820_crypto_capabilities.
Revert __e820__range_update, only adding the new argument for
__e820__range_add about crypto capabilities.
Add a function __e820__range_update_crypto similar to
__e820__range_update but to only update this new field.
Changes since v3:
Update date in Doc/ABI file.
More information about the fwupd usecase and the rationale behind
doing it in a per-NUMA-node.
Changes since v2:
e820__range_mark_crypto -> e820__range_mark_crypto_capable.
In e820__range_remove: Create a region with crypto capabilities
instead of creating one without it and then mark it.
Changes since v1:
Modify __e820__range_update to update the crypto capabilities of a
range; now this function will change the crypto capability of a range
if it's called with the same old_type and new_type. Rework
efi_mark_e820_regions_as_crypto_capable based on this.
Update do_add_efi_memmap to mark the regions as it creates them.
Change the type of crypto_capable in e820_entry from bool to u8.
Fix e820__update_table changes.
Remove memblock_add_crypto_capable. Now you have to add the region and
mark it then.
Better place for crypto_capable in pglist_data.
Martin Fernandez (8):
mm/memblock: Tag memblocks with crypto capabilities
mm/mmzone: Tag pg_data_t with crypto capabilities
x86/e820: Add infrastructure to refactor e820__range_{update,remove}
x86/e820: Refactor __e820__range_update
x86/e820: Refactor e820__range_remove
x86/e820: Tag e820_entry with crypto capabilities
x86/efi: Mark e820_entries as crypto capable from EFI memmap
drivers/node: Show in sysfs node's crypto capabilities
Documentation/ABI/testing/sysfs-devices-node | 10 +
arch/x86/include/asm/e820/api.h | 1 +
arch/x86/include/asm/e820/types.h | 12 +-
arch/x86/kernel/e820.c | 481 +++++++++++++++----
arch/x86/platform/efi/efi.c | 37 ++
drivers/base/node.c | 10 +
include/linux/memblock.h | 5 +
include/linux/mmzone.h | 3 +
mm/memblock.c | 62 +++
mm/page_alloc.c | 1 +
10 files changed, 524 insertions(+), 98 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-node
--
2.30.2
Refactor __e820__range_update with the introduction of
e820_type_updater_data, indented to be used as the void pointer in the
e820_entry_updater callbacks, and the implementation of the callbacks
to perform the update of the type in a range of a e820_table.
Signed-off-by: Martin Fernandez <[email protected]>
---
arch/x86/kernel/e820.c | 146 +++++++++++++++++++++++++----------------
1 file changed, 89 insertions(+), 57 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 923585ab8377..763b8b20a1fd 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -607,80 +607,112 @@ __e820__handle_range_update(struct e820_table *table,
return updated_size;
}
-static u64 __init
-__e820__range_update(struct e820_table *table, u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
-{
- u64 end;
- unsigned int i;
- u64 real_updated_size = 0;
-
- BUG_ON(old_type == new_type);
-
- if (size > (ULLONG_MAX - start))
- size = ULLONG_MAX - start;
+/**
+ * struct e820_type_updater_data - Helper type for
+ * __e820__range_update().
+ * @old_type: old_type parameter of __e820__range_update().
+ * @new_type: new_type parameter of __e820__range_update().
+ *
+ * This is intended to be used as the @data argument for the
+ * e820_entry_updater callbacks.
+ */
+struct e820_type_updater_data {
+ enum e820_type old_type;
+ enum e820_type new_type;
+};
- end = start + size;
- printk(KERN_DEBUG "e820: update [mem %#010Lx-%#010Lx] ", start, end - 1);
- e820_print_type(old_type);
- pr_cont(" ==> ");
- e820_print_type(new_type);
- pr_cont("\n");
+static bool __init type_updater__should_update(const struct e820_entry *entry,
+ const void *data)
+{
+ const struct e820_type_updater_data *type_updater_data =
+ (const struct e820_type_updater_data *)data;
- for (i = 0; i < table->nr_entries; i++) {
- struct e820_entry *entry = &table->entries[i];
- u64 final_start, final_end;
- u64 entry_end;
+ return entry->type == type_updater_data->old_type;
+}
- if (entry->type != old_type)
- continue;
+static void __init type_updater__update(struct e820_entry *entry,
+ const void *data)
+{
+ const struct e820_type_updater_data *type_updater_data =
+ (const struct e820_type_updater_data *)data;
- entry_end = entry->addr + entry->size;
+ entry->type = type_updater_data->new_type;
+}
- /* Completely covered by new range? */
- if (entry->addr >= start && entry_end <= end) {
- entry->type = new_type;
- real_updated_size += entry->size;
- continue;
- }
+static void __init type_updater__new(struct e820_table *table, u64 new_start,
+ u64 new_size,
+ const struct e820_entry *original,
+ const void *data)
+{
+ const struct e820_type_updater_data *type_updater_data =
+ (const struct e820_type_updater_data *)data;
- /* New range is completely covered? */
- if (entry->addr < start && entry_end > end) {
- __e820__range_add(table, start, size, new_type);
- __e820__range_add(table, end, entry_end - end, entry->type);
- entry->size = start - entry->addr;
- real_updated_size += size;
- continue;
- }
+ __e820__range_add(table, new_start, new_size,
+ type_updater_data->new_type, original->crypto_capable);
+}
- /* Partially covered: */
- final_start = max(start, entry->addr);
- final_end = min(end, entry_end);
- if (final_start >= final_end)
- continue;
+static u64 __init __e820__range_update(struct e820_table *table, u64 start,
+ u64 size, enum e820_type old_type,
+ enum e820_type new_type)
+{
+ struct e820_entry_updater updater = {
+ .should_update = type_updater__should_update,
+ .update = type_updater__update,
+ .new = type_updater__new
+ };
- __e820__range_add(table, final_start, final_end - final_start, new_type);
+ struct e820_type_updater_data data = {
+ .old_type = old_type,
+ .new_type = new_type
+ };
- real_updated_size += final_end - final_start;
+ BUG_ON(old_type == new_type);
- /*
- * Left range could be head or tail, so need to update
- * its size first:
- */
- entry->size -= final_end - final_start;
- if (entry->addr < final_start)
- continue;
+ printk(KERN_DEBUG "e820: update [mem %#018Lx-%#018Lx] ", start,
+ start + size - 1);
+ e820_print_type(old_type);
+ pr_cont(" ==> ");
+ e820_print_type(new_type);
+ pr_cont("\n");
- entry->addr = final_end;
- }
- return real_updated_size;
+ return __e820__handle_range_update(table, start, size, &updater, &data);
}
-u64 __init e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
+/**
+ * e820__range_update() - Update the type of a given address range in
+ * e820_table.
+ * @start: Start of the range.
+ * @size: Size of the range.
+ * @old_type: Type that we want to change.
+ * @new_type: New type to replace @old_type.
+ *
+ * Update type of addresses in [@start, @start + @size) from @old_type
+ * to @new_type in e820_table.
+ *
+ * Return: The size updated.
+ */
+u64 __init e820__range_update(u64 start, u64 size, enum e820_type old_type,
+ enum e820_type new_type)
{
return __e820__range_update(e820_table, start, size, old_type, new_type);
}
-static u64 __init e820__range_update_kexec(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type)
+/**
+ * e820__range_update_kexec() - Update the type of a given address
+ * range in e820_table_kexec.
+ * @start: Start of the range.
+ * @size: Size of the range.
+ * @old_type: Type that we want to change.
+ * @new_type: New type to replace @old_type.
+ *
+ * Update type of addresses in [@start, @start + @size) from @old_type
+ * to @new_type in e820_table_kexec.
+ *
+ * Return: The size updated.
+ */
+static u64 __init e820__range_update_kexec(u64 start, u64 size,
+ enum e820_type old_type,
+ enum e820_type new_type)
{
return __e820__range_update(e820_table_kexec, start, size, old_type, new_type);
}
--
2.30.2
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 <[email protected]>
---
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..5fd5dc7fc2eb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-node
@@ -0,0 +1,10 @@
+What: /sys/devices/system/node/nodeX/crypto_capable
+Date: April 2022
+Contact: Martin Fernandez <[email protected]>
+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 ec8bb24a5a22..1df15ea03c27 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 Mon, Apr 25, 2022 at 02:15:26PM -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 <[email protected]>
> ---
> 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..5fd5dc7fc2eb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-node
> @@ -0,0 +1,10 @@
> +What: /sys/devices/system/node/nodeX/crypto_capable
> +Date: April 2022
> +Contact: Martin Fernandez <[email protected]>
> +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.
I understand that currently this feature is only for x86, but if non-EFI
architectures will start using MEMBLOCK_CRYPTO_CAPABLE, the sysfs attribute
for will be relevant form them as well.
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 systems the node will be marked with EFI_MEMORY_CPU_CRYPTO.
> \ No newline at end of file
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index ec8bb24a5a22..1df15ea03c27 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
>
--
Sincerely yours,
Mike.
On 4/26/22, Mike Rapoport <[email protected]> wrote:
> On Mon, Apr 25, 2022 at 02:15:26PM -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 <[email protected]>
>> ---
>> 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..5fd5dc7fc2eb
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-devices-node
>> @@ -0,0 +1,10 @@
>> +What: /sys/devices/system/node/nodeX/crypto_capable
>> +Date: April 2022
>> +Contact: Martin Fernandez <[email protected]>
>> +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.
>
> I understand that currently this feature is only for x86, but if non-EFI
> architectures will start using MEMBLOCK_CRYPTO_CAPABLE, the sysfs attribute
> for will be relevant form them as well.
>
> 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 systems the node will be marked with EFI_MEMORY_CPU_CRYPTO.
>
Good point. I'll change it.
>> \ No newline at end of file
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index ec8bb24a5a22..1df15ea03c27 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
>>
>
> --
> Sincerely yours,
> Mike.
>