Re: [Qemu-devel] [PATCH 4/6] s390x: Add I/O adapter registration.

[Christian Borntraeger]

On 25/02/14 18:25, Cornelia Huck wrote:

> +int kvm_s390_io_adapter_map(uint32_t id, uint64_t map_addr, bool do_map)
> +{
> +struct kvm_s390_io_adapter_req req = {
> +.id = id,
> +.type = do_map ? KVM_S390_IO_ADAPTER_MAP : KVM_S390_IO_ADAPTER_UNMAP,
> +.addr = map_addr,
> +};
> +KVMS390FLICState *flic = s390_get_flic();
> +struct kvm_device_attr attr;

Can we use designated initializer for attr, e.g.
struct kvm_device_attr attr = {
.group = KVM_DEV_FLIC_ADAPTER_MODIFY,
.addr = (uint64_t)&req,
}

> +int r;
> +
> +if (!flic) {
> +return -ENOSYS;
> +}
> +if (!kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING)) {
> +return -ENOSYS;
> +}
> +
> +attr.group = KVM_DEV_FLIC_ADAPTER_MODIFY;
> +attr.addr = (uint64_t)&req;

and not do it here. That will zero out the other fields of attr.

Same for the register code.

Christian

Re: [Qemu-devel] [PATCH 4/6] s390x: Add I/O adapter registration.

[Cornelia Huck]

On Wed, 26 Feb 2014 09:29:12 +0100
Christian Borntraeger  wrote:

> On 25/02/14 18:25, Cornelia Huck wrote:
> 
> > +int kvm_s390_io_adapter_map(uint32_t id, uint64_t map_addr, bool do_map)
> > +{
> > +struct kvm_s390_io_adapter_req req = {
> > +.id = id,
> > +.type = do_map ? KVM_S390_IO_ADAPTER_MAP : 
> > KVM_S390_IO_ADAPTER_UNMAP,
> > +.addr = map_addr,
> > +};
> > +KVMS390FLICState *flic = s390_get_flic();
> > +struct kvm_device_attr attr;
> 
> Can we use designated initializer for attr, e.g.
> struct kvm_device_attr attr = {
> .group = KVM_DEV_FLIC_ADAPTER_MODIFY,
> .addr = (uint64_t)&req,
> }
> 
> > +int r;
> > +
> > +if (!flic) {
> > +return -ENOSYS;
> > +}
> > +if (!kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING)) {
> > +return -ENOSYS;
> > +}
> > +
> > +attr.group = KVM_DEV_FLIC_ADAPTER_MODIFY;
> > +attr.addr = (uint64_t)&req;
> 
> and not do it here. That will zero out the other fields of attr.
> 
> Same for the register code.
> 

Hm, yes, I missed that. I'll change it for the next version.

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 13:00:03 +0800
Hu Tao  wrote:

> On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
> > Il 25/02/2014 11:20, Hu Tao ha scritto:
> > >There is a problem that user_creatable_complete() is called before
> > >adding object to "/objects" (see object_create()), which triggers a
> > >assert failure when calling object_get_canonical_path() in
> > >ram_backend_memory_init(). Any ideas?
> > 
> > You can call object_property_add_child before calling
> > user_creatable_complete, and then call object_unparent (in addition
> > to object_unref) if creation fails.
> > 
> > Paolo
> 
> Something like this?
> 
> From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
> From: Hu Tao 
> Date: Wed, 26 Feb 2014 12:54:34 +0800
> Subject: [PATCH] call user_creatable_complete() after adding object to
>  "/objects"
> 
> This makes it possible to get the path of object by calling
> object_get_canonical_path() in the complete callback if
> someone wants it.
> 
> Signed-off-by: Hu Tao 
> ---
>  vl.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 1d27b34..30b4297 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void *opaque)
>  goto out;
>  }
>  
> +object_property_add_child(container_get(object_get_root(), "/objects"),
> +  id, obj, &local_err);
> +
>  user_creatable_complete(obj, &local_err);
>  if (local_err) {
> +object_unparent(obj);
>  goto out;
>  }
>  
> -object_property_add_child(container_get(object_get_root(), "/objects"),
> -  id, obj, &local_err);
> -
>  out:
>  object_unref(obj);
>  if (local_err) {
Failure case is not handled properly,
I'm sorry that I've forgotten to mention prerequisite path
https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8

[Qemu-devel] What does these functions do?

[Atlas Khan]

qdev_init_gpio_out
qdev_init_gpio_in
qdev_set_legacy_instance_id

these functions are from QEMUThese are used in different devices. Can
any body plz tell me what does these do?

[Qemu-devel] Do not compile hw/ for SOFTMMU

[Jun Koi]

Currently hw/ is compiled in for SOFTMMU setup, but actually it is always
compiled no matter what. This patch removes the related line in
Makefile.objs.

Signed-off-by: Jun Koi 


diff --git a/Makefile.objs b/Makefile.objs
index 4a62913..15d75be 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -64,7 +64,6 @@ common-obj-$(CONFIG_POSIX) += migration-exec.o
migration-unix.o migration-fd.o
 common-obj-$(CONFIG_SPICE) += spice-qemu-char.o

 common-obj-y += audio/
-common-obj-y += hw/

 common-obj-y += ui/
 common-obj-y += bt-host.o bt-vhci.o

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 06:57, Hu Tao ha scritto:

If lines about memory polices are moved up to hostmem.c, the only thing
left in ram_backend_memory_init() is calling memory_region_init_ram() to
allocate memory. Then it comes a problem that when to apply memory
polices? Choices:

1. apply memory polices in hostmem.c since this is the place user sets
   memory polices. But user_creatable_complete() seems not to support
   this.( but fix me)

2. cast to HostMemoryBackend in ram_backend_memory_init() (or in other
   memory backends) and add lines to apply memory polices.

3. provide an interface in HostMemoryBackendClass to do the thing and
   call it in subclasses. (this is basically the same as 2 except that
   we can reuse code)


I like (3).  I understand it's something like

void memory_backend_apply_mempolicy(HostMemoryBackend *be,
void *addr, size_t len, Error **err)

?

Paolo

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 13:57:06 +0800
Hu Tao  wrote:

> On Wed, Feb 19, 2014 at 10:36:57AM +0100, Igor Mammedov wrote:
> > On Wed, 19 Feb 2014 10:03:13 +0100
> > Paolo Bonzini  wrote:
> > 
> > >   19/02/2014 08:54, Hu Tao ha scritto:
> > > > Thus makes user control how to allocate memory for ram backend.
> > > >
> > > > Signed-off-by: Hu Tao 
> > > > ---
> > > >  backends/hostmem-ram.c  | 158 
> > > > 
> > > >  include/sysemu/sysemu.h |   2 +
> > > >  2 files changed, 160 insertions(+)
> > > >
> > > > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > [...]
> > 
> > > >  static int
> > > >  ram_backend_memory_init(HostMemoryBackend *backend, Error **errp)
> > > >  {
> > > > +HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(backend);
> > > > +int mode = ram_backend->policy;
> > > > +void *p;
> > > > +unsigned long maxnode;
> > > > +
> > > >  if (!memory_region_size(&backend->mr)) {
> > > >  memory_region_init_ram(&backend->mr, OBJECT(backend),
> > > > 
> > > > object_get_canonical_path(OBJECT(backend)),
> > > > backend->size);
> > > > +
> > > > +p = memory_region_get_ram_ptr(&backend->mr);
> > > > +maxnode = find_last_bit(ram_backend->host_nodes, MAX_NODES);
> > > > +
> > > > +mode |= ram_backend->relative ? MPOL_F_RELATIVE_NODES :
> > > > +MPOL_F_STATIC_NODES;
> > > > +/* This is a workaround for a long standing bug in Linux'
> > > > + * mbind implementation, which cuts off the last specified
> > > > + * node. To stay compatible should this bug be fixed, we
> > > > + * specify one more node and zero this one out.
> > > > + */
> > > > +if (syscall(SYS_mbind, p, backend->size, mode,
> > > > +ram_backend->host_nodes, maxnode + 2, 0)) {
> > > 
> > > This does not compile on non-Linux; also, does libnuma include the 
> > > workaround?  If so, this is a hint that we should be using libnuma 
> > > instead...
> > > 
> > > Finally, all this code should be in hostmem.c, not hostmem-ram.c, 
> > > because the same policies can be applied to hugepage-backed memory.
> > > 
> > > Currently host_memory_backend_get_memory is calling bc->memory_init. 
> > > Probably the call should be replaced by something like
> > I've pushed to github updated version of memdev, where
> > host_memory_backend_get_memory() is just convenience wrapper to get
> > access to memdev's internal MemoryRegion.
> > 
> > All initialization now is done in user_creatable->complete() method
> > which calls ram_backend_memory_init() so leaving it as is should be fine.
> 
> If lines about memory polices are moved up to hostmem.c, the only thing
> left in ram_backend_memory_init() is calling memory_region_init_ram() to
> allocate memory. Then it comes a problem that when to apply memory
> polices? Choices:
> 
> 1. apply memory polices in hostmem.c since this is the place user sets
>memory polices. But user_creatable_complete() seems not to support
>this.( but fix me)
if we assume that NUMA policies apply to every hostmem derived backend,
then we could realize() approach used by DEVICE. i.e.
set NUMA policies in hostmem.c:hostmemory_backend_memory_init()

Add parent_complete field to ram-backend class and store there parent's
complete pointer. Then we can do:

ram_backend_memory_init(UserCreatable *uc, Error **errp) {
memory_region_init_ram();
...
MEMORY_BACKEND_RAM_CLASS(uc)->parent_complete(uc, errp);
...
}

> 
> 2. cast to HostMemoryBackend in ram_backend_memory_init() (or in other
>memory backends) and add lines to apply memory polices.
> 
> 3. provide an interface in HostMemoryBackendClass to do the thing and
>call it in subclasses. (this is basically the same as 2 except that
>we can reuse code)
> 
> Opinions?
> 
> 

Re: [Qemu-devel] [PATCH v3] XBZRLE: Fix qemu crash when resize the xbzrle cache

[Gonglei (Arei)]

Hi, Juan. Thanks for your review.

> -Original Message-
> From: Juan Quintela [mailto:quint...@redhat.com]
> Sent: Tuesday, February 25, 2014 11:25 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; Dr. David Alan Gilbert; owass...@redhat.com;
> chenliang (T); Huangweidong (C)
> Subject: Re: [PATCH v3] XBZRLE: Fix qemu crash when resize the xbzrle cache
> 
> "Gonglei (Arei)"  wrote:
> > Resizing the xbzrle cache during migration causes qemu-crash,
> > because the main-thread and migration-thread modify the xbzrle
> > cache size concurrently without lock-protection.
> >
> > Signed-off-by: ChenLiang 
> > Signed-off-by: Gonglei 
> > Reviewed-by: Dr. David Alan Gilbert 
> 
> Sorry for the late review.
> 
> > ---
> > Changes against the previous version:
> >  *Remove the temporary variable cache in the xbzrle_cache_resize.
> >
> >  arch_init.c | 58
> +++---
> >  1 file changed, 55 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 80574a0..003640f 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -164,26 +164,69 @@ static struct {
> >  uint8_t *encoded_buf;
> >  /* buffer for storing page content */
> >  uint8_t *current_buf;
> > -/* Cache for XBZRLE */
> > +/* Cache for XBZRLE, Protected by lock. */
> >  PageCache *cache;
> > +QemuMutex lock;
> >  } XBZRLE = {
> >  .encoded_buf = NULL,
> >  .current_buf = NULL,
> >  .cache = NULL,
> > +/* use the lock carefully */
> > +.lock = {PTHREAD_MUTEX_INITIALIZER},
> >  };
> 
> Have you tried to compile for windows?  I think this would make it
> break.  We need to init it with qemu_mutex_init() in ram_save_setup()
> after the call to cache_init()?

Yeah，it isn't compatible with windows. There is a problem that we must 
initialize the lock at qemu startup. 
Because the xbzrle_cache_resize can be called anytime. I don't know how to 
handle it in windows. 
Do you have any suggestion?
> 
> 
> 
> >  /* buffer used for XBZRLE decoding */
> >  static uint8_t *xbzrle_decoded_buf;
> >
> > +static void XBZRLE_cache_lock(void)
> > +{
> > +qemu_mutex_lock(&XBZRLE.lock);
> > +}
> > +
> 
> if we change this to:
> 
> if (migrate_use_xbzrle())
> qemu_mutex_lock(&XBZRLE.lock);
> 
> On both cache operations, we would remove the overhead in the no XBRLE
> case, right?

Check.
> 
> > +static void XBZRLE_cache_unlock(void)
> > +{
> > +qemu_mutex_unlock(&XBZRLE.lock);
> > +}
> > +
> >  int64_t xbzrle_cache_resize(int64_t new_size)
> >  {
> > +PageCache *new_cache, *old_cache;
> > +
> >  if (new_size < TARGET_PAGE_SIZE) {
> >  return -1;
> >  }
> >
> > +XBZRLE_cache_lock();
> >  if (XBZRLE.cache != NULL) {
> > -return cache_resize(XBZRLE.cache, new_size /
> TARGET_PAGE_SIZE) *
> > -TARGET_PAGE_SIZE;
> > +/* check XBZRLE.cache again later */
> > +XBZRLE_cache_unlock();
> > +if (pow2floor(new_size) == migrate_xbzrle_cache_size()) {
> > +return pow2floor(new_size);
> > +}
> > +new_cache = cache_init(new_size / TARGET_PAGE_SIZE,
> > +TARGET_PAGE_SIZE);
> > +if (!new_cache) {
> > +DPRINTF("Error creating cache\n");
> > +return -1;
> > +}
> > +
> > +XBZRLE_cache_lock();
> > +/* the XBZRLE.cache may have be destroyed, check it again */
> > +if (XBZRLE.cache != NULL) {
> > +old_cache = XBZRLE.cache;
> > +XBZRLE.cache = new_cache;
> > +new_cache = NULL;
> > +}
> > +XBZRLE_cache_unlock();
> > +
> > +if (NULL == new_cache) {
> > +cache_fini(old_cache);
> > +} else {
> > +cache_fini(new_cache);
> > +}
> > +} else {
> > +XBZRLE_cache_unlock();
> >  }
> 
> Can we change this to:
> 
> if (pow2floor(new_size) == migrate_xbzrle_cache_size()) {
> return pow2floor(new_size);
> }
> 
> new_cache = cache_init(new_size / TARGET_PAGE_SIZE,
> TARGET_PAGE_SIZE);
> if (!new_cache) {
> DPRINTF("Error creating cache\n");
> return -1;
> }
> 
> XBZRLE_cache_lock();
> /* the XBZRLE.cache may have be destroyed, check it again */
> if (XBZRLE.cache != NULL) {
> cache_to_free = XBZRLE.cache;
> XBZRLE.cache = new_cache;
> } else {
> cache_to_free = new_cache;
> }
> XBZRLE_cache_unlock();
> 
> cache_fini(cache_to_free);
> 
> I think that looking is clearer this way.  BTW, we are losing
> _everything_ that is on the cache if we resize it.  I don't know if that
> is what we want.  If we have a big cache, and make it bigger because we
> are having too many misses, just dropping the whole cache looks like a
> bad idea :-(
> 

Dropping the whole

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Hu Tao]

On Wed, Feb 26, 2014 at 09:47:08AM +0100, Igor Mammedov wrote:
> On Wed, 26 Feb 2014 13:00:03 +0800
> Hu Tao  wrote:
> 
> > On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
> > > Il 25/02/2014 11:20, Hu Tao ha scritto:
> > > >There is a problem that user_creatable_complete() is called before
> > > >adding object to "/objects" (see object_create()), which triggers a
> > > >assert failure when calling object_get_canonical_path() in
> > > >ram_backend_memory_init(). Any ideas?
> > > 
> > > You can call object_property_add_child before calling
> > > user_creatable_complete, and then call object_unparent (in addition
> > > to object_unref) if creation fails.
> > > 
> > > Paolo
> > 
> > Something like this?
> > 
> > From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
> > From: Hu Tao 
> > Date: Wed, 26 Feb 2014 12:54:34 +0800
> > Subject: [PATCH] call user_creatable_complete() after adding object to
> >  "/objects"
> > 
> > This makes it possible to get the path of object by calling
> > object_get_canonical_path() in the complete callback if
> > someone wants it.
> > 
> > Signed-off-by: Hu Tao 
> > ---
> >  vl.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index 1d27b34..30b4297 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void 
> > *opaque)
> >  goto out;
> >  }
> >  
> > +object_property_add_child(container_get(object_get_root(), "/objects"),
> > +  id, obj, &local_err);
> > +
> >  user_creatable_complete(obj, &local_err);
> >  if (local_err) {
> > +object_unparent(obj);
> >  goto out;
> >  }
> >  
> > -object_property_add_child(container_get(object_get_root(), "/objects"),
> > -  id, obj, &local_err);
> > -
> >  out:
> >  object_unref(obj);
> >  if (local_err) {
> Failure case is not handled properly,
> I'm sorry that I've forgotten to mention prerequisite path
> https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
> in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8

No problem. I should've noticed the patch when cherry-picking. Will you
post it seperately?

Re: [Qemu-devel] [RFC 2/4] Added flag in ARMCPU to track last execution mode

[alvise rigo]

I see, but how can we know the type of migration (KVM to TCG or TCG to
KVM) which is taking place only looking at the incoming data? Of course,
I'm supposing that the two cases will require different handling and so
that distinguish the migration type (at the destination) has to be part
of the process.

thanks,
alvise



On Tue, Feb 25, 2014 at 7:19 PM, Peter Maydell wrote:

> On 25 February 2014 16:52, Alvise Rigo 
> wrote:
> > The value of this flag indicates the execution mode of the CPU prior the
> > migration. It is used to handle the KVM <-> TCG migration.
> >
> > Signed-off-by: Alvise Rigo 
> > ---
> >  target-arm/cpu-qom.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index afbd422..6819bfc 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -102,6 +102,9 @@ typedef struct ARMCPU {
> >   */
> >  uint32_t kvm_target;
> >
> > +/* true if this cpu is using KVM. Read and set in cpu_pre/post_load
> */
> > +bool running_kvm;
>
> This is definitely wrong. We should not care whether either
> end of the migration connection is using KVM.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [RFC 3/4] Add l2ctlr cp register to CPUARMState

[alvise rigo]

This was a polite way to alter the value of the register in order to not
fail the
migration when KVM migrates a value different from the TCG one.
KVM in particular does not set the irq bit while TCG does.

alvise


On Tue, Feb 25, 2014 at 7:22 PM, Peter Maydell wrote:

> On 25 February 2014 16:52, Alvise Rigo 
> wrote:
> > Since the irq bit seems to not be updated, exclude it from the check done
> > while copying data during migration.
> >
> > Signed-off-by: Alvise Rigo 
> > ---
> >  target-arm/cpu.c | 22 ++
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 6e7ce89..e8db00e 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -681,20 +681,34 @@ static void cortex_a9_initfn(Object *obj)
> >  }
> >
> >  #ifndef CONFIG_USER_ONLY
> > -static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo
> *ri)
> > +static void a15_l2ctlr_reset(CPUARMState *env, const ARMCPRegInfo
> *opaque)
> >  {
> >  /* Linux wants the number of processors from here.
> >   * Might as well set the interrupt-controller bit too.
> >   */
> > -return ((smp_cpus - 1) << 24) | (1 << 23);
> > +env->cp15.c9_l2ctlr = ((smp_cpus - 1) << 24) | (1 << 23);
> > +}
> > +
> > +static void a15_l2ctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> > +   uint64_t value)
> > +{
> > +int smp_cpus_new = ((value >> 24) & 3);
> > +int smp_cpus_old = ((env->cp15.c9_l2ctlr >> 24) & 3);
> > +
> > +if (smp_cpus_new != smp_cpus_old) {
> > +return;
> > +}
> > +
> > +env->cp15.c9_l2ctlr = value;
> >  }
> >  #endif
> >
> >  static const ARMCPRegInfo cortexa15_cp_reginfo[] = {
> >  #ifndef CONFIG_USER_ONLY
> >  { .name = "L2CTLR", .cp = 15, .crn = 9, .crm = 0, .opc1 = 1, .opc2
> = 2,
> > -  .access = PL1_RW, .resetvalue = 0, .readfn = a15_l2ctlr_read,
> > -  .writefn = arm_cp_write_ignore, },
> > +  .access = PL1_RW, .resetvalue = 0,
> > +  .resetfn = a15_l2ctlr_reset, .writefn = a15_l2ctlr_write,
> > +  .fieldoffset = offsetof(CPUARMState, cp15.c9_l2ctlr) },
> >  #endif
>
> Why are you changing the behaviour of this register from
> read-only/writes-ignored to permitting writes? This looks
> wrong.
>
> thanks
> -- PMM
>

[Qemu-devel] [PATCH] translate: remove file translate-all.h

[Xuebing Wang]

This patch does below:
-   Move the declaration of 2 translate functions from translate-all.h into
include/exec/exec-all.h
-   remove file translate-all.h

"translate-all.h" => "exec/exec-all.h" can be done by:
git grep -w "translate-all.h" | cut -d: -f1 |
xargs sed -i 's/\/exec\/exec-all.h/g'

Note:
1)  "exact whole word match" is considered.
2)  We may move translate related from include/exec/exec-all.h into
include/exec/translate.h later.

Signed-off-by: Xuebing Wang 
---
 exec.c  |2 +-
 include/exec/exec-all.h |4 
 translate-all.c |2 +-
 translate-all.h |   27 ---
 4 files changed, 6 insertions(+), 29 deletions(-)
 delete mode 100644 translate-all.h

diff --git a/exec.c b/exec.c
index b69fd29..ec3890e 100644
--- a/exec.c
+++ b/exec.c
@@ -47,7 +47,7 @@
 #include "exec/cpu-all.h"
 
 #include "exec/cputlb.h"
-#include "translate-all.h"
+#include "exec/exec-all.h"
 
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a387922..f9afb7a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -213,6 +213,10 @@ void tb_free(TranslationBlock *tb);
 void tb_flush(CPUArchState *env);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 
