Re: [PATCH 0/2] target/arm: Fix unaligned mte checks

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20210402053728.265173-1-richard.hender...@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210402053728.265173-1-richard.hender...@linaro.org
Subject: [PATCH 0/2] target/arm: Fix unaligned mte checks

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   00084ba..415fa2f  master -> master
 - [tag update]  
patchew/20210401125808.89241-1-fanwentao.0...@bytedance.com -> 
patchew/20210401125808.89241-1-fanwentao.0...@bytedance.com
 * [new tag] 
patchew/20210402053728.265173-1-richard.hender...@linaro.org -> 
patchew/20210402053728.265173-1-richard.hender...@linaro.org
Switched to a new branch 'test'
7730030 target/arm: Fix unaligned mte checks
ef1ac97 target/arm: Check PAGE_WRITE_ORG for MTE writeability

=== OUTPUT BEGIN ===
1/2 Checking commit ef1ac978a003 (target/arm: Check PAGE_WRITE_ORG for MTE 
writeability)
WARNING: line over 80 characters
#28: FILE: target/arm/mte_helper.c:86:
+if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG : 
PAGE_READ))) {

total: 0 errors, 1 warnings, 8 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/2 Checking commit 77300300fa91 (target/arm: Fix unaligned mte checks)
Use of uninitialized value $acpi_testexpected in string eq at 
./scripts/checkpatch.pl line 1529.
ERROR: spaces required around that '*' (ctx:WxV)
#407: FILE: target/arm/sve_helper.c:4438:
+   sve_ldst1_tlb_fn *tlb_fn)
 ^

ERROR: spaces required around that '*' (ctx:WxV)
#517: FILE: target/arm/sve_helper.c:5063:
+   sve_ldst1_tlb_fn *tlb_fn)
 ^

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#787: 
new file mode 100644

total: 2 errors, 1 warnings, 719 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210402053728.265173-1-richard.hender...@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH 2/2] target/arm: Fix unaligned mte checks

[Richard Henderson]

We were incorrectly assuming that only the first byte of an MTE access
is checked against the tags.  But per the ARM, unaligned accesses are
pre-decomposed into single-byte accesses.  So by the time we reach the
actual MTE check in the ARM pseudocode, all accesses are aligned.

Therefore, drop mte_check1, since we cannot know a priori that an
access is aligned.  Rename mte_checkN to mte_check, which now handles
all accesses.  Rename mte_probe1 to mte_probe, and use a common helper.

Drop the computation of the faulting nth element, since all accesses
can be considered to devolve to bytes, and simply compute the faulting
address.

Buglink: https://bugs.launchpad.net/bugs/1921948
Signed-off-by: Richard Henderson 
---
 target/arm/helper-a64.h   |   3 +-
 target/arm/internals.h|  13 +--
 target/arm/translate-a64.h|   2 +-
 target/arm/mte_helper.c   | 169 --
 target/arm/sve_helper.c   |  96 ++---
 target/arm/translate-a64.c|  52 -
 target/arm/translate-sve.c|   9 +-
 tests/tcg/aarch64/mte-5.c |  44 
 tests/tcg/aarch64/Makefile.target |   2 +-
 9 files changed, 178 insertions(+), 212 deletions(-)
 create mode 100644 tests/tcg/aarch64/mte-5.c

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index c139fa81f9..7b706571bb 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -104,8 +104,7 @@ DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, 
i64)
 DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
 
-DEF_HELPER_FLAGS_3(mte_check1, TCG_CALL_NO_WG, i64, env, i32, i64)
-DEF_HELPER_FLAGS_3(mte_checkN, TCG_CALL_NO_WG, i64, env, i32, i64)
+DEF_HELPER_FLAGS_3(mte_check, TCG_CALL_NO_WG, i64, env, i32, i64)
 DEF_HELPER_FLAGS_3(mte_check_zva, TCG_CALL_NO_WG, i64, env, i32, i64)
 DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_4(addsubg, TCG_CALL_NO_RWG_SE, i64, env, i64, s32, i32)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index f11bd32696..817d3aa51b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1137,19 +1137,16 @@ FIELD(PREDDESC, DATA, 8, 24)
  */
 #define SVE_MTEDESC_SHIFT 5
 
-/* Bits within a descriptor passed to the helper_mte_check* functions. */
+/* Bits within a descriptor passed to the helper_mte_check function. */
 FIELD(MTEDESC, MIDX,  0, 4)
 FIELD(MTEDESC, TBI,   4, 2)
 FIELD(MTEDESC, TCMA,  6, 2)
 FIELD(MTEDESC, WRITE, 8, 1)
-FIELD(MTEDESC, ESIZE, 9, 5)
-FIELD(MTEDESC, TSIZE, 14, 10)  /* mte_checkN only */
+FIELD(MTEDESC, SIZEM1, 12, 10)  /* size - 1 */
 
-bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr);
-uint64_t mte_check1(CPUARMState *env, uint32_t desc,
-uint64_t ptr, uintptr_t ra);
-uint64_t mte_checkN(CPUARMState *env, uint32_t desc,
-uint64_t ptr, uintptr_t ra);
+bool mte_probe(CPUARMState *env, uint32_t desc, uint64_t ptr);
+uint64_t mte_check(CPUARMState *env, uint32_t desc,
+   uint64_t ptr, uintptr_t ra);
 
 static inline int allocation_tag_from_addr(uint64_t ptr)
 {
diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 3668b671dd..6c4bbf9096 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -44,7 +44,7 @@ TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr);
 TCGv_i64 gen_mte_check1(DisasContext *s, TCGv_i64 addr, bool is_write,
 bool tag_checked, int log2_size);
 TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write,
-bool tag_checked, int count, int log2_esize);
+bool tag_checked, int total_size);
 
 /* We should have at some point before trying to access an FP register
  * done the necessary access check, so assert that
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 8be17e1b70..62bea7ad4a 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -121,7 +121,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int 
ptr_mmu_idx,
  * exception for inaccessible pages, and resolves the virtual address
  * into the softmmu tlb.
  *
- * When RA == 0, this is for mte_probe1.  The page is expected to be
+ * When RA == 0, this is for mte_probe.  The page is expected to be
  * valid.  Indicate to probe_access_flags no-fault, then assert that
  * we received a valid page.
  */
@@ -617,80 +617,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
 }
 }
 
-/*
- * Perform an MTE checked access for a single logical or atomic access.
- */
-static bool mte_probe1_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
-   uintptr_t ra, int bit55)
-{
-int mem_tag, mmu_idx, ptr_tag, size;
-MMUAccessType type;
-uint8_t *mem;
-
-ptr_tag = allocation_tag_from_addr(ptr);
-
-if 

[PATCH 1/2] target/arm: Check PAGE_WRITE_ORG for MTE writeability

[Richard Henderson]

We can remove PAGE_WRITE when (internally) marking a page
read-only because it contains translated code.

This can be triggered by tests/tcg/aarch64/bti-2, after
having serviced SIGILL trampolines on the stack.

Signed-off-by: Richard Henderson 
---
 target/arm/mte_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 0bbb9ec346..8be17e1b70 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -83,7 +83,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int 
ptr_mmu_idx,
 uint8_t *tags;
 uintptr_t index;
 
-if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE : PAGE_READ))) {
+if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG : 
PAGE_READ))) {
 /* SIGSEGV */
 arm_cpu_tlb_fill(env_cpu(env), ptr, ptr_size, ptr_access,
  ptr_mmu_idx, false, ra);
-- 
2.25.1

[PATCH 0/2] target/arm: Fix unaligned mte checks

[Richard Henderson]

I was incorrectly assuming that only the first byte of an MTE access
is checked against the tags.  But per the ARM, unaligned accesses are
pre-decomposed into single-byte accesses.  So by the time we reach the
actual MTE check in the ARM pseudocode, all accesses are aligned.

There's probably more that could be simplified here, but I did try to
keep the patch small-ish.


r~


Richare Henderson (2):
  target/arm: Check PAGE_WRITE_ORG for MTE writeability
  target/arm: Fix unaligned mte checks

 target/arm/helper-a64.h   |   3 +-
 target/arm/internals.h|  13 +--
 target/arm/translate-a64.h|   2 +-
 target/arm/mte_helper.c   | 171 --
 target/arm/sve_helper.c   |  96 ++---
 target/arm/translate-a64.c|  52 -
 target/arm/translate-sve.c|   9 +-
 tests/tcg/aarch64/mte-5.c |  44 
 tests/tcg/aarch64/Makefile.target |   2 +-
 9 files changed, 179 insertions(+), 213 deletions(-)
 create mode 100644 tests/tcg/aarch64/mte-5.c

-- 
2.25.1

Re: [1/1] tcg/mips: Fix SoftTLB comparison on mips backend

[Richard Henderson]

On 4/1/21 3:04 AM, Kele Huang wrote:

The addrl used to compare with SoftTLB entry should be sign-extended
in common case, and it will cause constant failing in SoftTLB
comparisons for the addrl whose address is over 0x8000 on the
emulation of 32-bit guest on 64-bit host.

This is an important performance bug fix. Spec2000 gzip rate increase
from ~45 to ~140 on Loongson 3A4000 (MIPS compatible platform).

Signed-off-by: Kele Huang
---
  tcg/mips/tcg-target.c.inc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Queued, thanks.


r~

Re: [PATCH v5 0/7] eBPF RSS support for virtio-net

[Yuri Benditovich]

More correctly, https://bugzilla.redhat.com/show_bug.cgi?id=1865786

On Fri, Apr 2, 2021 at 8:21 AM Yuri Benditovich
 wrote:
>
> Hi Jason,
>
> Yes, the work to support RSS in the Linux virtio-net driver is in progress.
> https://bugzilla.redhat.com/show_bug.cgi?id=1912082
>
> On Fri, Apr 2, 2021 at 5:57 AM Jason Wang  wrote:
> >
> >
> > 在 2021/3/25 下午11:35, Andrew Melnychenko 写道:
> > > This set of patches introduces the usage of eBPF for packet steering
> > > and RSS hash calculation:
> > > * RSS(Receive Side Scaling) is used to distribute network packets to
> > > guest virtqueues by calculating packet hash
> > > * Additionally adding support for the usage of RSS with vhost
> > >
> > > The eBPF works on kernels 5.8+
> > > On earlier kerneld it fails to load and the RSS feature is reported
> > > only without vhost and implemented in 'in-qemu' software.
> > >
> > > Implementation notes:
> > > Linux TAP TUNSETSTEERINGEBPF ioctl was used to set the eBPF program.
> > > Added libbpf dependency and eBPF support.
> > > The eBPF program is part of the qemu and presented as an array
> > > of BPF ELF file data. The eBPF array file initially generated by bpftool.
> > > The compilation of eBPF is not part of QEMU build and can be done
> > > using provided Makefile.ebpf.
> > > Added changes to virtio-net and vhost, primary eBPF RSS is used.
> > > 'in-qemu' RSS used in the case of hash population and as a fallback 
> > > option.
> > > For vhost, the hash population feature is not reported to the guest.
> > >
> > > Please also see the documentation in PATCH 6/7.
> > >
> > > Known issues:
> > > * hash population not supported by eBPF RSS: 'in-qemu' RSS used
> > > as a fallback, also, hash population feature is not reported to guests
> > > with vhost.
> > > * IPv6 extensions still in progress.
> >
> >
> > Hi Andrew:
> >
> > The patch looks good at a glance. I tend to queue it for 6.1.
> >
> > One issue is that, there's no easy way for testing it without a windows
> > guest.
> >
> > Do you have plan to extend Linux driver to support RSS (e.g via ethtool?).
> >
> > Thanks
> >
> >
> > >
> > > Changes since v1:
> > > * using libbpf instead of direct 'bpf' system call.
> > > * added libbpf dependency to the configure/meson scripts.
> > > * changed python script for eBPF .h file generation.
> > > * changed eBPF program - reading L3 proto from ethernet frame.
> > > * added TUNSETSTEERINGEBPF define for TUN.
> > > * changed the maintainer's info.
> > > * added license headers.
> > > * refactored code.
> > >
> > > Changes since v2:
> > > * using bpftool for eBPF skeleton generation.
> > > * ebpf_rss is refactored to use skeleton generated by bpftool.
> > > * added/adjasted license in comment sections and in eBPF file.
> > > * rss.bpf.c and Makefile.ebpf moved to the tool/ebpf folder.
> > > * virtio-net eBPF rss refactored. Now eBPF initialized during realize().
> > >
> > > Changes since v3:
> > > * rebased to last master.
> > > * fixed issue with failed build without libbpf.
> > > * fixed ebpf loading without rss option.
> > > * refactored labels in ebpf_rss.c
> > >
> > > Changes since v4:
> > > * refactored configure/meson script.
> > > * added checks for load_bytes in ebpf.
> > > * documentation added to the index.
> > > * refactored Makefile and rss.bpf.c.
> > > * rebased to last master.
> > >
> > > Andrew (7):
> > >net/tap: Added TUNSETSTEERINGEBPF code.
> > >net: Added SetSteeringEBPF method for NetClientState.
> > >ebpf: Added eBPF RSS program.
> > >ebpf: Added eBPF RSS loader.
> > >virtio-net: Added eBPF RSS to virtio-net.
> > >docs: Added eBPF documentation.
> > >MAINTAINERS: Added eBPF maintainers information.
> > >
> > >   MAINTAINERS|   8 +
> > >   configure  |   8 +-
> > >   docs/devel/ebpf_rss.rst| 125 
> > >   docs/devel/index.rst   |   1 +
> > >   ebpf/ebpf_rss-stub.c   |  40 +++
> > >   ebpf/ebpf_rss.c| 165 ++
> > >   ebpf/ebpf_rss.h|  44 +++
> > >   ebpf/meson.build   |   1 +
> > >   ebpf/rss.bpf.skeleton.h| 423 +
> > >   ebpf/trace-events  |   4 +
> > >   ebpf/trace.h   |   2 +
> > >   hw/net/vhost_net.c |   3 +
> > >   hw/net/virtio-net.c| 115 ++-
> > >   include/hw/virtio/virtio-net.h |   4 +
> > >   include/net/net.h  |   2 +
> > >   meson.build|   9 +
> > >   meson_options.txt  |   2 +
> > >   net/tap-bsd.c  |   5 +
> > >   net/tap-linux.c|  13 +
> > >   net/tap-linux.h|   1 +
> > >   net/tap-solaris.c  |   5 +
> > >   net/tap-stub.c |   5 +
> > >   net/tap.c  |   9 +
> > >   net/tap_int.h  |   1 +
> > >   net/vhost-vdpa.c   |   2 +
> > >   tools/ebpf/Makefile.ebpf   |  22 ++
> > >   tools/ebpf/rss.bpf.c   

Re: [PATCH v5 0/7] eBPF RSS support for virtio-net

[Yuri Benditovich]

Hi Jason,

Yes, the work to support RSS in the Linux virtio-net driver is in progress.
https://bugzilla.redhat.com/show_bug.cgi?id=1912082

On Fri, Apr 2, 2021 at 5:57 AM Jason Wang  wrote:
>
>
> 在 2021/3/25 下午11:35, Andrew Melnychenko 写道:
> > This set of patches introduces the usage of eBPF for packet steering
> > and RSS hash calculation:
> > * RSS(Receive Side Scaling) is used to distribute network packets to
> > guest virtqueues by calculating packet hash
> > * Additionally adding support for the usage of RSS with vhost
> >
> > The eBPF works on kernels 5.8+
> > On earlier kerneld it fails to load and the RSS feature is reported
> > only without vhost and implemented in 'in-qemu' software.
> >
> > Implementation notes:
> > Linux TAP TUNSETSTEERINGEBPF ioctl was used to set the eBPF program.
> > Added libbpf dependency and eBPF support.
> > The eBPF program is part of the qemu and presented as an array
> > of BPF ELF file data. The eBPF array file initially generated by bpftool.
> > The compilation of eBPF is not part of QEMU build and can be done
> > using provided Makefile.ebpf.
> > Added changes to virtio-net and vhost, primary eBPF RSS is used.
> > 'in-qemu' RSS used in the case of hash population and as a fallback option.
> > For vhost, the hash population feature is not reported to the guest.
> >
> > Please also see the documentation in PATCH 6/7.
> >
> > Known issues:
> > * hash population not supported by eBPF RSS: 'in-qemu' RSS used
> > as a fallback, also, hash population feature is not reported to guests
> > with vhost.
> > * IPv6 extensions still in progress.
>
>
> Hi Andrew:
>
> The patch looks good at a glance. I tend to queue it for 6.1.
>
> One issue is that, there's no easy way for testing it without a windows
> guest.
>
> Do you have plan to extend Linux driver to support RSS (e.g via ethtool?).
>
> Thanks
>
>
> >
> > Changes since v1:
> > * using libbpf instead of direct 'bpf' system call.
> > * added libbpf dependency to the configure/meson scripts.
> > * changed python script for eBPF .h file generation.
> > * changed eBPF program - reading L3 proto from ethernet frame.
> > * added TUNSETSTEERINGEBPF define for TUN.
> > * changed the maintainer's info.
> > * added license headers.
> > * refactored code.
> >
> > Changes since v2:
> > * using bpftool for eBPF skeleton generation.
> > * ebpf_rss is refactored to use skeleton generated by bpftool.
> > * added/adjasted license in comment sections and in eBPF file.
> > * rss.bpf.c and Makefile.ebpf moved to the tool/ebpf folder.
> > * virtio-net eBPF rss refactored. Now eBPF initialized during realize().
> >
> > Changes since v3:
> > * rebased to last master.
> > * fixed issue with failed build without libbpf.
> > * fixed ebpf loading without rss option.
> > * refactored labels in ebpf_rss.c
> >
> > Changes since v4:
> > * refactored configure/meson script.
> > * added checks for load_bytes in ebpf.
> > * documentation added to the index.
> > * refactored Makefile and rss.bpf.c.
> > * rebased to last master.
> >
> > Andrew (7):
> >net/tap: Added TUNSETSTEERINGEBPF code.
> >net: Added SetSteeringEBPF method for NetClientState.
> >ebpf: Added eBPF RSS program.
> >ebpf: Added eBPF RSS loader.
> >virtio-net: Added eBPF RSS to virtio-net.
> >docs: Added eBPF documentation.
> >MAINTAINERS: Added eBPF maintainers information.
> >
> >   MAINTAINERS|   8 +
> >   configure  |   8 +-
> >   docs/devel/ebpf_rss.rst| 125 
> >   docs/devel/index.rst   |   1 +
> >   ebpf/ebpf_rss-stub.c   |  40 +++
> >   ebpf/ebpf_rss.c| 165 ++
> >   ebpf/ebpf_rss.h|  44 +++
> >   ebpf/meson.build   |   1 +
> >   ebpf/rss.bpf.skeleton.h| 423 +
> >   ebpf/trace-events  |   4 +
> >   ebpf/trace.h   |   2 +
> >   hw/net/vhost_net.c |   3 +
> >   hw/net/virtio-net.c| 115 ++-
> >   include/hw/virtio/virtio-net.h |   4 +
> >   include/net/net.h  |   2 +
> >   meson.build|   9 +
> >   meson_options.txt  |   2 +
> >   net/tap-bsd.c  |   5 +
> >   net/tap-linux.c|  13 +
> >   net/tap-linux.h|   1 +
> >   net/tap-solaris.c  |   5 +
> >   net/tap-stub.c |   5 +
> >   net/tap.c  |   9 +
> >   net/tap_int.h  |   1 +
> >   net/vhost-vdpa.c   |   2 +
> >   tools/ebpf/Makefile.ebpf   |  22 ++
> >   tools/ebpf/rss.bpf.c   | 552 +
> >   27 files changed, 1567 insertions(+), 4 deletions(-)
> >   create mode 100644 docs/devel/ebpf_rss.rst
> >   create mode 100644 ebpf/ebpf_rss-stub.c
> >   create mode 100644 ebpf/ebpf_rss.c
> >   create mode 100644 ebpf/ebpf_rss.h
> >   create mode 100644 ebpf/meson.build
> >   create mode 100644 

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Jason Wang]

在 2021/3/26 下午5:18, Jason Wang 写道:

?

I assume that  both src and dest started with vhost=on.

As driver B supports both packed and split, you can switch from driver
A to driver B after migration
and driver B will work with split. Exactly as it does today.

The key question is what is more important - vhost or features that
vhost does not support?
current code says: vhost is more important always
v1 patch says: features are more important always.
v2 patch says: vhost is more important at init time, features are more
important at migration time.
Because we are able to drop vhost but we can't drop features when we
have a running driver.
Do you agree?



I think what came from cli is the most important. So if I understand 
correclty:


- vhost=on means "turn on vhost when possible" it implies that 
fallback is allowed (we had already had fallback codes)
- vhostforce=on means "turn on vhost unconditonally" it implies that 
we can't do fallback


So my understanding is that:

- "vhost=on, packed=on", we can fallback to userspace but must keep 
packed virtqueue works
- "vhost=on,vhostforce=on,packed=on", we can't fallback and must keep 
both vhost and packed virtqueue work, if we can't we need to fail


Thanks



Daniel and Michael, am I right here?

We need some inputs to move forward to fix the migration compatibility 
issue.


Thanks

[Question] qemu-img convert block alignment

[Zhenyu Ye]

Hi all,

commit 8dcd3c9b91 ("qemu-img: align result of is_allocated_sectors")
introduces block alignment when doing qemu-img convert. However, the
alignment is:

s.alignment = MAX(pow2floor(s.min_sparse),
  DIV_ROUND_UP(out_bs->bl.request_alignment,
   BDRV_SECTOR_SIZE));

(where the default s.min_sparse is 8)
When the target device's bl.request_alignment is smaller than 4K, this
will cause additional write-zero overhead and makes the size of target
file larger.

Is this as expected?  Should we change the MAX() to MIN()?


Thanks,
zhenyu

Re: [PATCH] hw/virtio: remove redundant code

[Jason Wang]

在 2021/4/1 下午8:58, Wentao Fan 写道:

From: Fan Wentao 

Signed-off-by: Wentao Fan 



Acked-by: Jason Wang 



---
  hw/virtio/virtio.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 07f4e60b30..aa5c283102 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -173,8 +173,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
int n)
  }
  new = g_new0(VRingMemoryRegionCaches, 1);
  size = virtio_queue_get_desc_size(vdev, n);
-packed = virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED) ?
-   true : false;
+packed = virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED);
  len = address_space_cache_init(>desc, vdev->dma_as,
 addr, size, packed);
  if (len < size) {

Re: [PATCH 0/6] Add debug interface to kick/call on purpose

[Jason Wang]

在 2021/3/30 下午3:29, Dongli Zhang 写道:


On 3/28/21 8:56 PM, Jason Wang wrote:

在 2021/3/27 上午5:16, Dongli Zhang 写道:

Hi Jason,

On 3/26/21 12:24 AM, Jason Wang wrote:

在 2021/3/26 下午1:44, Dongli Zhang 写道:

The virtio device/driver (e.g., vhost-scsi or vhost-net) may hang due to
the loss of doorbell kick, e.g.,

https://urldefense.com/v3/__https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01711.html__;!!GqivPVa7Brio!KS3pAU2cKjz4wgI4QSlE-YsJPhPG71nkE5_tGhaOf7mi_xvNxbvKkfn03rk5BNDLSEU$


... or due to the loss of IRQ, e.g., as fixed by linux kernel commit
fe200ae48ef5 ("genirq: Mark polled irqs and defer the real handler").

This patch introduces a new debug interface 'DeviceEvent' to DeviceClass
to help narrow down if the issue is due to loss of irq/kick. So far the new
interface handles only two events: 'call' and 'kick'. Any device (e.g.,
virtio/vhost or VFIO) may implement the interface (e.g., via eventfd, MSI-X
or legacy IRQ).

The 'call' is to inject irq on purpose by admin for a specific device (e.g.,
vhost-scsi) from QEMU/host to VM, while the 'kick' is to kick the doorbell
on purpose by admin at QEMU/host side for a specific device.


This device can be used as a workaround if call/kick is lost due to
virtualization software (e.g., kernel or QEMU) issue.

We may also implement the interface for VFIO PCI, e.g., to write to
VFIOPCIDevice->msi_vectors[i].interrupt will be able to inject IRQ to VM
on purpose. This is considered future work once the virtio part is done.


Below is from live crash analysis. Initially, the queue=2 has count=15 for
'kick' eventfd_ctx. Suppose there is data in vring avail while there is no
used available. We suspect this is because vhost-scsi was not notified by
VM. In order to narrow down and analyze the issue, we use live crash to
dump the current counter of eventfd for queue=2.

crash> eventfd_ctx 8f67f6bbe700
struct eventfd_ctx {
     kref = {
   refcount = {
     refs = {
   counter = 4
     }
   }
     },
     wqh = {
   lock = {
     {
   rlock = {
     raw_lock = {
   val = {
     counter = 0
   }
     }
   }
     }
   },
   head = {
     next = 0x8f841dc08e18,
     prev = 0x8f841dc08e18
   }
     },
     count = 15, ---> eventfd is 15 !!!
     flags = 526336
}

Now we kick the doorbell for vhost-scsi queue=2 on purpose for diagnostic
with this interface.

{ "execute": "x-debug-device-event",
     "arguments": { "dev": "/machine/peripheral/vscsi0",
    "event": "kick", "queue": 2 } }

The counter is increased to 16. Suppose the hang issue is resolved, it
indicates something bad is in software that the 'kick' is lost.

What do you mean by "software" here? And it looks to me you're testing whether
event_notifier_set() is called by virtio_queue_notify() here. If so, I'm not
sure how much value could we gain from a dedicated debug interface like this
consider there're a lot of exisinting general purpose debugging method like
tracing or gdb. I'd say the path from virtio_queue_notify() to
event_notifier_set() is only a very small fraction of the process of virtqueue
kick which is unlikey to be buggy. Consider usually the ioeventfd will be
offloaded to KVM, it's more a chance that something is wrong in setuping
ioeventfd instead of here. Irq is even more complicated.

Thank you very much!

I am not testing whether event_notifier_set() is called by 
virtio_queue_notify().

The 'software' indicates the data processing and event notification mechanism
involved with virtio/vhost PV driver frontend. E.g., while VM is waiting for an
extra IRQ, vhost side did not trigger IRQ, suppose vring_need_event()
erroneously returns false due to corrupted ring buffer status.


So there could be several factors that may block the notification:

1) eventfd bug (ioeventfd vs irqfd)
2) wrong virtqueue state (either driver or device)
3) missing barriers (either driver or device)
4) Qemu bug (irqchip and routing)
...

