Re: [Qemu-devel] [PATCH RFC v2 00/12] basic channel IO passthrough infrastructure based on vfio

2017-01-12 Thread no-reply
Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20170112072513.98411-1-bjsdj...@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH RFC v2 00/12] basic channel IO passthrough 
infrastructure based on vfio

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20170112072513.98411-1-bjsdj...@linux.vnet.ibm.com 
-> patchew/20170112072513.98411-1-bjsdj...@linux.vnet.ibm.com
Switched to a new branch 'test'
694b0ee vfio/ccw: update sense data if a unit check is pending
4afcfb2 s390x/css: ccws translation infrastructure
cba9095 s390x/css: introduce and realize ccw-request callback
4f8d64a vfio/ccw: get irqs info and set the eventfd fd
96bc1b5 vfio/ccw: get io region info
90691ec vfio/ccw: vfio based subchannel passthrough driver
b40a567 s390x/css: device support for s390-ccw passthrough
0d68f21 s390x/css: realize css_create_sch
01179e5 s390x/css: realize css_sch_build_schib
1564f88 s390x/css: add s390-map-css machine option
79b5e81 vfio: linux-headers update for vfio-ccw
52b52ad update-linux-headers: add asm-s390/vfio_ccw.h

=== OUTPUT BEGIN ===
Checking PATCH 1/12: update-linux-headers: add asm-s390/vfio_ccw.h...
ERROR: line over 90 characters
#19: FILE: scripts/update-linux-headers.sh:94:
+cp_portable "$tmpdir/include/asm/vfio_ccw.h" 
"$output/include/standard-headers/asm-s390/"

total: 1 errors, 0 warnings, 7 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/12: vfio: linux-headers update for vfio-ccw...
Checking PATCH 3/12: s390x/css: add s390-map-css machine option...
Checking PATCH 4/12: s390x/css: realize css_sch_build_schib...
WARNING: line over 80 characters
#129: FILE: hw/s390x/css.c:1980:
+static int css_sch_get_chpid_type(uint8_t chpid, uint32_t *type, CssDevId 
*dev_id)

total: 0 errors, 1 warnings, 233 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/12: s390x/css: realize css_create_sch...
Checking PATCH 6/12: s390x/css: device support for s390-ccw passthrough...
Checking PATCH 7/12: vfio/ccw: vfio based subchannel passthrough driver...
Checking PATCH 8/12: vfio/ccw: get io region info...
Checking PATCH 9/12: vfio/ccw: get irqs info and set the eventfd fd...
Checking PATCH 10/12: s390x/css: introduce and realize ccw-request callback...
ERROR: space required after that ',' (ctx:VxV)
#81: FILE: hw/vfio/ccw.c:73:
+error_report("vfio-ccw: wirte I/O region failed with errno=%d",errno);
   ^

total: 1 errors, 0 warnings, 126 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 11/12: s390x/css: ccws translation infrastructure...
Checking PATCH 12/12: vfio/ccw: update sense data if a unit check is pending...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [RFC]virtio-blk: add disk-name device property

2017-01-12 Thread Fam Zheng
On Thu, 01/12 15:39, Yang Zhang wrote:
> On 2017/1/12 10:22, Fam Zheng wrote:
> > On Thu, 01/12 09:22, Yang Zhang wrote:
> > > On 2017/1/4 22:44, Stefan Hajnoczi wrote:
> > > > On Tue, Jan 03, 2017 at 10:53:06AM -0600, Eric Blake wrote:
> > > > > On 12/29/2016 08:41 PM, Junkang Fu wrote:
> > > > > > >From 74e913fc41ea98d1dde692175f1e3fb6729342aa Mon Sep 17 00:00:00 
> > > > > > >2001
> > > > > > From: "junkang.fjk" 
> > > > > > Date: Wed, 24 Aug 2016 19:36:53 +0800
> > > > > > Subject: [PATCH] virtio-blk: add disk-name device property
> > > > > > 
> > > > > > Current virtio-blk disk name(ex. /dev/vdb) has nothing to do with 
> > > > > > the
> > > > > > target dev
> > > > > > name specified in libvirt xml file. For example, we may get disk 
> > > > > > name
> > > > > > /dev/vdb in
> > > > > > VM while target dev specified in libvirt xml is vdc.
> > > > > 
> > > > > It's not really libvirt's fault.  The libvirt XML names are for
> > > > > convenience, but nothing on the host side requires the guest to pick 
> > > > > the
> > > > > same naming scheme as the host.
> > > > > 
> > > > > I guess your proposal is to enhance the virtio spec such that clients
> > > > > that are new enough to honor the new addition to the virtio spec will
> > > > > change their name-picking algorithm to use the name provided by the
> > > > > host, rather than their current approach of picking whatever name they
> > > > > feel like, and then enhance libvirt to pass the XML name on down to 
> > > > > the
> > > > > guest?  It might work, but as others have pointed out, it will 
> > > > > require a
> > > > > virtio spec change first.
> > > > 
> > > > This change is unnecessary.  The -device virtio-blk-pci,serial= property
> > > > already exists for this purpose.
> > > 
> > > how about the /dev/vdabc? I guess lots of people prefer to use it instead 
> > > of
> > > /dev/disk/by-id/xxx?
> > 
> > I disagree. Using /dev/sdX has exactly the same issue and that's why fstab 
> > and
> > boot loader etc almost always use UUID or disk label by default because 
> > they are
> > more stable.
> 
> I mean does it also change the /dev/sdX to the name specified in serial=sdX
> or it just show the name under /dev/disk/by-id/

I was saying on real hardware, sata disks can have unstable /dev/sdX naming
across reboots if you add or remove disks on the controller, or add or remove
HBA.

Fam



Re: [Qemu-devel] qemu-2.8-rc4 is broken

2017-01-12 Thread Pavel Dovgalyuk
> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> >> From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
> >> >
> >> > Yes, this option helps.
> >> > Thank you.
> >>
> >> Good news.  This can be fixed in 2.8.1 once someone finds a solution.
> >
> > It seems that something still goes wrong.
> > I'm using this workaround, but there is a kind of deadlock in translation.
> > call_rcu_thread hangs at some moment in qemu_event_wait.
> >
> > As far as I understand, it is used by QHT in translate-all.c.
> > I can't get more information yet, because logging makes everything too slow.
> 
> There are a number of users of RCU bit for QHT I think it only gets
> activated when it needs to re-size its hash table on insertion of new
> TranslationBlocks.
> 
> Can you get a backtrace of all threads when it deadlocks?

Sorry, this is another problem which occurs only in icount replay mode:
1. cpu_handle_exception tries to force exception when is cannot occur due to
   running out all the planned instructions:
} else if (replay_has_exception()
   && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
/* try to cause an exception pending in the log */
cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
*ret = -1;
return true;

2. tb_find calls tb_gen_code, which cannot allocate new translation block 
   and calls tb_flush (which only queues the flushing) and cpu_loop_exit
3. cpu_loop_exit returns to infinite loop of cpu_exec and the condition
if (cpu_handle_exception(cpu, &ret)) {
break;
}
   is checked again causing an infinite loop.

TB cache is not flushed because we never execute that break and real work of 
tb_flush
is made outside this loop.

Pavel Dovgalyuk




Re: [Qemu-devel] [PATCH] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Markus Armbruster
Thomas Huth  writes:

> When running certain HMP commands ("info registers", "info cpustats"
> or dumping virtual memory) with the "none" machine, QEMU crashes
> with a segmentation fault. This happens because the "none" machine does
> not have any CPUs by default,

"Sachen gibt's!"

>   but these HMP commands did not check for
> a valid CPU pointer yet. Add such a check now and print a message
> about the missing CPU instead.

Have you checked uses of first_cpu elsewhere?  Out of scope for this
patch, of course.

> Signed-off-by: Thomas Huth 
> ---
>  monitor.c | 29 +
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 0841d43..0103979 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
>  CPUState *mon_get_cpu(void)
>  {
>  if (!cur_mon->mon_cpu) {
> +if (!first_cpu) {
> +return NULL;
> +}
>  monitor_set_cpu(first_cpu->cpu_index);
>  }
>  cpu_synchronize_state(cur_mon->mon_cpu);

Why are the following dereferences safe?

   CPUArchState *mon_get_cpu_env(void)
   {
   return mon_get_cpu()->env_ptr;
   }

   int monitor_get_cpu_index(void)
   {
   return mon_get_cpu()->cpu_index;
   }

> @@ -1043,7 +1046,13 @@ int monitor_get_cpu_index(void)
>  
>  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>  {
> -cpu_dump_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, 
> CPU_DUMP_FPU);
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs) {
> +monitor_printf(mon, "No CPU available\n");
> +return;
> +}
> +cpu_dump_state(cs, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
>  }
>  
>  static void hmp_info_jit(Monitor *mon, const QDict *qdict)
> @@ -1076,7 +1085,13 @@ static void hmp_info_history(Monitor *mon, const QDict 
> *qdict)
>  
>  static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>  {
> -cpu_dump_statistics(mon_get_cpu(), (FILE *)mon, &monitor_fprintf, 0);
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs) {
> +monitor_printf(mon, "No CPU available\n");
> +return;
> +}
> +cpu_dump_statistics(cs, (FILE *)mon, &monitor_fprintf, 0);
>  }
>  
>  static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
> @@ -1235,6 +1250,12 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  int l, line_size, i, max_digits, len;
>  uint8_t buf[16];
>  uint64_t v;
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs && (format == 'i' || !is_physical)) {
> +monitor_printf(mon, "Can not dump without CPU\n");
> +return;
> +}
>  
>  if (format == 'i') {
>  int flags = 0;
> @@ -1264,7 +1285,7 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  flags = msr_le << 16;
>  flags |= env->bfd_mach;
>  #endif
> -monitor_disas(mon, mon_get_cpu(), addr, count, is_physical, flags);
> +monitor_disas(mon, cs, addr, count, is_physical, flags);
>  return;
>  }
>  
> @@ -1303,7 +1324,7 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  if (is_physical) {
>  cpu_physical_memory_read(addr, buf, l);
>  } else {
> -if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) {
> +if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
>  monitor_printf(mon, " Cannot access memory\n");
>  break;
>  }



Re: [Qemu-devel] [PATCH] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Thomas Huth
On 12.01.2017 09:10, Markus Armbruster wrote:
> Thomas Huth  writes:
> 
>> When running certain HMP commands ("info registers", "info cpustats"
>> or dumping virtual memory) with the "none" machine, QEMU crashes
>> with a segmentation fault. This happens because the "none" machine does
>> not have any CPUs by default,
> 
> "Sachen gibt's!"
> 
>>   but these HMP commands did not check for
>> a valid CPU pointer yet. Add such a check now and print a message
>> about the missing CPU instead.
> 
> Have you checked uses of first_cpu elsewhere?  Out of scope for this
> patch, of course.

I only looked at monitor.c so far, and that's the only spot that uses
this variable there.

But it seems like gdbstub.c has the same bug, too. If I start the "none"
machine and attach a remote gdb, QEMU segfaults here, too.
I've put this on my TODO-list... (I think it should be fixed with a
separate patch).

