[Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU

[Valentin Rakush]

This is RFC because there is another implementation option: it is
possible to implement this functionality in the object_finalize for
all available targets. All targets change will require more testing.
Please let me know if all targets should be changed at once.

This patch changes qmp_device_list_properties and only target-i386
to allow TYPE_CPU class properties to be quered with QMP interface and
with -device core2duo-x86_64-cpu,help command line.

Signed-off-by: Valentin Rakush 
Cc: Luiz Capitulino 
Cc: Eric Blake 
Cc: Markus Armbruster 
Cc: Andreas Färber 
Cc: Daniel P. Berrange 
Cc: Eduardo Habkost 
---
 qmp.c | 19 +++
 target-i386/cpu.c | 32 ++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/qmp.c b/qmp.c
index 6ae7230..2721f16 100644
--- a/qmp.c
+++ b/qmp.c
@@ -516,6 +516,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
char *typename,
Error **errp)
 {
 ObjectClass *klass;
+ObjectClass *cpu_klass;
 Object *obj;
 ObjectProperty *prop;
 ObjectPropertyIterator iter;
@@ -580,6 +581,24 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
char *typename,
 prop_list = entry;
 }
 
+cpu_klass = object_class_dynamic_cast(klass, TYPE_CPU);
+if (cpu_klass) {
+CPUState *tmp_cpu = CPU(obj);
+CPUState *cs = first_cpu;
+
+CPU_FOREACH(cs) {
+if (tmp_cpu->cpu_index == cs->cpu_index) {
+#if defined(CONFIG_USER_ONLY)
+cpu_list_lock();
+#endif
+QTAILQ_REMOVE(, cs, node);
+#if defined(CONFIG_USER_ONLY)
+cpu_list_unlock();
+#endif
+}
+}
+}
+
 object_unref(obj);
 
 return prop_list;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3fa14bf..8c32794 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1479,7 +1479,7 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void 
*data)
 
 dc->props = host_x86_cpu_properties;
 /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
-dc->cannot_destroy_with_object_finalize_yet = true;
+dc->cannot_destroy_with_object_finalize_yet = false;
 }
 
 static void host_x86_cpu_initfn(Object *obj)
@@ -3225,11 +3225,39 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->cpu_exec_enter = x86_cpu_exec_enter;
 cc->cpu_exec_exit = x86_cpu_exec_exit;
 
+object_class_property_add(oc, "family", "int",
+x86_cpuid_version_get_family,
+x86_cpuid_version_set_family, NULL, NULL, NULL);
+object_class_property_add(oc, "model", "int",
+x86_cpuid_version_get_model,
+x86_cpuid_version_set_model, NULL, NULL, NULL);
+object_class_property_add(oc, "stepping", "int",
+x86_cpuid_version_get_stepping,
+x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_class_property_add_str(oc, "vendor",
+x86_cpuid_get_vendor,
+x86_cpuid_set_vendor, NULL);
+object_class_property_add_str(oc, "model-id",
+x86_cpuid_get_model_id,
+x86_cpuid_set_model_id, NULL);
+object_class_property_add(oc, "tsc-frequency", "int",
+x86_cpuid_get_tsc_freq,
+x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+object_class_property_add(oc, "apic-id", "int",
+x86_cpuid_get_apic_id,
+x86_cpuid_set_apic_id, NULL, NULL, NULL);
+object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
+x86_cpu_get_feature_words,
+NULL, NULL, NULL, NULL);
+object_class_property_add(oc, "filtered-features", "X86CPUFeatureWordInfo",
+x86_cpu_get_feature_words,
+NULL, NULL, NULL, NULL);
+
 /*
  * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
  * object in cpus -> dangling pointer after final object_unref().
  */
-dc->cannot_destroy_with_object_finalize_yet = true;
+dc->cannot_destroy_with_object_finalize_yet = false;
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
1.8.3.1

Re: [Qemu-devel] cache.direct

[Stefan Hajnoczi]

On Thu, Feb 11, 2016 at 03:11:55PM +, Jignasha Vithalani wrote:
> How to set cache.direct = on if using aio=native with qemu 2.3
> while mounting with nbd

The NBD block driver does not honor -drive cache=on|off.  It does not
have a client-side cache.

Instead you must set the cache mode on the NBD server side with qemu-nbd
--nocache.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Marcel Apfelbaum]

On 02/11/2016 09:41 PM, Eduardo Habkost wrote:

On Fri, Feb 05, 2016 at 09:51:07AM +0200, Marcel Apfelbaum wrote:

On 02/05/2016 09:49 AM, Markus Armbruster wrote:

"Michael S. Tsirkin"  writes:


On Thu, Feb 04, 2016 at 12:55:22PM +0100, Paolo Bonzini wrote:



On 04/02/2016 12:41, Andreas Färber wrote:

You're talking about machine, right? Some time ago I had proposed Marcel
who initially worked on it, but I'm fine with anyone taking it.


Yes.


For some (but not all) core qdev parts related to the (stalled) QOM
migration I've been taking care of via qom-next. Last time this came up
you didn't want anyone to be M: for qdev, so maybe we can use R: so that
at least people automatically get CC'ed and we avoid this recurring
discussion?


I might have changed my mind on that.  You definitely should be M: for qdev.

Paolo


If Andreas wants to, that's also fine. Several maintainers are
better than one.


*If* the maintainers are all willing and able to work together.



No problem here from my point of view :)


No problem to me, too. :)

I am going to be away from work for 15 days starting on Tuesday
Feb 16th. So if Marcel wants to start queueing patches already,
please be my guest. I will be able to help on that after I'm
back.



Hi,

If there are only a few patches on the mailing list, they can wait.
If the number will grow I'll send a pull request.

So the MAINTAINER file should look like this, right?

Regarding qdev, Andreas, I also think you are the most qualified
to take it, will you?

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d6ee17..a86491a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1200,6 +1200,13 @@ F: docs/*qmp-*
 F: scripts/qmp/
 T: git git://repo.or.cz/qemu/armbru.git qapi-next

+Machine
+M: Eduardo Habkost 
+M: Marcel Apfelbaum 
+S: Supported
+F: hw/core/machine.c
+F: include/hw/boards.h
+



Thanks,
Marcel

Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 11.02.2016 um 12:00 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 11.02.2016 um 07:05 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > > > > However, I don't understand yet which layer do you offer as the 
> > > > > > candidate
> > > > > > for record/replay? What functions should be changed?
> > > > > > I would like to investigate this way, but I don't got it yet.
> > > > >
> > > > > At the core, I wouldn't change any existing function, but introduce a
> > > > > new block driver. You could copy raw_bsd.c for a start and then tweak
> > > > > it. Leave out functions that you don't want to support, and add the
> > > > > necessary magic to .bdrv_co_readv/writev.
> > > > >
> > > > > Something like this (can probably be generalised for more than just
> > > > > reads as the part after the bdrv_co_reads() call should be the same 
> > > > > for
> > > > > reads, writes and any other request types):
> > > > >
> > > > > int blkreplay_co_readv()
> > > > > {
> > > > > BlockReplayState *s = bs->opaque;
> > > > > int reqid = s->reqid++;
> > > > >
> > > > > bdrv_co_readv(bs->file, ...);
> > > > >
> > > > > if (mode == record) {
> > > > > log(reqid, time);
> > > > > } else {
> > > > > assert(mode == replay);
> > > > > bool *done = req_replayed_list_get(reqid)
> > > > > if (done) {
> > > > > *done = true;
> > > > > } else {
> > > > > req_completed_list_insert(reqid, qemu_coroutine_self());
> > > > > qemu_coroutine_yield();
> > > > > }
> > > > > }
> > > > > }
> > > > >
> > > > > /* called by replay.c */
> > > > > int blkreplay_run_event()
> > > > > {
> > > > > if (mode == replay) {
> > > > > co = req_completed_list_get(e.reqid);
> > > > > if (co) {
> > > > > qemu_coroutine_enter(co);
> > > > > } else {
> > > > > bool done = false;
> > > > > req_replayed_list_insert(reqid, );
> > > > > /* wait synchronously for completion */
> > > > > while (!done) {
> > > > > aio_poll();
> > > > > }
> > > > > }
> > > > > }
> > > > > }
> > > > >
> > > > > Where we could consider changing existing code is that it might be
> > > > > desirable to automatically put an instance of this block driver on top
> > > > > of every block device when record/replay is used. If we don't do that,
> > > > > you need to explicitly specify -drive driver=blkreplay,...
> > > >
> > > > As far, as I understand, all synchronous read/write request are also 
> > > > passed
> > > > through this coroutines layer.
> > >
> > > Yes, all read/write requests go through the same function internally, no
> > > matter which external interface was used.
> > >
> > > > It means that every disk access in replay phase should match the 
> > > > recording phase.
> > >
> > > Right. If I'm not mistaken, this was the fundamental requirement you
> > > have, so I wouldn't have suggested this otherwise.
> > >
> > > > Record/replay is intended to be used for debugging and analysis.
> > > > When execution is replayed, guest machine cannot notice analysis 
> > > > overhead.
> > > > Some analysis methods may include disk image reading. E.g., qemu-based
> > > > analysis framework DECAF uses sleuthkit for disk forensics (
> > > https://github.com/sycurelab/DECAF ).
> > > > If similar framework will be used with replay, forensics disk access 
> > > > operations
> > > > won't work if we will record/replay the coroutines.
> > >
> > > Sorry, I'm not sure if I can follow.
> > >
> > > If such analysis software runs in the guest, it's not a replay any more
> > > and I completely fail to see what you're doing.
> > >
> > > If it's a qemu component independent from the guest, then my method
> > > gives you a clean way to bypass the replay driver that wouldn't be
> > > possible with yours.
> >
> > The second one. qemu may be extended with some components that
> > perform guest introspection.
> >
> > > If your plan was to record/replay only async requests and then use sync
> > > requests to bypass the record/replay, let me clearly state that this is
> > > the wrong approach: There are still guest devices which do synchronous
> > > I/O and need to be considered in the replay log, and you shouldn't
> > > prevent the analysis code from using AIO (in fact, using sync I/O in new
> > > code is very much frowned upon).
> >
> > Why do guest synchronous requests have to be recorded?
> > Aren't they completely deterministic?
> 
> Good point. I think you're right in practice. In theory, with dataplane
> (i.e. when running the request in a separate thread) it could happen,
> but I guess that isn't very compatible with replay anyway - and at the
> first sight I couldn't see it performing 

Re: [Qemu-devel] [PATCH v2 2/6] target-arm: Fix handling of SCR.SMD

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 04:03:25PM +, Peter Maydell wrote:
> We weren't quite implementing the handling of SCR.SMD correctly.
> The condition governing whether the SMD bit should apply only
> for NS state is "is EL3 is AArch32", not "is the current EL AArch32".
> Fix the condition, and clarify the comment both to reflect this and
> to expand slightly on what's going on for the v7-no-Virtualization case.
> 
> Signed-off-by: Peter Maydell 
> Reviewed-by: Sergey Fedorov 

Reviewed-by: Edgar E. Iglesias 



> ---
> The bit about forcing SMD to zero confused me, anyway, since I
> expected it to mean "in this function", not elsewhere...
> ---
>  target-arm/op_helper.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index bd48549..4c0980e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -614,12 +614,14 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t 
> syndrome)
>  int cur_el = arm_current_el(env);
>  bool secure = arm_is_secure(env);
>  bool smd = env->cp15.scr_el3 & SCR_SMD;
> -/* On ARMv8 AArch32, SMD only applies to NS state.
> - * On ARMv7 SMD only applies to NS state and only if EL2 is available.
> - * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check
> - * the EL2 condition here.
> +/* On ARMv8 with EL3 AArch64, SMD applies to both S and NS state.
> + * On ARMv8 with EL3 AArch32, or ARMv7 with the Virtualization
> + *  extensions, SMD only applies to NS state.
> + * On ARMv7 without the Virtualization extensions, the SMD bit
> + * doesn't exist, but we forbid the guest to set it to 1 in scr_write(),
> + * so we need not special case this here.
>   */
> -bool undef = is_a64(env) ? smd : (!secure && smd);
> +bool undef = arm_feature(env, ARM_FEATURE_AARCH64) ? smd : smd && 
> !secure;
>  
>  if (arm_is_psci_call(cpu, EXCP_SMC)) {
>  /* If PSCI is enabled and this looks like a valid PSCI call then
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 6/9] pc: acpi: create MADT.lapic entries only for valid lapics

[Igor Mammedov]

On Thu, 11 Feb 2016 14:11:34 -0200
Eduardo Habkost  wrote:

> On Fri, Feb 05, 2016 at 05:14:41PM +0100, Igor Mammedov wrote:
> > On Fri, 5 Feb 2016 13:28:31 -0200
> > Eduardo Habkost  wrote:
> >   
> > > On Thu, Feb 04, 2016 at 12:47:32PM +0100, Igor Mammedov wrote:  
> > > > do not assume that all lapics in range 0..apic_id_limit
> > > > are valid and do not create lapic entries for not
> > > > possible lapics in MADT.
> > > > 
> > > > Signed-off-by: Igor Mammedov 
> > > 
> > > Reviewed-by: Eduardo Habkost 
> > > 
> > > But there's one minor suggestion below:
> > >   
> > > > ---
> > > >  hw/i386/acpi-build.c | 21 ++---
> > > >  1 file changed, 14 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index df13c7d..9eeeffa 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -361,9 +361,11 @@ build_fadt(GArray *table_data, GArray *linker, 
> > > > AcpiPmInfo *pm,
> > > >  }
> > > >  
> > > >  static void
> > > > -build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu,
> > > > -   PcGuestInfo *guest_info)
> > > > +build_madt(GArray *table_data, GArray *linker,
> > > > +   MachineState *machine, PcGuestInfo *guest_info)
> > > >  {
> > > > +MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > > +GArray *apic_id_list = mc->possible_cpu_arch_ids();
> > > >  int madt_start = table_data->len;
> > > >  
> > > >  AcpiMultipleApicTable *madt;
> > > > @@ -376,18 +378,23 @@ build_madt(GArray *table_data, GArray *linker, 
> > > > AcpiCpuInfo *cpu,
> > > >  madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> > > >  madt->flags = cpu_to_le32(1);
> > > >  
> > > > -for (i = 0; i < guest_info->apic_id_limit; i++) {
> > > > +for (i = 0; i < apic_id_list->len; i++) {
> > > >  AcpiMadtProcessorApic *apic = acpi_data_push(table_data, 
> > > > sizeof *apic);
> > > > +CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
> > > > +int apic_id = id.arch_id;
> > > > +
> > > >  apic->type = ACPI_APIC_PROCESSOR;
> > > >  apic->length = sizeof(*apic);
> > > > -apic->processor_id = i;
> > > > -apic->local_apic_id = i;
> > > > -if (test_bit(i, cpu->found_cpus)) {
> > > > +apic->processor_id = apic_id;
> > > > +apic->local_apic_id = apic_id;
> > > > +if (id.cpu != NULL) {
> > > 
> > > This seems to be the only place where CPUArchId.cpu is being used
> > > (see my previous suggestion about making possible_cpu_arch_ids()
> > > return just an uint64_t list).
> > > 
> > > Also, using the existing found_cpus bitmap is more efficient than
> > > making multiple calls to qemu_get_cpu_by_arch_id(). I wouldn't
> > > mind keeping the bitmap.  
> > found_cpus bitmap is not better than (id.cpu != NULL) check
> > the cost of filling both is about the same.  
> 
> The cost doesn't look the same. Populating found_cpus should be
> O(smp_cpus)[1], and is being done only once. Filling id.cpu in
> pc_possible_cpu_arch_ids() is O(max_cpus*smp_cpus), and it is
> called multiple times.
I've refactored patch to make pc_possible_cpu_arch_ids() linear,
pls see v2:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg351298.html

> 
> [1] I just noticed it is actually O(size_of_qom_tree), but it
> is still linear, and could be changed to O(smp_cpus).
> 
> > The issue I have with bitmap is that it's harder to generalize
> > CPU hotplug code with it, while with possible_cpu_arch_ids()
> > returned array I have a list of CPUs to work with without any
> > assumptions on position in bitmap or array.
> > Also bitmap scales worse than a list of CPUs if ID space
> > is sparse and if ID is quite big.  
> 
> Yes, I agree that a list is better depending on how the arch ID
> space is used.
> 
> > That's why I'm dropping bitmap and switching to a list of
> > IDs which in worst case is upto max_cpus.  
> 
> I think the possible_cpu_arch_ids() interface looks good, maybe
> we just need to optimize it.
> 
> But I'm not sure if we need to optimize it now, or if we can live
> with the inefficient code and optimize it later. I won't complain
> if we do it later, if we warn about it in the commit message or
> comments.
> 

Re: [Qemu-devel] [PATCH 2/4] target-arm: Move get/set_r13_banked() to op_helper.c

[Sergey Fedorov]

On 11.02.2016 22:11, Peter Maydell wrote:
> Move get/set_r13_banked() from helper.c to op_helper.c. This will
> let us add exception-raising code to them, and also puts them
> in the same file as get/set_user_reg(), which makes some conceptual
> sense.
>
> (The original reason for the helper.c/op_helper.c split was that
> only op_helper.c had access to the CPU env pointer; this distinction
> has not been true for a long time, though, and so the split is
> now rather arbitrary.)
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Sergey Fedorov 

> ---
>  target-arm/helper.c| 33 -
>  target-arm/op_helper.c | 37 +
>  2 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index bb913c6..c46e3d0 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5365,21 +5365,6 @@ void switch_mode(CPUARMState *env, int mode)
>  }
>  }
>  
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 write\n");
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 read\n");
> -return 0;
> -}
> -
>  uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
>   uint32_t cur_el, bool secure)
>  {
> @@ -7762,24 +7747,6 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, 
> vaddr addr,
>  return phys_addr;
>  }
>  
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -if ((env->uncached_cpsr & CPSR_M) == mode) {
> -env->regs[13] = val;
> -} else {
> -env->banked_r13[bank_number(mode)] = val;
> -}
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -if ((env->uncached_cpsr & CPSR_M) == mode) {
> -return env->regs[13];
> -} else {
> -return env->banked_r13[bank_number(mode)];
> -}
> -}
> -
>  uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>  {
>  ARMCPU *cpu = arm_env_get_cpu(env);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 049b521..053e9b6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -457,6 +457,43 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
> regno, uint32_t val)
>  }
>  }
>  
> +#if defined(CONFIG_USER_ONLY)
> +void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> +{
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +
> +cpu_abort(CPU(cpu), "banked r13 write\n");
> +}
> +
> +uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> +{
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +
> +cpu_abort(CPU(cpu), "banked r13 read\n");
> +return 0;
> +}
> +
> +#else
> +
> +void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> +{
> +if ((env->uncached_cpsr & CPSR_M) == mode) {
> +env->regs[13] = val;
> +} else {
> +env->banked_r13[bank_number(mode)] = val;
> +}
> +}
> +
> +uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> +{
> +if ((env->uncached_cpsr & CPSR_M) == mode) {
> +return env->regs[13];
> +} else {
> +return env->banked_r13[bank_number(mode)];
> +}
> +}
> +#endif
> +
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
> syndrome,
>   uint32_t isread)
>  {

Re: [Qemu-devel] [PATCH 4/4] target-arm: UNDEF in the UNPREDICTABLE SRS-from-System case

[Sergey Fedorov]

On 11.02.2016 22:11, Peter Maydell wrote:
> Make get_r13_banked() raise an exception at runtime for the
> corner case of SRS from System mode, so that we can UNDEF it;
> this brings us in to line with the ARM ARM's set of permitted
> CONSTRAINED UNPREDICTABLE choices.
>
> Signed-off-by: Peter Maydell 

It's a bit misleading that the name "get_r13_banked" says nothing about
SRS instruction but raises an SRS-specific exception. Though, it's only
used for SRS and there seems to be no other candidate to use it; so

Reviewed-by: Sergey Fedorov 

> ---
>  target-arm/op_helper.c | 8 
>  target-arm/translate.c | 9 +
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 05f97a7..8183108 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -474,6 +474,14 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, 
> uint32_t mode)
>  #if defined(CONFIG_USER_ONLY)
>  g_assert_not_reached();
>  #endif
> +if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_SYS) {
> +/* SRS instruction is UNPREDICTABLE from System mode; we UNDEF.
> + * Other UNPREDICTABLE and UNDEF cases were caught at translate time.
> + */
> +raise_exception(env, EXCP_UDEF, syn_uncategorized(),
> +exception_target_el(env));
> +}
> +
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  return env->regs[13];
>  } else {
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 7bceb05..e69145d 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7590,10 +7590,7 @@ static void gen_srs(DisasContext *s,
>   * -- not a valid mode number
>   * -- a mode that's at a higher exception level
>   * -- Monitor, if we are Non-secure
> - * For the UNPREDICTABLE cases we choose to UNDEF, except that for
> - * "current mode is System" we will write a garbage SPSR.
> - * (This is because we don't have access to our current mode here
> - * and would have to do a runtime check to UNDEF for System.)
> + * For the UNPREDICTABLE cases we choose to UNDEF.
>   */
>  if (s->current_el == 1 && !s->ns) {
>  gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
> @@ -7639,6 +7636,9 @@ static void gen_srs(DisasContext *s,
>  
>  addr = tcg_temp_new_i32();
>  tmp = tcg_const_i32(mode);
> +/* get_r13_banked() will raise an exception if called from System mode */
> +gen_set_condexec(s);
> +gen_set_pc_im(s, s->pc - 4);
>  gen_helper_get_r13_banked(addr, cpu_env, tmp);
>  tcg_temp_free_i32(tmp);
>  switch (amode) {
> @@ -7688,6 +7688,7 @@ static void gen_srs(DisasContext *s,
>  tcg_temp_free_i32(tmp);
>  }
>  tcg_temp_free_i32(addr);
> +s->is_jmp = DISAS_UPDATE;
>  }
>  
>  static void disas_arm_insn(DisasContext *s, unsigned int insn)

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Sergey Fedorov]

On 11.02.2016 22:11, Peter Maydell wrote:
> The user-mode versions of get/set_r13_banked() exist just to assert
> if they're ever called -- the translate time code should never
> emit calls to them because SRS from user mode always UNDEF.
> There's no code in the softmmu versions that can't compile in
> CONFIG_USER_ONLY, so combine the two functions rather than
> having completely split versions under ifdefs.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Sergey Fedorov 

> ---
>  target-arm/op_helper.c | 25 ++---
>  1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 053e9b6..05f97a7 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -457,26 +457,11 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
> regno, uint32_t val)
>  }
>  }
>  
> -#if defined(CONFIG_USER_ONLY)
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 write\n");
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 read\n");
> -return 0;
> -}
> -
> -#else
> -
>  void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
>  {
> +#if defined(CONFIG_USER_ONLY)
> +g_assert_not_reached();
> +#endif
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  env->regs[13] = val;
>  } else {
> @@ -486,13 +471,15 @@ void HELPER(set_r13_banked)(CPUARMState *env, uint32_t 
> mode, uint32_t val)
>  
>  uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
>  {
> +#if defined(CONFIG_USER_ONLY)
> +g_assert_not_reached();
> +#endif
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  return env->regs[13];
>  } else {
>  return env->banked_r13[bank_number(mode)];
>  }
>  }
> -#endif
>  
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
> syndrome,
>   uint32_t isread)

Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

[Kevin Wolf]

