Re: [PATCH v6 01/25] doc: Add documentation about devicetree usage

[François Ozog]

Hi Simon

Le sam. 4 déc. 2021 à 18:42, Simon Glass  a écrit :

> Hi François,
>
> On Sat, 4 Dec 2021 at 04:06, François Ozog 
> wrote:
> >
> > Hi Simon
> >
> > Le sam. 4 déc. 2021 à 02:02, Simon Glass  a écrit :
> >>
> >> Hi Heinrich,
> >>
> >> On Fri, 3 Dec 2021 at 13:28, Heinrich Schuchardt 
> wrote:
> >> >
> >> > On 12/3/21 9:13 PM, Simon Glass wrote:
> >> > > Hi Heinrich,
> >> > >
> >> > > On Fri, 3 Dec 2021 at 06:09, Heinrich Schuchardt <
> xypron.g...@gmx.de> wrote:
> >> > >>
> >> > >> On 12/3/21 13:34, Heinrich Schuchardt wrote:
> >> > >>> On 12/2/21 16:58, Simon Glass wrote:
> >> >  At present some of the ideas and techniques behind devicetree in
> U-Boot
> >> >  are assumed, implied or unsaid. Add some documentation to cover
> how
> >> >  devicetree is build, how it can be modified and the rules about
> using
> >> >  the various CONFIG_OF_... options.
> >> > 
> >> >  Signed-off-by: Simon Glass 
> >> >  Reviewed-by: Marcel Ziswiler 
> >> >  ---
> >> >  This patch attracted quite a bit of discussion here:
> >> > 
> >> > 
> https://patchwork.ozlabs.org/project/uboot/patch/20210909201033.755713-4-...@chromium.org/
> >> > 
> >> > 
> >> >  I have not included the text suggested by François. While I
> agree that
> >> >  it would be useful to have an introduction in this space, I do
> not agree
> >> >  that we should have two devicetrees or that U-Boot should not
> have its
> >> >  own
> >> >  things in the devicetree, so it is not clear to me what we should
> >> >  actually
> >> >  write.
> >> > 
> >> >  The 'Devicetree Control in U-Boot' docs were recently merged and
> these
> >> >  provide some base info, for now.
> >> > 
> >> >  Changes in v6:
> >> >  - Fix description of OF_BOARD so it refers just to the current
> state
> >> >  - Explain that the 'two devicetrees' refers to two *control*
> devicetrees
> >> > 
> >> >  Changes in v5:
> >> >  - Bring into the OF_BOARD series
> >> >  - Rebase to master and drop mention of OF_PRIOR_STAGE, since
> removed
> >> >  - Refer to the 'control' DTB in the first paragraph
> >> >  - Use QEMU instead of qemu
> >> > 
> >> >  Changes in v3:
> >> >  - Clarify the 'bug' refered to at the top
> >> >  - Reword 'This means that there' paragraph to explain
> U-Boot-specific
> >> >  things
> >> >  - Move to doc/develop/devicetree now that OF_CONTROL is in the
> docs
> >> > 
> >> >  Changes in v2:
> >> >  - Fix typos per Sean (thank you!) and a few others
> >> >  - Add a 'Use of U-Boot /config node' section
> >> >  - Drop mention of dm-verity since that actually uses the kernel
> cmdline
> >> >  - Explain that OF_BOARD will still work after these changes (in
> >> >  'Once this bug is fixed...' paragraph)
> >> >  - Expand a bit on the reason why the 'Current situation' is bad
> >> >  - Clarify in a second place that Linux and U-Boot use the same
> devicetree
> >> >  in 'To be clear, while U-Boot...'
> >> >  - Expand on why we should have rules for other projects in
> >> >  'Devicetree in another project'
> >> >  - Add a comment as to why devicetree in U-Boot is not 'bad
> design'
> >> >  - Reword 'in-tree U-Boot devicetree' to 'devicetree source in
> U-Boot'
> >> >  - Rewrite 'Devicetree generated on-the-fly in another project'
> to cover
> >> >  points raised on v1
> >> >  - Add 'Why does U-Boot have its nodes and properties?'
> >> >  - Add 'Why not have two devicetrees?'
> >> > 
> >> > doc/develop/devicetree/dt_update.rst | 555
> +++
> >> > doc/develop/devicetree/index.rst |   1 +
> >> > 2 files changed, 556 insertions(+)
> >> > create mode 100644 doc/develop/devicetree/dt_update.rst
> >> > 
> >> > > [..]
> >> > >
> >> >  +
> >> >  +- The other project may not provide a way to support U-Boot's
> >> >  requirements for
> >> >  +  devicetree, such as the /config node. Note: On the U-Boot
> mailing
> >> >  linst, this
> >> > >>>
> >> > >>> Even if you remove these lines in 17/25 I would prefer not to
> introduce
> >> > >>> typos here:
> >> > >>>
> >> > >>> %s/linst/list/
> >> > >>>
> >> > >
> >> > > OK I can fix that.
> >> > >
> >> > > [..]
> >> > >
> >> >  +Normally, supporting U-Boot's features is trivial, since the
> >> >  devicetree compiler
> >> >  +(dtc) can compile the source, including any U-Boot pieces. So
> the
> >> >  burden is
> >> >  +extremely low.
> >> >  +
> >> >  +In this case, the devicetree in the other project must track
> U-Boot's
> >> >  use of
> >> >  +device tree, so that it remains compatible. See `Devicetree in
> >> >  another project`_
> >> >  +for reasons why.
> >> > >>>
> >> > >>> Did you ever ask the QEMU community what they think about your
> ideas?
> >> > >>> What was the 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[François Ozog]

Hi Simon

Le sam. 4 déc. 2021 à 18:35, Simon Glass  a écrit :

> Hi François,
>
> On Sat, 4 Dec 2021 at 09:55, François Ozog 
> wrote:
> >
> > Hi Simon
> >
> > Le sam. 4 déc. 2021 à 16:21, Simon Glass  a écrit :
> >>
> >> Hi Tom,
> >>
> >> On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> >> >
> >> > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> >> >
> >> > [huge snip]
> >> > > > There's things that need to be cleaned up because we have some
> small
> >> > > > number of platforms that went off and did their own thing.  But
> largely
> >> > > > yes, things make sense to me.  We have:
> >> > > > - We embedded the device tree that will configure U-Boot, because
> there
> >> > > >   is no way for the hardware to have provided us one.
> >> > > > - We do not embed the device tree that will configure U-Boot,
> because
> >> > > >   there is already one present in memory for us to use.
> >> > > >
> >> > > > Then we have the developer option of:
> >> > > > - We embedded the device tree that will configure U-Boot, because
> we're
> >> > > >   developing something.
> >> > >
> >> > > Yes, agreed those are the cases. To me this needs to be a run-time
> choice.
> >> >
> >> > But it's not possible.  That's the problem we keep going around and
> >> > around about.  People keep raising real life examples where you cannot
> >> > make a run time choice between "device tree we're passed at run time"
> >> > and "device tree we're compiled with".
> >>
> >> I haven't seen one. The most extreme case is QEMU and it works fine. I
> >> even added a test with it. What am I missing?
> >>
> >> >
> >> > And it's not helpful.  It is ALWAYS the case that we know that we want
> >> > to override the run time device tree with our own, because it's a
> >> > developer developing things or it's a user / production case where we
> >> > must use the provided tree.  NOT doing that is what leads to madness
> >> > like we see for example on Pi where if we don't use the passed tree we
> >> > still need to copy X/Y/Z out of it.
> >>
> >> Aren't you talking about the distro DT there, rather than the the one
> >> on the boot disk? That is my reading of that patch. If we need to do
> >> that sort of thing, it doesn't matter where the the cointrol DT comes
> >> from. You are still going to have to do that sort of thing.
> >>
> >> It is not ALWAYS the case. I've shown you how easy it is to disable
> >> OF_BOARD and still boot / iterate.
> >>
> >> >
> >> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we
> should
> >> > > > > provide a way to do that? But what is driving that desire?
> >> > > >
> >> > > > I'm looking for ways to convince you that we do not need to
> include a
> >> > > > device tree in the binary.  There's a growing set of devices
> where the
> >> > > > device tree exists with the device.  If it's missing, that's a
> huge
> >> > > > fatal error we can't do all that much about.  If we need to do
> something
> >> > > > to that device tree for U-Boot, yes, fine, we should make it
> straight
> >> > > > forward for the developer to do that.  But that's not the common
> case!
> >> > >
> >> > > Well we could add another Kconfig which tells U-Boot not to include
> a
> >> > > devicetree in u-boot.bin, if that would resolve this?
> >> > >
> >> > > I just want to make sure that we always build the devicetrees and
> that
> >> > > it is easy for a knowledgeable dev to switch over to use them,
> without
> >> > > spelunking through dozens of other projects to discover the secret
> DT
> >> > > that no one will tell us about.
> >> >
> >> > Should we demand better documentation for boards?  Yes.  But it's
> still
> >> > a valid case to have zero device trees for a given platform in-tree.
> >> > Xen is an example of this.  QEMU is an example of this.  Platforms
> need
> >> > to work without adding special tweaks for us.  Maybe that means some
> >> > features can't be tested in QEMU-as-virtual-platform and only in
> >> > QEMU-faithfully-emulating-specific-physical-platforms.
> >>
> >> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> >> case, I think. How about we create a special Kconfig for that case? We
> >> need to make some progress here.
> >>
> >> >
> >> > > > I guess another part of the problem is that historically almost
> all
> >> > > > platforms were in the first case I list above, no run time
> provided
> >> > > > device tree, so we took the kernel one and added our bindings to
> it.
> >> > > > Now we're being bit by the growing number of platforms that are
> the
> >> > > > second case, and how do we get our properties in there, and which
> ones
> >> > > > even make sense to do that for.
> >> > >
> >> > > I think upstreaming the bindings is the solution there. I've made a
> >> > > start, but we need to make progress with this series and all the
> other
> >> > > things in flight. I think a lot of people want U-Boot to not have a
> >> > > devicetree source files in it for ARMv8 platforms. I am 

[PATCH 2/2] net: dsa: sja1105: fix device id detection

[Vladimir Oltean]

The sja1105_check_device_id() function contains logic to work without
changing the device tree on reworked boards, one of which I have (the
NXP LS1021A-TSN normally has a SJA1105T, but I have a version with a
resoldered SJA1105Q which is pin compatible). This logic is taken from
the Linux driver.

However this logic gets shortcircuited in U-Boot by an earlier check for
the exact device ID specified in the device tree. So the reworked board
does not probe the SJA1105Q switch. Remove this duplicated logic and let
the automatic device ID detection do its job.

Fixes: f24b666b2204 ("net: dsa: add driver for NXP SJA1105 L2 switch")
Signed-off-by: Vladimir Oltean 
---
 drivers/net/sja1105.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/sja1105.c b/drivers/net/sja1105.c
index 17bab33eddb7..4ca8709e347c 100644
--- a/drivers/net/sja1105.c
+++ b/drivers/net/sja1105.c
@@ -3276,12 +3276,6 @@ static int sja1105_check_device_id(struct 
sja1105_private *priv)
sja1105_packing(packed_buf, _id, 31, 0, SJA1105_SIZE_DEVICE_ID,
UNPACK);
 
-   if (device_id != priv->info->device_id) {
-   printf("Expected device ID 0x%llx but read 0x%llx\n",
-  priv->info->device_id, device_id);
-   return -ENODEV;
-   }
-
rc = sja1105_xfer_buf(priv, SPI_READ, regs->prod_id, packed_buf,
  SJA1105_SIZE_DEVICE_ID);
if (rc < 0)
-- 
2.25.1

[PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY

[Vladimir Oltean]

If the DSA API is going to allow drivers to do things such as:

- phy_config in dsa_ops :: port_probe
- phy_startup in dsa_ops :: port_enable

then it would actually be good if the ->port_probe() method would
actually be called in all cases before the ->port_enable() is.

Currently this is true for user ports, but not true for the CPU port,
because the CPU port does not have a udevice registered for it (this is
all part of DSA's design). So the current issue is that after
phy_startup has finished for the CPU port, its phydev->speed is an
uninitialized value, because phy_config() was never called for the
priv->cpu_port_fixed_phy, and it is precisely phy_config() who copies
the speed into the phydev in the case of the fixed PHY driver.

So we need to simulate a probing event for the CPU port by manually
calling the driver's ->port_probe() method for the CPU port.

Fixes: 8a2982574854 ("net: dsa: introduce a .port_probe() method in struct 
dsa_ops")
Signed-off-by: Vladimir Oltean 
---
 net/dsa-uclass.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 606b1539a776..9ff55a02fb23 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -466,6 +466,8 @@ static int dsa_pre_probe(struct udevice *dev)
 {
struct dsa_pdata *pdata = dev_get_uclass_plat(dev);
struct dsa_priv *priv = dev_get_uclass_priv(dev);
+   struct dsa_ops *ops = dsa_get_ops(dev);
+   int err;
 
priv->num_ports = pdata->num_ports;
priv->cpu_port = pdata->cpu_port;
@@ -477,6 +479,15 @@ static int dsa_pre_probe(struct udevice *dev)
 
uclass_find_device_by_ofnode(UCLASS_ETH, pdata->master_node,
 >master_dev);
+
+   /* Simulate a probing event for the CPU port */
+   if (ops->port_probe) {
+   err = ops->port_probe(dev, priv->cpu_port,
+ priv->cpu_port_fixed_phy);
+   if (err)
+   return err;
+   }
+
return 0;
 }
 
-- 
2.25.1

[PATCH 0/2] DSA switch fixes

[Vladimir Oltean]

This small patch set contains some bug fixes. The first is for the
recently introduced ->port_probe() DSA switch method, and the second is
for the recently introduced SJA1105 driver.

Vladimir Oltean (2):
  net: dsa: fix phydev->speed being uninitialized for the CPU port fixed
PHY
  net: dsa: sja1105: fix device id detection

 drivers/net/sja1105.c |  6 --
 net/dsa-uclass.c  | 11 +++
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.25.1

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Tom,

On Sat, 4 Dec 2021 at 11:03, Tom Rini  wrote:
>
> On Sat, Dec 04, 2021 at 08:20:55AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> > >
> > > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> > >
> > > [huge snip]
> > > > > There's things that need to be cleaned up because we have some small
> > > > > number of platforms that went off and did their own thing.  But 
> > > > > largely
> > > > > yes, things make sense to me.  We have:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > there
> > > > >   is no way for the hardware to have provided us one.
> > > > > - We do not embed the device tree that will configure U-Boot, because
> > > > >   there is already one present in memory for us to use.
> > > > >
> > > > > Then we have the developer option of:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > we're
> > > > >   developing something.
> > > >
> > > > Yes, agreed those are the cases. To me this needs to be a run-time 
> > > > choice.
> > >
> > > But it's not possible.  That's the problem we keep going around and
> > > around about.  People keep raising real life examples where you cannot
> > > make a run time choice between "device tree we're passed at run time"
> > > and "device tree we're compiled with".
> >
> > I haven't seen one. The most extreme case is QEMU and it works fine. I
> > even added a test with it. What am I missing?
>
> QEMU and Xen should both never have an in-source-tree dts as they are
> dynamic.  I think you missed the explanation about how U-Boot + Xen
> works?  You're running the same U-Boot under Xen on any arbitrary ARMv8
> (with required features...) system.  For QEMU virtual machines you're
> not supposed to do what you're doing, for production.
>
> > > And it's not helpful.  It is ALWAYS the case that we know that we want
> > > to override the run time device tree with our own, because it's a
> > > developer developing things or it's a user / production case where we
> > > must use the provided tree.  NOT doing that is what leads to madness
> > > like we see for example on Pi where if we don't use the passed tree we
> > > still need to copy X/Y/Z out of it.
> >
> > Aren't you talking about the distro DT there, rather than the the one
> > on the boot disk? That is my reading of that patch. If we need to do
> > that sort of thing, it doesn't matter where the the cointrol DT comes
> > from. You are still going to have to do that sort of thing.
> >
> > It is not ALWAYS the case. I've shown you how easy it is to disable
> > OF_BOARD and still boot / iterate.
>
> The DT we're passed in is the DT to pass to the OS.  That's the hook for
> putting a DTB on the device as it ships, the OS will just work.  The
> production case of needing to update that stored DTB is handled.  It's
> always what should be used, again outside of developer doing
> development.
>
> Maybe that's part of the confusion here too.  The DTB U-Boot is using is
> the DTB the OS will consume too, in the passed at run time case.  Unless
> we're instead going to save that DTB aside?  Which leaves me with a
> different set of design questions...
>
> > > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > > provide a way to do that? But what is driving that desire?
> > > > >
> > > > > I'm looking for ways to convince you that we do not need to include a
> > > > > device tree in the binary.  There's a growing set of devices where the
> > > > > device tree exists with the device.  If it's missing, that's a huge
> > > > > fatal error we can't do all that much about.  If we need to do 
> > > > > something
> > > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > > forward for the developer to do that.  But that's not the common case!
> > > >
> > > > Well we could add another Kconfig which tells U-Boot not to include a
> > > > devicetree in u-boot.bin, if that would resolve this?
> > > >
> > > > I just want to make sure that we always build the devicetrees and that
> > > > it is easy for a knowledgeable dev to switch over to use them, without
> > > > spelunking through dozens of other projects to discover the secret DT
> > > > that no one will tell us about.
> > >
> > > Should we demand better documentation for boards?  Yes.  But it's still
> > > a valid case to have zero device trees for a given platform in-tree.
> > > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > > to work without adding special tweaks for us.  Maybe that means some
> > > features can't be tested in QEMU-as-virtual-platform and only in
> > > QEMU-faithfully-emulating-specific-physical-platforms.
> >
> > You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> > case, I think. How about we create a special Kconfig for that case? We
> > need to make some progress here.
>
> Yes, because there's a small number of 

Re: [PATCH v5 15/28] efi: Move exit_boot_services into a function

[Heinrich Schuchardt]

On 12/4/21 16:56, Simon Glass wrote:

At present this code is inline in the app and stub. But they do the same
thing. The difference is that the stub does it immediately and the app
doesn't want to do it until the end (when it boots a kernel) or not at
all, if returning to UEFI.

Move it into a function so it can be called as needed.

Also store the memory map so that it can be accessed within the app if
needed.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add a sentence about what the patch does

  include/efi.h  | 32 ++
  lib/efi/efi.c  | 68 ++
  lib/efi/efi_app.c  |  3 ++
  lib/efi/efi_stub.c | 66 
  4 files changed, 114 insertions(+), 55 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index ca301db7cb5..ed28c204140 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
   * @sys_table: Pointer to system table
   * @boot: Pointer to boot-services table
   * @run: Pointer to runtime-services table
+ * @memmap_key: Key returned from get_memory_map()
+ * @memmap_desc: List of memory-map records
+ * @memmap_alloc: Amount of memory allocated for memory map list
+ * @memmap_size Size of memory-map list in bytes
+ * @memmap_desc_size: Size of an individual memory-map record, in bytes
+ * @memmap_version: Memory-map version
   *
   * @use_pool_for_malloc: true if all allocation should go through the EFI 
'pool'
   *methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv {
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+   efi_uintn_t memmap_key;
+   struct efi_mem_desc *memmap_desc;
+   efi_uintn_t memmap_alloc;
+   efi_uintn_t memmap_size;
+   efi_uintn_t memmap_desc_size;
+   u32 memmap_version;

/* app: */
bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
   */
  int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);

+/**
+ * efi_store_memory_map() - Collect the memory-map info from EFI
+ *
+ * Collect the memory info and store it for later use, e.g. in calling
+ * exit_boot_services()
+ *
+ * @priv:  Pointer to private EFI structure
+ * @return 0 if OK, non-zero on error


%s/@return/Return:/

Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation


+ */
+int efi_store_memory_map(struct efi_priv *priv);
+
+/**
+ * efi_call_exit_boot_services() - Handlet eh exit-boot-service procedure
+ *
+ * Tell EFI we don't want their boot services anymore
+ *
+ * @return 0 if OK, non-zero on error


%s/@return/Return:/


+ */
+int efi_call_exit_boot_services(void);
+
  #endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index cd6bf47b180..20da88c9151 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)

boot->free_pool(ptr);
  }
+
+int efi_store_memory_map(struct efi_priv *priv)
+{
+   struct efi_boot_services *boot = priv->sys_table->boottime;
+   efi_uintn_t size, desc_size;
+   efi_status_t ret;
+
+   /* Get the memory map so we can switch off EFI */
+   size = 0;
+   ret = boot->get_memory_map(, NULL, >memmap_key,
+  >memmap_desc_size,
+  >memmap_version);
+   if (ret != EFI_BUFFER_TOO_SMALL) {
+   printhex2(EFI_BITS_PER_LONG);
+   putc(' ');
+   printhex2(ret);
+   puts(" No memory map\n");
+   return ret;
+   }
+   /*
+* Since doing a malloc() may change the memory map and also we want to
+* be able to read the memory map in efi_call_exit_boot_services()
+* below, after more changes have happened
+*/
+   priv->memmap_alloc = size + 1024;
+   priv->memmap_size = priv->memmap_alloc;
+   priv->memmap_desc = efi_malloc(priv, size, );
+   if (!priv->memmap_desc) {
+   printhex2(ret);
+   puts(" No memory for memory descriptor\n");
+   return ret;
+   }
+
+   ret = boot->get_memory_map(>memmap_size, priv->memmap_desc,
+  >memmap_key, _size,
+  >memmap_version);
+   if (ret) {
+   printhex2(ret);
+   puts(" Can't get memory map\n");
+   return ret;
+   }
+
+   return 0;
+}
+


Missing function description


+int efi_call_exit_boot_services(void)
+{
+   struct efi_priv *priv = efi_get_priv();
+   const struct efi_boot_services *boot = priv->boot;
+   efi_uintn_t size;
+   u32 version;
+   efi_status_t ret;
+
+   size = priv->memmap_alloc;
+   ret = 

Re: [PATCH v5 13/28] efi: Add a few comments to the stub

[Heinrich Schuchardt]

On 12/4/21 16:56, Simon Glass wrote:

Comment some functions that need more information.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  lib/efi/efi_stub.c | 22 ++
  1 file changed, 22 insertions(+)

diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
index b3393e47fae..156cbf0b928 100644
--- a/lib/efi/efi_stub.c
+++ b/lib/efi/efi_stub.c
@@ -225,6 +225,22 @@ static int get_codeseg32(void)
return cs32;
  }

+/**
+ * setup_info_table() - sets up a table containing information from EFI
+ *
+ * We must call exit_boot_services() before jumping out of the stub into U-Boot
+ * proper, so that U-Boot has full control of peripherals, memory, etc.
+ *
+ * Once we do this, we cannot call any boot-services functions so we must find
+ * out everything we need to before doing that.
+ *
+ * Set up a struct efi_info_hdr table which can hold various records (e.g.
+ * struct efi_entry_memmap) with information obtained from EFI.
+ *
+ * @priv: Pointer to our private information which contains the list
+ * @size: Size of the table to allocate
+ * @return 0 if OK, non-zero on error


%s/@return/Return:/

cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

Best regards

Heinrich


+ */
  static int setup_info_table(struct efi_priv *priv, int size)
  {
struct efi_info_hdr *info;
@@ -248,6 +264,12 @@ static int setup_info_table(struct efi_priv *priv, int 
size)
return 0;
  }

+/**
+ * add_entry_addr() - Add a new entry to the efi_info list
+ *
+ * @priv: Pointer to our private information which contains the list
+ *
+ */
  static void add_entry_addr(struct efi_priv *priv, enum efi_entry_t type,
   void *ptr1, int size1, void *ptr2, int size2)
  {

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Ilias Apalodimas]

Hi Simon, 

On Sat, Dec 04, 2021 at 10:25:29AM -0700, Simon Glass wrote:
> Hi Ilias,
> 
> On Sat, 4 Dec 2021 at 08:58, Ilias Apalodimas
>  wrote:
> >
> > Hi Simon,
> >
> > [...]
> > > > [huge snip]
> > > > > > There's things that need to be cleaned up because we have some small
> > > > > > number of platforms that went off and did their own thing.  But 
> > > > > > largely
> > > > > > yes, things make sense to me.  We have:
> > > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > > there
> > > > > >   is no way for the hardware to have provided us one.
> > > > > > - We do not embed the device tree that will configure U-Boot, 
> > > > > > because
> > > > > >   there is already one present in memory for us to use.
> > > > > >
> > > > > > Then we have the developer option of:
> > > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > > we're
> > > > > >   developing something.
> > > > >
> > > > > Yes, agreed those are the cases. To me this needs to be a run-time 
> > > > > choice.
> > > >
> >
> > I am not convinced the case is "we are developing something".  The
> > arguments for this are something along the lines of:
> > 1. U-Boot will fail to compile if OF_SEPARATE is selected and you provide
> > no DT so we need to include it.  With which I disagree.  The config option
> > says "You must provide an external device tree" for a reason.
> 
> Where are you reading that? I'm lost.

In the XEN patch commit message [1]

> 
> OF_SEPARATE means it is not embedded in U-Boot but is attached to the end of 
> it.
> 
> > 2. The DT's are  hard to find.  The 'hard to find' RPI DTs are in fact
> > committed in the kernel. I am not sure about the rest, but honestly this
> > isn't a convincing argument for me.
> 
> Great, so you've solved that one but even that was confusing for me,
> as my patch should make clear. Every single board would be its own
> special snowflake if we went that way.
> 
> So if they are in the kernel, we just need to put them in U-Boot too,
> so problem solved. The auto-sync thing that I believe Rob is working
> on will make things easy.

My problem is not usable DTs like the PRI4.  My problem is the OF_BOARD
being a runtime option and always building that DT, while at the same time
introduce a third option to not include it in the binary.  Another problem
is 'empty' DTs.

> 
> >
> > What else are we gaining with it being a run time choice?  One of the
> 
> We are requiring a DT to be present in the U-Boot tree for
> development, documentation and discoverability purposes. Devs can turn
> OF_BOARD on and off as it suits them when iterating on a platform.

They can also do the same thing without tangling the 2 options.  The *real*
problem is that in a month from now we'll get a patch saying "I need to
change X on RPI4 DTB because I have this special reason and I want a
specific u-boot binding" and then we are back at the start.  We also have
OF_EMBED for the 'developer' option which is also mentioned explicitly in
the Kconfig.

> 
> > things this approach does address, but we keep conveniently ignoring, is
> > that it tries to preserve the current status quo.  You can go and add the
> > special missing U-Boot nodes in those DT's.  So that would bypass problems
> > if the bindings get NAK'ed.  But this is going to work against the
> > fragmentation we are trying to resolve.
> 
> Well that's another reason why we need in-tree DTs at the moment. That
> reason goes away once we have our bindings upstream and are able to do
> what we need with DT there.
> 

Again, that's a huge if.  I am honestly not saying this in bad faith, but I
have my concerns on if and what gets upstreamed.  So what this really does
in my head is preserve the current functionality.  So what I am trying to
avoid here is, in case the bindings get NAK'ed, we go back and say "that's
fine we got this covered, look it's in docs and commit messages!" 

> >
> > > > But it's not possible.  That's the problem we keep going around and
> > > > around about.  People keep raising real life examples where you cannot
> > > > make a run time choice between "device tree we're passed at run time"
> > > > and "device tree we're compiled with".
> > >
> > > I haven't seen one. The most extreme case is QEMU and it works fine. I
> > > even added a test with it. What am I missing?
> 
> (I think my point there is made)
> 
> >
> > IMHO 3b595da441cf is the commit that started the problem and tangled those
> > 2 options.  Note that this support has been removed from the dragonboard
> > later in 0204d1b56b2f 
> 
> Yes it is one of them. There may be cases where we want to patch up
> the DT that U-Boot builds. In fact OF_BOARD does not mean it came from
> a prior stage. All sorts of things could be going on. We should not
> conflate building a DT with OF_BOARD.

It says 'board specific way'.  To means that means we either create it on
the fly or inherit it.  In the case you want to fixup the DTB 

Re: [PATCH v5 04/28] efi: Locate all block devices in the app

[Heinrich Schuchardt]

On 12/4/21 16:56, Simon Glass wrote:

When starting the app, locate all block devices and make them available
to U-Boot. This allows listing partitions and accessing files in
filesystems.

EFI also has the concept of 'disks', meaning boot media. For now, this
is not obviously useful in U-Boot, but add code to at least locate these.
This can be expanded later as needed.

Series-changes; 2
- Store device path in struct efi_media_plat
- Don't export efi_bind_block()
- Only bind devices for media devices, not for partitions
- Show devices that are processed
- Update documentation

Signed-off-by: Simon Glass 
---

(no changes since v1)

  doc/develop/uefi/u-boot_on_efi.rst |   4 +-
  include/efi.h  |   6 +-
  include/efi_api.h  |  15 ++
  lib/efi/efi_app.c  | 223 +
  4 files changed, 243 insertions(+), 5 deletions(-)

diff --git a/doc/develop/uefi/u-boot_on_efi.rst 
b/doc/develop/uefi/u-boot_on_efi.rst
index 5f2f850f071..8f81b799072 100644
--- a/doc/develop/uefi/u-boot_on_efi.rst
+++ b/doc/develop/uefi/u-boot_on_efi.rst
@@ -265,9 +265,7 @@ This work could be extended in a number of ways:

  - Figure out how to solve the interrupt problem

-- Add more drivers to the application side (e.g. block devices, USB,
-  environment access). This would mostly be an academic exercise as a strong
-  use case is not readily apparent, but it might be fun.
+- Add more drivers to the application side (e.g.USB, environment access).

  - Avoid turning off boot services in the stub. Instead allow U-Boot to make
use of boot services in case it wants to. It is unclear what it might want
diff --git a/include/efi.h b/include/efi.h
index 0ec5913ddd1..908c5dc6ebd 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -419,10 +419,12 @@ struct efi_priv {
   *
   * @handle: handle of the controller on which this driver is installed
   * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI path to the device
   */
  struct efi_media_plat {
-   efi_handle_thandle;
-   struct efi_block_io *blkio;
+   efi_handle_t handle;
+   struct efi_block_io *blkio;
+   struct efi_device_path *device_path;
  };

  /* Base address of the EFI image */
diff --git a/include/efi_api.h b/include/efi_api.h
index 80109f012bc..ec9fa89a935 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol {
const u16 *package_version_name);
  };

+#define EFI_DISK_IO_PROTOCOL_GUID  \
+   EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \
+0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+
+struct efi_disk {
+   u64 revision;
+   efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id,
+u64 offset, efi_uintn_t buffer_size,
+void *buffer);
+
+   efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id,
+ u64 offset, efi_uintn_t buffer_size,
+ void *buffer);
+};
+
  #endif
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index f61665686c5..e38d46b15db 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -21,6 +21,9 @@
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 

  DECLARE_GLOBAL_DATA_PTR;

@@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap, int 
*sizep)
return -ENOSYS;
  }

+/**
+ * efi_print_str() - Print a UFT-16 string to the U-Boot console
+ *
+ * @str: String to print
+ */
+static void efi_print_str(const u16 *str)
+{
+   while (*str) {
+   int ch = *str++;
+
+   if (ch > ' ' && ch < 127)
+   putc(ch);
+   }
+}
+
+/**
+ * efi_bind_block() - bind a new block device to an EFI device
+ *
+ * Binds a new top-level EFI_MEDIA device as well as a child block device so
+ * that the block device can be accessed in U-Boot.
+ *
+ * The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1',
+ * for example, just like any other interface type.
+ *
+ * @handle: handle of the controller on which this driver is installed
+ * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI device path structure for this
+ * @len: Length of @device_path in bytes
+ * @devp: Returns the bound device
+ * @return 0 if OK, -ve on error
+ */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
+  struct efi_device_path *device_path, int len,
+  struct udevice **devp)
+{
+   struct efi_media_plat plat;
+   struct udevice *dev;
+   char name[18];
+   int ret;
+
+   plat.handle = handle;
+   plat.blkio = blkio;
+   plat.device_path = malloc(device_path->length);
+   if (!plat.device_path)
+   return log_msg_ret("path", -ENOMEM);
+   

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Tom Rini]

On Sat, Dec 04, 2021 at 08:20:55AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> >
> > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> >
> > [huge snip]
> > > > There's things that need to be cleaned up because we have some small
> > > > number of platforms that went off and did their own thing.  But largely
> > > > yes, things make sense to me.  We have:
> > > > - We embedded the device tree that will configure U-Boot, because there
> > > >   is no way for the hardware to have provided us one.
> > > > - We do not embed the device tree that will configure U-Boot, because
> > > >   there is already one present in memory for us to use.
> > > >
> > > > Then we have the developer option of:
> > > > - We embedded the device tree that will configure U-Boot, because we're
> > > >   developing something.
> > >
> > > Yes, agreed those are the cases. To me this needs to be a run-time choice.
> >
> > But it's not possible.  That's the problem we keep going around and
> > around about.  People keep raising real life examples where you cannot
> > make a run time choice between "device tree we're passed at run time"
> > and "device tree we're compiled with".
> 
> I haven't seen one. The most extreme case is QEMU and it works fine. I
> even added a test with it. What am I missing?

QEMU and Xen should both never have an in-source-tree dts as they are
dynamic.  I think you missed the explanation about how U-Boot + Xen
works?  You're running the same U-Boot under Xen on any arbitrary ARMv8
(with required features...) system.  For QEMU virtual machines you're
not supposed to do what you're doing, for production.

> > And it's not helpful.  It is ALWAYS the case that we know that we want
> > to override the run time device tree with our own, because it's a
> > developer developing things or it's a user / production case where we
> > must use the provided tree.  NOT doing that is what leads to madness
> > like we see for example on Pi where if we don't use the passed tree we
> > still need to copy X/Y/Z out of it.
> 
> Aren't you talking about the distro DT there, rather than the the one
> on the boot disk? That is my reading of that patch. If we need to do
> that sort of thing, it doesn't matter where the the cointrol DT comes
> from. You are still going to have to do that sort of thing.
> 
> It is not ALWAYS the case. I've shown you how easy it is to disable
> OF_BOARD and still boot / iterate.

The DT we're passed in is the DT to pass to the OS.  That's the hook for
putting a DTB on the device as it ships, the OS will just work.  The
production case of needing to update that stored DTB is handled.  It's
always what should be used, again outside of developer doing
development.

Maybe that's part of the confusion here too.  The DTB U-Boot is using is
the DTB the OS will consume too, in the passed at run time case.  Unless
we're instead going to save that DTB aside?  Which leaves me with a
different set of design questions...

> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > provide a way to do that? But what is driving that desire?
> > > >
> > > > I'm looking for ways to convince you that we do not need to include a
> > > > device tree in the binary.  There's a growing set of devices where the
> > > > device tree exists with the device.  If it's missing, that's a huge
> > > > fatal error we can't do all that much about.  If we need to do something
> > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > forward for the developer to do that.  But that's not the common case!
> > >
> > > Well we could add another Kconfig which tells U-Boot not to include a
> > > devicetree in u-boot.bin, if that would resolve this?
> > >
> > > I just want to make sure that we always build the devicetrees and that
> > > it is easy for a knowledgeable dev to switch over to use them, without
> > > spelunking through dozens of other projects to discover the secret DT
> > > that no one will tell us about.
> >
> > Should we demand better documentation for boards?  Yes.  But it's still
> > a valid case to have zero device trees for a given platform in-tree.
> > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > to work without adding special tweaks for us.  Maybe that means some
> > features can't be tested in QEMU-as-virtual-platform and only in
> > QEMU-faithfully-emulating-specific-physical-platforms.
> 
> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> case, I think. How about we create a special Kconfig for that case? We
> need to make some progress here.

Yes, because there's a small number of OF_BOARD=y configs in tree right
now, most of which are QEMU virtual machines, others of which are Pi
(which we've talked to death), highbank (which Andre has explained), and
then the octeontx stuff I don't know how works.  I keep pushing to say
that OF_BOARD=y is the special case 

Re: [PULL] u-boot-riscv/master

[Tom Rini]

On Fri, Dec 03, 2021 at 02:19:32PM +0800, Leo Liang wrote:

> Hi Tom,
> 
> The following changes since commit 4a14bfffd42f968ed9d72a780a8d44a9053c5b95:
> 
>   Merge https://source.denx.de/u-boot/custodians/u-boot-marvell (2021-11-30 
> 08:59:22 -0500)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-riscv.git 
> 
> for you to fetch changes up to c0ffc12a701621dc72dfc896965cbfe5b0dbf9b4:
> 
>   riscv: Enable SPI flash env for SiFive Unmatched. (2021-12-02 16:43:56 
> +0800)
> 
> CI result shows no issue: 
> https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/10128
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v6 01/25] doc: Add documentation about devicetree usage

[Simon Glass]

Hi François,