>> Signed-off-by: Thomas Huth 
>> ---
>>  monitor.c | 29 +
>>  1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 0841d43..0103979 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
>>  CPUState *mon_get_cpu(void)
>>  {
>>  if (!cur_mon->mon_cpu) {
>> +if (!first_cpu) {
>> +return NULL;
>> +}
>>  monitor_set_cpu(first_cpu->cpu_index);
>>  }
>>  cpu_synchronize_state(cur_mon->mon_cpu);
> 
> Why are the following dereferences safe?
> 
>CPUArchState *mon_get_cpu_env(void)
>{
>return mon_get_cpu()->env_ptr;
>}
> 
>int monitor_get_cpu_index(void)
>{
>return mon_get_cpu()->cpu_index;
>}

Oh, they are apparently not safe either. The HMP commands "nmi" and
"memsave", which use these functions, are crashing on the "none"
machine, too... I'll send a v2 of my patch to fix these, too ...

Thanks for the review!

 Thomas




[Qemu-devel] [PATCH] ppc/prep: update MAINTAINERS file

2017-01-12 Thread Hervé Poussineau
Signed-off-by: Hervé Poussineau 
---
 MAINTAINERS | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1444b26..f4b02ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -657,10 +657,13 @@ F: hw/misc/macio/
 F: hw/intc/heathrow_pic.c
 
 PReP
+M: Hervé Poussineau 
 L: qemu-devel@nongnu.org
 L: qemu-...@nongnu.org
-S: Odd Fixes
+S: Maintained
 F: hw/ppc/prep.c
+F: hw/ppc/prep_systemio.c
+F: hw/ppc/rs6000_mc.c
 F: hw/pci-host/prep.[hc]
 F: hw/isa/pc87312.[hc]
 F: pc-bios/ppc_rom.bin
-- 
2.1.4




Re: [Qemu-devel] [PATCH v2 02/47] trace: switch io/ directory to modular trace.h file

2017-01-12 Thread Paolo Bonzini


On 12/01/2017 02:02, Lluís Vilanova wrote:
> Paolo Bonzini writes:
> [...]
>> A weird idea: what about doing
> 
>> -DGENERATED_TRACERS_H=\"hw/scsi/generated-tracers.h\"
> 
>> and then having
> 
>>  #ifdef GENERATED_TRACE_H
>>  #include GENERATED_TRACE_H
>>  #endif
> 
>> in include/trace.h?
> 
>> Then you can use full include path for special cases such as
>> include/hw/xen/xen_common.h, but the common case is handled directly
>> with just
> 
>>  #include "trace.h"
> 
>> which refers to $(srcdir)/include/trace.h? (Take the above with a grain
>> of salt because I haven't reviewed the patches closely).
> 
> Feels like too much black magic to me for the benefit of a bit less typing.

It's not less typing, it's about consistency.   Includes from the
current directory are currently included with no path.

Paolo



Re: [Qemu-devel] [PATCH v2 1/2] memory: provide common macros for mtree_print_mr()

2017-01-12 Thread Paolo Bonzini


On 12/01/2017 06:50, Peter Xu wrote:
> On Wed, Jan 11, 2017 at 06:21:46PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 21/12/2016 08:58, Peter Xu wrote:
>>> -   mr->romd_mode ? 'R' : '-',
>>> -   !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 
>>> 'W'
>>> -   : 
>>> '-',
>>> +   MR_CHAR_RD(mr),
>>> +   MR_CHAR_WR(mr),
>>
>> An alternative definition could be
>>
>>  memory_access_is_direct(mr, false) ? 'R' : '-'
>>  memory_access_is_direct(mr, true) ? 'W' : '-'
>>
>> for MR_CHAR_RD and MR_CHAR_WR.  With this change, I think the small code
>> duplication in the "? :" operator is tolerable and the code is clearer.
> 
> memory_access_is_direct() will check against whether mr is RAM, is
> that what we want here? In that case we'll get most of the regions as
> "--" as long as they are not RAM, while in fact IMHO we should want to
> know the rw permission for all cases.

Indeed.  My idea was that the RW permission is not well defined for
non-RAM memory regions, and ROMD regions in MMIO mode shows as "--"
while MMIO regions show as "RW".  But perhaps it's confusing.

What about writing one of "ram", "rom", "ramd", "romd", "i/o" (with I/O
also covering rom_device && !romd_mode)?

If you disagree, the below patch looks good.

Paolo

> 8<
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index bec9756..50974c8 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1619,14 +1619,27 @@ MemTxResult address_space_read_full(AddressSpace *as, 
> hwaddr addr,
>  MemTxAttrs attrs, uint8_t *buf, int len);
>  void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
> 
> +static inline bool memory_region_is_readable(MemoryRegion *mr)
> +{
> +return mr->rom_device ? mr->romd_mode : true;
> +}
> +
> +static inline bool memory_region_is_writable(MemoryRegion *mr)
> +{
> +return !mr->rom_device && !mr->readonly;
> +}
> +
> +static inline bool memory_region_is_direct(MemoryRegion *mr)
> +{
> +return memory_region_is_ram(mr) && !memory_region_is_ram_device(mr);
> +}
> +
>  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>  {
>  if (is_write) {
> -return memory_region_is_ram(mr) &&
> -   !mr->readonly && !memory_region_is_ram_device(mr);
> +return memory_region_is_direct(mr) && memory_region_is_writable(mr);
>  } else {
> -return (memory_region_is_ram(mr) && 
> !memory_region_is_ram_device(mr)) ||
> -   memory_region_is_romd(mr);
> +return memory_region_is_direct(mr) && memory_region_is_readable(mr);
>  }
>  }
> >8
> 
> Then, I can throw away MR_CHAR_* macros and use:
> 
> memory_access_is_readable(mr, false) ? 'R' : '-'
> memory_access_is_writable(mr, true) ? 'W' : '-'
> 
> Do you like this approach?
> 
> -- peterx
> 
> 



[Qemu-devel] [Bug 1268671] Re: CentOS guest crashing due to assertion failure in qemu-char.c

2017-01-12 Thread Paolo Bonzini
It's a glib bug.  It's fixed in RHEL 6.9 beta and in due time the fix
will get to CentOS.

** Changed in: qemu
   Status: Incomplete => Invalid

** Bug watch added: Red Hat Bugzilla #1212722
   https://bugzilla.redhat.com/show_bug.cgi?id=1212722

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

Title:
  CentOS guest crashing due to assertion failure in qemu-char.c

Status in QEMU:
  Invalid

Bug description:
  Here is the log in /var/log/libvirt/qemu/centos_heavy.log

  qemu-kvm: /builddir/build/BUILD/qemu-kvm-0.12.1.2/qemu-char.c:630: 
io_watch_poll_finalize: Assertion `iwp->src == ((void *)0)' failed.
  2014-01-13 16:50:31.576+: shutting down

  The code it's failing the assertion on has an interesting comment:

  static void io_watch_poll_finalize(GSource *source)
  {
  /* Due to a glib bug, removing the last reference to a source
  * inside a finalize callback causes recursive locking (and a
  * deadlock). This is not a problem inside other callbacks,
  * including dispatch callbacks, so we call io_remove_watch_poll
  * to remove this source. A t this point, iwp->src must
  * be NULL, or we would leak it.
  *
  * This would be solved much more elegantly by child sources,
  * but we support older glib versions that do not have them.
  */
  IOWatchPoll *iwp = io_watch_poll_from_source(source);
  assert(iwp->src == NULL);
  }

  --
  CPU Info:

  http://pastebin.com/U7MrzFxK

  

  Relevant RPM versions:

  qemu-kvm-0.12.1.2-2.415.el6_5.3.x86_64
  libvirt-0.10.2-29.el6_5.2.x86_64

  

  Domain config:

  http://pastebin.com/Nf2VsER8

  (Note the use of the vmchannels; I believe this is playing a part in
  this crash)

  -

  uname -a:

  Linux blizzard 2.6.32-431.3.1.el6.x86_64 #1 SMP Fri Jan 3 21:39:27 UTC
  2014 x86_64 x86_64 x86_64 GNU/Linux

  -

  CLI to start guest (included in attached dump):

  http://pastebin.com/W01Xzyb0

  -

  thread apply all bt:

  http://pastebin.com/FTpUDU7A

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1268671/+subscriptions



[Qemu-devel] [Bug 1268671] Re: CentOS guest crashing due to assertion failure in qemu-char.c

2017-01-12 Thread Paolo Bonzini
https://bugzilla.redhat.com/show_bug.cgi?id=1212722

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

Title:
  CentOS guest crashing due to assertion failure in qemu-char.c

Status in QEMU:
  Invalid

Bug description:
  Here is the log in /var/log/libvirt/qemu/centos_heavy.log

  qemu-kvm: /builddir/build/BUILD/qemu-kvm-0.12.1.2/qemu-char.c:630: 
io_watch_poll_finalize: Assertion `iwp->src == ((void *)0)' failed.
  2014-01-13 16:50:31.576+: shutting down

  The code it's failing the assertion on has an interesting comment:

  static void io_watch_poll_finalize(GSource *source)
  {
  /* Due to a glib bug, removing the last reference to a source
  * inside a finalize callback causes recursive locking (and a
  * deadlock). This is not a problem inside other callbacks,
  * including dispatch callbacks, so we call io_remove_watch_poll
  * to remove this source. A t this point, iwp->src must
  * be NULL, or we would leak it.
  *
  * This would be solved much more elegantly by child sources,
  * but we support older glib versions that do not have them.
  */
  IOWatchPoll *iwp = io_watch_poll_from_source(source);
  assert(iwp->src == NULL);
  }

  --
  CPU Info:

  http://pastebin.com/U7MrzFxK

  

  Relevant RPM versions:

  qemu-kvm-0.12.1.2-2.415.el6_5.3.x86_64
  libvirt-0.10.2-29.el6_5.2.x86_64

  

  Domain config:

  http://pastebin.com/Nf2VsER8

  (Note the use of the vmchannels; I believe this is playing a part in
  this crash)

  -

  uname -a:

  Linux blizzard 2.6.32-431.3.1.el6.x86_64 #1 SMP Fri Jan 3 21:39:27 UTC
  2014 x86_64 x86_64 x86_64 GNU/Linux

  -

  CLI to start guest (included in attached dump):

  http://pastebin.com/W01Xzyb0

  -

  thread apply all bt:

  http://pastebin.com/FTpUDU7A

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1268671/+subscriptions



[Qemu-devel] [Bug 622367] Re: No BIOS MPFP structure with smp=92 and more

2017-01-12 Thread Vic3Dexe
Man, really? xD
6 years have passed...
Close the ticket please, I don't have this code anymore.
...
still laughing... sorry

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

Title:
  No BIOS MPFP structure with smp=92 and more

Status in QEMU:
  Incomplete

Bug description:
  qemu 0.12.2, SeaBios 0.5.1, running qemu-system-x86_64.exe with option -smp.
  If smp>=92 then no MP floating point structure present in 1 Mb. This may be 
verified by pmemsave 0 0x10 in debugger and search for _MP_ signature in 
file.

  qemu 0.10.5 (bios build 05/08/09) can smp=128 (and even 255 if not
  hangs :).

  Host win 7 x64 RTM 7600.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/622367/+subscriptions



Re: [Qemu-devel] [PATCH v2 1/2] memory: provide common macros for mtree_print_mr()

2017-01-12 Thread Peter Xu
On Thu, Jan 12, 2017 at 09:54:24AM +0100, Paolo Bonzini wrote:
> 
> 
> On 12/01/2017 06:50, Peter Xu wrote:
> > On Wed, Jan 11, 2017 at 06:21:46PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 21/12/2016 08:58, Peter Xu wrote:
> >>> -   mr->romd_mode ? 'R' : '-',
> >>> -   !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 
> >>> 'W'
> >>> -   : 
> >>> '-',
> >>> +   MR_CHAR_RD(mr),
> >>> +   MR_CHAR_WR(mr),
> >>
> >> An alternative definition could be
> >>
> >>memory_access_is_direct(mr, false) ? 'R' : '-'
> >>memory_access_is_direct(mr, true) ? 'W' : '-'
> >>
> >> for MR_CHAR_RD and MR_CHAR_WR.  With this change, I think the small code
> >> duplication in the "? :" operator is tolerable and the code is clearer.
> > 
> > memory_access_is_direct() will check against whether mr is RAM, is
> > that what we want here? In that case we'll get most of the regions as
> > "--" as long as they are not RAM, while in fact IMHO we should want to
> > know the rw permission for all cases.
> 
> Indeed.  My idea was that the RW permission is not well defined for
> non-RAM memory regions, and ROMD regions in MMIO mode shows as "--"
> while MMIO regions show as "RW".  But perhaps it's confusing.
> 
> What about writing one of "ram", "rom", "ramd", "romd", "i/o" (with I/O
> also covering rom_device && !romd_mode)?
> 
> If you disagree, the below patch looks good.

Yes, above suggestion makes sense to me, since after all the RW
permissions are derived from the type of memory regions, and the type
itself tells more things than the RW bits. So I totally agree we can
replace the "RW" chars with its type directly (if no one else
disagree, of course).

While for below patch, do you want me to include it as well as a
standalone patch, for the purpose of refactoring
memory_access_is_direct()? Since IMHO it's tiny clearer and more
readable than before.

Thanks!

> 
> Paolo
> 
> > 8<
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index bec9756..50974c8 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1619,14 +1619,27 @@ MemTxResult address_space_read_full(AddressSpace 
> > *as, hwaddr addr,
> >  MemTxAttrs attrs, uint8_t *buf, int 
> > len);
> >  void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
> > 
> > +static inline bool memory_region_is_readable(MemoryRegion *mr)
> > +{
> > +return mr->rom_device ? mr->romd_mode : true;
> > +}
> > +
> > +static inline bool memory_region_is_writable(MemoryRegion *mr)
> > +{
> > +return !mr->rom_device && !mr->readonly;
> > +}
> > +
> > +static inline bool memory_region_is_direct(MemoryRegion *mr)
> > +{
> > +return memory_region_is_ram(mr) && !memory_region_is_ram_device(mr);
> > +}
> > +
> >  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
> >  {
> >  if (is_write) {
> > -return memory_region_is_ram(mr) &&
> > -   !mr->readonly && !memory_region_is_ram_device(mr);
> > +return memory_region_is_direct(mr) && 
> > memory_region_is_writable(mr);
> >  } else {
> > -return (memory_region_is_ram(mr) && 
> > !memory_region_is_ram_device(mr)) ||
> > -   memory_region_is_romd(mr);
> > +return memory_region_is_direct(mr) && 
> > memory_region_is_readable(mr);
> >  }
> >  }
> > >8
> > 
> > Then, I can throw away MR_CHAR_* macros and use:
> > 
> > memory_access_is_readable(mr, false) ? 'R' : '-'
> > memory_access_is_writable(mr, true) ? 'W' : '-'
> > 
> > Do you like this approach?
> > 
> > -- peterx
> > 
> > 

-- peterx



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 2/3] run_tests: put logs into per-test file

2017-01-12 Thread Andrew Jones
On Thu, Jan 12, 2017 at 11:36:21AM +0800, Peter Xu wrote:
> We were using test.log before to keep all the test logs. This patch
> creates one log file per test case under logs/ directory with name
> "TESTNAME.log". Meanwhile, we will keep the last time log into
> logs.old/.
> 
> Renaming scripts/functions.bash into scripts/common.bash to store some
> more global variables.
> 
> Signed-off-by: Peter Xu 
> ---
>  .gitignore  |  3 ++-
>  Makefile|  5 ++---
>  run_tests.sh| 19 ---
>  scripts/{functions.bash => common.bash} | 13 +++--
>  scripts/mkstandalone.sh |  2 +-
>  5 files changed, 28 insertions(+), 14 deletions(-)
>  rename scripts/{functions.bash => common.bash} (75%)
> 
> diff --git a/.gitignore b/.gitignore
> index 3155418..2213b9b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -12,7 +12,8 @@ cscope.*
>  /lib/asm
>  /config.mak
>  /*-run
> -/test.log
>  /msr.out
>  /tests
>  /build-head
> +/logs/
> +/logs.old/
> diff --git a/Makefile b/Makefile
> index a32333b..844bacc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,9 +94,8 @@ libfdt_clean:
>   $(LIBFDT_objdir)/.*.d
>  
>  distclean: clean libfdt_clean
> - $(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> -   build-head
> - $(RM) -r tests
> + $(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> + $(RM) -r tests logs logs.old
>  
>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) 
> lib/asm-generic
>  cscope:
> diff --git a/run_tests.sh b/run_tests.sh
> index 2cfa365..afd3d95 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
>  exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  function usage()
>  {
> @@ -46,17 +46,22 @@ while getopts "g:hv" opt; do
>  esac
>  done
>  
> -RUNTIME_log_stderr () { cat >> test.log; }
> +# RUNTIME_log_file will be configured later
> +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
>  RUNTIME_log_stdout () {
>  if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> -./scripts/pretty_print_stacks.py $1 >> test.log
> +./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
>  else
> -cat >> test.log
> +cat >> $RUNTIME_log_file
>  fi
>  }
>  
> -
>  config=$TEST_DIR/unittests.cfg
> -rm -f test.log
> -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> +
> +rm -rf $unittest_log_dir.old
> +[ -d $unittest_log_dir ] && mv $unittest_log_dir $unittest_log_dir.old
> +mkdir $unittest_log_dir || exit 2
> +
> +echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
> +

nit: to be 100% correct all the references to the log dir should have
"'s around them in order to handle spaces, or other shell ambiguous
characters, in the name. For example, mkdir $log_dir, where $log_dir
is "my tests" would create two directories, "my" and "tests" without
the quotes. Anyway, I'm not too worried about someone choosing a
weird log dir name, nor even changing it from the default.

Isn't bash fun :-)

>  for_each_unittest $config run
> diff --git a/scripts/functions.bash b/scripts/common.bash
> similarity index 75%
> rename from scripts/functions.bash
> rename to scripts/common.bash
> index ee9143c..2dd7360 100644
> --- a/scripts/functions.bash
> +++ b/scripts/common.bash
> @@ -1,3 +1,12 @@
> +: ${unittest_log_dir:=logs}
> +
> +function run_task()
> +{
> + local testname="$2"
> +
> + RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> + "$@"
> +}
>  
>  function for_each_unittest()
>  {
> @@ -17,7 +26,7 @@ function for_each_unittest()
>  
>   while read -u $fd line; do
>   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" 
> "$arch" "$check" "$accel" "$timeout"
> + run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> "$opts" "$arch" "$check" "$accel" "$timeout"
>   testname=${BASH_REMATCH[1]}
>   smp=1
>   kernel=""
> @@ -45,6 +54,6 @@ function for_each_unittest()
>   timeout=${BASH_REMATCH[1]}
>   fi
>   done
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
> "$accel" "$timeout"
> + run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
> "$check" "$accel" "$timeout"
>   exec {fd}<&-
>  }
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index d2bae19..3c1938e 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
>   exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  escape ()
>  {
> -- 
> 2.7.4
> 
>

Reviewed-by: Andrew Jones 



Re: [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files

2017-01-12 Thread Laszlo Ersek
On 01/12/17 03:25, Gabriel L. Somlo wrote:
> On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote:
>> This is the first (fw_cfg) half of the v5 iteration of the series posted
>> here:
>> .
>>
>> In this version, the fw_cfg patches have been separated into a
>> standalone "wave", for helping review / maintenance, and also for
>> enabling independent features on top of writeable blobs. More
>> importantly, I've addressed Igor's v4 feedback. See the individual
>> patches for the details.
>>
>> Patch #3 is included verbatim from Eduardo's pending series (see the
>> patch notes for the archive URL), as a dependency for patch #4. If
>> Eduardo's series is merged first, patch #3 can be dropped (in fact
>> git-rebase should do it automatically).
>>
>> Please excuse the surprisingly long list of CC's, it's due to the fact
>> that fw_cfg is quite widely used (see patch #4).
>>
>> Cc: "Gabriel L. Somlo" 
> 
> Whole series:
> 
> Acked-by: Gabriel Somlo 
> 
> Data passed in via the "-fw_cfg" qemu command still shows up fine in
> /sys/firmware/qemu-fw-cfg on the guest, so also:
> 
> Tested-by: Gabriel Somlo 

Thank you!
Laszlo

> 
> Thanks,
> --Gabriel
> 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Alexander Graf 
>> Cc: Anthony Perard 
>> Cc: Artyom Tarasenko 
>> Cc: David Gibson 
>> Cc: Eduardo Habkost 
>> Cc: Gerd Hoffmann 
>> Cc: Igor Mammedov 
>> Cc: Laszlo Ersek 
>> Cc: Mark Cave-Ayland 
>> Cc: Michael Walle 
>> Cc: Paolo Bonzini 
>> Cc: Peter Maydell 
>> Cc: Shannon Zhao 
>> Cc: Stefano Stabellini 
>> Cc: qemu-...@nongnu.org
>>
>> Thanks
>> Laszlo
>>
>>
>> Eduardo Habkost (1):
>>   pc: Add 2.9 machine-types
>>
>> Laszlo Ersek (2):
>>   fw-cfg: turn FW_CFG_FILE_SLOTS into a device property
>>   fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types
>>
>> Michael S. Tsirkin (1):
>>   fw-cfg: support writeable blobs
>>
>>  docs/specs/fw_cfg.txt  |  36 ++
>>  hw/lm32/lm32_hwsetup.h |   2 +-
>>  include/hw/compat.h|  10 +++-
>>  include/hw/i386/pc.h   |   2 +
>>  include/hw/loader.h|   7 +--
>>  include/hw/nvram/fw_cfg.h  |   3 +-
>>  include/hw/nvram/fw_cfg_keys.h |   3 +-
>>  hw/arm/virt-acpi-build.c   |   2 +-
>>  hw/core/loader.c   |  18 ---
>>  hw/i386/acpi-build.c   |   4 +-
>>  hw/i386/pc_piix.c  |  15 --
>>  hw/i386/pc_q35.c   |  13 -
>>  hw/nvram/fw_cfg.c  | 110 
>> +++--
>>  13 files changed, 177 insertions(+), 48 deletions(-)
>>
>> -- 
>> 2.9.3
>>




Re: [Qemu-devel] [PATCH v7 20/21] build-sys: add txt documentation rules

2017-01-12 Thread Markus Armbruster
Marc-André Lureau  writes:

> Build plain text documentation, and install it.
>
> Signed-off-by: Marc-André Lureau 
> Reviewed-by: Markus Armbruster 
> ---
>  .gitignore |  1 +
>  Makefile   | 12 +---
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 6f175b391e..e16bddc070 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -40,6 +40,7 @@
>  /qmp-marshal.c
>  /qemu-doc.html
>  /qemu-doc.info
> +/qemu-doc.txt
>  /qemu-img
>  /qemu-nbd
>  /qemu-options.def
> diff --git a/Makefile b/Makefile
> index d18bac1c31..37d45ee21b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -81,7 +81,7 @@ Makefile: ;
>  configure: ;
>  
>  .PHONY: all clean cscope distclean html info install install-doc \
> - pdf recurse-all speed test dist msi FORCE
> + pdf txt recurse-all speed test dist msi FORCE
>  
>  $(call set-vpath, $(SRC_PATH))
>  
> @@ -90,7 +90,7 @@ LIBS+=-lz $(LIBS_TOOLS)
>  HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
>  
>  ifdef BUILD_DOCS
> -DOCS=qemu-doc.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
> +DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
>  ifdef CONFIG_VIRTFS
>  DOCS+=fsdev/virtfs-proxy-helper.1
>  endif
> @@ -431,6 +431,7 @@ endif
>  install-doc: $(DOCS)
>   $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
>   $(INSTALL_DATA) qemu-doc.html "$(DESTDIR)$(qemu_docdir)"
> + $(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)"
>  ifdef CONFIG_POSIX
>   $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
>   $(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1"
> @@ -538,6 +539,10 @@ TEXIFLAG=$(if $(V),,--quiet)
>  %.info: %.texi
>   $(call quiet-command,$(MAKEINFO) $(MAKEINFOFLAGS) $< -o $@,"GEN","$@")
>  
> +%.txt: %.texi
> + $(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
> \
> + --plaintext $< -o $@,"GEN   $@")

I believe this needs to be "GEN", "$@" now.  See commit 0bdb12c.

> +
>  %.pdf: %.texi
>   $(call quiet-command,texi2pdf $(TEXIFLAG) -I . $<,"GEN","$@")
>  
> @@ -563,6 +568,7 @@ qemu-ga.8: qemu-ga.texi
>  html: qemu-doc.html
>  info: qemu-doc.info
>  pdf: qemu-doc.pdf
> +txt: qemu-doc.txt
>  
>  qemu-doc.html qemu-doc.info qemu-doc.pdf: \
>   qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
> @@ -661,7 +667,7 @@ help:
>   @echo  '  docker  - Help about targets running tests inside 
> Docker containers'
>   @echo  ''
>   @echo  'Documentation targets:'
> - @echo  '  html info pdf'
> + @echo  '  html info pdf txt'
>   @echo  '  - Build documentation in specified format'
>   @echo  ''
>  ifdef CONFIG_WIN32



Re: [Qemu-devel] [PATCH] hw/block/m25p80: Fix typo in local macro name

2017-01-12 Thread Michael Tokarev
01.11.2016 20:03, Stefan Weil wrote:
> Signed-off-by: Stefan Weil 
> ---
>  hw/block/m25p80.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to -trivial, quite a bit too late but better be late
than never :)  Thanks!

/mjt



[Qemu-devel] [PATCH v2] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Thomas Huth
When running certain HMP commands ("info registers", "info cpustats",
"nmi", "memsave" or dumping virtual memory) with the "none" machine,
QEMU crashes with a segmentation fault. This happens because the "none"
machine does not have any CPUs by default, but these HMP commands did
not check for a valid CPU pointer yet. Add such checks now, so we get
an error message about the missing CPU instead.

Signed-off-by: Thomas Huth 
---
 v2:
 - Added more checks to cover "nmi" and "memsave", too

 hmp.c |  8 +++-
 monitor.c | 37 +++--
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/hmp.c b/hmp.c
index b869617..b1c503a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1013,8 +1013,14 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
 const char *filename = qdict_get_str(qdict, "filename");
 uint64_t addr = qdict_get_int(qdict, "val");
 Error *err = NULL;
+int cpu_index = monitor_get_cpu_index();
 
-qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err);
+if (cpu_index < 0) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+
+qmp_memsave(addr, size, filename, true, cpu_index, &err);
 hmp_handle_error(mon, &err);
 }
 
diff --git a/monitor.c b/monitor.c
index 0841d43..74843eb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
 CPUState *mon_get_cpu(void)
 {
 if (!cur_mon->mon_cpu) {
+if (!first_cpu) {
+return NULL;
+}
 monitor_set_cpu(first_cpu->cpu_index);
 }
 cpu_synchronize_state(cur_mon->mon_cpu);
@@ -1033,17 +1036,27 @@ CPUState *mon_get_cpu(void)
 
 CPUArchState *mon_get_cpu_env(void)
 {
-return mon_get_cpu()->env_ptr;
+CPUState *cs = mon_get_cpu();
+
+return cs ? cs->env_ptr : NULL;
 }
 
 int monitor_get_cpu_index(void)
 {
-return mon_get_cpu()->cpu_index;
+CPUState *cs = mon_get_cpu();
+
+return cs ? cs->cpu_index : -1;
 }
 
 static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 {
-cpu_dump_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
+CPUState *cs = mon_get_cpu();
+
+if (!cs) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+cpu_dump_state(cs, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
 }
 
 static void hmp_info_jit(Monitor *mon, const QDict *qdict)
@@ -1076,7 +1089,13 @@ static void hmp_info_history(Monitor *mon, const QDict 
*qdict)
 
 static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
 {
-cpu_dump_statistics(mon_get_cpu(), (FILE *)mon, &monitor_fprintf, 0);
+CPUState *cs = mon_get_cpu();
+
+if (!cs) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+cpu_dump_statistics(cs, (FILE *)mon, &monitor_fprintf, 0);
 }
 
 static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
@@ -1235,6 +1254,12 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 int l, line_size, i, max_digits, len;
 uint8_t buf[16];
 uint64_t v;
+CPUState *cs = mon_get_cpu();
+
+if (!cs && (format == 'i' || !is_physical)) {
+monitor_printf(mon, "Can not dump without CPU\n");
+return;
+}
 
 if (format == 'i') {
 int flags = 0;
@@ -1264,7 +1289,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 flags = msr_le << 16;
 flags |= env->bfd_mach;
 #endif
-monitor_disas(mon, mon_get_cpu(), addr, count, is_physical, flags);
+monitor_disas(mon, cs, addr, count, is_physical, flags);
 return;
 }
 
@@ -1303,7 +1328,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 if (is_physical) {
 cpu_physical_memory_read(addr, buf, l);
 } else {
-if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) {
+if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
 monitor_printf(mon, " Cannot access memory\n");
 break;
 }
-- 
1.8.3.1




[Qemu-devel] How to online resize qemu disk with nbd protocol?

2017-01-12 Thread Bob Chen
Hi,


My qemu runs on a 3rd party distributed block storage, and the disk backend
protocol is nbd.

I notices that there are differences between default qcow2 local disk and
my nbd disk, in terms of resizing the disk on the fly.

Local qcow2 disk could work no matter using qemu-img resize or qemu monitor
'block_resize', but the nbd disk seemed to fail to detect the backend size
change(had resized the disk on EBS at first). It said "this feature or
command is not currently supported".

Is that possible to hack qemu nbd code, making it the same way as resizing
local qcow2 disk? I have the interface to resize EBS disk at backend.


Regards,
Bob


Re: [Qemu-devel] [Qemu-trivial] [PATCH] README: Add linux to macOS build info

2017-01-12 Thread Michael Tokarev
31.10.2016 20:01, Peter Maydell wrote:
> The README lists the URLs for the wiki pages describing
> how to build on Linux and Windows; add the equivalent
> link for building on macOS.

Applied to -trivial :)

Thanks,

/mjt



Re: [Qemu-devel] [PATCH for-2.8] Fix documentation and some comments (article, grammar)

2017-01-12 Thread Michael Tokarev
19.11.2016 22:22, Stefan Weil wrote:
[]

Applied to -trivial, thank you!

/mjt




Re: [Qemu-devel] [PATCH for-2.8] include: Fix typos found by codespell

2017-01-12 Thread Michael Tokarev
19.11.2016 22:47, Stefan Weil wrote:
> Add also a missing parenthesis in a comment.
..

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH for-2.8] hw: Fix typos found by codespell

2017-01-12 Thread Michael Tokarev
19.11.2016 22:29, Stefan Weil wrote:
...

Applied to -trivial, except of the hw/block/m25p80.c
hunk which were applied earlier.

Thank you!

/mjt




Re: [Qemu-devel] Proposal PCI/PCIe device placement on PAPR guests

2017-01-12 Thread Andrea Bolognani
On Mon, 2017-01-09 at 10:46 +1100, David Gibson wrote:
> > >* To allow for hotplugged devices, libvirt should also add a number
> > >  of additional, empty vPHBs (the PAPR spec allows for hotplug of
> > >  PHBs, but this is not yet implemented in qemu).
> > 
> > "A number" here will have to mean "one", same number of
> > empty PCIe Root Ports libvirt will add to a newly-defined
> > q35 guest.
> 
> Umm.. why?

Because some applications using libvirt would inevitably
start relying on the fact that such spare PHBs are
available, locking us into providing at least the same
number forever. In other words, increasing the amount at
a later time is always possible, but decreasing it isn't.
We did the same when we started automatically adding PCIe
Root Ports to q35 machines.

The rationale is that having a single spare hotpluggable
slot is extremely convenient for basic usage, eg. a simple
guest created by someone who's not necessarily very
familiar with virtualization; on the other hand, if you
are actually deploying in production you ought to conduct
proper capacity planning and figure out in advance how
many devices you're likely to need to hotplug throughout
the guest's life.

Of course this all will be moot once we can hotplug PHBs :)

-- 
Andrea Bolognani / Red Hat / Virtualization



[Qemu-devel] [Bug 622367] Re: No BIOS MPFP structure with smp=92 and more

2017-01-12 Thread Thomas Huth
Better late than never ;-)

** Changed in: qemu
   Status: Incomplete => Won't Fix

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

Title:
  No BIOS MPFP structure with smp=92 and more

Status in QEMU:
  Won't Fix

Bug description:
  qemu 0.12.2, SeaBios 0.5.1, running qemu-system-x86_64.exe with option -smp.
  If smp>=92 then no MP floating point structure present in 1 Mb. This may be 
verified by pmemsave 0 0x10 in debugger and search for _MP_ signature in 
file.

  qemu 0.10.5 (bios build 05/08/09) can smp=128 (and even 255 if not
  hangs :).

  Host win 7 x64 RTM 7600.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/622367/+subscriptions



[Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

2017-01-12 Thread David Engraf
The CFI entry for sector length must be set to sector length per device. 
This is important for boards using multiple devices like the ARM 
Vexpress board (width = 4, device-width = 2).


Linux and u-boots calculate the size ratio by dividing both values:

size_ratio = info->portwidth / info->chipwidth;

After that the sector length will be multiplied by the size_ratio, thus 
the CFI entry for sector length is doubled. When Linux or u-boot send a 
sector erase, they expect to erase the doubled sector length, but QEMU 
only erases the board specified sector length.


This patch fixes the sector length in the CFI table to match the length 
per device, equal to blocks_per_device.


Signed-off-by: David Engraf 

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 5f0ee9d..8bb61e4 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -703,7 +703,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
 pflash_t *pfl = CFI_PFLASH01(dev);
 uint64_t total_len;
 int ret;
-uint64_t blocks_per_device, device_len;
+uint64_t blocks_per_device, sector_len_per_device, device_len;
 int num_devices;
 Error *local_err = NULL;
 
@@ -727,7 +727,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
  */
 num_devices = pfl->device_width ? (pfl->bank_width / pfl->device_width) : 1;
 blocks_per_device = pfl->nb_blocs / num_devices;
-device_len = pfl->sector_len * blocks_per_device;
+sector_len_per_device = pfl->sector_len / num_devices;
+device_len = sector_len_per_device * blocks_per_device;
 
 /* XXX: to be fixed */
 #if 0
@@ -839,8 +840,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
 /* Erase block region 1 */
 pfl->cfi_table[0x2D] = blocks_per_device - 1;
 pfl->cfi_table[0x2E] = (blocks_per_device - 1) >> 8;
-pfl->cfi_table[0x2F] = pfl->sector_len >> 8;
-pfl->cfi_table[0x30] = pfl->sector_len >> 16;
+pfl->cfi_table[0x2F] = sector_len_per_device >> 8;
+pfl->cfi_table[0x30] = sector_len_per_device >> 16;
 
 /* Extended */
 pfl->cfi_table[0x31] = 'P';


Re: [Qemu-devel] [PATCH] qemu-options: cleanup duplicated help message for kernel_irqchip

2017-01-12 Thread Michael Tokarev
29.11.2016 06:34, Po-Hsu Lin wrote:
> Remove the duplicated help message for 'kernel_irqchip'.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 2/3] run_tests: put logs into per-test file

