Re: [RFC v4 0/9] Add support for zoned device

2022-07-11 Thread Markus Armbruster
Sam Li  writes:

> This patch series adds support for zoned device to virtio-blk emulation. Zoned
> Storage can support sequential writes, which reduces write amplification in 
> SSD,
> leading to higher write throughput and increased capacity.

Forgive me if this has already been discussed, or is explained deeper in
the patch series...

The commit message sounds like you're extending virtio-blk to optionally
emulate zoned storage.  Correct?

PATCH 1 adds a new block block device driver 'zoned_host_device', and
PATCH 9 exposes it in QAPI.  This is for passing through a zoned host
device, correct?




Re: [PATCH] target/riscv: move zmmul out of the experimental properties

2022-07-11 Thread Alistair Francis
On Sun, Jul 10, 2022 at 8:16 PM Weiwei Li  wrote:
>
> - Zmmul is ratified and is now version 1.0
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1bb3973806..6301871fdf 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -924,12 +924,13 @@ static Property riscv_cpu_extensions[] = {
>  DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
>  DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
>
> +DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> +
>  /* Vendor-specific custom extensions */
>  DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, 
> false),
>
>  /* These are experimental so mark with 'x-' */
>  DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
> -DEFINE_PROP_BOOL("x-zmmul", RISCVCPU, cfg.ext_zmmul, false),
>  /* ePMP 0.9.3 */
>  DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
>  DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
> --
> 2.17.1
>
>



Re: [PATCH] target/riscv: move zmmul out of the experimental properties

2022-07-11 Thread Alistair Francis
On Sun, Jul 10, 2022 at 8:16 PM Weiwei Li  wrote:
>
> - Zmmul is ratified and is now version 1.0
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1bb3973806..6301871fdf 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -924,12 +924,13 @@ static Property riscv_cpu_extensions[] = {
>  DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
>  DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
>
> +DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> +
>  /* Vendor-specific custom extensions */
>  DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, 
> false),
>
>  /* These are experimental so mark with 'x-' */
>  DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
> -DEFINE_PROP_BOOL("x-zmmul", RISCVCPU, cfg.ext_zmmul, false),
>  /* ePMP 0.9.3 */
>  DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
>  DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
> --
> 2.17.1
>
>



Re: [PATCH 1/3] accel/tcg: Fix unaligned stores to s390x low-address-protected lowcore

2022-07-11 Thread Richard Henderson

On 7/12/22 00:26, Ilya Leoshkevich wrote:

If low-address-protection is active, unaligned stores to non-protected
parts of lowcore lead to protection exceptions. The reason is that in
such cases tlb_fill() call in store_helper_unaligned() covers
[0, addr + size) range, which contains the protected portion of
lowcore. This range is too large.

The most straightforward fix would be to make sure we stay within the
original [addr, addr + size) range. However, if an unaligned access
affects a single page, we don't need to call tlb_fill() in
store_helper_unaligned() at all, since it would be identical to
the previous tlb_fill() call in store_helper(), and therefore a no-op.
If an unaligned access covers multiple pages, this situation does not
occur.

Therefore simply skip TLB handling in store_helper_unaligned() if we
are dealing with a single page.

Fixes: 2bcf018340cb ("s390x/tcg: low-address protection support")
Signed-off-by: Ilya Leoshkevich 


Reviewed-by: Richard Henderson 

Queueing to tcg-next.  I'll let the test case go through s390x.


r~



Re: [PATCH 2/3] hw/misc: Add mmio-debug-exit device

2022-07-11 Thread Richard Henderson

On 7/12/22 00:26, Ilya Leoshkevich wrote:

System tests on x86 use isa-debug-exit device in order to signal
success or failure to the test runner. Unfortunately it's not easily
usable on other architectures, since a guest needs to access
address_space_io, which may not be as straightforward as on x86.
Also, it requires adding ISA bus, which an architecture might not
otherwise need.

Introduce mmio-debug-exit device, which has the same semantics, but is
triggered by writes to memory.

Signed-off-by: Ilya Leoshkevich 


You shouldn't need this for s390x, as there are already (at least) two other paths to 
qemu_system_shutdown_request.


E.g. SIGP, which has a stop option.


r~



Re: [PATCH v2] target/riscv: fix shifts shamt value for rv128c

2022-07-11 Thread Alistair Francis
On Sun, Jul 10, 2022 at 9:05 PM Frédéric Pétrot
 wrote:
>
> For rv128c shifts, a shamt of 0 is a shamt of 64, while for rv32c/rv64c
> it stays 0 and is a hint instruction that does not change processor state.
> For rv128c right shifts, the 6-bit shamt is in addition sign extended to
> 7 bits.
>
> Signed-off-by: Frédéric Pétrot 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/insn16.decode |  7 ---
>  disas/riscv.c  | 27 +--
>  target/riscv/translate.c   | 20 ++--
>  3 files changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
> index 02c8f61b48..ccfe59f294 100644
> --- a/target/riscv/insn16.decode
> +++ b/target/riscv/insn16.decode
> @@ -31,7 +31,8 @@
>  %imm_cb12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
>  %imm_cj12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
>
> -%shimm_6bit   12:1 2:5   !function=ex_rvc_shifti
> +%shlimm_6bit  12:1 2:5   !function=ex_rvc_shiftli
> +%shrimm_6bit  12:1 2:5   !function=ex_rvc_shiftri
>  %uimm_6bit_lq 2:4 12:1 6:1   !function=ex_shift_4
>  %uimm_6bit_ld 2:3 12:1 5:2   !function=ex_shift_3
>  %uimm_6bit_lw 2:2 12:1 4:3   !function=ex_shift_2
> @@ -82,9 +83,9 @@
>  @c_addi16sp ... .  . . ..  imm=%imm_addi16sp rs1=2 rd=2
>
>  @c_shift... . .. ... . .. \
> - rd=%rs1_3 rs1=%rs1_3 shamt=%shimm_6bit
> + rd=%rs1_3 rs1=%rs1_3 shamt=%shrimm_6bit
>  @c_shift2   ... . .. ... . .. \
> - rd=%rd rs1=%rd shamt=%shimm_6bit
> + rd=%rd rs1=%rd shamt=%shlimm_6bit
>
>  @c_andi ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 7af6afc8fa..489c2ae5e8 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -2402,10 +2402,25 @@ static int32_t operand_sbimm12(rv_inst inst)
>  ((inst << 56) >> 63) << 11;
>  }
>
> -static uint32_t operand_cimmsh6(rv_inst inst)
> +static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>  {
> -return ((inst << 51) >> 63) << 5 |
> +int imm = ((inst << 51) >> 63) << 5 |
>  (inst << 57) >> 59;
> +if (isa == rv128) {
> +imm = imm ? imm : 64;
> +}
> +return imm;
> +}
> +
> +static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
> +{
> +int imm = ((inst << 51) >> 63) << 5 |
> +(inst << 57) >> 59;
> +if (isa == rv128) {
> +imm = imm | (imm & 32) << 1;
> +imm = imm ? imm : 64;
> +}
> +return imm;
>  }
>
>  static int32_t operand_cimmi(rv_inst inst)
> @@ -2529,7 +2544,7 @@ static uint32_t operand_rnum(rv_inst inst)
>
>  /* decode operands */
>
> -static void decode_inst_operands(rv_decode *dec)
> +static void decode_inst_operands(rv_decode *dec, rv_isa isa)
>  {
>  rv_inst inst = dec->inst;
>  dec->codec = opcode_data[dec->op].codec;
> @@ -2652,7 +2667,7 @@ static void decode_inst_operands(rv_decode *dec)
>  case rv_codec_cb_sh6:
>  dec->rd = dec->rs1 = operand_crs1rdq(inst) + 8;
>  dec->rs2 = rv_ireg_zero;
> -dec->imm = operand_cimmsh6(inst);
> +dec->imm = operand_cimmshr6(inst, isa);
>  break;
>  case rv_codec_ci:
>  dec->rd = dec->rs1 = operand_crs1rd(inst);
> @@ -2667,7 +2682,7 @@ static void decode_inst_operands(rv_decode *dec)
>  case rv_codec_ci_sh6:
>  dec->rd = dec->rs1 = operand_crs1rd(inst);
>  dec->rs2 = rv_ireg_zero;
> -dec->imm = operand_cimmsh6(inst);
> +dec->imm = operand_cimmshl6(inst, isa);
>  break;
>  case rv_codec_ci_16sp:
>  dec->rd = rv_ireg_sp;
> @@ -3193,7 +3208,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, 
> uint64_t pc, rv_inst inst)
>  dec.pc = pc;
>  dec.inst = inst;
>  decode_inst_opcode(, isa);
> -decode_inst_operands();
> +decode_inst_operands(, isa);
>  decode_inst_decompress(, isa);
>  decode_inst_lift_pseudo();
>  format_inst(buf, buflen, 16, );
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 63b04e8a94..d7c82a9c81 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -705,10 +705,26 @@ static int ex_rvc_register(DisasContext *ctx, int reg)
>  return 8 + reg;
>  }
>
> -static int ex_rvc_shifti(DisasContext *ctx, int imm)
> +static int ex_rvc_shiftli(DisasContext *ctx, int imm)
>  {
>  /* For RV128 a shamt of 0 means a shift by 64. */
> -return imm ? imm : 64;
> +if (get_ol(ctx) == MXL_RV128) {
> +imm = imm ? imm : 64;
> +}
> +return imm;
> +}
> +
> +static int ex_rvc_shiftri(DisasContext *ctx, int imm)
> +{
> +/*
> + * For RV128 a shamt of 0 means a shift by 64, furthermore, for right
> + * shifts, the shamt is sign-extended.
> + */
> +if (get_ol(ctx) == MXL_RV128) {
> +imm = imm | (imm & 32) << 1;

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-07-11 Thread Richard Henderson

On 7/12/22 03:30, Vitaly Buka wrote:

aarch64 stores MTE tags in target_date, and they should be reset by
MADV_DONTNEED.

Signed-off-by: Vitaly Buka 
---
  accel/tcg/translate-all.c | 24 
  include/exec/cpu-all.h|  1 +
  linux-user/mmap.c |  2 ++
  3 files changed, 27 insertions(+)


Reviewed-by: Richard Henderson 

r~



diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ef62a199c7..d6f2f1a40a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2314,6 +2314,30 @@ void page_set_flags(target_ulong start, target_ulong 
end, int flags)
  }
  }
  
+void page_reset_target_data(target_ulong start, target_ulong end)

+{
+target_ulong addr, len;
+
+/* This function should never be called with addresses outside the
+   guest address space.  If this assert fires, it probably indicates
+   a missing call to h2g_valid.  */
+assert(end - 1 <= GUEST_ADDR_MAX);
+assert(start < end);
+assert_memory_lock();
+
+start = start & TARGET_PAGE_MASK;
+end = TARGET_PAGE_ALIGN(end);
+
+for (addr = start, len = end - start;
+ len != 0;
+ len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+
+g_free(p->target_data);
+p->target_data = NULL;
+}
+}
+
  void *page_get_target_data(target_ulong address)
  {
  PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f5bda2c3ca..491629b9ba 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -271,6 +271,7 @@ int walk_memory_regions(void *, walk_memory_regions_fn);
  
  int page_get_flags(target_ulong address);

  void page_set_flags(target_ulong start, target_ulong end, int flags);
+void page_reset_target_data(target_ulong start, target_ulong end);
  int page_check_range(target_ulong start, target_ulong len, int flags);
  
  /**

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 4e7a6be6ee..c535dfdc7c 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -894,6 +894,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, 
int advice)
  if ((advice & MADV_DONTNEED) &&
  can_passthrough_madv_dontneed(start, end)) {
  ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
+if (ret == 0)
+page_reset_target_data(start, start + len);
  }
  mmap_unlock();
  





Re: Booting bare-metal RISC-V virt (Was: [PATCH] Add some documentation for "dtb" devices tree blobs)

2022-07-11 Thread Alistair Francis
On Mon, Jun 27, 2022 at 4:15 PM Simon Sapin  wrote:
>
> On 27/06/2022 07:40, Alistair Francis wrote:
> > We have previously kept the addresses backwards compatible. So that
> > software for an older virt machine will work on a newer one. There is
> > currently talks about changing the virt machine memory layout in a
> > breaking way and versioning in the current one though.
> >
> > So I don't really have a good answer for you. I would recommend
> > reading as much as possible from the device tree dynamically at boot.
> >
> > In general though we don't want to break people, we just might have to
> > make changes in the future to allow for new functionality.
>
> I agree that reading from the device tree as much as possible is good. We 
> there’s
> still a need to get code running at all, and finding the device tree.
>
> So it would be good to decide to make stable what’s needed to get there (like 
> was
> apparently decided for ARM) and document it.

Yeah, we are working towards that

>
> On principle maybe a firmware/bootloader could be entirely 
> position-independent? But

I don't link the RISC-V toolchains suppor fully position independent code

> in what I’ve done/seen so far https://docs.rs/riscv-rt/latest/riscv_rt/ has 
> address
> ranges hard-coded in a linker script for different regions, and when passing 
> an ELF
> file to -kernel, QEMU maps it to those addresses but boots at 0x8000_ 
> regardless.

Yeah, I suspect we will keep the 0x8000_ as that's pretty standard

>
>
> >> * With `qemu-system-riscv32 -machine virt -bios none -kernel something.elf 
> >> -s -S`,
> >> GDB shows that execution starts at the lowest address of RAM, not of flash 
> >> like I
> >> expected. Then what is emulated flash for?
> >
> > If you supply a flash image we will start executing from flash 
> > automatically.
>
> Passing with -drive? Should I use that instead of -kernel?

If you want to pass a drive then yes, that's the better option

>
>
> >> * To what extent is the above calling convention standardized? I found 
> >> similar things
> >> in coreboot[4] and in OpenSBI[5]
> >
> > Good question. I don't think it's specified in a spec, but it is very common
>
> Should we document this convention as something guest code can rely on?

We probably should at some point

Alistair

>
> --
> Simon Sapin



Re: [PATCH 3/3] tests/tcg/s390x: test signed vfmin/vfmax

2022-07-11 Thread Richard Henderson

On 7/12/22 07:27, Ilya Leoshkevich wrote:

+/*
+ * vfmin/vfmax code generation.
+ */
+extern const char vfminmax_template[];
+extern const int vfminmax_template_size;
+extern const int vfminmax_offset;
+asm(".globl vfminmax_template\n"
+"vfminmax_template:\n"
+"vl %v25,0(%r3)\n"
+"vl %v26,0(%r4)\n"
+"0: vfmax %v24,%v25,%v26,2,0,0\n"
+"vst %v24,0(%r2)\n"
+"br %r14\n"
+"1: .align 4\n"
+".globl vfminmax_template_size\n"
+"vfminmax_template_size: .long 1b - vfminmax_template\n"
+".globl vfminmax_offset\n"
+"vfminmax_offset: .long 0b - vfminmax_template\n");

...

+
+#define VFMIN 0xEE
+#define VFMAX 0xEF
+
+static void vfminmax(unsigned char *buf, unsigned int op,
+ unsigned int m4, unsigned int m5, unsigned int m6,
+ void *v1, const void *v2, const void *v3)
+{
+memcpy(buf, vfminmax_template, vfminmax_template_size);
+buf[vfminmax_offset + 3] = (m6 << 4) | m5;
+buf[vfminmax_offset + 4] &= 0x0F;
+buf[vfminmax_offset + 4] |= (m4 << 4);
+buf[vfminmax_offset + 5] = op;
+((void (*)(void *, const void *, const void *))buf)(v1, v2, v3);
+}


This works, of course.  It could be simpler using EXECUTE, to store just the one 
instruction and not worry about an executable mapped page, but I guess it doesn't matter.


Reviewed-by: Richard Henderson 


r~



Re: [PATCH 2/3] target/s390x: fix NaN propagation rules

2022-07-11 Thread Richard Henderson

On 7/12/22 07:27, Ilya Leoshkevich wrote:

s390x has the same NaN propagation rules as ARM, and not as x86.

Signed-off-by: Ilya Leoshkevich
---
  fpu/softfloat-specialize.c.inc | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


Yes, I think no one has those x86 rules, including x86.  :-)

Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 09/12] tests/vm: Remove docker cross-compile test from CentOS VM

2022-07-11 Thread Richard Henderson

On 7/8/22 21:05, John Snow wrote:

The fedora container has since been split apart, so there's no suitable
nearby target that would support "test-mingw" as it requires both x32
and x64 support -- so either fedora-cross-win32 nor fedora-cross-win64
would be truly suitable.

Just remove this test as superfluous with our current CI infrastructure.

Signed-off-by: John Snow
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/centos | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 1/3] target/s390x: fix handling of zeroes in vfmin/vfmax

2022-07-11 Thread Richard Henderson

On 7/12/22 07:27, Ilya Leoshkevich wrote:

vfmin_res() / vfmax_res() are trying to check whether a and b are both
zeroes, but in reality they check that they are the same kind of zero.
This causes incorrect results when comparing positive and negative
zeroes.

Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)")
Co-developed-by: Ulrich Weigand
Signed-off-by: Ilya Leoshkevich
---
  target/s390x/tcg/vec_fpu_helper.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [RFC PATCH v9 19/23] vdpa: Extract get features part from vhost_vdpa_get_max_queue_pairs

2022-07-11 Thread Jason Wang



在 2022/7/7 02:40, Eugenio Pérez 写道:

To know the device features is needed for CVQ SVQ, so SVQ knows if it
can handle all commands or not. Extract from
vhost_vdpa_get_max_queue_pairs so we can reuse it.

Signed-off-by: Eugenio Pérez 



Acked-by: Jason Wang 



---
  net/vhost-vdpa.c | 30 --
  1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index df1e69ee72..b0158f625e 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -219,20 +219,24 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
*peer,
  return nc;
  }
  
-static int vhost_vdpa_get_max_queue_pairs(int fd, int *has_cvq, Error **errp)

+static int vhost_vdpa_get_features(int fd, uint64_t *features, Error **errp)
+{
+int ret = ioctl(fd, VHOST_GET_FEATURES, features);
+if (ret) {
+error_setg_errno(errp, errno,
+ "Fail to query features from vhost-vDPA device");
+}
+return ret;
+}
+
+static int vhost_vdpa_get_max_queue_pairs(int fd, uint64_t features,
+  int *has_cvq, Error **errp)
  {
  unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
  g_autofree struct vhost_vdpa_config *config = NULL;
  __virtio16 *max_queue_pairs;
-uint64_t features;
  int ret;
  
-ret = ioctl(fd, VHOST_GET_FEATURES, );

-if (ret) {
-error_setg(errp, "Fail to query features from vhost-vDPA device");
-return ret;
-}
-
  if (features & (1 << VIRTIO_NET_F_CTRL_VQ)) {
  *has_cvq = 1;
  } else {
@@ -262,10 +266,11 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
  NetClientState *peer, Error **errp)
  {
  const NetdevVhostVDPAOptions *opts;
+uint64_t features;
  int vdpa_device_fd;
  g_autofree NetClientState **ncs = NULL;
  NetClientState *nc;
-int queue_pairs, i, has_cvq = 0;
+int queue_pairs, r, i, has_cvq = 0;
  
  assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);

  opts = >u.vhost_vdpa;
@@ -279,7 +284,12 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
  return -errno;
  }
  
-queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd,

+r = vhost_vdpa_get_features(vdpa_device_fd, , errp);
+if (r) {
+return r;
+}
+
+queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd, features,
   _cvq, errp);
  if (queue_pairs < 0) {
  qemu_close(vdpa_device_fd);





Re: [PATCH v4 08/12] tests/vm: add 1GB extra memory per core

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

If you try to run a 16 or 32 threaded test, you're going to run out of
memory very quickly with qom-test and a few others. Bump the memory
limit to try to scale with larger-core machines.

Granted, this means that a 16 core processor is going to ask for 16GB,
but you*probably*  meet that requirement if you have such a machine.

512MB per core didn't seem to be enough to avoid ENOMEM and SIGABRTs in
the test cases in practice on a six core machine; so I bumped it up to
1GB which seemed to help.

Add this magic in early to the configuration process so that the
config file, if provided, can still override it.

Signed-off-by: John Snow
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/basevm.py | 5 +
  1 file changed, 5 insertions(+)


Acked-by: Richard Henderson 



Re: [PATCH v4 07/12] tests/vm: remove duplicate 'centos' VM test

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

This is listed twice by accident; we require genisoimage to run the
test, so remove the unconditional entry.

Signed-off-by: John Snow
Reviewed-by: Thomas Huth
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/Makefile.include | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 03/12] tests/vm: switch CentOS 8 to CentOS 8 Stream

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

The old CentOS image didn't work anymore because it was already EOL at
the beginning of 2022.

Signed-off-by: John Snow
Reviewed-by: Thomas Huth
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/centos | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 06/12] tests/vm: remove ubuntu.i386 VM test

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

Ubuntu 18.04 is out of our support window, and Ubuntu 20.04 does not
support i386 anymore. The debian project does, but they do not provide
any cloud images for it, a new expect-style script would have to be
written.

Since we have i386 cross-compiler tests hosted on GitLab CI, we don't
need to support this VM test anymore.

Signed-off-by: John Snow
Reviewed-by: Thomas Huth
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/Makefile.include |  3 +--
  tests/vm/ubuntu.i386  | 40 ---
  2 files changed, 1 insertion(+), 42 deletions(-)
  delete mode 100755 tests/vm/ubuntu.i386


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 05/12] tests/vm: upgrade Ubuntu 18.04 VM to 20.04

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

18.04 has fallen out of our support window, so move ubuntu.aarch64
forward to ubuntu 20.04, which is now our oldest supported Ubuntu
release.

Notes:

This checksum changes periodically; use a fixed point image with a known
checksum so that the image isn't re-downloaded on every single
invocation. (The checksum for the 18.04 image was already incorrect at
the time of writing.)

Just like the centos.aarch64 test, this test currently seems very
flaky when run as a TCG test.

Signed-off-by: John Snow
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/ubuntu.aarch64 | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 04/12] tests/vm: switch centos.aarch64 to CentOS 8 Stream

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

Switch this test over to using a cloud image like the base CentOS8 VM
test, which helps make this script a bit simpler too.

Note: At time of writing, this test seems pretty flaky when run without
KVM support for aarch64. Certain unit tests like migration-test,
virtio-net-failover, test-hmp and qom-test seem quite prone to fail
under TCG. Still, this is an improvement in that at least pure build
tests are functional.

Signed-off-by: John Snow
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/centos.aarch64 | 174 ++--
  1 file changed, 24 insertions(+), 150 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 02/12] tests/vm: use 'cp' instead of 'ln' for temporary vm images

2022-07-11 Thread Richard Henderson

On 7/8/22 21:04, John Snow wrote:

If the initial setup fails, you've permanently altered the state of the
downloaded image in an unknowable way. Use 'cp' like our other test
setup scripts do.

Signed-off-by: John Snow
Reviewed-by: Thomas Huth
Reviewed-by: Daniel P. Berrangé
---
  tests/vm/centos | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning

2022-07-11 Thread Akihiko Odaki

On 2022/07/12 9:05, Peter Delevoryas wrote:

On Wed, Jul 06, 2022 at 05:58:38PM -0700, Peter Delevoryas wrote:

On Sat, Jul 02, 2022 at 11:30:16PM +0900, Akihiko Odaki wrote:

Reviewed-by: Akihiko Odaki 


Just checking in on the status of this: do I need to submit a pull request?
Or will this patch be picked up in a miscellaneous pull queue eventually?


Pinging this thread again, does this change need anyone else to review it?


The patch should be picked up later. You may ping again if there is no 
response after weeks.


Sorry for replying late,
Akihiko Odaki







On 2022/07/02 13:43, Peter Delevoryas wrote:

I noticed this error while building QEMU on Mac OS X:

  [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
  ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but 
not used [-Wunused-but-set-variable]
  static bool switched_to_fullscreen = false;
  ^
  1 warning generated.

I think the behavior is fine if you remove "switched_to_fullscreen", I can
still switch in and out of mouse grabbed mode and fullscreen mode with this
change, and Command keycodes will only be passed to the guest if the mouse
is grabbed, which I think is the right behavior. I'm not sure why a static
piece of state was needed to handle that in the first place. Perhaps the
refactoring of the flags-state-change fixed that by toggling the Command
keycode on.

I tested this with an Ubuntu core image on macOS 12.4

  wget 
https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
  xz -d ubuntu-core-18-i386.img.xz
  qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw

Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
Signed-off-by: Peter Delevoryas 
---
   ui/cocoa.m | 8 
   1 file changed, 8 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 84c84e98fc..13e208b037 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
   int buttons = 0;
   int keycode = 0;
   bool mouse_event = false;
-static bool switched_to_fullscreen = false;
   // Location of event in virtual screen coordinates
   NSPoint p = [self screenLocationOfEvent:event];
   NSUInteger modifiers = [event modifierFlags];
@@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
   // forward command key combos to the host UI unless the mouse is 
grabbed
   if (!isMouseGrabbed && ([event modifierFlags] & 
NSEventModifierFlagCommand)) {
-/*
- * Prevent the command key from being stuck down in the guest
- * when using Command-F to switch to full screen mode.
- */
-if (keycode == Q_KEY_CODE_F) {
-switched_to_fullscreen = true;
-}
   return false;
   }









Re: [PATCH] meson: place default firmware path under .../share

2022-07-11 Thread Richard Henderson

On 7/11/22 13:37, Paolo Bonzini wrote:

Fixes: c09c1ce7e9 ("configure: switch directory options to automatic parsing", 
2022-05-07)
Signed-off-by: Paolo Bonzini
---
  meson_options.txt | 2 +-
  scripts/meson-buildoptions.sh | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 0/2] gitlab-ci: msys2 improvements

2022-07-11 Thread Richard Henderson

On 7/11/22 13:26, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Hi

This is a small series to attempt to debug "Intermittent meson failures on
msys2" and improve a bit msys2/gitlab reports.


Thanks.  I've pushed this to staging for a test run:

https://gitlab.com/qemu-project/qemu/-/pipelines/585473909


r~



Re: [PATCH v2] target/riscv: fix shifts shamt value for rv128c

2022-07-11 Thread Richard Henderson

On 7/12/22 01:24, Frédéric Pétrot wrote:

   Agreed, on the non compressed insns, but the compressed ones have a 6-bit
   shamt only as visible on page 18.6 page 125. The explanation for rv128 shifts
   is further detailed in the emphasized paragraph on top of page 120.


I see.  I should have read the "c" more carefully there.
Indeed, the code is correct.

I think the language could be improved a little for clarity:


    +static int ex_rvc_shiftri(DisasContext *ctx, int imm)
    +{
    +    /*
    +     * For RV128 a shamt of 0 means a shift by 64, furthermore, for right
    +     * shifts, the shamt is sign-extended.
    +     */


For RV128C, a shamt of 0 means shift by 64, and the shamt is sign-extended.  Combine this 
with implicit truncation to 7 bits, and this equates to replicating bit 5 to bit 6.


Reviewed-by: Richard Henderson 


r~



[PATCH v3 2/3] hw/gpio/aspeed: Don't let guests modify input pins

2022-07-11 Thread Peter Delevoryas
Up until now, guests could modify input pins by overwriting the data
value register. The guest OS should only be allowed to modify output pin
values, and the QOM property setter should only be permitted to modify
input pins.

This change also updates the gpio input pin test to match this
expectation.

Andrew suggested this particularly refactoring here:


https://lore.kernel.org/qemu-devel/23523aa1-ba81-412b-92cc-8174faba3...@www.fastmail.com/

Suggested-by: Andrew Jeffery 
Signed-off-by: Peter Delevoryas 
Fixes: 4b7f956862dc ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and 
AST2500")
---
 hw/gpio/aspeed_gpio.c  | 15 ---
 tests/qtest/aspeed_gpio-test.c |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index a62a673857..1e267dd482 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -268,7 +268,7 @@ static ptrdiff_t aspeed_gpio_set_idx(AspeedGPIOState *s, 
GPIOSets *regs)
 }
 
 static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,
