diff mbox series

riscv: dmi: Add SMBIOS/DMI support

Message ID 20240307083154.346542-1-haibo1.xu@intel.com
State Superseded
Headers show
Series riscv: dmi: Add SMBIOS/DMI support | expand

Commit Message

Haibo Xu March 7, 2024, 8:31 a.m. UTC
Enable the dmi driver for riscv which would allow access the
SMBIOS info through some userspace file(/sys/firmware/dmi/*).

The change was based on that of arm64 and has been verified
by dmidecode tool.

Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
---
 arch/riscv/Kconfig                   | 11 +++++++++++
 arch/riscv/include/asm/dmi.h         | 29 ++++++++++++++++++++++++++++
 drivers/firmware/efi/riscv-runtime.c | 13 +++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/riscv/include/asm/dmi.h

Comments

Ard Biesheuvel March 7, 2024, 11 a.m. UTC | #1
Hello Haibo,

Some notes below.

On Thu, 7 Mar 2024 at 09:18, Haibo Xu <haibo1.xu@intel.com> wrote:
>
> Enable the dmi driver for riscv which would allow access the
> SMBIOS info through some userspace file(/sys/firmware/dmi/*).
>
> The change was based on that of arm64 and has been verified
> by dmidecode tool.
>
> Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
> ---
>  arch/riscv/Kconfig                   | 11 +++++++++++
>  arch/riscv/include/asm/dmi.h         | 29 ++++++++++++++++++++++++++++
>  drivers/firmware/efi/riscv-runtime.c | 13 +++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 arch/riscv/include/asm/dmi.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 0bfcfec67ed5..a123a3e7e5f3 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -918,6 +918,17 @@ config EFI
>           allow the kernel to be booted as an EFI application. This
>           is only useful on systems that have UEFI firmware.
>
> +config DMI
> +       bool "Enable support for SMBIOS (DMI) tables"
> +       depends on EFI
> +       default y
> +       help
> +         This enables SMBIOS/DMI feature for systems.
> +
> +         This option is only useful on systems that have UEFI firmware.
> +         However, even with this option, the resultant kernel should
> +         continue to boot on existing non-UEFI platforms.
> +
>  config CC_HAVE_STACKPROTECTOR_TLS
>         def_bool $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=tp -mstack-protector-guard-offset=0)
>
> diff --git a/arch/riscv/include/asm/dmi.h b/arch/riscv/include/asm/dmi.h
> new file mode 100644
> index 000000000000..a861043f02dc
> --- /dev/null
> +++ b/arch/riscv/include/asm/dmi.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024 Intel Corporation
> + *
> + * based on arch/arm64/include/asm/dmi.h
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#ifndef __ASM_DMI_H
> +#define __ASM_DMI_H
> +
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +
> +/*
> + * According to section 2.3.6 of the UEFI spec, the firmware should not
> + * request a virtual mapping for configuration tables such as SMBIOS.
> + * This means we have to map them before use.
> + */

You can drop this comment, it is not really accurate.

'Requesting a virtual mapping' means the memory is mapped by the OS
into the EFI page tables before calling a runtime service, so that the
firmware (which runs under the OS's memory translation regime) can
access the contents.

SMBIOS tables are informational and for consumption by the OS only,
not by the runtime service implementations themselves, and so they can
be omitted from the EFI runtime page tables.


> +#define dmi_early_remap(x, l)          ioremap_prot(x, l, _PAGE_KERNEL)
> +#define dmi_early_unmap(x, l)          iounmap(x)
> +#define dmi_remap(x, l)                        ioremap_prot(x, l, _PAGE_KERNEL)
> +#define dmi_unmap(x)                   iounmap(x)

Why not use memremap() here? That will reuse the linear map if it
happens to already cover the region.

> +#define dmi_alloc(l)                   kzalloc(l, GFP_KERNEL)
> +
> +#endif
> diff --git a/drivers/firmware/efi/riscv-runtime.c b/drivers/firmware/efi/riscv-runtime.c
> index 09525fb5c240..c3bfb9e77e02 100644
> --- a/drivers/firmware/efi/riscv-runtime.c
> +++ b/drivers/firmware/efi/riscv-runtime.c
> @@ -152,3 +152,16 @@ void arch_efi_call_virt_teardown(void)
>  {
>         efi_virtmap_unload();
>  }
> +
> +static int __init riscv_dmi_init(void)
> +{
> +       /*
> +        * On riscv, DMI depends on UEFI, and dmi_setup() needs to
> +        * be called early because dmi_id_init(), which is an arch_initcall
> +        * itself, depends on dmi_scan_machine() having been called already.
> +        */
> +       dmi_setup();
> +
> +       return 0;
> +}
> +core_initcall(riscv_dmi_init);
> --
> 2.34.1
>
>
Haibo Xu March 8, 2024, 7:38 a.m. UTC | #2
On Thu, Mar 7, 2024 at 7:00 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Hello Haibo,
>
> Some notes below.
>
> On Thu, 7 Mar 2024 at 09:18, Haibo Xu <haibo1.xu@intel.com> wrote:
> >
> > Enable the dmi driver for riscv which would allow access the
> > SMBIOS info through some userspace file(/sys/firmware/dmi/*).
> >
> > The change was based on that of arm64 and has been verified
> > by dmidecode tool.
> >
> > Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
> > ---
> >  arch/riscv/Kconfig                   | 11 +++++++++++
> >  arch/riscv/include/asm/dmi.h         | 29 ++++++++++++++++++++++++++++
> >  drivers/firmware/efi/riscv-runtime.c | 13 +++++++++++++
> >  3 files changed, 53 insertions(+)
> >  create mode 100644 arch/riscv/include/asm/dmi.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 0bfcfec67ed5..a123a3e7e5f3 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -918,6 +918,17 @@ config EFI
> >           allow the kernel to be booted as an EFI application. This
> >           is only useful on systems that have UEFI firmware.
> >
> > +config DMI
> > +       bool "Enable support for SMBIOS (DMI) tables"
> > +       depends on EFI
> > +       default y
> > +       help
> > +         This enables SMBIOS/DMI feature for systems.
> > +
> > +         This option is only useful on systems that have UEFI firmware.
> > +         However, even with this option, the resultant kernel should
> > +         continue to boot on existing non-UEFI platforms.
> > +
> >  config CC_HAVE_STACKPROTECTOR_TLS
> >         def_bool $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=tp -mstack-protector-guard-offset=0)
> >
> > diff --git a/arch/riscv/include/asm/dmi.h b/arch/riscv/include/asm/dmi.h
> > new file mode 100644
> > index 000000000000..a861043f02dc
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/dmi.h
> > @@ -0,0 +1,29 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2024 Intel Corporation
> > + *
> > + * based on arch/arm64/include/asm/dmi.h
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + */
> > +
> > +#ifndef __ASM_DMI_H
> > +#define __ASM_DMI_H
> > +
> > +#include <linux/io.h>
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * According to section 2.3.6 of the UEFI spec, the firmware should not
> > + * request a virtual mapping for configuration tables such as SMBIOS.
> > + * This means we have to map them before use.
> > + */
>
> You can drop this comment, it is not really accurate.
>
> 'Requesting a virtual mapping' means the memory is mapped by the OS
> into the EFI page tables before calling a runtime service, so that the
> firmware (which runs under the OS's memory translation regime) can
> access the contents.
>
> SMBIOS tables are informational and for consumption by the OS only,
> not by the runtime service implementations themselves, and so they can
> be omitted from the EFI runtime page tables.
>

Sure. Thanks for elaborating on it and it's very helpful!

> > +#define dmi_early_remap(x, l)          ioremap_prot(x, l, _PAGE_KERNEL)
> > +#define dmi_early_unmap(x, l)          iounmap(x)
> > +#define dmi_remap(x, l)                        ioremap_prot(x, l, _PAGE_KERNEL)
> > +#define dmi_unmap(x)                   iounmap(x)
>
> Why not use memremap() here? That will reuse the linear map if it
> happens to already cover the region.
>

Yes, memremap() is better here. Will update it in v2.
Thank you for the review!

> > +#define dmi_alloc(l)                   kzalloc(l, GFP_KERNEL)
> > +
> > +#endif
> > diff --git a/drivers/firmware/efi/riscv-runtime.c b/drivers/firmware/efi/riscv-runtime.c
> > index 09525fb5c240..c3bfb9e77e02 100644
> > --- a/drivers/firmware/efi/riscv-runtime.c
> > +++ b/drivers/firmware/efi/riscv-runtime.c
> > @@ -152,3 +152,16 @@ void arch_efi_call_virt_teardown(void)
> >  {
> >         efi_virtmap_unload();
> >  }
> > +
> > +static int __init riscv_dmi_init(void)
> > +{
> > +       /*
> > +        * On riscv, DMI depends on UEFI, and dmi_setup() needs to
> > +        * be called early because dmi_id_init(), which is an arch_initcall
> > +        * itself, depends on dmi_scan_machine() having been called already.
> > +        */
> > +       dmi_setup();
> > +
> > +       return 0;
> > +}
> > +core_initcall(riscv_dmi_init);
> > --
> > 2.34.1
> >
> >
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 0bfcfec67ed5..a123a3e7e5f3 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -918,6 +918,17 @@  config EFI
 	  allow the kernel to be booted as an EFI application. This
 	  is only useful on systems that have UEFI firmware.
 
+config DMI
+	bool "Enable support for SMBIOS (DMI) tables"
+	depends on EFI
+	default y
+	help
+	  This enables SMBIOS/DMI feature for systems.
+
+	  This option is only useful on systems that have UEFI firmware.
+	  However, even with this option, the resultant kernel should
+	  continue to boot on existing non-UEFI platforms.
+
 config CC_HAVE_STACKPROTECTOR_TLS
 	def_bool $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=tp -mstack-protector-guard-offset=0)
 
diff --git a/arch/riscv/include/asm/dmi.h b/arch/riscv/include/asm/dmi.h
new file mode 100644
index 000000000000..a861043f02dc
--- /dev/null
+++ b/arch/riscv/include/asm/dmi.h
@@ -0,0 +1,29 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 Intel Corporation
+ *
+ * based on arch/arm64/include/asm/dmi.h
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#ifndef __ASM_DMI_H
+#define __ASM_DMI_H
+
+#include <linux/io.h>
+#include <linux/slab.h>
+
+/*
+ * According to section 2.3.6 of the UEFI spec, the firmware should not
+ * request a virtual mapping for configuration tables such as SMBIOS.
+ * This means we have to map them before use.
+ */
+#define dmi_early_remap(x, l)		ioremap_prot(x, l, _PAGE_KERNEL)
+#define dmi_early_unmap(x, l)		iounmap(x)
+#define dmi_remap(x, l)			ioremap_prot(x, l, _PAGE_KERNEL)
+#define dmi_unmap(x)			iounmap(x)
+#define dmi_alloc(l)			kzalloc(l, GFP_KERNEL)
+
+#endif
diff --git a/drivers/firmware/efi/riscv-runtime.c b/drivers/firmware/efi/riscv-runtime.c
index 09525fb5c240..c3bfb9e77e02 100644
--- a/drivers/firmware/efi/riscv-runtime.c
+++ b/drivers/firmware/efi/riscv-runtime.c
@@ -152,3 +152,16 @@  void arch_efi_call_virt_teardown(void)
 {
 	efi_virtmap_unload();
 }
+
+static int __init riscv_dmi_init(void)
+{
+	/*
+	 * On riscv, DMI depends on UEFI, and dmi_setup() needs to
+	 * be called early because dmi_id_init(), which is an arch_initcall
+	 * itself, depends on dmi_scan_machine() having been called already.
+	 */
+	dmi_setup();
+
+	return 0;
+}
+core_initcall(riscv_dmi_init);