On Wed, Jul 10, 2024 at 07:48:14AM +0000, Dexuan Cui wrote:
> 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.
Yes, it will get more testing automatically but the period is important: if
I rush it now, it goes to Linus next week and then any fallout it causes needs
to be dealt with in mainline.
If I queue it after -rc1, it'll be only in tip and linux-next for an
additional 7 week cycle and I can always whack it if it breaks something. If
it doesn't, I can send it mainline in the 6.12 merge window.
But we won't have to revert it mainline.
See the difference?
> I guess we have different options on whether "the patch has changed
> substantially". My impression is that it hasn't.
If you're calling the difference between what I reverted and what you're
sending now unsubstantial:
--- /tmp/old 2024-07-10 10:03:20.016629439 +0200
+++ /tmp/new 2024-07-10 10:02:23.696872729 +0200
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
-index c1cb90369915..abf3cd591afd 100644
+index 078e2bac25531..8f471260924f7 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
-@@ -7,6 +7,7 @@
- #include <linux/cpufeature.h>
+@@ -8,6 +8,7 @@
#include <linux/export.h>
#include <linux/io.h>
+ #include <linux/kexec.h>
+#include <linux/mm.h>
#include <asm/coco.h>
#include <asm/tdx.h>
#include <asm/vmx.h>
-@@ -778,6 +779,19 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
+@@ -782,6 +783,19 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
return false;
}
@@ -53,7 +86,7 @@ index c1cb90369915..abf3cd591afd 100644
/*
* Inform the VMM of the guest's intent for this physical page: shared with
* the VMM or private to the guest. The VMM is expected to change its mapping
-@@ -785,15 +799,22 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
+@@ -789,15 +803,30 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
*/
static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
{
@@ -63,23 +96,34 @@ index c1cb90369915..abf3cd591afd 100644
+ unsigned long end = start + numpages * PAGE_SIZE;
+ unsigned long step = end - start;
+ unsigned long addr;
-
-- if (!tdx_map_gpa(start, end, enc))
-- return false;
++
+ /* Step through page-by-page for vmalloc() mappings */
+ if (is_vmalloc_addr((void *)vaddr))
+ step = PAGE_SIZE;
++
++ for (addr = start; addr < end; addr += step) {
++ phys_addr_t start_pa;
++ phys_addr_t end_pa;
++
++ /* The check fails on vmalloc() mappings */
++ if (virt_addr_valid(addr))
++ start_pa = __pa(addr);
++ else
++ start_pa = slow_virt_to_phys((void *)addr);
+
+- if (!tdx_map_gpa(start, end, enc))
+- return false;
++ end_pa = start_pa + step;
- /* shared->private conversion requires memory to be accepted before use */
- if (enc)
- return tdx_accept_memory(start, end);
-+ for (addr = start; addr < end; addr += step) {
-+ phys_addr_t start_pa = slow_virt_to_phys((void *)addr);
-+ phys_addr_t end_pa = start_pa + step;
-+
+ if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
+ return false;
+ }
return true;
}
especially for a patch which is already known to break things and where we're
especially careful, then yes, we strongly disagree here.
So yes, it will definitely not go in now.
> I started to add people's tags since v4 and my impression is that since then
> it's rebasing and minor changes.
When version N introduces changes like above in what is already non-trivial
code, you drop all tags. And if people want to review it again, then they
should give you those R-by tags.
Also, think about it: your patch broke a use case. How much are those R-by
tags worth if the patch is broken? And why do you want to hold on to them so
badly?
If a patch needs to be reverted because it breaks a use case, all reviewed and
acked tags should simply be removed too. It is that simple.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
