mirror of https://lore.kernel.org/lkml/
 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: 8+ 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 [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-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 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®