[PATCH v2] block/nbd.c: Fixed IO request coroutine not being wakeup when kill NBD server

[Rao Lei]

During the IO stress test, the IO request coroutine has a probability that is
can't be awakened when the NBD server is killed.

The GDB stack is as follows:
(gdb) bt
0  0x7f2ff990cbf6 in __ppoll (fds=0x55575de85000, nfds=1, 
timeout=, sigmask=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
1  0x55575c302e7c in qemu_poll_ns (fds=0x55575de85000, nfds=1, 
timeout=59603140) at ../util/qemu-timer.c:348
2  0x55575c2d3c34 in fdmon_poll_wait (ctx=0x55575dc480f0, 
ready_list=0x7ffd9dd1dae0, timeout=59603140) at ../util/fdmon-poll.c:80
3  0x55575c2d350d in aio_poll (ctx=0x55575dc480f0, blocking=true) at 
../util/aio-posix.c:655
4  0x55575c16eabd in bdrv_do_drained_begin(bs=0x55575dee7fe0, 
recursive=false, parent=0x0, ignore_bds_parents=false, poll=true)at 
../block/io.c:474
5  0x55575c16eba6 in bdrv_drained_begin (bs=0x55575dee7fe0) at 
../block/io.c:480
6  0x55575c1aff33 in quorum_del_child (bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block/quorum.c:1130
7  0x55575c14239b in bdrv_del_child (parent_bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block.c:7705
8  0x55575c12da28 in qmp_x_blockdev_change(parent=0x55575df404c0 
"colo-disk0", has_child=true, child=0x55575de867f0 "children.1", 
has_node=false, no   de=0x0, errp=0x7ffd9dd1dd08) at ../blockdev.c:3676
9  0x55575c258435 in qmp_marshal_x_blockdev_change (args=0x7f2fec008190, 
ret=0x7f2ff7b0bd98, errp=0x7f2ff7b0bd90) at qapi/qapi-commands-block-core.c   
:1675
10 0x55575c2c6201 in do_qmp_dispatch_bh (opaque=0x7f2ff7b0be30) at 
../qapi/qmp-dispatch.c:129
11 0x55575c2ebb1c in aio_bh_call (bh=0x55575dc429c0) at ../util/async.c:141
12 0x55575c2ebc2a in aio_bh_poll (ctx=0x55575dc480f0) at ../util/async.c:169
13 0x55575c2d2d96 in aio_dispatch (ctx=0x55575dc480f0) at 
../util/aio-posix.c:415
14 0x55575c2ec07f in aio_ctx_dispatch (source=0x55575dc480f0, callback=0x0, 
user_data=0x0) at ../util/async.c:311
15 0x7f2ff9e7cfbd in g_main_context_dispatch () at 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
16 0x55575c2fd581 in glib_pollfds_poll () at ../util/main-loop.c:232
17 0x55575c2fd5ff in os_host_main_loop_wait (timeout=0) at 
../util/main-loop.c:255
18 0x55575c2fd710 in main_loop_wait (nonblocking=0) at 
../util/main-loop.c:531
19 0x55575bfa7588 in qemu_main_loop () at ../softmmu/runstate.c:726
20 0x55575bbee57a in main (argc=60, argv=0x7ffd9dd1e0e8, 
envp=0x7ffd9dd1e2d0) at ../softmmu/main.c:50

(gdb) qemu coroutine 0x55575e16aac0
0  0x55575c2ee7dc in qemu_coroutine_switch (from_=0x55575e16aac0, 
to_=0x7f2ff830fba0, action=COROUTINE_YIELD) at ../util/coroutine-ucontext.c:302
1  0x55575c2fe2a9 in qemu_coroutine_yield () at ../util/qemu-coroutine.c:195
2  0x55575c2fe93c in qemu_co_queue_wait_impl (queue=0x55575dc46170, 
lock=0x7f2b32ad9850) at ../util/qemu-coroutine-lock.c:56
3  0x55575c17ddfb in nbd_co_send_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, qiov=0x55575dfc15d8) at ../block/nbd.c:478
4  0x55575c17f931 in nbd_co_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, write_qiov=0x55575dfc15d8) at ../block/nbd.c:1182
5  0x55575c17fe14 in nbd_client_co_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/nbd.c:1284
6  0x55575c170d25 in bdrv_driver_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:1264
7  0x55575c1733b4 in bdrv_aligned_pwritev
(child=0x55575dff6890, req=0x7f2b32ad9ad0, offset=403487858688, 
bytes=4538368, align=1, qiov=0x55575dfc15d8, qiov_offset=0, flags=0) at 
../block/io.c:2126
8  0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2314
9  0x55575c17391b in bdrv_co_pwritev (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
10 0x55575c1ee506 in replication_co_writev (bs=0x55575e9824f0, 
sector_num=788062224, remaining_sectors=8864, qiov=0x55575dfc15d8, flags=0)
at ../block/replication.c:270
11 0x55575c170eed in bdrv_driver_pwritev (bs=0x55575e9824f0, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:1297
12 0x55575c1733b4 in bdrv_aligned_pwritev
(child=0x55575dcea690, req=0x7f2b32ad9e00, offset=403487858688, 
bytes=4538368, align=512, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2126
13 0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2314
14 0x55575c17391b in bdrv_co_pwritev (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
15 0x55575c1aeffa in write_quorum_entry 

Re: [PATCH v5 00/15] vDPA shadow virtqueue

[Jason Wang]

On Wed, Mar 9, 2022 at 3:30 PM Michael S. Tsirkin  wrote:

> On Wed, Mar 09, 2022 at 11:38:35AM +0800, Jason Wang wrote:
> >
> > 在 2022/3/8 下午8:16, Michael S. Tsirkin 写道:
> > > On Tue, Mar 08, 2022 at 12:37:33PM +0100, Eugenio Perez Martin wrote:
> > > > On Tue, Mar 8, 2022 at 11:48 AM Michael S. Tsirkin 
> wrote:
> > > > > On Tue, Mar 08, 2022 at 04:20:53PM +0800, Jason Wang wrote:
> > > > > > > Not by itself but I'm not sure we can guarantee guest will not
> > > > > > > attempt to use the IOVA addresses we are reserving down
> > > > > > > the road.
> > > > > > The IOVA is allocated via the listeners and stored in the iova
> tree
> > > > > > per GPA range as IOVA->(GPA)->HVA.Guests will only see GPA, Qemu
> > > > > > virtio core see GPA to HVA mapping. And we do a reverse lookup
> to find
> > > > > > the HVA->IOVA we allocated previously.  So we have double check
> here:
> > > > > >
> > > > > > 1) Qemu memory core to make sure the GPA that guest uses is valid
> > > > > > 2) the IOVA tree that guarantees there will be no HVA beyond what
> > > > > > guest can see is used
> > > > > >
> > > > > > So technically, there's no way for the guest to use the IOVA
> address
> > > > > > allocated for the shadow virtqueue.
> > > > > >
> > > > > > Thanks
> > > > > I mean, IOVA is programmed in the host hardware to translate to
> HPA, right?
> > > > >
> > > > Yes, that's right if the device uses physical maps. Also to note, SVQ
> > > > vring is allocated in multiples of host huge pages to avoid garbage
> or
> > > > unintended access from the device.
> > > >
> > > > If a vdpa device uses physical addresses, kernel vdpa will pin qemu
> > > > memory first and then will send IOVA to HPA translation to hardware.
> > > > But this IOVA space is not controlled by the guest, but by SVQ. If a
> > > > guest's virtqueue buffer cannot be translated first to GPA, it will
> > > > not be forwarded.
> > > >
> > > > Thanks!
> > > Right. So if guests send a buffer where buffer address overlaps the
> > > range we used for the SVQ, then I think at the moment guest won't work.
> >
> >
> > There's no way for a guest to do this, it can only use GPA
>
> With a vIOMMU it can.
>

It should be the same or I may miss something.

With a vIOMMU, vDPA devices still won't use gIOVA. Instead the device will
use the IOVA that is managed by the Qemu.

Listeners: IOVA->HVA
Qemu virtqueue helper: gIOVA->GPA->HVA
SVQ: HVA->IOVA

So SVQ will use an IOVA that is overlapped with gIOVA/GPA

Thanks


>
> > but the Qemu
> > won't let vDPA to use GPA as IOVA. Dedicated IOVA ranges were allocated
> for
> > those GPA ranges so SVQ won't use IOVA that is overlapped with what Guest
> > use.
> >
> > Thanks
> >
> >
> > >
>
>

Re: [PATCH] block/nbd.c: Fixed IO request coroutine not being wakeup when kill NBD server.

[Rao, Lei]

On 3/3/2022 10:05 PM, Rao, Lei wrote:



On 3/3/2022 5:25 PM, Vladimir Sementsov-Ogievskiy wrote:

03.03.2022 05:21, Rao Lei wrote:

During the stress test, the IO request coroutine has a probability that it
can't be awakened when the NBD server is killed.

The GDB statck is as follows:
(gdb) bt
0  0x7f2ff990cbf6 in __ppoll (fds=0x55575de85000, nfds=1, timeout=, sigmask=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
1  0x55575c302e7c in qemu_poll_ns (fds=0x55575de85000, nfds=1, 
timeout=59603140) at ../util/qemu-timer.c:348
2  0x55575c2d3c34 in fdmon_poll_wait (ctx=0x55575dc480f0, 
ready_list=0x7ffd9dd1dae0, timeout=59603140) at ../util/fdmon-poll.c:80
3  0x55575c2d350d in aio_poll (ctx=0x55575dc480f0, blocking=true) at 
../util/aio-posix.c:655
4  0x55575c16eabd in bdrv_do_drained_begin (bs=0x55575dee7fe0, 
recursive=false, parent=0x0, ignore_bds_parents=false, poll=true) at 
../block/io.c:474
5  0x55575c16eba6 in bdrv_drained_begin (bs=0x55575dee7fe0) at 
../block/io.c:480
6  0x55575c1aff33 in quorum_del_child (bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block/quorum.c:1130
7  0x55575c14239b in bdrv_del_child (parent_bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block.c:7705
8  0x55575c12da28 in qmp_x_blockdev_change
 (parent=0x55575df404c0 "colo-disk0", has_child=true, child=0x55575de867f0 
"children.1", has_node=false, node=0x0, errp=0x7ffd9dd1dd08)
 at ../blockdev.c:3676
9  0x55575c258435 in qmp_marshal_x_blockdev_change (args=0x7f2fec008190, 
ret=0x7f2ff7b0bd98, errp=0x7f2ff7b0bd90) at qapi/qapi-commands-block-core.c:1675
10 0x55575c2c6201 in do_qmp_dispatch_bh (opaque=0x7f2ff7b0be30) at 
../qapi/qmp-dispatch.c:129
11 0x55575c2ebb1c in aio_bh_call (bh=0x55575dc429c0) at ../util/async.c:141
12 0x55575c2ebc2a in aio_bh_poll (ctx=0x55575dc480f0) at ../util/async.c:169
13 0x55575c2d2d96 in aio_dispatch (ctx=0x55575dc480f0) at 
../util/aio-posix.c:415
14 0x55575c2ec07f in aio_ctx_dispatch (source=0x55575dc480f0, callback=0x0, 
user_data=0x0) at ../util/async.c:311
15 0x7f2ff9e7cfbd in g_main_context_dispatch () at 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
16 0x55575c2fd581 in glib_pollfds_poll () at ../util/main-loop.c:232
17 0x55575c2fd5ff in os_host_main_loop_wait (timeout=0) at 
../util/main-loop.c:255
18 0x55575c2fd710 in main_loop_wait (nonblocking=0) at 
../util/main-loop.c:531
19 0x55575bfa7588 in qemu_main_loop () at ../softmmu/runstate.c:726
20 0x55575bbee57a in main (argc=60, argv=0x7ffd9dd1e0e8, 
envp=0x7ffd9dd1e2d0) at ../softmmu/main.c:50

(gdb) qemu coroutine 0x55575e16aac0
0  0x55575c2ee7dc in qemu_coroutine_switch (from_=0x55575e16aac0, 
to_=0x7f2ff830fba0, action=COROUTINE_YIELD) at ../util/coroutine-ucontext.c:302
1  0x55575c2fe2a9 in qemu_coroutine_yield () at ../util/qemu-coroutine.c:195
2  0x55575c2fe93c in qemu_co_queue_wait_impl (queue=0x55575dc46170, 
lock=0x7f2b32ad9850) at ../util/qemu-coroutine-lock.c:56
3  0x55575c17ddfb in nbd_co_send_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, qiov=0x55575dfc15d8) at ../block/nbd.c:478
4  0x55575c17f931 in nbd_co_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, write_qiov=0x55575dfc15d8) at ../block/nbd.c:1182
5  0x55575c17fe14 in nbd_client_co_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/nbd.c:1284
6  0x55575c170d25 in bdrv_driver_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:1264
7  0x55575c1733b4 in bdrv_aligned_pwritev
 (child=0x55575dff6890, req=0x7f2b32ad9ad0, offset=403487858688, 
bytes=4538368, align=1, qiov=0x55575dfc15d8, qiov_offset=0, flags=0) at 
../block/io.c:2126
8  0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2314
9  0x55575c17391b in bdrv_co_pwritev (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
10 0x55575c1ee506 in replication_co_writev (bs=0x55575e9824f0, 
sector_num=788062224, remaining_sectors=8864, qiov=0x55575dfc15d8, flags=0)
 at ../block/replication.c:270
11 0x55575c170eed in bdrv_driver_pwritev (bs=0x55575e9824f0, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:1297
12 0x55575c1733b4 in bdrv_aligned_pwritev
 (child=0x55575dcea690, req=0x7f2b32ad9e00, offset=403487858688, 
bytes=4538368, align=512, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2126
13 0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2314
14 0x55575c17391b in bdrv_co_pwritev (child=0x55575dcea690, 

Re: [PATCH v5 00/15] vDPA shadow virtqueue

[Michael S. Tsirkin]

On Wed, Mar 09, 2022 at 11:38:35AM +0800, Jason Wang wrote:
> 
> 在 2022/3/8 下午8:16, Michael S. Tsirkin 写道:
> > On Tue, Mar 08, 2022 at 12:37:33PM +0100, Eugenio Perez Martin wrote:
> > > On Tue, Mar 8, 2022 at 11:48 AM Michael S. Tsirkin  
> > > wrote:
> > > > On Tue, Mar 08, 2022 at 04:20:53PM +0800, Jason Wang wrote:
> > > > > > Not by itself but I'm not sure we can guarantee guest will not
> > > > > > attempt to use the IOVA addresses we are reserving down
> > > > > > the road.
> > > > > The IOVA is allocated via the listeners and stored in the iova tree
> > > > > per GPA range as IOVA->(GPA)->HVA.Guests will only see GPA, Qemu
> > > > > virtio core see GPA to HVA mapping. And we do a reverse lookup to find
> > > > > the HVA->IOVA we allocated previously.  So we have double check here:
> > > > > 
> > > > > 1) Qemu memory core to make sure the GPA that guest uses is valid
> > > > > 2) the IOVA tree that guarantees there will be no HVA beyond what
> > > > > guest can see is used
> > > > > 
> > > > > So technically, there's no way for the guest to use the IOVA address
> > > > > allocated for the shadow virtqueue.
> > > > > 
> > > > > Thanks
> > > > I mean, IOVA is programmed in the host hardware to translate to HPA, 
> > > > right?
> > > > 
> > > Yes, that's right if the device uses physical maps. Also to note, SVQ
> > > vring is allocated in multiples of host huge pages to avoid garbage or
> > > unintended access from the device.
> > > 
> > > If a vdpa device uses physical addresses, kernel vdpa will pin qemu
> > > memory first and then will send IOVA to HPA translation to hardware.
> > > But this IOVA space is not controlled by the guest, but by SVQ. If a
> > > guest's virtqueue buffer cannot be translated first to GPA, it will
> > > not be forwarded.
> > > 
> > > Thanks!
> > Right. So if guests send a buffer where buffer address overlaps the
> > range we used for the SVQ, then I think at the moment guest won't work.
> 
> 
> There's no way for a guest to do this, it can only use GPA

With a vIOMMU it can.

> but the Qemu
> won't let vDPA to use GPA as IOVA. Dedicated IOVA ranges were allocated for
> those GPA ranges so SVQ won't use IOVA that is overlapped with what Guest
> use.
> 
> Thanks
> 
> 
> > 

Re: [libvirt] [PATCH RESEND v2 0/4] re-introduce

[Ani Sinha]

On Tue, 8 Mar 2022, Michael S. Tsirkin wrote:

> On Tue, Mar 08, 2022 at 10:23:20PM +0530, Ani Sinha wrote:
> > On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin  wrote:
> > >
> > > On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
> > > >
> > > >
> > > > On Tue, 8 Mar 2022, Laine Stump wrote:
> > > >
> > > > > Aha! the domain of qemu-devel@nongnu.org was incorrect in the 
> > > > > original send
> > > > > (it was "nognu.org"), so none of this thread was making it to that 
> > > > > list.
> > > >
> > > >
> > > > Not to give any excuses but this happened because on Qemu side I never
> > > > have to type this manually. My git config is set up so that
> > > > the cc in send-email is filled up automatically using
> > > > scripts/get_maintainer.pl. On libvirt side also the domain and mailing
> > > > list is easy to remember. Its only when I have to manually type stuff 
> > > > that
> > > > shit happens :-)
> > >
> > > Donnu about alpine, but with mutt you can easily set up
> > > and alias and then it expands for you.
> >
> > I use alpine to only reply/review patches. I use git send-email to
> > actually send the patch. There I am not sure the best way to avoid
> > manually typing in the mailing list address.
>
> send-email supports aliases too.

Ah cool. I just set this up with some help from
https://felipec.wordpress.com/2009/10/25/git-send-email-tricks/ . Now I
can simply say

$ git send-email --to=qemu-list 

without worrying about typo :-) Thanks for the pointer.

Re: [PATCH 2/2] hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GICV3_TCG is unset

[Andrew Jones]

On Tue, Mar 08, 2022 at 07:24:52PM +0100, Eric Auger wrote:
> In TCG mode, if gic-version=max we always select GICv3 even if
> CONFIG_ARM_GICV3_TCG is unset. We shall rather select GICv2.
> This also brings the benefit of fixing qos tests errors for tests
> using gic-version=max with CONFIG_ARM_GICV3_TCG unset.
> 
> Signed-off-by: Eric Auger 
> 
> ---
> 
> v2 -> v3:
> - Use module_object_class_by_name() and refer to the renamed
>   CONFIG_ARM_GICV3_TCG config
> ---
>  hw/arm/virt.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 46bf7ceddf..39790d29d2 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1852,7 +1852,12 @@ static void finalize_gic_version(VirtMachineState *vms)
>  vms->gic_version = VIRT_GIC_VERSION_2;
>  break;
>  case VIRT_GIC_VERSION_MAX:
> -vms->gic_version = VIRT_GIC_VERSION_3;
> +if (module_object_class_by_name("arm-gicv3")) {
> +/* CONFIG_ARM_GICV3_TCG was set */
> +vms->gic_version = VIRT_GIC_VERSION_3;
> +} else {
> +vms->gic_version = VIRT_GIC_VERSION_2;
> +}
>  break;
>  case VIRT_GIC_VERSION_HOST:
>  error_report("gic-version=host requires KVM");
> -- 
> 2.26.3
>

 
Reviewed-by: Andrew Jones 

Re: [PATCH 1/2] hw/intc: Rename CONFIG_ARM_GIC_TCG into CONFIG_ARM_GICV3_TCG

[Andrew Jones]

On Tue, Mar 08, 2022 at 07:24:51PM +0100, Eric Auger wrote:
> CONFIG_ARM_GIC_TCG actually guards the compilation of TCG GICv3
> specific files. So let's rename it into CONFIG_ARM_GICV3_TCG
> 
> Signed-off-by: Eric Auger 
> ---
>  hw/intc/Kconfig | 2 +-
>  hw/intc/meson.build | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>

 
Reviewed-by: Andrew Jones 

Re: [PATCH 5/5] avocado/replay_kernel.py: make tcg-icount check in run_vm()

[Pavel Dovgalyuk]

On 07.03.2022 11:47, Cédric Le Goater wrote:

On 3/3/22 16:35, Daniel Henrique Barboza wrote:

The icount framework relies on TCG availability. If QEMU is built with
--disable-tcg we won't have icount either, and then this test will fail
with the following message in an IBM POWER9 host:

tests/avocado/replay_kernel.py:ReplayKernelNormal.test_ppc64_pseries:
ERROR: ConnectError: Failed to establish session:
(...)
/11-tests_avocado_replay_kernel.py_ReplayKernelNormal.test_ppc64_pseries/replay.bin: 


cannot configure icount, TCG support not available

Although this was revealed in a specific ppc64 scenario, the TCG check
is being done in the common code inside run_vm() because all archs need
TCG to have access to icount.

Cc: Pavel Dovgalyuk 
Signed-off-by: Daniel Henrique Barboza 



Reviewed-by: Cédric Le Goater 

Pavel,

Should I take this patch through the ppc tree ?


Nobody has queued it yet, so I think it is ok.



Thanks,

C.



---
  tests/avocado/replay_kernel.py | 4 
  1 file changed, 4 insertions(+)

diff --git a/tests/avocado/replay_kernel.py 
b/tests/avocado/replay_kernel.py

index c68a953730..0b2b0dc692 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -36,6 +36,9 @@ class ReplayKernelBase(LinuxKernelTest):
  def run_vm(self, kernel_path, kernel_command_line, console_pattern,
 record, shift, args, replay_path):
+    # icount requires TCG to be available
+    self.require_accelerator('tcg')
+
  logger = logging.getLogger('replay')
  start_time = time.time()
  vm = self.get_vm()
@@ -243,6 +246,7 @@ def test_ppc64_pseries(self):
  """
  :avocado: tags=arch:ppc64
  :avocado: tags=machine:pseries
+    :avocado: tags=accel:tcg
  """
  kernel_url = ('https://archives.fedoraproject.org/pub/archive'

'/fedora-secondary/releases/29/Everything/ppc64le/os'

[PATCH 13/14] iotests: make qemu_img_log() check log level

[John Snow]

Improve qemu_img_log() to actually check if logging is turned on. If it
isn't, revert to the behavior of qemu_img(). This is done so that there
really is no way to avoid scrutinizing qemu-ing subprocess calls by
accident.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f8d966cd73..6af503058f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -318,7 +318,19 @@ def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
 def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
-result = qemu_img(*args, check=False)
+"""
+Logged, unchecked variant of qemu_img() that allows non-zero exit codes.
+
+If logging is perceived to be disabled, this function will fall back
+to prohibiting non-zero return codes.
+
+:raise VerboseProcessError:
+When the return code is negative, or when logging is disabled
+on any non-zero return code.
+
+:return: a CompletedProcess instance with returncode and console output.
+"""
+result = qemu_img(*args, check=not logging_enabled())
 log(result.stdout, filters=[filter_testfiles])
 return result
 
@@ -1640,6 +1652,11 @@ def activate_logging():
 test_logger.setLevel(logging.INFO)
 test_logger.propagate = False
 
+def logging_enabled() -> bool:
+"""Return True if iotest logging is active."""
+return (test_logger.hasHandlers()
+and test_logger.getEffectiveLevel() >= logging.INFO)
+
 # This is called from script-style iotests without a single point of entry
 def script_initialize(*args, **kwargs):
 """Initialize script-style tests without running any tests."""
-- 
2.34.1

[PATCH 12/14] iotests: remove qemu_img_pipe_and_status()

[John Snow]

With the exceptional 'create' calls removed in the prior commit, change
qemu_img_log() and img_info_log() to call qemu_img() directly
instead.

In keeping with the spirit of diff-based tests, allow these calls to
qemu_img() to return an unchecked non-zero status code -- because any
error we'd see from the output is going into the log anyway.

Every last call to qemu-img is now either checked for a return code of
zero or has its output logged. It should be very hard to accidentally
ignore the return code *or* output from qemu-img now; intentional malice
remains unhandled.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1b6e28ba64..f8d966cd73 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -207,15 +207,6 @@ def qemu_img_create_prepare_args(args: List[str]) -> 
List[str]:
 
 return result
 
-def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
-"""
-Run qemu-img and return both its output and its exit code
-"""
-is_create = bool(args and args[0] == 'create')
-full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
-return qemu_tool_pipe_and_status('qemu-img', full_args,
- drop_successful_output=is_create)
-
 def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
  ) -> subprocess.CompletedProcess[str]:
 """
@@ -326,17 +317,14 @@ def qemu_img_info(*args: str) -> Any:
 def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
-def qemu_img_pipe(*args: str) -> str:
-'''Run qemu-img and return its output'''
-return qemu_img_pipe_and_status(*args)[0]
-
-def qemu_img_log(*args):
-result = qemu_img_pipe(*args)
-log(result, filters=[filter_testfiles])
+def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
+result = qemu_img(*args, check=False)
+log(result.stdout, filters=[filter_testfiles])
 return result
 
-def img_info_log(filename, filter_path=None, use_image_opts=False,
- extra_args=()):
+def img_info_log(filename: str, filter_path: Optional[str] = None,
+ use_image_opts: bool = False, extra_args: Sequence[str] = (),
+ ) -> None:
 args = ['info']
 if use_image_opts:
 args.append('--image-opts')
@@ -345,7 +333,7 @@ def img_info_log(filename, filter_path=None, 
use_image_opts=False,
 args += extra_args
 args.append(filename)
 
-output = qemu_img_pipe(*args)
+output = qemu_img(*args, check=False).stdout
 if not filter_path:
 filter_path = filename
 log(filter_img_info(output, filter_path))
-- 
2.34.1

[PATCH 11/14] iotests: replace qemu_img_log('create', ...) calls

[John Snow]

qemu_img_log() calls into qemu_img_pipe(), which always removes output
for 'create' commands on success anyway. Replace all of these calls to
the simpler qemu_img_create(...) which doesn't log, but raises a
detailed exception object on failure instead.

Blank lines are removed from output files where appropriate.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/255 |  8 
 tests/qemu-iotests/255.out |  4 
 tests/qemu-iotests/274 | 17 -
 tests/qemu-iotests/274.out | 29 -
 tests/qemu-iotests/280 |  2 +-
 tests/qemu-iotests/280.out |  1 -
 6 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
index 3d6d0e80cb..f86fa851b6 100755
--- a/tests/qemu-iotests/255
+++ b/tests/qemu-iotests/255
@@ -42,8 +42,8 @@ with iotests.FilePath('t.qcow2') as disk_path, \
 size_str = str(size)
 
 iotests.create_image(base_path, size)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, mid_path, size_str)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, disk_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, mid_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, size_str)
 
 # Create a backing chain like this:
 # base <- [throttled: bps-read=4096] <- mid <- overlay
@@ -92,8 +92,8 @@ with iotests.FilePath('src.qcow2') as src_path, \
 size = 128 * 1024 * 1024
 size_str = str(size)
 
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, src_path, size_str)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, dst_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, src_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, dst_path, size_str)
 
 iotests.log(iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write 0 1M',
 src_path),
diff --git a/tests/qemu-iotests/255.out b/tests/qemu-iotests/255.out
index 11a05a5213..2e837cbb5f 100644
--- a/tests/qemu-iotests/255.out
+++ b/tests/qemu-iotests/255.out
@@ -3,8 +3,6 @@ Finishing a commit job with background reads
 
 === Create backing chain and start VM ===
 
-
-
 === Start background read requests ===
 
 === Run a commit job ===
@@ -21,8 +19,6 @@ Closing the VM while a job is being cancelled
 
 === Create images and start VM ===
 
-
-
 wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274
index 080a90f10f..2495e051a2 100755
--- a/tests/qemu-iotests/274
+++ b/tests/qemu-iotests/274
@@ -31,12 +31,11 @@ size_long = 2 * 1024 * 1024
 size_diff = size_long - size_short
 
 def create_chain() -> None:
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, base,
- str(size_long))
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base,
- '-F', iotests.imgfmt, mid, str(size_short))
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', mid,
- '-F', iotests.imgfmt, top, str(size_long))
+iotests.qemu_img_create('-f', iotests.imgfmt, base, str(size_long))
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', base,
+'-F', iotests.imgfmt, mid, str(size_short))
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', mid,
+'-F', iotests.imgfmt, top, str(size_long))
 
 iotests.qemu_io_log('-c', 'write -P 1 0 %d' % size_long, base)
 
@@ -160,9 +159,9 @@ with iotests.FilePath('base') as base, \
 ('off',  '512k', '256k', '500k', '436k')]:
 
 iotests.log('=== preallocation=%s ===' % prealloc)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, base, base_size)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base,
- '-F', iotests.imgfmt, top, top_size_old)
+iotests.qemu_img_create('-f', iotests.imgfmt, base, base_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', base,
+'-F', iotests.imgfmt, top, top_size_old)
 iotests.qemu_io_log('-c', 'write -P 1 %s 64k' % off, base)
 
 # After this, top_size_old to base_size should be allocated/zeroed.
diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out
index 1ce40d839a..acd8b166a6 100644
--- a/tests/qemu-iotests/274.out
+++ b/tests/qemu-iotests/274.out
@@ -1,7 +1,4 @@
 == Commit tests ==
-
-
-
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
@@ -63,9 +60,6 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing HMP commit (top -> mid) ===
-
-
-
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
@@ -92,9 +86,6 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 

[PATCH 14/14] iotests: make img_info_log() call qemu_img_log()

[John Snow]

Add configurable filters to qemu_img_log(), and re-write img_info_log()
to call into qemu_img_log() with a custom filter instead.

After this patch, every last call to qemu_img() is now guaranteed to
either have its return code checked for zero, OR have its output
actually visibly logged somewhere.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6af503058f..0c69327c00 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -317,10 +317,14 @@ def qemu_img_info(*args: str) -> Any:
 def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
-def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
+def qemu_img_log(
+*args: str,
+filters: Iterable[Callable[[str], str]] = (),
+) -> subprocess.CompletedProcess[str]:
 """
 Logged, unchecked variant of qemu_img() that allows non-zero exit codes.
 
+By default, output will be filtered through filter_testfiles().
 If logging is perceived to be disabled, this function will fall back
 to prohibiting non-zero return codes.
 
@@ -331,7 +335,7 @@ def qemu_img_log(*args: str) -> 
subprocess.CompletedProcess[str]:
 :return: a CompletedProcess instance with returncode and console output.
 """
 result = qemu_img(*args, check=not logging_enabled())
-log(result.stdout, filters=[filter_testfiles])
+log(result.stdout, filters=filters or [filter_testfiles])
 return result
 
 def img_info_log(filename: str, filter_path: Optional[str] = None,
@@ -345,10 +349,11 @@ def img_info_log(filename: str, filter_path: 
Optional[str] = None,
 args += extra_args
 args.append(filename)
 
-output = qemu_img(*args, check=False).stdout
 if not filter_path:
 filter_path = filename
-log(filter_img_info(output, filter_path))
+qemu_img_log(
+*args,
+filters=[lambda output: filter_img_info(output, filter_path)])
 
 def qemu_io_wrap_args(args: Sequence[str]) -> List[str]:
 if '-f' in args or '--image-opts' in args:
-- 
2.34.1

[PATCH 08/14] iotests/149: Remove qemu_img_pipe() call

[John Snow]

qemu_img_pipe calls blank their output when the command being run is a
'create' call and the command succeeds. Thus, the normative output for
this command in iotest 149 is to print a blank line. We can remove the
logging from this invocation and use a checked invocation, but we still
need to inspect the actual output to see if we want to retroactively
skip the test due to missing cipher support.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/149 |  7 +--
 tests/qemu-iotests/149.out | 21 -
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index d49646ca60..9bb96d6a1d 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -265,8 +265,11 @@ def qemu_img_create(config, size_mb):
 "%dM" % size_mb]
 
 iotests.log("qemu-img " + " ".join(args), 
filters=[iotests.filter_test_dir])
-iotests.log(check_cipher_support(config, iotests.qemu_img_pipe(*args)),
-filters=[iotests.filter_test_dir])
+try:
+iotests.qemu_img(*args)
+except subprocess.CalledProcessError as exc:
+check_cipher_support(config, exc.output)
+raise
 
 def qemu_io_image_args(config, dev=False):
 """Get the args for access an image or device with qemu-io"""
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index ab879596ce..2cc5b82f7c 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -61,7 +61,6 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # = qemu-img aes-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -180,7 +179,6 @@ unlink TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # = qemu-img twofish-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=twofish-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 
qiotest-145-twofish-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -299,7 +297,6 @@ unlink TEST_DIR/luks-serpent-256-xts-plain64-sha1.img
 # = qemu-img serpent-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=serpent-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-serpent-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-serpent-256-xts-plain64-sha1.img 
qiotest-145-serpent-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -418,7 +415,6 @@ unlink TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img
 # = qemu-img cast5-128-cbc-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=cast5-128,cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img 
qiotest-145-cast5-128-cbc-plain64-sha1
 # Write test pattern 0xa7
@@ -538,7 +534,6 @@ unlink TEST_DIR/luks-aes-256-cbc-plain-sha1.img
 # = qemu-img aes-256-cbc-plain-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=cbc,ivgen-alg=plain,hash-alg=sha1
 TEST_DIR/luks-aes-256-cbc-plain-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-cbc-plain-sha1.img 
qiotest-145-aes-256-cbc-plain-sha1
 # Write test pattern 0xa7
@@ -657,7 +652,6 @@ unlink TEST_DIR/luks-aes-256-cbc-plain64-sha1.img
 # = qemu-img aes-256-cbc-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-cbc-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-cbc-plain64-sha1.img 
qiotest-145-aes-256-cbc-plain64-sha1
 # Write test pattern 0xa7
@@ -776,7 +770,6 @@ unlink TEST_DIR/luks-aes-256-cbc-essiv-sha256-sha1.img
 # = qemu-img aes-256-cbc-essiv-sha256-sha1 =
 # Create image
 qemu-img create -f luks --object 

[PATCH 06/14] iotests: change supports_quorum to use qemu_img

[John Snow]

Similar to other recent changes: use the qemu_img() invocation that
supports throwing loud, nasty exceptions when it fails for surprising
reasons.

(Why would "--help" ever fail? I don't know, but eliminating *all* calls
to qemu-img that do not go through qemu_img() is my goal, so
qemu_img_pipe() has to be removed.)

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3896440238..698d5a7fdc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1433,8 +1433,8 @@ def _verify_imgopts(unsupported: Sequence[str] = ()) -> 
None:
 notrun(f'not suitable for this imgopts: {imgopts}')
 
 
-def supports_quorum():
-return 'quorum' in qemu_img_pipe('--help')
+def supports_quorum() -> bool:
+return 'quorum' in qemu_img('--help').stdout
 
 def verify_quorum():
 '''Skip test suite if quorum support is not available'''
-- 
2.34.1

[PATCH 10/14] iotests: use qemu_img() in has_working_luks()

[John Snow]

Admittedly a mostly lateral move, but qemu_img() is essentially the
replacement for qemu_img_pipe_and_status(). It will give slightly better
diagnostics on crash.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 698d5a7fdc..1b6e28ba64 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1450,20 +1450,20 @@ def has_working_luks() -> Tuple[bool, str]:
 """
 
 img_file = f'{test_dir}/luks-test.luks'
-(output, status) = \
-qemu_img_pipe_and_status('create', '-f', 'luks',
- '--object', luks_default_secret_object,
- '-o', luks_default_key_secret_opt,
- '-o', 'iter-time=10',
- img_file, '1G')
+res = qemu_img('create', '-f', 'luks',
+   '--object', luks_default_secret_object,
+   '-o', luks_default_key_secret_opt,
+   '-o', 'iter-time=10',
+   img_file, '1G',
+   check=False)
 try:
 os.remove(img_file)
 except OSError:
 pass
 
-if status != 0:
-reason = output
-for line in output.splitlines():
+if res.returncode:
+reason = res.stdout
+for line in res.stdout.splitlines():
 if img_file + ':' in line:
 reason = line.split(img_file + ':', 1)[1].strip()
 break
-- 
2.34.1

[PATCH 07/14] iotests: replace unchecked calls to qemu_img_pipe()

[John Snow]

qemu_img_pipe() discards the return code from qemu-img in favor of
returning just its output. Some tests using this function don't save,
log, or check the output either, though, which is unsafe.

Replace all of these calls with a checked version.

Tests affected are 194, 202, 203, 234, 262, and 303.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/194 | 4 ++--
 tests/qemu-iotests/202 | 4 ++--
 tests/qemu-iotests/203 | 4 ++--
 tests/qemu-iotests/234 | 4 ++--
 tests/qemu-iotests/262 | 2 +-
 tests/qemu-iotests/303 | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index e44b8df728..68894371f5 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -33,8 +33,8 @@ with iotests.FilePath('source.img') as source_img_path, \
  iotests.VM('dest') as dest_vm:
 
 img_size = '1G'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, source_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, dest_img_path, img_size)
 
 iotests.log('Launching VMs...')
 (source_vm.add_drive(source_img_path)
diff --git a/tests/qemu-iotests/202 b/tests/qemu-iotests/202
index 8eb5f32d15..b784dcd791 100755
--- a/tests/qemu-iotests/202
+++ b/tests/qemu-iotests/202
@@ -35,8 +35,8 @@ with iotests.FilePath('disk0.img') as disk0_img_path, \
  iotests.VM() as vm:
 
 img_size = '10M'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk0_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk1_img_path, img_size)
 
 iotests.log('Launching VM...')
 vm.launch()
diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
index ea30e50497..ab80fd0e44 100755
--- a/tests/qemu-iotests/203
+++ b/tests/qemu-iotests/203
@@ -33,8 +33,8 @@ with iotests.FilePath('disk0.img') as disk0_img_path, \
  iotests.VM() as vm:
 
 img_size = '10M'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk0_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk1_img_path, img_size)
 
 iotests.log('Launching VM...')
 (vm.add_object('iothread,id=iothread0')
diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
index cb5f1753e0..a9f764bb2c 100755
--- a/tests/qemu-iotests/234
+++ b/tests/qemu-iotests/234
@@ -34,8 +34,8 @@ with iotests.FilePath('img') as img_path, \
  iotests.VM(path_suffix='a') as vm_a, \
  iotests.VM(path_suffix='b') as vm_b:
 
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, backing_path, '64M')
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, backing_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, img_path, '64M')
 
 os.mkfifo(fifo_a)
 os.mkfifo(fifo_b)
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index 32d69988ef..2294fd5ecb 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -51,7 +51,7 @@ with iotests.FilePath('img') as img_path, \
 
 vm.add_device('virtio-blk,drive=%s,iothread=iothread0' % root)
 
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, img_path, '64M')
 
 os.mkfifo(fifo)
 
diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index 16c2e10827..93aa5ce9b7 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -38,7 +38,7 @@ def create_bitmap(bitmap_number, disabled):
 if disabled:
 args.append('--disable')
 
-iotests.qemu_img_pipe(*args)
+iotests.qemu_img(*args)
 
 
 def write_to_disk(offset, size):
-- 
2.34.1

[PATCH 09/14] iotests: remove remaining calls to qemu_img_pipe()

[John Snow]

As part of moving all python iotest invocations of qemu-img onto a
single qemu_img() implementation, remove a few lingering uses of
qemu_img_pipe() from outside of iotests.py itself.

Several cases here rely on the knowledge that qemu_img_pipe() suppresses
*all* output on a successful case when the command being issued is
'create'.

065: This call's output is inspected, but it appears as if it's expected
 to succeed. Replace this call with the checked qemu_img() variant
 instead to get better diagnostics if/when qemu-img itself fails.

237: "create" call output isn't actually logged. Use qemu_img_create()
 instead, which checks the return code. Remove the empty lines from
 the test output.

296: Two calls;
 -create: Expected to succeed. Like other create calls, the output
  isn't actually logged.  Switch to a checked variant
  (qemu_img_create) instead. The output for this test is
  a mixture of both test styles, so actually replace the
  blank line for readability.
 -amend:  This is expected to fail. Log the output.

After this patch, the only uses of qemu_img_pipe are internal to
iotests.py and will be removed in subsequent patches.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/065 |  4 ++--
 tests/qemu-iotests/237 |  3 +--
 tests/qemu-iotests/237.out |  3 ---
 tests/qemu-iotests/296 | 12 ++--
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 9466ce7df4..ba94e19349 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_info, qemu_img_pipe
+from iotests import qemu_img, qemu_img_info
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -54,7 +54,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
 self.assertEqual(data['data'], self.json_compare)
 
 def test_human(self):
-data = qemu_img_pipe('info', '--output=human', test_img).split('\n')
+data = qemu_img('info', '--output=human', test_img).stdout.split('\n')
 data = data[(data.index('Format specific information:') + 1)
 :data.index('')]
 for field in data:
diff --git a/tests/qemu-iotests/237 b/tests/qemu-iotests/237
index 43dfd3bd40..5ea13eb01f 100755
--- a/tests/qemu-iotests/237
+++ b/tests/qemu-iotests/237
@@ -165,8 +165,7 @@ with iotests.FilePath('t.vmdk') as disk_path, \
 iotests.log("")
 
 for path in [ extent1_path, extent2_path, extent3_path ]:
-msg = iotests.qemu_img_pipe('create', '-f', imgfmt, path, '0')
-iotests.log(msg, [iotests.filter_testfiles])
+iotests.qemu_img_create('-f', imgfmt, path, '0')
 
 vm.add_blockdev('driver=file,filename=%s,node-name=ext1' % (extent1_path))
 vm.add_blockdev('driver=file,filename=%s,node-name=ext2' % (extent2_path))
diff --git a/tests/qemu-iotests/237.out b/tests/qemu-iotests/237.out
index aeb9724492..62b8865677 100644
--- a/tests/qemu-iotests/237.out
+++ b/tests/qemu-iotests/237.out
@@ -129,9 +129,6 @@ Job failed: Cannot find device='this doesn't exist' nor 
node-name='this doesn't
 
 === Other subformats ===
 
-
-
-
 == Missing extent ==
 
 {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": 
{"driver": "vmdk", "file": "node0", "size": 33554432, "subformat": 
"monolithicFlat"}}}
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index f80ef3434a..0d21b740a7 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -76,7 +76,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 # create the encrypted block device using qemu-img
 def createImg(self, file, secret):
 
-output = iotests.qemu_img_pipe(
+iotests.qemu_img(
 'create',
 '--object', *secret.to_cmdline_object(),
 '-f', iotests.imgfmt,
@@ -84,8 +84,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 '-o', 'iter-time=10',
 file,
 '1M')
-
-iotests.log(output, filters=[iotests.filter_test_dir])
+iotests.log('')
 
 # attempts to add a key using qemu-img
 def addKey(self, file, secret, new_secret):
@@ -99,7 +98,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 }
 }
 
-output = iotests.qemu_img_pipe(
+output = iotests.qemu_img(
 'amend',
 '--object', *secret.to_cmdline_object(),
 '--object', *new_secret.to_cmdline_object(),
@@ -108,8 +107,9 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 '-o', 'new-secret=' + new_secret.id(),
 '-o', 'iter-time=10',
 
-"json:" + json.dumps(image_options)
-)
+"json:" + json.dumps(image_options),
+check=False  # Expected to fail. Log output.
+).stdout
 
 iotests.log(output, 

[PATCH 02/14] iotests: use qemu_img_json() when applicable

[John Snow]

qemu_img_json() gives better diagnostic information on failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 546b142a6c..7b37938d45 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -314,11 +314,11 @@ def qemu_img_json(*args: str) -> Any:
 json_data = json.loads(res.stdout)
 return json_data
 
-def qemu_img_measure(*args):
-return json.loads(qemu_img_pipe("measure", "--output", "json", *args))
+def qemu_img_measure(*args: str) -> Any:
+return qemu_img_json("measure", "--output", "json", *args)
 
-def qemu_img_check(*args):
-return json.loads(qemu_img_pipe("check", "--output", "json", *args))
+def qemu_img_check(*args: str) -> Any:
+return qemu_img_json("check", "--output", "json", *args)
 
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
-- 
2.34.1

[PATCH 03/14] iotests: add qemu_img_info()

[John Snow]

Add qemu_img_info() by analogy with qemu_img_measure() and
qemu_img_check(). Modify image_size() to use this function instead to
take advantage of the better diagnostic information on failure provided
(ultimately) by qemu_img().

Signed-off-by: John Snow 
---
 tests/qemu-iotests/065|  5 ++---
 tests/qemu-iotests/242|  5 ++---
 tests/qemu-iotests/iotests.py | 15 +++
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index f7c1b68dad..9466ce7df4 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_pipe
+from iotests import qemu_img, qemu_img_info, qemu_img_pipe
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -49,8 +49,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
 human_compare = None
 
 def test_json(self):
-data = json.loads(qemu_img_pipe('info', '--output=json', test_img))
-data = data['format-specific']
+data = qemu_img_info(test_img)['format-specific']
 self.assertEqual(data['type'], iotests.imgfmt)
 self.assertEqual(data['data'], self.json_compare)
 
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index 96a30152b0..547bf382e3 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -22,7 +22,7 @@
 import iotests
 import json
 import struct
-from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+from iotests import qemu_img_create, qemu_io, qemu_img_info, \
 file_path, img_info_log, log, filter_qemu_io
 
 iotests.script_initialize(supported_fmts=['qcow2'],
@@ -39,8 +39,7 @@ flag_offset = 0x5000f
 def print_bitmap(extra_args):
 log('qemu-img info dump:\n')
 img_info_log(disk, extra_args=extra_args)
-result = json.loads(qemu_img_pipe('info', '--force-share',
-  '--output=json', disk))
+result = qemu_img_info('--force-share', disk)
 if 'bitmaps' in result['format-specific']['data']:
 bitmaps = result['format-specific']['data']['bitmaps']
 log('The same bitmaps in JSON format:')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7b37938d45..62f82281a9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -320,6 +320,9 @@ def qemu_img_measure(*args: str) -> Any:
 def qemu_img_check(*args: str) -> Any:
 return qemu_img_json("check", "--output", "json", *args)
 
+def qemu_img_info(*args: str) -> Any:
+return qemu_img_json('info', "--output", "json", *args)
+
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
 return qemu_img_pipe_and_status(*args)[0]
@@ -570,10 +573,14 @@ def create_image(name, size):
 file.write(sector)
 i = i + 512
 
-def image_size(img):
-'''Return image's virtual size'''
-r = qemu_img_pipe('info', '--output=json', '-f', imgfmt, img)
-return json.loads(r)['virtual-size']
+def image_size(img: str) -> int:
+"""Return image's virtual size"""
+value = qemu_img_info('-f', imgfmt, img)['virtual-size']
+if not isinstance(value, int):
+type_name = type(value).__name__
+raise TypeError("Expected 'int' for 'virtual-size', "
+f"got '{value}' of type '{type_name}'")
+return value
 
 def is_str(val):
 return isinstance(val, str)
-- 
2.34.1

[PATCH 04/14] iotests/remove-bitmap-from-backing: use qemu_img_info()

[John Snow]

This removes two more usages of qemu_img_pipe() and replaces them with
calls to qemu_img(), which provides better diagnostic information on
failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/tests/remove-bitmap-from-backing | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing 
b/tests/qemu-iotests/tests/remove-bitmap-from-backing
index fee3141340..15be32dcb9 100755
--- a/tests/qemu-iotests/tests/remove-bitmap-from-backing
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing
@@ -19,7 +19,7 @@
 #
 
 import iotests
-from iotests import log, qemu_img_create, qemu_img, qemu_img_pipe
+from iotests import log, qemu_img_create, qemu_img, qemu_img_info
 
 iotests.script_initialize(supported_fmts=['qcow2'],
   unsupported_imgopts=['compat'])
@@ -33,7 +33,7 @@ qemu_img_create('-f', iotests.imgfmt, '-b', base,
 
 qemu_img('bitmap', '--add', base, 'bitmap0')
 # Just assert that our method of checking bitmaps in the image works.
-assert 'bitmaps' in qemu_img_pipe('info', base)
+assert 'bitmaps' in qemu_img_info(base)['format-specific']['data']
 
 vm = iotests.VM().add_drive(top, 'backing.node-name=base')
 vm.launch()
@@ -68,5 +68,5 @@ if result != {'return': {}}:
 
 vm.shutdown()
 
-if 'bitmaps' in qemu_img_pipe('info', base):
+if 'bitmaps' in qemu_img_info(base)['format-specific']['data']:
 log('ERROR: Bitmap is still in the base image')
-- 
2.34.1

[PATCH 01/14] iotests: add qemu_img_json()

[John Snow]

qemu_img_json() is a new helper built on top of qemu_img() that tries to
pull a valid JSON document out of the stdout stream.

In the event that the return code is negative (the program crashed), or
the code is greater than zero and did not produce valid JSON output, the
VerboseProcessError raised by qemu_img() is re-raised.

In the event that the return code is zero but we can't parse valid JSON,
allow the JSON deserialization error to be raised.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 38 +++
 1 file changed, 38 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7057db0686..546b142a6c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -276,6 +276,44 @@ def ordered_qmp(qmsg, conv_keys=True):
 def qemu_img_create(*args: str) -> subprocess.CompletedProcess[str]:
 return qemu_img('create', *args)
 
+def qemu_img_json(*args: str) -> Any:
+"""
+Run qemu-img and return its output as deserialized JSON.
+
+:raise CalledProcessError:
+When qemu-img crashes, or returns a non-zero exit code without
+producing a valid JSON document to stdout.
+:raise JSONDecoderError:
+When qemu-img returns 0, but failed to produce a valid JSON document.
+
+:return: A deserialized JSON object; probably a dict[str, Any].
+"""
+json_data = ...  # json.loads can legitimately return 'None'.
+
+try:
+res = qemu_img(*args, combine_stdio=False)
+except subprocess.CalledProcessError as exc:
+# Terminated due to signal. Don't bother.
+if exc.returncode < 0:
+raise
+
+# Commands like 'check' can return failure (exit codes 2 and 3)
+# to indicate command completion, but with errors found. For
+# multi-command flexibility, ignore the exact error codes and
+# *try* to load JSON.
+try:
+json_data = json.loads(exc.stdout)
+except json.JSONDecodeError:
+# Nope. This thing is toast. Raise the process error.
+pass
+
+if json_data is ...:
+raise
+
+if json_data is ...:
+json_data = json.loads(res.stdout)
+return json_data
+
 def qemu_img_measure(*args):
 return json.loads(qemu_img_pipe("measure", "--output", "json", *args))
 
-- 
2.34.1

[PATCH 05/14] iotests: add qemu_img_map() function

[John Snow]

Add a qemu_img_map() function by analogy with qemu_img_measure(),
qemu_img_check(), and qemu_img_info() that all return JSON information.

Replace calls to qemu_img_pipe('map', '--output=json', ...) with this
new function, which provides better diagnostic information on failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/041 |  5 ++---
 tests/qemu-iotests/211 |  6 +++---
 tests/qemu-iotests/iotests.py  |  3 +++
 tests/qemu-iotests/tests/block-status-cache| 11 ---
 tests/qemu-iotests/tests/parallels-read-bitmap |  6 ++
 5 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index db9f5dc540..3e16acee56 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_pipe, qemu_io
+from iotests import qemu_img, qemu_img_map, qemu_io
 
 backing_img = os.path.join(iotests.test_dir, 'backing.img')
 target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
@@ -1360,8 +1360,7 @@ class TestFilters(iotests.QMPTestCase):
 
 self.vm.qmp('blockdev-del', node_name='target')
 
-target_map = qemu_img_pipe('map', '--output=json', target_img)
-target_map = json.loads(target_map)
+target_map = qemu_img_map(target_img)
 
 assert target_map[0]['start'] == 0
 assert target_map[0]['length'] == 512 * 1024
diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
index f52cadade1..1a3b4596c8 100755
--- a/tests/qemu-iotests/211
+++ b/tests/qemu-iotests/211
@@ -59,7 +59,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Successful image creation (explicit defaults)
@@ -83,7 +83,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Successful image creation (with non-default options)
@@ -107,7 +107,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Invalid BlockdevRef
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 62f82281a9..3896440238 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -323,6 +323,9 @@ def qemu_img_check(*args: str) -> Any:
 def qemu_img_info(*args: str) -> Any:
 return qemu_img_json('info', "--output", "json", *args)
 
+def qemu_img_map(*args: str) -> Any:
+return qemu_img_json('map', "--output", "json", *args)
+
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
 return qemu_img_pipe_and_status(*args)[0]
diff --git a/tests/qemu-iotests/tests/block-status-cache 
b/tests/qemu-iotests/tests/block-status-cache
index 40e648e251..5a7bc2c149 100755
--- a/tests/qemu-iotests/tests/block-status-cache
+++ b/tests/qemu-iotests/tests/block-status-cache
@@ -22,7 +22,7 @@
 import os
 import signal
 import iotests
-from iotests import qemu_img_create, qemu_img_pipe, qemu_nbd
+from iotests import qemu_img_create, qemu_img_map, qemu_nbd
 
 
 image_size = 1 * 1024 * 1024
@@ -76,8 +76,7 @@ class TestBscWithNbd(iotests.QMPTestCase):
 # to allocate the first sector to facilitate alignment probing), and
 # then the rest to be zero.  The BSC will thus contain (if anything)
 # one range covering the first sector.
-map_pre = qemu_img_pipe('map', '--output=json', '--image-opts',
-nbd_img_opts)
+map_pre = qemu_img_map('--image-opts', nbd_img_opts)
 
 # qemu:allocation-depth maps for want_zero=false.
 # want_zero=false should (with the file driver, which the server is
@@ -111,14 +110,12 @@ class TestBscWithNbd(iotests.QMPTestCase):
 # never loop too many times here.
 for _ in range(2):
 # (Ignore the result, this is just to contaminate the cache)
-qemu_img_pipe('map', '--output=json', '--image-opts',
-  nbd_img_opts_alloc_depth)
+qemu_img_map('--image-opts', nbd_img_opts_alloc_depth)
 
 # Now let's see whether the cache reports everything as data, or
 # whether we get correct information (i.e. the same as we got on our
 # first attempt).
-map_post = qemu_img_pipe('map', '--output=json', '--image-opts',
- nbd_img_opts)
+map_post = qemu_img_map('--image-opts', nbd_img_opts)
 
 if map_pre != 

[PATCH 00/14] iotests: ensure all qemu-img calls are either checked or logged

[John Snow]

Based-On: 20220308015728.1269649-1-js...@redhat.com

Hi, this series ensures all calls to qemu-img ultimately go through
qemu_img(). After the previous series, qemu_img() is a function that
defaults to raising a VerboseProcessError exception when qemu-img
returns a non-zero exit code.

After this series, all qemu-img invocations from the iotests suite are
guaranteed to go through either qemu_img_log(), which allows non-zero
exit codes (but logs its output), or qemu_img().

Some callsites manually disable the error checking with check=False,
which is a good visual reminder directly at the callsite that additional
verification must be performed. The callsites utilizing this are very
few and far between.

The VerboseProcessError exception will print the command line, return
code, and all console output to the terminal if it goes unhandled. In
effect, all non-logging calls to qemu-img are now guaranteed to print
detailed failure information to the terminal on any unanticipated
behavior.

(Even logging calls will still raise this exception if the exit code was
negative, so you get all of the same failure output whenever qemu-img
crashes *anywhere* in iotests now.)

As a result of this change, some of the helpers change. Here's a summary
of the changes between the last series and this one:

qemu_img()   => qemu_img().returncode
qemu_img_pipe()  => qemu_img().stdout
qemu_img_pipe_and_status()   => qemu_img()
json.loads(qemu_img_pipe())) => qemu_img_json()
qemu_img_log()   => qemu_img_log()

John Snow (14):
  iotests: add qemu_img_json()
  iotests: use qemu_img_json() when applicable
  iotests: add qemu_img_info()
  iotests/remove-bitmap-from-backing: use qemu_img_info()
  iotests: add qemu_img_map() function
  iotests: change supports_quorum to use qemu_img
  iotests: replace unchecked calls to qemu_img_pipe()
  iotests/149: Remove qemu_img_pipe() call
  iotests: remove remaining calls to qemu_img_pipe()
  iotests: use qemu_img() in has_working_luks()
  iotests: replace qemu_img_log('create', ...) calls
  iotests: remove qemu_img_pipe_and_status()
  iotests: make qemu_img_log() check log level
  iotests: make img_info_log() call qemu_img_log()

 tests/qemu-iotests/041|   5 +-
 tests/qemu-iotests/065|   7 +-
 tests/qemu-iotests/149|   7 +-
 tests/qemu-iotests/149.out|  21 ---
 tests/qemu-iotests/194|   4 +-
 tests/qemu-iotests/202|   4 +-
 tests/qemu-iotests/203|   4 +-
 tests/qemu-iotests/211|   6 +-
 tests/qemu-iotests/234|   4 +-
 tests/qemu-iotests/237|   3 +-
 tests/qemu-iotests/237.out|   3 -
 tests/qemu-iotests/242|   5 +-
 tests/qemu-iotests/255|   8 +-
 tests/qemu-iotests/255.out|   4 -
 tests/qemu-iotests/262|   2 +-
 tests/qemu-iotests/274|  17 ++-
 tests/qemu-iotests/274.out|  29 
 tests/qemu-iotests/280|   2 +-
 tests/qemu-iotests/280.out|   1 -
 tests/qemu-iotests/296|  12 +-
 tests/qemu-iotests/303|   2 +-
 tests/qemu-iotests/iotests.py | 134 +-
 tests/qemu-iotests/tests/block-status-cache   |  11 +-
 .../qemu-iotests/tests/parallels-read-bitmap  |   6 +-
 .../tests/remove-bitmap-from-backing  |   6 +-
 25 files changed, 150 insertions(+), 157 deletions(-)

-- 
2.34.1

Re: TCG Floating Point Support (Work in Progress)

[gaosong]

 On 2021/10/2 上午10:07, Matt wrote:


Not at the moment but it would certainly be a useful addition for the
unit tests if we could test arbitrary sequences of TCG ops. I'm not sure
how much test harness would be needed to exercise that though.

On a related note, in addition to testing TCG->Host translation, it
would be nice to also have a way to make sure TCG->TCG optimization
passes are working as expected. Is there existing work in this area?



We have a number of multiarch tcg tests for fused multiply-add and the
various fconv operations. There is also quite an exhaustive set of i386
specific tests (test-i386-fprem) but it doesn't get run by default as
the "reference" output is too big to include in the tree and has to be
generated in-situ. You get it by adding SPEED=slow to your make
invocation. [...]
You can run tests/fp/fp-bench -t host under translation to exercise that.

Thanks for the info! This will be useful.



I know the classic Doom and Quake benchmarks showed a performance
regression when we switched to softfloat:

   https://diasp.eu/posts/ec86de10240e01376f734061862b8e7b

That post was an interesting read, thanks for sharing!



Out of interest what game code still uses x87? [...]
however I kinda assumed more modern games would be taking advantaged of
SSE and later features. There is however some missing gaps in the x86
emulation that might mean code falls back to the x87. Maybe that would
be another area to look at.

This project is an emulator of the original Xbox game console, which
is now...twenty years old (time flies). The Xbox CPU (P3) does feature
SSE (not SSE2+), however most of the games I've tested for this
generation still make heavy use of x87.

I have seen at least one game make noticeable use of MMX/SSE features
though, which I also need to look at accelerating. Profiler indicates
they are also very costly. I have seen the TCG vector ops, which are a
very cool addition.

Matt


On Fri, Oct 1, 2021 at 1:24 AM Alex Bennée  wrote:


Matt  writes:


Thank you Alex, for your quick and thoughtful response.


I've not reviewed the code as it is a rather large diff. For your proper
submission could you please ensure that your patch series is broken up
into discreet commits to aid review.

Of course.


The phrase "if the user discovers some issues" is a bit of a red flag.
We should always be striving for correct emulation of floating point.

I agree. This is an option that I added for use during feature
development. Ultimately I would like not to have such an option, and
for it to always *just work*.

The closest I can think of is the --accel thread=single|multi option
which allowed for verifying if an issue was related to MTTCG. However
the default would always do the right thing.


Indeed we have recently got the code base to the point we pass all of
the Berkey softfloat test suite. This can be checked by running:

   make check-softfloat

However the test code links directly to the softfloat code so won't work
with direct code execution.

I had planned to leverage the existing soft float test suite, and I
think this can be done with the right harnessing. It would be nice to
have a mechanism to test translation of individual TCG ops, e.g. be
able to run translated blocks from test code and evaluate their
output. I'm not sure if any such op level testing like that is being
done.

Not at the moment but it would certainly be a useful addition for the
unit tests if we could test arbitrary sequences of TCG ops. I'm not sure
how much test harness would be needed to exercise that though.


There are also guest tests in tests/tcg, which could also be
expanded to include more FP tests.

We have a number of multiarch tcg tests for fused multiply-add and the
various fconv operations. There is also quite an exhaustive set of i386
specific tests (test-i386-fprem) but it doesn't get run by default as
the "reference" output is too big to include in the tree and has to be
generated in-situ. You get it by adding SPEED=slow to your make
invocation.


The existing 32/64 bit hardfloat
optimisations work within the helpers. While generating direct code is
appealing to avoid the cost of helper calls it's fairly well cached and
predicted code. Experience with the initial hardfloat code showed the
was still a performance win even with the cost of the helper call.

Unfortunately, even with the existing hardfloat support, the overhead
of the helper calls is still too costly for my particular application.

Once you start dealing with flag generation you might find that equation
changes somewhat if you have to mess around with bit masking and checks
using TCG ops. However providing benchmark results with your patch would
be required to argue the point. You can run tests/fp/fp-bench -t host
under translation to exercise that.


I don't think you'll see the same behaviour emulating an x87 on anything
that isn't an x87 because the boundaries for things like inexact
calculation will be different. Indeed if you 

Re: [PATCH v5 00/15] vDPA shadow virtqueue

[Jason Wang]

在 2022/3/8 下午8:16, Michael S. Tsirkin 写道:

On Tue, Mar 08, 2022 at 12:37:33PM +0100, Eugenio Perez Martin wrote:

On Tue, Mar 8, 2022 at 11:48 AM Michael S. Tsirkin  wrote:

On Tue, Mar 08, 2022 at 04:20:53PM +0800, Jason Wang wrote:

Not by itself but I'm not sure we can guarantee guest will not
attempt to use the IOVA addresses we are reserving down
the road.

The IOVA is allocated via the listeners and stored in the iova tree
per GPA range as IOVA->(GPA)->HVA.Guests will only see GPA, Qemu
virtio core see GPA to HVA mapping. And we do a reverse lookup to find
the HVA->IOVA we allocated previously.  So we have double check here:

1) Qemu memory core to make sure the GPA that guest uses is valid
2) the IOVA tree that guarantees there will be no HVA beyond what
guest can see is used

