* [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard
2021-02-11 22:00 [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Niek Linnenbank
@ 2021-02-11 22:00 ` Niek Linnenbank
2021-02-12 1:15 ` Cleber Rosa
2021-02-12 20:19 ` Willian Rampazzo
2021-02-11 22:00 ` [PATCH 2/2] hw/net/allwinner-sun8i-emac: traverse transmit queue using TX_CUR_DESC register value Niek Linnenbank
2021-02-12 14:10 ` [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Philippe Mathieu-Daudé
2 siblings, 2 replies; 13+ messages in thread
From: Niek Linnenbank @ 2021-02-11 22:00 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, f4bug, b.galvani, Niek Linnenbank, qemu-arm,
Pavel.Dovgaluk, crosa, philmd
Currently the automated acceptance tests for the Orange Pi PC and cubieboard
machines are disabled by default. The tests for both machines require artifacts
that are stored on the apt.armbian.com domain. Unfortunately, some of these artifacts
have been removed from apt.armbian.com and it is uncertain whether more will be removed.
This commit moves the artifacts previously stored on apt.armbian.com to a different
domain that is maintainted by me and retrieves them using the path: '/pub/qemu/<machine>/<artifact>'.
Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
tests/acceptance/boot_linux_console.py | 46 +++++++++-----------------
tests/acceptance/replay_kernel.py | 6 ++--
2 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index eb01286799..12fccbdb37 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -507,15 +507,13 @@ class BootLinuxConsole(LinuxKernelTest):
self.wait_for_console_pattern('Boot successful.')
# TODO user command, for now the uart is stuck
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_cubieboard_initrd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:cubieboard
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/cubieboard/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
@@ -549,15 +547,13 @@ class BootLinuxConsole(LinuxKernelTest):
'system-control@1c00000')
# cubieboard's reboot is not functioning; omit reboot test.
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_cubieboard_sata(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:cubieboard
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/cubieboard/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
@@ -678,15 +674,13 @@ class BootLinuxConsole(LinuxKernelTest):
self.wait_for_console_pattern(
'Give root password for system maintenance')
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_orangepi(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
@@ -705,15 +699,13 @@ class BootLinuxConsole(LinuxKernelTest):
console_pattern = 'Kernel command line: %s' % kernel_command_line
self.wait_for_console_pattern(console_pattern)
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_orangepi_initrd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
@@ -749,24 +741,23 @@ class BootLinuxConsole(LinuxKernelTest):
# Wait for VM to shut down gracefully
self.vm.wait()
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_orangepi_sd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
:avocado: tags=device:sd
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
'/boot/vmlinuz-4.20.7-sunxi')
dtb_path = '/usr/lib/linux-image-dev-sunxi/sun8i-h3-orangepi-pc.dtb'
dtb_path = self.extract_from_deb(deb_path, dtb_path)
- rootfs_url = ('http://storage.kernelci.org/images/rootfs/buildroot/'
- 'kci-2019.02/armel/base/rootfs.ext2.xz')
+ # Rootfs is based on buildroot 2019.02 from kernelci.org
+ rootfs_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
+ 'rootfs.ext2.xz')
rootfs_hash = '692510cb625efda31640d1de0a8d60e26040f061'
rootfs_path_xz = self.fetch_asset(rootfs_url, asset_hash=rootfs_hash)
rootfs_path = os.path.join(self.workdir, 'rootfs.cpio')
@@ -828,8 +819,6 @@ class BootLinuxConsole(LinuxKernelTest):
'to <orangepipc>')
self.wait_for_console_pattern('Starting Load Kernel Modules...')
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
@skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
@skipUnless(P7ZIP_AVAILABLE, '7z not installed')
def test_arm_orangepi_bionic_19_11(self):
@@ -840,7 +829,7 @@ class BootLinuxConsole(LinuxKernelTest):
"""
# This test download a 196MB compressed image and expand it to 1GB
- image_url = ('https://dl.armbian.com/orangepipc/archive/'
+ image_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
'Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.7z')
image_hash = '196a8ffb72b0123d92cea4a070894813d305c71e'
image_path_7z = self.fetch_asset(image_url, asset_hash=image_hash)
@@ -851,8 +840,6 @@ class BootLinuxConsole(LinuxKernelTest):
self.do_test_arm_orangepi_uboot_armbian(image_path)
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
@skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
def test_arm_orangepi_bionic_20_08(self):
"""
@@ -865,7 +852,7 @@ class BootLinuxConsole(LinuxKernelTest):
# to 1036 MiB, but the underlying filesystem is 1552 MiB...
# As we expand it to 2 GiB we are safe.
- image_url = ('https://dl.armbian.com/orangepipc/archive/'
+ image_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
'Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz')
image_hash = ('b4d6775f5673486329e45a0586bf06b6'
'dbe792199fd182ac6b9c7bb6c7d3e6dd')
@@ -884,8 +871,7 @@ class BootLinuxConsole(LinuxKernelTest):
:avocado: tags=device:sd
"""
# This test download a 304MB compressed image and expand it to 2GB
- deb_url = ('http://snapshot.debian.org/archive/debian/'
- '20200108T145233Z/pool/main/u/u-boot/'
+ deb_url = ('http://www.freenos.org/pub/qemu/orangepi-pc/'
'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
deb_hash = 'f67f404a80753ca3d1258f13e38f2b060e13db99'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
diff --git a/tests/acceptance/replay_kernel.py b/tests/acceptance/replay_kernel.py
index c1cb862468..6021b7427d 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -177,15 +177,13 @@ class ReplayKernelNormal(ReplayKernelBase):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=1)
@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
- @skipUnless(os.getenv('ARMBIAN_ARTIFACTS_CACHED'),
- 'Test artifacts fetched from unreliable apt.armbian.com')
def test_arm_cubieboard_initrd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:cubieboard
"""
- deb_url = ('https://apt.armbian.com/pool/main/l/'
- 'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
+ deb_url = ('http://www.freenos.org/pub/qemu/cubieboard/'
+ 'linux-image-dev-sunxi_5.75_armhf.deb')
deb_hash = '1334c29c44d984ffa05ed10de8c3361f33d78315'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard
2021-02-11 22:00 ` [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard Niek Linnenbank
@ 2021-02-12 1:15 ` Cleber Rosa
2021-02-12 20:19 ` Willian Rampazzo
1 sibling, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-02-12 1:15 UTC (permalink / raw)
To: Niek Linnenbank
Cc: peter.maydell, qemu-devel, f4bug, b.galvani, qemu-arm,
Pavel.Dovgaluk, philmd
[-- Attachment #1: Type: text/plain, Size: 3342 bytes --]
On Thu, Feb 11, 2021 at 11:00:54PM +0100, Niek Linnenbank wrote:
> Currently the automated acceptance tests for the Orange Pi PC and cubieboard
> machines are disabled by default. The tests for both machines require artifacts
> that are stored on the apt.armbian.com domain. Unfortunately, some of these artifacts
> have been removed from apt.armbian.com and it is uncertain whether more will be removed.
>
> This commit moves the artifacts previously stored on apt.armbian.com to a different
> domain that is maintainted by me and retrieves them using the path: '/pub/qemu/<machine>/<artifact>'.
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
> tests/acceptance/boot_linux_console.py | 46 +++++++++-----------------
> tests/acceptance/replay_kernel.py | 6 ++--
> 2 files changed, 18 insertions(+), 34 deletions(-)
Tested with:
$ export AVOCADO_ALLOW_LARGE_STORAGE=1
$ ./tests/venv/bin/avocado run \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi\$ \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_19_11 \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08 \
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9 \
tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_arm_cubieboard_initrd
Resulting in:
JOB ID : f767a43482aa90545e740120a44cba64ab07dd3a
JOB LOG : /home/cleber/avocado/job-results/job-2021-02-11T20.07-f767a43/job.log
(1/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd: PASS (23.49 s)
(2/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata: PASS (21.98 s)
(3/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi: PASS (12.37 s)
(4/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd: PASS (33.78 s)
(5/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: PASS (45.47 s)
(6/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_19_11: PASS (69.69 s)
(7/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08: PASS (89.06 s)
(8/9) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: PASS (40.82 s)
(9/9) tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_arm_cubieboard_initrd: PASS (61.19 s)
RESULTS : PASS 9 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 398.87 s
Both "test_arm_orangepi_bionic_19_11" and
"test_arm_orangepi_bionic_20_08" barely made within the 90 seconds
timeout. I realize that the system I used is quite limited, so I'll
let others reason about the increase of the timeout.
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard
2021-02-11 22:00 ` [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard Niek Linnenbank
2021-02-12 1:15 ` Cleber Rosa
@ 2021-02-12 20:19 ` Willian Rampazzo
1 sibling, 0 replies; 13+ messages in thread
From: Willian Rampazzo @ 2021-02-12 20:19 UTC (permalink / raw)
To: Niek Linnenbank
Cc: Peter Maydell, qemu-devel, Philippe Mathieu-Daudé,
b.galvani, qemu-arm, Pavel Dovgalyuk, Cleber Rosa Junior,
Philippe Mathieu Daude
On Thu, Feb 11, 2021 at 7:09 PM Niek Linnenbank
<nieklinnenbank@gmail.com> wrote:
>
> Currently the automated acceptance tests for the Orange Pi PC and cubieboard
> machines are disabled by default. The tests for both machines require artifacts
> that are stored on the apt.armbian.com domain. Unfortunately, some of these artifacts
> have been removed from apt.armbian.com and it is uncertain whether more will be removed.
>
> This commit moves the artifacts previously stored on apt.armbian.com to a different
> domain that is maintainted by me and retrieves them using the path: '/pub/qemu/<machine>/<artifact>'.
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
> tests/acceptance/boot_linux_console.py | 46 +++++++++-----------------
> tests/acceptance/replay_kernel.py | 6 ++--
> 2 files changed, 18 insertions(+), 34 deletions(-)
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] hw/net/allwinner-sun8i-emac: traverse transmit queue using TX_CUR_DESC register value
2021-02-11 22:00 [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Niek Linnenbank
2021-02-11 22:00 ` [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard Niek Linnenbank
@ 2021-02-11 22:00 ` Niek Linnenbank
2021-02-11 23:29 ` Philippe Mathieu-Daudé
2021-02-12 14:10 ` [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Philippe Mathieu-Daudé
2 siblings, 1 reply; 13+ messages in thread
From: Niek Linnenbank @ 2021-02-11 22:00 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, f4bug, b.galvani, Niek Linnenbank, qemu-arm,
Pavel.Dovgaluk, crosa, philmd
Currently the emulated EMAC for sun8i always traverses the transmit queue
from the head when transferring packets. It searches for a list of consecutive
descriptors whichs are flagged as ready for processing and transmits their payloads
accordingly. The controller stops processing once it finds a descriptor that is not
marked ready.
While the above behaviour works in most situations, it is not the same as the actual
EMAC in hardware. Actual hardware uses the TX_CUR_DESC register value to keep track
of the last position in the transmit queue and continues processing from that position
when software triggers the start of DMA processing. The currently emulated behaviour can
lead to packet loss on transmit when software fills the transmit queue with ready
descriptors that overlap the tail of the circular list.
This commit modifies the emulated EMAC for sun8i such that it processes
the transmit queue using the TX_CUR_DESC register in the same way as hardware.
Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
hw/net/allwinner-sun8i-emac.c | 58 +++++++++++++++++++----------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
index 042768922c..e586c147e5 100644
--- a/hw/net/allwinner-sun8i-emac.c
+++ b/hw/net/allwinner-sun8i-emac.c
@@ -339,35 +339,40 @@ static void allwinner_sun8i_emac_update_irq(AwSun8iEmacState *s)
qemu_set_irq(s->irq, (s->int_sta & s->int_en) != 0);
}
-static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s,
- FrameDescriptor *desc,
- size_t min_size)
+static bool allwinner_sun8i_emac_desc_owned(FrameDescriptor *desc,
+ size_t min_buf_size)
{
- uint32_t paddr = desc->next;
+ return (desc->status & DESC_STATUS_CTL) && (min_buf_size == 0 ||
+ (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_buf_size);
+}
- dma_memory_read(&s->dma_as, paddr, desc, sizeof(*desc));
+static void allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s,
+ FrameDescriptor *desc,
+ uint32_t phys_addr)
+{
+ dma_memory_read(&s->dma_as, phys_addr, desc, sizeof(*desc));
+}
- if ((desc->status & DESC_STATUS_CTL) &&
- (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) {
- return paddr;
- } else {
- return 0;
- }
+static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s,
+ FrameDescriptor *desc)
+{
+ const uint32_t nxt = desc->next;
+ allwinner_sun8i_emac_get_desc(s, desc, nxt);
+ return nxt;
}
-static uint32_t allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s,
- FrameDescriptor *desc,
- uint32_t start_addr,
- size_t min_size)
+static uint32_t allwinner_sun8i_emac_find_desc(AwSun8iEmacState *s,
+ FrameDescriptor *desc,
+ uint32_t start_addr,
+ size_t min_size)
{
uint32_t desc_addr = start_addr;
/* Note that the list is a cycle. Last entry points back to the head. */
while (desc_addr != 0) {
- dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc));
+ allwinner_sun8i_emac_get_desc(s, desc, desc_addr);
- if ((desc->status & DESC_STATUS_CTL) &&
- (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) {
+ if (allwinner_sun8i_emac_desc_owned(desc, min_size)) {
return desc_addr;
} else if (desc->next == start_addr) {
break;
@@ -383,14 +388,14 @@ static uint32_t allwinner_sun8i_emac_rx_desc(AwSun8iEmacState *s,
FrameDescriptor *desc,
size_t min_size)
{
- return allwinner_sun8i_emac_get_desc(s, desc, s->rx_desc_curr, min_size);
+ return allwinner_sun8i_emac_find_desc(s, desc, s->rx_desc_curr, min_size);
}
static uint32_t allwinner_sun8i_emac_tx_desc(AwSun8iEmacState *s,
- FrameDescriptor *desc,
- size_t min_size)
+ FrameDescriptor *desc)
{
- return allwinner_sun8i_emac_get_desc(s, desc, s->tx_desc_head, min_size);
+ allwinner_sun8i_emac_get_desc(s, desc, s->tx_desc_curr);
+ return s->tx_desc_curr;
}
static void allwinner_sun8i_emac_flush_desc(AwSun8iEmacState *s,
@@ -470,7 +475,8 @@ static ssize_t allwinner_sun8i_emac_receive(NetClientState *nc,
bytes_left -= desc_bytes;
/* Move to the next descriptor */
- s->rx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc, 64);
+ s->rx_desc_curr = allwinner_sun8i_emac_find_desc(s, &desc, desc.next,
+ AW_SUN8I_EMAC_MIN_PKT_SZ);
if (!s->rx_desc_curr) {
/* Not enough buffer space available */
s->int_sta |= INT_STA_RX_BUF_UA;
@@ -495,10 +501,10 @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s)
size_t transmitted = 0;
static uint8_t packet_buf[2048];
- s->tx_desc_curr = allwinner_sun8i_emac_tx_desc(s, &desc, 0);
+ s->tx_desc_curr = allwinner_sun8i_emac_tx_desc(s, &desc);
/* Read all transmit descriptors */
- while (s->tx_desc_curr != 0) {
+ while (allwinner_sun8i_emac_desc_owned(&desc, 0)) {
/* Read from physical memory into packet buffer */
bytes = desc.status2 & DESC_STATUS2_BUF_SIZE_MASK;
@@ -524,7 +530,7 @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s)
packet_bytes = 0;
transmitted++;
}
- s->tx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc, 0);
+ s->tx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc);
}
/* Raise transmit completed interrupt */
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] hw/net/allwinner-sun8i-emac: traverse transmit queue using TX_CUR_DESC register value
2021-02-11 22:00 ` [PATCH 2/2] hw/net/allwinner-sun8i-emac: traverse transmit queue using TX_CUR_DESC register value Niek Linnenbank
@ 2021-02-11 23:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-11 23:29 UTC (permalink / raw)
To: Niek Linnenbank, qemu-devel
Cc: peter.maydell, Stefan Weil, Jason Wang, f4bug, b.galvani,
qemu-arm, Pavel.Dovgaluk, crosa
On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> Currently the emulated EMAC for sun8i always traverses the transmit queue
> from the head when transferring packets. It searches for a list of consecutive
> descriptors whichs are flagged as ready for processing and transmits their payloads
> accordingly. The controller stops processing once it finds a descriptor that is not
> marked ready.
>
> While the above behaviour works in most situations, it is not the same as the actual
> EMAC in hardware. Actual hardware uses the TX_CUR_DESC register value to keep track
> of the last position in the transmit queue and continues processing from that position
> when software triggers the start of DMA processing. The currently emulated behaviour can
> lead to packet loss on transmit when software fills the transmit queue with ready
> descriptors that overlap the tail of the circular list.
>
> This commit modifies the emulated EMAC for sun8i such that it processes
> the transmit queue using the TX_CUR_DESC register in the same way as hardware.
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
> hw/net/allwinner-sun8i-emac.c | 58 +++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 26 deletions(-)
LGTM but I'd feel safer with another review on top.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
> index 042768922c..e586c147e5 100644
> --- a/hw/net/allwinner-sun8i-emac.c
> +++ b/hw/net/allwinner-sun8i-emac.c
> @@ -339,35 +339,40 @@ static void allwinner_sun8i_emac_update_irq(AwSun8iEmacState *s)
> qemu_set_irq(s->irq, (s->int_sta & s->int_en) != 0);
> }
>
> -static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s,
> - FrameDescriptor *desc,
> - size_t min_size)
> +static bool allwinner_sun8i_emac_desc_owned(FrameDescriptor *desc,
> + size_t min_buf_size)
> {
> - uint32_t paddr = desc->next;
> + return (desc->status & DESC_STATUS_CTL) && (min_buf_size == 0 ||
> + (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_buf_size);
> +}
>
> - dma_memory_read(&s->dma_as, paddr, desc, sizeof(*desc));
> +static void allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s,
> + FrameDescriptor *desc,
> + uint32_t phys_addr)
> +{
> + dma_memory_read(&s->dma_as, phys_addr, desc, sizeof(*desc));
> +}
>
> - if ((desc->status & DESC_STATUS_CTL) &&
> - (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) {
> - return paddr;
> - } else {
> - return 0;
> - }
> +static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s,
> + FrameDescriptor *desc)
> +{
> + const uint32_t nxt = desc->next;
> + allwinner_sun8i_emac_get_desc(s, desc, nxt);
> + return nxt;
> }
>
> -static uint32_t allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s,
> - FrameDescriptor *desc,
> - uint32_t start_addr,
> - size_t min_size)
> +static uint32_t allwinner_sun8i_emac_find_desc(AwSun8iEmacState *s,
> + FrameDescriptor *desc,
> + uint32_t start_addr,
> + size_t min_size)
> {
> uint32_t desc_addr = start_addr;
>
> /* Note that the list is a cycle. Last entry points back to the head. */
> while (desc_addr != 0) {
> - dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc));
> + allwinner_sun8i_emac_get_desc(s, desc, desc_addr);
>
> - if ((desc->status & DESC_STATUS_CTL) &&
> - (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) {
> + if (allwinner_sun8i_emac_desc_owned(desc, min_size)) {
> return desc_addr;
> } else if (desc->next == start_addr) {
> break;
> @@ -383,14 +388,14 @@ static uint32_t allwinner_sun8i_emac_rx_desc(AwSun8iEmacState *s,
> FrameDescriptor *desc,
> size_t min_size)
> {
> - return allwinner_sun8i_emac_get_desc(s, desc, s->rx_desc_curr, min_size);
> + return allwinner_sun8i_emac_find_desc(s, desc, s->rx_desc_curr, min_size);
> }
>
> static uint32_t allwinner_sun8i_emac_tx_desc(AwSun8iEmacState *s,
> - FrameDescriptor *desc,
> - size_t min_size)
> + FrameDescriptor *desc)
> {
> - return allwinner_sun8i_emac_get_desc(s, desc, s->tx_desc_head, min_size);
> + allwinner_sun8i_emac_get_desc(s, desc, s->tx_desc_curr);
> + return s->tx_desc_curr;
> }
>
> static void allwinner_sun8i_emac_flush_desc(AwSun8iEmacState *s,
> @@ -470,7 +475,8 @@ static ssize_t allwinner_sun8i_emac_receive(NetClientState *nc,
> bytes_left -= desc_bytes;
>
> /* Move to the next descriptor */
> - s->rx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc, 64);
> + s->rx_desc_curr = allwinner_sun8i_emac_find_desc(s, &desc, desc.next,
> + AW_SUN8I_EMAC_MIN_PKT_SZ);
> if (!s->rx_desc_curr) {
> /* Not enough buffer space available */
> s->int_sta |= INT_STA_RX_BUF_UA;
> @@ -495,10 +501,10 @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s)
> size_t transmitted = 0;
> static uint8_t packet_buf[2048];
>
> - s->tx_desc_curr = allwinner_sun8i_emac_tx_desc(s, &desc, 0);
> + s->tx_desc_curr = allwinner_sun8i_emac_tx_desc(s, &desc);
>
> /* Read all transmit descriptors */
> - while (s->tx_desc_curr != 0) {
> + while (allwinner_sun8i_emac_desc_owned(&desc, 0)) {
>
> /* Read from physical memory into packet buffer */
> bytes = desc.status2 & DESC_STATUS2_BUF_SIZE_MASK;
> @@ -524,7 +530,7 @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s)
> packet_bytes = 0;
> transmitted++;
> }
> - s->tx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc, 0);
> + s->tx_desc_curr = allwinner_sun8i_emac_next_desc(s, &desc);
> }
>
> /* Raise transmit completed interrupt */
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-11 22:00 [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Niek Linnenbank
2021-02-11 22:00 ` [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard Niek Linnenbank
2021-02-11 22:00 ` [PATCH 2/2] hw/net/allwinner-sun8i-emac: traverse transmit queue using TX_CUR_DESC register value Niek Linnenbank
@ 2021-02-12 14:10 ` Philippe Mathieu-Daudé
2021-02-15 20:30 ` Niek Linnenbank
2021-02-16 9:48 ` Daniel P. Berrangé
2 siblings, 2 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-12 14:10 UTC (permalink / raw)
To: Niek Linnenbank, Daniel P . Berrange, Alex Bennée, Stefan Hajnoczi
Cc: peter.maydell, Thomas Huth, Markus Armbruster, qemu-devel, f4bug,
b.galvani, qemu-arm, Pavel.Dovgaluk, crosa
Hi Niek and QEMU community,
On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> The following are maintenance patches for the Allwinner H3. The first patch
> is a proposal to relocate the binary artifacts of the acceptance tests away
> from the apt.armbian.com domain. In the past we had problems with artifacts being
> removed, and now the recently added Armbian 20.08.1 image has been removed as well:
>
> $ wget https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> Connecting to dl.armbian.com (dl.armbian.com)|2605:7900:20::5|:443... connected.
> ...
> HTTP request sent, awaiting response... 404 Not Found
> 2021-02-11 22:34:45 ERROR 404: Not Found.
>
> I've now added the artifacts to a server maintained by me. The machine has a stable
> uptime of several years, ~100Mbit bandwidth and plenty of available storage.
> Also for other artifacts if needed. I'm open to discuss if there is a proposal
> for a better location for these artifacts or a more generic qemu location.
Thanks for trying to fix this long standing problem.
While this works in your case, this doesn't scale to the community,
as not all contributors have access to such hardware and bandwidth /
storage.
While your first patch is useful in showing where the artifacts are
stored doesn't matter - as long as we use cryptographic hashes - I
think it is a step in the wrong direction, so I am not keen on
accepting it.
My personal view is that any contributor should have the same
possibilities to add tests to the project. Now I am also open to
discuss with the others :) I might be proven wrong, and it could
be better to rely on good willing contributors rather than having
nothing useful at all.
Regards,
Phil.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-12 14:10 ` [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Philippe Mathieu-Daudé
@ 2021-02-15 20:30 ` Niek Linnenbank
2021-02-16 9:48 ` Daniel P. Berrangé
1 sibling, 0 replies; 13+ messages in thread
From: Niek Linnenbank @ 2021-02-15 20:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: peter.maydell, Thomas Huth, Daniel P . Berrange, Pavel.Dovgaluk,
Markus Armbruster, qemu-devel, f4bug, b.galvani, qemu-arm,
Stefan Hajnoczi, crosa, Alex Bennée
[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]
Op vr 12 feb. 2021 15:10 schreef Philippe Mathieu-Daudé <philmd@redhat.com>:
> Hi Niek and QEMU community,
>
> On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> > The following are maintenance patches for the Allwinner H3. The first
> patch
> > is a proposal to relocate the binary artifacts of the acceptance tests
> away
> > from the apt.armbian.com domain. In the past we had problems with
> artifacts being
> > removed, and now the recently added Armbian 20.08.1 image has been
> removed as well:
> >
> > $ wget
> https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> > Connecting to dl.armbian.com (dl.armbian.com)|2605:7900:20::5|:443...
> connected.
> > ...
> > HTTP request sent, awaiting response... 404 Not Found
> > 2021-02-11 22:34:45 ERROR 404: Not Found.
> >
> > I've now added the artifacts to a server maintained by me. The machine
> has a stable
> > uptime of several years, ~100Mbit bandwidth and plenty of available
> storage.
> > Also for other artifacts if needed. I'm open to discuss if there is a
> proposal
> > for a better location for these artifacts or a more generic qemu
> location.
>
> Thanks for trying to fix this long standing problem.
>
> While this works in your case, this doesn't scale to the community,
> as not all contributors have access to such hardware and bandwidth /
> storage.
>
> While your first patch is useful in showing where the artifacts are
> stored doesn't matter - as long as we use cryptographic hashes - I
> think it is a step in the wrong direction, so I am not keen on
> accepting it.
> My personal view is that any contributor should have the same
> possibilities to add tests to the project.
Hi Philippe,
I see your point. How about I simply upload the artifacts to github
instead? There are already multiple tests right now that use artifacts
stored on github. And github is available to everyone. For me that would
work fine. If you agree, I can respin the patch.
Regards
Niek
Now I am also open to
> discuss with the others :) I might be proven wrong, and it could
> be better to rely on good willing contributors rather than having
> nothing useful at all.
> Regards,
>
> Phil.
>
>
[-- Attachment #2: Type: text/html, Size: 3718 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-12 14:10 ` [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests Philippe Mathieu-Daudé
2021-02-15 20:30 ` Niek Linnenbank
@ 2021-02-16 9:48 ` Daniel P. Berrangé
2021-02-17 20:57 ` Niek Linnenbank
1 sibling, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2021-02-16 9:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: peter.maydell, Thomas Huth, Pavel.Dovgaluk, Markus Armbruster,
qemu-devel, b.galvani, Niek Linnenbank, qemu-arm,
Stefan Hajnoczi, crosa, Alex Bennée, f4bug
On Fri, Feb 12, 2021 at 03:10:00PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Niek and QEMU community,
>
> On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> > The following are maintenance patches for the Allwinner H3. The first patch
> > is a proposal to relocate the binary artifacts of the acceptance tests away
> > from the apt.armbian.com domain. In the past we had problems with artifacts being
> > removed, and now the recently added Armbian 20.08.1 image has been removed as well:
> >
> > $ wget https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> > Connecting to dl.armbian.com (dl.armbian.com)|2605:7900:20::5|:443... connected.
> > ...
> > HTTP request sent, awaiting response... 404 Not Found
> > 2021-02-11 22:34:45 ERROR 404: Not Found.
> >
> > I've now added the artifacts to a server maintained by me. The machine has a stable
> > uptime of several years, ~100Mbit bandwidth and plenty of available storage.
> > Also for other artifacts if needed. I'm open to discuss if there is a proposal
> > for a better location for these artifacts or a more generic qemu location.
>
> Thanks for trying to fix this long standing problem.
>
> While this works in your case, this doesn't scale to the community,
> as not all contributors have access to such hardware and bandwidth /
> storage.
>
> While your first patch is useful in showing where the artifacts are
> stored doesn't matter - as long as we use cryptographic hashes - I
> think it is a step in the wrong direction, so I am not keen on
> accepting it.
>
> My personal view is that any contributor should have the same
> possibilities to add tests to the project. Now I am also open to
> discuss with the others :) I might be proven wrong, and it could
> be better to rely on good willing contributors rather than having
> nothing useful at all.
There aren't many options here
1. Rely on a vendor to provide stable download URLs for images
2. QEMU host all images we use in testing
3. Contributor finds some site to upload images to
For the armbian images we rely on (1), but the URLs didn't turn out to be
stable. In fact no OS vendor seems to have guaranteed stable URLs forever,
regardless of distro, though most eventually do have an archive site that
has good life. Armbian was an exception in this respect IIUC.
(2) would solve the long term stability problem as QEMU would be in full
control, and could open it up for any images we need. The big challenge
there is that QEMU now owns the license compliance problem. Merely uploading
binary images/packages without the corresponding source is generally a license
violation. QEMU could provide hosting, but we need to be clear about the fact
that we now own the license compliance problem ourselves. Many sites hosting
images simply ignore this problem, but that doesn't make it right.
This series is proposing (3), with a site the contributor happens to control
themselves, but using a free 3rd party hosting site is no different really.
Again there is a the same need for license compliance, but it is now the
responsibility of the user, not QEMU project.
In this http://www.freenos.org/pub/qemu/cubieboard/ site I can't even see a
directory listing, so even if corresponding source does exist in this server,
I can't find it.
The isn't really a problem for QEMU CI to consume the images, but as a free
software developer I don't like encouraging practices that are not compliant
with licensing reuqirement.
It is an open question whether the (3) is really better than (1) in terms
of URL stability long term, especially if running off a user's personal
server.
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] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-16 9:48 ` Daniel P. Berrangé
@ 2021-02-17 20:57 ` Niek Linnenbank
2021-02-19 18:24 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Niek Linnenbank @ 2021-02-17 20:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: peter.maydell, Thomas Huth, Pavel.Dovgaluk,
Philippe Mathieu-Daudé,
Markus Armbruster, qemu-devel, b.galvani, qemu-arm,
Stefan Hajnoczi, crosa, Alex Bennée, f4bug
[-- Attachment #1: Type: text/plain, Size: 4784 bytes --]
Hi Daniel, Philippe,
Op di 16 feb. 2021 10:49 schreef Daniel P. Berrangé <berrange@redhat.com>:
> On Fri, Feb 12, 2021 at 03:10:00PM +0100, Philippe Mathieu-Daudé wrote:
> > Hi Niek and QEMU community,
> >
> > On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> > > The following are maintenance patches for the Allwinner H3. The first
> patch
> > > is a proposal to relocate the binary artifacts of the acceptance tests
> away
> > > from the apt.armbian.com domain. In the past we had problems with
> artifacts being
> > > removed, and now the recently added Armbian 20.08.1 image has been
> removed as well:
> > >
> > > $ wget
> https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> > > Connecting to dl.armbian.com (dl.armbian.com)|2605:7900:20::5|:443...
> connected.
> > > ...
> > > HTTP request sent, awaiting response... 404 Not Found
> > > 2021-02-11 22:34:45 ERROR 404: Not Found.
> > >
> > > I've now added the artifacts to a server maintained by me. The machine
> has a stable
> > > uptime of several years, ~100Mbit bandwidth and plenty of available
> storage.
> > > Also for other artifacts if needed. I'm open to discuss if there is a
> proposal
> > > for a better location for these artifacts or a more generic qemu
> location.
> >
> > Thanks for trying to fix this long standing problem.
> >
> > While this works in your case, this doesn't scale to the community,
> > as not all contributors have access to such hardware and bandwidth /
> > storage.
> >
> > While your first patch is useful in showing where the artifacts are
> > stored doesn't matter - as long as we use cryptographic hashes - I
> > think it is a step in the wrong direction, so I am not keen on
> > accepting it.
> >
> > My personal view is that any contributor should have the same
> > possibilities to add tests to the project. Now I am also open to
> > discuss with the others :) I might be proven wrong, and it could
> > be better to rely on good willing contributors rather than having
> > nothing useful at all.
>
> There aren't many options here
>
> 1. Rely on a vendor to provide stable download URLs for images
>
> 2. QEMU host all images we use in testing
>
> 3. Contributor finds some site to upload images to
>
>
> For the armbian images we rely on (1), but the URLs didn't turn out to be
> stable. In fact no OS vendor seems to have guaranteed stable URLs forever,
> regardless of distro, though most eventually do have an archive site that
> has good life. Armbian was an exception in this respect IIUC.
>
> (2) would solve the long term stability problem as QEMU would be in full
> control, and could open it up for any images we need. The big challenge
> there is that QEMU now owns the license compliance problem. Merely
> uploading
> binary images/packages without the corresponding source is generally a
> license
> violation. QEMU could provide hosting, but we need to be clear about the
> fact
> that we now own the license compliance problem ourselves. Many sites
> hosting
> images simply ignore this problem, but that doesn't make it right.
>
>
> This series is proposing (3), with a site the contributor happens to
> control
> themselves, but using a free 3rd party hosting site is no different really.
> Again there is a the same need for license compliance, but it is now the
> responsibility of the user, not QEMU project.
>
> In this http://www.freenos.org/pub/qemu/cubieboard/ site I can't even see
> a
> directory listing, so even if corresponding source does exist in this
> server,
> I can't find it.
>
> The isn't really a problem for QEMU CI to consume the images, but as a free
> software developer I don't like encouraging practices that are not
> compliant
> with licensing reuqirement.
>
> It is an open question whether the (3) is really better than (1) in terms
> of URL stability long term, especially if running off a user's personal
> server.
>
I understand your concerns. My goal here was to be able to re-activate the
orangepi tests, so we can capture bugs early on. So what I can do instead
is:
- update the patch to use github to store the artifacts, and their
licenses (other tests also use github)
- or change the patch to use updated armbian links that work (for now)
If we can agree on either of these solutions, so the orangepi tests can be
re-activated, that would be great.
Kind regards,
Niek
> 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 :|
>
>
[-- Attachment #2: Type: text/html, Size: 6915 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-17 20:57 ` Niek Linnenbank
@ 2021-02-19 18:24 ` Philippe Mathieu-Daudé
2021-02-19 18:58 ` Cleber Rosa
2021-02-21 20:40 ` Niek Linnenbank
0 siblings, 2 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-19 18:24 UTC (permalink / raw)
To: Niek Linnenbank, Daniel P. Berrangé
Cc: peter.maydell, Thomas Huth, Pavel.Dovgaluk, Markus Armbruster,
Wainer dos Santos Moschetta, qemu-devel, b.galvani, qemu-arm,
Stefan Hajnoczi, crosa, Willian Rampazzo, Alex Bennée,
f4bug
Hi Niek,
On 2/17/21 9:57 PM, Niek Linnenbank wrote:
> Hi Daniel, Philippe,
>
> Op di 16 feb. 2021 10:49 schreef Daniel P. Berrangé <berrange@redhat.com
> <mailto:berrange@redhat.com>>:
>
> On Fri, Feb 12, 2021 at 03:10:00PM +0100, Philippe Mathieu-Daudé wrote:
> > Hi Niek and QEMU community,
> >
> > On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> > > The following are maintenance patches for the Allwinner H3. The
> first patch
> > > is a proposal to relocate the binary artifacts of the acceptance
> tests away
> > > from the apt.armbian.com <http://apt.armbian.com> domain. In the
> past we had problems with artifacts being
> > > removed, and now the recently added Armbian 20.08.1 image has
> been removed as well:
> > >
> > > $ wget
> https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> <https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz>
> > > Connecting to dl.armbian.com <http://dl.armbian.com>
> (dl.armbian.com <http://dl.armbian.com>)|2605:7900:20::5|:443...
> connected.
> > > ...
> > > HTTP request sent, awaiting response... 404 Not Found
> > > 2021-02-11 22:34:45 ERROR 404: Not Found.
> > >
> > > I've now added the artifacts to a server maintained by me. The
> machine has a stable
> > > uptime of several years, ~100Mbit bandwidth and plenty of
> available storage.
> > > Also for other artifacts if needed. I'm open to discuss if there
> is a proposal
> > > for a better location for these artifacts or a more generic qemu
> location.
> >
> > Thanks for trying to fix this long standing problem.
> >
> > While this works in your case, this doesn't scale to the community,
> > as not all contributors have access to such hardware and bandwidth /
> > storage.
> >
> > While your first patch is useful in showing where the artifacts are
> > stored doesn't matter - as long as we use cryptographic hashes - I
> > think it is a step in the wrong direction, so I am not keen on
> > accepting it.
> >
> > My personal view is that any contributor should have the same
> > possibilities to add tests to the project. Now I am also open to
> > discuss with the others :) I might be proven wrong, and it could
> > be better to rely on good willing contributors rather than having
> > nothing useful at all.
>
> There aren't many options here
>
> 1. Rely on a vendor to provide stable download URLs for images
>
> 2. QEMU host all images we use in testing
>
> 3. Contributor finds some site to upload images to
>
>
> For the armbian images we rely on (1), but the URLs didn't turn out
> to be
> stable. In fact no OS vendor seems to have guaranteed stable URLs
> forever,
> regardless of distro, though most eventually do have an archive site
> that
> has good life. Armbian was an exception in this respect IIUC.
>
> (2) would solve the long term stability problem as QEMU would be in full
> control, and could open it up for any images we need. The big challenge
> there is that QEMU now owns the license compliance problem. Merely
> uploading
> binary images/packages without the corresponding source is generally
> a license
> violation. QEMU could provide hosting, but we need to be clear about
> the fact
> that we now own the license compliance problem ourselves. Many sites
> hosting
> images simply ignore this problem, but that doesn't make it right.
>
>
> This series is proposing (3), with a site the contributor happens to
> control
> themselves, but using a free 3rd party hosting site is no different
> really.
> Again there is a the same need for license compliance, but it is now the
> responsibility of the user, not QEMU project.
>
> In this http://www.freenos.org/pub/qemu/cubieboard/
> <http://www.freenos.org/pub/qemu/cubieboard/> site I can't even see a
> directory listing, so even if corresponding source does exist in
> this server,
> I can't find it.
>
> The isn't really a problem for QEMU CI to consume the images, but as
> a free
> software developer I don't like encouraging practices that are not
> compliant
> with licensing reuqirement.
>
> It is an open question whether the (3) is really better than (1) in
> terms
> of URL stability long term, especially if running off a user's personal
> server.
>
>
> I understand your concerns. My goal here was to be able to re-activate
> the orangepi tests, so we can capture bugs early on.
I hope you understand the concern I have is not with you in particular,
and I used your case to start a discussion with the QEMU community.
FWIW I missed the URL change because I still have the image cached in
Avocado so my testing ran fine. Which makes me wonder...
Cleber, Willian, should Avocado display information about cached
artifacts? Such "Using artifact downloaded 7 months ago".
> So what I can do
> instead is:
>
> - update the patch to use github to store the artifacts, and their
> licenses (other tests also use github)
Until there is better solutions, this is the option I prefer.
> - or change the patch to use updated armbian links that work (for now)
>
> If we can agree on either of these solutions, so the orangepi tests can
> be re-activated, that would be great.
>
> Kind regards,
> Niek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-19 18:24 ` Philippe Mathieu-Daudé
@ 2021-02-19 18:58 ` Cleber Rosa
2021-02-21 20:40 ` Niek Linnenbank
1 sibling, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-02-19 18:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: peter.maydell, Thomas Huth, Daniel P. Berrangé,
Pavel.Dovgaluk, Markus Armbruster, Wainer dos Santos Moschetta,
qemu-devel, b.galvani, Niek Linnenbank, qemu-arm,
Stefan Hajnoczi, Willian Rampazzo, Alex Bennée, f4bug
[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]
On Fri, Feb 19, 2021 at 07:24:01PM +0100, Philippe Mathieu-Daudé wrote:
>
> I hope you understand the concern I have is not with you in particular,
> and I used your case to start a discussion with the QEMU community.
>
> FWIW I missed the URL change because I still have the image cached in
> Avocado so my testing ran fine. Which makes me wonder...
>
> Cleber, Willian, should Avocado display information about cached
> artifacts? Such "Using artifact downloaded 7 months ago".
>
As of Avocado 85.0 (currently used in QEMU), it's possible to set the
"expire" parameter to "fetch_asset", see:
https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.fetch_asset
In this case, if we want assets to not be used if they're are 30 days
or older, that could be set to 86400. The expired asset not being used,
and then not being able to be fetched again, would cause a test to be
canceled.
Cache browsing/listing/manipulation using the "avocado assets" command
is planned for Avocado 86.0, see:
https://github.com/avocado-framework/avocado/issues/4311
> > So what I can do
> > instead is:
> >
> > - update the patch to use github to store the artifacts, and their
> > licenses (other tests also use github)
>
> Until there is better solutions, this is the option I prefer.
>
+1.
Regards,
- Cleber.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
2021-02-19 18:24 ` Philippe Mathieu-Daudé
2021-02-19 18:58 ` Cleber Rosa
@ 2021-02-21 20:40 ` Niek Linnenbank
1 sibling, 0 replies; 13+ messages in thread
From: Niek Linnenbank @ 2021-02-21 20:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Daniel P. Berrangé,
Pavel.Dovgaluk, Markus Armbruster, Wainer dos Santos Moschetta,
QEMU Developers, Beniamino Galvani, qemu-arm, Stefan Hajnoczi,
Cleber Rosa, Willian Rampazzo, Alex Bennée,
Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 6336 bytes --]
On Fri, Feb 19, 2021 at 7:24 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:
> Hi Niek,
>
> On 2/17/21 9:57 PM, Niek Linnenbank wrote:
> > Hi Daniel, Philippe,
> >
> > Op di 16 feb. 2021 10:49 schreef Daniel P. Berrangé <berrange@redhat.com
> > <mailto:berrange@redhat.com>>:
> >
> > On Fri, Feb 12, 2021 at 03:10:00PM +0100, Philippe Mathieu-Daudé
> wrote:
> > > Hi Niek and QEMU community,
> > >
> > > On 2/11/21 11:00 PM, Niek Linnenbank wrote:
> > > > The following are maintenance patches for the Allwinner H3. The
> > first patch
> > > > is a proposal to relocate the binary artifacts of the acceptance
> > tests away
> > > > from the apt.armbian.com <http://apt.armbian.com> domain. In the
> > past we had problems with artifacts being
> > > > removed, and now the recently added Armbian 20.08.1 image has
> > been removed as well:
> > > >
> > > > $ wget
> >
> https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> > <
> https://dl.armbian.com/orangepipc/archive/Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img.xz
> >
> > > > Connecting to dl.armbian.com <http://dl.armbian.com>
> > (dl.armbian.com <http://dl.armbian.com>)|2605:7900:20::5|:443...
> > connected.
> > > > ...
> > > > HTTP request sent, awaiting response... 404 Not Found
> > > > 2021-02-11 22:34:45 ERROR 404: Not Found.
> > > >
> > > > I've now added the artifacts to a server maintained by me. The
> > machine has a stable
> > > > uptime of several years, ~100Mbit bandwidth and plenty of
> > available storage.
> > > > Also for other artifacts if needed. I'm open to discuss if there
> > is a proposal
> > > > for a better location for these artifacts or a more generic qemu
> > location.
> > >
> > > Thanks for trying to fix this long standing problem.
> > >
> > > While this works in your case, this doesn't scale to the community,
> > > as not all contributors have access to such hardware and bandwidth
> /
> > > storage.
> > >
> > > While your first patch is useful in showing where the artifacts are
> > > stored doesn't matter - as long as we use cryptographic hashes - I
> > > think it is a step in the wrong direction, so I am not keen on
> > > accepting it.
> > >
> > > My personal view is that any contributor should have the same
> > > possibilities to add tests to the project. Now I am also open to
> > > discuss with the others :) I might be proven wrong, and it could
> > > be better to rely on good willing contributors rather than having
> > > nothing useful at all.
> >
> > There aren't many options here
> >
> > 1. Rely on a vendor to provide stable download URLs for images
> >
> > 2. QEMU host all images we use in testing
> >
> > 3. Contributor finds some site to upload images to
> >
> >
> > For the armbian images we rely on (1), but the URLs didn't turn out
> > to be
> > stable. In fact no OS vendor seems to have guaranteed stable URLs
> > forever,
> > regardless of distro, though most eventually do have an archive site
> > that
> > has good life. Armbian was an exception in this respect IIUC.
> >
> > (2) would solve the long term stability problem as QEMU would be in
> full
> > control, and could open it up for any images we need. The big
> challenge
> > there is that QEMU now owns the license compliance problem. Merely
> > uploading
> > binary images/packages without the corresponding source is generally
> > a license
> > violation. QEMU could provide hosting, but we need to be clear about
> > the fact
> > that we now own the license compliance problem ourselves. Many sites
> > hosting
> > images simply ignore this problem, but that doesn't make it right.
> >
> >
> > This series is proposing (3), with a site the contributor happens to
> > control
> > themselves, but using a free 3rd party hosting site is no different
> > really.
> > Again there is a the same need for license compliance, but it is now
> the
> > responsibility of the user, not QEMU project.
> >
> > In this http://www.freenos.org/pub/qemu/cubieboard/
> > <http://www.freenos.org/pub/qemu/cubieboard/> site I can't even see
> a
> > directory listing, so even if corresponding source does exist in
> > this server,
> > I can't find it.
> >
> > The isn't really a problem for QEMU CI to consume the images, but as
> > a free
> > software developer I don't like encouraging practices that are not
> > compliant
> > with licensing reuqirement.
> >
> > It is an open question whether the (3) is really better than (1) in
> > terms
> > of URL stability long term, especially if running off a user's
> personal
> > server.
> >
> >
> > I understand your concerns. My goal here was to be able to re-activate
> > the orangepi tests, so we can capture bugs early on.
>
> I hope you understand the concern I have is not with you in particular,
> and I used your case to start a discussion with the QEMU community.
>
Hi Philippe,
Yeah I understand. I agree as well we should try to find a long-term
general solution.
>
> FWIW I missed the URL change because I still have the image cached in
> Avocado so my testing ran fine. Which makes me wonder...
>
> Cleber, Willian, should Avocado display information about cached
> artifacts? Such "Using artifact downloaded 7 months ago".
>
> > So what I can do
> > instead is:
> >
> > - update the patch to use github to store the artifacts, and their
> > licenses (other tests also use github)
>
> Until there is better solutions, this is the option I prefer.
>
Allright, I'll prepare a reworked patch soon that uses github and re-send
it.
Kind regards,
Niek
>
> > - or change the patch to use updated armbian links that work (for now)
> >
> > If we can agree on either of these solutions, so the orangepi tests can
> > be re-activated, that would be great.
> >
> > Kind regards,
> > Niek
>
>
--
Niek Linnenbank
[-- Attachment #2: Type: text/html, Size: 9299 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread