Make the 16MiB long memory-mapped BIOS region of the platform SPI flash
on X86_64 system available via /sys/kernel/firmware/flash_mmap/bios_region
for pen-testing, security analysis and malware detection on kernels
which restrict module loading and/or access to /dev/mem.
It will be used by the open source Converged Security Suite.
https://github.com/9elements/converged-security-suite
Signed-off-by: Hans-Gert Dahmen <[email protected]>
---
drivers/firmware/Kconfig | 9 ++++
drivers/firmware/Makefile | 1 +
drivers/firmware/x86_64_flash_mmap.c | 65 ++++++++++++++++++++++++++++
3 files changed, 75 insertions(+)
create mode 100644 drivers/firmware/x86_64_flash_mmap.c
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index db0ea2d2d75a..bd77ca2b4fa6 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -296,6 +296,15 @@ config TURRIS_MOX_RWTM
other manufacturing data and also utilize the Entropy Bit Generator
for hardware random number generation.
+config X86_64_FLASH_MMAP
+ tristate "Export platform flash memory-mapped BIOS region"
+ depends on X86_64
+ help
+ Export the memory-mapped BIOS region of the platform SPI flash as
+ a read-only sysfs binary attribute on X86_64 systems. The first 16MiB
+ will be accessible via /sys/kernel/firmware/flash_mmap/bios_region
+ for security and malware analysis for example.
+
source "drivers/firmware/broadcom/Kconfig"
source "drivers/firmware/google/Kconfig"
source "drivers/firmware/efi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 5e013b6a3692..eb7483c5a2ac 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
+obj-$(CONFIG_X86_64_FLASH_MMAP) += x86_64_flash_mmap.o
obj-y += arm_scmi/
obj-y += broadcom/
diff --git a/drivers/firmware/x86_64_flash_mmap.c b/drivers/firmware/x86_64_flash_mmap.c
new file mode 100644
index 000000000000..f9d871a8b516
--- /dev/null
+++ b/drivers/firmware/x86_64_flash_mmap.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Export the memory-mapped BIOS region of the platform SPI flash as
+ * a read-only sysfs binary attribute on X86_64 systems.
+ *
+ * Copyright © 2021 immune GmbH
+ */
+
+#include <linux/version.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+#define FLASH_REGION_START 0xFF000000ULL
+#define FLASH_REGION_SIZE 0x1000000ULL
+#define FLASH_REGION_MASK (FLASH_REGION_SIZE - 1)
+
+struct kobject *kobj_ref;
+
+static ssize_t bios_region_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buffer,
+ loff_t offset, size_t count)
+{
+ resource_size_t pa = FLASH_REGION_START + (offset & FLASH_REGION_MASK);
+ void __iomem *va = ioremap(pa, PAGE_SIZE);
+
+ memcpy_fromio(buffer, va, PAGE_SIZE);
+ iounmap(va);
+
+ return min(count, PAGE_SIZE);
+}
+
+BIN_ATTR_RO(bios_region, FLASH_REGION_SIZE);
+
+static int __init flash_mmap_init(void)
+{
+ int ret = 0;
+
+ kobj_ref = kobject_create_and_add("flash_mmap", firmware_kobj);
+ ret = sysfs_create_bin_file(kobj_ref, &bin_attr_bios_region);
+ if (ret) {
+ pr_err("sysfs_create_bin_file failed\n");
+ goto error;
+ }
+
+ return ret;
+
+error:
+ kobject_put(kobj_ref);
+ return ret;
+}
+
+static void __exit flash_mmap_exit(void)
+{
+ sysfs_remove_bin_file(kernel_kobj, &bin_attr_bios_region);
+ kobject_put(kobj_ref);
+}
+
+module_init(flash_mmap_init);
+module_exit(flash_mmap_exit);
+MODULE_DESCRIPTION("Export SPI platform flash memory mapped region via sysfs");
+MODULE_AUTHOR("Hans-Gert Dahmen <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.30.2
On Tue, Jun 22, 2021 at 04:23:34PM +0200, Hans-Gert Dahmen wrote:
> Make the 16MiB long memory-mapped BIOS region of the platform SPI flash
> on X86_64 system available via /sys/kernel/firmware/flash_mmap/bios_region
> for pen-testing, security analysis and malware detection on kernels
> which restrict module loading and/or access to /dev/mem.
>
> It will be used by the open source Converged Security Suite.
> https://github.com/9elements/converged-security-suite
>
> Signed-off-by: Hans-Gert Dahmen <[email protected]>
> ---
> drivers/firmware/Kconfig | 9 ++++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/x86_64_flash_mmap.c | 65 ++++++++++++++++++++++++++++
> 3 files changed, 75 insertions(+)
> create mode 100644 drivers/firmware/x86_64_flash_mmap.c
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index db0ea2d2d75a..bd77ca2b4fa6 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -296,6 +296,15 @@ config TURRIS_MOX_RWTM
> other manufacturing data and also utilize the Entropy Bit Generator
> for hardware random number generation.
>
> +config X86_64_FLASH_MMAP
> + tristate "Export platform flash memory-mapped BIOS region"
> + depends on X86_64
> + help
> + Export the memory-mapped BIOS region of the platform SPI flash as
> + a read-only sysfs binary attribute on X86_64 systems. The first 16MiB
> + will be accessible via /sys/kernel/firmware/flash_mmap/bios_region
> + for security and malware analysis for example.
Module name information here?
Can this be auto-loaded based on hardware-specific values somewhere?
Otherwise it just looks like if this module loads, it will "always
work"?
And why would you want to map the bios into userspace?
What bios, UEFI?
And you need a Documentation/ABI/ update for new sysfs files.
> +
> source "drivers/firmware/broadcom/Kconfig"
> source "drivers/firmware/google/Kconfig"
> source "drivers/firmware/efi/Kconfig"
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 5e013b6a3692..eb7483c5a2ac 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
> obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
> +obj-$(CONFIG_X86_64_FLASH_MMAP) += x86_64_flash_mmap.o
>
> obj-y += arm_scmi/
> obj-y += broadcom/
> diff --git a/drivers/firmware/x86_64_flash_mmap.c b/drivers/firmware/x86_64_flash_mmap.c
> new file mode 100644
> index 000000000000..f9d871a8b516
> --- /dev/null
> +++ b/drivers/firmware/x86_64_flash_mmap.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Export the memory-mapped BIOS region of the platform SPI flash as
> + * a read-only sysfs binary attribute on X86_64 systems.
> + *
> + * Copyright ? 2021 immune GmbH
> + */
> +
> +#include <linux/version.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/sysfs.h>
> +#include <linux/kobject.h>
> +
> +#define FLASH_REGION_START 0xFF000000ULL
> +#define FLASH_REGION_SIZE 0x1000000ULL
Where do these values come from?
> +#define FLASH_REGION_MASK (FLASH_REGION_SIZE - 1)
> +
> +struct kobject *kobj_ref;
Only 1? Not per-hardware-device?
> +
> +static ssize_t bios_region_read(struct file *file, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buffer,
> + loff_t offset, size_t count)
> +{
> + resource_size_t pa = FLASH_REGION_START + (offset & FLASH_REGION_MASK);
> + void __iomem *va = ioremap(pa, PAGE_SIZE);
Why PAGE_SIZE?
> +
> + memcpy_fromio(buffer, va, PAGE_SIZE);
> + iounmap(va);
> +
> + return min(count, PAGE_SIZE);
> +}
> +
> +BIN_ATTR_RO(bios_region, FLASH_REGION_SIZE);
> +
> +static int __init flash_mmap_init(void)
> +{
> + int ret = 0;
> +
> + kobj_ref = kobject_create_and_add("flash_mmap", firmware_kobj);
> + ret = sysfs_create_bin_file(kobj_ref, &bin_attr_bios_region);
You just raced with userspace and lost :(
Please make a sysfs attribute part of a default "group" for a kobject.
But as you are using a "raw" kobject here, that feels really wrong to
me. Isn't this some sort of platform device really? Why not go that
way, why tie this to the firmware subsystem?
> + if (ret) {
> + pr_err("sysfs_create_bin_file failed\n");
> + goto error;
> + }
> +
> + return ret;
So this just "always works"? That feels VERY dangerous.
As this is a x86 thing, you should also cc: the x86 maintainers.
thanks,
greg k-h
From: Hans-Gert Dahmen
> Sent: 22 June 2021 15:24
>
> Make the 16MiB long memory-mapped BIOS region of the platform SPI flash
> on X86_64 system available via /sys/kernel/firmware/flash_mmap/bios_region
> for pen-testing, security analysis and malware detection on kernels
> which restrict module loading and/or access to /dev/mem.
Are you saying that my 15 year old 64bit Athlon cpu and bios
have this large SPI flash and the required hardware to
convert bus cycles to serial spi reads?
I very much doubt it.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Hi,
these are some good points.
On 23.06.21 00:18, David Laight wrote:
> Are you saying that my 15 year old 64bit Athlon cpu and bios
> have this large SPI flash
No. The reads will wrap, i.e. if your flash is 2MB then it would be
repeated 8 times in the 16MB window.
> and the required hardware to
> convert bus cycles to serial spi reads?
Yes. The window is part of the DMI interface and the south bridge or PCH
converts the bus cycles to SPI reads. It is because this region contains
the reset vector address of your CPU and the very first instruction it
executes after a reset when the internal setup is done will actually be
loaded from the serial SPI bus. It is AFAIK part of AMD's original
64-bit specification.
However, after reading your mail I understand that I should have looked
up the exact explanations in the respective specs. So to definitively
answer your question I need to know which south bridge there is in your
15 year old system and have a look into its datasheet. Do you know which
one it is by any chance?
Hans-Gert Dahmen
On Wed, Jun 23, 2021 at 02:17:54PM +0200, Hans-Gert Dahmen wrote:
> Hi,
>
> these are some good points.
>
> On 23.06.21 00:18, David Laight wrote:
> > Are you saying that my 15 year old 64bit Athlon cpu and bios
> > have this large SPI flash
>
> No. The reads will wrap, i.e. if your flash is 2MB then it would be repeated
> 8 times in the 16MB window.
>
> > and the required hardware to
> > convert bus cycles to serial spi reads?
>
> Yes. The window is part of the DMI interface and the south bridge or PCH
> converts the bus cycles to SPI reads. It is because this region contains the
> reset vector address of your CPU and the very first instruction it executes
> after a reset when the internal setup is done will actually be loaded from
> the serial SPI bus. It is AFAIK part of AMD's original 64-bit specification.
>
> However, after reading your mail I understand that I should have looked up
> the exact explanations in the respective specs. So to definitively answer
> your question I need to know which south bridge there is in your 15 year old
> system and have a look into its datasheet. Do you know which one it is by
> any chance?
The point is that you will never be able to do this for all devices.
You should ONLY be allowed to have this module bind to the hardware that
you KNOW it will work with.
So please work off of a DMI table, or some such hardware description,
instead of just blindly enabling it for all systems.
thanks,
greg k-h
From: Hans-Gert Dahmen
> Sent: 23 June 2021 13:18
>
> these are some good points.
>
> On 23.06.21 00:18, David Laight wrote:
> > Are you saying that my 15 year old 64bit Athlon cpu and bios
> > have this large SPI flash
>
> No. The reads will wrap, i.e. if your flash is 2MB then it would be
> repeated 8 times in the 16MB window.
>
> > and the required hardware to
> > convert bus cycles to serial spi reads?
>
> Yes. The window is part of the DMI interface and the south bridge or PCH
> converts the bus cycles to SPI reads. It is because this region contains
> the reset vector address of your CPU and the very first instruction it
> executes after a reset when the internal setup is done will actually be
> loaded from the serial SPI bus. It is AFAIK part of AMD's original
> 64-bit specification.
>
> However, after reading your mail I understand that I should have looked
> up the exact explanations in the respective specs. So to definitively
> answer your question I need to know which south bridge there is in your
> 15 year old system and have a look into its datasheet. Do you know which
> one it is by any chance?
Absolutely no idea.
That particular system doesn't actually boot any more
with either cpu I have for it plugged it.
I suspect the psu voltages are out of range and have broken it.
But that isn't the point.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On 23.06.21 14:40, [email protected] wrote:
>> On Wed, Jun 23, 2021 at 02:17:54PM +0200, Hans-Gert Dahmen wrote:
>> Hi,
>> Yes. The window is part of the DMI interface and the south bridge or PCH
>> converts the bus cycles to SPI reads. It is because this region contains the
>> reset vector address of your CPU and the very first instruction it executes
>> after a reset when the internal setup is done will actually be loaded from
>> the serial SPI bus. It is AFAIK part of AMD's original 64-bit specification.
> The point is that you will never be able to do this for all devices.
> You should ONLY be allowed to have this module bind to the hardware that
> you KNOW it will work with.
>
> So please work off of a DMI table, or some such hardware description,
> instead of just blindly enabling it for all systems.
I was referring to the DMI/QPI/PCI interface that connects the
ICH/PCH/south bridge to the CPU. I have gone through all datasheets of
intel ICH and PCH and they state that the address range from 0xff000000
through 0xffffffff is a fixed mapping that cannot be changed (no BAR)
except for the original ICH (dating back to 1999) where the window is
only 8MB. The original ICH is for 32-bit systems only so all 64-bit
Intel systems that exist have this feature. I have talked to somebody
who works with future Intel hardware and the person indicated that it is
not likely to change.
This is why I made the module depend on X86_64. I still have to do the
same complete research for AMD systems which is a little harder to do,
so I am proposing to check if the root complex has Intel's vendor ID and
only load the module on 64-bit Intel systems until I can confirm the
same behavior for all 64-bit AMD systems. Then I could check if the root
complex is Intel or AMD. Would that suffice as "some such hardware
description"?
Here are my public sources:
ICH0 Datasheet Chapter 6.3 and Table 6-5
https://www.tautec-electronics.de/Datenblaetter/Schaltkreise/FW82801AA.pdf
ICH2 Datasheet Chapter 6.4 and Table 6-4
https://www.intel.com/content/dam/doc/datasheet/82801ba-i-o-controller-hub-2-82801bam-i-o-controller-hub-2-mobile-datasheet.pdf
ICH3 Datasheet Chapter 6.4 and Table 6-5
https://www.intel.com/content/dam/doc/datasheet/82801ca-io-controller-hub-3-datasheet.pdf
ICH4 Datasheet Chapter 6.4 and Table 6-5
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82801db-io-controller-hub-4-datasheet.pdf
ICH5 Datasheet Chapter 6.4 and Table 133
https://www.intel.com/content/dam/doc/datasheet/82801eb-82801er-io-controller-hub-datasheet.pdf
ICH6 Datasheet Chapter 6.4 and Table 6-4
https://www.intel.com/content/dam/doc/datasheet/io-controller-hub-6-datasheet.pdf
ICH7 Datasheet Chapter 6.4 and Table 6-4
https://www.intel.com/content/dam/doc/datasheet/i-o-controller-hub-7-datasheet.pdf
ICH8 Datasheet Chapter 6.4 and Table 102
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/io-controller-hub-8-datasheet.pdf
ICH9 Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/doc/datasheet/io-controller-hub-9-datasheet.pdf
ICH10 Datasheet Chapter 9.4 and Table 9-4
https://theswissbay.ch/pdf/Datasheets/Intel/io-controller-hub-10-family-datasheet.pdf
PCH Intel 5 Series Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/5-chipset-3400-chipset-datasheet.pdf
PCH Intel 6 Series Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/6-chipset-c200-chipset-datasheet.pdf
PCH Intel 7 Series Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7-series-chipset-pch-datasheet.pdf
PCH Intel 8 Series Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/8-series-chipset-pch-datasheet.pdf
PCH Intel 9 Series Datasheet Chapter 9.4 and Table 9-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/9-series-chipset-pch-datasheet.pdf
PCH Intel 100 Series Datasheet Vol 1 Chapter 4.3 and Table 4-4
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/100-series-chipset-datasheet-vol-1.pdf
PCH Intel 200 Series Datasheet Vol 1 Chapter 4.3 and Table 4-4
https://www.mouser.com/datasheet/2/612/200-series-chipset-pch-datasheet-vol-1-1391746.pdf
PCH Intel 300 Series Datasheet Vol 1 Chapter 4.3 and Table 4-4
https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/300-series-chipset-on-package-pch-datasheet-vol-1.pdf
PCH Intel 400 Series Datasheet Vol 1 Chapter 4.2 and Table 8
https://images-eu.ssl-images-amazon.com/images/I/B1TDsSyARKS.pdf
Hans-Gert Dahmen
On Thu, Jun 24, 2021 at 01:20:28PM +0200, Hans-Gert Dahmen wrote:
>
> On 23.06.21 14:40, [email protected] wrote:
> > > On Wed, Jun 23, 2021 at 02:17:54PM +0200, Hans-Gert Dahmen wrote:
> > > Hi,
> > > Yes. The window is part of the DMI interface and the south bridge or PCH
> > > converts the bus cycles to SPI reads. It is because this region contains the
> > > reset vector address of your CPU and the very first instruction it executes
> > > after a reset when the internal setup is done will actually be loaded from
> > > the serial SPI bus. It is AFAIK part of AMD's original 64-bit specification.
> > The point is that you will never be able to do this for all devices.
> > You should ONLY be allowed to have this module bind to the hardware that
> > you KNOW it will work with.
> >
> > So please work off of a DMI table, or some such hardware description,
> > instead of just blindly enabling it for all systems.
>
> I was referring to the DMI/QPI/PCI interface that connects the ICH/PCH/south
> bridge to the CPU. I have gone through all datasheets of intel ICH and PCH
> and they state that the address range from 0xff000000 through 0xffffffff is
> a fixed mapping that cannot be changed (no BAR) except for the original ICH
> (dating back to 1999) where the window is only 8MB. The original ICH is for
> 32-bit systems only so all 64-bit Intel systems that exist have this
> feature. I have talked to somebody who works with future Intel hardware and
> the person indicated that it is not likely to change.
>
> This is why I made the module depend on X86_64. I still have to do the same
> complete research for AMD systems which is a little harder to do, so I am
> proposing to check if the root complex has Intel's vendor ID and only load
> the module on 64-bit Intel systems until I can confirm the same behavior for
> all 64-bit AMD systems. Then I could check if the root complex is Intel or
> AMD. Would that suffice as "some such hardware description"?
That would help, yes. Especially given the other types of Intel-like
cpus we are seeing in the wild these days (not all the world is Intel
and AMD...)
But what is this really going to be used for? What userspace tools need
this type of direct access to do something useful?
thanks,
greg k-h
Hi,
this is the first time I am working on the Linux kernel, so please
excuse that I overlooked some things.
On 22.06.21 22:02, Greg KH wrote:
> On Tue, Jun 22, 2021 at 04:23:34PM +0200, Hans-Gert Dahmen wrote:
>> Make the 16MiB long memory-mapped BIOS region of the platform SPI flash
>> on X86_64 system available via /sys/kernel/firmware/flash_mmap/bios_region
>> for pen-testing, security analysis and malware detection on kernels
>> which restrict module loading and/or access to /dev/mem.
>>
>> It will be used by the open source Converged Security Suite.
>> https://github.com/9elements/converged-security-suite
>>
>> Signed-off-by: Hans-Gert Dahmen <[email protected]>
>> ---
>> drivers/firmware/Kconfig | 9 ++++
>> drivers/firmware/Makefile | 1 +
>> drivers/firmware/x86_64_flash_mmap.c | 65 ++++++++++++++++++++++++++++
>> 3 files changed, 75 insertions(+)
>> create mode 100644 drivers/firmware/x86_64_flash_mmap.c
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index db0ea2d2d75a..bd77ca2b4fa6 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -296,6 +296,15 @@ config TURRIS_MOX_RWTM
>> other manufacturing data and also utilize the Entropy Bit Generator
>> for hardware random number generation.
>>
>> +config X86_64_FLASH_MMAP
>> + tristate "Export platform flash memory-mapped BIOS region"
>> + depends on X86_64
>> + help
>> + Export the memory-mapped BIOS region of the platform SPI flash as
>> + a read-only sysfs binary attribute on X86_64 systems. The first 16MiB
>> + will be accessible via /sys/kernel/firmware/flash_mmap/bios_region
>> + for security and malware analysis for example.
>
> Module name information here?
>
> Can this be auto-loaded based on hardware-specific values somewhere?
> Otherwise it just looks like if this module loads, it will "always
> work"?
>
> And why would you want to map the bios into userspace?
>
> What bios, UEFI?
>
> And you need a Documentation/ABI/ update for new sysfs files.
The core use-case is security analysis and detecting BIOS/UEFI malware.
It is going to be used by the open-source Converged Security Suite
developed by Facebook, Google and 9elements security. The CSS dissects
UEFI binaries and checks it for common vulnerabilities.
The current state is that there are some drivers to access the SPI flash
bit they are in a questionable state and often don't work. Using this
memory mapped region works most of the time without requiring a real
hardware driver and significantly lowers the barrier to asses UEFI
security of systems deployed in the wild.
In another mail I have shown that this can safely be done on Intel
systems so I will make this module load on Intel systems for now and
also fix the documentation.
>
>
>> +
>> source "drivers/firmware/broadcom/Kconfig"
>> source "drivers/firmware/google/Kconfig"
>> source "drivers/firmware/efi/Kconfig"
>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>> index 5e013b6a3692..eb7483c5a2ac 100644
>> --- a/drivers/firmware/Makefile
>> +++ b/drivers/firmware/Makefile
>> @@ -21,6 +21,7 @@ obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
>> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
>> obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
>> obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
>> +obj-$(CONFIG_X86_64_FLASH_MMAP) += x86_64_flash_mmap.o
>>
>> obj-y += arm_scmi/
>> obj-y += broadcom/
>> diff --git a/drivers/firmware/x86_64_flash_mmap.c b/drivers/firmware/x86_64_flash_mmap.c
>> new file mode 100644
>> index 000000000000..f9d871a8b516
>> --- /dev/null
>> +++ b/drivers/firmware/x86_64_flash_mmap.c
>> @@ -0,0 +1,65 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Export the memory-mapped BIOS region of the platform SPI flash as
>> + * a read-only sysfs binary attribute on X86_64 systems.
>> + *
>> + * Copyright © 2021 immune GmbH
>> + */
>> +
>> +#include <linux/version.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/io.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/kobject.h>
>> +
>> +#define FLASH_REGION_START 0xFF000000ULL
>> +#define FLASH_REGION_SIZE 0x1000000ULL
>
> Where do these values come from?
I have listed the relevant Intel datasheets in another mail in this thread.
>
>> +#define FLASH_REGION_MASK (FLASH_REGION_SIZE - 1)
>> +
>> +struct kobject *kobj_ref;
>
> Only 1? Not per-hardware-device?
Yes, there is only one BIOS/UEFI that is configured to actively boot the
system. This method is not suitable to access shadow flash chips or
other wild things that mainboard manufacturers did in the past.
>
>> +
>> +static ssize_t bios_region_read(struct file *file, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buffer,
>> + loff_t offset, size_t count)
>> +{
>> + resource_size_t pa = FLASH_REGION_START + (offset & FLASH_REGION_MASK);
>> + void __iomem *va = ioremap(pa, PAGE_SIZE);
>
> Why PAGE_SIZE?
Please correct me if I'm wrong: the documentation is sparse and from
what I could see in the sources it appears that binary attributes pass a
page sized buffer around. I was assuming that the offset parameter would
be page aligned.
>
>> +
>> + memcpy_fromio(buffer, va, PAGE_SIZE);
>> + iounmap(va);
>> +
>> + return min(count, PAGE_SIZE);
>> +}
>> +
>> +BIN_ATTR_RO(bios_region, FLASH_REGION_SIZE);
>> +
>> +static int __init flash_mmap_init(void)
>> +{
>> + int ret = 0;
>> +
>> + kobj_ref = kobject_create_and_add("flash_mmap", firmware_kobj);
>> + ret = sysfs_create_bin_file(kobj_ref, &bin_attr_bios_region);
>
> You just raced with userspace and lost :(
I have taken inspiration from other modules. The documentation doesn't
say a lot. Could somebody point me to a proper example somewhere in the
source?
>
> Please make a sysfs attribute part of a default "group" for a kobject.
> But as you are using a "raw" kobject here, that feels really wrong to
> me. Isn't this some sort of platform device really? Why not go that
> way, why tie this to the firmware subsystem?
>
What this module provides read access to is the firmware. I am new to
Linux kernel development and found it quite hard to decide where to put
this. Suggestions are welcome.
>> + if (ret) {
>> + pr_err("sysfs_create_bin_file failed\n");
>> + goto error;
>> + }
>> +
>> + return ret;
>
> So this just "always works"? That feels VERY dangerous.
Will change that.
>
> As this is a x86 thing, you should also cc: the x86 maintainers.
Will do.
>
> thanks,
>
> greg k-h
>
Hans-Gert Dahmen