Am 12.02.2016 um 09:33 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 11.02.2016 um 12:00 hat Pavel Dovgalyuk geschrieben:
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > Am 11.02.2016 um 07:05 hat Pavel Dovgalyuk geschrieben:
> > > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > > > > > However, I don't understand yet which layer do you offer as the 
> > > > > > > candidate
> > > > > > > for record/replay? What functions should be changed?
> > > > > > > I would like to investigate this way, but I don't got it yet.
> > > > > >
> > > > > > At the core, I wouldn't change any existing function, but introduce 
> > > > > > a
> > > > > > new block driver. You could copy raw_bsd.c for a start and then 
> > > > > > tweak
> > > > > > it. Leave out functions that you don't want to support, and add the
> > > > > > necessary magic to .bdrv_co_readv/writev.
> > > > > >
> > > > > > Something like this (can probably be generalised for more than just
> > > > > > reads as the part after the bdrv_co_reads() call should be the same 
> > > > > > for
> > > > > > reads, writes and any other request types):
> > > > > >
> > > > > > int blkreplay_co_readv()
> > > > > > {
> > > > > > BlockReplayState *s = bs->opaque;
> > > > > > int reqid = s->reqid++;
> > > > > >
> > > > > > bdrv_co_readv(bs->file, ...);
> > > > > >
> > > > > > if (mode == record) {
> > > > > > log(reqid, time);
> > > > > > } else {
> > > > > > assert(mode == replay);
> > > > > > bool *done = req_replayed_list_get(reqid)
> > > > > > if (done) {
> > > > > > *done = true;
> > > > > > } else {
> > > > > > req_completed_list_insert(reqid, qemu_coroutine_self());
> > > > > > qemu_coroutine_yield();
> > > > > > }
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > /* called by replay.c */
> > > > > > int blkreplay_run_event()
> > > > > > {
> > > > > > if (mode == replay) {
> > > > > > co = req_completed_list_get(e.reqid);
> > > > > > if (co) {
> > > > > > qemu_coroutine_enter(co);
> > > > > > } else {
> > > > > > bool done = false;
> > > > > > req_replayed_list_insert(reqid, );
> > > > > > /* wait synchronously for completion */
> > > > > > while (!done) {
> > > > > > aio_poll();
> > > > > > }
> > > > > > }
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > Where we could consider changing existing code is that it might be
> > > > > > desirable to automatically put an instance of this block driver on 
> > > > > > top
> > > > > > of every block device when record/replay is used. If we don't do 
> > > > > > that,
> > > > > > you need to explicitly specify -drive driver=blkreplay,...
> > > > >
> > > > > As far, as I understand, all synchronous read/write request are also 
> > > > > passed
> > > > > through this coroutines layer.
> > > >
> > > > Yes, all read/write requests go through the same function internally, no
> > > > matter which external interface was used.
> > > >
> > > > > It means that every disk access in replay phase should match the 
> > > > > recording phase.
> > > >
> > > > Right. If I'm not mistaken, this was the fundamental requirement you
> > > > have, so I wouldn't have suggested this otherwise.
> > > >
> > > > > Record/replay is intended to be used for debugging and analysis.
> > > > > When execution is replayed, guest machine cannot notice analysis 
> > > > > overhead.
> > > > > Some analysis methods may include disk image reading. E.g., qemu-based
> > > > > analysis framework DECAF uses sleuthkit for disk forensics (
> > > > https://github.com/sycurelab/DECAF ).
> > > > > If similar framework will be used with replay, forensics disk access 
> > > > > operations
> > > > > won't work if we will record/replay the coroutines.
> > > >
> > > > Sorry, I'm not sure if I can follow.
> > > >
> > > > If such analysis software runs in the guest, it's not a replay any more
> > > > and I completely fail to see what you're doing.
> > > >
> > > > If it's a qemu component independent from the guest, then my method
> > > > gives you a clean way to bypass the replay driver that wouldn't be
> > > > possible with yours.
> > >
> > > The second one. qemu may be extended with some components that
> > > perform guest introspection.
> > >
> > > > If your plan was to record/replay only async requests and then use sync
> > > > requests to bypass the record/replay, let me clearly state that this is
> > > > the wrong approach: There are still guest devices which do synchronous
> > > > I/O and need to be considered in the replay log, and you shouldn't
> > > > prevent the analysis code from using AIO (in fact, using sync I/O in new
> > > > code is very much frowned upon).
> > >
> > > Why do guest synchronous requests have to 

Re: [Qemu-devel] broken HMP command: info mtree

[Daniel P. Berrange]

On Fri, Feb 12, 2016 at 12:15:26PM +0100, Igor Mammedov wrote:
> On Thu, 11 Feb 2016 16:35:39 +0100
> Igor Mammedov  wrote:
> 
> > executing 'info mtree' from monitor prompt causes infinite loop
> > printing it over and over.
> > 
> > to reproduce build current master adn run:
> > 
> > qemu-system-x86_64 -monitor stdio
> > 
> > and then execute 'info mtree' in monitor prompt
> > 
> 
> it bisects to:
> 
> commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2
> Author: Daniel P. Berrange 
> Date:   Tue Jan 19 11:14:29 2016 +
> 
> char: convert from GIOChannel to QIOChannel
> 
> In preparation for introducing TLS support to the TCP chardev
> backend, convert existing chardev code from using GIOChannel
> to QIOChannel. This simplifies the chardev code by removing
> most of the OS platform conditional code for dealing with
> file descriptor passing.
> 
> Signed-off-by: Daniel P. Berrange 
> Message-Id: <1453202071-10289-3-git-send-email-berra...@redhat.com>
> Signed-off-by: Paolo Bonzini 
> 
> 
> build with:
> ./configure --target-list=x86_64-softmmu --enable-debug 
> on RHEL72ish host
> 
> monitor output has to be stdio

Sigh, so much pain from the chardev code. I'll investigate and send a
suitable patch asap.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] broken HMP command: info mtree

[Paolo Bonzini]

On 12/02/2016 12:17, Daniel P. Berrange wrote:
> On Fri, Feb 12, 2016 at 12:15:26PM +0100, Igor Mammedov wrote:
>> On Thu, 11 Feb 2016 16:35:39 +0100
>> Igor Mammedov  wrote:
>>
>>> executing 'info mtree' from monitor prompt causes infinite loop
>>> printing it over and over.
>>>
>>> to reproduce build current master adn run:
>>>
>>> qemu-system-x86_64 -monitor stdio
>>>
>>> and then execute 'info mtree' in monitor prompt
>>
>> it bisects to:
>>
>> commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2
>> Author: Daniel P. Berrange 
>> Date:   Tue Jan 19 11:14:29 2016 +
>>
>> char: convert from GIOChannel to QIOChannel
>> 
>> In preparation for introducing TLS support to the TCP chardev
>> backend, convert existing chardev code from using GIOChannel
>> to QIOChannel. This simplifies the chardev code by removing
>> most of the OS platform conditional code for dealing with
>> file descriptor passing.
>> 
>> Signed-off-by: Daniel P. Berrange 
>> Message-Id: <1453202071-10289-3-git-send-email-berra...@redhat.com>
>> Signed-off-by: Paolo Bonzini 
>>
>>
>> build with:
>> ./configure --target-list=x86_64-softmmu --enable-debug 
>> on RHEL72ish host
>>
>> monitor output has to be stdio
> 
> Sigh, so much pain from the chardev code. I'll investigate and send a
> suitable patch asap.

Hmm, I cannot reproduce this though.

Paolo

Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Andreas Färber]

Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:
> From: "Alexey V. Kostyushko" 
> 
> With Hyper-V enabled CPU hotplug stops working. The CPU appears in device
> manager on Windows but does not appear in peformance monitor and control
> panel.
> 
> The root of the problem is the following. Windows checks
> HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
> this bit is enough to cure the situation.
> 
> Add option 'hv-cpuhotplug' to control this behavior.
> 
> Signed-off-by: Alexey V. Kostyushko 
> Signed-off-by: Denis V. Lunev 
> CC: Paolo Bonzini 
> CC: Richard Henderson 
> CC: Eduardo Habkost 
> CC: "Andreas Färber" 
> ---
>  target-i386/cpu-qom.h | 1 +
>  target-i386/cpu.c | 1 +
>  target-i386/kvm.c | 6 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 5f9d960..4aec616 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -96,6 +96,7 @@ typedef struct X86CPU {
>  bool hyperv_runtime;
>  bool hyperv_synic;
>  bool hyperv_stimer;
> +bool hyperv_cpuhotplug;
>  bool check_cpuid;
>  bool enforce_cpuid;
>  bool expose_kvm;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index b255644..32c38ae 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
>  DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
>  DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
>  DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
> +DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug, false),

Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
dashes convention for QOM properties, i.e. "hv-cpu-hotplug".

Regards,
Andreas

>  DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>  DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>  DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
[snip]

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH] qdev: Start disentangling bus from device

[Andreas Färber]

Move bus type and related APIs to a separate file bus.c.
This is a first step in breaking up qdev.c into more manageable chunks.

Signed-off-by: Andreas Färber 
---
 Here's a first step in breaking up qdev.c, originally prepared as part of my
 QOM device reset refactoring. Amazingly it still applied.
 
 hw/core/Makefile.objs |   1 +
 hw/core/bus.c | 248 ++
 hw/core/qdev.c| 222 
 3 files changed, 249 insertions(+), 222 deletions(-)
 create mode 100644 hw/core/bus.c

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index abb3560..b4f88e3 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,5 +1,6 @@
 # core qdev-related obj files, also used by *-user:
 common-obj-y += qdev.o qdev-properties.o
+common-obj-y += bus.o
 common-obj-y += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
diff --git a/hw/core/bus.c b/hw/core/bus.c
new file mode 100644
index 000..7294f91
--- /dev/null
+++ b/hw/core/bus.c
@@ -0,0 +1,248 @@
+/*
+ *  Dynamic device configuration and creation -- buses.
+ *
+ *  Copyright (c) 2009 CodeSourcery
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "hw/qdev.h"
+
+static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
+  Error **errp)
+{
+
+object_property_set_link(OBJECT(bus), OBJECT(handler),
+ QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+}
+
+void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error 
**errp)
+{
+qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
+}
+
+void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+{
+qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
+}
+
+int qbus_walk_children(BusState *bus,
+   qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
+   qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
+   void *opaque)
+{
+BusChild *kid;
+int err;
+
+if (pre_busfn) {
+err = pre_busfn(bus, opaque);
+if (err) {
+return err;
+}
+}
+
+QTAILQ_FOREACH(kid, >children, sibling) {
+err = qdev_walk_children(kid->child,
+ pre_devfn, pre_busfn,
+ post_devfn, post_busfn, opaque);
+if (err < 0) {
+return err;
+}
+}
+
+if (post_busfn) {
+err = post_busfn(bus, opaque);
+if (err) {
+return err;
+}
+}
+
+return 0;
+}
+
+static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
+{
+const char *typename = object_get_typename(OBJECT(bus));
+BusClass *bc;
+char *buf;
+int i, len, bus_id;
+
+bus->parent = parent;
+
+if (name) {
+bus->name = g_strdup(name);
+} else if (bus->parent && bus->parent->id) {
+/* parent device has id -> use it plus parent-bus-id for bus name */
+bus_id = bus->parent->num_child_bus;
+
+len = strlen(bus->parent->id) + 16;
+buf = g_malloc(len);
+snprintf(buf, len, "%s.%d", bus->parent->id, bus_id);
+bus->name = buf;
+} else {
+/* no id -> use lowercase bus type plus global bus-id for bus name */
+bc = BUS_GET_CLASS(bus);
+bus_id = bc->automatic_ids++;
+
+len = strlen(typename) + 16;
+buf = g_malloc(len);
+len = snprintf(buf, len, "%s.%d", typename, bus_id);
+for (i = 0; i < len; i++) {
+buf[i] = qemu_tolower(buf[i]);
+}
+bus->name = buf;
+}
+
+if (bus->parent) {
+QLIST_INSERT_HEAD(>parent->child_bus, bus, sibling);
+bus->parent->num_child_bus++;
+object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), 
NULL);
+object_unref(OBJECT(bus));
+} else if (bus != sysbus_get_default()) {
+/* TODO: once all bus devices are qdevified,
+   only reset handler for main_system_bus should be registered here. */
+qemu_register_reset(qbus_reset_all_fn, bus);
+}
+}
+
+static void bus_unparent(Object *obj)
+{
+BusState *bus = BUS(obj);
+BusChild *kid;
+
+while ((kid = 

Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Denis V. Lunev]

On 02/12/2016 02:13 PM, Andreas Färber wrote:

Am 12.02.2016 um 12:08 schrieb Denis V. Lunev:

On 02/12/2016 02:00 PM, Andreas Färber wrote:

Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:

From: "Alexey V. Kostyushko" 

With Hyper-V enabled CPU hotplug stops working. The CPU appears in
device
manager on Windows but does not appear in peformance monitor and control
panel.

The root of the problem is the following. Windows checks
HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
this bit is enough to cure the situation.

Add option 'hv-cpuhotplug' to control this behavior.

Signed-off-by: Alexey V. Kostyushko 
Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
---
   target-i386/cpu-qom.h | 1 +
   target-i386/cpu.c | 1 +
   target-i386/kvm.c | 6 +-
   3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5f9d960..4aec616 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -96,6 +96,7 @@ typedef struct X86CPU {
   bool hyperv_runtime;
   bool hyperv_synic;
   bool hyperv_stimer;
+bool hyperv_cpuhotplug;
   bool check_cpuid;
   bool enforce_cpuid;
   bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b255644..32c38ae 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
   DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
   DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
   DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
+DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug,
false),

Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
dashes convention for QOM properties, i.e. "hv-cpu-hotplug".

Regards,
Andreas

This name is for libvirt. We can take one one. This is not a problem.

Roman Kagan has proposed verbally a bit different approach.
He suggests not to introduce the option but
check here that HyperV is enabled (any single option is enough
to face the problem) and CPU hotplug is enabled and automatically
enable this bit.

Paolo, Andreas, is there any opinion on this?

That implicit proposal sounds even more appealing to me, yes.

You could still additionally do a dynamic property though, in case you
want to inspect or toggle it at runtime (no clue when Windows reads it).

no. this will not work. I should setup CPUID at the beginning
(before boot) and keep it persistent. Thus either option
coming from libvirt or this bit would be automatically
calculated by QEMU.

I'll check that we could create read-only property to export
the value.

OK. I'll redo this tomorrow if nobody will explicitly say 'no'
today :)

Den

Re: [Qemu-devel] broken HMP command: info mtree

[Igor Mammedov]

On Fri, 12 Feb 2016 14:08:32 +0100
Paolo Bonzini  wrote:

> On 12/02/2016 12:17, Daniel P. Berrange wrote:
> > On Fri, Feb 12, 2016 at 12:15:26PM +0100, Igor Mammedov wrote:  
> >> On Thu, 11 Feb 2016 16:35:39 +0100
> >> Igor Mammedov  wrote:
> >>  
> >>> executing 'info mtree' from monitor prompt causes infinite loop
> >>> printing it over and over.
> >>>
> >>> to reproduce build current master adn run:
> >>>
> >>> qemu-system-x86_64 -monitor stdio
> >>>
> >>> and then execute 'info mtree' in monitor prompt  
> >>
> >> it bisects to:
> >>
> >> commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2
> >> Author: Daniel P. Berrange 
> >> Date:   Tue Jan 19 11:14:29 2016 +
> >>
> >> char: convert from GIOChannel to QIOChannel
> >> 
> >> In preparation for introducing TLS support to the TCP chardev
> >> backend, convert existing chardev code from using GIOChannel
> >> to QIOChannel. This simplifies the chardev code by removing
> >> most of the OS platform conditional code for dealing with
> >> file descriptor passing.
> >> 
> >> Signed-off-by: Daniel P. Berrange 
> >> Message-Id: <1453202071-10289-3-git-send-email-berra...@redhat.com>
> >> Signed-off-by: Paolo Bonzini 
> >>
> >>
> >> build with:
> >> ./configure --target-list=x86_64-softmmu --enable-debug 
> >> on RHEL72ish host
> >>
> >> monitor output has to be stdio  
> > 
> > Sigh, so much pain from the chardev code. I'll investigate and send a
> > suitable patch asap.  
> 
> Hmm, I cannot reproduce this though.
Perhaps I'm affected because my stdout goes via remote ssh session.

It looks like monitor tries to flush buffer but succeeds only partially
and returns with EAGAIN and on the next flush attempt it tries to
flush the same buffer again from the first byte again

> 
> Paolo
> 

Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 10.02.2016 um 13:05 hat Pavel Dovgalyuk geschrieben:
> > > > > Am 09.02.2016 um 12:52 hat Pavel Dovgalyuk geschrieben:
> > > > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > > > But even this doesn't feel completely right, because block 
> > > > > > > drivers are
> > > > > > > already layered and there is no need to hardcode something 
> > > > > > > optional (and
> > > > > > > rarely used) in the hot code path that could just be another 
> > > > > > > layer.
> > > > > > >
> > > > > > > I assume that you know beforehand if you want to replay 
> > > > > > > something, so
> > > > > > > requiring you to configure your block devices with a replay 
> > > > > > > driver on
> > > > > > > top of the stack seems reasonable enough.
> > > > > >
> > > > > > I cannot use block drivers for this. When driver functions are 
> > > > > > used, QEMU
> > > > > > is already used coroutines (and probably started bottom halves).
> > > > > > Coroutines make execution non-deterministic.
> > > > > > That's why we have to intercept blk_aio_ functions, that are called
> > > > > > deterministically.
> > > > >
> > > > > What does "deterministic" mean in this context, i.e. what are your 
> > > > > exact
> > > > > requirements?
> > > >
> > > > "Deterministic" means that the replayed execution should run exactly
> > > > the same guest instructions in the same sequence, as in recording 
> > > > session.
> > >
> > > Okay. I think with this we can do better than what you have now.
> > >
> > > > > I don't think that coroutines introduce anything non-deterministic per
> > > > > se. Depending on what you mean by it, the block layer code paths in
> > > > > block.c may contain problematic code.
> > > >
> > > > They are non-deterministic if we need instruction-level accuracy.
> > > > Thread switching (and therefore callbacks and BH execution) is 
> > > > non-deterministic.
> > >
> > > Thread switching depends on an external event (the kernel scheduler
> > > deciding to switch), so agreed, if a thread switch ever influences what
> > > the guest sees, that would be a problem.
> > >
> > > Generally, however, callbacks and BHs don't involve a thread switch at
> > > all (BHs can be invoked from a different thread in theory, but we have
> > > very few of those cases and they shouldn't be visible for the guest).
> > > The same is true for coroutines, which are semantically equivalent to
> > > callbacks.
> > >
> > > > In two different executions these callbacks may happen at different 
> > > > moments of
> > > > time (counting in number of executed instructions).
> > > > All operations with virtual devices (including memory, interrupt 
> > > > controller,
> > > > and disk drive controller) should happen at deterministic moments of 
> > > > time
> > > > to be replayable.
> > >
> > > Right, so let's talk about what this external non-deterministic event
> > > really is.
> > >
> > > I think the only thing whose timing is unknown in the block layer is the
> > > completion of I/O requests. This non-determinism comes from the time the
> > > I/O syscalls made by the lowest layer (usually raw-posix) take.
> >
> > Right.
> >
> > > This means that we can add logic to remove the non-determinism at the
> > > point of our choice between raw-posix and the guest device emulation. A
> > > block driver on top is as good as anything else.
> > >
> > > While recording, this block driver would just pass the request to next
> > > lower layer (starting a request is deterministic, so it doesn't need to
> > > be logged) and once the request completes it logs it. While replaying,
> > > the completion of requests is delayed until we read it in the log; if we
> > > read it in the log and the request hasn't completed yet, we do a busy
> > > wait for it (while(!completed) aio_poll();).
> >
> > I tried serializing all bottom halves and worker thread callbacks in
> > previous version of the patches. That code was much more complicated
> > and error-prone than the current version. We had to classify all bottom
> > halves to recorded and non-recorded (because sometimes they are used
> > for qemu's purposes, not the guest ones).
> >
> > However, I don't understand yet which layer do you offer as the candidate
> > for record/replay? What functions should be changed?
> > I would like to investigate this way, but I don't got it yet.
> 
> At the core, I wouldn't change any existing function, but introduce a
> new block driver. You could copy raw_bsd.c for a start and then tweak
> it. Leave out functions that you don't want to support, and add the
> necessary magic to .bdrv_co_readv/writev.
> 
> Something like this (can probably be generalised for more than just
> reads as the part after the bdrv_co_reads() call should be the same for
> reads, writes and any other request types):
> 
> int blkreplay_co_readv()
> {
> 

Re: [Qemu-devel] [PATCH v6 00/16] Implement TLS support to QEMU NBD server & client

[Kashyap Chamarthy]

On Wed, Feb 10, 2016 at 06:40:58PM +, Daniel P. Berrange wrote:

[...]

I've applied all the series in this patches, to yesterday's Git master,
so I'm here:

$ git describe
pull-qcrypto-next-2016-02-02-1-378-gf9375d2

> Starting a QEMU system emulator built-in NBD server
> 
>   $ qemu-system-x86_64 \
>  -qmp unix:/tmp/qmp,server \
>  -hda /home/berrange/Fedora-Server-netinst-x86_64-23.iso \
>  -object 
> tls-creds-x509,id=tls0,dir=/home/berrange/security/qemutls,endpoint=server

Instead of an ISO, I have this command-line:

$QEMU \
  -display none \
  -nodefconfig \
  -nodefaults \
  -m 2048 \
  -device virtio-scsi-pci,id=scsi \
  -device virtio-serial-pci \
  -serial stdio \
  -drive file=./cirros-0.3.3.qcow2,format=qcow2,if=virtio \
  -object 
tls-creds-x509,id=tls0,dir=/export/security/gnutls,endpoint=server \
  -qmp unix:./qmp-sock,server

>   $ qmp-shell /tmp/qmp
>  (qmp) nbd-server-start addr={"host":"localhost","port":"9000"}
>  tls-creds=tls0

However, this invocation seem to work for me, am I doing something wrong?

$ ./qmp-shell ./qmp-sock
Welcome to the QMP low-level shell!
Connected to QEMU 2.5.50
  
(QEMU) nbd-server-start addr={'host:'localhost','port':'9000'} 
tls-creds=tls0
{"error": {"class": "GenericError", "desc": "Invalid parameter type for 
'addr', expected: QDict"}}
(QEMU) 


I also tried the `rlwrap` with UNIX socket approach, the below way.  You
corrected my invocation on #qemu (where I was incorrectly using
'tls-creds' as part of the "addr" struct); hope I got it right this
time:

$ rlwrap -H ~/.qmp_history socat UNIX-CONNECT:./qmp-sock STDIO
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 5, "major": 2}, 
"package": ""}, "capabilities": []}}
{ "execute": "qmp_capabilities" }
{"return": {}}

{"execute":"nbd-server-start","arguments":{"addr":{"type":"inet","data":{"host":"localhost","port":"9000"}},
 "tls-creds":"tls0"}

Which just seems to be running forever, and does not yield anything (I
was expecting a: {"return": {}}).


[...]


-- 
/kashyap

Re: [Qemu-devel] [PATCH 1/5] target-tricore: Add trap handling

[Bastian Koppelmann]

On 02/12/2016 03:39 AM, Richard Henderson wrote:
> On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
>> +void tricore_cpu_do_interrupt(CPUState *cs)
>> +{
>> +TriCoreCPU *cpu = TRICORE_CPU(cs);
>> +CPUTriCoreState *env = >env;
>> +
>> +/* The stack pointer in A[10] is set to the Interrupt Stack
>> Pointer (ISP)
>> +   when the processor was not previously using the interrupt stack
>> +   (in case of PSW.IS = 0). The stack pointer bit is set for
>> using the
>> +   interrupt stack: PSW.IS = 1. */
>> +if ((env->PSW & MASK_PSW_IS) == 0) {
>> +env->gpr_a[10] = env->ISP;
>> +}
> 
> You appear to have forgotten to save pre-interrupt state here.

What do you mean by "pre-interrupt state"? The register context is saved
by generate_trap() calls.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Andreas Färber]

Am 12.02.2016 um 12:08 schrieb Denis V. Lunev:
> On 02/12/2016 02:00 PM, Andreas Färber wrote:
>> Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:
>>> From: "Alexey V. Kostyushko" 
>>>
>>> With Hyper-V enabled CPU hotplug stops working. The CPU appears in
>>> device
>>> manager on Windows but does not appear in peformance monitor and control
>>> panel.
>>>
>>> The root of the problem is the following. Windows checks
>>> HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
>>> this bit is enough to cure the situation.
>>>
>>> Add option 'hv-cpuhotplug' to control this behavior.
>>>
>>> Signed-off-by: Alexey V. Kostyushko 
>>> Signed-off-by: Denis V. Lunev 
>>> CC: Paolo Bonzini 
>>> CC: Richard Henderson 
>>> CC: Eduardo Habkost 
>>> CC: "Andreas Färber" 
>>> ---
>>>   target-i386/cpu-qom.h | 1 +
>>>   target-i386/cpu.c | 1 +
>>>   target-i386/kvm.c | 6 +-
>>>   3 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
>>> index 5f9d960..4aec616 100644
>>> --- a/target-i386/cpu-qom.h
>>> +++ b/target-i386/cpu-qom.h
>>> @@ -96,6 +96,7 @@ typedef struct X86CPU {
>>>   bool hyperv_runtime;
>>>   bool hyperv_synic;
>>>   bool hyperv_stimer;
>>> +bool hyperv_cpuhotplug;
>>>   bool check_cpuid;
>>>   bool enforce_cpuid;
>>>   bool expose_kvm;
>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>> index b255644..32c38ae 100644
>>> --- a/target-i386/cpu.c
>>> +++ b/target-i386/cpu.c
>>> @@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
>>>   DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
>>>   DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
>>>   DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
>>> +DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug,
>>> false),
>> Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
>> dashes convention for QOM properties, i.e. "hv-cpu-hotplug".
>>
>> Regards,
>> Andreas
> This name is for libvirt. We can take one one. This is not a problem.
> 
> Roman Kagan has proposed verbally a bit different approach.
> He suggests not to introduce the option but
> check here that HyperV is enabled (any single option is enough
> to face the problem) and CPU hotplug is enabled and automatically
> enable this bit.
> 
> Paolo, Andreas, is there any opinion on this?

That implicit proposal sounds even more appealing to me, yes.

You could still additionally do a dynamic property though, in case you
want to inspect or toggle it at runtime (no clue when Windows reads it).

Andreas

> As far as I can see for the time being we lend such decisions to
> libvirt. But this bit does not require any processing.
> 
> Den


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all of of coroutine context

[Dr. David Alan Gilbert]

* Denis V. Lunev (d...@openvz.org) wrote:
> There is a possibility to hit an assert in qcow2_get_specific_info that
> s->qcow_version is undefined. This happens when VM in starting from
> suspended state, i.e. it processes incoming migration, and in the same
> time 'info block' is called.
> 
> The problem is that qcow2_invalidate_cache() closes the image and
> memset()s BDRVQcowState in the middle.
> 
> The patch moves processing of bdrv_invalidate_cache_all out of
> coroutine context for postcopy migration to avoid that. This function
> is called with the following stack:
>   process_incoming_migration_co
>   qemu_loadvm_state
>   qemu_loadvm_state_main
>   loadvm_process_command
>   loadvm_postcopy_handle_run
> 
> Signed-off-by: Denis V. Lunev 
> CC: Paolo Bonzini 
> CC: Juan Quintela 
> CC: Amit Shah 

I've tested my normal postcopy smoke test and it seems to work,
so
Tested-by: Dr. David Alan Gilbert 

however, can you add a comment before loadvm_postcopy_handle_run_bh
to explain why it's a bh, otherwise at some point someone might
put it back.

I'll admit to not really understanding what the difference is
between bh and coroutine context; I'd thought if it was all
in the main thread stuff was safe.

Dave

> ---
>  migration/savevm.c | 27 +--
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 94f2894..8415fd9 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1496,18 +1496,10 @@ static int 
> loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
>  return 0;
>  }
>  
> -/* After all discards we can start running and asking for pages */
> -static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> +static void loadvm_postcopy_handle_run_bh(void *opaque)
>  {
> -PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
>  Error *local_err = NULL;
>  
> -trace_loadvm_postcopy_handle_run();
> -if (ps != POSTCOPY_INCOMING_LISTENING) {
> -error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
> -return -1;
> -}
> -
>  /* TODO we should move all of this lot into postcopy_ram.c or a shared 
> code
>   * in migration.c
>   */
> @@ -1519,7 +1511,6 @@ static int 
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>  bdrv_invalidate_cache_all(_err);
>  if (local_err) {
>  error_report_err(local_err);
> -return -1;
>  }
>  
>  trace_loadvm_postcopy_handle_run_cpu_sync();
> @@ -1534,6 +1525,22 @@ static int 
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>  /* leave it paused and let management decide when to start the CPU */
>  runstate_set(RUN_STATE_PAUSED);
>  }
> +}
> +
> +/* After all discards we can start running and asking for pages */
> +static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
> +{
> +PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
> +QEMUBH *bh;
> +
> +trace_loadvm_postcopy_handle_run();
> +if (ps != POSTCOPY_INCOMING_LISTENING) {
> +error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
> +return -1;
> +}
> +
> +bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, NULL);
> +qemu_bh_schedule(bh);
>  
>  /* We need to finish reading the stream from the package
>   * and also stop reading anything more from the stream that loaded the
> -- 
> 2.1.4
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all of of coroutine context

[Paolo Bonzini]

On 12/02/2016 13:50, Dr. David Alan Gilbert wrote:
> I'll admit to not really understanding what the difference is
> between bh and coroutine context; I'd thought if it was all
> in the main thread stuff was safe.

It's arguably a bug in the block layer code.  It assumes that all code
called from a coroutine wants not to block.  Moving stuff to a bottom
half tells the block layer that you want bdrv_invalidate_cache_all to block.

Paolo

Re: [Qemu-devel] cache.direct

[Paolo Bonzini]

On 12/02/2016 11:25, Stefan Hajnoczi wrote:
> On Thu, Feb 11, 2016 at 03:11:55PM +, Jignasha Vithalani wrote:
>> > How to set cache.direct = on if using aio=native with qemu 2.3
>> > while mounting with nbd
> The NBD block driver does not honor -drive cache=on|off.  It does not
> have a client-side cache.

It also does not honor aio=native, which you can likewise set in qemu-nbd.

Paolo

> Instead you must set the cache mode on the NBD server side with qemu-nbd
> --nocache.

Re: [Qemu-devel] [PATCH v10] spec: add qcow2 bitmaps extension specification

[Fam Zheng]

On Fri, 02/05 11:58, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> v10
> - rewordings, thanks to Fam
> - fix:
> Given an offset byte_nr into the virtual disk and the bitmap's granularity, 
> the
> -bit offset into the bitmap can be calculated like this:
> +bit offset into the image file to the corresponding bit of the bitmap can be
> +calculated like this:
> - remove last sentence about 'auto' flasg consistency as superfluous.
> 
> 
> v9
> - rewordings, thanks to Max
> 
> v8
> - rewordings
> - bitmap_directory_size: 4b -> 8b
> - add more descriptive description in == Bitmaps == section
> - add paragraph "Dirty tracking bitmaps"
> 
>   Bitmap directory entry:
> - extra data should not allocate additional clusters
> - padding must be all-bytes-zero
> - add extra_data_compatible flag (now behavior in case of unknown
>   extra data is defined by this flag)
> 
> v7:
> 
> - Rewordings, grammar.
>   Max, Eric, John, thank you very much.
> 
> - add last paragraph: remaining bits in bitmap data clusters must be
>   zero.
> 
> - s/Bitmap Directory/bitmap directory/ and other names like this at
>   the request of Max.
> 
> v6:
> 
> - reword bitmap_directory_size description
> - bitmap type: make 0 reserved
> - extra_data_size: resize to 4bytes
>   Also, I've marked this field as "must be zero". We can always change
>   it, if we decide allowing managing app to specify any extra data, by
>   defining some magic value as a top of user extra data.. So, for now
>   non zeor extra_data_size should be considered as an error.
> - swap name and extra_data to give good alignment to extra_data.
> 
> 
> v5:
> 
> - 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
>   bitmaps.
> - rewordings
> - move upper bounds to "Notes about Qemu limits"
> - s/should/must somewhere. (but not everywhere)
> - move name_size field closer to name itself in bitmap header
> - add extra data area to bitmap header
> - move bitmap data description to separate section
> 
>  docs/specs/qcow2.txt | 221 
> ++-
>  1 file changed, 220 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index f236d8c..80cdfd0 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -103,7 +103,18 @@ in the description of a field.
>  write to an image with unknown auto-clear features if it
>  clears the respective bits from this field first.
>  
> -Bits 0-63:  Reserved (set to 0)
> +Bit 0:  Bitmaps extension bit
> +This bit indicates consistency for the 
> bitmaps
> +extension data.
> +
> +It is an error if this bit is set without the
> +bitmaps extension present.
> +
> +If the bitmaps extension is present but this
> +bit is unset, the bitmaps extension data 
> must be
> +considered inconsistent.
> +
> +Bits 1-63:  Reserved (set to 0)
>  
>   96 -  99:  refcount_order
>  Describes the width of a reference count block entry 
> (width
> @@ -123,6 +134,7 @@ be stored. Each extension has a structure like the 
> following:
>  0x - End of the header extension area
>  0xE2792ACA - Backing file format name
>  0x6803f857 - Feature name table
> +0x23852875 - Bitmaps extension
>  other  - Unknown header extension, can be safely
>   ignored
>  
> @@ -166,6 +178,36 @@ the header extension data. Each entry look like this:
>  terminated if it has full length)
>  
>  
> +== Bitmaps extension ==
> +
> +The bitmaps extension is an optional header extension. It provides the 
> ability
> +to store bitmaps related to a virtual disk. For now, there is only one bitmap
> +type: the dirty tracking bitmap, which tracks virtual disk changes from some
> +point in time.
> +
> +The data of the extension should be considered consistent only if the
> +corresponding auto-clear feature bit is set, see 

Re: [Qemu-devel] [PATCH v3 2/5] drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header

[Paolo Bonzini]

On 12/02/2016 09:10, Andrey Smetanin wrote:
>>>
>>> -hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
>>> +hv_do_hypercall(HV_X64_HCALL_SIGNAL_EVENT, channel->sig_event,
>>> NULL);
>>
>> What tree does this apply to?
> next-20160211

Ok, I'll check whether I can just merge something from KY to get the
hv.c->connection.c code movement.  Otherwise we probably should just
leave the HVCALL_* constants unchanged.

Paolo

Re: [Qemu-devel] [PATCH] build: Don't redefine 'inline'

[Peter Maydell]

On 9 February 2016 at 18:49, Eric Blake  wrote:
> Actively redefining 'inline' is wrong for C++, where gcc has an
> extension 'inline namespace' which fails to compile if the
> keyword 'inline' is replaced by a macro expansion.  This will
> matter once we start to include "qemu/osdep.h" first from C++
> files, depending also on whether the system headers are new
> enough to be using the gcc extension.
>
> But rather than just guard things by __cplusplus, let's look at
> the overall picture.  Commit df2542c737ea2 in 2007 defined 'inline'
> to the gcc attribute __always_inline__, with the rationale "To
> avoid discarded inlining bug".  But compilers have improved since
> then, and we are probably better off trusting the compiler rather
> than trying to force its hand.
>
> So just nuke our craziness.
>
> Signed-off-by: Eric Blake 

Reviewed-by: Peter Maydell 

(and tested that it passes my usual merge build tests).

Does this patch suffice to get your system to build all
my clean-includes patches?

thanks
-- PMM

Re: [Qemu-devel] broken HMP command: info mtree

[Igor Mammedov]

On Thu, 11 Feb 2016 16:35:39 +0100
Igor Mammedov  wrote:

> executing 'info mtree' from monitor prompt causes infinite loop
> printing it over and over.
> 
> to reproduce build current master adn run:
> 
> qemu-system-x86_64 -monitor stdio
> 
> and then execute 'info mtree' in monitor prompt
> 

it bisects to:

commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2
Author: Daniel P. Berrange 
Date:   Tue Jan 19 11:14:29 2016 +

char: convert from GIOChannel to QIOChannel

In preparation for introducing TLS support to the TCP chardev
backend, convert existing chardev code from using GIOChannel
to QIOChannel. This simplifies the chardev code by removing
most of the OS platform conditional code for dealing with
file descriptor passing.

Signed-off-by: Daniel P. Berrange 
Message-Id: <1453202071-10289-3-git-send-email-berra...@redhat.com>
Signed-off-by: Paolo Bonzini 


build with:
./configure --target-list=x86_64-softmmu --enable-debug 
on RHEL72ish host

monitor output has to be stdio 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: skip configuration section during migration of older machines

[Greg Kurz]

On Fri, 12 Feb 2016 16:24:26 +1100
David Gibson  wrote:

> On Thu, Feb 11, 2016 at 04:53:40PM +, Dr. David Alan Gilbert wrote:
> > * Greg Kurz (gk...@linux.vnet.ibm.com) wrote:  
> > > On Mon, 08 Feb 2016 16:59:47 +0100
> > > Greg Kurz  wrote:  
> > > > Since QEMU 2.4, we have a configuration section in the migration stream.
> > > > This must be skipped for older machines, like it is already done for 
> > > > x86.
> > > >   
> > > 
> > > Ouch ! It is more complex than I thought... the migration of pseries-2.3
> > > machine is already broken between QEMU-2.3 and QEMU-2.4. So this patch
> > > fixes indeed migration of a pseries-2.3 machine from QEMU-2.3, but it
> > > breaks migration of the same machine from QEMU-2.4 and up.
> > > 
> > > Not sure how to deal with that... is it reasonable to assume that
> > > pseries-2.3 running with QEMU-2.3 is the common case ? If so, this
> > > patch would bring more help than harm.  
> > 
> > Unfortunately we can not fix history, so we have to pick something to fix.
> > So unless there is another reason, then I normally say keep it working
> > between the latest versions of qemu; i.e. if someone is running qemu 2.5 
> > with
> > -M 2.3  then dont break it when they try and migrate to 2.6, even though
> > this would fix an older qemu migrating into 2.6.  
> 
> Yeah, I tend to agree, but I'd change my mind if there's evidence that
> the older qemu is much more widely deployed.
> 

I don't know how to provide proofs for that... just hints.

FWIW, both currently supported IBM's PowerKVM distros are based on older
QEMU (2.0 and 2.3), same for LTS ubuntu (2.0) and standard ubuntu (2.3).

I believe SLE 12, SLE 12 SP1 and RHEV don't ship a newer QEMU but I'm
not sure... Of course fedora already ships QEMU 2.4.1 but I don't
think so many people use it in production on expensive POWER8 based
hardware.

> IIUC that would entail no actual change to the code yes?  But I think
> we should put a comment there saying what the fix would be to talk to
> the older qemu, and why we chose not to apply it.
> 

Something like:

/* QEMU 2.4 introduced a configuration section in the migration stream.
 * It is mandatory for all machine types but it is possible to disable
 * it to stay compatible with older machines. Unfortunately, QEMU 2.4
 * got released without addressing the compatibility issue for pseries.
 * As a consequence, pseries-2.3 and older machines cannot be migrated
 * from QEMU <= 2.3 to QEMU >= 2.4. This won't be fixed as it would
 * break migration of these older pseries when started with the latest
 * QEMU, and we don't want that.
 */

And Dave's suggestion to disable configuration section from the command
line could allow to workaround the issue. I have the patch already, I'll
do some testing and post shortly.

> > However, as discussed on irc you might be able to fudge it; for example
> > using qemu_peek_byte to test whether or not you have a configuration
> > section, and making it not error for some machine types.   This isn't
> > pretty, but if it's important for you to get the coniguration working
> > then it's the type of trick that might work.
> > 
> > Dave
> >   
> > >   
> > > > Fixes: 61964c23e5ddd5a33f15699e45ce126f879e3e33
> > > > Cc: qemu-sta...@nongnu.org
> > > > Signed-off-by: Greg Kurz 
> > > > ---
> > > >  hw/ppc/spapr.c |1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 5bd8fd3ef842..bca7cb8a5d27 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -2446,6 +2446,7 @@ 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();
> > > >  }
> > > > 
> > > >  static void spapr_machine_2_3_class_options(MachineClass *mc)
> > > > 
> > > >   
> > >   
> 