-   uint32_t value)
+   uint32_t value, uint32_t mode_mask)
 {
 uint32_t input_mask = regs->input_mask;
 uint32_t direction = regs->direction;
@@ -277,7 +277,8 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets 
*regs,
 uint32_t diff;
 int gpio;
 
-diff = old ^ new;
+diff = (old ^ new);
+diff &= mode_mask;
 if (diff) {
 for (gpio = 0; gpio < ASPEED_GPIOS_PER_SET; gpio++) {
 uint32_t mask = 1 << gpio;
@@ -339,7 +340,7 @@ static void aspeed_gpio_set_pin_level(AspeedGPIOState *s, 
uint32_t set_idx,
 value &= ~pin_mask;
 }
 
-aspeed_gpio_update(s, >sets[set_idx], value);
+aspeed_gpio_update(s, >sets[set_idx], value, 
~s->sets[set_idx].direction);
 }
 
 /*
@@ -653,7 +654,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, 
hwaddr offset,
 reg_value = update_value_control_source(set, set->data_value,
 reg_value);
 set->data_read = reg_value;
-aspeed_gpio_update(s, set, reg_value);
+aspeed_gpio_update(s, set, reg_value, set->direction);
 return;
 case gpio_reg_idx_direction:
 reg_value = set->direction;
@@ -753,7 +754,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, 
hwaddr offset,
 __func__, offset, data, reg_idx_type);
 return;
 }
-aspeed_gpio_update(s, set, set->data_value);
+aspeed_gpio_update(s, set, set->data_value, UINT32_MAX);
 return;
 }
 
@@ -799,7 +800,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, 
uint64_t data,
 data &= props->output;
 data = update_value_control_source(set, set->data_value, data);
 set->data_read = data;
-aspeed_gpio_update(s, set, data);
+aspeed_gpio_update(s, set, data, set->direction);
 return;
 case gpio_reg_direction:
 /*
@@ -875,7 +876,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, 
uint64_t data,
   PRIx64"\n", __func__, offset);
 return;
 }
-aspeed_gpio_update(s, set, set->data_value);
+aspeed_gpio_update(s, set, set->data_value, UINT32_MAX);
 return;
 }
 
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index 8f52454099..d38f51d719 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -69,7 +69,7 @@ static void test_set_input_pins(const void *data)
 
 qtest_writel(s, AST2600_GPIO_BASE + GPIO_ABCD_DATA_VALUE, 0x);
 value = qtest_readl(s, AST2600_GPIO_BASE + GPIO_ABCD_DATA_VALUE);
-g_assert_cmphex(value, ==, 0x);
+g_assert_cmphex(value, ==, 0x);
 }
 
 int main(int argc, char **argv)
-- 
2.37.0




[PATCH v3 0/3] hw/gpio/aspeed: Don't let guests modify input pins

2022-07-11 Thread Peter Delevoryas
v3:
- Replaced fix in v2 with change suggested by arj
- Removed all changes to the body of the aspeed_gpio_update loop
- Removed guest error messages from v2

Peter Delevoryas (3):
  qtest/aspeed_gpio: Add input pin modification test
  hw/gpio/aspeed: Don't let guests modify input pins
  aspeed: Add fby35-bmc slot GPIO's

 hw/arm/aspeed.c| 14 +-
 hw/gpio/aspeed_gpio.c  | 15 ---
 tests/qtest/aspeed_gpio-test.c | 27 +++
 3 files changed, 48 insertions(+), 8 deletions(-)

-- 
2.37.0




[PATCH v3 3/3] aspeed: Add fby35-bmc slot GPIO's

2022-07-11 Thread Peter Delevoryas
Signed-off-by: Peter Delevoryas 
---
 hw/arm/aspeed.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6fe9b13548..0ce9a42c2b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1343,11 +1343,23 @@ static void fby35_reset(MachineState *state)
 
 qemu_devices_reset();
 
-/* Board ID */
+/* Board ID: 7 (Class-1, 4 slots) */
 object_property_set_bool(OBJECT(gpio), "gpioV4", true, _fatal);
 object_property_set_bool(OBJECT(gpio), "gpioV5", true, _fatal);
 object_property_set_bool(OBJECT(gpio), "gpioV6", true, _fatal);
 object_property_set_bool(OBJECT(gpio), "gpioV7", false, _fatal);
+
+/* Slot presence pins, inverse polarity. (False means present) */
+object_property_set_bool(OBJECT(gpio), "gpioH4", false, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioH5", true, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioH6", true, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioH7", true, _fatal);
+
+/* Slot 12v power pins, normal polarity. (True means powered-on) */
+object_property_set_bool(OBJECT(gpio), "gpioB2", true, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioB3", false, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioB4", false, _fatal);
+object_property_set_bool(OBJECT(gpio), "gpioB5", false, _fatal);
 }
 
 static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
-- 
2.37.0




[PATCH v3 1/3] qtest/aspeed_gpio: Add input pin modification test

2022-07-11 Thread Peter Delevoryas
Verify the current behavior, which is that input pins can be modified by
guest OS register writes.

Signed-off-by: Peter Delevoryas 
---
 tests/qtest/aspeed_gpio-test.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index bac63e8742..8f52454099 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -28,6 +28,11 @@
 #include "qapi/qmp/qdict.h"
 #include "libqtest-single.h"
 
+#define AST2600_GPIO_BASE 0x1E78
+
+#define GPIO_ABCD_DATA_VALUE 0x000
+#define GPIO_ABCD_DIRECTION  0x004
+
 static void test_set_colocated_pins(const void *data)
 {
 QTestState *s = (QTestState *)data;
@@ -46,6 +51,27 @@ static void test_set_colocated_pins(const void *data)
 g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
 }
 
+static void test_set_input_pins(const void *data)
+{
+QTestState *s = (QTestState *)data;
+char name[16];
+uint32_t value;
+
+qtest_writel(s, AST2600_GPIO_BASE + GPIO_ABCD_DIRECTION, 0x);
+for (char c = 'A'; c <= 'D'; c++) {
+for (int i = 0; i < 8; i++) {
+sprintf(name, "gpio%c%d", c, i);
+qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
+}
+}
+value = qtest_readl(s, AST2600_GPIO_BASE + GPIO_ABCD_DATA_VALUE);
+g_assert_cmphex(value, ==, 0x);
+
+qtest_writel(s, AST2600_GPIO_BASE + GPIO_ABCD_DATA_VALUE, 0x);
+value = qtest_readl(s, AST2600_GPIO_BASE + GPIO_ABCD_DATA_VALUE);
+g_assert_cmphex(value, ==, 0x);
+}
+
 int main(int argc, char **argv)
 {
 QTestState *s;
@@ -56,6 +82,7 @@ int main(int argc, char **argv)
 s = qtest_init("-machine ast2600-evb");
 qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
 test_set_colocated_pins);
+qtest_add_data_func("/ast2600/gpio/set_input_pins", s, 
test_set_input_pins);
 r = g_test_run();
 qtest_quit(s);
 
-- 
2.37.0




[RFC v4 9/9] qapi: add support for zoned host device

2022-07-11 Thread Sam Li
---
 block/file-posix.c   | 8 +++-
 qapi/block-core.json | 7 +--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index e9ad1d8e1e..4e0aa02acf 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3737,6 +3737,12 @@ static void hdev_parse_filename(const char *filename, 
QDict *options,
 bdrv_parse_filename_strip_prefix(filename, "host_device:", options);
 }
 
+static void zoned_host_device_parse_filename(const char *filename, QDict 
*options,
+Error **errp)
+{
+bdrv_parse_filename_strip_prefix(filename, "zoned_host_device:", options);
+}
+
 static bool hdev_is_sg(BlockDriverState *bs)
 {
 
@@ -3975,7 +3981,7 @@ static BlockDriver bdrv_zoned_host_device = {
 .is_zoned = true,
 .bdrv_needs_filename = true,
 .bdrv_probe_device  = hdev_probe_device,
-.bdrv_parse_filename = hdev_parse_filename,
+.bdrv_parse_filename = zoned_host_device_parse_filename,
 .bdrv_file_open = hdev_open,
 .bdrv_close = raw_close,
 .bdrv_reopen_prepare = raw_reopen_prepare,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2173e7734a..ab05c2ef99 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2955,7 +2955,8 @@
 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
 'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
 { 'name': 'replication', 'if': 'CONFIG_REPLICATION' },
-'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
+'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat',
+{ 'name': 'zoned_host_device', 'if': 'HAVE_HOST_BLOCK_DEVICE' } ] }
 
 ##
 # @BlockdevOptionsFile:
@@ -4329,7 +4330,9 @@
   'vhdx':   'BlockdevOptionsGenericFormat',
   'vmdk':   'BlockdevOptionsGenericCOWFormat',
   'vpc':'BlockdevOptionsGenericFormat',
-  'vvfat':  'BlockdevOptionsVVFAT'
+  'vvfat':  'BlockdevOptionsVVFAT',
+  'zoned_host_device': { 'type': 'BlockdevOptionsFile',
+ 'if': 'HAVE_HOST_BLOCK_DEVICE' }
   } }
 
 ##
-- 
2.36.1




[RFC v4 8/9] include: add support for zoned block devices

2022-07-11 Thread Sam Li
This is the virtio_blk.h header file from Dmitry's "virtio-blk: add
support for zoned block devices" patch. It introduces
virtio_blk_zoned_characteristics struct from Dmitry's virtio-blk zoned
storage spec.

Signed-off-by: Dmitry Fomichev 
Signed-off-by: Sam Li 
---
 include/standard-headers/linux/virtio_blk.h | 157 ++--
 1 file changed, 141 insertions(+), 16 deletions(-)

diff --git a/include/standard-headers/linux/virtio_blk.h 
b/include/standard-headers/linux/virtio_blk.h
index 2dcc90826a..f07fbe1b9b 100644
--- a/include/standard-headers/linux/virtio_blk.h
+++ b/include/standard-headers/linux/virtio_blk.h
@@ -25,10 +25,10 @@
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE. */
-#include "standard-headers/linux/types.h"
-#include "standard-headers/linux/virtio_ids.h"
-#include "standard-headers/linux/virtio_config.h"
-#include "standard-headers/linux/virtio_types.h"
+#include 
+#include 
+#include 
+#include 
 
 /* Feature bits */
 #define VIRTIO_BLK_F_SIZE_MAX  1   /* Indicates maximum segment size */
@@ -40,6 +40,7 @@
 #define VIRTIO_BLK_F_MQ12  /* support more than one vq */
 #define VIRTIO_BLK_F_DISCARD   13  /* DISCARD is supported */
 #define VIRTIO_BLK_F_WRITE_ZEROES  14  /* WRITE ZEROES is supported */
+#define VIRTIO_BLK_F_ZONED 17  /* Zoned block device */
 
 /* Legacy feature bits */
 #ifndef VIRTIO_BLK_NO_LEGACY
@@ -47,8 +48,10 @@
 #define VIRTIO_BLK_F_SCSI  7   /* Supports scsi command passthru */
 #define VIRTIO_BLK_F_FLUSH 9   /* Flush command supported */
 #define VIRTIO_BLK_F_CONFIG_WCE11  /* Writeback mode available in 
config */
+#ifndef __KERNEL__
 /* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */
 #define VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH
+#endif
 #endif /* !VIRTIO_BLK_NO_LEGACY */
 
 #define VIRTIO_BLK_ID_BYTES20  /* ID string length */
@@ -63,8 +66,8 @@ struct virtio_blk_config {
/* geometry of the device (if VIRTIO_BLK_F_GEOMETRY) */
struct virtio_blk_geometry {
__virtio16 cylinders;
-   uint8_t heads;
-   uint8_t sectors;
+   __u8 heads;
+   __u8 sectors;
} geometry;
 
/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
@@ -72,17 +75,17 @@ struct virtio_blk_config {
 
/* the next 4 entries are guarded by VIRTIO_BLK_F_TOPOLOGY  */
/* exponent for physical block per logical block. */
-   uint8_t physical_block_exp;
+   __u8 physical_block_exp;
/* alignment offset in logical blocks. */
-   uint8_t alignment_offset;
+   __u8 alignment_offset;
/* minimum I/O size without performance penalty in logical blocks. */
__virtio16 min_io_size;
/* optimal sustained I/O size in logical blocks. */
__virtio32 opt_io_size;
 
/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
-   uint8_t wce;
-   uint8_t unused;
+   __u8 wce;
+   __u8 unused;
 
/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
__virtio16 num_queues;
@@ -116,10 +119,24 @@ struct virtio_blk_config {
 * Set if a VIRTIO_BLK_T_WRITE_ZEROES request may result in the
 * deallocation of one or more of the sectors.
 */
-   uint8_t write_zeroes_may_unmap;
+   __u8 write_zeroes_may_unmap;
 
-   uint8_t unused1[3];
-} QEMU_PACKED;
+   __u8 unused1[3];
+
+   /* Secure erase fields that are defined in the virtio spec */
+   __u8 sec_erase[12];
+
+   /* Zoned block device characteristics (if VIRTIO_BLK_F_ZONED) */
+   struct virtio_blk_zoned_characteristics {
+   __virtio32 zone_sectors;
+   __virtio32 max_open_zones;
+   __virtio32 max_active_zones;
+   __virtio32 max_append_sectors;
+   __virtio32 write_granularity;
+   __u8 model;
+   __u8 unused2[3];
+   } zoned;
+} __attribute__((packed));
 
 /*
  * Command types
@@ -153,6 +170,24 @@ struct virtio_blk_config {
 /* Write zeroes command */
 #define VIRTIO_BLK_T_WRITE_ZEROES  13
 
+/* Zone append command */
+#define VIRTIO_BLK_T_ZONE_APPEND15
+
+/* Report zones command */
+#define VIRTIO_BLK_T_ZONE_REPORT16
+
+/* Open zone command */
+#define VIRTIO_BLK_T_ZONE_OPEN  18
+
+/* Close zone command */
+#define VIRTIO_BLK_T_ZONE_CLOSE 20
+
+/* Finish zone command */
+#define VIRTIO_BLK_T_ZONE_FINISH22
+
+/* Reset zone command */
+#define VIRTIO_BLK_T_ZONE_RESET 24
+
 #ifndef VIRTIO_BLK_NO_LEGACY
 /* Barrier before this op. */
 #define VIRTIO_BLK_T_BARRIER   0x8000
@@ -172,17 +207,100 @@ struct virtio_blk_outhdr {
__virtio64 sector;
 };
 
+/*
+ * Supported zoned device models.
+ */
+
+/* Host-managed zoned device */
+#define VIRTIO_BLK_Z_HM1
+/* Host-aware zoned device 

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

2022-07-11 Thread Richard Henderson

On 7/11/22 19:27, Peter Maydell wrote:

I don't have anything else queued up at the moment, so this is just
Richard's SME patches.

-- PMM

The following changes since commit 63b38f6c85acd312c2cab68554abf33adf4ee2b3:

   Merge tag 'pull-target-arm-20220707' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-07-08 
06:17:11 +0530)

are available in the Git repository at:

   https://git.linaro.org/people/pmaydell/qemu-arm.git 
tags/pull-target-arm-20220711

for you to fetch changes up to f9982ceaf26df27d15547a3a7990a95019e9e3a8:

   linux-user/aarch64: Add SME related hwcap entries (2022-07-11 13:43:52 +0100)


target-arm:
  * Implement SME emulation, for both system and linux-user


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as 
appropriate.


r~





Richard Henderson (45):
   target/arm: Handle SME in aarch64_cpu_dump_state
   target/arm: Add infrastructure for disas_sme
   target/arm: Trap non-streaming usage when Streaming SVE is active
   target/arm: Mark ADR as non-streaming
   target/arm: Mark RDFFR, WRFFR, SETFFR as non-streaming
   target/arm: Mark BDEP, BEXT, BGRP, COMPACT, FEXPA, FTSSEL as 
non-streaming
   target/arm: Mark PMULL, FMMLA as non-streaming
   target/arm: Mark FTSMUL, FTMAD, FADDA as non-streaming
   target/arm: Mark SMMLA, UMMLA, USMMLA as non-streaming
   target/arm: Mark string/histo/crypto as non-streaming
   target/arm: Mark gather/scatter load/store as non-streaming
   target/arm: Mark gather prefetch as non-streaming
   target/arm: Mark LDFF1 and LDNF1 as non-streaming
   target/arm: Mark LD1RO as non-streaming
   target/arm: Add SME enablement checks
   target/arm: Handle SME in sve_access_check
   target/arm: Implement SME RDSVL, ADDSVL, ADDSPL
   target/arm: Implement SME ZERO
   target/arm: Implement SME MOVA
   target/arm: Implement SME LD1, ST1
   target/arm: Export unpredicated ld/st from translate-sve.c
   target/arm: Implement SME LDR, STR
   target/arm: Implement SME ADDHA, ADDVA
   target/arm: Implement FMOPA, FMOPS (non-widening)
   target/arm: Implement BFMOPA, BFMOPS
   target/arm: Implement FMOPA, FMOPS (widening)
   target/arm: Implement SME integer outer product
   target/arm: Implement PSEL
   target/arm: Implement REVD
   target/arm: Implement SCLAMP, UCLAMP
   target/arm: Reset streaming sve state on exception boundaries
   target/arm: Enable SME for -cpu max
   linux-user/aarch64: Clear tpidr2_el0 if CLONE_SETTLS
   linux-user/aarch64: Reset PSTATE.SM on syscalls
   linux-user/aarch64: Add SM bit to SVE signal context
   linux-user/aarch64: Tidy target_restore_sigframe error return
   linux-user/aarch64: Do not allow duplicate or short sve records
   linux-user/aarch64: Verify extra record lock succeeded
   linux-user/aarch64: Move sve record checks into restore
   linux-user/aarch64: Implement SME signal handling
   linux-user: Rename sve prctls
   linux-user/aarch64: Implement PR_SME_GET_VL, PR_SME_SET_VL
   target/arm: Only set ZEN in reset if SVE present
   target/arm: Enable SME for user-only
   linux-user/aarch64: Add SME related hwcap entries

  docs/system/arm/emulation.rst |4 +
  linux-user/aarch64/target_cpu.h   |5 +-
  linux-user/aarch64/target_prctl.h |   62 +-
  target/arm/cpu.h  |7 +
  target/arm/helper-sme.h   |  126 
  target/arm/helper-sve.h   |4 +
  target/arm/helper.h   |   18 +
  target/arm/translate-a64.h|   45 ++
  target/arm/translate.h|   16 +
  target/arm/sme-fa64.decode|   60 ++
  target/arm/sme.decode |   88 +++
  target/arm/sve.decode |   41 +-
  linux-user/aarch64/cpu_loop.c |9 +
  linux-user/aarch64/signal.c   |  243 ++--
  linux-user/elfload.c  |   20 +
  linux-user/syscall.c  |   28 +-
  target/arm/cpu.c  |   35 +-
  target/arm/cpu64.c|   11 +
  target/arm/helper.c   |   56 +-
  target/arm/sme_helper.c   | 1140 +
  target/arm/sve_helper.c   |   28 +
  target/arm/translate-a64.c|  103 +++-
  target/arm/translate-sme.c|  373 
  target/arm/translate-sve.c|  393 ++---
  target/arm/translate-vfp.c|   12 +
  target/arm/translate.c|2 +
  target/arm/vec_helper.c   |   24 +
  target/arm/meson.build|3 +
  28 files changed, 2821 insertions(+), 135 deletions(-)
  create mode 100644 target/arm/sme-fa64.decode
  create mode 100644 target/arm/sme.decode
  create mode 100644 target/arm/translate-sme.c






[RFC v4 6/9] raw-format: add zone operations

2022-07-11 Thread Sam Li
Signed-off-by: Sam Li 
---
 block/raw-format.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/block/raw-format.c b/block/raw-format.c
index 69fd650eaf..96bdb6c1e2 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -314,6 +314,17 @@ static int coroutine_fn raw_co_pdiscard(BlockDriverState 
*bs,
 return bdrv_co_pdiscard(bs->file, offset, bytes);
 }
 
+static int coroutine_fn raw_co_zone_report(BlockDriverState *bs, int64_t 
offset,
+   int64_t *nr_zones,
+   BlockZoneDescriptor *zones) {
+return bdrv_co_zone_report(bs->file->bs, offset, nr_zones, zones);
+}
+
+static int coroutine_fn raw_co_zone_mgmt(BlockDriverState *bs, zone_op op,
+ int64_t offset, int64_t len) {
+return bdrv_co_zone_mgmt(bs->file->bs, op, offset, len);
+}
+
 static int64_t raw_getlength(BlockDriverState *bs)
 {
 int64_t len;
@@ -614,6 +625,8 @@ BlockDriver bdrv_raw = {
 .bdrv_co_pwritev  = _co_pwritev,
 .bdrv_co_pwrite_zeroes = _co_pwrite_zeroes,
 .bdrv_co_pdiscard = _co_pdiscard,
+.bdrv_co_zone_report  = _co_zone_report,
+.bdrv_co_zone_mgmt  = _co_zone_mgmt,
 .bdrv_co_block_status = _co_block_status,
 .bdrv_co_copy_range_from = _co_copy_range_from,
 .bdrv_co_copy_range_to  = _co_copy_range_to,
-- 
2.36.1




[RFC v4 7/9] config: add check to block layer

2022-07-11 Thread Sam Li
Putting zoned/non-zoned BlockDrivers on top of each other is not
allowed.

Signed-off-by: Sam Li 
---
 block.c  | 7 +++
 block/file-posix.c   | 2 ++
 include/block/block_int-common.h | 5 +
 3 files changed, 14 insertions(+)

diff --git a/block.c b/block.c
index 2c0080..0e24582c7d 100644
--- a/block.c
+++ b/block.c
@@ -7945,6 +7945,13 @@ void bdrv_add_child(BlockDriverState *parent_bs, 
BlockDriverState *child_bs,
 return;
 }
 
+if (parent_bs->drv->is_zoned != child_bs->drv->is_zoned) {
+error_setg(errp, "Cannot add a %s child to a %s parent",
+   child_bs->drv->is_zoned ? "zoned" : "non-zoned",
+   parent_bs->drv->is_zoned ? "zoned" : "non-zoned");
+return;
+}
+
 if (!QLIST_EMPTY(_bs->parents)) {
 error_setg(errp, "The node %s already has a parent",
child_bs->node_name);
diff --git a/block/file-posix.c b/block/file-posix.c
index 42708012ff..e9ad1d8e1e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3924,6 +3924,7 @@ static BlockDriver bdrv_host_device = {
 .format_name= "host_device",
 .protocol_name= "host_device",
 .instance_size  = sizeof(BDRVRawState),
+.is_zoned = false,
 .bdrv_needs_filename = true,
 .bdrv_probe_device  = hdev_probe_device,
 .bdrv_parse_filename = hdev_parse_filename,
@@ -3971,6 +3972,7 @@ static BlockDriver bdrv_zoned_host_device = {
 .format_name = "zoned_host_device",
 .protocol_name = "zoned_host_device",
 .instance_size = sizeof(BDRVRawState),
+.is_zoned = true,
 .bdrv_needs_filename = true,
 .bdrv_probe_device  = hdev_probe_device,
 .bdrv_parse_filename = hdev_parse_filename,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 6037871089..29f1ec9184 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -141,6 +141,11 @@ struct BlockDriver {
  */
 bool is_format;
 
+/*
+ * Set to true if the BlockDriver is a zoned block driver.
+ */
+bool is_zoned;
+
 /*
  * Drivers not implementing bdrv_parse_filename nor bdrv_open should have
  * this field set to true, except ones that are defined only by their
-- 
2.36.1




[RFC v4 5/9] qemu-iotests: test new zone operations.

2022-07-11 Thread Sam Li
We have added new block layer APIs of zoned block devices. Test it with:
(1) Create a null_blk device, run each zone operation on it and see
whether reporting right zone information.

Signed-off-by: Sam Li 
---
 tests/qemu-iotests/tests/zoned.sh | 69 +++
 1 file changed, 69 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/zoned.sh

diff --git a/tests/qemu-iotests/tests/zoned.sh 
b/tests/qemu-iotests/tests/zoned.sh
new file mode 100755
index 00..e14a3a420e
--- /dev/null
+++ b/tests/qemu-iotests/tests/zoned.sh
@@ -0,0 +1,69 @@
+#!/usr/bin/env bash
+#
+# Test zone management operations.
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+status=1 # failure is the default!
+
+_cleanup()
+{
+  _cleanup_test_img
+  sudo rmmod null_blk
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# This test only runs on Linux hosts with raw image files.
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+QEMU_IO="build/qemu-io"
+IMG="--image-opts driver=zoned_host_device,filename=/dev/nullb0"
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+
+echo "Testing a null_blk device"
+echo "Simple cases: if the operations work"
+sudo modprobe null_blk nr_devices=1 zoned=1
+# hidden issues:
+# 1. memory allocation error of "unaligned tcache chunk detected" when the 
nr_zone=1 in zone report
+# 2. qemu-io: after report 10 zones, the program failed at double free error 
and exited.
+echo "report the first zone"
+sudo $QEMU_IO $IMG -c "zr 0 0 1"
+echo "report: the first 10 zones"
+sudo $QEMU_IO $IMG -c "zr 0 0 10"
+
+echo "open the first zone"
+sudo $QEMU_IO $IMG -c "zo 0 0x8"
+echo "report after:"
+sudo $QEMU_IO $IMG -c "zr 0 0 1"
+echo "open the last zone"
+sudo $QEMU_IO $IMG -c "zo 0x3e7000 0x8"
+echo "report after:"
+sudo $QEMU_IO $IMG -c "zr 0x3e7000 0 2"
+
+echo "close the first zone"
+sudo $QEMU_IO $IMG -c "zc 0 0x8"
+echo "report after:"
+sudo $QEMU_IO $IMG -c "zr 0 0 1"
+echo "close the last zone"
+sudo $QEMU_IO $IMG -c "zc 0x3e7000 0x8"
+echo "report after:"
+sudo $QEMU_IO $IMG -c "zr 0x3e7000 0 2"
+
+
+echo "reset the second zone"
+sudo $QEMU_IO $IMG -c "zrs 0x8 0x8"
+echo "After resetting a zone:"
+sudo $QEMU_IO $IMG -c "zr 0x8 0 5"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
-- 
2.36.1




[RFC v4 4/9] file-posix: introduce get_sysfs_str_val for device zoned model.

2022-07-11 Thread Sam Li
Signed-off-by: Sam Li 
---
 block/file-posix.c   | 60 
 include/block/block-common.h |  4 +--
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 3161d39ea4..42708012ff 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1279,6 +1279,65 @@ out:
 #endif
 }
 
+/*
+ * Convert the zoned attribute file in sysfs to internal value.
+ */
+static zone_model get_sysfs_str_val(int fd, struct stat *st) {
+#ifdef CONFIG_LINUX
+char buf[32];
+char *sysfspath = NULL;
+int ret, offset;
+int sysfd = -1;
+
+if (S_ISCHR(st->st_mode)) {
+if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
+return ret;
+}
+return -ENOTSUP;
+}
+
+if (!S_ISBLK(st->st_mode)) {
+return -ENOTSUP;
+}
+
+sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/zoned",
+major(st->st_rdev), minor(st->st_rdev));
+sysfd = open(sysfspath, O_RDONLY);
+if (sysfd == -1) {
+ret = -errno;
+goto out;
+}
+offset = 0;
+do {
+ret = read(sysfd, buf + offset, sizeof(buf) - 1 + offset);
+if (ret > 0) {
+offset += ret;
+}
+} while (ret == -1);
+/* The file is ended with '\n' */
+if (buf[ret - 1] == '\n') {
+buf[ret - 1] = '\0';
+}
+
+if (strcmp(buf, "host-managed") == 0) {
+return BLK_Z_HM;
+} else if (strcmp(buf, "host-aware") == 0) {
+return BLK_Z_HA;
+} else {
+return -ENOTSUP;
+}
+
+out:
+if (sysfd != -1) {
+close(sysfd);
+}
+g_free(sysfspath);
+return ret;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static int hdev_get_max_segments(int fd, struct stat *st) {
 return get_sysfs_long_val(fd, st, "max_segments");
 }
