[PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Alistair Francis]

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(-)

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 },
-- 
2.23.0

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Palmer Dabbelt]

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 :)

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Alistair Francis]

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

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Peter Maydell]

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

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Alex Bennée]

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

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Alistair Francis]

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

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Bin Meng]

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>

Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size

[Alistair Francis]

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