On Sat, 4 Dec 2021 at 04:06, François Ozog  wrote:
>
> Hi Simon
>
> Le sam. 4 déc. 2021 à 02:02, Simon Glass  a écrit :
>>
>> Hi Heinrich,
>>
>> On Fri, 3 Dec 2021 at 13:28, Heinrich Schuchardt  wrote:
>> >
>> > On 12/3/21 9:13 PM, Simon Glass wrote:
>> > > Hi Heinrich,
>> > >
>> > > On Fri, 3 Dec 2021 at 06:09, Heinrich Schuchardt  
>> > > wrote:
>> > >>
>> > >> On 12/3/21 13:34, Heinrich Schuchardt wrote:
>> > >>> On 12/2/21 16:58, Simon Glass wrote:
>> >  At present some of the ideas and techniques behind devicetree in 
>> >  U-Boot
>> >  are assumed, implied or unsaid. Add some documentation to cover how
>> >  devicetree is build, how it can be modified and the rules about using
>> >  the various CONFIG_OF_... options.
>> > 
>> >  Signed-off-by: Simon Glass 
>> >  Reviewed-by: Marcel Ziswiler 
>> >  ---
>> >  This patch attracted quite a bit of discussion here:
>> > 
>> >  https://patchwork.ozlabs.org/project/uboot/patch/20210909201033.755713-4-...@chromium.org/
>> > 
>> > 
>> >  I have not included the text suggested by François. While I agree that
>> >  it would be useful to have an introduction in this space, I do not 
>> >  agree
>> >  that we should have two devicetrees or that U-Boot should not have its
>> >  own
>> >  things in the devicetree, so it is not clear to me what we should
>> >  actually
>> >  write.
>> > 
>> >  The 'Devicetree Control in U-Boot' docs were recently merged and these
>> >  provide some base info, for now.
>> > 
>> >  Changes in v6:
>> >  - Fix description of OF_BOARD so it refers just to the current state
>> >  - Explain that the 'two devicetrees' refers to two *control* 
>> >  devicetrees
>> > 
>> >  Changes in v5:
>> >  - Bring into the OF_BOARD series
>> >  - Rebase to master and drop mention of OF_PRIOR_STAGE, since removed
>> >  - Refer to the 'control' DTB in the first paragraph
>> >  - Use QEMU instead of qemu
>> > 
>> >  Changes in v3:
>> >  - Clarify the 'bug' refered to at the top
>> >  - Reword 'This means that there' paragraph to explain U-Boot-specific
>> >  things
>> >  - Move to doc/develop/devicetree now that OF_CONTROL is in the docs
>> > 
>> >  Changes in v2:
>> >  - Fix typos per Sean (thank you!) and a few others
>> >  - Add a 'Use of U-Boot /config node' section
>> >  - Drop mention of dm-verity since that actually uses the kernel 
>> >  cmdline
>> >  - Explain that OF_BOARD will still work after these changes (in
>> >  'Once this bug is fixed...' paragraph)
>> >  - Expand a bit on the reason why the 'Current situation' is bad
>> >  - Clarify in a second place that Linux and U-Boot use the same 
>> >  devicetree
>> >  in 'To be clear, while U-Boot...'
>> >  - Expand on why we should have rules for other projects in
>> >  'Devicetree in another project'
>> >  - Add a comment as to why devicetree in U-Boot is not 'bad design'
>> >  - Reword 'in-tree U-Boot devicetree' to 'devicetree source in U-Boot'
>> >  - Rewrite 'Devicetree generated on-the-fly in another project' to 
>> >  cover
>> >  points raised on v1
>> >  - Add 'Why does U-Boot have its nodes and properties?'
>> >  - Add 'Why not have two devicetrees?'
>> > 
>> > doc/develop/devicetree/dt_update.rst | 555 
>> >  +++
>> > doc/develop/devicetree/index.rst |   1 +
>> > 2 files changed, 556 insertions(+)
>> > create mode 100644 doc/develop/devicetree/dt_update.rst
>> > 
>> > > [..]
>> > >
>> >  +
>> >  +- The other project may not provide a way to support U-Boot's
>> >  requirements for
>> >  +  devicetree, such as the /config node. Note: On the U-Boot mailing
>> >  linst, this
>> > >>>
>> > >>> Even if you remove these lines in 17/25 I would prefer not to introduce
>> > >>> typos here:
>> > >>>
>> > >>> %s/linst/list/
>> > >>>
>> > >
>> > > OK I can fix that.
>> > >
>> > > [..]
>> > >
>> >  +Normally, supporting U-Boot's features is trivial, since the
>> >  devicetree compiler
>> >  +(dtc) can compile the source, including any U-Boot pieces. So the
>> >  burden is
>> >  +extremely low.
>> >  +
>> >  +In this case, the devicetree in the other project must track U-Boot's
>> >  use of
>> >  +device tree, so that it remains compatible. See `Devicetree in
>> >  another project`_
>> >  +for reasons why.
>> > >>>
>> > >>> Did you ever ask the QEMU community what they think about your ideas?
>> > >>> What was the reply?
>> > >>
>> > >> Looking at the thread
>> > >> https://lore.kernel.org/all/20210926183410.256484-1-...@chromium.org/
>> > >> the QEMU project said NAK. This matches the expectation that I expressed
>> > >> repeatedly.
>> > >>
>> > >> Why don't you mention the QEMU reply in this 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi François,

On Sat, 4 Dec 2021 at 09:55, François Ozog  wrote:
>
> Hi Simon
>
> Le sam. 4 déc. 2021 à 16:21, Simon Glass  a écrit :
>>
>> Hi Tom,
>>
>> On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
>> >
>> > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
>> >
>> > [huge snip]
>> > > > There's things that need to be cleaned up because we have some small
>> > > > number of platforms that went off and did their own thing.  But largely
>> > > > yes, things make sense to me.  We have:
>> > > > - We embedded the device tree that will configure U-Boot, because there
>> > > >   is no way for the hardware to have provided us one.
>> > > > - We do not embed the device tree that will configure U-Boot, because
>> > > >   there is already one present in memory for us to use.
>> > > >
>> > > > Then we have the developer option of:
>> > > > - We embedded the device tree that will configure U-Boot, because we're
>> > > >   developing something.
>> > >
>> > > Yes, agreed those are the cases. To me this needs to be a run-time 
>> > > choice.
>> >
>> > But it's not possible.  That's the problem we keep going around and
>> > around about.  People keep raising real life examples where you cannot
>> > make a run time choice between "device tree we're passed at run time"
>> > and "device tree we're compiled with".
>>
>> I haven't seen one. The most extreme case is QEMU and it works fine. I
>> even added a test with it. What am I missing?
>>
>> >
>> > And it's not helpful.  It is ALWAYS the case that we know that we want
>> > to override the run time device tree with our own, because it's a
>> > developer developing things or it's a user / production case where we
>> > must use the provided tree.  NOT doing that is what leads to madness
>> > like we see for example on Pi where if we don't use the passed tree we
>> > still need to copy X/Y/Z out of it.
>>
>> Aren't you talking about the distro DT there, rather than the the one
>> on the boot disk? That is my reading of that patch. If we need to do
>> that sort of thing, it doesn't matter where the the cointrol DT comes
>> from. You are still going to have to do that sort of thing.
>>
>> It is not ALWAYS the case. I've shown you how easy it is to disable
>> OF_BOARD and still boot / iterate.
>>
>> >
>> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
>> > > > > provide a way to do that? But what is driving that desire?
>> > > >
>> > > > I'm looking for ways to convince you that we do not need to include a
>> > > > device tree in the binary.  There's a growing set of devices where the
>> > > > device tree exists with the device.  If it's missing, that's a huge
>> > > > fatal error we can't do all that much about.  If we need to do 
>> > > > something
>> > > > to that device tree for U-Boot, yes, fine, we should make it straight
>> > > > forward for the developer to do that.  But that's not the common case!
>> > >
>> > > Well we could add another Kconfig which tells U-Boot not to include a
>> > > devicetree in u-boot.bin, if that would resolve this?
>> > >
>> > > I just want to make sure that we always build the devicetrees and that
>> > > it is easy for a knowledgeable dev to switch over to use them, without
>> > > spelunking through dozens of other projects to discover the secret DT
>> > > that no one will tell us about.
>> >
>> > Should we demand better documentation for boards?  Yes.  But it's still
>> > a valid case to have zero device trees for a given platform in-tree.
>> > Xen is an example of this.  QEMU is an example of this.  Platforms need
>> > to work without adding special tweaks for us.  Maybe that means some
>> > features can't be tested in QEMU-as-virtual-platform and only in
>> > QEMU-faithfully-emulating-specific-physical-platforms.
>>
>> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
>> case, I think. How about we create a special Kconfig for that case? We
>> need to make some progress here.
>>
>> >
>> > > > I guess another part of the problem is that historically almost all
>> > > > platforms were in the first case I list above, no run time provided
>> > > > device tree, so we took the kernel one and added our bindings to it.
>> > > > Now we're being bit by the growing number of platforms that are the
>> > > > second case, and how do we get our properties in there, and which ones
>> > > > even make sense to do that for.
>> > >
>> > > I think upstreaming the bindings is the solution there. I've made a
>> > > start, but we need to make progress with this series and all the other
>> > > things in flight. I think a lot of people want U-Boot to not have a
>> > > devicetree source files in it for ARMv8 platforms. I am strongly
>> > > opposed to that. I've laid out my reasons very clearly in the past. I
>> > > think this is a good summary:
>> > >
>> > > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03480.html
>> >
>> > Yes, there are some ARMv8 platforms we will have to have the source
>> > 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Mark,

On Sat, 4 Dec 2021 at 09:09, Mark Kettenis  wrote:
>
> > From: Simon Glass 
> > Date: Sat, 4 Dec 2021 08:20:55 -0700
> >
> > Hi Tom,
> >
> > On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> > >
> > > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> > >
> > > [huge snip]
> > > > > There's things that need to be cleaned up because we have some small
> > > > > number of platforms that went off and did their own thing.  But 
> > > > > largely
> > > > > yes, things make sense to me.  We have:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > there
> > > > >   is no way for the hardware to have provided us one.
> > > > > - We do not embed the device tree that will configure U-Boot, because
> > > > >   there is already one present in memory for us to use.
> > > > >
> > > > > Then we have the developer option of:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > we're
> > > > >   developing something.
> > > >
> > > > Yes, agreed those are the cases. To me this needs to be a run-time 
> > > > choice.
> > >
> > > But it's not possible.  That's the problem we keep going around and
> > > around about.  People keep raising real life examples where you cannot
> > > make a run time choice between "device tree we're passed at run time"
> > > and "device tree we're compiled with".
> >
> > I haven't seen one. The most extreme case is QEMU and it works fine. I
> > even added a test with it. What am I missing?
>
> Try it on your M1 Mac.

I have tested this series on a Mac Mini and it works fine.

You cannot run U-Boot by itself, of course, since it needs m1n1. Also
m1n1 fixes up the DT. I'm not sure exactly what changes it makes, as I
don't even have a serial console on the machine at present. But in any
case, I can easily iterate on U-Boot on this platform with U-Boot
building the DTs. It certainly meets my requirements at present.

>
> > > And it's not helpful.  It is ALWAYS the case that we know that we want
> > > to override the run time device tree with our own, because it's a
> > > developer developing things or it's a user / production case where we
> > > must use the provided tree.  NOT doing that is what leads to madness
> > > like we see for example on Pi where if we don't use the passed tree we
> > > still need to copy X/Y/Z out of it.
> >
> > Aren't you talking about the distro DT there, rather than the the one
> > on the boot disk? That is my reading of that patch. If we need to do
> > that sort of thing, it doesn't matter where the the cointrol DT comes
> > from. You are still going to have to do that sort of thing.
> >
> > It is not ALWAYS the case. I've shown you how easy it is to disable
> > OF_BOARD and still boot / iterate.
>
> You are cheating.  If you dump the device tree passed to U-Boot (or
> the OS) and don't change anything in the configuration of whatever is
> passing U-Boot that device tree this is likely to work fine.  But it
> won't work in general.

I didn't say it would. What are you getting at?

Can you please try to test my series so you get the hang of what it is
actually doing?

>
> > > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > > provide a way to do that? But what is driving that desire?
> > > > >
> > > > > I'm looking for ways to convince you that we do not need to include a
> > > > > device tree in the binary.  There's a growing set of devices where the
> > > > > device tree exists with the device.  If it's missing, that's a huge
> > > > > fatal error we can't do all that much about.  If we need to do 
> > > > > something
> > > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > > forward for the developer to do that.  But that's not the common case!
> > > >
> > > > Well we could add another Kconfig which tells U-Boot not to include a
> > > > devicetree in u-boot.bin, if that would resolve this?
> > > >
> > > > I just want to make sure that we always build the devicetrees and that
> > > > it is easy for a knowledgeable dev to switch over to use them, without
> > > > spelunking through dozens of other projects to discover the secret DT
> > > > that no one will tell us about.
> > >
> > > Should we demand better documentation for boards?  Yes.  But it's still
> > > a valid case to have zero device trees for a given platform in-tree.
> > > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > > to work without adding special tweaks for us.  Maybe that means some
> > > features can't be tested in QEMU-as-virtual-platform and only in
> > > QEMU-faithfully-emulating-specific-physical-platforms.
> >
> > You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> > case, I think. How about we create a special Kconfig for that case? We
> > need to make some progress here.
>
> No, Apple M1 and Raspberry Pi 4 also fall into that case.  And I
> suspect at least some of the other boards that currently 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Ilias,

On Sat, 4 Dec 2021 at 08:58, Ilias Apalodimas
 wrote:
>
> Hi Simon,
>
> [...]
> > > [huge snip]
> > > > > There's things that need to be cleaned up because we have some small
> > > > > number of platforms that went off and did their own thing.  But 
> > > > > largely
> > > > > yes, things make sense to me.  We have:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > there
> > > > >   is no way for the hardware to have provided us one.
> > > > > - We do not embed the device tree that will configure U-Boot, because
> > > > >   there is already one present in memory for us to use.
> > > > >
> > > > > Then we have the developer option of:
> > > > > - We embedded the device tree that will configure U-Boot, because 
> > > > > we're
> > > > >   developing something.
> > > >
> > > > Yes, agreed those are the cases. To me this needs to be a run-time 
> > > > choice.
> > >
>
> I am not convinced the case is "we are developing something".  The
> arguments for this are something along the lines of:
> 1. U-Boot will fail to compile if OF_SEPARATE is selected and you provide
> no DT so we need to include it.  With which I disagree.  The config option
> says "You must provide an external device tree" for a reason.

Where are you reading that? I'm lost.

OF_SEPARATE means it is not embedded in U-Boot but is attached to the end of it.

> 2. The DT's are  hard to find.  The 'hard to find' RPI DTs are in fact
> committed in the kernel. I am not sure about the rest, but honestly this
> isn't a convincing argument for me.

Great, so you've solved that one but even that was confusing for me,
as my patch should make clear. Every single board would be its own
special snowflake if we went that way.

So if they are in the kernel, we just need to put them in U-Boot too,
so problem solved. The auto-sync thing that I believe Rob is working
on will make things easy.

>
> What else are we gaining with it being a run time choice?  One of the

We are requiring a DT to be present in the U-Boot tree for
development, documentation and discoverability purposes. Devs can turn
OF_BOARD on and off as it suits them when iterating on a platform.

> things this approach does address, but we keep conveniently ignoring, is
> that it tries to preserve the current status quo.  You can go and add the
> special missing U-Boot nodes in those DT's.  So that would bypass problems
> if the bindings get NAK'ed.  But this is going to work against the
> fragmentation we are trying to resolve.

Well that's another reason why we need in-tree DTs at the moment. That
reason goes away once we have our bindings upstream and are able to do
what we need with DT there.

>
> > > But it's not possible.  That's the problem we keep going around and
> > > around about.  People keep raising real life examples where you cannot
> > > make a run time choice between "device tree we're passed at run time"
> > > and "device tree we're compiled with".
> >
> > I haven't seen one. The most extreme case is QEMU and it works fine. I
> > even added a test with it. What am I missing?

(I think my point there is made)

>
> IMHO 3b595da441cf is the commit that started the problem and tangled those
> 2 options.  Note that this support has been removed from the dragonboard
> later in 0204d1b56b2f 

Yes it is one of them. There may be cases where we want to patch up
the DT that U-Boot builds. In fact OF_BOARD does not mean it came from
a prior stage. All sorts of things could be going on. We should not
conflate building a DT with OF_BOARD.

>
> >
> > >
> > > And it's not helpful.  It is ALWAYS the case that we know that we want
> > > to override the run time device tree with our own, because it's a
> > > developer developing things or it's a user / production case where we
> > > must use the provided tree.  NOT doing that is what leads to madness
> > > like we see for example on Pi where if we don't use the passed tree we
> > > still need to copy X/Y/Z out of it.
> >
> > Aren't you talking about the distro DT there, rather than the the one
> > on the boot disk? That is my reading of that patch. If we need to do
> > that sort of thing, it doesn't matter where the the cointrol DT comes
> > from. You are still going to have to do that sort of thing.
> >
> > It is not ALWAYS the case. I've shown you how easy it is to disable
> > OF_BOARD and still boot / iterate.
> >
> > >
> > > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > > provide a way to do that? But what is driving that desire?
> > > > >
> > > > > I'm looking for ways to convince you that we do not need to include a
> > > > > device tree in the binary.  There's a growing set of devices where the
> > > > > device tree exists with the device.  If it's missing, that's a huge
> > > > > fatal error we can't do all that much about.  If we need to do 
> > > > > something
> > > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > > 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[François Ozog]

Hi Simon

Le sam. 4 déc. 2021 à 16:21, Simon Glass  a écrit :

> Hi Tom,
>
> On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> >
> > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> >
> > [huge snip]
> > > > There's things that need to be cleaned up because we have some small
> > > > number of platforms that went off and did their own thing.  But
> largely
> > > > yes, things make sense to me.  We have:
> > > > - We embedded the device tree that will configure U-Boot, because
> there
> > > >   is no way for the hardware to have provided us one.
> > > > - We do not embed the device tree that will configure U-Boot, because
> > > >   there is already one present in memory for us to use.
> > > >
> > > > Then we have the developer option of:
> > > > - We embedded the device tree that will configure U-Boot, because
> we're
> > > >   developing something.
> > >
> > > Yes, agreed those are the cases. To me this needs to be a run-time
> choice.
> >
> > But it's not possible.  That's the problem we keep going around and
> > around about.  People keep raising real life examples where you cannot
> > make a run time choice between "device tree we're passed at run time"
> > and "device tree we're compiled with".
>
> I haven't seen one. The most extreme case is QEMU and it works fine. I
> even added a test with it. What am I missing?
>
> >
> > And it's not helpful.  It is ALWAYS the case that we know that we want
> > to override the run time device tree with our own, because it's a
> > developer developing things or it's a user / production case where we
> > must use the provided tree.  NOT doing that is what leads to madness
> > like we see for example on Pi where if we don't use the passed tree we
> > still need to copy X/Y/Z out of it.
>
> Aren't you talking about the distro DT there, rather than the the one
> on the boot disk? That is my reading of that patch. If we need to do
> that sort of thing, it doesn't matter where the the cointrol DT comes
> from. You are still going to have to do that sort of thing.
>
> It is not ALWAYS the case. I've shown you how easy it is to disable
> OF_BOARD and still boot / iterate.
>
> >
> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we
> should
> > > > > provide a way to do that? But what is driving that desire?
> > > >
> > > > I'm looking for ways to convince you that we do not need to include a
> > > > device tree in the binary.  There's a growing set of devices where
> the
> > > > device tree exists with the device.  If it's missing, that's a huge
> > > > fatal error we can't do all that much about.  If we need to do
> something
> > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > forward for the developer to do that.  But that's not the common
> case!
> > >
> > > Well we could add another Kconfig which tells U-Boot not to include a
> > > devicetree in u-boot.bin, if that would resolve this?
> > >
> > > I just want to make sure that we always build the devicetrees and that
> > > it is easy for a knowledgeable dev to switch over to use them, without
> > > spelunking through dozens of other projects to discover the secret DT
> > > that no one will tell us about.
> >
> > Should we demand better documentation for boards?  Yes.  But it's still
> > a valid case to have zero device trees for a given platform in-tree.
> > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > to work without adding special tweaks for us.  Maybe that means some
> > features can't be tested in QEMU-as-virtual-platform and only in
> > QEMU-faithfully-emulating-specific-physical-platforms.
>
> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> case, I think. How about we create a special Kconfig for that case? We
> need to make some progress here.
>
> >
> > > > I guess another part of the problem is that historically almost all
> > > > platforms were in the first case I list above, no run time provided
> > > > device tree, so we took the kernel one and added our bindings to it.
> > > > Now we're being bit by the growing number of platforms that are the
> > > > second case, and how do we get our properties in there, and which
> ones
> > > > even make sense to do that for.
> > >
> > > I think upstreaming the bindings is the solution there. I've made a
> > > start, but we need to make progress with this series and all the other
> > > things in flight. I think a lot of people want U-Boot to not have a
> > > devicetree source files in it for ARMv8 platforms. I am strongly
> > > opposed to that. I've laid out my reasons very clearly in the past. I
> > > think this is a good summary:
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03480.html
> >
> > Yes, there are some ARMv8 platforms we will have to have the source
> > files to, in tree, because they won't come to us at run time.  But
> > others we won't for practical reasons, namely that we can't statically
> > provide something that 

[PATCH v5 27/28] x86: efi: Set the correct link flags for the 64-bit EFI app

[Simon Glass]

At present some 32-bit settings are used with the 64-bit app. Fix this by
separating out the two cases.

Be careful not to break the 64-bit payload, which needs to build a 64-bit
EFI stub with a 32-bit U-Boot.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to set the correct link flags for the 64-bit EFI app

 arch/x86/config.mk | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index 589f2aed2bc..889497b6bd7 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -20,6 +20,11 @@ IS_32BIT := y
 endif
 endif
 
+EFI_IS_32BIT := $(IS_32BIT)
+ifdef CONFIG_EFI_STUB_64BIT
+EFI_IS_32BIT :=
+endif
+
 ifeq ($(IS_32BIT),y)
 PLATFORM_CPPFLAGS += -march=i386 -m32
 else