Re: [Qemu-devel] [PATCH] qdev: Start disentangling bus from device

[Peter Maydell]

On 12 February 2016 at 11:09, Andreas Färber  wrote:
> Move bus type and related APIs to a separate file bus.c.
> This is a first step in breaking up qdev.c into more manageable chunks.
>
> Signed-off-by: Andreas Färber 
> ---
>  Here's a first step in breaking up qdev.c, originally prepared as part of my
>  QOM device reset refactoring. Amazingly it still applied.
>
>  hw/core/Makefile.objs |   1 +
>  hw/core/bus.c | 248 
> ++
>  hw/core/qdev.c| 222 
>  3 files changed, 249 insertions(+), 222 deletions(-)
>  create mode 100644 hw/core/bus.c

Makes sense to me. I eyeballed the patch to confirm that it's
purely moving code.

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code

[Markus Armbruster]

"Michael S. Tsirkin"  writes:

> On Thu, Feb 11, 2016 at 01:51:30PM -0200, Eduardo Habkost wrote:
>> On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
>> > On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
>> > > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
>> > > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
>> > > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
>> > > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>> > > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin 
>> > > > > > > wrote:
>> > > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost 
>> > > > > > > > wrote:
>> > > > > > > > > This is another attempt to remove old q35 machine code. Now 
>> > > > > > > > > I am
>> > > > > > > > > also removing unused compat code to demonstrate the benefit 
>> > > > > > > > > of
>> > > > > > > > > throwing away the old code that nobody uses.
>> > > > > > > > 
>> > > > > > > > The same thing I said applies - we don't know that nobody uses 
>> > > > > > > > old q35
>> > > > > > > > machine types.
>> > > > > > > > We do know we don't need to migrate to/from them,
>> > > > > > > > so we can drop compat code.
>> > > > > > > > But please add aliases so people can still start these 
>> > > > > > > > machines.
>> > > > > > > 
>> > > > > > > If people use them, they can easily update their configurations.
>> > > > > > > I will copy and paste the reply Markus sent 4 months ago below.
>> > > > > > > 
>> > > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster 
>> > > > > > > wrote:
>> > > > > > > > We've been through this before, but we can go through it once 
>> > > > > > > > more.
>> > > > > > > > Choices:
>> > > > > > > > 
>> > > > > > > > A. Remove old machine type
>> > > > > > > > 
>> > > > > > > >A guest using it can't be started.  Easy to understand on 
>> > > > > > > > the host.
>> > > > > > > >An error message advising to switch to a newer machine type 
>> > > > > > > > would be
>> > > > > > > >a nice touch.
>> > > > > > > > 
>> > > > > > > >This is a clean break in backward compatibility.  To be 
>> > > > > > > > mentioned in
>> > > > > > > >release notes, obviously.
>> > > > > > > > 
>> > > > > > > > B. Change old machine type in a guest-visible way
>> > > > > > > > 
>> > > > > > > >Depending on the nature of the change and the guest, a 
>> > > > > > > > guest using it
>> > > > > > > >either doesn't notice, copes with it successfully, or fails 
>> > > > > > > > in
>> > > > > > > >guest-specific ways.  If the latter, the failure can be 
>> > > > > > > > "guest
>> > > > > > > >hangs", which is much harder to figure out than A.
>> > > > > > > > 
>> > > > > > > >Unless we can *demonstrate* that nothing bad happens for 
>> > > > > > > > all the
>> > > > > > > >guests people actually use with the old machine types, this 
>> > > > > > > > is a
>> > > > > > > >different kind of backward compatibility break.
>> > > > > > > > 
>> > > > > > > >Demonstrating this is feels infeasible to me, but you're 
>> > > > > > > > welcome to
>> > > > > > > >try.
>> > > > > > > > 
>> > > > > > > > I could call the difference between the two a tradeoff, but 
>> > > > > > > > since we've
>> > > > > > > > been through this before, I'll be more blunt: choosing B robs 
>> > > > > > > > Peter (the
>> > > > > > > > guy with guests where badness happens) to pay Paul (the guy 
>> > > > > > > > with guests
>> > > > > > > > that cope).  Paul is saved the inconvenience of having to read 
>> > > > > > > > release
>> > > > > > > > notes or his logs, and change machine types.  Peter pays for 
>> > > > > > > > that with
>> > > > > > > > figuring out WTF his guests are doing now.
>> > > > > > > > 
>> > > > > > > > As a user, I'd pick a clean break in backward compatibility 
>> > > > > > > > over a hack
>> > > > > > > > that preserves effective compatibility when it works, but 
>> > > > > > > > breaks it
>> > > > > > > > uncleanly when it doesn't.
>> > > > > > > > 
>> > > > > > > > As a developer, I'm insisting on it.
>> > > > > > > > 
>> > > > > > > > So, if you want B, the onus is on *you* to show us why nothing 
>> > > > > > > > bad will
>> > > > > > > > happen.
>> > > > > > > > 
>> > > > > > 
>> > > > > > I agree with the conclusion for option B.  But I think the correct
>> > > > > > solution is not A, it is to analyse changes, maybe even test, and 
>> > > > > > show
>> > > > > > that nothing bad can happen.
>> > > > > 
>> > > > > Do you volunteer for that work?
>> > > > 
>> > > > Nope, sorry. It's your idea, your patchset.
>> > > 
>> > > It's your idea. You are the one proposing to waste resources
>> > > keeping an old machine-type name "working" just because you don't
>> > > want users (who we don't even know if they actually exist) to
>> > > update their configurations on a QEMU upgrade.
>> > > 
>> 

Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Denis V. Lunev]

On 02/12/2016 02:00 PM, Andreas Färber wrote:

Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:

From: "Alexey V. Kostyushko" 

With Hyper-V enabled CPU hotplug stops working. The CPU appears in device
manager on Windows but does not appear in peformance monitor and control
panel.

The root of the problem is the following. Windows checks
HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
this bit is enough to cure the situation.

Add option 'hv-cpuhotplug' to control this behavior.

Signed-off-by: Alexey V. Kostyushko 
Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
---
  target-i386/cpu-qom.h | 1 +
  target-i386/cpu.c | 1 +
  target-i386/kvm.c | 6 +-
  3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5f9d960..4aec616 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -96,6 +96,7 @@ typedef struct X86CPU {
  bool hyperv_runtime;
  bool hyperv_synic;
  bool hyperv_stimer;
+bool hyperv_cpuhotplug;
  bool check_cpuid;
  bool enforce_cpuid;
  bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b255644..32c38ae 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
  DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
  DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
  DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
+DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug, false),

Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
dashes convention for QOM properties, i.e. "hv-cpu-hotplug".

Regards,
Andreas

This name is for libvirt. We can take one one. This is not a problem.

Roman Kagan has proposed verbally a bit different approach.
He suggests not to introduce the option but
check here that HyperV is enabled (any single option is enough
to face the problem) and CPU hotplug is enabled and automatically
enable this bit.

Paolo, Andreas, is there any opinion on this?

As far as I can see for the time being we lend such decisions to
libvirt. But this bit does not require any processing.

Den

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Andreas Färber]

Am 12.02.2016 um 10:17 schrieb Marcel Apfelbaum:
> On 02/11/2016 09:41 PM, Eduardo Habkost wrote:
>> On Fri, Feb 05, 2016 at 09:51:07AM +0200, Marcel Apfelbaum wrote:
>>> On 02/05/2016 09:49 AM, Markus Armbruster wrote:
 "Michael S. Tsirkin"  writes:

> On Thu, Feb 04, 2016 at 12:55:22PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 04/02/2016 12:41, Andreas Färber wrote:
>>> You're talking about machine, right? Some time ago I had proposed
>>> Marcel
>>> who initially worked on it, but I'm fine with anyone taking it.
>>
>> Yes.
>>
>>> For some (but not all) core qdev parts related to the (stalled) QOM
>>> migration I've been taking care of via qom-next. Last time this
>>> came up
>>> you didn't want anyone to be M: for qdev, so maybe we can use R:
>>> so that
>>> at least people automatically get CC'ed and we avoid this recurring
>>> discussion?
>>
>> I might have changed my mind on that.  You definitely should be M:
>> for qdev.
>>
>> Paolo
>
> If Andreas wants to, that's also fine. Several maintainers are
> better than one.

 *If* the maintainers are all willing and able to work together.

>>>
>>> No problem here from my point of view :)
>>
>> No problem to me, too. :)
>>
>> I am going to be away from work for 15 days starting on Tuesday
>> Feb 16th. So if Marcel wants to start queueing patches already,
>> please be my guest. I will be able to help on that after I'm
>> back.
>>
> 
> Hi,
> 
> If there are only a few patches on the mailing list, they can wait.
> If the number will grow I'll send a pull request.
> 
> So the MAINTAINER file should look like this, right?
> 
> Regarding qdev, Andreas, I also think you are the most qualified
> to take it, will you?
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2d6ee17..a86491a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1200,6 +1200,13 @@ F: docs/*qmp-*
>  F: scripts/qmp/
>  T: git git://repo.or.cz/qemu/armbru.git qapi-next
> 
> +Machine
> +M: Eduardo Habkost 
> +M: Marcel Apfelbaum 
> +S: Supported
> +F: hw/core/machine.c
> +F: include/hw/boards.h
> +

Fine with me, ack.

For qdev.c itself I prefer not to create a misleading "QDev" section but
rather just proposed a first step to split up qdev.c not just into
common vs. system-only code but also in better maintainable subareas.
That's targeted at having a section like "Core device API" covering a
to-be-created device.c with myself plus some backup as maintainer, then
Igor/mst/whomever for "Device hotplug interface" or the like.
qdev-system.c we could consider to split up so that the block/net/char
specific parts can be assigned clear maintainers - haven't investigated
that part yet. In the meantime we could simply create multiple sections
covering different aspects of qdev* files.

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU

[Andreas Färber]

Hi,

Am 12.02.2016 um 10:13 schrieb Valentin Rakush:
> This is RFC because there is another implementation option: it is
> possible to implement this functionality in the object_finalize for
> all available targets. All targets change will require more testing.
> Please let me know if all targets should be changed at once.

I thought I had already seen some Fujitsu/IBM patch to fix this...
(pointers appreciated) It should be fixed on the level where the problem
is introduced - i.e. if qom/cpu.c calls the cpu_exec_init() in
instance_init it needs to call the corresponding exit function in
instance_finalize; dito for target-i386/cpu.c or wherever. Or we try to
move it from instance_init to realize, avoiding it getting called in the
first place.

> 
> This patch changes qmp_device_list_properties and only target-i386
> to allow TYPE_CPU class properties to be quered with QMP interface and
> with -device core2duo-x86_64-cpu,help command line.
> 
> Signed-off-by: Valentin Rakush 
> Cc: Luiz Capitulino 
> Cc: Eric Blake 
> Cc: Markus Armbruster 
> Cc: Andreas Färber 
> Cc: Daniel P. Berrange 
> Cc: Eduardo Habkost 
> ---
>  qmp.c | 19 +++
>  target-i386/cpu.c | 32 ++--
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/qmp.c b/qmp.c
> index 6ae7230..2721f16 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -516,6 +516,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
> char *typename,
> Error **errp)
>  {
>  ObjectClass *klass;
> +ObjectClass *cpu_klass;

Please use cpu_class, only "class" is a reserved identifier.

>  Object *obj;
>  ObjectProperty *prop;
>  ObjectPropertyIterator iter;
> @@ -580,6 +581,24 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
> char *typename,
>  prop_list = entry;
>  }
>  
> +cpu_klass = object_class_dynamic_cast(klass, TYPE_CPU);
> +if (cpu_klass) {
> +CPUState *tmp_cpu = CPU(obj);
> +CPUState *cs = first_cpu;
> +
> +CPU_FOREACH(cs) {
> +if (tmp_cpu->cpu_index == cs->cpu_index) {
> +#if defined(CONFIG_USER_ONLY)
> +cpu_list_lock();
> +#endif
> +QTAILQ_REMOVE(, cs, node);
> +#if defined(CONFIG_USER_ONLY)
> +cpu_list_unlock();
> +#endif
> +}
> +}
> +}
> +
>  object_unref(obj);
>  
>  return prop_list;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3fa14bf..8c32794 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1479,7 +1479,7 @@ static void host_x86_cpu_class_init(ObjectClass *oc, 
> void *data)
>  
>  dc->props = host_x86_cpu_properties;
>  /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
> -dc->cannot_destroy_with_object_finalize_yet = true;
> +dc->cannot_destroy_with_object_finalize_yet = false;
>  }
>  
>  static void host_x86_cpu_initfn(Object *obj)
> @@ -3225,11 +3225,39 @@ static void x86_cpu_common_class_init(ObjectClass 
> *oc, void *data)
>  cc->cpu_exec_enter = x86_cpu_exec_enter;
>  cc->cpu_exec_exit = x86_cpu_exec_exit;
>  
> +object_class_property_add(oc, "family", "int",
> +x86_cpuid_version_get_family,
> +x86_cpuid_version_set_family, NULL, NULL, NULL);

You add class properties here but I don't see you deleting the previous
instance properties anywhere?

> +object_class_property_add(oc, "model", "int",
> +x86_cpuid_version_get_model,
> +x86_cpuid_version_set_model, NULL, NULL, NULL);
> +object_class_property_add(oc, "stepping", "int",
> +x86_cpuid_version_get_stepping,
> +x86_cpuid_version_set_stepping, NULL, NULL, NULL);
> +object_class_property_add_str(oc, "vendor",
> +x86_cpuid_get_vendor,
> +x86_cpuid_set_vendor, NULL);
> +object_class_property_add_str(oc, "model-id",
> +x86_cpuid_get_model_id,
> +x86_cpuid_set_model_id, NULL);
> +object_class_property_add(oc, "tsc-frequency", "int",
> +x86_cpuid_get_tsc_freq,
> +x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> +object_class_property_add(oc, "apic-id", "int",
> +x86_cpuid_get_apic_id,
> +x86_cpuid_set_apic_id, NULL, NULL, NULL);
> +object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
> +x86_cpu_get_feature_words,
> +NULL, NULL, NULL, NULL);
> +object_class_property_add(oc, "filtered-features", 
> "X86CPUFeatureWordInfo",
> +

[Qemu-devel] [PATCH v1 4/9] target-arm: Add more fields to the data abort syndrome generator

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add the following flags to the data abort syndrome generator:
* isv - Instruction syndrome valid
* sas - Syndrome access size
* sse - Syndrome sign extend
* srt - Syndrome register transfer
* sf  - Sixty-Four bit register width
* ar  - Acquire/Release

These flags are not yet used, so this patch has no functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/internals.h | 16 ++--
 target-arm/op_helper.c |  8 ++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/target-arm/internals.h b/target-arm/internals.h
index b1c483b..0934709 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -359,13 +359,25 @@ static inline uint32_t syn_insn_abort(int same_el, int 
ea, int s1ptw, int fsc)
 | (ea << 9) | (s1ptw << 7) | fsc;
 }
 
-static inline uint32_t syn_data_abort(int same_el, int ea, int cm, int s1ptw,
+static inline uint32_t syn_data_abort(int same_el, int isv,
+  int sas, int sse, int srt,
+  int sf, int ar,
+  int ea, int cm, int s1ptw,
   int wnr, int fsc,
   bool is_thumb)
 {
-return (EC_DATAABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
+uint32_t v;
+
+v = (EC_DATAABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
 | (is_thumb ? 0 : ARM_EL_IL)
 | (ea << 9) | (cm << 8) | (s1ptw << 7) | (wnr << 6) | fsc;
+
+/* Insn Syndrome fields are RES0 if ISV is unset.  */
+if (isv) {
+v |= (isv << 24) | (sas << 22) | (sse << 21) | (srt << 16)
+ | (sf << 15) | (ar << 14);
+}
+return v;
 }
 
 static inline uint32_t syn_swstep(int same_el, int isv, int ex)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 4e629e1..9bf635f 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -115,7 +115,9 @@ void tlb_fill(CPUState *cs, target_ulong addr, int 
is_write, int mmu_idx,
 syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
 exc = EXCP_PREFETCH_ABORT;
 } else {
-syn = syn_data_abort(same_el, 0, 0, fi.s1ptw, is_write == 1, syn,
+syn = syn_data_abort(same_el,
+ 0, 0, 0, 0, 0, 0,
+ 0, 0, fi.s1ptw, is_write == 1, syn,
  env->thumb);
 if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
 fsr |= (1 << 11);
@@ -162,7 +164,9 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, 
int is_write,
 }
 
 raise_exception(env, EXCP_DATA_ABORT,
-syn_data_abort(same_el, 0, 0, 0, is_write == 1, 0x21,
+syn_data_abort(same_el,
+   0, 0, 0, 0, 0, 0,
+   0, 0, 0, is_write == 1, 0x21,
env->thumb),
 target_el);
 }
-- 
1.9.1

[Qemu-devel] [PATCH 2/4] loader: Add load_image_mr() to load ROM image to a MemoryRegion

[Peter Maydell]

Add a new function load_image_mr(), which behaves like
load_image_targphys() except that it loads the ROM image to
a specified MemoryRegion rather than to a specified physical
address. This is useful when a ROM blob needs to be loaded
to a particular flash or ROM device but the address of that
device in the machine's address space is not known. (For
instance, ROMs in devices, or ROMs which might exist in
a different address space to the system address space.)

Signed-off-by: Peter Maydell 
---
 hw/core/loader.c| 35 +++
 include/hw/loader.h | 18 --
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 3a57415..260b3d6 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -147,6 +147,28 @@ int load_image_targphys(const char *filename,
 return size;
 }
 
