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

[Pratik R. Sampat]

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