@@ -44,8 +49,14 @@ CFLAGS_EFI := -fpic -fshort-wchar
 # Compiler flags to be removed when building UEFI applications
 CFLAGS_NON_EFI := -mregparm=3 -fstack-protector-strong
 
-ifeq ($(CONFIG_EFI_STUB_64BIT),)
+ifeq ($(IS_32BIT),y)
+EFIPAYLOAD_BFDARCH = i386
+else
 CFLAGS_EFI += $(call cc-option, -mno-red-zone)
+EFIPAYLOAD_BFDARCH = x86_64
+endif
+
+ifeq ($(EFI_IS_32BIT),y)
 EFIARCH = ia32
 EFIPAYLOAD_BFDTARGET = elf32-i386
 else
@@ -53,8 +64,6 @@ EFIARCH = x86_64
 EFIPAYLOAD_BFDTARGET = elf64-x86-64
 endif
 
-EFIPAYLOAD_BFDARCH = i386
-
 LDSCRIPT_EFI := $(srctree)/arch/x86/lib/elf_$(EFIARCH)_efi.lds
 EFISTUB := crt0_$(EFIARCH)_efi.o reloc_$(EFIARCH)_efi.o
 OBJCOPYFLAGS_EFI += --target=efi-app-$(EFIARCH)
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 21/28] efi: Support the efi command in the app

[Simon Glass]

At present the 'efi' command only works in the EFI payload. Update it to
work in the app too, so the memory map can be examined.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 cmd/Makefile  |  2 +-
 cmd/efi.c | 48 ---
 include/efi.h | 15 +++
 lib/efi/efi_app.c | 33 
 4 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/cmd/Makefile b/cmd/Makefile
index 891819ae0f6..df50625bde7 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -58,7 +58,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o
 obj-$(CONFIG_CMD_ECHO) += echo.o
 obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
 obj-$(CONFIG_CMD_EEPROM) += eeprom.o
-obj-$(CONFIG_EFI_STUB) += efi.o
+obj-$(CONFIG_EFI) += efi.o
 obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
 obj-$(CONFIG_CMD_ELF) += elf.o
 obj-$(CONFIG_HUSH_PARSER) += exit.o
diff --git a/cmd/efi.c b/cmd/efi.c
index d2400acbbba..c0384e0db28 100644
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -13,6 +13,8 @@
 #include 
 #include 
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static const char *const type_name[] = {
"reserved",
"loader_code",
@@ -217,37 +219,53 @@ static void efi_print_mem_table(struct efi_mem_desc 
*desc, int desc_size,
 static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
-   struct efi_mem_desc *desc;
-   struct efi_entry_memmap *map;
+   struct efi_mem_desc *orig, *desc;
+   uint version, key;
+   int desc_size;
int size, ret;
bool skip_bs;
 
skip_bs = !argc || *argv[0] != 'a';
-   ret = efi_info_get(EFIET_MEMORY_MAP, (void **), );
-   switch (ret) {
-   case -ENOENT:
-   printf("No EFI table available\n");
-   goto done;
-   case -EPROTONOSUPPORT:
-   printf("Incorrect EFI table version\n");
-   goto done;
+   if (IS_ENABLED(CONFIG_EFI_APP)) {
+   ret = efi_get_mmap(, , , _size, );
+   if (ret) {
+   printf("Cannot read memory map (err=%d)\n", ret);
+   return CMD_RET_FAILURE;
+   }
+   } else {
+   struct efi_entry_memmap *map;
+
+   ret = efi_info_get(EFIET_MEMORY_MAP, (void **), );
+   switch (ret) {
+   case -ENOENT:
+   printf("No EFI table available\n");
+   goto done;
+   case -EPROTONOSUPPORT:
+   printf("Incorrect EFI table version\n");
+   goto done;
+   }
+   orig = map->desc;
+   desc_size = map->desc_size;
+   version = map->version;
}
-   printf("EFI table at %lx, memory map %p, size %x, version %x, descr. 
size %#x\n",
-  gd->arch.table, map, size, map->version, map->desc_size);
-   if (map->version != EFI_MEM_DESC_VERSION) {
+   printf("EFI table at %lx, memory map %p, size %x, key %x, version %x, 
descr. size %#x\n",
+  gd->arch.table, orig, size, key, version, desc_size);
+   if (version != EFI_MEM_DESC_VERSION) {
printf("Incorrect memory map version\n");
ret = -EPROTONOSUPPORT;
goto done;
}
 
-   desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs);
+   desc = efi_build_mem_table(orig, size, desc_size, skip_bs);
if (!desc) {
ret = -ENOMEM;
goto done;
}
 
-   efi_print_mem_table(desc, map->desc_size, skip_bs);
+   efi_print_mem_table(desc, desc_size, skip_bs);
free(desc);
+   if (IS_ENABLED(CONFIG_EFI_APP))
+   free(orig);
 done:
if (ret)
printf("Error: %d\n", ret);
diff --git a/include/efi.h b/include/efi.h
index 1dc806a1267..8a43430d3df 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -610,4 +610,19 @@ int efi_store_memory_map(struct efi_priv *priv);
  */
 int efi_call_exit_boot_services(void);
 
+/**
+ * efi_get_mmap() - Get the memory map from EFI
+ *
+ * This is used in the app. The caller must free *@descp when done
+ *
+ * @descp: Returns allocated pointer to EFI memory map table
+ * @sizep: Returns size of table in bytes
+ * @keyp:  Returns memory-map key
+ * @desc_sizep:Returns size of each @desc_base record
+ * @versionp:  Returns version number of memory map
+ * @return 0 on success, -ve on error
+ */
+int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp,
+int *desc_sizep, uint *versionp);
+
 #endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index 36e3f1de427..55fa1ac57d5 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -32,6 +32,39 @@ int efi_info_get(enum efi_entry_t type, void **datap, int 
*sizep)
return -ENOSYS;
 }
 
+int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp,
+  

[PATCH v5 25/28] x86: efi: Round out the link script for 64-bit EFI

[Simon Glass]

Make sure the linker lists are in the right place and drop the eh_frame
section, which is not needed.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to round out the link script for 64-bit EFI

 arch/x86/lib/elf_x86_64_efi.lds | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/lib/elf_x86_64_efi.lds b/arch/x86/lib/elf_x86_64_efi.lds
index b436429b33e..75727400aa4 100644
--- a/arch/x86/lib/elf_x86_64_efi.lds
+++ b/arch/x86/lib/elf_x86_64_efi.lds
@@ -63,6 +63,7 @@ SECTIONS
*(.rela.data*)
*(.rela.got)
*(.rela.stab)
+*(.rela.u_boot_list*)
}
 
. = ALIGN(4096);
@@ -70,9 +71,11 @@ SECTIONS
. = ALIGN(4096);
.dynstr : { *(.dynstr) }
. = ALIGN(4096);
+
+/DISCARD/ : { *(.eh_frame) }
+
.ignored.reloc : {
*(.rela.reloc)
-   *(.eh_frame)
*(.note.GNU-stack)
}
 
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 23/28] x86: efi: Don't set up global_data again with EFI

[Simon Glass]

Since EFI does not relocate and uses the same global_data pointer
throughout the board-init process, drop this unnecessary setup, to avoid
a hang.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to avoid setting up global_data again with EFI

 common/board_r.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 31a59c585a8..8b5948100b1 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -817,9 +817,8 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
 * TODO(s...@chromium.org): Consider doing this for all archs, or
 * dropping the new_gd parameter.
 */
-#if CONFIG_IS_ENABLED(X86_64)
-   arch_setup_gd(new_gd);
-#endif
+   if (CONFIG_IS_ENABLED(X86_64) && !IS_ENABLED(CONFIG_EFI_APP))
+   arch_setup_gd(new_gd);
 
 #ifdef CONFIG_NEEDS_MANUAL_RELOC
int i;
-- 
2.34.1.400.ga245620fadb-goog

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Mark Kettenis]

> From: Simon Glass 
> Date: Sat, 4 Dec 2021 08:20:55 -0700
> 
> Hi Tom,
> 
> On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
> >
> > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> >
> > [huge snip]
> > > > There's things that need to be cleaned up because we have some small
> > > > number of platforms that went off and did their own thing.  But largely
> > > > yes, things make sense to me.  We have:
> > > > - We embedded the device tree that will configure U-Boot, because there
> > > >   is no way for the hardware to have provided us one.
> > > > - We do not embed the device tree that will configure U-Boot, because
> > > >   there is already one present in memory for us to use.
> > > >
> > > > Then we have the developer option of:
> > > > - We embedded the device tree that will configure U-Boot, because we're
> > > >   developing something.
> > >
> > > Yes, agreed those are the cases. To me this needs to be a run-time choice.
> >
> > But it's not possible.  That's the problem we keep going around and
> > around about.  People keep raising real life examples where you cannot
> > make a run time choice between "device tree we're passed at run time"
> > and "device tree we're compiled with".
> 
> I haven't seen one. The most extreme case is QEMU and it works fine. I
> even added a test with it. What am I missing?

Try it on your M1 Mac.

> > And it's not helpful.  It is ALWAYS the case that we know that we want
> > to override the run time device tree with our own, because it's a
> > developer developing things or it's a user / production case where we
> > must use the provided tree.  NOT doing that is what leads to madness
> > like we see for example on Pi where if we don't use the passed tree we
> > still need to copy X/Y/Z out of it.
> 
> Aren't you talking about the distro DT there, rather than the the one
> on the boot disk? That is my reading of that patch. If we need to do
> that sort of thing, it doesn't matter where the the cointrol DT comes
> from. You are still going to have to do that sort of thing.
> 
> It is not ALWAYS the case. I've shown you how easy it is to disable
> OF_BOARD and still boot / iterate.

You are cheating.  If you dump the device tree passed to U-Boot (or
the OS) and don't change anything in the configuration of whatever is
passing U-Boot that device tree this is likely to work fine.  But it
won't work in general.

> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > provide a way to do that? But what is driving that desire?
> > > >
> > > > I'm looking for ways to convince you that we do not need to include a
> > > > device tree in the binary.  There's a growing set of devices where the
> > > > device tree exists with the device.  If it's missing, that's a huge
> > > > fatal error we can't do all that much about.  If we need to do something
> > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > forward for the developer to do that.  But that's not the common case!
> > >
> > > Well we could add another Kconfig which tells U-Boot not to include a
> > > devicetree in u-boot.bin, if that would resolve this?
> > >
> > > I just want to make sure that we always build the devicetrees and that
> > > it is easy for a knowledgeable dev to switch over to use them, without
> > > spelunking through dozens of other projects to discover the secret DT
> > > that no one will tell us about.
> >
> > Should we demand better documentation for boards?  Yes.  But it's still
> > a valid case to have zero device trees for a given platform in-tree.
> > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > to work without adding special tweaks for us.  Maybe that means some
> > features can't be tested in QEMU-as-virtual-platform and only in
> > QEMU-faithfully-emulating-specific-physical-platforms.
> 
> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> case, I think. How about we create a special Kconfig for that case? We
> need to make some progress here.

No, Apple M1 and Raspberry Pi 4 also fall into that case.  And I
suspect at least some of the other boards that currently defined
OF_BOARD fall into this category.  So rather than changing the meaning
of OF_BOARD, how about introducing another Kconfig option that
indicates that switching to a device tree built into U-Boot is ok?
You could use that to implement the runtime switching that you desire.
This would prevent confusing users with options that result in making
their systems unusable.

> > > > I guess another part of the problem is that historically almost all
> > > > platforms were in the first case I list above, no run time provided
> > > > device tree, so we took the kernel one and added our bindings to it.
> > > > Now we're being bit by the growing number of platforms that are the
> > > > second case, and how do we get our properties in there, and which ones
> > > > even make sense to do that for.
> > >
> > > I think 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Ilias Apalodimas]

Hi Simon,

[...]
> > [huge snip]
> > > > There's things that need to be cleaned up because we have some small
> > > > number of platforms that went off and did their own thing.  But largely
> > > > yes, things make sense to me.  We have:
> > > > - We embedded the device tree that will configure U-Boot, because there
> > > >   is no way for the hardware to have provided us one.
> > > > - We do not embed the device tree that will configure U-Boot, because
> > > >   there is already one present in memory for us to use.
> > > >
> > > > Then we have the developer option of:
> > > > - We embedded the device tree that will configure U-Boot, because we're
> > > >   developing something.
> > >
> > > Yes, agreed those are the cases. To me this needs to be a run-time choice.
> >

I am not convinced the case is "we are developing something".  The
arguments for this are something along the lines of:
1. U-Boot will fail to compile if OF_SEPARATE is selected and you provide
no DT so we need to include it.  With which I disagree.  The config option
says "You must provide an external device tree" for a reason.
2. The DT's are  hard to find.  The 'hard to find' RPI DTs are in fact
committed in the kernel. I am not sure about the rest, but honestly this
isn't a convincing argument for me.

What else are we gaining with it being a run time choice?  One of the
things this approach does address, but we keep conveniently ignoring, is
that it tries to preserve the current status quo.  You can go and add the
special missing U-Boot nodes in those DT's.  So that would bypass problems
if the bindings get NAK'ed.  But this is going to work against the
fragmentation we are trying to resolve.

> > But it's not possible.  That's the problem we keep going around and
> > around about.  People keep raising real life examples where you cannot
> > make a run time choice between "device tree we're passed at run time"
> > and "device tree we're compiled with".
> 
> I haven't seen one. The most extreme case is QEMU and it works fine. I
> even added a test with it. What am I missing?

IMHO 3b595da441cf is the commit that started the problem and tangled those
2 options.  Note that this support has been removed from the dragonboard
later in 0204d1b56b2f 

> 
> >
> > And it's not helpful.  It is ALWAYS the case that we know that we want
> > to override the run time device tree with our own, because it's a
> > developer developing things or it's a user / production case where we
> > must use the provided tree.  NOT doing that is what leads to madness
> > like we see for example on Pi where if we don't use the passed tree we
> > still need to copy X/Y/Z out of it.
> 
> Aren't you talking about the distro DT there, rather than the the one
> on the boot disk? That is my reading of that patch. If we need to do
> that sort of thing, it doesn't matter where the the cointrol DT comes
> from. You are still going to have to do that sort of thing.
> 
> It is not ALWAYS the case. I've shown you how easy it is to disable
> OF_BOARD and still boot / iterate.
> 
> >
> > > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > > provide a way to do that? But what is driving that desire?
> > > >
> > > > I'm looking for ways to convince you that we do not need to include a
> > > > device tree in the binary.  There's a growing set of devices where the
> > > > device tree exists with the device.  If it's missing, that's a huge
> > > > fatal error we can't do all that much about.  If we need to do something
> > > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > > forward for the developer to do that.  But that's not the common case!
> > >
> > > Well we could add another Kconfig which tells U-Boot not to include a
> > > devicetree in u-boot.bin, if that would resolve this?
> > >
> > > I just want to make sure that we always build the devicetrees and that
> > > it is easy for a knowledgeable dev to switch over to use them, without
> > > spelunking through dozens of other projects to discover the secret DT
> > > that no one will tell us about.

So the translation of this for me is:
We have 2 discrete options (that can be cleaned up further) which choose
either to embed or receive the DTB.  Let's tangle them and introduce a
*new* third option to separate them if someone needs to.  Which makes no
sense to me.

> >
> > Should we demand better documentation for boards?  Yes.  But it's still
> > a valid case to have zero device trees for a given platform in-tree.
> > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > to work without adding special tweaks for us.  Maybe that means some
> > features can't be tested in QEMU-as-virtual-platform and only in
> > QEMU-faithfully-emulating-specific-physical-platforms.
> 
> You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> case, I think. How about we create a special Kconfig for that case? We
> need to make some progress here.
> 
> 

[PATCH v5 26/28] x86: efi: Don't use the 64-bit link script for the EFI app

[Simon Glass]

That script is not intended for use with EFI, so update the logic to avoid
using it.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to avoid using the 64-bit link script for the EFI app

 arch/x86/cpu/config.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/config.mk b/arch/x86/cpu/config.mk
index d3033b41603..87e242a2065 100644
--- a/arch/x86/cpu/config.mk
+++ b/arch/x86/cpu/config.mk
@@ -9,7 +9,7 @@ LDPPFLAGS += -DRESET_VEC_LOC=$(CONFIG_RESET_VEC_LOC)
 LDPPFLAGS += -DSTART_16=$(CONFIG_SYS_X86_START16)
 
 ifdef CONFIG_X86_64
-ifndef CONFIG_SPL_BUILD
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_EFI_APP),)
 LDSCRIPT = $(srctree)/arch/x86/cpu/u-boot-64.lds
 endif
 endif
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 28/28] efi: Build the 64-bit app properly

[Simon Glass]

Now that the linker crash is resolved, build the 64-bit EFI app, including
all the required code.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to build the 64-bit app properly
- Add various patches to fix up the 64-bit app so that it boots

Changes in v2:
- Add new patch to support the efi command in the app

 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 57c3643d9a8..08b848bf048 100644
--- a/Makefile
+++ b/Makefile
@@ -1765,9 +1765,9 @@ else
 quiet_cmd_u-boot__ ?= LD  $@
   cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@  
\
-T u-boot.lds $(u-boot-init)
\
-   $(if $(CONFIG_EFI_APP_64BIT),,--whole-archive)  
\
+   --whole-archive 
\
$(u-boot-main)  
\
-   $(if $(CONFIG_EFI_APP_64BIT),,--no-whole-archive)   
\
+   --no-whole-archive  
\
$(PLATFORM_LIBS) -Map u-boot.map;   
\
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 endif
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 20/28] x86: efi: Update efi_get_next_mem_desc() to avoid needing a map

[Simon Glass]

At present this function requires a pointer to struct efi_entry_memmap
but the only field used in there is the desc_size. We want to be able
to use it from the app, so update it to use desc_size directly.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/x86/cpu/efi/payload.c |  8 
 cmd/efi.c  | 34 ++
 include/efi.h  |  4 ++--
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c
index 3a9f7d72868..d2aa889a2b9 100644
--- a/arch/x86/cpu/efi/payload.c
+++ b/arch/x86/cpu/efi/payload.c
@@ -50,7 +50,7 @@ ulong board_get_usable_ram_top(ulong total_size)
 
