[u-boot-gitdm PATCH 2/2] Add some more employers
This looks at the top unknowns: git log --pretty=%ae v2023.01.. |sed 's/.*@//' |sort |uniq -c | sort -nr | (while read count email; do if ! grep -q $email u-boot-config/domain-map; then echo "$count $email"; fi; done ) It reduces the (Unknown) count from a third to a fifth. Signed-off-by: Simon Glass --- u-boot-config/domain-map | 38 ++ 1 file changed, 38 insertions(+) diff --git a/u-boot-config/domain-map b/u-boot-config/domain-map index 63cb60f..27f1a7e 100644 --- a/u-boot-config/domain-map +++ b/u-boot-config/domain-map @@ -18,6 +18,7 @@ amd.com AMD anagramm.deAnagramm GmbH analog.com Analog Devices android.comGoogle LLC +andestech.com Andes Technology arastra.comArastra Inc areca.com.tw Areca argos-messtechnik.de Argos Meßtechnik GmbH @@ -44,6 +45,7 @@ bluewatersys.com Bluewater Systems boeing.com Boeing bootlin.comBootlin boundarydevices.comBoundary Devices +bp.renesas.com Renesas Electronics Corporation broadcom.com Broadcom brontes3d.com Brontes Technologies bt.com BT Group @@ -51,6 +53,7 @@ bull.net Bull SAS bus-elektronik.de BuS Elektronik calxeda.comCalxeda cam.ac.uk University of Cambridge +canonical.com Canonical Ltd. ccur.com Concurrent Computer Corporation cdi.cz CDI.CZ celestrius.com Celestrius @@ -58,6 +61,7 @@ celunite.com Azingo centtech.com Centaur Technology cesa.opbu.xerox.comXerox chelsio.comChelsio +chromium.org Google LLC cideas.com Custom IDEAS cisco.com Cisco citi.umich.edu Univ. of Michigan CITI @@ -78,6 +82,7 @@ cosmosbay.com Cosmosbay Vectis cozybit.comcozybit cray.com Cray cse-semaphore.com CSE Semaphore, Inc. +csgroup.eu CS Group csr.comCSR cyberguard.com Secure Computing cybernetics.comCybernetics @@ -89,11 +94,13 @@ dell.comDell denx.deDENX Software Engineering devicescape.comDevicescape digi.com Digi International +dimonoff.com Dimonoff inc. doredevelopment.dk Dore Development dti2.net DTI2 - Desarrollo de la tecnologia de las comunicaciones e-coninfotech.com e-con Infotech ecitele.comECI Telecom edesix.com Edesix Ltd +edgeble.ai Edgeble AI einstruction.com eInstruction eke.fi EKE-Electronics elandigitalsystems.com Elan Digital Systems @@ -119,10 +126,13 @@ feig.de Feig Electronic fixstars.com Fixstars Technologies free-electrons.com Bootlin freescale.com NXP +foss.st.comST Microelectronics +foundries.io Foundries.io fujitsu.comFujitsu gaisler.comGaisler Research ganssloser.com Ingenieurbuero Ganssloser garzik.org Red Hat +gateworks.com Gateworks Corporation gdsys.cc Guntermann & Drunck gdsys.de Guntermann & Drunck ge.com General Electric @@ -140,6 +150,7 @@ hevs.ch HES-SO Valais Wallis highpoint-tech.com HighPoint Technologies hitachi.co.jp Hitachi hitachi.comHitachi +hitachienergy.com Hitachi hitachisoft.jp Hitachi hp.com HP huawei.com Huawei Technologies @@ -164,10 +175,13 @@ jmicron.com jmicron.com jp.fujitsu.com Fujitsu juniper.netJuniper Networks katalix.comKatalix Systems +kernel-space.org kernelspace keymile.comKeymile keyspan.comInnoSys +kwiboo.se Kwiboo kiethp.com Intel konsulko.com Konsulko Group +kontron.de Kontron labxtechnologies.com Lab X Technologies laptop.org OLPC laurelnetworks.com ECI Telecom @@ -176,9 +190,11 @@ linaro.org Linaro linutronix.de linutronix linux-foundation.org Linux Foundation linuxant.com Linuxant +linux.microsoft.comMicrosoft Corporation linx.net London Internet Exchange lippert-at.de LiPPERT Embedded Computers GmbH lippertembedded.de LiPPERT Embedded Computers GmbH +lionizers.com Lionizers llnl.gov Lawrence Livermore National Laboratory lnxi.com Linux Networx logitech.com Logitech @@ -194,10 +210,14 @@ mandriva.com.br Mandriva marvell.comMarvell matrix-vision.de Matrix Vision mediamatech.comMediama Technologies +mediatek.com MediaTek Inc. mellanox.co.il
[PATCH] mmc: Poll CD in case cyclic framework is enabled
In case the cyclic framework is enabled, poll the card detect of already
initialized cards and deinitialize them in case they are removed. Since
the card initialization is a longer process and card initialization is
done on first access to an uninitialized card anyway, avoid initializing
newly detected uninitialized cards in the cyclic callback.
Signed-off-by: Marek Vasut
---
Cc: Jaehoon Chung
Cc: Peng Fan
---
drivers/mmc/mmc-uclass.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index cdead044177..c4c9881c40b 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -8,6 +8,7 @@
#define LOG_CATEGORY UCLASS_MMC
#include
+#include
#include
#include
#include
@@ -343,8 +344,23 @@ struct blk_desc *mmc_get_blk_desc(struct mmc *mmc)
return desc;
}
+static void mmc_cyclic_cd_poll(void *ctx)
+{
+ struct mmc *m = ctx;
+
+ if (!m->has_init)
+ return;
+
+ if (mmc_getcd(m))
+ return;
+
+ mmc_deinit(m);
+ m->has_init = 0;
+}
+
void mmc_do_preinit(void)
{
+ struct cyclic_info *cyclic;
struct udevice *dev;
struct uclass *uc;
int ret;
@@ -362,6 +378,17 @@ void mmc_do_preinit(void)
if (m->preinit)
mmc_start_init(m);
+
+ if (!CONFIG_IS_ENABLED(CYCLIC))
+ continue;
+
+ /* Register cyclic function for card detect polling */
+ cyclic = cyclic_register(mmc_cyclic_cd_poll, 100 * 1000,
+m->cfg->name, m);
+ if (cyclic)
+ continue;
+
+ printf("Failed to register %s CD poll function\n",
m->cfg->name);
}
}
--
2.42.0
[PATCH] mmc: Unconditionally call mmc_deinit()
Place the SDR104/HS200/HS400 checks into the mmc_deinit() and always
call it. This simplifies the code and removes ifdeffery. No functional
change is expected.
Signed-off-by: Marek Vasut
---
Cc: Jaehoon Chung
Cc: Peng Fan
---
drivers/mmc/mmc-uclass.c | 13 +
drivers/mmc/mmc.c| 9 +
2 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 328456831dd..cdead044177 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -494,10 +494,7 @@ static int mmc_blk_probe(struct udevice *dev)
if (ret) {
debug("Probing %s failed (err=%d)\n", dev->name, ret);
- if (CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) ||
- CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) ||
- CONFIG_IS_ENABLED(MMC_HS400_SUPPORT))
- mmc_deinit(mmc);
+ mmc_deinit(mmc);
return ret;
}
@@ -505,9 +502,6 @@ static int mmc_blk_probe(struct udevice *dev)
return 0;
}
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
static int mmc_blk_remove(struct udevice *dev)
{
struct udevice *mmc_dev = dev_get_parent(dev);
@@ -516,7 +510,6 @@ static int mmc_blk_remove(struct udevice *dev)
return mmc_deinit(mmc);
}
-#endif
static const struct blk_ops mmc_blk_ops = {
.read = mmc_bread,
@@ -532,12 +525,8 @@ U_BOOT_DRIVER(mmc_blk) = {
.id = UCLASS_BLK,
.ops= _blk_ops,
.probe = mmc_blk_probe,
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
.remove = mmc_blk_remove,
.flags = DM_FLAG_OS_PREPARE,
-#endif
};
#endif /* CONFIG_BLK */
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index d96db7a0f83..eb5010c1465 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -3010,13 +3010,15 @@ int mmc_init(struct mmc *mmc)
return err;
}
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
-CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
int mmc_deinit(struct mmc *mmc)
{
u32 caps_filtered;
+ if (!CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) &&
+ !CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) &&
+ !CONFIG_IS_ENABLED(MMC_HS400_SUPPORT))
+ return 0;
+
if (!mmc->has_init)
return 0;
@@ -3034,7 +3036,6 @@ int mmc_deinit(struct mmc *mmc)
return mmc_select_mode_and_width(mmc, caps_filtered);
}
}
-#endif
int mmc_set_dsr(struct mmc *mmc, u16 val)
{
--
2.42.0
Re: bootstd: Support for distro specific EFI folders
Hi Simon, On Sat, Dec 2, 2023 at 6:33 PM Simon Glass wrote: > > Hi, > > On Mon, 20 Nov 2023 at 00:02, Ilias Apalodimas > wrote: > > > > Hi Mark, > > > > On Sun, 19 Nov 2023 at 19:38, Mark Kettenis wrote: > > > > > > > Date: Sat, 18 Nov 2023 23:52:11 +0100 > > > > From: Heinrich Schuchardt > > > > > > Hi Heinrich, > > > > > > > On 11/18/23 22:28, Shantur Rathore wrote: > > > > > Hi Heinrich, > > > > > > > > > > On Fri, Nov 17, 2023 at 3:12 PM Heinrich Schuchardt > > > > > wrote: > > > > >> > > > > >> On 11/16/23 19:45, Shantur Rathore wrote: > > > > >>> On Thu, Nov 16, 2023 at 6:15 PM Heinrich Schuchardt > > > > >>> wrote: > > > > > > > > On 11/16/23 17:52, Shantur Rathore wrote: > > > > > Hi Simon, > > > > > > > > > > Currently bootstd - bootmethod_efi only looks for the default > > > > > bootxx64.efi in /EFI/boot folder only. > > > > > Generally many distros end up putting their bootloaders in > > > > > EFI/ folders like EFI/ubuntu and EFI/debian etc. > > > > > > > > > > In x86 worlds, the NVRAM is modified and new boot entries are > > > > > added to > > > > > support these but in the U-boot world the NVRAM variables are > > > > > read-only. > > > > > > > > I guess you are referring to UEFI boot options. These typically > > > > are not > > > > stored in non-volatile RAM but on a SPI flash device. > > > > > > > > >>> > > > > >>> Thanks for correcting me. > > > > >>> > > > > > > > > > > What would be the best way to implement this? > > > > > > > > > > I was thinking of having a "efi_prefixes" environment variable > > > > > which > > > > > can be set to "ubuntu debian centos" etc and bootmethod_efi can > > > > > try > > > > > all of them. Will bootmethod_efi be able to support multiple > > > > > entries ( > > > > > thinking of multiboot ) ? > > > > > > > > On my laptop I have: > > > > > > > > EFI/Microsoft/Boot/bootmgr.efi > > > > EFI/Microsoft/Boot/memtest.efi > > > > EFI/Boot/bootx64.efi > > > > EFI/Boot/fbx64.efi > > > > EFI/Boot/mmx64.efi > > > > EFI/debian/shimx64.efi > > > > EFI/debian/grubx64.efi > > > > EFI/debian/mmx64.efi > > > > EFI/debian/fbx64.efi > > > > EFI/ubuntu/grubx64.efi > > > > EFI/ubuntu/shimx64.efi > > > > EFI/ubuntu/mmx64.efi > > > > > > > > Obviously each installed operating system provides multiple EFI > > > > binaries > > > > and non uses the fallback file name BOOT.EFI. A value "ubuntu > > > > debian centos" would not be able to describe which file you are > > > > looking > > > > for. > > > > > > > > We already have the U-Boot command line eficonfig and efidebug > > > > commands > > > > to set up UEFI boot options which can describe which EFI binary to > > > > load > > > > and which command line to pass to it. These are considered by the > > > > existing boot flows. > > > > >>> > > > > >>> So, I am building a new RockPro64 based cluster and using Canonical > > > > >>> MAAS to set them up automatically, booting them up using DHCP and > > > > >>> installing them over the network. > > > > >>> I configured an Armbian image using Packer to be compatible with > > > > >>> MAAS > > > > >>> and it happily installs it. As part of installation process, a > > > > >>> grub-install is run which installs the grub efi, > > > > >>> this EFI ends up in EFI/debian instead of expected EFI/boot. > > > > >>> To be able to make it boot, I have to add commands to move it to > > > > >>> EFI/boot. I am trying to find a way in U-Boot that would allow me to > > > > >>> skip this step. > > > > >>> With eficonfig if I understand correctly, it would need manual > > > > >>> intervention to create boot entries. > > > > >>> > > > > > > > > If you are installing the shim-signed package on Ubuntu, the EFI > > > > boot > > > > option for Ubuntu is set up by EFI/BOOT/BOOT.EFI using the > > > > content > > > > of EFI/ubuntu/BOOT.CSV. This is done before > > > > ExitBootServices() and > > > > therefore should work with current U-Boot. > > > > > > > > Patches are pending upstream to make EFI variables writable from > > > > Linux > > > > if they are stored in the RPMB partition in the eMMC. See this > > > > series: > > > > > > > > https://lore.kernel.org/linux-efi/20231107054057.1893-2-masahisa.koj...@linaro.org/ > > > > > > > > >>> > > > > >>> Would it be possible to save it in SPI Flash as the U-Boot > > > > >>> environment ? > > > > >> > > > > >> Currently this is not supported by U-Boot. > > > > >> > > > > >> The U-Boot environment variables can be stored in lots of different > > > > >> places SPI flash is only one of these. But none of these locations is > > > > >> protected from OS access which would be preferable for UEFI variables > > > > >> for
Re: [PATCH v3 3/3] defconfig: rockpro64: Enable SF EFI var store
Hi Simon, On Fri, Dec 1, 2023 at 6:44 PM Simon Glass wrote: > > Hi Shantur, > > On Sun, 26 Nov 2023 at 15:09, Shantur Rathore wrote: > > > > RockPro64 uses SPI Flash for storing env, also use it store > > EFI variables. > > > > Signed-off-by: Shantur Rathore > > --- > > > > configs/rockpro64-rk3399_defconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/configs/rockpro64-rk3399_defconfig > > b/configs/rockpro64-rk3399_defconfig > > index 4cd6b76665..f550f2ae43 100644 > > --- a/configs/rockpro64-rk3399_defconfig > > +++ b/configs/rockpro64-rk3399_defconfig > > @@ -8,6 +8,8 @@ CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y > > CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30 > > CONFIG_ENV_SIZE=0x8000 > > CONFIG_ENV_OFFSET=0x3F8000 > > +CONFIG_EFI_VARIABLE_SF_STORE=y > > +CONFIG_EFI_VARIABLE_SF_OFFSET=0x7D > > Can we use this offset in binman when creating the SPI-flash image? Unless I missed something, binman is used when something is bundled with a u-boot image. This is similar to ENV_OFFSET letting u-boot know where it can write the EFI variables. I didn't see ENV_OFFSET referred in rk3399-u-boot.dtsi or rk3399-rockpro64-u-boot.dtsi Please correct me if I am wrong. Kind regards, Shantur
Re: efi: Set Variable Runtime implementation
Hi Simon, On Fri, Dec 1, 2023 at 6:44 PM Simon Glass wrote: > > Hi Shantur, > > On Mon, 27 Nov 2023 at 10:27, Shantur Rathore wrote: > > > > + Simon as he seems to have done a lot of work in the driver model. > > > > On Mon, Nov 27, 2023 at 10:12 AM Shantur Rathore wrote: > > > > > > Hi Ilias, > > > > > > On Mon, Nov 27, 2023 at 7:16 AM Ilias Apalodimas > > > wrote: > > > > > > > > Hi Shantur > > > > > > > > On Sun, 26 Nov 2023 at 12:33, Shantur Rathore wrote: > > > > > > > > > > Hi Peter, > > > > > > > > > > On Sat, Nov 25, 2023 at 6:19 AM Peter Robinson > > > > > wrote: > > > > > > > > > > > > Hi Shantur, > > > > > > > > > > > > On Fri, Nov 24, 2023 at 11:55 PM Shantur Rathore > > > > > > wrote: > > > > > > > > > > > > > > Hi Ilias, > > > > > > > > > > > > > > On Fri, Nov 24, 2023 at 10:50 PM Ilias Apalodimas > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Shantur > > > > > > > > > > > > > > > > On Fri, 24 Nov 2023 at 18:51, Shantur Rathore > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Heinrich, > > > > > > > > > > > > > > > > > > I am trying to work out how to enable the SetVariableRT > > > > > > > > > service in > > > > > > > > > U-Boot and came across your patch [1] which initially had the > > > > > > > > > SetVariable RT service enabled in EFI but in the final patch > > > > > > > > > this was > > > > > > > > > removed. > > > > > > > > > I am hoping to implement it on top of the SPI Flash EFI store > > > > > > > > > [2] to > > > > > > > > > be able to set Boot order and boot items from Linux the UEFI > > > > > > > > > way. > > > > > > > > > > > > > > > > > > Can I pick your brain on why it was dropped in the patch? > > > > > > > > > Is there any limitation in SetVariableRT service ? > > > > > > > > > > > > > > > > I recently had a talk about it in Plumbers [0]. Generally > > > > > > > > speaking, RT > > > > > > > > + hardware owned by the kernel is a very weird combination > > > > > > > > since you > > > > > > > > can't guarantee exclusive access to the flash or the bus and > > > > > > > > you have > > > > > > > > to preserve a *lot* of code alive in u-boot. > > > > > > > > > > > > > > > > I'll respond to your v1 patchset and we can discuss details > > > > > > > > there as well. > > > > > > > > > > > > > > > > [0] https://lpc.events/event/17/contributions/1653/ > > > > > > > > > > > > > > Thanks for the background and helping me understand the problem. > > > > > > > Makes me wonder how things work in the PC world. > > > > > > > U-boot being only ~1MB, can we not leave it all in memory and > > > > > > > maybe > > > > > > > just disable SPI access to Linux. > > > > > > > > That would work, but you cant guarantee Linux wont enable the SPI flash. > > > > > > > > > > > > > > > > That's basically it, on x86 there's specific HW that's owned by > > > > > > firmware, I don't know the exact low level details of how that > > > > > > works. > > > > > > > > > > > > I think x86 devices generally use eSPI for this HW [1] but I don't > > > > > > know the low level details. The Arm SBSA (Server HW spec) and SBBR > > > > > > (Server Base Boot Requirements) specs that are key to ServerReady > > > > > > may > > > > > > go into some details too if you're curious. > > > > > > > > On X86 the SPI flash is handled entirely by the firmware and SMM. You > > > > can find more details here [0] > > > > > > Thanks for more info. > > > > > > > > > > > > > > > > > Thanks, > > > > > I think the firmware is still accessible to PCs as one could update > > > > > the firmware > > > > > in Windows so Windows has access to that device. > > > > > > > > > > I had some try myself and found that setting a variable to memory > > > > > backed storage > > > > > is doable with SetVariable call but we want to store it in any > > > > > non-volatile storage > > > > > things really don't look good. > > > > > > > > > > To be able to write SetVariable to any device, the whole u-boot driver > > > > > model would need > > > > > to be kept in memory, might as well just keep the whole u-boot in > > > > > memory at this point, it's anyway small. > > > > > I don't have much knowledge on how to or pros and cons of doing this. > > > > > > > > The major problem here is who owns the hardware. With the SPI flash > > > > implementation as well as the RPMB implementation Linux owns that > > > > flash. > > > > For the RPMB we've introduced a mechanism so the kernel replaces the > > > > runtime calls with internal functions [1]. I think we should come up > > > > with a similar architecture for SPI. In any case we should keep in > > > > mind that setting authenticated EFI variables should be forbidden on > > > > the file/SPI backends since they are not really secure. > > > > > > > > > > Thanks, I understand now that we can't use SPI flash for saving secure > > > variables and stop Linux from accessing it. > > > My requirement is to be able to save non-Secure boot related variables > > > ( BootOrder, BootNext and
Re: [PATCH v3 2/3] efi_vars: Implement SPI Flash store
Hi Ilias,
On Thu, Nov 30, 2023 at 10:10 PM Ilias Apalodimas
wrote:
>
> Hi Shantur
>
> I have a few remarks on the architecture.
> Up to now, we are supporting
> 1. Variables on a file
> 2. Variables on an RPMB
>
> The reason those two are in different files is that we generally
> expect to use different bootime services and few differences in
> efi_variables_boot_exit_notify() and efi_init_variables().
> Whatever is common, for example the runtime functions are common
> across those two since they both implement variable runtime service
> via the memory backend, goes into efi_var_common.c, the memory backend
> operations should go in efi_var_mem.c.
>
> Since the SPI and file storage are similar -- and probably any storage
> medium controlled by the non-secure world, I am fine treating treat
> efi_variable.c as the variable management for the non-secure world and
> see if that needs changing in the future.
>
> As Heinrich pointed out the functions you are moving are better off
> moved into efi_var_mem.c.
>
> [...]
Happy to move them efi_var_mem.c
I will do that as part of v4
>
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index adc5ac6a80..f73fb40061 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -354,14 +354,16 @@ efi_status_t efi_set_variable_int(const u16
> > *variable_name,
> > ret = EFI_SUCCESS;
> >
> > /*
> > -* Write non-volatile EFI variables to file
> > +* Write non-volatile EFI variables to file or SPI Flash
> > * TODO: check if a value change has occured to avoid superfluous
> > writes
> > */
> > -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> > +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > efi_var_to_file();
> > - }
> > +#elif CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> > + efi_var_to_sf();
> > #endif
> > + }
> >
> > return EFI_SUCCESS;
> > }
> > @@ -471,9 +473,14 @@ efi_status_t efi_init_variables(void)
> >
> > #if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > ret = efi_var_from_file();
> > +#elif CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> > + ret = efi_var_from_sf();
> > +#else
> > + ret = EFI_SUCCESS;
> > +#endif
>
> Instead of those ifdefs can we do something different?
> Define a structure with two function callbacks for read/write(). Add
> the same function in both efi_var_file.c and efi_var_sp.c which fills
> in those callbacks and have an efi_init_variable() call to that
> function (obviously the SPI and file will be mutually exclusive)
>
Sure, will add to v4 too.
Thanks,
Shantur
Re: [PATCH v4 1/8] spl: Enforce framebuffer reservation from end of RAM
On Sat, 25 Nov 2023 at 09:27, Devarsh Thakkar wrote: > > Add an API which enforces framebuffer reservation from end of RAM. > This is done so that next stage can directly skip this region before > carrying out further reservations. > > Signed-off-by: Devarsh Thakkar > --- > V2: > No change. > > V3: > Change spl_reserve_video to spl_reserve_video_from_ram_top > which enforce framebuffer reservation from end of RAM. > > V4: > Split this to an independent patch with more details added > in comments for API in header file. > --- > common/spl/spl.c | 19 +++ > include/spl.h| 10 ++ Reviewed-by: Simon Glass
Re: [PATCH v2 1/2] common: board_f: change calculation of gd->mon_len to fix s5p4418 reloc
On Mon, 27 Nov 2023 at 11:28, Stefan Bosch wrote:
>
> ARM and MICROBLAZE: Change calculation of monitor length (gd->mon_len)
> to fix relocation at boards with s5p4418-SoC. At s5p4418, _start is
> after the header (NSIH) therefore the monitor length has to be
> calculated using __image_copy_start instead of _start in order the whole
> monitor code is relocated.
>
> Signed-off-by: Stefan Bosch
> ---
>
> Changes in v2:
> - Cosmetic: Fix spelling mistake in commit message
>
> common/board_f.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Simon Glass
> diff --git a/common/board_f.c b/common/board_f.c
> index d4d7d01f8f..d2e4d9eae2 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -283,7 +283,7 @@ static int init_func_i2c(void)
> static int setup_mon_len(void)
> {
> #if defined(__ARM__) || defined(__MICROBLAZE__)
> - gd->mon_len = (ulong)__bss_end - (ulong)_start;
> + gd->mon_len = (ulong)__bss_end - (ulong)__image_copy_start;
> #elif defined(CONFIG_SANDBOX) && !defined(__riscv)
> gd->mon_len = (ulong)_end - (ulong)_init;
> #elif defined(CONFIG_SANDBOX)
> --
> 2.17.1
>
Re: [PATCH v2 17/18] qemu-arm: Get bloblist from boot arguments
Hi Raymond,
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote:
>
> Add platform custom function to get bloblist from boot arguments.
This should be the same for all ARM platforms. The ultimate goal is
something like [1]
> Check whether boot arguments aligns with the register conventions
> defined in FW Handoff spec v0.9.
> Add bloblist related options into qemu default config.
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - New patch file created for v2.
>
> board/emulation/qemu-arm/Makefile| 1 +
> board/emulation/qemu-arm/lowlevel_init.S | 19 +
> board/emulation/qemu-arm/qemu-arm.c | 54
> configs/qemu_arm64_defconfig | 3 ++
> configs/qemu_arm_defconfig | 3 ++
> 5 files changed, 80 insertions(+)
> create mode 100644 board/emulation/qemu-arm/lowlevel_init.S
>
> diff --git a/board/emulation/qemu-arm/Makefile
> b/board/emulation/qemu-arm/Makefile
> index a22d1237ff..12821e7083 100644
> --- a/board/emulation/qemu-arm/Makefile
> +++ b/board/emulation/qemu-arm/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0+
>
> obj-y += qemu-arm.o
> +obj-$(CONFIG_OF_BOARD) += lowlevel_init.o
> diff --git a/board/emulation/qemu-arm/lowlevel_init.S
> b/board/emulation/qemu-arm/lowlevel_init.S
> new file mode 100644
> index 00..d72d7c938a
> --- /dev/null
> +++ b/board/emulation/qemu-arm/lowlevel_init.S
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include
> +
> +.global save_boot_params
> +save_boot_params:
> +#ifdef CONFIG_ARM64
> + adr x9, qemu_saved_args
> + stp x0, x1, [x9]
> + /* Increment the address by 16 bytes for the next pair of values */
> + stp x2, x3, [x9, #16]
> +#else
> + ldr r12, =qemu_saved_args
> + stm r12, {r0, r1, r2, r3}
> +#endif
> + b save_boot_params_ret
> diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> index 942f1fff57..a3892630d8 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -4,6 +4,9 @@
> */
>
> #include
> +#if IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST)
> +#include
> +#endif
> #include
> #include
> #include
> @@ -102,6 +105,16 @@ static struct mm_region qemu_arm64_mem_map[] = {
> struct mm_region *mem_map = qemu_arm64_mem_map;
> #endif
>
> +#if IS_ENABLED(CONFIG_OF_BOARD)
OF_BLOBLIST and please avoid #if
> +/* Boot parameters saved from lowlevel_init.S */
> +struct {
> + unsigned long arg0;
> + unsigned long arg1;
> + unsigned long arg2;
> + unsigned long arg3;
> +} qemu_saved_args __section(".data");
> +#endif
> +
> int board_init(void)
> {
> return 0;
> @@ -144,6 +157,47 @@ void *board_fdt_blob_setup(int *err)
> return (void *)CFG_SYS_SDRAM_BASE;
> }
>
> +int board_bloblist_from_boot_arg(unsigned long __maybe_unused addr,
> +unsigned long __maybe_unused size)
> +{
> + int ret = -ENOENT;
> +
> +#if IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST)
> + unsigned long fdt;
> +
> + ret = bloblist_check(qemu_saved_args.arg3, 0);
> + if (ret)
> + return ret;
> +
> + bloblist_show_stats();
> + bloblist_show_list();
> + if (gd->bloblist->total_size > size) {
> + gd->bloblist = NULL; /* Reset the gd bloblist pointer */
> + log_err("Bloblist total size:%d, board reserved size:%ld\n",
> + gd->bloblist->total_size, size);
> + return -ENOSPC;
> + }
> +
> + /* Check the register conventions */
> + fdt = (unsigned long)bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
This should happen in fdtdec.c automatically. See [2]
> + if (IS_ENABLED(CONFIG_ARM64)) {
> + if (fdt != qemu_saved_args.arg0 || qemu_saved_args.arg2 != 0)
> + ret = -EIO;
> + } else {
> + if (fdt != qemu_saved_args.arg2 || qemu_saved_args.arg0 != 0)
> + ret = -EIO;
> + }
> +
> + if (ret)
> + gd->bloblist = NULL; /* Reset the gd bloblist pointer */
> + else
> + memmove((void *)addr, (void *)qemu_saved_args.arg3,
> + gd->bloblist->total_size);
> +#endif
> +
> + return ret;
> +}
> +
> void enable_caches(void)
> {
> icache_enable();
> diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
> index 5fdf496a45..60fabb5db7 100644
> --- a/configs/qemu_arm64_defconfig
> +++ b/configs/qemu_arm64_defconfig
> @@ -71,3 +71,6 @@ CONFIG_USB_EHCI_HCD=y
> CONFIG_USB_EHCI_PCI=y
> CONFIG_SEMIHOSTING=y
> CONFIG_TPM=y
> +CONFIG_BLOBLIST=y
> +CONFIG_BLOBLIST_ADDR=0x40004000
> +CONFIG_BLOBLIST_SIZE=0x4000
> diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
> index
Re: [PATCH v2 16/18] bloblist: Align bloblist used_size and total_size to spec
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > Align used_size and total_size of the bloblist header to the definition > of the FW Handoff spec v0.9. > Update the related bloblist APIs and UT testcases. > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - New patch file created for v2. > > common/bloblist.c | 86 +++--- > include/bloblist.h | 26 -- > test/bloblist.c| 35 ++- > 3 files changed, 84 insertions(+), 63 deletions(-) Reviewed-by: Simon Glass
[PATCH 1/2] command: Allocate history buffer using calloc()
The history buffer is currently a static array which can be some
10-40 kiB depending on configuration, and so adds considerably to
the U-Boot binary size. Allocate it dynamically instead to reduce
the U-Boot binary size.
Signed-off-by: Marek Vasut
---
Cc: Simon Glass
---
common/cli_readline.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/common/cli_readline.c b/common/cli_readline.c
index 06b8d465044..85453beed76 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -85,7 +87,6 @@ static int hist_cur = -1;
static unsigned hist_num;
static char *hist_list[HIST_MAX];
-static char hist_lines[HIST_MAX][HIST_SIZE + 1]; /* Save room for NULL */
#define add_idx_minus_one() ((hist_add_idx == 0) ? hist_max : hist_add_idx-1)
@@ -97,8 +98,9 @@ static void getcmd_putchars(int count, int ch)
getcmd_putch(ch);
}
-static void hist_init(void)
+static int hist_init(void)
{
+ unsigned char *hist;
int i;
hist_max = 0;
@@ -106,10 +108,14 @@ static void hist_init(void)
hist_cur = -1;
hist_num = 0;
- for (i = 0; i < HIST_MAX; i++) {
- hist_list[i] = hist_lines[i];
- hist_list[i][0] = '\0';
- }
+ hist = calloc(HIST_MAX, HIST_SIZE + 1);
+ if (!hist)
+ return -ENOMEM;
+
+ for (i = 0; i < HIST_MAX; i++)
+ hist_list[i] = hist + (i * (HIST_SIZE + 1));
+
+ return 0;
}
static void cread_add_to_hist(char *line)
@@ -493,8 +499,9 @@ static int cread_line(const char *const prompt, char *buf,
unsigned int *len,
#else /* !CONFIG_CMDLINE_EDITING */
-static inline void hist_init(void)
+static inline int hist_init(void)
{
+ return 0;
}
static int cread_line(const char *const prompt, char *buf, unsigned int *len,
@@ -643,8 +650,9 @@ int cli_readline_into_buffer(const char *const prompt, char
*buffer,
*/
if (IS_ENABLED(CONFIG_CMDLINE_EDITING) && (gd->flags & GD_FLG_RELOC)) {
if (!initted) {
- hist_init();
- initted = 1;
+ rc = hist_init();
+ if (rc == 0)
+ initted = 1;
}
if (prompt)
--
2.42.0
[PATCH 2/2] command: Move command completion temporary buffer to stack
The command completion temporary buffer seems to be only
used by the argv tokenizer, move it to stack. This saves
2 kiB from the binary size (depends on configuration) per:
$ aarch64-linux-gnu-readelf -s u-boot | sort -n -k 3
Signed-off-by: Marek Vasut
---
Cc: Simon Glass
---
common/command.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c
index 846e16e2ada..7821c273dae 100644
--- a/common/command.c
+++ b/common/command.c
@@ -355,10 +355,9 @@ static int find_common_prefix(char *const argv[])
return len;
}
-static char tmp_buf[CONFIG_SYS_CBSIZE + 1];/* copy of console I/O buffer */
-
int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp)
{
+ char tmp_buf[CONFIG_SYS_CBSIZE + 1];/* copy of console I/O buffer */
int n = *np, col = *colp;
char *argv[CONFIG_SYS_MAXARGS + 1]; /* NULL terminated
*/
char *cmdv[20];
--
2.42.0
Re: [PATCH] doc: Replace examples of MD5 and SHA1 with SHA256
On 12/2/23 14:33, Sean Anderson wrote: Both SHA1 and (especially) MD5 are no longer as safe as they once were for cryptographic use. Replaces examples which use them with examples using SHA256 instead. This will provide more-secure defaults for users who use documentation examples as a base for their own use. This is not too necessary for non-verified-boot scenarios (since someone could just replace the checksum), but I wanted to be complete. Signed-off-by: Sean Anderson --- I forgot to mention this in the commit message, but all the new hashes were generated like echo fake kernel hash | sha256sum which should be fine, since the actual values were just for example anyway. --Sean
[PATCH] doc: Replace examples of MD5 and SHA1 with SHA256
Both SHA1 and (especially) MD5 are no longer as safe as they once were for
cryptographic use. Replaces examples which use them with examples using
SHA256 instead. This will provide more-secure defaults for users who use
documentation examples as a base for their own use. This is not too
necessary for non-verified-boot scenarios (since someone could just replace
the checksum), but I wanted to be complete.
Signed-off-by: Sean Anderson
---
doc/chromium/files/chromebook_jerry.its | 4 +-
doc/chromium/files/nyan-big.its | 4 +-
doc/usage/cmd/imxtract.rst | 6 +-
doc/usage/fit/beaglebone_vboot.rst | 74
doc/usage/fit/howto.rst | 40 ++---
doc/usage/fit/kernel.rst | 6 +-
doc/usage/fit/kernel_fdt.rst | 4 +-
doc/usage/fit/kernel_fdts_compressed.rst | 6 +-
doc/usage/fit/multi-with-fpga.rst| 6 +-
doc/usage/fit/multi-with-loadables.rst | 8 +--
doc/usage/fit/multi.rst | 12 ++--
doc/usage/fit/sign-configs.rst | 6 +-
doc/usage/fit/sign-images.rst| 4 +-
doc/usage/fit/signature.rst | 22 +++
doc/usage/fit/update3.rst| 6 +-
doc/usage/fit/update_uboot.rst | 2 +-
doc/usage/fit/x86-fit-boot.rst | 8 +--
17 files changed, 109 insertions(+), 109 deletions(-)
diff --git a/doc/chromium/files/chromebook_jerry.its
b/doc/chromium/files/chromebook_jerry.its
index 7505a20535b..02e5e1340f3 100644
--- a/doc/chromium/files/chromebook_jerry.its
+++ b/doc/chromium/files/chromebook_jerry.its
@@ -15,7 +15,7 @@
load = <0>;
entry = <0>;
hash-2 {
- algo = "sha1";
+ algo = "sha256";
};
};
@@ -26,7 +26,7 @@
arch = "arm";
compression = "none";
hash-1{
- algo = "sha1";
+ algo = "sha256";
};
};
};
diff --git a/doc/chromium/files/nyan-big.its b/doc/chromium/files/nyan-big.its
index bd412915e95..60bdffbb829 100644
--- a/doc/chromium/files/nyan-big.its
+++ b/doc/chromium/files/nyan-big.its
@@ -15,7 +15,7 @@
load = <0>;
entry = <0>;
hash-2 {
- algo = "sha1";
+ algo = "sha256";
};
};
@@ -26,7 +26,7 @@
arch = "arm";
compression = "none";
hash-1{
- algo = "sha1";
+ algo = "sha256";
};
};
};
diff --git a/doc/usage/cmd/imxtract.rst b/doc/usage/cmd/imxtract.rst
index eb64b1cefab..16be60b4aab 100644
--- a/doc/usage/cmd/imxtract.rst
+++ b/doc/usage/cmd/imxtract.rst
@@ -45,14 +45,14 @@ Examples
With verify=no incorrect hashes, signatures, or check sums don't stop the
extraction. But correct hashes are still indicated in the output
-(here: md5, sha1).
+(here: sha256, sha512).
.. code-block:: console
=> setenv verify no
=> imxtract $loadaddr kernel-1 $kernel_addr_r
## Copying 'kernel-1' subimage from FIT image at 4020 ...
-md5+ sha1+Loading part 0 ... OK
+sha256+ sha512+Loading part 0 ... OK
=>
With verify=yes incorrect hashes, signatures, or check sums stop the
extraction.
@@ -62,7 +62,7 @@ With verify=yes incorrect hashes, signatures, or check sums
stop the extraction.
=> setenv verify yes
=> imxtract $loadaddr kernel-1 $kernel_addr_r
## Copying 'kernel-1' subimage from FIT image at 4020 ...
-md5 error!
+sha256 error!
Bad hash value for 'hash-1' hash node in 'kernel-1' image node
Bad Data Hash
=>
diff --git a/doc/usage/fit/beaglebone_vboot.rst
b/doc/usage/fit/beaglebone_vboot.rst
index a102be187bd..cd6bb141910 100644
--- a/doc/usage/fit/beaglebone_vboot.rst
+++ b/doc/usage/fit/beaglebone_vboot.rst
@@ -145,7 +145,7 @@ Put this into a file in that directory called sign.its::
load = <0x80008000>;
entry = <0x80008000>;
hash-1 {
-algo = "sha1";
+algo = "sha256";
};
};
fdt-1 {
@@ -155,7 +155,7 @@ Put this into a file in that directory called sign.its::
arch = "arm";
compression = "none";
hash-1 {
-algo = "sha1";
+algo = "sha256";
};
};
};
@@ -165,7 +165,7 @@ Put this into a file in that directory called sign.its::
kernel = "kernel";
fdt = "fdt-1";
[PULL] u-boot-sh/master-rpc-off
The following changes since commit 43f2873fa98b1da6eb56d756315c7bd7db63db27: MAINTAINERS: Step up as co-maintainer of Tegra SOC platform (2023-11-28 11:23:02 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-sh.git master-rpc-off for you to fetch changes up to 13bdb6a269108d3f9b953bf4b73079d66ecc37af: ARM: dts: renesas: Disable RPC driver on R8A779G0 V4H White Hawk board (2023-12-02 17:16:01 +0100) Cong Dang (1): ARM: dts: renesas: Disable RPC driver on R8A779G0 V4H White Hawk board Marek Vasut (1): ARM: dts: renesas: Clean up R8A779G0 V4H RPC SPI DT node arch/arm/dts/r8a779g0-u-boot.dtsi | 18 -- arch/arm/dts/r8a779g0-white-hawk-u-boot.dts | 2 +- 2 files changed, 5 insertions(+), 15 deletions(-)
Re: [PATHv11 17/43] net: sandbox: fix NULL pointer derefences
On Sat, Dec 02, 2023 at 11:33:14AM -0700, Simon Glass wrote: > Hi Maxim, > > On Mon, 27 Nov 2023 at 11:20, Tom Rini wrote: > > > > On Mon, Nov 27, 2023 at 06:57:00PM +0600, Maxim Uvarov wrote: > > > Add additional checks for NULL pointers. > > > > > > Signed-off-by: Maxim Uvarov > > > --- > > > drivers/net/sandbox.c | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > > index 13022addb6..75d32db3a9 100644 > > > --- a/drivers/net/sandbox.c > > > +++ b/drivers/net/sandbox.c > > > @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, > > > void *packet, > > > struct ethernet_hdr *eth_recv; > > > struct arp_hdr *arp_recv; > > > > > > + if (!priv) > > > + return -EAGAIN; > > > + > > > if (ntohs(eth->et_protlen) != PROT_ARP) > > > return -EAGAIN; > > > > This part seems fine. > > > > > @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, > > > void *packet, > > > > > > /* Formulate a fake response */ > > > eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; > > > + if (!eth_recv) > > > + return -EAGAIN; > > > memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); > > > memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); > > > eth_recv->et_protlen = htons(PROT_ARP); > > > > How do we get to this dereference, and is that not a bug in the caller? > > I wonder if somehow the device has not been probed yet? Given the failures on a number of real hardware platforms in v11 as well (which I didn't see until after my review), I wonder if you've not spotted the cause of all of those other failures? -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
On Sat, Dec 02, 2023 at 11:27:15AM -0700, Simon Glass wrote:
> Hi,
>
> On Tue, 21 Nov 2023 at 13:49, Tom Rini wrote:
> >
> > On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote:
> > > Hi Simon
> > >
> > > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote:
> > > >
> > > > Hi Heinrich,
> > > >
> > > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt
> > > > wrote:
> > > > >
> > > > > On 10/15/23 04:45, Simon Glass wrote:
> > > > > > When the SMBIOS table is written to an address above 4GB a 32-bit
> > > > > > table
> > > > > > address is not large enough.
> > > > > >
> > > > > > Use an SMBIOS3 table in that case.
> > > > > >
> > > > > > Note that we cannot use efi_allocate_pages() since this function has
> > > > > > nothing to do with EFI. There is no equivalent function to allocate
> > > > > > memory below 4GB in U-Boot. One solution would be to create a
> > > > > > separate
> > > > > > malloc() pool, or just always put the malloc() pool below 4GB.
> > > > > >
> > > > > > Signed-off-by: Simon Glass
> > > > > > ---
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Check the end of the table rather than the start.
> > > > > >
> > > > > > include/smbios.h | 22 +-
> > > > > > lib/smbios.c | 24 +++-
> > > > > > 2 files changed, 40 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/include/smbios.h b/include/smbios.h
> > > > > > index c9df2706f5a6..ddabb558299e 100644
> > > > > > --- a/include/smbios.h
> > > > > > +++ b/include/smbios.h
> > > > > > @@ -12,7 +12,8 @@
> > > > > >
> > > > > > /* SMBIOS spec version implemented */
> > > > > > #define SMBIOS_MAJOR_VER3
> > > > > > -#define SMBIOS_MINOR_VER 0
> > > > > > +#define SMBIOS_MINOR_VER 7
> > > > > > +
> > > > > >
> > > > > > enum {
> > > > > > SMBIOS_STR_MAX = 64, /* Maximum length allowed for a
> > > > > > string */
> > > > > > @@ -54,6 +55,25 @@ struct __packed smbios_entry {
> > > > > > u8 bcd_rev;
> > > > > > };
> > > > > >
> > > > > > +struct __packed smbios3_entry {
> > > > > > + u8 anchor[5];
> > > > > > + u8 checksum;
> > > > > > + u8 length;
> > > > > > + u8 major_ver;
> > > > > > +
> > > > > > + u8 minor_ver;
> > > > > > + u8 docrev;
> > > > > > + u8 entry_point_rev;
> > > > > > + u8 reserved;
> > > > > > + u32 max_struct_size;
> > > > > > +
> > > > > > + u64 struct_table_address;
> > > > > > +};
> > > > > > +
> > > > > > +/* These two structures should use the same amount of
> > > > > > 16-byte-aligned space */
> > > > > > +static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
> > > > > > + ALIGN(16, sizeof(struct smbios3_entry)));
> > > > > > +
> > > > > > /* BIOS characteristics */
> > > > > > #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
> > > > > > #define BIOS_CHARACTERISTICS_UPGRADEABLE(1 << 11)
> > > > > > diff --git a/lib/smbios.c b/lib/smbios.c
> > > > > > index c7a557bc9b7b..92e98388084f 100644
> > > > > > --- a/lib/smbios.c
> > > > > > +++ b/lib/smbios.c
> > > > > > @@ -487,7 +487,11 @@ ulong write_smbios_table(ulong addr)
> > > > > > addr = ALIGN(addr, 16);
> > > > > > start_addr = addr;
> > > > > >
> > > > > > - addr += sizeof(struct smbios_entry);
> > > > > > + /*
> > > > > > + * So far we don't know which struct will be used, but they
> > > > > > both end
> > > > > > + * up using the same amount of 16-bit-aligned space
> > > > > > + */
> > > > > > + addr += max(sizeof(struct smbios_entry), sizeof(struct
> > > > > > smbios3_entry));
> > > > > > addr = ALIGN(addr, 16);
> > > > > > tables = addr;
> > > > > >
> > > > > > @@ -512,14 +516,24 @@ ulong write_smbios_table(ulong addr)
> > > > > >* sandbox's DRAM buffer.
> > > > > >*/
> > > > > > table_addr = (ulong)map_sysmem(tables, 0);
> > > > > > - if (sizeof(table_addr) > sizeof(u32) && table_addr >
> > > > > > (ulong)UINT_MAX) {
> > > > > > + if (sizeof(table_addr) > sizeof(u32) && addr >=
> > > > > > (ulong)UINT_MAX) {
> > > > >
> > > > > You have to check the end of the the last SMBIOS structure not the
> > > > > start
> > > > > of the first SMBIOS structure against UINT_MAX.
> > > > >
> > > > > > + struct smbios3_entry *se;
> > > > > > /*
> > > > > >* We need to put this >32-bit pointer into the table
> > > > > > but the
> > > > > >* field is only 32 bits wide.
> > > > > >*/
> > > > > > - printf("WARNING: SMBIOS table_address overflow
> > > > > > %llx\n",
> > > > > > -(unsigned long long)table_addr);
> > > > > > - addr = 0;
> > > > > > + printf("WARNING: Using SMBIOS3.0 due to table-address
> > > > > > overflow %lx\n",
> > > > > > +table_addr);
> > > > >
> > > > > This should be log_debug().
> > > > >
> > > > > > + se = map_sysmem(start_addr,
Re: [PATCH v2 01/18] bloblist: Update the tag numbering
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > Align bloblist tags with the FW handoff spec v0.9. > The most common ones are from 0. > TF related ones are from 0x100. > All non-standard ones from 0xfff000. > > Signed-off-by: Simon Glass > Co-developed-by: Raymond Mao > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Align bloblist tags to FW handoff spec v0.9. > > common/bloblist.c | 16 +--- > include/bloblist.h | 65 -- > test/bloblist.c| 4 +-- > 3 files changed, 48 insertions(+), 37 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 03/18] bloblist: Change the magic value
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote:
>
> From: Simon Glass
>
> This uses a new value with spec v0.9 so change it.
>
> Signed-off-by: Simon Glass
> Co-developed-by: Raymond Mao
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - Update the bloblist alignment to align to FW handoff spec v0.9.
>
> include/bloblist.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
But you will need a later patch (or update this one) due to [1]
>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 1e4f1a6342..30441f922b 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -75,7 +75,7 @@
>
> enum {
> BLOBLIST_VERSION= 0,
> - BLOBLIST_MAGIC = 0xb00757a3,
> + BLOBLIST_MAGIC = 0x6ed0ff,
>
> BLOBLIST_ALIGN_LOG2 = 3,
> BLOBLIST_ALIGN = 1 << BLOBLIST_ALIGN_LOG2,
> --
> 2.25.1
>
Regards,
Simon
[1] https://github.com/FirmwareHandoff/firmware_handoff/pull/24
Re: [PATCH v2 02/18] bloblist: Adjust API to align in powers of 2
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > The updated bloblist structure stores the alignment as a power-of-two > value in its structures. Adjust the API to use this, to avoid needing to > calling ilog2(). > > Drop a stale comment while we are here. > > Signed-off-by: Simon Glass > Co-developed-by: Raymond Mao > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Update the bloblist alignment to align to FW handoff spec v0.9. > > arch/x86/lib/tables.c | 3 ++- > common/bloblist.c | 24 +++- > include/bloblist.h| 12 +++- > test/bloblist.c | 4 ++-- > 4 files changed, 22 insertions(+), 21 deletions(-) > Reviewed-by: Simon Glass
Re: bootstd: Support for distro specific EFI folders
Hi, On Mon, 20 Nov 2023 at 00:02, Ilias Apalodimas wrote: > > Hi Mark, > > On Sun, 19 Nov 2023 at 19:38, Mark Kettenis wrote: > > > > > Date: Sat, 18 Nov 2023 23:52:11 +0100 > > > From: Heinrich Schuchardt > > > > Hi Heinrich, > > > > > On 11/18/23 22:28, Shantur Rathore wrote: > > > > Hi Heinrich, > > > > > > > > On Fri, Nov 17, 2023 at 3:12 PM Heinrich Schuchardt > > > > wrote: > > > >> > > > >> On 11/16/23 19:45, Shantur Rathore wrote: > > > >>> On Thu, Nov 16, 2023 at 6:15 PM Heinrich Schuchardt > > > >>> wrote: > > > > > > On 11/16/23 17:52, Shantur Rathore wrote: > > > > Hi Simon, > > > > > > > > Currently bootstd - bootmethod_efi only looks for the default > > > > bootxx64.efi in /EFI/boot folder only. > > > > Generally many distros end up putting their bootloaders in > > > > EFI/ folders like EFI/ubuntu and EFI/debian etc. > > > > > > > > In x86 worlds, the NVRAM is modified and new boot entries are added > > > > to > > > > support these but in the U-boot world the NVRAM variables are > > > > read-only. > > > > > > I guess you are referring to UEFI boot options. These typically are > > > not > > > stored in non-volatile RAM but on a SPI flash device. > > > > > > >>> > > > >>> Thanks for correcting me. > > > >>> > > > > > > > > What would be the best way to implement this? > > > > > > > > I was thinking of having a "efi_prefixes" environment variable which > > > > can be set to "ubuntu debian centos" etc and bootmethod_efi can try > > > > all of them. Will bootmethod_efi be able to support multiple > > > > entries ( > > > > thinking of multiboot ) ? > > > > > > On my laptop I have: > > > > > > EFI/Microsoft/Boot/bootmgr.efi > > > EFI/Microsoft/Boot/memtest.efi > > > EFI/Boot/bootx64.efi > > > EFI/Boot/fbx64.efi > > > EFI/Boot/mmx64.efi > > > EFI/debian/shimx64.efi > > > EFI/debian/grubx64.efi > > > EFI/debian/mmx64.efi > > > EFI/debian/fbx64.efi > > > EFI/ubuntu/grubx64.efi > > > EFI/ubuntu/shimx64.efi > > > EFI/ubuntu/mmx64.efi > > > > > > Obviously each installed operating system provides multiple EFI > > > binaries > > > and non uses the fallback file name BOOT.EFI. A value "ubuntu > > > debian centos" would not be able to describe which file you are > > > looking > > > for. > > > > > > We already have the U-Boot command line eficonfig and efidebug > > > commands > > > to set up UEFI boot options which can describe which EFI binary to > > > load > > > and which command line to pass to it. These are considered by the > > > existing boot flows. > > > >>> > > > >>> So, I am building a new RockPro64 based cluster and using Canonical > > > >>> MAAS to set them up automatically, booting them up using DHCP and > > > >>> installing them over the network. > > > >>> I configured an Armbian image using Packer to be compatible with MAAS > > > >>> and it happily installs it. As part of installation process, a > > > >>> grub-install is run which installs the grub efi, > > > >>> this EFI ends up in EFI/debian instead of expected EFI/boot. > > > >>> To be able to make it boot, I have to add commands to move it to > > > >>> EFI/boot. I am trying to find a way in U-Boot that would allow me to > > > >>> skip this step. > > > >>> With eficonfig if I understand correctly, it would need manual > > > >>> intervention to create boot entries. > > > >>> > > > > > > If you are installing the shim-signed package on Ubuntu, the EFI boot > > > option for Ubuntu is set up by EFI/BOOT/BOOT.EFI using the > > > content > > > of EFI/ubuntu/BOOT.CSV. This is done before ExitBootServices() > > > and > > > therefore should work with current U-Boot. > > > > > > Patches are pending upstream to make EFI variables writable from > > > Linux > > > if they are stored in the RPMB partition in the eMMC. See this > > > series: > > > > > > https://lore.kernel.org/linux-efi/20231107054057.1893-2-masahisa.koj...@linaro.org/ > > > > > > >>> > > > >>> Would it be possible to save it in SPI Flash as the U-Boot > > > >>> environment ? > > > >> > > > >> Currently this is not supported by U-Boot. > > > >> > > > >> The U-Boot environment variables can be stored in lots of different > > > >> places SPI flash is only one of these. But none of these locations is > > > >> protected from OS access which would be preferable for UEFI variables > > > >> for security reasons. > > > >> > > > >> To support boards without eMMC the right way forward would be writing a > > > >> new implementation of the OP-TEE standalone MM which writes the > > > >> variables to SPI flash instead of the RPMB partition and ensures that > > > >> the SPI flash' MMIO registers are protected against access from the > > > >> non-secure world. > >
Re: [PATHv11 17/43] net: sandbox: fix NULL pointer derefences
Hi Maxim, On Mon, 27 Nov 2023 at 11:20, Tom Rini wrote: > > On Mon, Nov 27, 2023 at 06:57:00PM +0600, Maxim Uvarov wrote: > > Add additional checks for NULL pointers. > > > > Signed-off-by: Maxim Uvarov > > --- > > drivers/net/sandbox.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > index 13022addb6..75d32db3a9 100644 > > --- a/drivers/net/sandbox.c > > +++ b/drivers/net/sandbox.c > > @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, > > void *packet, > > struct ethernet_hdr *eth_recv; > > struct arp_hdr *arp_recv; > > > > + if (!priv) > > + return -EAGAIN; > > + > > if (ntohs(eth->et_protlen) != PROT_ARP) > > return -EAGAIN; > > This part seems fine. > > > @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, > > void *packet, > > > > /* Formulate a fake response */ > > eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; > > + if (!eth_recv) > > + return -EAGAIN; > > memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); > > memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); > > eth_recv->et_protlen = htons(PROT_ARP); > > How do we get to this dereference, and is that not a bug in the caller? I wonder if somehow the device has not been probed yet? Regards, Simon
Re: [PATCH 6/7] WIP: efi: Allow helloworld to exit boot services
Hi Ilias, On Wed, 22 Nov 2023 at 00:39, Ilias Apalodimas wrote: > > Hi Simon, > > On Wed, 22 Nov 2023 at 00:10, Simon Glass wrote: > > > > Hi Heinrich, > > > > On Tue, 21 Nov 2023 at 10:17, Heinrich Schuchardt > > wrote: > > > > > > On 11/21/23 12:35, Simon Glass wrote: > > > > This allows testing of the exit_boot_services call, providing more > > > > coverage of the EFI bootmeth. > > > > > > > > Signed-off-by: Simon Glass > > > > > > I would prefer to keep helloworld.efi benign. > > > > > > Please, create a new EFI binary for testing ExitBootServices(). > > > > Eek...isn't helloworld already used in tets?\ > > Yes, but calling exitboot services from it means we need to be really > careful when/how we call it, since once that app runs the boottime > services along with any attached disks will disappear. > Instead having a single EFI app that calls that and then tests for any > runtime services is more scalable OK I can add a new app. I am still stuck on the actual bug mentioned in the cover letter, though. Regards, Simon
Re: [PATCH v2 18/18] bloblist: Load the bloblist from the previous loader
Hi Raymond,
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote:
>
> During bloblist initialization, when CONFIG_OF_BOARD is defined,
> invoke the platform custom function to load the bloblist via boot
> arguments from the previous loader.
> If the bloblist exists, copy it into the fixed bloblist memory region.
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - New patch file created for v2.
>
> common/bloblist.c | 29 +
> include/bloblist.h | 14 ++
> 2 files changed, 27 insertions(+), 16 deletions(-)
>
This should already work so I am hoping that this patch is not
needed...these changes seem to break sandbox_spl
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 91d69e9439..251093ecfa 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -477,15 +477,8 @@ int bloblist_init(void)
> bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
> int ret = -ENOENT;
> ulong addr, size;
> - bool expected;
> -
> - /**
> -* We don't expect to find an existing bloblist in the first phase of
> -* U-Boot that runs. Also we have no way to receive the address of an
> -* allocated bloblist from a previous stage, so it must be at a fixed
> -* address.
> -*/
> - expected = fixed && !u_boot_first_phase();
> + bool expected = fixed;
> +
> if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> expected = false;
> if (fixed)
> @@ -493,14 +486,17 @@ int bloblist_init(void)
> CONFIG_BLOBLIST_ADDR);
> size = CONFIG_BLOBLIST_SIZE;
> if (expected) {
> - ret = bloblist_check(addr, size);
> - if (ret) {
> - log_warning("Expected bloblist at %lx not found
> (err=%d)\n",
> + if (IS_ENABLED(CONFIG_OF_BOARD))
> + /* Get the bloblist from previous loader */
> + ret = board_bloblist_from_boot_arg(addr, size);
> + else
> + ret = bloblist_check(addr, size);
> +
> + if (ret)
> + log_warning("Bloblist at %lx not found, err=%d\n",
> addr, ret);
> - } else {
> - /* Get the real size, if it is not what we expected */
> + else
> size = gd->bloblist->total_size;
> - }
> }
> if (ret) {
> if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
> @@ -510,7 +506,8 @@ int bloblist_init(void)
> return log_msg_ret("alloc", -ENOMEM);
> addr = map_to_sysmem(ptr);
> } else if (!fixed) {
> - return log_msg_ret("!fixed", ret);
> + return log_msg_ret("BLOBLIST_FIXED is not enabled",
> + ret);
> }
> log_debug("Creating new bloblist size %lx at %lx\n", size,
> addr);
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 0f5afec9f4..e65cd27b90 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -438,6 +438,20 @@ void bloblist_reloc(void *to, uint to_size, void *from,
> uint from_size);
> */
> int bloblist_init(void);
>
> +#if CONFIG_IS_ENABLED(ARCH_QEMU)
> +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size);
> +#else
> +/*
> + * A board need to implement this custom function if it needs to retrieve
> + * bloblist from a previous loader
> + */
> +static inline int board_bloblist_from_boot_arg(unsigned long __always_unused
> addr,
> + unsigned long __always_unused
> size)
> +{
> + return -1;
> +}
> +#endif
> +
> #if CONFIG_IS_ENABLED(BLOBLIST)
> /**
> * bloblist_maybe_init() - Init the bloblist system if not already done
> --
> 2.25.1
>
Regards,
Simon
Re: [PATCH v2 12/18] bloblist: Reduce bloblist header size
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > From: Simon Glass > > The v0.9 spec provides for a 16-byte header with fewer fields. Update > the implementation to match this. > > This also adds an alignment field. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > include/bloblist.h | 26 +- > test/bloblist.c| 6 +++--- > 2 files changed, 16 insertions(+), 16 deletions(-) Reviewed-by: Simon Glass Sadly this will need a later patch to deal with [1] [1] https://github.com/FirmwareHandoff/firmware_handoff/pull/17/commits/331822f1985d70a7903f422789eab03f24355cfb
Re: [PATCH v2 11/18] bloblist: Reduce blob-header size
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > From: Simon Glass > > The v0.9 spec provides for an 8-byte header for each blob, with fewer > fields. > The blob start address should be aligned to the alignment specified > by the bloblist header. > Update the implementation to match this. > > Signed-off-by: Simon Glass > Co-developed-by: Raymond Mao > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Update the blob start address to align to the alignment required by > the bloblist header. > - Define the macros of bloblist header size and bloblist record header > size as the size of their structures. > > common/bloblist.c | 17 + > include/bloblist.h | 33 ++--- > test/bloblist.c| 16 > 3 files changed, 39 insertions(+), 27 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 13/18] bloblist: Add alignment to bloblist_new()
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > From: Simon Glass > > Allow the alignment to be specified when creating a bloblist. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 5 +++-- > include/bloblist.h | 3 ++- > test/bloblist.c| 40 ++-- > 3 files changed, 27 insertions(+), 21 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 14/18] bloblist: Update documentation and header comment
On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > From: Simon Glass > > Align the documentation with the v0.9 spec. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > doc/develop/bloblist.rst | 4 +++- > include/bloblist.h | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 15/18] fdt: Allow the devicetree to come from a bloblist
Hi Raymond, On Mon, 27 Nov 2023 at 12:53, Raymond Mao wrote: > > From: Simon Glass > > Standard passage provides for a bloblist to be passed from one firmware > phase to the next. That can be used to pass the devicetree along as well. > If CONFIG_OF_BOARD is defined, a board custom routine will provide a > bloblist or a specified memory address to retrieve the devicetree at > runtime. > A devicetree from a bloblist is prioritized than the one from specified > memory region. > > Tests for this will be added as part of the Universal Payload work. > > Signed-off-by: Simon Glass > Co-developed-by: Raymond Mao > Signed-off-by: Raymond Mao > --- > Changes in v2 > - New patch file created for v2. > Amended from the original patch > "[v2,30/32] fdt: Allow the devicetree to come from a bloblist". > Remove CONFIG_OF_BLOBLIST and FDTSRC_BLOBLIST, a DTB from a previous > loader is defined by CONFIG_OF_BOARD. The DTB can be located either in the > bloblist or from a specified memory address. > > doc/develop/devicetree/control.rst | 8 +++-- > dts/Kconfig| 9 -- > include/fdtdec.h | 3 +- > lib/fdtdec.c | 52 +++--- > 4 files changed, 53 insertions(+), 19 deletions(-) This is a bit mangled. I spoke to Ilias about this as osfc and thought we had it figured out, but it was a bit rushed so perhaps not. OF_BOARD refers to a board-specific mechanism We should have OF_BLOBLIST to reference to this, a standard mechanism and not board-specific. Ideally all boards should use a bloblist to send their DT to U-Boot. E.g. Raspberry Pi. So I believe my original patch was closer to what we want. Regards, SImon
Re: [PATCH v2 00/18] Support Firmware Handoff spec via bloblist
Hi Raymond,
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote:
>
> Major changes:
>
> Update bloblist to align to Firmware Handoff spec v0.9
> (https://github.com/FirmwareHandoff/firmware_handoff).
>
> Implement Qemu-Arm platform custom functions to retrieve the bloblist
> (aka. Transfer List) from previous loader via boot arguments when
> CONFIG_OF_BOARD option is enabled.
>
> If a board provides both custom functions for getting the bloblist and
> the address of external FDT, the FDT inside the bloblist will be prioritized
> during FDT setup.
>
> The patch set is tested with previous done TF-A/OP-TEE patches through a
> complete Firmware Handoff cycle (BL2, BL31, BL32, BL33).
> TF-A Patches:
> feat(handoff): introduce firmware handoff library
> (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22215)
> feat(qemu): implement firmware handoff on qemu
> (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22178)
> feat(handoff): enhance transfer list library
> (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23776)
> feat(optee): enable transfer list in opteed
> (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23777)
> feat(qemu): enable transfer list to BL31/32
> (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23778)
> OP-TEE Patch:
> Firmware handoff
> (https://github.com/OP-TEE/optee_os/pull/6308)
> fixup of transfer list entry overriding
> (https://github.com/OP-TEE/optee_os/pull/6461)
>
> Raymond Mao (3):
> bloblist: Align bloblist used_size and total_size to spec
> qemu-arm: Get bloblist from boot arguments
> bloblist: Load the bloblist from the previous loader
>
> Simon Glass (15):
> bloblist: Update the tag numbering
> bloblist: Adjust API to align in powers of 2
> bloblist: Change the magic value
> bloblist: Set version to 1
> bloblist: Access record hdr_size and tag via a function
> bloblist: Drop the flags value
> bloblist: Drop the spare values
> bloblist: Change the checksum algorithm
> bloblist: Checksum the entire bloblist
> bloblist: Handle alignment with a void entry
> bloblist: Reduce blob-header size
> bloblist: Reduce bloblist header size
> bloblist: Add alignment to bloblist_new()
> bloblist: Update documentation and header comment
> fdt: Allow the devicetree to come from a bloblist
>
> arch/x86/lib/tables.c| 3 +-
> board/emulation/qemu-arm/Makefile| 1 +
> board/emulation/qemu-arm/lowlevel_init.S | 19 ++
> board/emulation/qemu-arm/qemu-arm.c | 54 ++
> common/bloblist.c| 225 +--
> configs/qemu_arm64_defconfig | 3 +
> configs/qemu_arm_defconfig | 3 +
> doc/develop/bloblist.rst | 4 +-
> doc/develop/devicetree/control.rst | 8 +-
> dts/Kconfig | 9 +-
> include/bloblist.h | 181 ++
> include/fdtdec.h | 3 +-
> lib/fdtdec.c | 52 --
> test/bloblist.c | 86 -
> 14 files changed, 420 insertions(+), 231 deletions(-)
> create mode 100644 board/emulation/qemu-arm/lowlevel_init.S
>
> --
> 2.25.1
>
Thank you for taking this on!
It is great to have a qemu test, but please do hook it into test/py so
that it runs in CI.
We should also have a sandbox test. With this series this fails:
/tmp/b/sandbox_spl/spl/u-boot-spl -d /tmp/b/sandbox_spl/u-boot.dtb -m ram.bin
A simple C test could be something that checks that the bloblist is
present, e.g. in test/bloblist.c :
static int bloblist_test_from_spl(struct unit_test_state *uts)
{
if (!IS_ENABLED(SPL))
return -EAGAIN;
ut_assertnonnull(gd->bloblist);
return 0;
}
Also you could add jwerner to this as he reviewed v1
Here is my cover letter in case it helps (at u-boot-dm/blob-working):
Series-to: u-boot
Series-cc: trini, Jose Marinho
Series-cc: Julius Werner , ilias,
Series-cc: Dan Handley
Cover-letter:
bloblist: Align to firmware handoff
In moving from v0.8 to v0.9 the Firmware Handoff specification made some
changes, including:
- Use a packed format for headers to reduce space
- Add an explicit alignment field in the header
- Renumber all the tags and reduce their size to 24 bits
- Drop use of the blob header to specify alignment, in favour of a
'void' blob type
This series attempts to align to that specification, including updating
the API and tests. It is likely that refinements will be made as other
projects implement the spec too.
As before the code is dual-licensed, to permit use in projects with a
permissive license.
END
Regards,
Simon
Re: [PATCH v2 08/18] bloblist: Change the checksum algorithm
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > Use a sinple 8-bit checksum for bloblist, as specified by the spec > version 0.9 > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 14 -- > include/bloblist.h | 5 ++--- > 2 files changed, 10 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 07/18] bloblist: Drop the spare values
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > There are no spare values in spec v0.9 so drop them. > > For now they are still present in the headers, with an underscore, so that > tests continue to pass. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 1 - > include/bloblist.h | 6 ++ > test/bloblist.c| 4 > 3 files changed, 2 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 06/18] bloblist: Drop the flags value
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > There is no flags value in spec v0.9 so drop it. > > For now it is still present in the header, with an underscore, so that > tests continue to pass. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 5 ++--- > include/bloblist.h | 6 ++ > test/bloblist.c| 45 +++-- > 3 files changed, 23 insertions(+), 33 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 09/18] bloblist: Checksum the entire bloblist
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > Spec v0.9 specifies that the entire bloblist area is checksummed, > including unused portions. Update the code to follow this. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 9 + > test/bloblist.c | 10 -- > 2 files changed, 9 insertions(+), 10 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 10/18] bloblist: Handle alignment with a void entry
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > Rather than setting the alignment using the header size, add an entirely > new entry to cover the gap left by the alignment. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 23 +++ > 1 file changed, 19 insertions(+), 4 deletions(-) > Reviewed-by: Simon Glass I am not comfortable with this approach as it makes it more complicated to reused unused areas, but it is what we have ended up with.
Re: [PATCH v2 04/18] bloblist: Set version to 1
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > The new bloblist for v0.9 has version 1 so update this value. > > Signed-off-by: Simon Glass > Co-developed-by: Raymond Mao > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Update the bloblist alignment to align to FW handoff spec v0.9. > > include/bloblist.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 4/5] sandbox: Audit config.h and common.h usage
Hi Tom, On Sat, 18 Nov 2023 at 11:04, Tom Rini wrote: > > Splitting this in to two different threads.. > > On Sat, Nov 18, 2023 at 10:58:50AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 18 Nov 2023 at 10:45, Tom Rini wrote: > > > > > > On Sat, Nov 18, 2023 at 10:09:55AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 17 Nov 2023 at 15:54, Tom Rini wrote: > > > > > > > > > > Remove and replace common.h and config.h in sandbox when it's not > > > > > needed > > > > > and add some explicit includes where needed. > > > > > > > > > > Signed-off-by: Tom Rini > > > > > --- > > > > > arch/sandbox/cpu/cache.c | 1 - > > > > > arch/sandbox/cpu/cpu.c | 1 - > > > > > arch/sandbox/cpu/sdl.c | 2 +- > > > > > arch/sandbox/cpu/spl.c | 1 - > > > > > arch/sandbox/cpu/start.c | 2 +- > > > > > arch/sandbox/cpu/state.c | 2 +- > > > > > arch/sandbox/include/asm/io.h| 2 ++ > > > > > arch/sandbox/include/asm/state.h | 1 - > > > > > arch/sandbox/lib/bootm.c | 1 - > > > > > arch/sandbox/lib/fdt_fixup.c | 1 - > > > > > arch/sandbox/lib/interrupts.c| 1 - > > > > > arch/sandbox/lib/pci_io.c| 1 - > > > > > board/sandbox/sandbox.c | 2 +- > > > > > 13 files changed, 6 insertions(+), 12 deletions(-) > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > Is it possible to move CFG_SYS_SDRAM_BASE/SIZE to Kconfig? > > > > > > Possible? Sure. But I think it would end up being fairly horrible. What > > > would be nice I think is some Zephyr-style "convert > > > CONFIG_DEFAULT_DEVICE_TREE to some defines" because we could parse out > > > /memory to CFG_SYS_SDRAM_BASE/SIZE and that's something we _need_ at > > > build time. > > > > That would be pretty easy to do. But I would prefer to have it create > > CONFIG_SYS_SDRAM_BASE/SIZE so that it fits in with Kconfig, just that > > the value is set for you. > > I _think_ we need it to be non-CONFIG namespace or we'll run in to other > problems? We could get the new autogenerated header to be included along > with generated/autoconf.h but since we couldn't use it within Kconfig > logic it would be confusing. > > > If you like you could create an issue for it. > > Please, thanks. https://source.denx.de/u-boot/u-boot/-/issues/27 Regards, Simon
Re: [PATCH v4 7/8] doc: spl: Add info for missing Kconfigs
On Sat, 25 Nov 2023 at 09:27, Devarsh Thakkar wrote: > > Add info regarding splash screen, video, bloblist and GPIO related > Kconfigs which were missing in the documentation. > > Signed-off-by: Devarsh Thakkar > --- > V2: No change > V3: No change > V4: Patch split from parent > --- > doc/develop/spl.rst | 9 + > 1 file changed, 9 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi,
On Tue, 21 Nov 2023 at 13:49, Tom Rini wrote:
>
> On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote:
> > Hi Simon
> >
> > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote:
> > >
> > > Hi Heinrich,
> > >
> > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt
> > > wrote:
> > > >
> > > > On 10/15/23 04:45, Simon Glass wrote:
> > > > > When the SMBIOS table is written to an address above 4GB a 32-bit
> > > > > table
> > > > > address is not large enough.
> > > > >
> > > > > Use an SMBIOS3 table in that case.
> > > > >
> > > > > Note that we cannot use efi_allocate_pages() since this function has
> > > > > nothing to do with EFI. There is no equivalent function to allocate
> > > > > memory below 4GB in U-Boot. One solution would be to create a separate
> > > > > malloc() pool, or just always put the malloc() pool below 4GB.
> > > > >
> > > > > Signed-off-by: Simon Glass
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Check the end of the table rather than the start.
> > > > >
> > > > > include/smbios.h | 22 +-
> > > > > lib/smbios.c | 24 +++-
> > > > > 2 files changed, 40 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/include/smbios.h b/include/smbios.h
> > > > > index c9df2706f5a6..ddabb558299e 100644
> > > > > --- a/include/smbios.h
> > > > > +++ b/include/smbios.h
> > > > > @@ -12,7 +12,8 @@
> > > > >
> > > > > /* SMBIOS spec version implemented */
> > > > > #define SMBIOS_MAJOR_VER3
> > > > > -#define SMBIOS_MINOR_VER 0
> > > > > +#define SMBIOS_MINOR_VER 7
> > > > > +
> > > > >
> > > > > enum {
> > > > > SMBIOS_STR_MAX = 64, /* Maximum length allowed for a string
> > > > > */
> > > > > @@ -54,6 +55,25 @@ struct __packed smbios_entry {
> > > > > u8 bcd_rev;
> > > > > };
> > > > >
> > > > > +struct __packed smbios3_entry {
> > > > > + u8 anchor[5];
> > > > > + u8 checksum;
> > > > > + u8 length;
> > > > > + u8 major_ver;
> > > > > +
> > > > > + u8 minor_ver;
> > > > > + u8 docrev;
> > > > > + u8 entry_point_rev;
> > > > > + u8 reserved;
> > > > > + u32 max_struct_size;
> > > > > +
> > > > > + u64 struct_table_address;
> > > > > +};
> > > > > +
> > > > > +/* These two structures should use the same amount of
> > > > > 16-byte-aligned space */
> > > > > +static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
> > > > > + ALIGN(16, sizeof(struct smbios3_entry)));
> > > > > +
> > > > > /* BIOS characteristics */
> > > > > #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
> > > > > #define BIOS_CHARACTERISTICS_UPGRADEABLE(1 << 11)
> > > > > diff --git a/lib/smbios.c b/lib/smbios.c
> > > > > index c7a557bc9b7b..92e98388084f 100644
> > > > > --- a/lib/smbios.c
> > > > > +++ b/lib/smbios.c
> > > > > @@ -487,7 +487,11 @@ ulong write_smbios_table(ulong addr)
> > > > > addr = ALIGN(addr, 16);
> > > > > start_addr = addr;
> > > > >
> > > > > - addr += sizeof(struct smbios_entry);
> > > > > + /*
> > > > > + * So far we don't know which struct will be used, but they
> > > > > both end
> > > > > + * up using the same amount of 16-bit-aligned space
> > > > > + */
> > > > > + addr += max(sizeof(struct smbios_entry), sizeof(struct
> > > > > smbios3_entry));
> > > > > addr = ALIGN(addr, 16);
> > > > > tables = addr;
> > > > >
> > > > > @@ -512,14 +516,24 @@ ulong write_smbios_table(ulong addr)
> > > > >* sandbox's DRAM buffer.
> > > > >*/
> > > > > table_addr = (ulong)map_sysmem(tables, 0);
> > > > > - if (sizeof(table_addr) > sizeof(u32) && table_addr >
> > > > > (ulong)UINT_MAX) {
> > > > > + if (sizeof(table_addr) > sizeof(u32) && addr >=
> > > > > (ulong)UINT_MAX) {
> > > >
> > > > You have to check the end of the the last SMBIOS structure not the start
> > > > of the first SMBIOS structure against UINT_MAX.
> > > >
> > > > > + struct smbios3_entry *se;
> > > > > /*
> > > > >* We need to put this >32-bit pointer into the table
> > > > > but the
> > > > >* field is only 32 bits wide.
> > > > >*/
> > > > > - printf("WARNING: SMBIOS table_address overflow %llx\n",
> > > > > -(unsigned long long)table_addr);
> > > > > - addr = 0;
> > > > > + printf("WARNING: Using SMBIOS3.0 due to table-address
> > > > > overflow %lx\n",
> > > > > +table_addr);
> > > >
> > > > This should be log_debug().
> > > >
> > > > > + se = map_sysmem(start_addr, sizeof(struct
> > > > > smbios_entry));
> > > > > + memset(se, '\0', sizeof(struct smbios_entry));
> > > > > + memcpy(se->anchor, "_SM3_", 5);
> > > > > + se->length = sizeof(struct smbios3_entry);
> > > > > + se->major_ver = SMBIOS_MAJOR_VER;
> > > > > + se->minor_ver = SMBIOS_MINOR_VER;
Re: [PATCH v4 2/2] cmd: acpi: fix acpi list command
On Tue, 21 Nov 2023 at 07:41, Heinrich Schuchardt wrote: > > ACPI tables may comprise either RSDT, XSDT, or both. The current code fails > to check the presence of the RSDT table before accessing it. This leads to > an exception if the RSDT table is not provided. > > The XSDT table takes precedence over the RSDT table. > > The return values of list_rsdt() and list_rsdp() are always zero and not > checked. Remove the return values. > > Addresses in the XSDT table are 64-bit. Adjust the output accordingly. > > As the RSDT table has to be ignored if the XSDT command is present there is > no need to compare the tables in a display command. Anyway the > specification does not require that the sequence of addresses in the RSDT > and XSDT table are the same. > > The FACS table header does not provide revision information. Correct the > description of dump_hdr(). > > Adjust the ACPI test to match the changed output format of the 'acpi list' > command. > > Signed-off-by: Heinrich Schuchardt > --- > v4: > deduplicate code in list_rsdt() > v3: > consider map_sysmem(0, 0) != NULL on the sandbox > adjust signature of list_rsdt() and list_rsdp() > v2: > add unit test for offset of field Entry in RSDT, XSDT > merge patch 2 and 3 > remove leading zeros from address output > --- > cmd/acpi.c | 64 +++--- > test/dm/acpi.c | 18 +++--- > 2 files changed, 43 insertions(+), 39 deletions(-) Reviewed-by: Simon Glass I really don't like using 16 characters for a 64-bit address. I wonder if there is a better solution? But at least it is leading spaces, not zeroes.
Re: [PATCH v2 05/18] bloblist: Access record hdr_size and tag via a function
On Mon, 27 Nov 2023 at 12:52, Raymond Mao wrote: > > From: Simon Glass > > These values currently use a simple field. With spec v0.9 they have moved > to a packed format. Convert most accesses to use functions, so this change > can be accomodated. > > Signed-off-by: Simon Glass > Signed-off-by: Raymond Mao > --- > common/bloblist.c | 38 +- > 1 file changed, 25 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass
Re: How to use I2C in U-Boot SPL
kHi Thomas,
On Wed, 22 Nov 2023 at 13:57, Thomas Thielemann wrote:
>
> Hello U-Boot,
>
> I was able to use the I2C commands from within U-Boot to control my I2C
> device:
>
> Configured the Processor GPIOs for the I2C bus in device tree
> Enabled the I2C bus in device tree
> Added my device to the I2C bus in device tree
> Wrote my script into a U-Boot variable
> Run my script
> Now I want to do the same from within the U-Boot SPL. I extended int
> board_early_init_f(void) in board.c for my hardware. The debug output is
> printed at run time. But the I2C device is not found.
>
> How do I review whether the I2C bus is using the right GPIOs and whether my
> device is configured right?
>
> static int i2c_get_dev(uint bus_num, uint dev_num, struct udevice **i2c_dev)
> {
> int ret;
>
> if (!i2c_led_bus) {
> ret = uclass_get_device_by_seq(UCLASS_I2C, bus_num, _led_bus);
> if (ret) {
> printf("I2C bus %i not found (%d)\n", dev_num, ret);
> return -ENODEV;
> }
> }
>
> return i2c_get_chip(i2c_led_bus, dev_num, bus_num, i2c_dev);
> }
> #endif
>
> static int i2c_enable_leds(void)
> {
> log_info("i2c_enable_leds\n");
> const uint bus_num = 1;// i2c bus
> const uint dev_num = 0x28; // i2c chip address
> const uint dev_addr = 0x00;// write all bytes in one junk from start
> const uint dev_addr_len = 1;
> int ret;
> const uint cmd_len = 29; // cmd_len is the number of bytes.
> uchar cmd[29] = {
> 0x40, 0x3c, // init
> 0x00, 0x00, 0x00, 0x00, 0x00, // unused
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // set intensitiy to
> max
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // unused
> 0xff, 0x00, 0x00, // RGB LED
> 0xff, 0x00, 0x00 // RGB LED
> };
> #if CONFIG_IS_ENABLED(DM_I2C)
> struct udevice *i2c_dev;
> struct dm_i2c_chip *i2c_chip;
> #endif
>
> #if CONFIG_IS_ENABLED(DM_I2C)
> ret = i2c_get_dev(bus_num, dev_num, _dev);
> if (!ret)
> ret = i2c_set_chip_offset_len(i2c_dev, dev_addr_len);
> if (ret) {
> log_err("Failed to write to 0x28 (%d)\n", ret);
> return ret;
> }
> i2c_chip = dev_get_parent_plat(i2c_dev);
> if (!i2c_chip) {
> log_err("Failed to write to 0x28 (%d)\n", ret);
> return ret;
> }
> #endif
>
> #if CONFIG_IS_ENABLED(DM_I2C)
> i2c_chip->flags &= ~DM_I2C_CHIP_WR_ADDRESS;
> ret = dm_i2c_write(i2c_dev, dev_addr, cmd, cmd_len);
> #else
> ret = i2c_write(dev_num, dev_addr, dev_addr_len, cmd, cmd_len);
> #endif
> if (ret)
> log_err("Failed to write to 0x28 (%d)\n", ret);
>
> return 0;
> }
> int board_early_init_f(void)
> {
> i2c_enable_leds();
>
> return 0;
> }
You should use driver mode for this, i.e. add your i2c device to the
devicetree with a suitable driver in place, then probe it, e.g. with
uclass_first_device(UCLASS_LED)
Accessing the chip directly like this is a bit hacky :-)
If you include a bit more info about what piece breaks, it would help
people to offer ideas.
Regards,
SImon
Re: [PATCH 2/2 v2] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Ilias, On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas wrote: > > Hi Simon, > > [...] > >> > Changes since v1: >> > - Tokenize the DT node entry and use the appropriate value instead of >> > the entire string >> > - Removed Peters tested/reviewed-by tags due to the above >> > lib/smbios.c | 94 +--- >> > 1 file changed, 90 insertions(+), 4 deletions(-) >> > >> >> Can this be put behind a Kconfig? It adds quite a bit of code which >> punishes those boards which do the right thing. > > > Sure but OTOH the code increase should be really minimal. But I don't mind I > can add a Kconfig > >> >> > + >> > + dt_str = ofnode_read_string(ofnode_root(), nprop->dt_str); >> >> Could this use ofnode_read_string_index() ? > > > Maybe, I'll have a look and change it if that works > > [...] > >> >> Any chance of a test for this code? > > > Sure, but any suggestions on where to add the test? > SMBIOS tables are populated on OS booting, do we have a test somewhere that > boots an OS? They are written on startup, right? They should certainly be in place before U-Boot enters the command line. > Any other ideas? Probably a test in test/lib/ would work. We actually have smbios-parser.c which might help? Regards, Simon
Re: [PATCH 4/5] sandbox: Audit config.h and common.h usage
Hi Tom, On Sat, 18 Nov 2023 at 11:44, Tom Rini wrote: > > On Sat, Nov 18, 2023 at 10:58:50AM -0700, Simon Glass wrote: > > [snip] > > I found these: > > > > https://patchwork.ozlabs.org/project/uboot/list/?series=262148=* > > http://patchwork.ozlabs.org/project/uboot/patch/20210925121958.26001-1-p...@kernel.org/ > > > > IMO the second one from Pali makes sense, but it was never followed > > up. It doesn't look too difficult. > > > > We should have a policy that if people complain about a patch but > > don't follow up, we apply the patch we have. > > I think the patch from Pali was the right direction, but as I noted, I > wanted to see it implemented for some other UARTs as well, and it wasn't > high on anyone elses priority list. Covering ns16550-style UARTs too > would be good. https://source.denx.de/u-boot/u-boot/-/issues/28 Regards, Simon
Re: [PATCH 2/2] nvme: Update scan namespace to keep trying on busy
On Wed, 22 Nov 2023 at 16:53, Moritz Fischer wrote:
>
> A busy controller shouldn't be game-over for all controllers,
> so keep trying on hitting -EBUSY.
>
> This change brings the actual behavior of the routine in line
> with what the descriptions says.
>
> Fixes: 982388eaa991 ("nvme: Add NVM Express driver support")
> Signed-off-by: Moritz Fischer
> ---
> drivers/nvme/nvme.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass
Re: [PATCH v4 5/8] video: Skip framebuffer reservation if already reserved
On Sat, 25 Nov 2023 at 09:27, Devarsh Thakkar wrote:
>
> Skip framebufer reservation if it was already reserved from previous
> stage and whose information was passed using a bloblist.
>
> Return error in case framebuffer information received from bloblist is
> invalid i.e NULL or empty.
>
> While at it, improve the debug message to make it more clear that
> address in discussion is of framebuffer and not bloblist and also match
> it with printing scheme followed in video_reserve function.
>
> Signed-off-by: Devarsh Thakkar
> ---
> V2:
> - Add debug prints
> - Fix commenting style
> V3:
> - Fix commenting style
> V4:
> - Remove extra checks on gd for video data in video_reserve
> - Add check and return error if video handoff provided info is invalid
> - Improve debug message
> - Remove Reviewed-by due to additional changes per review comments
> ---
> common/board_f.c | 8 ++--
> drivers/video/video-uclass.c | 10 --
> 2 files changed, 14 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass
with nit below
>
> diff --git a/common/board_f.c b/common/board_f.c
> index acf802c9cb..442b8349d0 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -407,11 +407,15 @@ static int reserve_video_from_videoblob(void)
> {
> if (IS_ENABLED(CONFIG_SPL_VIDEO_HANDOFF) && spl_phase() > PHASE_SPL) {
This is board_f.c so the SPL phase will always be PHASE_BOARD_F so can
you please drop that second condition? It will confuse people.
> struct video_handoff *ho;
> + int ret = 0;
>
> ho = bloblist_find(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho));
> if (!ho)
> - return log_msg_ret("blf", -ENOENT);
> - video_reserve_from_bloblist(ho);
> + return log_msg_ret("Missing video bloblist", -ENOENT);
> +
> + ret = video_reserve_from_bloblist(ho);
> + if (ret)
> + return log_msg_ret("Invalid Video handoff info", ret);
>
> /* Sanity check fb from blob is before current relocaddr */
> if (likely(gd->relocaddr > (unsigned long)ho->fb))
[..]
Regards,
Simon
Re: [PATCH 1/2] nvme: Fix error code and log to indicate busy
On Wed, 22 Nov 2023 at 16:52, Moritz Fischer wrote:
>
> Return -EBUSY if controller is found busy rather than -ENOMEM
-ENODEV ?
> and update the error message accordingly.
>
> Fixes: 982388eaa991 ("nvme: Add NVM Express driver support")
> Signed-off-by: Moritz Fischer
> ---
> drivers/nvme/nvme.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index c39cd41aa3..ec45f831a3 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -835,8 +835,8 @@ int nvme_init(struct udevice *udev)
> ndev->udev = udev;
> INIT_LIST_HEAD(>namespaces);
> if (readl(>bar->csts) == -1) {
> - ret = -ENODEV;
> - printf("Error: %s: Out of memory!\n", udev->name);
> + ret = -EBUSY;
> + printf("Error: %s: Controller not ready!\n", udev->name);
> goto free_nvme;
> }
>
> --
> 2.43.0.rc1.413.gea7ed67945-goog
>
Re: [PATCH v5 05/12] usb: Avoid unbinding devices in use by bootflows
Hi Shantur, On Thu, 23 Nov 2023 at 04:35, Shantur Rathore wrote: > > Hi Simon, > > On Tue, Nov 21, 2023 at 2:17 AM Simon Glass wrote: > > > > Hi Shantur, > > > > On Sun, 19 Nov 2023 at 15:47, Shantur Rathore wrote: > > > > > > Hi Simon, > > > > > > On Sun, Nov 19, 2023 at 7:12 PM Simon Glass wrote: > > > > > > > > When a USB device is unbound, it causes any bootflows attached to it to > > > > be removed, via a call to bootdev_clear_bootflows() from > > > > bootdev_pre_unbind(). This obviously makes it impossible to boot the > > > > bootflow. > > > > > > > > However, when booting a bootflow that relies on USB, usb_stop() is > > > > called, which unbinds the device. At that point any information > > > > attached to the bootflow is dropped. > > > > > > > > This is quite risky since the contents of freed memory are not > > > > guaranteed to remain unchanged. Depending on what other options are > > > > done before boot, a hard-to-find bug may crop up. > > > > > > > > There is no need to unbind all the USB devices just to quiesce them. > > > > Add a new usb_pause() call which removes them but leaves them bound. > > > > > > > > > > I am no UEFI / bootloader expert and while trying to gather more info > > > on EFI ExitBootServies I came across this > > > https://github.com/tianocore-docs/edk2-UefiDriverWritersGuide/blob/master/7_driver_entry_point/77_adding_the_exit_boot_services_feature.md > > > > > > If I understand this correctly, EFI ( U-boot in this case) should be > > > halting all USB controllers like done in usb_stop() > > > Is my understanding correct? > > > > Yes, that is correct. The dm_remove_devices_flags() call should do > > that. The code in bootm_disable_interrupts() is a hangover from the > > pre-driver model days, I suspect. > > > > Perhaps we should also remove the eth_halt() and usb_pause() from > > bootm_disable_interrupts() ? > > > > Apologies for delayed response. > In this case, I think we should remove eth_halt() and > usb_stop() (there is no usb_pause() ) from bootm_disable_interrupts(). > > No point stopping things twice. Yes, I agree. I will take a look. Regards, Simon
Re: [PATCH] doc: Remove README.sha1 file
On Wed, 22 Nov 2023 at 15:57, Peter Robinson wrote:
>
> The contents of README.sha1 only refer to process around verification
> of the pcs440ep board firmware in flash. The device was removed in
> commit 242836a893ae ("powerpc: ppc4xx: remove pcs440ep support") in
> 2015 so this readme isn't really relevant anymore so can be removed.
>
> Signed-off-by: Peter Robinson
> ---
>
> With the wide deprecation of sha1 in general we should likely also
> review the use of sha1 in U-Boot as a whole and look to move the
> remaining users of sha1 to sha2 or sha3 in general.
We could start by adding a warning in Binman when it sees this being
used in a FIT. I suppose we can't really disable the feature in U-Boot
since so many images still use it?
>
> doc/README.sha1 | 58 -
> 1 file changed, 58 deletions(-)
> delete mode 100644 doc/README.sha1
Reviewed-by: Simon Glass
Re: [PATCH v4 8/8] doc: spl: Add info regarding memory reservation
On Sat, 25 Nov 2023 at 09:27, Devarsh Thakkar wrote: > > Add details regarding scheme which need to be followed in SPL and > further stages for those regions which need to be preserved across > bootstages. > > Signed-off-by: Devarsh Thakkar > --- > V1->V3: > No change. > > V4: > Split this to separate patch and add more details regarding > memory reservation scheme that needs to be followed at SPL. > --- > doc/develop/spl.rst | 28 > 1 file changed, 28 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 6/7] riscv: allow usage of ACPI
On Tue, 21 Nov 2023 at 08:28, Heinrich Schuchardt wrote: > > Select CONFIG_SUPPORT_ACPI to allow usage of ACPI tables with RISC-V. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > arch/Kconfig | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Simon Glass Oh dear, first ARM and now RISC-V
Re: [PATCH v2 4/7] risc-v: add support for QEMU firmware tables
On Tue, 21 Nov 2023 at 08:28, Heinrich Schuchardt wrote: > > Enable the QEMU firmware interface if ACPI tables are to be supported on > the QEMU platform. > > Enable the QFW MMIO interface if the QEMU firmware interface is enabled. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > depend on CONFIG_ACPI instead of CONFIG_GENERATE_ACPI_TABLES > --- > board/emulation/qemu-riscv/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v2 5/6] video: Fill video handoff in video post probe
Hi Devarsh,
On Sat, 25 Nov 2023 at 07:27, Devarsh Thakkar wrote:
>
> Hi Simon,
>
> Thanks for the review.
>
> On 13/11/23 01:31, Simon Glass wrote:
> > Hi Devarsh,
> >
> > On Fri, 10 Nov 2023 at 08:29, Devarsh Thakkar wrote:
> >>
> >> Fill video handoff fields in video_post_probe
> >> as at this point we have full framebuffer-related
> >> information.
> >>
> >> Also fill all the fields available in video hand-off
> >> struct as those were missing earlier and U-boot
> >
> > U-Boot
> >
> >> framework expects them to be filled for some of the
> >> functionalities.
> >
> > Can you wrap your commit messages to closer to 72 chars?
> >
> >>
> >> Reported-by: Simon Glass
> >> Signed-off-by: Devarsh Thakkar
> >> ---
> >> V2:
> >> - No change
> >>
> >> V3:
> >> - Fix commit message per review comment
> >> - Add a note explaining assumption of single framebuffer
> >> ---
> >> drivers/video/video-uclass.c | 29 +++--
> >> 1 file changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> >> index f619b5ae56..edc3376b46 100644
> >> --- a/drivers/video/video-uclass.c
> >> +++ b/drivers/video/video-uclass.c
> >> @@ -154,16 +154,6 @@ int video_reserve(ulong *addrp)
> >> debug("Video frame buffers from %lx to %lx\n", gd->video_bottom,
> >> gd->video_top);
> >>
> >> - if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(VIDEO_HANDOFF)) {
> >> - struct video_handoff *ho;
> >> -
> >> - ho = bloblist_add(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho), 0);
> >> - if (!ho)
> >> - return log_msg_ret("blf", -ENOENT);
> >> - ho->fb = *addrp;
> >> - ho->size = size;
> >> - }
> >> -
> >> return 0;
> >> }
> >>
> >> @@ -559,6 +549,25 @@ static int video_post_probe(struct udevice *dev)
> >>
> >> priv->fb_size = priv->line_length * priv->ysize;
> >>
> >> + /*
> >> +* Set up video handoff fields for passing video blob to next stage
> >> +* NOTE:
> >> +* This assumes that reserved video memory only uses a single
> >> framebuffer
> >> +*/
> >> + if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(BLOBLIST)) {
> >> + struct video_handoff *ho;
> >> +
> >> + ho = bloblist_add(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho), 0);
> >> + if (!ho)
> >> + return log_msg_ret("blf", -ENOENT);
> >> + ho->fb = gd->video_bottom;
> >> + ho->size = gd->video_top - gd->video_bottom;
> >
> > should be plat->base and plat->size
> >
>
> plat->size contains the unaligned size actually. While reserving video memory,
> the size of allocation is updated [0] as per default alignment (1 MiB) or
> alignment requested by driver. So I thought it is better to pass actual
> allocated size calculated using gd as the next stage receiving hand-off can
> directly skip the region as per passed size. And since I used gd for
> calculating size, I thought to stick to using gd for ho->fb too for
> consistency.
>
> Kindly let me know if any queries.
This sort of thing would have been useful to put in a comment in the
code, or commit message.
I still don't really see why the 'aligned' size isn't already in plat,
after alloc_fb() is called.
Anyway I will leave this to Anatolij
Reviewed-by: Simon Glass
>
> [0]:
> https://source.denx.de/u-boot/u-boot/-/blob/u-boot-2023.07.y/drivers/video/video-uclass.c?ref_type=heads#L88
>
> Regards
> Devarsh
>
> >> + ho->xsize = priv->xsize;
> >> + ho->ysize = priv->ysize;
> >> + ho->line_length = priv->line_length;
> >> + ho->bpix = priv->bpix;
> >> + }
> >> +
> >> if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->copy_base)
> >> priv->copy_fb = map_sysmem(plat->copy_base, plat->size);
> >>
> >> --
> >> 2.34.1
> >>
> >
> > Regards,
> > Simon
Re: [PATCH v2 5/7] riscv: qemu: copy ACPI tables from QEMU
On Tue, 21 Nov 2023 at 08:28, Heinrich Schuchardt
wrote:
>
> If CONFIG_GENERATE_ACPI_TABLES=y, read the ACPI tables provided by QEMU
> and make them available to U-Boot.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> board/emulation/qemu-riscv/qemu-riscv.c | 17 +
> 1 file changed, 17 insertions(+)
Reviewed-by: Simon Glass
Can we not use common code to do this? What is different about ARM and x86?
>
> diff --git a/board/emulation/qemu-riscv/qemu-riscv.c
> b/board/emulation/qemu-riscv/qemu-riscv.c
> index 181abbbf97..2cebce2128 100644
> --- a/board/emulation/qemu-riscv/qemu-riscv.c
> +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> @@ -4,17 +4,20 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -46,6 +49,20 @@ int board_late_init(void)
> if (CONFIG_IS_ENABLED(USB_KEYBOARD))
> usb_init();
>
> + if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) {
> + uintptr_t addr;
> + void *ptr;
> +
> + /* Reserve 64K for ACPI tables, aligned to a 4K boundary */
> + ptr = memalign(SZ_4K, SZ_64K);
> + addr = (uintptr_t)ptr;
> +
> + /* Generate ACPI tables */
> + write_acpi_tables(addr);
> + gd->arch.table_start = addr;
> + gd->arch.table_end = addr;
> + }
> +
> return 0;
> }
>
> --
> 2.40.1
>
Re: [PATCH v2 3/7] risc-v: add ACPI fields to global data
On Tue, 21 Nov 2023 at 08:28, Heinrich Schuchardt wrote: > > Add fields for the location of ACPI tables to the global data. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > arch/riscv/include/asm/global_data.h | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 01/11] global: Remove duplicate common.h inclusions
On Wed, 22 Nov 2023 at 06:12, Tom Rini wrote: > > These files include twice. Start by removing the second > inclusion of the file. > > Signed-off-by: Tom Rini > --- > board/data_modul/common/common.c | 1 - > board/grinn/liteboard/board.c | 1 - > board/toradex/colibri_imx7/colibri_imx7.c | 1 - > board/wandboard/wandboard.c | 1 - > drivers/gpio/gpio-aspeed.c| 1 - > drivers/spi/fsl_dspi.c| 1 - > drivers/video/exynos/exynos_dp.c | 1 - > 7 files changed, 7 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 06/11] lib/sha*.c: Update header list
Hi Tom, On Wed, 22 Nov 2023 at 06:12, Tom Rini wrote: > > Cleanup the list of headers we include here. For the tools build we only > need to exclude as that's used by the target build for the > prototype for schedule(), and we don't need to get that via > . We can also make use of our intentionally > existing as a redirection to to reduce ifdef'd lines. > > Signed-off-by: Tom Rini > --- > lib/sha1.c | 7 ++- > lib/sha256.c | 7 ++- > lib/sha512.c | 6 +- > 3 files changed, 5 insertions(+), 15 deletions(-) As mentioned in my series this will prevent dropping the #ifdefs in this file: #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) Actually I see that I didn't send the v2 so I just did that now[1]. Regards, Simon [1] https://patchwork.ozlabs.org/project/uboot/patch/20231202153400.537050-2-...@chromium.org/
Re: [PATCH 05/11] global: Rework architecture global_data.h to include
On Wed, 22 Nov 2023 at 06:12, Tom Rini wrote: > > In most cases, the architecture global data currently makes use of > assorted linux types, but does not include to provide > them. Add instead of relying on indirect inclusion. > > Signed-off-by: Tom Rini > --- > arch/mips/include/asm/global_data.h| 2 +- > arch/nios2/include/asm/global_data.h | 2 ++ > arch/powerpc/include/asm/global_data.h | 3 +-- > arch/riscv/include/asm/global_data.h | 1 + > arch/x86/include/asm/global_data.h | 1 + > 5 files changed, 6 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 02/11] arm: Remove from asm/global_data.h
On Wed, 22 Nov 2023 at 06:12, Tom Rini wrote: > > We need and include and this in turn already includes > , so drop it here. > > Signed-off-by: Tom Rini > --- > arch/arm/include/asm/global_data.h | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 10/11] include: Further cleanup includes
On Wed, 22 Nov 2023 at 06:13, Tom Rini wrote: > > Add some missing headers such as or or > to header files that make direct usage of things > provided by these headers. > > Signed-off-by: Tom Rini > --- > include/atmel_lcd.h | 2 ++ > include/getopt.h| 2 ++ > include/mapmem.h| 2 ++ > include/memalign.h | 1 + > include/net6.h | 1 + > include/rtc.h | 1 + > 6 files changed, 9 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 5/5] sysreset: Support reset via Renesas RAA215300 PMIC
On 11/19/23 21:52, Paul Barker wrote:
On Sun, Nov 19, 2023 at 09:17:40PM +0100, Marek Vasut wrote:
On 11/15/23 18:40, Paul Barker wrote:
This patch allows us to reset the RZ/G2L board via the RAA215300 PMIC.
Note that the RAA215300 documentation names the available reset types
differently to u-boot:
- A "warm" reset via the RAA215300 PMIC will fully reset the SoC
(CPU & GPIOs), so this corresponds to SYSRESET_COLD.
- A "cold" reset via the RAA215300 PMIC will cycle all power supply
rails, so this corresponds to SYSRESET_POWER.
Signed-off-by: Paul Barker
---
board/renesas/rzg2l/rzg2l.c | 8
configs/renesas_rzg2l_smarc_defconfig | 2 +
drivers/power/pmic/raa215300.c| 17
drivers/sysreset/Kconfig | 6 +++
drivers/sysreset/Makefile | 1 +
drivers/sysreset/sysreset_raa215300.c | 58 +++
6 files changed, 84 insertions(+), 8 deletions(-)
create mode 100644 drivers/sysreset/sysreset_raa215300.c
diff --git a/board/renesas/rzg2l/rzg2l.c b/board/renesas/rzg2l/rzg2l.c
index 73201a8c69e5..0f6d6e7f514f 100644
--- a/board/renesas/rzg2l/rzg2l.c
+++ b/board/renesas/rzg2l/rzg2l.c
@@ -56,11 +56,3 @@ int board_init(void)
{
return 0;
}
-
-void reset_cpu(void)
-{
- /*
-* TODO: Implement reset support once TrustedFirmware supports
-* the appropriate call.
-*/
-}
Board change -- separate patch please.
Ok, this makes sense. I'll move this and the defconfig change into a
separate patch.
diff --git a/configs/renesas_rzg2l_smarc_defconfig
b/configs/renesas_rzg2l_smarc_defconfig
index b62eae4ee0a4..ba96e746df9e 100644
--- a/configs/renesas_rzg2l_smarc_defconfig
+++ b/configs/renesas_rzg2l_smarc_defconfig
@@ -55,3 +55,5 @@ CONFIG_PMIC_RAA215300=y
CONFIG_DM_REGULATOR=y
CONFIG_DM_REGULATOR_FIXED=y
CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_SYSRESET=y
+CONFIG_SYSRESET_RAA215300=y
diff --git a/drivers/power/pmic/raa215300.c b/drivers/power/pmic/raa215300.c
index 9c0b720994b2..7f68f95f25cf 100644
--- a/drivers/power/pmic/raa215300.c
+++ b/drivers/power/pmic/raa215300.c
@@ -27,9 +27,26 @@ static const struct udevice_id raa215300_ids[] = {
{ /* sentinel */ }
};
+static int raa215300_bind(struct udevice *dev)
+{
+ struct driver *drv;
+
+ if (IS_ENABLED(CONFIG_SYSRESET_RAA215300)) {
+ drv = lists_driver_lookup_name("raa215300_sysreset");
+ if (!drv)
+ return -ENOENT;
+
+ return device_bind(dev, drv, dev->name, NULL, dev_ofnode(dev),
+ NULL);
+ }
+
+ return 0;
+}
Driver change should be squashed in 4/5.
Moving this to the previous patch doesn't make sense to me - the
sysreset driver needs to exist for this check to be meaningful.
Conversely, adding the sysreset driver first then the pmic driver
wouldn't make sense since the sysreset driver depends on the pmic
driver. This seemed the neatest way to do things to me.
Can't you split this into 4 or so patches , add the bind() callbacks in
first two, then the if (IS_ENABLED(...)) in next two ?
Re: [PATCH 3/5] i2c: rzg2l: Add I2C driver for RZ/G2L family
On 11/19/23 21:48, Paul Barker wrote: On Sun, Nov 19, 2023 at 09:15:36PM +0100, Marek Vasut wrote: On 11/15/23 18:40, Paul Barker wrote: This driver supports the I2C module on the Renesas RZ/G2L (R9A07G044) SoC, also known as the RIIC module. This patch is based on both the u-boot driver in the Renesas RZ BSP 3.0.5 release [1] (commit 7fcc1fdc2534), and the Linux v6.6 driver (commit ffc253263a13). Support for deblocking the I2C bus is included as this may be needed after triggering a reset via the Power Management IC (PMIC) over I2C (the PMIC asserts the reset line before the SoC completes the I2C write transaction with obvious bus locking effects). If the SDA line is observed to be low during initialisation, we automatically attempt to deblock. [1]: https://github.com/renesas-rz/renesas-u-boot-cip Signed-off-by: Paul Barker The driver seems very similar to drivers/i2c/rcar_iic.c , can there be some code reuse ? My initial idea was to extend the R-Car iic driver but I quickly saw that would be very messy. All the registers and bits are in different places, the calculations are different (particularly in regard to setting bus speed) and the read/write process has more edge cases for the RZ/G2L. The bus recovery mechanism we use also doesn't exist on previous R-Car SoCs. The Linux drivers for these modules are also separate, see drivers/i2c/busses/i2c-sh_mobile.c (for R-Car and earlier SoCs) and drivers/i2c/busses/i2c-riic.c (for RZ/G2L) in the Linux kernel. Ah, I thought some of the registers were the same, but indeed the RZG core is much more extensive. You should either use devm_clk_*() in probe() or implement .remove callback. With that fixed: Reviewed-by: Marek Vasut And sorry for the delayed reply.
Re: [PATCH v7 2/2] arm64: boot: Support Flat Image Tree
Hi Ahmad, On Thu, 30 Nov 2023 at 19:04, Ahmad Fatoum wrote: > > Hello Simon, > > On 30.11.23 21:30, Simon Glass wrote: > > On Wed, 29 Nov 2023 at 12:54, Ahmad Fatoum wrote: > >> On 29.11.23 20:44, Simon Glass wrote: > >>> On Wed, 29 Nov 2023 at 12:33, Ahmad Fatoum > >>> wrote: > > On 29.11.23 20:27, Simon Glass wrote: > > On Wed, 29 Nov 2023 at 12:15, Ahmad Fatoum > > wrote: > >> On 29.11.23 20:02, Simon Glass wrote: > >>> On Wed, 29 Nov 2023 at 11:59, Ahmad Fatoum > >>> wrote: > The specification says that this is the root U-Boot compatible, > which I presume to mean the top-level compatible, which makes sense > to me. > > The code here though adds all compatible strings from the device > tree though, > is this intended? > >>> > >>> Yes, since it saves needing to read in each DT just to get the > >>> compatible stringlist. > >> > >> The spec reads as if only one string (root) is supposed to be in the > >> list. > >> The script adds all compatibles though. This is not really useful as a > >> bootloader > >> that's compatible with e.g. fsl,imx8mm would just take the first > >> device tree > >> with that SoC, which is most likely to be wrong. It would be better to > >> just > >> specify the top-level compatible, so the bootloader fails instead of > >> taking > >> the first DT it finds. > > > > We do need to have a list, since we have to support different board > > revs, etc. > > Can you give me an example? The way I see it, a bootloader with > compatible "vendor,board" and a FIT with configuration with compatibles: > > "vendor,board-rev-a", "vendor,board" > "vendor,board-rev-b", "vendor,board" > > would just result in the bootloader booting the first configuration, > even if > the device is actually rev-b. > >>> > >>> You need to find the best match, not just any match. This is > >>> documented in the function comment for fit_conf_find_compat(). > >> > >> In my above example, both configuration are equally good. > >> Can you give me an example where it makes sense to have multiple > >> compatibles automatically extracted from the device tree compatible? > >> > >> The way I see it having more than one compatible here just has > >> downsides. > > > > I don't have an example to hand, but this is the required mechanism of > > FIT. This feature has been in place for many years and is used by > > ChromeOS, at least. > > I see the utility of a FIT configuration with > > compatible = "vendor,board-rev-a", "vendor,board-rev-b"; > > I fail to see a utility for a configuration with > > compatible = "vendor,board", "vendor,SoM", "vendor,SoC"; > > Any configuration that ends up being booted because "vendor,SoC" was matched > is > most likely doomed to fail. Therefore, I would suggest that only the top level > configuration is written into the FIT configurations automatically. Firstly, I am not an expert on this. Say you have a board with variants. The compatible string in U-Boot may be something like: "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron", "rockchip,rk3288"; If you then have several FIT configurations, they may be something like: "google,veyron-brain-rev0", "google,veyron-brain", "google,veyron", "rockchip,rk3288"; "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron", "rockchip,rk3288"; "google,veyron-brain-rev2", "google,veyron-brain", "google,veyron", "rockchip,rk3288"; You want to choose the second one, since it is a better match than the others. +Doug Anderson who knows a lot more about this than me. Regards, Simon
Re: [PATCH v2 1/9] cmd: bdinfo: Optionally use getopt and implement bdinfo -a
On 10/7/23 23:40, Marek Vasut wrote: Add optional support for getopt() and in case this is enabled via GETOPT configuration option, implement support for 'bdinfo -a'. The 'bdinfo -a' behaves exactly like bdinfo and prints 'all' the bdinfo information. This is implemented in preparation for other more fine-grained options. Reviewed-by: Simon Glass Signed-off-by: Marek Vasut --- Cc: Bin Meng Cc: Mario Six Cc: Nikhil M Jain Cc: Simon Glass --- V2: Add RB from Simon I see this series marked as Superseded in patchwork , why is that so ? Is there anything preventing this series from being applied , Tom ?
[PATCH v2 5/5] sandbox: Drop video-sync in serial driver
With sandbox, when U-Boot is waiting for input it syncs the video display, since presumably the user has finished typing. Now that cyclic is used for video syncing, we can drop this. Cyclic will automatically call the video_idle() function when idle. Signed-off-by: Simon Glass --- Changes in v2: - Fix 'groth' and 'work-around' typos in cover letter drivers/serial/sandbox.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index f6ac3d228526..be72fd2c65f7 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -139,8 +139,6 @@ static int sandbox_serial_pending(struct udevice *dev, bool input) return 0; os_usleep(100); - if (IS_ENABLED(CONFIG_VIDEO) && !IS_ENABLED(CONFIG_SPL_BUILD)) - video_sync_all(); avail = membuff_putraw(>buf, 100, false, ); if (!avail) return 1; /* buffer full */ -- 2.43.0.rc2.451.g8631bc7472-goog
[PATCH v2 4/5] sandbox: Increase cyclic CPU-time limit
Now that sandbox is using cyclic for video, it trips the 1us time limit. Updating the sandbox display often takes 20ms or more. Increase the limit to 100ms to avoid a warning. Signed-off-by: Simon Glass --- (no changes since v1) common/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/common/Kconfig b/common/Kconfig index 5906a4af7c33..f3f6f495adfb 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -636,6 +636,7 @@ config SPL_CYCLIC config CYCLIC_MAX_CPU_TIME_US int "Sets the max allowed time for a cyclic function in us" + default 10 if SANDBOX # sandbox video is quite slow default 1000 help The max allowed time for a cyclic function in us. If a functions -- 2.43.0.rc2.451.g8631bc7472-goog
[PATCH v2 3/5] video: Use cyclic to handle video sync
At present U-Boot flushes the cache after every character written to
ths display. This makes the command-line slower, to the point that
pasting in long strings can fail.
Add a cyclic function to sync the display every 10ms. Enable this by
default.
Allow much longer times for sandbox, since the SDL display is quite
slow.
Avoid size growth if the feature is disabled by making the new init and
destroy functions dependent on CYCLIC being enabled.
Signed-off-by: Simon Glass
---
Changes in v2:
- Expand help for CONFIG_VIDEO
drivers/video/Kconfig| 35 +++
drivers/video/video-uclass.c | 46
2 files changed, 77 insertions(+), 4 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6f319ba0d544..ccbfcbcfd824 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -7,6 +7,7 @@ menu "Graphics support"
config VIDEO
bool "Enable driver model support for LCD/video"
depends on DM
+ imply CYCLIC
help
This enables driver model for LCD and video devices. These support
a bitmap display of various sizes and depths which can be drawn on
@@ -14,6 +15,11 @@ config VIDEO
option compiles in the video uclass and routes all LCD/video access
through this.
+ If CYCLIC is enabled (which it is by default), the cyclic subsystem
+ is used to flush pending output to the display periodically, rather
+ than this happening with every chunk of output. This allows for more
+ efficient operation and faster display output.
+
if VIDEO
config VIDEO_FONT_4X6
@@ -231,6 +237,35 @@ config NO_FB_CLEAR
loads takes over the screen. This, for example, can be used to
keep splash image on screen until grub graphical boot menu starts.
+config VIDEO_SYNC_MS
+ int "Video-sync period in milliseconds for foreground processing"
+ default 300 if SANDBOX
+ default 100
+ help
+ This sets the requested, maximum time before a video sync will take
+ place, in milliseconds. Note that the time between video syncs
+ may be longer than this, since syncs only happen when the video system
+ is used, e.g. by outputting a character to the console.
+
+ It may also be shorter, since the video uclass will automatically
+ force a sync in certain situations.
+
+ Many video-output systems require a sync operation before any output
+ is visible. This may flush the CPU cache or perhaps copy the
+ display contents to a hardware framebuffer. Without this, change to
+ the video may never be displayed.
+
+config VIDEO_SYNC_CYCLIC_MS
+ int "Video-sync period in milliseconds for cyclic processing"
+ depends on CYCLIC
+ default 100 if SANDBOX
+ default 10
+ help
+ This sets the frequency of cyclic video syncs. The cyclic system is
+ used to ensure that when U-Boot is idle, it syncs the video. This
+ improves the responsiveness of the command line to new characters
+ being entered.
+
config PANEL
bool "Enable panel uclass support"
default y
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 600e12b802d1..fbed4ce861b4 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -53,6 +54,8 @@
*/
DECLARE_GLOBAL_DATA_PTR;
+struct cyclic_info;
+
/**
* struct video_uc_priv - Information for the video uclass
*
@@ -61,9 +64,11 @@ DECLARE_GLOBAL_DATA_PTR;
* available address to use for a device's framebuffer. It starts at
* gd->video_top and works downwards, running out of space when it hits
* gd->video_bottom.
+ * @cyc: handle for cyclic-execution function, or NULL if none
*/
struct video_uc_priv {
ulong video_ptr;
+ struct cyclic_info *cyc;
};
/** struct vid_rgb - Describes a video colour */
@@ -364,6 +369,10 @@ int video_sync(struct udevice *vid, bool force)
return ret;
}
+ if (CONFIG_IS_ENABLED(CYCLIC) && !force &&
+ get_timer(priv->last_sync) < CONFIG_VIDEO_SYNC_MS)
+ return 0;
+
/*
* flush_dcache_range() is declared in common.h but it seems that some
* architectures do not actually implement it. Is there a way to find
@@ -376,11 +385,10 @@ int video_sync(struct udevice *vid, bool force)
CONFIG_SYS_CACHELINE_SIZE));
}
#elif defined(CONFIG_VIDEO_SANDBOX_SDL)
- if (force || get_timer(priv->last_sync) > 100) {
- sandbox_sdl_sync(priv->fb);
- priv->last_sync = get_timer(0);
- }
+ sandbox_sdl_sync(priv->fb);
#endif
+ priv->last_sync = get_timer(0);
+
return 0;
}
@@ -525,10 +533,16 @@ int
[PATCH v2 1/5] cyclic: Add a symbol for SPL
The cyclic subsystem is currently enabled either in all build phases
or none. For tools this should not be enabled, but since lib/shc256.c
and other files include watchdog.h in the host build, we must make
sure that it is not enabled there.
Add an SPL symbol so that there is more control of this.
Add an include into cyclic.h so that tools can include this file.
Also add the kconfig.h header so that CONFIG_IS_ENABLED() works. We
could avoid this for now by moving the location of the watchdog.h
inclusion to outside the USE_HOSTCC area. But at some point the #ifdefs
from these files will likely be removed, so there is no benefit in
going that way.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add an SPL_CYCLIC symbol
- Add a lot more explanation about the header files
common/Kconfig| 8
common/Makefile | 2 +-
drivers/watchdog/Kconfig | 1 +
include/asm-generic/global_data.h | 2 +-
include/cyclic.h | 6 --
5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig
index 0283701f1d05..5906a4af7c33 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -626,6 +626,14 @@ config CYCLIC
if CYCLIC
+config SPL_CYCLIC
+ bool "General-purpose cyclic execution mechanism (SPL)"
+ help
+ This enables a general-purpose cyclic execution infrastructure in SPL,
+ to allow "small" (run-time wise) functions to be executed at
+ a specified frequency. Things like LED blinking or watchdog
+ triggering are examples for such tasks.
+
config CYCLIC_MAX_CPU_TIME_US
int "Sets the max allowed time for a cyclic function in us"
default 1000
diff --git a/common/Makefile b/common/Makefile
index 1495436d5d45..27443863bf9b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -77,7 +77,7 @@ obj-$(CONFIG_CROS_EC) += cros_ec.o
obj-y += dlmalloc.o
obj-$(CONFIG_$(SPL_TPL_)SYS_MALLOC_F) += malloc_simple.o
-obj-$(CONFIG_CYCLIC) += cyclic.o
+obj-$(CONFIG_$(SPL_TPL_)CYCLIC) += cyclic.o
obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o
obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 07fc4940e918..378435c55cc7 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -395,6 +395,7 @@ config WDT_ARM_SMC
config SPL_WDT
bool "Enable driver model for watchdog timer drivers in SPL"
depends on SPL_DM
+ select SPL_CYCLIC if CYCLIC
help
Enable driver model for watchdog timer in SPL.
This is similar to CONFIG_WDT in U-Boot.
diff --git a/include/asm-generic/global_data.h
b/include/asm-generic/global_data.h
index e8c6412e3f8d..77f11a4383c9 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -485,7 +485,7 @@ struct global_data {
*/
struct event_state event_state;
#endif
-#ifdef CONFIG_CYCLIC
+#if CONFIG_IS_ENABLED(CYCLIC)
/**
* @cyclic_list: list of registered cyclic functions
*/
diff --git a/include/cyclic.h b/include/cyclic.h
index 44ad3cb6b803..d3b368dd90df 100644
--- a/include/cyclic.h
+++ b/include/cyclic.h
@@ -11,6 +11,7 @@
#ifndef __cyclic_h
#define __cyclic_h
+#include
#include
#include
@@ -44,7 +45,8 @@ struct cyclic_info {
/** Function type for cyclic functions */
typedef void (*cyclic_func_t)(void *ctx);
-#if defined(CONFIG_CYCLIC)
+#if CONFIG_IS_ENABLED(CYCLIC)
+
/**
* cyclic_register - Register a new cyclic function
*
@@ -122,6 +124,6 @@ static inline int cyclic_unregister_all(void)
{
return 0;
}
-#endif
+#endif /* CYCLIC */
#endif
--
2.43.0.rc2.451.g8631bc7472-goog
[PATCH v2 2/5] video: Move last_sync to private data
Rather than using a static variable, use the video device's private
data to remember when the last video sync was completed. This allows
each display to have its own sync and avoids using static data in SPL.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/video/video-uclass.c | 10 +++---
include/video.h | 2 ++
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index f743ed74c818..600e12b802d1 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -354,6 +354,7 @@ void video_set_default_colors(struct udevice *dev, bool
invert)
/* Flush video activity to the caches */
int video_sync(struct udevice *vid, bool force)
{
+ struct video_priv *priv = dev_get_uclass_priv(vid);
struct video_ops *ops = video_get_ops(vid);
int ret;
@@ -369,20 +370,15 @@ int video_sync(struct udevice *vid, bool force)
* out whether it exists? For now, ARM is safe.
*/
#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
- struct video_priv *priv = dev_get_uclass_priv(vid);
-
if (priv->flush_dcache) {
flush_dcache_range((ulong)priv->fb,
ALIGN((ulong)priv->fb + priv->fb_size,
CONFIG_SYS_CACHELINE_SIZE));
}
#elif defined(CONFIG_VIDEO_SANDBOX_SDL)
- struct video_priv *priv = dev_get_uclass_priv(vid);
- static ulong last_sync;
-
- if (force || get_timer(last_sync) > 100) {
+ if (force || get_timer(priv->last_sync) > 100) {
sandbox_sdl_sync(priv->fb);
- last_sync = get_timer(0);
+ priv->last_sync = get_timer(0);
}
#endif
return 0;
diff --git a/include/video.h b/include/video.h
index 4d8df9baaada..4013a949983f 100644
--- a/include/video.h
+++ b/include/video.h
@@ -97,6 +97,7 @@ enum video_format {
* the LCD is updated
* @fg_col_idx:Foreground color code (bit 3 = bold, bit 0-2 = color)
* @bg_col_idx:Background color code (bit 3 = bold, bit 0-2 = color)
+ * @last_sync: Monotonic time of last video sync
*/
struct video_priv {
/* Things set up by the driver: */
@@ -121,6 +122,7 @@ struct video_priv {
bool flush_dcache;
u8 fg_col_idx;
u8 bg_col_idx;
+ ulong last_sync;
};
/**
--
2.43.0.rc2.451.g8631bc7472-goog
[PATCH v2 0/5] video: Improve syncing performance with cyclic
Now that U-Boot has a background-processing feature, it is possible to reduce the amount of 'foreground' syncing of the display. At present this happens quite often. Foreground syncing blocks all other processing, sometimes for 10ms or more. When pasting commands into U-Boot over the UART, this typically result in characters being dropped. For example, on rpi_4 it isn't possible to paste in more than 35 characters before things fail. This makes updating the environment or entering long commands very painful over the console, since text must be pasted in chunks, or the vidconsole device must be dropped from stdout. This series introduces background syncing, enabled by default for boards which use video. The sync rates for foreground and background are configurable. With this series it is possible to paste in any amount of text to the command line. Some sandbox-specific workarounds can now be removed and sandbox video (./u-boot -Dl) is significantly more responsive. This obviously increases code size, since it enables a subsystem not normally used by default. However it only applies to boards which have VIDEO enabled, which are presumably less worried about memory space since the video code is fairly large. Also it is possible to disable CMD_CYCLIC and reduce the growth to: aarch64: (for 1/1 boards) all +1081.0 rodata +65.0 text +1016.0 arm: (for 1/1 boards) all +945.0 rodata +65.0 text +880.0 Without that, the increase doubles. It is of course possible to disable CYCLIC and still use VIDEO but this reverts to the current behaviour Changes in v2: - Add an SPL_CYCLIC symbol - Add a lot more explanation about the header files - Expand help for CONFIG_VIDEO - Fix 'groth' and 'work-around' typos in cover letter Simon Glass (5): cyclic: Add a symbol for SPL video: Move last_sync to private data video: Use cyclic to handle video sync sandbox: Increase cyclic CPU-time limit sandbox: Drop video-sync in serial driver common/Kconfig| 9 ++ common/Makefile | 2 +- drivers/serial/sandbox.c | 2 -- drivers/video/Kconfig | 35 + drivers/video/video-uclass.c | 52 +-- drivers/watchdog/Kconfig | 1 + include/asm-generic/global_data.h | 2 +- include/cyclic.h | 6 ++-- include/video.h | 2 ++ 9 files changed, 96 insertions(+), 15 deletions(-) -- 2.43.0.rc2.451.g8631bc7472-goog
[PATCH v2] net: designware: Support high memory nodes
Some platforms (such as the Lichee Pi 4A) have their dwmac device
addressable only in high memory space. Storing the node's base address
on 32 bits is not possible in such case.
Use platform's physical address type to store the base address.
Signed-off-by: Nils Le Roux
Cc: Andre Przywara
---
Changes in v2:
- explicitly define and handle ioaddr as a virtual address
- use an intermediate variable to assign the PCI base address
- use appropriate placeholder to print physical addresses
---
drivers/net/designware.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index a174344b3e..78bfce07f6 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -678,8 +678,8 @@ int designware_eth_probe(struct udevice *dev)
{
struct eth_pdata *pdata = dev_get_plat(dev);
struct dw_eth_dev *priv = dev_get_priv(dev);
- u32 iobase = pdata->iobase;
- ulong ioaddr;
+ phys_addr_t iobase = pdata->iobase;
+ void *ioaddr;
int ret, err;
struct reset_ctl_bulk reset_bulk;
#ifdef CONFIG_CLK
@@ -740,16 +740,18 @@ int designware_eth_probe(struct udevice *dev)
* or via a PCI bridge, fill in plat before we probe the hardware.
*/
if (IS_ENABLED(CONFIG_PCI) && device_is_on_pci_bus(dev)) {
- dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0, );
- iobase &= PCI_BASE_ADDRESS_MEM_MASK;
- iobase = dm_pci_mem_to_phys(dev, iobase);
+ u32 pcibase;
+ dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0, );
+ pcibase &= PCI_BASE_ADDRESS_MEM_MASK;
+
+ iobase = dm_pci_mem_to_phys(dev, pcibase);
pdata->iobase = iobase;
pdata->phy_interface = PHY_INTERFACE_MODE_RMII;
}
- debug("%s, iobase=%x, priv=%p\n", __func__, iobase, priv);
- ioaddr = iobase;
+ debug("%s, iobase=%pa, priv=%p\n", __func__, , priv);
+ ioaddr = phys_to_virt(iobase);
priv->mac_regs_p = (struct eth_mac_regs *)ioaddr;
priv->dma_regs_p = (struct eth_dma_regs *)(ioaddr + DW_DMA_BASE_OFFSET);
priv->interface = pdata->phy_interface;
--
2.43.0