+/* translate-all.c */
+void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
+void tb_check_watchpoint(CPUArchState *env);
+
 #if defined(USE_DIRECT_JUMP)
 
 #if defined(CONFIG_TCG_INTERPRETER)
diff --git a/translate-all.c b/translate-all.c
index 1ac0246..6379b1a 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -57,7 +57,7 @@
 #endif
 
 #include "exec/cputlb.h"
-#include "translate-all.h"
+#include "exec/exec-all.h"
 #include "qemu/timer.h"
 
 //#define DEBUG_TB_INVALIDATE
diff --git a/translate-all.h b/translate-all.h
deleted file mode 100644
index f7e5932..000
--- a/translate-all.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- *  Translated block handling
- *
- *  Copyright (c) 2003 Fabrice Bellard
- *
- * 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 .
- */
-#ifndef TRANSLATE_ALL_H
-#define TRANSLATE_ALL_H
-
-/* translate-all.c */
-void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
-void cpu_unlink_tb(CPUState *cpu);
-void tb_check_watchpoint(CPUArchState *env);
-
-#endif /* TRANSLATE_ALL_H */
-- 
1.7.9.5

[Qemu-devel] [PATCH 1/1] block: use /var/tmp instead of /tmp for -snapshot

[Amit Shah]

If TMPDIR is not specified, the default was to use /tmp for the working
copy of the block devices.  Update this to /var/tmp instead, so systems
using tmp-on-tmpfs don't end up inadvertently using RAM for the block
device.

Signed-off-by: Amit Shah 
---
 block.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 2fd5482..38bbdf3 100644
--- a/block.c
+++ b/block.c
@@ -547,8 +547,9 @@ int get_tmp_filename(char *filename, int size)
 int fd;
 const char *tmpdir;
 tmpdir = getenv("TMPDIR");
-if (!tmpdir)
-tmpdir = "/tmp";
+if (!tmpdir) {
+tmpdir = "/var/tmp";
+}
 if (snprintf(filename, size, "%s/vl.XX", tmpdir) >= size) {
 return -EOVERFLOW;
 }
-- 
1.8.5.3

Re: [Qemu-devel] [PATCH v3 12/31] target-arm: Implement AArch64 TTBR*

[Peter Maydell]

On 26 February 2014 06:33, Hu Tao  wrote:
> On Sat, Feb 15, 2014 at 04:07:05PM +, Peter Maydell wrote:
>
> <...>
>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 06953ac..7cbe69b 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -173,10 +173,8 @@ typedef struct CPUARMState {
>>  uint32_t c1_coproc; /* Coprocessor access register.  */
>>  uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
>>  uint32_t c1_scr; /* secure config register.  */
>> -uint32_t c2_base0; /* MMU translation table base 0.  */
>> -uint32_t c2_base0_hi; /* MMU translation table base 0, high 32 bits 
>> */
>> -uint32_t c2_base1; /* MMU translation table base 0.  */
>> -uint32_t c2_base1_hi; /* MMU translation table base 1, high 32 bits 
>> */
>> +uint64_t ttbr0_el1; /* MMU translation table base 0. */
>> +uint32_t ttbr1_el1; /* MMU translation table base 1. */
>
> s/32/64/

Nice catch, not sure how I missed that.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/6] qdev: add get_pointer_and_free() for temporary strings

[Igor Mammedov]

On Fri, 21 Feb 2014 15:46:56 +0100
Stefan Hajnoczi  wrote:

> get_pointer() assumes the string has unspecified lifetime (at least as
> long as the object is alive).  In some cases we can only produce a
> temporary string that should be freed when get_pointer() is done.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/core/qdev-properties-system.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 5f5957e..3bffc01 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -31,6 +31,20 @@ static void get_pointer(Object *obj, Visitor *v, Property 
> *prop,
>  visit_type_str(v, &p, name, errp);
>  }
>  
> +/* Same as get_pointer() but frees heap-allocated print() return value */
> +static void get_pointer_and_free(Object *obj, Visitor *v, Property *prop,
> + char *(*print)(void *ptr),
> + const char *name, Error **errp)
> +{
> +DeviceState *dev = DEVICE(obj);
> +void **ptr = qdev_get_prop_ptr(dev, prop);
> +char *p;
> +
> +p = *ptr ? print(*ptr) : g_strdup("");
> +visit_type_str(v, &p, name, errp);
> +g_free(p);
> +}
it could be better if get_pointer() would free pointer if 3 current users
are converted to return temporary string, something like this:

From 65dbdb27e6ccc555e84ddf898d015827fec04d9c Mon Sep 17 00:00:00 2001
From: Igor Mammedov 
Date: Wed, 26 Feb 2014 10:37:36 +0100
Subject: [PATCH] qdev: make get_pointer() handle temporary strings

get_pointer()'s print() callback might return a heap allocated
string, to avoid adding dedicated get_pointer_foo for this case
convert current print() callbacks to return temporary heap
allocated string and make get_pointer() free it.

Signed-off-by: Igor Mammedov 
---
 hw/core/qdev-properties-system.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 5f5957e..d865ea3 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -27,8 +27,9 @@ static void get_pointer(Object *obj, Visitor *v, Property 
*prop,
 void **ptr = qdev_get_prop_ptr(dev, prop);
 char *p;
 
-p = (char *) (*ptr ? print(*ptr) : "");
+p = (char *) (*ptr ? print(*ptr) : g_strdup(""));
 visit_type_str(v, &p, name, errp);
+g_free(p);
 }
 
 static void set_pointer(Object *obj, Visitor *v, Property *prop,
@@ -93,7 +94,7 @@ static void release_drive(Object *obj, const char *name, void 
*opaque)
 
 static const char *print_drive(void *ptr)
 {
-return bdrv_get_device_name(ptr);
+return g_strdup(bdrv_get_device_name(ptr));
 }
 
 static void get_drive(Object *obj, Visitor *v, void *opaque,
@@ -148,8 +149,9 @@ static void release_chr(Object *obj, const char *name, void 
*opaque)
 static const char *print_chr(void *ptr)
 {
 CharDriverState *chr = ptr;
+char *val = chr->label ? chr->label : "";
 
-return chr->label ? chr->label : "";
+return g_strdup(val);
 }
 
 static void get_chr(Object *obj, Visitor *v, void *opaque,
@@ -227,8 +229,9 @@ err:
 static const char *print_netdev(void *ptr)
 {
 NetClientState *netdev = ptr;
+char *val = netdev->name ? netdev->name : "";
 
-return netdev->name ? netdev->name : "";
+return g_strdup(val);
 }
 
 static void get_netdev(Object *obj, Visitor *v, void *opaque,
-- 
1.7.1



>  static void set_pointer(Object *obj, Visitor *v, Property *prop,
>  int (*parse)(DeviceState *dev, const char *str,
>   void **ptr),

Re: [Qemu-devel] [PATCH] translate: remove file translate-all.h

[Peter Maydell]

On 26 February 2014 09:25, Xuebing Wang  wrote:
> This patch does below:
> -   Move the declaration of 2 translate functions from translate-all.h into
> include/exec/exec-all.h
> -   remove file translate-all.h
>
> "translate-all.h" => "exec/exec-all.h" can be done by:
> git grep -w "translate-all.h" | cut -d: -f1 |
> xargs sed -i 's/\/exec\/exec-all.h/g'
>
> Note:
> 1)  "exact whole word match" is considered.
> 2)  We may move translate related from include/exec/exec-all.h into
> include/exec/translate.h later.

Is there any particular benefit to this change? The function
prototypes go from being in a header only included by the file
that uses them to being in a header that's included by a lot
of other code...

thanks
-- PMM

Re: [Qemu-devel] [RFC 2/4] Added flag in ARMCPU to track last execution mode

[Peter Maydell]

On 26 February 2014 09:16, alvise rigo  wrote:
> I see, but how can we know the type of migration (KVM to TCG or TCG to
> KVM) which is taking place only looking at the incoming data? Of course,
> I'm supposing that the two cases will require different handling and so
> that distinguish the migration type (at the destination) has to be part
> of the process.

That's my point -- the two cases should not require different handling:
you should just handle the migration and succeed if possible or
fail if not, based on the incnoming data alone.

thanks
-- PMM

Re: [Qemu-devel] [RFC 4/4] Relevant changes to enable KVM to TCG migration

[alvise rigo]

I agree that this is a sort of workaround, but it seems to me that a
proper solution is not possible without changing the ideas contemplated
now in the migration code.
Are we willing to accept some major changes in the code to embrace
this type of migration?

Thanks,
alvise


On Tue, Feb 25, 2014 at 7:25 PM, Peter Maydell wrote:

> On 25 February 2014 16:52, Alvise Rigo 
> wrote:
> > CPUARMState:
> > * added adfsr cp register.
> > * added aifsr cp register.
> > These registers have been added because they are migrated by KVM. This
> prevents
> > the migration from failing when trying to copy their values.
>
> This should be done in a separate patch.
>
> > ARMCPRegInfo:
> > * added a pointer to the parent that generated the register (if any).
> > * a flag to inform that we have already copied the value of the register:
> >   this prevents the register from being overwritten by the parent
> register
> >   value, which could be not set.
> >
> > helper.c:
> > * added mechanism to track the cp register "parent".
> > * compare_cpreg_array(): compare the incoming cp registers with the
> >   cpreg_list keeping a list of the registers that do not succeeded the
> >   match.
> > * handle_cpreg_kvm2tcg_migration(): try to solve the mismatch of
> >   cp registers coming from KVM; without this additional step the
> >   migration would fail even if it's feasible.
> >
> > Signed-off-by: Alvise Rigo 
> > ---
> >  target-arm/cpu.h |  43 +
> >  target-arm/helper.c  | 167
> +--
> >  target-arm/machine.c |  46 ++
> >  3 files changed, 238 insertions(+), 18 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 3c8a2db..a97246d 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -184,6 +184,8 @@ typedef struct CPUARMState {
> >  MPU write buffer control.  */
> >  uint32_t c5_insn; /* Fault status registers.  */
> >  uint32_t c5_data;
> > +uint32_t c5_adfsr;
> > +uint32_t c5_aifsr;
> >  uint32_t c6_region[8]; /* MPU base/size registers.  */
> >  uint32_t c6_insn; /* Fault address registers.  */
> >  uint32_t c6_data;
> > @@ -197,6 +199,7 @@ typedef struct CPUARMState {
> >  uint32_t c9_pmxevtyper; /* perf monitor event type */
> >  uint32_t c9_pmuserenr; /* perf monitor user enable */
> >  uint32_t c9_pminten; /* perf monitor interrupt enables */
> > +uint32_t c9_l2ctlr; /* L2 Control Register */
> >  uint32_t c12_vbar; /* vector base address register */
> >  uint32_t c13_fcse; /* FCSE PID.  */
> >  uint32_t c13_context; /* Context ID.  */
> > @@ -867,6 +870,15 @@ struct ARMCPRegInfo {
> >  uint8_t opc0;
> >  uint8_t opc1;
> >  uint8_t opc2;
> > +/* When migrating this flag tells if the register's value has
> already
> > + * been copied. This is used in some unlikely cases where KVM
> migrates
> > + * a register that in TCG is generated by a wildcarded or
> > + * ARM_CP_STATE_BOTH parent (hence a not migratable register);
> > + * in those cases, we will set the field "parent" to point to the
> > + * generating register. Most of the time this field can stay false.
> > + * */
> > +bool skip_cpreglist;
>
> This reads to me like it's papering over a problem rather
> than fixing it.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [RFC 4/4] Relevant changes to enable KVM to TCG migration

[Peter Maydell]

On 26 February 2014 10:02, alvise rigo  wrote:
> I agree that this is a sort of workaround, but it seems to me that a
> proper solution is not possible without changing the ideas contemplated
> now in the migration code.
> Are we willing to accept some major changes in the code to embrace
> this type of migration?

Well, the migration code as it stands is in theory supposed to cope
with KVM<->TCG migration. I would expect that we'd need to improve
a number of places where our TCG cpreg emulation is wrong.

thanks
-- PMM

Re: [Qemu-devel] [RFC 3/4] Add l2ctlr cp register to CPUARMState

[Peter Maydell]

On 26 February 2014 09:17, alvise rigo  wrote:
> This was a polite way to alter the value of the register in order to not
> fail the
> migration when KVM migrates a value different from the TCG one.
> KVM in particular does not set the irq bit while TCG does.

You mean the "interrupt controller" bit 23? That seems like a bug
in the KVM emulation of this register which should be fixed in KVM.
Alternatively if it's a documentation error and the actual hardware
A15 doesn't set this bit (ie if it's not set when you're running outside
KVM) then we should fix the TCG emulation not to set the bit.

thanks
-- PMM

[Qemu-devel] qemu-img convert cache mode for source

[Peter Lieven]

Hi,

I was wondering if it would be a good idea to set the O_DIRECT mode for the 
source
files of a qemu-img convert process if the source is a host_device?

Currently the backup of a host device is polluting the page cache.

Peter

Re: [Qemu-devel] [PATCH] block/vmdk: do not report file offset for compressed extents

[Fam Zheng]

On Wed, 02/26 10:47, Peter Lieven wrote:
> Signed-off-by: Peter Lieven 
> ---
>  block/vmdk.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index ff6f5ee..5fa29b0 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1146,7 +1146,7 @@ static int64_t coroutine_fn 
> vmdk_co_get_block_status(BlockDriverState *bs,
>  break;
>  case VMDK_OK:
>  ret = BDRV_BLOCK_DATA;
> -if (extent->file == bs->file) {
> +if (extent->file == bs->file && !extent->compressed) {
>  ret |= BDRV_BLOCK_OFFSET_VALID | offset;
>  }
>  
> -- 
> 1.7.9.5
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH] modules: Fix building with --enable-modules

[Paolo Bonzini]

Il 26/02/2014 06:56, Fam Zheng ha scritto:

Compiling util/modules.c with modules enabled fails now.

Fix it by:

1) Add "#define CONFIG_MODULES" with --enable-modules

2) Include qemu-common.h before #ifdef testing in module.c.

Signed-off-by: Fam Zheng 
---
 scripts/create_config | 3 +++
 util/module.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/create_config b/scripts/create_config
index 546f889..a0654c6 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -30,6 +30,9 @@ case $line in
 value=${line#*=}
 echo "#define CONFIG_IASL $value"
 ;;
+ CONFIG_MODULES=y)
+echo "#define CONFIG_MODULES 1"
+;;


Not needed:

 CONFIG_*=y) # configuration
name=${line%=*}
echo "#define $name 1"
;;

Paolo


  CONFIG_AUDIO_DRIVERS=*)
 drivers=${line#*=}
 echo "#define CONFIG_AUDIO_DRIVERS \\"
diff --git a/util/module.c b/util/module.c
index dc08c16..863a8a3 100644
--- a/util/module.c
+++ b/util/module.c
@@ -14,10 +14,10 @@
  */

 #include 
+#include "qemu-common.h"
 #ifdef CONFIG_MODULES
 #include 
 #endif
-#include "qemu-common.h"
 #include "qemu/queue.h"
 #include "qemu/module.h"

Re: [Qemu-devel] [RFC 4/4] Relevant changes to enable KVM to TCG migration

[alvise rigo]

Improvements of cpreg emulation apart, we will still need to tackle
with the great amount of registers not migrated because generated
by a wildcarded register; some of these registers are in fact migrated
by KVM. I think that the idea of keeping a reference
to the generator register (i.e. the only register which is migrated in a
wildcarded case) could be considered to handle the migration.

alvise


On Wed, Feb 26, 2014 at 11:04 AM, Peter Maydell wrote:

> On 26 February 2014 10:02, alvise rigo 
> wrote:
> > I agree that this is a sort of workaround, but it seems to me that a
> > proper solution is not possible without changing the ideas contemplated
> > now in the migration code.
> > Are we willing to accept some major changes in the code to embrace
> > this type of migration?
>
> Well, the migration code as it stands is in theory supposed to cope
> with KVM<->TCG migration. I would expect that we'd need to improve
> a number of places where our TCG cpreg emulation is wrong.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v3 31/31] target-arm: Add v8 mmu translation support

[Peter Maydell]

On 26 February 2014 03:32, Hu Tao  wrote:
> On Wed, Feb 26, 2014 at 10:49:59AM +0800, Hu Tao wrote:
>> On Sat, Feb 15, 2014 at 04:07:24PM +, Peter Maydell wrote:
>> > From: Rob Herring 

>> >  /* Determine whether this address is in the region controlled by
>> >   * TTBR0 or TTBR1 (or if it is in neither region and should fault).
>> >   * This is a Non-secure PL0/1 stage 1 translation, so controlled by
>> >   * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32:
>> >   */
>> > -uint32_t t0sz = extract32(env->cp15.c2_control, 0, 3);
>> > -uint32_t t1sz = extract32(env->cp15.c2_control, 16, 3);
>> > -if (t0sz && !extract32(address, 32 - t0sz, t0sz)) {
>> > +uint32_t t0sz = extract32(env->cp15.c2_control, 0, 5);
>> > +uint32_t t1sz = extract32(env->cp15.c2_control, 16, 5);
>>
>> t0sz is bit [5:0], so shouldn't we extract 6 bits here? same for t1sz.

Yes.

>> > +if (t0sz && !extract64(address, va_size - t0sz, t0sz)) {
>> >  /* there is a ttbr0 region and we are in it (high bits all zero) 
>> > */
>> >  ttbr_select = 0;
>> > -} else if (t1sz && !extract32(~address, 32 - t1sz, t1sz)) {
>> > +} else if (t1sz && !extract64(~address, va_size - t1sz, t1sz)) {
>> >  /* there is a ttbr1 region and we are in it (high bits all one) */
>> >  ttbr_select = 1;
>> >  } else if (!t0sz) {
>>
>> Can't be true for Aarch64. the VA address space has a maximum address width
>> of 48 bits(page D5-1712 of ARM DDI 0487A.a), that means t0sz and t1sz should
>> have a minimum value of 16.
>
> It doesn't matter here. Maybe we should check the value when writing to
> TCR_EL1. What's the behaviour when writing an invalid tsz to TCR_EL1?

I haven't checked through all the details, but it looks like the answer is
you can write anything, and the pseudocode for AArch64.TranslationTableWalk
specifies what happens if the tsz is outside the expected range (we
clamp tablesize to be 25 <= tablesize <= 48).

I'm not sure we've correctly implemented the handling specified under
the AddrTop() pseudocode function either.

thanks
-- PMM

Re: [Qemu-devel] [RFC 4/4] Relevant changes to enable KVM to TCG migration

[Peter Maydell]

On 26 February 2014 10:27, alvise rigo  wrote:
> Improvements of cpreg emulation apart, we will still need to tackle
> with the great amount of registers not migrated because generated
> by a wildcarded register; some of these registers are in fact migrated
> by KVM. I think that the idea of keeping a reference
> to the generator register (i.e. the only register which is migrated in a
> wildcarded case) could be considered to handle the migration.

These probably need to be fixed by being actually implemented in
TCG. If there's a register which we wildcard on TCG that means
we should usually fail the incoming migration, because the guest
might be actually using that register for something and we would
just be dropping the state in migration if we allowed the migration
to succeed.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 10:10, Igor Mammedov ha scritto:

if we assume that NUMA policies apply to every hostmem derived backend,
then we could realize() approach used by DEVICE. i.e.
set NUMA policies in hostmem.c:hostmemory_backend_memory_init()

Add parent_complete field to ram-backend class and store there parent's
complete pointer. Then we can do:

ram_backend_memory_init(UserCreatable *uc, Error **errp) {
memory_region_init_ram();
...
MEMORY_BACKEND_RAM_CLASS(uc)->parent_complete(uc, errp);
...
}



The problem is that some backends might not be handled the same way. 
For example, not all backends might produce a single void*/size_t pair 
for the entire region.  Think of a "composite" backend that produces a 
large memory region from two smaller ones.


Paolo

Re: [Qemu-devel] [PATCH] translate: remove file translate-all.h

[Paolo Bonzini]

Il 26/02/2014 10:55, Peter Maydell ha scritto:

On 26 February 2014 09:25, Xuebing Wang  wrote:

This patch does below:
-   Move the declaration of 2 translate functions from translate-all.h into
include/exec/exec-all.h
-   remove file translate-all.h

"translate-all.h" => "exec/exec-all.h" can be done by:
git grep -w "translate-all.h" | cut -d: -f1 |
xargs sed -i 's/\/exec\/exec-all.h/g'

Note:
1)  "exact whole word match" is considered.
2)  We may move translate related from include/exec/exec-all.h into
include/exec/translate.h later.


Is there any particular benefit to this change? The function
prototypes go from being in a header only included by the file
that uses them to being in a header that's included by a lot
of other code...


Indeed, that's the point of translate-all.h.

Paolo

[Qemu-devel] [PATCH v4 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions

[Will Newton]

Add support for AArch32 CRC32 and CRC32C instructions added in ARMv8
and add a CPU feature flag to enable these instructions.

The CRC32-C implementation used is the built-in qemu implementation
and The CRC-32 implementation is from zlib. This requires adding zlib
to LIBS to ensure it is linked for the linux-user binary.

Signed-off-by: Will Newton 
---
 configure  |  2 +-
 target-arm/cpu.c   |  1 +
 target-arm/cpu.h   |  1 +
 target-arm/helper.c| 39 +++
 target-arm/helper.h|  3 +++
 target-arm/translate.c | 56 ++
 6 files changed, 101 insertions(+), 1 deletion(-)

Changes in v4:
 - Add feature flag for CRC instructions
 - Check for SBZ bits in ARM encoding
 - Use more accurate flags for the helper definition

diff --git a/configure b/configure
index 423f435..030da86 100755
--- a/configure
+++ b/configure
@@ -1657,7 +1657,7 @@ EOF
 "Make sure to have the zlib libs and headers installed."
 fi
 fi
-libs_softmmu="$libs_softmmu -lz"
+LIBS="$LIBS -lz"
 
 ##
 # libseccomp check
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 6e7ce89..8231e57 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -922,6 +922,7 @@ static void arm_any_initfn(Object *obj)
 set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
 set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
 set_feature(&cpu->env, ARM_FEATURE_V7MP);
+set_feature(&cpu->env, ARM_FEATURE_CRC);
 #ifdef TARGET_AARCH64
 set_feature(&cpu->env, ARM_FEATURE_AARCH64);
 #endif
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3c8a2db..c3aeb74 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -615,6 +615,7 @@ enum arm_features {
 ARM_FEATURE_AARCH64, /* supports 64 bit mode */
 ARM_FEATURE_V8_AES, /* implements AES part of v8 Crypto Extensions */
 ARM_FEATURE_CBAR, /* has cp15 CBAR */
+ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1b111b6..5e4f0fa 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5,6 +5,8 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
 #include "qemu/bitops.h"
+#include "qemu/crc32c.h"
+#include  /* For crc32 */
 
 #ifndef CONFIG_USER_ONLY
 static inline int get_phys_addr(CPUARMState *env, uint32_t address,
@@ -4392,3 +4394,40 @@ int arm_rmode_to_sf(int rmode)
 }
 return rmode;
 }
+
+static void crc_init_buffer(uint8_t *buf, uint32_t val, uint32_t bytes)
+{
+memset(buf, 0, 4);
+
+if (bytes == 1) {
+buf[0] = val & 0xff;
+} else if (bytes == 2) {
+buf[0] = val & 0xff;
+buf[1] = (val >> 8) & 0xff;
+} else {
+buf[0] = val & 0xff;
+buf[1] = (val >> 8) & 0xff;
+buf[2] = (val >> 16) & 0xff;
+buf[3] = (val >> 24) & 0xff;
+}
+}
+
+uint32_t HELPER(crc32)(uint32_t acc, uint32_t val, uint32_t bytes)
+{
+uint8_t buf[4];
+
+crc_init_buffer(buf, val, bytes);
+
+/* zlib crc32 converts the accumulator and output to one's complement.  */
+return crc32(acc ^ 0x, buf, bytes) ^ 0x;
+}
+
+uint32_t HELPER(crc32c)(uint32_t acc, uint32_t val, uint32_t bytes)
+{
+uint8_t buf[4];
+
+crc_init_buffer(buf, val, bytes);
+
+/* Linux crc32c converts the output to one's complement.  */
+return crc32c(acc, buf, bytes) ^ 0x;
+}
diff --git a/target-arm/helper.h b/target-arm/helper.h
index 19bd620..9738e49 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -497,6 +497,9 @@ DEF_HELPER_3(neon_qzip32, void, env, i32, i32)
 DEF_HELPER_4(crypto_aese, void, env, i32, i32, i32)
 DEF_HELPER_4(crypto_aesmc, void, env, i32, i32, i32)
 
+DEF_HELPER_FLAGS_3(crc32, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
+DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
+
 #ifdef TARGET_AARCH64
 #include "helper-a64.h"
 #endif
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6ccf0ba..253d2a1 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7561,6 +7561,36 @@ static void disas_arm_insn(CPUARMState * env, 
DisasContext *s)
 store_reg(s, 14, tmp2);
 gen_bx(s, tmp);
 break;
+case 0x4:
+{
+/* crc32/crc32c */
+uint32_t c = extract32(insn, 8, 4);
+
+/* Check this CPU supports ARMv8 CRC instructions.
+ * op1 == 3 is UNPREDICTABLE but handle as UNDEFINED.
+ * Bits 8, 10 and 11 should be zero.
+ */
+if (!arm_feature(env, ARM_FEATURE_CRC) || op1 == 0x3 ||
+(c & 0xd) != 0) {
+goto illegal_op;
+}
+
+rn = extract32(insn, 16, 4);
+rd = extract32(insn, 12, 4);
+
+tmp = load_reg(s, rn);
+tmp2 = load_reg(s, rm);
+tmp3 = tcg_const_i32(1 << op1);
+   

[Qemu-devel] [PATCH v4 1/2] include/qemu/crc32c.h: Rename include guards to match filename

[Will Newton]

Signed-off-by: Will Newton 
Reviewed-by: Peter Maydell 
---
 include/qemu/crc32c.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Changes in v4:
 - None

diff --git a/include/qemu/crc32c.h b/include/qemu/crc32c.h
index 56d1c3b..dafb6a1 100644
--- a/include/qemu/crc32c.h
+++ b/include/qemu/crc32c.h
@@ -25,8 +25,8 @@
  *
  */
 
-#ifndef QEMU_CRC32_H
-#define QEMU_CRC32_H
+#ifndef QEMU_CRC32C_H
+#define QEMU_CRC32C_H
 
 #include "qemu-common.h"
 
-- 
1.8.1.4

[Qemu-devel] [PATCH v4 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions

[Will Newton]

This series adds support for the AArch32 CRC32 instructions added in
ARMv8.

Will Newton (2):
  include/qemu/crc32c.h: Rename include guards to match filename
  target-arm: Add support for AArch32 ARMv8 CRC32 instructions

 configure  |  2 +-
 include/qemu/crc32c.h  |  4 ++--
 target-arm/cpu.c   |  1 +
 target-arm/cpu.h   |  1 +
 target-arm/helper.c| 39 +++
 target-arm/helper.h|  3 +++
 target-arm/translate.c | 56 ++
 7 files changed, 103 insertions(+), 3 deletions(-)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v4 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions

[Peter Maydell]

On 26 February 2014 10:46, Will Newton  wrote:
> This series adds support for the AArch32 CRC32 instructions added in
> ARMv8.
>
> Will Newton (2):
>   include/qemu/crc32c.h: Rename include guards to match filename
>   target-arm: Add support for AArch32 ARMv8 CRC32 instructions

Reviewed-by: Peter Maydell 

Thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 3/6] iothread: add I/O thread object

[Igor Mammedov]

On Fri, 21 Feb 2014 15:46:55 +0100
Stefan Hajnoczi  wrote:

> This is a stand-in for Michael Roth's QContext.  I expect this to be
> replaced once QContext is completed.
> 
> The IOThread object is an AioContext event loop thread.  This patch adds
> the concept of multiple event loop threads, allowing users to define
> them.
> 
> When SMP guests run on SMP hosts it makes sense to instantiate multiple
> IOThreads.  This spreads event loop processing across multiple cores.
> Note that additional patches are required to actually bind a device to
> an IOThread.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  Makefile.objs |   1 +
>  include/sysemu/iothread.h |  30 +++
>  iothread.c| 123 
> ++
>  3 files changed, 154 insertions(+)
>  create mode 100644 include/sysemu/iothread.h
>  create mode 100644 iothread.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index ac1d0e1..e24b89c 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -42,6 +42,7 @@ libcacard-y += libcacard/vcardt.o
>  
>  ifeq ($(CONFIG_SOFTMMU),y)
>  common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/
> +common-obj-y += iothread.o
>  common-obj-y += net/
>  common-obj-y += qdev-monitor.o device-hotplug.o
>  common-obj-$(CONFIG_WIN32) += os-win32.o
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> new file mode 100644
> index 000..a32214a
> --- /dev/null
> +++ b/include/sysemu/iothread.h
> @@ -0,0 +1,30 @@
> +/*
> + * Event loop thread
> + *
> + * Copyright Red Hat Inc., 2013
> + *
> + * Authors:
> + *  Stefan Hajnoczi   
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef IOTHREAD_H
> +#define IOTHREAD_H
> +
> +#include "block/aio.h"
> +
> +#define TYPE_IOTHREAD "iothread"
> +
> +typedef struct IOThread IOThread;
> +
> +#define IOTHREAD(obj) \
> +   OBJECT_CHECK(IOThread, obj, TYPE_IOTHREAD)
> +
> +IOThread *iothread_find(const char *id);
> +char *iothread_get_id(IOThread *iothread);
> +AioContext *iothread_get_aio_context(IOThread *iothread);
> +
> +#endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> new file mode 100644
> index 000..033de7f
> --- /dev/null
> +++ b/iothread.c
> @@ -0,0 +1,123 @@
> +/*
> + * Event loop thread
> + *
> + * Copyright Red Hat Inc., 2013
> + *
> + * Authors:
> + *  Stefan Hajnoczi   
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qom/object.h"
> +#include "qom/object_interfaces.h"
> +#include "qemu/module.h"
> +#include "qemu/thread.h"
> +#include "block/aio.h"
> +#include "sysemu/iothread.h"
> +
> +#define IOTHREADS_PATH "/objects"
> +
> +typedef ObjectClass IOThreadClass;
> +struct IOThread {
> +Object parent;
> +QemuThread thread;
> +AioContext *ctx;
> +bool stopping;
> +};
> +
> +#define IOTHREAD_GET_CLASS(obj) \
> +   OBJECT_GET_CLASS(IOThreadClass, obj, TYPE_IOTHREAD)
> +#define IOTHREAD_CLASS(klass) \
> +   OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD)
> +
> +static void *iothread_run(void *opaque)
> +{
> +IOThread *iothread = opaque;
> +
> +while (!iothread->stopping) {
> +aio_context_acquire(iothread->ctx);
> +while (!iothread->stopping && aio_poll(iothread->ctx, true)) {
> +/* Progress was made, keep going */
> +}
> +aio_context_release(iothread->ctx);
> +}
> +return NULL;
> +}
> +
> +static void iothread_instance_finalize(Object *obj)
> +{
> +IOThread *iothread = IOTHREAD(obj);
> +
> +iothread->stopping = true;
> +aio_notify(iothread->ctx);
> +qemu_thread_join(&iothread->thread);
> +aio_context_unref(iothread->ctx);
> +}
> +
> +static void iothread_complete(UserCreatable *obj, Error **errp)
> +{
> +IOThread *iothread = IOTHREAD(obj);
> +
> +iothread->stopping = false;
> +iothread->ctx = aio_context_new();
> +
> +/* This assumes we are called from a thread with useful CPU affinity for 
> us
> + * to inherit.
> + */
> +qemu_thread_create(&iothread->thread, iothread_run,
> +   iothread, QEMU_THREAD_JOINABLE);
> +}
> +
> +static void iothread_class_init(ObjectClass *klass, void *class_data)
> +{
> +UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
> +ucc->complete = iothread_complete;
> +}
> +
> +static const TypeInfo iothread_info = {
> +.name = TYPE_IOTHREAD,
> +.parent = TYPE_OBJECT,
> +.class_init = iothread_class_init,
> +.instance_size = sizeof(IOThread),
> +.instance_finalize = iothread_instance_finalize,
> +.interfaces = (InterfaceInfo[]) {
> +{TYPE_USER_CREATABLE},
> +{}
> +},
> +};
> +
> +static void iothread_register_types(void)
> +{
> +type_register_static(&iothread_info);
> +}
> +
> +type_init(iothread_register_types

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 09:47, Igor Mammedov ha scritto:

I'm sorry that I've forgotten to mention prerequisite path
https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8


Thanks, I'll add this patch to the numa-queue branch.

Paolo

[Qemu-devel] [PATCH 3/5] s390x/kvm: Add missing SIGP CPU RESET order

[Christian Borntraeger]

From: Thomas Huth 

The SIGP order CPU RESET was still missing in the list of our
supported handler. This patch now adds a simple implementation,
by using the cpu_reset() function that is already available in
target-s390x/cpu.c.

Signed-off-by: Thomas Huth 
Reviewed-by: Christian Borntraeger 
Reviewed-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 target-s390x/kvm.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 75e8822..20c711f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -635,6 +635,15 @@ static void sigp_initial_cpu_reset(void *arg)
 scc->initial_cpu_reset(cpu);
 }
 
+static void sigp_cpu_reset(void *arg)
+{
+CPUState *cpu = arg;
+S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+
+cpu_synchronize_state(cpu);
+scc->cpu_reset(cpu);
+}
+
 #define SIGP_ORDER_MASK 0x00ff
 
 static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
@@ -674,6 +683,10 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, 
uint8_t ipa1)
 run_on_cpu(CPU(target_cpu), sigp_initial_cpu_reset, CPU(target_cpu));
 cc = 0;
 break;
+case SIGP_CPU_RESET:
+run_on_cpu(CPU(target_cpu), sigp_cpu_reset, CPU(target_cpu));
+cc = 0;
+break;
 default:
 DPRINTF("KVM: unknown SIGP: 0x%x\n", order_code);
 *statusreg &= 0xUL;
-- 
1.8.4.2

[Qemu-devel] [PATCH 2/5] s390x/kvm: Rework SIGP INITIAL CPU RESET handler

[Christian Borntraeger]

From: Thomas Huth 

The s390_cpu_initial_reset() function had two deficiencies: First, it
used an ioctl for the destination CPU, and this ioctl could block
nearly forever, as long as the destination CPU was running in the SIE
loop. Second, it also cleared the general purpose registers - something
it should not do according to the Principles of Operations.
Since we've already got another function for the initial CPU reset in
cpu.c, we can also use that function instead. And by using run_on_cpu()
for executing this code, we make sure that the destination CPU is
correctly kicked out of kernel mode now.

Suggested-by: Christian Borntraeger 
Signed-off-by: Thomas Huth 
Reviewed-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 target-s390x/kvm.c | 25 +++--
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index d3f0d4a..75e8822 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -626,25 +626,13 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
 return 0;
 }
 
