| Message ID | 20220203164328.203629-2-martin.fernandez@eclypsium.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Show in sysfs if a memory node is able to do encryption | expand |
On Thu, Feb 03, 2022 at 01:43:23PM -0300, Martin Fernandez wrote: > Add the capability to mark regions of the memory memory_type able of > hardware memory encryption. > > Also add the capability to query if all regions of a memory node are > able to do hardware memory encryption to call it when initializing the > nodes. Warn the user if a node has both encryptable and > non-encryptable regions. > > Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> > --- > include/linux/memblock.h | 15 ++++++---- > mm/memblock.c | 64 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+), 5 deletions(-) > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h > index 9dc7cb239d21..73edcce165a5 100644 > --- a/include/linux/memblock.h > +++ b/include/linux/memblock.h > @@ -41,13 +41,15 @@ extern unsigned long long max_possible_pfn; > * via a driver, and never indicated in the firmware-provided memory map as > * system RAM. This corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED in the > * kernel resource tree. > + * @MEMBLOCK_CRYPTO_CAPABLE: capable of hardware encryption > */ > enum memblock_flags { > - MEMBLOCK_NONE = 0x0, /* No special request */ > - MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ > - MEMBLOCK_MIRROR = 0x2, /* mirrored region */ > - MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ > - MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ > + MEMBLOCK_NONE = 0x0, /* No special request */ > + MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ > + MEMBLOCK_MIRROR = 0x2, /* mirrored region */ > + MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ > + MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ > + MEMBLOCK_CRYPTO_CAPABLE = 0x10, /* capable of hardware encryption */ Please keep the comment indentation. > }; > > /** > @@ -121,6 +123,9 @@ int memblock_physmem_add(phys_addr_t base, phys_addr_t size); > void memblock_trim_memory(phys_addr_t align); > bool memblock_overlaps_region(struct memblock_type *type, > phys_addr_t base, phys_addr_t size); > +bool memblock_node_is_crypto_capable(int nid); > +int memblock_mark_crypto_capable(phys_addr_t base, phys_addr_t size); > +int memblock_clear_crypto_capable(phys_addr_t base, phys_addr_t size); > int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size); > int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size); > int memblock_mark_mirror(phys_addr_t base, phys_addr_t size); > diff --git a/mm/memblock.c b/mm/memblock.c > index 1018e50566f3..fcf79befeab3 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -191,6 +191,42 @@ bool __init_memblock memblock_overlaps_region(struct memblock_type *type, > return i < type->cnt; > } > > +/** > + * memblock_node_is_crypto_capable - get if whole node is capable > + * of encryption > + * @nid: number of node > + * > + * Iterate over all memory memblock_type and find if all regions under > + * node @nid are capable of hardware encryption. > + * > + * Return: > + * true if every region in memory memblock_type is capable of > + * encryption, false otherwise. > + */ > +bool __init_memblock memblock_node_is_crypto_capable(int nid) > +{ > + struct memblock_region *region; > + bool crypto_capable = false; > + bool not_crypto_capable = false; > + > + for_each_mem_region(region) { > + if (memblock_get_region_node(region) == nid) { > + crypto_capable = > + crypto_capable || > + (region->flags & MEMBLOCK_CRYPTO_CAPABLE); > + not_crypto_capable = > + not_crypto_capable || > + !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); Isn't if (region->flags & MEMBLOCK_CRYPTO_CAPABLE) crypto_capable++; else not_crypto_capable++; simpler and clearer? (of course s/bool/int in the declaration) > + } > + } > + > + if (crypto_capable && not_crypto_capable) > + pr_warn_once("Node %d has regions that are encryptable and regions that aren't", > + nid); This will print only the first node with mixed regions. With a single caller of memblock_node_is_crypto_capable() I think pr_warn() is ok. > + > + return !not_crypto_capable; > +} > + > /** > * __memblock_find_range_bottom_up - find free area utility in bottom-up > * @start: start of candidate range > @@ -885,6 +921,34 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base, > return 0; > } > > +/** > + * memblock_mark_crypto_capable - Mark memory regions capable of hardware > + * encryption with flag MEMBLOCK_CRYPTO_CAPABLE. > + * @base: the base phys addr of the region > + * @size: the size of the region > + * > + * Return: 0 on success, -errno on failure. > + */ > +int __init_memblock memblock_mark_crypto_capable(phys_addr_t base, > + phys_addr_t size) > +{ > + return memblock_setclr_flag(base, size, 1, MEMBLOCK_CRYPTO_CAPABLE); > +} > + > +/** > + * memblock_clear_crypto_capable - Clear flag MEMBLOCK_CRYPTO for a > + * specified region. > + * @base: the base phys addr of the region > + * @size: the size of the region > + * > + * Return: 0 on success, -errno on failure. > + */ > +int __init_memblock memblock_clear_crypto_capable(phys_addr_t base, > + phys_addr_t size) > +{ > + return memblock_setclr_flag(base, size, 0, MEMBLOCK_CRYPTO_CAPABLE); > +} > + > /** > * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_HOTPLUG. > * @base: the base phys addr of the region > -- > 2.30.2 >
On 2/3/22, Mike Rapoport <rppt@kernel.org> wrote: > On Thu, Feb 03, 2022 at 01:43:23PM -0300, Martin Fernandez wrote: >> +/** >> + * memblock_node_is_crypto_capable - get if whole node is capable >> + * of encryption >> + * @nid: number of node >> + * >> + * Iterate over all memory memblock_type and find if all regions under >> + * node @nid are capable of hardware encryption. >> + * >> + * Return: >> + * true if every region in memory memblock_type is capable of >> + * encryption, false otherwise. >> + */ >> +bool __init_memblock memblock_node_is_crypto_capable(int nid) >> +{ >> + struct memblock_region *region; >> + bool crypto_capable = false; >> + bool not_crypto_capable = false; >> + >> + for_each_mem_region(region) { >> + if (memblock_get_region_node(region) == nid) { >> + crypto_capable = >> + crypto_capable || >> + (region->flags & MEMBLOCK_CRYPTO_CAPABLE); >> + not_crypto_capable = >> + not_crypto_capable || >> + !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > > Isn't > > if (region->flags & MEMBLOCK_CRYPTO_CAPABLE) > crypto_capable++; > else > not_crypto_capable++; > > simpler and clearer? > > (of course s/bool/int in the declaration) > Yes! It is. I like that. >> + } >> + } >> + >> + if (crypto_capable && not_crypto_capable) >> + pr_warn_once("Node %d has regions that are encryptable and regions that >> aren't", >> + nid); > > This will print only the first node with mixed regions. With a single > caller of memblock_node_is_crypto_capable() I think pr_warn() is ok. > Yes, you are correct, don't really want _once here.
On Thu, Feb 03, 2022 at 01:43:23PM -0300, Martin Fernandez wrote: > Add the capability to mark regions of the memory memory_type able of > hardware memory encryption. > > Also add the capability to query if all regions of a memory node are > able to do hardware memory encryption to call it when initializing the > nodes. Warn the user if a node has both encryptable and > non-encryptable regions. > > Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> > --- > include/linux/memblock.h | 15 ++++++---- > mm/memblock.c | 64 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+), 5 deletions(-) > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h > index 9dc7cb239d21..73edcce165a5 100644 > --- a/include/linux/memblock.h > +++ b/include/linux/memblock.h > @@ -41,13 +41,15 @@ extern unsigned long long max_possible_pfn; > * via a driver, and never indicated in the firmware-provided memory map as > * system RAM. This corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED in the > * kernel resource tree. > + * @MEMBLOCK_CRYPTO_CAPABLE: capable of hardware encryption > */ > enum memblock_flags { > - MEMBLOCK_NONE = 0x0, /* No special request */ > - MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ > - MEMBLOCK_MIRROR = 0x2, /* mirrored region */ > - MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ > - MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ > + MEMBLOCK_NONE = 0x0, /* No special request */ > + MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ > + MEMBLOCK_MIRROR = 0x2, /* mirrored region */ > + MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ > + MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ > + MEMBLOCK_CRYPTO_CAPABLE = 0x10, /* capable of hardware encryption */ As already suggested, please keep the tabs like they were. If you're going to change every line, maybe expand the single-digit literals to 2 digits. (i.e. 0x0 -> 0x00, to keep the most significant bits lined up.) > }; > > /** > @@ -121,6 +123,9 @@ int memblock_physmem_add(phys_addr_t base, phys_addr_t size); > void memblock_trim_memory(phys_addr_t align); > bool memblock_overlaps_region(struct memblock_type *type, > phys_addr_t base, phys_addr_t size); > +bool memblock_node_is_crypto_capable(int nid); > +int memblock_mark_crypto_capable(phys_addr_t base, phys_addr_t size); > +int memblock_clear_crypto_capable(phys_addr_t base, phys_addr_t size); > int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size); > int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size); > int memblock_mark_mirror(phys_addr_t base, phys_addr_t size); > diff --git a/mm/memblock.c b/mm/memblock.c > index 1018e50566f3..fcf79befeab3 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -191,6 +191,42 @@ bool __init_memblock memblock_overlaps_region(struct memblock_type *type, > return i < type->cnt; > } > > +/** > + * memblock_node_is_crypto_capable - get if whole node is capable > + * of encryption > + * @nid: number of node > + * > + * Iterate over all memory memblock_type and find if all regions under > + * node @nid are capable of hardware encryption. > + * > + * Return: > + * true if every region in memory memblock_type is capable of > + * encryption, false otherwise. > + */ > +bool __init_memblock memblock_node_is_crypto_capable(int nid) > +{ > + struct memblock_region *region; > + bool crypto_capable = false; > + bool not_crypto_capable = false; > + > + for_each_mem_region(region) { > + if (memblock_get_region_node(region) == nid) { > + crypto_capable = > + crypto_capable || > + (region->flags & MEMBLOCK_CRYPTO_CAPABLE); This was already mentioned, but I just thought I'd add: this made me double-take, given the "||" (instead of "|") in an assignment. It looked like a typo, but yes it's correct. I was expecting something like: crypto_capable |= !!(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > + not_crypto_capable = > + not_crypto_capable || > + !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); not_crypto_capable |= !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > + } > + } > + > + if (crypto_capable && not_crypto_capable) > + pr_warn_once("Node %d has regions that are encryptable and regions that aren't", > + nid); > + > + return !not_crypto_capable; > +} > + > /** > * __memblock_find_range_bottom_up - find free area utility in bottom-up > * @start: start of candidate range > @@ -885,6 +921,34 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base, > return 0; > } > > +/** > + * memblock_mark_crypto_capable - Mark memory regions capable of hardware > + * encryption with flag MEMBLOCK_CRYPTO_CAPABLE. > + * @base: the base phys addr of the region > + * @size: the size of the region > + * > + * Return: 0 on success, -errno on failure. > + */ > +int __init_memblock memblock_mark_crypto_capable(phys_addr_t base, > + phys_addr_t size) > +{ > + return memblock_setclr_flag(base, size, 1, MEMBLOCK_CRYPTO_CAPABLE); > +} > + > +/** > + * memblock_clear_crypto_capable - Clear flag MEMBLOCK_CRYPTO for a > + * specified region. > + * @base: the base phys addr of the region > + * @size: the size of the region > + * > + * Return: 0 on success, -errno on failure. > + */ > +int __init_memblock memblock_clear_crypto_capable(phys_addr_t base, > + phys_addr_t size) > +{ > + return memblock_setclr_flag(base, size, 0, MEMBLOCK_CRYPTO_CAPABLE); > +} > + > /** > * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_HOTPLUG. > * @base: the base phys addr of the region > -- > 2.30.2 >
On 2/7/22, Kees Cook <keescook@chromium.org> wrote: > On Thu, Feb 03, 2022 at 01:43:23PM -0300, Martin Fernandez wrote: >> +/** >> + * memblock_node_is_crypto_capable - get if whole node is capable >> + * of encryption >> + * @nid: number of node >> + * >> + * Iterate over all memory memblock_type and find if all regions under >> + * node @nid are capable of hardware encryption. >> + * >> + * Return: >> + * true if every region in memory memblock_type is capable of >> + * encryption, false otherwise. >> + */ >> +bool __init_memblock memblock_node_is_crypto_capable(int nid) >> +{ >> + struct memblock_region *region; >> + bool crypto_capable = false; >> + bool not_crypto_capable = false; >> + >> + for_each_mem_region(region) { >> + if (memblock_get_region_node(region) == nid) { >> + crypto_capable = >> + crypto_capable || >> + (region->flags & MEMBLOCK_CRYPTO_CAPABLE); > > This was already mentioned, but I just thought I'd add: this made me > double-take, given the "||" (instead of "|") in an assignment. It looked > like a typo, but yes it's correct. I was expecting something like: > > crypto_capable |= > !!(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > >> + not_crypto_capable = >> + not_crypto_capable || >> + !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > > not_crypto_capable |= > !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); > Yes, this also works. Thanks.
diff --git a/include/linux/memblock.h b/include/linux/memblock.h index 9dc7cb239d21..73edcce165a5 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -41,13 +41,15 @@ extern unsigned long long max_possible_pfn; * via a driver, and never indicated in the firmware-provided memory map as * system RAM. This corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED in the * kernel resource tree. + * @MEMBLOCK_CRYPTO_CAPABLE: capable of hardware encryption */ enum memblock_flags { - MEMBLOCK_NONE = 0x0, /* No special request */ - MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ - MEMBLOCK_MIRROR = 0x2, /* mirrored region */ - MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ - MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ + MEMBLOCK_NONE = 0x0, /* No special request */ + MEMBLOCK_HOTPLUG = 0x1, /* hotpluggable region */ + MEMBLOCK_MIRROR = 0x2, /* mirrored region */ + MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */ + MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */ + MEMBLOCK_CRYPTO_CAPABLE = 0x10, /* capable of hardware encryption */ }; /** @@ -121,6 +123,9 @@ int memblock_physmem_add(phys_addr_t base, phys_addr_t size); void memblock_trim_memory(phys_addr_t align); bool memblock_overlaps_region(struct memblock_type *type, phys_addr_t base, phys_addr_t size); +bool memblock_node_is_crypto_capable(int nid); +int memblock_mark_crypto_capable(phys_addr_t base, phys_addr_t size); +int memblock_clear_crypto_capable(phys_addr_t base, phys_addr_t size); int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size); int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size); int memblock_mark_mirror(phys_addr_t base, phys_addr_t size); diff --git a/mm/memblock.c b/mm/memblock.c index 1018e50566f3..fcf79befeab3 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -191,6 +191,42 @@ bool __init_memblock memblock_overlaps_region(struct memblock_type *type, return i < type->cnt; } +/** + * memblock_node_is_crypto_capable - get if whole node is capable + * of encryption + * @nid: number of node + * + * Iterate over all memory memblock_type and find if all regions under + * node @nid are capable of hardware encryption. + * + * Return: + * true if every region in memory memblock_type is capable of + * encryption, false otherwise. + */ +bool __init_memblock memblock_node_is_crypto_capable(int nid) +{ + struct memblock_region *region; + bool crypto_capable = false; + bool not_crypto_capable = false; + + for_each_mem_region(region) { + if (memblock_get_region_node(region) == nid) { + crypto_capable = + crypto_capable || + (region->flags & MEMBLOCK_CRYPTO_CAPABLE); + not_crypto_capable = + not_crypto_capable || + !(region->flags & MEMBLOCK_CRYPTO_CAPABLE); + } + } + + if (crypto_capable && not_crypto_capable) + pr_warn_once("Node %d has regions that are encryptable and regions that aren't", + nid); + + return !not_crypto_capable; +} + /** * __memblock_find_range_bottom_up - find free area utility in bottom-up * @start: start of candidate range @@ -885,6 +921,34 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base, return 0; } +/** + * memblock_mark_crypto_capable - Mark memory regions capable of hardware + * encryption with flag MEMBLOCK_CRYPTO_CAPABLE. + * @base: the base phys addr of the region + * @size: the size of the region + * + * Return: 0 on success, -errno on failure. + */ +int __init_memblock memblock_mark_crypto_capable(phys_addr_t base, + phys_addr_t size) +{ + return memblock_setclr_flag(base, size, 1, MEMBLOCK_CRYPTO_CAPABLE); +} + +/** + * memblock_clear_crypto_capable - Clear flag MEMBLOCK_CRYPTO for a + * specified region. + * @base: the base phys addr of the region + * @size: the size of the region + * + * Return: 0 on success, -errno on failure. + */ +int __init_memblock memblock_clear_crypto_capable(phys_addr_t base, + phys_addr_t size) +{ + return memblock_setclr_flag(base, size, 0, MEMBLOCK_CRYPTO_CAPABLE); +} + /** * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_HOTPLUG. * @base: the base phys addr of the region
Add the capability to mark regions of the memory memory_type able of hardware memory encryption. Also add the capability to query if all regions of a memory node are able to do hardware memory encryption to call it when initializing the nodes. Warn the user if a node has both encryptable and non-encryptable regions. Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> --- include/linux/memblock.h | 15 ++++++---- mm/memblock.c | 64 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-)