Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On Tue, Nov 15, 2022 at 02:06:57PM +0100, Cédric Le Goater wrote:
> Hello Peter,
>
> On 11/14/22 20:08, Peter Delevoryas wrote:
> > I've been using this patch for a long time so that I don't have to use
> > dd to zero-extend stuff all the time. It's just doing what people are
> > doing already, right? I hope it's not controversial.
>
> I simply run :
>
>truncate --size
Ohhh I always forget about that command. That's certainly easier
than what I was doing.
>
> on the FW file when needed and it is rare because most FW image builders
> know the flash size of the target.
welll my yocto builds don't, I'm not sure we would want them to either?
Because that would be extra data with no value to transfer to the BMC/etc.
flashcp erases the whole flash initially, which is why I don't worry
about providing a firmware image that is only covering the first 30 MB of
flash.
>
> However, the current error message is confusing and the following could
> be an improvement :
>
> @@ -1606,6 +1606,13 @@ static void m25p80_realize(SSIPeripheral
> if (s->blk) {
> uint64_t perm = BLK_PERM_CONSISTENT_READ |
> (blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE :
> 0);
> +
> +if (blk_getlength(s->blk) != s->size) {
> +error_setg(errp, "backend file is too small for flash device %s
> (%d MB)",
> + object_class_get_name(OBJECT_CLASS(mc)), s->size >>
> 20);
> +return;
> +}
> +
> ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);
> if (ret < 0) {
> return;
>
> I can send a patch for the above.
Ohhh yay! Thanks!
>
>
>
> I mostly run the QEMU machines with -snapshot, this hack :
>
> blk_set_allow_write_beyond_eof(s->blk, true);
>
> makes it work also ...
!!! Ohhh! interesting...that seems more sketchy but certainly simpler.
>
>
>
>
> Thanks,
>
> C.
>
> > One note: I couldn't figure out how to make it work without changing the
> > permissions on the block device to allow truncation. If somebody knows
> > how to avoid the `blk_get_perm`, `blk_set_perm` calls here, let me know!
> >
> > Thanks,
> > Peter
> >
> > Peter Delevoryas (1):
> >hw/arm/aspeed: Automatically zero-extend flash images
> >
> > hw/arm/aspeed.c | 40 +++-
> > 1 file changed, 39 insertions(+), 1 deletion(-)
> >
>
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On Tue, Nov 15, 2022 at 10:48:40AM +, Peter Maydell wrote: > On Mon, 14 Nov 2022 at 19:08, Peter Delevoryas wrote: > > > > I've been using this patch for a long time so that I don't have to use > > dd to zero-extend stuff all the time. It's just doing what people are > > doing already, right? I hope it's not controversial. > > We just had a thread about this kind of thing for one of the > riscv boards (although there the attempted approach was to > change the size of the flash device to match the provided > file, rather than changing the file to match the flash device): > https://lore.kernel.org/qemu-devel/20221107130217.2243815-1-suni...@ventanamicro.com/ Thanks for linking this! > > The short summary is (a) yes, it's controversial and > (b) if we do something for this we need to have a standard > approach that we do on all boards, not "some boards do > some weird magic in different ways from everything else"... I see, that's ok, I'll investigate option (b) then. Thanks, Peter > > thanks > -- PMM
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On 11/15/22 15:06, Philippe Mathieu-Daudé wrote:
On 15/11/22 14:06, Cédric Le Goater wrote:
Hello Peter,
On 11/14/22 20:08, Peter Delevoryas wrote:
I've been using this patch for a long time so that I don't have to use
dd to zero-extend stuff all the time. It's just doing what people are
doing already, right? I hope it's not controversial.
I simply run :
truncate --size
on the FW file when needed and it is rare because most FW image builders
know the flash size of the target.
However, the current error message is confusing and the following could
be an improvement :
@@ -1606,6 +1606,13 @@ static void m25p80_realize(SSIPeripheral
if (s->blk) {
uint64_t perm = BLK_PERM_CONSISTENT_READ |
(blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE :
0);
+
+ if (blk_getlength(s->blk) != s->size) {
'!=' -> '<' ?
Hey. yes :)
I will send a patch. I am not sure this is 7.2 material though.
Thanks,
C.
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On 15/11/22 14:06, Cédric Le Goater wrote:
Hello Peter,
On 11/14/22 20:08, Peter Delevoryas wrote:
I've been using this patch for a long time so that I don't have to use
dd to zero-extend stuff all the time. It's just doing what people are
doing already, right? I hope it's not controversial.
I simply run :
truncate --size
on the FW file when needed and it is rare because most FW image builders
know the flash size of the target.
However, the current error message is confusing and the following could
be an improvement :
@@ -1606,6 +1606,13 @@ static void m25p80_realize(SSIPeripheral
if (s->blk) {
uint64_t perm = BLK_PERM_CONSISTENT_READ |
(blk_supports_write_perm(s->blk) ?
BLK_PERM_WRITE : 0);
+
+ if (blk_getlength(s->blk) != s->size) {
'!=' -> '<' ?
+ error_setg(errp, "backend file is too small for flash
device %s (%d MB)",
+ object_class_get_name(OBJECT_CLASS(mc)), s->size
>> 20);
+ return;
+ }
+
ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);
if (ret < 0) {
return;
I can send a patch for the above.
I mostly run the QEMU machines with -snapshot, this hack :
blk_set_allow_write_beyond_eof(s->blk, true);
makes it work also ...
Thanks,
C.
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
Hello Peter,
On 11/14/22 20:08, Peter Delevoryas wrote:
I've been using this patch for a long time so that I don't have to use
dd to zero-extend stuff all the time. It's just doing what people are
doing already, right? I hope it's not controversial.
I simply run :
truncate --size
on the FW file when needed and it is rare because most FW image builders
know the flash size of the target.
However, the current error message is confusing and the following could
be an improvement :
@@ -1606,6 +1606,13 @@ static void m25p80_realize(SSIPeripheral
if (s->blk) {
uint64_t perm = BLK_PERM_CONSISTENT_READ |
(blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE : 0);
+
+if (blk_getlength(s->blk) != s->size) {
+error_setg(errp, "backend file is too small for flash device %s (%d
MB)",
+ object_class_get_name(OBJECT_CLASS(mc)), s->size >> 20);
+return;
+}
+
ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);
if (ret < 0) {
return;
I can send a patch for the above.
I mostly run the QEMU machines with -snapshot, this hack :
blk_set_allow_write_beyond_eof(s->blk, true);
makes it work also ...
Thanks,
C.
One note: I couldn't figure out how to make it work without changing the
permissions on the block device to allow truncation. If somebody knows
how to avoid the `blk_get_perm`, `blk_set_perm` calls here, let me know!
Thanks,
Peter
Peter Delevoryas (1):
hw/arm/aspeed: Automatically zero-extend flash images
hw/arm/aspeed.c | 40 +++-
1 file changed, 39 insertions(+), 1 deletion(-)
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On Mon, 14 Nov 2022 at 19:08, Peter Delevoryas wrote: > > I've been using this patch for a long time so that I don't have to use > dd to zero-extend stuff all the time. It's just doing what people are > doing already, right? I hope it's not controversial. We just had a thread about this kind of thing for one of the riscv boards (although there the attempted approach was to change the size of the flash device to match the provided file, rather than changing the file to match the flash device): https://lore.kernel.org/qemu-devel/20221107130217.2243815-1-suni...@ventanamicro.com/ The short summary is (a) yes, it's controversial and (b) if we do something for this we need to have a standard approach that we do on all boards, not "some boards do some weird magic in different ways from everything else"... thanks -- PMM
[PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
I've been using this patch for a long time so that I don't have to use dd to zero-extend stuff all the time. It's just doing what people are doing already, right? I hope it's not controversial. One note: I couldn't figure out how to make it work without changing the permissions on the block device to allow truncation. If somebody knows how to avoid the `blk_get_perm`, `blk_set_perm` calls here, let me know! Thanks, Peter Peter Delevoryas (1): hw/arm/aspeed: Automatically zero-extend flash images hw/arm/aspeed.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) -- 2.38.1
