2024-01-17 19:06:05

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v3 0/4] Allow coreboot modules to autoload and enable cbmem in the arm64 defconfig

This series adds the missing pieces to the coreboot bus and the module
alias generation to allow coreboot modules to be automatically loaded
when matching devices are detected.

The configs for cbmem coreboot entries are then enabled in the arm64
defconfig, as modules, to allow reading logs from coreboot on arm64
Chromebooks, which is useful for debugging the boot process.

Changes in v3:
- Merged all "add to module device table" commits into a single commit
which also changes the coreboot_driver struct to contain an id table
and avoid unused variable warnings for the id tables.

Changes in v2:
- Added commits for vpd, memconsole and framebuffer drivers to add them
to the module device table

---
Nícolas F. R. A. Prado (4):
firmware: coreboot: Generate modalias uevent for devices
firmware: coreboot: Generate aliases for coreboot modules
firmware: coreboot: Replace tag with id table in driver struct
arm64: defconfig: Enable support for cbmem entries in the coreboot table

arch/arm64/configs/defconfig | 3 +++
drivers/firmware/google/cbmem.c | 8 +++++++-
drivers/firmware/google/coreboot_table.c | 20 +++++++++++++++++++-
drivers/firmware/google/coreboot_table.h | 3 ++-
drivers/firmware/google/framebuffer-coreboot.c | 8 +++++++-
drivers/firmware/google/memconsole-coreboot.c | 8 +++++++-
drivers/firmware/google/vpd.c | 8 +++++++-
include/linux/mod_devicetable.h | 8 ++++++++
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 10 ++++++++++
10 files changed, 73 insertions(+), 6 deletions(-)
---
base-commit: 0f067394dd3b2af3263339cf7183bdb6ee0ac1f8
change-id: 20240117-coreboot-mod-defconfig-826b01e242d9

Best regards,
--
Nícolas F. R. A. Prado <[email protected]>



2024-01-17 19:06:27

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v3 1/4] firmware: coreboot: Generate modalias uevent for devices

Generate a modalias uevent for devices in the coreboot bus to allow
userspace to automatically load the corresponding modules.

Acked-by: Brian Norris <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
drivers/firmware/google/coreboot_table.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 2a4469bf1b81..c1b9a9e8e8ed 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -53,11 +53,20 @@ static void coreboot_bus_remove(struct device *dev)
driver->remove(device);
}

+static int coreboot_bus_uevent(const struct device *dev, struct kobj_uevent_env *env)
+{
+ struct coreboot_device *device = CB_DEV(dev);
+ u32 tag = device->entry.tag;
+
+ return add_uevent_var(env, "MODALIAS=coreboot:t%08X", tag);
+}
+
static struct bus_type coreboot_bus_type = {
.name = "coreboot",
.match = coreboot_bus_match,
.probe = coreboot_bus_probe,
.remove = coreboot_bus_remove,
+ .uevent = coreboot_bus_uevent,
};

static void coreboot_device_release(struct device *dev)

--
2.43.0


2024-01-17 19:06:38

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v3 2/4] firmware: coreboot: Generate aliases for coreboot modules

Generate aliases for coreboot modules to allow automatic module probing.

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
include/linux/mod_devicetable.h | 8 ++++++++
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 10 ++++++++++
3 files changed, 21 insertions(+)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index f458469c5ce5..24e0dcfde809 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -960,4 +960,12 @@ struct vchiq_device_id {
char name[32];
};

+/**
+ * struct coreboot_device_id - Identifies a coreboot table entry
+ * @tag: tag ID
+ */
+struct coreboot_device_id {
+ __u32 tag;
+};
+
#endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index e91a3c38143b..518200813d4e 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -274,5 +274,8 @@ int main(void)
DEVID(vchiq_device_id);
DEVID_FIELD(vchiq_device_id, name);

+ DEVID(coreboot_device_id);
+ DEVID_FIELD(coreboot_device_id, tag);
+
return 0;
}
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4829680a0a6d..5d1c61fa5a55 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1494,6 +1494,15 @@ static int do_vchiq_entry(const char *filename, void *symval, char *alias)
return 1;
}