-static int s390_cpu_initial_reset(S390CPU *cpu)
+static void sigp_initial_cpu_reset(void *arg)
 {
-CPUState *cs = CPU(cpu);
-CPUS390XState *env = &cpu->env;
-int i;
-
-s390_del_running_cpu(cpu);
-if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL) < 0) {
-perror("cannot init reset vcpu");
-}
+CPUState *cpu = arg;
+S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
 
-/* Manually zero out all registers */
-cpu_synchronize_state(cs);
-for (i = 0; i < 16; i++) {
-env->regs[i] = 0;
-}
-
-DPRINTF("DONE: SIGP initial reset: %p\n", env);
-return 0;
+cpu_synchronize_state(cpu);
+scc->initial_cpu_reset(cpu);
 }
 
 #define SIGP_ORDER_MASK 0x00ff
@@ -683,7 +671,8 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, 
uint8_t ipa1)
 cc = 1;   /* status stored */
 break;
 case SIGP_INITIAL_CPU_RESET:
-cc = s390_cpu_initial_reset(target_cpu);
+run_on_cpu(CPU(target_cpu), sigp_initial_cpu_reset, CPU(target_cpu));
+cc = 0;
 break;
 default:
 DPRINTF("KVM: unknown SIGP: 0x%x\n", order_code);
-- 
1.8.4.2

[Qemu-devel] [PATCH 1/5] s390x/cpu: Use ioctl to reset state in the kernel

[Christian Borntraeger]

From: Thomas Huth 

Some of the state in the kernel can not be reset from QEMU yet.
For this we've got to use the KVM_S390_INITIAL_RESET ioctl to make
sure that the state in the kernel is set to the right values during
initial CPU reset, too.

Signed-off-by: Thomas Huth 
Acked-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 target-s390x/cpu.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index f1319e5..1a8c1cc 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -108,6 +108,15 @@ static void s390_cpu_initial_reset(CPUState *s)
 env->cregs[14] = CR14_RESET;
 
 env->pfault_token = -1UL;
+
+#if defined(CONFIG_KVM)
+/* Reset state inside the kernel that we cannot access yet from QEMU. */
+if (kvm_enabled()) {
+if (kvm_vcpu_ioctl(s, KVM_S390_INITIAL_RESET, NULL)) {
+perror("Initial CPU reset failed");
+}
+}
+#endif
 }
 
 /* CPUClass:reset() */
-- 
1.8.4.2

[Qemu-devel] [PATCH 0/5] s390x/kvm: additional fixes and cleanup

[Christian Borntraeger]

Folks,

here are 5 additional patches for s390x/kvm that I want to push for 2.0.

Frank Blaschka (1):
  s390x/kvm: Rework priv instruction handlers

Thomas Huth (4):
  s390x/cpu: Use ioctl to reset state in the kernel
  s390x/kvm: Rework SIGP INITIAL CPU RESET handler
  s390x/kvm: Add missing SIGP CPU RESET order
  s390x/ipl: Fix crash of ELF images with arbitrary entry points

 hw/s390x/ipl.c |  21 ---
 target-s390x/cpu.c |   9 +++
 target-s390x/kvm.c | 173 ++---
 3 files changed, 120 insertions(+), 83 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [PATCH 5/5] s390x/ipl: Fix crash of ELF images with arbitrary entry points

[Christian Borntraeger]

From: Thomas Huth 

When loading S390 kernels, the current code expects an ELF file with the
start address 0x1. Other ELF files cause a segmentation fault. To avoid
these crashes, we should get the start address from the ELF file instead
of always using a hard-coded address.

Signed-off-by: Thomas Huth 
Acked-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 hw/s390x/ipl.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 1a6397b..04fb1a8 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -95,7 +95,8 @@ static int s390_ipl_init(SysBusDevice *dev)
 }
 return 0;
 } else {
-kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+uint64_t pentry = KERN_IMAGE_START;
+kernel_size = load_elf(ipl->kernel, NULL, NULL, &pentry, NULL,
NULL, 1, ELF_MACHINE, 0);
 if (kernel_size == -1) {
 kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
@@ -104,15 +105,19 @@ static int s390_ipl_init(SysBusDevice *dev)
 fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
 return -1;
 }
-/* we have to overwrite values in the kernel image, which are "rom" */
-strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
-
 /*
- * we can not rely on the ELF entry point, since up to 3.2 this
- * value was 0x800 (the SALIPL loader) and it wont work. For
- * all (Linux) cases 0x1 (KERN_IMAGE_START) should be fine.
+ * Is it a Linux kernel (starting at 0x1)? If yes, we fill in the
+ * kernel parameters here as well. Note: For old kernels (up to 3.2)
+ * we can not rely on the ELF entry point - it was 0x800 (the SALIPL
+ * loader) and it won't work. For this case we force it to 0x1, 
too.
  */
-ipl->start_addr = KERN_IMAGE_START;
+if (pentry == KERN_IMAGE_START || pentry == 0x800) {
+ipl->start_addr = KERN_IMAGE_START;
+/* Overwrite parameters in the kernel image, which are "rom" */
+strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+} else {
+ipl->start_addr = pentry;
+}
 }
 if (ipl->initrd) {
 ram_addr_t initrd_offset;
-- 
1.8.4.2

[Qemu-devel] [PATCH 4/5] s390x/kvm: Rework priv instruction handlers

[Christian Borntraeger]

From: Frank Blaschka 

The current implementation uses the second byte of the instruction
to identify the instruction handler. This is not sufficient to
support instructions not starting with 0xb2. This patch
adds separate handlers for 0xb2, 0xb9 and 0xeb to be able to
support the full instruction set.

Signed-off-by: Frank Blaschka 
Reviewed-by: Cornelia Huck 
Reviewed-by: Christian Borntraeger 
Signed-off-by: Christian Borntraeger 
---
 target-s390x/kvm.c | 139 ++---
 1 file changed, 80 insertions(+), 59 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 20c711f..11feda9 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -53,25 +53,28 @@
 #define IPA0_B9 0xb900
 #define IPA0_EB 0xeb00
 
-#define PRIV_SCLP_CALL  0x20
-#define PRIV_CSCH   0x30
-#define PRIV_HSCH   0x31
-#define PRIV_MSCH   0x32
-#define PRIV_SSCH   0x33
-#define PRIV_STSCH  0x34
-#define PRIV_TSCH   0x35
-#define PRIV_TPI0x36
-#define PRIV_SAL0x37
-#define PRIV_RSCH   0x38
-#define PRIV_STCRW  0x39
-#define PRIV_STCPS  0x3a
-#define PRIV_RCHP   0x3b
-#define PRIV_SCHM   0x3c
-#define PRIV_CHSC   0x5f
-#define PRIV_SIGA   0x74
-#define PRIV_XSCH   0x76
-#define PRIV_SQBS   0x8a
-#define PRIV_EQBS   0x9c
+#define PRIV_B2_SCLP_CALL   0x20
+#define PRIV_B2_CSCH0x30
+#define PRIV_B2_HSCH0x31
+#define PRIV_B2_MSCH0x32
+#define PRIV_B2_SSCH0x33
+#define PRIV_B2_STSCH   0x34
+#define PRIV_B2_TSCH0x35
+#define PRIV_B2_TPI 0x36
+#define PRIV_B2_SAL 0x37
+#define PRIV_B2_RSCH0x38
+#define PRIV_B2_STCRW   0x39
+#define PRIV_B2_STCPS   0x3a
+#define PRIV_B2_RCHP0x3b
+#define PRIV_B2_SCHM0x3c
+#define PRIV_B2_CHSC0x5f
+#define PRIV_B2_SIGA0x74
+#define PRIV_B2_XSCH0x76
+
+#define PRIV_EB_SQBS0x8a
+
+#define PRIV_B9_EQBS0x9c
+
 #define DIAG_IPL0x308
 #define DIAG_KVM_HYPERCALL  0x500
 #define DIAG_KVM_BREAKPOINT 0x501
@@ -458,96 +461,110 @@ static int kvm_sclp_service_call(S390CPU *cpu, struct 
kvm_run *run,
 return 0;
 }
 
-static int kvm_handle_css_inst(S390CPU *cpu, struct kvm_run *run,
-   uint8_t ipa0, uint8_t ipa1, uint8_t ipb)
+static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
 CPUS390XState *env = &cpu->env;
-
-if (ipa0 != 0xb2) {
-/* Not handled for now. */
-return -1;
-}
+int rc = 0;
+uint16_t ipbh0 = (run->s390_sieic.ipb & 0x) >> 16;
 
 cpu_synchronize_state(CPU(cpu));
 
 switch (ipa1) {
-case PRIV_XSCH:
+case PRIV_B2_XSCH:
 ioinst_handle_xsch(cpu, env->regs[1]);
 break;
-case PRIV_CSCH:
+case PRIV_B2_CSCH:
 ioinst_handle_csch(cpu, env->regs[1]);
 break;
-case PRIV_HSCH:
+case PRIV_B2_HSCH:
 ioinst_handle_hsch(cpu, env->regs[1]);
 break;
-case PRIV_MSCH:
+case PRIV_B2_MSCH:
 ioinst_handle_msch(cpu, env->regs[1], run->s390_sieic.ipb);
 break;
-case PRIV_SSCH:
+case PRIV_B2_SSCH:
 ioinst_handle_ssch(cpu, env->regs[1], run->s390_sieic.ipb);
 break;
-case PRIV_STCRW:
+case PRIV_B2_STCRW:
 ioinst_handle_stcrw(cpu, run->s390_sieic.ipb);
 break;
-case PRIV_STSCH:
+case PRIV_B2_STSCH:
 ioinst_handle_stsch(cpu, env->regs[1], run->s390_sieic.ipb);
 break;
-case PRIV_TSCH:
+case PRIV_B2_TSCH:
 /* We should only get tsch via KVM_EXIT_S390_TSCH. */
 fprintf(stderr, "Spurious tsch intercept\n");
 break;
-case PRIV_CHSC:
+case PRIV_B2_CHSC:
 ioinst_handle_chsc(cpu, run->s390_sieic.ipb);
 break;
-case PRIV_TPI:
+case PRIV_B2_TPI:
 /* This should have been handled by kvm already. */
 fprintf(stderr, "Spurious tpi intercept\n");
 break;
-case PRIV_SCHM:
+case PRIV_B2_SCHM:
 ioinst_handle_schm(cpu, env->regs[1], env->regs[2],
run->s390_sieic.ipb);
 break;
-case PRIV_RSCH:
+case PRIV_B2_RSCH:
 ioinst_handle_rsch(cpu, env->regs[1]);
 break;
-case PRIV_RCHP:
+case PRIV_B2_RCHP:
 ioinst_handle_rchp(cpu, env->regs[1]);

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/28] target-ppc: Altivec 2.07

[Nikunj A Dadhania]

Hi Peter,

Peter Maydell  writes:
>>
>> That'd be awesome! FWIW "upstream" SLOF is at the following git URL:
>>
>>   git://github.com/aik/SLOF.git
>
> That would result in the following updates to our mirror:
>
>  - [deleted] benh
>  - [deleted] debug_mem_alloc

This is fine, there was some cleanup done to the repo.

>e2e8ac9..10306b5  master -> master

This is the current tree

>  + 09a3976...cc418be refs/pull/1/merge -> refs/pull/1/merge (forced update)

I do not understand the above, these commits are not there.

>  * [new tag] qemu-slof-20131118 -> qemu-slof-20131118
>  * [new tag] qemu-slof-20131122 -> qemu-slof-20131122
>  * [new tag] qemu-slof-20131126 -> qemu-slof-20131126
>  * [new tag] qemu-slof-20131209 -> qemu-slof-20131209
>  * [new tag] qemu-slof-20140121 -> qemu-slof-20140121
>  * [new tag] qemu-slof-20140204 -> qemu-slof-20140204
>
> Since in particular that includes deleting some references
> and a forced update on another I thought I would check it
> first.

Regards,
Nikunj

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/28] target-ppc: Altivec 2.07

