Re: [Qemu-devel] [PATCH] qemu-img.c: add help for each command
> On Sep 7, 2018, at 11:13 PM, Peter Maydell wrote:
>
> On 8 September 2018 at 04:01, John Arbuckle wrote:
>
>> +/* print the help for this command */
>> +if (strcmp("--help", argv[optind + 1]) == 0) {
>> +if (strcmp("amend", cmdname) == 0) {
>> +help_amend();
>> +} else if (strcmp("bench", cmdname) == 0) {
>> +help_bench();
>> +} else if (strcmp("check", cmdname) == 0) {
>> +help_check();
>> +} else if (strcmp("commit", cmdname) == 0) {
>> +help_commit();
>> +} else if (strcmp("compare", cmdname) == 0) {
>> +help_compare();
>> +} else if (strcmp("convert", cmdname) == 0) {
>> +help_convert();
>> +} else if (strcmp("create", cmdname) == 0) {
>> +help_create();
>> +} else if (strcmp("dd", cmdname) == 0) {
>> +help_dd();
>> +} else if (strcmp("info", cmdname) == 0) {
>> +help_info();
>> +} else if (strcmp("map", cmdname) == 0) {
>> +help_map();
>> +} else if (strcmp("measure", cmdname) == 0) {
>> +help_measure();
>> +} else if (strcmp("snapshot", cmdname) == 0) {
>> +help_snapshot();
>> +} else if (strcmp("rebase", cmdname) == 0) {
>> +help_rebase();
>> +} else if (strcmp("resize", cmdname) == 0) {
>> +help_resize();
>
> Any time you find yourself writing very repetitive code like
> this, it's a good idea to ask yourself "is there a way to make
> this data-driven?".
>
> thanks
> -- PMM
Did you want me to loop thru an array of strings instead?
Re: [Qemu-devel] [PATCH] qemu-img.c: add help for each command
On 8 September 2018 at 04:01, John Arbuckle wrote:
> +/* print the help for this command */
> +if (strcmp("--help", argv[optind + 1]) == 0) {
> +if (strcmp("amend", cmdname) == 0) {
> +help_amend();
> +} else if (strcmp("bench", cmdname) == 0) {
> +help_bench();
> +} else if (strcmp("check", cmdname) == 0) {
> +help_check();
> +} else if (strcmp("commit", cmdname) == 0) {
> +help_commit();
> +} else if (strcmp("compare", cmdname) == 0) {
> +help_compare();
> +} else if (strcmp("convert", cmdname) == 0) {
> +help_convert();
> +} else if (strcmp("create", cmdname) == 0) {
> +help_create();
> +} else if (strcmp("dd", cmdname) == 0) {
> +help_dd();
> +} else if (strcmp("info", cmdname) == 0) {
> +help_info();
> +} else if (strcmp("map", cmdname) == 0) {
> +help_map();
> +} else if (strcmp("measure", cmdname) == 0) {
> +help_measure();
> +} else if (strcmp("snapshot", cmdname) == 0) {
> +help_snapshot();
> +} else if (strcmp("rebase", cmdname) == 0) {
> +help_rebase();
> +} else if (strcmp("resize", cmdname) == 0) {
> +help_resize();
Any time you find yourself writing very repetitive code like
this, it's a good idea to ask yourself "is there a way to make
this data-driven?".
thanks
-- PMM
[Qemu-devel] [PATCH] qemu-img.c: add help for each command
Add the ability for the user to display help for a certain command.
Example: qemu-img create --help
What is printed is all the options available to this command and an example.
Signed-off-by: John Arbuckle
---
qemu-img.c | 767 +
1 file changed, 668 insertions(+), 99 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index b12f4cd..5eb38b8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -76,11 +76,6 @@ typedef enum OutputFormat {
/* Default to cache=writeback as data integrity is not important for qemu-img
*/
#define BDRV_DEFAULT_CACHE "writeback"
-static void format_print(void *opaque, const char *name)
-{
-printf(" %s", name);
-}
-
static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
{
va_list ap;
@@ -105,102 +100,639 @@ static void QEMU_NORETURN unrecognized_option(const
char *option)
error_exit("unrecognized option '%s'", option);
}
-/* Please keep in synch with qemu-img.texi */
-static void QEMU_NORETURN help(void)
+/* Prints an overview of available help options */
+static void help(void)
{
const char *help_msg =
- QEMU_IMG_VERSION
- "usage: qemu-img [standard options] command [command options]\n"
- "QEMU disk image utility\n"
- "\n"
- "'-h', '--help' display this help and exit\n"
- "'-V', '--version'output version information and exit\n"
- "'-T', '--trace'
[[enable=]][,events=][,file=]\n"
- " specify tracing options\n"
- "\n"
- "Command syntax:\n"
-#define DEF(option, callback, arg_string)\
- " " arg_string "\n"
-#include "qemu-img-cmds.h"
-#undef DEF
- "\n"
- "Command parameters:\n"
- " 'filename' is a disk image filename\n"
- " 'objectdef' is a QEMU user creatable object definition. See the
qemu(1)\n"
- "manual page for a description of the object properties. The
most common\n"
- "object type is a 'secret', which is used to supply passwords
and/or\n"
- "encryption keys.\n"
- " 'fmt' is the disk image format. It is guessed automatically in
most cases\n"
- " 'cache' is the cache mode used to write the output disk image,
the valid\n"
- "options are: 'none', 'writeback' (default, except for
convert), 'writethrough',\n"
- "'directsync' and 'unsafe' (default for convert)\n"
- " 'src_cache' is the cache mode used to read input disk images,
the valid\n"
- "options are the same as for the 'cache' option\n"
- " 'size' is the disk image size in bytes. Optional suffixes\n"
- "'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G'
(gigabyte, 1024M),\n"
- "'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte,
1024P) are\n"
- "supported. 'b' is ignored.\n"
- " 'output_filename' is the destination disk image filename\n"
- " 'output_fmt' is the destination format\n"
- " 'options' is a comma separated list of format specific options
in a\n"
- "name=value format. Use -o ? for an overview of the options
supported by the\n"
- "used format\n"
- " 'snapshot_param' is param used for internal snapshot, format\n"
- "is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
- "'[ID_OR_NAME]'\n"
- " '-c' indicates that target image must be compressed (qcow format
only)\n"
- " '-u' allows unsafe backing chains. For rebasing, it is assumed
that old and\n"
- " new backing file match exactly. The image doesn't need a
working\n"
- " backing file before rebasing in this case (useful for
renaming the\n"
- " backing file). For image creation, allow creating without
attempting\n"
- " to open the backing file.\n"
- " '-h' with or without a command shows this help and lists the
supported formats\n"
- " '-p' show progress of command (only certain commands)\n"
- " '-q' use Quiet mode - do not print any output (except errors)\n"
- " '-S' indicates the consecutive number of bytes (defaults to 4k)
that must\n"
- " contain only zeros for qemu-img to create a sparse image
during\n"
- " conversion. If the number of bytes is 0, the source will
not be scanned for\n"
- " unallocated or zero sectors, and the destination image will
always be\n"
- " fully allocated\n"
- " '--output' takes the format in which the output must be done
(human or json)\n"
- " '-n' skips the target volume creation (useful if the volume is
created\n"
- " prior to running qemu-img)\n"
- "\n"
- "Parameters to
Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
On 7 September 2018 at 15:39, Alex Bennée wrote: > > Pavel Dovgalyuk writes: > >> The following series implements dynamic binary instrumentation upon >> QEMU. > > OK I've done a pass through the patches, final comments bellow. > > >> >> For the current patches the plugins should provide the following >> callbacks: >> - "needs" callback to check whether the specific instruction >>should be instrumented by this plugin >> - "run" callback which called before executing the instuction > > I think with a little re-jigging we could reduce this to a single run > call which can then decide if it is going to deal with things in this > case. > >> Our instrumentation subsystem exploits TCG helper mechanism to embed >> callbacks into the translation blocks. These callbacks may be inserted >> before the specific instructions. >> >> The aim of submission of this series at that early stage is to get >> the feedback which will guide the development process. We are faced >> the following questions: >> 1. Does every plugins should have its own callback embedded into the TB >> (which will cause TB extra growth in case of multiple plugins), >> or the instrumentation layer's callback should invoke the plugins >> that wanted to instrument that specific instruction? > > The common case is having one plugin and I'd rather give the flexibility > of what helper to call and how much information to give it to the plugin > and deal with the potential to have multiple helper calls per > instruction for the complex cases. > > Now I can see arguments against it from an interface complexity point of > view but I think plugins should get access to the TCG functions so they > can generate their own op sequences if need be and not have to call a > helper at all if they don't need to. I strongly disagree -- plugins should not have access to details of QEMU internals that might change from version to version, and definitely not access to generating their own TCG code. thanks -- PMM
Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900
Hi, On 7/7/18 4:41 PM, Fredrik Noring wrote: [...] > --- a/target/mips/mips-defs.h > +++ b/target/mips/mips-defs.h > @@ -52,6 +52,7 @@ > #define ASE_MSA 0x0100 > > /* Chip specific instructions. */ > +#define INSN_R5900 0x1000 We have 4 bits to store the chip-specific instructions and this is the last bit available. Any objection to upraise CPUMIPSState.insn_flags to a uint64_t? I'd then shift the chip-specific bits to the 32 upper bits, to keep this flag clean. > #define INSN_LOONGSON2E 0x2000 > #define INSN_LOONGSON2F 0x4000 > #define INSN_VR54XX 0x8000 > @@ -62,6 +63,7 @@ > #define CPU_MIPS3 (CPU_MIPS2 | ISA_MIPS3) > #define CPU_MIPS4 (CPU_MIPS3 | ISA_MIPS4) > #define CPU_VR54XX (CPU_MIPS4 | INSN_VR54XX) > +#define CPU_R5900 (CPU_MIPS4 | INSN_R5900) > #define CPU_LOONGSON2E (CPU_MIPS3 | INSN_LOONGSON2E) > #define CPU_LOONGSON2F (CPU_MIPS3 | INSN_LOONGSON2F) [...] signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-arm] [PATCH 02/15] gdbstub: add multiprocess support to '?' packets
Hi Luc,
On 9/1/18 9:46 AM, Luc Michel wrote:
[...]
> +static char *get_thread_id(const GDBState *s, CPUState *cpu,
> + char *buf, size_t buf_size)
To avoid confusion with 'int qemu_get_thread_id()' from "qemu/osdep.h",
can we use another name such gdb_fmt_thread_id() or
get_thread_id_string() or better?
> +{
> +if (s->multiprocess) {
> +snprintf(buf, buf_size, "p%02x.%02x",
> + gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
> +} else {
> +snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
> +
extra newline ;)
> +}
> +
> +return buf;
> +}
Re: [Qemu-devel] [Qemu-arm] [PATCH 09/15] gdbstub: add multiprocess support to 'D' packets
Hi Luc,
On 9/1/18 9:46 AM, Luc Michel wrote:
> 'D' packets are used by GDB to detach from a process. In multiprocess
> mode, the PID to detach from is sent in the request.
>
> Signed-off-by: Luc Michel
> ---
> gdbstub.c | 55 ---
> 1 file changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 4ccd1153ce..af8864e251 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1025,24 +1025,39 @@ static int gdb_breakpoint_remove(target_ulong addr,
> target_ulong len, int type)
> default:
> return -ENOSYS;
> }
> }
>
> +static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
> +{
> +cpu_breakpoint_remove_all(cpu, BP_GDB);
> +#ifndef CONFIG_USER_ONLY
> +cpu_watchpoint_remove_all(cpu, BP_GDB);
> +#endif
> +}
> +
> +static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess
> *p)
> +{
> +CPUState *cpu = get_first_cpu_in_process(s, p);
> +
> +while (cpu) {
> +gdb_cpu_breakpoint_remove_all(cpu);
> +cpu = gdb_next_cpu_in_process(s, cpu);
> +}
> +}
> +
> static void gdb_breakpoint_remove_all(void)
> {
> CPUState *cpu;
>
> if (kvm_enabled()) {
> kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
> return;
> }
>
> CPU_FOREACH(cpu) {
> -cpu_breakpoint_remove_all(cpu, BP_GDB);
> -#ifndef CONFIG_USER_ONLY
> -cpu_watchpoint_remove_all(cpu, BP_GDB);
> -#endif
> +gdb_cpu_breakpoint_remove_all(cpu);
> }
> }
>
> static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
> {
> @@ -1318,13 +1333,39 @@ static int gdb_handle_packet(GDBState *s, const char
> *line_buf)
> /* Kill the target */
> error_report("QEMU: Terminated via GDBstub");
> exit(0);
> case 'D':
> /* Detach packet */
> -gdb_breakpoint_remove_all();
> -gdb_syscall_mode = GDB_SYS_DISABLED;
> -gdb_continue(s);
> +pid = 1;
> +
> +if (s->multiprocess) {
> +unsigned long lpid;
> +if (*p != ';') {
> +put_packet(s, "E22");
> +break;
> +}
> +qemu_strtoul(p + 1, , 16, );
You forgot to check for EINVAL/ERANGE.
The rest is OK, so once fixed:
Reviewed-by: Philippe Mathieu-Daudé
> +pid = lpid;
> +}
> +
> +process = gdb_get_process(s, pid);
> +gdb_process_breakpoint_remove_all(s, process);
> +process->attached = false;
> +
> +if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
> +s->c_cpu = gdb_first_cpu(s);
> +}
> +
> +if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> +s->g_cpu = gdb_first_cpu(s);
> +}
> +
> +if (s->c_cpu == NULL) {
> +/* No more process attached */
> +gdb_syscall_mode = GDB_SYS_DISABLED;
> +gdb_continue(s);
> +}
> put_packet(s, "OK");
> break;
> case 's':
> if (*p != '\0') {
> addr = strtoull(p, (char **), 16);
>
Re: [Qemu-devel] [PATCH v5 00/16] jobs: Job Exit Refactoring Pt 2
On 2018-09-06 15:02, John Snow wrote: > This is part two of a two part series that refactors the exit logic > of jobs. > > This series forces all jobs to use the "finalize" semantics that were > introduced previously, but only exposed via the backup jobs. > > Patches 1-3 add plumbing for the auto-dismiss and auto-finalize flags > but do not expose these via QAPI/QMP. > > Patches 4-7 refactor the .exit() callbacks into the component pieces > of .prepare(), .commit(), .abort() and .clean(). Except mirror, which > I cheat with. > > Patches 8-10 remove the last usages of .exit in a test. > Patche 11 removes the .exit callback and the machinery to invoke it. > > Patches 12-14 expose the new QMP options to all of the jobs. > Patch 15 is a doc fixup. Thanks, fixed the capitalization in patch 4's commit message, added the mirror_exit() comment you proposed in patch 6, and applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-arm] [PATCH 05/15] gdbstub: add multiprocess support to 'sC' packets
On 9/1/18 9:46 AM, Luc Michel wrote:
> Change the sC packet handling to support the multiprocess extension.
> Instead of returning the first thread, we return the first thread of the
> current process.
>
> Signed-off-by: Luc Michel
Reviewed-by: Philippe Mathieu-Daudé
> ---
> gdbstub.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 2cb9cb6d97..c15250ce65 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1531,13 +1531,18 @@ static int gdb_handle_packet(GDBState *s, const char
> *line_buf)
> type = strtoul(p, (char **), 16);
> sstep_flags = type;
> put_packet(s, "OK");
> break;
> } else if (strcmp(p,"C") == 0) {
> -/* "Current thread" remains vague in the spec, so always return
> - * the first CPU (gdb returns the first thread). */
> -put_packet(s, "QC1");
> +/* "Current thread" remains vague in the spec, so always return
> the
> + * first thread of the current process (gdb returns the first
> + * thread).
> + */
> +cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s,
> s->g_cpu));
> +snprintf(buf, sizeof(buf), "QC%s",
> + get_thread_id(s, cpu, thread_id, sizeof(thread_id)));
> +put_packet(s, buf);
> break;
> } else if (strcmp(p,"fThreadInfo") == 0) {
> s->query_cpu = first_cpu;
> goto report_cpuinfo;
> } else if (strcmp(p,"sThreadInfo") == 0) {
>
Re: [Qemu-devel] [PATCH v5 16/16] blockdev: document transactional shortcomings
On 2018-09-06 15:02, John Snow wrote: > Presently only the backup job really guarantees what one would consider > transactional semantics. To guard against someone helpfully adding them > in the future, document that there are shortcomings in the model that > would need to be audited at that time. > > Signed-off-by: John Snow > --- > blockdev.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 06/16] block/mirror: conservative mirror_exit refactor
On 2018-09-06 22:31, John Snow wrote:
>
>
> On 09/06/2018 12:57 PM, Jeff Cody wrote:
>> On Thu, Sep 06, 2018 at 09:02:15AM -0400, John Snow wrote:
>>> For purposes of minimum code movement, refactor the mirror_exit
>>> callback to use the post-finalization callbacks in a trivial way.
>>>
>>> Signed-off-by: John Snow
>>> ---
>>> block/mirror.c | 39 ---
>>> 1 file changed, 28 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/block/mirror.c b/block/mirror.c
>>> index bd3e908710..a92b4702c5 100644
>>> --- a/block/mirror.c
>>> +++ b/block/mirror.c
>>> @@ -79,6 +79,7 @@ typedef struct MirrorBlockJob {
>>> int max_iov;
>>> bool initial_zeroing_ongoing;
>>> int in_active_write_counter;
>>> +bool prepared;
>>> } MirrorBlockJob;
>>>
>>> typedef struct MirrorBDSOpaque {
>>> @@ -607,7 +608,7 @@ static void mirror_wait_for_all_io(MirrorBlockJob *s)
>>> }
>>> }
>>>
>>> -static void mirror_exit(Job *job)
>
> /**
> * mirror_exit_common: handle both abort() and prepare() cases.
> * for .prepare, returns 0 on success and -errno on failure.
> * for .abort cases, denoted by abort = true, MUST return 0.
> */
Any case:
Reviewed-by: Max Reitz
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 05/16] block/mirror: don't install backing chain on abort
On 2018-09-06 15:02, John Snow wrote: > In cases where we abort the block/mirror job, there's no point in > installing the new backing chain before we finish aborting. > > Signed-off-by: John Snow > --- > block/mirror.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 04/16] block/commit: refactor commit to use job callbacks
On 2018-09-06 15:02, John Snow wrote: > Use the component callbacks; prepare, abort, and clean. > > NB: prepare is only called when the job has not yet failed; > and abort can be called after prepare. > > complete -> prepare -> abort -> clean > complete -> abort -> clean > > During refactor, a potential problem with bdrv_drop_intermediate > was identified, The patched behavior is no worse than the pre-patch (s/T/t/) (Max) > behavior, so leave a FIXME for now to be fixed in a future patch. > > Signed-off-by: John Snow > Reviewed-by: Max Reitz > --- > block/commit.c | 92 > -- > 1 file changed, 51 insertions(+), 41 deletions(-) signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-arm] [PATCH 03/11] hw/arm/aspeed: Add an Aspeed machine class
On 8/31/18 7:38 AM, Cédric Le Goater wrote:
> The code looks better, it removes duplicated lines and it will ease
> the introduction of common properties for the Aspeed machines.
>
> Signed-off-by: Cédric Le Goater
Nice cleanup :)
Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/hw/arm/aspeed.h | 46 +
> hw/arm/aspeed.c | 212 +---
> 2 files changed, 116 insertions(+), 142 deletions(-)
> create mode 100644 include/hw/arm/aspeed.h
>
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> new file mode 100644
> index ..2b77f8d2b3c8
> --- /dev/null
> +++ b/include/hw/arm/aspeed.h
> @@ -0,0 +1,46 @@
> +/*
> + * Aspeed Machines
> + *
> + * Copyright 2018 IBM Corp.
> + *
> + * This code is licensed under the GPL version 2 or later. See
> + * the COPYING file in the top-level directory.
> + */
> +#ifndef _ARM_ASPEED_H
> +#define _ARM_ASPEED_H
> +
> +#include "hw/boards.h"
> +
> +typedef struct AspeedBoardState AspeedBoardState;
> +
> +typedef struct AspeedBoardConfig {
> +const char *name;
> +const char *desc;
> +const char *soc_name;
> +uint32_t hw_strap1;
> +const char *fmc_model;
> +const char *spi_model;
> +uint32_t num_cs;
> +void (*i2c_init)(AspeedBoardState *bmc);
> +} AspeedBoardConfig;
> +
> +#define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")
> +#define ASPEED_MACHINE(obj) \
> +OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE)
> +
> +typedef struct AspeedMachine {
> +MachineState parent_obj;
> +} AspeedMachine;
> +
> +#define ASPEED_MACHINE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(AspeedMachineClass, (klass), TYPE_ASPEED_MACHINE)
> +#define ASPEED_MACHINE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(AspeedMachineClass, (obj), TYPE_ASPEED_MACHINE)
> +
> +typedef struct AspeedMachineClass {
> +MachineClass parent_obj;
> +const AspeedBoardConfig *board;
> +} AspeedMachineClass;
> +
> +
> +#endif
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index f2d64e45511a..6b33ecd5aa43 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -15,6 +15,7 @@
> #include "cpu.h"
> #include "exec/address-spaces.h"
> #include "hw/arm/arm.h"
> +#include "hw/arm/aspeed.h"
> #include "hw/arm/aspeed_soc.h"
> #include "hw/boards.h"
> #include "hw/i2c/smbus.h"
> @@ -34,22 +35,6 @@ typedef struct AspeedBoardState {
> MemoryRegion max_ram;
> } AspeedBoardState;
>
> -typedef struct AspeedBoardConfig {
> -const char *soc_name;
> -uint32_t hw_strap1;
> -const char *fmc_model;
> -const char *spi_model;
> -uint32_t num_cs;
> -void (*i2c_init)(AspeedBoardState *bmc);
> -} AspeedBoardConfig;
> -
> -enum {
> -PALMETTO_BMC,
> -AST2500_EVB,
> -ROMULUS_BMC,
> -WITHERSPOON_BMC,
> -};
> -
> /* Palmetto hardware value: 0x120CE416 */
> #define PALMETTO_BMC_HW_STRAP1 (\
> SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) | \
> @@ -88,46 +73,6 @@ enum {
> /* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */
> #define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1
>
> -static void palmetto_bmc_i2c_init(AspeedBoardState *bmc);
> -static void ast2500_evb_i2c_init(AspeedBoardState *bmc);
> -static void romulus_bmc_i2c_init(AspeedBoardState *bmc);
> -static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc);
> -
> -static const AspeedBoardConfig aspeed_boards[] = {
> -[PALMETTO_BMC] = {
> -.soc_name = "ast2400-a1",
> -.hw_strap1 = PALMETTO_BMC_HW_STRAP1,
> -.fmc_model = "n25q256a",
> -.spi_model = "mx25l25635e",
> -.num_cs= 1,
> -.i2c_init = palmetto_bmc_i2c_init,
> -},
> -[AST2500_EVB] = {
> -.soc_name = "ast2500-a1",
> -.hw_strap1 = AST2500_EVB_HW_STRAP1,
> -.fmc_model = "w25q256",
> -.spi_model = "mx25l25635e",
> -.num_cs= 1,
> -.i2c_init = ast2500_evb_i2c_init,
> -},
> -[ROMULUS_BMC] = {
> -.soc_name = "ast2500-a1",
> -.hw_strap1 = ROMULUS_BMC_HW_STRAP1,
> -.fmc_model = "n25q256a",
> -.spi_model = "mx66l1g45g",
> -.num_cs= 2,
> -.i2c_init = romulus_bmc_i2c_init,
> -},
> -[WITHERSPOON_BMC] = {
> -.soc_name = "ast2500-a1",
> -.hw_strap1 = WITHERSPOON_BMC_HW_STRAP1,
> -.fmc_model = "mx25l25635e",
> -.spi_model = "mx66l1g45g",
> -.num_cs= 2,
> -.i2c_init = witherspoon_bmc_i2c_init,
> -},
> -};
> -
> /*
> * The max ram region is for firmwares that scan the address space
> * with load/store to guess how much RAM the SoC has.
> @@ -313,30 +258,6 @@ static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
> object_property_set_int(OBJECT(dev), 11, "temperature3",
> _abort);
> }
>
> -static void palmetto_bmc_init(MachineState *machine)
> -{
> -aspeed_board_init(machine,
Re: [Qemu-devel] [PATCH 06/11] aspeed/smc: fix default read value
Hi Cédric,
On 8/31/18 7:38 AM, Cédric Le Goater wrote:
> 0x should be returned for non implemented registers.
>
> Signed-off-by: Cédric Le Goater
> ---
> hw/ssi/aspeed_smc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 1270842dcf0c..6045ca11b969 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -665,12 +665,12 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr
> addr, unsigned int size)
> addr == s->r_ce_ctrl ||
> addr == R_INTR_CTRL ||
> (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
> -(addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
> +(addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) {
This change seems unrelated to this commit.
> return s->regs[addr];
> } else {
> qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>__func__, addr);
> -return 0;
> +return -1;
> }
> }
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH 11/11] aspeed/smc: Add dummy data register
On 8/31/18 8:15 AM, Cédric Le Goater wrote:
> The SMC controllers have a register containing the byte that will be
> used as dummy output. It can be modified by software.
>
> Signed-off-by: Cédric Le Goater
> ---
> hw/ssi/aspeed_smc.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index da2fedfcd3cd..f31bbc895caa 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -102,8 +102,8 @@
> /* Misc Control Register #1 */
> #define R_MISC_CTRL1 (0x50 / 4)
>
> -/* Misc Control Register #2 */
> -#define R_MISC_CTRL2 (0x54 / 4)
> +/* SPI dummy cycle data */
> +#define R_DUMMY_DATA (0x54 / 4)
>
> /* DMA Control/Status Register */
> #define R_DMA_CTRL(0x80 / 4)
> @@ -548,7 +548,7 @@ static void aspeed_smc_flash_setup(AspeedSMCFlash *fl,
> uint32_t addr)
> */
> if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
> for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
> -ssi_transfer(fl->controller->spi, 0xFF);
> +ssi_transfer(fl->controller->spi, s->regs[R_DUMMY_DATA] & 0xff);
The DUMMY_DATA register always contains a 8-bit value, so this AND
shouldn't be necessary. Regardless:
Reviewed-by: Philippe Mathieu-Daudé
> }
> }
> }
> @@ -680,6 +680,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr
> addr, unsigned int size)
> addr == s->r_timings ||
> addr == s->r_ce_ctrl ||
> addr == R_INTR_CTRL ||
> +addr == R_DUMMY_DATA ||
> (s->ctrl->has_dma && addr == R_DMA_CTRL) ||
> (s->ctrl->has_dma && addr == R_DMA_FLASH_ADDR) ||
> (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) ||
> @@ -912,6 +913,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr,
> uint64_t data,
> }
> } else if (addr == R_INTR_CTRL) {
> s->regs[addr] = value;
> +} else if (addr == R_DUMMY_DATA) {
> +s->regs[addr] = value & 0xff ;
> } else if (s->ctrl->has_dma && addr == R_DMA_CTRL) {
> aspeed_smc_dma_ctrl(s, value);
> } else if (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) {
>
[Qemu-devel] [PATCH v2 1/3] usb-mtp: fix error conditions for write operation
From: Bandan
Return STORE_FULL if we can't write all the bytes but
return incomplete transfer if data received is less then
what was specified in the metadata. Also, use d->offset
as the file size which is valid for all file sizes.
Signed-off-by: Bandan
---
hw/usb/dev-mtp.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 3fdc4b0da1..15edf3bb82 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1665,13 +1665,14 @@ static void usb_mtp_write_data(MTPState *s)
goto success;
}
-rc = write_retry(d->fd, d->data, s->dataset.size);
-if (!rc) {
+rc = write_retry(d->fd, d->data, d->offset);
+if (rc != d->offset) {
usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
0, 0, 0, 0);
goto done;
}
-if (rc != s->dataset.size) {
+/* Only for < 4G file sizes */
+if (s->dataset.size != 0x && rc != s->dataset.size) {
usb_mtp_queue_result(s, RES_INCOMPLETE_TRANSFER, d->trans,
0, 0, 0, 0);
goto done;
--
2.14.4
[Qemu-devel] [PATCH v2 3/3] usb-mtp: reset ObjectInfo dataset size on cleanup
Stale values in this field may result in qemu
expecting more data on the next operation
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 15edf3bb82..00a3691bae 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1568,6 +1568,7 @@ static void usb_mtp_handle_control(USBDevice *dev,
USBPacket *p,
if (s->write_pending) {
g_free(s->dataset.filename);
s->write_pending = false;
+s->dataset.size = 0;
}
usb_mtp_data_free(s->data_out);
s->data_out = NULL;
@@ -1693,6 +1694,7 @@ done:
}
free:
g_free(s->dataset.filename);
+s->dataset.size = 0;
g_free(path);
s->write_pending = false;
}
--
2.14.4
[Qemu-devel] [PATCH v2 2/3] doc: replace x-root with rootdir for usb-mtp
From: Bandan
Signed-off-by: Bandan
---
qemu-doc.texi | 2 +-
scripts/device-crash-test | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f74542a0e9..cc7d81181c 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -943,7 +943,7 @@ for details
Bulk-only transport storage device, see
@url{https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/usb-storage.txt,usb-storage.txt}
for details here, too
-@item usb-mtp,x-root=@var{dir}
+@item usb-mtp,rootdir=@var{dir}
Media transfer protocol device, using @var{dir} as root of the file tree
that is presented to the guest.
@item usb-host,hostbus=@var{bus},hostaddr=@var{addr}
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index e6c233e9bf..7045594bd4 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -125,7 +125,7 @@ ERROR_WHITELIST = [
{'device':'tpm-tis', 'expected':True}, # tpm_tis: backend
driver with id (null) could not be found
{'device':'unimplemented-device', 'expected':True},# property 'size'
not specified or zero
{'device':'usb-braille', 'expected':True}, # Property chardev
is required
-{'device':'usb-mtp', 'expected':True}, # x-root property
must be configured
+{'device':'usb-mtp', 'expected':True}, # rootdir property
must be configured
{'device':'usb-redir', 'expected':True}, # Parameter
'chardev' is missing
{'device':'usb-serial', 'expected':True}, # Property chardev
is required
{'device':'usb-storage', 'expected':True}, # drive property
not set
--
2.14.4
[Qemu-devel] [PATCH 0/2] softfloat tests based on berkeley's testfloat
Alex was not too pleased with my previous fp-test incarnation;
it only tested some 32 and 64 primitives, and it relied on test
files by IBM that had no particular license (so we could not
redistribute them, and IBM might pull them off the web any time).
This attempt leverages berkeley's softfloat and testfloat, which
are BSD-licensed and therefore we can pull in as submodules.
Not everything that berkeley's softfloat does is implemented in
QEMU (and vice versa), but the coverage of our implementation with
this test is quite high.
Regarding regressions, it shows at least one regression since
the recent FP refactor, e.g. with -all2 -errors 1:
Testing f64_div, rounding near_even.
46464 tests total.
Errors found in f64_div, rounding near_even:
+7FE.FFEFFBFFE +480.007FFEFFE
=> +77D.FEEFFE00B x expected +77D.FEEFFE00A x
15469 tests performed; 1 error found.
If I bring back the code before cf07323d49 ("fpu/softfloat:
re-factor div", 2018-02-21), that error goes away.
Many errors are reported for both 80-bit and 128-bit implementations.
I don't know whether those have always been there or not,
or perhaps I'm doing something wrong when converting
80-bit and 128-bit floats between our implementation and
softfloat.
I'm appending to this message the output of two runs (-all1 and -all2);
-errors 1 means "stop testing this particular op when 1 error is found".
You can pull this tree from:
https://github.com/cota/qemu/tree/fp-test
Bear in mind that you'll need to initialize the submodules.
BTW checkpatch points to some errors, but they're false positives.
Thanks,
Emilio
---
$ ./fp-test -all1 -errors 1
Testing ui32_to_f16, rounding near_even.
372 tests total.
372 tests performed.
In 372 tests, no errors found in ui32_to_f16, rounding near_even.
Testing ui32_to_f32, rounding near_even.
372 tests total.
372 tests performed.
In 372 tests, no errors found in ui32_to_f32, rounding near_even.
Testing ui32_to_f64.
372 tests total.
372 tests performed.
In 372 tests, no errors found in ui32_to_f64.
Testing ui32_to_extF80.
Testing ui32_to_f128.
Testing ui64_to_f16, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in ui64_to_f16, rounding near_even.
Testing ui64_to_f32, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in ui64_to_f32, rounding near_even.
Testing ui64_to_f64, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in ui64_to_f64, rounding near_even.
Testing ui64_to_extF80.
Testing ui64_to_f128.
756 tests total.
Errors found in ui64_to_f128:
F8003FFFDFFE => -3FFC.403EF0007FFF .
expected +403E.F0007FFFBFFC .
1 tests performed; 1 error found.
Testing i32_to_f16, rounding near_even.
372 tests total.
372 tests performed.
In 372 tests, no errors found in i32_to_f16, rounding near_even.
Testing i32_to_f32, rounding near_even.
372 tests total.
372 tests performed.
In 372 tests, no errors found in i32_to_f32, rounding near_even.
Testing i32_to_f64.
372 tests total.
372 tests performed.
In 372 tests, no errors found in i32_to_f64.
Testing i32_to_extF80.
372 tests total.
Errors found in i32_to_extF80:
011E
=> +.4017 . expected +4017.8F00 .
1 tests performed; 1 error found.
Testing i32_to_f128.
372 tests total.
Errors found in i32_to_f128:
3EFFDFFE => +.401CF7FEFFF0 .
expected +401C.F7FEFFF0 .
1 tests performed; 1 error found.
Testing i64_to_f16, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in i64_to_f16, rounding near_even.
Testing i64_to_f32, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in i64_to_f32, rounding near_even.
Testing i64_to_f64, rounding near_even.
756 tests total.
756 tests performed.
In 756 tests, no errors found in i64_to_f64, rounding near_even.
Testing i64_to_extF80.
756 tests total.
Errors found in i64_to_extF80:
00081FFE
=> +.4022 . expected +4022.81FFE000 .
1 tests performed; 1 error found.
Testing i64_to_f128.
756 tests total.
Errors found in i64_to_f128:
FF7FFF0001FE => +0400.C0360001FFFC .
expected -4036.0001FFFC0400 .
1 tests performed; 1 error found.
Testing f16_to_ui32, rounding near_even, exact.
408 tests total.
408 tests performed.
In 408 tests, no errors found in f16_to_ui32, rounding near_even, exact.
Testing f16_to_ui32, rounding minMag, exact.
408 tests total.
408 tests performed.
In 408 tests, no errors found in f16_to_ui32, rounding minMag, exact.
Testing f16_to_ui32, rounding min, exact.
408 tests total.
408 tests performed.
In 408 tests, no errors found in f16_to_ui32, rounding min, exact.
Testing f16_to_ui32, rounding max, exact.
408
[Qemu-devel] [PATCH v2 0/3] Misc usb-mtp fixes
v2: Same as v1 but with another minor cleanup patch. The write buffer breakup is still WIP. A documentation fix and changes to return the right error code on write failures. Bandan (2): usb-mtp: fix error conditions for write operation doc: replace x-root with rootdir for usb-mtp Bandan Das (1): usb-mtp: reset ObjectInfo dataset size on cleanup hw/usb/dev-mtp.c | 9 ++--- qemu-doc.texi | 2 +- scripts/device-crash-test | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) -- 2.14.4
[Qemu-devel] [PATCH 2/2] tests: add floating point tests
By leveraging berkeley's softfloat and testfloat. fp-test.c is derived from testfloat's testsoftfloat.c. To ease the tracking of upstream changes to the latter file, fp-test.c keeps the original camel-case variable naming, and includes most new code via wrap.inc.c. Most changes to the original code are simple style changes, although a few hacks have been added because our implementation does not provide exactly the same features as softfloat. I've noted these hacks with XXX comments. Signed-off-by: Emilio G. Cota --- configure |2 + tests/fp/platform.h| 41 ++ tests/fp/fp-test.c | 1052 tests/fp/wrap.inc.c| 599 +++ tests/Makefile.include |3 + tests/fp/.gitignore|1 + tests/fp/Makefile | 589 ++ 7 files changed, 2287 insertions(+) create mode 100644 tests/fp/platform.h create mode 100644 tests/fp/fp-test.c create mode 100644 tests/fp/wrap.inc.c create mode 100644 tests/fp/.gitignore create mode 100644 tests/fp/Makefile diff --git a/configure b/configure index 58862d2ae8..b02da8c0b4 100755 --- a/configure +++ b/configure @@ -7451,12 +7451,14 @@ fi # build tree in object directory in case the source is not in the current directory DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm" +DIRS="$DIRS tests/fp" DIRS="$DIRS docs docs/interop fsdev scsi" DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw" DIRS="$DIRS roms/seabios roms/vgabios" FILES="Makefile tests/tcg/Makefile qdict-test-data.txt" FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit" FILES="$FILES tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile" +FILES="$FILES tests/fp/Makefile" FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps" FILES="$FILES pc-bios/spapr-rtas/Makefile" FILES="$FILES pc-bios/s390-ccw/Makefile" diff --git a/tests/fp/platform.h b/tests/fp/platform.h new file mode 100644 index 00..80af8a94b6 --- /dev/null +++ b/tests/fp/platform.h @@ -0,0 +1,41 @@ +#ifndef QEMU_TESTFLOAT_PLATFORM_H +#define QEMU_TESTFLOAT_PLATFORM_H +/* + * Copyright 2011, 2012, 2013, 2014, 2015, 2016 The Regents of the University of + * California. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions, and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions, and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * 3. Neither the name of the University nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS "AS IS", AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +#include "config-host.h" + +#ifndef HOST_WORDS_BIGENDIAN +#define LITTLEENDIAN 1 +/* otherwise do not define it */ +#endif + +#define INLINE static inline + +#endif /* QEMU_TESTFLOAT_PLATFORM_H */ diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c new file mode 100644 index 00..58405fdd89 --- /dev/null +++ b/tests/fp/fp-test.c @@ -0,0 +1,1052 @@ +/* + * fp-test.c - test QEMU's softfloat implementation using Berkeley's Testfloat + * + * Derived from testfloat/source/testsoftfloat.c. + * + * Copyright (C) 2018, Emilio G. Cota + * Copyright 2011, 2012, 2013, 2014, 2015, 2016, 2017 The Regents of the + * University of California. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions, and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions, and the following disclaimer in the + *
[Qemu-devel] [PATCH 1/2] gitmodules: add berkeley's softfloat + testfloat version 3
These are BSD-licensed so we can add them as submodules. Signed-off-by: Emilio G. Cota --- .gitmodules | 6 ++ tests/fp/berkeley-softfloat-3 | 1 + tests/fp/berkeley-testfloat-3 | 1 + 3 files changed, 8 insertions(+) create mode 16 tests/fp/berkeley-softfloat-3 create mode 16 tests/fp/berkeley-testfloat-3 diff --git a/.gitmodules b/.gitmodules index d108478e0a..165c7c5286 100644 --- a/.gitmodules +++ b/.gitmodules @@ -43,3 +43,9 @@ [submodule "roms/u-boot-sam460ex"] path = roms/u-boot-sam460ex url = git://git.qemu.org/u-boot-sam460ex.git +[submodule "tests/fp/testfloat"] + path = tests/fp/berkeley-testfloat-3 + url = git://github.com/ucb-bar/berkeley-testfloat-3 +[submodule "tests/fp/berkeley-softfloat-3"] + path = tests/fp/berkeley-softfloat-3 + url = git://github.com/ucb-bar/berkeley-softfloat-3 diff --git a/tests/fp/berkeley-softfloat-3 b/tests/fp/berkeley-softfloat-3 new file mode 16 index 00..b64af41c32 --- /dev/null +++ b/tests/fp/berkeley-softfloat-3 @@ -0,0 +1 @@ +Subproject commit b64af41c3276f97f0e181920400ee056b9c88037 diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3 new file mode 16 index 00..06b20075dd --- /dev/null +++ b/tests/fp/berkeley-testfloat-3 @@ -0,0 +1 @@ +Subproject commit 06b20075dd3c1a5d0dd007a93643282832221612 -- 2.17.1
Re: [Qemu-devel] [PATCH v3 2/8] tests: add tests for hbitmap_next_zero with specified end parameter
On 08/14/2018 08:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/test-hbitmap.c | 32
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
> index 6b6a40bddd..dddb67c3c5 100644
> --- a/tests/test-hbitmap.c
> +++ b/tests/test-hbitmap.c
> @@ -937,31 +937,49 @@ static void test_hbitmap_iter_and_reset(TestHBitmapData
> *data,
> check_hbitmap_iter_next();
> }
>
> -static void test_hbitmap_next_zero_check(TestHBitmapData *data, int64_t
> start)
> +static void test_hbitmap_next_zero_check_range(TestHBitmapData *data,
> + int64_t start,
> + int64_t count)
Should this interface match the implementation? Because ...
> {
> -int64_t ret1 = hbitmap_next_zero(data->hb, start, -1);
> +int64_t ret1 = hbitmap_next_zero(data->hb, start,
> + count == -1 ? -1 : start + count);
I find it confusing that we use 0 as a sentinel value here and have to
swap it out with -1. If the interfaces matched this would read a little
cleaner, wouldn't it?
> int64_t ret2 = start;
> -for ( ; ret2 < data->size && hbitmap_get(data->hb, ret2); ret2++) {
> +int64_t end = count == -1 ? data->size : start + count;
> +
> +for ( ; ret2 < end && hbitmap_get(data->hb, ret2); ret2++) {
> ;
> }
> -if (ret2 == data->size) {
> +if (ret2 == end) {
> ret2 = -1;
> }
>
> g_assert_cmpint(ret1, ==, ret2);
> }
>
> +static void test_hbitmap_next_zero_check(TestHBitmapData *data, int64_t
> start)
> +{
> +test_hbitmap_next_zero_check_range(data, start, 0);
> +}
> +
> static void test_hbitmap_next_zero_do(TestHBitmapData *data, int granularity)
> {
> hbitmap_test_init(data, L3, granularity);
> test_hbitmap_next_zero_check(data, 0);
> test_hbitmap_next_zero_check(data, L3 - 1);
> +test_hbitmap_next_zero_check_range(data, 0, 1);
> +test_hbitmap_next_zero_check_range(data, L3 - 1, 1);
>
> hbitmap_set(data->hb, L2, 1);
> test_hbitmap_next_zero_check(data, 0);
> test_hbitmap_next_zero_check(data, L2 - 1);
> test_hbitmap_next_zero_check(data, L2);
> test_hbitmap_next_zero_check(data, L2 + 1);
> +test_hbitmap_next_zero_check_range(data, 0, 1);
> +test_hbitmap_next_zero_check_range(data, 0, L2);
> +test_hbitmap_next_zero_check_range(data, L2 - 1, 1);
> +test_hbitmap_next_zero_check_range(data, L2 - 1, 2);
> +test_hbitmap_next_zero_check_range(data, L2, 1);
> +test_hbitmap_next_zero_check_range(data, L2 + 1, 1);
>
> hbitmap_set(data->hb, L2 + 5, L1);
> test_hbitmap_next_zero_check(data, 0);
> @@ -970,6 +988,10 @@ static void test_hbitmap_next_zero_do(TestHBitmapData
> *data, int granularity)
> test_hbitmap_next_zero_check(data, L2 + 5);
> test_hbitmap_next_zero_check(data, L2 + L1 - 1);
> test_hbitmap_next_zero_check(data, L2 + L1);
> +test_hbitmap_next_zero_check_range(data, L2, 6);
> +test_hbitmap_next_zero_check_range(data, L2 + 1, 3);
> +test_hbitmap_next_zero_check_range(data, L2 + 4, L1);
> +test_hbitmap_next_zero_check_range(data, L2 + 5, L1);
>
> hbitmap_set(data->hb, L2 * 2, L3 - L2 * 2);
> test_hbitmap_next_zero_check(data, L2 * 2 - L1);
> @@ -977,6 +999,8 @@ static void test_hbitmap_next_zero_do(TestHBitmapData
> *data, int granularity)
> test_hbitmap_next_zero_check(data, L2 * 2 - 1);
> test_hbitmap_next_zero_check(data, L2 * 2);
> test_hbitmap_next_zero_check(data, L3 - 1);
> +test_hbitmap_next_zero_check_range(data, L2 * 2 - L1, L1 + 1);
> +test_hbitmap_next_zero_check_range(data, L2 * 2, L2);
>
> hbitmap_set(data->hb, 0, L3);
> test_hbitmap_next_zero_check(data, 0);
>
[Qemu-devel] [PULL 3/7] hw/pci: factor PCI reserve resources to a separate structure
From: Jing Liu
Factor "bus_reserve", "io_reserve", "mem_reserve", "pref32_reserve"
and "pref64_reserve" fields of the "GenPCIERootPort" structure out
to "PCIResReserve" structure, so that other PCI bridges can
reuse it to add resource reserve capability.
Signed-off-by: Jing Liu
Reviewed-by: Marcel Apfelbaum
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
include/hw/pci/pci_bridge.h| 18 ++
hw/pci-bridge/gen_pcie_root_port.c | 33 +-
hw/pci/pci_bridge.c| 38 +-
3 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 0347da52d2..cdff7edfd1 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -133,11 +133,19 @@ typedef struct PCIBridgeQemuCap {
#define REDHAT_PCI_CAP_RESOURCE_RESERVE 1
+/*
+ * PCI BUS/IO/MEM/PREFMEM additional resources recorded as a
+ * capability in PCI configuration space to reserve on firmware init.
+ */
+typedef struct PCIResReserve {
+uint32_t bus;
+uint64_t io;
+uint64_t mem_non_pref;
+uint64_t mem_pref_32;
+uint64_t mem_pref_64;
+} PCIResReserve;
+
int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
- uint32_t bus_reserve, uint64_t io_reserve,
- uint64_t mem_non_pref_reserve,
- uint64_t mem_pref_32_reserve,
- uint64_t mem_pref_64_reserve,
- Error **errp);
+ PCIResReserve res_reserve, Error **errp);
#endif /* QEMU_PCI_BRIDGE_H */
diff --git a/hw/pci-bridge/gen_pcie_root_port.c
b/hw/pci-bridge/gen_pcie_root_port.c
index d117e20325..299de429ec 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -29,12 +29,8 @@ typedef struct GenPCIERootPort {
bool migrate_msix;
-/* additional resources to reserve on firmware init */
-uint32_t bus_reserve;
-uint64_t io_reserve;
-uint64_t mem_reserve;
-uint64_t pref32_reserve;
-uint64_t pref64_reserve;
+/* additional resources to reserve */
+PCIResReserve res_reserve;
} GenPCIERootPort;
static uint8_t gen_rp_aer_vector(const PCIDevice *d)
@@ -82,16 +78,15 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
return;
}
-int rc = pci_bridge_qemu_reserve_cap_init(d, 0, grp->bus_reserve,
-grp->io_reserve, grp->mem_reserve, grp->pref32_reserve,
-grp->pref64_reserve, errp);
+int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
+ grp->res_reserve, errp);
if (rc < 0) {
rpc->parent_class.exit(d);
return;
}
-if (!grp->io_reserve) {
+if (!grp->res_reserve.io) {
pci_word_test_and_clear_mask(d->wmask + PCI_COMMAND,
PCI_COMMAND_IO);
d->wmask[PCI_IO_BASE] = 0;
@@ -117,12 +112,18 @@ static const VMStateDescription vmstate_rp_dev = {
};
static Property gen_rp_props[] = {
-DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, true),
-DEFINE_PROP_UINT32("bus-reserve", GenPCIERootPort, bus_reserve, -1),
-DEFINE_PROP_SIZE("io-reserve", GenPCIERootPort, io_reserve, -1),
-DEFINE_PROP_SIZE("mem-reserve", GenPCIERootPort, mem_reserve, -1),
-DEFINE_PROP_SIZE("pref32-reserve", GenPCIERootPort, pref32_reserve, -1),
-DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort, pref64_reserve, -1),
+DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort,
+ migrate_msix, true),
+DEFINE_PROP_UINT32("bus-reserve", GenPCIERootPort,
+ res_reserve.bus, -1),
+DEFINE_PROP_SIZE("io-reserve", GenPCIERootPort,
+ res_reserve.io, -1),
+DEFINE_PROP_SIZE("mem-reserve", GenPCIERootPort,
+ res_reserve.mem_non_pref, -1),
+DEFINE_PROP_SIZE("pref32-reserve", GenPCIERootPort,
+ res_reserve.mem_pref_32, -1),
+DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
+ res_reserve.mem_pref_64, -1),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 40a39f57cb..08b7e44e2e 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -411,38 +411,34 @@ void pci_bridge_map_irq(PCIBridge *br, const char*
bus_name,
int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
- uint32_t bus_reserve, uint64_t io_reserve,
- uint64_t mem_non_pref_reserve,
- uint64_t mem_pref_32_reserve,
- uint64_t mem_pref_64_reserve,
- Error **errp)
+ PCIResReserve res_reserve, Error **errp)
{
-
[Qemu-devel] [PULL 2/7] virtio: update MemoryRegionCaches when guest negotiates features
From: Paolo Bonzini
Because the cache is sized to include the rings and the event indices,
negotiating the VIRTIO_RING_F_EVENT_IDX feature will result in the size
of the cache changing. And because MemoryRegionCache accesses are
range-checked, if we skip this we end up with an assertion failure.
This happens with OpenBSD 6.3.
Reported-by: Fam Zheng
Fixes: 97cd965c070152bc626c7507df9fb356bbe1cd81
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
Tested-by: Fam Zheng
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/virtio/virtio.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d4e4d98b59..f6a588ab57 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2006,14 +2006,25 @@ static int virtio_set_features_nocheck(VirtIODevice
*vdev, uint64_t val)
int virtio_set_features(VirtIODevice *vdev, uint64_t val)
{
- /*
+int ret;
+/*
* The driver must not attempt to set features after feature negotiation
* has finished.
*/
if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
return -EINVAL;
}
-return virtio_set_features_nocheck(vdev, val);
+ret = virtio_set_features_nocheck(vdev, val);
+if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
+int i;
+for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+if (vdev->vq[i].vring.num != 0) {
+virtio_init_region_cache(vdev, i);
+}
+}
+}
+return ret;
}
int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
--
MST
[Qemu-devel] [PULL 7/7] tests: update acpi expected files
Fixes: dbb6da8ba7e ("pc: acpi: revert back to 1 SRAT entry for hotpluggable
area")
Signed-off-by: Michael S. Tsirkin
---
tests/acpi-test-data/pc/DSDT | Bin 5144 -> 5131 bytes
tests/acpi-test-data/pc/DSDT.bridge | Bin 7003 -> 6990 bytes
tests/acpi-test-data/pc/DSDT.cphp | Bin 5607 -> 5594 bytes
tests/acpi-test-data/pc/DSDT.dimmpxm | Bin 6803 -> 6790 bytes
tests/acpi-test-data/pc/DSDT.ipmikcs | Bin 5216 -> 5203 bytes
tests/acpi-test-data/pc/DSDT.memhp| Bin 6509 -> 6496 bytes
tests/acpi-test-data/pc/DSDT.numamem | Bin 5150 -> 5137 bytes
tests/acpi-test-data/pc/SRAT.dimmpxm | Bin 472 -> 392 bytes
tests/acpi-test-data/pc/SRAT.memhp| Bin 264 -> 264 bytes
tests/acpi-test-data/q35/DSDT | Bin 7828 -> 7815 bytes
tests/acpi-test-data/q35/DSDT.bridge | Bin 7845 -> 7832 bytes
tests/acpi-test-data/q35/DSDT.cphp| Bin 8291 -> 8278 bytes
tests/acpi-test-data/q35/DSDT.dimmpxm | Bin 9487 -> 9474 bytes
tests/acpi-test-data/q35/DSDT.ipmibt | Bin 7903 -> 7890 bytes
tests/acpi-test-data/q35/DSDT.memhp | Bin 9193 -> 9180 bytes
tests/acpi-test-data/q35/DSDT.numamem | Bin 7834 -> 7821 bytes
tests/acpi-test-data/q35/SRAT.dimmpxm | Bin 472 -> 392 bytes
tests/acpi-test-data/q35/SRAT.memhp | Bin 264 -> 264 bytes
18 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index
99f05a502752d9dbac38fdd93f1ebb79b4564fb4..c6adfe32d5ba6a5db2ca3e210766839547e122c8
100644
GIT binary patch
delta 45
zcmbQC(XGMd66_MfEyBRSxN##_B|DRk+2mIC)lB}{n~gYHnHVi5@8r_l{EkbD695z3
B42=K)
delta 57
zcmeCyn4!Vt66_KpA;Q4G7`~CKlAX!Zd~z%MY9=3@%|;xpOiZpUlXq}wa}
L1BuP|xpX)Iow5#g
diff --git a/tests/acpi-test-data/pc/DSDT.bridge
b/tests/acpi-test-data/pc/DSDT.bridge
index
cf23343e6402421f09da5d09f72811108fbd2661..f01fa3ad4ee6aed5262daef464a1ade41e06975d
100644
GIT binary patch
delta 45
zcmca@cFv5;CD
B4h#SQ
delta 57
zcmX?ScH4~0CDSe&_
M3?w$+=gJoV0Nbw*^#A|>
diff --git a/tests/acpi-test-data/pc/DSDT.cphp
b/tests/acpi-test-data/pc/DSDT.cphp
index
c99c49f43705e99d1e0a8ba19d44145dfa63d009..3295d81c7f725472671632ac612a1c3ed81d7492
100644
GIT binary patch
delta 45
zcmaE^eM_6mCDfHXCsqWMZ_N{FO^{vljPWE
B4gvrG
delta 57
zcmcbm{al;NCDi_@%
diff --git a/tests/acpi-test-data/pc/DSDT.dimmpxm
b/tests/acpi-test-data/pc/DSDT.dimmpxm
index
38661cb13ee348718ab45bfc69452cd642cf9bb9..f6ec911b1180a409e61ef8d50279ab6dba7f1bdd
100644
GIT binary patch
delta 45
zcmbPi+Gfh-66_MvCdI(OXtBIQ-{I2SEXdu&4*(kq
B41fRt
delta 57
zcmZoOooveG66_K(S@%l!tN_Hkk&?KvHXCuQU}ExSnS7f|lZ!jv)ysg<
M8Axnq;%?yw0M!W(X#fBK
diff --git a/tests/acpi-test-data/pc/DSDT.ipmikcs
b/tests/acpi-test-data/pc/DSDT.ipmikcs
index
5e970fda7296f9ce44487e0a578a1dead982ba66..2633a8cecf017bfce01ba8377428b8c5433e0be2
100644
GIT binary patch
delta 45
zcmaE$aan`QCD8*#KUG1^Vu$)=~E
A%m4rY
delta 56
zcmaE0^wx;WCD)hUB|D@4u
diff --git a/tests/acpi-test-data/pc/DSDT.numamem
b/tests/acpi-test-data/pc/DSDT.numamem
index
224cfdd9e983e02dac5f4bf7e210eaa64cb0dc78..71a975b3e25f8b5cc4491f16d5575a3a35afc777
100644
GIT binary patch
delta 44
zcmbQIF;RodCD~`(00AKk
A3jhEB
delta 56
zcmbQJF;9cbCDDi`0S16#An*f1F)$!x7#IKwZw2lE
delta 138
zcmeBRzQN2D9OM{sgOP!Oao0w!2*xrX3kskt4j7*q#sJcc8cZ-A2f8|Tm<(7QMA!fS
P|G$IOAxvjrKsFBmfb|MK
diff --git a/tests/acpi-test-data/pc/SRAT.memhp
b/tests/acpi-test-data/pc/SRAT.memhp
index
5de8a100a4adf968b79a7b154a7f98123d583474..e508b4ae3cd9e3000209a4f9597913faa4206ec1
100644
GIT binary patch
delta 51
pcmeBR>R{pu4ss0PU}RumTr`pEo=141z{FvA!?8UWLX1)2Z=
delta 51
pcmeBR>R{pu4ss0PU}RumTso2Kp2GkC|Gxt{3?K{x%rFL!1^_r@2$}!@
diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index
aa402cca667f82ed0a2dc4969508d8f6e38ad910..7576ffcd05991ad5a3901c0f7698a52fffc6d6e2
100644
GIT binary patch
delta 45
zcmbPY+iuI{66_MvF2}*szhySdz)dY_h%NY9@c}&2J=InHVi57t3gFULo^_5dbFH
B4Y>dS
delta 57
zcmZp-onp)766_K(MUH`iQEelau_TkH`DA;^)l5D*o8LHZ^)g^|
M1`?ZR%DiI)0Jy6Uq5uE@
diff --git a/tests/acpi-test-data/q35/DSDT.bridge
b/tests/acpi-test-data/q35/DSDT.bridge
index
fc3e79c583ababf5615e76ba2f7bc3df1483abb4..c623cc5d72a2e346793fa9128e7e88b6781241b2
100644
GIT binary patch
delta 45
zcmZ2#JHwXCCD0XfHouWP$i!$jdA*G0<|i`s%m7rz
B4=w-z
delta 57
zcmccS@YsRNCD{0Qw9O7XSbN
diff --git a/tests/acpi-test-data/q35/DSDT.dimmpxm
b/tests/acpi-test-data/q35/DSDT.dimmpxm
index
14904e8ea2376abd989aa9e99f5bf388a3b85032..3837792dec13c4c77c66b140f68959d86a09de8e
100644
GIT binary patch
delta 45
zcmeD8YVzW833dr#Qe|LZe7KRzSdz)bW3s*EY9@Ev&2J=EFfrOqo+hKY`Lv7+2LLB$
B4UPZ+
delta 57
zcmZqj>i6Pu33dtLS7l~RRbEXm~PIoV!vHIs|o<~Ncnn3%j-CQp^oHZ^)g^|
M1`?b1$hdI;0KEDScK`qY
diff --git a/tests/acpi-test-data/q35/DSDT.ipmibt
b/tests/acpi-test-data/q35/DSDT.ipmibt
index
332237529e114256384c051858fdac36b024c72e..c7f431f058bcb54e364be4edfd9d4609bc9bb602
100644
GIT binary patch
delta 45
zcmca_d&!o|CDb&^azW|O-lS2Ou*Z#I`|Wn#3LyiZ1R^Jkeii~v_f
B4>te+
delta 57
zcmca)d*7DJCDSe&_
M3?w!`k$J}m0QWEvAOHXW
diff --git
[Qemu-devel] [PULL 6/7] vhost: fix invalid downcast
From: Yury Kotov
virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 32-bit.
If returned hwaddr is not equal to 0 but least-significant 32 bits are
equal to 0 then this code will not actually stop running queue.
Signed-off-by: Yury Kotov
Acked-by: Jia He
Cc: qemu-sta...@nongnu.org
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/virtio/vhost.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index d4cb5894a8..569c4053ea 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1073,10 +1073,8 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
.index = vhost_vq_index,
};
int r;
-int a;
-a = virtio_queue_get_desc_addr(vdev, idx);
-if (a == 0) {
+if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
/* Don't stop the virtqueue which might have not been started */
return;
}
--
MST
[Qemu-devel] [PULL 5/7] pc: make sure that guest isn't able to unplug the first cpu
From: Igor Mammedov
The first cpu unplug wasn't ever supported and corresponding
monitor/qmp commands refuse to unplug it. However guest is able
to issue eject request either using following command:
# echo 1 >/sys/devices/system/cpu/cpu0/firmware_node/eject
or directly writing to cpu hotplug registers, which makes
qemu crash with SIGSEGV following back trace:
kvm_flush_coalesced_mmio_buffer ()
while (ring->first != ring->last)
...
qemu_flush_coalesced_mmio_buffer
prepare_mmio_access
flatview_read_continue
flatview_read
address_space_read_full
address_space_rw
kvm_cpu_exec(cpu!0)
qemu_kvm_cpu_thread_fn
the reason for which is that ring == KVMState::coalesced_mmio_ring
happens to be a part of 1st CPU that was uplugged by guest.
Fix it by forbidding 1st cpu unplug from guest side and in addition
remove CPU0._EJ0 ACPI method to make clear that unplug of the first
CPU is not supported.
Signed-off-by: Igor Mammedov
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/acpi/cpu.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 5ae595ecbe..4bb83713c5 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -117,7 +117,7 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr,
uint64_t data,
DeviceState *dev = NULL;
HotplugHandler *hotplug_ctrl = NULL;
-if (!cdev->cpu) {
+if (!cdev->cpu || cdev->cpu == first_cpu) {
trace_cpuhp_acpi_ejecting_invalid_cpu(cpu_st->selector);
break;
}
@@ -541,9 +541,11 @@ void build_cpus_aml(Aml *table, MachineState *machine,
CPUHotplugFeatures opts,
aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data)));
g_array_free(madt_buf, true);
-method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
-aml_append(method, aml_call1(CPU_EJECT_METHOD, uid));
-aml_append(dev, method);
+if (CPU(arch_ids->cpus[i].cpu) != first_cpu) {
+method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+aml_append(method, aml_call1(CPU_EJECT_METHOD, uid));
+aml_append(dev, method);
+}
method = aml_method("_OST", 3, AML_SERIALIZED);
aml_append(method,
--
MST
[Qemu-devel] [PULL 0/7] pci, pc, virtio: fixes, features
The following changes since commit 19b599f7664b2ebfd0f405fb79c14dd241557452: Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging (2018-08-27 16:44:20 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to d2a1b1d602986a5f02658f6d4fc9ed422f8ddebf: tests: update acpi expected files (2018-09-07 17:43:22 -0400) pci, pc, virtio: fixes, features pci resource capability + misc fixes everywhere. Signed-off-by: Michael S. Tsirkin Igor Mammedov (2): pc: acpi: revert back to 1 SRAT entry for hotpluggable area pc: make sure that guest isn't able to unplug the first cpu Jing Liu (2): hw/pci: factor PCI reserve resources to a separate structure hw/pci: add PCI resource reserve capability to legacy PCI bridge Michael S. Tsirkin (1): tests: update acpi expected files Paolo Bonzini (1): virtio: update MemoryRegionCaches when guest negotiates features Yury Kotov (1): vhost: fix invalid downcast include/hw/pci/pci_bridge.h | 18 ++--- hw/acpi/cpu.c | 10 +++-- hw/i386/acpi-build.c | 73 ++ hw/pci-bridge/gen_pcie_root_port.c| 33 +++ hw/pci-bridge/pci_bridge_dev.c| 24 +++ hw/pci/pci_bridge.c | 38 -- hw/virtio/vhost.c | 4 +- hw/virtio/virtio.c| 15 ++- tests/acpi-test-data/pc/DSDT | Bin 5144 -> 5131 bytes tests/acpi-test-data/pc/DSDT.bridge | Bin 7003 -> 6990 bytes tests/acpi-test-data/pc/DSDT.cphp | Bin 5607 -> 5594 bytes tests/acpi-test-data/pc/DSDT.dimmpxm | Bin 6803 -> 6790 bytes tests/acpi-test-data/pc/DSDT.ipmikcs | Bin 5216 -> 5203 bytes tests/acpi-test-data/pc/DSDT.memhp| Bin 6509 -> 6496 bytes tests/acpi-test-data/pc/DSDT.numamem | Bin 5150 -> 5137 bytes tests/acpi-test-data/pc/SRAT.dimmpxm | Bin 472 -> 392 bytes tests/acpi-test-data/pc/SRAT.memhp| Bin 264 -> 264 bytes tests/acpi-test-data/q35/DSDT | Bin 7828 -> 7815 bytes tests/acpi-test-data/q35/DSDT.bridge | Bin 7845 -> 7832 bytes tests/acpi-test-data/q35/DSDT.cphp| Bin 8291 -> 8278 bytes tests/acpi-test-data/q35/DSDT.dimmpxm | Bin 9487 -> 9474 bytes tests/acpi-test-data/q35/DSDT.ipmibt | Bin 7903 -> 7890 bytes tests/acpi-test-data/q35/DSDT.memhp | Bin 9193 -> 9180 bytes tests/acpi-test-data/q35/DSDT.numamem | Bin 7834 -> 7821 bytes tests/acpi-test-data/q35/SRAT.dimmpxm | Bin 472 -> 392 bytes tests/acpi-test-data/q35/SRAT.memhp | Bin 264 -> 264 bytes 26 files changed, 103 insertions(+), 112 deletions(-)
[Qemu-devel] [PULL 4/7] hw/pci: add PCI resource reserve capability to legacy PCI bridge
From: Jing Liu
Add hint to firmware (e.g. SeaBIOS) to reserve addtional
BUS/IO/MEM/PREF resource for legacy pci-pci bridge. Add the
resource reserve capability deleting in pci_bridge_dev_exitfn.
Signed-off-by: Jing Liu
Reviewed-by: Marcel Apfelbaum
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/pci-bridge/pci_bridge_dev.c | 24
1 file changed, 24 insertions(+)
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index b2d861d216..97a8e8b6a4 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -46,6 +46,9 @@ struct PCIBridgeDev {
uint32_t flags;
OnOffAuto msi;
+
+/* additional resources to reserve */
+PCIResReserve res_reserve;
};
typedef struct PCIBridgeDev PCIBridgeDev;
@@ -95,6 +98,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error
**errp)
error_free(local_err);
}
+err = pci_bridge_qemu_reserve_cap_init(dev, 0,
+ bridge_dev->res_reserve, errp);
+if (err) {
+goto cap_error;
+}
+
if (shpc_present(dev)) {
/* TODO: spec recommends using 64 bit prefetcheable BAR.
* Check whether that works well. */
@@ -103,6 +112,8 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error
**errp)
}
return;
+cap_error:
+msi_uninit(dev);
msi_error:
slotid_cap_cleanup(dev);
slotid_error:
@@ -116,6 +127,8 @@ shpc_error:
static void pci_bridge_dev_exitfn(PCIDevice *dev)
{
PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
+
+pci_del_capability(dev, PCI_CAP_ID_VNDR, sizeof(PCIBridgeQemuCap));
if (msi_present(dev)) {
msi_uninit(dev);
}
@@ -162,6 +175,17 @@ static Property pci_bridge_dev_properties[] = {
ON_OFF_AUTO_AUTO),
DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags,
PCI_BRIDGE_DEV_F_SHPC_REQ, true),
+DEFINE_PROP_UINT32("bus-reserve", PCIBridgeDev,
+ res_reserve.bus, -1),
+DEFINE_PROP_SIZE("io-reserve", PCIBridgeDev,
+ res_reserve.io, -1),
+DEFINE_PROP_SIZE("mem-reserve", PCIBridgeDev,
+ res_reserve.mem_non_pref, -1),
+DEFINE_PROP_SIZE("pref32-reserve", PCIBridgeDev,
+ res_reserve.mem_pref_32, -1),
+DEFINE_PROP_SIZE("pref64-reserve", PCIBridgeDev,
+ res_reserve.mem_pref_64, -1),
+
DEFINE_PROP_END_OF_LIST(),
};
--
MST
[Qemu-devel] [PULL 1/7] pc: acpi: revert back to 1 SRAT entry for hotpluggable area
From: Igor Mammedov
Commit
10efd7e108 "pc: acpi: fix memory hotplug regression by reducing stub SRAT
entry size"
attemped to fix hotplug regression introduced by
848a1cc1e "hw/acpi-build: build SRAT memory affinity structures for DIMM
devices"
fixed issue for Windows/3.0+ linux kernels, however it regressed 2.6 based
kernels (RHEL6) to the point where guest might crash at boot.
Reason is that 2.6 kernel discards SRAT table due too small last entry
which down the road leads to crashes. Hack I've tried in 10efd7e108 is also
not ACPI spec compliant according to which whole possible RAM should be
described in SRAT. Revert 10efd7e108 to fix regression for 2.6 based kernels.
With 10efd7e108 reverted, I've also tried splitting SRAT table statically
in different ways %/node and %/slot but Windows still fails to online
2nd pc-dimm hot-plugged into node 0 (as described in 10efd7e108) and
sometimes even coldplugged pc-dimms where affected with static SRAT
partitioning.
The only known so far way where Windows stays happy is when we have 1
SRAT entry in the last node covering all hotplug area.
Revert 848a1cc1e until we come up with a way to avoid regression
on Windows with hotplug area split in several entries.
Tested this with 2.6/3.0 based kernels (RHEL6/7) and WS20[08/12/12R2/16]).
Signed-off-by: Igor Mammedov
Acked-by: Laszlo Ersek
Reviewed-by: Eduardo Habkost
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/i386/acpi-build.c | 73
1 file changed, 12 insertions(+), 61 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e1ee8ae9e0..1599caa7c5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2251,64 +2251,6 @@ build_tpm2(GArray *table_data, BIOSLinker *linker,
GArray *tcpalog)
#define HOLE_640K_START (640 * KiB)
#define HOLE_640K_END (1 * MiB)
-static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base,
- uint64_t len, int default_node)
-{
-MemoryDeviceInfoList *info_list = qmp_memory_device_list();
-MemoryDeviceInfoList *info;
-MemoryDeviceInfo *mi;
-PCDIMMDeviceInfo *di;
-uint64_t end = base + len, cur, size;
-bool is_nvdimm;
-AcpiSratMemoryAffinity *numamem;
-MemoryAffinityFlags flags;
-
-for (cur = base, info = info_list;
- cur < end;
- cur += size, info = info->next) {
-numamem = acpi_data_push(table_data, sizeof *numamem);
-
-if (!info) {
-/*
- * Entry is required for Windows to enable memory hotplug in OS
- * and for Linux to enable SWIOTLB when booted with less than
- * 4G of RAM. Windows works better if the entry sets proximity
- * to the highest NUMA node in the machine at the end of the
- * reserved space.
- * Memory devices may override proximity set by this entry,
- * providing _PXM method if necessary.
- */
-build_srat_memory(numamem, end - 1, 1, default_node,
- MEM_AFFINITY_HOTPLUGGABLE |
MEM_AFFINITY_ENABLED);
-break;
-}
-
-mi = info->value;
-is_nvdimm = (mi->type == MEMORY_DEVICE_INFO_KIND_NVDIMM);
-di = !is_nvdimm ? mi->u.dimm.data : mi->u.nvdimm.data;
-
-if (cur < di->addr) {
-build_srat_memory(numamem, cur, di->addr - cur, default_node,
- MEM_AFFINITY_HOTPLUGGABLE |
MEM_AFFINITY_ENABLED);
-numamem = acpi_data_push(table_data, sizeof *numamem);
-}
-
-size = di->size;
-
-flags = MEM_AFFINITY_ENABLED;
-if (di->hotpluggable) {
-flags |= MEM_AFFINITY_HOTPLUGGABLE;
-}
-if (is_nvdimm) {
-flags |= MEM_AFFINITY_NON_VOLATILE;
-}
-
-build_srat_memory(numamem, di->addr, size, di->node, flags);
-}
-
-qapi_free_MemoryDeviceInfoList(info_list);
-}
-
static void
build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
{
@@ -2414,10 +2356,19 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
}
+/*
+ * Entry is required for Windows to enable memory hotplug in OS
+ * and for Linux to enable SWIOTLB when booted with less than
+ * 4G of RAM. Windows works better if the entry sets proximity
+ * to the highest NUMA node in the machine.
+ * Memory devices may override proximity set by this entry,
+ * providing _PXM method if necessary.
+ */
if (hotplugabble_address_space_size) {
-build_srat_hotpluggable_memory(table_data,
machine->device_memory->base,
- hotplugabble_address_space_size,
- pcms->numa_nodes - 1);
+numamem = acpi_data_push(table_data, sizeof *numamem);
+
Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero
On 08/14/2018 08:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add bytes parameter to the function, to limit searched range.
>
I'm going to assume that Eric Blake has been through here and commented
on the interface itself.
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> include/block/dirty-bitmap.h | 3 ++-
> include/qemu/hbitmap.h | 10 --
> block/backup.c | 2 +-
> block/dirty-bitmap.c | 5 +++--
> nbd/server.c | 2 +-
> tests/test-hbitmap.c | 2 +-
> util/hbitmap.c | 25 -
> 7 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 259bd27c40..27f5299c4e 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -98,7 +98,8 @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState
> *bs);
> BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap);
> char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
> -int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start);
> +int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start,
> +int64_t end);
> BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
>BdrvDirtyBitmap *bitmap,
>Error **errp);
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index ddca52c48e..fe4dfde27a 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -295,10 +295,16 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi);
> /* hbitmap_next_zero:
> * @hb: The HBitmap to operate on
> * @start: The bit to start from.
> + * @end: End of range to search in. If @end is -1, search up to the bitmap
> + * end.
> *
> - * Find next not dirty bit.
> + * Find next not dirty bit within range [@start, @end), or from
> + * @start to the bitmap end if @end is -1. If not found, return -1.
> + *
> + * @end may be greater than original bitmap size, in this case, search up to
"original" bitmap size? I think that's just an implementation detail, we
can drop 'original' here, yes?
> + * the bitmap end.
> */
> -int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start);
> +int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start, int64_t end);
>
The interface looks weird because we can define a 'start' that's beyond
the 'end'.
I realize that you need a signed integer for 'end' to signify EOF...
should we do a 'bytes' parameter instead? (Did you already do that in an
earlier version and we changed it?)
Well, it's not a big deal to me personally.
> /* hbitmap_create_meta:
> * Create a "meta" hbitmap to track dirtiness of the bits in this HBitmap.
> diff --git a/block/backup.c b/block/backup.c
> index 8630d32926..9bfd3f7189 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -458,7 +458,7 @@ static void
> backup_incremental_init_copy_bitmap(BackupBlockJob *job)
> break;
> }
>
> -offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset);
> +offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset, -1);
> if (offset == -1) {
> hbitmap_set(job->copy_bitmap, cluster, end - cluster);
> break;
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index c9b8a6fd52..037ae62726 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -785,9 +785,10 @@ char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap
> *bitmap, Error **errp)
> return hbitmap_sha256(bitmap->bitmap, errp);
> }
>
> -int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
> +int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset,
> +int64_t end)
> {
> -return hbitmap_next_zero(bitmap->bitmap, offset);
> +return hbitmap_next_zero(bitmap->bitmap, offset, end);
> }
>
> void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap
> *src,
> diff --git a/nbd/server.c b/nbd/server.c
> index ea5fe0eb33..07920d123b 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1952,7 +1952,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap
> *bitmap, uint64_t offset,
> assert(begin < overall_end && nb_extents);
> while (begin < overall_end && i < nb_extents) {
> if (dirty) {
> -end = bdrv_dirty_bitmap_next_zero(bitmap, begin);
> +end = bdrv_dirty_bitmap_next_zero(bitmap, begin, -1);
> } else {
> bdrv_set_dirty_iter(it, begin);
> end = bdrv_dirty_iter_next(it);
> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
> index 5e67ac1d3a..6b6a40bddd 100644
> --- a/tests/test-hbitmap.c
>
Re: [Qemu-devel] [virtio-dev] [PATCH v25 0/2] virtio-crypto: virtio crypto device specification
OK it is time to apply this. If I do I get latex errors (undefined and multiple-defined labels). Please post patches on top to fix this up, I will squash. On Fri, Aug 24, 2018 at 08:43:34AM +0800, Longpeng(Mike) wrote: > --- > v25 -> v24 > - fix some typos and letex grammar. > > v24 -> v23 > - Some enhancements based on Halil's suggestion. [Halil] > > v23 -> v22 > - rename MUX_MODE to REVISION_1 [Halil] > - fixed-length paramenters' instead of 'header' and >'variable-length parameters' instead of 'extra parameters' [Halil] > - add guidance about VIRTIO_CRYPTO_FLAG_SESSION_MODE [Halil] > - other fixes. [Longpeng] > > v22 -> v21 > - fix some typos and grammar fixes [Halil, Stefan] > - reorder names in alphabetical order [Stefan] > - redescribe the date format [Halil] > > v21 -> v20 > - rename 'queue_id' to 'reserved' [Halil] > - redescribe the format of the structures which using 'union' >in the previous version [Halil] > > v20 -> v19 > - fix some typos and grammar fixes [Halil] > - make queue_id reserved [Halil] > - remove 'Steps of Operation' > > v19 -> v18: > - fix some typos and grammar fixes [Stefan, Halil] > - rename VIRTIO_CRYPTO_F_STATELESS_MODE to VIRTIO_CRYPTO_F_MUX_MODE > - describe the VIRTIO_CRYPTO_STATUS in detial. [Halil] > - refactor and redescribe the controlq/dataq request's format >of mux mode. [Halil] > - other small fixes. [Halil] > > v18 -> v17: > - fix many English grammar problems suggested by Stefan, Thanks a lot! > > v17 -> v16: > - Some grammar fixes [Stefan, Halil, Michael] > - add a section named "Supported crypto services" in order to explain bit >numbers and valuse clearly. [Halil, Cornelia] > - avoid word reptition [Halil] > - rename non-session mode to stateless mode [Halil] > - change descriptions for all elements in struct virtio_crypto_config [Halil] > - add Halil as a reviewer in the ackonwledgement part, thanks for his work. > - other fixes here and there. > > Changes since v15: > - use feature bits for non-session mode in order to keep compatibility with >pre-existing code. [Halil & Michael] > - introduce VIRTIO_CRYPTO_F_ NON_SESSION_MODE feature bit to control all > other >non-session mode feature bits. > - fix some typos. [Stefan] > - introduce struct virtio_crypto_op_data_req_mux to support both session >and non-session based crypto operations and keep compatibility with >pre-existing code. > > Changes since v14: > - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia] > - correct a sentence about dataqueue and controlq in the first paragraph. > [Halil] > - change a MAY to MUST about max_dataqueues. [Halil] > - add non-session mode support >a) add four features for different crypto services to identify wheather > support session mode. >b) rewrite some > > For pervious versions of virtio crypto spec, Pls see: > > [v18]: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg444897.html > > [v14]: > https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html > > [v13]: > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html > > For more information, please see: > http://qemu-project.org/Features/VirtioCrypto > > Longpeng(Mike) (2): > virtio-crypto: Add virtio crypto device specification > virtio-crypto: Add conformance clauses > > acknowledgements.tex |4 + > conformance.tex | 29 + > content.tex |2 + > virtio-crypto.tex| 1533 > ++ > 4 files changed, 1568 insertions(+) > create mode 100644 virtio-crypto.tex > > -- > 1.8.3.1 > > > > - > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [Qemu-devel] [PATCH V5 1/4] target-i386: introduce coalesced_pio kvm header update
On Fri, Aug 31, 2018 at 05:05:02PM +0800, Peng Hao wrote:
> add coalesced_pio's struct and KVM_CAP_COALESCED_PIO header.
>
> Signed-off-by: Peng Hao
I can merge ths together with q35 patch, but pls
get acks from kvm folks, to this end fix up
commit logs and subjects to match reality and repost.
> ---
> accel/kvm/kvm-all.c | 4 ++--
> linux-headers/linux/kvm.h | 11 +--
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index eb7db92..4a3909d 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -536,7 +536,7 @@ static void kvm_coalesce_mmio_region(MemoryListener
> *listener,
>
> zone.addr = start;
> zone.size = size;
> -zone.pad = 0;
> +zone.pio = 0;
>
> (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, );
> }
> @@ -553,7 +553,7 @@ static void kvm_uncoalesce_mmio_region(MemoryListener
> *listener,
>
> zone.addr = start;
> zone.size = size;
> -zone.pad = 0;
> +zone.pio = 0;
>
> (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, );
> }
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 98f389a..1cb0d41 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -420,13 +420,19 @@ struct kvm_run {
> struct kvm_coalesced_mmio_zone {
> __u64 addr;
> __u32 size;
> - __u32 pad;
> + union {
> + __u32 pad;
> + __u32 pio;
> + };
> };
>
> struct kvm_coalesced_mmio {
> __u64 phys_addr;
> __u32 len;
> - __u32 pad;
> + union {
> + __u32 pad;
> + __u32 pio;
> + };
> __u8 data[8];
> };
>
> @@ -949,6 +955,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_GET_MSR_FEATURES 153
> #define KVM_CAP_HYPERV_EVENTFD 154
> #define KVM_CAP_HYPERV_TLBFLUSH 155
> +#define KVM_CAP_COALESCED_PIO 156
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 1.8.3.1
On Fri, Aug 31, 2018 at 05:05:03PM +0800, Peng Hao wrote:
> Signed-off-by: Peng Hao
> Reviewed-by: Eduardo Habkost
> ---
> accel/kvm/kvm-all.c | 57
> +++
> include/exec/memory.h | 4 ++--
> memory.c | 4 ++--
> 3 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4a3909d..11d8d78 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -78,6 +78,7 @@ struct KVMState
> int fd;
> int vmfd;
> int coalesced_mmio;
> +int coalesced_pio;
> struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
> bool coalesced_flush_in_progress;
> int vcpu_events;
> @@ -559,6 +560,45 @@ static void kvm_uncoalesce_mmio_region(MemoryListener
> *listener,
> }
> }
>
> +static void kvm_coalesce_pio_add(MemoryListener *listener,
> +MemoryRegionSection *section,
> +hwaddr start, hwaddr size)
> +{
> +KVMState *s = kvm_state;
> +
> +if (s->coalesced_pio) {
> +struct kvm_coalesced_mmio_zone zone;
> +
> +zone.addr = start;
> +zone.size = size;
> +zone.pio = 1;
> +
> +(void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, );
> +}
> +}
> +
> +static void kvm_coalesce_pio_del(MemoryListener *listener,
> +MemoryRegionSection *section,
> +hwaddr start, hwaddr size)
> +{
> +KVMState *s = kvm_state;
> +
> +if (s->coalesced_pio) {
> +struct kvm_coalesced_mmio_zone zone;
> +
> +zone.addr = start;
> +zone.size = size;
> +zone.pio = 1;
> +
> +(void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, );
> + }
> +}
> +
> +static MemoryListener kvm_coalesced_pio_listener = {
> +.coalesced_io_add = kvm_coalesce_pio_add,
> +.coalesced_io_del = kvm_coalesce_pio_del,
> +};
> +
> int kvm_check_extension(KVMState *s, unsigned int extension)
> {
> int ret;
> @@ -1615,6 +1655,8 @@ static int kvm_init(MachineState *ms)
> }
>
> s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
> +s->coalesced_pio = s->coalesced_mmio &&
> + kvm_check_extension(s, KVM_CAP_COALESCED_PIO);
>
> #ifdef KVM_CAP_VCPU_EVENTS
> s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
> @@ -1687,13 +1729,15 @@ static int kvm_init(MachineState *ms)
> s->memory_listener.listener.eventfd_add = kvm_mem_ioeventfd_add;
> s->memory_listener.listener.eventfd_del = kvm_mem_ioeventfd_del;
> }
> -s->memory_listener.listener.coalesced_mmio_add =
> kvm_coalesce_mmio_region;
> -s->memory_listener.listener.coalesced_mmio_del =
> kvm_uncoalesce_mmio_region;
> +s->memory_listener.listener.coalesced_io_add = kvm_coalesce_mmio_region;
> +s->memory_listener.listener.coalesced_io_del =
>
Re: [Qemu-devel] [PATCH V5 1/4] target-i386: introduce coalesced_pio kvm header update
On Fri, Aug 31, 2018 at 05:05:02PM +0800, Peng Hao wrote: > add coalesced_pio's struct and KVM_CAP_COALESCED_PIO header. > > Signed-off-by: Peng Hao Despite what the subject says, this is a kvm patch not a target-i386 one. In fact none of these are. And it does not add a header. Please change subjects so correct people review your patches. E.g. this one should be kvm: add coalesced_pio support Needs to be rebased too. -- MST
Re: [Qemu-devel] [PATCH] pc: make sure that guest isn't able to unplug the first cpu
On Wed, Aug 29, 2018 at 03:51:38PM +0200, Igor Mammedov wrote: > On Wed, 29 Aug 2018 09:15:53 -0400 > "Michael S. Tsirkin" wrote: > > > On Wed, Aug 29, 2018 at 10:43:11AM +0200, Igor Mammedov wrote: > > > On Wed, 29 Aug 2018 12:54:40 +1000 > > > David Gibson wrote: > > > > > > > On Tue, Aug 28, 2018 at 03:18:48PM +0200, Igor Mammedov wrote: > > > > > On Tue, 28 Aug 2018 10:52:37 +1000 > > > > > David Gibson wrote: > > > > > > > > > > > On Mon, Aug 27, 2018 at 04:02:39PM +0200, Greg Kurz wrote: > > > > > > > On Mon, 27 Aug 2018 13:07:10 +0200 > > > > > > > Igor Mammedov wrote: > > > > > > > > > > > > > > > The first cpu unplug wasn't ever supported and corresponding > > > > > > > > monitor/qmp commands refuse to unplug it. However guest is able > > > > > > > > to issue eject request either using following command: > > > > > > > > # echo 1 >/sys/devices/system/cpu/cpu0/firmware_node/eject > > > > > > > > > > > > > > > > > > > > > > I can't reproduce the issue with a pc guest and current master... > > > > > > > > > > > > > > All I seem to get is an error in dmesg: > > > > > > > > > > > > > > [ 97.435446] processor cpu0: Offline failed. > > > > > > > > > > > > > > > or directly writing to cpu hotplug registers, which makes > > > > > > > > qemu crash with SIGSEGV following back trace: > > > > > > > > > > > > > > > >kvm_flush_coalesced_mmio_buffer () > > > > > > > >while (ring->first != ring->last) > > > > > > > >... > > > > > > > >qemu_flush_coalesced_mmio_buffer > > > > > > > >prepare_mmio_access > > > > > > > >flatview_read_continue > > > > > > > >flatview_read > > > > > > > >address_space_read_full > > > > > > > >address_space_rw > > > > > > > >kvm_cpu_exec(cpu!0) > > > > > > > >qemu_kvm_cpu_thread_fn > > > > > > > > > > > > > > > > the reason for which is that ring == > > > > > > > > KVMState::coalesced_mmio_ring > > > > > > > > happens to be a part of 1st CPU that was uplugged by guest. > > > > > > > > > > > > > > > > Fix it by forbidding 1st cpu unplug from guest side and in > > > > > > > > addition > > > > > > > > remove CPU0._EJ0 ACPI method to make clear that unplug of the > > > > > > > > first > > > > > > > > CPU is not supported. > > > > > > > > > > > > > > > > Signed-off-by: Igor Mammedov > > > > > > > > --- > > > > > > > > CCing spapr and s390x folks in case targets need to prevent 1st > > > > > > > > CPU unplug as well > > > > > > > > > > > > > > > > > > > > > > A spapr guest can _release_ the first cpu by doing something like: > > > > > > > > > > > > > > # echo -n "/cpus/PowerPC,POWER8@0" > > > > > > > > /sys/devices/system/cpu/release > > > > > > > > > > > > > > But AFAIK, this doesn't unplug the cpu from a QEMU standpoint. > > > > > > > > > > > > > > > > > > > Unplugging CPU 0 with device_del should be ok too. > > > > > Do you mean real unplugging (cpu0 object freed) or just remove > > > > > request? > > > > > > > > Real unplugging should be possible. I'm not sure how thorougly it's > > > > been tested, though. > > > Well, common kvm code in qemu seems to be in disagreement with it > > > as backtrace in this patch shows also usage of first_cpu macro > > > won't survive such unplug. > > > > Paolo - any take on this? Do we need to make cpu 0 special like this? > It probably would take a bunch of refactoring to get rid of first_cpu > dependencies and besides of experimenting with cpu0 unplug in guest kernel > there isn't any other value in it, so it probably not worth the effort. > > On top of that, for pc/q35 machine we would need to select boot cpu > in some other way (right now it's hardwired to first_cpu). > > It seems that seabios might work if cpu0 isn't present, don't know about OVMF. OK, I queued this, but can you please add some code comments so we remember why it is like this if someone changes the code? Even better maybe to add an API along the lines of: cpu_is_hotpluggable return cpu != first_cpu -- MST
Re: [Qemu-devel] [PATCH 1/2] linux-headers: update to mainline 5c60a7389d79
On Fri, Aug 17, 2018 at 04:37:06PM +0800, Jason Wang wrote:
> Sync linux headers to 5c60a7389d79 ("Merge tag 'for-linus-4.19-ofs1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/hubcap/linux").
>
> Signed-off-by: Jason Wang
so there will be a new version of this as ioctl has changed, right?
> ---
> include/standard-headers/drm/drm_fourcc.h | 176
> +
> include/standard-headers/linux/ethtool.h | 11 +-
> include/standard-headers/linux/pci_regs.h | 4 +-
> include/standard-headers/linux/virtio_config.h | 16 ++-
> linux-headers/asm-generic/unistd.h | 4 +-
> linux-headers/asm-mips/unistd.h| 18 ++-
> linux-headers/asm-powerpc/kvm.h| 1 +
> linux-headers/asm-powerpc/unistd.h | 1 +
> linux-headers/asm-s390/unistd_32.h | 2 +
> linux-headers/asm-s390/unistd_64.h | 2 +
> linux-headers/linux/kvm.h | 1 +
> linux-headers/linux/vhost.h| 18 +++
> 12 files changed, 238 insertions(+), 16 deletions(-)
>
> diff --git a/include/standard-headers/drm/drm_fourcc.h
> b/include/standard-headers/drm/drm_fourcc.h
> index 11912fd..b53f8d7 100644
> --- a/include/standard-headers/drm/drm_fourcc.h
> +++ b/include/standard-headers/drm/drm_fourcc.h
> @@ -182,6 +182,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_QCOM0x05
> #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
> #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
> +#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> /* add more to the end as needed */
>
> #define DRM_FORMAT_RESERVED((1ULL << 56) - 1)
> @@ -297,6 +298,19 @@ extern "C" {
> */
> #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILEfourcc_mod_code(SAMSUNG, 1)
>
> +/*
> + * Qualcomm Compressed Format
> + *
> + * Refers to a compressed variant of the base format that is compressed.
> + * Implementation may be platform and base-format specific.
> + *
> + * Each macrotile consists of m x n (mostly 4 x 4) tiles.
> + * Pixel data pitch/stride is aligned with macrotile width.
> + * Pixel data height is aligned with macrotile height.
> + * Entire pixel data buffer is aligned with 4k(bytes).
> + */
> +#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1)
> +
> /* Vivante framebuffer modifiers */
>
> /*
> @@ -384,6 +398,23 @@ extern "C" {
> fourcc_mod_code(NVIDIA, 0x15)
>
> /*
> + * Some Broadcom modifiers take parameters, for example the number of
> + * vertical lines in the image. Reserve the lower 32 bits for modifier
> + * type, and the next 24 bits for parameters. Top 8 bits are the
> + * vendor code.
> + */
> +#define __fourcc_mod_broadcom_param_shift 8
> +#define __fourcc_mod_broadcom_param_bits 48
> +#define fourcc_mod_broadcom_code(val, params) \
> + fourcc_mod_code(BROADCOM, uint64_t)params) <<
> __fourcc_mod_broadcom_param_shift) | val))
> +#define fourcc_mod_broadcom_param(m) \
> + ((int)(((m) >> __fourcc_mod_broadcom_param_shift) & \
> +((1ULL << __fourcc_mod_broadcom_param_bits) - 1)))
> +#define fourcc_mod_broadcom_mod(m) \
> + ((m) & ~(((1ULL << __fourcc_mod_broadcom_param_bits) - 1) <<\
> + __fourcc_mod_broadcom_param_shift))
> +
> +/*
> * Broadcom VC4 "T" format
> *
> * This is the primary layout that the V3D GPU can texture from (it
> @@ -404,6 +435,151 @@ extern "C" {
> */
> #define DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED fourcc_mod_code(BROADCOM, 1)
>
> +/*
> + * Broadcom SAND format
> + *
> + * This is the native format that the H.264 codec block uses. For VC4
> + * HVS, it is only valid for H.264 (NV12/21) and RGBA modes.
> + *
> + * The image can be considered to be split into columns, and the
> + * columns are placed consecutively into memory. The width of those
> + * columns can be either 32, 64, 128, or 256 pixels, but in practice
> + * only 128 pixel columns are used.
> + *
> + * The pitch between the start of each column is set to optimally
> + * switch between SDRAM banks. This is passed as the number of lines
> + * of column width in the modifier (we can't use the stride value due
> + * to various core checks that look at it , so you should set the
> + * stride to width*cpp).
> + *
> + * Note that the column height for this format modifier is the same
> + * for all of the planes, assuming that each column contains both Y
> + * and UV. Some SAND-using hardware stores UV in a separate tiled
> + * image from Y to reduce the column height, which is not supported
> + * with these modifiers.
> + */
> +
> +#define DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(v) \
> + fourcc_mod_broadcom_code(2, v)
> +#define DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(v) \
> + fourcc_mod_broadcom_code(3, v)
> +#define DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(v) \
> + fourcc_mod_broadcom_code(4, v)
> +#define DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(v) \
> + fourcc_mod_broadcom_code(5, v)
> +
> +#define
[Qemu-devel] [PULL v1 0/1] Merge tpm 2018/09/07 v1
This patch fixes a TPM test failure in QEMU's test suite. Stefan The following changes since commit 19b599f7664b2ebfd0f405fb79c14dd241557452: Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging (2018-08-27 16:44:20 +0100) are available in the Git repository at: git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2018-09-07-1 for you to fetch changes up to 2271b75fa9019a9ba10d38ea2cf53a2032b1d8fd: tests: Fix signalling race condition in TPM tests (2018-09-07 16:37:47 -0400) Stefan Berger (1): tests: Fix signalling race condition in TPM tests tests/tpm-crb-test.c | 1 + tests/tpm-emu.c | 11 ++- tests/tpm-emu.h | 1 + tests/tpm-tis-test.c | 1 + 4 files changed, 13 insertions(+), 1 deletion(-) -- 2.14.4
[Qemu-devel] [PULL v1 1/1] tests: Fix signalling race condition in TPM tests
This patch fixes a race condition and test failure where the main process
waits for the signal of a thread but the thread already sent that signal
via a condition. Since these signals are non-sticky, we need to introduce a
separate variable to make this signal sticky.
Signed-off-by: Stefan Berger
Reviewed-by: Marc-André Lureau
---
tests/tpm-crb-test.c | 1 +
tests/tpm-emu.c | 11 ++-
tests/tpm-emu.h | 1 +
tests/tpm-tis-test.c | 1 +
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/tests/tpm-crb-test.c b/tests/tpm-crb-test.c
index d8f9569203..6fde579bab 100644
--- a/tests/tpm-crb-test.c
+++ b/tests/tpm-crb-test.c
@@ -151,6 +151,7 @@ int main(int argc, char **argv)
test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
g_mutex_init(_mutex);
g_cond_init(_cond);
+test.data_cond_signal = false;
thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
tpm_emu_test_wait_cond();
diff --git a/tests/tpm-emu.c b/tests/tpm-emu.c
index 8c2bd53cad..125e697181 100644
--- a/tests/tpm-emu.c
+++ b/tests/tpm-emu.c
@@ -23,9 +23,14 @@ void tpm_emu_test_wait_cond(TestState *s)
gint64 end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
g_mutex_lock(>data_mutex);
-if (!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
+
+if (!s->data_cond_signal &&
+!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
g_assert_not_reached();
}
+
+s->data_cond_signal = false;
+
g_mutex_unlock(>data_mutex);
}
@@ -72,6 +77,10 @@ void *tpm_emu_ctrl_thread(void *data)
QIOChannel *ioc;
qio_channel_socket_listen_sync(lioc, s->addr, _abort);
+
+g_mutex_lock(>data_mutex);
+s->data_cond_signal = true;
+g_mutex_unlock(>data_mutex);
g_cond_signal(>data_cond);
qio_channel_wait(QIO_CHANNEL(lioc), G_IO_IN);
diff --git a/tests/tpm-emu.h b/tests/tpm-emu.h
index 08f902485e..8eb802a79e 100644
--- a/tests/tpm-emu.h
+++ b/tests/tpm-emu.h
@@ -26,6 +26,7 @@ struct tpm_hdr {
typedef struct TestState {
GMutex data_mutex;
GCond data_cond;
+bool data_cond_signal;
SocketAddress *addr;
QIOChannel *tpm_ioc;
GThread *emu_tpm_thread;
diff --git a/tests/tpm-tis-test.c b/tests/tpm-tis-test.c
index 14754d9706..c8ec14888f 100644
--- a/tests/tpm-tis-test.c
+++ b/tests/tpm-tis-test.c
@@ -446,6 +446,7 @@ int main(int argc, char **argv)
test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
g_mutex_init(_mutex);
g_cond_init(_cond);
+test.data_cond_signal = false;
thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
tpm_emu_test_wait_cond();
--
2.14.4
Re: [Qemu-devel] [PATCH] pc: acpi: revert back to 1 SRAT entry for hotpluggable area
On Wed, Aug 22, 2018 at 11:46:44AM +0200, Igor Mammedov wrote:
> Commit
> 10efd7e108 "pc: acpi: fix memory hotplug regression by reducing stub SRAT
> entry size"
> attemped to fix hotplug regression introduced by
> 848a1cc1e "hw/acpi-build: build SRAT memory affinity structures for DIMM
> devices"
>
> fixed issue for Windows/3.0+ linux kernels, however it regressed 2.6 based
> kernels (RHEL6) to the point where guest might crash at boot.
> Reason is that 2.6 kernel discards SRAT table due too small last entry
> which down the road leads to crashes. Hack I've tried in 10efd7e108 is also
> not ACPI spec compliant according to which whole possible RAM should be
> described in SRAT. Revert 10efd7e108 to fix regression for 2.6 based kernels.
>
> With 10efd7e108 reverted, I've also tried splitting SRAT table statically
> in different ways %/node and %/slot but Windows still fails to online
> 2nd pc-dimm hot-plugged into node 0 (as described in 10efd7e108) and
> sometimes even coldplugged pc-dimms where affected with static SRAT
> partitioning.
> The only known so far way where Windows stays happy is when we have 1
> SRAT entry in the last node covering all hotplug area.
>
> Revert 848a1cc1e until we come up with a way to avoid regression
> on Windows with hotplug area split in several entries.
> Tested this with 2.6/3.0 based kernels (RHEL6/7) and WS20[08/12/12R2/16]).
>
> Signed-off-by: Igor Mammedov
BTW should this cause any aml test blobs to change?
Does not seem to ...
> ---
> CC: haozhong.zh...@intel.com
> CC: m...@redhat.com
> CC: qemu-sta...@nongnu.org
> CC: ehabk...@redhat.com
> CC: ler...@redhat.com
> ---
> hw/i386/acpi-build.c | 73
> +---
> 1 file changed, 12 insertions(+), 61 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e1ee8ae..1599caa 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2251,64 +2251,6 @@ build_tpm2(GArray *table_data, BIOSLinker *linker,
> GArray *tcpalog)
> #define HOLE_640K_START (640 * KiB)
> #define HOLE_640K_END (1 * MiB)
>
> -static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base,
> - uint64_t len, int default_node)
> -{
> -MemoryDeviceInfoList *info_list = qmp_memory_device_list();
> -MemoryDeviceInfoList *info;
> -MemoryDeviceInfo *mi;
> -PCDIMMDeviceInfo *di;
> -uint64_t end = base + len, cur, size;
> -bool is_nvdimm;
> -AcpiSratMemoryAffinity *numamem;
> -MemoryAffinityFlags flags;
> -
> -for (cur = base, info = info_list;
> - cur < end;
> - cur += size, info = info->next) {
> -numamem = acpi_data_push(table_data, sizeof *numamem);
> -
> -if (!info) {
> -/*
> - * Entry is required for Windows to enable memory hotplug in OS
> - * and for Linux to enable SWIOTLB when booted with less than
> - * 4G of RAM. Windows works better if the entry sets proximity
> - * to the highest NUMA node in the machine at the end of the
> - * reserved space.
> - * Memory devices may override proximity set by this entry,
> - * providing _PXM method if necessary.
> - */
> -build_srat_memory(numamem, end - 1, 1, default_node,
> - MEM_AFFINITY_HOTPLUGGABLE |
> MEM_AFFINITY_ENABLED);
> -break;
> -}
> -
> -mi = info->value;
> -is_nvdimm = (mi->type == MEMORY_DEVICE_INFO_KIND_NVDIMM);
> -di = !is_nvdimm ? mi->u.dimm.data : mi->u.nvdimm.data;
> -
> -if (cur < di->addr) {
> -build_srat_memory(numamem, cur, di->addr - cur, default_node,
> - MEM_AFFINITY_HOTPLUGGABLE |
> MEM_AFFINITY_ENABLED);
> -numamem = acpi_data_push(table_data, sizeof *numamem);
> -}
> -
> -size = di->size;
> -
> -flags = MEM_AFFINITY_ENABLED;
> -if (di->hotpluggable) {
> -flags |= MEM_AFFINITY_HOTPLUGGABLE;
> -}
> -if (is_nvdimm) {
> -flags |= MEM_AFFINITY_NON_VOLATILE;
> -}
> -
> -build_srat_memory(numamem, di->addr, size, di->node, flags);
> -}
> -
> -qapi_free_MemoryDeviceInfoList(info_list);
> -}
> -
> static void
> build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
> {
> @@ -2414,10 +2356,19 @@ build_srat(GArray *table_data, BIOSLinker *linker,
> MachineState *machine)
> build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
> }
>
> +/*
> + * Entry is required for Windows to enable memory hotplug in OS
> + * and for Linux to enable SWIOTLB when booted with less than
> + * 4G of RAM. Windows works better if the entry sets proximity
> + * to the highest NUMA node in the machine.
> + * Memory devices may override proximity set by this entry,
> + *
Re: [Qemu-devel] [PATCH] kvm: x86: Fix kvm_arch_fixup_msi_route for remap-less case
On Mon, Aug 27, 2018 at 10:47:51AM +0200, Jan Kiszka wrote:
> The AMD IOMMU does not (yet) support interrupt remapping. But
> kvm_arch_fixup_msi_route assumes that all implementations do and crashes
> when the AMD IOMMU is used in KVM mode.
>
> Fixes: 8b5ed7dffa1f ("intel_iommu: add support for split irqchip")
> Reported-by: Christopher Goldsworthy
> Signed-off-by: Jan Kiszka
Reviewed-by: Michael S. Tsirkin
> ---
> target/i386/kvm.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 9313602d3d..1fe3a73a10 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -3677,6 +3677,10 @@ int kvm_arch_fixup_msi_route(struct
> kvm_irq_routing_entry *route,
> MSIMessage src, dst;
> X86IOMMUClass *class = X86_IOMMU_GET_CLASS(iommu);
>
> +if (!class->int_remap) {
> +return 0;
> +}
> +
> src.address = route->u.msi.address_hi;
> src.address <<= VTD_MSI_ADDR_HI_SHIFT;
> src.address |= route->u.msi.address_lo;
> --
> 2.16.4
Re: [Qemu-devel] [PATCH v4 00/29] postcopy+vhost-user/shared ram
On Thu, Mar 08, 2018 at 07:57:42PM +, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > This set enables postcopy migration with shared memory to a vhost user > process. > It's based off current head. > > Testing is mostly performed with dpdk, with corresponding modifications by > Maxime. > > v4 is mostly just fixes from comments received during review of v3, > and the normal updating of all the virtio enum's to catch up with other > things getting in before us. > > Dave For vhost bits: Acked-by: Michael S. Tsirkin So how is all this going to be merged? If it's through my tree pls let me know. I'm fine with another tree though. > > Dr. David Alan Gilbert (29): > migrate: Update ram_block_discard_range for shared > qemu_ram_block_host_offset > postcopy: use UFFDIO_ZEROPAGE only when available > postcopy: Add notifier chain > postcopy: Add vhost-user flag for postcopy and check it > vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message > libvhost-user: Support sending fds back to qemu > libvhost-user: Open userfaultfd > postcopy: Allow registering of fd handler > vhost+postcopy: Register shared ufd with postcopy > vhost+postcopy: Transmit 'listen' to client > postcopy+vhost-user: Split set_mem_table for postcopy > migration/ram: ramblock_recv_bitmap_test_byte_offset > libvhost-user+postcopy: Register new regions with the ufd > vhost+postcopy: Send address back to qemu > vhost+postcopy: Stash RAMBlock and offset > vhost+postcopy: Send requests to source for shared pages > vhost+postcopy: Resolve client address > postcopy: helper for waking shared > postcopy: postcopy_notify_shared_wake > vhost+postcopy: Add vhost waker > vhost+postcopy: Call wakeups > libvhost-user: mprotect & madvises for postcopy > vhost-user: Add VHOST_USER_POSTCOPY_END message > vhost+postcopy: Wire up POSTCOPY_END notify > vhost: Huge page align and merge > postcopy: Allow shared memory > libvhost-user: Claim support for postcopy > postcopy shared docs > > contrib/libvhost-user/libvhost-user.c | 303 - > contrib/libvhost-user/libvhost-user.h | 11 + > docs/devel/migration.rst | 41 > docs/interop/vhost-user.txt | 51 + > exec.c| 86 +-- > hw/virtio/trace-events| 16 +- > hw/virtio/vhost-user.c| 411 > +- > hw/virtio/vhost.c | 66 +- > include/exec/cpu-common.h | 4 + > migration/migration.c | 6 + > migration/migration.h | 4 + > migration/postcopy-ram.c | 354 +++-- > migration/postcopy-ram.h | 69 ++ > migration/ram.c | 5 + > migration/ram.h | 1 + > migration/savevm.c| 13 ++ > migration/trace-events| 6 + > trace-events | 3 +- > vl.c | 2 + > 19 files changed, 1354 insertions(+), 98 deletions(-) > > -- > 2.14.3
Re: [Qemu-devel] [Bug 1776920] Re: qemu-img convert on Mac OSX creates corrupt images
On 09/07/2018 01:04 PM, Yan-Jie Wang wrote: I have done some experiments and find out that the behavior of lseek with whence set to SEEK_DATA is different from the behavior of Linux's lseek. If the supplied offset is in the middle of a data region, it returns the start of the next data region. There may be many data regions in a big file even though it has no hole. return value of lseek with whence set to SEEK_DATA: |--(offset)--Data|(return value)Data| |--(offset)--Data|Hole|(return value)Data| ** Patch added: "macOS-lseek.patch" https://bugs.launchpad.net/qemu/+bug/1776920/+attachment/5186138/+files/macOS-lseek.patch While a developer can chase a URL, our CI tools can't. Can you please also send that patch directly to qemu-devel@nongnu.org, so that it gets the same level of review as other patches? But I am very reluctant to take your patch. MacOS is flat-out buggy and in violation of the specification of lseek(SEEK_DATA), so making all applications work around their bug is not going to scale as well as having them fix their bug in the first place. Here's the proposed POSIX specification for what lseek(SEEK_DATA) is supposed to do: http://austingroupbugs.net/view.php?id=415#c862 That text has been present for 7 years now - so anyone implementing SEEK_DATA should really be paying attention to interoperability with that text. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900
Thank you, Richard,
> > Can QEMU be instructed to emulate
> > the FPU only for Linux user space programs as opposed to hardware emulation?
>
> Yes, that can be done. I would suggest something like
>
> /*
> * Hardware traps to the operating system for emulation.
> * For user-only, qemu is the operating system, so we
> * emulate the trap and emulate by simply emulating the
> * instruction directly.
> */
> #ifdef CONFIG_USER_ONLY
> # define CP0C1_FP_USER_ONLY (1 << CP0C1_FP)
> #else
> # define CP0C1_FP_USER_ONLY 0
> #endif
>
> and include that in your initialization of CP0_Config1.
I've made both CP0C1_FP and the LL/SC emulation conditional on user-only,
and adjusted the patch after the review by Maciej, as shown below.
Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?
Fredrik
Signed-off-by: Fredrik Noring
---
include/elf.h|3 ++
linux-user/mips/target_elf.h |3 ++
target/mips/mips-defs.h |2 +
target/mips/translate.c | 31 ++-
target/mips/translate_init.inc.c | 44 +++
5 files changed, 82 insertions(+), 1 deletion(-)
--- a/include/elf.h
+++ b/include/elf.h
@@ -48,6 +48,8 @@ typedef int64_t Elf64_Sxword;
#define EF_MIPS_ARCH_32R6 0x9000 /* MIPS32r6 code. */
#define EF_MIPS_ARCH_64R6 0xa000 /* MIPS64r6 code. */
+#define EF_MIPS_MACH_5900 0x0092
+
/* The ABI of a file. */
#define EF_MIPS_ABI_O320x1000 /* O32 ABI. */
#define EF_MIPS_ABI_O640x2000 /* O32 extended for 64
bit. */
@@ -62,6 +64,7 @@ typedef int64_t Elf64_Sxword;
#define EF_MIPS_FP64 0x0200
#define EF_MIPS_NAN2008 0x0400
#define EF_MIPS_ARCH 0xf000
+#define EF_MIPS_MACH 0x00ff
/* These constants define the different elf file types */
#define ET_NONE 0
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
return "mips32r6-generic";
}
+if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+return "R5900";
+}
return "24Kf";
}
#endif
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -52,6 +52,7 @@
#define ASE_MSA 0x0100
/* Chip specific instructions. */
+#defineINSN_R5900 0x1000
#defineINSN_LOONGSON2E 0x2000
#defineINSN_LOONGSON2F 0x4000
#defineINSN_VR54XX 0x8000
@@ -62,6 +63,7 @@
#defineCPU_MIPS3 (CPU_MIPS2 | ISA_MIPS3)
#defineCPU_MIPS4 (CPU_MIPS3 | ISA_MIPS4)
#defineCPU_VR54XX (CPU_MIPS4 | INSN_VR54XX)
+#defineCPU_R5900 (CPU_MIPS4 | INSN_R5900)
#defineCPU_LOONGSON2E (CPU_MIPS3 | INSN_LOONGSON2E)
#defineCPU_LOONGSON2F (CPU_MIPS3 | INSN_LOONGSON2F)
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3618,6 +3618,31 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
tcg_temp_free(t1);
}
+static void gen_mul_r5900 (DisasContext *ctx, uint32_t opc,
+int rd, int rs, int rt)
+{
+TCGv t0 = tcg_temp_new();
+TCGv t1 = tcg_temp_new();
+
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+
+switch (opc) {
+case OPC_MULT:
+case OPC_MULTU:
+tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+break;
+default:
+MIPS_INVAL("mul R5900");
+generate_exception_end(ctx, EXCP_RI);
+goto out;
+}
+
+ out:
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+}
+
static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
int rd, int rs, int rt)
{
@@ -17357,7 +17382,11 @@ static void decode_opc_special_legacy(CPUMIPSState
*env, DisasContext *ctx)
break;
case OPC_MULT:
case OPC_MULTU:
-if (sa) {
+if (ctx->insn_flags & INSN_R5900) {
+gen_muldiv(ctx, op1, 0, rs, rt);
+if (rd != 0)
+gen_mul_r5900(ctx, op1, rd, rs, rt);
+} else if (sa) {
check_insn(ctx, INSN_VR54XX);
op1 = MASK_MUL_VR54XX(ctx->opcode);
gen_mul_vr54xx(ctx, op1, rd, rs, rt);
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -410,6 +410,50 @@ const mips_def_t mips_defs[] =
.insn_flags = CPU_MIPS32R5 | ASE_MSA,
.mmu_type = MMU_TYPE_R4000,
},
+{
+.name = "R5900",
+.CP0_PRid = 0x3800,
+/* No L2 cache, icache size 32k, dcache size 32k, uncached coherency.
*/
+.CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
+/* Note: Config1 is only used internally, the R5900 has only Config0.
*/
+.CP0_Status_rw_bitmask
Re: [Qemu-devel] [PATCH v2 01/12] qdev-monitor: print help to stdout
Hi
On Fri, Sep 7, 2018 at 5:49 PM Eric Blake wrote:
>
> On 09/07/2018 02:59 AM, Marc-André Lureau wrote:
> > qdev_device_help() is used from command line "-device help", or from
> > HMP "device_add". If used from command line, print help to stdout
> > (it is only printed on explicit demand).
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/monitor/monitor.h | 2 ++
> > monitor.c | 16 +---
> > qdev-monitor.c| 32 +++-
> > 3 files changed, 34 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> > index 2ef5e04b37..e7667fda35 100644
> > --- a/include/monitor/monitor.h
> > +++ b/include/monitor/monitor.h
> > @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int
> > dup_fd);
> > void monitor_fdset_dup_fd_remove(int dup_fd);
> > int monitor_fdset_dup_fd_find(int dup_fd);
> >
> > +void out_vprintf(FILE *stream, const char *fmt, va_list ap)
> > GCC_FMT_ATTR(2, 0);
>
> Perhaps name this monitor_vfprintf()? (That would match the fact that
> you are using it like normal vfprintf).
>
> > +++ b/qdev-monitor.c
> > @@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc)
> > return (qdev_class_get_alias(dc) != NULL);
> > }
> >
> > +static void out_printf(const char *fmt, ...)
> > +{
> > +va_list ap;
> > +
> > +va_start(ap, fmt);
> > +out_vprintf(stdout, fmt, ap);
> > +va_end(ap);
> > +}
>
> But this name seems reasonable.
>
> And I just now see that Thomas also complained about the monitor.h
> naming, but I like 'monitor_vfprintf' better than his 'mon_file_vprintf'.
>
> If an improved name is your only change,
> Reviewed-by: Eric Blake
I'll change it to monitor_vfprintf,
thanks
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH] tests: Fix signalling race condition in TPM tests
On Fri, Sep 7, 2018 at 10:47 PM Stefan Berger
wrote:
>
> This patch fixes a race condition and test failure where the main process
> waits for the signal of a thread but the thread already sent that signal
> via a condition. Since these signals are non-sticky, we need to introduce a
> separate variable to make this signal sticky.
>
> Signed-off-by: Stefan Berger
Reviewed-by: Marc-André Lureau
> ---
> tests/tpm-crb-test.c | 1 +
> tests/tpm-emu.c | 11 ++-
> tests/tpm-emu.h | 1 +
> tests/tpm-tis-test.c | 1 +
> 4 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/tests/tpm-crb-test.c b/tests/tpm-crb-test.c
> index d8f9569203..6fde579bab 100644
> --- a/tests/tpm-crb-test.c
> +++ b/tests/tpm-crb-test.c
> @@ -151,6 +151,7 @@ int main(int argc, char **argv)
> test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
> g_mutex_init(_mutex);
> g_cond_init(_cond);
> +test.data_cond_signal = false;
>
> thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
> tpm_emu_test_wait_cond();
> diff --git a/tests/tpm-emu.c b/tests/tpm-emu.c
> index 8c2bd53cad..125e697181 100644
> --- a/tests/tpm-emu.c
> +++ b/tests/tpm-emu.c
> @@ -23,9 +23,14 @@ void tpm_emu_test_wait_cond(TestState *s)
> gint64 end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
>
> g_mutex_lock(>data_mutex);
> -if (!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
> +
> +if (!s->data_cond_signal &&
> +!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
> g_assert_not_reached();
> }
> +
> +s->data_cond_signal = false;
> +
> g_mutex_unlock(>data_mutex);
> }
>
> @@ -72,6 +77,10 @@ void *tpm_emu_ctrl_thread(void *data)
> QIOChannel *ioc;
>
> qio_channel_socket_listen_sync(lioc, s->addr, _abort);
> +
> +g_mutex_lock(>data_mutex);
> +s->data_cond_signal = true;
> +g_mutex_unlock(>data_mutex);
> g_cond_signal(>data_cond);
>
> qio_channel_wait(QIO_CHANNEL(lioc), G_IO_IN);
> diff --git a/tests/tpm-emu.h b/tests/tpm-emu.h
> index 08f902485e..8eb802a79e 100644
> --- a/tests/tpm-emu.h
> +++ b/tests/tpm-emu.h
> @@ -26,6 +26,7 @@ struct tpm_hdr {
> typedef struct TestState {
> GMutex data_mutex;
> GCond data_cond;
> +bool data_cond_signal;
> SocketAddress *addr;
> QIOChannel *tpm_ioc;
> GThread *emu_tpm_thread;
> diff --git a/tests/tpm-tis-test.c b/tests/tpm-tis-test.c
> index 14754d9706..c8ec14888f 100644
> --- a/tests/tpm-tis-test.c
> +++ b/tests/tpm-tis-test.c
> @@ -446,6 +446,7 @@ int main(int argc, char **argv)
> test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
> g_mutex_init(_mutex);
> g_cond_init(_cond);
> +test.data_cond_signal = false;
>
> thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
> tpm_emu_test_wait_cond();
> --
> 2.14.4
>
>
--
Marc-André Lureau
[Qemu-devel] [PATCH] tests: Fix signalling race condition in TPM tests
This patch fixes a race condition and test failure where the main process
waits for the signal of a thread but the thread already sent that signal
via a condition. Since these signals are non-sticky, we need to introduce a
separate variable to make this signal sticky.
Signed-off-by: Stefan Berger
---
tests/tpm-crb-test.c | 1 +
tests/tpm-emu.c | 11 ++-
tests/tpm-emu.h | 1 +
tests/tpm-tis-test.c | 1 +
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/tests/tpm-crb-test.c b/tests/tpm-crb-test.c
index d8f9569203..6fde579bab 100644
--- a/tests/tpm-crb-test.c
+++ b/tests/tpm-crb-test.c
@@ -151,6 +151,7 @@ int main(int argc, char **argv)
test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
g_mutex_init(_mutex);
g_cond_init(_cond);
+test.data_cond_signal = false;
thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
tpm_emu_test_wait_cond();
diff --git a/tests/tpm-emu.c b/tests/tpm-emu.c
index 8c2bd53cad..125e697181 100644
--- a/tests/tpm-emu.c
+++ b/tests/tpm-emu.c
@@ -23,9 +23,14 @@ void tpm_emu_test_wait_cond(TestState *s)
gint64 end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
g_mutex_lock(>data_mutex);
-if (!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
+
+if (!s->data_cond_signal &&
+!g_cond_wait_until(>data_cond, >data_mutex, end_time)) {
g_assert_not_reached();
}
+
+s->data_cond_signal = false;
+
g_mutex_unlock(>data_mutex);
}
@@ -72,6 +77,10 @@ void *tpm_emu_ctrl_thread(void *data)
QIOChannel *ioc;
qio_channel_socket_listen_sync(lioc, s->addr, _abort);
+
+g_mutex_lock(>data_mutex);
+s->data_cond_signal = true;
+g_mutex_unlock(>data_mutex);
g_cond_signal(>data_cond);
qio_channel_wait(QIO_CHANNEL(lioc), G_IO_IN);
diff --git a/tests/tpm-emu.h b/tests/tpm-emu.h
index 08f902485e..8eb802a79e 100644
--- a/tests/tpm-emu.h
+++ b/tests/tpm-emu.h
@@ -26,6 +26,7 @@ struct tpm_hdr {
typedef struct TestState {
GMutex data_mutex;
GCond data_cond;
+bool data_cond_signal;
SocketAddress *addr;
QIOChannel *tpm_ioc;
GThread *emu_tpm_thread;
diff --git a/tests/tpm-tis-test.c b/tests/tpm-tis-test.c
index 14754d9706..c8ec14888f 100644
--- a/tests/tpm-tis-test.c
+++ b/tests/tpm-tis-test.c
@@ -446,6 +446,7 @@ int main(int argc, char **argv)
test.addr->u.q_unix.path = g_build_filename(tmp_path, "sock", NULL);
g_mutex_init(_mutex);
g_cond_init(_cond);
+test.data_cond_signal = false;
thread = g_thread_new(NULL, tpm_emu_ctrl_thread, );
tpm_emu_test_wait_cond();
--
2.14.4
Re: [Qemu-devel] [Qemu-arm] [PATCH 05/11] aspeed/smc: fix some alignment issues
On 8/31/18 7:38 AM, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/ssi/aspeed_smc.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index b29bfd3124a9..1270842dcf0c 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -388,8 +388,8 @@ static uint64_t aspeed_smc_flash_default_read(void
> *opaque, hwaddr addr,
> static void aspeed_smc_flash_default_write(void *opaque, hwaddr addr,
> uint64_t data, unsigned size)
> {
> - qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u: 0x%"
> - PRIx64 "\n", __func__, addr, size, data);
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: To 0x%" HWADDR_PRIx " of size %u:
> 0x%"
> + PRIx64 "\n", __func__, addr, size, data);
> }
>
> static const MemoryRegionOps aspeed_smc_flash_default_ops = {
> @@ -529,7 +529,7 @@ static void aspeed_smc_flash_setup(AspeedSMCFlash *fl,
> uint32_t addr)
> */
> if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
> for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
> -ssi_transfer(fl->controller->spi, 0xFF);
> +ssi_transfer(fl->controller->spi, 0xFF);
> }
> }
> }
> @@ -567,7 +567,7 @@ static uint64_t aspeed_smc_flash_read(void *opaque,
> hwaddr addr, unsigned size)
> }
>
> static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
> - unsigned size)
> + unsigned size)
> {
> AspeedSMCFlash *fl = opaque;
> AspeedSMCState *s = fl->controller;
>
Re: [Qemu-devel] [PATCH] hw/arm/exynos4210: fix Exynos4210 UART support
On 8/31/18 7:23 AM, Bartlomiej Zolnierkiewicz wrote:
> commit 97274d0c05d4 ("hw/char/exynos4210_uart.c: Remove unneeded
> handling of NULL chardev") broke Exynos4210 support as it removed
> NULL 'Chardev *chr' handling from exynos4210_uart_create() and
> currently exynos4210_init() always passes NULL as 'Chardev *chr'
> argument to exynos4210_uart_create() calls. Fix it by adding
> missing serial_hd() calls to exynos4210_init().
>
> Signed-off-by: Bartlomiej Zolnierkiewicz
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/arm/exynos4210.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> Index: b/hw/arm/exynos4210.c
> ===
> --- a/hw/arm/exynos4210.c 2018-08-22 12:21:06.328813907 +0200
> +++ b/hw/arm/exynos4210.c 2018-08-22 13:00:54.344771039 +0200
> @@ -352,19 +352,19 @@ Exynos4210State *exynos4210_init(MemoryR
>
> /*** UARTs ***/
> exynos4210_uart_create(EXYNOS4210_UART0_BASE_ADDR,
> - EXYNOS4210_UART0_FIFO_SIZE, 0, NULL,
> + EXYNOS4210_UART0_FIFO_SIZE, 0, serial_hd(0),
>s->irq_table[exynos4210_get_irq(EXYNOS4210_UART_INT_GRP,
> 0)]);
>
> exynos4210_uart_create(EXYNOS4210_UART1_BASE_ADDR,
> - EXYNOS4210_UART1_FIFO_SIZE, 1, NULL,
> + EXYNOS4210_UART1_FIFO_SIZE, 1, serial_hd(1),
>s->irq_table[exynos4210_get_irq(EXYNOS4210_UART_INT_GRP,
> 1)]);
>
> exynos4210_uart_create(EXYNOS4210_UART2_BASE_ADDR,
> - EXYNOS4210_UART2_FIFO_SIZE, 2, NULL,
> + EXYNOS4210_UART2_FIFO_SIZE, 2, serial_hd(2),
>s->irq_table[exynos4210_get_irq(EXYNOS4210_UART_INT_GRP,
> 2)]);
>
> exynos4210_uart_create(EXYNOS4210_UART3_BASE_ADDR,
> - EXYNOS4210_UART3_FIFO_SIZE, 3, NULL,
> + EXYNOS4210_UART3_FIFO_SIZE, 3, serial_hd(3),
>s->irq_table[exynos4210_get_irq(EXYNOS4210_UART_INT_GRP,
> 3)]);
>
> /*** SD/MMC host controllers ***/
>
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [Bug 1776920] Re: qemu-img convert on Mac OSX creates corrupt images
I have done some experiments and find out that the behavior of lseek with whence set to SEEK_DATA is different from the behavior of Linux's lseek. If the supplied offset is in the middle of a data region, it returns the start of the next data region. There may be many data regions in a big file even though it has no hole. return value of lseek with whence set to SEEK_DATA: |--(offset)--Data|(return value)Data| |--(offset)--Data|Hole|(return value)Data| ** Patch added: "macOS-lseek.patch" https://bugs.launchpad.net/qemu/+bug/1776920/+attachment/5186138/+files/macOS-lseek.patch -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1776920 Title: qemu-img convert on Mac OSX creates corrupt images Status in QEMU: New Bug description: An image created by qemu-img create, then modified by another program is converted to bad/corrupt image when using convert sub command on Mac OSX. The same convert works on Linux. The version of qemu-img is 2.12. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1776920/+subscriptions
[Qemu-devel] [Bug 1790975] Re: Default arm virt machine broken
LPAE is actually disabled in my kernel config. Knowing the cause now, I can see that qemu would not be able to detect this problem. This error should have been detected in the linux kernel with an indication that the ECAM window was using a 40-bit address but LPAE was not enabled. ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1790975 Title: Default arm virt machine broken Status in QEMU: Invalid Bug description: This occurs on qemu_v3.0.0 but not on qemu_v2.12.2 (built from qemu_v3.0.0 tag on github) Symptom: You'll see something like this in the kernel output: [1.285210] OF: PCI: host bridge /pcie@1000 ranges: [1.286246] OF: PCI:IO 0x3eff..0x3eff -> 0x [1.287061] OF: PCI: MEM 0x1000..0x3efe -> 0x1000 [1.287820] OF: PCI: MEM 0x80..0xff -> 0x80 [1.289312] pci-host-generic 401000.pcie: can't claim ECAM area [mem 0x1000-0x1fff]: address conflict with /pcie@1000 [mem 0x1000-0x3efe] [1.290984] pci-host-generic: probe of 401000.pcie failed with error -16 Qemu Command Line: qemu-system-arm -machine virt -m 1024M -kernel zImage -serial stdio I can post my zImage if anyone has problems reproducing with their own zImage. Note that this problem breaks pci making the machine unusable. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1790975/+subscriptions
[Qemu-devel] [PATCH 14/14] test-bdrv-drain: Test nested poll in bdrv_drain_poll_top_level()
This is a regression test for a deadlock that could occur in callbacks
called from the aio_poll() in bdrv_drain_poll_top_level(). The
AioContext lock wasn't released and therefore would be taken a second
time in the callback. This would cause a possible AIO_WAIT_WHILE() in
the callback to hang.
Signed-off-by: Kevin Wolf
---
tests/test-bdrv-drain.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index 9641a20dd8..07ce5bfddc 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -636,6 +636,17 @@ static void test_iothread_aio_cb(void *opaque, int ret)
qemu_event_set(_event);
}
+static void test_iothread_main_thread_bh(void *opaque)
+{
+struct test_iothread_data *data = opaque;
+
+/* Test that the AioContext is not yet locked in a random BH that is
+ * executed during drain, otherwise this would deadlock. */
+aio_context_acquire(bdrv_get_aio_context(data->bs));
+bdrv_flush(data->bs);
+aio_context_release(bdrv_get_aio_context(data->bs));
+}
+
/*
* Starts an AIO request on a BDS that runs in the AioContext of iothread 1.
* The request involves a BH on iothread 2 before it can complete.
@@ -705,6 +716,8 @@ static void test_iothread_common(enum drain_type
drain_type, int drain_thread)
aio_context_acquire(ctx_a);
}
+aio_bh_schedule_oneshot(ctx_a, test_iothread_main_thread_bh, );
+
/* The request is running on the IOThread a. Draining its block device
* will make sure that it has completed as far as the BDS is concerned,
* but the drain in this thread can continue immediately after
--
2.13.6
[Qemu-devel] [PATCH 08/14] block-backend: Add .drained_poll callback
A bdrv_drain operation must ensure that all parents are quiesced, this
includes BlockBackends. Otherwise, callbacks called by requests that are
completed on the BDS layer, but not quite yet on the BlockBackend layer
could still create new requests.
Signed-off-by: Kevin Wolf
---
block/block-backend.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index fa120630be..efa8d8011c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -121,6 +121,7 @@ static void blk_root_inherit_options(int *child_flags,
QDict *child_options,
abort();
}
static void blk_root_drained_begin(BdrvChild *child);
+static bool blk_root_drained_poll(BdrvChild *child);
static void blk_root_drained_end(BdrvChild *child);
static void blk_root_change_media(BdrvChild *child, bool load);
@@ -294,6 +295,7 @@ static const BdrvChildRole child_root = {
.get_parent_desc= blk_root_get_parent_desc,
.drained_begin = blk_root_drained_begin,
+.drained_poll = blk_root_drained_poll,
.drained_end= blk_root_drained_end,
.activate = blk_root_activate,
@@ -2191,6 +2193,13 @@ static void blk_root_drained_begin(BdrvChild *child)
}
}
+static bool blk_root_drained_poll(BdrvChild *child)
+{
+BlockBackend *blk = child->opaque;
+assert(blk->quiesce_counter);
+return !!blk->in_flight;
+}
+
static void blk_root_drained_end(BdrvChild *child)
{
BlockBackend *blk = child->opaque;
--
2.13.6
[Qemu-devel] [PATCH 10/14] block-backend: Decrease in_flight only after callback
Request callbacks can do pretty much anything, including operations that
will yield from the coroutine (such as draining the backend). In that
case, a decreased in_flight would be visible to other code and could
lead to a drain completing while the callback hasn't actually completed
yet.
Signed-off-by: Kevin Wolf
---
block/block-backend.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 1b2d7a6ff5..2efaf0c968 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1338,8 +1338,16 @@ static const AIOCBInfo blk_aio_em_aiocb_info = {
static void blk_aio_complete(BlkAioEmAIOCB *acb)
{
if (acb->has_returned) {
-blk_dec_in_flight(acb->rwco.blk);
+if (qemu_get_current_aio_context() == qemu_get_aio_context()) {
+/* If we are in the main thread, the callback is allowed to unref
+ * the BlockBackend, so we have to hold an additional reference */
+blk_ref(acb->rwco.blk);
+}
acb->common.cb(acb->common.opaque, acb->rwco.ret);
+blk_dec_in_flight(acb->rwco.blk);
+if (qemu_get_current_aio_context() == qemu_get_aio_context()) {
+blk_unref(acb->rwco.blk);
+}
qemu_aio_unref(acb);
}
}
--
2.13.6
[Qemu-devel] [PATCH 12/14] blockjob: Lie better in child_job_drained_poll()
Block jobs claim in .drained_poll() that they are in a quiescent state
as soon as job->deferred_to_main_loop is true. This is obviously wrong,
they still have a completion BH to run. We only get away with this
because commit 91af091f923 added an unconditional aio_poll(false) to the
drain functions, but this is bypassing the regular drain mechanisms.
However, just removing this and telling that the job is still active
doesn't work either: The completion callbacks themselves call drain
functions (directly, or indirectly with bdrv_reopen), so they would
deadlock then.
As a better lie, tell that the job is active as long as the BH is
pending, but falsely call it quiescent from the point in the BH when the
completion callback is called. At this point, nested drain calls won't
deadlock because they ignore the job, and outer drains will wait for the
job to really reach a quiescent state because the callback is already
running.
Signed-off-by: Kevin Wolf
---
include/qemu/job.h | 3 +++
blockjob.c | 2 +-
job.c | 8
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 8ac48dbd28..5a49d6f9a5 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -76,6 +76,9 @@ typedef struct Job {
* Set to false by the job while the coroutine has yielded and may be
* re-entered by job_enter(). There may still be I/O or event loop activity
* pending. Accessed under block_job_mutex (in blockjob.c).
+ *
+ * When the job is deferred to the main loop, busy is true as long as the
+ * bottom half is still pending.
*/
bool busy;
diff --git a/blockjob.c b/blockjob.c
index 8d27e8e1ea..617d86fe93 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -164,7 +164,7 @@ static bool child_job_drained_poll(BdrvChild *c)
/* An inactive or completed job doesn't have any pending requests. Jobs
* with !job->busy are either already paused or have a pause point after
* being reentered, so no job driver code will run before they pause. */
-if (!job->busy || job_is_completed(job) || job->deferred_to_main_loop) {
+if (!job->busy || job_is_completed(job)) {
return false;
}
diff --git a/job.c b/job.c
index 8480eda188..d05795843b 100644
--- a/job.c
+++ b/job.c
@@ -978,6 +978,13 @@ static void job_defer_to_main_loop_bh(void *opaque)
AioContext *aio_context = job->aio_context;
aio_context_acquire(aio_context);
+
+/* This is a lie, we're not quiescent, but still doing the completion
+ * callbacks. However, completion callbacks tend to involve operations that
+ * drain block nodes, and if .drained_poll still returned true, we would
+ * deadlock. */
+job->busy = false;
+
data->fn(data->job, data->opaque);
aio_context_release(aio_context);
@@ -991,6 +998,7 @@ void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn
*fn, void *opaque)
data->fn = fn;
data->opaque = opaque;
job->deferred_to_main_loop = true;
+job->busy = true;
aio_bh_schedule_oneshot(qemu_get_aio_context(),
job_defer_to_main_loop_bh, data);
--
2.13.6
[Qemu-devel] [PATCH 13/14] block: Remove aio_poll() in bdrv_drain_poll variants
bdrv_drain_poll_top_level() was buggy because it didn't release the
AioContext lock of the node to be drained before calling aio_poll().
This way, callbacks called by aio_poll() would possibly take the lock a
second time and run into a deadlock with a nested AIO_WAIT_WHILE() call.
However, it turns out that the aio_poll() call isn't actually needed any
more. It was introduced in commit 91af091f923, which is effectively
reverted by this patch. The cases it was supposed to fix are now covered
by bdrv_drain_poll(), which waits for block jobs to reach a quiescent
state.
Signed-off-by: Kevin Wolf
---
block/io.c | 8
1 file changed, 8 deletions(-)
diff --git a/block/io.c b/block/io.c
index 914ba78f1a..8b81ff3913 100644
--- a/block/io.c
+++ b/block/io.c
@@ -268,10 +268,6 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive,
static bool bdrv_drain_poll_top_level(BlockDriverState *bs, bool recursive,
BdrvChild *ignore_parent)
{
-/* Execute pending BHs first and check everything else only after the BHs
- * have executed. */
-while (aio_poll(bs->aio_context, false));
-
return bdrv_drain_poll(bs, recursive, ignore_parent, false);
}
@@ -511,10 +507,6 @@ static bool bdrv_drain_all_poll(void)
BlockDriverState *bs = NULL;
bool result = false;
-/* Execute pending BHs first (may modify the graph) and check everything
- * else only after the BHs have executed. */
-while (aio_poll(qemu_get_aio_context(), false));
-
/* bdrv_drain_poll() can't make changes to the graph and we are holding the
* main AioContext lock, so iterating bdrv_next_all_states() is safe. */
while ((bs = bdrv_next_all_states(bs))) {
--
2.13.6
[Qemu-devel] [PATCH 05/14] test-bdrv-drain: Test AIO_WAIT_WHILE() in completion callback
This is a regression test for a deadlock that occurred in block job
completion callbacks (via job_defer_to_main_loop) because the AioContext
lock was taken twice: once in job_finish_sync() and then again in
job_defer_to_main_loop_bh(). This would cause AIO_WAIT_WHILE() to hang.
Signed-off-by: Kevin Wolf
---
tests/test-bdrv-drain.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index ab055e85f8..9641a20dd8 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -776,6 +776,11 @@ typedef struct TestBlockJob {
static void test_job_completed(Job *job, void *opaque)
{
+TestBlockJob *s = container_of(job, TestBlockJob, common.job);
+
+/* Provoke an AIO_WAIT_WHILE() call to verify there is no deadlock */
+blk_flush(s->common.blk);
+
job_completed(job, 0, NULL);
}
--
2.13.6
[Qemu-devel] [PATCH 06/14] block: Add missing locking in bdrv_co_drain_bh_cb()
bdrv_do_drained_begin/end() assume that they are called with the
AioContext lock of bs held. If we call drain functions from a coroutine
with the AioContext lock held, we yield and schedule a BH to move out of
coroutine context. This means that the lock for the home context of the
coroutine is released and must be re-acquired in the bottom half.
Signed-off-by: Kevin Wolf
---
include/qemu/coroutine.h | 5 +
block/io.c | 15 +++
util/qemu-coroutine.c| 5 +
3 files changed, 25 insertions(+)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 6f8a487041..9801e7f5a4 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -90,6 +90,11 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine
*co);
void coroutine_fn qemu_coroutine_yield(void);
/**
+ * Get the AioContext of the given coroutine
+ */
+AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co);
+
+/**
* Get the currently executing coroutine
*/
Coroutine *coroutine_fn qemu_coroutine_self(void);
diff --git a/block/io.c b/block/io.c
index 7100344c7b..914ba78f1a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -288,6 +288,18 @@ static void bdrv_co_drain_bh_cb(void *opaque)
BlockDriverState *bs = data->bs;
if (bs) {
+AioContext *ctx = bdrv_get_aio_context(bs);
+AioContext *co_ctx = qemu_coroutine_get_aio_context(co);
+
+/*
+ * When the coroutine yielded, the lock for its home context was
+ * released, so we need to re-acquire it here. If it explicitly
+ * acquired a different context, the lock is still held and we don't
+ * want to lock it a second time (or AIO_WAIT_WHILE() would hang).
+ */
+if (ctx == co_ctx) {
+aio_context_acquire(ctx);
+}
bdrv_dec_in_flight(bs);
if (data->begin) {
bdrv_do_drained_begin(bs, data->recursive, data->parent,
@@ -296,6 +308,9 @@ static void bdrv_co_drain_bh_cb(void *opaque)
bdrv_do_drained_end(bs, data->recursive, data->parent,
data->ignore_bds_parents);
}
+if (ctx == co_ctx) {
+aio_context_release(ctx);
+}
} else {
assert(data->begin);
bdrv_drain_all_begin();
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 1ba4191b84..2295928d33 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -198,3 +198,8 @@ bool qemu_coroutine_entered(Coroutine *co)
{
return co->caller;
}
+
+AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co)
+{
+return co->ctx;
+}
--
2.13.6
[Qemu-devel] [PATCH 01/14] blockjob: Wake up BDS when job becomes idle
In the context of draining a BDS, the .drained_poll callback of block
jobs is called. If this returns true (i.e. there is still some activity
pending), the drain operation may call aio_poll() with blocking=true to
wait for completion.
As soon as the pending activity is completed and the job finally arrives
in a quiescent state (i.e. its coroutine either yields with busy=false
or terminates), the block job must notify the aio_poll() loop to wake
up, otherwise we get a deadlock if both are running in different
threads.
Signed-off-by: Kevin Wolf
---
include/block/blockjob.h | 13 +
include/qemu/job.h | 3 +++
blockjob.c | 18 ++
job.c| 7 +++
4 files changed, 41 insertions(+)
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 32c00b7dc0..2290bbb824 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -70,6 +70,9 @@ typedef struct BlockJob {
/** Called when the job transitions to READY */
Notifier ready_notifier;
+/** Called when the job coroutine yields or terminates */
+Notifier idle_notifier;
+
/** BlockDriverStates that are involved in this block job */
GSList *nodes;
} BlockJob;
@@ -119,6 +122,16 @@ int block_job_add_bdrv(BlockJob *job, const char *name,
BlockDriverState *bs,
void block_job_remove_all_bdrv(BlockJob *job);
/**
+ * block_job_wakeup_all_bdrv:
+ * @job: The block job
+ *
+ * Calls bdrv_wakeup() for all BlockDriverStates that have been added to the
+ * job. This function is to be called whenever child_job_drained_poll() would
+ * go from true to false to notify waiting drain requests.
+ */
+void block_job_wakeup_all_bdrv(BlockJob *job);
+
+/**
* block_job_set_speed:
* @job: The job to set the speed for.
* @speed: The new value
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 18c9223e31..0dae5b8481 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -148,6 +148,9 @@ typedef struct Job {
/** Notifiers called when the job transitions to READY */
NotifierList on_ready;
+/** Notifiers called when the job coroutine yields or terminates */
+NotifierList on_idle;
+
/** Element of the list of jobs */
QLIST_ENTRY(Job) job_list;
diff --git a/blockjob.c b/blockjob.c
index be5903aa96..8d27e8e1ea 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -221,6 +221,22 @@ int block_job_add_bdrv(BlockJob *job, const char *name,
BlockDriverState *bs,
return 0;
}
+void block_job_wakeup_all_bdrv(BlockJob *job)
+{
+GSList *l;
+
+for (l = job->nodes; l; l = l->next) {
+BdrvChild *c = l->data;
+bdrv_wakeup(c->bs);
+}
+}
+
+static void block_job_on_idle(Notifier *n, void *opaque)
+{
+BlockJob *job = opaque;
+block_job_wakeup_all_bdrv(job);
+}
+
bool block_job_is_internal(BlockJob *job)
{
return (job->job.id == NULL);
@@ -419,6 +435,7 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
job->finalize_completed_notifier.notify = block_job_event_completed;
job->pending_notifier.notify = block_job_event_pending;
job->ready_notifier.notify = block_job_event_ready;
+job->idle_notifier.notify = block_job_on_idle;
notifier_list_add(>job.on_finalize_cancelled,
>finalize_cancelled_notifier);
@@ -426,6 +443,7 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
>finalize_completed_notifier);
notifier_list_add(>job.on_pending, >pending_notifier);
notifier_list_add(>job.on_ready, >ready_notifier);
+notifier_list_add(>job.on_idle, >idle_notifier);
error_setg(>blocker, "block device is in use by block job: %s",
job_type_str(>job));
diff --git a/job.c b/job.c
index e36ebaafd8..9ad0b7476a 100644
--- a/job.c
+++ b/job.c
@@ -410,6 +410,11 @@ static void job_event_ready(Job *job)
notifier_list_notify(>on_ready, job);
}
+static void job_event_idle(Job *job)
+{
+notifier_list_notify(>on_idle, job);
+}
+
void job_enter_cond(Job *job, bool(*fn)(Job *job))
{
if (!job_started(job)) {
@@ -455,6 +460,7 @@ static void coroutine_fn job_do_yield(Job *job, uint64_t ns)
timer_mod(>sleep_timer, ns);
}
job->busy = false;
+job_event_idle(job);
job_unlock();
qemu_coroutine_yield();
@@ -547,6 +553,7 @@ static void coroutine_fn job_co_entry(void *opaque)
assert(job && job->driver && job->driver->start);
job_pause_point(job);
job->driver->start(job);
+job_event_idle(job);
}
--
2.13.6
[Qemu-devel] [PATCH 11/14] mirror: Fix potential use-after-free in active commit
When starting an active commit job, other callbacks can run before
mirror_start_job() calls bdrv_ref() where needed and cause the nodes to
go away. Add another pair of bdrv_ref/unref() around it to protect
against this case.
Signed-off-by: Kevin Wolf
---
block/mirror.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 6cc10df5c9..c42999eadf 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1679,6 +1679,11 @@ void commit_active_start(const char *job_id,
BlockDriverState *bs,
orig_base_flags = bdrv_get_flags(base);
+/* bdrv_reopen() drains, which might make the BDSes go away before a
+ * reference is taken in mirror_start_job(). */
+bdrv_ref(bs);
+bdrv_ref(base);
+
if (bdrv_reopen(base, bs->open_flags, errp)) {
return;
}
@@ -1689,6 +1694,10 @@ void commit_active_start(const char *job_id,
BlockDriverState *bs,
_active_job_driver, false, base, auto_complete,
filter_node_name, false, MIRROR_COPY_MODE_BACKGROUND,
_err);
+
+bdrv_unref(bs);
+bdrv_unref(base);
+
if (local_err) {
error_propagate(errp, local_err);
goto error_restore_flags;
--
2.13.6
[Qemu-devel] [PATCH 07/14] aio-wait: Increase num_waiters even in home thread
Even if AIO_WAIT_WHILE() is called in the home context of the
AioContext, we still want to allow the condition to change depending on
other threads as long as they kick the AioWait. Specfically block jobs
can be running in an I/O thread and should then be able to kick a drain
in the main loop context.
Signed-off-by: Kevin Wolf
---
include/block/aio-wait.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index c85a62f798..46ba7f9111 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -77,10 +77,12 @@ typedef struct {
AioWait *wait_ = (wait); \
AioContext *ctx_ = (ctx); \
if (ctx_ && in_aio_context_home_thread(ctx_)) {\
+atomic_inc(_->num_waiters); \
while ((cond)) { \
aio_poll(ctx_, true); \
waited_ = true;\
} \
+atomic_dec(_->num_waiters); \
} else { \
assert(qemu_get_current_aio_context() == \
qemu_get_aio_context());\
--
2.13.6
[Qemu-devel] [PATCH 00/14] Fix some jobs/drain/aio_poll related hangs
Especially the combination of iothreads, block jobs and drain tends to
lead to hangs currently. This series fixes a few of these bugs, although
there are more of them, to be addressed in separate patches.
The primary goal of this series is to fix the scenario from:
https://bugzilla.redhat.com/show_bug.cgi?id=1601212
A simplified reproducer of the reported problem looks like this (two concurrent
commit block jobs for disks in an iothread):
$qemu -qmp stdio \
-object iothread,id=iothread1 \
-device
virtio-scsi-pci,id=virtio_scsi_pci0,bus=pci.0,addr=0x6,iothread=iothread1 \
-drive
id=drive_image1,if=none,snapshot=off,aio=threads,cache=none,format=qcow2,file=hd0
\
-device scsi-hd,drive=drive_image1,id=image1,bootindex=1 \
-drive
id=drive_image2,if=none,snapshot=off,aio=threads,cache=none,format=qcow2,file=hd1
\
-device scsi-hd,drive=drive_image2,id=image2,bootindex=2
{"execute":"qmp_capabilities"}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image1","snapshot-file":"sn1"}}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image1","snapshot-file":"sn11"}}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image1","snapshot-file":"sn111"}}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image2","snapshot-file":"sn2"}}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image2","snapshot-file":"sn22"}}
{"execute":"blockdev-snapshot-sync","arguments":{"device":"drive_image2","snapshot-file":"sn222"}}
{ "execute": "block-commit", "arguments": { "device":
"drive_image2","base":"sn2","backing-file":"sn2","top":"sn22"}}
{ "execute": "block-commit", "arguments": { "device":
"drive_image1","base":"sn1","backing-file":"sn1","top":"sn11"}}
{"execute":"quit"}
Kevin Wolf (14):
blockjob: Wake up BDS when job becomes idle
test-bdrv-drain: Drain with block jobs in an I/O thread
test-blockjob: Acquire AioContext around job_finish_sync()
job: Use AIO_WAIT_WHILE() in job_finish_sync()
test-bdrv-drain: Test AIO_WAIT_WHILE() in completion callback
block: Add missing locking in bdrv_co_drain_bh_cb()
aio-wait: Increase num_waiters even in home thread
block-backend: Add .drained_poll callback
block-backend: Fix potential double blk_delete()
block-backend: Decrease in_flight only after callback
mirror: Fix potential use-after-free in active commit
blockjob: Lie better in child_job_drained_poll()
block: Remove aio_poll() in bdrv_drain_poll variants
test-bdrv-drain: Test nested poll in bdrv_drain_poll_top_level()
include/block/aio-wait.h | 2 +
include/block/blockjob.h | 13 ++
include/qemu/coroutine.h | 5 +++
include/qemu/job.h | 12 ++
block/block-backend.c| 26 ++-
block/io.c | 23 ++
block/mirror.c | 9
blockjob.c | 20 -
job.c| 29 +
tests/test-bdrv-drain.c | 110 ---
tests/test-blockjob.c| 6 +++
util/qemu-coroutine.c| 5 +++
12 files changed, 235 insertions(+), 25 deletions(-)
--
2.13.6
[Qemu-devel] [PATCH 09/14] block-backend: Fix potential double blk_delete()
blk_unref() first decreases the refcount of the BlockBackend and calls
blk_delete() if the refcount reaches zero. Requests can still be in
flight at this point, they are only drained during blk_delete():
At this point, arbitrary callbacks can run. If any callback takes a
temporary BlockBackend reference, it will first increase the refcount to
1 and then decrease it to 0 again, triggering another blk_delete(). This
will cause a use-after-free crash in the outer blk_delete().
Fix it by draining the BlockBackend before decreasing to refcount to 0.
Assert in blk_ref() that it never takes the first refcount (which would
mean that the BlockBackend is already being deleted).
Signed-off-by: Kevin Wolf
---
block/block-backend.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index efa8d8011c..1b2d7a6ff5 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -435,6 +435,7 @@ int blk_get_refcnt(BlockBackend *blk)
*/
void blk_ref(BlockBackend *blk)
{
+assert(blk->refcnt > 0);
blk->refcnt++;
}
@@ -447,7 +448,11 @@ void blk_unref(BlockBackend *blk)
{
if (blk) {
assert(blk->refcnt > 0);
-if (!--blk->refcnt) {
+if (blk->refcnt > 1) {
+blk->refcnt--;
+} else {
+blk_drain(blk);
+blk->refcnt = 0;
blk_delete(blk);
}
}
--
2.13.6
[Qemu-devel] [PATCH 04/14] job: Use AIO_WAIT_WHILE() in job_finish_sync()
job_finish_sync() needs to release the AioContext lock of the job before
calling aio_poll(). Otherwise, callbacks called by aio_poll() would
possibly take the lock a second time and run into a deadlock with a
nested AIO_WAIT_WHILE() call.
Also, job_drain() without aio_poll() isn't necessarily enough to make
progress on a job, it could depend on bottom halves to be executed.
Combine both open-coded while loops into a single AIO_WAIT_WHILE() call
that solves both of these problems.
Signed-off-by: Kevin Wolf
---
job.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/job.c b/job.c
index 9ad0b7476a..8480eda188 100644
--- a/job.c
+++ b/job.c
@@ -29,6 +29,7 @@
#include "qemu/job.h"
#include "qemu/id.h"
#include "qemu/main-loop.h"
+#include "block/aio-wait.h"
#include "trace-root.h"
#include "qapi/qapi-events-job.h"
@@ -998,6 +999,7 @@ void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn
*fn, void *opaque)
int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error
**errp)
{
Error *local_err = NULL;
+AioWait dummy_wait = {};
int ret;
job_ref(job);
@@ -1010,14 +1012,10 @@ int job_finish_sync(Job *job, void (*finish)(Job *,
Error **errp), Error **errp)
job_unref(job);
return -EBUSY;
}
-/* job_drain calls job_enter, and it should be enough to induce progress
- * until the job completes or moves to the main thread. */
-while (!job->deferred_to_main_loop && !job_is_completed(job)) {
-job_drain(job);
-}
-while (!job_is_completed(job)) {
-aio_poll(qemu_get_aio_context(), true);
-}
+
+AIO_WAIT_WHILE(_wait, job->aio_context,
+ (job_drain(job), !job_is_completed(job)));
+
ret = (job_is_cancelled(job) && job->ret == 0) ? -ECANCELED : job->ret;
job_unref(job);
return ret;
--
2.13.6
[Qemu-devel] [PATCH 03/14] test-blockjob: Acquire AioContext around job_finish_sync()
All callers in QEMU proper hold the AioContext lock when calling
job_finish_sync(). test-blockjob should do the same.
Signed-off-by: Kevin Wolf
---
include/qemu/job.h| 6 ++
tests/test-blockjob.c | 6 ++
2 files changed, 12 insertions(+)
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 0dae5b8481..8ac48dbd28 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -520,6 +520,8 @@ void job_user_cancel(Job *job, bool force, Error **errp);
*
* Returns the return value from the job if the job actually completed
* during the call, or -ECANCELED if it was canceled.
+ *
+ * Callers must hold the AioContext lock of job->aio_context.
*/
int job_cancel_sync(Job *job);
@@ -537,6 +539,8 @@ void job_cancel_sync_all(void);
* function).
*
* Returns the return value from the job.
+ *
+ * Callers must hold the AioContext lock of job->aio_context.
*/
int job_complete_sync(Job *job, Error **errp);
@@ -579,6 +583,8 @@ void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn
*fn, void *opaque);
*
* Returns 0 if the job is successfully completed, -ECANCELED if the job was
* cancelled before completing, and -errno in other error cases.
+ *
+ * Callers must hold the AioContext lock of job->aio_context.
*/
int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error
**errp);
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index cb42f06e61..8c2babbe35 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -230,6 +230,10 @@ static void cancel_common(CancelJob *s)
BlockJob *job = >common;
BlockBackend *blk = s->blk;
JobStatus sts = job->job.status;
+AioContext *ctx;
+
+ctx = job->job.aio_context;
+aio_context_acquire(ctx);
job_cancel_sync(>job);
if (sts != JOB_STATUS_CREATED && sts != JOB_STATUS_CONCLUDED) {
@@ -239,6 +243,8 @@ static void cancel_common(CancelJob *s)
assert(job->job.status == JOB_STATUS_NULL);
job_unref(>job);
destroy_blk(blk);
+
+aio_context_release(ctx);
}
static void test_cancel_created(void)
--
2.13.6
[Qemu-devel] [PATCH 02/14] test-bdrv-drain: Drain with block jobs in an I/O thread
This extends the existing drain test with a block job to include
variants where the block job runs in a different AioContext.
Signed-off-by: Kevin Wolf
---
tests/test-bdrv-drain.c | 92 +
1 file changed, 86 insertions(+), 6 deletions(-)
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index 17bb8508ae..ab055e85f8 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -174,6 +174,28 @@ static void do_drain_end(enum drain_type drain_type,
BlockDriverState *bs)
}
}
+static void do_drain_begin_unlocked(enum drain_type drain_type,
BlockDriverState *bs)
+{
+if (drain_type != BDRV_DRAIN_ALL) {
+aio_context_acquire(bdrv_get_aio_context(bs));
+}
+do_drain_begin(drain_type, bs);
+if (drain_type != BDRV_DRAIN_ALL) {
+aio_context_release(bdrv_get_aio_context(bs));
+}
+}
+
+static void do_drain_end_unlocked(enum drain_type drain_type, BlockDriverState
*bs)
+{
+if (drain_type != BDRV_DRAIN_ALL) {
+aio_context_acquire(bdrv_get_aio_context(bs));
+}
+do_drain_end(drain_type, bs);
+if (drain_type != BDRV_DRAIN_ALL) {
+aio_context_release(bdrv_get_aio_context(bs));
+}
+}
+
static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
{
BlockBackend *blk;
@@ -790,11 +812,13 @@ BlockJobDriver test_job_driver = {
},
};
-static void test_blockjob_common(enum drain_type drain_type)
+static void test_blockjob_common(enum drain_type drain_type, bool use_iothread)
{
BlockBackend *blk_src, *blk_target;
BlockDriverState *src, *target;
BlockJob *job;
+IOThread *iothread = NULL;
+AioContext *ctx;
int ret;
src = bdrv_new_open_driver(_test, "source", BDRV_O_RDWR,
@@ -802,21 +826,31 @@ static void test_blockjob_common(enum drain_type
drain_type)
blk_src = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
blk_insert_bs(blk_src, src, _abort);
+if (use_iothread) {
+iothread = iothread_new();
+ctx = iothread_get_aio_context(iothread);
+blk_set_aio_context(blk_src, ctx);
+} else {
+ctx = qemu_get_aio_context();
+}
+
target = bdrv_new_open_driver(_test, "target", BDRV_O_RDWR,
_abort);
blk_target = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
blk_insert_bs(blk_target, target, _abort);
+aio_context_acquire(ctx);
job = block_job_create("job0", _job_driver, NULL, src, 0,
BLK_PERM_ALL,
0, 0, NULL, NULL, _abort);
block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, _abort);
job_start(>job);
+aio_context_release(ctx);
g_assert_cmpint(job->job.pause_count, ==, 0);
g_assert_false(job->job.paused);
g_assert_true(job->job.busy); /* We're in job_sleep_ns() */
-do_drain_begin(drain_type, src);
+do_drain_begin_unlocked(drain_type, src);
if (drain_type == BDRV_DRAIN_ALL) {
/* bdrv_drain_all() drains both src and target */
@@ -827,7 +861,14 @@ static void test_blockjob_common(enum drain_type
drain_type)
g_assert_true(job->job.paused);
g_assert_false(job->job.busy); /* The job is paused */
-do_drain_end(drain_type, src);
+do_drain_end_unlocked(drain_type, src);
+
+if (use_iothread) {
+/* paused is reset in the I/O thread, wait for it */
+while (job->job.paused) {
+aio_poll(qemu_get_aio_context(), false);
+}
+}
g_assert_cmpint(job->job.pause_count, ==, 0);
g_assert_false(job->job.paused);
@@ -846,32 +887,64 @@ static void test_blockjob_common(enum drain_type
drain_type)
do_drain_end(drain_type, target);
+if (use_iothread) {
+/* paused is reset in the I/O thread, wait for it */
+while (job->job.paused) {
+aio_poll(qemu_get_aio_context(), false);
+}
+}
+
g_assert_cmpint(job->job.pause_count, ==, 0);
g_assert_false(job->job.paused);
g_assert_true(job->job.busy); /* We're in job_sleep_ns() */
+aio_context_acquire(ctx);
ret = job_complete_sync(>job, _abort);
g_assert_cmpint(ret, ==, 0);
+if (use_iothread) {
+blk_set_aio_context(blk_src, qemu_get_aio_context());
+}
+aio_context_release(ctx);
+
blk_unref(blk_src);
blk_unref(blk_target);
bdrv_unref(src);
bdrv_unref(target);
+
+if (iothread) {
+iothread_join(iothread);
+}
}
static void test_blockjob_drain_all(void)
{
-test_blockjob_common(BDRV_DRAIN_ALL);
+test_blockjob_common(BDRV_DRAIN_ALL, false);
}
static void test_blockjob_drain(void)
{
-test_blockjob_common(BDRV_DRAIN);
+test_blockjob_common(BDRV_DRAIN, false);
}
static void test_blockjob_drain_subtree(void)
{
-test_blockjob_common(BDRV_SUBTREE_DRAIN);
+test_blockjob_common(BDRV_SUBTREE_DRAIN, false);
+}
+
+static void test_blockjob_iothread_drain_all(void)
+{
+test_blockjob_common(BDRV_DRAIN_ALL,
Re: [Qemu-devel] [PATCH 2/3] Improve xen_disk response latency
> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-
> bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Tim Smith
> Sent: 07 September 2018 11:21
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] [PATCH 2/3] Improve xen_disk response latency
>
> If the I/O ring is full, the guest cannot send any more requests
> until some responses are sent. Only sending all available responses
> just before checking for new work does not leave much time for the
> guest to supply new work, so this will cause stalls if the ring gets
> full. Also, not completing reads as soon as possible adds latency
> to the guest.
>
> To alleviate that, complete IO requests as soon as they come back.
> blk_send_response() already returns a value indicating whether
> a notify should be sent, which is all the batching we need.
>
> Signed-off-by: Tim Smith
Reviewed-by: Paul Durrant
> ---
> hw/block/xen_disk.c | 43 ---
> 1 file changed, 12 insertions(+), 31 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 6cb40d66fa..c11cd21d37 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -83,11 +83,9 @@ struct XenBlkDev {
>
> /* request lists */
> QLIST_HEAD(inflight_head, ioreq) inflight;
> -QLIST_HEAD(finished_head, ioreq) finished;
> QLIST_HEAD(freelist_head, ioreq) freelist;
> int requests_total;
> int requests_inflight;
> -int requests_finished;
> unsigned intmax_requests;
>
> gbooleanfeature_discard;
> @@ -104,6 +102,9 @@ struct XenBlkDev {
> /* Threshold of in-flight requests above which we will start using
> * blk_io_plug()/blk_io_unplug() to batch requests */
> #define IO_PLUG_THRESHOLD 1
> +
> +static int blk_send_response(struct ioreq *ioreq);
> +
> /* - */
>
> static void ioreq_reset(struct ioreq *ioreq)
> @@ -155,12 +156,10 @@ static void ioreq_finish(struct ioreq *ioreq)
> struct XenBlkDev *blkdev = ioreq->blkdev;
>
> QLIST_REMOVE(ioreq, list);
> -QLIST_INSERT_HEAD(>finished, ioreq, list);
> blkdev->requests_inflight--;
> -blkdev->requests_finished++;
> }
>
> -static void ioreq_release(struct ioreq *ioreq, bool finish)
> +static void ioreq_release(struct ioreq *ioreq)
> {
> struct XenBlkDev *blkdev = ioreq->blkdev;
>
> @@ -168,11 +167,7 @@ static void ioreq_release(struct ioreq *ioreq, bool
> finish)
> ioreq_reset(ioreq);
> ioreq->blkdev = blkdev;
> QLIST_INSERT_HEAD(>freelist, ioreq, list);
> -if (finish) {
> -blkdev->requests_finished--;
> -} else {
> -blkdev->requests_inflight--;
> -}
> +blkdev->requests_inflight--;
> }
>
> /*
> @@ -351,6 +346,10 @@ static void qemu_aio_complete(void *opaque, int
> ret)
> default:
> break;
> }
> +if (blk_send_response(ioreq)) {
> +xen_pv_send_notify(>xendev);
> +}
> +ioreq_release(ioreq);
> qemu_bh_schedule(blkdev->bh);
>
> done:
> @@ -455,7 +454,7 @@ err:
> return -1;
> }
>
> -static int blk_send_response_one(struct ioreq *ioreq)
> +static int blk_send_response(struct ioreq *ioreq)
> {
> struct XenBlkDev *blkdev = ioreq->blkdev;
> int send_notify = 0;
> @@ -504,22 +503,6 @@ static int blk_send_response_one(struct ioreq
> *ioreq)
> return send_notify;
> }
>
> -/* walk finished list, send outstanding responses, free requests */
> -static void blk_send_response_all(struct XenBlkDev *blkdev)
> -{
> -struct ioreq *ioreq;
> -int send_notify = 0;
> -
> -while (!QLIST_EMPTY(>finished)) {
> -ioreq = QLIST_FIRST(>finished);
> -send_notify += blk_send_response_one(ioreq);
> -ioreq_release(ioreq, true);
> -}
> -if (send_notify) {
> -xen_pv_send_notify(>xendev);
> -}
> -}
> -
> static int blk_get_request(struct XenBlkDev *blkdev, struct ioreq *ioreq,
> RING_IDX rc)
> {
> switch (blkdev->protocol) {
> @@ -554,7 +537,6 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> rp = blkdev->rings.common.sring->req_prod;
> xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
>
> -blk_send_response_all(blkdev);
> /* If there was more than IO_PLUG_THRESHOLD ioreqs in flight
> * when we got here, this is an indication that there the bottleneck
> * is below us, so it's worth beginning to batch up I/O requests
> @@ -597,10 +579,10 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> break;
> };
>
> -if (blk_send_response_one(ioreq)) {
> +if (blk_send_response(ioreq)) {
> xen_pv_send_notify(>xendev);
> }
> -ioreq_release(ioreq, false);
> +ioreq_release(ioreq);
> continue;
> }
>
> @@ -645,7 +627,6 @@ static void
Re: [Qemu-devel] [PATCH 3/3] Avoid repeated memory allocation in xen_disk
> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-
> bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Tim Smith
> Sent: 07 September 2018 11:22
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] [PATCH 3/3] Avoid repeated memory allocation in
> xen_disk
>
> xen_disk currently allocates memory to hold the data for each ioreq
> as that ioreq is used, and frees it afterwards. Because it requires
> page-aligned blocks, this interacts poorly with non-page-aligned
> allocations and balloons the heap.
>
> Instead, allocate the maximum possible requirement, which is
> BLKIF_MAX_SEGMENTS_PER_REQUEST pages (currently 11 pages) when
> the ioreq is created, and keep that allocation until it is destroyed.
> Since the ioreqs themselves are re-used via a free list, this
> should actually improve memory usage.
>
> Signed-off-by: Tim Smith
Reviewed-by: Paul Durrant
> ---
> hw/block/xen_disk.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index c11cd21d37..67f894bba5 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -112,7 +112,6 @@ static void ioreq_reset(struct ioreq *ioreq)
> memset(>req, 0, sizeof(ioreq->req));
> ioreq->status = 0;
> ioreq->start = 0;
> -ioreq->buf = NULL;
> ioreq->size = 0;
> ioreq->presync = 0;
>
> @@ -137,6 +136,10 @@ static struct ioreq *ioreq_start(struct XenBlkDev
> *blkdev)
> /* allocate new struct */
> ioreq = g_malloc0(sizeof(*ioreq));
> ioreq->blkdev = blkdev;
> +/* We cannot need more pages per ioreq than this, and we do re-use
> ioreqs,
> + * so allocate the memory once here, to be freed in blk_free() when
> the
> + * ioreq is freed. */
> +ioreq->buf = qemu_memalign(XC_PAGE_SIZE,
> BLKIF_MAX_SEGMENTS_PER_REQUEST * XC_PAGE_SIZE);
> blkdev->requests_total++;
> qemu_iovec_init(>v, 1);
> } else {
> @@ -313,14 +316,12 @@ static void qemu_aio_complete(void *opaque, int
> ret)
> if (ret == 0) {
> ioreq_grant_copy(ioreq);
> }
> -qemu_vfree(ioreq->buf);
> break;
> case BLKIF_OP_WRITE:
> case BLKIF_OP_FLUSH_DISKCACHE:
> if (!ioreq->req.nr_segments) {
> break;
> }
> -qemu_vfree(ioreq->buf);
> break;
> default:
> break;
> @@ -392,12 +393,10 @@ static int ioreq_runio_qemu_aio(struct ioreq
> *ioreq)
> {
> struct XenBlkDev *blkdev = ioreq->blkdev;
>
> -ioreq->buf = qemu_memalign(XC_PAGE_SIZE, ioreq->size);
> if (ioreq->req.nr_segments &&
> (ioreq->req.operation == BLKIF_OP_WRITE ||
> ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
> ioreq_grant_copy(ioreq)) {
> -qemu_vfree(ioreq->buf);
> goto err;
> }
>
> @@ -989,6 +988,7 @@ static int blk_free(struct XenDevice *xendev)
> ioreq = QLIST_FIRST(>freelist);
> QLIST_REMOVE(ioreq, list);
> qemu_iovec_destroy(>v);
> +qemu_vfree(ioreq->buf);
> g_free(ioreq);
> }
>
>
Re: [Qemu-devel] [Qemu-block] [PATCH v3 2/2] aio: Do aio_notify_accept only during blocking aio_poll
Am 09.08.2018 um 15:22 hat Fam Zheng geschrieben: > Furthermore, blocking aio_poll is only allowed on home thread > (in_aio_context_home_thread), because otherwise two blocking > aio_poll()'s can steal each other's ctx->notifier event and cause > hanging just like described above. It's good to have this assertion now at least, but after digging into some bugs, I think in fact that any aio_poll() (even non-blocking) is only allowed in the home thread: At least one reason is that if you run it from a different thread, qemu_get_current_aio_context() returns the wrong AioContext in any callbacks called by aio_poll(). Anything else using TLS can have similar problems. One instance where this matters is fixed/worked around by Sergio's "util/async: use qemu_aio_coroutine_enter in co_schedule_bh_cb". We wouldn't even need that patch if we could make sure that aio_poll() is never called from the wrong thread. This would feel more robust. I'll fix the aio_poll() calls in drain (the AIO_WAIT_WHILE() ones are already fine, the rest by removing them). After that, bdrv_set_aio_context() is still problematic, but the rest should be okay. Hopefully we can use the tighter assertion then. Kevin
Re: [Qemu-devel] [PATCH 2/6] qht: add qht_iter_remove
On Fri, Sep 07, 2018 at 15:51:12 +0100, Alex Bennée wrote:
>
> Emilio G. Cota writes:
>
> > This currently has no users, but the use case is so common that I
> > think we must support it.
> >
> > Note that without the appended we cannot safely remove a set of
> > elements; a 2-step approach (i.e. qht_iter first, keep track of
> > the to-be-deleted elements, and then a bunch of qht_remove calls)
> > would be racy, since between the iteration and the removals other
> > threads might insert additional elements.
> >
> > Signed-off-by: Emilio G. Cota
> > ---
> > include/qemu/qht.h | 19
> > util/qht.c | 74 +-
> > 2 files changed, 85 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/qemu/qht.h b/include/qemu/qht.h
> > index 1fb9116fa0..91bc9b00cf 100644
> > --- a/include/qemu/qht.h
> > +++ b/include/qemu/qht.h
> > @@ -44,6 +44,8 @@ struct qht_stats {
> >
> > typedef bool (*qht_lookup_func_t)(const void *obj, const void *userp);
> > typedef void (*qht_iter_func_t)(struct qht *ht, void *p, uint32_t h, void
> > *up);
> > +typedef bool (*qht_iter_bool_func_t)(struct qht *ht, void *p, uint32_t h,
> > + void *up);
> >
> > #define QHT_MODE_AUTO_RESIZE 0x1 /* auto-resize when heavily loaded */
> >
> > @@ -178,9 +180,26 @@ bool qht_resize(struct qht *ht, size_t n_elems);
> > *
> > * Each time it is called, user-provided @func is passed a pointer-hash
> > pair,
> > * plus @userp.
> > + *
> > + * Note: @ht cannot be accessed from @func
>
> I'm confused by this comment. If @ht cannot be accessed (or shouldn't be
> accessed) from @func then why are we passing it?
We could probably do without. My original thinking was to pass the *ht
to tell the iter function which ht his k-v pair comes from.
But that's not a common use case, so we should remove it to
simplify the interface.
I think the comment is important right now; note that we can't even
perform a lookup from the callback without deadlock.
So I'll take your R-b for this patch and remove the *ht in
another patch.
Thanks,
Emilio
Re: [Qemu-devel] [PATCH 6/6] test-qht: speed up + test qht_resize
Emilio G. Cota writes:
> Perform first the tests that exercise code paths that are
> easier to hit at small table sizes, and then resize the table
> to speed up subsequent tests. If this resize is not too large,
> we can make the test faster with no code coverage loss.
>
> - With gcov enabled:
>
> Before: 20.568s, 90.28% qht.c coverage
> After: 5.168s, 93.06% qht.c coverage
>
> The coverage increase is entirely due to calling qht_resize,
> which we weren't calling before. Note that the code paths
> that remain to be tested are either error handling or
> can only occur when several threads are accessing the
> hash table concurrently (e.g. seqlock retry, trylock fail).
>
> - Without gcov:
>
> Before: 1.987s
> After: 0.528s
>
> The speedup is almost the same as with gcov, although the
> "before" run is a lot faster.
>
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> tests/test-qht.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index 77666e8c5f..1ec039d636 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -189,6 +189,10 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
> rm_nonexist(7, 8);
> iter_rm_mod(1);
>
> +if (!(mode & QHT_MODE_AUTO_RESIZE)) {
> +qht_resize(, init_entries * 4 + 4);
> +}
> +
> check_n(0);
> rm_nonexist(0, 10);
> insert(0, N);
--
Alex Bennée
Re: [Qemu-devel] [PATCH 5/6] test-qht: test deletion of the last entry in a bucket
Emilio G. Cota writes: > This improves coverage by one (!) LoC in qht.c, bringing the > coverage rate up from 90.00% to 90.28%. > > Signed-off-by: Emilio G. Cota Reviewed-by: Alex Bennée > --- > tests/test-qht.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/test-qht.c b/tests/test-qht.c > index 05b1d6807a..77666e8c5f 100644 > --- a/tests/test-qht.c > +++ b/tests/test-qht.c > @@ -172,9 +172,20 @@ static void qht_do_test(unsigned int mode, size_t > init_entries) > > qht_init(, is_equal, 0, mode); > rm_nonexist(0, 4); > +/* > + * Test that we successfully delete the last element in a bucket. > + * This is a hard-to-reach code path when resizing is on, but without > + * resizing we can easily hit it if init_entries <= 1. > + * Given that the number of elements per bucket can be 4 or 6 depending > on > + * the host's pointer size, test the removal of the 4th and 6th elements. > + */ > insert(0, 4); > rm_nonexist(5, 6); > -insert(4, 6); > +rm(3, 4); > +check_n(3); > +insert(3, 6); > +rm(5, 6); > +check_n(5); > rm_nonexist(7, 8); > iter_rm_mod(1); -- Alex Bennée
Re: [Qemu-devel] [PATCH 4/6] test-qht: test removal of non-existent entries
Emilio G. Cota writes:
> This improves qht.c code coverage from 89.44% to 90.00%.
>
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> tests/test-qht.c | 26 --
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index 283fb3db39..05b1d6807a 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -41,7 +41,7 @@ static void insert(int a, int b)
> }
> }
>
> -static void rm(int init, int end)
> +static void do_rm(int init, int end, bool exist)
> {
> int i;
>
> @@ -49,10 +49,24 @@ static void rm(int init, int end)
> uint32_t hash;
>
> hash = arr[i];
> -g_assert_true(qht_remove(, [i], hash));
> +if (exist) {
> +g_assert_true(qht_remove(, [i], hash));
> +} else {
> +g_assert_false(qht_remove(, [i], hash));
> +}
> }
> }
>
> +static void rm(int init, int end)
> +{
> +do_rm(init, end, true);
> +}
> +
> +static void rm_nonexist(int init, int end)
> +{
> +do_rm(init, end, false);
> +}
> +
> static void check(int a, int b, bool expected)
> {
> struct qht_stats stats;
> @@ -157,8 +171,15 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
> check_n(0);
>
> qht_init(, is_equal, 0, mode);
> +rm_nonexist(0, 4);
> +insert(0, 4);
> +rm_nonexist(5, 6);
> +insert(4, 6);
> +rm_nonexist(7, 8);
> +iter_rm_mod(1);
>
> check_n(0);
> +rm_nonexist(0, 10);
> insert(0, N);
> check(0, N, true);
> check_n(N);
> @@ -183,6 +204,7 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
>
> qht_reset();
> insert(0, N);
> +rm_nonexist(N, N + 32);
> iter_rm_mod(10);
> iter_rm_mod_check(10);
> check_n(N * 9 / 10);
--
Alex Bennée
Re: [Qemu-devel] [PATCH 3/6] test-qht: test qht_iter_remove
Emilio G. Cota writes:
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> tests/test-qht.c | 50 ++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index dda6a067be..283fb3db39 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -108,6 +108,49 @@ static void iter_check(unsigned int count)
> g_assert_cmpuint(curr, ==, count);
> }
>
> +static void sum_func(struct qht *ht, void *p, uint32_t hash, void *userp)
> +{
> +uint32_t *sum = userp;
> +uint32_t a = *(uint32_t *)p;
> +
> +*sum += a;
> +}
> +
> +static void iter_sum_check(unsigned int expected)
> +{
> +unsigned int sum = 0;
> +
> +qht_iter(, sum_func, );
> +g_assert_cmpuint(sum, ==, expected);
> +}
> +
> +static bool rm_mod_func(struct qht *ht, void *p, uint32_t hash, void *userp)
> +{
> +uint32_t a = *(uint32_t *)p;
> +unsigned int mod = *(unsigned int *)userp;
> +
> +return a % mod == 0;
> +}
> +
> +static void iter_rm_mod(unsigned int mod)
> +{
> +qht_iter_remove(, rm_mod_func, );
> +}
> +
> +static void iter_rm_mod_check(unsigned int mod)
> +{
> +unsigned int expected = 0;
> +unsigned int i;
> +
> +for (i = 0; i < N; i++) {
> +if (i % mod == 0) {
> +continue;
> +}
> +expected += i;
> +}
> +iter_sum_check(expected);
> +}
> +
> static void qht_do_test(unsigned int mode, size_t init_entries)
> {
> /* under KVM we might fetch stats from an uninitialized qht */
> @@ -138,8 +181,11 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
> insert(10, 150);
> check_n(N);
>
> -rm(1, 2);
> -check_n(N - 1);
> +qht_reset();
> +insert(0, N);
> +iter_rm_mod(10);
> +iter_rm_mod_check(10);
> +check_n(N * 9 / 10);
> qht_reset_size(, 0);
> check_n(0);
> check(0, N, false);
--
Alex Bennée
Re: [Qemu-devel] [PATCH 2/6] qht: add qht_iter_remove
Emilio G. Cota writes:
> This currently has no users, but the use case is so common that I
> think we must support it.
>
> Note that without the appended we cannot safely remove a set of
> elements; a 2-step approach (i.e. qht_iter first, keep track of
> the to-be-deleted elements, and then a bunch of qht_remove calls)
> would be racy, since between the iteration and the removals other
> threads might insert additional elements.
>
> Signed-off-by: Emilio G. Cota
> ---
> include/qemu/qht.h | 19
> util/qht.c | 74 +-
> 2 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/include/qemu/qht.h b/include/qemu/qht.h
> index 1fb9116fa0..91bc9b00cf 100644
> --- a/include/qemu/qht.h
> +++ b/include/qemu/qht.h
> @@ -44,6 +44,8 @@ struct qht_stats {
>
> typedef bool (*qht_lookup_func_t)(const void *obj, const void *userp);
> typedef void (*qht_iter_func_t)(struct qht *ht, void *p, uint32_t h, void
> *up);
> +typedef bool (*qht_iter_bool_func_t)(struct qht *ht, void *p, uint32_t h,
> + void *up);
>
> #define QHT_MODE_AUTO_RESIZE 0x1 /* auto-resize when heavily loaded */
>
> @@ -178,9 +180,26 @@ bool qht_resize(struct qht *ht, size_t n_elems);
> *
> * Each time it is called, user-provided @func is passed a pointer-hash pair,
> * plus @userp.
> + *
> + * Note: @ht cannot be accessed from @func
I'm confused by this comment. If @ht cannot be accessed (or shouldn't be
accessed) from @func then why are we passing it?
> + * See also: qht_iter_remove()
> */
> void qht_iter(struct qht *ht, qht_iter_func_t func, void *userp);
>
> +/**
> + * qht_iter_remove - Iterate over a QHT, optionally removing entries
> + * @ht: QHT to be iterated over
> + * @func: function to be called for each entry in QHT
> + * @userp: additional pointer to be passed to @func
> + *
> + * Each time it is called, user-provided @func is passed a pointer-hash pair,
> + * plus @userp. If @func returns true, the pointer-hash pair is removed.
> + *
> + * Note: @ht cannot be accessed from @func
> + * See also: qht_iter()
> + */
> +void qht_iter_remove(struct qht *ht, qht_iter_bool_func_t func, void *userp);
> +
> /**
> * qht_statistics_init - Gather statistics from a QHT
> * @ht: QHT to gather statistics from
> diff --git a/util/qht.c b/util/qht.c
> index 7b57b50a24..caec53dd0e 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -89,6 +89,19 @@
> #define QHT_BUCKET_ENTRIES 4
> #endif
>
> +enum qht_iter_type {
> +QHT_ITER_VOID,/* do nothing; use retvoid */
> +QHT_ITER_RM, /* remove element if retbool returns true */
> +};
> +
> +struct qht_iter {
> +union {
> +qht_iter_func_t retvoid;
> +qht_iter_bool_func_t retbool;
> +} f;
> +enum qht_iter_type type;
> +};
> +
> /*
> * Note: reading partially-updated pointers in @pointers could lead to
> * segfaults. We thus access them with atomic_read/set; this guarantees
> @@ -706,9 +719,10 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t
> hash)
> return ret;
> }
>
> -static inline void qht_bucket_iter(struct qht *ht, struct qht_bucket *b,
> - qht_iter_func_t func, void *userp)
> +static inline void qht_bucket_iter(struct qht *ht, struct qht_bucket *head,
> + const struct qht_iter *iter, void *userp)
> {
> +struct qht_bucket *b = head;
> int i;
>
> do {
> @@ -716,7 +730,25 @@ static inline void qht_bucket_iter(struct qht *ht,
> struct qht_bucket *b,
> if (b->pointers[i] == NULL) {
> return;
> }
> -func(ht, b->pointers[i], b->hashes[i], userp);
> +switch (iter->type) {
> +case QHT_ITER_VOID:
> +iter->f.retvoid(ht, b->pointers[i], b->hashes[i], userp);
> +break;
> +case QHT_ITER_RM:
> +if (iter->f.retbool(ht, b->pointers[i], b->hashes[i],
> userp)) {
> +/* replace i with the last valid element in the bucket */
> +seqlock_write_begin(>sequence);
> +qht_bucket_remove_entry(b, i);
> +seqlock_write_end(>sequence);
> +qht_bucket_debug__locked(b);
> +/* reevaluate i, since it just got replaced */
> +i--;
> +continue;
> +}
> +break;
> +default:
> +g_assert_not_reached();
> +}
> }
> b = b->next;
> } while (b);
> @@ -724,26 +756,48 @@ static inline void qht_bucket_iter(struct qht *ht,
> struct qht_bucket *b,
>
> /* call with all of the map's locks held */
> static inline void qht_map_iter__all_locked(struct qht *ht, struct qht_map
> *map,
> -qht_iter_func_t func, void
> *userp)
> +
Re: [Qemu-devel] [PATCH 1/6] qht: remove unused map param from qht_remove__locked
Emilio G. Cota writes:
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> util/qht.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/util/qht.c b/util/qht.c
> index c138777a9c..7b57b50a24 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -665,8 +665,7 @@ static inline void qht_bucket_remove_entry(struct
> qht_bucket *orig, int pos)
>
> /* call with b->lock held */
> static inline
> -bool qht_remove__locked(struct qht_map *map, struct qht_bucket *head,
> -const void *p, uint32_t hash)
> +bool qht_remove__locked(struct qht_bucket *head, const void *p, uint32_t
> hash)
> {
> struct qht_bucket *b = head;
> int i;
> @@ -701,7 +700,7 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t
> hash)
> qht_debug_assert(p);
>
> b = qht_bucket_lock__no_stale(ht, hash, );
> -ret = qht_remove__locked(map, b, p, hash);
> +ret = qht_remove__locked(b, p, hash);
> qht_bucket_debug__locked(b);
> qemu_spin_unlock(>lock);
> return ret;
--
Alex Bennée
Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
Pavel Dovgalyuk writes: > The following series implements dynamic binary instrumentation upon > QEMU. OK I've done a pass through the patches, final comments bellow. > > For the current patches the plugins should provide the following > callbacks: > - "needs" callback to check whether the specific instruction >should be instrumented by this plugin > - "run" callback which called before executing the instuction I think with a little re-jigging we could reduce this to a single run call which can then decide if it is going to deal with things in this case. > Our instrumentation subsystem exploits TCG helper mechanism to embed > callbacks into the translation blocks. These callbacks may be inserted > before the specific instructions. > > The aim of submission of this series at that early stage is to get > the feedback which will guide the development process. We are faced > the following questions: > 1. Does every plugins should have its own callback embedded into the TB > (which will cause TB extra growth in case of multiple plugins), > or the instrumentation layer's callback should invoke the plugins > that wanted to instrument that specific instruction? The common case is having one plugin and I'd rather give the flexibility of what helper to call and how much information to give it to the plugin and deal with the potential to have multiple helper calls per instruction for the complex cases. Now I can see arguments against it from an interface complexity point of view but I think plugins should get access to the TCG functions so they can generate their own op sequences if need be and not have to call a helper at all if they don't need to. > 2. How the plugins should function? Will they work as a binary dynamic > libraries or a script on some interpreted language? I think plain C dynamic libraries with optional TCG ops would be a sensible starting point. For the sort of analysis that potentially involves lots of operations adding an interpreted language would be: - a bike-shedding operation (everyone has a favourite) - slower than C > 3. Should the plugins reuse QEMU configuration script results? > Now there is no possibility for using platform-specific macros > generated by QEMU configure. It depends but I'm of the opinion that you build your plugins for the QEMU you have and not expect them to run with any other version. Defining some sort of cross-version API stability is just making a rod for our own back. > 4. Maybe QEMU module infrastructure should be extended to support > plugins too? Pass - I'm not very knowledgeable about what the existing module stuff does. > 5. How the GDB-related CPU inspection interface may be used better? > We should pass a register code to read the value. These codes > are not described in any of the files. Maybe a function for > accessing register by name should be added? Personally it feels a bit hacky at the moment. Perhaps we should be looking at exposing tcg_global to the plugin interface. They are already registered with names and it would be more efficient to load from TCG code into the helper. > > > v2 changes: > - added a subsystem for the plugins > - added QEMU side API for plugins > - added sample plugins for simple tracing > > --- > > Pavel Dovgalyuk (7): > tcg: add headers for non-target helpers > Add plugin support > plugins: provide helper functions for plugins > tcg: add instrumenting module > plugins: add plugin template > plugin: add instruction execution logger > plugins: add syscall logging plugin sample > > > Makefile.target |1 > accel/tcg/translator.c|5 + > configure | 14 > include/exec/helper-register.h| 53 +++ > include/qemu/instrument.h |7 ++ > include/qemu/plugins.h|8 ++ > plugins/exec-log/Makefile | 19 + > plugins/exec-log/exec-log.c | 18 + > plugins/helper.h |1 > plugins/include/plugins.h | 18 + > plugins/plugins.c | 132 > + > plugins/qemulib.c | 31 + > plugins/syscall-log/Makefile | 19 + > plugins/syscall-log/syscall-log.c | 44 > plugins/template/Makefile | 19 + > plugins/template/template.c | 19 + > qemu-options.hx | 10 +++ > tcg/tcg.c | 12 +++ > tcg/tcg.h |3 + > vl.c |8 ++ > 20 files changed, 440 insertions(+), 1 deletion(-) > create mode 100644 include/exec/helper-register.h > create mode 100644 include/qemu/instrument.h > create mode 100644 include/qemu/plugins.h > create mode 100644 plugins/exec-log/Makefile > create mode 100644 plugins/exec-log/exec-log.c > create mode 100644
Re: [Qemu-devel] [RFC PATCH v2 2/7] Add plugin support
Pavel Dovgalyuk writes:
> This patch adds support for dynamically loaded plugins.
> Every plugin is a dynamic library with a set of optional exported
> functions that will be called from QEMU.
>
> +
> +static QLIST_HEAD(, QemuPluginInfo) qemu_plugins
> += QLIST_HEAD_INITIALIZER(qemu_plugins);
> +
> +static QemuOptsList qemu_plugin_opts = {
> +.name = "plugin",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_plugin_opts.head),
> +.desc = {
> +{
> +.name = "file",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "args",
> +.type = QEMU_OPT_STRING,
> +},
> +{ /* end of list */ }
> +},
> +};
> +
> +void qemu_plugin_parse_cmd_args(const char *optarg)
> +{
> +QemuOpts *opts = qemu_opts_parse_noisily(_plugin_opts, optarg,
> false);
> +qemu_plugin_load(qemu_opt_get(opts, "file"),
> +qemu_opt_get(opts, "args"));
> +}
Currently this is only available to system mode emulation. Can it be
extended to include linux-user as well?
> +
> +void qemu_plugin_load(const char *filename, const char *args)
> +{
> +GModule *g_module;
> +QemuPluginInfo *info = NULL;
> +if (!filename) {
> +error_report("plugin name was not specified");
> +return;
> +}
> +g_module = g_module_open(filename,
> +G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
> +if (!g_module) {
> +error_report("can't load plugin '%s'", filename);
> +return;
> +}
> +info = g_new0(QemuPluginInfo, 1);
> +info->filename = g_strdup(filename);
> +info->g_module = g_module;
> +if (args) {
> +info->args = g_strdup(args);
> +}
> +
> +g_module_symbol(g_module, "plugin_init", (gpointer*)>init);
> +
> +/* Get the instrumentation callbacks */
> +g_module_symbol(g_module, "plugin_needs_before_insn",
> +(gpointer*)>needs_before_insn);
> +g_module_symbol(g_module, "plugin_before_insn",
> +(gpointer*)>before_insn);
> +
> +QLIST_INSERT_HEAD(_plugins, info, next);
> +
> +return;
> +}
> +
> +void qemu_plugins_init(void)
> +{
> +QemuPluginInfo *info;
> +QLIST_FOREACH(info, _plugins, next) {
> +if (info->init) {
> +info->init(info->args);
> +}
> +}
> +}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c0d3951..d171544 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3950,6 +3950,16 @@ Dump json-encoded vmstate information for current
> machine type to file
> in @var{file}
> ETEXI
>
> +#ifdef CONFIG_PLUGINS
> +DEF("plugin", HAS_ARG, QEMU_OPTION_plugin, \
> + "-plugin file=[,args=] load plugin with
> \n", QEMU_ARCH_ALL)
> +STEXI
> +@item -plugin file=@var{file}[,args=@var{args}]
> +@findex -plugin
> +Load @var{file} plugin passing @var{args} arguments.
> +ETEXI
> +#endif
> +
> STEXI
> @end table
> ETEXI
> diff --git a/vl.c b/vl.c
> index 0603171..05420bf 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -129,6 +129,7 @@ int main(int argc, char **argv)
> #include "qapi/qapi-commands-run-state.h"
> #include "qapi/qmp/qerror.h"
> #include "sysemu/iothread.h"
> +#include "qemu/plugins.h"
>
> #define MAX_VIRTIO_CONSOLES 1
>
> @@ -3925,6 +3926,11 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
> break;
> +#ifdef CONFIG_PLUGINS
> +case QEMU_OPTION_plugin:
> +qemu_plugin_parse_cmd_args(optarg);
> +break;
> +#endif
> case QEMU_OPTION_nodefconfig:
> case QEMU_OPTION_nouserconfig:
> /* Nothing to be parsed here. Especially, do not error out
> below. */
> @@ -4470,6 +4476,8 @@ int main(int argc, char **argv, char **envp)
> }
> parse_numa_opts(current_machine);
>
> +qemu_plugins_init();
> +
> /* do monitor/qmp handling at preconfig state if requested */
> main_loop();
>
--
Alex Bennée
Re: [Qemu-devel] [PATCH 1/3] Improve xen_disk batching behaviour
> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-
> bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Tim Smith
> Sent: 07 September 2018 11:21
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] [PATCH 1/3] Improve xen_disk batching behaviour
>
> When I/O consists of many small requests, performance is improved by
> batching them together in a single io_submit() call. When there are
> relatively few requests, the extra overhead is not worth it. This
> introduces a check to start batching I/O requests via blk_io_plug()/
> blk_io_unplug() in an amount proportional to the number which were
> already in flight at the time we started reading the ring.
>
> Signed-off-by: Tim Smith
Reviewed-by: Paul Durrant
> ---
> hw/block/xen_disk.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94f84..6cb40d66fa 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -101,6 +101,9 @@ struct XenBlkDev {
> AioContext *ctx;
> };
>
> +/* Threshold of in-flight requests above which we will start using
> + * blk_io_plug()/blk_io_unplug() to batch requests */
> +#define IO_PLUG_THRESHOLD 1
> /* - */
>
> static void ioreq_reset(struct ioreq *ioreq)
> @@ -542,6 +545,8 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> {
> RING_IDX rc, rp;
> struct ioreq *ioreq;
> +int inflight_atstart = blkdev->requests_inflight;
> +int batched = 0;
>
> blkdev->more_work = 0;
>
> @@ -550,6 +555,16 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
>
> blk_send_response_all(blkdev);
> +/* If there was more than IO_PLUG_THRESHOLD ioreqs in flight
> + * when we got here, this is an indication that there the bottleneck
> + * is below us, so it's worth beginning to batch up I/O requests
> + * rather than submitting them immediately. The maximum number
> + * of requests we're willing to batch is the number already in
> + * flight, so it can grow up to max_requests when the bottleneck
> + * is below us */
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +blk_io_plug(blkdev->blk);
> +}
> while (rc != rp) {
> /* pull request from ring */
> if (RING_REQUEST_CONS_OVERFLOW(>rings.common, rc)) {
> @@ -589,7 +604,21 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> continue;
> }
>
> +if (inflight_atstart > IO_PLUG_THRESHOLD && batched >=
> inflight_atstart) {
> +blk_io_unplug(blkdev->blk);
> +}
> ioreq_runio_qemu_aio(ioreq);
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +if (batched >= inflight_atstart) {
> +blk_io_plug(blkdev->blk);
> +batched=0;
> +} else {
> +batched++;
> +}
> +}
> +}
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +blk_io_unplug(blkdev->blk);
> }
>
> if (blkdev->more_work && blkdev->requests_inflight < blkdev-
> >max_requests) {
>
Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
Pavel Dovgalyuk writes: > Peter, what about this one? > > Pavel Dovgalyuk > >> -Original Message- >> From: Pavel Dovgalyuk [mailto:dovga...@ispras.ru] >> Sent: Tuesday, June 05, 2018 2:56 PM >> To: 'Peter Maydell'; 'Pavel Dovgalyuk' >> Cc: 'QEMU Developers'; maria.klimushenk...@ispras.ru; 'Paolo Bonzini'; >> 'Lluís Vilanova' >> Subject: RE: [RFC PATCH v2 0/7] QEMU binary instrumentation prototype >> >> > From: Peter Maydell [mailto:peter.mayd...@linaro.org] >> > >> > This series doesn't seem to add anything to Documentation/ that >> > describes the API we make available to plugins. I'm a lot more >> > interested in reviewing the API that will be used by plugins >> > than I am in the implementation at this stage. Can you provide >> > a description/documentation of the API for review, please? >> >> >> Here is the draft: >> >> Introduction >> >> >> This document describes an API for creating the QEMU >> instrumentation plugins. >> >> It is based on the following prior sources: >> - KVM Forum 2017 talk "Instrumenting, Introspection, and Debugging with >> QEMU" >>https://www.linux-kvm.org/images/3/3d/Introspect.pdf >> - Discussion on Lluis Vilanova instrumentation patch series >>https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03357.html >> >> The aim of the instrumentation is implementing different runtime >> tracers that can track the executed instructions, memory and >> hardware operations. >> >> Instrumenting the code >> == >> >> Instrumentation subsystem exploits TCG helper mechanism to embed >> callbacks into the translation blocks. These callbacks may be inserted >> before the specific instructions, when the plugins require such filtering. >> >> Translator uses two functions for embedding the callbacks: >> - first function checks whether the current instruction should be >>instrumented >> - second function embeds the callback for executing the plugin-specific >>code before that instruction >> >> The similar method may be used for memory access instrumentation. >> >> QEMU->Plugin API >> >> >> Instrumentation layer passes the requests from the translator >> to the dynamically loaded plugins. Every plugin may provide >> the following functions to perform the instrumentation: >> >> 1. bool plugin_init(const char *args); >> Initialization function. May return false if the plugin >> can't work in the current environment. >> >> 2. bool plugin_needs_before_insn(uint64_t pc, void *cpu); >> Returns true if the plugin needs to instrument the current instruction. >> It may use the address (pc) for making the decision or the guest >> CPU state (cpu), which can be passed back to QEMU core API >> (e.g., for reading the guest memory). >> This function is called at both translation and execution phases. >> >> 3. void plugin_before_insn(uint64_t pc, void *cpu); >> If the previous function returned true for some instruction, >> then this function will be called. This process is repeated before >> every execution of the instruction, if it was instrumented. >> >> The similar pair of functions will also be added for the memory >> operations. >> >> Plugin->QEMU API >> >> >> QEMU core exports some functions to let the plugins introspect the guest >> or perform some interaction with other QEMU services (e.g., logging). >> API doesn't contain any data structures, because their memory layout depend >> on the compilation settings. >> >> QEMU exports the following functions that may be called from the plugins: >> >> 1. void qemulib_log(const char *fmt, ...); >> Wrapper for qemu_log. >> >> 2. int qemulib_read_memory(void *cpu, uint64_t addr, uint8_t *buf, int len); >> Reads guest memory into the buffer. Wrapper for cpu_memory_rw_debug. >> >> 3. int qemulib_read_register(void *cpu, uint8_t *mem_buf, int reg); >> Uses target gdb interface for accessing the guest registers. >> 'reg' is the id of the desired register as it is coded by gdb. >> >> There also should be a function for flushing the translated blocks to >> ensure that the instrumentation will occur in the case of changing >> the internal plugin state. This would certainly be the case if we could generate TCG code in the plugins. -- Alex Bennée
Re: [Qemu-devel] [RFC PATCH v2 7/7] plugins: add syscall logging plugin sample
Pavel Dovgalyuk writes:
> This is an example of plugin which instruments only specific instructions:
> sysenter and sysexit. When executing them, it prints system call id
> and return code to the QEMU log.
Again I'm not sure this is a very useful example either. It doesn't
achieve anything we can't already do with the existing logging/strace
stuff and it is quite ugly in it's knowledge of a single architecture to
try and figure out what's going on.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/syscall-log/Makefile | 19
> plugins/syscall-log/syscall-log.c | 44
> +
> 2 files changed, 63 insertions(+)
> create mode 100644 plugins/syscall-log/Makefile
> create mode 100644 plugins/syscall-log/syscall-log.c
>
> diff --git a/plugins/syscall-log/Makefile b/plugins/syscall-log/Makefile
> new file mode 100644
> index 000..1bbdf04
> --- /dev/null
> +++ b/plugins/syscall-log/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= syscall-log
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := syscall-log.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
> diff --git a/plugins/syscall-log/syscall-log.c
> b/plugins/syscall-log/syscall-log.c
> new file mode 100644
> index 000..1f5d55f
> --- /dev/null
> +++ b/plugins/syscall-log/syscall-log.c
> @@ -0,0 +1,44 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +uint8_t code = 0;
> +if (!qemulib_read_memory(cpu, pc, , 1)
> +&& code == 0x0f) {
> +if (qemulib_read_memory(cpu, pc + 1, , 1)) {
> +return false;
> +}
> +if (code == 0x34) {
> +/* sysenter */
> +return true;
> +}
> +if (code == 0x35) {
> +/* sysexit */
> +return true;
> +}
> +}
> +return false;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +uint8_t code = 0;
> +uint32_t reg;
> +qemulib_read_memory(cpu, pc + 1, , 1);
> +/* Read EAX. There should be a header with register ids
> + or a function for reading the register by the name */
> +qemulib_read_register(cpu, (uint8_t*), 0);
> +/* log system calls */
> +if (code == 0x34) {
> +qemulib_log("sysenter %x\n", reg);
> +} else if (code == 0x35) {
> +qemulib_log("sysexit %x\n", reg);
> +}
> +}
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 1/3] util: add qemu_write_pidfile()
On 09/07/2018 07:13 AM, Marc-André Lureau wrote: There are variants of qemu_create_pidfile() in qemu-pr-helper and qemu-ga. Let's have a common implementation in libqemuutil. Unrelated to this patch, but a question that this raises: should 'qemu-nbd' also have a mode for creating a pid file, since it has --fork for daemonizing the server process? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [Bug 1716292] Re: User mode emulation returns wrong value for write(fd, NULL, 0)
On 09/07/2018 06:51 AM, Tony Garnock-Jones wrote: ** Patch added: "0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch" https://bugs.launchpad.net/qemu/+bug/1716292/+attachment/5186008/+files/0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch While a developer can chase a URL, our CI tools can't. Can you please also send that patch directly to qemu-devel@nongnu.org, so that it gets the same level of review as other patches? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH v2 6/7] plugin: add instruction execution logger
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This patch adds a plugin for logging addresses of all executed instructions,
> making a complete instruction-level trace.
This isn't a good example. You can do this now with a much simpler:
${QEMU} -singlestep -d nochain,trace:exec_tb -D $trace ${BINARY}
Or even with a binary log:
${QEMU} -singlestep -d nochain -trace enable=exec_tb,file=$trace ${BINARY}
Which is all currently built-in. For the example to be worthwhile we
need to show how we can do something we currently can't do with the
existing infrastructure.
Perhaps a better example would be logging each PC execution to a hash
table so we can compute the hottest PC?
However that is going to require another API to allow information to be
exported from the plugin itself to report it's results.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/exec-log/Makefile | 19 +++
> plugins/exec-log/exec-log.c | 18 ++
> 2 files changed, 37 insertions(+)
> create mode 100644 plugins/exec-log/Makefile
> create mode 100644 plugins/exec-log/exec-log.c
>
> diff --git a/plugins/exec-log/Makefile b/plugins/exec-log/Makefile
> new file mode 100644
> index 000..86374f4
> --- /dev/null
> +++ b/plugins/exec-log/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
I would have:
QEMU_SRC=../..
CFLAGS += -I$(QEMU_SRC)/include -fno-PIE -fPIC -O3
to make it clearer for out of tree plugins.
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= exec-log
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := exec-log.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
If the example plugins are going to sit in the main tree we should build
them (and ideally test they load/work during make check/tcg-check).
> diff --git a/plugins/exec-log/exec-log.c b/plugins/exec-log/exec-log.c
> new file mode 100644
> index 000..7fc7975
> --- /dev/null
> +++ b/plugins/exec-log/exec-log.c
> @@ -0,0 +1,18 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +return true;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +qemulib_log("executing instruction at %lx\n", pc);
> +}
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 12/12] vl: list user creatable properties when 'help' is argument
On 09/07/2018 02:59 AM, Marc-André Lureau wrote: Iterate over the writable class properties, sort and print them out with the description if available. Ex: qemu -object memory-backend-file,help memory-backend-file.align=int memory-backend-file.discard-data=bool memory-backend-file.dump=bool - Set to 'off' to exclude from core dump memory-backend-file.host-nodes=int - Binds memory to the list of NUMA host nodes memory-backend-file.mem-path=string memory-backend-file.merge=bool - Mark memory as mergeable memory-backend-file.pmem=bool memory-backend-file.policy=HostMemPolicy - Set the NUMA policy memory-backend-file.prealloc=bool - Preallocate memory memory-backend-file.share=bool - Mark the memory as private to QEMU or shared memory-backend-file.size=int - Size of the memory region (ex: 500M) Signed-off-by: Marc-André Lureau --- qom/object_interfaces.c | 6 +++--- vl.c| 40 +--- 2 files changed, 40 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH v2 5/7] plugins: add plugin template
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This is a template of the QEMU plugin. It includes empty functions that
> plugins may implement.
>
I'm not sure it's worth having a null-template plugin if we can have one
or two well documented example plugins. It just runs the risk of
bitrot
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/template/Makefile | 19 +++
> plugins/template/template.c | 19 +++
> 2 files changed, 38 insertions(+)
> create mode 100644 plugins/template/Makefile
> create mode 100644 plugins/template/template.c
>
> diff --git a/plugins/template/Makefile b/plugins/template/Makefile
> new file mode 100644
> index 000..b9d10da
> --- /dev/null
> +++ b/plugins/template/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= template
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := template.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
> diff --git a/plugins/template/template.c b/plugins/template/template.c
> new file mode 100644
> index 000..fed1053
> --- /dev/null
> +++ b/plugins/template/template.c
> @@ -0,0 +1,19 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +printf("template plugin loaded successfully\n");
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +return true;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +printf("executing instruction at %lx\n", pc);
> +}
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 01/12] qdev-monitor: print help to stdout
On 09/07/2018 02:59 AM, Marc-André Lureau wrote:
qdev_device_help() is used from command line "-device help", or from
HMP "device_add". If used from command line, print help to stdout
(it is only printed on explicit demand).
Signed-off-by: Marc-André Lureau
---
include/monitor/monitor.h | 2 ++
monitor.c | 16 +---
qdev-monitor.c| 32 +++-
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 2ef5e04b37..e7667fda35 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
void monitor_fdset_dup_fd_remove(int dup_fd);
int monitor_fdset_dup_fd_find(int dup_fd);
+void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
Perhaps name this monitor_vfprintf()? (That would match the fact that
you are using it like normal vfprintf).
+++ b/qdev-monitor.c
@@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+static void out_printf(const char *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+out_vprintf(stdout, fmt, ap);
+va_end(ap);
+}
But this name seems reasonable.
And I just now see that Thomas also complained about the monitor.h
naming, but I like 'monitor_vfprintf' better than his 'mon_file_vprintf'.
If an improved name is your only change,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PULL 01/14] ppc: Remove deprecated ppcemb target
On 09/07/2018 02:31 AM, David Gibson wrote: From: Thomas Huth There is no known available OS for ppc around anymore that uses page sizes below 4k, so it does not make much sense that we keep wasting our time on building and testing the ppcemb-softmmu target. It has been deprecated since two releases, and nobody complained, so let's remove this now. Signed-off-by: Thomas Huth Signed-off-by: David Gibson --- +++ b/qapi/common.json @@ -146,6 +146,6 @@ 'data' : [ 'aarch64', 'alpha', 'arm', 'cris', 'hppa', 'i386', 'lm32', 'm68k', 'microblaze', 'microblazeel', 'mips', 'mips64', 'mips64el', 'mipsel', 'moxie', 'nios2', 'or1k', 'ppc', - 'ppc64', 'ppcemb', 'riscv32', 'riscv64', 's390x', 'sh4', + 'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4', 'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32', 'x86_64', 'xtensa', 'xtensaeb' ] } Can we also get documentation that this was deleted (probably as a followup, since I'm only now noticing this in the pull request)? See QKeyCode in qapi/ui.json for an example of documenting 'altgr: dropped in 2.10' -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH v2 4/4] qemu_thread_create: propagate the error to callers to handle
Make qemu_thread_create() return a Boolean to indicate if it succeeds
rather than failing with an error. And add an Error parameter to hold
the error message and let the callers handle it.
Besides, directly return if thread->data is NULL to avoid the
segmentation fault in qemu_thread_join in qemu-thread-win32.c.
Signed-off-by: Fei Li
---
cpus.c | 45 +++--
dump.c | 6 --
hw/misc/edu.c | 6 --
hw/ppc/spapr_hcall.c| 9 +++--
hw/rdma/rdma_backend.c | 3 ++-
hw/usb/ccid-card-emulated.c | 13
include/qemu/thread.h | 4 ++--
io/task.c | 3 ++-
iothread.c | 16 ++-
migration/migration.c | 49 +++--
migration/postcopy-ram.c| 12 +--
migration/ram.c | 40 +++-
migration/savevm.c | 11 +++---
tests/atomic_add-bench.c| 3 ++-
tests/iothread.c| 2 +-
tests/qht-bench.c | 3 ++-
tests/rcutorture.c | 3 ++-
tests/test-aio.c| 2 +-
tests/test-rcu-list.c | 3 ++-
ui/vnc-jobs.c | 8 ++--
util/compatfd.c | 7 +--
util/oslib-posix.c | 12 ---
util/qemu-thread-posix.c| 18 +++--
util/qemu-thread-win32.c| 15 +-
util/rcu.c | 3 ++-
util/thread-pool.c | 4 +++-
26 files changed, 210 insertions(+), 90 deletions(-)
diff --git a/cpus.c b/cpus.c
index 1feb308123..40db5c378f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1928,15 +1928,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name,
qemu_tcg_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name,
+qemu_tcg_cpu_thread_fn, cpu,
+QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
} else {
/* share a single thread for all cpus with TCG */
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-qemu_thread_create(cpu->thread, thread_name,
- qemu_tcg_rr_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name,
+qemu_tcg_rr_cpu_thread_fn, cpu,
+QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
single_tcg_halt_cond = cpu->halt_cond;
single_tcg_cpu_thread = cpu->thread;
@@ -1964,8 +1969,10 @@ static void qemu_hax_start_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
#ifdef _WIN32
cpu->hThread = qemu_thread_get_handle(cpu->thread);
#endif
@@ -1980,8 +1987,10 @@ static void qemu_kvm_start_vcpu(CPUState *cpu, Error
**errp)
qemu_cond_init(cpu->halt_cond);
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
}
static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
@@ -1998,8 +2007,10 @@ static void qemu_hvf_start_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
}
static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
@@ -2011,8 +2022,10 @@ static void qemu_whpx_start_vcpu(CPUState *cpu, Error
**errp)
qemu_cond_init(cpu->halt_cond);
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread,
Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross compilation in generating boot header file
On 09/07/2018 02:04 AM, Andrew Jones wrote:
> On Thu, Sep 06, 2018 at 12:23:45PM -0400, Wei Huang wrote:
>>
>>
>> - Original Message -
>>> From: "Andrew Jones"
>>> To: "Wei Huang"
>>> Cc: lviv...@redhat.com, "peter maydell" ,
>>> quint...@redhat.com, qemu-devel@nongnu.org,
>>> dgilb...@redhat.com, "alex bennee"
>>> Sent: Thursday, September 6, 2018 9:00:33 AM
>>> Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
>>> compilation in generating boot header file
>>>
>>> On Thu, Sep 06, 2018 at 09:37:04AM -0400, Wei Huang wrote:
- Original Message -
> From: "Andrew Jones"
> To: "Wei Huang"
> Cc: qemu-devel@nongnu.org, lviv...@redhat.com, "peter maydell"
> , quint...@redhat.com,
> dgilb...@redhat.com, "alex bennee"
> Sent: Thursday, September 6, 2018 7:03:32 AM
> Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
> compilation in generating boot header file
>
> On Wed, Sep 05, 2018 at 03:15:32PM -0400, Wei Huang wrote:
>> Recently a new configure option, CROSS_CC_GUEST, was added to
>> $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
>> patch tries to leverage this option to support cross compilation when
>> the
>> migration boot block file is being re-generated:
>>
>> * The x86 related files are moved to a new sub-dir (named ./i386).
>> * A new top-layer Makefile is created in tests/migration/ directory.
>>This Makefile searches and parses CROSS_CC_GUEST to generate
>>CROSS_PREFIX.
>>The CROSS_PREFIX, if available, is then passed to
>>migration/$ARCH/Makefile.
>>
>> Reviewed-by: Juan Quintela
>> Signed-off-by: Wei Huang
>> ---
>> tests/migration-test.c | 2 +-
>> tests/migration/Makefile | 44
>> --
>> tests/migration/i386/Makefile | 22 +++
>> .../{x86-a-b-bootblock.S => i386/a-b-bootblock.S} | 4 --
>> .../{x86-a-b-bootblock.h => i386/a-b-bootblock.h} | 8 ++--
>> 5 files changed, 51 insertions(+), 29 deletions(-)
>> create mode 100644 tests/migration/i386/Makefile
>> rename tests/migration/{x86-a-b-bootblock.S => i386/a-b-bootblock.S}
>> (93%)
>> rename tests/migration/{x86-a-b-bootblock.h => i386/a-b-bootblock.h}
>> (92%)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 0e687b7..fe6b41a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -83,7 +83,7 @@ static const char *tmpfs;
>> /* A simple PC boot sector that modifies memory (1-100MB) quickly
>> * outputting a 'B' every so often if it's still running.
>> */
>> -#include "tests/migration/x86-a-b-bootblock.h"
>> +#include "tests/migration/i386/a-b-bootblock.h"
>>
>> static void init_bootfile_x86(const char *bootpath)
>> {
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> index c0824b4..a9ed875 100644
>> --- a/tests/migration/Makefile
>> +++ b/tests/migration/Makefile
>> @@ -1,31 +1,35 @@
>> -# To specify cross compiler prefix, use CROSS_PREFIX=
>> -# $ make CROSS_PREFIX=x86_64-linux-gnu-
>> +#
>> +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +
>> +TARGET_LIST = i386
>> +
>> +SRC_PATH = ../..
>>
>> override define __note
>> -/* This file is automatically generated from
>> - * tests/migration/x86-a-b-bootblock.S, edit that and then run
>> - * tests/migration/rebuild-x86-bootblock.sh to update,
>> - * and then remember to send both in your patch submission.
>> +/* This file is automatically generated from the assembly file in
>> + * tests/migration/$@. Edit that file and then run "make all"
>> + * inside tests/migration to update, and then remember to send both
>> + * the header and the assembler differences in your patch submission.
>> */
>> endef
>> export __note
>>
>> -.PHONY: all clean
>> -all: x86-a-b-bootblock.h
>> -
>> -x86-a-b-bootblock.h: x86.bootsect
>> -echo "$$__note" > header.tmp
>> -xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> -mv header.tmp $@
>> +find-arch-cross-cc = $(lastword $(shell grep -h "CROSS_CC_GUEST="
>> $(wildcard $(SRC_PATH)/$(patsubst
>> i386,*86*,$(1))-softmmu/config-target.mak)))
>
> The above function hangs unless configuring with
> '--target-list=x86_64-softmmu,aarch64-softmmu'. I tried just x86_64
> alone,
> just aarch64 alone, and also configuring both x86_64 and i386, but none
> of those worked. For some reason
Re: [Qemu-devel] [PATCH v2 03/12] qemu-option: add help fallback to print the list of options
On 09/07/2018 02:59 AM, Marc-André Lureau wrote: QDev options accept 'help' (or '?', but that's problematic with shell globing) in the list of parameters, which is handy to list the s/globing/globbing/ available options. Unfortunately, this isn't built in QemuOpts. qemu_opts_parse_noisily() seems to be the common path for command line options, so place a fallback to print help, listing the available options. This is quite handy, for example with qemu "-spice help". Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake --- util/qemu-option.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) R-b still stands. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH v2 2/4] ui/vnc.c: polish vnc_init_func
Add a new Error parameter for vnc_display_init() to handle errors
in its caller: vnc_init_func(), just like vnc_display_open() does.
And let the call trace propagate the Error.
Besides, make vnc_start_worker_thread() return a bool to indicate
whether it succeeds instead of returning nothing.
Signed-off-by: Fei Li
---
include/ui/console.h | 2 +-
ui/vnc-jobs.c| 9 ++---
ui/vnc-jobs.h| 2 +-
ui/vnc.c | 12 +---
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index fb969caf70..c17803c530 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
/* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
void vnc_display_open(const char *id, Error **errp);
void vnc_display_add_client(const char *id, int csock, bool skipauth);
int vnc_display_password(const char *id, const char *password);
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 929391f85d..8807d7217c 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -331,15 +331,18 @@ static bool vnc_worker_thread_running(void)
return queue; /* Check global queue */
}
-void vnc_start_worker_thread(void)
+bool vnc_start_worker_thread(Error **errp)
{
VncJobQueue *q;
-if (vnc_worker_thread_running())
-return ;
+if (vnc_worker_thread_running()) {
+goto out;
+}
q = vnc_queue_init();
qemu_thread_create(>thread, "vnc_worker", vnc_worker_thread, q,
QEMU_THREAD_DETACHED);
queue = q; /* Set global queue */
+out:
+return true;
}
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index 59f66bcc35..14640593db 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job);
void vnc_jobs_join(VncState *vs);
void vnc_jobs_consume_buffer(VncState *vs);
-void vnc_start_worker_thread(void);
+bool vnc_start_worker_thread(Error **errp);
/* Locks */
static inline int vnc_trylock_display(VncDisplay *vd)
diff --git a/ui/vnc.c b/ui/vnc.c
index ccb1335d86..f632635ea4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
.dpy_cursor_define= vnc_dpy_cursor_define,
};
-void vnc_display_init(const char *id)
+void vnc_display_init(const char *id, Error **errp)
{
VncDisplay *vd;
@@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
vd->connections_limit = 32;
qemu_mutex_init(>mutex);
-vnc_start_worker_thread();
+if (!vnc_start_worker_thread(errp)) {
+return;
+}
vd->dcl.ops = _ops;
register_displaychangelistener(>dcl);
@@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error
**errp)
char *id = (char *)qemu_opts_id(opts);
assert(id);
-vnc_display_init(id);
+vnc_display_init(id, _err);
+if (local_err) {
+error_reportf_err(local_err, "Failed to init VNC server: ");
+exit(1);
+}
vnc_display_open(id, _err);
if (local_err != NULL) {
error_reportf_err(local_err, "Failed to start VNC server: ");
--
2.13.7
[Qemu-devel] [PATCH v2 3/4] qemu_init_vcpu: add a new Error parameter to propagate
The caller of qemu_init_vcpu() already passed the **errp to handle
errors. In view of this, add a new Error parameter to the following
call trace to propagate the error and let the further caller check it.
Besides, make qemu_init_vcpu() return a Boolean value to let its
callers know whether it succeeds.
Signed-off-by: Fei Li
---
accel/tcg/user-exec-stub.c | 2 +-
cpus.c | 34 +-
include/qom/cpu.h | 2 +-
target/alpha/cpu.c | 4 +++-
target/arm/cpu.c| 4 +++-
target/cris/cpu.c | 4 +++-
target/hppa/cpu.c | 4 +++-
target/i386/cpu.c | 4 +++-
target/lm32/cpu.c | 4 +++-
target/m68k/cpu.c | 4 +++-
target/microblaze/cpu.c | 4 +++-
target/mips/cpu.c | 4 +++-
target/moxie/cpu.c | 4 +++-
target/nios2/cpu.c | 4 +++-
target/openrisc/cpu.c | 4 +++-
target/ppc/translate_init.inc.c | 4 +++-
target/riscv/cpu.c | 4 +++-
target/s390x/cpu.c | 4 +++-
target/sh4/cpu.c| 4 +++-
target/sparc/cpu.c | 4 +++-
target/tilegx/cpu.c | 4 +++-
target/tricore/cpu.c| 4 +++-
target/unicore32/cpu.c | 4 +++-
target/xtensa/cpu.c | 4 +++-
24 files changed, 86 insertions(+), 36 deletions(-)
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index a32b4496af..38f6b928d4 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -10,7 +10,7 @@ void cpu_resume(CPUState *cpu)
{
}
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
{
}
diff --git a/cpus.c b/cpus.c
index 8ee6e5db93..1feb308123 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1898,7 +1898,7 @@ void cpu_remove_sync(CPUState *cpu)
/* For temporary buffers for forming a name */
#define VCPU_THREAD_NAME_SIZE 16
-static void qemu_tcg_init_vcpu(CPUState *cpu)
+static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
static QemuCond *single_tcg_halt_cond;
@@ -1954,7 +1954,7 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
}
}
-static void qemu_hax_start_vcpu(CPUState *cpu)
+static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -1971,7 +1971,7 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
#endif
}
-static void qemu_kvm_start_vcpu(CPUState *cpu)
+static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -1984,7 +1984,7 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
cpu, QEMU_THREAD_JOINABLE);
}
-static void qemu_hvf_start_vcpu(CPUState *cpu)
+static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2002,7 +2002,7 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
cpu, QEMU_THREAD_JOINABLE);
}
-static void qemu_whpx_start_vcpu(CPUState *cpu)
+static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2018,7 +2018,7 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
#endif
}
-static void qemu_dummy_start_vcpu(CPUState *cpu)
+static void qemu_dummy_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2031,11 +2031,12 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
QEMU_THREAD_JOINABLE);
}
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
{
cpu->nr_cores = smp_cores;
cpu->nr_threads = smp_threads;
cpu->stopped = true;
+Error *local_err = NULL;
if (!cpu->as) {
/* If the target cpu hasn't set up any address spaces itself,
@@ -2046,22 +2047,29 @@ void qemu_init_vcpu(CPUState *cpu)
}
if (kvm_enabled()) {
-qemu_kvm_start_vcpu(cpu);
+qemu_kvm_start_vcpu(cpu, _err);
} else if (hax_enabled()) {
-qemu_hax_start_vcpu(cpu);
+qemu_hax_start_vcpu(cpu, _err);
} else if (hvf_enabled()) {
-qemu_hvf_start_vcpu(cpu);
+qemu_hvf_start_vcpu(cpu, _err);
} else if (tcg_enabled()) {
-qemu_tcg_init_vcpu(cpu);
+qemu_tcg_init_vcpu(cpu, _err);
} else if (whpx_enabled()) {
-qemu_whpx_start_vcpu(cpu);
+qemu_whpx_start_vcpu(cpu, _err);
} else {
-qemu_dummy_start_vcpu(cpu);
+qemu_dummy_start_vcpu(cpu, _err);
+}
+
+if (local_err) {
+error_propagate(errp, local_err);
+return false;
}
while (!cpu->created) {
qemu_cond_wait(_cpu_cond, _global_mutex);
}
+
+return true;
}
void cpu_stop_current(void)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index dc130cd307..4d85dda175 100644
---
[Qemu-devel] [PATCH v2 0/4] qemu_thread_create: propagate errors to callers to check
Hi, This idea comes from BiteSizedTasks, and this patch series implement the error checking of qemu_thread_create: make qemu_thread_create return a flag to indicate if it succeeded rather than failing with an error; make all callers check it. The first three patches apply to those call traces whose further callers already have the *errp to pass, thus add a new Error paramater in the call trace to propagate the error and let the further caller check it. The last patch modifies the qemu_thread_create() and makes it return a bool to all direct callers to indicate if it succeeds. v2: - Pass errp straightly instead of using a local_err & error_propagate - Return a bool: false/true to indicate if one function succeeds - Merge v1's last two patches into one to avoid the compling error - Fix one omitted error in patch1 and update some error messages Please help to review. Thanks. Fei Li (4): Fix segmentation fault when qemu_signal_init fails ui/vnc.c: polish vnc_init_func qemu_init_vcpu: add a new Error parameter to propagate qemu_thread_create: propagate the error to callers to handle accel/tcg/user-exec-stub.c | 2 +- cpus.c | 79 ++--- dump.c | 6 ++-- hw/misc/edu.c | 6 ++-- hw/ppc/spapr_hcall.c| 9 +++-- hw/rdma/rdma_backend.c | 3 +- hw/usb/ccid-card-emulated.c | 13 --- include/qemu/osdep.h| 2 +- include/qemu/thread.h | 4 +-- include/qom/cpu.h | 2 +- include/ui/console.h| 2 +- io/task.c | 3 +- iothread.c | 16 ++--- migration/migration.c | 49 + migration/postcopy-ram.c| 12 +-- migration/ram.c | 40 +++-- migration/savevm.c | 11 -- target/alpha/cpu.c | 4 ++- target/arm/cpu.c| 4 ++- target/cris/cpu.c | 4 ++- target/hppa/cpu.c | 4 ++- target/i386/cpu.c | 4 ++- target/lm32/cpu.c | 4 ++- target/m68k/cpu.c | 4 ++- target/microblaze/cpu.c | 4 ++- target/mips/cpu.c | 4 ++- target/moxie/cpu.c | 4 ++- target/nios2/cpu.c | 4 ++- target/openrisc/cpu.c | 4 ++- target/ppc/translate_init.inc.c | 4 ++- target/riscv/cpu.c | 4 ++- target/s390x/cpu.c | 4 ++- target/sh4/cpu.c| 4 ++- target/sparc/cpu.c | 4 ++- target/tilegx/cpu.c | 4 ++- target/tricore/cpu.c| 4 ++- target/unicore32/cpu.c | 4 ++- target/xtensa/cpu.c | 4 ++- tests/atomic_add-bench.c| 3 +- tests/iothread.c| 2 +- tests/qht-bench.c | 3 +- tests/rcutorture.c | 3 +- tests/test-aio.c| 2 +- tests/test-rcu-list.c | 3 +- ui/vnc-jobs.c | 17 ++--- ui/vnc-jobs.h | 2 +- ui/vnc.c| 12 +-- util/compatfd.c | 16 ++--- util/main-loop.c| 10 +++--- util/oslib-posix.c | 12 +-- util/qemu-thread-posix.c| 18 ++ util/qemu-thread-win32.c| 15 +--- util/rcu.c | 3 +- util/thread-pool.c | 4 ++- 54 files changed, 325 insertions(+), 143 deletions(-) -- 2.13.7
[Qemu-devel] [PATCH v2 1/4] Fix segmentation fault when qemu_signal_init fails
Currently, when qemu_signal_init() fails it only returns a non-zero
value but without propagating any Error. But its callers need a
non-null err when runs error_report_err(err), or else 0->msg occurs.
To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.
This patch also adds the omitted error handling when creating signalfd
pipe fails in qemu_signalfd_compat().
Signed-off-by: Fei Li
---
include/qemu/osdep.h | 2 +-
util/compatfd.c | 9 ++---
util/main-loop.c | 10 +-
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index a91068df0e..09ed85fcb8 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
additional fields in the future) */
};
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
void sigaction_invoke(struct sigaction *action,
struct qemu_signalfd_siginfo *info);
#endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 980bd33e52..d3ed890405 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -16,6 +16,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/thread.h"
+#include "qapi/error.h"
#include
@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
}
}
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
{
struct sigfd_compat_info *info;
QemuThread thread;
@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
info = malloc(sizeof(*info));
if (info == NULL) {
+error_setg(errp, "Failed to allocate signalfd memory");
errno = ENOMEM;
return -1;
}
if (pipe(fds) == -1) {
+error_setg(errp, "Failed to create signalfd pipe");
free(info);
return -1;
}
@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
return fds[0];
}
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
{
#if defined(CONFIG_SIGNALFD)
int ret;
@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
}
#endif
-return qemu_signalfd_compat(mask);
+return qemu_signalfd_compat(mask, errp);
}
diff --git a/util/main-loop.c b/util/main-loop.c
index affe0403c5..22aa2b1007 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
}
}
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
int sigfd;
sigset_t set;
@@ -94,10 +94,10 @@ static int qemu_signal_init(void)
pthread_sigmask(SIG_BLOCK, , NULL);
sigdelset(, SIG_IPI);
-sigfd = qemu_signalfd();
+sigfd = qemu_signalfd(, errp);
if (sigfd == -1) {
fprintf(stderr, "failed to create signalfd\n");
-return -errno;
+return -1;
}
fcntl_setfl(sigfd, O_NONBLOCK);
@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
#else /* _WIN32 */
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
return 0;
}
@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
init_clocks(qemu_timer_notify_cb);
-ret = qemu_signal_init();
+ret = qemu_signal_init(errp);
if (ret) {
return ret;
}
--
2.13.7
Re: [Qemu-devel] [RFC PATCH v2 4/7] tcg: add instrumenting module
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This is a samples of the instrumenting interface and implementation
> of some instruction tracing tasks.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> accel/tcg/translator.c|5 +
> include/qemu/instrument.h |7 +++
> plugins/helper.h |1 +
> plugins/plugins.c | 41 +
> 4 files changed, 54 insertions(+)
> create mode 100644 include/qemu/instrument.h
> create mode 100644 plugins/helper.h
>
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 0f9dca9..48773ac 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -17,6 +17,7 @@
> #include "exec/gen-icount.h"
> #include "exec/log.h"
> #include "exec/translator.h"
> +#include "qemu/instrument.h"
>
> /* Pairs with tcg_clear_temp_count.
> To be called by #TranslatorOps.{translate_insn,tb_stop} if
> @@ -89,6 +90,10 @@ void translator_loop(const TranslatorOps *ops,
> DisasContextBase *db,
> }
> }
>
> +if (plugins_need_before_insn(db->pc_next, cpu)) {
> +plugins_instrument_before_insn(db->pc_next, cpu);
> +}
> +
I don't really see the need for a plugin_need_foo call here. Can't we
just iterate over all plugins that provide a before_insn binding and
leave it at that?
If the plugin decides it doesn't want to do anything with this
particular instruction it can always bail early.
> /* Disassemble one instruction. The translate_insn hook should
> update db->pc_next and db->is_jmp to indicate what should be
> done next -- either exiting this loop or locate the start of
> diff --git a/include/qemu/instrument.h b/include/qemu/instrument.h
> new file mode 100644
> index 000..e8f279f
> --- /dev/null
> +++ b/include/qemu/instrument.h
> @@ -0,0 +1,7 @@
> +#ifndef INSTRUMENT_H
> +#define INSTRUMENT_H
> +
> +bool plugins_need_before_insn(target_ulong pc, CPUState *cpu);
> +void plugins_instrument_before_insn(target_ulong pc, CPUState *cpu);
Need empty inline cases for #ifndef CONFIG_PLUGINS
> +
> +#endif /* INSTRUMENT_H */
> diff --git a/plugins/helper.h b/plugins/helper.h
> new file mode 100644
> index 000..007b395
> --- /dev/null
> +++ b/plugins/helper.h
> @@ -0,0 +1 @@
> +DEF_HELPER_2(before_insn, void, tl, ptr)
I wonder if we should have an explicit DEF_HELPER_FLAGS_2. Looking at
the flags though:
/* call flags */
/* Helper does not read globals (either directly or through an exception). It
implies TCG_CALL_NO_WRITE_GLOBALS. */
#define TCG_CALL_NO_READ_GLOBALS0x0010
/* Helper does not write globals */
#define TCG_CALL_NO_WRITE_GLOBALS 0x0020
/* Helper can be safely suppressed if the return value is not used. */
#define TCG_CALL_NO_SIDE_EFFECTS0x0040
/* convenience version of most used call flags */
#define TCG_CALL_NO_RWG TCG_CALL_NO_READ_GLOBALS
#define TCG_CALL_NO_WG TCG_CALL_NO_WRITE_GLOBALS
#define TCG_CALL_NO_SE TCG_CALL_NO_SIDE_EFFECTS
#define TCG_CALL_NO_RWG_SE (TCG_CALL_NO_RWG | TCG_CALL_NO_SE)
#define TCG_CALL_NO_WG_SE (TCG_CALL_NO_WG | TCG_CALL_NO_SE)
I guess no flags means machine state is fully consistent before we make
the helper call?
> diff --git a/plugins/plugins.c b/plugins/plugins.c
> index eabc931..5a08e71 100644
> --- a/plugins/plugins.c
> +++ b/plugins/plugins.c
> @@ -1,8 +1,13 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> +#include "cpu.h"
> #include "qemu/error-report.h"
> #include "qemu/plugins.h"
> +#include "qemu/instrument.h"
> +#include "tcg/tcg.h"
> +#include "tcg/tcg-op.h"
> #include "qemu/queue.h"
> +#include "qemu/option.h"
> #include
>
> typedef bool (*PluginInitFunc)(const char *);
> @@ -80,6 +85,40 @@ void qemu_plugin_load(const char *filename, const char
> *args)
> return;
> }
>
> +bool plugins_need_before_insn(target_ulong pc, CPUState *cpu)
> +{
> +QemuPluginInfo *info;
> +QLIST_FOREACH(info, _plugins, next) {
> +if (info->needs_before_insn && info->needs_before_insn(pc, cpu)) {
> +return true;
> +}
> +}
> +
> +return false;
> +}
> +
> +void plugins_instrument_before_insn(target_ulong pc, CPUState *cpu)
> +{
> +TCGv t_pc = tcg_const_tl(pc);
> +TCGv_ptr t_cpu = tcg_const_ptr(cpu);
> +/* We will dispatch plugins' callbacks in our own helper below */
> +gen_helper_before_insn(t_pc, t_cpu);
> +tcg_temp_free(t_pc);
> +tcg_temp_free_ptr(t_cpu);
> +}
OK I see what is happening here - but I worry about pushing off all
plugins into a single helper call with a fairly fixed amount of
information.
Would it be better to expose the generate helper API and maybe a few TCG
constants to the plugin function itself. It could then either emit
additional TCG operations or call to the helper - and deal with the
logic about if it should or shouldn't in a single call rather than doing
the need_foo call
[Qemu-devel] [PATCH] target/ppc/cpu-models: Re-group the 970 CPUs together again
The addition of the POWER9 CPUs divided the entries for the 970 CPUs,
which is a little bit confusing when you look at the code. So let's
re-group the 970 CPUs together again, and since these chips have been
based on the POWER4 processor, move them also in front of the POWER5
chips now.
Signed-off-by: Thomas Huth
---
target/ppc/cpu-models.c | 40 +++-
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 6c9bfde..7c75963 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -741,26 +741,8 @@
"PowerPC 7457A v1.2 (G4)")
/* 64 bits PowerPC */
#if defined (TARGET_PPC64)
-POWERPC_DEF("power5+_v2.1", CPU_POWERPC_POWER5P_v21,POWER5P,
-"POWER5+ v2.1")
-POWERPC_DEF("power7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
-"POWER7 v2.3")
-POWERPC_DEF("power7+_v2.1", CPU_POWERPC_POWER7P_v21,POWER7,
-"POWER7+ v2.1")
-POWERPC_DEF("power8e_v2.1", CPU_POWERPC_POWER8E_v21,POWER8,
-"POWER8E v2.1")
-POWERPC_DEF("power8_v2.0", CPU_POWERPC_POWER8_v20, POWER8,
-"POWER8 v2.0")
-POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8,
-"POWER8NVL v1.0")
POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22,970,
"PowerPC 970 v2.2")
-
-POWERPC_DEF("power9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9,
-"POWER9 v1.0")
-POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20,POWER9,
-"POWER9 v2.0")
-
POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10, 970,
"PowerPC 970FX v1.0 (G5)")
POWERPC_DEF("970fx_v2.0",CPU_POWERPC_970FX_v20, 970,
@@ -775,6 +757,22 @@
"PowerPC 970MP v1.0")
POWERPC_DEF("970mp_v1.1",CPU_POWERPC_970MP_v11, 970,
"PowerPC 970MP v1.1")
+POWERPC_DEF("power5+_v2.1", CPU_POWERPC_POWER5P_v21,POWER5P,
+"POWER5+ v2.1")
+POWERPC_DEF("power7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
+"POWER7 v2.3")
+POWERPC_DEF("power7+_v2.1", CPU_POWERPC_POWER7P_v21,POWER7,
+"POWER7+ v2.1")
+POWERPC_DEF("power8e_v2.1", CPU_POWERPC_POWER8E_v21,POWER8,
+"POWER8E v2.1")
+POWERPC_DEF("power8_v2.0", CPU_POWERPC_POWER8_v20, POWER8,
+"POWER8 v2.0")
+POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8,
+"POWER8NVL v1.0")
+POWERPC_DEF("power9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9,
+"POWER9 v1.0")
+POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20,POWER9,
+"POWER9 v2.0")
#endif /* defined (TARGET_PPC64) */
/***/
@@ -940,6 +938,9 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "7457a", "7457a_v1.2" },
{ "apollo7pm", "7457a_v1.0" },
#if defined(TARGET_PPC64)
+{ "970", "970_v2.2" },
+{ "970fx", "970fx_v3.1" },
+{ "970mp", "970mp_v1.1" },
{ "power5+", "power5+_v2.1" },
{ "power5gs", "power5+_v2.1" },
{ "power7", "power7_v2.3" },
@@ -948,9 +949,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "power8", "power8_v2.0" },
{ "power8nvl", "power8nvl_v1.0" },
{ "power9", "power9_v2.0" },
-{ "970", "970_v2.2" },
-{ "970fx", "970fx_v3.1" },
-{ "970mp", "970mp_v1.1" },
#endif
/* Generic PowerPCs */
--
1.8.3.1
Re: [Qemu-devel] [PATCH v4 00/29] vhost-user for input & GPU
Hi
On Wed, Aug 29, 2018 at 4:00 PM Marc-André Lureau
wrote:
>
> Hi
>
> On Wed, Aug 29, 2018 at 11:50 AM, Daniel P. Berrangé
> wrote:
> > On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> >> Hi,
> >>
> >> vhost-user allows to drive a virtio device in a seperate
> >> process. After vhost-user-net, we have seen
> >> vhost-user-{scsi,blk,crypto} added more recently.
> >>
> >> This series, initially proposed 2 years ago
> >> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> >> contributes with vhost-user-input and vhost-user-gpu.
> >>
> >> Additionally, to factor out common code and ease the usage, a
> >> vhost-user-backend is introduced as an intermediary object between the
> >> backend and the qemu device.
> >>
> >> You may start a vhost-user-gpu with virgl rendering in a separate
> >> process like this:
> >>
> >> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> >> $ qemu...
> >> -chardev socket,id=chr,path=vgpu.sock
> >> -object vhost-user-backend,id=vug,chardev=chr
> >> -device vhost-user-vga,vhost-user=vug
> >>
> >> You may also specify the backend command and the arguments as part of
> >> vhost-user-backend qemu arguments. For example, to start a
> >> vhost-user-input backend on input device /dev/input/event19:
> >>
> >> -object vhost-user-backend,id=vuid,cmd="vhost-user-input
> >> /dev/input/event19"
> >> -device virtio-input-host-pci,vhost-user=vuid
> >>
> >> The vhost-user-gpu backend requires virgl from git.
> >>
> >> The libvirt support is on-going work:
> >> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> >>
> >> The GPU benchmarks are encouraging, giving up to x5 performance on
> >> Unigine Heaven 4.0.
> >
> > What is the main driving motivation behind this featureset ? Is it aimed
> > at providing performance, or security, or allowing arbitrary out of tree
> > backends, or all three ?
>
> Mainly security/stability & performance. Allowing arbitrary out of
> tree backends is a bonus for me, I don't care much if we don't allow
> it.
>
> > Although we've got a number of vhost-user backends I'm pretty concerned
> > about the direction this is taking QEMU overall.
> >
> > Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> > work to provide standardized modelling of CLI args, guest ABI stability
> > via association with machine types, migration data stream stability,
> > QEMU feature capabilities detection and so on.
> >
> > The move to outsource backends to external binaries is discarding some
> > or all of these items, rewinding alot of progress we've made in the
> > managability of QEMU. Each external binary has to now reinvent the
> > things that are already solved in QEMU, and you can be sure each impl
> > will reinvent the concepts differently.
> >
> > I can't help feeling that we are shooting ourselves in the foot here
> > long term.
>
> I have a bit of the same feeling. For ex, I was a bit reluctant having
> the TPM emulator as an external process (new protocol, external
> binaries, with various management work, "opaque" migration). But in
> the end, the integration with libvirt makes thing quite easy for the
> user. So I changed a bit my mind, and I think this is one task that
> libvirt can be really good at.
>
> >
> > We've always rejected the idea of having loadable modules in QEMU, but
> > as a result we've end up with outsourcing the backend entirely via the
> > vhost-user framework, so the end result is even more opaque than if we
> > had loadable modules, and is unable to take advantage of our standardized
> > modelling frameworks & capabilities :-(
>
> I agree, that's one of the reason I put together libvhostuser in the
> first place. If qemu ships its own backend, there is no reason we
> don't share modelling & code etc.
>
> For ex, I hope more vhost-users will use the -object
> vhost-user-backend to do basic connection and virtio setup. (in the
> series, -gpu and -input, I didn't really investigate -crypto, -net,
> -blk etc)
>
> > If we just look at performance & security, and ignore 3rd party impls
> > for a minute, I really don't like that to gain perf + security we have
> > to change from:
> >
> > $ qemu...
> >-device virtio-vga
> >
> > To
> >
> > $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > $ qemu...
> >-chardev socket,id=chr,path=vgpu.sock
> >-object vhost-user-backend,id=vug,chardev=chr
> >-device vhost-user-vga,vhost-user=vug
> >
>
> That's a bit incovenient for qemu command line users. But who runs
> qemu this way but some developers? (others have higher-level scripts /
> tools or libvirt)
>
> > Once we have the ability to run the backend via an external process,
> > to gain performance & security benefits, assuming feature parity is
> > achieved, I question why anyone would want to continue with the old
> > in-process approach ? IOW the goal should be that the original args
> >
> > $ qemu...
> >-device virtio-vga
> >
> > should "do the right thing" to launch the
Re: [Qemu-devel] [PATCH v2 3/3] scsi: remove unused lsi53c895a_create() and lsi53c810_create() functions
On 2018-09-07 14:56, Mark Cave-Ayland wrote: > Now that these functions are no longer required they can be removed. > > Signed-off-by: Mark Cave-Ayland > --- > hw/scsi/lsi53c895a.c | 14 -- > include/hw/pci/pci.h | 2 -- > 2 files changed, 16 deletions(-) Reviewed-by: Thomas Huth