This is not only about whether notification is blocked.

It can also be used to help narrow down and understand if there is any
suspicious issue in blk-mq/scsi/netdev/napi code. The PV drivers are not only
drivers following virtio spec. It is closely related to many of other kernel
components.

Suppose IO was recovered after we inject an IRQ to vhost-scsi on purpose, we
will be able to analyze what may happen along the IO completion path starting
from when /where the IRQ is injected ... perhaps the root cause is not with
virtio but blk-mq/scsi (this is just an example).


In addition, this idea should help for vfio-pci. Suppose the developer for a
specific device driver suspects IO/networking hangs because of loss if IRQ, we
will be able to verify if that assumption is correct by injecting an IRQ on 
purpose.

Therefore, this is not only about virtio PV driver (frontend/backend), but also
used to help analyze the issue 

Re: [PATCH v5 0/7] eBPF RSS support for virtio-net

[Jason Wang]

在 2021/3/25 下午11:35, Andrew Melnychenko 写道:

This set of patches introduces the usage of eBPF for packet steering
and RSS hash calculation:
* RSS(Receive Side Scaling) is used to distribute network packets to
guest virtqueues by calculating packet hash
* Additionally adding support for the usage of RSS with vhost

The eBPF works on kernels 5.8+
On earlier kerneld it fails to load and the RSS feature is reported
only without vhost and implemented in 'in-qemu' software.

Implementation notes:
Linux TAP TUNSETSTEERINGEBPF ioctl was used to set the eBPF program.
Added libbpf dependency and eBPF support.
The eBPF program is part of the qemu and presented as an array
of BPF ELF file data. The eBPF array file initially generated by bpftool.
The compilation of eBPF is not part of QEMU build and can be done
using provided Makefile.ebpf.
Added changes to virtio-net and vhost, primary eBPF RSS is used.
'in-qemu' RSS used in the case of hash population and as a fallback option.
For vhost, the hash population feature is not reported to the guest.

Please also see the documentation in PATCH 6/7.

Known issues:
* hash population not supported by eBPF RSS: 'in-qemu' RSS used
as a fallback, also, hash population feature is not reported to guests
with vhost.
* IPv6 extensions still in progress.



Hi Andrew:

The patch looks good at a glance. I tend to queue it for 6.1.

One issue is that, there's no easy way for testing it without a windows 
guest.


Do you have plan to extend Linux driver to support RSS (e.g via ethtool?).

Thanks




Changes since v1:
* using libbpf instead of direct 'bpf' system call.
* added libbpf dependency to the configure/meson scripts.
* changed python script for eBPF .h file generation.
* changed eBPF program - reading L3 proto from ethernet frame.
* added TUNSETSTEERINGEBPF define for TUN.
* changed the maintainer's info.
* added license headers.
* refactored code.

Changes since v2:
* using bpftool for eBPF skeleton generation.
* ebpf_rss is refactored to use skeleton generated by bpftool.
* added/adjasted license in comment sections and in eBPF file.
* rss.bpf.c and Makefile.ebpf moved to the tool/ebpf folder.
* virtio-net eBPF rss refactored. Now eBPF initialized during realize().

Changes since v3:
* rebased to last master.
* fixed issue with failed build without libbpf.
* fixed ebpf loading without rss option.
* refactored labels in ebpf_rss.c

Changes since v4:
* refactored configure/meson script.
* added checks for load_bytes in ebpf.
* documentation added to the index.
* refactored Makefile and rss.bpf.c.
* rebased to last master.

Andrew (7):
   net/tap: Added TUNSETSTEERINGEBPF code.
   net: Added SetSteeringEBPF method for NetClientState.
   ebpf: Added eBPF RSS program.
   ebpf: Added eBPF RSS loader.
   virtio-net: Added eBPF RSS to virtio-net.
   docs: Added eBPF documentation.
   MAINTAINERS: Added eBPF maintainers information.

  MAINTAINERS|   8 +
  configure  |   8 +-
  docs/devel/ebpf_rss.rst| 125 
  docs/devel/index.rst   |   1 +
  ebpf/ebpf_rss-stub.c   |  40 +++
  ebpf/ebpf_rss.c| 165 ++
  ebpf/ebpf_rss.h|  44 +++
  ebpf/meson.build   |   1 +
  ebpf/rss.bpf.skeleton.h| 423 +
  ebpf/trace-events  |   4 +
  ebpf/trace.h   |   2 +
  hw/net/vhost_net.c |   3 +
  hw/net/virtio-net.c| 115 ++-
  include/hw/virtio/virtio-net.h |   4 +
  include/net/net.h  |   2 +
  meson.build|   9 +
  meson_options.txt  |   2 +
  net/tap-bsd.c  |   5 +
  net/tap-linux.c|  13 +
  net/tap-linux.h|   1 +
  net/tap-solaris.c  |   5 +
  net/tap-stub.c |   5 +
  net/tap.c  |   9 +
  net/tap_int.h  |   1 +
  net/vhost-vdpa.c   |   2 +
  tools/ebpf/Makefile.ebpf   |  22 ++
  tools/ebpf/rss.bpf.c   | 552 +
  27 files changed, 1567 insertions(+), 4 deletions(-)
  create mode 100644 docs/devel/ebpf_rss.rst
  create mode 100644 ebpf/ebpf_rss-stub.c
  create mode 100644 ebpf/ebpf_rss.c
  create mode 100644 ebpf/ebpf_rss.h
  create mode 100644 ebpf/meson.build
  create mode 100644 ebpf/rss.bpf.skeleton.h
  create mode 100644 ebpf/trace-events
  create mode 100644 ebpf/trace.h
  create mode 100755 tools/ebpf/Makefile.ebpf
  create mode 100644 tools/ebpf/rss.bpf.c

Re: [PATCH V2 4/6] tools/vhost-user-i2c: Add backend driver

[Jie Deng]

On 2021/4/1 20:12, Viresh Kumar wrote:

+
+/* vhost-user-i2c definitions */
+
+#define MAX_I2C_VDEV(1 << 7)
+#define MAX_I2C_ADAPTER 16


Generally speaking, 16 is big enough for most cases. But comparing with 
static configuration,
I think it is better if we can check how many adapters in the host when 
doing initialization and

use that number as "MAX_I2C_ADAPTER".



+
+static VI2cAdapter *vi2c_create_adapter(int32_t bus, uint16_t client_addr[],
+int32_t n_client)
+{
+VI2cAdapter *adapter;
+char path[20];
+uint64_t funcs;
+int32_t fd, i;
+
+if (bus < 0) {
+return NULL;
+}
+
+adapter = g_malloc0(sizeof(*adapter));
+if (!adapter) {
+g_printerr("failed to alloc adapter");
+return NULL;
+}
+
+snprintf(path, sizeof(path), "/dev/i2c-%d", bus);
+path[sizeof(path) - 1] = '\0';
+
+fd = open(path, O_RDWR);
+if (fd < 0) {
+g_printerr("virtio_i2c: failed to open %s\n", path);
+goto fail;
+}
+
+adapter->fd = fd;
+adapter->bus = bus;
+
+if (ioctl(fd, I2C_FUNCS, ) < 0) {
+g_printerr("virtio_i2c: failed to get functionality %s: %d\n", path,
+   errno);
+goto close_fd;
+}
+
+if (funcs & I2C_FUNC_I2C) {
+adapter->smbus = false;
+} else if (funcs & I2C_FUNC_SMBUS_WORD_DATA) {



Only I2C_FUNC_SMBUS_WORD_DATA is checked here. But in addition to it, 
the smbus_xfer
seems support I2C_FUNC_SMBUS_BYTE, I2C_FUNC_SMBUS_BYTE_DATA. So if an 
adapter only

support the latter two, it will never go to smbus_xfer.




+adapter->smbus = true;
+} else {
+g_printerr("virtio_i2c: invalid functionality %lx\n", funcs);
+goto close_fd;
+}
+

[1/1] tcg/mips: Fix SoftTLB comparison on mips backend

[Kele Huang]

The addrl used to compare with SoftTLB entry should be sign-extended
in common case, and it will cause constant failing in SoftTLB
comparisons for the addrl whose address is over 0x8000 on the
emulation of 32-bit guest on 64-bit host.

This is an important performance bug fix. Spec2000 gzip rate increase
from ~45 to ~140 on Loongson 3A4000 (MIPS compatible platform).

Signed-off-by: Kele Huang 
Signed-off-by: Fuxin Zhang 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tcg/mips/tcg-target.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
index 8738a3a581..8b16726242 100644
--- a/tcg/mips/tcg-target.c.inc
+++ b/tcg/mips/tcg-target.c.inc
@@ -1201,13 +1201,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg 
base, TCGReg addrl,
load the tlb addend for the fast path.  */
 tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off);
 }
-tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
 
 /* Zero extend a 32-bit guest address for a 64-bit host. */
 if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
 tcg_out_ext32u(s, base, addrl);
 addrl = base;
 }
+tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
 
 label_ptr[0] = s->code_ptr;
 tcg_out_opc_br(s, OPC_BNE, TCG_TMP1, TCG_TMP0);
-- 
2.30.0

Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()

[Alexander Bulekov]

By the way, this is also triggerable from the am53c974 device..
Should I provide a reproducer patch for this one?

On 210204 2350, Philippe Mathieu-Daudé wrote:
> Per the "SCSI Commands Reference Manual" (Rev. J) chapter 5.3
> "Mode parameters" and table 359 "Mode page codes and subpage
> codes", the last page code is 0x3f. When using it as array index,
> the array must have 0x40 elements. Replace the magic 0x3f value
> by its definition and increase the size of the mode_sense_valid[]
> to avoid an out of bound access (reproducer available in [Buglink]):
> 
>   hw/scsi/scsi-disk.c:1104:10: runtime error: index 63 out of bounds for type 
> 'const int [63]'
>   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
> hw/scsi/scsi-disk.c:1104:10 in
>   =
>   ==1813911==ERROR: AddressSanitizer: global-buffer-overflow
>   READ of size 4 at 0x5624b84aff1c thread T0
>   #0 0x5624b63004b3 in mode_sense_page hw/scsi/scsi-disk.c:1104:10
>   #1 0x5624b630f798 in scsi_disk_check_mode_select 
> hw/scsi/scsi-disk.c:1447:11
>   #2 0x5624b630efb8 in mode_select_pages hw/scsi/scsi-disk.c:1515:17
>   #3 0x5624b630988e in scsi_disk_emulate_mode_select 
> hw/scsi/scsi-disk.c:1570:13
>   #4 0x5624b62f08e7 in scsi_disk_emulate_write_data 
> hw/scsi/scsi-disk.c:1861:9
>   #5 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
>   #6 0x5624b62b2d4c in scsi_req_data hw/scsi/scsi-bus.c:1427:5
>   #7 0x5624b62f05f6 in scsi_disk_emulate_write_data 
> hw/scsi/scsi-disk.c:1853:9
>   #8 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
>   #9 0x5624b63e47ed in megasas_enqueue_req hw/scsi/megasas.c:1660:9
>   #10 0x5624b63b9cfa in megasas_handle_scsi hw/scsi/megasas.c:1735:5
>   #11 0x5624b63acf91 in megasas_handle_frame hw/scsi/megasas.c:1974:24
>   #12 0x5624b63aa200 in megasas_mmio_write hw/scsi/megasas.c:2131:9
>   #13 0x5624b63ebed3 in megasas_port_write hw/scsi/megasas.c:2182:5
>   #14 0x5624b6f43568 in memory_region_write_accessor 
> softmmu/memory.c:491:5
> 
> Cc: qemu-sta...@nongnu.org
> Reported-by: OSS-Fuzz
> Reported-by: Alexander Bulekov 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914638
> Fixes: a8f4bbe2900 ("scsi-disk: store valid mode pages in a table")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2: Mention reproducer link
> ---
>  hw/scsi/scsi-disk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState 
> *s, uint8_t *outbuf)
>  static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
>  {
> -static const int mode_sense_valid[0x3f] = {
> +static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
>  [MODE_PAGE_HD_GEOMETRY]= (1 << TYPE_DISK),
>  [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
>  [MODE_PAGE_CACHING]= (1 << TYPE_DISK) | (1 << 
> TYPE_ROM),
> -- 
> 2.26.2
> 

Re: [BUG FIX][PATCH v3 0/3] vhost-user-blk: fix bug on device disconnection during initialization

[Valentin Sinitsyn]

On 01.04.2021 14:21, Denis Plotnikov wrote:

This is a series fixing a bug in host-user-blk.

More specifically, it's not just a bug but crasher.

Valentine


Is there any chance for it to be considered for the next rc?

Thanks!

Denis

On 29.03.2021 16:44, Denis Plotnikov wrote:


ping!

On 25.03.2021 18:12, Denis Plotnikov wrote:

v3:
   * 0003: a new patch added fixing the problem on vm shutdown
 I stumbled on this bug after v2 sending.
   * 0001: gramma fixing (Raphael)
   * 0002: commit message fixing (Raphael)

v2:
   * split the initial patch into two (Raphael)
   * rename init to realized (Raphael)
   * remove unrelated comment (Raphael)

When the vhost-user-blk device lose the connection to the daemon during
the initialization phase it kills qemu because of the assert in the code.
The series fixes the bug.

0001 is preparation for the fix
0002 fixes the bug, patch description has the full motivation for the series
0003 (added in v3) fix bug on vm shutdown

Denis Plotnikov (3):
   vhost-user-blk: use different event handlers on initialization
   vhost-user-blk: perform immediate cleanup if disconnect on
 initialization
   vhost-user-blk: add immediate cleanup on shutdown

  hw/block/vhost-user-blk.c | 79 ---
  1 file changed, 48 insertions(+), 31 deletions(-)

Re: [PULL v2 0/9] For 6.0 patches

[Peter Maydell]

On Thu, 1 Apr 2021 at 12:55,  wrote:
>
> From: Marc-André Lureau 
>
> The following changes since commit 6ee55e1d10c25c2f6bf5ce2084ad2327e17affa5:
>
>   Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.0-20210331' 
> into staging (2021-03-31 13:14:18 +0100)
>
> are available in the Git repository at:
>
>   g...@gitlab.com:marcandre.lureau/qemu.git tags/for-6.0-pull-request
>
> for you to fetch changes up to d3a0bb7706520928f8493fabaee76532b5b1adb4:
>
>   tests: Add tests for yank with the chardev-change case (2021-04-01 15:27:44 
> +0400)
>
> 
> For 6.0 misc patches under my radar.
>
> V2:
>  - "tests: Add tests for yank with the chardev-change case" updated
>  - drop the readthedoc theme patch


Applied, thanks.

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

-- PMM

Re: [RFC PATCH] block: always update auto_backing_file to full path

[Joe Jin]

Hi Max,

Thanks very much for your replies.

On 4/1/21 2:57 AM, Max Reitz wrote:
> On 01.04.21 06:22, Joe Jin wrote:
>> Some time after created snapshot, auto_backing_file only has filename,
>> this confused overridden check, update it to full path if it is not.
>>
>> Signed-off-by: Joe Jin 
>> ---
>>   block.c | 13 +
>>   1 file changed, 13 insertions(+)
>
> Do you have a test for this?
My issue is my guest image is on NFS, I could created snapshot from ovirt but I 
could not delete it, tried to commit it by virsh and it complained qemu 
internal error:
# virsh blockcommit snap-test sda --active --verbose --pivot
error: internal error: qemu block name 'json:{"backing": {"driver": "qcow2", 
"file": {"driver": "file", "filename": 
"/rhev/data-center/mnt/nfs_server:_nfsexport/fee77d23-1291-4bdb-9157-12ff6cd5ee96/images/66bb625e-7458-478d-a238-b012f87062b8/919a4cda-bf11-4bb7-a2e3-d9e4515ed646"}},
 "driver": "qcow2", "file": {"driver": "file", "filename": 
"/rhev/data-center/mnt/nfs_server:_nfsexport/fee77d23-1291-4bdb-9157-12ff6cd5ee96/images/66bb625e-7458-478d-a238-b012f87062b8/24d28fcd-67e3-49d2-8860-2110f3a5e796"}}'
 doesn't match expected 
'/rhev/data-center/mnt/nfs_server:_nfsexport/fee77d23-1291-4bdb-9157-12ff6cd5ee96/images/66bb625e-7458-478d-a238-b012f87062b8/24d28fcd-67e3-49d2-8860-2110f3a5e796'
Tracked qemu block and I found when issue happened, bdrv_backing_overridden() 
was tried to compare absolute path(bs->backing->bs->filename) with relative 
path(bs->auto_backing_file) but they are same filename, then it triggered 
generated json filename.
Regarding auto_backing_file, it updated by qcow2_do_open(), and read the 
backing file name from image:
    /* read the backing file name */
  
    if (header.backing_file_offset != 0) {  
  
    len = header.backing_file_size; 
  
    if (len > MIN(1023, s->cluster_size - header.backing_file_offset) ||
  
    len >= sizeof(bs->backing_file)) {  
  
    error_setg(errp, "Backing file name too long"); 
  
    ret = -EINVAL;  
  
    goto fail;  
  
    }   
  
    ret = bdrv_pread(bs->file, header.backing_file_offset,  
  
 bs->auto_backing_file, len);   
  
    if (ret < 0) {  
  
    error_setg_errno(errp, -ret, "Could not read backing file name");   
  
    goto fail;  
  
    }   
  
    bs->auto_backing_file[len] = '\0';  
  
    pstrcpy(bs->backing_file, sizeof(bs->backing_file), 
  
    bs->auto_backing_file); 
  
    s->image_backing_file = g_strdup(bs->auto_backing_file);
  
    }   
  

Updated auto_backing_file to absolute path, I could successfully delete 
snapshot from ovirt, or execute blockcommit by virsh.

>
> The thing is, I’m not sure about this solution, and I think having a test 
> would help me understand better.
> bs->auto_backing_file is meant to track what filename a BDS would have if we 
> opened bs->backing_file.  To this end, we generally set it to whatever 
> bs->backing_file is and then refresh it when we actually do open a BDS from 
> it.
>
> So it seems strange to blindly modify it somewhere that doesn’t have to do 
> with any of these things.
>
> Now, when opening a backing file from bs->backing_file, we first do make it 
> an absolute filename via bdrv_get_full_backing_filename().  So it kind of 
> seems prudent to replicate that elsewhere, but I’m not sure where exactly.  I 
> would think the best place would be whenever auto_backing_file is set to be 
> equal to backing_file (which is generally in the image format drivers, when 
> they read the backing file string from the image metadata), but that might 
> break the strcmp() in bdrv_open_backing_file()...
>
> I don’t think bdrv_refresh_filename() is the right place, though, because I’m 
> afraid that this might modify filenames we’ve already retrieved from the 
> actual backing BDS, or something.
>
> For example, with this patch applied, iotest 024 fails.
Yes after applied my patch, I can reproduced the failure, it caused by 

Re: [PATCH v3] hw/smbios: support for type 41 (onboard devices extended information)

[Igor Mammedov]

On Thu, 01 Apr 2021 23:07:06 +0200
Vincent Bernat  wrote:

>  ❦  1 avril 2021 22:58 +02, Igor Mammedov:
> 
> >> This can be invoked with:
> >> 
> >> $QEMU -netdev user,id=internet
> >>   -device 
> >> virtio-net-pci,mac=50:54:00:00:00:42,netdev=internet,id=internet-dev \
> >>   -smbios type=41,designation='Onboard 
> >> LAN',instance=1,kind=ethernet,pcidev=internet-dev  
> >
> > an ACPI alternative was merged recently (current master).
> > assigning 'designation=' wasn't implemented there, but important part
> > of giving users control over PCI devices 'eno' index is implemented.
> >
> > When I looked into the issue, smbios way was a bit over-kill for the task
> > and didn't really work if hotplug were used.
> >
> > See, for example how to use new feature:
> >  https://www.mail-archive.com/qemu-devel@nongnu.org/msg794164.html  
> 
> It seems simpler this way. I don't think my patch is needed then.

SMBIOS ways is fine for static configs where no hot-plug is involved.
Also potentially SMBIOS way may be used by arm/virt board,
since acpi-index shares a lot with ACPI PCI hotplug infrastructure
and we haven't ported that to arm/virt impl. yet.

It also won't work for Q35 at the moment, but Julia is working
on adding support for ACPI PCI hotplug to it, and once it arrives
acpi-index will become available there.

Perhaps we should also add support for ACPI PCI hotplug to virt/arm,
along with Q35.

General question about parsing an rbd filename

[Connor Kuehl]

Hi,

block/rbd.c hints that:


 * Configuration values containing :, @, or = can be escaped with a
 * leading "\".


Right now, much of the parsing code will allow anyone to escape 
_anything_ so long as it's preceded by '\'.


Is this the intended behavior? Or should the parser be updated to allow 
escaping only certain sequences.


Just curious.

Thanks,

Connor

Re: [PATCH v3] hw/smbios: support for type 41 (onboard devices extended information)

[Vincent Bernat]

 ❦  1 avril 2021 22:58 +02, Igor Mammedov:

>> This can be invoked with:
>> 
>> $QEMU -netdev user,id=internet
>>   -device 
>> virtio-net-pci,mac=50:54:00:00:00:42,netdev=internet,id=internet-dev \
>>   -smbios type=41,designation='Onboard 
>> LAN',instance=1,kind=ethernet,pcidev=internet-dev
>
> an ACPI alternative was merged recently (current master).
> assigning 'designation=' wasn't implemented there, but important part
> of giving users control over PCI devices 'eno' index is implemented.
>
> When I looked into the issue, smbios way was a bit over-kill for the task
> and didn't really work if hotplug were used.
>
> See, for example how to use new feature:
>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg794164.html

It seems simpler this way. I don't think my patch is needed then.
-- 
Let the data structure the program.
- The Elements of Programming Style (Kernighan & Plauger)

[PATCH v2 2/2] block/rbd: Add an escape-aware strchr helper

[Connor Kuehl]

Sometimes the parser needs to further split a token it has collected
from the token input stream. Right now, it does a cursory check to see
if the relevant characters appear in the token to determine if it should
break it down further.

However, qemu_rbd_next_tok() will escape characters as it removes tokens
from the token stream and plain strchr() won't. This can make the
initial strchr() check slightly misleading since it implies
qemu_rbd_next_tok() will find the token and split on it, except the
reality is that qemu_rbd_next_tok() will pass over it if it is escaped.

Use a custom strchr to avoid mixing escaped and unescaped string
operations.

Reported-by: Han Han 
Fixes: https://bugzilla.redhat.com/1873913
Signed-off-by: Connor Kuehl 
---
 block/rbd.c| 20 ++--
 tests/qemu-iotests/231 |  4 
 tests/qemu-iotests/231.out |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9071a00e3f..c0e4d4a952 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -134,6 +134,22 @@ static char *qemu_rbd_next_tok(char *src, char delim, char 
**p)
 return src;
 }
 