[Nikunj A Dadhania]

Nikunj A Dadhania  writes:

> Hi Peter,
>
> Peter Maydell  writes:
>>>
>>> That'd be awesome! FWIW "upstream" SLOF is at the following git URL:
>>>
>>>   git://github.com/aik/SLOF.git
>>
>> That would result in the following updates to our mirror:
>>
>>  - [deleted] benh
>>  - [deleted] debug_mem_alloc
>
> This is fine, there was some cleanup done to the repo.
>
>>e2e8ac9..10306b5  master -> master
>
> This is the current tree
>
>>  + 09a3976...cc418be refs/pull/1/merge -> refs/pull/1/merge (forced update)
>
> I do not understand the above, these commits are not there.

Neither at git://github.com/aik/SLOF.git nor at
git://git.qemu.org/SLOF.git

Regards,
Nikunj

Re: [Qemu-devel] [PATCH] translate: remove file translate-all.h

[Xuebing wang]

Hi Peter/Paolo,

Thanks for replying.

1) I am wondering if some slight refactoring (in the sense of 
superficial change of moving things around) can make 
tcg/translationblock/cpu/exec/memory/softmmu more understandable, 
especially for new engineers.


My goal is to explore the possibility of implementing tcg 
multi-threading, in a similar fashion of kvm multi-threading. It's still 
at early stage of a work-in-progress.


As I am new to qemu code base, I spent some time trying to figure out 
the design. Examples are:

-- What does exec (as in the filename of exec.c) really mean?
-- What does "all" mean as in kvm-all,c (xen-all.c, translate-all.c)? I 
assume "all" means "target independent". But, aren't other files (e.g. 
block.c, gdbstub.c) target independent?
-- What is the difference between "all" and "common" as in 
include/exec/cpu-all.h and cpu-common.h?


In my local repository, I did some refactoring. Examples are:
-- I renamed cpu-exec.c => tcg.c, to reflect the design that it's about 
one of the 3 accelerators (tcg, kvm, xen).
-- I renamed cpus.c => vcpu.c (this can be tricky). My idea of the 
object hierarchy is:

[   qom/Object   ]
[   qdev/DeviceState ]
[   qom/CPUState   ]
[   vcpu API (or qom/vcpu)  ]   [  CPUArchState  ]
(in the other words, I am trying to abstract/model vcpu)

-- I renamed include/sysemu/cpus.h => include/exec/vcpu.h
-- And other changes. Examples:
1) cpu_exec_init_all() in exec.c is about memory, and its name is kind 
of confusing
2) include/exec/address-spaces.h says below, but "git grep -nw 
address-spaces.h" reveals a lot.

/*
 * Internal interfaces between memory.c/exec.c/vl.c.  Do not #include 
unless

 * you're one of them.
 */

I am wondering if we should slightly re-factor the organization of 
files, thus new engineers will be easier to understand the design about 
tcg/exec.



2) Theoretically, include/exec/exec-all.h should work when included from 
*ANY files*, even mistakenly, right?


I hope this patch does NOT do any harm.

In my local repository, I split TranslationBlock related from it to be 
another file include/translate.h.


I will see what is the best way to "git send-email" these refactoring 
for the community to review, without causing too much confusion. The 
purpose is to make the design easier to understand for new engineers.



Thanks again.


On 02/26/2014 06:34 PM, Paolo Bonzini wrote:

Il 26/02/2014 10:55, Peter Maydell ha scritto:

On 26 February 2014 09:25, Xuebing Wang  wrote:

This patch does below:
-   Move the declaration of 2 translate functions from 
translate-all.h into

include/exec/exec-all.h
-   remove file translate-all.h

"translate-all.h" => "exec/exec-all.h" can be done by:
git grep -w "translate-all.h" | cut -d: -f1 |
xargs sed -i 's/\/exec\/exec-all.h/g'

Note:
1)  "exact whole word match" is considered.
2)  We may move translate related from include/exec/exec-all.h into
include/exec/translate.h later.


Is there any particular benefit to this change? The function
prototypes go from being in a header only included by the file
that uses them to being in a header that's included by a lot
of other code...


Indeed, that's the point of translate-all.h.

Paolo




--
Thanks,
Xuebing Wang

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 16:59:44 +0800
Hu Tao  wrote:

> On Wed, Feb 26, 2014 at 09:47:08AM +0100, Igor Mammedov wrote:
> > On Wed, 26 Feb 2014 13:00:03 +0800
> > Hu Tao  wrote:
> > 
> > > On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
> > > > Il 25/02/2014 11:20, Hu Tao ha scritto:
> > > > >There is a problem that user_creatable_complete() is called before
> > > > >adding object to "/objects" (see object_create()), which triggers a
> > > > >assert failure when calling object_get_canonical_path() in
> > > > >ram_backend_memory_init(). Any ideas?
> > > > 
> > > > You can call object_property_add_child before calling
> > > > user_creatable_complete, and then call object_unparent (in addition
> > > > to object_unref) if creation fails.
> > > > 
> > > > Paolo
> > > 
> > > Something like this?
> > > 
> > > From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
> > > From: Hu Tao 
> > > Date: Wed, 26 Feb 2014 12:54:34 +0800
> > > Subject: [PATCH] call user_creatable_complete() after adding object to
> > >  "/objects"
> > > 
> > > This makes it possible to get the path of object by calling
> > > object_get_canonical_path() in the complete callback if
> > > someone wants it.
> > > 
> > > Signed-off-by: Hu Tao 
> > > ---
> > >  vl.c | 7 ---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/vl.c b/vl.c
> > > index 1d27b34..30b4297 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void 
> > > *opaque)
> > >  goto out;
> > >  }
> > >  
> > > +object_property_add_child(container_get(object_get_root(), 
> > > "/objects"),
> > > +  id, obj, &local_err);
> > > +
> > >  user_creatable_complete(obj, &local_err);
> > >  if (local_err) {
> > > +object_unparent(obj);
> > >  goto out;
> > >  }
> > >  
> > > -object_property_add_child(container_get(object_get_root(), 
> > > "/objects"),
> > > -  id, obj, &local_err);
> > > -
> > >  out:
> > >  object_unref(obj);
> > >  if (local_err) {
> > Failure case is not handled properly,
> > I'm sorry that I've forgotten to mention prerequisite path
> > https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
> > in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8
> 
> No problem. I should've noticed the patch when cherry-picking. Will you
> post it seperately?
> 
> 

sure

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 11:33:30 +0100
Paolo Bonzini  wrote:

> Il 26/02/2014 10:10, Igor Mammedov ha scritto:
> > if we assume that NUMA policies apply to every hostmem derived backend,
> > then we could realize() approach used by DEVICE. i.e.
> > set NUMA policies in hostmem.c:hostmemory_backend_memory_init()
> >
> > Add parent_complete field to ram-backend class and store there parent's
> > complete pointer. Then we can do:
> >
> > ram_backend_memory_init(UserCreatable *uc, Error **errp) {
> > memory_region_init_ram();
> > ...
> > MEMORY_BACKEND_RAM_CLASS(uc)->parent_complete(uc, errp);
> > ...
> > }
> >
> 
> The problem is that some backends might not be handled the same way. 
> For example, not all backends might produce a single void*/size_t pair 
> for the entire region.  Think of a "composite" backend that produces a 
> large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
Is there a need in composite one or something similar?

> 
> Paolo

[Qemu-devel] #qemu-gsoc IRC channel for Google Summer of Code

[Stefan Hajnoczi]

Students and Mentors,
You are invited to join QEMU's Google Summer of Code IRC channel:
#qemu-gsoc on irc.oftc.net

Students can interact with their mentors on #qemu-gsoc and discuss
project ideas.  If a mentor is unavailable on IRC, please send them an
email instead:
http://qemu-project.org/Google_Summer_of_Code_2014

Stefan

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 13:31, Igor Mammedov ha scritto:

> The problem is that some backends might not be handled the same way.
> For example, not all backends might produce a single void*/size_t pair
> for the entire region.  Think of a "composite" backend that produces a
> large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM memory 
region.  A composite backend could create a container memory region 
whose children are other HostMemoryBackend objects.



Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs and 
partially by regular RAM.  Not sure why.


Paolo

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Marcelo Tosatti]

On Wed, Feb 26, 2014 at 01:45:38PM +0100, Paolo Bonzini wrote:
> Il 26/02/2014 13:31, Igor Mammedov ha scritto:
> >>> The problem is that some backends might not be handled the same way.
> >>> For example, not all backends might produce a single void*/size_t pair
> >>> for the entire region.  Think of a "composite" backend that produces a
> >>> large memory region from two smaller ones.
> >I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
> 
> I agree.  However not all backends may have a mapping to a RAM
> memory region.  A composite backend could create a container memory
> region whose children are other HostMemoryBackend objects.
> 
> >Is there a need in composite one or something similar?
> 
> I've heard of users that want a node backed partially by hugetlbfs
> and partially by regular RAM.  Not sure why.
> 
> Paolo

To overcommit the non hugetlbfs backed guest RAM (think guest pagecache on that 
non
hugetlbfs backed memory, swappable and KSM-able).

The problem is, you have to in someway guarantee the guest allocates 
1GB pages out of the hugetlb backed GPA ranges. Some thoughts
(honestly, dislike all of them):

1) Boot guest with hugepages, allocate hugepages in guest,
later on hotplug 4K backed ranges. HV-unaware reboot might fail,
though.

2) Communicate hugepage GPAs to guest.

3) Create holes in non hugepage backed GPA range.

[Qemu-devel] How is address of helper function for slow path calculated ?

[Gaurav Sharma]

Hi,
I have been trying to trace the for how address translation is done for any
load/store instructions. I was trying to emulate arm on an x86-64 machine.
However, i need some clarifications :
1. During the slow path, qemu uses helper functions to translate address.
2. This is done by calling the function itself during the execution.
3. The host instrn for the slow path is added at the end of the TB block. I
tried a sample code and got the following host instrn :
0x2aaade72d120:  mov%r14,%rdi
0x2aaade72d123:  xor%edx,%edx
0x2aaade72d125:  lea-0x42(%rip),%rcx# 0x2aaade72d0ea
0x2aaade72d12c:  mov$0x2afd98602c10,%r10
0x2aaade72d136:  callq  *%r10 // Call helper function
0x2aaade72d139:  mov%eax,%ebp
0x2aaade72d13b:  jmpq   0x2aaade72d0ea

3. How does it gets the address of the helper function :
call instruction is added by ' tcg_out_calli(s,
(uintptr_t)qemu_ld_helpers[opc & ~MO_SIGN]' line of code which fetches the
address of the helper function.
However from the assembly generated, the address is calculated before :
tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[3],
 (uintptr_t)l->raddr)

How is the address for the helper function calculated ?

Thanks,
Gaurav

Re: [Qemu-devel] How is address of helper function for slow path calculated ?

[Peter Maydell]

On 26 February 2014 13:04, Gaurav Sharma  wrote:
> Hi,
> I have been trying to trace the for how address translation is done for any
> load/store instructions. I was trying to emulate arm on an x86-64 machine.
> However, i need some clarifications :
> 1. During the slow path, qemu uses helper functions to translate address.
> 2. This is done by calling the function itself during the execution.
> 3. The host instrn for the slow path is added at the end of the TB block. I
> tried a sample code and got the following host instrn :
> 0x2aaade72d120:  mov%r14,%rdi
> 0x2aaade72d123:  xor%edx,%edx
> 0x2aaade72d125:  lea-0x42(%rip),%rcx# 0x2aaade72d0ea
> 0x2aaade72d12c:  mov$0x2afd98602c10,%r10
> 0x2aaade72d136:  callq  *%r10 // Call helper function
> 0x2aaade72d139:  mov%eax,%ebp
> 0x2aaade72d13b:  jmpq   0x2aaade72d0ea
>
> 3. How does it gets the address of the helper function :
> call instruction is added by ' tcg_out_calli(s,
> (uintptr_t)qemu_ld_helpers[opc & ~MO_SIGN]' line of code which fetches the
> address of the helper function.
> However from the assembly generated, the address is calculated before :
> tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[3],
>  (uintptr_t)l->raddr)

This is just loading the 4th argument for the helper function into ECX
(which is the return address in generated code which corresponds to
the load we're going to do). It's not related to the address of the
helper function at all.

> How is the address for the helper function calculated ?

You've just quoted the code that does it:
tcg_out_calli(s, (uintptr_t)qemu_ld_helpers[opc & ~MO_SIGN]' ...)

tcg_out_calli spots that the displacement is too big for a call insn
and emits the
  0x2aaade72d12c:  mov$0x2afd98602c10,%r10
  0x2aaade72d136:  callq  *%r10 // Call helper function

thanks
-- PMM

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 13:58, Marcelo Tosatti ha scritto:

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM
memory region.  A composite backend could create a container memory
region whose children are other HostMemoryBackend objects.


Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs
and partially by regular RAM.  Not sure why.


To overcommit the non hugetlbfs backed guest RAM (think guest pagecache on that 
non
hugetlbfs backed memory, swappable and KSM-able).

The problem is, you have to in someway guarantee the guest allocates
1GB pages out of the hugetlb backed GPA ranges. Some thoughts
(honestly, dislike all of them):

1) Boot guest with hugepages, allocate hugepages in guest,
later on hotplug 4K backed ranges. HV-unaware reboot might fail,
though.

2) Communicate hugepage GPAs to guest.

3) Create holes in non hugepage backed GPA range.


I guess (2) is the only one I "like", and I like it just because it 
officially becomes Not Our Problem.


Paolo

Re: [Qemu-devel] [PATCH 2/2] vl: convert -m to QemuOpts

[Paolo Bonzini]

Il 10/02/2014 18:38, Laszlo Ersek ha scritto:

+"mem: initial amount of guest memory (default: "
> +stringify(DEFAULT_RAM_SIZE) "Mb)\n",

I wonder if it should rather say "MB" -- small "b" has this "bits"
connotation for me. But I could be wrong.


Thanks, will fix this.


Also, again, I believe explaining the default used to mean something
else, but I'm OK with that part as-is.


What do you mean exactly?  I cannot parse this.

Paolo

Re: [Qemu-devel] [PATCH 2/2] vl: convert -m to QemuOpts

[Laszlo Ersek]

On 02/26/14 14:29, Paolo Bonzini wrote:
> Il 10/02/2014 18:38, Laszlo Ersek ha scritto:
>>> +"mem: initial amount of guest memory (default: "
>>> > +stringify(DEFAULT_RAM_SIZE) "Mb)\n",
>> I wonder if it should rather say "MB" -- small "b" has this "bits"
>> connotation for me. But I could be wrong.
> 
> Thanks, will fix this.
> 
>> Also, again, I believe explaining the default used to mean something
>> else, but I'm OK with that part as-is.
> 
> What do you mean exactly?  I cannot parse this.

Sorry, I was ambiguous.

When you have a QemuOpt that takes an argument, then the documentation
of the default behavior usually explains the case when the QemuOpt is
present, and the argument is absent. The documentation of the default
behavior usually doesn't concern the case when the QemuOpt*s* itself is
absent. Cf.

(1) -foo bar
(2) -foo bar=baz
(3) [nothing]

The "default" in the docs tends to explain case (1), not case (3).

In this case we have: -m [mem=]megs

(1) -m mem -- makes no sense
(2) -m mem=megs -- works, and well documented
(3) [nothing] -- is what the proposed docs describe as "default", but it
doesn't match "historical practice".

(1) in general can make sense, eg. for booleans.

Anyway I don't feel strongly about this in the least -- I just thought
I'd point it out. Feel free to ignore it; I have no good suggestion as
to how to make it consistent across all options.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 13:45:38 +0100
Paolo Bonzini  wrote:

> Il 26/02/2014 13:31, Igor Mammedov ha scritto:
> >> > The problem is that some backends might not be handled the same way.
> >> > For example, not all backends might produce a single void*/size_t pair
> >> > for the entire region.  Think of a "composite" backend that produces a
> >> > large memory region from two smaller ones.
> > I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
> 
> I agree.  However not all backends may have a mapping to a RAM memory 
> region.  A composite backend could create a container memory region 
> whose children are other HostMemoryBackend objects.
> 
> > Is there a need in composite one or something similar?
> 
> I've heard of users that want a node backed partially by hugetlbfs and 
> partially by regular RAM.  Not sure why.
Isn't issue here in how backend is mapped into GPA? Well that is not
backend's job.

Once one starts to put layout (alignment, noncontinuously mapped
memory regions inside of container, ...), mapping HPA->GPA gets complicated.

It would be better to use simple building blocks and model as:
2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.


> 
> Paolo
> 

Re: [Qemu-devel] [PATCH 2/2] vl: convert -m to QemuOpts

[Paolo Bonzini]

Il 26/02/2014 14:42, Laszlo Ersek ha scritto:

When you have a QemuOpt that takes an argument, then the documentation
of the default behavior usually explains the case when the QemuOpt is
present, and the argument is absent. The documentation of the default
behavior usually doesn't concern the case when the QemuOpt*s* itself is
absent. Cf.

(1) -foo bar
(2) -foo bar=baz
(3) [nothing]

The "default" in the docs tends to explain case (1), not case (3).

In this case we have: -m [mem=]megs

(1) -m mem -- makes no sense
(2) -m mem=megs -- works, and well documented
(3) [nothing] -- is what the proposed docs describe as "default", but it
doesn't match "historical practice".

(1) in general can make sense, eg. for booleans.

Anyway I don't feel strongly about this in the least -- I just thought
I'd point it out.


Yeah, you're right.


Feel free to ignore it; I have no good suggestion as
to how to make it consistent across all options.


Neither do I, but at least "megs" is not marked as an optional element, 
so I think it's not that ambiguous.


Paolo

Re: [Qemu-devel] How is address of helper function for slow path calculated ?

[Gaurav Sharma]

Thanks Peter,
So, the following instruction only make up the call stack for the function
call :
0x2aaade72d120:  mov%r14,%rdi
0x2aaade72d123:  xor%edx,%edx
0x2aaade72d125:  lea-0x42(%rip),%rcx# 0x2aaade72d0ea

Thanks,
Gaurav


On Wed, Feb 26, 2014 at 6:44 PM, Peter Maydell wrote:

