lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 4 Oct 2022 10:56:58 -0600 From: Jack Rosenthal <jrosenth@...omium.org> To: Greg KH <gregkh@...uxfoundation.org> Cc: linux-kernel@...r.kernel.org, chrome-platform@...ts.linux.dev, Stephen Boyd <swboyd@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>, Guenter Roeck <groeck@...omium.org>, Julius Werner <jwerner@...omium.org> Subject: Re: [PATCH v12] firmware: google: Implement cbmem in sysfs driver On 2022-10-04 at 10:51 +0200, Greg KH wrote: > > + A list of ids known to Coreboot can be found in the coreboot > > + source tree at > > + ``src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h``. > > That will not age well, why not point to the reference in the kernel > tree instead? There is no copy in the kernel tree. > > +What: /sys/bus/coreboot/devices/cbmem-<id>/address > > +Date: August 2022 > > +Contact: Jack Rosenthal <jrosenth@...omium.org> > > +Description: > > + This is the pyhsical memory address that the CBMEM entry's data > > + begins at. > > In hex? Decimal? > > > + > > +What: /sys/bus/coreboot/devices/cbmem-<id>/size > > +Date: August 2022 > > +Contact: Jack Rosenthal <jrosenth@...omium.org> > > +Description: > > + This is the size of the CBMEM entry's data. > > In hex? Decimal? Octal? Binary? Be specific please :) Added "hexadecimal" and an example for both in v13. > > +What: /sys/bus/coreboot/devices/cbmem-<id>/id > > +Date: August 2022 > > +Contact: Jack Rosenthal <jrosenth@...omium.org> > > +Description: > > + This is the CBMEM id corresponding to the entry. > > so "id" is the same as "<id>" here? Why is that needed? Removed in v13, agreee it's reduntant with the device name now. > > + Say Y here to enable the kernel to search for Coreboot CBMEM > > + entries, and expose the memory for each entry in sysfs under > > + /sys/bus/coreboot/devices/cbmem-<id>. > > Module name? Added in v13. > > > + > > config GOOGLE_COREBOOT_TABLE > > tristate "Coreboot Table Access" > > depends on HAS_IOMEM && (ACPI || OF) > > diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile > > index d17caded5d88..8151e323cc43 100644 > > --- a/drivers/firmware/google/Makefile > > +++ b/drivers/firmware/google/Makefile > > @@ -7,5 +7,8 @@ obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o > > obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o > > obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o > > > > +# Must come after coreboot_table.o, as this driver depends on that bus type. > > Doesn't the linker handle this for us? Not in the case of compiling as a built-in module: I observed in this scenario the order in the Makefile deterimined the module initialization order, and, if this were to be listed alphabetically, the coreboot_table module would not have been loaded before the cbmem module. > > + entry->size = dev->cbmem_entry.entry_size; > > Ah nevermind you set the size here. The size that stat reports is still 0, as when creating this as a device attribute, the size is not known until the driver is probed. I observed this in some other sysfs attributes, so I imagine it's a common pattern. > > +/* Corresponds to LB_TAG_CBMEM_ENTRY */ > > +struct lb_cbmem_entry { > > + u32 tag; > > + u32 size; > > little or big endian? It's the native host endianness, as coreboot wrote these tables from the same CPU that Linux is running on.
Powered by blists - more mailing lists