[PATCH] qemu-arm: set CONFIG_SYS_BOOTM_LEN to SZ_64M
FIT image contents can be larger than default bootm limit 8M with initrd. Raise limit to 64MB which is commonly used elsewhere. Signed-off-by: Dhananjay Phadke --- include/configs/qemu-arm.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index fb599c9510..1ef75a8783 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -17,6 +17,8 @@ #define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_SDRAM_BASE + SZ_2M) #define CONFIG_SYS_MALLOC_LEN SZ_16M +#define CONFIG_SYS_BOOTM_LEN SZ_64M + /* For timer, QEMU emulates an ARMv7/ARMv8 architected timer */ #define CONFIG_SYS_HZ 1000 -- 2.17.1
Re: [PATCH] mtd: spi-nor-ids: Add support for W25Q512JV
On 4/20/2022 8:18 AM, Jaap Crezee wrote: Hi Dhananjay, On 1/9/22 00:47, Dhananjay Phadke wrote: Will send v2 to use "w25q512jvm" name to be consistent with other DTR parts. Any follow-up on this? Hi Jaap, I didn't get hold of the system with that flash part to collect SFDP output for Linux submission first, then will update u-boot if it's not there already. Regards, Dhananjay
Re: u-boot and IPv6
On Wed, 13 Jul 2022, Chris Packham wrote: > On Tue, Jul 12, 2022 at 7:40 PM Chris Packham wrote: >> >> Hi Sean and Dhananjay, >> >> Adding U-Boot ML to the Cc >> [...] >> >> IPv6 support is definitely something I want to get landed in upstream U-Boot. >> >> It seemed to be fairly well received last time I posted the series. The main >> thing lacking was tests. That's pretty much where things were at last time I >> touched it. As usual life and work commitments have stopped me from >> progressing it further. >> >> In the meantime U-Boot's testing infrastructure has improved a lot which >> should make adding the tests easier. I know there have been some more >> changes in net that will conflict but not badly. >> >> At $dayjob we have merged a newer u-boot version into our fork so I do have >> something that is effectively a post state for the merge conflict to refer >> to but it's not the broken out series that I'd like to submit. >> >> I think I should be able to get the series rebased against master (or the >> just released 2022.07 might be more useful as a base). I can probably get >> that up on github in the next couple of days. > > I've rebased my series against v2022.07 you can get the code from the > ipv6 branch of https://github.com/cpackham/u-boot.git > > I've only lightly compile tested it so I've no idea if it's still functional Hi Chris, Thank you for refreshing the topic and rebasing. Are you planning to post a v4? Besides tests, you had identified some gaps in previous series (DHCP, SLAAC, etc). We are looking for having IPv6 in upstream, happy to fill in what will help it. Regards, Dhananjay
[PATCH] gpio: aspeed: port Linux dt-bindings header file
Ported as is, makes it easier to add readable GPIO definitions in DTS files. Signed-off-by: Dhananjay Phadke --- include/dt-bindings/gpio/aspeed-gpio.h | 49 ++ 1 file changed, 49 insertions(+) create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h diff --git a/include/dt-bindings/gpio/aspeed-gpio.h b/include/dt-bindings/gpio/aspeed-gpio.h new file mode 100644 index 00..56fc4889b2 --- /dev/null +++ b/include/dt-bindings/gpio/aspeed-gpio.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * This header provides constants for binding aspeed,*-gpio. + * + * The first cell in Aspeed's GPIO specifier is the GPIO ID. The macros below + * provide names for this. + * + * The second cell contains standard flag values specified in gpio.h. + */ + +#ifndef _DT_BINDINGS_GPIO_ASPEED_GPIO_H +#define _DT_BINDINGS_GPIO_ASPEED_GPIO_H + +#include + +#define ASPEED_GPIO_PORT_A 0 +#define ASPEED_GPIO_PORT_B 1 +#define ASPEED_GPIO_PORT_C 2 +#define ASPEED_GPIO_PORT_D 3 +#define ASPEED_GPIO_PORT_E 4 +#define ASPEED_GPIO_PORT_F 5 +#define ASPEED_GPIO_PORT_G 6 +#define ASPEED_GPIO_PORT_H 7 +#define ASPEED_GPIO_PORT_I 8 +#define ASPEED_GPIO_PORT_J 9 +#define ASPEED_GPIO_PORT_K 10 +#define ASPEED_GPIO_PORT_L 11 +#define ASPEED_GPIO_PORT_M 12 +#define ASPEED_GPIO_PORT_N 13 +#define ASPEED_GPIO_PORT_O 14 +#define ASPEED_GPIO_PORT_P 15 +#define ASPEED_GPIO_PORT_Q 16 +#define ASPEED_GPIO_PORT_R 17 +#define ASPEED_GPIO_PORT_S 18 +#define ASPEED_GPIO_PORT_T 19 +#define ASPEED_GPIO_PORT_U 20 +#define ASPEED_GPIO_PORT_V 21 +#define ASPEED_GPIO_PORT_W 22 +#define ASPEED_GPIO_PORT_X 23 +#define ASPEED_GPIO_PORT_Y 24 +#define ASPEED_GPIO_PORT_Z 25 +#define ASPEED_GPIO_PORT_AA 26 +#define ASPEED_GPIO_PORT_AB 27 +#define ASPEED_GPIO_PORT_AC 28 + +#define ASPEED_GPIO(port, offset) \ + ((ASPEED_GPIO_PORT_##port * 8) + offset) + +#endif -- 2.25.1
Re: [PATCH] gpio: aspeed: port Linux dt-bindings header file
On 8/2/2022 10:34 PM, Billy Tsai wrote: Hello, On 2022/8/3, 10:10 AM, "Billy Tsai" wrote: Reviewed-by: Billy Tsai On 2022/8/3, 9:46 AM, "ChiaWei Wang" wrote: Acked-by: Chia-Wei Wang > > From: Dhananjay Phadke > > Sent: Wednesday, August 3, 2022 4:55 AM > > > > Ported as is, makes it easier to add readable GPIO definitions in DTS files. > > > > Signed-off-by: Dhananjay Phadke > > --- > > include/dt-bindings/gpio/aspeed-gpio.h | 49 ++ > > 1 file changed, 49 insertions(+) > > create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h [...] > > + > > +#define ASPEED_GPIO(port, offset) \ > > +((ASPEED_GPIO_PORT_##port * 8) + offset) Please help to fix the check patch warning. "Macro argument 'offset' may be better as '(offset)' to avoid precedence issues" ((ASPEED_GPIO_PORT_##port * 8) + (offset)) > > + Yes, I noticed this, but it would mean this will diverge from kernel. I can send v2 here and then sync to kernel later. Regards, Dhananjay
[PATCH v2] gpio: aspeed: port Linux dt-bindings header file
Ported as is, makes it easier to add readable GPIO definitions in DTS files. Signed-off-by: Dhananjay Phadke --- include/dt-bindings/gpio/aspeed-gpio.h | 49 ++ 1 file changed, 49 insertions(+) create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h diff --git a/include/dt-bindings/gpio/aspeed-gpio.h b/include/dt-bindings/gpio/aspeed-gpio.h new file mode 100644 index 00..adab2f56c2 --- /dev/null +++ b/include/dt-bindings/gpio/aspeed-gpio.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * This header provides constants for binding aspeed,*-gpio. + * + * The first cell in Aspeed's GPIO specifier is the GPIO ID. The macros below + * provide names for this. + * + * The second cell contains standard flag values specified in gpio.h. + */ + +#ifndef _DT_BINDINGS_GPIO_ASPEED_GPIO_H +#define _DT_BINDINGS_GPIO_ASPEED_GPIO_H + +#include + +#define ASPEED_GPIO_PORT_A 0 +#define ASPEED_GPIO_PORT_B 1 +#define ASPEED_GPIO_PORT_C 2 +#define ASPEED_GPIO_PORT_D 3 +#define ASPEED_GPIO_PORT_E 4 +#define ASPEED_GPIO_PORT_F 5 +#define ASPEED_GPIO_PORT_G 6 +#define ASPEED_GPIO_PORT_H 7 +#define ASPEED_GPIO_PORT_I 8 +#define ASPEED_GPIO_PORT_J 9 +#define ASPEED_GPIO_PORT_K 10 +#define ASPEED_GPIO_PORT_L 11 +#define ASPEED_GPIO_PORT_M 12 +#define ASPEED_GPIO_PORT_N 13 +#define ASPEED_GPIO_PORT_O 14 +#define ASPEED_GPIO_PORT_P 15 +#define ASPEED_GPIO_PORT_Q 16 +#define ASPEED_GPIO_PORT_R 17 +#define ASPEED_GPIO_PORT_S 18 +#define ASPEED_GPIO_PORT_T 19 +#define ASPEED_GPIO_PORT_U 20 +#define ASPEED_GPIO_PORT_V 21 +#define ASPEED_GPIO_PORT_W 22 +#define ASPEED_GPIO_PORT_X 23 +#define ASPEED_GPIO_PORT_Y 24 +#define ASPEED_GPIO_PORT_Z 25 +#define ASPEED_GPIO_PORT_AA 26 +#define ASPEED_GPIO_PORT_AB 27 +#define ASPEED_GPIO_PORT_AC 28 + +#define ASPEED_GPIO(port, offset) \ + ((ASPEED_GPIO_PORT_##port * 8) + (offset)) + +#endif -- 2.25.1
Re: [PATCH v2] gpio: aspeed: port Linux dt-bindings header file
On 8/3/2022 7:35 PM, Joel Stanley wrote: On Thu, 4 Aug 2022 at 01:02, Dhananjay Phadke wrote: Ported as is, makes it easier to add readable GPIO definitions in DTS files. Signed-off-by: Dhananjay Phadke --- include/dt-bindings/gpio/aspeed-gpio.h | 49 ++ 1 file changed, 49 insertions(+) create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h diff --git a/include/dt-bindings/gpio/aspeed-gpio.h b/include/dt-bindings/gpio/aspeed-gpio.h new file mode 100644 index 00..adab2f56c2 --- /dev/null +++ b/include/dt-bindings/gpio/aspeed-gpio.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ As you mention this is from the kernel, but you forgot to add the copyright information for the file you copied. I suggest you add: Copyright 2022 IBM Corp. It would have been nicer to assert copyrights in kernel source itself, so that it would have been inherited instead of inserting here. For now, will send v3 with just that. Thanks, Dhananjay
[PATCH v3] gpio: aspeed: port Linux dt-bindings header file
Makes it easier to add readable GPIO definitions in DTS files for Aspeed SOC based boards. Ported with small edits to add IBM copyright statement and fix for checkpatch warning. Signed-off-by: Dhananjay Phadke Reviewed-by: Billy Tsai Acked-by: Chia-Wei Wang --- include/dt-bindings/gpio/aspeed-gpio.h | 51 ++ 1 file changed, 51 insertions(+) create mode 100644 include/dt-bindings/gpio/aspeed-gpio.h diff --git a/include/dt-bindings/gpio/aspeed-gpio.h b/include/dt-bindings/gpio/aspeed-gpio.h new file mode 100644 index 00..a49f5d5b5a --- /dev/null +++ b/include/dt-bindings/gpio/aspeed-gpio.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2022 IBM Corp. + * + * This header provides constants for binding aspeed,*-gpio. + * + * The first cell in Aspeed's GPIO specifier is the GPIO ID. The macros below + * provide names for this. + * + * The second cell contains standard flag values specified in gpio.h. + */ + +#ifndef _DT_BINDINGS_GPIO_ASPEED_GPIO_H +#define _DT_BINDINGS_GPIO_ASPEED_GPIO_H + +#include + +#define ASPEED_GPIO_PORT_A 0 +#define ASPEED_GPIO_PORT_B 1 +#define ASPEED_GPIO_PORT_C 2 +#define ASPEED_GPIO_PORT_D 3 +#define ASPEED_GPIO_PORT_E 4 +#define ASPEED_GPIO_PORT_F 5 +#define ASPEED_GPIO_PORT_G 6 +#define ASPEED_GPIO_PORT_H 7 +#define ASPEED_GPIO_PORT_I 8 +#define ASPEED_GPIO_PORT_J 9 +#define ASPEED_GPIO_PORT_K 10 +#define ASPEED_GPIO_PORT_L 11 +#define ASPEED_GPIO_PORT_M 12 +#define ASPEED_GPIO_PORT_N 13 +#define ASPEED_GPIO_PORT_O 14 +#define ASPEED_GPIO_PORT_P 15 +#define ASPEED_GPIO_PORT_Q 16 +#define ASPEED_GPIO_PORT_R 17 +#define ASPEED_GPIO_PORT_S 18 +#define ASPEED_GPIO_PORT_T 19 +#define ASPEED_GPIO_PORT_U 20 +#define ASPEED_GPIO_PORT_V 21 +#define ASPEED_GPIO_PORT_W 22 +#define ASPEED_GPIO_PORT_X 23 +#define ASPEED_GPIO_PORT_Y 24 +#define ASPEED_GPIO_PORT_Z 25 +#define ASPEED_GPIO_PORT_AA 26 +#define ASPEED_GPIO_PORT_AB 27 +#define ASPEED_GPIO_PORT_AC 28 + +#define ASPEED_GPIO(port, offset) \ + ((ASPEED_GPIO_PORT_##port * 8) + (offset)) + +#endif -- 2.25.1
Re: [PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support
On Wed, 20 Oct 2021, Chia-Wei Wang wrote: > +static const struct hash_ops aspeed_hace_ops = { > + .hash_init = aspeed_hace_init, > + .hash_update = aspeed_hace_update, > + .hash_finish = aspeed_hace_finish, > + .hash_digest_wd = aspeed_hace_digest_wd, > + .hash_digest = aspeed_hace_digest, > +}; > + > +static const struct udevice_id aspeed_hace_ids[] = { > + { .compatible = "aspeed,ast2600-hace" }, > + { } > +}; > + Why only ast2600? Is ast2500 engine different or this driver can also support it? Dhananjay
Re: Query: Error: Unable to find signature node in Verified boot
Signature node must be in u-boot's FDT, written by mkimage -K ... On Tue, Dec 17, 2019 at 4:38 AM Priyanka Singh wrote: > Hi, > > I was trying to test the Verified boot feature on NXP's LX2160ARDB board > and I am facing the following issue during the same: > > 1. U-boot is unable to verify the signatures of the images and > configurations. I added more logs to debug this issue and found that U-boot > is unable to find the signature node and returns from common/image-sig.c > line: 260. > Log snippet: > Verifying Hash Integrity ... sha1,rsa2048:dev- OK > > The signing of the images and configuration has been done using mkimage. > > Can someone please suggest how can I correct this. > > Thanks, > Priyanka Singh >
[PATCH] mtd: spi-nor-ids: Add support for W25Q512JV
Add support for Winbond W25Q512JV 64MB SPI NOR flash. https://www.digikey.com/en/product-highlight/w/winbond/w25q512jv-512-mb-serial-flash-memory Signed-off-by: Dhananjay Phadke --- drivers/mtd/spi/spi-nor-ids.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index b551ebd75e..017b165141 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -355,6 +355,11 @@ const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, + { + INFO("w25q512jv", 0xef7020, 0, 64 * 1024, 1024, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) + }, { INFO("w25q01jv", 0xef4021, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | -- 2.25.1
Re: [PATCH] mtd: spi-nor-ids: Add support for W25Q512JV
On 1/8/2022 1:39 PM, Dhananjay Phadke wrote: Add support for Winbond W25Q512JV 64MB SPI NOR flash. https://www.digikey.com/en/product-highlight/w/winbond/w25q512jv-512-mb-serial-flash-memory Signed-off-by: Dhananjay Phadke --- drivers/mtd/spi/spi-nor-ids.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index b551ebd75e..017b165141 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -355,6 +355,11 @@ const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, + { + INFO("w25q512jv", 0xef7020, 0, 64 * 1024, 1024, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) + }, { INFO("w25q01jv", 0xef4021, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | Will send v2 to use "w25q512jvm" name to be consistent with other DTR parts. Regards, Dhananjay
Re: [PATCH v2] drivers: spi-nor: Add JEDEC id for W25Q16JV
On 1/13/2022 9:17 AM, Angus Ainslie wrote: Add a JEDEC id for the Winbond W25Q16JV 16M-BIT serial flash memory with DUAL/QUAD SPI Changes since v1: Updated the name for more suffixes Signed-off-by: Angus Ainslie --- drivers/mtd/spi/spi-nor-ids.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index b551ebd75e..3bd0ded443 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -314,6 +314,11 @@ const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, + { + INFO("w25q16jv-im/jm", 0xef7015, 0, 64 * 1024, 32, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) + }, Shouldn't this be w25q16jvm, which seems to be unofficial convention for *-DTR parts? I was looking for naming convention for another Winbond part - https://lore.kernel.org/u-boot/0c3e4727-0997-a4c5-dab1-b09ea8781...@linux.microsoft.com/ { INFO("w25q32jv", 0xef7016, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
Re: [PATCH 0/3] mkimage: allow to specify signing algorithm
On 1/13/2022 4:38 AM, Jan Kiszka wrote: On 25.11.21 20:03, Jan Kiszka wrote: Another step to decouple the FIT image specification from the actual signing: With these changes, the signature nodes can leave out an algo property, mkimage will initialize that as well while signing. This way, in-tree FIT source files can be prepared for gaining signatures without defining the key type or size upfront, forcing users to patch the code to change that. [resend to the list] While encryption algo (rsa2048 vs rsa4096, etc) shouldn't need to be explicitly specified as you noted below, how does it help to add it to (already exhaustive) mkimage args? Parsing OID / length from keyfile would be real change. While rotating keys is common, how often algo is changed? Patch 1 is preparatory for this, patch 2 a drive-by cleanup. A better solution would actually be if the algorithm was derived from the provided key. But the underlying crypto layer seems to be rather unprepared for that. Thanks, DHananjay
Re: [PATCH v2] drivers: spi-nor: Add JEDEC id for W25Q16JV
On 1/14/2022 6:04 AM, Angus Ainslie wrote: }, + { + INFO("w25q16jv-im/jm", 0xef7015, 0, 64 * 1024, 32, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) + }, Shouldn't this be w25q16jvm, which seems to be unofficial convention for *-DTR parts? I was looking for naming convention for another Winbond part - https://lore.kernel.org/u-boot/0c3e4727-0997-a4c5-dab1-b09ea8781...@linux.microsoft.com/ Would you like a v3 using this naming convention ? Sure, let's try to follow this convention, I'm also doing same in my v2 patch. Regards, Dhananjay
Re: [PATCH v3] drivers: spi-nor: Add JEDEC id for W25Q16JV
On 1/20/2022 7:46 AM, tudor.amba...@microchip.com wrote: FWIW, "w25q15jvm" would be the "correct" name in linux. Although for this particular flash the name in linux is "w25q16jv-im/jm".. would you fix it? :D Anyhow, see [1] for a lengthy discussion on that name. That being said, I still don't share Tudors opinion on using that made up name, just because no one will find any flash if one searches for w25q16jvm in the internet. The temperature range "I/J" is skipped in the name because it does not have any influence on the jedec-id. That's why we don't include the "Package Type" field in the naming convention as well. Okay, I'm merging v3. let me know if you have any questions? For now it should be fine. But if anyone is feeling brave enough maybe we should fix the naming scheme for winbond once and for all. It seems that the flashes were we append "m" support DTR, and in fact winbond names them blabla-dtr [1]. So instead of "w25q16jvm" maybe it's better to convert to "w25q16jv-dtr". Are there any volunteers to check if my assumption is correct? [1] https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/?__locale=en&selected=128Mb#Density I would be happy to do it if there's consensus that we can break downstream device-tree users that hold older compatible string :-). I thought of w25q512jv-dtr for my patch [1] but saw "m" suffix used for other dtr parts. On related note, that array can be sorted by Device ID. [1] https://lore.kernel.org/u-boot/0c3e4727-0997-a4c5-dab1-b09ea8781...@linux.microsoft.com/ Dhananjay
Re: [PATCH] image: Control FIT signature verification at runtime
On 2/13/2022 5:13 PM, Andrew Jeffery wrote: Right, I think this question is an indication that I could write a more informative commit message, so if we converge on something acceptable I'll update it. Let me provide some more context: As mentioned above this is motivated by use with BMCs, specifically on the ASPEED AST2600, in some specific platform contexts. Platforms can be designed with two styles of firmware management in mind: 1. The typical approach - No owner control: Manufacturer supplied firmware with secure-boot always enabled 2. The atypical approach - Full owner control: Owner-controlled firmware with secure-boot optionally enabled For quite a few contributing to OpenBMC, the manufacturer and the owner are the same, and so the typical approach is a good match. It probably is the use case Dhananjay was considering[1]. It also caters to the traditionally closed-source firmware ecosystem where manufacturer control is retained. [1]https://lore.kernel.org/openbmc/016ae207-2ecb-1817-d10e-12819c8c4...@linux.microsoft.com/ The second approach requires open-source firmware stacks combined with platforms designed to enable owner control. There are some ecosystems that allow this (e.g. OpenPOWER). On the host side for those systems it's possible to secure-boot them using firmware and kernels signed entirely and only by user-controlled keys. We're looking to enable this for the BMC side too, as much as we can. For completeness (i.e. stating this to make the argument self-contained, not implying that you're unaware of it), for secure-boot to be meaningful at a given point in the boot process we need all previous elements of the boot process to have been verified. That is, it's not enough to verify u-boot if the u-boot SPL is not verified. Where such systems use the AST2600, limitations of the AST2600 secure-boot design come into play: 3. All secure-boot configuration is held in OTP memory integrated into the SoC 4. The OTP memory must be write-protected to prevent attackers installing arbitrary keys without physical presence 5. The OTP is write-protected by configuration held in the OTP. The consequence with respect to 2. is that the system manufacturer either must: 6. Program and write-protect the OTP during production, or 7. Ship the system with the OTP in a vulnerable state. We figure the latter isn't desirable which means dealing with limitations of the former. If the OTP is programmed (with the required public keys) and write-protected by the manufacturer, then when configured as a secure-boot root-of-trust, the AST2600 must only boot an SPL image signed by the manufacturer. From here there are two options for owner control: 8. The manufacturer signs arbitrary SPLs upon request. This requires trust from both parties and potentially a lot of auditing focus from the manufacturer. 9. The manufacturer publishes the source for the signed u-boot SPL binary in a fashion that allows reproducible builds for verification by inspection. Firmware signed by owner-controlled keys can only be applied beyond this boot stage. Despite the compromise, the latter approach seems to be the most reasonable in the circumstances. Again for completeness, broadly, security can be divided into two categories: 10. Software security 11. Physical security Controlling secure-boot in a way that requires physical presence rules out the ability to influence the boot process via (remote) software attacks. This is beneficial. The approach at the platform level does not yet solve for physical security, however given this is motivated by use on BMCs, physical security concerns could be viewed as taken care of in the sense that the systems are likely installed in a datacenter or some other controlled environment. We can decouple HW RoT and runtime control on enforcing secure boot (requiring one or keys) on FIT image. Conflating two raises lot of questions. There's not much case for disabling HW RoT, which implies the bootloader (U-Boot or more) has to be trusted after board is manufactured (OTPstraps, especially OTPCFG0[6], are programmed). There's indeed a case for disabling secure boot on OS FIT image - If bootloader is trusted, it's possible to remotely push the policy to disable runtime FIT image secure boot. Such policy push must use secure transport (someway authenticated) and storage (simplest U-Boot env). This is helpful in cases like booting diagnostic images if board has to be RMA'ed and diagnostic images won't be signed by production keys. There's a key-requirement policy already implemented [1]. [1] https://lore.kernel.org/u-boot/cover.1597643014.git.thir...@linux.microsoft.com/ Board code can patch "required-policy" = none at runtime based appropriate logic. Regards, Dhananjay With that in mind: To escape the manufacturer's key-chain for owner-controlled signatures the concept is the manufacturer-signed SPL (or u-boot payload) will load keys from an external, write-protected EEPROM. Th
Re: [PATCH] image: Control FIT signature verification at runtime
On 2/14/2022 3:13 PM, Patrick Williams wrote: On Mon, Feb 14, 2022 at 11:14:53AM -0800, Dhananjay Phadke wrote: On 2/13/2022 5:13 PM, Andrew Jeffery wrote: We can decouple HW RoT and runtime control on enforcing secure boot (requiring one or keys) on FIT image. Conflating two raises lot of questions. I won't claim to be a security expert but I don't understand this statement. What are the "lots of questions" that are raised? There's not much case for disabling HW RoT, which implies the bootloader (U-Boot or more) has to be trusted after board is manufactured (OTPstraps, especially OTPCFG0[6], are programmed). There's indeed a case for disabling secure boot on OS FIT image - Why wouldn't you want to replace the bootloader just as easily as you can replace the kernel / OS itself? I don't understand why this is more special than any other software. Bootloaders are replaced on "real" systems all the time. There are multiple efforts to be able to replace BIOS/UEFI with a free implementation as well. I would consider the "HW RoT" to be the software in ROMs and not anything which can be replaced, like u-boot. Why are you extending it to include u-boot? Agree that ROM maybe just immutable code + some logic, but almost surely lacks high level stack e.g. network stack to communicate securely and change boot behavior (remote unlock). Bootloader (U-Boot in Aspeed case) happens to be the first component with rich stack to enable such workflows without / before physical intervention. It also means, for defense against physical attacks (e.g. replace boot SPI), first mutable component (bootloader) must be trusted by immutable component earlier in boot chain. Further secure boot chain may have its own control knobs from compile or runtime. If bootloader is trusted, it's possible to remotely push the policy to disable runtime FIT image secure boot. Such policy push must use secure transport (someway authenticated) and storage (simplest U-Boot env). This is helpful in cases like booting diagnostic images if board has to be RMA'ed and diagnostic images won't be signed by production keys. For second-hand / recycled hardware, I'm not sure the bootloader _is_ trusted. It is also possible that I punt on some aspects of supply-chain security and simply replace it all when it arrives in my hands. ie. If I can securely replace all the bits, I really don't care if it was tampered with in transit. There's a key-requirement policy already implemented [1]. [1] https://lore.kernel.org/u-boot/cover.1597643014.git.thir...@linux.microsoft.com/ Board code can patch "required-policy" = none at runtime based appropriate logic. [...] Isn't this jumper proposal just like the TCG Physical Presence requirements? This is a software implementation and requires a particular hardware design for it to be done right, but it seems to be along the same lines. I'm supporting idea of having control on FIT verification, just pointed that it maybe done by board code by just patching U-Boot control FDT, either the "required-policy" property at /signature or "required" property in individual key nodes. Regards, Dhananjay
[PATCH] tpm: add TPM2_GetRandom command support
Add support for TPM2 GetRandom command Signed-off-by: Dhananjay Phadke --- include/tpm-v2.h | 13 + lib/tpm-v2.c | 44 2 files changed, 57 insertions(+) diff --git a/include/tpm-v2.h b/include/tpm-v2.h index ae00803f6d..513697e9a1 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -70,6 +70,7 @@ enum tpm2_handles { * @TPM2_CC_DAM_RESET: TPM2_DictionaryAttackLockReset(). * @TPM2_CC_DAM_PARAMETERS: TPM2_DictionaryAttackParameters(). * @TPM2_CC_GET_CAPABILITY: TPM2_GetCapibility(). + * @TPM2_CC_GET_RANDOM: TPM2_GetRandom(). * @TPM2_CC_PCR_READ: TPM2_PCR_Read(). * @TPM2_CC_PCR_EXTEND: TPM2_PCR_Extend(). * @TPM2_CC_PCR_SETAUTHVAL: TPM2_PCR_SetAuthValue(). @@ -85,6 +86,7 @@ enum tpm2_command_codes { TPM2_CC_DAM_PARAMETERS = 0x013A, TPM2_CC_NV_READ = 0x014E, TPM2_CC_GET_CAPABILITY = 0x017A, + TPM2_CC_GET_RANDOM = 0x017B, TPM2_CC_PCR_READ= 0x017E, TPM2_CC_PCR_EXTEND = 0x0182, TPM2_CC_PCR_SETAUTHVAL = 0x0183, @@ -308,4 +310,15 @@ u32 tpm2_pcr_setauthvalue(struct udevice *dev, const char *pw, const ssize_t pw_sz, u32 index, const char *key, const ssize_t key_sz); +/** + * Issue a TPM2_GetRandom command. + * + * @devTPM device + * @param data output buffer for the random bytes + * @param countsize of output buffer + * + * @return return code of the operation + */ +u32 tpm2_get_random(struct udevice *dev, void *data, u32 count); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index f89592d6e2..9d078877bc 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -421,3 +421,47 @@ u32 tpm2_pcr_setauthvalue(struct udevice *dev, const char *pw, return tpm_sendrecv_command(dev, command_v2, NULL, NULL); } + +u32 tpm2_get_random(struct udevice *dev, void *data, u32 count) +{ + const u8 command_v2[10] = { + tpm_u16(TPM2_ST_NO_SESSIONS), + tpm_u32(12), + tpm_u32(TPM2_CC_GET_RANDOM), + }; + u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE]; + + const size_t data_size_offset = 10; + const size_t data_offset = 12; + size_t response_length = sizeof(response); + u32 data_size; + u8 *out = data; + + while (count > 0) { + u32 this_bytes = min((size_t)count, +sizeof(response) - data_offset); + u32 err; + + if (pack_byte_string(buf, sizeof(buf), "sw", +0, command_v2, sizeof(command_v2), +sizeof(command_v2), this_bytes)) + return TPM_LIB_ERROR; + err = tpm_sendrecv_command(dev, buf, response, + &response_length); + if (err) + return err; + if (unpack_byte_string(response, response_length, "w", + data_size_offset, &data_size)) + return TPM_LIB_ERROR; + if (data_size > this_bytes) + return TPM_LIB_ERROR; + if (unpack_byte_string(response, response_length, "s", + data_offset, out, data_size)) + return TPM_LIB_ERROR; + + count -= data_size; + out += data_size; + } + + return 0; +} -- 2.17.1
Re: two questions on verified boot
On 11/21/2021 6:55 AM, Rasmus Villemoes wrote: > (2) Assuming for the moment that I would be happy with just using > required=image, am I right in that not only does that mean that the > combination of kernel/fdt/initramfs is not verified, merely the > individual parts, but more importantly (a mix'n'match attack isn't > really very likely), _only_ the data property in each node is part of > what gets signed, not the other important properties such as load= and > entry=? IOW, suppose I have a FIT image with > > and I know that the boot process uses $loadaddr = 0x4000. What is to > stop me from modifying that FIT image to read > > where 0xabcde is chosen to coincide with where the data part of the > pwned property lies in the modified FIT? (That pwned property can be put > anywhere; I could even just replace the signer-name property inside the > signature node with a value of "mkimage\0".) > > In fit_config_process_sig(), there's this elaborate dance with > fit_config_get_data()/fdt_find_regions() which, AFAICT, ends up > including all the property values (and the FDT_PROP tags and string > offsets etc.), and then we call info.crypto->sign() with some > appropriate region_count. But in fit_image_process_sig(), we call > info.crypto->sign() with nregions==1, and AFAICT, the data being signed > is just the value of the "data" property, nothing else. Couldn't agree more, I've been wondering on similar lines. It would be great to actually run digest over entire image (data + attributes) or config node (minus signature and hash subnodes if re-signing). It would have avoided CVE-2020-10648? Regards, Dhananjay
[PATCH] lib/crypto: support sha384/sha512 in x509/pkcs7
Set digest_size SHA384 and SHA512 algorithms in pkcs7 and x509, (not set by ported linux code, but needed by __UBOOT__ part). EFI_CAPSULE_AUTHENTICATE doesn't select these algos but required for correctness if certificates contain sha384WithRSAEncryption or sha512WithRSAEncryption OIDs. Signed-off-by: Dhananjay Phadke --- lib/crypto/pkcs7_verify.c| 4 lib/crypto/x509_public_key.c | 4 2 files changed, 8 insertions(+) diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c index 82c5c745d4..b832f01356 100644 --- a/lib/crypto/pkcs7_verify.c +++ b/lib/crypto/pkcs7_verify.c @@ -65,6 +65,10 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, return -ENOPKG; if (!strcmp(sinfo->sig->hash_algo, "sha256")) sig->digest_size = SHA256_SUM_LEN; + else if (!strcmp(sinfo->sig->hash_algo, "sha384")) + sig->digest_size = SHA384_SUM_LEN; + else if (!strcmp(sinfo->sig->hash_algo, "sha512")) + sig->digest_size = SHA512_SUM_LEN; else if (!strcmp(sinfo->sig->hash_algo, "sha1")) sig->digest_size = SHA1_SUM_LEN; else diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c index d557ab27ae..5c0e2b622d 100644 --- a/lib/crypto/x509_public_key.c +++ b/lib/crypto/x509_public_key.c @@ -71,6 +71,10 @@ int x509_get_sig_params(struct x509_certificate *cert) return -ENOPKG; if (!strcmp(sig->hash_algo, "sha256")) sig->digest_size = SHA256_SUM_LEN; + else if (!strcmp(sig->hash_algo, "sha384")) + sig->digest_size = SHA384_SUM_LEN; + else if (!strcmp(sig->hash_algo, "sha512")) + sig->digest_size = SHA512_SUM_LEN; else if (!strcmp(sig->hash_algo, "sha1")) sig->digest_size = SHA1_SUM_LEN; else -- 2.25.1
Re: [PATCH] lib/crypto: support sha384/sha512 in x509/pkcs7
On 3/18/2022 12:44 AM, Ilias Apalodimas wrote: +cc Akashi-san who initially ported those. On Tue, 15 Mar 2022 at 19:19, Dhananjay Phadke wrote: Set digest_size SHA384 and SHA512 algorithms in pkcs7 and x509, (not set by ported linux code, but needed by __UBOOT__ part). EFI_CAPSULE_AUTHENTICATE doesn't select these algos but required for correctness if certificates contain sha384WithRSAEncryption or sha512WithRSAEncryption OIDs. Does the rest of the code parse those? Or expects -ENOPKG for the unsupported certificates? Yes these OIDs are parsed by Linux code, see x509_note_pkey_algo(). U-Boot code allocates digest buf for invoking hash_calculate(), that needs this digest_size. I've verified such certs (chain) with pkcs7_verify_one(). Thanks, Dhananjay