So technically, there's no way for the guest to use the IOVA address
allocated for the shadow virtqueue.

Thanks

I mean, IOVA is programmed in the host hardware to translate to HPA, right?


Yes, that's right if the device uses physical maps. Also to note, SVQ
vring is allocated in multiples of host huge pages to avoid garbage or
unintended access from the device.

If a vdpa device uses physical addresses, kernel vdpa will pin qemu
memory first and then will send IOVA to HPA translation to hardware.
But this IOVA space is not controlled by the guest, but by SVQ. If a
guest's virtqueue buffer cannot be translated first to GPA, it will
not be forwarded.

Thanks!

Right. So if guests send a buffer where buffer address overlaps the
range we used for the SVQ, then I think at the moment guest won't work.



There's no way for a guest to do this, it can only use GPA but the Qemu 
won't let vDPA to use GPA as IOVA. Dedicated IOVA ranges were allocated 
for those GPA ranges so SVQ won't use IOVA that is overlapped with what 
Guest use.


Thanks

[RFC PATCH 1/2] spapr: Report correct GTSE support via ov5

[Fabiano Rosas]

QEMU reports MMU support to the guest via the ibm,architecture-vec-5
property of the /chosen node. Byte number 26 specifies Radix Table
Expansions, currently only GTSE (Guest Translation Shootdown
Enable). This feature determines whether the tlbie instruction (and
others) are HV privileged.

Up until now, we always reported GTSE=1 to guests. Even after the
support for GTSE=0 was added. As part of that support, a kernel
command line radix_hcall_invalidate=on was introduced that overrides
the GTSE value received via CAS. So a guest can run with GTSE=0 and
use the H_RPT_INVALIDATE hcall instead of tlbie.

In this scenario, having GTSE always set to 1 by QEMU leads to a crash
when running nested KVM guests because KVM does not allow a nested
hypervisor to set GTSE support for its nested guests. So a nested
guest always uses the same value for LPCR_GTSE as its HV. Since the
nested HV disabled GTSE, but the L2 QEMU always reports GTSE=1, we run
into a crash when:

L1 LPCR_GTSE=0
L2 LPCR_GTSE=0
L2 CAS GTSE=1

The nested guest will run 'tlbie' and crash because the HW looks at
LPCR_GTSE, which is clear.

Having GTSE disabled in the L1 and enabled in the L2 is not an option
because the whole purpose of GTSE is to disallow access to tlbie and
we cannot allow L1 to spawn L2s that can access features that L1
itself cannot.

We also cannot have the guest check the LPCR bit, because LPCR is
HV-privileged.

So this patch goes through the most intuitive route which is to have
QEMU ask KVM about GTSE support and advertise the correct value to the
guest. A new KVM_CAP_PPC_GTSE capability is being added.

TCG continues to always enable GTSE.

Signed-off-by: Fabiano Rosas 
---
 hw/ppc/spapr.c   | 38 +++---
 target/ppc/kvm.c |  8 
 target/ppc/kvm_ppc.h |  6 ++
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4cc204f90d..3e95a1831f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -971,7 +971,7 @@ static void spapr_dt_ov5_platform_support(SpaprMachineState 
*spapr, void *fdt,
 23, 0x00, /* XICS / XIVE mode */
 24, 0x00, /* Hash/Radix, filled in below. */
 25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
-26, 0x40, /* Radix options: GTSE == yes. */
+26, 0x00, /* Radix options, filled in below. */
 };
 
 if (spapr->irq->xics && spapr->irq->xive) {
@@ -1000,10 +1000,16 @@ static void 
spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt,
 } else {
 val[3] = 0x00; /* Hash */
 }
+
+if (kvmppc_has_cap_gtse()) {
+val[7] = 0x40 /* OV5_MMU_RADIX_GTSE */;
+}
 } else {
 /* V3 MMU supports both hash and radix in tcg (with dynamic switching) 
*/
 val[3] = 0xC0;
+val[7] = 0x40 /* OV5_MMU_RADIX_GTSE */;
 }
+
 _FDT(fdt_setprop(fdt, chosen, "ibm,arch-vec-5-platform-support",
  val, sizeof(val)));
 }
@@ -2824,14 +2830,32 @@ static void spapr_machine_init(MachineState *machine)
 /* Init numa_assoc_array */
 spapr_numa_associativity_init(spapr, machine);
 
-if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
-ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
+if (ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
   spapr->max_compat_pvr)) {
-spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_300);
-/* KVM and TCG always allow GTSE with radix... */
-spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
+
+/* TCG always supports Radix w/ GTSE */
+if (!kvm_enabled()) {
+spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_300);
+spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
+} else {
+if (kvmppc_has_cap_mmu_radix()) {
+spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_300);
+}
+
+/*
+ * Only disable Guest Translation Shootdown if KVM
+ * supports the H_RPT_INVALIDATE hypercall, otherwise we'd
+ * leave the guest with no way to make TLB invalidations.
+ */
+if (kvmppc_has_cap_rpt_invalidate()) {
+if (kvmppc_has_cap_gtse()) {
+spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
+}
+} else {
+spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
+}
+}
 }
-/* ... but not with hash (currently). */
 
 if (kvm_enabled()) {
 /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index dc93b99189..91582c4b15 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -90,6 +90,7 @@ static int cap_ppc_nested_kvm_hv;
 static int cap_large_decr;
 static int cap_fwnmi;
 static int cap_rpt_invalidate;
+static int cap_gtse;
 
 static uint32_t debug_inst_opcode;
 
@@ -154,6 

[RFC PATCH 2/2] linux-headers: Add KVM_CAP_PPC_GTSE

[Fabiano Rosas]

Signed-off-by: Fabiano Rosas 
---
This is here just to facilitate review/testing of the feature.
---
 linux-headers/linux/kvm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 00af3bc333..15f19fd958 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1133,6 +1133,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206
 #define KVM_CAP_VM_GPA_BITS 207
 #define KVM_CAP_XSAVE2 208
+#define KVM_CAP_PPC_GTSE 211
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.34.1

[PATCH v4] target/riscv: Add isa extenstion strings to the device tree

[Atish Patra]

The Linux kernel parses the ISA extensions from "riscv,isa" DT
property. It used to parse only the single letter base extensions
until now. A generic ISA extension parsing framework was proposed[1]
recently that can parse multi-letter ISA extensions as well.

Generate the extended ISA string by appending  the available ISA extensions
to the "riscv,isa" string if it is enabled so that kernel can process it.

[1] https://lkml.org/lkml/2022/2/15/263

Reviewed-by: Anup Patel 
Reviewed-by: Alistair Francis 
Suggested-by: Heiko Stubner 
Signed-off-by: Atish Patra 
---

Changes from v3->v4:
1. Fixed the order of the extension names.
2. Added all the available ISA extensions in Qemu.

Changes from v2->v3:
1. Used g_strconcat to replace snprintf & a max isa string length as
suggested by Anup.
2. I have not included the Tested-by Tag from Heiko because the
implementation changed from v2 to v3.

Changes from v1->v2:
1. Improved the code redability by using arrays instead of individual check
---
 target/riscv/cpu.c | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ffb7..2521a6f31f9f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -34,6 +34,12 @@
 
 /* RISC-V CPU definitions */
 
+/* This includes the null terminated character '\0' */
+struct isa_ext_data {
+const char *name;
+bool enabled;
+};
+
 static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
 
 const char * const riscv_int_regnames[] = {
@@ -898,6 +904,42 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
*data)
 device_class_set_props(dc, riscv_cpu_properties);
 }
 
+#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
+
+static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int 
max_str_len)
+{
+char *old = *isa_str;
+char *new = *isa_str;
+int i;
+struct isa_ext_data isa_edata_arr[] = {
+ISA_EDATA_ENTRY(svinval, ext_svinval),
+ISA_EDATA_ENTRY(svnapot, ext_svnapot),
+ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
+ISA_EDATA_ENTRY(zba, ext_zba),
+ISA_EDATA_ENTRY(zbb, ext_zbb),
+ISA_EDATA_ENTRY(zbc, ext_zbc),
+ISA_EDATA_ENTRY(zbs, ext_zbs),
+ISA_EDATA_ENTRY(zdinx, ext_zdinx),
+ISA_EDATA_ENTRY(zfh, ext_zfhmin),
+ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
+ISA_EDATA_ENTRY(zfinx, ext_zfinx),
+ISA_EDATA_ENTRY(zhinx, ext_zhinx),
+ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+ISA_EDATA_ENTRY(zve32f, ext_zve32f),
+ISA_EDATA_ENTRY(zve64f, ext_zve64f),
+};
+
+for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
+if (isa_edata_arr[i].enabled) {
+new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
+g_free(old);
+old = new;
+}
+}
+
+*isa_str = new;
+}
+
 char *riscv_isa_string(RISCVCPU *cpu)
 {
 int i;
@@ -910,6 +952,7 @@ char *riscv_isa_string(RISCVCPU *cpu)
 }
 }
 *p = '\0';
+riscv_isa_string_ext(cpu, _str, maxlen);
 return isa_str;
 }
 
-- 
2.30.2

Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type

[Joel Stanley]

On Tue, 8 Mar 2022 at 17:23, Patrick Williams  wrote:
>
> On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote:
> >
> > >> There are two flash devices on the FMC. I can fix it inline since
> > >> it is the only change I would request.
> > >
> > > Yes, there are.  I think all of the Facebook systems have dual FMC, for
> > > redundancy in hardware, but we can get by in QEMU with just a single one.
> >
> > yes, the kernel will complain though and I don't know how robust
> > the spi-nor based driver is. I think you sent a patch for a related
> > issue.
> >
> > The newer spi-mem driver should be fine.
>
> Oh yes.  I already forgot that I'm running with that patch since Joel added it
> to our backport 5.15 branch.  One of the reasons I wrote that patch was to 
> make
> QEMU not kpanic. :(
>
> >
> > > I'll see however you fix it up and see I can update all the other systems 
> > > as
> > > well.
> >
> > ok. may be for 7.1 then.
> >
> > > We have an internal patch to expand the CS on FMC to 2 but we haven't
> > > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
> > > changing for adding images.
> >
> > Yes. That's the problem. I am afraid some CI systems will break with
> > these change in a newer QEMU. The command line options will need to
> > adapt.
>
> My recollection is that the Romulus CI uses a branch of QEMU that at this 
> point
> is rather old anyhow.  We should be able to fix up the CI scripts at the same
> time we upgrade.

It looks like that branch missed the 7.2 boat. Given it's imminent
release, we should update the branch to Cedric's aspeed-7.0 when 7.0
is tagged.

With this branch could do CI on Rainier (or S6Q, or transformers) with
the eMMC image. I think there's value in doing CI on both eMMC and SPI
machines.

I'd like to see a boot test added for all of the machines in CI. Most
(all?) machines will get to bmc standby by booting on a generic Qemu
machine, such as the evb.

The machines that do have more of the system modelled could boot that
instead, and in the future add further tests.

>
> Are you or Andrew J maintaining that branch?
>
> > > My recollection is that the Romulus CI on OpenBMC relies on the PNOR
> > > being the 2nd argument.
> >
> > That's the initial assumption made years ago. First mtd device is FMC,
> > second is the PNOR. It is reaching its limits.
> >
> > I am looking at improving the command line argument to support:
> >
> >-drive file=,format=raw,id=drive0 -device 
> > mx66l1g45g,bus=ssi.0,drive=drive0
> >
> > which we would clearly define the topology. Adding a cs=[0-5] or and
> > addr=[0-5] is the next step.
>
> Seems fine to me.
>
> --
> Patrick Williams

Re: [PATCH v3] target/riscv: Add isa extenstion strings to the device tree

[Atish Patra]

On Sat, Mar 5, 2022 at 10:43 PM Frank Chang  wrote:
>
> On Sun, Mar 6, 2022 at 2:12 PM Atish Kumar Patra  wrote:
>>
>>
>>
>> On Sat, Mar 5, 2022 at 9:36 PM Frank Chang  wrote:
>>>
>>> On Sun, Mar 6, 2022 at 7:42 AM Atish Kumar Patra  
>>> wrote:



 On Sat, Mar 5, 2022 at 10:05 AM Heiko Stuebner  wrote:
>
> Hi,
>
> Am Donnerstag, 3. März 2022, 19:58:38 CET schrieb Atish Patra:
> > On Fri, Feb 25, 2022 at 11:46 PM Frank Chang  
> > wrote:
> > > Atish Patra  於 2022年2月23日 週三 上午6:39寫道：
> > >>
> > >> The Linux kernel parses the ISA extensions from "riscv,isa" DT
> > >> property. It used to parse only the single letter base extensions
> > >> until now. A generic ISA extension parsing framework was proposed[1]
> > >> recently that can parse multi-letter ISA extensions as well.
> > >>
> > >> Generate the extended ISA string by appending  the available ISA 
> > >> extensions
> > >> to the "riscv,isa" string if it is enabled so that kernel can 
> > >> process it.
> > >>
> > >> [1] https://lkml.org/lkml/2022/2/15/263
> > >>
> > >> Suggested-by: Heiko Stubner 
> > >> Signed-off-by: Atish Patra 
> > >> ---
> > >> Changes from v2->v3:
> > >> 1. Used g_strconcat to replace snprintf & a max isa string length as
> > >> suggested by Anup.
> > >> 2. I have not included the Tested-by Tag from Heiko because the
> > >> implementation changed from v2 to v3.
> > >>
> > >> Changes from v1->v2:
> > >> 1. Improved the code redability by using arrays instead of 
> > >> individual check
> > >> ---
> > >>  target/riscv/cpu.c | 29 +
> > >>  1 file changed, 29 insertions(+)
> > >>
> > >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > >> index b0a40b83e7a8..2c7ff6ef555a 100644
> > >> --- a/target/riscv/cpu.c
> > >> +++ b/target/riscv/cpu.c
> > >> @@ -34,6 +34,12 @@
> > >>
> > >>  /* RISC-V CPU definitions */
> > >>
> > >> +/* This includes the null terminated character '\0' */
> > >> +struct isa_ext_data {
> > >> +const char *name;
> > >> +bool enabled;
> > >> +};
> > >> +
> > >>  static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> > >>
> > >>  const char * const riscv_int_regnames[] = {
> > >> @@ -881,6 +887,28 @@ static void riscv_cpu_class_init(ObjectClass 
> > >> *c, void *data)
> > >>  device_class_set_props(dc, riscv_cpu_properties);
> > >>  }
> > >>
> > >> +static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int 
> > >> max_str_len)
> > >> +{
> > >> +char *old = *isa_str;
> > >> +char *new = *isa_str;
> > >> +int i;
> > >> +struct isa_ext_data isa_edata_arr[] = {
> > >> +{ "svpbmt", cpu->cfg.ext_svpbmt   },
> > >> +{ "svinval", cpu->cfg.ext_svinval },
> > >> +{ "svnapot", cpu->cfg.ext_svnapot },
> > >
> > >
> > > We still have other sub-extensions, e.g. Zfh, Zba, Zbb, Zbc, Zbs... 
> > > etc.
> > > Do you mind adding them as well?
> > >
> >
> > Do we really need it ? Does any OS actually parse it from the device 
> > tree ?
> > AFAIK, Linux kernel doesn't use them. As the device tree is intended
> > to keep the information useful
> > for supervisor software,
>
> That actually isn't true ;-) .
>
> The devicetree is intended to _describe_ the hardware present in full
> and has really nothing to do with what the userspace needs.
> So the argument "Linux doesn't need this" is never true when talking
> about devicetrees ;-) .


 Yes. I didn’t mean that way. I was simply asking if these extensions 
 currently in use. I just mentioned Linux as an example.

 The larger point I was trying to make if we should add all the supported 
 extensions when they are added to Qemu or on a need basis.

 I don’t feel strongly either way. So I am okay with the former approach if 
 that’s what everyone prefers!
>>>
>>>
>>> Linux Kernel itself might not,
>>> but userspace applications may query /proc/cpuinfo to get core's 
>>> capabilities, e.g. for those vector applications.
>>> It really depends on how the application is written.
>>>
>>> I still think adding all the enabled extensions into the isa string would 
>>> be a proper solution, no matter they are used or not.
>>> However, we can leave that beyond this patch.
>>
>>
>>
>> Fair enough. I will update the patch to include all the extension available.
>
>
> Thanks, that would be great.
>
> BTW, I think current QEMU RISC-V isa string includes both "s" and "u":
> https://github.com/qemu/qemu/blob/master/target/riscv/cpu.c#L37
> But in fact, they should not be presented in the DTS isa string.
> Do you think we should exclude them as well?
>

There is a patch in the mailing list fixing 

Re: [PATCH 04/11] edk2: .git can be a file

[Eric Blake]

On Tue, Mar 08, 2022 at 03:55:14PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  roms/Makefile.edk2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
> index 3d75a842a4df..a669019fe5b2 100644
> --- a/roms/Makefile.edk2
> +++ b/roms/Makefile.edk2
> @@ -51,7 +51,7 @@ all: $(foreach 
> flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.bz2) \
>  # we're building from a tarball and that they've already been fetched by
>  # make-release/tarball scripts.
>  submodules:
> - if test -d edk2/.git; then \
> + if test -d edk2/.git -o -f edk2/.git; then \

'test ... -o ...' is not portable.  Either write this as:

if test -d edk2/.git || test -f edk2/.git; then \

or go with the shorter:

if test -e edk2/.git; then \

>   cd edk2 && git submodule update --init --force -- \
>   ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3 \
>   BaseTools/Source/C/BrotliCompress/brotli \
> -- 
> 2.35.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v3] target/riscv: Add isa extenstion strings to the device tree

[Atish Patra]

On Tue, Mar 8, 2022 at 2:53 PM Atish Patra  wrote:
>
> On Sat, Mar 5, 2022 at 10:43 PM Frank Chang  wrote:
> >
> > On Sun, Mar 6, 2022 at 2:12 PM Atish Kumar Patra  
> > wrote:
> >>
> >>
> >>
> >> On Sat, Mar 5, 2022 at 9:36 PM Frank Chang  wrote:
> >>>
> >>> On Sun, Mar 6, 2022 at 7:42 AM Atish Kumar Patra  
> >>> wrote:
> 
> 
> 
>  On Sat, Mar 5, 2022 at 10:05 AM Heiko Stuebner  wrote:
> >
> > Hi,
> >
> > Am Donnerstag, 3. März 2022, 19:58:38 CET schrieb Atish Patra:
> > > On Fri, Feb 25, 2022 at 11:46 PM Frank Chang  
> > > wrote:
> > > > Atish Patra  於 2022年2月23日 週三 上午6:39寫道：
> > > >>
> > > >> The Linux kernel parses the ISA extensions from "riscv,isa" DT
> > > >> property. It used to parse only the single letter base extensions
> > > >> until now. A generic ISA extension parsing framework was 
> > > >> proposed[1]
> > > >> recently that can parse multi-letter ISA extensions as well.
> > > >>
> > > >> Generate the extended ISA string by appending  the available ISA 
> > > >> extensions
> > > >> to the "riscv,isa" string if it is enabled so that kernel can 
> > > >> process it.
> > > >>
> > > >> [1] https://lkml.org/lkml/2022/2/15/263
> > > >>
> > > >> Suggested-by: Heiko Stubner 
> > > >> Signed-off-by: Atish Patra 
> > > >> ---
> > > >> Changes from v2->v3:
> > > >> 1. Used g_strconcat to replace snprintf & a max isa string length 
> > > >> as
> > > >> suggested by Anup.
> > > >> 2. I have not included the Tested-by Tag from Heiko because the
> > > >> implementation changed from v2 to v3.
> > > >>
> > > >> Changes from v1->v2:
> > > >> 1. Improved the code redability by using arrays instead of 
> > > >> individual check
> > > >> ---
> > > >>  target/riscv/cpu.c | 29 +
> > > >>  1 file changed, 29 insertions(+)
> > > >>
> > > >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > >> index b0a40b83e7a8..2c7ff6ef555a 100644
> > > >> --- a/target/riscv/cpu.c
> > > >> +++ b/target/riscv/cpu.c
> > > >> @@ -34,6 +34,12 @@
> > > >>
> > > >>  /* RISC-V CPU definitions */
> > > >>
> > > >> +/* This includes the null terminated character '\0' */
> > > >> +struct isa_ext_data {
> > > >> +const char *name;
> > > >> +bool enabled;
> > > >> +};
> > > >> +
> > > >>  static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> > > >>
> > > >>  const char * const riscv_int_regnames[] = {
> > > >> @@ -881,6 +887,28 @@ static void riscv_cpu_class_init(ObjectClass 
> > > >> *c, void *data)
> > > >>  device_class_set_props(dc, riscv_cpu_properties);
> > > >>  }
> > > >>
> > > >> +static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, 
> > > >> int max_str_len)
> > > >> +{
> > > >> +char *old = *isa_str;
> > > >> +char *new = *isa_str;
> > > >> +int i;
> > > >> +struct isa_ext_data isa_edata_arr[] = {
> > > >> +{ "svpbmt", cpu->cfg.ext_svpbmt   },
> > > >> +{ "svinval", cpu->cfg.ext_svinval },
> > > >> +{ "svnapot", cpu->cfg.ext_svnapot },
> > > >
> > > >
> > > > We still have other sub-extensions, e.g. Zfh, Zba, Zbb, Zbc, Zbs... 
> > > > etc.
> > > > Do you mind adding them as well?
> > > >
> > >
> > > Do we really need it ? Does any OS actually parse it from the device 
> > > tree ?
> > > AFAIK, Linux kernel doesn't use them. As the device tree is intended
> > > to keep the information useful
> > > for supervisor software,
> >
> > That actually isn't true ;-) .
> >
> > The devicetree is intended to _describe_ the hardware present in full
> > and has really nothing to do with what the userspace needs.
> > So the argument "Linux doesn't need this" is never true when talking
> > about devicetrees ;-) .
> 
> 
>  Yes. I didn’t mean that way. I was simply asking if these extensions 
>  currently in use. I just mentioned Linux as an example.
> 
>  The larger point I was trying to make if we should add all the supported 
>  extensions when they are added to Qemu or on a need basis.
> 
>  I don’t feel strongly either way. So I am okay with the former approach 
>  if that’s what everyone prefers!
> >>>
> >>>
> >>> Linux Kernel itself might not,
> >>> but userspace applications may query /proc/cpuinfo to get core's 
> >>> capabilities, e.g. for those vector applications.
> >>> It really depends on how the application is written.
> >>>
> >>> I still think adding all the enabled extensions into the isa string would 
> >>> be a proper solution, no matter they are used or not.
> >>> However, we can leave that beyond this patch.
> >>
> >>
> >>
> >> Fair enough. I will update the patch to include all the extension 
> >> 

Re: [PATCH v3] target/riscv: Add isa extenstion strings to the device tree

[Atish Patra]

On Sat, Mar 5, 2022 at 10:47 PM Frank Chang  wrote:
>
> Typo in patch title:
> s/extenstion/extension/g
>

Thanks for catching it. Will fix it.

> Regards,
> Frank Chang
>
> On Sat, Feb 26, 2022 at 3:45 PM Frank Chang  wrote:
>>
>>
>>
>> Atish Patra  於 2022年2月23日 週三 上午6:39寫道：
>>>
>>> The Linux kernel parses the ISA extensions from "riscv,isa" DT
>>> property. It used to parse only the single letter base extensions
>>> until now. A generic ISA extension parsing framework was proposed[1]
>>> recently that can parse multi-letter ISA extensions as well.
>>>
>>> Generate the extended ISA string by appending  the available ISA extensions
>>> to the "riscv,isa" string if it is enabled so that kernel can process it.
>>>
>>> [1] https://lkml.org/lkml/2022/2/15/263
>>>
>>> Suggested-by: Heiko Stubner 
>>> Signed-off-by: Atish Patra 
>>> ---
>>> Changes from v2->v3:
>>> 1. Used g_strconcat to replace snprintf & a max isa string length as
>>> suggested by Anup.
>>> 2. I have not included the Tested-by Tag from Heiko because the
>>> implementation changed from v2 to v3.
>>>
>>> Changes from v1->v2:
>>> 1. Improved the code redability by using arrays instead of individual check
>>> ---
>>>  target/riscv/cpu.c | 29 +
>>>  1 file changed, 29 insertions(+)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index b0a40b83e7a8..2c7ff6ef555a 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -34,6 +34,12 @@
>>>
>>>  /* RISC-V CPU definitions */
>>>
>>> +/* This includes the null terminated character '\0' */
>>> +struct isa_ext_data {
>>> +const char *name;
>>> +bool enabled;
>>> +};
>>> +
>>>  static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
>>>
>>>  const char * const riscv_int_regnames[] = {
>>> @@ -881,6 +887,28 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
>>> *data)
>>>  device_class_set_props(dc, riscv_cpu_properties);
>>>  }
>>>
>>> +static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int 
>>> max_str_len)
>>> +{
>>> +char *old = *isa_str;
>>> +char *new = *isa_str;
>>> +int i;
>>> +struct isa_ext_data isa_edata_arr[] = {
>>> +{ "svpbmt", cpu->cfg.ext_svpbmt   },
>>> +{ "svinval", cpu->cfg.ext_svinval },
>>> +{ "svnapot", cpu->cfg.ext_svnapot },
>>
>>
>> We still have other sub-extensions, e.g. Zfh, Zba, Zbb, Zbc, Zbs... etc.
>> Do you mind adding them as well?
>>
>> Also, I think the order of ISA strings should be alphabetical as described:
>> https://github.com/riscv/riscv-isa-manual/blob/master/src/naming.tex#L96
>>
>> Regards,
>> Frank Chang
>>
>>>
>>> +};
>>> +
>>> +for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>>> +if (isa_edata_arr[i].enabled) {
>>> +new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
>>> +g_free(old);
>>> +old = new;
>>> +}
>>> +}
>>> +
>>> +*isa_str = new;
>>> +}
>>> +
>>>  char *riscv_isa_string(RISCVCPU *cpu)
>>>  {
>>>  int i;
>>> @@ -893,6 +921,7 @@ char *riscv_isa_string(RISCVCPU *cpu)
>>>  }
>>>  }
>>>  *p = '\0';
>>> +riscv_isa_string_ext(cpu, _str, maxlen);
>>>  return isa_str;
>>>  }
>>>
>>> --
>>> 2.30.2
>>>