> On 26 February 2014 13:04, Gaurav Sharma  wrote:
> > Hi,
> > I have been trying to trace the for how address translation is done for
> any
> > load/store instructions. I was trying to emulate arm on an x86-64
> machine.
> > However, i need some clarifications :
> > 1. During the slow path, qemu uses helper functions to translate address.
> > 2. This is done by calling the function itself during the execution.
> > 3. The host instrn for the slow path is added at the end of the TB
> block. I
> > tried a sample code and got the following host instrn :
> > 0x2aaade72d120:  mov%r14,%rdi
> > 0x2aaade72d123:  xor%edx,%edx
> > 0x2aaade72d125:  lea-0x42(%rip),%rcx# 0x2aaade72d0ea
> > 0x2aaade72d12c:  mov$0x2afd98602c10,%r10
> > 0x2aaade72d136:  callq  *%r10 // Call helper function
> > 0x2aaade72d139:  mov%eax,%ebp
> > 0x2aaade72d13b:  jmpq   0x2aaade72d0ea
> >
> > 3. How does it gets the address of the helper function :
> > call instruction is added by ' tcg_out_calli(s,
> > (uintptr_t)qemu_ld_helpers[opc & ~MO_SIGN]' line of code which fetches
> the
> > address of the helper function.
> > However from the assembly generated, the address is calculated before :
> > tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[3],
> >  (uintptr_t)l->raddr)
>
> This is just loading the 4th argument for the helper function into ECX
> (which is the return address in generated code which corresponds to
> the load we're going to do). It's not related to the address of the
> helper function at all.
>
> > How is the address for the helper function calculated ?
>
> You've just quoted the code that does it:
> tcg_out_calli(s, (uintptr_t)qemu_ld_helpers[opc & ~MO_SIGN]' ...)
>
> tcg_out_calli spots that the displacement is too big for a call insn
> and emits the
>   0x2aaade72d12c:  mov$0x2afd98602c10,%r10
>   0x2aaade72d136:  callq  *%r10 // Call helper function
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 14:43, Igor Mammedov ha scritto:

On Wed, 26 Feb 2014 13:45:38 +0100
Paolo Bonzini  wrote:


Il 26/02/2014 13:31, Igor Mammedov ha scritto:

The problem is that some backends might not be handled the same way.
For example, not all backends might produce a single void*/size_t pair
for the entire region.  Think of a "composite" backend that produces a
large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM memory
region.  A composite backend could create a container memory region
whose children are other HostMemoryBackend objects.


Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs and
partially by regular RAM.  Not sure why.

Isn't issue here in how backend is mapped into GPA? Well that is not
backend's job.

Once one starts to put layout (alignment, noncontinuously mapped
memory regions inside of container, ...), mapping HPA->GPA gets complicated.

It would be better to use simple building blocks and model as:
2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.


Right, I had forgotten that you can have cold-plugged DIMM devices. 
That's a nice solution, also because it simplifies passing the GPA 
configuration down to the guest.


How would that translate to sharing HostMemoryBackend code for memory 
policies?  Which of Hu Tao's proposals do you like best?


Paolo

Re: [Qemu-devel] [PATCH 2/2] vl: convert -m to QemuOpts

[Igor Mammedov]

On Wed, 26 Feb 2014 14:29:54 +0100
Paolo Bonzini  wrote:

> Il 10/02/2014 18:38, Laszlo Ersek ha scritto:
> >> +"mem: initial amount of guest memory (default: "
> >> > +stringify(DEFAULT_RAM_SIZE) "Mb)\n",
> > I wonder if it should rather say "MB" -- small "b" has this "bits"
> > connotation for me. But I could be wrong.
> 
> Thanks, will fix this.
> 
> > Also, again, I believe explaining the default used to mean something
> > else, but I'm OK with that part as-is.
> 
> What do you mean exactly?  I cannot parse this.
> 
> Paolo
> 
> 

It should be fixed in v3 that you were going to apply to numa branch
https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02321.html

Re: [Qemu-devel] How is address of helper function for slow path calculated ?

[Peter Maydell]

On 26 February 2014 13:46, Gaurav Sharma  wrote:
> Thanks Peter,
> So, the following instruction only make up the call stack for the function
> call :
>
> 0x2aaade72d120:  mov%r14,%rdi
> 0x2aaade72d123:  xor%edx,%edx
> 0x2aaade72d125:  lea-0x42(%rip),%rcx# 0x2aaade72d0ea

This is nothing to do with the stack -- it's just setting up the arguments
for the function call. You probably want to find a reference to the x86_64
Linux calling convention which might make the code we're generating
make more sense to you.

-- PMM

[Qemu-devel] [Bug 902413] Re: qemu-i386-user on ARM host: wine hangs/spins when trying to run anything

[Juan Melgarejo Ludeña]

with QEMU expected to turn ver 2.0 in april I wonder this bug is still 
forgotten. 
Bugs list on Peter Maydell's post and Dan Kegel's have fixes commited, and I 
see there are alternative patches from http://patchwork.ozlabs.org/patch/45206/ 
and http://repo.or.cz/w/qemu/agraf.git linked from http://wiki.winehq.org/ARM

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

Title:
  qemu-i386-user on ARM host: wine hangs/spins when trying to run
  anything

Status in QEMU:
  New

Bug description:
  With qemu built from git from 217bfb445b54db618a30f3a39170bebd9fd9dbf2
  and configured with './configure --target-list=i386-linux-user
  --static --interp-prefix=/home/pgriffais/natty-i386/', trying to run
  wine 1.3.15 from an Ubuntu 11.04 chroot results in hangs. If I run an
  i386 emulated wineserver, wineserver hangs in:

  0x600c7f8c in read () at ../sysdeps/unix/syscall-template.S:82
  82../sysdeps/unix/syscall-template.S: No such file or directory.
in ../sysdeps/unix/syscall-template.S
  (gdb) bt
  #0  0x600c7f8c in read () at ../sysdeps/unix/syscall-template.S:82
  #1  0x6004a316 in read (cpu_env=0x622c3ee8, num=3, arg1=6, arg2=1121255519, 
  arg3=1, arg4=134875664, arg5=1, arg6=1121255528, arg7=0, arg8=0)
  at /usr/include/bits/unistd.h:45
  #2  do_syscall (cpu_env=0x622c3ee8, num=3, arg1=6, arg2=1121255519, arg3=1, 
  arg4=134875664, arg5=1, arg6=1121255528, arg7=0, arg8=0)
  at /home/ubuntu/src/qemu/linux-user/syscall.c:4691
  #3  0x600262f0 in cpu_loop (env=0x622c3ee8)
  at /home/ubuntu/src/qemu/linux-user/main.c:321
  #4  0x60026bbc in main (argc=, 
  argv=, envp=)
  at /home/ubuntu/src/qemu/linux-user/main.c:3817

  While wine hangs in:

  0x600c84ac in recvmsg () at ../sysdeps/unix/syscall-template.S:82
  82../sysdeps/unix/syscall-template.S: No such file or directory.
in ../sysdeps/unix/syscall-template.S
  (gdb) bt
  #0  0x600c84ac in recvmsg () at ../sysdeps/unix/syscall-template.S:82
  #1  0x60041c4e in do_sendrecvmsg (fd=4, target_msg=, 
  flags=1073741824, send=0)
  at /home/ubuntu/src/qemu/linux-user/syscall.c:1834
  #2  0x600497ec in do_socketcall (cpu_env=, num=102, 
  arg1=17, arg2=1122504544, arg3=2076831732, arg4=1122504568, 
  arg5=2076942688, arg6=1122504888, arg7=0, arg8=0)
  at /home/ubuntu/src/qemu/linux-user/syscall.c:2235
  #3  do_syscall (cpu_env=, num=102, arg1=17, 
  arg2=1122504544, arg3=2076831732, arg4=1122504568, arg5=2076942688, 
  arg6=1122504888, arg7=0, arg8=0)
  at /home/ubuntu/src/qemu/linux-user/syscall.c:6085
  #4  0x600262f0 in cpu_loop (env=0x622c3f08)
  at /home/ubuntu/src/qemu/linux-user/main.c:321
  #5  0x60026bbc in main (argc=, 
  argv=, envp=)
  at /home/ubuntu/src/qemu/linux-user/main.c:3817

  However if I build wineserver 1.3.15 natively for ARM and run it on
  the host while wine is emulated, I get the following:

  root@tiberiusstation:/home/ubuntu# ./natty-i386/usr/bin/wine notepad
  Unsupported ancillary data: 1/2
  Unsupported ancillary data: 1/2
  Unsupported ancillary data: 1/2
  err:process:__wine_kernel_init boot event wait timed out

  I assume the last one is due to wineboot.exe hanging. The main wine
  process hangs in there:

  cg_temp_new_internal_i32 (temp_local=)
  at /home/ubuntu/src/qemu/tcg/tcg.c:483
  483   }
  (gdb) bt
  #0  tcg_temp_new_internal_i32 (temp_local=)
  at /home/ubuntu/src/qemu/tcg/tcg.c:483
  #1  0x60052ac6 in tcg_temp_new_i32 (val=6)
  at /home/ubuntu/src/qemu/tcg/tcg.h:442
  #2  tcg_const_i32 (val=6) at /home/ubuntu/src/qemu/tcg/tcg.c:530
  #3  0x6005ef0c in tcg_gen_shri_i32 (ot=2, op1=2, op2=7, is_right=1, 
  is_arith=0, s=)
  at /home/ubuntu/src/qemu/tcg/tcg-op.h:605
  #4  gen_shift_rm_im (ot=2, op1=2, op2=7, is_right=1, is_arith=0, 
  s=)
  at /home/ubuntu/src/qemu/target-i386/translate.c:1514
  #5  0x6006df90 in gen_shifti (s=0xbefea970, pc_start=)
  at /home/ubuntu/src/qemu/target-i386/translate.c:1946
  #6  disas_insn (s=0xbefea970, pc_start=)
  at /home/ubuntu/src/qemu/target-i386/translate.c:5397
  #7  0x60091758 in gen_intermediate_code_internal (env=0x625656f8, 
  tb=0x402cdf48) at /home/ubuntu/src/qemu/target-i386/translate.c:7825
  #8  gen_intermediate_code_pc (env=0x625656f8, tb=0x402cdf48)
  at /home/ubuntu/src/qemu/target-i386/translate.c:7896
  #9  0x60054bf2 in cpu_restore_state (tb=0x402cdf48, env=0x62565690, 
  searched_pc=1617393812) at /home/ubuntu/src/qemu/translate-all.c:126
  #10 0x60091d9e in handle_cpu_signal (host_signum=, 
  pinfo=, puc=0xbefeab70)
  at /home/ubuntu/src/qemu/user-exec.c:117
  #11 cpu_x86_signal_handler (host_signum=, 
  pinfo=, puc=0xbefeab70)
  at /home/ubuntu/src/qemu/user-exec.c:458
  #12 0x6003c764 in host_signal_handler (host_signum=11, info=0xbefeaaf0, 
  puc=)
  at /home/ubuntu/src/qemu/linux-user/signal.

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 26 Feb 2014 14:47:28 +0100
Paolo Bonzini  wrote:

> Il 26/02/2014 14:43, Igor Mammedov ha scritto:
> > On Wed, 26 Feb 2014 13:45:38 +0100
> > Paolo Bonzini  wrote:
> >
> >> Il 26/02/2014 13:31, Igor Mammedov ha scritto:
> > The problem is that some backends might not be handled the same way.
> > For example, not all backends might produce a single void*/size_t pair
> > for the entire region.  Think of a "composite" backend that produces a
> > large memory region from two smaller ones.
> >>> I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
> >>
> >> I agree.  However not all backends may have a mapping to a RAM memory
> >> region.  A composite backend could create a container memory region
> >> whose children are other HostMemoryBackend objects.
> >>
> >>> Is there a need in composite one or something similar?
> >>
> >> I've heard of users that want a node backed partially by hugetlbfs and
> >> partially by regular RAM.  Not sure why.
> > Isn't issue here in how backend is mapped into GPA? Well that is not
> > backend's job.
> >
> > Once one starts to put layout (alignment, noncontinuously mapped
> > memory regions inside of container, ...), mapping HPA->GPA gets complicated.
> >
> > It would be better to use simple building blocks and model as:
> > 2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.
> 
> Right, I had forgotten that you can have cold-plugged DIMM devices. 
> That's a nice solution, also because it simplifies passing the GPA 
> configuration down to the guest.
> 
> How would that translate to sharing HostMemoryBackend code for memory 
> policies?  Which of Hu Tao's proposals do you like best?
possible choices could be:

 1: 'realize' approach I suggested
  drawback is: assumption that all backends derived from HostMemoryBackend
   will inherit NUMA controls even if backend shouldn't have
   one (for example: fictional remote memory backend)
  plus: derived types from HostMemoryBackend, don't need to know anything
about NUMA.
 2: #3 from Hu Tao's suggestion
  drawback is: every new backend have to explicitly call NUMA callbacks
  somewhat plus is that not NUMA aware backends could ignore NUMA callbacks,
  but they would still have NUMA properties available, which is confusing.

 3: might be over-engineered #1 from above: build proper class hierarchy:
  HostMemoryBackend  -> NumaMemoryBackend -> RamBackend
| -> HugepageBackend
|-> whatever else

> 
> Paolo

[Qemu-devel] [PATCH v2 0/3] nbd: fix issues when connection breaks

[Stefan Hajnoczi]

v2:
 * Use qemu-iotests 083 test number to avoid conflicts [kwolf]
 * Test class negotiation without export name [lupine]
 * Test short replies from the server [lupine]

The first patch ensures the nbd_receive_reply() fd handler is unregistered when
the connection to the server breaks.  This avoids high CPU consumption and
flooding error messages.

The second patch introduces an NBD server fault injection script.  Using this
fake NBD server it is possible to exercise error handling code paths in the NBD
client.

The third patch adds qemu-iotests test case 083 to verify qemu-io exits with an
error at each point where the connection can break.

Stefan Hajnoczi (3):
  nbd: close socket if connection breaks
  tests: add nbd-fault-injector.py utility
  qemu-iotests: add 083 NBD client disconnect tests

 block/nbd-client.c   |  33 ++--
 tests/qemu-iotests/083   | 129 +++
 tests/qemu-iotests/083.out   | 163 +++
 tests/qemu-iotests/group |   1 +
 tests/qemu-iotests/nbd-fault-injector.py | 264 +++
 5 files changed, 575 insertions(+), 15 deletions(-)
 create mode 100755 tests/qemu-iotests/083
 create mode 100644 tests/qemu-iotests/083.out
 create mode 100755 tests/qemu-iotests/nbd-fault-injector.py

-- 
1.8.5.3

[Qemu-devel] [PATCH v2 3/3] qemu-iotests: add 083 NBD client disconnect tests

[Stefan Hajnoczi]

This new test case uses nbd-fault-injector.py to simulate broken TCP
connections at each stage in the NBD protocol.  This way we can exercise
block/nbd-client.c's socket error handling code paths.

In particular, this serves as a regression test to make sure
nbd-client.c doesn't cause an infinite loop by leaving its
nbd_receive_reply() fd handler registered after the connection has been
closed.  This bug was fixed in an earlier patch.

Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/083 | 129 +++
 tests/qemu-iotests/083.out | 163 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 293 insertions(+)
 create mode 100755 tests/qemu-iotests/083
 create mode 100644 tests/qemu-iotests/083.out

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
new file mode 100755
index 000..f764534
--- /dev/null
+++ b/tests/qemu-iotests/083
@@ -0,0 +1,129 @@
+#!/bin/bash
+#
+# Test NBD client unexpected disconnect
+#
+# Copyright Red Hat, Inc. 2014
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=stefa...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto nbd
+_supported_os Linux
+
+# Pick a TCP port based on our pid.  This way multiple instances of this test
+# can run in parallel without conflicting.
+choose_tcp_port() {
+   echo $((($$ % 31744) + 1024)) # 1024 <= port < 32768
+}
+
+wait_for_tcp_port() {
+   while ! (netstat --tcp --listening --numeric | \
+grep "$1.*0.0.0.0:\*.*LISTEN") 2>&1 >/dev/null; do
+   sleep 0.1
+   done
+}
+
+filter_nbd() {
+   # nbd.c error messages contain function names and line numbers that are 
prone
+   # to change.  Message ordering depends on timing between send and 
receive
+   # callbacks sometimes, making them unreliable.
+   #
+   # Filter out the TCP port number since this changes between runs.
+   sed -e 's#^nbd.c:.*##g' \
+   -e 's#nbd:127.0.0.1:[^:]*:#nbd:127.0.0.1:PORT:#g'
+}
+
+check_disconnect() {
+   event=$1
+   when=$2
+   negotiation=$3
+   echo "=== Check disconnect $when $event ==="
+   echo
+
+   port=$(choose_tcp_port)
+
+   cat > "$TEST_DIR/nbd-fault-injector.conf" <&1 >/dev/null &
+   wait_for_tcp_port "127.0.0.1:$port"
+   $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd
+
+   echo
+}
+
+for event in neg1 "export" neg2 request reply data; do
+   for when in before after; do
+   check_disconnect "$event" "$when"
+   done
+
+   # Also inject short replies from the NBD server
+   case "$event" in
+   neg1)
+   for when in 8 16; do
+   check_disconnect "$event" "$when"
+   done
+   ;;
+   "export")
+   for when in 4 12 16; do
+   check_disconnect "$event" "$when"
+   done
+   ;;
+   neg2)
+   for when in 8 10; do
+   check_disconnect "$event" "$when"
+   done
+   ;;
+   reply)
+   for when in 4 8; do
+   check_disconnect "$event" "$when"
+   done
+   ;;
+   esac
+done
+
+# Also check classic negotiation without export information
+for when in before 8 16 24 28 after; do
+   check_disconnect "neg-classic" "$when" --classic-negotiation
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
new file mode 100644
index 000..85ee8d6
--- /dev/null
+++ b/tests/qemu-iotests/083.out
@@ -0,0 +1,163 @@
+QA output created by 083
+=== Check disconnect before neg1 ===
+
+
+qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo: Could not open 
image: Invalid argument
+no file open, try 'help open'
+
+=== Check disconnect after neg1 ===
+
+
+qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo: Could not open 
image: Invalid argument
+no file open, try 'help open'
+
+=== Check disconnect 8 neg1 ===
+
+
+qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=f

[Qemu-devel] [PATCH v2 2/3] tests: add nbd-fault-injector.py utility

[Stefan Hajnoczi]

The nbd-fault-injector.py script is a special kind of NBD server.  It
throws away all writes and produces zeroes for reads.  Given a list of
fault injection rules, it can simulate NBD protocol errors and is useful
for testing NBD client error handling code paths.

See the patch for documentation.  This scripts is modelled after Kevin
Wolf 's blkdebug block driver.

Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/nbd-fault-injector.py | 264 +++
 1 file changed, 264 insertions(+)
 create mode 100755 tests/qemu-iotests/nbd-fault-injector.py

diff --git a/tests/qemu-iotests/nbd-fault-injector.py 
b/tests/qemu-iotests/nbd-fault-injector.py
new file mode 100755
index 000..6c07191
--- /dev/null
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -0,0 +1,264 @@
+#!/usr/bin/env python
+# NBD server - fault injection utility
+#
+# Configuration file syntax:
+#   [inject-error "disconnect-neg1"]
+#   event=neg1
+#   io=readwrite
+#   when=before
+#
+# Note that Python's ConfigParser squashes together all sections with the same
+# name, so give each [inject-error] a unique name.
+#
+# inject-error options:
+#   event - name of the trigger event
+#   "neg1" - first part of negotiation struct
+#   "export" - export struct
+#   "neg2" - second part of negotiation struct
+#   "request" - NBD request struct
+#   "reply" - NBD reply struct
+#   "data" - request/reply data
+#   io- I/O direction that triggers this rule:
+#   "read", "write", or "readwrite"
+#   default: readwrite
+#   when  - after how many bytes to inject the fault
+#   -1 - inject error after I/O
+#   0 - inject error before I/O
+#   integer - inject error after integer bytes
+#   "before" - alias for 0
+#   "after" - alias for -1
+#   default: before
+#
+# Currently the only error injection action is to terminate the server process.
+# This resets the TCP connection and thus forces the client to handle
+# unexpected connection termination.
+#
+# Other error injection actions could be added in the future.
+#
+# Copyright Red Hat, Inc. 2014
+#
+# Authors:
+#   Stefan Hajnoczi 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+
+import sys
+import socket
+import struct
+import collections
+import ConfigParser
+
+FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
+
+# Protocol constants
+NBD_CMD_READ = 0
+NBD_CMD_WRITE = 1
+NBD_CMD_DISC = 2
+NBD_REQUEST_MAGIC = 0x25609513
+NBD_REPLY_MAGIC = 0x67446698
+NBD_PASSWD = 0x4e42444d41474943
+NBD_OPTS_MAGIC = 0x49484156454F5054
+NBD_CLIENT_MAGIC = 0x420281861253
+NBD_OPT_EXPORT_NAME = 1 << 0
+
+# Protocol structs
+neg_classic_struct = struct.Struct('>QQQI124x')
+neg1_struct = struct.Struct('>QQH')
+export_tuple = collections.namedtuple('Export', 'reserved magic opt len')
+export_struct = struct.Struct('>IQII')
+neg2_struct = struct.Struct('>QH124x')
+request_tuple = collections.namedtuple('Request', 'magic type handle from_ 
len')
+request_struct = struct.Struct('>IIQQI')
+reply_struct = struct.Struct('>IIQ')
+
+def err(msg):
+sys.stderr.write(msg + '\n')
+sys.exit(1)
+
+def recvall(sock, bufsize):
+received = 0
+chunks = []
+while received < bufsize:
+chunk = sock.recv(bufsize - received)
+if len(chunk) == 0:
+raise Exception('unexpected disconnect')
+chunks.append(chunk)
+received += len(chunk)
+return ''.join(chunks)
+
+class Rule(object):
+def __init__(self, name, event, io, when):
+self.name = name
+self.event = event
+self.io = io
+self.when = when
+
+def match(self, event, io):
+if event != self.event:
+return False
+if io != self.io and self.io != 'readwrite':
+return False
+return True
+
+class FaultInjectionSocket(object):
+def __init__(self, sock, rules):
+self.sock = sock
+self.rules = rules
+
+def check(self, event, io, bufsize=None):
+for rule in self.rules:
+if rule.match(event, io):
+if rule.when == 0 or bufsize is None:
+print 'Closing connection on rule match %s' % rule.name
+sys.exit(0)
+if rule.when != -1:
+return rule.when
+return bufsize
+
+def send(self, buf, event):
+bufsize = self.check(event, 'write', bufsize=len(buf))
+self.sock.sendall(buf[:bufsize])
+self.check(event, 'write')
+
+def recv(self, bufsize, event):
+bufsize = self.check(event, 'read', bufsize=bufsize)
+data = recvall(self.sock, bufsize)
+self.check(event, 'read')
+return data
+
+def close(self):
+self.sock.close()
+
+def negotiate_classic(conn):
+buf = neg_classic_struct.pack(NBD_PASSWD, NBD_CLIENT_MAGIC,
+  

[Qemu-devel] [PATCH] MAINTAINERS: drop an out of date address

[Michael S. Tsirkin]

Gleb's address seems to be out of date.  Since it stayed like that for a
while now, I'm guessing he's no longer interested in getting mail.

Signed-off-by: Michael S. Tsirkin 
---
 MAINTAINERS | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index adc5973..1f2d277 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -158,7 +158,6 @@ Guest CPU Cores (KVM):
 --
 
 Overall
-M: Gleb Natapov 
 M: Paolo Bonzini 
 L: k...@vger.kernel.org
 S: Supported
@@ -181,7 +180,6 @@ S: Maintained
 F: target-s390x/kvm.c
 
 X86
-M: Gleb Natapov 
 M: Marcelo Tosatti 
 L: k...@vger.kernel.org
 S: Supported
-- 
MST

Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Paolo Bonzini]

Il 26/02/2014 15:25, Igor Mammedov ha scritto:

 1: 'realize' approach I suggested
  drawback is: assumption that all backends derived from HostMemoryBackend
   will inherit NUMA controls even if backend shouldn't have
   one (for example: fictional remote memory backend)
  plus: derived types from HostMemoryBackend, don't need to know anything
about NUMA.


Let's go with this for now.  It keeps things simple.

Paolo

Re: [Qemu-devel] [PATCH v2 0/5] s390: Support for Hotplug of Standby Memory

[Christian Borntraeger]

On 24/02/14 22:30, Matthew Rosato wrote:
> This patchset adds support in s390 for a pool of standby memory,
> which can be set online/offline by the guest (ie, via chmem).
> New options, maxmem and slots, are added to the QEMU command line
> memory parameter to specify the total amount of memory available 
> to the guest as well as the number of memory slots available.
> As part of this work, additional results are provided for the 
> Read SCP Information SCLP, and new implentation is added for the 
> Read Storage Element Information, Attach Storage Element, 
> Assign Storage and Unassign Storage SCLPs, which enables the s390 
> guest to manipulate the standby memory pool.

Looks like Paolo will apply Igors latest patches to the numa branch,
I will defer patches 3-5 until Igors patches hit qemu master.

Christian

> 
> This patchset is based on work originally done by Jeng-Fang (Nick)
> Wang.
> 
> This patchset has been built to apply on the s390-next tree at:
> 
> git://github.com/borntraeger/qemu.git s390-next
> 
> Changes for v2:
>  * Removed the patch that introduced the standby-mem operand and 
>instead included Igor Mammedov's patches that add the mem-opts 
>'maxmem' and 'slots', with a slight modification due to the removal 
>of qemu_opts_create_nofail.
>  * Patch 3 was inserted to add a new qom object that encapsulate variables 
>used by s390 memory hotplug.  Patches 4 and 5 adjusted to use this 
>object.
>  * Added additional code comments and other minor changes per Alexander 
>Graf's comments
> 
> Igor Mammedov (2):
>   vl: convert -m to QemuOpts
>   vl.c: extend -m option to support options for memory hotplug
> 
> Matthew Rosato (3):
>   sclp-s390: Add device to manage s390 memory hotplug
>   virtio-ccw: Include standby memory when calculating storage increment
>   sclp-s390: Add memory hotplug SCLPs
> 
>  hw/s390x/s390-virtio-ccw.c |   42 +--
>  hw/s390x/sclp.c|  276 
> ++--
>  include/hw/s390x/sclp.h|   19 +++
>  qemu-options.hx|   10 +-
>  target-s390x/cpu.h |   18 +++
>  target-s390x/kvm.c |5 +
>  vl.c   |   98 ++--
>  7 files changed, 440 insertions(+), 28 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/5] s390: Support for Hotplug of Standby Memory