+/* Looks like: coreboot:tN */
+static int do_coreboot_entry(const char *filename, void *symval, char *alias)
+{
+ DEF_FIELD(symval, coreboot_device_id, tag);
+ sprintf(alias, "coreboot:t%08X", tag);
+
+ return 1;
+}
+
/* Does namelen bytes of name exactly match the symbol? */
static bool sym_is(const char *name, unsigned namelen, const char *symbol)
{
@@ -1575,6 +1584,7 @@ static const struct devtable devtable[] = {
{"ishtp", SIZE_ishtp_device_id, do_ishtp_entry},
{"cdx", SIZE_cdx_device_id, do_cdx_entry},
{"vchiq", SIZE_vchiq_device_id, do_vchiq_entry},
+ {"coreboot", SIZE_coreboot_device_id, do_coreboot_entry},
};

/* Create MODULE_ALIAS() statements.

--
2.43.0


2024-01-17 19:07:05

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v3 4/4] arm64: defconfig: Enable support for cbmem entries in the coreboot table

Enable the cbmem driver and dependencies in order to support reading
cbmem entries from the coreboot table, which are used to store logs from
coreboot on arm64 Chromebooks, and provide useful information for
debugging the boot process on those devices.

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
arch/arm64/configs/defconfig | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 361c31b5d064..49121133f045 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -255,6 +255,9 @@ CONFIG_INTEL_STRATIX10_RSU=m
CONFIG_MTK_ADSP_IPC=m
CONFIG_QCOM_QSEECOM=y
CONFIG_QCOM_QSEECOM_UEFISECAPP=y
+CONFIG_GOOGLE_FIRMWARE=y
+CONFIG_GOOGLE_CBMEM=m
+CONFIG_GOOGLE_COREBOOT_TABLE=m
CONFIG_EFI_CAPSULE_LOADER=y
CONFIG_IMX_SCU=y
CONFIG_IMX_SCU_PD=y

--
2.43.0


2024-01-17 19:08:39

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v3 3/4] firmware: coreboot: Replace tag with id table in driver struct

Switch the plain 'tag' field in struct coreboot_driver for the newly
created coreboot_device_id struct, which also contains a tag field and
has the benefit of allowing modalias generation, and update all coreboot
drivers accordingly.

While at it, also add the id table for each driver to the module device
table to allow automatically loading the module.

Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
drivers/firmware/google/cbmem.c | 8 +++++++-
drivers/firmware/google/coreboot_table.c | 11 ++++++++++-
drivers/firmware/google/coreboot_table.h | 3 ++-
drivers/firmware/google/framebuffer-coreboot.c | 8 +++++++-
drivers/firmware/google/memconsole-coreboot.c | 8 +++++++-
drivers/firmware/google/vpd.c | 8 +++++++-
6 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/google/cbmem.c b/drivers/firmware/google/cbmem.c
index 88e587ba1e0d..c2bffdc352a3 100644
--- a/drivers/firmware/google/cbmem.c
+++ b/drivers/firmware/google/cbmem.c
@@ -114,6 +114,12 @@ static int cbmem_entry_probe(struct coreboot_device *dev)
return 0;
}

+static const struct coreboot_device_id cbmem_ids[] = {
+ { .tag = LB_TAG_CBMEM_ENTRY },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(coreboot, cbmem_ids);
+
static struct coreboot_driver cbmem_entry_driver = {
.probe = cbmem_entry_probe,
.drv = {
@@ -121,7 +127,7 @@ static struct coreboot_driver cbmem_entry_driver = {
.owner = THIS_MODULE,
.dev_groups = dev_groups,
},
- .tag = LB_TAG_CBMEM_ENTRY,
+ .id_table = cbmem_ids,
};
module_coreboot_driver(cbmem_entry_driver);

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index c1b9a9e8e8ed..33971cf33112 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -28,8 +28,17 @@ static int coreboot_bus_match(struct device *dev, struct device_driver *drv)
{
struct coreboot_device *device = CB_DEV(dev);
struct coreboot_driver *driver = CB_DRV(drv);
+ const struct coreboot_device_id *id;

- return device->entry.tag == driver->tag;
+ if (!driver->id_table)
+ return 0;
+
+ for (id = driver->id_table; id->tag; id++) {
+ if (device->entry.tag == id->tag)
+ return 1;
+ }
+
+ return 0;
}

static int coreboot_bus_probe(struct device *dev)
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index d814dca33a08..86427989c57f 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -13,6 +13,7 @@
#define __COREBOOT_TABLE_H

#include <linux/device.h>
+#include <linux/mod_devicetable.h>

/* Coreboot table header structure */
struct coreboot_table_header {
@@ -93,7 +94,7 @@ struct coreboot_driver {
int (*probe)(struct coreboot_device *);
void (*remove)(struct coreboot_device *);
struct device_driver drv;
- u32 tag;
+ const struct coreboot_device_id *id_table;
};

/* Register a driver that uses the data from a coreboot table. */
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
index 5c84bbebfef8..07c458bf64ec 100644
--- a/drivers/firmware/google/framebuffer-coreboot.c
+++ b/drivers/firmware/google/framebuffer-coreboot.c
@@ -80,13 +80,19 @@ static void framebuffer_remove(struct coreboot_device *dev)
platform_device_unregister(pdev);
}

+static const struct coreboot_device_id framebuffer_ids[] = {
+ { .tag = CB_TAG_FRAMEBUFFER },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(coreboot, framebuffer_ids);
+
static struct coreboot_driver framebuffer_driver = {
.probe = framebuffer_probe,
.remove = framebuffer_remove,
.drv = {
.name = "framebuffer",
},
- .tag = CB_TAG_FRAMEBUFFER,
+ .id_table = framebuffer_ids,
};
module_coreboot_driver(framebuffer_driver);

diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index 74b5286518ee..24c97a70aa80 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -96,13 +96,19 @@ static void memconsole_remove(struct coreboot_device *dev)
memconsole_exit();
}

+static const struct coreboot_device_id memconsole_ids[] = {
+ { .tag = CB_TAG_CBMEM_CONSOLE },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(coreboot, memconsole_ids);
+
static struct coreboot_driver memconsole_driver = {
.probe = memconsole_probe,
.remove = memconsole_remove,
.drv = {
.name = "memconsole",
},
- .tag = CB_TAG_CBMEM_CONSOLE,
+ .id_table = memconsole_ids,
};
module_coreboot_driver(memconsole_driver);

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index ee6e08c0592b..8e4216714b29 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -306,13 +306,19 @@ static void vpd_remove(struct coreboot_device *dev)
kobject_put(vpd_kobj);
}

+static const struct coreboot_device_id vpd_ids[] = {
+ { .tag = CB_TAG_VPD },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(coreboot, vpd_ids);
+
static struct coreboot_driver vpd_driver = {
.probe = vpd_probe,
.remove = vpd_remove,
.drv = {
.name = "vpd",
},
- .tag = CB_TAG_VPD,
+ .id_table = vpd_ids,
};
module_coreboot_driver(vpd_driver);


--
2.43.0


Subject: Re: [PATCH v3 3/4] firmware: coreboot: Replace tag with id table in driver struct

Il 17/01/24 20:03, Nícolas F. R. A. Prado ha scritto:
> Switch the plain 'tag' field in struct coreboot_driver for the newly
> created coreboot_device_id struct, which also contains a tag field and
> has the benefit of allowing modalias generation, and update all coreboot
> drivers accordingly.
>
> While at it, also add the id table for each driver to the module device
> table to allow automatically loading the module.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



2024-01-19 02:39:36

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Allow coreboot modules to autoload and enable cbmem in the arm64 defconfig

On Wed, Jan 17, 2024 at 04:03:21PM -0300, N?colas F. R. A. Prado wrote:
> This series adds the missing pieces to the coreboot bus and the module
> alias generation to allow coreboot modules to be automatically loaded
> when matching devices are detected.
>
> The configs for cbmem coreboot entries are then enabled in the arm64
> defconfig, as modules, to allow reading logs from coreboot on arm64
> Chromebooks, which is useful for debugging the boot process.
>
> Changes in v3:
> - Merged all "add to module device table" commits into a single commit
> which also changes the coreboot_driver struct to contain an id table
> and avoid unused variable warnings for the id tables.
>
> Changes in v2:
> - Added commits for vpd, memconsole and framebuffer drivers to add them
> to the module device table
>
> ---
> N?colas F. R. A. Prado (4):
> firmware: coreboot: Generate modalias uevent for devices
> firmware: coreboot: Generate aliases for coreboot modules
> firmware: coreboot: Replace tag with id table in driver struct
> arm64: defconfig: Enable support for cbmem entries in the coreboot table

The series overall looks good to me.

I'm happy to queue all the patches into chrome-platform-firmware for the next
merge window (i.e. for v6.9-rc1). Let's wait a bit for the maintainers for
the other subsystems if they are OK with that.

2024-01-23 22:08:01

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Allow coreboot modules to autoload and enable cbmem in the arm64 defconfig

On Wed, Jan 17, 2024 at 04:03:21PM -0300, N?colas F. R. A. Prado wrote:
> This series adds the missing pieces to the coreboot bus and the module
> alias generation to allow coreboot modules to be automatically loaded
> when matching devices are detected.
>
> The configs for cbmem coreboot entries are then enabled in the arm64
> defconfig, as modules, to allow reading logs from coreboot on arm64
> Chromebooks, which is useful for debugging the boot process.
>
> Changes in v3:
> - Merged all "add to module device table" commits into a single commit
> which also changes the coreboot_driver struct to contain an id table
> and avoid unused variable warnings for the id tables.
>
> Changes in v2:
> - Added commits for vpd, memconsole and framebuffer drivers to add them
> to the module device table

For the series:

Reviewed-by: Brian Norris <[email protected]>

Thanks!

2024-02-05 01:37:44

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] firmware: coreboot: Generate modalias uevent for devices

On Wed, Jan 17, 2024 at 04:03:22PM -0300, N?colas F. R. A. Prado wrote:
> Generate a modalias uevent for devices in the coreboot bus to allow
> userspace to automatically load the corresponding modules.
>
> [...]

Applied, thanks!

[1/4] firmware: coreboot: Generate modalias uevent for devices
commit: c6b0a4ceb7c9d8bb014d2967c97c8c7cbf60b006

2024-02-05 14:17:40

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] firmware: coreboot: Generate modalias uevent for devices