-- 
Regards,
Atish

Re: [RFC PATCH 1/3] target/riscv: Rename timer & timecmp to mtimer and mtimecmp

[Atish Patra]

On Tue, Mar 8, 2022 at 1:33 PM Alistair Francis  wrote:
>
> On Fri, Mar 4, 2022 at 2:08 PM Anup Patel  wrote:
> >
> > On Fri, Mar 4, 2022 at 8:50 AM Atish Patra  wrote:
> > >
> > > Currently, the aclint and ibex timer devices uses the "timer" &
> > > "timecmp" to generate the m-mode timer interrupt. In future,
> > > we will have timer interrupt injected to S/VS mode directly.
> > > No functionality change introduced in this patch.
> >
> > s/have timer/have a timer/
> >
> > >
> > > Add a prefix "m" these enviornment variables to indicate its
> > > true purpose.
> >
> > s/enviornment/environment/
> >
> > >
> > > Signed-off-by: Atish Patra 
> >
> > I suggest we should remove mtimecmp and mtimer from
> > target/riscv because mtimer is a memory mapped device.
>
> I agree. I guess this is in the CPU as it's per hart, and it's easy to
> store here instead of in the timer, but we could convert aclint to use
> an array of timers (Ibex only ever has one hart).
>
> That would probably be a closer match to hardware, as the timer is
> external to the CPU
>

ok sure. I will revise that.

> Alistair
>
> >
> > Regards,
> > Anup
> >
> > > ---
> > >  hw/intc/riscv_aclint.c | 20 ++--
> > >  hw/timer/ibex_timer.c  | 14 +++---
> > >  target/riscv/cpu.h |  4 ++--
> > >  target/riscv/machine.c |  2 +-
> > >  4 files changed, 20 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> > > index f1a5d3d284fd..642794a06865 100644
> > > --- a/hw/intc/riscv_aclint.c
> > > +++ b/hw/intc/riscv_aclint.c
> > > @@ -59,8 +59,8 @@ static void 
> > > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> > >
> > >  uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
> > >
> > > -cpu->env.timecmp = value;
> > > -if (cpu->env.timecmp <= rtc_r) {
> > > +cpu->env.mtimecmp = value;
> > > +if (cpu->env.mtimecmp <= rtc_r) {
> > >  /*
> > >   * If we're setting an MTIMECMP value in the "past",
> > >   * immediately raise the timer interrupt
> > > @@ -71,7 +71,7 @@ static void 
> > > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> > >
> > >  /* otherwise, set up the future timer interrupt */
> > >  qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
> > > -diff = cpu->env.timecmp - rtc_r;
> > > +diff = cpu->env.mtimecmp - rtc_r;
> > >  /* back to ns (note args switched in muldiv64) */
> > >  uint64_t ns_diff = muldiv64(diff, NANOSECONDS_PER_SECOND, 
> > > timebase_freq);
> > >
> > > @@ -96,7 +96,7 @@ static void 
> > > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> > >  next = MIN(next, INT64_MAX);
> > >  }
> > >
> > > -timer_mod(cpu->env.timer, next);
> > > +timer_mod(cpu->env.mtimer, next);
> > >  }
> > >
> > >  /*
> > > @@ -127,11 +127,11 @@ static uint64_t riscv_aclint_mtimer_read(void 
> > > *opaque, hwaddr addr,
> > >"aclint-mtimer: invalid hartid: %zu", hartid);
> > >  } else if ((addr & 0x7) == 0) {
> > >  /* timecmp_lo */
> > > -uint64_t timecmp = env->timecmp;
> > > +uint64_t timecmp = env->mtimecmp;
> > >  return timecmp & 0x;
> > >  } else if ((addr & 0x7) == 4) {
> > >  /* timecmp_hi */
> > > -uint64_t timecmp = env->timecmp;
> > > +uint64_t timecmp = env->mtimecmp;
> > >  return (timecmp >> 32) & 0x;
> > >  } else {
> > >  qemu_log_mask(LOG_UNIMP,
> > > @@ -168,14 +168,14 @@ static void riscv_aclint_mtimer_write(void *opaque, 
> > > hwaddr addr,
> > >"aclint-mtimer: invalid hartid: %zu", hartid);
> > >  } else if ((addr & 0x7) == 0) {
> > >  /* timecmp_lo */
> > > -uint64_t timecmp_hi = env->timecmp >> 32;
> > > +uint64_t timecmp_hi = env->mtimecmp >> 32;
> > >  riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> > > hartid,
> > >  timecmp_hi << 32 | (value & 0x),
> > >  mtimer->timebase_freq);
> > >  return;
> > >  } else if ((addr & 0x7) == 4) {
> > >  /* timecmp_hi */
> > > -uint64_t timecmp_lo = env->timecmp;
> > > +uint64_t timecmp_lo = env->mtimecmp;
> > >  riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> > > hartid,
> > >  value << 32 | (timecmp_lo & 0x),
> > >  mtimer->timebase_freq);
> > > @@ -304,9 +304,9 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, 
> > > hwaddr size,
> > >
> > >  cb->s = RISCV_ACLINT_MTIMER(dev);
> > >  cb->num = i;
> > > -env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> > > +env->mtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> > >_aclint_mtimer_cb, cb);
> > > -env->timecmp = 

Re: [PULL 00/11] Python patches

[Peter Maydell]

On Mon, 7 Mar 2022 at 22:15, John Snow  wrote:
>
> The following changes since commit b49872aa8fc0f3f5a3036cc37aa2cb5c92866f33:
>
>   Merge remote-tracking branch 
> 'remotes/hreitz-gitlab/tags/pull-block-2022-03-07' into staging (2022-03-07 
> 17:14:09 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/jsnow/qemu.git tags/python-pull-request
>
> for you to fetch changes up to 7cba010e821bf227e5fa016d0df06f2a33a0c318:
>
>   scripts/qmp-shell-wrap: Fix import path (2022-03-07 14:36:47 -0500)
>
> 
> Python patches
>
> Hopefully, fixes the race conditions witnessed through the NetBSD vm tests.
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

[PULL 00/12] Hexagon (target/hexagon) queue

[Taylor Simpson]

The following changes since commit 33d102e92e41a65c817d85ff8bfd5ffa2c16b1d3:

  Merge remote-tracking branch 
'remotes/kraxel/tags/seabios-20220307-pull-request' into staging (2022-03-08 
12:40:58 +)

are available in the Git repository at:

  https://github.com/quic/qemu tags/pull-hex-20220308

for you to fetch changes up to ebbf0ee1335548fe9b42fcd1ff031aea2d27cc1a:

  target/hexagon: remove unused variable (2022-03-08 13:27:00 -0800)


Hexagon bug fixes and additional tests

Also includes a patch from Zongyuan Li 
to remove an unused variable


Michael Lambert (1):
  Hexagon (target/hexagon) fix bug in circular addressing

Taylor Simpson (10):
  Hexagon HVX (target/hexagon) fix bug in HVX saturate instructions
  Hexagon (target/hexagon) properly set FPINVF bit in sfcmp.uo and dfcmp.uo
  Hexagon (target/hexagon) properly handle denorm in arch_sf_recip_common
  Hexagon (target/hexagon) properly handle NaN in dfmin/dfmax/sfmin/sfmax
  Hexagon (tests/tcg/hexagon) test instructions that might set bits in USR
  Hexagon (tests/tcg/hexagon) add floating point instructions to usr.c
  Hexagon (tests/tcg/hexagon) update overflow test
  Hexagon (tests/tcg/hexagon) fix inline asm in preg_alias.c
  Hexagon (target/hexagon) fix bug in conv_df2uw_chop
  Hexagon (target/hexagon) assignment to c4 should wait until packet commit

Zongyuan Li (1):
  target/hexagon: remove unused variable

 target/hexagon/fma_emu.h  |6 +-
 target/hexagon/macros.h   |4 +-
 target/hexagon/mmvec/macros.h |6 +-
 target/hexagon/arch.c |6 +-
 target/hexagon/genptr.c   |   14 +-
 target/hexagon/op_helper.c|   28 +-
 tests/tcg/hexagon/circ.c  |5 +-
 tests/tcg/hexagon/fpstuff.c   |  123 +++-
 tests/tcg/hexagon/hvx_misc.c  |   71 ++-
 tests/tcg/hexagon/overflow.c  |   61 +-
 tests/tcg/hexagon/preg_alias.c|   84 ++-
 tests/tcg/hexagon/usr.c   | 1141 +
 tests/tcg/hexagon/Makefile.target |8 +-
 13 files changed, 1474 insertions(+), 83 deletions(-)
 create mode 100644 tests/tcg/hexagon/usr.c

[PULL 01/12] Hexagon (target/hexagon) fix bug in circular addressing

[Taylor Simpson]

From: Michael Lambert 

Versions V3 and earlier should treat the "K_const" and "length" values
as unsigned.

Modified circ_test_v3() in tests/tcg/hexagon/circ.c to reproduce the bug

Signed-off-by: Michael Lambert 
Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-2-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/op_helper.c | 6 +++---
 tests/tcg/hexagon/circ.c   | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 057baf9a48..47bd51e0ca 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -304,8 +304,8 @@ void HELPER(debug_commit_end)(CPUHexagonState *env, int 
has_st0, int has_st1)
 
 int32_t HELPER(fcircadd)(int32_t RxV, int32_t offset, int32_t M, int32_t CS)
 {
-int32_t K_const = sextract32(M, 24, 4);
-int32_t length = sextract32(M, 0, 17);
+uint32_t K_const = extract32(M, 24, 4);
+uint32_t length = extract32(M, 0, 17);
 uint32_t new_ptr = RxV + offset;
 uint32_t start_addr;
 uint32_t end_addr;
diff --git a/tests/tcg/hexagon/circ.c b/tests/tcg/hexagon/circ.c
index 67a1aa3054..354416eb6d 100644
--- a/tests/tcg/hexagon/circ.c
+++ b/tests/tcg/hexagon/circ.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -415,7 +415,8 @@ static void circ_test_v3(void)
 {
 int *p = wbuf;
 int size = 15;
-int K = 4;  /* 64 bytes */
+/* set high bit in K to test unsigned extract in fcirc */
+int K = 8;  /* 1024 bytes */
 int element;
 int i;
 
-- 
2.17.1

[PULL 03/12] Hexagon (target/hexagon) properly set FPINVF bit in sfcmp.uo and dfcmp.uo

[Taylor Simpson]

Instead of checking for nan arguments, use float??_unordered_quiet

test cases added in a subsequent patch to more extensively test USR bits

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-4-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/op_helper.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 47bd51e0ca..75dc0f23f0 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -938,8 +938,7 @@ int32_t HELPER(sfcmpuo)(CPUHexagonState *env, float32 RsV, 
float32 RtV)
 {
 int32_t PdV;
 arch_fpop_start(env);
-PdV = f8BITSOF(float32_is_any_nan(RsV) ||
-   float32_is_any_nan(RtV));
+PdV = f8BITSOF(float32_unordered_quiet(RsV, RtV, >fp_status));
 arch_fpop_end(env);
 return PdV;
 }
@@ -1097,8 +1096,7 @@ int32_t HELPER(dfcmpuo)(CPUHexagonState *env, float64 
RssV, float64 RttV)
 {
 int32_t PdV;
 arch_fpop_start(env);
-PdV = f8BITSOF(float64_is_any_nan(RssV) ||
-   float64_is_any_nan(RttV));
+PdV = f8BITSOF(float64_unordered_quiet(RssV, RttV, >fp_status));
 arch_fpop_end(env);
 return PdV;
 }
-- 
2.17.1

[PULL 05/12] Hexagon (target/hexagon) properly handle NaN in dfmin/dfmax/sfmin/sfmax

[Taylor Simpson]

The float??_minnum implementation differs from Hexagon for SNaN,
it returns NaN, but Hexagon returns the other input.  So, we use
float??_minimum_number.

Test cases added to tests/tcg/hexagon/fpstuff.c

Signed-off-by: Taylor Simpson 
Message-Id: <20220308190410.22355-1-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/op_helper.c  | 14 ++-
 tests/tcg/hexagon/fpstuff.c | 79 +
 2 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 75dc0f23f0..366caf9ec8 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -947,7 +947,7 @@ float32 HELPER(sfmax)(CPUHexagonState *env, float32 RsV, 
float32 RtV)
 {
 float32 RdV;
 arch_fpop_start(env);
-RdV = float32_maxnum(RsV, RtV, >fp_status);
+RdV = float32_maximum_number(RsV, RtV, >fp_status);
 arch_fpop_end(env);
 return RdV;
 }
@@ -956,7 +956,7 @@ float32 HELPER(sfmin)(CPUHexagonState *env, float32 RsV, 
float32 RtV)
 {
 float32 RdV;
 arch_fpop_start(env);
-RdV = float32_minnum(RsV, RtV, >fp_status);
+RdV = float32_minimum_number(RsV, RtV, >fp_status);
 arch_fpop_end(env);
 return RdV;
 }
@@ -1040,10 +1040,7 @@ float64 HELPER(dfmax)(CPUHexagonState *env, float64 
RssV, float64 RttV)
 {
 float64 RddV;
 arch_fpop_start(env);
-RddV = float64_maxnum(RssV, RttV, >fp_status);
-if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
-float_raise(float_flag_invalid, >fp_status);
-}
+RddV = float64_maximum_number(RssV, RttV, >fp_status);
 arch_fpop_end(env);
 return RddV;
 }
@@ -1052,10 +1049,7 @@ float64 HELPER(dfmin)(CPUHexagonState *env, float64 
RssV, float64 RttV)
 {
 float64 RddV;
 arch_fpop_start(env);
-RddV = float64_minnum(RssV, RttV, >fp_status);
-if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
-float_raise(float_flag_invalid, >fp_status);
-}
+RddV = float64_minimum_number(RssV, RttV, >fp_status);
 arch_fpop_end(env);
 return RddV;
 }
diff --git a/tests/tcg/hexagon/fpstuff.c b/tests/tcg/hexagon/fpstuff.c
index 043f18fab3..56bf562a40 100644
--- a/tests/tcg/hexagon/fpstuff.c
+++ b/tests/tcg/hexagon/fpstuff.c
@@ -41,7 +41,8 @@ const int SF_small_neg =  0xab98fba8;
 const int SF_denorm = 0x0001;
 const int SF_random = 0x346001d6;
 
-const long long DF_NaN =  0x7ff8ULL;
+const long long DF_QNaN = 0x7ff8ULL;
+const long long DF_SNaN = 0x7ff7ULL;
 const long long DF_ANY =  0x3f80ULL;
 const long long DF_HEX_NAN =  0xULL;
 const long long DF_small_neg =0xbd731f75ULL;
@@ -128,7 +129,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.eq(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -137,7 +138,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.gt(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -146,7 +147,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.ge(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -208,7 +209,7 @@ static void check_dfminmax(void)
 int usr;
 
 /*
- * Execute dfmin/dfmax instructions with one operand as NaN
+ * Execute dfmin/dfmax instructions with one operand as SNaN
  * Check that
  * Result is the other operand
  * Invalid bit in USR is set
@@ -216,7 +217,7 @@ static void check_dfminmax(void)
  asm (CLEAR_FPSTATUS
  "%0 = dfmin(%2, %3)\n\t"
  "%1 = usr\n\t"
- : "=r"(minmax), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(minmax), "=r"(usr) : "r"(DF_SNaN), "r"(DF_ANY)
  : "r2", "usr");
 check64(minmax, DF_ANY);
 check_fpstatus(usr, FPINVF);
@@ -224,13 +225,35 @@ static void check_dfminmax(void)
 asm (CLEAR_FPSTATUS
  "%0 = dfmax(%2, %3)\n\t"
  "%1 = usr\n\t"
- : "=r"(minmax), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(minmax), "=r"(usr) : "r"(DF_SNaN), "r"(DF_ANY)
  : "r2", "usr");
 check64(minmax, DF_ANY);
 check_fpstatus(usr, FPINVF);
 
 /*
- * Execute dfmin/dfmax instructions with both 

[PULL 02/12] Hexagon HVX (target/hexagon) fix bug in HVX saturate instructions

[Taylor Simpson]

Two tests added to tests/tcg/hexagon/hvx_misc.c
v21.uw = vadd(v11.uw, v10.uw):sat
v25:24.uw = vsub(v17:16.uw, v27:26.uw):sat

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-3-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/macros.h  |  4 +-
 tests/tcg/hexagon/hvx_misc.c | 71 +++-
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 19d103cad5..a78e84faa4 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -268,7 +268,7 @@ static inline void gen_pred_cancel(TCGv pred, int slot_num)
 
 #define fVSATUVALN(N, VAL) \
 ({ \
-(((int)(VAL)) < 0) ? 0 : ((1LL << (N)) - 1); \
+(((int64_t)(VAL)) < 0) ? 0 : ((1LL << (N)) - 1); \
 })
 #define fSATUVALN(N, VAL) \
 ({ \
diff --git a/tests/tcg/hexagon/hvx_misc.c b/tests/tcg/hexagon/hvx_misc.c
index 312bb98b41..b896f5897e 100644
--- a/tests/tcg/hexagon/hvx_misc.c
+++ b/tests/tcg/hexagon/hvx_misc.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2021 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *  Copyright(c) 2021-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 int err;
 
@@ -432,6 +433,71 @@ TEST_PRED_OP2(pred_and, and, &, "")
 TEST_PRED_OP2(pred_and_n, and, &, "!")
 TEST_PRED_OP2(pred_xor, xor, ^, "")
 
+static void test_vadduwsat(void)
+{
+/*
+ * Test for saturation by adding two numbers that add to more than UINT_MAX
+ * and make sure the result saturates to UINT_MAX
+ */
+const uint32_t x = 0x;
+const uint32_t y = 0x000f;
+
+memset(expect, 0x12, sizeof(MMVector));
+memset(output, 0x34, sizeof(MMVector));
+
+asm volatile ("v10 = vsplat(%0)\n\t"
+  "v11 = vsplat(%1)\n\t"
+  "v21.uw = vadd(v11.uw, v10.uw):sat\n\t"
+  "vmem(%2+#0) = v21\n\t"
+  : /* no outputs */
+  : "r"(x), "r"(y), "r"(output)
+  : "v10", "v11", "v21", "memory");
+
+for (int j = 0; j < MAX_VEC_SIZE_BYTES / 4; j++) {
+expect[0].uw[j] = UINT_MAX;
+}
+
+check_output_w(__LINE__, 1);
+}
+
+static void test_vsubuwsat_dv(void)
+{
+/*
+ * Test for saturation by subtracting two numbers where the result is
+ * negative and make sure the result saturates to zero
+ *
+ * vsubuwsat_dv operates on an HVX register pair, so we'll have a
+ * pair of subtractions
+ * w - x < 0
+ * y - z < 0
+ */
+const uint32_t w = 0x00b7;
+const uint32_t x = 0xff4e;
+const uint32_t y = 0x31fe88e7;
+const uint32_t z = 0x7f79;
+
+memset(expect, 0x12, sizeof(MMVector) * 2);
+memset(output, 0x34, sizeof(MMVector) * 2);
+
+asm volatile ("v16 = vsplat(%0)\n\t"
+  "v17 = vsplat(%1)\n\t"
+  "v26 = vsplat(%2)\n\t"
+  "v27 = vsplat(%3)\n\t"
+  "v25:24.uw = vsub(v17:16.uw, v27:26.uw):sat\n\t"
+  "vmem(%4+#0) = v24\n\t"
+  "vmem(%4+#1) = v25\n\t"
+  : /* no outputs */
+  : "r"(w), "r"(y), "r"(x), "r"(z), "r"(output)
+  : "v16", "v17", "v24", "v25", "v26", "v27", "memory");
+
+for (int j = 0; j < MAX_VEC_SIZE_BYTES / 4; j++) {
+expect[0].uw[j] = 0x;
+expect[1].uw[j] = 0x;
+}
+
+check_output_w(__LINE__, 2);
+}
+
 int main()
 {
 init_buffers();
@@ -464,6 +530,9 @@ int main()
 test_pred_and_n(true);
 test_pred_xor(false);
 
+test_vadduwsat();
+test_vsubuwsat_dv();
+
 puts(err ? "FAIL" : "PASS");
 return err ? 1 : 0;
 }
-- 
2.17.1

[PULL 11/12] Hexagon (target/hexagon) assignment to c4 should wait until packet commit

[Taylor Simpson]

On Hexagon, c4 is an alias for predicate registers P3:0.  If we assign to
c4 inside a packet with reads from predicate registers, the predicate
reads should get the old values.

Test case added to tests/tcg/hexagon/preg_alias.c

Co-authored-by: Michael Lambert 
Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-13-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/genptr.c| 14 -
 tests/tcg/hexagon/preg_alias.c | 38 ++
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 4419d30e23..cd6af4bceb 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -210,11 +210,15 @@ static inline void gen_read_ctrl_reg_pair(DisasContext 
*ctx, const int reg_num,
 }
 }
 
-static inline void gen_write_p3_0(TCGv control_reg)
+static void gen_write_p3_0(DisasContext *ctx, TCGv control_reg)
 {
+TCGv hex_p8 = tcg_temp_new();
 for (int i = 0; i < NUM_PREGS; i++) {
-tcg_gen_extract_tl(hex_pred[i], control_reg, i * 8, 8);
+tcg_gen_extract_tl(hex_p8, control_reg, i * 8, 8);
+gen_log_pred_write(ctx, i, hex_p8);
+ctx_log_pred_write(ctx, i);
 }
+tcg_temp_free(hex_p8);
 }
 
 /*
@@ -228,7 +232,7 @@ static inline void gen_write_ctrl_reg(DisasContext *ctx, 
int reg_num,
   TCGv val)
 {
 if (reg_num == HEX_REG_P3_0) {
-gen_write_p3_0(val);
+gen_write_p3_0(ctx, val);
 } else {
 gen_log_reg_write(reg_num, val);
 ctx_log_reg_write(ctx, reg_num);
@@ -250,7 +254,7 @@ static inline void gen_write_ctrl_reg_pair(DisasContext 
*ctx, int reg_num,
 if (reg_num == HEX_REG_P3_0) {
 TCGv val32 = tcg_temp_new();
 tcg_gen_extrl_i64_i32(val32, val);
-gen_write_p3_0(val32);
+gen_write_p3_0(ctx, val32);
 tcg_gen_extrh_i64_i32(val32, val);
 gen_log_reg_write(reg_num + 1, val32);
 tcg_temp_free(val32);
diff --git a/tests/tcg/hexagon/preg_alias.c b/tests/tcg/hexagon/preg_alias.c
index 9f7b125998..b04c45632c 100644
--- a/tests/tcg/hexagon/preg_alias.c
+++ b/tests/tcg/hexagon/preg_alias.c
@@ -97,6 +97,42 @@ static inline void creg_alias_pair(unsigned int cval, PRegs 
*pregs)
   check(c5, 0xdeadbeef);
 }
 
+static void test_packet(void)
+{
+/*
+ * Test that setting c4 inside a packet doesn't impact the predicates
+ * that are read during the packet.
+ */
+
+int result;
+int old_val = 0x001c;
+
+/* Test a predicated register transfer */
+result = old_val;
+asm (
+ "c4 = %1\n\t"
+ "{\n\t"
+ "c4 = %2\n\t"
+ "if (!p2) %0 = %3\n\t"
+ "}\n\t"
+ : "+r"(result)
+ : "r"(0x), "r"(0xff00), "r"(0x837ed653)
+ : "c4", "p0", "p1", "p2", "p3");
+check(result, old_val);
+
+/* Test a predicated store */
+result = 0x;
+asm ("c4 = %0\n\t"
+ "{\n\t"
+ "c4 = %1\n\t"
+ "if (!p2) memw(%2) = #0\n\t"
+ "}\n\t"
+ :
+ : "r"(0), "r"(0x), "r"()
+ : "c4", "p0", "p1", "p2", "p3", "memory");
+check(result, 0x0);
+}
+
 int main()
 {
 int c4;
@@ -162,6 +198,8 @@ int main()
 creg_alias_pair(0x, );
 check(pregs.creg, 0x);
 
+test_packet();
+
 puts(err ? "FAIL" : "PASS");
 return err;
 }
-- 
2.17.1

[PULL 07/12] Hexagon (tests/tcg/hexagon) add floating point instructions to usr.c

[Taylor Simpson]

Tests to confirm floating point instructions are properly
setting exception bits in USR

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-8-tsimp...@quicinc.com>
Acked-by: Richard Henderson 
---
 tests/tcg/hexagon/usr.c | 339 
 1 file changed, 339 insertions(+)

diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c
index e82727237e..11415f8295 100644
--- a/tests/tcg/hexagon/usr.c
+++ b/tests/tcg/hexagon/usr.c
@@ -78,6 +78,34 @@ static void __check64(int line, uint64_t val, uint64_t 
expect)
 #define USR_FPUNFF   (1 << USR_FPUNFF_BIT)
 #define USR_FPINPF   (1 << USR_FPINPF_BIT)
 
+/* Some useful floating point values */
+const uint32_t SF_INF =  0x7f80;
+const uint32_t SF_QNaN = 0x7fc0;
+const uint32_t SF_SNaN = 0x7fb0;
+const uint32_t SF_QNaN_neg = 0xffc0;
+const uint32_t SF_SNaN_neg = 0xffb0;
+const uint32_t SF_HEX_NaN =  0x;
+const uint32_t SF_zero = 0x;
+const uint32_t SF_one =  0x3f80;
+const uint32_t SF_one_recip =0x3f7f0001; /* 0.9960...  */
+const uint32_t SF_one_invsqrta = 0x3f7f; /* 0.99609375 */
+const uint32_t SF_two =  0x4000;
+const uint32_t SF_four = 0x4080;
+const uint32_t SF_small_neg =0xab98fba8;
+const uint32_t SF_large_pos =0x5afa572e;
+
+const uint64_t DF_QNaN = 0x7ff8ULL;
+const uint64_t DF_SNaN = 0x7ff7ULL;
+const uint64_t DF_QNaN_neg = 0xfff8ULL;
+const uint64_t DF_SNaN_neg = 0xfff7ULL;
+const uint64_t DF_HEX_NaN =  0xULL;
+const uint64_t DF_zero = 0xULL;
+const uint64_t DF_any =  0x3f80ULL;
+const uint64_t DF_one =  0x3ff0ULL;
+const uint64_t DF_one_hh =   0x3ff001ff8000ULL; /* 1.00048... 
*/
+const uint64_t DF_small_neg =0xbd731f75ULL;
+const uint64_t DF_large_pos =0x7f81ULL;
+
 /*
  * Templates for functions to execute an instruction
  *
@@ -309,6 +337,29 @@ static RESTYPE NAME(RESTYPE result, SRC1TYPE src1, 
SRC2TYPE src2, uint8_t pred,\
 #define FUNC_XR_OP_RRp(NAME, INSN) \
 FUNC_Xx_OP_xxp(uint32_t, uint32_t, uint32_t, NAME, INSN)
 
+/* Template for compare instructions with two register operands */
+#define FUNC_CMP_xx(SRC1TYPE, SRC2TYPE, NAME, INSN) \
+static uint32_t NAME(SRC1TYPE src1, SRC2TYPE src2, uint32_t *usr_result) \
+{ \
+uint32_t result; \
+uint32_t usr; \
+asm(CLEAR_USRBITS \
+INSN "\n\t" \
+"%0 = p1\n\t" \
+"%1 = usr\n\t" \
+: "=r"(result), "=r"(usr) \
+: "r"(src1), "r"(src2) \
+: "p1", "r2", "usr"); \
+*usr_result = usr & 0x3f; \
+return result; \
+}
+
+#define FUNC_CMP_RR(NAME, INSN) \
+FUNC_CMP_xx(uint32_t, uint32_t, NAME, INSN)
+
+#define FUNC_CMP_PP(NAME, INSN) \
+FUNC_CMP_xx(uint64_t, uint64_t, NAME, INSN)
+
 /*
  * Function declarations using the templates
  */
@@ -379,6 +430,69 @@ FUNC_R_OP_RR(asr_r_r_sat,   "%0 = asr(%2, %3):sat")
 
 FUNC_XPp_OP_PP(ACS, "%0, p2 = vacsh(%3, %4)")
 
+/* Floating point */
+FUNC_R_OP_RR(sfmin, "%0 = sfmin(%2, %3)")
+FUNC_R_OP_RR(sfmax, "%0 = sfmax(%2, %3)")
+FUNC_R_OP_RR(sfadd, "%0 = sfadd(%2, %3)")
+FUNC_R_OP_RR(sfsub, "%0 = sfsub(%2, %3)")
+FUNC_R_OP_RR(sfmpy, "%0 = sfmpy(%2, %3)")
+FUNC_XR_OP_RR(sffma,"%0 += sfmpy(%2, %3)")
+FUNC_XR_OP_RR(sffms,"%0 -= sfmpy(%2, %3)")
+FUNC_CMP_RR(sfcmpuo,"p1 = sfcmp.uo(%2, %3)")
+FUNC_CMP_RR(sfcmpeq,"p1 = sfcmp.eq(%2, %3)")
+FUNC_CMP_RR(sfcmpgt,"p1 = sfcmp.gt(%2, %3)")
+FUNC_CMP_RR(sfcmpge,"p1 = sfcmp.ge(%2, %3)")
+
+FUNC_P_OP_PP(dfadd, "%0 = dfadd(%2, %3)")
+FUNC_P_OP_PP(dfsub, "%0 = dfsub(%2, %3)")
+
+#if CORE_IS_V67
+FUNC_P_OP_PP(dfmin, "%0 = dfmin(%2, %3)")
+FUNC_P_OP_PP(dfmax, "%0 = dfmax(%2, %3)")
+FUNC_XP_OP_PP(dfmpyhh,  "%0 += dfmpyhh(%2, %3)")
+#endif
+
+FUNC_CMP_PP(dfcmpuo,"p1 = dfcmp.uo(%2, %3)")
+FUNC_CMP_PP(dfcmpeq,"p1 = dfcmp.eq(%2, %3)")
+FUNC_CMP_PP(dfcmpgt,"p1 = dfcmp.gt(%2, %3)")
+FUNC_CMP_PP(dfcmpge,"p1 = dfcmp.ge(%2, %3)")
+
+/* Conversions from sf */
+FUNC_P_OP_R(conv_sf2df, "%0 = convert_sf2df(%2)")
+FUNC_R_OP_R(conv_sf2uw, "%0 = convert_sf2uw(%2)")
+FUNC_R_OP_R(conv_sf2w,  "%0 = convert_sf2w(%2)")
+FUNC_P_OP_R(conv_sf2ud, "%0 = convert_sf2ud(%2)")
+FUNC_P_OP_R(conv_sf2d,  "%0 = convert_sf2d(%2)")
+FUNC_R_OP_R(conv_sf2uw_chop,"%0 = convert_sf2uw(%2):chop")
+FUNC_R_OP_R(conv_sf2w_chop, "%0 = convert_sf2w(%2):chop")
+FUNC_P_OP_R(conv_sf2ud_chop,"%0 = convert_sf2ud(%2):chop")

[PULL 09/12] Hexagon (tests/tcg/hexagon) fix inline asm in preg_alias.c

[Taylor Simpson]

Replace consecutive inline asm blocks with a single one with proper
outputs/inputs/clobbers rather than making assumptions about register
values being carried between separate blocks.

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-10-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 tests/tcg/hexagon/preg_alias.c | 46 --
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/tests/tcg/hexagon/preg_alias.c b/tests/tcg/hexagon/preg_alias.c
index 0cac469b78..9f7b125998 100644
--- a/tests/tcg/hexagon/preg_alias.c
+++ b/tests/tcg/hexagon/preg_alias.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -57,17 +57,15 @@ typedef union {
 
 static inline void creg_alias(int cval, PRegs *pregs)
 {
-  unsigned char val;
-  asm volatile("c4 = %0" : : "r"(cval));
-
-  asm volatile("%0 = p0" : "=r"(val));
-  pregs->pregs.p0 = val;
-  asm volatile("%0 = p1" : "=r"(val));
-  pregs->pregs.p1 = val;
-  asm volatile("%0 = p2" : "=r"(val));
-  pregs->pregs.p2 = val;
-  asm volatile("%0 = p3" : "=r"(val));
-  pregs->pregs.p3 = val;
+  asm("c4 = %4\n\t"
+  "%0 = p0\n\t"
+  "%1 = p1\n\t"
+  "%2 = p2\n\t"
+  "%3 = p3\n\t"
+  : "=r"(pregs->pregs.p0), "=r"(pregs->pregs.p1),
+"=r"(pregs->pregs.p2), "=r"(pregs->pregs.p3)
+  : "r"(cval)
+  : "c4", "p0", "p1", "p2", "p3");
 }
 
 int err;
@@ -83,19 +81,19 @@ static void check(int val, int expect)
 static inline void creg_alias_pair(unsigned int cval, PRegs *pregs)
 {
   unsigned long long cval_pair = (0xdeadbeefULL << 32) | cval;
-  unsigned char val;
   int c5;
-  asm volatile("c5:4 = %0" : : "r"(cval_pair));
-
-  asm volatile("%0 = p0" : "=r"(val));
-  pregs->pregs.p0 = val;
-  asm volatile("%0 = p1" : "=r"(val));
-  pregs->pregs.p1 = val;
-  asm volatile("%0 = p2" : "=r"(val));
-  pregs->pregs.p2 = val;
-  asm volatile("%0 = p3" : "=r"(val));
-  pregs->pregs.p3 = val;
-  asm volatile("%0 = c5" : "=r"(c5));
+
+  asm ("c5:4 = %5\n\t"
+   "%0 = p0\n\t"
+   "%1 = p1\n\t"
+   "%2 = p2\n\t"
+   "%3 = p3\n\t"
+   "%4 = c5\n\t"
+   : "=r"(pregs->pregs.p0), "=r"(pregs->pregs.p1),
+ "=r"(pregs->pregs.p2), "=r"(pregs->pregs.p3), "=r"(c5)
+   : "r"(cval_pair)
+   : "c4", "c5", "p0", "p1", "p2", "p3");
+
   check(c5, 0xdeadbeef);
 }
 
-- 
2.17.1

[PULL 08/12] Hexagon (tests/tcg/hexagon) update overflow test

[Taylor Simpson]

Add a test that sets USR multiple times in a packet

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-9-tsimp...@quicinc.com>
Acked-by: Richard Henderson 
---
 tests/tcg/hexagon/overflow.c | 61 +++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/hexagon/overflow.c b/tests/tcg/hexagon/overflow.c
index 196fcf7f3a..94087851b0 100644
--- a/tests/tcg/hexagon/overflow.c
+++ b/tests/tcg/hexagon/overflow.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2021 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *  Copyright(c) 2021-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -72,6 +72,20 @@ int read_usr_overflow(void)
 return result & 1;
 }
 
+int get_usr_overflow(int usr)
+{
+return usr & 1;
+}
+
+int get_usr_fp_invalid(int usr)
+{
+return (usr >> 1) & 1;
+}
+
+int get_usr_lpcfg(int usr)
+{
+return (usr >> 8) & 0x3;
+}
 
 jmp_buf jmp_env;
 int usr_overflow;
@@ -82,6 +96,49 @@ static void sig_segv(int sig, siginfo_t *info, void *puc)
 longjmp(jmp_env, 1);
 }
 