2017-01-12 Thread Peter Xu
On Thu, Jan 12, 2017 at 11:04:05AM +0100, Andrew Jones wrote:

[...]

> >  config=$TEST_DIR/unittests.cfg
> > -rm -f test.log
> > -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > +
> > +rm -rf $unittest_log_dir.old
> > +[ -d $unittest_log_dir ] && mv $unittest_log_dir $unittest_log_dir.old
> > +mkdir $unittest_log_dir || exit 2
> > +
> > +echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
> > +
> 
> nit: to be 100% correct all the references to the log dir should have
> "'s around them in order to handle spaces, or other shell ambiguous
> characters, in the name. For example, mkdir $log_dir, where $log_dir
> is "my tests" would create two directories, "my" and "tests" without
> the quotes. Anyway, I'm not too worried about someone choosing a
> weird log dir name, nor even changing it from the default.
> 
> Isn't bash fun :-)

It is, as long as it won't break my system... (Once I accidentally did
a "rm -rf /lib" in Bash when I was still using FreeBSD... Bash became
less funny since then ;-).

I'll add them if I'm going to have another spin.

[...]

> Reviewed-by: Andrew Jones 

Thanks for reviewing!

-- peterx



Re: [Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

2017-01-12 Thread Peter Maydell
On 12 January 2017 at 10:35, David Engraf  wrote:
> The CFI entry for sector length must be set to sector length per device.
> This is important for boards using multiple devices like the ARM Vexpress
> board (width = 4, device-width = 2).
>
> Linux and u-boots calculate the size ratio by dividing both values:
>
> size_ratio = info->portwidth / info->chipwidth;
>
> After that the sector length will be multiplied by the size_ratio, thus the
> CFI entry for sector length is doubled. When Linux or u-boot send a sector
> erase, they expect to erase the doubled sector length, but QEMU only erases
> the board specified sector length.
>
> This patch fixes the sector length in the CFI table to match the length per
> device, equal to blocks_per_device.

Thanks for the patch. I haven't checked against the pflash spec yet,
but this looks like it's probably the right thing.

The only two machines which use a setup with multiple devices (ie
which specify device_width to the pflash_cfi01) are vexpress and virt.
For all other machines this patch leaves the behaviour unchanged.

Q: do we need to have some kind of nasty hack so that pre-2.9 virt
still gets the old broken values in the CFI table, for version and
migration compatibility? Ccing Drew for an opinion...

thanks
-- PMM



Re: [Qemu-devel] [PATCH] linux-user: Use *at functions instead of caching interp_prefix contents

2017-01-12 Thread Peter Maydell
On 12 January 2017 at 04:05, Richard Henderson  wrote:
> If the interp_prefix is a complete chroot, it may have a *lot* of files.
> Setting up the cache for this is quite expensive.  Instead, use the *at
> versions of various syscalls to attempt the operation in the prefix.
>

Presumably this also fixes the completely broken handling of
symlinks in the interp_prefix tree?

Unfortunately patch will break bsd-user as it stands,
because bsd-user also calls init_paths. There's also a
usage in tests/tcg/.

Awkward problem: I suspect you'll find this breaks a few
guest programs which don't like the fact that there's now
an open fd which doesn't belong to the guest. (For instance
IIRC some LTP test programs do "check that the rlimit on
number of open files hits after opening exactly N files".)

Incidentally it's a shame C doesn't make it easier to
abstract out the repeated pattern

switch (PATHNAME[0]) {
case '/':
ret = SOMEFN(interp_dirfd, PATHNAME + 1, STUFF);
if (ret == 0 || errno != ENOENT) {
break;
}
/* fallthru */
default:
ret = SOMEFN(FD, PATHNAME, STUFF);
break;
}

(and the variant which uses a 2nd function).

A macro would get awkwardly heavily parameterised, I think.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v4 4/4] migration: Fail migration blocker for --only-migratble

2017-01-12 Thread Ashijeet Acharya
On Tuesday, 10 January 2017, Peter Maydell  wrote:

> On 9 January 2017 at 17:02, Ashijeet Acharya  > wrote:
> > migrate_add_blocker should rightly fail if the '--only-migratable'
> > option was specified and the device in use should not be able to
> > perform the action which results in an unmigratable VM.
> >
> > Make migrate_add_blocker return -EACCES in this case.
>
> > diff --git a/block/qcow.c b/block/qcow.c
> > index 11526a1..bdc6446 100644
> > --- a/block/qcow.c
> > +++ b/block/qcow.c
> > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> >  ret = migrate_add_blocker(s->migration_blocker, errp);
> >  if (ret < 0) {
> > -error_free(s->migration_blocker);
> > +if (ret == -EACCES) {
> > +error_append_hint(errp, "Cannot use a node with qcow format
> as "
> > +  "it does not support live migration");
> > +}
> >  goto fail;
> >  }
> >
>
> The error handling for these call sites should look just like
> that for any other function call that takes an Error**:
>
> Error *local_err = NULL;
> [...]
> migrate_add_blocker(s->migration_blocker, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return; // or otherwise handle failure appropriately
> }


One more reason Peter I found for returning an error value is that in cases
like 9pfs where we do not set errp inside migrate_add_blocker() (as
suggested by Greg) and other similar ones where we pass NULL for errp, we
cannot rely on checking for
"if (local_err)" as it will never be set and always be NULL.
Thus we will never fail appropriately at the caller sites when we fail to
add migration blocker.
Sounds right?

Ashijeet

>
> migrate_add_blocker() should just internally construct
> the error text and extra hint lines by looking at the
> text it can fish out of the s->migration_blocker argument
> and calling error_append_hint() itself.
>
> The patch is also a bit odd because the error_free() calls
> were only added in patch 3/4, right? Generally adding
> lines of code in one patch and deleting them in the next
> is a bad idea.
>
> thanks
> -- PMM
>


Re: [Qemu-devel] [PATCH v2] usb: Fix typo in documentation

2017-01-12 Thread Michael Tokarev
07.12.2016 19:00, Frediano Ziglio wrote:
> simliar -> similar

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH 2/2] pcie: fix typo in comments

2017-01-12 Thread Michael Tokarev
11.11.2016 06:02, Cao jin wrote:

Applied to -trivial, thank you!

/mjt




Re: [Qemu-devel] [PATCH 1/3] object.h: spelling fix

2017-01-12 Thread Michael Tokarev
12.12.2016 20:31, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  include/qom/object.h | 2 +-

Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] [PATCH 2/3] object.h: improve OBJECT/OBJECT_CLASS doc

2017-01-12 Thread Michael Tokarev
12.12.2016 20:31, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  include/qom/object.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index c5e6fc1f5d..c5456db05d 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -467,7 +467,7 @@ struct TypeInfo
>   * OBJECT:
>   * @obj: A derivative of #Object
>   *
> - * Converts an object to a #Object.  Since all objects are #Objects,
> + * Converts an object to a #Object.  Since all objects are #Object,
>   * this function will always succeed.

I'm not sure this is a good change. Yes the type name is "Object"
(singular), but we refer to multiple objectS (plural), so... :)
If we had some coloring, we'd wrote it like, eg, #Objects,
with "s" being outside of the type name.

Thanks,

/mjt



[Qemu-devel] [PATCH] ide: avoid unbounded recursion

2017-01-12 Thread Paolo Bonzini
The end_transfer_func can call ide_transfer_start immediately, before
returning, and unbounded recursion can happen at least for
ide_atapi_cmd_reply_end.  Use a bottom half to defer the call and
limit stack usage.

Cc: Peter Lieven 
Cc: John Snow 
Signed-off-by: Paolo Bonzini 
---
 hw/ide/core.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 43709e5..7b9831f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -482,6 +482,13 @@ static void ide_clear_retry(IDEState *s)
 s->bus->retry_nsector = 0;
 }
 
+static void ide_start_transfer_bh_cb(void *opaque)
+{
+IDEDMA *dma = opaque;
+
+dma->ops->start_transfer(dma);
+}
+
 /* prepare data transfer and tell what to do after */
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
 EndTransferFunc *end_transfer_func)
@@ -494,7 +501,12 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int 
size,
 s->status |= DRQ_STAT;
 }
 if (s->bus->dma->ops->start_transfer) {
-s->bus->dma->ops->start_transfer(s->bus->dma);
+/* There can be unbounded recursion between ops->start_transfer
+ * and end_transfer_func, so defer to a bottom half.
+ */
+aio_bh_schedule_oneshot(qemu_get_aio_context(),
+ide_start_transfer_bh_cb,
+s->bus->dma);
 }
 }
 
-- 
2.9.3




Re: [Qemu-devel] [PATCH] block: remove dead check

2017-01-12 Thread Michael Tokarev
04.01.2017 17:59, Paolo Bonzini wrote:
> options must be non-NULL here, because a NULL value is replaced with
> qdict_new earlier in the function.  Reported by Coverity.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH] qga: fix erroneous argument to strerror

2017-01-12 Thread Michael Tokarev
04.01.2017 17:52, Paolo Bonzini wrote:
> process_command returns a negative value in case of error.  Make this
> clear in the "if" statement and fix the strerror argument to flip it
> to positive.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH 3/3] object: make some funcs static

2017-01-12 Thread Michael Tokarev
12.12.2016 20:31, Marc-André Lureau wrote:
> There is no need to have those functions as public API.

Were "some" being object_initialize_with_type() and
object_new_with_type().

I'm applying this to -trivial, and Cc'ing Andreas, maybe
he will say more.

Thanks,

