[Qemu-devel] [PATCH v5 10/10] migration: hmp: dump globals
Now we have some globals that can be configured for migration. Dump them in HMP info migration for better debugging. (we can also use this to monitor whether COMPAT fields are applied correctly on compatible machines) Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- hmp.c| 3 +++ include/migration/misc.h | 1 + migration/migration.c| 11 +++ 3 files changed, 15 insertions(+) diff --git a/hmp.c b/hmp.c index 8c72c58..4c41cac 100644 --- a/hmp.c +++ b/hmp.c @@ -43,6 +43,7 @@ #include "exec/ramlist.h" #include "hw/intc/intc.h" #include "migration/snapshot.h" +#include "migration/misc.h" #ifdef CONFIG_SPICE #include @@ -164,6 +165,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) info = qmp_query_migrate(NULL); caps = qmp_query_migrate_capabilities(NULL); +migration_global_dump(mon); + /* do not display parameters during setup */ if (info->has_status && caps) { monitor_printf(mon, "capabilities: "); diff --git a/include/migration/misc.h b/include/migration/misc.h index 854c28d..2255121 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -54,5 +54,6 @@ bool migration_has_failed(MigrationState *); /* ...and after the device transmission */ bool migration_in_postcopy_after_devices(MigrationState *); void migration_only_migratable_set(void); +void migration_global_dump(Monitor *mon); #endif diff --git a/migration/migration.c b/migration/migration.c index e7e6cf3..b1b0825 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -43,6 +43,7 @@ #include "io/channel-buffer.h" #include "migration/colo.h" #include "hw/boards.h" +#include "monitor/monitor.h" #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ @@ -1993,6 +1994,16 @@ void migrate_fd_connect(MigrationState *s) s->migration_thread_running = true; } +void migration_global_dump(Monitor *mon) +{ +MigrationState *ms = migrate_get_current(); + +monitor_printf(mon, "globals: store-global-state=%d, only_migratable=%d, " + "send-configuration=%d, send-section-footer=%d\n", + ms->store_global_state, ms->only_migratable, + ms->send_configuration, ms->send_section_footer); +} + static Property migration_properties[] = { DEFINE_PROP_BOOL("store-global-state", MigrationState, store_global_state, true), -- 2.7.4
[Qemu-devel] [PATCH v5 09/10] migration: merge enforce_config_section somewhat
These two parameters: - MachineState::enforce_config_section - MigrationState::send_configuration are playing similar role here. This patch merges the first one into second, then we'll have a single place to reference whether we need to send the configuration section. I didn't remove the MachineState.enforce_config_section field since when applying that machine property (in machine_set_property()) we haven't yet initialized global properties and migration object. Then, it's still not easy to pass that boolean to MigrationState at such an early time. A natural benefit for current patch is that now we kept the meaning of "enforce-config-section" since it'll still have the highest priority (that's what "enforce" mean I guess). Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- migration/migration.c | 12 migration/savevm.c| 12 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 96c6412..e7e6cf3 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -42,6 +42,7 @@ #include "exec/target_page.h" #include "io/channel-buffer.h" #include "migration/colo.h" +#include "hw/boards.h" #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ @@ -102,9 +103,20 @@ static MigrationState *current_migration; void migration_object_init(void) { +MachineState *ms = MACHINE(qdev_get_machine()); + /* This can only be called once. */ assert(!current_migration); current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION)); + +/* + * We cannot really do this in migration_instance_init() since at + * that time global properties are not yet applied, then this + * value will be definitely replaced by something else. + */ +if (ms->enforce_config_section) { +current_migration->send_configuration = true; +} } /* For outgoing */ diff --git a/migration/savevm.c b/migration/savevm.c index 28fa1d5..6b1a9a5 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -943,20 +943,13 @@ bool qemu_savevm_state_blocked(Error **errp) return false; } -static bool enforce_config_section(void) -{ -MachineState *machine = MACHINE(qdev_get_machine()); -return machine->enforce_config_section; -} - void qemu_savevm_state_header(QEMUFile *f) { trace_savevm_state_header(); qemu_put_be32(f, QEMU_VM_FILE_MAGIC); qemu_put_be32(f, QEMU_VM_FILE_VERSION); -if (migrate_get_current()->send_configuration || -enforce_config_section()) { +if (migrate_get_current()->send_configuration) { qemu_put_byte(f, QEMU_VM_CONFIGURATION); vmstate_save_state(f, _configuration, _state, 0); } @@ -1980,8 +1973,7 @@ int qemu_loadvm_state(QEMUFile *f) return -ENOTSUP; } -if (migrate_get_current()->send_configuration || -enforce_config_section()) { +if (migrate_get_current()->send_configuration) { if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { error_report("Configuration section missing"); return -EINVAL; -- 2.7.4
[Qemu-devel] [PATCH v5 08/10] migration: move skip_section_footers
Move it into MigrationState, revert its meaning and renaming it to send_section_footer, with a property bound to it. Same trick is played like previous patches. Removing savevm_skip_section_footers(). Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- hw/i386/pc_piix.c| 1 - hw/ppc/spapr.c | 1 - hw/xen/xen-common.c | 8 +--- include/hw/compat.h | 4 include/migration/misc.h | 1 - migration/migration.c| 2 ++ migration/migration.h| 2 ++ migration/savevm.c | 11 ++- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 488fc0a..22dbef6 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -314,7 +314,6 @@ static void pc_init1(MachineState *machine, static void pc_compat_2_3(MachineState *machine) { PCMachineState *pcms = PC_MACHINE(machine); -savevm_skip_section_footers(); if (kvm_enabled()) { pcms->smm = ON_OFF_AUTO_OFF; } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 29fac1b..7c40fdf 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3579,7 +3579,6 @@ DEFINE_SPAPR_MACHINE(2_4, "2.4", false); static void spapr_machine_2_3_instance_options(MachineState *machine) { spapr_machine_2_4_instance_options(machine); -savevm_skip_section_footers(); } static void spapr_machine_2_3_class_options(MachineClass *mc) diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c index 6e52fb0..04512f4 100644 --- a/hw/xen/xen-common.c +++ b/hw/xen/xen-common.c @@ -138,9 +138,6 @@ static int xen_init(MachineState *ms) return -1; } qemu_add_vm_change_state_handler(xen_change_state_handler, NULL); - -savevm_skip_section_footers(); - return 0; } @@ -155,6 +152,11 @@ GlobalProperty xen_compat_props[] = { .property = "send-configuration", .value = "off", }, +{ +.driver = "migration", +.property = "send-section-footer", +.value = "off", +}, { .driver = NULL, .property = NULL, .value = NULL }, }; diff --git a/include/hw/compat.h b/include/hw/compat.h index 1a3fd94..08f3600 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -187,6 +187,10 @@ .value= "off",\ },{\ .driver = "migration",\ +.property = "send-section-footer",\ +.value= "off",\ +},{\ +.driver = "migration",\ .property = "store-global-state",\ .value= "off",\ }, diff --git a/include/migration/misc.h b/include/migration/misc.h index f30db4d..854c28d 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -41,7 +41,6 @@ int64_t self_announce_delay(int round) /* migration/savevm.c */ void dump_vmstate_json_to_file(FILE *out_fp); -void savevm_skip_section_footers(void); /* migration/migration.c */ void migration_object_init(void); diff --git a/migration/migration.c b/migration/migration.c index 414e14d..96c6412 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1987,6 +1987,8 @@ static Property migration_properties[] = { DEFINE_PROP_BOOL("only-migratable", MigrationState, only_migratable, false), DEFINE_PROP_BOOL("send-configuration", MigrationState, send_configuration, true), +DEFINE_PROP_BOOL("send-section-footer", MigrationState, + send_section_footer, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/migration/migration.h b/migration/migration.h index 4d4ea0d..994b017 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -145,6 +145,8 @@ struct MigrationState /* Whether we send QEMU_VM_CONFIGURATION during migration */ bool send_configuration; +/* Whether we send section footer during migration */ +bool send_section_footer; }; void migrate_set_state(int *state, int old_state, int new_state); diff --git a/migration/savevm.c b/migration/savevm.c index f1e144d..28fa1d5 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -62,8 +62,6 @@ const unsigned int postcopy_ram_discard_version = 0; -static bool skip_section_footers; - /* Subcommands for QEMU_VM_COMMAND */ enum qemu_vm_cmd { MIG_CMD_INVALID = 0, /* Must be 0 */ @@ -761,11 +759,6 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc) vmstate_save_state(f, se->vmsd, se->opaque, vmdesc); } -void savevm_skip_section_footers(void) -{ -skip_section_footers = true; -} - /* * Write the header for device section (QEMU_VM_SECTION START/END/PART/FULL) */ @@ -793,7 +786,7 @@ static void save_section_header(QEMUFile *f, SaveStateEntry *se, */ static void save_section_footer(QEMUFile *f, SaveStateEntry *se) { -if (!skip_section_footers) { +if (migrate_get_current()->send_section_footer) { qemu_put_byte(f, QEMU_VM_SECTION_FOOTER); qemu_put_be32(f, se->section_id); } @@ -1802,7 +1795,7 @@ static
[Qemu-devel] [PATCH v5 05/10] migration: move global_state.optional out
Put it into MigrationState then we can use the properties to specify whether to enable storing global state. Removing global_state_set_optional() since now we can use HW_COMPAT_2_3 for x86/power, and AccelClass.global_props for Xen. Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- hw/i386/pc_piix.c| 1 - hw/ppc/spapr.c | 1 - hw/xen/xen-common.c | 11 ++- include/hw/compat.h | 4 include/migration/global_state.h | 1 - migration/global_state.c | 9 ++--- migration/migration.c| 7 +++ migration/migration.h| 6 ++ 8 files changed, 29 insertions(+), 11 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 46a2bc4..3b51297 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -318,7 +318,6 @@ static void pc_compat_2_3(MachineState *machine) if (kvm_enabled()) { pcms->smm = ON_OFF_AUTO_OFF; } -global_state_set_optional(); savevm_skip_configuration(); } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e877d45..edbdbfd 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3580,7 +3580,6 @@ static void spapr_machine_2_3_instance_options(MachineState *machine) { spapr_machine_2_4_instance_options(machine); savevm_skip_section_footers(); -global_state_set_optional(); savevm_skip_configuration(); } diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c index d3fa705..9163a0a 100644 --- a/hw/xen/xen-common.c +++ b/hw/xen/xen-common.c @@ -139,19 +139,28 @@ static int xen_init(MachineState *ms) } qemu_add_vm_change_state_handler(xen_change_state_handler, NULL); -global_state_set_optional(); savevm_skip_configuration(); savevm_skip_section_footers(); return 0; } +GlobalProperty xen_compat_props[] = { +{ +.driver = "migration", +.property = "store-global-state", +.value = "off", +}, +{ .driver = NULL, .property = NULL, .value = NULL }, +}; + static void xen_accel_class_init(ObjectClass *oc, void *data) { AccelClass *ac = ACCEL_CLASS(oc); ac->name = "Xen"; ac->init_machine = xen_init; ac->allowed = _allowed; +ac->global_props = xen_compat_props; } #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") diff --git a/include/hw/compat.h b/include/hw/compat.h index 26cd585..a506a74 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -181,6 +181,10 @@ .driver = TYPE_PCI_DEVICE,\ .property = "x-pcie-lnksta-dllla",\ .value= "off",\ +},{\ +.driver = "migration",\ +.property = "store-global-state",\ +.value= "off",\ }, #define HW_COMPAT_2_2 \ diff --git a/include/migration/global_state.h b/include/migration/global_state.h index 90faea7..d307de8 100644 --- a/include/migration/global_state.h +++ b/include/migration/global_state.h @@ -16,7 +16,6 @@ #include "sysemu/sysemu.h" void register_global_state(void); -void global_state_set_optional(void); int global_state_store(void); void global_state_store_running(void); bool global_state_received(void); diff --git a/migration/global_state.c b/migration/global_state.c index f792cf5..dcbbcb2 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -15,12 +15,12 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qapi/util.h" +#include "migration.h" #include "migration/global_state.h" #include "migration/vmstate.h" #include "trace.h" typedef struct { -bool optional; uint32_t size; uint8_t runstate[100]; RunState state; @@ -57,11 +57,6 @@ RunState global_state_get_runstate(void) return global_state.state; } -void global_state_set_optional(void) -{ -global_state.optional = true; -} - static bool global_state_needed(void *opaque) { GlobalState *s = opaque; @@ -69,7 +64,7 @@ static bool global_state_needed(void *opaque) /* If it is not optional, it is mandatory */ -if (s->optional == false) { +if (migrate_get_current()->store_global_state) { return true; } diff --git a/migration/migration.c b/migration/migration.c index 2c25927..221b22c 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1976,11 +1976,18 @@ void migrate_fd_connect(MigrationState *s) s->migration_thread_running = true; } +static Property migration_properties[] = { +DEFINE_PROP_BOOL("store-global-state", MigrationState, + store_global_state, true), +DEFINE_PROP_END_OF_LIST(), +}; + static void migration_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->user_creatable = false; +dc->props = migration_properties; } static void migration_instance_init(Object *obj) diff --git a/migration/migration.h b/migration/migration.h index 3fca364..4b898e9 100644 --- a/migration/migration.h +++
[Qemu-devel] [PATCH v5 07/10] migration: move skip_configuration out
It was in SaveState but now moved to MigrationState altogether, reverted its meaning, then renamed to "send_configuration". Again, using HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for xen_init(). Removing savevm_skip_configuration(). Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- hw/i386/pc_piix.c| 1 - hw/ppc/spapr.c | 1 - hw/xen/xen-common.c | 6 +- include/hw/compat.h | 4 include/migration/misc.h | 1 - migration/migration.c| 2 ++ migration/migration.h| 3 +++ migration/savevm.c | 15 --- 8 files changed, 18 insertions(+), 15 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 3b51297..488fc0a 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -318,7 +318,6 @@ static void pc_compat_2_3(MachineState *machine) if (kvm_enabled()) { pcms->smm = ON_OFF_AUTO_OFF; } -savevm_skip_configuration(); } static void pc_compat_2_2(MachineState *machine) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index edbdbfd..29fac1b 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3580,7 +3580,6 @@ static void spapr_machine_2_3_instance_options(MachineState *machine) { spapr_machine_2_4_instance_options(machine); savevm_skip_section_footers(); -savevm_skip_configuration(); } static void spapr_machine_2_3_class_options(MachineClass *mc) diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c index 9163a0a..6e52fb0 100644 --- a/hw/xen/xen-common.c +++ b/hw/xen/xen-common.c @@ -139,7 +139,6 @@ static int xen_init(MachineState *ms) } qemu_add_vm_change_state_handler(xen_change_state_handler, NULL); -savevm_skip_configuration(); savevm_skip_section_footers(); return 0; @@ -151,6 +150,11 @@ GlobalProperty xen_compat_props[] = { .property = "store-global-state", .value = "off", }, +{ +.driver = "migration", +.property = "send-configuration", +.value = "off", +}, { .driver = NULL, .property = NULL, .value = NULL }, }; diff --git a/include/hw/compat.h b/include/hw/compat.h index a506a74..1a3fd94 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -183,6 +183,10 @@ .value= "off",\ },{\ .driver = "migration",\ +.property = "send-configuration",\ +.value= "off",\ +},{\ +.driver = "migration",\ .property = "store-global-state",\ .value= "off",\ }, diff --git a/include/migration/misc.h b/include/migration/misc.h index 6ac3307..f30db4d 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -42,7 +42,6 @@ int64_t self_announce_delay(int round) void dump_vmstate_json_to_file(FILE *out_fp); void savevm_skip_section_footers(void); -void savevm_skip_configuration(void); /* migration/migration.c */ void migration_object_init(void); diff --git a/migration/migration.c b/migration/migration.c index 67f9e68..414e14d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1985,6 +1985,8 @@ static Property migration_properties[] = { DEFINE_PROP_BOOL("store-global-state", MigrationState, store_global_state, true), DEFINE_PROP_BOOL("only-migratable", MigrationState, only_migratable, false), +DEFINE_PROP_BOOL("send-configuration", MigrationState, + send_configuration, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/migration/migration.h b/migration/migration.h index 34377dd..4d4ea0d 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -142,6 +142,9 @@ struct MigrationState /* Whether the VM is only allowing for migratable devices */ bool only_migratable; + +/* Whether we send QEMU_VM_CONFIGURATION during migration */ +bool send_configuration; }; void migrate_set_state(int *state, int old_state, int new_state); diff --git a/migration/savevm.c b/migration/savevm.c index 917ab05..f1e144d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -287,7 +287,6 @@ typedef struct SaveStateEntry { typedef struct SaveState { QTAILQ_HEAD(, SaveStateEntry) handlers; int global_section_id; -bool skip_configuration; uint32_t len; const char *name; uint32_t target_page_bits; @@ -296,15 +295,8 @@ typedef struct SaveState { static SaveState savevm_state = { .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers), .global_section_id = 0, -.skip_configuration = false, }; -void savevm_skip_configuration(void) -{ -savevm_state.skip_configuration = true; -} - - static void configuration_pre_save(void *opaque) { SaveState *state = opaque; @@ -970,11 +962,11 @@ void qemu_savevm_state_header(QEMUFile *f) qemu_put_be32(f, QEMU_VM_FILE_MAGIC); qemu_put_be32(f, QEMU_VM_FILE_VERSION); -if (!savevm_state.skip_configuration || enforce_config_section()) { +if
[Qemu-devel] [PATCH v5 06/10] migration: move only_migratable to MigrationState
One less global variable, and it does only matter with migration. We keep the old "--only-migratable" option, but also now we support: -global migration.only-migratable=true Currently still keep the old interface. Hmm, now vl.c has no way to access migrate_get_current(). Export a function for it to setup only_migratable. Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- include/migration/misc.h | 2 ++ include/sysemu/sysemu.h | 1 - migration/migration.c| 8 +++- migration/migration.h| 3 +++ migration/savevm.c | 2 +- vl.c | 9 +++-- 6 files changed, 20 insertions(+), 5 deletions(-) diff --git a/include/migration/misc.h b/include/migration/misc.h index 2d36cf5..6ac3307 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -55,4 +55,6 @@ bool migration_has_finished(MigrationState *); bool migration_has_failed(MigrationState *); /* ...and after the device transmission */ bool migration_in_postcopy_after_devices(MigrationState *); +void migration_only_migratable_set(void); + #endif diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 9841a52..b213696 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -15,7 +15,6 @@ /* vl.c */ extern const char *bios_name; -extern int only_migratable; extern const char *qemu_name; extern QemuUUID qemu_uuid; extern bool qemu_uuid_set; diff --git a/migration/migration.c b/migration/migration.c index 221b22c..67f9e68 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -115,6 +115,11 @@ MigrationState *migrate_get_current(void) return current_migration; } +void migration_only_migratable_set(void) +{ +migrate_get_current()->only_migratable = true; +} + MigrationIncomingState *migration_incoming_get_current(void) { static bool once; @@ -986,7 +991,7 @@ static GSList *migration_blockers; int migrate_add_blocker(Error *reason, Error **errp) { -if (only_migratable) { +if (migrate_get_current()->only_migratable) { error_propagate(errp, error_copy(reason)); error_prepend(errp, "disallowing migration blocker " "(--only_migratable) for: "); @@ -1979,6 +1984,7 @@ void migrate_fd_connect(MigrationState *s) static Property migration_properties[] = { DEFINE_PROP_BOOL("store-global-state", MigrationState, store_global_state, true), +DEFINE_PROP_BOOL("only-migratable", MigrationState, only_migratable, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/migration/migration.h b/migration/migration.h index 4b898e9..34377dd 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -139,6 +139,9 @@ struct MigrationState * during migration. */ bool store_global_state; + +/* Whether the VM is only allowing for migratable devices */ +bool only_migratable; }; void migrate_set_state(int *state, int old_state, int new_state); diff --git a/migration/savevm.c b/migration/savevm.c index 6bfd489..917ab05 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2314,7 +2314,7 @@ void vmstate_register_ram_global(MemoryRegion *mr) bool vmstate_check_only_migratable(const VMStateDescription *vmsd) { /* check needed if --only-migratable is specified */ -if (!only_migratable) { +if (!migrate_get_current()->only_migratable) { return true; } diff --git a/vl.c b/vl.c index 9b04ba7..57ff065 100644 --- a/vl.c +++ b/vl.c @@ -188,7 +188,6 @@ bool boot_strict; uint8_t *boot_splash_filedata; size_t boot_splash_filedata_size; uint8_t qemu_extra_params_fw[2]; -int only_migratable; /* turn it off unless user states otherwise */ int icount_align_option; @@ -3953,7 +3952,13 @@ int main(int argc, char **argv, char **envp) incoming = optarg; break; case QEMU_OPTION_only_migratable: -only_migratable = 1; +/* + * TODO: we can remove this option one day, and we + * should all use: + * + * "-global migration.only-migratable=true" + */ +migration_only_migratable_set(); break; case QEMU_OPTION_nodefaults: has_defaults = 0; -- 2.7.4
[Qemu-devel] [PATCH v5 03/10] vl: clean up global property registerations
It's not that clear on how the global properties are registered to global_props (and also its priority relationship). Let's provide a single function to be called in main() for that, with comment to explain it a bit. Signed-off-by: Peter Xu--- vl.c | 29 - 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/vl.c b/vl.c index 4452d7a..cdd2ec8 100644 --- a/vl.c +++ b/vl.c @@ -2969,6 +2969,25 @@ static int qemu_read_default_config_file(void) return 0; } +static void user_register_compat_props(void) +{ +qemu_opts_foreach(qemu_find_opts("global"), + global_init_func, NULL, NULL); +} + +/* + * Note: we should see that these compat properties are actually + * having a priority: accel < machine < user. This means e.g. when + * user specifies something in "-global", it'll always be used with + * highest priority. + */ +static void register_global_properties(MachineState *ms) +{ +accel_register_compat_props(ms->accelerator); +machine_register_compat_props(ms); +user_register_compat_props(); +} + int main(int argc, char **argv, char **envp) { int i; @@ -4571,11 +4590,11 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } -accel_register_compat_props(current_machine->accelerator); -machine_register_compat_props(current_machine); - -qemu_opts_foreach(qemu_find_opts("global"), - global_init_func, NULL, NULL); +/* + * Register all the global properties, including accel properties, + * machine properties, and user-specified ones. + */ +register_global_properties(current_machine); /* This checkpoint is required by replay to separate prior clock reading from the other reads, because timer polling functions query -- 2.7.4
[Qemu-devel] [PATCH v5 04/10] migration: let MigrationState be a qdev
Let the old man "MigrationState" join the object family. Direct benefit is that we can start to use all the property features derived from current QDev, like: HW_COMPAT_* bits, command line setup for migration parameters (so will never need to set them up each time using HMP/QMP, this is really, really attractive for test writters), etc. I see no reason to disallow this happen yet. So let's start from this one, to see whether it would be anything good. Now we init the MigrationState struct statically in main() to make sure it's initialized after global properties are applied, since we'll use them during creation of the object. No functional change at all. Reviewed-by: Juan QuintelaSigned-off-by: Peter Xu --- include/migration/misc.h | 1 + migration/migration.c| 78 ++-- migration/migration.h| 19 vl.c | 6 4 files changed, 81 insertions(+), 23 deletions(-) diff --git a/include/migration/misc.h b/include/migration/misc.h index 65c7070..2d36cf5 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -45,6 +45,7 @@ void savevm_skip_section_footers(void); void savevm_skip_configuration(void); /* migration/migration.c */ +void migration_object_init(void); void qemu_start_incoming_migration(const char *uri, Error **errp); bool migration_is_idle(void); void add_migration_state_change_notifier(Notifier *notify); diff --git a/migration/migration.c b/migration/migration.c index f588329..2c25927 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -98,32 +98,21 @@ enum mig_rp_message_type { migrations at once. For now we don't need to add dynamic creation of migration */ +static MigrationState *current_migration; + +void migration_object_init(void) +{ +/* This can only be called once. */ +assert(!current_migration); +current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION)); +} + /* For outgoing */ MigrationState *migrate_get_current(void) { -static bool once; -static MigrationState current_migration = { -.state = MIGRATION_STATUS_NONE, -.xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE, -.mbps = -1, -.parameters = { -.compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL, -.compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT, -.decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, -.cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL, -.cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, -.max_bandwidth = MAX_THROTTLE, -.downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, -.x_checkpoint_delay = DEFAULT_MIGRATE_X_CHECKPOINT_DELAY, -}, -}; - -if (!once) { -current_migration.parameters.tls_creds = g_strdup(""); -current_migration.parameters.tls_hostname = g_strdup(""); -once = true; -} -return _migration; +/* This can only be called after the object created. */ +assert(current_migration); +return current_migration; } MigrationIncomingState *migration_incoming_get_current(void) @@ -1987,3 +1976,46 @@ void migrate_fd_connect(MigrationState *s) s->migration_thread_running = true; } +static void migration_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); + +dc->user_creatable = false; +} + +static void migration_instance_init(Object *obj) +{ +MigrationState *ms = MIGRATION_OBJ(obj); + +ms->state = MIGRATION_STATUS_NONE; +ms->xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE; +ms->mbps = -1; +ms->parameters = (MigrationParameters) { +.compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL, +.compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT, +.decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, +.cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL, +.cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, +.max_bandwidth = MAX_THROTTLE, +.downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, +.x_checkpoint_delay = DEFAULT_MIGRATE_X_CHECKPOINT_DELAY, +}; +ms->parameters.tls_creds = g_strdup(""); +ms->parameters.tls_hostname = g_strdup(""); +} + +static const TypeInfo migration_type = { +.name = TYPE_MIGRATION, +.parent = TYPE_DEVICE, +.class_init = migration_class_init, +.class_size = sizeof(MigrationClass), +.instance_size = sizeof(MigrationState), +.instance_init = migration_instance_init, +}; + +static void register_migration_types(void) +{ +type_register_static(_type); +} + +type_init(register_migration_types); diff --git a/migration/migration.h b/migration/migration.h index d9a268a..3fca364 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -19,6 +19,7 @@ #include "qapi-types.h" #include
[Qemu-devel] [PATCH v5 02/10] accel: introduce AccelClass.global_props
Introduce this new field for the accelerator classes so that each specific accelerator in the future can register its own global properties to be used further by the system. It works just like how the old machine compatible properties do, but only tailored for accelerators. Introduce register_compat_props_array() for it. Export it so that it may be used in other codes as well in the future. Suggested-by: Eduardo HabkostSigned-off-by: Peter Xu --- accel/accel.c| 6 ++ hw/core/qdev-properties.c| 7 +++ include/hw/qdev-properties.h | 1 + include/sysemu/accel.h | 10 ++ vl.c | 1 + 5 files changed, 25 insertions(+) diff --git a/accel/accel.c b/accel/accel.c index 7c079a5..fa85844 100644 --- a/accel/accel.c +++ b/accel/accel.c @@ -120,6 +120,12 @@ void configure_accelerator(MachineState *ms) } } +void accel_register_compat_props(AccelState *accel) +{ +AccelClass *class = ACCEL_GET_CLASS(accel); +register_compat_props_array(class->global_props); +} + static void register_accel_types(void) { type_register_static(_type); diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 6ff1ac3..19fa335 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1060,6 +1060,13 @@ void register_compat_prop(const char *driver, qdev_prop_register_global(p); } +void register_compat_props_array(GlobalProperty *prop) +{ +for (; prop && prop->driver; prop++) { +register_compat_prop(prop->driver, prop->property, prop->value); +} +} + void qdev_prop_register_global_list(GlobalProperty *props) { int i; diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 1722ca4..2bac6a0 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -203,6 +203,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, void register_compat_prop(const char *driver, const char *property, const char *value); +void register_compat_props_array(GlobalProperty *prop); /** * qdev_property_add_static: diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h index 15944c1..effcd7f 100644 --- a/include/sysemu/accel.h +++ b/include/sysemu/accel.h @@ -24,6 +24,7 @@ #define HW_ACCEL_H #include "qom/object.h" +#include "hw/qdev-properties.h" typedef struct AccelState { /*< private >*/ @@ -40,6 +41,13 @@ typedef struct AccelClass { int (*available)(void); int (*init_machine)(MachineState *ms); bool *allowed; +/* + * Array of global properties that would be applied when specific + * accelerator is chosen. It works just like + * MachineClass.compat_props but it's for accelerators not + * machines. + */ +GlobalProperty *global_props; } AccelClass; #define TYPE_ACCEL "accel" @@ -57,5 +65,7 @@ typedef struct AccelClass { extern int tcg_tb_size; void configure_accelerator(MachineState *ms); +/* Register accelerator specific global properties */ +void accel_register_compat_props(AccelState *accel); #endif diff --git a/vl.c b/vl.c index 59fea15..4452d7a 100644 --- a/vl.c +++ b/vl.c @@ -4571,6 +4571,7 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } +accel_register_compat_props(current_machine->accelerator); machine_register_compat_props(current_machine); qemu_opts_foreach(qemu_find_opts("global"), -- 2.7.4
[Qemu-devel] [PATCH v5 00/10] migration: objectify MigrationState
v5: - add r-b for Juan on patch 4,5,7-10 - patch 2: introduce register_compat_props_array() that may be further used by machine codes [Eduardo] - patch 2: fix english error [Eduardo] v4: - dropped lots of patches related to AccelState.global_props. Now I am using AccelClass.global_props, and only use it in Xen codes. - one patch is added to clean up the global property registerations, with some comment to show the ordering. - one patch is added to dump migration globals in "info migrate" unconditionally (last patch). - changed all the following xen_init() patches to use AccelClass.global_props. - (since most of the patches are either touched-up again, or threw away, so no much r-b added, only one for Juan on the only_migratable patch, anyway, thanks for reviewing the old codes!) The main goal of this series is to let MigrationState be a QDev. It helps in many use cases. First of all, we can remove many legacy tricky functions. To name some: savevm_skip_section_footers(), savevm_skip_configuration(), etc. They didn't do much thing but setup a bool value. If MigrationState can be a QDev, then these things can be setup by the HW_COMPAT_* magic with some lines like: { .driver = "migration", .property = "send-configuration", .value= "off", } Next, if this can be merged and okay, we can move on to convert more things into properties for migration. A very attractive use case of it is, we will be able to do this for migration: -global migration.postcopy=on Then we don't need to use either HMP/QMP interface to enable it. It greatly simplifies the migration test scripts. Why QDev not QObject? The answer is simple: all the magic that we want for migration is bound to QDev (HW_COMPAT, "-global"). If one day we want to move these features from QDev to QObject, that'll be fine and easy for MigrationState. But before that, let's have MigrationState a QDev. :-) Please kindly review. Thanks. Peter Xu (10): machine: export register_compat_prop() accel: introduce AccelClass.global_props vl: clean up global property registerations migration: let MigrationState be a qdev migration: move global_state.optional out migration: move only_migratable to MigrationState migration: move skip_configuration out migration: move skip_section_footers migration: merge enforce_config_section somewhat migration: hmp: dump globals accel/accel.c| 6 ++ hmp.c| 3 + hw/core/machine.c| 13 - hw/core/qdev-properties.c| 20 +++ hw/i386/pc_piix.c| 3 - hw/ppc/spapr.c | 3 - hw/xen/xen-common.c | 25 +++-- include/hw/compat.h | 12 include/hw/qdev-properties.h | 4 ++ include/migration/global_state.h | 1 - include/migration/misc.h | 6 +- include/sysemu/accel.h | 10 include/sysemu/sysemu.h | 1 - migration/global_state.c | 9 +-- migration/migration.c| 118 +++ migration/migration.h| 33 +++ migration/savevm.c | 32 ++- vl.c | 41 -- 18 files changed, 250 insertions(+), 90 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v5 01/10] machine: export register_compat_prop()
We have HW_COMPAT_*, however that's only bound to machines, not other things (like accelerators). Behind it, it was register_compat_prop() that played the trick. Let's export the function for further use outside HW_COMPAT_* magic. Meanwhile, move it to qdev-properties.c where seems more proper (since it'll be used not only in machine codes). Signed-off-by: Peter Xu--- hw/core/machine.c| 13 - hw/core/qdev-properties.c| 13 + include/hw/qdev-properties.h | 3 +++ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 2e7e977..ecb5552 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -770,19 +770,6 @@ static void machine_class_finalize(ObjectClass *klass, void *data) g_free(mc->name); } -static void register_compat_prop(const char *driver, - const char *property, - const char *value) -{ -GlobalProperty *p = g_new0(GlobalProperty, 1); -/* Machine compat_props must never cause errors: */ -p->errp = _abort; -p->driver = driver; -p->property = property; -p->value = value; -qdev_prop_register_global(p); -} - static void machine_register_compat_for_subclass(ObjectClass *oc, void *opaque) { GlobalProperty *p = opaque; diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 9f1a497..6ff1ac3 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1047,6 +1047,19 @@ void qdev_prop_register_global(GlobalProperty *prop) global_props = g_list_append(global_props, prop); } +void register_compat_prop(const char *driver, + const char *property, + const char *value) +{ +GlobalProperty *p = g_new0(GlobalProperty, 1); +/* Machine compat_props must never cause errors: */ +p->errp = _abort; +p->driver = driver; +p->property = property; +p->value = value; +qdev_prop_register_global(p); +} + void qdev_prop_register_global_list(GlobalProperty *props) { int i; diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index d206fc9..1722ca4 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -201,6 +201,9 @@ void qdev_prop_set_globals(DeviceState *dev); void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, Property *prop, const char *value); +void register_compat_prop(const char *driver, const char *property, + const char *value); + /** * qdev_property_add_static: * @dev: Device to add the property to. -- 2.7.4
Re: [Qemu-devel] [PATCH v4 02/10] accel: introduce AccelClass.global_props
On Thu, Jun 22, 2017 at 02:28:52PM -0300, Eduardo Habkost wrote: > On Thu, Jun 22, 2017 at 12:25:30PM +0800, Peter Xu wrote: > > On Wed, Jun 21, 2017 at 09:23:11AM -0300, Eduardo Habkost wrote: > > > On Wed, Jun 21, 2017 at 03:52:00PM +0800, Peter Xu wrote: > [...] > > > > diff --git a/vl.c b/vl.c > > > > index 59fea15..4452d7a 100644 > > > > --- a/vl.c > > > > +++ b/vl.c > > > > @@ -4571,6 +4571,7 @@ int main(int argc, char **argv, char **envp) > > > > exit (i == 1 ? 1 : 0); > > > > } > > > > > > > > > > I suggest a comment here warning about global property > > > registration ordering. e.g.: > > > > > > /* > > > * Ordering of global property registration matters: > > > * - machine compat_props should override accelerator-specific > > > * globals. > > > * - user-provided globals (-global, and global properties > > > * derived from other command-line options like -cpu) should > > > * override machine compat_props and accelerator-specific > > > * globals. > > > */ > > > > I have a standalone patch (next one) to unify these three places and > > added some comment. Would that suite? > > Sorry, I didn't see that patch when I was reviewing this one. It looks > good to me. Thanks! Got it. Let me respin to fix the other places. Thanks! -- Peter Xu
[Qemu-devel] [PATCH] vhost: Fix use-after-free in vhost_log_put()
In commit 9e0bc24f dev->log_size was reset to zero too early before syncing vhost log. It causes syncing to be skipped. Move it to clear dev->log* after use. Signed-off-by: Jia-Shiun Li--- hw/virtio/vhost.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 6eddb09..c9ddf11 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -375,8 +375,6 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync) if (!log) { return; } -dev->log = NULL; -dev->log_size = 0; --log->refcnt; if (log->refcnt == 0) { @@ -396,6 +394,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync) g_free(log); } +dev->log = NULL; +dev->log_size = 0; } static bool vhost_dev_log_is_shared(struct vhost_dev *dev) -- 2.7.4
Re: [Qemu-devel] Query on VFIO in Virtual machine
On Thu, Jun 22, 2017 at 11:27:09AM -0600, Alex Williamson wrote: > On Thu, 22 Jun 2017 22:42:19 +0530 > Nitin Saxenawrote: > > > Thanks Alex. > > > > >> Without an iommu in the VM, you'd be limited to no-iommu support for VM > > >> userspace, > > So are you trying to say VFIO NO-IOMMU should work inside VM. Does > > that mean VFIO NO-IOMMU in VM and VFIO IOMMU in host for same device > > is a legitimate configuration? I did tried this configuration and the > > application (in VM) seems to get container_fd, group_fd, device_fd > > successfully but after VFIO_DEVICE_RESET ioctl the PCI link breaks > > from VM as well as from host. This could be specific to PCI endpoint > > device which I can dig. > > > > I will be happy if VFIO NO-IOMMU in VM and IOMMU in host for same > > device is legitimate configuration. > > Using no-iommu in the guest should work in that configuration, however > there's no isolation from the user to the rest of VM memory, so the VM > kernel will be tainted. Host memory does have iommu isolation. Device > reset from VM userspace sounds like another bug to investigate. Thanks, > > Alex Besides what Alex has mentioned, there is a wiki page for the usage. The command line will be slightly different on QEMU side comparing to without vIOMMU: http://wiki.qemu.org/Features/VT-d#With_Assigned_Devices One more thing to mention is that, when vfio-pci devices in the guest are used with emulated VT-d, huge performance degradation will be expected for dynamic allocations at least for now. While for mostly static allocations (like DPDK) the performance should be merely the same as no-IOMMU mode. It's just a hint on performance, and I believe for your own case it should mostly depend on how the application is managing DMA map/unmaps. Thanks, -- Peter Xu
[Qemu-devel] [PATCH RESEND] QEMU Guest Agent: Fix memory leak of device information set
The caller of SetupDiGetClassDevs must delete the returned device information set when it is no longer needed by calling SetupDiDestroyDeviceInfoList. Signed-off-by: Li PingReviewed-by: Marc-André Lureau --- qga/commands-win32.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 439d229..6f16457 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -512,7 +512,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) } else { error_setg_win32(errp, GetLastError(), "failed to get device name"); -goto out; +goto free_dev_info; } } @@ -560,6 +560,9 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) pci->bus = bus; break; } + +free_dev_info: +SetupDiDestroyDeviceInfoList(dev_info); out: g_free(buffer); g_free(name); -- 1.8.3.1
[Qemu-devel] [PATCH v3 2/2] virtio-net: code cleanup
Use is_power_of_2(), and remove trailing "." from error_setg(). Signed-off-by: Wei Wang--- hw/net/virtio-net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index d13ca60..b42423c 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1924,9 +1924,9 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) */ if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE || n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE || -(n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) { +!is_power_of_2(n->net_conf.rx_queue_size)) { error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), " - "must be a power of 2 between %d and %d.", + "must be a power of 2 between %d and %d", n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE, VIRTQUEUE_MAX_SIZE); virtio_cleanup(vdev); @@ -1947,7 +1947,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->max_queues = MAX(n->nic_conf.peers.queues, 1); if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) { error_setg(errp, "Invalid number of queues (= %" PRIu32 "), " - "must be a positive integer less than %d.", + "must be a positive integer less than %d", n->max_queues, (VIRTIO_QUEUE_MAX - 1) / 2); virtio_cleanup(vdev); return; -- 2.7.4
[Qemu-devel] [PATCH v3 1/2] virtio-net: enable configurable tx queue size
This patch enables the virtio-net tx queue size to be configurable between 256 (the default queue size) and 1024 by the user when the vhost-user backend is used. Currently, the maximum tx queue size for other backends is 512 due to the following limitations: - QEMU backend: the QEMU backend implementation in some cases may send 1024+1 iovs to writev. - Vhost_net backend: there are possibilities that the guest sends a vring_desc of memory which corsses a MemoryRegion thereby generating more than 1024 iovs after translattion from guest-physical address in the backend. Signed-off-by: Wei Wang--- hw/net/virtio-net.c| 45 +- include/hw/virtio/virtio-net.h | 1 + 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 91eddaf..d13ca60 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -34,8 +34,11 @@ /* previously fixed value */ #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 +#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 + /* for now, only allow larger queues; with virtio-1, guest can downsize */ #define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE /* * Calculate the number of bytes up to and including the given 'field' of @@ -1508,15 +1511,18 @@ static void virtio_net_add_queue(VirtIONet *n, int index) n->vqs[index].rx_vq = virtio_add_queue(vdev, n->net_conf.rx_queue_size, virtio_net_handle_rx); + if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) { n->vqs[index].tx_vq = -virtio_add_queue(vdev, 256, virtio_net_handle_tx_timer); +virtio_add_queue(vdev, n->net_conf.tx_queue_size, + virtio_net_handle_tx_timer); n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, virtio_net_tx_timer, >vqs[index]); } else { n->vqs[index].tx_vq = -virtio_add_queue(vdev, 256, virtio_net_handle_tx_bh); +virtio_add_queue(vdev, n->net_conf.tx_queue_size, + virtio_net_handle_tx_bh); n->vqs[index].tx_bh = qemu_bh_new(virtio_net_tx_bh, >vqs[index]); } @@ -1927,6 +1933,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) return; } +if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE || +n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE || +!is_power_of_2(n->net_conf.tx_queue_size)) { +error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), " + "must be a power of 2 between %d and %d", + n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE, + VIRTQUEUE_MAX_SIZE); +virtio_cleanup(vdev); +return; +} + n->max_queues = MAX(n->nic_conf.peers.queues, 1); if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) { error_setg(errp, "Invalid number of queues (= %" PRIu32 "), " @@ -1947,17 +1964,11 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) error_report("Defaulting to \"bh\""); } -for (i = 0; i < n->max_queues; i++) { -virtio_net_add_queue(n, i); -} - -n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); qemu_macaddr_default_if_unset(>nic_conf.macaddr); memcpy(>mac[0], >nic_conf.macaddr, sizeof(n->mac)); n->status = VIRTIO_NET_S_LINK_UP; n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, virtio_net_announce_timer, n); - if (n->netclient_type) { /* * Happen when virtio_net_set_netclient_name has been called. @@ -1968,6 +1979,21 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->nic = qemu_new_nic(_virtio_info, >nic_conf, object_get_typename(OBJECT(dev)), dev->id, n); } +nc = qemu_get_queue(n->nic); + +/* + * Currently, backends other than vhost-user don't support 1024 queue + * size. + */ +if (n->net_conf.tx_queue_size == VIRTQUEUE_MAX_SIZE && +nc->peer->info->type != NET_CLIENT_DRIVER_VHOST_USER) { +n->net_conf.tx_queue_size = VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; +} + +for (i = 0; i < n->max_queues; i++) { +virtio_net_add_queue(n, i); +} +n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); peer_test_vnet_hdr(n); if (peer_has_vnet_hdr(n)) { @@ -1990,7 +2016,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->vlans = g_malloc0(MAX_VLAN >> 3); -nc = qemu_get_queue(n->nic); nc->rxfilter_notify_enabled = 1; n->qdev = dev; @@ -2106,6 +2131,8 @@ static Property virtio_net_properties[] = {
Re: [Qemu-devel] [virtio-dev] Re: [PATCH v2 1/2] virtio-net: enable configurable tx queue size
On 06/22/2017 10:00 PM, Michael S. Tsirkin wrote: On Sat, Jun 17, 2017 at 04:38:03PM +0800, Wei Wang wrote: On 06/16/2017 10:31 PM, Michael S. Tsirkin wrote: On Fri, Jun 16, 2017 at 06:48:38PM +0800, Wei Wang wrote: This patch enables the virtio-net tx queue size to be configurable between 256 (the default queue size) and 1024 by the user when the vhost-user backend is used. Currently, the maximum tx queue size for other backends is 512 due to the following limitations: - QEMU backend: the QEMU backend implementation in some cases may send 1024+1 iovs to writev. - Vhost_net backend: there are possibilities that the guest sends a vring_desc of memory which corsses a MemoryRegion thereby generating more than 1024 iovs in total after translattion from guest-physical address in the backend. Signed-off-by: Wei Wang--- hw/net/virtio-net.c| 46 ++ include/hw/virtio/virtio-net.h | 1 + 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 7d091c9..e1a08fd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -33,8 +33,11 @@ /* previously fixed value */ #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 +#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 + /* for now, only allow larger queues; with virtio-1, guest can downsize */ #define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE /* * Calculate the number of bytes up to and including the given 'field' of @@ -1491,18 +1494,33 @@ static void virtio_net_tx_bh(void *opaque) static void virtio_net_add_queue(VirtIONet *n, int index) { VirtIODevice *vdev = VIRTIO_DEVICE(n); +NetClientState *nc = qemu_get_queue(n->nic); n->vqs[index].rx_vq = virtio_add_queue(vdev, n->net_conf.rx_queue_size, virtio_net_handle_rx); + +/* + * Currently, backends other than vhost-user don't support 1024 queue + * size. + */ +if (n->net_conf.tx_queue_size == VIRTQUEUE_MAX_SIZE && +nc->peer->info->type != NET_CLIENT_DRIVER_VHOST_USER) { +fprintf(stderr, "warning: %s: queue size %d not supported\n", +__func__, n->net_conf.tx_queue_size); +n->net_conf.tx_queue_size = VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; +} + Also, I suspect we can get here with no peer, and above will crash. It seems ugly to do this on each virtio_net_add_queue. How about moving this to realize? The code has been re-arranged to make sure nc->peer is ready before it's used, but I agree that it looks better to move the above to realize(). Best, Wei ping The issues left are minor, let's make progress and merge this asap OK. I'll send out the new version soon. Best, Wei
Re: [Qemu-devel] [PATCH 12/31] slirp: use DIV_ROUND_UP
Marc-André Lureau, on jeu. 22 juin 2017 14:41:45 +0200, wrote: > I used the clang-tidy qemu-round check to generate the fix: > https://github.com/elmarco/clang-tools-extra > > Signed-off-by: Marc-André LureauApplied to my tree, thanks! > --- > slirp/ip6.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/slirp/ip6.h b/slirp/ip6.h > index 0908855f0f..b1bea43b3c 100644 > --- a/slirp/ip6.h > +++ b/slirp/ip6.h > @@ -57,9 +57,9 @@ static inline bool in6_equal_mach(const struct in6_addr *a, >const struct in6_addr *b, >int prefix_len) > { > -if (memcmp(&(a->s6_addr[(prefix_len + 7) / 8]), > - &(b->s6_addr[(prefix_len + 7) / 8]), > - 16 - (prefix_len + 7) / 8) != 0) { > +if (memcmp(&(a->s6_addr[DIV_ROUND_UP(prefix_len, 8)]), > + &(b->s6_addr[DIV_ROUND_UP(prefix_len, 8)]), > + 16 - DIV_ROUND_UP(prefix_len, 8)) != 0) { > return 0; > } > > -- > 2.13.1.395.gf7b71de06 > -- Samuel N: j'aime bien Cut d'un truc enorme... ca montre de quel cote de l'ecran sont les gonades :))) -+- #ens-mim et la peufeupeu -+-
Re: [Qemu-devel] NVDIMM live migration broken?
On 06/22/17 15:08 +0100, Stefan Hajnoczi wrote: > I tried live migrating a guest with NVDIMM on qemu.git/master (edf8bc984): > > $ qemu -M accel=kvm,nvdimm=on -m 1G,slots=4,maxmem=8G -cpu host \ > -object > memory-backend-file,id=mem1,share=on,mem-path=nvdimm.dat,size=1G \ >-device nvdimm,id=nvdimm1,memdev=mem1 \ >-drive if=virtio,file=test.img,format=raw > > $ qemu -M accel=kvm,nvdimm=on -m 1G,slots=4,maxmem=8G -cpu host \ > -object > memory-backend-file,id=mem1,share=on,mem-path=nvdimm.dat,size=1G \ >-device nvdimm,id=nvdimm1,memdev=mem1 \ >-drive if=virtio,file=test.img,format=raw \ >-incoming tcp::1234 > > (qemu) migrate tcp:127.0.0.1:1234 > > The guest kernel panics or hangs every time on the destination. It > happens as long as the nvdimm device is present - I didn't even mount it > inside the guest. > > Is migration expected to work? Yes, I tested on QEMU 2.8.0 several months ago and it worked. I'll have a look at this issue. Haozhong > > If not we need a migration blocker so that users get a graceful error > message. > > Stefan
[Qemu-devel] [PATCH v1] target-s390x: fix risbg handling
If we have for example: r3 contains 0x ec 33 3f bf 61 55 risbg %r3,%r3,63,191,97 We want to rotate 33 to the left and only keep MSB bit 63 of that. So the result is then exactly 1 (we're reading the sign of the 32 bit value). Current code assumes that we can do that via an extract, which is not true (at least not that easy) and produces a 0. Let's just get rid of this special handling. Signed-off-by: David Hildenbrand--- This effectively allows to start a linux kernel, compiled for z10 using the qemu model under tcg (with other patches currently on the list): qemu-system-s390x ... -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on, \ eimm=on,stckf=on,csst=on,csst2=on,ginste=on, \ exrl=on ... I found this by compiling the kvm-unit-tests for z10 and noticing elementary selftests failing. The kernel would trigger weird BUG_ONs very early while starting up, which basically gave not really many hints of what was actually going wrong. target/s390x/translate.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 188ab8b..81419dd 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -3450,12 +3450,6 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o) pos += 32; } -/* In some cases we can implement this with extract. */ -if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) { -tcg_gen_extract_i64(o->out, o->in2, rot, len); -return NO_EXIT; -} - /* In some cases we can implement this with deposit. */ if (len > 0 && (imask == 0 || ~mask == imask)) { /* Note that we rotate the bits to be inserted to the lsb, not to -- 2.9.4
Re: [Qemu-devel] [PATCH v2 3/3] xen-disk: use an IOThread per instance
CC'ing Andreas Färber. Could you please give a quick look below at the way the iothread object is instantiate and destroyed? I am no object model expert and would appreaciate a second opinion. On Wed, 21 Jun 2017, Paul Durrant wrote: > This patch allocates an IOThread object for each xen_disk instance and > sets the AIO context appropriately on connect. This allows processing > of I/O to proceed in parallel. > > The patch also adds tracepoints into xen_disk to make it possible to > follow the state transtions of an instance in the log. > > Signed-off-by: Paul Durrant> --- > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: Kevin Wolf > Cc: Max Reitz > > v2: > - explicitly acquire and release AIO context in qemu_aio_complete() and >blk_bh() > --- > hw/block/trace-events | 7 ++ > hw/block/xen_disk.c | 69 > --- > 2 files changed, 67 insertions(+), 9 deletions(-) > > diff --git a/hw/block/trace-events b/hw/block/trace-events > index 65e83dc258..608b24ba66 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -10,3 +10,10 @@ virtio_blk_submit_multireq(void *mrb, int start, int > num_reqs, uint64_t offset, > # hw/block/hd-geometry.c > hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p > LCHS %d %d %d" > hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, > int trans) "blk %p CHS %u %u %u trans %d" > + > +# hw/block/xen_disk.c > +xen_disk_alloc(char *name) "%s" > +xen_disk_init(char *name) "%s" > +xen_disk_connect(char *name) "%s" > +xen_disk_disconnect(char *name) "%s" > +xen_disk_free(char *name) "%s" > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 0e6513708e..8548195195 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -27,10 +27,13 @@ > #include "hw/xen/xen_backend.h" > #include "xen_blkif.h" > #include "sysemu/blockdev.h" > +#include "sysemu/iothread.h" > #include "sysemu/block-backend.h" > #include "qapi/error.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qstring.h" > +#include "qom/object_interfaces.h" > +#include "trace.h" > > /* - */ > > @@ -128,6 +131,9 @@ struct XenBlkDev { > DriveInfo *dinfo; > BlockBackend*blk; > QEMUBH *bh; > + > +IOThread*iothread; > +AioContext *ctx; > }; > > /* - */ > @@ -599,9 +605,12 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq); > static void qemu_aio_complete(void *opaque, int ret) > { > struct ioreq *ioreq = opaque; > +struct XenBlkDev *blkdev = ioreq->blkdev; > + > +aio_context_acquire(blkdev->ctx); I think that Paolo was right that we need a aio_context_acquire here, however the issue is that with the current code: blk_handle_requests -> ioreq_runio_qemu_aio -> qemu_aio_complete leading to aio_context_acquire being called twice on the same lock, which I don't think is allowed? I think we need to get rid of the qemu_aio_complete call from ioreq_runio_qemu_aio, but to do that we need to be careful with the accounting of aio_inflight (today it's incremented unconditionally at the beginning of ioreq_runio_qemu_aio, I think we would have to change that to increment it only if presync). > if (ret != 0) { > -xen_pv_printf(>blkdev->xendev, 0, "%s I/O error\n", > +xen_pv_printf(>xendev, 0, "%s I/O error\n", >ioreq->req.operation == BLKIF_OP_READ ? "read" : > "write"); > ioreq->aio_errors++; > } > @@ -610,13 +619,13 @@ static void qemu_aio_complete(void *opaque, int ret) > if (ioreq->presync) { > ioreq->presync = 0; > ioreq_runio_qemu_aio(ioreq); > -return; > +goto done; > } > if (ioreq->aio_inflight > 0) { > -return; > +goto done; > } > > -if (ioreq->blkdev->feature_grant_copy) { > +if (blkdev->feature_grant_copy) { > switch (ioreq->req.operation) { > case BLKIF_OP_READ: > /* in case of failure ioreq->aio_errors is increased */ > @@ -638,7 +647,7 @@ static void qemu_aio_complete(void *opaque, int ret) > } > > ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY; > -if (!ioreq->blkdev->feature_grant_copy) { > +if (!blkdev->feature_grant_copy) { > ioreq_unmap(ioreq); > } > ioreq_finish(ioreq); > @@ -650,16 +659,19 @@ static void qemu_aio_complete(void *opaque, int ret) > } > case BLKIF_OP_READ: > if (ioreq->status == BLKIF_RSP_OKAY) { > -block_acct_done(blk_get_stats(ioreq->blkdev->blk), >acct); > +block_acct_done(blk_get_stats(blkdev->blk), >acct); > } else { > -
[Qemu-devel] [Bug 1047470] Re: qemu/kvm hangs reading from serial console
i don't use kvm/qemu any more and so don't have a means to determine if this is still an issue or not so please presume fixed, i guess -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1047470 Title: qemu/kvm hangs reading from serial console Status in QEMU: Incomplete Bug description: This is for a qemu-kvm running on RHEL 5, so it's pretty old, but i think the problem still exists in 1.2 We have conman running on our hosts, connecting to the kvm/qemu's using virsh console which just opens up the console /dev/pts/slave that qemu opens up when run with options -nographic -serial mon:pty Sometimes virsh console exits and then qemu locks up[*]. My guess is that something like this happens: virsh console exits qemu does a select() on /dev/ptmx (and other FDs) select() returns the FD of /dev/ptmx in the read-fdset qemu does a read() read() returns -1 (EIO) qemu does other stuff for a while select() ... /dev/ptmx read() .. EIO other stuff select() ... read() ... select() ... read() ... select() conman starts a new virsh console that connects qemu does a read() read() blocks b/c there is now a writer on the tty slave So i don't see any way around this, given the sorta rudi- mentary semantics of TTY IO on Linux (not that i know of any platform that does it better ... ?), except ... maybe qemu should fcntl(master_fd, F_SETFL, flags | O_NONBLOCK) in qemu-char.c:qemu_char_open_pty() and be prepared to handle E_WOULDBLOCK|E_AGAIN in qemu-char.c:fd_chr_read() ... ? --buck [*] i think, b/c in the old version we are running, sometimes the guest spits out the ^] character to its console, and virsh console reads it and doesn't check to see if its from stdin or the pty and exits, which, i think, can be fixed like this: --- libvirt-0.8.2/tools/console.c.ctrl_close_bracket_handling_fix 2012-09-06 10:30:43.606997191 -0400 +++ libvirt-0.8.2/tools/console.c 2012-09-06 10:34:52.154000464 -0400 @@ -155,6 +155,7 @@ int vshRunConsole(const char *tty) { /* Quit if end of file, or we got the Ctrl-] key */ if (!got || +fds[i].fd == STDIN_FILENO && (got == 1 && buf[0] == CTRL_CLOSE_BRACKET)) goto done; To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1047470/+subscriptions
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On Thu, 22 Jun 2017 19:34:58 +0100 "Dr. David Alan Gilbert"wrote: > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > On 22 June 2017 at 19:08, Thomas Huth wrote: > > > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > > >> Could do; I'm just not finding tiny header files with one or > > >> two entries each that useful. > > > > Well, it means that the bulk of code that doesn't care about the > > types doesn't get its compilation fractionally slowed by having > > to parse the typedef anyway. In general I think we're drifting > > towards "have each .c file get fewer things automatically" rather > > than otherwise (eg more finely focused files rather than stuffing > > everything into qemu-common.h). > > At the cost of things getting fractionally slower by including lots > more tiny headers. > > I generally agree in the case where there's a useful chunk, > but when it's down to one or two definitions I don't see the point. > > > > Do we really need these function typedefs at all? IMHO it's quite ugly > > > to hide such things in a typedef unless it is really necessary (and in > > > this case, it does not seem to be really necessary since it is only used > > > in a few places). So what about simply removing the typedefs in this > > > case? > > > > I find function typedefs much more readable than having the > > function-types inline in function arguments and the like. > > > > This is all fairly rapidly heading into bikeshed territory > > though -- in the final analysis I don't think it matters > > much what we do. > > Agreed. > Last question for my own comprehension. I can't think of a case where we would do something like: some_vmsd->load_state_old = some_se->ops->load_state; Does it make sense for VMStateDescription::load_state_old and SaveVMHandlers::load_state to be of the same type ? > Dave > > > thanks > > -- PMM > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK pgpkEBHEg5Z1c.pgp Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/5] memory: Add RAM_NONPERSISTENT flag
* Eduardo Habkost (ehabk...@redhat.com) wrote: > On Thu, Jun 22, 2017 at 01:14:58PM +0100, Dr. David Alan Gilbert wrote: > > * Eduardo Habkost (ehabk...@redhat.com) wrote: > > > The new flag will make qemu_ram_free() discard the contents of the > > > block. It will be used to let QEMU be configured to avoid flushing file > > > contents to disk when exiting. As MADV_REMOVE is not always supported, > > > the new code will try MADV_NOTNEEDED in case MADV_REMOVE fails. > > > > I'd like to understand what semantics you're trying to achieve and thus > > why you prefer REMOVE to DONTNEED. If you're trying to avoid changes > > being written back then doesn't a DONTNEED get rid of any changes that > > have yet to be written? Or are there changes that have already been > > queued that REMOVE will kill off? > > > > Generally speaking, it look(ed) like REMOVE is a superset of DONTNEED: > DONTNEED will free and zero pages only on anonymous private mappings; > REMOVE will free resources and zero pages on additional cases. IMHO these calls are all pretty badly defined, and I think there are a few combinations of requirements that are missing. (For example I don't think I've found an equivalent to MADV_DONTNEED for the case of shared hugetlbfs where you want to drop the current pages from your mapping to cause it to repopulate from the shared file). > One case where I can think REMOVE would be useful is tmpfs when swapping > is involved: with REMOVE, the host can drop swap contents or avoid > writing memory contents to swap even if we are using a shared tmpfs > mapping. Yes. > Other filesystems might have similar cases where unnecessary I/O > operations might be performed even after madvise(MADV_DONTNEED) is > called. MADV_REMOVE lets us simply tell the kernel to drop the data. > > I'm CCing Zack Cornelius, who initially suggested MADV_REMOVE, in case > he can describe more specific use cases. > > > > If you're just trying to save-time in writeback, it's interesting to > > note my requirement is that by the time I exit this function the > > process of throwing away the memory contents must be complete; > > I think your requirements are a lot lazier as to when it happens. > > This is a very good point. I was assuming that REMOVE is a superset of > DONTNEED, but based on the manpage it doesn't seem to be guaranteed. > Probably I shouldn't try to reuse ram_block_discard_range() and write a > separate helper for madvise(MADV_REMOVE), as the requirements are > different. > > > > > The new flag will also indicate that ram_block_discard_range() can use > > > MADV_REMOVE when discarding memory pages. I have considered calling > > > MADV_REMOVE unconditionally (as destroying the RAM contents seems to be > > > OK every time ram_block_discard_range() is called), but for safety I > > > decided to restrict the new code to blocks having RAM_NONPERSISTENT set. > > > > The manpage on MADV_REMOVE is confusing; it says it doesn't work on Huge > > TLB pages, but says it does work on anything that can do > > FALLOC_FL_PUNCH_HOLE - which as far as I can tell hugetlbfs does. > > Yes, it's confusing. I need to do some testing to find out if HugeTLBFS > supports MADV_REMOVE today. But my use case is just an optimization, so > it won't be a big deal if it doesn't cover every case in the first > version. > > > > > I've got some code in my shared-postcopy world that has this function do > > the following which is kind of similar: > > > > /* The logic here is messy; > > *madvise DONTNEED fails for hugepages > > *fallocate works on hugepages and shmem > > */ > > need_madvise = (rb->page_size == qemu_host_page_size) && > >(rb->fd == -1 || !(rb->flags & RAM_SHARED)); > > need_fallocate = rb->fd != -1; > > This looks safer to me. I was bothered by the missing check for > (rb->fd != -1) in the current code. > > > if (ret == -1 && need_fallocate) { > > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > > ret = fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | > > FALLOC_FL_KEEP_SIZE, > > start, length); > > #endif > > } > > if (need_madvise && (!need_fallocate || (ret == 0))) { > > I'm confused by the (ret == 0) check here. Do you still want to call > madvise() if fallocate() succeeded? Hmm now I'm confused by that check as well; if we Demorgan the right side we get: if (need_madvice && !(need_fallocate && (ret != 0)) { ie we skip the DONTNEED in the case that we wanted an fallocate but it failed, so that we preserve the error and fail the function. I'm not at all sure the combinations cover all the shared cases. > > #if defined(CONFIG_MADVISE) > > ret = madvise(host_startaddr, length, MADV_DONTNEED); > > fprintf(stderr, "%s: Did madvise for %p got %d\n", __func__, > > host_startaddr, ret); > > #endif > > } > > > Anyway, now I'm considering
Re: [Qemu-devel] BUG: KASAN: use-after-free in free_old_xmit_skbs
On Thu, Jun 22, 2017 at 08:15:58AM +0200, jean-philippe menil wrote: > 2017-06-06 1:52 GMT+02:00 Michael S. Tsirkin: > > On Mon, Jun 05, 2017 at 05:08:25AM +0300, Michael S. Tsirkin wrote: > > On Mon, Jun 05, 2017 at 12:48:53AM +0200, Jean-Philippe Menil wrote: > > > Hi, > > > > > > while playing with xdp and ebpf, i'm hitting the following: > > > > > > [ 309.993136] > > > == > > > [ 309.994735] BUG: KASAN: use-after-free in > > > free_old_xmit_skbs.isra.29+0x2b7/0x2e0 [virtio_net] > > > [ 309.998396] Read of size 8 at addr 88006aa64220 by task > sshd/323 > > > [ 310.000650] > > > [ 310.002305] CPU: 1 PID: 323 Comm: sshd Not tainted 4.12.0-rc3+ #2 > > > [ 310.004018] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS > > > 1.10.2-20170228_101828-anatol 04/01/2014 ... > > > > Since commit 680557cf79f82623e2c4fd42733077d60a843513 > > virtio_net: rework mergeable buffer handling > > > > we no longer must do the resets, we now have enough space > > to store a bit saying whether a buffer is xdp one or not. > > > > And that's probably a cleaner way to fix these issues than > > try to find and fix the race condition. > > > > John? > > > > -- > > MST > > > I think I see the source of the race. virtio net calls > netif_device_detach and assumes no packets will be sent after > this point. However, all it does is stop all queues so > no new packets will be transmitted. > > Try locking with HARD_TX_LOCK? > > > -- > MST > > > Hi Michael, > > from what i see, the race appear when we hit virtnet_reset in virtnet_xdp_set. > virtnet_reset > _remove_vq_common > virtnet_del_vqs > virtnet_free_queues > kfree(vi->sq) > when the xdp program (with two instances of the program to trigger it faster) > is added or removed. > > It's easily repeatable, with 2 cpus and 4 queues on the qemu command line, > running the xdp_ttl tool from Jesper. > > For now, i'm able to continue my qualification, testing if xdp_qp is not null, > but do not seem to be a sustainable trick. > if (xdp_qp && vi->xdp_queues_pairs != xdp_qp) > > Maybe it will be more clear to you with theses informations. > > Best regards. > > Jean-Philippe I'm pretty clear about the issue here, I was trying to figure out a fix. Jason, any thoughts? -- MST
Re: [Qemu-devel] [PATCH] xen/disk: don't leak stack data via response ring
On Thu, 22 Jun 2017, Jan Beulich wrote: > >>> On 21.06.17 at 20:46,wrote: > > On Wed, 21 Jun 2017, Jan Beulich wrote: > >> >>> On 20.06.17 at 23:48, wrote: > >> > On Tue, 20 Jun 2017, Jan Beulich wrote: > >> >> @@ -36,13 +33,7 @@ struct blkif_x86_32_request_discard { > >> >> blkif_sector_t sector_number;/* start sector idx on disk (r/w > > only) */ > >> >> uint64_t nr_sectors; /* # of contiguous sectors to > >> >> discard > > */ > >> >> }; > >> >> -struct blkif_x86_32_response { > >> >> -uint64_tid; /* copied from request */ > >> >> -uint8_t operation; /* copied from request */ > >> >> -int16_t status; /* BLKIF_RSP_??? */ > >> >> -}; > >> >> typedef struct blkif_x86_32_request blkif_x86_32_request_t; > >> >> -typedef struct blkif_x86_32_response blkif_x86_32_response_t; > >> >> #pragma pack(pop) > >> >> > >> >> /* x86_64 protocol version */ > >> >> @@ -62,20 +53,14 @@ struct blkif_x86_64_request_discard { > >> >> blkif_sector_t sector_number;/* start sector idx on disk (r/w > > only) */ > >> >> uint64_t nr_sectors; /* # of contiguous sectors to > >> >> discard > > */ > >> >> }; > >> >> -struct blkif_x86_64_response { > >> >> -uint64_t __attribute__((__aligned__(8))) id; > >> >> -uint8_t operation; /* copied from request */ > >> >> -int16_t status; /* BLKIF_RSP_??? */ > >> >> -}; > >> >> > >> >> typedef struct blkif_x86_64_request blkif_x86_64_request_t; > >> >> -typedef struct blkif_x86_64_response blkif_x86_64_response_t; > >> >> > >> >> DEFINE_RING_TYPES(blkif_common, struct blkif_common_request, > >> >> - struct blkif_common_response); > >> >> + struct blkif_response); > >> >> DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request, > >> >> - struct blkif_x86_32_response); > >> >> + struct blkif_response QEMU_PACKED); > >> > > >> > In my test, the previous sizes and alignments of the response structs > >> > were (on both x86_32 and x86_64): > >> > > >> > sizeof(blkif_x86_32_response)=12 sizeof(blkif_x86_64_response)=16 > >> > align(blkif_x86_32_response)=4 align(blkif_x86_64_response)=8 > >> > > >> > While with these changes are now, when compiled on x86_64: > >> > sizeof(blkif_x86_32_response)=11 sizeof(blkif_x86_64_response)=16 > >> > align(blkif_x86_32_response)=1 align(blkif_x86_64_response)=8 > >> > > >> > when compiled on x86_32: > >> > sizeof(blkif_x86_32_response)=11 sizeof(blkif_x86_64_response)=12 > >> > align(blkif_x86_32_response)=1 align(blkif_x86_64_response)=4 > >> > > >> > Did I do my tests wrong? > >> > > >> > QEMU_PACKED is not the same as #pragma pack(push, 4). In fact, it is the > >> > same as #pragma pack(push, 1), causing the struct to be densely packed, > >> > leaving no padding whatsever. > >> > > >> > In addition, without __attribute__((__aligned__(8))), > >> > blkif_x86_64_response won't be 8 bytes aligned when built on x86_32. > >> > > >> > Am I missing something? > >> > >> Well, you're mixing attribute application upon structure > >> declaration with attribute application upon structure use. It's > >> the latter here, and hence the attribute doesn't affect > >> structure layout at all. All it does is avoid the _containing_ > >> 32-bit union to become 8-byte aligned (and tail padding to be > >> inserted). > > > > Thanks for the explanation. I admit it's the first time I see the > > aligned attribute being used at structure usage only. I think it's the > > first time QEMU_PACKED is used this way in QEMU too. > > > > Anyway, even taking that into account, things are still not completely > > right: the alignment of struct blkif_x86_32_response QEMU_PACKED is 4 > > bytes as you wrote, but the size of struct blkif_x86_32_response is > > still 16 bytes instead of 12 bytes in my test. I suspect it worked for > > you because the other member of the union (blkif_x86_32_request) is > > larger than that. However, I think is not a good idea to rely on this > > implementation detail. The implementation of DEFINE_RING_TYPES should be > > opaque from our point of view. We shouldn't have to know that there is a > > union there. > > I don't follow - why should we not rely on this? It is a fundamental > aspect of the shared ring model that requests and responses share > space. > > > Moreover, the other problem is still unaddressed: the size and alignment > > of blkif_x86_64_response when built on x86_32 are 12 and 4 instead of 16 > > and 8 bytes. Is that working also because it's relying on the other > > member of the union to enforce the right alignment and bigger size? > > Yes. For these as well as your comments further up - sizeof() and > alignof() are completely uninteresting as long as we don't > instantiate objects of those types _and
Re: [Qemu-devel] [PATCH 04/31] vhost: use QEMU_ALIGN_DOWN
On Thu, Jun 22, 2017 at 02:41:37PM +0200, Marc-André Lureau wrote: > I used the clang-tidy qemu-round check to generate the fix: > https://github.com/elmarco/clang-tools-extra > > Signed-off-by: Marc-André LureauReviewed-by: Michael S. Tsirkin > --- > hw/virtio/vhost.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 6eddb099b0..0049a2c0b3 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -70,7 +70,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, > uint64_t end = MIN(mlast, rlast); > vhost_log_chunk_t *from = log + start / VHOST_LOG_CHUNK; > vhost_log_chunk_t *to = log + end / VHOST_LOG_CHUNK + 1; > -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; > +uint64_t addr = QEMU_ALIGN_DOWN(start, VHOST_LOG_CHUNK); > > if (end < start) { > return; > -- > 2.13.1.395.gf7b71de06
[Qemu-devel] [Bug 1037606] Re: vmwgfx does not work with kvm vmware vga
OK, got your point ... but AFAIK the vmware display device in QEMU is pretty much unmaintened anyway, so unless someone steps up and takes care of this device, I think the WONT-FIX status is appropriate for this bug. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in Linux: Confirmed Status in QEMU: Won't Fix Status in linux package in Ubuntu: Triaged Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 "Quantal Quetzal" - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs --- ApportVersion: 2.0.1-0ubuntu12 Architecture: i386 DistroRelease: Ubuntu 12.04 InstallationMedia: Ubuntu 10.10 "Maverick Meerkat" - Release i386 (20101007) Package: linux (not installed) ProcEnviron: TERM=xterm PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash Tags: precise running-unity Uname: Linux 3.6.0-030600rc3-generic i686 UnreportableReason: The running kernel is not an Ubuntu kernel UpgradeStatus: Upgraded to precise on 2012-08-30 (0 days ago) UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare To manage notifications about this bug go to: https://bugs.launchpad.net/linux/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1699867] [NEW] x86_64 qemu crashes at far call into long-mode
Public bug reported: I am experimenting with this OS https://github.com/phil-opp/blog_os and spotted a weird behavior with qemu. I am looking at code that does transition from 32bit mode to 64bit mode. Right now it does 'jmp $SELECTOR,64bitfunction'. https://github.com /phil- opp/blog_os/blob/557c6a58ea11e31685b9d014d307002d64df5c22/src/arch/x86_64/boot.asm#L32 This code works fine with qemu/kvm/vmware. To transition from 32 to 64 bit code it is possible to use 'call' operation. So I am trying to replace that code with 'call $SELECTOR,64bitfunction'. It works fine with kvm and wmware. But it fails with Qemu emulation. See the interrup debug log attached. qemu crashes at 10302c (far call mnemonic). 103016: e8 18 00 00 00 callq 103033 10301b: e8 5c 00 00 00 callq 10307c 103020: e8 ec 00 00 00 callq 103111 103025: 0f 01 15 28 00 10 00lgdt 0x100028(%rip)# 20305410302c: 9a (bad) 10302d: 40 31 10rex xor %edx,(%rax) 103030: 00 08 add%cl,(%rax) As the code works at hardware I expect it to work with qemu. Thus current qemu behavior looks like a bug. ** Affects: qemu Importance: Undecided Status: New ** Attachment added: "Debug log for qemu crash at far call" https://bugs.launchpad.net/bugs/1699867/+attachment/4901095/+files/debug.log -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1699867 Title: x86_64 qemu crashes at far call into long-mode Status in QEMU: New Bug description: I am experimenting with this OS https://github.com/phil-opp/blog_os and spotted a weird behavior with qemu. I am looking at code that does transition from 32bit mode to 64bit mode. Right now it does 'jmp $SELECTOR,64bitfunction'. https://github.com/phil- opp/blog_os/blob/557c6a58ea11e31685b9d014d307002d64df5c22/src/arch/x86_64/boot.asm#L32 This code works fine with qemu/kvm/vmware. To transition from 32 to 64 bit code it is possible to use 'call' operation. So I am trying to replace that code with 'call $SELECTOR,64bitfunction'. It works fine with kvm and wmware. But it fails with Qemu emulation. See the interrup debug log attached. qemu crashes at 10302c (far call mnemonic). 103016: e8 18 00 00 00 callq 103033 10301b: e8 5c 00 00 00 callq 10307c 103020: e8 ec 00 00 00 callq 103111 103025: 0f 01 15 28 00 10 00lgdt 0x100028(%rip)# 203054 10302c: 9a (bad) 10302d: 40 31 10rex xor %edx,(%rax) 103030: 00 08 add%cl,(%rax) As the code works at hardware I expect it to work with qemu. Thus current qemu behavior looks like a bug. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1699867/+subscriptions
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On 22 June 2017 at 19:08, Thomas Huthwrote: > > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > >> Could do; I'm just not finding tiny header files with one or > >> two entries each that useful. > > Well, it means that the bulk of code that doesn't care about the > types doesn't get its compilation fractionally slowed by having > to parse the typedef anyway. In general I think we're drifting > towards "have each .c file get fewer things automatically" rather > than otherwise (eg more finely focused files rather than stuffing > everything into qemu-common.h). At the cost of things getting fractionally slower by including lots more tiny headers. I generally agree in the case where there's a useful chunk, but when it's down to one or two definitions I don't see the point. > > Do we really need these function typedefs at all? IMHO it's quite ugly > > to hide such things in a typedef unless it is really necessary (and in > > this case, it does not seem to be really necessary since it is only used > > in a few places). So what about simply removing the typedefs in this case? > > I find function typedefs much more readable than having the > function-types inline in function arguments and the like. > > This is all fairly rapidly heading into bikeshed territory > though -- in the final analysis I don't think it matters > much what we do. Agreed. Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v8 00/26] translate: [tcg] Generic translation framework
On Thu, Jun 22, 2017 at 21:06:34 +0300, Lluís Vilanova wrote: > Please ignore, I'm having problems with my mail server and there's patches > being > dropeed. Can you post a public "v7" branch we can pull from? Thanks, E.
[Qemu-devel] [Bug 1037606] Re: vmwgfx does not work with kvm vmware vga
Also http://airlied.livejournal.com/69291.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in Linux: Confirmed Status in QEMU: Won't Fix Status in linux package in Ubuntu: Triaged Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 "Quantal Quetzal" - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs --- ApportVersion: 2.0.1-0ubuntu12 Architecture: i386 DistroRelease: Ubuntu 12.04 InstallationMedia: Ubuntu 10.10 "Maverick Meerkat" - Release i386 (20101007) Package: linux (not installed) ProcEnviron: TERM=xterm PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash Tags: precise running-unity Uname: Linux 3.6.0-030600rc3-generic i686 UnreportableReason: The running kernel is not an Ubuntu kernel UpgradeStatus: Upgraded to precise on 2012-08-30 (0 days ago) UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare To manage notifications about this bug go to: https://bugs.launchpad.net/linux/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] Re: vmwgfx does not work with kvm vmware vga
The kernel bugzilla response is: The vmwgfx kernel module does not support devices without the pitchlock capability. Sorry. In that case you need to use the xf86-video-vmware driver standalone without kernel modesetting. So presumably this is some capability to be added to the qemu device -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in Linux: Confirmed Status in QEMU: Won't Fix Status in linux package in Ubuntu: Triaged Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 "Quantal Quetzal" - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs --- ApportVersion: 2.0.1-0ubuntu12 Architecture: i386 DistroRelease: Ubuntu 12.04 InstallationMedia: Ubuntu 10.10 "Maverick Meerkat" - Release i386 (20101007) Package: linux (not installed) ProcEnviron: TERM=xterm PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash Tags: precise running-unity Uname: Linux 3.6.0-030600rc3-generic i686 UnreportableReason: The running kernel is not an Ubuntu kernel UpgradeStatus: Upgraded to precise on 2012-08-30 (0 days ago) UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare To manage notifications about this bug go to: https://bugs.launchpad.net/linux/+bug/1037606/+subscriptions
[Qemu-devel] [PATCH v4] Add manpage for QEMU Backup Tool
qemu-backup will be a command-line tool for performing full and incremental disk backups on running VMs. It is intended as a reference implementation for management stack and backup developers to see QEMU's backup features in action. The following commit is an initial implementation of manpage listing the commands which the backup tool will support. Signed-off-by: Ishani Chugh--- Makefile| 2 +- contrib/backup/qemu-backup.texi | 132 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 contrib/backup/qemu-backup.texi diff --git a/Makefile b/Makefile index c830d7a..094f291 100644 --- a/Makefile +++ b/Makefile @@ -504,7 +504,7 @@ clean: VERSION ?= $(shell cat VERSION) dist: qemu-$(VERSION).tar.bz2 - +qemu-backup.8: contrib/backup/qemu-backup.texi qemu-%.tar.bz2: $(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst qemu-%.tar.bz2,%,$@)" diff --git a/contrib/backup/qemu-backup.texi b/contrib/backup/qemu-backup.texi new file mode 100644 index 000..cdfad82 --- /dev/null +++ b/contrib/backup/qemu-backup.texi @@ -0,0 +1,132 @@ +\input texinfo +@setfilename qemu-backup + +@documentlanguage en +@documentencoding UTF-8 + +@settitle QEMU Backup Tool +@copying + +Copyright @copyright{} 2017 The QEMU Project developers +@end copying +@ifinfo +@direntry +* QEMU: (QEMU-backup).Man page for QEMU Backup Tool. +@end direntry +@end ifinfo +@iftex +@titlepage +@sp 7 +@center @titlefont{QEMU Backup Tool} +@sp 1 +@sp 3 +@end titlepage +@end iftex +@ifnottex +@node Top +@top Short Sample + +@menu +* Name:: +* Synopsis:: +* list of Commands:: +* Command Parameters:: +* Command Descriptions:: +* License:: +@end menu + +@end ifnottex + +@node Name +@chapter Name + +QEMU disk backup tool. + +@node Synopsis +@chapter Synopsis + +qemu-backup command [ command options]. + +@node List of Commands +@chapter List of Commands +@itemize +@item qemu-backup guest add --guest guestname --qmp socketpath +@item qemu-backup guest list +@item qemu-backup drive add --id driveid --guest guestname --target target +@item qemu-backup drive add --all --guest guestname --target target +@item qemu-backup drive list --guest guestname +@item qemu-backup backup [--inc] --guest guestname +@item qemu-backup restore --guest guestname +@item qemu-backup drive remove --guest guestname --id driveid +@item qemu-backup guest remove --guest guestname +@end itemize +@node Command Parameters +@chapter Command Parameters +@itemize +@item --guest: Name of the guest. +@item --id: id of guest or drive. +@item --target: Destination path on which you want your backup to be made. +@item --all: Add all the drives present in a guest which are suitable for backup. +@item --inc: For incremental backup. +@item --qmp: Path of qmp socket. +@end itemize + +@node Command Descriptions +@chapter Command Descriptions +@itemize +@item qemu-backup guest add --guest guestname --qmp socketpath +This command adds a guest to the configuration file given its path to qmp socket. + +example: +qemu-backup guest add --id=fedora –qmp=/var/run/qemu/fedora.sock + +@item qemu-backup guest list +This commands lists the names of guests which are added to configuration file. + +@item qemu-backup drive add --guest guestname --id driveid --target target +This command adds different drives for backup in a particular guest by giving the name of drive to be backed up and target imagefile in which we want to store the drive backup. + +example:qemu-backup drive add --guest=fedora --id=root +--target=/backup/root.img + +@item qemu-backup drive add --all --guest guestname --destination destination +This command adds all the drives of the guest for backup other than CDROM drive. Here all the backup drives will have the same names as original drives and target will be the destination folder. + +example: qemu-backup drive add --all --guest fedora --destination =/backup/fedora/ + +@item qemu-backup drive list --guest guestname +This commands gives the names of the drive present in a guest which are added for backup. + +example: qemu-backup drive list --guest=fedora + +@item qemu-backup backup --guest guestname + +This command makes the backup of the drives, in their respective given destinations. The ids of drive and their destinations are taken from the configuration file. + +example: qemu-backup backup --guest=fedora + +@item qemu-backup restore --guest guestname +This command is needed if we want to restore the backup. It will list the commands to be run for performing the same but will not perform any action. + +example: qemu-backup restore --guest=fedora + +@item qemu-backup drive remove --guest guestname --id driveid +This command helps remove a drive which is set for backup in configuration of given host. + +example: drive remove --guest=fedora --id=root + +@item qemu-backup guest remove --guest guestname +This command removes the guest
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On 22 June 2017 at 19:08, Thomas Huthwrote: > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: >> Could do; I'm just not finding tiny header files with one or >> two entries each that useful. Well, it means that the bulk of code that doesn't care about the types doesn't get its compilation fractionally slowed by having to parse the typedef anyway. In general I think we're drifting towards "have each .c file get fewer things automatically" rather than otherwise (eg more finely focused files rather than stuffing everything into qemu-common.h). > Do we really need these function typedefs at all? IMHO it's quite ugly > to hide such things in a typedef unless it is really necessary (and in > this case, it does not seem to be really necessary since it is only used > in a few places). So what about simply removing the typedefs in this case? I find function typedefs much more readable than having the function-types inline in function arguments and the like. This is all fairly rapidly heading into bikeshed territory though -- in the final analysis I don't think it matters much what we do. thanks -- PMM
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > * Greg Kurz (gr...@kaod.org) wrote: >> On Thu, 22 Jun 2017 18:25:55 +0100 >> "Dr. David Alan Gilbert"wrote: >> >>> * Peter Maydell (peter.mayd...@linaro.org) wrote: On 22 June 2017 at 18:03, Juan Quintela wrote: > Greg Kurz wrote: >> On Thu, 22 Jun 2017 17:14:08 +0100 >> Peter Maydell wrote: >> >>> On 22 June 2017 at 17:06, Greg Kurz wrote: Function types cannot reside in the same sorted list as opaque types since they may depend on a type which would be defined later. Of course, the same problem could arise if a function type depends on another function type with greater alphabetical order. Hopefully we don't have that at this time. >>> >>> The other approach would be to put function types somewhere >>> else and leave typedefs.h for the simple 'opaque types >>> for structures' that it was started as. >>> >>> For instance we have include/qemu/fprintf-fn.h as a precedent. >>> >> >> Indeed, and I'm not quite sure why Juan decided to put these types into >> typedefs.h instead of a dedicated header file in include/migration... is >> it only because it was the quickest fix ? > > All other typedefs were defined there. I can create a different include > file, but I think that is "overengineering", no? They are typedefs, > just not of structs. But I agree that they are the only ones. Well, the comment in the file says "opaque types so that device init declarations don't have to pull in all the real definitions", whereas the ones you've added aren't opaque types, they are the real definitions. They're also only used by a very small subset of .c files, whereas typedefs.h goes everywhere. >>> >>> mv fprintf-fn.f fn-typedefs.h >>> >>> move those two defs into that? >>> >> >> Wouldn't it be more appropriate to put them in a dedicated >> include/migration/handler-fn.h header included by both >> vmstate.h and register.h ? > > Could do; I'm just not finding tiny header files with one or > two entries each that useful. Do we really need these function typedefs at all? IMHO it's quite ugly to hide such things in a typedef unless it is really necessary (and in this case, it does not seem to be really necessary since it is only used in a few places). So what about simply removing the typedefs in this case? Thomas
Re: [Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
Please ignore, I'm having problems with my mail server and there's patches being dropeed. Thanks, Lluis Lluís Vilanova writes: > This series proposes a generic (target-agnostic) instruction translation > framework. > It basically provides a generic main loop for instruction disassembly, which > calls target-specific functions when necessary. This generalization makes > inserting new code in the main loop easier, and helps in keeping all targets > in > synch as to the contents of it. > This series also paves the way towards adding events to trace guest code > execution (BBLs and instructions). > I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in > the > current organization, but will port the rest when this series gets merged. > Signed-off-by: Lluís Vilanova> --- > Changes in v7 > = > * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). > * Move target-specific translation functions to a struct (TranslatorOps). > * Split target-specific changes into multiple patches. > * Rebase on edf8bc9842. > Changes in v6 > = > * Rebase on upstream master (64175afc69). > * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. > Changes in v5 > = > * Remove stray uses of "restrict" keyword. > Changes in v4 > = > * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. > * Fix coding style errors reported by checkpatch. > * Remove use of "restrict" in added functions; it makes older gcc versions > barf > about compilation errors. > Changes in v3 > = > * Rebase on 0737f32daf. > Changes in v2 > = > * Port ARM and AARCH64 targets. > * Fold single-stepping checks into "max_insns" [Richard Henderson]. > * Move instruction start marks to target code [Richard Henderson]. > * Add target hook for TB start. > * Check for TCG temporary leaks. > * Move instruction disassembly into a target hook. > * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). > Lluís Vilanova (26): > Pass generic CPUState to gen_intermediate_code() > queue: Add macro for incremental traversal > cpu-exec: Avoid global variables in icount-related functions > target: [tcg] Add generic translation framework > target: [tcg] Redefine DISAS_* onto the generic translation framework > (DJ_*) > target: [tcg,i386] Port to DisasContextBase > target: [tcg,i386] Refactor init_disas_context > target: [tcg,i386] Refactor init_globals > target: [tcg,i386] Refactor insn_start > target: [tcg,i386] Refactor breakpoint_check > target: [tcg,i386] Refactor disas_insn > target: [tcg,i386] Refactor tb_stop > target: [tcg,i386] Refactor disas_flags > target: [tcg,i386] Replace DISAS_* with DJ_* > target: [tcg,i386] Port to generic translation framework > target: [tcg,arm] Replace DISAS_* with DJ_* > target: [tcg,arm] Port to DisasContextBase > target: [tcg,arm] Port to init_disas_context > target: [tcg,arm] Port to init_globals > target: [tcg,arm] Port to tb_start > target: [tcg,arm] Port to insn_start > target: [tcg,arm] Port to breakpoint_check > target: [tcg,arm] Port to disas_insn > target: [tcg,arm] Port to tb_stop > target: [tcg,arm] Port to disas_flags > target: [tcg,arm] Port to generic translation framework > Makefile.target|1 > include/exec/exec-all.h| 13 + > include/exec/gen-icount.h |8 - > include/exec/translate-block.h | 125 ++ > include/qemu/queue.h | 12 + > include/qom/cpu.h | 22 ++ > target/alpha/translate.c | 25 +- > target/arm/translate-a64.c | 312 - > target/arm/translate.c | 503 > ++-- > target/arm/translate.h | 38 ++- > target/cris/translate.c| 26 +- > target/hppa/translate.c|6 > target/i386/translate.c| 353 +++- > target/lm32/translate.c| 36 +-- > target/m68k/translate.c| 24 +- > target/microblaze/translate.c | 28 +- > target/mips/translate.c| 41 ++- > target/moxie/translate.c | 16 + > target/nios2/translate.c |6 > target/openrisc/translate.c| 25 +- > target/ppc/translate.c | 21 +- > target/ppc/translate_init.c| 32 +-- > target/s390x/translate.c | 22 +- > target/sh4/translate.c | 21 +- > target/sparc/translate.c | 17 + > target/tilegx/translate.c |9 - > target/tricore/translate.c | 11 - > target/unicore32/translate.c | 26 +- > target/xtensa/translate.c | 39 ++- > translate-all.c|2 > translate-block.c | 185 +++ > 31 files changed, 1212 insertions(+), 793 deletions(-) >
Re: [Qemu-devel] [PATCH v8 00/26] translate: [tcg] Generic translation framework
Please ignore, I'm having problems with my mail server and there's patches being dropeed. Thanks, Lluis Lluís Vilanova writes: > This series proposes a generic (target-agnostic) instruction translation > framework. > It basically provides a generic main loop for instruction disassembly, which > calls target-specific functions when necessary. This generalization makes > inserting new code in the main loop easier, and helps in keeping all targets > in > synch as to the contents of it. > This series also paves the way towards adding events to trace guest code > execution (BBLs and instructions). > I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in > the > current organization, but will port the rest when this series gets merged. > Signed-off-by: Lluís Vilanova> --- > Changes in v8 > = > * Increase inter-mail sleep time during sending (list keeps refusing some > emails > due to an excessive send rate). > Changes in v7 > = > * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). > * Move target-specific translation functions to a struct (TranslatorOps). > * Split target-specific changes into multiple patches. > * Rebase on edf8bc9842. > Changes in v6 > = > * Rebase on upstream master (64175afc69). > * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. > Changes in v5 > = > * Remove stray uses of "restrict" keyword. > Changes in v4 > = > * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. > * Fix coding style errors reported by checkpatch. > * Remove use of "restrict" in added functions; it makes older gcc versions > barf > about compilation errors. > Changes in v3 > = > * Rebase on 0737f32daf. > Changes in v2 > = > * Port ARM and AARCH64 targets. > * Fold single-stepping checks into "max_insns" [Richard Henderson]. > * Move instruction start marks to target code [Richard Henderson]. > * Add target hook for TB start. > * Check for TCG temporary leaks. > * Move instruction disassembly into a target hook. > * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). > Lluís Vilanova (26): > Pass generic CPUState to gen_intermediate_code() > queue: Add macro for incremental traversal > cpu-exec: Avoid global variables in icount-related functions > target: [tcg] Add generic translation framework > target: [tcg] Redefine DISAS_* onto the generic translation framework > (DJ_*) > target: [tcg,i386] Port to DisasContextBase > target: [tcg,i386] Refactor init_disas_context > target: [tcg,i386] Refactor init_globals > target: [tcg,i386] Refactor insn_start > target: [tcg,i386] Refactor breakpoint_check > target: [tcg,i386] Refactor disas_insn > target: [tcg,i386] Refactor tb_stop > target: [tcg,i386] Refactor disas_flags > target: [tcg,i386] Replace DISAS_* with DJ_* > target: [tcg,i386] Port to generic translation framework > target: [tcg,arm] Replace DISAS_* with DJ_* > target: [tcg,arm] Port to DisasContextBase > target: [tcg,arm] Port to init_disas_context > target: [tcg,arm] Port to init_globals > target: [tcg,arm] Port to tb_start > target: [tcg,arm] Port to insn_start > target: [tcg,arm] Port to breakpoint_check > target: [tcg,arm] Port to disas_insn > target: [tcg,arm] Port to tb_stop > target: [tcg,arm] Port to disas_flags > target: [tcg,arm] Port to generic translation framework > Makefile.target|1 > include/exec/exec-all.h| 13 + > include/exec/gen-icount.h |8 - > include/exec/translate-block.h | 125 ++ > include/qemu/queue.h | 12 + > include/qom/cpu.h | 22 ++ > target/alpha/translate.c | 25 +- > target/arm/translate-a64.c | 312 - > target/arm/translate.c | 503 > ++-- > target/arm/translate.h | 38 ++- > target/cris/translate.c| 26 +- > target/hppa/translate.c|6 > target/i386/translate.c| 353 +++- > target/lm32/translate.c| 36 +-- > target/m68k/translate.c| 24 +- > target/microblaze/translate.c | 28 +- > target/mips/translate.c| 41 ++- > target/moxie/translate.c | 16 + > target/nios2/translate.c |6 > target/openrisc/translate.c| 25 +- > target/ppc/translate.c | 21 +- > target/ppc/translate_init.c| 32 +-- > target/s390x/translate.c | 22 +- > target/sh4/translate.c | 21 +- > target/sparc/translate.c | 17 + > target/tilegx/translate.c |9 - > target/tricore/translate.c | 11 - > target/unicore32/translate.c | 26 +- > target/xtensa/translate.c | 39 ++- >
Re: [Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
Please ignore, I'm having problems with my mail server and there's patches being dropeed. Thanks, Lluis Lluís Vilanova writes: > This series proposes a generic (target-agnostic) instruction translation > framework. > It basically provides a generic main loop for instruction disassembly, which > calls target-specific functions when necessary. This generalization makes > inserting new code in the main loop easier, and helps in keeping all targets > in > synch as to the contents of it. > This series also paves the way towards adding events to trace guest code > execution (BBLs and instructions). > I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in > the > current organization, but will port the rest when this series gets merged. > Signed-off-by: Lluís Vilanova> --- > Changes in v7 > = > * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). > * Move target-specific translation functions to a struct (TranslatorOps). > * Split target-specific changes into multiple patches. > * Rebase on edf8bc9842. > Changes in v6 > = > * Rebase on upstream master (64175afc69). > * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. > Changes in v5 > = > * Remove stray uses of "restrict" keyword. > Changes in v4 > = > * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. > * Fix coding style errors reported by checkpatch. > * Remove use of "restrict" in added functions; it makes older gcc versions > barf > about compilation errors. > Changes in v3 > = > * Rebase on 0737f32daf. > Changes in v2 > = > * Port ARM and AARCH64 targets. > * Fold single-stepping checks into "max_insns" [Richard Henderson]. > * Move instruction start marks to target code [Richard Henderson]. > * Add target hook for TB start. > * Check for TCG temporary leaks. > * Move instruction disassembly into a target hook. > * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). > Lluís Vilanova (26): > Pass generic CPUState to gen_intermediate_code() > queue: Add macro for incremental traversal > cpu-exec: Avoid global variables in icount-related functions > target: [tcg] Add generic translation framework > target: [tcg] Redefine DISAS_* onto the generic translation framework > (DJ_*) > target: [tcg,i386] Port to DisasContextBase > target: [tcg,i386] Refactor init_disas_context > target: [tcg,i386] Refactor init_globals > target: [tcg,i386] Refactor insn_start > target: [tcg,i386] Refactor breakpoint_check > target: [tcg,i386] Refactor disas_insn > target: [tcg,i386] Refactor tb_stop > target: [tcg,i386] Refactor disas_flags > target: [tcg,i386] Replace DISAS_* with DJ_* > target: [tcg,i386] Port to generic translation framework > target: [tcg,arm] Replace DISAS_* with DJ_* > target: [tcg,arm] Port to DisasContextBase > target: [tcg,arm] Port to init_disas_context > target: [tcg,arm] Port to init_globals > target: [tcg,arm] Port to tb_start > target: [tcg,arm] Port to insn_start > target: [tcg,arm] Port to breakpoint_check > target: [tcg,arm] Port to disas_insn > target: [tcg,arm] Port to tb_stop > target: [tcg,arm] Port to disas_flags > target: [tcg,arm] Port to generic translation framework > Makefile.target|1 > include/exec/exec-all.h| 13 + > include/exec/gen-icount.h |8 - > include/exec/translate-block.h | 125 ++ > include/qemu/queue.h | 12 + > include/qom/cpu.h | 22 ++ > target/alpha/translate.c | 25 +- > target/arm/translate-a64.c | 312 - > target/arm/translate.c | 503 > ++-- > target/arm/translate.h | 38 ++- > target/cris/translate.c| 26 +- > target/hppa/translate.c|6 > target/i386/translate.c| 353 +++- > target/lm32/translate.c| 36 +-- > target/m68k/translate.c| 24 +- > target/microblaze/translate.c | 28 +- > target/mips/translate.c| 41 ++- > target/moxie/translate.c | 16 + > target/nios2/translate.c |6 > target/openrisc/translate.c| 25 +- > target/ppc/translate.c | 21 +- > target/ppc/translate_init.c| 32 +-- > target/s390x/translate.c | 22 +- > target/sh4/translate.c | 21 +- > target/sparc/translate.c | 17 + > target/tilegx/translate.c |9 - > target/tricore/translate.c | 11 - > target/unicore32/translate.c | 26 +- > target/xtensa/translate.c | 39 ++- > translate-all.c|2 > translate-block.c | 185 +++ > 31 files changed, 1212 insertions(+), 793 deletions(-) >
Re: [Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
Please ignore, I'm having problems with my mail server and there's patches being dropeed. Thanks, Lluis Lluís Vilanova writes: > This series proposes a generic (target-agnostic) instruction translation > framework. > It basically provides a generic main loop for instruction disassembly, which > calls target-specific functions when necessary. This generalization makes > inserting new code in the main loop easier, and helps in keeping all targets > in > synch as to the contents of it. > This series also paves the way towards adding events to trace guest code > execution (BBLs and instructions). > I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in > the > current organization, but will port the rest when this series gets merged. > Signed-off-by: Lluís Vilanova> --- > Changes in v7 > = > * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). > * Move target-specific translation functions to a struct (TranslatorOps). > * Split target-specific changes into multiple patches. > * Rebase on edf8bc9842. > Changes in v6 > = > * Rebase on upstream master (64175afc69). > * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. > Changes in v5 > = > * Remove stray uses of "restrict" keyword. > Changes in v4 > = > * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. > * Fix coding style errors reported by checkpatch. > * Remove use of "restrict" in added functions; it makes older gcc versions > barf > about compilation errors. > Changes in v3 > = > * Rebase on 0737f32daf. > Changes in v2 > = > * Port ARM and AARCH64 targets. > * Fold single-stepping checks into "max_insns" [Richard Henderson]. > * Move instruction start marks to target code [Richard Henderson]. > * Add target hook for TB start. > * Check for TCG temporary leaks. > * Move instruction disassembly into a target hook. > * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). > Lluís Vilanova (26): > Pass generic CPUState to gen_intermediate_code() > queue: Add macro for incremental traversal > cpu-exec: Avoid global variables in icount-related functions > target: [tcg] Add generic translation framework > target: [tcg] Redefine DISAS_* onto the generic translation framework > (DJ_*) > target: [tcg,i386] Port to DisasContextBase > target: [tcg,i386] Refactor init_disas_context > target: [tcg,i386] Refactor init_globals > target: [tcg,i386] Refactor insn_start > target: [tcg,i386] Refactor breakpoint_check > target: [tcg,i386] Refactor disas_insn > target: [tcg,i386] Refactor tb_stop > target: [tcg,i386] Refactor disas_flags > target: [tcg,i386] Replace DISAS_* with DJ_* > target: [tcg,i386] Port to generic translation framework > target: [tcg,arm] Replace DISAS_* with DJ_* > target: [tcg,arm] Port to DisasContextBase > target: [tcg,arm] Port to init_disas_context > target: [tcg,arm] Port to init_globals > target: [tcg,arm] Port to tb_start > target: [tcg,arm] Port to insn_start > target: [tcg,arm] Port to breakpoint_check > target: [tcg,arm] Port to disas_insn > target: [tcg,arm] Port to tb_stop > target: [tcg,arm] Port to disas_flags > target: [tcg,arm] Port to generic translation framework > Makefile.target|1 > include/exec/exec-all.h| 13 + > include/exec/gen-icount.h |8 - > include/exec/translate-block.h | 125 ++ > include/qemu/queue.h | 12 + > include/qom/cpu.h | 22 ++ > target/alpha/translate.c | 25 +- > target/arm/translate-a64.c | 312 - > target/arm/translate.c | 503 > ++-- > target/arm/translate.h | 38 ++- > target/cris/translate.c| 26 +- > target/hppa/translate.c|6 > target/i386/translate.c| 353 +++- > target/lm32/translate.c| 36 +-- > target/m68k/translate.c| 24 +- > target/microblaze/translate.c | 28 +- > target/mips/translate.c| 41 ++- > target/moxie/translate.c | 16 + > target/nios2/translate.c |6 > target/openrisc/translate.c| 25 +- > target/ppc/translate.c | 21 +- > target/ppc/translate_init.c| 32 +-- > target/s390x/translate.c | 22 +- > target/sh4/translate.c | 21 +- > target/sparc/translate.c | 17 + > target/tilegx/translate.c |9 - > target/tricore/translate.c | 11 - > target/unicore32/translate.c | 26 +- > target/xtensa/translate.c | 39 ++- > translate-all.c|2 > translate-block.c | 185 +++ > 31 files changed, 1212 insertions(+), 793 deletions(-) > create
Re: [Qemu-devel] [PULL 0/1] Usb 20170621 patches
On 21 June 2017 at 16:43, Gerd Hoffmannwrote: > The following changes since commit 8dfaf23ae1f2273a9730a9b309cc8471269bb524: > > tcg/tci: fix tcg-interpreter build (2017-06-20 18:39:15 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/usb-20170621-pull-request > > for you to fetch changes up to 896b6757f9c4d176ce4439238efea223a2952411: > > usb-host: support devices with sparse/non-sequential USB interfaces > (2017-06-21 15:30:08 +0200) > > > > > > Samuel Brian (1): > usb-host: support devices with sparse/non-sequential USB interfaces > > hw/usb/host-libusb.c | 24 ++-- > 1 file changed, 14 insertions(+), 10 deletions(-) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
* Greg Kurz (gr...@kaod.org) wrote: > On Thu, 22 Jun 2017 18:25:55 +0100 > "Dr. David Alan Gilbert"wrote: > > > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > > On 22 June 2017 at 18:03, Juan Quintela wrote: > > > > Greg Kurz wrote: > > > >> On Thu, 22 Jun 2017 17:14:08 +0100 > > > >> Peter Maydell wrote: > > > >> > > > >>> On 22 June 2017 at 17:06, Greg Kurz wrote: > > > >>> > Function types cannot reside in the same sorted list as opaque > > > >>> > types since > > > >>> > they may depend on a type which would be defined later. > > > >>> > > > > >>> > Of course, the same problem could arise if a function type depends > > > >>> > on > > > >>> > another function type with greater alphabetical order. Hopefully we > > > >>> > don't have that at this time. > > > >>> > > > >>> The other approach would be to put function types somewhere > > > >>> else and leave typedefs.h for the simple 'opaque types > > > >>> for structures' that it was started as. > > > >>> > > > >>> For instance we have include/qemu/fprintf-fn.h as a precedent. > > > >>> > > > >> > > > >> Indeed, and I'm not quite sure why Juan decided to put these types into > > > >> typedefs.h instead of a dedicated header file in include/migration... > > > >> is > > > >> it only because it was the quickest fix ? > > > > > > > > All other typedefs were defined there. I can create a different include > > > > file, but I think that is "overengineering", no? They are typedefs, > > > > just not of structs. But I agree that they are the only ones. > > > > > > Well, the comment in the file says "opaque types so that device init > > > declarations don't have to pull in all the real definitions", whereas > > > the ones you've added aren't opaque types, they are the real > > > definitions. They're also only used by a very small subset of .c > > > files, whereas typedefs.h goes everywhere. > > > > mv fprintf-fn.f fn-typedefs.h > > > > move those two defs into that? > > > > Wouldn't it be more appropriate to put them in a dedicated > include/migration/handler-fn.h header included by both > vmstate.h and register.h ? Could do; I'm just not finding tiny header files with one or two entries each that useful. Dave > > Dave > > > > > thanks > > > -- PMM > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH v8 02/26] queue: Add macro for incremental traversal
Adds macro QTAILQ_FOREACH_CONTINUE to support incremental list traversal. Signed-off-by: Lluís Vilanova--- include/qemu/queue.h | 12 1 file changed, 12 insertions(+) diff --git a/include/qemu/queue.h b/include/qemu/queue.h index 35292c3155..eb2bf9cb1c 100644 --- a/include/qemu/queue.h +++ b/include/qemu/queue.h @@ -415,6 +415,18 @@ struct { \ (var); \ (var) = ((var)->field.tqe_next)) +/** + * QTAILQ_FOREACH_CONTINUE: + * @var: Variable to resume iteration from. + * @field: Field in @var holding a QTAILQ_ENTRY for this queue. + * + * Resumes iteration on a queue from the element in @var. + */ +#define QTAILQ_FOREACH_CONTINUE(var, field) \ +for ((var) = ((var)->field.tqe_next); \ +(var); \ +(var) = ((var)->field.tqe_next)) + #define QTAILQ_FOREACH_SAFE(var, head, field, next_var) \ for ((var) = ((head)->tqh_first); \ (var) && ((next_var) = ((var)->field.tqe_next), 1); \
[Qemu-devel] [PATCH v7 12/26] target: [tcg,i386] Refactor tb_stop
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 34e7cf6788..e7b46d282a 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -8510,8 +8510,6 @@ static target_ulong i386_trblock_disas_insn(DisasContextBase *db, CPUState *cpu) /* if irq were inhibited with HF_INHIBIT_IRQ_MASK, we clear the flag and abort the translation to give the irqs a change to be happen */ -gen_jmp_im(pc_next - dc->cs_base); -gen_eob(dc); db->is_jmp = DJ_TOO_MANY; } else if ((db->tb->cflags & CF_USE_ICOUNT) && ((db->pc_next & TARGET_PAGE_MASK) @@ -8524,18 +8522,24 @@ static target_ulong i386_trblock_disas_insn(DisasContextBase *db, CPUState *cpu) If current instruction already crossed the bound - it's ok, because an exception hasn't stopped this code. */ -gen_jmp_im(pc_next - dc->cs_base); -gen_eob(dc); db->is_jmp = DJ_TOO_MANY; } else if ((pc_next - db->pc_first) >= (TARGET_PAGE_SIZE - 32)) { -gen_jmp_im(pc_next - dc->cs_base); -gen_eob(dc); db->is_jmp = DJ_TOO_MANY; } return pc_next; } +static void i386_trblock_tb_stop(DisasContextBase *db, CPUState *cpu) +{ +DisasContext *dc = container_of(db, DisasContext, base); + +if (db->is_jmp == DJ_TOO_MANY) { +gen_jmp_im(db->pc_next - dc->cs_base); +gen_eob(dc); +} +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { @@ -8596,23 +8600,21 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) /* if single step mode, we generate only one instruction and generate an exception */ if (db->singlestep_enabled) { -gen_jmp_im(db->pc_next - dc->cs_base); -gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; break; } /* if too long translation, stop generation too */ if (tcg_op_buf_full() || num_insns >= max_insns) { -gen_jmp_im(db->pc_next - dc->cs_base); -gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; break; } if (singlestep) { -gen_jmp_im(db->pc_next - dc->cs_base); -gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; break; } } +i386_trblock_tb_stop(db, cpu); if (tb->cflags & CF_LAST_IO) gen_io_end(cpu_env); done_generating:
[Qemu-devel] [PATCH v8 00/26] translate: [tcg] Generic translation framework
This series proposes a generic (target-agnostic) instruction translation framework. It basically provides a generic main loop for instruction disassembly, which calls target-specific functions when necessary. This generalization makes inserting new code in the main loop easier, and helps in keeping all targets in synch as to the contents of it. This series also paves the way towards adding events to trace guest code execution (BBLs and instructions). I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in the current organization, but will port the rest when this series gets merged. Signed-off-by: Lluís Vilanova--- Changes in v8 = * Increase inter-mail sleep time during sending (list keeps refusing some emails due to an excessive send rate). Changes in v7 = * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). * Move target-specific translation functions to a struct (TranslatorOps). * Split target-specific changes into multiple patches. * Rebase on edf8bc9842. Changes in v6 = * Rebase on upstream master (64175afc69). * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. Changes in v5 = * Remove stray uses of "restrict" keyword. Changes in v4 = * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. * Fix coding style errors reported by checkpatch. * Remove use of "restrict" in added functions; it makes older gcc versions barf about compilation errors. Changes in v3 = * Rebase on 0737f32daf. Changes in v2 = * Port ARM and AARCH64 targets. * Fold single-stepping checks into "max_insns" [Richard Henderson]. * Move instruction start marks to target code [Richard Henderson]. * Add target hook for TB start. * Check for TCG temporary leaks. * Move instruction disassembly into a target hook. * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). Lluís Vilanova (26): Pass generic CPUState to gen_intermediate_code() queue: Add macro for incremental traversal cpu-exec: Avoid global variables in icount-related functions target: [tcg] Add generic translation framework target: [tcg] Redefine DISAS_* onto the generic translation framework (DJ_*) target: [tcg,i386] Port to DisasContextBase target: [tcg,i386] Refactor init_disas_context target: [tcg,i386] Refactor init_globals target: [tcg,i386] Refactor insn_start target: [tcg,i386] Refactor breakpoint_check target: [tcg,i386] Refactor disas_insn target: [tcg,i386] Refactor tb_stop target: [tcg,i386] Refactor disas_flags target: [tcg,i386] Replace DISAS_* with DJ_* target: [tcg,i386] Port to generic translation framework target: [tcg,arm] Replace DISAS_* with DJ_* target: [tcg,arm] Port to DisasContextBase target: [tcg,arm] Port to init_disas_context target: [tcg,arm] Port to init_globals target: [tcg,arm] Port to tb_start target: [tcg,arm] Port to insn_start target: [tcg,arm] Port to breakpoint_check target: [tcg,arm] Port to disas_insn target: [tcg,arm] Port to tb_stop target: [tcg,arm] Port to disas_flags target: [tcg,arm] Port to generic translation framework Makefile.target|1 include/exec/exec-all.h| 13 + include/exec/gen-icount.h |8 - include/exec/translate-block.h | 125 ++ include/qemu/queue.h | 12 + include/qom/cpu.h | 22 ++ target/alpha/translate.c | 25 +- target/arm/translate-a64.c | 312 - target/arm/translate.c | 503 ++-- target/arm/translate.h | 38 ++- target/cris/translate.c| 26 +- target/hppa/translate.c|6 target/i386/translate.c| 353 +++- target/lm32/translate.c| 36 +-- target/m68k/translate.c| 24 +- target/microblaze/translate.c | 28 +- target/mips/translate.c| 41 ++- target/moxie/translate.c | 16 + target/nios2/translate.c |6 target/openrisc/translate.c| 25 +- target/ppc/translate.c | 21 +- target/ppc/translate_init.c| 32 +-- target/s390x/translate.c | 22 +- target/sh4/translate.c | 21 +- target/sparc/translate.c | 17 + target/tilegx/translate.c |9 - target/tricore/translate.c | 11 - target/unicore32/translate.c | 26 +- target/xtensa/translate.c | 39 ++- translate-all.c|2 translate-block.c | 185 +++ 31 files changed, 1212 insertions(+), 793 deletions(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c To: qemu-devel@nongnu.org Cc: Paolo Bonzini Cc: Peter Crosthwaite Cc:
[Qemu-devel] [PATCH v7 10/26] target: [tcg, i386] Refactor breakpoint_check
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 48 +++ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 3c7ef4af67..04d65b8416 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "qemu/host-utils.h" #include "cpu.h" #include "disas/disas.h" @@ -8474,12 +8475,32 @@ static void i386_trblock_insn_start(DisasContextBase *db, CPUState *cpu) tcg_gen_insn_start(db->pc_next, dc->cc_op); } +static BreakpointCheckType i386_trblock_breakpoint_check( +DisasContextBase *db, CPUState *cpu, const CPUBreakpoint *bp) +{ +DisasContext *dc = container_of(db, DisasContext, base); +/* If RF is set, suppress an internally generated breakpoint. */ +int flags = db->tb->flags & HF_RF_MASK ? BP_GDB : BP_ANY; +if (bp->flags & flags) { +gen_debug(dc, db->pc_next - dc->cs_base); +/* The address covered by the breakpoint must be included in + [tb->pc, tb->pc + tb->size) in order to for it to be + properly cleared -- thus we increment the PC here so that + the logic setting tb->size below does the right thing. */ +db->pc_next += 1; +return BC_HIT_TB; +} else { +return BC_MISS; +} +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { CPUX86State *env = cpu->env_ptr; DisasContext dc1, *dc = DisasContextBase *db = +CPUBreakpoint *bp; int num_insns; int max_insns; @@ -8507,18 +8528,21 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) i386_trblock_insn_start(db, cpu); num_insns++; -/* If RF is set, suppress an internally generated breakpoint. */ -if (unlikely(cpu_breakpoint_test(cpu, db->pc_next, - tb->flags & HF_RF_MASK - ? BP_GDB : BP_ANY))) { -gen_debug(dc, db->pc_next - dc->cs_base); -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -db->pc_next += 1; -goto done_generating; -} +bp = NULL; +do { +bp = cpu_breakpoint_get(cpu, db->pc_next, bp); +if (unlikely(bp)) { +BreakpointCheckType bp_check = i386_trblock_breakpoint_check( +db, cpu, bp); +if (bp_check == BC_HIT_TB) { +goto done_generating; +} else { +error_report("Unexpected BreakpointCheckType %d", bp_check); +abort(); +} +} +} while (bp != NULL); + if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { gen_io_start(cpu_env); }
[Qemu-devel] [PATCH v7 11/26] target: [tcg, i386] Refactor disas_insn
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 72 +++ 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 04d65b8416..34e7cf6788 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -4436,16 +4436,17 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, /* convert one instruction. s->base.is_jmp is set if the translation must be stopped. Return the next pc value */ -static target_ulong disas_insn(CPUX86State *env, DisasContext *s, - target_ulong pc_start) +static target_ulong disas_insn(DisasContextBase *db, CPUState *cpu) { -DisasContextBase *db = >base; +DisasContext *s = container_of(db, DisasContext, base); +CPUX86State *env = cpu->env_ptr; int b, prefixes; int shift; TCGMemOp ot, aflag, dflag; int modrm, reg, rm, mod, op, opreg, val; target_ulong next_eip, tval; int rex_w, rex_r; +target_ulong pc_start = db->pc_next; s->pc_start = s->pc = pc_start; prefixes = 0; @@ -8494,10 +8495,50 @@ static BreakpointCheckType i386_trblock_breakpoint_check( } } +static target_ulong i386_trblock_disas_insn(DisasContextBase *db, CPUState *cpu) +{ +DisasContext *dc = container_of(db, DisasContext, base); +target_ulong pc_next = disas_insn(db, cpu); + +if (db->is_jmp) { +return pc_next; +} + +if (dc->tf || (db->tb->flags & HF_INHIBIT_IRQ_MASK)) { +/* if single step mode, we generate only one instruction and + generate an exception */ +/* if irq were inhibited with HF_INHIBIT_IRQ_MASK, we clear + the flag and abort the translation to give the irqs a + change to be happen */ +gen_jmp_im(pc_next - dc->cs_base); +gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; +} else if ((db->tb->cflags & CF_USE_ICOUNT) + && ((db->pc_next & TARGET_PAGE_MASK) + != ((db->pc_next + TARGET_MAX_INSN_SIZE - 1) + & TARGET_PAGE_MASK) + || (db->pc_next & ~TARGET_PAGE_MASK) == 0)) { +/* Do not cross the boundary of the pages in icount mode, + it can cause an exception. Do it only when boundary is + crossed by the first instruction in the block. + If current instruction already crossed the bound - it's ok, + because an exception hasn't stopped this code. + */ +gen_jmp_im(pc_next - dc->cs_base); +gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; +} else if ((pc_next - db->pc_first) >= (TARGET_PAGE_SIZE - 32)) { +gen_jmp_im(pc_next - dc->cs_base); +gen_eob(dc); +db->is_jmp = DJ_TOO_MANY; +} + +return pc_next; +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { -CPUX86State *env = cpu->env_ptr; DisasContext dc1, *dc = DisasContextBase *db = CPUBreakpoint *bp; @@ -8547,39 +8588,20 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) gen_io_start(cpu_env); } -db->pc_next = disas_insn(env, dc, db->pc_next); +db->pc_next = i386_trblock_disas_insn(db, cpu); /* stop translation if indicated */ if (db->is_jmp) { break; } /* if single step mode, we generate only one instruction and generate an exception */ -/* if irq were inhibited with HF_INHIBIT_IRQ_MASK, we clear - the flag and abort the translation to give the irqs a - change to be happen */ -if (dc->tf || db->singlestep_enabled || -(db->tb->flags & HF_INHIBIT_IRQ_MASK)) { -gen_jmp_im(db->pc_next - dc->cs_base); -gen_eob(dc); -break; -} -/* Do not cross the boundary of the pages in icount mode, - it can cause an exception. Do it only when boundary is - crossed by the first instruction in the block. - If current instruction already crossed the bound - it's ok, - because an exception hasn't stopped this code. - */ -if ((tb->cflags & CF_USE_ICOUNT) -&& ((db->pc_next & TARGET_PAGE_MASK) -!= ((db->pc_next + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK) -|| (db->pc_next & ~TARGET_PAGE_MASK) == 0)) { +if (db->singlestep_enabled) { gen_jmp_im(db->pc_next - dc->cs_base); gen_eob(dc); break; } /* if too long translation, stop generation too */ if (tcg_op_buf_full() || -(db->pc_next - db->pc_first) >= (TARGET_PAGE_SIZE - 32) || num_insns >= max_insns) {
[Qemu-devel] [PATCH v7 09/26] target: [tcg, i386] Refactor insn_start
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c |8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index f0d12a3d13..3c7ef4af67 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -8468,6 +8468,12 @@ static void i386_trblock_init_globals(DisasContextBase *db, CPUState *cpu) cpu_cc_srcT = tcg_temp_local_new(); } +static void i386_trblock_insn_start(DisasContextBase *db, CPUState *cpu) +{ +DisasContext *dc = container_of(db, DisasContext, base); +tcg_gen_insn_start(db->pc_next, dc->cc_op); +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { @@ -8498,7 +8504,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) gen_tb_start(tb, cpu_env); for(;;) { -tcg_gen_insn_start(db->pc_next, dc->cc_op); +i386_trblock_insn_start(db, cpu); num_insns++; /* If RF is set, suppress an internally generated breakpoint. */
[Qemu-devel] [PATCH v7 08/26] target: [tcg, i386] Refactor init_globals
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 84ff49030b..f0d12a3d13 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -8452,6 +8452,22 @@ static void i386_trblock_init_disas_context(DisasContextBase *db, CPUState *cpu) #endif } +static void i386_trblock_init_globals(DisasContextBase *db, CPUState *cpu) +{ +cpu_T0 = tcg_temp_new(); +cpu_T1 = tcg_temp_new(); +cpu_A0 = tcg_temp_new(); + +cpu_tmp0 = tcg_temp_new(); +cpu_tmp1_i64 = tcg_temp_new_i64(); +cpu_tmp2_i32 = tcg_temp_new_i32(); +cpu_tmp3_i32 = tcg_temp_new_i32(); +cpu_tmp4 = tcg_temp_new(); +cpu_ptr0 = tcg_temp_new_ptr(); +cpu_ptr1 = tcg_temp_new_ptr(); +cpu_cc_srcT = tcg_temp_local_new(); +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { @@ -8469,18 +8485,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) db->pc_next = db->pc_first; i386_trblock_init_disas_context(db, cpu); -cpu_T0 = tcg_temp_new(); -cpu_T1 = tcg_temp_new(); -cpu_A0 = tcg_temp_new(); - -cpu_tmp0 = tcg_temp_new(); -cpu_tmp1_i64 = tcg_temp_new_i64(); -cpu_tmp2_i32 = tcg_temp_new_i32(); -cpu_tmp3_i32 = tcg_temp_new_i32(); -cpu_tmp4 = tcg_temp_new(); -cpu_ptr0 = tcg_temp_new_ptr(); -cpu_ptr1 = tcg_temp_new_ptr(); -cpu_cc_srcT = tcg_temp_local_new(); +i386_trblock_init_globals(db, cpu); num_insns = 0; max_insns = tb->cflags & CF_COUNT_MASK;
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On Thu, 22 Jun 2017 18:25:55 +0100 "Dr. David Alan Gilbert"wrote: > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > On 22 June 2017 at 18:03, Juan Quintela wrote: > > > Greg Kurz wrote: > > >> On Thu, 22 Jun 2017 17:14:08 +0100 > > >> Peter Maydell wrote: > > >> > > >>> On 22 June 2017 at 17:06, Greg Kurz wrote: > > >>> > Function types cannot reside in the same sorted list as opaque types > > >>> > since > > >>> > they may depend on a type which would be defined later. > > >>> > > > >>> > Of course, the same problem could arise if a function type depends on > > >>> > another function type with greater alphabetical order. Hopefully we > > >>> > don't have that at this time. > > >>> > > >>> The other approach would be to put function types somewhere > > >>> else and leave typedefs.h for the simple 'opaque types > > >>> for structures' that it was started as. > > >>> > > >>> For instance we have include/qemu/fprintf-fn.h as a precedent. > > >>> > > >> > > >> Indeed, and I'm not quite sure why Juan decided to put these types into > > >> typedefs.h instead of a dedicated header file in include/migration... is > > >> it only because it was the quickest fix ? > > > > > > All other typedefs were defined there. I can create a different include > > > file, but I think that is "overengineering", no? They are typedefs, > > > just not of structs. But I agree that they are the only ones. > > > > Well, the comment in the file says "opaque types so that device init > > declarations don't have to pull in all the real definitions", whereas > > the ones you've added aren't opaque types, they are the real > > definitions. They're also only used by a very small subset of .c > > files, whereas typedefs.h goes everywhere. > > mv fprintf-fn.f fn-typedefs.h > > move those two defs into that? > Wouldn't it be more appropriate to put them in a dedicated include/migration/handler-fn.h header included by both vmstate.h and register.h ? > Dave > > > thanks > > -- PMM > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > pgpGuZ6GHJqtm.pgp Description: OpenPGP digital signature
[Qemu-devel] [PATCH v7 07/26] target: [tcg, i386] Refactor init_disas_context
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 43 --- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 5a801766e5..84ff49030b 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -8396,21 +8396,12 @@ void tcg_x86_init(void) } } -/* generate intermediate code for basic block 'tb'. */ -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) +static void i386_trblock_init_disas_context(DisasContextBase *db, CPUState *cpu) { +DisasContext *dc = container_of(db, DisasContext, base); CPUX86State *env = cpu->env_ptr; -DisasContext dc1, *dc = -DisasContextBase *db = -uint32_t flags; -target_ulong cs_base; -int num_insns; -int max_insns; - -/* generate intermediate code */ -db->pc_first = tb->pc; -cs_base = tb->cs_base; -flags = tb->flags; +uint32_t flags = db->tb->flags; +target_ulong cs_base = db->tb->cs_base; dc->pe = (flags >> HF_PE_SHIFT) & 1; dc->code32 = (flags >> HF_CS32_SHIFT) & 1; @@ -8421,11 +8412,9 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) dc->cpl = (flags >> HF_CPL_SHIFT) & 3; dc->iopl = (flags >> IOPL_SHIFT) & 3; dc->tf = (flags >> TF_SHIFT) & 1; -db->singlestep_enabled = cpu->singlestep_enabled; dc->cc_op = CC_OP_DYNAMIC; dc->cc_op_dirty = false; dc->cs_base = cs_base; -db->tb = tb; dc->popl_esp_hack = 0; /* select memory access functions */ dc->mem_index = 0; @@ -8455,12 +8444,30 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) record/replay modes and there will always be an additional step for ecx=0 when icount is enabled. */ -dc->repz_opt = !dc->jmp_opt && !(tb->cflags & CF_USE_ICOUNT); +dc->repz_opt = !dc->jmp_opt && !(db->tb->cflags & CF_USE_ICOUNT); #if 0 /* check addseg logic */ if (!dc->addseg && (dc->vm86 || !dc->pe || !dc->code32)) printf("ERROR addseg\n"); #endif +} + +/* generate intermediate code for basic block 'tb'. */ +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) +{ +CPUX86State *env = cpu->env_ptr; +DisasContext dc1, *dc = +DisasContextBase *db = +int num_insns; +int max_insns; + +/* generate intermediate code */ +db->singlestep_enabled = cpu->singlestep_enabled; +db->tb = tb; +db->is_jmp = DISAS_NEXT; +db->pc_first = tb->pc; +db->pc_next = db->pc_first; +i386_trblock_init_disas_context(db, cpu); cpu_T0 = tcg_temp_new(); cpu_T1 = tcg_temp_new(); @@ -8475,8 +8482,6 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) cpu_ptr1 = tcg_temp_new_ptr(); cpu_cc_srcT = tcg_temp_local_new(); -db->is_jmp = DISAS_NEXT; -db->pc_next = db->pc_first; num_insns = 0; max_insns = tb->cflags & CF_COUNT_MASK; if (max_insns == 0) { @@ -8518,7 +8523,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) the flag and abort the translation to give the irqs a change to be happen */ if (dc->tf || db->singlestep_enabled || -(flags & HF_INHIBIT_IRQ_MASK)) { +(db->tb->flags & HF_INHIBIT_IRQ_MASK)) { gen_jmp_im(db->pc_next - dc->cs_base); gen_eob(dc); break;
[Qemu-devel] [PATCH v7 03/26] cpu-exec: Avoid global variables in icount-related functions
Signed-off-by: Lluís Vilanova--- include/exec/gen-icount.h |6 +++-- target/alpha/translate.c | 14 ++-- target/arm/translate-a64.c| 10 - target/arm/translate.c| 10 - target/cris/translate.c |6 +++-- target/hppa/translate.c |6 +++-- target/i386/translate.c | 46 + target/lm32/translate.c | 14 ++-- target/m68k/translate.c |6 +++-- target/microblaze/translate.c |6 +++-- target/mips/translate.c | 26 --- target/moxie/translate.c |2 +- target/nios2/translate.c |6 +++-- target/openrisc/translate.c |6 +++-- target/ppc/translate.c|6 +++-- target/ppc/translate_init.c | 32 ++--- target/s390x/translate.c |6 +++-- target/sh4/translate.c|6 +++-- target/sparc/translate.c |6 +++-- target/tilegx/translate.c |2 +- target/tricore/translate.c|2 +- target/unicore32/translate.c |6 +++-- target/xtensa/translate.c | 26 --- 23 files changed, 128 insertions(+), 128 deletions(-) diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 62d462e494..9b26c7da5f 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -8,7 +8,7 @@ static int icount_start_insn_idx; static TCGLabel *exitreq_label; -static inline void gen_tb_start(TranslationBlock *tb) +static inline void gen_tb_start(TranslationBlock *tb, TCGv_env cpu_env) { TCGv_i32 count, imm; @@ -59,14 +59,14 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns) tcg_ctx.gen_op_buf[tcg_ctx.gen_op_buf[0].prev].next = 0; } -static inline void gen_io_start(void) +static inline void gen_io_start(TCGv_env cpu_env) { TCGv_i32 tmp = tcg_const_i32(1); tcg_gen_st_i32(tmp, cpu_env, -ENV_OFFSET + offsetof(CPUState, can_do_io)); tcg_temp_free_i32(tmp); } -static inline void gen_io_end(void) +static inline void gen_io_end(TCGv_env cpu_env) { TCGv_i32 tmp = tcg_const_i32(0); tcg_gen_st_i32(tmp, cpu_env, -ENV_OFFSET + offsetof(CPUState, can_do_io)); diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 9b60680454..fdc49109ad 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -1329,9 +1329,9 @@ static ExitStatus gen_mfpr(DisasContext *ctx, TCGv va, int regno) helper = gen_helper_get_vmtime; do_helper: if (use_icount) { -gen_io_start(); +gen_io_start(cpu_env); helper(va); -gen_io_end(); +gen_io_end(cpu_env); return EXIT_PC_STALE; } else { helper(va); @@ -2379,9 +2379,9 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* RPCC */ va = dest_gpr(ctx, ra); if (ctx->tb->cflags & CF_USE_ICOUNT) { -gen_io_start(); +gen_io_start(cpu_env); gen_helper_load_pcc(va, cpu_env); -gen_io_end(); +gen_io_end(cpu_env); ret = EXIT_PC_STALE; } else { gen_helper_load_pcc(va, cpu_env); @@ -2955,7 +2955,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) pc_mask = ~TARGET_PAGE_MASK; } -gen_tb_start(tb); +gen_tb_start(tb, cpu_env); do { tcg_gen_insn_start(ctx.pc); num_insns++; @@ -2970,7 +2970,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) break; } if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { -gen_io_start(); +gen_io_start(cpu_env); } insn = cpu_ldl_code(env, ctx.pc); @@ -2991,7 +2991,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) } while (ret == NO_EXIT); if (tb->cflags & CF_LAST_IO) { -gen_io_end(); +gen_io_end(cpu_env); } switch (ret) { diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 860e279658..43261e7939 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1558,7 +1558,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, } if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { -gen_io_start(); +gen_io_start(cpu_env); } tcg_rt = cpu_reg(s, rt); @@ -1590,7 +1590,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { /* I/O operations must end the TB here (whether read or write) */ -gen_io_end(); +gen_io_end(cpu_env); s->is_jmp = DISAS_UPDATE; } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) { /* We default to
[Qemu-devel] [PATCH v7 04/26] target: [tcg] Add generic translation framework
Signed-off-by: Lluís Vilanova--- Makefile.target|1 include/exec/gen-icount.h |2 include/exec/translate-block.h | 125 +++ include/qom/cpu.h | 22 + translate-block.c | 185 5 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c diff --git a/Makefile.target b/Makefile.target index ce8dfe44a8..253c6e7999 100644 --- a/Makefile.target +++ b/Makefile.target @@ -90,6 +90,7 @@ all: $(PROGS) stap # cpu emulator library obj-y = exec.o translate-all.o cpu-exec.o obj-y += translate-common.o +obj-y += translate-block.o obj-y += cpu-exec-common.o obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o obj-$(CONFIG_TCG_INTERPRETER) += tci.o diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 9b26c7da5f..f4ad61014b 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -44,7 +44,7 @@ static inline void gen_tb_start(TranslationBlock *tb, TCGv_env cpu_env) tcg_temp_free_i32(count); } -static void gen_tb_end(TranslationBlock *tb, int num_insns) +static inline void gen_tb_end(TranslationBlock *tb, int num_insns) { if (tb->cflags & CF_USE_ICOUNT) { /* Update the num_insn immediate parameter now that we know diff --git a/include/exec/translate-block.h b/include/exec/translate-block.h new file mode 100644 index 00..d14d23f2cb --- /dev/null +++ b/include/exec/translate-block.h @@ -0,0 +1,125 @@ +/* + * Generic intermediate code generation. + * + * Copyright (C) 2016-2017 Lluís Vilanova + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef EXEC__TRANSLATE_BLOCK_H +#define EXEC__TRANSLATE_BLOCK_H + +/* + * Include this header from a target-specific file, and add a + * + * DisasContextBase base; + * + * member in your target-specific DisasContext. + */ + + +#include "exec/exec-all.h" +#include "tcg/tcg.h" + + +/** + * BreakpointCheckType: + * @BC_MISS: No hit + * @BC_HIT_INSN: Hit, but continue translating TB + * @BC_HIT_TB: Hit, stop translating TB + * + * How to react to a breakpoint. A hit means no more breakpoints will be checked + * for the current instruction. + * + * Not all breakpoints associated to an address are necessarily raised by + * targets (e.g., due to conditions encoded in their flags), so tey can decide + * that a breakpoint missed the address (@BP_MISS). + */ +typedef enum BreakpointCheckType { +BC_MISS, +BC_HIT_INSN, +BC_HIT_TB, +} BreakpointCheckType; + +/** + * DisasJumpType: + * @DJ_NEXT: Next instruction in program order. + * @DJ_TOO_MANY: Too many instructions translated. + * @DJ_TARGET: Start of target-specific conditions. + * + * What instruction to disassemble next. + */ +typedef enum DisasJumpType { +DJ_NEXT, +DJ_TOO_MANY, +DJ_TARGET, +} DisasJumpType; + +/** + * DisasContextBase: + * @tb: Translation block for this disassembly. + * @pc_first: Address of first guest instruction in this TB. + * @pc_next: Address of next guest instruction in this TB (current during + * disassembly). + * @is_jmp: What instruction to disassemble next. + * @num_insns: Number of translated instructions (including current). + * @singlestep_enabled: "Hardware" single stepping enabled. + * + * Architecture-agnostic disassembly context. + */ +typedef struct DisasContextBase { +TranslationBlock *tb; +target_ulong pc_first; +target_ulong pc_next; +DisasJumpType is_jmp; +unsigned int num_insns; +bool singlestep_enabled; +} DisasContextBase; + +/** + * TranslatorOps: + * @init_disas_context: Initialize a DisasContext struct (DisasContextBase has + * already been initialized). + * @init_globals: Initialize global variables. + * @tb_start: Start translating a new TB. + * @insn_start: Start translating a new instruction. + * @breakpoint_check: Check if a breakpoint did hit. When called, the breakpoint + *has already been checked to match the PC. + * @disas_insn: Disassemble one instruction an return the PC for the next + * one. Can set db->is_jmp to DJ_TARGET or above to stop + * translation. + * @tb_stop: Stop translating a TB. + * @disas_flags: Get flags argument for log_target_disas(). + * + * Target-specific operations for the generic translator loop. + * + * All operations but disas_insn() are optional, and ignored when not set. + * A missing breakpoint_check() will ignore breakpoints. A missing disas_flags() + * will pass no flags. + */ +typedef struct TranslatorOps { +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); +void (*init_globals)(DisasContextBase *db, CPUState *cpu); +void (*tb_start)(DisasContextBase *db, CPUState *cpu); +void
[Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
This series proposes a generic (target-agnostic) instruction translation framework. It basically provides a generic main loop for instruction disassembly, which calls target-specific functions when necessary. This generalization makes inserting new code in the main loop easier, and helps in keeping all targets in synch as to the contents of it. This series also paves the way towards adding events to trace guest code execution (BBLs and instructions). I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in the current organization, but will port the rest when this series gets merged. Signed-off-by: Lluís Vilanova--- Changes in v7 = * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). * Move target-specific translation functions to a struct (TranslatorOps). * Split target-specific changes into multiple patches. * Rebase on edf8bc9842. Changes in v6 = * Rebase on upstream master (64175afc69). * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. Changes in v5 = * Remove stray uses of "restrict" keyword. Changes in v4 = * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. * Fix coding style errors reported by checkpatch. * Remove use of "restrict" in added functions; it makes older gcc versions barf about compilation errors. Changes in v3 = * Rebase on 0737f32daf. Changes in v2 = * Port ARM and AARCH64 targets. * Fold single-stepping checks into "max_insns" [Richard Henderson]. * Move instruction start marks to target code [Richard Henderson]. * Add target hook for TB start. * Check for TCG temporary leaks. * Move instruction disassembly into a target hook. * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). Lluís Vilanova (26): Pass generic CPUState to gen_intermediate_code() queue: Add macro for incremental traversal cpu-exec: Avoid global variables in icount-related functions target: [tcg] Add generic translation framework target: [tcg] Redefine DISAS_* onto the generic translation framework (DJ_*) target: [tcg,i386] Port to DisasContextBase target: [tcg,i386] Refactor init_disas_context target: [tcg,i386] Refactor init_globals target: [tcg,i386] Refactor insn_start target: [tcg,i386] Refactor breakpoint_check target: [tcg,i386] Refactor disas_insn target: [tcg,i386] Refactor tb_stop target: [tcg,i386] Refactor disas_flags target: [tcg,i386] Replace DISAS_* with DJ_* target: [tcg,i386] Port to generic translation framework target: [tcg,arm] Replace DISAS_* with DJ_* target: [tcg,arm] Port to DisasContextBase target: [tcg,arm] Port to init_disas_context target: [tcg,arm] Port to init_globals target: [tcg,arm] Port to tb_start target: [tcg,arm] Port to insn_start target: [tcg,arm] Port to breakpoint_check target: [tcg,arm] Port to disas_insn target: [tcg,arm] Port to tb_stop target: [tcg,arm] Port to disas_flags target: [tcg,arm] Port to generic translation framework Makefile.target|1 include/exec/exec-all.h| 13 + include/exec/gen-icount.h |8 - include/exec/translate-block.h | 125 ++ include/qemu/queue.h | 12 + include/qom/cpu.h | 22 ++ target/alpha/translate.c | 25 +- target/arm/translate-a64.c | 312 - target/arm/translate.c | 503 ++-- target/arm/translate.h | 38 ++- target/cris/translate.c| 26 +- target/hppa/translate.c|6 target/i386/translate.c| 353 +++- target/lm32/translate.c| 36 +-- target/m68k/translate.c| 24 +- target/microblaze/translate.c | 28 +- target/mips/translate.c| 41 ++- target/moxie/translate.c | 16 + target/nios2/translate.c |6 target/openrisc/translate.c| 25 +- target/ppc/translate.c | 21 +- target/ppc/translate_init.c| 32 +-- target/s390x/translate.c | 22 +- target/sh4/translate.c | 21 +- target/sparc/translate.c | 17 + target/tilegx/translate.c |9 - target/tricore/translate.c | 11 - target/unicore32/translate.c | 26 +- target/xtensa/translate.c | 39 ++- translate-all.c|2 translate-block.c | 185 +++ 31 files changed, 1212 insertions(+), 793 deletions(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c To: qemu-devel@nongnu.org Cc: Paolo Bonzini Cc: Peter Crosthwaite Cc: Richard Henderson Cc: Alex Bennée
[Qemu-devel] [PATCH v7 02/26] queue: Add macro for incremental traversal
Adds macro QTAILQ_FOREACH_CONTINUE to support incremental list traversal. Signed-off-by: Lluís Vilanova--- include/qemu/queue.h | 12 1 file changed, 12 insertions(+) diff --git a/include/qemu/queue.h b/include/qemu/queue.h index 35292c3155..eb2bf9cb1c 100644 --- a/include/qemu/queue.h +++ b/include/qemu/queue.h @@ -415,6 +415,18 @@ struct { \ (var); \ (var) = ((var)->field.tqe_next)) +/** + * QTAILQ_FOREACH_CONTINUE: + * @var: Variable to resume iteration from. + * @field: Field in @var holding a QTAILQ_ENTRY for this queue. + * + * Resumes iteration on a queue from the element in @var. + */ +#define QTAILQ_FOREACH_CONTINUE(var, field) \ +for ((var) = ((var)->field.tqe_next); \ +(var); \ +(var) = ((var)->field.tqe_next)) + #define QTAILQ_FOREACH_SAFE(var, head, field, next_var) \ for ((var) = ((head)->tqh_first); \ (var) && ((next_var) = ((var)->field.tqe_next), 1); \
[Qemu-devel] [PATCH v7 04/26] target: [tcg] Add generic translation framework
Signed-off-by: Lluís Vilanova--- Makefile.target|1 include/exec/gen-icount.h |2 include/exec/translate-block.h | 125 +++ include/qom/cpu.h | 22 + translate-block.c | 185 5 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c diff --git a/Makefile.target b/Makefile.target index ce8dfe44a8..253c6e7999 100644 --- a/Makefile.target +++ b/Makefile.target @@ -90,6 +90,7 @@ all: $(PROGS) stap # cpu emulator library obj-y = exec.o translate-all.o cpu-exec.o obj-y += translate-common.o +obj-y += translate-block.o obj-y += cpu-exec-common.o obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o obj-$(CONFIG_TCG_INTERPRETER) += tci.o diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 9b26c7da5f..f4ad61014b 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -44,7 +44,7 @@ static inline void gen_tb_start(TranslationBlock *tb, TCGv_env cpu_env) tcg_temp_free_i32(count); } -static void gen_tb_end(TranslationBlock *tb, int num_insns) +static inline void gen_tb_end(TranslationBlock *tb, int num_insns) { if (tb->cflags & CF_USE_ICOUNT) { /* Update the num_insn immediate parameter now that we know diff --git a/include/exec/translate-block.h b/include/exec/translate-block.h new file mode 100644 index 00..d14d23f2cb --- /dev/null +++ b/include/exec/translate-block.h @@ -0,0 +1,125 @@ +/* + * Generic intermediate code generation. + * + * Copyright (C) 2016-2017 Lluís Vilanova + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef EXEC__TRANSLATE_BLOCK_H +#define EXEC__TRANSLATE_BLOCK_H + +/* + * Include this header from a target-specific file, and add a + * + * DisasContextBase base; + * + * member in your target-specific DisasContext. + */ + + +#include "exec/exec-all.h" +#include "tcg/tcg.h" + + +/** + * BreakpointCheckType: + * @BC_MISS: No hit + * @BC_HIT_INSN: Hit, but continue translating TB + * @BC_HIT_TB: Hit, stop translating TB + * + * How to react to a breakpoint. A hit means no more breakpoints will be checked + * for the current instruction. + * + * Not all breakpoints associated to an address are necessarily raised by + * targets (e.g., due to conditions encoded in their flags), so tey can decide + * that a breakpoint missed the address (@BP_MISS). + */ +typedef enum BreakpointCheckType { +BC_MISS, +BC_HIT_INSN, +BC_HIT_TB, +} BreakpointCheckType; + +/** + * DisasJumpType: + * @DJ_NEXT: Next instruction in program order. + * @DJ_TOO_MANY: Too many instructions translated. + * @DJ_TARGET: Start of target-specific conditions. + * + * What instruction to disassemble next. + */ +typedef enum DisasJumpType { +DJ_NEXT, +DJ_TOO_MANY, +DJ_TARGET, +} DisasJumpType; + +/** + * DisasContextBase: + * @tb: Translation block for this disassembly. + * @pc_first: Address of first guest instruction in this TB. + * @pc_next: Address of next guest instruction in this TB (current during + * disassembly). + * @is_jmp: What instruction to disassemble next. + * @num_insns: Number of translated instructions (including current). + * @singlestep_enabled: "Hardware" single stepping enabled. + * + * Architecture-agnostic disassembly context. + */ +typedef struct DisasContextBase { +TranslationBlock *tb; +target_ulong pc_first; +target_ulong pc_next; +DisasJumpType is_jmp; +unsigned int num_insns; +bool singlestep_enabled; +} DisasContextBase; + +/** + * TranslatorOps: + * @init_disas_context: Initialize a DisasContext struct (DisasContextBase has + * already been initialized). + * @init_globals: Initialize global variables. + * @tb_start: Start translating a new TB. + * @insn_start: Start translating a new instruction. + * @breakpoint_check: Check if a breakpoint did hit. When called, the breakpoint + *has already been checked to match the PC. + * @disas_insn: Disassemble one instruction an return the PC for the next + * one. Can set db->is_jmp to DJ_TARGET or above to stop + * translation. + * @tb_stop: Stop translating a TB. + * @disas_flags: Get flags argument for log_target_disas(). + * + * Target-specific operations for the generic translator loop. + * + * All operations but disas_insn() are optional, and ignored when not set. + * A missing breakpoint_check() will ignore breakpoints. A missing disas_flags() + * will pass no flags. + */ +typedef struct TranslatorOps { +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); +void (*init_globals)(DisasContextBase *db, CPUState *cpu); +void (*tb_start)(DisasContextBase *db, CPUState *cpu); +void
[Qemu-devel] [PATCH v7 01/26] Pass generic CPUState to gen_intermediate_code()
Needed to implement a target-agnostic gen_intermediate_code() in the future. Signed-off-by: Lluís VilanovaReviewed-by: David Gibson Reviewed-by: Richard Henderson --- include/exec/exec-all.h |2 +- target/alpha/translate.c | 11 +-- target/arm/translate.c| 20 ++-- target/cris/translate.c | 17 - target/i386/translate.c | 13 ++--- target/lm32/translate.c | 22 +++--- target/m68k/translate.c | 15 +++ target/microblaze/translate.c | 22 +++--- target/mips/translate.c | 15 +++ target/moxie/translate.c | 14 +++--- target/openrisc/translate.c | 19 ++- target/ppc/translate.c| 15 +++ target/s390x/translate.c | 13 ++--- target/sh4/translate.c| 15 +++ target/sparc/translate.c | 11 +-- target/tilegx/translate.c |7 +++ target/tricore/translate.c|9 - target/unicore32/translate.c | 17 - target/xtensa/translate.c | 13 ++--- translate-all.c |2 +- 20 files changed, 130 insertions(+), 142 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 87ae10bcc9..1ec7637170 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -43,7 +43,7 @@ typedef ram_addr_t tb_page_addr_t; #include "qemu/log.h" -void gen_intermediate_code(CPUArchState *env, struct TranslationBlock *tb); +void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb); void restore_state_to_opc(CPUArchState *env, struct TranslationBlock *tb, target_ulong *data); diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 7c45ae360c..9b60680454 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -2900,10 +2900,9 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) return ret; } -void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) +void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) { -AlphaCPU *cpu = alpha_env_get_cpu(env); -CPUState *cs = CPU(cpu); +CPUAlphaState *env = cpu->env_ptr; DisasContext ctx, *ctxp = target_ulong pc_start; target_ulong pc_mask; @@ -2918,7 +2917,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) ctx.pc = pc_start; ctx.mem_idx = cpu_mmu_index(env, false); ctx.implver = env->implver; -ctx.singlestep_enabled = cs->singlestep_enabled; +ctx.singlestep_enabled = cpu->singlestep_enabled; #ifdef CONFIG_USER_ONLY ctx.ir = cpu_std_ir; @@ -2961,7 +2960,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) tcg_gen_insn_start(ctx.pc); num_insns++; -if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) { +if (unlikely(cpu_breakpoint_test(cpu, ctx.pc, BP_ANY))) { ret = gen_excp(, EXCP_DEBUG, 0); /* The address covered by the breakpoint must be included in [tb->pc, tb->pc + tb->size) in order to for it to be @@ -3030,7 +3029,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) && qemu_log_in_addr_range(pc_start)) { qemu_log_lock(); qemu_log("IN: %s\n", lookup_symbol(pc_start)); -log_target_disas(cs, pc_start, ctx.pc - pc_start, 1); +log_target_disas(cpu, pc_start, ctx.pc - pc_start, 1); qemu_log("\n"); qemu_log_unlock(); } diff --git a/target/arm/translate.c b/target/arm/translate.c index 0862f9e4aa..96272a9888 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -11787,10 +11787,10 @@ static bool insn_crosses_page(CPUARMState *env, DisasContext *s) } /* generate intermediate code for basic block 'tb'. */ -void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { -ARMCPU *cpu = arm_env_get_cpu(env); -CPUState *cs = CPU(cpu); +CPUARMState *env = cpu->env_ptr; +ARMCPU *arm_cpu = arm_env_get_cpu(env); DisasContext dc1, *dc = target_ulong pc_start; target_ulong next_page_start; @@ -11804,7 +11804,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) * the A32/T32 complexity to do with conditional execution/IT blocks/etc. */ if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) { -gen_intermediate_code_a64(cpu, tb); +gen_intermediate_code_a64(arm_cpu, tb); return; } @@ -11814,7 +11814,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) dc->is_jmp = DISAS_NEXT; dc->pc = pc_start; -dc->singlestep_enabled = cs->singlestep_enabled; +
[Qemu-devel] [PATCH v7 02/26] queue: Add macro for incremental traversal
Adds macro QTAILQ_FOREACH_CONTINUE to support incremental list traversal. Signed-off-by: Lluís Vilanova--- include/qemu/queue.h | 12 1 file changed, 12 insertions(+) diff --git a/include/qemu/queue.h b/include/qemu/queue.h index 35292c3155..eb2bf9cb1c 100644 --- a/include/qemu/queue.h +++ b/include/qemu/queue.h @@ -415,6 +415,18 @@ struct { \ (var); \ (var) = ((var)->field.tqe_next)) +/** + * QTAILQ_FOREACH_CONTINUE: + * @var: Variable to resume iteration from. + * @field: Field in @var holding a QTAILQ_ENTRY for this queue. + * + * Resumes iteration on a queue from the element in @var. + */ +#define QTAILQ_FOREACH_CONTINUE(var, field) \ +for ((var) = ((var)->field.tqe_next); \ +(var); \ +(var) = ((var)->field.tqe_next)) + #define QTAILQ_FOREACH_SAFE(var, head, field, next_var) \ for ((var) = ((head)->tqh_first); \ (var) && ((next_var) = ((var)->field.tqe_next), 1); \
[Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
This series proposes a generic (target-agnostic) instruction translation framework. It basically provides a generic main loop for instruction disassembly, which calls target-specific functions when necessary. This generalization makes inserting new code in the main loop easier, and helps in keeping all targets in synch as to the contents of it. This series also paves the way towards adding events to trace guest code execution (BBLs and instructions). I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in the current organization, but will port the rest when this series gets merged. Signed-off-by: Lluís Vilanova--- Changes in v7 = * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). * Move target-specific translation functions to a struct (TranslatorOps). * Split target-specific changes into multiple patches. * Rebase on edf8bc9842. Changes in v6 = * Rebase on upstream master (64175afc69). * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. Changes in v5 = * Remove stray uses of "restrict" keyword. Changes in v4 = * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. * Fix coding style errors reported by checkpatch. * Remove use of "restrict" in added functions; it makes older gcc versions barf about compilation errors. Changes in v3 = * Rebase on 0737f32daf. Changes in v2 = * Port ARM and AARCH64 targets. * Fold single-stepping checks into "max_insns" [Richard Henderson]. * Move instruction start marks to target code [Richard Henderson]. * Add target hook for TB start. * Check for TCG temporary leaks. * Move instruction disassembly into a target hook. * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). Lluís Vilanova (26): Pass generic CPUState to gen_intermediate_code() queue: Add macro for incremental traversal cpu-exec: Avoid global variables in icount-related functions target: [tcg] Add generic translation framework target: [tcg] Redefine DISAS_* onto the generic translation framework (DJ_*) target: [tcg,i386] Port to DisasContextBase target: [tcg,i386] Refactor init_disas_context target: [tcg,i386] Refactor init_globals target: [tcg,i386] Refactor insn_start target: [tcg,i386] Refactor breakpoint_check target: [tcg,i386] Refactor disas_insn target: [tcg,i386] Refactor tb_stop target: [tcg,i386] Refactor disas_flags target: [tcg,i386] Replace DISAS_* with DJ_* target: [tcg,i386] Port to generic translation framework target: [tcg,arm] Replace DISAS_* with DJ_* target: [tcg,arm] Port to DisasContextBase target: [tcg,arm] Port to init_disas_context target: [tcg,arm] Port to init_globals target: [tcg,arm] Port to tb_start target: [tcg,arm] Port to insn_start target: [tcg,arm] Port to breakpoint_check target: [tcg,arm] Port to disas_insn target: [tcg,arm] Port to tb_stop target: [tcg,arm] Port to disas_flags target: [tcg,arm] Port to generic translation framework Makefile.target|1 include/exec/exec-all.h| 13 + include/exec/gen-icount.h |8 - include/exec/translate-block.h | 125 ++ include/qemu/queue.h | 12 + include/qom/cpu.h | 22 ++ target/alpha/translate.c | 25 +- target/arm/translate-a64.c | 312 - target/arm/translate.c | 503 ++-- target/arm/translate.h | 38 ++- target/cris/translate.c| 26 +- target/hppa/translate.c|6 target/i386/translate.c| 353 +++- target/lm32/translate.c| 36 +-- target/m68k/translate.c| 24 +- target/microblaze/translate.c | 28 +- target/mips/translate.c| 41 ++- target/moxie/translate.c | 16 + target/nios2/translate.c |6 target/openrisc/translate.c| 25 +- target/ppc/translate.c | 21 +- target/ppc/translate_init.c| 32 +-- target/s390x/translate.c | 22 +- target/sh4/translate.c | 21 +- target/sparc/translate.c | 17 + target/tilegx/translate.c |9 - target/tricore/translate.c | 11 - target/unicore32/translate.c | 26 +- target/xtensa/translate.c | 39 ++- translate-all.c|2 translate-block.c | 185 +++ 31 files changed, 1212 insertions(+), 793 deletions(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c To: qemu-devel@nongnu.org Cc: Paolo Bonzini Cc: Peter Crosthwaite Cc: Richard Henderson Cc: Alex Bennée
Re: [Qemu-devel] [PATCH v2 0/3] target/s390x: implement idte and improve ipte
On 06/22/2017 02:41 AM, David Hildenbrand wrote: By adding idte, we are now able to expose the DAT-enhancement facility to our guest. Also, properly simulate and expose the local-tlb-clearing facility. To improve the TLB flushing, we will have to remember each used table (or at least a hash!) for each tlb entry, just like real HW does. This allows me to start an upstream kernel (having also the mvcos patch applied) compiled for z9 using: qemu-system-s390x ... -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ eimm=on,stckf=on,csst=on,csst2=on,ginste=on,\ exrl=on,dateh=on,ltlbc=on Linux will detect the DAT-enhancement facility and use idte+cspg. v1 -> v2: - Allow to enable the DAT-enhancement facility. - Fix wrong register in idte. - Simply set m4 to zero in case local-tlb-clearing is not enabled. Reviewed and applied to my target/s390x tree. r~
[Qemu-devel] [PATCH v7 03/26] cpu-exec: Avoid global variables in icount-related functions
Signed-off-by: Lluís Vilanova--- include/exec/gen-icount.h |6 +++-- target/alpha/translate.c | 14 ++-- target/arm/translate-a64.c| 10 - target/arm/translate.c| 10 - target/cris/translate.c |6 +++-- target/hppa/translate.c |6 +++-- target/i386/translate.c | 46 + target/lm32/translate.c | 14 ++-- target/m68k/translate.c |6 +++-- target/microblaze/translate.c |6 +++-- target/mips/translate.c | 26 --- target/moxie/translate.c |2 +- target/nios2/translate.c |6 +++-- target/openrisc/translate.c |6 +++-- target/ppc/translate.c|6 +++-- target/ppc/translate_init.c | 32 ++--- target/s390x/translate.c |6 +++-- target/sh4/translate.c|6 +++-- target/sparc/translate.c |6 +++-- target/tilegx/translate.c |2 +- target/tricore/translate.c|2 +- target/unicore32/translate.c |6 +++-- target/xtensa/translate.c | 26 --- 23 files changed, 128 insertions(+), 128 deletions(-) diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 62d462e494..9b26c7da5f 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -8,7 +8,7 @@ static int icount_start_insn_idx; static TCGLabel *exitreq_label; -static inline void gen_tb_start(TranslationBlock *tb) +static inline void gen_tb_start(TranslationBlock *tb, TCGv_env cpu_env) { TCGv_i32 count, imm; @@ -59,14 +59,14 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns) tcg_ctx.gen_op_buf[tcg_ctx.gen_op_buf[0].prev].next = 0; } -static inline void gen_io_start(void) +static inline void gen_io_start(TCGv_env cpu_env) { TCGv_i32 tmp = tcg_const_i32(1); tcg_gen_st_i32(tmp, cpu_env, -ENV_OFFSET + offsetof(CPUState, can_do_io)); tcg_temp_free_i32(tmp); } -static inline void gen_io_end(void) +static inline void gen_io_end(TCGv_env cpu_env) { TCGv_i32 tmp = tcg_const_i32(0); tcg_gen_st_i32(tmp, cpu_env, -ENV_OFFSET + offsetof(CPUState, can_do_io)); diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 9b60680454..fdc49109ad 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -1329,9 +1329,9 @@ static ExitStatus gen_mfpr(DisasContext *ctx, TCGv va, int regno) helper = gen_helper_get_vmtime; do_helper: if (use_icount) { -gen_io_start(); +gen_io_start(cpu_env); helper(va); -gen_io_end(); +gen_io_end(cpu_env); return EXIT_PC_STALE; } else { helper(va); @@ -2379,9 +2379,9 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* RPCC */ va = dest_gpr(ctx, ra); if (ctx->tb->cflags & CF_USE_ICOUNT) { -gen_io_start(); +gen_io_start(cpu_env); gen_helper_load_pcc(va, cpu_env); -gen_io_end(); +gen_io_end(cpu_env); ret = EXIT_PC_STALE; } else { gen_helper_load_pcc(va, cpu_env); @@ -2955,7 +2955,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) pc_mask = ~TARGET_PAGE_MASK; } -gen_tb_start(tb); +gen_tb_start(tb, cpu_env); do { tcg_gen_insn_start(ctx.pc); num_insns++; @@ -2970,7 +2970,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) break; } if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { -gen_io_start(); +gen_io_start(cpu_env); } insn = cpu_ldl_code(env, ctx.pc); @@ -2991,7 +2991,7 @@ void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) } while (ret == NO_EXIT); if (tb->cflags & CF_LAST_IO) { -gen_io_end(); +gen_io_end(cpu_env); } switch (ret) { diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 860e279658..43261e7939 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1558,7 +1558,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, } if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { -gen_io_start(); +gen_io_start(cpu_env); } tcg_rt = cpu_reg(s, rt); @@ -1590,7 +1590,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { /* I/O operations must end the TB here (whether read or write) */ -gen_io_end(); +gen_io_end(cpu_env); s->is_jmp = DISAS_UPDATE; } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) { /* We default to
[Qemu-devel] [PATCH v7 01/26] Pass generic CPUState to gen_intermediate_code()
Needed to implement a target-agnostic gen_intermediate_code() in the future. Signed-off-by: Lluís VilanovaReviewed-by: David Gibson Reviewed-by: Richard Henderson --- include/exec/exec-all.h |2 +- target/alpha/translate.c | 11 +-- target/arm/translate.c| 20 ++-- target/cris/translate.c | 17 - target/i386/translate.c | 13 ++--- target/lm32/translate.c | 22 +++--- target/m68k/translate.c | 15 +++ target/microblaze/translate.c | 22 +++--- target/mips/translate.c | 15 +++ target/moxie/translate.c | 14 +++--- target/openrisc/translate.c | 19 ++- target/ppc/translate.c| 15 +++ target/s390x/translate.c | 13 ++--- target/sh4/translate.c| 15 +++ target/sparc/translate.c | 11 +-- target/tilegx/translate.c |7 +++ target/tricore/translate.c|9 - target/unicore32/translate.c | 17 - target/xtensa/translate.c | 13 ++--- translate-all.c |2 +- 20 files changed, 130 insertions(+), 142 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 87ae10bcc9..1ec7637170 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -43,7 +43,7 @@ typedef ram_addr_t tb_page_addr_t; #include "qemu/log.h" -void gen_intermediate_code(CPUArchState *env, struct TranslationBlock *tb); +void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb); void restore_state_to_opc(CPUArchState *env, struct TranslationBlock *tb, target_ulong *data); diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 7c45ae360c..9b60680454 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -2900,10 +2900,9 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) return ret; } -void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) +void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) { -AlphaCPU *cpu = alpha_env_get_cpu(env); -CPUState *cs = CPU(cpu); +CPUAlphaState *env = cpu->env_ptr; DisasContext ctx, *ctxp = target_ulong pc_start; target_ulong pc_mask; @@ -2918,7 +2917,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) ctx.pc = pc_start; ctx.mem_idx = cpu_mmu_index(env, false); ctx.implver = env->implver; -ctx.singlestep_enabled = cs->singlestep_enabled; +ctx.singlestep_enabled = cpu->singlestep_enabled; #ifdef CONFIG_USER_ONLY ctx.ir = cpu_std_ir; @@ -2961,7 +2960,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) tcg_gen_insn_start(ctx.pc); num_insns++; -if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) { +if (unlikely(cpu_breakpoint_test(cpu, ctx.pc, BP_ANY))) { ret = gen_excp(, EXCP_DEBUG, 0); /* The address covered by the breakpoint must be included in [tb->pc, tb->pc + tb->size) in order to for it to be @@ -3030,7 +3029,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb) && qemu_log_in_addr_range(pc_start)) { qemu_log_lock(); qemu_log("IN: %s\n", lookup_symbol(pc_start)); -log_target_disas(cs, pc_start, ctx.pc - pc_start, 1); +log_target_disas(cpu, pc_start, ctx.pc - pc_start, 1); qemu_log("\n"); qemu_log_unlock(); } diff --git a/target/arm/translate.c b/target/arm/translate.c index 0862f9e4aa..96272a9888 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -11787,10 +11787,10 @@ static bool insn_crosses_page(CPUARMState *env, DisasContext *s) } /* generate intermediate code for basic block 'tb'. */ -void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { -ARMCPU *cpu = arm_env_get_cpu(env); -CPUState *cs = CPU(cpu); +CPUARMState *env = cpu->env_ptr; +ARMCPU *arm_cpu = arm_env_get_cpu(env); DisasContext dc1, *dc = target_ulong pc_start; target_ulong next_page_start; @@ -11804,7 +11804,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) * the A32/T32 complexity to do with conditional execution/IT blocks/etc. */ if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) { -gen_intermediate_code_a64(cpu, tb); +gen_intermediate_code_a64(arm_cpu, tb); return; } @@ -11814,7 +11814,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) dc->is_jmp = DISAS_NEXT; dc->pc = pc_start; -dc->singlestep_enabled = cs->singlestep_enabled; +
[Qemu-devel] [PATCH v7 04/26] target: [tcg] Add generic translation framework
Signed-off-by: Lluís Vilanova--- Makefile.target|1 include/exec/gen-icount.h |2 include/exec/translate-block.h | 125 +++ include/qom/cpu.h | 22 + translate-block.c | 185 5 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c diff --git a/Makefile.target b/Makefile.target index ce8dfe44a8..253c6e7999 100644 --- a/Makefile.target +++ b/Makefile.target @@ -90,6 +90,7 @@ all: $(PROGS) stap # cpu emulator library obj-y = exec.o translate-all.o cpu-exec.o obj-y += translate-common.o +obj-y += translate-block.o obj-y += cpu-exec-common.o obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o obj-$(CONFIG_TCG_INTERPRETER) += tci.o diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 9b26c7da5f..f4ad61014b 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -44,7 +44,7 @@ static inline void gen_tb_start(TranslationBlock *tb, TCGv_env cpu_env) tcg_temp_free_i32(count); } -static void gen_tb_end(TranslationBlock *tb, int num_insns) +static inline void gen_tb_end(TranslationBlock *tb, int num_insns) { if (tb->cflags & CF_USE_ICOUNT) { /* Update the num_insn immediate parameter now that we know diff --git a/include/exec/translate-block.h b/include/exec/translate-block.h new file mode 100644 index 00..d14d23f2cb --- /dev/null +++ b/include/exec/translate-block.h @@ -0,0 +1,125 @@ +/* + * Generic intermediate code generation. + * + * Copyright (C) 2016-2017 Lluís Vilanova + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef EXEC__TRANSLATE_BLOCK_H +#define EXEC__TRANSLATE_BLOCK_H + +/* + * Include this header from a target-specific file, and add a + * + * DisasContextBase base; + * + * member in your target-specific DisasContext. + */ + + +#include "exec/exec-all.h" +#include "tcg/tcg.h" + + +/** + * BreakpointCheckType: + * @BC_MISS: No hit + * @BC_HIT_INSN: Hit, but continue translating TB + * @BC_HIT_TB: Hit, stop translating TB + * + * How to react to a breakpoint. A hit means no more breakpoints will be checked + * for the current instruction. + * + * Not all breakpoints associated to an address are necessarily raised by + * targets (e.g., due to conditions encoded in their flags), so tey can decide + * that a breakpoint missed the address (@BP_MISS). + */ +typedef enum BreakpointCheckType { +BC_MISS, +BC_HIT_INSN, +BC_HIT_TB, +} BreakpointCheckType; + +/** + * DisasJumpType: + * @DJ_NEXT: Next instruction in program order. + * @DJ_TOO_MANY: Too many instructions translated. + * @DJ_TARGET: Start of target-specific conditions. + * + * What instruction to disassemble next. + */ +typedef enum DisasJumpType { +DJ_NEXT, +DJ_TOO_MANY, +DJ_TARGET, +} DisasJumpType; + +/** + * DisasContextBase: + * @tb: Translation block for this disassembly. + * @pc_first: Address of first guest instruction in this TB. + * @pc_next: Address of next guest instruction in this TB (current during + * disassembly). + * @is_jmp: What instruction to disassemble next. + * @num_insns: Number of translated instructions (including current). + * @singlestep_enabled: "Hardware" single stepping enabled. + * + * Architecture-agnostic disassembly context. + */ +typedef struct DisasContextBase { +TranslationBlock *tb; +target_ulong pc_first; +target_ulong pc_next; +DisasJumpType is_jmp; +unsigned int num_insns; +bool singlestep_enabled; +} DisasContextBase; + +/** + * TranslatorOps: + * @init_disas_context: Initialize a DisasContext struct (DisasContextBase has + * already been initialized). + * @init_globals: Initialize global variables. + * @tb_start: Start translating a new TB. + * @insn_start: Start translating a new instruction. + * @breakpoint_check: Check if a breakpoint did hit. When called, the breakpoint + *has already been checked to match the PC. + * @disas_insn: Disassemble one instruction an return the PC for the next + * one. Can set db->is_jmp to DJ_TARGET or above to stop + * translation. + * @tb_stop: Stop translating a TB. + * @disas_flags: Get flags argument for log_target_disas(). + * + * Target-specific operations for the generic translator loop. + * + * All operations but disas_insn() are optional, and ignored when not set. + * A missing breakpoint_check() will ignore breakpoints. A missing disas_flags() + * will pass no flags. + */ +typedef struct TranslatorOps { +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); +void (*init_globals)(DisasContextBase *db, CPUState *cpu); +void (*tb_start)(DisasContextBase *db, CPUState *cpu); +void
[Qemu-devel] [PATCH v7 08/26] target: [tcg, i386] Refactor init_globals
Incrementally paves the way towards using the generic instruction translation loop. Signed-off-by: Lluís Vilanova--- target/i386/translate.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 84ff49030b..f0d12a3d13 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -8452,6 +8452,22 @@ static void i386_trblock_init_disas_context(DisasContextBase *db, CPUState *cpu) #endif } +static void i386_trblock_init_globals(DisasContextBase *db, CPUState *cpu) +{ +cpu_T0 = tcg_temp_new(); +cpu_T1 = tcg_temp_new(); +cpu_A0 = tcg_temp_new(); + +cpu_tmp0 = tcg_temp_new(); +cpu_tmp1_i64 = tcg_temp_new_i64(); +cpu_tmp2_i32 = tcg_temp_new_i32(); +cpu_tmp3_i32 = tcg_temp_new_i32(); +cpu_tmp4 = tcg_temp_new(); +cpu_ptr0 = tcg_temp_new_ptr(); +cpu_ptr1 = tcg_temp_new_ptr(); +cpu_cc_srcT = tcg_temp_local_new(); +} + /* generate intermediate code for basic block 'tb'. */ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) { @@ -8469,18 +8485,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb) db->pc_next = db->pc_first; i386_trblock_init_disas_context(db, cpu); -cpu_T0 = tcg_temp_new(); -cpu_T1 = tcg_temp_new(); -cpu_A0 = tcg_temp_new(); - -cpu_tmp0 = tcg_temp_new(); -cpu_tmp1_i64 = tcg_temp_new_i64(); -cpu_tmp2_i32 = tcg_temp_new_i32(); -cpu_tmp3_i32 = tcg_temp_new_i32(); -cpu_tmp4 = tcg_temp_new(); -cpu_ptr0 = tcg_temp_new_ptr(); -cpu_ptr1 = tcg_temp_new_ptr(); -cpu_cc_srcT = tcg_temp_local_new(); +i386_trblock_init_globals(db, cpu); num_insns = 0; max_insns = tb->cflags & CF_COUNT_MASK;
[Qemu-devel] [RFC PATCH v7 00/26] translate: [tcg] Generic translation framework
This series proposes a generic (target-agnostic) instruction translation framework. It basically provides a generic main loop for instruction disassembly, which calls target-specific functions when necessary. This generalization makes inserting new code in the main loop easier, and helps in keeping all targets in synch as to the contents of it. This series also paves the way towards adding events to trace guest code execution (BBLs and instructions). I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in the current organization, but will port the rest when this series gets merged. Signed-off-by: Lluís Vilanova--- Changes in v7 = * Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*). * Move target-specific translation functions to a struct (TranslatorOps). * Split target-specific changes into multiple patches. * Rebase on edf8bc9842. Changes in v6 = * Rebase on upstream master (64175afc69). * Reorder fields in DisasContextBase to minimize padding [Richard Henderson]. Changes in v5 = * Remove stray uses of "restrict" keyword. Changes in v4 = * Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell]. * Fix coding style errors reported by checkpatch. * Remove use of "restrict" in added functions; it makes older gcc versions barf about compilation errors. Changes in v3 = * Rebase on 0737f32daf. Changes in v2 = * Port ARM and AARCH64 targets. * Fold single-stepping checks into "max_insns" [Richard Henderson]. * Move instruction start marks to target code [Richard Henderson]. * Add target hook for TB start. * Check for TCG temporary leaks. * Move instruction disassembly into a target hook. * Make breakpoint_hit() return an enum to accomodate target's needs (ARM). Lluís Vilanova (26): Pass generic CPUState to gen_intermediate_code() queue: Add macro for incremental traversal cpu-exec: Avoid global variables in icount-related functions target: [tcg] Add generic translation framework target: [tcg] Redefine DISAS_* onto the generic translation framework (DJ_*) target: [tcg,i386] Port to DisasContextBase target: [tcg,i386] Refactor init_disas_context target: [tcg,i386] Refactor init_globals target: [tcg,i386] Refactor insn_start target: [tcg,i386] Refactor breakpoint_check target: [tcg,i386] Refactor disas_insn target: [tcg,i386] Refactor tb_stop target: [tcg,i386] Refactor disas_flags target: [tcg,i386] Replace DISAS_* with DJ_* target: [tcg,i386] Port to generic translation framework target: [tcg,arm] Replace DISAS_* with DJ_* target: [tcg,arm] Port to DisasContextBase target: [tcg,arm] Port to init_disas_context target: [tcg,arm] Port to init_globals target: [tcg,arm] Port to tb_start target: [tcg,arm] Port to insn_start target: [tcg,arm] Port to breakpoint_check target: [tcg,arm] Port to disas_insn target: [tcg,arm] Port to tb_stop target: [tcg,arm] Port to disas_flags target: [tcg,arm] Port to generic translation framework Makefile.target|1 include/exec/exec-all.h| 13 + include/exec/gen-icount.h |8 - include/exec/translate-block.h | 125 ++ include/qemu/queue.h | 12 + include/qom/cpu.h | 22 ++ target/alpha/translate.c | 25 +- target/arm/translate-a64.c | 312 - target/arm/translate.c | 503 ++-- target/arm/translate.h | 38 ++- target/cris/translate.c| 26 +- target/hppa/translate.c|6 target/i386/translate.c| 353 +++- target/lm32/translate.c| 36 +-- target/m68k/translate.c| 24 +- target/microblaze/translate.c | 28 +- target/mips/translate.c| 41 ++- target/moxie/translate.c | 16 + target/nios2/translate.c |6 target/openrisc/translate.c| 25 +- target/ppc/translate.c | 21 +- target/ppc/translate_init.c| 32 +-- target/s390x/translate.c | 22 +- target/sh4/translate.c | 21 +- target/sparc/translate.c | 17 + target/tilegx/translate.c |9 - target/tricore/translate.c | 11 - target/unicore32/translate.c | 26 +- target/xtensa/translate.c | 39 ++- translate-all.c|2 translate-block.c | 185 +++ 31 files changed, 1212 insertions(+), 793 deletions(-) create mode 100644 include/exec/translate-block.h create mode 100644 translate-block.c To: qemu-devel@nongnu.org Cc: Paolo Bonzini Cc: Peter Crosthwaite Cc: Richard Henderson Cc: Alex Bennée
[Qemu-devel] [PATCH v7 02/26] queue: Add macro for incremental traversal
Adds macro QTAILQ_FOREACH_CONTINUE to support incremental list traversal. Signed-off-by: Lluís Vilanova--- include/qemu/queue.h | 12 1 file changed, 12 insertions(+) diff --git a/include/qemu/queue.h b/include/qemu/queue.h index 35292c3155..eb2bf9cb1c 100644 --- a/include/qemu/queue.h +++ b/include/qemu/queue.h @@ -415,6 +415,18 @@ struct { \ (var); \ (var) = ((var)->field.tqe_next)) +/** + * QTAILQ_FOREACH_CONTINUE: + * @var: Variable to resume iteration from. + * @field: Field in @var holding a QTAILQ_ENTRY for this queue. + * + * Resumes iteration on a queue from the element in @var. + */ +#define QTAILQ_FOREACH_CONTINUE(var, field) \ +for ((var) = ((var)->field.tqe_next); \ +(var); \ +(var) = ((var)->field.tqe_next)) + #define QTAILQ_FOREACH_SAFE(var, head, field, next_var) \ for ((var) = ((head)->tqh_first); \ (var) && ((next_var) = ((var)->field.tqe_next), 1); \
Re: [Qemu-devel] [PATCH v4 02/10] accel: introduce AccelClass.global_props
On Thu, Jun 22, 2017 at 12:25:30PM +0800, Peter Xu wrote: > On Wed, Jun 21, 2017 at 09:23:11AM -0300, Eduardo Habkost wrote: > > On Wed, Jun 21, 2017 at 03:52:00PM +0800, Peter Xu wrote: [...] > > > diff --git a/vl.c b/vl.c > > > index 59fea15..4452d7a 100644 > > > --- a/vl.c > > > +++ b/vl.c > > > @@ -4571,6 +4571,7 @@ int main(int argc, char **argv, char **envp) > > > exit (i == 1 ? 1 : 0); > > > } > > > > > > > I suggest a comment here warning about global property > > registration ordering. e.g.: > > > > /* > > * Ordering of global property registration matters: > > * - machine compat_props should override accelerator-specific > > * globals. > > * - user-provided globals (-global, and global properties > > * derived from other command-line options like -cpu) should > > * override machine compat_props and accelerator-specific > > * globals. > > */ > > I have a standalone patch (next one) to unify these three places and > added some comment. Would that suite? Sorry, I didn't see that patch when I was reviewing this one. It looks good to me. Thanks! -- Eduardo
Re: [Qemu-devel] [PATCH 3/5] memory: Add RAM_NONPERSISTENT flag
On Thu, Jun 22, 2017 at 01:14:58PM +0100, Dr. David Alan Gilbert wrote: > * Eduardo Habkost (ehabk...@redhat.com) wrote: > > The new flag will make qemu_ram_free() discard the contents of the > > block. It will be used to let QEMU be configured to avoid flushing file > > contents to disk when exiting. As MADV_REMOVE is not always supported, > > the new code will try MADV_NOTNEEDED in case MADV_REMOVE fails. > > I'd like to understand what semantics you're trying to achieve and thus > why you prefer REMOVE to DONTNEED. If you're trying to avoid changes > being written back then doesn't a DONTNEED get rid of any changes that > have yet to be written? Or are there changes that have already been > queued that REMOVE will kill off? > Generally speaking, it look(ed) like REMOVE is a superset of DONTNEED: DONTNEED will free and zero pages only on anonymous private mappings; REMOVE will free resources and zero pages on additional cases. One case where I can think REMOVE would be useful is tmpfs when swapping is involved: with REMOVE, the host can drop swap contents or avoid writing memory contents to swap even if we are using a shared tmpfs mapping. Other filesystems might have similar cases where unnecessary I/O operations might be performed even after madvise(MADV_DONTNEED) is called. MADV_REMOVE lets us simply tell the kernel to drop the data. I'm CCing Zack Cornelius, who initially suggested MADV_REMOVE, in case he can describe more specific use cases. > If you're just trying to save-time in writeback, it's interesting to > note my requirement is that by the time I exit this function the > process of throwing away the memory contents must be complete; > I think your requirements are a lot lazier as to when it happens. This is a very good point. I was assuming that REMOVE is a superset of DONTNEED, but based on the manpage it doesn't seem to be guaranteed. Probably I shouldn't try to reuse ram_block_discard_range() and write a separate helper for madvise(MADV_REMOVE), as the requirements are different. > > The new flag will also indicate that ram_block_discard_range() can use > > MADV_REMOVE when discarding memory pages. I have considered calling > > MADV_REMOVE unconditionally (as destroying the RAM contents seems to be > > OK every time ram_block_discard_range() is called), but for safety I > > decided to restrict the new code to blocks having RAM_NONPERSISTENT set. > > The manpage on MADV_REMOVE is confusing; it says it doesn't work on Huge > TLB pages, but says it does work on anything that can do > FALLOC_FL_PUNCH_HOLE - which as far as I can tell hugetlbfs does. Yes, it's confusing. I need to do some testing to find out if HugeTLBFS supports MADV_REMOVE today. But my use case is just an optimization, so it won't be a big deal if it doesn't cover every case in the first version. > > I've got some code in my shared-postcopy world that has this function do > the following which is kind of similar: > > /* The logic here is messy; > *madvise DONTNEED fails for hugepages > *fallocate works on hugepages and shmem > */ > need_madvise = (rb->page_size == qemu_host_page_size) && >(rb->fd == -1 || !(rb->flags & RAM_SHARED)); > need_fallocate = rb->fd != -1; This looks safer to me. I was bothered by the missing check for (rb->fd != -1) in the current code. > if (ret == -1 && need_fallocate) { > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > ret = fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | > FALLOC_FL_KEEP_SIZE, > start, length); > #endif > } > if (need_madvise && (!need_fallocate || (ret == 0))) { I'm confused by the (ret == 0) check here. Do you still want to call madvise() if fallocate() succeeded? > #if defined(CONFIG_MADVISE) > ret = madvise(host_startaddr, length, MADV_DONTNEED); > fprintf(stderr, "%s: Did madvise for %p got %d\n", __func__, > host_startaddr, ret); > #endif > } Anyway, now I'm considering simply not touching ram_block_discard_range() and adding a new helper, because the requirements are different. Maybe in the future we can make the two functions share code, if we decide FALLOC_FL_PUNCH_HOLE will be useful for RAM_NONPERSISTENT too. (BTW, I will probably rename "persistent=no"/RAM_NONPERSISTENT to something more explicit about data being dropped, like "free-on-exit=yes" or "disposable=yes"). > > Dave > > > Signed-off-by: Eduardo Habkost> > --- > > exec.c | 17 - > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/exec.c b/exec.c > > index 585d6ed6d7..a6e9ed4ece 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -102,6 +102,11 @@ static MemoryRegion io_mem_unassigned; > > */ > > #define RAM_RESIZEABLE (1 << 2) > > > > +/* RAMBlock contents are not persistent, and we can discard memory contents > > + * when freeing the memory
Re: [Qemu-devel] Query on VFIO in Virtual machine
On Thu, 22 Jun 2017 22:42:19 +0530 Nitin Saxenawrote: > Thanks Alex. > > >> Without an iommu in the VM, you'd be limited to no-iommu support for VM > >> userspace, > So are you trying to say VFIO NO-IOMMU should work inside VM. Does > that mean VFIO NO-IOMMU in VM and VFIO IOMMU in host for same device > is a legitimate configuration? I did tried this configuration and the > application (in VM) seems to get container_fd, group_fd, device_fd > successfully but after VFIO_DEVICE_RESET ioctl the PCI link breaks > from VM as well as from host. This could be specific to PCI endpoint > device which I can dig. > > I will be happy if VFIO NO-IOMMU in VM and IOMMU in host for same > device is legitimate configuration. Using no-iommu in the guest should work in that configuration, however there's no isolation from the user to the rest of VM memory, so the VM kernel will be tainted. Host memory does have iommu isolation. Device reset from VM userspace sounds like another bug to investigate. Thanks, Alex
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On 22 June 2017 at 18:03, Juan Quintelawrote: > > Greg Kurz wrote: > >> On Thu, 22 Jun 2017 17:14:08 +0100 > >> Peter Maydell wrote: > >> > >>> On 22 June 2017 at 17:06, Greg Kurz wrote: > >>> > Function types cannot reside in the same sorted list as opaque types > >>> > since > >>> > they may depend on a type which would be defined later. > >>> > > >>> > Of course, the same problem could arise if a function type depends on > >>> > another function type with greater alphabetical order. Hopefully we > >>> > don't have that at this time. > >>> > >>> The other approach would be to put function types somewhere > >>> else and leave typedefs.h for the simple 'opaque types > >>> for structures' that it was started as. > >>> > >>> For instance we have include/qemu/fprintf-fn.h as a precedent. > >>> > >> > >> Indeed, and I'm not quite sure why Juan decided to put these types into > >> typedefs.h instead of a dedicated header file in include/migration... is > >> it only because it was the quickest fix ? > > > > All other typedefs were defined there. I can create a different include > > file, but I think that is "overengineering", no? They are typedefs, > > just not of structs. But I agree that they are the only ones. > > Well, the comment in the file says "opaque types so that device init > declarations don't have to pull in all the real definitions", whereas > the ones you've added aren't opaque types, they are the real > definitions. They're also only used by a very small subset of .c > files, whereas typedefs.h goes everywhere. mv fprintf-fn.f fn-typedefs.h move those two defs into that? Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On 22 June 2017 at 18:03, Juan Quintelawrote: > Greg Kurz wrote: >> On Thu, 22 Jun 2017 17:14:08 +0100 >> Peter Maydell wrote: >> >>> On 22 June 2017 at 17:06, Greg Kurz wrote: >>> > Function types cannot reside in the same sorted list as opaque types since >>> > they may depend on a type which would be defined later. >>> > >>> > Of course, the same problem could arise if a function type depends on >>> > another function type with greater alphabetical order. Hopefully we >>> > don't have that at this time. >>> >>> The other approach would be to put function types somewhere >>> else and leave typedefs.h for the simple 'opaque types >>> for structures' that it was started as. >>> >>> For instance we have include/qemu/fprintf-fn.h as a precedent. >>> >> >> Indeed, and I'm not quite sure why Juan decided to put these types into >> typedefs.h instead of a dedicated header file in include/migration... is >> it only because it was the quickest fix ? > > All other typedefs were defined there. I can create a different include > file, but I think that is "overengineering", no? They are typedefs, > just not of structs. But I agree that they are the only ones. Well, the comment in the file says "opaque types so that device init declarations don't have to pull in all the real definitions", whereas the ones you've added aren't opaque types, they are the real definitions. They're also only used by a very small subset of .c files, whereas typedefs.h goes everywhere. thanks -- PMM
Re: [Qemu-devel] Query on VFIO in Virtual machine
Thanks Alex. >> Without an iommu in the VM, you'd be limited to no-iommu support for VM >> userspace, So are you trying to say VFIO NO-IOMMU should work inside VM. Does that mean VFIO NO-IOMMU in VM and VFIO IOMMU in host for same device is a legitimate configuration? I did tried this configuration and the application (in VM) seems to get container_fd, group_fd, device_fd successfully but after VFIO_DEVICE_RESET ioctl the PCI link breaks from VM as well as from host. This could be specific to PCI endpoint device which I can dig. I will be happy if VFIO NO-IOMMU in VM and IOMMU in host for same device is legitimate configuration. Thanks Nitin On Thu, Jun 22, 2017 at 10:29 PM, Alex Williamsonwrote: > [cc +qemu-devel, +peterx] > > On Thu, 22 Jun 2017 22:18:06 +0530 > Nitin Saxena wrote: > >> Hi, >> >> I have a PCI device connected as an endpoint to Intel host machine. >> The requirement is to run dpdk like user space data path application >> in VM using PCI PF passthrough (SRIOV disabled). This application >> works fine on host kernel and uses VFIO to get MSIX interrupts from >> PCI device. We are trying to run this existing application in VM using >> PCI passthrough. This application has capability to use >> VFIO_IOMMU_TYPE1 as wells as VFIO_NOIOMMU. >> >> On Intel host machine VT-d has been enabled and using virt-manager PCI >> device PF is assigned to the VM. This makes virt-manager to implicitly >> binds PCI device PF to vfio with vfio_iommu_type1. The VM LINUX kernel >> was booted with intel_iommu=on as boot parameter. >> >> My question: Is it possible that vfio can coexist in host (by >> virt-manager) as well as VM (by application)? If yes, does application >> running inside VM needs to configure VFIO with iommu_type=IOMMU or >> iommu_type=no-iommu. >> >> In VM I tried inserting vfio_iommu_type1.ko kernel module which failed >> with "No such device error". Thats why I am confused whether my >> requirement is legitimate or not. What could be the best solution? > > This is really more of a QEMU question. In order to use > vfio_iommu_type1 in the guest, you need an iommu in the guest. The > most recent release of QEMU supports this with an emulated VT-d > device. Therefore if you create a VM with emulated VT-d and a device > assigned through vfio-pci, you can expose it to userspace in the VM with > physical iommu protection. Without an iommu in the VM, you'd be > limited to no-iommu support for VM userspace, the physical iommu would > only protect the device to the extent of VM memory, no to specific > userspace mappings within the VM. Thanks, > > Alex
Re: [Qemu-devel] [RFC PATCH 1/3] vmstate: error hint for failed equal checks
* Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > > > On 06/22/2017 10:22 AM, Dr. David Alan Gilbert wrote: > > * Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > >> > >> > >> On 06/08/2017 01:05 PM, Halil Pasic wrote: > >>> > >>> > >>> On 06/07/2017 11:51 AM, Dr. David Alan Gilbert wrote: > * Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > > In some cases a failing VMSTATE_*_EQUAL does not mean we detected a bug > > (it's actually the best we can do). Especially in these cases a verbose > > error message is required. > > > > Let's introduce infrastructure for specifying a error hint to be used if > > equal check fails. > > > > Signed-off-by: Halil Pasic> > --- > > Macros come in part 2. Once we are happy with the macros > > this two patches should be squashed into one. > > --- > > include/migration/vmstate.h | 1 + > > migration/vmstate-types.c | 36 +++- > > 2 files changed, 32 insertions(+), 5 deletions(-) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 66895623da..d90d9b12ca 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -200,6 +200,7 @@ typedef enum { > > > > struct VMStateField { > > const char *name; > > +const char *err_hint; > > size_t offset; > > size_t size; > > size_t start; > > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > > index 7287c6baa6..84d0545a38 100644 > > --- a/migration/vmstate-types.c > > +++ b/migration/vmstate-types.c > > @@ -19,6 +19,7 @@ > > #include "qemu/error-report.h" > > #include "qemu/queue.h" > > #include "trace.h" > > +#include "qapi/error.h" > > > > /* bool */ > > > > @@ -118,6 +119,7 @@ const VMStateInfo vmstate_info_int32 = { > > static int get_int32_equal(QEMUFile *f, void *pv, size_t size, > > VMStateField *field) > > { > > +Error *err = NULL; > > int32_t *v = pv; > > int32_t v2; > > qemu_get_sbe32s(f, ); > > @@ -125,7 +127,11 @@ static int get_int32_equal(QEMUFile *f, void *pv, > > size_t size, > > if (*v == v2) { > > return 0; > > } > > -error_report("%" PRIx32 " != %" PRIx32, *v, v2); > > +error_setg(, "%" PRIx32 " != %" PRIx32, *v, v2); > > +if (field->err_hint) { > > +error_append_hint(, "%s\n", field->err_hint); > > +} > > +error_report_err(err); > > I'm a bit worried as to whether the error_append_hint data gets > printed out by error_report_err if we're being driven by a QMP > monitor. > error_report_err uses error_printf_unless_qmp > > Since this code doesn't really handle Error *'s back up, > and always prints it's errors into stderr, I'd prefer if you just > used error_report again for the hint, something like: > > if (field->err_hint) { > error_report("%" PRIx32 " != %" PRIx32 "(%s)", > *v, v2, field->err_hint); > } else { > error_report("%" PRIx32 " != %" PRIx32, *v, v2); > } > > Dave > >>> > >>> One reason I choose error_report_err is to be consistent about hint > >>> reporting (the other one is that was what Connie suggested). I do > >>> not understand why do we omit hints if QMP, but I figured that's > >>> our policy. So the hint I'm adding must not be printed in QMP > >>> context -- because that's our policy. I was pretty sure what I > >>> want to do is add a hint (and not make a very long 'core' error > >>> message). > >>> > >>> Can you (or somebody else) explain why are hints dropped in QMP > >>> context? > >>> > >>> Don't misunderstand I'm open towards your proposal, it's just > >>> that: > >>> 1) I would like to understand. > >>> 2) I would like to get the very same result as produced by > >>> https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg01472.html > >>> > >>> Regards, > >>> Halil > >>> > >>> > >> > >> ping. > >> > >> I would like to do a v2, but I want this sorted out first. > >> > >> 'This' basically boils down to the question and > >> 'Why aren't hints reported in QMP context?' and 'Why is this > >> case special (a hint should be reported > >> even in QMP context?' > >> > >> Regarding the first question hints being reported via > >> error_printf_unless_qmp seems to come from commit > >> 50b7b000c9 ("hmp: Allow for error message hints on HMP") > >> --> Cc-ing Eric maybe he can help. > > > > I don't understand the full logic behind error_append_hint; > > my only concern here is that the full text ends up on stderr > > even if the migration is driven by QMP. > > Since we can do that just by using error_report like it's already > > being used with the slight change I suggested, it
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
Greg Kurzwrote: > On Thu, 22 Jun 2017 17:14:08 +0100 > Peter Maydell wrote: > >> On 22 June 2017 at 17:06, Greg Kurz wrote: >> > Function types cannot reside in the same sorted list as opaque types since >> > they may depend on a type which would be defined later. >> > >> > Of course, the same problem could arise if a function type depends on >> > another function type with greater alphabetical order. Hopefully we >> > don't have that at this time. >> >> The other approach would be to put function types somewhere >> else and leave typedefs.h for the simple 'opaque types >> for structures' that it was started as. >> >> For instance we have include/qemu/fprintf-fn.h as a precedent. >> > > Indeed, and I'm not quite sure why Juan decided to put these types into > typedefs.h instead of a dedicated header file in include/migration... is > it only because it was the quickest fix ? All other typedefs were defined there. I can create a different include file, but I think that is "overengineering", no? They are typedefs, just not of structs. But I agree that they are the only ones. Later, Juan.
Re: [Qemu-devel] Query on VFIO in Virtual machine
[cc +qemu-devel, +peterx] On Thu, 22 Jun 2017 22:18:06 +0530 Nitin Saxenawrote: > Hi, > > I have a PCI device connected as an endpoint to Intel host machine. > The requirement is to run dpdk like user space data path application > in VM using PCI PF passthrough (SRIOV disabled). This application > works fine on host kernel and uses VFIO to get MSIX interrupts from > PCI device. We are trying to run this existing application in VM using > PCI passthrough. This application has capability to use > VFIO_IOMMU_TYPE1 as wells as VFIO_NOIOMMU. > > On Intel host machine VT-d has been enabled and using virt-manager PCI > device PF is assigned to the VM. This makes virt-manager to implicitly > binds PCI device PF to vfio with vfio_iommu_type1. The VM LINUX kernel > was booted with intel_iommu=on as boot parameter. > > My question: Is it possible that vfio can coexist in host (by > virt-manager) as well as VM (by application)? If yes, does application > running inside VM needs to configure VFIO with iommu_type=IOMMU or > iommu_type=no-iommu. > > In VM I tried inserting vfio_iommu_type1.ko kernel module which failed > with "No such device error". Thats why I am confused whether my > requirement is legitimate or not. What could be the best solution? This is really more of a QEMU question. In order to use vfio_iommu_type1 in the guest, you need an iommu in the guest. The most recent release of QEMU supports this with an emulated VT-d device. Therefore if you create a VM with emulated VT-d and a device assigned through vfio-pci, you can expose it to userspace in the VM with physical iommu protection. Without an iommu in the VM, you'd be limited to no-iommu support for VM userspace, the physical iommu would only protect the device to the extent of VM memory, no to specific userspace mappings within the VM. Thanks, Alex
Re: [Qemu-devel] [PATCH v2 00/31] qed: Convert to coroutines
Am 16.06.2017 um 19:36 hat Kevin Wolf geschrieben: > The qed block driver is one of the last remaining block drivers that use the > AIO callback style interfaces. This series converts it to the coroutine model > that other drivers are using and removes some AIO functions from the block > layer API afterwards. > > If this isn't compelling enough, the diffstat should speak for itself. > > This series is relatively long, but it consists mostly of mechanical > conversions of one function per patch, so it should be easy to review. > > v2: > - Add coroutine_fn markers [Stefan, Paolo] > - Use bdrv_co_*() instead of bdrv_*() in coroutine_fns > - Use ACB on stack in qed_co_request [Paolo] > - Updated some comments [Paolo] > - Unplug earlier in qed_clear_need_check() [Stefan] > - Removed now unused trace events [Stefan] > - Improved commit message of patch creating qed_aio_write_cow() [Eric] Applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On Thu, 22 Jun 2017 17:14:08 +0100 Peter Maydellwrote: > On 22 June 2017 at 17:06, Greg Kurz wrote: > > Function types cannot reside in the same sorted list as opaque types since > > they may depend on a type which would be defined later. > > > > Of course, the same problem could arise if a function type depends on > > another function type with greater alphabetical order. Hopefully we > > don't have that at this time. > > The other approach would be to put function types somewhere > else and leave typedefs.h for the simple 'opaque types > for structures' that it was started as. > > For instance we have include/qemu/fprintf-fn.h as a precedent. > Indeed, and I'm not quite sure why Juan decided to put these types into typedefs.h instead of a dedicated header file in include/migration... is it only because it was the quickest fix ? > thanks > -- PMM pgpIOhJFapbvk.pgp Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 1/4] qemu-img: add --shrink flag for resize
On 22.06.2017 17:49, Kevin Wolf wrote: Am 22.06.2017 um 15:54 hat Pavel Butsykin geschrieben: On 22.06.2017 01:17, Max Reitz wrote: On 2017-06-13 14:16, Pavel Butsykin wrote: The flag as additional precaution of data loss. Perhaps in the future the operation shrink without this flag will be banned, but while we need to maintain compatibility. Signed-off-by: Pavel ButsykinThe functional changes look good to me; even though I'd rather have it an error for qcow2 now (even if that means having to check the image format in img_resize(), and being inconsistent because you wouldn't need --shrink for raw, but for qcow2 you would). But, well, I'm not going to stop this series over that. Why shrink for qcow2 image is dangerous, but for raw is not dangerous? I think we should provide the same behavior for all formats. When --shrink option will become necessary, it also should be the same for all image formats. It is dangerous for both, but for raw we can't enforce the flag immediately without a deprecation period. With qcow2 we can (because it is new functionality), so we might as well enforce it from the start. Ah, exactly. I like the offer to print the warning for raw and enforce the flag for other formats. Kevin
Re: [Qemu-devel] [PATCH v3] ivshmem-server: ivshmem-client: Build when eventfd() is available
On Thu, Jun 22, 2017 at 12:54 PM, Peter Maydellwrote: > On 5 June 2017 at 15:29, Michael Tokarev wrote: [...] >> $ ../configure --disable-system --disable-linux-user --static >> $ make V=1 >> ... > > Why are you trying to build with both system emulation QEMU > and linux-user QEMU disabled anyway? That doesn't leave very > much left to build... Hmm the only point would be to build libqemuutil.a... Should the ./configure script return an error if $target_list is empty?
Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
On Thu, 22 Jun 2017 16:38:45 +0200 Markus Armbrusterwrote: > Pradeep Jagadeesh writes: > > > This patch factor out the duplicate throttle code that was present in > > block and fsdev devices. > > > > Signed-off-by: Pradeep Jagadeesh > > --- > > blockdev.c | 44 +--- > > fsdev/qemu-fsdev-throttle.c | 43 +-- > > fsdev/qemu-fsdev-throttle.h | 1 + > > include/qemu/throttle-options.h | 4 > > util/throttle.c | 50 > > + > > 5 files changed, 57 insertions(+), 85 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 6472548..5db9e5c 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts > > *opts, int *bdrv_flags, > > } > > > > if (throttle_cfg) { > > -throttle_config_init(throttle_cfg); > > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg = > > -qemu_opt_get_number(opts, "throttling.bps-total", 0); > > -throttle_cfg->buckets[THROTTLE_BPS_READ].avg = > > -qemu_opt_get_number(opts, "throttling.bps-read", 0); > > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg = > > -qemu_opt_get_number(opts, "throttling.bps-write", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg = > > -qemu_opt_get_number(opts, "throttling.iops-total", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_READ].avg = > > -qemu_opt_get_number(opts, "throttling.iops-read", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg = > > -qemu_opt_get_number(opts, "throttling.iops-write", 0); > > - > > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max = > > -qemu_opt_get_number(opts, "throttling.bps-total-max", 0); > > -throttle_cfg->buckets[THROTTLE_BPS_READ].max = > > -qemu_opt_get_number(opts, "throttling.bps-read-max", 0); > > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].max = > > -qemu_opt_get_number(opts, "throttling.bps-write-max", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max = > > -qemu_opt_get_number(opts, "throttling.iops-total-max", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_READ].max = > > -qemu_opt_get_number(opts, "throttling.iops-read-max", 0); > > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].max = > > -qemu_opt_get_number(opts, "throttling.iops-write-max", 0); > > - > > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = > > -qemu_opt_get_number(opts, "throttling.bps-total-max-length", > > 1); > > -throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length = > > -qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1); > > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length = > > -qemu_opt_get_number(opts, "throttling.bps-write-max-length", > > 1); > > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = > > -qemu_opt_get_number(opts, "throttling.iops-total-max-length", > > 1); > > -throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length = > > -qemu_opt_get_number(opts, "throttling.iops-read-max-length", > > 1); > > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length = > > -qemu_opt_get_number(opts, "throttling.iops-write-max-length", > > 1); > > - > > -throttle_cfg->op_size = > > -qemu_opt_get_number(opts, "throttling.iops-size", 0); > > - > > +throttle_parse_options(throttle_cfg, opts); > > if (!throttle_is_valid(throttle_cfg, errp)) { > > return; > > } > > diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c > > index 7ae4e86..da9c225 100644 > > --- a/fsdev/qemu-fsdev-throttle.c > > +++ b/fsdev/qemu-fsdev-throttle.c > > @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque) > > > > void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error > > **errp) > > { > > -throttle_config_init(>cfg); > > -fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg = > > -qemu_opt_get_number(opts, "throttling.bps-total", 0); > > -fst->cfg.buckets[THROTTLE_BPS_READ].avg = > > -qemu_opt_get_number(opts, "throttling.bps-read", 0); > > -fst->cfg.buckets[THROTTLE_BPS_WRITE].avg = > > -qemu_opt_get_number(opts, "throttling.bps-write", 0); > > -fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg = > > -qemu_opt_get_number(opts, "throttling.iops-total", 0); > > -fst->cfg.buckets[THROTTLE_OPS_READ].avg = > > -qemu_opt_get_number(opts, "throttling.iops-read", 0); > > -fst->cfg.buckets[THROTTLE_OPS_WRITE].avg = > > -qemu_opt_get_number(opts, "throttling.iops-write", 0); > > - > >
[Qemu-devel] myShare: fino a 30 Giga da condividere tra i tuoi device
Se non visualizzi correttamente questa email clicca:http://email.mailtoshop.com/c/eJwlj7FuxCAQRL8GuiBYYLELCisn_0LKCMP6TA4b5w5b-fw4ijTF6BWjN8nPcUbLswepnEQA2QFqK5zslBOoFbqhu9mhHw30mhm5hlxafS11F7GufPExzDIgWGMQrSPrlIlA0RAqmFxMfPVgetdbyYtfWtuZHhiMV9okvo4cKaTzb-si7yXHhwiv5YfpMeTE9G0vxACn_55oP2V88Kf_pvV4S3RSuZy2ut23Q9TnnTf_kbf0ef1A5X4B7BRBvQ;>qui. Scopri le migliori promozioni del giorno, aggiungi d...@mailtoshop.com alla tua rubrica. http://email.mailtoshop.com/c/eJw9j8tqwzAURL_G2kVI91qvhRYGkw8ogSyL66vEAj1SR4n7-VU3hWGYsxlmyN_Wm1YsehDSCA0gLGhU3AgrDdcotZnsrCZ3HsHhMIq8xNTqc6sPvtbMNo8jBVhRBnK4SvwyQIEEKG1RWTKCZQ-jM04JlvzW2mPAaYBz13Ec_EBe93uHy0e3n63lJHuYL_M_ntq-lGdssZYlcWrEdv8d8utE4R1Sn1RquZfXXw9r_hoLffYbWppfgqBBYg;> http://email.mailtoshop.com/c/eJwVjk0KwyAUhE8TdxV9Rs1buAiUXKHLkviTBKKmqWl6_FoYGAa-YcaZYIOSZDXAuGYKgHWghKSadVxTJbjSfXeXPQ4toGhaFsd1K_m95J3aHMliQEox1S6EKVg7sg69G6XyAb0OvJUkGmhRo2RkM0speyP6Boaq67roJWg-5ho4Ilb7LiVu5DAvH8-b8x-_1c2U05zOP0iKeazJPetPxfUPXT823Q;> WIND http://email.mailtoshop.com/c/eJwVT0FuxCAQe024FcEQhnDgEKnaL_RYsQwhVAHSbLJSf18iWbblg2WTW8KCmmUHQhqBAGICVJobMUnDUUk08_SpZ_sYwaphFMXn7Wyvte08tMJW54XF6J9eAyEI_QzWS9LKKqVpXCZkxcFojdWCbW49z31Q8wCPjlBe3NM7HuHv7uoJxZu_cqUu96IuKV-ptm5ySZ3X6Cke_GdP7HC_sVwfFN9x68tqq6levB2Jne7u-O5vUJp_UyBDKA; width="550" border="0" alt="Wind Tre Business" title="Wind Tre Business" style="display:block"> http://email.mailtoshop.com/c/eJwlj7FuxCAQRL8GuiBYYLELCisn_0LKCMP6TA4b5w5b-fw4ijTF6BWjN8nPcUbLswepnEQA2QFqK5zslBOoFbqhu9mhHw30mhm5hlxafS11F7GufPExzDIgWGMQrSPrlIlA0RAqmFxMfPVgetdbyYtfWtuZHhiMV9okvo4cKaTzb-si7yXHhwiv5YfpMeTE9G0vxACn_55oP2V88Kf_pvV4S3RSuZy2ut23Q9TnnTf_kbf0ef1A5X4B7BRBvQ;> http://email.mailtoshop.com/c/eJwVT80KwyAYe5p6m_j5Ww8eCmOvsOOw6qyjate1hb79LIQk5BASb97uLQVKhhJQRFJKeiqZwIr0oLBkINXQ38WgH5xq1nGSbZq3-pvqgl3NaDKB9QKEkHbUDjRYBsJz7yxnI4w0cJQN5VppQdBspm1bOjZ09NHg8g9bf4TVnVdXS3y4-JmKb3ItahLTHkttJuXYeKz-xJ8lotV8Q95vPhxhbrtKLbHsuK4RbeZqeLUvEtQfu09CYQ; width="550" border="0" alt="Per un business da numeri 1 ci vuole l'offerta numero 1 per condividere i GIGA - myShare fino a 30 GIGA da condividere tra i tuoi device - Per il massimo del risparmio e della flessibilit - Aggiungi il nuovo Samsung Galaxy S8 a 15 in pi al mese" title="Per un business da numeri 1 ci vuole l'offerta numero 1 per condividere i GIGA - myShare fino a 30 GIGA da condividere tra i tuoi device - Per il massimo del risparmio e della flessibilit - Aggiungi il nuovo Samsung Galaxy S8 a 15 in pi al mese" style="display:block"> http://email.mailtoshop.com/c/eJwlj7FuxCAQRL8GuiBYYLELCisn_0LKCMP6TA4b5w5b-fw4ijTF6BWjN8nPcUbLswepnEQA2QFqK5zslBOoFbqhu9mhHw30mhm5hlxafS11F7GufPExzDIgWGMQrSPrlIlA0RAqmFxMfPVgetdbyYtfWtuZHhiMV9okvo4cKaTzb-si7yXHhwiv5YfpMeTE9G0vxACn_55oP2V88Kf_pvV4S3RSuZy2ut23Q9TnnTf_kbf0ef1A5X4B7BRBvQ;>http://email.mailtoshop.com/c/eJwVj82KxCAQhJ8m3la0jW08eAgM8wp7XFz_kiVqNmMG9u1XoamvaYqi2pvoIkqyG2BcMQRgC6CQVLGFK4qCo1qXh1z1cwYtppllux-tvrZ6Ulcz2QxGiMyBVdp_R4GchQGMAsSMMmqSDcxaacnIYbbWzkmsEzz7uPyi1r_D5f5GVr_4MPRzL75jNOpI-51K7cue07DYZunPmchlfkO-P3x4h6P3KrWkctN6JdLMSPjqvyBX_83oQmQ; width="550" border="0" alt="Scopri subito" title="Scopri subito" style="display:block"> http://email.mailtoshop.com/c/eJwVj0uKwzAQRE9j7UZIbeu30MIQcoUsB0c_a7Akx5EDc_u0oamq7sWj2tvoohQkW2BcMQnANMhRUMU0V1SOXKpZ38Rs7hOYcZhYWfLW23ttO3WtkNUuBphh01MLA3qJJkgJDleujY6IIcXCZJQRjGx27X0fxnmAO44rb7r4Tzjc_8XCiw-XPnL1aFcjtJTPVBuGXBJqbK2Hg_7tiRz2Fcr548MnbNistprqSduRSLcX4xe_kVx9AftEQq8; width="550" border="0" alt="La scelta numero 1 per le aziende" title="La scelta numero 1 per le aziende" style="display:block"> Promozioni valide fino ad esaurimento scorte e per le sole nuove attivazioni in MNP, entro il 16 luglio 2017. MYSHARE prevede che il traffico internet utilizzato dalle SIM sia fornito esclusivamente dai pacchetti MYSHARE con un costo mensile di 40€ per MYSHARE 30 GIGA. Contributo di attivazione di 80 euro scontato del 100% per i clienti che non recedono prima dei 24 mesi dall'attivazione. SMARTPHONE L'offerta prevede una rata iniziale di importo pari a 50 euro per Samsung Galaxy S8 o di 100 euro per Sansung Galaxy S8+; con una rata mensile pari a 15 euro. Durata contrattuale 24 mesi. Prezzi IVA esclusa. Per maggiori informazioni visita http://email.mailtoshop.com/c/eJwNjksOgyAUAE8ju5LH7wELFiaNV-iyqQpKomAFy_VrMstJZmYXpoCKRMeBaUDOwXAUimowTFMUDHVvnqq3g-RWdBL2T9xqLms-6JR3sjqmDUiDSiphRBglA64sMIGzNypYJLvj0mqrgGxurfXoRN_x4aa1RltMcz39eJWYfCk0VnK6r9-vx-x_fruDKaclXTSfC6nudevvexKZ_gN05zZI;>http://email.mailtoshop.com/c/eJwNjksOwiAUAE8jOwmf8nkLFk2MV3BpsDwqSQtaqFxftpNJZoKLS9SKJCcYN0wLwazQUlHDLDdUS67NbG9qhvskQF4mtvu0tVLf5UOXspO3A0SJEI0F7zlXUXgZUAcGAyglJNmdmMCAYmRzvXfaUw7twNdZU8ZaaWrkcF_cz2vAH26jkUte80nLsZLmHkN_ji_NzR-QFDQJ
Re: [Qemu-devel] [dpdk-dev] Will huge page have negative effect on guest vm in qemu enviroment?
On Wed, Jun 21, 2017 at 8:22 AM, Samwrote: > Thank you~ > > 1. We have a compare test on qemu-kvm enviroment with huge page and without > huge page. Qemu start process is much longer in huge page enviromwnt. And I > write an email titled with '[DPDK-memory] how qemu waste such long time > under dpdk huge page envriment?'. I could resend it later. > > DPDK vhost code does some work when configuring the VM virtio port. If dequeue_zero_copy is set, the work to do is heavier, but I do not think this could imply huge longer bootup times. > 2. Then I have another test on qemu-kvm enviroment with huge page and > without huge page, which I didn't start ovs-dpdk and vhostuser port in qemu > start process. And I found Qemu start process is also much longer in huge > page enviroment. > If hugepages are available, starting a qemu VM with hugepages should not take too long. So in this case, I would say the hugepages need to be allocated before the VM can boot. The only reason I can think of is transparent hugepages in use, then the system needs to work for undoing those transparent hugepages for allocating space to the VM hugepages. Did you check how many hugepages are available just before starting the QEMU VM? cat /sys/kernel/mm/hugepages/hugepages-*/free_hugepages If this is the problem, easy to solve just disabling transparent hugepages: echo never > /sys/kernel/mm/transparent_hugepage/enabled > So I think huge page enviroment, which grub2.cfg file is specified in > ‘[DPDK-memory] > how qemu waste such long time under dpdk huge page envriment?’, will really > have negative effect on qemu start up process. > > That's why we don't like to use ovs-dpdk. Althrough ovs-dpdk is faster, but > the start up process of qemu is much longer then normal ovs, and the reason > is nothing with ovs but huge page. For customers, vm start up time is > important then network speed. > > BTW, ovs-dpdk start up process is also longer then normal ovs. But I know > the reason, it's dpdk EAL init process with forking big continous memory > and zero this memory. For qemu, I don't know why, as there is no log to > report this. > We had also problems with DPDK apps initialization with a server using 256GB. I guess this is something that maybe could be improved. > > 2017-06-21 14:15 GMT+08:00 Pavel Shirshov : > > > Hi Sam, > > > > Below I'm saying about KVM. I don't have experience with vbox and others. > > 1. I'd suggest don't use dpdk inside of VM if you want to see best > > perfomance on the box. > > 2. huge pages enabled globally will not have any bad effect to guest > > OS. Except you have to enable huge pages inside of VM and provide real > > huge page for VM's huge pages from the host system. Otherwise dpdk > > will use "hugepages" inside of VM, but this "huge pages" will not real > > ones. They will be constructed from normal pages outside. Also when > > you enable huge pages OS will reserve them from start and your OS will > > not able use them for other things. Also you can't swap out huge > > pages, KSM will not work for them and so on. > > 3. You can enable huge pages just for one numa node. It's impossible > > to enable them just for one core. Usually you reserve some memory for > > hugepages when the system starts and you can't use this memory in > > normal applications unless normal application knows how to use them. > > > > Also why it didn't work inside of the docker? > > > > > > On Tue, Jun 20, 2017 at 8:35 PM, Sam wrote: > > > BTW, we also think about use ovs-dpdk in docker enviroment, but test > > result > > > said it's not good idea, we don't know why. > > > > > > 2017-06-21 11:32 GMT+08:00 Sam : > > > > > >> Hi all, > > >> > > >> We plan to use DPDK on HP host machine with several core and big > memory. > > >> We plan to use qemu-kvm enviroment. The host will carry 4 or more > guest > > vm > > >> and 1 ovs. > > >> > > >> Ovs-dpdk is much faster then normal ovs, but to use ovs-dpdk, we have > to > > >> enable huge page globally. > > >> > > >> My question is, will huge page enabled globally have negative effect > on > > >> guest vm's memory orperate or something? If it is, how to prevent > this, > > or > > >> could I enable huge page on some core or enable huge page for a part > of > > >> memory? > > >> > > >> Thank you~ > > >> > > >
Re: [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
On 22 June 2017 at 17:06, Greg Kurzwrote: > Function types cannot reside in the same sorted list as opaque types since > they may depend on a type which would be defined later. > > Of course, the same problem could arise if a function type depends on > another function type with greater alphabetical order. Hopefully we > don't have that at this time. The other approach would be to put function types somewhere else and leave typedefs.h for the simple 'opaque types for structures' that it was started as. For instance we have include/qemu/fprintf-fn.h as a precedent. thanks -- PMM
[Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h
Function types cannot reside in the same sorted list as opaque types since they may depend on a type which would be defined later. Of course, the same problem could arise if a function type depends on another function type with greater alphabetical order. Hopefully we don't have that at this time. Signed-off-by: Greg Kurz--- include/qemu/typedefs.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index f745d5faf7fd..cd3e369ae01a 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -1,10 +1,9 @@ #ifndef QEMU_TYPEDEFS_H #define QEMU_TYPEDEFS_H -/* A load of opaque types so that device init declarations don't have to - pull in all the real definitions. */ - -/* Please keep this list in alphabetical order */ +/* First list is for opaque types only, second one for function types. + * Please keep both lists in alphabetical order. + */ typedef struct AdapterInfo AdapterInfo; typedef struct AddressSpace AddressSpace; typedef struct AioContext AioContext; @@ -96,7 +95,8 @@ typedef struct uWireSlave uWireSlave; typedef struct VirtIODevice VirtIODevice; typedef struct Visitor Visitor; typedef struct node_info NodeInfo; + +typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); typedef void SaveStateHandler(QEMUFile *f, void *opaque); -typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); #endif /* QEMU_TYPEDEFS_H */
Re: [Qemu-devel] [PATCH v2 1/3] target/s390x: Indicate and check for local tlb clearing
On 06/22/2017 02:41 AM, David Hildenbrand wrote: Let's allow to enable it for the qemu cpu model and correctly emulate it. Signed-off-by: David Hildenbrand--- target/s390x/cpu_models.c | 1 + target/s390x/mem_helper.c | 2 -- target/s390x/translate.c | 6 +- 3 files changed, 6 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 0/3] Fix qemu-bridge-helper with SUID
On 05/30/2017 10:23 AM, Michal Privoznik wrote: > For more description see patch 3. Long story short, if the bridge helper runs > with SUID, the mechanism we rely on (DAC denying access to ACL files) does not > work. > > Michal Privoznik (3): > qemu-bridge-helper: Reverse return value setting logic > qemu-bridge-helper: Reverse return value setting logic in > parse_acl_file > qemu-bridge-helper: Take ACL file gid into account > > qemu-bridge-helper.c | 79 > > 1 file changed, 42 insertions(+), 37 deletions(-) > ping? Michal
Re: [Qemu-devel] [PATCH v3] ivshmem-server: ivshmem-client: Build when eventfd() is available
On 5 June 2017 at 15:29, Michael Tokarevwrote: > 31.05.2017 15:00, Kamil Rytarowski wrote: >> Currently ivshmem requires eventfd() which is Linux specific. >> Do not and build it unconditionally on every Linux/BSD/Solaris. >> >> This patch indirectly fixes build failure on NetBSD, where these tools >> additionally require -lrt for shm_open(3). In future there should be >> added support for NetBSD and the linking addressed appropriately. > > Unfortunately this breaks static build. > > $ ../configure --disable-system --disable-linux-user --static > $ make V=1 > ... Why are you trying to build with both system emulation QEMU and linux-user QEMU disabled anyway? That doesn't leave very much left to build... thanks -- PMM
Re: [Qemu-devel] [Qemu-ppc] Floating point unit bugs
On Jun 22, 2017, at 3:25 AM, Peter Maydell wrote: On 22 June 2017 at 03:54, G 3wrote: The advantage a test image would have is the user doesn't have to worry about compiling a test using a cross compiler. Everything the user would need to test QEMU is already inside the image file (hopefully). Is the setup you want something like running 'make test' ? I imagine binary files included with QEMU would test the emulated CPU. There would be no need to cross compile anything. I want our tests to be easy to add new tests for, and easy for anybody to recreate the binary files. Certainly providing pre-built binaries would be helpful (faster than rebuilding whole system images), but we must have an automated mechanism for saying "build this image from sources" too, so that if we want to update the test program later it's easy to do that, or if we want to add a second test that's like the first one but slightly modified we can easily do that too. So what you want is a test system that is easy to write. Maybe as easy as writing a python script? What I thought up is a collection of functions that implement assembly language code. Say you want to make a test that runs the fadds instruction. You might run it like this: def my_float_test: inputA = 0.5 inputB = 0.25 result = fadds(inputA, inputB) if result != 0.75: print("Error with fadds calculation") if get_FPSCR() != expect_fpscr_value: print("FPSCR error") Then all we would be left to figure out is how to turn this code into a mach-o or elf binary. Then again this might not be a requirement. What if we had python running on the guest. The above code might work as is. No cross compiling required. Implementing the callable assembly language code from python could be done using a single command-line program. This program would simply receive its arguments and return a value. If this program was called assembly _adapter, it would run like this: % assembly_adapter fadds 0.5 0.25 The return value of this command is where we would see the result. This command-line program would only need to be compiled once and included with QEMU. The format of this program's arguments would be something like this: This input would probably work for most instructions but doesn't necessarily need to be applied to all instructions. Would this kind of system be something you want?
Re: [Qemu-devel] [PULL 00/21] Docker and shippable updates
On Thu, Jun 22, 2017 at 12:13 PM, Alex Bennéewrote: > Now we've been running shippable for a while is it worth turning on the > IRC notifications? What about moving such bot/scripts notifications in another channel like #QEMU-notifications? It is often hard to follow 3 concurrent topics while the travis-ci bot disrupts entering/broadcasting/leaving... I'd also like to see here checkpatch/patchew stripped output, and eventually gcov/gprof reports. I don't think full patchew reports are useful in a mailbox.
Re: [Qemu-devel] [PULL 00/21] Docker and shippable updates
Philippe Mathieu-Daudéwrites: > On 06/22/2017 11:09 AM, Peter Maydell wrote: >> On 21 June 2017 at 15:47, Alex Bennée wrote: > [...]>> We add the following cross-compile targets: >>>- mipsel-softmmu,mipsel-linux-user,mips64el-linux-user >>>- armeb-linux-user >>> >>> While I was rolling I discovered we could also back out a bunch of the >>> emdebian hacks as the newly released stretch handles cross compilers >>> as first class citizens. Unfortunately this also meant I had to drop >>> the powerpc support as that is no longer in Debian stable. >>> >> >> Applied, thanks. > > Yay! Shippable happily working again \o/ > > https://app.shippable.com/github/qemu/qemu/runs/238/summary/console > > Overall Status: Success Now we've been running shippable for a while is it worth turning on the IRC notifications? -- Alex Bennée
Re: [Qemu-devel] [PATCH v2] hmp, qmp: introduce "info memory" and "query-memory" commands
Hi Markus, Thank you for the input. > However, your query-memory looks like it could fail. With the latest version of a patch ( http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg03475.html) "query-memory" can fail only in two cases: a) if in QOM there is an object of type of TYPE_PC_DIMM which has no property of type PC_DIMM_SIZE_PROP, or b) PC_DIMM_SIZE_PROP is not represented as an integer. As far as I understand both (a) and (b) can happen only in case if QOM somehow corrupted which is not a normal case (please correct me if I am wrong). Please also note, this is not the last version of the patch since new functionality was suggested to be included (NUMA information). If patch will be accepted, I think we would need a test case for it since command output differs once memory was hot-added/removed per given NUMA node. When final functionality will be negotiated, I will come up with test scenario and test case itself. Thank you, Vadim On Tue, Jun 20, 2017 at 4:10 PM, Markus Armbrusterwrote: > "Dr. David Alan Gilbert" writes: > > > * Vadim Galitsyn (vadim.galit...@profitbricks.com) wrote: > >> Hi Dave, > >> > >> Thank you for the feedback! > >> > >> > I think you need to use the PRIu64 macros rather than 'lu' for the > types > >> > of the ints there to keep it portable. > >> > >> Agree, patch v3 will include this change. > > > > Thanks. > > > >> > Other than that; please add a test entry to tests/test-hmp.c > >> > and I'm guessing you'll also need a qmp test for it. > >> > >> As far as I can see in tests/test-hmp.c, it's automatically there. > >> The routine test_info_commands() enumerates all the available "info" > >> sub-commands with "info help" and tries to execute them, so it looks > >> like no extra stuff needs to be done here (please correct me if I am > wrong). > > > > Ah yes you're right; I forgot the 'info' commands were automatic. > > > >> Regarding to QMP test, I cannot find any test under tests/ which > >> does similar job as in tests/test-hmp.c. There is neither HMP commands > >> iteration nor command-specific separate tests. Under tests/qapi-schema/ > >> there are set of .json's though, however, again, it looks more like > general > >> tests set (not commands-specific one). > >> > >> It seems that all the HMP related tests do general checks -- targeting > >> HMP "engine" itself. I would say, Avocado (avocado-vt) test suite can > >> be extended with "query-memory" test, and I can certainly provide one. > >> But this topic is for another mainling list. > > > > Yes hmm not sure where to put the qmp test, I just know that Markus does > > like them (I think he's out this week). > > The best time to start requiring tests for new QMP commands is when the > first command is added. We missed that opportunity. The second best > time is right now[*]. > > The QMP equivalent to test-hmp.c's test_info_commands() would be good > enough for query-FOOs that can't fail. We should have that. It's not > fair to ask you to write it. Not least because doing it right involves > introspection, which is going to be a bit hairy. I guess I'll have to > do it myself[**]. > > However, your query-memory looks like it could fail. Failure modes need > to be covered by test cases. Please either explain why it can't > actually fail, or explain why testing the failure isn't practical, or > write a suitable test. A mere sketch hacked into qmp-test.c is > perfectly okay, we can take it from there together. > > > > [*] Or rather last March: > Message-ID: <871sugkrw5@dusky.pond.sub.org> > https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00296.html > > [**] Surprise me! Patches welcome :) >
[Qemu-devel] [Bug 1699824] [NEW] qemu-system-sparc64 -M sun4v aborts on tribblix-sparc-0m16.iso
Public bug reported: qemu-system-sparc64 qemu-2.9.0-3.10.x86_64 on openSUSE Leap 42.3 using 'sun4v' machine aborts with tribblix. With 2048 MB of RAM it takes considerably more time to abort (but the core is always truncated). > qemu-system-sparc64 -m 1024 -cdrom tribblix-sparc-0m16.iso -boot d -nographic > -M sun4v qemu: fatal: Trap 0x0010 while trap level (6) >= MAXTL (6), Error state pc: 0200 npc: 0204 %g0-3: %g4-7: %o0-3: %o4-7: %l0-3: 3ff0 01ff 01fff008 %l4-7: %i0-3: %i4-7: %f00: %f08: %f16: %f24: %f32: %f40: %f48: %f56: pstate: 0014 ccr: 44 (icc: -Z-- xcc: -Z--) asi: 00 tl: 6 pil: 0 gl: 8 tbr: hpstate: 0004 htba: cansave: 6 canrestore: 0 otherwin: 0 wstate: 0 cleanwin: 6 cwp: 7 fsr: y: fprs: Aborted (core dumped) PID: 26999 (qemu-system-spa) UID: 1000 (newman) GID: 100 (users) Signal: 6 (ABRT) Timestamp: Thu 2017-06-22 16:19:02 CEST (1min 5s ago) Command Line: qemu-system-sparc64 -m 1024 -cdrom tribblix-sparc-0m16.iso -boot d -nographic -M sun4v Executable: /usr/bin/qemu-system-sparc64 Control Group: / Slice: -.slice Boot ID: aa7431274f854fb7a02a773eefa8a9bb Machine ID: 89c660865c00403a9bacef32b6828556 Hostname: assam.suse.cz Coredump: /var/lib/systemd/coredump/core.qemu-system-spa.1000.aa7431274f854fb7a02a773eefa8a9bb.26999.149814114200.xz Message: Process 26999 (qemu-system-spa) of user 1000 dumped core. (gdb) thread apply all bt full Thread 4 (Thread 0x7f3896aca700 (LWP 27001)): #0 0x7f38bb983295 in do_futex_wait () at /lib64/libpthread.so.0 #1 0x7f38bb983349 in __new_sem_wait_slow () at /lib64/libpthread.so.0 #2 0x7f38bb9833f7 in sem_timedwait () at /lib64/libpthread.so.0 #3 0x5599ec6a1147 in qemu_sem_timedwait (sem=sem@entry=0x5599ef168628, ms=ms@entry=1) at util/qemu-thread-posix.c:255 rc = ts = {tv_sec = 1498141152, tv_nsec = 280531000} __func__ = "qemu_sem_timedwait" #4 0x5599ec69c83c in worker_thread (opaque=0x5599ef1685c0) at util/thread-pool.c:92 req = ret = pool = 0x5599ef1685c0 #5 0x7f38bb97c744 in start_thread () at /lib64/libpthread.so.0 #6 0x7f38b79bdd3d in clone () at /lib64/libc.so.6 Thread 3 (Thread 0x7f38bee01c40 (LWP 26999)): #0 0x7f38b79b555f in ppoll () at /lib64/libc.so.6 #1 0x5599ec69d289 in ppoll (__ss=0x0, __timeout=0x7ffd1dcf2a20, __nfds=, __fds=) at /usr/include/bits/poll2.h:77 ts = {tv_sec = 1, tv_nsec = 0} Python Exception That operation is not available on integers of more than 8 bytes.: #2 0x5599ec69d289 in qemu_poll_ns (fds=, nfds=, timeout=timeout@entry=10) at util/qemu-timer.c:334 ts = {tv_sec = 1, tv_nsec = 0} Python Exception That operation is not available on integers of more than 8 bytes.: #3 0x5599ec69dff8 in os_host_main_loop_wait (timeout=10) at util/main-loop.c:255 context = 0x5599ef147470 ret = spin_counter = 0 ret = -283872144 timeout = 1000 #4 0x5599ec69dff8 in main_loop_wait (nonblocking=) at util/main-loop.c:517 ret = -283872144 timeout = 1000 #5 0x5599ec3c8c5f in main_loop () at vl.c:1900 i = snapshot = linux_boot = initrd_filename = kernel_filename = kernel_cmdline = boot_order = boot_once = 0x0 ds = cyls = heads = secs = translation = opts = hda_opts = icount_opts = accel_opts = olist = optind = 10 optarg = 0x7ffd1dcf51d2 "sun4v" loadvm = machine_class = 0x5599ec6d6f6f cpu_model = vga_model = 0x5599ec6d6f81 "std" qtest_chrdev
Re: [Qemu-devel] [PULL 00/21] Docker and shippable updates
On 06/22/2017 11:09 AM, Peter Maydell wrote: On 21 June 2017 at 15:47, Alex Bennéewrote: [...]>> We add the following cross-compile targets: - mipsel-softmmu,mipsel-linux-user,mips64el-linux-user - armeb-linux-user While I was rolling I discovered we could also back out a bunch of the emdebian hacks as the newly released stretch handles cross compilers as first class citizens. Unfortunately this also meant I had to drop the powerpc support as that is no longer in Debian stable. Applied, thanks. Yay! Shippable happily working again \o/ https://app.shippable.com/github/qemu/qemu/runs/238/summary/console Overall Status: Success
Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add qobject_is_equal()
Max Reitzwrites: > On 2017-06-21 18:43, Markus Armbruster wrote: >> Max Reitz writes: >> >>> This generic function (along with its implementations for different >>> types) determines whether two QObjects are equal. >>> >>> Signed-off-by: Max Reitz >>> --- >>> include/qapi/qmp/qbool.h | 1 + >>> include/qapi/qmp/qdict.h | 1 + >>> include/qapi/qmp/qfloat.h | 1 + >>> include/qapi/qmp/qint.h| 1 + >>> include/qapi/qmp/qlist.h | 1 + >>> include/qapi/qmp/qnull.h | 2 ++ >>> include/qapi/qmp/qobject.h | 9 + >>> include/qapi/qmp/qstring.h | 1 + >>> qobject/qbool.c| 8 >>> qobject/qdict.c| 28 >>> qobject/qfloat.c | 8 >>> qobject/qint.c | 8 >>> qobject/qlist.c| 30 ++ >>> qobject/qnull.c| 5 + >>> qobject/qobject.c | 30 ++ >>> qobject/qstring.c | 9 + >>> 16 files changed, 143 insertions(+) >> >> No unit test? > > *cough* > >>> diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h >>> index a4c..f77ea86 100644 >>> --- a/include/qapi/qmp/qbool.h >>> +++ b/include/qapi/qmp/qbool.h >>> @@ -24,6 +24,7 @@ typedef struct QBool { >>> QBool *qbool_from_bool(bool value); >>> bool qbool_get_bool(const QBool *qb); >>> QBool *qobject_to_qbool(const QObject *obj); >>> +bool qbool_is_equal(const QObject *x, const QObject *y); >>> void qbool_destroy_obj(QObject *obj); >>> >>> #endif /* QBOOL_H */ >>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h >>> index 188440a..134a526 100644 >>> --- a/include/qapi/qmp/qdict.h >>> +++ b/include/qapi/qmp/qdict.h >>> @@ -41,6 +41,7 @@ void qdict_del(QDict *qdict, const char *key); >>> int qdict_haskey(const QDict *qdict, const char *key); >>> QObject *qdict_get(const QDict *qdict, const char *key); >>> QDict *qobject_to_qdict(const QObject *obj); >>> +bool qdict_is_equal(const QObject *x, const QObject *y); >>> void qdict_iter(const QDict *qdict, >>> void (*iter)(const char *key, QObject *obj, void *opaque), >>> void *opaque); >>> diff --git a/include/qapi/qmp/qfloat.h b/include/qapi/qmp/qfloat.h >>> index b5d1583..318e91e 100644 >>> --- a/include/qapi/qmp/qfloat.h >>> +++ b/include/qapi/qmp/qfloat.h >>> @@ -24,6 +24,7 @@ typedef struct QFloat { >>> QFloat *qfloat_from_double(double value); >>> double qfloat_get_double(const QFloat *qi); >>> QFloat *qobject_to_qfloat(const QObject *obj); >>> +bool qfloat_is_equal(const QObject *x, const QObject *y); >>> void qfloat_destroy_obj(QObject *obj); >>> >>> #endif /* QFLOAT_H */ >>> diff --git a/include/qapi/qmp/qint.h b/include/qapi/qmp/qint.h >>> index 3aaff76..20975da 100644 >>> --- a/include/qapi/qmp/qint.h >>> +++ b/include/qapi/qmp/qint.h >>> @@ -23,6 +23,7 @@ typedef struct QInt { >>> QInt *qint_from_int(int64_t value); >>> int64_t qint_get_int(const QInt *qi); >>> QInt *qobject_to_qint(const QObject *obj); >>> +bool qint_is_equal(const QObject *x, const QObject *y); >>> void qint_destroy_obj(QObject *obj); >>> >>> #endif /* QINT_H */ >>> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h >>> index 5dc4ed9..4380a5b 100644 >>> --- a/include/qapi/qmp/qlist.h >>> +++ b/include/qapi/qmp/qlist.h >>> @@ -57,6 +57,7 @@ QObject *qlist_peek(QList *qlist); >>> int qlist_empty(const QList *qlist); >>> size_t qlist_size(const QList *qlist); >>> QList *qobject_to_qlist(const QObject *obj); >>> +bool qlist_is_equal(const QObject *x, const QObject *y); >>> void qlist_destroy_obj(QObject *obj); >>> >>> static inline const QListEntry *qlist_first(const QList *qlist) >>> diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h >>> index 69555ac..9299683 100644 >>> --- a/include/qapi/qmp/qnull.h >>> +++ b/include/qapi/qmp/qnull.h >>> @@ -23,4 +23,6 @@ static inline QObject *qnull(void) >>> return _; >>> } >>> >>> +bool qnull_is_equal(const QObject *x, const QObject *y); >>> + >>> #endif /* QNULL_H */ >>> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h >>> index ef1d1a9..cac72e3 100644 >>> --- a/include/qapi/qmp/qobject.h >>> +++ b/include/qapi/qmp/qobject.h >>> @@ -68,6 +68,15 @@ static inline void qobject_incref(QObject *obj) >>> } >>> >>> /** >>> + * qobject_is_equal(): Returns whether the two objects are equal. >> >> Imperative mood, please: Return whether... > > OK, will do here and everywhere else. > >>> + * >>> + * Any of the pointers may be NULL; will return true if both are. Will >>> always >>> + * return false if only one is (therefore a QNull object is not considered >>> equal >>> + * to a NULL pointer). >>> + */ >> >> Humor me: wrap comment lines around column 70, and put two spaces >> between sentences. > > Do I hear "one checkpatch.pl per subsystem"? Nah, you hear "would you mind
Re: [Qemu-devel] [PATCH v2 3/4] block: qobject_is_equal() in bdrv_reopen_prepare()
Max Reitzwrites: > On 2017-06-21 18:06, Markus Armbruster wrote: >> Max Reitz writes: >> >>> Currently, bdrv_reopen_prepare() assumes that all BDS options are >>> strings. However, this is not the case if the BDS has been created >>> through the json: pseudo-protocol or blockdev-add. >>> >>> Note that the user-invokable reopen command is an HMP command, so you >>> can only specify strings there. Therefore, specifying a non-string >>> option with the "same" value as it was when originally created will now >>> return an error because the values are supposedly similar (and there is >>> no way for the user to circumvent this but to just not specify the >>> option again -- however, this is still strictly better than just >>> crashing). >>> >>> Reviewed-by: Kevin Wolf >>> Signed-off-by: Max Reitz >>> --- >>> block.c | 15 +++ >>> 1 file changed, 3 insertions(+), 12 deletions(-) >>> >>> diff --git a/block.c b/block.c >>> index fa1d06d..23424d5 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -2950,19 +2950,10 @@ int bdrv_reopen_prepare(BDRVReopenState >>> *reopen_state, BlockReopenQueue *queue, >>> const QDictEntry *entry = qdict_first(reopen_state->options); >>> >>> do { >>> -QString *new_obj = qobject_to_qstring(entry->value); >>> -const char *new = qstring_get_str(new_obj); >>> -/* >>> - * Caution: while qdict_get_try_str() is fine, getting >>> - * non-string types would require more care. When >>> - * bs->options come from -blockdev or blockdev_add, its >>> - * members are typed according to the QAPI schema, but >>> - * when they come from -drive, they're all QString. >>> - */ >> >> Your commit message makes me suspect this comment still applies in some >> form. Does it? > > The only thing that I can think of that may be applicable is what I > wrote in the commit message; this fails now: > > $ qemu-io -c 'reopen -o size=65536' \ > "json:{'driver':'null-co','size':65536}" > Cannot change the option 'size' > > As I wrote in the commit message, though, I don't think this is too bad. > First, before, it just crashed, so this definitely is better behavior. > > Secondly, I think this is just good for convenience; it allows the user > to specify an option (to "change" it) even if the block driver doesn't > support changing it. If it actually has not change, this is supposed to > be handled gracefully. But sometimes we cannot easily handle it, so... > We just give an error. > > I suspect that at some point we want to fix this by having everything > correctly typed internally...? Until then, in my opinion, we can just > not provide this feature. Correcting the types in a QDict to match the schema is no easier than converting it to a QAPI generated C type. In my opinion we should work towards the latter (QAPIfy stuff) instead of the former (dig us ever deeper into the QObject hole). > However, I should have probably not just deleted the comment and hoped > everyone can find the necessary information through git-blame. I should > leave a comment about this here. Yes, please! > Or we do make an effort to provide this test-unchanged functionality for > every case, in which case we would have to convert either string options > to the correct type (according to the schema) here (but if that were so > simple, we could do that centrally in bdrv_open() already, right?) or The one way we already have to check QObject types against the schema is the QObject input visitor. So, to get a QObject with correct types, you can convert to the QAPI-generated C type with the appropriate variant of the QObject input visitor, then right back with the QObject output visitor. But once we have the QAPI-generated C type, we should simply use *that* instead of dicking around with QDict. In other words: QAPIfy. "Appropriate variant" means you need to choose between the variant that expects correct scalar types (appropriate when the QObject comes from JSON) and the keyval variant, which expects strings (appropriate when the QObject comes from keyval_parse()). > convert typed options to strings and compare them -- but since there are > probably multiple different strings that can mean the same value for > whatever type the option has, this won't work in every case either. Converting to string throws away information. Not sure that works. > So I'm for leaving a comment and leaving this not quite as it should be > until we have fixed all of the block layer (O:-)). Maybe you have a > better idea though, which would be great. Let's go with a comment.
Re: [Qemu-devel] [PATCH v2 1/4] qemu-img: add --shrink flag for resize
Am 22.06.2017 um 15:54 hat Pavel Butsykin geschrieben: > On 22.06.2017 01:17, Max Reitz wrote: > >On 2017-06-13 14:16, Pavel Butsykin wrote: > >>The flag as additional precaution of data loss. Perhaps in the future the > >>operation shrink without this flag will be banned, but while we need to > >>maintain compatibility. > >> > >>Signed-off-by: Pavel Butsykin> >The functional changes look good to me; even though I'd rather have it > >an error for qcow2 now (even if that means having to check the image > >format in img_resize(), and being inconsistent because you wouldn't need > >--shrink for raw, but for qcow2 you would). But, well, I'm not going to > >stop this series over that. > > > > Why shrink for qcow2 image is dangerous, but for raw is not dangerous? I > think we should provide the same behavior for all formats. When --shrink > option will become necessary, it also should be the same for all image > formats. It is dangerous for both, but for raw we can't enforce the flag immediately without a deprecation period. With qcow2 we can (because it is new functionality), so we might as well enforce it from the start. Kevin
Re: [Qemu-devel] [PATCH v2 1/4] qapi/qnull: Add own header
Max Reitzwrites: > On 2017-06-21 18:24, Markus Armbruster wrote: >> Max Reitz writes: >> >>> Reviewed-by: Kevin Wolf >>> Signed-off-by: Max Reitz >>> --- >>> include/qapi/qmp/qnull.h | 26 ++ >>> include/qapi/qmp/qobject.h | 8 >>> include/qapi/qmp/types.h | 1 + >>> qobject/qnull.c| 1 + >>> target/i386/cpu.c | 6 +- >>> tests/check-qnull.c| 2 +- >>> 6 files changed, 30 insertions(+), 14 deletions(-) >>> create mode 100644 include/qapi/qmp/qnull.h >>> >>> diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h >>> new file mode 100644 >>> index 000..69555ac >>> --- /dev/null >>> +++ b/include/qapi/qmp/qnull.h >>> @@ -0,0 +1,26 @@ >>> +/* >>> + * QNull Module >>> + * >>> + * Copyright (C) 2009, 2017 Red Hat Inc. >>> + * >>> + * Authors: >>> + * Luiz Capitulino >>> + * >>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or >>> later. >>> + * See the COPYING.LIB file in the top-level directory. >> >> Copy the boilerplate from qnull.c instead, factual correctness. > > Sorry, will do. No need to be sorry! Copying it from qobject.h along with the code was the obvious thing to do. [...]
Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
Pradeep Jagadeeshwrites: > This patch factor out the duplicate throttle code that was present in > block and fsdev devices. > > Signed-off-by: Pradeep Jagadeesh > --- > blockdev.c | 44 +--- > fsdev/qemu-fsdev-throttle.c | 43 +-- > fsdev/qemu-fsdev-throttle.h | 1 + > include/qemu/throttle-options.h | 4 > util/throttle.c | 50 > + > 5 files changed, 57 insertions(+), 85 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 6472548..5db9e5c 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts > *opts, int *bdrv_flags, > } > > if (throttle_cfg) { > -throttle_config_init(throttle_cfg); > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg = > -qemu_opt_get_number(opts, "throttling.bps-total", 0); > -throttle_cfg->buckets[THROTTLE_BPS_READ].avg = > -qemu_opt_get_number(opts, "throttling.bps-read", 0); > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg = > -qemu_opt_get_number(opts, "throttling.bps-write", 0); > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg = > -qemu_opt_get_number(opts, "throttling.iops-total", 0); > -throttle_cfg->buckets[THROTTLE_OPS_READ].avg = > -qemu_opt_get_number(opts, "throttling.iops-read", 0); > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg = > -qemu_opt_get_number(opts, "throttling.iops-write", 0); > - > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max = > -qemu_opt_get_number(opts, "throttling.bps-total-max", 0); > -throttle_cfg->buckets[THROTTLE_BPS_READ].max = > -qemu_opt_get_number(opts, "throttling.bps-read-max", 0); > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].max = > -qemu_opt_get_number(opts, "throttling.bps-write-max", 0); > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max = > -qemu_opt_get_number(opts, "throttling.iops-total-max", 0); > -throttle_cfg->buckets[THROTTLE_OPS_READ].max = > -qemu_opt_get_number(opts, "throttling.iops-read-max", 0); > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].max = > -qemu_opt_get_number(opts, "throttling.iops-write-max", 0); > - > -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = > -qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1); > -throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length = > -qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1); > -throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length = > -qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1); > -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = > -qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1); > -throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length = > -qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1); > -throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length = > -qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1); > - > -throttle_cfg->op_size = > -qemu_opt_get_number(opts, "throttling.iops-size", 0); > - > +throttle_parse_options(throttle_cfg, opts); > if (!throttle_is_valid(throttle_cfg, errp)) { > return; > } > diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c > index 7ae4e86..da9c225 100644 > --- a/fsdev/qemu-fsdev-throttle.c > +++ b/fsdev/qemu-fsdev-throttle.c > @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque) > > void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp) > { > -throttle_config_init(>cfg); > -fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg = > -qemu_opt_get_number(opts, "throttling.bps-total", 0); > -fst->cfg.buckets[THROTTLE_BPS_READ].avg = > -qemu_opt_get_number(opts, "throttling.bps-read", 0); > -fst->cfg.buckets[THROTTLE_BPS_WRITE].avg = > -qemu_opt_get_number(opts, "throttling.bps-write", 0); > -fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg = > -qemu_opt_get_number(opts, "throttling.iops-total", 0); > -fst->cfg.buckets[THROTTLE_OPS_READ].avg = > -qemu_opt_get_number(opts, "throttling.iops-read", 0); > -fst->cfg.buckets[THROTTLE_OPS_WRITE].avg = > -qemu_opt_get_number(opts, "throttling.iops-write", 0); > - > -fst->cfg.buckets[THROTTLE_BPS_TOTAL].max = > -qemu_opt_get_number(opts, "throttling.bps-total-max", 0); > -fst->cfg.buckets[THROTTLE_BPS_READ].max = > -qemu_opt_get_number(opts, "throttling.bps-read-max", 0); > -fst->cfg.buckets[THROTTLE_BPS_WRITE].max = > -
Re: [Qemu-devel] [PATCH v3] live-block-ops.txt: Rename, rewrite, and improve it
On 06/22/2017 04:56 AM, Kashyap Chamarthy wrote: > On Wed, Jun 21, 2017 at 06:49:02PM -0400, John Snow wrote: > > [...] > >>> * TODO (after feedback from John Snow): >>>- Eric Blake suggested to consider documenting incremental backup >>> policies as part of the section: "Live disk backup --- >>> `drive-backup` and `blockdev-backup`" >> >> Perhaps it could be mentioned, but hopefully I've covered it in some >> sufficient detail in the (now) docs/devel/bitmaps.md file; > > Yes, that doc is very useful. That was my precise thought. > >> I'm a little wary of duplicating efforts in this area, > > I share your wariness. > >> but you've covered everything *else* in good detail here, so now my >> file is the odd one out. >> >> I will leave this up to you, really. Perhaps it could be paid some lip >> service with a link to the other document? > > Yes, I was thinking of this, too -- just link to the 'bitmaps' document. > > A quick side question here: Since upstream QEMU is converging onto > Sphinx, and rST, hope you mind if I convert docs/devel/bitmaps.md into > rST at somepoint, for consistency's sake. I'll file a separate review, > anyway for that. In the long term, all / most other documents would > also be converted. > Of course not. I chose bitmaps.md so that it would be nice to view from the github interface while remaining nice to read in plaintext, but feel free to convert it if we actually do standardize on Sphinx/rST. If you can make the generated output look prettier than the github rendering of the markdown I'll ACK it ;) >> The detail in bitmaps.md is a little more verbose than the rest of >> this file, so if you include it wholesale it'd dwarf the rest of this >> document. >> >> What do you think? > > Yes, I fully agree with your suggestion. I will simply link to the > detailed document you wrote, which I was thinking of anyhow. > > Thanks for your comments! > Sure. You could perhaps mention the different sync modes, including top, none, full and incremental and urge readers to check out the bitmaps document for detailed workings of the incremental mode.