All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/mm: Consolidate SME comments and use break
@ 2026-06-09 11:30 Thorsten Blum
  2026-06-09 14:23 ` Dave Hansen
  0 siblings, 1 reply; 4+ messages in thread
From: Thorsten Blum @ 2026-06-09 11:30 UTC (permalink / raw)
  To: Dave Hansen, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
  Cc: Thorsten Blum, linux-kernel

Combine the two SME comments and use break instead of return to exit the
switch consistently with the other cases.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/x86/mm/mem_encrypt.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 95bae74fdab2..c6bab01a5db8 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -53,14 +53,13 @@ static void print_mem_encrypt_feature_info(void)
 	case CC_VENDOR_AMD:
 		pr_cont("AMD");
 
-		/* Secure Memory Encryption */
-		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
 		/*
-		 * SME is mutually exclusive with any of the SEV
-		 * features below.
-		*/
+		 * Secure Memory Encryption is mutually exclusive with
+		 * any of the SEV features below.
+		 */
+		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
 			pr_cont(" SME\n");
-			return;
+			break;
 		}
 
 		/* Secure Encrypted Virtualization */

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/mm: Consolidate SME comments and use break
  2026-06-09 11:30 [PATCH] x86/mm: Consolidate SME comments and use break Thorsten Blum
@ 2026-06-09 14:23 ` Dave Hansen
  2026-06-09 15:32   ` Thorsten Blum
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Hansen @ 2026-06-09 14:23 UTC (permalink / raw)
  To: Thorsten Blum, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin
  Cc: linux-kernel

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/mm: Consolidate SME comments and use break
  2026-06-09 14:23 ` Dave Hansen
@ 2026-06-09 15:32   ` Thorsten Blum
  2026-06-09 16:26     ` Dave Hansen
  0 siblings, 1 reply; 4+ messages in thread
From: Thorsten Blum @ 2026-06-09 15:32 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Dave Hansen, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, linux-kernel

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.

Happy to resend it early next cycle unless you prefer that I drop it.

Thanks,
Thorsten

^ permalink raw reply	[flat|nested] 4+ messages in thread

* 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

end of thread, other threads:[~2026-06-09 16:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 11:30 [PATCH] x86/mm: Consolidate SME comments and use break Thorsten Blum
2026-06-09 14:23 ` Dave Hansen
2026-06-09 15:32   ` Thorsten Blum
2026-06-09 16:26     ` Dave Hansen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.