dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
Cc: sashiko-reviews@lists.linux.dev, dri-devel@lists.freedesktop.org,
	media-ci@linuxtv.org, Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.linux.dev, Oliver Upton <oupton@kernel.org>
Subject: Re: [RFC PATCH v8 01/14] mm: Add an allocator for CoCo shared memory
Date: Thu, 24 Sep 2026 15:26:43 -0300	[thread overview]
Message-ID: <20260924182643.GC9354@nvidia.com> (raw)
In-Reply-To: <yq5a1pai4tlc.fsf@kernel.org>

On Thu, Sep 24, 2026 at 07:52:39PM +0530, Aneesh Kumar K.V wrote:
> >> +int cc_make_shared(void *addr, size_t size)
> >> +{
> >> +	int ret = cc_validate_transition(addr, size);
> >> +
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return set_memory_decrypted((unsigned long)addr, size >> PAGE_SHIFT);
> >> +}
> >
> > [Severity: Medium]
> > Will this function be unavailable to dynamically loadable modules since it
> > is missing an EXPORT_SYMBOL_GPL() macro?
> >
> > The commit message states it should be usable by callers managing their own
> > backing memory, but virtio or guest drivers won't be able to link to it
> > without an export.

> >> +
> >> +int cc_make_private(void *addr, size_t size)
> >> +{
> >> +	int ret = cc_validate_transition(addr, size);
> >> +
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return set_memory_encrypted((unsigned long)addr, size >> PAGE_SHIFT);
> >> +}
> >
> > [Severity: Medium]
> > Does this function also need an EXPORT_SYMBOL_GPL() macro so that modules
> > can successfully link and call it?
> 
> I guess we don't want the cc_make_shared() and cc_make_private() APIs to be
> called by modules. Modules that need CoCo shared memory should use
> alloc_cc_shared_pages().

Indeed, but we may also be over indexing on the cc name given AMD also
uses this for their troubled host memory encryption address limit problem :\

The note about virtio seems a bit strange, these are new functions why
does it think virtio is calling them?

> >> +	/*
> >> +	 * State transitions require a linear-map address and may modify memory.
> >> +	 * Allocate from low memory and defer requested zeroing until afterwards.
> >> +	 */
> >> +	gfp &= ~(__GFP_HIGHMEM | __GFP_ZERO);
> >
> > [Severity: High]
> > Does this code inadvertently leave __GFP_MOVABLE intact if passed by the
> > caller?
> >
> > Pages are returned as shared memory, but they lack migration hooks
> > (movable_operations) to handle encryption state transitions during page
> > migration.
> >
> > If a caller passes __GFP_MOVABLE, the page might be placed in ZONE_MOVABLE.
> > Since the kernel migration code does not know how to handle encryption state
> > transitions, the page cannot be safely migrated. Could this permanently pin
> > the movable zone, thereby breaking memory hot-unplug and compaction?
> 
> I don't think we want CoCo shared memory allocations to be movable, but
> I'm also not sure whether we should filter out all of these GFP flags.

It sounds like we should at least *reject* movable. It is right that
seems incompatible with this, and we have no use case..

> >> +	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> >> +		return __alloc_cc_shared_pages_node(nid, gfp, requested, mem);
> >> +
> >> +	order = get_order(requested);
> >> +	if (order > MAX_PAGE_ORDER)
> >> +		return -EINVAL;
> >> +
> >> +	if (nid == NUMA_NO_NODE)
> >> +		page = alloc_pages(gfp, order);
> >> +	else
> >> +		page = alloc_pages_node(nid, gfp, order);
> >
> > [Severity: Medium]
> > Can this fallback path inadvertently allocate a highmem page?
> >
> > The function's kernel-doc explicitly guarantees: "A memory-state transition
> > requires a valid linear-map address, so such allocations never come from
> > high memory".
> 
> Only when a private-to-shared transition is required.

IMHO disable the whole thing at kconfig if highmem is enabled. Make
the calls always fail.

Jason

  reply	other threads:[~2026-09-24 18:26 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 10:05 [RFC PATCH v8 00/14] coco: guest: Add a shared-granule allocator for host-shared memory Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 01/14] mm: Add an allocator for CoCo shared memory Aneesh Kumar K.V (Arm)
2026-09-24 10:19   ` sashiko-bot
2026-09-24 14:22     ` Aneesh Kumar K.V
2026-09-24 18:26       ` Jason Gunthorpe [this message]
2026-09-24 10:05 ` [RFC PATCH v8 02/14] mm: Zero memory during shared memory transitions Aneesh Kumar K.V (Arm)
2026-09-24 10:22   ` sashiko-bot
2026-09-25 12:08   ` Kiryl Shutsemau
2026-09-24 10:05 ` [RFC PATCH v8 03/14] irqchip/gic-v3-its: Resolve the default NUMA node explicitly Aneesh Kumar K.V (Arm)
2026-09-24 10:19   ` sashiko-bot
2026-09-24 10:05 ` [RFC PATCH v8 04/14] irqchip/gic-v3-its: Allocate shared tables using CoCo shared memory allocator Aneesh Kumar K.V (Arm)
2026-09-24 10:19   ` sashiko-bot
2026-09-24 10:05 ` [RFC PATCH v8 05/14] dma-contiguous: Derive shared alignment from DMA attributes Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 06/14] dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator Aneesh Kumar K.V (Arm)
2026-09-24 10:19   ` sashiko-bot
2026-09-24 10:05 ` [RFC PATCH v8 07/14] dma-direct: Align CoCo shared DMA allocations to the shared granule size Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 08/14] swiotlb: Align shared IO TLB pools " Aneesh Kumar K.V (Arm)
2026-09-24 10:22   ` sashiko-bot
2026-09-24 10:05 ` [RFC PATCH v8 09/14] swiotlb: Reject misaligned restricted DMA pools for CoCo guests Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 10/14] dma-buf: system_heap: Limit scatterlist entries to the buffer size Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 11/14] dma-buf: system_heap: Allocate shared buffers using CoCo shared memory allocator Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 12/14] swiotlb: Make rounded shared pool capacity allocatable Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 13/14] mm: Assert CoCo shared allocations may sleep Aneesh Kumar K.V (Arm)
2026-09-24 10:05 ` [RFC PATCH v8 14/14] irqchip/gic-v3-its: Preallocate VPE L1 tables Aneesh Kumar K.V (Arm)

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=20260924182643.GC9354@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=aneesh.kumar@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=media-ci@linuxtv.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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