+static char *qemu_rbd_strchr(char *src, char delim)
+{
+char *p;
+
+for (p = src; *p; ++p) {
+if (*p == delim) {
+return p;
+}
+if (*p == '\\') {
+++p;
+}
+}
+
+return NULL;
+}
+
 static void qemu_rbd_unescape(char *src)
 {
 char *p;
@@ -171,7 +187,7 @@ static void qemu_rbd_parse_filename(const char *filename, 
QDict *options,
 qemu_rbd_unescape(found_str);
 qdict_put_str(options, "pool", found_str);
 
-if (strchr(p, '@')) {
+if (qemu_rbd_strchr(p, '@')) {
 image_name = qemu_rbd_next_tok(p, '@', );
 
 found_str = qemu_rbd_next_tok(p, ':', );
@@ -181,7 +197,7 @@ static void qemu_rbd_parse_filename(const char *filename, 
QDict *options,
 image_name = qemu_rbd_next_tok(p, ':', );
 }
 /* Check for namespace in the image_name */
-if (strchr(image_name, '/')) {
+if (qemu_rbd_strchr(image_name, '/')) {
 found_str = qemu_rbd_next_tok(image_name, '/', _name);
 qemu_rbd_unescape(found_str);
 qdict_put_str(options, "namespace", found_str);
diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
index 0f66d0ca36..8e6c6447c1 100755
--- a/tests/qemu-iotests/231
+++ b/tests/qemu-iotests/231
@@ -55,6 +55,10 @@ _filter_conf()
 $QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
 $QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
 
+# Regression test: the qemu-img invocation is expected to fail, but it should
+# not seg fault the parser.
+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 747dd221bb..a785a6e859 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: 
ceph-mon
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
+unable to get monitor info from DNS SRV with service name: ceph-mon
+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or 
directory
 *** done
-- 
2.30.2

[PATCH v2 1/2] iotests/231: Update expected deprecation message

[Connor Kuehl]

The deprecation message in the expected output has technically been
wrong since the wrong version of a patch was applied to it. Because of
this, the test fails. Correct the expected output so that it passes.

Signed-off-by: Connor Kuehl 
Reviewed-by: Max Reitz 
---
Reworded the commit log and added Max's R-b.

 tests/qemu-iotests/231.out | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 579ba11c16..747dd221bb 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -1,9 +1,7 @@
 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
+qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is 
deprecated
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
 *** done
-- 
2.30.2

[PATCH v2 0/2] Fix segfault in qemu_rbd_parse_filename

[Connor Kuehl]

Replaced the change to qemu_rbd_next_tok with a standalone escape-aware
helper for patch 2.

Connor Kuehl (2):
  iotests/231: Update expected deprecation message
  block/rbd: Add an escape-aware strchr helper

 block/rbd.c| 20 ++--
 tests/qemu-iotests/231 |  4 
 tests/qemu-iotests/231.out |  7 ---
 3 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.30.2

Re: [PATCH v3] hw/smbios: support for type 41 (onboard devices extended information)

[Igor Mammedov]

On Thu,  1 Apr 2021 14:26:58 +0200
Vincent Bernat  wrote:

> Type 41 defines the attributes of devices that are onboard. The
> original intent was to imply the BIOS had some level of control over
> the enablement of the associated devices.
> 
> If network devices are present in this table, by default, udev will
> name the corresponding interfaces enoX, X being the instance number.
> Without such information, udev will fallback to using the PCI ID and
> this usually gives ens3 or ens4. This can be a bit annoying as the
> name of the network card may depend on the order of options and may
> change if a new PCI device is added earlier on the commande line.
> Being able to provide SMBIOS type 41 entry ensure the name of the
> interface won't change and helps the user guess the right name without
> booting a first time.
> 
> This can be invoked with:
> 
> $QEMU -netdev user,id=internet
>   -device 
> virtio-net-pci,mac=50:54:00:00:00:42,netdev=internet,id=internet-dev \
>   -smbios type=41,designation='Onboard 
> LAN',instance=1,kind=ethernet,pcidev=internet-dev

an ACPI alternative was merged recently (current master).
assigning 'designation=' wasn't implemented there, but important part
of giving users control over PCI devices 'eno' index is implemented.

When I looked into the issue, smbios way was a bit over-kill for the task
and didn't really work if hotplug were used.

See, for example how to use new feature:
 https://www.mail-archive.com/qemu-devel@nongnu.org/msg794164.html


> The PCI segment is assumed to be 0. This should hold true for most
> cases.
> 
> $ dmidecode -t 41
> # dmidecode 3.3
> Getting SMBIOS data from sysfs.
> SMBIOS 2.8 present.
> 
> Handle 0x2900, DMI type 41, 11 bytes
> Onboard Device
> Reference Designation: Onboard LAN
> Type: Ethernet
> Status: Enabled
> Type Instance: 1
> Bus Address: :00:09.0
> 
> $ ip -brief a
> lo   UNKNOWN127.0.0.1/8 ::1/128
> eno1 UP 10.0.2.14/24 fec0::5254:ff:fe00:42/64 
> fe80::5254:ff:fe00:42/64
> 
> Signed-off-by: Vincent Bernat 
> ---
>  hw/arm/virt.c|   7 ++-
>  hw/i386/fw_cfg.c |   4 +-
>  hw/smbios/smbios.c   | 112 ++-
>  include/hw/firmware/smbios.h |  14 -
>  qemu-options.hx  |   7 ++-
>  5 files changed, 138 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index aa2bbd14e090..840ec0af02db 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -53,6 +53,7 @@
>  #include "sysemu/kvm.h"
>  #include "hw/loader.h"
>  #include "exec/address-spaces.h"
> +#include "qapi/error.h"
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
> @@ -1524,8 +1525,10 @@ static void virt_build_smbios(VirtMachineState *vms)
>  vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
>  true, SMBIOS_ENTRY_POINT_30);
>  
> -smbios_get_tables(MACHINE(vms), NULL, 0, _tables, 
> _tables_len,
> -  _anchor, _anchor_len);
> +smbios_get_tables(MACHINE(vms), NULL, 0,
> +  _tables, _tables_len,
> +  _anchor, _anchor_len,
> +  _fatal);
>  
>  if (smbios_anchor) {
>  fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-tables",
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index e48a54fa364b..4e68d5dea438 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -22,6 +22,7 @@
>  #include "hw/nvram/fw_cfg.h"
>  #include "e820_memory_layout.h"
>  #include "kvm/kvm_i386.h"
> +#include "qapi/error.h"
>  #include CONFIG_DEVICES
>  
>  struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> @@ -78,7 +79,8 @@ void fw_cfg_build_smbios(MachineState *ms, FWCfgState 
> *fw_cfg)
>  }
>  smbios_get_tables(ms, mem_array, array_count,
>_tables, _tables_len,
> -  _anchor, _anchor_len);
> +  _anchor, _anchor_len,
> +  _fatal);
>  g_free(mem_array);
>  
>  if (smbios_anchor) {
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index f22c4f5b734e..8d26564972c3 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -27,6 +27,7 @@
>  #include "hw/firmware/smbios.h"
>  #include "hw/loader.h"
>  #include "hw/boards.h"
> +#include "hw/pci/pci.h"
>  #include "smbios_build.h"
>  
>  /* legacy structures and constants for <= 2.0 machines */
> @@ -118,6 +119,28 @@ static struct {
>  uint16_t speed;
>  } type17;
>  
> +static QEnumLookup type41_kind_lookup = {
> +.array = (const char *const[]) {
> +"other",
> +"unknown",
> +"video",
> +"scsi",
> +"ethernet",
> +"tokenring",
> +"sound",
> +"pata",
> +"sata",
> +"sas",
> +},
> +.size = 10
> +};
> 

[PATCH v4 1/1] acpi: Consolidate the handling of OEM ID and OEM Table ID fields

[Marian Postevca]

Introduces structure AcpiBuildOem to hold the value of OEM fields and
uses dedicated helper functions to initialize/set the values.
Unnecessary dynamically allocated OEM fields are re-factored to static
allocation.

Signed-off-by: Marian Postevca 
---
 hw/acpi/hmat.h   |  2 +-
 hw/i386/acpi-common.h|  2 +-
 include/hw/acpi/acpi-build-oem.h | 61 +
 include/hw/acpi/aml-build.h  | 15 +++---
 include/hw/acpi/ghes.h   |  2 +-
 include/hw/acpi/pci.h|  2 +-
 include/hw/acpi/vmgenid.h|  2 +-
 include/hw/arm/virt.h|  4 +-
 include/hw/i386/x86.h|  4 +-
 include/hw/mem/nvdimm.h  |  4 +-
 hw/acpi/aml-build.c  | 27 ++-
 hw/acpi/ghes.c   |  5 +-
 hw/acpi/hmat.c   |  4 +-
 hw/acpi/nvdimm.c | 22 +
 hw/acpi/pci.c|  4 +-
 hw/acpi/vmgenid.c|  6 ++-
 hw/arm/virt-acpi-build.c | 40 ++--
 hw/arm/virt.c| 16 +++
 hw/i386/acpi-build.c | 78 +++-
 hw/i386/acpi-common.c|  4 +-
 hw/i386/acpi-microvm.c   | 13 ++
 hw/i386/x86.c| 19 
 22 files changed, 188 insertions(+), 148 deletions(-)
 create mode 100644 include/hw/acpi/acpi-build-oem.h

diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index b57f0e7e80..39c42328bd 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -38,6 +38,6 @@
 #define HMAT_PROXIMITY_INITIATOR_VALID  0x1
 
 void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *numa_state,
-const char *oem_id, const char *oem_table_id);
+struct AcpiBuildOem *bld_oem);
 
 #endif
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index b12cd73ea5..27c2e5b6a9 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -10,6 +10,6 @@
 
 void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
  X86MachineState *x86ms, AcpiDeviceIf *adev,
- const char *oem_id, const char *oem_table_id);
+ struct AcpiBuildOem *bld_oem);
 
 #endif
diff --git a/include/hw/acpi/acpi-build-oem.h b/include/hw/acpi/acpi-build-oem.h
new file mode 100644
index 00..d4b445677a
--- /dev/null
+++ b/include/hw/acpi/acpi-build-oem.h
@@ -0,0 +1,61 @@
+#ifndef QEMU_HW_ACPI_BUILD_OEM_H
+#define QEMU_HW_ACPI_BUILD_OEM_H
+
+/*
+ * Utilities for working with ACPI OEM ID and OEM TABLE ID fields
+ *
+ * Copyright (c) 2021 Marian Postevca
+ *
+ * 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 .
+ */
+#include "qemu/cutils.h"
+
+#define ACPI_BUILD_APPNAME6 "BOCHS "
+#define ACPI_BUILD_APPNAME8 "BXPC"
+
+#define ACPI_BUILD_OEM_ID_SIZE 6
+#define ACPI_BUILD_OEM_TABLE_ID_SIZE 8
+
+struct AcpiBuildOem {
+char oem_id[ACPI_BUILD_OEM_ID_SIZE + 1];
+char oem_table_id[ACPI_BUILD_OEM_TABLE_ID_SIZE + 1];
+};
+
+static inline void ACPI_BUILD_OEM_SET_ID(struct AcpiBuildOem *bld_oem,
+ const char *oem_id)
+{
+pstrcpy(bld_oem->oem_id, sizeof bld_oem->oem_id, oem_id);
+}
+
+static inline void ACPI_BUILD_OEM_SET_TABLE_ID(struct AcpiBuildOem *bld_oem,
+   const char *oem_table_id)
+{
+pstrcpy(bld_oem->oem_table_id,
+sizeof bld_oem->oem_table_id, oem_table_id);
+}
+
+static inline void ACPI_BUILD_OEM_INIT(struct AcpiBuildOem *bld_oem,
+   const char *oem_id,
+   const char *oem_table_id)
+{
+ACPI_BUILD_OEM_SET_ID(bld_oem, oem_id);
+ACPI_BUILD_OEM_SET_TABLE_ID(bld_oem, oem_table_id);
+}
+
+static inline void ACPI_BUILD_OEM_INIT_DEFAULT(struct AcpiBuildOem *bld_oem)
+{
+ACPI_BUILD_OEM_INIT(bld_oem, ACPI_BUILD_APPNAME6, ACPI_BUILD_APPNAME8);
+}
+
+#endif /* QEMU_HW_ACPI_BUILD_OEM_H */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 471266d739..b5a9223158 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,9 +3,8 @@
 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/acpi-build-oem.h"
 
-#define ACPI_BUILD_APPNAME6 "BOCHS "
-#define ACPI_BUILD_APPNAME8 "BXPC"
 
 #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables"
 #define 

[PATCH v4 0/1] Rework ACPI OEM fields handling to simplify code (was: acpi: Remove duplicated code handling OEM ID and OEM table ID fields)

[Marian Postevca]

This patch consolidates ACPI OEM fields handling
by:
- Moving common code in PC and MICROVM to X86.
- Changes unnecessary dynamic memory allocation to static allocation
- Uses dedicated structure to keep values of fields instead of two
  separate strings
- Adds helper functions to initialize the structure

v2:
- Move the setters/getters of OEM fields to X86MachineState to
  remove duplication
- Change commit message to make it clear the second commit is
  a re-factor

v3:
- Rebase "acpi: Consolidate the handling of OEM ID and OEM
  Table ID fields to latest" to latest HEAD
- Dropped "acpi: Move setters/getters of oem fields to
   X86MachineState" since it was accepted already

v4:
- Drop helper macros and use static inline functions instead
- Removed variables starting with __
- Used consistent naming for helper functions that start with ACPI_BUILD_OEM_*
- Didn't drop the defines ACPI_BUILD_APPNAME6 and ACPI_BUILD_APPNAME8 since
  ACPI_BUILD_APPNAME8 is still used in build_header() in aml-build.c and it
  feels better to keep them defined together. But if others prefer to drop the
  ACPI_BUILD_APPNAME6 define, will resend the patch.

Marian Postevca (1):
  acpi: Consolidate the handling of OEM ID and OEM Table ID fields

 hw/acpi/hmat.h   |  2 +-
 hw/i386/acpi-common.h|  2 +-
 include/hw/acpi/acpi-build-oem.h | 61 +
 include/hw/acpi/aml-build.h  | 15 +++---
 include/hw/acpi/ghes.h   |  2 +-
 include/hw/acpi/pci.h|  2 +-
 include/hw/acpi/vmgenid.h|  2 +-
 include/hw/arm/virt.h|  4 +-
 include/hw/i386/x86.h|  4 +-
 include/hw/mem/nvdimm.h  |  4 +-
 hw/acpi/aml-build.c  | 27 ++-
 hw/acpi/ghes.c   |  5 +-
 hw/acpi/hmat.c   |  4 +-
 hw/acpi/nvdimm.c | 22 +
 hw/acpi/pci.c|  4 +-
 hw/acpi/vmgenid.c|  6 ++-
 hw/arm/virt-acpi-build.c | 40 ++--
 hw/arm/virt.c| 16 +++
 hw/i386/acpi-build.c | 78 +++-
 hw/i386/acpi-common.c|  4 +-
 hw/i386/acpi-microvm.c   | 13 ++
 hw/i386/x86.c| 19 
 22 files changed, 188 insertions(+), 148 deletions(-)
 create mode 100644 include/hw/acpi/acpi-build-oem.h

-- 
2.26.2

Re: [1/1] tcg/mips: Fix SoftTLB comparison on mips backend

[Philippe Mathieu-Daudé]

On 4/1/21 12:04 PM, Kele Huang wrote:
> The addrl used to compare with SoftTLB entry should be sign-extended
> in common case, and it will cause constant failing in SoftTLB
> comparisons for the addrl whose address is over 0x8000 on the
> emulation of 32-bit guest on 64-bit host.

Apparently missed in commit f0d703314ec
("tcg-mips: Adjust qemu_ld/st for mips64").

> 
> This is an important performance bug fix. Spec2000 gzip rate increase
> from ~45 to ~140 on Loongson 3A4000 (MIPS compatible platform).
> 
> Signed-off-by: Kele Huang 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tcg/mips/tcg-target.c.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
> index 8738a3a581..8b16726242 100644
> --- a/tcg/mips/tcg-target.c.inc
> +++ b/tcg/mips/tcg-target.c.inc
> @@ -1201,13 +1201,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg 
> base, TCGReg addrl,
> load the tlb addend for the fast path.  */
>  tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off);
>  }
> -tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
>  
>  /* Zero extend a 32-bit guest address for a 64-bit host. */
>  if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
>  tcg_out_ext32u(s, base, addrl);
>  addrl = base;
>  }
> +tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
>  
>  label_ptr[0] = s->code_ptr;
>  tcg_out_opc_br(s, OPC_BNE, TCG_TMP1, TCG_TMP0);
> 

[qemu-web PATCH] add link to the code of conduct

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 contribute.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contribute.md b/contribute.md
index bcb048e..d7e295f 100644
--- a/contribute.md
+++ b/contribute.md
@@ -16,3 +16,5 @@ permalink: /contribute/
   [Contributor FAQ](https://wiki.qemu.org/Contribute/FAQ),
   [How to submit a 
patch](https://wiki.qemu.org/Contribute/SubmitAPatch),
   [Improve the 
website](https://www.qemu.org/2017/02/04/the-new-qemu-website-is-up/)
+
+Contributing to QEMU is subject to the [QEMU Code of 
Conduct](https://qemu.org/docs/master/devel/code-of-conduct.html).
-- 
2.30.1

[PATCH v3 0/4] Add support for Shakti SoC from IIT-M

[Vijai Kumar K]

Changes in v3:
 - Drop SHAKTI_C_DEBUG register

Changes in v2:
 - Moved CPU addition to a separate patch(P1)
 - Use riscv_setup_rom_resetvec API to setup reset vector
 - Dropped unused DPRINTF and unwanted break statements
 - Fixed uart_can_receive logic
 - Reused sifive_u_cpu_init routine for shakti
 - Error out when an unsupported CPU is specified
 - Addressed formatting changes pointed out in review

Vijai Kumar K (4):
  target/riscv: Add Shakti C class CPU
  riscv: Add initial support for Shakti C machine
  hw/char: Add Shakti UART emulation
  hw/riscv: Connect Shakti UART to Shakti platform

 MAINTAINERS |   9 +
 default-configs/devices/riscv64-softmmu.mak |   1 +
 hw/char/meson.build |   1 +
 hw/char/shakti_uart.c   | 185 
 hw/char/trace-events|   4 +
 hw/riscv/Kconfig|  10 ++
 hw/riscv/meson.build|   1 +
 hw/riscv/shakti_c.c | 178 +++
 include/hw/char/shakti_uart.h   |  74 
 include/hw/riscv/shakti_c.h |  75 
 target/riscv/cpu.c  |   1 +
 target/riscv/cpu.h  |   1 +
 12 files changed, 540 insertions(+)
 create mode 100644 hw/char/shakti_uart.c
 create mode 100644 hw/riscv/shakti_c.c
 create mode 100644 include/hw/char/shakti_uart.h
 create mode 100644 include/hw/riscv/shakti_c.h

-- 
2.25.1

[PATCH v3 3/4] hw/char: Add Shakti UART emulation

[Vijai Kumar K]

This is the initial implementation of Shakti UART.

Signed-off-by: Vijai Kumar K 
---
 MAINTAINERS   |   2 +
 hw/char/meson.build   |   1 +
 hw/char/shakti_uart.c | 185 ++
 hw/char/trace-events  |   4 +
 include/hw/char/shakti_uart.h |  74 ++
 5 files changed, 266 insertions(+)
 create mode 100644 hw/char/shakti_uart.c
 create mode 100644 include/hw/char/shakti_uart.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9f71c4cc3f..be084865db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1385,7 +1385,9 @@ M: Vijai Kumar K 
 L: qemu-ri...@nongnu.org
 S: Supported
 F: hw/riscv/shakti_c.c
+F: hw/char/shakti_uart.c
 F: include/hw/riscv/shakti_c.h
+F: include/hw/char/shakti_uart.h
 
 SiFive Machines
 M: Alistair Francis 
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 7ba38dbd96..61c43d4b51 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -19,6 +19,7 @@ softmmu_ss.add(when: 'CONFIG_SERIAL', if_true: 
files('serial.c'))
 softmmu_ss.add(when: 'CONFIG_SERIAL_ISA', if_true: files('serial-isa.c'))
 softmmu_ss.add(when: 'CONFIG_SERIAL_PCI', if_true: files('serial-pci.c'))
 softmmu_ss.add(when: 'CONFIG_SERIAL_PCI_MULTI', if_true: 
files('serial-pci-multi.c'))
+softmmu_ss.add(when: 'CONFIG_SHAKTI', if_true: files('shakti_uart.c'))
 softmmu_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: 
files('virtio-console.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen_console.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_uartlite.c'))
diff --git a/hw/char/shakti_uart.c b/hw/char/shakti_uart.c
new file mode 100644
index 00..6870821325
--- /dev/null
+++ b/hw/char/shakti_uart.c
@@ -0,0 +1,185 @@
+/*
+ * SHAKTI UART
+ *
+ * Copyright (c) 2021 Vijai Kumar K 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/char/shakti_uart.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "qemu/log.h"
+
+static uint64_t shakti_uart_read(void *opaque, hwaddr addr, unsigned size)
+{
+ShaktiUartState *s = opaque;
+
+switch (addr) {
+case SHAKTI_UART_BAUD:
+return s->uart_baud;
+case SHAKTI_UART_RX:
+qemu_chr_fe_accept_input(>chr);
+s->uart_status &= ~SHAKTI_UART_STATUS_RX_NOT_EMPTY;
+return s->uart_rx;
+case SHAKTI_UART_STATUS:
+return s->uart_status;
+case SHAKTI_UART_DELAY:
+return s->uart_delay;
+case SHAKTI_UART_CONTROL:
+return s->uart_control;
+case SHAKTI_UART_INT_EN:
+return s->uart_interrupt;
+case SHAKTI_UART_IQ_CYCLES:
+return s->uart_iq_cycles;
+case SHAKTI_UART_RX_THRES:
+return s->uart_rx_threshold;
+default:
+/* Also handles TX REG which is write only */
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+}
+
+return 0;
+}
+
+static void shakti_uart_write(void *opaque, hwaddr addr,
+  uint64_t data, unsigned size)
+{
+ShaktiUartState *s = opaque;
+uint32_t value = data;
+uint8_t ch;
+
+switch (addr) {
+case SHAKTI_UART_BAUD:
+s->uart_baud = value;
+break;
+case SHAKTI_UART_TX:
+ch = value;
+qemu_chr_fe_write_all(>chr, , 1);
+s->uart_status &= ~SHAKTI_UART_STATUS_TX_FULL;
+break;
+case SHAKTI_UART_STATUS:
+s->uart_status = value;
+break;
+case SHAKTI_UART_DELAY:
+s->uart_delay = value;
+break;
+case SHAKTI_UART_CONTROL:
+s->uart_control = value;
+break;
+case SHAKTI_UART_INT_EN:
+s->uart_interrupt = value;
+break;
+case SHAKTI_UART_IQ_CYCLES:
+s->uart_iq_cycles = value;
+break;
+case SHAKTI_UART_RX_THRES:
+s->uart_rx_threshold = value;
+break;
+default:
+

Re: [PATCH v3 05/11] esp: introduce esp_fifo_pop_buf() and use it instead of fifo8_pop_buf()

[Philippe Mathieu-Daudé]

On 4/1/21 12:51 PM, Mark Cave-Ayland wrote:
> On 01/04/2021 10:34, Philippe Mathieu-Daudé wrote:
>> On 4/1/21 9:49 AM, Mark Cave-Ayland wrote:
>>> The const pointer returned by fifo8_pop_buf() lies directly within
>>> the array used
>>> to model the FIFO. Building with address sanitisers enabled shows
>>> that if the
>>
>> Typo "sanitizers"
> 
> Ha. It's definitely "sanitiser" here in the UK (UK English) as opposed
> to "sanitizer" (US English).

Oh OK, TIL :)

> I don't really mind either way, but I can
> fix this if it needs a v4 following Paolo's comments.

Not needed since it is correct.

>>> caller expects a minimum number of bytes present then if the FIFO is
>>> nearly full,
>>> the caller may unexpectedly access past the end of the array.
>>
>> Why isn't it a problem with the other models? Because the pointed
>> buffer is consumed directly?
> 
> Yes that's correct, which is why Fifo8 currently doesn't support
> wraparound. I haven't analysed how other devices have used it but I
> would imagine there would be an ASan hit if it were being misused this way.
> 
>>> Introduce esp_fifo_pop_buf() which takes a destination buffer and
>>> performs a
>>> memcpy() in it to guarantee that the caller cannot overwrite the FIFO
>>> array and
>>> update all callers to use it. Similarly add underflow protection
>>> similar to
>>> esp_fifo_push() and esp_fifo_pop() so that instead of triggering an
>>> assert()
>>> the operation becomes a no-op.
>>
>> This is OK for your ESP model.
>>
>> Now thinking loudly about the Fifo8 API, shouldn't this be part of it?
>>
>> Something prototype like:
>>
>>    /**
>>     * fifo8_pop_buf:
>>     * @do_copy: If %true, also copy data to @bufptr.
>>     */
>>    size_t fifo8_pop_buf(Fifo8 *fifo,
>>     void **bufptr,
>>     size_t buflen,
>>     bool do_copy);
> 
> That could work, and may even allow support for wraparound in future. I
> suspect things would become clearer after looking at the other Fifo8
> users to see if this is worth an API change/alternative API.

Thanks for the feedback!

Phil.

[PATCH v3 2/4] riscv: Add initial support for Shakti C machine

[Vijai Kumar K]

Add support for emulating Shakti reference platform based on C-class
running on arty-100T board.

https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/README.rst

Signed-off-by: Vijai Kumar K 
---
 MAINTAINERS |   7 +
 default-configs/devices/riscv64-softmmu.mak |   1 +
 hw/riscv/Kconfig|  10 ++
 hw/riscv/meson.build|   1 +
 hw/riscv/shakti_c.c | 170 
 include/hw/riscv/shakti_c.h |  73 +
 6 files changed, 262 insertions(+)
 create mode 100644 hw/riscv/shakti_c.c
 create mode 100644 include/hw/riscv/shakti_c.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e9f0d591e..9f71c4cc3f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1380,6 +1380,13 @@ F: include/hw/misc/mchp_pfsoc_dmc.h
 F: include/hw/misc/mchp_pfsoc_ioscb.h
 F: include/hw/misc/mchp_pfsoc_sysreg.h
 
+Shakti C class SoC
+M: Vijai Kumar K 
+L: qemu-ri...@nongnu.org
+S: Supported
+F: hw/riscv/shakti_c.c
+F: include/hw/riscv/shakti_c.h
+
 SiFive Machines
 M: Alistair Francis 
 M: Bin Meng 
diff --git a/default-configs/devices/riscv64-softmmu.mak 
b/default-configs/devices/riscv64-softmmu.mak
index d5eec75f05..bc69301fa4 100644
--- a/default-configs/devices/riscv64-softmmu.mak
+++ b/default-configs/devices/riscv64-softmmu.mak
@@ -13,3 +13,4 @@ CONFIG_SIFIVE_E=y
 CONFIG_SIFIVE_U=y
 CONFIG_RISCV_VIRT=y
 CONFIG_MICROCHIP_PFSOC=y
+CONFIG_SHAKTI_C=y
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index d139074b02..92a62b5ce9 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -19,6 +19,16 @@ config OPENTITAN
 select IBEX
 select UNIMP
 
+config SHAKTI
+bool
+
+config SHAKTI_C
+bool
+select UNIMP
+select SHAKTI
+select SIFIVE_CLINT
+select SIFIVE_PLIC
+
 config RISCV_VIRT
 bool
 imply PCI_DEVICES
diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
index 275c0f7eb7..a97454661c 100644
--- a/hw/riscv/meson.build
+++ b/hw/riscv/meson.build
@@ -4,6 +4,7 @@ riscv_ss.add(files('numa.c'))
 riscv_ss.add(files('riscv_hart.c'))
 riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true: files('opentitan.c'))
 riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c'))
+riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c'))
 riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c'))
 riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c'))
 riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
new file mode 100644
index 00..c8205d3f22
--- /dev/null
+++ b/hw/riscv/shakti_c.c
@@ -0,0 +1,170 @@
+/*
+ * Shakti C-class SoC emulation
+ *
+ * Copyright (c) 2021 Vijai Kumar K 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "hw/riscv/shakti_c.h"
+#include "qapi/error.h"
+#include "hw/intc/sifive_plic.h"
+#include "hw/intc/sifive_clint.h"
+#include "sysemu/sysemu.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/riscv/boot.h"
+
+
+static const struct MemmapEntry {
+hwaddr base;
+hwaddr size;
+} shakti_c_memmap[] = {
+[SHAKTI_C_ROM]   =  {  0x1000,  0x2000   },
+[SHAKTI_C_RAM]   =  {  0x8000,  0x0  },
+[SHAKTI_C_UART]  =  {  0x00011300,  0x00040  },
+[SHAKTI_C_GPIO]  =  {  0x020d,  0x00100  },
+[SHAKTI_C_PLIC]  =  {  0x0c00,  0x2  },
+[SHAKTI_C_CLINT] =  {  0x0200,  0xc  },
+[SHAKTI_C_I2C]   =  {  0x20c0,  0x00100  },
+};
+
+static void shakti_c_machine_state_init(MachineState *mstate)
+{
+ShaktiCMachineState *sms = RISCV_SHAKTI_MACHINE(mstate);
+MemoryRegion *system_memory = get_system_memory();
+MemoryRegion *main_mem = g_new(MemoryRegion, 1);
+
+/* Allow only Shakti C CPU for this platform */
+if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) {
+error_report("This board can only be used with Shakti C CPU");
+exit(1);
+}
+
+/* Initialize SoC */
+object_initialize_child(OBJECT(mstate), "soc", >soc,
+TYPE_RISCV_SHAKTI_SOC);
+qdev_realize(DEVICE(>soc), NULL, _abort);
+
+/* register RAM */
+memory_region_init_ram(main_mem, NULL, "riscv.shakti.c.ram",
+   mstate->ram_size, _fatal);
+memory_region_add_subregion(system_memory,
+  

[PATCH v3 4/4] hw/riscv: Connect Shakti UART to Shakti platform

[Vijai Kumar K]

Connect one shakti uart to the shakti_c machine.

Signed-off-by: Vijai Kumar K 
---
 hw/riscv/shakti_c.c | 8 
 include/hw/riscv/shakti_c.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index c8205d3f22..e207fa83dd 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -125,6 +125,13 @@ static void shakti_c_soc_state_realize(DeviceState *dev, 
Error **errp)
 SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
 SIFIVE_CLINT_TIMEBASE_FREQ, false);
 
