[PATCH v4 3/3] cpus-common: implement dirty limit on vCPU
From: Hyman Huang(黄勇)
Implement dirtyrate calculation periodically basing on
dirty-ring and throttle vCPU until it reachs the quota
dirtyrate given by user.
Introduce qmp commands set-dirty-limit/cancel-dirty-limit to
set/cancel dirty limit on vCPU.
Signed-off-by: Hyman Huang(黄勇)
---
cpus-common.c | 41 +
include/hw/core/cpu.h | 9 +
qapi/migration.json | 43 +++
softmmu/vl.c | 1 +
4 files changed, 94 insertions(+)
diff --git a/cpus-common.c b/cpus-common.c
index 6e73d3e..43b0078 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -23,6 +23,11 @@
#include "hw/core/cpu.h"
#include "sysemu/cpus.h"
#include "qemu/lockable.h"
+#include "sysemu/dirtylimit.h"
+#include "sysemu/cpu-throttle.h"
+#include "sysemu/kvm.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-migration.h"
static QemuMutex qemu_cpu_list_lock;
static QemuCond exclusive_cond;
@@ -352,3 +357,39 @@ void process_queued_cpu_work(CPUState *cpu)
qemu_mutex_unlock(>work_mutex);
qemu_cond_broadcast(_work_cond);
}
+
+void qmp_set_dirty_limit(int64_t idx,
+ uint64_t dirtyrate,
+ Error **errp)
+{
+if (!kvm_dirty_ring_enabled()) {
+error_setg(errp, "dirty ring not enable, needed by dirty restraint!");
+return;
+}
+
+dirtylimit_calc();
+dirtylimit_vcpu(idx, dirtyrate);
+}
+
+void qmp_cancel_dirty_limit(int64_t idx,
+Error **errp)
+{
+if (!kvm_dirty_ring_enabled()) {
+error_setg(errp, "dirty ring not enable, needed by dirty restraint!");
+return;
+}
+
+if (unlikely(!dirtylimit_cancel_vcpu(idx))) {
+dirtylimit_calc_quit();
+}
+}
+
+void dirtylimit_setup(int max_cpus)
+{
+if (!kvm_dirty_ring_enabled()) {
+return;
+}
+
+dirtylimit_calc_state_init(max_cpus);
+dirtylimit_state_init(max_cpus);
+}
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index e948e81..11df012 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -881,6 +881,15 @@ void end_exclusive(void);
*/
void qemu_init_vcpu(CPUState *cpu);
+/**
+ * dirtylimit_setup:
+ *
+ * Initializes the global state of dirtylimit calculation and
+ * dirtylimit itself. This is prepared for vCPU dirtylimit which
+ * could be triggered during vm lifecycle.
+ */
+void dirtylimit_setup(int max_cpus);
+
#define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */
#define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
#define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */
diff --git a/qapi/migration.json b/qapi/migration.json
index bbfd48c..42b260e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1850,6 +1850,49 @@
{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
##
+# @set-dirty-limit:
+#
+# Set the upper limit of dirty page rate for the interested vCPU.
+#
+# This command could be used to cap the vCPU memory load, which is also
+# refered as "dirty page rate". Users can use set-dirty-limit unconditionally,
+# but if one want to know which vCPU is in high memory load and which vCPU
+# should be limited, "calc-dirty-rate" with "dirty-ring" mode maybe an
+# availiable method.
+#
+# @idx: vCPU index to set dirtylimit.
+#
+# @dirtyrate: upper limit for the specified vCPU's dirty page rate (MB/s)
+#
+# Since: 6.3
+#
+# Example:
+# {"execute": "set-dirty-limit"}
+#"arguments": { "idx": 0,
+# "dirtyrate": 200 } }
+#
+##
+{ 'command': 'set-dirty-limit',
+ 'data': { 'idx': 'int', 'dirtyrate': 'uint64' } }
+
+##
+# @cancel-dirty-limit:
+#
+# Cancel the dirtylimit for the vCPU which has been set with set-dirty-limit.
+#
+# @idx: vCPU index to canceled the dirtylimit
+#
+# Since: 6.3
+#
+# Example:
+# {"execute": "cancel-dirty-limit"}
+#"arguments": { "idx": 0 } }
+#
+##
+{ 'command': 'cancel-dirty-limit',
+ 'data': { 'idx': 'int' } }
+
+##
# @snapshot-save:
#
# Save a VM snapshot
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 620a1f1..0f83ce3 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3777,5 +3777,6 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_init_displays();
accel_setup_post(current_machine);
os_setup_post();
+dirtylimit_setup(current_machine->smp.max_cpus);
resume_mux_open();
}
--
1.8.3.1
[PATCH v4 0/3] support dirty restraint on vCPU
From: Hyman Huang(黄勇)
v4:
- rebase on master
- modify the following points according to the advice given by Markus
1. move the defination into migration.json
2. polish the comments of set-dirty-limit
3. do the syntax check and change dirty rate to dirty page rate
Thanks for the carefule reviews made by Markus.
Please review, thanks!
v3:
- rebase on master
- modify the following points according to the advice given by Markus
1. remove the DirtyRateQuotaVcpu and use its field as option directly
2. add comments to show details of what dirtylimit setup do
3. explain how to use dirtylimit in combination with existing qmp
commands "calc-dirty-rate" and "query-dirty-rate" in documentation.
Thanks for the carefule reviews made by Markus.
Please review, thanks!
Hyman
v2:
- rebase on master
- modify the following points according to the advices given by Juan
1. rename dirtyrestraint to dirtylimit
2. implement the full lifecyle function of dirtylimit_calc, include
dirtylimit_calc and dirtylimit_calc_quit
3. introduce 'quit' field in dirtylimit_calc_state to implement the
dirtylimit_calc_quit
4. remove the ready_cond and ready_mtx since it may not be suitable
5. put the 'record_dirtypage' function code at the beggining of the
file
6. remove the unnecesary return;
- other modifications has been made after code review
1. introduce 'bmap' and 'nr' field in dirtylimit_state to record the
number of running thread forked by dirtylimit
2. stop the dirtyrate calculation thread if all the dirtylimit thread
are stopped
3. do some renaming works
dirtyrate calulation thread -> dirtylimit-calc
dirtylimit thread -> dirtylimit-{cpu_index}
function name do_dirtyrestraint -> dirtylimit_check
qmp command dirty-restraint -> set-drity-limit
qmp command dirty-restraint-cancel -> cancel-dirty-limit
header file dirtyrestraint.h -> dirtylimit.h
Please review, thanks !
thanks for the accurate and timely advices given by Juan. we really
appreciate it if corrections and suggetions about this patchset are
proposed.
Best Regards !
Hyman
v1:
this patchset introduce a mechanism to impose dirty restraint
on vCPU, aiming to keep the vCPU running in a certain dirtyrate
given by user. dirty restraint on vCPU maybe an alternative
method to implement convergence logic for live migration,
which could improve guest memory performance during migration
compared with traditional method in theory.
For the current live migration implementation, the convergence
logic throttles all vCPUs of the VM, which has some side effects.
-'read processes' on vCPU will be unnecessarily penalized
- throttle increase percentage step by step, which seems
struggling to find the optimal throttle percentage when
dirtyrate is high.
- hard to predict the remaining time of migration if the
throttling percentage reachs 99%
to a certain extent, the dirty restraint machnism can fix these
effects by throttling at vCPU granularity during migration.
the implementation is rather straightforward, we calculate
vCPU dirtyrate via the Dirty Ring mechanism periodically
as the commit 0e21bf246 "implement dirty-ring dirtyrate calculation"
does, for vCPU that be specified to impose dirty restraint,
we throttle it periodically as the auto-converge does, once after
throttling, we compare the quota dirtyrate with current dirtyrate,
if current dirtyrate is not under the quota, increase the throttling
percentage until current dirtyrate is under the quota.
this patchset is the basis of implmenting a new auto-converge method
for live migration, we introduce two qmp commands for impose/cancel
the dirty restraint on specified vCPU, so it also can be an independent
api to supply the upper app such as libvirt, which can use it to
implement the convergence logic during live migration, supplemented
with the qmp 'calc-dirty-rate' command or whatever.
we post this patchset for RFC and any corrections and suggetions about
the implementation, api, throttleing algorithm or whatever are very
appreciated!
Please review, thanks !
Best Regards !
Hyman Huang (3):
migration/dirtyrate: implement vCPU dirtyrate calculation periodically
cpu-throttle: implement vCPU throttle
cpus-common: implement dirty limit on vCPU
cpus-common.c | 41 ++
include/exec/memory.h | 5 +-
include/hw/core/cpu.h | 9 ++
include/sysemu/cpu-throttle.h | 23 +++
include/sysemu/dirtylimit.h | 44 ++
migration/dirtyrate.c | 139 --
migration/dirtyrate.h | 2 +
qapi/migration.json | 43 ++
softmmu/cpu-throttle.c| 319 ++
softmmu/trace-events | 5 +
softmmu/vl.c | 1 +
11 files changed, 620 insertions(+), 11 deletions(-)
create mode 100644 include/sysemu/dirtylimit.h
--
1.8.3.1
[PATCH v4 2/3] cpu-throttle: implement vCPU throttle
From: Hyman Huang(黄勇)
Impose dirty restraint on vCPU by kicking it and sleep
as the auto-converge does during migration, but just
kick the specified vCPU instead, not all vCPUs of vm.
Start a thread to track the dirtylimit status and adjust
the throttle pencentage dynamically depend on current
and quota dirtyrate.
Introduce the util function in the header for dirtylimit
implementation.
Signed-off-by: Hyman Huang(黄勇)
---
include/sysemu/cpu-throttle.h | 23 +++
softmmu/cpu-throttle.c| 319 ++
softmmu/trace-events | 5 +
3 files changed, 347 insertions(+)
diff --git a/include/sysemu/cpu-throttle.h b/include/sysemu/cpu-throttle.h
index d65bdef..726c1ce 100644
--- a/include/sysemu/cpu-throttle.h
+++ b/include/sysemu/cpu-throttle.h
@@ -65,4 +65,27 @@ bool cpu_throttle_active(void);
*/
int cpu_throttle_get_percentage(void);
+/**
+ * dirtylimit_state_init:
+ *
+ * initialize golobal state for dirtylimit
+ */
+void dirtylimit_state_init(int max_cpus);
+
+/**
+ * dirtylimit_vcpu:
+ *
+ * impose dirtylimit on vcpu util reaching the quota dirtyrate
+ */
+void dirtylimit_vcpu(int cpu_index,
+ uint64_t quota);
+/**
+ * dirtylimit_cancel_vcpu:
+ *
+ * cancel dirtylimit for the specified vcpu
+ *
+ * Returns: the number of running threads for dirtylimit
+ */
+int dirtylimit_cancel_vcpu(int cpu_index);
+
#endif /* SYSEMU_CPU_THROTTLE_H */
diff --git a/softmmu/cpu-throttle.c b/softmmu/cpu-throttle.c
index 8c2144a..a5e67c9 100644
--- a/softmmu/cpu-throttle.c
+++ b/softmmu/cpu-throttle.c
@@ -29,6 +29,8 @@
#include "qemu/main-loop.h"
#include "sysemu/cpus.h"
#include "sysemu/cpu-throttle.h"
+#include "sysemu/dirtylimit.h"
+#include "trace.h"
/* vcpu throttling controls */
static QEMUTimer *throttle_timer;
@@ -38,6 +40,323 @@ static unsigned int throttle_percentage;
#define CPU_THROTTLE_PCT_MAX 99
#define CPU_THROTTLE_TIMESLICE_NS 1000
+#define DIRTYLIMIT_TOLERANCE_RANGE 15 /* 15MB/s */
+
+#define DIRTYLIMIT_THROTTLE_HEAVY_WATERMARK 75
+#define DIRTYLIMIT_THROTTLE_SLIGHT_WATERMARK90
+
+#define DIRTYLIMIT_THROTTLE_HEAVY_STEP_SIZE 5
+#define DIRTYLIMIT_THROTTLE_SLIGHT_STEP_SIZE2
+
+typedef enum {
+RESTRAIN_KEEP,
+RESTRAIN_RATIO,
+RESTRAIN_HEAVY,
+RESTRAIN_SLIGHT,
+} RestrainPolicy;
+
+typedef struct DirtyLimitState {
+int cpu_index;
+bool enabled;
+uint64_t quota; /* quota dirtyrate MB/s */
+QemuThread thread;
+char *name; /* thread name */
+} DirtyLimitState;
+
+struct {
+DirtyLimitState *states;
+int max_cpus;
+unsigned long *bmap; /* running thread bitmap */
+unsigned long nr;
+} *dirtylimit_state;
+
+static inline bool dirtylimit_enabled(int cpu_index)
+{
+return qatomic_read(_state->states[cpu_index].enabled);
+}
+
+static inline void dirtylimit_set_quota(int cpu_index, uint64_t quota)
+{
+qatomic_set(_state->states[cpu_index].quota, quota);
+}
+
+static inline uint64_t dirtylimit_quota(int cpu_index)
+{
+return qatomic_read(_state->states[cpu_index].quota);
+}
+
+static int64_t dirtylimit_current(int cpu_index)
+{
+return dirtylimit_calc_current(cpu_index);
+}
+
+static void dirtylimit_vcpu_thread(CPUState *cpu, run_on_cpu_data data)
+{
+double pct;
+double throttle_ratio;
+int64_t sleeptime_ns, endtime_ns;
+int *percentage = (int *)data.host_ptr;
+
+pct = (double)(*percentage) / 100;
+throttle_ratio = pct / (1 - pct);
+/* Add 1ns to fix double's rounding error (like 0.999...) */
+sleeptime_ns = (int64_t)(throttle_ratio * CPU_THROTTLE_TIMESLICE_NS + 1);
+endtime_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + sleeptime_ns;
+while (sleeptime_ns > 0 && !cpu->stop) {
+if (sleeptime_ns > SCALE_MS) {
+qemu_cond_timedwait_iothread(cpu->halt_cond,
+ sleeptime_ns / SCALE_MS);
+} else {
+qemu_mutex_unlock_iothread();
+g_usleep(sleeptime_ns / SCALE_US);
+qemu_mutex_lock_iothread();
+}
+sleeptime_ns = endtime_ns - qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+}
+qatomic_set(>throttle_thread_scheduled, 0);
+
+free(percentage);
+}
+
+static void dirtylimit_check(int cpu_index,
+ int percentage)
+{
+CPUState *cpu;
+int64_t sleeptime_ns, starttime_ms, currenttime_ms;
+int *pct_parameter;
+double pct;
+
+pct = (double) percentage / 100;
+
+starttime_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+
+while (true) {
+CPU_FOREACH(cpu) {
+if ((cpu_index == cpu->cpu_index) &&
+(!qatomic_xchg(>throttle_thread_scheduled, 1))) {
+pct_parameter = malloc(sizeof(*pct_parameter));
+*pct_parameter = percentage;
+async_run_on_cpu(cpu, dirtylimit_vcpu_thread,
+ RUN_ON_CPU_HOST_PTR(pct_parameter));
+
[PATCH v4 1/3] migration/dirtyrate: implement vCPU dirtyrate calculation periodically
From: Hyman Huang(黄勇)
Introduce the third method GLOBAL_DIRTY_LIMIT of dirty
tracking for calculate dirtyrate periodly for dirty restraint.
Implement thread for calculate dirtyrate periodly, which will
be used for dirty restraint.
Add dirtylimit.h to introduce the util function for dirty
limit implementation.
Signed-off-by: Hyman Huang(黄勇)
---
include/exec/memory.h | 5 +-
include/sysemu/dirtylimit.h | 44 ++
migration/dirtyrate.c | 139
migration/dirtyrate.h | 2 +
4 files changed, 179 insertions(+), 11 deletions(-)
create mode 100644 include/sysemu/dirtylimit.h
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 20f1b27..606bec8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -69,7 +69,10 @@ static inline void fuzz_dma_read_cb(size_t addr,
/* Dirty tracking enabled because measuring dirty rate */
#define GLOBAL_DIRTY_DIRTY_RATE (1U << 1)
-#define GLOBAL_DIRTY_MASK (0x3)
+/* Dirty tracking enabled because dirty limit */
+#define GLOBAL_DIRTY_LIMIT (1U << 2)
+
+#define GLOBAL_DIRTY_MASK (0x7)
extern unsigned int global_dirty_tracking;
diff --git a/include/sysemu/dirtylimit.h b/include/sysemu/dirtylimit.h
new file mode 100644
index 000..49298a2
--- /dev/null
+++ b/include/sysemu/dirtylimit.h
@@ -0,0 +1,44 @@
+/*
+ * dirty limit helper functions
+ *
+ * Copyright (c) 2021 CHINA TELECOM CO.,LTD.
+ *
+ * Authors:
+ * Hyman Huang(黄勇)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_DIRTYRLIMIT_H
+#define QEMU_DIRTYRLIMIT_H
+
+#define DIRTYLIMIT_CALC_PERIOD_TIME_S 15 /* 15s */
+
+/**
+ * dirtylimit_calc_current:
+ *
+ * get current dirty page rate for specified vCPU.
+ */
+int64_t dirtylimit_calc_current(int cpu_index);
+
+/**
+ * dirtylimit_calc:
+ *
+ * start dirty page rate calculation thread.
+ */
+void dirtylimit_calc(void);
+
+/**
+ * dirtylimit_calc_quit:
+ *
+ * quit dirty page rate calculation thread.
+ */
+void dirtylimit_calc_quit(void);
+
+/**
+ * dirtylimit_calc_state_init:
+ *
+ * initialize dirty page rate calculation state.
+ */
+void dirtylimit_calc_state_init(int max_cpus);
+#endif
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index d65e744..d370a21 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -27,6 +27,7 @@
#include "qapi/qmp/qdict.h"
#include "sysemu/kvm.h"
#include "sysemu/runstate.h"
+#include "sysemu/dirtylimit.h"
#include "exec/memory.h"
/*
@@ -46,6 +47,134 @@ static struct DirtyRateStat DirtyStat;
static DirtyRateMeasureMode dirtyrate_mode =
DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING;
+#define DIRTYLIMIT_CALC_TIME_MS 1000/* 1000ms */
+
+struct {
+DirtyRatesData data;
+int64_t period;
+bool quit;
+} *dirtylimit_calc_state;
+
+static void dirtylimit_global_dirty_log_start(void)
+{
+qemu_mutex_lock_iothread();
+memory_global_dirty_log_start(GLOBAL_DIRTY_LIMIT);
+qemu_mutex_unlock_iothread();
+}
+
+static void dirtylimit_global_dirty_log_stop(void)
+{
+qemu_mutex_lock_iothread();
+memory_global_dirty_log_sync();
+memory_global_dirty_log_stop(GLOBAL_DIRTY_LIMIT);
+qemu_mutex_unlock_iothread();
+}
+
+static inline void record_dirtypages(DirtyPageRecord *dirty_pages,
+ CPUState *cpu, bool start)
+{
+if (start) {
+dirty_pages[cpu->cpu_index].start_pages = cpu->dirty_pages;
+} else {
+dirty_pages[cpu->cpu_index].end_pages = cpu->dirty_pages;
+}
+}
+
+static void dirtylimit_calc_func(void)
+{
+CPUState *cpu;
+DirtyPageRecord *dirty_pages;
+int64_t start_time, end_time, calc_time;
+DirtyRateVcpu rate;
+int i = 0;
+
+dirty_pages = g_malloc0(sizeof(*dirty_pages) *
+dirtylimit_calc_state->data.nvcpu);
+
+dirtylimit_global_dirty_log_start();
+
+CPU_FOREACH(cpu) {
+record_dirtypages(dirty_pages, cpu, true);
+}
+
+start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+g_usleep(DIRTYLIMIT_CALC_TIME_MS * 1000);
+end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+calc_time = end_time - start_time;
+
+dirtylimit_global_dirty_log_stop();
+
+CPU_FOREACH(cpu) {
+record_dirtypages(dirty_pages, cpu, false);
+}
+
+for (i = 0; i < dirtylimit_calc_state->data.nvcpu; i++) {
+uint64_t increased_dirty_pages =
+dirty_pages[i].end_pages - dirty_pages[i].start_pages;
+uint64_t memory_size_MB =
+(increased_dirty_pages * TARGET_PAGE_SIZE) >> 20;
+int64_t dirtyrate = (memory_size_MB * 1000) / calc_time;
+
+rate.id = i;
+rate.dirty_rate = dirtyrate;
+dirtylimit_calc_state->data.rates[i] = rate;
+
+trace_dirtyrate_do_calculate_vcpu(i,
+dirtylimit_calc_state->data.rates[i].dirty_rate);
+}
+}
+
+static void
Re: [RFC PATCH v5 23/26] util: Add iova_tree_alloc
On Tue, Nov 23, 2021 at 7:57 AM Peter Xu wrote:
>
> Hi, Eugenio,
>
> Sorry for the super late response.
>
No problem!
> On Fri, Oct 29, 2021 at 08:35:22PM +0200, Eugenio Pérez wrote:
>
> [...]
>
> > +int iova_tree_alloc(IOVATree *tree, DMAMap *map, hwaddr iova_begin,
> > +hwaddr iova_last)
> > +{
> > +struct IOVATreeAllocArgs args = {
> > +.new_size = map->size,
> > +.iova_begin = iova_begin,
> > +.iova_last = iova_last,
> > +};
> > +
> > +if (iova_begin == 0) {
> > +/* Some devices does not like addr 0 */
> > +iova_begin += qemu_real_host_page_size;
> > +}
>
> Any explanation of why zero is not welcomed?
>
I didn't investigate too much, but neither vhost-net or qemu device
accepted a ring with address 0. Probably it's because some test like:
if (!vq->desc) { return; }
That assumes 0 == not initialized. Even if we fix that issue in the
devices, the vdpa device backend could be an old version, and since
the iova range should be big anyway I think we should skip 0 anyway.
> It would be great if we can move this out of iova-tree.c, because that doesn't
> look like a good place to, e.g. reference qemu_real_host_page_size, anyways.
> The caller can simply pass in qemu_real_host_page_size as iova_begin when
> needed (and I highly doubt it'll be a must for all iova-tree users..)
>
I think yes, it can be included in iova_begin. I'll rework that part.
> > +
> > +assert(iova_begin < iova_last);
> > +
> > +/*
> > + * Find a valid hole for the mapping
> > + *
> > + * Assuming low iova_begin, so no need to do a binary search to
> > + * locate the first node.
> > + *
> > + * TODO: We can improve the search speed if we save the beginning and
> > the
> > + * end of holes, so we don't iterate over the previous saved ones.
> > + *
> > + * TODO: Replace all this with g_tree_node_first/next/last when
> > available
> > + * (from glib since 2.68). To do it with g_tree_foreach complicates the
> > + * code a lot.
> > + *
> > + */
> > +g_tree_foreach(tree->tree, iova_tree_alloc_traverse, );
> > +if (!iova_tree_alloc_map_in_hole()) {
> > +/*
> > + * 2nd try: Last iteration left args->right as the last DMAMap. But
> > + * (right, end) hole needs to be checked too
> > + */
> > +iova_tree_alloc_args_iterate(, NULL);
> > +if (!iova_tree_alloc_map_in_hole()) {
> > +return IOVA_ERR_NOMEM;
> > +}
> > +}
> > +
> > +map->iova = MAX(iova_begin,
> > +args.hole_left ?
> > +args.hole_left->iova + args.hole_left->size + 1 : 0);
> > +return iova_tree_insert(tree, map);
> > +}
>
> Re the algorithm - I totally agree Jason's version is much better.
>
I'll try to accommodate it, but (if I understood it correctly) it
needs to deal with deallocation and a few other things. But it should
be doable.
Thanks!
> Thanks,
>
> --
> Peter Xu
>
Re: [RFC PATCH v5 23/26] util: Add iova_tree_alloc
Hi, Eugenio,
Sorry for the super late response.
On Fri, Oct 29, 2021 at 08:35:22PM +0200, Eugenio Pérez wrote:
[...]
> +int iova_tree_alloc(IOVATree *tree, DMAMap *map, hwaddr iova_begin,
> +hwaddr iova_last)
> +{
> +struct IOVATreeAllocArgs args = {
> +.new_size = map->size,
> +.iova_begin = iova_begin,
> +.iova_last = iova_last,
> +};
> +
> +if (iova_begin == 0) {
> +/* Some devices does not like addr 0 */
> +iova_begin += qemu_real_host_page_size;
> +}
Any explanation of why zero is not welcomed?
It would be great if we can move this out of iova-tree.c, because that doesn't
look like a good place to, e.g. reference qemu_real_host_page_size, anyways.
The caller can simply pass in qemu_real_host_page_size as iova_begin when
needed (and I highly doubt it'll be a must for all iova-tree users..)
> +
> +assert(iova_begin < iova_last);
> +
> +/*
> + * Find a valid hole for the mapping
> + *
> + * Assuming low iova_begin, so no need to do a binary search to
> + * locate the first node.
> + *
> + * TODO: We can improve the search speed if we save the beginning and the
> + * end of holes, so we don't iterate over the previous saved ones.
> + *
> + * TODO: Replace all this with g_tree_node_first/next/last when available
> + * (from glib since 2.68). To do it with g_tree_foreach complicates the
> + * code a lot.
> + *
> + */
> +g_tree_foreach(tree->tree, iova_tree_alloc_traverse, );
> +if (!iova_tree_alloc_map_in_hole()) {
> +/*
> + * 2nd try: Last iteration left args->right as the last DMAMap. But
> + * (right, end) hole needs to be checked too
> + */
> +iova_tree_alloc_args_iterate(, NULL);
> +if (!iova_tree_alloc_map_in_hole()) {
> +return IOVA_ERR_NOMEM;
> +}
> +}
> +
> +map->iova = MAX(iova_begin,
> +args.hole_left ?
> +args.hole_left->iova + args.hole_left->size + 1 : 0);
> +return iova_tree_insert(tree, map);
> +}
Re the algorithm - I totally agree Jason's version is much better.
Thanks,
--
Peter Xu
Re: [PATCH v2 1/5] target/riscv: Check PMP rules num before propagation
On Mon, Nov 22, 2021 at 9:15 PM LIU Zhiwei wrote:
>
> When an address is in [0-4K) and no pmp rule configured, the tlb_size will
> be set to 1.
>
> This is caused by pmp_get_tlb_size return a value 1.
>
> if (pmp_sa >= tlb_sa && pmp_ea <= tlb_ea) {
> return pmp_ea - pmp_sa + 1;
> }
>
> Here pmp_sa == 0 and pmp_ea == 0.
>
> Signed-off-by: LIU Zhiwei
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/pmp.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 54abf42583..190ff59fab 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -627,6 +627,10 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr
> tlb_sa,
> target_ulong val;
> target_ulong tlb_ea = (tlb_sa + TARGET_PAGE_SIZE - 1);
>
> +if (pmp_get_num_rules(env) == 0) {
> +return false;
> +}
> +
> for (i = 0; i < MAX_RISCV_PMPS; i++) {
> val = pmp_get_tlb_size(env, i, tlb_sa, tlb_ea);
> if (val) {
> --
> 2.25.1
>
>
Re: [PATCH v5 04/18] target/riscv: additional macros to check instruction support
On Sat, Nov 13, 2021 at 1:08 AM Frédéric Pétrot
wrote:
>
> Given that the 128-bit version of the riscv spec adds new instructions, and
> that some instructions that were previously only available in 64-bit mode
> are now available for both 64-bit and 128-bit, we added new macros to check
> for the processor mode during translation.
> Although RV128 is a superset of RV64, we keep for now the RV64 only tests
> for extensions other than RVI and RVM.
>
> Signed-off-by: Frédéric Pétrot
> Co-authored-by: Fabien Portas
> Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/translate.c | 20
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 1d57bc97b5..d98bde9b6b 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -368,10 +368,22 @@ EX_SH(12)
> } \
> } while (0)
>
> -#define REQUIRE_64BIT(ctx) do {\
> -if (get_xl(ctx) < MXL_RV64) { \
> -return false; \
> -} \
> +#define REQUIRE_64BIT(ctx) do { \
> +if (get_xl(ctx) != MXL_RV64) { \
> +return false; \
> +} \
> +} while (0)
> +
> +#define REQUIRE_128BIT(ctx) do {\
> +if (get_xl(ctx) != MXL_RV128) { \
> +return false; \
> +} \
> +} while (0)
> +
> +#define REQUIRE_64_OR_128BIT(ctx) do { \
> +if (get_xl(ctx) == MXL_RV32) { \
> +return false; \
> +} \
> } while (0)
>
> static int ex_rvc_register(DisasContext *ctx, int reg)
> --
> 2.33.1
>
>
Re: [RFC PATCH] s390: kvm: reduce frequency of CPU syncs of diag318 info
Am 22.11.21 um 23:33 schrieb Collin Walling:
DIAGNOSE 0318 is invoked only once during IPL. As such, the
diag318 info will only change once initially and during resets.
Let's only sync the register to convey the info to KVM if and
only if the diag318 info has changed. Only set the dirty bit
flag for diag318 whenever it must be updated.
Is this really necessary? In my logs the sync only happens for larger
changes (like reset) operations and then yes we log again.
But I think it is ok to see such a log entry in these rare events.
The migration handler will invoke the set_diag318 helper on
post_load to ensure the info is set on the destination machine.
Signed-off-by: Collin Walling
---
target/s390x/kvm/kvm.c | 5 -
target/s390x/machine.c | 14 ++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 6acf14d5ec..6a141399f7 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -599,11 +599,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
}
-if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
-cs->kvm_run->s.regs.diag318 = env->diag318_info;
-cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
-}
-
/* Finally the prefix */
if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
cs->kvm_run->s.regs.prefix = env->psa;
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 37a076858c..a5d113ce3a 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -234,6 +234,19 @@ const VMStateDescription vmstate_etoken = {
}
};
+static int diag318_post_load(void *opaque, int version_id)
+{
+S390CPU *cpu = opaque;
+CPUState *cs = CPU(cpu);
+CPUS390XState *env = _CPU(cs)->env;
+
+if (kvm_enabled()) {
+kvm_s390_set_diag318(cs, env->diag318_info);
+}
+
+return 0;
+}
+
static bool diag318_needed(void *opaque)
{
return s390_has_feat(S390_FEAT_DIAG_318);
@@ -243,6 +256,7 @@ const VMStateDescription vmstate_diag318 = {
.name = "cpu/diag318",
.version_id = 1,
.minimum_version_id = 1,
+.post_load = diag318_post_load,
.needed = diag318_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT64(env.diag318_info, S390CPU),
Re: [PATCH v5 08/18] target/riscv: moving some insns close to similar insns
On Sat, Nov 13, 2021 at 1:17 AM Frédéric Pétrot
wrote:
>
> lwu and ld are functionally close to the other loads, but were after the
> stores in the source file.
> Similarly, xor was away from or and and by two arithmetic functions, while
> the immediate versions were nicely put together.
> This patch moves the aforementioned loads after lhu, and xor above or,
> where they more logically belong.
>
> Signed-off-by: Frédéric Pétrot
> Co-authored-by: Fabien Portas
> Reviewed-by: Richard Henderson
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/riscv/insn_trans/trans_rvi.c.inc | 34 -
> meson | 2 +-
> 2 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 51607b3d40..710f5e6a85 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -176,6 +176,18 @@ static bool trans_lhu(DisasContext *ctx, arg_lhu *a)
> return gen_load(ctx, a, MO_TEUW);
> }
>
> +static bool trans_lwu(DisasContext *ctx, arg_lwu *a)
> +{
> +REQUIRE_64BIT(ctx);
> +return gen_load(ctx, a, MO_TEUL);
> +}
> +
> +static bool trans_ld(DisasContext *ctx, arg_ld *a)
> +{
> +REQUIRE_64BIT(ctx);
> +return gen_load(ctx, a, MO_TEUQ);
> +}
> +
> static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
> {
> TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
> @@ -207,18 +219,6 @@ static bool trans_sw(DisasContext *ctx, arg_sw *a)
> return gen_store(ctx, a, MO_TESL);
> }
>
> -static bool trans_lwu(DisasContext *ctx, arg_lwu *a)
> -{
> -REQUIRE_64BIT(ctx);
> -return gen_load(ctx, a, MO_TEUL);
> -}
> -
> -static bool trans_ld(DisasContext *ctx, arg_ld *a)
> -{
> -REQUIRE_64BIT(ctx);
> -return gen_load(ctx, a, MO_TEUQ);
> -}
> -
> static bool trans_sd(DisasContext *ctx, arg_sd *a)
> {
> REQUIRE_64BIT(ctx);
> @@ -317,11 +317,6 @@ static bool trans_sltu(DisasContext *ctx, arg_sltu *a)
> return gen_arith(ctx, a, EXT_SIGN, gen_sltu);
> }
>
> -static bool trans_xor(DisasContext *ctx, arg_xor *a)
> -{
> -return gen_logic(ctx, a, tcg_gen_xor_tl);
> -}
> -
> static bool trans_srl(DisasContext *ctx, arg_srl *a)
> {
> return gen_shift(ctx, a, EXT_ZERO, tcg_gen_shr_tl);
> @@ -332,6 +327,11 @@ static bool trans_sra(DisasContext *ctx, arg_sra *a)
> return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl);
> }
>
> +static bool trans_xor(DisasContext *ctx, arg_xor *a)
> +{
> +return gen_logic(ctx, a, tcg_gen_xor_tl);
> +}
> +
> static bool trans_or(DisasContext *ctx, arg_or *a)
> {
> return gen_logic(ctx, a, tcg_gen_or_tl);
> diff --git a/meson b/meson
> index 12f9f04ba0..b25d94e7c7 16
> --- a/meson
> +++ b/meson
> @@ -1 +1 @@
> -Subproject commit 12f9f04ba0decfda425dbbf9a501084c153a2d18
> +Subproject commit b25d94e7c77fda05a7fdfe8afe562cf9760d69da
This shouldn't be here
Alistair
> --
> 2.33.1
>
>
Re: [PATCH v1 01/12] update-linux-headers: Add asm-riscv/kvm.h
On Sat, Nov 20, 2021 at 5:51 PM Yifei Jiang wrote:
>
> Add asm-riscv/kvm.h for RISC-V KVM, and update linux/kvm.h
>
> Signed-off-by: Yifei Jiang
> Signed-off-by: Mingwang Li
Acked-by: Alistair Francis
Alistair
> ---
> linux-headers/asm-riscv/kvm.h | 128 ++
> linux-headers/linux/kvm.h | 8 +++
> 2 files changed, 136 insertions(+)
> create mode 100644 linux-headers/asm-riscv/kvm.h
>
> diff --git a/linux-headers/asm-riscv/kvm.h b/linux-headers/asm-riscv/kvm.h
> new file mode 100644
> index 00..f808ad1ce5
> --- /dev/null
> +++ b/linux-headers/asm-riscv/kvm.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2019 Western Digital Corporation or its affiliates.
> + *
> + * Authors:
> + * Anup Patel
> + */
> +
> +#ifndef __LINUX_KVM_RISCV_H
> +#define __LINUX_KVM_RISCV_H
> +
> +#ifndef __ASSEMBLY__
> +
> +#include
> +#include
> +
> +#define __KVM_HAVE_READONLY_MEM
> +
> +#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
> +
> +#define KVM_INTERRUPT_SET -1U
> +#define KVM_INTERRUPT_UNSET-2U
> +
> +/* for KVM_GET_REGS and KVM_SET_REGS */
> +struct kvm_regs {
> +};
> +
> +/* for KVM_GET_FPU and KVM_SET_FPU */
> +struct kvm_fpu {
> +};
> +
> +/* KVM Debug exit structure */
> +struct kvm_debug_exit_arch {
> +};
> +
> +/* for KVM_SET_GUEST_DEBUG */
> +struct kvm_guest_debug_arch {
> +};
> +
> +/* definition of registers in kvm_run */
> +struct kvm_sync_regs {
> +};
> +
> +/* for KVM_GET_SREGS and KVM_SET_SREGS */
> +struct kvm_sregs {
> +};
> +
> +/* CONFIG registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_config {
> + unsigned long isa;
> +};
> +
> +/* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_core {
> + struct user_regs_struct regs;
> + unsigned long mode;
> +};
> +
> +/* Possible privilege modes for kvm_riscv_core */
> +#define KVM_RISCV_MODE_S 1
> +#define KVM_RISCV_MODE_U 0
> +
> +/* CSR registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_csr {
> + unsigned long sstatus;
> + unsigned long sie;
> + unsigned long stvec;
> + unsigned long sscratch;
> + unsigned long sepc;
> + unsigned long scause;
> + unsigned long stval;
> + unsigned long sip;
> + unsigned long satp;
> + unsigned long scounteren;
> +};
> +
> +/* TIMER registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> +struct kvm_riscv_timer {
> + __u64 frequency;
> + __u64 time;
> + __u64 compare;
> + __u64 state;
> +};
> +
> +/* Possible states for kvm_riscv_timer */
> +#define KVM_RISCV_TIMER_STATE_OFF 0
> +#define KVM_RISCV_TIMER_STATE_ON 1
> +
> +#define KVM_REG_SIZE(id) \
> + (1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
> +
> +/* If you need to interpret the index values, here is the key: */
> +#define KVM_REG_RISCV_TYPE_MASK0xFF00
> +#define KVM_REG_RISCV_TYPE_SHIFT 24
> +
> +/* Config registers are mapped as type 1 */
> +#define KVM_REG_RISCV_CONFIG (0x01 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_CONFIG_REG(name) \
> + (offsetof(struct kvm_riscv_config, name) / sizeof(unsigned long))
> +
> +/* Core registers are mapped as type 2 */
> +#define KVM_REG_RISCV_CORE (0x02 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_CORE_REG(name) \
> + (offsetof(struct kvm_riscv_core, name) / sizeof(unsigned
> long))
> +
> +/* Control and status registers are mapped as type 3 */
> +#define KVM_REG_RISCV_CSR (0x03 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_CSR_REG(name)\
> + (offsetof(struct kvm_riscv_csr, name) / sizeof(unsigned long))
> +
> +/* Timer registers are mapped as type 4 */
> +#define KVM_REG_RISCV_TIMER(0x04 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_TIMER_REG(name) \
> + (offsetof(struct kvm_riscv_timer, name) / sizeof(__u64))
> +
> +/* F extension registers are mapped as type 5 */
> +#define KVM_REG_RISCV_FP_F (0x05 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_FP_F_REG(name) \
> + (offsetof(struct __riscv_f_ext_state, name) / sizeof(__u32))
> +
> +/* D extension registers are mapped as type 6 */
> +#define KVM_REG_RISCV_FP_D (0x06 << KVM_REG_RISCV_TYPE_SHIFT)
> +#define KVM_REG_RISCV_FP_D_REG(name) \
> + (offsetof(struct __riscv_d_ext_state, name) / sizeof(__u64))
> +
> +#endif
> +
> +#endif /* __LINUX_KVM_RISCV_H */
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index bcaf66cc4d..5e290c3c3e 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -269,6 +269,7 @@ struct kvm_xen_exit {
> #define KVM_EXIT_AP_RESET_HOLD32
> #define KVM_EXIT_X86_BUS_LOCK 33
> #define KVM_EXIT_XEN 34
> +#define
Re: [PATCH v5 06/18] target/riscv: array for the 64 upper bits of 128-bit registers
On Sat, Nov 13, 2021 at 1:07 AM Frédéric Pétrot
wrote:
>
> The upper 64-bit of the 128-bit registers have now a place inside
> the cpu state structure, and are created as globals for future use.
>
> Signed-off-by: Frédéric Pétrot
> Co-authored-by: Fabien Portas
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/cpu.c | 9 +
> target/riscv/machine.c | 20
> target/riscv/translate.c | 5 -
> 4 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0760c0af93..53a295efb7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -110,6 +110,7 @@ FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
>
> struct CPURISCVState {
> target_ulong gpr[32];
> +target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> uint64_t fpr[32]; /* assume both F and D extensions */
>
> /* vector coprocessor state. */
> @@ -339,6 +340,7 @@ static inline bool riscv_feature(CPURISCVState *env, int
> feature)
> #include "cpu_user.h"
>
> extern const char * const riscv_int_regnames[];
> +extern const char * const riscv_int_regnamesh[];
> extern const char * const riscv_fpr_regnames[];
>
> const char *riscv_cpu_get_trap_name(target_ulong cause, bool async);
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f812998123..364140f5ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -42,6 +42,15 @@ const char * const riscv_int_regnames[] = {
>"x28/t3", "x29/t4", "x30/t5", "x31/t6"
> };
>
> +const char * const riscv_int_regnamesh[] = {
> + "x0h/zeroh", "x1h/rah", "x2h/sph", "x3h/gph", "x4h/tph", "x5h/t0h",
> + "x6h/t1h", "x7h/t2h", "x8h/s0h", "x9h/s1h", "x10h/a0h", "x11h/a1h",
> + "x12h/a2h", "x13h/a3h", "x14h/a4h", "x15h/a5h", "x16h/a6h", "x17h/a7h",
> + "x18h/s2h", "x19h/s3h", "x20h/s4h", "x21h/s5h", "x22h/s6h", "x23h/s7h",
> + "x24h/s8h", "x25h/s9h", "x26h/s10h", "x27h/s11h", "x28h/t3h", "x29h/t4h",
> + "x30h/t5h", "x31h/t6h"
> +};
> +
> const char * const riscv_fpr_regnames[] = {
>"f0/ft0", "f1/ft1", "f2/ft2", "f3/ft3", "f4/ft4", "f5/ft5",
>"f6/ft6", "f7/ft7", "f8/fs0", "f9/fs1", "f10/fa0", "f11/fa1",
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 7b4c739564..7e2d02457e 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -92,6 +92,14 @@ static bool pointermasking_needed(void *opaque)
> return riscv_has_ext(env, RVJ);
> }
>
> +static bool rv128_needed(void *opaque)
> +{
> +RISCVCPU *cpu = opaque;
> +CPURISCVState *env = >env;
> +
> +return env->misa_mxl_max == MXL_RV128;
> +}
I think it would just be better to use riscv_cpu_mxl() directly
instead of adding a new function here.
Alistair
> +
> static const VMStateDescription vmstate_vector = {
> .name = "cpu/vector",
> .version_id = 1,
> @@ -164,6 +172,17 @@ static const VMStateDescription vmstate_hyper = {
> }
> };
>
> +static const VMStateDescription vmstate_rv128 = {
> +.name = "cpu/rv128",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = rv128_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINTTL_ARRAY(env.gprh, RISCVCPU, 32),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> const VMStateDescription vmstate_riscv_cpu = {
> .name = "cpu",
> .version_id = 3,
> @@ -218,6 +237,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> _hyper,
> _vector,
> _pointermasking,
> +_rv128,
> NULL
> }
> };
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index b4278a6a92..00a2cfa917 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -32,7 +32,7 @@
> #include "instmap.h"
>
> /* global register indices */
> -static TCGv cpu_gpr[32], cpu_pc, cpu_vl;
> +static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl;
> static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> static TCGv load_res;
> static TCGv load_val;
> @@ -777,10 +777,13 @@ void riscv_translate_init(void)
> * unless you specifically block reads/writes to reg 0.
> */
> cpu_gpr[0] = NULL;
> +cpu_gprh[0] = NULL;
>
> for (i = 1; i < 32; i++) {
> cpu_gpr[i] = tcg_global_mem_new(cpu_env,
> offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
> +cpu_gprh[i] = tcg_global_mem_new(cpu_env,
> +offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
> }
>
> for (i = 0; i < 32; i++) {
> --
> 2.33.1
>
>
Re: [PATCH v5 05/18] target/riscv: separation of bitwise logic and arithmetic helpers
On Sat, Nov 13, 2021 at 1:11 AM Frédéric Pétrot
wrote:
>
> Introduction of a gen_logic function for bitwise logic to implement
> instructions in which not propagation of information occurs between bits and
> use of this function on the bitwise instructions.
>
> Signed-off-by: Frédéric Pétrot
> Co-authored-by: Fabien Portas
> Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/translate.c| 27 +
> target/riscv/insn_trans/trans_rvb.c.inc | 6 +++---
> target/riscv/insn_trans/trans_rvi.c.inc | 12 +--
> 3 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index d98bde9b6b..b4278a6a92 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -400,6 +400,33 @@ static int ex_rvc_shifti(DisasContext *ctx, int imm)
> /* Include the auto-generated decoder for 32 bit insn */
> #include "decode-insn32.c.inc"
>
> +static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
> + void (*func)(TCGv, TCGv, target_long))
> +{
> +TCGv dest = dest_gpr(ctx, a->rd);
> +TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +
> +func(dest, src1, a->imm);
> +
> +gen_set_gpr(ctx, a->rd, dest);
> +
> +return true;
> +}
> +
> +static bool gen_logic(DisasContext *ctx, arg_r *a,
> + void (*func)(TCGv, TCGv, TCGv))
> +{
> +TCGv dest = dest_gpr(ctx, a->rd);
> +TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> +
> +func(dest, src1, src2);
> +
> +gen_set_gpr(ctx, a->rd, dest);
> +
> +return true;
> +}
> +
> static bool gen_arith_imm_fn(DisasContext *ctx, arg_i *a, DisasExtend ext,
> void (*func)(TCGv, TCGv, target_long))
> {
> diff --git a/target/riscv/insn_trans/trans_rvb.c.inc
> b/target/riscv/insn_trans/trans_rvb.c.inc
> index c8d31907c5..de2cd613b1 100644
> --- a/target/riscv/insn_trans/trans_rvb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> @@ -86,19 +86,19 @@ static bool trans_cpop(DisasContext *ctx, arg_cpop *a)
> static bool trans_andn(DisasContext *ctx, arg_andn *a)
> {
> REQUIRE_ZBB(ctx);
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_andc_tl);
> +return gen_logic(ctx, a, tcg_gen_andc_tl);
> }
>
> static bool trans_orn(DisasContext *ctx, arg_orn *a)
> {
> REQUIRE_ZBB(ctx);
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_orc_tl);
> +return gen_logic(ctx, a, tcg_gen_orc_tl);
> }
>
> static bool trans_xnor(DisasContext *ctx, arg_xnor *a)
> {
> REQUIRE_ZBB(ctx);
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_eqv_tl);
> +return gen_logic(ctx, a, tcg_gen_eqv_tl);
> }
>
> static bool trans_min(DisasContext *ctx, arg_min *a)
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 4a2aefe3a5..51607b3d40 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -252,17 +252,17 @@ static bool trans_sltiu(DisasContext *ctx, arg_sltiu *a)
>
> static bool trans_xori(DisasContext *ctx, arg_xori *a)
> {
> -return gen_arith_imm_fn(ctx, a, EXT_NONE, tcg_gen_xori_tl);
> +return gen_logic_imm_fn(ctx, a, tcg_gen_xori_tl);
> }
>
> static bool trans_ori(DisasContext *ctx, arg_ori *a)
> {
> -return gen_arith_imm_fn(ctx, a, EXT_NONE, tcg_gen_ori_tl);
> +return gen_logic_imm_fn(ctx, a, tcg_gen_ori_tl);
> }
>
> static bool trans_andi(DisasContext *ctx, arg_andi *a)
> {
> -return gen_arith_imm_fn(ctx, a, EXT_NONE, tcg_gen_andi_tl);
> +return gen_logic_imm_fn(ctx, a, tcg_gen_andi_tl);
> }
>
> static bool trans_slli(DisasContext *ctx, arg_slli *a)
> @@ -319,7 +319,7 @@ static bool trans_sltu(DisasContext *ctx, arg_sltu *a)
>
> static bool trans_xor(DisasContext *ctx, arg_xor *a)
> {
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_xor_tl);
> +return gen_logic(ctx, a, tcg_gen_xor_tl);
> }
>
> static bool trans_srl(DisasContext *ctx, arg_srl *a)
> @@ -334,12 +334,12 @@ static bool trans_sra(DisasContext *ctx, arg_sra *a)
>
> static bool trans_or(DisasContext *ctx, arg_or *a)
> {
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_or_tl);
> +return gen_logic(ctx, a, tcg_gen_or_tl);
> }
>
> static bool trans_and(DisasContext *ctx, arg_and *a)
> {
> -return gen_arith(ctx, a, EXT_NONE, tcg_gen_and_tl);
> +return gen_logic(ctx, a, tcg_gen_and_tl);
> }
>
> static bool trans_addiw(DisasContext *ctx, arg_addiw *a)
> --
> 2.33.1
>
>
Re: [PATCH v5 03/18] qemu/int128: addition of div/rem 128-bit operations
On Sat, Nov 13, 2021 at 1:06 AM Frédéric Pétrot
wrote:
>
> Addition of div and rem on 128-bit integers, using the 128/64->128 divu and
> 64x64->128 mulu in host-utils.
> These operations will be used within div/rem helpers in the 128-bit riscv
> target.
>
> Signed-off-by: Frédéric Pétrot
> Co-authored-by: Fabien Portas
Reviewed-by: Alistair Francis
Alistair
> ---
> include/qemu/int128.h | 6 ++
> util/int128.c | 145 ++
> util/meson.build | 1 +
> 3 files changed, 152 insertions(+)
> create mode 100644 util/int128.c
>
> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> index b6d517aea4..ef41892dac 100644
> --- a/include/qemu/int128.h
> +++ b/include/qemu/int128.h
> @@ -386,4 +386,10 @@ static inline void bswap128s(Int128 *s)
> *s = bswap128(*s);
> }
>
> +#define UINT128_MAX int128_make128(~0LL, ~0LL)
> +Int128 int128_divu(Int128, Int128);
> +Int128 int128_remu(Int128, Int128);
> +Int128 int128_divs(Int128, Int128);
> +Int128 int128_rems(Int128, Int128);
> +
> #endif /* INT128_H */
> diff --git a/util/int128.c b/util/int128.c
> new file mode 100644
> index 00..c2ddf197e1
> --- /dev/null
> +++ b/util/int128.c
> @@ -0,0 +1,145 @@
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "qemu/int128.h"
> +
> +#ifdef CONFIG_INT128
> +
> +Int128 int128_divu(Int128 a, Int128 b)
> +{
> +return (__uint128_t)a / (__uint128_t)b;
> +}
> +
> +Int128 int128_remu(Int128 a, Int128 b)
> +{
> +return (__uint128_t)a % (__uint128_t)b;
> +}
> +
> +Int128 int128_divs(Int128 a, Int128 b)
> +{
> +return a / b;
> +}
> +
> +Int128 int128_rems(Int128 a, Int128 b)
> +{
> +return a % b;
> +}
> +
> +#else
> +
> +/*
> + * Division and remainder algorithms for 128-bit due to Stefan Kanthak,
> + * https://skanthak.homepage.t-online.de/integer.html#udivmodti4
> + * Preconditions:
> + * - function should never be called with v equals to 0, it has to
> + * be dealt with beforehand
> + * - quotien pointer must be valid
> + */
> +static Int128 divrem128(Int128 u, Int128 v, Int128 *q)
> +{
> +Int128 qq;
> +uint64_t hi, lo, tmp;
> +int s;
> +
> +if ((s = clz64(v.hi)) == 64) {
> +/* we have uu÷0v => let's use divu128 */
> +hi = u.hi;
> +lo = u.lo;
> +tmp = divu128(, , v.lo);
> +*q = int128_make128(lo, hi);
> +return int128_make128(tmp, 0);
> +} else {
> +hi = int128_gethi(int128_lshift(v, s));
> +
> +if (hi > u.hi) {
> +lo = u.lo;
> +tmp = u.hi;
> +divu128(, , hi);
> +lo = int128_gethi(int128_lshift(int128_make128(lo, 0), s));
> +} else { /* prevent overflow */
> +lo = u.lo;
> +tmp = u.hi - hi;
> +divu128(, , hi);
> +lo = int128_gethi(int128_lshift(int128_make128(lo, 1), s));
> +}
> +
> +qq = int128_make64(lo);
> +
> +tmp = lo * v.hi;
> +mulu64(, , lo, v.lo);
> +hi += tmp;
> +
> +if (hi < tmp /* quotient * divisor >= 2**128 > dividend */
> +|| hi > u.hi /* quotient * divisor > dividend */
> +|| (hi == u.hi && lo > u.lo)) {
> +qq.lo -= 1;
> +mulu64(, , qq.lo, v.lo);
> +hi += qq.lo * v.hi;
> +}
> +
> +*q = qq;
> +u.hi -= hi + (u.lo < lo);
> +u.lo -= lo;
> +return u;
> +}
> +}
> +
> +Int128 int128_divu(Int128 a, Int128 b)
> +{
> +Int128 q;
> +divrem128(a, b, );
> +return q;
> +}
> +
> +Int128 int128_remu(Int128 a, Int128 b)
> +{
> +Int128 q;
> +return divrem128(a, b, );
> +}
> +
> +Int128 int128_divs(Int128 a, Int128 b)
> +{
> +Int128 q;
> +bool sgna = !int128_nonneg(a);
> +bool sgnb = !int128_nonneg(b);
> +
> +if (sgna) {
> +a = int128_neg(a);
> +}
> +
> +if (sgnb) {
> +b = int128_neg(b);
> +}
> +
> +divrem128(a, b, );
> +
> +if (sgna != sgnb) {
> +q = int128_neg(q);
> +}
> +
> +return q;
> +}
> +
> +Int128 int128_rems(Int128 a, Int128 b)
> +{
> +Int128 q, r;
> +bool sgna = !int128_nonneg(a);
> +bool sgnb = !int128_nonneg(b);
> +
> +if (sgna) {
> +a = int128_neg(a);
> +}
> +
> +if (sgnb) {
> +b = int128_neg(b);
> +}
> +
> +r = divrem128(a, b, );
> +
> +if (sgna) {
> +r = int128_neg(r);
> +}
> +
> +return r;
> +}
> +
> +#endif
> diff --git a/util/meson.build b/util/meson.build
> index 05b593055a..e676b2f6c6 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -48,6 +48,7 @@ util_ss.add(files('transactions.c'))
> util_ss.add(when: 'CONFIG_POSIX', if_true: files('drm.c'))
> util_ss.add(files('guest-random.c'))
> util_ss.add(files('yank.c'))
> +util_ss.add(files('int128.c'))
>
> if have_user
>util_ss.add(files('selfmap.c'))
> --
> 2.33.1
>
>
Re: [qemu-web PATCH v2] Add Sponsors page
On Fri, Nov 19, 2021 at 12:12 AM Philippe Mathieu-Daudé wrote: > > Cc'ing Alistair regarding the RISC-V foundation help: > https://www.cnx-software.com/2021/05/03/the-risc-v-foundation-to-give-away-1000-risc-v-development-boards/ Thanks! We have access to RISC-V boards for QEMU tests from the PLCT Lab (https://github.com/plctlab) Alistair > > On 11/18/21 13:29, Philippe Mathieu-Daudé wrote: > > Add a page listing QEMU sponsors. > > > > For now, only mention Fosshost which requested to be listed: > > https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg05381.html > > > > Cc: Thomas Markey > > Resolves: https://gitlab.com/qemu-project/qemu-web/-/issues/2 > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > Since v1: > > - move to footer (Daniel) > > - only list sponsor who asked to be listed (Stefan) > > --- > > > diff --git a/sponsors.md b/sponsors.md > > new file mode 100644 > > index 000..1c097c8 > > --- /dev/null > > +++ b/sponsors.md > > @@ -0,0 +1,9 @@ > > +--- > > +title: QEMU sponsors > > +permalink: /sponsors/ > > +--- > > + > > +QEMU has sponsors! > > + > > +For continuous integration and testing, hardware is provided by: > > +- [Fosshost](https://fosshost.org/) > > > >
Re: [PATCH v5 2/6] QIOChannelSocket: Add flags parameter for writing
Hello Daniel, Thanks for reviewing! Best regards, Leo On Fri, Nov 12, 2021 at 7:15 AM Daniel P. Berrangé wrote: > > On Fri, Nov 12, 2021 at 02:10:37AM -0300, Leonardo Bras wrote: > > Change qio_channel_socket_writev() in order to accept flags, so its possible > > to selectively make use of sendmsg() flags. > > > > qio_channel_socket_writev() contents were moved to a helper function > > qio_channel_socket_writev_flags() which accepts an extra argument for flags. > > (This argument is passed directly to sendmsg(). > > > > Signed-off-by: Leonardo Bras > > --- > > io/channel-socket.c | 26 +++--- > > 1 file changed, 19 insertions(+), 7 deletions(-) > > Reviewed-by: Daniel P. Berrangé > > > 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 v5 3/6] QIOChannelSocket: Implement io_writev_zerocopy & io_flush_zerocopy for CONFIG_LINUX
Hello Daniel,
On Fri, Nov 12, 2021 at 7:54 AM Daniel P. Berrangé wrote:
[...]
> > @@ -561,12 +577,15 @@ static ssize_t
> > qio_channel_socket_writev_flags(QIOChannel *ioc,
> > retry:
> > ret = sendmsg(sioc->fd, , flags);
> > if (ret <= 0) {
> > -if (errno == EAGAIN) {
> > +switch (errno) {
> > +case EAGAIN:
> > return QIO_CHANNEL_ERR_BLOCK;
> > -}
> > -if (errno == EINTR) {
> > +case EINTR:
> > goto retry;
> > +case ENOBUFS:
> > +return QIO_CHANNEL_ERR_NOBUFS;
>
> Why does ENOBUFS need handling separately instead of letting
> the error_setg_errno below handle it ?
>
> The caller immediately invokes error_setg_errno() again,
> just with different error message.
>
> No code in this series ever looks at QIO_CHANNEL_ERR_NOBUFS
> either, so we don't even need that special error return code
> added AFAICT ?
>
The idea was to add a custom message for ENOBUFS return when sending
with MSG_ZEROCOPY.
I mean, having this message is important for the user to understand
why the migration is failing, but it would
not make any sense to have this message while a non-zerocopy sendmsg()
returns with ENOBUFS.
ENOBUFS : The output queue for a network interface was full. This
generally indicates that the interface has stopped sending, but may be
caused by transient congestion.
As an alternative, I could add this message inside the switch, inside
an if (flags & MSG_ZEROCOPY) on qio_channel_socket_writev_flags()
instead of in it's caller.
But for me it looks bloated, I mean, dealing with an error for
ZEROCOPY only in the general function.
OTOH, if you think that it's a better idea to deal with every error in
qio_channel_socket_writev_flags() instead of in the caller, I will
change it for v6. Please let me know.
> > }
> > +
> > error_setg_errno(errp, errno,
> > "Unable to write to socket");
> > return -1;
> > @@ -670,6 +689,127 @@ static ssize_t qio_channel_socket_writev(QIOChannel
> > *ioc,
> > }
> > #endif /* WIN32 */
> >
> > +
> > +#ifdef CONFIG_LINUX
> > +
> > +static int qio_channel_socket_poll(QIOChannelSocket *sioc, bool zerocopy,
> > + Error **errp)
>
> There's only one caller and it always passes zerocopy=true,
> so this parmeter looks pointless.
I did that for possible reuse of this function in the future:
- As of today, this is certainly compiled out, but if at some point
someone wants to use poll for something other
than the reading of an zerocopy errqueue, it could be reused.
But sure, if that's not desirable, I can remove the parameter (and the
if clause for !zerocopy).
>
> > +{
> > +struct pollfd pfd;
> > +int ret;
> > +
> > +pfd.fd = sioc->fd;
> > +pfd.events = 0;
> > +
> > + retry:
> > +ret = poll(, 1, -1);
> > +if (ret < 0) {
> > +switch (errno) {
> > +case EAGAIN:
> > +case EINTR:
> > +goto retry;
> > +default:
> > +error_setg_errno(errp, errno,
> > + "Poll error");
> > +return ret;
>
>return -1;
>
> > +}
> > +}
> > +
> > +if (pfd.revents & (POLLHUP | POLLNVAL)) {
> > +error_setg(errp, "Poll error: Invalid or disconnected fd");
> > +return -1;
> > +}
> > +
> > +if (!zerocopy && (pfd.revents & POLLERR)) {
> > +error_setg(errp, "Poll error: Errors present in errqueue");
> > +return -1;
> > +}
>
> > +
> > +return ret;
>
> return 0;
In the idea of future reuse I spoke above, returning zero here would
make this function always look like the poll timed out. Some future
users may want to repeat the waiting if poll() timed out, or if
(return > 0) stop polling.
(That was an earlier approach of this series)
>
> > +}
> > +
> > +static ssize_t qio_channel_socket_writev_zerocopy(QIOChannel *ioc,
> > + const struct iovec *iov,
> > + size_t niov,
> > + Error **errp)
> > +{
> > +QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
> > +ssize_t ret;
> > +
> > +ret = qio_channel_socket_writev_flags(ioc, iov, niov, NULL, 0,
> > + MSG_ZEROCOPY, errp);
> > +if (ret == QIO_CHANNEL_ERR_NOBUFS) {
> > +error_setg_errno(errp, errno,
> > + "Process can't lock enough memory for using
> > MSG_ZEROCOPY");
>
> This should not be touching errno - the method should be setting the
> errp directly, not leaving it to the caller.
>
Discussed above.
If you think that's a better approach I can change for v6.
> > +return -1;
> > +}
> > +
> > +sioc->zerocopy_queued++;
>
> if (ret > 0)
> sio->zerocopy_queued++
>
Nice catch!
> since the kernel doesn't increase the counter if the data sent
> was zero
[PULL 7/7] python/aqmp: fix send_fd_scm for python 3.6.x
3.6 doesn't play keepaway with the socket object, so we don't need to go
fishing for it on this version. In fact, so long as 'sendmsg' is still
available, it's probably preferable to just use that method and only go
fishing for forbidden details when we absolutely have to.
Reported-by: Thomas Huth
Signed-off-by: John Snow
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-8-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/aqmp/qmp_client.py | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
index f987da02eb..8105e29fa8 100644
--- a/python/qemu/aqmp/qmp_client.py
+++ b/python/qemu/aqmp/qmp_client.py
@@ -639,9 +639,12 @@ def send_fd_scm(self, fd: int) -> None:
if sock.family != socket.AF_UNIX:
raise AQMPError("Sending file descriptors requires a UNIX socket.")
-# Void the warranty sticker.
-# Access to sendmsg in asyncio is scheduled for removal in Python 3.11.
-sock = sock._sock # pylint: disable=protected-access
+if not hasattr(sock, 'sendmsg'):
+# We need to void the warranty sticker.
+# Access to sendmsg is scheduled for removal in Python 3.11.
+# Find the real backing socket to use it anyway.
+sock = sock._sock # pylint: disable=protected-access
+
sock.sendmsg(
[b' '],
[(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
--
2.31.1
[PULL 5/7] python/machine: handle "fast" QEMU terminations
In the case that the QEMU process actually launches -- but then dies so
quickly that we can't establish a QMP connection to it -- QEMUMachine
currently calls _post_shutdown() assuming that it never launched the VM
process.
This isn't true, though: it "merely" may have failed to establish a QMP
connection and the process is in the middle of its own exit path.
If we don't wait for the subprocess, the caller may get a bogus `None`
return for .exitcode(). This behavior was observed from
device-crash-test; after the switch to Async QMP, the timings were
changed such that it was now seemingly possible to witness the failure
of "vm.launch()" *prior* to the exitcode becoming available.
The semantic of the `_launched` property is changed in this
patch. Instead of representing the condition "launch() executed
successfully", it will now represent "has forked a child process
successfully". This way, wait() when called in the exit path won't
become a no-op.
Signed-off-by: John Snow
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-6-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/machine/machine.py | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index f92e73de40..67ab06ca2b 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -349,9 +349,6 @@ def _post_shutdown(self) -> None:
Called to cleanup the VM instance after the process has exited.
May also be called after a failed launch.
"""
-# Comprehensive reset for the failed launch case:
-self._early_cleanup()
-
try:
self._close_qmp_connection()
except Exception as err: # pylint: disable=broad-except
@@ -400,9 +397,16 @@ def launch(self) -> None:
try:
self._launch()
-self._launched = True
except:
-self._post_shutdown()
+# We may have launched the process but it may
+# have exited before we could connect via QMP.
+# Assume the VM didn't launch or is exiting.
+# If we don't wait for the process, exitcode() may still be
+# 'None' by the time control is ceded back to the caller.
+if self._launched:
+self.wait()
+else:
+self._post_shutdown()
LOG.debug('Error launching VM')
if self._qemu_full_args:
@@ -426,6 +430,7 @@ def _launch(self) -> None:
stderr=subprocess.STDOUT,
shell=False,
close_fds=False)
+self._launched = True
self._post_launch()
def _close_qmp_connection(self) -> None:
@@ -457,8 +462,8 @@ def _early_cleanup(self) -> None:
"""
Perform any cleanup that needs to happen before the VM exits.
-May be invoked by both soft and hard shutdown in failover scenarios.
-Called additionally by _post_shutdown for comprehensive cleanup.
+This method may be called twice upon shutdown, once each by soft
+and hard shutdown in failover scenarios.
"""
# If we keep the console socket open, we may deadlock waiting
# for QEMU to exit, while QEMU is waiting for the socket to
--
2.31.1
[PULL 6/7] scripts/device-crash-test: Use a QMP timeout
Despite all the previous fixes, it's still possible for
device-crash-test to wedge itself in the case that QEMU terminates *so
quickly* that it doesn't even begin a connection attempt to our QMP
client. Python will just joyfully wait ad infinitum for a connection
that will now never arrive.
The real fix is to use asyncio to simultaneously poll both the health of
the launched process AND the connection attempt. That's quite a bit more
invasive than just setting a connection timeout, though.
Do the very simplest thing for now.
Signed-off-by: John Snow
Message-id: 2028204620.1897674-7-js...@redhat.com
Signed-off-by: John Snow
---
scripts/device-crash-test | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 1c73dac93e..7fbd99158b 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -353,7 +353,7 @@ def checkOneCase(args, testcase):
'-device', qemuOptsEscape(device)]
cmdline = ' '.join([binary] + args)
dbg("will launch QEMU: %s", cmdline)
-vm = QEMUMachine(binary=binary, args=args)
+vm = QEMUMachine(binary=binary, args=args, qmp_timer=15)
exc = None
exc_traceback = None
--
2.31.1
[PULL 4/7] python/machine: move more variable initializations to _pre_launch
No need to clear them only to set them later.
Signed-off-by: John Snow
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-5-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/machine/machine.py | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index ad529fd92a..f92e73de40 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -327,6 +327,14 @@ def _pre_launch(self) -> None:
self._qemu_log_path = os.path.join(self.log_dir, self._name + ".log")
self._qemu_log_file = open(self._qemu_log_path, 'wb')
+self._iolog = None
+self._qemu_full_args = tuple(chain(
+self._wrapper,
+[self._binary],
+self._base_args,
+self._args
+))
+
def _post_launch(self) -> None:
if self._qmp_connection:
self._qmp.accept(self._qmp_timer)
@@ -390,8 +398,6 @@ def launch(self) -> None:
if self._launched:
raise QEMUMachineError('VM already launched')
-self._iolog = None
-self._qemu_full_args = ()
try:
self._launch()
self._launched = True
@@ -410,12 +416,6 @@ def _launch(self) -> None:
Launch the VM and establish a QMP connection
"""
self._pre_launch()
-self._qemu_full_args = tuple(
-chain(self._wrapper,
- [self._binary],
- self._base_args,
- self._args)
-)
LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
# Cleaning up of this subprocess is guaranteed by _do_shutdown.
--
2.31.1
[PULL 3/7] python/machine: add instance disambiguator to default nickname
If you create two instances of QEMUMachine(), they'll both create the
same nickname by default -- which is not that helpful.
Luckily, they'll both create unique temporary directories ... but due to
user configuration, they may share logging and sockfile directories,
meaning two instances can collide. The Python logging will also be quite
confusing, with no differentiation between the two instances.
Add an instance disambiguator (The memory address of the instance) to
the default nickname to foolproof this in all cases.
Signed-off-by: John Snow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-4-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/machine/machine.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index ea9e07805d..ad529fd92a 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -133,7 +133,7 @@ def __init__(self,
self._wrapper = wrapper
self._qmp_timer = qmp_timer
-self._name = name or "qemu-%d" % os.getpid()
+self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
self._temp_dir: Optional[str] = None
self._base_temp_dir = base_temp_dir
self._sock_dir = sock_dir
--
2.31.1
[PULL 1/7] python/machine: add @sock_dir property
Analogous to temp_dir and log_dir, add a sock_dir property that defaults
to @temp_dir -- instead of base_temp_dir -- when the user hasn't
overridden the sock dir value in the initializer.
This gives us a much more unique directory to put sockfiles in by default.
Signed-off-by: John Snow
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-2-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/machine/machine.py | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a487c39745..b1dd77b538 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -134,8 +134,9 @@ def __init__(self,
self._qmp_timer = qmp_timer
self._name = name or "qemu-%d" % os.getpid()
+self._temp_dir: Optional[str] = None
self._base_temp_dir = base_temp_dir
-self._sock_dir = sock_dir or self._base_temp_dir
+self._sock_dir = sock_dir
self._log_dir = log_dir
if monitor_address is not None:
@@ -143,7 +144,7 @@ def __init__(self,
self._remove_monitor_sockfile = False
else:
self._monitor_address = os.path.join(
-self._sock_dir, f"{self._name}-monitor.sock"
+self.sock_dir, f"{self._name}-monitor.sock"
)
self._remove_monitor_sockfile = True
@@ -163,14 +164,13 @@ def __init__(self,
self._qmp_set = True # Enable QMP monitor by default.
self._qmp_connection: Optional[QEMUMonitorProtocol] = None
self._qemu_full_args: Tuple[str, ...] = ()
-self._temp_dir: Optional[str] = None
self._launched = False
self._machine: Optional[str] = None
self._console_index = 0
self._console_set = False
self._console_device_type: Optional[str] = None
self._console_address = os.path.join(
-self._sock_dir, f"{self._name}-console.sock"
+self.sock_dir, f"{self._name}-console.sock"
)
self._console_socket: Optional[socket.socket] = None
self._remove_files: List[str] = []
@@ -816,6 +816,15 @@ def temp_dir(self) -> str:
dir=self._base_temp_dir)
return self._temp_dir
+@property
+def sock_dir(self) -> str:
+"""
+Returns the directory used for sockfiles by this machine.
+"""
+if self._sock_dir:
+return self._sock_dir
+return self.temp_dir
+
@property
def log_dir(self) -> str:
"""
--
2.31.1
[PULL 2/7] python/machine: remove _remove_monitor_sockfile property
It doesn't matter if it was the user or the class itself that specified
where the sockfile should be created; the fact is that if we are using a
sockfile here, we created it and we can clean it up.
Signed-off-by: John Snow
Reviewed-by: Willian Rampazzo
Message-id: 2028204620.1897674-3-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/machine/machine.py | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index b1dd77b538..ea9e07805d 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -141,12 +141,10 @@ def __init__(self,
if monitor_address is not None:
self._monitor_address = monitor_address
-self._remove_monitor_sockfile = False
else:
self._monitor_address = os.path.join(
self.sock_dir, f"{self._name}-monitor.sock"
)
-self._remove_monitor_sockfile = True
self._console_log_path = console_log
if self._console_log_path:
@@ -315,8 +313,7 @@ def _pre_launch(self) -> None:
self._remove_files.append(self._console_address)
if self._qmp_set:
-if self._remove_monitor_sockfile:
-assert isinstance(self._monitor_address, str)
+if isinstance(self._monitor_address, str):
self._remove_files.append(self._monitor_address)
self._qmp_connection = QEMUMonitorProtocol(
self._monitor_address,
--
2.31.1
[PULL 0/7] Python patches
The following changes since commit 89d2f9e4c63799f7f03e9180c63b7dc45fc2a04a: Merge tag 'pull-target-arm-20211122' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2021-11-22 16:35:54 +0100) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to a57cb3e23d5ac918a69d0aab918470ff0b429ff9: python/aqmp: fix send_fd_scm for python 3.6.x (2021-11-22 18:41:21 -0500) Python testing fixes for 6.2 A few more fixes to help eliminate race conditions from device-crash-test, along with a fix that allows the SCM_RIGHTS functionality to work on hosts that only have Python 3.6. If this is too much this late in the RC process, I'd advocate for at least patch 7/7 by itself. John Snow (7): python/machine: add @sock_dir property python/machine: remove _remove_monitor_sockfile property python/machine: add instance disambiguator to default nickname python/machine: move more variable initializations to _pre_launch python/machine: handle "fast" QEMU terminations scripts/device-crash-test: Use a QMP timeout python/aqmp: fix send_fd_scm for python 3.6.x python/qemu/aqmp/qmp_client.py | 9 -- python/qemu/machine/machine.py | 59 -- scripts/device-crash-test | 2 +- 3 files changed, 42 insertions(+), 28 deletions(-) -- 2.31.1
Re: [RFC v2 PATCH 13/13] KVM: Enable memfd based page invalidation/fallocate
On Mon, Nov 22, 2021 at 05:16:47PM +0300, Kirill A. Shutemov wrote:
> On Fri, Nov 19, 2021 at 09:47:39PM +0800, Chao Peng wrote:
> > Since the memory backing store does not get notified when VM is
> > destroyed so need check if VM is still live in these callbacks.
> >
> > Signed-off-by: Yu Zhang
> > Signed-off-by: Chao Peng
> > ---
> > virt/kvm/memfd.c | 22 ++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/virt/kvm/memfd.c b/virt/kvm/memfd.c
> > index bd930dcb455f..bcfdc685ce22 100644
> > --- a/virt/kvm/memfd.c
> > +++ b/virt/kvm/memfd.c
> > @@ -12,16 +12,38 @@
> > #include
> > const static struct guest_mem_ops *memfd_ops;
> >
> > +static bool vm_is_dead(struct kvm *vm)
> > +{
> > + struct kvm *kvm;
> > +
> > + list_for_each_entry(kvm, _list, vm_list) {
> > + if (kvm == vm)
> > + return false;
> > + }
>
> I don't think this is enough. The struct kvm can be freed and re-allocated
> from the slab and this function will give false-negetive.
Right.
>
> Maybe the kvm has to be tagged with a sequential id that incremented every
> allocation. This id can be checked here.
Sounds like a sequential id will be needed, no existing fields in struct
kvm can work for this.
>
> > +
> > + return true;
> > +}
>
> --
> Kirill A. Shutemov
Re: [PULL 0/6] Documentation updates
On 11/22/21 3:43 PM, Thomas Huth wrote: Hi Richard! The following changes since commit c5fbdd60cf1fb52f01bdfe342b6fa65d5343e1b1: Merge tag 'qemu-sparc-20211121' of git://github.com/mcayland/qemu into staging (2021-11-21 14:12:25 +0100) are available in the Git repository at: https://gitlab.com/thuth/qemu.git tags/pull-request-2021-11-22 for you to fetch changes up to c5ba62195427d65a44472901cff3dddffc14b3b3: docs: Render binary names as monospaced text (2021-11-22 15:02:38 +0100) * Documentation updates Kashyap Chamarthy (2): docs: Fix botched rST conversion of 'submitting-a-patch.rst' docs: List more commit-message tags in "submitting-a-patch" Philippe Mathieu-Daudé (1): docs: Render binary names as monospaced text Rao, Lei (2): docs: Drop deprecated 'props' from object-add docs: Use double quotes instead of single quotes for COLO Stefan Weil (1): Fix some typos in documentation (found by codespell) docs/COLO-FT.txt | 106 docs/about/removed-features.rst | 8 +- docs/block-replication.txt | 52 docs/devel/build-system.rst | 6 +- docs/devel/multi-process.rst | 8 +- docs/devel/qgraph.rst| 2 +- docs/devel/stable-process.rst| 2 + docs/devel/style.rst | 2 + docs/devel/submitting-a-patch.rst| 202 +++ docs/devel/submitting-a-pull-request.rst | 9 +- docs/devel/testing.rst | 8 +- docs/devel/trivial-patches.rst | 2 + docs/devel/writing-monitor-commands.rst | 2 +- docs/hyperv.txt | 2 +- docs/image-fuzzer.txt| 6 +- docs/system/arm/orangepi.rst | 2 +- docs/system/authz.rst| 26 ++-- docs/system/cpu-models-x86.rst.inc | 2 +- docs/system/devices/nvme.rst | 2 +- docs/system/gdb.rst | 2 +- docs/system/images.rst | 2 +- docs/system/ppc/ppce500.rst | 2 +- docs/system/qemu-block-drivers.rst.inc | 6 +- docs/system/riscv/shakti-c.rst | 2 +- docs/system/tls.rst | 2 +- docs/throttle.txt| 8 +- docs/tools/qemu-img.rst | 18 +-- docs/tools/qemu-nbd.rst | 6 +- docs/tools/qemu-storage-daemon.rst | 7 +- docs/tools/virtiofsd.rst | 4 +- 30 files changed, 307 insertions(+), 201 deletions(-) Applied, thanks. r~
Re: [PATCH v1] virtio-mem: Don't skip alignment checks when warning about block size
On Mon, Nov 22, 2021 at 01:01:51PM +0100, David Hildenbrand wrote:
> On 12.10.21 09:27, Michael S. Tsirkin wrote:
> > On Mon, Oct 11, 2021 at 07:33:05PM +0200, David Hildenbrand wrote:
> >> If we warn about the block size being smaller than the default, we skip
> >> some alignment checks.
> >>
> >> This can currently only fail on x86-64, when specifying a block size of
> >> 1 MiB, however, we detect the THP size of 2 MiB.
> >>
> >> Fixes: 228957fea3a9 ("virtio-mem: Probe THP size to determine default
> >> block size")
> >> Cc: "Michael S. Tsirkin"
> >> Signed-off-by: David Hildenbrand
> >
> > Reviewed-by: Michael S. Tsirkin
>
> Thanks Michael, will you send this for the v6.2 release?
>
>
> --
> Thanks,
>
> David / dhildenb
sure
Re: [PATCH v5 1/6] QIOChannel: Add io_writev_zerocopy & io_flush_zerocopy callbacks
Hello Daniel, Thanks for the feedback! On Fri, Nov 12, 2021 at 7:13 AM Daniel P. Berrangé wrote: > > On Fri, Nov 12, 2021 at 02:10:36AM -0300, Leonardo Bras wrote: > > -int qio_channel_writev_all(QIOChannel *ioc, > > - const struct iovec *iov, > > - size_t niov, > > - Error **erp); > > +int qio_channel_writev_all_flags(QIOChannel *ioc, > > + const struct iovec *iov, > > + size_t niov, > > + int flags, > > + Error **errp); > > +#define qio_channel_writev_all(ioc, iov, niov, errp) \ > > +qio_channel_writev_all_flags(ioc, iov, niov, 0, errp) > > We already have separate methods for zerocopy, instead of adding > flags, so we shouldn't add flags to this either. > > Add a qio_channel_writev_zerocopy_all method instead. > > Internally, we can still make both qio_channel_writev_zerocopy_all > and qio_channel_writev_all use the same helper method, just don't > expose flags in the public API. Even internally we don't really > need flags, just a bool I see. The idea of having a flag was to make it easier to expand the interface in the future. I got some feedback on v1 that would suggest it would be desired: http://patchwork.ozlabs.org/project/qemu-devel/patch/20210831110238.299458-2-leob...@redhat.com/ > [...] > > +#define qio_channel_writev_full_all(ioc, iov, niov, fds, nfds, errp) \ > > +qio_channel_writev_full_all_flags(ioc, iov, niov, fds, nfds, 0, errp) > > There's no need for this at all. Since fd passing is not supported > with zerocopy, there's no reason to ever use this method. > > > +/** > > + * qio_channel_writev_zerocopy: > > + * @ioc: the channel object > > + * @iov: the array of memory regions to write data from > > + * @niov: the length of the @iov array > > + * @errp: pointer to a NULL-initialized error object > > + * > > + * Behaves like qio_channel_writev_full_all_flags, but may write > > qio_channel_writev > > > + * data asynchronously while avoiding unnecessary data copy. > > + * This function may return before any data is actually written, > > + * but should queue every buffer for writing. > > Callers mustn't rely on "should" docs - they must rely on the > return value indicating how many bytes were accepted. > > Also mention that this requires locked memory and can/will fail if > insufficient locked memory is available. > Sure, I will update that. > > +/** > > + * qio_channel_flush_zerocopy: > > + * @ioc: the channel object > > + * @errp: pointer to a NULL-initialized error object > > + * > > + * Will block until every packet queued with > > + * qio_channel_writev_zerocopy() is sent, or return > > + * in case of any error. > > + * > > + * Returns -1 if any error is found, 0 otherwise. > > Returns -1 if any error is found, 0 if all data was sent, >or 1 if all data was sent but at least some was copied. > I don't really get the return 1 part, I mean, per description it will 'block until every queued packet was sent, so "at least some was copied" doesn't seem to fit here. Could you elaborate? Best regards, Leo
Re: [RFC PATCH] s390: kvm: reduce frequency of CPU syncs of diag318 info
On 11/22/21 17:33, Collin Walling wrote:
> DIAGNOSE 0318 is invoked only once during IPL. As such, the
> diag318 info will only change once initially and during resets.
> Let's only sync the register to convey the info to KVM if and
> only if the diag318 info has changed. Only set the dirty bit
> flag for diag318 whenever it must be updated.
>
> The migration handler will invoke the set_diag318 helper on
> post_load to ensure the info is set on the destination machine.
>
> Signed-off-by: Collin Walling
This is a long-overdue response to this thread:
https://www.spinics.net/lists/kvm/msg258071.html
> ---
> target/s390x/kvm/kvm.c | 5 -
> target/s390x/machine.c | 14 ++
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 6acf14d5ec..6a141399f7 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -599,11 +599,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
> }
>
> -if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
> -cs->kvm_run->s.regs.diag318 = env->diag318_info;
> -cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
> -}
> -
> /* Finally the prefix */
> if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
> cs->kvm_run->s.regs.prefix = env->psa;
> diff --git a/target/s390x/machine.c b/target/s390x/machine.c
> index 37a076858c..a5d113ce3a 100644
> --- a/target/s390x/machine.c
> +++ b/target/s390x/machine.c
> @@ -234,6 +234,19 @@ const VMStateDescription vmstate_etoken = {
> }
> };
>
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> +S390CPU *cpu = opaque;
> +CPUState *cs = CPU(cpu);
> +CPUS390XState *env = _CPU(cs)->env;
> +
> +if (kvm_enabled()) {
> +kvm_s390_set_diag318(cs, env->diag318_info);
> +}
> +
> +return 0;
> +}
> +
> static bool diag318_needed(void *opaque)
> {
> return s390_has_feat(S390_FEAT_DIAG_318);
> @@ -243,6 +256,7 @@ const VMStateDescription vmstate_diag318 = {
> .name = "cpu/diag318",
> .version_id = 1,
> .minimum_version_id = 1,
> +.post_load = diag318_post_load,
> .needed = diag318_needed,
> .fields = (VMStateField[]) {
> VMSTATE_UINT64(env.diag318_info, S390CPU),
>
--
Regards,
Collin
Stay safe and stay healthy
Re: [PATCH v1 0/9] hw/mos6522: VIA timer emulation fixes and improvements
On Mon, 22 Nov 2021, Peter Maydell wrote: > On Sat, 20 Nov 2021 at 23:40, Finn Thain wrote: > > Anyway, thanks for taking the time to write. A competent reviewer has to > > do much more than that, but I'm not paying for competence so I suppose I'm > > asking too much. > > Please dial back the aggressive tone here, Finn: this kind of > thing is way out of line. We're all trying to help improve QEMU here, > and sniping at Mark is not constructive. > Peter, you seem to have misunderstood what I wrote. What I said was not sniping. "Incompetent" was my conclusion after I judiciously rejected "malicious". Here's what I mean by incompetent. CONTRIBUTOR: Here's a patch. MAINTAINER: I personally don't like that pattern. End of story. CONTRIBUTOR: I don't think I'll contribute further to this project. [Everyone loses.] Now, here's what I would consider "competent": CONTRIBUTOR: Here's a patch. MAINTAINER: That pattern (I've quoted it to help further the discussion) is widely deprecated. You should use a different pattern instead. [Or read this reference. Or refer to this code.] CONTRIBUTOR: OK, I see that this really is a problem, and I see that there really is a better way. However, the antipattern is already part of existing code, and my changes don't worsen the problem, and don't require that the problem persist. MAINTAINER: You're right. My bad (I'm new to this). Since I never bothered to fix the existing antipattern, and no-one else thought it was worth fixing either, clearly it's not that important, and I should not have sought to veto your work, which is substantially unrelated, and beneficial either way. CONTRIBUTOR: No problem. [Everyone wins.] Finally, here's the background for you to ponder, in case you would like to intervene to produce a better outcome. (I think you are potentially well positioned for that.) https://lore.kernel.org/qemu-devel/cover.1629799776.git.fth...@linux-m68k.org/ https://lore.kernel.org/qemu-devel/cover.1632437396.git.fth...@linux-m68k.org/
[RFC PATCH] s390: kvm: reduce frequency of CPU syncs of diag318 info
DIAGNOSE 0318 is invoked only once during IPL. As such, the
diag318 info will only change once initially and during resets.
Let's only sync the register to convey the info to KVM if and
only if the diag318 info has changed. Only set the dirty bit
flag for diag318 whenever it must be updated.
The migration handler will invoke the set_diag318 helper on
post_load to ensure the info is set on the destination machine.
Signed-off-by: Collin Walling
---
target/s390x/kvm/kvm.c | 5 -
target/s390x/machine.c | 14 ++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 6acf14d5ec..6a141399f7 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -599,11 +599,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
}
-if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
-cs->kvm_run->s.regs.diag318 = env->diag318_info;
-cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
-}
-
/* Finally the prefix */
if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
cs->kvm_run->s.regs.prefix = env->psa;
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 37a076858c..a5d113ce3a 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -234,6 +234,19 @@ const VMStateDescription vmstate_etoken = {
}
};
+static int diag318_post_load(void *opaque, int version_id)
+{
+S390CPU *cpu = opaque;
+CPUState *cs = CPU(cpu);
+CPUS390XState *env = _CPU(cs)->env;
+
+if (kvm_enabled()) {
+kvm_s390_set_diag318(cs, env->diag318_info);
+}
+
+return 0;
+}
+
static bool diag318_needed(void *opaque)
{
return s390_has_feat(S390_FEAT_DIAG_318);
@@ -243,6 +256,7 @@ const VMStateDescription vmstate_diag318 = {
.name = "cpu/diag318",
.version_id = 1,
.minimum_version_id = 1,
+.post_load = diag318_post_load,
.needed = diag318_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT64(env.diag318_info, S390CPU),
--
2.31.1
Re: [PULL 0/2] NBD patches for 6.2-rc2, 2021-11-22
On 11/22/21 3:02 PM, Eric Blake wrote: The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2: Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu into staging (2021-11-22 10:33:13 +0100) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-11-22 for you to fetch changes up to e35574226a63f29e32eda8da5cc14832f19850e2: nbd/server: Simplify zero and trim (2021-11-22 07:37:15 -0600) nbd patches for 2021-11-22 - Eric Blake: Avoid uninitialized memory on client hard disconnect - Eric Blake: Take advantage of block layer 64-bit zero/trim Eric Blake (2): nbd/server: Don't complain on certain client disconnects nbd/server: Simplify zero and trim nbd/server.c | 26 ++ 1 file changed, 6 insertions(+), 20 deletions(-) Applied, thanks. r~
Re: [PATCH v4 20/32] nbd/client-connection: implement connection retry
On Mon, Nov 22, 2021 at 08:17:34PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 22.11.2021 19:30, Eric Blake wrote: > > Reviving this thread, as a good as place as any for my question: > > > > But I can't > > find anywhere in the code base that ensures that on a reconnect, the > > new server supplies at least as many extensions as the original > > server, nor anywhere that we would be able to gracefully handle an > > in-flight block status command that can no longer be successfully > > continued because the reconnect landed on a downgraded server. > > > > Yes that's a problem. We previously noted it here > https://lists.gnu.org/archive/html/qemu-block/2021-06/msg00458.html Aha! And oops that we've let it slide this long. But now that we know the problem was also present in 6.1, and not a new regression in 6.2, it is less urgent to get a fix in. If we get one written in time, it's still game for inclusion for -rc2 or maybe -rc3, but as we get further along in the process, it should not hold up the final release (it would not be -rc4 material, for example). > > Honestly, I didn't start any fix for that :(.. I agree, it would be good to > fix it somehow in 6.2. I'll try to make something simple this week. Or did > you already started doing some fix? I haven't started writing anything on the topic. I've got some patches that I hope to post soon targetting 7.0 that allow an extension for NBD_CMD_BLOCK_STATUS to do a full round-trip 64-bit operation, which is why I noticed it (if the 64-bit extension is not present on reconnect, we have a problem - but it's no worse than the existing problems). But I'm currently so focused on getting the new feature interoperating between qemu, libnbd, and nbdkit, plus the US Thanksgiving break this week, that I'm happy to let you take first shot at client-side validation that a server reconnect does not lose capabilities. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH] MAINTAINERS: Remove me as a reviewer for the build and test/avocado
Willian Rampazzo writes: > Remove me as a reviewer for the Build and test automation and the > Integration Testing with the Avocado Framework and add Beraldo > Leal. Queued to for-6.2/tcg-gdb-plugin-fixes, thanks. -- Alex Bennée
Re: [PATCH for-6.2] hw/intc/arm_gicv3_its: Revert version increments in vmstate_its
Hi Peter,
On 11/22/21 7:04 PM, Peter Maydell wrote:
> On Mon, 22 Nov 2021 at 17:10, Eric Auger wrote:
>> Commit 18f6290a6a ("hw/intc: GICv3 ITS initial framework")
>> incremented version_id and minimum_version_id fields of
>> VMStateDescription vmstate_its. This breaks the migration between
>> 6.2 and 6.1 with the following message:
>>
>> qemu-system-aarch64: savevm: unsupported version 1 for 'arm_gicv3_its' v0
>> qemu-system-aarch64: load of migration failed: Invalid argument
>>
>> Revert that change.
>>
>> Signed-off-by: Eric Auger
>> ---
>> hw/intc/arm_gicv3_its_common.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
>> index 7d7f3882e76..90b85f1e25c 100644
>> --- a/hw/intc/arm_gicv3_its_common.c
>> +++ b/hw/intc/arm_gicv3_its_common.c
>> @@ -50,8 +50,6 @@ static int gicv3_its_post_load(void *opaque, int
>> version_id)
>>
>> static const VMStateDescription vmstate_its = {
>> .name = "arm_gicv3_its",
>> -.version_id = 1,
>> -.minimum_version_id = 1,
>> .pre_save = gicv3_its_pre_save,
>> .post_load = gicv3_its_post_load,
>> .priority = MIG_PRI_GICV3_ITS,
> Oops, I didn't notice that the version bump lines were still
> in the patchset I applied :-(
No problem. I did not notice either ;-)
Thanks!
Eric
>
> Reviewed-by: Peter Maydell
>
> -- PMM
>
Re: [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine
On Fri, Nov 19, 2021 at 2:29 PM Philippe Mathieu-Daudé wrote: > > Test NetBSD 9.2 on the Jazz Magnum machine. As the firmware is not > redistributable, it has to be extracted from the floppy configuration > disk coming with a Mips Magnum 4000 system, then the NTPROM_BIN_PATH > environment variable has to be set. For convenience a NVRAM pre- > initialized to boot NetBSD is included. The test can be run as: > > $ NTPROM_BIN_PATH=/path/to/ntprom.bin \ > avocado --show=app,console \ > run -t machine:magnum tests/avocado/ > Fetching asset from > tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2 >(1/1) tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2: > console: EISA Bus 0 Initialization In Progress... Direct Memory Access > (DMA) System Control Port B Timer 1 OK. > console: ARC Multiboot Version 174 (SGI Version 2.6) > console: Copyright (c) 1991,1992 Microsoft Corporation > console: Actions: > console: Start Windows NT > console: Run a program > console: Run setup > console: Use the arrow keys to select. > console: Press Enter to choose. > console: Program to run: > console: scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd > console: NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021) > console: devopen: scsi(0)cdrom(2)fdisk(0) type disk file netbsd > console: NetBSD 9.2 (RAMDISK) #0: Wed May 12 13:15:55 UTC 2021 > console: MIPS Magnum > console: total memory = 128 MB > console: avail memory = 117 MB > console: mainbus0 (root) > console: cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 > FPC Rev. 0.0 > console: cpu0: 8KB/16B direct-mapped L1 Instruction cache, 48 TLB entries > console: cpu0: 8KB/16B direct-mapped write-back L1 Data cache > console: jazzio0 at mainbus0 > console: timer0 at jazzio0 addr 0xe228 > console: mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible > time-of-day clock > console: LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured > console: fdc0 at jazzio0 addr 0xe0003000 intr 1 > console: fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec > console: MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured > console: VXL at jazzio0 addr 0xe080 intr 3 not configured > console: sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet > console: sn0: Ethernet address 00:00:00:00:00:00 > console: asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7 > console: scsibus0 at asc0: 8 targets, 8 luns per target > console: pckbc0 at jazzio0 addr 0xe0005000 intr 6 > console: pckbd0 at pckbc0 (kbd slot) > console: wskbd0 at pckbd0 (mux ignored) > console: pms at jazzio0 addr 0xe0005000 intr 7 not configured > console: com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo > console: com0: txfifo disabled > console: com0: console > console: com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo > console: com1: txfifo disabled > console: jazzisabr0 at mainbus0 > console: isa0 at jazzisabr0 > console: isapnp0 at isa0 port 0x279: ISA Plug 'n Play device support > console: scsibus0: waiting 2 seconds for devices to settle... > console: cd0 at scsibus0 target 2 lun 0: cdrom > removable > console: boot device: > console: root on md0a dumps on md0b > console: root file system type: ffs > console: WARNING: preposterous TOD clock time > console: WARNING: using filesystem time > console: WARNING: CHECK AND RESET THE DATE! > console: erase ^H, werase ^W, kill ^U, intr ^C, status ^T > console: Terminal type? [vt100] > console: Erase is backspace. > console: S > console: (I)nstall, (S)hell or (H)alt ? > console: # > console: # ifconfig > console: sn0: flags=0x8802 mtu 1500 > console: ec_capabilities=1 > console: ec_enabled=0 > console: address: 00:00:00:02:03:04 > console: lo0: flags=0x8048 mtu 33160 > console: # > console: # ifconfig sn0 10.0.2.3/24 > console: # > console: # ping -c 3 10.0.2.2 > console: PING 10.0.2.2 (10.0.2.2): 56 data bytes > console: 64 bytes from 10.0.2.2: icmp_seq=0 ttl=255 time=12.526 ms > console: 64 bytes from 10.0.2.2: icmp_seq=1 ttl=255 time=2.324 ms > console: 64 bytes from 10.0.2.2: icmp_seq=2 ttl=255 time=0.608 ms > console: 10.0.2.2 PING Statistics > console: 3 packets transmitted, 3 packets received, 0.0% packet loss > console: # shutdown -r now > console: round-trip min/avg/max/stddev = 0.608/5.153/12.526/6.443 ms > console: # Shutdown NOW! > console: shutdown: [pid 14] > console: # sh: /usr/bin/wall: not found > console: reboot by root: > console: System shutdown time has arrived > console: About to run shutdown hooks... > console: .: Can't open /etc/rc.shutdown > console: Done running shutdown hooks. > console: syncing disks... done > console: unmounting file systems... done > console: rebooting... > PASS (49.27 s) > RESULTS: PASS 1 | ERROR 0 | FAIL
Re: [PATCH] MAINTAINERS: Remove me as a reviewer for the build and test/avocado
On Mon, Nov 22, 2021 at 04:11:24PM -0300, Willian Rampazzo wrote: > Remove me as a reviewer for the Build and test automation and the > Integration Testing with the Avocado Framework and add Beraldo > Leal. > > Signed-off-by: Willian Rampazzo > --- > MAINTAINERS | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index d3879aa3c1..8f5156bfa7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3469,7 +3469,7 @@ M: Alex Bennée > M: Philippe Mathieu-Daudé > M: Thomas Huth > R: Wainer dos Santos Moschetta > -R: Willian Rampazzo > +R: Beraldo Leal > S: Maintained > F: .github/lockdown.yml > F: .gitlab-ci.yml > @@ -3507,7 +3507,7 @@ W: https://trello.com/b/6Qi1pxVn/avocado-qemu > R: Cleber Rosa > R: Philippe Mathieu-Daudé > R: Wainer dos Santos Moschetta > -R: Willian Rampazzo > +R: Beraldo Leal > S: Odd Fixes > F: tests/avocado/ Reviewed-by: Beraldo Leal -- Beraldo
[PATCH] MAINTAINERS: Remove me as a reviewer for the build and test/avocado
Remove me as a reviewer for the Build and test automation and the Integration Testing with the Avocado Framework and add Beraldo Leal. Signed-off-by: Willian Rampazzo --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d3879aa3c1..8f5156bfa7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3469,7 +3469,7 @@ M: Alex Bennée M: Philippe Mathieu-Daudé M: Thomas Huth R: Wainer dos Santos Moschetta -R: Willian Rampazzo +R: Beraldo Leal S: Maintained F: .github/lockdown.yml F: .gitlab-ci.yml @@ -3507,7 +3507,7 @@ W: https://trello.com/b/6Qi1pxVn/avocado-qemu R: Cleber Rosa R: Philippe Mathieu-Daudé R: Wainer dos Santos Moschetta -R: Willian Rampazzo +R: Beraldo Leal S: Odd Fixes F: tests/avocado/ -- 2.33.1
[PULL 1/1] hw/intc/arm_gicv3_its: Revert version increments in vmstate_its
From: Eric Auger
Commit 18f6290a6a ("hw/intc: GICv3 ITS initial framework")
incremented version_id and minimum_version_id fields of
VMStateDescription vmstate_its. This breaks the migration between
6.2 and 6.1 with the following message:
qemu-system-aarch64: savevm: unsupported version 1 for 'arm_gicv3_its' v0
qemu-system-aarch64: load of migration failed: Invalid argument
Revert that change.
Signed-off-by: Eric Auger
Message-id: 20211122171020.1195483-1-eric.au...@redhat.com
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/intc/arm_gicv3_its_common.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 7d7f3882e76..90b85f1e25c 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -50,8 +50,6 @@ static int gicv3_its_post_load(void *opaque, int version_id)
static const VMStateDescription vmstate_its = {
.name = "arm_gicv3_its",
-.version_id = 1,
-.minimum_version_id = 1,
.pre_save = gicv3_its_pre_save,
.post_load = gicv3_its_post_load,
.priority = MIG_PRI_GICV3_ITS,
--
2.25.1
[PULL 0/1] target-arm queue
Apologies for sending two pullreqs today; Eric's patch came in a few hours after I sent the first one but it's definitely a release-critical fix. -- PMM The following changes since commit 89d2f9e4c63799f7f03e9180c63b7dc45fc2a04a: Merge tag 'pull-target-arm-20211122' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2021-11-22 16:35:54 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211122-1 for you to fetch changes up to 33a0c404fb90a3fa8eea6ebf5c535fc7bc0b9912: hw/intc/arm_gicv3_its: Revert version increments in vmstate_its (2021-11-22 18:17:19 +) target-arm queue: * drop spurious bump of ITS vmstate version fields Eric Auger (1): hw/intc/arm_gicv3_its: Revert version increments in vmstate_its hw/intc/arm_gicv3_its_common.c | 2 -- 1 file changed, 2 deletions(-)
Re: [PULL 0/1] target-arm queue
On 11/22/21 7:06 PM, Peter Maydell wrote: On Mon, 22 Nov 2021 at 13:43, Peter Maydell wrote: Just one patch for rc2, a revert. -- PMM The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2: Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu into staging (2021-11-22 10:33:13 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211122 for you to fetch changes up to 4825eaae4fdd56fba0febdfbdd7bf9684ae3ee0d: Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2" (2021-11-22 13:41:48 +) target-arm queue: * revert SMCCC/PSCI change, as it regresses some usecases for some boards Since this hasn't been applied yet I'll reroll it to add Eric's "don't bump the ITS version fields" patch. (But if you get here first that's fine too.) Gitlab has been dreadfully slow this evening, but it's applied now. r~
Re: [PULL 0/1] target-arm queue
On Mon, 22 Nov 2021 at 13:43, Peter Maydell wrote: > > Just one patch for rc2, a revert. > > -- PMM > > The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2: > > Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu > into staging (2021-11-22 10:33:13 +0100) > > are available in the Git repository at: > > https://git.linaro.org/people/pmaydell/qemu-arm.git > tags/pull-target-arm-20211122 > > for you to fetch changes up to 4825eaae4fdd56fba0febdfbdd7bf9684ae3ee0d: > > Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2" (2021-11-22 13:41:48 > +) > > > target-arm queue: > * revert SMCCC/PSCI change, as it regresses some usecases for some boards Since this hasn't been applied yet I'll reroll it to add Eric's "don't bump the ITS version fields" patch. (But if you get here first that's fine too.) -- PMM
Re: [PATCH for-6.2] hw/intc/arm_gicv3_its: Revert version increments in vmstate_its
On Mon, 22 Nov 2021 at 17:10, Eric Auger wrote:
>
> Commit 18f6290a6a ("hw/intc: GICv3 ITS initial framework")
> incremented version_id and minimum_version_id fields of
> VMStateDescription vmstate_its. This breaks the migration between
> 6.2 and 6.1 with the following message:
>
> qemu-system-aarch64: savevm: unsupported version 1 for 'arm_gicv3_its' v0
> qemu-system-aarch64: load of migration failed: Invalid argument
>
> Revert that change.
>
> Signed-off-by: Eric Auger
> ---
> hw/intc/arm_gicv3_its_common.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index 7d7f3882e76..90b85f1e25c 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -50,8 +50,6 @@ static int gicv3_its_post_load(void *opaque, int version_id)
>
> static const VMStateDescription vmstate_its = {
> .name = "arm_gicv3_its",
> -.version_id = 1,
> -.minimum_version_id = 1,
> .pre_save = gicv3_its_pre_save,
> .post_load = gicv3_its_post_load,
> .priority = MIG_PRI_GICV3_ITS,
Oops, I didn't notice that the version bump lines were still
in the patchset I applied :-(
Reviewed-by: Peter Maydell
-- PMM
Re: [PATCH v3 3/3] cpus-common: implement dirty limit on vCPU
在 2021/11/22 19:26, Markus Armbruster 写道:
Hyman Huang writes:
在 2021/11/22 17:10, Markus Armbruster 写道:
Hyman Huang writes:
=E5=9C=A8 2021/11/22 15:35, Markus Armbruster =E5=86=99=E9=81=93:
huang...@chinatelecom.cn writes:
From: Hyman Huang(=E9=BB=84=E5=8B=87)
implement dirtyrate calculation periodically basing on
dirty-ring and throttle vCPU until it reachs the quota
dirtyrate given by user.
introduce qmp commands set-dirty-limit/cancel-dirty-limit to
set/cancel dirty limit on vCPU.
Please start sentences with a capital letter.
Ok，i'll check the syntax problem next version.
Signed-off-by: Hyman Huang(黄勇)
[...]
diff --git a/qapi/misc.json b/qapi/misc.json
index 358548a..98e6001 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -527,3 +527,42 @@
'data': { '*option': 'str' },
'returns': ['CommandLineOptionInfo'],
'allow-preconfig': true }
+
+##
+# @set-dirty-limit:
+#
+# This command could be used to cap the vCPU memory load, which is also
+# refered as dirtyrate. One should use "calc-dirty-rate" with "dirty-ring"
+# and to calculate vCPU dirtyrate and query it with "query-dirty-rate".
+# Once getting the vCPU current dirtyrate, "set-dirty-limit" can be used
+# to set the upper limit of dirtyrate for the interested vCPU.
"dirtyrate" is not a word. Let's spell it "dirty page rate", for
consistency with the documentation in migration.json.
Ok, sounds good.
Regarding "One should use ...": sounds like you have to run
calc-dirty-rate with argument @mode set to @dirty-ring before this
command. Correct? What happens when you don't? set-dirty-limit fails?
You didn't answer this question.
set-dirty-limit doesn't do any pre-check about if calc-dirty-rate has
executed, so it doesn't fail.
Peeking at qmp_set_dirty_limit()... it fails when
!kvm_dirty_ring_enabled(). kvm_dirty_ring_enabled() returns true when
kvm_state->kvm_dirty_ring_size is non-zero. How can it become non-zero?
If we enable dirty-ring with qemu commandline "-accel
kvm,dirty-ring-size=xxx",qemu will parse the dirty-ring-size and set it.
So we check if
dirty-ring is enabled by the kvm_dirty_ring_size.
Since only executing calc-dirty-rate with dirty-ring mode can we get
the vCPU dirty page rate currently(while the dirty-bitmap only get the
vm dirty page rate), "One should use ..." maybe misleading, what i
actually want to say is "One should use the dirty-ring mode to
calculate the vCPU dirty page rate".
I'm still confused on what exactly users must do for the page dirty rate
limiting to work as intended, and at least as importantly, what happens
when they get it wrong.
User can set-dirty-limit unconditionally and the dirtylimit will work.
"One should use ..." just emphasize if users want to know which vCPU is
in high memory load and want to limit it's dirty page rate, they can use
calc-dirty-rate but it is not prerequisite for set-dirty-limit.
Umm, I think "One should use ..." explanation make things complicated.
I'll reconsider the comment next version.
[...]
Re: [PATCH v4 20/32] nbd/client-connection: implement connection retry
22.11.2021 19:30, Eric Blake wrote:
Reviving this thread, as a good as place as any for my question:
On Thu, Jun 10, 2021 at 01:07:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Add an option for a thread to retry connection until succeeds. We'll
use nbd/client-connection both for reconnect and for initial connection
in nbd_open(), so we need a possibility to use same NBDClientConnection
instance to connect once in nbd_open() and then use retry semantics for
reconnect.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/nbd.h | 2 ++
nbd/client-connection.c | 56 +++--
2 files changed, 45 insertions(+), 13 deletions(-)
NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
bool do_negotiation,
const char *export_name,
@@ -154,23 +164,43 @@ static void *connect_thread_func(void *opaque)
NBDClientConnection *conn = opaque;
int ret;
bool do_free;
+uint64_t timeout = 1;
+uint64_t max_timeout = 16;
-conn->sioc = qio_channel_socket_new();
+while (true) {
+conn->sioc = qio_channel_socket_new();
-error_free(conn->err);
-conn->err = NULL;
-conn->updated_info = conn->initial_info;
+error_free(conn->err);
+conn->err = NULL;
+conn->updated_info = conn->initial_info;
-ret = nbd_connect(conn->sioc, conn->saddr,
- conn->do_negotiation ? >updated_info : NULL,
- conn->tlscreds, >ioc, >err);
This says that on each retry attempt, we reset whether to ask the
server for structured replies back to our original initial_info
values.
But when dealing with NBD retries in general, I suspect we have a bug.
Consider what happens if our first connection requests structured
replies and base:allocation block status, and we are successful. But
later, the server disconnects, triggering a retry. Suppose that on
our retry, we encounter a different server that no longer supports
structured replies. We would no longer be justified in sending
NBD_CMD_BLOCK_STATUS requests to the reconnected server. But I can't
find anywhere in the code base that ensures that on a reconnect, the
new server supplies at least as many extensions as the original
server, nor anywhere that we would be able to gracefully handle an
in-flight block status command that can no longer be successfully
continued because the reconnect landed on a downgraded server.
In general, you don't expect a server to downgrade its capabilities
across restarts, so assuming that a retried connection will hit a
server at least as capable as the original server is typical, even if
unsafe. But it is easy enough to use nbdkit to write a server that
purposefully downgrades its abilities after the first client
connection, for testing how qemu falls apart if it continues making
assumptions about the current server based solely on what it learned
prior to retrying from the first server.
Is this something we need to address quickly for inclusion in 6.2?
Maybe by having a retry connect fail if the new server does not have
the same capabilities as the old? Do we also need to care about a
server reporting a different size export than the old server?
Yes that's a problem. We previously noted it here
https://lists.gnu.org/archive/html/qemu-block/2021-06/msg00458.html
Honestly, I didn't start any fix for that :(.. I agree, it would be good to fix
it somehow in 6.2. I'll try to make something simple this week. Or did you
already started doing some fix?
--
Best regards,
Vladimir
[PATCH for-6.2] hw/intc/arm_gicv3_its: Revert version increments in vmstate_its
Commit 18f6290a6a ("hw/intc: GICv3 ITS initial framework")
incremented version_id and minimum_version_id fields of
VMStateDescription vmstate_its. This breaks the migration between
6.2 and 6.1 with the following message:
qemu-system-aarch64: savevm: unsupported version 1 for 'arm_gicv3_its' v0
qemu-system-aarch64: load of migration failed: Invalid argument
Revert that change.
Signed-off-by: Eric Auger
---
hw/intc/arm_gicv3_its_common.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 7d7f3882e76..90b85f1e25c 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -50,8 +50,6 @@ static int gicv3_its_post_load(void *opaque, int version_id)
static const VMStateDescription vmstate_its = {
.name = "arm_gicv3_its",
-.version_id = 1,
-.minimum_version_id = 1,
.pre_save = gicv3_its_pre_save,
.post_load = gicv3_its_post_load,
.priority = MIG_PRI_GICV3_ITS,
--
2.26.3
Re: [PATCH v1 0/9] hw/mos6522: VIA timer emulation fixes and improvements
On Sat, 20 Nov 2021 at 23:40, Finn Thain wrote: > Anyway, thanks for taking the time to write. A competent reviewer has to > do much more than that, but I'm not paying for competence so I suppose I'm > asking too much. Please dial back the aggressive tone here, Finn: this kind of thing is way out of line. We're all trying to help improve QEMU here, and sniping at Mark is not constructive. -- PMM
Re: [PATCH v4 20/32] nbd/client-connection: implement connection retry
Reviving this thread, as a good as place as any for my question:
On Thu, Jun 10, 2021 at 01:07:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add an option for a thread to retry connection until succeeds. We'll
> use nbd/client-connection both for reconnect and for initial connection
> in nbd_open(), so we need a possibility to use same NBDClientConnection
> instance to connect once in nbd_open() and then use retry semantics for
> reconnect.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> include/block/nbd.h | 2 ++
> nbd/client-connection.c | 56 +++--
> 2 files changed, 45 insertions(+), 13 deletions(-)
> NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
> bool do_negotiation,
> const char *export_name,
> @@ -154,23 +164,43 @@ static void *connect_thread_func(void *opaque)
> NBDClientConnection *conn = opaque;
> int ret;
> bool do_free;
> +uint64_t timeout = 1;
> +uint64_t max_timeout = 16;
>
> -conn->sioc = qio_channel_socket_new();
> +while (true) {
> +conn->sioc = qio_channel_socket_new();
>
> -error_free(conn->err);
> -conn->err = NULL;
> -conn->updated_info = conn->initial_info;
> +error_free(conn->err);
> +conn->err = NULL;
> +conn->updated_info = conn->initial_info;
>
> -ret = nbd_connect(conn->sioc, conn->saddr,
> - conn->do_negotiation ? >updated_info : NULL,
> - conn->tlscreds, >ioc, >err);
This says that on each retry attempt, we reset whether to ask the
server for structured replies back to our original initial_info
values.
But when dealing with NBD retries in general, I suspect we have a bug.
Consider what happens if our first connection requests structured
replies and base:allocation block status, and we are successful. But
later, the server disconnects, triggering a retry. Suppose that on
our retry, we encounter a different server that no longer supports
structured replies. We would no longer be justified in sending
NBD_CMD_BLOCK_STATUS requests to the reconnected server. But I can't
find anywhere in the code base that ensures that on a reconnect, the
new server supplies at least as many extensions as the original
server, nor anywhere that we would be able to gracefully handle an
in-flight block status command that can no longer be successfully
continued because the reconnect landed on a downgraded server.
In general, you don't expect a server to downgrade its capabilities
across restarts, so assuming that a retried connection will hit a
server at least as capable as the original server is typical, even if
unsafe. But it is easy enough to use nbdkit to write a server that
purposefully downgrades its abilities after the first client
connection, for testing how qemu falls apart if it continues making
assumptions about the current server based solely on what it learned
prior to retrying from the first server.
Is this something we need to address quickly for inclusion in 6.2?
Maybe by having a retry connect fail if the new server does not have
the same capabilities as the old? Do we also need to care about a
server reporting a different size export than the old server?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH for-6.2 0/2] linux-user: Create a common rewind_if_in_safe_syscall
Le 22/11/2021 à 16:50, Richard Henderson a écrit : On 11/22/21 4:43 PM, Laurent Vivier wrote: Hi, Le 22/11/2021 à 14:11, Richard Henderson a écrit : This is a re-packaging of two of Warner's patches that fix a build issue on aarch64 using musl: https://lore.kernel.org/qemu-devel/20211108194230.1836262-1-raj.k...@gmail.com/ r~ Warner Losh (2): linux-user: Add host_signal_set_pc to set pc in mcontext linux-user/signal.c: Create a common rewind_if_in_safe_syscall linux-user/host/aarch64/host-signal.h | 5 + linux-user/host/aarch64/hostdep.h | 20 linux-user/host/alpha/host-signal.h | 5 + linux-user/host/arm/host-signal.h | 5 + linux-user/host/arm/hostdep.h | 20 linux-user/host/i386/host-signal.h | 5 + linux-user/host/i386/hostdep.h | 20 linux-user/host/mips/host-signal.h | 5 + linux-user/host/ppc/host-signal.h | 5 + linux-user/host/ppc64/hostdep.h | 20 linux-user/host/riscv/host-signal.h | 5 + linux-user/host/riscv/hostdep.h | 20 linux-user/host/s390/host-signal.h | 5 + linux-user/host/s390x/hostdep.h | 20 linux-user/host/sparc/host-signal.h | 9 + linux-user/host/x86_64/host-signal.h | 5 + linux-user/host/x86_64/hostdep.h | 20 linux-user/safe-syscall.h | 3 +++ linux-user/signal.c | 15 --- 19 files changed, 69 insertions(+), 143 deletions(-) Richard, will you take this series via one of your branches or do you want I send a linux-user pull request for it? I have nothing pending myself, but I can send this if you'd like. I mostly wanted your ack on it. Acked-by: Laurent Vivier Thanks, Laurent
Re: [PATCH v2] linux-user: implement more loop ioctls
Le 22/11/2021 à 16:56, Andreas Schwab a écrit :
LOOP_CONFIGURE is now used by losetup, and it cannot cope with ENOSYS.
Signed-off-by: Andreas Schwab
---
v2: fix s/loop_configure/loop_config/ typo
linux-user/ioctls.h| 4
linux-user/linux_loop.h| 2 ++
linux-user/syscall_defs.h | 4
linux-user/syscall_types.h | 6 ++
4 files changed, 16 insertions(+)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 7193c3b226..f182d40190 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -637,6 +637,10 @@
IOCTL(LOOP_SET_STATUS64, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_GET_STATUS64, IOC_R, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_CHANGE_FD, 0, TYPE_INT)
+ IOCTL(LOOP_SET_CAPACITY, 0, TYPE_INT)
+ IOCTL(LOOP_SET_DIRECT_IO, 0, TYPE_INT)
+ IOCTL(LOOP_SET_BLOCK_SIZE, 0, TYPE_INT)
+ IOCTL(LOOP_CONFIGURE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_config)))
IOCTL(LOOP_CTL_ADD, 0, TYPE_INT)
IOCTL(LOOP_CTL_REMOVE, 0, TYPE_INT)
diff --git a/linux-user/linux_loop.h b/linux-user/linux_loop.h
index c69fea11e4..f80b96f1ff 100644
--- a/linux-user/linux_loop.h
+++ b/linux-user/linux_loop.h
@@ -96,6 +96,8 @@ struct loop_info64 {
#define LOOP_CHANGE_FD0x4C06
#define LOOP_SET_CAPACITY 0x4C07
#define LOOP_SET_DIRECT_IO 0x4C08
+#define LOOP_SET_BLOCK_SIZE 0x4C09
+#define LOOP_CONFIGURE 0x4C0A
/* /dev/loop-control interface */
#define LOOP_CTL_ADD0x4C80
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc..560a29afd8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1219,6 +1219,10 @@ struct target_rtc_pll_info {
#define TARGET_LOOP_SET_STATUS64 0x4C04
#define TARGET_LOOP_GET_STATUS64 0x4C05
#define TARGET_LOOP_CHANGE_FD 0x4C06
+#define TARGET_LOOP_SET_CAPACITY 0x4C07
+#define TARGET_LOOP_SET_DIRECT_IO 0x4C08
+#define TARGET_LOOP_SET_BLOCK_SIZE0x4C09
+#define TARGET_LOOP_CONFIGURE 0x4C0A
#define TARGET_LOOP_CTL_ADD 0x4C80
#define TARGET_LOOP_CTL_REMOVE0x4C81
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ba2c1518eb..c3b43f8022 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -201,6 +201,12 @@ STRUCT(loop_info64,
MK_ARRAY(TYPE_CHAR, 32), /* lo_encrypt_key */
MK_ARRAY(TYPE_ULONGLONG, 2)) /* lo_init */
+STRUCT(loop_config,
+ TYPE_INT, /* fd */
+ TYPE_INT, /* block_size */
+ MK_STRUCT(STRUCT_loop_info64), /* info */
+ MK_ARRAY(TYPE_ULONGLONG, 8)) /* __reserved */
+
/* mag tape ioctls */
STRUCT(mtop, TYPE_SHORT, TYPE_INT)
STRUCT(mtget, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG,
Reviewed-by: Laurent Vivier
[PATCH v2] linux-user: implement more loop ioctls
LOOP_CONFIGURE is now used by losetup, and it cannot cope with ENOSYS.
Signed-off-by: Andreas Schwab
---
v2: fix s/loop_configure/loop_config/ typo
linux-user/ioctls.h| 4
linux-user/linux_loop.h| 2 ++
linux-user/syscall_defs.h | 4
linux-user/syscall_types.h | 6 ++
4 files changed, 16 insertions(+)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 7193c3b226..f182d40190 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -637,6 +637,10 @@
IOCTL(LOOP_SET_STATUS64, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_GET_STATUS64, IOC_R, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_CHANGE_FD, 0, TYPE_INT)
+ IOCTL(LOOP_SET_CAPACITY, 0, TYPE_INT)
+ IOCTL(LOOP_SET_DIRECT_IO, 0, TYPE_INT)
+ IOCTL(LOOP_SET_BLOCK_SIZE, 0, TYPE_INT)
+ IOCTL(LOOP_CONFIGURE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_config)))
IOCTL(LOOP_CTL_ADD, 0, TYPE_INT)
IOCTL(LOOP_CTL_REMOVE, 0, TYPE_INT)
diff --git a/linux-user/linux_loop.h b/linux-user/linux_loop.h
index c69fea11e4..f80b96f1ff 100644
--- a/linux-user/linux_loop.h
+++ b/linux-user/linux_loop.h
@@ -96,6 +96,8 @@ struct loop_info64 {
#define LOOP_CHANGE_FD 0x4C06
#define LOOP_SET_CAPACITY 0x4C07
#define LOOP_SET_DIRECT_IO 0x4C08
+#define LOOP_SET_BLOCK_SIZE 0x4C09
+#define LOOP_CONFIGURE 0x4C0A
/* /dev/loop-control interface */
#define LOOP_CTL_ADD0x4C80
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc..560a29afd8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1219,6 +1219,10 @@ struct target_rtc_pll_info {
#define TARGET_LOOP_SET_STATUS64 0x4C04
#define TARGET_LOOP_GET_STATUS64 0x4C05
#define TARGET_LOOP_CHANGE_FD 0x4C06
+#define TARGET_LOOP_SET_CAPACITY 0x4C07
+#define TARGET_LOOP_SET_DIRECT_IO 0x4C08
+#define TARGET_LOOP_SET_BLOCK_SIZE0x4C09
+#define TARGET_LOOP_CONFIGURE 0x4C0A
#define TARGET_LOOP_CTL_ADD 0x4C80
#define TARGET_LOOP_CTL_REMOVE0x4C81
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ba2c1518eb..c3b43f8022 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -201,6 +201,12 @@ STRUCT(loop_info64,
MK_ARRAY(TYPE_CHAR, 32), /* lo_encrypt_key */
MK_ARRAY(TYPE_ULONGLONG, 2)) /* lo_init */
+STRUCT(loop_config,
+ TYPE_INT, /* fd */
+ TYPE_INT, /* block_size */
+ MK_STRUCT(STRUCT_loop_info64), /* info */
+ MK_ARRAY(TYPE_ULONGLONG, 8)) /* __reserved */
+
/* mag tape ioctls */
STRUCT(mtop, TYPE_SHORT, TYPE_INT)
STRUCT(mtget, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG,
--
2.34.0
Re: [PATCH for-6.2 0/2] linux-user: Create a common rewind_if_in_safe_syscall
On 11/22/21 4:43 PM, Laurent Vivier wrote: Hi, Le 22/11/2021 à 14:11, Richard Henderson a écrit : This is a re-packaging of two of Warner's patches that fix a build issue on aarch64 using musl: https://lore.kernel.org/qemu-devel/20211108194230.1836262-1-raj.k...@gmail.com/ r~ Warner Losh (2): linux-user: Add host_signal_set_pc to set pc in mcontext linux-user/signal.c: Create a common rewind_if_in_safe_syscall linux-user/host/aarch64/host-signal.h | 5 + linux-user/host/aarch64/hostdep.h | 20 linux-user/host/alpha/host-signal.h | 5 + linux-user/host/arm/host-signal.h | 5 + linux-user/host/arm/hostdep.h | 20 linux-user/host/i386/host-signal.h | 5 + linux-user/host/i386/hostdep.h | 20 linux-user/host/mips/host-signal.h | 5 + linux-user/host/ppc/host-signal.h | 5 + linux-user/host/ppc64/hostdep.h | 20 linux-user/host/riscv/host-signal.h | 5 + linux-user/host/riscv/hostdep.h | 20 linux-user/host/s390/host-signal.h | 5 + linux-user/host/s390x/hostdep.h | 20 linux-user/host/sparc/host-signal.h | 9 + linux-user/host/x86_64/host-signal.h | 5 + linux-user/host/x86_64/hostdep.h | 20 linux-user/safe-syscall.h | 3 +++ linux-user/signal.c | 15 --- 19 files changed, 69 insertions(+), 143 deletions(-) Richard, will you take this series via one of your branches or do you want I send a linux-user pull request for it? I have nothing pending myself, but I can send this if you'd like. I mostly wanted your ack on it. r~
Re: [PATCH for-6.2 0/2] linux-user: Create a common rewind_if_in_safe_syscall
Hi, Le 22/11/2021 à 14:11, Richard Henderson a écrit : This is a re-packaging of two of Warner's patches that fix a build issue on aarch64 using musl: https://lore.kernel.org/qemu-devel/20211108194230.1836262-1-raj.k...@gmail.com/ r~ Warner Losh (2): linux-user: Add host_signal_set_pc to set pc in mcontext linux-user/signal.c: Create a common rewind_if_in_safe_syscall linux-user/host/aarch64/host-signal.h | 5 + linux-user/host/aarch64/hostdep.h | 20 linux-user/host/alpha/host-signal.h | 5 + linux-user/host/arm/host-signal.h | 5 + linux-user/host/arm/hostdep.h | 20 linux-user/host/i386/host-signal.h| 5 + linux-user/host/i386/hostdep.h| 20 linux-user/host/mips/host-signal.h| 5 + linux-user/host/ppc/host-signal.h | 5 + linux-user/host/ppc64/hostdep.h | 20 linux-user/host/riscv/host-signal.h | 5 + linux-user/host/riscv/hostdep.h | 20 linux-user/host/s390/host-signal.h| 5 + linux-user/host/s390x/hostdep.h | 20 linux-user/host/sparc/host-signal.h | 9 + linux-user/host/x86_64/host-signal.h | 5 + linux-user/host/x86_64/hostdep.h | 20 linux-user/safe-syscall.h | 3 +++ linux-user/signal.c | 15 --- 19 files changed, 69 insertions(+), 143 deletions(-) Richard, will you take this series via one of your branches or do you want I send a linux-user pull request for it? Thanks, Laurent
Re: [PULL 0/8] Fixes 20211122 patches
On 11/22/21 1:40 PM, Gerd Hoffmann wrote: The following changes since commit c5fbdd60cf1fb52f01bdfe342b6fa65d5343e1b1: Merge tag 'qemu-sparc-20211121' of git://github.com/mcayland/qemu into staging (2021-11-21 14:12:25 +0100) are available in the Git repository at: git://git.kraxel.org/qemu tags/fixes-20211122-pull-request for you to fetch changes up to b9e5628ca5d42994cc6f82752d9bf0bc98f51f64: microvm: check g_file_set_contents() return value (2021-11-22 11:14:28 +0100) fixes for 6.2: microvm, ui, modules. Alexander Orzechowski (2): ui: fix incorrect scaling on highdpi with gtk/opengl ui: fix incorrect pointer position on highdpi with gtk Dongwon Kim (1): ui/gtk: graphic_hw_gl_flushed after closing dmabuf->fence_fd Gerd Hoffmann (2): microvm: add missing g_free() call microvm: check g_file_set_contents() return value Laurent Vivier (1): migration: fix dump-vmstate with modules Philippe Mathieu-Daudé (1): hw/i386/microvm: Reduce annoying debug message in dt_setup_microvm() Vladimir Sementsov-Ogievskiy (1): ui/vnc-clipboard: fix adding notifier twice hw/i386/microvm-dt.c | 11 +-- softmmu/vl.c | 1 + ui/gtk-gl-area.c | 7 --- ui/gtk.c | 17 ++--- ui/vnc-clipboard.c | 10 ++ 5 files changed, 30 insertions(+), 16 deletions(-) Applied, thanks. r~
Re: [PATCH] linux-user: implement more loop ioctls
Le 22/11/2021 à 15:18, Andreas Schwab a écrit :
LOOP_CONFIGURE is now used by losetup, and it cannot cope with ENOSYS.
Signed-off-by: Andreas Schwab
---
linux-user/ioctls.h| 4
linux-user/linux_loop.h| 2 ++
linux-user/syscall_defs.h | 4
linux-user/syscall_types.h | 6 ++
4 files changed, 16 insertions(+)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 7193c3b226..5ac5efc8aa 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -637,6 +637,10 @@
IOCTL(LOOP_SET_STATUS64, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_GET_STATUS64, IOC_R, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_CHANGE_FD, 0, TYPE_INT)
+ IOCTL(LOOP_SET_CAPACITY, 0, TYPE_INT)
+ IOCTL(LOOP_SET_DIRECT_IO, 0, TYPE_INT)
+ IOCTL(LOOP_SET_BLOCK_SIZE, 0, TYPE_INT)
+ IOCTL(LOOP_CONFIGURE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_configure)))
IOCTL(LOOP_CTL_ADD, 0, TYPE_INT)
IOCTL(LOOP_CTL_REMOVE, 0, TYPE_INT)
diff --git a/linux-user/linux_loop.h b/linux-user/linux_loop.h
index c69fea11e4..f80b96f1ff 100644
--- a/linux-user/linux_loop.h
+++ b/linux-user/linux_loop.h
@@ -96,6 +96,8 @@ struct loop_info64 {
#define LOOP_CHANGE_FD0x4C06
#define LOOP_SET_CAPACITY 0x4C07
#define LOOP_SET_DIRECT_IO 0x4C08
+#define LOOP_SET_BLOCK_SIZE 0x4C09
+#define LOOP_CONFIGURE 0x4C0A
/* /dev/loop-control interface */
#define LOOP_CTL_ADD0x4C80
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc..560a29afd8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1219,6 +1219,10 @@ struct target_rtc_pll_info {
#define TARGET_LOOP_SET_STATUS64 0x4C04
#define TARGET_LOOP_GET_STATUS64 0x4C05
#define TARGET_LOOP_CHANGE_FD 0x4C06
+#define TARGET_LOOP_SET_CAPACITY 0x4C07
+#define TARGET_LOOP_SET_DIRECT_IO 0x4C08
+#define TARGET_LOOP_SET_BLOCK_SIZE0x4C09
+#define TARGET_LOOP_CONFIGURE 0x4C0A
#define TARGET_LOOP_CTL_ADD 0x4C80
#define TARGET_LOOP_CTL_REMOVE0x4C81
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ba2c1518eb..7c46e4fb25 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -201,6 +201,12 @@ STRUCT(loop_info64,
MK_ARRAY(TYPE_CHAR, 32), /* lo_encrypt_key */
MK_ARRAY(TYPE_ULONGLONG, 2)) /* lo_init */
+STRUCT(loop_configure,
It should be named "loop_config", like int he kernel.
Except that:
Reviewed-by: Laurent Vivier
+ TYPE_INT, /* fd */
+ TYPE_INT, /* block_size */
+ MK_STRUCT(STRUCT_loop_info64), /* info */
+ MK_ARRAY(TYPE_ULONGLONG, 8)) /* __reserved */
+
/* mag tape ioctls */
STRUCT(mtop, TYPE_SHORT, TYPE_INT)
STRUCT(mtget, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG,
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On 22.11.21 16:09, Jason Gunthorpe wrote: > On Mon, Nov 22, 2021 at 03:57:17PM +0100, David Hildenbrand wrote: >> On 22.11.21 15:01, Jason Gunthorpe wrote: >>> On Mon, Nov 22, 2021 at 02:35:49PM +0100, David Hildenbrand wrote: On 22.11.21 14:31, Jason Gunthorpe wrote: > On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: > >> I do wonder if we want to support sharing such memfds between processes >> in all cases ... we most certainly don't want to be able to share >> encrypted memory between VMs (I heard that the kernel has to forbid >> that). It would make sense in the use case you describe, though. > > If there is a F_SEAL_XX that blocks every kind of new access, who > cares if userspace passes the FD around or not? I was imagining that you actually would want to do some kind of "change ownership". But yeah, the intended semantics and all use cases we have in mind are not fully clear to me yet. If it's really "no new access" (side note: is "access" the right word?) then sure, we can pass the fd around. >>> >>> What is "ownership" in a world with kvm and iommu are reading pages >>> out of the same fd? >> >> In the world of encrypted memory / TDX, KVM somewhat "owns" that memory >> IMHO (for example, only it can migrate or swap out these pages; it's >> might be debatable if the TDX module or KVM actually "own" these pages ). > > Sounds like it is a swap provider more than an owner? Yes, I think we can phrase it that way, + "migrate provider" -- Thanks, David / dhildenb
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On Mon, Nov 22, 2021 at 03:57:17PM +0100, David Hildenbrand wrote: > On 22.11.21 15:01, Jason Gunthorpe wrote: > > On Mon, Nov 22, 2021 at 02:35:49PM +0100, David Hildenbrand wrote: > >> On 22.11.21 14:31, Jason Gunthorpe wrote: > >>> On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: > >>> > I do wonder if we want to support sharing such memfds between processes > in all cases ... we most certainly don't want to be able to share > encrypted memory between VMs (I heard that the kernel has to forbid > that). It would make sense in the use case you describe, though. > >>> > >>> If there is a F_SEAL_XX that blocks every kind of new access, who > >>> cares if userspace passes the FD around or not? > >> I was imagining that you actually would want to do some kind of "change > >> ownership". But yeah, the intended semantics and all use cases we have > >> in mind are not fully clear to me yet. If it's really "no new access" > >> (side note: is "access" the right word?) then sure, we can pass the fd > >> around. > > > > What is "ownership" in a world with kvm and iommu are reading pages > > out of the same fd? > > In the world of encrypted memory / TDX, KVM somewhat "owns" that memory > IMHO (for example, only it can migrate or swap out these pages; it's > might be debatable if the TDX module or KVM actually "own" these pages ). Sounds like it is a swap provider more than an owner? Jason
Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
On 19/11/2021 18.11, Philippe Mathieu-Daudé wrote:
Inline vga_mm_init() in vga_mmio_init() to simplify the
next patch review. Kind of.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/display/vga-mmio.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
index 8aaf44e7b1d..0aefbcf53a0 100644
--- a/hw/display/vga-mmio.c
+++ b/hw/display/vga-mmio.c
@@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
-hwaddr ctrl_base, int it_shift,
-MemoryRegion *address_space)
+int vga_mmio_init(hwaddr vram_base,
+hwaddr ctrl_base, int it_shift,
+MemoryRegion *address_space)
{
+VGAMmioState *s;
MemoryRegion *s_ioport_ctrl, *vga_io_memory;
+s = g_malloc0(sizeof(*s));
+
+s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
+s->vga.global_vmstate = true;
+vga_common_init(>vga, NULL);
+
s->it_shift = it_shift;
s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
memory_region_init_io(s_ioport_ctrl, NULL, _mm_ctrl_ops, s,
@@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
memory_region_add_subregion(address_space,
vram_base + 0x000a, vga_io_memory);
memory_region_set_coalescing(vga_io_memory);
-}
-
-int vga_mmio_init(hwaddr vram_base,
-hwaddr ctrl_base, int it_shift,
-MemoryRegion *address_space)
Preferably with the indentation fixed (already in the first patch):
Reviewed-by: Thomas Huth
Re: [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO
On 19/11/2021 18.11, Philippe Mathieu-Daudé wrote:
There is no ISA bus part in the MMIO VGA device, so rename:
* hw/display/vga-isa-mm.c -> hw/display/vga-mmio.c
* CONFIG_VGA_ISA_MM -> CONFIG_VGA_MMIO
* ISAVGAMMState -> VGAMmioState
* isa_vga_mm_init() -> vga_mmio_init()
Signed-off-by: Philippe Mathieu-Daudé
---
configs/devices/mips-softmmu/common.mak | 2 +-
include/hw/display/vga.h| 2 +-
hw/display/{vga-isa-mm.c => vga-mmio.c} | 16
hw/mips/jazz.c | 2 +-
hw/display/Kconfig | 2 +-
hw/display/meson.build | 2 +-
hw/mips/Kconfig | 2 +-
7 files changed, 14 insertions(+), 14 deletions(-)
rename hw/display/{vga-isa-mm.c => vga-mmio.c} (93%)
Reviewed-by: Thomas Huth
Re: [PATCH] s390x/ipl: support extended kernel command line size
David Hildenbrand writes:
> On 22.11.21 12:29, Marc Hartmayer wrote:
>> In the past s390 used a fixed command line length of 896 bytes. This has
>> changed
>> with the Linux commit 5ecb2da660ab ("s390: support command lines longer than
>> 896
>> bytes"). There is now a parm area indicating the maximum command line size.
>> This
>> parm area has always been initialized to zero, so with older kernels this
>> field
>> would read zero and we must then assume that only 896 bytes are available.
>>
>> Acked-by: Viktor Mihajlovski
>> Signed-off-by: Marc Hartmayer
>> ---
>> hw/s390x/ipl.c | 23 ---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 7ddca0127fc2..092c66b3f9f1 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -37,8 +37,9 @@
>>
>> #define KERN_IMAGE_START0x01UL
>> #define LINUX_MAGIC_ADDR0x010008UL
>> +#define KERN_PARM_AREA_SIZE_ADDR0x010430UL
>> #define KERN_PARM_AREA 0x010480UL
>> -#define KERN_PARM_AREA_SIZE 0x000380UL
>> +#define LEGACY_KERN_PARM_AREA_SIZE 0x000380UL
>> #define INITRD_START0x80UL
>> #define INITRD_PARM_START 0x010408UL
>> #define PARMFILE_START 0x001000UL
>> @@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque,
>> uint64_t srcaddr)
>> return srcaddr + dstaddr;
>> }
>>
>> +static uint64_t get_max_kernel_cmdline_size(void)
>> +{
>> +uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR,
>> sizeof(*size_ptr));
>> +
>> +if (size_ptr) {
>> +uint64_t size;
>> +
>> +size = be64_to_cpu(*size_ptr);
>> +if (size != 0) {
>
> Could do "if (size) {"
Ok.
>
>> +return size;
>> +}
>> +}
>> +return LEGACY_KERN_PARM_AREA_SIZE;
>> +}
>> +
>> static void s390_ipl_realize(DeviceState *dev, Error **errp)
>> {
>> MachineState *ms = MACHINE(qdev_get_machine());
>> @@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error
>> **errp)
>> ipl->start_addr = KERN_IMAGE_START;
>> /* Overwrite parameters in the kernel image, which are "rom" */
>> if (parm_area) {
>> -if (cmdline_size > KERN_PARM_AREA_SIZE) {
>> +uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
>
> We might want an empty line here.
Yep.
>
>> +if (cmdline_size > max_cmdline_size) {
>> error_setg(errp,
>> "kernel command line exceeds maximum size:
>> %zu > %lu",
>> - cmdline_size, KERN_PARM_AREA_SIZE);
>> + cmdline_size, max_cmdline_size);
>> return;
>> }
>>
>>
>
> Reviewed-by: David Hildenbrand
Thanks for the review!
>
> --
> Thanks,
>
> David / dhildenb
>
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH v5 04/17] common-user: Move syscall error detection into safe_syscall_base
On Mon, 22 Nov 2021 at 12:21, Richard Henderson wrote: > > On 11/22/21 12:55 PM, Peter Maydell wrote: > >> + /* > >> + * The syscall calling convention isn't the same as the C one: > > > > Looks like the indent here is wrong ? > > Irritatingly, these files are a mix of tabs/spaces. Hmm, so they are; I wonder how we let that slip in. Maybe do a set of preparatory patches doing a mechanical tab-to-space conversion? -- PMM
Re: [PATCH for-6.2 1/2] linux-user: Add host_signal_set_pc to set pc in mcontext
On Mon, Nov 22, 2021 at 6:12 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> From: Warner Losh
>
> Add a new function host_signal_set_pc to set the next pc in an
> mcontext. The caller should ensure this is a valid PC for execution.
>
> Signed-off-by: Warner Losh
> Reviewed-by: Philippe Mathieu-Daudé
> Reviewed-by: Richard Henderson
> Message-Id: <2023045603.60391-2-...@bsdimp.com>
> Signed-off-by: Richard Henderson
> ---
> linux-user/host/aarch64/host-signal.h | 5 +
> linux-user/host/alpha/host-signal.h | 5 +
> linux-user/host/arm/host-signal.h | 5 +
> linux-user/host/i386/host-signal.h| 5 +
> linux-user/host/mips/host-signal.h| 5 +
> linux-user/host/ppc/host-signal.h | 5 +
> linux-user/host/riscv/host-signal.h | 5 +
> linux-user/host/s390/host-signal.h| 5 +
> linux-user/host/sparc/host-signal.h | 9 +
> linux-user/host/x86_64/host-signal.h | 5 +
> 10 files changed, 54 insertions(+)
>
Reviewed-by: Warner Losh
This looks like what I submitted, and will work with the cleaned up other
half.
Warner
> diff --git a/linux-user/host/aarch64/host-signal.h
> b/linux-user/host/aarch64/host-signal.h
> index 0c0b08383a..9770b36dc1 100644
> --- a/linux-user/host/aarch64/host-signal.h
> +++ b/linux-user/host/aarch64/host-signal.h
> @@ -35,6 +35,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.pc;
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.pc = pc;
> +}
> +
> static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
> {
> struct _aarch64_ctx *hdr;
> diff --git a/linux-user/host/alpha/host-signal.h
> b/linux-user/host/alpha/host-signal.h
> index e080be412f..f4c942948a 100644
> --- a/linux-user/host/alpha/host-signal.h
> +++ b/linux-user/host/alpha/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.sc_pc;
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.sc_pc = pc;
> +}
> +
> static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
> {
> uint32_t *pc = (uint32_t *)host_signal_pc(uc);
> diff --git a/linux-user/host/arm/host-signal.h
> b/linux-user/host/arm/host-signal.h
> index efb165c0c5..6c095773c0 100644
> --- a/linux-user/host/arm/host-signal.h
> +++ b/linux-user/host/arm/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.arm_pc;
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.arm_pc = pc;
> +}
> +
> static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
> {
> /*
> diff --git a/linux-user/host/i386/host-signal.h
> b/linux-user/host/i386/host-signal.h
> index 4c8eef99ce..abe1ece5c9 100644
> --- a/linux-user/host/i386/host-signal.h
> +++ b/linux-user/host/i386/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.gregs[REG_EIP];
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.gregs[REG_EIP] = pc;
> +}
> +
> static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
> {
> return uc->uc_mcontext.gregs[REG_TRAPNO] == 0xe
> diff --git a/linux-user/host/mips/host-signal.h
> b/linux-user/host/mips/host-signal.h
> index ef341f7c20..c666ed8c3f 100644
> --- a/linux-user/host/mips/host-signal.h
> +++ b/linux-user/host/mips/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.pc;
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.pc = pc;
> +}
> +
> #if defined(__misp16) || defined(__mips_micromips)
> #error "Unsupported encoding"
> #endif
> diff --git a/linux-user/host/ppc/host-signal.h
> b/linux-user/host/ppc/host-signal.h
> index a491c413dc..1d8e658ff7 100644
> --- a/linux-user/host/ppc/host-signal.h
> +++ b/linux-user/host/ppc/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.regs->nip;
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +uc->uc_mcontext.regs->nip = pc;
> +}
> +
> static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
> {
> return uc->uc_mcontext.regs->trap != 0x400
> diff --git a/linux-user/host/riscv/host-signal.h
> b/linux-user/host/riscv/host-signal.h
> index 3b168cb58b..a4f170efb0 100644
> --- a/linux-user/host/riscv/host-signal.h
> +++ b/linux-user/host/riscv/host-signal.h
> @@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
> return uc->uc_mcontext.__gregs[REG_PC];
> }
>
> +static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
> +{
> +
Re: [PATCH for-6.2 2/2] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
On Mon, Nov 22, 2021 at 6:12 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> From: Warner Losh
>
> All instances of rewind_if_in_safe_syscall are the same, differing only
> in how the instruction point is fetched from the ucontext and the size
> of the registers. Use host_signal_pc and new host_signal_set_pc
> interfaces to fetch the pointer to the PC and adjust if needed. Delete
> all the old copies of rewind_if_in_safe_syscall.
>
> Signed-off-by: Warner Losh
> Reviewed-by: Richard Henderson
> Reviewed-by: Philippe Mathieu-Daudé
> Message-Id: <2023045603.60391-3-...@bsdimp.com>
> [rth: include safe-syscall.h, simplify ifdefs]
> Signed-off-by: Richard Henderson
> ---
> linux-user/host/aarch64/hostdep.h | 20
> linux-user/host/arm/hostdep.h | 20
> linux-user/host/i386/hostdep.h| 20
> linux-user/host/ppc64/hostdep.h | 20
> linux-user/host/riscv/hostdep.h | 20
> linux-user/host/s390x/hostdep.h | 20
> linux-user/host/x86_64/hostdep.h | 20
> linux-user/safe-syscall.h | 3 +++
> linux-user/signal.c | 15 ---
> 9 files changed, 15 insertions(+), 143 deletions(-)
>
Reviewed-by: Warner Losh
The changes to what I wrote look good.
Warner
> diff --git a/linux-user/host/aarch64/hostdep.h
> b/linux-user/host/aarch64/hostdep.h
> index a8d41a21ad..39299d798a 100644
> --- a/linux-user/host/aarch64/hostdep.h
> +++ b/linux-user/host/aarch64/hostdep.h
> @@ -15,24 +15,4 @@
> /* We have a safe-syscall.inc.S */
> #define HAVE_SAFE_SYSCALL
>
> -#ifndef __ASSEMBLER__
> -
> -/* These are defined by the safe-syscall.inc.S file */
> -extern char safe_syscall_start[];
> -extern char safe_syscall_end[];
> -
> -/* Adjust the signal context to rewind out of safe-syscall if we're in it
> */
> -static inline void rewind_if_in_safe_syscall(void *puc)
> -{
> -ucontext_t *uc = puc;
> -__u64 *pcreg = >uc_mcontext.pc;
> -
> -if (*pcreg > (uintptr_t)safe_syscall_start
> -&& *pcreg < (uintptr_t)safe_syscall_end) {
> -*pcreg = (uintptr_t)safe_syscall_start;
> -}
> -}
> -
> -#endif /* __ASSEMBLER__ */
> -
> #endif
> diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
> index 9276fe6ceb..86b137875a 100644
> --- a/linux-user/host/arm/hostdep.h
> +++ b/linux-user/host/arm/hostdep.h
> @@ -15,24 +15,4 @@
> /* We have a safe-syscall.inc.S */
> #define HAVE_SAFE_SYSCALL
>
> -#ifndef __ASSEMBLER__
> -
> -/* These are defined by the safe-syscall.inc.S file */
> -extern char safe_syscall_start[];
> -extern char safe_syscall_end[];
> -
> -/* Adjust the signal context to rewind out of safe-syscall if we're in it
> */
> -static inline void rewind_if_in_safe_syscall(void *puc)
> -{
> -ucontext_t *uc = puc;
> -unsigned long *pcreg = >uc_mcontext.arm_pc;
> -
> -if (*pcreg > (uintptr_t)safe_syscall_start
> -&& *pcreg < (uintptr_t)safe_syscall_end) {
> -*pcreg = (uintptr_t)safe_syscall_start;
> -}
> -}
> -
> -#endif /* __ASSEMBLER__ */
> -
> #endif
> diff --git a/linux-user/host/i386/hostdep.h
> b/linux-user/host/i386/hostdep.h
> index 073be74d87..ce7136501f 100644
> --- a/linux-user/host/i386/hostdep.h
> +++ b/linux-user/host/i386/hostdep.h
> @@ -15,24 +15,4 @@
> /* We have a safe-syscall.inc.S */
> #define HAVE_SAFE_SYSCALL
>
> -#ifndef __ASSEMBLER__
> -
> -/* These are defined by the safe-syscall.inc.S file */
> -extern char safe_syscall_start[];
> -extern char safe_syscall_end[];
> -
> -/* Adjust the signal context to rewind out of safe-syscall if we're in it
> */
> -static inline void rewind_if_in_safe_syscall(void *puc)
> -{
> -ucontext_t *uc = puc;
> -greg_t *pcreg = >uc_mcontext.gregs[REG_EIP];
> -
> -if (*pcreg > (uintptr_t)safe_syscall_start
> -&& *pcreg < (uintptr_t)safe_syscall_end) {
> -*pcreg = (uintptr_t)safe_syscall_start;
> -}
> -}
> -
> -#endif /* __ASSEMBLER__ */
> -
> #endif
> diff --git a/linux-user/host/ppc64/hostdep.h
> b/linux-user/host/ppc64/hostdep.h
> index 98979ad917..0c290dd904 100644
> --- a/linux-user/host/ppc64/hostdep.h
> +++ b/linux-user/host/ppc64/hostdep.h
> @@ -15,24 +15,4 @@
> /* We have a safe-syscall.inc.S */
> #define HAVE_SAFE_SYSCALL
>
> -#ifndef __ASSEMBLER__
> -
> -/* These are defined by the safe-syscall.inc.S file */
> -extern char safe_syscall_start[];
> -extern char safe_syscall_end[];
> -
> -/* Adjust the signal context to rewind out of safe-syscall if we're in it
> */
> -static inline void rewind_if_in_safe_syscall(void *puc)
> -{
> -ucontext_t *uc = puc;
> -unsigned long *pcreg = >uc_mcontext.gp_regs[PT_NIP];
> -
> -if (*pcreg > (uintptr_t)safe_syscall_start
> -&& *pcreg < (uintptr_t)safe_syscall_end) {
> -*pcreg = (uintptr_t)safe_syscall_start;
> -}
> -}
> -
> -#endif /* __ASSEMBLER__ */
> -
> #endif
> diff --git
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On 22.11.21 15:01, Jason Gunthorpe wrote: > On Mon, Nov 22, 2021 at 02:35:49PM +0100, David Hildenbrand wrote: >> On 22.11.21 14:31, Jason Gunthorpe wrote: >>> On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: >>> I do wonder if we want to support sharing such memfds between processes in all cases ... we most certainly don't want to be able to share encrypted memory between VMs (I heard that the kernel has to forbid that). It would make sense in the use case you describe, though. >>> >>> If there is a F_SEAL_XX that blocks every kind of new access, who >>> cares if userspace passes the FD around or not? >> I was imagining that you actually would want to do some kind of "change >> ownership". But yeah, the intended semantics and all use cases we have >> in mind are not fully clear to me yet. If it's really "no new access" >> (side note: is "access" the right word?) then sure, we can pass the fd >> around. > > What is "ownership" in a world with kvm and iommu are reading pages > out of the same fd? In the world of encrypted memory / TDX, KVM somewhat "owns" that memory IMHO (for example, only it can migrate or swap out these pages; it's might be debatable if the TDX module or KVM actually "own" these pages ). -- Thanks, David / dhildenb
Re: [PATCH-for-6.2 0/2] hw/block/fdc: Fix CVE-2021-3507
ping for 6.2? On 11/18/21 12:57, Philippe Mathieu-Daudé wrote: > Trivial fix for CVE-2021-3507. > > Philippe Mathieu-Daudé (2): > hw/block/fdc: Prevent end-of-track overrun (CVE-2021-3507) > tests/qtest/fdc-test: Add a regression test for CVE-2021-3507 > > hw/block/fdc.c | 8 > tests/qtest/fdc-test.c | 20 > 2 files changed, 28 insertions(+) >
Re: [PATCH-for-6.2 v3 0/2] hw/block/fdc: Fix CVE-2021-20196
ping for 6.2? > Alexander Bulekov (1): > tests/qtest/fdc-test: Add a regression test for CVE-2021-20196 > > Philippe Mathieu-Daudé (1): > hw/block/fdc: Kludge missing floppy drive to fix CVE-2021-20196 > > hw/block/fdc.c | 14 +- > tests/qtest/fdc-test.c | 21 + > 2 files changed, 34 insertions(+), 1 deletion(-) >
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On Mon, Nov 22, 2021 at 03:01:58PM +0100, Thomas Huth wrote: > On 22/11/2021 14.53, Peter Maydell wrote: [...] > > I don't think we should be recommending to new contributors that > > they do things that established contributors generally do not do. > > The document has enough "things you should do or think about" already. > > My preference would be for simply not mentioning spelling-checking. Fair points; and yes, the doc can be intimidating as is. > > (If we do want to come up with some process for dealing with > > spelling issues in the codebase, then we either need to put it > > into CI so it's run automatically, or we need to have something > > that works on the individual patch level.) For individual patches, some projects use commit hooks for `aspell` / `codespell`. The contributor still needs to wade through false positives, though. For Sphinx-based documentation, there's "sphinxcontrib-spelling"[1]. I don't know how effective it is, but it lets one configure project-specific private dictionaries[2], which could eliminate many false positives. > Ok ... In any case - seems like this needs more discussion, so I'll > drop it from the patch for now. We can still add some wording or CI > magic later, but that's certainly something that we rather want to do > after version 6.2 has been released... Yeah, dropping it sounds fine. [1] https://github.com/sphinx-contrib/spelling [2] https://sphinxcontrib-spelling.readthedocs.io/en/latest/customize.html#private-dictionaries -- /kashyap
[PULL 3/6] Fix some typos in documentation (found by codespell)
From: Stefan Weil Signed-off-by: Stefan Weil Message-Id: <2027210702.1393570-1...@weilnetz.de> Reviewed-by: Philippe Mathieu-Daudé [thuth: "what's" --> "what is" as suggested by philmd] Signed-off-by: Thomas Huth --- docs/devel/multi-process.rst| 2 +- docs/devel/qgraph.rst | 2 +- docs/devel/writing-monitor-commands.rst | 2 +- docs/hyperv.txt | 2 +- docs/system/cpu-models-x86.rst.inc | 2 +- docs/system/devices/nvme.rst| 2 +- docs/system/gdb.rst | 2 +- docs/system/ppc/ppce500.rst | 2 +- docs/system/riscv/shakti-c.rst | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/devel/multi-process.rst b/docs/devel/multi-process.rst index e5758a79ab..5c857ff3b9 100644 --- a/docs/devel/multi-process.rst +++ b/docs/devel/multi-process.rst @@ -641,7 +641,7 @@ the CPU that issued the MMIO. +==++ | rid | range MMIO is within | +--++ -| offset | offset withing *rid* | +| offset | offset within *rid*| +--++ | type | e.g., load or store| +--++ diff --git a/docs/devel/qgraph.rst b/docs/devel/qgraph.rst index db44d71002..43342d9d65 100644 --- a/docs/devel/qgraph.rst +++ b/docs/devel/qgraph.rst @@ -14,7 +14,7 @@ support that device. Using only libqos APIs, the test has to manually take care of covering all the setups, and build the correct command line. -This also introduces backward compability issues: if a device/driver command +This also introduces backward compatibility issues: if a device/driver command line name is changed, all tests that use that will not work properly anymore and need to be adjusted. diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst index b3e2c8481d..1693822f8f 100644 --- a/docs/devel/writing-monitor-commands.rst +++ b/docs/devel/writing-monitor-commands.rst @@ -677,7 +677,7 @@ return a single text string:: The ``HumanReadableText`` struct is intended to be used for all commands, under the ``x-`` name prefix that are returning unstructured -text targetted at humans. It should never be used for commands outside +text targeted at humans. It should never be used for commands outside the ``x-`` name prefix, as those should be using structured QAPI types. Implementing the QMP command diff --git a/docs/hyperv.txt b/docs/hyperv.txt index 5d99fd9a72..0417c183a3 100644 --- a/docs/hyperv.txt +++ b/docs/hyperv.txt @@ -195,7 +195,7 @@ The enlightenment allows to use Hyper-V SynIC with hardware APICv/AVIC enabled. Normally, Hyper-V SynIC disables these hardware feature and suggests the guest to use paravirtualized AutoEOI feature. Note: enabling this feature on old hardware (without APICv/AVIC support) may -have negative effect on guest's performace. +have negative effect on guest's performance. 3.19. hv-no-nonarch-coresharing=on/off/auto === diff --git a/docs/system/cpu-models-x86.rst.inc b/docs/system/cpu-models-x86.rst.inc index 6e8be7d79b..7f6368f999 100644 --- a/docs/system/cpu-models-x86.rst.inc +++ b/docs/system/cpu-models-x86.rst.inc @@ -49,7 +49,7 @@ future OS and toolchains are likely to target newer ABIs. The table that follows illustrates which ABI compatibility levels can be satisfied by the QEMU CPU models. Note that the table only lists the long term stable CPU model versions (eg Haswell-v4). -In addition to whats listed, there are also many CPU model +In addition to what is listed, there are also many CPU model aliases which resolve to a different CPU model version, depending on the machine type is in use. diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst index a1c0db01f6..b5acb2a9c1 100644 --- a/docs/system/devices/nvme.rst +++ b/docs/system/devices/nvme.rst @@ -70,7 +70,7 @@ namespaces and additional features, the ``nvme-ns`` device must be used. The namespaces defined by the ``nvme-ns`` device will attach to the most recently defined ``nvme-bus`` that is created by the ``nvme`` device. Namespace -identifers are allocated automatically, starting from ``1``. +identifiers are allocated automatically, starting from ``1``. There are a number of parameters available: diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst index bdb42dae2f..453eb73f6c 100644 --- a/docs/system/gdb.rst +++ b/docs/system/gdb.rst @@ -56,7 +56,7 @@ machine has more than one CPU, QEMU exposes each CPU cluster as a separate "inferior", where each CPU within the cluster is a separate "thread". Most QEMU machine types have identical CPUs, so there is a single cluster which has all the CPUs in it. A few machine types are -heterogenous and have multiple clusters: for example the ``sifive_u`` +heterogeneous and have multiple clusters: for example the ``sifive_u`` machine
[PULL 6/6] docs: Render binary names as monospaced text
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Message-Id: <2028192744.64325-1-phi...@redhat.com> Reviewed-by: Darren Kenny Signed-off-by: Thomas Huth --- docs/about/removed-features.rst| 8 docs/devel/build-system.rst| 6 +++--- docs/devel/multi-process.rst | 6 +++--- docs/devel/testing.rst | 8 docs/image-fuzzer.txt | 6 +++--- docs/system/arm/orangepi.rst | 2 +- docs/system/images.rst | 2 +- docs/system/qemu-block-drivers.rst.inc | 6 +++--- docs/system/tls.rst| 2 +- docs/tools/qemu-img.rst| 18 +- docs/tools/qemu-nbd.rst| 4 ++-- docs/tools/qemu-storage-daemon.rst | 7 --- docs/tools/virtiofsd.rst | 4 ++-- 13 files changed, 40 insertions(+), 39 deletions(-) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 9d0d90c90d..d42c3341de 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -658,8 +658,8 @@ enforce that any failure to open the backing image (including if the backing file is missing or an incorrect format was specified) is an error when ``-u`` is not used. -qemu-img amend to adjust backing file (removed in 6.1) -'' +``qemu-img amend`` to adjust backing file (removed in 6.1) +'' The use of ``qemu-img amend`` to modify the name or format of a qcow2 backing image was never fully documented or tested, and interferes @@ -670,8 +670,8 @@ backing chain should be performed with ``qemu-img rebase -u`` either before or after the remaining changes being performed by amend, as appropriate. -qemu-img backing file without format (removed in 6.1) -' +``qemu-img`` backing file without format (removed in 6.1) +' The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img convert`` to create or modify an image that depends on a backing file diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst index 7a83f5fc0d..431caba7aa 100644 --- a/docs/devel/build-system.rst +++ b/docs/devel/build-system.rst @@ -121,11 +121,11 @@ process for: 1) executables, which include: - - Tools - qemu-img, qemu-nbd, qga (guest agent), etc + - Tools - ``qemu-img``, ``qemu-nbd``, ``qga`` (guest agent), etc - - System emulators - qemu-system-$ARCH + - System emulators - ``qemu-system-$ARCH`` - - Userspace emulators - qemu-$ARCH + - Userspace emulators - ``qemu-$ARCH`` - Unit tests diff --git a/docs/devel/multi-process.rst b/docs/devel/multi-process.rst index 5c857ff3b9..e4801751f2 100644 --- a/docs/devel/multi-process.rst +++ b/docs/devel/multi-process.rst @@ -187,9 +187,9 @@ desired, in which the emulation application should only be allowed to access the files or devices the VM it's running on behalf of can access. qemu-io model -Qemu-io is a test harness used to test changes to the QEMU block backend -object code. (e.g., the code that implements disk images for disk driver -emulation) Qemu-io is not a device emulation application per se, but it +``qemu-io`` is a test harness used to test changes to the QEMU block backend +object code (e.g., the code that implements disk images for disk driver +emulation). ``qemu-io`` is not a device emulation application per se, but it does compile the QEMU block objects into a separate binary from the main QEMU one. This could be useful for disk device emulation, since its emulation applications will need to include the QEMU block objects. diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 60c59023e5..755343c7dd 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -564,11 +564,11 @@ exploiting a QEMU security bug to compromise the host. QEMU binaries ~ -By default, qemu-system-x86_64 is searched in $PATH to run the guest. If there -isn't one, or if it is older than 2.10, the test won't work. In this case, +By default, ``qemu-system-x86_64`` is searched in $PATH to run the guest. If +there isn't one, or if it is older than 2.10, the test won't work. In this case, provide the QEMU binary in env var: ``QEMU=/path/to/qemu-2.10+``. -Likewise the path to qemu-img can be set in QEMU_IMG environment variable. +Likewise the path to ``qemu-img`` can be set in QEMU_IMG environment variable. Make jobs ~ @@ -650,7 +650,7 @@ supported. To start the fuzzer, run tests/image-fuzzer/runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test qcow2 -Alternatively, some command different from "qemu-img info" can be tested, by +Alternatively, some command different from ``qemu-img info`` can be tested, by changing the ``-c`` option. Integration
[PULL 2/6] docs: List more commit-message tags in "submitting-a-patch"
From: Kashyap Chamarthy Add some more examples of commonly used commit-message tags. (Thanks: Alex Bennée) Signed-off-by: Kashyap Chamarthy Message-Id: <2029193118.949698-3-kcham...@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Thomas Huth --- docs/devel/submitting-a-patch.rst | 4 1 file changed, 4 insertions(+) diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index 6acded5c93..e51259eb9c 100644 --- a/docs/devel/submitting-a-patch.rst +++ b/docs/devel/submitting-a-patch.rst @@ -195,6 +195,10 @@ Example:: Resolves: https://gitlab.com/qemu-project/qemu/-/issues/42 Buglink: https://bugs.launchpad.net/qemu/+bug/1804323`` +Some other tags that are used in commit messages include "Message-Id:" +"Tested-by:", "Acked-by:", "Reported-by:", "Suggested-by:". See ``git +log`` for these keywords for example usage. + .. _test_your_patches: Test your patches -- 2.27.0
[PULL 5/6] docs: Use double quotes instead of single quotes for COLO
From: "Rao, Lei"
Signed-off-by: Lei Rao
Message-Id: <1637567387-28250-2-git-send-email-lei@intel.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Thomas Huth
---
docs/COLO-FT.txt | 106 ++---
docs/block-replication.txt | 52 +-
2 files changed, 79 insertions(+), 79 deletions(-)
diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index fd5ffcc6e5..8ec653f81c 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -209,9 +209,9 @@ children.0=childs0 \
3. On Secondary VM's QEMU monitor, issue command
-{'execute':'qmp_capabilities'}
-{'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet', 'data':
{'host': '0.0.0.0', 'port': ''} } } }
-{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0', 'writable':
true } }
+{"execute":"qmp_capabilities"}
+{"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet", "data":
{"host": "0.0.0.0", "port": ""} } } }
+{"execute": "nbd-server-add", "arguments": {"device": "parent0", "writable":
true } }
Note:
a. The qmp command nbd-server-start and nbd-server-add must be run
@@ -222,11 +222,11 @@ Note:
will be merged into the parent disk on failover.
4. On Primary VM's QEMU monitor, issue command:
-{'execute':'qmp_capabilities'}
-{'execute': 'human-monitor-command', 'arguments': {'command-line': 'drive_add
-n buddy
driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,file.port=,file.export=parent0,node-name=replication0'}}
-{'execute': 'x-blockdev-change', 'arguments':{'parent': 'colo-disk0', 'node':
'replication0' } }
-{'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [
{'capability': 'x-colo', 'state': true } ] } }
-{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' } }
+{"execute":"qmp_capabilities"}
+{"execute": "human-monitor-command", "arguments": {"command-line": "drive_add
-n buddy
driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,file.port=,file.export=parent0,node-name=replication0"}}
+{"execute": "x-blockdev-change", "arguments":{"parent": "colo-disk0", "node":
"replication0" } }
+{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [
{"capability": "x-colo", "state": true } ] } }
+{"execute": "migrate", "arguments": {"uri": "tcp:127.0.0.2:9998" } }
Note:
a. There should be only one NBD Client for each primary disk.
@@ -249,59 +249,59 @@ if you want to resume the replication, follow "Secondary
resume replication"
== Primary Failover ==
The Secondary died, resume on the Primary
-{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
'child': 'children.1'} }
-{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_del
replication0' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
-{'execute': 'x-colo-lost-heartbeat' }
+{"execute": "x-blockdev-change", "arguments":{ "parent": "colo-disk0",
"child": "children.1"} }
+{"execute": "human-monitor-command", "arguments":{ "command-line": "drive_del
replication0" } }
+{"execute": "object-del", "arguments":{ "id": "comp0" } }
+{"execute": "object-del", "arguments":{ "id": "iothread1" } }
+{"execute": "object-del", "arguments":{ "id": "m0" } }
+{"execute": "object-del", "arguments":{ "id": "redire0" } }
+{"execute": "object-del", "arguments":{ "id": "redire1" } }
+{"execute": "x-colo-lost-heartbeat" }
== Secondary Failover ==
The Primary died, resume on the Secondary and prepare to become the new Primary
-{'execute': 'nbd-server-stop'}
-{'execute': 'x-colo-lost-heartbeat'}
+{"execute": "nbd-server-stop"}
+{"execute": "x-colo-lost-heartbeat"}
-{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
-{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
-{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
-{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
+{"execute": "object-del", "arguments":{ "id": "f2" } }
+{"execute": "object-del", "arguments":{ "id": "f1" } }
+{"execute": "chardev-remove", "arguments":{ "id": "red1" } }
+{"execute": "chardev-remove", "arguments":{ "id": "red0" } }
-{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend': {'type':
'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '0.0.0.0',
'port': '9003' } }, 'server': true } } } }
-{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend': {'type':
'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '0.0.0.0',
'port': '9004' } }, 'server': true } } } }
-{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend': {'type':
'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1',
'port': '9001' } }, 'server': true } } } }
[PULL 4/6] docs: Drop deprecated 'props' from object-add
From: "Rao, Lei"
In commit 5024340745 "qapi/qom: Drop deprecated 'props' from
object-add" (v6.0.0), we also should update documents.
Signed-off-by: Lei Rao
Message-Id: <1637567387-28250-1-git-send-email-lei@intel.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Thomas Huth
---
docs/COLO-FT.txt| 16
docs/system/authz.rst | 26 ++
docs/throttle.txt | 8 +++-
docs/tools/qemu-nbd.rst | 2 +-
4 files changed, 22 insertions(+), 30 deletions(-)
diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index 8d6d53a5a2..fd5ffcc6e5 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -289,11 +289,11 @@ Wait until disk is synced, then:
{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_add
-n buddy
driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,file.port=,file.export=parent0,node-name=replication0'}}
{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0', 'node':
'replication0' } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id':
'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire0', 'props': { 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' }
} }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire1', 'props': { 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id':
'm0', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire1', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } }
{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
'iothread1' } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id':
'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in': 'compare1',
'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id':
'comp0', 'primary_in': 'compare0-0', 'secondary_in': 'compare1', 'outdev':
'compare_out0', 'iothread': 'iothread1' } }
{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
{'capability': 'x-colo', 'state': true } ] } }
{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' } }
@@ -318,11 +318,11 @@ Wait until disk is synced, then:
{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_add
-n buddy
driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1,file.port=,file.export=parent0,node-name=replication0'}}
{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0', 'node':
'replication0' } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id':
'm0', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0',
'queue': 'tx', 'outdev': 'mirror0' } } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire0', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev':
'hn0', 'queue': 'rx', 'indev': 'compare_out' } } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire1', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev':
'hn0', 'queue': 'rx', 'outdev': 'compare0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id':
'm0', 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue':
'tx', 'outdev': 'mirror0' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire0', 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue':
'rx', 'indev': 'compare_out' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id':
'redire1', 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue':
'rx', 'outdev': 'compare0' } }
{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
'iothread1' } }
-{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id':
'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in': 'compare1',
'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id':
'comp0', 'primary_in': 'compare0-0', 'secondary_in': 'compare1', 'outdev':
'compare_out0', 'iothread': 'iothread1' } }
{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
{'capability': 'x-colo', 'state': true } ] } }
{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' } }
diff --git a/docs/system/authz.rst b/docs/system/authz.rst
index 942af39602..55b7315e49 100644
--- a/docs/system/authz.rst
[PULL 1/6] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
From: Kashyap Chamarthy
I completely botched up the merged[0] rST conversion of this document by
accidentally dropping entire hunks (!) of text. :-( I made it very hard
for reviewers to spot it, as the omitted text was buried deep in the
document. To fix my hatchet job, I reconverted the "SubmitAPatch"
wiki[1] page from scratch and replaced the existing rST with it, while
making sure I incorporated previous feedback.
In summary, in this reconverted edition:
- I did a careful (to the extent my eyes allowed) para-by-para
comparison of the wiki and the rST to make sure I didn't omit
anything accidentally.
- I made sure to work in the cosmetic feedback[2] that Thomas Huth
pointed out in the merged (and botched) edition:
- fix the hyperlinks in "Split up long patches"
- replace ".". with "does not end with a dot" (in "Write a meaningful
commit message" section)
- replace "---" with ``---`` so that it doesn't render as an em-dash
(there were two other occurrences; I fixed those too)
- Use "QEMU" spelling consistently in prose usage
- Add a consistent "refer to git-config" link where appropriate
Thanks to Thomas Huth and Alex Bennée for noticing it on IRC. And sorry
for my sloppiness.
Fixes: 9f73de8df033 ("docs: rSTify the "SubmitAPatch" wiki")
[0] https://gitlab.com/qemu-project/qemu/-/commit/9f73de8df033
[1] https://wiki.qemu.org/index.php?title=Contribute/SubmitAPatch=10387
[2] https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg03600.html
Signed-off-by: Kashyap Chamarthy
Message-Id: <2029193118.949698-2-kcham...@redhat.com>
Reviewed-by: Eric Blake
[thuth: Some more cosmetical changes, fixed links from external to internal]
Signed-off-by: Thomas Huth
---
docs/devel/stable-process.rst| 2 +
docs/devel/style.rst | 2 +
docs/devel/submitting-a-patch.rst| 198 +--
docs/devel/submitting-a-pull-request.rst | 9 +-
docs/devel/trivial-patches.rst | 2 +
5 files changed, 161 insertions(+), 52 deletions(-)
diff --git a/docs/devel/stable-process.rst b/docs/devel/stable-process.rst
index e541b983fa..c21fb86645 100644
--- a/docs/devel/stable-process.rst
+++ b/docs/devel/stable-process.rst
@@ -1,3 +1,5 @@
+.. _stable-process:
+
QEMU and the stable process
===
diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index e00af62e76..9c5c0fffd9 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -1,3 +1,5 @@
+.. _coding-style:
+
=
QEMU Coding Style
=
diff --git a/docs/devel/submitting-a-patch.rst
b/docs/devel/submitting-a-patch.rst
index 6fefc67d52..6acded5c93 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -1,3 +1,5 @@
+.. _submitting-a-patch:
+
Submitting a Patch
==
@@ -20,11 +22,11 @@ one-shot fix, the bare minimum we ask is that:
should not be posted on the bug tracker, posted on forums, or
externally hosted and linked to. (We have other mailing lists too,
but all patches must go to qemu-devel, possibly with a Cc: to another
- list.) ``git send-email`` works best for delivering the patch without
- mangling it (`hints for setting it
- up
`__),
- but attachments can be used as a last resort on a first-time
- submission.
+ list.) ``git send-email`` (`step-by-step setup
+ guide `__)
+ works best for delivering the patch without mangling it, but
+ attachments can be used as a last resort on a first-time submission.
- You must read replies to your message, and be willing to act on them.
Note, however, that maintainers are often willing to manually fix up
first-time contributions, since there is a learning curve involved in
@@ -45,6 +47,8 @@ Reading the table of contents below should already give you
an idea of
the basic requirements. Use the table of contents as a reference, and
read the parts that you have doubts about.
+.. contents:: Table of Contents
+
.. _writing_your_patches:
Writing your Patches
@@ -60,11 +64,9 @@ check that you are in compliance with our coding standards.
Be aware
that ``checkpatch.pl`` is not infallible, though, especially where C
preprocessor macros are involved; use some common sense too. See also:
-- `QEMU Coding Style
- `__
-
+- :ref:`coding-style`
- `Automate a checkpatch run on
- commit
`__
+ commit
`__
.. _base_patches_against_current_git_master:
@@ -76,6 +78,13 @@ of QEMU because development will have moved on from then and
it
[PULL 0/6] Documentation updates
Hi Richard! The following changes since commit c5fbdd60cf1fb52f01bdfe342b6fa65d5343e1b1: Merge tag 'qemu-sparc-20211121' of git://github.com/mcayland/qemu into staging (2021-11-21 14:12:25 +0100) are available in the Git repository at: https://gitlab.com/thuth/qemu.git tags/pull-request-2021-11-22 for you to fetch changes up to c5ba62195427d65a44472901cff3dddffc14b3b3: docs: Render binary names as monospaced text (2021-11-22 15:02:38 +0100) * Documentation updates Kashyap Chamarthy (2): docs: Fix botched rST conversion of 'submitting-a-patch.rst' docs: List more commit-message tags in "submitting-a-patch" Philippe Mathieu-Daudé (1): docs: Render binary names as monospaced text Rao, Lei (2): docs: Drop deprecated 'props' from object-add docs: Use double quotes instead of single quotes for COLO Stefan Weil (1): Fix some typos in documentation (found by codespell) docs/COLO-FT.txt | 106 docs/about/removed-features.rst | 8 +- docs/block-replication.txt | 52 docs/devel/build-system.rst | 6 +- docs/devel/multi-process.rst | 8 +- docs/devel/qgraph.rst| 2 +- docs/devel/stable-process.rst| 2 + docs/devel/style.rst | 2 + docs/devel/submitting-a-patch.rst| 202 +++ docs/devel/submitting-a-pull-request.rst | 9 +- docs/devel/testing.rst | 8 +- docs/devel/trivial-patches.rst | 2 + docs/devel/writing-monitor-commands.rst | 2 +- docs/hyperv.txt | 2 +- docs/image-fuzzer.txt| 6 +- docs/system/arm/orangepi.rst | 2 +- docs/system/authz.rst| 26 ++-- docs/system/cpu-models-x86.rst.inc | 2 +- docs/system/devices/nvme.rst | 2 +- docs/system/gdb.rst | 2 +- docs/system/images.rst | 2 +- docs/system/ppc/ppce500.rst | 2 +- docs/system/qemu-block-drivers.rst.inc | 6 +- docs/system/riscv/shakti-c.rst | 2 +- docs/system/tls.rst | 2 +- docs/throttle.txt| 8 +- docs/tools/qemu-img.rst | 18 +-- docs/tools/qemu-nbd.rst | 6 +- docs/tools/qemu-storage-daemon.rst | 7 +- docs/tools/virtiofsd.rst | 4 +- 30 files changed, 307 insertions(+), 201 deletions(-)
Re: [RFC PATCH-for-6.2 v3] qdev-monitor: Only allow full --global .= option
Daniel P. Berrangé writes:
> On Sat, Nov 20, 2021 at 07:53:20AM +0100, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé writes:
>>
>> > When not all fields of the --global option are provided,
>> > QEMU might crash:
>> >
>> > $ qemu-system-x86_64 -global driver=isa-fdc
>> > qemu-system-x86_64: ../../devel/qemu/qapi/string-input-visitor.c:394:
>> > string_input_visitor_new: Assertion `str' failed.
>> > Aborted (core dumped)
>> >
>> > Fix by only allowing --global with all 3 fields:
>> >
>> > $ qemu-system-x86_64 -global driver=isa-fdc
>> > Invalid 'global' option format. It must be provided as:
>> > --global .=
>> >
>> > Reported-by: Thomas Huth
>> > Suggested-by: Markus Armbruster
>> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/604
>> > Signed-off-by: Philippe Mathieu-Daudé
>> > ---
>> > v3: Change qemu_global_option (Markus)
>> >
>> > Supersedes: <2029122911.365036-1-phi...@redhat.com>
>> > ---
>> > softmmu/qdev-monitor.c | 9 +++--
>> > 1 file changed, 3 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
>> > index 01f3834db57..558272b147c 100644
>> > --- a/softmmu/qdev-monitor.c
>> > +++ b/softmmu/qdev-monitor.c
>> > @@ -1029,13 +1029,10 @@ int qemu_global_option(const char *str)
>> > qemu_opt_set(opts, "value", str + offset + 1, _abort);
>> > return 0;
>> > }
>> > +printf("Invalid 'global' option format. It must be provided as:\n");
>> > +printf(" --global .=\n");
>> >
>> > -opts = qemu_opts_parse_noisily(_global_opts, str, false);
>> > -if (!opts) {
>> > -return -1;
>> > -}
>> > -
>> > -return 0;
>> > +return -1;
>> > }
>> >
>> > bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>>
>> This drops a documented part of the external interface:
>>
>> $ qemu-system-x86_64 -help | grep -C 1 global
>> i.e. -set drive.$id.file=/path/to/image
>> -global driver.property=value
>> --> -global driver=driver,property=property,value=value
>> set a global default for a driver property
>
> This doc makes it look like the two syntaxes are functionally
> equivalent, but it seems that's not quite the case.
>
> libvirt uses the driver.propert=value syntax for everything
> except one case
>
> -global driver=cfi.pflash01,property=secure,value=on
>
> for that one if we try to use
>
> -global cfi.pflash01.secure=on
>
> it complains
>
> qemu-system-x86_64: warning: global cfi.pflash01.secure has invalid class
> name
>
> what's going on here ?
Off-the-cuff guess: cfi.pflash01.secure=on gets parsed as
driver=cfi
property=pflash01.secure
value=on
Once again our "anything goes" attitude to naming wastes us time and
thus money.
In contrast, QAPI restricts names to "only ASCII letters, digits,
hyphen, and underscore" (see docs/devel/qapi-code-gen.rst section Naming
rules and reserved names).
[PATCH] linux-user: implement more loop ioctls
LOOP_CONFIGURE is now used by losetup, and it cannot cope with ENOSYS.
Signed-off-by: Andreas Schwab
---
linux-user/ioctls.h| 4
linux-user/linux_loop.h| 2 ++
linux-user/syscall_defs.h | 4
linux-user/syscall_types.h | 6 ++
4 files changed, 16 insertions(+)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 7193c3b226..5ac5efc8aa 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -637,6 +637,10 @@
IOCTL(LOOP_SET_STATUS64, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_GET_STATUS64, IOC_R, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
IOCTL(LOOP_CHANGE_FD, 0, TYPE_INT)
+ IOCTL(LOOP_SET_CAPACITY, 0, TYPE_INT)
+ IOCTL(LOOP_SET_DIRECT_IO, 0, TYPE_INT)
+ IOCTL(LOOP_SET_BLOCK_SIZE, 0, TYPE_INT)
+ IOCTL(LOOP_CONFIGURE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_configure)))
IOCTL(LOOP_CTL_ADD, 0, TYPE_INT)
IOCTL(LOOP_CTL_REMOVE, 0, TYPE_INT)
diff --git a/linux-user/linux_loop.h b/linux-user/linux_loop.h
index c69fea11e4..f80b96f1ff 100644
--- a/linux-user/linux_loop.h
+++ b/linux-user/linux_loop.h
@@ -96,6 +96,8 @@ struct loop_info64 {
#define LOOP_CHANGE_FD 0x4C06
#define LOOP_SET_CAPACITY 0x4C07
#define LOOP_SET_DIRECT_IO 0x4C08
+#define LOOP_SET_BLOCK_SIZE 0x4C09
+#define LOOP_CONFIGURE 0x4C0A
/* /dev/loop-control interface */
#define LOOP_CTL_ADD0x4C80
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc..560a29afd8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1219,6 +1219,10 @@ struct target_rtc_pll_info {
#define TARGET_LOOP_SET_STATUS64 0x4C04
#define TARGET_LOOP_GET_STATUS64 0x4C05
#define TARGET_LOOP_CHANGE_FD 0x4C06
+#define TARGET_LOOP_SET_CAPACITY 0x4C07
+#define TARGET_LOOP_SET_DIRECT_IO 0x4C08
+#define TARGET_LOOP_SET_BLOCK_SIZE0x4C09
+#define TARGET_LOOP_CONFIGURE 0x4C0A
#define TARGET_LOOP_CTL_ADD 0x4C80
#define TARGET_LOOP_CTL_REMOVE0x4C81
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ba2c1518eb..7c46e4fb25 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -201,6 +201,12 @@ STRUCT(loop_info64,
MK_ARRAY(TYPE_CHAR, 32), /* lo_encrypt_key */
MK_ARRAY(TYPE_ULONGLONG, 2)) /* lo_init */
+STRUCT(loop_configure,
+ TYPE_INT, /* fd */
+ TYPE_INT, /* block_size */
+ MK_STRUCT(STRUCT_loop_info64), /* info */
+ MK_ARRAY(TYPE_ULONGLONG, 8)) /* __reserved */
+
/* mag tape ioctls */
STRUCT(mtop, TYPE_SHORT, TYPE_INT)
STRUCT(mtget, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG,
--
2.34.0
Re: [RFC v2 PATCH 13/13] KVM: Enable memfd based page invalidation/fallocate
On Fri, Nov 19, 2021 at 09:47:39PM +0800, Chao Peng wrote:
> Since the memory backing store does not get notified when VM is
> destroyed so need check if VM is still live in these callbacks.
>
> Signed-off-by: Yu Zhang
> Signed-off-by: Chao Peng
> ---
> virt/kvm/memfd.c | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/virt/kvm/memfd.c b/virt/kvm/memfd.c
> index bd930dcb455f..bcfdc685ce22 100644
> --- a/virt/kvm/memfd.c
> +++ b/virt/kvm/memfd.c
> @@ -12,16 +12,38 @@
> #include
> const static struct guest_mem_ops *memfd_ops;
>
> +static bool vm_is_dead(struct kvm *vm)
> +{
> + struct kvm *kvm;
> +
> + list_for_each_entry(kvm, _list, vm_list) {
> + if (kvm == vm)
> + return false;
> + }
I don't think this is enough. The struct kvm can be freed and re-allocated
from the slab and this function will give false-negetive.
Maybe the kvm has to be tagged with a sequential id that incremented every
allocation. This id can be checked here.
> +
> + return true;
> +}
--
Kirill A. Shutemov
Re: [PATCH for-6.2] qapi/machine.json: Fix incorrect description for die-id
On Mon, Nov 22, 2021 at 11:26:51AM +0800, Yanan Wang wrote:
> In terms of scope, die-id should mean "the die number within
> socket the CPU belongs to" instead of "the die number within
> node/board the CPU belongs to". Fix it to avoid confusing
> the Doc reader.
>
> Fixes: 176d2cda0d ("i386/cpu: Consolidate die-id validity in smp context")
> Signed-off-by: Yanan Wang
> ---
> qapi/machine.json | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 067e3f5378..f1839acf20 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -867,7 +867,7 @@
> #
> # @node-id: NUMA node ID the CPU belongs to
> # @socket-id: socket number within node/board the CPU belongs to
> -# @die-id: die number within node/board the CPU belongs to (Since 4.1)
> +# @die-id: die number within socket the CPU belongs to (since 4.1)
> # @core-id: core number within die the CPU belongs to
> # @thread-id: thread number within core the CPU belongs to
> #
> --
> 2.19.1
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On 22/11/2021 14.53, Peter Maydell wrote: On Mon, 22 Nov 2021 at 13:30, Thomas Huth wrote: On 22/11/2021 14.25, Peter Maydell wrote: On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: What about simply replacing it with a new sentence below the bullet list, saying: "Please also use a spell checker like `codespell https://github.com/codespell-project/codespell` with your patches" How many regular contributors actually do that? Considering the typos that we have in the code, not enough ;-) Anyway, it's just a polite recommendation here, not a must-do, so mentioning codespell here doesn't really hurt, does it? I don't think we should be recommending to new contributors that they do things that established contributors generally do not do. The document has enough "things you should do or think about" already. My preference would be for simply not mentioning spelling-checking. (If we do want to come up with some process for dealing with spelling issues in the codebase, then we either need to put it into CI so it's run automatically, or we need to have something that works on the individual patch level.) Ok ... In any case - seems like this needs more discussion, so I'll drop it from the patch for now. We can still add some wording or CI magic later, but that's certainly something that we rather want to do after version 6.2 has been released... Thomas
[PULL 1/2] nbd/server: Don't complain on certain client disconnects
When a client disconnects abruptly, but did not have any pending
requests (for example, when using nbdsh without calling h.shutdown),
we used to output the following message:
$ qemu-nbd -f raw file
$ nbdsh -u 'nbd://localhost:10809' -c 'h.trim(1,0)'
qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected
end-of-file before all bytes were read
Then in commit f148ae7, we refactored nbd_receive_request() to use
nbd_read_eof(); when this returns 0, we regressed into tracing
uninitialized memory (if tracing is enabled) and reporting a
less-specific:
qemu-nbd: Disconnect client, due to: Request handling failed in intermediate
state
Note that with Unix sockets, we have yet another error message,
unchanged by the 6.0 regression:
$ qemu-nbd -k /tmp/sock -f raw file
$ nbdsh -u 'nbd+unix:///?socket=/tmp/sock' -c 'h.trim(1,0)'
qemu-nbd: Disconnect client, due to: Failed to send reply: Unable to write to
socket: Broken pipe
But in all cases, the error message goes away if the client performs a
soft shutdown by using NBD_CMD_DISC, rather than a hard shutdown by
abrupt disconnect:
$ nbdsh -u 'nbd://localhost:10809' -c 'h.trim(1,0)' -c 'h.shutdown()'
This patch fixes things to avoid uninitialized memory, and in general
avoids warning about a client that does a hard shutdown when not in
the middle of a packet. A client that aborts mid-request, or which
does not read the full server's reply, can still result in warnings,
but those are indeed much more unusual situations.
CC: qemu-sta...@nongnu.org
Fixes: f148ae7d36 ("nbd/server: Quiesce coroutines on context switch", v6.0.0)
Signed-off-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
[eblake: defer unrelated typo fixes to later patch]
Message-Id: <2027170230.1128262-2-ebl...@redhat.com>
Signed-off-by: Eric Blake
---
nbd/server.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/nbd/server.c b/nbd/server.c
index d9164ee6d0da..74ba48709451 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1418,6 +1418,9 @@ static int nbd_receive_request(NBDClient *client,
NBDRequest *request,
if (ret < 0) {
return ret;
}
+if (ret == 0) {
+return -EIO;
+}
/* Request
[ 0 .. 3] magic (NBD_REQUEST_MAGIC)
--
2.33.1
[PULL 0/2] NBD patches for 6.2-rc2, 2021-11-22
The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2: Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu into staging (2021-11-22 10:33:13 +0100) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-11-22 for you to fetch changes up to e35574226a63f29e32eda8da5cc14832f19850e2: nbd/server: Simplify zero and trim (2021-11-22 07:37:15 -0600) nbd patches for 2021-11-22 - Eric Blake: Avoid uninitialized memory on client hard disconnect - Eric Blake: Take advantage of block layer 64-bit zero/trim Eric Blake (2): nbd/server: Don't complain on certain client disconnects nbd/server: Simplify zero and trim nbd/server.c | 26 ++ 1 file changed, 6 insertions(+), 20 deletions(-) -- 2.33.1
[PULL 2/2] nbd/server: Simplify zero and trim
Now that the block layer supports 64-bit operations (see commit
2800637a and friends, new to v6.2), we no longer have to self-fragment
requests larger than 2G, reverting the workaround added in 890cbccb08
("nbd: Fix large trim/zero requests", v5.1.0).
Signed-off-by: Eric Blake
Message-Id: <2027170230.1128262-3-ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
nbd/server.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 74ba48709451..4630dd732250 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2509,16 +2509,8 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
if (request->flags & NBD_CMD_FLAG_FAST_ZERO) {
flags |= BDRV_REQ_NO_FALLBACK;
}
-ret = 0;
-/* FIXME simplify this when blk_pwrite_zeroes switches to 64-bit */
-while (ret >= 0 && request->len) {
-int align = client->check_align ?: 1;
-int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
-align));
-ret = blk_pwrite_zeroes(exp->common.blk, request->from, len,
flags);
-request->len -= len;
-request->from += len;
-}
+ret = blk_pwrite_zeroes(exp->common.blk, request->from, request->len,
+flags);
return nbd_send_generic_reply(client, request->handle, ret,
"writing to file failed", errp);
@@ -2532,16 +2524,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
"flush failed", errp);
case NBD_CMD_TRIM:
-ret = 0;
-/* FIXME simplify this when blk_co_pdiscard switches to 64-bit */
-while (ret >= 0 && request->len) {
-int align = client->check_align ?: 1;
-int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
-align));
-ret = blk_co_pdiscard(exp->common.blk, request->from, len);
-request->len -= len;
-request->from += len;
-}
+ret = blk_co_pdiscard(exp->common.blk, request->from, request->len);
if (ret >= 0 && request->flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->common.blk);
}
--
2.33.1
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On Mon, Nov 22, 2021 at 02:35:49PM +0100, David Hildenbrand wrote: > On 22.11.21 14:31, Jason Gunthorpe wrote: > > On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: > > > >> I do wonder if we want to support sharing such memfds between processes > >> in all cases ... we most certainly don't want to be able to share > >> encrypted memory between VMs (I heard that the kernel has to forbid > >> that). It would make sense in the use case you describe, though. > > > > If there is a F_SEAL_XX that blocks every kind of new access, who > > cares if userspace passes the FD around or not? > I was imagining that you actually would want to do some kind of "change > ownership". But yeah, the intended semantics and all use cases we have > in mind are not fully clear to me yet. If it's really "no new access" > (side note: is "access" the right word?) then sure, we can pass the fd > around. What is "ownership" in a world with kvm and iommu are reading pages out of the same fd? "no new access" makes sense to me, we have access through read/write/mmap/splice/etc and access to pages through the private in kernel interface (kvm, iommu) Jason
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On Fri, Nov 19, 2021 at 02:51:11PM +0100, David Hildenbrand wrote: > On 19.11.21 14:47, Chao Peng wrote: > > From: "Kirill A. Shutemov" > > > > The new seal type provides semantics required for KVM guest private > > memory support. A file descriptor with the seal set is going to be used > > as source of guest memory in confidential computing environments such as > > Intel TDX and AMD SEV. > > > > F_SEAL_GUEST can only be set on empty memfd. After the seal is set > > userspace cannot read, write or mmap the memfd. > > > > Userspace is in charge of guest memory lifecycle: it can allocate the > > memory with falloc or punch hole to free memory from the guest. > > > > The file descriptor passed down to KVM as guest memory backend. KVM > > register itself as the owner of the memfd via memfd_register_guest(). > > > > KVM provides callback that needed to be called on fallocate and punch > > hole. > > > > memfd_register_guest() returns callbacks that need be used for > > requesting a new page from memfd. > > > > Repeating the feedback I already shared in a private mail thread: > > > As long as page migration / swapping is not supported, these pages > behave like any longterm pinned pages (e.g., VFIO) or secretmem pages. > > 1. These pages are not MOVABLE. They must not end up on ZONE_MOVABLE or > MIGRATE_CMA. > > That should be easy to handle, you have to adjust the gfp_mask to > mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER); > just as mm/secretmem.c:secretmem_file_create() does. Okay, fair enough. mapping_set_unevictable() also makes sesne. > 2. These pages behave like mlocked pages and should be accounted as such. > > This is probably where the accounting "fun" starts, but maybe it's > easier than I think to handle. > > See mm/secretmem.c:secretmem_mmap(), where we account the pages as > VM_LOCKED and will consequently check per-process mlock limits. As we > don't mmap(), the same approach cannot be reused. > > See drivers/vfio/vfio_iommu_type1.c:vfio_pin_map_dma() and > vfio_pin_pages_remote() on how to manually account via mm->locked_vm . > > But it's a bit hairy because these pages are not actually mapped into > the page tables of the MM, so it might need some thought. Similarly, > these pages actually behave like "pinned" (as in mm->pinned_vm), but we > just don't increase the refcount AFAIR. Again, accounting really is a > bit hairy ... Accounting is fun indeed. Non-mapped mlocked memory is going to be confusing. Hm... I will look closer. -- Kirill A. Shutemov
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On Mon, 22 Nov 2021 at 13:30, Thomas Huth wrote: > > On 22/11/2021 14.25, Peter Maydell wrote: > > On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: > >> What about simply replacing it with a new sentence below the bullet list, > >> saying: > >> > >> "Please also use a spell checker like `codespell > >> https://github.com/codespell-project/codespell` with your patches" > > > > How many regular contributors actually do that? > > Considering the typos that we have in the code, not enough ;-) > > Anyway, it's just a polite recommendation here, not a must-do, so mentioning > codespell here doesn't really hurt, does it? I don't think we should be recommending to new contributors that they do things that established contributors generally do not do. The document has enough "things you should do or think about" already. My preference would be for simply not mentioning spelling-checking. (If we do want to come up with some process for dealing with spelling issues in the codebase, then we either need to put it into CI so it's run automatically, or we need to have something that works on the individual patch level.) -- PMM
[PULL 0/1] target-arm queue
Just one patch for rc2, a revert. -- PMM The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2: Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu into staging (2021-11-22 10:33:13 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211122 for you to fetch changes up to 4825eaae4fdd56fba0febdfbdd7bf9684ae3ee0d: Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2" (2021-11-22 13:41:48 +) target-arm queue: * revert SMCCC/PSCI change, as it regresses some usecases for some boards Peter Maydell (1): Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2" target/arm/psci.c | 35 +-- 1 file changed, 29 insertions(+), 6 deletions(-)
[PULL 1/1] Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2"
This reverts commit 9fcd15b9193e819b6cc2fd0a45e3506148812bb4.
This change turns out to cause regressions, for instance on the
imx6ul boards as described here:
https://lore.kernel.org/qemu-devel/c8b89685-7490-328b-51a3-48711c140...@tribudubois.net/
The primary cause of that regression is that the guest code running
at EL3 expects SMCs (not related to PSCI) to do what they would if
our PSCI emulation was not present at all, but after this change
they instead set a value in R0/X0 and continue.
We could fix that by a refactoring that allowed us to only turn on
the PSCI emulation if we weren't booting the guest at EL3, but there
is a more tangled problem with the highbank board, which:
(1) wants to enable PSCI emulation
(2) has a bit of guest code that it wants to run at EL3 and
to perform SMC calls that trap to the monitor vector table:
this is the boot stub code that is written to memory by
arm_write_secure_board_setup_dummy_smc() and which the
highbank board enables by setting bootinfo->secure_board_setup
We can't satisfy both of those and also have the PSCI emulation
handle all SMC instruction executions regardless of function
identifier value.
This is too tricky to try to sort out before 6.2 is released;
revert this commit so we can take the time to get it right in
the 7.0 release.
Signed-off-by: Peter Maydell
Message-id: 2029163419.557623-1-peter.mayd...@linaro.org
---
target/arm/psci.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/target/arm/psci.c b/target/arm/psci.c
index b279c0b9a45..6709e280133 100644
--- a/target/arm/psci.c
+++ b/target/arm/psci.c
@@ -27,13 +27,15 @@
bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
{
-/*
- * Return true if the exception type matches the configured PSCI conduit.
- * This is called before the SMC/HVC instruction is executed, to decide
- * whether we should treat it as a PSCI call or with the architecturally
+/* Return true if the r0/x0 value indicates a PSCI call and
+ * the exception type matches the configured PSCI conduit. This is
+ * called before the SMC/HVC instruction is executed, to decide whether
+ * we should treat it as a PSCI call or with the architecturally
* defined behaviour for an SMC or HVC (which might be UNDEF or trap
* to EL2 or to EL3).
*/
+CPUARMState *env = >env;
+uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];
switch (excp_type) {
case EXCP_HVC:
@@ -50,7 +52,27 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
return false;
}
-return true;
+switch (param) {
+case QEMU_PSCI_0_2_FN_PSCI_VERSION:
+case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
+case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
+case QEMU_PSCI_0_1_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN64_CPU_ON:
+case QEMU_PSCI_0_1_FN_CPU_OFF:
+case QEMU_PSCI_0_2_FN_CPU_OFF:
+case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
+case QEMU_PSCI_0_1_FN_MIGRATE:
+case QEMU_PSCI_0_2_FN_MIGRATE:
+return true;
+default:
+return false;
+}
}
void arm_handle_psci_call(ARMCPU *cpu)
@@ -172,9 +194,10 @@ void arm_handle_psci_call(ARMCPU *cpu)
break;
case QEMU_PSCI_0_1_FN_MIGRATE:
case QEMU_PSCI_0_2_FN_MIGRATE:
-default:
ret = QEMU_PSCI_RET_NOT_SUPPORTED;
break;
+default:
+g_assert_not_reached();
}
err:
--
2.25.1
Re: [PATCH] vfio/migration: Improve to read/write full migration region per chunk
On Mon, 22 Nov 2021 09:40:56 +0200
Yishai Hadas wrote:
> Gentle ping for review, CCing more people who may be involved.
I'll wait for comments from others, but since we're already in the 6.2
freeze and vfio migration is still experimental (and I'm on PTO this
week), I expect this to be queued when the next development window
opens. Thanks,
Alex
> On 11/11/2021 11:50 AM, Yishai Hadas wrote:
> > Upon reading/writing the migration data there is no real reason to limit
> > the read/write system call from the file to be 8 bytes.
> >
> > In addition, there is no reason to depend on the file offset alignment.
> > The offset is just some logical value which depends also on the region
> > index and has nothing to do with the amount of data that can be
> > accessed.
> >
> > Move to read/write the full region size per chunk, this reduces
> > dramatically the number of the systems calls that are needed and improve
> > performance.
> >
> > Signed-off-by: Yishai Hadas
> > ---
> > hw/vfio/migration.c | 36 ++--
> > 1 file changed, 2 insertions(+), 34 deletions(-)
> >
> > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > index ff6b45de6b5..b5f310bb831 100644
> > --- a/hw/vfio/migration.c
> > +++ b/hw/vfio/migration.c
> > @@ -62,40 +62,8 @@ static inline int vfio_mig_access(VFIODevice *vbasedev,
> > void *val, int count,
> > return 0;
> > }
> >
> > -static int vfio_mig_rw(VFIODevice *vbasedev, __u8 *buf, size_t count,
> > - off_t off, bool iswrite)
> > -{
> > -int ret, done = 0;
> > -__u8 *tbuf = buf;
> > -
> > -while (count) {
> > -int bytes = 0;
> > -
> > -if (count >= 8 && !(off % 8)) {
> > -bytes = 8;
> > -} else if (count >= 4 && !(off % 4)) {
> > -bytes = 4;
> > -} else if (count >= 2 && !(off % 2)) {
> > -bytes = 2;
> > -} else {
> > -bytes = 1;
> > -}
> > -
> > -ret = vfio_mig_access(vbasedev, tbuf, bytes, off, iswrite);
> > -if (ret) {
> > -return ret;
> > -}
> > -
> > -count -= bytes;
> > -done += bytes;
> > -off += bytes;
> > -tbuf += bytes;
> > -}
> > -return done;
> > -}
> > -
> > -#define vfio_mig_read(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o,
> > false)
> > -#define vfio_mig_write(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o,
> > true)
> > +#define vfio_mig_read(f, v, c, o) vfio_mig_access(f, (__u8 *)v, c,
> > o, false)
> > +#define vfio_mig_write(f, v, c, o) vfio_mig_access(f, (__u8 *)v, c,
> > o, true)
> >
> > #define VFIO_MIG_STRUCT_OFFSET(f) \
> >offsetof(struct
> > vfio_device_migration_info, f)
>
>
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On 22/11/2021 14.33, Daniel P. Berrangé wrote: On Mon, Nov 22, 2021 at 02:30:10PM +0100, Thomas Huth wrote: On 22/11/2021 14.25, Peter Maydell wrote: On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: What about simply replacing it with a new sentence below the bullet list, saying: "Please also use a spell checker like `codespell https://github.com/codespell-project/codespell` with your patches" How many regular contributors actually do that? Considering the typos that we have in the code, not enough ;-) Anyway, it's just a polite recommendation here, not a must-do, so mentioning codespell here doesn't really hurt, does it? Well if you run 'codespell' with no args on qemu.git right now, you get over 5000 possible mistakes reported. Many (perhaps even most) will be false positives, but with that amount of existing report, I don't think its credible to request contributors to run this and wade through its results to see if they made things worse or not. Ok, then maybe something like this: "If your patch adds new documentation, then please also consider to use a spell checker like `codespell` to avoid typos in the new text" ? Otherwise, I can also simply drop that sentence about spell checking. Thomas
Re: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On 22.11.21 14:31, Jason Gunthorpe wrote: > On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: > >> I do wonder if we want to support sharing such memfds between processes >> in all cases ... we most certainly don't want to be able to share >> encrypted memory between VMs (I heard that the kernel has to forbid >> that). It would make sense in the use case you describe, though. > > If there is a F_SEAL_XX that blocks every kind of new access, who > cares if userspace passes the FD around or not? I was imagining that you actually would want to do some kind of "change ownership". But yeah, the intended semantics and all use cases we have in mind are not fully clear to me yet. If it's really "no new access" (side note: is "access" the right word?) then sure, we can pass the fd around. -- Thanks, David / dhildenb
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On Mon, Nov 22, 2021 at 02:30:10PM +0100, Thomas Huth wrote: > On 22/11/2021 14.25, Peter Maydell wrote: > > On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: > > > What about simply replacing it with a new sentence below the bullet list, > > > saying: > > > > > > "Please also use a spell checker like `codespell > > > https://github.com/codespell-project/codespell` with your patches" > > > > How many regular contributors actually do that? > > Considering the typos that we have in the code, not enough ;-) > > Anyway, it's just a polite recommendation here, not a must-do, so mentioning > codespell here doesn't really hurt, does it? Well if you run 'codespell' with no args on qemu.git right now, you get over 5000 possible mistakes reported. Many (perhaps even most) will be false positives, but with that amount of existing report, I don't think its credible to request contributors to run this and wade through its results to see if they made things worse or not. 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: [RFC v2 PATCH 01/13] mm/shmem: Introduce F_SEAL_GUEST
On Mon, Nov 22, 2021 at 10:26:12AM +0100, David Hildenbrand wrote: > I do wonder if we want to support sharing such memfds between processes > in all cases ... we most certainly don't want to be able to share > encrypted memory between VMs (I heard that the kernel has to forbid > that). It would make sense in the use case you describe, though. If there is a F_SEAL_XX that blocks every kind of new access, who cares if userspace passes the FD around or not? Jason
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On 22/11/2021 14.25, Peter Maydell wrote: On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: What about simply replacing it with a new sentence below the bullet list, saying: "Please also use a spell checker like `codespell https://github.com/codespell-project/codespell` with your patches" How many regular contributors actually do that? Considering the typos that we have in the code, not enough ;-) Anyway, it's just a polite recommendation here, not a must-do, so mentioning codespell here doesn't really hurt, does it? Thomas
Re: [PATCH 1/2] docs: Fix botched rST conversion of 'submitting-a-patch.rst'
On Mon, 22 Nov 2021 at 12:37, Thomas Huth wrote: > What about simply replacing it with a new sentence below the bullet list, > saying: > > "Please also use a spell checker like `codespell > https://github.com/codespell-project/codespell` with your patches" How many regular contributors actually do that? -- PMM
Re: [RFC PATCH-for-6.2 v3] qdev-monitor: Only allow full --global .= option
On Sat, Nov 20, 2021 at 07:53:20AM +0100, Markus Armbruster wrote:
> Philippe Mathieu-Daudé writes:
>
> > When not all fields of the --global option are provided,
> > QEMU might crash:
> >
> > $ qemu-system-x86_64 -global driver=isa-fdc
> > qemu-system-x86_64: ../../devel/qemu/qapi/string-input-visitor.c:394:
> > string_input_visitor_new: Assertion `str' failed.
> > Aborted (core dumped)
> >
> > Fix by only allowing --global with all 3 fields:
> >
> > $ qemu-system-x86_64 -global driver=isa-fdc
> > Invalid 'global' option format. It must be provided as:
> > --global .=
> >
> > Reported-by: Thomas Huth
> > Suggested-by: Markus Armbruster
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/604
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > v3: Change qemu_global_option (Markus)
> >
> > Supersedes: <2029122911.365036-1-phi...@redhat.com>
> > ---
> > softmmu/qdev-monitor.c | 9 +++--
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> > index 01f3834db57..558272b147c 100644
> > --- a/softmmu/qdev-monitor.c
> > +++ b/softmmu/qdev-monitor.c
> > @@ -1029,13 +1029,10 @@ int qemu_global_option(const char *str)
> > qemu_opt_set(opts, "value", str + offset + 1, _abort);
> > return 0;
> > }
> > +printf("Invalid 'global' option format. It must be provided as:\n");
> > +printf(" --global .=\n");
> >
> > -opts = qemu_opts_parse_noisily(_global_opts, str, false);
> > -if (!opts) {
> > -return -1;
> > -}
> > -
> > -return 0;
> > +return -1;
> > }
> >
> > bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>
> This drops a documented part of the external interface:
>
> $ qemu-system-x86_64 -help | grep -C 1 global
> i.e. -set drive.$id.file=/path/to/image
> -global driver.property=value
> --> -global driver=driver,property=property,value=value
> set a global default for a driver property
This doc makes it look like the two syntaxes are functionally
equivalent, but it seems that's not quite the case.
libvirt uses the driver.propert=value syntax for everything
except one case
-global driver=cfi.pflash01,property=secure,value=on
for that one if we try to use
-global cfi.pflash01.secure=on
it complains
qemu-system-x86_64: warning: global cfi.pflash01.secure has invalid class name
what's going on here ?
> -boot [order=drives][,once=drives][,menu=on|off]
>
> It goes back to commit 3751d7c43f "vl: allow full-blown QemuOpts syntax
> for -global", v2.4.0.
>
> The appropriate fix is to check @opts for presence of all three
> parameters.
>
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 :|
[PATCH for-6.2 2/2] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
From: Warner Losh
All instances of rewind_if_in_safe_syscall are the same, differing only
in how the instruction point is fetched from the ucontext and the size
of the registers. Use host_signal_pc and new host_signal_set_pc
interfaces to fetch the pointer to the PC and adjust if needed. Delete
all the old copies of rewind_if_in_safe_syscall.
Signed-off-by: Warner Losh
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <2023045603.60391-3-...@bsdimp.com>
[rth: include safe-syscall.h, simplify ifdefs]
Signed-off-by: Richard Henderson
---
linux-user/host/aarch64/hostdep.h | 20
linux-user/host/arm/hostdep.h | 20
linux-user/host/i386/hostdep.h| 20
linux-user/host/ppc64/hostdep.h | 20
linux-user/host/riscv/hostdep.h | 20
linux-user/host/s390x/hostdep.h | 20
linux-user/host/x86_64/hostdep.h | 20
linux-user/safe-syscall.h | 3 +++
linux-user/signal.c | 15 ---
9 files changed, 15 insertions(+), 143 deletions(-)
diff --git a/linux-user/host/aarch64/hostdep.h
b/linux-user/host/aarch64/hostdep.h
index a8d41a21ad..39299d798a 100644
--- a/linux-user/host/aarch64/hostdep.h
+++ b/linux-user/host/aarch64/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* Adjust the signal context to rewind out of safe-syscall if we're in it */
-static inline void rewind_if_in_safe_syscall(void *puc)
-{
-ucontext_t *uc = puc;
-__u64 *pcreg = >uc_mcontext.pc;
-
-if (*pcreg > (uintptr_t)safe_syscall_start
-&& *pcreg < (uintptr_t)safe_syscall_end) {
-*pcreg = (uintptr_t)safe_syscall_start;
-}
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
index 9276fe6ceb..86b137875a 100644
--- a/linux-user/host/arm/hostdep.h
+++ b/linux-user/host/arm/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* Adjust the signal context to rewind out of safe-syscall if we're in it */
-static inline void rewind_if_in_safe_syscall(void *puc)
-{
-ucontext_t *uc = puc;
-unsigned long *pcreg = >uc_mcontext.arm_pc;
-
-if (*pcreg > (uintptr_t)safe_syscall_start
-&& *pcreg < (uintptr_t)safe_syscall_end) {
-*pcreg = (uintptr_t)safe_syscall_start;
-}
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/i386/hostdep.h b/linux-user/host/i386/hostdep.h
index 073be74d87..ce7136501f 100644
--- a/linux-user/host/i386/hostdep.h
+++ b/linux-user/host/i386/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* Adjust the signal context to rewind out of safe-syscall if we're in it */
-static inline void rewind_if_in_safe_syscall(void *puc)
-{
-ucontext_t *uc = puc;
-greg_t *pcreg = >uc_mcontext.gregs[REG_EIP];
-
-if (*pcreg > (uintptr_t)safe_syscall_start
-&& *pcreg < (uintptr_t)safe_syscall_end) {
-*pcreg = (uintptr_t)safe_syscall_start;
-}
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/ppc64/hostdep.h b/linux-user/host/ppc64/hostdep.h
index 98979ad917..0c290dd904 100644
--- a/linux-user/host/ppc64/hostdep.h
+++ b/linux-user/host/ppc64/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* Adjust the signal context to rewind out of safe-syscall if we're in it */
-static inline void rewind_if_in_safe_syscall(void *puc)
-{
-ucontext_t *uc = puc;
-unsigned long *pcreg = >uc_mcontext.gp_regs[PT_NIP];
-
-if (*pcreg > (uintptr_t)safe_syscall_start
-&& *pcreg < (uintptr_t)safe_syscall_end) {
-*pcreg = (uintptr_t)safe_syscall_start;
-}
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/riscv/hostdep.h b/linux-user/host/riscv/hostdep.h
index 2ba07456ae..7f67c22868 100644
--- a/linux-user/host/riscv/hostdep.h
+++ b/linux-user/host/riscv/hostdep.h
@@ -11,24 +11,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* Adjust the signal context to rewind out of
Re: [PATCH 35/35] test/tcg/ppc64le: Add float reference files
On 11/22/21 2:04 PM, Cédric Le Goater wrote: But this alone of course causes other "failures", because we've got some incorrect reference files. Looks fine. Will you send this patch independently ? Yes. The patchset doesn't seem to break anything. You may not have docker cross-compilers enabled. The hexagon reference files have incorrect contents. I'll pursue an update with Taylor. r~
[PATCH for-6.2 1/2] linux-user: Add host_signal_set_pc to set pc in mcontext
From: Warner Losh
Add a new function host_signal_set_pc to set the next pc in an
mcontext. The caller should ensure this is a valid PC for execution.
Signed-off-by: Warner Losh
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <2023045603.60391-2-...@bsdimp.com>
Signed-off-by: Richard Henderson
---
linux-user/host/aarch64/host-signal.h | 5 +
linux-user/host/alpha/host-signal.h | 5 +
linux-user/host/arm/host-signal.h | 5 +
linux-user/host/i386/host-signal.h| 5 +
linux-user/host/mips/host-signal.h| 5 +
linux-user/host/ppc/host-signal.h | 5 +
linux-user/host/riscv/host-signal.h | 5 +
linux-user/host/s390/host-signal.h| 5 +
linux-user/host/sparc/host-signal.h | 9 +
linux-user/host/x86_64/host-signal.h | 5 +
10 files changed, 54 insertions(+)
diff --git a/linux-user/host/aarch64/host-signal.h
b/linux-user/host/aarch64/host-signal.h
index 0c0b08383a..9770b36dc1 100644
--- a/linux-user/host/aarch64/host-signal.h
+++ b/linux-user/host/aarch64/host-signal.h
@@ -35,6 +35,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
struct _aarch64_ctx *hdr;
diff --git a/linux-user/host/alpha/host-signal.h
b/linux-user/host/alpha/host-signal.h
index e080be412f..f4c942948a 100644
--- a/linux-user/host/alpha/host-signal.h
+++ b/linux-user/host/alpha/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.sc_pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.sc_pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
uint32_t *pc = (uint32_t *)host_signal_pc(uc);
diff --git a/linux-user/host/arm/host-signal.h
b/linux-user/host/arm/host-signal.h
index efb165c0c5..6c095773c0 100644
--- a/linux-user/host/arm/host-signal.h
+++ b/linux-user/host/arm/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.arm_pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.arm_pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
/*
diff --git a/linux-user/host/i386/host-signal.h
b/linux-user/host/i386/host-signal.h
index 4c8eef99ce..abe1ece5c9 100644
--- a/linux-user/host/i386/host-signal.h
+++ b/linux-user/host/i386/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.gregs[REG_EIP];
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.gregs[REG_EIP] = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
return uc->uc_mcontext.gregs[REG_TRAPNO] == 0xe
diff --git a/linux-user/host/mips/host-signal.h
b/linux-user/host/mips/host-signal.h
index ef341f7c20..c666ed8c3f 100644
--- a/linux-user/host/mips/host-signal.h
+++ b/linux-user/host/mips/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.pc = pc;
+}
+
#if defined(__misp16) || defined(__mips_micromips)
#error "Unsupported encoding"
#endif
diff --git a/linux-user/host/ppc/host-signal.h
b/linux-user/host/ppc/host-signal.h
index a491c413dc..1d8e658ff7 100644
--- a/linux-user/host/ppc/host-signal.h
+++ b/linux-user/host/ppc/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.regs->nip;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.regs->nip = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
return uc->uc_mcontext.regs->trap != 0x400
diff --git a/linux-user/host/riscv/host-signal.h
b/linux-user/host/riscv/host-signal.h
index 3b168cb58b..a4f170efb0 100644
--- a/linux-user/host/riscv/host-signal.h
+++ b/linux-user/host/riscv/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.__gregs[REG_PC];
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+uc->uc_mcontext.__gregs[REG_PC] = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
/*
diff --git a/linux-user/host/s390/host-signal.h
b/linux-user/host/s390/host-signal.h
index 26990e4893..a524f2ab00 100644
--- a/linux-user/host/s390/host-signal.h
+++ b/linux-user/host/s390/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.psw.addr;
}
+static inline void
[PATCH for-6.2 0/2] linux-user: Create a common rewind_if_in_safe_syscall
This is a re-packaging of two of Warner's patches that fix a build issue on aarch64 using musl: https://lore.kernel.org/qemu-devel/20211108194230.1836262-1-raj.k...@gmail.com/ r~ Warner Losh (2): linux-user: Add host_signal_set_pc to set pc in mcontext linux-user/signal.c: Create a common rewind_if_in_safe_syscall linux-user/host/aarch64/host-signal.h | 5 + linux-user/host/aarch64/hostdep.h | 20 linux-user/host/alpha/host-signal.h | 5 + linux-user/host/arm/host-signal.h | 5 + linux-user/host/arm/hostdep.h | 20 linux-user/host/i386/host-signal.h| 5 + linux-user/host/i386/hostdep.h| 20 linux-user/host/mips/host-signal.h| 5 + linux-user/host/ppc/host-signal.h | 5 + linux-user/host/ppc64/hostdep.h | 20 linux-user/host/riscv/host-signal.h | 5 + linux-user/host/riscv/hostdep.h | 20 linux-user/host/s390/host-signal.h| 5 + linux-user/host/s390x/hostdep.h | 20 linux-user/host/sparc/host-signal.h | 9 + linux-user/host/x86_64/host-signal.h | 5 + linux-user/host/x86_64/hostdep.h | 20 linux-user/safe-syscall.h | 3 +++ linux-user/signal.c | 15 --- 19 files changed, 69 insertions(+), 143 deletions(-) -- 2.25.1