+int load_image_mr(const char *filename, MemoryRegion *mr)
+{
+int size;
+
+if (!memory_access_is_direct(mr, false)) {
+/* Can only load an image into RAM or ROM */
+return -1;
+}
+
+size = get_image_size(filename);
+
+if (size > memory_region_size(mr)) {
+return -1;
+}
+if (size > 0) {
+if (rom_add_file_mr(filename, mr, -1) < 0) {
+return -1;
+}
+}
+return size;
+}
+
 void pstrcpy_targphys(const char *name, hwaddr dest, int buf_size,
   const char *source)
 {
@@ -751,7 +773,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char 
*name)
 
 int rom_add_file(const char *file, const char *fw_dir,
  hwaddr addr, int32_t bootindex,
- bool option_rom)
+ bool option_rom, MemoryRegion *mr)
 {
 MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
 Rom *rom;
@@ -818,7 +840,12 @@ int rom_add_file(const char *file, const char *fw_dir,
 
 fw_cfg_add_file(fw_cfg, fw_file_name, data, rom->romsize);
 } else {
-snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
+if (mr) {
+rom->mr = mr;
+snprintf(devpath, sizeof(devpath), "/rom@%s", file);
+} else {
+snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
+}
 }
 
 add_boot_device_path(bootindex, NULL, devpath);
@@ -892,12 +919,12 @@ int rom_add_elf_program(const char *name, void *data, 
size_t datasize,
 
 int rom_add_vga(const char *file)
 {
-return rom_add_file(file, "vgaroms", 0, -1, true);
+return rom_add_file(file, "vgaroms", 0, -1, true, NULL);
 }
 
 int rom_add_option(const char *file, int32_t bootindex)
 {
-return rom_add_file(file, "genroms", 0, bootindex, true);
+return rom_add_file(file, "genroms", 0, bootindex, true, NULL);
 }
 
 static void rom_reset(void *unused)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index f7b43ab..09c3764 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -16,6 +16,18 @@ int load_image(const char *filename, uint8_t *addr); /* 
deprecated */
 ssize_t load_image_size(const char *filename, void *addr, size_t size);
 int load_image_targphys(const char *filename, hwaddr,
 uint64_t max_sz);
+/**
+ * load_image_mr: load an image into a memory region
+ * @filename: Path to the image file
+ * @mr: Memory Region to load into
+ *
+ * Load the specified file into the memory region.
+ * The file loaded is registered as a ROM, so its contents will be
+ * reinstated whenever the system is reset.
+ * If the file is larger than the memory region's size the call will fail.
+ * Returns -1 on failure, or the size of the file.
+ */
+int load_image_mr(const char *filename, MemoryRegion *mr);
 
 /* This is the limit on the maximum uncompressed image size that
  * load_image_gzipped_buffer() and load_image_gzipped() will read. It prevents
@@ -67,7 +79,7 @@ extern bool rom_file_has_mr;
 
 int rom_add_file(const char *file, const char *fw_dir,
  hwaddr addr, int32_t bootindex,
- bool option_rom);
+ bool option_rom, MemoryRegion *mr);
 MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
size_t max_len, hwaddr addr,
const char *fw_file_name,
@@ -82,9 +94,11 @@ void *rom_ptr(hwaddr addr);
 void hmp_info_roms(Monitor *mon, const QDict *qdict);
 
 #define rom_add_file_fixed(_f, _a, _i)  \
-rom_add_file(_f, NULL, _a, _i, false)
+rom_add_file(_f, NULL, _a, _i, false, NULL)
 #define rom_add_blob_fixed(_f, _b, _l, _a)  \
 rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL)
+#define rom_add_file_mr(_f, _mr, _i)\
+rom_add_file(_f, NULL, 0, _i, false, mr)
 
 #define PC_ROM_MIN_VGA 0xc
 #define PC_ROM_MIN_OPTION  0xc8000
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 04:03:24PM +, Peter Maydell wrote:
> Correct some corner cases we were getting wrong for
> CNTFRQ access rights:
>  * should UNDEF from 32-bit Secure EL1
>  * only writable from the highest implemented exception level,
>which might not be EL1 now
> 
> To clarify the code, provide a new utility function
> arm_highest_el() which returns the highest implemented
> exception level.

Reviewed-by: Edgar E. Iglesias 


> 
> Signed-off-by: Peter Maydell 
> ---
> Rewritten to use arm_highest_el() to improve clarity
> ---
>  target-arm/cpu.h| 12 
>  target-arm/helper.c | 29 ++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5137632..afbf366 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
>  #define PL1_RW (PL1_R | PL1_W)
>  #define PL0_RW (PL0_R | PL0_W)
>  
> +/* Return the highest implemented Exception Level */
> +static inline int arm_highest_el(CPUARMState *env)
> +{
> +if (arm_feature(env, ARM_FEATURE_EL3)) {
> +return 3;
> +}
> +if (arm_feature(env, ARM_FEATURE_EL2)) {
> +return 2;
> +}
> +return 1;
> +}
> +
>  /* Return the current Exception Level (as per ARMv8; note that this differs
>   * from the ARMv7 Privilege Level).
>   */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2f9db72..4d27c00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
>  static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo 
> *ri,
> bool isread)
>  {
> -/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
> -if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) 
> {
> -return CP_ACCESS_TRAP;
> +/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
> + * Writable only at the highest implemented exception level.
> + */
> +int el = arm_current_el(env);
> +
> +switch (el) {
> +case 0:
> +if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
> +return CP_ACCESS_TRAP;
> +}
> +break;
> +case 1:
> +if (!isread && ri->state == ARM_CP_STATE_AA32 &&
> +arm_is_secure_below_el3(env)) {
> +/* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
> +return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +break;
> +case 2:
> +case 3:
> +break;
>  }
> +
> +if (!isread && el < arm_highest_el(env)) {
> +return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  return CP_ACCESS_OK;
>  }
>  
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 1/4] target-arm: Clean up trap/undef handling of SRS

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 07:11:46PM +, Peter Maydell wrote:
> The SRS instruction is:
>  * UNDEFINED in Hyp mode
>  * UNPREDICTABLE in User or System mode
>  * UNPREDICTABLE if the specified mode isn't accessible
>  * trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
> 
> Clean up the code to handle all these cases cleanly, including
> picking UNDEF as our choice of UNPREDICTABLE behaviour rather
> blindly trusting the mode field passed in the instruction.
> As part of this, move the check for IS_USER into gen_srs()
> itself rather than having it done by the caller.
> 
> The exception is that we don't UNDEF for calls from System
> mode, which need a runtime check. This will be dealt with in
> the following commits.
> 
> Signed-off-by: Peter Maydell 


Reviewed-by: Edgar E. Iglesias 


> ---
>  target-arm/translate.c | 66 
> ++
>  1 file changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index cf3dc33..7bceb05 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7578,8 +7578,67 @@ static void gen_srs(DisasContext *s,
>  uint32_t mode, uint32_t amode, bool writeback)
>  {
>  int32_t offset;
> -TCGv_i32 addr = tcg_temp_new_i32();
> -TCGv_i32 tmp = tcg_const_i32(mode);
> +TCGv_i32 addr, tmp;
> +bool undef = false;
> +
> +/* SRS is:
> + * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
> + * - UNDEFINED in Hyp mode
> + * - UNPREDICTABLE in User or System mode
> + * - UNPREDICTABLE if the specified mode is:
> + * -- not implemented
> + * -- not a valid mode number
> + * -- a mode that's at a higher exception level
> + * -- Monitor, if we are Non-secure
> + * For the UNPREDICTABLE cases we choose to UNDEF, except that for
> + * "current mode is System" we will write a garbage SPSR.
> + * (This is because we don't have access to our current mode here
> + * and would have to do a runtime check to UNDEF for System.)
> + */
> +if (s->current_el == 1 && !s->ns) {
> +gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
> +return;
> +}
> +
> +if (s->current_el == 0 || s->current_el == 2) {
> +undef = true;
> +}
> +
> +switch (mode) {
> +case ARM_CPU_MODE_USR:
> +case ARM_CPU_MODE_FIQ:
> +case ARM_CPU_MODE_IRQ:
> +case ARM_CPU_MODE_SVC:
> +case ARM_CPU_MODE_ABT:
> +case ARM_CPU_MODE_UND:
> +case ARM_CPU_MODE_SYS:
> +break;
> +case ARM_CPU_MODE_HYP:
> +if (s->current_el == 1 || !arm_dc_feature(s, ARM_FEATURE_EL2)) {
> +undef = true;
> +}
> +break;
> +case ARM_CPU_MODE_MON:
> +/* No need to check specifically for "are we non-secure" because
> + * we've already made EL0 UNDEF and handled the trap for S-EL1;
> + * so if this isn't EL3 then we must be non-secure.
> + */
> +if (s->current_el != 3) {
> +undef = true;
> +}
> +break;
> +default:
> +undef = true;
> +}
> +
> +if (undef) {
> +gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
> +   default_exception_el(s));
> +return;
> +}
> +
> +addr = tcg_temp_new_i32();
> +tmp = tcg_const_i32(mode);
>  gen_helper_get_r13_banked(addr, cpu_env, tmp);
>  tcg_temp_free_i32(tmp);
>  switch (amode) {
> @@ -7739,9 +7798,6 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  }
>  } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
>  /* srs */
> -if (IS_USER(s)) {
> -goto illegal_op;
> -}
>  ARCH(6);
>  gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
>  return;
> -- 
> 1.9.1
> 

[Qemu-devel] [Bug 1545024] [NEW] compiling on armv7 crashes compile qlx.o

[Klaftenegger Felix]

Public bug reported:

If i try to compile qemu on armv7 cpu i get this error:

  LINK  qemu-nbd
  CCqemu-img.o
  LINK  qemu-img
  LINK  qemu-io
  LINK  qemu-bridge-helper
  CCqmp-marshal.o
  CChw/display/qxl.o
{standard input}: Assembler messages:
{standard input}:1704: Error: bad instruction `lock'
{standard input}:1704: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:1864: Error: bad instruction `lock'
{standard input}:1864: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:5239: Error: bad instruction `lock'
{standard input}:5239: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:5731: Error: bad instruction `lock'
{standard input}:5731: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:11923: Error: bad instruction `lock'
{standard input}:11923: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:13960: Error: bad instruction `lock'
{standard input}:13960: Error: bad instruction `addl $0,0(%rsp)'
{standard input}:14349: Error: bad instruction `lock'
{standard input}:14349: Error: bad instruction `addl $0,0(%rsp)'
/home/fleixi/git/qemu/rules.mak:57: recipe for target 'hw/display/qxl.o' failed
make: *** [hw/display/qxl.o] Error 1

Build options are:

 ./configure --target-list=i386-softmmu
Install prefix/usr/local
BIOS directory/usr/local/share/qemu
binary directory  /usr/local/bin
library directory /usr/local/lib
module directory  /usr/local/lib/qemu
libexec directory /usr/local/libexec
include directory /usr/local/include
config directory  /usr/local/etc
local state directory   /usr/local/var
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /home/fleixi/git/qemu
C compilercc
Host C compiler   cc
C++ compiler  c++
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabihf/glib-2.0/include  -g 
-mcpu=cortex-a15.cortex-a7 -mfloat-abi=hard -mfpu=neon-vfpv4 -O2 -pipe 
-ffast-math -ftree-vectorize -mvectorize-with-neon-quad -fstack-protector 
--param=ssp-buffer-size=4
QEMU_CFLAGS   -I/usr/include/pixman-1   -Werror  -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-strong   -I/usr/include/libpng12  
-I/usr/local/include/spice-server -I/usr/local/include 
-I/usr/local/include/spice-1 -I/usr/include/glib-2.0 
-I/usr/lib/arm-linux-gnueabihf/glib-2.0/include -I/usr/include/pixman-1 
LDFLAGS   -Wl,--warn-common -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  arm
host big endian   no
target list   i386-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   no
GTK support   yes
GTK GL supportno
GNUTLS supportno
GNUTLS hash   no
libgcrypt no
nettleno
libtasn1  no
VTE support   no
curses supportno
virgl support no
curl support  yes
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  yes
VNC JPEG support  yes
VNC PNG support   yes
xen support   no
brlapi supportno
bluez  supportyes
Documentation no
PIE   no
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   no
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
sigev_thread_id   yes
uuid support  no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
Trace backendslog
spice support yes (0.12.10/0.12.6)
rbd support   no
xfsctl supportno
smartcard support no
libusbno
usb net redir no
OpenGL supportno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   no
QOM debugging yes
vhdx  no
lzo support   no
snappy supportno
bzip2 support yes
NUMA host support no
tcmalloc support  no
jemalloc support  no

testet with  qemu-git branch stable-2.4 and git master

Shoulded the configure detecting "bigendian" too?

** Affects: qemu
 Importance: Undecided
  

[Qemu-devel] [PATCH v1 6/9] target-arm/translate-a64.c: Unify some of the ldst_reg decoding

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

The various load/store variants under disas_ldst_reg can all reuse the
same decoding for opc, size, rt and is_vector.

This patch unifies the decoding in preparation for generating
instruction syndromes for data aborts.
This will allow us to reduce the number of places to hook in updates
to the load/store state needed to generate the insn syndromes.

No functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/translate-a64.c | 41 +++--
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index bf31f8a..9e26d5e 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2075,19 +2075,19 @@ static void disas_ldst_pair(DisasContext *s, uint32_t 
insn)
  * size: 00 -> 8 bit, 01 -> 16 bit, 10 -> 32 bit, 11 -> 64bit
  * opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
  */
-static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn)
+static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn,
+int opc,
+int size,
+int rt,
+bool is_vector)
 {
-int rt = extract32(insn, 0, 5);
 int rn = extract32(insn, 5, 5);
 int imm9 = sextract32(insn, 12, 9);
-int opc = extract32(insn, 22, 2);
-int size = extract32(insn, 30, 2);
 int idx = extract32(insn, 10, 2);
 bool is_signed = false;
 bool is_store = false;
 bool is_extended = false;
 bool is_unpriv = (idx == 2);
-bool is_vector = extract32(insn, 26, 1);
 bool post_index;
 bool writeback;
 
@@ -2194,19 +2194,19 @@ static void disas_ldst_reg_imm9(DisasContext *s, 
uint32_t insn)
  * Rn: address register or SP for base
  * Rm: offset register or ZR for offset
  */
-static void disas_ldst_reg_roffset(DisasContext *s, uint32_t insn)
+static void disas_ldst_reg_roffset(DisasContext *s, uint32_t insn,
+   int opc,
+   int size,
+   int rt,
+   bool is_vector)
 {
-int rt = extract32(insn, 0, 5);
 int rn = extract32(insn, 5, 5);
 int shift = extract32(insn, 12, 1);
 int rm = extract32(insn, 16, 5);
-int opc = extract32(insn, 22, 2);
 int opt = extract32(insn, 13, 3);
-int size = extract32(insn, 30, 2);
 bool is_signed = false;
 bool is_store = false;
 bool is_extended = false;
-bool is_vector = extract32(insn, 26, 1);
 
 TCGv_i64 tcg_rm;
 TCGv_i64 tcg_addr;
@@ -2283,14 +2283,14 @@ static void disas_ldst_reg_roffset(DisasContext *s, 
uint32_t insn)
  * Rn: base address register (inc SP)
  * Rt: target register
  */
-static void disas_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn)
+static void disas_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn,
+int opc,
+int size,
+int rt,
+bool is_vector)
 {
-int rt = extract32(insn, 0, 5);
 int rn = extract32(insn, 5, 5);
 unsigned int imm12 = extract32(insn, 10, 12);
-bool is_vector = extract32(insn, 26, 1);
-int size = extract32(insn, 30, 2);
-int opc = extract32(insn, 22, 2);
 unsigned int offset;
 
 TCGv_i64 tcg_addr;
@@ -2349,20 +2349,25 @@ static void disas_ldst_reg_unsigned_imm(DisasContext 
*s, uint32_t insn)
 /* Load/store register (all forms) */
 static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
+int rt = extract32(insn, 0, 5);
+int opc = extract32(insn, 22, 2);
+bool is_vector = extract32(insn, 26, 1);
+int size = extract32(insn, 30, 2);
+
 switch (extract32(insn, 24, 2)) {
 case 0:
 if (extract32(insn, 21, 1) == 1 && extract32(insn, 10, 2) == 2) {
-disas_ldst_reg_roffset(s, insn);
+disas_ldst_reg_roffset(s, insn, opc, size, rt, is_vector);
 } else {
 /* Load/store register (unscaled immediate)
  * Load/store immediate pre/post-indexed
  * Load/store register unprivileged
  */
-disas_ldst_reg_imm9(s, insn);
+disas_ldst_reg_imm9(s, insn, opc, size, rt, is_vector);
 }
 break;
 case 1:
-disas_ldst_reg_unsigned_imm(s, insn);
+disas_ldst_reg_unsigned_imm(s, insn, opc, size, rt, is_vector);
 break;
 default:
 unallocated_encoding(s);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2] qemu-img: initialize MapEntry object

[Kevin Wolf]

Am 05.02.2016 um 20:56 hat Eric Blake geschrieben:
> On 02/05/2016 11:12 AM, John Snow wrote:
> > Commit 16b0d555 introduced an issue where we are not initializing
> > has_filename for the 'next' MapEntry object, which leads to interesting
> > errors in Valgrind and Clang -fsanitize=undefined both.
> 
> grammar:
> 
> errors in both Valgrind and Clang -fsanitize=undefined.

Fixed up.

> > 
> > Zero the stack object at allocation AND make sure the utility to
> > populate the fields properly marks has_filename as false if applicable.
> > 
> > Signed-off-by: John Snow 
> > ---
> > v2: Initialize with a compound literal as a future-proofing measure.
> > 
> 
> Reviewed-by: Eric Blake 

Thanks, applied to the block branch.

Kevin


pgpCxX69bLl2L.pgp
Description: PGP signature

[Qemu-devel] [PATCH v1 7/9] target-arm: Add the ARMInsnSyndrome type

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add the ARMInsnSyndrome type including helper functions to
encode and decode it into an u32. This is in preparation for
Instruction Syndrome generation for Data Aborts.

No functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/cpu.h   | 22 +++
 target-arm/translate.h | 57 ++
 2 files changed, 79 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5137632..a00a121 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -123,6 +123,28 @@ typedef struct {
 uint32_t base_mask;
 } TCR;
 
+/* Holds the state needed to create an instruction syndrome.  */
+typedef struct ARMInsnSyndrome {
+/* Data Abort section.  */
+struct {
+bool valid;
+unsigned int sas;
+bool sse;
+unsigned int srt;
+bool sf;
+bool ar;
+} dabt;
+
+/* SWStep section.  */
+struct {
+/* True if the insn just emitted was a load-exclusive instruction
+ * (necessary for syndrome information for single step exceptions),
+ * ie A64 LDX*, LDAX*, A32/T32 LDREX*, LDAEX*.
+ */
+bool ex;
+} swstep;
+} ARMInsnSyndrome;
+
 typedef struct CPUARMState {
 /* Regs for current mode.  */
 uint32_t regs[16];
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 53ef971..a94e17e 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -151,4 +151,61 @@ void arm_free_cc(DisasCompare *cmp);
 void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
 void arm_gen_test_cc(int cc, TCGLabel *label);
 
+
+/* The following describes the packing and unpacking of the Data Abort
+ * section of an ARMInsnSyndrome from/into an u32.
+ */
+
+/* Field widths.  */
+#define ISYN_WIDTH_valid 1
+#define ISYN_WIDTH_sas 2
+#define ISYN_WIDTH_sse 1
+#define ISYN_WIDTH_srt 5
+#define ISYN_WIDTH_sf  1
+#define ISYN_WIDTH_ar  1
+
+/* We use 64bit deposit to allow for overflow checking.  */
+#define ISYN_SHIFT_IN(val, isyn, field)  \
+{\
+unsigned int width = xglue(ISYN_WIDTH_, field);  \
+val <<= width;   \
+val = deposit64(val, 0, width, (isyn).field);\
+} while (0)
+
+#define ISYN_SHIFT_OUT(val, isyn, field) \
+{\
+unsigned int width = xglue(ISYN_WIDTH_, field);  \
+(isyn).field = extract32(val, 0, width); \
+val >>= width;   \
+} while (0)
+
+static inline uint32_t arm_encode_dabt_isyn_u32(ARMInsnSyndrome *isyn)
+{
+uint64_t v = 0;
+uint32_t v32;
+
+ISYN_SHIFT_IN(v, isyn->dabt, valid);
+ISYN_SHIFT_IN(v, isyn->dabt, sas);
+ISYN_SHIFT_IN(v, isyn->dabt, sse);
+ISYN_SHIFT_IN(v, isyn->dabt, srt);
+ISYN_SHIFT_IN(v, isyn->dabt, sf);
+ISYN_SHIFT_IN(v, isyn->dabt, ar);
+/* Check for overflows.  */
+v32 = v;
+assert(v32 == v);
+return v32;
+}
+
+static inline void arm_decode_dabt_isyn_u32(ARMInsnSyndrome *isyn, uint32_t v)
+{
+/* The fields must be shifted out in reverse order.  */
+ISYN_SHIFT_OUT(v, isyn->dabt, ar);
+ISYN_SHIFT_OUT(v, isyn->dabt, sf);
+ISYN_SHIFT_OUT(v, isyn->dabt, srt);
+ISYN_SHIFT_OUT(v, isyn->dabt, sse);
+ISYN_SHIFT_OUT(v, isyn->dabt, sas);
+ISYN_SHIFT_OUT(v, isyn->dabt, valid);
+assert(v == 0);
+}
+
 #endif /* TARGET_ARM_TRANSLATE_H */
-- 
1.9.1

[Qemu-devel] [PATCH v1 2/9] gen-icount: Use tcg_set_insn_param

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Use tcg_set_insn_param() instead of directly accessing internal
tcg data structures to update an insn param.

Reviewed-by: Richard Henderson 
Signed-off-by: Edgar E. Iglesias 
---
 include/exec/gen-icount.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 05d89d3..a011324 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -5,14 +5,13 @@
 
 /* Helpers for instruction counting code generation.  */
 
-static TCGArg *icount_arg;
+static int icount_start_insn_idx;
 static TCGLabel *icount_label;
 static TCGLabel *exitreq_label;
 
 static inline void gen_tb_start(TranslationBlock *tb)
 {
 TCGv_i32 count, flag, imm;
-int i;
 
 exitreq_label = gen_new_label();
 flag = tcg_temp_new_i32();
@@ -31,13 +30,12 @@ static inline void gen_tb_start(TranslationBlock *tb)
-ENV_OFFSET + offsetof(CPUState, icount_decr.u32));
 
 imm = tcg_temp_new_i32();
+/* We emit a movi with a dummy immediate argument. Keep the insn index
+ * of the movi so that we later (when we know the actual insn count)
+ * can update the immediate argument with the actual insn count.  */
+icount_start_insn_idx = tcg_op_buf_count();
 tcg_gen_movi_i32(imm, 0xdeadbeef);
 
-/* This is a horrid hack to allow fixing up the value later.  */
-i = tcg_ctx.gen_last_op_idx;
-i = tcg_ctx.gen_op_buf[i].args;
-icount_arg = _ctx.gen_opparam_buf[i + 1];
-
 tcg_gen_sub_i32(count, count, imm);
 tcg_temp_free_i32(imm);
 
@@ -53,7 +51,9 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns)
 tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_REQUESTED);
 
 if (tb->cflags & CF_USE_ICOUNT) {
-*icount_arg = num_insns;
+/* Update the num_insn immediate parameter now that we know
+ * the actual insn count.  */
+tcg_set_insn_param(icount_start_insn_idx, 1, num_insns);
 gen_set_label(icount_label);
 tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_ICOUNT_EXPIRED);
 }
-- 
1.9.1

[Qemu-devel] [PATCH v1 5/9] target-arm/translate-a64.c: Use extract32 in disas_ldst_reg_imm9

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Use extract32 instead of open coding the bit masking when decoding
is_signed and is_extended. This streamlines the decoding with some
of the other ldst variants.

No functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/translate-a64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 7f65aea..bf31f8a 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2117,8 +2117,8 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t 
insn)
 return;
 }
 is_store = (opc == 0);
-is_signed = opc & (1<<1);
-is_extended = (size < 3) && (opc & 1);
+is_signed = extract32(opc, 1, 1);
+is_extended = (size < 3) && extract32(opc, 0, 1);
 }
 
 switch (idx) {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v10] spec: add qcow2 bitmaps extension specification

[Kevin Wolf]

Am 05.02.2016 um 09:58 hat Vladimir Sementsov-Ogievskiy geschrieben:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

[Kevin Wolf]

Am 12.02.2016 um 14:19 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > Am 10.02.2016 um 13:05 hat Pavel Dovgalyuk geschrieben:
> > > > > > Am 09.02.2016 um 12:52 hat Pavel Dovgalyuk geschrieben:
> > > > > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > > > > But even this doesn't feel completely right, because block 
> > > > > > > > drivers are
> > > > > > > > already layered and there is no need to hardcode something 
> > > > > > > > optional (and
> > > > > > > > rarely used) in the hot code path that could just be another 
> > > > > > > > layer.
> > > > > > > >
> > > > > > > > I assume that you know beforehand if you want to replay 
> > > > > > > > something, so
> > > > > > > > requiring you to configure your block devices with a replay 
> > > > > > > > driver on
> > > > > > > > top of the stack seems reasonable enough.
> > > > > > >
> > > > > > > I cannot use block drivers for this. When driver functions are 
> > > > > > > used, QEMU
> > > > > > > is already used coroutines (and probably started bottom halves).
> > > > > > > Coroutines make execution non-deterministic.
> > > > > > > That's why we have to intercept blk_aio_ functions, that are 
> > > > > > > called
> > > > > > > deterministically.
> > > > > >
> > > > > > What does "deterministic" mean in this context, i.e. what are your 
> > > > > > exact
> > > > > > requirements?
> > > > >
> > > > > "Deterministic" means that the replayed execution should run exactly
> > > > > the same guest instructions in the same sequence, as in recording 
> > > > > session.
> > > >
> > > > Okay. I think with this we can do better than what you have now.
> > > >
> > > > > > I don't think that coroutines introduce anything non-deterministic 
> > > > > > per
> > > > > > se. Depending on what you mean by it, the block layer code paths in
> > > > > > block.c may contain problematic code.
> > > > >
> > > > > They are non-deterministic if we need instruction-level accuracy.
> > > > > Thread switching (and therefore callbacks and BH execution) is 
> > > > > non-deterministic.
> > > >
> > > > Thread switching depends on an external event (the kernel scheduler
> > > > deciding to switch), so agreed, if a thread switch ever influences what
> > > > the guest sees, that would be a problem.
> > > >
> > > > Generally, however, callbacks and BHs don't involve a thread switch at
> > > > all (BHs can be invoked from a different thread in theory, but we have
> > > > very few of those cases and they shouldn't be visible for the guest).
> > > > The same is true for coroutines, which are semantically equivalent to
> > > > callbacks.
> > > >
> > > > > In two different executions these callbacks may happen at different 
> > > > > moments of
> > > > > time (counting in number of executed instructions).
> > > > > All operations with virtual devices (including memory, interrupt 
> > > > > controller,
> > > > > and disk drive controller) should happen at deterministic moments of 
> > > > > time
> > > > > to be replayable.
> > > >
> > > > Right, so let's talk about what this external non-deterministic event
> > > > really is.
> > > >
> > > > I think the only thing whose timing is unknown in the block layer is the
> > > > completion of I/O requests. This non-determinism comes from the time the
> > > > I/O syscalls made by the lowest layer (usually raw-posix) take.
> > >
> > > Right.
> > >
> > > > This means that we can add logic to remove the non-determinism at the
> > > > point of our choice between raw-posix and the guest device emulation. A
> > > > block driver on top is as good as anything else.
> > > >
> > > > While recording, this block driver would just pass the request to next
> > > > lower layer (starting a request is deterministic, so it doesn't need to
> > > > be logged) and once the request completes it logs it. While replaying,
> > > > the completion of requests is delayed until we read it in the log; if we
> > > > read it in the log and the request hasn't completed yet, we do a busy
> > > > wait for it (while(!completed) aio_poll();).
> > >
> > > I tried serializing all bottom halves and worker thread callbacks in
> > > previous version of the patches. That code was much more complicated
> > > and error-prone than the current version. We had to classify all bottom
> > > halves to recorded and non-recorded (because sometimes they are used
> > > for qemu's purposes, not the guest ones).
> > >
> > > However, I don't understand yet which layer do you offer as the candidate
> > > for record/replay? What functions should be changed?
> > > I would like to investigate this way, but I don't got it yet.
> > 
> > At the core, I wouldn't change any existing function, but introduce a
> > new block driver. You could copy raw_bsd.c for a start and then tweak
> > it. Leave out functions that you don't want to support, and add the
> > 

[Qemu-devel] [PATCH v1 9/9] target-arm: Use isyn.swstep.ex to hold the is_ldex state

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Switch to using isyn.swstep.ex to hold the is_ldex state for
SWStep syndrome generation.

No functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/translate-a64.c | 6 +++---
 target-arm/translate.c | 6 +++---
 target-arm/translate.h | 5 -
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 2f17cba..1d7fbcb 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -260,7 +260,7 @@ static void gen_step_complete_exception(DisasContext *s)
  * of the exception, and our syndrome information is always correct.
  */
 gen_ss_advance(s);
-gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex),
+gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->isyn.swstep.ex),
   default_exception_el(s));
 s->is_jmp = DISAS_EXC;
 }
@@ -1865,7 +1865,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 
 if (is_excl) {
 if (!is_store) {
-s->is_ldex = true;
+s->isyn.swstep.ex = true;
 gen_load_exclusive(s, rt, rt2, tcg_addr, size, is_pair);
 } else {
 gen_store_exclusive(s, rs, rt, rt2, tcg_addr, size, is_pair);
@@ -11127,7 +11127,7 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
TranslationBlock *tb)
  */
 dc->ss_active = ARM_TBFLAG_SS_ACTIVE(tb->flags);
 dc->pstate_ss = ARM_TBFLAG_PSTATE_SS(tb->flags);
-dc->is_ldex = false;
+dc->isyn.swstep.ex = false;
 dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el);
 
 init_tmp_a64_array(dc);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 0d53e7d..605d21b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -250,7 +250,7 @@ static void gen_step_complete_exception(DisasContext *s)
  * of the exception, and our syndrome information is always correct.
  */
 gen_ss_advance(s);
-gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex),
+gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->isyn.swstep.ex),
   default_exception_el(s));
 s->is_jmp = DISAS_EXC;
 }
@@ -7431,7 +7431,7 @@ static void gen_load_exclusive(DisasContext *s, int rt, 
int rt2,
 {
 TCGv_i32 tmp = tcg_temp_new_i32();
 
-s->is_ldex = true;
+s->isyn.swstep.ex = true;
 
 switch (size) {
 case 0:
@@ -11284,7 +11284,7 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
  */
 dc->ss_active = ARM_TBFLAG_SS_ACTIVE(tb->flags);
 dc->pstate_ss = ARM_TBFLAG_PSTATE_SS(tb->flags);
-dc->is_ldex = false;
+dc->isyn.swstep.ex = false;
 dc->ss_same_el = false; /* Can't be true since EL_d must be AArch64 */
 
 cpu_F0s = tcg_temp_new_i32();
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 2130a84..d500342 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -49,11 +49,6 @@ typedef struct DisasContext {
  */
 bool ss_active;
 bool pstate_ss;
-/* True if the insn just emitted was a load-exclusive instruction
- * (necessary for syndrome information for single step exceptions),
- * ie A64 LDX*, LDAX*, A32/T32 LDREX*, LDAEX*.
- */
-bool is_ldex;
 /* True if a single-step exception will be taken to the current EL */
 bool ss_same_el;
 /* Bottom two bits of XScale c15_cpar coprocessor access control reg */
-- 
1.9.1

[Qemu-devel] [PATCH 3/4] hw/arm/virt: Load bios image to MemoryRegion, not physaddr

[Peter Maydell]

If we're loading a BIOS image into the first flash device,
load it into the flash's memory region specifically, not
into the physical address where the flash resides. This will
make a difference when the flash might be in the Secure
address space rather than the Nonsecure one.

Signed-off-by: Peter Maydell 
---
 hw/arm/virt.c | 39 ---
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5bdfe0f..391cf3f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -679,13 +679,14 @@ static void create_virtio_devices(const VirtBoardInfo 
*vbi, qemu_irq *pic)
 }
 
 static void create_one_flash(const char *name, hwaddr flashbase,
- hwaddr flashsize)
+ hwaddr flashsize, const char *file)
 {
 /* Create and map a single flash device. We use the same
  * parameters as the flash devices on the Versatile Express board.
  */
 DriveInfo *dinfo = drive_get_next(IF_PFLASH);
 DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 const uint64_t sectorlength = 256 * 1024;
 
 if (dinfo) {
@@ -705,19 +706,9 @@ static void create_one_flash(const char *name, hwaddr 
flashbase,
 qdev_prop_set_string(dev, "name", name);
 qdev_init_nofail(dev);
 
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, flashbase);
-}
-
-static void create_flash(const VirtBoardInfo *vbi)
-{
-/* Create two flash devices to fill the VIRT_FLASH space in the memmap.
- * Any file passed via -bios goes in the first of these.
- */
-hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
-hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
-char *nodename;
+sysbus_mmio_map(sbd, 0, flashbase);
 
-if (bios_name) {
+if (file) {
 char *fn;
 int image_size;
 
@@ -727,21 +718,31 @@ static void create_flash(const VirtBoardInfo *vbi)
  "but you cannot use both options at once");
 exit(1);
 }
-fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
 if (!fn) {
-error_report("Could not find ROM image '%s'", bios_name);
+error_report("Could not find ROM image '%s'", file);
 exit(1);
 }
-image_size = load_image_targphys(fn, flashbase, flashsize);
+image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
 g_free(fn);
 if (image_size < 0) {
-error_report("Could not load ROM image '%s'", bios_name);
+error_report("Could not load ROM image '%s'", file);
 exit(1);
 }
 }
+}
+
+static void create_flash(const VirtBoardInfo *vbi)
+{
+/* Create two flash devices to fill the VIRT_FLASH space in the memmap.
+ * Any file passed via -bios goes in the first of these.
+ */
+hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
+hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
+char *nodename;
 
-create_one_flash("virt.flash0", flashbase, flashsize);
-create_one_flash("virt.flash1", flashbase + flashsize, flashsize);
+create_one_flash("virt.flash0", flashbase, flashsize, bios_name);
+create_one_flash("virt.flash1", flashbase + flashsize, flashsize, NULL);
 
 nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
 qemu_fdt_add_subnode(vbi->fdt, nodename);
-- 
1.9.1

[Qemu-devel] [PATCH 0/4] virt: provide secure-only RAM and first flash

[Peter Maydell]

This patchset adds some more secure-only devices to the virt board:
 (1) a 16MB secure-only RAM
 (2) the first flash device is secure-only

The second of these is strictly speaking a breaking change, but I don't
expect it in practice to break anybody:
 (a) there's not much use of the secure support in virt yet
 (b) anything booting a rom image from that flash if TZ is enabled
  will be booting it in Secure mode anyway so will be able to access
  the code -- the only thing that would stop working would be if the
  guest flipped to NS and still expected to be able to access the flash

The second flash device remains NS-accessible (with the expectation that
it will be used for NS UEFI environment variable storage).

In particular, the ATF+OPTEE+UEFI+Linux stack still works fine with
these changes.


NOTE: to get the -bios option to correctly load to the secure-only
flash I had to implement a new function in loader.c. load_image_mr()
is just like load_image_targphys() except that it requests loading
to a MemoryRegion rather than a physaddr. I think we can also use this
to clean up the Sparc cg3 and tcx display devices, which currently take
a qdev property which is "the address I'm going to be mapped at"
purely so they can use load_image_targphys() to load their ROMs.

I have to say I found the loader.c code a bit confusing (it has some
support for "load image to MR" already, but it seems to be tangled
up with the fw_cfg and PC option rom support); review of that
patch in particular appreciated.

thanks
-- PMM

Peter Maydell (4):
  hw/arm/virt: Provide a secure-only RAM if booting in Secure mode
  loader: Add load_image_mr() to load ROM image to a MemoryRegion
  hw/arm/virt: Load bios image to MemoryRegion, not physaddr
  hw/arm/virt: Make first flash device Secure-only if booting secure

 hw/arm/virt.c | 118 ++
 hw/core/loader.c  |  35 +--
 include/hw/arm/virt.h |   1 +
 include/hw/loader.h   |  18 +++-
 4 files changed, 138 insertions(+), 34 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/4] hw/arm/virt: Provide a secure-only RAM if booting in Secure mode

[Peter Maydell]

If we're booting in Secure mode, provide a secure-only RAM
(just 16MB) so that secure firmware has somewhere to run
from that won't be accessible to the Non-secure guest.

Signed-off-by: Peter Maydell 
---
 hw/arm/virt.c | 26 ++
 include/hw/arm/virt.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 44bbbea..5bdfe0f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -127,6 +127,7 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
+[VIRT_SECURE_MEM] = { 0x0e00, 0x0100 },
 [VIRT_PCIE_MMIO] =  { 0x1000, 0x2eff },
 [VIRT_PCIE_PIO] =   { 0x3eff, 0x0001 },
 [VIRT_PCIE_ECAM] =  { 0x3f00, 0x0100 },
@@ -960,6 +961,30 @@ static void create_platform_bus(VirtBoardInfo *vbi, 
qemu_irq *pic)
 sysbus_mmio_get_region(s, 0));
 }
 
+static void create_secure_ram(VirtBoardInfo *vbi, MemoryRegion *secure_sysmem)
+{
+MemoryRegion *secram = g_new(MemoryRegion, 1);
+char *nodename;
+hwaddr base = vbi->memmap[VIRT_SECURE_MEM].base;
+hwaddr size = vbi->memmap[VIRT_SECURE_MEM].size;
+
+memory_region_init_ram(secram, NULL, "virt.secure-ram",
+   size, _fatal);
+vmstate_register_ram_global(secram);
+memory_region_add_subregion(secure_sysmem, base, secram);
+
+nodename = g_strdup_printf("/secram@%" PRIx64, base);
+qemu_fdt_add_subnode(vbi->fdt, nodename);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type",
+"memory");
+qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+ 2, base, 2, size);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "status", "disabled");
+qemu_fdt_setprop_string(vbi->fdt, nodename, "secure-status", "okay");
+
+g_free(nodename);
+}
+
 static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 {
 const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
@@ -1152,6 +1177,7 @@ static void machvirt_init(MachineState *machine)
 create_uart(vbi, pic, VIRT_UART, sysmem);
 
 if (vms->secure) {
+create_secure_ram(vbi, secure_sysmem);
 create_uart(vbi, pic, VIRT_SECURE_UART, secure_sysmem);
 }
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 1ce7847..ecd8589 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -61,6 +61,7 @@ enum {
 VIRT_PCIE_MMIO_HIGH,
 VIRT_GPIO,
 VIRT_SECURE_UART,
+VIRT_SECURE_MEM,
 };
 
 typedef struct MemMapEntry {
-- 
1.9.1

Re: [Qemu-devel] broken HMP command: info mtree

[Daniel P. Berrange]

On Fri, Feb 12, 2016 at 02:27:23PM +0100, Igor Mammedov wrote:
> On Fri, 12 Feb 2016 14:08:32 +0100
> Paolo Bonzini  wrote:
> 
> > On 12/02/2016 12:17, Daniel P. Berrange wrote:
> > > On Fri, Feb 12, 2016 at 12:15:26PM +0100, Igor Mammedov wrote:  
> > >> On Thu, 11 Feb 2016 16:35:39 +0100
> > >> Igor Mammedov  wrote:
> > >>  
> > >>> executing 'info mtree' from monitor prompt causes infinite loop
> > >>> printing it over and over.
> > >>>
> > >>> to reproduce build current master adn run:
> > >>>
> > >>> qemu-system-x86_64 -monitor stdio
> > >>>
> > >>> and then execute 'info mtree' in monitor prompt  
> > >>
> > >> it bisects to:
> > >>
> > >> commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2
> > >> Author: Daniel P. Berrange 
> > >> Date:   Tue Jan 19 11:14:29 2016 +
> > >>
> > >> char: convert from GIOChannel to QIOChannel
> > >> 
> > >> In preparation for introducing TLS support to the TCP chardev
> > >> backend, convert existing chardev code from using GIOChannel
> > >> to QIOChannel. This simplifies the chardev code by removing
> > >> most of the OS platform conditional code for dealing with
> > >> file descriptor passing.
> > >> 
> > >> Signed-off-by: Daniel P. Berrange 
> > >> Message-Id: <1453202071-10289-3-git-send-email-berra...@redhat.com>
> > >> Signed-off-by: Paolo Bonzini 
> > >>
> > >>
> > >> build with:
> > >> ./configure --target-list=x86_64-softmmu --enable-debug 
> > >> on RHEL72ish host
> > >>
> > >> monitor output has to be stdio  
> > > 
> > > Sigh, so much pain from the chardev code. I'll investigate and send a
> > > suitable patch asap.  
> > 
> > Hmm, I cannot reproduce this though.
> Perhaps I'm affected because my stdout goes via remote ssh session.
> 
> It looks like monitor tries to flush buffer but succeeds only partially
> and returns with EAGAIN and on the next flush attempt it tries to
> flush the same buffer again from the first byte again

Yes, it turned out to need a moderately slow console like a distant
ssh connection to trigger the EGAIN situation. I've just sent a patch.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v6 00/16] Implement TLS support to QEMU NBD server & client

[Daniel P. Berrange]

On Fri, Feb 12, 2016 at 02:28:33PM +0100, Kashyap Chamarthy wrote:
> On Wed, Feb 10, 2016 at 06:40:58PM +, Daniel P. Berrange wrote:
> 
> [...]
> 
> I've applied all the series in this patches, to yesterday's Git master,
> so I'm here:
> 
> $ git describe
> pull-qcrypto-next-2016-02-02-1-378-gf9375d2
> 
> > Starting a QEMU system emulator built-in NBD server
> > 
> >   $ qemu-system-x86_64 \
> >  -qmp unix:/tmp/qmp,server \
> >  -hda /home/berrange/Fedora-Server-netinst-x86_64-23.iso \
> >  -object 
> > tls-creds-x509,id=tls0,dir=/home/berrange/security/qemutls,endpoint=server
> 
> Instead of an ISO, I have this command-line:
> 
> $QEMU \
>   -display none \
>   -nodefconfig \
>   -nodefaults \
>   -m 2048 \
>   -device virtio-scsi-pci,id=scsi \
>   -device virtio-serial-pci \
>   -serial stdio \
>   -drive file=./cirros-0.3.3.qcow2,format=qcow2,if=virtio \
>   -object 
> tls-creds-x509,id=tls0,dir=/export/security/gnutls,endpoint=server \
>   -qmp unix:./qmp-sock,server
> 
> >   $ qmp-shell /tmp/qmp
> >  (qmp) nbd-server-start addr={"host":"localhost","port":"9000"}
> >  tls-creds=tls0
> 
> However, this invocation seem to work for me, am I doing something wrong?
> 
> $ ./qmp-shell ./qmp-sock
> Welcome to the QMP low-level shell!
> Connected to QEMU 2.5.50
>   
> (QEMU) nbd-server-start addr={'host:'localhost','port':'9000'} 
> tls-creds=tls0
> {"error": {"class": "GenericError", "desc": "Invalid parameter type for 
> 'addr', expected: QDict"}}
> (QEMU)

Yes, my bad - I provided the wrong syntax here. it should be

  (QEMU) nbd-server-start 
addr={"type":"inet","data":{"host":"localhost","port":"9000"}} tls-creds=tls0
  {"return": {}}
  (QEMU) nbd-server-add device=ide0-hd0
  {"return": {}}

I've genuinely tested it this time :-)

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 2/4] target-arm: Move get/set_r13_banked() to op_helper.c

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 07:11:47PM +, Peter Maydell wrote:
> Move get/set_r13_banked() from helper.c to op_helper.c. This will
> let us add exception-raising code to them, and also puts them
> in the same file as get/set_user_reg(), which makes some conceptual
> sense.
> 
> (The original reason for the helper.c/op_helper.c split was that
> only op_helper.c had access to the CPU env pointer; this distinction
> has not been true for a long time, though, and so the split is
> now rather arbitrary.)
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target-arm/helper.c| 33 -
>  target-arm/op_helper.c | 37 +
>  2 files changed, 37 insertions(+), 33 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index bb913c6..c46e3d0 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5365,21 +5365,6 @@ void switch_mode(CPUARMState *env, int mode)
>  }
>  }
>  
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 write\n");
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 read\n");
> -return 0;
> -}
> -
>  uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
>   uint32_t cur_el, bool secure)
>  {
> @@ -7762,24 +7747,6 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, 
> vaddr addr,
>  return phys_addr;
>  }
>  
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -if ((env->uncached_cpsr & CPSR_M) == mode) {
> -env->regs[13] = val;
> -} else {
> -env->banked_r13[bank_number(mode)] = val;
> -}
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -if ((env->uncached_cpsr & CPSR_M) == mode) {
> -return env->regs[13];
> -} else {
> -return env->banked_r13[bank_number(mode)];
> -}
> -}
> -
>  uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>  {
>  ARMCPU *cpu = arm_env_get_cpu(env);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 049b521..053e9b6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -457,6 +457,43 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
> regno, uint32_t val)
>  }
>  }
>  
> +#if defined(CONFIG_USER_ONLY)
> +void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> +{
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +
> +cpu_abort(CPU(cpu), "banked r13 write\n");
> +}
> +
> +uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> +{
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +
> +cpu_abort(CPU(cpu), "banked r13 read\n");
> +return 0;
> +}
> +
> +#else
> +
> +void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> +{
> +if ((env->uncached_cpsr & CPSR_M) == mode) {
> +env->regs[13] = val;
> +} else {
> +env->banked_r13[bank_number(mode)] = val;
> +}
> +}
> +
> +uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> +{
> +if ((env->uncached_cpsr & CPSR_M) == mode) {
> +return env->regs[13];
> +} else {
> +return env->banked_r13[bank_number(mode)];
> +}
> +}
> +#endif
> +
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
> syndrome,
>   uint32_t isread)
>  {
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Edgar E. Iglesias]

On Fri, Feb 12, 2016 at 04:12:18PM +0100, Edgar E. Iglesias wrote:
> On Thu, Feb 11, 2016 at 07:11:48PM +, Peter Maydell wrote:
> > The user-mode versions of get/set_r13_banked() exist just to assert
> > if they're ever called -- the translate time code should never
> > emit calls to them because SRS from user mode always UNDEF.
> > There's no code in the softmmu versions that can't compile in
> > CONFIG_USER_ONLY, so combine the two functions rather than
> > having completely split versions under ifdefs.
> > 
> > Signed-off-by: Peter Maydell 
> 
> Hi Peter,
> 
> Do we really need the assert?
> If we keep it, can't we have it for both -user and -softmmu (avoiding the 
> ifdef)?

e.g:

assert(arm_current_el(env) != 0);

Allthough, I'd probably vote for removing it...

Cheers,
Edgar



> 
> Cheers,
> Edgar
> 
> 
> 
> > ---
> >  target-arm/op_helper.c | 25 ++---
> >  1 file changed, 6 insertions(+), 19 deletions(-)
> > 
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index 053e9b6..05f97a7 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -457,26 +457,11 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
> > regno, uint32_t val)
> >  }
> >  }
> >  
> > -#if defined(CONFIG_USER_ONLY)
> > -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> > -{
> > -ARMCPU *cpu = arm_env_get_cpu(env);
> > -
> > -cpu_abort(CPU(cpu), "banked r13 write\n");
> > -}
> > -
> > -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> > -{
> > -ARMCPU *cpu = arm_env_get_cpu(env);
> > -
> > -cpu_abort(CPU(cpu), "banked r13 read\n");
> > -return 0;
> > -}
> > -
> > -#else
> > -
> >  void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> >  {
> > +#if defined(CONFIG_USER_ONLY)
> > +g_assert_not_reached();
> > +#endif
> >  if ((env->uncached_cpsr & CPSR_M) == mode) {
> >  env->regs[13] = val;
> >  } else {
> > @@ -486,13 +471,15 @@ void HELPER(set_r13_banked)(CPUARMState *env, 
> > uint32_t mode, uint32_t val)
> >  
> >  uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> >  {
> > +#if defined(CONFIG_USER_ONLY)
> > +g_assert_not_reached();
> > +#endif
> >  if ((env->uncached_cpsr & CPSR_M) == mode) {
> >  return env->regs[13];
> >  } else {
> >  return env->banked_r13[bank_number(mode)];
> >  }
> >  }
> > -#endif
> >  
> >  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
> > syndrome,
> >   uint32_t isread)
> > -- 
> > 1.9.1
> > 

Re: [Qemu-devel] [PATCH] Keep pty slave file descriptor open until the master is closed

[Marc-André Lureau]

Hi

On Fri, Feb 12, 2016 at 3:29 AM, Marc-André Lureau
 wrote:
> Hi
>
> On Mon, Jan 11, 2016 at 10:13 AM, Paolo Bonzini  wrote:
>>
>>
>> On 11/01/2016 09:33, Michael Tokarev wrote:
>>> 11.12.2015 14:29, Ashley Jonathan wrote:
 I have experienced a minor difficulty using QEMU with the "-serial pty" 
 option:

 If a process opens the slave pts device, writes data to it, then 
 immediately closes it, the data doesn't reliably get delivered to the 
 emulated serial port. This seems to be because a read of the master pty 
 device returns EIO on Linux if no process has the pts device open, even 
 when data is waiting "in the pipe".

 A fix seems to be for QEMU to keep the pts file descriptor open until the 
 pty is closed, as per the below patch.
>>>
>>> The patch looks fine, so
>>>
>>> Reviewed-by: Michael Tokarev 
>>>
>>> but I'd love to have an ACK from the maintainer about this one,
>>> or for it to pick it up.
>>
>> Ok, I'll pick it up after I've read up a bit more on PTYs.
>
> That patch slows down qemu a lot when using -chardev
> pty,id=charserial0 -device isa-serial,chardev=charserial0. I forgot a
> lot about how pty/pts work, and reading some man pages didn't help me
> much to understand the issue, I would suggest to revert it until a
> better solution is found.

It looks like if a the slave is opened, then Linux will buffer the
master writes, up to a few kb and then throttle, so it's not entirely
blocked but eventually the guest VM dies. However, not having any
slave open it will simply let the write go and discard the data. At
least, virt-install configures a pty for the serial but viewers like
virt-manager do not necessarily open it. And, if there are no viewers,
it will just hang. If qemu starts reading all the data from the slave,
I don't think interactions with other slaves will work. I don't see
much options but to close the slave, thus reverting this patch.

Regarding the original bug, I wonder if the slave writing process
couldn't simply fsync/fdatasync (I guess you tried that already)
Probably better to interact with the guest with something else than a
terminal though..


-- 
Marc-André Lureau

[Qemu-devel] [PATCH v1 0/9] arm: Steps towards EL2 support round 6

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Hi,

Another round of patches towards EL2 support. This one adds partial
Instruction Syndrome generation for Data Aborts while running in AArch64.

I don't feel very confident with the way I collect the regsize info used
to fill out the SF field. Feedback on that would be great.

Once we sort out the details on how this should be implemented we can
fill out the parts needed for AArch32. Possibly in a future version of
this same series.

Comments welcome!

Best regards,
Edgar

Edgar E. Iglesias (9):
  tcg: Add tcg_set_insn_param
  gen-icount: Use tcg_set_insn_param
  target-arm: Add the thumb/IL flag to syn_data_abort
  target-arm: Add more fields to the data abort syndrome generator
  target-arm/translate-a64.c: Use extract32 in disas_ldst_reg_imm9
  target-arm/translate-a64.c: Unify some of the ldst_reg decoding
  target-arm: Add the ARMInsnSyndrome type
  target-arm: A64: Create Instruction Syndromes for Data Aborts
  target-arm: Use isyn.swstep.ex to hold the is_ldex state

 include/exec/gen-icount.h  |  16 +++
 target-arm/cpu.h   |  27 ++-
 target-arm/internals.h |  20 ++--
 target-arm/op_helper.c |  40 +++-
 target-arm/translate-a64.c | 114 +++--
 target-arm/translate.c |  11 +++--
 target-arm/translate.h |  65 --
 tcg/tcg.h  |   6 +++
 8 files changed, 252 insertions(+), 47 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v1 3/9] target-arm: Add the thumb/IL flag to syn_data_abort

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/internals.h | 4 +++-
 target-arm/op_helper.c | 6 --
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-arm/internals.h b/target-arm/internals.h
index 70bec4a..b1c483b 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -360,9 +360,11 @@ static inline uint32_t syn_insn_abort(int same_el, int ea, 
int s1ptw, int fsc)
 }
 
 static inline uint32_t syn_data_abort(int same_el, int ea, int cm, int s1ptw,
-  int wnr, int fsc)
+  int wnr, int fsc,
+  bool is_thumb)
 {
 return (EC_DATAABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
+| (is_thumb ? 0 : ARM_EL_IL)
 | (ea << 9) | (cm << 8) | (s1ptw << 7) | (wnr << 6) | fsc;
 }
 
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index bd48549..4e629e1 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -115,7 +115,8 @@ void tlb_fill(CPUState *cs, target_ulong addr, int 
is_write, int mmu_idx,
 syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
 exc = EXCP_PREFETCH_ABORT;
 } else {
-syn = syn_data_abort(same_el, 0, 0, fi.s1ptw, is_write == 1, syn);
+syn = syn_data_abort(same_el, 0, 0, fi.s1ptw, is_write == 1, syn,
+ env->thumb);
 if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
 fsr |= (1 << 11);
 }
@@ -161,7 +162,8 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, 
int is_write,
 }
 
 raise_exception(env, EXCP_DATA_ABORT,
-syn_data_abort(same_el, 0, 0, 0, is_write == 1, 0x21),
+syn_data_abort(same_el, 0, 0, 0, is_write == 1, 0x21,
+   env->thumb),
 target_el);
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH v1 1/9] tcg: Add tcg_set_insn_param

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add tcg_set_insn_param as a mechanism to modify an insn
parameter after emiting the insn. This is useful for icount
and also for embedding fault information for a specific insn.

Reviewed-by: Richard Henderson 
Signed-off-by: Edgar E. Iglesias 
---
 tcg/tcg.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 83da5fb..00dd124 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -585,6 +585,12 @@ struct TCGContext {
 
 extern TCGContext tcg_ctx;
 
+static inline void tcg_set_insn_param(int op_idx, int arg, TCGArg v)
+{
+int op_argi = tcg_ctx.gen_op_buf[op_idx].args;
+tcg_ctx.gen_opparam_buf[op_argi + arg] = v;
+}
+
 /* The number of opcodes emitted so far.  */
 static inline int tcg_op_buf_count(void)
 {
-- 
1.9.1

[Qemu-devel] [PATCH v1 8/9] target-arm: A64: Create Instruction Syndromes for Data Aborts

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add support for generating the instruction syndrome for Data Aborts.
These syndromes are used by hypervisors for example to trap and emulate
memory accesses.

We save the decoded data out-of-band with the TBs at translation time.
When exceptions hit, the extra data attached to the TB is used to
recreate the state needed to encode instruction syndromes.
This avoids the need to emit moves with every load/store.

Based on a suggestion from Peter Maydell.

Suggested-by: Peter Maydell 
Signed-off-by: Edgar E. Iglesias 
---
 target-arm/cpu.h   |  5 +++-
 target-arm/op_helper.c | 34 +++--
 target-arm/translate-a64.c | 63 +-
 target-arm/translate.c |  5 +++-
 target-arm/translate.h |  3 +++
 5 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index a00a121..fecc48f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -95,7 +95,7 @@
 struct arm_boot_info;
 
 #define NB_MMU_MODES 7
-#define TARGET_INSN_START_EXTRA_WORDS 1
+#define TARGET_INSN_START_EXTRA_WORDS 2
 
 /* We currently assume float and double are IEEE single and double
precision respectively.
@@ -196,6 +196,9 @@ typedef struct CPUARMState {
 uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
 uint64_t daif; /* exception masks, in the bits they are in PSTATE */
 
+/* IS state from decoded instructions. Only valid after Data aborts.  */
+ARMInsnSyndrome isyn;
+
 uint64_t elr_el[4]; /* AArch64 exception link regs  */
 uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9bf635f..b195848 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -115,8 +115,23 @@ void tlb_fill(CPUState *cs, target_ulong addr, int 
is_write, int mmu_idx,
 syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
 exc = EXCP_PREFETCH_ABORT;
 } else {
+if (target_el != 2 || fi.s1ptw) {
+/* ISV is only set for data aborts routed to EL2 and
+ * never for S1PTWalks faulting on Stage 2.
+ *
+ * See ARMv8 specs:
+ * ISS encoding for an exception from a Data Abort, the
+ * ISV field.
+ */
+env->isyn.dabt.valid = 0;
+}
 syn = syn_data_abort(same_el,
- 0, 0, 0, 0, 0, 0,
+ env->isyn.dabt.valid,
+ env->isyn.dabt.sas,
+ env->isyn.dabt.sse,
+ env->isyn.dabt.srt,
+ env->isyn.dabt.sf,
+ env->isyn.dabt.ar,
  0, 0, fi.s1ptw, is_write == 1, syn,
  env->thumb);
 if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
@@ -163,9 +178,24 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr 
vaddr, int is_write,
 env->exception.fsr |= (1 << 11);
 }
 
+if (target_el != 2) {
+/* ISV is only set for data aborts routed to EL2 and
+ * never for S1PTWalks faulting on Stage 2.
+ *
+ * See ARMv8 specs:
+ * ISS encoding for an exception from a Data Abort, the
+ * ISV field.
+ */
+env->isyn.dabt.valid = 0;
+}
 raise_exception(env, EXCP_DATA_ABORT,
 syn_data_abort(same_el,
-   0, 0, 0, 0, 0, 0,
+   env->isyn.dabt.valid,
+   env->isyn.dabt.sas,
+   env->isyn.dabt.sse,
+   env->isyn.dabt.srt,
+   env->isyn.dabt.sf,
+   env->isyn.dabt.ar,
0, 0, 0, is_write == 1, 0x21,
env->thumb),
 target_el);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9e26d5e..2f17cba 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1803,6 +1803,24 @@ static void gen_store_exclusive(DisasContext *s, int rd, 
int rt, int rt2,
 }
 #endif
 
+static void disas_ldr_update_isyn_sse_sf(DisasContext *s, int size,
+bool is_signed, int opc)
+{
+int opc0 = extract32(opc, 0, 1);
+int regsize;
+
+s->isyn.dabt.sse = is_signed;
+/* Update the Sixty-Four bit (SF) registersize. This logic is derived
+ * from the ARMv8 specs for LDR (Shared decode for all encodings).
+ */
+if (is_signed) {
+regsize = opc0 ? 32 : 64;
+} else {
+regsize = size == 3 ? 64 : 32;
+}
+ 

[Qemu-devel] [PATCH] MAINTAINERS: add machine section

[Marcel Apfelbaum]

Eduardo and me will maintain it.

Signed-off-by: Marcel Apfelbaum 
Acked-by: Andreas Färber 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d6ee17..a86491a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1200,6 +1200,13 @@ F: docs/*qmp-*
 F: scripts/qmp/
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
+Machine
+M: Eduardo Habkost 
+M: Marcel Apfelbaum 
+S: Supported
+F: hw/core/machine.c
+F: include/hw/boards.h
+
 SLIRP
 M: Jan Kiszka 
 S: Maintained
-- 
2.4.3

[Qemu-devel] [PATCH 4/4] hw/arm/virt: Make first flash device Secure-only if booting secure

[Peter Maydell]

If the virt board is started with the 'secure' property set to
request a Secure setup, then make the first flash device be
visible only to the Secure world.

This is a breaking change, but I don't expect it to be noticed
by anybody, because running TZ-aware guests isn't common and
those guests are generally going to be booting from the flash
and implicitly expecting their Non-secure guests to not touch it.

Signed-off-by: Peter Maydell 
---
 hw/arm/virt.c | 63 ++-
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 391cf3f..4499e2c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -679,7 +679,8 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, 
qemu_irq *pic)
 }
 
 static void create_one_flash(const char *name, hwaddr flashbase,
- hwaddr flashsize, const char *file)
+ hwaddr flashsize, const char *file,
+ MemoryRegion *sysmem)
 {
 /* Create and map a single flash device. We use the same
  * parameters as the flash devices on the Versatile Express board.
@@ -706,7 +707,8 @@ static void create_one_flash(const char *name, hwaddr 
flashbase,
 qdev_prop_set_string(dev, "name", name);
 qdev_init_nofail(dev);
 
-sysbus_mmio_map(sbd, 0, flashbase);
+memory_region_add_subregion(sysmem, flashbase,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 
0));
 
 if (file) {
 char *fn;
@@ -732,26 +734,59 @@ static void create_one_flash(const char *name, hwaddr 
flashbase,
 }
 }
 
-static void create_flash(const VirtBoardInfo *vbi)
+static void create_flash(const VirtBoardInfo *vbi,
+ MemoryRegion *sysmem,
+ MemoryRegion *secure_sysmem)
 {
 /* Create two flash devices to fill the VIRT_FLASH space in the memmap.
  * Any file passed via -bios goes in the first of these.
+ * sysmem is the system memory space. secure_sysmem is the secure view
+ * of the system, and the first flash device should be made visible only
+ * there. The second flash device is visible to both secure and nonsecure.
+ * If sysmem == secure_sysmem this means there is no separate Secure
+ * address space and both flash devices are generally visible.
  */
 hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
 hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
 char *nodename;
 
-create_one_flash("virt.flash0", flashbase, flashsize, bios_name);
-create_one_flash("virt.flash1", flashbase + flashsize, flashsize, NULL);
+create_one_flash("virt.flash0", flashbase, flashsize,
+ bios_name, secure_sysmem);
+create_one_flash("virt.flash1", flashbase + flashsize, flashsize,
+ NULL, sysmem);
 
-nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
-qemu_fdt_add_subnode(vbi->fdt, nodename);
-qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
-qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
- 2, flashbase, 2, flashsize,
- 2, flashbase + flashsize, 2, flashsize);
-qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
-g_free(nodename);
+if (sysmem == secure_sysmem) {
+/* Report both flash devices as a single node in the DT */
+nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
+qemu_fdt_add_subnode(vbi->fdt, nodename);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
+qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+ 2, flashbase, 2, flashsize,
+ 2, flashbase + flashsize, 2, flashsize);
+qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
+g_free(nodename);
+} else {
+/* Report the devices as separate nodes so we can mark one as
+ * only visible to the secure world.
+ */
+nodename = g_strdup_printf("/secflash@%" PRIx64, flashbase);
+qemu_fdt_add_subnode(vbi->fdt, nodename);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
+qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+ 2, flashbase, 2, flashsize);
+qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "status", "disabled");
+qemu_fdt_setprop_string(vbi->fdt, nodename, "secure-status", "okay");
+g_free(nodename);
+
+nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
+qemu_fdt_add_subnode(vbi->fdt, nodename);
+qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
+qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+ 

[Qemu-devel] [PATCH] char: fix handling of QIO_CHANNEL_ERR_BLOCK

[Daniel P. Berrange]

If io_channel_send_full gets QIO_CHANNEL_ERR_BLOCK it
and has already sent some of the data, it should return
that amount of data, not EAGAIN, as that would cause
the caller to re-try already sent data.

Unfortunately due to a previous rebase conflict resolution
error, the code for dealing with this was in the wrong
part of the conditional, and so mistakenly ran on other
I/O errors.

This be seen running

   qemu-system-x86_64 -monitor stdio

and entering 'info mtree', when running on a slow console
(eg a slow remote ssh session). The monitor would get into
an indefinite loop writing the same data until it managed
to send it all without getting EAGAIN.

Reported-by: Igor Mammedov 
Signed-off-by: Daniel P. Berrange 
---
 qemu-char.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 1b7d5da..c2e24a5 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -896,13 +896,13 @@ static int io_channel_send_full(QIOChannel *ioc,
 ioc, , 1,
 fds, nfds, NULL);
 if (ret == QIO_CHANNEL_ERR_BLOCK) {
-errno = EAGAIN;
-return -1;
-} else if (ret < 0) {
 if (offset) {
 return offset;
 }
 
+errno = EAGAIN;
+return -1;
+} else if (ret < 0) {
 errno = EINVAL;
 return -1;
 }
-- 
2.5.0

[Qemu-devel] [PATCH] MAINTAINERS: Drop target-i386 from CPU subsystem

[Andreas Färber]

X86CPU QOM type is in good hands and actively maintained these days, so
drop it from the generic QOM CPU subsystem.

Some refactorings and design questions will still intersect, but review
and discussions of individual series can still take place while opting out
of general X86CPU patch review.

Cc: Eduardo Habkost 
Signed-off-by: Andreas Färber 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 02710f8..c9b5893 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1032,7 +1032,6 @@ M: Andreas Färber 
 S: Supported
 F: qom/cpu.c
 F: include/qom/cpu.h
-F: target-i386/cpu.c
 
 ICC Bus
 M: Igor Mammedov 
-- 
2.6.2

Re: [Qemu-devel] [PATCH COLO-Frame v14 24/40] COLO: Process shutdown command for VM in COLO state

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> If VM is in COLO FT state, we should do some extra work before normal shutdown
> process. SVM will ignore the shutdown command if this command is issued 
> directly
> to it, PVM will send the shutdown command to SVM if it gets this command.
> 
> Cc: Paolo Bonzini 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 

Reviewed-by: Dr. David Alan Gilbert 

although one question below.

Dave

> ---
> v14:
> - Remove 'colo_shutdown' variable, use colo_shutdown_request directly
> v13:
> - Move COLO shutdown related codes to colo.c file (Dave's suggestion)
> ---
>  include/migration/colo.h |  2 ++
>  include/sysemu/sysemu.h  |  3 +++
>  migration/colo.c | 42 --
>  qapi-schema.json |  4 +++-
>  stubs/migration-colo.c   |  5 +
>  vl.c | 19 ---
>  6 files changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/include/migration/colo.h b/include/migration/colo.h
> index e32eef4..919b135 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -35,4 +35,6 @@ COLOMode get_colo_mode(void);
>  
>  /* failover */
>  void colo_do_failover(MigrationState *s);
> +
> +bool colo_shutdown(void);
>  #endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 3bb8897..91eeda3 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -52,6 +52,8 @@ typedef enum WakeupReason {
>  QEMU_WAKEUP_REASON_OTHER,
>  } WakeupReason;
>  
> +extern int colo_shutdown_requested;
> +
>  void qemu_system_reset_request(void);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
> @@ -59,6 +61,7 @@ void qemu_system_wakeup_request(WakeupReason reason);
>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
>  void qemu_system_shutdown_request(void);
> +void qemu_system_shutdown_request_core(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_system_debug_request(void);
> diff --git a/migration/colo.c b/migration/colo.c
> index 515d561..92be985 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -330,6 +330,18 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  goto out;
>  }
>  
> +if (colo_shutdown_requested) {
> +colo_put_cmd(s->to_dst_file, COLO_MESSAGE_GUEST_SHUTDOWN, 
> _err);
> +if (local_err) {
> +goto out;
> +}

I wonder what actually happens in this case? Does it eventually shutdown?

> +qemu_fflush(s->to_dst_file);
> +colo_shutdown_requested = 0;
> +qemu_system_shutdown_request_core();
> +/* Fix me: Just let the colo thread exit ? */
> +qemu_thread_exit(0);
> +}
> +
>  ret = 0;
>  /* Resume primary guest */
>  qemu_mutex_lock_iothread();
> @@ -390,8 +402,9 @@ static void colo_process_checkpoint(MigrationState *s)
>  }
>  
>  current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> -if (current_time - checkpoint_time <
> -s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY]) {
> +if ((current_time - checkpoint_time <
> +s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY]) &&
> +!colo_shutdown_requested) {
>  int64_t delay_ms;
>  
>  delay_ms = s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY] 
> -
> @@ -465,6 +478,15 @@ static void colo_wait_handle_cmd(QEMUFile *f, int 
> *checkpoint_request,
>  case COLO_MESSAGE_CHECKPOINT_REQUEST:
>  *checkpoint_request = 1;
>  break;
> +case COLO_MESSAGE_GUEST_SHUTDOWN:
> +qemu_mutex_lock_iothread();
> +vm_stop_force_state(RUN_STATE_COLO);
> +qemu_system_shutdown_request_core();
> +qemu_mutex_unlock_iothread();
> +/* the main thread will exit and terminate the whole
> +* process, do we need some cleanup?
> +*/
> +qemu_thread_exit(0);
>  default:
>  *checkpoint_request = 0;
>  error_setg(errp, "Got unknown COLO command: %d", cmd);
> @@ -636,3 +658,19 @@ out:
>  
>  return NULL;
>  }
> +
> +bool colo_shutdown(void)
> +{
> +/*
> +* if in colo mode, we need do some significant work before respond
> +* to the shutdown request.
> +*/
> +if (migration_incoming_in_colo_state()) {
> +return true; /* primary's responsibility */
> +}
> +if (migration_in_colo_state()) {
> +colo_shutdown_requested = 1;
> +return true;
> +}
> +return false;
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7fec696..4d8ba04 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -752,12 +752,14 @@
>  

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 07:11:48PM +, Peter Maydell wrote:
> The user-mode versions of get/set_r13_banked() exist just to assert
> if they're ever called -- the translate time code should never
> emit calls to them because SRS from user mode always UNDEF.
> There's no code in the softmmu versions that can't compile in
> CONFIG_USER_ONLY, so combine the two functions rather than
> having completely split versions under ifdefs.
> 
> Signed-off-by: Peter Maydell 

Hi Peter,

Do we really need the assert?
If we keep it, can't we have it for both -user and -softmmu (avoiding the 
ifdef)?

Cheers,
Edgar



> ---
>  target-arm/op_helper.c | 25 ++---
>  1 file changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 053e9b6..05f97a7 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -457,26 +457,11 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
> regno, uint32_t val)
>  }
>  }
>  
> -#if defined(CONFIG_USER_ONLY)
> -void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 write\n");
> -}
> -
> -uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
> -{
> -ARMCPU *cpu = arm_env_get_cpu(env);
> -
> -cpu_abort(CPU(cpu), "banked r13 read\n");
> -return 0;
> -}
> -
> -#else
> -
>  void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
>  {
> +#if defined(CONFIG_USER_ONLY)
> +g_assert_not_reached();
> +#endif
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  env->regs[13] = val;
>  } else {
> @@ -486,13 +471,15 @@ void HELPER(set_r13_banked)(CPUARMState *env, uint32_t 
> mode, uint32_t val)
>  
>  uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
>  {
> +#if defined(CONFIG_USER_ONLY)
> +g_assert_not_reached();
> +#endif
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  return env->regs[13];
>  } else {
>  return env->banked_r13[bank_number(mode)];
>  }
>  }
> -#endif
>  
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
> syndrome,
>   uint32_t isread)
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Peter Maydell]

On 12 February 2016 at 15:12, Edgar E. Iglesias
 wrote:
> On Thu, Feb 11, 2016 at 07:11:48PM +, Peter Maydell wrote:
>> The user-mode versions of get/set_r13_banked() exist just to assert
>> if they're ever called -- the translate time code should never
>> emit calls to them because SRS from user mode always UNDEF.
>> There's no code in the softmmu versions that can't compile in
>> CONFIG_USER_ONLY, so combine the two functions rather than
>> having completely split versions under ifdefs.
>>
>> Signed-off-by: Peter Maydell 
>
> Hi Peter,
>
> Do we really need the assert?
> If we keep it, can't we have it for both -user and -softmmu (avoiding the 
> ifdef)?

I would be happy to entirely drop the assert, yes.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Edgar E. Iglesias]

On Fri, Feb 12, 2016 at 03:15:22PM +, Peter Maydell wrote:
> On 12 February 2016 at 15:12, Edgar E. Iglesias
>  wrote:
> > On Thu, Feb 11, 2016 at 07:11:48PM +, Peter Maydell wrote:
> >> The user-mode versions of get/set_r13_banked() exist just to assert
> >> if they're ever called -- the translate time code should never
> >> emit calls to them because SRS from user mode always UNDEF.
> >> There's no code in the softmmu versions that can't compile in
> >> CONFIG_USER_ONLY, so combine the two functions rather than
> >> having completely split versions under ifdefs.
> >>
> >> Signed-off-by: Peter Maydell 
> >
> > Hi Peter,
> >
> > Do we really need the assert?
> > If we keep it, can't we have it for both -user and -softmmu (avoiding the 
> > ifdef)?
> 
> I would be happy to entirely drop the assert, yes.

OK, thanks.

With that change:
Reviewed-by: Edgar E. Iglesias 

Re: [Qemu-devel] [PATCH 4/4] target-arm: UNDEF in the UNPREDICTABLE SRS-from-System case

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 07:11:49PM +, Peter Maydell wrote:
> Make get_r13_banked() raise an exception at runtime for the
> corner case of SRS from System mode, so that we can UNDEF it;
> this brings us in to line with the ARM ARM's set of permitted
> CONSTRAINED UNPREDICTABLE choices.

Reviewed-by: Edgar E. Iglesias 


> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/op_helper.c | 8 
>  target-arm/translate.c | 9 +
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 05f97a7..8183108 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -474,6 +474,14 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, 
> uint32_t mode)
>  #if defined(CONFIG_USER_ONLY)
>  g_assert_not_reached();
>  #endif
> +if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_SYS) {
> +/* SRS instruction is UNPREDICTABLE from System mode; we UNDEF.
> + * Other UNPREDICTABLE and UNDEF cases were caught at translate time.
> + */
> +raise_exception(env, EXCP_UDEF, syn_uncategorized(),
> +exception_target_el(env));
> +}
> +
>  if ((env->uncached_cpsr & CPSR_M) == mode) {
>  return env->regs[13];
>  } else {
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 7bceb05..e69145d 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7590,10 +7590,7 @@ static void gen_srs(DisasContext *s,
>   * -- not a valid mode number
>   * -- a mode that's at a higher exception level
>   * -- Monitor, if we are Non-secure
> - * For the UNPREDICTABLE cases we choose to UNDEF, except that for
> - * "current mode is System" we will write a garbage SPSR.
> - * (This is because we don't have access to our current mode here
> - * and would have to do a runtime check to UNDEF for System.)
> + * For the UNPREDICTABLE cases we choose to UNDEF.
>   */
>  if (s->current_el == 1 && !s->ns) {
>  gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
> @@ -7639,6 +7636,9 @@ static void gen_srs(DisasContext *s,
>  
>  addr = tcg_temp_new_i32();
>  tmp = tcg_const_i32(mode);
> +/* get_r13_banked() will raise an exception if called from System mode */
> +gen_set_condexec(s);
> +gen_set_pc_im(s, s->pc - 4);
>  gen_helper_get_r13_banked(addr, cpu_env, tmp);
>  tcg_temp_free_i32(tmp);
>  switch (amode) {
> @@ -7688,6 +7688,7 @@ static void gen_srs(DisasContext *s,
>  tcg_temp_free_i32(tmp);
>  }
>  tcg_temp_free_i32(addr);
> +s->is_jmp = DISAS_UPDATE;
>  }
>  
>  static void disas_arm_insn(DisasContext *s, unsigned int insn)
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Igor Mammedov]

On Fri, 12 Feb 2016 14:27:25 +0300
"Denis V. Lunev"  wrote:

> On 02/12/2016 02:13 PM, Andreas Färber wrote:
> > Am 12.02.2016 um 12:08 schrieb Denis V. Lunev:  
> >> On 02/12/2016 02:00 PM, Andreas Färber wrote:  
> >>> Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:  
>  From: "Alexey V. Kostyushko" 
> 
>  With Hyper-V enabled CPU hotplug stops working. The CPU appears in
>  device
>  manager on Windows but does not appear in peformance monitor and control
>  panel.
> 
>  The root of the problem is the following. Windows checks
>  HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
>  this bit is enough to cure the situation.
> 
>  Add option 'hv-cpuhotplug' to control this behavior.
> 
>  Signed-off-by: Alexey V. Kostyushko 
>  Signed-off-by: Denis V. Lunev 
>  CC: Paolo Bonzini 
>  CC: Richard Henderson 
>  CC: Eduardo Habkost 
>  CC: "Andreas Färber" 
>  ---
> target-i386/cpu-qom.h | 1 +
> target-i386/cpu.c | 1 +
> target-i386/kvm.c | 6 +-
> 3 files changed, 7 insertions(+), 1 deletion(-)
> 
>  diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
>  index 5f9d960..4aec616 100644
>  --- a/target-i386/cpu-qom.h
>  +++ b/target-i386/cpu-qom.h
>  @@ -96,6 +96,7 @@ typedef struct X86CPU {
> bool hyperv_runtime;
> bool hyperv_synic;
> bool hyperv_stimer;
>  +bool hyperv_cpuhotplug;
> bool check_cpuid;
> bool enforce_cpuid;
> bool expose_kvm;
>  diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>  index b255644..32c38ae 100644
>  --- a/target-i386/cpu.c
>  +++ b/target-i386/cpu.c
>  @@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
> DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
> DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
> DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
>  +DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug,
>  false),  
> >>> Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
> >>> dashes convention for QOM properties, i.e. "hv-cpu-hotplug".
> >>>
> >>> Regards,
> >>> Andreas  
> >> This name is for libvirt. We can take one one. This is not a problem.
> >>
> >> Roman Kagan has proposed verbally a bit different approach.
> >> He suggests not to introduce the option but
> >> check here that HyperV is enabled (any single option is enough
> >> to face the problem) and CPU hotplug is enabled and automatically
> >> enable this bit.
> >>
> >> Paolo, Andreas, is there any opinion on this?  
> > That implicit proposal sounds even more appealing to me, yes.
> >
> > You could still additionally do a dynamic property though, in case you
> > want to inspect or toggle it at runtime (no clue when Windows reads it).  
> no. this will not work. I should setup CPUID at the beginning
> (before boot) and keep it persistent. Thus either option
> coming from libvirt or this bit would be automatically
> calculated by QEMU.
> 
> I'll check that we could create read-only property to export
> the value.
> 
> OK. I'll redo this tomorrow if nobody will explicitly say 'no'
> today :)
+to Anreas suggestion and readonly prop

there is not need to create a field for it in CPU structure,
just checking for present hyperv flags should be sufficient.


> 
> Den
> 

Re: [Qemu-devel] [PATCH 01/14] cpu: Clean up includes

[Andreas Färber]

Am 09.02.2016 um 16:24 schrieb Peter Maydell:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Peter Maydell 
> ---
>  qom/cpu.c | 1 +
>  target-i386/cpu.c | 5 +
>  2 files changed, 2 insertions(+), 4 deletions(-)

Applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v3 2/5] drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header

[Andrey Smetanin]

On 02/12/2016 12:46 AM, Paolo Bonzini wrote:



On 11/02/2016 14:44, Andrey Smetanin wrote:

VMBus hypercall codes inside Hyper-V UAPI header will
be used by QEMU to implement VMBus host devices support.

Signed-off-by: Andrey Smetanin 
Acked-by: K. Y. Srinivasan 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
  arch/x86/include/uapi/asm/hyperv.h | 2 ++
  drivers/hv/connection.c| 2 +-
  drivers/hv/hv.c| 2 +-
  drivers/hv/hyperv_vmbus.h  | 6 --
  4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 0c50fab..bc1c8a9 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -227,6 +227,8 @@

  /* Declare the various hypercall operations. */
  #define HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT0x0008
+#define HV_X64_HCALL_POST_MESSAGE  0x005c
+#define HV_X64_HCALL_SIGNAL_EVENT  0x005d

  #define HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE0x0001
  #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT 12
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index fa86b2c..84700c6 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -485,5 +485,5 @@ void vmbus_set_event(struct vmbus_channel *channel)
(child_relid >> 5));
}

-   hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
+   hv_do_hypercall(HV_X64_HCALL_SIGNAL_EVENT, channel->sig_event, NULL);


What tree does this apply to?

next-20160211


Paolo


  }
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index ccb335f..48388dc 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -337,7 +337,7 @@ int hv_post_message(union hv_connection_id connection_id,
aligned_msg->payload_size = payload_size;
memcpy((void *)aligned_msg->payload, payload, payload_size);

-   status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL);
+   status = hv_do_hypercall(HV_X64_HCALL_POST_MESSAGE, aligned_msg, NULL);

put_cpu();
return status & 0x;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index b9ea7f5..1dabaa4 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -256,12 +256,6 @@ struct hv_monitor_page {
u8 rsvdz4[1984];
  };

-/* Declare the various hypercall operations. */
-enum hv_call_code {
-   HVCALL_POST_MESSAGE = 0x005c,
-   HVCALL_SIGNAL_EVENT = 0x005d,
-};
-
  /* Definition of the hv_post_message hypercall input structure. */
  struct hv_input_post_message {
union hv_connection_id connectionid;

Re: [Qemu-devel] [PATCH 1/4] target-arm: Clean up trap/undef handling of SRS

[Sergey Fedorov]

On 11.02.2016 22:11, Peter Maydell wrote:
> The SRS instruction is:
>  * UNDEFINED in Hyp mode
>  * UNPREDICTABLE in User or System mode
>  * UNPREDICTABLE if the specified mode isn't accessible
>  * trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
>
> Clean up the code to handle all these cases cleanly, including
> picking UNDEF as our choice of UNPREDICTABLE behaviour rather
> blindly trusting the mode field passed in the instruction.
> As part of this, move the check for IS_USER into gen_srs()
> itself rather than having it done by the caller.
>
> The exception is that we don't UNDEF for calls from System
> mode, which need a runtime check. This will be dealt with in
> the following commits.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Sergey Fedorov 

> ---
>  target-arm/translate.c | 66 
> ++
>  1 file changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index cf3dc33..7bceb05 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7578,8 +7578,67 @@ static void gen_srs(DisasContext *s,
>  uint32_t mode, uint32_t amode, bool writeback)
>  {
>  int32_t offset;
> -TCGv_i32 addr = tcg_temp_new_i32();
> -TCGv_i32 tmp = tcg_const_i32(mode);
> +TCGv_i32 addr, tmp;
> +bool undef = false;
> +
> +/* SRS is:
> + * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
> + * - UNDEFINED in Hyp mode
> + * - UNPREDICTABLE in User or System mode
> + * - UNPREDICTABLE if the specified mode is:
> + * -- not implemented
> + * -- not a valid mode number
> + * -- a mode that's at a higher exception level
> + * -- Monitor, if we are Non-secure
> + * For the UNPREDICTABLE cases we choose to UNDEF, except that for
> + * "current mode is System" we will write a garbage SPSR.
> + * (This is because we don't have access to our current mode here
> + * and would have to do a runtime check to UNDEF for System.)
> + */
> +if (s->current_el == 1 && !s->ns) {
> +gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
> +return;
> +}
> +
> +if (s->current_el == 0 || s->current_el == 2) {
> +undef = true;
> +}
> +
> +switch (mode) {
> +case ARM_CPU_MODE_USR:
> +case ARM_CPU_MODE_FIQ:
> +case ARM_CPU_MODE_IRQ:
> +case ARM_CPU_MODE_SVC:
> +case ARM_CPU_MODE_ABT:
> +case ARM_CPU_MODE_UND:
> +case ARM_CPU_MODE_SYS:
> +break;
> +case ARM_CPU_MODE_HYP:
> +if (s->current_el == 1 || !arm_dc_feature(s, ARM_FEATURE_EL2)) {
> +undef = true;
> +}
> +break;
> +case ARM_CPU_MODE_MON:
> +/* No need to check specifically for "are we non-secure" because
> + * we've already made EL0 UNDEF and handled the trap for S-EL1;
> + * so if this isn't EL3 then we must be non-secure.
> + */
> +if (s->current_el != 3) {
> +undef = true;
> +}
> +break;
> +default:
> +undef = true;
> +}
> +
> +if (undef) {
> +gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
> +   default_exception_el(s));
> +return;
> +}
> +
> +addr = tcg_temp_new_i32();
> +tmp = tcg_const_i32(mode);
>  gen_helper_get_r13_banked(addr, cpu_env, tmp);
>  tcg_temp_free_i32(tmp);
>  switch (amode) {
> @@ -7739,9 +7798,6 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  }
>  } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
>  /* srs */
> -if (IS_USER(s)) {
> -goto illegal_op;
> -}
>  ARCH(6);
>  gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
>  return;

Re: [Qemu-devel] [PATCH 7/9] pc: acpi: drop not needed intermediate bitmap cpu->found_cpus

[Igor Mammedov]

On Thu, 11 Feb 2016 13:59:33 -0200
Eduardo Habkost  wrote:

> On Fri, Feb 05, 2016 at 05:44:49PM +0100, Igor Mammedov wrote:
> > On Fri, 5 Feb 2016 17:19:50 +0100
> > Igor Mammedov  wrote:
> >   
> > > On Fri, 5 Feb 2016 13:39:07 -0200
> > > Eduardo Habkost  wrote:
> > >   
> > > > On Thu, Feb 04, 2016 at 12:47:33PM +0100, Igor Mammedov wrote:
> > > > > cpu->found_cpus bitmap is used for setting present
> > > > > flag in CPON AML package at start up. But it takes
> > > > > a bunch of code to fill bitmap and cloud be simplified
> > > > > by calling qemu_get_cpu_by_arch_id(apic_id) directly.
> > > > > 
> > > > > Hence do so and remove not used anymore bitmap
> > > > > with related utilities, which saves us ~32LOC
> > > > > and also would simplify consolidating APCI parts
> > > > > of CPU hotplug.
> > > > > 
> > > > > Signed-off-by: Igor Mammedov   
> > > > 
> > > > This makes the code loops through all smp_cpus CPUs max_cpus
> > > > times, instead of just looping through the smp_cpus CPUs once.
> > > Yep, that looks bad.
> > > I'll redo it using possible_cpu_arch_ids(), it is a little
> > > bit bigger refactoring.  
> > I think I'll make bitmap local to build_processor_devices() as it's
> > needed only for building CPON package, that way it will go along
> > with related legacy hotplug and won't get in the way of new hotplug.  
> 
> What about pc_possible_cpu_arch_ids() itself? It has exactly the
> same problem: it looks at CPU objects smp_cpus*max_cpus times, to
> fill the CPUArchId.cpu field. If that's a problem for
> build_processor_devices(), I assume that's a problem for all
> other possible_cpu_arch_ids() callers?
pls see v2
https://www.mail-archive.com/qemu-devel@nongnu.org/msg351298.html

Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
>> Hi,
>> 
>> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the 
>> following command line:
>> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
>> 
>> Before:
>> [nothing is print on console]
>> 
>> After:
>> QEMU 2.5.50 monitor - type 'help' for more information
>> (qemu)
>> 
>> In both cases, a working vc is created in the GTK+ UI.
>> 
>> Reverting the commit (and fixing the trivial conflict) makes things
>> work again.
>
> Thanks for the heads-up, I'll investigate and submit a fix

For the record, it also breaks ivshmem-test when the slow tests are
included.  I'd be willing to test your fix; got a pointer for me?

Re: [Qemu-devel] IVSHMEM support on qemu-system-arm

[Peter Maydell]

On 12 February 2016 at 15:05, sridhar kulkarni  wrote:
> I am working on a requirement where I need to have shared memory IPC between
> two guest VM's running different OS. Both VM's use vexpress-A9 boards. I
> came across the "ivshmem" implementation and have some questions. Whether
> the "ivshmem" implementation can be used on qemu-system-arm? I understand
> that "ivshmem" device is a PCI device and require PCI bus to connect to.
> Does vexpress board support this kind of arrangement?

No, there is no PCI bus on vexpress. If you need PCI you'll have to
switch to 'virt' instead.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 07/22] migration: introduce a new QEMUFile impl based on QIOChannel

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> On Tue, Feb 02, 2016 at 05:06:24PM +, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berra...@redhat.com) wrote:
> > > Introduce a new QEMUFile implementation that is based on
> > > the QIOChannel objects. This impl is different from existing
> > > impls in that there is no file descriptor that can be made
> > > available, as some channels may be based on higher level
> > > protocols such as TLS.
> > > 
> > > Although the QIOChannel based implementation can trivially
> > > provide a bi-directional stream, initially we have separate
> > > functions for opening input & output directions to fit with
> > > the expectation of the current QEMUFile interface.
> > > 
> > > Signed-off-by: Daniel P. Berrange 
> > > ---
> > >  include/migration/qemu-file.h |   4 +
> > >  migration/Makefile.objs   |   1 +
> > >  migration/qemu-file-channel.c | 201 
> > > ++
> > >  3 files changed, 206 insertions(+)
> > >  create mode 100644 migration/qemu-file-channel.c
> 
> > > +static ssize_t channel_writev_buffer(void *opaque,
> > > + struct iovec *iov,
> > > + int iovcnt,
> > > + int64_t pos)
> > > +{
> > > +QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > +ssize_t done = 0;
> > > +ssize_t want = iov_size(iov, iovcnt);
> > > +struct iovec oldiov = { NULL, 0 };
> > > +
> > > +while (done < want) {
> > > +ssize_t len;
> > > +struct iovec *cur = iov;
> > > +int curcnt = iovcnt;
> > > +
> > > +channel_skip_iov(done, , , );
> > > +
> > > +len = qio_channel_writev(ioc, cur, curcnt, NULL);
> > > +if (oldiov.iov_base) {
> > > +/* Restore original caller's info in @iov */
> > > +cur[0].iov_base = oldiov.iov_base;
> > > +cur[0].iov_len = oldiov.iov_len;
> > > +oldiov.iov_base = NULL;
> > > +oldiov.iov_len = 0;
> > > +}
> > > +if (len == QIO_CHANNEL_ERR_BLOCK) {
> > > +qio_channel_wait(ioc, G_IO_OUT);
> > > +continue;
> > > +}
> > > +if (len < 0) {
> > > +/* XXX handle Error objects */
> > > +return -EIO;
> > 
> > It's best to return 'len' rather than lose what little
> > error information you had (similarly below).
> 
> In this case 'len' is in fact just '-1', as all error
> info is in the Error ** parameter, but the QEMUFile API
> contract requires an errno value. So we don't have much
> choice but to return a fixed EIO - returning 'len' would
> also be a fixed errno - whichever errno corresponds to
> the value -1.

Oh, I see - that's a shame that they didn't pass the
error along, but yeh if they're just returning -1 that makes
sense.

> I'd like to switch QEMUFile over to use Error **errp
> parameters for error reporting, so that we can make
> detailed error info available throughout the migration
> I/O code. That ought to wait until after this series
> is done though, to avoid complicating it yet more.

OK, you could use a local_error I guess;  I just worry
about not having any error information at all.

Dave

> 
> > 
> > > +}
> > > +
> > > +done += len;
> > > +}
> > > +return done;
> > > +}
> > > +
> > > +
> > > +static ssize_t channel_get_buffer(void *opaque,
> > > +  uint8_t *buf,
> > > +  int64_t pos,
> > > +  size_t size)
> > > +{
> > > +QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > +ssize_t ret;
> > > +
> > > + reread:
> > > +ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> > > +if (ret < 0) {
> > > +if (ret == QIO_CHANNEL_ERR_BLOCK) {
> > > +qio_channel_yield(ioc, G_IO_IN);
> > > +goto reread;
> > > +} else {
> > > +/* XXX handle Error * object */
> > > +return -EIO;
> > > +}
> > > +}
> > > +return ret;
> > 
> > 
> > I'd prefer a loop to a goto; generally the only places we
> > use goto is an error exit.
> > 
> >   do {
> >ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> >if (ret == QIO_CHANNEL_ERR_BLOCK) {
> >qio_channel_yield(ioc, G_IO_IN);
> >}
> >   } while (ret == QIO_CHANNEL_ERR_BLOCK);
> > 
> >   return ret;
> > 
> > and IMHO the loop is clearer.
> 
> Ok, will change that.
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o- http://virt-manager.org :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PULL 0/5] tags/xen-2016-02-12

[Stefano Stabellini]

The following changes since commit f075c89f0a9cb31daf38892371d2822177505706:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2016-02-09 17:56:46 +)

are available in the git repository at:


  git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-2016-02-12

for you to fetch changes up to 47d3df2387ed6927732584ffa4159c26d9f4dee8:

  xen: Drop __XEN_LATEST_INTERFACE_VERSION__ checks from prior to Xen 4.2 
(2016-02-10 12:01:32 +)


Xen 2016-02-12


Ian Campbell (5):
  xen: drop support for Xen 4.1 and older.
  xen: drop xen_xc_hvm_inject_msi wrapper
  xen: drop XenXC and associated interface wrappers
  xen: move xenforeignmemory compat layer into common place
  xen: Drop __XEN_LATEST_INTERFACE_VERSION__ checks from prior to Xen 4.2

 configure|  101 +
 hw/display/xenfb.c   |7 --
 hw/i386/xen/xen_apic.c   |4 -
 hw/xen/xen_backend.c |4 +-
 include/hw/xen/xen.h |4 -
 include/hw/xen/xen_backend.h |2 +-
 include/hw/xen/xen_common.h  |  254 +++---
 xen-common.c |4 +-
 xen-hvm.c|   56 ++
 9 files changed, 61 insertions(+), 375 deletions(-)

[Qemu-devel] [PULL 1/5] xen: drop support for Xen 4.1 and older.

[Stefano Stabellini]

From: Ian Campbell 

Xen 4.2 become unsupported upstream in 09/2015 (see
http://wiki.xen.org/wiki/Xen_Release_Features). However as far as the
interfaces provided by the toolstack libraries go 4.2 and 4.3 are
indistinguishable.

Therefore drop support for Xen 4.1 and earlier which removes a whole
pile of compatibility code which makes future work (to use stable
library interfaces provided by upstream) more difficult. In particular
all supported versions now use a pointer as a libxc handle (4.1 and
earlier used an integer, resulting in various shim layers).

Also Xen 4.2 was the first version of Xen to formally support upstream
QEMU (as a preview) so that makes sense as a cut-off now.

This change drops all the configure-y and resulting ifdefs in a mostly
mechanical way. A follow up will refactor wrappers which are now
unused.

Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 configure   |  101 +-
 hw/i386/xen/xen_apic.c  |4 --
 include/hw/xen/xen.h|4 --
 include/hw/xen/xen_common.h |  146 ++-
 xen-hvm.c   |   19 --
 5 files changed, 7 insertions(+), 267 deletions(-)

diff --git a/configure b/configure
index d4411a1..16c7956 100755
--- a/configure
+++ b/configure
@@ -2111,100 +2111,10 @@ EOF
 xen_ctrl_version=420
 xen=yes
 
-  elif
-  cat > $TMPC <
-#include 
-#include 
-#include 
-#if !defined(HVM_MAX_VCPUS)
-# error HVM_MAX_VCPUS not defined
-#endif
-int main(void) {
-  xs_daemon_open();
-  xc_interface_open(0, 0, 0);
-  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
-  xc_gnttab_open(NULL, 0);
-  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
-  return 0;
-}
-EOF
-  compile_prog "" "$xen_libs"
-then
-xen_ctrl_version=410
-xen=yes
-
-  # Xen 4.0.0
-  elif
-  cat > $TMPC <
-#include 
-#include 
-#include 
-#if !defined(HVM_MAX_VCPUS)
-# error HVM_MAX_VCPUS not defined
-#endif
-int main(void) {
-  struct xen_add_to_physmap xatp = {
-.domid = 0, .space = XENMAPSPACE_gmfn, .idx = 0, .gpfn = 0,
-  };
-  xs_daemon_open();
-  xc_interface_open();
-  xc_gnttab_open();
-  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
-  xc_memory_op(0, XENMEM_add_to_physmap, );
-  return 0;
-}
-EOF
-  compile_prog "" "$xen_libs"
-then
-xen_ctrl_version=400
-xen=yes
-
-  # Xen 3.4.0
-  elif
-  cat > $TMPC <
-#include 
-int main(void) {
-  struct xen_add_to_physmap xatp = {
-.domid = 0, .space = XENMAPSPACE_gmfn, .idx = 0, .gpfn = 0,
-  };
-  xs_daemon_open();
-  xc_interface_open();
-  xc_gnttab_open();
-  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
-  xc_memory_op(0, XENMEM_add_to_physmap, );
-  return 0;
-}
-EOF
-  compile_prog "" "$xen_libs"
-then
-xen_ctrl_version=340
-xen=yes
-
-  # Xen 3.3.0
-  elif
-  cat > $TMPC <
-#include 
-int main(void) {
-  xs_daemon_open();
-  xc_interface_open();
-  xc_gnttab_open();
-  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
-  return 0;
-}
-EOF
-  compile_prog "" "$xen_libs"
-then
-xen_ctrl_version=330
-xen=yes
-
-  # Xen version unsupported
   else
 if test "$xen" = "yes" ; then
-  feature_not_found "xen (unsupported version)" "Install supported xen 
(e.g. 4.0, 3.4, 3.3)"
+  feature_not_found "xen (unsupported version)" \
+"Install a supported xen (xen 4.2 or newer)"
 fi
 xen=no
   fi
@@ -2218,15 +2128,10 @@ EOF
 fi
 
 if test "$xen_pci_passthrough" != "no"; then
-  if test "$xen" = "yes" && test "$linux" = "yes" &&
-test "$xen_ctrl_version" -ge 340; then
+  if test "$xen" = "yes" && test "$linux" = "yes"; then
 xen_pci_passthrough=yes
   else
 if test "$xen_pci_passthrough" = "yes"; then
-  if test "$xen_ctrl_version" -lt 340; then
-error_exit "User requested feature Xen PCI Passthrough" \
-"This feature does not work with Xen 3.3"
-  fi
   error_exit "User requested feature Xen PCI Passthrough" \
   " but this feature requires /sys from Linux"
 fi
diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c
index 5128146..2b8d709 100644
--- a/hw/i386/xen/xen_apic.c
+++ b/hw/i386/xen/xen_apic.c
@@ -44,11 +44,7 @@ static void xen_apic_realize(DeviceState *dev, Error **errp)
 s->vapic_control = 0;
 memory_region_init_io(>io_memory, OBJECT(s), _apic_io_ops, s,
   "xen-apic-msi", APIC_SPACE_SIZE);
-
-#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
-&& CONFIG_XEN_CTRL_INTERFACE_VERSION >= 420
 msi_supported = true;
-#endif
 }
 
 static void xen_apic_set_base(APICCommonState *s, uint64_t val)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index c577354..df33481 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -48,8 +48,4 @@ void 

[Qemu-devel] [PULL 3/5] xen: drop XenXC and associated interface wrappers

[Stefano Stabellini]

From: Ian Campbell 

Now that 4.2 and earlier are no longer supported "xc_interface *" is
always the right type for the xc interface handle.

With this we can also simplify the handling of the xenforeignmemory
compatibility wrapper by making xenforeignmemory_handle ==
xc_interface, instead of an xc_interface* and remove various uses of &
and *h.

Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 hw/xen/xen_backend.c |4 +--
 include/hw/xen/xen_backend.h |2 +-
 include/hw/xen/xen_common.h  |   82 --
 xen-common.c |4 +--
 xen-hvm.c|   16 -
 5 files changed, 44 insertions(+), 64 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index ef7843f..60575ad 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -36,7 +36,7 @@
 /* - */
 
 /* public */
-XenXC xen_xc = XC_HANDLER_INITIAL_VALUE;
+xc_interface *xen_xc = NULL;
 xenforeignmemory_handle *xen_fmem = NULL;
 struct xs_handle *xenstore = NULL;
 const char *xen_protocol;
@@ -710,7 +710,7 @@ int xen_be_init(void)
 
 qemu_set_fd_handler(xs_fileno(xenstore), xenstore_update, NULL, NULL);
 
-if (xen_xc == XC_HANDLER_INITIAL_VALUE || xen_fmem == NULL) {
+if (xen_xc == NULL || xen_fmem == NULL) {
 /* Check if xen_init() have been called */
 goto err;
 }
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index e0d52ee..c839eeb 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -56,7 +56,7 @@ struct XenDevice {
 /* - */
 
 /* variables */
-extern XenXC xen_xc;
+extern xc_interface *xen_xc;
 extern xenforeignmemory_handle *xen_fmem;
 extern struct xs_handle *xenstore;
 extern const char *xen_protocol;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index ec3ca56..254ef14 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -32,14 +32,10 @@
 /* Xen 4.2 thru 4.6 */
 #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 471
 
-typedef xc_interface *XenXC;
-typedef xc_interface *xenforeignmemory_handle;
+typedef xc_interface xenforeignmemory_handle;
 typedef xc_evtchn xenevtchn_handle;
 typedef xc_gnttab xengnttab_handle;
 
-#  define XC_INTERFACE_FMT "%p"
-#  define XC_HANDLER_INITIAL_VALUENULL
-
 #define xenevtchn_open(l, f) xc_evtchn_open(l, f);
 #define xenevtchn_close(h) xc_evtchn_close(h)
 #define xenevtchn_fd(h) xc_evtchn_fd(h)
@@ -57,30 +53,14 @@ typedef xc_gnttab xengnttab_handle;
 #define xengnttab_map_grant_refs(h, c, d, r, p) \
 xc_gnttab_map_grant_refs(h, c, d, r, p)
 
-static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
-  unsigned int open_flags)
-{
-return xc_interface_open(logger, dombuild_logger, open_flags);
-}
-
 /* See below for xenforeignmemory_* APIs */
 
 #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 471 */
 
-typedef xc_interface *XenXC;
-
-#  define XC_INTERFACE_FMT "%p"
-#  define XC_HANDLER_INITIAL_VALUENULL
-
 #include 
 #include 
 #include 
 
-static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
-  unsigned int open_flags)
-{
-return xc_interface_open(logger, dombuild_logger, open_flags);
-}
 #endif
 
 void destroy_hvm_domain(bool reboot);
@@ -89,7 +69,7 @@ void destroy_hvm_domain(bool reboot);
 void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 #ifdef HVM_PARAM_VMPORT_REGS_PFN
-static inline int xen_get_vmport_regs_pfn(XenXC xc, domid_t dom,
+static inline int xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
   xen_pfn_t *vmport_regs_pfn)
 {
 int rc;
@@ -101,7 +81,7 @@ static inline int xen_get_vmport_regs_pfn(XenXC xc, domid_t 
dom,
 return rc;
 }
 #else
-static inline int xen_get_vmport_regs_pfn(XenXC xc, domid_t dom,
+static inline int xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
   xen_pfn_t *vmport_regs_pfn)
 {
 return -ENOSYS;
@@ -128,54 +108,54 @@ static inline int xen_get_vmport_regs_pfn(XenXC xc, 
domid_t dom,
 
 typedef uint16_t ioservid_t;
 
-static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
   ioservid_t ioservid,
   MemoryRegionSection *section)
 {
 }
 
-static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+static inline void xen_unmap_memory_section(xc_interface *xc, domid_t dom,
 ioservid_t 

[Qemu-devel] [PULL 2/5] xen: drop xen_xc_hvm_inject_msi wrapper

[Stefano Stabellini]

From: Ian Campbell 

The xc version is now always present.

Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 include/hw/xen/xen_common.h |6 --
 xen-hvm.c   |2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 3a5b537..ec3ca56 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -83,12 +83,6 @@ static inline XenXC xen_xc_interface_open(void *logger, void 
*dombuild_logger,
 }
 #endif
 
-static inline int xen_xc_hvm_inject_msi(XenXC xen_xc, domid_t dom,
-uint64_t addr, uint32_t data)
-{
-return xc_hvm_inject_msi(xen_xc, dom, addr, data);
-}
-
 void destroy_hvm_domain(bool reboot);
 
 /* shutdown/destroy current domain because of an error */
diff --git a/xen-hvm.c b/xen-hvm.c
index 8350ca2..918cabc 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -169,7 +169,7 @@ int xen_is_pirq_msi(uint32_t msi_data)
 
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
-xen_xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
+xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
 }
 
 static void xen_suspend_notifier(Notifier *notifier, void *data)
-- 
1.7.10.4

[Qemu-devel] [PULL 5/5] xen: Drop __XEN_LATEST_INTERFACE_VERSION__ checks from prior to Xen 4.2

[Stefano Stabellini]

From: Ian Campbell 

We assume (and check for in configure) 4.2 or later now. In reality
all of the removed checks are for far older versions.

FMT_ioreq_size is no longer needed.

Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 hw/display/xenfb.c |7 ---
 xen-hvm.c  |   19 ++-
 2 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index a533c3d..40b096a 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -241,9 +241,7 @@ static int xenfb_send_motion(struct XenInput *xenfb,
 event.type = XENKBD_TYPE_MOTION;
 event.motion.rel_x = rel_x;
 event.motion.rel_y = rel_y;
-#if __XEN_LATEST_INTERFACE_VERSION__ >= 0x00030207
 event.motion.rel_z = rel_z;
-#endif
 
 return xenfb_kbd_event(xenfb, );
 }
@@ -258,12 +256,7 @@ static int xenfb_send_position(struct XenInput *xenfb,
 event.type = XENKBD_TYPE_POS;
 event.pos.abs_x = abs_x;
 event.pos.abs_y = abs_y;
-#if __XEN_LATEST_INTERFACE_VERSION__ == 0x00030207
-event.pos.abs_z = z;
-#endif
-#if __XEN_LATEST_INTERFACE_VERSION__ >= 0x00030208
 event.pos.rel_z = z;
-#endif
 
 return xenfb_kbd_event(xenfb, );
 }
diff --git a/xen-hvm.c b/xen-hvm.c
index fbe0e3a..039680a 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -67,17 +67,6 @@ struct shared_vmport_iopage {
 typedef struct shared_vmport_iopage shared_vmport_iopage_t;
 #endif
 
-#if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
-static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
-{
-return shared_page->vcpu_iodata[i].vp_eport;
-}
-static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu)
-{
-return _page->vcpu_iodata[vcpu].vp_ioreq;
-}
-#  define FMT_ioreq_size PRIx64
-#else
 static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
 {
 return shared_page->vcpu_ioreq[i].vp_eport;
@@ -86,8 +75,6 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t 
*shared_page, int vcpu)
 {
 return _page->vcpu_ioreq[vcpu];
 }
-#  define FMT_ioreq_size "u"
-#endif
 
 #define BUFFER_IO_MAX_DELAY  100
 
@@ -688,7 +675,7 @@ static ioreq_t *cpu_get_ioreq_from_shared_memory(XenIOState 
*state, int vcpu)
 if (req->state != STATE_IOREQ_READY) {
 DPRINTF("I/O request not ready: "
 "%x, ptr: %x, port: %"PRIx64", "
-"data: %"PRIx64", count: %" FMT_ioreq_size ", size: %" 
FMT_ioreq_size "\n",
+"data: %"PRIx64", count: %u, size: %u\n",
 req->state, req->data_is_ptr, req->addr,
 req->data, req->count, req->size);
 return NULL;
@@ -1050,9 +1037,7 @@ static void cpu_handle_ioreq(void *opaque)
 if (req->state != STATE_IOREQ_INPROCESS) {
 fprintf(stderr, "Badness in I/O request ... not in service?!: "
 "%x, ptr: %x, port: %"PRIx64", "
-"data: %"PRIx64", count: %" FMT_ioreq_size
-", size: %" FMT_ioreq_size
-", type: %"FMT_ioreq_size"\n",
+"data: %"PRIx64", count: %u, size: %u, type: %u\n",
 req->state, req->data_is_ptr, req->addr,
 req->data, req->count, req->size, req->type);
 destroy_hvm_domain(false);
-- 
1.7.10.4

[Qemu-devel] [PULL 4/5] xen: move xenforeignmemory compat layer into common place

[Stefano Stabellini]

From: Ian Campbell 

Now that we no longer support Xen 4.2 and earlier only the <470 case
needs this so it can live with all the others.

Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 include/hw/xen/xen_common.h |   34 ++
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 254ef14..7a3cce0 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -53,7 +53,20 @@ typedef xc_gnttab xengnttab_handle;
 #define xengnttab_map_grant_refs(h, c, d, r, p) \
 xc_gnttab_map_grant_refs(h, c, d, r, p)
 
-/* See below for xenforeignmemory_* APIs */
+#define xenforeignmemory_open(l, f) xen_xc
+
+static inline void *xenforeignmemory_map(xc_interface *h, uint32_t dom,
+ int prot, size_t pages,
+ const xen_pfn_t arr[/*pages*/],
+ int err[/*pages*/])
+{
+if (err)
+return xc_map_foreign_bulk(h, dom, prot, arr, err, pages);
+else
+return xc_map_foreign_pages(h, dom, prot, arr, pages);
+}
+
+#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
 
 #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 471 */
 
@@ -359,23 +372,4 @@ static inline int xen_domain_create(xc_interface *xc, 
uint32_t ssidref,
 #endif
 #endif
 
-#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 471
-
-#define xenforeignmemory_open(l, f) xen_xc
-
-static inline void *xenforeignmemory_map(xc_interface *h, uint32_t dom,
- int prot, size_t pages,
- const xen_pfn_t arr[/*pages*/],
- int err[/*pages*/])
-{
-if (err)
-return xc_map_foreign_bulk(h, dom, prot, arr, err, pages);
-else
-return xc_map_foreign_pages(h, dom, prot, arr, pages);
-}
-
-#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
-
-#endif
-
 #endif /* QEMU_HW_XEN_COMMON_H */
-- 
1.7.10.4

Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends

[Daniel P. Berrange]

On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
> 
> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> >> Hi,
> >> 
> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the 
> >> following command line:
> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
> >> 
> >> Before:
> >> [nothing is print on console]
> >> 
> >> After:
> >> QEMU 2.5.50 monitor - type 'help' for more information
> >> (qemu)
> >> 
> >> In both cases, a working vc is created in the GTK+ UI.
> >> 
> >> Reverting the commit (and fixing the trivial conflict) makes things
> >> work again.
> >
> > Thanks for the heads-up, I'll investigate and submit a fix
> 
> For the record, it also breaks ivshmem-test when the slow tests are
> included.  I'd be willing to test your fix; got a pointer for me?

https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

[Qemu-devel] [Bug 1545024] Re: compiling on armv7 crashes compile qlx.o

[Klaftenegger Felix]

if i try to compile with target-list=i386-linux-user it is working so
the problem must the target i386-softmmu

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1545024

Title:
  compiling on armv7 crashes compile qlx.o

Status in QEMU:
  New

Bug description:
  If i try to compile qemu on armv7 cpu i get this error:

LINK  qemu-nbd
CCqemu-img.o
LINK  qemu-img
LINK  qemu-io
LINK  qemu-bridge-helper
CCqmp-marshal.o
CChw/display/qxl.o
  {standard input}: Assembler messages:
  {standard input}:1704: Error: bad instruction `lock'
  {standard input}:1704: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:1864: Error: bad instruction `lock'
  {standard input}:1864: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:5239: Error: bad instruction `lock'
  {standard input}:5239: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:5731: Error: bad instruction `lock'
  {standard input}:5731: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:11923: Error: bad instruction `lock'
  {standard input}:11923: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:13960: Error: bad instruction `lock'
  {standard input}:13960: Error: bad instruction `addl $0,0(%rsp)'
  {standard input}:14349: Error: bad instruction `lock'
  {standard input}:14349: Error: bad instruction `addl $0,0(%rsp)'
  /home/fleixi/git/qemu/rules.mak:57: recipe for target 'hw/display/qxl.o' 
failed
  make: *** [hw/display/qxl.o] Error 1

  Build options are:

   ./configure --target-list=i386-softmmu
  Install prefix/usr/local
  BIOS directory/usr/local/share/qemu
  binary directory  /usr/local/bin
  library directory /usr/local/lib
  module directory  /usr/local/lib/qemu
  libexec directory /usr/local/libexec
  include directory /usr/local/include
  config directory  /usr/local/etc
  local state directory   /usr/local/var
  Manual directory  /usr/local/share/man
  ELF interp prefix /usr/gnemul/qemu-%M
  Source path   /home/fleixi/git/qemu
  C compilercc
  Host C compiler   cc
  C++ compiler  c++
  Objective-C compiler cc
  ARFLAGS   rv
  CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabihf/glib-2.0/include  -g 
-mcpu=cortex-a15.cortex-a7 -mfloat-abi=hard -mfpu=neon-vfpv4 -O2 -pipe 
-ffast-math -ftree-vectorize -mvectorize-with-neon-quad -fstack-protector 
--param=ssp-buffer-size=4
  QEMU_CFLAGS   -I/usr/include/pixman-1   -Werror  -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-strong   -I/usr/include/libpng12  
-I/usr/local/include/spice-server -I/usr/local/include 
-I/usr/local/include/spice-1 -I/usr/include/glib-2.0 
-I/usr/lib/arm-linux-gnueabihf/glib-2.0/include -I/usr/include/pixman-1 
  LDFLAGS   -Wl,--warn-common -g 
  make  make
  install   install
  pythonpython -B
  smbd  /usr/sbin/smbd
  module supportno
  host CPU  arm
  host big endian   no
  target list   i386-softmmu
  tcg debug enabled no
  gprof enabled no
  sparse enabledno
  strip binariesyes
  profiler  no
  static build  no
  pixmansystem
  SDL support   no
  GTK support   yes
  GTK GL supportno
  GNUTLS supportno
  GNUTLS hash   no
  libgcrypt no
  nettleno
  libtasn1  no
  VTE support   no
  curses supportno
  virgl support no
  curl support  yes
  mingw32 support   no
  Audio drivers oss
  Block whitelist (rw) 
  Block whitelist (ro) 
  VirtFS supportno
  VNC support   yes
  VNC SASL support  yes
  VNC JPEG support  yes
  VNC PNG support   yes
  xen support   no
  brlapi supportno
  bluez  supportyes
  Documentation no
  PIE   no
  vde support   no
  netmap supportno
  Linux AIO support no
  ATTR/XATTR support yes
  Install blobs yes
  KVM support   yes
  RDMA support  no
  TCG interpreter   no
  fdt support   no
  preadv supportyes
  fdatasync yes
  madvise   yes
  posix_madvise yes
  sigev_thread_id   yes
  uuid support  no
  libcap-ng support no
  vhost-net support yes
  vhost-scsi support yes
  Trace backendslog
  spice support yes (0.12.10/0.12.6)
  rbd support   no
  xfsctl supportno
  smartcard support no
  libusbno
  usb net redir no
  OpenGL supportno
  libiscsi support  no
  libnfs supportno
  build guest agent yes
  QGA VSS support   no
  QGA w32 disk 

Re: [Qemu-devel] KVH call for agenda for 2016-02-16

[Andreas Färber]

Am 10.02.2016 um 16:48 schrieb Christian Borntraeger:
> On 02/10/2016 04:42 PM, Juan Quintela wrote:
>> Please, send any topic that you are interested in covering.
>>
>> At the end of Monday I will send an email with the agenda or the
>> cancellation of the call, so hurry up.
>>
>> After discussions on the QEMU Summit, we are going to have always open a
>> KVM call where you can add topics.
> 
> Since we seem to be stuck, I propose CPU hotplug as agenda item.

Thank you, confirming that I will be able to attend.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] IVSHMEM support on qemu-system-arm