+qdev_prop_set_chr(DEVICE(&(sss->uart)), "chardev", serial_hd(0));
+if (!sysbus_realize(SYS_BUS_DEVICE(>uart), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>uart), 0,
+shakti_c_memmap[SHAKTI_C_UART].base);
+
 /* ROM */
 memory_region_init_rom(>rom, OBJECT(dev), "riscv.shakti.c.rom",
shakti_c_memmap[SHAKTI_C_ROM].size, _fatal);
@@ -143,6 +150,7 @@ static void shakti_c_soc_instance_init(Object *obj)
 ShaktiCSoCState *sss = RISCV_SHAKTI_SOC(obj);
 
 object_initialize_child(obj, "cpus", >cpus, TYPE_RISCV_HART_ARRAY);
+object_initialize_child(obj, "uart", >uart, TYPE_SHAKTI_UART);
 
 /*
  * CPU type is fixed and we are not supporting passing from commandline 
yet.
diff --git a/include/hw/riscv/shakti_c.h b/include/hw/riscv/shakti_c.h
index 8ffc2b0213..50a2b79086 100644
--- a/include/hw/riscv/shakti_c.h
+++ b/include/hw/riscv/shakti_c.h
@@ -21,6 +21,7 @@
 
 #include "hw/riscv/riscv_hart.h"
 #include "hw/boards.h"
+#include "hw/char/shakti_uart.h"
 
 #define TYPE_RISCV_SHAKTI_SOC "riscv.shakti.cclass.soc"
 #define RISCV_SHAKTI_SOC(obj) \
@@ -33,6 +34,7 @@ typedef struct ShaktiCSoCState {
 /*< public >*/
 RISCVHartArrayState cpus;
 DeviceState *plic;
+ShaktiUartState uart;
 MemoryRegion rom;
 
 } ShaktiCSoCState;
-- 
2.25.1

[PULL 9/9] pci: sprinkle assert in PCI pin number

[Michael S. Tsirkin]

From: Isaku Yamahata 

If a device model
(a) doesn't set the value to a correct interrupt number and then
(b) triggers an interrupt for itself,
it's device model bug. Add assert on interrupt pin number to catch
this kind of bug more obviously.

Suggested-by: Peter Maydell 
Signed-off-by: Isaku Yamahata 
Message-Id: 
<9cf8ac3b17e162daac0971d7be32deb6a33ae6ec.1616532563.git.isaku.yamah...@intel.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/pci/pci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ac9a24889c..8f35e13a0c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1450,6 +1450,8 @@ static void pci_irq_handler(void *opaque, int irq_num, 
int level)
 PCIDevice *pci_dev = opaque;
 int change;
 
+assert(0 <= irq_num && irq_num < PCI_NUM_PINS);
+assert(level == 0 || level == 1);
 change = level - pci_irq_state(pci_dev, irq_num);
 if (!change)
 return;
@@ -1469,6 +1471,7 @@ static inline int pci_intx(PCIDevice *pci_dev)
 qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
 {
 int intx = pci_intx(pci_dev);
+assert(0 <= intx && intx < PCI_NUM_PINS);
 
 return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
 }
-- 
MST

[PATCH v3 1/4] target/riscv: Add Shakti C class CPU

[Vijai Kumar K]

C-Class is a member of the SHAKTI family of processors from IIT-M.

It is an extremely configurable and commercial-grade 5-stage in-order
core supporting the standard RV64GCSUN ISA extensions.

Signed-off-by: Vijai Kumar K 
---
 target/riscv/cpu.c | 1 +
 target/riscv/cpu.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2a990f6253..140094fd52 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -707,6 +707,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
 DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   rv64_base_cpu_init),
 DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64_sifive_e_cpu_init),
 DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64_sifive_u_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, rv64_sifive_u_cpu_init),
 #endif
 };
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0edb2826a2..ebbf15fb1c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -38,6 +38,7 @@
 #define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
 #define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
 #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
+#define TYPE_RISCV_CPU_SHAKTI_C RISCV_CPU_TYPE_NAME("shakti-c")
 #define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
 #define TYPE_RISCV_CPU_SIFIVE_E34   RISCV_CPU_TYPE_NAME("sifive-e34")
 #define TYPE_RISCV_CPU_SIFIVE_E51   RISCV_CPU_TYPE_NAME("sifive-e51")
-- 
2.25.1

[PULL 6/9] acpi/piix4: reinitialize acpi PM device on reset

[Michael S. Tsirkin]

From: Isaku Yamahata 

Commit 6be8cf56bc8b made sure that SCI is enabled in PM1.CNT
on reset in acpi_only mode by modifying acpi_pm1_cnt_reset() and
that worked for q35 as expected.

The function was introduced by commit
  eaba51c573a (acpi, acpi_piix, vt82c686: factor out PM1_CNT logic)
that forgot to actually call it at piix4 reset time and as result
SCI_EN wasn't set as was expected by 6be8cf56bc8b in acpi_only mode.

So Windows crashes when it notices that SCI_EN is not set and FADT is
not providing information about how to enable it anymore.
Reproducer:
   qemu-system-x86_64 -enable-kvm -M pc-i440fx-6.0,smm=off -cdrom 
any_windows_10x64.iso

Fix it by calling acpi_pm1_cnt_reset() at piix4 reset time.

Occasionally this patch adds reset acpi PM related registers on
piix4 reset time and de-assert sci.
piix4_pm_realize() initializes acpi pm tmr, evt, cnt and gpe.
Reset them on device reset. pm_reset() in ich9.c correctly calls
corresponding reset functions.

Fixes: 6be8cf56bc8b (acpi/core: always set SCI_EN when SMM isn't supported)
Reported-by: Reinoud Zandijk 
Co-developed-by: Igor Mammedov 
Signed-off-by: Igor Mammedov 
Signed-off-by: Isaku Yamahata 
Message-Id: 
<8a5bbd19727045ec863523830078dd4ca63f6a9a.1616532563.git.isaku.yamah...@intel.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/acpi/piix4.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 6056d51667..8f8b0e95e5 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -326,6 +326,13 @@ static void piix4_pm_reset(DeviceState *dev)
 /* Mark SMM as already inited (until KVM supports SMM). */
 pci_conf[0x5B] = 0x02;
 }
+
+acpi_pm1_evt_reset(>ar);
+acpi_pm1_cnt_reset(>ar);
+acpi_pm_tmr_reset(>ar);
+acpi_gpe_reset(>ar);
+acpi_update_sci(>ar, s->irq);
+
 pm_io_space_update(s);
 acpi_pcihp_reset(>acpi_pci_hotplug, !s->use_acpi_root_pci_hotplug);
 }
-- 
MST

[PULL 7/9] vt82c686.c: don't raise SCI when PCI_INTERRUPT_PIN isn't setup

[Michael S. Tsirkin]

From: Isaku Yamahata 

Without this patch, the following patch will triger clan runtime
sanitizer warnings as follows. This patch proactively works around it.
I leave a correct fix to v582c686.c maintainerfix as I'm not sure
about fuloong2e device model.

> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/qom-test --tap -k
> PASS 1 qtest-mips64el/qom-test /mips64el/qom/loongson3-virt
> PASS 2 qtest-mips64el/qom-test /mips64el/qom/none
> PASS 3 qtest-mips64el/qom-test /mips64el/qom/magnum
> PASS 4 qtest-mips64el/qom-test /mips64el/qom/mipssim
> PASS 5 qtest-mips64el/qom-test /mips64el/qom/malta
> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative
> PASS 6 qtest-mips64el/qom-test /mips64el/qom/fuloong2e
> PASS 7 qtest-mips64el/qom-test /mips64el/qom/boston
> PASS 8 qtest-mips64el/qom-test /mips64el/qom/pica61
>
> and similarly for eg
>
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/endianness-test
> --tap -k
> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative
> PASS 1 qtest-mips64el/endianness-test /mips64el/endianness/fuloong2e
> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative
> PASS 2 qtest-mips64el/endianness-test /mips64el/endianness/split/fuloong2e
> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative
> PASS 3 qtest-mips64el/endianness-test /mips64el/endianness/combine/fuloong2e

Cc: BALATON Zoltan 
Cc: Huacai Chen 
Cc: "Philippe Mathieu-Daudé" 
Cc: Jiaxun Yang 
Reported-by: Peter Maydell 
Signed-off-by: Isaku Yamahata 
Message-Id: 
<62a5fc69e453fb848bfd4794bae1852a75af73c5.1616532563.git.isaku.yamah...@intel.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/isa/vt82c686.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 05d084f698..f0fb309f12 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -144,7 +144,18 @@ static void pm_update_sci(ViaPMState *s)
ACPI_BITMASK_POWER_BUTTON_ENABLE |
ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
ACPI_BITMASK_TIMER_ENABLE)) != 0);
-pci_set_irq(>dev, sci_level);
+if (pci_get_byte(s->dev.config + PCI_INTERRUPT_PIN)) {
+/*
+ * FIXME:
+ * Fix device model that realizes this PM device and remove
+ * this work around.
+ * The device model should wire SCI and setup
+ * PCI_INTERRUPT_PIN properly.
+ * If PIN# = 0(interrupt pin isn't used), don't raise SCI as
+ * work around.
+ */
+pci_set_irq(>dev, sci_level);
+}
 /* schedule a timer interruption if needed */
 acpi_pm_tmr_update(>ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) 
&&
!(pmsts & ACPI_BITMASK_TIMER_STATUS));
-- 
MST

Re: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

[Connor Kuehl]

On 4/1/21 12:24 PM, Max Reitz wrote:

On 01.04.21 17:52, Connor Kuehl wrote:

That's qemu_rbd_unescape()'s job! No need to duplicate the labor.

Furthermore, this was causing some confusion in the parsing logic to
where the caller might test for the presence of a character to split on
like so:

