Re: [PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-25 Thread Bernhard Beschow



Am 25. Februar 2025 14:29:15 UTC schrieb Peter Maydell 
:
>On Sun, 23 Feb 2025 at 11:47, Bernhard Beschow  wrote:
>>
>> On a real device, the boot ROM contains the very first instructions the CPU
>> executes. Also, U-Boot calls into the ROM to determine the boot device. While
>> we're not actually implementing this here, let's create the infrastructure 
>> and
>> add a dummy ROM with all zeros. This allows for implementing a ROM later 
>> without
>> touching the source code and even allows for users to provide their own ROMs.
>>
>> The imx8mp-boot.rom was created with
>> `dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.
>>
>> Signed-off-by: Bernhard Beschow 
>
>Hi; I've thought about this a bit, and I think we should just
>postpone having code to load a rom blob until we have an
>actual rom blob that's not all zeroes. This makes this
>board the same as e.g. imx7 which also doesn't try to load
>a ROM blob. You can create a memory region for the ROM,
>like imx7; just don't load anything into it, and that's
>exactly the same as if there was an all-zeroes file that
>got loaded.
>
>This doesn't prevent users from experimenting with providing
>their own ROM code, because they can do that with the
>generic loader (passing it either an ELF file set up to
>load to the ROM area or else a binary blob plus the
>start address of 0).
>
>That way we aren't carrying code we aren't using and an
>awkward binary blob in git; we can add those when we need
>them.

Yes, makes sense.

Best regards,
Bernhard

>
>thanks
>-- PMM



Re: [PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-25 Thread Peter Maydell
On Sun, 23 Feb 2025 at 11:47, Bernhard Beschow  wrote:
>
> On a real device, the boot ROM contains the very first instructions the CPU
> executes. Also, U-Boot calls into the ROM to determine the boot device. While
> we're not actually implementing this here, let's create the infrastructure and
> add a dummy ROM with all zeros. This allows for implementing a ROM later 
> without
> touching the source code and even allows for users to provide their own ROMs.
>
> The imx8mp-boot.rom was created with
> `dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.
>
> Signed-off-by: Bernhard Beschow 

Hi; I've thought about this a bit, and I think we should just
postpone having code to load a rom blob until we have an
actual rom blob that's not all zeroes. This makes this
board the same as e.g. imx7 which also doesn't try to load
a ROM blob. You can create a memory region for the ROM,
like imx7; just don't load anything into it, and that's
exactly the same as if there was an all-zeroes file that
got loaded.

This doesn't prevent users from experimenting with providing
their own ROM code, because they can do that with the
generic loader (passing it either an ELF file set up to
load to the ROM area or else a binary blob plus the
start address of 0).

That way we aren't carrying code we aren't using and an
awkward binary blob in git; we can add those when we need
them.

thanks
-- PMM



[PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-23 Thread Bernhard Beschow
On a real device, the boot ROM contains the very first instructions the CPU
executes. Also, U-Boot calls into the ROM to determine the boot device. While
we're not actually implementing this here, let's create the infrastructure and
add a dummy ROM with all zeros. This allows for implementing a ROM later without
touching the source code and even allows for users to provide their own ROMs.

The imx8mp-boot.rom was created with
`dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.

Signed-off-by: Bernhard Beschow 
---
 MAINTAINERS |   1 +
 include/hw/arm/fsl-imx8mp.h |   1 +
 hw/arm/fsl-imx8mp.c |  18 ++
 pc-bios/imx8mp-boot.rom | Bin 0 -> 258048 bytes
 pc-bios/meson.build |   1 +
 5 files changed, 21 insertions(+)
 create mode 100644 pc-bios/imx8mp-boot.rom

diff --git a/MAINTAINERS b/MAINTAINERS
index 2e7fc6fa91..489e426d85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -831,6 +831,7 @@ F: hw/pci-host/fsl_imx8m_phy.c
 F: include/hw/arm/fsl-imx8mp.h
 F: include/hw/misc/imx8mp_*.h
 F: include/hw/pci-host/fsl_imx8m_phy.h
+F: pc-bios/imx8mp*
 F: docs/system/arm/imx8mp-evk.rst
 
 MPS2 / MPS3
diff --git a/include/hw/arm/fsl-imx8mp.h b/include/hw/arm/fsl-imx8mp.h
index 5247e972b8..4dbe30f524 100644
--- a/include/hw/arm/fsl-imx8mp.h
+++ b/include/hw/arm/fsl-imx8mp.h
@@ -68,6 +68,7 @@ struct FslImx8mpState {
 DesignwarePCIEHost pcie;
 FslImx8mPciePhyState   pcie_phy;
 OrIRQState gpt5_gpt6_irq;
+MemoryRegion   boot_rom;
 
 uint32_t   phy_num;
 bool   phy_connected;
diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
index f0d5980300..f26cf5984e 100644
--- a/hw/arm/fsl-imx8mp.c
+++ b/hw/arm/fsl-imx8mp.c
@@ -9,12 +9,14 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/datadir.h"
 #include "exec/address-spaces.h"
 #include "hw/arm/bsa.h"
 #include "hw/arm/fsl-imx8mp.h"
 #include "hw/intc/arm_gicv3.h"
 #include "hw/misc/unimp.h"
 #include "hw/boards.h"
+#include "hw/loader.h"
 #include "system/system.h"
 #include "target/arm/cpu-qom.h"
 #include "qapi/error.h"
@@ -263,6 +265,7 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error 
**errp)
 MachineState *ms = MACHINE(qdev_get_machine());
 FslImx8mpState *s = FSL_IMX8MP(dev);
 DeviceState *gicdev = DEVICE(&s->gic);
+g_autofree char *filename = NULL;
 int i;
 
 if (ms->smp.cpus > FSL_IMX8MP_NUM_CPUS) {
@@ -644,10 +647,25 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error 
**errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->pcie_phy), 0,
 fsl_imx8mp_memmap[FSL_IMX8MP_PCIE_PHY1].addr);
 
+/* ROM memory */
+if (!memory_region_init_rom(&s->boot_rom, OBJECT(dev),
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].name,
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].size,
+errp)) {
+return;
+}
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "imx8mp-boot.rom");
+load_image_size(filename, memory_region_get_ram_ptr(&s->boot_rom),
+memory_region_size(&s->boot_rom));
+memory_region_add_subregion(get_system_memory(),
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].addr,
+&s->boot_rom);
+
 /* Unimplemented devices */
 for (i = 0; i < ARRAY_SIZE(fsl_imx8mp_memmap); i++) {
 switch (i) {
 case FSL_IMX8MP_ANA_PLL:
+case FSL_IMX8MP_BOOT_ROM:
 case FSL_IMX8MP_CCM:
 case FSL_IMX8MP_GIC_DIST:
 case FSL_IMX8MP_GIC_REDIST:
diff --git a/pc-bios/imx8mp-boot.rom b/pc-bios/imx8mp-boot.rom
new file mode 100644
index 
..5324b5eed200e723d048f8476e4d96d45622fd4d
GIT binary patch
literal 258048
zcmeIuF#!Mo0K%a4Pi+Q&h(KY$fB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM
z7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*
z1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd
z0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA
zz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEj
zFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r
z3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@
z0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VK
zfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5
zV8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM
z7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*
z1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd
z0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA
zz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEj
zFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r
z3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@
z0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VK
zfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA

Re: [PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-23 Thread Bernhard Beschow



Am 17. Februar 2025 22:48:23 UTC schrieb Bernhard Beschow :
>
>
>Am 17. Februar 2025 13:28:42 UTC schrieb Peter Maydell 
>:
>>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
>>>
>>> On a real device, the boot ROM contains the very first instructions the CPU
>>> executes. Also, U-Boot calls into the ROM to determine the boot device. 
>>> While
>>> we're not actually implementing this here, let's create the infrastructure 
>>> and
>>> add a dummy ROM with all zeros. This allows for implementing a ROM later 
>>> without
>>> touching the source code and even allows for users to provide their own 
>>> ROMs.
>>>
>>> The imx8mp-boot.rom was created with
>>> `dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.
>>
>>If it's all zeroes, we don't need to commit it to git as a binary,
>>we can generate it at build time. (We should avoid having
>>binaries in git as much as we can manage.)
>
>I went with this solution since it trivially works on any build platform. Any 
>idea how to generate the file in a portable way?

Besides this question, the compressed size of the file in Git is just 276 bytes:

  $ git hash-object pc-bios/imx8mp-boot.rom
  5324b5eed200e723d048f8476e4d96d45622fd4d
  $ git verify-pack -v .git/objects/pack/pack-*.idx | grep 5324b5eed2
  5324b5eed200e723d048f8476e4d96d45622fd4d blob   258048 276 787647

Is it really worth generating it during the build process?

Best regards,
Bernhard

>
>Thanks,
>Bernhard
>
>>
>>thanks
>>-- PMM



Re: [PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-17 Thread Bernhard Beschow



Am 17. Februar 2025 13:28:42 UTC schrieb Peter Maydell 
:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
>>
>> On a real device, the boot ROM contains the very first instructions the CPU
>> executes. Also, U-Boot calls into the ROM to determine the boot device. While
>> we're not actually implementing this here, let's create the infrastructure 
>> and
>> add a dummy ROM with all zeros. This allows for implementing a ROM later 
>> without
>> touching the source code and even allows for users to provide their own ROMs.
>>
>> The imx8mp-boot.rom was created with
>> `dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.
>
>If it's all zeroes, we don't need to commit it to git as a binary,
>we can generate it at build time. (We should avoid having
>binaries in git as much as we can manage.)

I went with this solution since it trivially works on any build platform. Any 
idea how to generate the file in a portable way?

Thanks,
Bernhard

>
>thanks
>-- PMM



Re: [PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-17 Thread Peter Maydell
On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
>
> On a real device, the boot ROM contains the very first instructions the CPU
> executes. Also, U-Boot calls into the ROM to determine the boot device. While
> we're not actually implementing this here, let's create the infrastructure and
> add a dummy ROM with all zeros. This allows for implementing a ROM later 
> without
> touching the source code and even allows for users to provide their own ROMs.
>
> The imx8mp-boot.rom was created with
> `dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.

If it's all zeroes, we don't need to commit it to git as a binary,
we can generate it at build time. (We should avoid having
binaries in git as much as we can manage.)

thanks
-- PMM



[PATCH v2 16/18] hw/arm/fsl-imx8mp: Add boot ROM

2025-02-04 Thread Bernhard Beschow
On a real device, the boot ROM contains the very first instructions the CPU
executes. Also, U-Boot calls into the ROM to determine the boot device. While
we're not actually implementing this here, let's create the infrastructure and
add a dummy ROM with all zeros. This allows for implementing a ROM later without
touching the source code and even allows for users to provide their own ROMs.

The imx8mp-boot.rom was created with
`dd if=/dev/zero of=imx8mp-boot.rom bs=1 count=258048`.

Signed-off-by: Bernhard Beschow 
---
 MAINTAINERS |   1 +
 include/hw/arm/fsl-imx8mp.h |   1 +
 hw/arm/fsl-imx8mp.c |  18 ++
 pc-bios/imx8mp-boot.rom | Bin 0 -> 258048 bytes
 pc-bios/meson.build |   1 +
 5 files changed, 21 insertions(+)
 create mode 100644 pc-bios/imx8mp-boot.rom

diff --git a/MAINTAINERS b/MAINTAINERS
index 94af3d90e4..ee837a3f6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -831,6 +831,7 @@ F: hw/pci-host/fsl_imx8m_phy.c
 F: include/hw/arm/fsl-imx8mp.h
 F: include/hw/misc/imx8mp_*.h
 F: include/hw/pci-host/fsl_imx8m_phy.h
+F: pc-bios/imx8mp*
 F: docs/system/arm/imx8mp-evk.rst
 
 MPS2 / MPS3
diff --git a/include/hw/arm/fsl-imx8mp.h b/include/hw/arm/fsl-imx8mp.h
index 5247e972b8..4dbe30f524 100644
--- a/include/hw/arm/fsl-imx8mp.h
+++ b/include/hw/arm/fsl-imx8mp.h
@@ -68,6 +68,7 @@ struct FslImx8mpState {
 DesignwarePCIEHost pcie;
 FslImx8mPciePhyState   pcie_phy;
 OrIRQState gpt5_gpt6_irq;
+MemoryRegion   boot_rom;
 
 uint32_t   phy_num;
 bool   phy_connected;
diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
index 14d696957a..6439639110 100644
--- a/hw/arm/fsl-imx8mp.c
+++ b/hw/arm/fsl-imx8mp.c
@@ -9,12 +9,14 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/datadir.h"
 #include "exec/address-spaces.h"
 #include "hw/arm/bsa.h"
 #include "hw/arm/fsl-imx8mp.h"
 #include "hw/intc/arm_gicv3.h"
 #include "hw/misc/unimp.h"
 #include "hw/boards.h"
+#include "hw/loader.h"
 #include "system/system.h"
 #include "target/arm/cpu-qom.h"
 #include "qapi/qmp/qlist.h"
@@ -266,6 +268,7 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error 
**errp)
 MachineState *ms = MACHINE(qdev_get_machine());
 FslImx8mpState *s = FSL_IMX8MP(dev);
 DeviceState *gicdev = DEVICE(&s->gic);
+g_autofree char *filename = NULL;
 int i;
 
 if (ms->smp.cpus > FSL_IMX8MP_NUM_CPUS) {
@@ -648,10 +651,25 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error 
**errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->pcie_phy), 0,
 fsl_imx8mp_memmap[FSL_IMX8MP_PCIE_PHY1].addr);
 
+/* ROM memory */
+if (!memory_region_init_rom(&s->boot_rom, OBJECT(dev),
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].name,
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].size,
+errp)) {
+return;
+}
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "imx8mp-boot.rom");
+load_image_size(filename, memory_region_get_ram_ptr(&s->boot_rom),
+memory_region_size(&s->boot_rom));
+memory_region_add_subregion(get_system_memory(),
+fsl_imx8mp_memmap[FSL_IMX8MP_BOOT_ROM].addr,
+&s->boot_rom);
+
 /* Unimplemented devices */
 for (i = 0; i < ARRAY_SIZE(fsl_imx8mp_memmap); i++) {
 switch (i) {
 case FSL_IMX8MP_ANA_PLL:
+case FSL_IMX8MP_BOOT_ROM:
 case FSL_IMX8MP_CCM:
 case FSL_IMX8MP_GIC_DIST:
 case FSL_IMX8MP_GIC_REDIST:
diff --git a/pc-bios/imx8mp-boot.rom b/pc-bios/imx8mp-boot.rom
new file mode 100644
index 
..5324b5eed200e723d048f8476e4d96d45622fd4d
GIT binary patch
literal 258048
zcmeIuF#!Mo0K%a4Pi+Q&h(KY$fB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM
z7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*
z1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd
z0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA
zz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEj
zFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r
z3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@
z0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VK
zfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5
zV8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM
z7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*
z1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd
z0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA
zz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEj
zFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r
z3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@
z0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VK
zfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3