| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250102200411.GA230071@yaz-khff2.amd.com>
Date: Thu, 2 Jan 2025 15:04:11 -0500
From: Yazen Ghannam <yazen.ghannam@....com>
To: Suma Hegde <Suma.Hegde@....com>
Cc: Borislav Petkov <bp@...en8.de>, x86@...nel.org, tony.luck@...el.com,
mario.limonciello@....com, bhelgaas@...gle.com, jdelvare@...e.com,
linux@...ck-us.net, clemens@...isch.de, Shyam-sundar.S-k@....com,
hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com,
naveenkrishna.chatradhi@....com, linux-kernel@...r.kernel.org,
linux-edac@...r.kernel.org, linux-pci@...r.kernel.org,
linux-hwmon@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v2.2] x86/amd_node, platform/x86/amd/hsmp: Have HSMP use
SMN through AMD_NODE
On Tue, Dec 24, 2024 at 06:44:17AM +0530, Suma Hegde wrote:
> Hi Yazen,
>
>
> On 12/17/2024 12:03 AM, Yazen Ghannam wrote:
> > On Sat, Dec 14, 2024 at 11:05:07AM +0100, Borislav Petkov wrote:
> > > On Fri, Dec 13, 2024 at 03:22:06PM +0000, Yazen Ghannam wrote:
> > > > The HSMP interface is just an SMN interface with different offsets.
> > > >
> > > > Define an HSMP wrapper in the SMN code and have the HSMP platform driver
> > > > use that rather than a local solution.
> > > >
> > > > Also, remove the "root" member from AMD_NB, since there are no more
> > > > users of it.
> > > >
> > > > Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> > > > ---
> > > >
> > > > Notes:
> > > > Link:
> > > > https://lore.kernel.org/20241212172711.1944927-1-yazen.ghannam@amd.com
> > > > v2.1-v2.2:
> > > > * Include <linux/build_bug.h> for static_assert()
> > > > v2->v2.1:
> > > > * Include static_assert() and comment for sysfs attributes.
> > > > v1->v2:
> > > > * Rebase on recent HSMP rework.
> > > >
> > > > arch/x86/include/asm/amd_nb.h | 1 -
> > > > arch/x86/include/asm/amd_node.h | 3 +++
> > > > arch/x86/kernel/amd_nb.c | 1 -
> > > > arch/x86/kernel/amd_node.c | 9 +++++++
> > > > drivers/platform/x86/amd/hsmp/Kconfig | 2 +-
> > > > drivers/platform/x86/amd/hsmp/acpi.c | 7 +++---
> > > > drivers/platform/x86/amd/hsmp/hsmp.c | 1 -
> > > > drivers/platform/x86/amd/hsmp/hsmp.h | 3 ---
> > > > drivers/platform/x86/amd/hsmp/plat.c | 36 +++++++++------------------
> > > > 9 files changed, 29 insertions(+), 34 deletions(-)
> > > ld: drivers/platform/x86/amd/hsmp/plat.o: in function `amd_hsmp_pci_rdwr':
> > > /home/amd/kernel/linux/drivers/platform/x86/amd/hsmp/plat.c:44: undefined reference to `amd_smn_hsmp_rdwr'
> > > make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1
> > > make[1]: *** [/home/amd/bpetkov/kernel/linux/Makefile:1225: vmlinux] Error 2
> > > make[1]: *** Waiting for unfinished jobs....
> > > make: *** [Makefile:251: __sub-make] Error 2
> > >
> > > Config attached.
> > >
> > I think the fix is this:
> >
> > diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
> > index d6f7a62d55b5..fc90ef11a8ad 100644
> > --- a/drivers/platform/x86/amd/hsmp/Kconfig
> > +++ b/drivers/platform/x86/amd/hsmp/Kconfig
> > @@ -7,7 +7,6 @@ config AMD_HSMP
> > tristate
> > menu "AMD HSMP Driver"
> > - depends on AMD_NODE || COMPILE_TEST
> > config AMD_HSMP_ACPI
> > tristate "AMD HSMP ACPI device driver"
> > @@ -29,6 +28,7 @@ config AMD_HSMP_ACPI
> > config AMD_HSMP_PLAT
> > tristate "AMD HSMP platform device driver"
> > + depends on AMD_NODE
> > select AMD_HSMP
> > help
> > Host System Management Port (HSMP) interface is a mailbox interface
> >
> > With all the recent rework, only AMD_HSMP_PLAT has a hard dependency on
> > AMD_NODE due to needing the SMN register access interface.
> >
> > Various HSMP files still pull in <asm/amd_node.h>, but they just need
> > defines and an inline function.
> >
> > It seems COMPILE_TEST is not needed, since this change allows
> > AMD_HSMP_ACPI to build without AMD_NODE which seems like the intent.
>
> COMPILE_TEST was added to provide build coverage to HSMP based on Ilpo's
> suggestion.
>
> We can probably add #ifdef CONFIG_AMD_NODE and #else as below to fix this
> instead of removing COMPILE_TEST?
>
> #ifdef CONFIG_AMD_NODE
> /* Should only be used by the HSMP driver. */
> int __must_check amd_smn_hsmp_rdwr(u16 node, u32 address, u32 *value, bool
> write);
> #else
> static int amd_smn_hsmp_rdwr(u16 node, u32 address, u32 *value, bool write);
> {
> return 0;
> }
>
> #endif
>
> May be Ilpo can comment more.
>
Thanks Suma for the feedback.
If the intent is to increase build coverage, then I think AMD_HSMP_ACPI
also needs COMPILE_TEST.
We could do the following in addition to the #ifdef.
---
drivers/platform/x86/amd/hsmp/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
index d6f7a62d55b5..b60933806eed 100644
--- a/drivers/platform/x86/amd/hsmp/Kconfig
+++ b/drivers/platform/x86/amd/hsmp/Kconfig
@@ -7,11 +7,10 @@ config AMD_HSMP
tristate
menu "AMD HSMP Driver"
- depends on AMD_NODE || COMPILE_TEST
config AMD_HSMP_ACPI
tristate "AMD HSMP ACPI device driver"
- depends on ACPI
+ depends on ACPI || COMPILE_TEST
select AMD_HSMP
help
Host System Management Port (HSMP) interface is a mailbox interface
@@ -29,6 +28,7 @@ config AMD_HSMP_ACPI
config AMD_HSMP_PLAT
tristate "AMD HSMP platform device driver"
+ depends on AMD_NODE || COMPILE_TEST
select AMD_HSMP
help
Host System Management Port (HSMP) interface is a mailbox interface
--
The scope of AMD_NODE is reduced to only what needs it. And each option
can be independently built to increase coverage.
How does this sound?
Thanks,
Yazen
Powered by blists - more mailing lists