Messages in this thread | | | From | "Michael Kelley (LINUX)" <> | Subject | RE: [PATCH] x86/mm/cpa: Warn if set_memory_XXcrypted() fails | Date | Fri, 27 Oct 2023 17:08:11 +0000 |
| |
From: Edgecombe, Rick P <rick.p.edgecombe@intel.com> 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.
| |