if (strchr(image_name, '/')) {
 found_str = qemu_rbd_next_tok(image_name, '/', _name);
[..]

When qemu_rbd_next_tok() performs unescaping as a side effect, the
parser can get confused thinking that it can split this string, but
really the delimiter '/' gets unescaped and so qemu_rbd_next_tok() never
"finds" the delimiter and consumes the rest of the token input stream.


I don’t fully understand.  I understand the strchr() problem and the 
thing you explain next.  But I would have thought that’s a problem of 
using strchr(), i.e. that we’d need a custom function to do strchr() but 
consider escaped characters.  If it’s just about true/false like in your 
example, it could be a new version of qemu_rbd_next_tok() that does not 
modify the string.


I went back and forth a lot on the different ways this can be fixed 
before sending this, and I agree the most consistent fix here would be 
to add an escape-aware strchr. Initially, adding another libc-like 
function with more side effects wasn't as appealing to me as removing 
the side effects entirely to separate mechanism vs. policy. Your example 
below convinced me that this patch would split the token in unexpected 
ways. I'll send a v2 :-)


Thanks,

Connor


[..]
Should it?  I would have fully expected it to not be split and the 
parser complains that the input is invalid.  Or, let’s say, 
"foo\/bar/baz” should be split into “foo\/bar” and “baz”.

[PULL 3/9] vhost-user-blk: add immediate cleanup on shutdown

[Michael S. Tsirkin]

From: Denis Plotnikov 

Qemu crashes on shutdown if the chardev used by vhost-user-blk has been
finalized before the vhost-user-blk.

This happens with char-socket chardev operating in the listening mode (server).
The char-socket chardev emits "close" event at the end of finalizing when
its internal data is destroyed. This calls vhost-user-blk event handler
which in turn tries to manipulate with destroyed chardev by setting an empty
event handler for vhost-user-blk cleanup postponing.

This patch separates the shutdown case from the cleanup postponing removing
the need to set an event handler.

Signed-off-by: Denis Plotnikov 
Message-Id: <20210325151217.262793-4-den-plotni...@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/block/vhost-user-blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 4e215f71f1..0b5b9d44cd 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -411,7 +411,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent 
event,
  * other code perform its own cleanup sequence using vhost_dev data
  * (e.g. vhost_dev_set_log).
  */
-if (realized) {
+if (realized && !runstate_check(RUN_STATE_SHUTDOWN)) {
 /*
  * A close event may happen during a read/write, but vhost
  * code assumes the vhost_dev remains setup, so delay the
-- 
MST

[PULL 5/9] virtio-pci: remove explicit initialization of val

[Michael S. Tsirkin]

From: Yuri Benditovich 

The value is assigned later in this procedure.

Signed-off-by: Yuri Benditovich 
Message-Id: <20210315115937.14286-3-yuri.benditov...@daynix.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio-pci.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4a3dcee771..c1b67cf6fc 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1385,10 +1385,10 @@ static uint64_t virtio_pci_device_read(void *opaque, 
hwaddr addr,
 {
 VirtIOPCIProxy *proxy = opaque;
 VirtIODevice *vdev = virtio_bus_get_device(>bus);
-uint64_t val = 0;
+uint64_t val;
 
 if (vdev == NULL) {
-return val;
+return 0;
 }
 
 switch (size) {
@@ -1401,6 +1401,9 @@ static uint64_t virtio_pci_device_read(void *opaque, 
hwaddr addr,
 case 4:
 val = virtio_config_modern_readl(vdev, addr);
 break;
+default:
+val = 0;
+break;
 }
 return val;
 }
-- 
MST

[PULL 4/9] virtio-pci: add check for vdev in virtio_pci_isr_read

[Michael S. Tsirkin]

From: Yuri Benditovich 

https://bugzilla.redhat.com/show_bug.cgi?id=1743098
This commit completes the solution of segfault in hot unplug flow
(by commit ccec7e9603f446fe75c6c563ba335c00cfda6a06).
Added missing check for vdev in virtio_pci_isr_read.
Typical stack of crash:
virtio_pci_isr_read ../hw/virtio/virtio-pci.c:1365 with proxy-vdev = 0
memory_region_read_accessor at ../softmmu/memory.c:442
access_with_adjusted_size at ../softmmu/memory.c:552
memory_region_dispatch_read1 at ../softmmu/memory.c:1420
memory_region_dispatch_read  at ../softmmu/memory.c:1449
flatview_read_continue at ../softmmu/physmem.c:2822
flatview_read at ../softmmu/physmem.c:2862
address_space_read_full at ../softmmu/physmem.c:2875

Signed-off-by: Yuri Benditovich 
Message-Id: <20210315115937.14286-2-yuri.benditov...@daynix.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio-pci.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 883045a223..4a3dcee771 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1364,9 +1364,14 @@ static uint64_t virtio_pci_isr_read(void *opaque, hwaddr 
addr,
 {
 VirtIOPCIProxy *proxy = opaque;
 VirtIODevice *vdev = virtio_bus_get_device(>bus);
-uint64_t val = qatomic_xchg(>isr, 0);
-pci_irq_deassert(>pci_dev);
+uint64_t val;
 
+if (vdev == NULL) {
+return 0;
+}
+
+val = qatomic_xchg(>isr, 0);
+pci_irq_deassert(>pci_dev);
 return val;
 }
 
-- 
MST

[PULL 8/9] isa/v582c686: Reinitialize ACPI PM device on reset

[Michael S. Tsirkin]

From: Isaku Yamahata 

Commit 6be8cf56bc8b made sure that SCI is enabled in PM1.CNT
on reset in acpi_only mode by modifying acpi_pm1_cnt_reset() and
that worked for q35 as expected.

This patch adds reset ACPI PM related registers on vt82c686 reset time
and de-assert sci.
via_pm_realize() initializes acpi pm tmr, evt, cnt and gpe.
Reset them on device reset.

Cc: BALATON Zoltan 
Cc: Huacai Chen 
Cc: "Philippe Mathieu-Daudé" 
Cc: Jiaxun Yang 
Signed-off-by: Isaku Yamahata 
Message-Id: 
<0a3fe998525552860919a690ce83dab8f663ab99.1616532563.git.isaku.yamah...@intel.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/isa/vt82c686.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index f0fb309f12..98325bb32b 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -178,6 +178,11 @@ static void via_pm_reset(DeviceState *d)
 /* SMBus IO base */
 pci_set_long(s->dev.config + 0x90, 1);
 
+acpi_pm1_evt_reset(>ar);
+acpi_pm1_cnt_reset(>ar);
+acpi_pm_tmr_reset(>ar);
+pm_update_sci(s);
+
 pm_io_space_update(s);
 smb_io_space_update(s);
 }
-- 
MST

[PULL 1/9] vhost-user-blk: use different event handlers on initialization

[Michael S. Tsirkin]

From: Denis Plotnikov 

It is useful to use different connect/disconnect event handlers
on device initialization and operation as seen from the further
commit fixing a bug on device initialization.

This patch refactors the code to make use of them: we don't rely any
more on the VM state for choosing how to cleanup the device, instead
we explicitly use the proper event handler depending on whether
the device has been initialized.

Signed-off-by: Denis Plotnikov 
Reviewed-by: Raphael Norwitz 
Message-Id: <20210325151217.262793-2-den-plotni...@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/block/vhost-user-blk.c | 31 ---
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index b870a50e6b..1af95ec6aa 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -362,7 +362,18 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
 vhost_dev_cleanup(>dev);
 }
 
-static void vhost_user_blk_event(void *opaque, QEMUChrEvent event);
+static void vhost_user_blk_event(void *opaque, QEMUChrEvent event,
+ bool realized);
+
+static void vhost_user_blk_event_realize(void *opaque, QEMUChrEvent event)
+{
+vhost_user_blk_event(opaque, event, false);
+}
+
+static void vhost_user_blk_event_oper(void *opaque, QEMUChrEvent event)
+{
+vhost_user_blk_event(opaque, event, true);
+}
 
 static void vhost_user_blk_chr_closed_bh(void *opaque)
 {
@@ -371,11 +382,12 @@ static void vhost_user_blk_chr_closed_bh(void *opaque)
 VHostUserBlk *s = VHOST_USER_BLK(vdev);
 
 vhost_user_blk_disconnect(dev);
-qemu_chr_fe_set_handlers(>chardev, NULL, NULL, vhost_user_blk_event,
-NULL, opaque, NULL, true);
+qemu_chr_fe_set_handlers(>chardev, NULL, NULL,
+vhost_user_blk_event_oper, NULL, opaque, NULL, true);
 }
 
-static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
+static void vhost_user_blk_event(void *opaque, QEMUChrEvent event,
+ bool realized)
 {
 DeviceState *dev = opaque;
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -406,7 +418,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent 
event)
  * TODO: maybe it is a good idea to make the same fix
  * for other vhost-user devices.
  */
-if (runstate_is_running()) {
+if (realized) {
 AioContext *ctx = qemu_get_current_aio_context();
 
 qemu_chr_fe_set_handlers(>chardev, NULL, NULL, NULL, NULL,
@@ -473,8 +485,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, 
Error **errp)
 s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
 s->connected = false;
 
-qemu_chr_fe_set_handlers(>chardev,  NULL, NULL, vhost_user_blk_event,
- NULL, (void *)dev, NULL, true);
+qemu_chr_fe_set_handlers(>chardev,  NULL, NULL,
+ vhost_user_blk_event_realize, NULL, (void *)dev,
+ NULL, true);
 
 reconnect:
 if (qemu_chr_fe_wait_connected(>chardev, ) < 0) {
@@ -494,6 +507,10 @@ reconnect:
 goto reconnect;
 }
 
+/* we're fully initialized, now we can operate, so change the handler */
+qemu_chr_fe_set_handlers(>chardev,  NULL, NULL,
+ vhost_user_blk_event_oper, NULL, (void *)dev,
+ NULL, true);
 return;
 
 virtio_err:
-- 
MST

[PULL 0/9] pc,virtio,pci: bugfixes

[Michael S. Tsirkin]

The following changes since commit 1bd16067b652cce41a9214d0c62c73d5b45ab4b1:

  Merge remote-tracking branch 
'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-31 
16:38:49 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 8ddf54324858ce5e35272efa449f27fc0a19f957:

  pci: sprinkle assert in PCI pin number (2021-04-01 12:19:52 -0400)


pc,virtio,pci: bugfixes

Fixes all over the place.

Signed-off-by: Michael S. Tsirkin 


Denis Plotnikov (3):
  vhost-user-blk: use different event handlers on initialization
  vhost-user-blk: perform immediate cleanup if disconnect on initialization
  vhost-user-blk: add immediate cleanup on shutdown

Isaku Yamahata (4):
  acpi/piix4: reinitialize acpi PM device on reset
  vt82c686.c: don't raise SCI when PCI_INTERRUPT_PIN isn't setup
  isa/v582c686: Reinitialize ACPI PM device on reset
  pci: sprinkle assert in PCI pin number

Yuri Benditovich (2):
  virtio-pci: add check for vdev in virtio_pci_isr_read
  virtio-pci: remove explicit initialization of val

 hw/acpi/piix4.c   |  7 +
 hw/block/vhost-user-blk.c | 79 ---
 hw/isa/vt82c686.c | 18 ++-
 hw/pci/pci.c  |  3 ++
 hw/virtio/virtio-pci.c| 16 +++---
 5 files changed, 87 insertions(+), 36 deletions(-)

[PULL 2/9] vhost-user-blk: perform immediate cleanup if disconnect on initialization

[Michael S. Tsirkin]

From: Denis Plotnikov 

Commit 4bcad76f4c39 ("vhost-user-blk: delay vhost_user_blk_disconnect")
introduced postponing vhost_dev cleanup aiming to eliminate qemu aborts
because of connection problems with vhost-blk daemon.

However, it introdues a new problem. Now, any communication errors
during execution of vhost_dev_init() called by vhost_user_blk_device_realize()
lead to qemu abort on assert in vhost_dev_get_config().

This happens because vhost_user_blk_disconnect() is postponed but
it should have dropped s->connected flag by the time
vhost_user_blk_device_realize() performs a new connection opening.
On the connection opening, vhost_dev initialization in
vhost_user_blk_connect() relies on s->connection flag and
if it's not dropped, it skips vhost_dev initialization and returns
with success. Then, vhost_user_blk_device_realize()'s execution flow
goes to vhost_dev_get_config() where it's aborted on the assert.

To fix the problem this patch adds immediate cleanup on device
initialization(in vhost_user_blk_device_realize()) using different
event handlers for initialization and operation introduced in the
previous patch.
On initialization (in vhost_user_blk_device_realize()) we fully
control the initialization process. At that point, nobody can use the
device since it isn't initialized and we don't need to postpone any
cleanups, so we can do cleaup right away when there is a communication
problem with the vhost-blk daemon.
On operation we leave it as is, since the disconnect may happen when
the device is in use, so the device users may want to use vhost_dev's data
to do rollback before vhost_dev is re-initialized (e.g. in vhost_dev_set_log()).

Signed-off-by: Denis Plotnikov 
Reviewed-by: Raphael Norwitz 
Message-Id: <20210325151217.262793-3-den-plotni...@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/block/vhost-user-blk.c | 48 +++
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 1af95ec6aa..4e215f71f1 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -402,38 +402,38 @@ static void vhost_user_blk_event(void *opaque, 
QEMUChrEvent event,
 break;
 case CHR_EVENT_CLOSED:
 /*
- * A close event may happen during a read/write, but vhost
- * code assumes the vhost_dev remains setup, so delay the
- * stop & clear. There are two possible paths to hit this
- * disconnect event:
- * 1. When VM is in the RUN_STATE_PRELAUNCH state. The
- * vhost_user_blk_device_realize() is a caller.
- * 2. In tha main loop phase after VM start.
- *
- * For p2 the disconnect event will be delayed. We can't
- * do the same for p1, because we are not running the loop
- * at this moment. So just skip this step and perform
- * disconnect in the caller function.
- *
- * TODO: maybe it is a good idea to make the same fix
- * for other vhost-user devices.
+ * Closing the connection should happen differently on device
+ * initialization and operation stages.
+ * On initalization, we want to re-start vhost_dev initialization
+ * from the very beginning right away when the connection is closed,
+ * so we clean up vhost_dev on each connection closing.
+ * On operation, we want to postpone vhost_dev cleanup to let the
+ * other code perform its own cleanup sequence using vhost_dev data
+ * (e.g. vhost_dev_set_log).
  */
 if (realized) {
+/*
+ * A close event may happen during a read/write, but vhost
+ * code assumes the vhost_dev remains setup, so delay the
+ * stop & clear.
+ */
 AioContext *ctx = qemu_get_current_aio_context();
 
 qemu_chr_fe_set_handlers(>chardev, NULL, NULL, NULL, NULL,
 NULL, NULL, false);
 aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, opaque);
-}
 
-/*
- * Move vhost device to the stopped state. The vhost-user device
- * will be clean up and disconnected in BH. This can be useful in
- * the vhost migration code. If disconnect was caught there is an
- * option for the general vhost code to get the dev state without
- * knowing its type (in this case vhost-user).
- */
-s->dev.started = false;
+/*
+ * Move vhost device to the stopped state. The vhost-user device
+ * will be clean up and disconnected in BH. This can be useful in
+ * the vhost migration code. If disconnect was caught there is an
+ * option for the general vhost code to get the dev state without
+ * knowing its type (in this case vhost-user).
+ */
+s->dev.started = false;
+ 

Re: [PATCH RESEND v2 0/6] target/arm: Add nested virtualization support

[Andrea Bolognani]

On Thu, 2021-04-01 at 12:55 +, Haibo Xu wrote:
> v2:
>   - Move the NV to a CPU feature flag(Andrea)
>   - Add CPU feature 'el2' test(Andrew)
> 
> Many thanks to Andrea and Andrew for their comments!
> 
> This series add support for ARMv8.3/8.4 nested virtualization support
> in KVM mode. It's based on Marc Zyngier's kernel KVM patches[1], and 
> has been tested on a FVP model to run a L2 guest with Qemu. Now the 
> feature can be enabled by "-M virt,accel=kvm -cpu host,el2=on" when
> starting a VM. 

This looks good from the user interface point of view, thanks for
addressing the concerns that were raised!

I'll leave Drew to review the actual code changes :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2 2/4] riscv: Add initial support for Shakti C machine

[Vijai Kumar K]

 On Thu, 01 Apr 2021 22:51:42 +0530 Alistair Francis  
wrote 

 > On Thu, Apr 1, 2021 at 1:18 PM Vijai Kumar K  wrote: 
 > > 
 > > 
 > > 
 > > 
 > >  On Wed, 31 Mar 2021 21:05:47 +0530 Alistair Francis 
 > >  wrote  
 > > 
 > >  > On Sun, Mar 21, 2021 at 1:09 AM Vijai Kumar K  
 > > wrote: 
 > >  > > 
 > >  > > Add support for emulating Shakti reference platform based on C-class 
 > >  > > running on arty-100T board. 
 > >  > > 
 > >  > > 
 > > https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/README.rst 
 > >  > > 
 > >  > > Signed-off-by: Vijai Kumar K  
 > >  > > --- 
 > >  > >  MAINTAINERS |   7 + 
 > >  > >  default-configs/devices/riscv64-softmmu.mak |   1 + 
 > >  > >  hw/riscv/Kconfig|  10 ++ 
 > >  > >  hw/riscv/meson.build|   1 + 
 > >  > >  hw/riscv/shakti_c.c | 171 
 > >  
 > >  > >  include/hw/riscv/shakti_c.h |  74 + 
 > >  > >  6 files changed, 264 insertions(+) 
 > >  > >  create mode 100644 hw/riscv/shakti_c.c 
 > >  > >  create mode 100644 include/hw/riscv/shakti_c.h 
 > >  > > 
 > >  > > diff --git a/MAINTAINERS b/MAINTAINERS 
 > >  > > index 8e9f0d591e..9f71c4cc3f 100644 
 > >  > > --- a/MAINTAINERS 
 > >  > > +++ b/MAINTAINERS 
 > >  > > @@ -1380,6 +1380,13 @@ F: include/hw/misc/mchp_pfsoc_dmc.h 
 > >  > >  F: include/hw/misc/mchp_pfsoc_ioscb.h 
 > >  > >  F: include/hw/misc/mchp_pfsoc_sysreg.h 
 > >  > > 
 > >  > > +Shakti C class SoC 
 > >  > > +M: Vijai Kumar K  
 > >  > > +L: qemu-ri...@nongnu.org 
 > >  > > +S: Supported 
 > >  > > +F: hw/riscv/shakti_c.c 
 > >  > > +F: include/hw/riscv/shakti_c.h 
 > >  > > + 
 > >  > >  SiFive Machines 
 > >  > >  M: Alistair Francis  
 > >  > >  M: Bin Meng  
 > >  > > diff --git a/default-configs/devices/riscv64-softmmu.mak 
 > > b/default-configs/devices/riscv64-softmmu.mak 
 > >  > > index d5eec75f05..bc69301fa4 100644 
 > >  > > --- a/default-configs/devices/riscv64-softmmu.mak 
 > >  > > +++ b/default-configs/devices/riscv64-softmmu.mak 
 > >  > > @@ -13,3 +13,4 @@ CONFIG_SIFIVE_E=y 
 > >  > >  CONFIG_SIFIVE_U=y 
 > >  > >  CONFIG_RISCV_VIRT=y 
 > >  > >  CONFIG_MICROCHIP_PFSOC=y 
 > >  > > +CONFIG_SHAKTI_C=y 
 > >  > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig 
 > >  > > index d139074b02..92a62b5ce9 100644 
 > >  > > --- a/hw/riscv/Kconfig 
 > >  > > +++ b/hw/riscv/Kconfig 
 > >  > > @@ -19,6 +19,16 @@ config OPENTITAN 
 > >  > >  select IBEX 
 > >  > >  select UNIMP 
 > >  > > 
 > >  > > +config SHAKTI 
 > >  > > +bool 
 > >  > > + 
 > >  > > +config SHAKTI_C 
 > >  > > +bool 
 > >  > > +select UNIMP 
 > >  > > +select SHAKTI 
 > >  > > +select SIFIVE_CLINT 
 > >  > > +select SIFIVE_PLIC 
 > >  > > + 
 > >  > >  config RISCV_VIRT 
 > >  > >  bool 
 > >  > >  imply PCI_DEVICES 
 > >  > > diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build 
 > >  > > index 275c0f7eb7..a97454661c 100644 
 > >  > > --- a/hw/riscv/meson.build 
 > >  > > +++ b/hw/riscv/meson.build 
 > >  > > @@ -4,6 +4,7 @@ riscv_ss.add(files('numa.c')) 
 > >  > >  riscv_ss.add(files('riscv_hart.c')) 
 > >  > >  riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true: 
 > > files('opentitan.c')) 
 > >  > >  riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c')) 
 > >  > > +riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c')) 
 > >  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c')) 
 > >  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c')) 
 > >  > >  riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c')) 
 > >  > > diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c 
 > >  > > new file mode 100644 
 > >  > > index 00..45d0eedabd 
 > >  > > --- /dev/null 
 > >  > > +++ b/hw/riscv/shakti_c.c 
 > >  > > @@ -0,0 +1,171 @@ 
 > >  > > +/* 
 > >  > > + * Shakti C-class SoC emulation 
 > >  > > + * 
 > >  > > + * Copyright (c) 2021 Vijai Kumar K  
 > >  > > + * 
 > >  > > + * This program is free software; you can redistribute it and/or 
 > > modify it 
 > >  > > + * under the terms and conditions of the GNU General Public License, 
 > >  > > + * version 2 or later, as published by the Free Software Foundation. 
 > >  > > + * 
 > >  > > + * This program is distributed in the hope 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 . 
 > >  > > + */ 
 > >  > > + 
 > >  > > +#include "qemu/osdep.h" 
 > >  > > +#include "hw/boards.h" 
 > >  > > +#include "hw/riscv/shakti_c.h" 
 > >  > > +#include 

[PATCH] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure

[Brad Smith]

OpenBSD prior to 6.3 required a workaround to utilize fcntl(F_SETFL) on memory
devices.

Since modern verions of OpenBSD that are only officialy supported and buildable
on do not have this issue I am garbage collecting this workaround.


Signed-off-by: Brad Smith 

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 36820fec16..7b4bec1402 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -273,17 +273,6 @@ int qemu_try_set_nonblock(int fd)
 return -errno;
 }
 if (fcntl(fd, F_SETFL, f | O_NONBLOCK) == -1) {
-#ifdef __OpenBSD__
-/*
- * Previous to OpenBSD 6.3, fcntl(F_SETFL) is not permitted on
- * memory devices and sets errno to ENODEV.
- * It's OK if we fail to set O_NONBLOCK on devices like /dev/null,
- * because they will never block anyway.
- */
-if (errno == ENODEV) {
-return 0;
-}
-#endif
 return -errno;
 }
 return 0;

Re: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

[Max Reitz]

On 01.04.21 17:52, Connor Kuehl wrote:

That's qemu_rbd_unescape()'s job! No need to duplicate the labor.

Furthermore, this was causing some confusion in the parsing logic to
where the caller might test for the presence of a character to split on
like so:

if (strchr(image_name, '/')) {
 found_str = qemu_rbd_next_tok(image_name, '/', _name);
[..]

When qemu_rbd_next_tok() performs unescaping as a side effect, the
parser can get confused thinking that it can split this string, but
really the delimiter '/' gets unescaped and so qemu_rbd_next_tok() never
"finds" the delimiter and consumes the rest of the token input stream.


I don’t fully understand.  I understand the strchr() problem and the 
thing you explain next.  But I would have thought that’s a problem of 
using strchr(), i.e. that we’d need a custom function to do strchr() but 
consider escaped characters.  If it’s just about true/false like in your 
example, it could be a new version of qemu_rbd_next_tok() that does not 
modify the string.


Or, well, actually, in your example, one could unconditionally invoke 
qemu_rbd_next_tok() and check whether *found_str is '\0' or not.



This is problematic because qemu_rbd_next_tok() also steals the input
pointer to the token stream and sets it to NULL. This causes a segfault
where the parser expects to split one string into two.


I would say that’s a problem of the caller.  It can pass a different 
variable for the out-pointer, so the input pointer isn’t stolen.



In this case, the parser is determining if a string is a
namespace/image_name pair like so:

"foo/bar"

And clearly it's looking to split it like so:

namespace:  foo
image_name: bar

but if the input is "foo\/bar", it *should* split into

namespace:  foo\
image_name: bar


Should it?  I would have fully expected it to not be split and the 
parser complains that the input is invalid.  Or, let’s say, 
"foo\/bar/baz” should be split into “foo\/bar” and “baz”.


Max


and its subordinate parts can be unescaped after tokenization.

So, instead of tokenizing *and* escaping all at once, do one before the
other to avoid stumbling into a segfault by confusing the parsing logic.

Reported-by: Han Han 
Fixes: https://bugzilla.redhat.com/1873913
Signed-off-by: Connor Kuehl 
---
  block/rbd.c| 3 ---
  tests/qemu-iotests/231 | 4 
  tests/qemu-iotests/231.out | 3 +++
  3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9071a00e3f..9bed0863e5 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -123,9 +123,6 @@ static char *qemu_rbd_next_tok(char *src, char delim, char 
**p)
  if (*end == delim) {
  break;
  }
-if (*end == '\\' && end[1] != '\0') {
-end++;
-}
  }
  if (*end == delim) {
  *p = end + 1;
diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
index 0f66d0ca36..8e6c6447c1 100755
--- a/tests/qemu-iotests/231
+++ b/tests/qemu-iotests/231
@@ -55,6 +55,10 @@ _filter_conf()
  $QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
  $QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
  
+# Regression test: the qemu-img invocation is expected to fail, but it should

+# not seg fault the parser.
+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
+
  # success, all done
  echo "*** done"
  rm -f $seq.full
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 747dd221bb..a785a6e859 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: 
ceph-mon
  qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
  unable to get monitor info from DNS SRV with service name: ceph-mon
  qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
+unable to get monitor info from DNS SRV with service name: ceph-mon
+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or 
directory
  *** done

Re: [PATCH v2 2/4] riscv: Add initial support for Shakti C machine

[Alistair Francis]

On Thu, Apr 1, 2021 at 1:18 PM Vijai Kumar K  wrote:
>
>
>
>
>  On Wed, 31 Mar 2021 21:05:47 +0530 Alistair Francis 
>  wrote 
>
>  > On Sun, Mar 21, 2021 at 1:09 AM Vijai Kumar K  
> wrote:
>  > >
>  > > Add support for emulating Shakti reference platform based on C-class
>  > > running on arty-100T board.
>  > >
>  > > 
> https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/README.rst
>  > >
>  > > Signed-off-by: Vijai Kumar K 
>  > > ---
>  > >  MAINTAINERS |   7 +
>  > >  default-configs/devices/riscv64-softmmu.mak |   1 +
>  > >  hw/riscv/Kconfig|  10 ++
>  > >  hw/riscv/meson.build|   1 +
>  > >  hw/riscv/shakti_c.c | 171 
>  > >  include/hw/riscv/shakti_c.h |  74 +
>  > >  6 files changed, 264 insertions(+)
>  > >  create mode 100644 hw/riscv/shakti_c.c
>  > >  create mode 100644 include/hw/riscv/shakti_c.h
>  > >
>  > > diff --git a/MAINTAINERS b/MAINTAINERS
>  > > index 8e9f0d591e..9f71c4cc3f 100644
>  > > --- a/MAINTAINERS
>  > > +++ b/MAINTAINERS
>  > > @@ -1380,6 +1380,13 @@ F: include/hw/misc/mchp_pfsoc_dmc.h
>  > >  F: include/hw/misc/mchp_pfsoc_ioscb.h
>  > >  F: include/hw/misc/mchp_pfsoc_sysreg.h
>  > >
>  > > +Shakti C class SoC
>  > > +M: Vijai Kumar K 
>  > > +L: qemu-ri...@nongnu.org
>  > > +S: Supported
>  > > +F: hw/riscv/shakti_c.c
>  > > +F: include/hw/riscv/shakti_c.h
>  > > +
>  > >  SiFive Machines
>  > >  M: Alistair Francis 
>  > >  M: Bin Meng 
>  > > diff --git a/default-configs/devices/riscv64-softmmu.mak 
> b/default-configs/devices/riscv64-softmmu.mak
>  > > index d5eec75f05..bc69301fa4 100644
>  > > --- a/default-configs/devices/riscv64-softmmu.mak
>  > > +++ b/default-configs/devices/riscv64-softmmu.mak
>  > > @@ -13,3 +13,4 @@ CONFIG_SIFIVE_E=y
>  > >  CONFIG_SIFIVE_U=y
>  > >  CONFIG_RISCV_VIRT=y
>  > >  CONFIG_MICROCHIP_PFSOC=y
>  > > +CONFIG_SHAKTI_C=y
>  > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>  > > index d139074b02..92a62b5ce9 100644
>  > > --- a/hw/riscv/Kconfig
>  > > +++ b/hw/riscv/Kconfig
>  > > @@ -19,6 +19,16 @@ config OPENTITAN
>  > >  select IBEX
>  > >  select UNIMP
>  > >
>  > > +config SHAKTI
>  > > +bool
>  > > +
>  > > +config SHAKTI_C
>  > > +bool
>  > > +select UNIMP
>  > > +select SHAKTI
>  > > +select SIFIVE_CLINT
>  > > +select SIFIVE_PLIC
>  > > +
>  > >  config RISCV_VIRT
>  > >  bool
>  > >  imply PCI_DEVICES
>  > > diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
>  > > index 275c0f7eb7..a97454661c 100644
>  > > --- a/hw/riscv/meson.build
>  > > +++ b/hw/riscv/meson.build
>  > > @@ -4,6 +4,7 @@ riscv_ss.add(files('numa.c'))
>  > >  riscv_ss.add(files('riscv_hart.c'))
>  > >  riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true: files('opentitan.c'))
>  > >  riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c'))
>  > > +riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
>  > > diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
>  > > new file mode 100644
>  > > index 00..45d0eedabd
>  > > --- /dev/null
>  > > +++ b/hw/riscv/shakti_c.c
>  > > @@ -0,0 +1,171 @@
>  > > +/*
>  > > + * Shakti C-class SoC emulation
>  > > + *
>  > > + * Copyright (c) 2021 Vijai Kumar K 
>  > > + *
>  > > + * This program is free software; you can redistribute it and/or modify 
> it
>  > > + * under the terms and conditions of the GNU General Public License,
>  > > + * version 2 or later, as published by the Free Software Foundation.
>  > > + *
>  > > + * This program is distributed in the hope 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 .
>  > > + */
>  > > +
>  > > +#include "qemu/osdep.h"
>  > > +#include "hw/boards.h"
>  > > +#include "hw/riscv/shakti_c.h"
>  > > +#include "qapi/error.h"
>  > > +#include "hw/intc/sifive_plic.h"
>  > > +#include "hw/intc/sifive_clint.h"
>  > > +#include "sysemu/sysemu.h"
>  > > +#include "hw/qdev-properties.h"
>  > > +#include "exec/address-spaces.h"
>  > > +#include "hw/riscv/boot.h"
>  > > +
>  > > +
>  > > +static const struct MemmapEntry {
>  > > +hwaddr base;
>  > > +hwaddr size;
>  > > +} shakti_c_memmap[] = {
>  > > +[SHAKTI_C_ROM]   =  {  0x1000,  0x2000   },
>  > > +[SHAKTI_C_RAM]   =  {  0x8000,  0x0  },
>  > > +[SHAKTI_C_UART]  =  {  0x00011300,  0x00040  },
>  

Re: [PATCH v2 2/4] riscv: Add initial support for Shakti C machine

[Vijai Kumar K]

 On Wed, 31 Mar 2021 21:05:47 +0530 Alistair Francis  
wrote 

 > On Sun, Mar 21, 2021 at 1:09 AM Vijai Kumar K  wrote: 
 > > 
 > > Add support for emulating Shakti reference platform based on C-class 
 > > running on arty-100T board. 
 > > 
 > > https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/README.rst 
 > > 
 > > Signed-off-by: Vijai Kumar K  
 > > --- 
 > >  MAINTAINERS |   7 + 
 > >  default-configs/devices/riscv64-softmmu.mak |   1 + 
 > >  hw/riscv/Kconfig|  10 ++ 
 > >  hw/riscv/meson.build|   1 + 
 > >  hw/riscv/shakti_c.c | 171  
 > >  include/hw/riscv/shakti_c.h |  74 + 
 > >  6 files changed, 264 insertions(+) 
 > >  create mode 100644 hw/riscv/shakti_c.c 
 > >  create mode 100644 include/hw/riscv/shakti_c.h 
 > > 
 > > diff --git a/MAINTAINERS b/MAINTAINERS 
 > > index 8e9f0d591e..9f71c4cc3f 100644 
 > > --- a/MAINTAINERS 
 > > +++ b/MAINTAINERS 
 > > @@ -1380,6 +1380,13 @@ F: include/hw/misc/mchp_pfsoc_dmc.h 
 > >  F: include/hw/misc/mchp_pfsoc_ioscb.h 
 > >  F: include/hw/misc/mchp_pfsoc_sysreg.h 
 > > 
 > > +Shakti C class SoC 
 > > +M: Vijai Kumar K  
 > > +L: qemu-ri...@nongnu.org 
 > > +S: Supported 
 > > +F: hw/riscv/shakti_c.c 
 > > +F: include/hw/riscv/shakti_c.h 
 > > + 
 > >  SiFive Machines 
 > >  M: Alistair Francis  
 > >  M: Bin Meng  
 > > diff --git a/default-configs/devices/riscv64-softmmu.mak 
 > > b/default-configs/devices/riscv64-softmmu.mak 
 > > index d5eec75f05..bc69301fa4 100644 
 > > --- a/default-configs/devices/riscv64-softmmu.mak 
 > > +++ b/default-configs/devices/riscv64-softmmu.mak 
 > > @@ -13,3 +13,4 @@ CONFIG_SIFIVE_E=y 
 > >  CONFIG_SIFIVE_U=y 
 > >  CONFIG_RISCV_VIRT=y 
 > >  CONFIG_MICROCHIP_PFSOC=y 
 > > +CONFIG_SHAKTI_C=y 
 > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig 
 > > index d139074b02..92a62b5ce9 100644 
 > > --- a/hw/riscv/Kconfig 
 > > +++ b/hw/riscv/Kconfig 
 > > @@ -19,6 +19,16 @@ config OPENTITAN 
 > >  select IBEX 
 > >  select UNIMP 
 > > 
 > > +config SHAKTI 
 > > +bool 
 > > + 
 > > +config SHAKTI_C 
 > > +bool 
 > > +select UNIMP 
 > > +select SHAKTI 
 > > +select SIFIVE_CLINT 
 > > +select SIFIVE_PLIC 
 > > + 
 > >  config RISCV_VIRT 
 > >  bool 
 > >  imply PCI_DEVICES 
 > > diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build 
 > > index 275c0f7eb7..a97454661c 100644 
 > > --- a/hw/riscv/meson.build 
 > > +++ b/hw/riscv/meson.build 
 > > @@ -4,6 +4,7 @@ riscv_ss.add(files('numa.c')) 
 > >  riscv_ss.add(files('riscv_hart.c')) 
 > >  riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true: files('opentitan.c')) 
 > >  riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c')) 
 > > +riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c')) 
 > >  riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c')) 
 > >  riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c')) 
 > >  riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c')) 
 > > diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c 
 > > new file mode 100644 
 > > index 00..45d0eedabd 
 > > --- /dev/null 
 > > +++ b/hw/riscv/shakti_c.c 
 > > @@ -0,0 +1,171 @@ 
 > > +/* 
 > > + * Shakti C-class SoC emulation 
 > > + * 
 > > + * Copyright (c) 2021 Vijai Kumar K  
 > > + * 
 > > + * This program is free software; you can redistribute it and/or modify 
 > > it 
 > > + * under the terms and conditions of the GNU General Public License, 
 > > + * version 2 or later, as published by the Free Software Foundation. 
 > > + * 
 > > + * This program is distributed in the hope 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 . 
 > > + */ 
 > > + 
 > > +#include "qemu/osdep.h" 
 > > +#include "hw/boards.h" 
 > > +#include "hw/riscv/shakti_c.h" 
 > > +#include "qapi/error.h" 
 > > +#include "hw/intc/sifive_plic.h" 
 > > +#include "hw/intc/sifive_clint.h" 
 > > +#include "sysemu/sysemu.h" 
 > > +#include "hw/qdev-properties.h" 
 > > +#include "exec/address-spaces.h" 
 > > +#include "hw/riscv/boot.h" 
 > > + 
 > > + 
 > > +static const struct MemmapEntry { 
 > > +hwaddr base; 
 > > +hwaddr size; 
 > > +} shakti_c_memmap[] = { 
 > > +[SHAKTI_C_ROM]   =  {  0x1000,  0x2000   }, 
 > > +[SHAKTI_C_RAM]   =  {  0x8000,  0x0  }, 
 > > +[SHAKTI_C_UART]  =  {  0x00011300,  0x00040  }, 
 > > +[SHAKTI_C_GPIO]  =  {  0x020d,  0x00100  }, 
 > > +[SHAKTI_C_PLIC]  =  {  0x0c00,  0x2  }, 
 > > +[SHAKTI_C_CLINT] =  {  0x0200,  0xc  }, 
 > > + 

Re: [PATCH] migration: Remove time_t cast for OpenBSD

[Brad Smith]

On 4/1/2021 4:14 AM, Daniel P. Berrangé wrote:

On Wed, Mar 31, 2021 at 03:26:16PM -0400, Brad Smith wrote:

On 3/13/2021 6:33 PM, Brad Smith wrote:

On 3/11/2021 1:39 PM, Daniel P. Berrangé wrote:

On Thu, Mar 11, 2021 at 06:28:57PM +, Dr. David Alan Gilbert wrote:

* Laurent Vivier (laur...@vivier.eu) wrote:

Le 08/03/2021 à 12:46, Thomas Huth a écrit :

On 22/02/2021 08.28, Brad Smith wrote:

OpenBSD has supported 64-bit time_t across all archs
since 5.5 released in 2014.

Remove a time_t cast that is no longer necessary.


Signed-off-by: Brad Smith 

diff --git a/migration/savevm.c b/migration/savevm.c
index 52e2d72e4b..9557f85ba9 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2849,8 +2849,7 @@ bool save_snapshot(const char
*name, bool overwrite, const char *vmstate,
  if (name) {
  pstrcpy(sn->name, sizeof(sn->name), name);
  } else {
-    /* cast below needed for OpenBSD where
tv_sec is still 'long' */
-    localtime_r((const time_t *)_sec, );
+    localtime_r(_sec, );
  strftime(sn->name, sizeof(sn->name),
"vm-%Y%m%d%H%M%S", );
  }

but the qemu_timeval from "include/sysemu/os-win32.h" still
uses a long: is this file compiled for
win32?

Yep this fails for me when built with x86_64-w64-mingw32- (it's fine
with i686-w64-mingw32- )

We could just switch the code to use GDateTime from GLib and thus
avoid portability issues. I think this should be equivalent:

  g_autoptr(GDateTime) now = g_date_time_new_now_local();
  g_autofree char *nowstr = g_date_time_format(now,
"vm-%Y%m%d%H%M%s");
  strncpy(sn->name, sizeof(sn->name), nowstr);

Which way do you guys want to go? Something like above, remove the
comment
or some variation on the comment but not mentioning OpenBSD since it is
no
longer relevant?

Anyone?

Personally I always favour using GLib APIs if there's an applicable one,
since it eliminates portability problems - or rather offloads them to
the GLib maintainers, who have already solved them generally.


Can you please submit a proper diff and I'll drop this?

[PATCH v4] hw/smbios: support for type 41 (onboard devices extended information)

[Vincent Bernat]

Type 41 defines the attributes of devices that are onboard. The
original intent was to imply the BIOS had some level of control over
the enablement of the associated devices.

If network devices are present in this table, by default, udev will
name the corresponding interfaces enoX, X being the instance number.
Without such information, udev will fallback to using the PCI ID and
this usually gives ens3 or ens4. This can be a bit annoying as the
name of the network card may depend on the order of options and may
change if a new PCI device is added earlier on the commande line.
Being able to provide SMBIOS type 41 entry ensure the name of the
interface won't change and helps the user guess the right name without
booting a first time.

This can be invoked with:

$QEMU -netdev user,id=internet
  -device 
virtio-net-pci,mac=50:54:00:00:00:42,netdev=internet,id=internet-dev \
  -smbios type=41,designation='Onboard 
LAN',instance=1,kind=ethernet,pcidev=internet-dev

The PCI segment is assumed to be 0. This should hold true for most
cases.

$ dmidecode -t 41
# dmidecode 3.3
Getting SMBIOS data from sysfs.
SMBIOS 2.8 present.

Handle 0x2900, DMI type 41, 11 bytes
Onboard Device
Reference Designation: Onboard LAN
Type: Ethernet
Status: Enabled
Type Instance: 1
Bus Address: :00:09.0

$ ip -brief a
lo   UNKNOWN127.0.0.1/8 ::1/128
eno1 UP 10.0.2.14/24 fec0::5254:ff:fe00:42/64 
fe80::5254:ff:fe00:42/64

Signed-off-by: Vincent Bernat 
---
 hw/arm/virt.c|   7 +-
 hw/i386/fw_cfg.c |   4 +-
 hw/smbios/smbios.c   | 124 ++-
 include/hw/firmware/smbios.h |  14 +++-
 qemu-options.hx  |  30 -
 5 files changed, 173 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index aa2bbd14e090..840ec0af02db 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -53,6 +53,7 @@
 #include "sysemu/kvm.h"
 #include "hw/loader.h"
 #include "exec/address-spaces.h"
+#include "qapi/error.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
@@ -1524,8 +1525,10 @@ static void virt_build_smbios(VirtMachineState *vms)
 vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
 true, SMBIOS_ENTRY_POINT_30);
 
-smbios_get_tables(MACHINE(vms), NULL, 0, _tables, 
_tables_len,
-  _anchor, _anchor_len);
+smbios_get_tables(MACHINE(vms), NULL, 0,
+  _tables, _tables_len,
+  _anchor, _anchor_len,
+  _fatal);
 
 if (smbios_anchor) {
 fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-tables",
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index e48a54fa364b..4e68d5dea438 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -22,6 +22,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "e820_memory_layout.h"
 #include "kvm/kvm_i386.h"
+#include "qapi/error.h"
 #include CONFIG_DEVICES
 
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
@@ -78,7 +79,8 @@ void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
 }
 smbios_get_tables(ms, mem_array, array_count,
   _tables, _tables_len,
-  _anchor, _anchor_len);
+  _anchor, _anchor_len,
+  _fatal);
 g_free(mem_array);
 
 if (smbios_anchor) {
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index f22c4f5b734e..7397e567373b 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -27,6 +27,7 @@
 #include "hw/firmware/smbios.h"
 #include "hw/loader.h"
 #include "hw/boards.h"
+#include "hw/pci/pci_bus.h"
 #include "smbios_build.h"
 
 /* legacy structures and constants for <= 2.0 machines */
@@ -118,6 +119,28 @@ static struct {
 uint16_t speed;
 } type17;
 
+static QEnumLookup type41_kind_lookup = {
+.array = (const char *const[]) {
+"other",
+"unknown",
+"video",
+"scsi",
+"ethernet",
+"tokenring",
+"sound",
+"pata",
+"sata",
+"sas",
+},
+.size = 10
+};
+struct type41_instance {
+const char *designation, *pcidev;
+uint8_t instance, kind;
+QTAILQ_ENTRY(type41_instance) next;
+};
+static QTAILQ_HEAD(, type41_instance) type41 = QTAILQ_HEAD_INITIALIZER(type41);
+
 static QemuOptsList qemu_smbios_opts = {
 .name = "smbios",
 .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -358,6 +381,32 @@ static const QemuOptDesc qemu_smbios_type17_opts[] = {
 { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type41_opts[] = {
+{
+.name = "type",
+.type = QEMU_OPT_NUMBER,
+.help = "SMBIOS element type",
+},{
+.name = "designation",
+.type = QEMU_OPT_STRING,
+.help = "reference designation string",
+ 

Re: [PATCH 2/2] KVM: use KVM_{GET|SET}_SREGS2 when supported by kvm.

[Maxim Levitsky]

On Thu, 2021-04-01 at 18:09 +0200, Paolo Bonzini wrote:
> On 01/04/21 16:45, Maxim Levitsky wrote:
> > +
> > +for (i = 0; i < 4; i++) {
> > +sregs.pdptrs[i] = env->pdptrs[i];
> > +}
> > +
> > +sregs.flags = 0;
> > +sregs.padding = 0;
> > +
> > +return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS2, );
> > +}
> > +
> 
> This breaks when migrating from old to new kernel, because in that case 
> the PDPTRs are not initialized.

True, I haven't thought about it!
I'll fix this in the next version.

Best regards,
Maxim Levitsky


> 
> Paolo
> 

Re: [PATCH 1/2] iotests/231: Update expected deprecation message

[Connor Kuehl]

On 4/1/21 12:07 PM, Max Reitz wrote:

On 01.04.21 18:52, Max Reitz wrote:

On 01.04.21 17:52, Connor Kuehl wrote:

The deprecation message changed slightly at some point in the past but
the expected output wasn't updated along with it; causing it to fail.
Fix it, so it passes.

Signed-off-by: Connor Kuehl 
---
  tests/qemu-iotests/231.out | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)


Uh, well, you know what, I can’t find any version where there was any 
other output.  Even back in 66e6a735e97450ac50fcaf40f78600c688534cae, 
where this test was introduced, I get this diff.


What’s going on there?


Okay.  So:

Jeff’s original patch[1] included the “Future versions may cease to 
parse...” part.  v1 of his subsequent pull request[2] did, too.  But 
v2[3] didn’t.  Looks like Markus made a comment on v4 of the patch, and 
then Jeff fixed up the patch in his branch, but didn’t change the test. 
  In any case it’s clear that the reference output was wrong all along.


About the “no monitors specified” part...  The only place where I can 
find “no monitors” is in Jeff’s patches to add this iotest.  I have no 
idea where that orignated from.


So:

Reviewed-by: Max Reitz 


Thanks! And excellent sleuthing. This accidental conspiracy went much 
farther beyond the git log than I thought...


Connor




[1]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00282.html

[2]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00307.html

[3]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00592.html

Re: [PATCH 1/2] iotests/231: Update expected deprecation message

[Max Reitz]

On 01.04.21 18:52, Max Reitz wrote:

On 01.04.21 17:52, Connor Kuehl wrote:

The deprecation message changed slightly at some point in the past but
the expected output wasn't updated along with it; causing it to fail.
Fix it, so it passes.

Signed-off-by: Connor Kuehl 
---
  tests/qemu-iotests/231.out | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)


Uh, well, you know what, I can’t find any version where there was any 
other output.  Even back in 66e6a735e97450ac50fcaf40f78600c688534cae, 
where this test was introduced, I get this diff.


What’s going on there?


Okay.  So:

Jeff’s original patch[1] included the “Future versions may cease to 
parse...” part.  v1 of his subsequent pull request[2] did, too.  But 
v2[3] didn’t.  Looks like Markus made a comment on v4 of the patch, and 
then Jeff fixed up the patch in his branch, but didn’t change the test. 
 In any case it’s clear that the reference output was wrong all along.


About the “no monitors specified” part...  The only place where I can 
find “no monitors” is in Jeff’s patches to add this iotest.  I have no 
idea where that orignated from.


So:

Reviewed-by: Max Reitz 


[1]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00282.html

[2]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00307.html

[3]
https://lists.nongnu.org/archive/html/qemu-block/2018-09/msg00592.html

Re: [PATCH v3 00/11] esp: fix asserts/segfaults discovered by fuzzer

[Alexander Bulekov]

On 210401 0849, Mark Cave-Ayland wrote:
> Recently there have been a number of issues raised on Launchpad as a result of
> fuzzing the am53c974 (ESP) device. I spent some time over the past couple of
> days checking to see if anything had improved since my last patchset: from
> what I can tell the issues are still present, but the cmdfifo related failures
> now assert rather than corrupting memory.
> 
> This patchset applied to master passes my local tests using the qtest fuzz 
> test
> cases added by Alexander for the following Launchpad bugs:
> 
>   https://bugs.launchpad.net/qemu/+bug/1919035
>   https://bugs.launchpad.net/qemu/+bug/1919036
>   https://bugs.launchpad.net/qemu/+bug/1910723
>   https://bugs.launchpad.net/qemu/+bug/1909247
>   
> I'm posting this now just before soft freeze since I see that some of the 
> issues
> have recently been allocated CVEs and so it could be argued that even though
> they have existed for some time, it is worth fixing them for 6.0.
> 
> Signed-off-by: Mark Cave-Ayland 
> 
> v3:
> - Rebase onto master
> - Rearrange patch ordering (move patch 5 to the front) to help reduce 
> cross-talk
>   between the regression tests
> - Introduce patch 2 to remove unnecessary FIFO usage
> - Introduce patches 3-4 to consolidate esp_fifo_pop()/esp_fifo_push() wrapper
>   functions to avoid having to introduce 2 variants of esp_fifo_pop_buf()
> - Introduce esp_fifo_pop_buf() in patch 5 to prevent callers from overflowing
>   the array used to model the FIFO
> - Introduce patch 10 to clarify cancellation logic should all occur in the 
> .cancel
>   SCSI callback rather than at the site of the caller
> - Add extra qtests in patch 11 to cover addition test cases provided on LP
> 

Hi Mark,
I applied this and ran through the whole fuzzer corpus, and all I'm
seeing are just a few assertion failures:
handle_satn_stop -> get_cmd -> util/fifo8.c:43:5 and
hw/scsi/esp.c:790:5

Tested-by: Alexander Bulekov 

Thank you
-Alex

> v2:
> - Add Alexander's R-B tag for patch 2 and Phil's R-B for patch 3
> - Add patch 4 for additional testcase provided in Alexander's patch 1 comment
> - Move current_req NULL checks forward in DMA functions (fixes ASAN bug 
> reported
>   at https://bugs.launchpad.net/qemu/+bug/1909247/comments/6) in patch 3
> - Add qtest for am53c974 containing a basic set of regression tests using the
>   automatic test cases generated by the fuzzer as requested by Paolo
> 
> 
> Mark Cave-Ayland (11):
>   esp: always check current_req is not NULL before use in DMA callbacks
>   esp: rework write_response() to avoid using the FIFO for DMA
> transactions
>   esp: consolidate esp_cmdfifo_push() into esp_fifo_push()
>   esp: consolidate esp_cmdfifo_pop() into esp_fifo_pop()
>   esp: introduce esp_fifo_pop_buf() and use it instead of
> fifo8_pop_buf()
>   esp: ensure cmdfifo is not empty and current_dev is non-NULL
>   esp: don't underflow cmdfifo in do_cmd()
>   esp: don't overflow cmdfifo in get_cmd()
>   esp: don't overflow cmdfifo if TC is larger than the cmdfifo size
>   esp: don't reset async_len directly in esp_select() if cancelling
> request
>   tests/qtest: add tests for am53c974 device
> 
>  MAINTAINERS |   1 +
>  hw/scsi/esp.c   | 116 ++-
>  tests/qtest/am53c974-test.c | 216 
>  tests/qtest/meson.build |   1 +
>  4 files changed, 282 insertions(+), 52 deletions(-)
>  create mode 100644 tests/qtest/am53c974-test.c
> 
> -- 
> 2.20.1
> 

Re: [PATCH v3 11/11] tests/qtest: add tests for am53c974 device

[Alexander Bulekov]

On 210401 0849, Mark Cave-Ayland wrote:
> Use the autogenerated fuzzer test cases as the basis for a set of am53c974
> regression tests.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  MAINTAINERS |   1 +
>  tests/qtest/am53c974-test.c | 216 
>  tests/qtest/meson.build |   1 +
>  3 files changed, 218 insertions(+)
>  create mode 100644 tests/qtest/am53c974-test.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 554be84b32..675f35d3af 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1776,6 +1776,7 @@ F: include/hw/scsi/*
>  F: hw/scsi/*
>  F: tests/qtest/virtio-scsi-test.c
>  F: tests/qtest/fuzz-virtio-scsi-test.c
> +F: tests/qtest/am53c974-test.c
>  T: git https://github.com/bonzini/qemu.git scsi-next
>  
>  SSI
> diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
> new file mode 100644
> index 00..9c4285d0c0
> --- /dev/null
> +++ b/tests/qtest/am53c974-test.c
> @@ -0,0 +1,216 @@
> +/*
> + * QTest testcase for am53c974
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "libqos/libqtest.h"
> +
> +
> +static void test_cmdfifo_underflow_ok(void)
> +{
> +QTestState *s = qtest_init(
> +"-device am53c974,id=scsi "
> +"-device scsi-hd,drive=disk0 -drive "
> +"id=disk0,if=none,file=null-co://,format=raw -nodefaults");
> +qtest_outl(s, 0xcf8, 0x80001004);
> +qtest_outw(s, 0xcfc, 0x01);
> +qtest_outl(s, 0xcf8, 0x8000100e);
> +qtest_outl(s, 0xcfc, 0x8a00);
> +qtest_outl(s, 0x8a09, 0x4200);
> +qtest_outl(s, 0x8a0d, 0x00);
> +qtest_outl(s, 0x8a0b, 0x1000);
> +qtest_quit(s);
> +}
> +

Hi Mark,
> +/* Reported as crash_1548bd10e7 */
^^^
These numbers were just the filename/hash of the crashing test-case. I'm
not sure if they are useful to keep them around - I just needed some way
to name a bunch of functions :)
-Alex

Re: [PATCH 1/2] iotests/231: Update expected deprecation message

[Max Reitz]

On 01.04.21 17:52, Connor Kuehl wrote:

The deprecation message changed slightly at some point in the past but
the expected output wasn't updated along with it; causing it to fail.
Fix it, so it passes.

Signed-off-by: Connor Kuehl 
---
  tests/qemu-iotests/231.out | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)


Uh, well, you know what, I can’t find any version where there was any 
other output.  Even back in 66e6a735e97450ac50fcaf40f78600c688534cae, 
where this test was introduced, I get this diff.


What’s going on there?

Max

Re: [PATCH v2 2/2] hw/smbios: retrieve PCI address from specified device for Type 41

[Michael S. Tsirkin]

On Thu, Apr 01, 2021 at 04:26:50PM +0200, Vincent Bernat wrote:
>  ❦  1 avril 2021 09:59 -04, Michael S. Tsirkin:
> 
> >> +/*
> >> + * TODO: Extract the appropriate value. Most of the
> >> + * time, this will be 0.
> >> + */
> >> +t->segment_group_number = cpu_to_le16(0);
> >> +t->bus_number = pci_dev_bus_num(pdev);
> >> +t->device_number = pdev->devfn;
> >
> > Problem is, for devices behind bridges for example, bus is only
> > configured by guest, after pci has been enumerated.
> >
> > So I suspect this either
> > - needs to be limited to only work for the root bus
> > - needs to be re-evaluted on guest access, like we do
> >   with ACPI
> 
> Or the address can be provided by the user. I didn't want to keep that
> at this is error prone and there may be surprises after adding a device
> or after a QEMU upgrade.

Or on guest changes.

> 
> Otherwise, limiting to the root bus seems a fine limitation by me. How
> do I check that?

pci_bus_is_root will do this. Pls document the reason for the
limitation.


> -- 
> Don't just echo the code with comments - make every comment count.
> - The Elements of Programming Style (Kernighan & Plauger)

[PATCH v2] iotests: add test for removing persistent bitmap from backing file

[Vladimir Sementsov-Ogievskiy]

Just demonstrate one of x-blockdev-reopen usecases. We can't simply
remove persistent bitmap from RO node (for example from backing file),
as we need to remove it from the image too. So, we should reopen the
node first.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

v2: [by Max's review]
 - more imports for convenience
 - assert that qemu_img_create and qemu_img succeeds
 - add -F argument to qemu_img_create
 - fix error message

 .../tests/remove-bitmap-from-backing  | 69 +++
 .../tests/remove-bitmap-from-backing.out  |  6 ++
 2 files changed, 75 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/remove-bitmap-from-backing
 create mode 100644 tests/qemu-iotests/tests/remove-bitmap-from-backing.out

diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing 
b/tests/qemu-iotests/tests/remove-bitmap-from-backing
new file mode 100755
index 00..0ea4c36507
--- /dev/null
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing
@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+#
+# Test removing persistent bitmap from backing
+#
+# Copyright (c) 2021 Virtuozzo International GmbH.
+#
+# 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 .
+#
+
+import iotests
+from iotests import log, qemu_img_create, qemu_img, qemu_img_pipe
+
+iotests.script_initialize(supported_fmts=['qcow2'])
+
+top, base = iotests.file_path('top', 'base')
+size = '1M'
+
+assert qemu_img_create('-f', iotests.imgfmt, base, size) == 0
+assert qemu_img_create('-f', iotests.imgfmt, '-b', base,
+   '-F', iotests.imgfmt, top, size) == 0
+
+assert qemu_img('bitmap', '--add', base, 'bitmap0') == 0
+# Just assert that our method of checking bitmaps in the image works.
+assert 'bitmaps' in qemu_img_pipe('info', base)
+
+vm = iotests.VM().add_drive(top, 'backing.node-name=base')
+vm.launch()
+
+log('Trying to remove persistent bitmap from r-o base node, should fail:')
+vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0')
+
+new_base_opts = {
+'node-name': 'base',
+'driver': 'qcow2',
+'file': {
+'driver': 'file',
+'filename':  base
+},
+'read-only': False
+}
+
+# Don't want to bother with filtering qmp_log for reopen command
+result = vm.qmp('x-blockdev-reopen', **new_base_opts)
+if result != {'return': {}}:
+log('Failed to reopen: ' + str(result))
+
+log('Remove persistent bitmap from base node reopened to RW:')
+vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0')
+
+new_base_opts['read-only'] = True
+result = vm.qmp('x-blockdev-reopen', **new_base_opts)
+if result != {'return': {}}:
+log('Failed to reopen: ' + str(result))
+
+vm.shutdown()
+
+if 'bitmaps' in qemu_img_pipe('info', base):
+log('ERROR: Bitmap is still in the base image')
diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing.out 
b/tests/qemu-iotests/tests/remove-bitmap-from-backing.out
new file mode 100644
index 00..c28af82c75
--- /dev/null
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing.out
@@ -0,0 +1,6 @@
+Trying to remove persistent bitmap from r-o base node, should fail:
+{"execute": "block-dirty-bitmap-remove", "arguments": {"name": "bitmap0", 
"node": "base"}}
+{"error": {"class": "GenericError", "desc": "Bitmap 'bitmap0' is readonly and 
cannot be modified"}}
+Remove persistent bitmap from base node reopened to RW:
+{"execute": "block-dirty-bitmap-remove", "arguments": {"name": "bitmap0", 
"node": "base"}}
+{"return": {}}
-- 
2.29.2

Re: [PATCH] iotests/qsd-jobs: Filter events in the first test

[Vladimir Sementsov-Ogievskiy]

01.04.2021 18:30, Max Reitz wrote:

On 01.04.21 16:37, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 17:37, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 16:28, Max Reitz wrote:

The job may or may not be ready before the 'quit' is issued.  Whether it
is is irrelevant; for the purpose of the test, it only needs to still be
there.  Filter the job status change and READY events from the output so
it becomes reliable.

Reported-by: Peter Maydell
Suggested-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy



Hmm, in your mail (and in copypasted by r-b) there is no space between name and 
"<"..


Are you sure?  I can see the spaces in my original mail, and on
https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg00182.html

But OTOH I don’t see a space between your name and the < in your R-b...

Max



Ah yes. It's someone (possibly my thunderbird) eats spaces. I see them in you 
original mail, but not in quote in my first answer

--
Best regards,
Vladimir

Re: [PATCH 2/2] KVM: use KVM_{GET|SET}_SREGS2 when supported by kvm.

[Paolo Bonzini]

On 01/04/21 16:45, Maxim Levitsky wrote:

+
+for (i = 0; i < 4; i++) {
+sregs.pdptrs[i] = env->pdptrs[i];
+}
+
+sregs.flags = 0;
+sregs.padding = 0;
+
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS2, );
+}
+


This breaks when migrating from old to new kernel, because in that case 
the PDPTRs are not initialized.


Paolo

Re: [PULL 00/12] Misc patches for QEMU 6.0-rc2

[Peter Maydell]

On Thu, 1 Apr 2021 at 12:25, Paolo Bonzini  wrote:
>
> The following changes since commit 6ee55e1d10c25c2f6bf5ce2084ad2327e17affa5:
>
>   Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.0-20210331' 
> into staging (2021-03-31 13:14:18 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 7dd6acc82fa6fbc2ff99f173635ac6be0c72b1eb:
>
>   docs: Add a QEMU Code of Conduct and Conflict Resolution Policy document 
> (2021-04-01 10:37:20 +0200)
>
> 
> * Bugfixes
> * Code of conduct and conflict resolution policy
>



Applied, thanks.

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

-- PMM

Re: [PATCH for-6.0] hw/timer/renesas_tmr: Add default-case asserts in read_tcnt()

[Yoshinori Sato]

On Sat, 20 Mar 2021 01:24:58 +0900,
Peter Maydell wrote:
> 
> In commit 81b3ddaf8772ec we fixed a use of uninitialized data
> in read_tcnt(). However this change wasn't enough to placate
> Coverity, which is not smart enough to see that if we read a
> 2 bit field and then handle cases 0, 1, 2 and 3 then there cannot
> be a flow of execution through the switch default. Add explicit
> default cases which assert that they can't be reached, which
> should help silence Coverity.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Yoshinori Sato 

> ---
>  hw/timer/renesas_tmr.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c
> index eed39917fec..d96002e1ee6 100644
> --- a/hw/timer/renesas_tmr.c
> +++ b/hw/timer/renesas_tmr.c
> @@ -146,6 +146,8 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, 
> int ch)
>  case CSS_CASCADING:
>  tcnt[1] = tmr->tcnt[1];
>  break;
> +default:
> +g_assert_not_reached();
>  }
>  switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) {
>  case CSS_INTERNAL:
> @@ -159,6 +161,8 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, 
> int ch)
>  case CSS_EXTERNAL: /* QEMU doesn't implement this */
>  tcnt[0] = tmr->tcnt[0];
>  break;
> +default:
> +g_assert_not_reached();
>  }
>  } else {
>  tcnt[0] = tmr->tcnt[0];
> -- 
> 2.20.1
> 

I'm back.
---
Yoshinori Sato

Re: [PATCH v3 2/2] block: Add backend_defaults property

[Stefan Hajnoczi]

On Thu, Mar 11, 2021 at 12:39:16AM +0900, Akihiko Odaki wrote:
> backend_defaults property allow users to control if default block
> properties should be decided with backend information.
> 
> If it is off, any backend information will be discarded, which is
> suitable if you plan to perform live migration to a different disk backend.
> 
> If it is on, a block device may utilize backend information more
> aggressively.
> 
> By default, it is auto, which uses backend information from physical
> devices and ignores one from file. It is consistent with the older
> versions, and should be aligned with the user expectation that a file
> backend is more independent of host than a physical device backend.

Can BlockLimits pdiscard_alignment and opt_transfer be used instead of
adding discard_granularity and opt_io fields to BlockSizes? They seem to
do the same thing except the QEMU block layer currently uses BlockLimits
values internally to ensure that requests are suitable for the host
device.


signature.asc
Description: PGP signature

Re: [PATCH 0/2] i386: Fix interrupt based Async PF enablement

[Paolo Bonzini]

On 01/04/21 17:19, Vitaly Kuznetsov wrote:

I noticed two issues with 'kvm-asyncpf-int' enablement:
1) We forgot to add to to kvm_default_props[] so it doesn't get enabled
  automatically (unless '-cpu host' is used or the feature is enabled
  manually on the command line)
