diff mbox series

hw/sd: Zero out function selection fields before being populated

Message ID 20201024014954.21330-1-bmeng.cn@gmail.com
State New
Headers show
Series hw/sd: Zero out function selection fields before being populated | expand

Commit Message

Bin Meng Oct. 24, 2020, 1:49 a.m. UTC 
From: Bin Meng <bin.meng@windriver.com>

The function selection fields (399:376) should be zeroed out to
prevent leftover from being or'ed into the switch function status
data structure.

This fixes the boot failure as seen in the acceptance testing on
the orangepi target.

Fixes: b638627c723a ("hw/sd: Fix incorrect populated function switch status data structure")
Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Philippe Mathieu-Daudé Oct. 24, 2020, 7:47 p.m. UTC | #1 
On 10/24/20 3:49 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> The function selection fields (399:376) should be zeroed out to
> prevent leftover from being or'ed into the switch function status
> data structure.
> 
> This fixes the boot failure as seen in the acceptance testing on
> the orangepi target.
> 
> Fixes: b638627c723a ("hw/sd: Fix incorrect populated function switch status data structure")
> Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>   hw/sd/sd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c3febed243..bd10ec8fc4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -824,6 +824,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>       sd->data[12] = 0x80;	/* Supported group 1 functions */
>       sd->data[13] = 0x03;
>   
> +    memset(&sd->data[14], 0, 3);

This is indeed the simpler fix. Thanks for looking at
it so quickly!

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>       for (i = 0; i < 6; i ++) {
>           new_func = (arg >> (i * 4)) & 0x0f;
>           if (mode && new_func != 0x0f)
>
Philippe Mathieu-Daudé Oct. 26, 2020, 8:08 a.m. UTC | #2 
On 10/24/20 3:49 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> The function selection fields (399:376) should be zeroed out to
> prevent leftover from being or'ed into the switch function status
> data structure.
> 
> This fixes the boot failure as seen in the acceptance testing on
> the orangepi target.
> 
> Fixes: b638627c723a ("hw/sd: Fix incorrect populated function switch status data structure")
> Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>   hw/sd/sd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c3febed243..bd10ec8fc4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -824,6 +824,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>       sd->data[12] = 0x80;	/* Supported group 1 functions */
>       sd->data[13] = 0x03;
>   
> +    memset(&sd->data[14], 0, 3);
>       for (i = 0; i < 6; i ++) {
>           new_func = (arg >> (i * 4)) & 0x0f;
>           if (mode && new_func != 0x0f)
> 

Thanks, series applied to sd-next tree.
Bin Meng Oct. 28, 2020, 1:36 a.m. UTC | #3 
Hi Niek,

On Wed, Oct 28, 2020 at 3:55 AM Niek Linnenbank
<nieklinnenbank@gmail.com> wrote:
>
> Hello Philippe, Bin,
>
> Thanks for your support on this. I've just tried this patch and unfortunately it doesn't seem to
> resolve the issue, at least on my machine. This is the output that I get when running the avocado test for NetBSD9.0:
>
>  (5/5) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: |console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000)
> console: DRAM: 1024 MiB
> console: Failed to set core voltage! Can't set CPU frequency
> console: Trying to boot from MMC1
> console: U-Boot 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000) Allwinner Technology
> console: CPU:   Allwinner H3 (SUN8I 0000)
> ...
> console: [   1.2957642] sdmmc0: SD card status: 4-bit, C0
> console: [   1.3094731] ld0 at sdmmc0: <0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
> console: [   1.3159383] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512 bytes/sect x 2097152 sectors
> console: [   1.3763222] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
> console: [   2.0592109] WARNING: 4 errors while detecting hardware; check system log.
> console: [   2.0693403] boot device: ld0
> console: [   2.0798960] root on ld0a dumps on ld0b
> console: [   2.0994141] vfs_mountroot: can't open root device
> console: [   2.0994141] cannot mount root, error = 6
> <FREEZE>
>
> When starting NetBSD 9.0 manually, it shows clearly that the SD card is recognized with 1GiB size, also from U-Boot:
> $ qemu-system-arm -M orangepi-pc -nographic -nic user -sd ./armv7.img
> WARNING: Image format was not specified for './armv7.img' and probing guessed raw.
>          Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>          Specify the 'raw' format explicitly to remove the restrictions.
>
> U-Boot SPL 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200)
> DRAM: 1024 MiB
> Failed to set core voltage! Can't set CPU frequency
> Trying to boot from MMC1
>
> U-Boot 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200) Allwinner Technology
>
> CPU:   Allwinner H3 (SUN8I 0000)
> Model: Xunlong Orange Pi PC
> DRAM:  1 GiB
> MMC:   mmc@1c0f000: 0
> ...
> Hit any key to stop autoboot:  0
> => mmc info
> Device: mmc@1c0f000
> Manufacturer ID: aa
> OEM: 5859
> Name: QEMU!
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 2.0
> High Capacity: No
> Capacity: 1 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> =>
> => boot
> 8846552 bytes read in 931 ms (9.1 MiB/s)
> ...
> [   1.3447558] sdmmc0: SD card status: 4-bit, C0
> [   1.3546801] ld0 at sdmmc0: <0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
> [   1.3647790] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512 bytes/sect x 2097152 sectors
> [   1.4150230] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
> [   2.0800893] WARNING: 4 errors while detecting hardware; check system log.
> [   2.0800893] boot device: ld0
> [   2.0900792] root on ld0a dumps on ld0b
> [   2.1004160] vfs_mountroot: can't open root device
> [   2.1004160] cannot mount root, error = 6
> [   2.1004160] root device (default ld0a):
> <FREEZE>
>
> Note that the image has been resized to 2GiB with qemu-img:
> $ ls -alh armv7.img
> -rw-rw-r-- 1 user user 2,0G okt 28 22:45 armv7.img
>
> The previous patch proposed by Bin did resolve the error ("hw/sd: Fix 2GiB card CSD register values" ):
>  https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg07318.html

