Re: [PATCH v3] efi: Ignore unrealistically large option roms
* Hans de Goede wrote: > diff --git a/arch/x86/boot/compressed/eboot.c > b/arch/x86/boot/compressed/eboot.c > index 47d3efff6805..8650ab268ee7 100644 > --- a/arch/x86/boot/compressed/eboot.c > +++ b/arch/x86/boot/compressed/eboot.c > @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct > pci_setup_rom **__rom) > if (status != EFI_SUCCESS) > return status; > > - if (!pci->romimage || !pci->romsize) > + /* > + * Some firmwares contain EFI function pointers at the place where the > + * romimage and romsize fields are supposed to be. Typically the EFI > + * code is mapped at high addresses, translating to an unrealistically > + * large romsize. The UEFI spec limits the size of option ROMs to 16 > + * MiB so we reject any roms over 16 MiB in size to catch this. > + */ > + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) > return EFI_INVALID_PARAMETER; > > size = pci->romsize + sizeof(*rom); > @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct > pci_setup_rom **__rom) > if (status != EFI_SUCCESS) > return status; > > - if (!pci->romimage || !pci->romsize) > + /* > + * Some firmwares contain EFI function pointers at the place where the > + * romimage and romsize fields are supposed to be. Typically the EFI > + * code is mapped at high addresses, translating to an unrealistically > + * large romsize. The UEFI spec limits the size of option ROMs to 16 > + * MiB so we reject any roms over 16 MiB in size to catch this. > + */ > + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) > return EFI_INVALID_PARAMETER; Any reason why this couldn't be factored out into a efi_check_rom(pci) kind of helper function, which would unify the logic and would also avoid the duplicate comment blocks? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Ignore unrealistically large option roms
On Sun, Apr 29, 2018 at 09:43:18AM +0200, Ingo Molnar wrote: > * Hans de Goede wrote: > > diff --git a/arch/x86/boot/compressed/eboot.c > > b/arch/x86/boot/compressed/eboot.c > > index 47d3efff6805..8650ab268ee7 100644 > > --- a/arch/x86/boot/compressed/eboot.c > > +++ b/arch/x86/boot/compressed/eboot.c > > @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct > > pci_setup_rom **__rom) > > if (status != EFI_SUCCESS) > > return status; > > > > - if (!pci->romimage || !pci->romsize) > > + /* > > +* Some firmwares contain EFI function pointers at the place where the > > +* romimage and romsize fields are supposed to be. Typically the EFI > > +* code is mapped at high addresses, translating to an unrealistically > > +* large romsize. The UEFI spec limits the size of option ROMs to 16 > > +* MiB so we reject any roms over 16 MiB in size to catch this. > > +*/ > > + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) > > return EFI_INVALID_PARAMETER; > > > > size = pci->romsize + sizeof(*rom); > > @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct > > pci_setup_rom **__rom) > > if (status != EFI_SUCCESS) > > return status; > > > > - if (!pci->romimage || !pci->romsize) > > + /* > > +* Some firmwares contain EFI function pointers at the place where the > > +* romimage and romsize fields are supposed to be. Typically the EFI > > +* code is mapped at high addresses, translating to an unrealistically > > +* large romsize. The UEFI spec limits the size of option ROMs to 16 > > +* MiB so we reject any roms over 16 MiB in size to catch this. > > +*/ > > + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) > > return EFI_INVALID_PARAMETER; > > Any reason why this couldn't be factored out into a efi_check_rom(pci) > kind of helper function, which would unify the logic and would also > avoid the duplicate comment blocks? The real fix would be to deduplicate __setup_efi_pci32 and __setup_efi_pci64 à la commits 2bd79f30eea1 and db4545d9a788. (That said the comment seems overly wordy. A short pointer to the UEFI spec with chapter number should be sufficient, anything else should be in the changelog. Just my 2 cents anyway.) Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Ignore unrealistically large option roms
Hi, On 29-04-18 09:43, Ingo Molnar wrote: * Hans de Goede wrote: diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 47d3efff6805..8650ab268ee7 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) return status; - if (!pci->romimage || !pci->romsize) + /* +* Some firmwares contain EFI function pointers at the place where the +* romimage and romsize fields are supposed to be. Typically the EFI +* code is mapped at high addresses, translating to an unrealistically +* large romsize. The UEFI spec limits the size of option ROMs to 16 +* MiB so we reject any roms over 16 MiB in size to catch this. +*/ + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) return EFI_INVALID_PARAMETER; size = pci->romsize + sizeof(*rom); @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) return status; - if (!pci->romimage || !pci->romsize) + /* +* Some firmwares contain EFI function pointers at the place where the +* romimage and romsize fields are supposed to be. Typically the EFI +* code is mapped at high addresses, translating to an unrealistically +* large romsize. The UEFI spec limits the size of option ROMs to 16 +* MiB so we reject any roms over 16 MiB in size to catch this. +*/ + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) return EFI_INVALID_PARAMETER; Any reason why this couldn't be factored out into a efi_check_rom(pci) kind of helper function The pci pointer is of 2 different types: __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... I guess I could give the helper a romimage and romsize argument to get around that. Ard, do you want me to do a v4 with such a helper ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Ignore unrealistically large option roms
On 29 April 2018 at 10:40, Hans de Goede wrote: > Hi, > > > On 29-04-18 09:43, Ingo Molnar wrote: >> >> >> * Hans de Goede wrote: >> >>> diff --git a/arch/x86/boot/compressed/eboot.c >>> b/arch/x86/boot/compressed/eboot.c >>> index 47d3efff6805..8650ab268ee7 100644 >>> --- a/arch/x86/boot/compressed/eboot.c >>> +++ b/arch/x86/boot/compressed/eboot.c >>> @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, >>> struct pci_setup_rom **__rom) >>> if (status != EFI_SUCCESS) >>> return status; >>> - if (!pci->romimage || !pci->romsize) >>> + /* >>> +* Some firmwares contain EFI function pointers at the place >>> where the >>> +* romimage and romsize fields are supposed to be. Typically the >>> EFI >>> +* code is mapped at high addresses, translating to an >>> unrealistically >>> +* large romsize. The UEFI spec limits the size of option ROMs to >>> 16 >>> +* MiB so we reject any roms over 16 MiB in size to catch this. >>> +*/ >>> + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) >>> return EFI_INVALID_PARAMETER; >>> size = pci->romsize + sizeof(*rom); >>> @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, >>> struct pci_setup_rom **__rom) >>> if (status != EFI_SUCCESS) >>> return status; >>> - if (!pci->romimage || !pci->romsize) >>> + /* >>> +* Some firmwares contain EFI function pointers at the place >>> where the >>> +* romimage and romsize fields are supposed to be. Typically the >>> EFI >>> +* code is mapped at high addresses, translating to an >>> unrealistically >>> +* large romsize. The UEFI spec limits the size of option ROMs to >>> 16 >>> +* MiB so we reject any roms over 16 MiB in size to catch this. >>> +*/ >>> + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) >>> return EFI_INVALID_PARAMETER; >> >> >> Any reason why this couldn't be factored out into a efi_check_rom(pci) >> kind of >> helper function > > > The pci pointer is of 2 different types: > > __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... > __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... > > I guess I could give the helper a romimage and romsize argument to get > around that. > Ard, do you want me to do a v4 with such a helper ? > I think Lukas has a point, although I shouldn't expect you to implement that. I will look into this myself, and rebase your patch on top of that. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Ignore unrealistically large option roms
Hi, On 29-04-18 11:07, Ard Biesheuvel wrote: On 29 April 2018 at 10:40, Hans de Goede wrote: Hi, On 29-04-18 09:43, Ingo Molnar wrote: * Hans de Goede wrote: diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 47d3efff6805..8650ab268ee7 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) return status; - if (!pci->romimage || !pci->romsize) + /* +* Some firmwares contain EFI function pointers at the place where the +* romimage and romsize fields are supposed to be. Typically the EFI +* code is mapped at high addresses, translating to an unrealistically +* large romsize. The UEFI spec limits the size of option ROMs to 16 +* MiB so we reject any roms over 16 MiB in size to catch this. +*/ + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) return EFI_INVALID_PARAMETER; size = pci->romsize + sizeof(*rom); @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) return status; - if (!pci->romimage || !pci->romsize) + /* +* Some firmwares contain EFI function pointers at the place where the +* romimage and romsize fields are supposed to be. Typically the EFI +* code is mapped at high addresses, translating to an unrealistically +* large romsize. The UEFI spec limits the size of option ROMs to 16 +* MiB so we reject any roms over 16 MiB in size to catch this. +*/ + if (!pci->romimage || !pci->romsize || pci->romsize > 0x100) return EFI_INVALID_PARAMETER; Any reason why this couldn't be factored out into a efi_check_rom(pci) kind of helper function The pci pointer is of 2 different types: __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... I guess I could give the helper a romimage and romsize argument to get around that. Ard, do you want me to do a v4 with such a helper ? I think Lukas has a point, although I shouldn't expect you to implement that. I will look into this myself, and rebase your patch on top of that. Ok, thanks. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/5] efi/firmware/platform-x86: Add EFI embedded fw support
Hi All, Here is v5 of my patch-set to add support for EFI embedded fw to the kernel. Changes since v4: -Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS So I think this patch-set is getting close to ready for merging, which brings us to the question of how to merge this, I think that patches 1 and 2 should probably both be merged through the same tree. Then an unmutable branch should be created on that tree, merged into the platform/x86 tree and then the last 3 patches can be merged through that tree. Ard has already indicated he is fine with the EFI bits going upstream through another tree, so perhaps patches 1-2 can be merged through the firmware-loader-tree and then do an unmutable branch on the firmware-loader-tree for the platform/x86 tree to merge? I don't think taking all 5 through 1 tree is a good idea because of the file rename under platform/x86. For the record here are the cover letters of the previous versions: Changes since v3: -Drop note in docs about EFI_FIRMWARE_VOLUME_PROTOCOL, it is not part of UEFI proper, so the EFI maintainers don't want us referring people to it -Use new EFI_BOOT_SERVICES flag -Put the new fw_get_efi_embedded_fw() function in its own fallback_efi.c file which only gets built when EFI_EMBEDDED_FIRMWARE is selected -Define an empty stub for fw_get_efi_embedded_fw() in fallback.h hwen EFI_EMBEDDED_FIRMWARE is not selected, to avoid the need for #ifdefs in firmware_loader/main.c -Properly call security_kernel_post_read_file() on the firmware returned by efi_get_embedded_fw() to make sure that we are allowed to use it The 3 most prominent changes in v2 are: 1) Add documentation describing the EFI embedded firmware mechanism to: Documentation/driver-api/firmware/request_firmware.rst 2) Instead of having a single dmi_system_id array with its driver_data members pointing to efi_embedded_fw_desc structs, have the drivers which need EFI embedded-fw support export a dmi_system_id array and register that with the EFI embedded-fw code This series also includes the first driver to use this, in the form of the touchscreen_dmi code (formerly silead_dmi) from drivers/platfrom/x86 3) As discussed during the review of v1 we want to make the firmware_loader code fallback to EFI embedded-fw optional. Rather the adding yet another firmware_request_foo variant for this, with the risk of later also needing firmware_request_foo_nowait, etc. variants I've decided to make the code check if the device has a "efi-embedded-firmware" device-property bool set. This also seemed better because the same driver may want to use the fallback on some systems, but not on others since e.g. not all (x86) systems with a silead touchscreen have their touchscreen firmware embedded in their EFI. Note that (as discussed) when the EFI fallback path is requested, the usermodehelper fallback path is skipped. Here is the full changelog of patch 2/5 which is where most of the changes are: Changes in v2: -Rebased on driver-core/driver-core-next -Add documentation describing the EFI embedded firmware mechanism to: Documentation/driver-api/firmware/request_firmware.rst -Add a new EFI_EMBEDDED_FIRMWARE Kconfig bool and only build the embedded fw support if this is set. This is an invisible option which should be selected by drivers which need this -Remove the efi_embedded_fw_desc and dmi_system_id-s for known devices from the efi-embedded-fw code, instead drivers using this are expected to export a dmi_system_id array, with each entries' driver_data pointing to a efi_embedded_fw_desc struct and register this with the efi-embedded-fw code -Use kmemdup to make a copy instead of efi_mem_reserve()-ing the firmware, this avoids us messing with the EFI memmap and avoids the need to make changes to efi_mem_desc_lookup() -Make the firmware-loader code only fallback to efi_get_embedded_fw() if the passed in device has the "efi-embedded-firmware" device-property bool set -Skip usermodehelper fallback when "efi-embedded-firmware" device-property is set Patches 3-5 are new and implement using the EFI embedded-fw mechanism for Silead gsl and Chipone icn8505 touchscreens on x86 devices. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/5] platform/x86: Rename silead_dmi to touchscreen_dmi
Not only silead touchscreens need some extra info not available in the ACPI tables to work properly. X86 devices with a Chipone ICN8505 chip also need some DMI based extra configuration. There is no reason to have separate dmi config code per touchscreen controller vendor. This commit renames silead_dmi to a more generic touchscreen_dmi name (and Kconfig option) in preparation of adding info for tablets with an ICN8505 based touchscreen. Note there are no functional changes all code changes are limited to removing references to silead where these are no longer applicable. Acked-by: Andy Shevchenko Acked-by: Ard Biesheuvel Signed-off-by: Hans de Goede --- MAINTAINERS | 2 +- drivers/platform/x86/Kconfig | 16 ++--- drivers/platform/x86/Makefile | 2 +- .../x86/{silead_dmi.c => touchscreen_dmi.c} | 66 +-- 4 files changed, 43 insertions(+), 43 deletions(-) rename drivers/platform/x86/{silead_dmi.c => touchscreen_dmi.c} (87%) diff --git a/MAINTAINERS b/MAINTAINERS index bee29977a3bd..5a215bcd6660 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12795,7 +12795,7 @@ L: linux-in...@vger.kernel.org L: platform-driver-...@vger.kernel.org S: Maintained F: drivers/input/touchscreen/silead.c -F: drivers/platform/x86/silead_dmi.c +F: drivers/platform/x86/touchscreen_dmi.c SILICON MOTION SM712 FRAME BUFFER DRIVER M: Sudip Mukherjee diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 39d06dd1f63a..7ff206e5edfe 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1196,16 +1196,16 @@ config INTEL_TURBO_MAX_3 This driver is only required when the system is not using Hardware P-States (HWP). In HWP mode, priority can be read from ACPI tables. -config SILEAD_DMI - bool "Tablets with Silead touchscreens" +config TOUCHSCREEN_DMI + bool "DMI based touchscreen configuration info" depends on ACPI && DMI && I2C=y && TOUCHSCREEN_SILEAD ---help--- - Certain ACPI based tablets with Silead touchscreens do not have - enough data in ACPI tables for the touchscreen driver to handle - the touchscreen properly, as OEMs expected the data to be baked - into the tablet model specific version of the driver shipped - with the OS-image for the device. This option supplies the missing - information. Enable this for x86 tablets with Silead touchscreens. + Certain ACPI based tablets with e.g. Silead or Chipone touchscreens + do not have enough data in ACPI tables for the touchscreen driver to + handle the touchscreen properly, as OEMs expect the data to be baked + into the tablet model specific version of the driver shipped with the + the OS-image for the device. This option supplies the missing info. + Enable this for x86 tablets with Silead or Chipone touchscreens. config INTEL_CHTDC_TI_PWRBTN tristate "Intel Cherry Trail Dollar Cove TI power button driver" diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 2ba6cb795338..8d9477114fb5 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -78,7 +78,7 @@ obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o obj-$(CONFIG_PVPANIC) += pvpanic.o obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o -obj-$(CONFIG_SILEAD_DMI) += silead_dmi.o +obj-$(CONFIG_TOUCHSCREEN_DMI) += touchscreen_dmi.o obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o obj-$(CONFIG_SURFACE_3_BUTTON) += surface3_button.o obj-$(CONFIG_INTEL_PUNIT_IPC) += intel_punit_ipc.o diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/touchscreen_dmi.c similarity index 87% rename from drivers/platform/x86/silead_dmi.c rename to drivers/platform/x86/touchscreen_dmi.c index 452aacabaa8e..87fc839b28f7 100644 --- a/drivers/platform/x86/silead_dmi.c +++ b/drivers/platform/x86/touchscreen_dmi.c @@ -1,5 +1,5 @@ /* - * Silead touchscreen driver DMI based configuration code + * Touchscreen driver DMI based configuration code * * Copyright (c) 2017 Red Hat Inc. * @@ -20,7 +20,7 @@ #include #include -struct silead_ts_dmi_data { +struct ts_dmi_data { const char *acpi_name; const struct property_entry *properties; }; @@ -34,7 +34,7 @@ static const struct property_entry cube_iwork8_air_props[] = { { } }; -static const struct silead_ts_dmi_data cube_iwork8_air_data = { +static const struct ts_dmi_data cube_iwork8_air_data = { .acpi_name = "MSSL1680:00", .properties = cube_iwork8_air_props, }; @@ -48,7 +48,7 @@ static const struct property_entry jumper_ezpad_mini3_props[] = { { } }; -static const struct silead_ts_dmi_data jumper_ezpad_mini3_data = { +static const struct ts_dmi_data j
[PATCH v5 4/5] platform/x86: touchscreen_dmi: Add EFI embedded firmware info support
Sofar we have been unable to get permission from the vendors to put the firmware for touchscreens listed in touchscreen_dmi in linux-firmware. Some of the tablets with such a touchscreen have a touchscreen driver, and thus a copy of the firmware, as part of their EFI code. This commit adds the necessary info for the new EFI embedded-firmware code to extract these firmwares, making the touchscreen work OOTB without the user needing to manually add the firmware. Acked-by: Andy Shevchenko Acked-by: Ard Biesheuvel Signed-off-by: Hans de Goede --- drivers/firmware/efi/embedded-firmware.c | 3 +++ drivers/platform/x86/Kconfig | 1 + drivers/platform/x86/touchscreen_dmi.c | 26 +++- include/linux/efi_embedded_fw.h | 2 ++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/embedded-firmware.c b/drivers/firmware/efi/embedded-firmware.c index 22a0f598b53d..36a93a6938f0 100644 --- a/drivers/firmware/efi/embedded-firmware.c +++ b/drivers/firmware/efi/embedded-firmware.c @@ -23,6 +23,9 @@ struct embedded_fw { static LIST_HEAD(found_fw_list); static const struct dmi_system_id * const embedded_fw_table[] = { +#ifdef CONFIG_TOUCHSCREEN_DMI + touchscreen_dmi_table, +#endif NULL }; diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 7ff206e5edfe..cc4d95459190 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1199,6 +1199,7 @@ config INTEL_TURBO_MAX_3 config TOUCHSCREEN_DMI bool "DMI based touchscreen configuration info" depends on ACPI && DMI && I2C=y && TOUCHSCREEN_SILEAD + select EFI_EMBEDDED_FIRMWARE if EFI ---help--- Certain ACPI based tablets with e.g. Silead or Chipone touchscreens do not have enough data in ACPI tables for the touchscreen driver to diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c index 87fc839b28f7..6488cd50ba79 100644 --- a/drivers/platform/x86/touchscreen_dmi.c +++ b/drivers/platform/x86/touchscreen_dmi.c @@ -15,12 +15,15 @@ #include #include #include +#include #include #include #include #include struct ts_dmi_data { + /* The EFI embedded-fw code expects this to be the first member! */ + struct efi_embedded_fw_desc embedded_fw; const char *acpi_name; const struct property_entry *properties; }; @@ -31,10 +34,17 @@ static const struct property_entry cube_iwork8_air_props[] = { PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), PROPERTY_ENTRY_STRING("firmware-name", "gsl3670-cube-iwork8-air.fw"), PROPERTY_ENTRY_U32("silead,max-fingers", 10), + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), { } }; static const struct ts_dmi_data cube_iwork8_air_data = { + .embedded_fw = { + .name = "silead/gsl3670-cube-iwork8-air.fw", + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, + .length = 38808, + .crc= 0xfecde51f, + }, .acpi_name = "MSSL1680:00", .properties = cube_iwork8_air_props, }; @@ -119,10 +129,17 @@ static const struct property_entry pipo_w2s_props[] = { PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-pipo-w2s.fw"), + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), { } }; static const struct ts_dmi_data pipo_w2s_data = { + .embedded_fw = { + .name = "silead/gsl1680-pipo-w2s.fw", + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, + .length = 39072, + .crc= 0x28d5dc6c, + }, .acpi_name = "MSSL1680:00", .properties = pipo_w2s_props, }; @@ -162,10 +179,17 @@ static const struct property_entry chuwi_hi8_pro_props[] = { PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), PROPERTY_ENTRY_STRING("firmware-name", "gsl3680-chuwi-hi8-pro.fw"), PROPERTY_ENTRY_BOOL("silead,home-button"), + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), { } }; static const struct ts_dmi_data chuwi_hi8_pro_data = { + .embedded_fw = { + .name = "silead/gsl3680-chuwi-hi8-pro.fw", + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, + .length = 39864, + .crc= 0xfe2bedba, + }, .acpi_name = "MSSL1680:00", .properties = chuwi_hi8_pro_props, }; @@ -277,7 +301,7 @@ static const struct ts_dmi_data teclast_x3_plus_data = { .properties = teclast_x3_plus_props, }; -static const struct dmi_system_id touchscreen_dmi_table[] = { +const struct dmi_system_id touchscreen_dmi_table[] = { { /* CUBE iwork8 Air */ .driver_data = (void *)&cube_iwork8_air_data, diff --git a/include/linux/
[PATCH v5 2/5] efi: Add embedded peripheral firmware support
Just like with PCI options ROMs, which we save in the setup_efi_pci* functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself sometimes may contain data which is useful/necessary for peripheral drivers to have access to. Specifically the EFI code may contain an embedded copy of firmware which needs to be (re)loaded into the peripheral. Normally such firmware would be part of linux-firmware, but in some cases this is not feasible, for 2 reasons: 1) The firmware is customized for a specific use-case of the chipset / use with a specific hardware model, so we cannot have a single firmware file for the chipset. E.g. touchscreen controller firmwares are compiled specifically for the hardware model they are used with, as they are calibrated for a specific model digitizer. 2) Despite repeated attempts we have failed to get permission to redistribute the firmware. This is especially a problem with customized firmwares, these get created by the chip vendor for a specific ODM and the copyright may partially belong with the ODM, so the chip vendor cannot give a blanket permission to distribute these. This commit adds support for finding peripheral firmware embedded in the EFI code and making this available to peripheral drivers through the standard firmware loading mechanism. Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware near the end of start_kernel(), just before calling rest_init(), this is on purpose because the typical EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap(), so the check must be done after mm_init(). This relies on EFI_BOOT_SERVICES_CODE not being free-ed until efi_free_boot_services() is called, which means that this will only work on x86 for now. Reported-by: Dave Olsthoorn Suggested-by: Peter Jones Acked-by: Ard Biesheuvel Signed-off-by: Hans de Goede --- Changes in v5: -Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS Changes in v4: -Drop note in docs about EFI_FIRMWARE_VOLUME_PROTOCOL, it is not part of UEFI proper, so the EFI maintainers don't want us referring people to it -Use new EFI_BOOT_SERVICES flag -Put the new fw_get_efi_embedded_fw() function in its own fallback_efi.c file which only gets built when EFI_EMBEDDED_FIRMWARE is selected -Define an empty stub for fw_get_efi_embedded_fw() in fallback.h hwen EFI_EMBEDDED_FIRMWARE is not selected, to avoid the need for #ifdefs in firmware_loader/main.c -Properly call security_kernel_post_read_file() on the firmware returned by efi_get_embedded_fw() to make sure that we are allowed to use it Changes in v3: -Fix the docs using "efi-embedded-fw" as property name instead of "efi-embedded-firmware" Changes in v2: -Rebased on driver-core/driver-core-next -Add documentation describing the EFI embedded firmware mechanism to: Documentation/driver-api/firmware/request_firmware.rst -Add a new EFI_EMBEDDED_FIRMWARE Kconfig bool and only build the embedded fw support if this is set. This is an invisible option which should be selected by drivers which need this -Remove the efi_embedded_fw_desc and dmi_system_id-s for known devices from the efi-embedded-fw code, instead drivers using this are expected to export a dmi_system_id array, with each entries' driver_data pointing to a efi_embedded_fw_desc struct and register this with the efi-embedded-fw code -Use kmemdup to make a copy instead of efi_mem_reserve()-ing the firmware, this avoids us messing with the EFI memmap and avoids the need to make changes to efi_mem_desc_lookup() -Make the firmware-loader code only fallback to efi_get_embedded_fw() if the passed in device has the "efi-embedded-firmware" device-property bool set -Skip usermodehelper fallback when "efi-embedded-firmware" device-property is set --- .../driver-api/firmware/request_firmware.rst | 66 drivers/base/firmware_loader/Makefile | 1 + drivers/base/firmware_loader/fallback.h | 12 ++ drivers/base/firmware_loader/fallback_efi.c | 51 ++ drivers/base/firmware_loader/main.c | 2 + drivers/firmware/efi/Kconfig | 3 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/embedded-firmware.c | 149 ++ include/linux/efi.h | 6 + include/linux/efi_embedded_fw.h | 25 +++ init/main.c | 3 + 11 files changed, 319 insertions(+) create mode 100644 drivers/base/firmware_loader/fallback_efi.c create mode 100644 drivers/firmware/efi/embedded-firmware.c create mode 100644 include/linux/efi_embedded_fw.h diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst index c8bddbdcfd10..560dfed76e38 100644 --- a/Documentation/driver-api/firmware/request_firmware.rst +++ b/Documentation/driver-api/firmware/request_firmware.rst @@ -73,3 +73,69 @@ If something went wrong firmware_request() returns non-zero and fw_
[PATCH v5 5/5] platform/x86: touchscreen_dmi: Add info for the Chuwi Vi8 Plus tablet
Add touchscreen info for the Chuwi Vi8 Plus tablet. This tablet uses a Chipone ICN8505 touchscreen controller, with the firmware used by the touchscreen embedded in the EFI firmware. Acked-by: Andy Shevchenko Acked-by: Ard Biesheuvel Signed-off-by: Hans de Goede --- drivers/platform/x86/touchscreen_dmi.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c index 6488cd50ba79..5fdb6fe878f4 100644 --- a/drivers/platform/x86/touchscreen_dmi.c +++ b/drivers/platform/x86/touchscreen_dmi.c @@ -301,6 +301,22 @@ static const struct ts_dmi_data teclast_x3_plus_data = { .properties = teclast_x3_plus_props, }; +static const struct property_entry efi_embedded_fw_props[] = { + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), + { } +}; + +static const struct ts_dmi_data chuwi_vi8_plus_data = { + .embedded_fw = { + .name = "chipone/icn8505-HAMP0002.fw", + .prefix = { 0xb0, 0x07, 0x00, 0x00, 0xe4, 0x07, 0x00, 0x00 }, + .length = 35012, + .crc= 0x74dfd3fc, + }, + .acpi_name = "CHPN0001:00", + .properties = efi_embedded_fw_props, +}; + const struct dmi_system_id touchscreen_dmi_table[] = { { /* CUBE iwork8 Air */ @@ -487,6 +503,15 @@ const struct dmi_system_id touchscreen_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "Y8W81"), }, }, + { + /* Chuwi Vi8 Plus (CWI506) */ + .driver_data = (void *)&chuwi_vi8_plus_data, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Hampoo"), + DMI_MATCH(DMI_PRODUCT_NAME, "D2D3_Vi8A1"), + DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"), + }, + }, { }, }; -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/5] efi: Export boot-services code and data as debugfs-blobs
Sometimes it is useful to be able to dump the efi boot-services code and data. This commit adds these as debugfs-blobs to /sys/kernel/debug/efi, but only if efi=debug is passed on the kernel-commandline as this requires not freeing those memory-regions, which costs 20+ MB of RAM. Reviewed-by: Greg Kroah-Hartman Acked-by: Ard Biesheuvel Signed-off-by: Hans de Goede --- Changes in v5: -Rename the EFI_BOOT_SERVICES flag to EFI_PRESERVE_BS_REGIONS Changes in v4: -Add new EFI_BOOT_SERVICES flag and use it to determine if the boot-services memory segments are available (and thus if it makes sense to register the debugfs bits for them) Changes in v2: -Do not call pr_err on debugfs call failures --- arch/x86/platform/efi/efi.c| 1 + arch/x86/platform/efi/quirks.c | 4 +++ drivers/firmware/efi/efi.c | 53 ++ include/linux/efi.h| 1 + 4 files changed, 59 insertions(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 9061babfbc83..82f0836d 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -208,6 +208,7 @@ int __init efi_memblock_x86_reserve_range(void) efi.memmap.desc_version); memblock_reserve(pmap, efi.memmap.nr_map * efi.memmap.desc_size); + set_bit(EFI_PRESERVE_BS_REGIONS, &efi.flags); return 0; } diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c index 36c1f8b9f7e0..16bdb9e3b343 100644 --- a/arch/x86/platform/efi/quirks.c +++ b/arch/x86/platform/efi/quirks.c @@ -376,6 +376,10 @@ void __init efi_free_boot_services(void) int num_entries = 0; void *new, *new_md; + /* Keep all regions for /sys/kernel/debug/efi */ + if (efi_enabled(EFI_DBG)) + return; + for_each_efi_memory_desc(md) { unsigned long long start = md->phys_addr; unsigned long long size = md->num_pages << EFI_PAGE_SHIFT; diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 232f4915223b..1590e4d6a857 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -325,6 +326,55 @@ static __init int efivar_ssdt_load(void) static inline int efivar_ssdt_load(void) { return 0; } #endif +#ifdef CONFIG_DEBUG_FS + +#define EFI_DEBUGFS_MAX_BLOBS 32 + +static struct debugfs_blob_wrapper debugfs_blob[EFI_DEBUGFS_MAX_BLOBS]; + +static void __init efi_debugfs_init(void) +{ + struct dentry *efi_debugfs; + efi_memory_desc_t *md; + char name[32]; + int type_count[EFI_BOOT_SERVICES_DATA + 1] = {}; + int i = 0; + + efi_debugfs = debugfs_create_dir("efi", NULL); + if (IS_ERR_OR_NULL(efi_debugfs)) + return; + + for_each_efi_memory_desc(md) { + switch (md->type) { + case EFI_BOOT_SERVICES_CODE: + snprintf(name, sizeof(name), "boot_services_code%d", +type_count[md->type]++); + break; + case EFI_BOOT_SERVICES_DATA: + snprintf(name, sizeof(name), "boot_services_data%d", +type_count[md->type]++); + break; + default: + continue; + } + + debugfs_blob[i].size = md->num_pages << EFI_PAGE_SHIFT; + debugfs_blob[i].data = memremap(md->phys_addr, + debugfs_blob[i].size, + MEMREMAP_WB); + if (!debugfs_blob[i].data) + continue; + + debugfs_create_blob(name, 0400, efi_debugfs, &debugfs_blob[i]); + i++; + if (i == EFI_DEBUGFS_MAX_BLOBS) + break; + } +} +#else +static inline void efi_debugfs_init(void) {} +#endif + /* * We register the efi subsystem with the firmware subsystem and the * efivars subsystem with the efi subsystem, if the system was booted with @@ -369,6 +419,9 @@ static int __init efisubsys_init(void) goto err_remove_group; } + if (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS)) + efi_debugfs_init(); + return 0; err_remove_group: diff --git a/include/linux/efi.h b/include/linux/efi.h index f1b7d68ac460..791088360c1e 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1144,6 +1144,7 @@ extern int __init efi_setup_pcdp_console(char *); #define EFI_DBG8 /* Print additional debug info at runtime */ #define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */ #define EFI_MEM_ATTR 10 /* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */ +#define EFI_PRESERVE_BS_REGIONS11 /* A
Re: [PATCH v3] efi: Ignore unrealistically large option roms
On 29 April 2018 at 11:08, Hans de Goede wrote: > Hi, > > > On 29-04-18 11:07, Ard Biesheuvel wrote: >> >> On 29 April 2018 at 10:40, Hans de Goede wrote: >>> >>> Hi, >>> >>> >>> On 29-04-18 09:43, Ingo Molnar wrote: * Hans de Goede wrote: > diff --git a/arch/x86/boot/compressed/eboot.c > b/arch/x86/boot/compressed/eboot.c > index 47d3efff6805..8650ab268ee7 100644 > --- a/arch/x86/boot/compressed/eboot.c > +++ b/arch/x86/boot/compressed/eboot.c > @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, > struct pci_setup_rom **__rom) > if (status != EFI_SUCCESS) > return status; >- if (!pci->romimage || !pci->romsize) > + /* > +* Some firmwares contain EFI function pointers at the place > where the > +* romimage and romsize fields are supposed to be. Typically > the > EFI > +* code is mapped at high addresses, translating to an > unrealistically > +* large romsize. The UEFI spec limits the size of option ROMs > to > 16 > +* MiB so we reject any roms over 16 MiB in size to catch this. > +*/ > + if (!pci->romimage || !pci->romsize || pci->romsize > > 0x100) > return EFI_INVALID_PARAMETER; > size = pci->romsize + sizeof(*rom); > @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, > struct pci_setup_rom **__rom) > if (status != EFI_SUCCESS) > return status; >- if (!pci->romimage || !pci->romsize) > + /* > +* Some firmwares contain EFI function pointers at the place > where the > +* romimage and romsize fields are supposed to be. Typically > the > EFI > +* code is mapped at high addresses, translating to an > unrealistically > +* large romsize. The UEFI spec limits the size of option ROMs > to > 16 > +* MiB so we reject any roms over 16 MiB in size to catch this. > +*/ > + if (!pci->romimage || !pci->romsize || pci->romsize > > 0x100) > return EFI_INVALID_PARAMETER; Any reason why this couldn't be factored out into a efi_check_rom(pci) kind of helper function >>> >>> >>> >>> The pci pointer is of 2 different types: >>> >>> __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... >>> __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... >>> >>> I guess I could give the helper a romimage and romsize argument to get >>> around that. >>> Ard, do you want me to do a v4 with such a helper ? >>> >> >> I think Lukas has a point, although I shouldn't expect you to implement >> that. >> >> I will look into this myself, and rebase your patch on top of that. > > > Ok, thanks. > Actually, looking into this, I think I may have spotted a bug in this code. Does the issue you are solving here occur when running a 64-bit kernel on 32-bit EFI? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Ignore unrealistically large option roms
On 29 April 2018 at 12:00, Ard Biesheuvel wrote: > On 29 April 2018 at 11:08, Hans de Goede wrote: >> Hi, >> >> >> On 29-04-18 11:07, Ard Biesheuvel wrote: >>> >>> On 29 April 2018 at 10:40, Hans de Goede wrote: Hi, On 29-04-18 09:43, Ingo Molnar wrote: > > > > * Hans de Goede wrote: > >> diff --git a/arch/x86/boot/compressed/eboot.c >> b/arch/x86/boot/compressed/eboot.c >> index 47d3efff6805..8650ab268ee7 100644 >> --- a/arch/x86/boot/compressed/eboot.c >> +++ b/arch/x86/boot/compressed/eboot.c >> @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, >> struct pci_setup_rom **__rom) >> if (status != EFI_SUCCESS) >> return status; >>- if (!pci->romimage || !pci->romsize) >> + /* >> +* Some firmwares contain EFI function pointers at the place >> where the >> +* romimage and romsize fields are supposed to be. Typically >> the >> EFI >> +* code is mapped at high addresses, translating to an >> unrealistically >> +* large romsize. The UEFI spec limits the size of option ROMs >> to >> 16 >> +* MiB so we reject any roms over 16 MiB in size to catch this. >> +*/ >> + if (!pci->romimage || !pci->romsize || pci->romsize > >> 0x100) >> return EFI_INVALID_PARAMETER; >> size = pci->romsize + sizeof(*rom); >> @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, >> struct pci_setup_rom **__rom) >> if (status != EFI_SUCCESS) >> return status; >>- if (!pci->romimage || !pci->romsize) >> + /* >> +* Some firmwares contain EFI function pointers at the place >> where the >> +* romimage and romsize fields are supposed to be. Typically >> the >> EFI >> +* code is mapped at high addresses, translating to an >> unrealistically >> +* large romsize. The UEFI spec limits the size of option ROMs >> to >> 16 >> +* MiB so we reject any roms over 16 MiB in size to catch this. >> +*/ >> + if (!pci->romimage || !pci->romsize || pci->romsize > >> 0x100) >> return EFI_INVALID_PARAMETER; > > > > Any reason why this couldn't be factored out into a efi_check_rom(pci) > kind of > helper function The pci pointer is of 2 different types: __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... I guess I could give the helper a romimage and romsize argument to get around that. Ard, do you want me to do a v4 with such a helper ? >>> >>> I think Lukas has a point, although I shouldn't expect you to implement >>> that. >>> >>> I will look into this myself, and rebase your patch on top of that. >> >> >> Ok, thanks. >> > > Actually, looking into this, I think I may have spotted a bug in this code. > Does the issue you are solving here occur when running a 64-bit kernel > on 32-bit EFI? Never mind. I was looking at romimage being defined as a void* in efi_pci_io_protocol32, which is a bug, but the romsize field is unambiguously defined as a uint64_t, and this is the field holding the bogus value. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/4] Ignore unrealistically large option roms in EFI stub code
This is a continuation of Hans's work [0] to ignore bogus romimage/romsize values in the EFI PCI I/O protocol instances exposed by some UEFI firmwares on x86. I have only build tested this, both on 32 and 64 bit x86. Changes in v4: - Deduplicate the 32 and 64 bit code paths so that the actual change needs to be applied only once. This requires some preparatory work (#1, #2, #3), of which the first one should go to -stable. Changes in v3: - Limit the rom-size to 16MiB to match the EFI spec Changes in v2: - Add the check to both __setup_efi_pci32 and __setup_efi_pci64 instead of only to __setup_efi_pci64, some CHT devices which need this still use a 32 bit UEFI [0] https://marc.info/?l=linux-efi&m=152494632116321 Ard Biesheuvel (3): efi: fix efi_pci_io_protocol32 prototype for mixed mode efi: align efi_pci_io_protocol typedefs to type naming convention efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one Hans de Goede (1): efi/x86: Ignore unrealistically large option roms arch/x86/boot/compressed/eboot.c | 112 ++-- include/linux/efi.h | 14 +-- 2 files changed, 39 insertions(+), 87 deletions(-) -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] efi: fix efi_pci_io_protocol32 prototype for mixed mode
Mixed mode allows a kernel built for x86_64 to interact with 32-bit EFI firmware, but requires us to define all struct definitions carefully when it comes to pointer sizes. efi_pci_io_protocol32 currently uses a void* for the 'romimage' field, which will be interpreted as a 64-bit field on such kernels, potentially resulting in bogus memory references and subsequent crashes. Cc: Signed-off-by: Ard Biesheuvel --- arch/x86/boot/compressed/eboot.c | 6 -- include/linux/efi.h | 8 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 47d3efff6805..09f36c0d9d4f 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -163,7 +163,8 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) goto free_struct; - memcpy(rom->romdata, pci->romimage, pci->romsize); + memcpy(rom->romdata, (void *)(unsigned long)pci->romimage, + pci->romsize); return status; free_struct: @@ -269,7 +270,8 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) goto free_struct; - memcpy(rom->romdata, pci->romimage, pci->romsize); + memcpy(rom->romdata, (void *)(unsigned long)pci->romimage, + pci->romsize); return status; free_struct: diff --git a/include/linux/efi.h b/include/linux/efi.h index f1b7d68ac460..3016d8c456bc 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -395,8 +395,8 @@ typedef struct { u32 attributes; u32 get_bar_attributes; u32 set_bar_attributes; - uint64_t romsize; - void *romimage; + u64 romsize; + u32 romimage; } efi_pci_io_protocol_32; typedef struct { @@ -415,8 +415,8 @@ typedef struct { u64 attributes; u64 get_bar_attributes; u64 set_bar_attributes; - uint64_t romsize; - void *romimage; + u64 romsize; + u64 romimage; } efi_pci_io_protocol_64; typedef struct { -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] efi: align efi_pci_io_protocol typedefs to type naming convention
In order to use the helper macros that perform type mangling with the EFI PCI I/O protocol struct typedefs, align their Linux typenames with the convention we use for definitionns that originate in the UEFI spec, and add the trailing _t to each. Signed-off-by: Ard Biesheuvel --- arch/x86/boot/compressed/eboot.c | 8 include/linux/efi.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 09f36c0d9d4f..3994f48c4043 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -109,7 +109,7 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str) } static efi_status_t -__setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom) +__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom) { struct pci_setup_rom *rom = NULL; efi_status_t status; @@ -176,7 +176,7 @@ static void setup_efi_pci32(struct boot_params *params, void **pci_handle, unsigned long size) { - efi_pci_io_protocol_32 *pci = NULL; + efi_pci_io_protocol_32_t *pci = NULL; efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID; u32 *handles = (u32 *)(unsigned long)pci_handle; efi_status_t status; @@ -218,7 +218,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle, } static efi_status_t -__setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom) +__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom) { struct pci_setup_rom *rom; efi_status_t status; @@ -284,7 +284,7 @@ static void setup_efi_pci64(struct boot_params *params, void **pci_handle, unsigned long size) { - efi_pci_io_protocol_64 *pci = NULL; + efi_pci_io_protocol_64_t *pci = NULL; efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID; u64 *handles = (u64 *)(unsigned long)pci_handle; efi_status_t status; diff --git a/include/linux/efi.h b/include/linux/efi.h index 3016d8c456bc..56add823f190 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -397,7 +397,7 @@ typedef struct { u32 set_bar_attributes; u64 romsize; u32 romimage; -} efi_pci_io_protocol_32; +} efi_pci_io_protocol_32_t; typedef struct { u64 poll_mem; @@ -417,7 +417,7 @@ typedef struct { u64 set_bar_attributes; u64 romsize; u64 romimage; -} efi_pci_io_protocol_64; +} efi_pci_io_protocol_64_t; typedef struct { void *poll_mem; @@ -437,7 +437,7 @@ typedef struct { void *set_bar_attributes; uint64_t romsize; void *romimage; -} efi_pci_io_protocol; +} efi_pci_io_protocol_t; #define EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO 0x0001 #define EFI_PCI_IO_ATTRIBUTE_ISA_IO 0x0002 -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] efi/x86: Ignore unrealistically large option roms
From: Hans de Goede setup_efi_pci() tries to save a copy of each PCI option ROM as this may be necessary for the device driver for the PCI device to have access too. On some systems the efi_pci_io_protocol's romimage and romsize fields contain invalid data, which looks a bit like pointers pointing back into other EFI code or data. Interpreting these pointers as romsize leads to a very large value and if we then try to alloc this amount of memory to save a copy the alloc call fails. This leads to a "Failed to alloc mem for rom" error being printed on the EFI console for each PCI device. This commit avoids the printing of these errors, by checking romsize before doing the alloc and if it is larger then the EFI spec limit of 16 MiB silently ignore the ROM fields instead of trying to alloc mem and fail. Signed-off-by: Hans de Goede [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant] Signed-off-by: Ard Biesheuvel --- arch/x86/boot/compressed/eboot.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index dadf32312082..720b06e86698 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -123,10 +123,17 @@ __setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom) if (status != EFI_SUCCESS) return status; + /* +* Some firmwares contain EFI function pointers at the place where the +* romimage and romsize fields are supposed to be. Typically the EFI +* code is mapped at high addresses, translating to an unrealistically +* large romsize. The UEFI spec limits the size of option ROMs to 16 +* MiB so we reject any roms over 16 MiB in size to catch this. +*/ romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol, romimage, pci); romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci); - if (!romimage || !romsize) + if (!romimage || !romsize || romsize > SZ_16M) return EFI_INVALID_PARAMETER; size = romsize + sizeof(*rom); -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one
As suggested by Lukas, use his efi_call_proto() and efi_table_attr() macros to merge __setup_efi_pci32() and __setup_efi_pci64() into a single function, removing the need to duplicate changes made in subsequent patches across both. Cc: Lukas Wunner Signed-off-by: Ard Biesheuvel --- arch/x86/boot/compressed/eboot.c | 107 +--- 1 file changed, 25 insertions(+), 82 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 3994f48c4043..dadf32312082 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -109,23 +109,27 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str) } static efi_status_t -__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom) +__setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom) { struct pci_setup_rom *rom = NULL; efi_status_t status; unsigned long size; - uint64_t attributes; + uint64_t attributes, romsize; + void *romimage; - status = efi_early->call(pci->attributes, pci, -EfiPciIoAttributeOperationGet, 0, 0, -&attributes); + status = efi_call_proto(efi_pci_io_protocol, attributes, pci, + EfiPciIoAttributeOperationGet, 0, 0, + &attributes); if (status != EFI_SUCCESS) return status; - if (!pci->romimage || !pci->romsize) + romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol, +romimage, pci); + romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci); + if (!romimage || !romsize) return EFI_INVALID_PARAMETER; - size = pci->romsize + sizeof(*rom); + size = romsize + sizeof(*rom); status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom); if (status != EFI_SUCCESS) { @@ -141,30 +145,32 @@ __setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom) rom->pcilen = pci->romsize; *__rom = rom; - status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16, -PCI_VENDOR_ID, 1, &(rom->vendor)); + status = efi_call_proto(efi_pci_io_protocol, pci.read, pci, + EfiPciIoWidthUint16, PCI_VENDOR_ID, 1, + &rom->vendor); if (status != EFI_SUCCESS) { efi_printk(sys_table, "Failed to read rom->vendor\n"); goto free_struct; } - status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16, -PCI_DEVICE_ID, 1, &(rom->devid)); + status = efi_call_proto(efi_pci_io_protocol, pci.read, pci, + EfiPciIoWidthUint16, PCI_DEVICE_ID, 1, + &rom->devid); if (status != EFI_SUCCESS) { efi_printk(sys_table, "Failed to read rom->devid\n"); goto free_struct; } - status = efi_early->call(pci->get_location, pci, &(rom->segment), -&(rom->bus), &(rom->device), &(rom->function)); + status = efi_call_proto(efi_pci_io_protocol, get_location, pci, + &rom->segment, &rom->bus, &rom->device, + &rom->function); if (status != EFI_SUCCESS) goto free_struct; - memcpy(rom->romdata, (void *)(unsigned long)pci->romimage, - pci->romsize); + memcpy(rom->romdata, romimage, romsize); return status; free_struct: @@ -176,7 +182,7 @@ static void setup_efi_pci32(struct boot_params *params, void **pci_handle, unsigned long size) { - efi_pci_io_protocol_32_t *pci = NULL; + efi_pci_io_protocol_t *pci = NULL; efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID; u32 *handles = (u32 *)(unsigned long)pci_handle; efi_status_t status; @@ -203,7 +209,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle, if (!pci) continue; - status = __setup_efi_pci32(pci, &rom); + status = __setup_efi_pci(pci, &rom); if (status != EFI_SUCCESS) continue; @@ -217,74 +223,11 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle, } } -static efi_status_t -__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom) -{ - struct pci_setup_rom *rom; - efi_status_t status; - unsigned long size; - uint64_t attributes; - - status = efi_early->call(pci->attributes, pci, -EfiPciIoAttributeOperationGet, 0, -&attributes); - if (status != EFI_SUCCE
RE: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration table
> -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Thursday, April 26, 2018 2:12 PM > To: Grant Likely > Cc: Udit Kumar ; Leif Lindholm > ; Pankaj Bansal ; > mark.rutl...@arm.com; linux-efi@vger.kernel.org; Varun Sethi > ; Wasim Khan ; n...@arm.com; Rod > Dorris > Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in > configuration > table > > On 26 April 2018 at 10:37, Grant Likely wrote: > > On 25/04/2018 15:48, Udit Kumar wrote: > >> > >> > >> > >>> -Original Message- > >>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] > >>> Sent: Wednesday, April 25, 2018 4:20 PM > >>> To: Grant Likely > >>> Cc: Udit Kumar ; Ard Biesheuvel > >>> ; Pankaj Bansal ; > >>> mark.rutl...@arm.com; linux-efi@vger.kernel.org; Varun Sethi > >>> ; Wasim Khan ; n...@arm.com; > Rod > >>> Dorris > >>> Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in > >>> configuration table > >>> > >>> On Wed, Apr 25, 2018 at 11:04:13AM +0100, Grant Likely wrote: > > On 25/04/2018 07:49, Udit Kumar wrote: > >> > >> 2) Kernel provides/overrides DTB > >> The kernel always has the option of loading it's own DTB, either > >> because firmware doesn't provide one, or it needs a newer DTB. > >> This is similar to CONFIG_ACPI_CUSTOM_DSDT in ACPI land. However, > >> if the kernel is overriding the DTB, then firmware has no part of > >> it, and it should not be allowed to modify the DTB at > >> ExitBootServices() > >>> > >>> time. > > > > > > For our platforms at least, this will not work unless firmware is > > doing modifications. I think this I likely true for Ard as well. > > Firmware has a logic to modify default DTB for available devices, > > reading board configuration programming required muxes, based upon > > this removing/adding devices into DTB. > > IMO, this will not be an optimal way to use untouched DTB provided > > by > >>> > >>> kernel. > > > Right, which is part of the reason for insisting on the firmare > being responsible. If the kernel or Grub overrides the DTB, then it > is an explicit > *rejection* of anything firmware provides; presumable because > something is broken and it has to be worked around. > >>> > >>> > >>> So, the functionality in GRUB to replace a firmware-provided > >>> devicetree needs to go. And dtb= loading should ideally be made > >>> dependent on some sort of DEBUG config option. > >> > >> > >> In my view , dtb= sort of option should go away. If we are booting > >> some debug image in development environment then why not to re-flash > >> DTB > > > > > > I don't want to drop the prototyping use cases on the floor. ie. > > Wiring up a development board to external hardware. The development > > board is already supported, but the user is going to be experimenting > > with DT changes as they go. Reflashing the DT is not a good option in > > this case. It is more usefule to be able to load the DT modifications > > at runtime before booting the kernel, because they will change every boot. > > > > I can see this scenario playing out for both full DT replacement and > > DTBO scenarios. > > > > This applies mainly to prototyping under the OS, right? > > One aspect we did not consider yet I think is the use of DT to describe the > platform to ARM-TF and UEFI itself. I expect this to become more common > going forward, and will make replacing just the DTB even trickier. Should we keep the dtb for UEFI and OS separate or not? The bootloader is expected to control only those devices from which it expects to get the OS to boot. The dtb file for bootloader will contain only these devices' nodes, while the dtb passed to OS will contain list of all devices that are available on a platform. > > So overriding the DTB like we allow for DSDT makes perfect sense, but as a > development feature only. I still think the idea of just dropping an updated > DTB > into an existing firmware build consisting of ARM-TF, UEFI etc (and perhaps > even > OP-TEE?) is not something that is going to be feasible in general. N�r��yb�X��ǧv�^�){.n�+{�y^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