[Paolo Bonzini]

Il 26/02/2014 15:42, Christian Borntraeger ha scritto:

On 24/02/14 22:30, Matthew Rosato wrote:

This patchset adds support in s390 for a pool of standby memory,
which can be set online/offline by the guest (ie, via chmem).
New options, maxmem and slots, are added to the QEMU command line
memory parameter to specify the total amount of memory available
to the guest as well as the number of memory slots available.
As part of this work, additional results are provided for the
Read SCP Information SCLP, and new implentation is added for the
Read Storage Element Information, Attach Storage Element,
Assign Storage and Unassign Storage SCLPs, which enables the s390
guest to manipulate the standby memory pool.


Looks like Paolo will apply Igors latest patches to the numa branch,
I will defer patches 3-5 until Igors patches hit qemu master.


What are the dependencies of these series?  I can include it in my queue if:

(a) you give me your Acked-by

(b) Igor reviews it and tells me if this is the right version of his 
patch "extend -m option to support options for memory hotplug"


Paolo


Christian



This patchset is based on work originally done by Jeng-Fang (Nick)
Wang.

This patchset has been built to apply on the s390-next tree at:

git://github.com/borntraeger/qemu.git s390-next

Changes for v2:
 * Removed the patch that introduced the standby-mem operand and
   instead included Igor Mammedov's patches that add the mem-opts
   'maxmem' and 'slots', with a slight modification due to the removal
   of qemu_opts_create_nofail.
 * Patch 3 was inserted to add a new qom object that encapsulate variables
   used by s390 memory hotplug.  Patches 4 and 5 adjusted to use this
   object.
 * Added additional code comments and other minor changes per Alexander
   Graf's comments

Igor Mammedov (2):
  vl: convert -m to QemuOpts
  vl.c: extend -m option to support options for memory hotplug

Matthew Rosato (3):
  sclp-s390: Add device to manage s390 memory hotplug
  virtio-ccw: Include standby memory when calculating storage increment
  sclp-s390: Add memory hotplug SCLPs

 hw/s390x/s390-virtio-ccw.c |   42 +--
 hw/s390x/sclp.c|  276 ++--
 include/hw/s390x/sclp.h|   19 +++
 qemu-options.hx|   10 +-
 target-s390x/cpu.h |   18 +++
 target-s390x/kvm.c |5 +
 vl.c   |   98 ++--
 7 files changed, 440 insertions(+), 28 deletions(-)

[Qemu-devel] [PATCH v2 5/6] NUMA: convert -numa option to use OptsVisitor

[Igor Mammedov]

From: Wanlong Gao 

Signed-off-by: Wanlong Gao 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Igor Mammedov 
---
v2:
  - altered doc comment according to Eric Blake 
suggestions.

Signed-off-by: Igor Mammedov 
---
 include/sysemu/sysemu.h |3 +-
 numa.c  |  147 +++
 qapi-schema.json|   32 ++
 vl.c|   11 +++-
 4 files changed, 115 insertions(+), 78 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d873b42..20b05a3 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -140,9 +140,10 @@ typedef struct node_info {
 DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
 } NodeInfo;
 extern NodeInfo numa_info[MAX_NODES];
-void numa_add(const char *optarg);
 void set_numa_nodes(void);
 void set_numa_modes(void);
+extern QemuOptsList qemu_numa_opts;
+int numa_init_func(QemuOpts *opts, void *opaque);
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {
diff --git a/numa.c b/numa.c
index c3eca78..40c28b3 100644
--- a/numa.c
+++ b/numa.c
@@ -24,101 +24,96 @@
  */
 
 #include "sysemu/sysemu.h"
-
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+QemuOptsList qemu_numa_opts = {
+.name = "numa",
+.implied_opt_name = "type",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
+.desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+static int numa_node_parse(NumaNodeOptions *node, QemuOpts *opts)
 {
-char *endptr;
-unsigned long long value, endvalue;
-
-/* Empty CPU range strings will be considered valid, they will simply
- * not set any bit in the CPU bitmap.
- */
-if (!*cpus) {
-return;
-}
+uint16_t nodenr;
+uint16List *cpus = NULL;
 
-if (parse_uint(cpus, &value, &endptr, 10) < 0) {
-goto error;
-}
-if (*endptr == '-') {
-if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
-goto error;
-}
-} else if (*endptr == '\0') {
-endvalue = value;
+if (node->has_nodeid) {
+nodenr = node->nodeid;
 } else {
-goto error;
+nodenr = nb_numa_nodes;
 }
 
-if (endvalue >= MAX_CPUMASK_BITS) {
-endvalue = MAX_CPUMASK_BITS - 1;
-fprintf(stderr,
-"qemu: NUMA: A max of %d VCPUs are supported\n",
- MAX_CPUMASK_BITS);
+if (nodenr >= MAX_NODES) {
+fprintf(stderr, "qemu: Max number of NUMA nodes reached: %"
+PRIu16 "\n", nodenr);
+return -1;
 }
 
-if (endvalue < value) {
-goto error;
+for (cpus = node->cpus; cpus; cpus = cpus->next) {
+if (cpus->value > MAX_CPUMASK_BITS) {
+fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d",
+cpus->value, MAX_CPUMASK_BITS);
+continue;
+}
+bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
 }
 
-bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
-return;
+if (node->has_mem) {
+uint64_t mem_size = node->mem;
+const char *mem_str = qemu_opt_get(opts, "mem");
+/* Fix up legacy suffix-less format */
+if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
+mem_size <<= 20;
+}
+numa_info[nodenr].node_mem = mem_size;
+}
 
-error:
-fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
-exit(1);
+return 0;
 }
 
-void numa_add(const char *optarg)
+int numa_init_func(QemuOpts *opts, void *opaque)
 {
-char option[128];
-char *endptr;
-unsigned long long nodenr;
-
-optarg = get_opt_name(option, 128, optarg, ',');
-if (*optarg == ',') {
-optarg++;
+NumaOptions *object = NULL;
+Error *err = NULL;
+int ret = 0;
+
+{
+OptsVisitor *ov = opts_visitor_new(opts);
+visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
+opts_visitor_cleanup(ov);
 }
-if (!strcmp(option, "node")) {
-
-if (nb_numa_nodes >= MAX_NODES) {
-fprintf(stderr, "qemu: too many NUMA nodes\n");
-exit(1);
-}
 
-if (get_param_value(option, 128, "nodeid", optarg) == 0) {
-nodenr = nb_numa_nodes;
-} else {
-if (parse_uint_full(option, &nodenr, 10) < 0) {
-fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
-exit(1);
-}
-}
-
-if (nodenr >= MAX_NODES) {
-fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
-exit(1);
-}
+if (error_is_set(&err)) {
+fprintf(stderr, "qemu: %s\n", error_get_pretty(err));
+error_free(err);
+ret = -1;
+goto error;
+}
 
-if (get_param_value(option, 128, "mem", optarg) == 0) {
-numa_info[nodenr].node_mem

Re: [Qemu-devel] [PATCH v2 0/5] s390: Support for Hotplug of Standby Memory

[Christian Borntraeger]

On 26/02/14 15:45, Paolo Bonzini wrote:
> Il 26/02/2014 15:42, Christian Borntraeger ha scritto:
>> On 24/02/14 22:30, Matthew Rosato wrote:
>>> This patchset adds support in s390 for a pool of standby memory,
>>> which can be set online/offline by the guest (ie, via chmem).
>>> New options, maxmem and slots, are added to the QEMU command line
>>> memory parameter to specify the total amount of memory available
>>> to the guest as well as the number of memory slots available.
>>> As part of this work, additional results are provided for the
>>> Read SCP Information SCLP, and new implentation is added for the
>>> Read Storage Element Information, Attach Storage Element,
>>> Assign Storage and Unassign Storage SCLPs, which enables the s390
>>> guest to manipulate the standby memory pool.
>>
>> Looks like Paolo will apply Igors latest patches to the numa branch,
>> I will defer patches 3-5 until Igors patches hit qemu master.
> 
> What are the dependencies of these series?  I can include it in my queue if:

The last version of the "memory hotplug" platches (its actually concept called
standby memory that the guest can request to be enabled from the hypervisor)
gained feedback from you to use Igors patch set instead of cooking up a new 
parameter
(http://lists.gnu.org/archive/html/qemu-devel/2013-12/msg03081.html)

Seems that the latest version
(https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02321.html)
does not match the patch in this series.

I have not fully reviewed patches 3-5 yet, so lets just defer memory hotplug
on s390 until Igors patch hit master.

Christian

> 
> (a) you give me your Acked-by
> 
> (b) Igor reviews it and tells me if this is the right version of his patch 
> "extend -m option to support options for memory hotplug"
> 
> Paolo
> 
>> Christian
>>
>>>
>>> This patchset is based on work originally done by Jeng-Fang (Nick)
>>> Wang.
>>>
>>> This patchset has been built to apply on the s390-next tree at:
>>>
>>> git://github.com/borntraeger/qemu.git s390-next
>>>
>>> Changes for v2:
>>>  * Removed the patch that introduced the standby-mem operand and
>>>instead included Igor Mammedov's patches that add the mem-opts
>>>'maxmem' and 'slots', with a slight modification due to the removal
>>>of qemu_opts_create_nofail.
>>>  * Patch 3 was inserted to add a new qom object that encapsulate variables
>>>used by s390 memory hotplug.  Patches 4 and 5 adjusted to use this
>>>object.
>>>  * Added additional code comments and other minor changes per Alexander
>>>Graf's comments
>>>
>>> Igor Mammedov (2):
>>>   vl: convert -m to QemuOpts
>>>   vl.c: extend -m option to support options for memory hotplug
>>>
>>> Matthew Rosato (3):
>>>   sclp-s390: Add device to manage s390 memory hotplug
>>>   virtio-ccw: Include standby memory when calculating storage increment
>>>   sclp-s390: Add memory hotplug SCLPs
>>>
>>>  hw/s390x/s390-virtio-ccw.c |   42 +--
>>>  hw/s390x/sclp.c|  276 
>>> ++--
>>>  include/hw/s390x/sclp.h|   19 +++
>>>  qemu-options.hx|   10 +-
>>>  target-s390x/cpu.h |   18 +++
>>>  target-s390x/kvm.c |5 +
>>>  vl.c   |   98 ++--
>>>  7 files changed, 440 insertions(+), 28 deletions(-)
>>>
>>
> 

Re: [Qemu-devel] [PATCH] s390: Storage key global access

[Jason J. Herne]

On 02/25/2014 02:34 PM, Andreas Färber wrote:

Hi,

Am 25.02.2014 20:15, schrieb Christian Borntraeger:

On 25/02/14 15:34, Jason J. Herne wrote:


Christian, at one point you mentioned that it might be helpful to see this 
patch in the context of the rest of the hotplug patches. If you still feel this 
way let me know and I'll post the 4-patch series. If not, I still propose this 
one for s390-next. Thanks :).


Do you feel your series is ready for upstream, then yet please post the whole 
series.
Posting independent things is good, but I feel that the storage key rework 
makes more
sense if the followup patches make clear why.


I had requested changes to that series that apparently I could not
communicate in a form Jason could digest, and I have since been caught
in downstream work and a backlog of other patches, not getting to
writing the alternative myself yet nor will I the next few days.



Andreas,

I believe I understood about 90% of what you were asking for. I had made 
many
of the changes you requested. The last version of my patches can be seen 
here


https://www.mail-archive.com/qemu-devel@nongnu.org/msg201796.html

Most of the requested changes should be found within this set of patches.


An outline of the idea as far as I remember was dropping the ipi array
instead of refactoring it to dynamic allocation and - having discussed
that a topology will not be needed - add them as cpu[n] child
properties of /machine, allowing access via QOM property getters instead
of some self-cooked solution. Open question was link<> or child<>
property and, if link<>, whether some setter hook in QOM infrastructure
may be needed to trigger the hot-add or whether QOM realize event will
be sufficient.


As per your original suggestion to try link<>, I wrote the code to do 
exactly

that. Please see patch 6/8 in the above series.

I have a version of these patches rebased for the latest master, but my test
system is currently in use. I would like a chance to regression test them
before I post them. I anticipate I should be able to do this today.

At your option, you may opt to review the patches as posted at the above 
link.
While some modifications have since been made, the overall design and 
function

is the same.

I look forward to your comments. Thank you for your time.

--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)

Re: [Qemu-devel] [PATCH v2 1/2] block: gluster - code movements, state storage changes

[Stefan Hajnoczi]

On Mon, Feb 17, 2014 at 11:11:11AM -0500, Jeff Cody wrote:
> In preparation for supporting reopen on gluster, move flag
> parsing out to a function.  Also, store open_flags and filename
> in the gluster state storage struct, and add a NULL check in the
> gconf cleanup.

It no longer stores open_flags and filename in the gluster state storage
struct.  I can fix this up when merging.

Re: [Qemu-devel] [PATCH v2 1/2] block: gluster - code movements, state storage changes

[Jeff Cody]

On Wed, Feb 26, 2014 at 03:58:40PM +0100, Stefan Hajnoczi wrote:
> On Mon, Feb 17, 2014 at 11:11:11AM -0500, Jeff Cody wrote:
> > In preparation for supporting reopen on gluster, move flag
> > parsing out to a function.  Also, store open_flags and filename
> > in the gluster state storage struct, and add a NULL check in the
> > gconf cleanup.
> 
> It no longer stores open_flags and filename in the gluster state storage
> struct.  I can fix this up when merging.

Ah, yes.  Thanks.

Re: [Qemu-devel] [PATCH v2 2/2] block: gluster - add reopen support.

[Stefan Hajnoczi]

On Mon, Feb 17, 2014 at 11:11:12AM -0500, Jeff Cody wrote:
> +static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> +   BlockReopenQueue *queue, Error **errp)
> +{
> +int ret = 0;
> +BDRVGlusterReopenState *reop_s;
> +GlusterConf *gconf = NULL;
> +int open_flags = 0;
> +
> +assert(state != NULL);
> +assert(state->bs != NULL);
> +
> +state->opaque = g_malloc0(sizeof(BDRVGlusterReopenState));

Not worth respinning but if you use the type name, then the more concise
notation is:

state->opaque = g_new0(BDRVGlusterReopenState, 1);

Re: [Qemu-devel] [PATCH v2 0/2] block: add suppoort for gluster reopen

[Stefan Hajnoczi]

On Mon, Feb 17, 2014 at 11:11:10AM -0500, Jeff Cody wrote:
> Changes v1->v2:
> 
> Patch 1: Removed unneeded state variables 'filename' and
>  'open_flags' (Stefan)
> Patch 2: Removed unneeded reopen state variable 'open_flags'
>  (Stefan)
> 
> This series provides support for bdrv_reopen() with gluster
> protocol drivers, and thereby also enabling block-commit to
> gluster-based images.
> 
> Jeff Cody (2):
>   block: gluster - code movements, state storage changes
>   block: gluster - add reopen support.
> 
>  block/gluster.c | 143 
> ++--
>  1 file changed, 128 insertions(+), 15 deletions(-)

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

Stefan

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Michael S. Tsirkin]

On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> If the expected (offline) acpi tables loaded correctly,
> it is safe to assume the iasl installation is OK and
> issue an error if the actual tables differ from expected
> ones.
> 
> Signed-off-by: Marcel Apfelbaum 

I'm not sure I agree with this one.
It turned out to be too aggressive in the past
as expected files get out of sync sometimes.
What I would do is this:
if IASL did not produce errors or warnings
on expected files, it should not produce
them on actual files either.



> ---
>  tests/acpi-test.c | 24 
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> index 2ce8c18..639b3ab 100644
> --- a/tests/acpi-test.c
> +++ b/tests/acpi-test.c
> @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t 
> signature)
> return sdt->header.signature == signature;
>  }
>  
> -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
>  {
>  AcpiSdtTable *temp;
>  GError *error = NULL;
> @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
>  g_string_append_printf(command_line, "-d %s", sdt->aml_file);
>  
>  /* pass 'out' and 'out_err' in order to be redirected */
> -g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> &error);
> +ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> &error);
>  g_assert_no_error(error);
>  
> -ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> -  &sdt->asl_len, &error);
> -g_assert(ret);
> -g_assert_no_error(error);
> -g_assert(sdt->asl_len);
> +if (ret) {
> +ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> +  &sdt->asl_len, &error);
> +g_assert(ret);
> +g_assert_no_error(error);
> +g_assert(sdt->asl_len);
> +}
>  
>  g_free(out);
>  g_free(out_err);
>  g_string_free(command_line, true);
> +
> +return !ret;
>  }
>  
>  #define COMMENT_END "*/"
> @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
>  int i;
>  AcpiSdtTable *sdt, *exp_sdt;
>  test_data exp_data;
> +gboolean err;
>  
>  memset(&exp_data, 0, sizeof(exp_data));
>  exp_data.tables = load_expected_aml(data);
> @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
>  load_asl(data->tables, sdt);
>  asl = normalize_asl(sdt->asl);
>  
> -load_asl(exp_data.tables, exp_sdt);
> +err = load_asl(exp_data.tables, exp_sdt);
>  exp_asl = normalize_asl(exp_sdt->asl);
>  
>  if (g_strcmp0(asl->str, exp_asl->str)) {
> @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
>  (gchar *)&exp_sdt->header.signature,
>  sdt->asl_file, sdt->aml_file,
>  exp_sdt->asl_file, exp_sdt->aml_file);
> +if (!err) { /* expected data loaded, iasl OK */
> +g_assert(false);
> +}
>  }
>  g_string_free(asl, true);
>  g_string_free(exp_asl, true);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH V3 0/3] Fix some quorum nits after merge

[Stefan Hajnoczi]

On Sat, Feb 22, 2014 at 06:43:39PM +0100, Benoît Canet wrote:
> in v3:
> use $QEMU_IMG (Fam)
> apply Eric reviewed by
> 
> in v2:
> 
> make error optional and return strerror(-ret) [Eric]
> better documentation of the error string [Eric]
> apply Eric reviewed by
> 
> 
> Benoît Canet (3):
>   qmp: Fix BlockdevOptionQuorum.
>   qmp: Make Quorum error events more palatable.
>   qemu-io-test: Disable Quorum test when not compiled in.
> 
>  block/quorum.c |  9 --
>  docs/qmp/qmp-events.txt| 75 
> --
>  qapi-schema.json   |  5 ++--
>  tests/qemu-iotests/081 |  3 ++
>  tests/qemu-iotests/081.out |  2 +-
>  5 files changed, 52 insertions(+), 42 deletions(-)

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

