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

From: Dexuan Cui
Date: Wed Jul 10 2024 - 03:48:39 EST


> From: Borislav Petkov <bp@xxxxxxxxx>
> Sent: Tuesday, July 9, 2024 4:07 AM
> To: Dexuan Cui <decui@xxxxxxxxxxxxx>
> > [...]
> > 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.
>
> So this is a corner-case thing. I guess CC:stable is ok, we have packported
> similar "fixes" in the past.

Thanks for sharing your thoughts! Then I'll use these in next version (v12):
Fixes: 68f2f2bc163d ("Drivers: hv: vmbus: Support fully enlightened TDX guests")
Cc: stable@xxxxxxxxxxxxxxx # 6.6+

> > 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.
>
> You seem to think that a patch which has been tested in some out-of-tree
> kernel,
>
> - gets modified
> - gets applied to the upstream kernel
> - it *breaks* a use case,
>
> and then it can still be considered tested.
>
> Are you seriously claiming that?!

I should have made it clear that I think Kirill helped fix and test this as well.
Besides Kirill's testing and my testing, I totally agree that more testing is
needed. I appreciate it very much if someone can help identify more
potential issues in the patch. I didn't mean to rush the patch.

> > 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.
>
> Yes, it will be after -rc1 because what you consider "tested" and what I do
> consider "tested" can just as well be from two different planets.

It's ok to me it will be after -rc1. I just thought the patch would get more
testing if it could be on some branch (e.g., x86/tdx ?) in the tip.git tree, e.g.,
if the patch is on some tip.git branch, I suppose the linux-next tree would
merge the patch so the patch will get more testing automatically.

> Since you'd go the length to quote the mail messages which gave you the
> tags but you will not read what I point you to, lemme read it for you:
>
> "Both Tested-by and Reviewed-by tags, once received on mailing list from
> tester or reviewer, should be added by author to the applicable patches
> when sending next versions. However if the patch has changed
> substantially in following version, these tags might not be applicable
> anymore and thus should be removed. Usually removal of someone's
> Tested-by or Reviewed-by tags should be mentioned in the patch
> changelog (after the '---' separator)."

I guess we have different options on whether "the patch has changed
substantially". My impression is that it hasn't. Please refer to the
changelogs of v9, v10 and v11:
https://lwn.net/ml/linux-kernel/20230621191317.4129-3-decui@xxxxxxxxxxxxx/
https://lwn.net/ml/linux-kernel/20230811214826.9609-3-decui%40microsoft.com/
https://lwn.net/ml/linux-kernel/20240521021238.1803-1-decui@xxxxxxxxxxxxx/
(v11 is basically a repost of v10)
I started to add people's tags since v4 and my impression is that since then
it's rebasing and minor changes. Anyway, I'll go through the history thoroughly
and document the changes in detail. I'll remove all the people's tags and
mention the removal in the changelog in next version (i.e., v12), and request
the people to review/ack again, and ask for Kirill's explicit permission for adding
the Co-developed-by and Signed-off-by.

> From Documentation/process/submitting-patches.rst
>
> Again, if you want to keep sending patches to the kernel, I'd strongly urge
> you to read that document!
Ok. I promise I'll read the document again, word by word.

> > This is not really a newly submitted patch :-)
>
> If you still think that and you want to keep your tags, all I can give you is
> a big fat NAK until you read and understand how the process works.
>
> Your decision.
>
> --
> Regards/Gruss,
> Boris.

Stay tuned, please. I'll try my best to make a good v12, which will have a
long changelog after the '---'.

Thanks,
Dexuan