2) We forgot to disable it for older machine types to preserve migration.
  This went unnoticed because of 1) I believe.

Vitaly Kuznetsov (2):
   i386: Add 'kvm-asyncpf-int' to kvm_default_props array
   i386: Disable 'kvm-asyncpf-int' feature for machine types <= 5.1

  hw/i386/pc.c  | 1 +
  target/i386/cpu.c | 1 +
  2 files changed, 2 insertions(+)



Wasn't this intentional to avoid requiring a new kernel version?

Paolo

[PATCH 1/2] iotests/231: Update expected deprecation message

[Connor Kuehl]

The deprecation message changed slightly at some point in the past but
the expected output wasn't updated along with it; causing it to fail.
Fix it, so it passes.

Signed-off-by: Connor Kuehl 
---
 tests/qemu-iotests/231.out | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 579ba11c16..747dd221bb 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -1,9 +1,7 @@
 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
+qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is 
deprecated
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
 *** done
-- 
2.30.2

[PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

[Connor Kuehl]

That's qemu_rbd_unescape()'s job! No need to duplicate the labor.

Furthermore, this was causing some confusion in the parsing logic to
where the caller might test for the presence of a character to split on
like so:

if (strchr(image_name, '/')) {
found_str = qemu_rbd_next_tok(image_name, '/', _name);
[..]

When qemu_rbd_next_tok() performs unescaping as a side effect, the
parser can get confused thinking that it can split this string, but
really the delimiter '/' gets unescaped and so qemu_rbd_next_tok() never
"finds" the delimiter and consumes the rest of the token input stream.

This is problematic because qemu_rbd_next_tok() also steals the input
pointer to the token stream and sets it to NULL. This causes a segfault
where the parser expects to split one string into two.

In this case, the parser is determining if a string is a
namespace/image_name pair like so:

"foo/bar"

And clearly it's looking to split it like so:

namespace:  foo
image_name: bar

but if the input is "foo\/bar", it *should* split into

namespace:  foo\
image_name: bar

and its subordinate parts can be unescaped after tokenization.

So, instead of tokenizing *and* escaping all at once, do one before the
other to avoid stumbling into a segfault by confusing the parsing logic.

Reported-by: Han Han 
Fixes: https://bugzilla.redhat.com/1873913
Signed-off-by: Connor Kuehl 
---
 block/rbd.c| 3 ---
 tests/qemu-iotests/231 | 4 
 tests/qemu-iotests/231.out | 3 +++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9071a00e3f..9bed0863e5 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -123,9 +123,6 @@ static char *qemu_rbd_next_tok(char *src, char delim, char 
**p)
 if (*end == delim) {
 break;
 }
-if (*end == '\\' && end[1] != '\0') {
-end++;
-}
 }
 if (*end == delim) {
 *p = end + 1;
diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
index 0f66d0ca36..8e6c6447c1 100755
--- a/tests/qemu-iotests/231
+++ b/tests/qemu-iotests/231
@@ -55,6 +55,10 @@ _filter_conf()
 $QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
 $QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
 
+# Regression test: the qemu-img invocation is expected to fail, but it should
+# not seg fault the parser.
+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 747dd221bb..a785a6e859 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: 
ceph-mon
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
+unable to get monitor info from DNS SRV with service name: ceph-mon
+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or 
directory
 *** done
-- 
2.30.2

[PATCH 0/2] Fix segfault in qemu_rbd_parse_filename

[Connor Kuehl]

Connor Kuehl (2):
  iotests/231: Update expected deprecation message
  block/rbd: Don't unescape in qemu_rbd_next_tok()

 block/rbd.c| 3 ---
 tests/qemu-iotests/231 | 4 
 tests/qemu-iotests/231.out | 7 ---
 3 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.30.2

Re: [Virtio-fs] [PATCH] virtiofsd: Fix security.capability comparison

[Connor Kuehl]

On 4/1/21 9:58 AM, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

My security fix for the security.capability remap has a silly early
segfault in a simple case where there is an xattrmapping but it doesn't
remap the securty.capability.


s/securty/security



Fixes: e586edcb41054 ("virtiofs: drop remapped security.capability xattr as 
needed")
Signed-off-by: Dr. David Alan Gilbert 


Reviewed-by: Connor Kuehl 


---
  tools/virtiofsd/passthrough_ll.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index b144320e48..1553d2ef45 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2636,7 +2636,8 @@ static void parse_xattrmap(struct lo_data *lo)
  strerror(ret));
  exit(1);
  }
-if (!strcmp(lo->xattr_security_capability, "security.capability")) {
+if (!lo->xattr_security_capability ||
+!strcmp(lo->xattr_security_capability, "security.capability")) {
  /* 1-1 mapping, don't need to do anything */
  free(lo->xattr_security_capability);
  lo->xattr_security_capability = NULL;

Re: [PATCH v3 1/2] block/file-posix: Optimize for macOS

[Stefan Hajnoczi]

On Thu, Mar 11, 2021 at 12:39:15AM +0900, Akihiko Odaki wrote:
> @@ -1586,6 +1589,7 @@ out:
>  }
>  }
>  
> +G_GNUC_UNUSED

Why isn't translate_err() used in the F_PUNCHHOLE case below?

If you really want to avoid using it on macOS, please add a #if with the
necessary conditions here so it's clear when this translate_err() is
needed.

> @@ -514,7 +515,8 @@ static int raw_probe_blocksizes(BlockDriverState *bs, 
> BlockSizes *bsz)
>  return ret;
>  }
>  
> -if (!QEMU_IS_ALIGNED(s->offset, MAX(bsz->log, bsz->phys))) {
> +size = MAX(bsz->log, bsz->phys);
> +if (size && !QEMU_IS_ALIGNED(s->offset, size)) {
>  return -ENOTSUP;
>  }
>  

This patch changes the semantics of bdrv_probe_blocksizes(). It used to
return -ENOTSUP when phys/log weren't available. Now it returns 0 and
the fields are 0. Please update the bdrv_probe_blocksizes doc comment in
include/block/block_int.h to mention phys and log, as well as that
fields can be set to 0 (or -1 in the case of discard_granularity).


signature.asc
Description: PGP signature

Re: [PATCH] iotests/qsd-jobs: Filter events in the first test

[Max Reitz]

On 01.04.21 16:37, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 17:37, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 16:28, Max Reitz wrote:

The job may or may not be ready before the 'quit' is issued.  Whether it
is is irrelevant; for the purpose of the test, it only needs to still be
there.  Filter the job status change and READY events from the output so
it becomes reliable.

Reported-by: Peter Maydell
Suggested-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy



Hmm, in your mail (and in copypasted by r-b) there is no space between 
name and "<"..


Are you sure?  I can see the spaces in my original mail, and on
https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg00182.html

But OTOH I don’t see a space between your name and the < in your R-b...

Max

Re: [PATCH 2/2] iotests/qsd-jobs: Use common.qemu for the QSD

[Max Reitz]

On 01.04.21 16:44, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 16:28, Max Reitz wrote:

Using common.qemu allows us to wait for specific replies, so we can for
example wait for events.  This allows starting the active commit job and
then wait for it to be ready before quitting the QSD, so we the output
is always the same.

(Strictly speaking, this is only necessary for the first test in
qsd-jobs, but we might as well make the second one use common.qemu's
infrastructure, too.)



Reviewed-by: Vladimir Sementsov-Ogievskiy 


Reported-by: Peter Maydell 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/tests/qsd-jobs | 55 ---
  tests/qemu-iotests/tests/qsd-jobs.out | 10 -
  2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/tests/qsd-jobs 
b/tests/qemu-iotests/tests/qsd-jobs

index 972b6b3898..af7f886f15 100755
--- a/tests/qemu-iotests/tests/qsd-jobs
+++ b/tests/qemu-iotests/tests/qsd-jobs
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  cd ..
  . ./common.rc
  . ./common.filter
+. ./common.qemu
  _supported_fmt qcow2
  _supported_proto generic
@@ -52,32 +53,58 @@ echo "=== Job still present at shutdown ==="
  echo
  # Just make sure that this doesn't crash
-$QSD --chardev stdio,id=stdio --monitor chardev=stdio \
+qsd=y _launch_qemu \


sounds a bit strange.. Like we are starting qemu.


Yeah, well.  Yeah.  We could have a

_launch_qsd()
{
qsd=y _launch_qemu
}

But this would still make it weird for all the other commands from 
common.qemu, and I don’t think it makes much sense to introduce aliases 
for all of them.  So I think it’d be best to live with that bit of 
weirdness.


Max

[Bug 1880518] Re: issue while installing docker inside s390x container

[Laurent Vivier]

insmod/modprobe inside the container cannot load the modules because
they are the one for the target architecture while the kernel is the one
of the host. If you need functionalities in the container provided by
some modules you need to load these modules using modprobe/insmod on the
host.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1880518

Title:
  issue while installing docker inside s390x container

Status in QEMU:
  New

Bug description:
  This is in reference to 
https://github.com/multiarch/qemu-user-static/issues/108.
  I am facing issue while installing docker inside s390x container under qemu 
on Ubuntu 18.04 host running on amd64.
  Following are the contents of /proc/sys/fs/binfmt_misc/qemu-s390x on Intel 
host after running 
  docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
  enabled
  interpreter /usr/bin/qemu-s390x-static
  flags: F
  offset 0
  magic 7f454c46020201020016
  mask ff00fffe
  I could get docker service up with the following trick. 
  printf '{"iptables": false,"ip-masq": false,"bridge": "none" }' > 
/etc/docker/daemon.json
  But even though docker service comes up, images are not getting pulled, 
docker pull fails with the following error.
  failed to register layer: Error processing tar file(exit status 1):

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1880518/+subscriptions

[RFC PATCH] block: always update auto_backing_file to full path

[Joe Jin]

Some time after created snapshot, auto_backing_file only has filename,
this confused overridden check, update it to full path if it is not.

Signed-off-by: Joe Jin 
---
 block.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/block.c b/block.c
index c5b887cec1..8f9a027dee 100644
--- a/block.c
+++ b/block.c
@@ -6969,6 +6969,19 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 return;
 }
 