+static void test_packet(void)
+{
+int convres;
+int satres;
+int usr;
+
+asm("r2 = usr\n\t"
+"r2 = clrbit(r2, #0)\n\t"/* clear overflow bit */
+"r2 = clrbit(r2, #1)\n\t"/* clear FP invalid bit */
+"usr = r2\n\t"
+"{\n\t"
+"%0 = convert_sf2uw(%3):chop\n\t"
+"%1 = satb(%4)\n\t"
+"}\n\t"
+"%2 = usr\n\t"
+: "=r"(convres), "=r"(satres), "=r"(usr)
+: "r"(0x6a051b86), "r"(0x0410eec0)
+: "r2", "usr");
+
+check(convres, 0x);
+check(satres, 0x7f);
+check(get_usr_overflow(usr), 1);
+check(get_usr_fp_invalid(usr), 1);
+
+asm("r2 = usr\n\t"
+"r2 = clrbit(r2, #0)\n\t"/* clear overflow bit */
+"usr = r2\n\t"
+"%2 = r2\n\t"
+"p3 = sp3loop0(1f, #1)\n\t"
+"1:\n\t"
+"{\n\t"
+"%0 = satb(%2)\n\t"
+"}:endloop0\n\t"
+"%1 = usr\n\t"
+: "=r"(satres), "=r"(usr)
+: "r"(0x0410eec0)
+: "r2", "usr", "p3", "sa0", "lc0");
+
+check(satres, 0x7f);
+check(get_usr_overflow(usr), 1);
+check(get_usr_lpcfg(usr), 2);
+}
+
 int main()
 {
 struct sigaction act;
@@ -102,6 +159,8 @@ int main()
 
 check(usr_overflow, 0);
 
+test_packet();
+
 puts(err ? "FAIL" : "PASS");
 return err ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.17.1

[PULL 10/12] Hexagon (target/hexagon) fix bug in conv_df2uw_chop

[Taylor Simpson]

Fix typo that checked for 32 bit nan instead of 64 bit

Test case added in tests/tcg/hexagon/usr.c

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-11-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/op_helper.c | 2 +-
 tests/tcg/hexagon/usr.c| 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 366caf9ec8..63e5ad5d68 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -829,7 +829,7 @@ uint32_t HELPER(conv_df2uw_chop)(CPUHexagonState *env, 
float64 RssV)
 uint32_t RdV;
 arch_fpop_start(env);
 /* Hexagon checks the sign before rounding */
-if (float64_is_neg(RssV) && !float32_is_any_nan(RssV)) {
+if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
 float_raise(float_flag_invalid, >fp_status);
 RdV = 0;
 } else {
diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c
index 11415f8295..a531511cec 100644
--- a/tests/tcg/hexagon/usr.c
+++ b/tests/tcg/hexagon/usr.c
@@ -1068,6 +1068,10 @@ int main()
 TEST_P_OP_P(conv_df2d,DF_SNaN,  0xULL,  
USR_FPINVF);
 TEST_R_OP_P(conv_df2uw_chop,  DF_QNaN,  0x, 
USR_FPINVF);
 TEST_R_OP_P(conv_df2uw_chop,  DF_SNaN,  0x, 
USR_FPINVF);
+
+/* Test for typo in HELPER(conv_df2uw_chop) */
+TEST_R_OP_P(conv_df2uw_chop, 0xff7f0001ULL, 0x, 
USR_FPINVF);
+
 TEST_R_OP_P(conv_df2w_chop,   DF_QNaN,  0x, 
USR_FPINVF);
 TEST_R_OP_P(conv_df2w_chop,   DF_SNaN,  0x, 
USR_FPINVF);
 TEST_P_OP_P(conv_df2ud_chop,  DF_QNaN,  0xULL,  
USR_FPINVF);
-- 
2.17.1

[PULL 12/12] target/hexagon: remove unused variable

[Taylor Simpson]

From: Zongyuan Li 

When building with clang version 13.0.0 (eg. Fedora 13.0.0-3.fc35),
two unused variables introduced by macro GATHER_FUNCTION and
SCATTER_FUNCTION will cause building process failure due to
[-Werror -Wunused-variable].

Signed-off-by: Zongyuan Li 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/831
Message-Id: <20220124064339.56027-1-zongyuan...@smartx.com>
Reviewed-by: Thomas Huth 
Reviewed-by: Taylor Simpson 
Signed-off-by: Taylor Simpson 
---
 target/hexagon/mmvec/macros.h | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
index 10f4630364..8345753580 100644
--- a/target/hexagon/mmvec/macros.h
+++ b/target/hexagon/mmvec/macros.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -164,11 +164,9 @@
 target_ulong va = EA; \
 target_ulong va_high = EA + LEN; \
 uintptr_t ra = GETPC(); \
-int log_bank = 0; \
 int log_byte = 0; \
 for (i0 = 0; i0 < ELEMENT_SIZE; i0++) { \
 log_byte = ((va + i0) <= va_high) && QVAL; \
-log_bank |= (log_byte << i0); \
 uint8_t B; \
 B = cpu_ldub_data_ra(env, EA + i0, ra); \
 env->tmp_VRegs[0].ub[ELEMENT_SIZE * IDX + i0] = B; \
@@ -243,11 +241,9 @@
 int i0; \
 target_ulong va = EA; \
 target_ulong va_high = EA + LEN; \
-int log_bank = 0; \
 int log_byte = 0; \
 for (i0 = 0; i0 < ELEM_SIZE; i0++) { \
 log_byte = ((va + i0) <= va_high) && QVAL; \
-log_bank |= (log_byte << i0); \
 LOG_VTCM_BYTE(va + i0, log_byte, IN.ub[ELEM_SIZE * IDX + i0], \
   ELEM_SIZE * IDX + i0); \
 } \
-- 
2.17.1

[PULL 04/12] Hexagon (target/hexagon) properly handle denorm in arch_sf_recip_common

[Taylor Simpson]

The arch_sf_recip_common function was calling float32_getexp which
adjusts for denorm, but the we actually need the raw exponent bits.

This function is called from 3 instructions
sfrecipa
sffixupn
sffixupd

Test cases added to tests/tcg/hexagon/fpstuff.c

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-6-tsimp...@quicinc.com>
Reviewed-by: Richard Henderson 
---
 target/hexagon/fma_emu.h|  6 -
 target/hexagon/arch.c   |  6 ++---
 tests/tcg/hexagon/fpstuff.c | 44 ++---
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/target/hexagon/fma_emu.h b/target/hexagon/fma_emu.h
index e3b99a8cf4..91591d6050 100644
--- a/target/hexagon/fma_emu.h
+++ b/target/hexagon/fma_emu.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -24,6 +24,10 @@ static inline bool is_finite(float64 x)
 }
 
 int32_t float64_getexp(float64 f64);
+static inline uint32_t float32_getexp_raw(float32 f32)
+{
+return extract32(f32, 23, 8);
+}
 int32_t float32_getexp(float32 f32);
 float32 infinite_float32(uint8_t sign);
 float32 internal_fmafx(float32 a, float32 b, float32 c,
diff --git a/target/hexagon/arch.c b/target/hexagon/arch.c
index 68a55b3bd4..da79b41c4d 100644
--- a/target/hexagon/arch.c
+++ b/target/hexagon/arch.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -298,8 +298,8 @@ int arch_sf_recip_common(float32 *Rs, float32 *Rt, float32 
*Rd, int *adjust,
 } else {
 PeV = 0x00;
 /* Basic checks passed */
-n_exp = float32_getexp(RsV);
-d_exp = float32_getexp(RtV);
+n_exp = float32_getexp_raw(RsV);
+d_exp = float32_getexp_raw(RtV);
 if ((n_exp - d_exp + SF_BIAS) <= SF_MANTBITS) {
 /* Near quotient underflow / inexact Q */
 PeV = 0x80;
diff --git a/tests/tcg/hexagon/fpstuff.c b/tests/tcg/hexagon/fpstuff.c
index 0dff429f4c..043f18fab3 100644
--- a/tests/tcg/hexagon/fpstuff.c
+++ b/tests/tcg/hexagon/fpstuff.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2020-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2020-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -38,6 +38,8 @@ const int SF_NaN_special =0x7f81;
 const int SF_ANY =0x3f80;
 const int SF_HEX_NAN =0x;
 const int SF_small_neg =  0xab98fba8;
+const int SF_denorm = 0x0001;
+const int SF_random = 0x346001d6;
 
 const long long DF_NaN =  0x7ff8ULL;
 const long long DF_ANY =  0x3f80ULL;
@@ -250,10 +252,11 @@ static void check_dfminmax(void)
 check_fpstatus(usr, FPINVF);
 }
 
-static void check_recip_exception(void)
+static void check_sfrecipa(void)
 {
 int result;
 int usr;
+int pred;
 
 /*
  * Check that sfrecipa doesn't set status bits when
@@ -329,6 +332,17 @@ static void check_recip_exception(void)
  : "r2", "p0", "usr");
 check32(result, 0x3f80);
 check_fpstatus(usr, 0);
+
+/*
+ * Check that sfrecipa properly handles denorm
+ */
+asm (CLEAR_FPSTATUS
+ "%0,p0 = sfrecipa(%2, %3)\n\t"
+ "%1 = p0\n\t"
+ : "=r"(result), "=r"(pred) : "r"(SF_denorm), "r"(SF_random)
+ : "p0", "usr");
+check32(result, 0x6a920001);
+check32(pred, 0x80);
 }
 
 static void check_canonical_NaN(void)
@@ -455,6 +469,28 @@ static void check_invsqrta(void)
 check32(predval, 0x0);
 }
 
+static void check_sffixupn(void)
+{
+int result;
+
+/* Check that sffixupn properly deals with denorm */
+asm volatile("%0 = sffixupn(%1, %2)\n\t"
+ : "=r"(result)
+ : "r"(SF_random), "r"(SF_denorm));
+check32(result, 0x246001d6);
+}
+
+static void check_sffixupd(void)
+{
+int result;
+
+/* Check that sffixupd properly deals with denorm */
+asm volatile("%0 = sffixupd(%1, %2)\n\t"
+ : "=r"(result)
+ : "r"(SF_denorm), "r"(SF_random));
+check32(result, 0x146001d6);
+}
+
 static void check_float2int_convs()
 {
 int res32;
@@ -602,9 +638,11 @@ int main()
 check_compare_exception();
 check_sfminmax();
 

[PULL 06/12] Hexagon (tests/tcg/hexagon) test instructions that might set bits in USR

[Taylor Simpson]

Hexagon has ~200 instructions that set the saturate bit in USR, these
were broken into groups of similar instructions and one instruction
from each group is tested with at least one input that does not
saturate and at least one input that does saturate.

Signed-off-by: Taylor Simpson 
Message-Id: <20220210021556.9217-7-tsimp...@quicinc.com>
Acked-by: Richard Henderson 
---
 tests/tcg/hexagon/usr.c   | 798 ++
 tests/tcg/hexagon/Makefile.target |   8 +-
 2 files changed, 805 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/hexagon/usr.c

diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c
new file mode 100644
index 00..e82727237e
--- /dev/null
+++ b/tests/tcg/hexagon/usr.c
@@ -0,0 +1,798 @@
+/*
+ *  Copyright(c) 2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+/*
+ * Test instructions that might set bits in user status register (USR)
+ */
+
+#include 
+#include 
+
+int err;
+
+static void __check(int line, uint32_t val, uint32_t expect)
+{
+if (val != expect) {
+printf("ERROR at line %d: %d != %d\n", line, val, expect);
+err++;
+}
+}
+
+#define check(RES, EXP) __check(__LINE__, RES, EXP)
+
+static void __check32(int line, uint32_t val, uint32_t expect)
+{
+if (val != expect) {
+printf("ERROR at line %d: 0x%08x != 0x%08x\n", line, val, expect);
+err++;
+}
+}
+
+#define check32(RES, EXP) __check32(__LINE__, RES, EXP)
+
+static void __check64(int line, uint64_t val, uint64_t expect)
+{
+if (val != expect) {
+printf("ERROR at line %d: 0x%016llx != 0x%016llx\n", line, val, 
expect);
+err++;
+}
+}
+
+#define check64(RES, EXP) __check64(__LINE__, RES, EXP)
+
+/*
+ * Some of the instructions tested are only available on certain versions
+ * of the Hexagon core
+ */
+#define CORE_HAS_AUDIO(__HEXAGON_ARCH__ >= 67 && 
defined(__HEXAGON_AUDIO__))
+#define CORE_IS_V67   (__HEXAGON_ARCH__ >= 67)
+
+/* Define the bits in Hexagon USR register */
+#define USR_OVF_BIT  0/* Sticky saturation overflow */
+#define USR_FPINVF_BIT   1/* IEEE FP invalid sticky flag */
+#define USR_FPDBZF_BIT   2/* IEEE FP divide-by-zero sticky flag */
+#define USR_FPOVFF_BIT   3/* IEEE FP overflow sticky flag */
+#define USR_FPUNFF_BIT   4/* IEEE FP underflow sticky flag */
+#define USR_FPINPF_BIT   5/* IEEE FP inexact sticky flag */
+
+/* Corresponding values in USR */
+#define USR_CLEAR0
+#define USR_OVF  (1 << USR_OVF_BIT)
+#define USR_FPINVF   (1 << USR_FPINVF_BIT)
+#define USR_FPDBZF   (1 << USR_FPDBZF_BIT)
+#define USR_FPOVFF   (1 << USR_FPOVFF_BIT)
+#define USR_FPUNFF   (1 << USR_FPUNFF_BIT)
+#define USR_FPINPF   (1 << USR_FPINPF_BIT)
+
+/*
+ * Templates for functions to execute an instruction
+ *
+ * The templates vary by the number of arguments and the types of the args
+ * and result.  We use one letter in the macro name for the result and each
+ * argument:
+ * x unknown (specified in a subsequent template) or don't care
+ * R register (32 bits)
+ * P pair (64 bits)
+ * p predicate
+ * I immediate
+ * Xxread/write
+ */
+
+/* Clear bits 0-5 in USR */
+#define CLEAR_USRBITS \
+"r2 = usr\n\t" \
+"r2 = and(r2, #0xffc0)\n\t" \
+"usr = r2\n\t"
+
+/* Template for instructions with one register operand */
+#define FUNC_x_OP_x(RESTYPE, SRCTYPE, NAME, INSN) \
+static RESTYPE NAME(SRCTYPE src, uint32_t *usr_result) \
+{ \
+RESTYPE result; \
+uint32_t usr; \
+asm(CLEAR_USRBITS \
+INSN  "\n\t" \
+"%1 = usr\n\t" \
+: "=r"(result), "=r"(usr) \
+: "r"(src) \
+: "r2", "usr"); \
+  *usr_result = usr & 0x3f; \
+  return result; \
+}
+
+#define FUNC_R_OP_R(NAME, INSN) \
+FUNC_x_OP_x(uint32_t, uint32_t, NAME, INSN)
+
+#define FUNC_R_OP_P(NAME, INSN) \
+FUNC_x_OP_x(uint32_t, uint64_t, NAME, INSN)
+
+#define FUNC_P_OP_P(NAME, INSN) \
+FUNC_x_OP_x(uint64_t, uint64_t, NAME, INSN)
+
+#define FUNC_P_OP_R(NAME, INSN) \
+FUNC_x_OP_x(uint64_t, uint32_t, NAME, INSN)
+
+/*
+ * Template for instructions with a register and predicate result
+ * 

Re: [RFC PATCH 1/3] target/riscv: Rename timer & timecmp to mtimer and mtimecmp

[Alistair Francis]

On Fri, Mar 4, 2022 at 2:08 PM Anup Patel  wrote:
>
> On Fri, Mar 4, 2022 at 8:50 AM Atish Patra  wrote:
> >
> > Currently, the aclint and ibex timer devices uses the "timer" &
> > "timecmp" to generate the m-mode timer interrupt. In future,
> > we will have timer interrupt injected to S/VS mode directly.
> > No functionality change introduced in this patch.
>
> s/have timer/have a timer/
>
> >
> > Add a prefix "m" these enviornment variables to indicate its
> > true purpose.
>
> s/enviornment/environment/
>
> >
> > Signed-off-by: Atish Patra 
>
> I suggest we should remove mtimecmp and mtimer from
> target/riscv because mtimer is a memory mapped device.

I agree. I guess this is in the CPU as it's per hart, and it's easy to
store here instead of in the timer, but we could convert aclint to use
an array of timers (Ibex only ever has one hart).

That would probably be a closer match to hardware, as the timer is
external to the CPU

Alistair

>
> Regards,
> Anup
>
> > ---
> >  hw/intc/riscv_aclint.c | 20 ++--
> >  hw/timer/ibex_timer.c  | 14 +++---
> >  target/riscv/cpu.h |  4 ++--
> >  target/riscv/machine.c |  2 +-
> >  4 files changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> > index f1a5d3d284fd..642794a06865 100644
> > --- a/hw/intc/riscv_aclint.c
> > +++ b/hw/intc/riscv_aclint.c
> > @@ -59,8 +59,8 @@ static void 
> > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> >
> >  uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
> >
> > -cpu->env.timecmp = value;
> > -if (cpu->env.timecmp <= rtc_r) {
> > +cpu->env.mtimecmp = value;
> > +if (cpu->env.mtimecmp <= rtc_r) {
> >  /*
> >   * If we're setting an MTIMECMP value in the "past",
> >   * immediately raise the timer interrupt
> > @@ -71,7 +71,7 @@ static void 
> > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> >
> >  /* otherwise, set up the future timer interrupt */
> >  qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
> > -diff = cpu->env.timecmp - rtc_r;
> > +diff = cpu->env.mtimecmp - rtc_r;
> >  /* back to ns (note args switched in muldiv64) */
> >  uint64_t ns_diff = muldiv64(diff, NANOSECONDS_PER_SECOND, 
> > timebase_freq);
> >
> > @@ -96,7 +96,7 @@ static void 
> > riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> >  next = MIN(next, INT64_MAX);
> >  }
> >
> > -timer_mod(cpu->env.timer, next);
> > +timer_mod(cpu->env.mtimer, next);
> >  }
> >
> >  /*
> > @@ -127,11 +127,11 @@ static uint64_t riscv_aclint_mtimer_read(void 
> > *opaque, hwaddr addr,
> >"aclint-mtimer: invalid hartid: %zu", hartid);
> >  } else if ((addr & 0x7) == 0) {
> >  /* timecmp_lo */
> > -uint64_t timecmp = env->timecmp;
> > +uint64_t timecmp = env->mtimecmp;
> >  return timecmp & 0x;
> >  } else if ((addr & 0x7) == 4) {
> >  /* timecmp_hi */
> > -uint64_t timecmp = env->timecmp;
> > +uint64_t timecmp = env->mtimecmp;
> >  return (timecmp >> 32) & 0x;
> >  } else {
> >  qemu_log_mask(LOG_UNIMP,
> > @@ -168,14 +168,14 @@ static void riscv_aclint_mtimer_write(void *opaque, 
> > hwaddr addr,
> >"aclint-mtimer: invalid hartid: %zu", hartid);
> >  } else if ((addr & 0x7) == 0) {
> >  /* timecmp_lo */
> > -uint64_t timecmp_hi = env->timecmp >> 32;
> > +uint64_t timecmp_hi = env->mtimecmp >> 32;
> >  riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> > hartid,
> >  timecmp_hi << 32 | (value & 0x),
> >  mtimer->timebase_freq);
> >  return;
> >  } else if ((addr & 0x7) == 4) {
> >  /* timecmp_hi */
> > -uint64_t timecmp_lo = env->timecmp;
> > +uint64_t timecmp_lo = env->mtimecmp;
> >  riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> > hartid,
> >  value << 32 | (timecmp_lo & 0x),
> >  mtimer->timebase_freq);
> > @@ -304,9 +304,9 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, 
> > hwaddr size,
> >
> >  cb->s = RISCV_ACLINT_MTIMER(dev);
> >  cb->num = i;
> > -env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> > +env->mtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> >_aclint_mtimer_cb, cb);
> > -env->timecmp = 0;
> > +env->mtimecmp = 0;
> >
> >  qdev_connect_gpio_out(dev, i,
> >qdev_get_gpio_in(DEVICE(rvcpu), 
> > IRQ_M_TIMER));
> > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> > index 8c2ca364daab..4c34f9e08282 100644
> > --- a/hw/timer/ibex_timer.c
> > +++ 