> Signed-off-by: Marc-André Lureau 
> ---
>  qom/object.c |  4 ++--
>  include/qom/object.h | 24 
>  2 files changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 7a05e35ed9..eb3d0f64e4 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -351,7 +351,7 @@ static void object_post_init_with_type(Object *obj, 
> TypeImpl *ti)
>  }
>  }
>  
> -void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
> +static void object_initialize_with_type(void *data, size_t size, TypeImpl 
> *type)
>  {
>  Object *obj = data;
>  
> @@ -467,7 +467,7 @@ static void object_finalize(void *data)
>  }
>  }
>  
> -Object *object_new_with_type(Type type)
> +static Object *object_new_with_type(Type type)
>  {
>  Object *obj;
>  
> diff --git a/include/qom/object.h b/include/qom/object.h
> index c5456db05d..e9791a210e 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -586,18 +586,6 @@ struct InterfaceClass
>   */
>  Object *object_new(const char *typename);
>  
> -/**
> - * object_new_with_type:
> - * @type: The type of the object to instantiate.
> - *
> - * This function will initialize a new object using heap allocated memory.
> - * The returned object has a reference count of 1, and will be freed when
> - * the last reference is dropped.
> - *
> - * Returns: The newly allocated and instantiated object.
> - */
> -Object *object_new_with_type(Type type);
> -
>  /**
>   * object_new_with_props:
>   * @typename:  The name of the type of the object to instantiate.
> @@ -726,18 +714,6 @@ int object_set_propv(Object *obj,
>   Error **errp,
>   va_list vargs);
>  
> -/**
> - * object_initialize_with_type:
> - * @data: A pointer to the memory to be used for the object.
> - * @size: The maximum size available at @data for the object.
> - * @type: The type of the object to instantiate.
> - *
> - * This function will initialize an object.  The memory for the object should
> - * have already been allocated.  The returned object has a reference count 
> of 1,
> - * and will be finalized when the last reference is dropped.
> - */
> -void object_initialize_with_type(void *data, size_t size, Type type);
> -
>  /**
>   * object_initialize:
>   * @obj: A pointer to the memory to be used for the object.
> 




Re: [Qemu-devel] [PATCH] doc/usb2: fix typo

2017-01-12 Thread Michael Tokarev
Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] [PATCH] qemu-img: remove dead check

2017-01-12 Thread Michael Tokarev
04.01.2017 17:56, Paolo Bonzini wrote:
> options must be non-NULL here, because it has been checked before.
> Reported by Coverity.

Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] [PATCH] lm32: milkymist-tmu2: fix another integer overflow

2017-01-12 Thread Michael Tokarev
06.01.2017 20:45, Peter Maydell wrote:
> Don't truncate the multiplication and do a 64 bit one instead
> because the result is stored in a 64 bit variable.
> 
> This fixes a similar coverity warning to commit 237a8650d640,
> in a similar way, and is the other half of the fix for
> coverity CID 1167561.

Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] [PATCH] hw/i386/kvmvapic: Remove dead code in patch_hypercalls()

2017-01-12 Thread Michael Tokarev
09.01.2017 20:05, Peter Maydell wrote:
> The patch_hypercalls() function sets up a 'patches'
> variable and checks it at the end of the function, but
> never modifies it in the middle. Remove this dead code,
> which seems to have been present since the function was
> added in commit e5ad936b0fd7 in 2012.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL

2017-01-12 Thread Michael Tokarev
09.01.2017 22:05, Peter Maydell wrote:
> Coverity (CID 1005689) warns that we don't check that
> spec_reg_info() returned non-NULL before dereferencing.
> Add the check, though as the comment notes this is
> a can't-really-happen case because the earlier constraint
> matching should have ruled out the "unknown reg" case.

Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] [PATCH] pci-assign: avoid pointless stat

2017-01-12 Thread Michael Tokarev
04.01.2017 18:05, Paolo Bonzini wrote:
> Just check the errno value after fopen and follow it with fstat.
> This shuts up Coverity's complaint about TOC/TOU violation.

Applied to -trivial, thanks!

/mjt



[Qemu-devel] [Bug 1130533] Re: Documentation cannot be build since commit c70a01e449536c616c85ab820c6fbad7d7e9cf39

2017-01-12 Thread Thomas Huth
This should have been fixed by this commit here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=5d6768e3b8908a60f0

** Changed in: qemu
   Status: New => Fix Released

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

Title:
  Documentation cannot be build since commit
  c70a01e449536c616c85ab820c6fbad7d7e9cf39

Status in QEMU:
  Fix Released

Bug description:
  I tried to build git -based qemu and when documentation is processed I
  got this error :

  ./qemu-options.texi:1526: unknown command `list'
  ./qemu-options.texi:1526: table requires an argument: the formatter for @item
  ./qemu-options.texi:1526: warning: @table has text but no @item

  Looks like commit c70a01e449536c616c85ab820c6fbad7d7e9cf39 is guilty
  ?!

  Or any modification related to documentation, I think.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1130533/+subscriptions



Re: [Qemu-devel] [PATCH] docs: sync pci-ids.txt

2017-01-12 Thread Michael Tokarev
11.01.2017 14:02, Gerd Hoffmann wrote:
> Three commits allocated pci ids in include/hw/pci/pci.h
> without also updating docs/specs/pci-ids.txt:
> 
>   bf439db pci: Allocate PCIe host bridge PCI ID
>   40d14be hw/pci: introduce PCI Expander Bridge (PXB)
>   02b0743 hw/pxb: introduce pxb-pcie expander for PCIe machines
> 
> This patch updates pci-ids.txt accordingly.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH 1/2] vfio: remove a duplicated word in comments

2017-01-12 Thread Michael Tokarev
11.11.2016 06:01, Cao jin wrote:
[]

Applied to -trivial, thank you!

/mjt




Re: [Qemu-devel] [PATCH v4 0/2] trivial changes on util/mmap-alloc

2017-01-12 Thread Michael Tokarev
02.11.2016 16:44, Cao jin wrote:
> Cao jin (2):
>   util/mmap-alloc: check parameter before using
>   util/mmap-alloc: refactor a little bit for readability
> 
>  util/mmap-alloc.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)

Applied (finally) to -trivial, thank you very much!

/mjt




Re: [Qemu-devel] [PATCH v4 4/4] migration: Fail migration blocker for --only-migratble

2017-01-12 Thread Greg Kurz
On Thu, 12 Jan 2017 16:15:28 +0530
Ashijeet Acharya  wrote:

> On Tuesday, 10 January 2017, Peter Maydell  wrote:
> 
> > On 9 January 2017 at 17:02, Ashijeet Acharya  > > wrote:
> > > migrate_add_blocker should rightly fail if the '--only-migratable'
> > > option was specified and the device in use should not be able to
> > > perform the action which results in an unmigratable VM.
> > >
> > > Make migrate_add_blocker return -EACCES in this case.
> >
> > > diff --git a/block/qcow.c b/block/qcow.c
> > > index 11526a1..bdc6446 100644
> > > --- a/block/qcow.c
> > > +++ b/block/qcow.c
> > > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > >  ret = migrate_add_blocker(s->migration_blocker, errp);
> > >  if (ret < 0) {
> > > -error_free(s->migration_blocker);
> > > +if (ret == -EACCES) {
> > > +error_append_hint(errp, "Cannot use a node with qcow format
> > as "
> > > +  "it does not support live migration");
> > > +}
> > >  goto fail;
> > >  }
> > >
> >
> > The error handling for these call sites should look just like
> > that for any other function call that takes an Error**:
> >
> > Error *local_err = NULL;
> > [...]
> > migrate_add_blocker(s->migration_blocker, &local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > return; // or otherwise handle failure appropriately
> > }
> 
> 
> One more reason Peter I found for returning an error value is that in cases
> like 9pfs where we do not set errp inside migrate_add_blocker() (as
> suggested by Greg) and other similar ones where we pass NULL for errp, we
> cannot rely on checking for
> "if (local_err)" as it will never be set and always be NULL.
> Thus we will never fail appropriately at the caller sites when we fail to
> add migration blocker.
> Sounds right?
> 

The need for 9pfs is just to return an error to the guest, which will end
up being the errno for the failed mount(). And we don't want to print out
any error message to avoid a stupid guest to fill the QEMU log in case it
would loop over mount().

The only reason I see for migration_add_blocker() to return an error
would be to differentiate between the --only-migratable and the migration
in progress cases... But honestly, I don't care that much and something
like the following would be perfectly ok:

Error *local_err = NULL;
[...]
migrate_add_blocker(s->migration_blocker, &local_err);
if (local_err) {
error_free(local_err);
err = -EBUSY
goto out_nofid;
}

> Ashijeet
> 
> >
> > migrate_add_blocker() should just internally construct
> > the error text and extra hint lines by looking at the
> > text it can fish out of the s->migration_blocker argument
> > and calling error_append_hint() itself.
> >
> > The patch is also a bit odd because the error_free() calls
> > were only added in patch 3/4, right? Generally adding
> > lines of code in one patch and deleting them in the next
> > is a bad idea.
> >
> > thanks
> > -- PMM
> >




Re: [Qemu-devel] [PATCH 2/3] object.h: improve OBJECT/OBJECT_CLASS doc

2017-01-12 Thread Peter Maydell
On 12 January 2017 at 10:46, Michael Tokarev  wrote:
> 12.12.2016 20:31, Marc-André Lureau wrote:
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  include/qom/object.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index c5e6fc1f5d..c5456db05d 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -467,7 +467,7 @@ struct TypeInfo
>>   * OBJECT:
>>   * @obj: A derivative of #Object
>>   *
>> - * Converts an object to a #Object.  Since all objects are #Objects,
>> + * Converts an object to a #Object.  Since all objects are #Object,
>>   * this function will always succeed.
>
> I'm not sure this is a good change. Yes the type name is "Object"
> (singular), but we refer to multiple objectS (plural), so... :)
> If we had some coloring, we'd wrote it like, eg, #Objects,
> with "s" being outside of the type name.

Mmm, but doc-comment parsers won't be able to figure out that
#Objects should refer to the #Object type. You could rephrase
as "Since every object is an #Object" to get around that.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 1/2] memory: provide common macros for mtree_print_mr()

2017-01-12 Thread Paolo Bonzini


On 12/01/2017 10:46, Peter Xu wrote:
> Yes, above suggestion makes sense to me, since after all the RW
> permissions are derived from the type of memory regions, and the type
> itself tells more things than the RW bits. So I totally agree we can
> replace the "RW" chars with its type directly (if no one else
> disagree, of course).
> 
> While for below patch, do you want me to include it as well as a
> standalone patch, for the purpose of refactoring
> memory_access_is_direct()? Since IMHO it's tiny clearer and more
> readable than before.

It is more readable, but my plan was to turn these fields into a single
field (with bits) to speed up memory_access_is_direct.  For that we'd
need to undo your change.  So I'm undecided.

Paolo



Re: [Qemu-devel] [PATCH v6 6/7] trace: [tcg, trivial] Re-align generated code

2017-01-12 Thread Michael Tokarev
28.12.2016 21:41, Lluís Vilanova wrote:
> Last patch removed a nesting level in generated code. Re-align all code
> generated by backends to be 4-column aligned.

Lluís, I'm not applying these (6/7 and 7/7) to -trivial because
while they're trivial indeed, they aren't independent and should
be applied after all other patches in your series, or better,
together.

Thanks,

/mjt



Re: [Qemu-devel] [PATCH] win32: use glib gpoll if glib >= 2.50

2017-01-12 Thread Michael Tokarev
03.01.2017 22:19, Marc-André Lureau wrote:
> A fix has been committed in upstream glib commit
> 210a9796f78eb90f76f1bd6a304e9fea05e97617.
> (See also related bug https://bugzilla.gnome.org/show_bug.cgi?id=764415)
> 
> It is desirable to use the glib version instead of qemu copy, since it
> provides more debugging facilities (G_MAIN_POLL_DEBUG etc), and
> hopefully has a better maintainance. Hopefully, we can drop the qemu
> copy in a few years.

Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH] hw/display/framebuffer.c: Avoid overflow for framebuffers > 4GB

2017-01-12 Thread Michael Tokarev
09.01.2017 19:45, Peter Maydell wrote:
> Coverity points out that calculating src_len by multiplying
> src_width by rows could overflow. This can only happen in
> the implausible case of a framebuffer larger than 4GB, but
> we may as well fix it, placating Coverity. (CID1005515)

Applied to -trivial, thanks!

/mjt



Re: [Qemu-devel] implementing architectural timers using QEMU timers

2017-01-12 Thread Pavel Dovgalyuk
> From: Max Filippov [mailto:jcmvb...@gmail.com]
> On Tue, Jan 10, 2017 at 12:31 AM, Pavel Dovgalyuk  wrote:
> >> From: Max Filippov [mailto:jcmvb...@gmail.com]
> >
> >> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
> >> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
> >> timers. That is CCOUNT value is derived from the
> >> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
> >> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
> >> The code is here:
> >>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
> >>
> >> I've got the following issues doing that:
> >>
> >> - I thought that could be improved in -icount mode, so I tried that.
> >>   It is better with -icount, but it's still not 100% accurate. That is
> >>   I was able to observe guest reading QEMU clock value that is
> >>   past QEMU timer deadline before that timer callback was
> >>   invoked.
> >
> > icount is meant to be 100% accurate.
> > tcg_get_icount_limit function calculates the deadline before the soonest
> > virtual timer and executes number of instructions that will fit this
> > timeout.
> 
> Ok, looks like what happens in my case is that instruction that
> sets CCOMPARE and thus changes remaining icount does not
> cause exit from the cpu_exec. So merely ending TB on
> QEMU_CLOCK_VIRTUAL timer update is not enough, I need to
> throw an exception of some kind? Or does the timer code need
> to take care of that?

Yes, it seems that you should end the block with an exception,
to allow icount loop recalculate the timeouts.

Pavel Dovgalyuk




Re: [Qemu-devel] [PATCH v15 0/2] virtio-crypto: virtio crypto device specification

2017-01-12 Thread Gonglei (Arei)
Ping...

Any Comments?

Thanks,
-Gonglei

> -Original Message-
> From: Gonglei (Arei)
> Sent: Wednesday, January 04, 2017 6:03 PM
> To: qemu-devel@nongnu.org; virtio-...@lists.oasis-open.org
> Cc: Luonengjun; m...@redhat.com; cornelia.h...@de.ibm.com;
> stefa...@redhat.com; denglin...@chinamobile.com; Jani Kokkonen;
> ola.liljed...@arm.com; varun.se...@freescale.com; xin.z...@intel.com;
> brian.a.keat...@intel.com; liang.j...@intel.com; john.grif...@intel.com;
> Huangweidong (C); mike.cara...@nxp.com; ag...@suse.de; Claudio Fontana;
> Zhoujian (jay, Euler); nmo...@kalray.eu; vincent.jar...@6wind.com; Wubin (H);
> Shiqing Fan; arei.gong...@hotmail.com; pa...@linux.vnet.ibm.com; Gonglei
> (Arei)
> Subject: [PATCH v15 0/2] virtio-crypto: virtio crypto device specification
> 
> Changes since v14:
>  - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia]
>  - correct a sentence about dataqueue and controlq in the first paragraph.
> [Halil]
>  - change a MAY to MUST about max_dataqueues. [Halil]
>  - add non-session mode support
>1) add four features for different crypto services to identify wheather
> support session mode.
>2) extend virtio_crypto_*_para structures, for example, add the content of
>  struct virtio_crypto_cipher_session_para into struct
> virtio_crypto_cipher_para.
>3) use the flag property of struct virtio_crypto_op_header to identify the
>  type of crypto request. Aka Is it a session-based or non-session request
> 
> For pervious versions of virtio crypto spec, Pls see:
> 
> [v14]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html
> 
> [v13]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html
> 
> For more information, please see:
>  http://qemu-project.org/Features/VirtioCrypto
> 
> Please help to review, thanks.
> 
> Gonglei (2):
>   virtio-crypto: Add virtio crypto device specification
>   virtio-crypto: Add conformance clauses
> 
>  conformance.tex   |   30 ++
>  content.tex   |2 +
>  virtio-crypto.tex | 1029
> +
>  3 files changed, 1061 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 
> --
> 1.7.12.4
> 




Re: [Qemu-devel] [PATCH] linux-user: fix build with musl on aarch64

2017-01-12 Thread Michael Tokarev
15.12.2016 14:04, Natanael Copa wrote:
> Use the standard uint64_t instead of internal __u64.
> 
> This fixes compiler error with musl libc on aarch64:
> .../qemu-2.7.0/linux-user/host/aarch64/hostdep.h:28:5:
> error: unknown type name '__u64'
>  __u64 *pcreg = &uc->uc_mcontext.pc;
>  ^
> 
> Signed-off-by: Natanael Copa 
> ---
>  linux-user/host/aarch64/hostdep.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/host/aarch64/hostdep.h 
> b/linux-user/host/aarch64/hostdep.h
> index 64f75cef49..6fd6e36b2a 100644
> --- a/linux-user/host/aarch64/hostdep.h
> +++ b/linux-user/host/aarch64/hostdep.h
> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>  static inline void rewind_if_in_safe_syscall(void *puc)
>  {
>  struct ucontext *uc = puc;
> -__u64 *pcreg = &uc->uc_mcontext.pc;
> +uint64_t *pcreg = &uc->uc_mcontext.pc;

Is this type available in all places where this header is
being included (apparently these are linux-user/qemu.h and
linux-user/safe-syscall.S, with the latter not being
interesting. But in linux-user/qemu.h, this is the first
file which is included..

Why do we have a mix of __u64 and uint64_t ? :)

Hmm.

/mjt

>  if (*pcreg > (uintptr_t)safe_syscall_start
>  && *pcreg < (uintptr_t)safe_syscall_end) {
> 




Re: [Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

2017-01-12 Thread Andrew Jones
On Thu, Jan 12, 2017 at 10:42:41AM +, Peter Maydell wrote:
> On 12 January 2017 at 10:35, David Engraf  wrote:
> > The CFI entry for sector length must be set to sector length per device.
> > This is important for boards using multiple devices like the ARM Vexpress
> > board (width = 4, device-width = 2).
> >
> > Linux and u-boots calculate the size ratio by dividing both values:
> >
> > size_ratio = info->portwidth / info->chipwidth;
> >
> > After that the sector length will be multiplied by the size_ratio, thus the
> > CFI entry for sector length is doubled. When Linux or u-boot send a sector
> > erase, they expect to erase the doubled sector length, but QEMU only erases
> > the board specified sector length.
> >
> > This patch fixes the sector length in the CFI table to match the length per
> > device, equal to blocks_per_device.
> 
> Thanks for the patch. I haven't checked against the pflash spec yet,
> but this looks like it's probably the right thing.
> 
> The only two machines which use a setup with multiple devices (ie
> which specify device_width to the pflash_cfi01) are vexpress and virt.
> For all other machines this patch leaves the behaviour unchanged.
> 
> Q: do we need to have some kind of nasty hack so that pre-2.9 virt
> still gets the old broken values in the CFI table, for version and
> migration compatibility? Ccing Drew for an opinion...
>

I'm pretty sure we need the nasty hack, but I'm also Ccing David for
his opinion.

Thanks,
drew



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 3/3] run_tests: allow run tests in parallel

2017-01-12 Thread Andrew Jones
On Thu, Jan 12, 2017 at 11:36:22AM +0800, Peter Xu wrote:
> run_task.sh is getting slow. This patch is trying to make it faster by
> running the tests concurrently.
> 
> We provide a new parameter "-j" for the run_tests.sh, which can be used
> to specify how many run queues we want for the tests. Default queue
> length is 1, which is the old behavior.
> 
> Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
> 
>|-+---|
>| command | time used |
>|-+---|
>| run_test.sh | 75s   |
>| run_test.sh -j8 | 27s   |
>|-+---|
> 
> Signed-off-by: Peter Xu 
> ---
>  run_tests.sh| 12 ++--
>  scripts/common.bash | 16 +++-
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index afd3d95..4d57ff9 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
>  {
>  cat <  
> -Usage: $0 [-g group] [-h] [-v]
> +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
>  
>  -g: Only execute tests in the given group
>  -h: Output this help text
> +-j: Execute tests in parallel
>  -v: Enables verbose mode
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> @@ -28,7 +29,7 @@ EOF
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hj:v" opt; do
>  case $opt in
>  g)
>  only_group=$OPTARG
> @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
>  usage
>  exit
>  ;;
> +j)
> +unittest_run_queues=$OPTARG
> +if (( $unittest_run_queues <= 0 )); then
> +echo "Invalid -j option: $unittest_run_queues"
> +exit 2
> +fi
> +;;
>  v)
>  verbose="yes"
>  ;;
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 2dd7360..9bd560f 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -1,11 +1,19 @@
>  : ${unittest_log_dir:=logs}
> +: ${unittest_run_queues:=1}
>  
>  function run_task()
>  {
>   local testname="$2"
>  
> + while (( $(jobs | wc -l) == $unittest_run_queues )); do
> + # wait for any background test to finish
> + wait -n
> + done
> +
>   RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> - "$@"
> +
> + # start the testcase in the background
> + "$@" &
>  }
>  
>  function for_each_unittest()
> @@ -22,6 +30,8 @@ function for_each_unittest()
>   local accel
>   local timeout
>  
> + trap "wait; exit 130" SIGINT
> +
>   exec {fd}<"$unittests"
>  
>   while read -u $fd line; do
> @@ -55,5 +65,9 @@ function for_each_unittest()
>   fi
>   done
>   run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
> "$check" "$accel" "$timeout"
> +
> + # wait until all tasks finish
> + wait
> +
>   exec {fd}<&-
>  }
> -- 
> 2.7.4
> 
>

Reviewed-by: Andrew Jones  



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 1/3] run_tests: fix errno for param parsing

2017-01-12 Thread Andrew Jones
On Thu, Jan 12, 2017 at 11:36:20AM +0800, Peter Xu wrote:
> Signed-off-by: Peter Xu 
> ---
>  run_tests.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 254129d..2cfa365 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -41,7 +41,7 @@ while getopts "g:hv" opt; do
>  verbose="yes"
>  ;;
>  *)
> -exit 1
> +exit 2
>  ;;
>  esac
>  done
> -- 
> 2.7.4
> 
>

Reviewed-by: Andrew Jones  



Re: [Qemu-devel] [PATCH] Further tidy-up on block status

2017-01-12 Thread Vladimir Sementsov-Ogievskiy
(Sorry, it may be a bit out of topic, but I hope it is comfortable for 
all that I just comment current version of the feature by answering 
cover letter of last related patch set)


From |"NBD_OPT_LIST_META_CONTEXT| (9)":
The server MUST either reply with an error (for instance |EINVAL| if 
the option is not supported), or reply with a list of 
|NBD_REP_META_CONTEXT| replies followed by |NBD_REP_ACK|.:


NBD_REP_ERR_UNSUP for the case in braces? Should it be normal option 
reply packet?




--
Best regards,
Vladimir



Re: [Qemu-devel] [PATCH v4 4/4] migration: Fail migration blocker for --only-migratble

2017-01-12 Thread Ashijeet Acharya
On Thursday, 12 January 2017, Greg Kurz  wrote:

> On Thu, 12 Jan 2017 16:15:28 +0530
> Ashijeet Acharya > wrote:
>
> > On Tuesday, 10 January 2017, Peter Maydell  > wrote:
> >
> > > On 9 January 2017 at 17:02, Ashijeet Acharya <
> ashijeetacha...@gmail.com 
> > > > wrote:
> > > > migrate_add_blocker should rightly fail if the '--only-migratable'
> > > > option was specified and the device in use should not be able to
> > > > perform the action which results in an unmigratable VM.
> > > >
> > > > Make migrate_add_blocker return -EACCES in this case.
> > >
> > > > diff --git a/block/qcow.c b/block/qcow.c
> > > > index 11526a1..bdc6446 100644
> > > > --- a/block/qcow.c
> > > > +++ b/block/qcow.c
> > > > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > >  ret = migrate_add_blocker(s->migration_blocker, errp);
> > > >  if (ret < 0) {
> > > > -error_free(s->migration_blocker);
> > > > +if (ret == -EACCES) {
> > > > +error_append_hint(errp, "Cannot use a node with qcow
> format
> > > as "
> > > > +  "it does not support live migration");
> > > > +}
> > > >  goto fail;
> > > >  }
> > > >
> > >
> > > The error handling for these call sites should look just like
> > > that for any other function call that takes an Error**:
> > >
> > > Error *local_err = NULL;
> > > [...]
> > > migrate_add_blocker(s->migration_blocker, &local_err);
> > > if (local_err) {
> > > error_propagate(errp, local_err);
> > > return; // or otherwise handle failure appropriately
> > > }
> >
> >
> > One more reason Peter I found for returning an error value is that in
> cases
> > like 9pfs where we do not set errp inside migrate_add_blocker() (as
> > suggested by Greg) and other similar ones where we pass NULL for errp, we
> > cannot rely on checking for
> > "if (local_err)" as it will never be set and always be NULL.
> > Thus we will never fail appropriately at the caller sites when we fail to
> > add migration blocker.
> > Sounds right?
> >
>
> The need for 9pfs is just to return an error to the guest, which will end
> up being the errno for the failed mount(). And we don't want to print out
> any error message to avoid a stupid guest to fill the QEMU log in case it
> would loop over mount().


Yes, I completely understood that earlier :-)
I had a doubt further because atm I was passing NULL as an argument i.e.

migrate_add_blocker(s->migration_blocker, NULL);

which is why the whole 'local_err being returned as  NULL' situation
arrived. But I will make it pass &local_err now.

Still, I think we need to return an error value from migrate_add_blocker()
to avoid setting it manually and also avoid repetition of code at call
sites.

>
> The only reason I see for migration_add_blocker() to return an error
> would be to differentiate between the --only-migratable and the migration
> in progress cases... But honestly, I don't care that much and something
> like the following would be perfectly ok:
>
> Error *local_err = NULL;
> [...]
> migrate_add_blocker(s->migration_blocker, &local_err);
> if (local_err) {
> error_free(local_err);
> err = -EBUSY
> goto out_nofid;
> }
>
> Maybe yes, if everyone is okay with the same error value being set in both
the cases, then I will submit the patches like you proposed above :-)

Ashijeet


Re: [Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

2017-01-12 Thread Peter Maydell
On 11 January 2017 at 08:49, Greg Kurz  wrote:
> Some recently added tests pass a zero length to qtest_memwrite().
> Unfortunately, the qtest protocol doesn't implement an on-the-wire
> syntax for zero-length writes and the current code happily sends
> garbage to QEMU. This causes intermittent failures.
>
> It isn't worth the pain to enhance the protocol, so this patch
> simply fixes the issue by "just return, doing nothing". The same
> fix is applied to qtest_memread() since the issue also exists in
> the QEMU part of the "memread" command.
>
> Suggested-by: Peter Maydell 
> Signed-off-by: Greg Kurz 
> ---

Thanks; applied to master since it should fix some
intermittent failures in the tests I run on new merges.

-- PMM



Re: [Qemu-devel] about post copy recovery

2017-01-12 Thread Dr. David Alan Gilbert
* Li, Liang Z (liang.z...@intel.com) wrote:
> 
> Hi David,
> 
> I remembered some guys wanted to solve the issue of post copy recovery when 
> network broken down, do you know latest status?

Hi Liang,
  Yes, Haris looked at it as part of GSoC, the latest
version is what was posted:

https://lists.nongnu.org/archive/html/qemu-devel/2016-08/msg03468.html

I've not done any work on it since then;  there are a couple of
hard problems to be solved.  The simpler is making sure that
we always correctly detect a migration error due to networking
(rather than some other non-recoverable error); there's lots of
migration code that doesn't check for a file error straight away
and only hits the error code later on when it's too late to recover.

The harder problem is that we often end up with the case where
the main thread is blocked trying to access postcopied-RAM,
e.g. an emulated network driver tries to write an incoming
packet to guest RAM but finds the guest RAM hasn't arrived
yet.
With the main thread blocked it's very difficult to recover -
we can't issue any commands to trigger the recovery and even
if we could we'll have to be very careful about what things
those commands need the main thread to do.

Dave

> 
> Thanks!
> Liang
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v15 0/2] virtio-crypto: virtio crypto device specification

2017-01-12 Thread Halil Pasic


On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> Hi all,
> 
> I attach the diff files between v14 and v15 for better review.
> 
Hi,

only had a quick look. Will try to come back to this later.

> diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> index 9f7faf0..884ee95 100644
> --- a/virtio-crypto.tex
> +++ b/virtio-crypto.tex
> @@ -2,8 +2,8 @@
>  
>  The virtio crypto device is a virtual cryptography device as well as a kind 
> of
>  virtual hardware accelerator for virtual machines. The encryption and
> -decryption requests are placed in the data queue and are ultimately handled 
> by the
> -backend crypto accelerators. The second queue is the control queue used to 
> create 
> +decryption requests are placed in any of the data active queues and are 
> ultimately handled by the
s/data active/active data/
> +backend crypto accelerators. The second kind of queue is the control queue 
> used to create 
>  or destroy sessions for symmetric algorithms and will control some advanced
>  features in the future. The virtio crypto device provides the following 
> crypto
>  services: CIPHER, MAC, HASH, and AEAD.

[..]

> ===The below diff shows the changes of add non-session mode 
> support:
> 
> diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> index 884ee95..44819f9 100644
> --- a/virtio-crypto.tex
> +++ b/virtio-crypto.tex
> @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
>  
>  \subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature 
> bits}
>  
> -None currently defined.
> +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is available for CIPHER 
> service.
> +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is available for HASH 
> service.
> +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is available for MAC 
> service.
> +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is available for AEAD 
> service.
>  
>  \subsection{Device configuration layout}\label{sec:Device Types / Crypto 
> Device / Device configuration layout}
>  
> @@ -208,6 +211,9 @@ Operation parameters are algorithm-specific parameters, 
> output data is the
>  data that should be utilized in operations, and input data is equal to
>  "operation result + result data".
>  
> +The device can support both session mode (See \ref{sec:Device Types / Crypto 
> Device / Device Operation / Control Virtqueue / Session operation}) and 
> non-session mode, for example,
> +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated, the driver can 
> use session mode for CIPHER service, otherwise it can only use non-session 
> mode.
> +

As far as I understand you are adding non-session mode to the mix but
providing feature bits for session mode. Would this render the the current
implementation non-compliant?

Halil




Re: [Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

2017-01-12 Thread David Engraf

Am 12.01.2017 um 12:36 schrieb Andrew Jones:

On Thu, Jan 12, 2017 at 10:42:41AM +, Peter Maydell wrote:

On 12 January 2017 at 10:35, David Engraf  wrote:

The CFI entry for sector length must be set to sector length per device.
This is important for boards using multiple devices like the ARM Vexpress
board (width = 4, device-width = 2).

Linux and u-boots calculate the size ratio by dividing both values:

size_ratio = info->portwidth / info->chipwidth;

After that the sector length will be multiplied by the size_ratio, thus the
CFI entry for sector length is doubled. When Linux or u-boot send a sector
erase, they expect to erase the doubled sector length, but QEMU only erases
the board specified sector length.

This patch fixes the sector length in the CFI table to match the length per
device, equal to blocks_per_device.


Thanks for the patch. I haven't checked against the pflash spec yet,
but this looks like it's probably the right thing.

The only two machines which use a setup with multiple devices (ie
which specify device_width to the pflash_cfi01) are vexpress and virt.
For all other machines this patch leaves the behaviour unchanged.

Q: do we need to have some kind of nasty hack so that pre-2.9 virt
still gets the old broken values in the CFI table, for version and
migration compatibility? Ccing Drew for an opinion...



I'm pretty sure we need the nasty hack, but I'm also Ccing David for
his opinion.


I can update the patch if you give me some more information about the 
hack. Some ifdef for older versions?


Best regards
- David




Re: [Qemu-devel] implementing architectural timers using QEMU timers

2017-01-12 Thread Peter Maydell
On 12 January 2017 at 11:28, Pavel Dovgalyuk  wrote:
>> From: Max Filippov [mailto:jcmvb...@gmail.com]
>> Ok, looks like what happens in my case is that instruction that
>> sets CCOMPARE and thus changes remaining icount does not
>> cause exit from the cpu_exec. So merely ending TB on
>> QEMU_CLOCK_VIRTUAL timer update is not enough, I need to
>> throw an exception of some kind? Or does the timer code need
>> to take care of that?
>
> Yes, it seems that you should end the block with an exception,
> to allow icount loop recalculate the timeouts.

Really? The ARM translate.c doesn't generate an exception.
It just does
 gen_io_end();
 gen_lookup_tb();

(so we force a lookup of the next TB, but don't throw an
exception of any kind).

thanks
-- PMM



Re: [Qemu-devel] [kvm-unit-tests PATCH 0/8] VT-d ioapic irq test

2017-01-12 Thread Paolo Bonzini


On 30/12/2016 09:55, Peter Xu wrote:
> The previous vt-d unittest series only contains the very basic tests.
> Let's enlarge it step by step.
> 
> This series expanded it with IOAPIC irq test.
> 
> Peter Xu (8):
>   pci: introduce pci_intx_line()
>   pci: introduce pci_msi_set_enable()
>   lib/asm-generic: add atomic.h
>   x86: ioapic: generalize trigger mode
>   intel-iommu: add report prefixes
>   intel-iommu: use atomic ops for irte index alloc
>   intel-iommu: allow setup trigger mode for irte
>   intel-iommu: add ioapic irq test
> 
>  lib/asm-generic/atomic.h | 21 ++
>  lib/pci.c| 24 ++--
>  lib/pci.h|  2 ++
>  lib/x86/apic.h   |  6 +
>  lib/x86/atomic.h |  2 ++
>  lib/x86/intel-iommu.c| 58 
> +++-
>  lib/x86/intel-iommu.h|  3 +++
>  x86/intel-iommu.c| 50 -
>  x86/ioapic.c | 34 +---
>  9 files changed, 169 insertions(+), 31 deletions(-)
>  create mode 100644 lib/asm-generic/atomic.h
> 

Queued, thanks.

Paolo



Re: [Qemu-devel] implementing architectural timers using QEMU timers

2017-01-12 Thread Pavel Dovgalyuk
> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> On 12 January 2017 at 11:28, Pavel Dovgalyuk  wrote:
> >> From: Max Filippov [mailto:jcmvb...@gmail.com]
> >> Ok, looks like what happens in my case is that instruction that
> >> sets CCOMPARE and thus changes remaining icount does not
> >> cause exit from the cpu_exec. So merely ending TB on
> >> QEMU_CLOCK_VIRTUAL timer update is not enough, I need to
> >> throw an exception of some kind? Or does the timer code need
> >> to take care of that?
> >
> > Yes, it seems that you should end the block with an exception,
> > to allow icount loop recalculate the timeouts.
> 
> Really? The ARM translate.c doesn't generate an exception.
> It just does
>  gen_io_end();
>  gen_lookup_tb();
> 
> (so we force a lookup of the next TB, but don't throw an
> exception of any kind).

Maybe I missing something. As far as I understand, changing the virtual timer 
notifies the iothread and os_host_main_loop_wait kicks the CPU thread.

But within that period of time before changing the timer and kicking the thread
CPU may proceed some instructions and the timer will be expired if it was set
to one of the soonest instructions.

Pavel Dovgalyuk




Re: [Qemu-devel] [PATCH v2] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Dr. David Alan Gilbert
* Thomas Huth (th...@redhat.com) wrote:
> When running certain HMP commands ("info registers", "info cpustats",
> "nmi", "memsave" or dumping virtual memory) with the "none" machine,
> QEMU crashes with a segmentation fault. This happens because the "none"
> machine does not have any CPUs by default, but these HMP commands did
> not check for a valid CPU pointer yet. Add such checks now, so we get
> an error message about the missing CPU instead.
> 
> Signed-off-by: Thomas Huth 
> ---
>  v2:
>  - Added more checks to cover "nmi" and "memsave", too
> 
>  hmp.c |  8 +++-
>  monitor.c | 37 +++--
>  2 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index b869617..b1c503a 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1013,8 +1013,14 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
>  const char *filename = qdict_get_str(qdict, "filename");
>  uint64_t addr = qdict_get_int(qdict, "val");
>  Error *err = NULL;
> +int cpu_index = monitor_get_cpu_index();
>  
> -qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err);
> +if (cpu_index < 0) {
> +monitor_printf(mon, "No CPU available\n");
> +return;
> +}

OK, that includes UNASSIGNED_CPU_INDEX.

> +
> +qmp_memsave(addr, size, filename, true, cpu_index, &err);
>  hmp_handle_error(mon, &err);
>  }
>  
> diff --git a/monitor.c b/monitor.c
> index 0841d43..74843eb 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
>  CPUState *mon_get_cpu(void)
>  {
>  if (!cur_mon->mon_cpu) {
> +if (!first_cpu) {
> +return NULL;
> +}
>  monitor_set_cpu(first_cpu->cpu_index);
>  }
>  cpu_synchronize_state(cur_mon->mon_cpu);
> @@ -1033,17 +1036,27 @@ CPUState *mon_get_cpu(void)
>  
>  CPUArchState *mon_get_cpu_env(void)
>  {
> -return mon_get_cpu()->env_ptr;
> +CPUState *cs = mon_get_cpu();
> +
> +return cs ? cs->env_ptr : NULL;
>  }
>  
>  int monitor_get_cpu_index(void)
>  {
> -return mon_get_cpu()->cpu_index;
> +CPUState *cs = mon_get_cpu();
> +
> +return cs ? cs->cpu_index : -1;
>  }

OK, do you think that should use UNASSIGNED_CPU_INDEX
explicitly rather than -1 ?

>  
>  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>  {
> -cpu_dump_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, 
> CPU_DUMP_FPU);
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs) {
> +monitor_printf(mon, "No CPU available\n");
> +return;
> +}
> +cpu_dump_state(cs, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
>  }
>  
>  static void hmp_info_jit(Monitor *mon, const QDict *qdict)
> @@ -1076,7 +1089,13 @@ static void hmp_info_history(Monitor *mon, const QDict 
> *qdict)
>  
>  static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>  {
> -cpu_dump_statistics(mon_get_cpu(), (FILE *)mon, &monitor_fprintf, 0);
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs) {
> +monitor_printf(mon, "No CPU available\n");
> +return;
> +}
> +cpu_dump_statistics(cs, (FILE *)mon, &monitor_fprintf, 0);
>  }
>  
>  static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
> @@ -1235,6 +1254,12 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  int l, line_size, i, max_digits, len;
>  uint8_t buf[16];
>  uint64_t v;
> +CPUState *cs = mon_get_cpu();
> +
> +if (!cs && (format == 'i' || !is_physical)) {
> +monitor_printf(mon, "Can not dump without CPU\n");
> +return;
> +}
>  
>  if (format == 'i') {
>  int flags = 0;
> @@ -1264,7 +1289,7 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  flags = msr_le << 16;
>  flags |= env->bfd_mach;
>  #endif
> -monitor_disas(mon, mon_get_cpu(), addr, count, is_physical, flags);
> +monitor_disas(mon, cs, addr, count, is_physical, flags);
>  return;
>  }
>  
> @@ -1303,7 +1328,7 @@ static void memory_dump(Monitor *mon, int count, int 
> format, int wsize,
>  if (is_physical) {
>  cpu_physical_memory_read(addr, buf, l);
>  } else {
> -if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) {
> +if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
>  monitor_printf(mon, " Cannot access memory\n");
>  break;
>  }
> -- 
> 1.8.3.1


Reviewed-by: Dr. David Alan Gilbert 

I'm sure we'll find loads more similar cases where -M none breaks stuff.

(I see you've cc'd to trivial so I'll let them take it).

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [virtio-dev] Re: [PATCH v15 0/2] virtio-crypto: virtio crypto device specification

2017-01-12 Thread Gonglei (Arei)
Hi,


> 
> On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> > Hi all,
> >
> > I attach the diff files between v14 and v15 for better review.
> >
> Hi,
> 
> only had a quick look. Will try to come back to this later.
> 
That's cool.

> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > index 9f7faf0..884ee95 100644
> > --- a/virtio-crypto.tex
> > +++ b/virtio-crypto.tex
> > @@ -2,8 +2,8 @@
> >
> >  The virtio crypto device is a virtual cryptography device as well as a 
> > kind of
> >  virtual hardware accelerator for virtual machines. The encryption and
> > -decryption requests are placed in the data queue and are ultimately handled
> by the
> > -backend crypto accelerators. The second queue is the control queue used to
> create
> > +decryption requests are placed in any of the data active queues and are
> ultimately handled by the
> s/data active/active data/
> > +backend crypto accelerators. The second kind of queue is the control queue
> used to create
> >  or destroy sessions for symmetric algorithms and will control some
> advanced
> >  features in the future. The virtio crypto device provides the following 
> > crypto
> >  services: CIPHER, MAC, HASH, and AEAD.
> 
> [..]
> 
> > ===The below diff shows the changes of add non-session mode
> support:
> >
> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > index 884ee95..44819f9 100644
> > --- a/virtio-crypto.tex
> > +++ b/virtio-crypto.tex
> > @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
> >
> >  \subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> >
> > -None currently defined.
> > +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is available
> for CIPHER service.
> > +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is available for
> HASH service.
> > +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is available for
> MAC service.
> > +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is available for
> AEAD service.
> >
> >  \subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> >
> > @@ -208,6 +211,9 @@ Operation parameters are algorithm-specific
> parameters, output data is the
> >  data that should be utilized in operations, and input data is equal to
> >  "operation result + result data".
> >
> > +The device can support both session mode (See \ref{sec:Device Types /
> Crypto Device / Device Operation / Control Virtqueue / Session operation}) and
> non-session mode, for example,
> > +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated, the driver
> can use session mode for CIPHER service, otherwise it can only use non-session
> mode.
> > +
> 
> As far as I understand you are adding non-session mode to the mix but
> providing feature bits for session mode. Would this render the the current
> implementation non-compliant?
> 
You are right, shall we use feature bits for non-session mode for compliancy?
Or because the spec is on the fly, and some structures in the virtio_crypto.h 
need to
be modified, can we keep the compliancy completely?

Thanks,
-Gonglei




Re: [Qemu-devel] [PATCH v2] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Thomas Huth
On 12.01.2017 13:19, Dr. David Alan Gilbert wrote:
> * Thomas Huth (th...@redhat.com) wrote:
>> When running certain HMP commands ("info registers", "info cpustats",
>> "nmi", "memsave" or dumping virtual memory) with the "none" machine,
>> QEMU crashes with a segmentation fault. This happens because the "none"
>> machine does not have any CPUs by default, but these HMP commands did
>> not check for a valid CPU pointer yet. Add such checks now, so we get
>> an error message about the missing CPU instead.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  v2:
>>  - Added more checks to cover "nmi" and "memsave", too
>>
>>  hmp.c |  8 +++-
>>  monitor.c | 37 +++--
>>  2 files changed, 38 insertions(+), 7 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index b869617..b1c503a 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1013,8 +1013,14 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
>>  const char *filename = qdict_get_str(qdict, "filename");
>>  uint64_t addr = qdict_get_int(qdict, "val");
>>  Error *err = NULL;
>> +int cpu_index = monitor_get_cpu_index();
>>  
>> -qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err);
>> +if (cpu_index < 0) {
>> +monitor_printf(mon, "No CPU available\n");
>> +return;
>> +}
> 
> OK, that includes UNASSIGNED_CPU_INDEX.
> 
>> +
>> +qmp_memsave(addr, size, filename, true, cpu_index, &err);
>>  hmp_handle_error(mon, &err);
>>  }
>>  
>> diff --git a/monitor.c b/monitor.c
>> index 0841d43..74843eb 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
>>  CPUState *mon_get_cpu(void)
>>  {
>>  if (!cur_mon->mon_cpu) {
>> +if (!first_cpu) {
>> +return NULL;
>> +}
>>  monitor_set_cpu(first_cpu->cpu_index);
>>  }
>>  cpu_synchronize_state(cur_mon->mon_cpu);
>> @@ -1033,17 +1036,27 @@ CPUState *mon_get_cpu(void)
>>  
>>  CPUArchState *mon_get_cpu_env(void)
>>  {
>> -return mon_get_cpu()->env_ptr;
>> +CPUState *cs = mon_get_cpu();
>> +
>> +return cs ? cs->env_ptr : NULL;
>>  }
>>  
>>  int monitor_get_cpu_index(void)
>>  {
>> -return mon_get_cpu()->cpu_index;
>> +CPUState *cs = mon_get_cpu();
>> +
>> +return cs ? cs->cpu_index : -1;
>>  }
> 
> OK, do you think that should use UNASSIGNED_CPU_INDEX
> explicitly rather than -1 ?

I wasn't aware of the fact that we've even got a macro for this ... I'll
send a v3 with that change.

> Reviewed-by: Dr. David Alan Gilbert 

Thanks for the review!

> I'm sure we'll find loads more similar cases where -M none breaks stuff.

I've added two more cases (migration and gdbstub) to the "Potentially
easy bugs" section on http://qemu-project.org/BiteSizedTasks now. I
think these are simple and easy tasks to get started with QEMU hacking...

 Thomas






Re: [Qemu-devel] [PATCH 0/5] Use non-blacklisted family/model/stepping for Haswell CPU model

2017-01-12 Thread Eduardo Habkost
On Mon, Jan 09, 2017 at 11:35:54AM +, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabk...@redhat.com) wrote:
> > A recent glibc commit[1] added a blacklist to ensure it won't use
> > TSX on hosts that are known to have a broken TSX implementation.
> > 
> > Our existing Haswell CPU model has a blacklisted
> > family/model/stepping combination, so it has to be updated to
> > make sure guests will really use TSX. This is done by patch 5/5.
> > 
> > However, to do this safely we need to ensure the host CPU is not
> > a blacklisted one, so we won't mislead guests by exposing
> > known-to-be-good FMS values on a known-to-be-broken host. This is
> > done by patch 3/5.
> 
> I'd just like to mke sure I understand the way this will fail in a migration;
> lets say we have a guest that doesn't have the new libc and hosts
> with a blacklisted CPU, and -cpu Haswell.
> 
> If I understand correctly then:
>   a) With 'enforce' the destination qemu will fail to start
>  printing an error about the host lack of tsx feature.

Yes.

>   b) Without 'enforce' the destination will start but print 
>  the same error as a warning, but the guest will probably
>  break as soon as it tries to use a tsx feature?

Yes. The general rule is: without "enforce", live migration can
break in unpredictable ways.

Without "enforce", QEMU will print a warning, and the VCPU will
run _without_ the TSX features on CPUID. If we're live-migrating,
it may break the guest if it tries to use a TSX feature, or break
migration if a TSX-related bit is already set on a MSR.


> 
> Any other combination?
> 
> Dave
> 
> > [1] 
> > https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2064f5aa15c0ceb6359
> > 
> > ---
> > Cc: dgilb...@redhat.com
> > Cc: fwei...@redhat.com
> > Cc: car...@redhat.com
> > Cc: trie...@redhat.com
> > Cc: berra...@redhat.com
> > Cc: jdene...@redhat.com
> > Cc: pbonz...@redhat.com
> > 
> > Eduardo Habkost (5):
> >   i386: Add explicit array size to x86_cpu_vendor_words2str()
> >   i386: host_vendor_fms() helper function
> >   i386/kvm: Blacklist TSX on known broken hosts
> >   pc: Add 2.9 machine-types
> >   i386: Change stepping of Haswell to non-blacklisted value
> > 
> >  include/hw/i386/pc.h |  6 ++
> >  target/i386/cpu.h|  1 +
> >  hw/i386/pc_piix.c| 15 ---
> >  hw/i386/pc_q35.c | 13 +++--
> >  target/i386/cpu.c| 32 ++--
> >  target/i386/kvm.c| 17 +
> >  6 files changed, 69 insertions(+), 15 deletions(-)
> > 
> > -- 
> > 2.11.0.259.g40922b1
> > 
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

-- 
Eduardo



Re: [Qemu-devel] [PATCH 4/5] pc: Add 2.9 machine-types

2017-01-12 Thread Eduardo Habkost
On Tue, Jan 10, 2017 at 06:06:33AM +0200, Michael S. Tsirkin wrote:
> On Sun, Jan 08, 2017 at 05:40:40PM -0200, Eduardo Habkost wrote:
> > Cc: "Michael S. Tsirkin" 
> > Cc: Laszlo Ersek 
> > Cc: Igor Mammedov 
> > Signed-off-by: Eduardo Habkost 
> 
> Do I understand it correctly that you are merging this through
> another tree?

I will apply it when including this series only if it is not
merged through another tree first. I believe multiple series
depend on adding the pc-2.9 machine-types. Feel free to apply it
to your tree if you want to.

> 
> In that case
> 
> Reviewed-by: Michael S. Tsirkin 

Thanks!

> 
> 
> > ---
> > Changes v1 -> v2:
> > * Added extra backslash to PC_COMPAT_2_8 definition
> >   * Suggested-by: Laszlo Ersek 
> > ---
> >  include/hw/i386/pc.h |  2 ++
> >  hw/i386/pc_piix.c| 15 ---
> >  hw/i386/pc_q35.c | 13 +++--
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index b22e699c46..230e9e70c5 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -375,6 +375,8 @@ int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >  
> >  #define PC_COMPAT_2_8 \
> > +HW_COMPAT_2_8 \
> > +
> >  
> >  #define PC_COMPAT_2_7 \
> >  HW_COMPAT_2_7 \
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 5e1adbe53c..9f102aa388 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m)
> >  m->default_display = "std";
> >  }
> >  
> > -static void pc_i440fx_2_8_machine_options(MachineClass *m)
> > +static void pc_i440fx_2_9_machine_options(MachineClass *m)
> >  {
> >  pc_i440fx_machine_options(m);
> >  m->alias = "pc";
> >  m->is_default = 1;
> >  }
> >  
> > +DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
> > +  pc_i440fx_2_9_machine_options);
> > +
> > +static void pc_i440fx_2_8_machine_options(MachineClass *m)
> > +{
> > +pc_i440fx_2_9_machine_options(m);
> > +m->is_default = 0;
> > +m->alias = NULL;
> > +SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
> > +}
> > +
> >  DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
> >pc_i440fx_2_8_machine_options);
> >  
> > @@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL,
> >  static void pc_i440fx_2_7_machine_options(MachineClass *m)
> >  {
> >  pc_i440fx_2_8_machine_options(m);
> > -m->is_default = 0;
> > -m->alias = NULL;
> >  SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
> >  }
> >  
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index d042fe0843..dd792a8547 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m)
> >  m->max_cpus = 288;
> >  }
> >  
> > -static void pc_q35_2_8_machine_options(MachineClass *m)
> > +static void pc_q35_2_9_machine_options(MachineClass *m)
> >  {
> >  pc_q35_machine_options(m);
> >  m->alias = "q35";
> >  }
> >  
> > +DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
> > +   pc_q35_2_9_machine_options);
> > +
> > +static void pc_q35_2_8_machine_options(MachineClass *m)
> > +{
> > +pc_q35_2_9_machine_options(m);
> > +m->alias = NULL;
> > +SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
> > +}
> > +
> >  DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL,
> > pc_q35_2_8_machine_options);
> >  
> >  static void pc_q35_2_7_machine_options(MachineClass *m)
> >  {
> >  pc_q35_2_8_machine_options(m);
> > -m->alias = NULL;
> >  m->max_cpus = 255;
> >  SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
> >  }
> > -- 
> > 2.11.0.259.g40922b1

-- 
Eduardo



Re: [Qemu-devel] [PATCH 0/5] Use non-blacklisted family/model/stepping for Haswell CPU model

2017-01-12 Thread Dr. David Alan Gilbert
* Eduardo Habkost (ehabk...@redhat.com) wrote:
> On Mon, Jan 09, 2017 at 11:35:54AM +, Dr. David Alan Gilbert wrote:
> > * Eduardo Habkost (ehabk...@redhat.com) wrote:
> > > A recent glibc commit[1] added a blacklist to ensure it won't use
> > > TSX on hosts that are known to have a broken TSX implementation.
> > > 
> > > Our existing Haswell CPU model has a blacklisted
> > > family/model/stepping combination, so it has to be updated to
> > > make sure guests will really use TSX. This is done by patch 5/5.
> > > 
> > > However, to do this safely we need to ensure the host CPU is not
> > > a blacklisted one, so we won't mislead guests by exposing
> > > known-to-be-good FMS values on a known-to-be-broken host. This is
> > > done by patch 3/5.
> > 
> > I'd just like to mke sure I understand the way this will fail in a 
> > migration;
> > lets say we have a guest that doesn't have the new libc and hosts
> > with a blacklisted CPU, and -cpu Haswell.
> > 
> > If I understand correctly then:
> >   a) With 'enforce' the destination qemu will fail to start
> >  printing an error about the host lack of tsx feature.
> 
> Yes.
> 
> >   b) Without 'enforce' the destination will start but print 
> >  the same error as a warning, but the guest will probably
> >  break as soon as it tries to use a tsx feature?
> 
> Yes. The general rule is: without "enforce", live migration can
> break in unpredictable ways.
> 
> Without "enforce", QEMU will print a warning, and the VCPU will
> run _without_ the TSX features on CPUID. If we're live-migrating,
> it may break the guest if it tries to use a TSX feature, or break
> migration if a TSX-related bit is already set on a MSR.

OK, but you've been telling people to use "enforce" long enough that
they should have listened.

Are there any other cases we have to worry about;  lets say a VM with the
new libc being migrated from an older QEMU, it suddenly changes
CPU ID to one that's supported; what happens?
I'm hoping the guest CPU ID is preserved with the TSX disabled until
a reboot?

Dave

> 
> > 
> > Any other combination?
> > 
> > Dave
> > 
> > > [1] 
> > > https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2064f5aa15c0ceb6359
> > > 
> > > ---
> > > Cc: dgilb...@redhat.com
> > > Cc: fwei...@redhat.com
> > > Cc: car...@redhat.com
> > > Cc: trie...@redhat.com
> > > Cc: berra...@redhat.com
> > > Cc: jdene...@redhat.com
> > > Cc: pbonz...@redhat.com
> > > 
> > > Eduardo Habkost (5):
> > >   i386: Add explicit array size to x86_cpu_vendor_words2str()
> > >   i386: host_vendor_fms() helper function
> > >   i386/kvm: Blacklist TSX on known broken hosts
> > >   pc: Add 2.9 machine-types
> > >   i386: Change stepping of Haswell to non-blacklisted value
> > > 
> > >  include/hw/i386/pc.h |  6 ++
> > >  target/i386/cpu.h|  1 +
> > >  hw/i386/pc_piix.c| 15 ---
> > >  hw/i386/pc_q35.c | 13 +++--
> > >  target/i386/cpu.c| 32 ++--
> > >  target/i386/kvm.c| 17 +
> > >  6 files changed, 69 insertions(+), 15 deletions(-)
> > > 
> > > -- 
> > > 2.11.0.259.g40922b1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 
> -- 
> Eduardo
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PATCH v3] monitor: Fix crashes when using HMP commands without CPU

2017-01-12 Thread Thomas Huth
When running certain HMP commands ("info registers", "info cpustats",
"nmi", "memsave" or dumping virtual memory) with the "none" machine,
QEMU crashes with a segmentation fault. This happens because the "none"
machine does not have any CPUs by default, but these HMP commands did
not check for a valid CPU pointer yet. Add such checks now, so we get
an error message about the missing CPU instead.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Thomas Huth 
---
 v3:
 - Use UNASSIGNED_CPU_INDEX instead of hard-coded -1
 v2:
 - Added more checks to cover "nmi" and "memsave", too

 hmp.c |  8 +++-
 monitor.c | 37 +++--
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/hmp.c b/hmp.c
index b869617..b1c503a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1013,8 +1013,14 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
 const char *filename = qdict_get_str(qdict, "filename");
 uint64_t addr = qdict_get_int(qdict, "val");
 Error *err = NULL;
+int cpu_index = monitor_get_cpu_index();
 
-qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err);
+if (cpu_index < 0) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+
+qmp_memsave(addr, size, filename, true, cpu_index, &err);
 hmp_handle_error(mon, &err);
 }
 
diff --git a/monitor.c b/monitor.c
index 0841d43..17121ff 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
 CPUState *mon_get_cpu(void)
 {
 if (!cur_mon->mon_cpu) {
+if (!first_cpu) {
+return NULL;
+}
 monitor_set_cpu(first_cpu->cpu_index);
 }
 cpu_synchronize_state(cur_mon->mon_cpu);
@@ -1033,17 +1036,27 @@ CPUState *mon_get_cpu(void)
 
 CPUArchState *mon_get_cpu_env(void)
 {
-return mon_get_cpu()->env_ptr;
+CPUState *cs = mon_get_cpu();
+
+return cs ? cs->env_ptr : NULL;
 }
 
 int monitor_get_cpu_index(void)
 {
-return mon_get_cpu()->cpu_index;
+CPUState *cs = mon_get_cpu();
+
+return cs ? cs->cpu_index : UNASSIGNED_CPU_INDEX;
 }
 
 static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 {
-cpu_dump_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
+CPUState *cs = mon_get_cpu();
+
+if (!cs) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+cpu_dump_state(cs, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
 }
 
 static void hmp_info_jit(Monitor *mon, const QDict *qdict)
@@ -1076,7 +1089,13 @@ static void hmp_info_history(Monitor *mon, const QDict 
*qdict)
 
 static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
 {
-cpu_dump_statistics(mon_get_cpu(), (FILE *)mon, &monitor_fprintf, 0);
+CPUState *cs = mon_get_cpu();
+
+if (!cs) {
+monitor_printf(mon, "No CPU available\n");
+return;
+}
+cpu_dump_statistics(cs, (FILE *)mon, &monitor_fprintf, 0);
 }
 
 static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
@@ -1235,6 +1254,12 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 int l, line_size, i, max_digits, len;
 uint8_t buf[16];
 uint64_t v;
+CPUState *cs = mon_get_cpu();
+
+if (!cs && (format == 'i' || !is_physical)) {
+monitor_printf(mon, "Can not dump without CPU\n");
+return;
+}
 
 if (format == 'i') {
 int flags = 0;
@@ -1264,7 +1289,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 flags = msr_le << 16;
 flags |= env->bfd_mach;
 #endif
-monitor_disas(mon, mon_get_cpu(), addr, count, is_physical, flags);
+monitor_disas(mon, cs, addr, count, is_physical, flags);
 return;
 }
 
@@ -1303,7 +1328,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 if (is_physical) {
 cpu_physical_memory_read(addr, buf, l);
 } else {
-if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) {
+if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
 monitor_printf(mon, " Cannot access memory\n");
 break;
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH v5 wave 2 3/3] hw/isa/lpc_ich9: negotiate SMI broadcast on pc-q35-2.9+ machine types

2017-01-12 Thread Eduardo Habkost
On Wed, Jan 11, 2017 at 06:35:28PM +0100, Laszlo Ersek wrote:
> Cc: "Michael S. Tsirkin" 
> Cc: Eduardo Habkost 
> Cc: Gerd Hoffmann 
> Cc: Igor Mammedov 
> Cc: Paolo Bonzini 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> v5:
> - replace the v4 patch "hw/i386/pc_q35: advertise broadcast SMI if VCPU
>   hotplug is turned off" with a simple device property and compat
>   setting [Igor]
> 
>  include/hw/i386/pc.h | 5 +
>  hw/isa/lpc_ich9.c| 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 230e9e70c504..fb8ca7c907f6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -376,6 +376,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
> *);
>  
>  #define PC_COMPAT_2_8 \
>  HW_COMPAT_2_8 \
> +{\
> +.driver   = "ICH9-LPC",\
> +.property = "smi_broadcast",\
> +.value= "off",\
> +},\
>  
>  
>  #define PC_COMPAT_2_7 \
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index ced6f803a4f2..27fae5144744 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -776,6 +776,8 @@ static const VMStateDescription vmstate_ich9_lpc = {
>  
>  static Property ich9_lpc_properties[] = {
>  DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
> +DEFINE_PROP_BIT64("smi_broadcast", ICH9LPCState, smi_host_features,
> +  ICH9_LPC_SMI_F_BROADCAST_BIT, true),

Please use hyphens instead of underscores on QOM property names.

Also, if this is not supposed to be configured directly by the
user, please use a "x-" prefix.

With that fixed:

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 2/3] run_tests: put logs into per-test file

2017-01-12 Thread Paolo Bonzini


On 12/01/2017 04:36, Peter Xu wrote:
> We were using test.log before to keep all the test logs. This patch
> creates one log file per test case under logs/ directory with name
> "TESTNAME.log". Meanwhile, we will keep the last time log into
> logs.old/.
> 
> Renaming scripts/functions.bash into scripts/common.bash to store some
> more global variables.
> 
> Signed-off-by: Peter Xu 
> ---
>  .gitignore  |  3 ++-
>  Makefile|  5 ++---
>  run_tests.sh| 19 ---
>  scripts/{functions.bash => common.bash} | 13 +++--
>  scripts/mkstandalone.sh |  2 +-
>  5 files changed, 28 insertions(+), 14 deletions(-)
>  rename scripts/{functions.bash => common.bash} (75%)
> 
> diff --git a/.gitignore b/.gitignore
> index 3155418..2213b9b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -12,7 +12,8 @@ cscope.*
>  /lib/asm
>  /config.mak
>  /*-run
> -/test.log
>  /msr.out
>  /tests
>  /build-head
> +/logs/
> +/logs.old/
> diff --git a/Makefile b/Makefile
> index a32333b..844bacc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,9 +94,8 @@ libfdt_clean:
>   $(LIBFDT_objdir)/.*.d
>  
>  distclean: clean libfdt_clean
> - $(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> -   build-head
> - $(RM) -r tests
> + $(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> + $(RM) -r tests logs logs.old
>  
>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) 
> lib/asm-generic
>  cscope:
> diff --git a/run_tests.sh b/run_tests.sh
> index 2cfa365..afd3d95 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
>  exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  function usage()
>  {
> @@ -46,17 +46,22 @@ while getopts "g:hv" opt; do
>  esac
>  done
>  
> -RUNTIME_log_stderr () { cat >> test.log; }
> +# RUNTIME_log_file will be configured later
> +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
>  RUNTIME_log_stdout () {
>  if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> -./scripts/pretty_print_stacks.py $1 >> test.log
> +./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
>  else
> -cat >> test.log
> +cat >> $RUNTIME_log_file
>  fi
>  }
>  
> -
>  config=$TEST_DIR/unittests.cfg
> -rm -f test.log
> -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> +
> +rm -rf $unittest_log_dir.old
> +[ -d $unittest_log_dir ] && mv $unittest_log_dir $unittest_log_dir.old
> +mkdir $unittest_log_dir || exit 2
> +
> +echo "BUILD_HEAD=$(cat build-head)" > $unittest_log_dir/SUMMARY
> +
>  for_each_unittest $config run
> diff --git a/scripts/functions.bash b/scripts/common.bash
> similarity index 75%
> rename from scripts/functions.bash
> rename to scripts/common.bash
> index ee9143c..2dd7360 100644
> --- a/scripts/functions.bash
> +++ b/scripts/common.bash
> @@ -1,3 +1,12 @@
> +: ${unittest_log_dir:=logs}
> +
> +function run_task()
> +{
> + local testname="$2"
> +
> + RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
> + "$@"
> +}

This should go in run_tests.sh since that's the only place that uses
unittest_log_dir.  (The last line then becomes 'run "$@"' and the
for_each_unittest call must be adjusted).  Done and queued.

Paolo

>  function for_each_unittest()
>  {
> @@ -17,7 +26,7 @@ function for_each_unittest()
>  
>   while read -u $fd line; do
>   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" 
> "$arch" "$check" "$accel" "$timeout"
> + run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> "$opts" "$arch" "$check" "$accel" "$timeout"
>   testname=${BASH_REMATCH[1]}
>   smp=1
>   kernel=""
> @@ -45,6 +54,6 @@ function for_each_unittest()
>   timeout=${BASH_REMATCH[1]}
>   fi
>   done
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
> "$accel" "$timeout"
> + run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
> "$check" "$accel" "$timeout"
>   exec {fd}<&-
>  }
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index d2bae19..3c1938e 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -5,7 +5,7 @@ if [ ! -f config.mak ]; then
>   exit 1
>  fi
>  source config.mak
> -source scripts/functions.bash
> +source scripts/common.bash
>  
>  escape ()
>  {
> 




Re: [Qemu-devel] [PATCH 0/6] ppc: add a IBM 40p machine (RS/6000, PReP)

2017-01-12 Thread Hervé Poussineau

Le 11/01/2017 à 17:58, Artyom Tarasenko a écrit :

Hi Hervé,

nice work!

On Thu, Dec 29, 2016 at 11:12 PM, Hervé Poussineau  wrote:

Hi,

This patchset adds the emulation of the IBM RS/6000 7020 (40p). The real 
machine is
able to run AIX (up to 4.3.3), Windows NT (up to 4.0 SP1), the beta of OS/2 
PowerPC,
Solaris, Linux, NetBSD/PReP ...

I've tested current emulation with Open Hack'Ware, OpenBIOS and official 
firmware.

Linux kernel starts, and freezes during boot (like with 'prep' machine).


I already saw a regression during 2.7.0 cycle with 603 CPU. However, I was 
unable to provide kernel source, so Benjamin was unable to find the problem.
http://lists.gnu.org/archive/html/qemu-devel/2016-08/msg03760.html



If prep can't do it anymore, it looks like a regression. I definitely
remember seen a sitting penguin and a login prompt ~ 2 years ago. At
least with OFW.


Windows NT starts up to the point where it wants to change endianness.


I hit that with Solaris/PPC a few years back as you published your
previous attempt. Do you know what is missing? I guess CPU endianness
switch emulation is working because it is used in the newer POWER
CPUs. Is it just the systemIO which has to be improved, or is it more?


Yes, PReP System I/O has LE flag which is not implemented.
You may be interested by 
ftp://ftp.software.ibm.com/rs6000/technology/spec/endian.ps
which deals about endianness switching, with some code from Windows NT/PPC




Other OSes have not been tested.

This machine is a superset of the 'prep' one, because we know exactly what 
is/should
emulated, and that operating system list running on it is quite wide.
I hope that 'prep' machine can be deprecated soon and then later removed.


Would be nice to keep 'prep' until the 40p can boot Linux and NetBSD
6.1.3 (this version used to work with -M prep last time I checked).


Some Linux kernels seem to work, some other ones seem to not work (hang while 
booting)
I've not searched why.

I tried NetBSD 6.1.3/PReP.
cdroms/harddisks don't boot anymore with Open Hack'Ware since some changes in 
IDE core
Kernel boots better with 40p than with prep


prep:

Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
2006, 2007, 2008, 2009, 2010, 2011, 2012
The NetBSD Foundation, Inc.  All rights reserved.
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.

NetBSD 6.1.3 (INSTALL)
Model: Qemu
total memory = 128 MB
avail memory = 119 MB
panic: call to null-ptr from 0x0

The operating system has halted.
Please press any key to reboot.


40p/Open Hack'Ware:

Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
2006, 2007, 2008, 2009, 2010, 2011, 2012
The NetBSD Foundation, Inc.  All rights reserved.
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.

NetBSD 6.1.3 (INSTALL)
Model: Qemu
total memory = 128 MB
avail memory = 119 MB
mainbus0 (root)
cpu0 at mainbus0: 604 (Revision 1.3), ID 0 (primary)
cpu0: HID0 0xc084, powersave: 1
cpu0: 0.00 MHz
Couldn't find PNP data for bus 0 devfunc 0x0
pnpbus0 at mainbus0
pci0 at mainbus0 bus 0: indirect configuration space access
pchb0 at pci0 dev 0 function 0
pchb0: vendor 0x1057 product 0x4801 (rev. 0x00)
siop0 at pci0 dev 1 function 0: Symbios Logic 53c810 (fast scsi)
siop0: couldn't map interrupt
vga0 at pci0 dev 2 function 0: vendor 0x1234 product 0x (rev. 0x02)
wsdisplay0 at vga0 (kbdmux ignored)
drm at vga0 not configured
pcn0 at pci0 dev 3 function 0: AMD PCnet-PCI Ethernet
pcn0: Am79c970A PCnet-PCI II rev 0, Ethernet address 52:54:00:12:34:56
pcn0: unable to map interrupt
pcib0 at pci0 dev 11 function 0: vendor 0x8086 product 0x0484 (rev. 0x03)
isa0 at pcib0
com0 at isa0 port 0x3f8-0x3ff irq 4: ns16550a, working fifo
com0: console
com1 at isa0 port 0x2f8-0x2ff irq 3: ns16550a, working fifo
pckbc0 at isa0 port 0x60-0x64
pckbd0 at pckbc0 (kbd slot)
pckbc0: using irq 1 for kbd slot
wskbd0 at pckbd0 (mux ignored)
vmmask 1000 schedmask 1000 highmask 7000
boot device: mainbus0
root on md0a dumps on md0b
root file system type: ffs
WARNING: no TOD clock present
WARNING: using filesystem time
WARNING: CHECK AND RESET THE DATE!
erase ^H, werase ^W, kill ^U, intr ^C, status ^T
Terminal type? [vt100]


40p/official firmware with cdrom boot:

Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
2006, 2007, 2008, 2009, 2010, 2011, 2012
The NetBSD Foundation, Inc.  All rights reserved.
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.

NetBSD 6.1.3 (INSTALL)
Model: IBM PPS Model 6015
total memory = 128 MB
avail memory = 119 MB
trap: kernel read DSI trap @ 0x481b4cae by 0x2a4f50 (DSISR 0x4000, err=1

Re: [Qemu-devel] [PATCH 0/5] Use non-blacklisted family/model/stepping for Haswell CPU model

2017-01-12 Thread Eduardo Habkost
On Thu, Jan 12, 2017 at 12:38:00PM +, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabk...@redhat.com) wrote:
> > On Mon, Jan 09, 2017 at 11:35:54AM +, Dr. David Alan Gilbert wrote:
> > > * Eduardo Habkost (ehabk...@redhat.com) wrote:
> > > > A recent glibc commit[1] added a blacklist to ensure it won't use
> > > > TSX on hosts that are known to have a broken TSX implementation.
> > > > 
> > > > Our existing Haswell CPU model has a blacklisted
> > > > family/model/stepping combination, so it has to be updated to
> > > > make sure guests will really use TSX. This is done by patch 5/5.
> > > > 
> > > > However, to do this safely we need to ensure the host CPU is not
> > > > a blacklisted one, so we won't mislead guests by exposing
> > > > known-to-be-good FMS values on a known-to-be-broken host. This is
> > > > done by patch 3/5.
> > > 
> > > I'd just like to mke sure I understand the way this will fail in a 
> > > migration;
> > > lets say we have a guest that doesn't have the new libc and hosts
> > > with a blacklisted CPU, and -cpu Haswell.
> > > 
> > > If I understand correctly then:
> > >   a) With 'enforce' the destination qemu will fail to start
> > >  printing an error about the host lack of tsx feature.
> > 
> > Yes.
> > 
> > >   b) Without 'enforce' the destination will start but print 
> > >  the same error as a warning, but the guest will probably
> > >  break as soon as it tries to use a tsx feature?
> > 
> > Yes. The general rule is: without "enforce", live migration can
> > break in unpredictable ways.
> > 
> > Without "enforce", QEMU will print a warning, and the VCPU will
> > run _without_ the TSX features on CPUID. If we're live-migrating,
> > it may break the guest if it tries to use a TSX feature, or break
> > migration if a TSX-related bit is already set on a MSR.
> 
> OK, but you've been telling people to use "enforce" long enough that
> they should have listened.
> 
> Are there any other cases we have to worry about;  lets say a VM with the
> new libc being migrated from an older QEMU, it suddenly changes
> CPU ID to one that's supported; what happens?

I assume you are talking about the stepping change, when
migrating from an old QEMU to a host that is _not_ on the
blacklist. In this case, the guest won't see any changes: CPUID
family/model/stepping will be kept the same for the whole life of
the VM (even if it is shut down), thanks to the machine-type
compatibility code.

> I'm hoping the guest CPU ID is preserved with the TSX disabled until
> a reboot?

CPUID changes are effective immediately on migration. But guests
often notice the change only on the next reboot.

We could do something to make these problems less likely:
including CPUID data on the migration stream. I have considered
it on the past, but never implemented it. Maybe I should
reconsider that.

(This is another case where it would be interesting to have a
mechanism to let the destination host abort migration early: we
could make QEMU work in "enforce" mode when live-migrating, but
using the migrated CPUID data. This way CPUID changes would never
happen.)

> 
> Dave
> 
> > 
> > > 
> > > Any other combination?
> > > 
> > > Dave
> > > 
> > > > [1] 
> > > > https://sourceware.org/git/?p=glibc.git;a=commit;h=2702856bf45c82cf8e69f2064f5aa15c0ceb6359
> > > > 
> > > > ---
> > > > Cc: dgilb...@redhat.com
> > > > Cc: fwei...@redhat.com
> > > > Cc: car...@redhat.com
> > > > Cc: trie...@redhat.com
> > > > Cc: berra...@redhat.com
> > > > Cc: jdene...@redhat.com
> > > > Cc: pbonz...@redhat.com
> > > > 
> > > > Eduardo Habkost (5):
> > > >   i386: Add explicit array size to x86_cpu_vendor_words2str()
> > > >   i386: host_vendor_fms() helper function
> > > >   i386/kvm: Blacklist TSX on known broken hosts
> > > >   pc: Add 2.9 machine-types
> > > >   i386: Change stepping of Haswell to non-blacklisted value
> > > > 
> > > >  include/hw/i386/pc.h |  6 ++
> > > >  target/i386/cpu.h|  1 +
> > > >  hw/i386/pc_piix.c| 15 ---
> > > >  hw/i386/pc_q35.c | 13 +++--
> > > >  target/i386/cpu.c| 32 ++--
> > > >  target/i386/kvm.c| 17 +
> > > >  6 files changed, 69 insertions(+), 15 deletions(-)
> > > > 
> > > > -- 
> > > > 2.11.0.259.g40922b1
> > > > 
> > > --
> > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> > 
> > -- 
> > Eduardo
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

-- 
Eduardo



Re: [Qemu-devel] [PATCH v2 1/2] memory: provide common macros for mtree_print_mr()

2017-01-12 Thread Peter Xu
On Thu, Jan 12, 2017 at 12:19:21PM +0100, Paolo Bonzini wrote:
> 
> 
> On 12/01/2017 10:46, Peter Xu wrote:
> > Yes, above suggestion makes sense to me, since after all the RW
> > permissions are derived from the type of memory regions, and the type
> > itself tells more things than the RW bits. So I totally agree we can
> > replace the "RW" chars with its type directly (if no one else
> > disagree, of course).
> > 
> > While for below patch, do you want me to include it as well as a
> > standalone patch, for the purpose of refactoring
> > memory_access_is_direct()? Since IMHO it's tiny clearer and more
> > readable than before.
> 
> It is more readable, but my plan was to turn these fields into a single
> field (with bits) to speed up memory_access_is_direct.  For that we'd
> need to undo your change.  So I'm undecided.

No problem. Then I'll just ignore it and repost with above. Thanks!

-- peterx



Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property

2017-01-12 Thread Eduardo Habkost
On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote:
[...]
>  static Property fw_cfg_io_properties[] = {
>  DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1),
>  DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1),
>  DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
>   true),
> +DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots,
> +   FW_CFG_FILE_SLOTS_MIN),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error 
> **errp)

I'm not sure what is more important here: following the QOM
property name convention using "-" instead of "_", or being
consistent with the other existing properties.

In either case, we could add a "x-" prefix to indicate it is not
supposed to be configured directly by the user.

[...]
> @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = {
>  DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
>  DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
>   true),
> +DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots,
> +   FW_CFG_FILE_SLOTS_MIN),

It looks like you can add the property to the TYPE_FW_CFG parent
class instead of duplicating it on the subclasses. The existing
"dma_enabled" property could be moved there as well.

-- 
Eduardo



Re: [Qemu-devel] [PATCH] Further tidy-up on block status

2017-01-12 Thread Alex Bligh

> On 12 Jan 2017, at 07:05, Vladimir Sementsov-Ogievskiy 
>  wrote:
> 
> Yes this is better. But is it actually needed to force contexts have some 
> safe default? If context wants it may define such default without this 
> requirement.. So, should it be requirement at all?

I've changed this to:

of the file), a server MAY reply with a single block status
descriptor with *length* matching the requested length, rather than
reporting the error; in this case the context MAY mandate the
status returned.


-- 
Alex Bligh







Re: [Qemu-devel] [PATCH] Further tidy-up on block status

2017-01-12 Thread Alex Bligh

> On 12 Jan 2017, at 11:43, Vladimir Sementsov-Ogievskiy 
>  wrote:
> 
> From "NBD_OPT_LIST_META_CONTEXT (9)":
>> The server MUST either reply with an error (for instance EINVAL if the 
>> option is not supported), or reply with a list of NBD_REP_META_CONTEXT 
>> replies followed by NBD_REP_ACK.:
> NBD_REP_ERR_UNSUP for the case in braces? Should it be normal option reply 
> packet?

Thanks, fixed.

-- 
Alex Bligh







[Qemu-devel] [PULL 1/2] travis: trim out most clang builds

2017-01-12 Thread Alex Bennée
From: "Daniel P. Berrange" 

We test with both gcc and clang in order to detect cases
where clang issues warnings that gcc misses. To achieve
this though we don't need to build QEMU in multiple
different configurations. Just a single clang-on-linux
build will be sufficient, if we have an "all enabled"
config.

This cuts the number of build jobs from 21 to 16,
reducing the load imposed on shared Travis CI infra.
This will make it practical to enable jobs for other
interesting & useful configurations without DOS'ing
Travis to much.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Alex Bennée 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 9916178bf3..0706b9a1df 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,7 +4,6 @@ python:
   - "2.4"
 compiler:
   - gcc
-  - clang
 cache: ccache
 addons:
   apt:
@@ -68,6 +67,9 @@ script:
   - make -j3 && ${TEST_CMD}
 matrix:
   include:
+# Test with CLang for compile portability
+- env: CONFIG=""
+  compiler: clang
 # gprof/gcov are GCC features
 - env: CONFIG="--enable-gprof --enable-gcov --disable-pie"
   compiler: gcc
-- 
2.11.0




[Qemu-devel] [PULL 0/2] Travis updates

2017-01-12 Thread Alex Bennée
The following changes since commit b44486dfb9447c88e4b216e730adcc780190852c:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20170110-1' into 
staging (2017-01-10 14:52:34 +)

are available in the git repository at:

  https://github.com/stsquad/qemu.git tags/pull-travis-20170112-1

for you to fetch changes up to ae1a772c5607f25cbdc80063f0f5a85290434058:

  travis: add Trusty with clang stable build (2017-01-12 10:04:17 +)


A couple of fixes to reduce the matrix some more that just missed the
last iteration.


Alex Bennée (1):
  travis: add Trusty with clang stable build

Daniel P. Berrange (1):
  travis: trim out most clang builds

 .travis.yml | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
2.11.0




[Qemu-devel] [PULL 2/2] travis: add Trusty with clang stable build

2017-01-12 Thread Alex Bennée
Although we've reduced the matrix to avoid repeating clang builds we can
still add an additional clang build to use the latest stable version of
clang which will typically be available on current distros.

Signed-off-by: Alex Bennée 
---
 .travis.yml | 20 
 1 file changed, 20 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 0706b9a1df..d83e2d493b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -103,6 +103,26 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
+# Trusty build with latest stable clang
+- env: CONFIG=""
+  sudo: required
+  addons:
+  dist: trusty
+  language: generic
+  compiler: none
+  env:
+- COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
+- CONFIG="--cc=clang-3.9 --cxx=clang++-3.9"
+  before_install:
+- wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
+- sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
+- sudo apt-get update -qq
+- sudo apt-get install -qq -y clang-3.9
+- sudo apt-get build-dep -qq qemu
+- wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
+- git submodule update --init --recursive
+  before_script:
+- ./configure ${CONFIG} || cat config.log
 # Using newer GCC with sanitizers
 - addons:
 apt:
-- 
2.11.0




Re: [Qemu-devel] [PATCH v7 15/21] qapi: add qapi2texi script

2017-01-12 Thread Markus Armbruster
Marc-André Lureau  writes:

> Hi
>
> - Original Message -
>> Marc-André Lureau  writes:
>> 
>> > As the name suggests, the qapi2texi script converts JSON QAPI
>> > description into a texi file suitable for different target
>> > formats (info/man/txt/pdf/html...).
>> >
>> > It parses the following kind of blocks:
>> >
>> > Free-form:
>> >
>> >   ##
>> >   # = Section
>> >   # == Subsection
>> >   #
>> >   # Some text foo with *emphasis*
>> >   # 1. with a list
>> >   # 2. like that
>> >   #
>> >   # And some code:
>> >   # | $ echo foo
>> >   # | -> do this
>> >   # | <- get that
>> >   #
>> >   ##
>> >
>> > Symbol description:
>> >
>> >   ##
>> >   # @symbol:
>> >   #
>> >   # Symbol body ditto ergo sum. Foo bar
>> >   # baz ding.
>> >   #
>> >   # @param1: the frob to frobnicate
>> >   # @param2: #optional how hard to frobnicate
>> >   #
>> >   # Returns: the frobnicated frob.
>> >   #  If frob isn't frobnicatable, GenericError.
>> >   #
>> >   # Since: version
>> >   # Notes: notes, comments can have
>> >   #- itemized list
>> >   #- like this
>> >   #
>> >   # Example:
>> >   #
>> >   # -> { "execute": "quit" }
>> >   # <- { "return": {} }
>> >   #
>> >   ##
>> >
>> > That's roughly following the following EBNF grammar:
>> >
>> > api_comment = "##\n" comment "##\n"
>> > comment = freeform_comment | symbol_comment
>> > freeform_comment = { "# " text "\n" | "#\n" }
>> > symbol_comment = "# @" name ":\n" { member | tag_section | freeform_comment
>> > }
>> > member = "# @" name ':' [ text ] "\n" freeform_comment
>> > tag_section = "# " ( "Returns:", "Since:", "Note:", "Notes:", "Example:",
>> > "Examples:" ) [ text ]  "\n" freeform_comment
>> > text = free text with markup
>> >
>> > Note that the grammar is ambiguous: a line "# @foo:\n" can be parsed
>> > both as freeform_comment and as symbol_comment.  The actual parser
>> > recognizes symbol_comment.
>> >
>> > See docs/qapi-code-gen.txt for more details.
>> >
>> > Deficiencies:
>> 
>> Perhaps "Deficiencies and limitations".
>
> ok
>
>> 
>> > - the generated QMP documentation includes internal types
>> > - union-type support is lacking
>> 
>> "union type" (no dash).
>> 
>> > - type information is lacking in generated documentation
>> > - doc comment error message positions are imprecise, they point
>> >   to the beginning of the comment.
>> > - see other TODO/FIXME in this commit
>> 
>> Suggest: - a few minor issues, all marked TODO/FIXME in the code
>> 
>
> ok
>
>> >
>> > Signed-off-by: Marc-André Lureau 
>> > ---
>> [diffstat snipped...]
>> > diff --git a/scripts/qapi.py b/scripts/qapi.py
>> > index 3d5f9e1eaf..a92a86f428 100644
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -125,6 +125,122 @@ class QAPISemError(QAPIError):
>> > info['parent'], msg)
>> >  
>> >  
>> > +class QAPIDoc(object):
>> > +class Section(object):
>> > +def __init__(self, name=None):
>> > +# optional section name (argument/member or section name)
>> > +self.name = name
>> > +# the list of lines for this section
>> > +self.content = []
>> > +
>> > +def append(self, line):
>> > +self.content.append(line)
>> > +
>> > +def __repr__(self):
>> > +return "\n".join(self.content).strip()
>> > +
>> > +class ArgSection(Section):
>> > +pass
>> > +
>> > +def __init__(self, parser, info):
>> > +# self.parser is used to report errors with QAPIParseError.  The
>> > +# resulting error position depends on the state of the parser.
>> > +# It happens to be the beginning of the comment.  More or less
>> > +# servicable, but action at a distance.
>> > +self.parser = parser
>> > +self.info = info
>> > +self.symbol = None
>> > +self.body = QAPIDoc.Section()
>> > +# dict mapping parameter name to ArgSection
>> > +self.args = OrderedDict()
>> > +# a list of Section
>> > +self.sections = []
>> > +# the current section
>> > +self.section = self.body
>> > +# associated expression (to be set by expression parser)
>> > +self.expr = None
>> > +
>> > +def has_section(self, name):
>> > +"""Return True if we have a section with this name."""
>> > +for i in self.sections:
>> > +if i.name == name:
>> > +return True
>> > +return False
>> > +
>> > +def append(self, line):
>> > +"""Parse a comment line and add it to the documentation."""
>> > +line = line[1:]
>> > +if not line:
>> > +self._append_freeform(line)
>> > +return
>> > +
>> > +if line[0] != ' ':
>> > +raise QAPIParseError(self.parser, "Missing space after #")
>> > +line = line[1:]
>> > +
>> > +# FIXME not nice: things like '#  @foo:' and '# @foo: ' aren't
>> > +# recognized, and get silently treated

Re: [Qemu-devel] [PATCH] nvdimm acpi: fix g_array_free() with NULL pointer

2017-01-12 Thread Stefan Hajnoczi
On Wed, Jan 11, 2017 at 09:44:43AM +, Stefan Hajnoczi wrote:
> Unlike g_free(), g_array_free() does not accept a NULL pointer argument.
> The following error is logged when an nvdimm device is realized:
> 
>   GLib-CRITICAL **: g_array_free: assertion 'array' failed
> 
> Cc: Xiao Guangrong 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/acpi/nvdimm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

NACK

Guangrong pointed out this patch is masking another bug.  There seems to
be a problem with the order in which functions are called when the
device is realized.


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH 2/2] virtio: disable notifications again after poll succeeded

2017-01-12 Thread Stefan Hajnoczi
While AioContext is in polling mode virtqueue notifications are not
necessary.  Some device virtqueue handlers enable notifications.  Make
sure they stay disabled to avoid unnecessary vmexits.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f04ab7a..34065c7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2126,6 +2126,9 @@ static bool virtio_queue_host_notifier_aio_poll(void 
*opaque)
 }
 
 virtio_queue_notify_aio_vq(vq);
+
+/* In case the handler function re-enabled notifications */
+virtio_queue_set_notification(vq, 0);
 return true;
 }
 
