* [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-08-05 19:37 ` Eric Blake
2021-06-23 18:00 ` [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses Philippe Mathieu-Daudé
` (8 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
We report the card is in an inconsistent state, but don't precise
in which state it is. Add this information, as it is useful when
debugging problems.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 282d39a7042..d8fdf84f4db 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1504,7 +1504,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
return sd_illegal;
}
- qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
+ qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
+ req.cmd, sd_state_name(sd->state));
return sd_illegal;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is
2021-06-23 18:00 ` [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is Philippe Mathieu-Daudé
@ 2021-08-05 19:37 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2021-08-05 19:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 08:00:13PM +0200, Philippe Mathieu-Daudé wrote:
> We report the card is in an inconsistent state, but don't precise
s/don't/aren't/
> in which state it is. Add this information, as it is useful when
> debugging problems.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/sd/sd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 282d39a7042..d8fdf84f4db 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1504,7 +1504,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
> return sd_illegal;
> }
>
> - qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
> + qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
> + req.cmd, sd_state_name(sd->state));
> return sd_illegal;
> }
>
> --
> 2.31.1
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
2021-06-23 18:00 ` [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-08-05 19:46 ` Eric Blake
2021-06-23 18:00 ` [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd' Philippe Mathieu-Daudé
` (7 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
Multiple commands have to check the address requested is valid.
Extract this code pattern as a new address_in_range() helper, and
log invalid accesses as guest errors.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d8fdf84f4db..9c8dd11bad1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -937,6 +937,18 @@ static void sd_lock_command(SDState *sd)
sd->card_status &= ~CARD_IS_LOCKED;
}
+static bool address_in_range(SDState *sd, const char *desc,
+ uint64_t addr, uint32_t length)
+{
+ if (addr + length > sd->size) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s offset %lu > card %lu [%%%u]\n",
+ desc, addr, sd->size, length);
+ sd->card_status |= ADDRESS_ERROR;
+ return false;
+ }
+ return true;
+}
+
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
{
uint32_t rca = 0x0000;
@@ -1218,8 +1230,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
switch (sd->state) {
case sd_transfer_state:
- if (addr + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "READ_BLOCK", addr, sd->blk_len)) {
return sd_r1;
}
@@ -1264,8 +1275,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
switch (sd->state) {
case sd_transfer_state:
- if (addr + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "WRITE_BLOCK", addr, sd->blk_len)) {
return sd_r1;
}
@@ -1325,8 +1335,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
switch (sd->state) {
case sd_transfer_state:
- if (addr >= sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "SET_WRITE_PROT", addr, 1)) {
return sd_r1b;
}
@@ -1348,8 +1357,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
switch (sd->state) {
case sd_transfer_state:
- if (addr >= sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "CLR_WRITE_PROT", addr, 1)) {
return sd_r1b;
}
@@ -1826,8 +1834,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
case 25: /* CMD25: WRITE_MULTIPLE_BLOCK */
if (sd->data_offset == 0) {
/* Start of the block - let's check the address is valid */
- if (sd->data_start + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "WRITE_MULTIPLE_BLOCK",
+ sd->data_start, sd->blk_len)) {
break;
}
if (sd->size <= SDSC_MAX_CAPACITY) {
@@ -1999,8 +2007,8 @@ uint8_t sd_read_byte(SDState *sd)
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
if (sd->data_offset == 0) {
- if (sd->data_start + io_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
+ if (!address_in_range(sd, "READ_MULTIPLE_BLOCK",
+ sd->data_start, io_len)) {
return 0x00;
}
BLK_READ_BLOCK(sd->data_start, io_len);
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses
2021-06-23 18:00 ` [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses Philippe Mathieu-Daudé
@ 2021-08-05 19:46 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2021-08-05 19:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 08:00:14PM +0200, Philippe Mathieu-Daudé wrote:
> Multiple commands have to check the address requested is valid.
check that the
> Extract this code pattern as a new address_in_range() helper, and
> log invalid accesses as guest errors.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/sd/sd.c | 32 ++++++++++++++++++++------------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index d8fdf84f4db..9c8dd11bad1 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -937,6 +937,18 @@ static void sd_lock_command(SDState *sd)
> sd->card_status &= ~CARD_IS_LOCKED;
> }
>
> +static bool address_in_range(SDState *sd, const char *desc,
> + uint64_t addr, uint32_t length)
> +{
> + if (addr + length > sd->size) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s offset %lu > card %lu [%%%u]\n",
> + desc, addr, sd->size, length);
For a (fictitiously small) device with 2048 bytes and a read request
of 2k at offset 1k, this results in the odd message:
READ_BLOCK offset 1024 > card 2048 [%2048]
Would it be any better as:
"%s offset+length %lu+%lu > card size %lu\n"
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
2021-06-23 18:00 ` [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is Philippe Mathieu-Daudé
2021-06-23 18:00 ` [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-07-03 8:41 ` Philippe Mathieu-Daudé
` (2 more replies)
2021-06-23 18:00 ` [PATCH 4/9] tests/acceptance: Extract image_expand() helper Philippe Mathieu-Daudé
` (6 subsequent siblings)
9 siblings, 3 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
Avocado allows us to select set of tests using tags.
When wanting to run all tests using a NetBSD guest OS,
it is convenient to have them tagged, add the 'os:netbsd'
tag.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 1 +
tests/acceptance/ppc_prep_40p.py | 2 ++
2 files changed, 3 insertions(+)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index cded547d1d4..20d57c1a8c6 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -862,6 +862,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
:avocado: tags=device:sd
+ :avocado: tags=os:netbsd
"""
# This test download a 304MB compressed image and expand it to 2GB
deb_url = ('http://snapshot.debian.org/archive/debian/'
diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index 96ba13b8943..2993ee3b078 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -27,6 +27,7 @@ def test_factory_firmware_and_netbsd(self):
"""
:avocado: tags=arch:ppc
:avocado: tags=machine:40p
+ :avocado: tags=os:netbsd
:avocado: tags=slowness:high
"""
bios_url = ('http://ftpmirror.your.org/pub/misc/'
@@ -64,6 +65,7 @@ def test_openbios_and_netbsd(self):
"""
:avocado: tags=arch:ppc
:avocado: tags=machine:40p
+ :avocado: tags=os:netbsd
"""
drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
'NetBSD-7.1.2-prep.iso')
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-06-23 18:00 ` [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd' Philippe Mathieu-Daudé
@ 2021-07-03 8:41 ` Philippe Mathieu-Daudé
2021-07-03 8:43 ` Philippe Mathieu-Daudé
2021-07-05 15:25 ` Willian Rampazzo
2021-07-13 16:57 ` Cleber Rosa
2 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-03 8:41 UTC (permalink / raw)
To: qemu-devel
Cc: Ryo ONODERA, Daniel P . Berrangé,
qemu-block, Bin Meng, Kamil Rytarowski, Tom Yan,
Alexander Bulekov, Niek Linnenbank, Reinoud Zandijk,
Michal Suchánek, Warner Losh
CC'ing NetBSD maintainers.
On 6/23/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> Avocado allows us to select set of tests using tags.
> When wanting to run all tests using a NetBSD guest OS,
> it is convenient to have them tagged, add the 'os:netbsd'
> tag.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 1 +
> tests/acceptance/ppc_prep_40p.py | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index cded547d1d4..20d57c1a8c6 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -862,6 +862,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
> :avocado: tags=arch:arm
> :avocado: tags=machine:orangepi-pc
> :avocado: tags=device:sd
> + :avocado: tags=os:netbsd
> """
> # This test download a 304MB compressed image and expand it to 2GB
> deb_url = ('http://snapshot.debian.org/archive/debian/'
> diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
> index 96ba13b8943..2993ee3b078 100644
> --- a/tests/acceptance/ppc_prep_40p.py
> +++ b/tests/acceptance/ppc_prep_40p.py
> @@ -27,6 +27,7 @@ def test_factory_firmware_and_netbsd(self):
> """
> :avocado: tags=arch:ppc
> :avocado: tags=machine:40p
> + :avocado: tags=os:netbsd
> :avocado: tags=slowness:high
> """
> bios_url = ('http://ftpmirror.your.org/pub/misc/'
> @@ -64,6 +65,7 @@ def test_openbios_and_netbsd(self):
> """
> :avocado: tags=arch:ppc
> :avocado: tags=machine:40p
> + :avocado: tags=os:netbsd
> """
> drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
> 'NetBSD-7.1.2-prep.iso')
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-07-03 8:41 ` Philippe Mathieu-Daudé
@ 2021-07-03 8:43 ` Philippe Mathieu-Daudé
2021-07-04 12:35 ` Niek Linnenbank
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-03 8:43 UTC (permalink / raw)
To: qemu-devel@nongnu.org Developers
Cc: Ryo ONODERA, Daniel P . Berrangé,
open list:Block layer core, Bin Meng, Kamil Rytarowski, Tom Yan,
Alexander Bulekov, Niek Linnenbank, Reinoud Zandijk,
Michal Suchánek, Warner Losh
On Sat, Jul 3, 2021 at 10:41 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> CC'ing NetBSD maintainers.
>
> On 6/23/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> > Avocado allows us to select set of tests using tags.
> > When wanting to run all tests using a NetBSD guest OS,
> > it is convenient to have them tagged, add the 'os:netbsd'
> > tag.
I'll amend an command line example to run the NetBSD tests:
$ avocado --show=app,console run -t os:netbsd tests/acceptance/
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > tests/acceptance/boot_linux_console.py | 1 +
> > tests/acceptance/ppc_prep_40p.py | 2 ++
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> > index cded547d1d4..20d57c1a8c6 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -862,6 +862,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
> > :avocado: tags=arch:arm
> > :avocado: tags=machine:orangepi-pc
> > :avocado: tags=device:sd
> > + :avocado: tags=os:netbsd
> > """
> > # This test download a 304MB compressed image and expand it to 2GB
> > deb_url = ('http://snapshot.debian.org/archive/debian/'
> > diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
> > index 96ba13b8943..2993ee3b078 100644
> > --- a/tests/acceptance/ppc_prep_40p.py
> > +++ b/tests/acceptance/ppc_prep_40p.py
> > @@ -27,6 +27,7 @@ def test_factory_firmware_and_netbsd(self):
> > """
> > :avocado: tags=arch:ppc
> > :avocado: tags=machine:40p
> > + :avocado: tags=os:netbsd
> > :avocado: tags=slowness:high
> > """
> > bios_url = ('http://ftpmirror.your.org/pub/misc/'
> > @@ -64,6 +65,7 @@ def test_openbios_and_netbsd(self):
> > """
> > :avocado: tags=arch:ppc
> > :avocado: tags=machine:40p
> > + :avocado: tags=os:netbsd
> > """
> > drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
> > 'NetBSD-7.1.2-prep.iso')
> >
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-07-03 8:43 ` Philippe Mathieu-Daudé
@ 2021-07-04 12:35 ` Niek Linnenbank
2021-07-05 8:55 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 34+ messages in thread
From: Niek Linnenbank @ 2021-07-04 12:35 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Ryo ONODERA, Daniel P . Berrangé,
open list:Block layer core, Bin Meng,
qemu-devel@nongnu.org Developers, Tom Yan, Alexander Bulekov,
Kamil Rytarowski, Reinoud Zandijk, Michal Suchánek,
Warner Losh
[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]
for test_arm_orangepi_uboot_netbsd9:
Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Op za 3 jul. 2021 10:44 schreef Philippe Mathieu-Daudé <f4bug@amsat.org>:
> On Sat, Jul 3, 2021 at 10:41 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
> >
> > CC'ing NetBSD maintainers.
> >
> > On 6/23/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> > > Avocado allows us to select set of tests using tags.
> > > When wanting to run all tests using a NetBSD guest OS,
> > > it is convenient to have them tagged, add the 'os:netbsd'
> > > tag.
>
> I'll amend an command line example to run the NetBSD tests:
>
> $ avocado --show=app,console run -t os:netbsd tests/acceptance/
>
> > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > > tests/acceptance/boot_linux_console.py | 1 +
> > > tests/acceptance/ppc_prep_40p.py | 2 ++
> > > 2 files changed, 3 insertions(+)
> > >
> > > diff --git a/tests/acceptance/boot_linux_console.py
> b/tests/acceptance/boot_linux_console.py
> > > index cded547d1d4..20d57c1a8c6 100644
> > > --- a/tests/acceptance/boot_linux_console.py
> > > +++ b/tests/acceptance/boot_linux_console.py
> > > @@ -862,6 +862,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
> > > :avocado: tags=arch:arm
> > > :avocado: tags=machine:orangepi-pc
> > > :avocado: tags=device:sd
> > > + :avocado: tags=os:netbsd
> > > """
> > > # This test download a 304MB compressed image and expand it
> to 2GB
> > > deb_url = ('http://snapshot.debian.org/archive/debian/'
> > > diff --git a/tests/acceptance/ppc_prep_40p.py
> b/tests/acceptance/ppc_prep_40p.py
> > > index 96ba13b8943..2993ee3b078 100644
> > > --- a/tests/acceptance/ppc_prep_40p.py
> > > +++ b/tests/acceptance/ppc_prep_40p.py
> > > @@ -27,6 +27,7 @@ def test_factory_firmware_and_netbsd(self):
> > > """
> > > :avocado: tags=arch:ppc
> > > :avocado: tags=machine:40p
> > > + :avocado: tags=os:netbsd
> > > :avocado: tags=slowness:high
> > > """
> > > bios_url = ('http://ftpmirror.your.org/pub/misc/'
> > > @@ -64,6 +65,7 @@ def test_openbios_and_netbsd(self):
> > > """
> > > :avocado: tags=arch:ppc
> > > :avocado: tags=machine:40p
> > > + :avocado: tags=os:netbsd
> > > """
> > > drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
> > > 'NetBSD-7.1.2-prep.iso')
> > >
> >
>
[-- Attachment #2: Type: text/html, Size: 4134 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-07-04 12:35 ` Niek Linnenbank
@ 2021-07-05 8:55 ` Philippe Mathieu-Daudé
2021-07-11 21:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-05 8:55 UTC (permalink / raw)
To: Niek Linnenbank
Cc: Michal Suchánek, Daniel P . Berrangé,
open list:Block layer core, Bin Meng,
qemu-devel@nongnu.org Developers, Tom Yan, Alexander Bulekov,
Kamil Rytarowski, Reinoud Zandijk, Ryo ONODERA, Warner Losh
Hi Niek,
On 7/4/21 2:35 PM, Niek Linnenbank wrote:
> for test_arm_orangepi_uboot_netbsd9:
>
> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com
> <mailto:nieklinnenbank@gmail.com>>
Thanks for the review. Does your R-b tag applies for this single
patch or all patches related to test_arm_orangepi_uboot_netbsd9
in this series (3-5)?
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-07-05 8:55 ` Philippe Mathieu-Daudé
@ 2021-07-11 21:05 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-11 21:05 UTC (permalink / raw)
To: Niek Linnenbank
Cc: Ryo ONODERA, Daniel P . Berrangé,
open list:Block layer core, Bin Meng,
qemu-devel@nongnu.org Developers, Tom Yan, Alexander Bulekov,
Kamil Rytarowski, Reinoud Zandijk, Michal Suchánek,
Warner Losh
On 7/5/21 10:55 AM, Philippe Mathieu-Daudé wrote:
> Hi Niek,
>
> On 7/4/21 2:35 PM, Niek Linnenbank wrote:
>> for test_arm_orangepi_uboot_netbsd9:
>>
>> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com
>> <mailto:nieklinnenbank@gmail.com>>
>
> Thanks for the review. Does your R-b tag applies for this single
> patch or all patches related to test_arm_orangepi_uboot_netbsd9
> in this series (3-5)?
Soft-freeze is in 2 days and no review from the NetBSD team,
so postponing these patches to v6.2.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-06-23 18:00 ` [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd' Philippe Mathieu-Daudé
2021-07-03 8:41 ` Philippe Mathieu-Daudé
@ 2021-07-05 15:25 ` Willian Rampazzo
2021-07-13 16:57 ` Cleber Rosa
2 siblings, 0 replies; 34+ messages in thread
From: Willian Rampazzo @ 2021-07-05 15:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 3:03 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Avocado allows us to select set of tests using tags.
> When wanting to run all tests using a NetBSD guest OS,
> it is convenient to have them tagged, add the 'os:netbsd'
> tag.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 1 +
> tests/acceptance/ppc_prep_40p.py | 2 ++
> 2 files changed, 3 insertions(+)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd'
2021-06-23 18:00 ` [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd' Philippe Mathieu-Daudé
2021-07-03 8:41 ` Philippe Mathieu-Daudé
2021-07-05 15:25 ` Willian Rampazzo
@ 2021-07-13 16:57 ` Cleber Rosa
2 siblings, 0 replies; 34+ messages in thread
From: Cleber Rosa @ 2021-07-13 16:57 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
Philippe Mathieu-Daudé writes:
> Avocado allows us to select set of tests using tags.
> When wanting to run all tests using a NetBSD guest OS,
> it is convenient to have them tagged, add the 'os:netbsd'
> tag.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 1 +
> tests/acceptance/ppc_prep_40p.py | 2 ++
> 2 files changed, 3 insertions(+)
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Phil,
I can ammend the commit message and queue this one if you think it's a
good idea.
Cheers,
- Cleber.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 4/9] tests/acceptance: Extract image_expand() helper
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-06-23 18:00 ` [PATCH 3/9] tests/acceptance: Tag NetBSD tests as 'os:netbsd' Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-07-05 15:27 ` Willian Rampazzo
2021-07-13 17:02 ` Cleber Rosa
2021-06-23 18:00 ` [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9 Philippe Mathieu-Daudé
` (5 subsequent siblings)
9 siblings, 2 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
To be able to expand an image to a non-power-of-2 value,
extract image_expand() from image_pow2ceil_expand().
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 20d57c1a8c6..61069f0064f 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -35,15 +35,19 @@
def pow2ceil(x):
return 1 if x == 0 else 2**(x - 1).bit_length()
+"""
+Expand file size
+"""
+def image_expand(path, size):
+ if size != os.path.getsize(path):
+ with open(path, 'ab+') as fd:
+ fd.truncate(size)
+
"""
Expand file size to next power of 2
"""
def image_pow2ceil_expand(path):
- size = os.path.getsize(path)
- size_aligned = pow2ceil(size)
- if size != size_aligned:
- with open(path, 'ab+') as fd:
- fd.truncate(size_aligned)
+ image_expand(path, pow2ceil(os.path.getsize(path)))
class LinuxKernelTest(Test):
KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 4/9] tests/acceptance: Extract image_expand() helper
2021-06-23 18:00 ` [PATCH 4/9] tests/acceptance: Extract image_expand() helper Philippe Mathieu-Daudé
@ 2021-07-05 15:27 ` Willian Rampazzo
2021-07-13 17:02 ` Cleber Rosa
1 sibling, 0 replies; 34+ messages in thread
From: Willian Rampazzo @ 2021-07-05 15:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 3:06 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> To be able to expand an image to a non-power-of-2 value,
> extract image_expand() from image_pow2ceil_expand().
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/9] tests/acceptance: Extract image_expand() helper
2021-06-23 18:00 ` [PATCH 4/9] tests/acceptance: Extract image_expand() helper Philippe Mathieu-Daudé
2021-07-05 15:27 ` Willian Rampazzo
@ 2021-07-13 17:02 ` Cleber Rosa
1 sibling, 0 replies; 34+ messages in thread
From: Cleber Rosa @ 2021-07-13 17:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
Philippe Mathieu-Daudé writes:
> To be able to expand an image to a non-power-of-2 value,
> extract image_expand() from image_pow2ceil_expand().
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 20d57c1a8c6..61069f0064f 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -35,15 +35,19 @@
> def pow2ceil(x):
> return 1 if x == 0 else 2**(x - 1).bit_length()
>
> +"""
> +Expand file size
> +"""
> +def image_expand(path, size):
> + if size != os.path.getsize(path):
> + with open(path, 'ab+') as fd:
> + fd.truncate(size)
> +
This would be a good time to make the comment section, into a proper
docstring, that is:
def image_expand(path, size):
"""Expand file size"""
if size != os.path.getsize(path):
with open(path, 'ab+') as fd:
fd.truncate(size)
- Cleber.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-06-23 18:00 ` [PATCH 4/9] tests/acceptance: Extract image_expand() helper Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-07-05 15:28 ` Willian Rampazzo
2021-08-04 20:54 ` Philippe Mathieu-Daudé
2021-06-23 18:00 ` [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08 Philippe Mathieu-Daudé
` (4 subsequent siblings)
9 siblings, 2 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
The NetBSD OrangePi image must be at least 2GiB, not less.
Expand the SD card image to this size before using it.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 61069f0064f..b10f7257503 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -868,7 +868,12 @@ def test_arm_orangepi_uboot_netbsd9(self):
:avocado: tags=device:sd
:avocado: tags=os:netbsd
"""
- # This test download a 304MB compressed image and expand it to 2GB
+ # This test download a 304MB compressed image and expand it to 2GB,
+ # which is the minimum card size required by the NetBSD installer:
+ # https://wiki.netbsd.org/ports/evbarm/raspberry_pi/#index7h2
+ # "A 2 GB card is the smallest workable size that the installation
+ # image will fit on."
+ NETBSD_SDCARD_MINSIZE = 2 * 1024 * 1024 * 1024
deb_url = ('http://snapshot.debian.org/archive/debian/'
'20200108T145233Z/pool/main/u/u-boot/'
'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
@@ -886,7 +891,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
image_path = os.path.join(self.workdir, 'armv7.img')
archive.gzip_uncompress(image_path_gz, image_path)
- image_pow2ceil_expand(image_path)
+ image_expand(image_path, NETBSD_SDCARD_MINSIZE)
image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
# dd if=u-boot-sunxi-with-spl.bin of=armv7.img bs=1K seek=8 conv=notrunc
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9
2021-06-23 18:00 ` [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9 Philippe Mathieu-Daudé
@ 2021-07-05 15:28 ` Willian Rampazzo
2021-08-04 20:54 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 34+ messages in thread
From: Willian Rampazzo @ 2021-07-05 15:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 3:08 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The NetBSD OrangePi image must be at least 2GiB, not less.
> Expand the SD card image to this size before using it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9
2021-06-23 18:00 ` [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9 Philippe Mathieu-Daudé
2021-07-05 15:28 ` Willian Rampazzo
@ 2021-08-04 20:54 ` Philippe Mathieu-Daudé
2021-08-05 17:06 ` Niek Linnenbank
1 sibling, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-04 20:54 UTC (permalink / raw)
To: qemu-devel, Niek Linnenbank
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Tom Yan, Alexander Bulekov,
Michal Suchánek, Warner Losh
Hi Niek,
On 6/23/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> The NetBSD OrangePi image must be at least 2GiB, not less.
> Expand the SD card image to this size before using it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 61069f0064f..b10f7257503 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -868,7 +868,12 @@ def test_arm_orangepi_uboot_netbsd9(self):
> :avocado: tags=device:sd
> :avocado: tags=os:netbsd
> """
> - # This test download a 304MB compressed image and expand it to 2GB
> + # This test download a 304MB compressed image and expand it to 2GB,
> + # which is the minimum card size required by the NetBSD installer:
> + # https://wiki.netbsd.org/ports/evbarm/raspberry_pi/#index7h2
> + # "A 2 GB card is the smallest workable size that the installation
> + # image will fit on."
Do you agree with this comment and the one in the next patch?
> + NETBSD_SDCARD_MINSIZE = 2 * 1024 * 1024 * 1024
> deb_url = ('http://snapshot.debian.org/archive/debian/'
> '20200108T145233Z/pool/main/u/u-boot/'
> 'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
> @@ -886,7 +891,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
> image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
> image_path = os.path.join(self.workdir, 'armv7.img')
> archive.gzip_uncompress(image_path_gz, image_path)
> - image_pow2ceil_expand(image_path)
> + image_expand(image_path, NETBSD_SDCARD_MINSIZE)
> image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
>
> # dd if=u-boot-sunxi-with-spl.bin of=armv7.img bs=1K seek=8 conv=notrunc
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9
2021-08-04 20:54 ` Philippe Mathieu-Daudé
@ 2021-08-05 17:06 ` Niek Linnenbank
0 siblings, 0 replies; 34+ messages in thread
From: Niek Linnenbank @ 2021-08-05 17:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
Qemu-block, Bin Meng, QEMU Developers, Tom Yan,
Alexander Bulekov, Michal Suchánek, Warner Losh
[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]
Hi Philippe,
Op wo 4 aug. 2021 22:55 schreef Philippe Mathieu-Daudé <f4bug@amsat.org>:
> Hi Niek,
>
> On 6/23/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> > The NetBSD OrangePi image must be at least 2GiB, not less.
> > Expand the SD card image to this size before using it.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > tests/acceptance/boot_linux_console.py | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/acceptance/boot_linux_console.py
> b/tests/acceptance/boot_linux_console.py
> > index 61069f0064f..b10f7257503 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -868,7 +868,12 @@ def test_arm_orangepi_uboot_netbsd9(self):
> > :avocado: tags=device:sd
> > :avocado: tags=os:netbsd
> > """
> > - # This test download a 304MB compressed image and expand it to
> 2GB
> > + # This test download a 304MB compressed image and expand it to
> 2GB,
> > + # which is the minimum card size required by the NetBSD
> installer:
> > + # https://wiki.netbsd.org/ports/evbarm/raspberry_pi/#index7h2
> > + # "A 2 GB card is the smallest workable size that the
> installation
> > + # image will fit on."
>
> Do you agree with this comment and the one in the next patch?
>
Yes, this change looks fine for me.
Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> > + NETBSD_SDCARD_MINSIZE = 2 * 1024 * 1024 * 1024
> > deb_url = ('http://snapshot.debian.org/archive/debian/'
> > '20200108T145233Z/pool/main/u/u-boot/'
> > 'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
> > @@ -886,7 +891,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
> > image_path_gz = self.fetch_asset(image_url,
> asset_hash=image_hash)
> > image_path = os.path.join(self.workdir, 'armv7.img')
> > archive.gzip_uncompress(image_path_gz, image_path)
> > - image_pow2ceil_expand(image_path)
> > + image_expand(image_path, NETBSD_SDCARD_MINSIZE)
> > image_drive_args = 'if=sd,format=raw,snapshot=on,file=' +
> image_path
> >
> > # dd if=u-boot-sunxi-with-spl.bin of=armv7.img bs=1K seek=8
> conv=notrunc
> >
>
>
[-- Attachment #2: Type: text/html, Size: 3601 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-06-23 18:00 ` [PATCH 5/9] tests/acceptance: Use image_expand() in test_arm_orangepi_uboot_netbsd9 Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-07-05 15:30 ` Willian Rampazzo
2021-08-05 17:11 ` Niek Linnenbank
2021-06-23 18:00 ` [RFC PATCH 7/9] tests/acceptance: Do not expand SD card image in test_arm_orangepi_sd Philippe Mathieu-Daudé
` (3 subsequent siblings)
9 siblings, 2 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
U-Boot expects the SD card size to be at least 2GiB, so
expand the SD card image to this size before using it.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
TODO: U-Boot reference?
---
tests/acceptance/boot_linux_console.py | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index b10f7257503..c4c0f0b393d 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -820,11 +820,13 @@ def test_arm_orangepi_bionic_20_08(self):
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
:avocado: tags=device:sd
+ :avocado: tags=u-boot
"""
- # This test download a 275 MiB compressed image and expand it
- # to 1036 MiB, but the underlying filesystem is 1552 MiB...
- # As we expand it to 2 GiB we are safe.
+ # This test download a 275 MiB compressed image and inflate it
+ # to 1036 MiB, but 1/ the underlying filesystem is 1552 MiB,
+ # 2/ U-Boot loads TFTP filenames from the last sector below
+ # 2 GiB, so we need to expand the image further more to 2 GiB.
image_url = ('https://archive.armbian.com/orangepipc/archive/'
'Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz')
@@ -833,7 +835,7 @@ def test_arm_orangepi_bionic_20_08(self):
image_path_xz = self.fetch_asset(image_url, asset_hash=image_hash,
algorithm='sha256')
image_path = archive.extract(image_path_xz, self.workdir)
- image_pow2ceil_expand(image_path)
+ image_expand(image_path, 2 * 1024 * 1024 * 1024)
self.vm.set_console()
self.vm.add_args('-drive', 'file=' + image_path + ',if=sd,format=raw',
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08
2021-06-23 18:00 ` [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08 Philippe Mathieu-Daudé
@ 2021-07-05 15:30 ` Willian Rampazzo
2021-08-05 17:11 ` Niek Linnenbank
1 sibling, 0 replies; 34+ messages in thread
From: Willian Rampazzo @ 2021-07-05 15:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 3:09 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> U-Boot expects the SD card size to be at least 2GiB, so
> expand the SD card image to this size before using it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> TODO: U-Boot reference?
> ---
> tests/acceptance/boot_linux_console.py | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08
2021-06-23 18:00 ` [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08 Philippe Mathieu-Daudé
2021-07-05 15:30 ` Willian Rampazzo
@ 2021-08-05 17:11 ` Niek Linnenbank
1 sibling, 0 replies; 34+ messages in thread
From: Niek Linnenbank @ 2021-08-05 17:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
Qemu-block, Bin Meng, QEMU Developers, Tom Yan,
Alexander Bulekov, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]
Hi Philippe,
Op wo 23 jun. 2021 20:00 schreef Philippe Mathieu-Daudé <f4bug@amsat.org>:
> U-Boot expects the SD card size to be at least 2GiB, so
> expand the SD card image to this size before using it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> TODO: U-Boot reference?
> ---
> tests/acceptance/boot_linux_console.py | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py
> b/tests/acceptance/boot_linux_console.py
> index b10f7257503..c4c0f0b393d 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -820,11 +820,13 @@ def test_arm_orangepi_bionic_20_08(self):
> :avocado: tags=arch:arm
> :avocado: tags=machine:orangepi-pc
> :avocado: tags=device:sd
> + :avocado: tags=u-boot
> """
>
> - # This test download a 275 MiB compressed image and expand it
> - # to 1036 MiB, but the underlying filesystem is 1552 MiB...
> - # As we expand it to 2 GiB we are safe.
> + # This test download a 275 MiB compressed image and inflate it
>
Only a few typos here:
download -> downloads
inflate -> inflates
Otherwise, looks fine:
Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
+ # to 1036 MiB, but 1/ the underlying filesystem is 1552 MiB,
> + # 2/ U-Boot loads TFTP filenames from the last sector below
> + # 2 GiB, so we need to expand the image further more to 2 GiB.
>
> image_url = ('https://archive.armbian.com/orangepipc/archive/'
>
> 'Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz')
> @@ -833,7 +835,7 @@ def test_arm_orangepi_bionic_20_08(self):
> image_path_xz = self.fetch_asset(image_url, asset_hash=image_hash,
> algorithm='sha256')
> image_path = archive.extract(image_path_xz, self.workdir)
> - image_pow2ceil_expand(image_path)
> + image_expand(image_path, 2 * 1024 * 1024 * 1024)
>
> self.vm.set_console()
> self.vm.add_args('-drive', 'file=' + image_path +
> ',if=sd,format=raw',
> --
> 2.31.1
>
>
[-- Attachment #2: Type: text/html, Size: 3392 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [RFC PATCH 7/9] tests/acceptance: Do not expand SD card image in test_arm_orangepi_sd
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-06-23 18:00 ` [RFC PATCH 6/9] tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08 Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-06-23 18:00 ` [PATCH 8/9] tests/acceptance: Remove now unused pow2ceil() Philippe Mathieu-Daudé
` (2 subsequent siblings)
9 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
XXX it seems to work...
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index c4c0f0b393d..48c0ba09117 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -783,7 +783,6 @@ def test_arm_orangepi_sd(self):
rootfs_path_xz = self.fetch_asset(rootfs_url, asset_hash=rootfs_hash)
rootfs_path = os.path.join(self.workdir, 'rootfs.cpio')
archive.lzma_uncompress(rootfs_path_xz, rootfs_path)
- image_pow2ceil_expand(rootfs_path)
self.vm.set_console()
kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 8/9] tests/acceptance: Remove now unused pow2ceil()
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-06-23 18:00 ` [RFC PATCH 7/9] tests/acceptance: Do not expand SD card image in test_arm_orangepi_sd Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-07-05 15:32 ` Willian Rampazzo
2021-06-23 18:00 ` [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
2021-06-24 2:50 ` [RFC PATCH 0/9] " Alexander Bulekov
9 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
We don't use pow2ceil() anymore, remove it.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 48c0ba09117..77bc80c505d 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -29,12 +29,6 @@
except CmdNotFoundError:
P7ZIP_AVAILABLE = False
-"""
-Round up to next power of 2
-"""
-def pow2ceil(x):
- return 1 if x == 0 else 2**(x - 1).bit_length()
-
"""
Expand file size
"""
@@ -43,12 +37,6 @@ def image_expand(path, size):
with open(path, 'ab+') as fd:
fd.truncate(size)
-"""
-Expand file size to next power of 2
-"""
-def image_pow2ceil_expand(path):
- image_expand(path, pow2ceil(os.path.getsize(path)))
-
class LinuxKernelTest(Test):
KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 8/9] tests/acceptance: Remove now unused pow2ceil()
2021-06-23 18:00 ` [PATCH 8/9] tests/acceptance: Remove now unused pow2ceil() Philippe Mathieu-Daudé
@ 2021-07-05 15:32 ` Willian Rampazzo
0 siblings, 0 replies; 34+ messages in thread
From: Willian Rampazzo @ 2021-07-05 15:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 3:10 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> We don't use pow2ceil() anymore, remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/acceptance/boot_linux_console.py | 12 ------------
> 1 file changed, 12 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-06-23 18:00 ` [PATCH 8/9] tests/acceptance: Remove now unused pow2ceil() Philippe Mathieu-Daudé
@ 2021-06-23 18:00 ` Philippe Mathieu-Daudé
2021-06-24 10:17 ` Daniel P. Berrangé
2021-06-24 10:24 ` Tom Yan
2021-06-24 2:50 ` [RFC PATCH 0/9] " Alexander Bulekov
9 siblings, 2 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-23 18:00 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, Philippe Mathieu-Daudé,
Tom Yan, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
In commit a9bcedd15a5 ("hw/sd/sdcard: Do not allow invalid SD card
sizes") we tried to protect us from CVE-2020-13253 by only allowing
card with power-of-2 sizes. However doing so we disrupted valid user
cases. As a compromise, allow any card size, but warn only power of 2
sizes are supported, still suggesting the user how to increase a
card with 'qemu-img resize'.
Cc: Tom Yan <tom.ty89@gmail.com>
Cc: Warner Losh <imp@bsdimp.com>
Buglink: https://bugs.launchpad.net/qemu/+bug/1910586
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9c8dd11bad1..cab4aab1475 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2131,23 +2131,16 @@ static void sd_realize(DeviceState *dev, Error **errp)
blk_size = blk_getlength(sd->blk);
if (blk_size > 0 && !is_power_of_2(blk_size)) {
int64_t blk_size_aligned = pow2ceil(blk_size);
- char *blk_size_str;
+ g_autofree char *blk_size_s = size_to_str(blk_size);
+ g_autofree char *blk_size_aligned_s = size_to_str(blk_size_aligned);
- blk_size_str = size_to_str(blk_size);
- error_setg(errp, "Invalid SD card size: %s", blk_size_str);
- g_free(blk_size_str);
-
- blk_size_str = size_to_str(blk_size_aligned);
- error_append_hint(errp,
- "SD card size has to be a power of 2, e.g. %s.\n"
- "You can resize disk images with"
- " 'qemu-img resize <imagefile> <new-size>'\n"
- "(note that this will lose data if you make the"
- " image smaller than it currently is).\n",
- blk_size_str);
- g_free(blk_size_str);
-
- return;
+ warn_report("SD card size is not a power of 2 (%s). It might work"
+ " but is not supported by QEMU. If possible, resize"
+ " your disk image to %s with:",
+ blk_size_s, blk_size_aligned_s);
+ warn_report(" 'qemu-img resize <imagefile> <new-size>'");
+ warn_report("(note that this will lose data if you make the"
+ " image smaller than it currently is).");
}
ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
--
2.31.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again
2021-06-23 18:00 ` [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
@ 2021-06-24 10:17 ` Daniel P. Berrangé
2021-06-24 10:24 ` Tom Yan
1 sibling, 0 replies; 34+ messages in thread
From: Daniel P. Berrangé @ 2021-06-24 10:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-block, Bin Meng, qemu-devel, Tom Yan, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
On Wed, Jun 23, 2021 at 08:00:21PM +0200, Philippe Mathieu-Daudé wrote:
> In commit a9bcedd15a5 ("hw/sd/sdcard: Do not allow invalid SD card
> sizes") we tried to protect us from CVE-2020-13253 by only allowing
> card with power-of-2 sizes. However doing so we disrupted valid user
> cases. As a compromise, allow any card size, but warn only power of 2
> sizes are supported, still suggesting the user how to increase a
> card with 'qemu-img resize'.
>
> Cc: Tom Yan <tom.ty89@gmail.com>
> Cc: Warner Losh <imp@bsdimp.com>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1910586
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/sd/sd.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 9c8dd11bad1..cab4aab1475 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2131,23 +2131,16 @@ static void sd_realize(DeviceState *dev, Error **errp)
> blk_size = blk_getlength(sd->blk);
> if (blk_size > 0 && !is_power_of_2(blk_size)) {
> int64_t blk_size_aligned = pow2ceil(blk_size);
> - char *blk_size_str;
> + g_autofree char *blk_size_s = size_to_str(blk_size);
> + g_autofree char *blk_size_aligned_s = size_to_str(blk_size_aligned);
>
> - blk_size_str = size_to_str(blk_size);
> - error_setg(errp, "Invalid SD card size: %s", blk_size_str);
> - g_free(blk_size_str);
> -
> - blk_size_str = size_to_str(blk_size_aligned);
> - error_append_hint(errp,
> - "SD card size has to be a power of 2, e.g. %s.\n"
> - "You can resize disk images with"
> - " 'qemu-img resize <imagefile> <new-size>'\n"
> - "(note that this will lose data if you make the"
> - " image smaller than it currently is).\n",
> - blk_size_str);
> - g_free(blk_size_str);
> -
> - return;
> + warn_report("SD card size is not a power of 2 (%s). It might work"
> + " but is not supported by QEMU. If possible, resize"
> + " your disk image to %s with:",
> + blk_size_s, blk_size_aligned_s);
> + warn_report(" 'qemu-img resize <imagefile> <new-size>'");
> + warn_report("(note that this will lose data if you make the"
> + " image smaller than it currently is).");
In what scenarios will non-power of 2 not work and what is the effect ?
Is it a QEMU bug or not ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again
2021-06-23 18:00 ` [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
2021-06-24 10:17 ` Daniel P. Berrangé
@ 2021-06-24 10:24 ` Tom Yan
2021-06-24 10:56 ` Peter Maydell
1 sibling, 1 reply; 34+ messages in thread
From: Tom Yan @ 2021-06-24 10:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Alexander Bulekov,
Niek Linnenbank, Michal Suchánek,
Philippe Mathieu-Daudé,
Warner Losh
Hi,
On Thu, 24 Jun 2021 at 02:01, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> In commit a9bcedd15a5 ("hw/sd/sdcard: Do not allow invalid SD card
> sizes") we tried to protect us from CVE-2020-13253 by only allowing
> card with power-of-2 sizes. However doing so we disrupted valid user
> cases. As a compromise, allow any card size, but warn only power of 2
> sizes are supported, still suggesting the user how to increase a
> card with 'qemu-img resize'.
>
> Cc: Tom Yan <tom.ty89@gmail.com>
> Cc: Warner Losh <imp@bsdimp.com>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1910586
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/sd/sd.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 9c8dd11bad1..cab4aab1475 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2131,23 +2131,16 @@ static void sd_realize(DeviceState *dev, Error **errp)
> blk_size = blk_getlength(sd->blk);
> if (blk_size > 0 && !is_power_of_2(blk_size)) {
> int64_t blk_size_aligned = pow2ceil(blk_size);
> - char *blk_size_str;
> + g_autofree char *blk_size_s = size_to_str(blk_size);
> + g_autofree char *blk_size_aligned_s = size_to_str(blk_size_aligned);
>
> - blk_size_str = size_to_str(blk_size);
> - error_setg(errp, "Invalid SD card size: %s", blk_size_str);
> - g_free(blk_size_str);
> -
> - blk_size_str = size_to_str(blk_size_aligned);
> - error_append_hint(errp,
> - "SD card size has to be a power of 2, e.g. %s.\n"
> - "You can resize disk images with"
> - " 'qemu-img resize <imagefile> <new-size>'\n"
> - "(note that this will lose data if you make the"
> - " image smaller than it currently is).\n",
> - blk_size_str);
> - g_free(blk_size_str);
> -
> - return;
> + warn_report("SD card size is not a power of 2 (%s). It might work"
> + " but is not supported by QEMU. If possible, resize"
> + " your disk image to %s with:",
> + blk_size_s, blk_size_aligned_s);
> + warn_report(" 'qemu-img resize <imagefile> <new-size>'");
> + warn_report("(note that this will lose data if you make the"
> + " image smaller than it currently is).");
Not trying to be picky, but I don't think this is much better. IMHO
it's quite irresponsible to give a warning like that, leaving users in
a state like "Should I use it or not then?", without giving a concrete
reference to what exactly might/would lead to the warned problem.
I really think we should get (/ have gotten) things clear first. What
exactly is the bug we have been talking about here? I mean like, where
does it occur and what's the nature of it.
1. Is it specific to a certain type / model of backend / physical
storage device that will be made use of by qemu for the emulated
storage? (I presume not since you mention about image, unless you
irrationally related/bound the emulated storage type and the physical
storage type together.)
2. Does it have anything to do with a certain flaw in qemu itself?
Like the code that does read/write operation is flawed that it cannot
be handled by a certain *proper* backend device?
3. Or is it actually a bug in a certain driver / firmware blob that
will be used by an *emulated* device in the guest? In that case, can
we emulate another model so that it won't be using the problematic
driver / firmware?
Also, could you provide any information / reference to what the bug
really is? Like a bug report (for the problem itself, not some vague
claim that qemu should workaround the problem)?
> }
>
> ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again
2021-06-24 10:24 ` Tom Yan
@ 2021-06-24 10:56 ` Peter Maydell
2021-06-24 16:08 ` Warner Losh
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2021-06-24 10:56 UTC (permalink / raw)
To: Tom Yan
Cc: Daniel P . Berrangé,
Qemu-block, Bin Meng, Philippe Mathieu-Daudé,
QEMU Developers, Alexander Bulekov, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
On Thu, 24 Jun 2021 at 11:27, Tom Yan <tom.ty89@gmail.com> wrote:
> I really think we should get (/ have gotten) things clear first. What
> exactly is the bug we have been talking about here? I mean like, where
> does it occur and what's the nature of it.
>
> 1. Is it specific to a certain type / model of backend / physical
> storage device that will be made use of by qemu for the emulated
> storage? (I presume not since you mention about image, unless you
> irrationally related/bound the emulated storage type and the physical
> storage type together.)
>
> 2. Does it have anything to do with a certain flaw in qemu itself?
> Like the code that does read/write operation is flawed that it cannot
> be handled by a certain *proper* backend device?
>
> 3. Or is it actually a bug in a certain driver / firmware blob that
> will be used by an *emulated* device in the guest? In that case, can
> we emulate another model so that it won't be using the problematic
> driver / firmware?
Definitely agreed -- before we start changing QEMU code we need
to identify clearly (a) what the real hardware does and (b) what
the situation was we were originally trying to fix.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again
2021-06-24 10:56 ` Peter Maydell
@ 2021-06-24 16:08 ` Warner Losh
0 siblings, 0 replies; 34+ messages in thread
From: Warner Losh @ 2021-06-24 16:08 UTC (permalink / raw)
To: Peter Maydell
Cc: Michal Suchánek, "Daniel P . Berrangé",
Qemu-block, Bin Meng, QEMU Developers,
Philippe Mathieu-Daudé,
Alexander Bulekov, Niek Linnenbank, Tom Yan,
Philippe Mathieu-Daudé,
Warner Losh
[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]
> On Jun 24, 2021, at 4:56 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 24 Jun 2021 at 11:27, Tom Yan <tom.ty89@gmail.com> wrote:
>> I really think we should get (/ have gotten) things clear first. What
>> exactly is the bug we have been talking about here? I mean like, where
>> does it occur and what's the nature of it.
>>
>> 1. Is it specific to a certain type / model of backend / physical
>> storage device that will be made use of by qemu for the emulated
>> storage? (I presume not since you mention about image, unless you
>> irrationally related/bound the emulated storage type and the physical
>> storage type together.)
>>
>> 2. Does it have anything to do with a certain flaw in qemu itself?
>> Like the code that does read/write operation is flawed that it cannot
>> be handled by a certain *proper* backend device?
>>
>> 3. Or is it actually a bug in a certain driver / firmware blob that
>> will be used by an *emulated* device in the guest? In that case, can
>> we emulate another model so that it won't be using the problematic
>> driver / firmware?
>
> Definitely agreed -- before we start changing QEMU code we need
> to identify clearly (a) what the real hardware does and (b) what
> the situation was we were originally trying to fix.
Real SD and MMC cards do not have power-of-two constraints in the
number of blocks. None of the SD/MMC bridges I’ve ever dealt with
have a power-of-two constraint on the size of the card. The only
constraint on the size related to the card is the block size.
If non-power-of-2 sized cards fail, that’s a cat-1 sev-1 bug that needs
to be fixed, rather than a warning be generated.
Warner
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 874 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again
2021-06-23 18:00 [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2021-06-23 18:00 ` [RFC PATCH 9/9] hw/sd: Allow card size not power of 2 again Philippe Mathieu-Daudé
@ 2021-06-24 2:50 ` Alexander Bulekov
2021-06-24 8:12 ` Philippe Mathieu-Daudé
9 siblings, 1 reply; 34+ messages in thread
From: Alexander Bulekov @ 2021-06-24 2:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P . Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
On 210623 2000, Philippe Mathieu-Daudé wrote:
> Hi Ubi-Wan Kenubi and Tom,
>
> In commit a9bcedd (SD card size has to be power of 2) we decided
> to restrict SD card size to avoid security problems (CVE-2020-13253)
> but this became not practical to some users.
>
> This RFC series tries to remove the limitation, keeping our
> functional tests working. It is unfinished work because I had to
> attend other topics, but sending it early as RFC to get feedback.
> I'll keep working when I get more time, except if one if you can
> help me.
>
> Alexander, could you generate a qtest reproducer with the fuzzer
> corpus? See: https://bugs.launchpad.net/qemu/+bug/1878054
I think that bug was already fixed - the reproducer no logner causes a
timeout on 6.0. Did I misunderstand something?
I applied this series and ran the OSS-Fuzz corpus for the sdhci-v3
config. The only problem it found is this assert() (that exists without the
patch anyways):
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29225
Let me know if this is something you think I should report on gitlab..
I'll leave the fuzzer running for another 24h or so, but otherwise I'm
happy to leave a Tested-by, once there is a V1 series
-Alex
> Thanks,
>
> Phil.
>
> Philippe Mathieu-Daudé (9):
> hw/sd: When card is in wrong state, log which state it is
> hw/sd: Extract address_in_range() helper, log invalid accesses
> tests/acceptance: Tag NetBSD tests as 'os:netbsd'
> tests/acceptance: Extract image_expand() helper
> tests/acceptance: Use image_expand() in
> test_arm_orangepi_uboot_netbsd9
> tests/acceptance: Use image_expand() in test_arm_orangepi_bionic_20_08
> tests/acceptance: Do not expand SD card image in test_arm_orangepi_sd
> tests/acceptance: Remove now unused pow2ceil()
> hw/sd: Allow card size not power of 2 again
>
> hw/sd/sd.c | 60 +++++++++++++-------------
> tests/acceptance/boot_linux_console.py | 39 ++++++++---------
> tests/acceptance/ppc_prep_40p.py | 2 +
> 3 files changed, 52 insertions(+), 49 deletions(-)
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again
2021-06-24 2:50 ` [RFC PATCH 0/9] " Alexander Bulekov
@ 2021-06-24 8:12 ` Philippe Mathieu-Daudé
2021-06-26 4:04 ` Alexander Bulekov
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-24 8:12 UTC (permalink / raw)
To: Alexander Bulekov
Cc: Daniel P. Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
On 6/24/21 4:50 AM, Alexander Bulekov wrote:
> On 210623 2000, Philippe Mathieu-Daudé wrote:
>> Hi Ubi-Wan Kenubi and Tom,
>>
>> In commit a9bcedd (SD card size has to be power of 2) we decided
>> to restrict SD card size to avoid security problems (CVE-2020-13253)
>> but this became not practical to some users.
>>
>> This RFC series tries to remove the limitation, keeping our
>> functional tests working. It is unfinished work because I had to
>> attend other topics, but sending it early as RFC to get feedback.
>> I'll keep working when I get more time, except if one if you can
>> help me.
>>
>> Alexander, could you generate a qtest reproducer with the fuzzer
>> corpus? See: https://bugs.launchpad.net/qemu/+bug/1878054
>
> I think that bug was already fixed - the reproducer no logner causes a
> timeout on 6.0. Did I misunderstand something?
That bug was fixed but now I'm changing the code and would like to feel
sure I'm not re-introducing the problem, so having the reproducer in the
tree would help.
> I applied this series and ran the OSS-Fuzz corpus for the sdhci-v3
> config. The only problem it found is this assert() (that exists without the
> patch anyways):
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29225
Sigh.
> Let me know if this is something you think I should report on gitlab..
Yes please :(
> I'll leave the fuzzer running for another 24h or so, but otherwise I'm
> happy to leave a Tested-by, once there is a V1 series
> -Alex
Thanks!
Phil.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC PATCH 0/9] hw/sd: Allow card size not power of 2 again
2021-06-24 8:12 ` Philippe Mathieu-Daudé
@ 2021-06-26 4:04 ` Alexander Bulekov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Bulekov @ 2021-06-26 4:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Daniel P. Berrangé,
qemu-block, Bin Meng, qemu-devel, Tom Yan, Niek Linnenbank,
Michal Suchánek, Philippe Mathieu-Daudé,
Warner Losh
On 210624 1012, Philippe Mathieu-Daudé wrote:
> On 6/24/21 4:50 AM, Alexander Bulekov wrote:
> > On 210623 2000, Philippe Mathieu-Daudé wrote:
> >> Hi Ubi-Wan Kenubi and Tom,
> >>
> >> In commit a9bcedd (SD card size has to be power of 2) we decided
> >> to restrict SD card size to avoid security problems (CVE-2020-13253)
> >> but this became not practical to some users.
> >>
> >> This RFC series tries to remove the limitation, keeping our
> >> functional tests working. It is unfinished work because I had to
> >> attend other topics, but sending it early as RFC to get feedback.
> >> I'll keep working when I get more time, except if one if you can
> >> help me.
> >>
> >> Alexander, could you generate a qtest reproducer with the fuzzer
> >> corpus? See: https://bugs.launchpad.net/qemu/+bug/1878054
> >
> > I think that bug was already fixed - the reproducer no logner causes a
> > timeout on 6.0. Did I misunderstand something?
>
> That bug was fixed but now I'm changing the code and would like to feel
> sure I'm not re-introducing the problem, so having the reproducer in the
> tree would help.
Ok - I'll try to come up with one. Minimizing reproducers for timeouts
is trickier than crashes :-)
>
> > I applied this series and ran the OSS-Fuzz corpus for the sdhci-v3
> > config. The only problem it found is this assert() (that exists without the
> > patch anyways):
> > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29225
>
> Sigh.
>
> > Let me know if this is something you think I should report on gitlab..
>
> Yes please :(
Ok here it is: https://gitlab.com/qemu-project/qemu/-/issues/450
The fuzzer I left running for 24h also found another bug (looks like DMA
reentrancy), which I thought was introduced by this patchset, since
OSS-Fuzz had not reported it in months of fuzzing. However, as a
sanity-check I tried it against qemu.git and it crashed - strange...
Anyways here it is: https://gitlab.com/qemu-project/qemu/-/issues/451
-Alex
>
> > I'll leave the fuzzer running for another 24h or so, but otherwise I'm
> > happy to leave a Tested-by, once there is a V1 series
> > -Alex
>
> Thanks!
>
> Phil.
^ permalink raw reply [flat|nested] 34+ messages in thread