Re: [PATCH] hw/dma/xlnx_csu_dma: Set TYPE_XLNX_CSU_DMA class_size

[Alistair Francis]

On Wed, Mar 9, 2022 at 1:07 AM Peter Maydell  wrote:
>
> In commit 00f05c02f9e7342f we gave the TYPE_XLNX_CSU_DMA object its
> own class struct, but forgot to update the TypeInfo::class_size
> accordingly.  This meant that not enough memory was allocated for the
> class struct, and the initialization of xcdc->read in the class init
> function wrote off the end of the memory. Add the missing line.
>
> Found by running 'check-qtest-aarch64' with a clang
> address-sanitizer build, which complains:
>
> ==2542634==ERROR: AddressSanitizer: heap-buffer-overflow on address 
> 0x6100ab00 at pc 0x559a20aebc29 bp 0x7fff97df74d0 sp 0x7fff97df74c8
> WRITE of size 8 at 0x6100ab00 thread T0
> #0 0x559a20aebc28 in xlnx_csu_dma_class_init 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../hw/dma/xlnx_csu_dma.c:722:16
> #1 0x559a21bf297c in type_initialize 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:365:9
> #2 0x559a21bf3442 in object_class_foreach_tramp 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1070:5
> #3 0x7f09bcb641b7 in g_hash_table_foreach 
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x401b7)
> #4 0x559a21bf3c27 in object_class_foreach 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1092:5
> #5 0x559a21bf3c27 in object_class_get_list 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1149:5
> #6 0x559a2081a2fd in select_machine 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:1661:24
> #7 0x559a2081a2fd in qemu_create_machine 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:2146:35
> #8 0x559a2081a2fd in qemu_init 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:3706:5
> #9 0x559a20720ed5 in main 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/main.c:49:5
> #10 0x7f09baec00b2 in __libc_start_main 
> /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
> #11 0x559a2067673d in _start 
> (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/qemu-system-aarch64+0xf4b73d)
>
> 0x6100ab00 is located 0 bytes to the right of 192-byte region 
> [0x6100aa40,0x6100ab00)
> allocated by thread T0 here:
> #0 0x559a206eeff2 in calloc 
> (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/qemu-system-aarch64+0xfc3ff2)
> #1 0x7f09bcb7bef0 in g_malloc0 
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57ef0)
> #2 0x559a21bf3442 in object_class_foreach_tramp 
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1070:5
>
> Fixes: 00f05c02f9e7342f ("hw/dma/xlnx_csu_dma: Support starting a read 
> transfer through a class method")
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/dma/xlnx_csu_dma.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c
> index 84f782fcdc0..60ada3286b4 100644
> --- a/hw/dma/xlnx_csu_dma.c
> +++ b/hw/dma/xlnx_csu_dma.c
> @@ -744,6 +744,7 @@ static const TypeInfo xlnx_csu_dma_info = {
>  .parent= TYPE_SYS_BUS_DEVICE,
>  .instance_size = sizeof(XlnxCSUDMA),
>  .class_init= xlnx_csu_dma_class_init,
> +.class_size= sizeof(XlnxCSUDMAClass),
>  .instance_init = xlnx_csu_dma_init,
>  .interfaces = (InterfaceInfo[]) {
>  { TYPE_STREAM_SINK },
> --
> 2.25.1
>
>

Re: [PATCH v4 22/33] target/nios2: Introduce dest_gpr

[Richard Henderson]

On 3/8/22 01:07, Peter Maydell wrote:

I assume the TCG dead-code elimination will mostly throw away the
write-to-R_ZERO stuff, but here for rdctl I suspect it won't.


I believe it will.  We don't indicate that normal load has side effects (as opposed to 
guest load), so it should all fall out of unused values.



But probably real code doesn't do that kind of silly thing.


Indeed.


r~

Re: [PATCH v4 17/33] target/nios2: Prevent writes to read-only or reserved control fields

[Richard Henderson]

On 3/8/22 10:24, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


Create an array of masks which detail the writable and readonly
bits for each control register.  Apply them when writing to
control registers.

Signed-off-by: Richard Henderson 



diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index f2813d3b47..189adf111c 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -88,6 +88,55 @@ static void nios2_cpu_initfn(Object *obj)

  cpu_set_cpustate_pointers(cpu);

+/* Begin with all fields of all registers are reserved. */
+memset(cpu->cr_state, 0, sizeof(cpu->cr_state));
+
+/*
+ * The combination of writable and readonly is the set of all
+ * non-reserved fields.  We apply writable as a mask to bits,
+ * and merge in existing readonly bits, before storing.
+ */
+#define WR_REG(C)   cpu->cr_state[C].writable = -1
+#define RO_REG(C)   cpu->cr_state[C].readonly = -1
+#define WR_FIELD(C, F)  cpu->cr_state[C].writable |= R_##C##_##F##_MASK
+#define RO_FIELD(C, F)  cpu->cr_state[C].readonly |= R_##C##_##F##_MASK
+
+WR_FIELD(CR_STATUS, PIE);


I think you need to claim (CR_STATUS, RSIE) is a RO bit, because without
EIC it's should-be-one.


That's patch 19.




+WR_REG(CR_ESTATUS);
+WR_REG(CR_BSTATUS);
+RO_REG(CR_CPUID);
+WR_FIELD(CR_EXCEPTION, CAUSE);
+WR_REG(CR_BADADDR);
+
+/* TODO: These control registers are not present with the EIC. */
+WR_REG(CR_IENABLE);
+RO_REG(CR_IPENDING);


Missing CR_CONFIG register ?


Present with MPU or ECC, and we implement neither.  Perhaps these should be listed below 
with the TODO?





+
+if (cpu->mmu_present) {
+WR_FIELD(CR_STATUS, U);
+WR_FIELD(CR_STATUS, EH);


True by the documentation, but we don't seem to prevent EH from
being set to 1 when we take an exception on the no-MMU config...


Yeah, I noticed this when cleaning things up in patch 28, but didn't want to change things 
too much in that patch.  I also don't have a no-mmu kernel to test against.



+WR_FIELD(CR_TLBMISC, WR);


(the docs call this field 'WE', incidentally)


Yeah, noticed that and meant to fix it, but forgot.


+WR_FIELD(CR_TLBMISC, RD);


If you claim this bit to be writable you'll allow the gdbstub
to set it, which is probably not what you want. (Actual writes to
this register are handled via the helper function.)


Mm.  Perhaps calling it reserved is the easiest way out of that.  For these mmu registers, 
we don't apply the two masks, but pass it all to the helper.  I'm open to ideas there.



+WR_FIELD(CR_TLBMISC, WAY);


Missing PID field ?


Yep, thanks.


+WR_REG(CR_TLBACC);



+}


You don't enforce the reserved/readonly bits on status when
we copy it from estatus during eret. (That change appears later,
in patch 26.)


Yep.  I could move the masking back to this patch, leave only the estatus/sstatus change 
to patch 26.



The same *ought* to apply for bret, except that we have a bug in
our implementation of it, where we fail to copy bstatus into status...


Hah, thanks, yes.


r~

Re: [PATCH v4 17/33] target/nios2: Prevent writes to read-only or reserved control fields

[Peter Maydell]

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:
>
> Create an array of masks which detail the writable and readonly
> bits for each control register.  Apply them when writing to
> control registers.
>
> Signed-off-by: Richard Henderson 

> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index f2813d3b47..189adf111c 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -88,6 +88,55 @@ static void nios2_cpu_initfn(Object *obj)
>
>  cpu_set_cpustate_pointers(cpu);
>
> +/* Begin with all fields of all registers are reserved. */
> +memset(cpu->cr_state, 0, sizeof(cpu->cr_state));
> +
> +/*
> + * The combination of writable and readonly is the set of all
> + * non-reserved fields.  We apply writable as a mask to bits,
> + * and merge in existing readonly bits, before storing.
> + */
> +#define WR_REG(C)   cpu->cr_state[C].writable = -1
> +#define RO_REG(C)   cpu->cr_state[C].readonly = -1
> +#define WR_FIELD(C, F)  cpu->cr_state[C].writable |= R_##C##_##F##_MASK
> +#define RO_FIELD(C, F)  cpu->cr_state[C].readonly |= R_##C##_##F##_MASK
> +
> +WR_FIELD(CR_STATUS, PIE);

I think you need to claim (CR_STATUS, RSIE) is a RO bit, because without
EIC it's should-be-one.

> +WR_REG(CR_ESTATUS);
> +WR_REG(CR_BSTATUS);
> +RO_REG(CR_CPUID);
> +WR_FIELD(CR_EXCEPTION, CAUSE);
> +WR_REG(CR_BADADDR);
> +
> +/* TODO: These control registers are not present with the EIC. */
> +WR_REG(CR_IENABLE);
> +RO_REG(CR_IPENDING);

Missing CR_CONFIG register ?

> +
> +if (cpu->mmu_present) {
> +WR_FIELD(CR_STATUS, U);
> +WR_FIELD(CR_STATUS, EH);

True by the documentation, but we don't seem to prevent EH from
being set to 1 when we take an exception on the no-MMU config...

> +
> +WR_FIELD(CR_PTEADDR, VPN);
> +WR_FIELD(CR_PTEADDR, PTBASE);
> +
> +RO_FIELD(CR_TLBMISC, D);
> +RO_FIELD(CR_TLBMISC, PERM);
> +RO_FIELD(CR_TLBMISC, BAD);
> +RO_FIELD(CR_TLBMISC, DBL);
> +WR_FIELD(CR_TLBMISC, WR);

(the docs call this field 'WE', incidentally)

> +WR_FIELD(CR_TLBMISC, RD);

If you claim this bit to be writable you'll allow the gdbstub
to set it, which is probably not what you want. (Actual writes to
this register are handled via the helper function.)

> +WR_FIELD(CR_TLBMISC, WAY);

Missing PID field ?

> +
> +WR_REG(CR_TLBACC);

> +}

You don't enforce the reserved/readonly bits on status when
we copy it from estatus during eret. (That change appears later,
in patch 26.)

The same *ought* to apply for bret, except that we have a bug in
our implementation of it, where we fail to copy bstatus into status...

The machinery itself looks OK.

thanks
-- PMM

Re: [PATCH v4 33/33] hw/nios2: Machine with a Vectored Interrupt Controller

[Richard Henderson]

On 3/7/22 22:43, Mark Cave-Ayland wrote:

+    qdev_realize(DEVICE(cpu), NULL, _fatal);
+    object_unref(CPU(cpu));


I believe this can be replaced with qdev_realize_and_unref()?


Oh, nice.  I copied this from hw/arm/virt, which has code between these two 
points.


+    if (nms->vic) {
+    DeviceState *dev = qdev_new("nios2-vic");


And with a separate header for nios2_vic.h you can include that and use TYPE_NIOS2_VIC 
here instead of hard-coding the type name.


Ok.


r~

Re: [PATCH v4 31/33] hw/nios2: Introduce Nios2MachineState

[Richard Henderson]

On 3/7/22 22:39, Mark Cave-Ayland wrote:

  static void nios2_10m50_ghrd_init(MachineState *machine)
  {
+    Nios2MachineState *nms = NIOS2_MACHINE(machine);
  Nios2CPU *cpu;
  DeviceState *dev;
  MemoryRegion *address_space_mem = get_system_memory();
@@ -101,15 +109,29 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
  cpu->exception_addr = 0xc8000120;
  cpu->fast_tlb_miss_addr = 0xc100;
-    nios2_load_kernel(cpu, ram_base, ram_size, machine->initrd_filename,
+    nios2_load_kernel(cpu, ram_base, ram_size, nms->parent_obj.initrd_filename,
    BINARY_DEVICE_TREE_FILE, NULL);


I think you should be able to keep this as machine->initrd_filename? Certainly there 
should be no direct access to parent_obj here, and if you did need it a QOM cast macro 
would be the way to do this.


Ok.


+static const TypeInfo nios2_10m50_ghrd_type_info = {
+    .name  = TYPE_NIOS2_MACHINE,
+    .parent    = TYPE_MACHINE,
+    .instance_size = sizeof(Nios2MachineState),
+    .class_size    = sizeof(MachineClass),


Technically you can drop .class_size here since this should be inherited automatically 
from MachineClass.


Ok.  This is a leftover from an intermediate when Nios2MachineClass existed.


r~

Re: [PATCH v4 30/33] hw/intc: Vectored Interrupt Controller (VIC)

[Richard Henderson]

On 3/8/22 01:27, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


From: Amir Gonnen 

Implement nios2 Vectored Interrupt Controller (VIC).
VIC is connected to EIC. It needs to update rha, ril, rrs and rnmi
fields on Nios2CPU before raising an IRQ.
For that purpose, VIC has a "cpu" property which should refer to the
nios2 cpu and set by the board that connects VIC.

Signed-off-by: Amir Gonnen 
Message-Id: <20220303153906.2024748-5-amir.gon...@neuroblade.ai>
Signed-off-by: Richard Henderson 
---


I reviewed the version of this patch that was in Amir's v3 -- has
it changed, and if so how, or did you just drop the R-by by accident?


I don't believe that anything changed in this patch. I used tooling to grab the patches + 
tags, so I'm not sure what went wrong.



r~

Re: [PATCH v4 18/33] target/nios2: Implement cpuid

[Richard Henderson]

On 3/8/22 00:52, Peter Maydell wrote:

I guess. This will have no effect as all our nios2 boards are
single-CPU.

Reviewed-by: Peter Maydell 


Oh, fair enough.  I didn't even think of that (even though I've just spent quite a bit of 
time on interrupts, and there's no sign of an inter-processor interrupt).



r~

Re: [PATCH v4 21/33] target/nios2: Use tcg_constant_tl

[Richard Henderson]

On 3/8/22 01:00, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


Replace current uses of tcg_const_tl, and remove the frees.

Signed-off-by: Richard Henderson 
---



@@ -675,8 +663,8 @@ static void divu(DisasContext *dc, uint32_t code, uint32_t 
flags)

  TCGv t0 = tcg_temp_new();
  TCGv t1 = tcg_temp_new();
-TCGv t2 = tcg_const_tl(0);
-TCGv t3 = tcg_const_tl(1);
+TCGv t2 = tcg_constant_tl(0);
+TCGv t3 = tcg_constant_tl(1);


Maybe just use tcg_constant_tl(0) and (1) in-place at
the only two uses of t2, t3 rather than retaining the TCGv
local variables ?


Sure.


r~

Re: [PATCH v4 17/33] target/nios2: Prevent writes to read-only or reserved control fields

[Richard Henderson]

On 3/8/22 00:57, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


Create an array of masks which detail the writable and readonly
bits for each control register.  Apply them when writing to
control registers.

Signed-off-by: Richard Henderson 


What's the justification for this extra machinery? Does
existing guest code rely on writes to r/o bits being ignored ?


During review of a previous edition of this patch set, I asked myself: why isn't Amir 
changing the shadow register set on WRCTL to STATUS, as the CRS field could change.


The answer is that the architecture disallows this, by making the CRS field read-only from 
software.  CRS can only be modified by interrupt entry and return.


Then I looked further and found more read-only fields, and lots of fields that are 
reserved unless the appropriate cpu options are enabled.  Again thining of CRS more so to 
PRS when shadow register sets are *not* enabled -- we don't want software to put us into 
some weird state.


I probably should have put all that in the patch description.  :-)


r~

Re: [PATCH v4 12/33] target/nios2: Use hw/registerfields.h for CR_EXCEPTION fields

[Richard Henderson]

On 3/8/22 00:12, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


Sink the set of env->exception to the end of nios2_cpu_do_interrupt.


This splits the two things the patch is doing between the subject line
and the commit message body; the subject is supposed to be a summary
and the body should be able to stand alone without the subject.


Yea, I should merge this sink to patch 28, which does other cleanup to this 
function.


+env->exception = FIELD_DP32(env->exception, CR_EXCEPTION, CAUSE,
+cs->exception_index);
  }


This is a behaviour change in the semihosting case, which
previously did not change env->exception and now does.
Since that's guest visible I think it's not correct.
You could fix that by making the semihosting handling end
with 'return' rather than 'break'.


Yea, poor patch ordering, as you discovered.


r~

Re: [PATCH v4 15/33] target/nios2: Use hw/registerfields.h for CR_TLBMISC fields

[Richard Henderson]

On 3/8/22 00:46, Peter Maydell wrote:

@@ -130,24 +128,25 @@ void helper_mmu_write_tlbacc(CPUNios2State *env, uint32_t 
v)
  void helper_mmu_write_tlbmisc(CPUNios2State *env, uint32_t v)
  {
  Nios2CPU *cpu = env_archcpu(env);
+uint32_t new_pid = FIELD_EX32(v, CR_TLBMISC, PID);
+uint32_t old_pid = FIELD_EX32(env->mmu.tlbmisc_wr, CR_TLBMISC, PID);
+uint32_t way = FIELD_EX32(v, CR_TLBMISC, WAY);

-trace_nios2_mmu_write_tlbmisc(v >> CR_TLBMISC_WAY_SHIFT,
+trace_nios2_mmu_write_tlbmisc(way,
(v & CR_TLBMISC_RD) ? 'R' : '.',
(v & CR_TLBMISC_WR) ? 'W' : '.',
(v & CR_TLBMISC_DBL) ? '2' : '.',
(v & CR_TLBMISC_BAD) ? 'B' : '.',
(v & CR_TLBMISC_PERM) ? 'P' : '.',
(v & CR_TLBMISC_D) ? 'D' : '.',
-  (v & CR_TLBMISC_PID_MASK) >> 4);
+  new_pid);

-if ((v & CR_TLBMISC_PID_MASK) !=
-(env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK)) {
-mmu_flush_pid(env, (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >>
-   CR_TLBMISC_PID_SHIFT);
+if (new_pid != old_pid) {
+mmu_flush_pid(env, old_pid);
  }
+
  /* if tlbmisc.RD == 1 then trigger a TLB read on writes to TLBMISC */
  if (v & CR_TLBMISC_RD) {
-int way = (v >> CR_TLBMISC_WAY_SHIFT);
  int vpn = FIELD_EX32(env->mmu.pteaddr_wr, CR_PTEADDR, VPN);
  Nios2TLBEntry *entry =
  >mmu.tlb[(way * cpu->tlb_num_ways) +



Any reason for hoisting the declaration of 'way' up to the top of the
function ?


For use in the tracepoint, rather than extracting it twice.


r~

Re: [PATCH v4 11/33] target/nios2: Use hw/registerfields.h for CR_STATUS fields

[Richard Henderson]

On 3/8/22 00:08, Peter Maydell wrote:

+#define CR_STATUS_RSIE (1u << R_CR_STATUS_RSIE_SHIFT)


Since these are all 1 bit fields you can use
#define CR_STATUS_PIE R_CR_STATUS_PIE_MASK
etc rather than manually shifting by the shift count.


Quite right, thanks.

r~

Re: [PATCH v4 07/33] linux-user/nios2: Trim target_pc_regs to sp and pc

[Richard Henderson]

On 3/8/22 00:00, Peter Maydell wrote:

On Tue, 8 Mar 2022 at 07:20, Richard Henderson
 wrote:


The only thing this struct is used for is passing startup values
from elfload.c to the cpu.  We do not need all registers to be
represented, we do not need the kernel internal stack slots.

The userland argc, argv, and envp values are passed on
the stack, so only SP and PC need updating.

Signed-off-by: Richard Henderson 
---
  linux-user/nios2/target_syscall.h | 25 ++---
  linux-user/elfload.c  |  3 +--
  linux-user/nios2/cpu_loop.c   | 24 +---
  3 files changed, 4 insertions(+), 48 deletions(-)


Well, I guess we're not using it for anything else currently,
but if you do this then it's not the target arch's pt_regs
struct any more. And all our other target archs seem to define
the struct to follow the kernel definition even if we don't
happen to use it all.


Yes, something I've meant to clean up for ages.

My real goal here was removing the reference to estatus, which then does not need to be 
converted to a different form in the coming patches.


(Assigning to estatus is unusable in user-mode, because rdctl and eret are supervisor 
insns.  This code appears to be mirroring the kernel code in which it is trying to get the 
right bits ready for the context switch to user-mode.  For qemu, we've done that for 
user-only during reset, and with proper symbolic constants, so no need to do it again here.)


I could just remove the two mentions of estatus in init_thread and copy_regs, if that 
seems better.  Leave the target_pt_regs cleanup to a larger patch set touching them all.



r~

Re: [PULL 0/7] s390x, tests and misc patches

[Peter Maydell]

On Mon, 7 Mar 2022 at 18:26, Thomas Huth  wrote:
>
>  Hi Peter!
>
> The following changes since commit 9d662a6b22a0838a85c5432385f35db2488a33a5:
>
>   Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220305' into 
> staging (2022-03-05 18:03:15 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/thuth/qemu.git tags/pull-request-2022-03-07
>
> for you to fetch changes up to 818e1636080768749dc826acd4825e71828ec7e6:
>
>   Check and report for incomplete 'global' option format (2022-03-07 19:00:05 
> +0100)
>
> 
> * Fixes for s390x TCG tests
> * Update Haiku VM to a usable level
> * Some other miscellaneous small fixes


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

Re: [PATCH v4] Hexagon (target/hexagon) properly handle NaN in dfmin/dfmax/sfmin/sfmax

[Richard Henderson]

On 3/8/22 09:04, Taylor Simpson wrote:

The float??_minnum implementation differs from Hexagon for SNaN,
it returns NaN, but Hexagon returns the other input.  So, we use
float??_minimum_number.

Test cases added to tests/tcg/hexagon/fpstuff.c

Signed-off-by: Taylor Simpson
---
  target/hexagon/op_helper.c  | 14 ++-
  tests/tcg/hexagon/fpstuff.c | 79 +
  2 files changed, 66 insertions(+), 27 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH] tests/avocado: Cancel BootLinux tests in case there is no free port

[Thomas Huth]

On 07/03/2022 19.48, Daniel P. Berrangé wrote:

On Mon, Mar 07, 2022 at 07:31:50PM +0100, Thomas Huth wrote:

On 07/03/2022 13.50, Daniel P. Berrangé wrote:

On Mon, Feb 28, 2022 at 12:43:25PM +0100, Thomas Huth wrote:

The BootLinux tests are currently failing with an ugly python
stack trace on my RHEL8 system since they cannot get a free port
(likely due to the firewall settings on my system). Let's properly
check the return value of find_free_port() instead and cancel the
test gracefully if it cannot get a free port.

Signed-off-by: Thomas Huth 
---
   Unfortunately, it still takes > 70 seconds for each and every
   tests from tests/avocado/boot_linux.py to get canceled, so
   tests/avocado/boot_linux.py still renders "make check-avocado"
   for me pretty unusable... looking at the implementation of
   find_free_port() in Avocado, I wonder whether there isn't a
   better way to get a free port number in Python? Brute-forcing
   all ports between 1024 and 65536 seems just quite cumbersome
   to me...


Even in the worst case of testing every single port,
for INET and INET6 and for STREAM and DGRAM sockets,
that find_free_port port completes in a couple of
seconds.


Weird, on my system, the test runs for 70 seconds, just to finally
discovered that there was no free port available.


Incidentally I'm really suprised you even hit the 'no free port'
scenario. I've never seen that happen unless the machine was
basically doomed due to something leaking open sockets by the
10's of 1000's.

You mention firewall settings above, but I don't think that's
relevant. The find_free_port() call is with no args, so it
gets set to 'address=localhost' which means is_port_free
takes the bind() codepath. The firewall has no interaction
with the bind() codepath in the kernel AFAIK.


Yes, I've now had another closer look, and indeed, the firewall is not the 
problem here - but is_port_free() seems to be very buggy. The problem is 
that I do not have any IPv6 address configured on my system, and in that 
case the current code fails.
See https://github.com/avocado-framework/avocado/issues/5273 ... I need this 
patch to fix it:


diff --git a/avocado/utils/network/ports.py b/avocado/utils/network/ports.py
--- a/avocado/utils/network/ports.py
+++ b/avocado/utils/network/ports.py
@@ -60,7 +60,7 @@ def is_port_free(port, address):
 if localhost:
 return False
 sock.close()
-return True
+return True
 finally:
 if sock is not None:
 sock.close()

 Thomas

[PATCH v4] Hexagon (target/hexagon) properly handle NaN in dfmin/dfmax/sfmin/sfmax

[Taylor Simpson]

The float??_minnum implementation differs from Hexagon for SNaN,
it returns NaN, but Hexagon returns the other input.  So, we use
float??_minimum_number.

Test cases added to tests/tcg/hexagon/fpstuff.c

Signed-off-by: Taylor Simpson 
---
 target/hexagon/op_helper.c  | 14 ++-
 tests/tcg/hexagon/fpstuff.c | 79 +
 2 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 057baf9a48..33a86be7be 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -948,7 +948,7 @@ float32 HELPER(sfmax)(CPUHexagonState *env, float32 RsV, 
float32 RtV)
 {
 float32 RdV;
 arch_fpop_start(env);
-RdV = float32_maxnum(RsV, RtV, >fp_status);
+RdV = float32_maximum_number(RsV, RtV, >fp_status);
 arch_fpop_end(env);
 return RdV;
 }
@@ -957,7 +957,7 @@ float32 HELPER(sfmin)(CPUHexagonState *env, float32 RsV, 
float32 RtV)
 {
 float32 RdV;
 arch_fpop_start(env);
-RdV = float32_minnum(RsV, RtV, >fp_status);
+RdV = float32_minimum_number(RsV, RtV, >fp_status);
 arch_fpop_end(env);
 return RdV;
 }
@@ -1041,10 +1041,7 @@ float64 HELPER(dfmax)(CPUHexagonState *env, float64 
RssV, float64 RttV)
 {
 float64 RddV;
 arch_fpop_start(env);
-RddV = float64_maxnum(RssV, RttV, >fp_status);
-if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
-float_raise(float_flag_invalid, >fp_status);
-}
+RddV = float64_maximum_number(RssV, RttV, >fp_status);
 arch_fpop_end(env);
 return RddV;
 }
@@ -1053,10 +1050,7 @@ float64 HELPER(dfmin)(CPUHexagonState *env, float64 
RssV, float64 RttV)
 {
 float64 RddV;
 arch_fpop_start(env);
-RddV = float64_minnum(RssV, RttV, >fp_status);
-if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
-float_raise(float_flag_invalid, >fp_status);
-}
+RddV = float64_minimum_number(RssV, RttV, >fp_status);
 arch_fpop_end(env);
 return RddV;
 }
diff --git a/tests/tcg/hexagon/fpstuff.c b/tests/tcg/hexagon/fpstuff.c
index 0dff429f4c..7a65ebc74d 100644
--- a/tests/tcg/hexagon/fpstuff.c
+++ b/tests/tcg/hexagon/fpstuff.c
@@ -39,7 +39,8 @@ const int SF_ANY =0x3f80;
 const int SF_HEX_NAN =0x;
 const int SF_small_neg =  0xab98fba8;
 
-const long long DF_NaN =  0x7ff8ULL;
+const long long DF_QNaN = 0x7ff8ULL;
+const long long DF_SNaN = 0x7ff7ULL;
 const long long DF_ANY =  0x3f80ULL;
 const long long DF_HEX_NAN =  0xULL;
 const long long DF_small_neg =0xbd731f75ULL;
@@ -126,7 +127,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.eq(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -135,7 +136,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.gt(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -144,7 +145,7 @@ static void check_compare_exception(void)
  "p0 = dfcmp.ge(%2, %3)\n\t"
  "%0 = p0\n\t"
  "%1 = usr\n\t"
- : "=r"(cmp), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(cmp), "=r"(usr) : "r"(DF_QNaN), "r"(DF_ANY)
  : "r2", "p0", "usr");
 check32(cmp, 0);
 check_fpstatus(usr, 0);
@@ -206,7 +207,7 @@ static void check_dfminmax(void)
 int usr;
 
 /*
- * Execute dfmin/dfmax instructions with one operand as NaN
+ * Execute dfmin/dfmax instructions with one operand as SNaN
  * Check that
  * Result is the other operand
  * Invalid bit in USR is set
@@ -214,7 +215,7 @@ static void check_dfminmax(void)
  asm (CLEAR_FPSTATUS
  "%0 = dfmin(%2, %3)\n\t"
  "%1 = usr\n\t"
- : "=r"(minmax), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(minmax), "=r"(usr) : "r"(DF_SNaN), "r"(DF_ANY)
  : "r2", "usr");
 check64(minmax, DF_ANY);
 check_fpstatus(usr, FPINVF);
@@ -222,13 +223,35 @@ static void check_dfminmax(void)
 asm (CLEAR_FPSTATUS
  "%0 = dfmax(%2, %3)\n\t"
  "%1 = usr\n\t"
- : "=r"(minmax), "=r"(usr) : "r"(DF_NaN), "r"(DF_ANY)
+ : "=r"(minmax), "=r"(usr) : "r"(DF_SNaN), "r"(DF_ANY)
  : "r2", "usr");
 check64(minmax, DF_ANY);
 check_fpstatus(usr, FPINVF);
 
 /*
- * Execute dfmin/dfmax instructions with both operands NaN
+ * Execute dfmin/dfmax instructions with one operand as QNaN
+ * 

Re: [PATCH] hw/misc/npcm7xx_clk: Don't leak string in npcm7xx_clk_sel_init()

[Hao Wu]

On Tue, Mar 8, 2022 at 10:09 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 3/8/22 07:03, Peter Maydell wrote:
> > In npcm7xx_clk_sel_init() we allocate a string with g_strdup_printf().
> > Use g_autofree so we free it rather than leaking it.
> >
> > (Detected with the clang leak sanitizer.)
> >
> > Signed-off-by: Peter Maydell
> > ---
> >   hw/misc/npcm7xx_clk.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson 
>
> Reviewed-by: Hao Wu 

> r~
>
>

Re: [PULL 00/18] migration queue

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philippe.mathieu.da...@gmail.com) wrote:
> On 3/3/22 15:46, Peter Maydell wrote:
> > On Wed, 2 Mar 2022 at 18:32, Dr. David Alan Gilbert (git)
> >  wrote:
> > > 
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > The following changes since commit 
> > > 64ada298b98a51eb2512607f6e6180cb330c47b1:
> > > 
> > >Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220302' 
> > > into staging (2022-03-02 12:38:46 +)
> > > 
> > > are available in the Git repository at:
> > > 
> > >https://gitlab.com/dagrh/qemu.git tags/pull-migration-20220302b
> > > 
> > > for you to fetch changes up to 18621987027b1800f315fb9e29967e7b5398ef6f:
> > > 
> > >migration: Remove load_state_old and minimum_version_id_old 
> > > (2022-03-02 18:20:45 +)
> > > 
> > > 
> > > Migration/HMP/Virtio pull 2022-03-02
> > > 
> > > A bit of a mix this time:
> > >* Minor fixes from myself, Hanna, and Jack
> > >* VNC password rework by Stefan and Fabian
> > >* Postcopy changes from Peter X that are
> > >  the start of a larger series to come
> > >* Removing the prehistoic load_state_old
> > >  code from Peter M
> 
> I'm seeing an error on the s390x runner:
> 
> ▶  26/547 ERROR:../tests/qtest/migration-test.c:276:check_guests_ram:
> assertion failed: (bad == 0) ERROR
> 
>  26/547 qemu:qtest+qtest-i386 / qtest-i386/migration-testERROR
> 78.87s   killed by signal 6 SIGABRT
> 
> https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/562515884#L7848

Yeh, thuth mentioned that, it seems to only be s390 which is odd.
I'm not seeing anything obviously architecture dependent in that set, or
for that matter that plays with the ram migration stream much.
Is this reliable enough that someone with a tame s390 could bisect?

Dave

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PULL 00/18] migration queue

[Philippe Mathieu-Daudé]

On 3/3/22 15:46, Peter Maydell wrote:

On Wed, 2 Mar 2022 at 18:32, Dr. David Alan Gilbert (git)
 wrote:


From: "Dr. David Alan Gilbert" 

The following changes since commit 64ada298b98a51eb2512607f6e6180cb330c47b1:

   Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220302' into 
staging (2022-03-02 12:38:46 +)

are available in the Git repository at:

   https://gitlab.com/dagrh/qemu.git tags/pull-migration-20220302b

for you to fetch changes up to 18621987027b1800f315fb9e29967e7b5398ef6f:

   migration: Remove load_state_old and minimum_version_id_old (2022-03-02 
18:20:45 +)


Migration/HMP/Virtio pull 2022-03-02

A bit of a mix this time:
   * Minor fixes from myself, Hanna, and Jack
   * VNC password rework by Stefan and Fabian
   * Postcopy changes from Peter X that are
 the start of a larger series to come
   * Removing the prehistoic load_state_old
 code from Peter M


I'm seeing an error on the s390x runner:

▶  26/547 ERROR:../tests/qtest/migration-test.c:276:check_guests_ram: 
assertion failed: (bad == 0) ERROR


 26/547 qemu:qtest+qtest-i386 / qtest-i386/migration-test 
   ERROR  78.87s   killed by signal 6 SIGABRT


https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/562515884#L7848

Re: [PATCH 07/13] hw/mips/gt64xxx_pci: Resolve gt64120_register()

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

Now that gt64120_register() lost its pic parameter, there is an
opportunity to remove it. gt64120_register() is old style by wrapping
qdev API, and the new style is to use qdev directly. So take the
opportunity and modernize the code.

Suggested-by: BALATON Zoltan
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: BALATON Zoltan
Message-Id:<20220217101924.15347-8-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/mips/gt64xxx_pci.c  | 21 -
  hw/mips/malta.c|  3 ++-
  include/hw/mips/mips.h |  3 ---
  3 files changed, 6 insertions(+), 21 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PATCH 2/2] hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GICV3_TCG is unset

[Eric Auger]

In TCG mode, if gic-version=max we always select GICv3 even if
CONFIG_ARM_GICV3_TCG is unset. We shall rather select GICv2.
This also brings the benefit of fixing qos tests errors for tests
using gic-version=max with CONFIG_ARM_GICV3_TCG unset.

Signed-off-by: Eric Auger 

---

v2 -> v3:
- Use module_object_class_by_name() and refer to the renamed
  CONFIG_ARM_GICV3_TCG config
---
 hw/arm/virt.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 46bf7ceddf..39790d29d2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1852,7 +1852,12 @@ static void finalize_gic_version(VirtMachineState *vms)
 vms->gic_version = VIRT_GIC_VERSION_2;
 break;
 case VIRT_GIC_VERSION_MAX:
-vms->gic_version = VIRT_GIC_VERSION_3;
+if (module_object_class_by_name("arm-gicv3")) {
+/* CONFIG_ARM_GICV3_TCG was set */
+vms->gic_version = VIRT_GIC_VERSION_3;
+} else {
+vms->gic_version = VIRT_GIC_VERSION_2;
+}
 break;
 case VIRT_GIC_VERSION_HOST:
 error_report("gic-version=host requires KVM");
-- 
2.26.3

[PATCH 0/2] hw/arm/virt: Fix make check-qtest-aarch64 when CONFIG_ARM_GIC_TCG is unset

[Eric Auger]

When CONFIG_ARM_GIC_TCG is unset, qtests fail with
ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)

This is due to the fact a bunch tests use gic-version=max which
currectly unconditionally selects GICv3, ignoring the fact this
latter may have been disabled.

This series renames CONFIG_ARM_GIC_TCG into CONFIG_ARM_GICv3_TCG.
Also it selects GICv2 if gic-version=max and CONFIG_ARM_GICV3_TCG is
unset, in TCG mode. With those fixes qtests pass along with
virt machine node.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/gicv3_tcg_v3

Eric Auger (2):
  hw/intc: Rename CONFIG_ARM_GIC_TCG into CONFIG_ARM_GICV3_TCG
  hw/arm/virt: Fix gic-version=max when CONFIG_ARM_GICV3_TCG is unset

 hw/arm/virt.c   | 7 ++-
 hw/intc/Kconfig | 2 +-
 hw/intc/meson.build | 4 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.26.3

[PATCH 1/2] hw/intc: Rename CONFIG_ARM_GIC_TCG into CONFIG_ARM_GICV3_TCG

[Eric Auger]

CONFIG_ARM_GIC_TCG actually guards the compilation of TCG GICv3
specific files. So let's rename it into CONFIG_ARM_GICV3_TCG

Signed-off-by: Eric Auger 
---
 hw/intc/Kconfig | 2 +-
 hw/intc/meson.build | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index ec8d4cec29..a7cf301eab 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -25,7 +25,7 @@ config APIC
 select MSI_NONBROKEN
 select I8259
 
-config ARM_GIC_TCG
+config ARM_GICV3_TCG
 bool
 default y
 depends on ARM_GIC && TCG
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 81ccdb0d78..d6d012fb26 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -6,7 +6,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files(
   'arm_gicv3_common.c',
   'arm_gicv3_its_common.c',
 ))
-softmmu_ss.add(when: 'CONFIG_ARM_GIC_TCG', if_true: files(
+softmmu_ss.add(when: 'CONFIG_ARM_GICV3_TCG', if_true: files(
   'arm_gicv3.c',
   'arm_gicv3_dist.c',
   'arm_gicv3_its.c',
@@ -28,7 +28,7 @@ softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: 
files('xlnx-pmu-iomod-in
 specific_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: 
files('allwinner-a10-pic.c'))
 specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c'))
 specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: 
files('arm_gicv3_cpuif_common.c'))
-specific_ss.add(when: 'CONFIG_ARM_GIC_TCG', if_true: 
files('arm_gicv3_cpuif.c'))
+specific_ss.add(when: 'CONFIG_ARM_GICV3_TCG', if_true: 
files('arm_gicv3_cpuif.c'))
 specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files('arm_gic_kvm.c'))
 specific_ss.add(when: ['CONFIG_ARM_GIC_KVM', 'TARGET_AARCH64'], if_true: 
files('arm_gicv3_kvm.c', 'arm_gicv3_its_kvm.c'))
 specific_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('armv7m_nvic.c'))
-- 
2.26.3

Re: [PATCH 06/13] hw/isa/piix4: Replace some magic IRQ constants

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx_pci to piix4". gt64xxx_pci used magic constants, and probably
didn't want to use piix4-specific constants. Now that the interrupt
handing resides in piix4, its constants can be used.

Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Michael S. Tsirkin
Message-Id:<20220217101924.15347-7-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/isa/piix4.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 5/9] hw/i2c: pmbus: update MAINTAINERS

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

add self to MAINTAINERS for the PMBus subsystem and related sensors,
and set PMBus as maintained.

Signed-off-by: Titus Rwantare 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-6-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f0cc1e448..600bf820da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3138,6 +3138,16 @@ F: include/hw/i2c/smbus_master.h
 F: include/hw/i2c/smbus_slave.h
 F: include/hw/i2c/smbus_eeprom.h
 
+PMBus
+M: Titus Rwantare 
+S: Maintained
+F: hw/i2c/pmbus_device.c
+F: hw/sensor/adm1272.c
+F: hw/sensor/max34451.c
+F: include/hw/i2c/pmbus_device.h
+F: tests/qtest/adm1272-test.c
+F: tests/qtest/max34451-test.c
+
 Firmware schema specifications
 M: Philippe Mathieu-Daudé 
 R: Daniel P. Berrange 
-- 
2.34.1

Re: [PATCH 04/13] hw/isa/piix4: Pass PIIX4State as opaque parameter for piix4_set_irq()

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

Passing PIIX4State rather than just the qemu_irq allows for resolving
the global piix4_dev variable.

Signed-off-by: Bernhard Beschow
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Michael S. Tsirkin
Message-Id:<20220217101924.15347-5-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/isa/piix4.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 4/9] hw/i2c: pmbus: refactor uint handling

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

This change cleans up the inputs to pmbus_receive uint, the length of
received data is contained in PMBusDevice state and doesn't need to be
passed around.

Signed-off-by: Titus Rwantare 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-5-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/pmbus_device.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index ff644c1d4a..8cb9db0f80 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -89,16 +89,16 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
 }
 
 
-static uint64_t pmbus_receive_uint(const uint8_t *buf, uint8_t len)
+static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
 {
 uint64_t ret = 0;
 
 /* Exclude command code from return value */
-buf++;
-len--;
+pmdev->in_buf++;
+pmdev->in_buf_len--;
 
-for (int i = len - 1; i >= 0; i--) {
-ret = ret << 8 | buf[i];
+for (int i = pmdev->in_buf_len - 1; i >= 0; i--) {
+ret = ret << 8 | pmdev->in_buf[i];
 }
 return ret;
 }
@@ -110,7 +110,7 @@ uint8_t pmbus_receive8(PMBusDevice *pmdev)
   "%s: length mismatch. Expected 1 byte, got %d bytes\n",
   __func__, pmdev->in_buf_len - 1);
 }
-return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+return pmbus_receive_uint(pmdev);
 }
 
 uint16_t pmbus_receive16(PMBusDevice *pmdev)
@@ -120,7 +120,7 @@ uint16_t pmbus_receive16(PMBusDevice *pmdev)
   "%s: length mismatch. Expected 2 bytes, got %d bytes\n",
   __func__, pmdev->in_buf_len - 1);
 }