-- 
2.9.3




[Qemu-devel] [PATCH 0/2] virtio: revert virtio_queue_set_notification() nesting

2017-01-12 Thread Stefan Hajnoczi
The virtio_queue_set_notification() nesting introduced for AioContext polling
raised an assertion with virtio-net (even in non-polling mode).  Converting
virtio-net and virtio-crypto to use virtio_queue_set_notification() in a
nesting fashion would be invasive and isn't worth it.

Patch 1 contains the revert to resolve the bug that Doug noticed.

Patch 2 is a less efficient but safe alternative.

Stefan Hajnoczi (2):
  Revert "virtio: turn vq->notification into a nested counter"
  virtio: disable notifications again after poll succeeded

 hw/virtio/virtio.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

-- 
2.9.3




Re: [Qemu-devel] [PATCH] nvdimm acpi: fix g_array_free() with NULL pointer

2017-01-12 Thread Stefan Hajnoczi
On Thu, Jan 12, 2017 at 11:18:25AM +0800, Xiao Guangrong wrote:
> 
> 
> On 01/11/2017 05:36 PM, Stefan Hajnoczi wrote:
> > Unlike g_free(), g_array_free() does not accept a NULL pointer argument.
> > The following error is logged when an nvdimm device is realized:
> > 
> >   GLib-CRITICAL **: g_array_free: assertion 'array' failed
> > 
> > Cc: Xiao Guangrong 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  hw/acpi/nvdimm.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)