[sridhar kulkarni]

Hi,
I am working on a requirement where I need to have shared memory IPC between 
two guest VM's running different OS. Both VM's use vexpress-A9 boards. I came 
across the "ivshmem" implementation and have some questions. Whether the 
"ivshmem" implementation can be used on qemu-system-arm? I understand that 
"ivshmem" device is a PCI device and require PCI bus to connect to. Does 
vexpress board support this kind of arrangement?
RegardsSridhar

Re: [Qemu-devel] [PATCH v1 20/22] migration: support TLS encryption with TCP migration backend

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> This extends the TCP migration backend so that it can make use
> of QIOChannelTLS to provide transparent TLS encryption. To
> trigger enablement the URI on the incoming and outgoing sides
> should have 'tls-creds=ID' appended, eg
> 
>tcp:$HOST:$PORT,tls-creds=ID
> 
> where ID is the object identifier of a set of TLS credentials
> previously created using object_add / -object. There is not
> currently any support for checking the migration client
> certificate names against ACLs. This is pending a conversion
> of the ACL code to QOM.

Does that change the option passed or is that just different
in the way the tls-creds are set up?

> There is no support for dynamically negotiating use of TLS
> between the incoming/outgoing side. Both sides must agree
> on use of TLS out of band and set the URI accordingly. In
> practice it is expected that the administrator will just
> turn on use of TLS on their hosts in the libvirt config
> and then libvirt will instruct QEMU to use TLS for migration.
> 
> Signed-off-by: Daniel P. Berrange 

> ---
>  migration/tcp.c | 273 
> +---
>  qemu-options.hx |   7 +-
>  2 files changed, 264 insertions(+), 16 deletions(-)
> 
> diff --git a/migration/tcp.c b/migration/tcp.c
> index ac73977..bef861c 100644
> --- a/migration/tcp.c
> +++ b/migration/tcp.c
> @@ -22,6 +22,8 @@
>  #include "migration/migration.h"
>  #include "migration/qemu-file.h"
>  #include "io/channel-socket.h"
> +#include "io/channel-tls.h"
> +#include "crypto/tlscreds.h"
>  
>  //#define DEBUG_MIGRATION_TCP
>  
> @@ -33,6 +35,22 @@
>  do { } while (0)
>  #endif
>  
> +typedef struct {
> +MigrationState *s;
> +QCryptoTLSCreds *tlscreds;
> +char *hostname;
> +} TCPConnectData;
> +
> +typedef struct {
> +MigrationState *s;
> +QCryptoTLSCreds *tlscreds;
> +} TCPListenData;
> +
> +typedef struct {
> +MigrationState *s;
> +QIOChannel *ioc;
> +} TCPConnectTLSData;
> +

what makes it TCP specific rather than sharing most of this between transports? 
i.e. should
this work for fd migration ? (rdma is probably not reasonable
giving it's scribbling directly in the other hosts RAM).
Certainly those types dont really look TCP specific.

>  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  {
> @@ -51,21 +69,174 @@ static SocketAddress *tcp_build_address(const char 
> *host_port, Error **errp)
>  }
>  
>  
> +static void tcp_connect_data_free(gpointer opaque)
> +{
> +TCPConnectData *data = opaque;
> +if (!data) {
> +return;
> +}
> +g_free(data->hostname);
> +object_unref(OBJECT(data->tlscreds));
> +g_free(data);
> +}
> +
> +
> +static void tcp_listen_data_free(gpointer opaque)
> +{
> +TCPListenData *data = opaque;
> +if (!data) {
> +return;
> +}
> +object_unref(OBJECT(data->tlscreds));
> +g_free(data);
> +}
> +
> +
> +static void tcp_connect_tls_data_free(gpointer opaque)
> +{
> +TCPConnectTLSData *data = opaque;
> +if (!data) {
> +return;
> +}
> +object_unref(OBJECT(data->ioc));
> +g_free(data);
> +}
> +
> +
> +static char *tcp_get_opt_str(const char *host_port,
> + const char *key)
> +{
> +const char *offset, *end;
> +
> +offset = strstr(host_port, key);
> +if (!offset) {
> +return NULL;
> +}
> +
> +offset += strlen(key);
> +if (offset[0] != '=') {
> +return NULL;
> +}
> +offset++;
> +end = strchr(offset, ',');
> +if (end) {
> +return g_strndup(offset, end - offset);
> +} else {
> +return g_strdup(offset);
> +}
> +}
> +
> +
> +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
> +  bool is_listen,
> +  Error **errp)
> +{
> +char *credname = NULL;
> +Object *creds;
> +QCryptoTLSCreds *ret;
> +
> +credname = tcp_get_opt_str(host_port, "tls-creds");
> +if (!credname) {
> +return NULL;
> +}

At what point does it get saner just to throw host_port into a qemu_opts 
and let it parse it?

> +creds = object_resolve_path_component(
> +object_get_objects_root(), credname);
> +if (!creds) {
> +error_setg(errp, "No TLS credentials with id '%s'",
> +   credname);
> +goto error;
> +}
> +ret = (QCryptoTLSCreds *)object_dynamic_cast(
> +creds, TYPE_QCRYPTO_TLS_CREDS);
> +if (!ret) {
> +error_setg(errp, "Object with id '%s' is not TLS credentials",
> +   credname);
> +goto error;
> +}
> +if (is_listen) {
> +if (ret->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> +error_setg(errp, "%s",
> +   "Expected TLS credentials for server endpoint");
> +goto error;
> +}
> +} else {

Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
>> "Daniel P. Berrange"  writes:
>> 
>> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
>> >> Hi,
>> >> 
>> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses 
>> >> the following command line:
>> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
>> >> 
>> >> Before:
>> >> [nothing is print on console]
>> >> 
>> >> After:
>> >> QEMU 2.5.50 monitor - type 'help' for more information
>> >> (qemu)
>> >> 
>> >> In both cases, a working vc is created in the GTK+ UI.
>> >> 
>> >> Reverting the commit (and fixing the trivial conflict) makes things
>> >> work again.
>> >
>> > Thanks for the heads-up, I'll investigate and submit a fix
>> 
>> For the record, it also breaks ivshmem-test when the slow tests are
>> included.  I'd be willing to test your fix; got a pointer for me?
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html

No luck.  Please try the following reproducer:

  $ make tests/ivshmem-test
  $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' 
QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 
1))} gtester -k --verbose -m slow tests/ivshmem-test

