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@xxxxxxx> > > > > --- > > > > > > > > Notes: > > > > Link: > > > > https://lore.kernel.org/20241212172711.1944927-1-yazen.ghannam@xxxxxxx > > > > 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