Correct. The patch above has not been applied yet, and only this patch
is now in mainline, so you will still see errors in the NetBSD 9.0
test.

>
> Although I see that this patch is now in master (89c6700fe7eed9195f10055751edbc6d5e7ab940),
> can you please confirm that the issue is still present when testing this on your machine as well?
>

Regards,
Bin
Niek Linnenbank Oct. 28, 2020, 9:47 a.m. UTC | #4 
Hello Philippe, Bin,

Thanks for your support on this. I've just tried this patch and
unfortunately it doesn't seem to
resolve the issue, at least on my machine. This is the output that I get
when running the avocado test for NetBSD9.0:

 (5/5)
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
|console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000)
console: DRAM: 1024 MiB
console: Failed to set core voltage! Can't set CPU frequency
console: Trying to boot from MMC1
console: U-Boot 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000) Allwinner
Technology
console: CPU:   Allwinner H3 (SUN8I 0000)
...
console: [   1.2957642] sdmmc0: SD card status: 4-bit, C0
console: [   1.3094731] ld0 at sdmmc0:
<0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
console: [   1.3159383] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512
bytes/sect x 2097152 sectors
console: [   1.3763222] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
console: [   2.0592109] WARNING: 4 errors while detecting hardware; check
system log.
console: [   2.0693403] boot device: ld0
console: [   2.0798960] root on ld0a dumps on ld0b
console: [   2.0994141] vfs_mountroot: can't open root device
console: [   2.0994141] cannot mount root, error = 6
<FREEZE>

When starting NetBSD 9.0 manually, it shows clearly that the SD card is
recognized with 1GiB size, also from U-Boot:
$ qemu-system-arm -M orangepi-pc -nographic -nic user -sd ./armv7.img
WARNING: Image format was not specified for './armv7.img' and probing
guessed raw.
         Automatically detecting the format is dangerous for raw images,
write operations on block 0 will be restricted.
         Specify the 'raw' format explicitly to remove the restrictions.

U-Boot SPL 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200)
DRAM: 1024 MiB
Failed to set core voltage! Can't set CPU frequency
Trying to boot from MMC1

