| Message ID | 03c2f42b32fb4e304319c241122ae83584f085e0.1573087610.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for,4.2,v1,1/1] riscv/virt: Increase flash size | expand |
On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > Coreboot developers have requested that they have at least 32MB of flash > to load binaries. We currently have 32MB of flash, but it is split in > two to allow loading two flash binaries. Let's increase the flash size > from 32MB to 64MB to ensure we have a single region that is 32MB. > > No QEMU release has include flash in the RISC-V virt machine, so this > isn't a breaking change. Even if we had, I wouldn't consider it a breaking change because it adds to the memory map so existing programs will continue to run fine. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/virt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index cc8f311e6b..23f340df19 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -62,7 +62,7 @@ static const struct MemmapEntry { > [VIRT_PLIC] = { 0xc000000, 0x4000000 }, > [VIRT_UART0] = { 0x10000000, 0x100 }, > [VIRT_VIRTIO] = { 0x10001000, 0x1000 }, > - [VIRT_FLASH] = { 0x20000000, 0x2000000 }, > + [VIRT_FLASH] = { 0x20000000, 0x4000000 }, > [VIRT_DRAM] = { 0x80000000, 0x0 }, > [VIRT_PCIE_MMIO] = { 0x40000000, 0x40000000 }, > [VIRT_PCIE_PIO] = { 0x03000000, 0x00010000 }, Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com> I'll include this in my next PR, which should be soon -- I was about to send it, but figure I should look at my email first :)
On Thu, Nov 7, 2019 at 10:01 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt <palmer@dabbelt.com> wrote: > > > > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > > > Coreboot developers have requested that they have at least 32MB of flash > > > to load binaries. We currently have 32MB of flash, but it is split in > > > two to allow loading two flash binaries. Let's increase the flash size > > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > > isn't a breaking change. > > > > Even if we had, I wouldn't consider it a breaking change because it adds to > > the memory map so existing programs will continue to run fine. > > I have a feeling you may find that some old command lines won't > work any more because they specified a flash contents binary > that was the old 32MB and now it needs to be padded out to 64MB. Yes, that is correct. Everyone using -pflash will need to change the size of their binaries. This was only just merged into QEMU master though and hasn't been in a release so I don't think many people are using it. I only know of two users, one is me and someone from Coreboot who requested the larger size. It doesn't seem like a problem users will see. Alistair > But I haven't tested whether this theory is correct (it will > depend on how the flash contents are specified -- --bios will > be ok, as will loading contents directly as an ELF file or > similar, specifying contents by a -drive option intended to be > consumed by the pflash is the case which likely needs extra padding.) > > thanks > -- PMM
On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > > Coreboot developers have requested that they have at least 32MB of flash > > to load binaries. We currently have 32MB of flash, but it is split in > > two to allow loading two flash binaries. Let's increase the flash size > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > isn't a breaking change. > > Even if we had, I wouldn't consider it a breaking change because it adds to > the memory map so existing programs will continue to run fine. I have a feeling you may find that some old command lines won't work any more because they specified a flash contents binary that was the old 32MB and now it needs to be padded out to 64MB. But I haven't tested whether this theory is correct (it will depend on how the flash contents are specified -- --bios will be ok, as will loading contents directly as an ELF file or similar, specifying contents by a -drive option intended to be consumed by the pflash is the case which likely needs extra padding.) thanks -- PMM
Alistair Francis <alistair23@gmail.com> writes: > On Thu, Nov 7, 2019 at 10:01 AM Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt <palmer@dabbelt.com> wrote: >> > >> > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: >> > > Coreboot developers have requested that they have at least 32MB of flash >> > > to load binaries. We currently have 32MB of flash, but it is split in >> > > two to allow loading two flash binaries. Let's increase the flash size >> > > from 32MB to 64MB to ensure we have a single region that is 32MB. >> > > >> > > No QEMU release has include flash in the RISC-V virt machine, so this >> > > isn't a breaking change. >> > >> > Even if we had, I wouldn't consider it a breaking change because it adds to >> > the memory map so existing programs will continue to run fine. >> >> I have a feeling you may find that some old command lines won't >> work any more because they specified a flash contents binary >> that was the old 32MB and now it needs to be padded out to 64MB. > > Yes, that is correct. Everyone using -pflash will need to change the > size of their binaries. This was only just merged into QEMU master > though and hasn't been in a release so I don't think many people are > using it. > > I only know of two users, one is me and someone from Coreboot who > requested the larger size. It doesn't seem like a problem users will > see. At least the error message they get will be more informative now ;-) -- Alex Bennée
On Thu, Nov 7, 2019 at 8:58 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > > Coreboot developers have requested that they have at least 32MB of flash > > to load binaries. We currently have 32MB of flash, but it is split in > > two to allow loading two flash binaries. Let's increase the flash size > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > isn't a breaking change. > > Even if we had, I wouldn't consider it a breaking change because it adds to > the memory map so existing programs will continue to run fine. > > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/virt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > > index cc8f311e6b..23f340df19 100644 > > --- a/hw/riscv/virt.c > > +++ b/hw/riscv/virt.c > > @@ -62,7 +62,7 @@ static const struct MemmapEntry { > > [VIRT_PLIC] = { 0xc000000, 0x4000000 }, > > [VIRT_UART0] = { 0x10000000, 0x100 }, > > [VIRT_VIRTIO] = { 0x10001000, 0x1000 }, > > - [VIRT_FLASH] = { 0x20000000, 0x2000000 }, > > + [VIRT_FLASH] = { 0x20000000, 0x4000000 }, > > [VIRT_DRAM] = { 0x80000000, 0x0 }, > > [VIRT_PCIE_MMIO] = { 0x40000000, 0x40000000 }, > > [VIRT_PCIE_PIO] = { 0x03000000, 0x00010000 }, > > Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com> > > I'll include this in my next PR, which should be soon -- I was about to send > it, but figure I should look at my email first :) Ping! I want to make sure the current patches you have make it into 4.2. Alistair
On Thu, Nov 7, 2019 at 8:54 AM Alistair Francis <alistair.francis@wdc.com> wrote: > > Coreboot developers have requested that they have at least 32MB of flash > to load binaries. We currently have 32MB of flash, but it is split in > two to allow loading two flash binaries. Let's increase the flash size > from 32MB to 64MB to ensure we have a single region that is 32MB. > > No QEMU release has include flash in the RISC-V virt machine, so this > isn't a breaking change. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/virt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
On Mon, Nov 11, 2019 at 7:30 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Thu, Nov 7, 2019 at 8:54 AM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > Coreboot developers have requested that they have at least 32MB of flash > > to load binaries. We currently have 32MB of flash, but it is split in > > two to allow loading two flash binaries. Let's increase the flash size > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > isn't a breaking change. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/virt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Thanks! Ping! I really want this in 4.2. Otherwise we are stuck with a compatibility issue. Alistair
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index cc8f311e6b..23f340df19 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -62,7 +62,7 @@ static const struct MemmapEntry { [VIRT_PLIC] = { 0xc000000, 0x4000000 }, [VIRT_UART0] = { 0x10000000, 0x100 }, [VIRT_VIRTIO] = { 0x10001000, 0x1000 }, - [VIRT_FLASH] = { 0x20000000, 0x2000000 }, + [VIRT_FLASH] = { 0x20000000, 0x4000000 }, [VIRT_DRAM] = { 0x80000000, 0x0 }, [VIRT_PCIE_MMIO] = { 0x40000000, 0x40000000 }, [VIRT_PCIE_PIO] = { 0x03000000, 0x00010000 },
Coreboot developers have requested that they have at least 32MB of flash to load binaries. We currently have 32MB of flash, but it is split in two to allow loading two flash binaries. Let's increase the flash size from 32MB to 64MB to ensure we have a single region that is 32MB. No QEMU release has include flash in the RISC-V virt machine, so this isn't a breaking change. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)