NACK

> > 
> > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > index 8e7d6ec..8f0a484 100644
> > --- a/hw/acpi/nvdimm.c
> > +++ b/hw/acpi/nvdimm.c
> > @@ -375,7 +375,9 @@ static void nvdimm_init_fit_buffer(NvdimmFitBuffer 
> > *fit_buf)
> > 
> >  static void nvdimm_build_fit_buffer(NvdimmFitBuffer *fit_buf)
> >  {
> > -g_array_free(fit_buf->fit, true);
> > +if (fit_buf->fit) {
> > +g_array_free(fit_buf->fit, true);
> > +}
> 
> Er, i do not know why it is NULL as we have init-ed it in 
> nvdimm_init_fit_buffer:
> 
> static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
> {
> fit_buf->fit = g_array_new(false, true /* clear */, 1);
> }
> 
> And i can not reproduce it on my box, could you share your command line and 
> the
> based commit id?

Good point, it happens when nvdimm_plug() is called but -M pc,nvdimm is
missing from the command-line.  This means nvdimm_init_acpi_state() was
not called by pc_init1():

  $ x86_64-softmmu/qemu-system-x86_64 \
  -enable-kvm \
  -m 1G,slots=2,maxmem=16G \
  -drive if=virtio,file=test.img,format=raw \
  -object memory-backend-file,id=hostmem0,mem-path=mydimm,share=on,size=8G \
  -device nvdimm,id=nvdimm0,memdev=hostmem0

Do you want to audit the code to check if anything else misbehaves when
-device nvdimm is used without -M pc,nvdimm?

Stefan


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH 1/2] Revert "virtio: turn vq->notification into a nested counter"

