[PATCH 0/4] Allow coreboot modules to autoload and enable cbmem in the arm64 defconfig

[PATCH 0/4] Allow coreboot modules to autoload and enable cbmem in the arm64 defconfig

[Nícolas F. R. A. Prado]

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.


Nícolas F. R. A. Prado (4):
  firmware: coreboot: Generate modalias uevent for devices
  firmware: coreboot: Generate aliases for coreboot modules
  firmware: google: cbmem: Add to module device table
  arm64: defconfig: Enable support for cbmem entries in the coreboot
    table

 arch/arm64/configs/defconfig             |  3 +++
 drivers/firmware/google/cbmem.c          |  7 +++++++
 drivers/firmware/google/coreboot_table.c |  9 +++++++++
 include/linux/mod_devicetable.h          |  8 ++++++++
 scripts/mod/devicetable-offsets.c        |  3 +++
 scripts/mod/file2alias.c                 | 10 ++++++++++
 6 files changed, 40 insertions(+)

-- 
2.43.0

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

[Nícolas F. R. A. Prado]

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

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 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

[PATCH 3/4] firmware: google: cbmem: Add to module device table

[Nícolas F. R. A. Prado]

Create an id table and add it to the module device table to allow the
cbmem driver to be automatically loaded when a matching device is found.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/firmware/google/cbmem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firmware/google/cbmem.c b/drivers/firmware/google/cbmem.c
index 88e587ba1e0d..ceb89b4cdbe0 100644
--- a/drivers/firmware/google/cbmem.c
+++ b/drivers/firmware/google/cbmem.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/kobject.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
@@ -114,6 +115,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 = {
-- 
2.43.0

Re: [PATCH 1/4] firmware: coreboot: Generate modalias uevent for devices

[Brian Norris]

It would have been nice to get the whole series, or at least direct to
all the same mailing lists -- specifically, chrome-platform@lists.linux.dev

But I found the whole thing eventually.

On Thu, Jan 11, 2024 at 12:11:46PM -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.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Acked-by: Brian Norris <briannorris@chromium.org>

Re: [PATCH 3/4] firmware: google: cbmem: Add to module device table

[Brian Norris]

On Thu, Jan 11, 2024 at 12:11:48PM -0300, Nícolas F. R. A. Prado wrote:
> Create an id table and add it to the module device table to allow the
> cbmem driver to be automatically loaded when a matching device is found.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Might as well also patch framebuffer-coreboot.c and
memconsole-coreboot.c while you're at it?

But for this one:

Acked-by: Brian Norris <briannorris@chromium.org>

Re: [PATCH 1/4] firmware: coreboot: Generate modalias uevent for devices

[Nícolas F. R. A. Prado]

On Thu, Jan 11, 2024 at 04:37:10PM -0800, Brian Norris wrote:
> It would have been nice to get the whole series, or at least direct to
> all the same mailing lists -- specifically, chrome-platform@lists.linux.dev

Yeah, that's an artifact of using patman for patch submission... But I'll make
sure that list gets the whole series for v2.

Thanks,
Nícolas

Re: [PATCH 3/4] firmware: google: cbmem: Add to module device table

[Nícolas F. R. A. Prado]

On Thu, Jan 11, 2024 at 04:38:44PM -0800, Brian Norris wrote:
> On Thu, Jan 11, 2024 at 12:11:48PM -0300, Nícolas F. R. A. Prado wrote:
> > Create an id table and add it to the module device table to allow the
> > cbmem driver to be automatically loaded when a matching device is found.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Might as well also patch framebuffer-coreboot.c and
> memconsole-coreboot.c while you're at it?

Yeah, no reason not to. Will do in v2.

Thanks,
Nícolas

Re: [PATCH 3/4] firmware: google: cbmem: Add to module device table

[kernel test robot]

Hi Nícolas,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20240111]
[also build test WARNING on v6.7]
[cannot apply to chrome-platform/for-next chrome-platform/for-firmware-next masahiroy-kbuild/for-next masahiroy-kbuild/fixes arm64/for-next/core linus/master v6.7 v6.7-rc8 v6.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/N-colas-F-R-A-Prado/firmware-coreboot-Generate-modalias-uevent-for-devices/20240111-231841
base:   next-20240111
patch link:    https://lore.kernel.org/r/20240111151226.842603-4-nfraprado%40collabora.com
patch subject: [PATCH 3/4] firmware: google: cbmem: Add to module device table
config: i386-randconfig-013-20240115 (https://download.01.org/0day-ci/archive/20240115/202401151013.Xioj5wZo-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240115/202401151013.Xioj5wZo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401151013.Xioj5wZo-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/firmware/google/cbmem.c:118:40: warning: unused variable 'cbmem_ids' [-Wunused-const-variable]
     118 | static const struct coreboot_device_id cbmem_ids[] = {
         |                                        ^~~~~~~~~
   1 warning generated.


vim +/cbmem_ids +118 drivers/firmware/google/cbmem.c

   117	
 > 118	static const struct coreboot_device_id cbmem_ids[] = {
   119		{ .tag = LB_TAG_CBMEM_ENTRY },
   120		{ /* sentinel */ }
   121	};
   122	MODULE_DEVICE_TABLE(coreboot, cbmem_ids);
   123	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/4] firmware: google: cbmem: Add to module device table

[Brian Norris]

Hi Nicolas,

On Mon, Jan 15, 2024 at 10:53:48AM +0800, kernel test robot wrote:
> All warnings (new ones prefixed by >>):
> 
> >> drivers/firmware/google/cbmem.c:118:40: warning: unused variable 'cbmem_ids' [-Wunused-const-variable]
>      118 | static const struct coreboot_device_id cbmem_ids[] = {
>          |                                        ^~~~~~~~~
>    1 warning generated.
> 
> 
> vim +/cbmem_ids +118 drivers/firmware/google/cbmem.c
> 
>    117	
>  > 118	static const struct coreboot_device_id cbmem_ids[] = {
>    119		{ .tag = LB_TAG_CBMEM_ENTRY },
>    120		{ /* sentinel */ }
>    121	};
>    122	MODULE_DEVICE_TABLE(coreboot, cbmem_ids);
>    123	

I was wondering why we have a seemingly unique "unused variable" failure
mode in comparison to other similarly-structured device/bus drivers, and
I realized that's because we're not relying on the same structure for
both MODULE_DEVICE_TABLE (struct coreboot_device_id) and for the driver
definition (struct coreboot_driver -> 'u32 tag'). Thus, this structure
is only used for #define MODULE builds, and otherwise not used.

Rather than wrapping these definitions in "#ifdef MODULE", perhaps we
can settle on a single field, and replace `struct coreboot_driver::tag`
with an instance of `struct coreboot_device_id`? That would normally be
a breaking change that would require changing all drivers at the same
time as the bus (or else some kind of intermediate transition state),
but considering there are only 4 driver implementations and they all
live under the same maintainer tree, that seems like it should still be
OK (IMO).

If it makes the series more readable/incremental, perhaps the switchover
can be the last patch in the series, and there can remain some
duplication (and potential -Wunused-const-variable issues) for the
middle of the series.

Brian