Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

Hi Cédric,

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
- HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)


Do you mind splitting as:

- Add TYPE_EMMC, emmc_class_init and sd_proto_emmc[] with
  already existing handlers (1 patch)

- Add new handlers, from smaller to sd_emmc_set_csd(),
  and finally mmc_set_ext_csd() with the EXT_CSD definitions
  (various patches).

Otherwise LGTM!

What is your test suite?

Thanks,

Phil.

Re: [PATCH] target/arm: Implement FEAT_IDST

[Richard Henderson]

On 5/9/22 10:54, Peter Maydell wrote:

@@ -841,6 +841,7 @@ static void aarch64_max_initfn(Object *obj)
  t = FIELD_DP64(t, ID_AA64MMFR2, VARANGE, 1); /* FEAT_LVA */
  t = FIELD_DP64(t, ID_AA64MMFR2, TTL, 1); /* FEAT_TTL */
  t = FIELD_DP64(t, ID_AA64MMFR2, BBM, 2); /* FEAT_BBM at level 2 */
+t = FIELD_DP64(t, ID_AA64MMFR2, IDS, 1); /* FEAT_IDST */


Ideally this should be sorted before TTL, by bit order.

Otherwise,
Reviewed-by: Richard Henderson 


r~

[PATCH v3 1/3] hw/intc: Move mtimer/mtimecmp to aclint

[Atish Patra]

Historically, The mtimer/mtimecmp has been part of the CPU because
they are per hart entities. However, they actually belong to aclint
which is a MMIO device.

Move them to the ACLINT device. This also emulates the real hardware
more closely.

Signed-off-by: Atish Patra 
---
 hw/intc/riscv_aclint.c | 41 --
 hw/timer/ibex_timer.c  | 18 ++-
 include/hw/intc/riscv_aclint.h |  2 ++
 include/hw/timer/ibex_timer.h  |  2 ++
 target/riscv/cpu.h |  2 --
 target/riscv/machine.c |  5 ++---
 6 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index 0412edc98257..83d317def395 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -32,6 +32,7 @@
 #include "hw/intc/riscv_aclint.h"
 #include "qemu/timer.h"
 #include "hw/irq.h"
+#include "migration/vmstate.h"
 
 typedef struct riscv_aclint_mtimer_callback {
 RISCVAclintMTimerState *s;
@@ -65,8 +66,8 @@ static void 
riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
 
 uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
 
-cpu->env.timecmp = value;
-if (cpu->env.timecmp <= rtc_r) {
+mtimer->timecmp[hartid] = value;
+if (mtimer->timecmp[hartid] <= rtc_r) {
 /*
  * If we're setting an MTIMECMP value in the "past",
  * immediately raise the timer interrupt
@@ -77,7 +78,7 @@ static void 
riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
 
 /* otherwise, set up the future timer interrupt */
 qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
-diff = cpu->env.timecmp - rtc_r;
+diff = mtimer->timecmp[hartid] - rtc_r;
 /* back to ns (note args switched in muldiv64) */
 uint64_t ns_diff = muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq);
 
@@ -102,7 +103,7 @@ static void 
riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
 next = MIN(next, INT64_MAX);
 }
 
-timer_mod(cpu->env.timer, next);
+timer_mod(mtimer->timers[hartid], next);
 }
 
 /*
@@ -133,11 +134,11 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, 
hwaddr addr,
   "aclint-mtimer: invalid hartid: %zu", hartid);
 } else if ((addr & 0x7) == 0) {
 /* timecmp_lo for RV32/RV64 or timecmp for RV64 */
-uint64_t timecmp = env->timecmp;
+uint64_t timecmp = mtimer->timecmp[hartid];
 return (size == 4) ? (timecmp & 0x) : timecmp;
 } else if ((addr & 0x7) == 4) {
 /* timecmp_hi */
-uint64_t timecmp = env->timecmp;
+uint64_t timecmp = mtimer->timecmp[hartid];
 return (timecmp >> 32) & 0x;
 } else {
 qemu_log_mask(LOG_UNIMP,
@@ -177,7 +178,7 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr 
addr,
 } else if ((addr & 0x7) == 0) {
 if (size == 4) {
 /* timecmp_lo for RV32/RV64 */
-uint64_t timecmp_hi = env->timecmp >> 32;
+uint64_t timecmp_hi = mtimer->timecmp[hartid] >> 32;
 riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
hartid,
 timecmp_hi << 32 | (value & 0x));
 } else {
@@ -188,7 +189,7 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr 
addr,
 } else if ((addr & 0x7) == 4) {
 if (size == 4) {
 /* timecmp_hi for RV32/RV64 */
-uint64_t timecmp_lo = env->timecmp;
+uint64_t timecmp_lo = mtimer->timecmp[hartid];
 riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
hartid,
 value << 32 | (timecmp_lo & 0x));
 } else {
@@ -233,7 +234,7 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr 
addr,
 continue;
 }
 riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
-  i, env->timecmp);
+  i, mtimer->timecmp[i]);
 }
 return;
 }
@@ -283,6 +284,8 @@ static void riscv_aclint_mtimer_realize(DeviceState *dev, 
Error **errp)
 s->timer_irqs = g_new(qemu_irq, s->num_harts);
 qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts);
 
+s->timers = g_malloc0(s->num_harts * sizeof(QEMUTimer));
+s->timecmp = g_new0(uint64_t, s->num_harts);
 /* Claim timer interrupt bits */
 for (i = 0; i < s->num_harts; i++) {
 RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
@@ -309,6 +312,18 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, 
ResetType type)
 riscv_aclint_mtimer_write(mtimer, mtimer->time_base, 0, 8);
 }
 
+static const VMStateDescription vmstate_riscv_mtimer = {
+.name = "riscv_mtimer",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+

[PATCH v3 0/3] Implement Sstc extension

[Atish Patra]

This series implements Sstc extension[1] which was ratified recently.

The first patch is a prepartory patches while PATCH 2 adds stimecmp
support while PATCH 3 adds vstimecmp support. This series is based on
on top of upstream commit (faee5441a038).

The series can also be found at
https://github.com/atishp04/qemu/tree/sstc_v3

It is tested on RV32 & RV64 with latest OpenSBI & Linux kernel[2]
patches. It does require the mcountinhibi support patch from PMU series[3]
or dummy one from Anup's series [4]. Otherwise, OpenSBI doesn't detect v1.12
priv version.

Changes from v2->v3:
1. Dropped generic migration code improvement patches.
2. Removed the order constraints while updating stimecmp/vstimecmp.

Changes from v1->v2:
1. Rebased on the latest upstream commit.
2. Replaced PATCH 1 with another patch where mtimer/timecmp is
   moved from CPU to ACLINT.
3. Added ACLINT migration support.

[1] https://drive.google.com/file/d/1m84Re2yK8m_vbW7TspvevCDR82MOBaSX/view
[2] https://github.com/atishp04/linux/tree/sstc_v3
[3] https://github.com/atishp04/qemu/tree/riscv_pmu_v7
[4] https://www.mail-archive.com/qemu-devel@nongnu.org/msg885157.html

Atish Patra (3):
hw/intc: Move mtimer/mtimecmp to aclint
target/riscv: Add stimecmp support
target/riscv: Add vstimecmp support

hw/intc/riscv_aclint.c |  41 +--
hw/timer/ibex_timer.c  |  18 ++-
include/hw/intc/riscv_aclint.h |   2 +
include/hw/timer/ibex_timer.h  |   2 +
target/riscv/cpu.c |   8 ++
target/riscv/cpu.h |  14 ++-
target/riscv/cpu_bits.h|   8 ++
target/riscv/cpu_helper.c  |  11 +-
target/riscv/csr.c | 200 +
target/riscv/machine.c |   7 +-
target/riscv/meson.build   |   3 +-
target/riscv/time_helper.c | 114 +++
target/riscv/time_helper.h |  30 +
13 files changed, 426 insertions(+), 32 deletions(-)
create mode 100644 target/riscv/time_helper.c
create mode 100644 target/riscv/time_helper.h

--
2.25.1

[PATCH v3 2/3] target/riscv: Add stimecmp support

[Atish Patra]

stimecmp allows the supervisor mode to update stimecmp CSR directly
to program the next timer interrupt. This CSR is part of the Sstc
extension which was ratified recently.

Signed-off-by: Atish Patra 
---
 target/riscv/cpu.c |  8 
 target/riscv/cpu.h |  7 +++
 target/riscv/cpu_bits.h|  4 ++
 target/riscv/csr.c | 92 +++
 target/riscv/machine.c |  1 +
 target/riscv/meson.build   |  3 +-
 target/riscv/time_helper.c | 98 ++
 target/riscv/time_helper.h | 30 
 8 files changed, 242 insertions(+), 1 deletion(-)
 create mode 100644 target/riscv/time_helper.c
 create mode 100644 target/riscv/time_helper.h

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 19f4e8294042..d58dd2f857a7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -23,6 +23,7 @@
 #include "qemu/log.h"
 #include "cpu.h"
 #include "internals.h"
+#include "time_helper.h"
 #include "exec/exec-all.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -779,7 +780,12 @@ static void riscv_cpu_init(Object *obj)
 #ifndef CONFIG_USER_ONLY
 qdev_init_gpio_in(DEVICE(cpu), riscv_cpu_set_irq,
   IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
+
+if (cpu->cfg.ext_sstc) {
+riscv_timer_init(cpu);
+}
 #endif /* CONFIG_USER_ONLY */
+
 }
 
 static Property riscv_cpu_properties[] = {
@@ -806,6 +812,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
 DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true),
 
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
 DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
@@ -965,6 +972,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
**isa_str, int max_str_len)
 ISA_EDATA_ENTRY(zbs, ext_zbs),
 ISA_EDATA_ENTRY(zve32f, ext_zve32f),
 ISA_EDATA_ENTRY(zve64f, ext_zve64f),