2017-01-12 Thread Stefan Hajnoczi
This reverts commit aff8fd18f1786fc5af259a9bc0077727222f51ca.

Both virtio-net and virtio-crypto do not balance
virtio_queue_set_notification() enable and disable calls.  This makes
the notifications_disabled counter unreliable and Doug Goldstein
reported the following assertion failure:

  #3  0x744d1c62 in __GI___assert_fail (
  assertion=assertion@entry=0x55ae8e8a "vq->notification_disabled > 0",
  file=file@entry=0x55ae89c0 "/home/doug/work/qemu/hw/virtio/virtio.c",
  line=line@entry=215,
  function=function@entry=0x55ae9630 <__PRETTY_FUNCTION__.43707>
  "virtio_queue_set_notification") at assert.c:101
  #4  0x557f25d6 in virtio_queue_set_notification (vq=0x5666aa90,
  enable=enable@entry=1) at /home/doug/work/qemu/hw/virtio/virtio.c:215
  #5  0x557dc311 in virtio_net_has_buffers (q=,
  q=, bufsize=102)
  at /home/doug/work/qemu/hw/net/virtio-net.c:1008
  #6  virtio_net_receive (nc=, buf=0x57386b88 "", size=102)
  at /home/doug/work/qemu/hw/net/virtio-net.c:1148
  #7  0x559cad33 in nc_sendv_compat (flags=, iovcnt=1,
  iov=0x7fffead746d0, nc=0x5788b340) at net/net.c:705
  #8  qemu_deliver_packet_iov (sender=, flags=,
  iov=0x7fffead746d0, iovcnt=1, opaque=0x5788b340) at net/net.c:732
  #9  0x559cd929 in qemu_net_queue_deliver (size=,
  data=, flags=, sender=,
  queue=0x5788b550) at net/queue.c:164
  #10 qemu_net_queue_flush (queue=0x5788b550) at net/queue.c:261

This patch is safe to revert since it's just an optimization for
virtqueue polling.  The next patch will improve the situation again
without resorting to nesting.

Reported-by: Doug Goldstein 
Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aa4f38f..f04ab7a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -88,8 +88,8 @@ struct VirtQueue
 /* Last used index value we have signalled on */
 bool signalled_used_valid;
 
-/* Nested host->guest notification disabled counter */
-unsigned int notification_disabled;
+/* Notification enabled? */
+bool notification;
 
 uint16_t queue_index;
 
