On Tue, 2023-10-31 at 11:43 +0100, Petr Tesařík wrote: > > I admit I'm not familiar with the encryption/decryption API, but if a > __free_pages() is not sufficient here, then it is quite confusing. > The error label is reached only if set_memory_decrypted() returns > non-zero. My naive expectation is that the memory is *not* decrypted > in > that case and does not require special treatment. Is this assumption > wrong? Yea, the memory can still be decrypted, or partially decrypted. On x86, all the set_memory() calls can fail part way through the work, and they don't rollback the changes they had made up to that point. I was thinking about trying to changes this, but that is the current behavior. But in TDX's case set_memory_decrypted() can be fully successful and just return an error. And there aren't any plans to fix the TDX case (which has special VMM work that the kernel doesn't have control over). So instead, the plan is to WARN about it and count on the caller to handle the error properly: https://lore.kernel.org/lkml/20231027214744.1742056-1-rick.p.edgecombe@xxxxxxxxx/ > > OTOH I believe there is a bug in the logic. The subsequent > __free_pages() in swiotlb_alloc_tlb() would have to be changed to a > free_decrypted_pages(). However, I'm proposing a different approach > to > address the latter issue here: > > https://lore.kernel.org/linux-iommu/20231026095123.222-1-petrtesarik@xxxxxxxxxxxxxxx/T/ Oh, yes, that makes sense. I was planning to send a patch to just leak the pages if set_memory_decrypted() fails, after my v2 linked above is accepted. It could have a different label than the phys_limit check error path added in your linked patch, so that case would still free the perfectly fine encrypted pages.
