Re: [PATCH V3] dm: core: Add late driver remove option
On 11/21/20 9:13 PM, Marek Vasut wrote: On 11/22/20 12:07 AM, Simon Glass wrote: [...] That way we are describing the property of the device rather than what we want to do with it. The device is not critical or vital, it just needs to be torn down late. What is it about the device that requires it to be torn down 'late'? I think perhaps the problem isn't that it needs to be "late", it's that it has perhaps not obviously described children. Which gets back to what you just said as well about "later" and "fairly late". It's an ordering problem. Yes it is. We currently don't record devices that depend on others. It would be possible to add a refcount to DM to cope with this and implement it for clocks. I wonder if that might be better than what we have here? This is still a bootloader, not a general-purpose OS, so I would argue we should not complicate this more than is necessary. The DM already addsa lot of bloat to U-Boot, no need to make that worse unless there is a real good reason for that. Also, in V1 of this patch, Simon did suggest that a simple approach is OK if I recall correctly. FWIW the clock subsystem already does refcounting of clocks (though it is for struct clk and not for the devices themselves). However, I think it would be difficult to use those counts to determine when the clock driver is no longer needed. For example, some devices (such as CPUs) may use clocks, but should never stop those clocks. A quick-and-dirty method could be to create a linked list of all probed devices, and then remove devices in reverse order. So an example call sequence could be ... 1. mmc's probe() 2. clk_get_*() 3. clk's probe() 4. clk's finishes; clk added to probed list 5. mmc's probe finishes; mmc added to probed list ... 7. mmc's remove() called 8. clk's remove() called Note that this would be a change to dm_uninit, not device_remove; we would still need the recursive removal logic for when devices are removed via other means. Unfortunately, this method would require yet another list_head in udevice, so perhaps it should just be an option? --Sean
Re: Pull request for UEFI sub-system for efi-2021-01-rc3 (2)
On Sat, Nov 21, 2020 at 01:55:11PM +0100, Heinrich Schuchardt wrote: > Dear Tom, > > The following changes since commit b80680633dc954d32f81f3afacd3d1f2f3d290b0: > > Merge branch '2020-11-18-assorted-fixes' (2020-11-19 10:23:50 -0500) > > are available in the Git repository at: > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git > tags/efi-2021-01-rc3-2 > > for you to fetch changes up to 7e5875a85689d762bde58a587a7d531667358ee4: > > efi_loader: parameter check in GetNextVariableName() (2020-11-21 > 07:26:16 +0100) > > Gilab CI reported no problems: > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/5384 > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH V3] dm: core: Add late driver remove option
On 11/22/20 12:07 AM, Simon Glass wrote: [...] That way we are describing the property of the device rather than what we want to do with it. The device is not critical or vital, it just needs to be torn down late. What is it about the device that requires it to be torn down 'late'? I think perhaps the problem isn't that it needs to be "late", it's that it has perhaps not obviously described children. Which gets back to what you just said as well about "later" and "fairly late". It's an ordering problem. Yes it is. We currently don't record devices that depend on others. It would be possible to add a refcount to DM to cope with this and implement it for clocks. I wonder if that might be better than what we have here? This is still a bootloader, not a general-purpose OS, so I would argue we should not complicate this more than is necessary. The DM already adds a lot of bloat to U-Boot, no need to make that worse unless there is a real good reason for that. Also, in V1 of this patch, Simon did suggest that a simple approach is OK if I recall correctly.
[PATCH v3] Add optional salt to AUTOBOOT_STOP_STR_SHA256
Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256. If a string
followed by a ":" is prepended to the sha256, the portion to the left
of the colon will be used as a salt and the password will be appended
to the salt before the sha256 is computed and compared.
Signed-off-by: Joel Peshkin
Cc: Simon Glass
Cc: Bin Meng
Cc: Patrick Delaunay
Cc: Heiko Schocher
Cc: Heinrich Schuchardt
Cc: Joel Peshkin
To: u-boot@lists.denx.de
---
Changes for v2:
- Increase MAX_DELAY_STOP_STR
- Check salt size against MAX_DELAY_STOP_STR before copying
- Minor cleanup
Changes for v3:
- Cleanup changing (c) to c after review feedback
---
common/Kconfig.boot | 5 -
common/autoboot.c | 12 ++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 3f6d9c1..8a98672 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
This option adds the feature to only stop the autobooting,
and therefore boot into the U-Boot prompt, when the input
string / password matches a values that is encypted via
- a SHA256 hash and saved in the environment.
+ a SHA256 hash and saved in the environment variable
+ "bootstopkeysha256". If the value in that variable
+ includes a ":", the portion prior to the ":" will be treated
+ as a salt value.
config AUTOBOOT_USE_MENUKEY
bool "Allow a specify key to run a menu from the environment"
diff --git a/common/autoboot.c b/common/autoboot.c
index e628baf..ddb6246 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -25,7 +25,7 @@
DECLARE_GLOBAL_DATA_PTR;
-#define MAX_DELAY_STOP_STR 32
+#define MAX_DELAY_STOP_STR 64
#ifndef DEBUG_BOOTKEYS
#define DEBUG_BOOTKEYS 0
@@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
u8 sha_env[SHA256_SUM_LEN];
u8 *sha;
char *presskey;
+ char *c;
const char *algo_name = "sha256";
u_int presskey_len = 0;
int abort = 0;
@@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
if (sha_env_str == NULL)
sha_env_str = AUTOBOOT_STOP_STR_SHA256;
+ presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
+ c = strstr(sha_env_str, ":");
+ if (c && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
+ /* preload presskey with salt */
+ memcpy(presskey, sha_env_str, c - sha_env_str);
+ presskey_len = c - sha_env_str;
+ sha_env_str = c + 1;
+ }
/*
* Generate the binary value from the environment hash value
* so that we can compare this value with the computed hash
@@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
return 0;
}
- presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
sha = malloc_cache_aligned(SHA256_SUM_LEN);
size = SHA256_SUM_LEN;
/*
--
1.8.3.1
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH 1/3] test: cmd_ut_category: raise a error when the test is not found
On Thu, 19 Nov 2020 at 03:09, Patrick Delaunay wrote: > > Raise an error when test is not found, for example with manual test > with bad test name, as following, doesn't raise an error > > => ut lib bad > Failures: 0 > > After the patch: > > => ut lib bad > lib test bad not found > Failures: 1 > > This patch allows also to detect tests which don't respect the expected > format with "prefix" used in cmd_ut_category and defined in ut_subtest > (./test/py/conftest.py). When I execute "make qcheck" this patch detects > 2 issues, corrected by the 2 next patches. > > Signed-off-by: Patrick Delaunay > --- > > test/cmd_ut.c | 7 +++ > 1 file changed, 7 insertions(+) Reviewed-by: Simon Glass
Re: binman: u-boot-dtb vs. filename
Hi Jan,
On Sat, 21 Nov 2020 at 07:28, Jan Kiszka wrote:
>
> Hi,
>
> I stumbled over README.entries claiming
>
> Entry: u-boot-dtb: U-Boot device tree
> -
>
> Properties / Entry arguments:
> - filename: Filename of u-boot.dtb (default 'u-boot.dtb')
>
>
> However,
>
> u-boot-dtb {
> filename = "foo.dtb"
> }
>
> only pulls u-boot.dtb. Tried to fix that but I failed to understand the
> binman logic. Maybe the documentation (also that of
> u-boot-dtb-with-ucode) was just not supposed to refer to filename.
>
> Using 'blob' now, like other boards.
This was by design, since it is trying to pick up a particular .dtb
file, but as you point out the docs do not match. I wonder if using
'blob-dtb' as the entry type would work?
The reason is that Entry_u_boot_dtb has a GetDefaultFilename()
function and that ignores the filename property.
I suspect that changing it to:
def GetDefaultFilename(self):
return self._filename or 'u-boot.dtb'
might work too. I will have a think about which fix is best and see if
I can add a test to ftest.py
Regards,
Simon
Re: [PATCH v2] Add optional salt to AUTOBOOT_STOP_STR_SHA256
On Fri, 20 Nov 2020 at 12:05, Joel Peshkin wrote:
>
> From: Joel Peshkin
>
> Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256. If a string
> followed by a ":" is prepended to the sha256, the portion to the left
> of the colon will be used as a salt and the password will be appended
> to the salt before the sha256 is computed and compared.
>
> Signed-off-by: Joel Peshkin
> Cc: Simon Glass
> Cc: Bin Meng
> Cc: Patrick Delaunay
> Cc: Heiko Schocher
> Cc: tr...@konsulko.com
> Cc: Heinrich Schuchardt
> Cc: Joel Peshkin
> To: u-boot@lists.denx.de
>
> ---
> Changes for v2:
>- Increase MAX_DELAY_STOP_STR
>- Check salt size against MAX_DELAY_STOP_STR before copying
>- Minor cleanup
> ---
> common/Kconfig.boot | 5 -
> common/autoboot.c | 12 ++--
> 2 files changed, 14 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass
Please see below
>
> diff --git a/common/Kconfig.boot b/common/Kconfig.boot
> index 3f6d9c1..8a98672 100644
> --- a/common/Kconfig.boot
> +++ b/common/Kconfig.boot
> @@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
> This option adds the feature to only stop the autobooting,
> and therefore boot into the U-Boot prompt, when the input
> string / password matches a values that is encypted via
> - a SHA256 hash and saved in the environment.
> + a SHA256 hash and saved in the environment variable
> + "bootstopkeysha256". If the value in that variable
> + includes a ":", the portion prior to the ":" will be treated
> + as a salt value.
>
> config AUTOBOOT_USE_MENUKEY
> bool "Allow a specify key to run a menu from the environment"
> diff --git a/common/autoboot.c b/common/autoboot.c
> index e628baf..982b561 100644
> --- a/common/autoboot.c
> +++ b/common/autoboot.c
> @@ -25,7 +25,7 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -#define MAX_DELAY_STOP_STR 32
> +#define MAX_DELAY_STOP_STR 64
>
> #ifndef DEBUG_BOOTKEYS
> #define DEBUG_BOOTKEYS 0
> @@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
> u8 sha_env[SHA256_SUM_LEN];
> u8 *sha;
> char *presskey;
> + char *c;
> const char *algo_name = "sha256";
> u_int presskey_len = 0;
> int abort = 0;
> @@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
> if (sha_env_str == NULL)
> sha_env_str = AUTOBOOT_STOP_STR_SHA256;
>
> + presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
> + c = strstr(sha_env_str, ":");
> + if ((c) && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
Use c instead of (c)
> + /* preload presskey with salt */
> + memcpy(presskey, sha_env_str, c - sha_env_str);
> + presskey_len = c - sha_env_str;
> + sha_env_str = c + 1;
> + }
> /*
> * Generate the binary value from the environment hash value
> * so that we can compare this value with the computed hash
> @@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
> return 0;
> }
>
> - presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
> sha = malloc_cache_aligned(SHA256_SUM_LEN);
> size = SHA256_SUM_LEN;
> /*
> --
> 1.8.3.1
>
Re: [PATCH] sandbox: remove ram buffer file when U-Boot is loaded by SPL
On Fri, 20 Nov 2020 at 02:48, Patrick Delaunay wrote: > > Update management of "--rm_memory" sandbox's option and force > this option when U-Boot is loaded by SPL in os_spl_to_uboot() > and remove the ram file after reading in main() as described > in option help message: "Remove memory file after reading". > > This patch avoids that the file "/tmp/u-boot.mem.XX" [created in > os_jump_to_file() when U-Boot is loaded by SPL] is never deleted > because state_uninit() is not called after U-Boot execution > (CtrlC or with running pytest for example). > > This issue is reproduced by > > build-sandbox_spl/spl/u-boot-spl > and CtrlC in U-Bot console > > > make qcheck > > One temp file is created after each SPL and U-Boot execution > (7 tims in qcheck after test_handoff.py, test_ofplatdata.py, > test_spl.py execution). > > Signed-off-by: Patrick Delaunay > --- > > arch/sandbox/cpu/os.c| 5 + > arch/sandbox/cpu/start.c | 7 +++ > arch/sandbox/cpu/state.c | 4 > 3 files changed, 12 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 3/3] test: correct the test prefix in ut str
On Thu, 19 Nov 2020 at 03:09, Patrick Delaunay wrote:
>
> Align the prefix used in cmd_ut_category function and name of tests
> for ut str.
> This patch solves the issues detected by "make qcheck" after previous
> patch.
>
> Fixes: fdc79a6b125d ("lib: Add a function to convert a string to upper case")
> Signed-off-by: Patrick Delaunay
> ---
>
> test/str_ut.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass
Re: [PATCH 2/3] test: correct the test prefix in ut cmd_mem
On Thu, 19 Nov 2020 at 03:09, Patrick Delaunay wrote:
>
> Align the prefix used in cmd_ut_category function and name of tests
> for ut mem.
> This patch solves the issues detected by "make qcheck" after previous
> patch.
>
> Fixes: 550a9e7902ce ("cmd: Update the memory-search command")
> Signed-off-by: Patrick Delaunay
> ---
>
> test/cmd/mem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Simon Glass
Re: [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
Hi,
On Wed, 18 Nov 2020 at 10:55, Vignesh Raghavendra wrote:
>
>
>
> On 11/18/20 8:44 PM, Aswath Govindraju wrote:
> > Hi Simon,
> >
> > On 18/11/20 8:07 pm, Simon Glass wrote:
> >> Hi Aswath,
> >>
> >> On Mon, 16 Nov 2020 at 07:29, Aswath Govindraju
> >> wrote:
> >>>
> >>> While assigning the sequence number to subsystem instances by reading the
> >>> aliases property, only DT nodes names are compared and not the complete
> >>> path. This causes a problem when there are two DT nodes with same name but
> >>> have different paths.
> >>>
> >>> Fix it by comparing the phandles of DT nodes after the node names match.
> >>>
> >>> Signed-off-by: Aswath Govindraju
> >>> ---
> >>>
> >>> Resending this patch as it was held awaiting for moderator approval
> >>> because
> >>> patch was sent by non-member.
> >>>
> >>> lib/fdtdec.c | 5 +
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> >>> index 2015907dee7d..9e1bfe0b519e 100644
> >>> --- a/lib/fdtdec.c
> >>> +++ b/lib/fdtdec.c
> >>> @@ -478,6 +478,11 @@ int fdtdec_get_alias_seq(const void *blob, const
> >>> char *base, int offset,
> >>> slash = strrchr(prop, '/');
> >>> if (strcmp(slash + 1, find_name))
> >>> continue;
> >>> +
> >>> + if (fdt_get_phandle(blob, offset) !=
> >>> + fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
> >>> + continue;
> >>
> >> The call to fdt_path_offset() is very slow. Perhaps we can do this
> >> check only with livetree? What situation is causing a problem for you?
> >> What are the node / alias names?
> >
> > In the case of live tree for getting the sequence number the node
> > pointers are compared. So, I don't think this problem would come up.
> >
> > As for the use case,
> >
> > In AM654 Device tree there are two instances of USB controllers and both
> > the controller nodes have the same name usb@1
> >
> > If dfu is performed through the port connected to second controller.
> > Then based on the dr_mode of first controller the instance number to be
> > used in dfu command will vary. In order to make the instance number for
> > dfu command to be independent, aliases can be used(If aliases are
> > defined then the sequence number is assigned as the alias number.).
> >
> > The problem with current method for acquiring sequence number using
> > aliases is that only the name of the node is compared with node name
> > from the aliases property. So in the above case both the controllers
> > will have the same name. This leads to the first alias number being used
> > for the both the controllers to assign sequence number.
> >
> >
> > aliases {
> > serial2 = _uart0;
> > ethernet0 = _port1;
> > usb0 = // This property being used to
> > //alias both the controllers
> > usb1 =
> > };
>
>
> To explain a bit more, here is the DT snippet around usb0 and usb1
>
> dwc3_0: dwc3@400 {
> compatible = "ti,am654-dwc3";
> reg = <0x0 0x400 0x0 0x4000>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x400 0x2>;
> ...
>
> usb0: usb@1 {
> compatible = "snps,dwc3";
> reg = <0x1 0x1>;
> ...
> };
> };
>
> dwc3_1: dwc3@402 {
> compatible = "ti,am654-dwc3";
> reg = <0x0 0x402 0x0 0x4000>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x402 0x2>;
> ...
>
> usb1: usb@1 {
> compatible = "snps,dwc3";
> reg = <0x1 0x1>;
> ...
> };
> };
>
> In above case, (with CONFIG_OF_LIVE disabled),
> fdtdec_get_alias_seq() fails to pick the correct instance for USB
> controller for a given index. This is because fdtdec_get_alias_seq()
> only compares the leaf node name (usb@1) with alias path and thus
> both usb instances match to usb0.
>
> >
> > So, to distinguish nodes with same name, phandles can be used while
> > assigning sequence numbers.
Thank you both for the detai. I understand it and in fact I think this
has come up before.
Would it be OK to use livetree?
If not, can we put this code behind a Kconfig so the extra time
penalty is only incurred on boards that need it?
Regards,
Simon
Re: [PATCH v2] time: Fix get_ticks being non-monotonic
Hi Michael, On Thu, 19 Nov 2020 at 04:23, Michael Opdenacker wrote: > > Hi, > > Sorry, no messaging quoting, I was not subscribed to the list at that time. > > Merging this change into master actually broke the SPL on > Atmel/Microchip SAMA5D3, at least booting from MMC: > > RomBOOT > > > Could not initialize timer (err -11) > > Could not initialize timer (err -11) > > Could not initialize timer (err -11) > ... > > I'll look for a fix, but suggestions are welcome! The board might need CONFIG_TIMER_EARLY, but otherwise I think it is a case of figuring out why the timer is used before it is available. You can use dm_dump_all() to print out available devices and whether they are probed. Regards, Simon
Re: [PATCH 1/1] fs: fat: avoid NULL dereference when root dir is full
On Thu, 19 Nov 2020 at 05:42, Heinrich Schuchardt wrote:
>
> When trying to create a file in the full root directory of a FAT32
> filesystem a NULL dereference can be observed.
>
> When the root directory of a FAT16 filesystem is full fill_dir_slot() must
> return -1 to signal that a new directory entry could not be allocated.
>
> Fixes: cd2d727fff7e ("fs: fat: allocate a new cluster for root directory of
> fat32")
> Signed-off-by: Heinrich Schuchardt
> ---
> fs/fat/fat_write.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass
Re: [PATCH 4/5] env: allow default environment to be amended from control dtb
Hi Wolfgang, On Fri, 20 Nov 2020 at 04:28, Wolfgang Denk wrote: > > Dear Simon, > > In message > you > wrote: > > > > Some years ago I did a series to allow the environment to come from a > > text file, thus avoiding the \0 stuff. > > "env import -t" does that, you know? Not to derail the discussion, but I mean as a way of specifying the default environment. > > > Now binman has a 'u-boot-env' > > entry type, allowing creating an environment from a text file, with > > suitable checksumming. > > "env import -b" ... For the binman case, this is an environment area which is loaded by U-Boot on boot, so does not need any such command. > > > There is some advantage to having a default environment compiled into > > U-Boot that covers everything needed to boot. For one, the environment > > can be clobbered from userspace, which would otherwise render the > > device unbootable. For another, it is more secure to avoid loading > > unsigned data (the environment) from flash. Generally, for a secure > > boot, one would need to avoid loading the environment, at least > > without a lot of careful filtering. > > One idea behind my rewrite of the environment handling (when I added > hast table support) was that there should be more than one way to > initialize the environment. Until then, we always had exactly one > fixed location for the environment, probaly with a redundant copy. > > The code we have now actuially allows for a much greater > flexibility. You can initialize the environment from a selection of > copies, and (now, with proper driver support) also from several > devices. If doen correctly, we could implement things like > "profiles", where for example each user (or use case) can select his > specific profile, initialize the environment from that, and save > change to that. Ths could - for example - be used to switch between > "development" and "production" modes. A "reset to factory defaults" > would then just be an import from the (read-only) factory-defaults > copy. etc. > > Importing from a DT is just a logical extension as it is considered > just another storage device / driver. [In a Unix environment, all > these would just be "files".] > > It's all there. We just have to use it. OK I'll let you figure this out with Rasmus. Regards, Simon
Re: [PATCH V3] dm: core: Add late driver remove option
Hi Tom, On Wed, 18 Nov 2020 at 08:53, Tom Rini wrote: > > On Wed, Nov 18, 2020 at 07:37:27AM -0700, Simon Glass wrote: > > Hi Marek, > > > > On Sun, 15 Nov 2020 at 06:19, Marek Vasut wrote: > > > > > > On 11/9/20 1:21 AM, Simon Glass wrote: > > > > Hi Marek, > > > > > > [...] > > > > > > >> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > > >> index 1206e306db..f9091a3d41 100644 > > > >> --- a/arch/arm/lib/bootm.c > > > >> +++ b/arch/arm/lib/bootm.c > > > >> @@ -120,6 +120,7 @@ static void announce_and_cleanup(int fake) > > > >> * of DMA operation or releasing device internal buffers. > > > >> */ > > > >> dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); > > > >> + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL | DM_REMOVE_LATE); > > > > > > > > Please see my previous comments about the naming and semantics. I'm > > > > repeating them here: > > > > > > > > Firstly I think we should use a different name. 'Late' doesn't really > > > > tell me anything. > > > > > > > > If I understand it correctly, this is supposed to be after OS_PREPARE > > > > but before booting the OS? > > > > > > No. The drivers which are marked as remove-late should be removed late, > > > after all the normal drivers were removed. The example in the commit > > > message explains where this is needed -- first remove mmc driver to set > > > eMMC out of HS mode and change the bus clock and after that remove clock > > > driver ; the clock driver is still required during the removal of the > > > mmc driver. > > > > > > > I think we need to separate the flag names as they are too similar. > > > > > > > > I think DM_FLAG_BASIC and DM_REMOVE_NON_BASIC would be better (or some > > > > term that explains that this is a device used by many others) > > > > > > > > If BASIC is too terrible, perhaps CORE, or VITAL, or PRIME or CRITICAL > > > > or something like that? > > > > > > This makes no sense to me. > > > > I don't want the word 'late'. Then we'll end up with 'later' and > > 'fairly late', etc. and it will get out of hand. We need a word that > > describes what is special about the devices, not when to do stuff with > > them. > > > > > > > > > That way we are describing the property of the device rather than what > > > > we want to do with it. > > > > > > The device is not critical or vital, it just needs to be torn down late. > > > > What is it about the device that requires it to be torn down 'late'? > > I think perhaps the problem isn't that it needs to be "late", it's that > it has perhaps not obviously described children. Which gets back to > what you just said as well about "later" and "fairly late". It's an > ordering problem. Yes it is. We currently don't record devices that depend on others. It would be possible to add a refcount to DM to cope with this and implement it for clocks. I wonder if that might be better than what we have here? Regards, Simon
[PATCH] Nokia RX-51: Convert to CONFIG_DM_MMC
Move twl4030_power_mmc_init() from board_mmc_power_init() to misc_init_r()
and disable CONFIG_SYS_MALLOC_F. Otherwise U-Boot cannot initialize MMC.
Also disable CONFIG_CMD_SLEEP and CONFIG_DM_DEVICE_REMOVE to free some
space.
Signed-off-by: Pali Rohár
---
board/nokia/rx51/rx51.c | 32
configs/nokia_rx51_defconfig | 4
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
index 2dd41604c9..7390e5be65 100644
--- a/board/nokia/rx51/rx51.c
+++ b/board/nokia/rx51/rx51.c
@@ -415,6 +415,8 @@ int misc_init_r(void)
/* initialize twl4030 power managment */
twl4030_power_init();
+ twl4030_power_mmc_init(0);
+ twl4030_power_mmc_init(1);
/* set VSIM to 1.8V */
twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VSIM_DEDICATED,
@@ -686,22 +688,18 @@ int rx51_kp_getc(struct stdio_dev *sdev)
return keybuf[keybuf_head++];
}
-/*
- * Routine: board_mmc_init
- * Description: Initialize mmc devices.
- */
-int board_mmc_init(struct bd_info *bis)
-{
- omap_mmc_init(0, 0, 0, -1, -1);
- omap_mmc_init(1, 0, 0, -1, -1);
- return 0;
-}
+static const struct mmc_config rx51_mmc_cfg = {
+ .host_caps = MMC_MODE_4BIT | MMC_MODE_HS_52MHz | MMC_MODE_HS,
+ .f_min = 40,
+ .f_max = 5200,
+ .b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
+ .voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
+};
-void board_mmc_power_init(void)
-{
- twl4030_power_mmc_init(0);
- twl4030_power_mmc_init(1);
-}
+static const struct omap_hsmmc_plat rx51_mmc[] = {
+ { rx51_mmc_cfg, (struct hsmmc *)OMAP_HSMMC1_BASE },
+ { rx51_mmc_cfg, (struct hsmmc *)OMAP_HSMMC2_BASE },
+};
static const struct omap_i2c_platdata rx51_i2c[] = {
{ I2C_BASE1, 10, OMAP_I2C_REV_V1 },
@@ -709,7 +707,9 @@ static const struct omap_i2c_platdata rx51_i2c[] = {
{ I2C_BASE3, 10, OMAP_I2C_REV_V1 },
};
-U_BOOT_DEVICES(rx51_i2c) = {
+U_BOOT_DEVICES(rx51) = {
+ { "omap_hsmmc", _mmc[0] },
+ { "omap_hsmmc", _mmc[1] },
{ "i2c_omap", _i2c[0] },
{ "i2c_omap", _i2c[1] },
{ "i2c_omap", _i2c[2] },
diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
index 6310a12aaa..0f05fe6fc3 100644
--- a/configs/nokia_rx51_defconfig
+++ b/configs/nokia_rx51_defconfig
@@ -29,6 +29,7 @@ CONFIG_CMD_BOOTMENU=y
# CONFIG_CMD_SAVEENV is not set
# CONFIG_CMD_ENV_EXISTS is not set
# CONFIG_CMD_FLASH is not set
+# CONFIG_CMD_SLEEP is not set
CONFIG_CMD_GPIO=y
CONFIG_CMD_I2C=y
CONFIG_CMD_MMC=y
@@ -62,3 +63,6 @@ CONFIG_SPLASH_SCREEN=y
# CONFIG_GZIP is not set
CONFIG_DM=y
CONFIG_DM_I2C=y
+CONFIG_DM_MMC=y
+# CONFIG_DM_DEVICE_REMOVE is not set
+# CONFIG_SYS_MALLOC_F is not set
--
2.20.1
[PATCH] Nokia RX-51: Decrease i2c speed to 100000
It looks like that i2c bus lot of times timeout on some units. Prior
migration to CONFIG_DM_I2C i2c speed was set to CONFIG_SYS_OMAP24_I2C_SPEED
value which was 10. Lower speed fixes timeout problems, so change speed
back to its previous value.
Signed-off-by: Pali Rohár
Fixes: 8d8c18170325 ("Nokia RX-51: Convert to CONFIG_DM_I2C")
---
Please include this patch into U-Boot master branch for 2020.01 release
to have i2c bus working.
---
board/nokia/rx51/rx51.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
index 3d62b5d9ad..2dd41604c9 100644
--- a/board/nokia/rx51/rx51.c
+++ b/board/nokia/rx51/rx51.c
@@ -704,9 +704,9 @@ void board_mmc_power_init(void)
}
static const struct omap_i2c_platdata rx51_i2c[] = {
- { I2C_BASE1, 220, OMAP_I2C_REV_V1 },
+ { I2C_BASE1, 10, OMAP_I2C_REV_V1 },
{ I2C_BASE2, 10, OMAP_I2C_REV_V1 },
- { I2C_BASE3, 40, OMAP_I2C_REV_V1 },
+ { I2C_BASE3, 10, OMAP_I2C_REV_V1 },
};
U_BOOT_DEVICES(rx51_i2c) = {
--
2.20.1
[PATCH 1/1] efi_loader: enable EFI_SET_TIME on sandbox and QEMU ARM
Enable EFI_SET_TIME on the sandbox and QEMU ARM to ensure that we compile and test the relevant code. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 29ea14b2ee..7fd3a3c90c 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -88,6 +88,7 @@ config EFI_GET_TIME config EFI_SET_TIME bool "SetTime() runtime service" depends on EFI_GET_TIME + default y if ARCH_QEMU || SANDBOX default n help Provide the SetTime() runtime service at boottime. This service -- 2.29.2
Re: [PATCH v9 04/11] efi_loader: capsule: support firmware update
hi Takahiro,
On Tue, 17 Nov 2020 at 05:58, AKASHI Takahiro
wrote:
> A capsule tagged with the guid, EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID,
> is handled as a firmware update object.
> What efi_update_capsule() basically does is to load any firmware management
> protocol (or fmp) drivers contained in a capsule, find out an appropriate
> fmp driver and then invoke its set_image() interface against each binary
> in a capsule.
> In this commit, however, loading drivers is not supported.
>
> The result of applying a capsule is set to be stored in "Capsule"
> variable, but its implementation is deferred to a fmp driver.
>
> Signed-off-by: AKASHI Takahiro
> ---
> include/efi_api.h| 129 +++
> include/efi_loader.h | 2 +
> lib/efi_loader/Kconfig | 8 ++
> lib/efi_loader/efi_capsule.c | 238 ++-
> lib/efi_loader/efi_setup.c | 4 +
> 5 files changed, 380 insertions(+), 1 deletion(-)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 7a2a087c60ed..966bc6e590bf 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -217,6 +217,9 @@ enum efi_reset_type {
> #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE0x0002
> #define CAPSULE_FLAGS_INITIATE_RESET 0x0004
>
> +#define CAPSULE_SUPPORT_AUTHENTICATION 0x0001
> +#define CAPSULE_SUPPORT_DEPENDENCY 0x0002
> +
> #define EFI_CAPSULE_REPORT_GUID \
> EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
> 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
> @@ -225,6 +228,10 @@ enum efi_reset_type {
> EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
> 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
>
> +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \
> + EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
> +0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
> +
> struct efi_capsule_header {
> efi_guid_t capsule_guid;
> u32 header_size;
> @@ -253,6 +260,33 @@ struct efi_memory_range_capsule {
> struct efi_memory_range memory_ranges[];
> } __packed;
>
> +struct efi_firmware_management_capsule_header {
> + u32 version;
> + u16 embedded_driver_count;
> + u16 payload_item_count;
> + u64 item_offset_list[];
> +} __packed;
> +
> +struct efi_firmware_management_capsule_image_header {
> + u32 version;
> + efi_guid_t update_image_type_id;
> + u8 update_image_index;
> + u8 reserved[3];
> + u32 update_image_size;
> + u32 update_vendor_code_size;
> + u64 update_hardware_instance;
> + u64 image_capsule_support;
> +} __packed;
> +
> +struct efi_capsule_result_variable_fmp {
> + u16 version;
> + u8 payload_index;
> + u8 update_image_index;
> + efi_guid_t update_image_type_id;
> + // u16 capsule_file_name[];
> + // u16 capsule_target[];
> +} __packed;
> +
>
> +/**
> + * efi_capsule_update_firmware - update firmware from capsule
> + * @capsule_data: Capsule
> + *
> + * Update firmware, using a capsule, @capsule_data. Loading any FMP
> + * drivers embedded in a capsule is not supported.
> + *
> + * Return: status code
> + */
> +static efi_status_t efi_capsule_update_firmware(
> + struct efi_capsule_header *capsule_data)
> +{
> + struct efi_firmware_management_capsule_header *capsule;
> + struct efi_firmware_management_capsule_image_header *image;
> + size_t capsule_size;
> + void *image_binary, *vendor_code;
> + efi_handle_t *handles;
> + efi_uintn_t no_handles;
> + int item;
> + struct efi_firmware_management_protocol *fmp;
> + u16 *abort_reason;
> + efi_status_t ret = EFI_SUCCESS;
> +
> + /* sanity check */
> + if (capsule_data->header_size < sizeof(*capsule) ||
> + capsule_data->header_size >= capsule_data->capsule_image_size)
> + return EFI_INVALID_PARAMETER;
> +
> + capsule = (void *)capsule_data + capsule_data->header_size;
> + capsule_size = capsule_data->capsule_image_size
> + - capsule_data->header_size;
> +
> + if (capsule->version != 0x0001)
> + return EFI_INVALID_PARAMETER;
> +
> + /* Drivers */
> + /* TODO: support loading drivers */
> +
> + handles = NULL;
> + ret = EFI_CALL(efi_locate_handle_buffer(
> + BY_PROTOCOL,
> + _guid_firmware_management_protocol,
> + NULL, _handles, (efi_handle_t **)));
> + if (ret != EFI_SUCCESS)
> + return EFI_UNSUPPORTED;
> +
> + /* Payload */
> + for (item = capsule->embedded_driver_count;
> +item < capsule->embedded_driver_count
> + + capsule->payload_item_count; item++) {
> + /* sanity check */
> + if ((capsule->item_offset_list[item] + sizeof(*image)
> +
[PATCH] tpm: use more than sha256 on pcr_extend
The current tpm2_pcr_extend is hardcoded using SHA256.
Let's make the actual command to the TPM2 configurable so we can support
a wider range of algorithms and keep the current command line as-is i.e
limited to SHA256 only
Signed-off-by: Ilias Apalodimas
---
cmd/tpm-v2.c | 3 ++-
include/tpm-v2.h | 5 -
lib/tpm-v2.c | 11 ++-
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index 5fa4788a72de..daae91100a2b 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -116,7 +116,8 @@ static int do_tpm2_pcr_extend(struct cmd_tbl *cmdtp, int
flag, int argc,
if (index >= priv->pcr_count)
return -EINVAL;
- rc = tpm2_pcr_extend(dev, index, digest);
+ rc = tpm2_pcr_extend(dev, index, TPM2_ALG_SHA256, digest,
+TPM2_DIGEST_LEN);
unmap_sysmem(digest);
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index d8cf0ab05185..fde44e5d98cd 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -376,11 +376,14 @@ u32 tpm2_clear(struct udevice *dev, u32 handle, const
char *pw,
*
* @devTPM device
* @index Index of the PCR
+ * @algorithm Algorithm used
* @digest Value representing the event to be recorded
+ * @digest_len len of the hash
*
* @return code of the operation
*/
-u32 tpm2_pcr_extend(struct udevice *dev, u32 index, const uint8_t *digest);
+u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm,
+ const u8 *digest, u32 digest_len);
/**
* Issue a TPM2_PCR_Read command.
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 91759068cf03..1f3deb06e487 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -80,11 +80,12 @@ u32 tpm2_clear(struct udevice *dev, u32 handle, const char
*pw,
return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
}
-u32 tpm2_pcr_extend(struct udevice *dev, u32 index, const uint8_t *digest)
+u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm,
+ const u8 *digest, u32 digest_len)
{
u8 command_v2[COMMAND_BUFFER_SIZE] = {
tpm_u16(TPM2_ST_SESSIONS), /* TAG */
- tpm_u32(33 + TPM2_DIGEST_LEN), /* Length */
+ tpm_u32(33 + digest_len), /* Length */
tpm_u32(TPM2_CC_PCR_EXTEND),/* Command code */
/* HANDLE */
@@ -99,7 +100,7 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, const
uint8_t *digest)
tpm_u16(0), /* Size of */
/* (if any) */
tpm_u32(1), /* Count (number of hashes) */
- tpm_u16(TPM2_ALG_SHA256), /* Algorithm of the hash */
+ tpm_u16(algorithm), /* Algorithm of the hash */
/* STRING(digest) Digest */
};
unsigned int offset = 33;
@@ -110,8 +111,8 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, const
uint8_t *digest)
* - the digest
*/
ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
- offset, digest, TPM2_DIGEST_LEN);
- offset += TPM2_DIGEST_LEN;
+ offset, digest, digest_len);
+ offset += digest_len;
if (ret)
return TPM_LIB_ERROR;
--
2.29.2
binman: u-boot-dtb vs. filename
Hi,
I stumbled over README.entries claiming
Entry: u-boot-dtb: U-Boot device tree
-
Properties / Entry arguments:
- filename: Filename of u-boot.dtb (default 'u-boot.dtb')
However,
u-boot-dtb {
filename = "foo.dtb"
}
only pulls u-boot.dtb. Tried to fix that but I failed to understand the
binman logic. Maybe the documentation (also that of
u-boot-dtb-with-ucode) was just not supposed to refer to filename.
Using 'blob' now, like other boards.
Jan
Re: [GIT PULL] xilinx patches for v2021.01-rc3
On Fri, Nov 20, 2020 at 02:42:11PM +0100, Michal Simek wrote: > Hi Tom, > > please pull these patches to your tree. CI builds look good. > Especially fru fixes are needed but enabling microblaze GC with other > fixes are also good. > > Thanks, > Michal > > The following changes since commit b80680633dc954d32f81f3afacd3d1f2f3d290b0: > > Merge branch '2020-11-18-assorted-fixes' (2020-11-19 10:23:50 -0500) > > are available in the Git repository at: > > g...@gitlab.denx.de:u-boot/custodians/u-boot-microblaze.git > tags/xilinx-for-v2021.01-rc3 > > for you to fetch changes up to cd40b826554e535d56dcc09dbb97c47048bd3b32: > > fru: common: Record pcie/uuid fields in custom board area (2020-11-20 > 10:42:54 +0100) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCHv2 05/25] common: fit: Update board_fit_image_post_process() to pass fit and node_offset
On Fri, Nov 20, 2020 at 01:23:06PM +0200, Tero Kristo wrote: > From: Lokesh Vutla > > board_fit_image_post_process() passes only start and size of the image, > but type of the image is not passed. So pass fit and node_offset, to > derive information about image to be processed. > > Signed-off-by: Lokesh Vutla Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCHv2 01/25] lib: rational: copy the rational fraction lib routines from Linux
On Fri, Nov 20, 2020 at 01:23:02PM +0200, Tero Kristo wrote: > Copy the best rational approximation calculation routines from Linux. > Typical usecase for these routines is to calculate the M/N divider > values for PLLs to reach a specific clock rate. > > This is based on linux kernel commit: > "lib/math/rational.c: fix possible incorrect result from rational > fractions helper" > (sha1: 323dd2c3ed0641f49e89b4e420f9eef5d3d5a881) > > Signed-off-by: Tero Kristo Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Pull request for UEFI sub-system for efi-2021-01-rc3 (2)
Dear Tom, The following changes since commit b80680633dc954d32f81f3afacd3d1f2f3d290b0: Merge branch '2020-11-18-assorted-fixes' (2020-11-19 10:23:50 -0500) are available in the Git repository at: https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2021-01-rc3-2 for you to fetch changes up to 7e5875a85689d762bde58a587a7d531667358ee4: efi_loader: parameter check in GetNextVariableName() (2020-11-21 07:26:16 +0100) Gilab CI reported no problems: https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/5384 Pull request for UEFI sub-system for efi-2021-01-rc3 (2) The parameter check for UEFI service GetNextVariableName() is corrected. The dependencies of CONFIG_DFU_TFTP are simplified. The set of supported hash algorithms reported by the EFI_TCG2_PROTOCOL is corrected. AKASHI Takahiro (1): dfu: simplify the dependencies of DFU_TFTP Heinrich Schuchardt (1): efi_loader: parameter check in GetNextVariableName() Ilias Apalodimas (1): efi_loader: tcg2 protocol updates drivers/dfu/Kconfig | 3 +- include/efi_tcg2.h | 2 - lib/efi_loader/efi_tcg2.c| 160 +-- lib/efi_loader/efi_var_mem.c | 11 ++- 4 files changed, 101 insertions(+), 75 deletions(-)
[PATCH] fs: fat: directory entries starting with 0x05
0x05 is used as replacement letter for 0xe5 at the first position of short
file names. We must not skip over directory entries starting with 0x05.
Cf. Microsoft FAT Specification, August 30 2005
Fixes: 39606d462c97 ("fs: fat: handle deleted directory entries correctly")
Signed-off-by: Heinrich Schuchardt
---
fs/fat/fat.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 28aa5aaa9f..fb6ba89466 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -926,8 +926,7 @@ static int fat_itr_next(fat_itr *itr)
if (!dent)
return 0;
- if (dent->name[0] == DELETED_FLAG ||
- dent->name[0] == aRING)
+ if (dent->name[0] == DELETED_FLAG)
continue;
if (dent->attr & ATTR_VOLUME) {
--
2.29.2
Re: Re: [linux-sunxi] [PATCH] sunxi: Add arm64 FEL support
Hi Andre, thanks for working on this! Dne petek, 20. november 2020 ob 10:42:15 CET je André Przywara napisal(a): > On 19/11/2020 19:59, Priit Laes wrote: > > On Thu, Nov 19, 2020 at 10:54:42AM +, Andre Przywara wrote: > >> So far we did not support the BootROM based FEL USB debug mode on the > >> 64-bit builds for Allwinner SoCs: The BootROM is using AArch32, but the > >> SPL runs in AArch64. > >> Returning back to AArch32 was not working as expected, since the RMR > >> reset into 32-bit mode always starts execution in the BootROM, but not > >> in the FEL routine. > >> > >> After some debug and research and with help via IRC, the CPU hotplug > >> mechanism emerged as a solution: If a certain R_CPUCFG register contains > >> some magic, the BootROM will immediately branch to an address stored in > >> some other register. This works well for our purposes. > >> > >> Enable the FEL feature by providing early AArch32 code to first save the > >> FEL state, *before* initially entering AArch64. > >> If we eventually determine that we should return to FEL, we reset back > >> into AArch32, and use the CPU hotplug mechanism to run some small > >> AArch32 code snippet that restores the initially saved FEL state. > >> > >> That allows the normal AArch64 SPL build to be loaded via the sunxi-fel > >> tool, with it returning into FEL mode, so that other payloads can be > >> transferred via FEL as well. > >> > >> Tested on A64, H5 and H6. > >> > >> Signed-off-by: Andre Przywara > >> --- > >> arch/arm/cpu/armv8/Makefile | 2 + > >> arch/arm/cpu/armv8/fel_utils.S | 78 + > >> arch/arm/include/asm/arch-sunxi/boot0.h | 14 + > >> include/configs/sunxi-common.h | 2 - > >> 4 files changed, 94 insertions(+), 2 deletions(-) > >> create mode 100644 arch/arm/cpu/armv8/fel_utils.S > > > > Don't you want to also update board/sunxi/README.sunxi64 ? > > Yes, but I am in the process of rewriting this anyway, since some parts > apply to 32-bit SoCs as well (SD card, SPI flash), and there is > currently no documentation about them. So bear with me, I will post a > documentation update ASAP, just didn't want to hold back this patch. > > Thanks! > Andre. > > >> > >> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile > >> index 93d26f98568..f7b4a5ee46c 100644 > >> --- a/arch/arm/cpu/armv8/Makefile > >> +++ b/arch/arm/cpu/armv8/Makefile > >> @@ -27,6 +27,8 @@ obj-$(CONFIG_ARM_SMCCC) += smccc- call.o > >> > >> ifndef CONFIG_SPL_BUILD > >> obj-$(CONFIG_ARMV8_SPIN_TABLE) += spin_table.o spin_table_v8.o > >> +else > >> +obj-$(CONFIG_ARCH_SUNXI) += fel_utils.o > >> endif > >> obj-$(CONFIG_$(SPL_)ARMV8_SEC_FIRMWARE_SUPPORT) += sec_firmware.o sec_firmware_asm.o > >> > >> diff --git a/arch/arm/cpu/armv8/fel_utils.S b/arch/arm/cpu/armv8/ fel_utils.S > >> new file mode 100644 > >> index 000..334fdef7fa0 > >> --- /dev/null > >> +++ b/arch/arm/cpu/armv8/fel_utils.S > >> @@ -0,0 +1,78 @@ > >> +/* > >> + * Utility functions for FEL mode, when running SPL in AArch64. > >> + * > >> + * Copyright (c) 2017 Arm Ltd. > >> + * > >> + * SPDX-License-Identifier: GPL-2.0+ > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +/* > >> + * We don't overwrite save_boot_params() here, to save the FEL state upon > >> + * entry, since this would run *after* the RMR reset, which clobbers that > >> + * state. > >> + * Instead we store the state _very_ early in the boot0 hook, *before* > >> + * resetting to AArch64. > >> + */ > >> + > >> +/* > >> + * The FEL routines in BROM run in AArch32. > >> + * Reset back into 32-bit mode here and restore the saved FEL state > >> + * afterwards. > >> + * Resetting back into AArch32/EL3 using the RMR always enters the BROM, > >> + * but we can use the CPU hotplug mechanism to branch back to our code > >> + * immediately. > >> + */ > >> +ENTRY(return_to_fel) > >> + /* > >> + * the RMR reset will clear all registers, so save the arguments > >> + * (LR and SP) in the fel_stash structure, which we read anyways later > >> + */ > >> + adr x2, fel_stash > >> + str w0, [x2] > >> + str w1, [x2, #4] > >> + > >> + adr x1, fel_stash_addr // to find the fel_stash address in AA32 > >> + str w2, [x1] > >> + > >> + ldr x0, =0xfa50392f // CPU hotplug magic > >> +#ifndef CONFIG_MACH_SUN50I_H6 > >> + ldr x2, =(SUNXI_CPUCFG_BASE + 0x1a4) // offset for CPU hotplug base > >> + str w0, [x2, #0x8] > >> +#else > >> + ldr x2, =(SUNXI_RTC_BASE + 0x1b8) // BOOT_CPU_HP_FLAG_REG > >> + str w0, [x2], #0x4 > >> +#endif I suggest to turn around those ifdefs, e.g. put specific one first and general last. That way it will be much more readable when new exception will be added. For example, H616 has it's own address. Best regards, Jernej > >> + adr x0, back_in_32 > >> + str w0, [x2] > >> + > >> + dsb sy > >> + isb
Re: [PATCH] lib/efi_loader: fix ABI in efi_mm_communicate_header
Thanks Etienne!
On Sat, 21 Nov 2020 at 13:00, Etienne Carriere
wrote:
>
> Pack struct efi_mm_communicate_header as done in EDK2 as seen in
> release 201808 [1]. If not packed sizeof() for the structure adds
> 4 additional bytes on 32bit targets which breaks the ABI.
>
> Link: [1]
> https://github.com/tianocore/edk2/blob/edk2-stable201808/MdePkg/Include/Protocol/MmCommunication.h#L21
> Fixes: 23a397d2e2fb ("efi_loader: Add headers for EDK2 StandAloneMM
> communication")
> Signed-off-by: Etienne Carriere
> ---
> include/mm_communication.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/mm_communication.h b/include/mm_communication.h
> index e464cbb48e..e65fbde60d 100644
> --- a/include/mm_communication.h
> +++ b/include/mm_communication.h
> @@ -43,7 +43,7 @@
> * To avoid confusion in interpreting frames, the communication buffer should
> * always begin with efi_mm_communicate_header.
> */
> -struct efi_mm_communicate_header {
> +struct __packed efi_mm_communicate_header {
> efi_guid_t header_guid;
> size_t message_len;
> u8 data[];
> --
> 2.17.1
Reviewed-by: Ilias Apalodimas
[PATCH] lib/efi_loader: fix ABI in efi_mm_communicate_header
Pack struct efi_mm_communicate_header as done in EDK2 as seen in
release 201808 [1]. If not packed sizeof() for the structure adds
4 additional bytes on 32bit targets which breaks the ABI.
Link: [1]
https://github.com/tianocore/edk2/blob/edk2-stable201808/MdePkg/Include/Protocol/MmCommunication.h#L21
Fixes: 23a397d2e2fb ("efi_loader: Add headers for EDK2 StandAloneMM
communication")
Signed-off-by: Etienne Carriere
---
include/mm_communication.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/mm_communication.h b/include/mm_communication.h
index e464cbb48e..e65fbde60d 100644
--- a/include/mm_communication.h
+++ b/include/mm_communication.h
@@ -43,7 +43,7 @@
* To avoid confusion in interpreting frames, the communication buffer should
* always begin with efi_mm_communicate_header.
*/
-struct efi_mm_communicate_header {
+struct __packed efi_mm_communicate_header {
efi_guid_t header_guid;
size_t message_len;
u8 data[];
--
2.17.1