end = (struct efi_mem_desc *)((ulong)map + size);
desc = map->desc;
-   for (; desc < end; desc = efi_get_next_mem_desc(map, desc)) {
+   for (; desc < end; desc = efi_get_next_mem_desc(desc, map->desc_size)) {
if (desc->type != EFI_CONVENTIONAL_MEMORY ||
desc->physical_start >= 1ULL << 32)
continue;
@@ -88,7 +88,7 @@ int dram_init(void)
end = (struct efi_mem_desc *)((ulong)map + size);
gd->ram_size = 0;
desc = map->desc;
-   for (; desc < end; desc = efi_get_next_mem_desc(map, desc)) {
+   for (; desc < end; desc = efi_get_next_mem_desc(desc, map->desc_size)) {
if (desc->type < EFI_MMAP_IO)
gd->ram_size += desc->num_pages << EFI_PAGE_SHIFT;
}
@@ -113,7 +113,7 @@ int dram_init_banksize(void)
desc = map->desc;
for (num_banks = 0;
 desc < end && num_banks < CONFIG_NR_DRAM_BANKS;
-desc = efi_get_next_mem_desc(map, desc)) {
+desc = efi_get_next_mem_desc(desc, map->desc_size)) {
/*
 * We only use conventional memory and ignore
 * anything less than 1MB.
@@ -196,7 +196,7 @@ unsigned int install_e820_map(unsigned int max_entries,
 
end = (struct efi_mem_desc *)((ulong)map + size);
for (desc = map->desc; desc < end;
-desc = efi_get_next_mem_desc(map, desc)) {
+desc = efi_get_next_mem_desc(desc, map->desc_size)) {
if (desc->num_pages == 0)
continue;
 
diff --git a/cmd/efi.c b/cmd/efi.c
index f2ed26bd4b2..d2400acbbba 100644
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -75,16 +75,17 @@ static int h_cmp_entry(const void *v1, const void *v2)
 /**
  * efi_build_mem_table() - make a sorted copy of the memory table
  *
- * @map:   Pointer to EFI memory map table
+ * @desc_base: Pointer to EFI memory map table
  * @size:  Size of table in bytes
+ * @desc_size: Size of each @desc_base record
  * @skip_bs:   True to skip boot-time memory and merge it with conventional
  * memory. This will significantly reduce the number of table
  * entries.
  * Return: pointer to the new table. It should be freed with free() by the
  * caller.
  */
-static void *efi_build_mem_table(struct efi_entry_memmap *map, int size,
-bool skip_bs)
+static void *efi_build_mem_table(struct efi_mem_desc *desc_base, int size,
+int desc_size, bool skip_bs)
 {
struct efi_mem_desc *desc, *end, *base, *dest, *prev;
int count;
@@ -95,15 +96,16 @@ static void *efi_build_mem_table(struct efi_entry_memmap 
*map, int size,
debug("%s: Cannot allocate %#x bytes\n", __func__, size);
return NULL;
}
-   end = (struct efi_mem_desc *)((ulong)map + size);
-   count = ((ulong)end - (ulong)map->desc) / map->desc_size;
-   memcpy(base, map->desc, (ulong)end - (ulong)map->desc);
-   qsort(base, count, map->desc_size, h_cmp_entry);
+   end = (void *)desc_base + size;
+   count = ((ulong)end - (ulong)desc_base) / desc_size;
+   memcpy(base, desc_base, (ulong)end - (ulong)desc_base);
+   qsort(base, count, desc_size, h_cmp_entry);
prev = NULL;
addr = 0;
dest = base;
-   end = (struct efi_mem_desc *)((ulong)base + count * map->desc_size);
-   for (desc = base; desc < end; desc = efi_get_next_mem_desc(map, desc)) {
+   end = (struct efi_mem_desc *)((ulong)base + count * desc_size);
+   for (desc = base; desc < end;
+desc = efi_get_next_mem_desc(desc, desc_size)) {
bool merge = true;
u32 type = desc->type;
 
@@ -116,7 +118,7 @@ static void *efi_build_mem_table(struct efi_entry_memmap 
*map, int size,
if (skip_bs && is_boot_services(desc->type))
type = EFI_CONVENTIONAL_MEMORY;
 
-   memcpy(dest, desc, map->desc_size);
+   memcpy(dest, desc, desc_size);
dest->type = type;
if (!skip_bs || !prev)
merge = false;
@@ -131,7 +133,7 @@ static void *efi_build_mem_table(struct 

[PATCH v5 24/28] x86: efi: Tweak the code used for the 64-bit EFI app

[Simon Glass]

Add an empty CPU init function to avoid fiddling with low-level CPU
features in the app. Set up the C runtime correctly for 64-bit use
and avoid clearing BSS, since this is done by EFI when U-Boot is loaded.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to tweak the code used for the 64-bit EFI app

 arch/x86/cpu/x86_64/cpu.c | 5 +
 arch/x86/lib/Makefile | 5 ++---
 arch/x86/lib/relocate.c   | 2 ++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index a3674e8e29a..6a387612916 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -45,3 +45,8 @@ int cpu_phys_address_size(void)
 {
return CONFIG_CPU_ADDR_BITS;
 }
+
+int x86_cpu_init_f(void)
+{
+   return 0;
+}
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 18757b29aa9..e5235b7c4f4 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -65,9 +65,8 @@ endif
 
 lib-$(CONFIG_USE_PRIVATE_LIBGCC) += div64.o
 
-ifeq ($(CONFIG_$(SPL_)X86_64),)
-obj-$(CONFIG_EFI_APP) += crt0_ia32_efi.o reloc_ia32_efi.o
-endif
+obj-$(CONFIG_EFI_APP_32BIT) += crt0_ia32_efi.o reloc_ia32_efi.o
+obj-$(CONFIG_EFI_APP_64BIT) += crt0_x86_64_efi.o reloc_x86_64_efi.o
 
 ifneq ($(CONFIG_EFI_STUB),)
 
diff --git a/arch/x86/lib/relocate.c b/arch/x86/lib/relocate.c
index 6fe51516477..9060d19d46a 100644
--- a/arch/x86/lib/relocate.c
+++ b/arch/x86/lib/relocate.c
@@ -35,6 +35,7 @@ int copy_uboot_to_ram(void)
return 0;
 }
 
+#ifndef CONFIG_EFI_APP
 int clear_bss(void)
 {
ulong dst_addr = (ulong)&__bss_start + gd->reloc_off;
@@ -46,6 +47,7 @@ int clear_bss(void)
 
return 0;
 }
+#endif
 
 #if CONFIG_IS_ENABLED(X86_64)
 static void do_elf_reloc_fixups64(unsigned int text_base, uintptr_t size,
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 22/28] x86: efi: Show the system-table revision

[Simon Glass]

Show the revision of this table as it can be important.

Alo update the 'efi table' entry to show the actual address of the EFI
table rather than our table that points to it. This saves a step and the
intermediate table has nothing else in it.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Fix grammar in commit message

Changes in v3:
- Add new patch to show the system-table revision

 arch/x86/cpu/efi/payload.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c
index d2aa889a2b9..b7778565b19 100644
--- a/arch/x86/cpu/efi/payload.c
+++ b/arch/x86/cpu/efi/payload.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -296,8 +297,14 @@ void setup_efi_info(struct efi_info *efi_info)
 void efi_show_bdinfo(void)
 {
struct efi_entry_systable *table = NULL;
+   struct efi_system_table *sys_table;
int size, ret;
 
ret = efi_info_get(EFIET_SYS_TABLE, (void **), );
-   bdinfo_print_num_l("efi_table", (ulong)table);
+   if (!ret) {
+   bdinfo_print_num_l("efi_table", table->sys_table);
+   sys_table = (struct efi_system_table *)(uintptr_t)
+   table->sys_table;
+   bdinfo_print_num_l(" revision", sys_table->fw_revision);
+   }
 }
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 18/28] efi: Show when allocated pages are used

[Simon Glass]

Add a message here so that both paths of memory allocation are reported.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Use log_info() instead of printf()

 lib/efi/efi_app.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index e454f1a1564..36e3f1de427 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -121,13 +121,14 @@ static efi_status_t setup_memory(struct efi_priv *priv)
ret = boot->allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
   priv->image_data_type, pages, );
if (ret) {
-   printf("(using pool %lx) ", ret);
+   log_info("(using pool %lx) ", ret);
priv->ram_base = (ulong)efi_malloc(priv, CONFIG_EFI_RAM_SIZE,
   );
if (!priv->ram_base)
return ret;
priv->use_pool_for_malloc = true;
} else {
+   log_info("(using allocated RAM address %lx) ", (ulong)addr);
priv->ram_base = addr;
}
gd->ram_size = pages << 12;
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 19/28] efi: Allow easy selection of serial-only operation

[Simon Glass]

Add info about how to select vidconsole or serial.

Also set up a demo boot command.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add a better boot command too

 include/configs/efi-x86_app.h | 25 +
 1 file changed, 25 insertions(+)

diff --git a/include/configs/efi-x86_app.h b/include/configs/efi-x86_app.h
index 6061a6db0a4..33afb7ca0f9 100644
--- a/include/configs/efi-x86_app.h
+++ b/include/configs/efi-x86_app.h
@@ -10,8 +10,33 @@
 
 #undef CONFIG_TPM_TIS_BASE_ADDRESS
 
+/*
+ * Select the output device: Put an 'x' prefix before one of these to disable 
it
+ */
+
+/*
+ * Video output - can normally continue after exit_boot_services has been
+ * called, since output to the display does not require EFI services at that
+ * point. U-Boot sets up the console memory and does its own drawing.
+ */
 #define CONFIG_STD_DEVICES_SETTINGS"stdin=serial\0" \
"stdout=vidconsole\0" \
"stderr=vidconsole\0"
 
+/*
+ * Serial output with no console. Run qemu with:
+ *
+ *-display none -serial mon:stdio
+ *
+ * This will hang or fail to output on the console after exit_boot_services is
+ * called.
+ */
+#define xCONFIG_STD_DEVICES_SETTINGS   "stdin=serial\0" \
+   "stdout=serial\0" \
+   "stderr=serial\0"
+
+#undef CONFIG_BOOTCOMMAND
+
+#define CONFIG_BOOTCOMMAND "part list efi 0; fatls efi 0:1"
+
 #endif
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 17/28] efi: Mention that efi_info_get() is only used in the stub

[Simon Glass]

This provides access to EFI tables after U-Boot has exited boot services.
It is not needed in the app since boot services remain alive and we can
just call them whenever needed.

Add a comment to explain this.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Fix 'as' typo

 include/efi.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/efi.h b/include/efi.h
index ed28c204140..69321cc5cc4 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -578,6 +578,10 @@ void efi_putc(struct efi_priv *priv, const char ch);
 /**
  * efi_info_get() - get an entry from an EFI table
  *
+ * This is called from U-Boot proper to read information set up by the EFI 
stub.
+ * It can only be used when running from the EFI stuff, not when U-Boot is
+ * running as an app.
+ *
  * @type:  Entry type to search for
  * @datap: Returns pointer to entry data
  * @sizep: Returns pointer to entry size
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 16/28] efi: Check for failure when initing the app

[Simon Glass]

The stub checks for failure with efi_init(). Add this for the app as well.
It is unlikely that anything can be done, but we may as well stop.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 lib/efi/efi_app.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index 23a65c46fd4..e454f1a1564 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -325,8 +325,11 @@ efi_status_t EFIAPI efi_main(efi_handle_t image,
efi_status_t ret;
 
/* Set up access to EFI data structures */
-   efi_init(priv, "App", image, sys_table);
-
+   ret = efi_init(priv, "App", image, sys_table);
+   if (ret) {
+   printf("Failed to set up ARP: err=%lx\n", ret);
+   return ret;
+   }
efi_set_priv(priv);
 
/*
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 15/28] efi: Move exit_boot_services into a function

[Simon Glass]

At present this code is inline in the app and stub. But they do the same
thing. The difference is that the stub does it immediately and the app
doesn't want to do it until the end (when it boots a kernel) or not at
all, if returning to UEFI.

Move it into a function so it can be called as needed.

Also store the memory map so that it can be accessed within the app if
needed.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add a sentence about what the patch does

 include/efi.h  | 32 ++
 lib/efi/efi.c  | 68 ++
 lib/efi/efi_app.c  |  3 ++
 lib/efi/efi_stub.c | 66 
 4 files changed, 114 insertions(+), 55 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index ca301db7cb5..ed28c204140 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
  * @sys_table: Pointer to system table
  * @boot: Pointer to boot-services table
  * @run: Pointer to runtime-services table
+ * @memmap_key: Key returned from get_memory_map()
+ * @memmap_desc: List of memory-map records
+ * @memmap_alloc: Amount of memory allocated for memory map list
+ * @memmap_size Size of memory-map list in bytes
+ * @memmap_desc_size: Size of an individual memory-map record, in bytes
+ * @memmap_version: Memory-map version
  *
  * @use_pool_for_malloc: true if all allocation should go through the EFI 
'pool'
  * methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv {
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+   efi_uintn_t memmap_key;
+   struct efi_mem_desc *memmap_desc;
+   efi_uintn_t memmap_alloc;
+   efi_uintn_t memmap_size;
+   efi_uintn_t memmap_desc_size;
+   u32 memmap_version;
 
/* app: */
bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
  */
 int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
 
+/**
+ * efi_store_memory_map() - Collect the memory-map info from EFI
+ *
+ * Collect the memory info and store it for later use, e.g. in calling
+ * exit_boot_services()
+ *
+ * @priv:  Pointer to private EFI structure
+ * @return 0 if OK, non-zero on error
+ */
+int efi_store_memory_map(struct efi_priv *priv);
+
+/**
+ * efi_call_exit_boot_services() - Handlet eh exit-boot-service procedure
+ *
+ * Tell EFI we don't want their boot services anymore
+ *
+ * @return 0 if OK, non-zero on error
+ */
+int efi_call_exit_boot_services(void);
+
 #endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index cd6bf47b180..20da88c9151 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
 
boot->free_pool(ptr);
 }
+
+int efi_store_memory_map(struct efi_priv *priv)
+{
+   struct efi_boot_services *boot = priv->sys_table->boottime;
+   efi_uintn_t size, desc_size;
+   efi_status_t ret;
+
+   /* Get the memory map so we can switch off EFI */
+   size = 0;
+   ret = boot->get_memory_map(, NULL, >memmap_key,
+  >memmap_desc_size,
+  >memmap_version);
+   if (ret != EFI_BUFFER_TOO_SMALL) {
+   printhex2(EFI_BITS_PER_LONG);
+   putc(' ');
+   printhex2(ret);
+   puts(" No memory map\n");
+   return ret;
+   }
+   /*
+* Since doing a malloc() may change the memory map and also we want to
+* be able to read the memory map in efi_call_exit_boot_services()
+* below, after more changes have happened
+*/
+   priv->memmap_alloc = size + 1024;
+   priv->memmap_size = priv->memmap_alloc;
+   priv->memmap_desc = efi_malloc(priv, size, );
+   if (!priv->memmap_desc) {
+   printhex2(ret);
+   puts(" No memory for memory descriptor\n");
+   return ret;
+   }
+
+   ret = boot->get_memory_map(>memmap_size, priv->memmap_desc,
+  >memmap_key, _size,
+  >memmap_version);
+   if (ret) {
+   printhex2(ret);
+   puts(" Can't get memory map\n");
+   return ret;
+   }
+
+   return 0;
+}
+
+int efi_call_exit_boot_services(void)
+{
+   struct efi_priv *priv = efi_get_priv();
+   const struct efi_boot_services *boot = priv->boot;
+   efi_uintn_t size;
+   u32 version;
+   efi_status_t ret;
+
+   size = priv->memmap_alloc;
+   ret = boot->get_memory_map(, priv->memmap_desc,
+  >memmap_key,
+  >memmap_desc_size, );
+   if (ret) {
+   printhex2(ret);
+   puts(" Can't get 

[PATCH v5 10/28] efi: Drop device_path from struct efi_priv

[Simon Glass]

This is not used anywhere drop it.

Signed-off-by: Simon Glass 
---

(no changes since v3)

Changes in v3:
- Move device_path path change to its own patch

 include/efi.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/efi.h b/include/efi.h
index 908c5dc6ebd..77e599c256e 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -402,7 +402,6 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
 
 struct efi_priv {
efi_handle_t parent_image;
-   struct efi_device_path *device_path;
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 14/28] efi: Share struct efi_priv between the app and stub code

[Simon Glass]

At present each of these has its own static variable and helper functions.
Move them into a shared file.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 include/efi.h  | 21 +
 lib/efi/efi.c  | 29 +
 lib/efi/efi_app.c  | 21 ++---
 lib/efi/efi_stub.c |  7 ---
 4 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index 6622a733e6e..ca301db7cb5 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -474,6 +474,27 @@ extern char _binary_u_boot_bin_start[], 
_binary_u_boot_bin_end[];

EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
EFI_VARIABLE_APPEND_WRITE)
 
+/**
+ * efi_get_priv() - Get access to the EFI-private information
+ *
+ * This struct it used by both the stub and the app to record things about the
+ * EFI environment. It is not available in U-Boot proper after the stub has
+ * jumped there. Use efi_info_get() to obtain info in that case.
+ *
+ * @return pointer to private info
+ */
+struct efi_priv *efi_get_priv(void);
+
+/**
+ * efi_set_priv() - Set up a pointer to the EFI-private information
+ *
+ * This is called in the stub and app to record the location of this
+ * information.
+ *
+ * @priv: New location of private data
+ */
+void efi_set_priv(struct efi_priv *priv);
+
 /**
  * efi_get_sys_table() - Get access to the main EFI system table
  *
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index 69e52e45748..cd6bf47b180 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * Functions shared by the app and stub
+ *
  * Copyright (c) 2015 Google, Inc
  *
  * EFI information obtained here:
@@ -17,6 +19,33 @@
 #include 
 #include 
 
+static struct efi_priv *global_priv;
+
+struct efi_priv *efi_get_priv(void)
+{
+   return global_priv;
+}
+
+void efi_set_priv(struct efi_priv *priv)
+{
+   global_priv = priv;
+}
+
+struct efi_system_table *efi_get_sys_table(void)
+{
+   return global_priv->sys_table;
+}
+
+struct efi_boot_services *efi_get_boot(void)
+{
+   return global_priv->boot;
+}
+
+unsigned long efi_get_ram_base(void)
+{
+   return global_priv->ram_base;
+}
+
 /*
  * Global declaration of gd.
  *
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index e38d46b15db..a0ae2a531ee 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -27,23 +27,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static struct efi_priv *global_priv;
-
-struct efi_system_table *efi_get_sys_table(void)
-{
-   return global_priv->sys_table;
-}
-
-struct efi_boot_services *efi_get_boot(void)
-{
-   return global_priv->boot;
-}
-
-unsigned long efi_get_ram_base(void)
-{
-   return global_priv->ram_base;
-}
-
 int efi_info_get(enum efi_entry_t type, void **datap, int *sizep)
 {
return -ENOSYS;
@@ -344,7 +327,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image,
/* Set up access to EFI data structures */
efi_init(priv, "App", image, sys_table);
 
-   global_priv = priv;
+   efi_set_priv(priv);
 
/*
 * Set up the EFI debug UART so that printf() works. This is
@@ -370,7 +353,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image,
 
 static void efi_exit(void)
 {
-   struct efi_priv *priv = global_priv;
+   struct efi_priv *priv = efi_get_priv();
 
free_memory(priv);
printf("U-Boot EFI exiting\n");
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
index 156cbf0b928..bc4c3a48720 100644
--- a/lib/efi/efi_stub.c
+++ b/lib/efi/efi_stub.c
@@ -31,7 +31,6 @@
 #error "This file needs to be ported for use on architectures"
 #endif
 
-static struct efi_priv *global_priv;
 static bool use_uart;
 
 struct __packed desctab_info {
@@ -63,6 +62,8 @@ void _debug_uart_init(void)
 
 void putc(const char ch)
 {
+   struct efi_priv *priv = efi_get_priv();
+
if (ch == '\n')
putc('\r');
 
@@ -73,7 +74,7 @@ void putc(const char ch)
;
outb(ch, (ulong)_port->thr);
} else {
-   efi_putc(global_priv, ch);
+   efi_putc(priv, ch);
}
 }
 
@@ -313,7 +314,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image,
puts(" efi_init() failed\n");
return ret;
}
-   global_priv = priv;
+   efi_set_priv(priv);
 
cs32 = get_codeseg32();
if (cs32 < 0)
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 11/28] efi: Add comments to struct efi_priv

[Simon Glass]

This structure is uncommented. Fix it.

Signed-off-by: Simon Glass 
---

(no changes since v3)

Changes in v3:
- Drop comments that confuse sphinx
- Move device_path path change to its own patch

 include/efi.h | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/include/efi.h b/include/efi.h
index 77e599c256e..6622a733e6e 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -400,14 +400,37 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
return (struct efi_mem_desc *)((ulong)desc + map->desc_size);
 }
 
+/**
+ * struct efi_priv - Information about the environment provided by EFI
+ *
+ * @parent_image: image passed into the EFI app or stub
+ * @sys_table: Pointer to system table
+ * @boot: Pointer to boot-services table
+ * @run: Pointer to runtime-services table
+ *
+ * @use_pool_for_malloc: true if all allocation should go through the EFI 
'pool'
+ * methods allocate_pool() and free_pool(); false to use 'pages' methods
+ * allocate_pages() and free_pages()
+ * @ram_base: Base address of RAM (size CONFIG_EFI_RAM_SIZE)
+ * @image_data_type: Type of the loaded image (e.g. EFI_LOADER_CODE)
+ *
+ * @info: Header of the info list, holding info collected by the stub and 
passed
+ * to U-Boot
+ * @info_size: Size of the info list, in bytes from @info
+ * @next_hdr: Pointer to where to put the next header when adding to the list
+ */
 struct efi_priv {
efi_handle_t parent_image;
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+
+   /* app: */
bool use_pool_for_malloc;
unsigned long ram_base;
unsigned int image_data_type;
+
+   /* stub: */
struct efi_info_hdr *info;
unsigned int info_size;
void *next_hdr;
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 13/28] efi: Add a few comments to the stub

[Simon Glass]

Comment some functions that need more information.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 lib/efi/efi_stub.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
index b3393e47fae..156cbf0b928 100644
--- a/lib/efi/efi_stub.c
+++ b/lib/efi/efi_stub.c
@@ -225,6 +225,22 @@ static int get_codeseg32(void)
return cs32;
 }
 
+/**
+ * setup_info_table() - sets up a table containing information from EFI
+ *
+ * We must call exit_boot_services() before jumping out of the stub into U-Boot
+ * proper, so that U-Boot has full control of peripherals, memory, etc.
+ *
+ * Once we do this, we cannot call any boot-services functions so we must find
+ * out everything we need to before doing that.
+ *
+ * Set up a struct efi_info_hdr table which can hold various records (e.g.
+ * struct efi_entry_memmap) with information obtained from EFI.
+ *
+ * @priv: Pointer to our private information which contains the list
+ * @size: Size of the table to allocate
+ * @return 0 if OK, non-zero on error
+ */
 static int setup_info_table(struct efi_priv *priv, int size)
 {
struct efi_info_hdr *info;
@@ -248,6 +264,12 @@ static int setup_info_table(struct efi_priv *priv, int 
size)
return 0;
 }
 
+/**
+ * add_entry_addr() - Add a new entry to the efi_info list
+ *
+ * @priv: Pointer to our private information which contains the list
+ *
+ */
 static void add_entry_addr(struct efi_priv *priv, enum efi_entry_t type,
   void *ptr1, int size1, void *ptr2, int size2)
 {
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 12/28] efi: Fix ll_boot_init() operation with the app

[Simon Glass]

This should return false when the EFI app is running, since UEFI has done
the required low-level init. Fix it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 include/init.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/init.h b/include/init.h
index f2cd46dead0..3ce7b8b95bd 100644
--- a/include/init.h
+++ b/include/init.h
@@ -15,7 +15,7 @@
 #include 
 
 /* Avoid using CONFIG_EFI_STUB directly as we may boot from other loaders */
-#ifdef CONFIG_EFI_STUB
+#ifdef CONFIG_EFI
 #define ll_boot_init() false
 #else
 #include 
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 09/28] x86: efi: Add room for the binman definition in the dtb

[Simon Glass]

At present only 4KB of spare space is left in the DTB when building the
EFI app. Increase this to 32KB so there is plenty of space to insert the
binman definition. This cannot be expanded later (as with OF_SEPARATE)
because the ELF image has already been built.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/x86/dts/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/dts/Makefile b/arch/x86/dts/Makefile
index be209aaaf8f..5c8c05ec499 100644
--- a/arch/x86/dts/Makefile
+++ b/arch/x86/dts/Makefile
@@ -24,7 +24,7 @@ dtb-y += bayleybay.dtb \
 
 targets += $(dtb-y)
 
-DTC_FLAGS += -R 4 -p 0x1000
+DTC_FLAGS += -R 4 -p $(if $(CONFIG_EFI_APP),0x8000,0x1000)
 
 PHONY += dtbs
 dtbs: $(addprefix $(obj)/, $(dtb-y))
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 08/28] x86: Don't process the kernel command line unless enabled

[Simon Glass]

If the 'bootm' command is not enabled then this code is not available and
this causes a link error. Fix it.

Note that for the EFI app, there is no indication of missing code. It just
hangs!

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/x86/lib/zimage.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
index 7ce02226ef9..9cc04490307 100644
--- a/arch/x86/lib/zimage.c
+++ b/arch/x86/lib/zimage.c
@@ -365,11 +365,14 @@ int setup_zimage(struct boot_params *setup_base, char 
*cmd_line, int auto_boot,
strcpy(cmd_line, (char *)cmdline_force);
else
build_command_line(cmd_line, auto_boot);
-   ret = bootm_process_cmdline(cmd_line, max_size, BOOTM_CL_ALL);
-   if (ret) {
-   printf("Cmdline setup failed (max_size=%x, 
bootproto=%x, err=%d)\n",
-  max_size, bootproto, ret);
-   return ret;
+   if (IS_ENABLED(CONFIG_CMD_BOOTM)) {
+   ret = bootm_process_cmdline(cmd_line, max_size,
+   BOOTM_CL_ALL);
+   if (ret) {
+   printf("Cmdline setup failed (max_size=%x, 
bootproto=%x, err=%d)\n",
+  max_size, bootproto, ret);
+   return ret;
+   }
}
printf("Kernel command line: \"");
puts(cmd_line);
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 04/28] efi: Locate all block devices in the app

[Simon Glass]

When starting the app, locate all block devices and make them available
to U-Boot. This allows listing partitions and accessing files in
filesystems.

EFI also has the concept of 'disks', meaning boot media. For now, this
is not obviously useful in U-Boot, but add code to at least locate these.
This can be expanded later as needed.

Series-changes; 2
- Store device path in struct efi_media_plat
- Don't export efi_bind_block()
- Only bind devices for media devices, not for partitions
- Show devices that are processed
- Update documentation

Signed-off-by: Simon Glass 
---

(no changes since v1)

 doc/develop/uefi/u-boot_on_efi.rst |   4 +-
 include/efi.h  |   6 +-
 include/efi_api.h  |  15 ++
 lib/efi/efi_app.c  | 223 +
 4 files changed, 243 insertions(+), 5 deletions(-)

diff --git a/doc/develop/uefi/u-boot_on_efi.rst 
b/doc/develop/uefi/u-boot_on_efi.rst
index 5f2f850f071..8f81b799072 100644
--- a/doc/develop/uefi/u-boot_on_efi.rst
+++ b/doc/develop/uefi/u-boot_on_efi.rst
@@ -265,9 +265,7 @@ This work could be extended in a number of ways:
 
 - Figure out how to solve the interrupt problem
 
-- Add more drivers to the application side (e.g. block devices, USB,
-  environment access). This would mostly be an academic exercise as a strong
-  use case is not readily apparent, but it might be fun.
+- Add more drivers to the application side (e.g.USB, environment access).
 
 - Avoid turning off boot services in the stub. Instead allow U-Boot to make
   use of boot services in case it wants to. It is unclear what it might want
diff --git a/include/efi.h b/include/efi.h
index 0ec5913ddd1..908c5dc6ebd 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -419,10 +419,12 @@ struct efi_priv {
  *
  * @handle: handle of the controller on which this driver is installed
  * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI path to the device
  */
 struct efi_media_plat {
-   efi_handle_thandle;
-   struct efi_block_io *blkio;
+   efi_handle_t handle;
+   struct efi_block_io *blkio;
+   struct efi_device_path *device_path;
 };
 
 /* Base address of the EFI image */
diff --git a/include/efi_api.h b/include/efi_api.h
index 80109f012bc..ec9fa89a935 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol {
const u16 *package_version_name);
 };
 
+#define EFI_DISK_IO_PROTOCOL_GUID  \
+   EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \
+0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+
+struct efi_disk {
+   u64 revision;
+   efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id,
+u64 offset, efi_uintn_t buffer_size,
+void *buffer);
+
+   efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id,
+ u64 offset, efi_uintn_t buffer_size,
+ void *buffer);
+};
+
 #endif
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
index f61665686c5..e38d46b15db 100644
--- a/lib/efi/efi_app.c
+++ b/lib/efi/efi_app.c
@@ -21,6 +21,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap, int 
*sizep)
return -ENOSYS;
 }
 
