Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
On 10/23/20 4:02 AM, Bin Meng wrote: Hi Niek, On Thu, Oct 22, 2020 at 11:20 PM Niek Linnenbank 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.
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
Hi Niek, On Thu, Oct 22, 2020 at 11:20 PM Niek Linnenbank 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
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
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 :
> Hi Philippe,
>
> On Wed, Oct 21, 2020 at 6:07 PM Philippe Mathieu-Daudé
> 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é <
> f4...@amsat.org> wrote:
> > >>
> > >> Hi Bin,
> > >>
> > >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
> > >>> From: Bin Meng
> > >>>
> > >>> 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
> > >>> Reviewed-by: Philippe Mathieu-Daudé
> > >>> Tested-by: Sai Pavan Boddu
> > >>> Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
> > >>> Signed-off-by: Philippe Mathieu-Daudé
> > >>> ---
> > >>>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
>
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
Hi Philippe,
On Wed, Oct 21, 2020 at 6:07 PM Philippe Mathieu-Daudé 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é
> > wrote:
> >>
> >> Hi Bin,
> >>
> >> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
> >>> From: Bin Meng
> >>>
> >>> 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
> >>> Reviewed-by: Philippe Mathieu-Daudé
> >>> Tested-by: Sai Pavan Boddu
> >>> Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
> >>> Signed-off-by: Philippe Mathieu-Daudé
> >>> ---
> >>>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
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
On 10/21/20 11:57 AM, Bin Meng wrote:
Hi Philippe,
On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé wrote:
Hi Bin,
On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
From: Bin Meng
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
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Sai Pavan Boddu
Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
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
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
Hi Philippe,
On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé wrote:
>
> Hi Bin,
>
> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
> > From: Bin Meng
> >
> > 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
> > Reviewed-by: Philippe Mathieu-Daudé
> > Tested-by: Sai Pavan Boddu
> > Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > 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
Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
Hi Bin,
On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
From: Bin Meng
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
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Sai Pavan Boddu
Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
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));
[PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
From: Bin Meng
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
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Sai Pavan Boddu
Message-Id: <1598021136-49525-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
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);
}
memset(&sd->data[17], 0, 47);
stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));
--
2.26.2