+ISA_EDATA_ENTRY(sstc, ext_sstc),
 ISA_EDATA_ENTRY(svinval, ext_svinval),
 ISA_EDATA_ENTRY(svnapot, ext_svnapot),
 ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1119d5201066..9a01e6d0f587 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -276,6 +276,11 @@ struct CPUArchState {
 uint64_t mfromhost;
 uint64_t mtohost;
 
+/* Sstc CSRs */
+uint64_t stimecmp;
+/* For RV32 only */
+uint8_t stimecmp_wr_done;
+
 /* physical memory protection */
 pmp_table_t pmp_state;
 target_ulong mseccfg;
@@ -329,6 +334,7 @@ struct CPUArchState {
 float_status fp_status;
 
 /* Fields from here on are preserved across CPU reset. */
+QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
 
 hwaddr kernel_addr;
 hwaddr fdt_addr;
@@ -379,6 +385,7 @@ struct RISCVCPUConfig {
 bool ext_counters;
 bool ext_ifencei;
 bool ext_icsr;
+bool ext_sstc;
 bool ext_svinval;
 bool ext_svnapot;
 bool ext_svpbmt;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 4e5b630f5965..29d0e4a1be01 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -215,6 +215,10 @@
 #define CSR_STVAL   0x143
 #define CSR_SIP 0x144
 
+/* Sstc supervisor CSRs */
+#define CSR_STIMECMP0x14D
+#define CSR_STIMECMPH   0x15D
+
 /* Supervisor Protection and Translation */
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 245f007e66e1..8952d1308008 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -21,6 +21,7 @@
 #include "qemu/log.h"
 #include "qemu/timer.h"
 #include "cpu.h"
+#include "time_helper.h"
 #include "qemu/main-loop.h"
 #include "exec/exec-all.h"
 #include "sysemu/cpu-timers.h"
@@ -537,6 +538,87 @@ static RISCVException read_timeh(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException sstc(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (env->priv == PRV_M) {
+return RISCV_EXCP_NONE;
+}
+
+if (env->priv != PRV_S) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+/*
+ * No need of separate function for rv32 as menvcfg stores both menvcfg
+ * menvcfgh for RV32.
+ */
+if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
+  get_field(env->menvcfg, MENVCFG_STCE))) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_stimecmp(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->stimecmp;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_stimecmph(CPUR

[PATCH v3 3/3] target/riscv: Add vstimecmp support

[Atish Patra]

vstimecmp CSR allows the guest OS or to program the next guest timer
interrupt directly. Thus, hypervisor no longer need to inject the
timer interrupt to the guest if vstimecmp is used. This was ratified
as a part of the Sstc extension.

Signed-off-by: Atish Patra 
---
 target/riscv/cpu.h |   5 ++
 target/riscv/cpu_bits.h|   4 ++
 target/riscv/cpu_helper.c  |  11 +++-
 target/riscv/csr.c | 112 -
 target/riscv/machine.c |   1 +
 target/riscv/time_helper.c |  16 ++
 6 files changed, 144 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 9a01e6d0f587..798874d6189d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -280,6 +280,9 @@ struct CPUArchState {
 uint64_t stimecmp;
 /* For RV32 only */
 uint8_t stimecmp_wr_done;
+uint64_t vstimecmp;
+/* For RV32 only */
+uint8_t vstimecmp_wr_done;
 
 /* physical memory protection */
 pmp_table_t pmp_state;
@@ -335,6 +338,8 @@ struct CPUArchState {
 
 /* Fields from here on are preserved across CPU reset. */
 QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
+QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
+bool vstime_irq;
 
 hwaddr kernel_addr;
 hwaddr fdt_addr;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 29d0e4a1be01..5c9f512872e1 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -272,6 +272,10 @@
 #define CSR_VSIP0x244
 #define CSR_VSATP   0x280
 
+/* Sstc virtual CSRs */
+#define CSR_VSTIMECMP   0x24D
+#define CSR_VSTIMECMPH  0x25D
+
 #define CSR_MTINST  0x34a
 #define CSR_MTVAL2  0x34b
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e1aa4f2097c1..2715021c022e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -344,8 +344,9 @@ static uint64_t riscv_cpu_all_pending(CPURISCVState *env)
 {
 uint32_t gein = get_field(env->hstatus, HSTATUS_VGEIN);
 uint64_t vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0;
+uint64_t vstip = (env->vstime_irq) ? MIP_VSTIP : 0;
 
-return (env->mip | vsgein) & env->mie;
+return (env->mip | vsgein | vstip) & env->mie;
 }
 
 int riscv_cpu_mirq_pending(CPURISCVState *env)
@@ -604,7 +605,7 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, 
uint64_t value)
 {
 CPURISCVState *env = &cpu->env;
 CPUState *cs = CPU(cpu);
-uint64_t gein, vsgein = 0, old = env->mip;
+uint64_t gein, vsgein = 0, vstip = 0, old = env->mip;
 bool locked = false;
 
 if (riscv_cpu_virt_enabled(env)) {
@@ -612,6 +613,10 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t 
mask, uint64_t value)
 vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0;
 }
 
+/* No need to update mip for VSTIP */
+mask = ((mask == MIP_VSTIP) && env->vstime_irq) ? 0 : mask;
+vstip = env->vstime_irq ? MIP_VSTIP : 0;
+
 if (!qemu_mutex_iothread_locked()) {
 locked = true;
 qemu_mutex_lock_iothread();
@@ -619,7 +624,7 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, 
uint64_t value)
 
 env->mip = (env->mip & ~mask) | (value & mask);
 
-if (env->mip | vsgein) {
+if (env->mip | vsgein | vstip) {
 cpu_interrupt(cs, CPU_INTERRUPT_HARD);
 } else {
 cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8952d1308008..35eb2c4d84eb 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -567,17 +567,110 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException sstc_hmode(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (env->priv == PRV_M) {
+return RISCV_EXCP_NONE;
+}
+
+if (!(get_field(env->mcounteren, COUNTEREN_TM) &
+  get_field(env->menvcfg, MENVCFG_STCE))) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (!(get_field(env->hcounteren, COUNTEREN_TM) &
+  get_field(env->henvcfg, HENVCFG_STCE))) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->vstimecmp;
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->vstimecmp >> 32;
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
+target_ulong val)
+{
+RISCVCPU *cpu = env_archcpu(env);
+
+if (riscv_cpu_mxl(env) == MXL_RV32) {
+env->vstimecmp = de

Re: [PATCH] Hexagon (target/hexagon) remove unused encodings

[Philippe Mathieu-Daudé via]

On 9/5/22 23:14, Taylor Simpson wrote:

Remove encodings guarded by ifdef that is not defined

Signed-off-by: Taylor Simpson 
---
  target/hexagon/imported/encode_pp.def | 23 ---
  1 file changed, 23 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/3] ui/gtk: new param monitor to specify target monitor for launching QEMU

[Dongwon Kim]

Daniel,

I found a way to make the monitor arguments in array type (['uint32']).
And I know how to retrieve monitor values from it but I could not find
how to pass the monitor values when starting qemu. Like,

qemu-system-x86_64 . gtk,gl=on.monitor=

I tried several different things but it keeps getting error saying
Invalid parameter type, expected 'array'.

Do you know how to pass this arg?

Thanks,
DW

On Tue, May 03, 2022 at 10:15:13AM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 28, 2022 at 04:13:02PM -0700, Dongwon Kim wrote:
> > Introducing a new integer parameter to specify the monitor where the
> > Qemu window is placed upon launching.
> > 
> > Monitor can be any number between 0 and (total number of monitors - 1).
> > 
> > It can be used together with full-screen=on, which will make the QEMU
> > window full-screened on the targeted monitor.
> > 
> > v2: fixed typos and updated commit subject and msg
> > (Philippe Mathieu-Daudé)
> > 
> > changed param name to monitor, removed unnecessary condition check
> > on the parameter
> > (Paolo Bonzini)
> > 
> > v3: updated Since version to 7.1 for monitor parameter
> > 
> > Cc: Philippe Mathieu-Daudé 
> > Cc: Paolo Bonzini 
> > Cc: Gerd Hoffmann 
> > Cc: Vivek Kasireddy 
> > Signed-off-by: Dongwon Kim 
> > ---
> >  qapi/ui.json| 6 +-
> >  qemu-options.hx | 2 +-
> >  ui/gtk.c| 8 
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 059302a5ef..ddcea7349b 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1204,13 +1204,17 @@
> >  #   assuming the guest will resize the display to match
> >  #   the window size then.  Otherwise it defaults to "off".
> >  #   Since 3.1
> > +# @monitor: Indicate monitor where QEMU window is lauched. monitor
> > +#   could be any number from 0 to (total num of monitors - 1).
> > +#   since 7.1
> >  #
> >  # Since: 2.12
> >  #
> >  ##
> >  { 'struct'  : 'DisplayGTK',
> >'data': { '*grab-on-hover' : 'bool',
> > -'*zoom-to-fit'   : 'bool'  } }
> > +'*zoom-to-fit'   : 'bool',
> > +'*monitor'   : 'uint32' } }
> 
> I feel like this ought to be an array of monitors, so that we can have
> explicit positioning when we have multiple graphical outputs and are
> creating a separate window for each.
> 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> 

Re: [PATCH] Hexagon (target/hexagon) move store size tracking to translation

[Philippe Mathieu-Daudé via]

On 9/5/22 23:14, Taylor Simpson wrote:

The store width is needed for packet commit, so it is stored in
ctx->store_width.  Currently, it is set when a store has a TCG
override instead of a QEMU helper.  In the QEMU helper case, the
ctx->store_width is not set, we invoke a helper during packet commit
that uses the runtime store width.

This patch ensures ctx->store_width is set for all store instructions,
so performance is improved because packet commit can generate the proper
TCG store rather than the generic helper.

We do this by
- Create new attributes to indicate the store size
- During gen_semantics, convert the fSTORE instances to fSTORE
- Assign the new attributes to the new macros
- Add definitions for the new macros
- Use the attributes from the instructions during translation to
   set ctx->store_width
- Remove setting of ctx->store_width from genptr.c

Signed-off-by: Taylor Simpson 
---
  target/hexagon/macros.h  | 16 ++
  target/hexagon/attribs_def.h.inc |  4 
  target/hexagon/gen_semantics.c   | 26 +++
  target/hexagon/genptr.c  | 36 +++-
  target/hexagon/translate.c   | 26 +++
  5 files changed, 80 insertions(+), 28 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/2] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't

[Richard Henderson]

On 5/9/22 07:48, Víctor Colombo wrote:

-static inline void float_inexact_excp(CPUPPCState *env)
+static inline void float_inexact_excp(CPUPPCState *env, bool set_fi)
  {
  CPUState *cs = env_cpu(env);
  
-env->fpscr |= FP_FI;

+if (set_fi) {
+env->fpscr |= FP_FI;
+}
  env->fpscr |= FP_XX;
  /* Update the floating-point exception summary */
  env->fpscr |= FP_FX;


I think it would be better to move the change to FI from here...


@@ -462,7 +464,8 @@ void helper_fpscr_check_status(CPUPPCState *env)
  }
  }
  
-static void do_float_check_status(CPUPPCState *env, uintptr_t raddr)

+static void do_float_check_status(CPUPPCState *env, bool change_fi,
+  uintptr_t raddr)
  {
  CPUState *cs = env_cpu(env);
  int status = get_float_exception_flags(&env->fp_status);
@@ -473,8 +476,8 @@ static void do_float_check_status(CPUPPCState *env, 
uintptr_t raddr)
  float_underflow_excp(env);
  }
  if (status & float_flag_inexact) {
-float_inexact_excp(env);
-} else {
+float_inexact_excp(env, change_fi);
+} else if (change_fi) {
  env->fpscr &= ~FP_FI; /* clear the FPSCR[FI] bit */
  }


... to here.  E.g.

if (status & float_flag_inexact) {
float_inexact_excp(env);
}
if (change_fi) {
if (status & float_flag_inexact) {
env->fpscr |= FP_FI;
} else {
env->fpscr &= ~FP_FI;
}
}

or indeed

env->fpscr = FIELD_DP64(env->fpscr, FPSCR, FI,
!!(status & float_flag_inexact));

Otherwise it all looks plausible.


@@ -1690,9 +1693,9 @@ uint32_t helper_efdcmpeq(CPUPPCState *env, uint64_t op1, 
uint64_t op2)
   *   nels  - number of elements (1, 2 or 4)
   *   tp- type (float32 or float64)
   *   fld   - vsr_t field (VsrD(*) or VsrW(*))
- *   sfprf - set FPRF
+ *   sfifprf - set FI and FPRF
   */
-#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf, r2sp)\
+#define VSX_ADD_SUB(name, op, nels, tp, fld, sfifprf, r2sp)  \



It might be easier to read if this renaming is done as a separate step.


r~

Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices

[Daniel Henrique Barboza]

On 5/9/22 18:17, Mark Cave-Ayland wrote:

On 07/05/2022 20:06, Daniel Henrique Barboza wrote:


Hi,

Since the 7.0.0 release cycle we have a desire to use the powernv
emulation with libvirt. To do that we need to enable user creatable
pnv-phb devices to allow more user customization an to avoid spamming
multiple default devices in the domain XML. In the middle of the
previous cycle we experimented with user created
pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
end result, although functional, is that the user needs to deal with a
lot of versioned devices that, from the user perspective, does the same
thing. In a way we outsourced the implementation details of the PHBs
(e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
devices also puts an extra burden in the libvirt support we want to
have.

To solve this, Cedric and Frederic gave the idea of adding a common
virtual pnv-phb device that the user can add in the command line, and
QEMU handles the details internally. Unfortunatelly this idea turned out
to be way harder than anticipated. Between creating a device that would
just forward the callbacks to the existing devices internally, creating
a PnvPHB device with the minimal attributes and making the other devices
inherit from it, and making an 'abstract' device that could be casted
for both phb3 and phb4 PHBs,


This bit sounds all good...


all sorts of very obscure problems occured:
PHBs not being detected, interrupts not being delivered and memory
regions not being able to read/write registers. My initial impression is
that there are assumptions made both in ppc/pnv and hw/pci-host that
requires the memory region and the bus being in the same device. Even
if we somehow figure all this out, the resulting code is hacky and
annoying to maitain.


But this seems really surprising since there are many examples of similar QOM 
patterns within the codebase, and in my experience they work really well. Do 
you have a particular example that you can share to demonstrate why the result 
of the QOM mapping is making things more difficult?



It's not so much about the OOM getting in the way, but rather myself not being
able to use QOM in a way to get what I want. There is a good probability that
QOM is able to do it.

Talking about the 2 PHBs pnv-phb3 (powernv8 only) and pnv-phb4 (powernv9 only),
what we want is a way to let the user instantiate both using an alias, or
an abstract object if you will, called "pnv-phb". This alias/abstraction would
instantiate either a pnv-phb3 or a pnv-phb4 depending on the actual machine
running.

QOM has the "interface" concept that is used internally to make the device 
behave
like the interface describes it, but I wasn't able to expose this kind of object
to the user. It also has the "abstract" concept that, by the documentation 
itself,
isn't supposed to be user created. Eventually I gave up this idea, accepting 
that
only real devices can be exposed to the user (please correct me if I'm wrong).

After that I tried to create a pnv-phb object that contains the common 
attributes
of both pnv-phb3 and pnv-phb4. This object would be the parent of phb3 and phb4,
and during realize time creating a pnv-phb object would create a pnv-phb3/4 and
we go from there. This attempt went far after a few tweaks but then I started
to hit the problems I mentioned above: some strange behaviors with interrupts,
PHBs not appearing and so on. I got a little farther by moving all the memory
regions from phb3/phb4 to the parent phb object and I got up to the guest login,
but without network. Since moving the memory regions in the same object as the
pci root bus got me farther I concluded that there might some 
relation/assumption
made somewhere that I was breaking before.

After all that I got more than halfway of what I ended up doing. I decided to
unify phb3/phb4 into a single device, renamed their attributes that has 
different
meanings between v3 and v4 code, and I ended up with this series.





This brings us to this series. The cleaner way I found to accomplish
what we want to do is to create real, unified pnv-phb/phb-phb-root-port
devices, and get rid of all the versioned ones. This is done by
merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
files end up using the same pnv-phb and phb-phb-root-port unified devices,
with the difference that pnv_phb3* only cares about version 3 attributes
and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
versions in the future will be a matter of adding any non-existent
attributes in the unified pnv-phb* devices and using them in separated
pnv_phbN* files.

The pnv-phb implementation per se is just a router for either phb3 or
phb4 logic, done in init() and realize() time, after we check which powernv
machine we're running. If running with powernv8 we redirect control to
pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
unified device does not do anyth

Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory

[Michael Roth]

On Fri, Apr 22, 2022 at 06:56:12PM +0800, Chao Peng wrote:
> Great thanks for the discussions. I summarized the requirements/gaps and the
> potential changes for next step. Please help to review.

Hi Chao,

Thanks for writing this up. I've been meaning to respond, but wanted to
make a bit more progress with SNP+UPM prototype to get a better idea of
what's needed on that end. I've needed to make some changes on the KVM
and QEMU side to get things working so hopefully with your proposed
rework those changes can be dropped.

> 
> 
> Terminologies:
> --
>   - memory conversion: the action of converting guest memory between private
> and shared.
>   - explicit conversion: an enlightened guest uses a hypercall to explicitly
> request a memory conversion to VMM.
>   - implicit conversion: the conversion when VMM reacts to a page fault due
> to different guest/host memory attributes (private/shared).
>   - destructive conversion: the memory content is lost/destroyed during
> conversion.
>   - non-destructive conversion: the memory content is preserved during
> conversion.
> 
> 
> Requirements & Gaps
> -
>   - Confidential computing(CC): TDX/SEV/CCA
> * Need support both explicit/implicit conversions.
> * Need support only destructive conversion at runtime.
> * The current patch should just work, but prefer to have pre-boot guest
>   payload/firmware population into private memory for performance.

Not just performance in the case of SEV, it's needed there because firmware
only supports in-place encryption of guest memory, there's no mechanism to
provide a separate buffer to load into guest memory at pre-boot time. I
think you're aware of this but wanted to point that out just in case.

> 
>   - pKVM
> * Support explicit conversion only. Hard to achieve implicit conversion,
>   does not record the guest access info (private/shared) in page fault,
>   also makes little sense.
> * Expect to support non-destructive conversion at runtime. Additionally
>   in-place conversion (the underlying physical page is unchanged) is
>   desirable since copy is not disirable. The current destructive 
> conversion
>   does not fit well.
> * The current callbacks between mm/KVM is useful and reusable for pKVM.
> * Pre-boot guest payload population is nice to have.
> 
> 
> Change Proposal
> ---
> Since there are some divergences for pKVM from CC usages and at this time 
> looks
> whether we will and how we will support pKVM with this private memory patchset
> is still not quite clear, so this proposal does not imply certain detailed 
> pKVM
> implementation. But from the API level, we want this can be possible to be 
> future
> extended for pKVM or other potential usages.
> 
>   - No new user APIs introduced for memory backing store, e.g. remove the
> current MFD_INACCESSIBLE. This info will be communicated from 
> memfile_notifier
> consumers to backing store via the new 'flag' field in memfile_notifier
> described below. At creation time, the fd is normal shared fd. At rumtime 
> CC
> usages will keep using current fallocate/FALLOC_FL_PUNCH_HOLE to do the
> conversion, but pKVM may also possible use a different way (e.g. rely on
> mmap/munmap or mprotect as discussed). These are all not new APIs anyway.

For SNP most of the explicit conversions are via GHCB page-state change
requests. Each of these PSC requests can request shared/private
conversions for up to 252 individual pages, along with whether or not
they should be treated as 4K or 2M pages. Currently, like with
KVM_EXIT_MEMORY_ERROR, these requests get handled in userspace and call
back into the kernel via fallocate/PUNCH_HOLE calls.

For each fallocate(), we need to update the RMP table to mark a page as
private, and for PUNCH_HOLE we need to mark it as shared (otherwise it
would be freed back to the host as guest-owned/private and cause a crash if
the host tries to re-use it for something). I needed to add some callbacks
to the memfile_notifier to handle these RMP table updates. There might be
some other bits of book-keeping like clflush's, and adding/removing guest
pages from the kernel's direct map.

Not currently implemented, but the guest can also issue requests to
"smash"/"unsmash" a 2M private range into individual 4K private ranges
(generally in advance of flipping one of the pages to shared, or
vice-versa) in the RMP table. Hypervisor code tries to handle this
automatically, by determining when to smash/unsmash on it's own, but...

I'm wondering how all these things can be properly conveyed through this
fallocate/PUNCH_HOLE interface if we ever needed to add support for all
of this, as it seems a bit restrictive as-is. For instance, with the
current approach, one possible scheme is:

  - explicit conversion of shared->private for 252 4K pages:
- we could do 252 individual fallocate()'s of 4K each, and make su

RE: [PATCH 4/5] hw/intc/arm_gicv3: Use correct number of priority bits for the CPU

[ishii.shuuic...@fujitsu.com]

Hi, Peter.

> Shuuichirou, Itaru: do either of you know the right setting for the A64FX for 
> this? If
> you can find what the hardware value of the ICC_CTLR_EL3 or ICC_CTLR_EL1
> register is (more specifically, the PRIBits subfield) that should be enough 
> to tell
> us.

The value of the PRIbits field in the A64FX is 0x4.
Therefore, the following values is fine.

> > +cpu->gic_pribits = 5;

Best regards,
Shuuichirou.

P.S.
Itaru, thank you for the follow-up.

> -Original Message-
> From: Peter Maydell 
> Sent: Saturday, May 7, 2022 1:34 AM
> To: qemu-...@nongnu.org; qemu-devel@nongnu.org
> Cc: Ishii, Shuuichirou/石井 周一郎 ; Itaru Kitayama
> 
> Subject: Re: [PATCH 4/5] hw/intc/arm_gicv3: Use correct number of priority 
> bits
> for the CPU
> 
> On Fri, 6 May 2022 at 17:21, Peter Maydell  wrote:
> >
> > Make the GICv3 set its number of bits of physical priority from the
> > implementation-specific value provided in the CPU state struct, in the
> > same way we already do for virtual priority bits.  Because this would
> > be a migration compatibility break, we provide a property
> > force-8-bit-prio which is enabled for 7.0 and earlier versioned board
> > models to retain the legacy "always use 8 bits" behaviour.
> >
> > Signed-off-by: Peter Maydell 
> > ---
> > I have guessed at the right value for the A64FX, but if we can find
> > the correct ICC_CTLR_EL1 value that would be better.
> 
> Shuuichirou, Itaru: do either of you know the right setting for the A64FX for 
> this? If
> you can find what the hardware value of the ICC_CTLR_EL3 or ICC_CTLR_EL1
> register is (more specifically, the PRIBits subfield) that should be enough 
> to tell
> us.
> 
> > @@ -961,6 +964,12 @@ static void aarch64_a64fx_initfn(Object *obj)
> >  cpu->gic_num_lrs = 4;
> >  cpu->gic_vpribits = 5;
> >  cpu->gic_vprebits = 5;
> > +/*
> > + * TODO: What does the real A64FX GICv3 provide ?
> > + * This is a guess based on what other Arm CPUs do; to find the correct
> > + * answer we need the value of the A64FX's ICC_CTLR_EL1 register.
> > + */
> > +cpu->gic_pribits = 5;
> >
> >  /* Suppport of A64FX's vector length are 128,256 and 512bit only */
> >  aarch64_add_sve_properties(obj);
> > --
> > 2.25.1
> 
> thanks
> -- PMM

Re: [PATCH 2/2] target/ppc: Fix FPSCR.FI changing in float_overflow_excp()

[Richard Henderson]

On 5/9/22 07:48, Víctor Colombo wrote:

This patch fixes another not-so-clear situation in Power ISA
regarding the inexact bits in FPSCR. The ISA states that:

"""
When Overflow Exception is disabled (OE=0) and an
Overflow Exception occurs, the following actions are
taken:
...
2. Inexact Exception is set
XX <- 1
...
FI is set to 1
...
"""

However, when tested on a Power 9 hardware, some instructions that
trigger an OX don't set the FI bit:

xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> CLEARED
xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> CLEARED
(just a few examples. Other instructions are also affected)

The root cause for this seems to be that only instructions that list
the bit FI in the "Special Registers Altered" should modify it.

QEMU is, today, not working like the hardware:

xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> SET
xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> SET

(all tests assume FI is cleared beforehand)

Fix this by passing an argument to float_overflow_excp() indicating
if the FI should be set.

Signed-off-by: Víctor Colombo 
---
  target/ppc/fpu_helper.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 773c80e12d..ee1259ede1 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -329,7 +329,7 @@ static inline void float_zero_divide_excp(CPUPPCState *env, 
uintptr_t raddr)
  }
  }
  
-static inline void float_overflow_excp(CPUPPCState *env)

+static inline void float_overflow_excp(CPUPPCState *env, bool set_fi)
  {
  CPUState *cs = env_cpu(env);
  
@@ -345,7 +345,9 @@ static inline void float_overflow_excp(CPUPPCState *env)

  env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_OX;
  } else {
  env->fpscr |= FP_XX;
-env->fpscr |= FP_FI;
+if (set_fi) {
+env->fpscr |= FP_FI;
+}



Again, I believe setting FP_FI here is wrong, it should only be set later in 
do_float_check_status.  Indeed, setting XX here probably isn't best...

..

@@ -471,7 +473,7 @@ static void do_float_check_status(CPUPPCState *env, bool 
change_fi,
  int status = get_float_exception_flags(&env->fp_status);
  
  if (status & float_flag_overflow) {

-float_overflow_excp(env);
+float_overflow_excp(env, change_fi);


I think the ideal solution would be to return an update to status from float_overflow_excp 
so that all of the inexact handling happens below.  Since inexact is the last bit to be 
processed, this could be as simple as


if (status & overflow) {
status = float_overflow_excp(env);
} else if (status & underflow) {
...
}
if (status & inexact) {
...

returning OE ? 0 : float_flag_inexact, without trying to merge inexact into the full set 
of status flags.



r~

Re: [PATCH v2 1/3] linux-user/elfload: Remove pointless non-const CPUArchState cast

[Richard Henderson]

On 5/9/22 15:57, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

fill_thread_info() takes a pointer to const.

Signed-off-by: Philippe Mathieu-Daudé
---
  linux-user/elfload.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 2/3] linux-user: Have do_syscall() use CPUArchState* instead of void*

[Richard Henderson]

On 5/9/22 15:57, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé
---
  linux-user/strace.c | 202 ++--
  linux-user/strace.h |   4 +-
  linux-user/syscall.c|  32 +++---
  linux-user/uname.c  |   2 +-
  linux-user/uname.h  |   2 +-
  linux-user/user-internals.h |  16 +--
  6 files changed, 129 insertions(+), 129 deletions(-)


This could probably be split into parts, but not worth the extra effort.

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 3/3] linux-user: Remove pointless CPU{ARCH}State casts

[Richard Henderson]

On 5/9/22 15:57, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé
---
  linux-user/syscall.c| 49 +
  linux-user/uname.c  |  2 +-
  linux-user/user-internals.h |  2 +-
  3 files changed, 25 insertions(+), 28 deletions(-)


This is the real win.  Thanks,

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory

[Sean Christopherson]

On Mon, May 09, 2022, Michael Roth wrote:
> On Fri, Apr 22, 2022 at 06:56:12PM +0800, Chao Peng wrote:
> > Requirements & Gaps
> > -
> >   - Confidential computing(CC): TDX/SEV/CCA
> > * Need support both explicit/implicit conversions.
> > * Need support only destructive conversion at runtime.
> > * The current patch should just work, but prefer to have pre-boot guest
> >   payload/firmware population into private memory for performance.
> 
> Not just performance in the case of SEV, it's needed there because firmware
> only supports in-place encryption of guest memory, there's no mechanism to
> provide a separate buffer to load into guest memory at pre-boot time. I
> think you're aware of this but wanted to point that out just in case.

I view it as a performance problem because nothing stops KVM from copying from
userspace into the private fd during the SEV ioctl().  What's missing is the
ability for userspace to directly initialze the private fd, which may or may not
avoid an extra memcpy() depending on how clever userspace is.

> 
> > 
> >   - pKVM
> > * Support explicit conversion only. Hard to achieve implicit conversion,
> >   does not record the guest access info (private/shared) in page fault,
> >   also makes little sense.
> > * Expect to support non-destructive conversion at runtime. Additionally
> >   in-place conversion (the underlying physical page is unchanged) is
> >   desirable since copy is not disirable. The current destructive 
> > conversion
> >   does not fit well.
> > * The current callbacks between mm/KVM is useful and reusable for pKVM.
> > * Pre-boot guest payload population is nice to have.
> > 
> > 
> > Change Proposal
> > ---
> > Since there are some divergences for pKVM from CC usages and at this time 
> > looks
> > whether we will and how we will support pKVM with this private memory 
> > patchset
> > is still not quite clear, so this proposal does not imply certain detailed 
> > pKVM
> > implementation. But from the API level, we want this can be possible to be 
> > future
> > extended for pKVM or other potential usages.
> > 
> >   - No new user APIs introduced for memory backing store, e.g. remove the
> > current MFD_INACCESSIBLE. This info will be communicated from 
> > memfile_notifier
> > consumers to backing store via the new 'flag' field in memfile_notifier
> > described below. At creation time, the fd is normal shared fd. At 
> > rumtime CC
> > usages will keep using current fallocate/FALLOC_FL_PUNCH_HOLE to do the
> > conversion, but pKVM may also possible use a different way (e.g. rely on
> > mmap/munmap or mprotect as discussed). These are all not new APIs 
> > anyway.
> 
> For SNP most of the explicit conversions are via GHCB page-state change
> requests. Each of these PSC requests can request shared/private
> conversions for up to 252 individual pages, along with whether or not
> they should be treated as 4K or 2M pages. Currently, like with
> KVM_EXIT_MEMORY_ERROR, these requests get handled in userspace and call
> back into the kernel via fallocate/PUNCH_HOLE calls.
> 
> For each fallocate(), we need to update the RMP table to mark a page as
> private, and for PUNCH_HOLE we need to mark it as shared (otherwise it
> would be freed back to the host as guest-owned/private and cause a crash if
> the host tries to re-use it for something). I needed to add some callbacks
> to the memfile_notifier to handle these RMP table updates. There might be
> some other bits of book-keeping like clflush's, and adding/removing guest
> pages from the kernel's direct map.
> 
> Not currently implemented, but the guest can also issue requests to
> "smash"/"unsmash" a 2M private range into individual 4K private ranges
> (generally in advance of flipping one of the pages to shared, or
> vice-versa) in the RMP table. Hypervisor code tries to handle this
> automatically, by determining when to smash/unsmash on it's own, but...
> 
> I'm wondering how all these things can be properly conveyed through this
> fallocate/PUNCH_HOLE interface if we ever needed to add support for all
> of this, as it seems a bit restrictive as-is. For instance, with the
> current approach, one possible scheme is:
> 
>   - explicit conversion of shared->private for 252 4K pages:
> - we could do 252 individual fallocate()'s of 4K each, and make sure the
>   kernel code will do notifier callbacks / RMP updates for each individual
>   4K page
> 
>   - shared->private for 252 2M pages:
> - we could do 252 individual fallocate()'s of 2M each, and make sure the
>   kernel code will do notifier callbacks / RMP updates for each individual
>   2M page
> 
> But for SNP most of these bulk PSC changes are when the guest switches
> *all* of it's pages from shared->private during early boot when it
> validates all of it's memory. So these pages tend to be contiguous
> ranges, and a 

[PATCH 2/2] target/arm: Use FIELD definitions for CPACR, CPTR_ELx

[Richard Henderson]

We had a few CPTR_* bits defined, but missed quite a few.
Complete all of the fields up to ARMv9.2.
Use FIELD_EX64 instead of manual extract32.

Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h| 44 +++-
 hw/arm/boot.c   |  2 +-
 target/arm/cpu.c| 11 ++---
 target/arm/helper.c | 54 ++---
 4 files changed, 75 insertions(+), 36 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b35b117fe7..c44acd8b84 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1259,11 +1259,45 @@ void pmu_init(ARMCPU *cpu);
 #define SCTLR_SPINTMASK (1ULL << 62) /* FEAT_NMI */
 #define SCTLR_TIDCP   (1ULL << 63) /* FEAT_TIDCP1 */
 
-#define CPTR_TCPAC(1U << 31)
-#define CPTR_TTA  (1U << 20)
-#define CPTR_TFP  (1U << 10)
-#define CPTR_TZ   (1U << 8)   /* CPTR_EL2 */
-#define CPTR_EZ   (1U << 8)   /* CPTR_EL3 */
+/* Bit definitions for CPACR (AArch32 only) */
+FIELD(CPACR, CP10, 20, 2)
+FIELD(CPACR, CP11, 22, 2)
+FIELD(CPACR, TRCDIS, 28, 1)/* matches CPACR_EL1.TTA */
+FIELD(CPACR, D32DIS, 31, 1)/* up to v7; RAZ in v8 */
+FIELD(CPACR, ASEDIS, 31, 1)
+
+/* Bit definitions for CPACR_EL1 (AArch64 only) */
+FIELD(CPACR_EL1, ZEN, 16, 2)
+FIELD(CPACR_EL1, FPEN, 20, 2)
+FIELD(CPACR_EL1, SMEN, 24, 2)
+FIELD(CPACR_EL1, TTA, 28, 1)   /* matches CPACR.TRCDIS */
+
+/* Bit definitions for HCPTR (AArch32 only) */
+FIELD(HCPTR, TCP10, 10, 1)
+FIELD(HCPTR, TCP11, 11, 1)
+FIELD(HCPTR, TSAE, 15, 1)
+FIELD(HCPTR, TTA, 20, 1)
+FIELD(HCPTR, TAM, 30, 1)   /* matches CPTR_EL2.TAM */
+FIELD(HCPTR, TCPAC, 31, 1) /* matches CPTR_EL2.TCPAC */
+
+/* Bit definitions for CPTR_EL2 (AArch64 only) */
+FIELD(CPTR_EL2, TZ, 8, 1)  /* !E2H */
+FIELD(CPTR_EL2, TFP, 10, 1)/* !E2H, matches HCPTR.TCP10 */
+FIELD(CPTR_EL2, TSM, 12, 1)/* !E2H */
+FIELD(CPTR_EL2, ZEN, 16, 2)/* E2H */
+FIELD(CPTR_EL2, FPEN, 20, 2)   /* E2H */
+FIELD(CPTR_EL2, SMEN, 24, 2)   /* E2H */
+FIELD(CPTR_EL2, TTA, 28, 1)
+FIELD(CPTR_EL2, TAM, 30, 1)/* matches HCPTR.TAM */
+FIELD(CPTR_EL2, TCPAC, 31, 1)  /* matches HCPTR.TCPAC */
+
+/* Bit definitions for CPTR_EL3 (AArch64 only) */
+FIELD(CPTR_EL3, EZ, 8, 1)
+FIELD(CPTR_EL3, TFP, 10, 1)
+FIELD(CPTR_EL3, ESM, 12, 1)
+FIELD(CPTR_EL3, TTA, 20, 1)
+FIELD(CPTR_EL3, TAM, 30, 1)
+FIELD(CPTR_EL3, TCPAC, 31, 1)
 
 #define MDCR_EPMAD(1U << 21)
 #define MDCR_EDAD (1U << 20)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index a47f38dfc9..a8de33fd64 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -761,7 +761,7 @@ static void do_cpu_reset(void *opaque)
 env->cp15.scr_el3 |= SCR_ATA;
 }
 if (cpu_isar_feature(aa64_sve, cpu)) {
-env->cp15.cptr_el[3] |= CPTR_EZ;
+env->cp15.cptr_el[3] |= R_CPTR_EL3_EZ_MASK;
 }
 /* AArch64 kernels never boot in secure mode */
 assert(!info->secure_boot);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 029f644768..d2bd74c2ed 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -201,9 +201,11 @@ static void arm_cpu_reset(DeviceState *dev)
 /* Trap on btype=3 for PACIxSP. */
 env->cp15.sctlr_el[1] |= SCTLR_BT0;
 /* and to the FP/Neon instructions */
-env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);
+env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
+ CPACR_EL1, FPEN, 3);
 /* and to the SVE instructions */
-env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);
+env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
+ CPACR_EL1, ZEN, 3);
 /* with reasonable vector length */
 if (cpu_isar_feature(aa64_sve, cpu)) {
 env->vfp.zcr_el[1] =
@@ -252,7 +254,10 @@ static void arm_cpu_reset(DeviceState *dev)
 } else {
 #if defined(CONFIG_USER_ONLY)
 /* Userspace expects access to cp10 and cp11 for FP/Neon */
-env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 4, 0xf);
+env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
+ CPACR, CP10, 3);
+env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
+ CPACR, CP11, 3);
 #endif
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 93ab552346..5fd64b742a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -767,11 +767,14 @@ static void cpacr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
  */
 if (cpu_isar_feature(aa32_vfp_simd, env_archcpu(env))) {
 /* VFP coprocessor: cp10 & cp11 [23:20] */
-mask |= (1 << 31) | (1 << 30) | (0xf << 20);
+mask |= R_CPACR_ASEDIS_MASK |
+R_CPACR_D32DIS_MASK |
+R_CPACR_CP11_MASK |
+

[PATCH 0/2] target/arm: SME prep patches

[Richard Henderson]

Add HCRX_EL2 with no supported bits, and bit definitions for CPACR*.
Just trying to keep the queue smaller.


r~


Richard Henderson (2):
  target/arm: Enable FEAT_HCX for -cpu max
  target/arm: Use FIELD definitions for CPACR, CPTR_ELx

 target/arm/cpu.h|  64 ---
 hw/arm/boot.c   |   2 +-
 target/arm/cpu.c|  11 +++--
 target/arm/cpu64.c  |   1 +
 target/arm/helper.c | 104 
 5 files changed, 146 insertions(+), 36 deletions(-)

-- 
2.34.1

[PATCH 1/2] target/arm: Enable FEAT_HCX for -cpu max

[Richard Henderson]

This feature adds a new register, HCRX_EL2, which controls
many of the newer AArch64 features.  So far the register is
effectively RES0, because none of the new features are done.

Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h| 20 ++
 target/arm/cpu64.c  |  1 +
 target/arm/helper.c | 50 +
 3 files changed, 71 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 18ca61e8e2..b35b117fe7 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -362,6 +362,7 @@ typedef struct CPUArchState {
 uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
 uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
 uint64_t hcr_el2; /* Hypervisor configuration register */
+uint64_t hcrx_el2; /* Extended Hypervisor configuration register */
 uint64_t scr_el3; /* Secure configuration register.  */
 union { /* Fault status registers.  */
 struct {
@@ -1543,6 +1544,19 @@ static inline void xpsr_write(CPUARMState *env, uint32_t 
val, uint32_t mask)
 #define HCR_TWEDEN(1ULL << 59)
 #define HCR_TWEDELMAKE_64BIT_MASK(60, 4)
 
+#define HCRX_ENAS0(1ULL << 0)
+#define HCRX_ENALS(1ULL << 1)
+#define HCRX_ENASR(1ULL << 2)
+#define HCRX_FNXS (1ULL << 3)
+#define HCRX_FGTNXS   (1ULL << 4)
+#define HCRX_SMPME(1ULL << 5)
+#define HCRX_TALLINT  (1ULL << 6)
+#define HCRX_VINMI(1ULL << 7)
+#define HCRX_VFNMI(1ULL << 8)
+#define HCRX_CMOW (1ULL << 9)
+#define HCRX_MCE2 (1ULL << 10)
+#define HCRX_MSCEN(1ULL << 11)
+
 #define HPFAR_NS  (1ULL << 63)
 
 #define SCR_NS(1U << 0)
@@ -2310,6 +2324,7 @@ static inline bool arm_is_el2_enabled(CPUARMState *env)
  * Not included here is HCR_RW.
  */
 uint64_t arm_hcr_el2_eff(CPUARMState *env);
+uint64_t arm_hcrx_el2_eff(CPUARMState *env);
 
 /* Return true if the specified exception level is running in AArch64 state. */
 static inline bool arm_el_is_aa64(CPUARMState *env, int el)
@@ -3931,6 +3946,11 @@ static inline bool isar_feature_aa64_ats1e1(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, PAN) >= 2;
 }
 
+static inline bool isar_feature_aa64_hcx(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, HCX) != 0;
+}
+
 static inline bool isar_feature_aa64_uao(const ARMISARegisters *id)
 {
 return FIELD_EX64(id->id_aa64mmfr2, ID_AA64MMFR2, UAO) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 04427e073f..4ab1dcf2ef 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -910,6 +910,7 @@ static void aarch64_max_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);   /* FEAT_LOR */
 t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2);  /* FEAT_PAN2 */
 t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1);  /* FEAT_XNX */
+t = FIELD_DP64(t, ID_AA64MMFR1, HCX, 1);  /* FEAT_HCX */
 cpu->isar.id_aa64mmfr1 = t;
 
 t = cpu->isar.id_aa64mmfr2;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 432bd81919..93ab552346 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5278,6 +5278,52 @@ uint64_t arm_hcr_el2_eff(CPUARMState *env)
 return ret;
 }
 
+static void hcrx_write(CPUARMState *env, const ARMCPRegInfo *ri,
+   uint64_t value)
+{
+uint64_t valid_mask = 0;
+
+/* No features adding bits to HCRX are implemented. */
+
+/* Clear RES0 bits.  */
+env->cp15.hcrx_el2 = value & valid_mask;
+}
+
+static CPAccessResult access_hxen(CPUARMState *env, const ARMCPRegInfo *ri,
+  bool isread)
+{
+if (arm_current_el(env) < 3
+&& arm_feature(env, ARM_FEATURE_EL3)
+&& !(env->cp15.scr_el3 & SCR_HXEN)) {
+return CP_ACCESS_TRAP_EL3;
+}
+return CP_ACCESS_OK;
+}
+
+static const ARMCPRegInfo hcrx_el2_reginfo = {
+.name = "HCRX_EL2", .state = ARM_CP_STATE_AA64,
+.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 2,
+.access = PL2_RW, .writefn = hcrx_write, .accessfn = access_hxen,
+.fieldoffset = offsetof(CPUARMState, cp15.hcrx_el2),
+};
+
+/* Return the effective value of HCRX_EL2.  */
+uint64_t arm_hcrx_el2_eff(CPUARMState *env)
+{
+/*
+ * The bits in this register behave as 0 for all purposes other than
+ * direct reads of the register if:
+ *   - EL2 is not enabled in the current security state,
+ *   - SCR_EL3.HXEn is 0.
+ */
+if (!arm_is_el2_enabled(env)
+|| (arm_feature(env, ARM_FEATURE_EL3)
+&& !(env->cp15.scr_el3 & SCR_HXEN))) {
+return 0;
+}
+return env->cp15.hcrx_el2;
+}
+
 static void cptr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
 {
@@ -8384,6 +8430,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 define_arm_cp_regs(cpu, zcr_reginfo);
 }
 
+if (cpu_isar_feature(aa64_hcx, cpu)) {
+define_one_arm_cp

RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows

[Shi, Guohuai]

> -Original Message-
> From: Greg Kurz 
> Sent: 2022年5月10日 0:20
> To: Shi, Guohuai 
> Cc: Bin Meng ; Christian Schoenebeck 
> ;
> qemu-devel@nongnu.org; Meng, Bin 
> Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for
> Windows
> 
> [Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> On Mon, 9 May 2022 15:09:46 +
> "Shi, Guohuai"  wrote:
> 
> >
> >
> > > -Original Message-
> > > From: Greg Kurz 
> > > Sent: 2022年5月9日 22:29
> > > To: Bin Meng 
> > > Cc: Christian Schoenebeck ; qemu-devel@nongnu.org;
> Shi,
> > > Guohuai ; Meng, Bin 
> > > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver
> for
> > > Windows
> > >
> > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > >
> > > On Mon, 25 Apr 2022 22:27:01 +0800
> > > Bin Meng  wrote:
> > >
> > > > From: Guohuai Shi 
> > > >
> > > > Add a 9p local file system backend driver to support Windows,
> > > > including open, read, write, close, rename, remove, etc.
> > > >
> > > > All security models are supported. The mapped (mapped-xattr)
> > > > security model is implemented using NTFS Alternate Data Stream
> > > > (ADS) so the 9p export path shall be on an NTFS partition.
> > > >
> > > > Signed-off-by: Guohuai Shi 
> > > > Signed-off-by: Bin Meng 
> > > > ---
> > > >
> > >
> > > Hi !
> > >
> > > I tend to agree with Christian's remarks that this patch is too big
> > > and that the choice of introducing right away a new implementation
> > > of 9p-local for windows hosts is too bold to start with. We need to
> > > clearly understand what's diverging between windows and linux in order
> > > to make such a decision. You should first try to introduce the required
> > > abstractions to cope with these differences, so that we can review.
> > >
> >
> > Here is the basic introductions of 9PFS for Windows development:
> >
> > Windows always returns -1 when try to call open() for a directory.
> > Windows (actually MinGW library) only allows opendir() for a directory.
> 
> Does MinGW have dirfd() ?

No.
MinGW does not open any directory.
Here is opendir() source code of MinGW:
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-crt/misc/dirent.c#L42

So MinGW do not have a fd associated to a directory.


> 
> > Windows does not support APIs like "*at" (openat(), renameat(), etc.)
> 
> Ouch...
> 
> > So 9PFS can not use any openat() for opening a sub file or directory in 9P 
> > mount
> directory.
> > This commit use merge_fs_path() to build up full filename by string 
> > concatenation.
> > I know that may have a risk of security, but Windows does fully support 
> > POSIX
> APIs.
> >
> 
> s/does/doesn't ?
> 
> I understand from your various answers that symlinks aren't
> currently supported by window's POSIX API. Is this forever ?
> Google do mentions symlinks in windows 10. What's the story
> there ? How do they behave ? How would they be exposed to the
> client ? Be aware that, even if the client cannot create symlinks,
> an existing symlink could be used to escape with rename().
> 
> If the code "may have a risk of security" then it must be
> fixed or avoided in some way before being merged upstream.
> 
> Other thing that comes to mind is that windows hosts should
> maybe use the mapped or mapped-file security modes since
> they emulate symlinks with a simple file hidden in the
> VIRTFS_META_DIR directory.
> 
> Cheers,
> 
> --
> Greg
> 

Windows native API support symbolic link file start from Windows Vista:
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createsymboliclinka

I mean Windows POSIX APIs do not support symbolic link (MinGW use Win32 POSIX 
APIs)
So we can not create symbolic link by MinGW.

Anyway, there is another solution: re-work whole 9PFS code: not only 
9p-local.c, but also every file in 9p driver.
Replace every MinGW/POSIX APIs (e.g. open, lseek, read, write, close),
by Windows Native APIs (e.g. open -> CreateFile, lseek -> SetFilePointer, read 
-> ReadFile, write -> WriteFile, close -> CloseHandle, etc.)
Then 9P can use Windows symbolic link feature.
However, I do think it is a good idea to replace everything.

Thanks
Guohuai

> > > Some inlined remarks below anyway.
> > >
> > > >  hw/9pfs/9p-linux-errno.h |  151 +
> > > >  hw/9pfs/9p-local.h   |4 +
> > > >  hw/9pfs/9p-util.h|   41 ++
> > > >  hw/9pfs/9p.h |   23 +
> > > >  hw/9pfs/9p-local-win32.c | 1242 ++
> > > >  hw/9pfs/9p-util-win32.c  |  303 ++
> > > >  hw/9pfs/9p-xattr.c   |  113 
> > > >  hw/9pfs/9p.c |   91 ++-
> > > >  hw/9pfs/codir.c  |   15 +
> > > >  9 files changed, 1982 insertions(+), 1 deletion(-)
> > > >  create mode 100644 hw/9pfs/9p-linux-errno.h
> > > >  create mode 100644 hw/9pfs/9p-local-win32.c
> > > >  create mode 100644 hw/9pfs/9p-util-win32.c
> > > >
> > > > diff --git a/hw/9pfs/9p-linux-errno.h b/hw/9pfs/9p-linux-errno.h
> > > > new file mo

答复: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

[Gao, Lu]

Ping

https://patchew.org/QEMU/20220321055618.4026-1-lu@verisilicon.com/

Please help review the patch.
Thanks.
B.R.

-邮件原件-
发件人: Gao, Lu 
发送时间: Monday, April 25, 2022 9:35 AM
收件人: Gao, Lu; qemu-devel@nongnu.org
抄送: Wen, Jianxian; Philippe Mathieu-Daudé; Bin Meng; open list:SD (Secure Card)
主题: 答复: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

ping

https://patchew.org/QEMU/20220321055618.4026-1-lu@verisilicon.com/

Please help review the patch.
Thanks.
B.R.

-邮件原件-
发件人: Gao, Lu 
发送时间: Monday, March 21, 2022 1:56 PM
收件人: qemu-devel@nongnu.org
抄送: Gao, Lu; Wen, Jianxian; Philippe Mathieu-Daudé; Bin Meng; open list:SD 
(Secure Card)
主题: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

Block Size Register bits [14:12] is SDMA Buffer Boundary, it is missed
in register write, but it is needed in SDMA transfer. e.g. it will be
used in sdhci_sdma_transfer_multi_blocks to calculate boundary_ variables.

Missing this field will cause wrong operation for different SDMA Buffer
Boundary settings.

Signed-off-by: Lu Gao 
Signed-off-by: Jianxian Wen 
---
 hw/sd/sdhci.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e0bbc90344..350ceb487d 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -321,6 +321,8 @@ static void sdhci_poweron_reset(DeviceState *dev)
 
 static void sdhci_data_transfer(void *opaque);
 
+#define BLOCK_SIZE_MASK (4 * KiB - 1)
+
 static void sdhci_send_command(SDHCIState *s)
 {
 SDRequest request;
@@ -371,7 +373,8 @@ static void sdhci_send_command(SDHCIState *s)
 
 sdhci_update_irq(s);
 
-if (!timeout && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
+if (!timeout && (s->blksize & BLOCK_SIZE_MASK) &&
+(s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
 s->data_count = 0;
 sdhci_data_transfer(s);
 }
@@ -406,7 +409,6 @@ static void sdhci_end_transfer(SDHCIState *s)
 /*
  * Programmed i/o data transfer
  */
-#define BLOCK_SIZE_MASK (4 * KiB - 1)
 
 /* Fill host controller's read buffer with BLKSIZE bytes of data from card */
 static void sdhci_read_block_from_card(SDHCIState *s)
@@ -1137,7 +1139,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 s->sdmasysad = (s->sdmasysad & mask) | value;
 MASKED_WRITE(s->sdmasysad, mask, value);
 /* Writing to last byte of sdmasysad might trigger transfer */
-if (!(mask & 0xFF00) && s->blkcnt && s->blksize &&
+if (!(mask & 0xFF00) && s->blkcnt &&
+(s->blksize & BLOCK_SIZE_MASK) &&
 SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA) {
 if (s->trnmod & SDHC_TRNS_MULTI) {
 sdhci_sdma_transfer_multi_blocks(s);
@@ -1151,7 +1154,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 if (!TRANSFERRING_DATA(s->prnsts)) {
 uint16_t blksize = s->blksize;
 
-MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12));
+/*
+ * [14:12] SDMA Buffer Boundary
+ * [11:00] Transfer Block Size
+ */
+MASKED_WRITE(s->blksize, mask, extract32(value, 0, 15));
 MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
 
 /* Limit block size to the maximum buffer size */
-- 
2.17.1

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[maobibo]

在 2022/5/10 02:25, Richard Henderson 写道:
> On 5/9/22 13:04, Peter Maydell wrote:
>> On Mon, 9 May 2022 at 18:56, Richard Henderson
>>  wrote:
>>> I'm not 100% sure how this "Other configuration control register" should be 
>>> handled, but
>>> definitely not like this.
>>>
>>> I see you're putting control of this register into loongarch_qemu_read in
>>> target/loongarch/cpu.c.  Which, I suppose is fair, because this is 
>>> documented as part of
>>> the 3A5000 cpu documentation.  But then you split out all of the devices 
>>> which are *also*
>>> documented as part of the cpu into the board configuration.
>>>
>>> This reminds me of the memory-mapped interface that the armv7m cpu has with 
>>> its own
>>> registers.  I believe that you need to model this similarly, where you will 
>>> have a device
>>> that represents the cpu, and then instantiates all of the devices that are 
>>> listed in the
>>> Loongson 3A5000 TRM -- call this ls3a to match the ls7a name you have for 
>>> the 7A1000
>>> bridge device.
>>>
>>> When there is a write to the ls3a "Other function configuration register", 
>>> the ls3a will
>>> need to communicate the changes to the various bits to its various 
>>> sub-devices.  I do not
>>> think it unreasonable to have direct function calls between the components.
>>>
>>> Peter, do you have any advice from the armv7m side?
>>
>> Nothing concrete. I'm not sure that we'd structure the armv7m stuff the way
>> we have now if we were writing it from scratch, but it's functional enough.
>> (In particular, if MMIO regions were part of Device rather than SysBusDevice
>> then I'd be tempted to suggest that CPUs with MMIO-mapped registers should
>> directly own their MemoryRegions for them. But they aren't, so we can't do
>> that.)
> 
> Having thought about this a little more, I believe that these registers are 
> part of the "cpu package", because they are shared between the 4 cpu cores 
> within the package.  Thus their current placement attached to LoongArchCPU -- 
> as well as the current placement of address_space_iocsr -- is incorrect.

The extioi hardware design is not friend to software developer, local cpu INTC
is mixed with board INTC with extioi/iocsr. Local cpu INTC registers should be 
banked,
address space is space for local cpu review point.

how about put address_space_iocsr as board rather than percpu since there is no 
concept
of "cpu package".

regards
bibo, mao
> 
> 
> r~

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[maobibo]

在 2022/5/10 10:54, maobibo 写道:
> 
> 
> 在 2022/5/10 02:25, Richard Henderson 写道:
>> On 5/9/22 13:04, Peter Maydell wrote:
>>> On Mon, 9 May 2022 at 18:56, Richard Henderson
>>>  wrote:
 I'm not 100% sure how this "Other configuration control register" should 
 be handled, but
 definitely not like this.

 I see you're putting control of this register into loongarch_qemu_read in
 target/loongarch/cpu.c.  Which, I suppose is fair, because this is 
 documented as part of
 the 3A5000 cpu documentation.  But then you split out all of the devices 
 which are *also*
 documented as part of the cpu into the board configuration.

 This reminds me of the memory-mapped interface that the armv7m cpu has 
 with its own
 registers.  I believe that you need to model this similarly, where you 
 will have a device
 that represents the cpu, and then instantiates all of the devices that are 
 listed in the
 Loongson 3A5000 TRM -- call this ls3a to match the ls7a name you have for 
 the 7A1000
 bridge device.

 When there is a write to the ls3a "Other function configuration register", 
 the ls3a will
 need to communicate the changes to the various bits to its various 
 sub-devices.  I do not
 think it unreasonable to have direct function calls between the components.

 Peter, do you have any advice from the armv7m side?
>>>
>>> Nothing concrete. I'm not sure that we'd structure the armv7m stuff the way
>>> we have now if we were writing it from scratch, but it's functional enough.
>>> (In particular, if MMIO regions were part of Device rather than SysBusDevice
>>> then I'd be tempted to suggest that CPUs with MMIO-mapped registers should
>>> directly own their MemoryRegions for them. But they aren't, so we can't do
>>> that.)
>>
>> Having thought about this a little more, I believe that these registers are 
>> part of the "cpu package", because they are shared between the 4 cpu cores 
>> within the package.  Thus their current placement attached to LoongArchCPU 
>> -- as well as the current placement of address_space_iocsr -- is incorrect.
> 
> The extioi hardware design is not friend to software developer, local cpu INTC
> is mixed with board INTC with extioi/iocsr. Local cpu INTC registers should 
> be banked,
> address space is space for local cpu review point.
address space of local cpu INTC should be the same from cpu viewpoint.

> 
> how about put address_space_iocsr as board rather than percpu since there is 
> no concept
> of "cpu package".
> 
> regards
> bibo, mao
>>
>>
>> r~
> 

Re: [RFC 00/18] vfio: Adopt iommufd

[Yi Liu]

Hi Zhangfei,

On 2022/5/9 22:24, Zhangfei Gao wrote:

Hi, Alex

On 2022/4/27 上午12:35, Alex Williamson wrote:

On Tue, 26 Apr 2022 12:43:35 +
Shameerali Kolothum Thodi  wrote:


-Original Message-
From: Eric Auger [mailto:eric.au...@redhat.com]
Sent: 26 April 2022 12:45
To: Shameerali Kolothum Thodi ; Yi
Liu ; alex.william...@redhat.com; coh...@redhat.com;
qemu-devel@nongnu.org
Cc: da...@gibson.dropbear.id.au; th...@redhat.com; far...@linux.ibm.com;
mjros...@linux.ibm.com; akrow...@linux.ibm.com; pa...@linux.ibm.com;
jjhe...@linux.ibm.com; jasow...@redhat.com; k...@vger.kernel.org;
j...@nvidia.com; nicol...@nvidia.com; eric.auger@gmail.com;
kevin.t...@intel.com; chao.p.p...@intel.com; yi.y@intel.com;
pet...@redhat.com; Zhangfei Gao 
Subject: Re: [RFC 00/18] vfio: Adopt iommufd

[...]

https://lore.kernel.org/kvm/0-v1-e79cd8d168e8+6-iommufd_...@nvidia.com

/
[2] https://github.com/luxis1999/iommufd/tree/iommufd-v5.17-rc6
[3] https://github.com/luxis1999/qemu/tree/qemu-for-5.17-rc6-vm-rfcv1

Hi,

I had a go with the above branches on our ARM64 platform trying to

pass-through

a VF dev, but Qemu reports an error as below,

[    0.444728] hisi_sec2 :00:01.0: enabling device ( -> 0002)
qemu-system-aarch64-iommufd: IOMMU_IOAS_MAP failed: Bad address
qemu-system-aarch64-iommufd: vfio_container_dma_map(0xfeb40ce0,

0x80, 0x1, 0xb40ef000) = -14 (Bad address)

I think this happens for the dev BAR addr range. I haven't debugged the

kernel

yet to see where it actually reports that.

Does it prevent your assigned device from working? I have such errors
too but this is a known issue. This is due to the fact P2P DMA is not
supported yet.
Yes, the basic tests all good so far. I am still not very clear how it 
works if

the map() fails though. It looks like it fails in,

iommufd_ioas_map()
   iopt_map_user_pages()
    iopt_map_pages()
    ..
  pfn_reader_pin_pages()

So does it mean it just works because the page is resident()?

No, it just means that you're not triggering any accesses that require
peer-to-peer DMA support.  Any sort of test where the device is only
performing DMA to guest RAM, which is by far the standard use case,
will work fine.  This also doesn't affect vCPU access to BAR space.
It's only a failure of the mappings of the BAR space into the IOAS,
which is only used when a device tries to directly target another
device's BAR space via DMA.  Thanks,


I also get this issue when trying adding prereg listenner

+    container->prereg_listener = vfio_memory_prereg_listener;
+    memory_listener_register(&container->prereg_listener,
+    &address_space_memory);

host kernel log:
iommufd_ioas_map 1 iova=80, iova1=80, cmd->iova=80, 
cmd->user_va=9c495000, cmd->length=1

iopt_alloc_area input area=859a2d00 iova=80
iopt_alloc_area area=859a2d00 iova=80
pin_user_pages_remote rc=-14

qemu log:
vfio_prereg_listener_region_add
iommufd_map iova=0x80
qemu-system-aarch64: IOMMU_IOAS_MAP failed: Bad address
qemu-system-aarch64: vfio_dma_map(0xfb96a930, 0x80, 0x1, 
0x9c495000) = -14 (Bad address)

qemu-system-aarch64: (null)
double free or corruption (fasttop)
Aborted (core dumped)

With hack of ignoring address 0x80 in map and unmap, kernel can boot.


do you know if the iova 0x80 guest RAM or MMIO? Currently, iommufd 
kernel part doesn't support mapping device BAR MMIO. This is a known gap.


--
Regards,
Yi Liu

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[Richard Henderson]

On 5/9/22 19:54, maobibo wrote:

how about put address_space_iocsr as board rather than percpu since there is no 
concept
of "cpu package".


"cpu package" works ok as a device on the board.

I don't know if it's possible to have the iocsr address space controlled by the device, 
especially since it appears that the iocsr address space is *also* available -- or at 
least partially available -- in the main address space at base 0x1fe0?



r~

Re: [PATCH v4 6/7] vhost-vdpa: change name and polarity for vhost_vdpa_one_time_request()

[Jason Wang]

On Sat, May 7, 2022 at 10:28 AM Si-Wei Liu  wrote:
>
> The name vhost_vdpa_one_time_request() was confusing. No
> matter whatever it returns, its typical occurrence had
> always been at requests that only need to be applied once.
> And the name didn't suggest what it actually checks for.
> Change it to vhost_vdpa_first_dev() with polarity flipped
> for better readibility of code. That way it is able to
> reflect what the check is really about.
>
> This call is applicable to request which performs operation
> only once, before queues are set up, and usually at the beginning
> of the caller function. Document the requirement for it in place.
>
> Signed-off-by: Si-Wei Liu 

Acked-by: Jason Wang 

> ---
>  hw/virtio/vhost-vdpa.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 6e3dbd9..33dcaa1 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -366,11 +366,18 @@ static void vhost_vdpa_get_iova_range(struct vhost_vdpa 
> *v)
>  v->iova_range.last);
>  }
>
> -static bool vhost_vdpa_one_time_request(struct vhost_dev *dev)
> +/*
> + * The use of this function is for requests that only need to be
> + * applied once. Typically such request occurs at the beginning
> + * of operation, and before setting up queues. It should not be
> + * used for request that performs operation until all queues are
> + * set, which would need to check dev->vq_index_end instead.
> + */
> +static bool vhost_vdpa_first_dev(struct vhost_dev *dev)
>  {
>  struct vhost_vdpa *v = dev->opaque;
>
> -return v->index != 0;
> +return v->index == 0;
>  }
>
>  static int vhost_vdpa_get_dev_features(struct vhost_dev *dev,
> @@ -451,7 +458,7 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void 
> *opaque, Error **errp)
>
>  vhost_vdpa_get_iova_range(v);
>
> -if (vhost_vdpa_one_time_request(dev)) {
> +if (!vhost_vdpa_first_dev(dev)) {
>  return 0;
>  }
>
> @@ -594,7 +601,7 @@ static int vhost_vdpa_memslots_limit(struct vhost_dev 
> *dev)
>  static int vhost_vdpa_set_mem_table(struct vhost_dev *dev,
>  struct vhost_memory *mem)
>  {
> -if (vhost_vdpa_one_time_request(dev)) {
> +if (!vhost_vdpa_first_dev(dev)) {
>  return 0;
>  }
>
> @@ -623,7 +630,7 @@ static int vhost_vdpa_set_features(struct vhost_dev *dev,
>  struct vhost_vdpa *v = dev->opaque;
>  int ret;
>
> -if (vhost_vdpa_one_time_request(dev)) {
> +if (!vhost_vdpa_first_dev(dev)) {
>  return 0;
>  }
>
> @@ -665,7 +672,7 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev 
> *dev)
>
>  features &= f;
>
> -if (!vhost_vdpa_one_time_request(dev)) {
> +if (vhost_vdpa_first_dev(dev)) {
>  r = vhost_vdpa_call(dev, VHOST_SET_BACKEND_FEATURES, &features);
>  if (r) {
>  return -EFAULT;
> @@ -1118,7 +1125,7 @@ static int vhost_vdpa_set_log_base(struct vhost_dev 
> *dev, uint64_t base,
>   struct vhost_log *log)
>  {
>  struct vhost_vdpa *v = dev->opaque;
> -if (v->shadow_vqs_enabled || vhost_vdpa_one_time_request(dev)) {
> +if (v->shadow_vqs_enabled || !vhost_vdpa_first_dev(dev)) {
>  return 0;
>  }
>
> @@ -1240,7 +1247,7 @@ static int vhost_vdpa_get_features(struct vhost_dev 
> *dev,
>
>  static int vhost_vdpa_set_owner(struct vhost_dev *dev)
>  {
> -if (vhost_vdpa_one_time_request(dev)) {
> +if (!vhost_vdpa_first_dev(dev)) {
>  return 0;
>  }
>
> --
> 1.8.3.1
>

[PATCH 1/2] qapi/expr: Enforce feature naming rules again

[Markus Armbruster]

Commit e42648dccd "qapi/expr.py: Remove single-letter variable"
accidentally removed the check for "only lower case letters and
hyphens".  Restore it.

Fixes: e42648dccdd1defe8f35f247966cd7283f865cd6
Signed-off-by: Markus Armbruster 
---
 scripts/qapi/expr.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 48578e1698..5a1782b57e 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -443,7 +443,7 @@ def check_features(features: Optional[object],
 check_keys(feat, info, source, ['name'], ['if'])
 check_name_is_str(feat['name'], info, source)
 source = "%s '%s'" % (source, feat['name'])
-check_name_str(feat['name'], info, source)
+check_name_lower(feat['name'], info, source)
 check_if(feat, info, source)
 
 
-- 
2.35.1

[PATCH 2/2] docs/devel/qapi-code-gen: Belatedly document feature naming rules

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 docs/devel/qapi-code-gen.rst | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 7b968433a6..cd9b544376 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -739,10 +739,11 @@ Type names ending with ``Kind`` or ``List`` are reserved 
for the
 generator, which uses them for implicit union enums and array types,
 respectively.
 
-Command names, and member names within a type, should be all lower
-case with words separated by a hyphen.  However, some existing older
-commands and complex types use underscore; when extending them,
-consistency is preferred over blindly avoiding underscore.
+Command names, member names within a type, and feature names should be
+all lower case with words separated by a hyphen.  However, some
+existing older commands and complex types use underscore; when
+extending them, consistency is preferred over blindly avoiding
+underscore.
 
 Event names should be ALL_CAPS with words separated by underscore.
 
-- 
2.35.1

[PATCH 0/2] qapi: Minor fixes around feature names

[Markus Armbruster]

Markus Armbruster (2):
  qapi/expr: Enforce feature naming rules again
  docs/devel/qapi-code-gen: Belatedly document feature naming rules

 docs/devel/qapi-code-gen.rst | 9 +
 scripts/qapi/expr.py | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.35.1

Re: [PATCH][RESEND] MAINTAINERS: Add myself as a reviewer for Hyper-V VMBus

[Thomas Huth]

On 09/05/2022 19.59, Maciej S. Szmigiero wrote:

On 6.05.2022 07:31, Thomas Huth wrote:

On 05/05/2022 21.36, Maciej S. Szmigiero wrote:

From: "Maciej S. Szmigiero" 

This way there is at least some contact point for incoming patches.

We'll see whether the code still gets just a random patch a few times
a year or whether it requires a permanent maintainer to take care of it.

Signed-off-by: Maciej S. Szmigiero 
---
 Resending, since the previous submission at [1] wasn't picked up.
 [1]: 
https://lore.kernel.org/qemu-devel/b145dcf08ae606e9d29e55b2f701a3fe4f16b347.1637433881.git.maciej.szmigi...@oracle.com/ 



  MAINTAINERS | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 218c9459b6..907f1d4a88 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1776,6 +1776,12 @@ F: include/hw/block/fdc.h
  F: tests/qtest/fdc-test.c
  T: git https://gitlab.com/jsnow/qemu.git ide
+Hyper-V VMBus
+S: Odd Fixes
+R: Maciej S. Szmigiero 
+F: hw/hyperv/vmbus.c
+F: include/hw/hyperv/vmbus*.h


If there's no dedicated maintainer, I'd rather pick "Orphan" instead of 
"Odd Fixes" here?

Who's supposed to pick up the related patches?


I think I can do this, as long as I know who to send pull requests to
and on what schedule.


Pull requests are sent via mail to the qemu-devel mailing list, see 
https://www.qemu.org/docs/master/devel/submitting-a-pull-request.html for 
some details. During the normal development phase, pull requests can be sent 
as necessary for new patches. Just during the freeze period (the next one 
will be announced on https://wiki.qemu.org/Planning/7.1 later), only bug 
fixes (no new feature) should be sent.


 Thomas

Re: [RFC 00/18] vfio: Adopt iommufd

[Eric Auger]

Hi Hi, Zhangfei,

On 5/10/22 05:17, Yi Liu wrote:
> Hi Zhangfei,
>
> On 2022/5/9 22:24, Zhangfei Gao wrote:
>> Hi, Alex
>>
>> On 2022/4/27 上午12:35, Alex Williamson wrote:
>>> On Tue, 26 Apr 2022 12:43:35 +
>>> Shameerali Kolothum Thodi  wrote:
>>>
> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: 26 April 2022 12:45
> To: Shameerali Kolothum Thodi
> ; Yi
> Liu ; alex.william...@redhat.com;
> coh...@redhat.com;
> qemu-devel@nongnu.org
> Cc: da...@gibson.dropbear.id.au; th...@redhat.com;
> far...@linux.ibm.com;
> mjros...@linux.ibm.com; akrow...@linux.ibm.com; pa...@linux.ibm.com;
> jjhe...@linux.ibm.com; jasow...@redhat.com; k...@vger.kernel.org;
> j...@nvidia.com; nicol...@nvidia.com; eric.auger@gmail.com;
> kevin.t...@intel.com; chao.p.p...@intel.com; yi.y@intel.com;
> pet...@redhat.com; Zhangfei Gao 
> Subject: Re: [RFC 00/18] vfio: Adopt iommufd
 [...]
> https://lore.kernel.org/kvm/0-v1-e79cd8d168e8+6-iommufd_...@nvidia.com
>
>>> /
>>> [2] https://github.com/luxis1999/iommufd/tree/iommufd-v5.17-rc6
>>> [3]
>>> https://github.com/luxis1999/qemu/tree/qemu-for-5.17-rc6-vm-rfcv1
>> Hi,
>>
>> I had a go with the above branches on our ARM64 platform trying to
> pass-through
>> a VF dev, but Qemu reports an error as below,
>>
>> [    0.444728] hisi_sec2 :00:01.0: enabling device ( ->
>> 0002)
>> qemu-system-aarch64-iommufd: IOMMU_IOAS_MAP failed: Bad address
>> qemu-system-aarch64-iommufd: vfio_container_dma_map(0xfeb40ce0,
> 0x80, 0x1, 0xb40ef000) = -14 (Bad address)
>> I think this happens for the dev BAR addr range. I haven't
>> debugged the
> kernel
>> yet to see where it actually reports that.
> Does it prevent your assigned device from working? I have such errors
> too but this is a known issue. This is due to the fact P2P DMA is not
> supported yet.
 Yes, the basic tests all good so far. I am still not very clear how
 it works if
 the map() fails though. It looks like it fails in,

 iommufd_ioas_map()
    iopt_map_user_pages()
     iopt_map_pages()
     ..
   pfn_reader_pin_pages()

 So does it mean it just works because the page is resident()?
>>> No, it just means that you're not triggering any accesses that require
>>> peer-to-peer DMA support.  Any sort of test where the device is only
>>> performing DMA to guest RAM, which is by far the standard use case,
>>> will work fine.  This also doesn't affect vCPU access to BAR space.
>>> It's only a failure of the mappings of the BAR space into the IOAS,
>>> which is only used when a device tries to directly target another
>>> device's BAR space via DMA.  Thanks,
>>
>> I also get this issue when trying adding prereg listenner
>>
>> +    container->prereg_listener = vfio_memory_prereg_listener;
>> +    memory_listener_register(&container->prereg_listener,
>> +    &address_space_memory);
>>
>> host kernel log:
>> iommufd_ioas_map 1 iova=80, iova1=80,
>> cmd->iova=80, cmd->user_va=9c495000, cmd->length=1
>> iopt_alloc_area input area=859a2d00 iova=80
>> iopt_alloc_area area=859a2d00 iova=80
>> pin_user_pages_remote rc=-14
>>
>> qemu log:
>> vfio_prereg_listener_region_add
>> iommufd_map iova=0x80
>> qemu-system-aarch64: IOMMU_IOAS_MAP failed: Bad address
>> qemu-system-aarch64: vfio_dma_map(0xfb96a930, 0x80,
>> 0x1, 0x9c495000) = -14 (Bad address)
>> qemu-system-aarch64: (null)
>> double free or corruption (fasttop)
>> Aborted (core dumped)
>>
>> With hack of ignoring address 0x80 in map and unmap, kernel
>> can boot.
>
> do you know if the iova 0x80 guest RAM or MMIO? Currently,
> iommufd kernel part doesn't support mapping device BAR MMIO. This is a
> known gap.
In qemu arm virt machine this indeed matches the PCI MMIO region.

Thanks

Eric

[PATCH] util: NUMA aware memory preallocation

[Michal Privoznik]

When allocating large amounts of memory the task is offloaded
onto threads. These threads then use various techniques to
allocate the memory fully (madvise(), writing into the memory).
However, these threads are free to run on any CPU, which becomes
problematic on NUMA machines because it may happen that a thread
is running on a distant node.

Ideally, this is something that a management application would
resolve, but we are not anywhere close to that, Firstly, memory
allocation happens before monitor socket is even available. But
okay, that's what -preconfig is for. But then the problem is that
'object-add' would not return until all memory is preallocated.

Long story short, management application has no way of learning
TIDs of allocator threads so it can't make them run NUMA aware.

But what we can do is to propagate the 'host-nodes' attribute of
MemoryBackend object down to where preallocation threads are
created and set their affinity according to the attribute.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2074000
Signed-off-by: Michal Privoznik 
---
 backends/hostmem.c |  6 ++--
 hw/virtio/virtio-mem.c |  2 +-
 include/qemu/osdep.h   |  2 ++
 util/meson.build   |  2 +-
 util/oslib-posix.c | 74 --
 util/oslib-win32.c |  2 ++
 6 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index a7bae3d713..7373472c7e 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -232,7 +232,8 @@ static void host_memory_backend_set_prealloc(Object *obj, 
bool value,
 void *ptr = memory_region_get_ram_ptr(&backend->mr);
 uint64_t sz = memory_region_size(&backend->mr);
 
-os_mem_prealloc(fd, ptr, sz, backend->prealloc_threads, &local_err);
+os_mem_prealloc(fd, ptr, sz, backend->prealloc_threads,
+backend->host_nodes, MAX_NODES, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
@@ -394,7 +395,8 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
Error **errp)
  */
 if (backend->prealloc) {
 os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
-backend->prealloc_threads, &local_err);
+backend->prealloc_threads, backend->host_nodes,
+MAX_NODES, &local_err);
 if (local_err) {
 goto out;
 }
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 5aca408726..48b104cdf6 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -467,7 +467,7 @@ static int virtio_mem_set_block_state(VirtIOMEM *vmem, 
uint64_t start_gpa,
 int fd = memory_region_get_fd(&vmem->memdev->mr);
 Error *local_err = NULL;
 
-os_mem_prealloc(fd, area, size, 1, &local_err);
+os_mem_prealloc(fd, area, size, 1, NULL, 0, &local_err);
 if (local_err) {
 static bool warned;
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1c1e7eca98..474cbf3b86 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -577,6 +577,8 @@ unsigned long qemu_getauxval(unsigned long type);
 void qemu_set_tty_echo(int fd, bool echo);
 
 void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
+ const unsigned long *host_nodes,
+ unsigned long max_node,
  Error **errp);
 
 /**
diff --git a/util/meson.build b/util/meson.build
index 8f16018cd4..393ff74570 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -15,7 +15,7 @@ freebsd_dep = []
 if targetos == 'freebsd'
   freebsd_dep = util
 endif
-util_ss.add(when: 'CONFIG_POSIX', if_true: [files('oslib-posix.c'), 
freebsd_dep])
+util_ss.add(when: 'CONFIG_POSIX', if_true: [files('oslib-posix.c'), 
freebsd_dep, numa])
 util_ss.add(when: 'CONFIG_POSIX', if_true: files('qemu-thread-posix.c'))
 util_ss.add(when: 'CONFIG_POSIX', if_true: files('memfd.c'))
 util_ss.add(when: 'CONFIG_WIN32', if_true: files('aio-win32.c'))
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 477990f39b..1572b9b178 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -73,6 +73,10 @@
 #include "qemu/error-report.h"
 #endif
 
+#ifdef CONFIG_NUMA
+#include 
+#endif
+
 #define MAX_MEM_PREALLOC_THREAD_COUNT 16
 
 struct MemsetThread;
@@ -82,6 +86,9 @@ typedef struct MemsetContext {
 bool any_thread_failed;
 struct MemsetThread *threads;
 int num_threads;
+#ifdef CONFIG_NUMA
+struct bitmask *nodemask;
+#endif
 } MemsetContext;
 
 struct MemsetThread {
@@ -420,6 +427,12 @@ static void *do_touch_pages(void *arg)
 }
 qemu_mutex_unlock(&page_mutex);
 
+#ifdef CONFIG_NUMA
+if (memset_args->context->nodemask) {
+numa_run_on_node_mask(memset_args->context->nodemask);
+}
+#endif
+
 /* unblock SIGBUS */
 sigemptyset(&set);
 sigaddset(&set, 

Re: [PATCH] pc: q35: Bump max_cpus to 512

[Igor Mammedov]

On Wed, 4 May 2022 08:16:39 -0500
Suravee Suthikulpanit  wrote:

> This is the maximum number of vCPU supported by
> the AMD x2APIC virtualization.
> 
> Signed-off-by: Suravee Suthikulpanit 
> ---
>  hw/i386/pc_q35.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 302288342a..e82b1c690d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
> -m->max_cpus = 288;
> +m->max_cpus = 512;

Maybe we should bump it to KVM VCPU maximum,
and make sure we error out if asked for combination of
hardware/irqchip is not usable.


>  }
>  
>  static void pc_q35_7_1_machine_options(MachineClass *m)

[PULL v2 00/11] Misc patches

[Thomas Huth]

The following changes since commit 554623226f800acf48a2ed568900c1c968ec9a8b:

  Merge tag 'qemu-sparc-20220508' of https://github.com/mcayland/qemu into 
staging (2022-05-08 17:03:26 -0500)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2022-05-09

for you to fetch changes up to ddc5a6cc70398ed7ec76220d59c123d8cb14b0ad:

  docs/devel/writing-monitor-commands: Replace obsolete STEXI/ETEXI tags 
(2022-05-09 08:21:14 +0200)


* Remove redundant/obsolete x86, arm and ppc disassemblers (Capstone is better)
* Limit some Xen-related code to builds where Xen is really available
* Remove hxtool-conv.pl and remove STEXI/ETEXI references from the docs
* Update MinGW and OpenBSD to a more recent version in the CI
* Warn user if the -vga flag is passed but no vga device is created



v2:
 - Fixed the -vga warning patch to not warn in case of "-device"
 - Added the ppc disassembler patch
 - Added the STEXI/ETEXI doc patch

Brad Smith (1):
  tests/vm: update openbsd to release 7.1

Gautam Agrawal (1):
  Warn user if the vga flag is passed but no vga device is created

Thomas Huth (6):
  disas: Remove old libopcode arm disassembler
  disas: Remove old libopcode i386 disassembler
  disas: Remove old libopcode ppc disassembler
  softmmu/vl: Fence 'xenfb' if Xen support is not compiled in
  qemu-options: Limit the -xen options to x86 and arm
  docs/devel/writing-monitor-commands: Replace obsolete STEXI/ETEXI tags

Yonggang Luo (3):
  doc: remove hxtool-conv.pl
  cirrus/win32: upgrade mingw base packages
  gitlab-ci: Upgrade mingw base package.

 docs/devel/writing-monitor-commands.rst |   11 +-
 include/disas/dis-asm.h |3 -
 include/sysemu/sysemu.h |1 +
 disas.c |5 -
 disas/arm.c | 4012 --
 disas/i386.c| 6771 ---
 disas/ppc.c | 5435 -
 hw/hppa/machine.c   |1 +
 hw/isa/isa-bus.c|1 +
 hw/mips/fuloong2e.c |1 +
 hw/pci/pci.c|1 +
 hw/ppc/spapr.c  |1 +
 hw/sparc/sun4m.c|2 +
 hw/sparc64/sun4u.c  |1 +
 hw/xenpv/xen_machine_pv.c   |1 +
 softmmu/globals.c   |1 +
 softmmu/vl.c|9 +
 target/arm/cpu.c|8 -
 target/i386/cpu.c   |1 -
 target/ppc/cpu_init.c   |2 -
 .cirrus.yml |2 +-
 .gitlab-ci.d/windows.yml|2 +-
 MAINTAINERS |6 -
 disas/meson.build   |3 -
 qemu-options.hx |7 +-
 scripts/hxtool-conv.pl  |  137 -
 tests/vm/openbsd|4 +-
 27 files changed, 32 insertions(+), 16397 deletions(-)
 delete mode 100644 disas/arm.c
 delete mode 100644 disas/i386.c
 delete mode 100644 disas/ppc.c
 delete mode 100755 scripts/hxtool-conv.pl

[PULL v2 11/11] docs/devel/writing-monitor-commands: Replace obsolete STEXI/ETEXI tags

[Thomas Huth]

STEXI and ETEXI is not used anymore since we switched to Sphinx.
Replace them in the example with SRST and ERST, too.

Message-Id: <20220506150146.564244-1-th...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Thomas Huth 
---
 docs/devel/writing-monitor-commands.rst | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/docs/devel/writing-monitor-commands.rst 
b/docs/devel/writing-monitor-commands.rst
index 1693822f8f..4aa2bb904d 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -331,13 +331,10 @@ we should add it to the hmp-commands.hx file::
 .cmd= hmp_hello_world,
 },
 
-::
-
- STEXI
- @item hello_world @var{message}
- @findex hello_world
- Print message to the standard output
- ETEXI
+ SRST
+ ``hello_world`` *message*
+   Print message to the standard output
+ ERST
 
 To test this you have to open a user monitor and issue the "hello-world"
 command. It might be instructive to check the command's documentation with
-- 
2.27.0

[PULL v2 10/11] Warn user if the vga flag is passed but no vga device is created

[Thomas Huth]

From: Gautam Agrawal 

A global boolean variable "vga_interface_created"(declared in softmmu/globals.c)
has been used to track the creation of vga interface. If the vga flag is passed
in the command line "default_vga"(declared in softmmu/vl.c) variable is set to 
0.
To warn user, the condition checks if vga_interface_created is false
and default_vga is equal to 0. If "-vga none" is passed, this patch will not 
warn the
user regarding the creation of VGA device.

The warning "A -vga option was passed but this
machine type does not use that option; no VGA device has been created"
is logged if vga flag is passed but no vga device is created.

This patch has been tested for x86_64, i386, sparc, sparc64 and arm boards.

Signed-off-by: Gautam Agrawal 
Reviewed-by: Peter Maydell 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/581
Message-Id: <20220501122505.29202-1-gautamnagra...@gmail.com>
[thuth: Fix wrong warning with "-device" in some cases as reported by Paolo]
Signed-off-by: Thomas Huth 
---
 include/sysemu/sysemu.h   | 1 +
 hw/hppa/machine.c | 1 +
 hw/isa/isa-bus.c  | 1 +
 hw/mips/fuloong2e.c   | 1 +
 hw/pci/pci.c  | 1 +
 hw/ppc/spapr.c| 1 +
 hw/sparc/sun4m.c  | 2 ++
 hw/sparc64/sun4u.c| 1 +
 hw/xenpv/xen_machine_pv.c | 1 +
 softmmu/globals.c | 1 +
 softmmu/vl.c  | 7 +++
 11 files changed, 18 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 10e283c170..360a408edf 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -34,6 +34,7 @@ typedef enum {
 } VGAInterfaceType;
 
 extern int vga_interface_type;
+extern bool vga_interface_created;
 
 extern int graphic_width;
 extern int graphic_height;
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index ae0bc07e75..4d054ca869 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -265,6 +265,7 @@ static void machine_hppa_init(MachineState *machine)
 
 /* Graphics setup. */
 if (machine->enable_graphics && vga_interface_type != VGA_NONE) {
+vga_interface_created = true;
 dev = qdev_new("artist");
 s = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(s, &error_fatal);
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 0ad1c5fd65..cd5ad3687d 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -166,6 +166,7 @@ bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, 
Error **errp)
 
 ISADevice *isa_vga_init(ISABus *bus)
 {
+vga_interface_created = true;
 switch (vga_interface_type) {
 case VGA_CIRRUS:
 return isa_create_simple(bus, "isa-cirrus-vga");
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 7b13098f9b..5ee546f5f6 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -320,6 +320,7 @@ static void mips_fuloong2e_init(MachineState *machine)
 
 /* GPU */
 if (vga_interface_type != VGA_NONE) {
+vga_interface_created = true;
 pci_dev = pci_new(-1, "ati-vga");
 dev = DEVICE(pci_dev);
 qdev_prop_set_uint32(dev, "vgamem_mb", 16);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e99417e501..9c58f02853 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2037,6 +2037,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
*rootbus,
 
 PCIDevice *pci_vga_init(PCIBus *bus)
 {
+vga_interface_created = true;
 switch (vga_interface_type) {
 case VGA_CIRRUS:
 return pci_create_simple(bus, -1, "cirrus-vga");
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fe9937e811..8bbae68e1b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1742,6 +1742,7 @@ static void spapr_rtc_create(SpaprMachineState *spapr)
 /* Returns whether we want to use VGA or not */
 static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
 {
+vga_interface_created = true;
 switch (vga_interface_type) {
 case VGA_NONE:
 return false;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index fccaed1eb4..b693eea0e0 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -920,6 +920,7 @@ static void sun4m_hw_init(MachineState *machine)
 /* sbus irq 5 */
 cg3_init(hwdef->tcx_base, slavio_irq[11], 0x0010,
  graphic_width, graphic_height, graphic_depth);
+vga_interface_created = true;
 } else {
 /* If no display specified, default to TCX */
 if (graphic_depth != 8 && graphic_depth != 24) {
@@ -935,6 +936,7 @@ static void sun4m_hw_init(MachineState *machine)
 
 tcx_init(hwdef->tcx_base, slavio_irq[11], 0x0010,
  graphic_width, graphic_height, graphic_depth);
+vga_interface_created = true;
 }
 }
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 6fd08e2298..7c461d194a 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -632,6 +632,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 switch (vga_interface_type) {
 case VGA_STD:
 

Re: [PATCH 3/3] include/hw/ide: Unexport pci_piix3_xen_ide_unplug()

[Durrant, Paul]

On 08/05/2022 11:34, Bernhard Beschow wrote:

This function was declared in a generic and public header, implemented
in a device-specific source file but only used in xen_platform. Given its
'aux' parameter, this function is more xen-specific than piix-specific.
Also, the hardcoded magic constants seem to be generic and related to
PCIIDEState and IDEBus rather than piix.

Therefore, move this function to xen_platform, unexport it, and drop the
"piix3" in the function name as well.

Signed-off-by: Bernhard Beschow 


Reviewed-by: Paul Durrant 

... with one suggestion...


---
  hw/i386/xen/xen_platform.c | 49 +-
  hw/ide/piix.c  | 46 ---
  include/hw/ide.h   |  3 ---
  3 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 72028449ba..124ffeae35 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -26,6 +26,7 @@
  #include "qemu/osdep.h"
  #include "qapi/error.h"
  #include "hw/ide.h"
+#include "hw/ide/pci.h"
  #include "hw/pci/pci.h"
  #include "hw/xen/xen_common.h"
  #include "migration/vmstate.h"
@@ -134,6 +135,52 @@ static void pci_unplug_nics(PCIBus *bus)
  pci_for_each_device(bus, 0, unplug_nic, NULL);
  }
  
+/*

+ * The Xen HVM unplug protocol [1] specifies a mechanism to allow guests to
+ * request unplug of 'aux' disks (which is stated to mean all IDE disks,
+ * except the primary master).
+ *
+ * NOTE: The semantics of what happens if unplug of all disks and 'aux' disks
+ *   is simultaneously requested is not clear. The implementation assumes
+ *   that an 'all' request overrides an 'aux' request.
+ *
+ * [1] 
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
+ */
+static int pci_xen_ide_unplug(DeviceState *dev, bool aux)
+{
+PCIIDEState *pci_ide;
+int i;
+IDEDevice *idedev;
+IDEBus *idebus;
+BlockBackend *blk;
+
+pci_ide = PCI_IDE(dev);
+
+for (i = aux ? 1 : 0; i < 4; i++) {
+idebus = &pci_ide->bus[i / 2];
+blk = idebus->ifs[i % 2].blk;
+
+if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
+if (!(i % 2)) {
+idedev = idebus->master;
+} else {
+idedev = idebus->slave;
+}
+
+blk_drain(blk);
+blk_flush(blk);
+
+blk_detach_dev(blk, DEVICE(idedev));
+idebus->ifs[i % 2].blk = NULL;
+idedev->conf.blk = NULL;
+monitor_remove_blk(blk);
+blk_unref(blk);
+}
+}
+qdev_reset_all(dev);
+return 0;


The return value is ignored so you may as well make this a static void 
function.


  Paul


+}
+
  static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque)
  {
  uint32_t flags = *(uint32_t *)opaque;
@@ -147,7 +194,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
*opaque)
  
  switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {

  case PCI_CLASS_STORAGE_IDE:
-pci_piix3_xen_ide_unplug(DEVICE(d), aux);
+pci_xen_ide_unplug(DEVICE(d), aux);
  break;
  
  case PCI_CLASS_STORAGE_SCSI:

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index bc1b37512a..9a9b28078e 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -173,52 +173,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
  }
  }
  
-/*

- * The Xen HVM unplug protocol [1] specifies a mechanism to allow guests to
- * request unplug of 'aux' disks (which is stated to mean all IDE disks,
- * except the primary master).
- *
- * NOTE: The semantics of what happens if unplug of all disks and 'aux' disks
- *   is simultaneously requested is not clear. The implementation assumes
- *   that an 'all' request overrides an 'aux' request.
- *
- * [1] 
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
- */
-int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
-{
-PCIIDEState *pci_ide;
-int i;
-IDEDevice *idedev;
-IDEBus *idebus;
-BlockBackend *blk;
-
-pci_ide = PCI_IDE(dev);
-
-for (i = aux ? 1 : 0; i < 4; i++) {
-idebus = &pci_ide->bus[i / 2];
-blk = idebus->ifs[i % 2].blk;
-
-if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
-if (!(i % 2)) {
-idedev = idebus->master;
-} else {
-idedev = idebus->slave;
-}
-
-blk_drain(blk);
-blk_flush(blk);
-
-blk_detach_dev(blk, DEVICE(idedev));
-idebus->ifs[i % 2].blk = NULL;
-idedev->conf.blk = NULL;
-monitor_remove_blk(blk);
-blk_unref(blk);
-}
-}
-qdev_reset_all(dev);
-return 0;
-}
-
  static void pci_piix_ide_exitfn(PCIDevice *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
diff --git a/include/hw/ide.h b/include/hw/ide.h
index c5ce5da4f4..60

Re: [PATCH v2] MAINTAINERS/.mailmap: update email for Leif Lindholm

[Philippe Mathieu-Daudé via]

On Thu, May 5, 2022 at 1:38 PM Leif Lindholm  wrote:
>
> NUVIA was acquired by Qualcomm in March 2021, but kept functioning on
> separate infrastructure for a transitional period. We've now switched
> over to contributing as Qualcomm Innocation Center (quicinc), so update
> my email address to reflect this.
>
> Signed-off-by: Leif Lindholm 
> Cc: Leif Lindholm 
> Cc: Peter Maydell 
> ---

> diff --git a/.mailmap b/.mailmap
> index 2976a675ea..8c326709cf 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -62,7 +62,8 @@ Greg Kurz  
>  Huacai Chen  
>  Huacai Chen  
>  James Hogan  
> -Leif Lindholm  
> +Leif Lindholm  
> +Leif Lindholm  

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 0/2] s390x: kvm: Honor storage keys during emulation

[Cornelia Huck]

On Fri, May 06 2022, Janis Schoetterl-Glausch  wrote:

> Make use of the storage key support of the MEMOP ioctl, if available,
> in order to support storage key checking during emulation.
>
> I did not update all the headers, since that broke the build,
> not sure what the best way of dealing with that is.

Yeah, the vfio change is expected to break the build; the fix should be
easy (simple rename), and the code affected is deprecated anyway (there
hasn't been any upstream implementation that actually exposed the
interfaces). I think we should do that in a single commit to preserve
bisectability; I have not seen any patches posted yet to actually use
the new vfio migration interface, so a simple compile fixup should be
all that is needed.

>
> Janis Schoetterl-Glausch (2):
>   Pull in MEMOP changes in linux-headers
>   target/s390x: kvm: Honor storage keys during emulation
>
>  linux-headers/linux/kvm.h | 11 +--
>  target/s390x/kvm/kvm.c|  9 +
>  2 files changed, 18 insertions(+), 2 deletions(-)
>
>
> base-commit: 31abf61c4929a91275fe32f1fafe6e6b3e840b2a
> -- 
> 2.32.0

Re: [PATCH v6 0/3] qtests/libqos: Allow PCI tests to be run with virt-machine

[Eric Auger]

Hi,

On 5/4/22 17:20, Eric Auger wrote:
> Up to now the virt-machine node only contains a virtio-mmio
> driver node but no driver that eventually produces any pci-bus
> interface.
>
> Hence, PCI libqos tests cannot be run with aarch64 binary.
>
> This series brings the pieces needed to be able to run PCI tests
> with the aarch64 binary: a generic-pcihost driver node gets
> instantiated by the machine. This later contains a pci-bus-generic
> driver which produces a pci-bus interface. Then all tests
> consuming the pci-bus interface can be run with the libqos arm
> virt machine.
>
> One of the first goal was to be able to run the virtio-iommu-pci
> tests as the virtio-iommu was initially targetting ARM and it
> was awkard to run the tests with the pc machine. This is now
> possible.
>
> Only the tests doing hotplug cannot be run yet as hotplug is
> not possible on the root bus. This will be dealt with separately
> by adding a root port to the object tree.
>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/libqos-pci-arm-v6

A gentle ping on this series.

Now 5b4f72f5e86 ("tests/qtest: properly initialise the vring used idx")
has landed and the all the enabled tests pass, Could someone please consider
pulling it. I have rebased it on

907b5105f1b  ("tests: move libqtest.h back under qtest")

so now I hope everything is OK

Thank you in advance

Eric

>
> History
>
> v5 -> v6:
> - change the libqtest.h header path after pull of
>   "tests: move libqtest.h back under qtest/"
>
> v4 -> v5:
> - Added Alex' R-b
> - Removed [PATCH v3 4/5] tests/qtest/vhost-user-blk-test:
>   Temporary hack to get tests passing on aarch64
>   following Alex' fix
>
> v3 -> v4:
> - handle endianess when accessing the cfg space (fix PPC64
>   BE failure). Tested on such machine.
>
> v2 -> v3:
> - force -cpu=max along with aarch64/virt
> - reduced the vhost-user-block-pci issue workaround to a
>   single guest_alloc() instead of enabling MSIs. Call for
>   help on this specific issue. The 2 tests which fail are:
>   test_basic and indirect.
>
> v1 -> v2:
> - copyright updated to 2022
> - QPCIBusARM renamed into QGenericPCIBus
> - QGenericPCIHost declarations and definitions moved in the same
>   place as the generic pci implementation
> - rename pci-arm.c/h in generic-pcihost.c/h and remove any ref to
>   ARM there
> - remove qos_node_produces_opts, qpci_new_arm, qpci_free_arm
> - ecam_alloc_ptr now is a field of QGenericPCIBus and not QPCIBus
> - new libqos_init to create generic-pcihost driver that contains
>   pci-bus-generic
> - QGenericPCIHost moved in the same place as the generic pci
>   bindings
> - collected Thomas A-b/R-b
>
> Eric Auger (3):
>   tests/qtest/libqos/pci: Introduce pio_limit
>   tests/qtest/libqos: Skip hotplug tests if pci root bus is not
> hotpluggable
>   tests/qtest/libqos: Add generic pci host bridge in arm-virt machine
>
>  tests/qtest/e1000e-test.c |   6 +
>  tests/qtest/libqos/arm-virt-machine.c |  19 ++-
>  tests/qtest/libqos/generic-pcihost.c  | 231 ++
>  tests/qtest/libqos/generic-pcihost.h  |  54 ++
>  tests/qtest/libqos/meson.build|   1 +
>  tests/qtest/libqos/pci-pc.c   |   1 +
>  tests/qtest/libqos/pci-spapr.c|   1 +
>  tests/qtest/libqos/pci.c  |  78 +
>  tests/qtest/libqos/pci.h  |   6 +-
>  tests/qtest/vhost-user-blk-test.c |  10 ++
>  tests/qtest/virtio-blk-test.c |   5 +
>  tests/qtest/virtio-net-test.c |   5 +
>  tests/qtest/virtio-rng-test.c |   5 +
>  13 files changed, 387 insertions(+), 35 deletions(-)
>  create mode 100644 tests/qtest/libqos/generic-pcihost.c
>  create mode 100644 tests/qtest/libqos/generic-pcihost.h
>

Re: [PATCH] target/riscv: Fix VS mode hypervisor CSR access

[Alistair Francis]

On Fri, May 6, 2022 at 11:16 PM Dylan Reid  wrote:
>
> VS mode access to hypervisor CSRs should generate virtual, not illegal,
> instruction exceptions.
>
> Don't return early and indicate an illegal instruction exception when
> accessing a hypervisor CSR from VS mode. Instead, fall through to the
> `hmode` predicate to return the correct virtual instruction exception.
>
> Signed-off-by: Dylan Reid 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/csr.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..4ea7df02c9 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3141,13 +3141,13 @@ static inline RISCVException 
> riscv_csrrw_check(CPURISCVState *env,
>  #if !defined(CONFIG_USER_ONLY)
>  int effective_priv = env->priv;
>
> -if (riscv_has_ext(env, RVH) &&
> -env->priv == PRV_S &&
> -!riscv_cpu_virt_enabled(env)) {
> +if (riscv_has_ext(env, RVH) && env->priv == PRV_S) {
>  /*
> - * We are in S mode without virtualisation, therefore we are in HS 
> Mode.
> + * We are in either HS or VS mode.
>   * Add 1 to the effective privledge level to allow us to access the
> - * Hypervisor CSRs.
> + * Hypervisor CSRs. The `hmode` predicate will determine if access
> + * should be allowed(HS) or if a virtual instruction exception 
> should be
> + * raised(VS).
>   */
>  effective_priv++;
>  }
> --
> 2.30.2
>
>

Re: [PATCH v4 6/7] vhost-vdpa: change name and polarity for vhost_vdpa_one_time_request()

[Stefano Garzarella]

On Fri, May 06, 2022 at 07:28:17PM -0700, Si-Wei Liu wrote:

The name vhost_vdpa_one_time_request() was confusing. No
matter whatever it returns, its typical occurrence had
always been at requests that only need to be applied once.
And the name didn't suggest what it actually checks for.
Change it to vhost_vdpa_first_dev() with polarity flipped
for better readibility of code. That way it is able to
reflect what the check is really about.

This call is applicable to request which performs operation
only once, before queues are set up, and usually at the beginning
of the caller function. Document the requirement for it in place.

Signed-off-by: Si-Wei Liu 
---
hw/virtio/vhost-vdpa.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)


Reviewed-by: Stefano Garzarella 

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Peter Maydell]

On Mon, 9 May 2022 at 06:30, Markus Armbruster  wrote:
> Always, always, *always* document your reasons for doing stuff right in
> the commit message, unless they are blindingly obvious.  I understand
> reasons can be obvious enough to the author.  Document them anyway if
> there is any chance they are not obvious to others.

It's also nice for code-generators to say who they are
in this kind of "this file is autogenerated" comment.
For instance our own decodetree script's comments read
  /* This file is autogenerated by scripts/decodetree.py.  */

-- PMM

[PATCH] qom/object: Remove circular include dependency

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

"qom/object.h" doesn't need to include itself.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qom/object.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 5f3d5b5bf5..ef7258a5e1 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -16,7 +16,6 @@
 
 #include "qapi/qapi-builtin-types.h"
 #include "qemu/module.h"
-#include "qom/object.h"
 
 struct TypeImpl;
 typedef struct TypeImpl *Type;
-- 
2.35.1

Re: [PATCH v2 1/2] hw/core: Sync uboot_image.h from U-Boot v2022.01

[Alistair Francis]

On Fri, May 6, 2022 at 11:25 AM Alex Bennée  wrote:
>
>
> Bin Meng  writes:
>
> > +more
> >
> > On Tue, May 3, 2022 at 11:44 AM Bin Meng  wrote:
> >>
> >> On Thu, Apr 28, 2022 at 4:43 PM Bin Meng  wrote:
> >> >
> >> > On Fri, Apr 22, 2022 at 11:00 AM Bin Meng  wrote:
> >> > >
> >> > > +Richard
> >> > >
> >> > > On Wed, Apr 20, 2022 at 4:16 PM Bin Meng  wrote:
> >> > > >
> >> > > > On Tue, Apr 12, 2022 at 9:11 AM Bin Meng  wrote:
> >> > > > >
> >> > > > > On Thu, Mar 24, 2022 at 9:48 PM Bin Meng  
> >> > > > > wrote:
> >> > > > > >
> >> > > > > > From: Bin Meng 
> >> > > > > >
> >> > > > > > Sync uboot_image.h from upstream U-Boot v2022.01 release [1].
> >> > > > > >
> >> > > > > > [1] 
> >> > > > > > https://source.denx.de/u-boot/u-boot/-/blob/v2022.01/include/image.h
> >> > > > > >
> >> > > > > > Signed-off-by: Bin Meng 
> >> > > > > > ---
> >> > > > > >
> >> > > > > > (no changes since v1)
> >> > > > > >
> >> > > > > >  hw/core/uboot_image.h | 213 
> >> > > > > > --
> >> > > > > >  1 file changed, 142 insertions(+), 71 deletions(-)
> >> > > > > >
> >> > > > >
> >> > > > > Ping?
> >> > > >
> >> > > > Ping?
> >> > >
> >> > > Richard, is that you to pick up this series?
> >> > >
> >> >
> >> > Ping?
> >>
> >> Ping? Can you please indicate who is the right person to pick up this
> >> series? Thanks.
> >>
> >
> > I pinged several times, but there's radio silence, sigh. Can you
> > please let me know who is supposed to pick up this series?
>
> The only file that includes this is hw/core/loader.c so I would assume
> that's in Alistair's domain. However it's not well matched by
> MAINTAINERS and has only seen periodic updates since it's inclusion in
> 2007.
>
> Alistair can you take this and update MAINTAINERS so it doesn't fall
> through the cracks again?

Thanks!

Applied to riscv-to-apply.next

I'll update MAINTAINERS as well

Alistair

>
>
> >
> > Regards,
> > Bin
>
>
> --
> Alex Bennée
>

Re: [PATCH v2 1/2] hw/core: Sync uboot_image.h from U-Boot v2022.01

[Bin Meng]

On Mon, May 9, 2022 at 5:07 PM Alistair Francis  wrote:
>
> On Fri, May 6, 2022 at 11:25 AM Alex Bennée  wrote:
> >
> >
> > Bin Meng  writes:
> >
> > > +more
> > >
> > > On Tue, May 3, 2022 at 11:44 AM Bin Meng  wrote:
> > >>
> > >> On Thu, Apr 28, 2022 at 4:43 PM Bin Meng  wrote:
> > >> >
> > >> > On Fri, Apr 22, 2022 at 11:00 AM Bin Meng  wrote:
> > >> > >
> > >> > > +Richard
> > >> > >
> > >> > > On Wed, Apr 20, 2022 at 4:16 PM Bin Meng  wrote:
> > >> > > >
> > >> > > > On Tue, Apr 12, 2022 at 9:11 AM Bin Meng  
> > >> > > > wrote:
> > >> > > > >
> > >> > > > > On Thu, Mar 24, 2022 at 9:48 PM Bin Meng  
> > >> > > > > wrote:
> > >> > > > > >
> > >> > > > > > From: Bin Meng 
> > >> > > > > >
> > >> > > > > > Sync uboot_image.h from upstream U-Boot v2022.01 release [1].
> > >> > > > > >
> > >> > > > > > [1] 
> > >> > > > > > https://source.denx.de/u-boot/u-boot/-/blob/v2022.01/include/image.h
> > >> > > > > >
> > >> > > > > > Signed-off-by: Bin Meng 
> > >> > > > > > ---
> > >> > > > > >
> > >> > > > > > (no changes since v1)
> > >> > > > > >
> > >> > > > > >  hw/core/uboot_image.h | 213 
> > >> > > > > > --
> > >> > > > > >  1 file changed, 142 insertions(+), 71 deletions(-)
> > >> > > > > >
> > >> > > > >
> > >> > > > > Ping?
> > >> > > >
> > >> > > > Ping?
> > >> > >
> > >> > > Richard, is that you to pick up this series?
> > >> > >
> > >> >
> > >> > Ping?
> > >>
> > >> Ping? Can you please indicate who is the right person to pick up this
> > >> series? Thanks.
> > >>
> > >
> > > I pinged several times, but there's radio silence, sigh. Can you
> > > please let me know who is supposed to pick up this series?
> >
> > The only file that includes this is hw/core/loader.c so I would assume
> > that's in Alistair's domain. However it's not well matched by
> > MAINTAINERS and has only seen periodic updates since it's inclusion in
> > 2007.
> >
> > Alistair can you take this and update MAINTAINERS so it doesn't fall
> > through the cracks again?
>
> Thanks!
>
> Applied to riscv-to-apply.next
>
> I'll update MAINTAINERS as well
>

Thank you Alex, Alistair!

[PATCH] MAINTAINERS: Add myself as hw/core/uboot_image.h maintainer

[Alistair Francis via]

Signed-off-by: Alistair Francis 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 662ec47246..9ba30cec8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2173,6 +2173,7 @@ Generic Loader
 M: Alistair Francis 
 S: Maintained
 F: hw/core/generic-loader.c
+F: hw/core/uboot_image.h
 F: include/hw/core/generic-loader.h
 F: docs/system/generic-loader.rst
 
-- 
2.34.3

Re: [PATCH] target/riscv: Fix VS mode hypervisor CSR access

[Alistair Francis]

On Fri, May 6, 2022 at 11:16 PM Dylan Reid  wrote:
>
> VS mode access to hypervisor CSRs should generate virtual, not illegal,
> instruction exceptions.
>
> Don't return early and indicate an illegal instruction exception when
> accessing a hypervisor CSR from VS mode. Instead, fall through to the
> `hmode` predicate to return the correct virtual instruction exception.
>
> Signed-off-by: Dylan Reid 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/csr.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..4ea7df02c9 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3141,13 +3141,13 @@ static inline RISCVException 
> riscv_csrrw_check(CPURISCVState *env,
>  #if !defined(CONFIG_USER_ONLY)
>  int effective_priv = env->priv;
>
> -if (riscv_has_ext(env, RVH) &&
> -env->priv == PRV_S &&
> -!riscv_cpu_virt_enabled(env)) {
> +if (riscv_has_ext(env, RVH) && env->priv == PRV_S) {
>  /*
> - * We are in S mode without virtualisation, therefore we are in HS 
> Mode.
> + * We are in either HS or VS mode.
>   * Add 1 to the effective privledge level to allow us to access the
> - * Hypervisor CSRs.
> + * Hypervisor CSRs. The `hmode` predicate will determine if access
> + * should be allowed(HS) or if a virtual instruction exception 
> should be
> + * raised(VS).
>   */
>  effective_priv++;
>  }
> --
> 2.30.2
>
>

Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode

[Alistair Francis]

On Thu, May 5, 2022 at 12:36 PM Anup Patel  wrote:
>
> On Thu, May 5, 2022 at 3:21 PM Alistair Francis  wrote:
> >
> > On Fri, Apr 29, 2022 at 1:38 PM Anup Patel  wrote:
> > >
> > > Currently, QEMU does not set hstatus.GVA bit for traps taken from
> > > HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> > > on QEMU. This was working previously.
> > >
> > > This patch updates riscv_cpu_do_interrupt() to fix the above issue.
> > >
> > > Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> > > Signed-off-by: Anup Patel 
> > > ---
> > >  target/riscv/cpu_helper.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index e1aa4f2097..d83579accf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > >  /* Trap into HS mode */
> > >  env->hstatus = set_field(env->hstatus, HSTATUS_SPV, 
> > > false);
> > >  htval = env->guest_phys_fault_addr;
> > > -write_gva = false;
> >
> > This doesn't seem right.
> >
> > "Field GVA (Guest Virtual Address) is written by the implementation
> > whenever a trap is taken
> > into HS-mode. For any trap (breakpoint, address misaligned, access
> > fault, page fault, or guest-
> > page fault) that writes a guest virtual address to stval, GVA is set
> > to 1. For any other trap into
> > HS-mode, GVA is set to 0"
> >
> > So if we are trapping from HS to HS, the address in stval should not
> > be a guest virtual address, at least in general.
>
> That's not correct. The HLV/HSV instructions executed by hypervisor
> (HS-mode) take guest virtual address. These instructions can trap
> from HS-mode to HS-mode.

Ah, I forgot about those instructions, but still they are the
exception. In general we would expect a trap from HS to HS to contain
HS addresses. We should just handle the other cases specially

Alistair

>
> >
> > We probably aren't correctly setting GVA if MPRV is set though, as
> > then the page faults should be guest addresses. That's probably the
> > issue you are seeing.
>
> The Xvisor nested MMU test-suit is broken on QEMU because it
> uses HLV/HSV instructions in HS-mode.
>
> Regards,
> Anup
>
> >
> > Alistair
> >
> > >  }
> > >  env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 
> > > write_gva);
> > >  }
> > > --
> > > 2.34.1
> > >
> > >

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Mon, May 9, 2022 at 4:42 PM Peter Maydell  wrote:
>
> On Mon, 9 May 2022 at 06:30, Markus Armbruster  wrote:
> > Always, always, *always* document your reasons for doing stuff right in
> > the commit message, unless they are blindingly obvious.  I understand
> > reasons can be obvious enough to the author.  Document them anyway if
> > there is any chance they are not obvious to others.
>
> It's also nice for code-generators to say who they are
> in this kind of "this file is autogenerated" comment.
> For instance our own decodetree script's comments read
>   /* This file is autogenerated by scripts/decodetree.py.  */

Unfortunately, this is not what bpftool did right now.

Have posted a patch (with Markus and you cced).

Thanks

>
> -- PMM
>

Re: [PATCH v6 00/24] target/arm: Cleanups, new features, new cpus

[Peter Maydell]

On Fri, 6 May 2022 at 19:03, Richard Henderson
 wrote:
>
> Changes for v6:
>   * Expand the commit message for "Drop EL3 no EL2 fallbacks" (pmm)
>
> All patches are reviewed.
>
>
> r~



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[yangxiaojuan]

Hi Richard,

On 2022/5/7 下午11:31, Richard Henderson wrote:

On 4/29/22 05:07, Xiaojuan Yang wrote:

+    int ipmap_mask = 0xff << ipmap_offset;

...

+    int cpu_mask = 0xff << ipmap_offset;


These two masks are redundant with

+    ipnum = ((s->ipmap[ipmap_index] & ipmap_mask) >> ipmap_offset) & 
0xf;

...

+    cpu = ((s->coremap[cpu_index] & cpu_mask) >> cpu_offset) & 0xf;


the 0xf masking here.


+    cpu = ctz32(cpu);
+    cpu = (cpu >= 4) ? 0 : cpu;


You are not considering CSR[0x420][49], which changes the format of 
this mapping.


Thanks very much, I will consider the mapping format by read 
iocsr[0x420][49] like this:

static uint64_t map_format(void)
{
    LoongArchCPU *cpu;
    CPULoongArchState *env;
    uint64_t val;

    cpu = LOONGARCH_CPU(current_cpu);
    env = &(cpu->env);

    val = address_space_ldq(&env->address_space_iocsr, 0x420,
 MEMTXATTRS_UNSPECIFIED, NULL);
    val &= 1 << 49;
    return val;
}
...
if (!map_format()) {
    cpu = ctz32(cpu);
    cpu = (cpu >= 4) ? 0 : cpu;
}
...
I think this function is wrong because you maintain an unmapped enable 
bitmap, but you do not maintain an unmapped status bitmap, which 
*should* be readable from EXTIOI_ISR_{START,END}, but is not present 
in extioi_readw.


I think that only extioi_setirq should actually change the unmapped 
status bitmap, and that extioi_update_irq should only evaluate the 
mapping to apply changes to the cpus.


Ok, there should be ISR registers(the status bitmap), i will add it to 
the LoongArchExtIOI, like this:

struct LoongArchExtIOI {
...
+    uint32_t isr[EXTIOI_IRQS / 32]
...
}

when extioi_setirq, update the isr filed.
static void extioi_setirq(void *opaque, int irq, int level)
{
    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
    trace_loongarch_extioi_setirq(irq, level);
    s->isr[irq / 32] |= 1 << irq % 32;
    extioi_update_irq(s, irq, level);
}

and add ISR_START ... ISR_END to extioi_readw, like this
...
    case EXTIOI_ISR_START ... EXTIOI_ISR_END - 1:
    index = ((offset - EXTIOI_ISR_START) >> 2;
    ret = s->isr[index];
    break;
...



This final bit, updating the cpu irq is also wrong, in that it should 
be unconditional. This is the only way that it will work for the usage 
in updating the enable mask.


I think you are not considering when the MAP registers overlap 
outputs.  For instance, if all 256 bits of EXT_IOIMap contain 0, then 
all of EXT_IOI[n*32+31 : n*32] overlap.  When that happens, you cannot 
lower the level of the cpu pin until all of the matching ioi 
interrupts are low.


Or, perhaps I don't understand how this is supposed to work?
The documentation is very weak.



+static void extioi_writew(void *opaque, hwaddr addr,
+   uint64_t val, unsigned size)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+    int cpu, index, old_data, data_offset;
+    uint32_t offset;
+    trace_loongarch_extioi_writew(size, (uint32_t)addr, val);
+
+    offset = addr & 0x;
+
+    switch (offset) {
+    case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+    index = (offset - EXTIOI_NODETYPE_START) >> 2;
+    s->nodetype[index] = val;
+    break;
+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    index = (offset - EXTIOI_IPMAP_START) >> 2;
+    s->ipmap[index] = val;
+    break;


Do you need to recompute the entire interrupt map when ipmap changes?

Sorry, could you explain it in more detail? i can not understand the 
meanning of 'the entire interrupt map'?
we only have ipmap and coremap registers in the LoongArchExtIOI, should 
we add an entire interrupt map?

+    case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+    index = (offset - EXTIOI_ENABLE_START) >> 2;
+    old_data = s->enable[index];
+    if (old_data != (int)val) {
+    s->enable[index] = val;
+    old_data = old_data ^ val;
+    data_offset = ctz32(old_data);
+    while (data_offset != 32) {
+    if (!(val & (1 << data_offset))) {
+    extioi_update_irq(s, data_offset + index * 32, 0);


This is not correct -- you're unconditionally setting level=0, 
corrupting the old value of coreisr[cpu][index].  You need to 
recompute *without* changning those levels.


Thanks, i will add a condition to judge coreisr[cpu][index], excute 
extioi_update_irq when the coreisr val is 1, like this:


static int get_coremap(int irq_num)
{
    int cpu;
    int cpu_index = irq_num / 4;
    int cpu_offset = irq_num & 0x3;
    int cpu_mask = 0xf << cpu_offset;

    cpu = (s->coremap[cpu_index] & cpu_mask) >> cpu_offset;
    if (!map_format()) {
    cpu = ctz32(cpu);
    cpu = (cpu >= 4) ? 0 : cpu;
    }
    return cpu;
}

static int coreisr_level(LoongArchExtIOI *s, int irq_num)
{
    int cpu = get_coremap(irq_num);
    return s->coreisr[cpu][irq_num / 32];
}

...
 while (data_offset != 32) {
 if (!(val & (1 << data_offse

Re: [PATCH] qom/object: Remove circular include dependency

[Damien Hedde]

On 5/9/22 10:46, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé 

"qom/object.h" doesn't need to include itself.

Signed-off-by: Philippe Mathieu-Daudé 


Reviewed-by: Damien Hedde 

---
  include/qom/object.h | 1 -
  1 file changed, 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 5f3d5b5bf5..ef7258a5e1 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -16,7 +16,6 @@
  
  #include "qapi/qapi-builtin-types.h"

  #include "qemu/module.h"
-#include "qom/object.h"
  
  struct TypeImpl;

  typedef struct TypeImpl *Type;

Re: [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps

[Alistair Francis]

On Fri, Apr 29, 2022 at 5:36 AM Anup Patel  wrote:>
> Currently, the [m|s]tval CSRs are set with trapping instruction encoding
> only for illegal instruction traps taken at the time of instruction
> decoding.
>
> In RISC-V world, a valid instructions might also trap as illegal or
> virtual instruction based to trapping bits in various CSRs (such as
> mstatus.TVM or hstatus.VTVM).
>
> We improve setting of [m|s]tval CSRs for all types of illegal and
> virtual instruction traps.
>
> Signed-off-by: Anup Patel 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c|  2 ++
>  target/riscv/cpu.h|  8 +++-
>  target/riscv/cpu_helper.c |  1 +
>  target/riscv/translate.c  | 17 +
>  4 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dff4606585..f0a702fee6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env, 
> TranslationBlock *tb,
>  } else {
>  env->pc = data[0];
>  }
> +env->bins = data[1];
>  }
>
>  static void riscv_cpu_reset(DeviceState *dev)
> @@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
>  env->mcause = 0;
>  env->miclaim = MIP_SGEIP;
>  env->pc = env->resetvec;
> +env->bins = 0;
>  env->two_stage_lookup = false;
>
>  /* Initialized default priorities of local interrupts. */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe6c9a2c92..a55c918274 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -30,6 +30,12 @@
>
>  #define TCG_GUEST_DEFAULT_MO 0
>
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: Original instruction opcode
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +
>  #define TYPE_RISCV_CPU "riscv-cpu"
>
>  #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> @@ -140,7 +146,7 @@ struct CPUArchState {
>  target_ulong frm;
>
>  target_ulong badaddr;
> -uint32_t bins;
> +target_ulong bins;
>
>  target_ulong guest_phys_fault_addr;
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index d83579accf..bba4fce777 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  tval = env->badaddr;
>  break;
>  case RISCV_EXCP_ILLEGAL_INST:
> +case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
>  tval = env->bins;
>  break;
>  default:
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0cd1d9ee94..55a4713af2 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,8 @@ typedef struct DisasContext {
>  /* PointerMasking extension */
>  bool pm_mask_enabled;
>  bool pm_base_enabled;
> +/* TCG of the current insn_start */
> +TCGOp *insn_start;
>  } DisasContext;
>
>  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext *ctx, 
> int excp)
>
>  static void gen_exception_illegal(DisasContext *ctx)
>  {
> -tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
> -   offsetof(CPURISCVState, bins));
> -
>  generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
>  }
>
> @@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase, 
> target_ulong pc)
>  /* Include decoders for factored-out extensions */
>  #include "decode-XVentanaCondOps.c.inc"
>
> +static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
> +{
> +assert(ctx->insn_start != NULL);
> +tcg_set_insn_start_param(ctx->insn_start, 1, opc);
> +ctx->insn_start = NULL;
> +}
> +
>  static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t 
> opcode)
>  {
>  /*
> @@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env, DisasContext 
> *ctx, uint16_t opcode)
>
>  /* Check for compressed insn */
>  if (extract16(opcode, 0, 2) != 3) {
> +decode_save_opc(ctx, opcode);
>  if (!has_ext(ctx, RVC)) {
>  gen_exception_illegal(ctx);
>  } else {
> @@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env, DisasContext 
> *ctx, uint16_t opcode)
>  opcode32 = deposit32(opcode32, 16, 16,
>   translator_lduw(env, &ctx->base,
>   ctx->base.pc_next + 2));
> +decode_save_opc(ctx, opcode32);
>  ctx->opcode = opcode32;
>  ctx->pc_succ_insn = ctx->base.pc_next + 4;
>
> @@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase 
> *dcbase, CPUState *cpu)
>  {
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> -tcg_gen_insn_start(ctx->base.pc_next);
> +tcg_gen_insn_start(ctx->base.pc_next, 0);
> +ctx->insn_start = tcg_last_op();
>  }
>
>  static void riscv_tr_translate_insn(DisasCont

Re: [PATCH] qom/object: Remove circular include dependency

[Peter Maydell]

On Mon, 9 May 2022 at 09:53, Philippe Mathieu-Daudé
 wrote:
>
> From: Philippe Mathieu-Daudé 
>
> "qom/object.h" doesn't need to include itself.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/qom/object.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 5f3d5b5bf5..ef7258a5e1 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -16,7 +16,6 @@
>
>  #include "qapi/qapi-builtin-types.h"
>  #include "qemu/module.h"
> -#include "qom/object.h"
>
>  struct TypeImpl;
>  typedef struct TypeImpl *Type;

Accidentally (but harmlessly) added in commit db1015e92e0483 by a
change generated by a script.

Reviewed-by: Peter Maydell 

Eduardo: is it worth making ./scripts/codeconverter/converter.py
handle this as a special case, so it doesn't add the include line
to qom/object.h itself ? Or do we not really expect that script
to be run on the codebase again in future ?

thanks
-- PMM

Re: [PATCH qemu] target/riscv: rvv: Fix early exit condition for whole register load/store

[Alistair Francis]

On Fri, May 6, 2022 at 7:17 AM ~eopxd  wrote:
>
> From: eopXD 
>
> Vector whole register load instructions have EEW encoded in the opcode,
> so we shouldn't take SEW here. Vector whole register store instructions
> are always EEW=8.
>
> Signed-off-by: eop Chen 
> Reviewed-by: Frank Chang 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 58 +
>  1 file changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 90327509f7..391c61fe93 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -1118,10 +1118,10 @@ GEN_VEXT_TRANS(vle64ff_v, MO_64, r2nfvm, ldff_op, 
> ld_us_check)
>  typedef void gen_helper_ldst_whole(TCGv_ptr, TCGv, TCGv_env, TCGv_i32);
>
>  static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf,
> - gen_helper_ldst_whole *fn, DisasContext *s,
> - bool is_store)
> + uint32_t width, gen_helper_ldst_whole *fn,
> + DisasContext *s, bool is_store)
>  {
> -uint32_t evl = (s->cfg_ptr->vlen / 8) * nf / (1 << s->sew);
> +uint32_t evl = (s->cfg_ptr->vlen / 8) * nf / width;
>  TCGLabel *over = gen_new_label();
>  tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, evl, over);
>
> @@ -1153,38 +1153,42 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t 
> rs1, uint32_t nf,
>   * load and store whole register instructions ignore vtype and vl setting.
>   * Thus, we don't need to check vill bit. (Section 7.9)
>   */
> -#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF, IS_STORE)  \
> +#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF, WIDTH, IS_STORE)   \
>  static bool trans_##NAME(DisasContext *s, arg_##NAME * a) \
>  { \
>  if (require_rvv(s) && \
>  QEMU_IS_ALIGNED(a->rd, ARG_NF)) { \
> -return ldst_whole_trans(a->rd, a->rs1, ARG_NF, gen_helper_##NAME, \
> -s, IS_STORE); \
> +return ldst_whole_trans(a->rd, a->rs1, ARG_NF, WIDTH, \
> +gen_helper_##NAME, s, IS_STORE);  \
>  } \
>  return false; \
>  }
>
> -GEN_LDST_WHOLE_TRANS(vl1re8_v,  1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re16_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re32_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re64_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl2re8_v,  2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re16_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re32_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re64_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl4re8_v,  4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re16_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re32_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re64_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl8re8_v,  8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re16_v, 8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re32_v, 8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re64_v, 8, false)
> -
> -GEN_LDST_WHOLE_TRANS(vs1r_v, 1, true)
> -GEN_LDST_WHOLE_TRANS(vs2r_v, 2, true)
> -GEN_LDST_WHOLE_TRANS(vs4r_v, 4, true)
> -GEN_LDST_WHOLE_TRANS(vs8r_v, 8, true)
> +GEN_LDST_WHOLE_TRANS(vl1re8_v,  1, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl1re16_v, 1, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl1re32_v, 1, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl1re64_v, 1, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl2re8_v,  2, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl2re16_v, 2, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl2re32_v, 2, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl2re64_v, 2, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl4re8_v,  4, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl4re16_v, 4, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl4re32_v, 4, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl4re64_v, 4, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl8re8_v,  8, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl8re16_v, 8, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl8re32_v, 8, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl8re64_v, 8, 8, false)
> +
> +/*
> + * The vector whole register store instructions are encoded similar to
> + * unmasked unit-stride store of elements with EEW=8.
> + */
> +GEN_LDST_WHOLE_TRANS(vs1r_v, 1, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs2r_v, 2, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs4r_v, 4, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs8r_v, 8, 1, true)
>
>  /*
>   *** Vector Integer Arithmetic Instructions
> --
> 2.34.2
>

Re: [PATCH v2] hw/arm: add versioning to sbsa-ref machine DT

[Peter Maydell]

On Thu, 5 May 2022 at 12:39, Leif Lindholm  wrote:
>
> The sbsa-ref machine is continuously evolving. Some of the changes we
> want to make in the near future, to align with real components (e.g.
> the GIC-700), will break compatibility for existing firmware.
>
> Introduce two new properties to the DT generated on machine generation:
> - machine-version-major
>   To be incremented when a platform change makes the machine
>   incompatible with existing firmware.
> - machine-version-minor
>   To be incremented when functionality is added to the machine
>   without causing incompatibility with existing firmware.
>   to be reset to 0 when machine-version-major is incremented.
>
> This versioning scheme is *neither*:
> - A QEMU versioned machine type; a given version of QEMU will emulate
>   a given version of the platform.
> - A reflection of level of SBSA (now SystemReady SR) support provided.
>
> The version will increment on guest-visible functional changes only,
> akin to a revision ID register found on a physical platform.
>
> These properties are both introduced with the value 0.
> (Hence, a machine where the DT is lacking these nodes is equivalent
> to version 0.0.)
>



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH] qom/object: Remove circular include dependency

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> From: Philippe Mathieu-Daudé 
>
> "qom/object.h" doesn't need to include itself.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/qom/object.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 5f3d5b5bf5..ef7258a5e1 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -16,7 +16,6 @@
>  
>  #include "qapi/qapi-builtin-types.h"
>  #include "qemu/module.h"
> -#include "qom/object.h"
>  
>  struct TypeImpl;
>  typedef struct TypeImpl *Type;

I figure this is scripts/codeconverter/converter.py's doing, in commit
db1015e92e "Move QOM typedefs and add missing includes".  Probably not
worth fixing there.  Possibly worth mentioning in the commit message.

Reviewed-by: Markus Armbruster 

Re: [PATCH 0/6] virtio-scsi: fix 100% CPU consumption in IOThread

[Stefan Hajnoczi]

On Wed, Apr 27, 2022 at 03:35:35PM +0100, Stefan Hajnoczi wrote:
> Nir Soffer reported that virtio-scsi,iothread=... consumes 100% CPU in QEMU
> 7.0. This patch series addresses two bugs in hw/scsi/virtio-scsi.c (see 
> patches
> 1 & 2) and follows up with code cleanups.
> 
> Stefan Hajnoczi (6):
>   virtio-scsi: fix ctrl and event handler functions in dataplane mode
>   virtio-scsi: don't waste CPU polling the event virtqueue
>   virtio-scsi: clean up virtio_scsi_handle_event_vq()
>   virtio-scsi: clean up virtio_scsi_handle_ctrl_vq()
>   virtio-scsi: clean up virtio_scsi_handle_cmd_vq()
>   virtio-scsi: move request-related items from .h to .c
> 
>  include/hw/virtio/virtio-scsi.h |  43 --
>  include/hw/virtio/virtio.h  |   1 +
>  hw/scsi/virtio-scsi-dataplane.c |   2 +-
>  hw/scsi/virtio-scsi.c   | 101 ++--
>  hw/virtio/virtio.c  |  13 
>  5 files changed, 86 insertions(+), 74 deletions(-)
> 
> -- 
> 2.35.1
> 

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v3] Warn user if the vga flag is passed but no vga device is created

[Gerd Hoffmann]

  Hi,

> Oh, I see now -- qemu_disable_default_devices() does a
> preliminary scan through of every supplied -device option,
> looking to see if it has a driver=foo that matches some
> value in the default_list[] array; if it does then we
> set default_vga or whatever to false. (So effectively we
> have a hardcoded list of things we consider to be "VGA
> devices" for this purpose, which might or might not be the same
> as the set of actual VGA devices we support...)
> 
> I guess this is all here for backwards compatibility purposes?

Well, sort of.  This tries to deal with the messy side effects of
the default devices, i.e. avoid users end up with *two* vga devices
in case they use "qemu -device VGA" (same for serial, ...).

take care,
  Gerd

Re: [PATCH 3/3] include/hw/ide: Unexport pci_piix3_xen_ide_unplug()

[Bernhard Beschow]

Am 9. Mai 2022 08:02:13 UTC schrieb "Durrant, Paul" :
>On 08/05/2022 11:34, Bernhard Beschow wrote:
>> This function was declared in a generic and public header, implemented
>> in a device-specific source file but only used in xen_platform. Given its
>> 'aux' parameter, this function is more xen-specific than piix-specific.
>> Also, the hardcoded magic constants seem to be generic and related to
>> PCIIDEState and IDEBus rather than piix.
>> 
>> Therefore, move this function to xen_platform, unexport it, and drop the
>> "piix3" in the function name as well.
>> 
>> Signed-off-by: Bernhard Beschow 
>
>Reviewed-by: Paul Durrant 
>
>... with one suggestion...
>
>> ---
>>   hw/i386/xen/xen_platform.c | 49 +-
>>   hw/ide/piix.c  | 46 ---
>>   include/hw/ide.h   |  3 ---
>>   3 files changed, 48 insertions(+), 50 deletions(-)
>> 
>> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
>> index 72028449ba..124ffeae35 100644
>> --- a/hw/i386/xen/xen_platform.c
>> +++ b/hw/i386/xen/xen_platform.c
>> @@ -26,6 +26,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>>   #include "hw/ide.h"
>> +#include "hw/ide/pci.h"
>>   #include "hw/pci/pci.h"
>>   #include "hw/xen/xen_common.h"
>>   #include "migration/vmstate.h"
>> @@ -134,6 +135,52 @@ static void pci_unplug_nics(PCIBus *bus)
>>   pci_for_each_device(bus, 0, unplug_nic, NULL);
>>   }
>>   +/*
>> + * The Xen HVM unplug protocol [1] specifies a mechanism to allow guests to
>> + * request unplug of 'aux' disks (which is stated to mean all IDE disks,
>> + * except the primary master).
>> + *
>> + * NOTE: The semantics of what happens if unplug of all disks and 'aux' 
>> disks
>> + *   is simultaneously requested is not clear. The implementation 
>> assumes
>> + *   that an 'all' request overrides an 'aux' request.
>> + *
>> + * [1] 
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
>> + */
>> +static int pci_xen_ide_unplug(DeviceState *dev, bool aux)
>> +{
>> +PCIIDEState *pci_ide;
>> +int i;
>> +IDEDevice *idedev;
>> +IDEBus *idebus;
>> +BlockBackend *blk;
>> +
>> +pci_ide = PCI_IDE(dev);
>> +
>> +for (i = aux ? 1 : 0; i < 4; i++) {
>> +idebus = &pci_ide->bus[i / 2];
>> +blk = idebus->ifs[i % 2].blk;
>> +
>> +if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
>> +if (!(i % 2)) {
>> +idedev = idebus->master;
>> +} else {
>> +idedev = idebus->slave;
>> +}
>> +
>> +blk_drain(blk);
>> +blk_flush(blk);
>> +
>> +blk_detach_dev(blk, DEVICE(idedev));
>> +idebus->ifs[i % 2].blk = NULL;
>> +idedev->conf.blk = NULL;
>> +monitor_remove_blk(blk);
>> +blk_unref(blk);
>> +}
>> +}
>> +qdev_reset_all(dev);
>> +return 0;
>
>The return value is ignored so you may as well make this a static void 
>function.

Good catch! I'll prepare a v2. Meanwhile, I'm looking forward to comments on 
the other patches as well.

Thanks,
Bernhard

>  Paul
>
>> +}
>> +
>>   static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque)
>>   {
>>   uint32_t flags = *(uint32_t *)opaque;
>> @@ -147,7 +194,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
>> *opaque)
>> switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
>>   case PCI_CLASS_STORAGE_IDE:
>> -pci_piix3_xen_ide_unplug(DEVICE(d), aux);
>> +pci_xen_ide_unplug(DEVICE(d), aux);
>>   break;
>> case PCI_CLASS_STORAGE_SCSI:
>> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>> index bc1b37512a..9a9b28078e 100644
>> --- a/hw/ide/piix.c
>> +++ b/hw/ide/piix.c
>> @@ -173,52 +173,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
>> **errp)
>>   }
>>   }
>>   -/*
>> - * The Xen HVM unplug protocol [1] specifies a mechanism to allow guests to
>> - * request unplug of 'aux' disks (which is stated to mean all IDE disks,
>> - * except the primary master).
>> - *
>> - * NOTE: The semantics of what happens if unplug of all disks and 'aux' 
>> disks
>> - *   is simultaneously requested is not clear. The implementation 
>> assumes
>> - *   that an 'all' request overrides an 'aux' request.
>> - *
>> - * [1] 
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
>> - */
>> -int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>> -{
>> -PCIIDEState *pci_ide;
>> -int i;
>> -IDEDevice *idedev;
>> -IDEBus *idebus;
>> -BlockBackend *blk;
>> -
>> -pci_ide = PCI_IDE(dev);
>> -
>> -for (i = aux ? 1 : 0; i < 4; i++) {
>> -idebus = &pci_ide->bus[i / 2];
>> -blk = idebus->ifs[i % 2].blk;
>> -
>> -if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
>> -if (!(i % 2)) {
>> -idedev = idebus->mas

Re: [PATCH 2/2] target/riscv: Add short-isa-string option

[Alistair Francis]

On Sun, Apr 24, 2022 at 7:22 AM Tsukasa OI  wrote:
>
> Because some operating systems don't correctly parse long ISA extension
> string, this commit adds short-isa-string boolean option to disable
> generating long ISA extension strings on Device Tree.
>
> Operating Systems which short-isa-string might be helpful:
>
> 1.  Linux (5.17 or earlier)
> 2.  FreeBSD (at least 14.0-CURRENT)
> 3.  OpenBSD (at least current development version)
>
> Signed-off-by: Tsukasa OI 
> ---
>  target/riscv/cpu.c | 5 -
>  target/riscv/cpu.h | 2 ++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c765f7ff00..9718cd0e7e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -834,6 +834,8 @@ static Property riscv_cpu_properties[] = {
>  DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
>
>  DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
> +
> +DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
> false),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -989,7 +991,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
>  }
>  }
>  *p = '\0';
> -riscv_isa_string_ext(cpu, &isa_str, maxlen);
> +if (!cpu->cfg.short_isa_string)
> +riscv_isa_string_ext(cpu, &isa_str, maxlen);

I don't love this, the long strings are part of the ISA, it seems
strange to add an option to disable them.

Can you provide more details on what this breaks?

Alistair

>  return isa_str;
>  }
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 34c22d5d3b..5b7fe32218 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -408,6 +408,8 @@ struct RISCVCPUConfig {
>  bool aia;
>  bool debug;
>  uint64_t resetvec;
> +
> +bool short_isa_string;
>  };
>
>  typedef struct RISCVCPUConfig RISCVCPUConfig;
> --
> 2.32.0
>

Re: [PATCH 0/2] thread-pool: fix performance regression

[Paolo Bonzini]

On 5/9/22 08:42, Lukáš Doktor wrote:

Dne 06. 05. 22 v 20:55 Lukáš Doktor napsal(a):

Hello Paolo, folks, I gave it a try (on top of the f9fc8932) and
it's better than the f9fc8932, better than the previous patch by
Stefan, but still I'm not reaching the performance of d7482ffe97
(before the f9fc8932 commit):

f9f|  0.0 | -2.8 |  0.6 stefan | -3.1 | -1.2 | -2.2 paolo  |
5.3 |  5.4 |  7.1 d74|  7.2 |  9.1 |  8.2

Anyway it's definitely closer to the previous baseline (~-2%). Note
I have not tried other scenarios, just the 4K nbd writes on
rotational disk. I'll try running more throughout the night.



I tried a couple of iterations of fio-nbd 4/64/256KB read/writes on a
rotational disk and overall the latest fix results in a steady 2.5%
throughput regression for the 4KiB writes. The remaining tested
variants performed similarly. Please let me know if you want me to
test the fio execution inside the guest as well or some other
variants.


Considering we have conflicting results (I get a 2-3% improvement over 
6.2), and that in general aio=native/aio=io_uring is preferred, I think 
we can proceed with these patches at least for now.


Paolo

Re: [PATCH qemu] target/riscv: rvv: Fix early exit condition for whole register load/store

[Alistair Francis]

On Fri, May 6, 2022 at 7:17 AM ~eopxd  wrote:
>
> From: eopXD 
>
> Vector whole register load instructions have EEW encoded in the opcode,
> so we shouldn't take SEW here. Vector whole register store instructions
> are always EEW=8.
>
> Signed-off-by: eop Chen 
> Reviewed-by: Frank Chang 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 58 +
>  1 file changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 90327509f7..391c61fe93 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -1118,10 +1118,10 @@ GEN_VEXT_TRANS(vle64ff_v, MO_64, r2nfvm, ldff_op, 
> ld_us_check)
>  typedef void gen_helper_ldst_whole(TCGv_ptr, TCGv, TCGv_env, TCGv_i32);
>
>  static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf,
> - gen_helper_ldst_whole *fn, DisasContext *s,
> - bool is_store)
> + uint32_t width, gen_helper_ldst_whole *fn,
> + DisasContext *s, bool is_store)
>  {
> -uint32_t evl = (s->cfg_ptr->vlen / 8) * nf / (1 << s->sew);
> +uint32_t evl = (s->cfg_ptr->vlen / 8) * nf / width;
>  TCGLabel *over = gen_new_label();
>  tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, evl, over);
>
> @@ -1153,38 +1153,42 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t 
> rs1, uint32_t nf,
>   * load and store whole register instructions ignore vtype and vl setting.
>   * Thus, we don't need to check vill bit. (Section 7.9)
>   */
> -#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF, IS_STORE)  \
> +#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF, WIDTH, IS_STORE)   \
>  static bool trans_##NAME(DisasContext *s, arg_##NAME * a) \
>  { \
>  if (require_rvv(s) && \
>  QEMU_IS_ALIGNED(a->rd, ARG_NF)) { \
> -return ldst_whole_trans(a->rd, a->rs1, ARG_NF, gen_helper_##NAME, \
> -s, IS_STORE); \
> +return ldst_whole_trans(a->rd, a->rs1, ARG_NF, WIDTH, \
> +gen_helper_##NAME, s, IS_STORE);  \
>  } \
>  return false; \
>  }
>
> -GEN_LDST_WHOLE_TRANS(vl1re8_v,  1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re16_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re32_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl1re64_v, 1, false)
> -GEN_LDST_WHOLE_TRANS(vl2re8_v,  2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re16_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re32_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl2re64_v, 2, false)
> -GEN_LDST_WHOLE_TRANS(vl4re8_v,  4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re16_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re32_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl4re64_v, 4, false)
> -GEN_LDST_WHOLE_TRANS(vl8re8_v,  8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re16_v, 8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re32_v, 8, false)
> -GEN_LDST_WHOLE_TRANS(vl8re64_v, 8, false)
> -
> -GEN_LDST_WHOLE_TRANS(vs1r_v, 1, true)
> -GEN_LDST_WHOLE_TRANS(vs2r_v, 2, true)
> -GEN_LDST_WHOLE_TRANS(vs4r_v, 4, true)
> -GEN_LDST_WHOLE_TRANS(vs8r_v, 8, true)
> +GEN_LDST_WHOLE_TRANS(vl1re8_v,  1, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl1re16_v, 1, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl1re32_v, 1, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl1re64_v, 1, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl2re8_v,  2, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl2re16_v, 2, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl2re32_v, 2, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl2re64_v, 2, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl4re8_v,  4, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl4re16_v, 4, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl4re32_v, 4, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl4re64_v, 4, 8, false)
> +GEN_LDST_WHOLE_TRANS(vl8re8_v,  8, 1, false)
> +GEN_LDST_WHOLE_TRANS(vl8re16_v, 8, 2, false)
> +GEN_LDST_WHOLE_TRANS(vl8re32_v, 8, 4, false)
> +GEN_LDST_WHOLE_TRANS(vl8re64_v, 8, 8, false)
> +
> +/*
> + * The vector whole register store instructions are encoded similar to
> + * unmasked unit-stride store of elements with EEW=8.
> + */
> +GEN_LDST_WHOLE_TRANS(vs1r_v, 1, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs2r_v, 2, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs4r_v, 4, 1, true)
> +GEN_LDST_WHOLE_TRANS(vs8r_v, 8, 1, true)
>
>  /*
>   *** Vector Integer Arithmetic Instructions
> --
> 2.34.2
>

[PATCH] crypto: make loaded property read-only

[Paolo Bonzini]

The ``loaded=on`` option in the command line or QMP ``object-add`` either had
no effect (if ``loaded`` was the last option) or caused options to be
effectively ignored as if they were not given.  The property is therefore
useless and was deprecated in 6.0; make it read-only now.

The patch is best reviewed with "-b".

Signed-off-by: Paolo Bonzini 
---
 crypto/secret_common.c  | 84 ++---
 crypto/tlscredsanon.c   | 20 ++--
 crypto/tlscredspsk.c| 20 ++--
 crypto/tlscredsx509.c   | 20 ++--
 docs/about/deprecated.rst   | 10 
 docs/about/removed-features.rst |  8 
 6 files changed, 55 insertions(+), 107 deletions(-)

diff --git a/crypto/secret_common.c b/crypto/secret_common.c
index 714a15d5e5..3441c44ca8 100644
--- a/crypto/secret_common.c
+++ b/crypto/secret_common.c
@@ -138,36 +138,44 @@ static void qcrypto_secret_decode(const uint8_t *input,
 
 
 static void
-qcrypto_secret_prop_set_loaded(Object *obj,
-   bool value,
-   Error **errp)
+qcrypto_secret_complete(UserCreatable *uc, Error **errp)
 {
-QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(obj);
+QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(uc);
 QCryptoSecretCommonClass *sec_class
-= QCRYPTO_SECRET_COMMON_GET_CLASS(obj);
+= QCRYPTO_SECRET_COMMON_GET_CLASS(uc);
 
-if (value) {
-Error *local_err = NULL;
-uint8_t *input = NULL;
-size_t inputlen = 0;
-uint8_t *output = NULL;
-size_t outputlen = 0;
+Error *local_err = NULL;
+uint8_t *input = NULL;
+size_t inputlen = 0;
+uint8_t *output = NULL;
+size_t outputlen = 0;
 
-if (sec_class->load_data) {
-sec_class->load_data(secret, &input, &inputlen, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-} else {
-error_setg(errp, "%s provides no 'load_data' method'",
- object_get_typename(obj));
+if (sec_class->load_data) {
+sec_class->load_data(secret, &input, &inputlen, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
+} else {
+error_setg(errp, "%s provides no 'load_data' method'",
+ object_get_typename(OBJECT(uc)));
+return;
+}
 
-if (secret->keyid) {
-qcrypto_secret_decrypt(secret, input, inputlen,
-   &output, &outputlen, &local_err);
+if (secret->keyid) {
+qcrypto_secret_decrypt(secret, input, inputlen,
+   &output, &outputlen, &local_err);
+g_free(input);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+input = output;
+inputlen = outputlen;
+} else {
+if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
+qcrypto_secret_decode(input, inputlen,
+  &output, &outputlen, &local_err);
 g_free(input);
 if (local_err) {
 error_propagate(errp, local_err);
@@ -175,26 +183,11 @@ qcrypto_secret_prop_set_loaded(Object *obj,
 }
 input = output;
 inputlen = outputlen;
-} else {
-if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
-qcrypto_secret_decode(input, inputlen,
-  &output, &outputlen, &local_err);
-g_free(input);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-input = output;
-inputlen = outputlen;
-}
 }
-
-secret->rawdata = input;
-secret->rawlen = inputlen;
-} else if (secret->rawdata) {
-error_setg(errp, "Cannot unload secret");
-return;
 }
+
+secret->rawdata = input;
+secret->rawlen = inputlen;
 }
 
 
@@ -268,13 +261,6 @@ qcrypto_secret_prop_get_keyid(Object *obj,
 }
 
 
-static void
-qcrypto_secret_complete(UserCreatable *uc, Error **errp)
-{
-object_property_set_bool(OBJECT(uc), "loaded", true, errp);
-}
-
-
 static void
 qcrypto_secret_finalize(Object *obj)
 {
@@ -294,7 +280,7 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
 
 object_class_property_add_bool(oc, "loaded",
qcrypto_secret_prop_get_loaded,
-   qcrypto_secret_prop_set_loaded);
+   NULL);
 object_class_property_add_enum(oc, "format",
"QCryptoSecretFormat",
&QCryptoSecretFormat_lookup,
diff --git a/crypto/tlscredsanon.

Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface

[Victor Toso]

Hi!

Sorry for taking some time to reply.

On Tue, Apr 19, 2022 at 11:12:28AM -0700, Andrea Bolognani wrote:
> On Sat, Apr 02, 2022 at 12:40:56AM +0200, Victor Toso wrote:
> > Thanks for taking a look, let me know if you have questions, ideas
> > or suggestions.
> 
> Full disclosure: I have only given the actual implementation a very
> cursory look so far, and I've focused on the generated Go API
> instead.
> 
> Overall things look pretty good.

Glad to hear.

> One concern that I have is about naming struct members: things like
> SpiceInfo.MouseMode and most others are translated from the QAPI
> schema exactly the way you'd expect them, but for example
> ChardevCommon.Logappend doesn't look quite right. Of course there's
> no way to programmatically figure out what to capitalize, but maybe
> there's room for adding this kind of information in the form of
> additional annotations or something like that? Same for the various
> structs or members that have unexpectedly-capitalized "Tls" or "Vnc"
> in them.
> 
> To be clear, I don't think the above is a blocker - just something to
> be aware of, and think about.

There was a good discussion around this with Markus so I don't
want to break it in another thread.

I'm happy that you have found those inconsistencies. I'll reply
on the other thread about it but I don't mind working towards
fixing it, either at code generator level or at QAPI level.

> My biggest concern is about the interface offered for commands.
> 
> Based on the example you have in the README and how commands are
> defined, invoking (a simplified version of) the trace-event-get-state
> command would look like
> 
>   cmd := Command{
>   Name: "trace-event-get-state",
>   Arg: TraceEventGetStateCommand{
>   Name: "qemu_memalign",
>   },
>   }
>   qmp_input, _ := json.Marshal(&cmd)
>   // qmp_input now contains
>   //   
> {"execute":"trace-event-get-state","arguments":{"name":"qemu_memalign"}}
>   // do something with it
> 
>   qmp_output :=
> ([]byte)(`{"return":{"name":"qemu_memalign","state":"disabled"}}`)
>   ret := cmd.GetReturnType()
>   _ = json.Unmarshal(qmp_output, &ret)
>   // ret is a CommandResult instance whose Value member can be cast
>   // to a TraceEventInfo struct
> 
> First of all, from an application's point of view there are way too
> many steps involved:

It can actually get worse. I've used a lot of nested struct to
define a Base type for a given Type. In Go, If you try to
initialize a Type that has a nested Struct, you'll need to use
the nested struct Type as field name and this is too verbose.

See https://github.com/golang/go/issues/29438 (merged with:
https://github.com/golang/go/issues/12854)

The main reason that I kept it is because it maps very well with
the over-the-wire protocol.

> performing this operation should really be as
> simple as
> 
>   ret, _ := qmp.TraceEventGetState("qemu_memalign")
>   // ret is a TraceEventInfo instance
> 
> That's the end state we should be working towards.
> 
> Of course that assumes that the "qmp" object knows where the
> QMP socket is, knows how to talk the QMP protocol,
> transparently deals with serializing and deserializing data...
> Plus, in some case you might want to deal with the wire
> transfer yourself in an application-specific manner. So it
> makes sense to have the basic building blocks available and
> then build the more ergonomic SDK on top of that - with only
> the first part being in scope for this series.

Right. Indeed, I thought a bit about what I want to fit into the
code generator that will reside in QEMU and what we might want to
develop on top of that.

The goal for this series really is generating the data types that
can be converted to/from QMP messages.

I completely agree with the message below: Type validation is
important at this stage.

> Even with that in mind, the current interface is IMO
> problematic because of its almost complete lack of type safety.
> Both Command.Arg and CommandResult.Value are of type Any and
> CommandBase.Name, which is used to drive the JSON unmarshal
> logic as well as ending up on the wire when executing a
> command, is just a plain string.
> 
> I think the low-level interface should look more like
> 
>   cmd := TraceEventGetStateCommand{
>   Name: "qemu_memalign",
>   }
>   qmp_input, _ := json.Marshal(&cmd)
>   // qmp_input looks the same as before

That isn't too hard to implement and I've started with this
design at first. Each QAPI Command can implement a method Name()
which returns the over-the-wire name for that Command.

I'm not yet sure if this is preferable over some other syntactic
sugar function that might be generated (this series) or the next
layer that will be on top of this.

But I agree with you that it should be improved before reaching
actual Applications.

>   qmp_output :=
> ([]byte)(`{"return":{"name":"qemu_memalign","state":"disabled"}}`)
>   ret := TraceEventInfo{}
>   _ = json.Unmarshal(qmp_output, &ret)
>   // ret is a Tr

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[yangxiaojuan]

On 2022/5/7 下午11:31, Richard Henderson wrote:

+    if (level) {
+    /* if not enable return false */
+    if (((s->enable[enable_index]) & (1 << enable_mask)) == 0) {
+    return;
+    }
+    s->coreisr[cpu][coreisr_index] |= (1 << coreisr_mask);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    } else {
+    s->coreisr[cpu][coreisr_index] &= ~(1 << coreisr_mask);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    }


This final bit, updating the cpu irq is also wrong, in that it should 
be unconditional. This is the only way that it will work for the usage 
in updating the enable mask.


I think you are not considering when the MAP registers overlap 
outputs.  For instance, if all 256 bits of EXT_IOIMap contain 0, then 
all of EXT_IOI[n*32+31 : n*32] overlap.  When that happens, you cannot 
lower the level of the cpu pin until all of the matching ioi 
interrupts are low.



Thanks, i should consider the MAP registers overlap outputs.
And i want to add 'uint32_t sw_isr_group[256 / 32]', when each bit of 
sw_isr_group[n*32+31 : n*32] is 0, then lower the level of the cpu pin.


Thanks.
Xiaojuan

[PATCH v2 00/26] block: fix coroutine_fn annotations

[Paolo Bonzini]

This is the initial result of reviving Marc-André's series at
https://patchew.org/QEMU/20170704220346.29244-1-marcandre.lur...@redhat.com/.
A lot of the patches are similar to the ones that Marc-André wrote,
but due to the changes in the code it was easier to redo them.

For nbd, the patch is on top of "nbd: mark more coroutine_fns" that
I sent a few days ago and that (AIUI) Eric has already queued; only
one function was missing, much to my surprise.

Apart from this, I also identified the following functions that
can be called both in coroutine context and outside:

- qmp_dispatch
- schedule_next_request
- nvme_get_free_req
- bdrv_create
- bdrv_remove_persistent_dirty_bitmap
- bdrv_can_store_new_dirty_bitmap
- bdrv_do_drained_begin
- bdrv_do_drained_end
- bdrv_drain_all_begin
- qcow2_open
- qcow2_has_zero_init
- bdrv_qed_open
- qio_channel_readv_full_all_eof
- qio_channel_writev_full_all

besides, of course, everything that is generated by
scripts/block-coroutine-wrapper.py.

The patches are exactly the same as v1, but I have improved the commit
messages for the "remove incorrect coroutine_fn annotations" bits.

Marc-André Lureau (3):
  9p: add missing coroutine_fn annotations
  migration: add missing coroutine_fn annotations
  test-coroutine: add missing coroutine_fn annotations

Paolo Bonzini (23):
  block: remove incorrect coroutine_fn annotations
  qcow2: remove incorrect coroutine_fn annotations
  nbd: remove incorrect coroutine_fn annotations
  coroutine: remove incorrect coroutine_fn annotations
  blkdebug: add missing coroutine_fn annotations
  blkverify: add missing coroutine_fn annotations
  block: add missing coroutine_fn annotations
  file-posix: add missing coroutine_fn annotations
  iscsi: add missing coroutine_fn annotations
  nbd: add missing coroutine_fn annotations
  nfs: add missing coroutine_fn annotations
  nvme: add missing coroutine_fn annotations
  parallels: add missing coroutine_fn annotations
  qcow2: add missing coroutine_fn annotations
  copy-before-write: add missing coroutine_fn annotations
  curl: add missing coroutine_fn annotations
  qed: add missing coroutine_fn annotations
  quorum: add missing coroutine_fn annotations
  throttle: add missing coroutine_fn annotations
  vmdk: add missing coroutine_fn annotations
  job: add missing coroutine_fn annotations
  coroutine-lock: add missing coroutine_fn annotations
  raw-format: add missing coroutine_fn annotations

 block/blkdebug.c| 14 +++---
 block/blkverify.c   |  2 +-
 block/block-backend.c   | 26 +-
 block/copy-before-write.c   |  8 
 block/curl.c|  2 +-
 block/file-posix.c  |  2 +-
 block/io.c  | 24 
 block/iscsi.c   |  2 +-
 block/nbd.c | 10 +-
 block/nfs.c |  2 +-
 block/nvme.c|  5 +++--
 block/parallels.c   |  5 +++--
 block/qcow2-cluster.c   | 18 +-
 block/qcow2-refcount.c  |  6 +++---
 block/qcow2.c   |  4 ++--
 block/qcow2.h   | 18 +-
 block/qed.c |  4 ++--
 block/quorum.c  | 35 ++-
 block/raw-format.c  |  2 +-
 block/throttle.c|  2 +-
 block/vmdk.c| 20 ++--
 hw/9pfs/9p.h|  9 ++---
 include/block/nbd.h |  2 +-
 include/qemu/coroutine.h|  2 +-
 include/qemu/job.h  |  2 +-
 job.c   |  2 +-
 migration/migration.c   |  3 ++-
 tests/unit/test-coroutine.c |  2 +-
 util/qemu-coroutine-lock.c  | 14 +++---
 util/qemu-coroutine.c   |  2 +-
 30 files changed, 128 insertions(+), 121 deletions(-)

-- 
2.35.1

[PATCH v2 04/26] coroutine: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

qemu_coroutine_get_aio_context inspects a coroutine, but it does
not have to be called from the coroutine itself (or from any
coroutine).

Signed-off-by: Paolo Bonzini 
---
 include/qemu/coroutine.h | 2 +-
 util/qemu-coroutine.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 284571badb..2d9211faff 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -92,7 +92,7 @@ void coroutine_fn qemu_coroutine_yield(void);
 /**
  * Get the AioContext of the given coroutine
  */
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co);
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co);
 
 /**
  * Get the currently executing coroutine
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index f3e8300c8d..32c7ae8f21 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -207,7 +207,7 @@ bool qemu_coroutine_entered(Coroutine *co)
 return co->caller;
 }
 
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co)
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co)
 {
 return co->ctx;
 }
-- 
2.35.1

[PATCH] rng: make opened property read-only

[Paolo Bonzini]

The ``opened=on`` option in the command line or QMP ``object-add`` either had
no effect (if ``opened`` was the last option) or caused errors.  The property
is therefore useless and was deprecated in 6.0; make it read-only now.

Based-on: <20220509101907.212687-1-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 backends/rng.c  | 18 ++
 docs/about/deprecated.rst   |  9 -
 docs/about/removed-features.rst |  7 +++
 3 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/backends/rng.c b/backends/rng.c
index 3757b04485..6c7bf64426 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -48,24 +48,10 @@ static bool rng_backend_prop_get_opened(Object *obj, Error 
**errp)
 
 static void rng_backend_complete(UserCreatable *uc, Error **errp)
 {
-object_property_set_bool(OBJECT(uc), "opened", true, errp);
-}
-
-static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp)
-{
-RngBackend *s = RNG_BACKEND(obj);
+RngBackend *s = RNG_BACKEND(uc);
 RngBackendClass *k = RNG_BACKEND_GET_CLASS(s);
 Error *local_err = NULL;
 
-if (value == s->opened) {
-return;
-}
-
-if (!value && s->opened) {
-error_setg(errp, QERR_PERMISSION_DENIED);
-return;
-}
-
 if (k->opened) {
 k->opened(s, &local_err);
 if (local_err) {
@@ -122,7 +108,7 @@ static void rng_backend_class_init(ObjectClass *oc, void 
*data)
 
 object_class_property_add_bool(oc, "opened",
rng_backend_prop_get_opened,
-   rng_backend_prop_set_opened);
+   NULL);
 }
 
 static const TypeInfo rng_backend_info = {
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 2feb0c506c..25bc92dc65 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -90,15 +90,6 @@ the process listing. This is replaced by the new 
``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
-``opened`` property of ``rng-*`` objects (since 6.0)
-
-
-The only effect of specifying ``opened=on`` in the command line or QMP
-``object-add`` is that the device is opened immediately, possibly before all
-other options have been processed.  This will either have no effect (if
-``opened`` was the last option) or cause errors.  The property is therefore
-useless and should not be specified.
-
 ``-display sdl,window_close=...`` (since 6.1)
 '
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 2032608314..715b5f4f4d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -749,6 +749,13 @@ no effect (if ``loaded`` was the last option) or caused 
options to be
 effectively ignored as if they were not given.  The property is therefore
 useless and should simply be removed.
 
+``opened`` property of ``rng-*`` objects (removed in 7.1)
+'
+
+The ``opened=on`` option in the command line or QMP ``object-add`` either had
+no effect (if ``opened`` was the last option) or caused errors.  The property
+is therefore useless and should not be specified.
+
 Block devices
 -
 
-- 
2.35.1

[PATCH v2 02/26] qcow2: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

This is incorrect because qcow2_mark_clean() calls qcow2_flush_caches().
qcow2_mark_clean() is called from non-coroutine context in
qcow2_inactivate() and qcow2_close().

Signed-off-by: Paolo Bonzini 
---
 block/qcow2-refcount.c | 4 ++--
 block/qcow2.h  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ed0ecfaa89..404d56e258 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1206,7 +1206,7 @@ void qcow2_free_any_cluster(BlockDriverState *bs, 
uint64_t l2_entry,
 }
 }
 
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
+int qcow2_write_caches(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
 int ret;
@@ -1226,7 +1226,7 @@ int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
 return 0;
 }
 
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs)
+int qcow2_flush_caches(BlockDriverState *bs)
 {
 int ret = qcow2_write_caches(bs);
 if (ret < 0) {
diff --git a/block/qcow2.h b/block/qcow2.h
index ba436a8d0d..c8d9e8ea79 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -874,8 +874,8 @@ void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t 
l2_entry,
 int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int64_t l1_table_offset, int l1_size, int addend);
 
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs);
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs);
+int qcow2_flush_caches(BlockDriverState *bs);
+int qcow2_write_caches(BlockDriverState *bs);
 int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
   BdrvCheckMode fix);
 
-- 
2.35.1

[PATCH v2 05/26] blkdebug: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/blkdebug.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index bbf2948703..a93ba61487 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -587,8 +587,8 @@ out:
 return ret;
 }
 
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-  BlkdebugIOType iotype)
+static int coroutine_fn rule_check(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
+   BlkdebugIOType iotype)
 {
 BDRVBlkdebugState *s = bs->opaque;
 BlkdebugRule *rule = NULL;
@@ -672,7 +672,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, 
int64_t bytes,
 return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 }
 
-static int blkdebug_co_flush(BlockDriverState *bs)
+static int coroutine_fn blkdebug_co_flush(BlockDriverState *bs)
 {
 int err = rule_check(bs, 0, 0, BLKDEBUG_IO_TYPE_FLUSH);
 
@@ -791,7 +791,7 @@ static void blkdebug_close(BlockDriverState *bs)
 }
 
 /* Called with lock held.  */
-static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
+static void coroutine_fn suspend_request(BlockDriverState *bs, BlkdebugRule 
*rule)
 {
 BDRVBlkdebugState *s = bs->opaque;
 BlkdebugSuspendedReq *r;
@@ -810,8 +810,8 @@ static void suspend_request(BlockDriverState *bs, 
BlkdebugRule *rule)
 }
 
 /* Called with lock held.  */
-static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
- int *action_count, int *new_state)
+static void coroutine_fn process_rule(BlockDriverState *bs, struct 
BlkdebugRule *rule,
+  int *action_count, int *new_state)
 {
 BDRVBlkdebugState *s = bs->opaque;
 
@@ -840,7 +840,7 @@ static void process_rule(BlockDriverState *bs, struct 
BlkdebugRule *rule,
 }
 }
 
-static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+static void coroutine_fn blkdebug_debug_event(BlockDriverState *bs, 
BlkdebugEvent event)
 {
 BDRVBlkdebugState *s = bs->opaque;
 struct BlkdebugRule *rule, *next;
-- 
2.35.1

[PATCH v2 03/26] nbd: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

nbd_co_establish_connection_cancel() cancels a coroutine but is not called
from coroutine context itself, for example in nbd_cancel_in_flight()
and in timer callbacks reconnect_delay_timer_cb() and open_timer_cb().

Signed-off-by: Paolo Bonzini 
---
 include/block/nbd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index a98eb665da..5c3710fa52 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -423,6 +423,6 @@ QIOChannel *coroutine_fn
 nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
 bool blocking, Error **errp);
 
-void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection 
*conn);
+void nbd_co_establish_connection_cancel(NBDClientConnection *conn);
 
 #endif
-- 
2.35.1

[PATCH v2 09/26] iscsi: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index d707d0b354..b33eeec794 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -290,7 +290,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
 }
 }
 
-static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask 
*iTask)
+static void coroutine_fn iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct 
IscsiTask *iTask)
 {
 *iTask = (struct IscsiTask) {
 .co = qemu_coroutine_self(),
-- 
2.35.1

[PATCH v2 07/26] block: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/block-backend.c | 18 +-
 block/io.c| 24 
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index fedf2eca83..52009b8949 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1413,8 +1413,8 @@ typedef struct BlkRwCo {
 BdrvRequestFlags flags;
 } BlkRwCo;
 
-int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-  int64_t bytes, BdrvRequestFlags flags)
+int coroutine_fn blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+   int64_t bytes, BdrvRequestFlags flags)
 {
 IO_OR_GS_CODE();
 return blk_pwritev_part(blk, offset, bytes, NULL, 0,
@@ -1534,7 +1534,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
int64_t offset,
 return &acb->common;
 }
 
-static void blk_aio_read_entry(void *opaque)
+static void coroutine_fn blk_aio_read_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = &acb->rwco;
@@ -1546,7 +1546,7 @@ static void blk_aio_read_entry(void *opaque)
 blk_aio_complete(acb);
 }
 
-static void blk_aio_write_entry(void *opaque)
+static void coroutine_fn blk_aio_write_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = &acb->rwco;
@@ -1580,8 +1580,8 @@ int blk_pread(BlockBackend *blk, int64_t offset, void 
*buf, int bytes)
 return ret < 0 ? ret : bytes;
 }
 
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
-   BdrvRequestFlags flags)
+int coroutine_fn blk_pwrite(BlockBackend *blk, int64_t offset, const void 
*buf, int bytes,
+BdrvRequestFlags flags)
 {
 int ret;
 QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
@@ -1681,7 +1681,7 @@ int blk_ioctl(BlockBackend *blk, unsigned long int req, 
void *buf)
 return ret;
 }
 
-static void blk_aio_ioctl_entry(void *opaque)
+static void coroutine_fn blk_aio_ioctl_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = &acb->rwco;
@@ -1715,7 +1715,7 @@ blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, 
int64_t bytes)
 return bdrv_co_pdiscard(blk->root, offset, bytes);
 }
 
-static void blk_aio_pdiscard_entry(void *opaque)
+static void coroutine_fn blk_aio_pdiscard_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = &acb->rwco;
@@ -1771,7 +1771,7 @@ int coroutine_fn blk_co_do_flush(BlockBackend *blk)
 return bdrv_co_flush(blk_bs(blk));
 }
 
-static void blk_aio_flush_entry(void *opaque)
+static void coroutine_fn blk_aio_flush_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = &acb->rwco;
diff --git a/block/io.c b/block/io.c
index 9769ec53b0..7db9be3c03 100644
--- a/block/io.c
+++ b/block/io.c
@@ -751,7 +751,7 @@ void bdrv_drain_all(void)
  *
  * This function should be called when a tracked request is completing.
  */
-static void tracked_request_end(BdrvTrackedRequest *req)
+static void coroutine_fn tracked_request_end(BdrvTrackedRequest *req)
 {
 if (req->serialising) {
 qatomic_dec(&req->bs->serialising_in_flight);
@@ -766,11 +766,11 @@ static void tracked_request_end(BdrvTrackedRequest *req)
 /**
  * Add an active request to the tracked requests list
  */
-static void tracked_request_begin(BdrvTrackedRequest *req,
-  BlockDriverState *bs,
-  int64_t offset,
-  int64_t bytes,
-  enum BdrvTrackedRequestType type)
+static void coroutine_fn tracked_request_begin(BdrvTrackedRequest *req,
+   BlockDriverState *bs,
+   int64_t offset,
+   int64_t bytes,
+   enum BdrvTrackedRequestType 
type)
 {
 bdrv_check_request(offset, bytes, &error_abort);
 
@@ -809,7 +809,7 @@ static bool tracked_request_overlaps(BdrvTrackedRequest 
*req,
 }
 
 /* Called with self->bs->reqs_lock held */
-static BdrvTrackedRequest *
+static coroutine_fn BdrvTrackedRequest *
 bdrv_find_conflicting_request(BdrvTrackedRequest *self)
 {
 BdrvTrackedRequest *req;
@@ -1704,10 +1704,10 @@ static bool bdrv_init_padding(BlockDriverState *bs,
 return true;
 }
 
-static int bdrv_padding_rmw_read(BdrvChild *child,
- BdrvTrackedRequest *req,
- BdrvRequestPadding *pad,
- bool zero_middle)
+static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
+  BdrvTrackedRequest *req,
+  BdrvRequestPadding *pad,
+  bool zero_middle)
 {
 QEMUIOVector local_qiov;
 BlockDriverState *bs = child

[PATCH v2 06/26] blkverify: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/blkverify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index e4a37af3b2..020b1ae7b6 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -258,7 +258,7 @@ blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, 
int64_t bytes,
 return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
 }
 
-static int blkverify_co_flush(BlockDriverState *bs)
+static int coroutine_fn blkverify_co_flush(BlockDriverState *bs)
 {
 BDRVBlkverifyState *s = bs->opaque;
 
-- 
2.35.1

[PATCH v2 01/26] block: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

This is incorrect because blk_pwritev_part() is called by
blk_pwrite_zeroes() and blk_pwrite(), neither of which has to be called
from a coroutine.

Signed-off-by: Paolo Bonzini 
---
 block/block-backend.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index e0e1aff4b1..fedf2eca83 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1391,10 +1391,10 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, 
int64_t offset,
 return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
 }
 
-static int coroutine_fn blk_pwritev_part(BlockBackend *blk, int64_t offset,
- int64_t bytes,
- QEMUIOVector *qiov, size_t 
qiov_offset,
- BdrvRequestFlags flags)
+static int blk_pwritev_part(BlockBackend *blk, int64_t offset,
+int64_t bytes,
+QEMUIOVector *qiov, size_t qiov_offset,
+BdrvRequestFlags flags)
 {
 int ret;
 
-- 
2.35.1

[PATCH v2 13/26] parallels: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/parallels.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 8879b7027a..bee2ff023d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -165,8 +165,9 @@ static int64_t block_status(BDRVParallelsState *s, int64_t 
sector_num,
 return start_off;
 }
 
-static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, int *pnum)
+static coroutine_fn int64_t allocate_clusters(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
 {
 int ret = 0;
 BDRVParallelsState *s = bs->opaque;
-- 
2.35.1

[PATCH v2 11/26] nfs: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index 444c40b458..596ebe98cb 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -223,7 +223,7 @@ static void nfs_process_write(void *arg)
 qemu_mutex_unlock(&client->mutex);
 }
 
-static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
+static void coroutine_fn nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
 {
 *task = (NFSRPC) {
 .co = qemu_coroutine_self(),
-- 
2.35.1

[PATCH v2 10/26] nbd: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/nbd.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 6085ab1d2c..fe913a6db4 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -983,11 +983,11 @@ static void nbd_iter_request_error(NBDReplyChunkIter 
*iter, int ret)
  * nbd_reply_chunk_iter_receive
  * The pointer stored in @payload requires g_free() to free it.
  */
-static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
- NBDReplyChunkIter *iter,
- uint64_t handle,
- QEMUIOVector *qiov, NBDReply *reply,
- void **payload)
+static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
+  NBDReplyChunkIter *iter,
+  uint64_t handle,
+  QEMUIOVector *qiov, 
NBDReply *reply,
+  void **payload)
 {
 int ret, request_ret;
 NBDReply local_reply;
-- 
2.35.1

[PATCH v2 08/26] file-posix: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 48cd096624..76eea8d350 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2158,7 +2158,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #endif
 }
 
-static int raw_co_flush_to_disk(BlockDriverState *bs)
+static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 {
 BDRVRawState *s = bs->opaque;
 RawPosixAIOData acb;
-- 
2.35.1

[PATCH v2 18/26] quorum: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/quorum.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index f33f30d36b..5ff69d7443 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -161,11 +161,10 @@ static bool quorum_64bits_compare(QuorumVoteValue *a, 
QuorumVoteValue *b)
 return a->l == b->l;
 }
 
-static QuorumAIOCB *quorum_aio_get(BlockDriverState *bs,
-   QEMUIOVector *qiov,
-   uint64_t offset,
-   uint64_t bytes,
-   int flags)
+static QuorumAIOCB *coroutine_fn quorum_aio_get(BlockDriverState *bs,
+QEMUIOVector *qiov,
+uint64_t offset, uint64_t 
bytes,
+int flags)
 {
 BDRVQuorumState *s = bs->opaque;
 QuorumAIOCB *acb = g_new(QuorumAIOCB, 1);
@@ -273,7 +272,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
 }
 }
 
-static void quorum_rewrite_entry(void *opaque)
+static void coroutine_fn quorum_rewrite_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -574,7 +573,7 @@ free_exit:
 quorum_free_vote_list(&acb->votes);
 }
 
-static void read_quorum_children_entry(void *opaque)
+static void coroutine_fn read_quorum_children_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -602,7 +601,7 @@ static void read_quorum_children_entry(void *opaque)
 }
 }
 
-static int read_quorum_children(QuorumAIOCB *acb)
+static int coroutine_fn read_quorum_children(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
 int i;
@@ -643,7 +642,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
 return acb->vote_ret;
 }
 
-static int read_fifo_child(QuorumAIOCB *acb)
+static int coroutine_fn read_fifo_child(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
 int n, ret;
@@ -664,8 +663,9 @@ static int read_fifo_child(QuorumAIOCB *acb)
 return ret;
 }
 
-static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t 
bytes,
-QEMUIOVector *qiov, BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_preadv(BlockDriverState *bs,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags 
flags)
 {
 BDRVQuorumState *s = bs->opaque;
 QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
@@ -684,7 +684,7 @@ static int quorum_co_preadv(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
 return ret;
 }
 
-static void write_quorum_entry(void *opaque)
+static void coroutine_fn write_quorum_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -715,9 +715,9 @@ static void write_quorum_entry(void *opaque)
 }
 }
 
-static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
- int64_t bytes, QEMUIOVector *qiov,
- BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
+  int64_t bytes, QEMUIOVector *qiov,
+  BdrvRequestFlags flags)
 {
 BDRVQuorumState *s = bs->opaque;
 QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
@@ -746,8 +746,9 @@ static int quorum_co_pwritev(BlockDriverState *bs, int64_t 
offset,
 return ret;
 }
 
-static int quorum_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
-   int64_t bytes, BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_pwrite_zeroes(BlockDriverState *bs,
+int64_t offset, int64_t bytes,
+BdrvRequestFlags flags)
 
 {
 return quorum_co_pwritev(bs, offset, bytes, NULL,
-- 
2.35.1

[PATCH v2 15/26] copy-before-write: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/copy-before-write.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a8a06fdc09..5ad9693b13 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -165,9 +165,9 @@ static int coroutine_fn cbw_co_flush(BlockDriverState *bs)
  * It's guaranteed that guest writes will not interact in the region until
  * cbw_snapshot_read_unlock() called.
  */
-static BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
-int64_t offset, int64_t bytes,
-int64_t *pnum, BdrvChild **file)
+static coroutine_fn BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
+ int64_t offset, int64_t 
bytes,
+ int64_t *pnum, BdrvChild 
**file)
 {
 BDRVCopyBeforeWriteState *s = bs->opaque;
 BlockReq *req = g_new(BlockReq, 1);
@@ -197,7 +197,7 @@ static BlockReq *cbw_snapshot_read_lock(BlockDriverState 
*bs,
 return req;
 }
 
-static void cbw_snapshot_read_unlock(BlockDriverState *bs, BlockReq *req)
+static coroutine_fn void cbw_snapshot_read_unlock(BlockDriverState *bs, 
BlockReq *req)
 {
 BDRVCopyBeforeWriteState *s = bs->opaque;
 
-- 
2.35.1

[PATCH v2 21/26] job: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 include/qemu/job.h | 2 +-
 job.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index c105b31076..397ac39608 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -436,7 +436,7 @@ void coroutine_fn job_pause_point(Job *job);
  *
  * Yield the job coroutine.
  */
-void job_yield(Job *job);
+void coroutine_fn job_yield(Job *job);
 
 /**
  * @job: The job that calls the function.
diff --git a/job.c b/job.c
index 075c6f3a20..20f0d8b2cd 100644
--- a/job.c
+++ b/job.c
@@ -525,7 +525,7 @@ void coroutine_fn job_pause_point(Job *job)
 }
 }
 
-void job_yield(Job *job)
+void coroutine_fn job_yield(Job *job)
 {
 assert(job->busy);
 
-- 
2.35.1

[PATCH v2 14/26] qcow2: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/qcow2-cluster.c  | 18 +-
 block/qcow2-refcount.c |  2 +-
 block/qcow2.c  |  4 ++--
 block/qcow2.h  | 14 +++---
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 20a16ba6ee..37fc7b905a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -884,7 +884,7 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState 
*bs,
 return 0;
 }
 
-static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 {
 BDRVQcow2State *s = bs->opaque;
 Qcow2COWRegion *start = &m->cow_start;
@@ -1024,7 +1024,7 @@ fail:
 return ret;
 }
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int coroutine_fn qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta 
*m)
 {
 BDRVQcow2State *s = bs->opaque;
 int i, j = 0, l2_index, ret;
@@ -1397,8 +1397,8 @@ static int count_single_write_clusters(BlockDriverState 
*bs, int nb_clusters,
  *   information on cluster allocation may be invalid now. The caller
  *   must start over anyway, so consider *cur_bytes undefined.
  */
-static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
-uint64_t *cur_bytes, QCowL2Meta **m)
+static int coroutine_fn handle_dependencies(BlockDriverState *bs, uint64_t 
guest_offset,
+uint64_t *cur_bytes, QCowL2Meta 
**m)
 {
 BDRVQcow2State *s = bs->opaque;
 QCowL2Meta *old_alloc;
@@ -1772,9 +1772,9 @@ out:
  *
  * Return 0 on success and -errno in error cases
  */
-int qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
-unsigned int *bytes, uint64_t *host_offset,
-QCowL2Meta **m)
+int coroutine_fn qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
+ unsigned int *bytes, uint64_t 
*host_offset,
+ QCowL2Meta **m)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t start, remaining;
@@ -2105,8 +2105,8 @@ out:
 return ret;
 }
 
-int qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, int flags)
+int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t 
offset,
+  uint64_t bytes, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t end_offset = offset + bytes;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 404d56e258..17be4425f2 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3704,7 +3704,7 @@ int64_t qcow2_get_last_cluster(BlockDriverState *bs, 
int64_t size)
 return -EIO;
 }
 
-int qcow2_detect_metadata_preallocation(BlockDriverState *bs)
+int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
 int64_t i, end_cluster, cluster_count = 0, threshold;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4f5e6440fb..62cb153987 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2438,7 +2438,7 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
  * Return 1 if the COW regions read as zeroes, 0 if not, < 0 on error.
  * Note that returning 0 does not guarantee non-zero data.
  */
-static int is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
 {
 /*
  * This check is designed for optimization shortcut so it must be
@@ -2456,7 +2456,7 @@ static int is_zero_cow(BlockDriverState *bs, QCowL2Meta 
*m)
 m->cow_end.nb_bytes);
 }
 
-static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
+static int coroutine_fn handle_alloc_space(BlockDriverState *bs, QCowL2Meta 
*l2meta)
 {
 BDRVQcow2State *s = bs->opaque;
 QCowL2Meta *m;
diff --git a/block/qcow2.h b/block/qcow2.h
index c8d9e8ea79..36495d9051 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -895,7 +895,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int 
refcount_order,
 void *cb_opaque, Error **errp);
 int qcow2_shrink_reftable(BlockDriverState *bs);
 int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);
-int qcow2_detect_metadata_preallocation(BlockDriverState *bs);
+int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs);
 
 /* qcow2-cluster.c functions */
 int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
@@ -908,9 +908,9 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
   unsigned int *bytes, uint64_t *host_offset,
   QCow2SubclusterType *subcluster_type);
-int qcow2_alloc_host_offset(BlockDriverState *bs, uin

[PATCH v2 12/26] nvme: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/nvme.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 01fb28aa63..6519697e40 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1234,8 +1234,9 @@ static inline bool nvme_qiov_aligned(BlockDriverState *bs,
 return true;
 }
 
-static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-   QEMUIOVector *qiov, bool is_write, int flags)
+static coroutine_fn int nvme_co_prw(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, bool is_write, int 
flags)
 {
 BDRVNVMeState *s = bs->opaque;
 int r;
-- 
2.35.1

[PATCH v2 17/26] qed: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/qed.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index f34d9a3ac1..208128d679 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -259,7 +259,7 @@ static CachedL2Table *qed_new_l2_table(BDRVQEDState *s)
 return l2_table;
 }
 
-static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
+static bool coroutine_fn qed_plug_allocating_write_reqs(BDRVQEDState *s)
 {
 qemu_co_mutex_lock(&s->table_lock);
 
@@ -278,7 +278,7 @@ static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
 return true;
 }
 
-static void qed_unplug_allocating_write_reqs(BDRVQEDState *s)
+static void coroutine_fn qed_unplug_allocating_write_reqs(BDRVQEDState *s)
 {
 qemu_co_mutex_lock(&s->table_lock);
 assert(s->allocating_write_reqs_plugged);
-- 
2.35.1

[PATCH v2 23/26] raw-format: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/raw-format.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 69fd650eaf..45440345b6 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -411,7 +411,7 @@ static void raw_lock_medium(BlockDriverState *bs, bool 
locked)
 bdrv_lock_medium(bs->file->bs, locked);
 }
 
-static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+static int coroutine_fn raw_co_ioctl(BlockDriverState *bs, unsigned long int 
req, void *buf)
 {
 BDRVRawState *s = bs->opaque;
 if (s->offset || s->has_size) {
-- 
2.35.1

[PATCH v2 16/26] curl: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/curl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 1e0f609579..cba4c4cac7 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -855,7 +855,7 @@ out_noclean:
 return -EINVAL;
 }
 
-static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
+static void coroutine_fn curl_setup_preadv(BlockDriverState *bs, CURLAIOCB 
*acb)
 {
 CURLState *state;
 int running;
-- 
2.35.1

[PATCH v2 26/26] test-coroutine: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Message-Id: <20170704220346.29244-4-marcandre.lur...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 tests/unit/test-coroutine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index aa77a3bcb3..e16b80c245 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -610,7 +610,7 @@ static void perf_baseline(void)
 g_test_message("Function call %u iterations: %f s", maxcycles, duration);
 }
 
-static __attribute__((noinline)) void perf_cost_func(void *opaque)
+static __attribute__((noinline)) void coroutine_fn perf_cost_func(void *opaque)
 {
 qemu_coroutine_yield();
 }
-- 
2.35.1

[PATCH v2 24/26] 9p: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Acked-by: Greg Kurz 
Signed-off-by: Paolo Bonzini 
---
 hw/9pfs/9p.h | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 994f952600..a523ac34a9 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -424,21 +424,24 @@ typedef struct V9fsGetlock
 extern int open_fd_hw;
 extern int total_open_fd;
 
-static inline void v9fs_path_write_lock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_write_lock(V9fsState *s)
 {
 if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
 qemu_co_rwlock_wrlock(&s->rename_lock);
 }
 }
 
-static inline void v9fs_path_read_lock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_read_lock(V9fsState *s)
 {
 if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
 qemu_co_rwlock_rdlock(&s->rename_lock);
 }
 }
 
-static inline void v9fs_path_unlock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_unlock(V9fsState *s)
 {
 if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
 qemu_co_rwlock_unlock(&s->rename_lock);
-- 
2.35.1

[PATCH v2 22/26] coroutine-lock: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 util/qemu-coroutine-lock.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 2669403839..ec55490b52 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -144,7 +144,7 @@ typedef struct CoWaitRecord {
 QSLIST_ENTRY(CoWaitRecord) next;
 } CoWaitRecord;
 
-static void push_waiter(CoMutex *mutex, CoWaitRecord *w)
+static void coroutine_fn push_waiter(CoMutex *mutex, CoWaitRecord *w)
 {
 w->co = qemu_coroutine_self();
 QSLIST_INSERT_HEAD_ATOMIC(&mutex->from_push, w, next);
@@ -341,7 +341,7 @@ void qemu_co_rwlock_init(CoRwlock *lock)
 }
 
 /* Releases the internal CoMutex.  */
-static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
+static void coroutine_fn qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
 {
 CoRwTicket *tkt = QSIMPLEQ_FIRST(&lock->tickets);
 Coroutine *co = NULL;
@@ -374,7 +374,7 @@ static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
 }
 }
 
-void qemu_co_rwlock_rdlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock)
 {
 Coroutine *self = qemu_coroutine_self();
 
@@ -399,7 +399,7 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock)
 self->locks_held++;
 }
 
