Transition memory to the shared state during a hot-remove operation so
that it can be re-used by the hypervisor. This also applies when memory
is intended to be hotplugged back in later, as those pages will need to
be re-accepted after crossing the trust boundary.
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
arch/x86/coco/sev/core.c | 13 ++++++
arch/x86/include/asm/sev.h | 2 +
arch/x86/include/asm/unaccepted_memory.h | 9 ++++
drivers/firmware/efi/unaccepted_memory.c | 56 ++++++++++++++++++++++++
include/linux/mm.h | 9 ++++
mm/memory_hotplug.c | 2 +
6 files changed, 91 insertions(+)
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 9ae3b11754e6..63d8f44b76eb 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -703,6 +703,19 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ unsigned long vaddr, npages;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ vaddr = (unsigned long)__va(start);
+ npages = (end - start) >> PAGE_SHIFT;
+
+ set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED);
+}
+
static int vmgexit_ap_control(u64 event, struct sev_es_save_area *vmsa, u32 apic_id)
{
bool create = event != SVM_VMGEXIT_AP_DESTROY;
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0e6c0940100f..3327de663793 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -514,6 +514,7 @@ bool snp_init(struct boot_params *bp);
void snp_dmi_setup(void);
int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input);
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
+void snp_unaccept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
@@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
return -ENOTTY;
}
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
+static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
static inline void sev_show_status(void) { }
diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
index f5937e9866ac..8715be843e65 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
}
}
+static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
+{
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
+ snp_unaccept_memory(start, end);
+ } else {
+ panic("Cannot unaccept memory: unknown platform\n");
+ }
+}
+
static inline struct efi_unaccepted_memory *efi_get_unaccepted_table(void)
{
if (efi.unaccepted == EFI_INVALID_TABLE_ADDR)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index 5a4c8b0f56c8..9f1d594dba33 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
}
+void unaccept_memory(phys_addr_t start, unsigned long size)
+{
+ unsigned long range_start, range_end, bitrange_end;
+ struct efi_unaccepted_memory *unaccepted;
+ phys_addr_t end = start + size;
+ u64 unit_size, phys_base;
+ unsigned long flags;
+
+ unaccepted = efi_get_unaccepted_table();
+ if (!unaccepted)
+ return;
+
+ phys_base = unaccepted->phys_base;
+ unit_size = unaccepted->unit_size;
+
+ if (start < unaccepted->phys_base)
+ start = unaccepted->phys_base;
+ if (end < unaccepted->phys_base)
+ return;
+
+ start -= phys_base;
+ end -= phys_base;
+
+ /* Make sure not to overrun the bitmap */
+ if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
+ end = unaccepted->size * unit_size * BITS_PER_BYTE;
+
+ range_start = start / unit_size;
+ bitrange_end = DIV_ROUND_UP(end, unit_size);
+
+ /* Only unaccept memory that was previously accepted in the range */
+ spin_lock_irqsave(&unaccepted_memory_lock, flags);
+ for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
+ bitrange_end) {
+ unsigned long phys_start, phys_end;
+ unsigned long len = range_end - range_start;
+
+ phys_start = range_start * unit_size + phys_base;
+ phys_end = range_end * unit_size + phys_base;
+
+ arch_unaccept_memory(phys_start, phys_end);
+ bitmap_set(unaccepted->bitmap, range_start, len);
+ }
+ spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
+}
+
bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
{
struct efi_unaccepted_memory *unaccepted;
@@ -227,6 +273,16 @@ void accept_hotplug_memory(phys_addr_t start, unsigned long size)
arch_accept_memory(start, start + size);
}
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+ if (range_contains_unaccepted_memory(start, size)) {
+ unaccept_memory(start, size);
+ return;
+ }
+
+ arch_unaccept_memory(start, start + size);
+}
+
#ifdef CONFIG_PROC_VMCORE
static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
unsigned long pfn)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2d3c1ea40606..faefaa9b92c6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4504,7 +4504,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
void accept_memory(phys_addr_t start, unsigned long size);
+void unaccept_memory(phys_addr_t start, unsigned long size);
void accept_hotplug_memory(phys_addr_t start, unsigned long size);
+void unaccept_hotplug_memory(phys_addr_t start, unsigned long size);
#else
@@ -4518,10 +4520,17 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
{
}
+static inline void unaccept_memory(phys_addr_t start, unsigned long size)
+{
+}
+
static inline void accept_hotplug_memory(phys_addr_t start, unsigned long size)
{
}
+static inline void unaccept_hotplug_memory(phys_addr_t start, unsigned long size)
+{
+}
#endif
static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 549ccfd190ee..21b87f2af930 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2240,6 +2240,8 @@ static int try_remove_memory(u64 start, u64 size)
mem_hotplug_begin();
+ unaccept_hotplug_memory(start, size);
+
rc = memory_blocks_have_altmaps(start, size);
if (rc < 0) {
mem_hotplug_done();
--
2.52.0On 1/28/26 12:41, Pratik R. Sampat wrote:
> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
> +{
> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> + snp_unaccept_memory(start, end);
> + } else {
> + panic("Cannot unaccept memory: unknown platform\n");
> + }
> +}
This panic() is pretty nasty.
Can't we just disable memory hotplug up front if it's:
!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)
?
On Wed, Jan 28, 2026 at 01:15:06PM -0800, Dave Hansen wrote:
> On 1/28/26 12:41, Pratik R. Sampat wrote:
> > +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
> > +{
> > + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> > + snp_unaccept_memory(start, end);
> > + } else {
> > + panic("Cannot unaccept memory: unknown platform\n");
> > + }
> > +}
>
> This panic() is pretty nasty.
>
> Can't we just disable memory hotplug up front if it's:
>
> !cc_platform_has(CC_ATTR_GUEST_SEV_SNP)
>
> ?
I don't understand SEV-SNP situation, but I don't think we need to do
anything on unplug for TDX. We should expect the unplugged memory to be
removed from SEPT. If VMM doesn't do this, it is effectively DoS and we
don't protect against DoS in CoCo.
Converting the memory to shared will do no good for us.
--
Kiryl Shutsemau / Kirill A. Shutemov
On 1/29/26 4:40 AM, Kiryl Shutsemau wrote:
> On Wed, Jan 28, 2026 at 01:15:06PM -0800, Dave Hansen wrote:
>> On 1/28/26 12:41, Pratik R. Sampat wrote:
>>> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
>>> +{
>>> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
>>> + snp_unaccept_memory(start, end);
>>> + } else {
>>> + panic("Cannot unaccept memory: unknown platform\n");
>>> + }
>>> +}
>>
>> This panic() is pretty nasty.
>>
>> Can't we just disable memory hotplug up front if it's:
>>
>> !cc_platform_has(CC_ATTR_GUEST_SEV_SNP)
>>
>> ?
>
> I don't understand SEV-SNP situation, but I don't think we need to do
> anything on unplug for TDX. We should expect the unplugged memory to be
> removed from SEPT. If VMM doesn't do this, it is effectively DoS and we
> don't protect against DoS in CoCo.
>
> Converting the memory to shared will do no good for us.
In that case a fall through for TDX (with a comment explaining why) and
panic for rest may be the way to go?
>
On 1/29/26 09:32, Pratik R. Sampat wrote: > In that case a fall through for TDX (with a comment explaining why) and > panic for rest may be the way to go? No. panic() is an absolute last resort. It's almost never the way to go. What else can we do to ensure we never reach this code if the platform doesn't support memory un-acceptance?
On 1/29/26 11:39 AM, Dave Hansen wrote: > On 1/29/26 09:32, Pratik R. Sampat wrote: >> In that case a fall through for TDX (with a comment explaining why) and >> panic for rest may be the way to go? > > No. panic() is an absolute last resort. It's almost never the way to go. > > What else can we do to ensure we never reach this code if the platform > doesn't support memory un-acceptance? The panic() here similar to its existing arch_accept_memory() counterpart is mostly to guard against a cant-happen scenario (unless Kiryl had a different intention writing the initial hook). It is called from functions that compile this in only if CONFIG_UNACCEPTED_MEMORY is enabled. TDX and SNP are the only two users of it today.
On Wed, 28 Jan 2026 14:41:05 -0600 "Pratik R. Sampat" <prsampat@amd.com> wrote:
> Transition memory to the shared state during a hot-remove operation so
> that it can be re-used by the hypervisor. This also applies when memory
> is intended to be hotplugged back in later, as those pages will need to
> be re-accepted after crossing the trust boundary.
>
> ...
>
> @@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
> return -ENOTTY;
> }
> static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
> +static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
> static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> static inline u64 sev_get_status(void) { return 0; }
> static inline void sev_show_status(void) { }
> diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
> index f5937e9866ac..8715be843e65 100644
> --- a/arch/x86/include/asm/unaccepted_memory.h
> +++ b/arch/x86/include/asm/unaccepted_memory.h
> @@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
> }
> }
>
> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
> +{
> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> + snp_unaccept_memory(start, end);
> + } else {
> + panic("Cannot unaccept memory: unknown platform\n");
Seems severe. Dropping a WARN() and continuing would be preferred.
What exactly happened here? Am I correct in thinking that the check in
snp_unaccept_memory() makes this a cant-happen?
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
> spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> }
>
> +void unaccept_memory(phys_addr_t start, unsigned long size)
> +{
> + unsigned long range_start, range_end, bitrange_end;
> + struct efi_unaccepted_memory *unaccepted;
> + phys_addr_t end = start + size;
> + u64 unit_size, phys_base;
> + unsigned long flags;
> +
> + unaccepted = efi_get_unaccepted_table();
> + if (!unaccepted)
> + return;
> +
> + phys_base = unaccepted->phys_base;
> + unit_size = unaccepted->unit_size;
> +
> + if (start < unaccepted->phys_base)
> + start = unaccepted->phys_base;
max()?
> + if (end < unaccepted->phys_base)
> + return;
> +
> + start -= phys_base;
> + end -= phys_base;
> +
> + /* Make sure not to overrun the bitmap */
> + if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
> + end = unaccepted->size * unit_size * BITS_PER_BYTE;
min()?
If you like min() and max(). Sometimes I find them annoying - need to mentally
expand them to figure out what's going on.
> + range_start = start / unit_size;
> + bitrange_end = DIV_ROUND_UP(end, unit_size);
> +
> + /* Only unaccept memory that was previously accepted in the range */
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
> + bitrange_end) {
> + unsigned long phys_start, phys_end;
> + unsigned long len = range_end - range_start;
> +
> + phys_start = range_start * unit_size + phys_base;
> + phys_end = range_end * unit_size + phys_base;
> +
> + arch_unaccept_memory(phys_start, phys_end);
> + bitmap_set(unaccepted->bitmap, range_start, len);
> + }
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> +}
Hi Dave, Andrew
On 1/28/26 3:08 PM, Andrew Morton wrote:
> On Wed, 28 Jan 2026 14:41:05 -0600 "Pratik R. Sampat" <prsampat@amd.com> wrote:
>
>> Transition memory to the shared state during a hot-remove operation so
>> that it can be re-used by the hypervisor. This also applies when memory
>> is intended to be hotplugged back in later, as those pages will need to
>> be re-accepted after crossing the trust boundary.
>>
>> ...
>>
>> @@ -623,6 +624,7 @@ static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
>> return -ENOTTY;
>> }
>> static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
>> +static inline void snp_unaccept_memory(phys_addr_t start, phys_addr_t end) { }
>> static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
>> static inline u64 sev_get_status(void) { return 0; }
>> static inline void sev_show_status(void) { }
>> diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
>> index f5937e9866ac..8715be843e65 100644
>> --- a/arch/x86/include/asm/unaccepted_memory.h
>> +++ b/arch/x86/include/asm/unaccepted_memory.h
>> @@ -18,6 +18,15 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
>> }
>> }
>>
>> +static inline void arch_unaccept_memory(phys_addr_t start, phys_addr_t end)
>> +{
>> + if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
>> + snp_unaccept_memory(start, end);
>> + } else {
>> + panic("Cannot unaccept memory: unknown platform\n");
>
> Seems severe. Dropping a WARN() and continuing would be preferred.
>
> What exactly happened here? Am I correct in thinking that the check in
> snp_unaccept_memory() makes this a cant-happen?
>
You're right, a WARN() is probably more appropriate here.
Based on my rudimentary understanding of TDX from Kiryl, TDX module is
what maintains this metadata for the HPA.
So, maybe the WARN could just be:
"Cannot unaccept memory: VMM responsible for unaccepting memory" for
TDX? Or, we could let it fall-through for TDX (if and when there is
support for that)
>> --- a/drivers/firmware/efi/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>> @@ -157,6 +157,52 @@ void accept_memory(phys_addr_t start, unsigned long size)
>> spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> }
>>
>> +void unaccept_memory(phys_addr_t start, unsigned long size)
>> +{
>> + unsigned long range_start, range_end, bitrange_end;
>> + struct efi_unaccepted_memory *unaccepted;
>> + phys_addr_t end = start + size;
>> + u64 unit_size, phys_base;
>> + unsigned long flags;
>> +
>> + unaccepted = efi_get_unaccepted_table();
>> + if (!unaccepted)
>> + return;
>> +
>> + phys_base = unaccepted->phys_base;
>> + unit_size = unaccepted->unit_size;
>> +
>> + if (start < unaccepted->phys_base)
>> + start = unaccepted->phys_base;
>
> max()?
>
>> + if (end < unaccepted->phys_base)
>> + return;
>> +
>> + start -= phys_base;
>> + end -= phys_base;
>> +
>> + /* Make sure not to overrun the bitmap */
>> + if (end > unaccepted->size * unit_size * BITS_PER_BYTE)
>> + end = unaccepted->size * unit_size * BITS_PER_BYTE;
>
> min()?
>
> If you like min() and max(). Sometimes I find them annoying - need to mentally
> expand them to figure out what's going on.
Same! :-)
That is why I just aped the implementation from its counterpart
accept_memory() but can definitely do it this way and shave off a couple
of lines.
Thanks,
--Pratik
>
>> + range_start = start / unit_size;
>> + bitrange_end = DIV_ROUND_UP(end, unit_size);
>> +
>> + /* Only unaccept memory that was previously accepted in the range */
>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> + for_each_clear_bitrange_from(range_start, range_end, unaccepted->bitmap,
>> + bitrange_end) {
>> + unsigned long phys_start, phys_end;
>> + unsigned long len = range_end - range_start;
>> +
>> + phys_start = range_start * unit_size + phys_base;
>> + phys_end = range_end * unit_size + phys_base;
>> +
>> + arch_unaccept_memory(phys_start, phys_end);
>> + bitmap_set(unaccepted->bitmap, range_start, len);
>> + }
>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> +}
>
© 2016 - 2026 Red Hat, Inc.