| Message ID | 20200821172916.1260954-23-f4bug@amsat.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SD/MMC patches for 2020-08-21 | expand |
Hi Bin, On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote: > From: Bin Meng <bin.meng@windriver.com> > > At present the function switch status data structure bit [399:376] > are wrongly pupulated. These 3 bytes encode function switch status > for the 6 function groups, with 4 bits per group, starting from > function group 6 at bit 399, then followed by function group 5 at > bit 395, and so on. > > However the codes mistakenly fills in the function group 1 status > at bit 399. This fixes the code logic. > > Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") > Signed-off-by: Bin Meng <bin.meng@windriver.com> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> > 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 7c9d956f113..805e21fc883 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) > sd->data[11] = 0x43; > sd->data[12] = 0x80; /* Supported group 1 functions */ > sd->data[13] = 0x03; > + > for (i = 0; i < 6; i ++) { > new_func = (arg >> (i * 4)) & 0x0f; > if (mode && new_func != 0x0f) > sd->function_group[i] = new_func; > - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); > + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); This patch broke the orangepi machine, reproducible running test_arm_orangepi_bionic: https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html Can you have a look? > } > memset(&sd->data[17], 0, 47); > stw_be_p(sd->data + 64, sd_crc16(sd->data, 64)); >
Hi Philippe, On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Hi Bin, > > On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote: > > From: Bin Meng <bin.meng@windriver.com> > > > > At present the function switch status data structure bit [399:376] > > are wrongly pupulated. These 3 bytes encode function switch status > > for the 6 function groups, with 4 bits per group, starting from > > function group 6 at bit 399, then followed by function group 5 at > > bit 395, and so on. > > > > However the codes mistakenly fills in the function group 1 status > > at bit 399. This fixes the code logic. > > > > Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > > Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> > > 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 7c9d956f113..805e21fc883 100644 > > --- a/hw/sd/sd.c > > +++ b/hw/sd/sd.c > > @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) > > sd->data[11] = 0x43; > > sd->data[12] = 0x80; /* Supported group 1 functions */ > > sd->data[13] = 0x03; > > + > > for (i = 0; i < 6; i ++) { > > new_func = (arg >> (i * 4)) & 0x0f; > > if (mode && new_func != 0x0f) > > sd->function_group[i] = new_func; > > - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); > > + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); > > This patch broke the orangepi machine, reproducible running > test_arm_orangepi_bionic: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html > > Can you have a look? Yes, I can take a look. Could you please send more details on how to run "test_arm_orangepi_bionic"? Regards, Bin
On 10/21/20 11:57 AM, Bin Meng wrote: > Hi Philippe, > > On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> Hi Bin, >> >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote: >>> From: Bin Meng <bin.meng@windriver.com> >>> >>> At present the function switch status data structure bit [399:376] >>> are wrongly pupulated. These 3 bytes encode function switch status >>> for the 6 function groups, with 4 bits per group, starting from >>> function group 6 at bit 399, then followed by function group 5 at >>> bit 395, and so on. >>> >>> However the codes mistakenly fills in the function group 1 status >>> at bit 399. This fixes the code logic. >>> >>> Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") >>> Signed-off-by: Bin Meng <bin.meng@windriver.com> >>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> >>> Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> >>> 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 7c9d956f113..805e21fc883 100644 >>> --- a/hw/sd/sd.c >>> +++ b/hw/sd/sd.c >>> @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) >>> sd->data[11] = 0x43; >>> sd->data[12] = 0x80; /* Supported group 1 functions */ >>> sd->data[13] = 0x03; >>> + >>> for (i = 0; i < 6; i ++) { >>> new_func = (arg >> (i * 4)) & 0x0f; >>> if (mode && new_func != 0x0f) >>> sd->function_group[i] = new_func; >>> - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); >>> + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); >> >> This patch broke the orangepi machine, reproducible running >> test_arm_orangepi_bionic: >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html >> >> Can you have a look? > > Yes, I can take a look. Could you please send more details on how to > run "test_arm_orangepi_bionic"? Looking at the previous link, I think this should work: $ make check-venv qemu-system-arm $ AVOCADO_ALLOW_LARGE_STORAGE=1 \ tests/venv/bin/python -m \ avocado --show=app,console run \ run -t machine:orangepi-pc tests/acceptance > > Regards, > Bin >
Hi Philippe, On Wed, Oct 21, 2020 at 6:07 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 10/21/20 11:57 AM, Bin Meng wrote: > > Hi Philippe, > > > > On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> > >> Hi Bin, > >> > >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote: > >>> From: Bin Meng <bin.meng@windriver.com> > >>> > >>> At present the function switch status data structure bit [399:376] > >>> are wrongly pupulated. These 3 bytes encode function switch status > >>> for the 6 function groups, with 4 bits per group, starting from > >>> function group 6 at bit 399, then followed by function group 5 at > >>> bit 395, and so on. > >>> > >>> However the codes mistakenly fills in the function group 1 status > >>> at bit 399. This fixes the code logic. > >>> > >>> Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") > >>> Signed-off-by: Bin Meng <bin.meng@windriver.com> > >>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >>> Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > >>> Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> > >>> 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 7c9d956f113..805e21fc883 100644 > >>> --- a/hw/sd/sd.c > >>> +++ b/hw/sd/sd.c > >>> @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) > >>> sd->data[11] = 0x43; > >>> sd->data[12] = 0x80; /* Supported group 1 functions */ > >>> sd->data[13] = 0x03; > >>> + > >>> for (i = 0; i < 6; i ++) { > >>> new_func = (arg >> (i * 4)) & 0x0f; > >>> if (mode && new_func != 0x0f) > >>> sd->function_group[i] = new_func; > >>> - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); > >>> + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); > >> > >> This patch broke the orangepi machine, reproducible running > >> test_arm_orangepi_bionic: > >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html > >> > >> Can you have a look? > > > > Yes, I can take a look. Could you please send more details on how to > > run "test_arm_orangepi_bionic"? > > Looking at the previous link, I think this should work: > > $ make check-venv qemu-system-arm > $ AVOCADO_ALLOW_LARGE_STORAGE=1 \ > tests/venv/bin/python -m \ > avocado --show=app,console run \ > run -t machine:orangepi-pc tests/acceptance > I tried the above command in my build tree, and got: avocado run: error: unrecognized arguments: tests/acceptance Perhaps a plugin is missing; run 'avocado plugins' to list the installed ones I switched to `make check-acceptance` which seems to work, however not for orangepi-pc related tests? (09/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi: SKIP: Test artifacts fetched from unreliable apt.armbian.com (10/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd: SKIP: Test artifacts fetched from unreliable apt.armbian.com (11/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: SKIP: Test artifacts fetched from unreliable apt.armbian.com (12/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic: SKIP: storage limited (13/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: SKIP: storage limited Any ideas? Regards, Bin
Hi Bin, Philippe, If im correct the acceptance tests for orange pi need to be run with a flag ARMBIAN_ARTIFACTS_CACHED set that explicitly allows them to be run using the armbian mirror. So if you pass that flag on the same command that Philippe gave, the rests should run. I have a follow up question and Im interested to hear your opinion on that Philippe. Should we perhaps update the orange pi tests (and maybe others) so they use a reliable mirror that we can control, for example a github repo? I would be happy to create a repo for that, at least for the orange pi tests. But maybe there is already something planned as a more general solution for artifacts of other machines as well? regards, Niek Op do 22 okt. 2020 16:47 schreef Bin Meng <bmeng.cn@gmail.com>: > Hi Philippe, > > On Wed, Oct 21, 2020 at 6:07 PM Philippe Mathieu-Daudé <f4bug@amsat.org> > wrote: > > > > On 10/21/20 11:57 AM, Bin Meng wrote: > > > Hi Philippe, > > > > > > On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé < > f4bug@amsat.org> wrote: > > >> > > >> Hi Bin, > > >> > > >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote: > > >>> From: Bin Meng <bin.meng@windriver.com> > > >>> > > >>> At present the function switch status data structure bit [399:376] > > >>> are wrongly pupulated. These 3 bytes encode function switch status > > >>> for the 6 function groups, with 4 bits per group, starting from > > >>> function group 6 at bit 399, then followed by function group 5 at > > >>> bit 395, and so on. > > >>> > > >>> However the codes mistakenly fills in the function group 1 status > > >>> at bit 399. This fixes the code logic. > > >>> > > >>> Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") > > >>> Signed-off-by: Bin Meng <bin.meng@windriver.com> > > >>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > >>> Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > > >>> Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> > > >>> 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 7c9d956f113..805e21fc883 100644 > > >>> --- a/hw/sd/sd.c > > >>> +++ b/hw/sd/sd.c > > >>> @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, > uint32_t arg) > > >>> sd->data[11] = 0x43; > > >>> sd->data[12] = 0x80; /* Supported group 1 functions */ > > >>> sd->data[13] = 0x03; > > >>> + > > >>> for (i = 0; i < 6; i ++) { > > >>> new_func = (arg >> (i * 4)) & 0x0f; > > >>> if (mode && new_func != 0x0f) > > >>> sd->function_group[i] = new_func; > > >>> - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); > > >>> + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); > > >> > > >> This patch broke the orangepi machine, reproducible running > > >> test_arm_orangepi_bionic: > > >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html > > >> > > >> Can you have a look? > > > > > > Yes, I can take a look. Could you please send more details on how to > > > run "test_arm_orangepi_bionic"? > > > > Looking at the previous link, I think this should work: > > > > $ make check-venv qemu-system-arm > > $ AVOCADO_ALLOW_LARGE_STORAGE=1 \ > > tests/venv/bin/python -m \ > > avocado --show=app,console run \ > > run -t machine:orangepi-pc tests/acceptance > > > > I tried the above command in my build tree, and got: > > avocado run: error: unrecognized arguments: tests/acceptance > Perhaps a plugin is missing; run 'avocado plugins' to list the installed > ones > > I switched to `make check-acceptance` which seems to work, however not > for orangepi-pc related tests? > > (09/32) > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi: > SKIP: Test artifacts fetched from unreliable apt.armbian.com > (10/32) > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd: > SKIP: Test artifacts fetched from unreliable apt.armbian.com > (11/32) > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: > SKIP: Test artifacts fetched from unreliable apt.armbian.com > (12/32) > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic: > SKIP: storage limited > (13/32) > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: > SKIP: storage limited > > Any ideas? > > Regards, > Bin > <div dir="auto">Hi Bin, Philippe,<div dir="auto"><br></div><div dir="auto">If im correct the acceptance tests for orange pi need to be run with a flag ARMBIAN_ARTIFACTS_CACHED set that explicitly allows them to be run using the armbian mirror. So if you pass that flag on the same command that Philippe gave, the rests should run.</div><div dir="auto"><br></div><div dir="auto">I have a follow up question and Im interested to hear your opinion on that Philippe. Should we perhaps update the orange pi tests (and maybe others) so they use a reliable mirror that we can control, for example a github repo? I would be happy to create a repo for that, at least for the orange pi tests. But maybe there is already something planned as a more general solution for artifacts of other machines as well?</div><div dir="auto"><br></div><div dir="auto">regards,</div><div dir="auto">Niek</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Op do 22 okt. 2020 16:47 schreef Bin Meng <<a href="mailto:bmeng.cn@gmail.com">bmeng.cn@gmail.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Philippe,<br> <br> On Wed, Oct 21, 2020 at 6:07 PM Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank" rel="noreferrer">f4bug@amsat.org</a>> wrote:<br> ><br> > On 10/21/20 11:57 AM, Bin Meng wrote:<br> > > Hi Philippe,<br> > ><br> > > On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank" rel="noreferrer">f4bug@amsat.org</a>> wrote:<br> > >><br> > >> Hi Bin,<br> > >><br> > >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:<br> > >>> From: Bin Meng <<a href="mailto:bin.meng@windriver.com" target="_blank" rel="noreferrer">bin.meng@windriver.com</a>><br> > >>><br> > >>> At present the function switch status data structure bit [399:376]<br> > >>> are wrongly pupulated. These 3 bytes encode function switch status<br> > >>> for the 6 function groups, with 4 bits per group, starting from<br> > >>> function group 6 at bit 399, then followed by function group 5 at<br> > >>> bit 395, and so on.<br> > >>><br> > >>> However the codes mistakenly fills in the function group 1 status<br> > >>> at bit 399. This fixes the code logic.<br> > >>><br> > >>> Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)")<br> > >>> Signed-off-by: Bin Meng <<a href="mailto:bin.meng@windriver.com" target="_blank" rel="noreferrer">bin.meng@windriver.com</a>><br> > >>> Reviewed-by: Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank" rel="noreferrer">f4bug@amsat.org</a>><br> > >>> Tested-by: Sai Pavan Boddu <<a href="mailto:sai.pavan.boddu@xilinx.com" target="_blank" rel="noreferrer">sai.pavan.boddu@xilinx.com</a>><br> > >>> Message-Id: <<a href="mailto:1598021136-49525-1-git-send-email-bmeng.cn@gmail.com" target="_blank" rel="noreferrer">1598021136-49525-1-git-send-email-bmeng.cn@gmail.com</a>><br> > >>> Signed-off-by: Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank" rel="noreferrer">f4bug@amsat.org</a>><br> > >>> ---<br> > >>> hw/sd/sd.c | 3 ++-<br> > >>> 1 file changed, 2 insertions(+), 1 deletion(-)<br> > >>><br> > >>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c<br> > >>> index 7c9d956f113..805e21fc883 100644<br> > >>> --- a/hw/sd/sd.c<br> > >>> +++ b/hw/sd/sd.c<br> > >>> @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg)<br> > >>> sd->data[11] = 0x43;<br> > >>> sd->data[12] = 0x80; /* Supported group 1 functions */<br> > >>> sd->data[13] = 0x03;<br> > >>> +<br> > >>> for (i = 0; i < 6; i ++) {<br> > >>> new_func = (arg >> (i * 4)) & 0x0f;<br> > >>> if (mode && new_func != 0x0f)<br> > >>> sd->function_group[i] = new_func;<br> > >>> - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);<br> > >>> + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);<br> > >><br> > >> This patch broke the orangepi machine, reproducible running<br> > >> test_arm_orangepi_bionic:<br> > >> <a href="https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html" rel="noreferrer noreferrer" target="_blank">https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html</a><br> > >><br> > >> Can you have a look?<br> > ><br> > > Yes, I can take a look. Could you please send more details on how to<br> > > run "test_arm_orangepi_bionic"?<br> ><br> > Looking at the previous link, I think this should work:<br> ><br> > $ make check-venv qemu-system-arm<br> > $ AVOCADO_ALLOW_LARGE_STORAGE=1 \<br> > tests/venv/bin/python -m \<br> > avocado --show=app,console run \<br> > run -t machine:orangepi-pc tests/acceptance<br> ><br> <br> I tried the above command in my build tree, and got:<br> <br> avocado run: error: unrecognized arguments: tests/acceptance<br> Perhaps a plugin is missing; run 'avocado plugins' to list the installed ones<br> <br> I switched to `make check-acceptance` which seems to work, however not<br> for orangepi-pc related tests?<br> <br> (09/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi:<br> SKIP: Test artifacts fetched from unreliable <a href="http://apt.armbian.com" rel="noreferrer noreferrer" target="_blank">apt.armbian.com</a><br> (10/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd:<br> SKIP: Test artifacts fetched from unreliable <a href="http://apt.armbian.com" rel="noreferrer noreferrer" target="_blank">apt.armbian.com</a><br> (11/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd:<br> SKIP: Test artifacts fetched from unreliable <a href="http://apt.armbian.com" rel="noreferrer noreferrer" target="_blank">apt.armbian.com</a><br> (12/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic:<br> SKIP: storage limited<br> (13/32) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:<br> SKIP: storage limited<br> <br> Any ideas?<br> <br> Regards,<br> Bin<br> </blockquote></div>
Hi Niek, On Thu, Oct 22, 2020 at 11:20 PM Niek Linnenbank <nieklinnenbank@gmail.com> wrote: > > Hi Bin, Philippe, > > If im correct the acceptance tests for orange pi need to be run with a flag ARMBIAN_ARTIFACTS_CACHED set that explicitly allows them to be run using the armbian mirror. So if you pass that flag on the same command that Philippe gave, the rests should run. Thank you for the hints. Actually I noticed the environment variable ARMBIAN_ARTIFACTS_CACHED when looking at the test codes, but after I turned on the flag it still could not download the test asset from the apt.armbian.com website. > I have a follow up question and Im interested to hear your opinion on that Philippe. Should we perhaps update the orange pi tests (and maybe others) so they use a reliable mirror that we can control, for example a github repo? I would be happy to create a repo for that, at least for the orange pi tests. But maybe there is already something planned as a more general solution for artifacts of other machines as well? > Regards, Bin
On 10/23/20 4:02 AM, Bin Meng wrote: > Hi Niek, > > On Thu, Oct 22, 2020 at 11:20 PM Niek Linnenbank > <nieklinnenbank@gmail.com> wrote: >> >> Hi Bin, Philippe, >> >> If im correct the acceptance tests for orange pi need to be run with a flag ARMBIAN_ARTIFACTS_CACHED set that explicitly allows them to be run using the armbian mirror. So if you pass that flag on the same command that Philippe gave, the rests should run. > > Thank you for the hints. Actually I noticed the environment variable > ARMBIAN_ARTIFACTS_CACHED when looking at the test codes, but after I > turned on the flag it still could not download the test asset from the > apt.armbian.com website. This patch could help you, but it use a different image [*]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg737760.html (I haven't tested the image yet but I assume the same bug is present). > >> I have a follow up question and Im interested to hear your opinion on that Philippe. Should we perhaps update the orange pi tests (and maybe others) so they use a reliable mirror that we can control, for example a github repo? I would be happy to create a repo for that, at least for the orange pi tests. But maybe there is already something planned as a more general solution for artifacts of other machines as well? This is a technical debt between the Avocado and QEMU communities (or a misunderstanding?). QEMU community understood the Avocado caching would be an helpful feature, but it ended being more of a problem. I.e. here Niek, Michael Roth and myself still have the image cached, so we can keep running the tests committed to the repository. You Bin can not, as the armbian mirror is not stable and remove the old image. The old image (Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.img) has been manually tested by Niek and myself, I enabled tracing and look at the SD packets for some time, was happy how the model worked and decided we should keep running a test with this particular image. Armbian released new images, in particular the one Cleber sent in [*]: Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz. While it might work similarly, I haven't tested it, and don't have time to do again the same audit. From my experience with the Raspberry Pi, these images never work the same way, as the Linux kernel evolves quickly with these kinda recent embedded boards. The QEMU raspi models have to emulate new devices (or complete current ones) every years, because Linux started to use a device differently, or started to use a part of the SoC that was not used before. Either the kernel doesn't boot, or there are side-effect later when userspace program is executed. PITA to debug TBH. For this reason I disagree with updating test images to the latest releases. It would be nice to know QEMU works with the latest Armbian, but nobody popped up on the mailing list asking for it. I am personally happy enough using the 19.11 release for now.
Hi Philippe, On Fri, Oct 23, 2020 at 11:34 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 10/23/20 11:23 AM, Philippe Mathieu-Daudé wrote: > > On 10/23/20 4:02 AM, Bin Meng wrote: > >> Hi Niek, > >> > >> On Thu, Oct 22, 2020 at 11:20 PM Niek Linnenbank > >> <nieklinnenbank@gmail.com> wrote: > >>> > >>> Hi Bin, Philippe, > >>> > >>> If im correct the acceptance tests for orange pi need to be run with > >>> a flag ARMBIAN_ARTIFACTS_CACHED set that explicitly allows them to be > >>> run using the armbian mirror. So if you pass that flag on the same > >>> command that Philippe gave, the rests should run. > >> > >> Thank you for the hints. Actually I noticed the environment variable > >> ARMBIAN_ARTIFACTS_CACHED when looking at the test codes, but after I > >> turned on the flag it still could not download the test asset from the > >> apt.armbian.com website. > > > > This patch could help you, but it use a different image [*]: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg737760.html > > (I haven't tested the image yet but I assume the same bug is present). > > > >> > >>> I have a follow up question and Im interested to hear your opinion on > >>> that Philippe. Should we perhaps update the orange pi tests (and > >>> maybe others) so they use a reliable mirror that we can control, for > >>> example a github repo? I would be happy to create a repo for that, at > >>> least for the orange pi tests. But maybe there is already something > >>> planned as a more general solution for artifacts of other machines as > >>> well? > > > > This is a technical debt between the Avocado and QEMU communities > > (or a misunderstanding?). > > > > QEMU community understood the Avocado caching would be an helpful > > feature, but it ended being more of a problem. > > > > > > I.e. here Niek, Michael Roth and myself still have the image cached, > > so we can keep running the tests committed to the repository. You > > Bin can not, as the armbian mirror is not stable and remove the old > > image. > > > > The old image (Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.img) > > has been manually tested by Niek and myself, I enabled tracing and > > look at the SD packets for some time, was happy how the model worked > > and decided we should keep running a test with this particular image. > > > > Armbian released new images, in particular the one Cleber sent in [*]: > > Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz. While it might > > work similarly, I haven't tested it, and don't have time to do again > > the same audit. > > From my experience with the Raspberry Pi, these images never work the > > same way, as the Linux kernel evolves quickly with these kinda recent > > embedded boards. The QEMU raspi models have to emulate new devices > > (or complete current ones) every years, because Linux started to use > > a device differently, or started to use a part of the SoC that was not > > used before. Either the kernel doesn't boot, or there are side-effect > > later when userspace program is executed. PITA to debug TBH. For this > > reason I disagree with updating test images to the latest releases. > Yes, I agree that we should try to use stable artifacts for the acceptance tests. > > > > It would be nice to know QEMU works with the latest Armbian, but > > nobody popped up on the mailing list asking for it. I am personally > > happy enough using the 19.11 release for now. > > Back to the cache problem, I started to ask 2 years ago, > https://www.mail-archive.com/avocado-devel@redhat.com/msg00860.html > note this is the exact situation we are having: > > >> - What will happens if I add tests downloading running on their > >> compiled u-boot > >> > ( > https://downloads.gumstix.com/images/angstrom/developer/2012-01-22-1750/u-boot.bin > ) > >> and the company decides to remove this old directory? > >> Since sometimes old open-source software are hard to rebuild with > >> recent compilers, should we consider to use a public storage to keep > >> open-source (signed) blobs we can use for integration testing? > > > > For Avocado-VT, there are the JeOS images[1], which we keep on a test > > "assets" directory. We have a lot of storage/bandwidth availability, > > so it can be used for other assets proven to be necessary for tests. > > > > As long as distribution rights and licensing are not issues, we can > > definitely use the same server for kernels, u-boot images and what > > not. > > > > [1] - https://avocado-project.org/data/assets/ > > > We discussed about this again last year at the KVM forum. Various > RFE have been filled: > https://www.mail-archive.com/avocado-devel@redhat.com/msg01183.html > > > What we need is a QEMU community file server similar to the asset > one used by the Avocado community. The problem is who is going to > pay and sysadmin it. IIRC Gerd suggested to use GitLab Page, but > the artifact file size is limited to a maximum of 1GiB: > https://docs.gitlab.com/ee/user/gitlab_com/#gitlab-pages > Alternative is to use a git-lfs server: > https://docs.gitlab.com/ee/topics/git/lfs/#hosting-lfs-objects-externally > This is now a project management problem. > > What about the host / server where qemu.org is located? If we could store them there in a sub-directory, that should work. Perhaps people from the community would be willing to provide a mirror server to balance to traffic load. > Next week is the KVM forum 2020 and there might be a BoF session > to talk about that: > > https://kvmforum2020.sched.com/overview/type/Birds+of+a+Feather+Sessions+(BoF) > > > Now that the QEMU community started to use gitlab-ci more and more > I received complains that Avocado is not practical (hard to > understand how to reproduce command line to debug, options to use > not clear, timeouts not clear, why download all artifacts to run > a single test, various issues regarding caching, cache filled /home > filesystem) so I have been asked to look at Avocado alternatives, > because we want contributors run more tests and CI, not them be > scared by it. > I haven't looked at alternatives yet, from the various features > we could have use, the biggest one is the ability to interact with > the serial console. And that feature is duplicated with the VM > testing class: tests/vm/basevm.py. > The SSH session used in linux_ssh_mips_malta.py is similar to the > one from basevm.py too (see 'make vm-boot-ssh-%'). > Some tests are not 'acceptance' but simple qtest written in Python, > such cpu_queries.py / pc_cpu_hotplug_props.py / migration.py / > pc_cpu_hotplug_props.py / x86_cpu_model_versions.py. > > The classes I find practical are downloading artifact, uncompressing > or extracting archive, and eventually the cloudinit one. But we can > probably use them directly. > > Interesting to hear that. Yeah that is probably a big task that takes time to find the best solution. There are quite some files indeed in the tests/acceptance directory, almost a full framework on its own. In fact it could even be something to consider, to make ./tests/acceptance a stand-alone framework when avocado isn't used anymore in the future? > > Lot of material to discuss :) > Thanks for your extensive reply Philippe. Looking forward to hear what the outcome is on that with the discussions at the KVM forum. > > Regards, > > Phil. > >
On Fri, Oct 23, 2020 at 11:34:29AM +0200, Philippe Mathieu-Daudé wrote: > On 10/23/20 11:23 AM, Philippe Mathieu-Daudé wrote: > > Back to the cache problem, I started to ask 2 years ago, > https://www.mail-archive.com/avocado-devel@redhat.com/msg00860.html > note this is the exact situation we are having: > Hi Phil, I think the issue on this thread was more on the server mirror side, than on the *cache* side of things, right? Still, it's an open issue that needs to be discussed indeed. > >> - What will happens if I add tests downloading running on their > >> compiled u-boot > >> (https://downloads.gumstix.com/images/angstrom/developer/2012-01-22-1750/u-boot.bin) > >> and the company decides to remove this old directory? > >> Since sometimes old open-source software are hard to rebuild with > >> recent compilers, should we consider to use a public storage to keep > >> open-source (signed) blobs we can use for integration testing? > > > > For Avocado-VT, there are the JeOS images[1], which we keep on a test > > "assets" directory. We have a lot of storage/bandwidth availability, > > so it can be used for other assets proven to be necessary for tests. > > > > As long as distribution rights and licensing are not issues, we can > > definitely use the same server for kernels, u-boot images and what > > not. > > > > [1] - https://avocado-project.org/data/assets/ > > > We discussed about this again last year at the KVM forum. Various > RFE have been filled: > https://www.mail-archive.com/avocado-devel@redhat.com/msg01183.html > I understand it has been a year, which is a lot of time specially when it comes to an urgent need such as more and better testing and CI. But some things can be patched quickly, while others need further structural work. More on that later... > > What we need is a QEMU community file server similar to the asset > one used by the Avocado community. The problem is who is going to > pay and sysadmin it. IIRC Gerd suggested to use GitLab Page, but > the artifact file size is limited to a maximum of 1GiB: > https://docs.gitlab.com/ee/user/gitlab_com/#gitlab-pages > Alternative is to use a git-lfs server: > https://docs.gitlab.com/ee/topics/git/lfs/#hosting-lfs-objects-externally > This is now a project management problem. > > Next week is the KVM forum 2020 and there might be a BoF session > to talk about that: > https://kvmforum2020.sched.com/overview/type/Birds+of+a+Feather+Sessions+(BoF) > > > Now that the QEMU community started to use gitlab-ci more and more > I received complains that Avocado is not practical (hard to > understand how to reproduce command line to debug, options to use > not clear, timeouts not clear, why download all artifacts to run > a single test, various issues regarding caching, cache filled /home > filesystem) so I have been asked to look at Avocado alternatives, > because we want contributors run more tests and CI, not them be > scared by it. What I can say is that the Avocado developers have been working on a major overhaul to address those issues. I'm not sure this is the place/time to go over each of those items, but there may be some confusion going on some of those issues. For instance, your point about "why download all artifacts to run a single test". It shouldn't, and it doesn't: $ du -hcs ~/avocado/data/cache/ 4.0K /home/cleber/avocado/data/cache/ 4.0K total $ ./tests/venv/bin/avocado run tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_x86_64_pc Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_x86_64_pc JOB ID : a3c08d7a0acc613c207b27e4b291a0010965af2d JOB LOG : /home/cleber/avocado/job-results/job-2020-10-26T11.50-a3c08d7/job.log (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_x86_64_pc: PASS (2.04 s) RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB TIME : 3.22 s $ du -hcs ~/avocado/data/cache/ 8.3M /home/cleber/avocado/data/cache/ 8.3M total About Avocado being hard to understand, I'm a bit saddened by that, because the team spent a lot of effort in: * Restructuring / rewriting / reviewing the documention * Normalizing the command line options * Deprecating and removing a lot of non-relevant functionality I've probably said the same thing in a few different channels and contexts, but for the last ~9 months, the Avocado project had been working on a new architecture that was highly influenced by the limitations we found while using Avocado in QEMU. Version 82.0 was the result of that work, and it'd be very bad timing if, instead of integrating this "new Avocado", QEMU would start "from scratch" again. I've been looking for an opportunity to present what this "new Avocado" is. Maybe this or another BoF would be a good place? > I haven't looked at alternatives yet, from the various features > we could have use, the biggest one is the ability to interact with > the serial console. And that feature is duplicated with the VM > testing class: tests/vm/basevm.py. > The SSH session used in linux_ssh_mips_malta.py is similar to the > one from basevm.py too (see 'make vm-boot-ssh-%'). > Some tests are not 'acceptance' but simple qtest written in Python, > such cpu_queries.py / pc_cpu_hotplug_props.py / migration.py / > pc_cpu_hotplug_props.py / x86_cpu_model_versions.py. > > The classes I find practical are downloading artifact, uncompressing > or extracting archive, and eventually the cloudinit one. But we can probably > use them directly. > I understand this is your PoV with regards to how much value you believe exists in those particular utility modules. The challenge is how to combine other people's PoV with regards to the same matter, and manage them accordingly as a group. One example is how Pavel Dovgaluk may find the "gdb" utility module important. Now consider that Red Hat is vested to have some of the functional / integration tests that run downstream (and which are based on Avocado+Avocado-VT) migrated upstreamed. Then, all of a sudden, a much larger group of "practical classes" arise. Finally, these are, as you mentioned in another thread, utility libraries (such as python/qemu) are commonly needed in other projects (specially those related to QEMU). I understand that suggesting to have more code infrastructure code living in the QEMU would actually make it harder to solve similar problems once. > > Lot of material to discuss :) > Absolutely! Best, - Cleber. > Regards, > > Phil. >
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 7c9d956f113..805e21fc883 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) sd->data[11] = 0x43; sd->data[12] = 0x80; /* Supported group 1 functions */ sd->data[13] = 0x03; + for (i = 0; i < 6; i ++) { new_func = (arg >> (i * 4)) & 0x0f; if (mode && new_func != 0x0f) sd->function_group[i] = new_func; - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); } memset(&sd->data[17], 0, 47); stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));