On Mon, Feb 05, 2024 at 09:37:29AM +0800, Tzung-Bi Shih wrote:
> On Wed, Jan 17, 2024 at 04:03:22PM -0300, N?colas F. R. A. Prado wrote:
> > Generate a modalias uevent for devices in the coreboot bus to allow
> > userspace to automatically load the corresponding modules.
> >
> > [...]
>
> Applied, thanks!
>
> [1/4] firmware: coreboot: Generate modalias uevent for devices
> commit: c6b0a4ceb7c9d8bb014d2967c97c8c7cbf60b006

Hi Tzung-Bi,

I was going to send a v4 with tag changed into 64 bit long as suggested by Greg:
https://lore.kernel.org/all/2024020105-dash-antiquity-a56b@gregkh

And that includes this commit, as the modalias field would need to be 16 hex
long.

But since you already merged this, would you prefer a change on top of this
making it 64 bit long, or do you want to keep it 32 bits?

Thanks,
N?colas

2024-02-06 03:54:33

by Tzung-Bi Shih

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] firmware: coreboot: Generate modalias uevent for devices

On Mon, Feb 05, 2024 at 09:13:51AM -0500, N?colas F. R. A. Prado wrote:
> On Mon, Feb 05, 2024 at 09:37:29AM +0800, Tzung-Bi Shih wrote:
> > On Wed, Jan 17, 2024 at 04:03:22PM -0300, N?colas F. R. A. Prado wrote:
> > > Generate a modalias uevent for devices in the coreboot bus to allow
> > > userspace to automatically load the corresponding modules.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/4] firmware: coreboot: Generate modalias uevent for devices
> > commit: c6b0a4ceb7c9d8bb014d2967c97c8c7cbf60b006
>
> Hi Tzung-Bi,
>
> I was going to send a v4 with tag changed into 64 bit long as suggested by Greg:
> https://lore.kernel.org/all/2024020105-dash-antiquity-a56b@gregkh
>
> And that includes this commit, as the modalias field would need to be 16 hex
> long.
>
> But since you already merged this, would you prefer a change on top of this
> making it 64 bit long, or do you want to keep it 32 bits?

Oops, I overlooked `u32` in the patch thus I thought the patch is indepedent.
Please go ahead to send v4 with 64 bit long tag. I will drop
c6b0a4ceb7c9d8bb014d2967c97c8c7cbf60b006 from the queue.