* Re: [PATCH] x86/mm: Consolidate SME comments and use break
2026-06-09 15:32 ` Thorsten Blum
@ 2026-06-09 16:26 ` Dave Hansen
0 siblings, 0 replies; 4+ messages in thread
From: Dave Hansen @ 2026-06-09 16:26 UTC (permalink / raw)
To: Thorsten Blum
Cc: Dave Hansen, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]
On 6/9/26 08:32, Thorsten Blum wrote:
> On Tue, Jun 09, 2026 at 07:23:14AM -0700, Dave Hansen wrote:
>> On 6/9/26 04:30, Thorsten Blum wrote:
>>> Combine the two SME comments and use break instead of return to exit the
>>> switch consistently with the other cases.
>> Hey Thorsten,
>>
>> While this arguably makes the code a wee bit more consistent, it does it
>> at the cost of churn. Churn can introduce bugs. It creates conflicts
>> with other work and it muddies the git history. I don't think the
>> risk/return on this kind of thing is worth it.
>>
>> Also, for pure cleanups, I'd personally appreciate seeing them
>> concentrated really close after -rc1.
> I initially just wanted to fix the broken indentation/formatting here:
>
> if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> /*
> * SME is mutually exclusive with any of the SEV
> * features below.
> */
> pr_cont(" SME\n");
> return;
> }
>
> And then decided to combine both comments and replace return with break
> while at it.
Yeah, you saw the loose thread and kept pulling. ;)
There are tons of things in the tree that are ugly or don't meet modern
standards. If they're not a hindrance to development, I tend to just
grumble and leave them alone. Otherwise, all I would be doing all day is
fixing these things up. If I took every patch to fix them up, all I'd be
doing all day was merging patches to fix them up.
I actually spend a time regularly doing archaeology to figure out how
code came to be the way it is. More than half of that time is spent
tracking through "cleanups" that don't help me get the answers I seek.
If the code is ever moved or refactored or changed, I'm all for fixing
these. But that's a privilege folks earn doing a series that fixes bugs
or refactors code to prepare for a new feature.
Oh, and there's also the problem of:
❯ find me three coding style problems in arch/x86 and commit
patches to fix them
being able to produce streams of patches like I've attached.
[-- Attachment #2: 0001-x86-xen-remove-trailing-whitespace-on-include-line.patch --]
[-- Type: text/x-patch, Size: 892 bytes --]
From c38cb571e9f47785830329787082ad2bfc40b79b Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave@sr71.net>
Date: Tue, 9 Jun 2026 09:21:53 -0700
Subject: [PATCH 1/3] x86/xen: remove trailing whitespace on #include line
A stray space at the end of the #include <asm/e820/api.h> line trips
checkpatch's "trailing whitespace" ERROR. No functional change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
arch/x86/xen/enlighten.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 23b91bf9b663..8fb2a79ea0cc 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -17,7 +17,7 @@
#include <asm/xen/hypercall.h>
#include <asm/xen/hypervisor.h>
#include <asm/cpu.h>
-#include <asm/e820/api.h>
+#include <asm/e820/api.h>
#include <asm/setup.h>
#include "xen-ops.h"
--
2.34.1
[-- Attachment #3: 0003-x86-lib-iomem-add-spaces-around-in-string_memcpy_-fr.patch --]
[-- Type: text/x-patch, Size: 1379 bytes --]
From 1ec0b689b58d1864a49d8e8d45944f50bd405608 Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave@sr71.net>
Date: Tue, 9 Jun 2026 09:22:36 -0700
Subject: [PATCH 3/3] x86/lib/iomem: add spaces around '-=' in
string_memcpy_{from,to}io()
Both string_memcpy_fromio() and string_memcpy_toio() decrement the
remaining byte count after a 2-byte movs with 'n-=2;'. checkpatch
flags the missing spaces:
ERROR: spaces required around that '-=' (ctx:VxV)
Insert the conventional spaces. No functional change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
arch/x86/lib/iomem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/iomem.c b/arch/x86/lib/iomem.c
index c20e04764edc..f6acdafd4802 100644
--- a/arch/x86/lib/iomem.c
+++ b/arch/x86/lib/iomem.c
@@ -38,7 +38,7 @@ static void string_memcpy_fromio(void *to, const volatile void __iomem *from, si
}
if (n > 1 && unlikely(2 & (unsigned long)from)) {
movs("w", to, from);
- n-=2;
+ n -= 2;
}
rep_movs(to, (const void *)from, n);
/* KMSAN must treat values read from devices as initialized. */
@@ -59,7 +59,7 @@ static void string_memcpy_toio(volatile void __iomem *to, const void *from, size
}
if (n > 1 && unlikely(2 & (unsigned long)to)) {
movs("w", to, from);
- n-=2;
+ n -= 2;
}
rep_movs((void *)to, (const void *) from, n);
}
--
2.34.1
[-- Attachment #4: 0002-x86-pci-add-missing-space-after-for-keyword-in-pci_f.patch --]
[-- Type: text/x-patch, Size: 1431 bytes --]
From 9512e62397233e2646853f0cca2e5b590f108a5a Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave@sr71.net>
Date: Tue, 9 Jun 2026 09:22:12 -0700
Subject: [PATCH 2/3] x86/pci: add missing space after 'for' keyword in
pci_fixup_i450nx() and pci_fixup_umc_ide()
Two for-loops in arch/x86/pci/fixup.c were written as 'for(' instead of
'for (', which checkpatch flags as:
ERROR: space required before the open parenthesis '('
No functional change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
arch/x86/pci/fixup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b301c6c8df75..0ee90b224f99 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -23,7 +23,7 @@ static void pci_fixup_i450nx(struct pci_dev *d)
dev_warn(&d->dev, "Searching for i450NX host bridges\n");
reg = 0xd0;
- for(pxb = 0; pxb < 2; pxb++) {
+ for (pxb = 0; pxb < 2; pxb++) {
pci_read_config_byte(d, reg++, &busno);
pci_read_config_byte(d, reg++, &suba);
pci_read_config_byte(d, reg++, &subb);
@@ -61,7 +61,7 @@ static void pci_fixup_umc_ide(struct pci_dev *d)
int i;
dev_warn(&d->dev, "Fixing base address flags\n");
- for(i = 0; i < 4; i++)
+ for (i = 0; i < 4; i++)
d->resource[i].flags |= PCI_BASE_ADDRESS_SPACE_IO;
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_UMC, PCI_DEVICE_ID_UMC_UM8886BF, pci_fixup_umc_ide);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread