All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Pratyush Yadav <pratyush@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: Thu, 29 Oct 2026 09:35:45 +0200	[thread overview]
Message-ID: <auL3UW2InVqYU6Fs@kernel.org> (raw)
In-Reply-To: <2vxzh5i5orbv.fsf@kernel.org>

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().

This way the callers won't need to keep the checks for e.g
CMA_MIN_ALIGNMENT and we'll have the clear way to differentiate areas with
fixed based from allocated ones.
 
> With that fixed,
> 
> Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> 
> >  	*basep = base;
> >  
> >  	return 0;
> > diff --git a/mm/cma.h b/mm/cma.h
> > index 68e574b0f951..1bab24033284 100644
> > --- a/mm/cma.h
> > +++ b/mm/cma.h
> > @@ -80,6 +80,8 @@ enum cma_flags {
> >  	CMA_ZONES_VALID,
> >  	CMA_ZONES_INVALID,
> >  	CMA_ACTIVATED,
> > +	/* Base address was dictated by the caller, not picked by CMA. */
> > +	CMA_FIXED_BASE,
> >  };
> >  
> >  extern struct cma cma_areas[MAX_CMA_AREAS];
> 
> -- 
> Regards,
> Pratyush Yadav

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2026-10-29  7:35 UTC|newest]

Thread overview: 10+ 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 CMA area 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-14 15:07   ` sashiko-bot
2026-10-28 14:03   ` Pratyush Yadav
2026-10-29  7:35     ` Mike Rapoport [this message]
2026-10-29 13:11       ` Pratyush Yadav
2026-10-14 14:57 ` [RFC PATCH 2/2] mm/cma: reject CMA areas that overlap the kernel image Radhey Shyam Pandey
2026-10-14 15:09   ` sashiko-bot
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=auL3UW2InVqYU6Fs@kernel.org \
    --to=rppt@kernel.org \
    --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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.