@@ -1885,6 +1944,7 @@ static int handle_aiocb_zone_mgmt(void *opaque) {
 int64_t len = aiocb->aio_nbytes;
 zone_op op = aiocb->zone_mgmt.op;
 
+zone_model mod;
 struct blk_zone_range range;
 const char *ioctl_name;
 unsigned long ioctl_op;
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 78cddeeda5..35e00afe8e 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -56,8 +56,8 @@ typedef enum zone_op {
 } zone_op;
 
 typedef enum zone_model {
-BLK_Z_HM,
-BLK_Z_HA,
+BLK_Z_HM = 0x1,
+BLK_Z_HA = 0x2,
 } zone_model;
 
 typedef enum BlkZoneCondition {
-- 
2.36.1




[RFC v4 2/9] qemu-io: add zoned block device operations.

2022-07-11 Thread Sam Li
Add zoned storage commands of the device: zone_open(zo), zone_close(zc),
zone_reset(zs), zone_report(zp), zone_finish(zf).

For example, it can be called by:
./build/qemu-io --image-opts driver=zoned_host_device, filename=/dev/nullb0
-c "zone_report 0 0 1"

Signed-off-by: Sam Li 
---
 block/io.c   |  57 
 include/block/block-io.h |  13 
 qemu-io-cmds.c   | 143 +++
 3 files changed, 213 insertions(+)

diff --git a/block/io.c b/block/io.c
index 1e9bf09a49..a760be0131 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3243,6 +3243,63 @@ out:
 return co.ret;
 }
 
+int bdrv_co_zone_report(BlockDriverState *bs, int64_t offset,
+int64_t *nr_zones,
+BlockZoneDescriptor *zones)
+{
+BlockDriver *drv = bs->drv;
+CoroutineIOCompletion co = {
+.coroutine = qemu_coroutine_self(),
+};
+IO_CODE();
+
+bdrv_inc_in_flight(bs);
+if (!drv || (!drv->bdrv_co_zone_report)) {
+co.ret = -ENOTSUP;
+goto out;
+}
+
+if (drv->bdrv_co_zone_report) {
+co.ret = drv->bdrv_co_zone_report(bs, offset, nr_zones, zones);
+} else {
+co.ret = -ENOTSUP;
+goto out;
+qemu_coroutine_yield();
+}
+
+out:
+bdrv_dec_in_flight(bs);
+return co.ret;
+}
+
+int bdrv_co_zone_mgmt(BlockDriverState *bs, enum zone_op op,
+int64_t offset, int64_t len)
+{
+BlockDriver *drv = bs->drv;
+CoroutineIOCompletion co = {
+.coroutine = qemu_coroutine_self(),
+};
+IO_CODE();
+
+bdrv_inc_in_flight(bs);
+if (!drv || (!drv->bdrv_co_zone_mgmt)) {
+co.ret = -ENOTSUP;
+goto out;
+}
+
+if (drv->bdrv_co_zone_mgmt) {
+co.ret = drv->bdrv_co_zone_mgmt(bs, op, offset, len);
+} else {
+co.ret = -ENOTSUP;
+goto out;
+qemu_coroutine_yield();
+}
+
+out:
+bdrv_dec_in_flight(bs);
+return co.ret;
+}
+
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
 IO_CODE();
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 053a27141a..a0ae140452 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -80,6 +80,13 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf);
 /* Ensure contents are flushed to disk.  */
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
 
+/* Report zone information of zone block device. */
+int coroutine_fn bdrv_co_zone_report(BlockDriverState *bs, int64_t offset,
+ int64_t *nr_zones,
+ BlockZoneDescriptor *zones);
+int coroutine_fn bdrv_co_zone_mgmt(BlockDriverState *bs, zone_op op,
+   int64_t offset, int64_t len);
+
 int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 int bdrv_block_status(BlockDriverState *bs, int64_t offset,
@@ -289,6 +296,12 @@ bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector 
*qiov, int64_t pos);
 int generated_co_wrapper
 bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
 
+int generated_co_wrapper
+blk_zone_report(BlockBackend *blk, int64_t offset, int64_t *nr_zones,
+BlockZoneDescriptor *zones);
+int generated_co_wrapper
+blk_zone_mgmt(BlockBackend *blk, enum zone_op op, int64_t offset, int64_t len);
+
 /**
  * bdrv_parent_drained_begin_single:
  *
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 2f0d8ac25a..a88fa322d2 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1706,6 +1706,144 @@ static const cmdinfo_t flush_cmd = {
 .oneline= "flush all in-core file state to disk",
 };
 
+static int zone_report_f(BlockBackend *blk, int argc, char **argv)
+{
+int ret;
+int64_t offset, nr_zones;
+
+++optind;
+offset = cvtnum(argv[optind]);
+++optind;
+nr_zones = cvtnum(argv[optind]);
+
+g_autofree BlockZoneDescriptor *zones = NULL;
+zones = g_new(BlockZoneDescriptor, nr_zones);
+ret = blk_zone_report(blk, offset, _zones, zones);
+if (ret < 0) {
+printf("zone report failed: %s\n", strerror(-ret));
+} else {
+for (int i = 0; i < nr_zones; ++i) {
+printf("start: 0x%" PRIx64 ", len 0x%" PRIx64 ", "
+   "cap"" 0x%" PRIx64 ",wptr 0x%" PRIx64 ", "
+   "zcond:%u, [type: %u]\n",
+   zones[i].start, zones[i].length, zones[i].cap, zones[i].wp,
+   zones[i].cond, zones[i].type);
+}
+}
+return ret;
+}
+
+static const cmdinfo_t zone_report_cmd = {
+.name = "zone_report",
+.altname = "zp",
+.cfunc = zone_report_f,
+.argmin = 2,
+.argmax = 2,
+.args = "offset number",
+.oneline = "report zone information",
+};
+
+static int zone_open_f(BlockBackend *blk, int argc, char **argv)
+{
+int ret;
+int64_t offset, len;
+

[RFC v4 0/9] Add support for zoned device

2022-07-11 Thread Sam Li
This patch series adds support for zoned device to virtio-blk emulation. Zoned
Storage can support sequential writes, which reduces write amplification in SSD,
leading to higher write throughput and increased capacity.

v4:
- add block layer APIs (revision)
- add configurations for zoned block device

Sam Li (9):
  block: add block layer APIs resembling Linux ZonedBlockDevice ioctls.
  qemu-io: add zoned block device operations.
  file-posix: introduce get_sysfs_long_val for a block queue of sysfs
attribute
  file-posix: introduce get_sysfs_str_val for device zoned model.
  qemu-iotests: test new zone operations.
  raw-format: add zone operations
  config: add check to block layer
  include: add support for zoned block devices
  qapi: add support for zoned host device

 block.c |   7 +
 block/block-backend.c   |  41 +++
 block/coroutines.h  |   5 +
 block/file-posix.c  | 334 +++-
 block/io.c  |  57 
 block/raw-format.c  |  13 +
 include/block/block-common.h|  43 ++-
 include/block/block-io.h|  13 +
 include/block/block_int-common.h|  25 ++
 include/standard-headers/linux/virtio_blk.h | 157 -
 qapi/block-core.json|   7 +-
 qemu-io-cmds.c  | 143 +
 tests/qemu-iotests/tests/zoned.sh   |  69 
 13 files changed, 888 insertions(+), 26 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/zoned.sh

-- 
2.36.1




[RFC v4 1/9] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls.

2022-07-11 Thread Sam Li
By adding zone management operations in BlockDriver, storage
controller emulation can use the new block layer APIs including
zone_report and zone_mgmt(open, close, finish, reset).

Signed-off-by: Sam Li 
---
 block/block-backend.c|  41 ++
 block/coroutines.h   |   5 +
 block/file-posix.c   | 236 +++
 include/block/block-common.h |  43 +-
 include/block/block_int-common.h |  20 +++
 5 files changed, 344 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index f425b00793..0a05247ae4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1806,6 +1806,47 @@ int blk_flush(BlockBackend *blk)
 return ret;
 }
 
+/*
+ * Send a zone_report command.
+ * offset can be any number within the zone size. No alignment for offset.
+ * nr_zones represents IN maximum and OUT actual.
+ */
+int coroutine_fn blk_co_zone_report(BlockBackend *blk, int64_t offset,
+int64_t *nr_zones,
+BlockZoneDescriptor *zones)
+{
+int ret;
+IO_CODE();
+
+blk_inc_in_flight(blk); /* increase before waiting */
+blk_wait_while_drained(blk);
+ret = bdrv_co_zone_report(blk->root->bs, offset, nr_zones, zones);
+blk_dec_in_flight(blk);
+return ret;
+}
+
+/*
+ * Send a zone_management command.
+ * Offset is the start of a zone and len is aligned to zones.
+ */
+int coroutine_fn blk_co_zone_mgmt(BlockBackend *blk, enum zone_op op,
+int64_t offset, int64_t len)
+{
+int ret;
+IO_CODE();
+
+blk_inc_in_flight(blk);
+blk_wait_while_drained(blk);
+ret = blk_check_byte_request(blk, offset, len);
+if (ret < 0) {
+return ret;
+}
+
+ret = bdrv_co_zone_mgmt(blk->root->bs, op, offset, len);
+blk_dec_in_flight(blk);
+return ret;
+}
+
 void blk_drain(BlockBackend *blk)
 {
 BlockDriverState *bs = blk_bs(blk);
diff --git a/block/coroutines.h b/block/coroutines.h
index 830ecaa733..19aa96cc56 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -80,6 +80,11 @@ int coroutine_fn
 blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes);
 
 int coroutine_fn blk_co_do_flush(BlockBackend *blk);