+/**
+ * efi_print_str() - Print a UFT-16 string to the U-Boot console
+ *
+ * @str: String to print
+ */
+static void efi_print_str(const u16 *str)
+{
+   while (*str) {
+   int ch = *str++;
+
+   if (ch > ' ' && ch < 127)
+   putc(ch);
+   }
+}
+
+/**
+ * efi_bind_block() - bind a new block device to an EFI device
+ *
+ * Binds a new top-level EFI_MEDIA device as well as a child block device so
+ * that the block device can be accessed in U-Boot.
+ *
+ * The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1',
+ * for example, just like any other interface type.
+ *
+ * @handle: handle of the controller on which this driver is installed
+ * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI device path structure for this
+ * @len: Length of @device_path in bytes
+ * @devp: Returns the bound device
+ * @return 0 if OK, -ve on error
+ */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
+  struct efi_device_path *device_path, int len,
+  struct udevice **devp)
+{
+   struct efi_media_plat plat;
+   struct udevice *dev;
+   char name[18];
+   int ret;
+
+   plat.handle = handle;
+   plat.blkio = blkio;
+   plat.device_path = malloc(device_path->length);
+   if (!plat.device_path)
+   return log_msg_ret("path", -ENOMEM);
+   memcpy(plat.device_path, device_path, device_path->length);
+   ret 

[PATCH v5 06/28] bloblist: Support allocating the bloblist

[Simon Glass]

Typically the bloblist is positioned at a fixed address in memory until
relocation. This is convenient when it is set up in SPL or before
relocation.

But for EFI we want to set it up only when U-Boot proper is running. Add
a way to allocate it using malloc() and update the documentation to cover
this aspect of bloblist.

Note there are no tests of this feature at present, nor any direct testing
of bloblist_init().

This can be added, e.g. by making this option controllable at runtime.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 common/Kconfig   | 15 +--
 common/bloblist.c| 16 ++--
 common/board_f.c |  8 +++-
 doc/develop/bloblist.rst | 16 
 4 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 176fda9449e..50ac4331f5c 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -727,6 +727,8 @@ config TPL_BLOBLIST
  This enables a bloblist in TPL. The bloblist is set up in TPL and
  passed to SPL and U-Boot proper.
 
+if BLOBLIST
+
 config BLOBLIST_SIZE
hex "Size of bloblist"
depends on BLOBLIST
@@ -737,17 +739,24 @@ config BLOBLIST_SIZE
  is set up in the first part of U-Boot to run (TPL, SPL or U-Boot
  proper), and this sane bloblist is used for subsequent stages.
 
+config BLOBLIST_ALLOC
+   bool "Allocate bloblist"
+   help
+ Allocate the bloblist using malloc(). This avoids the need to
+ specify a fixed address on systems where this is unknown or can
+ change at runtime.
+
 config BLOBLIST_ADDR
hex "Address of bloblist"
-   depends on BLOBLIST
default 0xc000 if SANDBOX
help
  Sets the address of the bloblist, set up by the first part of U-Boot
  which runs. Subsequent U-Boot stages typically use the same address.
 
+ This is not used if BLOBLIST_ALLOC is selected.
+
 config BLOBLIST_SIZE_RELOC
hex "Size of bloblist after relocation"
-   depends on BLOBLIST
default BLOBLIST_SIZE
help
  Sets the size of the bloblist in bytes after relocation. Since U-Boot
@@ -755,6 +764,8 @@ config BLOBLIST_SIZE_RELOC
  size than the one set up by SPL. This bloblist is set up during the
  relocation process.
 
+endif # BLOBLIST
+
 endmenu
 
 source "common/spl/Kconfig"
diff --git a/common/bloblist.c b/common/bloblist.c
index 1290fff8504..01b04103d91 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -416,10 +417,21 @@ int bloblist_init(void)
ret = bloblist_check(CONFIG_BLOBLIST_ADDR,
 CONFIG_BLOBLIST_SIZE);
if (ret) {
+   ulong addr;
+
log(LOGC_BLOBLIST, expected ? LOGL_WARNING : LOGL_DEBUG,
"Existing bloblist not found: creating new bloblist\n");
-   ret = bloblist_new(CONFIG_BLOBLIST_ADDR, CONFIG_BLOBLIST_SIZE,
-  0);
+   if (IS_ENABLED(CONFIG_BLOBLIST_ALLOC)) {
+   void *ptr = memalign(BLOBLIST_ALIGN,
+CONFIG_BLOBLIST_SIZE);
+
+   if (!ptr)
+   return log_msg_ret("alloc", -ENOMEM);
+   addr = map_to_sysmem(ptr);
+   } else {
+   addr = CONFIG_BLOBLIST_ADDR;
+   }
+   ret = bloblist_new(addr, CONFIG_BLOBLIST_SIZE, 0);
} else {
log(LOGC_BLOBLIST, LOGL_DEBUG, "Found existing bloblist\n");
}
diff --git a/common/board_f.c b/common/board_f.c
index f7ea7c7a1e4..dd69c3b6b77 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -655,8 +655,14 @@ static int reloc_bootstage(void)
 static int reloc_bloblist(void)
 {
 #ifdef CONFIG_BLOBLIST
-   if (gd->flags & GD_FLG_SKIP_RELOC)
+   /*
+* Relocate only if we are supposed to send it
+*/
+   if ((gd->flags & GD_FLG_SKIP_RELOC) &&
+   CONFIG_BLOBLIST_SIZE == CONFIG_BLOBLIST_SIZE_RELOC) {
+   debug("Not relocating bloblist\n");
return 0;
+   }
if (gd->new_bloblist) {
int size = CONFIG_BLOBLIST_SIZE;
 
diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst
index 317ebc4919d..47274cf8e26 100644
--- a/doc/develop/bloblist.rst
+++ b/doc/develop/bloblist.rst
@@ -59,6 +59,22 @@ Bloblist provides a fairly simple API which allows blobs to 
be created and
 found. All access is via the blob's tag. Blob records are zeroed when added.
 
 
+Placing the bloblist
+
+
+The bloblist is typically positioned at a fixed address by TPL, or SPL. This
+is controlled by `CONFIG_BLOBLIST_ADDR`. But in some cases it is preferable to
+allocate the bloblist in the malloc() space. Use the `CONFIG_BLOBLIST_ALLOC`

[PATCH v5 03/28] efi: Add a media/block driver for EFI block devices

[Simon Glass]

Add a block driver which handles read/write for EFI block devices. This
driver actually already exists ('efi_block') but is not really suitable
for use as a real U-Boot driver:

- The operations do not provide a udevice
- The code is designed for running as part of EFI loader, so uses
EFI_PRINT() and EFI_CALL().
- The bind method probes the device, which is not permitted
- It uses 'EFI' as its parent device

The new driver is more 'normal', just requiring its platform data be set
up in advance.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Drop mention of partitions from the commit message

 drivers/block/Kconfig   |  10 
 drivers/block/Makefile  |   1 +
 drivers/block/efi_blk.c | 115 
 include/efi.h   |  11 
 4 files changed, 137 insertions(+)
 create mode 100644 drivers/block/efi_blk.c

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 755fdccb574..8235430497d 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -82,6 +82,16 @@ config EFI_MEDIA_SANDBOX
  EFI_MEDIA uclass. It does not do anything useful, since sandbox does
  not actually support running on top of UEFI.
 
+config EFI_MEDIA_BLK
+   bool "EFI media block driver"
+   depends on EFI_APP
+   default y
+   help
+ Enables a block driver for providing access to UEFI devices. This
+ allows use of block devices detected by the underlying UEFI
+ implementation. With this it is possible to use filesystems on these
+ devices, for example.
+
 endif  # EFI_MEDIA
 
 config IDE
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index 3778633da1d..b221a7c6eea 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
 
 obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
 obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
+obj-$(CONFIG_EFI_MEDIA_BLK) += efi_blk.o
diff --git a/drivers/block/efi_blk.c b/drivers/block/efi_blk.c
new file mode 100644
index 000..9d25ecbf37f
--- /dev/null
+++ b/drivers/block/efi_blk.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Block driver for EFI devices
+ * This supports a media driver of UCLASS_EFI with a child UCLASS_BLK
+ * It allows block-level access to EFI devices made available via EFI boot
+ * services
+ *
+ * Copyright 2021 Google LLC
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct efi_block_plat {
+   struct efi_block_io *blkio;
+};
+
+/**
+ * Read from block device
+ *
+ * @dev:   device
+ * @blknr: first block to be read
+ * @blkcnt:number of blocks to read
+ * @buffer:output buffer
+ * Return: number of blocks transferred
+ */
+static ulong efi_bl_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+void *buffer)
+{
+   struct efi_block_plat *plat = dev_get_plat(dev);
+   struct efi_block_io *io = plat->blkio;
+   efi_status_t ret;
+
+   log_debug("read buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
+ (ulong)blkcnt);
+   ret = io->read_blocks(io, io->media->media_id, blknr,
+ blkcnt * io->media->block_size, buffer);
+   log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK,
+ ret & ~EFI_ERROR_MASK);
+   if (ret)
+   return 0;
+
+   return blkcnt;
+}
+
+/**
+ * Write to block device
+ *
+ * @dev:   device
+ * @blknr: first block to be write
+ * @blkcnt:number of blocks to write
+ * @buffer:input buffer
+ * Return: number of blocks transferred
+ */
+static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+ const void *buffer)
+{
+   struct efi_block_plat *plat = dev_get_plat(dev);
+   struct efi_block_io *io = plat->blkio;
+   efi_status_t ret;
+
+   log_debug("write buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
+ (ulong)blkcnt);
+   ret = io->write_blocks(io, io->media->media_id, blknr,
+  blkcnt * io->media->block_size, (void *)buffer);
+   log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK,
+ ret & ~EFI_ERROR_MASK);
+   if (ret)
+   return 0;
+
+   return blkcnt;
+}
+
+/* Block device driver operators */
+static const struct blk_ops efi_blk_ops = {
+   .read   = efi_bl_read,
+   .write  = efi_bl_write,
+};
+
+U_BOOT_DRIVER(efi_block) = {
+   .name   = "efi_block",
+   .id = UCLASS_BLK,
+   .ops= _blk_ops,
+   .plat_auto  = sizeof(struct efi_block_plat),
+};
+
+static int efi_media_bind(struct udevice *dev)
+{
+   struct efi_media_plat *plat = dev_get_plat(dev);
+   struct efi_block_plat *blk_plat;
+   struct udevice *blk;
+   int ret;
+
+   ret = blk_create_devicef(dev, "efi_block", 

[PATCH v5 05/28] efi: serial: Support arrow keys

[Simon Glass]

At present only the backspace key is supported in U-Boot, when running as
an EFI app. Add support for arrows, home and end as well, to make the CLI
more friendly.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/serial/serial_efi.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c
index 33ddbd6080c..0067576389d 100644
--- a/drivers/serial/serial_efi.c
+++ b/drivers/serial/serial_efi.c
@@ -24,6 +24,9 @@ struct serial_efi_priv {
bool have_key;
 };
 
+/* Convert a lower-case character to its ctrl-char equivalent */
+#define CTL_CH(c)  ((c) - 'a' + 1)
+
 int serial_efi_setbrg(struct udevice *dev, int baudrate)
 {
return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv)
 static int serial_efi_getc(struct udevice *dev)
 {
struct serial_efi_priv *priv = dev_get_priv(dev);
+   char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8};
int ret, ch;
 
ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev)
 * key scan code of 8. Handle this so that backspace works correctly
 * in the U-Boot command line.
 */
-   if (!ch && priv->key.scan_code == 8)
-   ch = 8;
+   if (!ch && priv->key.scan_code < sizeof(conv_scan)) {
+   ch = conv_scan[priv->key.scan_code];
+   if (ch >= 'a')
+   ch -= 'a' - 1;
+   }
debug(" [%x %x %x] ", ch, priv->key.unicode_char, priv->key.scan_code);
 
return ch;
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 07/28] x86: Allow booting a kernel from the EFI app

