coreboot exposes (S)MM related data in the coreboot table. Extends existing interface,
with structure corresponding to (S)MM data, and adds parser. Parser exposes this data
to the modules executed later.
Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
---
drivers/firmware/google/Kconfig | 12 +++++
drivers/firmware/google/Makefile | 3 ++
drivers/firmware/google/coreboot_table.h | 34 ++++++++-----
drivers/firmware/google/mm_info.c | 63 ++++++++++++++++++++++++
drivers/firmware/google/mm_payload.h | 58 ++++++++++++++++++++++
5 files changed, 158 insertions(+), 12 deletions(-)
create mode 100644 drivers/firmware/google/mm_info.c
create mode 100644 drivers/firmware/google/mm_payload.h
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 41b78f5cb735..5d918c076f25 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -81,4 +81,16 @@ config GOOGLE_VPD
This option enables the kernel to expose the content of Google VPD
under /sys/firmware/vpd.
+config COREBOOT_PAYLOAD_MM
+ tristate "SMI handling in Linux (LinuxBootSMM)"
+ depends on GOOGLE_COREBOOT_TABLE
+ help
+ Enables support for SMI handling by Linux-owned code.
+ coreboot reserves region for payload-owned SMI handler, the Linux
+ driver prepares its SMI handler outside of SMRAM, and lets coreboot
+ know where the handler is placed by issuing an SMI. On this SMI, the
+ handler is being placed in SMRAM and all supported SMIs from that point
+ on are handled by Linux-owned SMI handler.
+ If in doubt, say N.
+
endif # GOOGLE_FIRMWARE
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 8151e323cc43..d2a690e8379d 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -12,3 +12,6 @@ obj-$(CONFIG_GOOGLE_CBMEM) += cbmem.o
vpd-sysfs-y := vpd.o vpd_decode.o
obj-$(CONFIG_GOOGLE_VPD) += vpd-sysfs.o
+
+# LinuxBootSMM related.
+obj-$(CONFIG_COREBOOT_PAYLOAD_MM) += mm_info.o
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index bb6f0f7299b4..e0b087933c5a 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -41,7 +41,6 @@ struct lb_cbmem_ref {
};
#define LB_TAG_CBMEM_ENTRY 0x31
-
/* Corresponds to LB_TAG_CBMEM_ENTRY */
struct lb_cbmem_entry {
u32 tag;
@@ -52,6 +51,16 @@ struct lb_cbmem_entry {
u32 id;
};
+/* Corresponds to LB_TAG_PLD_MM_INTERFACE_INFO */
+#define LB_TAG_PLD_MM_INTERFACE_INFO 0x3b
+struct lb_pld_mm_interface_info {
+ u32 tag;
+ u32 size;
+ u8 revision;
+ u8 requires_long_mode_call;
+ u8 register_mm_entry_command;
+};
+
/* Describes framebuffer setup by coreboot */
struct lb_framebuffer {
u32 tag;
@@ -61,15 +70,15 @@ struct lb_framebuffer {
u32 x_resolution;
u32 y_resolution;
u32 bytes_per_line;
- u8 bits_per_pixel;
- u8 red_mask_pos;
- u8 red_mask_size;
- u8 green_mask_pos;
- u8 green_mask_size;
- u8 blue_mask_pos;
- u8 blue_mask_size;
- u8 reserved_mask_pos;
- u8 reserved_mask_size;
+ u8 bits_per_pixel;
+ u8 red_mask_pos;
+ u8 red_mask_size;
+ u8 green_mask_pos;
+ u8 green_mask_size;
+ u8 blue_mask_pos;
+ u8 blue_mask_size;
+ u8 reserved_mask_pos;
+ u8 reserved_mask_size;
};
/* A device, additionally with information from coreboot. */
@@ -80,6 +89,7 @@ struct coreboot_device {
struct lb_cbmem_ref cbmem_ref;
struct lb_cbmem_entry cbmem_entry;
struct lb_framebuffer framebuffer;
+ struct lb_pld_mm_interface_info mm_info;
DECLARE_FLEX_ARRAY(u8, raw);
};
};
@@ -112,8 +122,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
* boilerplate. Each module may only use this macro once, and
* calling it replaces module_init() and module_exit()
*/
-#define module_coreboot_driver(__coreboot_driver) \
+#define module_coreboot_driver(__coreboot_driver) \
module_driver(__coreboot_driver, coreboot_driver_register, \
- coreboot_driver_unregister)
+ coreboot_driver_unregister)
#endif /* __COREBOOT_TABLE_H */
diff --git a/drivers/firmware/google/mm_info.c b/drivers/firmware/google/mm_info.c
new file mode 100644
index 000000000000..55bcdc8b8d53
--- /dev/null
+++ b/drivers/firmware/google/mm_info.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * mm_info.c
+ *
+ * Driver for exporting MM payload information from coreboot table.
+ *
+ * Copyright 2025 9elements gmbh
+ * Copyright 2025 Michal Gorlas <michal.gorlas@9elements.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "coreboot_table.h"
+#include "mm_payload.h"
+
+static struct lb_pld_mm_interface_info *mm_cbtable_info;
+struct mm_info *mm_info;
+
+static int mm_driver_probe(struct coreboot_device *dev)
+{
+ mm_cbtable_info = &dev->mm_info;
+ if (mm_cbtable_info->tag != LB_TAG_PLD_MM_INTERFACE_INFO)
+ return -ENXIO;
+
+ mm_info = kmalloc(sizeof(*mm_info), GFP_KERNEL);
+ mm_info->revision = mm_cbtable_info->revision;
+ mm_info->requires_long_mode_call =
+ mm_cbtable_info->requires_long_mode_call;
+ mm_info->register_mm_entry_command =
+ mm_cbtable_info->register_mm_entry_command;
+ return 0;
+}
+EXPORT_SYMBOL(mm_info);
+
+static void mm_driver_remove(struct coreboot_device *dev)
+{
+ if (mm_info)
+ kfree(mm_info);
+}
+
+static const struct coreboot_device_id mm_info_ids[] = {
+ { .tag = LB_TAG_PLD_MM_INTERFACE_INFO },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(coreboot, mm_info_ids);
+
+static struct coreboot_driver mm_driver = {
+ .probe = mm_driver_probe,
+ .remove = mm_driver_remove,
+ .drv = {
+ .name = "mm_info",
+ },
+ .id_table = mm_info_ids,
+};
+
+module_coreboot_driver(mm_driver);
+
+MODULE_AUTHOR("Michal Gorlas <michal.gorlas@9elements.com>");
+MODULE_DESCRIPTION("Driver for exporting MM info from coreboot table");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/firmware/google/mm_payload.h b/drivers/firmware/google/mm_payload.h
new file mode 100644
index 000000000000..bb2f55c4f240
--- /dev/null
+++ b/drivers/firmware/google/mm_payload.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * mm_payload.h
+ *
+ * Internal header for MM payload driver.
+ *
+ * Copyright 2025 9elements gmbh
+ * Copyright 2025 Michal Gorlas <michal.gorlas@9elements.com>
+ */
+
+#ifndef __MM_PAYLOAD_H
+#define __MM_PAYLOAD_H
+
+#define PAYLOAD_MM_RET_SUCCESS 0
+#define PAYLOAD_MM_RET_FAILURE 1
+#define PAYLOAD_MM_REGISTER_ENTRY 2
+
+#define REALMODE_END_SIGNATURE 0x65a22c82
+
+struct mm_info {
+ u8 revision;
+ u8 requires_long_mode_call;
+ u8 register_mm_entry_command;
+};
+
+extern struct mm_info *mm_info;
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/* This must match data at mm_handler/mm_header.S */
+struct mm_header {
+ u32 text_start;
+ u32 mm_entry_32;
+ u32 mm_entry_64;
+ u32 mm_signature;
+ u32 mm_blob_size;
+};
+
+extern struct mm_header *mm_header;
+extern unsigned char mm_blob_end[];
+
+extern unsigned char mm_blob[];
+extern unsigned char mm_relocs[];
+
+/*
+ * This has to be under 1MB (see tseg_region.c in coreboot source tree).
+ * The actual check for this is made in coreboot after we fill the header
+ * (see above) with the blob size.
+ */
+static inline size_t mm_payload_size_needed(void)
+{
+ return mm_blob_end - mm_blob;
+}
+
+#endif /* __ASSEMBLER__ */
+#endif /* __MM_PAYLOAD_H */
--
2.49.0Hi,
On Thu, Jun 12, 2025 at 04:05:48PM +0200, Michal Gorlas wrote:
> coreboot exposes (S)MM related data in the coreboot table. Extends existing interface,
> with structure corresponding to (S)MM data, and adds parser. Parser exposes this data
> to the modules executed later.
I don't think I have much opinion or knowledge about this feature, so
I'd probably defer to someone actually involved in the coreboot project
(Julius?) for some of that.
But a few cursory thoughts on the driver mechanics:
> Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
> ---
> drivers/firmware/google/Kconfig | 12 +++++
> drivers/firmware/google/Makefile | 3 ++
> drivers/firmware/google/coreboot_table.h | 34 ++++++++-----
> drivers/firmware/google/mm_info.c | 63 ++++++++++++++++++++++++
> drivers/firmware/google/mm_payload.h | 58 ++++++++++++++++++++++
> 5 files changed, 158 insertions(+), 12 deletions(-)
> create mode 100644 drivers/firmware/google/mm_info.c
> create mode 100644 drivers/firmware/google/mm_payload.h
>
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> index 41b78f5cb735..5d918c076f25 100644
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -81,4 +81,16 @@ config GOOGLE_VPD
> This option enables the kernel to expose the content of Google VPD
> under /sys/firmware/vpd.
>
> +config COREBOOT_PAYLOAD_MM
> + tristate "SMI handling in Linux (LinuxBootSMM)"
> + depends on GOOGLE_COREBOOT_TABLE
This all looks highly X86-specific, so you probably need 'depends on
X86'.
> + help
> + Enables support for SMI handling by Linux-owned code.
> + coreboot reserves region for payload-owned SMI handler, the Linux
> + driver prepares its SMI handler outside of SMRAM, and lets coreboot
> + know where the handler is placed by issuing an SMI. On this SMI, the
> + handler is being placed in SMRAM and all supported SMIs from that point
> + on are handled by Linux-owned SMI handler.
> + If in doubt, say N.
> +
> endif # GOOGLE_FIRMWARE
> diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
> index bb6f0f7299b4..e0b087933c5a 100644
> --- a/drivers/firmware/google/coreboot_table.h
> +++ b/drivers/firmware/google/coreboot_table.h
> @@ -112,8 +122,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
> * boilerplate. Each module may only use this macro once, and
> * calling it replaces module_init() and module_exit()
> */
> -#define module_coreboot_driver(__coreboot_driver) \
> +#define module_coreboot_driver(__coreboot_driver) \
> module_driver(__coreboot_driver, coreboot_driver_register, \
> - coreboot_driver_unregister)
> + coreboot_driver_unregister)
You're making arbitrary whitespace changes in this hunk. Try to avoid
that, please.
>
> #endif /* __COREBOOT_TABLE_H */
> diff --git a/drivers/firmware/google/mm_info.c b/drivers/firmware/google/mm_info.c
> new file mode 100644
> index 000000000000..55bcdc8b8d53
> --- /dev/null
> +++ b/drivers/firmware/google/mm_info.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * mm_info.c
> + *
> + * Driver for exporting MM payload information from coreboot table.
> + *
> + * Copyright 2025 9elements gmbh
> + * Copyright 2025 Michal Gorlas <michal.gorlas@9elements.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include "coreboot_table.h"
> +#include "mm_payload.h"
> +
> +static struct lb_pld_mm_interface_info *mm_cbtable_info;
> +struct mm_info *mm_info;
> +
> +static int mm_driver_probe(struct coreboot_device *dev)
> +{
> + mm_cbtable_info = &dev->mm_info;
> + if (mm_cbtable_info->tag != LB_TAG_PLD_MM_INTERFACE_INFO)
> + return -ENXIO;
> +
> + mm_info = kmalloc(sizeof(*mm_info), GFP_KERNEL);
Error handling? (Needs a NULL check.)
And might as well use devm_*() (e.g., devm_kzalloc()); then you can drop
mm_driver_remove().
> + mm_info->revision = mm_cbtable_info->revision;
> + mm_info->requires_long_mode_call =
> + mm_cbtable_info->requires_long_mode_call;
> + mm_info->register_mm_entry_command =
> + mm_cbtable_info->register_mm_entry_command;
> + return 0;
> +}
> +EXPORT_SYMBOL(mm_info);
Why non-GPL? IIUC, EXPORT_SYMBOL_GPL() is the usual default.
Or, I suspect you don't actually need two separate modules, so you
probably don't need this EXPORT at all.
> +
> +static void mm_driver_remove(struct coreboot_device *dev)
> +{
> + if (mm_info)
> + kfree(mm_info);
> +}
> +
...
Brian
Hi,
Thanks for taking your time to go over the patches.
On Thu, Jun 12, 2025 at 03:37:40PM -0700, Brian Norris wrote:
>
> This all looks highly X86-specific, so you probably need 'depends on
> X86'.
>
Indeed, this was also why CI build was failed.
> > @@ -112,8 +122,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
> > * boilerplate. Each module may only use this macro once, and
> > * calling it replaces module_init() and module_exit()
> > */
> > -#define module_coreboot_driver(__coreboot_driver) \
> > +#define module_coreboot_driver(__coreboot_driver) \
> > module_driver(__coreboot_driver, coreboot_driver_register, \
> > - coreboot_driver_unregister)
> > + coreboot_driver_unregister)
>
> You're making arbitrary whitespace changes in this hunk. Try to avoid
> that, please.
>
Sure, will do. It came from a style warning when running
scripts/checkpatch.pl. I thought it could be useful to fix it on the
same go.
> >
> > #endif /* __COREBOOT_TABLE_H */
> > diff --git a/drivers/firmware/google/mm_info.c b/drivers/firmware/google/mm_info.c
> > new file mode 100644
> > index 000000000000..55bcdc8b8d53
> > --- /dev/null
> > +++ b/drivers/firmware/google/mm_info.c
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * mm_info.c
> > + *
> > + * Driver for exporting MM payload information from coreboot table.
> > + *
> > + * Copyright 2025 9elements gmbh
> > + * Copyright 2025 Michal Gorlas <michal.gorlas@9elements.com>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +
> > +#include "coreboot_table.h"
> > +#include "mm_payload.h"
> > +
> > +static struct lb_pld_mm_interface_info *mm_cbtable_info;
> > +struct mm_info *mm_info;
> > +
> > +static int mm_driver_probe(struct coreboot_device *dev)
> > +{
> > + mm_cbtable_info = &dev->mm_info;
> > + if (mm_cbtable_info->tag != LB_TAG_PLD_MM_INTERFACE_INFO)
> > + return -ENXIO;
> > +
> > + mm_info = kmalloc(sizeof(*mm_info), GFP_KERNEL);
>
> Error handling? (Needs a NULL check.)
>
> And might as well use devm_*() (e.g., devm_kzalloc()); then you can drop
> mm_driver_remove().
>
> > + mm_info->revision = mm_cbtable_info->revision;
> > + mm_info->requires_long_mode_call =
> > + mm_cbtable_info->requires_long_mode_call;
> > + mm_info->register_mm_entry_command =
> > + mm_cbtable_info->register_mm_entry_command;
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(mm_info);
>
> Why non-GPL? IIUC, EXPORT_SYMBOL_GPL() is the usual default.
>
> Or, I suspect you don't actually need two separate modules, so you
> probably don't need this EXPORT at all.
>
Yep these two definetely can be merged into one module. Will do that in
v2 series.
Best,
Michal
On Sat, Jun 14, 2025 at 02:53:18PM +0200, Michal Gorlas wrote: > On Thu, Jun 12, 2025 at 03:37:40PM -0700, Brian Norris wrote: > > > @@ -112,8 +122,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver); > > > * boilerplate. Each module may only use this macro once, and > > > * calling it replaces module_init() and module_exit() > > > */ > > > -#define module_coreboot_driver(__coreboot_driver) \ > > > +#define module_coreboot_driver(__coreboot_driver) \ > > > module_driver(__coreboot_driver, coreboot_driver_register, \ > > > - coreboot_driver_unregister) > > > + coreboot_driver_unregister) > > > > You're making arbitrary whitespace changes in this hunk. Try to avoid > > that, please. > > > > Sure, will do. It came from a style warning when running > scripts/checkpatch.pl. I thought it could be useful to fix it on the > same go. That's odd, I don't see any such warning. Anyway, typically I'd expect such things not to be lumped together under the "separate your changes" guidance of Documentation/process/submitting-patches.rst (if they're worth changing at all), although that may not be a hard and fast rule. Brian
On Mon, Jun 16, 2025 at 11:16:18AM -0700, Brian Norris wrote:
> On Sat, Jun 14, 2025 at 02:53:18PM +0200, Michal Gorlas wrote:
> > On Thu, Jun 12, 2025 at 03:37:40PM -0700, Brian Norris wrote:
> > > > @@ -112,8 +122,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
> > > > * boilerplate. Each module may only use this macro once, and
> > > > * calling it replaces module_init() and module_exit()
> > > > */
> > > > -#define module_coreboot_driver(__coreboot_driver) \
> > > > +#define module_coreboot_driver(__coreboot_driver) \
> > > > module_driver(__coreboot_driver, coreboot_driver_register, \
> > > > - coreboot_driver_unregister)
> > > > + coreboot_driver_unregister)
> > >
> > > You're making arbitrary whitespace changes in this hunk. Try to avoid
> > > that, please.
> > >
> >
> > Sure, will do. It came from a style warning when running
> > scripts/checkpatch.pl. I thought it could be useful to fix it on the
> > same go.
>
> That's odd, I don't see any such warning. Anyway, typically I'd expect
> such things not to be lumped together under the "separate your changes"
> guidance of Documentation/process/submitting-patches.rst (if they're
> worth changing at all), although that may not be a hard and fast rule.
My bad, the whitespace were not a consequence of checkpatch.pl
complaints, but rather running clang-format on coreboot_table.h
on a clean tree:
* $ clang-format -i drivers/firmware/google/coreboot_table.h
* $ git diff
* diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
* index bb6f0f7299b4..3d933c657535 100644
* --- a/drivers/firmware/google/coreboot_table.h
* +++ b/drivers/firmware/google/coreboot_table.h
* @@ -61,15 +61,15 @@ struct lb_framebuffer {
* u32 x_resolution;
* u32 y_resolution;
* u32 bytes_per_line;
* - u8 bits_per_pixel;
* - u8 red_mask_pos;
* - u8 red_mask_size;
* - u8 green_mask_pos;
* - u8 green_mask_size;
* - u8 blue_mask_pos;
* - u8 blue_mask_size;
* - u8 reserved_mask_pos;
* - u8 reserved_mask_size;
* + u8 bits_per_pixel;
* + u8 red_mask_pos;
* + u8 red_mask_size;
* + u8 green_mask_pos;
* + u8 green_mask_size;
* + u8 blue_mask_pos;
* + u8 blue_mask_size;
* + u8 reserved_mask_pos;
* + u8 reserved_mask_size;
* };
*
* /* A device, additionally with information from coreboot. */
* @@ -112,8 +112,8 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
* * boilerplate. Each module may only use this macro once, and
* * calling it replaces module_init() and module_exit()
* */
* -#define module_coreboot_driver(__coreboot_driver) \
* +#define module_coreboot_driver(__coreboot_driver) \
* module_driver(__coreboot_driver, coreboot_driver_register, \
* - coreboot_driver_unregister)
* + coreboot_driver_unregister)
*
* #endif /* __COREBOOT_TABLE_H */
Anyway, I am not sure if it is even worth changing, its rather cosmetic. I
removed these changes in v2.
Best,
Michal
© 2016 - 2025 Red Hat, Inc.