+int coroutine_fn blk_co_zone_report(BlockBackend *blk, int64_t offset,
+int64_t *nr_zones,
+BlockZoneDescriptor *zones);
+int coroutine_fn blk_co_zone_mgmt(BlockBackend *blk, enum zone_op op,
+  int64_t offset, int64_t len);
 
 
 /*
diff --git a/block/file-posix.c b/block/file-posix.c
index 48cd096624..e7523ae2ed 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -67,6 +67,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -216,6 +217,13 @@ typedef struct RawPosixAIOData {
 PreallocMode prealloc;
 Error **errp;
 } truncate;
+struct {
+int64_t *nr_zones;
+BlockZoneDescriptor *zones;
+} zone_report;
+struct {
+zone_op op;
+} zone_mgmt;
 };
 } RawPosixAIOData;
 
@@ -1801,6 +1809,130 @@ static off_t copy_file_range(int in_fd, off_t *in_off, 
int out_fd,
 }
 #endif
 
+/*
+ * parse_zone - Fill a zone descriptor
+ */
+static inline void parse_zone(struct BlockZoneDescriptor *zone,
+  struct blk_zone *blkz) {
+zone->start = blkz->start;
+zone->length = blkz->len;
+zone->cap = blkz->capacity;
+zone->wp = blkz->wp - blkz->start;
+zone->type = blkz->type;
+zone->cond = blkz->cond;
+}
+
+static int handle_aiocb_zone_report(void *opaque) {
+RawPosixAIOData *aiocb = opaque;
+int fd = aiocb->aio_fildes;
+int64_t *nr_zones = aiocb->zone_report.nr_zones;
+BlockZoneDescriptor *zones = aiocb->zone_report.zones;
+int64_t offset = aiocb->aio_offset;
+
+struct blk_zone *blkz;
+int64_t rep_size, nrz;
+int ret, n = 0, i = 0;
+
+nrz = *nr_zones;
+rep_size = sizeof(struct blk_zone_report) + nrz * sizeof(struct blk_zone);
+g_autofree struct blk_zone_report *rep = NULL;
+rep = g_malloc(rep_size);
+offset = offset / 512; /* get the unit of the start sector: sector size is 
512 bytes. */
+printf("start to report zone with offset: 0x%lx\n", offset);
+
+blkz = (struct blk_zone *)(rep + 1);
+while (n < nrz) {
+memset(rep, 0, rep_size);
+rep->sector = offset;
+rep->nr_zones = nrz;
+
+ret = ioctl(fd, BLKREPORTZONE, rep);
+if (ret != 0) {
+ret = -errno;
+error_report("%d: ioctl BLKREPORTZONE at %ld failed %d",
+ fd, offset, errno);
+return ret;
+}
+
+if (!rep->nr_zones) {
+break;
+}
+
+for (i = 0; i < rep->nr_zones; i++, n++) {
+parse_zone([n], [i]);
+/* The next report should start after the 

[PATCH 2/2] i386/cpuid: Remove subleaf constraint on CPUID leaf 1F

2022-07-11 Thread Xiaoyao Li
No such constraint that subleaf index needs to be less than 64.

Signed-off-by: Xiaoyao Li 
---
 target/i386/kvm/kvm.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a0b412a1129f..3efa524b4b93 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1843,10 +1843,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
 break;
 }
 
-if (i == 0x1f && j == 64) {
-break;
-}
-
 c->function = i;
 c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
 c->index = j;
-- 
2.27.0




[RFC v4 3/9] file-posix: introduce get_sysfs_long_val for a block queue of sysfs attribute

2022-07-11 Thread Sam Li
Use sysfs attribute files to get the zoned device information in case
that ioctl() commands of zone management interface won't work.

Signed-off-by: Sam Li 
---
 block/file-posix.c | 38 +++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index e7523ae2ed..3161d39ea4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1218,15 +1218,19 @@ static int hdev_get_max_hw_transfer(int fd, struct stat 
*st)
 #endif
 }
 
-static int hdev_get_max_segments(int fd, struct stat *st)
-{
+/*
+ * Get zoned device information (chunk_sectors, zoned_append_max_bytes,
+ * max_open_zones, max_active_zones) through sysfs attribute files.
+ */
+static long get_sysfs_long_val(int fd, struct stat *st,
+   const char *attribute) {
 #ifdef CONFIG_LINUX
 char buf[32];
 const char *end;
 char *sysfspath = NULL;
 int ret;
 int sysfd = -1;
-long max_segments;
+long val;
 
 if (S_ISCHR(st->st_mode)) {
 if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
@@ -1239,8 +1243,9 @@ static int hdev_get_max_segments(int fd, struct stat *st)
 return -ENOTSUP;
 }
 
-sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
-major(st->st_rdev), minor(st->st_rdev));
+sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s",
+major(st->st_rdev), minor(st->st_rdev),
+attribute);
 sysfd = open(sysfspath, O_RDONLY);
 if (sysfd == -1) {
 ret = -errno;
@@ -1258,9 +1263,9 @@ static int hdev_get_max_segments(int fd, struct stat *st)
 }
 buf[ret] = 0;
 /* The file is ended with '\n', pass 'end' to accept that. */
-ret = qemu_strtol(buf, , 10, _segments);
+ret = qemu_strtol(buf, , 10, );
 if (ret == 0 && end && *end == '\n') {
-ret = max_segments;
+ret = val;
 }
 
 out:
@@ -1274,6 +1279,10 @@ out:
 #endif
 }
 
+static int hdev_get_max_segments(int fd, struct stat *st) {
+return get_sysfs_long_val(fd, st, "max_segments");
+}
+
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -1883,10 +1892,17 @@ static int handle_aiocb_zone_mgmt(void *opaque) {
 int64_t zone_size_mask;
 int ret;
 
-g_autofree struct stat *file = NULL;
-file = g_new(struct stat, 1);
-stat(s->filename, file);
-zone_size = get_sysfs_long_val(fd, file, "chunk_sectors");
+struct stat file;
+if (fstat(fd, ) < 0) {
+return -errno;
+}
+mod = get_sysfs_str_val(fd, );
+if (mod != BLK_Z_HM) {
+ret = -ENOTSUP;
+return ret;
+}
+
+zone_size = get_sysfs_long_val(fd, , "chunk_sectors");
 zone_size_mask = zone_size - 1;
 if (offset & zone_size_mask) {
 error_report("offset is not the start of a zone");
-- 
2.36.1




[PATCH 0/2] i386/cpuid: Minor fixes for CPUID leaf 1f setup

2022-07-11 Thread Xiaoyao Li
The issue that fixed by Patch 1 looks fatal though it doesn't appear on
KVM because KVM always searches with assending order and hit with the
correct cpuid leaf 0.

Patch 2 removes the wrong constraint on CPUID leaf 1f

Xiaoyao Li (2):
  i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F
  i386/cpuid: Remove subleaf constraint on CPUID leaf 1F

 target/i386/kvm/kvm.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

-- 
2.27.0




[PATCH 1/2] i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F

2022-07-11 Thread Xiaoyao Li
Decrease array index cpuid_i when CPUID leaf 1F is skipped, otherwise it
will get an all zero'ed CPUID entry with leaf 0 and subleaf 0. It
conflicts with correct leaf 0.

Signed-off-by: Xiaoyao Li 
---
 target/i386/kvm/kvm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index b8578882c12b..a0b412a1129f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1831,6 +1831,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 case 0x1f:
 if (env->nr_dies < 2) {
+cpuid_i--;
 break;
 }
 /* fallthrough */
-- 
2.27.0




[PATCH 3/3] tests/tcg/s390x: test signed vfmin/vfmax

2022-07-11 Thread Ilya Leoshkevich
Add a test to prevent regressions. Try all floating point value sizes
and all combinations of floating point value classes. Verify the results
against PoP tables, which are represented as close to the original as
possible - this produces a lot of checkpatch complaints, but it seems
to be justified in this case.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.target |   7 +
 tests/tcg/s390x/vfminmax.c  | 426 
 2 files changed, 433 insertions(+)
 create mode 100644 tests/tcg/s390x/vfminmax.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 3124172736..1a7a4a2f59 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -17,6 +17,13 @@ TESTS+=trap
 TESTS+=signals-s390x
 TESTS+=branch-relative-long
 
+Z14_TESTS=vfminmax
+vfminmax: LDFLAGS+=-lm
+$(Z14_TESTS): CFLAGS+=-march=z14 -O2
+
+TESTS+=$(if $(shell $(CC) -march=z14 -S -o /dev/null -xc /dev/null \
+>/dev/null 2>&1 && echo OK),$(Z14_TESTS))
+
 VECTOR_TESTS=vxeh2_vs
 VECTOR_TESTS+=vxeh2_vcvt
 VECTOR_TESTS+=vxeh2_vlstr
diff --git a/tests/tcg/s390x/vfminmax.c b/tests/tcg/s390x/vfminmax.c
new file mode 100644
index 00..daf58b6b33
--- /dev/null
+++ b/tests/tcg/s390x/vfminmax.c
@@ -0,0 +1,426 @@
+#define _GNU_SOURCE
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * vfmin/vfmax code generation.
+ */
+extern const char vfminmax_template[];
+extern const int vfminmax_template_size;
+extern const int vfminmax_offset;
+asm(".globl vfminmax_template\n"
+"vfminmax_template:\n"
+"vl %v25,0(%r3)\n"
+"vl %v26,0(%r4)\n"
+"0: vfmax %v24,%v25,%v26,2,0,0\n"
+"vst %v24,0(%r2)\n"
+"br %r14\n"
+"1: .align 4\n"
+".globl vfminmax_template_size\n"
+"vfminmax_template_size: .long 1b - vfminmax_template\n"
+".globl vfminmax_offset\n"
+"vfminmax_offset: .long 0b - vfminmax_template\n");
+
+#define VFMIN 0xEE
+#define VFMAX 0xEF
+
+static void vfminmax(unsigned char *buf, unsigned int op,
+ unsigned int m4, unsigned int m5, unsigned int m6,
+ void *v1, const void *v2, const void *v3)
+{
+memcpy(buf, vfminmax_template, vfminmax_template_size);
+buf[vfminmax_offset + 3] = (m6 << 4) | m5;
+buf[vfminmax_offset + 4] &= 0x0F;
+buf[vfminmax_offset + 4] |= (m4 << 4);
+buf[vfminmax_offset + 5] = op;
+((void (*)(void *, const void *, const void *))buf)(v1, v2, v3);
+}
+
+/*
+ * Floating-point value classes.
+ */
+#define N_FORMATS 3
+#define N_SIGNED_CLASSES 8
+static const size_t float_sizes[N_FORMATS] = {
+/* M4 == 2: short*/ 4,
+/* M4 == 3: long */ 8,
+/* M4 == 4: extended */ 16,
+};
+static const size_t e_bits[N_FORMATS] = {
+/* M4 == 2: short*/ 8,
+/* M4 == 3: long */ 11,
+/* M4 == 4: extended */ 15,
+};
+static const unsigned char signed_floats[N_FORMATS][N_SIGNED_CLASSES][2][16] = 
{
+/* M4 == 2: short */
+{
+/* -inf */ {{0xff, 0x80, 0x00, 0x00},
+{0xff, 0x80, 0x00, 0x00}},
+/* -Fn */  {{0xc2, 0x28, 0x00, 0x00},
+{0xc2, 0x29, 0x00, 0x00}},
+/* -0 */   {{0x80, 0x00, 0x00, 0x00},
+{0x80, 0x00, 0x00, 0x00}},
+/* +0 */   {{0x00, 0x00, 0x00, 0x00},
+{0x00, 0x00, 0x00, 0x00}},
+/* +Fn */  {{0x42, 0x28, 0x00, 0x00},
+{0x42, 0x2a, 0x00, 0x00}},
+/* +inf */ {{0x7f, 0x80, 0x00, 0x00},
+{0x7f, 0x80, 0x00, 0x00}},
+/* QNaN */ {{0x7f, 0xff, 0xff, 0xff},
+{0x7f, 0xff, 0xff, 0xfe}},
+/* SNaN */ {{0x7f, 0xbf, 0xff, 0xff},
+{0x7f, 0xbf, 0xff, 0xfd}},
+},
+
+/* M4 == 3: long */
+{
+/* -inf */ {{0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* -Fn */  {{0xc0, 0x45, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0xc0, 0x46, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* -0 */   {{0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* +0 */   {{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* +Fn */  {{0x40, 0x45, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0x40, 0x47, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* +inf */ {{0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+{0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
+/* QNaN */ {{0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
+{0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe}},
+/* SNaN */ {{0x7f, 0xf7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
+{0x7f, 0xf7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfd}},
+},
+
+/* M4 == 4: extended */
+{
+/* 

Re: [PATCH 1/2] hw/gpio/aspeed: Don't let guests modify input pins

2022-07-11 Thread Peter Delevoryas
On Mon, Jul 11, 2022 at 10:56:08PM +0930, Andrew Jeffery wrote:
> 
> 
> On Fri, 8 Jul 2022, at 04:34, Peter Delevoryas wrote:
> > On Thu, Jul 07, 2022 at 10:53:57AM -0700, Peter Delevoryas wrote:
> >> On Thu, Jul 07, 2022 at 10:56:02AM +0200, Cédric Le Goater wrote:
> >> > On 7/7/22 09:17, Peter Delevoryas wrote:
> >> > > It seems that aspeed_gpio_update is allowing the value for input pins 
> >> > > to be
> >> > > modified through register writes and QOM property modification.
> >> > > 
> >> > > The QOM property modification is fine, but modifying the value through
> >> > > register writes from the guest OS seems wrong if the pin's direction 
> >> > > is set
> >> > > to input.
> >> > > 
> >> > > The datasheet specifies that "0" bits in the direction register select 
> >> > > input
> >> > > mode, and "1" selects output mode.
> >> > > 
> >> > > OpenBMC userspace code is accidentally writing 0's to the GPIO data
> >> > > registers somewhere (or perhaps the driver is doing it through a reset 
> >> > > or
> >> > > something), and this is overwriting GPIO FRU information (board ID, 
> >> > > slot
> >> > > presence pins) that is initialized in Aspeed machine reset code (see
> >> > > fby35_reset() in hw/arm/aspeed.c).
> >> > 
> >> > It might be good to log a GUEST_ERROR in that case, when writing to an
> >> > input GPIO and when reading from an output GPIO.
> >> 
> >> Good idea, I'll include a GUEST_ERROR for writing to an input GPIO.
> >> 
> >> I'm actually not totally certain about emitting an error when reading from 
> >> an
> >> output GPIO, because the driver can only do 8-bit reads at the finest
> >> granularity, and if 1 of the 8 pins' direction is output, then it will be
> >> reading the value of an output pin. But, that's not really bad, because
> >> presumably the value will be ignored. Maybe I can go test this out on
> >> hardware and figure out what happens though.
> >
> > Did a small experiment, I was looking at some of the most significant
> > bits:
> >
> > root@dhcp-100-96-192-133:~# devmem 0x1e78
> > 0x3CFF303E
> > root@dhcp-100-96-192-133:~# devmem 0x1e780004
> > 0x280C
> > root@dhcp-100-96-192-133:~# devmem 0x1e78 32 0x
> > root@dhcp-100-96-192-133:~# devmem 0x1e780004
> > 0x280C
> > root@dhcp-100-96-192-133:~# devmem 0x1e78
> > 0x3CFF303E
> > root@dhcp-100-96-192-133:~# devmem 0x1e78
> > 0x3CFF303E
> > root@dhcp-100-96-192-133:~# devmem 0x1e78 32 0
> > root@dhcp-100-96-192-133:~# devmem 0x1e78
> > 0x14FF303A
> >
> > Seems like the output pin 0x2000 was initially high, and the input
> > pin right next to it 0x1000 was also high. After writing 0 to the
> > data register, the value in the data register changed for the output
> > pin, but not the input pin.  Which matches what we're planning on doing
> > in the controller.
> >
> > So yeah, I'll add GUEST_ERROR for writes to input pins but not output
> > pins.  The driver should probably be doing a read-modify-update.
> > Although...if it's not, that technically wouldn't matter, behavior
> > wise...maybe GUEST_ERROR isn't appropriate for writes to input pins
> > either, for the same reason as I mentioned with reads of output pins.
> > I'll let you guys comment on what you think we should do.
> >
> 
> With the quick hack below I think I got sensible behaviour?
> 
> ```
> # devmem 0x1e78
> 0x
> # devmem 0x1e780004
> 0x
> # devmem 0x1e780004 32 1
> # devmem 0x1e78 32 1
> # devmem 0x1e78
> 0x0001
> # devmem 0x1e78 32 3
> # devmem 0x1e78
> 0x0001
> # QEMU 7.0.0 monitor - type 'help' for more information
> (qemu) qom-set gpio gpioA1 on
> (qemu) 
> 
> # devmem 0x1e78
> 0x0003
> # (qemu) qom-set gpio gpioA1 off
> (qemu) 
> 
> # devmem 0x1e78
> 0x0001
> # (qemu) qom-set gpio gpioA0 off
> (qemu) 
> # devmem 0x1e78
> 0x0001
> # 
> ```
> 
> That was with the patch below. However, I think there's a deeper issue 
> with the input masking that needs to be addressed. Essentially we lack 
> modelling for the actual line state, we were proxying that with 
> register state. As it stands if we input-mask a line and use qom-set to 
> change its state the state update will go missing. However, as Joel 
> notes, I don't think we have anything configuring input masking.
> 
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index c63634d3d3e2..a1aa6504a8d8 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -244,7 +244,7 @@ static ptrdiff_t aspeed_gpio_set_idx(AspeedGPIOState *s, 
> GPIOSets *regs)
>  }
>  
>  static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,
> -   uint32_t value)
> +   uint32_t value, uint32_t mode_mask)
>  {
>  uint32_t input_mask = regs->input_mask;
>  uint32_t direction = regs->direction;
> @@ -253,7 +253,8 @@ static void aspeed_gpio_update(AspeedGPIOState *s, 
> GPIOSets *regs,
>  uint32_t diff;
>  int gpio;
>  
> -

[PATCH 2/3] target/s390x: fix NaN propagation rules

2022-07-11 Thread Ilya Leoshkevich
s390x has the same NaN propagation rules as ARM, and not as x86.

Signed-off-by: Ilya Leoshkevich 
---
 fpu/softfloat-specialize.c.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index 943e3301d2..a43ff5e02e 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -390,7 +390,8 @@ bool float32_is_signaling_nan(float32 a_, float_status 
*status)
 static int pickNaN(FloatClass a_cls, FloatClass b_cls,
bool aIsLargerSignificand, float_status *status)
 {
-#if defined(TARGET_ARM) || defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if defined(TARGET_ARM) || defined(TARGET_MIPS) || defined(TARGET_HPPA) || \
+defined(TARGET_S390X)
 /* ARM mandated NaN propagation rules (see FPProcessNaNs()), take
  * the first of:
  *  1. A if it is signaling
-- 
2.35.3




[PATCH 1/3] target/s390x: fix handling of zeroes in vfmin/vfmax

2022-07-11 Thread Ilya Leoshkevich
vfmin_res() / vfmax_res() are trying to check whether a and b are both
zeroes, but in reality they check that they are the same kind of zero.
This causes incorrect results when comparing positive and negative
zeroes.

Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)")
Co-developed-by: Ulrich Weigand 
Signed-off-by: Ilya Leoshkevich 
---
 target/s390x/tcg/vec_fpu_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/tcg/vec_fpu_helper.c 
b/target/s390x/tcg/vec_fpu_helper.c
index 2a618a1093..75cf605b9f 100644
--- a/target/s390x/tcg/vec_fpu_helper.c
+++ b/target/s390x/tcg/vec_fpu_helper.c
@@ -824,7 +824,7 @@ static S390MinMaxRes vfmin_res(uint16_t dcmask_a, uint16_t 
dcmask_b,
 default:
 g_assert_not_reached();
 }
-} else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
+} else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) 
{
 switch (type) {
 case S390_MINMAX_TYPE_JAVA:
 return neg_a ? S390_MINMAX_RES_A : S390_MINMAX_RES_B;
@@ -874,7 +874,7 @@ static S390MinMaxRes vfmax_res(uint16_t dcmask_a, uint16_t 
dcmask_b,
 default:
 g_assert_not_reached();
 }
-} else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
+} else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) 
{
 const bool neg_a = dcmask_a & DCMASK_NEGATIVE;
 
 switch (type) {
-- 
2.35.3




[PATCH 0/3] target/s390x: vfmin/vfmax fixes

2022-07-11 Thread Ilya Leoshkevich
Hi,

Uli has found an issue with finding maximum of different kinds of 0s; I
wrote a test and found another one with finding maximum of different
kinds of NaNs.

Patches 1 and 2 fix those issues, patch 3 adds a vfmin/vfmax test.

Best regards,
Ilya

Ilya Leoshkevich (3):
  target/s390x: fix handling of zeroes in vfmin/vfmax
  target/s390x: fix NaN propagation rules
  tests/tcg/s390x: test signed vfmin/vfmax

 fpu/softfloat-specialize.c.inc|   3 +-
 target/s390x/tcg/vec_fpu_helper.c |   4 +-
 tests/tcg/s390x/Makefile.target   |   7 +
 tests/tcg/s390x/vfminmax.c| 426 ++
 4 files changed, 437 insertions(+), 3 deletions(-)
 create mode 100644 tests/tcg/s390x/vfminmax.c

-- 
2.35.3




Re: [PATCH RESEND] python/machine: Fix AF_UNIX path too long on macOS

2022-07-11 Thread Peter Delevoryas
On Mon, Jul 11, 2022 at 04:56:28PM -0400, John Snow wrote:
> On Thu, Jul 7, 2022 at 2:46 PM Peter Delevoryas  wrote:
> >
> > On Wed, Jul 06, 2022 at 05:52:48PM -0700, Peter Delevoryas wrote:
> > > On Wed, Jul 06, 2022 at 09:46:48AM -0700, Peter Delevoryas wrote:
> > > > On Wed, Jul 06, 2022 at 09:02:14AM +0100, Daniel P. Berrangé wrote:
> > > > > On Tue, Jul 05, 2022 at 02:46:59PM -0700, Peter Delevoryas wrote:
> > > > > > I noticed that I can't run any avocado tests on macOS because the 
> > > > > > QMP
> > > > > > unix socket path is too long:
> > > > >
> > > > >
> > > > > > I think the path limit for unix sockets on macOS might be 104 [1]
> > > > >
> > > > > All platforms have a very limited path limit, so it isn't really
> > > > > a macOS specific problem, rather
> > > > >
> > > > > >
> > > > > > /*
> > > > > >  * [XSI] Definitions for UNIX IPC domain.
> > > > > >  */
> > > > > > struct  sockaddr_un {
> > > > > > unsigned char   sun_len;/* sockaddr len including null 
> > > > > > */
> > > > > > sa_family_t sun_family; /* [XSI] AF_UNIX */
> > > > > > charsun_path[104];  /* [XSI] path name (gag) */
> > > > > > };
> > > > > >
> > > > > > The path we're using is exactly 105 characters:
> > > > > >
> > > > > > $ python
> > > > > > Python 2.7.10 (default, Jan 19 2016, 22:24:01)
> > > > > > [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
> > > > > > Type "help", "copyright", "credits" or "license" for more 
> > > > > > information.
> > > > > > >>> len('/var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock')
> > > > >
> > > > > It is a problem related to where the test suite is creating the
> > > > > paths.
> > > > >
> > > > > /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/avo_qemu_sock_uh3w_dgc/
> > > > >
> > > > > is way too deep a directory location.
> > > >
> > > > That's a good point.
> > > >
> > > > >
> > > > > It seems we just create this location using 
> > > > > 'tempfile.TemporyDirectory'
> > > > > to get a standard tmp dir.
> > > > >
> > > > > Do you know why python is choosing
> > > > >
> > > > >   /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/
> > > > >
> > > > > as the temp dir ? Is that a standard location on macOS or is it
> > > > > from some env variable you have set ?
> > > >
> > > > Hmmm yeah it is odd, I'm not really sure why it's created there or if
> > > > standard glibc tmpfile creation goes there too, I'll go experiment and
> > > > report back. And yeah, maybe I'll double check any environment 
> > > > variables or
> > > > other things.
> > > >
> > > > The macOS system I use happens to be a Facebook work laptop, which could
> > > > also be related now that I think about it.
> > >
> > > Hmmm yeah looks like this is because my TMPDIR is weird.
> > >
> > > $ echo $TMPDIR
> > > /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/
> > >
> > > I didn't think to check this cause I wasn't familiar with TMPDIR. 路
> > >
> > > Thanks for responding, I'll just use TMPDIR=/tmp for now. It's probably
> > > something wrong with the Facebook development environment.
> > >
> > > Peter
> >
> > Update: Actually, this might not be a Facebook-work-laptop specific
> > thing.  I asked my non-engineer friend to print out $TMPDIR on his
> > macbook and he got the same thing.
> >
> > https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
> >
> > I guess this person suggests it's just to separate the permissions for
> > each user's /tmp directory, for better isolation.
> >
> > I'll resubmit this patch with the suggestions you had, because perhaps
> > this is actually affecting other macOS users too.
> >
> > >
> > > >
> > > > >
> > > > > > diff --git a/python/qemu/machine/machine.py 
> > > > > > b/python/qemu/machine/machine.py
> > > > > > index 37191f433b..93451774e3 100644
> > > > > > --- a/python/qemu/machine/machine.py
> > > > > > +++ b/python/qemu/machine/machine.py
> > > > > > @@ -157,7 +157,7 @@ def __init__(self,
> > > > > >  self._wrapper = wrapper
> > > > > >  self._qmp_timer = qmp_timer
> > > > > >
> > > > > > -self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
> > > > > > +self._name = name or f"{os.getpid()}{id(self):02x}"
> > > > >
> > > > > I don't think this is the right fix really, because IMHO the problem
> > > > > is the hugely long path, rather than the final socket name.
> > > >
> > > > True, yeah let me try to investigate the directory placement.
> > > >
> > > > >
> > > > > That said, there is redundancy in the path - avocado is passing in
> > > > > a dierctory created using 'tempfile.TemporyDirectory' so there is no
> > > > > reason why we need to add more entropy via the POD and the 'id(self)'
> > > > > hex string.
> > > >
> > > > Oh good point, I hadn't thought about that.
> > > >
> > > > >
> > > > > IMHO avocado should pass in the 'name' parameter explicitly, using a
> > > > > plain name and thus 

Re: [PATCH v2] scsi/lsi53c895a: fix use-after-free in lsi_do_msgout (CVE-2022-0216)

2022-07-11 Thread Alexander Bulekov
On 220711 1433, Mauro Matteo Cascella wrote:
> Set current_req to NULL to prevent reusing a free'd buffer in case of repeated
> SCSI cancel requests. Thanks to Thomas Huth for suggesting the first version 
> of
> the patch and Alexander Bulekov for providing a reproducer.
> 
> Fixes: CVE-2022-0216
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> Signed-off-by: Mauro Matteo Cascella 

Thank you - With this, the fuzzer isn't finding any more heap-related
crashes.

Tested-by: Alexander Bulekov 

> ---
> v2:
> - handle CLEAR QUEUE and BUS DEVICE RESET messages
> - new qtest: test_lsi_do_msgout_cancel_req
> 
>  hw/scsi/lsi53c895a.c   |  2 +
>  tests/qtest/fuzz-lsi53c895a-test.c | 71 ++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index c8773f73f7..6934040c59 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1030,6 +1030,7 @@ static void lsi_do_msgout(LSIState *s)
>  trace_lsi_do_msgout_abort(current_tag);
>  if (current_req) {
>  scsi_req_cancel(current_req->req);
> +current_req = NULL;
>  }
>  lsi_disconnect(s);
>  break;
> @@ -1055,6 +1056,7 @@ static void lsi_do_msgout(LSIState *s)
>  /* clear the current I/O process */
>  if (s->current) {
>  scsi_req_cancel(s->current->req);
> +current_req = NULL;
>  }
>  
>  /* As the current implemented devices scsi_disk and scsi_generic
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c 
> b/tests/qtest/fuzz-lsi53c895a-test.c
> index 2e8e67859e..6872c70d3a 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -8,6 +8,74 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  
> +/*
> + * This used to trigger a UAF in lsi_do_msgout()
> + * https://gitlab.com/qemu-project/qemu/-/issues/972
> + */
> +static void test_lsi_do_msgout_cancel_req(void)
> +{
> +QTestState *s;
> +
> +s = qtest_init("-M q35 -m 4G -display none -nodefaults "
> +   "-device lsi53c895a,id=scsi "
> +   "-device scsi-hd,drive=disk0 "
> +   "-drive file=null-co://,id=disk0,if=none,format=raw");
> +
> +qtest_outl(s, 0xcf8, 0x8810);
> +qtest_outl(s, 0xcf8, 0xc000);
> +qtest_outl(s, 0xcf8, 0x8810);
> +qtest_outw(s, 0xcfc, 0x7);
> +qtest_outl(s, 0xcf8, 0x8810);
> +qtest_outl(s, 0xcfc, 0xc000);
> +qtest_outl(s, 0xcf8, 0x8804);
> +qtest_outw(s, 0xcfc, 0x05);
> +qtest_writeb(s, 0x69736c10, 0x08);
> +qtest_writeb(s, 0x69736c13, 0x58);
> +qtest_writeb(s, 0x69736c1a, 0x01);
> +qtest_writeb(s, 0x69736c1b, 0x06);
> +qtest_writeb(s, 0x69736c22, 0x01);
> +qtest_writeb(s, 0x69736c23, 0x07);
> +qtest_writeb(s, 0x69736c2b, 0x02);
> +qtest_writeb(s, 0x69736c48, 0x08);
> +qtest_writeb(s, 0x69736c4b, 0x58);
> +qtest_writeb(s, 0x69736c52, 0x04);
> +qtest_writeb(s, 0x69736c53, 0x06);
> +qtest_writeb(s, 0x69736c5b, 0x02);
> +qtest_outl(s, 0xc02d, 0x697300);
> +qtest_writeb(s, 0x5a554662, 0x01);
> +qtest_writeb(s, 0x5a554663, 0x07);
> +qtest_writeb(s, 0x5a55466a, 0x10);
> +qtest_writeb(s, 0x5a55466b, 0x22);
> +qtest_writeb(s, 0x5a55466c, 0x5a);
> +qtest_writeb(s, 0x5a55466d, 0x5a);
> +qtest_writeb(s, 0x5a55466e, 0x34);
> +qtest_writeb(s, 0x5a55466f, 0x5a);
> +qtest_writeb(s, 0x5a345a5a, 0x77);
> +qtest_writeb(s, 0x5a345a5b, 0x55);
> +qtest_writeb(s, 0x5a345a5c, 0x51);
> +qtest_writeb(s, 0x5a345a5d, 0x27);
> +qtest_writeb(s, 0x27515577, 0x41);
> +qtest_outl(s, 0xc02d, 0x5a5500);
> +qtest_writeb(s, 0x364001d0, 0x08);
> +qtest_writeb(s, 0x364001d3, 0x58);
> +qtest_writeb(s, 0x364001da, 0x01);
> +qtest_writeb(s, 0x364001db, 0x26);
> +qtest_writeb(s, 0x364001dc, 0x0d);
> +qtest_writeb(s, 0x364001dd, 0xae);
> +qtest_writeb(s, 0x364001de, 0x41);
> +qtest_writeb(s, 0x364001df, 0x5a);
> +qtest_writeb(s, 0x5a41ae0d, 0xf8);
> +qtest_writeb(s, 0x5a41ae0e, 0x36);
> +qtest_writeb(s, 0x5a41ae0f, 0xd7);
> +qtest_writeb(s, 0x5a41ae10, 0x36);
> +qtest_writeb(s, 0x36d736f8, 0x0c);
> +qtest_writeb(s, 0x36d736f9, 0x80);
> +qtest_writeb(s, 0x36d736fa, 0x0d);
> +qtest_outl(s, 0xc02d, 0x364000);
> +
> +qtest_quit(s);
> +}
> +
>  /*
>   * This used to trigger the assert in lsi_do_dma()
>   * https://bugs.launchpad.net/qemu/+bug/697510
> @@ -44,5 +112,8 @@ int main(int argc, char **argv)
>  qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
> test_lsi_do_dma_empty_queue);
>  
> +qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> +   test_lsi_do_msgout_cancel_req);
> +
>  return g_test_run();
>  }
> -- 
> 2.35.3
> 



Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning

2022-07-11 Thread Peter Delevoryas
On Wed, Jul 06, 2022 at 05:58:38PM -0700, Peter Delevoryas wrote:
> On Sat, Jul 02, 2022 at 11:30:16PM +0900, Akihiko Odaki wrote:
> > Reviewed-by: Akihiko Odaki 
> 
> Just checking in on the status of this: do I need to submit a pull request?
> Or will this patch be picked up in a miscellaneous pull queue eventually?

Pinging this thread again, does this change need anyone else to review it?

> 
> > 
> > On 2022/07/02 13:43, Peter Delevoryas wrote:
> > > I noticed this error while building QEMU on Mac OS X:
> > > 
> > >  [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> > >  ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set 
> > > but not used [-Wunused-but-set-variable]
> > >  static bool switched_to_fullscreen = false;
> > >  ^
> > >  1 warning generated.
> > > 
> > > I think the behavior is fine if you remove "switched_to_fullscreen", I can
> > > still switch in and out of mouse grabbed mode and fullscreen mode with 
> > > this
> > > change, and Command keycodes will only be passed to the guest if the mouse
> > > is grabbed, which I think is the right behavior. I'm not sure why a static
> > > piece of state was needed to handle that in the first place. Perhaps the
> > > refactoring of the flags-state-change fixed that by toggling the Command
> > > keycode on.
> > > 
> > > I tested this with an Ubuntu core image on macOS 12.4
> > > 
> > >  wget 
> > > https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
> > >  xz -d ubuntu-core-18-i386.img.xz
> > >  qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
> > > 
> > > Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
> > > Signed-off-by: Peter Delevoryas 
> > > ---
> > >   ui/cocoa.m | 8 
> > >   1 file changed, 8 deletions(-)
> > > 
> > > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > > index 84c84e98fc..13e208b037 100644
> > > --- a/ui/cocoa.m
> > > +++ b/ui/cocoa.m
> > > @@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > >   int buttons = 0;
> > >   int keycode = 0;
> > >   bool mouse_event = false;
> > > -static bool switched_to_fullscreen = false;
> > >   // Location of event in virtual screen coordinates
> > >   NSPoint p = [self screenLocationOfEvent:event];
> > >   NSUInteger modifiers = [event modifierFlags];
> > > @@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > >   // forward command key combos to the host UI unless the 
> > > mouse is grabbed
> > >   if (!isMouseGrabbed && ([event modifierFlags] & 
> > > NSEventModifierFlagCommand)) {
> > > -/*
> > > - * Prevent the command key from being stuck down in the 
> > > guest
> > > - * when using Command-F to switch to full screen mode.
> > > - */
> > > -if (keycode == Q_KEY_CODE_F) {
> > > -switched_to_fullscreen = true;
> > > -}
> > >   return false;
> > >   }
> > 
> 



[PATCH v4 2/2] ui/gtk: a new array param monitor to specify the target displays

2022-07-11 Thread Dongwon Kim
New integer array parameter, 'monitor' is for specifying the target
monitors where individual GTK windows are placed upon launching.

Monitor numbers in the array are associated with virtual consoles
in the order of [VC0, VC1, VC2 ... VCn].

Every GTK window containing each VC will be placed in the region
of corresponding monitors.

Usage: -display gtk,monitor.=,..
   ex)-display gtk,monitor.0=1,monitor.1=0

Cc: Daniel P. Berrangé 
Cc: Markus Armbruster 
Cc: Philippe Mathieu-Daudé 
Cc: Paolo Bonzini 
Cc: Gerd Hoffmann 
Cc: Vivek Kasireddy 
Signed-off-by: Dongwon Kim 
---
 qapi/ui.json|  9 -
 qemu-options.hx |  3 ++-
 ui/gtk.c| 30 --
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 413371d5e8..ee0f9244ef 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1195,12 +1195,19 @@
 #   assuming the guest will resize the display to match
 #   the window size then.  Otherwise it defaults to "off".
 #   Since 3.1
+# @monitor: Array of numbers, each of which represents physical
+#   monitor where GTK window containing a given VC will be
+#   placed. Each monitor number in the array will be
+#   associated with a virtual console starting from VC0.
+#
+#   since 7.1
 #
 # Since: 2.12
 ##
 { 'struct'  : 'DisplayGTK',
   'data': { '*grab-on-hover' : 'bool',
-'*zoom-to-fit'   : 'bool'  } }
+'*zoom-to-fit'   : 'bool',
+'*monitor'   : ['uint16']  } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index 377d22fbd8..aabdfb0636 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1938,7 +1938,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #endif
 #if defined(CONFIG_GTK)
 "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
-"[,show-cursor=on|off][,window-close=on|off]\n"
+"[,monitor.=][,show-cursor=on|off]"
+"[,window-close=on|off]\n"
 #endif
 #if defined(CONFIG_VNC)
 "-display vnc=[,]\n"
diff --git a/ui/gtk.c b/ui/gtk.c
index e6878c3209..598ab4970f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2316,6 +2316,10 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 GtkDisplayState *s = g_malloc0(sizeof(*s));
 GdkDisplay *window_display;
 GtkIconTheme *theme;
+GtkWidget *win;
+GdkRectangle dest;
+uint16List *mon;
+int n_mon;
 int i;
 char *dir;
 
@@ -2393,10 +2397,32 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
 }
 }
-if (opts->has_full_screen &&
-opts->full_screen) {
+
+if (opts->u.gtk.has_monitor) {
+i = 0;
+n_mon = gdk_display_get_n_monitors(window_display);
+for (mon = opts->u.gtk.monitor; mon; mon = mon->next) {
+if (mon->value < n_mon && i < s->nb_vcs) {
+win = s->vc[i].window ? s->vc[i].window : s->window;
+if (opts->full_screen) {
+gtk_window_fullscreen_on_monitor(
+GTK_WINDOW(win),
+gdk_display_get_default_screen(window_display),
+mon->value);
+} else {
+gdk_monitor_get_geometry(
+gdk_display_get_monitor(window_display, mon->value),
+);
+gtk_window_move(GTK_WINDOW(win),
+dest.x, dest.y);
+}
+i++;
+}
+}
+} else if (opts->full_screen) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));
 }
+
 if (opts->u.gtk.has_grab_on_hover &&
 opts->u.gtk.grab_on_hover) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
-- 
2.30.2




[PATCH v4 1/2] ui/gtk: detach VCs for additional guest displays

2022-07-11 Thread Dongwon Kim
Detaching any addtional guest displays in case multiple displays are
assigned to the guest OS (e.g. max_outputs=n) so that all of them are
visible upon lauching.

Cc: Daniel P. Berrangé 
Cc: Markus Armbruster 
Cc: Philippe Mathieu-Daudé 
Cc: Paolo Bonzini 
Cc: Gerd Hoffmann 
Cc: Vivek Kasireddy 
Signed-off-by: Dongwon Kim 
---
 ui/gtk.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 2a791dd2aa..e6878c3209 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1361,6 +1361,11 @@ static void gd_menu_untabify(GtkMenuItem *item, void 
*opaque)
 
 g_signal_connect(vc->window, "delete-event",
  G_CALLBACK(gd_tab_window_close), vc);
+
+gtk_window_set_default_size(GTK_WINDOW(vc->window),
+surface_width(vc->gfx.ds),
+surface_height(vc->gfx.ds));
+
 gtk_widget_show_all(vc->window);
 
 if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
@@ -2311,6 +2316,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 GtkDisplayState *s = g_malloc0(sizeof(*s));
 GdkDisplay *window_display;
 GtkIconTheme *theme;
+int i;
 char *dir;
 
 if (!gtkinit) {
@@ -2381,7 +2387,12 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 gtk_widget_set_sensitive(s->copy_item,
  vc && vc->type == GD_VC_VTE);
 #endif
-
+for (i = 1; i < s->nb_vcs; i++) {
+if (vc->type == GD_VC_GFX &&
+qemu_console_is_graphic(s->vc[i].gfx.dcl.con)) {
+gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
+}
+}
 if (opts->has_full_screen &&
 opts->full_screen) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));
-- 
2.30.2




[PATCH v4 0/2] handling guest multiple displays

2022-07-11 Thread Dongwon Kim
This patch seires is for adding some useful features for the guest os with
multi-displays. First patch is to make all of guest displays visible
when guest os is launched using "detach". Second patch is for providing
a method to assign each guest display to specific physical monitor,
which would be useful if someone wants to directly full-screen individual
guest scanouts to host's physical displays.

Changes in v4:

* ui/gtk: a new array param monitor to specify the target

  - changed "virtual-console" to an official term, "virtual console"
  - made if condition to check only 'full_screen' since 'has_full_screen' won't
affect the result as 'full_screen' is always false if 'has_full_screen' is 
false.

Changes in v3:

* ui/gtk: a new array param monitor to specify the target

  - Revised commit message
  - Rewrote desription of the new parameter for clarification
  - 'for' loop that iterates through virtual consoles is actually executed only 
once
only if the condition is met so replaced it with 'if' statement

Changes in v2:

* ui/gtk: detach VCS for additional guest displays

  - must check if the type of VC is GD_VC_GFX before qemu_console_is_graphic
  - It is not needed to pre-calculate n_gfx_vcs to determine how many times 
"detach"
should be executed (n_gfx_vcs - 1) because the first virtual console (vc[0])
is always in graphic mode so we can simply detach all other graphic mode
virtual consoles.
  - making sure detached window's size same as original surface size

Dongwon Kim (2):
  ui/gtk: detach VCS for additional guest displays (v4)
  ui/gtk: a new array param monitor to specify the target displays (v4)

 qapi/ui.json|  9 -
 qemu-options.hx |  3 ++-
 ui/gtk.c| 41 +++--
 3 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.30.2




[RFC PATCH v3 5/7] tests: add 'check-venv' as a dependency of 'make check'

2022-07-11 Thread John Snow
This patch adds the 'check-venv' target as a requisite of all meson
driven check-* targets. As of this commit, it will only install the
"qemu" namespace package from the source tree, and nothing else.

In the future, the "qemu" namespace package in qemu.git will begin to
require an external qemu.qmp package, and this would be installed into
this environment as well.

The avocado test dependencies will *not* be pulled into this venv by
default, but they may be added in at a later point in time by running
'make check-avocado' or, without running the tests, 'make
check-venv-avocado'.

Signed-off-by: John Snow 
---
 tests/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d8af6a38112..d484a335be5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -155,6 +155,9 @@ check-acceptance-deprecated-warning:
 
 check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
+# The do-meson-check and do-meson-bench targets are defined in Makefile.mtest
+do-meson-check do-meson-bench: check-venv
+
 # Consolidated targets
 
 .PHONY: check check-clean get-vm-images
-- 
2.34.3




[RFC PATCH v3 7/7] iotests: self-bootstrap testing venv

2022-07-11 Thread John Snow
When iotests are invoked manually from
e.g. $build/tests/qemu-iotests/check, it is not necessarily guaranteed
that we'll have run 'check-venv' yet.

With this patch, teach testenv.py how to create its own environment.

Note: this self-bootstrapping is fairly rudimentary and will miss
certain triggers to refresh the venv. It will miss when new dependencies
are added to either python/setup.cfg or tests/setup.cfg. It can be
coaxed into updating by running 'make check', 'make check-block', 'make
check-venv', etc.

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

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 29404ac94be..e985eaf3e97 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -112,10 +112,10 @@ def init_directories(self) -> None:
 """
 venv_path = Path(self.build_root, 'tests/venv/')
 if not venv_path.exists():
-raise FileNotFoundError(
-f"Virtual environment \"{venv_path!s}\" isn't found."
-" (Maybe you need to run 'make check-venv'"
-" from the build dir?)"
+mkvenv = Path(self.source_iotests, '../mkvenv.py')
+subprocess.run(
+(sys.executable, str(mkvenv), str(venv_path)),
+check=True,
 )
 self.virtual_env: str = str(venv_path)
 
-- 
2.34.3




[RFC PATCH v3 1/7] tests: create optional tests/venv dependency groups

2022-07-11 Thread John Snow
This patch uses a dummy package and setup.cfg/setup.py files to manage
optional dependency groups for the test venv specification. Now, there's
a core set of dependencies which for now includes just "qemu" (but soon,
qemu.qmp) and a separate, optional 'avocado' group that includes
avocado-framework and pycdlib.

Practical upshot: We install only a minimum of things for the majority
of check-* targets, but allow optional add-ons to be processed when
running avocado tests. This will spare downstreams from having to add
more dependencies than is necessary as a build dependencies when
invoking "make check".

(We also keep both sets of dependencies in one file, which is helpful
for review to ensure that different option groups don't conflict with
one another.)

NOTE: There is a non-fatal caveat introduced by this patch on Ubuntu
20.04 systems; see the subsequent commit "tests: Remove spurious pip
warnings on Ubuntu20.04" for more information.

Signed-off-by: John Snow 
---
 tests/Makefile.include | 21 ++---
 tests/requirements.txt |  6 --
 tests/setup.cfg| 20 
 tests/setup.py | 16 
 4 files changed, 50 insertions(+), 13 deletions(-)
 delete mode 100644 tests/requirements.txt
 create mode 100644 tests/setup.cfg
 create mode 100644 tests/setup.py

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3accb83b132..82c697230e0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -81,13 +81,13 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
 # Python venv for running tests
 
-.PHONY: check-venv check-avocado check-acceptance 
check-acceptance-deprecated-warning
+.PHONY: check-venv check-venv-avocado check-avocado check-acceptance \
+check-acceptance-deprecated-warning
 
 # Build up our target list from the filtered list of ninja targets
 TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
 
 TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
-TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
 TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
 TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python3
 ifndef AVOCADO_TESTS
@@ -108,10 +108,16 @@ quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
 $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
 "VENVPIP","$1")
 
-$(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
+# Core dependencies for tests/venv
+$(TESTS_VENV_DIR): $(SRC_PATH)/tests/setup.cfg $(SRC_PATH)/python/setup.cfg
$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
$(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
-   $(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
+   $(call quiet-venv-pip,install "$(SRC_PATH)/tests/")
+   $(call quiet-command, touch $@)
+
+# Optional avocado dependencies for tests/venv
+$(TESTS_VENV_DIR)/avocado: $(TESTS_VENV_DIR)
+   $(call quiet-venv-pip,install "$(SRC_PATH)/tests/[avocado]")
$(call quiet-command, touch $@)
 
 $(TESTS_RESULTS_DIR):
@@ -119,6 +125,7 @@ $(TESTS_RESULTS_DIR):
 MKDIR, $@)
 
 check-venv: $(TESTS_VENV_DIR)
+check-venv-avocado: $(TESTS_VENV_DIR)/avocado
 
 FEDORA_31_ARCHES_TARGETS=$(patsubst %-softmmu,%, $(filter 
%-softmmu,$(TARGETS)))
 FEDORA_31_ARCHES_CANDIDATES=$(patsubst 
ppc64,ppc64le,$(FEDORA_31_ARCHES_TARGETS))
@@ -126,16 +133,16 @@ FEDORA_31_ARCHES := x86_64 aarch64 ppc64le s390x
 FEDORA_31_DOWNLOAD=$(filter $(FEDORA_31_ARCHES),$(FEDORA_31_ARCHES_CANDIDATES))
 
 # download one specific Fedora 31 image
-get-vm-image-fedora-31-%: check-venv
+get-vm-image-fedora-31-%: check-venv-avocado
$(call quiet-command, \
  $(TESTS_PYTHON) -m avocado vmimage get \
  --distro=fedora --distro-version=31 --arch=$*, \
"AVOCADO", "Downloading avocado tests VM image for $*")
 
 # download all vm images, according to defined targets
-get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, 
$(FEDORA_31_DOWNLOAD))
+get-vm-images: check-venv-avocado $(patsubst %,get-vm-image-fedora-31-%, 
$(FEDORA_31_DOWNLOAD))
 
-check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
+check-avocado: check-venv-avocado $(TESTS_RESULTS_DIR) get-vm-images
$(call quiet-command, \
 $(TESTS_PYTHON) -m avocado \
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
diff --git a/tests/requirements.txt b/tests/requirements.txt
deleted file mode 100644
index 0ba561b6bdf..000
--- a/tests/requirements.txt
+++ /dev/null
@@ -1,6 +0,0 @@
-# Add Python module requirements, one per line, to be installed
-# in the tests/venv Python virtual environment. For more info,
-# refer to: https://pip.pypa.io/en/stable/user_guide/#id1
-# Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
-pycdlib==1.11.0
diff --git a/tests/setup.cfg b/tests/setup.cfg
new file mode 100644
index 000..263a5de01af
--- /dev/null
+++ b/tests/setup.cfg
@@ -0,0 +1,20 @@
+# This file represents a "dummy" package that expresses
+# 

[RFC PATCH v3 4/7] tests/vm: add venv pre-requisites to VM building recipes

2022-07-11 Thread John Snow
NetBSD needs "py37-pip" in order to create virtual environments.

Signed-off-by: John Snow 
---
 tests/vm/netbsd | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 45aa9a7fda7..53fe508e487 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -31,6 +31,7 @@ class NetBSDVM(basevm.BaseVM):
 "pkgconf",
 "xz",
 "python37",
+"py37-pip",
 "ninja-build",
 
 # gnu tools
-- 
2.34.3




[RFC PATCH v3 3/7] tests: Remove spurious pip warnings on Ubuntu20.04

2022-07-11 Thread John Snow
The version of pip ("20.0.2") that ships with Ubuntu 20.04 has a bug
where it will try to attempt building a wheel even if the "wheel" python
package that enables it to do so is not installed. Even though pip
continues gracefully from source, The result is a lot of irrelevant
failure output.

Upstream pip 20.0.2 does not have this problem, and pip 20.1 introduces
a new info message that informs a user that wheel building is being skipped.

On this version, the output can be silenced by passing --no-binary to
coax pip into skipping that step to begin with. Note, this error does
not seem to show up for the "qemu" package because we install that
package in editable mode. (I think, but did not test, that installing an
empty package in editable mode caused more problems than it fixed.)

Signed-off-by: John Snow 
---
 tests/mkvenv.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/mkvenv.py b/tests/mkvenv.py
index 0667106d6aa..78f8d0382e0 100644
--- a/tests/mkvenv.py
+++ b/tests/mkvenv.py
@@ -144,7 +144,8 @@ def make_qemu_venv(
 with enter_venv(venv_path):
 if do_initialize:
 install("-e", str(pysrc_path), offline=offline)
-install(str(test_src_path), offline=offline)
+install("--no-binary", "qemu.dummy-tests",
+str(test_src_path), offline=offline)
 venv_path.touch()
 
 for option in options:
-- 
2.34.3




[RFC PATCH v3 6/7] iotests: use tests/venv for running tests

2022-07-11 Thread John Snow
Essentially, the changes to testenv.py here mimic the changes that occur when
you "source venv/bin/activate.fish" or similar.

(1) update iotest's internal notion of which python binary to use,
(2) export the VIRTUAL_ENV variable,
(3) front-load the venv/bin directory to PATH.

If the venv directory isn't found, raise a friendly exception that tries
to give the human operator a friendly clue as to what's gone wrong. The
subsequent commit attempts to address this shortcoming by teaching
iotests how to invoke the venv bootstrapper in this circumstance
instead.

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

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index a864c74b123..29404ac94be 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
 # lot of them. Silence pylint:
 # pylint: disable=too-many-instance-attributes
 
-env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
- 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON', 'PATH',
+ 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+ 'QEMU_PROG', 'QEMU_IMG_PROG',
  'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
  'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
  'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
@@ -102,18 +103,29 @@ def get_env(self) -> Dict[str, str]:
 
 def init_directories(self) -> None:
 """Init directory variables:
+ VIRTUAL_ENV
+ PATH
  PYTHONPATH
  TEST_DIR
  SOCK_DIR
  SAMPLE_IMG_DIR
 """
+venv_path = Path(self.build_root, 'tests/venv/')
+if not venv_path.exists():
+raise FileNotFoundError(
+f"Virtual environment \"{venv_path!s}\" isn't found."
+" (Maybe you need to run 'make check-venv'"
+" from the build dir?)"
+)
+self.virtual_env: str = str(venv_path)
 
-# Path where qemu goodies live in this source tree.
-qemu_srctree_path = Path(__file__, '../../../python').resolve()
+self.path = os.pathsep.join((
+str(venv_path / 'bin'),
+os.environ['PATH'],
+))
 
 self.pythonpath = os.pathsep.join(filter(None, (
 self.source_iotests,
-str(qemu_srctree_path),
 os.getenv('PYTHONPATH'),
 )))
 
@@ -138,7 +150,7 @@ def init_binaries(self) -> None:
  PYTHON (for bash tests)
  QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
 """
-self.python = sys.executable
+self.python: str = os.path.join(self.virtual_env, 'bin', 'python3')
 
 def root(*names: str) -> str:
 return os.path.join(self.build_root, *names)
@@ -300,6 +312,7 @@ def print_env(self, prefix: str = '') -> None:
 {prefix}GDB_OPTIONS   -- {GDB_OPTIONS}
 {prefix}VALGRIND_QEMU -- {VALGRIND_QEMU}
 {prefix}PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
+{prefix}VIRTUAL_ENV   -- {VIRTUAL_ENV}
 {prefix}"""
 
 args = collections.defaultdict(str, self.get_env())
-- 
2.34.3




[RFC PATCH v3 2/7] tests: pythonize test venv creation

2022-07-11 Thread John Snow
This splits the venv creation logic out of the Makefile and into a
Python script.

One reason for doing this is to be able to call the venv bootstrapper
from places outside of the Makefile, e.g. configure and iotests. Another
reason is to be able to add "offline" logic to modify the behavior of
the script in certain circumstances.

RFC: I don't like how I have an entire "enter_venv" function here, and
by the end of the series, completely separate logic for entering a venv
in testenv.py -- but they both operate in different ways: this patch
offers a method that alters the current ENV immediately, whereas the
testenv.py method alters a copied ENV that is explicitly passed to
subprocesses.

Still, there's a bit more boilerplate than I'd like, but it's probably
fine and it probably won't need to be updated much, but... less code is
usually better.

But, even if I did write it more generically here; iotests wouldn't be
able to use it anyway, because importing it would mean more sys.path
hacking. So... eh?

Signed-off-by: John Snow 
---
 tests/Makefile.include |  16 ++--
 tests/mkvenv.py| 186 +
 2 files changed, 192 insertions(+), 10 deletions(-)
 create mode 100644 tests/mkvenv.py

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 82c697230e0..d8af6a38112 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -104,21 +104,17 @@ else
AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
 endif
 
-quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
-$(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
-"VENVPIP","$1")
-
 # Core dependencies for tests/venv
 $(TESTS_VENV_DIR): $(SRC_PATH)/tests/setup.cfg $(SRC_PATH)/python/setup.cfg
-   $(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
-   $(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
-   $(call quiet-venv-pip,install "$(SRC_PATH)/tests/")
-   $(call quiet-command, touch $@)
+   $(call quiet-command, \
+$(PYTHON) "$(SRC_PATH)/tests/mkvenv.py" "$@", \
+MKVENV, $@)
 
 # Optional avocado dependencies for tests/venv
 $(TESTS_VENV_DIR)/avocado: $(TESTS_VENV_DIR)
-   $(call quiet-venv-pip,install "$(SRC_PATH)/tests/[avocado]")
-   $(call quiet-command, touch $@)
+   $(call quiet-command, \
+$(PYTHON) "$(SRC_PATH)/tests/mkvenv.py" --opt avocado "$<", \
+MKVENV, $@)
 
 $(TESTS_RESULTS_DIR):
$(call quiet-command, mkdir -p $@, \
diff --git a/tests/mkvenv.py b/tests/mkvenv.py
new file mode 100644
index 000..0667106d6aa
--- /dev/null
+++ b/tests/mkvenv.py
@@ -0,0 +1,186 @@
+"""
+mkvenv - QEMU venv bootstrapping utility
+
+usage: mkvenv.py [-h] [--offline] [--opt OPT] target
+
+Bootstrap QEMU testing venv.
+
+positional arguments:
+  target  Target directory to install virtual environment into.
+
+optional arguments:
+  -h, --help  show this help message and exit
+  --offline   Use system packages and work entirely offline.
+  --opt OPT   Install an optional dependency group.
+"""
+
+# Copyright (C) 2022 Red Hat, Inc.
+#
+# Authors:
+#  John Snow 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+# Do not add any dependencies from outside the stdlib:
+# This script must be usable on its own!
+
+import argparse
+from contextlib import contextmanager
+import logging
+import os
+from pathlib import Path
+import subprocess
+import sys
+from typing import Iterable, Iterator, Union
+import venv
+
+
+def make_venv(
+venv_path: Union[str, Path],
+system_site_packages: bool = False
+) -> None:
+"""
+Create a venv using the python3 'venv' module.
+
+Identical to:
+``python3 -m venv --clear [--system-site-packages] {venv_path}``
+
+:param venv_path: The target directory
+:param system_site_packages: If True, allow system packages in venv.
+"""
+logging.debug("%s: make_venv(venv_path=%s, system_site_packages=%s)",
+  __file__, str(venv_path), system_site_packages)
+venv.create(
+str(venv_path),
+clear=True,
+symlinks=os.name != "nt",  # Same as venv CLI's default.
+with_pip=True,
+system_site_packages=system_site_packages,
+)
+
+
+@contextmanager
+def enter_venv(venv_dir: Union[str, Path]) -> Iterator[None]:
+"""Scoped activation of an existing venv."""
+venv_keys = ('PATH', 'PYTHONHOME', 'VIRTUAL_ENV')
+old = {k: v for k, v in os.environ.items() if k in venv_keys}
+vdir = Path(venv_dir).resolve()
+
+def _delete_keys() -> None:
+for k in venv_keys:
+try:
+del os.environ[k]
+except KeyError:
+pass
+
+try:
+_delete_keys()
+
+os.environ['VIRTUAL_ENV'] = str(vdir)
+os.environ['PATH'] = os.pathsep.join([
+str(vdir / "bin/"),
+old['PATH'],
+])
+

[RFC PATCH v3 0/7] tests: run python tests under a venv

2022-07-11 Thread John Snow
Hi, here's another RFC for bringing external Python dependencies to the
QEMU test suite.

This is mostly a refresh of a version I sent out before, but mixes in my
VM improvement test as an optional pre-requisite to improve VM test
stability to ensure that the BSDs all pass with the new
infrastructure. (And our oldest supported Debian and Ubuntu targets,
too.)

(Note: this requires dropping support for Ubuntu 18.04, which ships with
a version of setuptools that is simply too old.)

This patchset is still not without some problems that I am working on,
but progress has been slow.

Problems I am aware of:

- This version of the patch series does not itself enforce any
  offline-only behavior for venv creation, but downstreams can modify
  any call to 'mkvenv' to pass '--offline'. I am working on a configure
  file toggle to swap the default behavior when running tests.

- iotests will now actually never run mypy or pylint tests by default
  anymore, because the bootstrapper won't select those packages by
  default, and the virtual environment won't utilize the system
  packages, so iotest 297 will just "skip" all of the time now.

  The reason we don't want to install these packages by default is
  because we don't want to add dependencies on mypy and pylint for
  downstream builds.

  With these patches, 297 would still work if you manually opened up the
  testing venv and installed suitable mypy/pylint packages there. I
  could also add a new optional dependency group, and one could
  theoretically invoke a once-per-build-dir command of 'make
  check-venv-iotests' to help make the process only semi-manual, but
  it's still annoying.

  Ideally, the python checks in qemu.git/python/ can handle the same
  tests as 297 does -- but we need to give a shorthand invocation like
  "make check-python" that is excluded from the default "make check" to
  allow block developers to quickly opt-in to the same tests.

  I've covered some of the problems here on-list before:
  https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg03661.html

  ...But I haven't quite solved them yet.

- iotests can now self-bootstrap the venv when it isn't present, but
  this self-bootstrapping has some issues in that because it skips the
  Makefile magic, it cannot update the venv when changes are made to the
  venv configuration piece.

That's all for now. Work on the configure file integration is ongoing. I
don't know if I'll beat soft freeze (It's looking unlikely given the
amount of IRL issues I am juggling right now), but I'm hoping to push as
much of this forward as I can to try and get some testing in for RC
phase to determine what problems might exist that I haven't noticed yet.

--js

John Snow (7):
  tests: create optional tests/venv dependency groups
  tests: pythonize test venv creation
  tests: Remove spurious pip warnings on Ubuntu20.04
  tests/vm: add venv pre-requisites to VM building recipes
  tests: add 'check-venv' as a dependency of 'make check'
  iotests: use tests/venv for running tests
  iotests: self-bootstrap testing venv

 tests/Makefile.include|  32 +++---
 tests/mkvenv.py   | 187 ++
 tests/qemu-iotests/testenv.py |  25 +++--
 tests/requirements.txt|   6 --
 tests/setup.cfg   |  20 
 tests/setup.py|  16 +++
 tests/vm/netbsd   |   1 +
 7 files changed, 262 insertions(+), 25 deletions(-)
 create mode 100644 tests/mkvenv.py
 delete mode 100644 tests/requirements.txt
 create mode 100644 tests/setup.cfg
 create mode 100644 tests/setup.py

-- 
2.34.3





Re: [PATCH v6 03/10] i386/pc: pass pci_hole64_size to pc_memory_init()

2022-07-11 Thread B



Am 11. Juli 2022 10:01:49 UTC schrieb Joao Martins :
>On 7/9/22 21:51, B wrote:
>> Am 1. Juli 2022 16:10:07 UTC schrieb Joao Martins 
>> :
>>> Use the pre-initialized pci-host qdev and fetch the
>>> pci-hole64-size into pc_memory_init() newly added argument.
>>> piix needs a bit of care given all the !pci_enabled()
>>> and that the pci_hole64_size is private to i440fx.
>> 
>> It exposes this value as the property PCI_HOST_PROP_PCI_HOLE64_SIZE. 
>
>Indeed.
>
>> Reusing it allows for not touching i440fx in this patch at all.
>> 
>> For code symmetry reasons the analogue property could be used for Q35 as 
>> well.
>> 
>Presumably you want me to change into below while deleting 
>i440fx_pci_hole64_size()
>from this patch (see snip below).

Yes, exactly.

>IMHO, gotta say that in q35 the code symmetry
>doesn't buy much readability here,

That's true. It communicates, though, that a value is used which was 
deliberately made public, IOW that the code isn't sneaky. (That's just my 
interpretation, not sure what the common understanding is) Feel free to do 
however you prefer.

Best regards,
Bernhard

>albeit it does remove any need for that other
>helper in i440fx.
>
>@Igor let me know if you agree with the change and whether I can keep the 
>Reviewed-by.
>
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index 504ddd0deece..cc0855066d06 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -167,7 +167,9 @@ static void pc_init1(MachineState *machine,
> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> rom_memory = pci_memory;
> i440fx_host = qdev_new(host_type);
>-hole64_size = i440fx_pci_hole64_size(i440fx_host);
>+hole64_size = object_property_get_uint(OBJECT(i440fx_host),
>+   PCI_HOST_PROP_PCI_HOLE64_SIZE,
>+   _abort);
> } else {
> pci_memory = NULL;
> rom_memory = system_memory;
>diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>index 4b747c59c19a..466f3ef3c918 100644
>--- a/hw/i386/pc_q35.c
>+++ b/hw/i386/pc_q35.c
>@@ -208,7 +208,9 @@ static void pc_q35_init(MachineState *machine)
> q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
>
> if (pcmc->pci_enabled) {
>-pci_hole64_size = q35_host->mch.pci_hole64_size;
>+pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
>+   
>PCI_HOST_PROP_PCI_HOLE64_SIZE,
>+   _abort);
> }
>
> /* allocate ram and load rom/bios */
>diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
>index 15680da7d709..d5426ef4a53c 100644
>--- a/hw/pci-host/i440fx.c
>+++ b/hw/pci-host/i440fx.c
>@@ -237,13 +237,6 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
> }
> }
>
>-uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
>-{
>-I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
>-
>-return i440fx->pci_hole64_size;
>-}
>-
> PCIBus *i440fx_init(const char *pci_type,
> DeviceState *dev,
> MemoryRegion *address_space_mem,



Re: Regarding QEMU on eclipse IDE:

2022-07-11 Thread Vaidehi Deshpande
On Mon, 11 Jul 2022 at 13:34, Vaidehi Deshpande <
vaidehideshpand...@gmail.com> wrote:

> Hello,
>
> I am Vaidehi and I am new to QEMU. I am working on emulating STM32F407
> board on QEMU and I am using eclipse IDE v4.24.0 and QEMU v7.0.0. I am
> trying to check the SPI loopback but the loop gets stuck at the
> BSY register check. And the xPacks is not installing
> qemu-system-gnuarmeclipse of the same version but installs
> qemu-system-gnuarmeclipse v2.8.0. I tried the blinky code and it works
> fine. Not sure about why the SPI is not functioning properly and why I
> couldn't install the qemu-system-gnuarmeclipse v7.0.0.
>
> Best regards,
> Vaidehi. D
>


[PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-07-11 Thread Vitaly Buka
aarch64 stores MTE tags in target_date, and they should be reset by
MADV_DONTNEED.

Signed-off-by: Vitaly Buka 
---
 accel/tcg/translate-all.c | 24 
 include/exec/cpu-all.h|  1 +
 linux-user/mmap.c |  2 ++
 3 files changed, 27 insertions(+)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ef62a199c7..d6f2f1a40a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2314,6 +2314,30 @@ void page_set_flags(target_ulong start, target_ulong 
end, int flags)
 }
 }
 
+void page_reset_target_data(target_ulong start, target_ulong end)
+{
+target_ulong addr, len;
+
+/* This function should never be called with addresses outside the
+   guest address space.  If this assert fires, it probably indicates
+   a missing call to h2g_valid.  */
+assert(end - 1 <= GUEST_ADDR_MAX);
+assert(start < end);
+assert_memory_lock();
+
+start = start & TARGET_PAGE_MASK;
+end = TARGET_PAGE_ALIGN(end);
+
+for (addr = start, len = end - start;
+ len != 0;
+ len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+
+g_free(p->target_data);
+p->target_data = NULL;
+}
+}
+
 void *page_get_target_data(target_ulong address)
 {
 PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f5bda2c3ca..491629b9ba 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -271,6 +271,7 @@ int walk_memory_regions(void *, walk_memory_regions_fn);
 
 int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
+void page_reset_target_data(target_ulong start, target_ulong end);
 int page_check_range(target_ulong start, target_ulong len, int flags);
 
 /**
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 4e7a6be6ee..c535dfdc7c 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -894,6 +894,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, 
int advice)
 if ((advice & MADV_DONTNEED) &&
 can_passthrough_madv_dontneed(start, end)) {
 ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
+if (ret == 0)
+page_reset_target_data(start, start + len);
 }
 mmap_unlock();
 
-- 
2.37.0.144.g8ac04bfd2-goog




Re: [PATCH] target/ppc/kvm: Skip ".." directory in kvmppc_find_cpu_dt

2022-07-11 Thread Daniel Henrique Barboza




On 7/11/22 16:37, Murilo Opsfelder Araujo wrote:

Some systems have /proc/device-tree/cpus/../clock-frequency. However,
this is not the expected path for a CPU device tree directory.

Signed-off-by: Murilo Opsfelder Araujo 
Signed-off-by: Fabiano Rosas 
---


Reviewed-by: Daniel Henrique Barboza 


  target/ppc/kvm.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 6eed466f80..c8485a5cc0 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len)
  buf[0] = '\0';
  while ((dirp = readdir(dp)) != NULL) {
  FILE *f;
+
+/* Don't accidentally read from the upper directory */
+if (strcmp(dirp->d_name, "..") == 0) {
+continue;
+}
+
  snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU,
   dirp->d_name);
  f = fopen(buf, "r");




Re: [PATCH] iotests: fix source directory location

2022-07-11 Thread John Snow
On Fri, May 27, 2022, 12:29 PM Kevin Wolf  wrote:

> Am 26.05.2022 um 16:21 hat John Snow geschrieben:
> > On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé 
> > wrote:
> >
> > > On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> > > > If you invoke the check script from outside of the tests/qemu-iotests
> > > > directory, the directories initialized as source_iotests and
> > > > build_iotests will be incorrect.
> > > >
> > > > We can use the location of the source file itself to be more
> accurate.
> > > >
> > > > Signed-off-by: John Snow 
> > > > Reviewed-by: Paolo Bonzini 
> > > > ---
> > > >  tests/qemu-iotests/testenv.py | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tests/qemu-iotests/testenv.py
> > > b/tests/qemu-iotests/testenv.py
> > > > index a864c74b123..9b0f01e84db 100644
> > > > --- a/tests/qemu-iotests/testenv.py
> > > > +++ b/tests/qemu-iotests/testenv.py
> > > > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str,
> > > aiomode: str,
> > > >  self.build_iotests =
> > > os.path.dirname(os.path.abspath(sys.argv[0]))
> > > >  else:
> > > >  # called from the source tree
> > > > -self.source_iotests = os.getcwd()
> > > > +self.source_iotests = str(Path(__file__,
> '..').resolve())
> > >
> > > Path(__file__).parent
> > >
> > > >  self.build_iotests = self.source_iotests
> > > >
> > > > -self.build_root = os.path.join(self.build_iotests, '..',
> '..')
> > > > +self.build_root = str(Path(self.build_iotests,
> > > '../..').resolve())
> > >
> > > Path(self.build_iotests).parent.parent
> > >
> > > to be portable
> > >
> >
> > With windows? I think Path() is meant to be a fully portable class as-is,
> > but I'll double-check my assumption. I use ".." elsewhere in code already
> > checked in, so if it's a problem I ought to fix it everywhere.
>
> I don't see any potential problem with the second hunk because we're
> dealing with the path of a directory there, but "regular_file.py/.."
> looks a bit fishy to me and doesn't work if you ask the kernel. Is this
> guaranteed to work in Python or is it an implementation detail of Path
> that may change?
>

... I apparently never hit send on this draft reply:

Good question, I don't know. I just know that when starting from __file__,
it seems to work in that manner. I used that trick when I added the
PYTHONPATH stuff directly into testenv a while back and never thought more
of it.

If it makes people uneasy to look at, I can just use .parent for the
Principle of Least Surprise.


Re: [PATCH] python: QEMUMachine: enable qmp accept timeout by default

2022-07-11 Thread John Snow
On Mon, Jul 11, 2022 at 5:16 PM John Snow  wrote:
>
> On Fri, Jun 24, 2022 at 3:53 PM Vladimir Sementsov-Ogievskiy
>  wrote:
> >
> > I've spent much time trying to debug hanging pipeline in gitlab. I
> > started from and idea that I have problem in code in my series (which
> > has some timeouts). Finally I found that the problem is that I've used
> > QEMUMachine class directly to avoid qtest, and didn't add necessary
> > arguments. Qemu fails and we wait for qmp accept endlessly. In gitlab
> > it's just stopped by timeout (one hour) with no sign of what's going
> > wrong.
> >
> > With timeout enabled, gitlab don't wait for an hour and prints all
> > needed information.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > ---
> >
> > Hi all!
> >
> > Just compare this
> >   https://gitlab.com/vsementsov/qemu/-/pipelines/572232557
> > and this
> >   https://gitlab.com/vsementsov/qemu/-/pipelines/572526252
> >
> > and you'll see that the latter is much better.
> >
> >  python/qemu/machine/machine.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> > index 37191f433b..01a12f6f73 100644
> > --- a/python/qemu/machine/machine.py
> > +++ b/python/qemu/machine/machine.py
> > @@ -131,7 +131,7 @@ def __init__(self,
> >   drain_console: bool = False,
> >   console_log: Optional[str] = None,
> >   log_dir: Optional[str] = None,
> > - qmp_timer: Optional[float] = None):
> > + qmp_timer: float = 30):
> >  '''
> >  Initialize a QEMUMachine
> >
> > --
> > 2.25.1
> >
>
> Oh, this is because machine.py uses the qmp_timer for *all* timeouts,
> and not just the QMP commands themselves, and this relates to the work
> Marc Andre is doing with regards to changing the launch mechanism to
> handle the race condition when the QEMU launch fails, but the QMP
> connection just sits waiting.
>
> I'm quite of the mind that it's really time to rewrite machine.py to
> use the native asyncio interfaces I've been writing to help manage
> this, but in the meantime I think this is probably a reasonable
> concession and a more useful default.
>
> ...I think. Willing to take it for now and re-investigate when the
> other fixes make it to the tree.
>
> Reviewed-by: John Snow 

Oh, keep the type as Optional[float], though, so the timeout can be
disabled again, and keeps the type consistent with the qtest
derivative class. I've staged your patch with that change made, let me
know if that's not OK. Modified patch is on my python branch:

Thanks, merged.

--js




Re: [PATCH] python: QEMUMachine: enable qmp accept timeout by default

2022-07-11 Thread John Snow
On Fri, Jun 24, 2022 at 3:53 PM Vladimir Sementsov-Ogievskiy
 wrote:
>
> I've spent much time trying to debug hanging pipeline in gitlab. I
> started from and idea that I have problem in code in my series (which
> has some timeouts). Finally I found that the problem is that I've used
> QEMUMachine class directly to avoid qtest, and didn't add necessary
> arguments. Qemu fails and we wait for qmp accept endlessly. In gitlab
> it's just stopped by timeout (one hour) with no sign of what's going
> wrong.
>
> With timeout enabled, gitlab don't wait for an hour and prints all
> needed information.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> Hi all!
>
> Just compare this
>   https://gitlab.com/vsementsov/qemu/-/pipelines/572232557
> and this
>   https://gitlab.com/vsementsov/qemu/-/pipelines/572526252
>
> and you'll see that the latter is much better.
>
>  python/qemu/machine/machine.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 37191f433b..01a12f6f73 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -131,7 +131,7 @@ def __init__(self,
>   drain_console: bool = False,
>   console_log: Optional[str] = None,
>   log_dir: Optional[str] = None,
> - qmp_timer: Optional[float] = None):
> + qmp_timer: float = 30):
>  '''
>  Initialize a QEMUMachine
>
> --
> 2.25.1
>

Oh, this is because machine.py uses the qmp_timer for *all* timeouts,
and not just the QMP commands themselves, and this relates to the work
Marc Andre is doing with regards to changing the launch mechanism to
handle the race condition when the QEMU launch fails, but the QMP
connection just sits waiting.

I'm quite of the mind that it's really time to rewrite machine.py to
use the native asyncio interfaces I've been writing to help manage
this, but in the meantime I think this is probably a reasonable
concession and a more useful default.

...I think. Willing to take it for now and re-investigate when the
other fixes make it to the tree.

Reviewed-by: John Snow 




[PATCH v4 3/3] migration/multifd: Report to user when zerocopy not working

2022-07-11 Thread Leonardo Bras
Some errors, like the lack of Scatter-Gather support by the network
interface(NETIF_F_SG) may cause sendmsg(...,MSG_ZEROCOPY) to fail on using
zero-copy, which causes it to fall back to the default copying mechanism.

After each full dirty-bitmap scan there should be a zero-copy flush
happening, which checks for errors each of the previous calls to
sendmsg(...,MSG_ZEROCOPY). If all of them failed to use zero-copy, then
increment dirty_sync_missed_zero_copy migration stat to let the user know
about it.

Signed-off-by: Leonardo Bras 
Reviewed-by: Daniel P. Berrangé 
---
 migration/ram.h | 2 ++
 migration/multifd.c | 2 ++
 migration/ram.c | 5 +
 3 files changed, 9 insertions(+)

diff --git a/migration/ram.h b/migration/ram.h
index ded0a3a086..d3c7eb96f5 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -87,4 +87,6 @@ void ram_write_tracking_prepare(void);
 int ram_write_tracking_start(void);
 void ram_write_tracking_stop(void);
 
+void dirty_sync_missed_zero_copy(void);
+
 #endif
diff --git a/migration/multifd.c b/migration/multifd.c
index 684c014c86..3909b34967 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -624,6 +624,8 @@ int multifd_send_sync_main(QEMUFile *f)
 if (ret < 0) {
 error_report_err(err);
 return -1;
+} else if (ret == 1) {
+dirty_sync_missed_zero_copy();
 }
 }
 }
diff --git a/migration/ram.c b/migration/ram.c
index 01f9cc1d72..db948c4787 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -407,6 +407,11 @@ static void ram_transferred_add(uint64_t bytes)
 ram_counters.transferred += bytes;
 }
 
+void dirty_sync_missed_zero_copy(void)
+{
+ram_counters.dirty_sync_missed_zero_copy++;
+}
+
 /* used by the search for pages to send */
 struct PageSearchStatus {
 /* Current block being searched */
-- 
2.37.0




[PATCH v4 2/3] Add dirty-sync-missed-zero-copy migration stat

2022-07-11 Thread Leonardo Bras
Signed-off-by: Leonardo Bras 
Acked-by: Markus Armbruster 
Reviewed-by: Daniel P. Berrangé 
---
 qapi/migration.json   | 7 ++-
 migration/migration.c | 2 ++
 monitor/hmp-cmds.c| 5 +
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 7102e474a6..4a03e8f173 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -55,6 +55,10 @@
 # @postcopy-bytes: The number of bytes sent during the post-copy phase
 #  (since 7.0).
 #
+# @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization could
+#   not avoid copying dirty pages. This is between
+#   0 and @dirty-sync-count * @multifd-channels.
+#   (since 7.1)
 # Since: 0.14
 ##
 { 'struct': 'MigrationStats',
@@ -65,7 +69,8 @@
'postcopy-requests' : 'int', 'page-size' : 'int',
'multifd-bytes' : 'uint64', 'pages-per-second' : 'uint64',
'precopy-bytes' : 'uint64', 'downtime-bytes' : 'uint64',
-   'postcopy-bytes' : 'uint64' } }
+   'postcopy-bytes' : 'uint64',
+   'dirty-sync-missed-zero-copy' : 'uint64' } }
 
 ##
 # @XBZRLECacheStats:
diff --git a/migration/migration.c b/migration/migration.c
index 78f5057373..048f7f8bdb 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1027,6 +1027,8 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->normal_bytes = ram_counters.normal * page_size;
 info->ram->mbps = s->mbps;
 info->ram->dirty_sync_count = ram_counters.dirty_sync_count;
+info->ram->dirty_sync_missed_zero_copy =
+ram_counters.dirty_sync_missed_zero_copy;
 info->ram->postcopy_requests = ram_counters.postcopy_requests;
 info->ram->page_size = page_size;
 info->ram->multifd_bytes = ram_counters.multifd_bytes;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ca98df0495..a6dc79e0d5 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -307,6 +307,11 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "postcopy ram: %" PRIu64 " kbytes\n",
info->ram->postcopy_bytes >> 10);
 }
+if (info->ram->dirty_sync_missed_zero_copy) {
+monitor_printf(mon,
+   "Zero-copy-send fallbacks happened: %" PRIu64 " 
times\n",
+   info->ram->dirty_sync_missed_zero_copy);
+}
 }
 
 if (info->has_disk) {
-- 
2.37.0




[PATCH v4 1/3] QIOChannelSocket: Fix zero-copy flush returning code 1 when nothing sent

2022-07-11 Thread Leonardo Bras
If flush is called when no buffer was sent with MSG_ZEROCOPY, it currently
returns 1. This return code should be used only when Linux fails to use
MSG_ZEROCOPY on a lot of sendmsg().

Fix this by returning early from flush if no sendmsg(...,MSG_ZEROCOPY)
was attempted.

Fixes: 2bc58ffc2926 ("QIOChannelSocket: Implement io_writev zero copy flag & 
io_flush for CONFIG_LINUX")
Signed-off-by: Leonardo Bras 
Reviewed-by: Daniel P. Berrangé 
Acked-by: Daniel P. Berrangé 
Reviewed-by: Juan Quintela 
---
 io/channel-socket.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index 4466bb1cd4..74a936cc1f 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -716,12 +716,18 @@ static int qio_channel_socket_flush(QIOChannel *ioc,
 struct cmsghdr *cm;
 char control[CMSG_SPACE(sizeof(*serr))];
 int received;
-int ret = 1;
+int ret;
+
+if (sioc->zero_copy_queued == sioc->zero_copy_sent) {
+return 0;
+}
 
 msg.msg_control = control;
 msg.msg_controllen = sizeof(control);
 memset(control, 0, sizeof(control));
 
+ret = 1;
+
 while (sioc->zero_copy_sent < sioc->zero_copy_queued) {
 received = recvmsg(sioc->fd, , MSG_ERRQUEUE);
 if (received < 0) {
-- 
2.37.0




[PATCH v4 0/3] Zero copy improvements (QIOChannel + multifd)

2022-07-11 Thread Leonardo Bras
The first patch avoid spuriously returning 1 [*] when zero-copy flush is
attempted before any buffer was sent using MSG_ZEROCOPY.

[*] zero-copy not being used, even though it's enabled and supported
by kernel

The second patch introduces a new migration stat
(dirty-sync-missed-zero-copy) that will be used to keep track of [*]. 

The third patch keeps track of how many zero-copy flushes retured 1 [*]

Changes since v3:
- Patch#1: Also checks if no packet was queued after the last flush
- Patch#2: Improve dirty-sync-missed-zero-copy doc and hmp print message

Changes since v2:
- Documentation release number changed from 7.2 to 7.1
- migration stat renamed from zero-copy-copied to 
  dirty-sync-missed-zero-copy
- Updated documentation to make it more user-friendly

Changes since v1:
- Idea of using a warning replaced by using a migration stat counter



Leonardo Bras (3):
  QIOChannelSocket: Fix zero-copy flush returning code 1 when nothing
sent
  Add dirty-sync-missed-zero-copy migration stat
  migration/multifd: Report to user when zerocopy not working

 qapi/migration.json   | 7 ++-
 migration/ram.h   | 2 ++
 io/channel-socket.c   | 8 +++-
 migration/migration.c | 2 ++
 migration/multifd.c   | 2 ++
 migration/ram.c   | 5 +
 monitor/hmp-cmds.c| 5 +
 7 files changed, 29 insertions(+), 2 deletions(-)

-- 
2.37.0




Re: [PATCH RESEND] python/machine: Fix AF_UNIX path too long on macOS

2022-07-11 Thread John Snow
On Thu, Jul 7, 2022 at 2:46 PM Peter Delevoryas  wrote:
>
> On Wed, Jul 06, 2022 at 05:52:48PM -0700, Peter Delevoryas wrote:
> > On Wed, Jul 06, 2022 at 09:46:48AM -0700, Peter Delevoryas wrote:
> > > On Wed, Jul 06, 2022 at 09:02:14AM +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Jul 05, 2022 at 02:46:59PM -0700, Peter Delevoryas wrote:
> > > > > I noticed that I can't run any avocado tests on macOS because the QMP
> > > > > unix socket path is too long:
> > > >
> > > >
> > > > > I think the path limit for unix sockets on macOS might be 104 [1]
> > > >
> > > > All platforms have a very limited path limit, so it isn't really
> > > > a macOS specific problem, rather
> > > >
> > > > >
> > > > > /*
> > > > >  * [XSI] Definitions for UNIX IPC domain.
> > > > >  */
> > > > > struct  sockaddr_un {
> > > > > unsigned char   sun_len;/* sockaddr len including null */
> > > > > sa_family_t sun_family; /* [XSI] AF_UNIX */
> > > > > charsun_path[104];  /* [XSI] path name (gag) */
> > > > > };
> > > > >
> > > > > The path we're using is exactly 105 characters:
> > > > >
> > > > > $ python
> > > > > Python 2.7.10 (default, Jan 19 2016, 22:24:01)
> > > > > [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
> > > > > Type "help", "copyright", "credits" or "license" for more information.
> > > > > >>> len('/var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock')
> > > >
> > > > It is a problem related to where the test suite is creating the
> > > > paths.
> > > >
> > > > /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/avo_qemu_sock_uh3w_dgc/
> > > >
> > > > is way too deep a directory location.
> > >
> > > That's a good point.
> > >
> > > >
> > > > It seems we just create this location using 'tempfile.TemporyDirectory'
> > > > to get a standard tmp dir.
> > > >
> > > > Do you know why python is choosing
> > > >
> > > >   /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/
> > > >
> > > > as the temp dir ? Is that a standard location on macOS or is it
> > > > from some env variable you have set ?
> > >
> > > Hmmm yeah it is odd, I'm not really sure why it's created there or if
> > > standard glibc tmpfile creation goes there too, I'll go experiment and
> > > report back. And yeah, maybe I'll double check any environment variables 
> > > or
> > > other things.
> > >
> > > The macOS system I use happens to be a Facebook work laptop, which could
> > > also be related now that I think about it.
> >
> > Hmmm yeah looks like this is because my TMPDIR is weird.
> >
> > $ echo $TMPDIR
> > /var/folders/d7/rz20f6hd709c1ty8f6_6y_z4gn/T/
> >
> > I didn't think to check this cause I wasn't familiar with TMPDIR. 路
> >
> > Thanks for responding, I'll just use TMPDIR=/tmp for now. It's probably
> > something wrong with the Facebook development environment.
> >
> > Peter
>
> Update: Actually, this might not be a Facebook-work-laptop specific
> thing.  I asked my non-engineer friend to print out $TMPDIR on his
> macbook and he got the same thing.
>
> https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
>
> I guess this person suggests it's just to separate the permissions for
> each user's /tmp directory, for better isolation.
>
> I'll resubmit this patch with the suggestions you had, because perhaps
> this is actually affecting other macOS users too.
>
> >
> > >
> > > >
> > > > > diff --git a/python/qemu/machine/machine.py 
> > > > > b/python/qemu/machine/machine.py
> > > > > index 37191f433b..93451774e3 100644
> > > > > --- a/python/qemu/machine/machine.py
> > > > > +++ b/python/qemu/machine/machine.py
> > > > > @@ -157,7 +157,7 @@ def __init__(self,
> > > > >  self._wrapper = wrapper
> > > > >  self._qmp_timer = qmp_timer
> > > > >
> > > > > -self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
> > > > > +self._name = name or f"{os.getpid()}{id(self):02x}"
> > > >
> > > > I don't think this is the right fix really, because IMHO the problem
> > > > is the hugely long path, rather than the final socket name.
> > >
> > > True, yeah let me try to investigate the directory placement.
> > >
> > > >
> > > > That said, there is redundancy in the path - avocado is passing in
> > > > a dierctory created using 'tempfile.TemporyDirectory' so there is no
> > > > reason why we need to add more entropy via the POD and the 'id(self)'
> > > > hex string.
> > >
> > > Oh good point, I hadn't thought about that.
> > >
> > > >
> > > > IMHO avocado should pass in the 'name' parameter explicitly, using a
> > > > plain name and thus get a shorter string.
> > >
> > > I see, yeah that makes sense. Maybe I can include a couple patches for 
> > > this,
> > > one fixing the directory location, and one refactoring the temporary file
> > > name template (If I'm understanding your suggestion correctly).
> > >
> > > Thanks for the review! I really appreciate it.
> > > Peter

I 

Re: [PATCH v2 10/11] pytest: add pytest to the meson build system

2022-07-11 Thread John Snow
On Sun, Jul 10, 2022 at 1:01 PM Ani Sinha  wrote:
>
> Integrate the pytest framework with the meson build system. This will make 
> meson
> run all the pytests under the pytest directory.
>
> Signed-off-by: Ani Sinha 
> ---
>  tests/Makefile.include   |  4 +++-
>  tests/meson.build|  1 +
>  tests/pytest/meson.build | 49 
>  3 files changed, 53 insertions(+), 1 deletion(-)
>  create mode 100644 tests/pytest/meson.build
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 3accb83b13..40755a6bd1 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -3,12 +3,14 @@
>  .PHONY: check-help
>  check-help:
> @echo "Regression testing targets:"
> -   @echo " $(MAKE) check  Run block, qapi-schema, unit, 
> softfloat, qtest and decodetree tests"
> +   @echo " $(MAKE) check  Run block, qapi-schema, unit, 
> softfloat, qtest, pytest and decodetree tests"

Does this mean that "make check" *requires* an internet connection? If
so, I'm afraid that introduces some complications for downstreams
which require that "make check" can run without an internet
connection. It's something I've been trying to wrestle with as I split
the qemu.qmp library out of the QEMU tree, and I have been working
(slowly) on remedying it with some additional Makefile logic.

I have been looking into making a testing "dummy package" for python
with optional dependency groups that use a venv building script to
either pull from PyPI in online mode, or use system packages in
offline mode. In the case of offline mode, I am working on augmenting
the configure script to test that dependencies are met, and disabling
test groups when the correct dependencies cannot be located.

I hope to have another version of that series soon; it should be
trivial to add a new optional dependency group to it. I'll CC you on
the next version of the series.

--js

> @echo " $(MAKE) bench  Run speed tests"
> @echo
> @echo "Individual test suites:"
> @echo " $(MAKE) check-qtest-TARGET Run qtest tests for given 
> target"
> @echo " $(MAKE) check-qtestRun qtest tests"
> +   @echo " $(MAKE) check-pytest   Run pytest tests"
> +   @echo " $(MAKE) check-pytest-TARGETRun pytest for a given target"
> @echo " $(MAKE) check-unit Run qobject tests"
> @echo " $(MAKE) check-qapi-schema  Run QAPI schema tests"
> @echo " $(MAKE) check-blockRun block tests"
> diff --git a/tests/meson.build b/tests/meson.build
> index 8e318ec513..f344cbdc6c 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -93,3 +93,4 @@ subdir('unit')
>  subdir('qapi-schema')
>  subdir('qtest')
>  subdir('migration')
> +subdir('pytest')
> diff --git a/tests/pytest/meson.build b/tests/pytest/meson.build
> new file mode 100644
> index 00..e60d481ae4
> --- /dev/null
> +++ b/tests/pytest/meson.build
> @@ -0,0 +1,49 @@
> +slow_pytests = {
> +  'acpi-bits' : 120,
> +}
> +
> +pytests_generic = []
> +
> +# biosbits tests are currenly only supported on x86_64 platforms.
> +pytests_x86_64 = ['acpi-bits-test']
> +
> +pytest_executables = {}
> +other_deps = []
> +
> +subdir('acpi-bits')
> +
> +foreach dir : target_dirs
> +  if not dir.endswith('-softmmu')
> +continue
> +  endif
> +
> +  target_base = dir.split('-')[0]
> +  pytest_emulator = emulators['qemu-system-' + target_base]
> +  target_pytests = get_variable('pytests_' + target_base, []) + 
> pytests_generic
> +
> +  test_deps = roms
> +  pytest_env = environment()
> +  if have_tools
> +pytest_env.set('QTEST_QEMU_IMG', './qemu-img')
> +test_deps += [qemu_img]
> +  endif
> +  pytest_env.set('G_TEST_DBUS_DAEMON', meson.project_source_root() / 
> 'tests/dbus-vmstate-daemon.sh')
> +  pytest_env.set('PYTEST_QEMU_BINARY', './qemu-system-' + target_base)
> +  pytest_env.set('PYTEST_SOURCE_ROOT', meson.project_source_root())
> +  if have_tools and have_vhost_user_blk_server
> +pytest_env.set('PYTEST_QEMU_STORAGE_DAEMON_BINARY', 
> './storage-daemon/qemu-storage-daemon')
> +test_deps += [qsd]
> +  endif
> +
> +  foreach test : target_pytests
> +test('pytest-@0@/@1@'.format(target_base, test),
> + pytest_executables[test],
> + depends: [test_deps, pytest_emulator, emulator_modules, other_deps],
> + env: pytest_env,
> + args: ['--tap', '-k'],
> + protocol: 'tap',
> + timeout: slow_pytests.get(test, 30),
> + priority: slow_pytests.get(test, 30),
> + suite: ['pytest', 'pytest-' + target_base])
> +  endforeach
> +endforeach
> --
> 2.25.1
>




Re: [PATCH v2 07/11] acpi/tests/bits: add python test that exercizes QEMU bios tables using biosbits

2022-07-11 Thread John Snow
On Sun, Jul 10, 2022 at 1:01 PM Ani Sinha  wrote:
>
> This change adds python based test environment that can be used to run pytest
> from within a virtual environment. A bash script sets up a virtual environment
> and then runs the python based tests from within that environment.
> All dependent python packages are installed in the virtual environment using
> pip python module. QEMU python test modules are also available in the 
> environment
> for spawning the QEMU based VMs.
>
> It also introduces QEMU acpi/smbios biosbits python test script which is run
> from within the python virtual environment. When the bios bits tests are run,
> bios bits binaries are downloaded from an external repo/location.
> Currently, the test points to an external private github repo where the bits
> archives are checked in.
>

Oh, I see -- requirements are handled here in this patch.

Is this test designed to run the host/build system? I'm a little
confused about the environment here.

Is this test going to be run "by default" or will users have to opt
into running it using a special invocation?

--js

> Signed-off-by: Ani Sinha 
> ---
>  tests/pytest/acpi-bits/acpi-bits-test-venv.sh |  59 +++
>  tests/pytest/acpi-bits/acpi-bits-test.py  | 382 ++
>  tests/pytest/acpi-bits/meson.build|  33 ++
>  tests/pytest/acpi-bits/requirements.txt   |   1 +
>  4 files changed, 475 insertions(+)
>  create mode 100644 tests/pytest/acpi-bits/acpi-bits-test-venv.sh
>  create mode 100644 tests/pytest/acpi-bits/acpi-bits-test.py
>  create mode 100644 tests/pytest/acpi-bits/meson.build
>  create mode 100644 tests/pytest/acpi-bits/requirements.txt
>
> diff --git a/tests/pytest/acpi-bits/acpi-bits-test-venv.sh 
> b/tests/pytest/acpi-bits/acpi-bits-test-venv.sh
> new file mode 100644
> index 00..186395473b
> --- /dev/null
> +++ b/tests/pytest/acpi-bits/acpi-bits-test-venv.sh
> @@ -0,0 +1,59 @@
> +#!/usr/bin/env bash
> +# Generates a python virtual environment for the test to run.
> +# Then runs python test scripts from within that virtual environment.
> +#
> +# 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 .
> +#
> +# Author: Ani Sinha 
> +
> +set -e
> +
> +MYPATH=$(realpath ${BASH_SOURCE:-$0})
> +MYDIR=$(dirname $MYPATH)
> +
> +if [ -z "$PYTEST_SOURCE_ROOT" ]; then
> +echo -n "Please set QTEST_SOURCE_ROOT env pointing"
> +echo " to the root of the qemu source tree."
> +echo -n "This is required so that the test can find the "
> +echo "python modules that it needs for execution."
> +exit 1
> +fi
> +SRCDIR=$PYTEST_SOURCE_ROOT
> +TESTSCRIPTS=("acpi-bits-test.py")
> +PIPCMD="-m pip -q --disable-pip-version-check"
> +# we need to save the old value of PWD before we do a change-dir later
> +PYTEST_PWD=$PWD
> +
> +TESTS_PYTHON=/usr/bin/python3
> +TESTS_VENV_REQ=requirements.txt
> +
> +# sadly for pip -e and -t options do not work together.
> +# please see https://github.com/pypa/pip/issues/562
> +cd $MYDIR
> +
> +$TESTS_PYTHON -m venv .
> +$TESTS_PYTHON $PIPCMD install -e $SRCDIR/python/
> +[ -f $TESTS_VENV_REQ ] && \
> +$TESTS_PYTHON $PIPCMD install -r $TESTS_VENV_REQ || exit 0
> +
> +# venv is activated at this point.
> +
> +# run the test
> +for testscript in ${TESTSCRIPTS[@]} ; do
> +export PYTEST_PWD; python3 $testscript
> +done
> +
> +cd $PYTEST_PWD
> +
> +exit 0
> diff --git a/tests/pytest/acpi-bits/acpi-bits-test.py 
> b/tests/pytest/acpi-bits/acpi-bits-test.py
> new file mode 100644
> index 00..97e61eb709
> --- /dev/null
> +++ b/tests/pytest/acpi-bits/acpi-bits-test.py
> @@ -0,0 +1,382 @@
> +#!/usr/bin/env python3
> +# group: rw quick
> +# Exercize QEMU generated ACPI/SMBIOS tables using biosbits,
> +# https://biosbits.org/
> +#
> +# 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 

Re: [PATCH v2 01/11] acpi/tests/bits: initial commit of test scripts that are run by biosbits

2022-07-11 Thread John Snow
On Sun, Jul 10, 2022 at 1:01 PM Ani Sinha  wrote:
>
> This is initial commit of cpuid, acpi and smbios python test scripts for
> biosbits to execute. No change has been made to them from the original code
> written by the biosbits author Josh Triplett. They are required to be 
> installed
> into the bits iso file and then run from within the virtual machine booted off
> with biosbits iso.
>
> The original location of these tests are here:
> https://github.com/biosbits/bits/blob/master/python/testacpi.py
> https://github.com/biosbits/bits/blob/master/python/smbios.py
> https://github.com/biosbits/bits/blob/master/python/testcpuid.py
>
> Signed-off-by: Ani Sinha 
> ---
>  tests/pytest/acpi-bits/bits-tests/meson.build |   11 +
>  tests/pytest/acpi-bits/bits-tests/smbios.py   | 2430 +
>  tests/pytest/acpi-bits/bits-tests/testacpi.py |  283 ++
>  .../pytest/acpi-bits/bits-tests/testcpuid.py  |   83 +
>  4 files changed, 2807 insertions(+)
>  create mode 100644 tests/pytest/acpi-bits/bits-tests/meson.build
>  create mode 100644 tests/pytest/acpi-bits/bits-tests/smbios.py
>  create mode 100644 tests/pytest/acpi-bits/bits-tests/testacpi.py
>  create mode 100644 tests/pytest/acpi-bits/bits-tests/testcpuid.py
>
> diff --git a/tests/pytest/acpi-bits/bits-tests/meson.build 
> b/tests/pytest/acpi-bits/bits-tests/meson.build
> new file mode 100644
> index 00..3056731a53
> --- /dev/null
> +++ b/tests/pytest/acpi-bits/bits-tests/meson.build
> @@ -0,0 +1,11 @@
> +test_files = ['smbios.py', 'testacpi.py', 'testcpuid.py']
> +
> +copytestfiles = custom_target('copy test files',
> +  input : test_files,
> +  output :  test_files,
> +  command : ['cp', '@INPUT@', '@OUTDIR@'],
> +  install : true,
> +  install_dir : 'bits-tests',
> +  build_by_default : true)
> +
> +other_deps += copytestfiles
> diff --git a/tests/pytest/acpi-bits/bits-tests/smbios.py 
> b/tests/pytest/acpi-bits/bits-tests/smbios.py
> new file mode 100644
> index 00..9667d0542c
> --- /dev/null
> +++ b/tests/pytest/acpi-bits/bits-tests/smbios.py
> @@ -0,0 +1,2430 @@
> +# Copyright (c) 2015, Intel Corporation
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are met:
> +#
> +# * Redistributions of source code must retain the above copyright 
> notice,
> +#   this list of conditions and the following disclaimer.
> +# * Redistributions in binary form must reproduce the above copyright 
> notice,
> +#   this list of conditions and the following disclaimer in the 
> documentation
> +#   and/or other materials provided with the distribution.
> +# * Neither the name of Intel Corporation nor the names of its 
> contributors
> +#   may be used to endorse or promote products derived from this software
> +#   without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS" AND
> +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE 
> IMPLIED
> +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE 
> LIABLE FOR
> +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL 
> DAMAGES
> +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND ON
> +# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +"""SMBIOS/DMI module."""
> +
> +import bits
> +import bitfields

New deps?

> +import ctypes
> +import redirect

Also a new dep?

> +import struct
> +import uuid
> +import unpack

And another?

> +import ttypager
> +import sys

What's the proposed strategy for dependency management for these
tests? I know there's some mail I'm backlogged on ...




Re: [PATCH] python/qemu/qmp/legacy: Replace 'returns-whitelist' with the correct type

2022-07-11 Thread John Snow
On Mon, Jul 11, 2022 at 4:30 PM John Snow  wrote:
>
> On Mon, Jul 11, 2022 at 5:57 AM Thomas Huth  wrote:
> >
> > 'returns-whitelist' has been renamed to 'command-returns-exceptions' in
> > commit b86df3747848 ("qapi: Rename pragma *-whitelist to *-exceptions").
> >
> > Signed-off-by: Thomas Huth 
> > ---
> >  python/qemu/qmp/legacy.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
> > index 03b5574618..1951754455 100644
> > --- a/python/qemu/qmp/legacy.py
> > +++ b/python/qemu/qmp/legacy.py
> > @@ -50,7 +50,7 @@
> >
> >  # QMPMessage can be outgoing commands or incoming events/returns.
> >  # QMPReturnValue is usually a dict/json object, but due to QAPI's
> > -# 'returns-whitelist', it can actually be anything.
> > +# 'command-returns-exceptions', it can actually be anything.
> >  #
> >  # {'return': {}} is a QMPMessage,
> >  # {} is the QMPReturnValue.
> > --
> > 2.31.1
> >
>
> May I cajole you to send a MR to
> https://gitlab.com/qemu-project/python-qemu-qmp ?
>
> (Sorry for the duplicated effort while the library isn't 100% split
> out of the tree yet.)
>
> In this case, for qemu.git:
>
> Reviewed-by: John Snow 

Queued.

--js




Re: [PATCH] python/qemu/qmp/legacy: Replace 'returns-whitelist' with the correct type

2022-07-11 Thread John Snow
On Mon, Jul 11, 2022 at 5:57 AM Thomas Huth  wrote:
>
> 'returns-whitelist' has been renamed to 'command-returns-exceptions' in
> commit b86df3747848 ("qapi: Rename pragma *-whitelist to *-exceptions").
>
> Signed-off-by: Thomas Huth 
> ---
>  python/qemu/qmp/legacy.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
> index 03b5574618..1951754455 100644
> --- a/python/qemu/qmp/legacy.py
> +++ b/python/qemu/qmp/legacy.py
> @@ -50,7 +50,7 @@
>
>  # QMPMessage can be outgoing commands or incoming events/returns.
>  # QMPReturnValue is usually a dict/json object, but due to QAPI's
> -# 'returns-whitelist', it can actually be anything.
> +# 'command-returns-exceptions', it can actually be anything.
>  #
>  # {'return': {}} is a QMPMessage,
>  # {} is the QMPReturnValue.
> --
> 2.31.1
>

May I cajole you to send a MR to
https://gitlab.com/qemu-project/python-qemu-qmp ?

(Sorry for the duplicated effort while the library isn't 100% split
out of the tree yet.)

In this case, for qemu.git:

Reviewed-by: John Snow 




Re: [PATCH] Replace 'whitelist' with 'allow'

2022-07-11 Thread John Snow
On Mon, Jul 11, 2022 at 5:53 AM Thomas Huth  wrote:
>
> Let's use more inclusive language here and avoid terms
> that are frowned upon nowadays.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: John Snow 

> ---
>  docs/devel/submitting-a-patch.rst | 2 +-
>  docs/tools/qemu-nbd.rst   | 2 +-
>  scripts/vmstate-static-checker.py | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/docs/devel/submitting-a-patch.rst 
> b/docs/devel/submitting-a-patch.rst
> index 09a8d12c2c..fec33ce148 100644
> --- a/docs/devel/submitting-a-patch.rst
> +++ b/docs/devel/submitting-a-patch.rst
> @@ -39,7 +39,7 @@ ideas from other posts. If you do subscribe, be prepared 
> for a high
>  volume of email, often over one thousand messages in a week. The list is
>  moderated; first-time posts from an email address (whether or not you
>  subscribed) may be subject to some delay while waiting for a moderator
> -to whitelist your address.
> +to allow your address.
>
>  The larger your contribution is, or if you plan on becoming a long-term
>  contributor, then the more important the rest of this page becomes.
> diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
> index 8e08a29e89..faf6349ea5 100644
> --- a/docs/tools/qemu-nbd.rst
> +++ b/docs/tools/qemu-nbd.rst
> @@ -225,7 +225,7 @@ disconnects:
>qemu-nbd -f qcow2 file.qcow2
>
>  Start a long-running server listening with encryption on port 10810,
> -and whitelist clients with a specific X.509 certificate to connect to
> +and allow clients with a specific X.509 certificate to connect to
>  a 1 megabyte subset of a raw file, using the export name 'subset':
>
>  ::
> diff --git a/scripts/vmstate-static-checker.py 
> b/scripts/vmstate-static-checker.py
> index 539ead62b4..b369388360 100755
> --- a/scripts/vmstate-static-checker.py
> +++ b/scripts/vmstate-static-checker.py
> @@ -40,7 +40,7 @@ def check_fields_match(name, s_field, d_field):
>  return True
>
>  # Some fields changed names between qemu versions.  This list
> -# is used to whitelist such changes in each section / description.
> +# is used to allow such changes in each section / description.
>  changed_names = {
>  'apic': ['timer', 'timer_expiry'],
>  'e1000': ['dev', 'parent_obj'],
> --
> 2.31.1
>




Re: [PATCH v6 09/10] i386/pc: relocate 4g start to 1T where applicable

2022-07-11 Thread Joao Martins
On 7/11/22 16:31, Joao Martins wrote:
> On 7/11/22 15:52, Joao Martins wrote:
>> On 7/11/22 13:56, Igor Mammedov wrote:
>>> On Fri,  1 Jul 2022 17:10:13 +0100
>>> Joao Martins  wrote:
>>>
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index a79fa1b6beeb..07025b510540 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -907,6 +907,87 @@ static uint64_t pc_get_cxl_range_end(PCMachineState 
 *pcms)
  return start;
  }
  
 +static hwaddr pc_max_used_gpa(PCMachineState *pcms,
 +hwaddr above_4g_mem_start,
 +uint64_t pci_hole64_size)
 +{
 +X86MachineState *x86ms = X86_MACHINE(pcms);
 +
>>>
 +if (!x86ms->above_4g_mem_size) {
 +/*
 + * 32-bit pci hole goes from
 + * end-of-low-ram (@below_4g_mem_size) to IOAPIC.
 +  */
 +return IO_APIC_DEFAULT_ADDRESS - 1;
 +}
>>> this hunk still bothers me (nothing changed wrt v5 issues around it)
>>> issues recap: (
>>>  1. correctness of it
>>>  2. being limited to AMD only, while it seems pretty generic to me
>>>  3. should be a separate patch
>>> )
>>>
>> How about I instead delete this hunk, and only call 
>> pc_set_amd_above_4g_mem_start()
>> when there's @above_4g_mem_size ? Like in pc_memory_init() I would instead:
>>
>> if (IS_AMD_CPU(>env) && x86ms->above_4g_mem_size) {
>> hwaddr start = x86ms->above_4g_mem_start;
>>
>> if (pc_max_used_gpa(pcms, start, pci_hole64_size) >= AMD_HT_START) {
>> pc_set_amd_above_4g_mem_start(pcms, pci_hole64_size);
>> }
>> ...
>> }
>>
>> Given that otherwise it is impossible to ever encounter the 1T boundary.
>>
> 
> And while at it I would also remove any unneeded arguments from 
> pc_max_used_gpa(),
> which would turn the function into this:
> 
> +static hwaddr pc_max_used_gpa(uint64_t pci_hole64_size)
> +{
> +return pc_pci_hole64_start() + pci_hole64_size;
> +}
> 
> I would nuke the added helper if it wasn't for having 2 call sites in this 
> patch.
> 

Full patch diff further below -- after removing pc_max_used_gpa() and made 
further
cleanups given this code can be much simpler after using this approach.

>> If not ... what other alternative would address your concern?
>>

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e178bbc4129c..1ded3faeffe0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -882,6 +882,62 @@ static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
 return start;
 }

+/*
+ * AMD systems with an IOMMU have an additional hole close to the
+ * 1Tb, which are special GPAs that cannot be DMA mapped. Depending
+ * on kernel version, VFIO may or may not let you DMA map those ranges.
+ * Starting Linux v5.4 we validate it, and can't create guests on AMD machines
+ * with certain memory sizes. It's also wrong to use those IOVA ranges
+ * in detriment of leading to IOMMU INVALID_DEVICE_REQUEST or worse.
+ * The ranges reserved for Hyper-Transport are:
+ *
+ * FD__h - FF__h
+ *
+ * The ranges represent the following:
+ *
+ * Base Address   Top Address  Use
+ *
+ * FD__h FD_F7FF_h Reserved interrupt address space
+ * FD_F800_h FD_F8FF_h Interrupt/EOI IntCtl
+ * FD_F900_h FD_F90F_h Legacy PIC IACK
+ * FD_F910_h FD_F91F_h System Management
+ * FD_F920_h FD_FAFF_h Reserved Page Tables
+ * FD_FB00_h FD_FBFF_h Address Translation
+ * FD_FC00_h FD_FDFF_h I/O Space
+ * FD_FE00_h FD__h Configuration
+ * FE__h FE_1FFF_h Extended Configuration/Device Messages
+ * FE_2000_h FF__h Reserved
+ *
+ * See AMD IOMMU spec, section 2.1.2 "IOMMU Logical Topology",
+ * Table 3: Special Address Controls (GPA) for more information.
+ */
+#define AMD_HT_START 0xfdUL
+#define AMD_HT_END   0xffUL
+#define AMD_ABOVE_1TB_START  (AMD_HT_END + 1)
+#define AMD_HT_SIZE  (AMD_ABOVE_1TB_START - AMD_HT_START)
+
+static void pc_set_amd_above_4g_mem_start(PCMachineState *pcms,
+  hwaddr maxusedaddr)
+{
+X86MachineState *x86ms = X86_MACHINE(pcms);
+hwaddr maxphysaddr;
+
+/*
+ * Relocating ram-above-4G requires more than TCG_PHYS_ADDR_BITS (40).
+ * So make sure phys-bits is required to be appropriately sized in order
+ * to proceed with the above-4g-region relocation and thus boot.
+ */
+maxphysaddr = ((hwaddr)1 << X86_CPU(first_cpu)->phys_bits) - 1;
+if (maxphysaddr < maxusedaddr) {
+error_report("Address space limit 0x%"PRIx64" < 0x%"PRIx64
+ " phys-bits too low (%u) cannot avoid AMD HT range",
+ maxphysaddr, maxusedaddr, X86_CPU(first_cpu)->phys_bits);
+exit(EXIT_FAILURE);
+}
+
+x86ms->above_4g_mem_start = AMD_ABOVE_1TB_START;
+}
+
 void pc_memory_init(PCMachineState *pcms,
 MemoryRegion *system_memory,
 

Re: [PATCH v2] target/riscv: fix shifts shamt value for rv128c

2022-07-11 Thread Frédéric Pétrot

Hi Richard,

Le 11/07/2022 à 06:44, Richard Henderson a écrit :
On Sun, 10 July 2022, 16:36 Frédéric Pétrot, 
> wrote:


For rv128c shifts, a shamt of 0 is a shamt of 64, while for rv32c/rv64c
it stays 0 and is a hint instruction that does not change processor state.
For rv128c right shifts, the 6-bit shamt is in addition sign extended to
7 bits.


Um, what says that? First, it makes no sense — while some systems define 
negative shifts as opposite direction, riscv is not one of them.


  This is here (I just pulled riscv-isa-manual and recompiled to be right on
  the page): Volume I: RISC-V Unprivileged ISA V20191214-draft (same branch as
  the one you indicate), page 119 & 120.
  This applies *only* to the compressed instructions c.srli and c.srai, not to
  their uncompressed version (the 0 becoming 64 also applies to c.slli).
  "Sign extension" is duplication of the 6th bit on the 7th bit only, and the
  top bits are zeroed, thus leading to shamts of 1-31, 64, 96-127.


Second, the 20220707 draft spec does not agree with you:

   Shifts by an immediate (SLLI/SRLI/SRAI)
   are now encoded using the low 7 bits of
   the I-immediate >
Nothing about that sentence says "signed".


  Agreed, on the non compressed insns, but the compressed ones have a 6-bit
  shamt only as visible on page 18.6 page 125. The explanation for rv128 shifts
  is further detailed in the emphasized paragraph on top of page 120.

  Frédéric



r~


Signed-off-by: Frédéric Pétrot mailto:frederic.pet...@univ-grenoble-alpes.fr>>
---
  target/riscv/insn16.decode |  7 ---
  disas/riscv.c              | 27 +--
  target/riscv/translate.c   | 20 ++--
  3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 02c8f61b48..ccfe59f294 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -31,7 +31,8 @@
  %imm_cb        12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
  %imm_cj        12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1

-%shimm_6bit   12:1 2:5               !function=ex_rvc_shifti
+%shlimm_6bit  12:1 2:5               !function=ex_rvc_shiftli
+%shrimm_6bit  12:1 2:5               !function=ex_rvc_shiftri
  %uimm_6bit_lq 2:4 12:1 6:1           !function=ex_shift_4
  %uimm_6bit_ld 2:3 12:1 5:2           !function=ex_shift_3
  %uimm_6bit_lw 2:2 12:1 4:3           !function=ex_shift_2
@@ -82,9 +83,9 @@
  @c_addi16sp     ... .  . . ..  imm=%imm_addi16sp rs1=2 rd=2

  @c_shift        ... . .. ... . .. \
-                 rd=%rs1_3 rs1=%rs1_3 shamt=%shimm_6bit
+                 rd=%rs1_3 rs1=%rs1_3 shamt=%shrimm_6bit
  @c_shift2       ... . .. ... . .. \
-                 rd=%rd rs1=%rd shamt=%shimm_6bit
+                 rd=%rd rs1=%rd shamt=%shlimm_6bit

  @c_andi         ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3

diff --git a/disas/riscv.c b/disas/riscv.c
index 7af6afc8fa..489c2ae5e8 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2402,10 +2402,25 @@ static int32_t operand_sbimm12(rv_inst inst)
          ((inst << 56) >> 63) << 11;
  }

-static uint32_t operand_cimmsh6(rv_inst inst)
+static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
  {
-    return ((inst << 51) >> 63) << 5 |
+    int imm = ((inst << 51) >> 63) << 5 |
          (inst << 57) >> 59;
+    if (isa == rv128) {
+        imm = imm ? imm : 64;
+    }
+    return imm;
+}
+
+static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
+{
+    int imm = ((inst << 51) >> 63) << 5 |
+        (inst << 57) >> 59;
+    if (isa == rv128) {
+        imm = imm | (imm & 32) << 1;
+        imm = imm ? imm : 64;
+    }
+    return imm;
  }

  static int32_t operand_cimmi(rv_inst inst)
@@ -2529,7 +2544,7 @@ static uint32_t operand_rnum(rv_inst inst)

  /* decode operands */

-static void decode_inst_operands(rv_decode *dec)
+static void decode_inst_operands(rv_decode *dec, rv_isa isa)
  {
      rv_inst inst = dec->inst;
      dec->codec = opcode_data[dec->op].codec;
@@ -2652,7 +2667,7 @@ static void decode_inst_operands(rv_decode *dec)
      case rv_codec_cb_sh6:
          dec->rd = dec->rs1 = operand_crs1rdq(inst) + 8;
          dec->rs2 = rv_ireg_zero;
-        dec->imm = operand_cimmsh6(inst);
+        dec->imm = operand_cimmshr6(inst, isa);
          break;
      case rv_codec_ci:
          dec->rd = dec->rs1 = operand_crs1rd(inst);
@@ -2667,7 +2682,7 @@ static void decode_inst_operands(rv_decode *dec)
      case rv_codec_ci_sh6:
          dec->rd = dec->rs1 = operand_crs1rd(inst);
          dec->rs2 = rv_ireg_zero;
-        dec->imm = 

[PATCH] target/ppc/kvm: Skip ".." directory in kvmppc_find_cpu_dt

2022-07-11 Thread Murilo Opsfelder Araujo
Some systems have /proc/device-tree/cpus/../clock-frequency. However,
this is not the expected path for a CPU device tree directory.

Signed-off-by: Murilo Opsfelder Araujo 
Signed-off-by: Fabiano Rosas 
---
 target/ppc/kvm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 6eed466f80..c8485a5cc0 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len)
 buf[0] = '\0';
 while ((dirp = readdir(dp)) != NULL) {
 FILE *f;
+
+/* Don't accidentally read from the upper directory */
+if (strcmp(dirp->d_name, "..") == 0) {
+continue;
+}
+
 snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU,
  dirp->d_name);
 f = fopen(buf, "r");
-- 
2.36.1




Re: [PATCH v3 1/3] QIOChannelSocket: Fix zero-copy flush returning code 1 when nothing sent

2022-07-11 Thread Leonardo Bras Soares Passos
On Thu, Jul 7, 2022 at 7:18 PM Peter Xu  wrote:
>
> On Thu, Jul 07, 2022 at 06:14:17PM -0300, Leonardo Brás wrote:
> > Having 'if(queued == sent)' will cause us to falsely return '1' in two buggy
> > cases, while 'if queued == 0) will either skip early or go into 'infinite' 
> > loop.
>
> I'm not sure I strictly follow here..
>

Sorry, I was thinking of a different scenario.

> Imagine the case we do flush() twice without sending anything, then in the
> 1st flush we'll see queued>sent, we'll finish flush() until queued==sent.
> Then in the 2nd (continuous) flush() we'll see queued==sent immediately.
>
> IIUC with the current patch we'll return 1 which I think is wrong because
> fallback didn't happen, and if with the change to "if (queued==sent) return
> 0" it'll fix it?

Yes, you are correct.
It's a possible scenario to have a flush happen just after another
without any sending in between.

I will fix it as suggested.

Best regards,
Leo

>
> --
> Peter Xu
>




[PATCH 3/3] tests/tcg/s390x: Test unaligned accesses to lowcore

2022-07-11 Thread Ilya Leoshkevich
Add a small test to avoid regressions.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.softmmu-target |  9 +
 tests/tcg/s390x/unaligned-lowcore.S | 24 
 2 files changed, 33 insertions(+)
 create mode 100644 tests/tcg/s390x/Makefile.softmmu-target
 create mode 100644 tests/tcg/s390x/unaligned-lowcore.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target 
b/tests/tcg/s390x/Makefile.softmmu-target
new file mode 100644
index 00..fdee4a7616
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -0,0 +1,9 @@
+S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
+VPATH+=$(S390X_SRC)
+QEMU_OPTS=-device mmio-debug-exit,base=0x2000 -kernel
+
+%: %.S
+   $(CC) -march=z13 -m64 -nostartfiles -static -Wl,-Ttext=0 \
+   -Wl,--build-id=none $< -o $@
+
+TESTS += unaligned-lowcore
diff --git a/tests/tcg/s390x/unaligned-lowcore.S 
b/tests/tcg/s390x/unaligned-lowcore.S
new file mode 100644
index 00..2b34c3d5ef
--- /dev/null
+++ b/tests/tcg/s390x/unaligned-lowcore.S
@@ -0,0 +1,24 @@
+.org 0x1D0 /* Program new PSW */
+.quad 0x18000, _pgm/* BA, EA */
+.org 0x200 /* lowcore padding */
+
+.globl _start
+_start:
+lctlg %c0,%c0,_c0
+vst %v0,_unaligned
+mviy _exit,0
+
+_pgm:
+mviy _exit,1
+
+.align 8
+_c0:
+.quad 0x1006   /* lowcore protection, AFP, VX */
+
+.byte 0
+_unaligned:
+.octa 0
+
+.org 0x2000/* mmio-debug-exit base */
+_exit:
+.byte 0
-- 
2.35.3




[PATCH 1/3] accel/tcg: Fix unaligned stores to s390x low-address-protected lowcore

2022-07-11 Thread Ilya Leoshkevich
If low-address-protection is active, unaligned stores to non-protected
parts of lowcore lead to protection exceptions. The reason is that in
such cases tlb_fill() call in store_helper_unaligned() covers
[0, addr + size) range, which contains the protected portion of
lowcore. This range is too large.

The most straightforward fix would be to make sure we stay within the
original [addr, addr + size) range. However, if an unaligned access
affects a single page, we don't need to call tlb_fill() in
store_helper_unaligned() at all, since it would be identical to
the previous tlb_fill() call in store_helper(), and therefore a no-op.
If an unaligned access covers multiple pages, this situation does not
occur.

Therefore simply skip TLB handling in store_helper_unaligned() if we
are dealing with a single page.

Fixes: 2bcf018340cb ("s390x/tcg: low-address protection support")
Signed-off-by: Ilya Leoshkevich 
---
 accel/tcg/cputlb.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index f90f4312ea..a46f3a654d 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2248,7 +2248,7 @@ store_helper_unaligned(CPUArchState *env, target_ulong 
addr, uint64_t val,
 const size_t tlb_off = offsetof(CPUTLBEntry, addr_write);
 uintptr_t index, index2;
 CPUTLBEntry *entry, *entry2;
-target_ulong page2, tlb_addr, tlb_addr2;
+target_ulong page1, page2, tlb_addr, tlb_addr2;
 MemOpIdx oi;
 size_t size2;
 int i;
@@ -2256,15 +2256,17 @@ store_helper_unaligned(CPUArchState *env, target_ulong 
addr, uint64_t val,
 /*
  * Ensure the second page is in the TLB.  Note that the first page
  * is already guaranteed to be filled, and that the second page
- * cannot evict the first.
+ * cannot evict the first.  An exception to this rule is PAGE_WRITE_INV
+ * handling: the first page could have evicted itself.
  */
+page1 = addr & TARGET_PAGE_MASK;
 page2 = (addr + size) & TARGET_PAGE_MASK;
 size2 = (addr + size) & ~TARGET_PAGE_MASK;
 index2 = tlb_index(env, mmu_idx, page2);
 entry2 = tlb_entry(env, mmu_idx, page2);
 
 tlb_addr2 = tlb_addr_write(entry2);
-if (!tlb_hit_page(tlb_addr2, page2)) {
+if (page1 != page2 && !tlb_hit_page(tlb_addr2, page2)) {
 if (!victim_tlb_hit(env, mmu_idx, index2, tlb_off, page2)) {
 tlb_fill(env_cpu(env), page2, size2, MMU_DATA_STORE,
  mmu_idx, retaddr);
-- 
2.35.3




[PATCH 0/3] accel/tcg: Fix unaligned stores to s390x low-address-protected lowcore

2022-07-11 Thread Ilya Leoshkevich
Hi,

I noticed that certain accesses to lowcore incorrectly trigger
protection exceptions. I tracked it down to store_helper_unaligned()
calling tlb_fill() with ranges like [0, 2000).

Patch 1 fixes the issue, patch 2 adds a new MMIO device that enables
writing system tests for s390x, patch 3 adds a system test for this
issue.

Best regards,
Ilya

Ilya Leoshkevich (3):
  accel/tcg: Fix unaligned stores to s390x low-address-protected lowcore
  hw/misc: Add mmio-debug-exit device
  tests/tcg/s390x: Test unaligned accesses to lowcore

 accel/tcg/cputlb.c  |  8 ++-
 hw/misc/Kconfig |  3 +
 hw/misc/debugexit_mmio.c| 80 +
 hw/misc/meson.build |  1 +
 hw/s390x/Kconfig|  1 +
 tests/tcg/s390x/Makefile.softmmu-target |  9 +++
 tests/tcg/s390x/unaligned-lowcore.S | 24 
 7 files changed, 123 insertions(+), 3 deletions(-)
 create mode 100644 hw/misc/debugexit_mmio.c
 create mode 100644 tests/tcg/s390x/Makefile.softmmu-target
 create mode 100644 tests/tcg/s390x/unaligned-lowcore.S

-- 
2.35.3




[PATCH 2/3] hw/misc: Add mmio-debug-exit device

2022-07-11 Thread Ilya Leoshkevich
System tests on x86 use isa-debug-exit device in order to signal
success or failure to the test runner. Unfortunately it's not easily
usable on other architectures, since a guest needs to access
address_space_io, which may not be as straightforward as on x86.
Also, it requires adding ISA bus, which an architecture might not
otherwise need.

Introduce mmio-debug-exit device, which has the same semantics, but is
triggered by writes to memory.

Signed-off-by: Ilya Leoshkevich 
---
 hw/misc/Kconfig  |  3 ++
 hw/misc/debugexit_mmio.c | 80 
 hw/misc/meson.build  |  1 +
 hw/s390x/Kconfig |  1 +
 4 files changed, 85 insertions(+)
 create mode 100644 hw/misc/debugexit_mmio.c

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cbabe9f78c..0f12735ef7 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -15,6 +15,9 @@ config ISA_DEBUG
 bool
 depends on ISA_BUS
 
+config MMIO_DEBUGEXIT
+bool
+
 config SGA
 bool
 depends on ISA_BUS
diff --git a/hw/misc/debugexit_mmio.c b/hw/misc/debugexit_mmio.c
new file mode 100644
index 00..5e823cc01c
--- /dev/null
+++ b/hw/misc/debugexit_mmio.c
@@ -0,0 +1,80 @@
+/*
+ * Exit with status X when the guest writes X (little-endian) to a specified
+ * address. For testing purposes only.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+
+#define TYPE_MMIO_DEBUG_EXIT_DEVICE "mmio-debug-exit"
+OBJECT_DECLARE_SIMPLE_TYPE(MMIODebugExitState, MMIO_DEBUG_EXIT_DEVICE)
+
+struct MMIODebugExitState {
+DeviceState parent_obj;
+
+uint32_t base;
+uint32_t size;
+MemoryRegion region;
+};
+
+static uint64_t mmio_debug_exit_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
+static void mmio_debug_exit_write(void *opaque, hwaddr addr, uint64_t val,
+  unsigned width)
+{
+exit(val);
+}
+
+static const MemoryRegionOps mmio_debug_exit_ops = {
+.read = mmio_debug_exit_read,
+.write = mmio_debug_exit_write,
+.valid.min_access_size = 1,
+.valid.max_access_size = 8,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void mmio_debug_exit_realizefn(DeviceState *d, Error **errp)
+{
+MMIODebugExitState *s = MMIO_DEBUG_EXIT_DEVICE(d);
+
+memory_region_init_io(>region, OBJECT(s), _debug_exit_ops, s,
+  TYPE_MMIO_DEBUG_EXIT_DEVICE, s->size);
+memory_region_add_subregion(get_system_memory(), s->base, >region);
+}
+
+static Property mmio_debug_exit_properties[] = {
+DEFINE_PROP_UINT32("base", MMIODebugExitState, base, 0),
+DEFINE_PROP_UINT32("size", MMIODebugExitState, size, 1),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mmio_debug_exit_class_initfn(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->realize = mmio_debug_exit_realizefn;
+device_class_set_props(dc, mmio_debug_exit_properties);
+set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo mmio_debug_exit_info = {
+.name  = TYPE_MMIO_DEBUG_EXIT_DEVICE,
+.parent= TYPE_DEVICE,
+.instance_size = sizeof(MMIODebugExitState),
+.class_init= mmio_debug_exit_class_initfn,
+};
+
+static void mmio_debug_exit_register_types(void)
+{
+type_register_static(_debug_exit_info);
+}
+
+type_init(mmio_debug_exit_register_types)
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 95268eddc0..1d2a1067dc 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -2,6 +2,7 @@ softmmu_ss.add(when: 'CONFIG_APPLESMC', if_true: 
files('applesmc.c'))
 softmmu_ss.add(when: 'CONFIG_EDU', if_true: files('edu.c'))
 softmmu_ss.add(when: 'CONFIG_FW_CFG_DMA', if_true: files('vmcoreinfo.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
+softmmu_ss.add(when: 'CONFIG_MMIO_DEBUGEXIT', if_true: 
files('debugexit_mmio.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
 softmmu_ss.add(when: 'CONFIG_PCA9552', if_true: files('pca9552.c'))
 softmmu_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 5e7d8a2bae..9223715dcc 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -5,6 +5,7 @@ config S390_CCW_VIRTIO
 imply VFIO_AP
 imply VFIO_CCW
 imply WDT_DIAG288
+imply MMIO_DEBUGEXIT
 select PCI
 select S390_FLIC
 select SCLPCONSOLE
-- 
2.35.3




Re: [RFC PATCH v9 14/23] vhost: add vhost_svq_poll

2022-07-11 Thread Eugenio Perez Martin
On Mon, Jul 11, 2022 at 11:19 AM Jason Wang  wrote:
>
>
> 在 2022/7/7 02:39, Eugenio Pérez 写道:
> > It allows the Shadow Control VirtQueue to wait the device to use the 
> > commands
> > that restore the net device state after a live migration.
> >
> > Signed-off-by: Eugenio Pérez 
> > ---
> >   hw/virtio/vhost-shadow-virtqueue.h |  1 +
> >   hw/virtio/vhost-shadow-virtqueue.c | 54 --
> >   2 files changed, 52 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
> > b/hw/virtio/vhost-shadow-virtqueue.h
> > index d01d2370db..c8668fbdd6 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -100,6 +100,7 @@ void vhost_svq_push_elem(VhostShadowVirtqueue *svq,
> >const VirtQueueElement *elem, uint32_t len);
> >   int vhost_svq_inject(VhostShadowVirtqueue *svq, const struct iovec *iov,
> >size_t out_num, size_t in_num, void *opaque);
> > +ssize_t vhost_svq_poll(VhostShadowVirtqueue *svq);
> >   void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int 
> > svq_kick_fd);
> >   void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd);
> >   void vhost_svq_get_vring_addr(const VhostShadowVirtqueue *svq,
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> > b/hw/virtio/vhost-shadow-virtqueue.c
> > index bd9e34b413..ed7f1d0bc9 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -10,6 +10,8 @@
> >   #include "qemu/osdep.h"
> >   #include "hw/virtio/vhost-shadow-virtqueue.h"
> >
> > +#include 
> > +
> >   #include "qemu/error-report.h"
> >   #include "qapi/error.h"
> >   #include "qemu/main-loop.h"
> > @@ -490,10 +492,11 @@ void vhost_svq_push_elem(VhostShadowVirtqueue *svq,
> >   }
> >   }
> >
> > -static void vhost_svq_flush(VhostShadowVirtqueue *svq,
> > -bool check_for_avail_queue)
> > +static size_t vhost_svq_flush(VhostShadowVirtqueue *svq,
> > +  bool check_for_avail_queue)
> >   {
> >   VirtQueue *vq = svq->vq;
> > +size_t ret = 0;
> >
> >   /* Forward as many used buffers as possible. */
> >   do {
> > @@ -510,7 +513,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
> >"More than %u used buffers obtained in a %u size 
> > SVQ",
> >i, svq->vring.num);
> >   virtqueue_flush(vq, svq->vring.num);
> > -return;
> > +return ret;
> >   }
> >
> >   svq_elem = vhost_svq_get_buf(svq, );
> > @@ -520,6 +523,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
> >
> >   elem = g_steal_pointer(_elem.opaque);
> >   virtqueue_fill(vq, elem, len, i++);
> > +ret++;
> >   }
> >
> >   virtqueue_flush(vq, i);
> > @@ -533,6 +537,50 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
> >   vhost_handle_guest_kick(svq);
> >   }
> >   } while (!vhost_svq_enable_notification(svq));
> > +
> > +return ret;
> > +}
> > +
> > +/**
> > + * Poll the SVQ for device used buffers.
> > + *
> > + * This function race with main event loop SVQ polling, so extra
> > + * synchronization is needed.
> > + *
> > + * Return the number of descriptors read from the device.
> > + */
> > +ssize_t vhost_svq_poll(VhostShadowVirtqueue *svq)
> > +{
> > +int fd = event_notifier_get_fd(>hdev_call);
> > +GPollFD poll_fd = {
> > +.fd = fd,
> > +.events = G_IO_IN,
> > +};
> > +assert(fd >= 0);
> > +int r = g_poll(_fd, 1, -1);
>
>
> Any reason we can't simply (busy) polling the used ring here? It might
> help to reduce the latency (and it is what kernel driver uses).
>

Yes, I'll change to a busy polling. I forgot to change it.

Thanks!

> Thanks
>
>
> > +
> > +if (unlikely(r < 0)) {
> > +error_report("Cannot poll device call fd "G_POLLFD_FORMAT": (%d) 
> > %s",
> > + poll_fd.fd, errno, g_strerror(errno));
> > +return -errno;
> > +}
> > +
> > +if (r == 0) {
> > +return 0;
> > +}
> > +
> > +if (unlikely(poll_fd.revents & ~(G_IO_IN))) {
> > +error_report(
> > +"Error polling device call fd "G_POLLFD_FORMAT": revents=%d",
> > +poll_fd.fd, poll_fd.revents);
> > +return -1;
> > +}
> > +
> > +/*
> > + * Max return value of vhost_svq_flush is (uint16_t)-1, so it's safe to
> > + * convert to ssize_t.
> > + */
> > +return vhost_svq_flush(svq, false);
> >   }
> >
> >   /**
>




Re: Information needed regarding gnu-arm-eclipse version:

2022-07-11 Thread Alex Bennée


"Vaidehi Anant Deshpande (i)"  writes:

> Hello,
>
> I am Vaidehi and I am currently trying to run a code on QEMU emulator using 
> eclipse IDE. However, I came across some documentation
> regarding the deprecation of qemu-system-gnuarmeclipse and I need to know if 
> there is v7.0.0 available for
> qemu-syetem-gnuarmeclipse.

qemu-system-gnuarmeclipse is nothing to do with the upstream qemu
project. It's a fork of QEMU which was focused on M-profile and IDE
integration. You would have to ask xPack developers how to migrate to
upstream QEMU.

Do you know what board/processor you are trying to emulate?

> Please let me know the links for the same if available.
>
> Best regards,
> Vaidehi. D
>
> Email Signature
>
>
>  Banner   
>  This email and any files transmitted with it are confidential and intended 
> solely for the use of the individual or entity to whom they are  
>  addressed. You may NOT use, disclose, copy or disseminate this information. 
> If you have received this email in error, please notify the  
>  sender and destroy all copies of the original message and all attachments. 
> Please note that any views or opinions presented in this  
>  email are solely those of the author and do not necessarily represent those 
> of the company. Finally, the recipient should check this  
>  email and any attachments for the presence of viruses. The company accepts 
> no liability for any damage caused by any virus  
>  transmitted by this email.

You may want to tweak your email client not to dump this banner on posts
to public mailing lists...

-- 
Alex Bennée



[PATCH v2] hw/nvram: Add at24c-eeprom support for small eeproms

2022-07-11 Thread Patrick Venture
Tested: Verified at24c02 driver happy and contents matched
expectations.

Signed-off-by: Patrick Venture 
Reviewed-by: Hao Wu 
---
v2: Added comment describing the new behavior.
---
 hw/nvram/eeprom_at24c.c | 56 -
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
index d695f6ae89..eb91ec6813 100644
--- a/hw/nvram/eeprom_at24c.c
+++ b/hw/nvram/eeprom_at24c.c
@@ -45,6 +45,8 @@ struct EEPROMState {
 bool changed;
 /* during WRITE, # of address bytes transfered */
 uint8_t haveaddr;
+/* whether it's 8-bit addressed or 16-bit */
+bool eightbit;
 
 uint8_t *mem;
 
@@ -87,7 +89,7 @@ uint8_t at24c_eeprom_recv(I2CSlave *s)
 EEPROMState *ee = AT24C_EE(s);
 uint8_t ret;
 
-if (ee->haveaddr == 1) {
+if (ee->haveaddr == 1 && !ee->eightbit) {
 return 0xff;
 }
 
@@ -104,26 +106,35 @@ int at24c_eeprom_send(I2CSlave *s, uint8_t data)
 {
 EEPROMState *ee = AT24C_EE(s);
 
-if (ee->haveaddr < 2) {
-ee->cur <<= 8;
-ee->cur |= data;
+if (ee->haveaddr < 1) {
+ee->cur = data;
 ee->haveaddr++;
-if (ee->haveaddr == 2) {
-ee->cur %= ee->rsize;
+if (ee->eightbit) {
 DPRINTK("Set pointer %04x\n", ee->cur);
 }
+return 0;
+} else if (ee->haveaddr < 2) {
+if (!ee->eightbit) {
+ee->cur <<= 8;
+ee->cur |= data;
+ee->haveaddr++;
+if (ee->haveaddr == 2) {
+ee->cur %= ee->rsize;
+DPRINTK("Set pointer %04x\n", ee->cur);
+}
 
-} else {
-if (ee->writable) {
-DPRINTK("Send %02x\n", data);
-ee->mem[ee->cur] = data;
-ee->changed = true;
-} else {
-DPRINTK("Send error %02x read-only\n", data);
+return 0;
 }
-ee->cur = (ee->cur + 1u) % ee->rsize;
+}
 
+if (ee->writable) {
+DPRINTK("Send %02x\n", data);
+ee->mem[ee->cur] = data;
+ee->changed = true;
+} else {
+DPRINTK("Send error %02x read-only\n", data);
 }
+ee->cur = (ee->cur + 1u) % ee->rsize;
 
 return 0;
 }
@@ -133,12 +144,16 @@ static void at24c_eeprom_realize(DeviceState *dev, Error 
**errp)
 EEPROMState *ee = AT24C_EE(dev);
 
 if (ee->blk) {
+/* blk length is a minimum of 1 sector. */
 int64_t len = blk_getlength(ee->blk);
 
 if (len != ee->rsize) {
-error_setg(errp, "%s: Backing file size %" PRId64 " != %u",
-   TYPE_AT24C_EE, len, ee->rsize);
-return;
+/* for the at24c01 and at24c02, they are smaller than a sector. */
+if (ee->rsize >= BDRV_SECTOR_SIZE) {
+error_setg(errp, "%s: Backing file size %" PRId64 " != %u",
+   TYPE_AT24C_EE, len, ee->rsize);
+return;
+}
 }
 
 if (blk_set_perm(ee->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
@@ -150,6 +165,13 @@ static void at24c_eeprom_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
+/*
+ * The eeprom sizes are 2**x based, so if it's strictly less than 512, we
+ * expect it to be 256 or 128.
+ */
+if (ee->rsize < 512) {
+ee->eightbit = true;
+}
 ee->mem = g_malloc0(ee->rsize);
 }
 
-- 
2.37.0.144.g8ac04bfd2-goog




Re: [PATCH] Replace 'whitelist' with 'allow'

2022-07-11 Thread Alex Bennée


Thomas Huth  writes:

> Let's use more inclusive language here and avoid terms
> that are frowned upon nowadays.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée



Re: [PATCH] hw/riscv: virt: pass random seed to fdt

2022-07-11 Thread Jason A. Donenfeld
Hi Alistair,

On Mon, Jul 11, 2022 at 01:36:28PM +1000, Alistair Francis wrote:
> On Mon, Jul 11, 2022 at 10:28 AM Jason A. Donenfeld  wrote:
> >
> > On 7/11/22, Alistair Francis  wrote:
> > > On Fri, Jul 8, 2022 at 7:56 PM Jason A. Donenfeld  wrote:
> > >>
> > >> Hi Alistair,
> > >>
> > >> On 7/8/22, Alistair Francis  wrote:
> > >>
> > >> >> > but I think that's just the way things go unfortunately.
> > >> >
> > >> > Hmm... That's a pain. So there is a bug in older kernels where they
> > >> > won't boot if we specify this?
> > >> >
> > >> > Can you point to the fixes?
> > >>
> > >> Actually, in trying to reproduce this, I don't actually think this is
> > >> affected by those old random.c bugs.
> > >>
> > >>
> > >> >> As for your 5.8 issue, I've been trying to reproduce that to
> > >> >> understand
> > >> >> more about it, but I'm unable to. I've been trying with
> > >> >> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
> > >> >> possible in testing this out you were testing the wrong branch?
> > >> >> Anyway,
> > >> >> it'd be nice to get this queued up...
> > >> >
> > >> > Hmm... you can't reproduce it?
> > >>
> > >> No, I can't, and I'm now no longer convinced that there *is* a bug.
> > >> Can you try to repro again and send me detailed reproduction steps?
> > >
> > > I just checked again and I can confirm it is this patch that causes
> > > the regression.
> > >
> > > This is the command line:
> > >
> > > qemu-system-riscv64 \
> > > -machine virt -m 64M \
> > > -cpu rv64,mmu=false \
> > > -serial mon:stdio -serial null -nographic \
> > > -append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
> > > earlycon=sbi" \
> > > -device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 -netdev
> > > user,id=net0 \
> > > -object rng-random,filename=/dev/urandom,id=rng0 -device
> > > virtio-rng-device,rng=rng0 \
> > > -smp 1 -d guest_errors \
> > > -kernel ./images/qemuriscv64/nommu-Image \
> > > -drive
> > > id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
> > > -device virtio-blk-device,drive=disk0 -bios none
> > >
> > > You can access the images from:
> > > https://nextcloud.alistair23.me/index.php/s/a2zrtbT7DjdTx9t
> > >
> >
> > Thanks. Can you send the kernel . config too?
> 
> Unfortunately I don't have it, it should just be the 5.8 no MMU defconfig 
> though

I've reproduced the problem and determined the root cause. This is a
generic issue with the mmio get_cycles() implementation before 5.9 on
no-MMU configs, which was fixed during the 5.9 cycle. I don't believe
that this is the only thing affected on that .0 kernel, where fixes were
ostensibly backported. Given the relative age of risc-v, the fact that
5.8.0 was broken anyway, and that likely nobody is using this kernel in
that configuration without applying updates, I'm pretty sure my patch is
safe to apply. I'd recommend updating the broken kernel in your CI.

Meanwhile, the rng-seed field is part of the DT spec. Holding back the
(virtual) hardware just because some random dot-zero non-LTS release had
a quickly fixed bug seems ridiculous, and the way in which progress gets
held up, hacks accumulate, and generally nothing good gets done. It will
only hamper security, functionality, and boot speed, while helping no
real practical case that can't be fixed in a better way.

So I believe you should apply the rng-seed commit so that the RISC-V
machine honors that DT field.

Regards,
Jason



Re: [PATCH 0/5] ppc: Remove irq_inputs

2022-07-11 Thread Daniel Henrique Barboza

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 7/5/22 11:58, Cédric Le Goater wrote:

Hello,

This replaces the IRQ array 'irq_inputs' with GPIO lines and removes
'irq_inputs' when all CPUs have been converted.

Thanks,

C.

Cédric Le Goater (5):
   ppc64: Allocate IRQ lines with qdev_init_gpio_in()
   ppc/40x: Allocate IRQ lines with qdev_init_gpio_in()
   ppc/6xx: Allocate IRQ lines with qdev_init_gpio_in()
   ppc/e500: Allocate IRQ lines with qdev_init_gpio_in()
   ppc: Remove unused irq_inputs

  target/ppc/cpu.h   |  1 -
  hw/intc/xics.c | 10 ++
  hw/intc/xive.c |  4 ++--
  hw/ppc/e500.c  |  8 
  hw/ppc/mac_newworld.c  | 16 
  hw/ppc/mac_oldworld.c  |  2 +-
  hw/ppc/pegasos2.c  |  2 +-
  hw/ppc/ppc.c   | 30 ++
  hw/ppc/ppc405_uc.c |  4 ++--
  hw/ppc/ppc440_bamboo.c |  4 ++--
  hw/ppc/prep.c  |  2 +-
  hw/ppc/prep_systemio.c |  2 +-
  hw/ppc/sam460ex.c  |  4 ++--
  hw/ppc/virtex_ml507.c  | 10 +-
  target/ppc/cpu_init.c  |  5 -
  15 files changed, 41 insertions(+), 63 deletions(-)





Re: [PATCH v2] target/riscv: fix shifts shamt value for rv128c

2022-07-11 Thread Richard Henderson
On Sun, 10 July 2022, 16:36 Frédéric Pétrot, <
frederic.pet...@univ-grenoble-alpes.fr> wrote:

> For rv128c shifts, a shamt of 0 is a shamt of 64, while for rv32c/rv64c
> it stays 0 and is a hint instruction that does not change processor state.
> For rv128c right shifts, the 6-bit shamt is in addition sign extended to
> 7 bits.
>

Um, what says that? First, it makes no sense — while some systems define
negative shifts as opposite direction, riscv is not one of them.

Second, the 20220707 draft spec does not agree with you:

  Shifts by an immediate (SLLI/SRLI/SRAI)
  are now encoded using the low 7 bits of
  the I-immediate

Nothing about that sentence says "signed".


r~


> Signed-off-by: Frédéric Pétrot 
> ---
>  target/riscv/insn16.decode |  7 ---
>  disas/riscv.c  | 27 +--
>  target/riscv/translate.c   | 20 ++--
>  3 files changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
> index 02c8f61b48..ccfe59f294 100644
> --- a/target/riscv/insn16.decode
> +++ b/target/riscv/insn16.decode
> @@ -31,7 +31,8 @@
>  %imm_cb12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
>  %imm_cj12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
>
> -%shimm_6bit   12:1 2:5   !function=ex_rvc_shifti
> +%shlimm_6bit  12:1 2:5   !function=ex_rvc_shiftli
> +%shrimm_6bit  12:1 2:5   !function=ex_rvc_shiftri
>  %uimm_6bit_lq 2:4 12:1 6:1   !function=ex_shift_4
>  %uimm_6bit_ld 2:3 12:1 5:2   !function=ex_shift_3
>  %uimm_6bit_lw 2:2 12:1 4:3   !function=ex_shift_2
> @@ -82,9 +83,9 @@
>  @c_addi16sp ... .  . . ..  imm=%imm_addi16sp rs1=2 rd=2
>
>  @c_shift... . .. ... . .. \
> - rd=%rs1_3 rs1=%rs1_3 shamt=%shimm_6bit
> + rd=%rs1_3 rs1=%rs1_3 shamt=%shrimm_6bit
>  @c_shift2   ... . .. ... . .. \
> - rd=%rd rs1=%rd shamt=%shimm_6bit
> + rd=%rd rs1=%rd shamt=%shlimm_6bit
>
>  @c_andi ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 7af6afc8fa..489c2ae5e8 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -2402,10 +2402,25 @@ static int32_t operand_sbimm12(rv_inst inst)
>  ((inst << 56) >> 63) << 11;
>  }
>
> -static uint32_t operand_cimmsh6(rv_inst inst)
> +static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>  {
> -return ((inst << 51) >> 63) << 5 |
> +int imm = ((inst << 51) >> 63) << 5 |
>  (inst << 57) >> 59;
> +if (isa == rv128) {
> +imm = imm ? imm : 64;
> +}
> +return imm;
> +}
> +
> +static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
> +{
> +int imm = ((inst << 51) >> 63) << 5 |
> +(inst << 57) >> 59;
> +if (isa == rv128) {
> +imm = imm | (imm & 32) << 1;
> +imm = imm ? imm : 64;
> +}
> +return imm;
>  }
>
>  static int32_t operand_cimmi(rv_inst inst)
> @@ -2529,7 +2544,7 @@ static uint32_t operand_rnum(rv_inst inst)
>
>  /* decode operands */
>
> -static void decode_inst_operands(rv_decode *dec)
> +static void decode_inst_operands(rv_decode *dec, rv_isa isa)
>  {
>  rv_inst inst = dec->inst;
>  dec->codec = opcode_data[dec->op].codec;
> @@ -2652,7 +2667,7 @@ static void decode_inst_operands(rv_decode *dec)
>  case rv_codec_cb_sh6:
>  dec->rd = dec->rs1 = operand_crs1rdq(inst) + 8;
>  dec->rs2 = rv_ireg_zero;
> -dec->imm = operand_cimmsh6(inst);
> +dec->imm = operand_cimmshr6(inst, isa);
>  break;
>  case rv_codec_ci:
>  dec->rd = dec->rs1 = operand_crs1rd(inst);
> @@ -2667,7 +2682,7 @@ static void decode_inst_operands(rv_decode *dec)
>  case rv_codec_ci_sh6:
>  dec->rd = dec->rs1 = operand_crs1rd(inst);
>  dec->rs2 = rv_ireg_zero;
> -dec->imm = operand_cimmsh6(inst);
> +dec->imm = operand_cimmshl6(inst, isa);
>  break;
>  case rv_codec_ci_16sp:
>  dec->rd = rv_ireg_sp;
> @@ -3193,7 +3208,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa,
> uint64_t pc, rv_inst inst)
>  dec.pc = pc;
>  dec.inst = inst;
>  decode_inst_opcode(, isa);
> -decode_inst_operands();
> +decode_inst_operands(, isa);
>  decode_inst_decompress(, isa);
>  decode_inst_lift_pseudo();
>  format_inst(buf, buflen, 16, );
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 63b04e8a94..d7c82a9c81 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -705,10 +705,26 @@ static int ex_rvc_register(DisasContext *ctx, int
> reg)
>  return 8 + reg;
>  }
>
> -static int ex_rvc_shifti(DisasContext *ctx, int imm)
> +static int ex_rvc_shiftli(DisasContext *ctx, int imm)
>  {
>  /* For RV128 a shamt of 0 means a shift by 64. */
> -return imm ? imm : 64;
> +if (get_ol(ctx) == MXL_RV128) {
> +

[PATCH v11 09/10] virtio-mmio: add support for configure interrupt

2022-07-11 Thread Cindy Lu
Add configure interrupt support in virtio-mmio bus.
add function to set configure guest notifier.

Signed-off-by: Cindy Lu 
---
 hw/virtio/virtio-mmio.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 688eccda94..5c613a96d9 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -672,7 +672,30 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, 
int n, bool assign,
 
 return 0;
 }
+static int virtio_mmio_set_config_guest_notifier(DeviceState *d, bool assign,
+ bool with_irqfd)
+{
+VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+VirtIODevice *vdev = virtio_bus_get_device(>bus);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+EventNotifier *notifier = virtio_config_get_guest_notifier(vdev);
+int r = 0;
 
+if (assign) {
+r = event_notifier_init(notifier, 0);
+if (r < 0) {
+return r;
+}
+virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
+} else {
+virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
+event_notifier_cleanup(notifier);
+}
+if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
+vdc->guest_notifier_mask(vdev, VIRTIO_CONFIG_IRQ_IDX, !assign);
+}
+return r;
+}
 static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
bool assign)
 {
@@ -694,6 +717,10 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, 
int nvqs,
 goto assign_error;
 }
 }
+r = virtio_mmio_set_config_guest_notifier(d, assign, with_irqfd);
+if (r < 0) {
+goto assign_error;
+}
 
 return 0;
 
-- 
2.34.3




[PATCH v11 07/10] vhost: add support for configure interrupt

2022-07-11 Thread Cindy Lu
Add functions to support configure interrupt.
The configure interrupt process will start in vhost_dev_start
and stop in vhost_dev_stop.

Also add the functions to support vhost_config_pending and
vhost_config_mask.

Signed-off-by: Cindy Lu 
---
 hw/virtio/vhost.c | 78 ++-
 include/hw/virtio/vhost.h |  4 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index b643f42ea4..e23be58d69 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1550,7 +1550,68 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, 
VirtIODevice *vdev, int n,
 file.index = hdev->vhost_ops->vhost_get_vq_index(hdev, n);
 r = hdev->vhost_ops->vhost_set_vring_call(hdev, );
 if (r < 0) {
-VHOST_OPS_DEBUG(r, "vhost_set_vring_call failed");
+error_report("vhost_set_vring_call failed %d", -r);
+}
+}
+
+bool vhost_config_pending(struct vhost_dev *hdev)
+{
+assert(hdev->vhost_ops);
+if ((hdev->started == false) ||
+(hdev->vhost_ops->vhost_set_config_call == NULL)) {
+return false;
+}
+
+EventNotifier *notifier =
+>vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier;
+return event_notifier_test_and_clear(notifier);
+}
+
+void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev, bool mask)
+{
+int fd;
+int r;
+EventNotifier *notifier =
+>vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier;
+EventNotifier *config_notifier = >config_notifier;
+assert(hdev->vhost_ops);
+
+if ((hdev->started == false) ||
+(hdev->vhost_ops->vhost_set_config_call == NULL)) {
+return;
+}
+if (mask) {
+assert(vdev->use_guest_notifier_mask);
+fd = event_notifier_get_fd(notifier);
+} else {
+fd = event_notifier_get_fd(config_notifier);
+}
+r = hdev->vhost_ops->vhost_set_config_call(hdev, fd);
+if (r < 0) {
+error_report("vhost_set_config_call failed %d", -r);
+}
+}
+
+static void vhost_stop_config_intr(struct vhost_dev *dev)
+{
+int fd = -1;
+assert(dev->vhost_ops);
+if (dev->vhost_ops->vhost_set_config_call) {
+dev->vhost_ops->vhost_set_config_call(dev, fd);
+}
+}
+
+static void vhost_start_config_intr(struct vhost_dev *dev)
+{
+int r;
+
+assert(dev->vhost_ops);
+int fd = event_notifier_get_fd(>vdev->config_notifier);
+if (dev->vhost_ops->vhost_set_config_call) {
+r = dev->vhost_ops->vhost_set_config_call(dev, fd);
+if (!r) {
+event_notifier_set(>vdev->config_notifier);
+}
 }
 }
 
@@ -1766,6 +1827,16 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 }
 }
 
+r = event_notifier_init(
+>vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier, 0);
+if (r < 0) {
+return r;
+}
+event_notifier_test_and_clear(
+>vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier);
+if (!vdev->use_guest_notifier_mask) {
+vhost_config_mask(hdev, vdev, true);
+}
 if (hdev->log_enabled) {
 uint64_t log_base;
 
@@ -1798,6 +1869,7 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 vhost_device_iotlb_miss(hdev, vq->used_phys, true);
 }
 }
+vhost_start_config_intr(hdev);
 return 0;
 fail_log:
 vhost_log_put(hdev, false);
@@ -1823,6 +1895,9 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 
 /* should only be called after backend is connected */
 assert(hdev->vhost_ops);
+event_notifier_test_and_clear(
+>vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier);
+event_notifier_test_and_clear(>config_notifier);
 
 if (hdev->vhost_ops->vhost_dev_start) {
 hdev->vhost_ops->vhost_dev_start(hdev, false);
@@ -1840,6 +1915,7 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 }
 memory_listener_unregister(>iommu_listener);
 }
+vhost_stop_config_intr(hdev);
 vhost_log_put(hdev, true);
 hdev->started = false;
 hdev->vdev = NULL;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 58a73e7b7a..b0f3b78987 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -29,6 +29,7 @@ struct vhost_virtqueue {
 unsigned long long used_phys;
 unsigned used_size;
 EventNotifier masked_notifier;
+EventNotifier masked_config_notifier;
 struct vhost_dev *dev;
 };
 
@@ -37,6 +38,7 @@ typedef unsigned long vhost_log_chunk_t;
 #define VHOST_LOG_BITS (8 * sizeof(vhost_log_chunk_t))
 #define VHOST_LOG_CHUNK (VHOST_LOG_PAGE * VHOST_LOG_BITS)
 #define VHOST_INVALID_FEATURE_BIT   (0xff)
+#define VHOST_QUEUE_NUM_CONFIG_INR 0
 
 struct vhost_log {
 unsigned long long size;
@@ -116,6 +118,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev);
 void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
 int 

[PATCH v11 03/10] virtio-pci: decouple the single vector from the interrupt process

2022-07-11 Thread Cindy Lu
To reuse the interrupt process in configure interrupt
Need to decouple the single vector from the interrupt process.
We add new function kvm_virtio_pci_vector_use_one and _release_one.
These functions are used for the single vector, the whole process will
finish in the loop with vq number.

Signed-off-by: Cindy Lu 
---
 hw/virtio/virtio-pci.c | 131 +++--
 1 file changed, 73 insertions(+), 58 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2869d0d2f6..4b86008bcf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -676,7 +676,6 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev,
 }
 
 static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
-unsigned int queue_no,
 unsigned int vector)
 {
 VirtIOIRQFD *irqfd = >vector_irqfd[vector];
@@ -741,87 +740,103 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy 
*proxy, int queue_no,
 return 0;
 }
 
-static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
+static int kvm_virtio_pci_vector_use_one(VirtIOPCIProxy *proxy, int queue_no)
 {
+unsigned int vector;
+int ret;
+EventNotifier *n;
 PCIDevice *dev = >pci_dev;
 VirtIODevice *vdev = virtio_bus_get_device(>bus);
 VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-unsigned int vector;
-int ret, queue_no;
-EventNotifier *n;
-for (queue_no = 0; queue_no < nvqs; queue_no++) {
-if (!virtio_queue_get_num(vdev, queue_no)) {
-break;
-}
-ret = virtio_pci_get_notifier(proxy, queue_no, , );
-if (ret < 0) {
-break;
-}
-if (vector >= msix_nr_vectors_allocated(dev)) {
-continue;
-}
-ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
+
+ret = virtio_pci_get_notifier(proxy, queue_no, , );
+if (ret < 0) {
+return ret;
+}
+if (vector >= msix_nr_vectors_allocated(dev)) {
+return 0;
+}
+ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
+if (ret < 0) {
+goto undo;
+}
+/*
+ * If guest supports masking, set up irqfd now.
+ * Otherwise, delay until unmasked in the frontend.
+ */
+if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
+ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
 if (ret < 0) {
+kvm_virtio_pci_vq_vector_release(proxy, vector);
 goto undo;
 }
-/* If guest supports masking, set up irqfd now.
- * Otherwise, delay until unmasked in the frontend.
- */
-if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
-ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
-if (ret < 0) {
-kvm_virtio_pci_vq_vector_release(proxy, vector);
-goto undo;
-}
-}
 }
-return 0;
 
+return 0;
 undo:
-while (--queue_no >= 0) {
-vector = virtio_queue_vector(vdev, queue_no);
-if (vector >= msix_nr_vectors_allocated(dev)) {
-continue;
+
+vector = virtio_queue_vector(vdev, queue_no);
+if (vector >= msix_nr_vectors_allocated(dev)) {
+return ret;
+}
+if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
+ret = virtio_pci_get_notifier(proxy, queue_no, , );
+if (ret < 0) {
+return ret;
 }
-if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
-ret = virtio_pci_get_notifier(proxy, queue_no, , );
-if (ret < 0) {
-break;
-}
-kvm_virtio_pci_irqfd_release(proxy, n, vector);
+kvm_virtio_pci_irqfd_release(proxy, n, vector);
+}
+return ret;
+}
+static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
+{
+int queue_no;
+int ret = 0;
+VirtIODevice *vdev = virtio_bus_get_device(>bus);
+
+for (queue_no = 0; queue_no < nvqs; queue_no++) {
+if (!virtio_queue_get_num(vdev, queue_no)) {
+return -1;
 }
-kvm_virtio_pci_vq_vector_release(proxy, vector);
+ret = kvm_virtio_pci_vector_use_one(proxy, queue_no);
 }
 return ret;
 }
 
-static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
+
+static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
+  int queue_no)
 {
-PCIDevice *dev = >pci_dev;
 VirtIODevice *vdev = virtio_bus_get_device(>bus);
 unsigned int vector;
-int queue_no;
-VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 EventNotifier *n;
-int ret ;
+int ret;
+VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+PCIDevice *dev = >pci_dev;
+
+ret = virtio_pci_get_notifier(proxy, queue_no, , );
+if (ret < 0) {
+return;
+}
+if (vector >= 

Re: [PATCH v9 00/21] job: replace AioContext lock with job_mutex

2022-07-11 Thread Vladimir Sementsov-Ogievskiy

That seems a lot closer!

Now I'm going to vocation from tomorrow up to the end of week, so I'll return 
next Monday.


On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:

In this series, we want to remove the AioContext lock and instead
use the already existent job_mutex to protect the job structures
and list. This is part of the work to get rid of AioContext lock
usage in favour of smaller granularity locks.
[..]



--
Best regards,
Vladimir



[PATCH v11 05/10] vhost-vdpa: add support for config interrupt

2022-07-11 Thread Cindy Lu
Add new call back function in vhost-vdpa, The function
vhost_set_config_call can set the event fd to kernel.
This function will be called in the vhost_dev_start
and vhost_dev_stop

Signed-off-by: Cindy Lu 
---
 hw/virtio/trace-events | 1 +
 hw/virtio/vhost-vdpa.c | 8 
 2 files changed, 9 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index a5102eac9e..b968ba9e4e 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -53,6 +53,7 @@ vhost_vdpa_get_features(void *dev, uint64_t features) "dev: 
%p features: 0x%"PRI
 vhost_vdpa_set_owner(void *dev) "dev: %p"
 vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t 
avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 
0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
 vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p 
first: 0x%"PRIx64" last: 0x%"PRIx64
+vhost_vdpa_set_config_call(void *dev, int fd)"dev: %p fd: %d"
 
 # virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
out_num) "elem %p size %zd in_num %u out_num %u"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 8adf7c0b92..02bafb61b9 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -733,6 +733,13 @@ static int vhost_vdpa_set_vring_ready(struct vhost_dev 
*dev)
 return 0;
 }
 
+static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
+   int fd)
+{
+trace_vhost_vdpa_set_config_call(dev, fd);
+return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, );
+}
+
 static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t 
*config,
uint32_t config_len)
 {
@@ -1297,4 +1304,5 @@ const VhostOps vdpa_ops = {
 .vhost_get_device_id = vhost_vdpa_get_device_id,
 .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
 .vhost_force_iommu = vhost_vdpa_force_iommu,
+.vhost_set_config_call = vhost_vdpa_set_config_call,
 };
-- 
2.34.3




[PATCH v11 06/10] virtio: add support for configure interrupt

2022-07-11 Thread Cindy Lu
Add the functions to support the configure interrupt in virtio
The function virtio_config_guest_notifier_read will notify the
guest if there is an configure interrupt.
The function virtio_config_set_guest_notifier_fd_handler is
to set the fd hander for the notifier

Signed-off-by: Cindy Lu 
---
 hw/virtio/virtio.c | 29 +
 include/hw/virtio/virtio.h |  4 
 2 files changed, 33 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9d637e043e..ff1f72b9ff 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3471,7 +3471,14 @@ static void 
virtio_queue_guest_notifier_read(EventNotifier *n)
 virtio_irq(vq);
 }
 }
+static void virtio_config_guest_notifier_read(EventNotifier *n)
+{
+VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
 
+if (event_notifier_test_and_clear(n)) {
+virtio_notify_config(vdev);
+}
+}
 void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
 bool with_irqfd)
 {
@@ -3488,6 +3495,23 @@ void 
virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
 }
 }
 
+void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
+ bool assign, bool with_irqfd)
+{
+EventNotifier *n;
+n = >config_notifier;
+if (assign && !with_irqfd) {
+event_notifier_set_handler(n, virtio_config_guest_notifier_read);
+} else {
+event_notifier_set_handler(n, NULL);
+}
+if (!assign) {
+/* Test and clear notifier before closing it,*/
+/* in case poll callback didn't have time to run. */
+virtio_config_guest_notifier_read(n);
+}
+}
+
 EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
 {
 return >guest_notifier;
@@ -3555,6 +3579,11 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue 
*vq)
 return >host_notifier;
 }
 
+EventNotifier *virtio_config_get_guest_notifier(VirtIODevice *vdev)
+{
+return >config_notifier;
+}
+
 void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
 {
 vq->host_notifier_enabled = enabled;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 4512205503..d3087ed5e8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -111,6 +111,7 @@ struct VirtIODevice
 bool use_guest_notifier_mask;
 AddressSpace *dma_as;
 QLIST_HEAD(, VirtQueue) *vector_queues;
+EventNotifier config_notifier;
 };
 
 struct VirtioDeviceClass {
@@ -323,6 +324,9 @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, 
AioContext *ctx);
 void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx);
 VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
 VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
+EventNotifier *virtio_config_get_guest_notifier(VirtIODevice *vdev);
+void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
+ bool assign, bool with_irqfd);
 
 static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
 {
-- 
2.34.3




  1   2   3   >