Re: [PATCH v6 1/3] efi_loader: Add check for event log passed from firmware
Hi Heinrich, > > > + > > + /* > > +* initialize log area as 0xff so the OS can easily figure out the > > +* last log entry > > Where is this standardized? > > EFI_TCG2_PROTOCOL.GetEventLog() returns EventLogLastEntry to indicate > the last log entry. > Indeed. However while I was looking into EDK2 code I found a similar comment [1]. Since EDK2 is used to boot windows as well I assumed that was a good policy to keep. > > +*/ > > + memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE); > > + > > + /* > > +* The log header is defined to be in SHA1 event log entry format. > > +* Setup event header > > +*/ [...] [1] https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c#L1578 Cheers /Ilias
Re: [PATCH v3 2/2] usb: gadget: Add CDC ACM function
Hi Pali, On Thu, 25 Nov 2021 at 19:10, Pali Rohár wrote: > > On Thursday 25 November 2021 18:16:15 Loic Poulain wrote: > > Add support for CDC ACM using the new UDC and gadget API. This protocol > > can be used for serial over USB data transfer and is widely supported > > by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of > > such link is to access device debug console and can be useful for > > products not exposing regular UART to the user. > > > > A default stdio device named 'usbacm' is created, and can be used > > to redirect console to USB link over CDC ACM: > > > > > setenv stdin usbacm; setenv stdout usbacm > > > > Signed-off-by: Loic Poulain > > --- > > v2: - remove possible infinite recursipe print loop > > - Remove acmconsole command, start function along the stdio device > > v3: - Use __constant_cpu_to_le16() when possible > > - Rename stdio dev to 'usbacm' > > - Rename init function to drv_usbacm_init > > Hello! Just one question: is in this v3 patch fixed stdout-only > configuration, or it still has a bug which you described in email? > https://lore.kernel.org/u-boot/CAMZdPi8185pAiO4w2QYMtWGAFJiX=ej_bjw1cusre990wr7...@mail.gmail.com/ > > Because neither in above change long nor in driver help description nor > in driver source code is any comment related to stdout-only device. No, actually it ended not being an issue, we can have it stdout/stdin independently. The driver loops on 'irq'-handling to ensure the USB controller has completely flushed TX before returning. > > Note that stdout-only does not have to be too uncommon. For example > Nokia N900 has enabled UART, keyboard+lcd and usbtty by default and > sometimes I enabled input only from keyboard and looked at usbtty > terminal (as stdout-only) if everything is working fine (that every > pressed key on keyboard was echoed correctly back to usbtty). Yes, checked this case on a imx7d based board and stdout-only works as you describe. Regards, Loic
Re: [PATCH v5 3/3] mtd: spi-nor-core: Add support for Macronix Octal flash
Hi Tudor > > On 11/18/21 12:13 PM, JaimeLiao wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > Adding Macronix Octal flash for Octal DTR support. > > > > The octaflash series can be divided into the following types: > > > > MX25 series : Serial NOR Flash. > > MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb) > > LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation. > > LW/UW series : Support simultaneous Read-while-Write operation in multiple > >bank architecture. Read-while-write feature which means read > >data one bank while another bank is programing or erasing. > > > > MX25LM : 3.0V Octal I/O > > > > -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf > > > > MX25UM : 1.8V Octal I/O > > > > -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7525/MX25UM51245G%20Extreme%20Speed,%201.8V,%20512Mb,%20v1.0.pdf > > > > MX66LM : 3.0V Octal I/O with stacked die > > > > -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf > > > > MX66UM : 1.8V Octal I/O with stacked die > > > > -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7721/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf > > > > MX25LW : 3.0V Octal I/O with Read-while-Write > > MX25UW : 1.8V Octal I/O with Read-while-Write > > MX66LW : 3.0V Octal I/O with Read-while-Write and stack die > > MX66UW : 1.8V Octal I/O with Read-while-Write and stack die > > > > About LW/UW series, please contact us freely if you have any > > questions. For adding Octal NOR Flash IDs, we have validated > > each Flash on plateform zynq-picozed. > > > > Signed-off-by: JaimeLiao > > --- > > drivers/mtd/spi/spi-nor-ids.c | 22 +- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c > > index cb3a08872d..5c13ea3a78 100644 > > --- a/drivers/mtd/spi/spi-nor-ids.c > > +++ b/drivers/mtd/spi/spi-nor-ids.c > > @@ -169,7 +169,27 @@ const struct flash_info spi_nor_ids[] = { > > { INFO("mx66l1g45g", 0xc2201b, 0, 64 * 1024, 2048, SECT_4K | > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > { INFO("mx25l1633e", 0xc22415, 0, 64 * 1024, 32, > > SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) }, > > { INFO("mx25r6435f", 0xc22817, 0, 64 * 1024, 128, SECT_4K) }, > > - { INFO("mx66uw2g345g", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66uw2g345gx0", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66lm1g45g",0xc2853b, 0, 32 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > which one of all these flashes (including the ones from below) support > SFDP and which not? Which can discover the octal dtr method by parsing > SFDP and which not? Depending on these those flash_info flags may change. I agree that we need a method for octal dtr support on flash which didn't include SFDP but the patch of method is separate with Octal DTR patch. > Why do you need SPI_NOR_4B_OPCODES, can't this support be discovered > when parsing SFDP? How about SECT_4K? > > I know for sure there are variants of mx66lm1g45g that do not support > SFDP, and flavors that do support SFDP. How you'll differentiate between > the two flavors of the same flash? > > I chose to SKIP SFDP parsing for mx66lm1g45g as there's no infrastructure > to handle its case, neither in linux, nor u-boot. > Here's what I proposed for now: > https://lore.kernel.org/all/20211103234950.202289-3-tudor.amba...@microchip.com/ > > > Cheers, > ta > > > + { INFO("mx25lm51245g", 0xc2853a, 0, 16 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx25lw51245g", 0xc2863a, 0, 16 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx25lm25645g", 0xc28539, 0, 8 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66um2g45g",0xc2803c, 0, 64 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66uw2g345g", 0xc2843c, 0, 64 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66um1g45g",0xc2803b, 0, 32 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx66uw1g45g",0xc2813b, 0, 32 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx25um51245g", 0xc2803a, 0, 16 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx25uw51245g", 0xc2813a, 0, 16 * 1024, 4096, SECT_4K | > > SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) }, > > + { INFO("mx25uw51345g"
[PATCH] board: ti: am43xx: pass boot device information from SPL to U-Boot proper
TI AM43xx SoC supports various boot devices (peripherals). There is already handoff mechanism prepared to allow passing the information which boot device was used to load the SPL. Use the handoff mechanism to pass this information to U-Boot proper and set the "boot_device" environment variable in board_late_init. Signed-off-by: Josef Lusticky Cc: Tom Rini Cc: Lokesh Vutla Cc: Michael Trimarchi --- I use the boot_device variable later in U-Boot scripting - e.g. to avoid running bootcmd when the SPL was loaded from UART but run it when loaded from MMC. Only AM43xx is supported by this patch, but for other TI SoCs the procedure should be the same: - figure out supported boot devices from arch/arm/include/asm/arch-am33xx/spl.h or arch/arm/include/asm/arch-omapX/spl.h - implement setting the boot_device env variable in board_late_init() You'll need to enable the following in the config: CONFIG_BLOBLIST=y (required by CONFIG_HANDOFF) CONFIG_HANDOFF=y CONFIG_BLOBLIST_ADDR=0x8700 (i set this based on other values defined by the DEFAULT_LINUX_BOOT_ENV macro in include/configs/ti_armv7_common.h, you may want to use a different address) arch/arm/include/asm/handoff.h| 3 +++ arch/arm/mach-omap2/boot-common.c | 9 board/ti/am43xx/board.c | 38 +++ 3 files changed, 50 insertions(+) diff --git a/arch/arm/include/asm/handoff.h b/arch/arm/include/asm/handoff.h index 0790d2ab1e..1b7aa432a2 100644 --- a/arch/arm/include/asm/handoff.h +++ b/arch/arm/include/asm/handoff.h @@ -16,6 +16,9 @@ */ struct arch_spl_handoff { ulong usable_ram_top; +#ifdef CONFIG_ARCH_OMAP2PLUS + u32 omap_boot_device; +#endif }; #endif diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index 1268a32503..191bb2a42d 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -236,3 +236,12 @@ void arch_preboot_os(void) ahci_reset((void __iomem *)DWC_AHSATA_BASE); } #endif + +#if CONFIG_IS_ENABLED(HANDOFF) +int handoff_arch_save(struct spl_handoff *ho) +{ + ho->arch.omap_boot_device = spl_boot_device(); + + return 0; +} +#endif diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c index a71b588efc..8c5834fc25 100644 --- a/board/ti/am43xx/board.c +++ b/board/ti/am43xx/board.c @@ -726,6 +726,44 @@ static int device_okay(const char *path) int board_late_init(void) { struct udevice *dev; + +#if CONFIG_IS_ENABLED(HANDOFF) + /* Read peripheral SPL was loaded from */ + if (gd->spl_handoff) { + switch (gd->spl_handoff->arch.omap_boot_device) { + case BOOT_DEVICE_CPGMAC: + env_set("boot_device", "cpgmac"); + break; + case BOOT_DEVICE_MMC1: + env_set("boot_device", "mmc1"); + break; + case BOOT_DEVICE_MMC2: + env_set("boot_device", "mmc2"); + break; + case BOOT_DEVICE_NAND: + env_set("boot_device", "nand"); + break; + case BOOT_DEVICE_NOR: + env_set("boot_device", "nor"); + break; + case BOOT_DEVICE_SPI: + env_set("boot_device", "spi"); + break; + case BOOT_DEVICE_UART: + env_set("boot_device", "uart"); + break; + case BOOT_DEVICE_USB: + env_set("boot_device", "usb"); + break; + case BOOT_DEVICE_USBETH: + env_set("boot_device", "usbeth"); + break; + default: + env_set("boot_device", "unknown"); + } + } +#endif + #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG set_board_info_env(NULL); -- 2.30.2
Re: [PATCH v3 2/2] usb: gadget: Add CDC ACM function
On Friday 26 November 2021 09:39:18 Loic Poulain wrote: > Hi Pali, > > On Thu, 25 Nov 2021 at 19:10, Pali Rohár wrote: > > > > On Thursday 25 November 2021 18:16:15 Loic Poulain wrote: > > > Add support for CDC ACM using the new UDC and gadget API. This protocol > > > can be used for serial over USB data transfer and is widely supported > > > by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of > > > such link is to access device debug console and can be useful for > > > products not exposing regular UART to the user. > > > > > > A default stdio device named 'usbacm' is created, and can be used > > > to redirect console to USB link over CDC ACM: > > > > > > > setenv stdin usbacm; setenv stdout usbacm > > > > > > Signed-off-by: Loic Poulain > > > --- > > > v2: - remove possible infinite recursipe print loop > > > - Remove acmconsole command, start function along the stdio device > > > v3: - Use __constant_cpu_to_le16() when possible > > > - Rename stdio dev to 'usbacm' > > > - Rename init function to drv_usbacm_init > > > > Hello! Just one question: is in this v3 patch fixed stdout-only > > configuration, or it still has a bug which you described in email? > > https://lore.kernel.org/u-boot/CAMZdPi8185pAiO4w2QYMtWGAFJiX=ej_bjw1cusre990wr7...@mail.gmail.com/ > > > > Because neither in above change long nor in driver help description nor > > in driver source code is any comment related to stdout-only device. > > No, actually it ended not being an issue, we can have it stdout/stdin > independently. The driver loops on 'irq'-handling to ensure the USB > controller has completely flushed TX before returning. This is probably suboptimal, but important is that there is no such issue. So it means that this new code can be replacement for old usbtty, which is really nice!! > > > > Note that stdout-only does not have to be too uncommon. For example > > Nokia N900 has enabled UART, keyboard+lcd and usbtty by default and > > sometimes I enabled input only from keyboard and looked at usbtty > > terminal (as stdout-only) if everything is working fine (that every > > pressed key on keyboard was echoed correctly back to usbtty). > > Yes, checked this case on a imx7d based board and stdout-only works as > you describe. > > Regards, > Loic
Re: [PATCH v6 1/3] efi_loader: Add check for event log passed from firmware
Hi Heinrich, On Fri, 26 Nov 2021 at 13:01, Heinrich Schuchardt wrote: > On 11/26/21 06:00, Ruchika Gupta wrote: > > Platforms may have support to measure their initial firmware components > > and pass the event log to u-boot. The event log address can be passed > > in property tpm_event_log_addr and tpm_event_log_size of the tpm node. > > Platforms may choose their own specific mechanism to do so. A weak > > function is added to check if even log has been passed to u-boot > > from earlier firmware components. If available, the eventlog is parsed > > to check for its correctness and further event logs are appended to the > > passed log. > > > > Signed-off-by: Ruchika Gupta > > Tested-by: Ilias Apalodimas > > Reviewed-by: Ilias Apalodimas > > --- > > v6: No change > > > > v5: > > Shift the efi_init_event_log() to a different location in the file. > > This help fixes compilation issue introduced by calling > efi_append_scrtm_version() > > from it. > > > > v4: > > Add SCRTM version to log only if previous firmware doesn't pass the > eventlog > > > > v3: > > Return as soon as you detect error > > > > v2: > > Moved firmware eventlog code parsing to tcg2_get_fw_eventlog() > > > > lib/efi_loader/efi_tcg2.c | 438 -- > > 1 file changed, 369 insertions(+), 69 deletions(-) > > > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > > index 8c1f22e337..a789c44660 100644 > > --- a/lib/efi_loader/efi_tcg2.c > > +++ b/lib/efi_loader/efi_tcg2.c > > @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct > udevice **dev) > > return EFI_NOT_FOUND; > > } > > > > +/** > > + * platform_get_eventlog() - retrieve the eventlog address and size > > + * > > + * This function retrieves the eventlog address and size if the > underlying > > + * firmware has done some measurements and passed them. > > + * > > + * This function may be overridden based on platform specific method of > > + * passing the eventlog address and size. > > + * > > + * @dev: udevice > > + * @addr:eventlog address > > + * @sz: eventlog size > > + * Return: status code > > + */ > > +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 > *addr, > > + u32 *sz) > > This function must be declared in a header to be overridden. > > > +{ > > + const u64 *basep; > > + const u32 *sizep; > > + > > + basep = dev_read_prop(dev, "tpm_event_log_addr", NULL); > > + if (!basep) > > + return EFI_NOT_FOUND; > > + > > + *addr = be64_to_cpup((__force __be64 *)basep); > > + > > + sizep = dev_read_prop(dev, "tpm_event_log_size", NULL); > > + if (!sizep) > > + return EFI_NOT_FOUND; > > + > > + *sz = be32_to_cpup((__force __be32 *)sizep); > > + if (*sz == 0) { > > + log_debug("event log empty\n"); > > + return EFI_NOT_FOUND; > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > /** > >* tpm2_get_max_command_size() - get the supported max command size > >* > > @@ -1181,6 +1220,250 @@ static const struct efi_tcg2_protocol > efi_tcg2_protocol = { > > .get_result_of_set_active_pcr_banks = > efi_tcg2_get_result_of_set_active_pcr_banks, > > }; > > > > +/** > > + * parse_event_log_header() - Parse and verify the event log header > fields > > + * > > + * @buffer: Pointer to the event header > > + * @size:Size of the eventlog > > + * @pos: Position in buffer after event log header > > + * > > + * Return: status code > > + */ > > +efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos) > > This function should be declared in a header or be static. > > Should buffer have type struct tcg_pcr_event *? > Since buffer points to the complete eventlog, it would be probably better to keep it as it is i.e void *. I will correct the description of this parameter in the function description to avoid confusion. > > +{ > > + struct tcg_pcr_event *event_header = (struct tcg_pcr_event > *)buffer; > > + int i = 0; > > + > > + if (size < sizeof(*event_header)) > > + return EFI_COMPROMISED_DATA; > > + > > + if (get_unaligned_le32(&event_header->pcr_index) != 0 || > > + get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION) > > + return EFI_COMPROMISED_DATA; > > + > > + for (i = 0; i < sizeof(event_header->digest); i++) { > > + if (event_header->digest[i] != 0) > > if (event_header->digest[i]) > > > + return EFI_COMPROMISED_DATA; > > + } > > + > > + *pos += sizeof(*event_header); > > Do you re > Probably you are asking about parameter pos here. pos points to the offset in the buffer where the next event starts (after the event header) > > > + > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + * parse_specid_event() - Parse and verify the specID Event in the > ev
[PATCH u-boot-next 00/12] Common U-Boot macros for PCI Configuration Mechanism #1
This patch series add new U-Boot macros for composing Config Address for PCI Configuration Mechanism #1 as defined in PCI Local Bus Specification, including extended variant (which do not have any formal specification) and then use these new macros (PCI_CONF1_ADDRESS and PCI_CONF1_EXT_ADDRESS) in all PCI drivers that use Config Address according to PCI Configuration Mechanism #1. PCI Configuration Mechanism #1 was originally specified for x86 platforms and it is still supported on x86 together with PCIe ECAM. Nowadays lot of non-ECAM-compliant ARM PCIe controllers use only extended variant of this address construction and some of them requires cleared Enable bit. Extended variant is also supported by x86 AMD Barcelona (and new) CPUs, but U-Boot code does not provide this support yet. Note that it exists also PCI Configuration Mechanism #2, but this one was removed from PCI Local Bus Specification revision 3.0 and seems that it is not used by any PCI driver in U-Boot. So I have not added macros for this mechanism in this patch series. Because lot of hardware still use this (rather old) mechanism, relevant U-Boot PCI and PCIe drivers have own ad-hoc code address construction, which is repeated and written in different ways. This patch series is code cleanup of these PCIe drivers to use common U-Boot macros for PCI Configuration Mechanism #1. The last change is fixing construction of Config Address in PCI driver sh7751. Construction with this change matches Linux kernel code and also U-Boot code prior commit mentioned in commit message. I have tested this patch series only for PCI mvebu driver on A385 board and it is working fine. So Please properly review all other changes. Ideally test them. I have run CI tests with these changes on github and everything passed: https://github.com/u-boot/u-boot/pull/101 Please let me know what do you think about this change. Pali Rohár (12): pci: Add standard PCI Config Address macros pci: gt64120: Use PCI_CONF1_ADDRESS() macro pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro pci: msc01: Use PCI_CONF1_ADDRESS() macro pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro pci: sh7780: Use PCI_CONF1_ADDRESS() macro x86: pci: Use PCI_CONF1_ADDRESS() macro m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro arch/m68k/cpu/mcf5445x/pci.c | 7 +++--- arch/x86/cpu/pci.c | 4 ++-- drivers/pci/pci_gt64120.c| 7 ++ drivers/pci/pci_mpc85xx.c| 4 ++-- drivers/pci/pci_msc01.c | 7 ++ drivers/pci/pci_mvebu.c | 17 -- drivers/pci/pci_sh7751.c | 29 ++- drivers/pci/pci_sh7780.c | 8 +++ drivers/pci/pci_tegra.c | 11 +++-- drivers/pci/pcie_fsl.c | 10 drivers/pci/pcie_mediatek.c | 17 +++--- include/gt64120.h| 12 -- include/msc01.h | 9 include/pci.h| 45 14 files changed, 83 insertions(+), 104 deletions(-) -- 2.20.1
[PATCH u-boot-next 01/12] pci: Add standard PCI Config Address macros
Lot of PCI and PCIe controllers are using standard Config Address for PCI Configuration Mechanism #1 or its extended version. So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's pci.h header file which can be suitable for most PCI and PCIe controller drivers. Drivers do not have to invent their own macros and can use these new U-Boot macros. Signed-off-by: Pali Rohár --- include/pci.h | 45 + 1 file changed, 45 insertions(+) diff --git a/include/pci.h b/include/pci.h index 6c1094d72998..0ea41a7e1ba2 100644 --- a/include/pci.h +++ b/include/pci.h @@ -522,6 +522,51 @@ #include +/* + * Config Address for PCI Configuration Mechanism #1 + * + * See PCI Local Bus Specification, Revision 3.0, + * Section 3.2.2.3.2, Figure 3-2, p. 50. + */ + +#define PCI_CONF1_BUS_SHIFT16 /* Bus number */ +#define PCI_CONF1_DEV_SHIFT11 /* Device number */ +#define PCI_CONF1_FUNC_SHIFT 8 /* Function number */ + +#define PCI_CONF1_BUS_MASK 0xff +#define PCI_CONF1_DEV_MASK 0x1f +#define PCI_CONF1_FUNC_MASK0x7 +#define PCI_CONF1_REG_MASK 0xfc /* Limit aligned offset to a maximum of 256B */ + +#define PCI_CONF1_ENABLE BIT(31) +#define PCI_CONF1_BUS(x) (((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT) +#define PCI_CONF1_DEV(x) (((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT) +#define PCI_CONF1_FUNC(x) (((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT) +#define PCI_CONF1_REG(x) ((x) & PCI_CONF1_REG_MASK) + +#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \ + (PCI_CONF1_ENABLE | \ +PCI_CONF1_BUS(bus) | \ +PCI_CONF1_DEV(dev) | \ +PCI_CONF1_FUNC(func) | \ +PCI_CONF1_REG(reg)) + +/* + * Extension of PCI Config Address for accessing extended PCIe registers + * + * No standardized specification, but used on lot of non-ECAM-compliant ARM SoCs + * or on AMD Barcelona and new CPUs. Reserved bits [27:24] of PCI Config Address + * are used for specifying additional 4 high bits of PCI Express register. + */ + +#define PCI_CONF1_EXT_REG_SHIFT16 +#define PCI_CONF1_EXT_REG_MASK 0xf00 +#define PCI_CONF1_EXT_REG(x) (((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT) + +#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \ + (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \ +PCI_CONF1_EXT_REG(reg)) + /* * Enhanced Configuration Access Mechanism (ECAM) * -- 2.20.1
[PATCH u-boot-next 03/12] pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro
PCI mpc85xx driver uses extended format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_EXT_ADDRESS(). Signed-off-by: Pali Rohár --- drivers/pci/pci_mpc85xx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c index 574cb784a893..1e180ee289b0 100644 --- a/drivers/pci/pci_mpc85xx.c +++ b/drivers/pci/pci_mpc85xx.c @@ -23,7 +23,7 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf, struct mpc85xx_pci_priv *priv = dev_get_priv(dev); u32 addr; - addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x8000; + addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); out_be32(priv->cfg_addr, addr); sync(); *value = pci_conv_32_to_size(in_le32(priv->cfg_data), offset, size); @@ -38,7 +38,7 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf, struct mpc85xx_pci_priv *priv = dev_get_priv(dev); u32 addr; - addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x8000; + addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); out_be32(priv->cfg_addr, addr); sync(); out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size)); -- 2.20.1
[PATCH u-boot-next 04/12] pci: msc01: Use PCI_CONF1_ADDRESS() macro
PCI msc01 driver uses standard format of Config Address for PCI Configuration Mechanism #1 but with cleared Enable bit. So use new U-Boot macro PCI_CONF1_ADDRESS() with clearing PCI_CONF1_ENABLE bit and remove old custom driver address macros. Signed-off-by: Pali Rohár --- drivers/pci/pci_msc01.c | 7 ++- include/msc01.h | 9 - 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/pci/pci_msc01.c b/drivers/pci/pci_msc01.c index 2f1b688fc321..8d363d60498b 100644 --- a/drivers/pci/pci_msc01.c +++ b/drivers/pci/pci_msc01.c @@ -34,16 +34,13 @@ static int msc01_config_access(struct msc01_pci_controller *msc01, void *cfgdata = msc01->base + MSC01_PCI_CFGDATA_OFS; unsigned int bus = PCI_BUS(bdf); unsigned int dev = PCI_DEV(bdf); - unsigned int devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf); + unsigned int func = PCI_FUNC(bdf); /* clear abort status */ __raw_writel(aborts, intstat); /* setup address */ - __raw_writel((bus << MSC01_PCI_CFGADDR_BNUM_SHF) | -(dev << MSC01_PCI_CFGADDR_DNUM_SHF) | -(devfn << MSC01_PCI_CFGADDR_FNUM_SHF) | -((where / 4) << MSC01_PCI_CFGADDR_RNUM_SHF), + __raw_writel((PCI_CONF1_ADDRESS(bus, dev, func, where) & ~PCI_CONF1_ENABLE), msc01->base + MSC01_PCI_CFGADDR_OFS); /* perform access */ diff --git a/include/msc01.h b/include/msc01.h index ec18a724eb93..20158123494a 100644 --- a/include/msc01.h +++ b/include/msc01.h @@ -71,15 +71,6 @@ #define MSC01_PCI_INTSTAT_MA_SHF 7 #define MSC01_PCI_INTSTAT_MA_MSK (0x1 << MSC01_PCI_INTSTAT_MA_SHF) -#define MSC01_PCI_CFGADDR_BNUM_SHF 16 -#define MSC01_PCI_CFGADDR_BNUM_MSK (0xff << MSC01_PCI_CFGADDR_BNUM_SHF) -#define MSC01_PCI_CFGADDR_DNUM_SHF 11 -#define MSC01_PCI_CFGADDR_DNUM_MSK (0x1f << MSC01_PCI_CFGADDR_DNUM_SHF) -#define MSC01_PCI_CFGADDR_FNUM_SHF 8 -#define MSC01_PCI_CFGADDR_FNUM_MSK (0x3 << MSC01_PCI_CFGADDR_FNUM_SHF) -#define MSC01_PCI_CFGADDR_RNUM_SHF 2 -#define MSC01_PCI_CFGADDR_RNUM_MSK (0x3f << MSC01_PCI_CFGADDR_RNUM_SHF) - #define MSC01_PCI_HEAD0_VENDORID_SHF 0 #define MSC01_PCI_HEAD0_DEVICEID_SHF 16 -- 2.20.1
[PATCH u-boot-next 02/12] pci: gt64120: Use PCI_CONF1_ADDRESS() macro
PCI gt64120 driver uses standard format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS() and remove old custom driver address macros. Signed-off-by: Pali Rohár --- drivers/pci/pci_gt64120.c | 7 ++- include/gt64120.h | 12 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/pci/pci_gt64120.c b/drivers/pci/pci_gt64120.c index 153c65b119a4..2c2a80eeaa06 100644 --- a/drivers/pci/pci_gt64120.c +++ b/drivers/pci/pci_gt64120.c @@ -48,7 +48,7 @@ static int gt_config_access(struct gt64120_pci_controller *gt, { unsigned int bus = PCI_BUS(bdf); unsigned int dev = PCI_DEV(bdf); - unsigned int devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf); + unsigned int func = PCI_FUNC(bdf); u32 intr; u32 addr; u32 val; @@ -65,10 +65,7 @@ static int gt_config_access(struct gt64120_pci_controller *gt, /* Clear cause register bits */ writel(~GT_INTRCAUSE_ABORT_BITS, >->regs->intrcause); - addr = GT_PCI0_CFGADDR_CONFIGEN_BIT; - addr |= bus << GT_PCI0_CFGADDR_BUSNUM_SHF; - addr |= devfn << GT_PCI0_CFGADDR_FUNCTNUM_SHF; - addr |= (where / 4) << GT_PCI0_CFGADDR_REGNUM_SHF; + addr = PCI_CONF1_ADDRESS(bus, dev, func, where); /* Setup address */ writel(addr, >->regs->pci0_cfgaddr); diff --git a/include/gt64120.h b/include/gt64120.h index 0b577f3f44b9..b58afe3c4afe 100644 --- a/include/gt64120.h +++ b/include/gt64120.h @@ -491,18 +491,6 @@ #define GT_INTRCAUSE_TARABORT0_BIT GT_INTRCAUSE_TARABORT0_MSK -#define GT_PCI0_CFGADDR_REGNUM_SHF 2 -#define GT_PCI0_CFGADDR_REGNUM_MSK (MSK(6) << GT_PCI0_CFGADDR_REGNUM_SHF) -#define GT_PCI0_CFGADDR_FUNCTNUM_SHF 8 -#define GT_PCI0_CFGADDR_FUNCTNUM_MSK (MSK(3) << GT_PCI0_CFGADDR_FUNCTNUM_SHF) -#define GT_PCI0_CFGADDR_DEVNUM_SHF 11 -#define GT_PCI0_CFGADDR_DEVNUM_MSK (MSK(5) << GT_PCI0_CFGADDR_DEVNUM_SHF) -#define GT_PCI0_CFGADDR_BUSNUM_SHF 16 -#define GT_PCI0_CFGADDR_BUSNUM_MSK (MSK(8) << GT_PCI0_CFGADDR_BUSNUM_SHF) -#define GT_PCI0_CFGADDR_CONFIGEN_SHF 31 -#define GT_PCI0_CFGADDR_CONFIGEN_MSK (MSK(1) << GT_PCI0_CFGADDR_CONFIGEN_SHF) -#define GT_PCI0_CFGADDR_CONFIGEN_BIT GT_PCI0_CFGADDR_CONFIGEN_MSK - #define GT_PCI0_CMD_MBYTESWAP_SHF 0 #define GT_PCI0_CMD_MBYTESWAP_MSK (MSK(1) << GT_PCI0_CMD_MBYTESWAP_SHF) #define GT_PCI0_CMD_MBYTESWAP_BIT GT_PCI0_CMD_MBYTESWAP_MSK -- 2.20.1
[PATCH u-boot-next 05/12] pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro
PCI mvebu driver uses extended format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() and remove old custom driver address macros. Signed-off-by: Pali Rohár --- drivers/pci/pci_mvebu.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 14cd82db6ff8..aa0d6bd5162c 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -48,15 +48,6 @@ DECLARE_GLOBAL_DATA_PTR; #define PCIE_WIN5_BASE_OFF 0x1884 #define PCIE_WIN5_REMAP_OFF0x188c #define PCIE_CONF_ADDR_OFF 0x18f8 -#define PCIE_CONF_ADDR_EN BIT(31) -#define PCIE_CONF_REG(r) r) & 0xf00) << 16) | ((r) & 0xfc)) -#define PCIE_CONF_BUS(b) (((b) & 0xff) << 16) -#define PCIE_CONF_DEV(d) (((d) & 0x1f) << 11) -#define PCIE_CONF_FUNC(f) (((f) & 0x7) << 8) -#define PCIE_CONF_ADDR(b, d, f, reg) \ - (PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d)| \ -PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \ -PCIE_CONF_ADDR_EN) #define PCIE_CONF_DATA_OFF 0x18fc #define PCIE_MASK_OFF 0x1910 #define PCIE_MASK_ENABLE_INTS (0xf << 24) @@ -188,9 +179,9 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, * secondary bus with device number 1. */ if (busno == pcie->first_busno) - addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset); + addr = PCI_CONF1_EXT_ADDRESS(pcie->sec_busno, 1, 0, offset); else - addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); + addr = PCI_CONF1_EXT_ADDRESS(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); /* write address */ writel(addr, pcie->base + PCIE_CONF_ADDR_OFF); @@ -286,9 +277,9 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf, * secondary bus with device number 1. */ if (busno == pcie->first_busno) - addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset); + addr = PCI_CONF1_EXT_ADDRESS(pcie->sec_busno, 1, 0, offset); else - addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); + addr = PCI_CONF1_EXT_ADDRESS(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); /* write address */ writel(addr, pcie->base + PCIE_CONF_ADDR_OFF); -- 2.20.1
[PATCH u-boot-next 07/12] pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro
PCI fsl driver uses extended format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_EXT_ADDRESS(). Signed-off-by: Pali Rohár --- drivers/pci/pcie_fsl.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c index 3c2a2a476111..cc6efdd5b464 100644 --- a/drivers/pci/pcie_fsl.c +++ b/drivers/pci/pcie_fsl.c @@ -58,8 +58,9 @@ static int fsl_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, return 0; } - bdf = bdf - PCI_BDF(dev_seq(bus), 0, 0); - val = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x8000; + val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus), + PCI_DEV(bdf), PCI_FUNC(bdf), + offset); out_be32(®s->cfg_addr, val); sync(); @@ -94,8 +95,9 @@ static int fsl_pcie_write_config(struct udevice *bus, pci_dev_t bdf, if (fsl_pcie_addr_valid(pcie, bdf)) return 0; - bdf = bdf - PCI_BDF(dev_seq(bus), 0, 0); - val = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x8000; + val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus), + PCI_DEV(bdf), PCI_FUNC(bdf), + offset); out_be32(®s->cfg_addr, val); sync(); -- 2.20.1
[PATCH u-boot-next 06/12] pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro
PCI tegra driver uses extended format of Config Address for PCI Configuration Mechanism #1 but with cleared Enable bit. So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing PCI_CONF1_ENABLE bit and remove old custom driver address function. Signed-off-by: Pali Rohár --- drivers/pci/pci_tegra.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c index 9cb4414836f8..fc05ee00f1fc 100644 --- a/drivers/pci/pci_tegra.c +++ b/drivers/pci/pci_tegra.c @@ -275,13 +275,6 @@ static void rp_writel(struct tegra_pcie_port *port, unsigned long value, writel(value, port->regs.start + offset); } -static unsigned long tegra_pcie_conf_offset(pci_dev_t bdf, int where) -{ - return ((where & 0xf00) << 16) | (PCI_BUS(bdf) << 16) | - (PCI_DEV(bdf) << 11) | (PCI_FUNC(bdf) << 8) | - (where & 0xfc); -} - static int tegra_pcie_conf_address(struct tegra_pcie *pcie, pci_dev_t bdf, int where, unsigned long *address) { @@ -305,7 +298,9 @@ static int tegra_pcie_conf_address(struct tegra_pcie *pcie, pci_dev_t bdf, return -EFAULT; #endif - *address = pcie->cs.start + tegra_pcie_conf_offset(bdf, where); + *address = pcie->cs.start + + (PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), + PCI_FUNC(bdf), where) & ~PCI_CONF1_ENABLE); return 0; } } -- 2.20.1
[PATCH u-boot-next 09/12] pci: sh7780: Use PCI_CONF1_ADDRESS() macro
PCI sh7780 driver uses standard format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS(). Signed-off-by: Pali Rohár --- drivers/pci/pci_sh7780.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci_sh7780.c b/drivers/pci/pci_sh7780.c index 06d711a6cb9e..7533286c0156 100644 --- a/drivers/pci/pci_sh7780.c +++ b/drivers/pci/pci_sh7780.c @@ -34,9 +34,9 @@ int pci_sh4_read_config_dword(struct pci_controller *hose, pci_dev_t dev, int offset, u32 *value) { - u32 par_data = 0x8000 | dev; + u32 par_data = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev), offset); - p4_out(par_data | (offset & 0xfc), SH7780_PCIPAR); + p4_out(par_data, SH7780_PCIPAR); *value = p4_in(SH7780_PCIPDR); return 0; @@ -45,9 +45,9 @@ int pci_sh4_read_config_dword(struct pci_controller *hose, int pci_sh4_write_config_dword(struct pci_controller *hose, pci_dev_t dev, int offset, u32 value) { - u32 par_data = 0x8000 | dev; + u32 par_data = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev), offset); - p4_out(par_data | (offset & 0xfc), SH7780_PCIPAR); + p4_out(par_data, SH7780_PCIPAR); p4_out(value, SH7780_PCIPDR); return 0; } -- 2.20.1
[PATCH u-boot-next 08/12] pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro
PCI mediatek driver uses extended format of Config Address for PCI Configuration Mechanism #1 but with cleared Enable bit. So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing PCI_CONF1_ENABLE bit and remove old custom driver address macros. Signed-off-by: Pali Rohár --- drivers/pci/pcie_mediatek.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/pci/pcie_mediatek.c b/drivers/pci/pcie_mediatek.c index f5556713878f..051a3bc96935 100644 --- a/drivers/pci/pcie_mediatek.c +++ b/drivers/pci/pcie_mediatek.c @@ -41,10 +41,6 @@ #define PCIE_BAR_ENABLEBIT(0) #define PCIE_REVISION_ID BIT(0) #define PCIE_CLASS_CODE(0x60400 << 8) -#define PCIE_CONF_REG(regn)(((regn) & GENMASK(7, 2)) | \ - regn) >> 8) & GENMASK(3, 0)) << 24)) -#define PCIE_CONF_ADDR(regn, bdf) \ - (PCIE_CONF_REG(regn) | (bdf)) /* MediaTek specific configuration registers */ #define PCIE_FTS_NUM 0x70c @@ -147,8 +143,11 @@ static int mtk_pcie_config_address(const struct udevice *udev, pci_dev_t bdf, uint offset, void **paddress) { struct mtk_pcie *pcie = dev_get_priv(udev); + u32 val; - writel(PCIE_CONF_ADDR(offset, bdf), pcie->base + PCIE_CFG_ADDR); + val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), + PCI_FUNC(bdf), offset) & ~PCI_CONF1_ENABLE; + writel(val, pcie->base + PCIE_CFG_ADDR); *paddress = pcie->base + PCIE_CFG_DATA + (offset & 3); return 0; @@ -330,7 +329,6 @@ static void mtk_pcie_port_free(struct mtk_pcie_port *port) static int mtk_pcie_startup_port(struct mtk_pcie_port *port) { struct mtk_pcie *pcie = port->pcie; - u32 slot = PCI_DEV(port->slot << 11); u32 val; int err; @@ -357,13 +355,14 @@ static int mtk_pcie_startup_port(struct mtk_pcie_port *port) writel(PCIE_CLASS_CODE | PCIE_REVISION_ID, port->base + PCIE_CLASS); /* configure FC credit */ - writel(PCIE_CONF_ADDR(PCIE_FC_CREDIT, slot), - pcie->base + PCIE_CFG_ADDR); + val = PCI_CONF1_EXT_ADDRESS(0, port->slot, 0, PCIE_FC_CREDIT) & ~PCI_CONF1_ENABLE; + writel(val, pcie->base + PCIE_CFG_ADDR); clrsetbits_le32(pcie->base + PCIE_CFG_DATA, PCIE_FC_CREDIT_MASK, PCIE_FC_CREDIT_VAL(0x806c)); /* configure RC FTS number to 250 when it leaves L0s */ - writel(PCIE_CONF_ADDR(PCIE_FTS_NUM, slot), pcie->base + PCIE_CFG_ADDR); + val = PCI_CONF1_EXT_ADDRESS(0, port->slot, 0, PCIE_FTS_NUM) & ~PCI_CONF1_ENABLE; + writel(val, pcie->base + PCIE_CFG_ADDR); clrsetbits_le32(pcie->base + PCIE_CFG_DATA, PCIE_FTS_NUM_MASK, PCIE_FTS_NUM_L0(0x50)); -- 2.20.1
[PATCH u-boot-next 10/12] x86: pci: Use PCI_CONF1_ADDRESS() macro
x86 platform uses standard format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS(). Signed-off-by: Pali Rohár --- arch/x86/cpu/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index d4f9290ca73b..8a992ed82339 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -20,7 +20,7 @@ int pci_x86_read_config(pci_dev_t bdf, uint offset, ulong *valuep, enum pci_size_t size) { - outl(bdf | (offset & 0xfc) | PCI_CFG_EN, PCI_REG_ADDR); + outl(PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset), PCI_REG_ADDR); switch (size) { case PCI_SIZE_8: *valuep = inb(PCI_REG_DATA + (offset & 3)); @@ -39,7 +39,7 @@ int pci_x86_read_config(pci_dev_t bdf, uint offset, ulong *valuep, int pci_x86_write_config(pci_dev_t bdf, uint offset, ulong value, enum pci_size_t size) { - outl(bdf | (offset & 0xfc) | PCI_CFG_EN, PCI_REG_ADDR); + outl(PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset), PCI_REG_ADDR); switch (size) { case PCI_SIZE_8: outb(value, PCI_REG_DATA + (offset & 3)); -- 2.20.1
[PATCH u-boot-next 11/12] m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro
mcf5445x platform uses standard format of Config Address for PCI Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS(). Signed-off-by: Pali Rohár --- arch/m68k/cpu/mcf5445x/pci.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/m68k/cpu/mcf5445x/pci.c b/arch/m68k/cpu/mcf5445x/pci.c index af02c4934c97..d487468d0bfa 100644 --- a/arch/m68k/cpu/mcf5445x/pci.c +++ b/arch/m68k/cpu/mcf5445x/pci.c @@ -26,12 +26,11 @@ int pci_##rw##_cfg_##size(struct pci_controller *hose, \ pci_dev_t dev, int offset, type val)\ { \ - u32 addr = 0; \ - u16 cfg_type = 0; \ - addr = ((offset & 0xfc) | cfg_type | (dev) | 0x8000); \ + u32 addr = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev),\ +PCI_FUNC(dev), offset);\ out_be32(hose->cfg_addr, addr); \ cfg_##rw(val, hose->cfg_data + (offset & mask), type, op); \ - out_be32(hose->cfg_addr, addr & 0x7fff);\ + out_be32(hose->cfg_addr, addr & ~PCI_CONF1_ENABLE); \ return 0; \ } -- 2.20.1
[PATCH u-boot-next 12/12] pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro
sh7751 platform uses standard format of Config Address for PCI Configuration Mechanism #1. Commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") which did conversion of PCI sh7751 driver to DM, broke access to config space as that commit somehow swapped device and function bits in config address. Fix all these issues by using new U-Boot macro PCI_CONF1_ADDRESS() which calculates Config Address correctly. Also remove nonsense function sh7751_pci_addr_valid() which was introduced in commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") probably due to workarounded issues with mixing/swapping device and function bits of config address which probably resulted in non-working access to some devices. With correct composing of config address there should not be such issue anymore. Signed-off-by: Pali Rohár Fixes: 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") Cc: Marek Vasut --- drivers/pci/pci_sh7751.c | 29 ++--- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/drivers/pci/pci_sh7751.c b/drivers/pci/pci_sh7751.c index e110550c71c8..d514c040344c 100644 --- a/drivers/pci/pci_sh7751.c +++ b/drivers/pci/pci_sh7751.c @@ -74,33 +74,13 @@ #define p4_in(addr)(*addr) #define p4_out(data, addr) (*addr) = (data) -static int sh7751_pci_addr_valid(pci_dev_t d, uint offset) -{ - if (PCI_FUNC(d)) - return -EINVAL; - - return 0; -} - -static u32 get_bus_address(const struct udevice *dev, pci_dev_t bdf, u32 offset) -{ - return BIT(31) | (PCI_DEV(bdf) << 8) | (offset & ~3); -} - static int sh7751_pci_read_config(const struct udevice *dev, pci_dev_t bdf, uint offset, ulong *value, enum pci_size_t size) { u32 addr, reg; - int ret; - ret = sh7751_pci_addr_valid(bdf, offset); - if (ret) { - *value = pci_get_ff(size); - return 0; - } - - addr = get_bus_address(dev, bdf, offset); + addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); p4_out(addr, SH7751_PCIPAR); reg = p4_in(SH7751_PCIPDR); *value = pci_conv_32_to_size(reg, offset, size); @@ -113,13 +93,8 @@ static int sh7751_pci_write_config(struct udevice *dev, pci_dev_t bdf, enum pci_size_t size) { u32 addr, reg, old; - int ret; - - ret = sh7751_pci_addr_valid(bdf, offset); - if (ret) - return ret; - addr = get_bus_address(dev, bdf, offset); + addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); p4_out(addr, SH7751_PCIPAR); old = p4_in(SH7751_PCIPDR); reg = pci_conv_size_to_32(old, value, offset, size); -- 2.20.1
Re: [RESEND PATCH] rpi: Copy properties from firmware dtb to the loaded dtb
On 25/11/2021 20:42, Sjoerd Simons wrote: The RPI firmware adjusts several property values in the dtb it passes to u-boot depending on the board/SoC revision. Inherit some of these when u-boot loads a dtb itself. Specificaly copy: * /model: The firmware provides a more specific string * /memreserve: The firmware defines a reserved range, better keep it * emmc2bus and pcie0 dma-ranges: The C0T revision of the bcm2711 Soc (as present on rpi 400 and some rpi 4B boards) has different values for these then the B0T revision. So these need to be adjusted to boot on these boards * blconfig: The firmware defines the memory area where the blconfig stored. Copy those over so it can be enabled. * /chosen/kaslr-seed: The firmware generates a kaslr seed, take advantage of that. So my question is, why don't you want to use the devicetree provided by the firmware in the first place? Wouldn't that fix all the problems you have? Regards, Matthias Signed-off-by: Sjoerd Simons --- board/raspberrypi/rpi/rpi.c | 48 + 1 file changed, 48 insertions(+) diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index 55afaa54d9f..cdde32c8143 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -499,10 +499,58 @@ void *board_fdt_blob_setup(int *err) return (void *)fw_dtb_pointer; } +int copy_property(void *dst, void *src, char *path, char *property) +{ + int dst_offset, src_offset; + const fdt32_t *prop; + int len; + + src_offset = fdt_path_offset(src, path); + dst_offset = fdt_path_offset(dst, path); + + if (src_offset < 0 || dst_offset < 0) + return -1; + + prop = fdt_getprop(src, src_offset, property, &len); + if (!prop) + return -1; + + return fdt_setprop(dst, dst_offset, property, prop, len); +} + +/* Copy tweaks from the firmware dtb to the loaded dtb */ +void update_fdt_from_fw(void *fdt, void *fw_fdt) +{ + /* Using dtb from firmware directly; leave it alone */ + if (fdt == fw_fdt) + return; + + /* The firmware provides a more precie model; so copy that */ + copy_property(fdt, fw_fdt, "/", "model"); + + /* memory reserve as suggested by the firmware */ + copy_property(fdt, fw_fdt, "/", "memreserve"); + + /* Adjust dma-ranges for the SD card and PCI bus as they can depend on +* the SoC revision +*/ + copy_property(fdt, fw_fdt, "emmc2bus", "dma-ranges"); + copy_property(fdt, fw_fdt, "pcie0", "dma-ranges"); + + /* Bootloader configuration template exposes as nvmem */ + if (copy_property(fdt, fw_fdt, "blconfig", "reg") == 0) + copy_property(fdt, fw_fdt, "blconfig", "status"); + + /* kernel address randomisation seed as provided by the firmware */ + copy_property(fdt, fw_fdt, "/chosen", "kaslr-seed"); +} + int ft_board_setup(void *blob, struct bd_info *bd) { int node; + update_fdt_from_fw(blob, (void *)fw_dtb_pointer); + node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer"); if (node < 0) lcd_dt_simplefb_add_node(blob);
Re: [RESEND RFC PATCH 03/10] FWU: Add metadata structure and functions for accessing metadata
Hi Sughosh, On Thu, Nov 25, 2021 at 12:42:55PM +0530, Sughosh Ganu wrote: > In the FWU Multi Bank Update feature, the information about the > updatable images is stored as part of the metadata, which is stored on > a dedicated partition. Add the metadata structure, and functions to > access the metadata. These are generic API's, and implementations can > be added based on parameters like how the metadata partition is > accessed and what type of storage device houses the metadata. > > Signed-off-by: Sughosh Ganu > --- > include/fwu_metadata.h | 125 +++ > lib/fwu_updates/fwu_metadata.c | 275 + > 2 files changed, 400 insertions(+) > create mode 100644 include/fwu_metadata.h > create mode 100644 lib/fwu_updates/fwu_metadata.c > > diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h > new file mode 100644 > index 00..e692ef7506 > --- /dev/null > +++ b/include/fwu_metadata.h > @@ -0,0 +1,125 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2021, Linaro Limited Please add a link here to the arm spec that describes the metadata etc > + */ > + > +#if !defined _FWU_METADATA_H_ > +#define _FWU_METADATA_H_ > + > +#include > +#include > +#include > + > +#include > + > +/** > + * struct fwu_image_bank_info - firmware image information > + * @image_uuid: Guid value of the image in this bank > + * @accepted: Acceptance status of the image > + * @reserved: Reserved > + * > + * The structure contains image specific fields which are > + * used to identify the image and to specify the image's > + * acceptance status > + */ > +struct fwu_image_bank_info { > + efi_guid_t image_uuid; > + u32 accepted; > + u32 reserved; > +}; fwu_image_bank_info -> fwu_img_bank_info > + > +/** > + * struct fwu_image_entry - information for a particular type of image > + * @image_type_uuid: Guid value for identifying the image type > + * @location_uuid: Guid of the storage volume where the image is located /s/Guid/GUID > + * @img_bank_info: Array containing properties of images > + * > + * This structure contains information on various types of updatable > + * firmware images. Each image type then contains an array of image > + * information per bank. > + */ > +struct fwu_image_entry { > + efi_guid_t image_type_uuid; > + efi_guid_t location_uuid; > + struct fwu_image_bank_info img_bank_info[CONFIG_FWU_NUM_BANKS]; > +}; > + It seems like you've followed the naming proposed in the spec, which makes reading spec -- code easier. However I feel we should add a few more comments on the naming to make reading easier or change the naming and mention the original name in comments. A 'bank' is supposed to contain: bank[0]: Uboot(0), TF-A(0) etc bank[1]: Uboot(1), TF-A(1) etc However there's no structure that defines an entire bank. Instead the bank information is constructed by reading the metadata and fixing it up on the fly. fwu_image_bank_info -- Information for a specific image (e.g OP-TEE, U-Boot, TF-A, whatever) but not within a *bank*. That's amongst a collection of images of the same type. IOW img_bank_info looks like: img_bank_info[0] -> U-Boot(0), U-Boot(1) etc img_bank_info[1] -> TF-A(0), TF-A(1) etc @Jose can we tweak the spec naming a bit to be more intuitive? I am terrible at naming stuff but what about: fwu_image_bank_info -> fwu_img_repo_info, fwu_img_vault_info, fwu_img_storage_info, fwu_img_array_info, > +/** > + * struct fwu_metadata - Metadata structure for multi-bank updates > + * @crc32: crc32 value for the metadata > + * @version: Metadata version > + * @active_index: Index of the bank currently used for booting images > + * @previous_active_inde: Index of the bank used before the current bank > + *being used for booting > + * @img_entry: Array of information on various firmware images that can > + * be updated > + * > + * This structure is used to store all the needed information for performing > + * multi bank updates on the platform. This contains info on the bank being > + * used to boot along with the information needed for identification of > + * individual images > + */ > +struct fwu_metadata { > + u32 crc32; > + u32 version; > + u32 active_index; > + u32 previous_active_index; > + > + struct fwu_image_entry img_entry[CONFIG_FWU_NUM_IMAGES_PER_BANK]; > +}; > + > +/** > + * @get_active_index: get the current active_index value > + * @update_active_index: update the active_index value > + * @fill_partition_guid_array: fill the array with guid values of the > + * partitions found on the storage media > + * @get_image_alt_num: get the alt number to be used for the image > + * @metadata_check: check the validity of the metadata partitions > + * @revert_boot_index: set the active_index to previous_active_index > + * @set_accept_image: set the accepted bit for th
[PATCH v7 1/3] efi_loader: Add check for event log passed from firmware
Platforms may have support to measure their initial firmware components and pass the event log to u-boot. The event log address can be passed in property tpm_event_log_addr and tpm_event_log_size of the tpm node. Platforms may choose their own specific mechanism to do so. A weak function is added to check if even log has been passed to u-boot from earlier firmware components. If available, the eventlog is parsed to check for its correctness and further event logs are appended to the passed log. Signed-off-by: Ruchika Gupta --- v7: Addressed Heinrich's comments Changed functions not exported out of this file as static. Corrected function decsriptions and added few. Added declaration of weak function in header file Moved offset check to parse functions v6: No change v5: Shift the efi_init_event_log() to a different location in the file. This help fixes compilation issue introduced by calling efi_append_scrtm_version() from it. v4: Add SCRTM version to log only if previous firmware doesn't pass the eventlog v3: Return as soon as you detect error v2: Moved firmware eventlog code parsing to tcg2_get_fw_eventlog() include/efi_loader.h | 2 + lib/efi_loader/efi_tcg2.c | 472 -- 2 files changed, 405 insertions(+), 69 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index d52e399841..67c40ca57a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -988,4 +988,6 @@ efi_status_t efi_esrt_register(void); */ efi_status_t efi_esrt_populate(void); efi_status_t efi_load_capsule_drivers(void); + +efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz); #endif /* _EFI_LOADER_H */ diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 8c1f22e337..5ded57fd29 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct udevice **dev) return EFI_NOT_FOUND; } +/** + * platform_get_eventlog() - retrieve the eventlog address and size + * + * This function retrieves the eventlog address and size if the underlying + * firmware has done some measurements and passed them. + * + * This function may be overridden based on platform specific method of + * passing the eventlog address and size. + * + * @dev: udevice + * @addr: eventlog address + * @sz:eventlog size + * Return: status code + */ +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, + u32 *sz) +{ + const u64 *basep; + const u32 *sizep; + + basep = dev_read_prop(dev, "tpm_event_log_addr", NULL); + if (!basep) + return EFI_NOT_FOUND; + + *addr = be64_to_cpup((__force __be64 *)basep); + + sizep = dev_read_prop(dev, "tpm_event_log_size", NULL); + if (!sizep) + return EFI_NOT_FOUND; + + *sz = be32_to_cpup((__force __be32 *)sizep); + if (*sz == 0) { + log_debug("event log empty\n"); + return EFI_NOT_FOUND; + } + + return EFI_SUCCESS; +} + /** * tpm2_get_max_command_size() - get the supported max command size * @@ -1181,6 +1220,283 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = { .get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks, }; +/** + * parse_event_log_header() - Parse and verify the event log header fields + * + * @buffer:Pointer to the start of the eventlog + * @size: Size of the eventlog + * @pos: Return offset of the next event in buffer right + * after the event header i.e specID + * + * Return: status code + */ +static efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos) +{ + struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer; + int i = 0; + + if (size < sizeof(*event_header)) + return EFI_COMPROMISED_DATA; + + if (get_unaligned_le32(&event_header->pcr_index) != 0 || + get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION) + return EFI_COMPROMISED_DATA; + + for (i = 0; i < sizeof(event_header->digest); i++) { + if (event_header->digest[i]) + return EFI_COMPROMISED_DATA; + } + + *pos += sizeof(*event_header); + + return EFI_SUCCESS; +} + +/** + * parse_specid_event() - Parse and verify the specID Event in the eventlog + * + * @dev: udevice + * @buffer:Pointer to the start of the eventlog + * @log_size: Size of the eventlog + * @pos: [in] Offset of specID event in the eventlog buffer + * [out] Return offset of the next event in the buffer + * after the specID + * @digest_list: list of digests in the event + * + * Return:
[v7 PATCH 2/3] tpm: use more algorithms than sha256 on pcr_read
The current tpm2_pcr_read is hardcoded using SHA256. Make the actual command to TPM configurable to use wider range of algorithms. The current command line is kept as is i.e limited to SHA-256 only. Signed-off-by: Ruchika Gupta Reviewed-by: Ilias Apalodimas --- v7: No change v6: No change v5: No change v4: No change v3: No change v2: Change algorithm from u32 to u16 Add parameter description in function declaration cmd/tpm-v2.c | 3 ++- include/tpm-v2.h | 5 - lib/tpm-v2.c | 12 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c index daae91100a..4ea5f9f094 100644 --- a/cmd/tpm-v2.c +++ b/cmd/tpm-v2.c @@ -151,7 +151,8 @@ static int do_tpm_pcr_read(struct cmd_tbl *cmdtp, int flag, int argc, data = map_sysmem(simple_strtoul(argv[2], NULL, 0), 0); - rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, data, &updates); + rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, TPM2_ALG_SHA256, + data, TPM2_DIGEST_LEN, &updates); if (!rc) { printf("PCR #%u content (%u known updates):\n", index, updates); print_byte_string(data, TPM2_DIGEST_LEN); diff --git a/include/tpm-v2.h b/include/tpm-v2.h index ceff7d245e..4e9dd52cb6 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -512,13 +512,16 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data, * @devTPM device * @idxIndex of the PCR * @idx_min_sz Minimum size in bytes of the pcrSelect array + * @algorithm Algorithm used, defined in 'enum tpm2_algorithms' * @data Output buffer for contents of the named PCR + * @digest_len len of the data * @updatesOptional out parameter: number of updates for this PCR * * @return code of the operation */ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz, - void *data, unsigned int *updates); + u16 algorithm, void *data, u32 digest_len, + unsigned int *updates); /** * Issue a TPM2_GetCapability command. This implementation is limited diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 2e7b27bd6b..1bf627853a 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -254,7 +254,8 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data, } u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz, - void *data, unsigned int *updates) + u16 algorithm, void *data, u32 digest_len, + unsigned int *updates) { u8 idx_array_sz = max(idx_min_sz, DIV_ROUND_UP(idx, 8)); u8 command_v2[COMMAND_BUFFER_SIZE] = { @@ -264,7 +265,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz, /* TPML_PCR_SELECTION */ tpm_u32(1), /* Number of selections */ - tpm_u16(TPM2_ALG_SHA256), /* Algorithm of the hash */ + tpm_u16(algorithm), /* Algorithm of the hash */ idx_array_sz, /* Array size for selection */ /* bitmap(idx) Selected PCR bitmap */ }; @@ -283,10 +284,13 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz, if (ret) return ret; + if (digest_len > response_len) + return TPM_LIB_ERROR; + if (unpack_byte_string(response, response_len, "ds", 10, &counter, - response_len - TPM2_DIGEST_LEN, data, - TPM2_DIGEST_LEN)) + response_len - digest_len, data, + digest_len)) return TPM_LIB_ERROR; if (updates) -- 2.25.1
[PATCH v7 3/3] efi_loader: Extend PCR's for firmware measurements
Firmwares before U-Boot may be capable of doing tpm measurements and passing them to U-Boot in the form of eventlog. However there may be scenarios where the firmwares don't have TPM driver and are not capable of extending the measurements in the PCRs. Based on TCG spec, if previous firnware has extended PCR's, PCR0 would not be 0. So, read the PCR0 to determine if the PCR's need to be extended as eventlog is parsed or not. Signed-off-by: Ruchika Gupta Reviewed-by: Ilias Apalodimas Tested-by: Ilias Apalodimas --- v7: Addressed Heinrick's comments - Added missing parameter in function header v6: Changed TPM2_DIGEST_LEN to TPM2_SHA512_DIGEST_SIZE v5 : No change v4 : No change v3 : Rebase changes on top of changes made in first patch series v2 : Removed check for PCR0 in eventlog lib/efi_loader/efi_tcg2.c | 76 +++ 1 file changed, 76 insertions(+) diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 5ded57fd29..d247179fbf 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -199,6 +199,44 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index, return EFI_SUCCESS; } +/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values + * + * @dev: device + * @pcr_index: PCR index + * @digest_list: list of digest algorithms to extend + * + * @Return: status code + */ +static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index, + struct tpml_digest_values *digest_list) +{ + struct tpm_chip_priv *priv; + unsigned int updates, pcr_select_min; + u32 rc; + size_t i; + + priv = dev_get_uclass_priv(dev); + if (!priv) + return EFI_DEVICE_ERROR; + + pcr_select_min = priv->pcr_select_min; + + for (i = 0; i < digest_list->count; i++) { + u16 hash_alg = digest_list->digests[i].hash_alg; + u8 *digest = (u8 *)&digest_list->digests[i].digest; + + rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min, + hash_alg, digest, alg_to_len(hash_alg), + &updates); + if (rc) { + EFI_PRINT("Failed to read PCR\n"); + return EFI_DEVICE_ERROR; + } + } + + return EFI_SUCCESS; +} + /* put_event - Append an agile event to an eventlog * * @pcr_index: PCR index @@ -1461,6 +1499,8 @@ static efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, void *log_buffer, u32 pcr, pos; u64 base; u32 sz; + bool extend_pcr = false; + int i; ret = platform_get_eventlog(dev, &base, &sz); if (ret != EFI_SUCCESS) @@ -1482,6 +1522,26 @@ static efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, void *log_buffer, return ret; } + ret = tcg2_pcr_read(dev, 0, &digest_list); + if (ret) { + log_err("Error reading PCR 0\n"); + return ret; + } + + /* +* If PCR0 is 0, previous firmware didn't have the capability +* to extend the PCR. In this scenario, extend the PCR as +* the eventlog is parsed. +*/ + for (i = 0; i < digest_list.count; i++) { + u8 buffer[TPM2_SHA512_DIGEST_SIZE] = { 0 }; + u16 hash_alg = digest_list.digests[i].hash_alg; + + if (!memcmp((u8 *)&digest_list.digests[i].digest, buffer, + alg_to_len(hash_alg))) + extend_pcr = true; + } + while (pos < sz) { ret = tcg2_parse_event(dev, buffer, sz, &pos, &digest_list, &pcr); @@ -1489,6 +1549,22 @@ static efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, void *log_buffer, log_err("Error parsing event\n"); return ret; } + if (extend_pcr) { + ret = tcg2_pcr_extend(dev, pcr, &digest_list); + if (ret != EFI_SUCCESS) { + log_err("Error in extending PCR\n"); + return ret; + } + + /* Clear the digest for next event */ + for (i = 0; i < digest_list.count; i++) { + u16 hash_alg = digest_list.digests[i].hash_alg; + u8 *digest = + (u8 *)&digest_list.digests[i].digest; + + memset(digest, 0, alg_to_len(hash_alg)); + } + } } memcpy(log_buffer, buffer, sz); -- 2.25.1
Re: [RESEND RFC PATCH 00/10] FWU: Add support for FWU Multi Bank Update feature
On 11/25/21 08:12, Sughosh Ganu wrote: (resending to including the first paragraph which got deleted for some reason). The patchset adds support for the FWU Multi Bank Update[1] The patch set is lacking a documentation update The patch set is lacking an integration test. Please, add both to enable reviewing. Best regards Heinrich feature. Certain aspects of the Dependable Boot[2] specification have also been implemented. The FWU multi bank update feature is used for supporting multiple sets(also called banks) of firmware image(s), allowing the platform to boot from a different bank, in case it fails to boot from the active bank. This functionality is supported by keeping the relevant information in a structure called metadata, which provides information on the images. Among other parameters, the metadata structure contains information on the currect active bank that is being used to boot image(s). Functionality is being added to work with the UEFI capsule driver in u-boot. The metadata is read to gather information on the update bank, which is the bank to which the firmware images would be flashed to. On a successful completion of the update of all components, the active bank field in the metadata is updated, to reflect the bank from which the platform will boot on the subsequent boots. Currently, the feature is being enabled on the STM32MP157C-DK2 board which boots a FIP image from a uSD card partitioned with the GPT partioning scheme. This also requires changes in the previous stage of bootloader, which parses the metadata and selects the bank to boot the image(s) from. Support is being added in tf-a(BL2 stage) for the STM32MP157C-DK2 board to boot the active bank images. These changes are under review currently[3]. Todo's -- 1) Add a test(selftest) for the metadata access. 2) Add a tool for generation of the metadata. Not sure if this needs to be part of the u-boot repository though. 3) Add a tool for generation of the firmware accept/reject dummy capsule. Need to check if this can be added to the mkeficapsule tool in u-boot. [1] - https://developer.arm.com/documentation/den0118/a [2] - https://staging-git.codelinaro.org/linaro/firmware-dual-banked-updates/test [3] - https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/12566 Sughosh Ganu (10): GPT: Add function to get gpt header and partition entries stm32mp: dfu: Move the ram partitions to the end of the dfu_alt_info variable FWU: Add metadata structure and functions for accessing metadata FWU: Add metadata access functions for GPT partitioned block devices FWU: stm32mp1: Add helper functions for accessing metadata FWU: STM32MP1: Add support to read boot index from backup register EFI: FMP: Add provision to update image's ImageTypeId in image descriptor FWU: Add boot time checks as highlighted by the FWU specification FWU: Add support for FWU Multi Bank Update feature FWU: cmd: Add a command to read metadata arch/arm/mach-stm32mp/include/mach/stm32.h | 1 + board/st/common/stm32mp_dfu.c | 11 +- board/st/stm32mp1/stm32mp1.c | 70 ++ cmd/Kconfig| 6 + cmd/Makefile | 1 + cmd/fwu_metadata.c | 65 ++ common/board_r.c | 6 + disk/part_efi.c| 10 + include/fwu_metadata.h | 140 include/part.h | 14 + lib/Kconfig| 32 + lib/Makefile | 1 + lib/efi_loader/efi_capsule.c | 190 +- lib/efi_loader/efi_firmware.c | 76 ++- lib/fwu_updates/Makefile | 11 + lib/fwu_updates/fwu.c | 170 + lib/fwu_updates/fwu_metadata.c | 275 lib/fwu_updates/fwu_metadata_gpt_blk.c | 716 + 18 files changed, 1784 insertions(+), 11 deletions(-) create mode 100644 cmd/fwu_metadata.c create mode 100644 include/fwu_metadata.h create mode 100644 lib/fwu_updates/Makefile create mode 100644 lib/fwu_updates/fwu.c create mode 100644 lib/fwu_updates/fwu_metadata.c create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c
Re: [RESEND RFC PATCH 07/10] EFI: FMP: Add provision to update image's ImageTypeId in image descriptor
On 11/25/21 08:12, Sughosh Ganu wrote: The FWU Multi Banks Update feature allows updating different types of updatable firmware images on the platform. These image types are identified using the ImageTypeId GUID value. Add support in the GetImageInfo function of the FMP protocol to get the GUID values for the individual images and populate these in the image descriptor for the corresponding images. Signed-off-by: Sughosh Ganu --- lib/efi_loader/efi_firmware.c | 76 --- 1 file changed, 71 insertions(+), 5 deletions(-) diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index a1b88dbfc2..a2b639b448 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -106,7 +107,8 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported( * @descriptor_size: Pointer to descriptor size * package_version: Package version * package_version_name: Package version's name - * image_type: Image type GUID + * guid_array: Image type GUID array + * nparts: Number of partions on the storage device * * Return information bout the current firmware image in @image_info. * @image_info will consist of a number of descriptors. @@ -122,7 +124,7 @@ static efi_status_t efi_get_dfu_info( efi_uintn_t *descriptor_size, u32 *package_version, u16 **package_version_name, - const efi_guid_t *image_type) + const efi_guid_t *guid_array, u32 nparts) { struct dfu_entity *dfu; size_t names_len, total_size; @@ -145,6 +147,19 @@ static efi_status_t efi_get_dfu_info( return EFI_SUCCESS; } + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) { The concept of multiple banks is not related to the concept of multiple GUIDs. So this if statement should be removed. + /* +* For FWU multi bank updates, the number of partitions +* should at least be same as dfu partitions or less +*/ + if (nparts > dfu_num) { + log_err("Number of dfu alt no's less than partitions\n"); + dfu_free_entities(); + + return EFI_INVALID_PARAMETER; + } + } + total_size = sizeof(*image_info) * dfu_num + names_len; /* * we will assume that sizeof(*image_info) * dfu_name @@ -172,7 +187,11 @@ static efi_status_t efi_get_dfu_info( next = name; list_for_each_entry(dfu, &dfu_list, list) { image_info[i].image_index = dfu->alt + 1; - image_info[i].image_type_id = *image_type; + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) The number of GUIDs is not related to the number of banks. Please, remove this test. + image_info[i].image_type_id = guid_array[i]; The sequence of GUIDs in a capsule should not influence the update. The design lacks a configuration defining which GUID maps to which DFU part. Please, get rid of all DFU environment variables and describe the update in a configuration file that you add to the capsule. + else + image_info[i].image_type_id = *guid_array; Do you want to write the same image to multiple places? Why? Best regards Heirnich + image_info[i].image_id = dfu->alt; /* copy the DFU entity name */ @@ -249,7 +268,9 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( u32 *package_version, u16 **package_version_name) { + u32 nparts; efi_status_t ret; + efi_guid_t *part_guid_arr; EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, image_info_size, image_info, @@ -264,12 +285,24 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( !descriptor_size || !package_version || !package_version_name)) return EFI_EXIT(EFI_INVALID_PARAMETER); + part_guid_arr = malloc(sizeof(efi_guid_t)); + if (!part_guid_arr) { + log_err("Unable to allocate memory for guid array\n"); + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + guidcpy(part_guid_arr, &efi_firmware_image_type_uboot_fit); + nparts = 1; + ret = efi_get_dfu_info(image_info_size, image_info, descriptor_version, descriptor_count, descriptor_size, package_version, package_version_name, - &efi_firmware_image_type_uboot_fit); + part_guid_arr, nparts); +out: + free(part_guid_arr); return EFI_EXIT(ret); } @@ -358,7 +391,10 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info( u32 *package_version,
Re: [RESEND RFC PATCH 00/10] FWU: Add support for FWU Multi Bank Update feature
Hi Heincrich, On Fri, Nov 26, 2021 at 01:29:02PM +0100, Heinrich Schuchardt wrote: > On 11/25/21 08:12, Sughosh Ganu wrote: > > (resending to including the first paragraph which got deleted for some > > reason). > > > > The patchset adds support for the FWU Multi Bank Update[1] > > The patch set is lacking a documentation update > The patch set is lacking an integration test. > > Please, add both to enable reviewing. The entire idea, as well as the structures used is documented in [1] [2]. I understand that many of you don't have time to go through the entire spec, but if it helps I'll be happy to present it on a U-Boot contributors call. Sughosh will add the relevant short documentation in U-Boot, explaining the basic usage and referring to the specs once the discussion has settled down. However this is an RFC, it's primary purpose is to discuss the general architecture and idea of rollback protected firmware updates. Adding selftests to an RFC to enable reviewing is a bit too much imho. [1] https://staging-git.codelinaro.org/linaro/firmware-dual-banked-updates/test/-/releases [2] https://developer.arm.com/documentation/den0118/a Regards /Ilias > > Best regards > > Heinrich > > > feature. Certain aspects of the Dependable Boot[2] specification have > > also been implemented. > > > > The FWU multi bank update feature is used for supporting multiple > > sets(also called banks) of firmware image(s), allowing the platform to > > boot from a different bank, in case it fails to boot from the active > > bank. This functionality is supported by keeping the relevant > > information in a structure called metadata, which provides information > > on the images. Among other parameters, the metadata structure contains > > information on the currect active bank that is being used to boot > > image(s). > > > > Functionality is being added to work with the UEFI capsule driver in > > u-boot. The metadata is read to gather information on the update bank, > > which is the bank to which the firmware images would be flashed to. On > > a successful completion of the update of all components, the active > > bank field in the metadata is updated, to reflect the bank from which > > the platform will boot on the subsequent boots. > > > > Currently, the feature is being enabled on the STM32MP157C-DK2 > > board which boots a FIP image from a uSD card partitioned with the GPT > > partioning scheme. This also requires changes in the previous stage of > > bootloader, which parses the metadata and selects the bank to boot the > > image(s) from. Support is being added in tf-a(BL2 stage) for the > > STM32MP157C-DK2 board to boot the active bank images. These changes > > are under review currently[3]. > > > > Todo's > > -- > > 1) Add a test(selftest) for the metadata access. > > 2) Add a tool for generation of the metadata. Not sure if this needs to > > be part of the u-boot repository though. > > 3) Add a tool for generation of the firmware accept/reject dummy > > capsule. Need to check if this can be added to the mkeficapsule > > tool in u-boot. > > > > [1] - https://developer.arm.com/documentation/den0118/a > > [2] - > > https://staging-git.codelinaro.org/linaro/firmware-dual-banked-updates/test > > [3] - https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/12566 > > > > Sughosh Ganu (10): > >GPT: Add function to get gpt header and partition entries > >stm32mp: dfu: Move the ram partitions to the end of the dfu_alt_info > > variable > >FWU: Add metadata structure and functions for accessing metadata > >FWU: Add metadata access functions for GPT partitioned block devices > >FWU: stm32mp1: Add helper functions for accessing metadata > >FWU: STM32MP1: Add support to read boot index from backup register > >EFI: FMP: Add provision to update image's ImageTypeId in image > > descriptor > >FWU: Add boot time checks as highlighted by the FWU specification > >FWU: Add support for FWU Multi Bank Update feature > >FWU: cmd: Add a command to read metadata > > > > arch/arm/mach-stm32mp/include/mach/stm32.h | 1 + > > board/st/common/stm32mp_dfu.c | 11 +- > > board/st/stm32mp1/stm32mp1.c | 70 ++ > > cmd/Kconfig| 6 + > > cmd/Makefile | 1 + > > cmd/fwu_metadata.c | 65 ++ > > common/board_r.c | 6 + > > disk/part_efi.c| 10 + > > include/fwu_metadata.h | 140 > > include/part.h | 14 + > > lib/Kconfig| 32 + > > lib/Makefile | 1 + > > lib/efi_loader/efi_capsule.c | 190 +- > > lib/efi_loader/efi_firmware.c | 76 ++- > > lib/fwu_updates/Makefile | 11 + > > lib/fwu_updates/fwu.c
Re: [RESEND RFC PATCH 09/10] FWU: Add support for FWU Multi Bank Update feature
On 11/25/21 08:13, Sughosh Ganu wrote: The FWU Multi Bank Update feature supports updation of firmware images to one of multiple sets(also called banks) of images. The firmware images are clubbed together in banks, with the system booting images from the active bank. Information on the images such as which bank they belong to is stored as part of the metadata structure, which is stored on the same storage media as the firmware images on a dedicated partition. At the time of update, the metadata is read to identify the bank to which the images need to be flashed(update bank). On a successful update, the metadata is modified to set the updated bank as active bank to subsequently boot from. Signed-off-by: Sughosh Ganu --- include/fwu_metadata.h | 10 ++ lib/Kconfig | 32 ++ lib/Makefile | 1 + lib/efi_loader/efi_capsule.c | 190 ++- lib/fwu_updates/Makefile | 11 ++ lib/fwu_updates/fwu.c| 29 +- 6 files changed, 269 insertions(+), 4 deletions(-) create mode 100644 lib/fwu_updates/Makefile diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h index 02897f33a8..2d276a019a 100644 --- a/include/fwu_metadata.h +++ b/include/fwu_metadata.h @@ -106,7 +106,16 @@ struct fwu_metadata_ops { EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \ 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23) +#define FWU_OS_REQUEST_FW_REVERT_GUID \ + EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \ +0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0) + +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \ + EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \ +0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8) + #define FWU_METADATA_VERSION 0x1 +#define FWU_IMAGE_ACCEPTED 0x1 extern struct fwu_metadata_ops fwu_gpt_blk_ops; @@ -126,5 +135,6 @@ int fwu_plat_get_update_index(u32 *update_idx); int fwu_plat_get_blk_desc(struct blk_desc **desc); void fwu_plat_get_bootidx(void *boot_idx); int fwu_boottime_checks(void); +int fwu_trial_state_ctr_start(void); #endif /* _FWU_METADATA_H_ */ diff --git a/lib/Kconfig b/lib/Kconfig index 807a4c6ade..7cb306317c 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ When there are multiple device tree nodes with same name, enable this config option to distinguish them using phandles in fdtdec_get_alias_seq() function. + +config FWU_MULTI_BANK_UPDATE + bool "Enable FWU Multi Bank Update Feature" Why do we need a configuration variable? Having 1 bank is just a special case which should not use separate code. + depends on EFI_HAVE_CAPSULE_SUPPORT + select PARTITION_TYPE_GUID + select EFI_SETUP_EARLY + help + Feature for updating firmware images on platforms having + multiple banks(copies) of the firmware images. One of the + bank is selected for updating all the firmware components + +config FWU_NUM_BANKS + int "Number of Banks defined by the platform" + depends on FWU_MULTI_BANK_UPDATE This should default to 1. + help + Define the number of banks of firmware images on a platform + +config FWU_NUM_IMAGES_PER_BANK + int "Number of firmware images per bank" + depends on FWU_MULTI_BANK_UPDATE This dependency makes no sense. Even if I have one bank I can have multiple images. Why should this configuration variable be needed? The dfu variables and the capsule define how many images are updated. + help + Define the number of firmware images per bank. This value + should be the same for all the banks. + +config FWU_TRIAL_STATE_CNT + int "Number of times system boots in Trial State" + depends on FWU_MULTI_BANK_UPDATE + default 3 + help + With FWU Multi Bank Update feature enabled, number of times + the platform is allowed to boot in Trial State after an + update. Do you mean: "This is the number of boots in trial state before falling back to the last successfully used bank." Best regards Heinrich diff --git a/lib/Makefile b/lib/Makefile index 5ddbc77ed6..bc5c1e22fc 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/ obj-$(CONFIG_EFI_LOADER) += efi_driver/ obj-$(CONFIG_EFI_LOADER) += efi_loader/ obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/ +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/ obj-$(CONFIG_LZMA) += lzma/ obj-$(CONFIG_BZIP2) += bzip2/ obj-$(CONFIG_TIZEN) += tizen/ diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 502bcfca6e..40b9e87e92 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id = EFI_FIRMWARE_MANAGE
Re: [PATCH v7 1/3] efi_loader: Add check for event log passed from firmware
On Fri, Nov 26, 2021 at 05:22:59PM +0530, Ruchika Gupta wrote: > Platforms may have support to measure their initial firmware components > and pass the event log to u-boot. The event log address can be passed > in property tpm_event_log_addr and tpm_event_log_size of the tpm node. > Platforms may choose their own specific mechanism to do so. A weak > function is added to check if even log has been passed to u-boot > from earlier firmware components. If available, the eventlog is parsed > to check for its correctness and further event logs are appended to the > passed log. > > Signed-off-by: Ruchika Gupta > --- > v7: > Addressed Heinrich's comments > Changed functions not exported out of this file as static. > Corrected function decsriptions and added few. > Added declaration of weak function in header file > Moved offset check to parse functions > > v6: No change > > v5: > Shift the efi_init_event_log() to a different location in the file. > This help fixes compilation issue introduced by calling > efi_append_scrtm_version() > from it. > > v4: > Add SCRTM version to log only if previous firmware doesn't pass the eventlog > > v3: > Return as soon as you detect error > > v2: > Moved firmware eventlog code parsing to tcg2_get_fw_eventlog() > > include/efi_loader.h | 2 + > lib/efi_loader/efi_tcg2.c | 472 -- > 2 files changed, 405 insertions(+), 69 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index d52e399841..67c40ca57a 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -988,4 +988,6 @@ efi_status_t efi_esrt_register(void); > */ > efi_status_t efi_esrt_populate(void); > efi_status_t efi_load_capsule_drivers(void); > + > +efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz); > #endif /* _EFI_LOADER_H */ > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index 8c1f22e337..5ded57fd29 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct > udevice **dev) > return EFI_NOT_FOUND; > } > > +/** > + * platform_get_eventlog() - retrieve the eventlog address and size > + * > + * This function retrieves the eventlog address and size if the underlying > + * firmware has done some measurements and passed them. > + * > + * This function may be overridden based on platform specific method of > + * passing the eventlog address and size. > + * > + * @dev: udevice > + * @addr:eventlog address > + * @sz: eventlog size > + * Return: status code > + */ > +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, > + u32 *sz) > +{ > + const u64 *basep; > + const u32 *sizep; > + > + basep = dev_read_prop(dev, "tpm_event_log_addr", NULL); > + if (!basep) > + return EFI_NOT_FOUND; > + > + *addr = be64_to_cpup((__force __be64 *)basep); > + > + sizep = dev_read_prop(dev, "tpm_event_log_size", NULL); > + if (!sizep) > + return EFI_NOT_FOUND; > + > + *sz = be32_to_cpup((__force __be32 *)sizep); > + if (*sz == 0) { > + log_debug("event log empty\n"); > + return EFI_NOT_FOUND; > + } > + > + return EFI_SUCCESS; > +} > + > /** > * tpm2_get_max_command_size() - get the supported max command size > * > @@ -1181,6 +1220,283 @@ static const struct efi_tcg2_protocol > efi_tcg2_protocol = { > .get_result_of_set_active_pcr_banks = > efi_tcg2_get_result_of_set_active_pcr_banks, > }; > > +/** > + * parse_event_log_header() - Parse and verify the event log header fields > + * > + * @buffer: Pointer to the start of the eventlog > + * @size:Size of the eventlog > + * @pos: Return offset of the next event in buffer right > + * after the event header i.e specID > + * > + * Return: status code > + */ > +static efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos) > +{ > + struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer; > + int i = 0; > + > + if (size < sizeof(*event_header)) > + return EFI_COMPROMISED_DATA; > + > + if (get_unaligned_le32(&event_header->pcr_index) != 0 || > + get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION) > + return EFI_COMPROMISED_DATA; > + > + for (i = 0; i < sizeof(event_header->digest); i++) { > + if (event_header->digest[i]) > + return EFI_COMPROMISED_DATA; > + } > + > + *pos += sizeof(*event_header); > + > + return EFI_SUCCESS; > +} > + > +/** > + * parse_specid_event() - Parse and verify the specID Event in the eventlog > + * > + * @dev: udevice > + * @buffer: Pointer to the start of the eventlog > + * @log_size:Size of the eventlo
[PATCH u-boot-marvell RESEND 01/11] include/linux/byteorder: Fix compilation of __constant_cpu_to_be32()
From: Pali Rohár The macro __constant_cpu_to_be32() uses ___constant_swab32(), which for some reason is not defined and causes the following error during compilation: include/linux/byteorder/little_endian.h:28:52: warning: implicit declaration of function ‘___constant_swab32’; did you mean ‘__builtin_bswap32’? [-Wimplicit-function-declaration] #define __constant_cpu_to_be32(x) ((__force __be32)___constant_swab32((x))) Declare all ___constant_swabXX() macros. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- include/linux/byteorder/swab.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/byteorder/swab.h b/include/linux/byteorder/swab.h index 4334fa77e3..5efc252acf 100644 --- a/include/linux/byteorder/swab.h +++ b/include/linux/byteorder/swab.h @@ -39,6 +39,10 @@ (__u64)(((__u64)(x) & (__u64)0x00ffULL) >> 40) | \ (__u64)(((__u64)(x) & (__u64)0xff00ULL) >> 56) )) +#define ___constant_swab16(x) ___swab16(x) +#define ___constant_swab32(x) ___swab32(x) +#define ___constant_swab64(x) ___swab64(x) + /* * provide defaults when no architecture-specific optimization is detected */ -- 2.32.0
[PATCH u-boot-marvell RESEND 00/11] Some mvebu comphy + mox + fdt_support changes
From: Marek Behún Hello Stefan, as requested I am resending this series with board maintainers added in Ccs. Original message: Pali prepared patches that convert A3720 comphy driver to use Linux' DT bindings. (Yes, I have patches that convert the whole driver into using SMC calls into ATF, but haven't found time yet to rebase them since last year, and Pali has done this in the meantime :-D Maybe I'll look into my old patches sometimes, but this is now usable.) These patches needed some changes in Turris MOX board code, and I added some more changes to handling device-tree fixups, and this lead to adding some code into fdt_support, which can hopefully be used by other people as well. Marek Marek Behún (8): treewide: Use fdt_create_phandle() where appropriate fdt_support: Remove fdt_alloc_phandle() in favor of fdt_generate_phandle() fdt_support: Remove FDT_STATUS_FAIL_ERROR_CODE fdt_support: Fix comment for fdt_create_phandle() fdt_support: Add some useful functions arm: mvebu: turris_mox: Find DT nodes by compatible or alias instead of path arm: mvebu: turris_mox: Fix unstable board topology reading fdt_support: Add fdt_delete_disabled_nodes() and use in Turris MOX Pali Rohár (3): include/linux/byteorder: Fix compilation of __constant_cpu_to_be32() arm: mvebu: turris_mox: Enable eth1 in U-Boot if a network module is present phy: marvell: a3700: Convert to official DT bindings in COMPHY driver arch/arm/cpu/armv7/ls102xa/fdt.c | 6 +- arch/arm/dts/armada-3720-espressobin.dts | 21 +-- arch/arm/dts/armada-3720-turris-mox.dts | 25 +-- arch/arm/dts/armada-3720-uDPU.dts| 23 +-- arch/arm/dts/armada-37xx.dtsi| 20 +- board/CZ.NIC/turris_mox/turris_mox.c | 223 --- board/Marvell/octeon_ebb7304/board.c | 5 +- board/freescale/lx2160a/eth_lx2160aqds.c | 8 +- board/freescale/lx2160a/eth_lx2162aqds.c | 8 +- board/gateworks/gw_ventana/common.c | 3 +- board/kontron/sl28/sl28.c| 2 +- common/fdt_support.c | 174 +++--- drivers/misc/fsl_portals.c | 10 +- drivers/pci/pcie_layerscape_fixup.c | 8 +- drivers/pci/pcie_layerscape_gen4_fixup.c | 8 +- drivers/phy/marvell/comphy_a3700.c | 133 ++ drivers/phy/marvell/comphy_core.c| 59 +- drivers/phy/marvell/comphy_core.h| 23 +++ drivers/phy/marvell/comphy_cp110.c | 58 ++ include/fdt_support.h| 51 -- include/linux/byteorder/swab.h | 4 + 21 files changed, 551 insertions(+), 321 deletions(-) -- 2.32.0
[PATCH u-boot-marvell RESEND 02/11] treewide: Use fdt_create_phandle() where appropriate
From: Marek Behún Replace fdt_alloc_phandle() with subsequent fdt_set_phandle() by fdt_create_phandle(). Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Cc: Aaron Williams Cc: Ramon Fried Cc: Vladimir Oltean --- board/Marvell/octeon_ebb7304/board.c | 5 ++--- drivers/misc/fsl_portals.c | 10 -- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/board/Marvell/octeon_ebb7304/board.c b/board/Marvell/octeon_ebb7304/board.c index e8e2d547c1..c6c7c13483 100644 --- a/board/Marvell/octeon_ebb7304/board.c +++ b/board/Marvell/octeon_ebb7304/board.c @@ -205,7 +205,7 @@ static int fdt_fix_mix(const void *fdt) int env_lmac = -1; int lmac_fdt_node = -1; int mix_fdt_node = -1; - int lmac_phandle; + unsigned int lmac_phandle; char *compat; /* Get the lmac for this environment variable */ @@ -229,8 +229,7 @@ static int fdt_fix_mix(const void *fdt) } } - lmac_phandle = fdt_alloc_phandle((void *)fdt); - fdt_set_phandle((void *)fdt, lmac_fdt_node, lmac_phandle); + lmac_phandle = fdt_create_phandle((void *)fdt, lmac_fdt_node); /* Get the fdt mix node corresponding to this lmac */ mix_fdt_node = get_mix_fdt_node(fdt, env_node, env_lmac); diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c index 632430e420..02bc3f86ca 100644 --- a/drivers/misc/fsl_portals.c +++ b/drivers/misc/fsl_portals.c @@ -106,7 +106,7 @@ static int fdt_qportal(void *blob, int off, int id, char *name, enum fsl_dpaa_dev dev, int create) { int childoff, dev_off, ret = 0; - u32 dev_handle; + unsigned int dev_handle; #ifdef CONFIG_FSL_CORENET int num; u32 liodns[2]; @@ -142,11 +142,9 @@ static int fdt_qportal(void *blob, int off, int id, char *name, if (childoff > 0) { dev_handle = fdt_get_phandle(blob, dev_off); if (dev_handle <= 0) { - dev_handle = fdt_alloc_phandle(blob); - ret = fdt_set_phandle(blob, dev_off, - dev_handle); - if (ret < 0) - return ret; + dev_handle = fdt_create_phandle(blob, dev_off); + if (!dev_handle) + return -FDT_ERR_NOPHANDLES; } ret = fdt_setprop(blob, childoff, "dev-handle", -- 2.32.0
[PATCH u-boot-marvell RESEND 03/11] fdt_support: Remove fdt_alloc_phandle() in favor of fdt_generate_phandle()
From: Marek Behún Commit f0921f5098d ("fdt: Sync up to the latest libfdt") introduced fdt_generate_phandle() in libfdt, making fdt_alloc_phandle() obsolete in fdt_support. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Cc: Simon Glass Cc: "hui.song" Cc: Meenakshi Aggarwal Cc: Priyanka Jain Cc: Ioana Ciornei --- board/freescale/lx2160a/eth_lx2160aqds.c | 8 +-- board/freescale/lx2160a/eth_lx2162aqds.c | 8 +-- common/fdt_support.c | 28 +++- include/fdt_support.h| 1 - 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/board/freescale/lx2160a/eth_lx2160aqds.c b/board/freescale/lx2160a/eth_lx2160aqds.c index a2b6442b54..1819b27561 100644 --- a/board/freescale/lx2160a/eth_lx2160aqds.c +++ b/board/freescale/lx2160a/eth_lx2160aqds.c @@ -775,10 +775,11 @@ int fdt_fixup_board_phy(void *fdt) int fpga_offset, offset, subnodeoffset; struct mii_dev *mii_dev; struct list_head *mii_devs, *entry; - int ret, dpmac_id, phandle, i; + int ret, dpmac_id, i; struct phy_device *phy_dev; char ethname[ETH_NAME_LEN]; phy_interface_t phy_iface; + uint32_t phandle; ret = 0; /* we know FPGA is connected to i2c0, therefore search path directly, @@ -794,7 +795,10 @@ int fdt_fixup_board_phy(void *fdt) return fpga_offset; } - phandle = fdt_alloc_phandle(fdt); + ret = fdt_generate_phandle(fdt, &phandle); + if (ret < 0) + return ret; + mii_devs = mdio_get_list_head(); list_for_each(entry, mii_devs) { diff --git a/board/freescale/lx2160a/eth_lx2162aqds.c b/board/freescale/lx2160a/eth_lx2162aqds.c index 3b04dea39c..ac6218ebe4 100644 --- a/board/freescale/lx2160a/eth_lx2162aqds.c +++ b/board/freescale/lx2160a/eth_lx2162aqds.c @@ -787,10 +787,11 @@ int fdt_fixup_board_phy(void *fdt) int fpga_offset, offset, subnodeoffset; struct mii_dev *mii_dev; struct list_head *mii_devs, *entry; - int ret, dpmac_id, phandle, i; + int ret, dpmac_id, i; struct phy_device *phy_dev; char ethname[ETH_NAME_LEN]; phy_interface_t phy_iface; + uint32_t phandle; ret = 0; /* we know FPGA is connected to i2c0, therefore search path directly, @@ -806,7 +807,10 @@ int fdt_fixup_board_phy(void *fdt) return fpga_offset; } - phandle = fdt_alloc_phandle(fdt); + ret = fdt_generate_phandle(fdt, &phandle); + if (ret < 0) + return ret; + mii_devs = mdio_get_list_head(); list_for_each(entry, mii_devs) { diff --git a/common/fdt_support.c b/common/fdt_support.c index 8992ac5d3f..be03a87d42 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1463,24 +1463,6 @@ int fdt_node_offset_by_compat_reg(void *blob, const char *compat, return -FDT_ERR_NOTFOUND; } -/** - * fdt_alloc_phandle: Return next free phandle value - * - * @blob: ptr to device tree - */ -int fdt_alloc_phandle(void *blob) -{ - int offset; - uint32_t phandle = 0; - - for (offset = fdt_next_node(blob, -1, NULL); offset >= 0; -offset = fdt_next_node(blob, offset, NULL)) { - phandle = max(phandle, fdt_get_phandle(blob, offset)); - } - - return phandle + 1; -} - /* * fdt_set_phandle: Create a phandle property for the given node * @@ -1530,13 +1512,19 @@ int fdt_set_phandle(void *fdt, int nodeoffset, uint32_t phandle) unsigned int fdt_create_phandle(void *fdt, int nodeoffset) { /* see if there is a phandle already */ - int phandle = fdt_get_phandle(fdt, nodeoffset); + uint32_t phandle = fdt_get_phandle(fdt, nodeoffset); /* if we got 0, means no phandle so create one */ if (phandle == 0) { int ret; - phandle = fdt_alloc_phandle(fdt); + ret = fdt_generate_phandle(fdt, &phandle); + if (ret < 0) { + printf("Can't generate phandle: %s\n", + fdt_strerror(ret)); + return 0; + } + ret = fdt_set_phandle(fdt, nodeoffset, phandle); if (ret < 0) { printf("Can't set phandle %u: %s\n", phandle, diff --git a/include/fdt_support.h b/include/fdt_support.h index 88d129c803..90f5a4c28c 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -285,7 +285,6 @@ int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu, int fdt_node_offset_by_compat_reg(void *blob, const char *compat, phys_addr_t compat_off); -int fdt_alloc_phandle(void *blob); int fdt_set_phandle(void *fdt, int nodeoffset, uint32_t phandle); unsigned int fdt_create_phandle(void *fdt, int nodeoffset); int fdt_add_edid(void *blob, const char *compat, unsigned char *buf); --
[PATCH u-boot-marvell RESEND 05/11] fdt_support: Fix comment for fdt_create_phandle()
From: Marek Behún This function does not necessarily create a new phandle. If a phandle exists, no new phandle is created. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- common/fdt_support.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/fdt_support.c b/common/fdt_support.c index 8ac905011c..df111f708c 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1504,7 +1504,7 @@ int fdt_set_phandle(void *fdt, int nodeoffset, uint32_t phandle) } /* - * fdt_create_phandle: Create a phandle property for the given node + * fdt_create_phandle: Get or create a phandle property for the given node * * @fdt: ptr to device tree * @nodeoffset: node to update -- 2.32.0
[PATCH u-boot-marvell RESEND 07/11] arm: mvebu: turris_mox: Find DT nodes by compatible or alias instead of path
From: Marek Behún It is better to find DT nodes by compatible strings or aliases instead of path. There were issues with Linux some DTBs having different names of some nodes, e.g. internal-regs instead of internal-regs@d000 This should be a generic fix for such issues. Also since fdt_support now contains needed functions, we can drop our own implementations. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- board/CZ.NIC/turris_mox/turris_mox.c | 186 +-- 1 file changed, 63 insertions(+), 123 deletions(-) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 52fa77d68c..77d1c01175 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -41,22 +41,14 @@ #define ARMADA_37XX_SPI_DOUT (MVEBU_REGISTER(0x10608)) #define ARMADA_37XX_SPI_DIN(MVEBU_REGISTER(0x1060c)) -#define ETH1_PATH "/soc/internal-regs@d000/ethernet@4" -#define MDIO_PATH "/soc/internal-regs@d000/mdio@32004" -#define SFP_GPIO_PATH "/soc/internal-regs@d000/spi@10600/moxtet@1/gpio@0" -#define PCIE_PATH "/soc/pcie@d007" -#define SFP_PATH "/sfp" -#define LED_PATH "/leds/led" -#define BUTTON_PATH"/gpio-keys/reset" - DECLARE_GLOBAL_DATA_PTR; #if defined(CONFIG_OF_BOARD_FIXUP) int board_fix_fdt(void *blob) { u8 topology[MAX_MOX_MODULES]; - int i, size, node; - bool enable; + enum fdt_status status; + int i, size, ret; /* * SPI driver is not loaded in driver model yet, but we have to find out @@ -94,21 +86,15 @@ int board_fix_fdt(void *blob) if (size > 1 && (topology[1] == MOX_MODULE_PCI || topology[1] == MOX_MODULE_USB3 || topology[1] == MOX_MODULE_PASSPCI)) - enable = true; + status = FDT_STATUS_OKAY; else - enable = false; - - node = fdt_path_offset(blob, PCIE_PATH); + status = FDT_STATUS_DISABLED; - if (node < 0) { - printf("Cannot find PCIe node in U-Boot's device tree!\n"); - return 0; - } - - if (fdt_setprop_string(blob, node, "status", - enable ? "okay" : "disabled") < 0) { - printf("Cannot %s PCIe in U-Boot's device tree!\n", - enable ? "enable" : "disable"); + ret = fdt_set_status_by_compatible(blob, "marvell,armada-3700-pcie", + status); + if (ret < 0) { + printf("Cannot set status for PCIe in U-Boot's device tree: %s!\n", + fdt_strerror(ret)); return 0; } @@ -416,12 +402,18 @@ static bool read_reset_button(void) struct udevice *button, *led; int i; - if (device_get_global_by_ofnode(ofnode_path(BUTTON_PATH), &button)) { + if (device_get_global_by_ofnode( + ofnode_first_subnode(ofnode_by_compatible(ofnode_null(), + "gpio-keys")), + &button)) { printf("Cannot find reset button!\n"); return false; } - if (device_get_global_by_ofnode(ofnode_path(LED_PATH), &led)) { + if (device_get_global_by_ofnode( + ofnode_first_subnode(ofnode_by_compatible(ofnode_null(), + "gpio-leds")), + &led)) { printf("Cannot find status LED!\n"); return false; } @@ -661,92 +653,34 @@ handle_reset_btn: #if defined(CONFIG_OF_BOARD_SETUP) -static int vnode_by_path(void *blob, const char *fmt, va_list ap) +static bool is_topaz(int id) { - char path[128]; - - vsnprintf(path, 128, fmt, ap); - return fdt_path_offset(blob, path); + return topaz && id == peridot + topaz - 1; } -static int node_by_path(void *blob, const char *fmt, ...) +static int switch_addr(int id) { - va_list ap; - int res; - - va_start(ap, fmt); - res = vnode_by_path(blob, fmt, ap); - va_end(ap); - - return res; + return is_topaz(id) ? 0x2 : 0x10 + id; } -static int phandle_by_path(void *blob, const char *fmt, ...) +static int setup_switch(void *blob, int id) { - va_list ap; - int node, phandle, res; - - va_start(ap, fmt); - node = vnode_by_path(blob, fmt, ap); - va_end(ap); + int res, addr, i, node; + char mdio_path[64]; + node = fdt_node_offset_by_compatible(blob, -1, "marvell,orion-mdio"); if (node < 0) return node; - phandle = fdt_get_phandle(blob, node); - if (phandle > 0) - return phandle; - - phandle = fdt_get_max_phandle(blob); - if (phandle < 0) - return phandle; - -
[PATCH u-boot-marvell RESEND 04/11] fdt_support: Remove FDT_STATUS_FAIL_ERROR_CODE
From: Marek Behún Since no one uses this feature and I am not aware of any parsers of this in Linux, remove it. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Cc: Simon Glass Cc: Andy Shevchenko Cc: Pratyush Yadav Cc: Tim Harvey Cc: Michael Walle Cc: Priyanka Jain --- arch/arm/cpu/armv7/ls102xa/fdt.c | 6 +++--- board/gateworks/gw_ventana/common.c | 3 +-- board/kontron/sl28/sl28.c| 2 +- common/fdt_support.c | 20 +--- drivers/pci/pcie_layerscape_fixup.c | 8 drivers/pci/pcie_layerscape_gen4_fixup.c | 8 include/fdt_support.h| 18 -- 7 files changed, 26 insertions(+), 39 deletions(-) diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c index 0daf8234fb..bf6cc6d4e7 100644 --- a/arch/arm/cpu/armv7/ls102xa/fdt.c +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c @@ -184,13 +184,13 @@ void ft_cpu_setup(void *blob, struct bd_info *bd) #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI) off = fdt_node_offset_by_compat_reg(blob, FSL_IFC_COMPAT, CONFIG_SYS_IFC_ADDR); - fdt_set_node_status(blob, off, FDT_STATUS_DISABLED, 0); + fdt_set_node_status(blob, off, FDT_STATUS_DISABLED); #else off = fdt_node_offset_by_compat_reg(blob, FSL_QSPI_COMPAT, QSPI0_BASE_ADDR); - fdt_set_node_status(blob, off, FDT_STATUS_DISABLED, 0); + fdt_set_node_status(blob, off, FDT_STATUS_DISABLED); off = fdt_node_offset_by_compat_reg(blob, FSL_DSPI_COMPAT, DSPI1_BASE_ADDR); - fdt_set_node_status(blob, off, FDT_STATUS_DISABLED, 0); + fdt_set_node_status(blob, off, FDT_STATUS_DISABLED); #endif } diff --git a/board/gateworks/gw_ventana/common.c b/board/gateworks/gw_ventana/common.c index 2be921f47a..7ec931c8a8 100644 --- a/board/gateworks/gw_ventana/common.c +++ b/board/gateworks/gw_ventana/common.c @@ -1681,8 +1681,7 @@ void ft_early_fixup(void *blob, int board_type) * disable serial2 node for GW54xx for compatibility with older * 3.10.x kernel that improperly had this node enabled in the DT */ - fdt_set_status_by_alias(blob, "serial2", FDT_STATUS_DISABLED, - 0); + fdt_set_status_by_alias(blob, "serial2", FDT_STATUS_DISABLED); /* GW54xx-E adds WDOG2_B external reset */ if (rev < 'E') diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c index c8ed7ac81a..e84b356918 100644 --- a/board/kontron/sl28/sl28.c +++ b/board/kontron/sl28/sl28.c @@ -75,7 +75,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) if (CONFIG_IS_ENABLED(SL28_SPL_LOADS_OPTEE_BL32)) { node = fdt_node_offset_by_compatible(blob, -1, "linaro,optee-tz"); if (node) - fdt_set_node_status(blob, node, FDT_STATUS_OKAY, 0); + fdt_set_node_status(blob, node, FDT_STATUS_OKAY); } return 0; diff --git a/common/fdt_support.c b/common/fdt_support.c index be03a87d42..8ac905011c 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1541,14 +1541,10 @@ unsigned int fdt_create_phandle(void *fdt, int nodeoffset) * * @fdt: ptr to device tree * @nodeoffset: node to update - * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, - * FDT_STATUS_FAIL, FDT_STATUS_FAIL_ERROR_CODE - * @error_code: optional, only used if status is FDT_STATUS_FAIL_ERROR_CODE + * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, FDT_STATUS_FAIL */ -int fdt_set_node_status(void *fdt, int nodeoffset, - enum fdt_status status, unsigned int error_code) +int fdt_set_node_status(void *fdt, int nodeoffset, enum fdt_status status) { - char buf[16]; int ret = 0; if (nodeoffset < 0) @@ -1564,10 +1560,6 @@ int fdt_set_node_status(void *fdt, int nodeoffset, case FDT_STATUS_FAIL: ret = fdt_setprop_string(fdt, nodeoffset, "status", "fail"); break; - case FDT_STATUS_FAIL_ERROR_CODE: - sprintf(buf, "fail-%d", error_code); - ret = fdt_setprop_string(fdt, nodeoffset, "status", buf); - break; default: printf("Invalid fdt status: %x\n", status); ret = -1; @@ -1582,16 +1574,14 @@ int fdt_set_node_status(void *fdt, int nodeoffset, * * @fdt: ptr to device tree * @alias: alias of node to update - * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, - * FDT_STATUS_FAIL, FDT_STATUS_FAIL_ERROR_CODE - * @error_code: optional, only used if status is FDT_STATUS_FAIL_ERROR_CODE + * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, FDT_STATUS_FAIL */ int fdt_set_status_by_alias(void *fdt, const char* alias, -
[PATCH u-boot-marvell RESEND 06/11] fdt_support: Add some useful functions
From: Marek Behún Add functions fdt_node_offset_by_pathf(), fdt_create_phandle_by_pathf(), fdt_set_status_by_pathf() to get node offset, get/create node phandle and set status for node given by path/alias formatted with sprintf. Add functions fdt_create_phandle_by_compatible(), fdt_set_status_by_compatible() to get/create node phandle and set status for first node given by compatible. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- common/fdt_support.c | 119 ++ include/fdt_support.h | 30 +++ 2 files changed, 149 insertions(+) diff --git a/common/fdt_support.c b/common/fdt_support.c index df111f708c..c2e16727e1 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1463,6 +1463,37 @@ int fdt_node_offset_by_compat_reg(void *blob, const char *compat, return -FDT_ERR_NOTFOUND; } +static int vnode_offset_by_pathf(void *blob, const char *fmt, va_list ap) +{ + char path[512]; + int len; + + len = vsnprintf(path, sizeof(path), fmt, ap); + if (len < 0 || len + 1 > sizeof(path)) + return -FDT_ERR_NOSPACE; + + return fdt_path_offset(blob, path); +} + +/** + * fdt_node_offset_by_pathf: Find node offset by sprintf formatted path + * + * @blob: ptr to device tree + * @fmt: path format + * @ap: vsnprintf arguments + */ +int fdt_node_offset_by_pathf(void *blob, const char *fmt, ...) +{ + va_list ap; + int res; + + va_start(ap, fmt); + res = vnode_offset_by_pathf(blob, fmt, ap); + va_end(ap); + + return res; +} + /* * fdt_set_phandle: Create a phandle property for the given node * @@ -1536,6 +1567,51 @@ unsigned int fdt_create_phandle(void *fdt, int nodeoffset) return phandle; } +/** + * fdt_create_phandle_by_compatible: Get or create a phandle for first node with + * given compatible + * + * @fdt: ptr to device tree + * @compat: node's compatible string + */ +unsigned int fdt_create_phandle_by_compatible(void *fdt, const char *compat) +{ + int offset = fdt_node_offset_by_compatible(fdt, -1, compat); + + if (offset < 0) { + printf("Can't find node with compatible \"%s\": %s\n", compat, + fdt_strerror(offset)); + return 0; + } + + return fdt_create_phandle(fdt, offset); +} + +/** + * fdt_create_phandle_by_pathf: Get or create a phandle for node given by + * sprintf-formatted path + * + * @fdt: ptr to device tree + * @fmt, ...: path format string and arguments to pass to sprintf + */ +unsigned int fdt_create_phandle_by_pathf(void *fdt, const char *fmt, ...) +{ + va_list ap; + int offset; + + va_start(ap, fmt); + offset = vnode_offset_by_pathf(fdt, fmt, ap); + va_end(ap); + + if (offset < 0) { + printf("Can't find node by given path: %s\n", + fdt_strerror(offset)); + return 0; + } + + return fdt_create_phandle(fdt, offset); +} + /* * fdt_set_node_status: Set status for the given node * @@ -1584,6 +1660,49 @@ int fdt_set_status_by_alias(void *fdt, const char* alias, return fdt_set_node_status(fdt, offset, status); } +/** + * fdt_set_status_by_compatible: Set node status for first node with given + * compatible + * + * @fdt: ptr to device tree + * @compat: node's compatible string + * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, FDT_STATUS_FAIL + */ +int fdt_set_status_by_compatible(void *fdt, const char *compat, +enum fdt_status status) +{ + int offset = fdt_node_offset_by_compatible(fdt, -1, compat); + + if (offset < 0) + return offset; + + return fdt_set_node_status(fdt, offset, status); +} + +/** + * fdt_set_status_by_pathf: Set node status for node given by sprintf-formatted + * path + * + * @fdt: ptr to device tree + * @status: FDT_STATUS_OKAY, FDT_STATUS_DISABLED, FDT_STATUS_FAIL + * @fmt, ...: path format string and arguments to pass to sprintf + */ +int fdt_set_status_by_pathf(void *fdt, enum fdt_status status, const char *fmt, + ...) +{ + va_list ap; + int offset; + + va_start(ap, fmt); + offset = vnode_offset_by_pathf(fdt, fmt, ap); + va_end(ap); + + if (offset < 0) + return offset; + + return fdt_set_node_status(fdt, offset, status); +} + #if defined(CONFIG_VIDEO) || defined(CONFIG_LCD) int fdt_add_edid(void *blob, const char *compat, unsigned char *edid_buf) { diff --git a/include/fdt_support.h b/include/fdt_support.h index 850c860bd4..d40586725b 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -285,8 +285,13 @@ int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu, int fdt_node_offset_by_compat_reg(void *blob, const char *compat,
[PATCH u-boot-marvell RESEND 08/11] arm: mvebu: turris_mox: Enable eth1 in U-Boot if a network module is present
From: Pali Rohár Enable eth1 node in U-Boot's device-tree if a network module (SFP, Topaz or Peridot) is detected. This is required for proper detection of eth1 comphy in a3700 comphy driver by the following patches. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- board/CZ.NIC/turris_mox/turris_mox.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 77d1c01175..06616257d7 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -46,8 +46,8 @@ DECLARE_GLOBAL_DATA_PTR; #if defined(CONFIG_OF_BOARD_FIXUP) int board_fix_fdt(void *blob) { + enum fdt_status status_pcie, status_eth1; u8 topology[MAX_MOX_MODULES]; - enum fdt_status status; int i, size, ret; /* @@ -65,6 +65,9 @@ int board_fix_fdt(void *blob) while (!(readl(ARMADA_37XX_SPI_CTRL) & 0x2)) udelay(1); + status_pcie = FDT_STATUS_DISABLED; + status_eth1 = FDT_STATUS_DISABLED; + for (i = 0; i < MAX_MOX_MODULES; ++i) { writel(0x0, ARMADA_37XX_SPI_DOUT); @@ -76,6 +79,11 @@ int board_fix_fdt(void *blob) break; topology[i] &= 0xf; + + if (topology[i] == MOX_MODULE_SFP || + topology[i] == MOX_MODULE_TOPAZ || + topology[i] == MOX_MODULE_PERIDOT) + status_eth1 = FDT_STATUS_OKAY; } size = i; @@ -83,15 +91,18 @@ int board_fix_fdt(void *blob) /* disable SPI CS1 */ clrbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17)); + ret = fdt_set_status_by_alias(blob, "ethernet1", status_eth1); + if (ret < 0) + printf("Cannot set status for eth1 in U-Boot's device tree: %s!\n", + fdt_strerror(ret)); + if (size > 1 && (topology[1] == MOX_MODULE_PCI || topology[1] == MOX_MODULE_USB3 || topology[1] == MOX_MODULE_PASSPCI)) - status = FDT_STATUS_OKAY; - else - status = FDT_STATUS_DISABLED; + status_pcie = FDT_STATUS_OKAY; ret = fdt_set_status_by_compatible(blob, "marvell,armada-3700-pcie", - status); + status_pcie); if (ret < 0) { printf("Cannot set status for PCIe in U-Boot's device tree: %s!\n", fdt_strerror(ret)); -- 2.32.0
[PATCH u-boot-marvell RESEND 11/11] fdt_support: Add fdt_delete_disabled_nodes() and use in Turris MOX
From: Marek Behún Move Turris MOX specific remove_disabled_nodes() to fdt_support with name fdt_delete_disabled_nodes(), so that others can potentially use it. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- board/CZ.NIC/turris_mox/turris_mox.c | 20 +--- common/fdt_support.c | 23 +++ include/fdt_support.h| 2 ++ 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index fab41c04b5..3eb5cb4256 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -746,24 +746,6 @@ static int setup_switch(void *blob, int id) return 0; } -static int remove_disabled_nodes(void *blob) -{ - while (1) { - int res, offset; - - offset = fdt_node_offset_by_prop_value(blob, -1, "status", - "disabled", 9); - if (offset < 0) - break; - - res = fdt_del_node(blob, offset); - if (res < 0) - return res; - } - - return 0; -} - int ft_board_setup(void *blob, struct bd_info *bd) { int res; @@ -869,7 +851,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) fdt_fixup_ethernet(blob); /* Finally remove disabled nodes, as per Rob Herring's request. */ - remove_disabled_nodes(blob); + fdt_delete_disabled_nodes(blob); return 0; } diff --git a/common/fdt_support.c b/common/fdt_support.c index c2e16727e1..b2ba0825df 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -695,6 +695,29 @@ int fdt_shrink_to_minimum(void *blob, uint extrasize) return actualsize; } +/** + * fdt_delete_disabled_nodes: Delete all nodes with status == "disabled" + * + * @blob: ptr to device tree + */ +int fdt_delete_disabled_nodes(void *blob) +{ + while (1) { + int ret, offset; + + offset = fdt_node_offset_by_prop_value(blob, -1, "status", + "disabled", 9); + if (offset < 0) + break; + + ret = fdt_del_node(blob, offset); + if (ret < 0) + return ret; + } + + return 0; +} + #ifdef CONFIG_PCI #define CONFIG_SYS_PCI_NR_INBOUND_WIN 4 diff --git a/include/fdt_support.h b/include/fdt_support.h index d40586725b..8ec461af6c 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -228,6 +228,8 @@ void set_working_fdt_addr(ulong addr); int fdt_shrink_to_minimum(void *blob, uint extrasize); int fdt_increase_size(void *fdt, int add_len); +int fdt_delete_disabled_nodes(void *blob); + int fdt_fixup_nor_flash_size(void *blob); struct node_info; -- 2.32.0
[PATCH u-boot-marvell RESEND 10/11] arm: mvebu: turris_mox: Fix unstable board topology reading
From: Marek Behún The pre-relocation board topology reading in board_fix_fdt() is unstable: sometimes wrong data are read from the SPI bus. This is due to wrong order of SPI bus configuration instructions: we first need to set the pins to SPI mode, and only after that configure the bus. Also add a 1ms delay before enabling chip-select, so that the clock pin is high for some time before reading the bus. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- board/CZ.NIC/turris_mox/turris_mox.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 06616257d7..fab41c04b5 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -56,9 +56,11 @@ int board_fix_fdt(void *blob) * to read SPI by reading/writing SPI registers directly */ - writel(0x10df, ARMADA_37XX_SPI_CFG); /* put pin from GPIO to SPI mode */ clrbits_le32(ARMADA_37XX_NB_GPIO_SEL, BIT(12)); + /* configure cpol, cpha, prescale */ + writel(0x10df, ARMADA_37XX_SPI_CFG); + mdelay(1); /* enable SPI CS1 */ setbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17)); -- 2.32.0
[PATCH u-boot-marvell RESEND 09/11] phy: marvell: a3700: Convert to official DT bindings in COMPHY driver
From: Pali Rohár Convert A3720 common PHY driver to official DT bindings. This puts us closer to be able to synchronize A3720 device-trees with those from Linux. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Cc: Konstantin Porotchkin Cc: Robert Marko Cc: Luka Perkov Cc: Marcin Wojtas Cc: Grzegorz Jaszczyk --- arch/arm/dts/armada-3720-espressobin.dts | 21 +--- arch/arm/dts/armada-3720-turris-mox.dts | 25 ++--- arch/arm/dts/armada-3720-uDPU.dts| 23 +--- arch/arm/dts/armada-37xx.dtsi| 20 +++- drivers/phy/marvell/comphy_a3700.c | 133 +++ drivers/phy/marvell/comphy_core.c| 59 +- drivers/phy/marvell/comphy_core.h| 23 drivers/phy/marvell/comphy_cp110.c | 58 ++ 8 files changed, 250 insertions(+), 112 deletions(-) diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts index cba6139be6..360d521bba 100644 --- a/arch/arm/dts/armada-3720-espressobin.dts +++ b/arch/arm/dts/armada-3720-espressobin.dts @@ -80,24 +80,6 @@ }; }; -&comphy { - max-lanes = <3>; - phy0 { - phy-type = ; - phy-speed = ; - }; - - phy1 { - phy-type = ; - phy-speed = ; - }; - - phy2 { - phy-type = ; - phy-speed = ; - }; -}; - ð0 { status = "okay"; pinctrl-names = "default"; @@ -119,6 +101,7 @@ /* CON3 */ &sata { status = "okay"; + phys = <&comphy2 0>; }; &sdhci0 { @@ -200,6 +183,7 @@ /* CON31 */ &usb3 { status = "okay"; + phys = <&comphy0 0>; }; &pcie0 { @@ -207,4 +191,5 @@ pinctrl-0 = <&pcie_pins>; reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>; status = "okay"; + phys = <&comphy1 0>; }; diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts index f47ced05c5..d01757062f 100644 --- a/arch/arm/dts/armada-3720-turris-mox.dts +++ b/arch/arm/dts/armada-3720-turris-mox.dts @@ -94,24 +94,6 @@ }; }; -&comphy { - max-lanes = <3>; - phy0 { - phy-type = ; - phy-speed = ; - }; - - phy1 { - phy-type = ; - phy-speed = ; - }; - - phy2 { - phy-type = ; - phy-speed = ; - }; -}; - ð0 { status = "okay"; pinctrl-names = "default"; @@ -120,6 +102,11 @@ phy = <ð_phy1>; }; +ð1 { + phy-mode = "2500base-x"; + phys = <&comphy0 1>; +}; + &i2c0 { pinctrl-names = "default"; pinctrl-0 = <&i2c1_pins>; @@ -222,6 +209,7 @@ &usb3 { vbus-supply = <®_usb3_vbus>; status = "okay"; + phys = <&comphy2 0>; }; &pcie0 { @@ -229,4 +217,5 @@ pinctrl-0 = <&pcie_pins>; reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>; status = "disabled"; + phys = <&comphy1 0>; }; diff --git a/arch/arm/dts/armada-3720-uDPU.dts b/arch/arm/dts/armada-3720-uDPU.dts index 4bf6d2eac7..58557c680a 100644 --- a/arch/arm/dts/armada-3720-uDPU.dts +++ b/arch/arm/dts/armada-3720-uDPU.dts @@ -106,36 +106,21 @@ }; }; -&comphy { - phy0 { - phy-type = ; - phy-speed = ; - }; - - phy1 { - phy-type = ; - phy-speed = ; - }; - - phy2 { - phy-type = ; - phy-speed = ; - }; -}; - ð0 { pinctrl-0 = <&pcie_pins>; status = "okay"; - phy-mode = "2500base-x"; + phy-mode = "sgmii"; managed = "in-band-status"; phy = <ðphy0>; + phys = <&comphy1 0>; }; ð1 { status = "okay"; - phy-mode = "2500base-x"; + phy-mode = "sgmii"; managed = "in-band-status"; phy = <ðphy1>; + phys = <&comphy0 1>; }; &i2c0 { diff --git a/arch/arm/dts/armada-37xx.dtsi b/arch/arm/dts/armada-37xx.dtsi index fec34609cf..bef6ef03df 100644 --- a/arch/arm/dts/armada-37xx.dtsi +++ b/arch/arm/dts/armada-37xx.dtsi @@ -316,9 +316,23 @@ compatible = "marvell,mvebu-comphy", "marvell,comphy-armada-3700"; reg = <0x18300 0x28>, <0x1f300 0x3d000>; - mux-bitcount = <4>; - mux-lane-order = <1 0 2>; - max-lanes = <3>; + #address-cells = <1>; + #size-cells = <0>; + + comphy0: phy@0 { + reg = <0>; + #phy-cells = <1>; + }; + + comphy1: phy@1 { + reg = <1>; + #phy-cells = <1>; + }; + +
Re: a question about falcon mode
On 11/26/21 1:53 AM, Chan Kim wrote: Hi Alex, Thanks for the reply. So I gather that to be able to use 'spl export fdt' to store the 'snapshot' to the storage I should make the spl program runnable at least to that stage. (being able to load kernel image, dtb, initrd and give the spl export command to capture it to storage like SD card). What I'm thinking of is to let another processor(cortext-M7 based scp, system control processor) load the u-boot-spl.bin into on-chip RAM and kernel(including initramfs)and dtb into SDRAM all from SD card (RAW mode, no file system) and let the AP(application processor) start from u-boot-spl.bin. In this scheme u-boot-spl doesn't need to load anything and DRAM is already initialized and it just needs to run from kernel image after very minimal setup (passing kernel arguments). Do you think this is possible or an absurd idea? Any comment will be a big help for me. I'm not sure if that mode is currently supported. You could try using CONFIG_SPL_RAM_SUPPORT=y, and one of two things: a) Package your kernel and DTB in a FIT. b) Extend common/spl/spl_ram.c to support the 'snapshot' dtb Alex Thanks! Chan -Original Message- From: Alex G. Sent: Thursday, November 25, 2021 11:57 PM To: Chan Kim ; U-Boot Mailing List Subject: Re: a question about falcon mode On 11/25/21 1:07 AM, Chan Kim wrote: Hello all, I'm trying to implement falcon mode for our board. Then should I first implement the normal mode(spl + proper)? It looks like so while I'm reading doc/README.falcon. (It says, after loading kernel, DT etc. I should give 'spl export' command). Falcon mode is a bit board dependent. There are a couple of ways you could go about this. The first is to have an "fdtargs" partition. This is where "spl export" comes in. Once you run "spl export", it will give a modified dtb at "$fdtargsaddr". It's that DTB that you need to write to your ftdargs partition. For example: > spl export fdt $loadaddr - $fdt_addr_r > mmc write $fdtargsaddr 0x9800 0x8000 In this example the ftdargs partition starts at sector 0x9800, and is 0x800 sectors long. The second option is to forget about "spl export" and "fdtargs", and package your kernel, devicetree, and overlays in a FIT container. You'd make sure to enable SPL_LOAD_FIT_APPLY_OVERLAY. There isn't much more to this other than the usual gotcha's with FIT and overlays. Alex
[PATCH u-boot-marvell v2 0/9] More verifications for kwbimage in SPL
From: Marek Behún Hello Stefan, this is v2 of series that adds more checks for kwbimage validity and consistency to SPL, mainly checking image data checksum. Changes since v1: - updated error messages as requested by Stefan - fixed checkpatch warnings for uintN_t types (converted to preferred uN) - added more checkpatch fixes Marek Marek Behún (4): arm: mvebu: spl: Print srcaddr in error message arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible arm: mvebu: spl: Fix 100 column exceeds Pali Rohár (5): arm: mvebu: Check that kwbimage offset and blocksize are valid SPL: Add struct spl_boot_device parameter into spl_parse_board_header() arm: mvebu: Check that kwbimage blockid matches boot mode SPL: Add support for checking board / BootROM specific image types arm: mvebu: Check for kwbimage data checksum arch/arm/mach-mvebu/spl.c | 153 +++- arch/arm/mach-sunxi/spl_spi_sunxi.c | 2 +- common/spl/spl.c| 13 ++- common/spl/spl_ext.c| 9 +- common/spl/spl_fat.c| 11 +- common/spl/spl_legacy.c | 3 +- common/spl/spl_mmc.c| 43 +--- common/spl/spl_nand.c | 5 +- common/spl/spl_net.c| 2 +- common/spl/spl_nor.c| 4 +- common/spl/spl_onenand.c| 2 +- common/spl/spl_ram.c| 2 +- common/spl/spl_sata.c | 9 +- common/spl/spl_sdp.c| 2 +- common/spl/spl_spi.c| 9 +- common/spl/spl_ubi.c| 4 +- common/spl/spl_usb.c| 4 +- common/spl/spl_xip.c| 4 +- common/spl/spl_ymodem.c | 4 +- drivers/usb/gadget/f_sdp.c | 12 ++- include/sdp.h | 3 +- include/spl.h | 7 ++ 22 files changed, 201 insertions(+), 106 deletions(-) -- 2.32.0
[PATCH u-boot-marvell v2 1/9] arm: mvebu: Check that kwbimage offset and blocksize are valid
From: Pali Rohár There are certain restrictions for kwbimage offset and blocksize. Validate them. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- Changes since v1: - updated error messages as requested by Stefan --- arch/arm/mach-mvebu/spl.c | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 73c4b9af3e..a6ed4367dd 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -163,6 +163,18 @@ int spl_parse_board_header(struct spl_image_info *spl_image, spl_image->offset *= 512; #endif + if (spl_image->offset % 4 != 0) { + printf("ERROR: Wrong srcaddr (%u) in kwbimage\n", + spl_image->offset); + return -EINVAL; + } + + if (mhdr->blocksize <= 4 || mhdr->blocksize % 4 != 0) { + printf("ERROR: Wrong blocksize (%u) in kwbimage\n", + mhdr->blocksize); + return -EINVAL; + } + spl_image->size = mhdr->blocksize; spl_image->entry_point = mhdr->execaddr; spl_image->load_addr = mhdr->destaddr; -- 2.32.0
[PATCH u-boot-marvell v2 3/9] arm: mvebu: Check that kwbimage blockid matches boot mode
From: Pali Rohár Each boot mode has its own kwbimage specified by blockid. So check that kwbimage is valid by blockid. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- Changes since v1: - updated error messages as requested by Stefan --- arch/arm/mach-mvebu/spl.c | 35 ++- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 716217083b..4af52c626c 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -118,22 +118,39 @@ int spl_parse_board_header(struct spl_image_info *spl_image, * (including SPL content) which is not included in U-Boot image_header. */ if (mhdr->version != 1 || - ((mhdr->headersz_msb << 16) | mhdr->headersz_lsb) < sizeof(*mhdr) || - ( + ((mhdr->headersz_msb << 16) | mhdr->headersz_lsb) < sizeof(*mhdr)) { + printf("ERROR: Invalid kwbimage v1\n"); + return -EINVAL; + } + #ifdef CONFIG_SPL_SPI_FLASH_SUPPORT -mhdr->blockid != IBR_HDR_SPI_ID && + if (bootdev->boot_device == BOOT_DEVICE_SPI && + mhdr->blockid != IBR_HDR_SPI_ID) { + printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n", + mhdr->blockid); + return -EINVAL; + } #endif + #ifdef CONFIG_SPL_SATA -mhdr->blockid != IBR_HDR_SATA_ID && + if (bootdev->boot_device == BOOT_DEVICE_SATA && + mhdr->blockid != IBR_HDR_SATA_ID) { + printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n", + mhdr->blockid); + return -EINVAL; + } #endif + #ifdef CONFIG_SPL_MMC -mhdr->blockid != IBR_HDR_SDIO_ID && -#endif -1 - )) { - printf("ERROR: Not valid SPI/NAND/SATA/SDIO kwbimage v1\n"); + if ((bootdev->boot_device == BOOT_DEVICE_MMC1 || +bootdev->boot_device == BOOT_DEVICE_MMC2 || +bootdev->boot_device == BOOT_DEVICE_MMC2_2) && + mhdr->blockid != IBR_HDR_SDIO_ID) { + printf("ERROR: Wrong blockid (%u) in SDIO kwbimage\n", + mhdr->blockid); return -EINVAL; } +#endif spl_image->offset = mhdr->srcaddr; -- 2.32.0
[PATCH u-boot-marvell v2 4/9] SPL: Add support for checking board / BootROM specific image types
From: Pali Rohár Commit 9baab60b8054 ("SPL: Add support for parsing board / BootROM specific image types") added support for loading board specific image types. This commit adds support for a new weak function spl_parse_board_header() which is called after loading boot image. Board may implement this function for checking if loaded board specific image is valid. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- common/spl/spl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/common/spl/spl.c b/common/spl/spl.c index bf2139a058..cc3b3b3438 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -589,6 +589,12 @@ static struct spl_image_loader *spl_ll_find_loader(uint boot_device) return NULL; } +__weak int spl_check_board_image(struct spl_image_info *spl_image, +const struct spl_boot_device *bootdev) +{ + return 0; +} + static int spl_load_image(struct spl_image_info *spl_image, struct spl_image_loader *loader) { @@ -610,6 +616,9 @@ static int spl_load_image(struct spl_image_info *spl_image, } } #endif + if (!ret) + ret = spl_check_board_image(spl_image, &bootdev); + return ret; } -- 2.32.0
[PATCH u-boot-marvell v2 2/9] SPL: Add struct spl_boot_device parameter into spl_parse_board_header()
From: Pali Rohár Add parameter spl_boot_device to spl_parse_board_header(), which allows the implementations to see from which device we are booting and do boot-device-specific checks of the image header. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- arch/arm/mach-mvebu/spl.c | 1 + arch/arm/mach-sunxi/spl_spi_sunxi.c | 2 +- common/spl/spl.c| 4 ++- common/spl/spl_ext.c| 9 -- common/spl/spl_fat.c| 11 +--- common/spl/spl_legacy.c | 3 +- common/spl/spl_mmc.c| 43 ++--- common/spl/spl_nand.c | 5 ++-- common/spl/spl_net.c| 2 +- common/spl/spl_nor.c| 4 +-- common/spl/spl_onenand.c| 2 +- common/spl/spl_ram.c| 2 +- common/spl/spl_sata.c | 9 +++--- common/spl/spl_sdp.c| 2 +- common/spl/spl_spi.c| 9 +++--- common/spl/spl_ubi.c| 4 +-- common/spl/spl_usb.c| 4 +-- common/spl/spl_xip.c| 4 +-- common/spl/spl_ymodem.c | 4 +-- drivers/usb/gadget/f_sdp.c | 12 include/sdp.h | 3 +- include/spl.h | 7 + 22 files changed, 90 insertions(+), 56 deletions(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index a6ed4367dd..716217083b 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -101,6 +101,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device) #endif int spl_parse_board_header(struct spl_image_info *spl_image, + const struct spl_boot_device *bootdev, const void *image_header, size_t size) { const struct kwbimage_main_hdr_v1 *mhdr = image_header; diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c index 3499c4cc5f..910e805016 100644 --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c @@ -348,7 +348,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image, ret = spl_load_simple_fit(spl_image, &load, load_offset, header); } else { - ret = spl_parse_image_header(spl_image, header); + ret = spl_parse_image_header(spl_image, bootdev, header); if (ret) return ret; diff --git a/common/spl/spl.c b/common/spl/spl.c index 4c101ec5d3..bf2139a058 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -312,6 +312,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, #endif __weak int spl_parse_board_header(struct spl_image_info *spl_image, + const struct spl_boot_device *bootdev, const void *image_header, size_t size) { return -EINVAL; @@ -326,6 +327,7 @@ __weak int spl_parse_legacy_header(struct spl_image_info *spl_image, } int spl_parse_image_header(struct spl_image_info *spl_image, + const struct spl_boot_device *bootdev, const struct image_header *header) { #if CONFIG_IS_ENABLED(LOAD_FIT_FULL) @@ -369,7 +371,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, } #endif - if (!spl_parse_board_header(spl_image, (const void *)header, sizeof(*header))) + if (!spl_parse_board_header(spl_image, bootdev, (const void *)header, sizeof(*header))) return 0; #ifdef CONFIG_SPL_RAW_IMAGE_SUPPORT diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c index 6a28fe9bdb..ebd914c492 100644 --- a/common/spl/spl_ext.c +++ b/common/spl/spl_ext.c @@ -10,6 +10,7 @@ #include int spl_load_image_ext(struct spl_image_info *spl_image, + struct spl_boot_device *bootdev, struct blk_desc *block_dev, int partition, const char *filename) { @@ -46,7 +47,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image, goto end; } - err = spl_parse_image_header(spl_image, header); + err = spl_parse_image_header(spl_image, bootdev, header); if (err < 0) { puts("spl: ext: failed to parse image header\n"); goto end; @@ -66,6 +67,7 @@ end: #if CONFIG_IS_ENABLED(OS_BOOT) int spl_load_image_ext_os(struct spl_image_info *spl_image, + struct spl_boot_device *bootdev, struct blk_desc *block_dev, int partition) { int err; @@ -103,7 +105,7 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image, } file = env_get("falcon_image_file"); if (file) { - err = spl_load_image_ext(spl_image, block_dev, +
[PATCH u-boot-marvell v2 5/9] arm: mvebu: Check for kwbimage data checksum
From: Pali Rohár Last 4 bytes of kwbimage boot image is checksum. Verify it via the new spl_check_board_image() function which is called by U-Boot SPL after loading kwbimage. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- Changes since v1: - changed uint32_t to u32 --- arch/arm/mach-mvebu/spl.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 4af52c626c..af9e45ac7a 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -100,6 +100,33 @@ u32 spl_mmc_boot_mode(const u32 boot_device) } #endif +static u32 checksum32(void *start, u32 len) +{ + u32 csum = 0; + u32 *p = start; + + while (len > 0) { + csum += *p++; + len -= sizeof(u32); + }; + + return csum; +} + +int spl_check_board_image(struct spl_image_info *spl_image, + const struct spl_boot_device *bootdev) +{ + u32 csum = *(u32 *)(spl_image->load_addr + spl_image->size - 4); + + if (checksum32((void *)spl_image->load_addr, + spl_image->size - 4) != csum) { + printf("ERROR: Invalid data checksum in kwbimage\n"); + return -EINVAL; + } + + return 0; +} + int spl_parse_board_header(struct spl_image_info *spl_image, const struct spl_boot_device *bootdev, const void *image_header, size_t size) -- 2.32.0
[PATCH u-boot-marvell v2 7/9] arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t
From: Marek Behún Checkpatch warns about using uint32/16/8_t instead of u32/16/8. Use the preferred types. Signed-off-by: Marek Behún --- arch/arm/mach-mvebu/spl.c | 34 +- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 8c8cbc833f..97d7aea179 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -74,23 +74,23 @@ /* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */ struct kwbimage_main_hdr_v1 { - uint8_t blockid; /* 0x0 */ - uint8_t flags; /* 0x1 */ - uint16_t nandpagesize; /* 0x2-0x3 */ - uint32_t blocksize; /* 0x4-0x7 */ - uint8_t version; /* 0x8 */ - uint8_t headersz_msb; /* 0x9 */ - uint16_t headersz_lsb; /* 0xA-0xB */ - uint32_t srcaddr; /* 0xC-0xF */ - uint32_t destaddr; /* 0x10-0x13 */ - uint32_t execaddr; /* 0x14-0x17 */ - uint8_t options; /* 0x18 */ - uint8_t nandblocksize; /* 0x19 */ - uint8_t nandbadblklocation;/* 0x1A */ - uint8_t reserved4; /* 0x1B */ - uint16_t reserved5; /* 0x1C-0x1D */ - uint8_t ext; /* 0x1E */ - uint8_t checksum; /* 0x1F */ + u8 blockid; /* 0x0 */ + u8 flags; /* 0x1 */ + u16 nandpagesize; /* 0x2-0x3 */ + u32 blocksize; /* 0x4-0x7 */ + u8 version; /* 0x8 */ + u8 headersz_msb; /* 0x9 */ + u16 headersz_lsb; /* 0xA-0xB */ + u32 srcaddr; /* 0xC-0xF */ + u32 destaddr; /* 0x10-0x13 */ + u32 execaddr; /* 0x14-0x17 */ + u8 options; /* 0x18 */ + u8 nandblocksize; /* 0x19 */ + u8 nandbadblklocation;/* 0x1A */ + u8 reserved4; /* 0x1B */ + u16 reserved5; /* 0x1C-0x1D */ + u8 ext; /* 0x1E */ + u8 checksum; /* 0x1F */ } __packed; #ifdef CONFIG_SPL_MMC -- 2.32.0
[PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message
From: Marek Behún Print the wrong srcaddr (spl_image->offset) in error message also for SATA case. Signed-off-by: Marek Behún --- arch/arm/mach-mvebu/spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index af9e45ac7a..8c8cbc833f 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image, */ if (mhdr->blockid == IBR_HDR_SATA_ID) { if (spl_image->offset < 1) { - printf("ERROR: Wrong SATA srcaddr in kwbimage\n"); + printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n", + spl_image->offset); return -EINVAL; } spl_image->offset -= 1; -- 2.32.0
[PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
From: Marek Behún Use the preferred if (IS_ENABLED(X)) instead of #ifdef X where possible. There are still places where this is not possible or is more complicated to convert in this file. Leave those be for now. Signed-off-by: Marek Behún --- arch/arm/mach-mvebu/spl.c | 43 --- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 97d7aea179..7dbe8eeba3 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info *spl_image, return -EINVAL; } -#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT - if (bootdev->boot_device == BOOT_DEVICE_SPI && + if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) && + bootdev->boot_device == BOOT_DEVICE_SPI && mhdr->blockid != IBR_HDR_SPI_ID) { printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n", mhdr->blockid); return -EINVAL; } -#endif -#ifdef CONFIG_SPL_SATA - if (bootdev->boot_device == BOOT_DEVICE_SATA && + if (IS_ENABLED(CONFIG_SPL_SATA) && + bootdev->boot_device == BOOT_DEVICE_SATA && mhdr->blockid != IBR_HDR_SATA_ID) { printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n", mhdr->blockid); return -EINVAL; } -#endif -#ifdef CONFIG_SPL_MMC - if ((bootdev->boot_device == BOOT_DEVICE_MMC1 || + if (IS_ENABLED(CONFIG_SPL_MMC) && + (bootdev->boot_device == BOOT_DEVICE_MMC1 || bootdev->boot_device == BOOT_DEVICE_MMC2 || bootdev->boot_device == BOOT_DEVICE_MMC2_2) && mhdr->blockid != IBR_HDR_SDIO_ID) { @@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info *spl_image, mhdr->blockid); return -EINVAL; } -#endif spl_image->offset = mhdr->srcaddr; -#ifdef CONFIG_SPL_SATA /* * For SATA srcaddr is specified in number of sectors. * The main header is must be stored at sector number 1. * This expects that sector size is 512 bytes and recalculates * data offset to bytes relative to the main header. */ - if (mhdr->blockid == IBR_HDR_SATA_ID) { + if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) { if (spl_image->offset < 1) { printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n", spl_image->offset); @@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info *spl_image, spl_image->offset -= 1; spl_image->offset *= 512; } -#endif -#ifdef CONFIG_SPL_MMC /* * For SDIO (eMMC) srcaddr is specified in number of sectors. * This expects that sector size is 512 bytes and recalculates * data offset to bytes. */ - if (mhdr->blockid == IBR_HDR_SDIO_ID) + if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID) spl_image->offset *= 512; -#endif if (spl_image->offset % 4 != 0) { printf("ERROR: Wrong srcaddr (%u) in kwbimage\n", @@ -340,17 +333,17 @@ void board_init_f(ulong dummy) timer_init(); /* Armada 375 does not support SerDes and DDR3 init yet */ -#if !defined(CONFIG_ARMADA_375) - /* First init the serdes PHY's */ - serdes_phy_config(); - - /* Setup DDR */ - ret = ddr3_init(); - if (ret) { - debug("ddr3_init() failed: %d\n", ret); - hang(); + if (!IS_ENABLED(CONFIG_ARMADA_375)) { + /* First init the serdes PHY's */ + serdes_phy_config(); + + /* Setup DDR */ + ret = ddr3_init(); + if (ret) { + debug("ddr3_init() failed: %d\n", ret); + hang(); + } } -#endif /* Initialize Auto Voltage Scaling */ mv_avs_init(); -- 2.32.0
[PATCH u-boot-marvell v2 9/9] arm: mvebu: spl: Fix 100 column exceeds
From: Marek Behún Fix 100 column exceeds in arch/arm/mach-mvebu/spl.c. Signed-off-by: Marek Behún --- arch/arm/mach-mvebu/spl.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 7dbe8eeba3..7450e1e3da 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -47,7 +47,8 @@ #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0 #endif -#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0 +#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && \ +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0 #endif #endif @@ -58,7 +59,8 @@ * set to 1. Otherwise U-Boot SPL would not be able to load U-Boot proper. */ #ifdef CONFIG_SPL_SATA -#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || !defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1 +#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || \ +!defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1 #error CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR must be set to 1 #endif #endif -- 2.32.0
Re: [PATCH v3] efi_loader: check tcg2 protocol installation outside the TCG protocol
Hi Kojima-san, On Fri, Nov 26, 2021 at 10:31:16AM +0900, Masahisa Kojima wrote: > There are functions that calls tcg2_agile_log_append() outside > of the TCG protocol invocation (e.g tcg2_measure_pe_image). > These functions must to check that tcg2 protocol is installed. > If not, measurement shall be skipped. > > Together with above change, this commit also removes the > unnecessary tcg2_uninit() call in efi_tcg2_register(), > refactors efi_tcg2_register() not to include the process > that is not related to the tcg2 protocol registration. > > Signed-off-by: Masahisa Kojima > --- > Changes in v3: > - add static qualifier to is_tcg2_protocol_installed() > - simplify is_tcg2_protocol_installed() return handling > > Changes in v2: > - rebased on top of the TF-A eventlog handover support > > include/efi_loader.h | 4 ++ > lib/efi_loader/efi_setup.c | 3 ++ > lib/efi_loader/efi_tcg2.c | 89 +++--- > 3 files changed, 81 insertions(+), 15 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index d52e399841..fe29c10906 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void); > efi_status_t efi_rng_register(void); > /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ > efi_status_t efi_tcg2_register(void); > +/* Called by efi_init_obj_list() to do the required setup for the > measurement */ > +efi_status_t efi_tcg2_setup_measurement(void); > +/* Called by efi_init_obj_list() to do initial measurement */ > +efi_status_t efi_tcg2_do_initial_measurement(void); > /* measure the pe-coff image, extend PCR and add Event Log */ > efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, > struct efi_loaded_image_obj *handle, > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index a2338d74af..781d10590d 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void) > ret = efi_tcg2_register(); > if (ret != EFI_SUCCESS) > goto out; > + > + efi_tcg2_setup_measurement(); > + efi_tcg2_do_initial_measurement(); > } > > /* Secure boot */ > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index b44eed0ec9..2196af49a6 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg) > return 0; > } > > +/** > + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed s/chech/check > + * > + * @Return: true if tcg2 protocol is installed, false if not > + */ > +static bool is_tcg2_protocol_installed(void) > +{ > + struct efi_handler *handler; > + efi_status_t ret; > + > + ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler); > + return (ret == EFI_SUCCESS); > +} > + > static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) > { > u32 len; > @@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 > efi_size, > IMAGE_NT_HEADERS32 *nt; > struct efi_handler *handler; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > return ret; > @@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct > efi_loaded_image_obj *ha > u32 event = 0; > struct smbios_entry *entry; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > if (tcg2_efi_app_invoked) > return EFI_SUCCESS; > > @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) > efi_status_t ret; > struct udevice *dev; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > return ret; > @@ -2214,6 +2237,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event > *event, void *context) > > EFI_ENTRY("%p, %p", event, context); > > + if (!is_tcg2_protocol_installed()) { > + ret = EFI_NOT_READY; > + goto out; > + } > + > event_log.ebs_called = true; > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > @@ -2244,6 +2272,9 @@ efi_status_t > efi_tcg2_notify_exit_boot_services_failed(void) > struct udevice *dev; > efi_status_t ret; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > goto out; > @@ -2313,6 +2344,45 @@ error: > return ret; > } > > +/** > + * efi_tcg2_setup_measurement() - setup for the measurement > + * > + * Return: stat
Re: [PATCH v1 1/1] tools/netconsole: Add support for socat
On Fri, Nov 19, 2021 at 1:29 AM Ferry Toth wrote: > > Hi, > > Op 17-11-2021 om 18:15 schreef Andy Shevchenko: > > socat is a very powerful tool to work with socets (and not only) > > in UNIX systems. Let's add support for it in netconsole. > > > > Signed-off-by: Andy Shevchenko > Tested-by: Ferry Toth Thanks! Can thi be applied, please? > > --- > > tools/netconsole | 12 ++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/tools/netconsole b/tools/netconsole > > index 1a0ef22244e3..155453320f73 100755 > > --- a/tools/netconsole > > +++ b/tools/netconsole > > @@ -34,7 +34,7 @@ if [ -z "${ip}" ] || [ -n "$4" ] ; then > > usage "Invalid number of arguments" > > fi > > > > -for nc in netcat nc ; do > > +for nc in socat netcat nc ; do > > type ${nc} >/dev/null 2>&1 && break > > done > > > > @@ -47,6 +47,10 @@ if type ncb 2>/dev/null ; then > > # see if ncb is in $PATH > > exec ncb ${board_out_port} > > > > +elif [ "${nc}" = "socat" ] ; then > > + # socat does support broadcast > > + while ${nc} STDIO "UDP4-LISTEN:${board_out_port}"; do :; done > > + > > elif [ -x ${0%/*}/ncb ] ; then > > # maybe it's in the same dir as the netconsole script > > exec ${0%/*}/ncb ${board_out_port} > > @@ -59,5 +63,9 @@ else > > fi > > ) & > > pid=$! > > -${nc} -u ${ip} ${board_in_port} > > +if [ "${nc}" = "socat" ] ; then > > + ${nc} - "UDP4:${ip}:${board_in_port}" > > +else > > + ${nc} -u ${ip} ${board_in_port} > > +fi > > kill ${pid} 2>/dev/null -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 04/11] Convert CONFIG_PHYSMEM to Kconfig
On Thu, Nov 25, 2021 at 2:12 AM Simon Glass wrote: > On Wed, 24 Nov 2021 at 09:33, Andy Shevchenko > wrote: > > > > On Wed, Nov 24, 2021 at 6:28 PM Simon Glass wrote: > > > > > > This converts the following to Kconfig: > > >CONFIG_PHYSMEM > > > > Why? > > Because we want to avoid ad-hoc CONFIG options. I mean why to do this effort if it can be simply removed for good. > > And why my message [1] left unanswered? > > > > [1]: > > https://lore.kernel.org/u-boot/cahp75vcvpvupwkjcufwjcs0roq7woiz_cud57vpzd5hf6se...@mail.gmail.com/T/#u > > Because I have not got to it yet. It can take me a week or more to > catch up on email. I do it in batches. > > We could enable this option always, but it is a bit risky as it > doesn't work on 32-bit machines unless there is an implementation > available. I haven't got this. It's always enabled on x86. And it seems the only user of it. I don't see any benefit of keeping this option. -- With Best Regards, Andy Shevchenko
[PATCH] armv8: sec_firmware: drop unneeded ARMV8_SEC_FIRMWARE_SUPPORT check
This module will only be compiled if this symbol is set. Remove the useless check. While at it, invert the return code, to return 0 if the function succeeds. The only user of this function in arch/arm/cpu/armv8/fsl-layerscape/fdt.c doesn't check the return code anyway, so its safe change it. Signed-off-by: Michael Walle --- arch/arm/cpu/armv8/sec_firmware.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index 267894fbcb..e62e9dc380 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -458,47 +458,45 @@ int sec_firmware_init(const void *sec_firmware_img, /* * fdt_fix_kaslr - Add kalsr-seed node in Device tree * @fdt: Device tree - * @eret: 0 in case of error, 1 for success + * + * Returns 0 on success and 1 in case of an error. */ int fdt_fixup_kaslr(void *fdt) { int nodeoffset; - int err, ret = 0; + int err; u8 rand[8]; -#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) /* Check if random seed generation is supported */ if (sec_firmware_support_hwrng() == false) { printf("WARNING: SEC firmware not running, no kaslr-seed\n"); - return 0; + return 1; } ret = sec_firmware_get_random(rand, 8); if (ret < 0) { printf("WARNING: No random number to set kaslr-seed\n"); - return 0; + return 1; } err = fdt_check_header(fdt); if (err < 0) { printf("fdt_chosen: %s\n", fdt_strerror(err)); - return 0; + return 1; } /* find or create "/chosen" node. */ nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen"); if (nodeoffset < 0) - return 0; + return 1; err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand, sizeof(rand)); if (err < 0) { printf("WARNING: can't set kaslr-seed %s.\n", fdt_strerror(err)); - return 0; + return 1; } - ret = 1; -#endif - return ret; + return 0; } -- 2.30.2
[PATCH 0/5] board: sl28: add basic PSCI implementation
Add PSCI support to reset and power-off the board. Because this board can be used without TF-A, supply a (mandatory) PSCI implementation. The armv8 u-boot part already contains most bits for the implementation, it is just, that it isn't compatible with the layerscape parts. Thus, we first need to clean that up and then we can add our three little functions for the PSCI support. Michael Walle (5): armv8: include psci_update_dt() unconditionally armv8: layerscape: get rid of smc_call() armv8: psci: skip setup code if we are not EL3 armv8: psci: add ARMV8_PSCI_RELOCATE Kconfig option board: sl28: add basic PSCI implementation arch/arm/cpu/armv8/Kconfig| 30 +- arch/arm/cpu/armv8/cpu-dt.c | 7 ++-- arch/arm/cpu/armv8/cpu.c | 3 ++ arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 3 ++ arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 49 +-- arch/arm/cpu/armv8/fsl-layerscape/mp.c| 11 ++--- arch/arm/cpu/armv8/sec_firmware.c | 19 - board/freescale/ls1043ardb/Kconfig| 8 board/kontron/sl28/Makefile | 2 + board/kontron/sl28/psci.c | 42 +++ 10 files changed, 103 insertions(+), 71 deletions(-) create mode 100644 board/kontron/sl28/psci.c -- 2.30.2
[PATCH 1/5] armv8: include psci_update_dt() unconditionally
psci_update_dt() is also required if CONFIG_ARMV8_PSCI is set, that is, if u-boot is the PSCI provider. Guard the check which is intended to call into the PSCI implementation in the secure firmware, by the proper macro SEC_FIRMWARE_ARMV8_PSCI. Signed-off-by: Michael Walle --- arch/arm/cpu/armv8/cpu-dt.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c index 61c38b17cb..1bf8fbaae3 100644 --- a/arch/arm/cpu/armv8/cpu-dt.c +++ b/arch/arm/cpu/armv8/cpu-dt.c @@ -8,8 +8,8 @@ #include #include #include +#include -#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) int psci_update_dt(void *fdt) { /* @@ -18,8 +18,10 @@ int psci_update_dt(void *fdt) * number to support detecting PSCI dynamically and then switching * the SMP boot method between PSCI and spin-table. */ - if (sec_firmware_support_psci_version() == PSCI_INVALID_VER) + if (CONFIG_IS_ENABLED(SEC_FIRMWARE_ARMV8_PSCI) && + sec_firmware_support_psci_version() == PSCI_INVALID_VER) return 0; + fdt_psci(fdt); #if defined(CONFIG_ARMV8_PSCI) && !defined(CONFIG_ARMV8_SECURE_BASE) @@ -30,4 +32,3 @@ int psci_update_dt(void *fdt) return 0; } -#endif -- 2.30.2
[PATCH 2/5] armv8: layerscape: get rid of smc_call()
There are two different implementations to do a secure monitor call: smc_call() and arm_smccc_smc(). The former is defined in fwcall.c and seems to be an ad-hoc implementation. The latter is imported from linux. smc_call() is also only available if CONFIG_ARMV8_PSCI is not defined. This makes it impossible to have both PSCI calls and PSCI implementation in one u-boot build. The layerscape SoC code decide at runtime via check_psci() if there is a PSCI support. Therefore, this is a prerequisite patch to add PSCI implementation support for the layerscape SoCs. Note, for the TFA part, this is only compile time tested with (ls1028ardb_tfa_defconfig). Signed-off-by: Michael Walle --- arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 1 + arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 49 +-- arch/arm/cpu/armv8/fsl-layerscape/mp.c| 11 ++--- arch/arm/cpu/armv8/sec_firmware.c | 19 - 4 files changed, 31 insertions(+), 49 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig index 1a057f7059..14ef07e74f 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -321,6 +321,7 @@ menu "Layerscape architecture" config FSL_LAYERSCAPE bool + select ARM_SMCCC config HAS_FEATURE_GIC64K_ALIGN bool diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index 1a359d060e..38bef4432d 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -766,7 +767,7 @@ enum boot_src __get_boot_src(u32 porsr1) enum boot_src get_boot_src(void) { - struct pt_regs regs; + struct arm_smccc_res res; u32 porsr1 = 0; #if defined(CONFIG_FSL_LSCH3) @@ -776,11 +777,9 @@ enum boot_src get_boot_src(void) #endif if (current_el() == 2) { - regs.regs[0] = SIP_SVC_RCW; - - smc_call(®s); - if (!regs.regs[0]) - porsr1 = regs.regs[1]; + arm_smccc_smc(SIP_SVC_RCW, 0, 0, 0, 0, 0, 0, 0, &res); + if (!res.a0) + porsr1 = res.a1; } if (current_el() == 3 || !porsr1) { @@ -1079,9 +1078,9 @@ static void config_core_prefetch(void) char *buf = NULL; char buffer[HWCONFIG_BUFFER_SIZE]; const char *prefetch_arg = NULL; + struct arm_smccc_res res; size_t arglen; unsigned int mask; - struct pt_regs regs; if (env_get_f("hwconfig", buffer, sizeof(buffer)) > 0) buf = buffer; @@ -1099,11 +1098,10 @@ static void config_core_prefetch(void) } #define SIP_PREFETCH_DISABLE_64 0xC200FF13 - regs.regs[0] = SIP_PREFETCH_DISABLE_64; - regs.regs[1] = mask; - smc_call(®s); + arm_smccc_smc(SIP_PREFETCH_DISABLE_64, mask, 0, 0, 0, 0, 0, 0, + &res); - if (regs.regs[0]) + if (res.a0) printf("Prefetch disable config failed for mask "); else printf("Prefetch disable config passed for mask "); @@ -1343,25 +1341,20 @@ phys_size_t get_effective_memsize(void) #ifdef CONFIG_TFABOOT phys_size_t tfa_get_dram_size(void) { - struct pt_regs regs; - phys_size_t dram_size = 0; - - regs.regs[0] = SMC_DRAM_BANK_INFO; - regs.regs[1] = -1; + struct arm_smccc_res res; - smc_call(®s); - if (regs.regs[0]) + arm_smccc_smc(SMC_DRAM_BANK_INFO, -1, 0, 0, 0, 0, 0, 0, &res); + if (res.a0) return 0; - dram_size = regs.regs[1]; - return dram_size; + return res.a1; } static int tfa_dram_init_banksize(void) { int i = 0, ret = 0; - struct pt_regs regs; phys_size_t dram_size = tfa_get_dram_size(); + struct arm_smccc_res res; debug("dram_size %llx\n", dram_size); @@ -1369,19 +1362,15 @@ static int tfa_dram_init_banksize(void) return -EINVAL; do { - regs.regs[0] = SMC_DRAM_BANK_INFO; - regs.regs[1] = i; - - smc_call(®s); - if (regs.regs[0]) { + arm_smccc_smc(SMC_DRAM_BANK_INFO, i, 0, 0, 0, 0, 0, 0, &res); + if (res.a0) { ret = -EINVAL; break; } - debug("bank[%d]: start %lx, size %lx\n", i, regs.regs[1], - regs.regs[2]); - gd->bd->bi_dram[i].start = regs.regs[1]; - gd->bd->bi_dram[i].size = regs.regs[2]; + debug("bank[%d]: start %lx, size %lx\n", i, res.a1, res.a2); + gd->bd->bi_dram[i].start = res.a1; + gd->bd->bi_dram[i].size = res.a2
[PATCH 3/5] armv8: psci: skip setup code if we are not EL3
If we are running in EL2 skip PSCI implementation setup. This avoids an exception if CONFIG_ARMV8_PSCI is set, but u-boot is started by TF-A. Signed-off-by: Michael Walle --- arch/arm/cpu/armv8/cpu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/cpu/armv8/cpu.c b/arch/arm/cpu/armv8/cpu.c index ea40c55dd2..db5d460eb4 100644 --- a/arch/arm/cpu/armv8/cpu.c +++ b/arch/arm/cpu/armv8/cpu.c @@ -79,6 +79,9 @@ static void relocate_secure_section(void) void armv8_setup_psci(void) { + if (current_el() != 3) + return; + relocate_secure_section(); secure_ram_addr(psci_setup_vectors)(); secure_ram_addr(psci_arch_init)(); -- 2.30.2
[PATCH 4/5] armv8: psci: add ARMV8_PSCI_RELOCATE Kconfig option
There is an user-selectable SYS_HAS_ARMV8_SECURE_BASE, which has the same meaning but is just for the ls1043ardb board. As no in-tree config uses this, drop it and replace it with something more sophiticated: ARMV8_PSCI_RELOCATE. This option will then enable the ARMV8_SECURE_BASE option which is used as the base to relocate the PSCI code (or any code in the secure region, but that is only PSCI). A SoC (or board) can now opt-in into having such a secure region by enabling SYS_HAS_ARMV8_SECURE_BASE. Enable it for the LS1043A SoC, where it was possible to relocate the PSCI code before as well as on the LS1028A SoC where there will be PSCI support soon. Additionally, make ARMV8_PSCI and SEC_FIRMWARE_ARMV8_PSCI exclusive. Signed-off-by: Michael Walle --- arch/arm/cpu/armv8/Kconfig| 30 ++- arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 2 ++ board/freescale/ls1043ardb/Kconfig| 8 -- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 0a3fdfa471..9289526230 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -84,6 +84,7 @@ config SPL_RECOVER_DATA_SECTION config SEC_FIRMWARE_ARMV8_PSCI bool "PSCI implementation in secure monitor firmware" depends on ARMV8_SEC_FIRMWARE_SUPPORT || SPL_ARMV8_SEC_FIRMWARE_SUPPORT + depends on ARMV8_PSCI=n help This config enables the ARMv8 PSCI implementation in secure monitor firmware. This is a private PSCI implementation and different from @@ -125,6 +126,9 @@ config PSCI_RESET Select Y here to make use of PSCI calls for system reset +config SYS_HAS_ARMV8_SECURE_BASE + bool + config ARMV8_PSCI bool "Enable PSCI support" if EXPERT help @@ -152,23 +156,27 @@ config ARMV8_PSCI_CPUS_PER_CLUSTER A value 0 or no definition of it works for single cluster system. System with multi-cluster should difine their own exact value. -config ARMV8_EA_EL3_FIRST - bool "External aborts and SError interrupt exception are taken in EL3" +config ARMV8_PSCI_RELOCATE + bool "Relocate PSCI code" + depends on ARMV8_PSCI + depends on SYS_HAS_ARMV8_SECURE_BASE help - Exception handling at all exception levels for External Abort and - SError interrupt exception are taken in EL3. - -if SYS_HAS_ARMV8_SECURE_BASE + Relocate PSCI code, for example to a secure memory on the SoC. If not + set, the PSCI sections are placed together with the u-boot and the + regions will be marked as reserved before linux is started. config ARMV8_SECURE_BASE hex "Secure address for PSCI image" - depends on ARMV8_PSCI + depends on ARMV8_PSCI_RELOCATE + default 0x1800 if ARCH_LS1028A help Address for placing the PSCI text, data and stack sections. - If not defined, the PSCI sections are placed together with the u-boot - but platform can choose to place PSCI code image separately in other - places such as some secure RAM built-in SOC etc. -endif + +config ARMV8_EA_EL3_FIRST + bool "External aborts and SError interrupt exception are taken in EL3" + help + Exception handling at all exception levels for External Abort and + SError interrupt exception are taken in EL3. endif diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig index 14ef07e74f..8b1728862d 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -53,6 +53,7 @@ config ARCH_LS1028A select SYS_FSL_ERRATUM_A011334 select SYS_FSL_ESDHC_UNRELIABLE_PULSE_DETECTION_WORKAROUND select RESV_RAM if GIC_V3_ITS + select SYS_HAS_ARMV8_SECURE_BASE imply PANIC_HANG config ARCH_LS1043A @@ -88,6 +89,7 @@ config ARCH_LS1043A select SYS_I2C_MXC_I2C2 if !DM_I2C select SYS_I2C_MXC_I2C3 if !DM_I2C select SYS_I2C_MXC_I2C4 if !DM_I2C + select SYS_HAS_ARMV8_SECURE_BASE imply CMD_PCI imply ID_EEPROM diff --git a/board/freescale/ls1043ardb/Kconfig b/board/freescale/ls1043ardb/Kconfig index 778b8d8d5a..d66c7804b1 100644 --- a/board/freescale/ls1043ardb/Kconfig +++ b/board/freescale/ls1043ardb/Kconfig @@ -13,14 +13,6 @@ config SYS_SOC config SYS_CONFIG_NAME default "ls1043ardb" -config SYS_HAS_ARMV8_SECURE_BASE - bool "Enable secure address for PSCI image" - depends on ARMV8_PSCI - help - PSCI image can be re-located to secure RAM. - If enabled, please also define the value for ARMV8_SECURE_BASE, - for LS1043ARDB, it could be some address in OCRAM. - if FSL_LS_PPA config SYS_LS_PPA_FW_ADDR hex "PPA Firmware Addr" -- 2.30.2
[PATCH 5/5] board: sl28: add basic PSCI implementation
For now, this only provides reset and poweroff functions. Signed-off-by: Michael Walle --- board/kontron/sl28/Makefile| 2 ++ board/kontron/sl28/psci.c | 42 ++ configs/kontron_sl28_defconfig | 2 ++ 3 files changed, 46 insertions(+) create mode 100644 board/kontron/sl28/psci.c diff --git a/board/kontron/sl28/Makefile b/board/kontron/sl28/Makefile index 5d220f0744..084c11da37 100644 --- a/board/kontron/sl28/Makefile +++ b/board/kontron/sl28/Makefile @@ -6,6 +6,8 @@ endif obj-y += common.o ddr.o +obj-$(CONFIG_ARMV8_PSCI) += psci.o + ifdef CONFIG_SPL_BUILD obj-y += spl.o obj-$(CONFIG_SPL_ATF) += spl_atf.o diff --git a/board/kontron/sl28/psci.c b/board/kontron/sl28/psci.c new file mode 100644 index 00..19f0ef3b6d --- /dev/null +++ b/board/kontron/sl28/psci.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include + +#define GPIO2_GPDIR0x231 +#define GPIO2_GPDAT0x2310008 +#define RSTCR 0x1e6 +#define RESET_REQ BIT(1) + +u32 __secure psci_version(void) +{ + return ARM_PSCI_VER_0_2; +} + +void __secure psci_system_reset(void) +{ + writel(RESET_REQ, RSTCR); + + while (1) + wfi(); +} + +void __secure psci_system_off(void) +{ + int i; + + writel(0x0200, GPIO2_GPDIR); + writel(0, GPIO2_GPDAT); + + /* make sure the management controller has sampled the input */ + for (i = 0; i < (1 << 11); i++) + asm("nop"); + + writel(RESET_REQ, RSTCR); + + while (1) + wfi(); +} diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig index 7fb6bdbe82..c9d6767055 100644 --- a/configs/kontron_sl28_defconfig +++ b/configs/kontron_sl28_defconfig @@ -19,6 +19,8 @@ CONFIG_ENV_OFFSET_REDUND=0x3f CONFIG_SPL_SPI_FLASH_SUPPORT=y CONFIG_SPL_SPI=y # CONFIG_PSCI_RESET is not set +CONFIG_ARMV8_PSCI=y +CONFIG_ARMV8_PSCI_RELOCATE=y CONFIG_AHCI=y CONFIG_DISTRO_DEFAULTS=y CONFIG_SYS_LOAD_ADDR=0x8200 -- 2.30.2
Re: [PATCH 00/16] tools: Add support for signing devicetree blobs
On 12/11/2021 20.28, Simon Glass wrote: > At present mkimage supports signing FITs, the standard U-Boot image type. > > Various people are opposed to using FIT since: > > a) it requires adding support for FIT into other bootloaders, notably >UEFI > b) it requires packaging a kernel in this standard U-Boot format, meaning >that distros must run 'mkimage' and deal with the kernel and initrd >being inside a FIT > > The kernel and initrd can be dealt with in other ways. But without FIT, > we have no standard way of signing and grouping FDT files. Instead we must > include them in the distro as separate files. > > In particular, some sort of mechanism for verifying FDT files is needed. > One option would be to tack a signature on before or after the file, > processing it accordingly. But due to the nature of the FDT binary format, > it is possible to embed a signature inside the FDT itself, which is very > convenient. > > This series provides a tool, fdt_sign, which can add a signature to an > FDT. The signature can be checked later, preventing any change to the FDT, > other than in permitted nodes (e.g. /chosen). > > This series also provides a fdt_check_sign tool, used to check signatures. This could be useful. Not because I'm interested in signing device-tree blobs as such, but as a replacement for the current incomprehensible way FIT images are signed and verified. This seems to be a much better and simpler scheme, that avoids (can avoid) the overly simplistic approach of signing just image nodes, and the way too complex signing-configurations-and-then-add-a-property-saying-which-nodes-I-signed-and-sign-that-as-well. In order not to hard-code anything in the tool about /chosen or whatnot, make the rule be that the blob (I'll use that word, because it can as well be a FIT image containing kernel image+initramfs etc., or a FIT containing a U-Boot script) has a property in the top node unsigned-nodes = "/chosen", "/signatures"; (maybe it could be unsigned-nodes = <&chosen &sigs> but that may make the implementation a little more complex). That property itself is by definition part of what gets signed. The verifier can and should choose to reject the whole thing if "/" is mentioned. Then, please, for debugging, inside the /signatures node, beside the signature data, also add a copy of the offset/len pairs that were actually hashed, and that hash value. /signatures { hashed-regions = ; hash-1 { value = 0xabcd; algo = "sha256"; }; signature-1 { ... }; ... And make the tool start by adding dummy properties, so those strings "hashed-regions", "value", "algo", "signer" and whatever other property names are used already exist in the string table. After that, fill in the actual values - and have the rule that the entire string table is hashed, not just some initial part of it. The verifier will of course not rely on /signatures/hashed-regions, but will compute the regions itself based on /unsigned-nodes (and implicitly add a region consisting of the string table and the memreserve table). Then we'd have a generic scheme for signing blobs, disentangled from whether they are actually describing hardware or used as a generic container format. And instead of "image" or "conf", we could mark a public key as being required for "whole-blob" (bikeshedding welcome), using that same scheme for verifying a container with a boot script and a container with kernel+initramfs+dtbs. > For now there is absolutely no configurability in the signature mechanism. > It would of course be possible to adjust which nodes are signed, as is Yes, let the blob itself define that. And don't add any ad hoc "oh, skip a property if it is called data-offset or data or...". Rasmus
[PATCH 1/1] configs: opos6uldev: update to have screen working at power on
Signed-off-by: Sébastien Szymanski --- configs/opos6uldev_defconfig | 3 --- 1 file changed, 3 deletions(-) diff --git a/configs/opos6uldev_defconfig b/configs/opos6uldev_defconfig index c52d2a5d39..409c48451e 100644 --- a/configs/opos6uldev_defconfig +++ b/configs/opos6uldev_defconfig @@ -103,13 +103,10 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5 CONFIG_CI_UDC=y CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_DM_VIDEO=y -# CONFIG_VIDEO_BPP8 is not set -# CONFIG_VIDEO_BPP32 is not set CONFIG_SYS_WHITE_ON_BLACK=y CONFIG_VIDEO_MXS=y CONFIG_SPLASH_SCREEN=y CONFIG_SPLASH_SCREEN_ALIGN=y -CONFIG_SPLASH_SOURCE=y CONFIG_VIDEO_BMP_RLE8=y CONFIG_BMP_16BPP=y CONFIG_BMP_24BPP=y -- 2.32.0
[PATCH 1/1] video: mxsfb: fix pixel clock polarity
DISPLAY_FLAGS_PIXDATA_NEGEDGE means the controller drives the data on pixel clocks falling edge. That is DOTCLK_POL=0 (default) not 1. The same change has been made on the Linux's driver: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mxsfb?h=v5.16-rc2&id=53990e416bb7adaa59d045f325a47f31a11b75ee Signed-off-by: Sébastien Szymanski --- drivers/video/mxsfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 98d2965711..7aeea4b23f 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -161,7 +161,7 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, vdctrl0 |= LCDIF_VDCTRL0_HSYNC_POL; if(flags & DISPLAY_FLAGS_VSYNC_HIGH) vdctrl0 |= LCDIF_VDCTRL0_VSYNC_POL; - if(flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + if(flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) vdctrl0 |= LCDIF_VDCTRL0_DOTCLK_POL; if(flags & DISPLAY_FLAGS_DE_HIGH) vdctrl0 |= LCDIF_VDCTRL0_ENABLE_POL; -- 2.32.0
[PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
Changes to the am33xx device (33e9021a) trees have been merged in from the upstream linux kernel which now means the device tree uses the new pins format (as of 5.10) where the confinguration can be stores as a separate configuration value and pin mux mode which are then OR'd together. This patch adds support for the new format to u-boot so that pinctrl-cells is now respected when reading in pinctrl-single,pins --- drivers/pinctrl/pinctrl-single.c | 55 +--- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 8fc07e3498..bc9c17bce8 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -28,6 +28,7 @@ struct single_pdata { int offset; u32 mask; u32 width; + u32 args_count; bool bits_per_mux; }; @@ -78,20 +79,6 @@ struct single_priv { struct list_head gpiofuncs; }; -/** - * struct single_fdt_pin_cfg - pin configuration - * - * This structure is used for the pin configuration parameters in case - * the register controls only one pin. - * - * @reg: configuration register offset - * @val: configuration register value - */ -struct single_fdt_pin_cfg { - fdt32_t reg; - fdt32_t val; -}; - /** * struct single_fdt_bits_cfg - pin configuration * @@ -314,25 +301,28 @@ static int single_pin_compare(const void *s1, const void *s2) * @dev: Pointer to single pin configuration device which is the parent of * the pins node holding the pin configuration data. * @pins: Pointer to the first element of an array of register/value pairs - *of type 'struct single_fdt_pin_cfg'. Each such pair describes the - *the pin to be configured and the value to be used for configuration. + *of type 'u32'. Each such pair describes the pin to be configured + *and the value to be used for configuration. + *The value can either be a simple value if #pinctrl-cells = 1 + *or a configuration value and a pin mux mode value if it is 2 *This pointer points to a 'pinctrl-single,pins' property in the *device-tree. * @size: Size of the 'pins' array in bytes. - *The number of register/value pairs in the 'pins' array therefore - *equals to 'size / sizeof(struct single_fdt_pin_cfg)'. + *The number of cells in the array therefore equals to + *'size / sizeof(u32)'. * @fname: Function name. */ static int single_configure_pins(struct udevice *dev, -const struct single_fdt_pin_cfg *pins, +const u32 *pins, int size, const char *fname) { struct single_pdata *pdata = dev_get_plat(dev); struct single_priv *priv = dev_get_priv(dev); - int n, pin, count = size / sizeof(struct single_fdt_pin_cfg); + int stride = pdata->args_count + 1; + int n, pin, count = size / sizeof(u32); struct single_func *func; phys_addr_t reg; - u32 offset, val; + u32 offset, val, mux; /* If function mask is null, needn't enable it. */ if (!pdata->mask) @@ -344,16 +334,22 @@ static int single_configure_pins(struct udevice *dev, func->name = fname; func->npins = 0; - for (n = 0; n < count; n++, pins++) { - offset = fdt32_to_cpu(pins->reg); + for (n = 0; n < count; n += stride) { + offset = fdt32_to_cpu(pins[n]); if (offset > pdata->offset) { dev_err(dev, " invalid register offset 0x%x\n", offset); continue; } + /* if the pinctrl-cells is 2 then the second cell contains the mux */ + if (stride == 3) + mux = fdt32_to_cpu(pins[n + 2]); + else + mux = 0; + reg = pdata->base + offset; - val = fdt32_to_cpu(pins->val) & pdata->mask; + val = (fdt32_to_cpu(pins[n + 1]) | mux) & pdata->mask; pin = single_get_pin_by_offset(dev, offset); if (pin < 0) { dev_err(dev, " failed to get pin by offset %x\n", @@ -453,7 +449,7 @@ static int single_configure_bits(struct udevice *dev, static int single_set_state(struct udevice *dev, struct udevice *config) { - const struct single_fdt_pin_cfg *prop; + const u32 *prop; const struct single_fdt_bits_cfg *prop_bits; int len; @@ -461,7 +457,7 @@ static int single_set_state(struct udevice *dev, if (prop) { dev_dbg(dev, "configuring pins for %s\n", config->name); - if (len % sizeof(struct single_fdt_pin_cfg)) { + if (len % sizeof(u32)) { dev_dbg(dev, " invalid pin configuration i
Re: [PATCH] board: ti: am43xx: pass boot device information from SPL to U-Boot proper
Hi Josef On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky wrote: > > TI AM43xx SoC supports various boot devices (peripherals). > There is already handoff mechanism prepared to allow passing > the information which boot device was used to load the SPL. > > Use the handoff mechanism to pass this information to U-Boot proper > and set the "boot_device" environment variable in board_late_init. > > Signed-off-by: Josef Lusticky > Cc: Tom Rini > Cc: Lokesh Vutla > Cc: Michael Trimarchi > --- > > I use the boot_device variable later in U-Boot scripting - e.g. to avoid > running > bootcmd when the SPL was loaded from UART but run it when loaded from MMC. > Only AM43xx is supported by this patch, but for other TI SoCs > the procedure should be the same: > - figure out supported boot devices from > arch/arm/include/asm/arch-am33xx/spl.h > or arch/arm/include/asm/arch-omapX/spl.h > - implement setting the boot_device env variable in board_late_init() > > You'll need to enable the following in the config: > CONFIG_BLOBLIST=y (required by CONFIG_HANDOFF) > CONFIG_HANDOFF=y > CONFIG_BLOBLIST_ADDR=0x8700 (i set this based on other values defined by > the DEFAULT_LINUX_BOOT_ENV macro in include/configs/ti_armv7_common.h, you > may want to use a different address) > > arch/arm/include/asm/handoff.h| 3 +++ > arch/arm/mach-omap2/boot-common.c | 9 > board/ti/am43xx/board.c | 38 +++ > 3 files changed, 50 insertions(+) > > diff --git a/arch/arm/include/asm/handoff.h b/arch/arm/include/asm/handoff.h > index 0790d2ab1e..1b7aa432a2 100644 > --- a/arch/arm/include/asm/handoff.h > +++ b/arch/arm/include/asm/handoff.h > @@ -16,6 +16,9 @@ > */ > struct arch_spl_handoff { > ulong usable_ram_top; > +#ifdef CONFIG_ARCH_OMAP2PLUS > + u32 omap_boot_device; > +#endif > }; Simon is working on a more structured way to pass arguments in multi-stage boot. I forget to read all the patches. Anyway adding a specific handoff parameter for one architecture makes no such sense. I will remind you that this was already implemented in the past using dts injection (something that I don't like) Michael > > #endif > diff --git a/arch/arm/mach-omap2/boot-common.c > b/arch/arm/mach-omap2/boot-common.c > index 1268a32503..191bb2a42d 100644 > --- a/arch/arm/mach-omap2/boot-common.c > +++ b/arch/arm/mach-omap2/boot-common.c > @@ -236,3 +236,12 @@ void arch_preboot_os(void) > ahci_reset((void __iomem *)DWC_AHSATA_BASE); > } > #endif > + > +#if CONFIG_IS_ENABLED(HANDOFF) > +int handoff_arch_save(struct spl_handoff *ho) > +{ > + ho->arch.omap_boot_device = spl_boot_device(); > + > + return 0; > +} > +#endif > diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c > index a71b588efc..8c5834fc25 100644 > --- a/board/ti/am43xx/board.c > +++ b/board/ti/am43xx/board.c > @@ -726,6 +726,44 @@ static int device_okay(const char *path) > int board_late_init(void) > { > struct udevice *dev; > + > +#if CONFIG_IS_ENABLED(HANDOFF) > + /* Read peripheral SPL was loaded from */ > + if (gd->spl_handoff) { > + switch (gd->spl_handoff->arch.omap_boot_device) { > + case BOOT_DEVICE_CPGMAC: > + env_set("boot_device", "cpgmac"); > + break; > + case BOOT_DEVICE_MMC1: > + env_set("boot_device", "mmc1"); > + break; > + case BOOT_DEVICE_MMC2: > + env_set("boot_device", "mmc2"); > + break; > + case BOOT_DEVICE_NAND: > + env_set("boot_device", "nand"); > + break; > + case BOOT_DEVICE_NOR: > + env_set("boot_device", "nor"); > + break; > + case BOOT_DEVICE_SPI: > + env_set("boot_device", "spi"); > + break; > + case BOOT_DEVICE_UART: > + env_set("boot_device", "uart"); > + break; > + case BOOT_DEVICE_USB: > + env_set("boot_device", "usb"); > + break; > + case BOOT_DEVICE_USBETH: > + env_set("boot_device", "usbeth"); > + break; > + default: > + env_set("boot_device", "unknown"); > + } > + } > +#endif > + > #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG > set_board_info_env(NULL); > > -- > 2.30.2 > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: [PATCH v2 03/14] clk: mtmips: add clock driver for MediaTek MT7621 SoC
On 11/18/21 8:35 PM, Weijie Gao wrote: This patch adds a clock driver for MediaTek MT7621 SoC. This driver provides clock gate control as well as getting clock frequency for CPU/SYS/XTAL and some peripherals. Signed-off-by: Weijie Gao --- v2 changes: none --- drivers/clk/mtmips/Makefile| 1 + drivers/clk/mtmips/clk-mt7621.c| 260 + include/dt-bindings/clock/mt7621-clk.h | 42 3 files changed, 303 insertions(+) create mode 100644 drivers/clk/mtmips/clk-mt7621.c create mode 100644 include/dt-bindings/clock/mt7621-clk.h diff --git a/drivers/clk/mtmips/Makefile b/drivers/clk/mtmips/Makefile index 732e7f2545..ee8b5afe87 100644 --- a/drivers/clk/mtmips/Makefile +++ b/drivers/clk/mtmips/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_SOC_MT7620) += clk-mt7620.o +obj-$(CONFIG_SOC_MT7621) += clk-mt7621.o obj-$(CONFIG_SOC_MT7628) += clk-mt7628.o diff --git a/drivers/clk/mtmips/clk-mt7621.c b/drivers/clk/mtmips/clk-mt7621.c new file mode 100644 index 00..3799d1806a --- /dev/null +++ b/drivers/clk/mtmips/clk-mt7621.c @@ -0,0 +1,260 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2021 MediaTek Inc. All Rights Reserved. + * + * Author: Weijie Gao + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SYSC_MAP_SIZE 0x100 +#define MEMC_MAP_SIZE 0x1000 + +/* SYSC */ +#define SYSCFG0_REG0x10 +#define XTAL_MODE_SEL_S6 +#define XTAL_MODE_SEL_M0x1c0 Please use genmask to define this: #define XTAL_MODE_SEL_M GENMASK(8, 6) and SEL_S is unnecessary, see commentary below. + +#define CLKCFG0_REG0x2c +#define CPU_CLK_SEL_S 30 +#define CPU_CLK_SEL_M 0xc000 +#define PERI_CLK_SEL 0x10 + +#define CLKCFG1_REG0x30 + +#define CUR_CLK_STS_REG0x44 +#define CUR_CPU_FDIV_S 8 +#define CUR_CPU_FDIV_M 0x1f00 +#define CUR_CPU_FFRAC_S0 +#define CUR_CPU_FFRAC_M0x1f + +/* MEMC */ +#define MEMPLL1_REG0x0604 +#define RG_MEPL_DIV2_SEL_S 1 +#define RG_MEPL_DIV2_SEL_M 0x06 + +#define MEMPLL6_REG0x0618 +#define MEMPLL18_REG 0x0648 +#define RG_MEPL_PREDIV_S 12 +#define RG_MEPL_PREDIV_M 0x3000 +#define RG_MEPL_FBDIV_S4 +#define RG_MEPL_FBDIV_M0x7f0 + +/* Clock sources */ +#define CLK_SRC_CPU-1 +#define CLK_SRC_CPU_D2 -2 +#define CLK_SRC_DDR-3 +#define CLK_SRC_SYS-4 +#define CLK_SRC_XTAL -5 +#define CLK_SRC_PERI -6 Please use an enum. And why are these negative? +/* EPLL clock */ +#define EPLL_CLK 5000 + +struct mt7621_clk_priv { + void __iomem *sysc_base; + void __iomem *memc_base; + int cpu_clk; + int ddr_clk; + int sys_clk; + int xtal_clk; +}; + +static const int mt7621_clks[] = { + [CLK_SYS] = CLK_SRC_SYS, + [CLK_DDR] = CLK_SRC_DDR, + [CLK_CPU] = CLK_SRC_CPU, + [CLK_XTAL] = CLK_SRC_XTAL, + [CLK_MIPS_CNT] = CLK_SRC_CPU_D2, + [CLK_UART3] = CLK_SRC_PERI, + [CLK_UART2] = CLK_SRC_PERI, + [CLK_UART1] = CLK_SRC_PERI, + [CLK_SPI] = CLK_SRC_SYS, + [CLK_I2C] = CLK_SRC_PERI, + [CLK_TIMER] = CLK_SRC_PERI, +}; + +static ulong mt7621_clk_get_rate(struct clk *clk) +{ + struct mt7621_clk_priv *priv = dev_get_priv(clk->dev); + u32 val; + + if (clk->id >= ARRAY_SIZE(mt7621_clks)) + return 0; + + switch (mt7621_clks[clk->id]) { + case CLK_SRC_CPU: + return priv->cpu_clk; + case CLK_SRC_CPU_D2: + return priv->cpu_clk / 2; + case CLK_SRC_DDR: + return priv->ddr_clk; + case CLK_SRC_SYS: + return priv->sys_clk; + case CLK_SRC_XTAL: + return priv->xtal_clk; + case CLK_SRC_PERI: + val = readl(priv->sysc_base + CLKCFG0_REG); + if (val & PERI_CLK_SEL) + return priv->xtal_clk; + else + return EPLL_CLK; + default: + return 0; -ENOSYS + } +} + +static int mt7621_clk_enable(struct clk *clk) +{ + struct mt7621_clk_priv *priv = dev_get_priv(clk->dev); + + if (clk->id > 31) Please compare with a symbol. + return -1; -ENOSYS + + setbits_32(priv->sysc_base + CLKCFG1_REG, BIT(clk->id)); + + return 0; +} + +static int mt7621_clk_disable(struct clk *clk) +{ + struct mt7621_clk_priv *priv = dev
[RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level
This series move env_get_location from soc to board level. As suggested by Michael make no sense to define an unique way for multiple board. One board can boot from emmc and having env on spi flash etc.. Anyways, this function is kept in both imx8mn and imx8mp evk boards instead of being completely dropped. (as suggested by Andrey ) Tommaso Merciai (3): imx8m: drop env_get_location for imx8mn and imx8mp imx: imx8mn_evk: override env_get_location imx: imx8mp_evk: override env_get_location arch/arm/mach-imx/imx8m/soc.c | 39 --- board/freescale/imx8mn_evk/imx8mn_evk.c | 42 + board/freescale/imx8mp_evk/imx8mp_evk.c | 41 3 files changed, 83 insertions(+), 39 deletions(-) -- 2.25.1
[RFC PATCH 1/3] imx8m: drop env_get_location for imx8mn and imx8mp
This function defined for two architecture is not really generic and can generate problem when people add a new board. Signed-off-by: Tommaso Merciai --- Changes since v1: - Fix commit: drop down only env_get_location arch/arm/mach-imx/imx8m/soc.c | 39 --- 1 file changed, 39 deletions(-) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 863508776d..f0030a557a 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1313,45 +1313,6 @@ void do_error(struct pt_regs *pt_regs, unsigned int esr) #endif #if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP) -enum env_location env_get_location(enum env_operation op, int prio) -{ - enum boot_device dev = get_boot_device(); - enum env_location env_loc = ENVL_UNKNOWN; - - if (prio) - return env_loc; - - switch (dev) { -#ifdef CONFIG_ENV_IS_IN_SPI_FLASH - case QSPI_BOOT: - env_loc = ENVL_SPI_FLASH; - break; -#endif -#ifdef CONFIG_ENV_IS_IN_NAND - case NAND_BOOT: - env_loc = ENVL_NAND; - break; -#endif -#ifdef CONFIG_ENV_IS_IN_MMC - case SD1_BOOT: - case SD2_BOOT: - case SD3_BOOT: - case MMC1_BOOT: - case MMC2_BOOT: - case MMC3_BOOT: - env_loc = ENVL_MMC; - break; -#endif - default: -#if defined(CONFIG_ENV_IS_NOWHERE) - env_loc = ENVL_NOWHERE; -#endif - break; - } - - return env_loc; -} - #ifndef ENV_IS_EMBEDDED long long env_get_offset(long long defautl_offset) { -- 2.25.1
[RFC PATCH 2/3] imx: imx8mn_evk: override env_get_location
Override env_get_location function at board level, previously dropped down from soc.c References: - commit f1575f23df1ef704051f218d5bc4aeeb20c2c542 Signed-off-by: Tommaso Merciai --- Changes since v1: - Remove wrong env_get_offset function from commit board/freescale/imx8mn_evk/imx8mn_evk.c | 42 + 1 file changed, 42 insertions(+) diff --git a/board/freescale/imx8mn_evk/imx8mn_evk.c b/board/freescale/imx8mn_evk/imx8mn_evk.c index 9a0a0488bf..ef89a91ea2 100644 --- a/board/freescale/imx8mn_evk/imx8mn_evk.c +++ b/board/freescale/imx8mn_evk/imx8mn_evk.c @@ -5,11 +5,53 @@ #include #include +#include #include +#include +#include #include DECLARE_GLOBAL_DATA_PTR; +enum env_location env_get_location(enum env_operation op, int prio) +{ + enum boot_device dev = get_boot_device(); + enum env_location env_loc = ENVL_UNKNOWN; + + if (prio) + return env_loc; + + switch (dev) { +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH + case QSPI_BOOT: + env_loc = ENVL_SPI_FLASH; + break; +#endif +#ifdef CONFIG_ENV_IS_IN_NAND + case NAND_BOOT: + env_loc = ENVL_NAND; + break; +#endif +#ifdef CONFIG_ENV_IS_IN_MMC + case SD1_BOOT: + case SD2_BOOT: + case SD3_BOOT: + case MMC1_BOOT: + case MMC2_BOOT: + case MMC3_BOOT: + env_loc = ENVL_MMC; + break; +#endif + default: +#if defined(CONFIG_ENV_IS_NOWHERE) + env_loc = ENVL_NOWHERE; +#endif + break; + } + + return env_loc; +} + int board_init(void) { return 0; -- 2.25.1
[RFC PATCH 3/3] imx: imx8mp_evk: override env_get_location
Override env_get_location function at board level, previously dropped down from soc.c References: - commit f1575f23df1ef704051f218d5bc4aeeb20c2c542 Signed-off-by: Tommaso Merciai --- Changes since v1: - Remove wrong env_get_offset function from commit board/freescale/imx8mp_evk/imx8mp_evk.c | 41 + 1 file changed, 41 insertions(+) diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c b/board/freescale/imx8mp_evk/imx8mp_evk.c index 62096c24fb..6b2eeaf152 100644 --- a/board/freescale/imx8mp_evk/imx8mp_evk.c +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -32,6 +34,45 @@ static iomux_v3_cfg_t const wdog_pads[] = { MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B | MUX_PAD_CTRL(WDOG_PAD_CTRL), }; +enum env_location env_get_location(enum env_operation op, int prio) +{ + enum boot_device dev = get_boot_device(); + enum env_location env_loc = ENVL_UNKNOWN; + + if (prio) + return env_loc; + + switch (dev) { +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH + case QSPI_BOOT: + env_loc = ENVL_SPI_FLASH; + break; +#endif +#ifdef CONFIG_ENV_IS_IN_NAND + case NAND_BOOT: + env_loc = ENVL_NAND; + break; +#endif +#ifdef CONFIG_ENV_IS_IN_MMC + case SD1_BOOT: + case SD2_BOOT: + case SD3_BOOT: + case MMC1_BOOT: + case MMC2_BOOT: + case MMC3_BOOT: + env_loc = ENVL_MMC; + break; +#endif + default: +#if defined(CONFIG_ENV_IS_NOWHERE) + env_loc = ENVL_NOWHERE; +#endif + break; + } + + return env_loc; +} + int board_early_init_f(void) { struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR; -- 2.25.1
Re: [RFC PATCH 3/3] imx: imx8mp_evk: override env_get_location
On Fri, 26 Nov 2021 18:43:31 +0100 Tommaso Merciai wrote: > Override env_get_location function at board level, previously dropped > down from soc.c > > References: > - commit f1575f23df1ef704051f218d5bc4aeeb20c2c542 > > Signed-off-by: Tommaso Merciai > --- > Changes since v1: > - Remove wrong env_get_offset function from commit > > board/freescale/imx8mp_evk/imx8mp_evk.c | 41 + > 1 file changed, 41 insertions(+) > > diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c > b/board/freescale/imx8mp_evk/imx8mp_evk.c > index 62096c24fb..6b2eeaf152 100644 > --- a/board/freescale/imx8mp_evk/imx8mp_evk.c > +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c > @@ -5,6 +5,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -17,6 +18,7 @@ > #include > #include > #include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -32,6 +34,45 @@ static iomux_v3_cfg_t const wdog_pads[] = { > MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B | MUX_PAD_CTRL(WDOG_PAD_CTRL), > }; > > +enum env_location env_get_location(enum env_operation op, int prio) > +{ > + enum boot_device dev = get_boot_device(); > + enum env_location env_loc = ENVL_UNKNOWN; > + > + if (prio) > + return env_loc; > + > + switch (dev) { > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH > + case QSPI_BOOT: > + env_loc = ENVL_SPI_FLASH; > + break; > +#endif > +#ifdef CONFIG_ENV_IS_IN_NAND > + case NAND_BOOT: > + env_loc = ENVL_NAND; > + break; > +#endif > +#ifdef CONFIG_ENV_IS_IN_MMC > + case SD1_BOOT: > + case SD2_BOOT: > + case SD3_BOOT: > + case MMC1_BOOT: > + case MMC2_BOOT: > + case MMC3_BOOT: > + env_loc = ENVL_MMC; > + break; > +#endif > + default: > +#if defined(CONFIG_ENV_IS_NOWHERE) > + env_loc = ENVL_NOWHERE; > +#endif Using if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) instead of #ifdefs is preferred because the compiler then warns about bugs even in the disabled codepaths (that's why checkpatch complains about #ifdefs). I know that this cannot be combined with switch() in a simple way, though. Do you need to use switch? Can't you rewrite it to use if()s and use IS_ENABLED()? Marek
Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults
On 11/24/21 9:10 AM, Tom Rini wrote: On Tue, Nov 23, 2021 at 09:16:14PM -0500, Sean Anderson wrote: On 11/22/21 10:02 PM, Peng Fan (OSS) wrote: Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote: + Rob On 2021/11/20 20:57, Tom Rini wrote: On Sat, Nov 20, 2021 at 12:10:54PM +, Peng Fan (OSS) wrote: Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults From: Peng Fan Current code has a force clk_set_defaults in multiple stages, U-Boot reuse the same device tree and Linux Kernel device tree, but we not register all the clks as Linux Kernel, so clk_set_defaults will fail and cause the clk provider registeration fail. So introduce a new property to ignore the default settings which could be used by any node that wanna ignore default settings. Reviewed-by: Simon Glass Signed-off-by: Peng Fan --- V2: Add R-b tag Tom, Simon After a thought, I think still put it as a u-boot thing. assigned-clock-x is actually Linux specific, however I could not add the new property to Linux, because we are supporting SystemReady-IR, we need the assigned-clock-x property in linux working and ignore it in U-Boot. Any more thoughts? Just my continued request that you treat this as generic and submit the binding upstream so it can be in the device tree for the platform. As Sean said, this is to serve cast that linux and U-Boot use the same device tree, I mean U-Boot runtime export device tree to linux for SR-IR (system-ready IR) booting. Linux needs assigned-clocks to some reason, but U-Boot not need that because the driver not added the support or not a must to have that. Because assigned-clocks failure in U-Boot will cause probe fail now, the device driver will report failure. You mean rename this to "ignore-clk-defaults" or keep "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" to linux device tree binding? I could try to send to linux kernel with "firmware" as a prefix. What I mean is that first I'm not seeing the description of the property as being clear enough, either in commit message or the binding itself. That's why in my mind I keep seeing this as "we set the properties linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't know why we need both. Is there not something we can do based on seeing linux,assigned-clocks ? Showing something that makes use of the property you're wishing to add would be helpful. In fact, some specific dts snippets would be helpful to understand what's going on here. clk: clock-controller@3038 { compatible = "fsl,imx8mp-ccm"; reg = <0x3038 0x1>; #clock-cells = <1>; clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>, <&clk_ext3>, <&clk_ext4>; clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2", "clk_ext3", "clk_ext4"; assigned-clocks = <&clk IMX8MP_CLK_A53_SRC>, <&clk IMX8MP_CLK_A53_CORE>, <&clk IMX8MP_CLK_NOC>, <&clk IMX8MP_CLK_NOC_IO>, <&clk IMX8MP_CLK_GIC>, <&clk IMX8MP_CLK_AUDIO_AHB>, <&clk IMX8MP_CLK_AUDIO_AXI_SRC>, <&clk IMX8MP_CLK_IPG_AUDIO_ROOT>, <&clk IMX8MP_AUDIO_PLL1>, <&clk IMX8MP_AUDIO_PLL2>; assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, <&clk IMX8MP_ARM_PLL_OUT>, <&clk IMX8MP_SYS_PLL2_1000M>, <&clk IMX8MP_SYS_PLL1_800M>, <&clk IMX8MP_SYS_PLL2_500M>, <&clk IMX8MP_SYS_PLL1_800M>, <&clk IMX8MP_SYS_PLL1_800M>; assigned-clock-rates = <0>, <0>, <10>, <8>, <5>, <4>, <8>, <4>, <393216000>, <361267200>; u-boot,ignore-clk-defaults; }; The above node is that I wanna have in U-Boot device tree. This node is same as the one used in linux device tree except the new property introduced here. In i.MX U-Boot, we not support the clks here in the clk driver, but linux needs the assigned-clocks property, so I could not delete it because U-Boot runtime export the device tree to Linux. So add a new property here for U-Boot specific usage, if the property is there, U-Boot ignore the assigned-clock settings for a node. I hope this is clear and please suggest what to do next. Hm. Well perhaps we can d
Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults
On Fri, Nov 26, 2021 at 01:17:10PM -0500, Sean Anderson wrote: > On 11/24/21 9:10 AM, Tom Rini wrote: > > On Tue, Nov 23, 2021 at 09:16:14PM -0500, Sean Anderson wrote: > > > On 11/22/21 10:02 PM, Peng Fan (OSS) wrote: > > > > > Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults > > > > > > > > > > On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote: > > > > > > + Rob > > > > > > > > > > > > On 2021/11/20 20:57, Tom Rini wrote: > > > > > > > On Sat, Nov 20, 2021 at 12:10:54PM +, Peng Fan (OSS) wrote: > > > > > > > > > Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults > > > > > > > > > > > > > > > > > > From: Peng Fan > > > > > > > > > > > > > > > > > > Current code has a force clk_set_defaults in multiple stages, > > > > > > > > > U-Boot reuse the same device tree and Linux Kernel device > > > > > > > > > tree, > > > > > > > > > but we not register all the clks as Linux Kernel, so > > > > > > > > > clk_set_defaults will fail and cause the clk provider > > > > > > > > > registeration fail. > > > > > > > > > > > > > > > > > > So introduce a new property to ignore the default settings > > > > > > > > > which > > > > > > > > > could be used by any node that wanna ignore default settings. > > > > > > > > > > > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > --- > > > > > > > > > > > > > > > > > > V2: > > > > > > > > > Add R-b tag > > > > > > > > > Tom, Simon > > > > > > > > > After a thought, I think still put it as a u-boot thing. > > > > > assigned-clock-x is > > > > > > > > > actually Linux specific, however I could not add the > > > > > > > > > new property > > > > > to Linux, > > > > > > > > > because we are supporting SystemReady-IR, we need the > > > > > > > > > assigned-clock-x property > > > > > > > > > in linux working and ignore it in U-Boot. > > > > > > > > > > > > > > > > Any more thoughts? > > > > > > > > > > > > > > Just my continued request that you treat this as generic and > > > > > > > submit > > > > > > > the binding upstream so it can be in the device tree for the > > > > > > > platform. > > > > > > > > > > > > > > > > > > > As Sean said, this is to serve cast that linux and U-Boot use the > > > > > > same > > > > > > device tree, I mean U-Boot runtime export device tree to linux for > > > > > > SR-IR (system-ready IR) booting. > > > > > > > > > > > > Linux needs assigned-clocks to some reason, but U-Boot not need that > > > > > > because the driver not added the support or not a must to have that. > > > > > > > > > > > > Because assigned-clocks failure in U-Boot will cause probe fail now, > > > > > > the device driver will report failure. > > > > > > > > > > > > You mean rename this to "ignore-clk-defaults" or keep > > > > > > "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" to > > > > > > linux > > > > > > device tree binding? > > > > > > > > > > > > I could try to send to linux kernel with "firmware" as a prefix. > > > > > > > > > > What I mean is that first I'm not seeing the description of the > > > > > property as > > > > > being clear enough, either in commit message or the binding itself. > > > > > That's why in my mind I keep seeing this as "we set the properties > > > > > linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't > > > > > know why > > > > > we need both. Is there not something we can do based on seeing > > > > > linux,assigned-clocks ? Showing something that makes use of the > > > > > property > > > > > you're wishing to add would be helpful. In fact, some specific dts > > > > > snippets > > > > > would be helpful to understand what's going on here. > > > > > > > > clk: clock-controller@3038 { > > > > compatible = "fsl,imx8mp-ccm"; > > > > reg = <0x3038 0x1>; > > > > #clock-cells = <1>; > > > > clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>, > > > ><&clk_ext3>, <&clk_ext4>; > > > > clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2", > > > > "clk_ext3", "clk_ext4"; > > > > assigned-clocks = <&clk IMX8MP_CLK_A53_SRC>, > > > > <&clk IMX8MP_CLK_A53_CORE>, > > > > <&clk IMX8MP_CLK_NOC>, > > > > <&clk IMX8MP_CLK_NOC_IO>, > > > > <&clk IMX8MP_CLK_GIC>, > > > > <&clk IMX8MP_CLK_AUDIO_AHB>, > > > > <&clk IMX8MP_CLK_AUDIO_AXI_SRC>, > > > > <&clk IMX8MP_CLK_IPG_AUDIO_ROOT>, > > > > <&clk IMX8MP_AUDIO_PLL1>, > > > > <&clk IMX8MP_AUDIO_PLL2>; > > > > assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, > > > ><&clk IMX8MP_ARM_PLL_OUT>, > > > >
Re: [PATCH] include/linux/byteorder: fix cpu_to_be32_array()
On 5/23/21 21:54, Heinrich Schuchardt wrote: On 5/23/21 9:40 PM, Tom Rini wrote: This was fixed in what commit from Linux? Thanks! include/linux/byteorder/generic.h in Linux is equally broken. This should not stop you from merging this patch. I will create a patch for Linux. Best regards Heinrich Greg Kroah-Hartman will push the correponding patch to Linux Next now: [PATCH 1/1] include/linux/byteorder/generic.h: fix index variables https://lore.kernel.org/all/20210523204958.64575-1-xypron.g...@gmx.de/ Best regards Heinrich
[BUG] cppcheck results for efi_tcg2.c
Hello Masahisa, Ilias, Ruchika, please, have a look at the cppcheck results for efi_tcg2.c. [unusedFunction] may be due to checking a single file. [arithOperationsOnVoidPointer] you might ignore. But the rest should be addressed. Best regards Heinrich $ cppcheck lib/efi_loader/efi_tcg2.c --enable=all --std=c11 Checking lib/efi_loader/efi_tcg2.c ... Checking lib/efi_loader/efi_tcg2.c: __ASSEMBLY__... lib/efi_loader/efi_tcg2.c:1537:6: style: Local variable 'buffer' shadows outer variable [shadowVariable] u8 buffer[TPM2_SHA512_DIGEST_SIZE] = { 0 }; ^ lib/efi_loader/efi_tcg2.c:1497:8: note: Shadowed declaration void *buffer; ^ lib/efi_loader/efi_tcg2.c:1537:6: note: Shadow variable u8 buffer[TPM2_SHA512_DIGEST_SIZE] = { 0 }; ^ include/efi_api.h:991:23: portability: '(void*)blk' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] return ((void *)blk) + sizeof(*blk) + ^ include/efi_api.h:991:38: portability: '((void*)blk)+sizeof(*blk)' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] return ((void *)blk) + sizeof(*blk) + ^ lib/efi_loader/efi_tcg2.c:858:55: portability: 'event_log.buffer' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] *event_log_last_entry = (uintptr_t)(event_log.buffer + event_log.pos - ^ lib/efi_loader/efi_tcg2.c:858:71: portability: 'event_log.buffer+event_log.pos' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] *event_log_last_entry = (uintptr_t)(event_log.buffer + event_log.pos - ^ lib/efi_loader/efi_tcg2.c:1016:34: portability: 'efi' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] nt = (IMAGE_NT_HEADERS32 *)(efi + dos->e_lfanew); ^ lib/efi_loader/efi_tcg2.c:1644:16: portability: 'buffer' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] memset(buffer + spec_event_size, 0, 1); ^ Heinrich: For writing a single byte you should use an assignment after converting to u8* instead of calling a library function. lib/efi_loader/efi_tcg2.c:1359:11: style: Variable 'hash_sz' is assigned a value that is never used. [unreadVariable] hash_sz = ^ lib/efi_loader/efi_tcg2.c:1439:13: style: Variable 'event_type' is assigned a value that is never used. [unreadVariable] event_type = get_unaligned_le32(&event->event_type); ^ lib/efi_loader/efi_tcg2.c:2223:0: style: The function 'efi_tcg2_measure_efi_app_exit' is never used. [unusedFunction] ^ lib/efi_loader/efi_tcg2.c:2170:0: style: The function 'efi_tcg2_measure_efi_app_invocation' is never used. [unusedFunction] ^ lib/efi_loader/efi_tcg2.c:2277:0: style: The function 'efi_tcg2_notify_exit_boot_services_failed' is never used. [unusedFunction] ^ lib/efi_loader/efi_tcg2.c:2358:0: style: The function 'efi_tcg2_register' is never used. [unusedFunction] ^ lib/efi_loader/efi_tcg2.c:951:0: style: The function 'tcg2_measure_pe_image' is never used. [unusedFunction]
Re: [PATCH v7 1/3] efi_loader: Add check for event log passed from firmware
On 11/26/21 14:52, Ilias Apalodimas wrote: On Fri, Nov 26, 2021 at 05:22:59PM +0530, Ruchika Gupta wrote: Platforms may have support to measure their initial firmware components and pass the event log to u-boot. The event log address can be passed in property tpm_event_log_addr and tpm_event_log_size of the tpm node. Platforms may choose their own specific mechanism to do so. A weak function is added to check if even log has been passed to u-boot from earlier firmware components. If available, the eventlog is parsed to check for its correctness and further event logs are appended to the passed log. Signed-off-by: Ruchika Gupta --- v7: Addressed Heinrich's comments Changed functions not exported out of this file as static. Corrected function decsriptions and added few. Added declaration of weak function in header file Moved offset check to parse functions v6: No change v5: Shift the efi_init_event_log() to a different location in the file. This help fixes compilation issue introduced by calling efi_append_scrtm_version() from it. v4: Add SCRTM version to log only if previous firmware doesn't pass the eventlog v3: Return as soon as you detect error v2: Moved firmware eventlog code parsing to tcg2_get_fw_eventlog() include/efi_loader.h | 2 + lib/efi_loader/efi_tcg2.c | 472 -- 2 files changed, 405 insertions(+), 69 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index d52e399841..67c40ca57a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -988,4 +988,6 @@ efi_status_t efi_esrt_register(void); */ efi_status_t efi_esrt_populate(void); efi_status_t efi_load_capsule_drivers(void); + +efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz); #endif /* _EFI_LOADER_H */ diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 8c1f22e337..5ded57fd29 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct udevice **dev) return EFI_NOT_FOUND; } +/** + * platform_get_eventlog() - retrieve the eventlog address and size + * + * This function retrieves the eventlog address and size if the underlying + * firmware has done some measurements and passed them. + * + * This function may be overridden based on platform specific method of + * passing the eventlog address and size. + * + * @dev: udevice + * @addr: eventlog address + * @sz:eventlog size + * Return: status code + */ +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, + u32 *sz) +{ + const u64 *basep; + const u32 *sizep; + + basep = dev_read_prop(dev, "tpm_event_log_addr", NULL); + if (!basep) + return EFI_NOT_FOUND; + + *addr = be64_to_cpup((__force __be64 *)basep); + + sizep = dev_read_prop(dev, "tpm_event_log_size", NULL); + if (!sizep) + return EFI_NOT_FOUND; + + *sz = be32_to_cpup((__force __be32 *)sizep); + if (*sz == 0) { + log_debug("event log empty\n"); + return EFI_NOT_FOUND; + } + + return EFI_SUCCESS; +} + /** * tpm2_get_max_command_size() - get the supported max command size * @@ -1181,6 +1220,283 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = { .get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks, }; +/** + * parse_event_log_header() - Parse and verify the event log header fields + * + * @buffer:Pointer to the start of the eventlog + * @size: Size of the eventlog + * @pos: Return offset of the next event in buffer right + * after the event header i.e specID + * + * Return: status code + */ +static efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos) +{ + struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer; + int i = 0; + + if (size < sizeof(*event_header)) + return EFI_COMPROMISED_DATA; + + if (get_unaligned_le32(&event_header->pcr_index) != 0 || + get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION) + return EFI_COMPROMISED_DATA; + + for (i = 0; i < sizeof(event_header->digest); i++) { + if (event_header->digest[i]) + return EFI_COMPROMISED_DATA; + } + + *pos += sizeof(*event_header); + + return EFI_SUCCESS; +} + +/** + * parse_specid_event() - Parse and verify the specID Event in the eventlog + * + * @dev: udevice + * @buffer:Pointer to the start of the eventlog + * @log_size: Size of the eventlog + * @pos: [in] Offset of specID event in the eventlog buffer + * [out] Return offset of the next event in the buffer + *
Re: [PATCH v7 1/3] efi_loader: Add check for event log passed from firmware
Hi Heinrich, > > +} > > > + > > > +/** > > > + * parse_specid_event() - Parse and verify the specID Event in the > > > eventlog > > > + * > > > + * @dev: udevice > > > + * @buffer: Pointer to the start of the eventlog > > > + * @log_size:Size of the eventlog > > > + * @pos: [in] Offset of specID event in the eventlog buffer > > > + * [out] Return offset of the next event in the > > > buffer > > > + * after the specID > > > + * @digest_list: list of digests in the event > > > + * > > > + * Return: status code > > > + * @pos Offset in the eventlog where the specID event > > > ends > > > + * @digest_list: list of digests in the event > > > + */ > > > +static efi_status_t parse_specid_event(struct udevice *dev, void *buffer, > > > +u32 log_size, u32 *pos, > > > +struct tpml_digest_values *digest_list) > > > +{ > > > + struct tcg_efi_spec_id_event *spec_event; > > > + struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer; > > > + size_t spec_event_size; > > > + u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0; > > > + u32 spec_active = 0; > > > + u16 hash_alg, hash_sz; > > > + u8 vendor_sz; > > > + int err, i; > > > + > > > + if (*pos >= log_size || (*pos + sizeof(*spec_event)) > log_size) > > > + return EFI_COMPROMISED_DATA; > > > + > > > + /* Check specID event data */ > > > + spec_event = (struct tcg_efi_spec_id_event *)((uintptr_t)buffer + *pos); > > > + /* Check for signature */ > > > + if (memcmp(spec_event->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03, > > > +sizeof(TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03))) { > > > + log_err("specID Event: Signature mismatch\n"); > > > + return EFI_COMPROMISED_DATA; > > > + } > > > + > > > + if (spec_event->spec_version_minor != > > > + TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2 || > > > + spec_event->spec_version_major != > > > + TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2) > > > + return EFI_COMPROMISED_DATA; > > > + > > > + if (spec_event->number_of_algorithms > MAX_HASH_COUNT || > > > + spec_event->number_of_algorithms < 1) { > > > + log_err("specID Event: Number of algorithms incorrect\n"); > > > + return EFI_COMPROMISED_DATA; > > > + } > > > + > > > + alg_count = spec_event->number_of_algorithms; > > > + > > > + err = tpm2_get_pcr_info(dev, &supported, &active, &pcr_count); > > > + if (err) > > > + return EFI_DEVICE_ERROR; > > > + > > > + digest_list->count = 0; > > > + /* > > > + * We have to take care that the sequence of algorithms that we record > > > + * in digest_list matches the sequence in eventlog. > > > + */ > > > + for (i = 0; i < alg_count; i++) { > > > + hash_alg = > > > + get_unaligned_le16(&spec_event->digest_sizes[i].algorithm_id); > > > + hash_sz = > > > +get_unaligned_le16(&spec_event->digest_sizes[i].digest_size); This is unused. Do we need it ? > > > + > > > + if (!(supported & alg_to_mask(hash_alg))) { > > > + log_err("specID Event: Unsupported algorithm\n"); > > > + return EFI_COMPROMISED_DATA; > > > + } > > > + digest_list->digests[digest_list->count++].hash_alg = hash_alg; > > > + > > > + spec_active |= alg_to_mask(hash_alg); > > > + } > > > + > > > + /* > > > + * TCG specification expects the event log to have hashes for all > > > + * active PCR's > > > + */ > > > + if (spec_active != active) { > > > + /* > > > + * Previous stage bootloader should know all the active PCR's > > > + * and use them in the Eventlog. > > > + */ > > > + log_err("specID Event: All active hash alg not present\n"); > > > + return EFI_COMPROMISED_DATA; > > > + } > > > + > > > + /* > > > + * the size of the spec event and placement of vendor_info_size > > > + * depends on supported algoriths > > > + */ > > > + spec_event_size = > > > + offsetof(struct tcg_efi_spec_id_event, digest_sizes) + > > > + alg_count * sizeof(spec_event->digest_sizes[0]); > > > + > > > + if (*pos + spec_event_size >= log_size) > > > + return EFI_COMPROMISED_DATA; > > > + > > > + vendor_sz = *(uint8_t *)((uintptr_t)buffer + *pos + spec_event_size); > > > + > > > + spec_event_size += sizeof(vendor_sz) + vendor_sz; > > > + *pos += spec_event_size; > > > + > > > + if (get_unaligned_le32(&event_header->event_size) != spec_event_size) { > > > + log_err("specID event: header event size mismatch\n"); > > > + /* Right way to handle this can be to call SetActive PCR's */ > > > + return EFI_COMPROMISED_DATA; > > > + } > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +/** > > > + * tcg2_parse_event() - Parse the event in the eventlog > > > + * > > > + * @dev: udev
Re: [PATCH v7 3/3] efi_loader: Extend PCR's for firmware measurements
Hi Ruchika, On Fri, 26 Nov 2021 at 13:53, Ruchika Gupta wrote: > > Firmwares before U-Boot may be capable of doing tpm measurements > and passing them to U-Boot in the form of eventlog. However there > may be scenarios where the firmwares don't have TPM driver and > are not capable of extending the measurements in the PCRs. > Based on TCG spec, if previous firnware has extended PCR's, PCR0 > would not be 0. So, read the PCR0 to determine if the PCR's need > to be extended as eventlog is parsed or not. > > Signed-off-by: Ruchika Gupta > Reviewed-by: Ilias Apalodimas > Tested-by: Ilias Apalodimas > --- > v7: > Addressed Heinrick's comments - Added missing parameter in function header > > v6: Changed TPM2_DIGEST_LEN to TPM2_SHA512_DIGEST_SIZE > > v5 : No change > > v4 : No change > > v3 : > Rebase changes on top of changes made in first patch series > > v2 : > Removed check for PCR0 in eventlog > > lib/efi_loader/efi_tcg2.c | 76 +++ > 1 file changed, 76 insertions(+) > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index 5ded57fd29..d247179fbf 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -199,6 +199,44 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, > u32 pcr_index, > return EFI_SUCCESS; > } > > +/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values > + * > + * @dev: device > + * @pcr_index: PCR index > + * @digest_list: list of digest algorithms to extend > + * > + * @Return: status code > + */ > +static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index, > + struct tpml_digest_values *digest_list) > +{ > + struct tpm_chip_priv *priv; > + unsigned int updates, pcr_select_min; > + u32 rc; > + size_t i; > + > + priv = dev_get_uclass_priv(dev); > + if (!priv) > + return EFI_DEVICE_ERROR; > + > + pcr_select_min = priv->pcr_select_min; > + > + for (i = 0; i < digest_list->count; i++) { > + u16 hash_alg = digest_list->digests[i].hash_alg; > + u8 *digest = (u8 *)&digest_list->digests[i].digest; > + > + rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min, > + hash_alg, digest, alg_to_len(hash_alg), > + &updates); > + if (rc) { > + EFI_PRINT("Failed to read PCR\n"); > + return EFI_DEVICE_ERROR; > + } > + } > + > + return EFI_SUCCESS; > +} > + > /* put_event - Append an agile event to an eventlog > * > * @pcr_index: PCR index > @@ -1461,6 +1499,8 @@ static efi_status_t tcg2_get_fw_eventlog(struct udevice > *dev, void *log_buffer, > u32 pcr, pos; > u64 base; > u32 sz; > + bool extend_pcr = false; > + int i; > > ret = platform_get_eventlog(dev, &base, &sz); > if (ret != EFI_SUCCESS) > @@ -1482,6 +1522,26 @@ static efi_status_t tcg2_get_fw_eventlog(struct > udevice *dev, void *log_buffer, > return ret; > } > > + ret = tcg2_pcr_read(dev, 0, &digest_list); > + if (ret) { > + log_err("Error reading PCR 0\n"); > + return ret; > + } > + > + /* > +* If PCR0 is 0, previous firmware didn't have the capability > +* to extend the PCR. In this scenario, extend the PCR as > +* the eventlog is parsed. > +*/ > + for (i = 0; i < digest_list.count; i++) { > + u8 buffer[TPM2_SHA512_DIGEST_SIZE] = { 0 }; I missed that since it doesn't show on this diff but on [1/3]. However there's a prexisting variable in the begging called 'void *buffer', so this needs a different name > + u16 hash_alg = digest_list.digests[i].hash_alg; > + > + if (!memcmp((u8 *)&digest_list.digests[i].digest, buffer, > + alg_to_len(hash_alg))) > + extend_pcr = true; > + } > + > while (pos < sz) { > ret = tcg2_parse_event(dev, buffer, sz, &pos, &digest_list, >&pcr); > @@ -1489,6 +1549,22 @@ static efi_status_t tcg2_get_fw_eventlog(struct > udevice *dev, void *log_buffer, > log_err("Error parsing event\n"); > return ret; > } > + if (extend_pcr) { > + ret = tcg2_pcr_extend(dev, pcr, &digest_list); > + if (ret != EFI_SUCCESS) { > + log_err("Error in extending PCR\n"); > + return ret; > + } > + > + /* Clear the digest for next event */ > + for (i = 0; i < digest_list.count; i++) { > + u16 hash_alg =
Pull request for efi-2022-01-rc3-2
Dear Tom, The following changes since commit 2ad8d0cb950da2233a2ec030533f4e54c6d04126: Merge branch 'efi-2022-01' of https://source.denx.de/u-boot/custodians/u-boot-efi (2021-11-20 09:36:37 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2022-01-rc3-2 for you to fetch changes up to 2b5e7108594d4e100c88f44353d1fed6456d6471: test: fix pylint error in u_boot_console_exec_attach.py (2021-11-26 22:02:37 +0100) Gitlab CI indicated no problems: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/9996 Pull request for efi-2022-01-rc3-2 Test: * fix pylint warnings UEFI: * disable interrupts before removing devices in ExitBootServices() * implement poweroff in efi_system_reset() on sandbox * allow booting via EFI even if some block device fails Heinrich Schuchardt (10): efi_loader: efi_disk_register() should not fail sandbox: poweroff in efi_system_reset() test: address some pylint warnings efi_loader: segfault in efi_clear_os_indications() efi_selftest: simplify endian conversion for FDT test test: fix pylint errors in multiplexed_log.py test: fix pylint errors in u_boot_spawn.py test: fix pylint errors in u_boot_utils.py test: fix pylint error in u_boot_console_sandbox.py test: fix pylint error in u_boot_console_exec_attach.py Tom Rini (1): efi: Call bootm_disable_interrupts earlier in efi_exit_boot_services arch/sandbox/cpu/start.c | 6 +++-- lib/efi_loader/efi_boottime.c | 4 +--- lib/efi_loader/efi_capsule.c | 45 +++ lib/efi_loader/efi_disk.c | 4 ++-- lib/efi_loader/efi_setup.c| 36 +--- lib/efi_selftest/efi_selftest_fdt.c | 15 ++-- test/py/multiplexed_log.py| 14 ++- test/py/tests/test_efi_loader.py | 10 test/py/tests/test_efi_selftest.py| 6 ++--- test/py/u_boot_console_exec_attach.py | 6 +++-- test/py/u_boot_console_sandbox.py | 4 +++- test/py/u_boot_spawn.py | 26 ++-- test/py/u_boot_utils.py | 25 ++- 13 files changed, 95 insertions(+), 106 deletions(-)
[PATCH 1/1] test: fix pylint warnings in test_env.py
* assert does not need parentheses * add module docstring * fix misspelled constant True * limit lines to 100 characters Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_env.py | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index 9bed2f48d7..39fad4194b 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -2,11 +2,13 @@ # Copyright (c) 2015 Stephen Warren # Copyright (c) 2015-2016, NVIDIA CORPORATION. All rights reserved. -# Test operation of shell commands relating to environment variables. +""" +Test operation of shell commands relating to environment variables. +""" import os import os.path -from subprocess import call, check_call, CalledProcessError +from subprocess import call, CalledProcessError import pytest import u_boot_utils @@ -191,7 +193,7 @@ def test_env_printenv_non_existent(state_test_env): c = state_test_env.u_boot_console with c.disable_check('error_notification'): response = c.run_command('printenv %s' % var) -assert(response == '## Error: "%s" not defined' % var) +assert response == '## Error: "%s" not defined' % var @pytest.mark.buildconfigspec('cmd_echo') def test_env_unset_non_existent(state_test_env): @@ -256,7 +258,7 @@ def test_env_import_checksum_no_size(state_test_env): with c.disable_check('error_notification'): response = c.run_command('env import -c %s -' % addr) -assert(response == '## Error: external checksum format must pass size') +assert response == '## Error: external checksum format must pass size' @pytest.mark.buildconfigspec('cmd_importenv') def test_env_import_whitelist_checksum_no_size(state_test_env): @@ -269,7 +271,7 @@ def test_env_import_whitelist_checksum_no_size(state_test_env): with c.disable_check('error_notification'): response = c.run_command('env import -c %s - foo1 foo2 foo4' % addr) -assert(response == '## Error: external checksum format must pass size') +assert response == '## Error: external checksum format must pass size' @pytest.mark.buildconfigspec('cmd_exportenv') @pytest.mark.buildconfigspec('cmd_importenv') @@ -358,12 +360,14 @@ def test_env_info(state_test_env): assert '= true' in l or '= false' in l nb_line += 1 else: -assert true +assert True assert nb_line == 3 response = c.run_command('env info -p -d') -assert 'Default environment is used' in response or "Environment was loaded from persistent storage" in response -assert 'Environment can be persisted' in response or "Environment cannot be persisted" in response +assert 'Default environment is used' in response or \ + "Environment was loaded from persistent storage" in response +assert 'Environment can be persisted' in response or \ + "Environment cannot be persisted" in response response = c.run_command('env info -p -d -q') assert response == "" -- 2.32.0
Re: a question about falcon mode
Hi Alex, Just a quick remarque that intrigued me: Le jeu. 25 nov. 2021 à 15:57, Alex G. a écrit : > > On 11/25/21 1:07 AM, Chan Kim wrote: > > Hello all, > > > > I'm trying to implement falcon mode for our board. Then should I first > > implement the normal mode(spl + proper)? > > > > It looks like so while I'm reading doc/README.falcon. (It says, after > > loading kernel, DT etc. I should give 'spl export' command). > > > > Falcon mode is a bit board dependent. There are a couple of ways you > could go about this. > > The first is to have an "fdtargs" partition. This is where "spl export" > comes in. Once you run "spl export", it will give a modified dtb at > "$fdtargsaddr". It's that DTB that you need to write to your ftdargs > partition. For example: > > > spl export fdt $loadaddr - $fdt_addr_r > > mmc write $fdtargsaddr 0x9800 0x8000 > > In this example the ftdargs partition starts at sector 0x9800, and is > 0x800 sectors long. > > > The second option is to forget about "spl export" and "fdtargs", and > package your kernel, devicetree, and overlays in a FIT container. You'd > make sure to enable SPL_LOAD_FIT_APPLY_OVERLAY. There isn't much more to > this other than the usual gotcha's with FIT and overlays. > Do you mean by this that the SPL has the capability to generate the "fdtargs" by it self (if we provide it with the dtb in the fitImage) ? Form my last experience with the falcon mode, I had a - not sure - conclusion that the only way to generate the "fdtargs" is by using the "spl export" command from uboot cmdline ! because the reality of the fdtargs blob, as its name indicates, is not just the fdt but it has also the bootargs (inside the chosen node ) that are required by the kernel. So if you give only the DTB to the SPL it will not work - to my knowledge -, cuz the data that will be passed to the kernel needs to contain also the bootargs ! Can you please confirm to me if this capability is implemented on the SPL and that we can actually forget about the "spl export" command ? Thanks And apologies Chan for jumping on your thread, Best regards, -- Abder
[PATCH] tpm: avoid NULL pointer dereference in tpm_tis_send()
You should not dereference a pointer before checking if it is NULL. Signed-off-by: Heinrich Schuchardt --- drivers/tpm/tpm2_tis_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c index ec8c730fe9..0ff21a1ef7 100644 --- a/drivers/tpm/tpm2_tis_core.c +++ b/drivers/tpm/tpm2_tis_core.c @@ -218,7 +218,7 @@ static int tpm_tis_ready(struct udevice *dev) int tpm_tis_send(struct udevice *dev, const u8 *buf, size_t len) { struct tpm_chip *chip = dev_get_priv(dev); - struct tpm_tis_phy_ops *phy_ops = chip->phy_ops; + struct tpm_tis_phy_ops *phy_ops; size_t burstcnt, wr_size, sent = 0; u8 data = TPM_STS_GO; u8 status; @@ -227,6 +227,8 @@ int tpm_tis_send(struct udevice *dev, const u8 *buf, size_t len) if (!chip) return -ENODEV; + phy_ops = chip->phy_ops; + ret = tpm_tis_request_locality(dev, 0); if (ret < 0) return -EBUSY; -- 2.32.0
Re: [PATCH] tpm: avoid NULL pointer dereference in tpm_tis_send()
Hi Heinrich, On Fri, Nov 26, 2021 at 11:54:52PM +0100, Heinrich Schuchardt wrote: > You should not dereference a pointer before checking if it is NULL. > > Signed-off-by: Heinrich Schuchardt > --- > drivers/tpm/tpm2_tis_core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c > index ec8c730fe9..0ff21a1ef7 100644 > --- a/drivers/tpm/tpm2_tis_core.c > +++ b/drivers/tpm/tpm2_tis_core.c > @@ -218,7 +218,7 @@ static int tpm_tis_ready(struct udevice *dev) > int tpm_tis_send(struct udevice *dev, const u8 *buf, size_t len) > { > struct tpm_chip *chip = dev_get_priv(dev); > - struct tpm_tis_phy_ops *phy_ops = chip->phy_ops; > + struct tpm_tis_phy_ops *phy_ops; > size_t burstcnt, wr_size, sent = 0; > u8 data = TPM_STS_GO; > u8 status; > @@ -227,6 +227,8 @@ int tpm_tis_send(struct udevice *dev, const u8 *buf, > size_t len) > if (!chip) > return -ENODEV; > > + phy_ops = chip->phy_ops; > + At this point I believe dev_get_priv() wont fail, since the device is opened and running. We can remove the if check though. > ret = tpm_tis_request_locality(dev, 0); > if (ret < 0) > return -EBUSY; > -- > 2.32.0 > Thanks! /Ilias
[PATCH v8] driver: spi: add bcm iproc qspi support
From: Rayagonda Kokatanur IPROC qspi driver supports both BSPI and MSPI modes. Signed-off-by: Rayagonda Kokatanur Signed-off-by: Bharat Gooty Acked-by: Rayagonda Kokatanur Signed-off-by: Roman Bacik --- Changes in v8: - add 4-byte address support Changes in v7: - remove hardcorded IPROC_BSPI_READ_DUMMY_CYCLES - remove unnecessary flags from bspi_read - fix BSPI supported operation condition Changes in v6: - remove priv->mode_4byte - remove IPROC_BSPI_READ_SUPPORTED Changes in v5: - add binding document - implement spi-mem interface for bspi mode - use op->cmd.opcode for BSPI_CMD_AND_MODE_BYTE_REG - move iproc_qspi.c to spi Changes in v4: - move iproc_qspi.c from spi to mtd/spi - remove iproc_qspi.h - rename IPROC_QSPI to SPI_FLASH_IPROC Changes in v3: - fix warning by including linux/delay.h - change ofdata_to_platdata to of_to_plat - change priv_auto_alloc_size to priv_auto Changes in v2: - remove include spi-nor.h - define and use named BITs for writing register values - remove bspi_set_4byte_mode() method .../spi/spi-iproc-qspi.txt| 29 + drivers/spi/Kconfig | 6 + drivers/spi/Makefile | 1 + drivers/spi/iproc_qspi.c | 679 ++ 4 files changed, 715 insertions(+) create mode 100644 doc/device-tree-bindings/spi/spi-iproc-qspi.txt create mode 100644 drivers/spi/iproc_qspi.c diff --git a/doc/device-tree-bindings/spi/spi-iproc-qspi.txt b/doc/device-tree-bindings/spi/spi-iproc-qspi.txt new file mode 100644 index ..fb9f1c2ae2da --- /dev/null +++ b/doc/device-tree-bindings/spi/spi-iproc-qspi.txt @@ -0,0 +1,29 @@ +Broadcom Iproc QSPI controller Device Tree Bindings +--- + +Required properties: +- compatible: should be "brcm,iproc-qspi". +- reg: Base address and size of the controllers memory area. +- reg-names: "bspi", "bspi_raf", "mspi". +- #address-cells: <1>, as required by generic SPI binding. +- #size-cells: <0>, also as required by generic SPI binding. + +Example: + qspi: spi@37 { + compatible = "brcm,iproc-qspi"; + reg = <0x0037 0x100>, + <0x00370100 0x100>, + <0x00370200 0x200>; + reg-names = "bspi", "bspi_raf", "mspi"; + #address-cells = <1>; + #size-cells = <0>; + spi_nor_flash: spi_flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <1250>; + spi-cpol; + spi-cpha; + spi-tx-bus-width = <1>; + spi-rx-bus-width = <4>; + }; + }; diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index d07e9a28af82..faebc212753e 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -178,6 +178,12 @@ config ICH_SPI access the SPI NOR flash on platforms embedding this Intel ICH IP core. +config IPROC_QSPI + bool "Broadcom iProc QSPI Flash Controller driver" + help + Enable Broadcom iProc QSPI Flash Controller driver. + This driver can be used to access the SPI NOR flash. + config KIRKWOOD_SPI bool "Marvell Kirkwood SPI Driver" help diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index d2f24bccefd3..869763187062 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_FSL_DSPI) += fsl_dspi.o obj-$(CONFIG_FSL_ESPI) += fsl_espi.o obj-$(CONFIG_SYNQUACER_SPI) += spi-synquacer.o obj-$(CONFIG_ICH_SPI) += ich.o +obj-$(CONFIG_IPROC_QSPI) += iproc_qspi.o obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o obj-$(CONFIG_MESON_SPIFC) += meson_spifc.o obj-$(CONFIG_MPC8XX_SPI) += mpc8xx_spi.o diff --git a/drivers/spi/iproc_qspi.c b/drivers/spi/iproc_qspi.c new file mode 100644 index ..72b1d09c0b58 --- /dev/null +++ b/drivers/spi/iproc_qspi.c @@ -0,0 +1,679 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2020-2021 Broadcom + */ + +#include +#include +#include +#include +#include +#include +#include + +/* 175MHz */ +#define QSPI_AXI_CLK 17500 +#define QSPI_WAIT_TIMEOUT_MS 200U +#define DWORD_ALIGNED(a) (!(((ulong)(a)) & 3)) + +/* Chip attributes */ +#define SPBR_MIN 8U +#define SPBR_MAX 255U +#define NUM_CDRAM 16U + +#define CDRAM_PCS0 2 +#define CDRAM_CONT BIT(7) +#define CDRAM_BITS_EN BIT(6) +#define CDRAM_QUAD_MODEBIT(8) +#define CDRAM_RBIT_INPUT BIT(10) +#define MSPI_SPE BIT(6) +#define MSPI_CONT_AFTER_CMDBIT(7) + +/* Register
Re: [U-BOOT-TEST-HOOKS PATCH 1/1] Enable TPMv2 emulation
On Wed, Nov 24, 2021 at 08:33:42AM +0100, Heinrich Schuchardt wrote: > On 11/24/21 08:23, Ilias Apalodimas wrote: > > Hi Heinrich, > > > > On Mon, 15 Nov 2021 at 12:11, Heinrich Schuchardt > > wrote: > > > > > > Provide a QEMU helper script to launch swtpm and add extra parameters to > > > conf.qemu_arm64_na and conf.qemu_arm_na to provide an emulated TPMv2. > > > > > > Signed-off-by: Heinrich Schuchardt > > > --- > > > bin/qemu.swtpm | 19 +++ > > > bin/travis-ci/conf.qemu_arm64_na | 3 ++- > > > bin/travis-ci/conf.qemu_arm_na | 3 ++- > > > 3 files changed, 23 insertions(+), 2 deletions(-) > > > create mode 100755 bin/qemu.swtpm > > > > > > diff --git a/bin/qemu.swtpm b/bin/qemu.swtpm > > > new file mode 100755 > > > index 000..089feba > > > --- /dev/null > > > +++ b/bin/qemu.swtpm > > > @@ -0,0 +1,19 @@ > > > +#!/bin/sh > > > +# SPDX-License-Identifier: BSD-2 > > > +# > > > +# This script launches swtpm to emulate a TPMv2. The parameter -t makes > > > it > > > +# unload when the connection to QEMU is terminated. To make use of it add > > > +# > > > +# qemu_helper_script="swtpm" > > > +# > > > +# to the board script and the following arguments to qemu_extra_args > > > +# > > > +# -chardev socket,id=chrtpm,path=/tmp/tpm/swtpm-sock \ > > > +# -tpmdev emulator,id=tpm0,chardev=chrtpm \ > > > +# -device tpm-tis-device,tpmdev=tpm0 > > > +# > > > +# U-Boot must be built with CONFIG_TPM2_MMIO=y. > > > + > > > +mkdir -p /tmp/tpm > > > +swtpm socket -t --tpmstate dir=/tmp/tpm --tpm2 \ > > > +--ctrl type=unixio,path=/tmp/tpm/swtpm-sock & > > > > Nit pick the & can be '-d' > > Daemonizing will ensure that we don't get console output. I will change > this. > > > > > > diff --git a/bin/travis-ci/conf.qemu_arm64_na > > > b/bin/travis-ci/conf.qemu_arm64_na > > > index e7c9426..14577d8 100644 > > > --- a/bin/travis-ci/conf.qemu_arm64_na > > > +++ b/bin/travis-ci/conf.qemu_arm64_na > > > @@ -22,8 +22,9 @@ > > > > > > console_impl=qemu > > > qemu_machine="virt" > > > +qemu_helper_script="swtpm" > > > qemu_binary="qemu-system-aarch64" > > > -qemu_extra_args="-cpu cortex-a57 -nographic -netdev > > > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 > > > -device virtio-rng-pci" > > > +qemu_extra_args="-cpu cortex-a57 -nographic -netdev > > > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 > > > -device virtio-rng-pci -chardev socket,id=chrtpm,path=/tmp/tpm/swtpm-sock > > > -tpmdev emulator,id=tpm0,chardev=chrtpm -device > > > tpm-tis-device,tpmdev=tpm0" > > > qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/u-boot.bin" > > > reset_impl=none > > > flash_impl=none > > > diff --git a/bin/travis-ci/conf.qemu_arm_na > > > b/bin/travis-ci/conf.qemu_arm_na > > > index 0f07c80..de0694d 100644 > > > --- a/bin/travis-ci/conf.qemu_arm_na > > > +++ b/bin/travis-ci/conf.qemu_arm_na > > > @@ -22,8 +22,9 @@ > > > > > > console_impl=qemu > > > qemu_machine="virt" > > > +qemu_helper_script="swtpm" > > > qemu_binary="qemu-system-arm" > > > -qemu_extra_args="-nographic -netdev > > > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 > > > -device virtio-rng-pci" > > > +qemu_extra_args="-nographic -netdev > > > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 > > > -device virtio-rng-pci -chardev socket,id=chrtpm,path=/tmp/tpm/swtpm-sock > > > -tpmdev emulator,id=tpm0,chardev=chrtpm -device > > > tpm-tis-device,tpmdev=tpm0" > > > > Just a note here 'tpm-tis-device' works for arm. If we evenr need > > this on x86 it's 'tpm-tis' > > This file is ARM specific. Sure, but it's worth noting since if we can also use these features and tests on qemu-x86_64 we should. Doesn't need to be to start with tho. And I will apply this shortly. -- Tom signature.asc Description: PGP signature
Re: Pull request for efi-2022-01-rc3-2
On Fri, Nov 26, 2021 at 10:42:56PM +0100, Heinrich Schuchardt wrote: > Dear Tom, > > The following changes since commit 2ad8d0cb950da2233a2ec030533f4e54c6d04126: > > Merge branch 'efi-2022-01' of > https://source.denx.de/u-boot/custodians/u-boot-efi (2021-11-20 09:36:37 > -0500) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-efi.git > tags/efi-2022-01-rc3-2 > > for you to fetch changes up to 2b5e7108594d4e100c88f44353d1fed6456d6471: > > test: fix pylint error in u_boot_console_exec_attach.py (2021-11-26 > 22:02:37 +0100) > > Gitlab CI indicated no problems: > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/9996 > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] dt-bindings: u-boot: Add a few more options bindings
+ others Are there no comments on this? - Simon On Fri, 19 Nov 2021 at 17:13, Simon Glass wrote: > > +Rob Herring oops > > > On Fri, 19 Nov 2021 at 17:04, Simon Glass wrote: > > > > This adds three new options with varying degree of interest / precedent. > > > > This being sent to the mailing list since it might attract more review. > > A PR will be sent when this has had some review. That is why the file > > path is set up for https://github.com/devicetree-org/dt-schema rather > > than the Linux kernel. > > > > Signed-off-by: Simon Glass > > --- > > > > schemas/options/u-boot.yaml | 51 + > > 1 file changed, 51 insertions(+) > > > > diff --git a/schemas/options/u-boot.yaml b/schemas/options/u-boot.yaml > > index 71dfda7..b8bdec1 100644 > > --- a/schemas/options/u-boot.yaml > > +++ b/schemas/options/u-boot.yaml > > @@ -71,6 +71,37 @@ properties: > >2: use simplified command line (e.g. avoid hush) > >3... reserved > > > > + load-environment: > > +$ref: /schemas/types.yaml#/definitions/uint32 > > +default: 1 > > +maximum: 1 > > +description: | > > + This allows control over whether U-Boot loads its environment after > > + relocation. This normally happens automatically, but can pose a > > security > > + risk, so disabling it in certain situations is useful. > > + > > + Note: This could be a boolean. It is defined as an integer since that > > + allows changing the value without resizing the devicetree. I'm not > > sure > > + how ugly that is, but IMO the fact that 'false' boolean values are > > + represented by being missing is a bit of a pain. One must either add > > or > > + delete the property. > > + > > + Values: > > + > > + 0: don't load the environment > > + 1: do load the environment > > + > > + no-apm-final: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: | > > + For devices running on coreboot, this tells U-Boot not to lock down > > the > > + Intel Management Engine (ME) registers. This allows U-Boot to access > > the > > + hardware more fully for platforms that need it. > > + > > + Absence of this property indicates that the ME registers should be > > locked > > + down as part of U-Boot's start-up sequence and before the command > > line is > > + available. > > + > >silent-console: > > $ref: /schemas/types.yaml#/definitions/uint32 > > default: 0 > > @@ -88,6 +119,23 @@ properties: > > enabled) > >2: console output is suppressed and not recorded > > > > + spl-payload-offset: > > +$ref: /schemas/types.yaml#/definitions/uint32 > > +default: 0 > > +description: | > > + If present (and SPL is controlled by the devicetree), this allows the > > + offset of the SPL payload (typically U-Boot) to be specified. The > > offset > > + is in bytes from the start of the media (typically SPI flash). > > + > > + Note: This is quite widely used in U-Boot, but since v2018.01 it is > > + possible to use Binman instead, to provide this offset (and various > > + others) to SPL, or even to U-Boot proper. So far I have not tried > > sending > > + the Binman bindings upstream, but perhaps that should be done > > instead. > > + > > + See here for details: > > + > > + > > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format > > + > > required: > >- compatible > > > > @@ -101,6 +149,9 @@ examples: > > bootcmd = "vboot go auto"; > > bootdelay-sec = <(-1)>; > > bootsecure = <1>; > > +load-environment = <0>; > > +no-apm-final; > > silent-console = <1>; > > +spl-payload-offset = <0x4>; /* 256K */ > >}; > > }; > > -- > > 2.34.0.rc2.393.gf8c9666880-goog > >