mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Pandey, Radhey Shyam" <radheys@amd.com>
To: Pratyush Yadav <pratyush@kernel.org>, Mike Rapoport <rppt@kernel.org>
Cc: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>,
	akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org,
	liam@infradead.org, vbabka@kernel.org, surenb@google.com,
	mhocko@suse.com, baoquan.he@linux.dev, pasha.tatashin@soleen.com,
	graf@amazon.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, kexec@lists.infradead.org
Subject: Re: [RFC PATCH 1/2] kexec: do not place the next kernel in a fixed-address CMA area
Date: Fri, 30 Oct 2026 13:22:50 +0530	[thread overview]
Message-ID: <7625fa23-5f4b-4658-b17a-fe4e58103e7c@amd.com> (raw)
In-Reply-To: <2vxzzevwmz2v.fsf@kernel.org>

On 10/29/2026 6:41 PM, Pratyush Yadav wrote:
> On Thu, Oct 29 2026, Mike Rapoport wrote:
> 
>> On Wed, Oct 28, 2026 at 03:03:16PM +0100, Pratyush Yadav wrote:
>>> On Wed, Oct 14 2026, Radhey Shyam Pandey wrote:
>>>
>>>> kexec_alloc_contig() picks the destination of the next kernel out of the
>>>> default CMA area, and the next kernel then runs from there in place
>>>> instead of being copied out. That is only safe if the area will not exist
>>>> at the same address in the kernel being loaded.
>>>>
>>>> For an area whose base came from the device tree, it will. The loaded
>>>> kernel parses the same description, declares the range it is executing
>>>> from as a shared-dma-pool, and cma_activate_area() releases the pages
>>>> holding its own image to the page allocator.
>>>>
>>>> Distinguish the two cases. An area placed by cma_declare_contiguous_nid()
>>>> without a fixed base lands wherever this kernel found room, and the next
>>>> kernel repeats that search against its own memblock state, so it cannot
>>>> collide. Record the difference as CMA_FIXED_BASE and refuse to use such
>>>> an area for the payload, which leaves kexec_locate_mem_hole() to fall
>>>> back to the memblock hole search as it did before CMA placement existed.
>>>>
>>>> This is what makes the KEXEC_FILE_NO_CMA flag unnecessary on platforms
>>>> that describe their CMA pools in the device tree.
>>>>
>>>> Tested on a AMD Versal AI Edge Series Gen 2 VEK385 Evaluation Kit whose
>>>> device tree declares three CMA pools, the first of which is
>>>> linux,cma-default:
>>>>
>>>>    cma_reserved@800000000     4 GiB at 0x800000000    (default)
>>>>    cma_reserved@50000000000   6 GiB at 0x50000000000
>>>>    cma_reserved@60000000000   4 GiB at 0x60000000000
>>>>
>>>> All three have a fixed base, so kexec_alloc_contig() declines the default
>>>> area and kexec_locate_mem_hole() falls through to the memblock search,
>>>> which places the Image segment at 0x9c00000 size 0x1de0000. The kernel
>>>> that boots from it reports its image as [0x09c00000-0x0b9e0000] and
>>>> 14680064K cma-reserved, i.e. all three pools still present and none of
>>>> them overlapping the running kernel.
>>>>
>>>> Fixes: 07d24902977e ("kexec: enable CMA based contiguous allocation")
>>>> Assisted-by: Claude:claude-opus-5
>>>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>>>> ---
>>>>   include/linux/cma.h | 10 ++++++++++
>>>>   kernel/kexec_file.c | 12 ++++++++++++
>>>>   mm/cma.c            | 16 ++++++++++++++++
>>>>   mm/cma.h            |  2 ++
>>>>   4 files changed, 40 insertions(+)
>>>>
>>>> diff --git a/include/linux/cma.h b/include/linux/cma.h
>>>> index 8555d38a97b1..2fe66377e5d3 100644
>>>> --- a/include/linux/cma.h
>>>> +++ b/include/linux/cma.h
>>>> @@ -61,4 +61,14 @@ extern int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)
>>>>   extern bool cma_intersects(struct cma *cma, unsigned long start, unsigned long end);
>>>>   
>>>>   extern void cma_reserve_pages_on_error(struct cma *cma);
>>>> +
>>>> +#ifdef CONFIG_CMA
>>>> +bool cma_has_fixed_base(const struct cma *cma);
>>>> +#else
>>>> +static inline bool cma_has_fixed_base(const struct cma *cma)
>>>> +{
>>>> +	return false;
>>>> +}
>>>> +#endif
>>>> +
>>>>   #endif
>>>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>>>> index b668ce8d68ee..10e770bcee0b 100644
>>>> --- a/kernel/kexec_file.c
>>>> +++ b/kernel/kexec_file.c
>>>> @@ -28,6 +28,7 @@
>>>>   #include <linux/vmalloc.h>
>>>>   #include <linux/dma-map-ops.h>
>>>>   #include <linux/kexec_handover.h>
>>>> +#include <linux/cma.h>
>>>>   #include "kexec_internal.h"
>>>>   
>>>>   #ifdef CONFIG_KEXEC_SIG
>>>> @@ -691,6 +692,7 @@ static int kexec_walk_resources(struct kexec_buf *kbuf,
>>>>   static int kexec_alloc_contig(struct kexec_buf *kbuf)
>>>>   {
>>>>   	size_t nr_pages = kbuf->memsz >> PAGE_SHIFT;
>>>> +	struct cma *cma;
>>>>   	unsigned long mem;
>>>>   	struct page *p;
>>>>   
>>>> @@ -702,6 +704,16 @@ static int kexec_alloc_contig(struct kexec_buf *kbuf)
>>>>   	if (kbuf->image->type == KEXEC_TYPE_CRASH)
>>>>   		return -EPERM;
>>>>   
>>>> +	/*
>>>> +	 * The next kernel runs from here rather than being copied out, so it
>>>> +	 * would declare this area again from the same description and release
>>>> +	 * the pages holding its own image to the page allocator. Only an area
>>>> +	 * this kernel placed itself is safe to use.
>>>> +	 */
>>>> +	cma = dev_get_cma_area(NULL);
>>>> +	if (!cma || cma_has_fixed_base(cma))
>>>> +		return -EOPNOTSUPP;
>>>> +
>>>>   	p = dma_alloc_from_contiguous(NULL, nr_pages, get_order(kbuf->buf_align), true);
>>>>   	if (!p)
>>>>   		return -ENOMEM;
>>>> diff --git a/mm/cma.c b/mm/cma.c
>>>> index a10ea37a261d..1f4d3d48a3bb 100644
>>>> --- a/mm/cma.c
>>>> +++ b/mm/cma.c
>>>> @@ -55,6 +55,19 @@ const char *cma_get_name(const struct cma *cma)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(cma_get_name);
>>>>   
>>>> +/**
>>>> + * cma_has_fixed_base() - was the area placed at an address it did not choose?
>>>> + * @cma: Contiguous memory region
>>>> + *
>>>> + * Such an area is described outside the kernel, typically by a device tree
>>>> + * reserved-memory node, and so lands at the same address in every kernel
>>>> + * booted with that description.
>>>> + */
>>>> +bool cma_has_fixed_base(const struct cma *cma)
>>>> +{
>>>> +	return test_bit(CMA_FIXED_BASE, &cma->flags);
>>>> +}
>>>> +
>>>>   static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
>>>>   					     unsigned int align_order)
>>>>   {
>>>> @@ -304,6 +317,7 @@ int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
>>>>   	cma->ranges[0].count = cma->count;
>>>>   	cma->nranges = 1;
>>>>   	cma->nid = NUMA_NO_NODE;
>>>> +	set_bit(CMA_FIXED_BASE, &cma->flags);
>>>>   
>>>>   	*res_cma = cma;
>>>>   
>>>> @@ -517,6 +531,8 @@ static int __init __cma_declare_contiguous_nid(phys_addr_t *basep,
>>>>   	}
>>>>   
>>>>   	(*res_cma)->nid = nid;
>>>> +	if (!fixed)
>>>> +		clear_bit(CMA_FIXED_BASE, &(*res_cma)->flags);
>>>
>>> Nit: This reads a bit odd. cma_init_reserved_mem() sets the fixed base
>>> and then you clear it later. Wouldn't it be better to just pass the
>>> fixed boolean to cma_init_reserved_mem()?
>>
>> Actually, we shouldn't expose cma_init_reserved_mem() at all.
>>
>> The current callers of cma_init_reserved_mem() should be switched to
>> cma_declare_contiguous_nid() that does sanity checks before calling
>> cma_init_reserved_mem().
> 

Thanks Pratyush and Mike for the review.

The switch from cma_init_reserved_mem() to cma_declare_contiguous_nid() 
looks like a broader CMA refactoring and requires additional changes to 
work correctly. See below for details.

cma_init_reserved_mem() is the entry point for memory the caller
already reserved, while cma_declare_contiguous_nid() is for memory
CMA reserves itself, makes the split read as intentional.

Run second time on memory stage one already claimed, and
cma_fixed_reserve()'s guard exists specifically to reject a fixed
base that is already reserved, so it returns -EBUSY.

fdt_scan_reserved_mem()  → __reserved_mem_reserve_reg()
→ early_init_dt_reserve_memory() → memblock_reserve(base, size)

The CMA initfn runs in a second, separate walk much later:
fdt_scan_reserved_mem_late() → fdt_init_reserved_mem_node()
→ __reserved_mem_init_node() → rmem_cma_setup().

Mike: If the redundant caller-side check is a concern, I can address it 
with a separate patch in this series. v2 would then look as follows:

mm/cma: drop the duplicate alignment check in rmem_cma_setup()
of: reserved_mem: keep the reservation when node_init returns -EBUSY
mm/cma: reject CMA areas that overlap the kernel image
kexec: do not place the next kernel in a fixed-address CMA area

However if you still think CMA refactor is necessary, please
advise on how should we approach to fix cma_declare_contiguous_nid() for 
already reserved segments.

Thanks,
Radhey


  reply	other threads:[~2026-10-30  7:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 14:57 [RFC PATCH 0/2] kexec,mm/cma: do not run the next kernel from a fixed-base " Radhey Shyam Pandey
2026-10-14 14:57 ` [RFC PATCH 1/2] kexec: do not place the next kernel in a fixed-address " Radhey Shyam Pandey
2026-10-28 14:03   ` Pratyush Yadav
2026-10-29  7:35     ` Mike Rapoport
2026-10-29 13:11       ` Pratyush Yadav
2026-10-30  7:52         ` Pandey, Radhey Shyam [this message]
2026-10-30 12:03           ` Mike Rapoport
2026-10-30 17:59             ` Pandey, Radhey Shyam
2026-10-14 14:57 ` [RFC PATCH 2/2] mm/cma: reject CMA areas that overlap the kernel image Radhey Shyam Pandey
2026-10-28 14:16   ` Pratyush Yadav
2026-10-29  7:42   ` Mike Rapoport

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7625fa23-5f4b-4658-b17a-fe4e58103e7c@amd.com \
    --to=radheys@amd.com \
    --cc=akpm@linux-foundation.org \
    --cc=baoquan.he@linux.dev \
    --cc=david@kernel.org \
    --cc=graf@amazon.com \
    --cc=kexec@lists.infradead.org \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=pratyush@kernel.org \
    --cc=radhey.shyam.pandey@amd.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®