You can instead run make check SPEED=slow, but that runs many more
tests.

Re: [Qemu-devel] [PATCH 00/13] throttle: Allow I/O bursts for a user-defined period of time

[Kevin Wolf]

Am 05.02.2016 um 11:59 hat Alberto Garcia geschrieben:
> Hello everyone,
> 
> the current throttling code in QEMU allows limiting the I/O rate on
> block devices. Limits can be set in operations per second (IOPS) or
> bytes per second, allowing separate limits for read and write
> operations on both cases.
> 
> In its basic usage the user can set a limit of, say, 100 IOPS on a
> block device passing this option to -drive:
> 
>   throttling.iops-total=100
> 
> In addition to that, QEMU can also allow the user to do I/O bursts
> exceeding that limit up to a configurable rate:
> 
>   throttling.iops-total=100,throttling.iops-total-max=2000
> 
> With this, the user can do a burst of 2000 IOPS before they're
> throttled down to 100 IOPS. Then, after a sufficiently long period of
> unused I/O they will be able to do a burst again.
> 
> This patch series introduces the possibility to do bursts for longer
> period of times. A new setting called throttling.iops-total-max-length
> is used to define for how long bursts can be sustained.
> 
> So adding throttling.iops-total-max-length=60 to the previous
> configuration allows the user to do I/O at a rate of 2000 IOPS for 1
> minute before going down to the base rate of 100 IOPS.
> 
> This is essentially the same as described in this AWS blog post:
> 
>https://aws.amazon.com/blogs/aws/new-ssd-backed-elastic-block-storage/
> 
> As described in the article, a use case for this feature is to allow
> better performance when booting the OS or restarting a service while
> keeping the average I/O limits lower the rest of the time.
> 
> Comments:
> 
>  - There are 6 different settings for setting I/O limits: iops-total,
>iops-read, iops-write, bps-total, bps-read, bps-write. This series
>adds one new setting to set the length for each one of those.
> 
>I don't know if there's a good use case that requires such
>fine-grained control. It's of course also possible to make it
>simpler and add just one 'burst-length' setting that would apply
>for all cases, but the current solution is IMHO simple enough and
>consistent with the current API, and if we need to extend it later
>the result is probably going to be ugly.
> 
>  - With this series we set "a maximum of X operations/second for a
>period of T seconds". If would also be possible to make it "a
>maximum of X operations/second up to a total of Y operations". It
>would be equivalent (Y = X * T) but I thought the current proposal
>makes a more clear API.
> 
> And I think that's all.