Stefan

[Qemu-devel] [PATCH] seccomp: add shmctl(), mlock(), and munlock() to the syscall whitelist

[Paul Moore]

Additional testing reveals that PulseAudio requires shmctl() and the
mlock()/munlock() syscalls on some systems/configurations.  As before,
on systems that do require these syscalls, the problem can be seen with
the following command line:

  # qemu -monitor stdio  -sandbox on \
 -device intel-hda -device hda-duplex

Signed-off-by: Paul Moore 
---
 qemu-seccomp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index caa926e..3db1e9b 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -225,7 +225,10 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(fchmod), 240 },
 { SCMP_SYS(shmget), 240 },
 { SCMP_SYS(shmat), 240 },
-{ SCMP_SYS(shmdt), 240 }
+{ SCMP_SYS(shmdt), 240 },
+{ SCMP_SYS(shmctl), 240 },
+{ SCMP_SYS(mlock), 240 },
+{ SCMP_SYS(munlock), 240 }
 };
 
 int seccomp_start(void)

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > If the expected (offline) acpi tables loaded correctly,
> > it is safe to assume the iasl installation is OK and
> > issue an error if the actual tables differ from expected
> > ones.
> > 
> > Signed-off-by: Marcel Apfelbaum 
> 
> I'm not sure I agree with this one.
> It turned out to be too aggressive in the past
> as expected files get out of sync sometimes.
> What I would do is this:
>   if IASL did not produce errors or warnings
>   on expected files, it should not produce
>   them on actual files either.
The whole point of this patch (series) was to find
a way to discover "real" errors in some cases (not in all).
What you suggested above removes the only "discoverable error"
and returns us to warning only mode.

However, the code does not look into stdout/stderr for iasl
errors (it would involve parsing because iasl *always* writes
to both), but checks if the invocation of iasl command terminates successfully,
merely saying: "Well, iasl invocation succeeded  for the expected files, there
is no reason to fail for the actual ones". The actual output is not verified.

No impact on "out of sync expected files".

Thanks,
Marcel

 

> 
> 
> > ---
> >  tests/acpi-test.c | 24 
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > index 2ce8c18..639b3ab 100644
> > --- a/tests/acpi-test.c
> > +++ b/tests/acpi-test.c
> > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, 
> > uint32_t signature)
> > return sdt->header.signature == signature;
> >  }
> >  
> > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> >  {
> >  AcpiSdtTable *temp;
> >  GError *error = NULL;
> > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> >  g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> >  
> >  /* pass 'out' and 'out_err' in order to be redirected */
> > -g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> > &error);
> > +ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, 
> > NULL, &error);
> >  g_assert_no_error(error);
> >  
> > -ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > -  &sdt->asl_len, &error);
> > -g_assert(ret);
> > -g_assert_no_error(error);
> > -g_assert(sdt->asl_len);
> > +if (ret) {
> > +ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > +  &sdt->asl_len, &error);
> > +g_assert(ret);
> > +g_assert_no_error(error);
> > +g_assert(sdt->asl_len);
> > +}
> >  
> >  g_free(out);
> >  g_free(out_err);
> >  g_string_free(command_line, true);
> > +
> > +return !ret;
> >  }
> >  
> >  #define COMMENT_END "*/"
> > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> >  int i;
> >  AcpiSdtTable *sdt, *exp_sdt;
> >  test_data exp_data;
> > +gboolean err;
> >  
> >  memset(&exp_data, 0, sizeof(exp_data));
> >  exp_data.tables = load_expected_aml(data);
> > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> >  load_asl(data->tables, sdt);
> >  asl = normalize_asl(sdt->asl);
> >  
> > -load_asl(exp_data.tables, exp_sdt);
> > +err = load_asl(exp_data.tables, exp_sdt);
> >  exp_asl = normalize_asl(exp_sdt->asl);
> >  
> >  if (g_strcmp0(asl->str, exp_asl->str)) {
> > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> >  (gchar *)&exp_sdt->header.signature,
> >  sdt->asl_file, sdt->aml_file,
> >  exp_sdt->asl_file, exp_sdt->aml_file);
> > +if (!err) { /* expected data loaded, iasl OK */
> > +g_assert(false);
> > +}
> >  }
> >  g_string_free(asl, true);
> >  g_string_free(exp_asl, true);
> > -- 
> > 1.8.3.1

Re: [Qemu-devel] qemu-img convert cache mode for source

[Stefan Hajnoczi]

On Wed, Feb 26, 2014 at 11:14:04AM +0100, Peter Lieven wrote:
> I was wondering if it would be a good idea to set the O_DIRECT mode for the 
> source
> files of a qemu-img convert process if the source is a host_device?
> 
> Currently the backup of a host device is polluting the page cache.

Points to consider:

1. O_DIRECT does not work on Linux tmpfs, you get EINVAL when opening
   the file.  A fallback is necessary.

2. O_DIRECT has no readahead so performance could actually decrease.
   The question is, how important is reahead versus polluting page
   cache?

3. For raw files it would make sense to tell the kernel that access is
   sequential and data will be used only once.  Then we can get the best
   of both worlds (avoid polluting page cache but still get readahead).
   This is done using posix_fadvise(2).

   The problem is what to do for image formats.  An image file can be
   very fragmented so the readahead might not be a win.  Does this mean
   that for image formats we should tell the kernel access will be
   random?

   Furthermore, maybe it's best to do readahead inside QEMU so that even
   network protocols (nbd, iscsi, etc) can get good performance.  They
   act like O_DIRECT is always on.

It seems reasonable to investigate this stuff more.  Please run
benchmarks so we have justification to merge patches.

Stefan

Re: [Qemu-devel] qemu-img convert cache mode for source

[Eric Blake]

On 02/26/2014 08:41 AM, Stefan Hajnoczi wrote:
> On Wed, Feb 26, 2014 at 11:14:04AM +0100, Peter Lieven wrote:
>> I was wondering if it would be a good idea to set the O_DIRECT mode for the 
>> source
>> files of a qemu-img convert process if the source is a host_device?
>>
>> Currently the backup of a host device is polluting the page cache.
> 
> Points to consider:
> 
> 1. O_DIRECT does not work on Linux tmpfs, you get EINVAL when opening
>the file.  A fallback is necessary.
> 
> 2. O_DIRECT has no readahead so performance could actually decrease.
>The question is, how important is reahead versus polluting page
>cache?
> 
> 3. For raw files it would make sense to tell the kernel that access is
>sequential and data will be used only once.  Then we can get the best
>of both worlds (avoid polluting page cache but still get readahead).
>This is done using posix_fadvise(2).

Except that posix_fadvise is advisory only (the kernel is free to ignore
it), and currently not stateful enough inside the kernel to be useful
when handing fds between processes.  For several years now, I've asked
if the kernel could provide better guarantees about what posix_fadvise
can actually do, and expose user-space introspection of those guarantees
through procfs and/or fpathconf.

See https://bugzilla.redhat.com/show_bug.cgi?id=634653 for some
backstory on libvirt's dealings with O_DIRECT. I'd really like to ditch
libvirt's use of O_DIRECT in favor of posix_fadvise for avoiding page
cache pollution, but the kernel isn't at a point yet that lets libvirt
do that.  I suppose that if the kernel ever does improve posix_fadvise,
then both libvirt and qemu would benefit from it.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

On Wed, 2014-02-26 at 17:52 +0200, Michael S. Tsirkin wrote:
> On Wed, Feb 26, 2014 at 05:39:39PM +0200, Marcel Apfelbaum wrote:
> > On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> > > On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > > > If the expected (offline) acpi tables loaded correctly,
> > > > it is safe to assume the iasl installation is OK and
> > > > issue an error if the actual tables differ from expected
> > > > ones.
> > > > 
> > > > Signed-off-by: Marcel Apfelbaum 
> > > 
> > > I'm not sure I agree with this one.
> > > It turned out to be too aggressive in the past
> > > as expected files get out of sync sometimes.
> > > What I would do is this:
> > >   if IASL did not produce errors or warnings
> > >   on expected files, it should not produce
> > >   them on actual files either.
> > The whole point of this patch (series) was to find
> > a way to discover "real" errors in some cases (not in all).
> > What you suggested above removes the only "discoverable error"
> > and returns us to warning only mode.
> > 
> > However, the code does not look into stdout/stderr for iasl
> > errors (it would involve parsing because iasl *always* writes
> > to both), but checks if the invocation of iasl command terminates 
> > successfully,
> > merely saying: "Well, iasl invocation succeeded  for the expected files, 
> > there
> > is no reason to fail for the actual ones". The actual output is not 
> > verified.
> > 
> > No impact on "out of sync expected files".
> > 
> > Thanks,
> > Marcel
> > 
> 
> aha
> what confused me is this:
> if (!err) { /* expected data loaded, iasl OK */
>g_assert(false);
> }
> 
> how about
> 
>   g_assert(err);
> 
> instead?
You are right!
I'll send a new version with this fix.

Thanks,
Marcel
> 
> > 
> > > 
> > > 
> > > > ---
> > > >  tests/acpi-test.c | 24 
> > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > > > index 2ce8c18..639b3ab 100644
> > > > --- a/tests/acpi-test.c
> > > > +++ b/tests/acpi-test.c
> > > > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, 
> > > > uint32_t signature)
> > > > return sdt->header.signature == signature;
> > > >  }
> > > >  
> > > > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > >  {
> > > >  AcpiSdtTable *temp;
> > > >  GError *error = NULL;
> > > > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable 
> > > > *sdt)
> > > >  g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> > > >  
> > > >  /* pass 'out' and 'out_err' in order to be redirected */
> > > > -g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> > > > &error);
> > > > +ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, 
> > > > NULL, &error);
> > > >  g_assert_no_error(error);
> > > >  
> > > > -ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > > -  &sdt->asl_len, &error);
> > > > -g_assert(ret);
> > > > -g_assert_no_error(error);
> > > > -g_assert(sdt->asl_len);
> > > > +if (ret) {
> > > > +ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > > +  &sdt->asl_len, &error);
> > > > +g_assert(ret);
> > > > +g_assert_no_error(error);
> > > > +g_assert(sdt->asl_len);
> > > > +}
> > > >  
> > > >  g_free(out);
> > > >  g_free(out_err);
> > > >  g_string_free(command_line, true);
> > > > +
> > > > +return !ret;
> > > >  }
> > > >  
> > > >  #define COMMENT_END "*/"
> > > > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> > > >  int i;
> > > >  AcpiSdtTable *sdt, *exp_sdt;
> > > >  test_data exp_data;
> > > > +gboolean err;
> > > >  
> > > >  memset(&exp_data, 0, sizeof(exp_data));
> > > >  exp_data.tables = load_expected_aml(data);
> > > > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> > > >  load_asl(data->tables, sdt);
> > > >  asl = normalize_asl(sdt->asl);
> > > >  
> > > > -load_asl(exp_data.tables, exp_sdt);
> > > > +err = load_asl(exp_data.tables, exp_sdt);
> > > >  exp_asl = normalize_asl(exp_sdt->asl);
> > > >  
> > > >  if (g_strcmp0(asl->str, exp_asl->str)) {
> > > > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> > > >  (gchar *)&exp_sdt->header.signature,
> > > >  sdt->asl_file, sdt->aml_file,
> > > >  exp_sdt->asl_file, exp_sdt->aml_file);
> > > > +if (!err) { /* expected data loaded, iasl OK */
> > > > +g_assert(false);
> > > > +}
> > > >  }
> > > >  g_string_free(asl, true);
> > > >  g_string_free(exp_as

Re: [Qemu-devel] qemu-img convert cache mode for source

[Peter Lieven]

On 26.02.2014 16:41, Stefan Hajnoczi wrote:

On Wed, Feb 26, 2014 at 11:14:04AM +0100, Peter Lieven wrote:

I was wondering if it would be a good idea to set the O_DIRECT mode for the 
source
files of a qemu-img convert process if the source is a host_device?

Currently the backup of a host device is polluting the page cache.

Points to consider:

1. O_DIRECT does not work on Linux tmpfs, you get EINVAL when opening
the file.  A fallback is necessary.

2. O_DIRECT has no readahead so performance could actually decrease.
The question is, how important is reahead versus polluting page
cache?

3. For raw files it would make sense to tell the kernel that access is
sequential and data will be used only once.  Then we can get the best
of both worlds (avoid polluting page cache but still get readahead).
This is done using posix_fadvise(2).

The problem is what to do for image formats.  An image file can be
very fragmented so the readahead might not be a win.  Does this mean
that for image formats we should tell the kernel access will be
random?

Furthermore, maybe it's best to do readahead inside QEMU so that even
network protocols (nbd, iscsi, etc) can get good performance.  They
act like O_DIRECT is always on.

your comments are regarding qemu-img convert, right?
How would you implement this? A new open flag because
the fadvise had to goto inside the protocol driver.

I would start with host_devices first and see how it performs there.

For qemu-img convert I would issue a FADV_DONTNEED after
a write for the bytes that have been written
(i have tested this with Linux and it seems to work quite well).

Question is, what is the right paramter for reads? Also FADV_DONTNEED?

Peter


It seems reasonable to investigate this stuff more.  Please run
benchmarks so we have justification to merge patches.

Stefan

Geschäftsführer: Heiner Lante | Michael Lante Amtsgericht Duisburg | HRB Nr. 
12154 USt-Id-Nr.: DE 120607556 
...

[Qemu-devel] [PATCH RESEND] CODING_STYLE: Section about mixed declarations

[Eduardo Habkost]

We had an unwritten rule about declarations having to be at beginning of
blocks. Make it a written rule.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Stefan Weil 
---
Changes v2:
 * s/be at beginning/be at the beginning/
---
 CODING_STYLE | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/CODING_STYLE b/CODING_STYLE
index dcbce28..efa5cc3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -84,3 +84,10 @@ and clarity it comes on a line by itself:
 Rationale: a consistent (except for functions...) bracing style reduces
 ambiguity and avoids needless churn when lines are added or removed.
 Furthermore, it is the QEMU coding style.
+
+5. Declarations
+
+Mixed declarations (interleaving statements and declarations within blocks) are
+not allowed; declarations should be at the beginning of blocks.  In other 
words,
+the code should not generate warnings if using GCC's
+-Wdeclaration-after-statement option.
-- 
1.8.5.3

[Qemu-devel] [PATCH v2 1/3] nbd: close socket if connection breaks

[Stefan Hajnoczi]

nbd_receive_reply() is called by the event loop whenever data is
available or the socket has been closed by the remote side.

This patch closes the socket when an error occurs to prevent the
nbd_receive_reply() handler from being called indefinitely after the
connection has failed.

Note that we were already correctly returning EIO for pending requests
but leaving the nbd_receive_reply() handler registered resulted in high
CPU consumption and a flood of error messages.

Reuse nbd_teardown_connection() to close the socket.

Reported-by: Zhifeng Cai 
Signed-off-by: Stefan Hajnoczi 
---
 block/nbd-client.c | 33 ++---
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 0922b78..7d698cb 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -43,6 +43,17 @@ static void nbd_recv_coroutines_enter_all(NbdClientSession 
*s)
 }
 }
 
+static void nbd_teardown_connection(NbdClientSession *client)
+{
+/* finish any pending coroutines */
+shutdown(client->sock, 2);
+nbd_recv_coroutines_enter_all(client);
+
+qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
+closesocket(client->sock);
+client->sock = -1;
+}
+
 static void nbd_reply_ready(void *opaque)
 {
 NbdClientSession *s = opaque;
@@ -78,7 +89,7 @@ static void nbd_reply_ready(void *opaque)
 }
 
 fail:
-nbd_recv_coroutines_enter_all(s);
+nbd_teardown_connection(s);
 }
 
 static void nbd_restart_write(void *opaque)
@@ -324,7 +335,7 @@ int nbd_client_session_co_discard(NbdClientSession *client, 
int64_t sector_num,
 
 }
 