-return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+return pmbus_receive_uint(pmdev);
 }
 
 uint32_t pmbus_receive32(PMBusDevice *pmdev)
@@ -130,7 +130,7 @@ uint32_t pmbus_receive32(PMBusDevice *pmdev)
   "%s: length mismatch. Expected 4 bytes, got %d bytes\n",
   __func__, pmdev->in_buf_len - 1);
 }
-return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+return pmbus_receive_uint(pmdev);
 }
 
 uint64_t pmbus_receive64(PMBusDevice *pmdev)
@@ -140,7 +140,7 @@ uint64_t pmbus_receive64(PMBusDevice *pmdev)
   "%s: length mismatch. Expected 8 bytes, got %d bytes\n",
   __func__, pmdev->in_buf_len - 1);
 }
-return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+return pmbus_receive_uint(pmdev);
 }
 
 static uint8_t pmbus_out_buf_pop(PMBusDevice *pmdev)
-- 
2.34.1

Re: [PATCH 05/13] hw/isa/piix4: Resolve global instance variable

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

Now that piix4_set_irq's opaque parameter references own PIIX4State,
piix4_dev becomes redundant.

Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Michael S. Tsirkin
Message-Id:<20220217101924.15347-6-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/isa/piix4.c| 10 +++---
  include/hw/southbridge/piix.h |  2 --
  2 files changed, 3 insertions(+), 9 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 0/9] I²C / SMBus / PMBus patches for 2022-03-08

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

The following changes since commit 9740b907a5363c06ecf61e08b21966a81eb0dab4:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20220307' 
into staging (2022-03-08 15:26:10 +)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/pmbus-20220308

for you to fetch changes up to 5f14cd7032beab6cac8d7ed1b09efc58baddb48c:

  hw/sensor: add Renesas raa228000 device (2022-03-08 18:46:48 +0100)


I²C / SMBus / PMBus patches

- Add some Renesas models
- Add Titus Rwantare to MAINTAINERS



Shengtan Mao (1):
  hw/i2c: Added linear mode translation for pmbus devices

Titus Rwantare (8):
  hw/i2c: pmbus: add registers
  hw/i2c: pmbus: fix error returns and guard against out of range
accesses
  hw/i2c: pmbus: add PEC unsupported warning
  hw/i2c: pmbus: refactor uint handling
  hw/i2c: pmbus: update MAINTAINERS
  hw/sensor: add Intersil ISL69260 device model
  hw/sensor: add Renesas raa229004 PMBus device
  hw/sensor: add Renesas raa228000 device

 MAINTAINERS  |  13 +
 hw/arm/Kconfig   |   1 +
 hw/i2c/pmbus_device.c| 112 +++-
 hw/sensor/Kconfig|   4 +
 hw/sensor/isl_pmbus_vr.c | 279 ++
 hw/sensor/meson.build|   1 +
 include/hw/i2c/pmbus_device.h|  25 +-
 include/hw/sensor/isl_pmbus_vr.h |  52 
 tests/qtest/isl_pmbus_vr-test.c  | 474 +++
 tests/qtest/meson.build  |   1 +
 10 files changed, 948 insertions(+), 14 deletions(-)
 create mode 100644 hw/sensor/isl_pmbus_vr.c
 create mode 100644 include/hw/sensor/isl_pmbus_vr.h
 create mode 100644 tests/qtest/isl_pmbus_vr-test.c

-- 
2.34.1

Re: [PATCH 03/13] hw/isa/piix4: Resolve redundant i8259[] attribute

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx_pci to piix4" where i8259[] was moved from MaltaState to
PIIX4State to make the code movement more obvious. However, i8259[]
seems redundant to *isa, so remove it.

Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Michael S. Tsirkin
Message-Id:<20220217101924.15347-4-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/isa/piix4.c | 7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 7/9] hw/sensor: add Intersil ISL69260 device model

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

Signed-off-by: Titus Rwantare 
Reviewed-by: Hao Wu 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-8-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS  |   3 +
 hw/arm/Kconfig   |   1 +
 hw/sensor/Kconfig|   4 +
 hw/sensor/isl_pmbus_vr.c | 211 +
 hw/sensor/meson.build|   1 +
 include/hw/sensor/isl_pmbus_vr.h |  50 
 tests/qtest/isl_pmbus_vr-test.c  | 394 +++
 tests/qtest/meson.build  |   1 +
 8 files changed, 665 insertions(+)
 create mode 100644 hw/sensor/isl_pmbus_vr.c
 create mode 100644 include/hw/sensor/isl_pmbus_vr.h
 create mode 100644 tests/qtest/isl_pmbus_vr-test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 600bf820da..8839af7f2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3143,10 +3143,13 @@ M: Titus Rwantare 
 S: Maintained
 F: hw/i2c/pmbus_device.c
 F: hw/sensor/adm1272.c
+F: hw/sensor/isl_pmbus_vr.c
 F: hw/sensor/max34451.c
 F: include/hw/i2c/pmbus_device.h
+F: include/hw/sensor/isl_pmbus_vr.h
 F: tests/qtest/adm1272-test.c
 F: tests/qtest/max34451-test.c
+F: tests/qtest/isl_pmbus_vr-test.c
 
 Firmware schema specifications
 M: Philippe Mathieu-Daudé 
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 6945330030..97f3b38019 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -400,6 +400,7 @@ config NPCM7XX
 select SMBUS
 select AT24C  # EEPROM
 select MAX34451
+select ISL_PMBUS_VR
 select PL310  # cache controller
 select PMBUS
 select SERIAL
diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
index 215944decc..df392e7869 100644
--- a/hw/sensor/Kconfig
+++ b/hw/sensor/Kconfig
@@ -30,3 +30,7 @@ config LSM303DLHC_MAG
 bool
 depends on I2C
 default y if I2C_DEVICES
+
+config ISL_PMBUS_VR
+bool
+depends on PMBUS
diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
new file mode 100644
index 00..f8cc75fc31
--- /dev/null
+++ b/hw/sensor/isl_pmbus_vr.c
@@ -0,0 +1,211 @@
+/*
+ * PMBus device for Renesas Digital Multiphase Voltage Regulators
+ *
+ * Copyright 2021 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sensor/isl_pmbus_vr.h"
+#include "hw/qdev-properties.h"
+#include "qapi/visitor.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+static uint8_t isl_pmbus_vr_read_byte(PMBusDevice *pmdev)
+{
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: reading from unsupported register: 0x%02x\n",
+  __func__, pmdev->code);
+return PMBUS_ERR_BYTE;
+}
+
+static int isl_pmbus_vr_write_data(PMBusDevice *pmdev, const uint8_t *buf,
+   uint8_t len)
+{
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: write to unsupported register: 0x%02x\n",
+  __func__, pmdev->code);
+return PMBUS_ERR_BYTE;
+}
+
+/* TODO: Implement coefficients support in pmbus_device.c for qmp */
+static void isl_pmbus_vr_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+visit_type_uint16(v, name, (uint16_t *)opaque, errp);
+}
+
+static void isl_pmbus_vr_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+uint16_t *internal = opaque;
+uint16_t value;
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+*internal = value;
+pmbus_check_limits(pmdev);
+}
+
+static void isl_pmbus_vr_exit_reset(Object *obj)
+{
+PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+
+pmdev->page = 0;
+pmdev->capability = ISL_CAPABILITY_DEFAULT;
+for (int i = 0; i < pmdev->num_pages; i++) {
+pmdev->pages[i].operation = ISL_OPERATION_DEFAULT;
+pmdev->pages[i].on_off_config = ISL_ON_OFF_CONFIG_DEFAULT;
+pmdev->pages[i].vout_mode = ISL_VOUT_MODE_DEFAULT;
+pmdev->pages[i].vout_command = ISL_VOUT_COMMAND_DEFAULT;
+pmdev->pages[i].vout_max = ISL_VOUT_MAX_DEFAULT;
+pmdev->pages[i].vout_margin_high = ISL_VOUT_MARGIN_HIGH_DEFAULT;
+pmdev->pages[i].vout_margin_low = ISL_VOUT_MARGIN_LOW_DEFAULT;
+pmdev->pages[i].vout_transition_rate = 
ISL_VOUT_TRANSITION_RATE_DEFAULT;
+pmdev->pages[i].vout_ov_fault_limit = ISL_VOUT_OV_FAULT_LIMIT_DEFAULT;
+pmdev->pages[i].ot_fault_limit = ISL_OT_FAULT_LIMIT_DEFAULT;
+pmdev->pages[i].ot_warn_limit = ISL_OT_WARN_LIMIT_DEFAULT;
+pmdev->pages[i].vin_ov_warn_limit = ISL_VIN_OV_WARN_LIMIT_DEFAULT;
+pmdev->pages[i].vin_uv_warn_limit = ISL_VIN_UV_WARN_LIMIT_DEFAULT;
+pmdev->pages[i].iin_oc_fault_limit = ISL_IIN_OC_FAULT_LIMIT_DEFAULT;
+pmdev->pages[i].ton_delay = ISL_TON_DELAY_DEFAULT;
+pmdev->pages[i].ton_rise = ISL_TON_RISE_DEFAULT;
+pmdev->pages[i].toff_fall = 

Re: [PATCH 01/13] hw/mips/gt64xxx_pci: Fix PCI IRQ levels to be preserved during migration

[Richard Henderson]

On 3/7/22 03:43, Philippe Mathieu-Daudé wrote:

From: Bernhard Beschow

Based on commit e735b55a8c11dd455e31ccd4420e6c9485191d0c:

   piix_pci: eliminate PIIX3State::pci_irq_levels

   PIIX3State::pci_irq_levels are redundant which is already tracked by
   PCIBus layer. So eliminate them.

The IRQ levels in the PCIBus layer are already preserved during
migration. By reusing them and rather than having a redundant implementation
the bug is avoided in the first place.

Suggested-by: Peter Maydell
Signed-off-by: Bernhard Beschow
Reviewed-by: Peter Maydell
Message-Id:<20220217101924.15347-2-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
  hw/mips/gt64xxx_pci.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 6/9] hw/i2c: Added linear mode translation for pmbus devices

[Philippe Mathieu-Daudé]

From: Shengtan Mao 

Signed-off-by: Shengtan Mao 
Reviewed-by: Titus Rwantare 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-7-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/pmbus_device.c | 18 ++
 include/hw/i2c/pmbus_device.h | 20 +++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 8cb9db0f80..62885fa6a1 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -28,6 +28,24 @@ uint32_t pmbus_direct_mode2data(PMBusCoefficients c, 
uint16_t value)
 return x;
 }
 