-void qemu_co_rwlock_unlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock)
 {
 Coroutine *self = qemu_coroutine_self();
 
@@ -417,7 +417,7 @@ void qemu_co_rwlock_unlock(CoRwlock *lock)
 qemu_co_rwlock_maybe_wake_one(lock);
 }
 
-void qemu_co_rwlock_downgrade(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_downgrade(CoRwlock *lock)
 {
 qemu_co_mutex_lock(&lock->mutex);
 assert(lock->owners == -1);
@@ -427,7 +427,7 @@ void qemu_co_rwlock_downgrade(CoRwlock *lock)
 qemu_co_rwlock_maybe_wake_one(lock);
 }
 
-void qemu_co_rwlock_wrlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock)
 {
 Coroutine *self = qemu_coroutine_self();
 
@@ -447,7 +447,7 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock)
 self->locks_held++;
 }
 
-void qemu_co_rwlock_upgrade(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_upgrade(CoRwlock *lock)
 {
 qemu_co_mutex_lock(&lock->mutex);
 assert(lock->owners > 0);
-- 
2.35.1

[PATCH v2 19/26] throttle: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/throttle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/throttle.c b/block/throttle.c
index 6e8d52fa24..ddd450593a 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -162,7 +162,7 @@ static int coroutine_fn 
throttle_co_pwritev_compressed(BlockDriverState *bs,
BDRV_REQ_WRITE_COMPRESSED);
 }
 
-static int throttle_co_flush(BlockDriverState *bs)
+static int coroutine_fn throttle_co_flush(BlockDriverState *bs)
 {
 return bdrv_co_flush(bs->file->bs);
 }
-- 
2.35.1

[PATCH v2 25/26] migration: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Reviewed-by: Juan Quintela 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
---
 migration/migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 5a31b23bd6..e4ccfb9496 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -565,7 +565,8 @@ static void process_incoming_migration_bh(void *opaque)
 migration_incoming_state_destroy();
 }
 
-static void process_incoming_migration_co(void *opaque)
+static void coroutine_fn
+process_incoming_migration_co(void *opaque)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
 PostcopyState ps;
-- 
2.35.1

Re: [PATCH] crypto: make loaded property read-only

[Daniel P . Berrangé]

On Mon, May 09, 2022 at 12:19:07PM +0200, Paolo Bonzini wrote:
> The ``loaded=on`` option in the command line or QMP ``object-add`` either had
> no effect (if ``loaded`` was the last option) or caused options to be
> effectively ignored as if they were not given.  The property is therefore
> useless and was deprecated in 6.0; make it read-only now.

Why read-only, as opposed to deleting it entirely ? Unless I'm missing
something, nothing will read the property either

> 
> The patch is best reviewed with "-b".
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  crypto/secret_common.c  | 84 ++---
>  crypto/tlscredsanon.c   | 20 ++--
>  crypto/tlscredspsk.c| 20 ++--
>  crypto/tlscredsx509.c   | 20 ++--
>  docs/about/deprecated.rst   | 10 
>  docs/about/removed-features.rst |  8 
>  6 files changed, 55 insertions(+), 107 deletions(-)
> 
> diff --git a/crypto/secret_common.c b/crypto/secret_common.c
> index 714a15d5e5..3441c44ca8 100644
> --- a/crypto/secret_common.c
> +++ b/crypto/secret_common.c
> @@ -138,36 +138,44 @@ static void qcrypto_secret_decode(const uint8_t *input,
>  
>  
>  static void
> -qcrypto_secret_prop_set_loaded(Object *obj,
> -   bool value,
> -   Error **errp)
> +qcrypto_secret_complete(UserCreatable *uc, Error **errp)
>  {
> -QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(obj);
> +QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(uc);
>  QCryptoSecretCommonClass *sec_class
> -= QCRYPTO_SECRET_COMMON_GET_CLASS(obj);
> += QCRYPTO_SECRET_COMMON_GET_CLASS(uc);
>  
> -if (value) {
> -Error *local_err = NULL;
> -uint8_t *input = NULL;
> -size_t inputlen = 0;
> -uint8_t *output = NULL;
> -size_t outputlen = 0;
> +Error *local_err = NULL;
> +uint8_t *input = NULL;
> +size_t inputlen = 0;
> +uint8_t *output = NULL;
> +size_t outputlen = 0;
>  
> -if (sec_class->load_data) {
> -sec_class->load_data(secret, &input, &inputlen, &local_err);
> -if (local_err) {
> -error_propagate(errp, local_err);
> -return;
> -}
> -} else {
> -error_setg(errp, "%s provides no 'load_data' method'",
> - object_get_typename(obj));
> +if (sec_class->load_data) {
> +sec_class->load_data(secret, &input, &inputlen, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
>  return;
>  }
> +} else {
> +error_setg(errp, "%s provides no 'load_data' method'",
> + object_get_typename(OBJECT(uc)));
> +return;
> +}
>  
> -if (secret->keyid) {
> -qcrypto_secret_decrypt(secret, input, inputlen,
> -   &output, &outputlen, &local_err);
> +if (secret->keyid) {
> +qcrypto_secret_decrypt(secret, input, inputlen,
> +   &output, &outputlen, &local_err);
> +g_free(input);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +input = output;
> +inputlen = outputlen;
> +} else {
> +if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
> +qcrypto_secret_decode(input, inputlen,
> +  &output, &outputlen, &local_err);
>  g_free(input);
>  if (local_err) {
>  error_propagate(errp, local_err);
> @@ -175,26 +183,11 @@ qcrypto_secret_prop_set_loaded(Object *obj,
>  }
>  input = output;
>  inputlen = outputlen;
> -} else {
> -if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
> -qcrypto_secret_decode(input, inputlen,
> -  &output, &outputlen, &local_err);
> -g_free(input);
> -if (local_err) {
> -error_propagate(errp, local_err);
> -return;
> -}
> -input = output;
> -inputlen = outputlen;
> -}
>  }
> -
> -secret->rawdata = input;
> -secret->rawlen = inputlen;
> -} else if (secret->rawdata) {
> -error_setg(errp, "Cannot unload secret");
> -return;
>  }
> +
> +secret->rawdata = input;
> +secret->rawlen = inputlen;
>  }
>  
>  
> @@ -268,13 +261,6 @@ qcrypto_secret_prop_get_keyid(Object *obj,
>  }
>  
>  
> -static void
> -qcrypto_secret_complete(UserCreatable *uc, Error **errp)
> -{
> -object_property_set_bool(OBJECT(uc), "loaded", true, errp);
> -}
> -
> -
>  static void
>  qcrypto_secret_finalize(Object *obj)
>  {
> @@ -294,7 +280,7 @@ qcrypto_secret_class_init(ObjectClass *oc

[PATCH v2 20/26] vmdk: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 block/vmdk.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 38e5ab3806..2c7f1858f9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1793,10 +1793,10 @@ static int coroutine_fn 
vmdk_co_block_status(BlockDriverState *bs,
 return ret;
 }
 
-static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
-int64_t offset_in_cluster, QEMUIOVector *qiov,
-uint64_t qiov_offset, uint64_t n_bytes,
-uint64_t offset)
+static int coroutine_fn vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
+ int64_t offset_in_cluster, 
QEMUIOVector *qiov,
+ uint64_t qiov_offset, uint64_t 
n_bytes,
+ uint64_t offset)
 {
 int ret;
 VmdkGrainMarker *data = NULL;
@@ -1874,9 +1874,9 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 return ret;
 }
 
-static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
-int64_t offset_in_cluster, QEMUIOVector *qiov,
-int bytes)
+static int coroutine_fn vmdk_read_extent(VmdkExtent *extent, int64_t 
cluster_offset,
+int64_t offset_in_cluster, 
QEMUIOVector *qiov,
+int bytes)
 {
 int ret;
 int cluster_bytes, buf_bytes;
@@ -2023,9 +2023,9 @@ fail:
  *
  * Returns: error code with 0 for success.
  */
-static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
-   uint64_t bytes, QEMUIOVector *qiov,
-   bool zeroed, bool zero_dry_run)
+static int coroutine_fn vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+uint64_t bytes, QEMUIOVector *qiov,
+bool zeroed, bool zero_dry_run)
 {
 BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent = NULL;
-- 
2.35.1

Re: [PULL 0/3] Block patches

[Stefan Hajnoczi]

On Thu, 5 May 2022 at 17:43, Richard Henderson
 wrote:
>
> On 5/5/22 03:42, Stefan Hajnoczi wrote:
> > The following changes since commit 9cf289af47bcfae5c75de37d8e5d6fd23705322c:
> >
> >Merge tag 'qga-pull-request' of gitlab.com:marcandre.lureau/qemu into 
> > staging (2022-05-04 03:42:49 -0700)
> >
> > are available in the Git repository at:
> >
> >https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to bef2e050d6a7feb865854c65570c496ac5a8cf53:
> >
> >util/event-loop-base: Introduce options to set the thread pool size 
> > (2022-05-04 17:02:19 +0100)
> >
> > 
> > Pull request
> >
> > Add new thread-pool-min/thread-pool-max parameters to control the thread 
> > pool
> > used for async I/O.
> >
> > 
> >
> > Nicolas Saenz Julienne (3):
> >Introduce event-loop-base abstract class
> >util/main-loop: Introduce the main loop into QOM
> >util/event-loop-base: Introduce options to set the thread pool size
> >
> >   qapi/qom.json|  43 --
> >   meson.build  |  26 +++---
> >   include/block/aio.h  |  10 +++
> >   include/block/thread-pool.h  |   3 +
> >   include/qemu/main-loop.h |  10 +++
> >   include/sysemu/event-loop-base.h |  41 +
> >   include/sysemu/iothread.h|   6 +-
> >   event-loop-base.c| 140 +++
> >   iothread.c   |  68 +--
> >   util/aio-posix.c |   1 +
> >   util/async.c |  20 +
> >   util/main-loop.c |  65 ++
> >   util/thread-pool.c   |  55 +++-
> >   13 files changed, 419 insertions(+), 69 deletions(-)
> >   create mode 100644 include/sysemu/event-loop-base.h
> >   create mode 100644 event-loop-base.c
> >
>
> This appears to introduce a new error on msys2-64bit:
>
>
> 14/85 qemu:unit / test-aio  ERROR 
>   2.14s
>exit status 3
>  >>> MALLOC_PERTURB_=82 
> G_TEST_SRCDIR=C:/GitLab-Runner/builds/qemu-project/qemu/tests/unit
> G_TEST_BUILDDIR=C:/GitLab-Runner/builds/qemu-project/qemu/build/tests/unit
> C:/GitLab-Runner/builds/qemu-project/qemu/build/tests/unit/test-aio.exe --tap 
> -k
> - 8< -
> stderr:
> (test program exited with status code 3)
>
> https://gitlab.com/qemu-project/qemu/-/jobs/2418935125
>
> Are you in a position to test this yourself locally?

I haven't reproduced it yet but will dig a bit more.

test-aio.exe succeeds under Wine:
# random seed: R02S572ad8b9cfeac92bb23a64678114e66d
1..29
# Start of aio tests
ok 1 /aio/acquire
ok 2 /aio/external-client
# Start of bh tests
ok 3 /aio/bh/schedule
ok 4 /aio/bh/schedule10
ok 5 /aio/bh/cancel
ok 6 /aio/bh/delete
ok 7 /aio/bh/flush
# Start of callback-delete tests
ok 8 /aio/bh/callback-delete/one
ok 9 /aio/bh/callback-delete/many
# End of callback-delete tests
# End of bh tests
# Start of event tests
ok 10 /aio/event/add-remove
ok 11 /aio/event/wait
ok 12 /aio/event/flush
# Start of wait tests
ok 13 /aio/event/wait/no-flush-cb
# End of wait tests
# End of event tests
# Start of timer tests
ok 14 /aio/timer/schedule
# End of timer tests
# Start of coroutine tests
ok 15 /aio/coroutine/queue-chaining
ok 16 /aio/coroutine/worker-thread-co-enter
# End of coroutine tests
# End of aio tests
# Start of aio-gsource tests
ok 17 /aio-gsource/flush
# Start of bh tests
ok 18 /aio-gsource/bh/schedule
ok 19 /aio-gsource/bh/schedule10
ok 20 /aio-gsource/bh/cancel
ok 21 /aio-gsource/bh/delete
ok 22 /aio-gsource/bh/flush
# Start of callback-delete tests
ok 23 /aio-gsource/bh/callback-delete/one
ok 24 /aio-gsource/bh/callback-delete/many
# End of callback-delete tests
# End of bh tests
# Start of event tests
ok 25 /aio-gsource/event/add-remove
ok 26 /aio-gsource/event/wait
ok 27 /aio-gsource/event/flush
# Start of wait tests
ok 28 /aio-gsource/event/wait/no-flush-cb
# End of wait tests
# End of event tests
# Start of timer tests
ok 29 /aio-gsource/timer/schedule
# End of timer tests
# End of aio-gsource tests

Stefan

Re: [PATCH v9 0/6] hw/arm/virt: Fix CPU's default NUMA node ID

[Peter Maydell]

On Mon, 9 May 2022 at 04:08, Gavin Shan  wrote:
>
> Hi Peter and maintainers,
>
> On 5/3/22 10:02 PM, Gavin Shan wrote:
> > When the CPU-to-NUMA association isn't provided by user, the default NUMA
> > node ID for the specific CPU is returned from 
> > virt_get_default_cpu_node_id().
> > Unfortunately, the default NUMA node ID breaks socket boundary and leads to
> > the broken CPU topology warning message in Linux guest. This series intends
> > to fix the issue by populating full CPU toplogy on arm/virt machine.
> >
> >PATCH[1/6] Add cluster-id to CPU instance property
> >PATCH[2/6] Fixes test failure in qtest/numa-test/aarch64_numa_cpu()
> >PATCH[3/6] Uses SMP configuration to populate CPU topology
> >PATCH[4/6] Corrects CPU/NUMA association in 
> > qtest/numa-test/aarch64_numa_cpu()
> >PATCH[5/6] Fixes the broken CPU topology by considering the socket 
> > boundary
> >   when the default NUMA node ID is given
> >PATCH[6/6] Uses the populated CPU topology to build PPTT table, instead 
> > of
> >   calculate it again
> >
> > Changelog
> > =
> > v9:
> > * Fix typo by replacing 'thrad-id' with 'thread-id' in
> >   PATCH[v9 2/6]'s commit log (Igor)
> > * Added PATCH[v9 4/6] to make CPU-to-NUMA association
> >   real   (Igor)
>
> [...]
>
> > Gavin Shan (6):
> >qapi/machine.json: Add cluster-id
> >qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()
> >hw/arm/virt: Consider SMP configuration in CPU topology
> >qtest/numa-test: Correct CPU and NUMA association in
> >  aarch64_numa_cpu()
> >hw/arm/virt: Fix CPU's default NUMA node ID
> >hw/acpi/aml-build: Use existing CPU topology to build PPTT table
> >
> >   hw/acpi/aml-build.c| 111 -
> >   hw/arm/virt.c  |  19 ++-
> >   hw/core/machine-hmp-cmds.c |   4 ++
> >   hw/core/machine.c  |  16 ++
> >   qapi/machine.json  |   6 +-
> >   tests/qtest/numa-test.c|  19 +--
> >   6 files changed, 102 insertions(+), 73 deletions(-)
> >
>
> I think this series is ready to be merged. Could you help to see how it
> can be merged to QEMU 7.1?



Applied to target-arm.next, thanks.

-- PMM