-static void nbd_teardown_connection(NbdClientSession *client)
+void nbd_client_session_close(NbdClientSession *client)
 {
 struct nbd_request request = {
 .type = NBD_CMD_DISC,
@@ -332,22 +343,14 @@ static void nbd_teardown_connection(NbdClientSession 
*client)
 .len = 0
 };
 
-nbd_send_request(client->sock, &request);
-
-/* finish any pending coroutines */
-shutdown(client->sock, 2);
-nbd_recv_coroutines_enter_all(client);
-
-qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
-closesocket(client->sock);
-client->sock = -1;
-}
-
-void nbd_client_session_close(NbdClientSession *client)
-{
 if (!client->bs) {
 return;
 }
+if (client->sock == -1) {
+return;
+}
+
+nbd_send_request(client->sock, &request);
 
 nbd_teardown_connection(client);
 client->bs = NULL;
-- 
1.8.5.3

Re: [Qemu-devel] [PATCH 2/2] vl: convert -m to QemuOpts

[Paolo Bonzini]

Il 26/02/2014 14:50, Igor Mammedov ha scritto:

It should be fixed in v3 that you were going to apply to numa branch
https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02321.html


Yes, applied now.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor

[Michael S. Tsirkin]

On Mon, Feb 17, 2014 at 04:49:41PM +0100, Igor Mammedov wrote:
> ... while at it move most of numa related code to
> a dedicated file so that it won't to clatter vl.c.
> 
> git tree for testing:
> https://github.com/imammedo/qemu/commits/numa_prep_v1
> 
> Igor Mammedov (1):
>   vl.c: fix style issue

Just to clarify - Paolo, you intend to merge this?
I acked pc.c changes.

> Wanlong Gao (5):
>   NUMA: move numa related code to new file numa.c
>   NUMA: check if the total numa memory size is equal to ram_size
>   NUMA: Add numa_info structure to contain numa nodes info
>   NUMA: convert -numa option to use OptsVisitor
>   NUMA: expand MAX_NODES from 64 to 128
> 
>  Makefile.target |2 +-
>  cpus.c  |   14 
>  hw/i386/pc.c|   14 ++-
>  hw/ppc/spapr.c  |   11 ++-
>  include/sysemu/cpus.h   |1 -
>  include/sysemu/sysemu.h |   14 +++-
>  monitor.c   |2 +-
>  numa.c  |  189 
> +++
>  qapi-schema.json|   32 
>  vl.c|  155 +++---
>  10 files changed, 262 insertions(+), 172 deletions(-)
>  create mode 100644 numa.c

Re: [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info

[Michael S. Tsirkin]

On Mon, Feb 17, 2014 at 04:49:45PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao 
> 
> Add the numa_info structure to contain the numa nodes memory,
> VCPUs information and the future added numa nodes host memory
> policies.
> 
> Reviewed-by: Eduardo Habkost 
> Signed-off-by: Andre Przywara 
> Signed-off-by: Wanlong Gao 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Igor Mammedov 

For pc.c changes here:

Reviewed-by: Michael S. Tsirkin 


> ---
>  hw/i386/pc.c|   14 +-
>  hw/ppc/spapr.c  |   11 ++-
>  include/sysemu/sysemu.h |8 ++--
>  monitor.c   |2 +-
>  numa.c  |   23 ---
>  vl.c|7 +++
>  6 files changed, 37 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e715a33..42182f9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -674,14 +674,14 @@ static FWCfgState *bochs_bios_init(void)
>  unsigned int apic_id = x86_cpu_apic_id_from_index(i);
>  assert(apic_id < apic_id_limit);
>  for (j = 0; j < nb_numa_nodes; j++) {
> -if (test_bit(i, node_cpumask[j])) {
> +if (test_bit(i, numa_info[j].node_cpu)) {
>  numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
>  break;
>  }
>  }
>  }
>  for (i = 0; i < nb_numa_nodes; i++) {
> -numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
> +numa_fw_cfg[apic_id_limit + 1 + i] = 
> cpu_to_le64(numa_info[i].node_mem);
>  }
>  fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
>   (1 + apic_id_limit + nb_numa_nodes) *
> @@ -1077,8 +1077,12 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>  guest_info->apic_id_limit = pc_apic_id_limit(max_cpus);
>  guest_info->apic_xrupt_override = kvm_allows_irq0_override();
>  guest_info->numa_nodes = nb_numa_nodes;
> -guest_info->node_mem = g_memdup(node_mem, guest_info->numa_nodes *
> -sizeof *guest_info->node_mem);
> +guest_info->node_mem = g_malloc0(guest_info->numa_nodes *
> + sizeof *guest_info->node_mem);
> +for (i = 0; i < nb_numa_nodes; i++) {
> +guest_info->node_mem[i] = numa_info[i].node_mem;
> +}
> +
>  guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit *
>   sizeof *guest_info->node_cpu);
>  
> @@ -1086,7 +1090,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>  unsigned int apic_id = x86_cpu_apic_id_from_index(i);
>  assert(apic_id < guest_info->apic_id_limit);
>  for (j = 0; j < nb_numa_nodes; j++) {
> -if (test_bit(i, node_cpumask[j])) {
> +if (test_bit(i, numa_info[j].node_cpu)) {
>  guest_info->node_cpu[apic_id] = j;
>  break;
>  }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 93d02c1..6fd2d95 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -531,8 +531,8 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, 
> void *fdt)
>  int i, off;
>  
>  /* memory node(s) */
> -if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
> -node0_size = node_mem[0];
> +if (nb_numa_nodes > 1 && numa_info[0].node_mem < ram_size) {
> +node0_size = numa_info[0].node_mem;
>  } else {
>  node0_size = ram_size;
>  }
> @@ -570,7 +570,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, 
> void *fdt)
>  if (mem_start >= ram_size) {
>  node_size = 0;
>  } else {
> -node_size = node_mem[i];
> +node_size = numa_info[i].node_mem;
>  if (node_size > ram_size - mem_start) {
>  node_size = ram_size - mem_start;
>  }
> @@ -697,7 +697,8 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>  
>  /* Update the RMA size if necessary */
>  if (spapr->vrma_adjust) {
> -hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> +hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem :
> +  ram_size;
>  spapr->rma_size = kvmppc_rma_size(node0_size, spapr->htab_shift);
>  }
>  }
> @@ -1115,7 +1116,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>  MemoryRegion *sysmem = get_system_memory();
>  MemoryRegion *ram = g_new(MemoryRegion, 1);
>  hwaddr rma_alloc_size;
> -hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> +hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem : 
> ram_size;
>  uint32_t initrd_base = 0;
>  long kernel_size = 0, initrd_size = 0;
>  long load_limit, rtas_limit, fw_size;
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 2509649..d873b42 100644
> --- a/include/sysemu/sysemu.h
>

Re: [Qemu-devel] [PATCH v8 00/17] Vhost and vhost-net support for userspace based backends

[Michael S. Tsirkin]

On Thu, Feb 13, 2014 at 01:03:11PM +0100, Antonios Motakis wrote:
> In this patch series we would like to introduce our approach for putting a
> virtio-net backend in an external userspace process. Our eventual target is to
> run the network backend in the Snabbswitch ethernet switch, while receiving
> traffic from a guest inside QEMU/KVM which runs an unmodified virtio-net
> implementation.
> 
> For this, we are working into extending vhost to allow equivalent 
> functionality
> for userspace. Vhost already passes control of the data plane of virtio-net to
> the host kernel; we want to realize a similar model, but for userspace.
> 
> In this patch series the concept of a vhost-backend is introduced.
> 
> We define two vhost backend types - vhost-kernel and vhost-user. The former is
> the interface to the current kernel module implementation. Its control plane 
> is
> ioctl based. The data plane is realized by the kernel directly accessing the
> QEMU allocated, guest memory.
> 
> In the new vhost-user backend, the control plane is based on communication
> between QEMU and another userspace process using a unix domain socket. This
> allows to implement a virtio backend for a guest running in QEMU, inside the
> other userspace process. For this communication we use a chardev with a Unix
> domain socket backend. Vhost-user is client/server agnostic regarding the
> chardev, however it does not support the 'nowait' and 'telnet' options.
> 
> A reconnection in 'server' mode is supported, but the backend's exposed virtio
> features need to be compatible with the first connected slave.
> 
> We change -mem-path to QemuOpts and add prealloc and share as properties
> to it. HugeTLBFS is required for this option to work.
> 
> The data path is realized by directly accessing the vrings and the buffer data
> off the guest's memory.
> 
> The current user of vhost-user is only vhost-net. We add new netdev backend
> that is intended to initialize vhost-net with vhost-user backend.
> 
> Example usage:
> 
> qemu -m 1024 -mem-path /hugetlbfs,share=on \
>  -chardev socket,id=chr0,path=/path/to/socket \
>  -netdev type=vhost-user,id=net0,chardev=chr0 \
>  -device virtio-net-pci,netdev=net0
> 
> On non-MSIX guests the vhost feture can be forced using a special option:
> 
> ...
>  -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce
> ...
> 
> This code can be pulled from g...@github.com:virtualopensystems/qemu.git 
> vhost-user-v8
> A simple functional test is available in tests/vhost-user-test.c
> 
> A reference vhost-user slave for testing is also available from 
> g...@github.com:virtualopensystems/vapp.git

OK so this will have to be rebased on top of qemu opts
rework once that's in.
Also pls address Eric's comments.

> Changes from v7:
>  - Slave reconnection when using chardev in server mode
>  - qtest vhost-user-test added
>  - New qemu_chr_fe_get_msgfds for reading multiple fds from the chardev
>  - Mandatory features in vhost_dev, used on reconnect to verify for conflicts
>  - Add vhostforce parameter to -netdev vhost-user (for non-MSIX guests)
>  - Extend libqemustub.a to support qemu-char.c
> 
> Changes from v6:
>  - Remove the 'unlink' property of '-mem-path'
>  - Extend qemu-char: blocking read, send fds, monitor for connection close
>  - Vhost-user uses chardev as a backend
>  - Poll and reconnect removed (no VHOST_USER_ECHO).
>  - Disconnect is deteced by the chardev (G_IO_HUP event)
>  - vhost-backend.c split to vhost-user.c
> 
> Changes from v5:
>  - Split -mem-path unlink option to a separate patch
>  - Fds are passed only in the ancillary data
>  - Stricter message size checks on receive/send
>  - Netdev vhost-user now includes path and poll_time options
>  - The connection probing interval is configurable
> 
> Changes from v4:
>  - Use error_report for errors
>  - VhostUserMsg has new field `size` indicating the following payload length.
>Field `flags` now has version and reply bits. The structure is packed.
>  - Send data is of variable length (`size` field in message)
>  - Receive in 2 steps, header and payload
>  - Add new message type VHOST_USER_ECHO, to check connection status
> 
> Changes from v3:
>  - Convert -mem-path to QemuOpts with prealloc, share and unlink properties
>  - Set 1 sec timeout when read/write to the unix domain socket
>  - Fix file descriptor leak
> 
> Changes from v2:
>  - Reconnect when the backend disappears
> 
> Changes from v1:
>  - Implementation of vhost-user netdev backend
>  - Code improvements
> 
> Antonios Motakis (17):
>   Convert -mem-path to QemuOpts and add prealloc and share properties
>   Add chardev API qemu_chr_fe_read_all
>   Add chardev API qemu_chr_fe_set_msgfds
>   Add chardev API qemu_chr_fe_get_msgfds
>   Add G_IO_HUP handler for socket chardev
>   vhost_net should call the poll callback only when it is set
>   Refactor virtio-net to use generic get_vhost_net
>   vhost_net_init will use VhostNetOptions to get all its arguments
>   Add vh

Re: [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor

[Paolo Bonzini]

Il 26/02/2014 17:42, Michael S. Tsirkin ha scritto:

> ... while at it move most of numa related code to
> a dedicated file so that it won't to clatter vl.c.
>
> git tree for testing:
> https://github.com/imammedo/qemu/commits/numa_prep_v1
>
> Igor Mammedov (1):
>   vl.c: fix style issue

Just to clarify - Paolo, you intend to merge this?


Some cleanups could go in 2.0 but the meat will have to wait, so there's 
no need to hurry.  I haven't yet talked with all involved people about 
the merging plan but here is a possibility:


* during soft/hard freeze we'll finish development of the memdev 
backend, to include the -mem-path replacement.  I already have some 
patches for this, that I intend to post to the list sometime soon.


* at the beginning of the 2.1 cycle I will post a pull request with Hu 
Tao's NUMA policies, which includes all the memdev backend.


* memory hotplug would go through your tree so it should wait for NUMA 
policies to be in.  Of course, Igor can also post it soon, so that it's 
also mostly ready at the beginning of the 2.1 cycle.



I acked pc.c changes.


Thanks!

Paolo

Re: [Qemu-devel] [PATCH v4] target-sparc: Add and use CPU_FEATURE_CASA

[Fabien Chouteau]

On 02/26/2014 08:56 AM, Sebastian Huber wrote:
> Hello,
> 
> exists there someone who is able to commit this?
> 

I'm sorry Sebastian, as you probably understood the SPARC maintainer is
missing which makes commit process more difficult that it used to be...

Peter, as discussed in a previous mail, do you agree to apply SPARC
patches that have been reviewed?

Regards,

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Michael S. Tsirkin]

On Wed, Feb 26, 2014 at 05:39:39PM +0200, Marcel Apfelbaum wrote:
> On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > > If the expected (offline) acpi tables loaded correctly,
> > > it is safe to assume the iasl installation is OK and
> > > issue an error if the actual tables differ from expected
> > > ones.
> > > 
> > > Signed-off-by: Marcel Apfelbaum 
> > 
> > I'm not sure I agree with this one.
> > It turned out to be too aggressive in the past
> > as expected files get out of sync sometimes.
> > What I would do is this:
> > if IASL did not produce errors or warnings
> > on expected files, it should not produce
> > them on actual files either.
> The whole point of this patch (series) was to find
> a way to discover "real" errors in some cases (not in all).
> What you suggested above removes the only "discoverable error"
> and returns us to warning only mode.
> 
> However, the code does not look into stdout/stderr for iasl
> errors (it would involve parsing because iasl *always* writes
> to both), but checks if the invocation of iasl command terminates 
> successfully,
> merely saying: "Well, iasl invocation succeeded  for the expected files, there
> is no reason to fail for the actual ones". The actual output is not verified.
> 
> No impact on "out of sync expected files".
> 
> Thanks,
> Marcel
> 

aha
what confused me is this:
if (!err) { /* expected data loaded, iasl OK */
   g_assert(false);
}

how about

g_assert(err);

instead?

> 
> > 
> > 
> > > ---
> > >  tests/acpi-test.c | 24 
> > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > > index 2ce8c18..639b3ab 100644
> > > --- a/tests/acpi-test.c
> > > +++ b/tests/acpi-test.c
> > > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, 
> > > uint32_t signature)
> > > return sdt->header.signature == signature;
> > >  }
> > >  
> > > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > >  {
> > >  AcpiSdtTable *temp;
> > >  GError *error = NULL;
> > > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable 
> > > *sdt)
> > >  g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> > >  
> > >  /* pass 'out' and 'out_err' in order to be redirected */
> > > -g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> > > &error);
> > > +ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, 
> > > NULL, &error);
> > >  g_assert_no_error(error);
> > >  
> > > -ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > -  &sdt->asl_len, &error);
> > > -g_assert(ret);
> > > -g_assert_no_error(error);
> > > -g_assert(sdt->asl_len);
> > > +if (ret) {
> > > +ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > +  &sdt->asl_len, &error);
> > > +g_assert(ret);
> > > +g_assert_no_error(error);
> > > +g_assert(sdt->asl_len);
> > > +}
> > >  
> > >  g_free(out);
> > >  g_free(out_err);
> > >  g_string_free(command_line, true);
> > > +
> > > +return !ret;
> > >  }
> > >  
> > >  #define COMMENT_END "*/"
> > > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> > >  int i;
> > >  AcpiSdtTable *sdt, *exp_sdt;
> > >  test_data exp_data;
> > > +gboolean err;
> > >  
> > >  memset(&exp_data, 0, sizeof(exp_data));
> > >  exp_data.tables = load_expected_aml(data);
> > > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> > >  load_asl(data->tables, sdt);
> > >  asl = normalize_asl(sdt->asl);
> > >  
> > > -load_asl(exp_data.tables, exp_sdt);
> > > +err = load_asl(exp_data.tables, exp_sdt);
> > >  exp_asl = normalize_asl(exp_sdt->asl);
> > >  
> > >  if (g_strcmp0(asl->str, exp_asl->str)) {
> > > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> > >  (gchar *)&exp_sdt->header.signature,
> > >  sdt->asl_file, sdt->aml_file,
> > >  exp_sdt->asl_file, exp_sdt->aml_file);
> > > +if (!err) { /* expected data loaded, iasl OK */
> > > +g_assert(false);
> > > +}
> > >  }
> > >  g_string_free(asl, true);
> > >  g_string_free(exp_asl, true);
> > > -- 
> > > 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 4/4] virt: Set reset-cbar on CPUs

[Peter Maydell]

On 25 February 2014 08:23, Peter Crosthwaite
 wrote:
> On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell  
> wrote:
>> Set the reset-cbar property on CPUs used by the virt board,
>> if they have it. This isn't necessary for correct functioning
>> under Linux (since the A9 isn't a valid CPU for the virt board),
>> but it is the correct behaviour.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  hw/arm/virt.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 517f2fe..b418b77 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -379,6 +379,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>>  for (n = 0; n < smp_cpus; n++) {
>>  ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>>  Object *cpuobj;
>> +Error *err = NULL;
>>
>>  if (!oc) {
>>  fprintf(stderr, "Unable to find CPU definition\n");
>> @@ -390,6 +391,16 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>>  if (n > 0) {
>>  object_property_set_bool(cpuobj, true, "start-powered-off", 
>> NULL);
>>  }
>> +
>> +if (object_property_find(cpuobj, "reset-cbar", NULL)) {
>> +object_property_set_int(cpuobj, 
>> vbi->memmap[VIRT_CPUPERIPHS].base,
>> +"reset-cbar", &err);
>> +if (err) {
>> +error_report("%s", error_get_pretty(err));
>> +exit(1);
>> +}
>
> If the property exists (as enforced by the if), and you cant set it,
> its probably a fatal and candidate for error_abort.

Yeah. It was rather unclear to me what the right error handling
behaviour for these was.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] Extract non-QDicts in qdict_array_split()

[Max Reitz]

On 21.02.2014 23:32, Eric Blake wrote:

On 02/21/2014 11:11 AM, Max Reitz wrote:

Currently, qdict_array_split() splits a QDict like
   { "0.a": 42, "1": 23, "2.b": 84 }
into the QList
   [ { "a": 42 } ]
with the QDict still being
   { "1": 23, "2.b": 84 }

However, it makes more sense to create the QList
   [ { "a": 42 }, 23, { "b": 84 } ]
and having emptied the QDict.

This is implemented by this series.


Question - in the code, we have a comment:

/**
  * qdict_flatten(): For each nested QDict with key x, all fields with key y
  * are moved to this QDict and their key is renamed to "x.y". For each
nested
  * QList with key x, the field at index y is moved to this QDict with
the key
  * "x.y" (i.e., the reverse of what qdict_array_split() does).
  * This operation is applied recursively for nested QDicts and QLists.
  */

With your new split rules, do we need a followup patch to qdict_flatten
that can regenerate the QDict with "%u" keys for non-dict members of the
QList?


If you want qdict_flatten() to return a QDict with "%u" keys, you'd have 
to specify a QList. However, this would no longer be a qdict_flatten(), 
but rather a qlist_flatten().


It would be easy to make use of qdict_flatten_qdict() and 
qdict_flatten_qlist() to implement qlist_flatten() as well, but since 
there is currently no such function, apparently there is no need for it. 
Of course, I could just implement it preemptively, but then I'd probably 
be asked for its purpose. ;-)



Max

Re: [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size

[Eduardo Habkost]

On Mon, Feb 17, 2014 at 04:49:44PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao 
> 
> If the total number of the assigned numa nodes memory is not
> equal to the assigned ram size, it will write the wrong data
> to ACPI table, then the guest will ignore the wrong ACPI table
> and recognize all memory to one node. It's buggy, we should
> check it to ensure that we write the right data to ACPI table.
> 
> Signed-off-by: Wanlong Gao 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Igor Mammedov 
> ---
>  numa.c |   10 ++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/numa.c b/numa.c
> index 7845036..d12a4f2 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -151,6 +151,16 @@ void set_numa_nodes(void)
>  node_mem[i] = ram_size - usedmem;
>  }
>  
> +uint64_t numa_total = 0;

I was going to point out that variable declarations in the middle of
blocks goes against coding style (at least I was told so), but my patch
to amend CODING_STYLE to document it was ignored for 2 weeks, already.
So, I am not sure we really have that rule.

(Personally I am not against declaring variables in the middle of
blocks, I think it makes the code more readable, and it is perfectly
valid C99 code.)

Reviewed-by: Eduardo Habkost 


> +for (i = 0; i < nb_numa_nodes; i++) {
> +numa_total += node_mem[i];
> +}
> +if (numa_total != ram_size) {
> +fprintf(stderr, "qemu: numa nodes total memory size "
> +"should equal ram_size\n");
> +exit(1);
> +}
> +
>  for (i = 0; i < nb_numa_nodes; i++) {
>  if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
>  break;
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH target-arm v2 0/7] PL330 Changes

[Peter Maydell]

On 25 February 2014 23:56, Peter Crosthwaite
 wrote:
> Hi Peter,
>
> Some minor fixups and tweaks to PL330.
>
> changed since v1:
> Addressed PMM review.

Thanks; applied to target-arm.next.

-- PMM

[Qemu-devel] [PATCH 1/4] s390: Storage key global access

[Jason J. Herne]

From: "Jason J. Herne" 

Introduces global access to storage key data so we can set it for each cpu in
the S390 cpu initialization routine.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio-ccw.c | 3 +--
 hw/s390x/s390-virtio.c | 6 +++---
 hw/s390x/s390-virtio.h | 2 +-
 target-s390x/cpu.h | 3 +++
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 733d988..62319b9 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -80,7 +80,6 @@ static void ccw_init(QEMUMachineInitArgs *args)
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 int shift = 0;
-uint8_t *storage_keys;
 int ret;
 VirtualCssBus *css_bus;
 
@@ -112,7 +111,7 @@ static void ccw_init(QEMUMachineInitArgs *args)
 storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
 
 /* init CPUs */
-s390_init_cpus(args->cpu_model, storage_keys);
+s390_init_cpus(args->cpu_model);
 
 if (kvm_enabled()) {
 kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 9eeda97..00807e7 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -53,6 +53,7 @@
 
 static VirtIOS390Bus *s390_bus;
 static S390CPU **ipi_states;
+uint8_t *storage_keys;
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
@@ -176,7 +177,7 @@ void s390_init_ipl_dev(const char *kernel_filename,
 qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
+void s390_init_cpus(const char *cpu_model)
 {
 int i;
 
@@ -231,7 +232,6 @@ static void s390_init(QEMUMachineInitArgs *args)
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 int shift = 0;
-uint8_t *storage_keys;
 void *virtio_region;
 hwaddr virtio_region_len;
 hwaddr virtio_region_start;
@@ -273,7 +273,7 @@ static void s390_init(QEMUMachineInitArgs *args)
 storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
 
 /* init CPUs */
-s390_init_cpus(args->cpu_model, storage_keys);
+s390_init_cpus(args->cpu_model);
 
 /* Create VirtIO network adapters */
 s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index 5c405e7..c1cb042 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -20,7 +20,7 @@
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys);
+void s390_init_cpus(const char *cpu_model);
 void s390_init_ipl_dev(const char *kernel_filename,
const char *kernel_cmdline,
const char *initrd_filename,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 96c2b4a..1f4ca4f 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -369,6 +369,9 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
*cpu, int type,
 {
 }
 #endif
+
+extern uint8_t *storage_keys;
+
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v4] target-sparc: Add and use CPU_FEATURE_CASA

[Peter Maydell]

On 26 February 2014 16:59, Fabien Chouteau  wrote:
> On 02/26/2014 08:56 AM, Sebastian Huber wrote:
>> Hello,
>>
>> exists there someone who is able to commit this?
>>
>
> I'm sorry Sebastian, as you probably understood the SPARC maintainer is
> missing which makes commit process more difficult that it used to be...
>
> Peter, as discussed in a previous mail, do you agree to apply SPARC
> patches that have been reviewed?

My preference is for one of the people who does care about SPARC
to take on the job of assembling these patches into pull requests,
testing them, checking they're not obviously broken or lunatic, etc.
Mark Cave-Ayland has been doing this recently, so I would prefer
this patch to go via his tree if he is happy with it and willing to take it.

thanks
-- PMM

[Qemu-devel] [PATCH 4/4] s390-hotplug: Implement hot_add_cpu hook

[Jason J. Herne]

From: "Jason J. Herne" 

Implement hot_add_cpu for S390 to allow hot plugging of cpus.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio-ccw.c |  1 +
 target-s390x/cpu.c | 29 +
 target-s390x/cpu.h |  2 ++
 3 files changed, 32 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 62319b9..8b89886 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -132,6 +132,7 @@ static QEMUMachine ccw_machine = {
 .alias = "s390-ccw",
 .desc = "VirtIO-ccw based S390 machine",
 .init = ccw_init,
+.hot_add_cpu = ccw_hot_add_cpu,
 .block_default_type = IF_VIRTIO,
 .no_cdrom = 1,
 .no_floppy = 1,
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index a6fa82c..fd667d9 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -27,6 +27,8 @@
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "hw/hw.h"
+#include "hw/s390x/sclp.h"
+#include "sysemu/sysemu.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
 #endif
@@ -206,6 +208,33 @@ static void s390_cpu_finalize(Object *obj)
 #endif
 }
 
+#if !defined(CONFIG_USER_ONLY)
+void ccw_hot_add_cpu(const int64_t id, Error **errp)
+{
+S390CPU *new_cpu;
+CPUState *cpu;
+int cpu_count = 0;
+
+CPU_FOREACH(cpu) {
+cpu_count++;
+}
+
+if (cpu_count == max_cpus) {
+error_setg(errp, "Maximum number of cpus already defined");
+return;
+}
+
+if (id != next_cpu_num) {
+error_setg(errp, "Unable to add CPU: %" PRIi64
+   ", The next available id is %d", id, next_cpu_num);
+return;
+}
+
+new_cpu = S390_CPU(object_new(TYPE_S390_CPU));
+object_property_set_bool(OBJECT(new_cpu), true, "realized", NULL);
+}
+#endif
+
 static const VMStateDescription vmstate_s390_cpu = {
 .name = "cpu",
 .unmigratable = 1,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 79f47b4..af199e5 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -377,6 +377,8 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
 
+void ccw_hot_add_cpu(const int64_t id, Error **errp);
+
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size

[Paolo Bonzini]

Il 26/02/2014 18:15, Eduardo Habkost ha scritto:

On Mon, Feb 17, 2014 at 04:49:44PM +0100, Igor Mammedov wrote:

From: Wanlong Gao 

If the total number of the assigned numa nodes memory is not
equal to the assigned ram size, it will write the wrong data
to ACPI table, then the guest will ignore the wrong ACPI table
and recognize all memory to one node. It's buggy, we should
check it to ensure that we write the right data to ACPI table.

Signed-off-by: Wanlong Gao 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Igor Mammedov 
---
 numa.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/numa.c b/numa.c
index 7845036..d12a4f2 100644
--- a/numa.c
+++ b/numa.c
@@ -151,6 +151,16 @@ void set_numa_nodes(void)
 node_mem[i] = ram_size - usedmem;
 }

+uint64_t numa_total = 0;


I was going to point out that variable declarations in the middle of
blocks goes against coding style (at least I was told so), but my patch
to amend CODING_STYLE to document it was ignored for 2 weeks, already.
So, I am not sure we really have that rule.

(Personally I am not against declaring variables in the middle of
blocks, I think it makes the code more readable, and it is perfectly
valid C99 code.)

Reviewed-by: Eduardo Habkost 


I agree, but I fixed it already.

Paolo