diff mbox series

[v7,2/4] mm: Introduce memblock_isolate_memory

Message ID 20230310094539.764357-3-alexghiti@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series riscv: Use PUD/P4D/PGD pages for the linear mapping | expand

Commit Message

Alexandre Ghiti March 10, 2023, 9:45 a.m. UTC
This function allows to split a region in memblock.memory and will be
useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
allows to isolate the kernel text/rodata and then avoid to map those
regions with a PUD/P4D/PGD.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

Comments

Mike Rapoport March 12, 2023, 9:35 a.m. UTC | #1
Hi Alexandre,

On Fri, Mar 10, 2023 at 10:45:37AM +0100, Alexandre Ghiti wrote:
> This function allows to split a region in memblock.memory and will be
> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
> allows to isolate the kernel text/rodata and then avoid to map those
> regions with a PUD/P4D/PGD.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  include/linux/memblock.h |  1 +
>  mm/memblock.c            | 22 +++++++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 50ad19662a32..2f7ef97c0da7 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>  int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>  
>  void memblock_free_all(void);
>  void memblock_free(void *ptr, size_t size);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 25fd0626a9e7..d8cf1c9eccf0 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -753,7 +753,8 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>  	int idx;
>  	struct memblock_region *rgn;
>  
> -	*start_rgn = *end_rgn = 0;
> +	if (start_rgn && end_rgn)
> +		*start_rgn = *end_rgn = 0;

Generally, it's possible that either start_rgn or end_rgn will be a valid
pointer and this should be handled here and below.

My preference, though would be to leave memblock_isolate_range() as is and
have unused start_rgn and end_rgn in memblock_isolate_memory().

>  
>  	if (!size)
>  		return 0;
> @@ -795,6 +796,9 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>  					       memblock_get_region_node(rgn),
>  					       rgn->flags);
>  		} else {
> +			if (!end_rgn || !start_rgn)
> +				continue;
> +
>  			/* @rgn is fully contained, record it */
>  			if (!*end_rgn)
>  				*start_rgn = idx;
> @@ -805,6 +809,22 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>  	return 0;
>  }
>  
> +/**
> + * memblock_isolate_memory - isolate given range from memblock.memory

I think it'd better to use "... range in memblock.memory"

> + * @base: base of range to isolate
> + * @size: size of range to isolate
> + *
> + * Call memblock_isolate_range on memblock.memory to isolate the given range.

Please elaborate that isolate means that the range does not share regions
with other ranges.

> + *
> + * Return:
> + * 0 on success, -errno on failure.
> + */
> +
> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
> +{
> +	return memblock_isolate_range(&memblock.memory, base, size, NULL, NULL);
> +}
> +
>  static int __init_memblock memblock_remove_range(struct memblock_type *type,
>  					  phys_addr_t base, phys_addr_t size)
>  {
> -- 
> 2.37.2
>
Alexandre Ghiti March 12, 2023, 4:14 p.m. UTC | #2
On 3/12/23 10:35, Mike Rapoport wrote:
> Hi Alexandre,
>
> On Fri, Mar 10, 2023 at 10:45:37AM +0100, Alexandre Ghiti wrote:
>> This function allows to split a region in memblock.memory and will be
>> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
>> allows to isolate the kernel text/rodata and then avoid to map those
>> regions with a PUD/P4D/PGD.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   include/linux/memblock.h |  1 +
>>   mm/memblock.c            | 22 +++++++++++++++++++++-
>>   2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index 50ad19662a32..2f7ef97c0da7 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>   int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>   int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>   int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
>> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>>   
>>   void memblock_free_all(void);
>>   void memblock_free(void *ptr, size_t size);
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 25fd0626a9e7..d8cf1c9eccf0 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -753,7 +753,8 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>   	int idx;
>>   	struct memblock_region *rgn;
>>   
>> -	*start_rgn = *end_rgn = 0;
>> +	if (start_rgn && end_rgn)
>> +		*start_rgn = *end_rgn = 0;
> Generally, it's possible that either start_rgn or end_rgn will be a valid
> pointer and this should be handled here and below.
>
> My preference, though would be to leave memblock_isolate_range() as is and
> have unused start_rgn and end_rgn in memblock_isolate_memory().


Sure, I'll do that then.


>
>>   
>>   	if (!size)
>>   		return 0;
>> @@ -795,6 +796,9 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>   					       memblock_get_region_node(rgn),
>>   					       rgn->flags);
>>   		} else {
>> +			if (!end_rgn || !start_rgn)
>> +				continue;
>> +
>>   			/* @rgn is fully contained, record it */
>>   			if (!*end_rgn)
>>   				*start_rgn = idx;
>> @@ -805,6 +809,22 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>   	return 0;
>>   }
>>   
>> +/**
>> + * memblock_isolate_memory - isolate given range from memblock.memory
> I think it'd better to use "... range in memblock.memory"


Ok


>
>> + * @base: base of range to isolate
>> + * @size: size of range to isolate
>> + *
>> + * Call memblock_isolate_range on memblock.memory to isolate the given range.
> Please elaborate that isolate means that the range does not share regions
> with other ranges.


Sure, thanks, I'll come up with a v8 tomorrow along with other fixes 
(rv32 and arm64 comments that I forgot to remove).

Thanks again,

Alex


>> + *
>> + * Return:
>> + * 0 on success, -errno on failure.
>> + */
>> +
>> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
>> +{
>> +	return memblock_isolate_range(&memblock.memory, base, size, NULL, NULL);
>> +}
>> +
>>   static int __init_memblock memblock_remove_range(struct memblock_type *type,
>>   					  phys_addr_t base, phys_addr_t size)
>>   {
>> -- 
>> 2.37.2
>>
diff mbox series

Patch

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 50ad19662a32..2f7ef97c0da7 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -125,6 +125,7 @@  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
 int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
 int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
+int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
 
 void memblock_free_all(void);
 void memblock_free(void *ptr, size_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index 25fd0626a9e7..d8cf1c9eccf0 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -753,7 +753,8 @@  static int __init_memblock memblock_isolate_range(struct memblock_type *type,
 	int idx;
 	struct memblock_region *rgn;
 
-	*start_rgn = *end_rgn = 0;
+	if (start_rgn && end_rgn)
+		*start_rgn = *end_rgn = 0;
 
 	if (!size)
 		return 0;
@@ -795,6 +796,9 @@  static int __init_memblock memblock_isolate_range(struct memblock_type *type,
 					       memblock_get_region_node(rgn),
 					       rgn->flags);
 		} else {
+			if (!end_rgn || !start_rgn)
+				continue;
+
 			/* @rgn is fully contained, record it */
 			if (!*end_rgn)
 				*start_rgn = idx;
@@ -805,6 +809,22 @@  static int __init_memblock memblock_isolate_range(struct memblock_type *type,
 	return 0;
 }
 
+/**
+ * memblock_isolate_memory - isolate given range from memblock.memory
+ * @base: base of range to isolate
+ * @size: size of range to isolate
+ *
+ * Call memblock_isolate_range on memblock.memory to isolate the given range.
+ *
+ * Return:
+ * 0 on success, -errno on failure.
+ */
+
+int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
+{
+	return memblock_isolate_range(&memblock.memory, base, size, NULL, NULL);
+}
+
 static int __init_memblock memblock_remove_range(struct memblock_type *type,
 					  phys_addr_t base, phys_addr_t size)
 {