[Simon Glass]

At present this is disabled, but it should work so long as the kernel does
not need EFI services. Enable it and add a note about remaining work.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Update documentation

 arch/x86/lib/bootm.c   | 11 +++
 doc/develop/uefi/u-boot_on_efi.rst |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 667e5e689e3..57cba5c65d3 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -179,10 +179,14 @@ int boot_linux_kernel(ulong setup_base, ulong 
load_address, bool image_64bit)
* U-Boot is setting them up that way for itself in
* arch/i386/cpu/cpu.c.
*
-   * Note that we cannot currently boot a kernel while running as
-   * an EFI application. Please use the payload option for that.
+   * Note: this is incomplete for EFI kernels!
+   *
+   * This can boot a kernel while running as an EFI application,
+   * but if the kernel requires EFI support then that support needs
+   * to be enabled first (see EFI_LOADER). Also the EFI information
+   * must enabled with setup_efi_info(). See setup_zimage() for
+   * how this is done with the stub.
*/
-#ifndef CONFIG_EFI_APP
__asm__ __volatile__ (
"movl $0, %%ebp\n"
"cli\n"
@@ -191,7 +195,6 @@ int boot_linux_kernel(ulong setup_base, ulong load_address, 
bool image_64bit)
[boot_params] "S"(setup_base),
"b"(0), "D"(0)
);
-#endif
}
 
/* We can't get to here */
diff --git a/doc/develop/uefi/u-boot_on_efi.rst 
b/doc/develop/uefi/u-boot_on_efi.rst
index 8f81b799072..acad6397e81 100644
--- a/doc/develop/uefi/u-boot_on_efi.rst
+++ b/doc/develop/uefi/u-boot_on_efi.rst
@@ -269,7 +269,7 @@ This work could be extended in a number of ways:
 
 - Avoid turning off boot services in the stub. Instead allow U-Boot to make
   use of boot services in case it wants to. It is unclear what it might want
-  though.
+  though. It is better to use the app.
 
 Where is the code?
 --
-- 
2.34.1.400.ga245620fadb-goog

[PATCH v5 02/28] efi: Add EFI uclass for media

[Simon Glass]

At present UCLASS_EFI is used to represent an EFI filesystem among other
things. The description of this uclass is "EFI managed devices" which is
pretty vague. The only driver that uses this uclass is in fact not a real
U-Boot driver, since its operations do not include a struct udevice.

Rather than mess with this, create a new UCLASS_EFI_MEDIA uclass to handle
EFI media such as disks. Make this the uclass to use for EFI media so that
it can be used with 'part list', for example.

The existing implementation using UCLASS_EFI remains as is, for
discussion.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Add MAINTAINERS entry
- Show the correct interface type with 'part list'
- Update the commit message to explain things better

 MAINTAINERS  |  3 +++
 arch/sandbox/dts/test.dts|  4 
 disk/part.c  |  5 -
 drivers/block/Kconfig| 23 +++
 drivers/block/Makefile   |  3 +++
 drivers/block/blk-uclass.c   |  2 ++
 drivers/block/efi-media-uclass.c | 15 +++
 drivers/block/sb_efi_media.c | 20 
 include/blk.h|  1 +
 include/dm/uclass-id.h   |  1 +
 test/dm/Makefile |  1 +
 test/dm/efi_media.c  | 24 
 12 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 drivers/block/efi-media-uclass.c
 create mode 100644 drivers/block/sb_efi_media.c
 create mode 100644 test/dm/efi_media.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9045e509d73..5b162ad2978 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -714,8 +714,11 @@ W: 
https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
 F: board/efi/efi-x86_app
 F: configs/efi-x86_app*
 F: doc/develop/uefi/u-boot_on_efi.rst
+F: drivers/block/efi-media-uclass.c
+F: drivers/block/sb_efi_media.c
 F: lib/efi/efi_app.c
 F: scripts/build-efi.sh
+F: test/dm/efi_media.c
 
 EFI PAYLOAD
 M: Heinrich Schuchardt 
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index e5261bb9fa2..48ca3e1e472 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -499,6 +499,10 @@
compatible = "sandbox,clk-ccf";
};
 
+   efi-media {
+   compatible = "sandbox,efi-media";
+   };
+
eth@10002000 {
compatible = "sandbox,eth";
reg = <0x10002000 0x1000>;
diff --git a/disk/part.c b/disk/part.c
index fe1ebd4adf7..e857a9f9585 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -296,8 +296,11 @@ static void print_part_header(const char *type, struct 
blk_desc *dev_desc)
case IF_TYPE_VIRTIO:
puts("VirtIO");
break;
+   case IF_TYPE_EFI_MEDIA:
+   puts("EFI");
+   break;
default:
-   puts ("UNKNOWN");
+   puts("UNKNOWN");
break;
}
printf (" device %d  --   Partition Type: %s\n\n",
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 56a4eec05ac..755fdccb574 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -61,6 +61,29 @@ config TPL_BLOCK_CACHE
help
  This option enables the disk-block cache in TPL
 
+config EFI_MEDIA
+   bool "Support EFI media drivers"
+   default y if EFI || SANDBOX
+   help
+ Enable this to support media devices on top of UEFI. This enables
+ just the uclass so you also need a specific driver to make this do
+ anything.
+
+ For sandbox there is a test driver.
+
+if EFI_MEDIA
+
+config EFI_MEDIA_SANDBOX
+   bool "Sandbox EFI media driver"
+   depends on SANDBOX
+   default y
+   help
+ Enables a simple sandbox media driver, used for testing just the
+ EFI_MEDIA uclass. It does not do anything useful, since sandbox does
+ not actually support running on top of UEFI.
+
+endif  # EFI_MEDIA
+
 config IDE
bool "Support IDE controllers"
select HAVE_BLOCK_DEVICE
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index 94ab5c6f906..3778633da1d 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -14,3 +14,6 @@ obj-$(CONFIG_IDE) += ide.o
 endif
 obj-$(CONFIG_SANDBOX) += sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
+
+obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
+obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index a7470ae28d5..4ae8af6d609 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -28,6 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = {
[IF_TYPE_SATA]  = "sata",
[IF_TYPE_HOST]  = "host",
[IF_TYPE_NVME]  = "nvme",
+   [IF_TYPE_EFI_MEDIA] = "efi",
[IF_TYPE_EFI_LOADER]= "efiloader",
[IF_TYPE_VIRTIO]   

[PATCH v5 01/28] efi: Rename UCLASS_EFI and IF_TYPE_EFI

[Simon Glass]

These names are better used for access to devices provided by an EFI
layer. Use EFI_LOADER instead here, since these are only available in
U-Boot's EFI_LOADER layer.

Signed-off-by: Simon Glass 
---

Changes in v5:
- Add new patch to resolve EFI/EFI_LOADER conflict

 doc/develop/uefi/uefi.rst | 8 
 drivers/block/blk-uclass.c| 4 ++--
 include/blk.h | 2 +-
 include/dm/uclass-id.h| 2 +-
 lib/efi_driver/efi_block_device.c | 8 
 lib/efi_driver/efi_uclass.c   | 8 
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index f17138f5c76..a3e2656ab81 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -620,12 +620,12 @@ EFI_DRIVER_BINDING_PROTOCOL implementation for the UEFI 
drivers.
 
 A linker created list is used to keep track of the UEFI drivers. To create an
 entry in the list the UEFI driver uses the U_BOOT_DRIVER macro specifying
-UCLASS_EFI as the ID of its uclass, e.g::
+UCLASS_EFI_LOADER as the ID of its uclass, e.g::
 
 /* Identify as UEFI driver */
 U_BOOT_DRIVER(efi_block) = {
 .name  = "EFI block driver",
-.id= UCLASS_EFI,
+.id= UCLASS_EFI_LOADER,
 .ops   = _ops,
 };
 
@@ -651,8 +651,8 @@ UEFI block IO driver
 The UEFI block IO driver supports devices exposing the EFI_BLOCK_IO_PROTOCOL.
 
 When connected it creates a new U-Boot block IO device with interface type
-IF_TYPE_EFI, adds child controllers mapping the partitions, and installs the
-EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with the
+IF_TYPE_EFI_LOADER, adds child controllers mapping the partitions, and installs
+the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with 
the
 software iPXE to boot from iSCSI network drives [4].
 
 This driver is only available if U-Boot is configured with::
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 83682dcc181..a7470ae28d5 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -28,7 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = {
[IF_TYPE_SATA]  = "sata",
[IF_TYPE_HOST]  = "host",
[IF_TYPE_NVME]  = "nvme",
-   [IF_TYPE_EFI]   = "efi",
+   [IF_TYPE_EFI_LOADER]= "efiloader",
[IF_TYPE_VIRTIO]= "virtio",
[IF_TYPE_PVBLOCK]   = "pvblock",
 };
@@ -44,7 +44,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
[IF_TYPE_SATA]  = UCLASS_AHCI,
[IF_TYPE_HOST]  = UCLASS_ROOT,
[IF_TYPE_NVME]  = UCLASS_NVME,
-   [IF_TYPE_EFI]   = UCLASS_EFI,
+   [IF_TYPE_EFI_LOADER]= UCLASS_EFI_LOADER,
[IF_TYPE_VIRTIO]= UCLASS_VIRTIO,
[IF_TYPE_PVBLOCK]   = UCLASS_PVBLOCK,
 };
diff --git a/include/blk.h b/include/blk.h
index f0cc7ca1a28..f0835c3fed5 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -34,7 +34,7 @@ enum if_type {
IF_TYPE_SATA,
IF_TYPE_HOST,
IF_TYPE_NVME,
-   IF_TYPE_EFI,
+   IF_TYPE_EFI_LOADER,
IF_TYPE_PVBLOCK,
IF_TYPE_VIRTIO,
 
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index fd139b9b2a0..5e503960aa7 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -48,7 +48,7 @@ enum uclass_id {
UCLASS_DMA, /* Direct Memory Access */
UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */
UCLASS_ECDSA,   /* Elliptic curve cryptographic device */
-   UCLASS_EFI, /* EFI managed devices */
+   UCLASS_EFI_LOADER,  /* Devices managed by EFI_LOADER */
UCLASS_ETH, /* Ethernet device */
UCLASS_ETH_PHY, /* Ethernet PHY device */
UCLASS_FIRMWARE,/* Firmware */
diff --git a/lib/efi_driver/efi_block_device.c 
b/lib/efi_driver/efi_block_device.c
index 0937e3595a4..04cb3ef0d4e 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -147,7 +147,7 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
if (!obj)
return -ENOENT;
 
-   devnum = blk_find_max_devnum(IF_TYPE_EFI);
+   devnum = blk_find_max_devnum(IF_TYPE_EFI_LOADER);
if (devnum == -ENODEV)
devnum = 0;
else if (devnum < 0)
@@ -159,8 +159,8 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
sprintf(name, "efiblk#%d", devnum);
 
/* Create driver model udevice for the EFI block io device */
-   ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI, devnum,
-   io->media->block_size,
+   ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI_LOADER,
+   devnum, io->media->block_size,
(lbaint_t)io->media->last_block, );
  

[PATCH v5 00/28] efi: Improvements to U-Boot running on top of UEFI

[Simon Glass]

At present U-Boot can be built as an EFI app, but it is really just for
testing, with very few features. Instead, the payload build is used for
booting on top of UEFI, where U-Boot takes over the machine immediately
and supplies its own drivers.

But the app could be made more useful.

This series provides access to EFI block devices and the video console
within U-Boot. It also restructures the app to make it possible to boot
a kernel, although further work is needed to implement this.

This can be thought of as making U-Boot perform a little like 'grub', in
that it runs purely based on UEFI-provided services.

Of course a lot more work is required, but this is a start.

Note: It would be very useful to include qemu tests of the app and stub
in CI.

This is available at u-boot-dm/efi-working

Changes in v5:
- Add new patch to avoid setting up global_data again with EFI
- Add new patch to avoid using the 64-bit link script for the EFI app
- Add new patch to build the 64-bit app properly
- Add new patch to resolve EFI/EFI_LOADER conflict
- Add new patch to round out the link script for 64-bit EFI
- Add new patch to set the correct link flags for the 64-bit EFI app
- Add new patch to tweak the code used for the 64-bit EFI app
- Add various patches to fix up the 64-bit app so that it boots
- Fix grammar in commit message

Changes in v3:
- Add new patch to show the system-table revision
- Drop comments that confuse sphinx
- Move device_path path change to its own patch

Changes in v2:
- Add MAINTAINERS entry
- Add a better boot command too
- Add a sentence about what the patch does
- Add new patch to support the efi command in the app
- Drop mention of partitions from the commit message
- Fix 'as' typo
- Show the correct interface type with 'part list'
- Update documentation
- Update the commit message to explain things better
- Use log_info() instead of printf()

Simon Glass (28):
  efi: Rename UCLASS_EFI and IF_TYPE_EFI
  efi: Add EFI uclass for media
  efi: Add a media/block driver for EFI block devices
  efi: Locate all block devices in the app
  efi: serial: Support arrow keys
  bloblist: Support allocating the bloblist
  x86: Allow booting a kernel from the EFI app
  x86: Don't process the kernel command line unless enabled
  x86: efi: Add room for the binman definition in the dtb
  efi: Drop device_path from struct efi_priv
  efi: Add comments to struct efi_priv
  efi: Fix ll_boot_init() operation with the app
  efi: Add a few comments to the stub
  efi: Share struct efi_priv between the app and stub code
  efi: Move exit_boot_services into a function
  efi: Check for failure when initing the app
  efi: Mention that efi_info_get() is only used in the stub
  efi: Show when allocated pages are used
  efi: Allow easy selection of serial-only operation
  x86: efi: Update efi_get_next_mem_desc() to avoid needing a map
  efi: Support the efi command in the app
  x86: efi: Show the system-table revision
  x86: efi: Don't set up global_data again with EFI
  x86: efi: Tweak the code used for the 64-bit EFI app
  x86: efi: Round out the link script for 64-bit EFI
  x86: efi: Don't use the 64-bit link script for the EFI app
  x86: efi: Set the correct link flags for the 64-bit EFI app
  efi: Build the 64-bit app properly

 MAINTAINERS|   3 +
 Makefile   |   4 +-
 arch/sandbox/dts/test.dts  |   4 +
 arch/x86/config.mk |  15 +-
 arch/x86/cpu/config.mk |   2 +-
 arch/x86/cpu/efi/payload.c |  17 +-
 arch/x86/cpu/x86_64/cpu.c  |   5 +
 arch/x86/dts/Makefile  |   2 +-
 arch/x86/lib/Makefile  |   5 +-
 arch/x86/lib/bootm.c   |  11 +-
 arch/x86/lib/elf_x86_64_efi.lds|   5 +-
 arch/x86/lib/relocate.c|   2 +
 arch/x86/lib/zimage.c  |  13 +-
 cmd/Makefile   |   2 +-
 cmd/efi.c  |  78 +---
 common/Kconfig |  15 +-
 common/bloblist.c  |  16 +-
 common/board_f.c   |   8 +-
 common/board_r.c   |   5 +-
 disk/part.c|   5 +-
 doc/develop/bloblist.rst   |  16 ++
 doc/develop/uefi/u-boot_on_efi.rst |   6 +-
 doc/develop/uefi/uefi.rst  |   8 +-
 drivers/block/Kconfig  |  33 
 drivers/block/Makefile |   4 +
 drivers/block/blk-uclass.c |   6 +-
 drivers/block/efi-media-uclass.c   |  15 ++
 drivers/block/efi_blk.c| 115 
 drivers/block/sb_efi_media.c   |  20 +++
 drivers/serial/serial_efi.c|  11 +-
 include/blk.h  |   3 +-
 include/configs/efi-x86_app.h  |  25 +++
 include/dm/uclass-id.h |   3 +-
 include/efi.h  | 113 +++-
 include/efi_api.h  |  15 ++
 include/init.h |   2 +-
 lib/efi/efi.c  |  97 ++
 lib/efi/efi_app.c  

Re: [PATCH v4 09/34] efi: Add EFI uclass for media

[Simon Glass]

Hi Heinrich,

On Sat, 13 Nov 2021 at 02:29, Heinrich Schuchardt  wrote:
>
>
>
> On 11/8/21 06:29, AKASHI Takahiro wrote:
> > On Sun, Nov 07, 2021 at 09:43:22AM -0700, Simon Glass wrote:
> >> Hi Heinrich,
> >>
> >> On Sun, 7 Nov 2021 at 01:21, Heinrich Schuchardt  
> >> wrote:
> >>>
> >>>
> >>>
> >>> On 11/4/21 04:09, Simon Glass wrote:
>  At present UCLASS_EFI is used to represent an EFI filesystem among other
>  things. The description of this uclass is "EFI managed devices" which is
>  pretty vague. The only driver that uses this uclass is in fact not a real
>  U-Boot driver, since its operations do not include a struct udevice.
> 
>  Rather than mess with this, create a new UCLASS_EFI_MEDIA uclass to 
>  handle
>  EFI media such as disks. Make this the uclass to use for EFI media so 
>  that
>  it can be used with 'part list', for example.
> 
>  The existing implementation using UCLASS_EFI remains as is, for
>  discussion.
> 
>  Signed-off-by: Simon Glass 
>  ---
> 
>  (no changes since v2)
> 
>  Changes in v2:
>  - Add MAINTAINERS entry
>  - Show the correct interface type with 'part list'
>  - Update the commit message to explain things better
> 
> MAINTAINERS  |  3 +++
> arch/sandbox/dts/test.dts|  4 
> disk/part.c  |  5 -
> drivers/block/Kconfig| 23 +++
> drivers/block/Makefile   |  3 +++
> drivers/block/blk-uclass.c   |  2 +-
> drivers/block/efi-media-uclass.c | 15 +++
> drivers/block/sb_efi_media.c | 20 
> include/dm/uclass-id.h   |  1 +
> test/dm/Makefile |  1 +
> test/dm/efi_media.c  | 24 
> 11 files changed, 99 insertions(+), 2 deletions(-)
> create mode 100644 drivers/block/efi-media-uclass.c
> create mode 100644 drivers/block/sb_efi_media.c
> create mode 100644 test/dm/efi_media.c
> 

[..]

> [IF_TYPE_NVME]  = UCLASS_NVME,
>  - [IF_TYPE_EFI]   = UCLASS_EFI,
>  + [IF_TYPE_EFI]   = UCLASS_EFI_MEDIA,
> >>>
> >>> Don't break existing code. Create a new if_type here.
> >>
> >> My understanding is that this is not actually used at present. The
> >> tests appear to pass without trouble.
> >>
> >> What problem are you seeing?
>
> I want the following to be successful:
>
> make sandconfig
> make menuconfig
> # set CONFIG_EFI_SELFTEST=y
> make -j$(nproc)
> ./u-boot -T
> setenv efi_selftest block device
> bootefi selftest
> ls efi 0:1
> load efi 0:1 $fdt_addr_r hello.txt
> mm.b $fdt_addr_r
>
> A file with the following string is loaded to memory: "Hello world!\r"

I still am lost here.

Do you mean CONFIG_CMD_BOOTEFI_SELFTEST=y

I cannot find CONFIG_EFI_SELFTEST

Assuming it is that, this is what I see when I try that on -next :

/tmp/b/sandbox/u-boot -T
sandbox_serial serial: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19


U-Boot 2022.01-rc3-00126-gf89615088fb-dirty (Dec 03 2021 - 20:34:58 -0700)

Model: sandbox
DRAM:  128 MiB
WDT:   Not starting gpio-wdt
WDT:   Not starting wdt@0
MMC:   mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
Loading Environment from nowhere... OK
In:cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: sandbox
SCSI:
Net:   eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6:
eth@10004000, eth4: dsa-test-eth, eth2: lan0, eth7: lan1
Hit any key to stop autoboot:  0
=> setenv efi_selftest block device
=> bootefi selftest
Scanning disk mmc2.blk...
Scanning disk mmc1.blk...
Scanning disk mmc0.blk...
Found 3 disks
No EFI system partition
Cannot install EFI_TCG2_PROTOCOL
"dfu_alt_info" env variable not defined!
Probably dfu_alt_info not defined
"dfu_alt_info" env variable not defined!
Probably dfu_alt_info not defined

Testing EFI API implementation

Selected test: 'block device'

Setting up 'block device'
Setting up 'block device' succeeded

Executing 'block device'
lib/efi_selftest/efi_selftest_block_device.c(404):
TODO: Wrong volume label '', expected 'U-BOOT TEST'
Executing 'block device' succeeded

Tearing down 'block device'
Tearing down 'block device' succeeded

Summary: 0 failures

=> ls efi 0:1
Couldn't find partition efi 0:1
=> load efi 0:1 $fdt_addr_r hello.txt
Couldn't find partition efi 0:1
Can't set block device


What am I missing?

>
> It works if I apply
>
> [PATCH 1/1] blk: simplify blk_get_devnum_by_typename()
> https://lists.denx.de/pipermail/u-boot/2021-October/464585.html
>
> If you want to keep the superfluous if_type-uclass table, please, use a
> separate if_type for each uclass in this patch and suggest an
> alternative fix for the test above.

Regards,
Simon
[..]

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Tom,

On Sat, 4 Dec 2021 at 06:52, Tom Rini  wrote:
>
> On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
>
> [huge snip]
> > > There's things that need to be cleaned up because we have some small
> > > number of platforms that went off and did their own thing.  But largely
> > > yes, things make sense to me.  We have:
> > > - We embedded the device tree that will configure U-Boot, because there
> > >   is no way for the hardware to have provided us one.
> > > - We do not embed the device tree that will configure U-Boot, because
> > >   there is already one present in memory for us to use.
> > >
> > > Then we have the developer option of:
> > > - We embedded the device tree that will configure U-Boot, because we're
> > >   developing something.
> >
> > Yes, agreed those are the cases. To me this needs to be a run-time choice.
>
> But it's not possible.  That's the problem we keep going around and
> around about.  People keep raising real life examples where you cannot
> make a run time choice between "device tree we're passed at run time"
> and "device tree we're compiled with".

I haven't seen one. The most extreme case is QEMU and it works fine. I
even added a test with it. What am I missing?

>
> And it's not helpful.  It is ALWAYS the case that we know that we want
> to override the run time device tree with our own, because it's a
> developer developing things or it's a user / production case where we
> must use the provided tree.  NOT doing that is what leads to madness
> like we see for example on Pi where if we don't use the passed tree we
> still need to copy X/Y/Z out of it.

Aren't you talking about the distro DT there, rather than the the one
on the boot disk? That is my reading of that patch. If we need to do
that sort of thing, it doesn't matter where the the cointrol DT comes
from. You are still going to have to do that sort of thing.

It is not ALWAYS the case. I've shown you how easy it is to disable
OF_BOARD and still boot / iterate.

>
> > > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > > provide a way to do that? But what is driving that desire?
> > >
> > > I'm looking for ways to convince you that we do not need to include a
> > > device tree in the binary.  There's a growing set of devices where the
> > > device tree exists with the device.  If it's missing, that's a huge
> > > fatal error we can't do all that much about.  If we need to do something
> > > to that device tree for U-Boot, yes, fine, we should make it straight
> > > forward for the developer to do that.  But that's not the common case!
> >
> > Well we could add another Kconfig which tells U-Boot not to include a
> > devicetree in u-boot.bin, if that would resolve this?
> >
> > I just want to make sure that we always build the devicetrees and that
> > it is easy for a knowledgeable dev to switch over to use them, without
> > spelunking through dozens of other projects to discover the secret DT
> > that no one will tell us about.
>
> Should we demand better documentation for boards?  Yes.  But it's still
> a valid case to have zero device trees for a given platform in-tree.
> Xen is an example of this.  QEMU is an example of this.  Platforms need
> to work without adding special tweaks for us.  Maybe that means some
> features can't be tested in QEMU-as-virtual-platform and only in
> QEMU-faithfully-emulating-specific-physical-platforms.

You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
case, I think. How about we create a special Kconfig for that case? We
need to make some progress here.

>
> > > I guess another part of the problem is that historically almost all
> > > platforms were in the first case I list above, no run time provided
> > > device tree, so we took the kernel one and added our bindings to it.
> > > Now we're being bit by the growing number of platforms that are the
> > > second case, and how do we get our properties in there, and which ones
> > > even make sense to do that for.
> >
> > I think upstreaming the bindings is the solution there. I've made a
> > start, but we need to make progress with this series and all the other
> > things in flight. I think a lot of people want U-Boot to not have a
> > devicetree source files in it for ARMv8 platforms. I am strongly
> > opposed to that. I've laid out my reasons very clearly in the past. I
> > think this is a good summary:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03480.html
>
> Yes, there are some ARMv8 platforms we will have to have the source
> files to, in tree, because they won't come to us at run time.  But
> others we won't for practical reasons, namely that we can't statically
> provide something that exists dynamically without massive duplication of
> code or just taking things from that passed to us tree.

So let's require that the static ones have the Linux DT in our tree
for now. The dynamic ones are just QEMU for ARM and Xen, I think. If
that's it then I 

Re: Question/issues about i.MX6 DDR configuration

[Fabio Estevam]

Hi Francesco,

On Fri, Dec 3, 2021 at 5:47 AM Francesco Dolcini
 wrote:

> I think that this applies even with just one chip select, it is just
> prescribing a procedure and explicitly saying that it must be done for
> all the chip select in use, either 1 or 2.

According to the NXP application note:
https://www.voipac.com/downloads/imx/test/nxp_doc/AN/AN4467_DDR_Calibration.pdf

"To issue a Precharge-All to CS0, write 0x0450 to MDSCR."

but such setting is not done in NXP U-Boot for mx6sabresd:
https://source.codeaurora.org/external/imx/uboot-imx/tree/board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg?h=lf_v2021.04

Back to your original instability issue: I suppose you are talking
about colibri_imx6.c.

Does it work well if you convert it to the mx6_dram_cfg() scheme?

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Tom Rini]

On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:

[huge snip]
> > There's things that need to be cleaned up because we have some small
> > number of platforms that went off and did their own thing.  But largely
> > yes, things make sense to me.  We have:
> > - We embedded the device tree that will configure U-Boot, because there
> >   is no way for the hardware to have provided us one.
> > - We do not embed the device tree that will configure U-Boot, because
> >   there is already one present in memory for us to use.
> >
> > Then we have the developer option of:
> > - We embedded the device tree that will configure U-Boot, because we're
> >   developing something.
> 
> Yes, agreed those are the cases. To me this needs to be a run-time choice.

But it's not possible.  That's the problem we keep going around and
around about.  People keep raising real life examples where you cannot
make a run time choice between "device tree we're passed at run time"
and "device tree we're compiled with".

And it's not helpful.  It is ALWAYS the case that we know that we want
to override the run time device tree with our own, because it's a
developer developing things or it's a user / production case where we
must use the provided tree.  NOT doing that is what leads to madness
like we see for example on Pi where if we don't use the passed tree we
still need to copy X/Y/Z out of it.

> > > Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> > > provide a way to do that? But what is driving that desire?
> >
> > I'm looking for ways to convince you that we do not need to include a
> > device tree in the binary.  There's a growing set of devices where the
> > device tree exists with the device.  If it's missing, that's a huge
> > fatal error we can't do all that much about.  If we need to do something
> > to that device tree for U-Boot, yes, fine, we should make it straight
> > forward for the developer to do that.  But that's not the common case!
> 
> Well we could add another Kconfig which tells U-Boot not to include a
> devicetree in u-boot.bin, if that would resolve this?
> 
> I just want to make sure that we always build the devicetrees and that
> it is easy for a knowledgeable dev to switch over to use them, without
> spelunking through dozens of other projects to discover the secret DT
> that no one will tell us about.

Should we demand better documentation for boards?  Yes.  But it's still
a valid case to have zero device trees for a given platform in-tree.
Xen is an example of this.  QEMU is an example of this.  Platforms need
to work without adding special tweaks for us.  Maybe that means some
features can't be tested in QEMU-as-virtual-platform and only in
QEMU-faithfully-emulating-specific-physical-platforms.

> > I guess another part of the problem is that historically almost all
> > platforms were in the first case I list above, no run time provided
> > device tree, so we took the kernel one and added our bindings to it.
> > Now we're being bit by the growing number of platforms that are the
> > second case, and how do we get our properties in there, and which ones
> > even make sense to do that for.
> 
> I think upstreaming the bindings is the solution there. I've made a
> start, but we need to make progress with this series and all the other
> things in flight. I think a lot of people want U-Boot to not have a
> devicetree source files in it for ARMv8 platforms. I am strongly
> opposed to that. I've laid out my reasons very clearly in the past. I
> think this is a good summary:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03480.html

Yes, there are some ARMv8 platforms we will have to have the source
files to, in tree, because they won't come to us at run time.  But
others we won't for practical reasons, namely that we can't statically
provide something that exists dynamically without massive duplication of
code or just taking things from that passed to us tree.

> I believe I have been consistent in this although with all the
> discussion I'm really not sure anymore.

Yes, everyone has been consistent in these discussions.

> The problem is that various people have various views about how U-Boot
> should work with devicetree. I strongly believe that until we have
> bindings upstream, a central repo for DTs with easy downloading for
> builds, automated validation, among other things, we must maintain the
> devicetree in U-Boot. Just from the POV of energy expended, I do not
> want to be arguing with the Linaro folks about what U-Boot is allowed
> to do every month for the next two years. I'd rather set out the stall
> now and then deal with the problems it causes from that perspective.

The problems of the last going on 12 years won't be solved instantly.
The conflict as I see it is that you're insisting that all platforms
must have statically usable device trees, and I (and I believe others)
are saying that's unreasonable in cases where the trees are dynamic at

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[François Ozog]

Hi Simon and Sandrine

Le jeu. 2 déc. 2021 à 08:10, Sandrine Bailleux 
a écrit :

> Hi Simon,
>
> On 12/1/21 5:51 PM, Simon Glass wrote:
> > Hi Sandrine,
> >
> > On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
> >  wrote:
> >>
> >> Hi everyone,
> >>
> >> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
> >> Apalodimas CC'ed me on this thread.
> >>
> >> First of all, thanks for involving the TF-A developers in this thread
> >> and my apologies for the delay in responding.
> >
> > Thank you for your response.
> >
> >>
> >> On 11/25/21 6:01 PM, François Ozog wrote:
> >>> Hi Simon,
> >>>
> >>> On Thu, 25 Nov 2021 at 17:49, Simon Glass  >>> > wrote:
> >>>
> >>> Hi François,
> >>>
> >>> On Thu, 25 Nov 2021 at 08:11, François Ozog
> >>> mailto:francois.o...@linaro.org>>
> wrote:
> >>> >
> >>> > Hi Simon,
> >>> >
> >>> >
> >>> >
> >>> >
> >>> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
> >>> mailto:ilias.apalodi...@linaro.org>>
> >>> wrote:
> >>> >>
> >>> >> +cc Sandrine
> >>> >>
> >>> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> >>> >>  >>> > wrote:
> >>> >> >
> >>> >> > Hi Simon,
> >>> >> >
> >>> >> >
> >>> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass  >>> > wrote:
> >>> >> > >
> >>> >> > >
> >>> >> > > This series adds support for the FIP format as used by ARM
> >>> Trusted
> >>> >> > > Firmware (in particular TF-A).
> >>> >> > >
> >>> >
> >>> > I will use a question you use often "what problem are you trying
> >>> to solve?". If FIP format is used it means that TF-A/BL2 is going
> to
> >>> parse it and verify the hashes/signatures according to TF-A scheme.
> >>> >
> >>> > The packager will embed in a FIP components like Secure Monitor,
> >>> Secure hypervisor, Secure partitions code and manifests.
> >>> >
> >>> > All in all, U-Boot will be representing a small percentage of the
> >>> functionality offered by secure firmware  as a whole and it feels
> >>> odd to make another implementation that is more "accessory" rather
> >>> than critical for the U-Boot community. It may be a good idea but I
> >>> wish you could explain it.
> >>>
> >>> Here is a talk about Binman, its goals and how it works.
> >>>
> >>> https://www.youtube.com/watch?v=L84ujgUXBOQ
> >>>
> >>> Think of Binman as a separate tool that brings everything
> together. It
> >>> has grown out of U-Boot, largely because U-Boot is the main
> firmware
> >>> in most cases. Getting U-Boot to start up and boot successfully is
> the
> >>> goal of this packaging process. There are lots of instructions in
> the
> >>> tree and elsewhere about how to build an image comprising U-Boot
> and
> >>> various binary blobs. Binman aims to provide documentation for that
> >>> process and an image description that can be used to build an image
> >>> reliably.
> >>>
> >>> We are slowly converting these text instructions into an in-tree
> image
> >>> description.
> >>>
> >>> I believe that Binman can help bring order to the chaos that is
> >>> otherwise only going to grow, with lots of shell scripts, manual
> >>> instructions, obscure binary tools, etc. Binman brings everything
> >>> together and makes it clear what is needing/missing to build an
> image.
> >>>
> >>> >
> >>> >> > > This allows images to be created containing a FIP, which
> >>> itself contains
> >>> >> > > various binaries. With this, image creation can be handled
> >>> from an in-tree
> >>> >> > > image description instead of needing to perform a lot of
> >>> manual steps or
> >>> >> > > custom scripts to build the FIP.
> >>> >> > >
> >>> >
> >>> > That's not my experience of building a fip.  Packaging even Linux
> >>> as a BL33 (instead of U-Boot) is very simple.
> >>>
> >>> But not automatic. With Binman you don't need to worry about the
> >>> packaging. It is done for you. You just need to find all the binary
> >>> blobs that are needed.  This ability is quite important as
> firmware is
> >>> fragmenting and getting very complicated these days.
> >>>
> >>> Binman runs twice...once in the U-Boot tree to do a build and again
> >>> later to repackage, put in a final fdtmap, add signatures and any
> >>> final pieces needed.
> >>>
> >>> See this patch for an example of complicated build instructions
> with
> >>> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the
> changes
> >>> in the .rst file):
> >>>
> >>>
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-...@chromium.org/
> >>>
> >>> That's indeed complicated! I can't comment whether this build process
> is
> >>> "canonical" as per TF-A standards so I'll let TF-A community comment.
> 