Congratulations, you found the unmaintained spot in the block layer! :-)

Anyway, the cover letter makes sense to me. And thanks for this great
writeup! Can we turn it into documentation? Throttling seems to be quite
underdocumented, and your explanation above let me understand for the
first time what these *-max options actually were.

I'll have to see whether I can review the meat of this series next week,
but for now:

Patches 1-7: Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [PATCH v1 01/22] s390: use FILE instead of QEMUFile for creating text file

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> The s390 skeys monitor command needs to write out a plain text
> file. Currently it is using the QEMUFile class for this. There
> is no real benefit to this, and the downside is that it needs to
> snprintf via an intermediate buffer. Switching to regular FILE
> objects simplifies the code.
> 
> Signed-off-by: Daniel P. Berrange 

Reviewed-by: Dr. David Alan Gilbert 

Dave

> ---
>  hw/s390x/s390-skeys.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 539ef6d..637ccf5 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -45,15 +45,11 @@ void s390_skeys_init(void)
>  qdev_init_nofail(DEVICE(obj));
>  }
>  
> -static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
> +static void write_keys(FILE *f, uint8_t *keys, uint64_t startgfn,
> uint64_t count, Error **errp)
>  {
>  uint64_t curpage = startgfn;
>  uint64_t maxpage = curpage + count - 1;
> -const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> -  " ch=%d, reserved=%d\n";
> -char buf[128];
> -int len;
>  
>  for (; curpage <= maxpage; curpage++) {
>  uint8_t acc = (*keys & 0xF0) >> 4;
> @@ -62,10 +58,9 @@ static void write_keys(QEMUFile *f, uint8_t *keys, 
> uint64_t startgfn,
>  int ch = (*keys & 0x02);
>  int res = (*keys & 0x01);
>  
> -len = snprintf(buf, sizeof(buf), fmt, curpage,
> -   *keys, acc, fp, ref, ch, res);
> -assert(len < sizeof(buf));
> -qemu_put_buffer(f, (uint8_t *)buf, len);
> +fprintf(f, "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> +" ch=%d, reserved=%d\n",
> +curpage, *keys, acc, fp, ref, ch, res);
>  keys++;
>  }
>  }
> @@ -115,7 +110,7 @@ void qmp_dump_skeys(const char *filename, Error **errp)
>  vaddr cur_gfn = 0;
>  uint8_t *buf;
>  int ret;
> -QEMUFile *f;
> +FILE *f;
>  
>  /* Quick check to see if guest is using storage keys*/
>  if (!skeyclass->skeys_enabled(ss)) {
> @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp)
>  return;
>  }
>  
> -f = qemu_fopen(filename, "wb");
> +f = fopen(filename, "wb");
>  if (!f) {
>  error_setg_file_open(errp, errno, filename);
>  return;
> @@ -161,7 +156,7 @@ out_free:
>  error_propagate(errp, lerr);
>  g_free(buf);
>  out:
> -qemu_fclose(f);
> +fclose(f);
>  }
>  
>  static void qemu_s390_skeys_init(Object *obj)
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] IVSHMEM support on qemu-system-arm

[Markus Armbruster]

sridhar kulkarni  writes:

> Hi,
> I am working on a requirement where I need to have shared memory IPC
> between two guest VM's running different OS. Both VM's use vexpress-A9
> boards. I came across the "ivshmem" implementation and have some
> questions. Whether the "ivshmem" implementation can be used on
> qemu-system-arm? I understand that "ivshmem" device is a PCI device
> and require PCI bus to connect to. Does vexpress board support this
> kind of arrangement?
> RegardsSridhar

What are you trying to accomplish?

I'm asking because "shared memory" is means towards an end.  Really,
really low-level means, in fact.  Requirements can compel choice of
certain means, but means cannot be requirements.  What are your *real*
requirements?

Re: [Qemu-devel] [PATCH] target-arm: Move bank_number() into internals.h

[Sergey Fedorov]

On 12.02.2016 18:50, Peter Maydell wrote:
> Move bank_number()'s implementation into internals.h, so
> it's available in the user-mode-only compile as well.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Sergey Fedorov 

> ---
> Embarrassingly light on testing on that last change.
>
>  target-arm/helper.c| 25 -
>  target-arm/internals.h | 26 +-
>  2 files changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index c46e3d0..a420a2a 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5378,31 +5378,6 @@ void aarch64_sync_64_to_32(CPUARMState *env)
>  
>  #else
>  
> -/* Map CPU modes onto saved register banks.  */
> -int bank_number(int mode)
> -{
> -switch (mode) {
> -case ARM_CPU_MODE_USR:
> -case ARM_CPU_MODE_SYS:
> -return BANK_USRSYS;
> -case ARM_CPU_MODE_SVC:
> -return BANK_SVC;
> -case ARM_CPU_MODE_ABT:
> -return BANK_ABT;
> -case ARM_CPU_MODE_UND:
> -return BANK_UND;
> -case ARM_CPU_MODE_IRQ:
> -return BANK_IRQ;
> -case ARM_CPU_MODE_FIQ:
> -return BANK_FIQ;
> -case ARM_CPU_MODE_HYP:
> -return BANK_HYP;
> -case ARM_CPU_MODE_MON:
> -return BANK_MON;
> -}
> -g_assert_not_reached();
> -}
> -
>  void switch_mode(CPUARMState *env, int mode)
>  {
>  int old_mode;
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 70bec4a..2e70272 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -109,7 +109,31 @@ static inline unsigned int 
> aarch64_banked_spsr_index(unsigned int el)
>  return map[el];
>  }
>  
> -int bank_number(int mode);
> +/* Map CPU modes onto saved register banks.  */
> +static inline int bank_number(int mode)
> +{
> +switch (mode) {
> +case ARM_CPU_MODE_USR:
> +case ARM_CPU_MODE_SYS:
> +return BANK_USRSYS;
> +case ARM_CPU_MODE_SVC:
> +return BANK_SVC;
> +case ARM_CPU_MODE_ABT:
> +return BANK_ABT;
> +case ARM_CPU_MODE_UND:
> +return BANK_UND;
> +case ARM_CPU_MODE_IRQ:
> +return BANK_IRQ;
> +case ARM_CPU_MODE_FIQ:
> +return BANK_FIQ;
> +case ARM_CPU_MODE_HYP:
> +return BANK_HYP;
> +case ARM_CPU_MODE_MON:
> +return BANK_MON;
> +}
> +g_assert_not_reached();
> +}
> +
>  void switch_mode(CPUARMState *, int);
>  void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
>  void arm_translate_init(void);

Re: [Qemu-devel] [PATCH v1 20/22] migration: support TLS encryption with TCP migration backend

[Daniel P. Berrange]

On Fri, Feb 12, 2016 at 05:09:52PM +, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berra...@redhat.com) wrote:
> > This extends the TCP migration backend so that it can make use
> > of QIOChannelTLS to provide transparent TLS encryption. To
> > trigger enablement the URI on the incoming and outgoing sides
> > should have 'tls-creds=ID' appended, eg
> > 
> >tcp:$HOST:$PORT,tls-creds=ID
> > 
> > where ID is the object identifier of a set of TLS credentials
> > previously created using object_add / -object. There is not
> > currently any support for checking the migration client
> > certificate names against ACLs. This is pending a conversion
> > of the ACL code to QOM.
> 
> Does that change the option passed or is that just different
> in the way the tls-creds are set up?

It will mean another new paramter will be added. For example
the above command will become something like this:

   tcp:$HOST:$PORT,tls-creds=ID,auth=ID

where 'auth=ID' provides the ID of an object implementing
the authentication/authorization check

> > +typedef struct {
> > +MigrationState *s;
> > +QCryptoTLSCreds *tlscreds;
> > +char *hostname;
> > +} TCPConnectData;
> > +
> > +typedef struct {
> > +MigrationState *s;
> > +QCryptoTLSCreds *tlscreds;
> > +} TCPListenData;
> > +
> > +typedef struct {
> > +MigrationState *s;
> > +QIOChannel *ioc;
> > +} TCPConnectTLSData;
> > +
> 
> what makes it TCP specific rather than sharing most of this between 
> transports? i.e. should
> this work for fd migration ? (rdma is probably not reasonable
> giving it's scribbling directly in the other hosts RAM).
> Certainly those types dont really look TCP specific.

TLS as a protocol doesn't have any limitations, but as part of having
the client validate the x509 certificates, it needs to have a hostname
or IP address to validate against the certificate. This is available
for TCP and RDMA, but there's no user provided hostname for unix,
exec, and fd migration protocols.

We could extend the syntax for each of those, so that the user could
provide a hostname, and that would then allow us to wire up TLS for
that backend. If we did that, then it would make sense to have the
TLS setup in a separate migration/tls.c file, that we could activate
over all channels.

> > +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
> > +  bool is_listen,
> > +  Error **errp)
> > +{
> > +char *credname = NULL;
> > +Object *creds;
> > +QCryptoTLSCreds *ret;
> > +
> > +credname = tcp_get_opt_str(host_port, "tls-creds");
> > +if (!credname) {
> > +return NULL;
> > +}
> 
> At what point does it get saner just to throw host_port into a qemu_opts 
> and let it parse it?

Possibly quite soon :-)

> > +static void tcp_outgoing_migration_tls(Object *src,
> > +   Error *err,
> > +   gpointer opaque)
> > +{
> > +TCPConnectTLSData *data = opaque;
> > +QIOChannel *ioc = QIO_CHANNEL(src);
> > +
> > +if (err) {
> > +DPRINTF("migrate connect TLS error: %s\n", error_get_pretty(err));
> 
> error_report ?

Is it desirable for code triggered from the QMP/HMP monitor to be
doing that. I thought we were not supposed todo that in general.
Normally we could propagate an error back to the monitor, but
since migrate happens in the background, we've no way todo that.
We really need to be able to feed the errors back to the client
app over QMP somehow, but there's no mechanism for that right
now :-(

Perhaps 'info migrate' should have a field to report the actual
error message when status == failed ?  Or we could define an
async event that we emit to the client with the actual error.
Either would make life nicer for libvirt, which can't report
any good errors - this is the main reason libvirt currently
does all TCP connection setup itself and uses fd:, and not
tcp:.  Libvirt will need to use tcp: again though, to get
TLS working, unless we add a 'hostname' field to the 'fd:'
protocol

> > +data->s->file = NULL;
> > +migrate_fd_error(data->s);
> > +} else {
> > +DPRINTF("migrate connect success\n");
> 
> trace_

Yep, I mentioned in a repl to a previous review, that I
intend to do a single cleanup patch at the end which
adds some consistent trace points across all the backend
drivers for migrate.

> > +data->s->file = qemu_fopen_channel_output(ioc);
> > +migrate_fd_connect(data->s);
> > +}
> > +}
> > +
> > +
> >  static void tcp_outgoing_migration(Object *src,
> > Error *err,
> > gpointer opaque)
> >  {
> > -MigrationState *s = opaque;
> > +TCPConnectData *data = opaque;
> >  QIOChannel *sioc = QIO_CHANNEL(src);
> >  
> >  if (err) {
> >  DPRINTF("migrate connect error: %s\n", 

Re: [Qemu-devel] [Bug 1545024] [NEW] compiling on armv7 crashes compile qlx.o

[Peter Maydell]

On 12 February 2016 at 15:13, Klaftenegger Felix
<1545...@bugs.launchpad.net> wrote:
> Public bug reported:
>
> If i try to compile qemu on armv7 cpu i get this error:
>
>   LINK  qemu-nbd
>   CCqemu-img.o
>   LINK  qemu-img
>   LINK  qemu-io
>   LINK  qemu-bridge-helper
>   CCqmp-marshal.o
>   CChw/display/qxl.o
> {standard input}: Assembler messages:
> {standard input}:1704: Error: bad instruction `lock'
> {standard input}:1704: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:1864: Error: bad instruction `lock'
> {standard input}:1864: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:5239: Error: bad instruction `lock'
> {standard input}:5239: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:5731: Error: bad instruction `lock'
> {standard input}:5731: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:11923: Error: bad instruction `lock'
> {standard input}:11923: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:13960: Error: bad instruction `lock'
> {standard input}:13960: Error: bad instruction `addl $0,0(%rsp)'
> {standard input}:14349: Error: bad instruction `lock'
> {standard input}:14349: Error: bad instruction `addl $0,0(%rsp)'
> /home/fleixi/git/qemu/rules.mak:57: recipe for target 'hw/display/qxl.o' 
> failed
> make: *** [hw/display/qxl.o] Error 1

Looks like we're picking up the x86 implementations of the atomic.h
smp_mb() somehow. That's odd because it shouldn't happen unless
something has managed to #define __i386__ somehow.

What host platform (OS, hardware) are you building on? What version is your
compiler?

> Shoulded the configure detecting "bigendian" too?

Why should it? ARM is little endian.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Peter Maydell]

On 12 February 2016 at 15:16, Edgar E. Iglesias
 wrote:
> On Fri, Feb 12, 2016 at 03:15:22PM +, Peter Maydell wrote:
>> On 12 February 2016 at 15:12, Edgar E. Iglesias
>>  wrote:
>> > On Thu, Feb 11, 2016 at 07:11:48PM +, Peter Maydell wrote:
>> >> The user-mode versions of get/set_r13_banked() exist just to assert
>> >> if they're ever called -- the translate time code should never
>> >> emit calls to them because SRS from user mode always UNDEF.
>> >> There's no code in the softmmu versions that can't compile in
>> >> CONFIG_USER_ONLY, so combine the two functions rather than
>> >> having completely split versions under ifdefs.
>> >>
>> >> Signed-off-by: Peter Maydell 
>> >
>> > Hi Peter,
>> >
>> > Do we really need the assert?
>> > If we keep it, can't we have it for both -user and -softmmu (avoiding the 
>> > ifdef)?
>>
>> I would be happy to entirely drop the assert, yes.
>
> OK, thanks.

It turns out that the compiler was being clever and dropping all
the code after the g_assert_not_reached(), which meant I didn't
notice that bank_number() is only compiled in in the softmmu build.
We should just move bank_number() into being a static inline in
internals.h, I think -- it's pretty trivial. Will send that patch...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all of of coroutine context

[Denis V. Lunev]

On 02/12/2016 03:55 PM, Paolo Bonzini wrote:


On 12/02/2016 13:50, Dr. David Alan Gilbert wrote:

I'll admit to not really understanding what the difference is
between bh and coroutine context; I'd thought if it was all
in the main thread stuff was safe.

It's arguably a bug in the block layer code.  It assumes that all code
called from a coroutine wants not to block.  Moving stuff to a bottom
half tells the block layer that you want bdrv_invalidate_cache_all to block.

Paolo

cool description! Really short and exhaustive!

[Qemu-devel] [PATCH] target-arm: Move bank_number() into internals.h

[Peter Maydell]

Move bank_number()'s implementation into internals.h, so
it's available in the user-mode-only compile as well.

Signed-off-by: Peter Maydell 
---
Embarrassingly light on testing on that last change.

 target-arm/helper.c| 25 -
 target-arm/internals.h | 26 +-
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index c46e3d0..a420a2a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5378,31 +5378,6 @@ void aarch64_sync_64_to_32(CPUARMState *env)
 
 #else
 
-/* Map CPU modes onto saved register banks.  */
-int bank_number(int mode)
-{
-switch (mode) {
-case ARM_CPU_MODE_USR:
-case ARM_CPU_MODE_SYS:
-return BANK_USRSYS;
-case ARM_CPU_MODE_SVC:
-return BANK_SVC;
-case ARM_CPU_MODE_ABT:
-return BANK_ABT;
-case ARM_CPU_MODE_UND:
-return BANK_UND;
-case ARM_CPU_MODE_IRQ:
-return BANK_IRQ;
-case ARM_CPU_MODE_FIQ:
-return BANK_FIQ;
-case ARM_CPU_MODE_HYP:
-return BANK_HYP;
-case ARM_CPU_MODE_MON:
-return BANK_MON;
-}
-g_assert_not_reached();
-}
-
 void switch_mode(CPUARMState *env, int mode)
 {
 int old_mode;
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 70bec4a..2e70272 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -109,7 +109,31 @@ static inline unsigned int 
aarch64_banked_spsr_index(unsigned int el)
 return map[el];
 }
 
-int bank_number(int mode);
+/* Map CPU modes onto saved register banks.  */
+static inline int bank_number(int mode)
+{
+switch (mode) {
+case ARM_CPU_MODE_USR:
+case ARM_CPU_MODE_SYS:
+return BANK_USRSYS;
+case ARM_CPU_MODE_SVC:
+return BANK_SVC;
+case ARM_CPU_MODE_ABT:
+return BANK_ABT;
+case ARM_CPU_MODE_UND:
+return BANK_UND;
+case ARM_CPU_MODE_IRQ:
+return BANK_IRQ;
+case ARM_CPU_MODE_FIQ:
+return BANK_FIQ;
+case ARM_CPU_MODE_HYP:
+return BANK_HYP;
+case ARM_CPU_MODE_MON:
+return BANK_MON;
+}
+g_assert_not_reached();
+}
+
 void switch_mode(CPUARMState *, int);
 void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
 void arm_translate_init(void);
-- 
1.9.1

[Qemu-devel] [PATCH v2 0/4] target-arm: Clean up trap/undef handling of SRS

[Peter Maydell]

The SRS instruction is a bit of an oddity because it isn't
used by Linux these days. Nonetheless it has a bunch of
UNPREDICTABLE, UNDEF and trapping behaviour that we weren't
correctly implementing:

 - trap to EL3 if EL3 is AArch64 and we are at Secure EL1
 - UNDEFINED in Hyp mode
 - UNPREDICTABLE in User or System mode
 - UNPREDICTABLE if the specified mode is:
 -- not implemented
 -- not a valid mode number
 -- a mode that's at a higher exception level
 -- Monitor, if we are Non-secure

This series implements the checks we were missing and makes
us UNDEF for all the UNPREDICTABLE cases.

Patch 1 does the easy checks that can be done at translate time;
patches 2 and 3 are code motion in preparation for patch 4, which
puts in a run-time check for the one awkward case we don't have
enough information to UNDEF at translate time.

Changes v1->v2: drop the user-mode-only assertions from the
get_/set_r13_banked() functions as suggested in review.

These have all now been reviewed so I'm just sending them out
to the list for completeness -- I'll put the patches in
target-arm.next in a moment.


thanks
-- PMM


Peter Maydell (4):
  target-arm: Clean up trap/undef handling of SRS
  target-arm: Move get/set_r13_banked() to op_helper.c
  target-arm: Combine user-only and softmmu get/set_r13_banked()
  target-arm: UNDEF in the UNPREDICTABLE SRS-from-System case

 target-arm/helper.c| 33 -
 target-arm/op_helper.c | 26 
 target-arm/translate.c | 67 ++
 3 files changed, 88 insertions(+), 38 deletions(-)

-- 
1.9.1

Re: [Qemu-devel] [PATCH] build: Don't redefine 'inline'

[Eric Blake]

On 02/12/2016 06:24 AM, Peter Maydell wrote:
> On 9 February 2016 at 18:49, Eric Blake  wrote:
>> Actively redefining 'inline' is wrong for C++, where gcc has an
>> extension 'inline namespace' which fails to compile if the
>> keyword 'inline' is replaced by a macro expansion.  This will
>> matter once we start to include "qemu/osdep.h" first from C++
>> files, depending also on whether the system headers are new
>> enough to be using the gcc extension.
>>
>> But rather than just guard things by __cplusplus, let's look at
>> the overall picture.  Commit df2542c737ea2 in 2007 defined 'inline'
>> to the gcc attribute __always_inline__, with the rationale "To
>> avoid discarded inlining bug".  But compilers have improved since
>> then, and we are probably better off trusting the compiler rather
>> than trying to force its hand.
>>
>> So just nuke our craziness.
>>
>> Signed-off-by: Eric Blake 
> 
> Reviewed-by: Peter Maydell 
> 
> (and tested that it passes my usual merge build tests).
> 
> Does this patch suffice to get your system to build all
> my clean-includes patches?

Yes.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

[Peter Maydell]

The user-mode versions of get/set_r13_banked() exist just to assert
if they're ever called -- the translate time code should never
emit calls to them because SRS from user mode always UNDEF.
There's no code in the softmmu versions that can't compile in
CONFIG_USER_ONLY, and the assertion is not particularly useful,
so combine the two functions rather than having completely split
versions under ifdefs.

Signed-off-by: Peter Maydell 
Reviewed-by: Edgar E. Iglesias 
---
 target-arm/op_helper.c | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 053e9b6..cfdbc8d 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -457,24 +457,6 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
regno, uint32_t val)
 }
 }
 
-#if defined(CONFIG_USER_ONLY)
-void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 write\n");
-}
-
-uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 read\n");
-return 0;
-}
-
-#else
-
 void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
 {
 if ((env->uncached_cpsr & CPSR_M) == mode) {
@@ -492,7 +474,6 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t 
mode)
 return env->banked_r13[bank_number(mode)];
 }
 }
-#endif
 
 void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
syndrome,
  uint32_t isread)
-- 
1.9.1

[Qemu-devel] [Bug 1545052] [NEW] RDMA migration will hang forever if target QEMU fails to load vmstate

[Daniel Berrange]

Public bug reported:

Get a pair of machines with infiniband support. On one host run

$ qemu-system-x86_64 -monitor stdio -incoming rdma:ibme: -vnc :1 -m
1000

To start an incoming migration.


Now on the other host, run QEMU with an intentionally different configuration 
(ie different RAM size)

$ qemu-system-x86_64 -monitor stdio -vnc :1 -m 2000

Now trigger a migration on this source host

(qemu) migrate rdma:ibpair:


You will see on the target host, that it failed to load migration:

dest_init RDMA Device opened: kernel name mlx4_0 uverbs device name uverbs0, 
infiniband_verbs class device path /sys/class/infiniband_verbs/uverbs0, 
infiniband class device path /sys/class/infiniband/mlx4_0, transport: (2) 
Ethernet
qemu-system-x86_64: Length mismatch: pc.ram: 0x7d00 in != 0x3e80: 
Invalid argument
qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram'

This is to be expected, however, at this point QEMU has hung and no
longer responds to the monitor

GDB shows the target host is stuck in this callpath

#0  0x739141cd in write () at ../sysdeps/unix/syscall-template.S:81
#1  0x727fe795 in rdma_get_cm_event.part.15 () from 
/lib64/librdmacm.so.1
#2  0x5593e445 in qemu_rdma_cleanup (rdma=0x7fff9647e010) at 
migration/rdma.c:2210
#3  0x5593ea45 in qemu_rdma_close (opaque=0x57796770) at 
migration/rdma.c:2652
#4  0x559397cc in qemu_fclose (f=f@entry=0x564b1450) at 
migration/qemu-file.c:270
#5  0x55936b88 in process_incoming_migration_co (opaque=0x564b1450) 
at migration/migration.c:361
#6  0x55a25a1a in coroutine_trampoline (i0=, 
i1=) at util/coroutine-ucontext.c:79
#7  0x7fffef5b3110 in ?? () from /lib64/libc.so.6


Now, back on the source host again, you would expect to see that the
migrate command failed. Instead, this QEMU is hung too.

GDB shows the source host, migrate thread, is stuck in this callpath:

#0  0x7391522d in read#1  0x700efd93 in ibv_get_cq_event () at 
/lib64/libibverbs.so.1
#2  0x559403f2 in qemu_rdma_block_for_wrid 
(rdma=rdma@entry=0x7fff3d07e010, wrid_requested=wrid_requested@entry=4000, 
byte_len=byte_len@entry=0x7fff39de370c) at migration/rdma.c:1511
#3  0x5594058a in qemu_rdma_exchange_get_response (rdma=0x7fff3d07e010, 
head=head@entry=0x7fff39de3780, expecting=expecting@entry=2, idx=idx@entry=0)
at migration/rdma.c:1648
#4  0x55941e71 in qemu_rdma_exchange_send (rdma=0x7fff3d07e010, 
head=0x7fff39de3840, data=0x0, resp=0x7fff39de3870, resp_idx=0x7fff39de3880, 
callback=0x0) at migration/rdma.c:1725
#5  0x559447e4 in qemu_rdma_registration_stop (f=, 
opaque=, flags=0, data=) at migration/rdma.c:3302
#6  0x5593bc4b in ram_control_after_iterate (f=f@entry=0x564c20f0, 
flags=flags@entry=0) at migration/qemu-file.c:157
#7  0x55740b59 in ram_save_setup (f=0x564c20f0, opaque=) at /home/berrange/src/virt/qemu/migration/ram.c:1959
#8  0x557451c1 in qemu_savevm_state_begin (f=0x564c20f0, 
params=params@entry=0x55f6f048 )
at /home/berrange/src/virt/qemu/migration/savevm.c:919
#9  0x559381a5 in migration_thread (opaque=0x55f6f000 
) at migration/migration.c:1633
#10 0x7390edc5 in start_thread (arg=0x7fff39de4700) at 
pthread_create.c:308


It should have aborted migrate and set the status to failed.

** Affects: qemu
 Importance: Undecided
 Status: Confirmed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1545052

Title:
  RDMA migration will hang forever if target QEMU fails to load vmstate

Status in QEMU:
  Confirmed

Bug description:
  Get a pair of machines with infiniband support. On one host run

  $ qemu-system-x86_64 -monitor stdio -incoming rdma:ibme: -vnc :1
  -m 1000

  To start an incoming migration.

  
  Now on the other host, run QEMU with an intentionally different configuration 
(ie different RAM size)

  $ qemu-system-x86_64 -monitor stdio -vnc :1 -m 2000

  Now trigger a migration on this source host

  (qemu) migrate rdma:ibpair:

  
  You will see on the target host, that it failed to load migration:

  dest_init RDMA Device opened: kernel name mlx4_0 uverbs device name uverbs0, 
infiniband_verbs class device path /sys/class/infiniband_verbs/uverbs0, 
infiniband class device path /sys/class/infiniband/mlx4_0, transport: (2) 
Ethernet
  qemu-system-x86_64: Length mismatch: pc.ram: 0x7d00 in != 0x3e80: 
Invalid argument
  qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram'

  This is to be expected, however, at this point QEMU has hung and no
  longer responds to the monitor

  GDB shows the target host is stuck in this callpath

  #0  0x739141cd in write () at ../sysdeps/unix/syscall-template.S:81
  #1  0x727fe795 in rdma_get_cm_event.part.15 () from