Re: [PATCH 04/10] swiotlb: Use free_decrypted_pages()

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

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@intel.com/

> 
> 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@huaweicloud.com/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.