RE: [PATCH] x86/mm/cpa: Warn if set_memory_XXcrypted() fails

[Michael Kelley (LINUX)]

From: Edgecombe, Rick P <rick.p.edgecombe@xxxxxxxxx> Sent: Friday, October 27, 2023 9:47 AM
> 
> On Fri, 2023-10-27 at 16:37 +0000, Michael Kelley (LINUX) wrote:
> > When a caller gets an error from set_memory_decrypted(), it will
> > take steps to try to get the memory back into a "good" state so
> > that it can put the memory back on the free list.   If it can't get
> > the memory back into a good state, then it will leak the memory.
> > I was thinking about how the caller will make that determination.
> > Is it based on whether set_memory_encrypted() succeeds?  I think
> > that works, as long as (for x86 at least) set_memory_encrypted()
> > ensures that the guest PTEs are all marked "private" before it
> > returns success.
> >
> > So maybe my comment applies to the caller in the sense of
> > understanding what steps the caller should take to recover from
> > an error, and the possible outcomes from the attempted recovery.
> 
> Since I was dropping free_decrypted_pages() helper, I was thinking to
> actually just leak the pages if set_memory_decryted() fails. As in, not
> try to recover them with set_memory_encrypted(). So the kernel will do
> the 3 retries that the recent HyperV focused patch added, and then walk
> away.
> 
> The kernel will already be warning about this situation, so we are not
> expecting for it to be common. For rare cases, it seems simpler to just
> leak it, and then set_memory_encrypted() can be simpler as it doesn't
> need to worry about handling mixed ranges returning success.
> 

I like that approach even better than trying to fix things up and get
the memory back on the guest free list.  I agree the error case should
be rare, and I'm generally leery of putting memory on the free list
when there's some doubt about the private/shared state of the page
from the host/VMM standpoint.

Michael

> I'll update the log to clarify the importance of the PTE being marked
> shared in the guest, and post a v2.