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

["Pratik R. Sampat" <prsampat@amd.com>]

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