+    /* auto_backing_file only has filename, update it to the full path */
+    if (bs->backing && bs->auto_backing_file[0] != '/') {
+    char *backing_filename = NULL;
+    Error *local_err = NULL;
+
+    backing_filename = bdrv_make_absolute_filename(bs,
+ bs->auto_backing_file, _err);
+    if (!local_err && backing_filename) {
+    pstrcpy(bs->auto_backing_file, sizeof(bs->auto_backing_file),
+ backing_filename);
+    g_free(backing_filename);
+    }
+    }
 backing_overridden = bdrv_backing_overridden(bs);
 
 if (bs->open_flags & BDRV_O_NO_IO) {
-- 
2.21.1 (Apple Git-122.3)

[PATCH 1/2] i386: Add 'kvm-asyncpf-int' to kvm_default_props array

[Vitaly Kuznetsov]

Just like all other KVM PV features, 'kvm-asyncpf-int' needs to be
added to all CPU models when KVM is enabled or the feature will always
remain 'off' unless specified explicitly on the command line.

Signed-off-by: Vitaly Kuznetsov 
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6b3e9467f177..c7f8a8a8fec0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4244,6 +4244,7 @@ static PropValue kvm_default_props[] = {
 { "kvmclock", "on" },
 { "kvm-nopiodelay", "on" },
 { "kvm-asyncpf", "on" },
+{ "kvm-asyncpf-int", "on" },
 { "kvm-steal-time", "on" },
 { "kvm-pv-eoi", "on" },
 { "kvmclock-stable-bit", "on" },
-- 
2.30.2

[PATCH 2/2] i386: Disable 'kvm-asyncpf-int' feature for machine types <= 5.1

[Vitaly Kuznetsov]

'kvm-asyncpf-int' was implemented in QEMU-5.2 so older machine types
should have it disabled to make migration to an older QEMU which does not
support this feature possible.

The issue went unnoticed probably because we also forgot to add
'kvm-asyncpf-int' to 'kvm_default_props[]' so it was rarely enabled.

Fixes: db5daafab2 ("target/i386: support KVM_FEATURE_ASYNC_PF_INT")
Signed-off-by: Vitaly Kuznetsov 
---
 hw/i386/pc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8a84b25a031e..04d5f76bf133 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -103,6 +103,7 @@ const size_t pc_compat_5_2_len = 
G_N_ELEMENTS(pc_compat_5_2);
 
 GlobalProperty pc_compat_5_1[] = {
 { "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
+{ TYPE_X86_CPU, "kvm-asyncpf-int", "off" },
 { TYPE_X86_CPU, "kvm-msi-ext-dest-id", "off" },
 };
 const size_t pc_compat_5_1_len = G_N_ELEMENTS(pc_compat_5_1);
-- 
2.30.2

[PATCH v2 5/5] target/riscv: Use RISCVException enum for CSR access

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h   | 11 +++
 target/riscv/csr.c   | 37 ++---
 target/riscv/gdbstub.c   |  8 
 target/riscv/op_helper.c | 18 +-
 4 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7b9b9da6b7..d3df8b9292 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -455,10 +455,13 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
*env, target_ulong *pc,
 *pflags = flags;
 }
 
-int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
-target_ulong new_value, target_ulong write_mask);
-int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
-  target_ulong new_value, target_ulong write_mask);
+RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
+   target_ulong *ret_value,
+   target_ulong new_value, target_ulong write_mask);
+RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno,
+ target_ulong *ret_value,
+ target_ulong new_value,
+ target_ulong write_mask);
 
 static inline void riscv_csr_write(CPURISCVState *env, int csrno,
target_ulong val)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9df65a609c..459db93c83 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1404,10 +1404,11 @@ static RISCVException write_pmpaddr(CPURISCVState *env, 
int csrno,
  * csrrc  <->  riscv_csrrw(env, csrno, ret_value, 0, value);
  */
 
-int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
-target_ulong new_value, target_ulong write_mask)
+RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
+   target_ulong *ret_value,
+   target_ulong new_value, target_ulong write_mask)
 {
-int ret;
+RISCVException ret;
 target_ulong old_value;
 RISCVCPU *cpu = env_archcpu(env);
 
@@ -1429,41 +1430,37 @@ int riscv_csrrw(CPURISCVState *env, int csrno, 
target_ulong *ret_value,
 
 if ((write_mask && read_only) ||
 (!env->debugger && (effective_priv < get_field(csrno, 0x300 {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
 
 /* ensure the CSR extension is enabled. */
 if (!cpu->cfg.ext_icsr) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 
 /* check predicate */
 if (!csr_ops[csrno].predicate) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 ret = csr_ops[csrno].predicate(env, csrno);
 if (ret != RISCV_EXCP_NONE) {
-return -ret;
+return ret;
 }
 
 /* execute combined read/write operation if it exists */
 if (csr_ops[csrno].op) {
-ret = csr_ops[csrno].op(env, csrno, ret_value, new_value, write_mask);
-if (ret != RISCV_EXCP_NONE) {
-return -ret;
-}
-return 0;
+return csr_ops[csrno].op(env, csrno, ret_value, new_value, write_mask);
 }
 
 /* if no accessor exists then return failure */
 if (!csr_ops[csrno].read) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 /* read old value */
 ret = csr_ops[csrno].read(env, csrno, _value);
 if (ret != RISCV_EXCP_NONE) {
-return -ret;
+return ret;
 }
 
 /* write value if writable and write mask set, otherwise drop writes */
@@ -1472,7 +1469,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, 
target_ulong *ret_value,
 if (csr_ops[csrno].write) {
 ret = csr_ops[csrno].write(env, csrno, new_value);
 if (ret != RISCV_EXCP_NONE) {
-return -ret;
+return ret;
 }
 }
 }
@@ -1482,17 +1479,19 @@ int riscv_csrrw(CPURISCVState *env, int csrno, 
target_ulong *ret_value,
 *ret_value = old_value;
 }
 
-return 0;
+return RISCV_EXCP_NONE;
 }
 
 /*
  * Debugger support.  If not in user mode, set env->debugger before the
  * riscv_csrrw call and clear it after the call.
  */
-int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
-target_ulong new_value, target_ulong write_mask)
+RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno,
+ target_ulong *ret_value,
+ target_ulong new_value,
+ target_ulong write_mask)
 {
-int ret;
+RISCVException ret;
 #if !defined(CONFIG_USER_ONLY)
 env->debugger = true;
 #endif
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 5f96b7ea2a..ca78682cf4 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ 

[PATCH 0/2] i386: Fix interrupt based Async PF enablement

[Vitaly Kuznetsov]

I noticed two issues with 'kvm-asyncpf-int' enablement:
1) We forgot to add to to kvm_default_props[] so it doesn't get enabled
 automatically (unless '-cpu host' is used or the feature is enabled
 manually on the command line)
2) We forgot to disable it for older machine types to preserve migration.
 This went unnoticed because of 1) I believe.

Vitaly Kuznetsov (2):
  i386: Add 'kvm-asyncpf-int' to kvm_default_props array
  i386: Disable 'kvm-asyncpf-int' feature for machine types <= 5.1

 hw/i386/pc.c  | 1 +
 target/i386/cpu.c | 1 +
 2 files changed, 2 insertions(+)

-- 
2.30.2

[PATCH v2 0/5] RISC-V: Convert the CSR access functions to use

[Alistair Francis]

V2:
 - Renmae the enum
 - Rebase on master
 - Fix a few incorrect returns

Alistair Francis (5):
  target/riscv: Convert the RISC-V exceptions to an enum
  target/riscv: Use the RISCVException enum for CSR predicates
  target/riscv: Fix 32-bit HS mode access permissions
  target/riscv: Use the RISCVException enum for CSR operations
  target/riscv: Use RISCVException enum for CSR access

 target/riscv/cpu.h|  28 +-
 target/riscv/cpu_bits.h   |  44 +--
 target/riscv/cpu.c|   2 +-
 target/riscv/cpu_helper.c |   4 +-
 target/riscv/csr.c| 740 ++
 target/riscv/gdbstub.c|   8 +-
 target/riscv/op_helper.c  |  18 +-
 7 files changed, 492 insertions(+), 352 deletions(-)

-- 
2.31.0

[PATCH v2 2/5] target/riscv: Use the RISCVException enum for CSR predicates

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h |  3 +-
 target/riscv/csr.c | 80 +-
 2 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0a33d387ba..1291ddc381 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -473,7 +473,8 @@ static inline target_ulong riscv_csr_read(CPURISCVState 
*env, int csrno)
 return val;
 }
 
-typedef int (*riscv_csr_predicate_fn)(CPURISCVState *env, int csrno);
+typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env,
+ int csrno);
 typedef int (*riscv_csr_read_fn)(CPURISCVState *env, int csrno,
 target_ulong *ret_value);
 typedef int (*riscv_csr_write_fn)(CPURISCVState *env, int csrno,
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..5dc2aa9845 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -35,29 +35,29 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
-static int fs(CPURISCVState *env, int csrno)
+static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
 /* loose check condition for fcsr in vector extension */
 if ((csrno == CSR_FCSR) && (env->misa & RVV)) {
-return 0;
+return RISCV_EXCP_NONE;
 }
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int vs(CPURISCVState *env, int csrno)
+static RISCVException vs(CPURISCVState *env, int csrno)
 {
 if (env->misa & RVV) {
-return 0;
+return RISCV_EXCP_NONE;
 }
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 
-static int ctr(CPURISCVState *env, int csrno)
+static RISCVException ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
 CPUState *cs = env_cpu(env);
@@ -65,7 +65,7 @@ static int ctr(CPURISCVState *env, int csrno)
 
 if (!cpu->cfg.ext_counters) {
 /* The Counters extensions is not enabled */
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 
 if (riscv_cpu_virt_enabled(env)) {
@@ -73,25 +73,25 @@ static int ctr(CPURISCVState *env, int csrno)
 case CSR_CYCLE:
 if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
 get_field(env->mcounteren, HCOUNTEREN_CY)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_TIME:
 if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
 get_field(env->mcounteren, HCOUNTEREN_TM)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_INSTRET:
 if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
 get_field(env->mcounteren, HCOUNTEREN_IR)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
 if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3)) &&
 get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3))) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 }
@@ -100,93 +100,101 @@ static int ctr(CPURISCVState *env, int csrno)
 case CSR_CYCLEH:
 if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
 get_field(env->mcounteren, HCOUNTEREN_CY)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_TIMEH:
 if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
 get_field(env->mcounteren, HCOUNTEREN_TM)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_INSTRETH:
 if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
 get_field(env->mcounteren, HCOUNTEREN_IR)) {
-return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
 }
 break;
 case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
 if (!get_field(env->hcounteren, 1 << (csrno - 
CSR_HPMCOUNTER3H)) &&
 get_field(env->mcounteren, 1 << (csrno - 
CSR_HPMCOUNTER3H))) {
-return 

[PATCH v2 4/5] target/riscv: Use the RISCVException enum for CSR operations

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h |  14 +-
 target/riscv/csr.c | 643 +++--
 2 files changed, 390 insertions(+), 267 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1291ddc381..7b9b9da6b7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -475,12 +475,14 @@ static inline target_ulong riscv_csr_read(CPURISCVState 
*env, int csrno)
 
 typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env,
  int csrno);
-typedef int (*riscv_csr_read_fn)(CPURISCVState *env, int csrno,
-target_ulong *ret_value);
-typedef int (*riscv_csr_write_fn)(CPURISCVState *env, int csrno,
-target_ulong new_value);
-typedef int (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
-target_ulong *ret_value, target_ulong new_value, target_ulong write_mask);
+typedef RISCVException (*riscv_csr_read_fn)(CPURISCVState *env, int csrno,
+target_ulong *ret_value);
+typedef RISCVException (*riscv_csr_write_fn)(CPURISCVState *env, int csrno,
+ target_ulong new_value);
+typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
+  target_ulong *ret_value,
+  target_ulong new_value,
+  target_ulong write_mask);
 
 typedef struct {
 const char *name;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a82a98061b..9df65a609c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -203,57 +203,62 @@ static RISCVException pmp(CPURISCVState *env, int csrno)
 #endif
 
 /* User Floating-Point CSRs */
-static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
+static RISCVException read_fflags(CPURISCVState *env, int csrno,
+  target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
 *val = riscv_cpu_get_fflags(env);
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
+static RISCVException write_fflags(CPURISCVState *env, int csrno,
+   target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 env->mstatus |= MSTATUS_FS;
 #endif
 riscv_cpu_set_fflags(env, val & (FSR_AEXC >> FSR_AEXC_SHIFT));
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
+static RISCVException read_frm(CPURISCVState *env, int csrno,
+   target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
 *val = env->frm;
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
+static RISCVException write_frm(CPURISCVState *env, int csrno,
+target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 env->mstatus |= MSTATUS_FS;
 #endif
 env->frm = val & (FSR_RD >> FSR_RD_SHIFT);
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
+static RISCVException read_fcsr(CPURISCVState *env, int csrno,
+target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
 *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
@@ -262,14 +267,15 @@ static int read_fcsr(CPURISCVState *env, int csrno, 
target_ulong *val)
 *val |= (env->vxrm << FSR_VXRM_SHIFT)
 | (env->vxsat << FSR_VXSAT_SHIFT);
 }
-return 0;
+return RISCV_EXCP_NONE;
 }
 
-static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
+static RISCVException write_fcsr(CPURISCVState *env, int csrno,
+ target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
 if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
-return -RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_ILLEGAL_INST;
 }
 env->mstatus |= MSTATUS_FS;
 #endif
@@ -279,59 +285,68 @@ static int write_fcsr(CPURISCVState *env, int csrno, 
target_ulong val)
 env->vxsat = (val & FSR_VXSAT) >> FSR_VXSAT_SHIFT;
 }
 

[PATCH v2 1/5] target/riscv: Convert the RISC-V exceptions to an enum

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Bin Meng 
---
 target/riscv/cpu_bits.h   | 44 ---
 target/riscv/cpu.c|  2 +-
 target/riscv/cpu_helper.c |  4 ++--
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599207..f9ff932e85 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -527,27 +527,29 @@
 #define DEFAULT_RSTVEC  0x1000
 
 /* Exception causes */
-#define EXCP_NONE-1 /* sentinel value */
-#define RISCV_EXCP_INST_ADDR_MIS 0x0
-#define RISCV_EXCP_INST_ACCESS_FAULT 0x1
-#define RISCV_EXCP_ILLEGAL_INST  0x2
-#define RISCV_EXCP_BREAKPOINT0x3
-#define RISCV_EXCP_LOAD_ADDR_MIS 0x4
-#define RISCV_EXCP_LOAD_ACCESS_FAULT 0x5
-#define RISCV_EXCP_STORE_AMO_ADDR_MIS0x6
-#define RISCV_EXCP_STORE_AMO_ACCESS_FAULT0x7
-#define RISCV_EXCP_U_ECALL   0x8
-#define RISCV_EXCP_S_ECALL  0x9
-#define RISCV_EXCP_VS_ECALL  0xa
-#define RISCV_EXCP_M_ECALL   0xb
-#define RISCV_EXCP_INST_PAGE_FAULT   0xc /* since: priv-1.10.0 */
-#define RISCV_EXCP_LOAD_PAGE_FAULT   0xd /* since: priv-1.10.0 */
-#define RISCV_EXCP_STORE_PAGE_FAULT  0xf /* since: priv-1.10.0 */
-#define RISCV_EXCP_SEMIHOST  0x10
-#define RISCV_EXCP_INST_GUEST_PAGE_FAULT 0x14
-#define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT   0x15
-#define RISCV_EXCP_VIRT_INSTRUCTION_FAULT0x16
-#define RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT  0x17
+typedef enum RISCVException {
+RISCV_EXCP_NONE = -1, /* sentinel value */
+RISCV_EXCP_INST_ADDR_MIS = 0x0,
+RISCV_EXCP_INST_ACCESS_FAULT = 0x1,
+RISCV_EXCP_ILLEGAL_INST = 0x2,
+RISCV_EXCP_BREAKPOINT = 0x3,
+RISCV_EXCP_LOAD_ADDR_MIS = 0x4,
+RISCV_EXCP_LOAD_ACCESS_FAULT = 0x5,
+RISCV_EXCP_STORE_AMO_ADDR_MIS = 0x6,
+RISCV_EXCP_STORE_AMO_ACCESS_FAULT = 0x7,
+RISCV_EXCP_U_ECALL = 0x8,
+RISCV_EXCP_S_ECALL = 0x9,
+RISCV_EXCP_VS_ECALL = 0xa,
+RISCV_EXCP_M_ECALL = 0xb,
+RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
+RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
+RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
+RISCV_EXCP_SEMIHOST = 0x10,
+RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
+RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT = 0x15,
+RISCV_EXCP_VIRT_INSTRUCTION_FAULT = 0x16,
+RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT = 0x17,
+} RISCVException;
 
 #define RISCV_EXCP_INT_FLAG0x8000
 #define RISCV_EXCP_INT_MASK0x7fff
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7d6ed80f6b..500c9595bc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -358,7 +358,7 @@ static void riscv_cpu_reset(DeviceState *dev)
 env->pc = env->resetvec;
 env->two_stage_lookup = false;
 #endif
-cs->exception_index = EXCP_NONE;
+cs->exception_index = RISCV_EXCP_NONE;
 env->load_res = -1;
 set_default_nan_mode(1, >fp_status);
 }
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 21c54ef561..be7aaa0965 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -72,7 +72,7 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 if (irqs) {
 return ctz64(irqs); /* since non-zero */
 } else {
-return EXCP_NONE; /* indicates no pending interrupt */
+return RISCV_EXCP_NONE; /* indicates no pending interrupt */
 }
 }
 #endif
@@ -1069,5 +1069,5 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 
 env->two_stage_lookup = false;
 #endif
-cs->exception_index = EXCP_NONE; /* mark handled to qemu */
+cs->exception_index = RISCV_EXCP_NONE; /* mark handled to qemu */
 }
-- 
2.31.0