Re: [PATCH v6 01/25] doc: Add documentation about devicetree usage

[François Ozog]

Hi Simon

Le sam. 4 déc. 2021 à 02:02, Simon Glass  a écrit :

> Hi Heinrich,
>
> On Fri, 3 Dec 2021 at 13:28, Heinrich Schuchardt 
> wrote:
> >
> > On 12/3/21 9:13 PM, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, 3 Dec 2021 at 06:09, Heinrich Schuchardt 
> wrote:
> > >>
> > >> On 12/3/21 13:34, Heinrich Schuchardt wrote:
> > >>> On 12/2/21 16:58, Simon Glass wrote:
> >  At present some of the ideas and techniques behind devicetree in
> U-Boot
> >  are assumed, implied or unsaid. Add some documentation to cover how
> >  devicetree is build, how it can be modified and the rules about
> using
> >  the various CONFIG_OF_... options.
> > 
> >  Signed-off-by: Simon Glass 
> >  Reviewed-by: Marcel Ziswiler 
> >  ---
> >  This patch attracted quite a bit of discussion here:
> > 
> > 
> https://patchwork.ozlabs.org/project/uboot/patch/20210909201033.755713-4-...@chromium.org/
> > 
> > 
> >  I have not included the text suggested by François. While I agree
> that
> >  it would be useful to have an introduction in this space, I do not
> agree
> >  that we should have two devicetrees or that U-Boot should not have
> its
> >  own
> >  things in the devicetree, so it is not clear to me what we should
> >  actually
> >  write.
> > 
> >  The 'Devicetree Control in U-Boot' docs were recently merged and
> these
> >  provide some base info, for now.
> > 
> >  Changes in v6:
> >  - Fix description of OF_BOARD so it refers just to the current state
> >  - Explain that the 'two devicetrees' refers to two *control*
> devicetrees
> > 
> >  Changes in v5:
> >  - Bring into the OF_BOARD series
> >  - Rebase to master and drop mention of OF_PRIOR_STAGE, since removed
> >  - Refer to the 'control' DTB in the first paragraph
> >  - Use QEMU instead of qemu
> > 
> >  Changes in v3:
> >  - Clarify the 'bug' refered to at the top
> >  - Reword 'This means that there' paragraph to explain
> U-Boot-specific
> >  things
> >  - Move to doc/develop/devicetree now that OF_CONTROL is in the docs
> > 
> >  Changes in v2:
> >  - Fix typos per Sean (thank you!) and a few others
> >  - Add a 'Use of U-Boot /config node' section
> >  - Drop mention of dm-verity since that actually uses the kernel
> cmdline
> >  - Explain that OF_BOARD will still work after these changes (in
> >  'Once this bug is fixed...' paragraph)
> >  - Expand a bit on the reason why the 'Current situation' is bad
> >  - Clarify in a second place that Linux and U-Boot use the same
> devicetree
> >  in 'To be clear, while U-Boot...'
> >  - Expand on why we should have rules for other projects in
> >  'Devicetree in another project'
> >  - Add a comment as to why devicetree in U-Boot is not 'bad design'
> >  - Reword 'in-tree U-Boot devicetree' to 'devicetree source in
> U-Boot'
> >  - Rewrite 'Devicetree generated on-the-fly in another project' to
> cover
> >  points raised on v1
> >  - Add 'Why does U-Boot have its nodes and properties?'
> >  - Add 'Why not have two devicetrees?'
> > 
> > doc/develop/devicetree/dt_update.rst | 555
> +++
> > doc/develop/devicetree/index.rst |   1 +
> > 2 files changed, 556 insertions(+)
> > create mode 100644 doc/develop/devicetree/dt_update.rst
> > 
> > > [..]
> > >
> >  +
> >  +- The other project may not provide a way to support U-Boot's
> >  requirements for
> >  +  devicetree, such as the /config node. Note: On the U-Boot mailing
> >  linst, this
> > >>>
> > >>> Even if you remove these lines in 17/25 I would prefer not to
> introduce
> > >>> typos here:
> > >>>
> > >>> %s/linst/list/
> > >>>
> > >
> > > OK I can fix that.
> > >
> > > [..]
> > >
> >  +Normally, supporting U-Boot's features is trivial, since the
> >  devicetree compiler
> >  +(dtc) can compile the source, including any U-Boot pieces. So the
> >  burden is
> >  +extremely low.
> >  +
> >  +In this case, the devicetree in the other project must track
> U-Boot's
> >  use of
> >  +device tree, so that it remains compatible. See `Devicetree in
> >  another project`_
> >  +for reasons why.
> > >>>
> > >>> Did you ever ask the QEMU community what they think about your ideas?
> > >>> What was the reply?
> > >>
> > >> Looking at the thread
> > >> https://lore.kernel.org/all/20210926183410.256484-1-...@chromium.org/
> > >> the QEMU project said NAK. This matches the expectation that I
> expressed
> > >> repeatedly.
> > >>
> > >> Why don't you mention the QEMU reply in this patch series and adjust
> > >> your design accordingly?
> > >
> > > The QEMU maintainer may react when he sees a problem.
> >
> > Why are you unwilling to admit the problem? QEMU will never support
> > U-Boot specific stuff.
> >
> >