U-Boot 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200) Allwinner
Technology

CPU:   Allwinner H3 (SUN8I 0000)
Model: Xunlong Orange Pi PC
DRAM:  1 GiB
MMC:   mmc@1c0f000: 0
...
Hit any key to stop autoboot:  0
=> mmc info
Device: mmc@1c0f000
Manufacturer ID: aa
OEM: 5859
Name: QEMU!
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 2.0
High Capacity: No
Capacity: 1 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
=>
=> boot
8846552 bytes read in 931 ms (9.1 MiB/s)
...
[   1.3447558] sdmmc0: SD card status: 4-bit, C0
[   1.3546801] ld0 at sdmmc0: <0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
[   1.3647790] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512 bytes/sect x
2097152 sectors
[   1.4150230] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
[   2.0800893] WARNING: 4 errors while detecting hardware; check system log.
[   2.0800893] boot device: ld0
[   2.0900792] root on ld0a dumps on ld0b
[   2.1004160] vfs_mountroot: can't open root device
[   2.1004160] cannot mount root, error = 6
[   2.1004160] root device (default ld0a):
<FREEZE>

Note that the image has been resized to 2GiB with qemu-img:
$ ls -alh armv7.img
-rw-rw-r-- 1 user user 2,0G okt 28 22:45 armv7.img

