linux-kernel - Re: [PATCH v3 2/2] x86/sev: Add support to unaccept memory after hot-remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <eaae739c-223f-43aa-9f08-fe0e7c2ec290@amd.com>
Date: Wed, 28 Jan 2026 16:25:29 -0600
From: "Pratik R. Sampat" <prsampat@....com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-mm@...ck.org, linux-coco@...ts.linux.dev, x86@...nel.org,
 linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
 bp@...en8.de, dave.hansen@...ux.intel.com, kas@...nel.org, ardb@...nel.org,
 david@...nel.org, osalvador@...e.de, thomas.lendacky@....com,
 michael.roth@....com
Subject: Re: [PATCH v3 2/2] x86/sev: Add support to unaccept memory after
 hot-remove

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@....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);
>> +}
>