RE: [PATCH] x86/tdx: Support vmalloc() for tdx_enc_status_changed()

[Dexuan Cui]

 > From: Borislav Petkov <bp@xxxxxxxxx>
> [...]
> On Mon, Jul 08, 2024 at 06:39:45PM +0000, Dexuan Cui wrote:
> > When a TDX guest runs on Hyper-V, the hv_netvsc driver's
> >  netvsc_init_buf()
> > allocates buffers using vzalloc(), and needs to share the buffers with the
> > host OS by calling set_memory_decrypted(), which is not working for
> > vmalloc() yet. Add the support by handling the pages one by one.
> 
> "Add support..." and the patch is cc:stable?

I meant to use "Cc: stable@xxxxxxxxxxxxxxx # 6.6+". 
Sorry for missing the "# 6.6+". 
 
> This looks like it is fixing something and considering how you're rushing
> this, I'd let this cook for a whole round and queue it after 6.11-rc1. So that
> it gets tested properly.

x86/tdx: Fix set_memory_decrypted() for vmalloc() buffers

When a TD mode Linux TDX VM runs on Hyper-V, the Linux hv_netvsc driver
needs to share a vmalloc()'d  buffer with the host OS: see
netvsc_init_buf() -> vmbus_establish_gpadl() -> ... ->
__vmbus_establish_gpadl() -> set_memory_decrypted().

Currently set_memory_decrypted() doesn't work for a vmalloc()'d  buffer
because tdx_enc_status_changed() uses __pa(vaddr), i.e., it assumes that
the 'vaddr' can't be from vmalloc(), and consequently hv_netvsc fails
to load.

Fix this by handling the pages one by one.

hv_netvsc is the first user of vmalloc() + set_memory_decrypted(), which
is why nobody noticed this until now.

v6.6 is a longterm kernel, which is used by some distros, so I hope
this patch can be in v6.6.y and newer, so it won't be carried out of tree.

I think the patch (without Kirill's kexec fix)  has been well tested, e.g.,
it has been in Ubuntu's linux-azure kernel for about 2 years. Kirill's 
kexec fix works in my testing and it looks safe to me. 

I hope this can be in 6.11-rc1 if you see no high risks. 
It's also fine to me if you decide to queue the patch after 6.11-rc1.

> > Co-developed-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
https://lwn.net/ml/linux-kernel/20230412151937.pxfyralfichwzyv6@box/

> > Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> > Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=e1b8ac3aae589bb57a2c2e49fa76235c687c4d23

> > Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
https://lwn.net/ml/linux-kernel/BYAPR21MB16885F59B6F5594F31AE957AD79A9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

> > Reviewed-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
https://lwn.net/ml/linux-kernel/d20baf1e-a736-667f-2082-0c0539013f2b@xxxxxxxxxxxxxxx/

> > Reviewed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
https://lwn.net/ml/linux-kernel/e8b1b0b5f32115c0ef8f1aeb0b805c4d9a953b31.camel@xxxxxxxxx/

> > Reviewed-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
https://lwn.net/ml/linux-kernel/4732ef96-9a47-3513-4494-48e4684d65cd@xxxxxxxxx/

> > Acked-by: Kai Huang <kai.huang@xxxxxxxxx>
https://lwn.net/ml/linux-kernel/6b6e7f943b7e28fa6ae6c77e1002ac61c41c1ee2.camel@xxxxxxxxx/

> When were you able to collect all those tags on a newly submitted patch?

This is not really a newly submitted patch :-)
Please refer to the links above.

v9 was posted here (Jun 2023): 
https://lwn.net/ml/linux-kernel/20230621191317.4129-3-decui@xxxxxxxxxxxxx/

v10 was posted here (Aug 2023):
https://lwn.net/ml/linux-kernel/20230811214826.9609-3-decui%40microsoft.com/

The last submission was May 2024:
https://lwn.net/ml/linux-kernel/20240521021238.1803-1-decui@xxxxxxxxxxxxx/
(Sorry, I should have made it clear that this is actually v11)

> Do you even know what the meaning of those tags is or you just slap them
> willy-nilly, just for fun?

The original patch was submitted in Nov 2022:
https://lwn.net/ml/linux-kernel/20221121195151.21812-4-decui@xxxxxxxxxxxxx/

I added Kirill's Co-developed-by in v4 (Apr 2023)
https://lwn.net/ml/linux-kernel/20230412151937.pxfyralfichwzyv6@box/
and added Kirill's Signed-off-by in v5, and added other people's Reviewed-by
and Acked-by over time. There are only minor changes since v4, so I think
it's appropriate to keep all the tags in the final commit.

> > Cc: stable@xxxxxxxxxxxxxxx
> 
> Why?
> 
> Fixes: what?

Please refer to my reply above. 

This is not to fix a buggy commit. The described scenario never worked before,
so I suppose a "Fixes:" tag is not needed.

> From reading this, it seems to me you need to brush up on 
> https://kernel.org/doc/html/latest/process/submitting-patches.html
Thanks for the link! I read it and did learn something.

> while waiting.
> 
> Thx.
> 
> --
> Regards/Gruss,
>     Boris.

I hope I have provided a satisfactory reply above.

How do you like the v12 below? It's also attached.
If this looks good to you, I can post it today or tomorrow.

Thanks,
Dexuan