@@ -202,7 +202,7 @@ static inline void vring_used_flags_unset_bit(VirtQueue 
*vq, int mask)
 static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
 {
 hwaddr pa;
-if (vq->notification_disabled) {
+if (!vq->notification) {
 return;
 }
 pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
@@ -211,13 +211,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, 
uint16_t val)
 
 void virtio_queue_set_notification(VirtQueue *vq, int enable)
 {
-if (enable) {
-assert(vq->notification_disabled > 0);
-vq->notification_disabled--;
-} else {
-vq->notification_disabled++;
-}
-
+vq->notification = enable;
 if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
 vring_set_avail_event(vq, vring_avail_idx(vq));
 } else if (enable) {
@@ -1020,7 +1014,7 @@ void virtio_reset(void *opaque)
 virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
 vdev->vq[i].signalled_used = 0;
 vdev->vq[i].signalled_used_valid = false;
-vdev->vq[i].notification_disabled = 0;
+vdev->vq[i].notification = true;
 vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
 vdev->vq[i].inuse = 0;
 }
@@ -1831,7 +1825,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 vdev->vq[i].vring.desc = qemu_get_be64(f);
 qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
 vdev->vq[i].signalled_used_valid = false;
-vdev->vq[i].notification_disabled = 0;
+vdev->vq[i].notification = true;
 
 if (vdev->vq[i].vring.desc) {
 /* XXX virtio-1 devices */
-- 
2.9.3




Re: [Qemu-devel] [PATCH] Revert "win32: don't run subprocess tests on Mingw32 platform"

2017-01-12 Thread Eduardo Habkost
On Wed, Jan 04, 2017 at 09:57:22PM +0100, Marc-André Lureau wrote:
> This reverts commit 7ad9339e372fcd12d584684d7f52ac259604a4f4.
> 
> The error "Failed to execute helper program (No such file or directory)"
> is due to broken glib installation, missing windows gspawn helpers.
> 
> Signed-off-by: Marc-André Lureau 

Daniel, are you still unable to run subprocess tests on Migw32?

Anybody else able to test this and send a Tested-by line?


> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 218df87d21..54a222d5e7 100755
> --- a/configure
> +++ b/configure
> @@ -3077,7 +3077,7 @@ fi
>  
>  # g_test_trap_subprocess added in 2.38. Used by some tests.
>  glib_subprocess=yes
> -if test "$mingw32" = "yes" || ! $pkg_config --atleast-version=2.38 glib-2.0; 
> then
> +if ! $pkg_config --atleast-version=2.38 glib-2.0; then
>  glib_subprocess=no
>  fi
>  
> -- 
> 2.11.0
> 

-- 
Eduardo



Re: [Qemu-devel] [kvm-unit-tests PATCH v6 3/3] run_tests: allow run tests in parallel

2017-01-12 Thread Paolo Bonzini


On 12/01/2017 12:42, Andrew Jones wrote:
> On Thu, Jan 12, 2017 at 11:36:22AM +0800, Peter Xu wrote:
>> run_task.sh is getting slow. This patch is trying to make it faster by
>> running the tests concurrently.
>>
>> We provide a new parameter "-j" for the run_tests.sh, which can be used
>> to specify how many run queues we want for the tests. Default queue
>> length is 1, which is the old behavior.
>>
>> Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost:
>>
>>|-+---|
>>| command | time used |
>>|-+---|
>>| run_test.sh | 75s   |
>>| run_test.sh -j8 | 27s   |
>>|-+---|

wait -n is very new, so I've added "2> /dev/null" (the result will be a
busy wait).  To avoid a busy wait in the common -j1 case, I also added:

+   if [ $unittest_run_queues = 1 ]; then
+   run "$@"
+   else
+   run "$@" &
+   fi

(I tried using a named pipe too, but it's messy to open both ends
without blocking).

Paolo

>> Signed-off-by: Peter Xu 
>> ---
>>  run_tests.sh| 12 ++--
>>  scripts/common.bash | 16 +++-
>>  2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/run_tests.sh b/run_tests.sh
>> index afd3d95..4d57ff9 100755
>> --- a/run_tests.sh
>> +++ b/run_tests.sh
>> @@ -13,10 +13,11 @@ function usage()
>>  {
>>  cat <>  
>> -Usage: $0 [-g group] [-h] [-v]
>> +Usage: $0 [-g group] [-h] [-v] [-j num_run_queues]
>>  
>>  -g: Only execute tests in the given group
>>  -h: Output this help text
>> +-j: Execute tests in parallel
>>  -v: Enables verbose mode
>>  
>>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>> @@ -28,7 +29,7 @@ EOF
>>  RUNTIME_arch_run="./$TEST_DIR/run"
>>  source scripts/runtime.bash
>>  
>> -while getopts "g:hv" opt; do
>> +while getopts "g:hj:v" opt; do
>>  case $opt in
>>  g)
>>  only_group=$OPTARG
>> @@ -37,6 +38,13 @@ while getopts "g:hv" opt; do
>>  usage
>>  exit
>>  ;;
>> +j)
>> +unittest_run_queues=$OPTARG
>> +if (( $unittest_run_queues <= 0 )); then
>> +echo "Invalid -j option: $unittest_run_queues"
>> +exit 2
>> +fi
>> +;;
>>  v)
>>  verbose="yes"
>>  ;;
>> diff --git a/scripts/common.bash b/scripts/common.bash
>> index 2dd7360..9bd560f 100644
>> --- a/scripts/common.bash
>> +++ b/scripts/common.bash
>> @@ -1,11 +1,19 @@
>>  : ${unittest_log_dir:=logs}
>> +: ${unittest_run_queues:=1}
>>  
>>  function run_task()
>>  {
>>  local testname="$2"
>>  
>> +while (( $(jobs | wc -l) == $unittest_run_queues )); do
>> +# wait for any background test to finish
>> +wait -n
>> +done
>> +
>>  RUNTIME_log_file="${unittest_log_dir}/${testname}.log"
>> -"$@"
>> +
>> +# start the testcase in the background
>> +"$@" &
>>  }
>>  
>>  function for_each_unittest()
>> @@ -22,6 +30,8 @@ function for_each_unittest()
>>  local accel
>>  local timeout
>>  
>> +trap "wait; exit 130" SIGINT
>> +
>>  exec {fd}<"$unittests"
>>  
>>  while read -u $fd line; do
>> @@ -55,5 +65,9 @@ function for_each_unittest()
>>  fi
>>  done
>>  run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
>> "$check" "$accel" "$timeout"
>> +
>> +# wait until all tasks finish
>> +wait
>> +
>>  exec {fd}<&-
>>  }
>> -- 
>> 2.7.4
>>
>>
> 
> Reviewed-by: Andrew Jones  
> 



Re: [Qemu-devel] [PATCH 04/10] qemu-thread: optimize QemuLockCnt with futexes on Linux

2017-01-12 Thread Fam Zheng
On Wed, 01/04 14:26, Paolo Bonzini wrote:
> diff --git a/include/qemu/futex.h b/include/qemu/futex.h
> new file mode 100644
> index 000..852d612
> --- /dev/null
> +++ b/include/qemu/futex.h
> @@ -0,0 +1,36 @@
> +/*
> + * Wrappers around Linux futex syscall
> + *
> + * Copyright Red Hat, Inc. 2015

2015 - 2017, too?

> + *
> + * Author:
> + *  Paolo Bonzini 
> + *
> + * 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 
> +#include 
> +
> +#define qemu_futex(...)  syscall(__NR_futex, __VA_ARGS__)
> +
> +static inline void qemu_futex_wake(void *f, int n)
> +{
> +qemu_futex(f, FUTEX_WAKE, n, NULL, NULL, 0);
> +}
> +
> +static inline void qemu_futex_wait(void *f, unsigned val)
> +{
> +while (qemu_futex(f, FUTEX_WAIT, (int) val, NULL, NULL, 0)) {
> +switch (errno) {
> +case EWOULDBLOCK:
> +return;
> +case EINTR:
> +break; /* get out of switch and retry */
> +default:
> +abort();
> +}
> +}
> +}
> diff --git a/util/lockcnt.c b/util/lockcnt.c
> index 78ed1e4..40cc02a 100644
> --- a/util/lockcnt.c
> +++ b/util/lockcnt.c
> @@ -9,7 +9,288 @@
>  #include "qemu/osdep.h"
>  #include "qemu/thread.h"
>  #include "qemu/atomic.h"
> +#include "trace.h"
>  
> +#ifdef CONFIG_LINUX
> +#include "qemu/futex.h"
> +
> +/* On Linux, bits 0-1 are a futex-based lock, bits 2-31 are the counter.
> + * For the mutex algorithm see Ulrich Drepper's "Futexes Are Tricky" (ok,
> + * this is not the most relaxing citation I could make...).  It is similar
> + * to mutex2 in the paper.
> + */
> +
> +#define QEMU_LOCKCNT_STATE_MASK3
> +#define QEMU_LOCKCNT_STATE_FREE0
> +#define QEMU_LOCKCNT_STATE_LOCKED  1

I find the macro names a bit incomplete in describing the semantics but maybe
you want to limit the length, making it harder to understand the mutex
implementation without reading the paper. How about adding a comment saying
"locked" is "locked but _not waited_" and "waiting" is "_locked_ and waited"?
It's up to you, because this is trivial compared to the real complexity of this
patch. :)

> +#define QEMU_LOCKCNT_STATE_WAITING 2
> +
> +#define QEMU_LOCKCNT_COUNT_STEP4
> +#define QEMU_LOCKCNT_COUNT_SHIFT   2
> +
> +void qemu_lockcnt_init(QemuLockCnt *lockcnt)
> +{
> +lockcnt->count = 0;
> +}
> +
> +void qemu_lockcnt_destroy(QemuLockCnt *lockcnt)
> +{
> +}
> +
> +/* *val is the current value of lockcnt->count.
> + *
> + * If the lock is free, try a cmpxchg from *val to new_if_free; return
> + * true and set *val to the old value found by the cmpxchg in
> + * lockcnt->count.
> + *
> + * If the lock is taken, wait for it to be released and return false
> + * *without trying again to take the lock*.  Again, set *val to the
> + * new value of lockcnt->count.
> + *
> + * new_if_free's bottom two bits must not be QEMU_LOCKCNT_STATE_LOCKED
> + * if calling this function a second time after it has returned
> + * false.

"and waited"? I think it is possible this function return false with the lock
actually being free, when ...

> + */
> +static bool qemu_lockcnt_cmpxchg_or_wait(QemuLockCnt *lockcnt, int *val,
> + int new_if_free, bool *waited)
> +{
> +/* Fast path for when the lock is free.  */
> +if ((*val & QEMU_LOCKCNT_STATE_MASK) == QEMU_LOCKCNT_STATE_FREE) {
> +int expected = *val;
> +
> +trace_lockcnt_fast_path_attempt(lockcnt, expected, new_if_free);
> +*val = atomic_cmpxchg(&lockcnt->count, expected, new_if_free);
> +if (*val == expected) {
> +trace_lockcnt_fast_path_success(lockcnt, expected, new_if_free);
> +*val = new_if_free;
> +return true;
> +}
> +}
> +
> +/* The slow path moves from locked to waiting if necessary, then
> + * does a futex wait.  Both steps can be repeated ad nauseam,
> + * only getting out of the loop if we can have another shot at the
> + * fast path.  Once we can, get out to compute the new destination
> + * value for the fast path.
> + */
> +while ((*val & QEMU_LOCKCNT_STATE_MASK) != QEMU_LOCKCNT_STATE_FREE) {
> +if ((*val & QEMU_LOCKCNT_STATE_MASK) == QEMU_LOCKCNT_STATE_LOCKED) {
> +int expected = *val;
> +int new = expected - QEMU_LOCKCNT_STATE_LOCKED + 
> QEMU_LOCKCNT_STATE_WAITING;
> +
> +trace_lockcnt_futex_wait_prepare(lockcnt, expected, new);

... the holder thread releases the lock at this point. In this case a second
call to this function in qemu_lockcnt_dec_and_lock does pass
QEMU_LOCKCNT_STATE_LOCKED in new_if_free, because 'waited' is left false there.
The code is okay, but the comment above is too strict.

> +*val = atomic_cmpxchg(&lockcnt->count, expected, new);
> +if (*val == expected) {
> +*val = new;
> +}
> +continue;
>

Re: [Qemu-devel] [PATCH v2 0/3] fix query-memdev not repporting IDs of memory backends

2017-01-12 Thread Eduardo Habkost
On Tue, Jan 10, 2017 at 01:53:12PM +0100, Igor Mammedov wrote:
> 
> Changelog:
> since v1:
> * fix mistakes in commit messages/change them as suggested
> * extend 3/3 commit message to explain why it's ok to use 'id'
> * 3/3 set 'id' property directly instead of injecting it back
>   into qdict.
> 
> 
> Series is a couple of preparratory cleanups which simplify fix
> and a fix itself.
> 
> Before fix HMP 'info memdevs' for CLI:
> qemu-system-x86_64 -object memory-backend-ram,id=mem0,size=1G
> outputs:
> 
>   memory backend: 0
> size:  1073741824
> merge: true
> dump: true
> prealloc: false
> policy: default
> host nodes: 128
> 
> after fix:
> 
>   memory backend: mem0
> size:  1073741824
> merge: true
> dump: true
> prealloc: false
> policy: default
> host nodes: 128
> 
> it should help to avoid remembering hotplugged IDs as they
> could be queried back via HMP/QMP interface.
> 

Applied to machine-next. Thanks!

-- 
Eduardo



Re: [Qemu-devel] [PATCH 0/6] ppc: add a IBM 40p machine (RS/6000, PReP)

2017-01-12 Thread Artyom Tarasenko
On Thu, Jan 12, 2017 at 1:57 PM, Hervé Poussineau  wrote:
> Le 11/01/2017 à 17:58, Artyom Tarasenko a écrit :
>>
>> Hi Hervé,
>>
>> nice work!
>>
>> On Thu, Dec 29, 2016 at 11:12 PM, Hervé Poussineau 
>> wrote:
>>>
>>> Hi,
>>>
>>> This patchset adds the emulation of the IBM RS/6000 7020 (40p). The real
>>> machine is
>>> able to run AIX (up to 4.3.3), Windows NT (up to 4.0 SP1), the beta of
>>> OS/2 PowerPC,
>>> Solaris, Linux, NetBSD/PReP ...
>>>
>>> I've tested current emulation with Open Hack'Ware, OpenBIOS and official
>>> firmware.
>>>
>>> Linux kernel starts, and freezes during boot (like with 'prep' machine).
>
>
> I already saw a regression during 2.7.0 cycle with 603 CPU. However, I was
> unable to provide kernel source, so Benjamin was unable to find the problem.
> http://lists.gnu.org/archive/html/qemu-devel/2016-08/msg03760.html
>
>>
>> If prep can't do it anymore, it looks like a regression. I definitely
>> remember seen a sitting penguin and a login prompt ~ 2 years ago. At
>> least with OFW.
>>
>>> Windows NT starts up to the point where it wants to change endianness.
>>
>>
>> I hit that with Solaris/PPC a few years back as you published your
>> previous attempt. Do you know what is missing? I guess CPU endianness
>> switch emulation is working because it is used in the newer POWER
>> CPUs. Is it just the systemIO which has to be improved, or is it more?
>
>
> Yes, PReP System I/O has LE flag which is not implemented.
> You may be interested by
> ftp://ftp.software.ibm.com/rs6000/technology/spec/endian.ps
> which deals about endianness switching, with some code from Windows NT/PPC
>
>>
>>> Other OSes have not been tested.
>>>
>>> This machine is a superset of the 'prep' one, because we know exactly
>>> what is/should
>>> emulated, and that operating system list running on it is quite wide.
>>> I hope that 'prep' machine can be deprecated soon and then later removed.
>>
>>
>> Would be nice to keep 'prep' until the 40p can boot Linux and NetBSD
>> 6.1.3 (this version used to work with -M prep last time I checked).
>
>
> Some Linux kernels seem to work, some other ones seem to not work (hang
> while booting)
> I've not searched why.

I should have somewhere the Debian Woody Image I used. Will test it tonight.

> I tried NetBSD 6.1.3/PReP.
> cdroms/harddisks don't boot anymore with Open Hack'Ware since some changes
> in IDE core
> Kernel boots better with 40p than with prep
>
> 
> prep:
>
> Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
> 2006, 2007, 2008, 2009, 2010, 2011, 2012
> The NetBSD Foundation, Inc.  All rights reserved.
> Copyright (c) 1982, 1986, 1989, 1991, 1993
> The Regents of the University of California.  All rights reserved.
>
> NetBSD 6.1.3 (INSTALL)
> Model: Qemu
> total memory = 128 MB
> avail memory = 119 MB
> panic: call to null-ptr from 0x0
>
> The operating system has halted.
> Please press any key to reboot.
>
> 
> 40p/Open Hack'Ware:
>
> Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
> 2006, 2007, 2008, 2009, 2010, 2011, 2012
> The NetBSD Foundation, Inc.  All rights reserved.
> Copyright (c) 1982, 1986, 1989, 1991, 1993
> The Regents of the University of California.  All rights reserved.
>
> NetBSD 6.1.3 (INSTALL)
> Model: Qemu
> total memory = 128 MB
> avail memory = 119 MB
> mainbus0 (root)
> cpu0 at mainbus0: 604 (Revision 1.3), ID 0 (primary)
> cpu0: HID0 0xc084, powersave: 1
> cpu0: 0.00 MHz
> Couldn't find PNP data for bus 0 devfunc 0x0
> pnpbus0 at mainbus0
> pci0 at mainbus0 bus 0: indirect configuration space access
> pchb0 at pci0 dev 0 function 0
> pchb0: vendor 0x1057 product 0x4801 (rev. 0x00)
> siop0 at pci0 dev 1 function 0: Symbios Logic 53c810 (fast scsi)
> siop0: couldn't map interrupt
> vga0 at pci0 dev 2 function 0: vendor 0x1234 product 0x (rev. 0x02)
> wsdisplay0 at vga0 (kbdmux ignored)
> drm at vga0 not configured
> pcn0 at pci0 dev 3 function 0: AMD PCnet-PCI Ethernet
> pcn0: Am79c970A PCnet-PCI II rev 0, Ethernet address 52:54:00:12:34:56
> pcn0: unable to map interrupt
> pcib0 at pci0 dev 11 function 0: vendor 0x8086 product 0x0484 (rev. 0x03)
> isa0 at pcib0
> com0 at isa0 port 0x3f8-0x3ff irq 4: ns16550a, working fifo
> com0: console
> com1 at isa0 port 0x2f8-0x2ff irq 3: ns16550a, working fifo
> pckbc0 at isa0 port 0x60-0x64
> pckbd0 at pckbc0 (kbd slot)
> pckbc0: using irq 1 for kbd slot
> wskbd0 at pckbd0 (mux ignored)
> vmmask 1000 schedmask 1000 highmask 7000
> boot device: mainbus0
> root on md0a dumps on md0b
> root file system type: ffs
> WARNING: no TOD clock present
> WARNING: using filesystem time
> WARNING: CHECK AND RESET THE DATE!
> erase ^H, werase ^W, kill ^U, intr ^C, status ^T
> Terminal type? [vt100]
>
> 
> 40p/official firmware with cdrom boot:
>
> Copyright (c) 1996, 1997, 1

Re: [Qemu-devel] [PATCH] Revert "win32: don't run subprocess tests on Mingw32 platform"

2017-01-12 Thread Vincent Palatin
On Thu, Jan 12, 2017 at 2:25 PM, Eduardo Habkost  wrote:
> On Wed, Jan 04, 2017 at 09:57:22PM +0100, Marc-André Lureau wrote:
>> This reverts commit 7ad9339e372fcd12d584684d7f52ac259604a4f4.
>>
>> The error "Failed to execute helper program (No such file or directory)"
>> is due to broken glib installation, missing windows gspawn helpers.
>>
>> Signed-off-by: Marc-André Lureau 
>
> Daniel, are you still unable to run subprocess tests on Migw32?
>
> Anybody else able to test this and send a Tested-by line?

Works on my setup (mingw32-w64 build on Windows 10, with glib-2.46.2)

Tested-by: Vincent Palatin 

>
>
>> ---
>>  configure | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 218df87d21..54a222d5e7 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3077,7 +3077,7 @@ fi
>>
>>  # g_test_trap_subprocess added in 2.38. Used by some tests.
>>  glib_subprocess=yes
>> -if test "$mingw32" = "yes" || ! $pkg_config --atleast-version=2.38 
>> glib-2.0; then
>> +if ! $pkg_config --atleast-version=2.38 glib-2.0; then
>>  glib_subprocess=no
>>  fi
>>
>> --
>> 2.11.0
>>
>
> --
> Eduardo
>



[Qemu-devel] [PATCH v3 0/2] memory: extend "info mtree" with flat view dump

2017-01-12 Thread Peter Xu
v3:
- rather than dump flatview directly in "info mtree", provide a new
  parameter "-f" for it. [Paolo]
- replace "RW" chars with the type of memory region [Paolo]
- (cc dave as well since it touches HMP interface)

v2:
- fix a size error in patch 2
- add r-b for Marc-André in patch 1

Each address space has its own flatview. It's another way to observe
memory info besides the default memory region hierachy, for example,
if we want to know which memory region will handle the write to
specific address, a flatview will suite more here than the default
hierachical dump.

Please review. Thanks,

Peter Xu (2):
  memory: tune mtree_print_mr() to dump mr type
  memory: hmp: add "-f" for "info mtree"

 hmp-commands-info.hx  |  6 ++--
 include/exec/memory.h |  2 +-
 memory.c  | 88 ++-
 monitor.c |  4 ++-
 4 files changed, 73 insertions(+), 27 deletions(-)

-- 
2.7.4




  1   2   3   4   >