+uint16_t pmbus_data2linear_mode(uint16_t value, int exp)
+{
+/* L = D * 2^(-e) */
+if (exp < 0) {
+return value << (-exp);
+}
+return value >> exp;
+}
+
+uint16_t pmbus_linear_mode2data(uint16_t value, int exp)
+{
+/* D = L * 2^e */
+if (exp < 0) {
+return value >> (-exp);
+}
+return value << exp;
+}
+
 void pmbus_send(PMBusDevice *pmdev, const uint8_t *data, uint16_t len)
 {
 if (pmdev->out_buf_len + len > SMBUS_DATA_MAX_LEN) {
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index bab4526734..0f4d6b3fad 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -448,7 +448,7 @@ typedef struct PMBusCoefficients {
  *
  * Y = (m * x - b) * 10^R
  *
- * @return uint32_t
+ * @return uint16_t
  */
 uint16_t pmbus_data2direct_mode(PMBusCoefficients c, uint32_t value);
 
@@ -461,6 +461,24 @@ uint16_t pmbus_data2direct_mode(PMBusCoefficients c, 
uint32_t value);
  */
 uint32_t pmbus_direct_mode2data(PMBusCoefficients c, uint16_t value);
 
+/**
+ * Convert sensor values to linear mode format
+ *
+ * L = D * 2^(-e)
+ *
+ * @return uint16
+ */
+uint16_t pmbus_data2linear_mode(uint16_t value, int exp);
+
+/**
+ * Convert linear mode formatted data into sensor reading
+ *
+ * D = L * 2^e
+ *
+ * @return uint16
+ */
+uint16_t pmbus_linear_mode2data(uint16_t value, int exp);
+
 /**
  * @brief Send a block of data over PMBus
  * Assumes that the bytes in the block are already ordered correctly,
-- 
2.34.1

[PULL 9/9] hw/sensor: add Renesas raa228000 device

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

Signed-off-by: Titus Rwantare 
Reviewed-by: Hao Wu 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-10-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sensor/isl_pmbus_vr.c | 50 ++
 include/hw/sensor/isl_pmbus_vr.h |  1 +
 tests/qtest/isl_pmbus_vr-test.c  | 72 
 3 files changed, 123 insertions(+)

diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
index 53187d619a..e11e028884 100644
--- a/hw/sensor/isl_pmbus_vr.c
+++ b/hw/sensor/isl_pmbus_vr.c
@@ -89,6 +89,24 @@ static void isl_pmbus_vr_exit_reset(Object *obj)
 }
 }
 
+/* The raa228000 uses different direct mode coefficents from most isl devices 
*/
+static void raa228000_exit_reset(Object *obj)
+{
+PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+
+isl_pmbus_vr_exit_reset(obj);
+
+pmdev->pages[0].read_iout = 0;
+pmdev->pages[0].read_pout = 0;
+pmdev->pages[0].read_vout = 0;
+pmdev->pages[0].read_vin = 0;
+pmdev->pages[0].read_iin = 0;
+pmdev->pages[0].read_pin = 0;
+pmdev->pages[0].read_temperature_1 = 0;
+pmdev->pages[0].read_temperature_2 = 0;
+pmdev->pages[0].read_temperature_3 = 0;
+}
+
 static void isl_pmbus_vr_add_props(Object *obj, uint64_t *flags, uint8_t pages)
 {
 PMBusDevice *pmdev = PMBUS_DEVICE(obj);
@@ -177,6 +195,20 @@ static void raa22xx_init(Object *obj)
 isl_pmbus_vr_add_props(obj, flags, ARRAY_SIZE(flags));
 }
 
+static void raa228000_init(Object *obj)
+{
+PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+uint64_t flags[1];
+
+flags[0] = PB_HAS_VIN | PB_HAS_VOUT | PB_HAS_VOUT_MODE |
+   PB_HAS_VOUT_RATING | PB_HAS_VOUT_MARGIN | PB_HAS_IIN |
+   PB_HAS_IOUT | PB_HAS_PIN | PB_HAS_POUT | PB_HAS_TEMPERATURE |
+   PB_HAS_TEMP2 | PB_HAS_TEMP3 | PB_HAS_STATUS_MFR_SPECIFIC;
+
+pmbus_page_config(pmdev, 0, flags[0]);
+isl_pmbus_vr_add_props(obj, flags, 1);
+}
+
 static void isl_pmbus_vr_class_init(ObjectClass *klass, void *data,
 uint8_t pages)
 {
@@ -195,6 +227,15 @@ static void isl69260_class_init(ObjectClass *klass, void 
*data)
 isl_pmbus_vr_class_init(klass, data, 2);
 }
 
+static void raa228000_class_init(ObjectClass *klass, void *data)
+{
+ResettableClass *rc = RESETTABLE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
+dc->desc = "Renesas 228000 Digital Multiphase Voltage Regulator";
+rc->phases.exit = raa228000_exit_reset;
+isl_pmbus_vr_class_init(klass, data, 1);
+}
+
 static void raa229004_class_init(ObjectClass *klass, void *data)
 {
 ResettableClass *rc = RESETTABLE_CLASS(klass);
@@ -220,9 +261,18 @@ static const TypeInfo raa229004_info = {
 .class_init = raa229004_class_init,
 };
 
+static const TypeInfo raa228000_info = {
+.name = TYPE_RAA228000,
+.parent = TYPE_PMBUS_DEVICE,
+.instance_size = sizeof(ISLState),
+.instance_init = raa228000_init,
+.class_init = raa228000_class_init,
+};
+
 static void isl_pmbus_vr_register_types(void)
 {
 type_register_static(_info);
+type_register_static(_info);
 type_register_static(_info);
 }
 
diff --git a/include/hw/sensor/isl_pmbus_vr.h b/include/hw/sensor/isl_pmbus_vr.h
index 233916f70a..3e47ff7e48 100644
--- a/include/hw/sensor/isl_pmbus_vr.h
+++ b/include/hw/sensor/isl_pmbus_vr.h
@@ -13,6 +13,7 @@
 #include "qom/object.h"
 
 #define TYPE_ISL69260   "isl69260"
+#define TYPE_RAA228000  "raa228000"
 #define TYPE_RAA229004  "raa229004"
 
 struct ISLState {
diff --git a/tests/qtest/isl_pmbus_vr-test.c b/tests/qtest/isl_pmbus_vr-test.c
index dc0ccae2aa..5553ea410a 100644
--- a/tests/qtest/isl_pmbus_vr-test.c
+++ b/tests/qtest/isl_pmbus_vr-test.c
@@ -150,6 +150,70 @@ static void test_defaults(void *obj, void *data, 
QGuestAllocator *alloc)
 g_assert_cmphex(i2c_value, ==, ISL_REVISION_DEFAULT);
 }
 
+static void raa228000_test_defaults(void *obj, void *data,
+QGuestAllocator *alloc)
+{
+uint16_t value, i2c_value;
+QI2CDevice *i2cdev = (QI2CDevice *)obj;
+
+value = qmp_isl_pmbus_vr_get(TEST_ID, "vout[0]");
+g_assert_cmpuint(value, ==, 0);
+
+i2c_value = isl_pmbus_vr_i2c_get16(i2cdev, PMBUS_READ_IOUT);
+g_assert_cmpuint(i2c_value, ==, 0);
+
+value = qmp_isl_pmbus_vr_get(TEST_ID, "pout[0]");
+g_assert_cmpuint(value, ==, 0);
+
+i2c_value = i2c_get8(i2cdev, PMBUS_CAPABILITY);
+g_assert_cmphex(i2c_value, ==, ISL_CAPABILITY_DEFAULT);
+
+i2c_value = i2c_get8(i2cdev, PMBUS_OPERATION);
+g_assert_cmphex(i2c_value, ==, ISL_OPERATION_DEFAULT);
+
+i2c_value = i2c_get8(i2cdev, PMBUS_ON_OFF_CONFIG);
+g_assert_cmphex(i2c_value, ==, ISL_ON_OFF_CONFIG_DEFAULT);
+
+i2c_value = i2c_get8(i2cdev, PMBUS_VOUT_MODE);
+g_assert_cmphex(i2c_value, ==, ISL_VOUT_MODE_DEFAULT);
+
+i2c_value = isl_pmbus_vr_i2c_get16(i2cdev, PMBUS_VOUT_COMMAND);
+

[PULL 3/9] hw/i2c: pmbus: add PEC unsupported warning

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

Signed-off-by: Titus Rwantare 
Acked-by: Corey Minyard 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20220307200605.4001451-4-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/pmbus_device.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index c7ec8e5499..ff644c1d4a 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -307,6 +307,11 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
 
 case PMBUS_CAPABILITY:
 pmbus_send8(pmdev, pmdev->capability);
+if (pmdev->capability & BIT(7)) {
+qemu_log_mask(LOG_UNIMP,
+  "%s: PEC is enabled but not yet supported.\n",
+  __func__);
+}
 break;
 
 case PMBUS_VOUT_MODE: /* R/W byte */
-- 
2.34.1

[PULL 8/9] hw/sensor: add Renesas raa229004 PMBus device

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

The Renesas RAA229004 is a PMBus Multiphase Voltage Regulator

Signed-off-by: Titus Rwantare 
Reviewed-by: Hao Wu 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-9-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sensor/isl_pmbus_vr.c | 18 ++
 include/hw/sensor/isl_pmbus_vr.h |  1 +
 tests/qtest/isl_pmbus_vr-test.c  |  8 
 3 files changed, 27 insertions(+)

diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
index f8cc75fc31..53187d619a 100644
--- a/hw/sensor/isl_pmbus_vr.c
+++ b/hw/sensor/isl_pmbus_vr.c
@@ -195,6 +195,15 @@ static void isl69260_class_init(ObjectClass *klass, void 
*data)
 isl_pmbus_vr_class_init(klass, data, 2);
 }
 
+static void raa229004_class_init(ObjectClass *klass, void *data)
+{
+ResettableClass *rc = RESETTABLE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
+dc->desc = "Renesas 229004 Digital Multiphase Voltage Regulator";
+rc->phases.exit = isl_pmbus_vr_exit_reset;
+isl_pmbus_vr_class_init(klass, data, 2);
+}
+
 static const TypeInfo isl69260_info = {
 .name = TYPE_ISL69260,
 .parent = TYPE_PMBUS_DEVICE,
@@ -203,9 +212,18 @@ static const TypeInfo isl69260_info = {
 .class_init = isl69260_class_init,
 };
 
+static const TypeInfo raa229004_info = {
+.name = TYPE_RAA229004,
+.parent = TYPE_PMBUS_DEVICE,
+.instance_size = sizeof(ISLState),
+.instance_init = raa22xx_init,
+.class_init = raa229004_class_init,
+};
+
 static void isl_pmbus_vr_register_types(void)
 {
 type_register_static(_info);
+type_register_static(_info);
 }
 
 type_init(isl_pmbus_vr_register_types)
diff --git a/include/hw/sensor/isl_pmbus_vr.h b/include/hw/sensor/isl_pmbus_vr.h
index 4e12e95efb..233916f70a 100644
--- a/include/hw/sensor/isl_pmbus_vr.h
+++ b/include/hw/sensor/isl_pmbus_vr.h
@@ -13,6 +13,7 @@
 #include "qom/object.h"
 
 #define TYPE_ISL69260   "isl69260"
+#define TYPE_RAA229004  "raa229004"
 
 struct ISLState {
 PMBusDevice parent;
diff --git a/tests/qtest/isl_pmbus_vr-test.c b/tests/qtest/isl_pmbus_vr-test.c
index f77732ae96..dc0ccae2aa 100644
--- a/tests/qtest/isl_pmbus_vr-test.c
+++ b/tests/qtest/isl_pmbus_vr-test.c
@@ -390,5 +390,13 @@ static void isl_pmbus_vr_register_nodes(void)
 qos_add_test("test_pages_rw", "isl69260", test_pages_rw, NULL);
 qos_add_test("test_ro_regs", "isl69260", test_ro_regs, NULL);
 qos_add_test("test_ov_faults", "isl69260", test_voltage_faults, NULL);
+
+qos_node_create_driver("raa229004", i2c_device_create);
+qos_node_consumes("raa229004", "i2c-bus", );
+
+qos_add_test("test_tx_rx", "raa229004", test_tx_rx, NULL);
+qos_add_test("test_rw_regs", "raa229004", test_rw_regs, NULL);
+qos_add_test("test_pages_rw", "raa229004", test_pages_rw, NULL);
+qos_add_test("test_ov_faults", "raa229004", test_voltage_faults, NULL);
 }
 libqos_init(isl_pmbus_vr_register_nodes);
-- 
2.34.1

[PULL 2/9] hw/i2c: pmbus: fix error returns and guard against out of range accesses

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

Signed-off-by: Titus Rwantare 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-3-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/pmbus_device.c | 47 ---
 include/hw/i2c/pmbus_device.h |  2 ++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 07a45c99f9..c7ec8e5499 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -149,7 +149,7 @@ static uint8_t pmbus_out_buf_pop(PMBusDevice *pmdev)
 qemu_log_mask(LOG_GUEST_ERROR,
   "%s: tried to read from empty buffer",
   __func__);
-return 0xFF;
+return PMBUS_ERR_BYTE;
 }
 uint8_t data = pmdev->out_buf[pmdev->out_buf_len - 1];
 pmdev->out_buf_len--;
@@ -243,18 +243,47 @@ void pmbus_check_limits(PMBusDevice *pmdev)
 }
 }
 
+/* assert the status_cml error upon receipt of malformed command */
+static void pmbus_cml_error(PMBusDevice *pmdev)
+{
+for (int i = 0; i < pmdev->num_pages; i++) {
+pmdev->pages[i].status_word |= PMBUS_STATUS_CML;
+pmdev->pages[i].status_cml |= PB_CML_FAULT_INVALID_CMD;
+}
+}
+
 static uint8_t pmbus_receive_byte(SMBusDevice *smd)
 {
 PMBusDevice *pmdev = PMBUS_DEVICE(smd);
 PMBusDeviceClass *pmdc = PMBUS_DEVICE_GET_CLASS(pmdev);
-uint8_t ret = 0xFF;
-uint8_t index = pmdev->page;
+uint8_t ret = PMBUS_ERR_BYTE;
+uint8_t index;
 
 if (pmdev->out_buf_len != 0) {
 ret = pmbus_out_buf_pop(pmdev);
 return ret;
 }
 
+/*
+ * Reading from all pages will return the value from page 0,
+ * this is unspecified behaviour in general.
+ */
+if (pmdev->page == PB_ALL_PAGES) {
+index = 0;
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: tried to read from all pages\n",
+  __func__);
+pmbus_cml_error(pmdev);
+} else if (pmdev->page > pmdev->num_pages - 1) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: page %d is out of range\n",
+  __func__, pmdev->page);
+pmbus_cml_error(pmdev);
+return PMBUS_ERR_BYTE;
+} else {
+index = pmdev->page;
+}
+
 switch (pmdev->code) {
 case PMBUS_PAGE:
 pmbus_send8(pmdev, pmdev->page);
@@ -1019,7 +1048,7 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t 
*buf, uint8_t len)
 
 if (len == 0) {
 qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
-return -1;
+return PMBUS_ERR_BYTE;
 }
 
 if (!pmdev->pages) { /* allocate memory for pages on first use */
@@ -1038,6 +1067,7 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t 
*buf, uint8_t len)
 pmdev->page = pmbus_receive8(pmdev);
 return 0;
 }
+
 /* loop through all the pages when 0xFF is received */
 if (pmdev->page == PB_ALL_PAGES) {
 for (int i = 0; i < pmdev->num_pages; i++) {
@@ -1048,6 +1078,15 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t 
*buf, uint8_t len)
 return 0;
 }
 
+if (pmdev->page > pmdev->num_pages - 1) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: page %u is out of range\n",
+__func__, pmdev->page);
+pmdev->page = 0; /* undefined behaviour - reset to page 0 */
+pmbus_cml_error(pmdev);
+return PMBUS_ERR_BYTE;
+}
+
 index = pmdev->page;
 
 switch (pmdev->code) {
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 72c0483149..bab4526734 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -228,6 +228,8 @@ enum pmbus_registers {
 #define PB_MAX_PAGES0x1F
 #define PB_ALL_PAGES0xFF
 
+#define PMBUS_ERR_BYTE  0xFF
+
 #define TYPE_PMBUS_DEVICE "pmbus-device"
 OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass,
 PMBUS_DEVICE)
-- 
2.34.1

[PULL 1/9] hw/i2c: pmbus: add registers

[Philippe Mathieu-Daudé]

From: Titus Rwantare 

   - add the VOUT_MIN and STATUS_MFR registers

Signed-off-by: Titus Rwantare 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Corey Minyard 
Message-Id: <20220307200605.4001451-2-tit...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/pmbus_device.c | 24 
 include/hw/i2c/pmbus_device.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 24f8f522d9..07a45c99f9 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -368,6 +368,14 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
 }
 break;
 
+case PMBUS_VOUT_MIN:/* R/W word */
+if (pmdev->pages[index].page_flags & PB_HAS_VOUT_RATING) {
+pmbus_send16(pmdev, pmdev->pages[index].vout_min);
+} else {
+goto passthough;
+}
+break;
+
 /* TODO: implement coefficients support */
 
 case PMBUS_POUT_MAX:  /* R/W word */
@@ -708,6 +716,10 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
 pmbus_send8(pmdev, pmdev->pages[index].status_other);
 break;
 
+case PMBUS_STATUS_MFR_SPECIFIC:   /* R/W byte */
+pmbus_send8(pmdev, pmdev->pages[index].status_mfr_specific);
+break;
+
 case PMBUS_READ_EIN:  /* Read-Only block 5 bytes */
 if (pmdev->pages[index].page_flags & PB_HAS_EIN) {
 pmbus_send(pmdev, pmdev->pages[index].read_ein, 5);
@@ -1149,6 +1161,14 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t 
*buf, uint8_t len)
 }
 break;
 
+case PMBUS_VOUT_MIN:  /* R/W word */
+if (pmdev->pages[index].page_flags & PB_HAS_VOUT_RATING) {
+pmdev->pages[index].vout_min = pmbus_receive16(pmdev);
+} else {
+goto passthrough;
+}
+break;
+
 case PMBUS_POUT_MAX:  /* R/W word */
 if (pmdev->pages[index].page_flags & PB_HAS_VOUT) {
 pmdev->pages[index].pout_max = pmbus_receive16(pmdev);
@@ -1482,6 +1502,10 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t 
*buf, uint8_t len)
 pmdev->pages[index].status_other = pmbus_receive8(pmdev);
 break;
 
+case PMBUS_STATUS_MFR_SPECIFIC:/* R/W byte */
+pmdev->pages[index].status_mfr_specific = pmbus_receive8(pmdev);
+break;
+
 case PMBUS_PAGE_PLUS_READ:/* Block Read-only */
 case PMBUS_CAPABILITY:/* Read-Only byte */
 case PMBUS_COEFFICIENTS:  /* Read-only block 5 bytes */
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 62bd38c83f..72c0483149 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -43,6 +43,7 @@ enum pmbus_registers {
 PMBUS_VOUT_DROOP= 0x28, /* R/W word */
 PMBUS_VOUT_SCALE_LOOP   = 0x29, /* R/W word */
 PMBUS_VOUT_SCALE_MONITOR= 0x2A, /* R/W word */
+PMBUS_VOUT_MIN  = 0x2B, /* R/W word */
 PMBUS_COEFFICIENTS  = 0x30, /* Read-only block 5 bytes */
 PMBUS_POUT_MAX  = 0x31, /* R/W word */
 PMBUS_MAX_DUTY  = 0x32, /* R/W word */
@@ -255,6 +256,7 @@ OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass,
 #define PB_HAS_TEMP3   BIT_ULL(42)
 #define PB_HAS_TEMP_RATING BIT_ULL(43)
 #define PB_HAS_MFR_INFOBIT_ULL(50)
+#define PB_HAS_STATUS_MFR_SPECIFIC BIT_ULL(51)
 
 struct PMBusDeviceClass {
 SMBusDeviceClass parent_class;
@@ -295,6 +297,7 @@ typedef struct PMBusPage {
 uint16_t vout_droop;   /* R/W word */
 uint16_t vout_scale_loop;  /* R/W word */
 uint16_t vout_scale_monitor;   /* R/W word */
+uint16_t vout_min; /* R/W word */
 uint8_t coefficients[5];   /* Read-only block 5 bytes */
 uint16_t pout_max; /* R/W word */
 uint16_t max_duty; /* R/W word */
-- 
2.34.1

Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type

[Patrick Williams]

On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote:
> 
> >> There are two flash devices on the FMC. I can fix it inline since
> >> it is the only change I would request.
> > 
> > Yes, there are.  I think all of the Facebook systems have dual FMC, for
> > redundancy in hardware, but we can get by in QEMU with just a single one.
> 
> yes, the kernel will complain though and I don't know how robust
> the spi-nor based driver is. I think you sent a patch for a related
> issue.
> 
> The newer spi-mem driver should be fine.

Oh yes.  I already forgot that I'm running with that patch since Joel added it
to our backport 5.15 branch.  One of the reasons I wrote that patch was to make
QEMU not kpanic. :(

>   
> > I'll see however you fix it up and see I can update all the other systems as
> > well.  
> 
> ok. may be for 7.1 then.
> 
> > We have an internal patch to expand the CS on FMC to 2 but we haven't
> > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
> > changing for adding images.  
> 
> Yes. That's the problem. I am afraid some CI systems will break with
> these change in a newer QEMU. The command line options will need to
> adapt.

My recollection is that the Romulus CI uses a branch of QEMU that at this point
is rather old anyhow.  We should be able to fix up the CI scripts at the same
time we upgrade.

Are you or Andrew J maintaining that branch?

> > My recollection is that the Romulus CI on OpenBMC relies on the PNOR 
> > being the 2nd argument.
> 
> That's the initial assumption made years ago. First mtd device is FMC,
> second is the PNOR. It is reaching its limits.
> 
> I am looking at improving the command line argument to support:
> 
>-drive file=,format=raw,id=drive0 -device 
> mx66l1g45g,bus=ssi.0,drive=drive0
> 
> which we would clearly define the topology. Adding a cs=[0-5] or and
> addr=[0-5] is the next step.

Seems fine to me.

-- 
Patrick Williams


signature.asc
Description: PGP signature

Re: [PATCH v3 0/9] This patch series contains updates to PMBus in QEMU along with some PMBus device models for Renesas regulators. I have also added myself to MAINTAINERS as this code is in use daily,

[Philippe Mathieu-Daudé]

On 8/3/22 14:53, Corey Minyard wrote:

On Mon, Mar 07, 2022 at 01:00:02AM +0100, Philippe Mathieu-Daudé wrote:

On 5/3/22 00:42, Titus Rwantare wrote:

On Fri, 4 Mar 2022 at 13:43, Corey Minyard  wrote:


On Tue, Mar 01, 2022 at 05:50:44PM -0800, Titus Rwantare wrote:

v2:
- split PMBus commit with updates into individual fixes
- renamed isl_pmbus[.ch] adding _vr for voltage regulators

v3:
- split uint refactor commit and removed commit renaming files
- rename rolled into preceding commits
- update commit description for uint refactoring change


This all looks good to me:

Acked-by: Corey Minyard 

Do you have a plan for getting this in to qemu?  Like through the ARM
tree?  I could take it into an I2C tree, but there's really not much
activity or work there.

-corey


In general PMBus is more specific to i2c than ARM, but I'm not sure of
the QEMU implications.


Titus, could you address my comments?

Corey, if you are busy, I can take care of this series.


It's not a "too busy" sort of thing, the i2c tree doesn't get much
traffic.  I can take it, but it's not much different than just pulling
it directly.

So it's probably best if you take it.


OK. I'll send a pullreq shortly.

Regards,

Phil.

Re: [PATCH] hw/misc/npcm7xx_clk: Don't leak string in npcm7xx_clk_sel_init()

[Richard Henderson]

On 3/8/22 07:03, Peter Maydell wrote:

In npcm7xx_clk_sel_init() we allocate a string with g_strdup_printf().
Use g_autofree so we free it rather than leaking it.

(Detected with the clang leak sanitizer.)

Signed-off-by: Peter Maydell
---
  hw/misc/npcm7xx_clk.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [qemu-web PATCH] Announce Google Summer of Code 2022

[Thomas Huth]

On 08/03/2022 17.15, Stefan Hajnoczi wrote:

QEMU has been accepted into Google Summer of Code 2022. Let people know
so they can apply!

Signed-off-by: Stefan Hajnoczi 
---
  _posts/2022-03-07-gsoc-2022.md | 35 ++
  1 file changed, 35 insertions(+)
  create mode 100644 _posts/2022-03-07-gsoc-2022.md


Thanks, pushed:

https://www.qemu.org/2022/03/07/gsoc-2022/

 Thomas

Re: [PATCH v3] target/arm: Fix sve2 ldnt1 and stnt1

[Richard Henderson]

On 3/8/22 01:56, Peter Maydell wrote:

-# SVE2 64-bit gather non-temporal load
-#   (scalar plus unpacked 32-bit unscaled offsets)
+# SVE2 64-bit gather non-temporal load (scalar plus 64-bit unscaled offsets)
  LDNT1_zprz  1100010 msz:2 00 rm:5 1 u:1 0 pg:3 rn:5 rd:5 \
-_gather_load xs=0 esz=3 scale=0 ff=0
+_gather_load xs=2 esz=3 scale=0 ff=0


We correct the xs= value here...

...

+switch (a->esz) {
+case MO_32:
+fn = gather_load_fn32[mte][be][0][0][a->u][a->msz];
+break;
+case MO_64:
+fn = gather_load_fn64[mte][be][0][2][a->u][a->msz];
+break;
+}


...but then instead of using it we hard code use of 0 or 2 here,
which makes the change above a bit moot.


Yeah, sorry for that.  I was in the middle of reducing the argument set collected by those 
decode patterns, then decided there was little advantage, and left something in the middle.




r~

Re: [PATCH] hw/misc/npcm7xx_clk: Don't leak string in npcm7xx_clk_sel_init()

[Philippe Mathieu-Daudé]

On 8/3/22 18:03, Peter Maydell wrote:

In npcm7xx_clk_sel_init() we allocate a string with g_strdup_printf().
Use g_autofree so we free it rather than leaking it.

(Detected with the clang leak sanitizer.)

Signed-off-by: Peter Maydell 
---
  hw/misc/npcm7xx_clk.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] hw/dma/xlnx_csu_dma: Set TYPE_XLNX_CSU_DMA class_size

[Philippe Mathieu-Daudé]

On 8/3/22 16:02, Peter Maydell wrote:

In commit 00f05c02f9e7342f we gave the TYPE_XLNX_CSU_DMA object its
own class struct, but forgot to update the TypeInfo::class_size
accordingly.  This meant that not enough memory was allocated for the
class struct, and the initialization of xcdc->read in the class init
function wrote off the end of the memory. Add the missing line.

Found by running 'check-qtest-aarch64' with a clang
address-sanitizer build, which complains:

==2542634==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x6100ab00 at pc 0x559a20aebc29 bp 0x7fff97df74d0 sp 0x7fff97df74c8
WRITE of size 8 at 0x6100ab00 thread T0
 #0 0x559a20aebc28 in xlnx_csu_dma_class_init 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../hw/dma/xlnx_csu_dma.c:722:16
 #1 0x559a21bf297c in type_initialize 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:365:9
 #2 0x559a21bf3442 in object_class_foreach_tramp 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1070:5
 #3 0x7f09bcb641b7 in g_hash_table_foreach 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x401b7)
 #4 0x559a21bf3c27 in object_class_foreach 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1092:5
 #5 0x559a21bf3c27 in object_class_get_list 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1149:5
 #6 0x559a2081a2fd in select_machine 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:1661:24
 #7 0x559a2081a2fd in qemu_create_machine 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:2146:35
 #8 0x559a2081a2fd in qemu_init 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/vl.c:3706:5
 #9 0x559a20720ed5 in main 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../softmmu/main.c:49:5
 #10 0x7f09baec00b2 in __libc_start_main 
/build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
 #11 0x559a2067673d in _start 
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/qemu-system-aarch64+0xf4b73d)

0x6100ab00 is located 0 bytes to the right of 192-byte region 
[0x6100aa40,0x6100ab00)
allocated by thread T0 here:
 #0 0x559a206eeff2 in calloc 
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/qemu-system-aarch64+0xfc3ff2)
 #1 0x7f09bcb7bef0 in g_malloc0 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57ef0)
 #2 0x559a21bf3442 in object_class_foreach_tramp 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/san/../../qom/object.c:1070:5

Fixes: 00f05c02f9e7342f ("hw/dma/xlnx_csu_dma: Support starting a read transfer 
through a class method")
Signed-off-by: Peter Maydell 
---
  hw/dma/xlnx_csu_dma.c | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PULL 00/18] target-arm queue

[Peter Maydell]

On Mon, 7 Mar 2022 at 16:47, Peter Maydell  wrote:
>
> Last lot of target-arm stuff: cleanups, bug fixes; nothing major here.
>
> -- PMM
>
> The following changes since commit 9d662a6b22a0838a85c5432385f35db2488a33a5:
>
>   Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220305' into 
> staging (2022-03-05 18:03:15 +)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20220307
>
> for you to fetch changes up to 0942820408dc788560f6968e9b5f011803b846c2:
>
>   hw/arm/virt: Disable LPA2 for -machine virt-6.2 (2022-03-07 14:32:21 +)
>
> 
> target-arm queue:
>  * cleanups of qemu_oom_check() and qemu_memalign()
>  * target/arm/translate-neon: UNDEF if VLD1/VST1 stride bits are non-zero
>  * target/arm/translate-neon: Simplify align field check for VLD3
>  * GICv3 ITS: add more trace events
>  * GICv3 ITS: implement 8-byte accesses properly
>  * GICv3: fix minor issues with some trace/log messages
>  * ui/cocoa: Use the standard about panel
>  * target/arm: Provide cpu property for controling FEAT_LPA2
>  * hw/arm/virt: Disable LPA2 for -machine virt-6.2


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

Re: [libvirt] [PATCH RESEND v2 0/4] re-introduce

[Michael S. Tsirkin]

On Tue, Mar 08, 2022 at 10:23:20PM +0530, Ani Sinha wrote:
> On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
> > >
> > >
> > > On Tue, 8 Mar 2022, Laine Stump wrote:
> > >
> > > > Aha! the domain of qemu-devel@nongnu.org was incorrect in the original 
> > > > send
> > > > (it was "nognu.org"), so none of this thread was making it to that list.
> > >
> > >
> > > Not to give any excuses but this happened because on Qemu side I never
> > > have to type this manually. My git config is set up so that
> > > the cc in send-email is filled up automatically using
> > > scripts/get_maintainer.pl. On libvirt side also the domain and mailing
> > > list is easy to remember. Its only when I have to manually type stuff that
> > > shit happens :-)
> >
> > Donnu about alpine, but with mutt you can easily set up
> > and alias and then it expands for you.
> 
> I use alpine to only reply/review patches. I use git send-email to
> actually send the patch. There I am not sure the best way to avoid
> manually typing in the mailing list address.

send-email supports aliases too.

-- 
MST

Re: [PATCH v3 3/5] iotests: Remove explicit checks for qemu_img() == 0

[John Snow]

On Tue, Mar 8, 2022, 10:16 AM Eric Blake  wrote:

> On Mon, Mar 07, 2022 at 08:57:26PM -0500, John Snow wrote:
> > qemu_img() returning zero ought to be the rule, not the
> > exception. Remove all explicit checks against the condition in
> > preparation for making non-zero returns an Exception.
> >
> > Signed-off-by: John Snow 
> > ---
>
> Reviewed-by: Eric Blake 
>
> As this is a testsuite improvement rather than a new feature, I think
> it's fine for the series to go in during soft freeze.
>

Yup, I agree. I'd like to move this in sooner rather than later to guard
against rot, and to have the better failure messages during testing season.

I have followup patches that finish the audit of qemu-img calls. It's less
clear if those should also go in during soft freeze, but I suppose I can
send them and we can see how confident we feel about it.

(Also note, I am giving the same treatment to qemu-io in another branch,
too. That branch has revealed actual logical errors in our testing in
several places. That series isn't 100% ready yet, but it might also qualify
for freeze because it fixes real test defects.)


> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>
>

[PATCH] hw/misc/npcm7xx_clk: Don't leak string in npcm7xx_clk_sel_init()

[Peter Maydell]

In npcm7xx_clk_sel_init() we allocate a string with g_strdup_printf().
Use g_autofree so we free it rather than leaking it.

(Detected with the clang leak sanitizer.)

Signed-off-by: Peter Maydell 
---
 hw/misc/npcm7xx_clk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index 0b61070c52f..bc2b879feb5 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -612,8 +612,8 @@ static void npcm7xx_clk_sel_init(Object *obj)
 NPCM7xxClockSELState *sel = NPCM7XX_CLOCK_SEL(obj);
 
 for (i = 0; i < NPCM7XX_CLK_SEL_MAX_INPUT; ++i) {
-sel->clock_in[i] = qdev_init_clock_in(DEVICE(sel),
-g_strdup_printf("clock-in[%d]", i),
+g_autofree char *s = g_strdup_printf("clock-in[%d]", i);
+sel->clock_in[i] = qdev_init_clock_in(DEVICE(sel), s,
 npcm7xx_clk_update_sel_cb, sel, ClockUpdate);
 }
 sel->clock_out = qdev_init_clock_out(DEVICE(sel), "clock-out");
-- 
2.25.1