[PATCH v2 3/5] target/riscv: Fix 32-bit HS mode access permissions

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/csr.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5dc2aa9845..a82a98061b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -181,7 +181,11 @@ static RISCVException hmode(CPURISCVState *env, int csrno)
 static RISCVException hmode32(CPURISCVState *env, int csrno)
 {
 if (!riscv_cpu_is_32bit(env)) {
-return RISCV_EXCP_NONE;
+if (riscv_cpu_virt_enabled(env)) {
+return RISCV_EXCP_ILLEGAL_INST;
+} else {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
 }
 
 return hmode(env, csrno);
-- 
2.31.0

[PATCH] virtiofsd: Fix security.capability comparison

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

My security fix for the security.capability remap has a silly early
segfault in a simple case where there is an xattrmapping but it doesn't
remap the securty.capability.

Fixes: e586edcb41054 ("virtiofs: drop remapped security.capability xattr as 
needed")
Signed-off-by: Dr. David Alan Gilbert 
---
 tools/virtiofsd/passthrough_ll.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index b144320e48..1553d2ef45 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2636,7 +2636,8 @@ static void parse_xattrmap(struct lo_data *lo)
 strerror(ret));
 exit(1);
 }
-if (!strcmp(lo->xattr_security_capability, "security.capability")) {
+if (!lo->xattr_security_capability ||
+!strcmp(lo->xattr_security_capability, "security.capability")) {
 /* 1-1 mapping, don't need to do anything */
 free(lo->xattr_security_capability);
 lo->xattr_security_capability = NULL;
-- 
2.31.1

[PATCH 0/2] kvm: use KVM_{GET|SET}_SREGS2 when available

[Maxim Levitsky]

clone of "starship_unstable"

Maxim Levitsky (2):
  kvm: update kernel headers for KVM_{GET|SET}_SREGS2
  KVM: use KVM_{GET|SET}_SREGS2 when supported by kvm.

 accel/kvm/kvm-all.c |   4 ++
 include/sysemu/kvm.h|   4 ++
 linux-headers/asm-x86/kvm.h |  13 +
 linux-headers/linux/kvm.h   |   5 ++
 target/i386/cpu.h   |   1 +
 target/i386/kvm/kvm.c   | 101 +++-
 target/i386/machine.c   |  33 
 7 files changed, 159 insertions(+), 2 deletions(-)

-- 
2.26.2

[PATCH 2/2] KVM: use KVM_{GET|SET}_SREGS2 when supported by kvm.

[Maxim Levitsky]

This allows qemu to make PDPTRs be part of the migration
stream and thus not reload them after a migration which
is against X86 spec.

Signed-off-by: Maxim Levitsky 
---
 accel/kvm/kvm-all.c   |   4 ++
 include/sysemu/kvm.h  |   4 ++
 target/i386/cpu.h |   1 +
 target/i386/kvm/kvm.c | 101 +-
 target/i386/machine.c |  33 ++
 5 files changed, 141 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b6d9f92f15..082b791b01 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -142,6 +142,7 @@ bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
+bool kvm_sregs2;
 bool kvm_readonly_mem_allowed;
 bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
@@ -2186,6 +2187,9 @@ static int kvm_init(MachineState *ms)
 kvm_ioeventfd_any_length_allowed =
 (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
 
+kvm_sregs2 =
+(kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
+
 kvm_state = s;
 
 ret = kvm_arch_init(ms, s);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..b3d4538c55 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -32,6 +32,7 @@
 #ifdef CONFIG_KVM_IS_POSSIBLE
 
 extern bool kvm_allowed;
+extern bool kvm_sregs2;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_split_irqchip;
 extern bool kvm_async_interrupts_allowed;
@@ -139,6 +140,9 @@ extern bool kvm_msi_use_devid;
  */
 #define kvm_gsi_direct_mapping() (kvm_gsi_direct_mapping)
 
+
+#define kvm_supports_sregs2() (kvm_sregs2)
+
 /**
  * kvm_readonly_mem_enabled:
  *
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 570f916878..4595d47409 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1422,6 +1422,7 @@ typedef struct CPUX86State {
 SegmentCache idt; /* only base and limit are used */
 
 target_ulong cr[5]; /* NOTE: cr1 is unused */
+uint64_t pdptrs[4];
 int32_t a20_mask;
 
 BNDReg bnd_regs[4];
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 7fe9f52710..71769f82ae 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2514,6 +2514,59 @@ static int kvm_put_sregs(X86CPU *cpu)
 return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, );
 }
 
+static int kvm_put_sregs2(X86CPU *cpu)
+{
+CPUX86State *env = >env;
+struct kvm_sregs2 sregs;
+int i;
+
+if ((env->eflags & VM_MASK)) {
+set_v8086_seg(, >segs[R_CS]);
+set_v8086_seg(, >segs[R_DS]);
+set_v8086_seg(, >segs[R_ES]);
+set_v8086_seg(, >segs[R_FS]);
+set_v8086_seg(, >segs[R_GS]);
+set_v8086_seg(, >segs[R_SS]);
+} else {
+set_seg(, >segs[R_CS]);
+set_seg(, >segs[R_DS]);
+set_seg(, >segs[R_ES]);
+set_seg(, >segs[R_FS]);
+set_seg(, >segs[R_GS]);
+set_seg(, >segs[R_SS]);
+}
+
+set_seg(, >tr);
+set_seg(, >ldt);
+
+sregs.idt.limit = env->idt.limit;
+sregs.idt.base = env->idt.base;
+memset(sregs.idt.padding, 0, sizeof sregs.idt.padding);
+sregs.gdt.limit = env->gdt.limit;
+sregs.gdt.base = env->gdt.base;
+memset(sregs.gdt.padding, 0, sizeof sregs.gdt.padding);
+
+sregs.cr0 = env->cr[0];
+sregs.cr2 = env->cr[2];
+sregs.cr3 = env->cr[3];
+sregs.cr4 = env->cr[4];
+
+sregs.cr8 = cpu_get_apic_tpr(cpu->apic_state);
+sregs.apic_base = cpu_get_apic_base(cpu->apic_state);
+
+sregs.efer = env->efer;
+
+for (i = 0; i < 4; i++) {
+sregs.pdptrs[i] = env->pdptrs[i];
+}
+
+sregs.flags = 0;
+sregs.padding = 0;
+
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS2, );
+}
+
+
 static void kvm_msr_buf_reset(X86CPU *cpu)
 {
 memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
@@ -3175,6 +3228,49 @@ static int kvm_get_sregs(X86CPU *cpu)
 return 0;
 }
 
+static int kvm_get_sregs2(X86CPU *cpu)
+{
+CPUX86State *env = >env;
+struct kvm_sregs2 sregs;
+int i, ret;
+
+ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS2, );
+if (ret < 0) {
+return ret;
+}
+
+get_seg(>segs[R_CS], );
+get_seg(>segs[R_DS], );
+get_seg(>segs[R_ES], );
+get_seg(>segs[R_FS], );
+get_seg(>segs[R_GS], );
+get_seg(>segs[R_SS], );
+
+get_seg(>tr, );
+get_seg(>ldt, );
+
+env->idt.limit = sregs.idt.limit;
+env->idt.base = sregs.idt.base;
+env->gdt.limit = sregs.gdt.limit;
+env->gdt.base = sregs.gdt.base;
+
+env->cr[0] = sregs.cr0;
+env->cr[2] = sregs.cr2;
+env->cr[3] = sregs.cr3;
+env->cr[4] = sregs.cr4;
+
+env->efer = sregs.efer;
+
+for (i = 0; i < 4; i++) {
+env->pdptrs[i] = sregs.pdptrs[i];
+}
+
+/* changes to apic base and cr8/tpr are read back via kvm_arch_post_run */
+x86_update_hflags(env);
+
+return 0;
+}
+
 static int kvm_get_msrs(X86CPU *cpu)
 {
 CPUX86State *env = >env;
@@ -4000,7 +4096,8 @@ int kvm_arch_put_registers(CPUState *cpu, int 

[PATCH 1/2] kvm: update kernel headers for KVM_{GET|SET}_SREGS2

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
---
 linux-headers/asm-x86/kvm.h | 13 +
 linux-headers/linux/kvm.h   |  5 +
 2 files changed, 18 insertions(+)

diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 8e76d3701d..8c604e6bb1 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -158,6 +158,19 @@ struct kvm_sregs {
__u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 63) / 64];
 };
 
+struct kvm_sregs2 {
+   /* out (KVM_GET_SREGS2) / in (KVM_SET_SREGS2) */
+   struct kvm_segment cs, ds, es, fs, gs, ss;
+   struct kvm_segment tr, ldt;
+   struct kvm_dtable gdt, idt;
+   __u64 cr0, cr2, cr3, cr4, cr8;
+   __u64 efer;
+   __u64 apic_base;
+   __u64 flags; /* must be zero*/
+   __u64 pdptrs[4];
+   __u64 padding;
+};
+
 /* for KVM_GET_FPU and KVM_SET_FPU */
 struct kvm_fpu {
__u8  fpr[8][16];
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 020b62a619..a97f0f2d03 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1056,6 +1056,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
 #define KVM_CAP_SYS_HYPERV_CPUID 191
 #define KVM_CAP_DIRTY_LOG_RING 192
+#define KVM_CAP_SREGS2 196
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1563,6 +1564,10 @@ struct kvm_pv_cmd {
 /* Available with KVM_CAP_DIRTY_LOG_RING */
 #define KVM_RESET_DIRTY_RINGS  _IO(KVMIO, 0xc7)
 
+
+#define KVM_GET_SREGS2 _IOR(KVMIO,  0xca, struct kvm_sregs2)
+#define KVM_SET_SREGS2 _IOW(KVMIO,  0xcb, struct kvm_sregs2)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
/* Guest initialization commands */
-- 
2.26.2

[Bug 1922252] [NEW] [feature request] webcam support

[promeneur]

Public bug reported:

Please

I am impatient to get something as "-device usb-webcam" to share
dynamically the webcam between host and guest.

Thanks

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1922252

Title:
  [feature request] webcam support

Status in QEMU:
  New

Bug description:
  Please

  I am impatient to get something as "-device usb-webcam" to share
  dynamically the webcam between host and guest.

  Thanks

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1922252/+subscriptions

[PATCH 0/2] gdbstub: implement support for blocking interrupts on single stepping

[Maxim Levitsky]

clone of "starship_unstable"

Maxim Levitsky (2):
  kvm: update kernel headers for KVM_GUESTDBG_BLOCKEVENTS
  gdbstub: implement NOIRQ support for single step on KVM, when kvm's
KVM_GUESTDBG_BLOCKIRQ debug flag is supported.

 accel/kvm/kvm-all.c | 25 
 gdbstub.c   | 59 ++---
 include/sysemu/kvm.h| 13 
 linux-headers/asm-x86/kvm.h |  2 ++
 linux-headers/linux/kvm.h   |  1 +
 5 files changed, 90 insertions(+), 10 deletions(-)

-- 
2.26.2

Re: [PATCH 2/2] iotests/qsd-jobs: Use common.qemu for the QSD

[Vladimir Sementsov-Ogievskiy]

01.04.2021 16:28, Max Reitz wrote:

Using common.qemu allows us to wait for specific replies, so we can for
example wait for events.  This allows starting the active commit job and
then wait for it to be ready before quitting the QSD, so we the output
is always the same.

(Strictly speaking, this is only necessary for the first test in
qsd-jobs, but we might as well make the second one use common.qemu's
infrastructure, too.)



Reviewed-by: Vladimir Sementsov-Ogievskiy 


Reported-by: Peter Maydell 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/tests/qsd-jobs | 55 ---
  tests/qemu-iotests/tests/qsd-jobs.out | 10 -
  2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/tests/qsd-jobs 
b/tests/qemu-iotests/tests/qsd-jobs
index 972b6b3898..af7f886f15 100755
--- a/tests/qemu-iotests/tests/qsd-jobs
+++ b/tests/qemu-iotests/tests/qsd-jobs
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  cd ..
  . ./common.rc
  . ./common.filter
+. ./common.qemu
  
  _supported_fmt qcow2

  _supported_proto generic
@@ -52,32 +53,58 @@ echo "=== Job still present at shutdown ==="
  echo
  
  # Just make sure that this doesn't crash

-$QSD --chardev stdio,id=stdio --monitor chardev=stdio \
+qsd=y _launch_qemu \


sounds a bit strange.. Like we are starting qemu.


  --blockdev node-name=file0,driver=file,filename="$TEST_IMG" \
---blockdev node-name=fmt0,driver=qcow2,file=file0 <  
  echo

  echo "=== Streaming can't get permission on base node ==="
  echo
  
  # Just make sure that this doesn't crash

-$QSD --chardev stdio,id=stdio --monitor chardev=stdio \
+qsd=y _launch_qemu \
  --blockdev node-name=file_base,driver=file,filename="$TEST_IMG.base" \
  --blockdev node-name=fmt_base,driver=qcow2,file=file_base \
  --blockdev node-name=file_overlay,driver=file,filename="$TEST_IMG" \
  --blockdev 
node-name=fmt_overlay,driver=qcow2,file=file_overlay,backing=fmt_base \
  --nbd-server addr.type=unix,addr.path="$SOCK_DIR/nbd.sock" \
---export type=nbd,id=export1,node-name=fmt_base,writable=on,name=export1 \
-<  
  # success, all done

  echo "*** done"
diff --git a/tests/qemu-iotests/tests/qsd-jobs.out 
b/tests/qemu-iotests/tests/qsd-jobs.out
index 05e1165e80..5a14668939 100644
--- a/tests/qemu-iotests/tests/qsd-jobs.out
+++ b/tests/qemu-iotests/tests/qsd-jobs.out
@@ -4,13 +4,16 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file=TEST_DIR/
  
  === Job still present at shutdown ===
  
-QMP_VERSION

+{"execute":"qmp_capabilities"}
  {"return": {}}
+{"execute": "block-commit",
+  "arguments": {"device": "fmt0", "job-id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "created", "id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "running", "id": "job0"}}
  {"return": {}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "ready", "id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", 
"len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"execute": "quit"}
  {"return": {}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "standby", "id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "ready", "id": "job0"}}
@@ -22,11 +25,14 @@ QMP_VERSION
  
  === Streaming can't get permission on base node ===
  
-QMP_VERSION

+{"execute": "qmp_capabilities"}
  {"return": {}}
+{"execute": "block-stream",
+  "arguments": {"device": "fmt_overlay", "job-id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "created", "id": "job0"}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": 
{"status": "null", "id": "job0"}}
  {"error": {"class": "GenericError", "desc": "Conflicts with use by a block device 
as 'root', which uses 'write' on fmt_base"}}
+{"execute": "quit"}
  {"return": {}}
  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", 
"data": {"id": "export1"}}
  *** done




--
Best regards,
Vladimir

[PATCH 1/2] kvm: update kernel headers for KVM_GUESTDBG_BLOCKEVENTS

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
---
 linux-headers/asm-x86/kvm.h | 2 ++
 linux-headers/linux/kvm.h   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 8e76d3701d..33878cdc34 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -281,6 +281,8 @@ struct kvm_debug_exit_arch {
 #define KVM_GUESTDBG_USE_HW_BP 0x0002
 #define KVM_GUESTDBG_INJECT_DB 0x0004
 #define KVM_GUESTDBG_INJECT_BP 0x0008
+#define KVM_GUESTDBG_BLOCKIRQ  0x0010
+
 
 /* for KVM_SET_GUEST_DEBUG */
 struct kvm_guest_debug_arch {
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 020b62a619..2ded7a0630 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1056,6 +1056,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
 #define KVM_CAP_SYS_HYPERV_CPUID 191
 #define KVM_CAP_DIRTY_LOG_RING 192
+#define KVM_CAP_SET_GUEST_DEBUG2 195
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.26.2

[PATCH 2/2] gdbstub: implement NOIRQ support for single step on KVM, when kvm's KVM_GUESTDBG_BLOCKIRQ debug flag is supported.

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
---
 accel/kvm/kvm-all.c  | 25 +++
 gdbstub.c| 59 
 include/sysemu/kvm.h | 13 ++
 3 files changed, 87 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b6d9f92f15..bc7955fb19 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -147,6 +147,8 @@ bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 bool kvm_msi_use_devid;
+bool kvm_has_guest_debug;
+int kvm_sstep_flags;
 static bool kvm_immediate_exit;
 static hwaddr kvm_max_slot_size = ~0;
 
@@ -2186,6 +2188,25 @@ static int kvm_init(MachineState *ms)
 kvm_ioeventfd_any_length_allowed =
 (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
 
+kvm_has_guest_debug =
+(kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0);
+
+kvm_sstep_flags = 0;
+
+if (kvm_has_guest_debug) {
+/* Assume that single stepping is supported */
+kvm_sstep_flags = SSTEP_ENABLE;
+
+int guest_debug_flags =
+kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2);
+
+if (guest_debug_flags > 0) {
+if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) {
+kvm_sstep_flags |= SSTEP_NOIRQ;
+}
+}
+}
+
 kvm_state = s;
 
 ret = kvm_arch_init(ms, s);
@@ -2796,6 +2817,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long 
reinject_trap)
 
 if (cpu->singlestep_enabled) {
 data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
+
+if (cpu->singlestep_enabled & SSTEP_NOIRQ) {
+data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ;
+}
 }
 kvm_arch_update_guest_debug(cpu, );
 
diff --git a/gdbstub.c b/gdbstub.c
index 054665e93e..f789ded99d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -369,12 +369,11 @@ typedef struct GDBState {
 gdb_syscall_complete_cb current_syscall_cb;
 GString *str_buf;
 GByteArray *mem_buf;
+int sstep_flags;
+int supported_sstep_flags;
 } GDBState;
 
-/* By default use no IRQs and no timers while single stepping so as to
- * make single stepping like an ICE HW step.
- */
-static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+static GDBState gdbserver_state;
 
 /* Retrieves flags for single step mode. */
 static int get_sstep_flags(void)
@@ -386,11 +385,10 @@ static int get_sstep_flags(void)
 if (replay_mode != REPLAY_MODE_NONE) {
 return SSTEP_ENABLE;
 } else {
-return sstep_flags;
+return gdbserver_state.sstep_flags;
 }
 }
 
-static GDBState gdbserver_state;
 
 static void init_gdbserver_state(void)
 {
@@ -400,6 +398,23 @@ static void init_gdbserver_state(void)
 gdbserver_state.str_buf = g_string_new(NULL);
 gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
 gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 
4);
+
+
+if (kvm_enabled()) {
+gdbserver_state.supported_sstep_flags = 
kvm_get_supported_sstep_flags();
+} else {
+gdbserver_state.supported_sstep_flags =
+SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
+}
+
+/*
+ * By default use no IRQs and no timers while single stepping so as to
+ * make single stepping like an ICE HW step.
+ */
+
+gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
+gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags;
+
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -2023,24 +2038,43 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, 
void *user_ctx)
 
 static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
-SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER);
+g_string_printf(gdbserver_state.str_buf, "ENABLE=%x", SSTEP_ENABLE);
+
+if (gdbserver_state.supported_sstep_flags & SSTEP_NOIRQ) {
+g_string_append_printf(gdbserver_state.str_buf, ",NOIRQ=%x",
+   SSTEP_NOIRQ);
+}
+
+if (gdbserver_state.supported_sstep_flags & SSTEP_NOTIMER) {
+g_string_append_printf(gdbserver_state.str_buf, ",NOTIMER=%x",
+   SSTEP_NOTIMER);
+}
+
 put_strbuf();
 }
 
 static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
+int new_sstep_flags;
 if (!gdb_ctx->num_params) {
 return;
 }
 
-sstep_flags = gdb_ctx->params[0].val_ul;
+new_sstep_flags = gdb_ctx->params[0].val_ul;
+
+if (new_sstep_flags  & ~gdbserver_state.supported_sstep_flags) {
+put_packet("E22");
+return;
+}
+
+gdbserver_state.sstep_flags = new_sstep_flags;
 put_packet("OK");
 }
 
 static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags);
+

Re: [PATCH] iotests/qsd-jobs: Filter events in the first test

[Vladimir Sementsov-Ogievskiy]

01.04.2021 17:37, Vladimir Sementsov-Ogievskiy wrote:

01.04.2021 16:28, Max Reitz wrote:

The job may or may not be ready before the 'quit' is issued.  Whether it
is is irrelevant; for the purpose of the test, it only needs to still be
there.  Filter the job status change and READY events from the output so
it becomes reliable.

Reported-by: Peter Maydell
Suggested-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy



Hmm, in your mail (and in copypasted by r-b) there is no space between name and 
"<"..

--
Best regards,
Vladimir

Re: [PATCH] iotests/qsd-jobs: Filter events in the first test

[Vladimir Sementsov-Ogievskiy]

01.04.2021 16:28, Max Reitz wrote:

The job may or may not be ready before the 'quit' is issued.  Whether it
is is irrelevant; for the purpose of the test, it only needs to still be
there.  Filter the job status change and READY events from the output so
it becomes reliable.

Reported-by: Peter Maydell
Suggested-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy

--
Best regards,
Vladimir

Re: [PATCH V2 4/6] tools/vhost-user-i2c: Add backend driver

[Alex Bennée]

Viresh Kumar  writes:

> This adds the vhost-user backend driver to support virtio based I2C and
> SMBUS devices.
>
> vhost-user-i2c --help
>
> Signed-off-by: Viresh Kumar 

> +
> +static VI2cAdapter *vi2c_create_adapter(int32_t bus, uint16_t client_addr[],
> +int32_t n_client)
> +{
> +VI2cAdapter *adapter;
> +char path[20];

If you use:

g_autofree char *path = NULL;

If fact you could also use g_autofree with adapter to make you exit case
a little easier.

> +uint64_t funcs;
> +int32_t fd, i;
> +
> +if (bus < 0) {
> +return NULL;
> +}
> +
> +adapter = g_malloc0(sizeof(*adapter));

g_malloc will abort() on lack of memory so you can skip the check here.

> +if (!adapter) {
> +g_printerr("failed to alloc adapter");
> +return NULL;
> +}
> +
> +snprintf(path, sizeof(path), "/dev/i2c-%d", bus);
> +path[sizeof(path) - 1] = '\0';

then this becomes:

  path = g_strdup_printf("/dev/i2c-%d", bus);

> +
> +fd = open(path, O_RDWR);
> +if (fd < 0) {
> +g_printerr("virtio_i2c: failed to open %s\n", path);
> +goto fail;
> +}
> +
> +adapter->fd = fd;
> +adapter->bus = bus;
> +
> +if (ioctl(fd, I2C_FUNCS, ) < 0) {
> +g_printerr("virtio_i2c: failed to get functionality %s: %d\n", path,
> +   errno);
> +goto close_fd;
> +}
> +
> +if (funcs & I2C_FUNC_I2C) {
> +adapter->smbus = false;
> +} else if (funcs & I2C_FUNC_SMBUS_WORD_DATA) {
> +adapter->smbus = true;
> +} else {
> +g_printerr("virtio_i2c: invalid functionality %lx\n", funcs);
> +goto close_fd;
> +}
> +
> +for (i = 0; i < n_client; i++) {
> +if (client_addr[i]) {
> +if (!vi2c_set_client_addr(adapter, client_addr[i])) {
> +goto close_fd;
> +}
> +
> +if (adapter->clients[client_addr[i]]) {
> +g_printerr("client addr 0x%x repeat, not allowed.\n",
> +   client_addr[i]);
> +goto close_fd;
> +}
> +
> +adapter->clients[client_addr[i]] = true;
> +if (verbose) {
> +g_print("Added client 0x%x to bus %u\n", client_addr[i], 
> bus);
> +}
> +}
> +}
> +
> +if (verbose) {
> +g_print("Added adapter: bus: %d, func %s\n", bus,
> +adapter->smbus ? "smbus" : "i2c");
> +}
> +return adapter;

This would then be:

  return g_steal_pointer(adapter);

> +
> +close_fd:
> +close(fd);
> +fail:
> +g_free(adapter);
> +return NULL;
> +}
> +
> +/*
> + * Virtio I2C device list format.
> + *
> + * :[:],
> + * [:[:]]
> + *
> + * bus (dec): adatper bus number.
> + * e.g. 2 for /dev/i2c-2
> + * client_addr (hex): address for client device
> + * e.g. 0x1C or 1C
> + *
> + * Example: --device-list="2:0x1c:0x20,3:0x10:0x2c"
> + *
> + * Note: client address can not repeat.
> + */
> +static int32_t vi2c_parse(VuI2c *i2c)
> +{
> +uint16_t client_addr[MAX_I2C_VDEV];
> +int32_t n_adapter = 0, n_client;
> +int64_t addr, bus;
> +const char *cp, *t;
> +
> +while (device_list) {
> +/* Read :[:] entries one by one */
> +cp = strsep(_list, ",");

The glib approach would be to use g_strsplit(device_list, ",", 0) which
you can then iterate over with something like:

for (i = 0; cp[i] != NULL; i++) {

} 

g_strfreev(cp);

> +
> +if (!cp || *cp == '\0') {
> +break;
> +}
> +
> +if (n_adapter == MAX_I2C_ADAPTER) {
> +g_printerr("too many adapter (%d), only support %d\n", n_adapter,
> +   MAX_I2C_ADAPTER);
> +goto out;
> +}
> +
> +if (qemu_strtol(cp, , 10, ) || bus < 0) {
> +g_printerr("Invalid bus number %s\n", cp);
> +goto out;
> +}
> +
> +cp = t;
> +n_client = 0;
> +
> +/* Parse clients [:] entries one by
> one */

Then this would be:

 **dev = g_strsplit(cp[i], ":", 2);
 if (dev && dev[0]) {

 if (dev[1]) {
 }
 }
 g_freestrv(dev);

which would avoid you manually iterating over the string.

> +while (cp != NULL && *cp != '\0') {
> +if (*cp == ':') {
> +cp++;
> +}
> +
> +if (n_client == MAX_I2C_VDEV) {
> +g_printerr("too many devices (%d), only support %d\n",
> +   n_client, MAX_I2C_VDEV);
> +goto out;
> +}
> +
> +if (qemu_strtol(cp, , 16, ) ||
> +addr < 0 || addr > MAX_I2C_VDEV) {
> +g_printerr("Invalid address %s : %lx\n", cp, addr);
> +goto out;
> +}
> +
> +client_addr[n_client++] = addr;
> +cp = t;
> +if (verbose) {
> +g_print("i2c adapter %ld:0x%lx\n", bus, 

Re: [PATCH v2 2/2] hw/smbios: retrieve PCI address from specified device for Type 41

[Vincent Bernat]

 ❦  1 avril 2021 09:59 -04, Michael S. Tsirkin:

>> +/*
>> + * TODO: Extract the appropriate value. Most of the
>> + * time, this will be 0.
>> + */
>> +t->segment_group_number = cpu_to_le16(0);
>> +t->bus_number = pci_dev_bus_num(pdev);
>> +t->device_number = pdev->devfn;
>
> Problem is, for devices behind bridges for example, bus is only
> configured by guest, after pci has been enumerated.
>
> So I suspect this either
> - needs to be limited to only work for the root bus
> - needs to be re-evaluted on guest access, like we do
>   with ACPI

Or the address can be provided by the user. I didn't want to keep that
at this is error prone and there may be surprises after adding a device
or after a QEMU upgrade.

Otherwise, limiting to the root bus seems a fine limitation by me. How
do I check that?
-- 
Don't just echo the code with comments - make every comment count.
- The Elements of Programming Style (Kernighan & Plauger)

Re: [PATCH v2 10/11] gitlab-ci.yml: Fix the filtering for the git submodules

[Willian Rampazzo]

On Thu, Apr 1, 2021 at 7:25 AM Alex Bennée  wrote:
>
> From: Thomas Huth 
>
> Commit 7d7dbf9dc15be6e introduced a new line starting with
> "GIT_SUBMODULES_ACTION=" in the config-host.mak file. The grep that
> tries to determine the submodules in the gitlab-ci.yml file matches
> this new line, too, causing a warning message when updating the modules:
>
>  warn: ignoring non-existent submodule GIT_SUBMODULES_ACTION=update
>
> Fix it by matching the "GIT_SUBMODULES=..." line only.
>
> Signed-off-by: Thomas Huth 
> Signed-off-by: Alex Bennée 
> Message-Id: <20210331073316.2965928-1-th...@redhat.com>
> ---
>  .gitlab-ci.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Missing my review from the original patch, anyway:

Reviewed-by: Willian Rampazzo