The previous patch proposed by Bin did resolve the error ("hw/sd: Fix 2GiB
card CSD register values" ):
 https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg07318.html

Although I see that this patch is now in master
(89c6700fe7eed9195f10055751edbc6d5e7ab940),
can you please confirm that the issue is still present when testing this on
your machine as well?

With kind regards,
Niek


On Mon, Oct 26, 2020 at 9:10 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 10/24/20 3:49 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > The function selection fields (399:376) should be zeroed out to
> > prevent leftover from being or'ed into the switch function status
> > data structure.
> >
> > This fixes the boot failure as seen in the acceptance testing on
> > the orangepi target.
> >
> > Fixes: b638627c723a ("hw/sd: Fix incorrect populated function switch
> status data structure")
> > Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >   hw/sd/sd.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index c3febed243..bd10ec8fc4 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -824,6 +824,7 @@ static void sd_function_switch(SDState *sd, uint32_t
> arg)
> >       sd->data[12] = 0x80;    /* Supported group 1 functions */
> >       sd->data[13] = 0x03;
> >
> > +    memset(&sd->data[14], 0, 3);
> >       for (i = 0; i < 6; i ++) {
> >           new_func = (arg >> (i * 4)) & 0x0f;
> >           if (mode && new_func != 0x0f)
> >
>
> Thanks, series applied to sd-next tree.
>
>
Philippe Mathieu-Daudé Oct. 28, 2020, 10:10 a.m. UTC | #5 
On 10/28/20 2:36 AM, Bin Meng wrote:
> Hi Niek,
> 
> On Wed, Oct 28, 2020 at 3:55 AM Niek Linnenbank
> <nieklinnenbank@gmail.com> wrote:
>>
>> Hello Philippe, Bin,
>>
>> Thanks for your support on this. I've just tried this patch and unfortunately it doesn't seem to
>> resolve the issue, at least on my machine. This is the output that I get when running the avocado test for NetBSD9.0:
>>
>>  (5/5) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: |console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000)
>> console: DRAM: 1024 MiB
>> console: Failed to set core voltage! Can't set CPU frequency
>> console: Trying to boot from MMC1
>> console: U-Boot 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000) Allwinner Technology
>> console: CPU:   Allwinner H3 (SUN8I 0000)
>> ...
>> console: [   1.2957642] sdmmc0: SD card status: 4-bit, C0
>> console: [   1.3094731] ld0 at sdmmc0: <0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
>> console: [   1.3159383] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512 bytes/sect x 2097152 sectors

Same problem as before.

>> console: [   1.3763222] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
>> console: [   2.0592109] WARNING: 4 errors while detecting hardware; check system log.
>> console: [   2.0693403] boot device: ld0
>> console: [   2.0798960] root on ld0a dumps on ld0b
>> console: [   2.0994141] vfs_mountroot: can't open root device
>> console: [   2.0994141] cannot mount root, error = 6
>> <FREEZE>
>>
>> When starting NetBSD 9.0 manually, it shows clearly that the SD card is recognized with 1GiB size, also from U-Boot:
>> $ qemu-system-arm -M orangepi-pc -nographic -nic user -sd ./armv7.img
>> WARNING: Image format was not specified for './armv7.img' and probing guessed raw.
>>          Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>>          Specify the 'raw' format explicitly to remove the restrictions.
>>
>> U-Boot SPL 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200)
>> DRAM: 1024 MiB
>> Failed to set core voltage! Can't set CPU frequency
>> Trying to boot from MMC1
>>
>> U-Boot 2020.07-00610-g610e1487c8 (Jul 11 2020 - 22:31:58 +0200) Allwinner Technology
>>
>> CPU:   Allwinner H3 (SUN8I 0000)
>> Model: Xunlong Orange Pi PC
>> DRAM:  1 GiB
>> MMC:   mmc@1c0f000: 0
>> ...
>> Hit any key to stop autoboot:  0
>> => mmc info
>> Device: mmc@1c0f000
>> Manufacturer ID: aa
>> OEM: 5859
>> Name: QEMU!
>> Bus Speed: 50000000
>> Mode: SD High Speed (50MHz)
>> Rd Block Len: 512
>> SD version 2.0
>> High Capacity: No
>> Capacity: 1 GiB
>> Bus Width: 4-bit
>> Erase Group Size: 512 Bytes
>> =>
>> => boot
>> 8846552 bytes read in 931 ms (9.1 MiB/s)
>> ...
>> [   1.3447558] sdmmc0: SD card status: 4-bit, C0
>> [   1.3546801] ld0 at sdmmc0: <0xaa:0x5859:QEMU!:0x01:0xdeadbeef:0x062>
>> [   1.3647790] ld0: 1024 MB, 1040 cyl, 32 head, 63 sec, 512 bytes/sect x 2097152 sectors
>> [   1.4150230] ld0: 4-bit width, High-Speed/SDR25, 50.000 MHz
>> [   2.0800893] WARNING: 4 errors while detecting hardware; check system log.
>> [   2.0800893] boot device: ld0
>> [   2.0900792] root on ld0a dumps on ld0b
>> [   2.1004160] vfs_mountroot: can't open root device
>> [   2.1004160] cannot mount root, error = 6
>> [   2.1004160] root device (default ld0a):
>> <FREEZE>
>>
>> Note that the image has been resized to 2GiB with qemu-img:
>> $ ls -alh armv7.img
>> -rw-rw-r-- 1 user user 2,0G okt 28 22:45 armv7.img
>>
>> The previous patch proposed by Bin did resolve the error ("hw/sd: Fix 2GiB card CSD register values" ):
>>  https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg07318.html
> 
> Correct. The patch above has not been applied yet, and only this patch
> is now in mainline, so you will still see errors in the NetBSD 9.0
> test.

Yesterday was "feature freeze" but we still have time to do more testing
and fix bugs :)
I didn't want to rush and squeeze this fix too quickly. I kept it for
the next pull request so I have time to review and think about it more.

Thanks for the testing!

> 
>>
>> Although I see that this patch is now in master (89c6700fe7eed9195f10055751edbc6d5e7ab940),
>> can you please confirm that the issue is still present when testing this on your machine as well?
>>
> 
> Regards,
> Bin
>
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c3febed243..bd10ec8fc4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -824,6 +824,7 @@  static void sd_function_switch(SDState *sd, uint32_t arg)
     sd->data[12] = 0x80;	/* Supported group 1 functions */
     sd->data[13] = 0x03;
 
+    memset(&sd->data[14], 0, 3);
     for (i = 0; i < 6; i ++) {
         new_func = (arg >> (i * 4)) & 0x0f;
         if (mode && new_func != 0x0f)