[Qemu-devel] [Bug 1095531] Re: sparc32plus (and others?) has x86 code generation errors on 64bit hosts

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  sparc32plus (and others?) has x86 code generation errors on 64bit
  hosts

Status in QEMU:
  Expired

Bug description:
  On 64bit hosts, the load and store functions compile improperly. The
  issue is the call to gen_address_mask() under the ld and st functions
  in target-sparc/translate.c. Below are some snips from the log file.
  Doing a gdb debug, this results in constant access violation errors
  which I do not see when debugging qemu in powerpc mode.

  --
  IN: 
  0x40804aa8:  st  %i0, [ %fp + 0x44 ]

  OP:
    0x40804aa8
   ld_i64 tmp1,regwptr,$0xb0
   mov_i64 tmp0,tmp1
   movi_i64 tmp2,$0x44
   add_i64 tmp0,tmp0,tmp2
   ld_i64 tmp2,regwptr,$0x80
   ext32u_i64 tmp0,tmp0
   qemu_st32 tmp2,tmp0,$0x0

  OUT: [size=345]
  0x6032d7f0:  mov0x40(%r14),%rbp
  0x6032d7f4:  mov0xb0(%rbp),%rbx
  0x6032d7fb:  add$0x44,%rbx
  0x6032d7ff:  mov0x80(%rbp),%rbp
  0x6032d806:  mov%ebx,%ebx <- bug
  0x6032d808:  mov%ebp,%edi
  0x6032d80a:  bswap  %edi
  0x6032d80c:  mov%edi,(%rbx)

  --
  IN: 
  0x40804aec:  add  %l7, %o7, %l7
  0x40804af0:  ld  [ %l7 ], %g2

  OP:
    0x40804aec
   ld_i64 tmp1,regwptr,$0x78
   ld_i64 tmp2,regwptr,$0x38
   add_i64 tmp0,tmp1,tmp2
   st_i64 tmp0,regwptr,$0x78

    0x40804af0
   ld_i64 tmp1,regwptr,$0x78
   mov_i64 tmp0,tmp1
   ext32u_i64 tmp0,tmp0
   qemu_ld32u g2,tmp0,$0x0

  OUT: [size=395]
  0x6032da80:  mov0x40(%r14),%rbp
  0x6032da84:  mov0x78(%rbp),%rbx
  0x6032da88:  mov0x38(%rbp),%r12
  0x6032da8c:  add%r12,%rbx
  0x6032da8f:  mov%rbx,0x78(%rbp)
  0x6032da93:  mov0x78(%rbp),%rbx
  0x6032da97:  mov%ebx,%ebx<- bug
  0x6032da99:  mov(%rbx),%ebx

  In 64bit mode, doing a 32bit operation will result in the top 32bit's
  being zero'd. I attempted to simply disable the call to
  gen_address_mask() but that did not fix the issue and actually caused
  the sparc32plus I was testing to become unusable.

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

[Qemu-devel] [Bug 1093360] Re: files on microsoft iso images mounted to qemu VM get stripped from Version info. E.G. Microsoft UAG installation fails

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  files on microsoft iso images mounted to qemu VM  get stripped  from
  Version info. E.G. Microsoft UAG installation fails

Status in QEMU:
  Expired

Bug description:
  QEMU 0.9.0-0.14.1
  KVM 60-88-0.14.1
  there is a reference for a root cause, why installation of Microsoft UAG 
fails.
  
http://blogs.technet.com/b/isablog/archive/2010/07/13/another-tmg-2010-installation-failure-with-error-0x80070643.aspx

  when checking available information on the mounted UAG ISO in my qemu 
machine, I realized simliar reduced information.
  this was found:
  using AQEMU 0.8.2 of 2011.07.27

  using QEMU 0.9.0-0.14.1 and KVM 60-88-0.14.1
  in an KVM managed  machine

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

[Qemu-devel] [Bug 1095857] Re: incorrect handling of [r32] address (long mode)

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  incorrect handling of [r32] address (long mode)

Status in QEMU:
  Expired

Bug description:
  while executing in Long Mode (x86-64) instructions such as

  mov eax,[r15d]

  end up executing as

  mov eax,[r15]

  according to x86 programmer manuals the behavior of using the Address-
  Size override (in long mode) is supposed to ignore the high 32bits of
  the register. I use this fact in my operating system to reduce
  register usage (the high 32 bits of r15 holds other data).
  consequently a general protection exception occurs since the memory
  address isn't "canonical". this error doesn't always appear since the
  high 32 bits might not be zero in those conditions.

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

[Qemu-devel] [Bug 1094950] Re: crash at qemu_iohandler_poll (iohandler.c:124) on macos 10.8.2

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  crash at  qemu_iohandler_poll (iohandler.c:124) on macos 10.8.2

Status in QEMU:
  Expired

Bug description:
  I'm seeing consistent hangs / crashes on MacOS 10.8.2 with 1.3.0.
  I've tried both gcc-4.2 and clang.  I've tried a half a dozen
  different images/kernels.

  I configured qemu like this:

  ./configure --disable-sdl --disable-kvm --enable-cocoa --cc=gcc-4.2
  --host-cc=gcc-4.2 --enable-debug   --extra-cflags=-g   --extra-
  ldflags=-g

  And ran it like this:

  qemu-system-arm -nographic -M versatilepb -kernel
  vmlinuz-2.6.32-5-versatile -initrd initrd.img-2.6.32-5-versatile -hda
  debian_squeeze_armel_standard.qcow2 -append "root=/dev/sda1
  console=ttyAMA0"

  With images, kernel, and initrd described here:

  http://psellos.com/2012/08/2012.08.qemu-arm-osx.html

  And I get:

  Program received signal EXC_BAD_ACCESS, Could not access memory.
  Reason: KERN_PROTECTION_FAILURE at address: 0x00010142f2d0
  0x00010142f2d0 in ?? ()

  (gdb) bt
  #0  0x00010142f2d0 in ?? ()
  #1  0x00010016e209 in qemu_iohandler_poll (readfds=0x10097ca00, 
writefds=0x10097ca80, xfds=0x10097cb00, ret=4) at iohandler.c:124
  #2  0x000100172acf in main_loop_wait (nonblocking=0) at main-loop.c:418
  #3  0x000100207bbf in main_loop () at vl.c:1765
  #4  0x00010020e7b0 in qemu_main (argc=12, argv=0x7fff5fbff360, 
envp=0x7fff5fbff3c8) at vl.c:3992
  #5  0x0001001d6013 in main (argc=12, argv=0x7fff5fbff360) at 
ui/cocoa.m:884
  (gdb) frame 1
  #1  0x00010016e209 in qemu_iohandler_poll (readfds=0x10097ca00, 
writefds=0x10097ca80, xfds=0x10097cb00, ret=4) at iohandler.c:124
  124   ioh->fd_read(ioh->opaque);
  Current language:  auto; currently c
  (gdb) p ioh
  $1 = (IOHandlerRecord *) 0x10142f110
  (gdb) p *ioh
  $2 = {
fd_read_poll = 0, 
fd_read = 0x10017212b , 
fd_write = 0, 
opaque = 0x3, 
next = {
  le_next = 0x0, 
  le_prev = 0x105d00bc0
}, 
fd = 3, 
deleted = false
  }

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

[Qemu-devel] [Bug 1099403] Re: High CPU utilization in vnc mode

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  High CPU utilization in vnc mode

Status in QEMU:
  Expired

Bug description:
  We start a gentoo guest using ./x86-64-softmmu/qemu-x86-64 -hda
  .qcow2 -vnc :6.

  Then we start a vncviewer session to this guest from a remote
  computer. In this session, we start a video. After starting the video,
  the CPU utilization of the guest (the qemu-x86-64 process) increases
  to about 90%. The high CPU utilization persists even after closing the
  vncviewer session.

  However, the CPU usage while running a video inside a gentoo guest
  (without a remote computer connecting via vncviewer) is only 20-30%.
  So we suspect the high CPU usage to be due to the vncserver code
  running inside QEMU which has to do a lot of work to send the
  framebuffer updates to the client.

  My question is why does the usage not decrease when the remote
  vncviewer is disconnected? On simple computers (no virtual guests),
  the CPU usage of vncserver decreases drastically when the vncviewer
  client is disconnected. Why does this not happen in the vncserver
  provided by QEMU (through -vnc :6).

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

[Qemu-devel] [Bug 1089006] Re: Qemu scrambles order of eth devices in vm

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  Qemu scrambles order of eth devices in vm

Status in QEMU:
  Expired

Bug description:
  HV = 12.04 LTS plus libvirt 1.0x
  VM = 12.04 LTS

  On the HV there are 12 eth interfaces which we make available to the
  VM. We have 4 10G virtual function interfaces, and 8 1G conventionally
  bridged interfaces. No matter what order we present the interfaces in
  the xml file, they come up in eth0-eth11 order on the VM as follows:
  ( the interfcaes do work, once you figure out which is which)

  eth0-eth7 not in order as compoared to the bridges on the HV (interfaces 
file) or compared to the xml file for the VM, or compared to the bus numbers. 
MAC addresses are random.
  eth8-eth11 show up in the VM  in order of PCU bus numbers just as you'd 
expect, always after the bridged interfaces.

  Consulting the libvirt mailing list, the developer says they present
  the list in bus order to qemu, but qemu scrambles that order. That
  appears to me too, to be the case.

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

Re: [Qemu-devel] [PATCH 3/4] ppc: kvm: introduce a helper to update SDR1 for a single CPU

[David Gibson]

On Mon, Sep 04, 2017 at 11:47:14PM +0200, Greg Kurz wrote:
> When running with KVM PR, we hijack the SDR1 slot to pass the address of
> the HPT allocated by QEMU to KVM.  On pseries virtual machines, we have to
> do this when the guest calls the KVMPPC_H_CAS or the H_RESIZE_HPT_COMMIT
> hypercalls. This is currently handled by kvmppc_update_sdr1() which updates
> SDR1 for all CPUs. But we also need to update SDR1 at machine reset, and
> this is currently open-coded in spapr_cpu_reset() on a per-CPU basis.
> 
> This patch renames kvmppc_update_sdr1() to kvmppc_update_sdr1_all() and
> reuses the kvmppc_update_sdr1() function name to update a single CPU,
> like we already do with in the CPU compat mode code.
> 
> It finally converts the sPAPR code to use the all CPUs or single CPU helpers
> where appropriate.
> 
> Signed-off-by: Greg Kurz 

Seems reasonable, but I'll wait for the next spin of the series, since
the previous patch wants some work.

> ---
>  hw/ppc/spapr_cpu_core.c |6 +-
>  hw/ppc/spapr_hcall.c|4 ++--
>  target/ppc/kvm.c|   12 
>  target/ppc/kvm_ppc.h|3 ++-
>  4 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 581eb4d92de9..da81688b0f4d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -95,11 +95,7 @@ static void spapr_cpu_reset(void *opaque)
>  if (kvm_enabled()) {
>  target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>  if (sdr1) {
> -env->spr[SPR_SDR1] = sdr1;
> -if (kvmppc_put_books_sregs(cpu) < 0) {
> -error_report("Unable to update SDR1 in KVM");
> -exit(1);
> -}
> +kvmppc_update_sdr1(cpu, sdr1);
>  }
>  }
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 06059b44ab40..e090b69efe7f 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -737,7 +737,7 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>  /* For KVM PR, update the HPT pointer */
>  target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>  if (sdr1) {
> -kvmppc_update_sdr1(sdr1);
> +kvmppc_update_sdr1_all(sdr1);
>  }
>  }
>  
> @@ -1569,7 +1569,7 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  /* For KVM PR, update the HPT pointer */
>  target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>  if (sdr1) {
> -kvmppc_update_sdr1(sdr1);
> +kvmppc_update_sdr1_all(sdr1);
>  }
>  }
>  }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 6442dfcb95b3..e69366968f15 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2806,10 +2806,9 @@ int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, 
> target_ulong flags, int shift)
>  return kvm_vm_ioctl(cs->kvm_state, KVM_PPC_RESIZE_HPT_COMMIT, );
>  }
>  
> -static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1)
>  {
> -target_ulong sdr1 = arg.target_ptr;
> -PowerPCCPU *cpu = POWERPC_CPU(cs);
> +CPUState *cs = CPU(cpu);
>  CPUPPCState *env = >env;
>  
>  /* This is just for the benefit of PR KVM */
> @@ -2821,7 +2820,12 @@ static void kvmppc_pivot_hpt_cpu(CPUState *cs, 
> run_on_cpu_data arg)
>  }
>  }
>  
> -void kvmppc_update_sdr1(target_ulong sdr1)
> +static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +{
> +kvmppc_update_sdr1(POWERPC_CPU(cs), arg.target_ptr);
> +}
> +
> +void kvmppc_update_sdr1_all(target_ulong sdr1)
>  {
>  CPUState *cs;
>  
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index f780e6ec7b72..9524a7a0c21c 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -68,7 +68,8 @@ PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
>  void kvmppc_check_papr_resize_hpt(Error **errp);
>  int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int 
> shift);
>  int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
> -void kvmppc_update_sdr1(target_ulong sdr1);
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1);
> +void kvmppc_update_sdr1_all(target_ulong sdr1);
>  bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
>  
>  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[David Gibson]

On Mon, Sep 04, 2017 at 11:47:05PM +0200, Greg Kurz wrote:
> The formula used to compute the address of the HPT allocated by QEMU is
> open-coded in several places. This patch moves the magic to a dedicated
> helper. While here, we also patch the callers to only pass the address
> to KVM if we indeed have a userland HPT (ie, KVM PR).

The helper function seems reasonable, though I'm not sure about the
name (a. it's not just a pointer, since it includes the encoded size
and b. the name doesn't indicate this is basically KVM PR specific).

THe "only pass the address to KVM if we indeed have a userland HPT
(ie, KVM PR)" bit doesn't really work.  You're doing it by testing for
(sdr1 != 0), but that can only be true if the HPT is minimum size,
which doesn't have much to do with anything meaningful.

> Signed-off-by: Greg Kurz 
> ---
>  hw/ppc/spapr.c  |9 +
>  hw/ppc/spapr_cpu_core.c |   12 +++-
>  hw/ppc/spapr_hcall.c|   14 --
>  include/hw/ppc/spapr.h  |1 +
>  4 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index caffa1276328..bf24c26b756d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1290,6 +1290,15 @@ static void spapr_store_hpte(PPCVirtualHypervisor 
> *vhyp, hwaddr ptex,
>  }
>  }
>  
> +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
> +{
> +if (!spapr->htab) {
> +return 0;
> +}
> +
> +return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> +}
> +
>  int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
>  {
>  int shift;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 85037ef71e27..581eb4d92de9 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -93,11 +93,13 @@ static void spapr_cpu_reset(void *opaque)
>   * HPT
>   */
>  if (kvm_enabled()) {
> -env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab
> -| (spapr->htab_shift - 18);
> -if (kvmppc_put_books_sregs(cpu) < 0) {
> -error_report("Unable to update SDR1 in KVM");
> -exit(1);
> +target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +if (sdr1) {
> +env->spr[SPR_SDR1] = sdr1;
> +if (kvmppc_put_books_sregs(cpu) < 0) {
> +error_report("Unable to update SDR1 in KVM");
> +exit(1);
> +}
>  }
>  }
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6ab8c188f381..06059b44ab40 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -735,9 +735,10 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>  
>  if (kvm_enabled()) {
>  /* For KVM PR, update the HPT pointer */
> -target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> -| (spapr->htab_shift - 18);
> -kvmppc_update_sdr1(sdr1);
> +target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +if (sdr1) {
> +kvmppc_update_sdr1(sdr1);
> +}
>  }
>  
>  pending->hpt = NULL; /* so it's not free()d */
> @@ -1566,9 +1567,10 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  spapr_reallocate_hpt(spapr, maxshift, _fatal);
>  if (kvm_enabled()) {
>  /* For KVM PR, update the HPT pointer */
> -target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> -| (spapr->htab_shift - 18);
> -kvmppc_update_sdr1(sdr1);
> +target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +if (sdr1) {
> +kvmppc_update_sdr1(sdr1);
> +}
>  }
>  }
>  }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c1b365f56431..a1f5edc15018 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -709,4 +709,5 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, 
> run_on_cpu_data arg);
>  int spapr_vcpu_id(PowerPCCPU *cpu);
>  PowerPCCPU *spapr_find_cpu(int vcpu_id);
>  
> +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr);
>  #endif /* HW_SPAPR_H */
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.11] hw/misc/ivshmem: Fix ivshmem_recv_msg() to also work on big endian systems

[David Gibson]

On Wed, Aug 30, 2017 at 03:59:07PM +0100, Peter Maydell wrote:
> On 30 August 2017 at 15:53, Philippe Mathieu-Daudé  wrote:
> > On 08/30/2017 10:39 AM, Thomas Huth wrote:
> >> The problem is that the server side code in ivshmem_server_send_one_msg()
> >> correctly translates all messages IDs into little endian 64-bit values,
> >> but the client side code in the ivshmem_recv_msg() function does not swap
> >> the byte order back. Fix it by passing the value through le64_to_cpu().
> >
> >
> > Yes, we lack BE testing :(
> 
> My pre-pull-request test set includes s390 and ppc64 bigendian.
> This one was just missed because the 'slow' tests aren't in
> 'make check'.

I'm not what makes sense for staging this fix.  I could take it
through my tree, but it's not an obvious match, since this isn't
really related to ppc at all.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 4/5] target/ppc: Handle NMI guest exit

[David Gibson]

On Fri, Sep 08, 2017 at 01:39:37PM +0530, Aravinda Prasad wrote:
> 
> 
> On Wednesday 23 August 2017 02:09 PM, David Gibson wrote:
> > On Mon, Aug 21, 2017 at 06:00:52PM +0530, Aravinda Prasad wrote:
> >>
> >>
> >> On Thursday 17 August 2017 07:27 AM, David Gibson wrote:
> >>> On Wed, Aug 16, 2017 at 02:42:39PM +0530, Aravinda Prasad wrote:
>  Memory error such as bit flips that cannot be corrected
>  by hardware are passed on to the kernel for handling.
>  If the memory address in error belongs to guest then
>  guest kernel is responsible for taking suitable action.
>  Patch [1] enhances KVM to exit guest with exit reason
>  set to KVM_EXIT_NMI in such cases.
> 
>  This patch handles KVM_EXIT_NMI exit. If the guest OS
>  has registered the machine check handling routine by
>  calling "ibm,nmi-register", then the handler builds
>  the error log and invokes the registered handler else
>  invokes the handler at 0x200.
> 
>  [1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
>   (e20bbd3d and related commits)
> 
>  Signed-off-by: Aravinda Prasad 
>  ---
>   hw/ppc/spapr.c   |4 ++
>   target/ppc/kvm.c |   86 
>  ++
>   target/ppc/kvm_ppc.h |   81 
>  +++
>   3 files changed, 171 insertions(+)
> 
>  diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>  index 0bb2c4a..6cc3f69 100644
>  --- a/hw/ppc/spapr.c
>  +++ b/hw/ppc/spapr.c
>  @@ -2346,6 +2346,10 @@ static void ppc_spapr_init(MachineState *machine)
>   error_report("Could not get size of LPAR rtas '%s'", filename);
>   exit(1);
>   }
>  +
>  +/* Resize blob to accommodate error log. */
>  +spapr->rtas_size = RTAS_ERRLOG_OFFSET + sizeof(struct RtasMCELog);
>  +
>   spapr->rtas_blob = g_malloc(spapr->rtas_size);
>   if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 
>  0) {
>   error_report("Could not load LPAR rtas '%s'", filename);
>  diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>  index 8571379..73f64ed 100644
>  --- a/target/ppc/kvm.c
>  +++ b/target/ppc/kvm.c
>  @@ -1782,6 +1782,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct 
>  kvm_run *run)
>   ret = 0;
>   break;
>   
>  +case KVM_EXIT_NMI:
>  +DPRINTF("handle NMI exception\n");
>  +ret = kvm_handle_nmi(cpu);
>  +break;
>  +
>   default:
>   fprintf(stderr, "KVM: unknown exit reason %d\n", 
>  run->exit_reason);
>   ret = -1;
>  @@ -2704,6 +2709,87 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>   return data & 0x;
>   }
>   
>  +int kvm_handle_nmi(PowerPCCPU *cpu)
> >>>
> >>> So you only handle NMIs with KVM.  Wouldn't it make sense to also
> >>> handle them for TCG (where they can be triggered with the "nmi"
> >>> command on the monitor).
> >>
> >> For TCG it is already handled in spapr_nmi() which ultimately branches
> >> to guest's 0x200 vector. Even with KVM when KVM_CAP_PPC_FWNMI is not
> >> enabled we branch to guest's 0x200 vector. Should TCG behave as if
> >> KVM_CAP_PPC_FWNMI is not enabled?
> > 
> > No, we should implement the FWNMI feature for TCG, which means it
> > needs to respect the address given by nmi-register.
> 
> I have couple of questions regarding supporting NMIs triggered through
> "nmi" command from the monitor.
> 
> 1. The FWNNMI builds an error log upon machine check that includes the
> effective address that triggered the machine check and places the error
> log in RTAS blob. Hence, we need to build error log for the NMI
> triggered via the monitor "nmi" command. What should be the effective
> address and the other values of the error log in this case? Should "nmi"
> command be enhanced to take these arguments (as of now it does not take
> any arguments)?

Ah!  That's something I hadn't realised - sounds like FWNMI is only
designed to handle synchronous machine checks, whereas the monitor
command (obviously) generates asynchronous checks.

In that case it's probably reasonable that the monitor command doesn't
go through the FWNMI path.  However, it would be nice to make it a bit
more obvious, by updating some of the comments and commit messages in
the FWNMI code to emphasise that it's only for synchronous checks.

> 2. On Power architecture "nmi" monitor command triggers NMI on all the
> CPUs, while the NMI due to hardware error is triggered only on the CPU
> that accessed the bad memory region. Triggering NMI on all the CPUs
> could be a problem as it cause kernel panic if the CPU was in kernel
> space at that time or kills the process running on that CPU if it was in
> userspace.
> 

-- 
David Gibson| I'll have my music 

Re: [Qemu-devel] [PATCH 1/3] ppc/xive: fix OV5_XIVE_EXPLOIT bits

[David Gibson]

On Fri, Sep 08, 2017 at 04:33:42PM +0200, Cédric Le Goater wrote:
> On POWER9, the Client Architecture Support (CAS) negotiation process
> determines whether the guest operates in XIVE Legacy compatibility or
> in XIVE exploitation mode. Now that we have initial guest support for
> the XIVE interrupt controller, let's fix the bits definition which have
> evolved in the latest specs.
> 
> The platform advertises the XIVE Exploitation Mode support using the
> property "ibm,arch-vec-5-platform-support-vec-5", byte 23 bits 0-1 :
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
>  - 0b10 XIVE legacy or exploitation mode
> 
> The OS asks for XIVE Exploitation Mode support using the property
> "ibm,architecture-vec-5", byte 23 bits 0-1:
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
> 
> Signed-off-by: Cédric Le Goater 

Applied to ppc-for-2.11, thanks.

> ---
>  hw/ppc/spapr.c  | 2 +-
>  include/hw/ppc/spapr_ovec.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index cec441cbf48d..3e3ff1fbc988 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -914,7 +914,7 @@ static void spapr_dt_ov5_platform_support(void *fdt, int 
> chosen)
>  PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
>  
>  char val[2 * 4] = {
> -23, 0x00, /* Xive mode: 0 = legacy (as in ISA 2.7), 1 = Exploitation 
> */
> +23, 0x00, /* Xive mode, filled in below. */
>  24, 0x00, /* Hash/Radix, filled in below. */
>  25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
>  26, 0x40, /* Radix options: GTSE == yes. */
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index 9edfa5ff7530..bf25e5d954a1 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -51,7 +51,8 @@ typedef struct sPAPROptionVector sPAPROptionVector;
>  #define OV5_FORM1_AFFINITY  OV_BIT(5, 0)
>  #define OV5_HP_EVT  OV_BIT(6, 5)
>  #define OV5_HPT_RESIZE  OV_BIT(6, 7)
> -#define OV5_XIVE_EXPLOITOV_BIT(23, 7)
> +#define OV5_XIVE_BOTH   OV_BIT(23, 0)
> +#define OV5_XIVE_EXPLOITOV_BIT(23, 1) /* 1=exploitation 0=legacy */
>  
>  /* ISA 3.00 MMU features: */
>  #define OV5_MMU_BOTHOV_BIT(24, 0) /* Radix and hash */

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/3] spapr: fix CAS-generated reset

[David Gibson]

On Fri, Sep 08, 2017 at 04:33:43PM +0200, Cédric Le Goater wrote:
> The OV5_MMU_RADIX_300 requires special handling in the CAS negotiation
> process. It is cleared from the option vector of the guest before
> evaluating the changes and re-added later. But, when testing for a
> possible CAS reset :
> 
> spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
> ov5_cas_old, spapr->ov5_cas);
> 
> the bit OV5_MMU_RADIX_300 will each time be seen as removed from the
> previous OV5 set, hence generating a reset loop.
> 
> Fix this problem by also clearing the same bit in the ov5_cas_old set.
> 
> Signed-off-by: Cédric Le Goater 

Kind of an ugly hack, but probably the easiest fix for now. Applied to 
ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4cd..92f1e21358b8 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1575,6 +1575,13 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>   * to worry about this for now.
>   */
>  ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
> +
> +/* also clear the radix/hash bit from the current ov5_cas bits to
> + * be in sync with the newly ov5 bits. Else the radix bit will be
> + * seen as being removed and this will generate a reset loop
> + */
> +spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
> +
>  /* full range of negotiated ov5 capabilities */
>  spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
>  spapr_ovec_cleanup(ov5_guest);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH 3/3] spapr: generate a CAS reset for the XIVE exploitation mode

[David Gibson]

On Fri, Sep 08, 2017 at 04:33:44PM +0200, Cédric Le Goater wrote:
> When the platform and the guest agree on using the XIVE exploitation
> mode for interrupts, the "interrupt-controller" node needs to reflect
> the change and the device tree needs an update.
> 
> Reseting the guest after the CAS negotiation makes this change
> possible, as the device tree is built at reset time. We use the
> 'ov5_cas' field to check which interrupt model was negotiated before
> reset and populate the tree accordingly.
> 
> Signed-off-by: Cédric Le Goater 

Looks reasonable, though I'm not going to apply it until we actually
have something to go..

> ---
>  hw/ppc/spapr.c   | 6 +-
>  hw/ppc/spapr_hcall.c | 6 ++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3e3ff1fbc988..be467ab61ad0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1073,7 +1073,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>  
>  /* /interrupt controller */
> -spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +} else {
> +/* populate device tree for XIVE */ ;

.. here.

> +}
>  
>  ret = spapr_populate_memory(spapr, fdt);
>  if (ret < 0) {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 92f1e21358b8..ba00b8d3fdd6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1613,6 +1613,12 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  (spapr_h_cas_compose_response(spapr, args[1], args[2],
>ov5_updates) != 0);
>  }
> +
> +/* We need to rebuild the device tree for XIVE, generate a reset */
> +if (!spapr->cas_reboot) {
> +spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT);
> +}

At some point we may well want to make XIVE exploitation the default
for POWER9 guests, but that's a detail easily adjusted later.

>  spapr_ovec_cleanup(ov5_updates);
>  
>  if (spapr->cas_reboot) {

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 8/8] tcg/i386: Add vector operations

[Richard Henderson]

On 09/08/2017 06:10 AM, Alex Bennée wrote:
> Also this commit breaks RISU:
> 
>  qemu-aarch64 build/aarch64-linux-gnu/risu
> testcases.aarch64/insn_ANDSi_RES8_ANDS_RES_ANDv_ASRV__INC.risu.bin \
> -t 
> testcases.aarch64/insn_ANDSi_RES8_ANDS_RES_ANDv_ASRV__INC.risu.bin.trace
> 
> Gives:
> 
> mismatch detail (master : apprentice):
>   V29   : 05388083c1444242 vs 2a000e0416a30018
> 
> The insn is:
> 
>   37c:   6f56a29dumull2  v29.4s, v20.8h, v6.h[1]
> 
> Which is odd because I didn't think we'd touched that.

Indeed we didn't.  Still, I'll check it out next week.


r~

Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Fam Zheng]

On Sat, 09/09 17:08, Kamil Rytarowski wrote:
> >>> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> >>> +
> >>> +if test -n "$submodules"; then
> >>> +{
> >>> +git ls-files
> >>> +for sm in $submodules; do
> >>> +(cd $sm; git ls-files) | sed "s:^:$sm/:"
> >>> +done
> >>> +} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
> >>> $1.list
> >>
> >> Supporting '-e something' in a tar -T listfile seems to
> >> be GNU tar specific. Do we care?
> > 
> > The "-e $sm" will go to the grep command line, so it isn't GNU specific, is 
> > it?
> > 
> 
> BSD OSes use GNU grep(1) the latest version GPLv2. There is a work in
> progress BSD grep(1), but not turned on by default as it's slower.
> 
> The -e option is supported by SmartOS grep(1).

Is there a more portable way to filter out multiple patterns?

Fam

Re: [Qemu-devel] [PATCH 0/5] spapr_pci: various cleanups and improvements

[David Gibson]

On Sat, Sep 09, 2017 at 05:05:46PM +0200, Greg Kurz wrote:
> Before resuming the huge work on PHB hotplug, here are some patches
> that maybe worth to apply.
> 
> Patches 1 to 3 are basic improvements.

I've applied these to ppc-for-2.11.

> Patch 4 and 5 may be a bit controversial. Everywhere in the spapr
> code where we build an FDT portion, libfdt failures cause QEMU to
> exit, even on hotplug paths. Only spapr_pci doesn't do that and
> propagates the error instead. My understanding is that a failure
> when building the FDT is likely to happen because of a bug in QEMU.

Still looking at these.

> 
> Hence the choice to convert spapr_pci to do like the others. We may
> even consider changing _FDT() to abort() instead of exit().
> 
> Alternatively, if libfdt failures shouldn't be necessarily fatal,
> especially on post-realize paths, then we should probably introduce
> an _FDT_ERR() helper to propagate errors. And use it in may places
> where we currently terminate QEMU: memory hotplug, CPU hotplug, CAS,
> machine reset...
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/4] spapr: only update SDR1 once per-cpu during CAS

[David Gibson]

On Mon, Sep 04, 2017 at 11:46:55PM +0200, Greg Kurz wrote:
> Commit b55d295e3ec9 added the possibility to support HPT resizing with KVM.
> In the case of PR, we need to pass the userspace address of the HPT to KVM
> using the SDR1 slot.
> This is handled by kvmppc_update_sdr1() which uses CPU_FOREACH() to update
> all CPUs. It is hence not needed to call kvmppc_update_sdr1() for each CPU.
> 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c |   14 +-
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8b3c0e17e75c..6ab8c188f381 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1559,20 +1559,16 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  }
>  
>  if (spapr->htab_shift < maxshift) {
> -CPUState *cs;
> -
>  /* Guest doesn't know about HPT resizing, so we
>   * pre-emptively resize for the maximum permitted RAM.  At
>   * the point this is called, nothing should have been
>   * entered into the existing HPT */
>  spapr_reallocate_hpt(spapr, maxshift, _fatal);
> -CPU_FOREACH(cs) {
> -if (kvm_enabled()) {
> -/* For KVM PR, update the HPT pointer */
> -target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> -| (spapr->htab_shift - 18);
> -kvmppc_update_sdr1(sdr1);
> -}
> +if (kvm_enabled()) {
> +/* For KVM PR, update the HPT pointer */
> +target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> +| (spapr->htab_shift - 18);
> +kvmppc_update_sdr1(sdr1);
>  }
>  }
>  }
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v5 3/8] fw_cfg: add vmcoreinfo file

[Michael S. Tsirkin]

On Fri, Sep 08, 2017 at 11:49:46AM -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > On Fri, Sep 08, 2017 at 06:39:01PM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Aug 07, 2017 at 08:16:13PM +0200, Marc-André Lureau wrote:
> > > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > > > index 08c00bdf44..37d0f9f40a 100644
> > > > --- a/docs/specs/fw_cfg.txt
> > > > +++ b/docs/specs/fw_cfg.txt
> > > > @@ -136,6 +136,22 @@ struct FWCfgFile { /* an individual file 
> > > > entry, 64
> > > > bytes total */
> > > >  char name[56]; /* fw_cfg item name, NUL-terminated 
> > > > ascii */
> > > >  };
> > > >  
> > > > +=== etc/vmcoreinfo ===
> > > > +
> > > > +A guest may use this entry to add information details to qemu
> > > > +dumps. The entry gives location and size of an ELF note that is
> > > > +appended in qemu dumps.
> > > > +
> > > > +The entry is of 12 bytes with this format:
> > > > +
> > > > +struct FWCfgVMCoreInfo {
> > > > +uint64_t paddr; /* physical address of ELF note, LE */
> > > > +uint32_t size;  /* size of ELF note region, LE */
> > > > +};
> > > > +
> > > > +The note format/class must be of the target bitness and the size must
> > > > +be less than 1Mb.
> > > > +
> > > 
> > > I would say adding a format bitmap would make sense for future
> > > compatibility.
> > > How about:
> > > 
> > > struct FWCfgVMCoreInfo {
> > > uint16_t host_format;   /* Formats host supports. 0x1 LE - ELF
> > > note. Other bits - ignored. */
> 
> Could this be added later?
>
> For ex, extend the entry to 16 bytes, with those 2 values appended?

I think aligned size is better overall so you can write
bindings in any language without issues.
I'm ok with just keeping 0s there for now if you prefer.


> If the size is 12, assume it is ELF note only, and that the host supports it.

Seems more like a hack that will make code ugly down the road.

> > > uint16_t guest_format;  /* Formats guest supplies. Must be 0x1 LE
> > > */
> > 
> > .. to set the ELF note info, or 0x0 to reset the device.
> > 
> > > uint32_t size;  /* size of ELF note region, LE */
> > > uint64_t paddr; /* physical address of ELF note, LE */
> > > };
> > > 
> > > 
> > > >  === All Other Data Items ===
> > > >  
> > > >  Please consult the QEMU source for the most up-to-date and 
> > > > authoritative
> > > >  list
> > > > --
> > > > 2.14.0.1.geff633fa0
> > 

Re: [Qemu-devel] [PATCH 2/8] target/arm: Use generic vector infrastructure for aa64 add/sub/logic

[Richard Henderson]

On 09/07/2017 09:58 AM, Alex Bennée wrote:
>> +switch (size + 4 * is_u) {
> 
> Hmm I find this switch a little too magical. I mean I can see that the
> encoding abuses size for the final opcode when I look at the manual but
> it reads badly.
> 
>> +case 0: /* AND */
>> +gvec_op = tcg_gen_gvec_and8;
>> +goto do_gvec;
>> +case 1: /* BIC */
>> +gvec_op = tcg_gen_gvec_andc8;
>> +goto do_gvec;
>> +case 2: /* ORR */
>> +gvec_op = tcg_gen_gvec_or8;
>> +goto do_gvec;
>> +case 3: /* ORN */
>> +gvec_op = tcg_gen_gvec_orc8;
>> +goto do_gvec;
>> +case 4: /* EOR */
>> +gvec_op = tcg_gen_gvec_xor8;
>> +goto do_gvec;
>> +do_gvec:
>> +gvec_op(vec_full_reg_offset(s, rd),
>> +vec_full_reg_offset(s, rn),
>> +vec_full_reg_offset(s, rm),
>> +is_q ? 16 : 8, vec_full_reg_size(s));
>> +return;
> 
> No default case (although I guess we just fall through). What's wrong
> with just having a !is_u test with gvec_op = tbl[size] and skipping all
> the goto stuff?

Because that would still leave EOR out in the woods.
I do think this is the cleanest way to filter out these 5 operations.


r~

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Eduardo Habkost]

On Sat, Sep 09, 2017 at 11:41:55PM +0100, Peter Maydell wrote:
> On 9 September 2017 at 21:30, Eduardo Habkost  wrote:
> > However, I have a suggestion for Igor: making a separate patch
> > that renames the existing property to "x-cpu-model", and using
> > "x-cpu-type" in this series.  This way we will explicitly
> > document the fact that the property is not a stable
> > user/management interface.
> 
> We have lots and lots of properties on non-user-instantiated
> devices that aren't intended as user level configurable
> things...

Well, we need to deal with them somehow, otherwise we have no
simple way to know if removing/renaming a property is really safe
or not.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Peter Maydell]

On 9 September 2017 at 21:30, Eduardo Habkost  wrote:
> However, I have a suggestion for Igor: making a separate patch
> that renames the existing property to "x-cpu-model", and using
> "x-cpu-type" in this series.  This way we will explicitly
> document the fact that the property is not a stable
> user/management interface.

We have lots and lots of properties on non-user-instantiated
devices that aren't intended as user level configurable
things...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h

[Eduardo Habkost]

On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:
> On 08.09.2017 06:21, Thomas Huth wrote:
> > On 07.09.2017 22:13, David Hildenbrand wrote:
> >> Implemented in sclp.c, so let's move it to the right include file.
> >> Fix up one include. Do a forward declaration of CPUS390XState to fix the
> >> two sclp consoles complaining.
> >>
> >> Signed-off-by: David Hildenbrand 
> >> ---
> >>  include/hw/s390x/sclp.h| 2 ++
> >>  target/s390x/cpu.h | 1 -
> >>  target/s390x/misc_helper.c | 1 +
> >>  3 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> >> index a72d096081..4b86a8a293 100644
> >> --- a/include/hw/s390x/sclp.h
> >> +++ b/include/hw/s390x/sclp.h
> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev 
> >> *init_sclp_memory_hotplug_dev(void);
> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
> >>  void sclp_service_interrupt(uint32_t sccb);
> >>  void raise_irq_cpu_hotplug(void);
> >> +typedef struct CPUS390XState CPUS390XState;
> >> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> > 
> > That's dangerous and likely does not work with certain versions of GCC.
> > You can't do a "forward declaration" with typedef in C, I'm afraid. See
> > for example:
> > 
> >  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html
> >  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html
> >  https://stackoverflow.com/questions/8367646/redefinition-of-typedef
> > 
> > All this typedef'ing in QEMU is pretty bad ... we run into this problem
> > again and again. include/qemu/typedefs.h is just a work-around for this.
> > I know people like typedefs for some reasons (I used to do that, too,
> > before I realized the trouble with them), but IMHO we should rather
> > adopt the typedef-related rules from the kernel coding conventions instead:
> > 
> >  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs
> > 
> >   Thomas
> > 
> 
> Yes, this is really nasty. And I wasn't aware of the involved issues.
> 
> This seems to be the only feasible solution (including cpu.h sounds
> wrong and will require a bunch of other includes):
> 
> 
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index a72d096081..ce80915a02 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev
> *init_sclp_memory_hotplug_dev(void);
>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
>  void sclp_service_interrupt(uint32_t sccb);
>  void raise_irq_cpu_hotplug(void);
> +struct CPUS390XState;
> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,
> uint32_t code);
> 
>  #endif

Why not use typedefs.h?

Signed-off-by: Eduardo Habkost 
---
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 4b86a8a293..3512bf8283 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);
 sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
 void sclp_service_interrupt(uint32_t sccb);
 void raise_irq_cpu_hotplug(void);
-typedef struct CPUS390XState CPUS390XState;
 int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
 
 #endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 39bc8351a3..9c97bffa92 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -21,6 +21,7 @@ typedef struct Chardev Chardev;
 typedef struct CompatProperty CompatProperty;
 typedef struct CPUAddressSpace CPUAddressSpace;
 typedef struct CPUState CPUState;
+typedef struct CPUS390XState CPUS390XState;
 typedef struct DeviceListener DeviceListener;
 typedef struct DeviceState DeviceState;
 typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 032d1de2e8..f99a82cd5e 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -80,7 +80,7 @@ typedef struct MchkQueue {
 uint16_t type;
 } MchkQueue;
 
-typedef struct CPUS390XState {
+struct CPUS390XState {
 uint64_t regs[16]; /* GP registers */
 /*
  * The floating point registers are part of the vector registers.
@@ -174,7 +174,7 @@ typedef struct CPUS390XState {
 /* currently processed sigp order */
 uint8_t sigp_order;
 
-} CPUS390XState;
+};
 
 static inline CPU_DoubleU *get_freg(CPUS390XState *cs, int nr)
 {



> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 3aa2e46aac..032d1de2e8 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -721,6 +721,5 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr,
> uint8_t ar, void *hostbuf,
> 
>  /* outside of target/s390x/ */
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
> -int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> 
>  #endif
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] dma/i82374: avoid double creation of i82374 device

[Eduardo Habkost]

On Thu, Sep 07, 2017 at 10:38:29AM +0200, Eduardo Otubo wrote:
> On Fri, Sep 01, 2017 at 12:44:30PM -0300, Eduardo Habkost wrote:
> > On Fri, Sep 01, 2017 at 05:34:34PM +0200, Markus Armbruster wrote:
> > > Eduardo Habkost  writes:
> > > > On Fri, Sep 01, 2017 at 01:03:32PM +0200, Eduardo Otubo wrote:
[...]
> > > >> diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
> > > >> index 6c0f975df0..5275d822e0 100644
> > > >> --- a/hw/dma/i82374.c
> > > >> +++ b/hw/dma/i82374.c
> > > >> @@ -139,6 +139,7 @@ static void i82374_class_init(ObjectClass *klass, 
> > > >> void *data)
> > > >>  dc->realize = i82374_realize;
> > > >>  dc->vmsd = _i82374;
> > > >>  dc->props = i82374_properties;
> > > >> +dc->user_creatable = false;
> > > >
> > > > A "Reason:" comment explaining why user_creatable=false is
> > > > mandatory.  See the comment above user_creatable declaration in
> > > > qdev-core.h for reference.
> > > >
> > > > I suggest the following:
> > > >
> > > > /*
> > > >  * Reason: i82374_realize() crashes (assertion failure inside 
> > > > isa_bus_dma()
> > > >  * if the device is instantiated twice.
> > > >  */
> 
> I agree with the comment above. If there's nothing left to fix/add
> I'll just send a v2 for this shortly.

I suggest amending it with the additional info below, explaining
that isa_bus_dma() isn't supposed to be called twice, and that we
need to make isa_bus_dma()/DMA_init()/i82374_realize() return an
appropriate error if the device is instantiated twice.


> 
> > > 
> > > We need to find out *why* it crashes.  Once we know, we can likely write
> > > a better comment.
> > 
> > It crashes because isa_bus_dma() isn't supposed to be called
> > twice for the same bus.
> > 
> > Making isa_bus_dma()/DMA_init()/i82374_realize() return an error
> > instead of asserting would be even better than setting
> > user_creatable=false.
> > 
> > -- 
> > Eduardo
> > 
> 
> -- 
> Eduardo Otubo
> Senior Software Engineer @ RedHat

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] i386/cpu/hyperv: support over 64 vcpus for windows guests

[Eduardo Habkost]

On Thu, Sep 07, 2017 at 01:05:33AM +, Gonglei (Arei) wrote:
> 
> 
> > -Original Message-
> > From: Eduardo Habkost [mailto:ehabk...@redhat.com]
> > Sent: Tuesday, September 05, 2017 9:17 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; m...@redhat.com; pbonz...@redhat.com;
> > r...@twiddle.net; mtosa...@redhat.com; vroze...@redhat.com;
> > Huangweidong (C)
> > Subject: Re: [PATCH 2/2] i386/cpu/hyperv: support over 64 vcpus for windows
> > guests
> > 
> > On Tue, Sep 05, 2017 at 05:30:05PM +0800, Gonglei wrote:
> > > Starting with Windows Server 2012 and Windows 8, if
> > > CPUID.4005.EAX contains a value of -1, Windows assumes specific
> > > limit to the number of VPs. In this case, Windows Server 2012
> > > guest VMs may use more than 64 VPs, up to the maximum supported
> > > number of processors applicable to the specific Windows
> > > version being used.
> > >
> > >
> > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/referenc
> > e/tlfs
> > >
> > > For compatibility, Let's introduce a new property for X86CPU,
> > > named "hv-cpuid-limits-eax" as Paolo's suggestion, and set it
> > > to "on" before machine 2.10.
> > >
> > > Signed-off-by: Gonglei 
> > > ---
> > >  include/hw/i386/pc.h |  5 +
> > >  target/i386/cpu.c|  1 +
> > >  target/i386/cpu.h|  2 ++
> > >  target/i386/kvm.c| 18 +-
> > >  4 files changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index 8226904..db32e58 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -371,6 +371,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> > uint64_t *);
> > >
> > >  #define PC_COMPAT_2_10 \
> > >  HW_COMPAT_2_10 \
> > > +{\
> > > +.driver   = TYPE_X86_CPU,\
> > > +.property = "hv_cpuid_limits_eax",\
> > 
> > The property name is hv-cpuid-limits-eax.
> > 
> Make sense to me.
> 
> > > +.value= "on",\
> > > +},\
> > >
> > >  #define PC_COMPAT_2_9 \
> > >  HW_COMPAT_2_9 \
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index 69676e1..0d47bdd 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -4145,6 +4145,7 @@ static Property x86_cpu_properties[] = {
> > >   false),
> > >  DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU,
> > vmware_cpuid_freq, true),
> > >  DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> > > +DEFINE_PROP_BOOL("hv-cpuid-limits-eax", X86CPU,
> > hv_cpuid_limits_eax, false),
> > 
> > The property name "hv-cpuid-limits-eax" doesn't say anything
> > about what it does exactly when set to true.
> > 
> > What about just making it int32?  e.g.:
> > 
> > DEFINE_PROP_INT32("x-hv-max-vps", X86CPU, hv_max_vps, -1)
> > [...]
> > {\
> > .driver   = TYPE_X86_CPU,\
> > .property = "x-hv-max-vps",\
> > .value= "0x40",\
> > },\
> > [...]
> > c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
> > c->eax = cpu->hv_max_vps;
> > 
> > 
> > (The "x-" prefix indicates that the property is not supposed to
> > be a stable user interface.)
> > 
> I thought about this as well.
> but actually we can't assure that users set the x-hv-max-vps an invalid value 
> if
> we use this method. Do we really need to expose eax directly?

We don't really need to expose eax directly and I'm not against a
boolean property, but I think an integer property with the actual
CPUID value makes the compat code simpler and the purpose of the
entry at PC_COMPAT_* more obvious.

Properties prefixed with "x-" are for internal QEMU usage or
debugging, if users want to fiddle with it, they do it at their
own risk.  I don't see a problem with that.

> 
> > 
> > >  DEFINE_PROP_END_OF_LIST()
> > >  };
> > >
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index 525d35d..f8b455a 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > @@ -1282,6 +1282,8 @@ struct X86CPU {
> > >  int32_t socket_id;
> > >  int32_t core_id;
> > >  int32_t thread_id;
> > > +
> > > +bool hv_cpuid_limits_eax;
> > >  };
> > >
> > >  static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
> > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > > index 6db7783..cf6ef96 100644
> > > --- a/target/i386/kvm.c
> > > +++ b/target/i386/kvm.c
> > > @@ -751,7 +751,23 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > >
> > >  c = _data.entries[cpuid_i++];
> > >  c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
> > > -c->eax = 0x40;
> > > +
> > > +if (!cpu->hv_cpuid_limits_eax) {
> > > +/*
> > > + * Starting with Windows Server 2012 and Windows 8, if
> > > + * CPUID.4005.EAX contains a value of -1, Windows
> > > + * assumes specific limit to the number of VPs. In this case,
> > > + * Windows Server 2012 guest VMs may use more than 64
> > VPs,
> > 

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test

[Eduardo Habkost]

On Wed, Sep 06, 2017 at 08:59:32AM +0200, Markus Armbruster wrote:
> Thomas Huth  writes:
> 
> > On 05.09.2017 18:48, Dr. David Alan Gilbert wrote:
> >> * Markus Armbruster (arm...@redhat.com) wrote:
> >>> Thomas Huth  writes:
> >>>
>  People tend to forget to mark internal devices with "user_creatable = 
>  false
>  or hotpluggable = false, and these devices can crash QEMU if added via 
>  the
>  HMP monitor. So let's add a test to run through all devices and that 
>  tries
>  to add them blindly (without arguments) to see whether this could crash 
>  the
>  QEMU instance.
> 
>  Signed-off-by: Thomas Huth 
>  ---
>   I've marked the patch as RFC since not all of the required device bug
>   fixes have been merged yet (so this patch can not be included yet 
>  without
>   breaking "make check"). It's also sad that "macio-oldworld" currently
>   has to be blacklisted - I tried to find a fix for that device,  but I 
>  was
>   not able to spot the exact problem so far. So help for fixing that 
>  device
>   is very very welcome! The crash can be reproduced like this:
> 
>   $ qemu-system-ppc64 -nographic -S -monitor stdio -serial none
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) device_add macio-oldworld,id=x
>   (qemu) device_del x
>   (qemu) **
>   ERROR:qom/object.c:1611:object_get_canonical_path_component:
>    assertion failed: (obj->parent != NULL)
>   Aborted (core dumped)
> 
>   tests/test-hmp.c | 60 
>  +++-
>   1 file changed, 59 insertions(+), 1 deletion(-)
> 
>  diff --git a/tests/test-hmp.c b/tests/test-hmp.c
>  index 7a38cdc..e8a25c4 100644
>  --- a/tests/test-hmp.c
>  +++ b/tests/test-hmp.c
>  @@ -28,7 +28,6 @@ static const char *hmp_cmds[] = {
>   "commit all",
>   "cpu-add 1",
>   "cpu 0",
>  -"device_add ?",
>   "device_add usb-mouse,id=mouse1",
>   "mouse_button 7",
>   "mouse_move 10 10",
>  @@ -116,6 +115,64 @@ static void test_info_commands(void)
>   g_free(info_buf);
>   }
>   
>  +/*
>  + * People tend to forget to mark internal devices with "user_creatable 
>  = false
>  + * and these devices can crash QEMU if added via the HMP monitor. So 
>  let's run
>  + * through all devices and try to add them blindly (without arguments) 
>  to see
>  + * whether this could crash the QEMU instance.
>  + */
>  +static void test_device_add_commands(void)
>  +{
>  +char *resp, *devices, *devices_buf, *endp;
>  +
>  +devices = devices_buf = hmp("device_add help");
>  +
>  +while (*devices) {
>  +/* Ignore header lines etc. */
>  +if (strncmp(devices, "name \"", 6)) {
>  +devices = strchr(devices, '\n');
>  +if (!devices) {
>  +break;
>  +}
>  +devices += 1;
>  +continue;
>  +}
>  +/* Extract the device name, ignore parameters and description */
>  +devices += 6;
>  +endp = strchr(devices, '"');
>  +g_assert(endp != NULL);
>  +*endp = '\0';
>  +/* Check whether it is blacklisted... */
>  +if (g_str_equal("macio-oldworld", devices)) {
>  +devices = strchr(endp + 1, '\n');
>  +if (!devices) {
>  +break;
>  +}
>  +devices += 1;
>  +continue;
>  +}
>  +/* Now run the device_add + device_del commands */
>  +if (verbose) {
>  +fprintf(stderr, "\tdevice_add %s,id=%s\n", devices, 
>  devices);
>  +}
>  +resp = hmp("device_add %s,id=%s", devices, devices);
>  +g_free(resp);
>  +if (verbose) {
>  +fprintf(stderr, "\tdevice_del %s\n", devices);
>  +}
>  +resp = hmp("device_del %s", devices);
>  +g_free(resp);
>  +/* And move forward to the next line */
>  +devices = strchr(endp + 1, '\n');
>  +if (!devices) {
>  +break;
>  +}
>  +devices += 1;
>  +}
>  +
>  +g_free(devices_buf);
>  +}
>  +
>   static void test_machine(gconstpointer data)
>   {
>   const char *machine = data;
>  @@ -125,6 +182,7 @@ static void test_machine(gconstpointer data)
>   qtest_start(args);
>   
>   test_info_commands();
>  +test_device_add_commands();
>   test_commands();
>   
>   qtest_end();
> >>>
> >>> This finds devices by parsing output of HMP help.  I think 

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Eduardo Habkost]

On Tue, Sep 05, 2017 at 03:46:07PM -0700, Alistair Francis wrote:
> On Tue, Sep 5, 2017 at 3:12 PM, Eduardo Habkost  wrote:
> > On Tue, Sep 05, 2017 at 02:47:52PM -0700, Alistair Francis wrote:
> >> On Tue, Sep 5, 2017 at 2:31 PM, Eduardo Habkost  
> >> wrote:
> > [...]
> >> >> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> >> >> index f61e735..1cd6374 100644
> >> >> --- a/hw/arm/stm32f205_soc.c
> >> >> +++ b/hw/arm/stm32f205_soc.c
> >> >> @@ -112,7 +112,7 @@ static void stm32f205_soc_realize(DeviceState 
> >> >> *dev_soc, Error **errp)
> >> >>
> >> >>  armv7m = DEVICE(>armv7m);
> >> >>  qdev_prop_set_uint32(armv7m, "num-irq", 96);
> >> >> -qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
> >> >> +qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> >> >>  object_property_set_link(OBJECT(>armv7m), 
> >> >> OBJECT(get_system_memory()),
> >> >>   "memory", _abort);
> >> >>  object_property_set_bool(OBJECT(>armv7m), true, "realized", 
> >> >> );
> >> >> @@ -200,7 +200,7 @@ static void stm32f205_soc_realize(DeviceState 
> >> >> *dev_soc, Error **errp)
> >> >>  }
> >> >>
> >> >>  static Property stm32f205_soc_properties[] = {
> >> >> -DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
> >> >> +DEFINE_PROP_STRING("cpu-type", STM32F205State, cpu_type),
> >> >
> >> > Same as armv7m: are we 100% sure users are not setting this
> >> > manually?
> >>
> >> In an embedded board like this it really doesn't make sense to let the
> >> user overwrite the CPU. The SoC will take it as an option, but the
> >> board (which creates the SoC) just blindly always uses the same CPU.
> >> That feature is more for QOMificatoion then any real reason though.
> >>
> >
> > I'm not talking about -cpu (no user-visible change in the
> > handling of -cpu should result from this patch), but about
> > possible cases where the user set the "cpu-model" property using
> > another mechanism, like -global.  Probably it's impossible for an
> > user to override the property successfully, but I would like to
> > be sure.
> 
> Ah, that is trickier.
> 
> I guess that is possible to do, but the object setting logic should
> handle the error gracefully and inform the user of the error.

After looking at the code more closely, I think we can be 100%
sure the user doesn't rely on the property, because:

* TYPE_ARMV7M and TYPE_STM32F205_SOC are both sysbus devices
  with user_creatable=false, so the user can't instantiate them
  directly;
* The only places where those objects are realized inside the
  code are:
  * mps2_common_init()
  * netduino2_init()
  * stm32f205_soc_realize()
  * armv7m_init()
  Those functions always set the "cpu-model" property immediately
  before realize.

This means any value set by the user (e.g. using -global) would
be always overwritten before realize.

However, I have a suggestion for Igor: making a separate patch
that renames the existing property to "x-cpu-model", and using
"x-cpu-type" in this series.  This way we will explicitly
document the fact that the property is not a stable
user/management interface.

> 
> >
> >
> >> In saying that I think a warning if the user tries to set the CPU
> >> would make sense. I know that this issues comes up in other ARM boards
> >> (Zynq-7000 has the same issue as well) so maybe a machine property
> >> saying that the board doesn't accept custom CPUs would be a good idea.
> >
> > Yeah, there are multiple cases in this patch where boards are
> > validating the CPU model, but not all boards do that.  A generic
> > MachineClass::valid_cpu_types[] field would be useful.
> >
> >>
> >> Overall I think this patch is moving in the right direction though and
> >> this CPU option being ignored existed before this series.
> >
> > I agree this is going on the right direction.  However, I don't
> > see any board that ignore the CPU option: all of them seem to use
> > cpu_model when creating the CPUs, already.
> 
> The Netduino2 will ignore any CPU options and always use a Cortex-m3.
> I was wrong about Zynq-7000 though, it does respect the -cpu option.
> 
> Thanks,
> Alistair
> 
> >
> > --
> > Eduardo

-- 
Eduardo

Re: [Qemu-devel] [RFC v1 1/2] machine: Add a valid_cpu_types property

[Eduardo Habkost]

On Tue, Sep 05, 2017 at 05:12:01PM -0700, Alistair Francis wrote:
> Signed-off-by: Alistair Francis 
> ---
> 
>  hw/core/machine.c   | 27 +++
>  include/hw/boards.h |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 41b53a17ad..de0f127d27 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -758,6 +758,33 @@ void machine_run_board_init(MachineState *machine)
>  machine_numa_finish_init(machine);
>  }
>  machine_class->init(machine);
> +
> +if (machine_class->valid_cpu_types && machine->cpu_model) {
> +const char *temp;
> +int i, len = machine_class->valid_cpu_types->len;

I suggest doing this after Igor's series that replaces the
cpu_model field (full -cpu string) with cpu_type (only the CPU
type name).

> +
> +for (i = 0; i < len; i++) {
> +temp = g_array_index(machine_class->valid_cpu_types, char *, i);
> +if (!strcmp(machine->cpu_model, temp)) {
> +/* The user specificed CPU is in the valid field, we are
> + * good to go.
> + */
> +g_array_free(machine_class->valid_cpu_types, true);
> +return;

I suggest checking for:
  object_class_dynamic_cast(object_class_get_name(machine->cpu_type), type)
instead.  This way machines could just enumerate a common parent
type to all supported CPU models.

> +}
> +}
> +/* The user specified CPU must not be a valid CPU, print a sane 
> error */
> +temp = g_array_index(machine_class->valid_cpu_types, char *, 0);
> +error_report("Invalid CPU: %s", machine->cpu_model);
> +error_printf("The valid options are: %s", temp);
> +for (i = 1; i < len; i++) {
> +temp = g_array_index(machine_class->valid_cpu_types, char *, i);
> +error_printf(", %s", temp);
> +}
> +error_printf("\n");

Now we have a completely new method to list the valid CPU types
in addition to arch_query_cpu_definitions() and list_cpus()
(which are already a bit messy and need to share more code).

I think this should share code with "-cpu
help"/query-cpu-definitions instead.  This means
arch_query_cpu_definitions() will need a MachineClass* argument,
if the user wants only the CPU types supported by a specific
machine type, but I think it would be an interesting improvement
to query-cpu-definitions.

> +g_array_free(machine_class->valid_cpu_types, true);
> +exit(1);
> +}
>  }
>  
>  static void machine_class_finalize(ObjectClass *klass, void *data)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3363dd19fd..78678f84a9 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -172,6 +172,7 @@ struct MachineClass {
>  int minimum_page_bits;
>  bool has_hotpluggable_cpus;
>  int numa_mem_align_shift;
> +GArray *valid_cpu_types;

The list of CPU types for a machine are very likely to be
statically defined at build time.  Any specific reason to not
make it a simple char** pointer?

>  void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
>   int nb_nodes, ram_addr_t size);
>  
> -- 
> 2.11.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] slirp: Add explanation for hostfwd parsing failure

[Samuel Thibault]

Philippe Mathieu-Daudé, on ven. 08 sept. 2017 13:19:56 -0300, wrote:
> Hi David,
> 
> On 09/08/2017 12:53 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > e.g.
> > ./x86_64-softmmu/qemu-system-x86_64 -nographic -netdev 
> > 'user,id=vnet,hostfwd=:555.0.0.0:0-:22'
> > qemu-system-x86_64: -netdev user,id=vnet,hostfwd=:555.0.0.0:0-:22: Invalid 
> > host forwarding rule ':555.0.0.0:0-:22' (Bad host address)
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >   net/slirp.c | 13 -
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/slirp.c b/net/slirp.c
> > index 01ed21c006..d87664d42e 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -496,9 +496,11 @@ static int slirp_hostfwd(SlirpState *s, const char 
> > *redir_str,
> >   char buf[256];
> >   int is_udp;
> >   char *end;
> > +const char *fail_reason = "";
> 
> Isn't it better not initialize this? So if one add a new failed syntax case
> the build with abort with -Werror=uninitialized
> 
> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé 

Applied to my tree, thanks!

(I have put Unknown reason there, otherwise the message would have
oddly-looking "()" )

Samuel

Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Peter Maydell]

On 9 September 2017 at 16:07, Fam Zheng  wrote:
> On Sat, 09/09 13:07, Peter Maydell wrote:
>> On 9 September 2017 at 06:45, Fam Zheng  wrote:
>> > Signed-off-by: Fam Zheng 
>> > Message-Id: <20170905021201.25684-4-f...@redhat.com>
>> > ---
>> >  scripts/archive-source.sh | 31 +++
>> >  1 file changed, 31 insertions(+)
>> >  create mode 100755 scripts/archive-source.sh
>> >
>> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
>> > new file mode 100755
>> > index 00..1de369532e
>> > --- /dev/null
>> > +++ b/scripts/archive-source.sh
>> > @@ -0,0 +1,31 @@
>> > +#!/bin/sh
>> > +#
>> > +# Author: Fam Zheng 
>> > +#
>> > +# Create archive of source tree, including submodules
>> > +#
>> > +# This code is licensed under the GPL version 2 or later.  See
>> > +# the COPYING file in the top-level directory.
>>
>> So is this the script that for instance Mike Roth would use
>> to create the release tarballs? If it isn't, should it be?
>
> I don't know the workflow of release tarballs, there is ./scripts/make-release
> for that. So this one is not.
>
>> Is it intended for end users to create tarballs with, or
>> is it really just a helper script for the docker stuff?
>> If the latter, it would be helpful to say so. If the former,
>> it could really use more usage information/documentation...
>
> I'm not sure what end users would use this for, but it should do its work just
> the same.. What kind of usage information do you suggest? More explaination in
> the header, or in the "usage" output when no target is specified, or a "-h"
> option for help?

Basically, if it's a special purpose script that is only
being used by docker, then it should clearly say that
so that anybody who looks at it will know it's not
something relevant to them. If it's not special purpose
then it should say what it's for and how you use it.
Just putting this in comments at the top of the script
is fine.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Kamil Rytarowski]

On 09.09.2017 17:07, Fam Zheng wrote:
> On Sat, 09/09 13:07, Peter Maydell wrote:
>> On 9 September 2017 at 06:45, Fam Zheng  wrote:
>>> Signed-off-by: Fam Zheng 
>>> Message-Id: <20170905021201.25684-4-f...@redhat.com>
>>> ---
>>>  scripts/archive-source.sh | 31 +++
>>>  1 file changed, 31 insertions(+)
>>>  create mode 100755 scripts/archive-source.sh
>>>
>>> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
>>> new file mode 100755
>>> index 00..1de369532e
>>> --- /dev/null
>>> +++ b/scripts/archive-source.sh
>>> @@ -0,0 +1,31 @@
>>> +#!/bin/sh
>>> +#
>>> +# Author: Fam Zheng 
>>> +#
>>> +# Create archive of source tree, including submodules
>>> +#
>>> +# This code is licensed under the GPL version 2 or later.  See
>>> +# the COPYING file in the top-level directory.
>>
>> So is this the script that for instance Mike Roth would use
>> to create the release tarballs? If it isn't, should it be?
> 
> I don't know the workflow of release tarballs, there is ./scripts/make-release
> for that. So this one is not.
> 
>> Is it intended for end users to create tarballs with, or
>> is it really just a helper script for the docker stuff?
>> If the latter, it would be helpful to say so. If the former,
>> it could really use more usage information/documentation...
> 
> I'm not sure what end users would use this for, but it should do its work just
> the same.. What kind of usage information do you suggest? More explaination in
> the header, or in the "usage" output when no target is specified, or a "-h"
> option for help?
> 
>>
>>> +
>>> +set -e
>>> +
>>> +if test $# -lt 1; then
>>> +echo "Usage: $0 "
>>> +exit 1
>>> +fi
>>> +
>>> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
>>> +
>>> +if test -n "$submodules"; then
>>> +{
>>> +git ls-files
>>> +for sm in $submodules; do
>>> +(cd $sm; git ls-files) | sed "s:^:$sm/:"
>>> +done
>>> +} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
>>> $1.list
>>
>> Supporting '-e something' in a tar -T listfile seems to
>> be GNU tar specific. Do we care?
> 
> The "-e $sm" will go to the grep command line, so it isn't GNU specific, is 
> it?
> 

BSD OSes use GNU grep(1) the latest version GPLv2. There is a work in
progress BSD grep(1), but not turned on by default as it's slower.

The -e option is supported by SmartOS grep(1).

>>
>>> +else
>>> +git ls-files > $1.list
>>> +fi
>>
>> This will blow up if we ever have a file in the repo
>> that starts with a '-'. Do we care?
> 
> I'm sure such a file will be a trouble, but I'd rather we deal with it when
> there is one: I don't think anyone will think about adding, or welcome such a
> file.
> 
>>
>>> +
>>> +tar -cf $1 -T $1.list
>>> +rm $1.list
>>
>> This is missing a lot of quoting for $1, so it will go wrong
>> if there's a space in that filename argument.
> 
> Yes, good point. I will fix it before sending another pull request along with
> other comments that are just raised.
> 
> Fam
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 3/5] spapr_pci: use g_strdup_printf()

[Greg Kurz]

Building strings with g_strdup_printf() instead of snprintf() is
a QEMU common practice.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |   19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index cd8efb181223..6da73fe6bc29 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -61,8 +61,6 @@
 #define RTAS_TYPE_MSI   1
 #define RTAS_TYPE_MSIX  2
 
-#define FDT_NAME_MAX  128
-
 #define _FDT(exp) \
 do { \
 int ret = (exp);   \
@@ -1194,7 +1192,7 @@ static const char *pci_find_device_name(uint8_t class, 
uint8_t subclass,
 return name;
 }
 
-static void pci_get_node_name(char *nodename, int len, PCIDevice *dev)
+static gchar *pci_get_node_name(PCIDevice *dev)
 {
 int slot = PCI_SLOT(dev->devfn);
 int func = PCI_FUNC(dev->devfn);
@@ -1205,9 +1203,9 @@ static void pci_get_node_name(char *nodename, int len, 
PCIDevice *dev)
 ccode & 0xff);
 
 if (func != 0) {
-snprintf(nodename, len, "%s@%x,%x", name, slot, func);
+return g_strdup_printf("%s@%x,%x", name, slot, func);
 } else {
-snprintf(nodename, len, "%s@%x", name, slot);
+return g_strdup_printf("%s@%x", name, slot);
 }
 }
 
@@ -1325,10 +1323,12 @@ static int spapr_create_pci_child_dt(sPAPRPHBState 
*phb, PCIDevice *dev,
  void *fdt, int node_offset)
 {
 int offset, ret;
-char nodename[FDT_NAME_MAX];
+gchar *nodename;
 
-pci_get_node_name(nodename, FDT_NAME_MAX, dev);
+nodename = pci_get_node_name(dev);
 offset = fdt_add_subnode(fdt, node_offset, nodename);
+g_free(nodename);
+
 ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb);
 
 g_assert(!ret);
@@ -2072,7 +2072,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
   void *fdt)
 {
 int bus_off, i, j, ret;
-char nodename[FDT_NAME_MAX];
+gchar *nodename;
 uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
 struct {
 uint32_t hi;
@@ -2121,8 +2121,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
 sPAPRFDT s_fdt;
 
 /* Start populating the FDT */
-snprintf(nodename, FDT_NAME_MAX, "pci@%" PRIx64, phb->buid);
+nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
 bus_off = fdt_add_subnode(fdt, 0, nodename);
+g_free(nodename);
 if (bus_off < 0) {
 return bus_off;
 }

Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Fam Zheng]

On Sat, 09/09 13:07, Peter Maydell wrote:
> On 9 September 2017 at 06:45, Fam Zheng  wrote:
> > Signed-off-by: Fam Zheng 
> > Message-Id: <20170905021201.25684-4-f...@redhat.com>
> > ---
> >  scripts/archive-source.sh | 31 +++
> >  1 file changed, 31 insertions(+)
> >  create mode 100755 scripts/archive-source.sh
> >
> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> > new file mode 100755
> > index 00..1de369532e
> > --- /dev/null
> > +++ b/scripts/archive-source.sh
> > @@ -0,0 +1,31 @@
> > +#!/bin/sh
> > +#
> > +# Author: Fam Zheng 
> > +#
> > +# Create archive of source tree, including submodules
> > +#
> > +# This code is licensed under the GPL version 2 or later.  See
> > +# the COPYING file in the top-level directory.
> 
> So is this the script that for instance Mike Roth would use
> to create the release tarballs? If it isn't, should it be?

I don't know the workflow of release tarballs, there is ./scripts/make-release
for that. So this one is not.

> Is it intended for end users to create tarballs with, or
> is it really just a helper script for the docker stuff?
> If the latter, it would be helpful to say so. If the former,
> it could really use more usage information/documentation...

I'm not sure what end users would use this for, but it should do its work just
the same.. What kind of usage information do you suggest? More explaination in
the header, or in the "usage" output when no target is specified, or a "-h"
option for help?

> 
> > +
> > +set -e
> > +
> > +if test $# -lt 1; then
> > +echo "Usage: $0 "
> > +exit 1
> > +fi
> > +
> > +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> > +
> > +if test -n "$submodules"; then
> > +{
> > +git ls-files
> > +for sm in $submodules; do
> > +(cd $sm; git ls-files) | sed "s:^:$sm/:"
> > +done
> > +} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
> > $1.list
> 
> Supporting '-e something' in a tar -T listfile seems to
> be GNU tar specific. Do we care?

The "-e $sm" will go to the grep command line, so it isn't GNU specific, is it?

> 
> > +else
> > +git ls-files > $1.list
> > +fi
> 
> This will blow up if we ever have a file in the repo
> that starts with a '-'. Do we care?

I'm sure such a file will be a trouble, but I'd rather we deal with it when
there is one: I don't think anyone will think about adding, or welcome such a
file.

> 
> > +
> > +tar -cf $1 -T $1.list
> > +rm $1.list
> 
> This is missing a lot of quoting for $1, so it will go wrong
> if there's a space in that filename argument.

Yes, good point. I will fix it before sending another pull request along with
other comments that are just raised.

Fam

[Qemu-devel] [PATCH 5/5] spapr_pci: handle FDT creation errors with _FDT()

[Greg Kurz]

libfdt failures when creating the FDT should cause QEMU to terminate.

Let's use the _FDT() macro which does just that instead of propagating
the error to the caller. spapr_populate_pci_child_dt() no longer needs
to return a value in this case.

Note that, on the way, this get rids of the following nonsensical lines:

g_assert(!ret);
if (ret) {

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |   30 +++---
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index abb9f05e7b75..75cd9392233e 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1204,12 +1204,12 @@ static gchar *pci_get_node_name(PCIDevice *dev)
 static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb,
 PCIDevice *pdev);
 
-static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
+static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
sPAPRPHBState *sphb)
 {
 ResourceProps rp;
 bool is_bridge = false;
-int pci_status, err;
+int pci_status;
 char *buf = NULL;
 uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
 uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
@@ -1274,11 +1274,8 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
  ccode & 0xff)));
 
 buf = spapr_phb_get_loc_code(sphb, dev);
-err = fdt_setprop_string(fdt, offset, "ibm,loc-code", buf);
+_FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
 g_free(buf);
-if (err < 0) {
-return err;
-}
 
 if (drc_index) {
 _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
@@ -1306,27 +1303,21 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
 if (sphb->pcie_ecs && pci_is_express(dev)) {
 _FDT(fdt_setprop_cell(fdt, offset, "ibm,pci-config-space-type", 0x1));
 }
-
-return 0;
 }
 
 /* create OF node for pci device and required OF DT properties */
 static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
  void *fdt, int node_offset)
 {
-int offset, ret;
+int offset;
 gchar *nodename;
 
 nodename = pci_get_node_name(dev);
-offset = fdt_add_subnode(fdt, node_offset, nodename);
+_FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
 g_free(nodename);
 
-ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb);
+spapr_populate_pci_child_dt(dev, fdt, offset, phb);
 
-g_assert(!ret);
-if (ret) {
-return 0;
-}
 return offset;
 }
 
@@ -1416,10 +1407,6 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
 
 fdt = create_device_tree(_size);
 fdt_start_offset = spapr_create_pci_child_dt(phb, pdev, fdt, 0);
-if (!fdt_start_offset) {
-error_setg(_err, "Failed to create pci child device tree node");
-goto out;
-}
 
 spapr_drc_attach(drc, DEVICE(pdev), fdt, fdt_start_offset, _err);
 if (local_err) {
@@ -2114,11 +2101,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
 
 /* Start populating the FDT */
 nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
-bus_off = fdt_add_subnode(fdt, 0, nodename);
+_FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
 g_free(nodename);
-if (bus_off < 0) {
-return bus_off;
-}
 
 /* Write PHB properties */
 _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));

[Qemu-devel] [PATCH 1/5] spapr_pci: drop useless check in spapr_phb_vfio_get_loc_code()

[Greg Kurz]

g_strdup_printf() either returns a non-null pointer, either aborts
if it failed to allocate memory.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index d7880f257aa1..ef982f2ef370 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -766,7 +766,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState 
*sphb,  PCIDevice *pdev)
 /* Construct the path of the file that will give us the DT location */
 path = g_strdup_printf("/sys/bus/pci/devices/%s/devspec", host);
 g_free(host);
-if (!path || !g_file_get_contents(path, , NULL, NULL)) {
+if (!g_file_get_contents(path, , NULL, NULL)) {
 goto err_out;
 }
 g_free(path);
@@ -774,7 +774,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState 
*sphb,  PCIDevice *pdev)
 /* Construct and read from host device tree the loc-code */
 path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
 g_free(buf);
-if (!path || !g_file_get_contents(path, , NULL, NULL)) {
+if (!g_file_get_contents(path, , NULL, NULL)) {
 goto err_out;
 }
 return buf;

[Qemu-devel] [PATCH 4/5] spapr_pci: use the common _FDT() helper

[Greg Kurz]

All other users in hw/ppc already consider an error when building
the FDT to be fatal, even on hotplug paths. There's no valid reason
for spapr_pci to behave differently. So let's used the common _FDT()
helper which terminates QEMU when libfdt fails.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |   10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 6da73fe6bc29..abb9f05e7b75 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -40,7 +40,7 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "qapi/qmp/qerror.h"
-
+#include "hw/ppc/fdt.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_ids.h"
@@ -61,14 +61,6 @@
 #define RTAS_TYPE_MSI   1
 #define RTAS_TYPE_MSIX  2
 
-#define _FDT(exp) \
-do { \
-int ret = (exp);   \
-if (ret < 0) { \
-return ret;\
-}  \
-} while (0)
-
 sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t buid)
 {
 sPAPRPHBState *sphb;

[Qemu-devel] [PATCH 2/5] spapr_pci: drop useless check in spapr_populate_pci_child_dt()

[Greg Kurz]

spapr_phb_get_loc_code() either returns a non-null pointer, either
aborts if g_strdup_printf() failed to allocate memory.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index ef982f2ef370..cd8efb181223 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1282,12 +1282,8 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
 pci_find_device_name((ccode >> 16) & 0xff,
  (ccode >> 8) & 0xff,
  ccode & 0xff)));
-buf = spapr_phb_get_loc_code(sphb, dev);
-if (!buf) {
-error_report("Failed setting the ibm,loc-code");
-return -1;
-}
 
+buf = spapr_phb_get_loc_code(sphb, dev);
 err = fdt_setprop_string(fdt, offset, "ibm,loc-code", buf);
 g_free(buf);
 if (err < 0) {

[Qemu-devel] [PATCH 0/5] spapr_pci: various cleanups and improvements

[Greg Kurz]

Before resuming the huge work on PHB hotplug, here are some patches
that maybe worth to apply.

Patches 1 to 3 are basic improvements.

Patch 4 and 5 may be a bit controversial. Everywhere in the spapr
code where we build an FDT portion, libfdt failures cause QEMU to
exit, even on hotplug paths. Only spapr_pci doesn't do that and
propagates the error instead. My understanding is that a failure
when building the FDT is likely to happen because of a bug in QEMU.

Hence the choice to convert spapr_pci to do like the others. We may
even consider changing _FDT() to abort() instead of exit().

Alternatively, if libfdt failures shouldn't be necessarily fatal,
especially on post-realize paths, then we should probably introduce
an _FDT_ERR() helper to propagate errors. And use it in may places
where we currently terminate QEMU: memory hotplug, CPU hotplug, CAS,
machine reset...

--
Greg

---

Greg Kurz (5):
  spapr_pci: drop useless check in spapr_phb_vfio_get_loc_code()
  spapr_pci: drop useless check in spapr_populate_pci_child_dt()
  spapr_pci: use g_strdup_printf()
  spapr_pci: use the common _FDT() helper
  spapr_pci: handle FDT creation errors with _FDT()


 hw/ppc/spapr_pci.c |   69 
 1 file changed, 21 insertions(+), 48 deletions(-)

[Qemu-devel] [PATCH v2] scsi/esp: Rename the ESP macro to ESP_STATE

[Kamil Rytarowski]

SunOS defines ESP (x86 register) in  as 7.

This fixes build on SmartOS (Joyent).

Signed-off-by: Kamil Rytarowski 
---
 hw/scsi/esp.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index eee831efeb..22c2d91e39 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -593,7 +593,7 @@ const VMStateDescription vmstate_esp = {
 };
 
 #define TYPE_ESP "esp"
-#define ESP(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
+#define ESP_STATE(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
 
 typedef struct {
 /*< private >*/
@@ -644,7 +644,7 @@ void esp_init(hwaddr espaddr, int it_shift,
 ESPState *esp;
 
 dev = qdev_create(NULL, TYPE_ESP);
-sysbus = ESP(dev);
+sysbus = ESP_STATE(dev);
 esp = >esp;
 esp->dma_memory_read = dma_memory_read;
 esp->dma_memory_write = dma_memory_write;
@@ -672,7 +672,7 @@ static const struct SCSIBusInfo esp_scsi_info = {
 
 static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
 {
-SysBusESPState *sysbus = ESP(opaque);
+SysBusESPState *sysbus = ESP_STATE(opaque);
 ESPState *s = >esp;
 
 switch (irq) {
@@ -688,7 +688,7 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, 
int level)
 static void sysbus_esp_realize(DeviceState *dev, Error **errp)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-SysBusESPState *sysbus = ESP(dev);
+SysBusESPState *sysbus = ESP_STATE(dev);
 ESPState *s = >esp;
 
 sysbus_init_irq(sbd, >irq);
@@ -706,7 +706,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error 
**errp)
 
 static void sysbus_esp_hard_reset(DeviceState *dev)
 {
-SysBusESPState *sysbus = ESP(dev);
+SysBusESPState *sysbus = ESP_STATE(dev);
 esp_hard_reset(>esp);
 }
 
-- 
2.14.1

[Qemu-devel] [Bug 1615823] Re: Windows 10 reports no compatible TPM found yet device manager shows it?

[Kelvin Middleton]

@Nelson, please can you provide a little more info as I'm still having
problems with TPM preparation in Windows.  What make and version is your
TPM device.  How was your guest configured i.e. SeaBios vs. OVMF, Q35 or
i440FX?  Are you using kvm/qemu with libvirt or just qemu on the command
line?  Any info/pointers would be appreciated.

Thanks,

K

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

Title:
  Windows 10 reports no compatible TPM found yet device manager shows
  it?

Status in QEMU:
  New

Bug description:
  Ubuntu 16.04 with stock kvm, libvirt, ovmf
  Qemu 2.5 installed from stock ubuntu ppa
  Qemu 2.6.1 built from tarball.
  Qemu 2.7.0-rc4 built from tarball.

  Windows 10 guest reports a TPM device is installed and the driver
  functional under Device Manager-->Security Devices.  TPM Administrator
  however advises no compatible TPM chip can be found.

  Qemu 2.5 is buggy and prevents the guest loading the TPM driver, this
  was addressed by
  
http://git.qemu.org/?p=qemu.git;a=commit;h=2b1c2e8e5f1990f0a201a8cbf9d366fca60f4aa8

  Have tested the below cmd out on both qemu-2.6.1 and qemu-2.7.0-rc4,
  both suffer the same problem.  My TPM is most certainly compatible as
  installing Win10Pro onto the same host as bare metal provides me the
  desired and expected functionality aka Bitlocker and TPM Administrator
  work.

  sudo ./qemu-system-x86_64 \
  -enable-kvm \
  -machine q35 \
  -cpu host \
  -m 4096 \
  -smp 4,sockets=1,cores=2,threads=2 \
  -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
  -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
  -device 
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vgamem_mb=16,bus=pcie.0,addr=0x2
 \
  -drive file=/usr/share/qemu/OVMF.fd,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=/mnt/120GB_SSD/wintpm_VARS.fd,if=pflash,format=raw,unit=1 \
  -drive 
file=/mnt/120GB_SSD/wintpm.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
  -device 
virtio-blk-pci,scsi=off,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=2
 \
  -drive file="/mnt/share/Filestorage/Images/Microsoft Windows 10 Pro 
x64.iso",format=raw,if=none,media=cdrom,id=drive-sata0-0-0,readonly=on \
  -device ide-cd,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
  -drive 
file=/mnt/share/Filestorage/Images/virtio-win-0.1.117.iso,format=raw,if=none,media=cdrom,id=drive-sata0-0-1,readonly=on
 \
  -device ide-cd,bus=ide.1,drive=drive-sata0-0-1,id=sata0-0-1 \
  -tpmdev 
passthrough,id=tpm-tpm0,path=/dev/tpm0,cancel-path=/sys/class/tpm/tpm0/device/cancel
 \
  -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0

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

[Qemu-devel] [Bug 1716132] [NEW] Win 10 bitlocker won't initialise pass-through TPM

[Kelvin Middleton]

Public bug reported:

All stock Ubuntu Zesty, Win10Pro KVM guest configured with OVMF and Q35.
My host has an ASRock Z97 Extreme 6 board with a TPM header which is
populated with v1.2 complaint device.

Testing in my host the TPM device is function, I can tpm_takeownership
and tpm_clear successfully and similar testing by passing the device
through to a linux guest also succeeds.

However using Bitlocker in Windows 10 Pro release 1703 Windows advises
it cannot "Prepare" the device which I take to mean it cannot take
ownership of it.  I believe this to be related to Windows inability to
view the TCG Event Log which is evidenced in the below 2 screencaps,
however I'm no expert.

https://s26.postimg.org/vter35eh5/Screenshot_20170907_114644.png
https://s26.postimg.org/klo854qyx/Screenshot_20170909_143841.png

I've also tested the scenario with qemu 2.10 which provided the exact
same results.  The only difference in the test setup is that I had to
make the guest boot with SeaBios instead of OVMF.  (Windows wouldn't
boot with OVMF with the boot manager giving me an error pointing to a
BCD issue.  Researching this it seemed related to an old ACPI problem, I
believe this unrelated to my TPM issue so will do more research and
raise a separate bug for this if needed.)

Happy to provide further configurations and build logs as necessary so
please advise me what is needed.

Lastly for background reading.  I've been trying to get TPM passthrough
working with Windows for a long time now and have hit several different
issues which I believe have been addressed by both code maturity in Qemu
but also in Windows releases.  An earlier bug report can be found here
(https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1615722) which
concludes advising me to raise this new/separate issue.

Thanks in advance,

Kelvin

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Win 10 bitlocker won't initialise pass-through TPM

Status in QEMU:
  New

Bug description:
  All stock Ubuntu Zesty, Win10Pro KVM guest configured with OVMF and
  Q35.  My host has an ASRock Z97 Extreme 6 board with a TPM header
  which is populated with v1.2 complaint device.

  Testing in my host the TPM device is function, I can tpm_takeownership
  and tpm_clear successfully and similar testing by passing the device
  through to a linux guest also succeeds.

  However using Bitlocker in Windows 10 Pro release 1703 Windows advises
  it cannot "Prepare" the device which I take to mean it cannot take
  ownership of it.  I believe this to be related to Windows inability to
  view the TCG Event Log which is evidenced in the below 2 screencaps,
  however I'm no expert.

  https://s26.postimg.org/vter35eh5/Screenshot_20170907_114644.png
  https://s26.postimg.org/klo854qyx/Screenshot_20170909_143841.png

  I've also tested the scenario with qemu 2.10 which provided the exact
  same results.  The only difference in the test setup is that I had to
  make the guest boot with SeaBios instead of OVMF.  (Windows wouldn't
  boot with OVMF with the boot manager giving me an error pointing to a
  BCD issue.  Researching this it seemed related to an old ACPI problem,
  I believe this unrelated to my TPM issue so will do more research and
  raise a separate bug for this if needed.)

  Happy to provide further configurations and build logs as necessary so
  please advise me what is needed.

  Lastly for background reading.  I've been trying to get TPM
  passthrough working with Windows for a long time now and have hit
  several different issues which I believe have been addressed by both
  code maturity in Qemu but also in Windows releases.  An earlier bug
  report can be found here
  (https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1615722) which
  concludes advising me to raise this new/separate issue.

  Thanks in advance,

  Kelvin

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

Re: [Qemu-devel] [PATCH] scsi/esp: Rename the ESP symbol to QESP (QEMU ESP)

[Paolo Bonzini]

On 03/09/2017 18:36, Kamil Rytarowski wrote:
> SunOS defines ESP (x86 register) in  as 7.
> 
> This fixes build on SmartOS (Joyent).
> 
> Signed-off-by: Kamil Rytarowski 
> ---
>  hw/scsi/esp.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index eee831efeb..38ddfd6715 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -593,7 +593,7 @@ const VMStateDescription vmstate_esp = {
>  };
>  
>  #define TYPE_ESP "esp"
> -#define ESP(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
> +#define QESP(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
>  
>  typedef struct {
>  /*< private >*/
> @@ -644,7 +644,7 @@ void esp_init(hwaddr espaddr, int it_shift,
>  ESPState *esp;
>  
>  dev = qdev_create(NULL, TYPE_ESP);
> -sysbus = ESP(dev);
> +sysbus = QESP(dev);
>  esp = >esp;
>  esp->dma_memory_read = dma_memory_read;
>  esp->dma_memory_write = dma_memory_write;
> @@ -672,7 +672,7 @@ static const struct SCSIBusInfo esp_scsi_info = {
>  
>  static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
>  {
> -SysBusESPState *sysbus = ESP(opaque);
> +SysBusESPState *sysbus = QESP(opaque);
>  ESPState *s = >esp;
>  
>  switch (irq) {
> @@ -688,7 +688,7 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, 
> int level)
>  static void sysbus_esp_realize(DeviceState *dev, Error **errp)
>  {
>  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -SysBusESPState *sysbus = ESP(dev);
> +SysBusESPState *sysbus = QESP(dev);
>  ESPState *s = >esp;
>  
>  sysbus_init_irq(sbd, >irq);
> @@ -706,7 +706,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error 
> **errp)
>  
>  static void sysbus_esp_hard_reset(DeviceState *dev)
>  {
> -SysBusESPState *sysbus = ESP(dev);
> +SysBusESPState *sysbus = QESP(dev);
>  esp_hard_reset(>esp);
>  }
>  
> 

Sorry for bikeshedding, let's rename it to ESP_STATE instead.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions

[Paolo Bonzini]

On 06/09/2017 15:40, Dr. David Alan Gilbert wrote:
>> There's a race if someone does a 'stop' near the end of migrate;
>> the migration process goes through two runstates:
>> 'finish migrate'
>> 'postmigrate'
>>
>> If the user issues a 'stop' between the two we end up with invalid
>> state transitions.
>> Add the transitions as valid.
>>
>> Signed-off-by: Dr. David Alan Gilbert 
> Queued
> 
>> ---
>>  vl.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 99fcfa0442..bacb03f49d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -621,6 +621,7 @@ static const RunStateTransition 
>> runstate_transitions_def[] = {
>>  
>>  { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
>>  { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
>> +{ RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },

There is:

if (s->state == MIGRATION_STATUS_COMPLETED) {
...
runstate_set(RUN_STATE_POSTMIGRATE);
} else {
...
if (old_vm_running && !entered_postcopy) {
vm_start();
} else {
if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
runstate_set(RUN_STATE_POSTMIGRATE);
}
}
}

Maybe the runstate_check should be in the "then" branch too, instead
of adding this transition?

Paolo

>>  { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
>>  { RUN_STATE_PAUSED, RUN_STATE_COLO},
>>  
>> @@ -633,6 +634,7 @@ static const RunStateTransition 
>> runstate_transitions_def[] = {
>>  { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
>>  
>>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>> +{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
>>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
>> -- 
>> 2.13.3
>>
>>

Re: [Qemu-devel] [PATCH] vhost: Release memory references on cleanup

[Paolo Bonzini]

On 07/09/2017 22:27, Alex Williamson wrote:
> vhost registers a MemoryListener where it adds and removes references
> to MemoryRegions as the MemoryRegionSections pass through.  The
> region_add callback is invoked for each existing section when the
> MemoryListener is registered, but unregistering the MemoryListener
> performs no reciprocal region_del callback.  It's therefore the
> owner of the MemoryListener's responsibility to cleanup any persistent
> changes, such as these memory references, after unregistering.
> 
> The consequence of this bug is that if we have both a vhost device
> and a vfio device, the vhost device will reference any mmap'd MMIO of
> the vfio device via this MemoryListener.  If the vhost device is then
> removed, those references remain outstanding.  If we then attempt to
> remove the vfio device, it never gets finalized and the only way to
> release the kernel file descriptors is to terminate the QEMU process.
> 
> Fixes: dfde4e6e1a86 ("memory: add ref/unref calls")
> Cc: Michael S. Tsirkin 
> Cc: Paolo Bonzini 
> Cc: qemu-sta...@nongnu.org # v1.6.0+
> Signed-off-by: Alex Williamson 

This fix is correct.  It's surprising that it was only found now!

Thanks,

Paolo

> ---
>  hw/virtio/vhost.c |4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb099b02f..b737ca915b06 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1356,6 +1356,10 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
>  if (hdev->mem) {
>  /* those are only safe after successful init */
>  memory_listener_unregister(>memory_listener);
> +for (i = 0; i < hdev->n_mem_sections; ++i) {
> +MemoryRegionSection *section = >mem_sections[i];
> +memory_region_unref(section->mr);
> +}
>  QLIST_REMOVE(hdev, entry);
>  }
>  if (hdev->migration_blocker) {
> 

Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Peter Maydell]

On 9 September 2017 at 06:45, Fam Zheng  wrote:
> Signed-off-by: Fam Zheng 
> Message-Id: <20170905021201.25684-4-f...@redhat.com>
> ---
>  scripts/archive-source.sh | 31 +++
>  1 file changed, 31 insertions(+)
>  create mode 100755 scripts/archive-source.sh
>
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> new file mode 100755
> index 00..1de369532e
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +#
> +# Author: Fam Zheng 
> +#
> +# Create archive of source tree, including submodules
> +#
> +# This code is licensed under the GPL version 2 or later.  See
> +# the COPYING file in the top-level directory.

So is this the script that for instance Mike Roth would use
to create the release tarballs? If it isn't, should it be?
Is it intended for end users to create tarballs with, or
is it really just a helper script for the docker stuff?
If the latter, it would be helpful to say so. If the former,
it could really use more usage information/documentation...

> +
> +set -e
> +
> +if test $# -lt 1; then
> +echo "Usage: $0 "
> +exit 1
> +fi
> +
> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> +
> +if test -n "$submodules"; then
> +{
> +git ls-files
> +for sm in $submodules; do
> +(cd $sm; git ls-files) | sed "s:^:$sm/:"
> +done
> +} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list

Supporting '-e something' in a tar -T listfile seems to
be GNU tar specific. Do we care?

> +else
> +git ls-files > $1.list
> +fi

This will blow up if we ever have a file in the repo
that starts with a '-'. Do we care?

> +
> +tar -cf $1 -T $1.list
> +rm $1.list

This is missing a lot of quoting for $1, so it will go wrong
if there's a space in that filename argument.

thanks
-- PMM

[Qemu-devel] [Bug 1119861] Re: Poor console performance in Windows 7

[Francois Gouget]

It does seem to be ok now. The test did get simplified to remove parts
that were mostly redundant so it runs faster now. But still it now takes
the same time, 7 seconds, on the VMware and QEMU Windows 7 VMs. So as
far as I'm concerned this can be closed.

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

Title:
  Poor console performance in Windows 7

Status in QEMU:
  Incomplete

Bug description:
  As part of its conformance test suite, Wine tests the behavior of the
  Windows console API. Part of this test involves opening a test console
  and scrolling things around. The test probably does not need to
  perform that many scroll operations to achieve its goal. However as is
  it illustrates a significant performance issue in QEmu. Unfortunately
  it does so by timing out (the tests must run in less than 2 minutes).
  Here are the run times on a few configurations:

   10s - QEmu 1.4 + Q9450@2.6GHz + Windows XP + QXL + QXL driver
8s - QEmu 1.12 + Opteron 6128 + Windows XP + QXL + QXL driver
  127s - QEmu 1.12 + Opteron 6128 + Windows 7 + cirrus + vga driver
  127s - QEmu 1.12 + Opteron 6128 + Windows 7 + QXL + QXL driver
  147s - QEmu 1.12 + Opteron 6128 + Windows 7 + vmvga + vga driver
  145s - QEmu 1.12 + Opteron 6128 + Windows 7 + vmvga + vmware driver (xpdm, no 
better with all graphics effects disabled)

   10s - Metal + Atom N270 + Windows XP + GMA 950 + Intel driver
6s - Metal + i5-3317U + Windows 8 + HD4000 + Intel driver
3s - VMware + Q9450@2.6GHz + Windows XP + vmvga + vmware driver
   65s - VMware + Q9450@2.6GHz + Windows 7 + vmvga + vmware driver

  So when running on the bare metal all versions of Windows are about as
  fast. However in QEmu Windows 7 is almost 16 times slower than Windows
  XP! VMware is impacted too but it's still maintains a good lead in
  performance.

  Disabling all graphics effects did not help so it's not clear that the
  fault lies with Windows 7's compositing desktop window manager. Maybe
  it has to do with the lack of a proper wddm driver?

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

Re: [Qemu-devel] [PATCH] usb/dev-smartcard-reader: convert to realize

[Marc-André Lureau]

Hi

On Sat, Sep 9, 2017 at 10:33 AM Mao Zhongyi 
wrote:

> Convert CCID Device to realize and rename the ccid_card_init()
> to ccid_card_realize().
>
> Cc: Gerd Hoffmann 
> Signed-off-by: Mao Zhongyi 
> ---
>  hw/usb/dev-smartcard-reader.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index bef1f03..27cd48d 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1306,27 +1306,25 @@ static int ccid_card_exit(DeviceState *qdev)
>  return 0;
>  }
>
> -static int ccid_card_init(DeviceState *qdev)
> +static void ccid_card_realize(DeviceState *qdev, Error **errp)
>  {
>  CCIDCardState *card = CCID_CARD(qdev);
>  USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
>  USBCCIDState *s = USB_CCID_DEV(dev);
> -int ret = 0;
>
>  if (card->slot != 0) {
> -warn_report("usb-ccid supports one slot, can't add %d",
> +error_setg(errp, "usb-ccid supports one slot, can't add %d",
>  card->slot);
> -return -1;
> +return;
>  }
>  if (s->card != NULL) {
> -warn_report("usb-ccid card already full, not adding");
> -return -1;
> +error_setg(errp, "usb-ccid card already full, not adding");
> +return;
>  }
> -ret = ccid_card_initfn(card);
> -if (ret == 0) {
> +
> +if (!ccid_card_initfn(card)) {
>  s->card = card;
>  }
>

You should set an error here, or the caller won't be aware of card init
error anymore. Best would be to convert ccid_card_initfn() and callbacks to
take an Error **.

-return ret;
>  }
>
>  static void ccid_realize(USBDevice *dev, Error **errp)
> @@ -1496,7 +1494,7 @@ static void ccid_card_class_init(ObjectClass *klass,
> void *data)
>  {
>  DeviceClass *k = DEVICE_CLASS(klass);
>  k->bus_type = TYPE_CCID_BUS;
> -k->init = ccid_card_init;
> +k->realize = ccid_card_realize;
>  k->exit = ccid_card_exit;
>  k->props = ccid_props;
>  }
> --
> 2.9.4
>
>
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [RFC PATCH 16/26] ppc/xive: notify CPU when interrupt priority is more privileged

[Benjamin Herrenschmidt]

On Sat, 2017-09-09 at 10:08 +0200, Cédric Le Goater wrote:
> On 09/09/2017 09:39 AM, Benjamin Herrenschmidt wrote:
> > On Wed, 2017-07-05 at 19:13 +0200, Cédric Le Goater wrote:
> > > Signed-off-by: Cédric Le Goater 
> > > ---
> > >  hw/intc/xive.c | 21 +
> > >  1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> > > index c3c1e9c9db2d..cda1fa18e44d 100644
> > > --- a/hw/intc/xive.c
> > > +++ b/hw/intc/xive.c
> > > @@ -53,6 +53,21 @@ static uint64_t xive_icp_accept(XiveICPState *xicp)
> > >  return (nsr << 8) | xicp->tima_os[TM_CPPR];
> > >  }
> > >  
> > > +static uint8_t ipb_to_pipr(uint8_t ibp)
> > > +{
> > > +return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
> > > +}
> > 
> > The PIPR needs to be updated also on accept etc... anything that change
> > IPBs or CPPR really.
> 
> I did forget the update in accept ... thanks. 
> 
> > Also I just learned something from the designers: The IPIR is clamped
> > to CPPR.
> > 
> > So basically the value in the PIPR is:
> > 
> > v = leftmost_bit_of(ipb) (or 0xff);
> > pipr = v < cppr ? v : cppr;
> > 
> > which means it's never actually 0xff ... surprise !
> 
> ah. that is not what the specs say but it shouldn't be a problem.
> I will change the code to reflect that.

Right, I've asked the HW guys to update the spec.

Cheers,
Ben.

> C.
> 
> > That also means I need to fix my implementation of H_IPOLL in KVM.
> > 
> > > +static void xive_icp_notify(XiveICPState *xicp)
> > > +{
> > > +xicp->tima_os[TM_PIPR] = ipb_to_pipr(xicp->tima_os[TM_IPB]);
> > > +
> > > +if (xicp->tima_os[TM_PIPR] < xicp->tima_os[TM_CPPR]) {
> > > +xicp->tima_os[TM_NSR] |= TM_QW1_NSR_EO;
> > > +qemu_irq_raise(ICP(xicp)->output);
> > > +}
> > > +}
> > > +
> > >  static void xive_icp_set_cppr(XiveICPState *xicp, uint8_t cppr)
> > >  {
> > >  if (cppr > XIVE_PRIORITY_MAX) {
> > > @@ -60,6 +75,10 @@ static void xive_icp_set_cppr(XiveICPState *xicp, 
> > > uint8_t cppr)
> > >  }
> > >  
> > >  xicp->tima_os[TM_CPPR] = cppr;
> > > +
> > > +/* CPPR has changed, inform the ICP which might raise an
> > > + * exception */
> > > +xive_icp_notify(xicp);
> > >  }
> > >  
> > >  /*
> > > @@ -339,6 +358,8 @@ static void xive_icp_irq(XiveICSState *xs, int lisn)
> > >  } else {
> > >  qemu_log_mask(LOG_UNIMP, "XIVE: w7 format1 not implemented\n");
> > >  }
> > > +
> > > +xive_icp_notify(xicp);
> > >  }
> > >  
> > >  /*

[Qemu-devel] [PATCH] usb/dev-smartcard-reader: convert to realize

[Mao Zhongyi]

Convert CCID Device to realize and rename the ccid_card_init()
to ccid_card_realize().

Cc: Gerd Hoffmann 
Signed-off-by: Mao Zhongyi 
---
 hw/usb/dev-smartcard-reader.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index bef1f03..27cd48d 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1306,27 +1306,25 @@ static int ccid_card_exit(DeviceState *qdev)
 return 0;
 }
 
-static int ccid_card_init(DeviceState *qdev)
+static void ccid_card_realize(DeviceState *qdev, Error **errp)
 {
 CCIDCardState *card = CCID_CARD(qdev);
 USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
 USBCCIDState *s = USB_CCID_DEV(dev);
-int ret = 0;
 
 if (card->slot != 0) {
-warn_report("usb-ccid supports one slot, can't add %d",
+error_setg(errp, "usb-ccid supports one slot, can't add %d",
 card->slot);
-return -1;
+return;
 }
 if (s->card != NULL) {
-warn_report("usb-ccid card already full, not adding");
-return -1;
+error_setg(errp, "usb-ccid card already full, not adding");
+return;
 }
-ret = ccid_card_initfn(card);
-if (ret == 0) {
+
+if (!ccid_card_initfn(card)) {
 s->card = card;
 }
-return ret;
 }
 
 static void ccid_realize(USBDevice *dev, Error **errp)
@@ -1496,7 +1494,7 @@ static void ccid_card_class_init(ObjectClass *klass, void 
*data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->bus_type = TYPE_CCID_BUS;
-k->init = ccid_card_init;
+k->realize = ccid_card_realize;
 k->exit = ccid_card_exit;
 k->props = ccid_props;
 }
-- 
2.9.4

Re: [Qemu-devel] [RFC PATCH 16/26] ppc/xive: notify CPU when interrupt priority is more privileged

[Cédric Le Goater]

On 09/09/2017 09:39 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2017-07-05 at 19:13 +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/xive.c | 21 +
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index c3c1e9c9db2d..cda1fa18e44d 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -53,6 +53,21 @@ static uint64_t xive_icp_accept(XiveICPState *xicp)
>>  return (nsr << 8) | xicp->tima_os[TM_CPPR];
>>  }
>>  
>> +static uint8_t ipb_to_pipr(uint8_t ibp)
>> +{
>> +return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
>> +}
> 
> The PIPR needs to be updated also on accept etc... anything that change
> IPBs or CPPR really.

but not for the SET_OS_PENDING special write I suppose.

C.

Re: [Qemu-devel] [RFC PATCH 16/26] ppc/xive: notify CPU when interrupt priority is more privileged

[Cédric Le Goater]

On 09/09/2017 09:39 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2017-07-05 at 19:13 +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/xive.c | 21 +
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index c3c1e9c9db2d..cda1fa18e44d 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -53,6 +53,21 @@ static uint64_t xive_icp_accept(XiveICPState *xicp)
>>  return (nsr << 8) | xicp->tima_os[TM_CPPR];
>>  }
>>  
>> +static uint8_t ipb_to_pipr(uint8_t ibp)
>> +{
>> +return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
>> +}
> 
> The PIPR needs to be updated also on accept etc... anything that change
> IPBs or CPPR really.

I did forget the update in accept ... thanks. 

> Also I just learned something from the designers: The IPIR is clamped
> to CPPR.
> 
> So basically the value in the PIPR is:
> 
>   v = leftmost_bit_of(ipb) (or 0xff);
>   pipr = v < cppr ? v : cppr;
> 
> which means it's never actually 0xff ... surprise !

ah. that is not what the specs say but it shouldn't be a problem.
I will change the code to reflect that.

C.

> That also means I need to fix my implementation of H_IPOLL in KVM.
> 
>> +static void xive_icp_notify(XiveICPState *xicp)
>> +{
>> +xicp->tima_os[TM_PIPR] = ipb_to_pipr(xicp->tima_os[TM_IPB]);
>> +
>> +if (xicp->tima_os[TM_PIPR] < xicp->tima_os[TM_CPPR]) {
>> +xicp->tima_os[TM_NSR] |= TM_QW1_NSR_EO;
>> +qemu_irq_raise(ICP(xicp)->output);
>> +}
>> +}
>> +
>>  static void xive_icp_set_cppr(XiveICPState *xicp, uint8_t cppr)
>>  {
>>  if (cppr > XIVE_PRIORITY_MAX) {
>> @@ -60,6 +75,10 @@ static void xive_icp_set_cppr(XiveICPState *xicp, uint8_t 
>> cppr)
>>  }
>>  
>>  xicp->tima_os[TM_CPPR] = cppr;
>> +
>> +/* CPPR has changed, inform the ICP which might raise an
>> + * exception */
>> +xive_icp_notify(xicp);
>>  }
>>  
>>  /*
>> @@ -339,6 +358,8 @@ static void xive_icp_irq(XiveICSState *xs, int lisn)
>>  } else {
>>  qemu_log_mask(LOG_UNIMP, "XIVE: w7 format1 not implemented\n");
>>  }
>> +
>> +xive_icp_notify(xicp);
>>  }
>>  
>>  /*

Re: [Qemu-devel] [RFC PATCH 16/26] ppc/xive: notify CPU when interrupt priority is more privileged

[Benjamin Herrenschmidt]

On Wed, 2017-07-05 at 19:13 +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/xive.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index c3c1e9c9db2d..cda1fa18e44d 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -53,6 +53,21 @@ static uint64_t xive_icp_accept(XiveICPState *xicp)
>  return (nsr << 8) | xicp->tima_os[TM_CPPR];
>  }
>  
> +static uint8_t ipb_to_pipr(uint8_t ibp)
> +{
> +return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
> +}

The PIPR needs to be updated also on accept etc... anything that change
IPBs or CPPR really.

Also I just learned something from the designers: The IPIR is clamped
to CPPR.

So basically the value in the PIPR is:

v = leftmost_bit_of(ipb) (or 0xff);
pipr = v < cppr ? v : cppr;

which means it's never actually 0xff ... surprise !

That also means I need to fix my implementation of H_IPOLL in KVM.

> +static void xive_icp_notify(XiveICPState *xicp)
> +{
> +xicp->tima_os[TM_PIPR] = ipb_to_pipr(xicp->tima_os[TM_IPB]);
> +
> +if (xicp->tima_os[TM_PIPR] < xicp->tima_os[TM_CPPR]) {
> +xicp->tima_os[TM_NSR] |= TM_QW1_NSR_EO;
> +qemu_irq_raise(ICP(xicp)->output);
> +}
> +}
> +
>  static void xive_icp_set_cppr(XiveICPState *xicp, uint8_t cppr)
>  {
>  if (cppr > XIVE_PRIORITY_MAX) {
> @@ -60,6 +75,10 @@ static void xive_icp_set_cppr(XiveICPState *xicp, uint8_t 
> cppr)
>  }
>  
>  xicp->tima_os[TM_CPPR] = cppr;
> +
> +/* CPPR has changed, inform the ICP which might raise an
> + * exception */
> +xive_icp_notify(xicp);
>  }
>  
>  /*
> @@ -339,6 +358,8 @@ static void xive_icp_irq(XiveICSState *xs, int lisn)
>  } else {
>  qemu_log_mask(LOG_UNIMP, "XIVE: w7 format1 not implemented\n");
>  }
> +
> +xive_icp_notify(xicp);
>  }
>  
>  /*

Re: [Qemu-devel] [RFC PATCH qemu 2/4] memory: Prepare for shared flat views

[David Gibson]

On Thu, Sep 07, 2017 at 07:20:08PM +1000, Alexey Kardashevskiy wrote:
> We are going to share flat views and dispatch trees between address
> spaces. This moves bits around but no change in behaviour is expected
> here. The following patch will implement sharing.
> 
> This switches from AddressSpace to AddressSpaceDispatch as in many
> places this is what is used actually. Also, since multiple address
> spaces will be sharing a flat view, MemoryRegionSection cannot store
> the address space pointer, this changes it to AddressSpaceDispatch
> as well. As a not very obvious result, IOMMUTLBEntry also switched
> from AddressSpace to AddressSpaceDispatch to make
> address_space_get_iotlb_entry() work.

So, mostly this seems to be exposing AddressSpaceDispatch in more
places.  It's not obvious to me why that's useful.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus

[David Gibson]

On Wed, Sep 06, 2017 at 01:57:48PM +0530, Nikunj A Dadhania wrote:
> When the user does not provide the cpu topology, e.g. "-smp 4", machine fails 
> to
> initialize 4 cpus. Compute the chip per cores depending on the number of chips
> and smt threads.
> 
> Signed-off-by: Nikunj A Dadhania 

I don't understand why simply treating smp_cores as cores per chip is wrong.

> ---
>  hw/ppc/pnv.c | 20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 9724719..3fbaafb 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -642,7 +642,7 @@ static void ppc_powernv_init(MachineState *machine)
>  MemoryRegion *ram;
>  char *fw_filename;
>  long fw_size;
> -int i;
> +int i, cores_per_chip;
>  char *chip_typename;
>  PCIBus *pbus;
>  bool has_gfx = false;
> @@ -710,6 +710,22 @@ static void ppc_powernv_init(MachineState *machine)
>  }
>  
>  pnv->chips = g_new0(PnvChip *, pnv->num_chips);
> +
> +/* If user has specified number of cores, use it. Otherwise, compute it. 
> */
> +if (smp_cores != 1) {
> +cores_per_chip = smp_cores;
> +} else {
> +cores_per_chip = smp_cpus / (smp_threads * pnv->num_chips);
> +}
> +
> +if (smp_cpus != (smp_threads * pnv->num_chips * cores_per_chip)) {
> +error_report("cpu topology not balanced: "
> + "chips (%u) * cores (%u) * threads (%u) != "
> + "number of cpus (%u)",
> + pnv->num_chips, cores_per_chip, smp_threads, smp_cpus);
> +exit(1);
> +}
> +
>  for (i = 0; i < pnv->num_chips; i++) {
>  char chip_name[32];
>  Object *chip = object_new(chip_typename);
> @@ -728,7 +744,7 @@ static void ppc_powernv_init(MachineState *machine)
>  object_property_add_child(OBJECT(pnv), chip_name, chip, 
> _fatal);
>  object_property_set_int(chip, PNV_CHIP_HWID(i), "chip-id",
>  _fatal);
> -object_property_set_int(chip, smp_cores, "nr-cores", _fatal);
> +object_property_set_int(chip, cores_per_chip, "nr-cores", 
> _fatal);
>  object_property_set_int(chip, 1, "num-phbs", _fatal);
>  object_property_set_bool(chip, true, "realized", _fatal);
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH] hw/ppc/spapr.c: cleaning up qdev_get_machine() calls

[David Gibson]

On Thu, Sep 07, 2017 at 05:21:23PM +0200, Greg Kurz wrote:
> On Wed,  6 Sep 2017 15:43:05 -0300
> Daniel Henrique Barboza  wrote:
> 
> > This patch removes the qdev_get_machine() calls that are made in
> > spapr.c in situations where we can get an existing pointer for
> > the MachineState by either passing it as an argument to the function
> > or by using other already available pointers.
> > 
> > The following changes were made:
> > 
> > - spapr_node0_size: static function that is called two times:
> > at spapr_setup_hpt_and_vrma and ppc_spapr_init. In both cases we can
> > pass an existing MachineState pointer to it.
> > 
> > - spapr_build_fdt: MachineState pointer can be retrieved from
> > the existing sPAPRMachineState pointer.
> > 
> > - spapr_boot_set: the opaque in the first arg is a sPAPRMachineState
> > pointer as we can see inside ppc_spapr_init:
> > 
> > qemu_register_boot_set(spapr_boot_set, spapr);
> > 
> > We can get a MachineState pointer from it.
> > 
> > - spapr_machine_device_plug and spapr_machine_device_unplug_request: the
> > MachineState, sPAPRMachineState, MachineClass and sPAPRMachineClass pointers
> > can all be retrieved from the HotplugHandler pointer.
> > 
> > Signed-off-by: Daniel Henrique Barboza 
> > ---
> 
> It makes sense for me.
> 
> Reviewed-by: Greg Kurz 

Applied to ppc-for-2.11.

> 
> >  hw/ppc/spapr.c | 19 +--
> >  1 file changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index cec441c..6ac3390 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -390,10 +390,8 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> > sPAPRMachineState *spapr)
> >  return ret;
> >  }
> >  
> > -static hwaddr spapr_node0_size(void)
> > +static hwaddr spapr_node0_size(MachineState *machine)
> >  {
> > -MachineState *machine = MACHINE(qdev_get_machine());
> > -
> >  if (nb_numa_nodes) {
> >  int i;
> >  for (i = 0; i < nb_numa_nodes; ++i) {
> > @@ -1027,7 +1025,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >   hwaddr rtas_addr,
> >   hwaddr rtas_size)
> >  {
> > -MachineState *machine = MACHINE(qdev_get_machine());
> > +MachineState *machine = MACHINE(spapr);
> >  MachineClass *mc = MACHINE_GET_CLASS(machine);
> >  sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> >  int ret;
> > @@ -1347,7 +1345,7 @@ void spapr_setup_hpt_and_vrma(sPAPRMachineState 
> > *spapr)
> >  spapr_reallocate_hpt(spapr, hpt_shift, _fatal);
> >  
> >  if (spapr->vrma_adjust) {
> > -spapr->rma_size = kvmppc_rma_size(spapr_node0_size(),
> > +spapr->rma_size = kvmppc_rma_size(spapr_node0_size(MACHINE(spapr)),
> >spapr->htab_shift);
> >  }
> >  /* We're setting up a hash table, so that means we're not radix */
> > @@ -2007,7 +2005,7 @@ static SaveVMHandlers savevm_htab_handlers = {
> >  static void spapr_boot_set(void *opaque, const char *boot_device,
> > Error **errp)
> >  {
> > -MachineState *machine = MACHINE(qdev_get_machine());
> > +MachineState *machine = MACHINE(opaque);
> >  machine->boot_order = g_strdup(boot_device);
> >  }
> >  
> > @@ -2154,7 +2152,7 @@ static void ppc_spapr_init(MachineState *machine)
> >  MemoryRegion *rma_region;
> >  void *rma = NULL;
> >  hwaddr rma_alloc_size;
> > -hwaddr node0_size = spapr_node0_size();
> > +hwaddr node0_size = spapr_node0_size(machine);
> >  long load_limit, fw_size;
> >  char *filename;
> >  Error *resize_hpt_err = NULL;
> > @@ -3198,7 +3196,8 @@ out:
> >  static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >DeviceState *dev, Error **errp)
> >  {
> > -sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> > +MachineState *ms = MACHINE(hotplug_dev);
> > +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> >  
> >  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >  int node;
> > @@ -3247,8 +3246,8 @@ static void spapr_machine_device_plug(HotplugHandler 
> > *hotplug_dev,
> >  static void spapr_machine_device_unplug_request(HotplugHandler 
> > *hotplug_dev,
> >  DeviceState *dev, Error 
> > **errp)
> >  {
> > -sPAPRMachineState *sms = SPAPR_MACHINE(qdev_get_machine());
> > -MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> > +sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > +MachineClass *mc = MACHINE_GET_CLASS(sms);
> >  
> >  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >  if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> 



-- 
David Gibson| I'll have my music baroque, and my code
david AT 

Re: [Qemu-devel] [PATCHv2 1/1] net: Add SunGEM device emulation as found on Apple UniNorth

[David Gibson]

On Wed, Sep 06, 2017 at 07:19:55PM +0100, Mark Cave-Ayland wrote:
> From: Benjamin Herrenschmidt 
> 
> This adds a simplistic emulation of the Sun GEM ethernet controller
> found in Apple ASICs.
> 
> Currently we only support the Apple UniNorth 1.x variant, but the
> other Apple or Sun variants should mostly be a matter of adding
> PCI IDs options.
> 
> We have a very primitive emulation of a single Broadcom 5201 PHY
> which is supported by the MacOS driver.
> 
> This model brings out-of-the-box networking to MacOS 9, and all
> versions of OS X I tried with the mac99 platform.
> 
> Further improvements from Mark:
> - Remove sungem.h file, moving constants into sungem.c as required
> - Switch to using tracepoints for debugging
> - Split register blocks into separate memory regions
> - Use arrays in SunGEMState to hold register values
> - Add state-saving support
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Signed-off-by: Mark Cave-Ayland 

Applied to ppc-for-2.11.

> ---
>  default-configs/ppc-softmmu.mak |1 +
>  hw/net/Makefile.objs|1 +
>  hw/net/sungem.c | 1447 
> +++
>  hw/net/trace-events |   44 ++
>  hw/pci/pci.c|2 +
>  include/hw/pci/pci_ids.h|1 +
>  6 files changed, 1496 insertions(+)
>  create mode 100644 hw/net/sungem.c
> 
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index 1f1cd85..c12ba9e 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -17,6 +17,7 @@ CONFIG_PREP_PCI=y
>  CONFIG_I82378=y
>  CONFIG_PC87312=y
>  CONFIG_MACIO=y
> +CONFIG_SUNGEM=y
>  CONFIG_PCSPK=y
>  CONFIG_CS4231A=y
>  CONFIG_CUDA=y
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 5ddaffe..7e87d01 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -27,6 +27,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_gem.o
>  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
>  common-obj-$(CONFIG_LANCE) += lance.o
>  common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
> +common-obj-$(CONFIG_SUNGEM) += sungem.o
>  
>  obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
>  obj-$(CONFIG_COLDFIRE) += mcf_fec.o
> diff --git a/hw/net/sungem.c b/hw/net/sungem.c
> new file mode 100644
> index 000..dffa0c9
> --- /dev/null
> +++ b/hw/net/sungem.c
> @@ -0,0 +1,1447 @@
> +/*
> + * QEMU model of SUN GEM ethernet controller
> + *
> + * As found in Apple ASICs among others
> + *
> + * Copyright 2016 Ben Herrenschmidt
> + * Copyright 2017 Mark Cave-Ayland
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "qemu/log.h"
> +#include "net/net.h"
> +#include "net/checksum.h"
> +#include "hw/net/mii.h"
> +#include "sysemu/sysemu.h"
> +#include "trace.h"
> +/* For crc32 */
> +#include 
> +
> +#define TYPE_SUNGEM "sungem"
> +
> +#define SUNGEM(obj) OBJECT_CHECK(SunGEMState, (obj), TYPE_SUNGEM)
> +
> +#define MAX_PACKET_SIZE 9016
> +
> +#define SUNGEM_MMIO_SIZE0x20
> +
> +/* Global registers */
> +#define SUNGEM_MMIO_GREG_SIZE   0x2000
> +
> +#define GREG_SEBSTATE 0xUL/* SEB State Register */
> +
> +#define GREG_STAT 0x000CUL/* Status Register */
> +#define GREG_STAT_TXINTME 0x0001/* TX INTME frame transferred */
> +#define GREG_STAT_TXALL   0x0002/* All TX frames transferred */
> +#define GREG_STAT_TXDONE  0x0004/* One TX frame transferred */
> +#define GREG_STAT_RXDONE  0x0010/* One RX frame arrived */
> +#define GREG_STAT_RXNOBUF 0x0020/* No free RX buffers available 
> */
> +#define GREG_STAT_RXTAGERR0x0040/* RX tag framing is corrupt */
> +#define GREG_STAT_TXMAC   0x4000/* TX MAC signalled interrupt */
> +#define GREG_STAT_RXMAC   0x8000/* RX MAC signalled interrupt */
> +#define GREG_STAT_MAC 0x0001/* MAC Control signalled irq */
> +#define GREG_STAT_TXNR0xfff8/* == TXDMA_TXDONE reg val */
> +#define GREG_STAT_TXNR_SHIFT  19
> +
> +/* These interrupts are edge latches in the status register,
> + * reading it (or writing the corresponding bit in IACK) will
> + * clear them
> + */
> +#define GREG_STAT_LATCH   (GREG_STAT_TXALL  | GREG_STAT_TXINTME | \
> +   GREG_STAT_RXDONE | GREG_STAT_RXDONE |  \
> +   GREG_STAT_RXNOBUF | GREG_STAT_RXTAGERR)
> +
> +#define GREG_IMASK0x0010UL/* Interrupt Mask Register */
> +#define GREG_IACK 0x0014UL/* Interrupt ACK Register */
> +#define GREG_STAT20x001CUL/* Alias of GREG_STAT */
> +#define GREG_PCIESTAT 0x1000UL/* PCI Error Status Register */
> +#define GREG_PCIEMASK 0x1004UL/* PCI Error Mask Register */
> +
> +#define GREG_SWRST0x1010UL/* Software Reset Register */
> +#define GREG_SWRST_TXRST  0x0001/* 

[Qemu-devel] [PATCH v4] pci: Set err to errp directly rather than through error_propagate()

[Mao Zhongyi]

ioh3420_interrupts_init() pass error message to local_err, then
propagate it to errp by error_propagate(), which is not necessary.
So eliminate it and pass errp directly instead of local_err.

Cc: "Michael S. Tsirkin" 
Cc: Marcel Apfelbaum  
Signed-off-by: Mao Zhongyi 
Reviewed-by: Eric Blake 
Reviewed-by: Marcel Apfelbaum 
Reviewed-by: Philippe Mathieu-Daudé 
---
v4:
it not apply clearly on HEAD, so rebase it and modify the commit
message.
---

 hw/pci-bridge/ioh3420.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index da4e5bd..5f56a2f 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -64,15 +64,13 @@ static uint8_t ioh3420_aer_vector(const PCIDevice *d)
 static int ioh3420_interrupts_init(PCIDevice *d, Error **errp)
 {
 int rc;
-Error *local_err = NULL;
 
 rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
   IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
   IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
-  _err);
+  errp);
 if (rc < 0) {
 assert(rc == -ENOTSUP);
-error_propagate(errp, local_err);
 }
 
 return rc;
-- 
2.9.4

Re: [Qemu-devel] [PULL v2 00/37] Staging patches

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PULL v2 00/37] Staging patches
Message-id: 20170909054613.19148-1-f...@redhat.com
Type: series

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

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

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
 t [tag update]
patchew/1504815384-8490-1-git-send-email-stef...@linux.vnet.ibm.com -> 
patchew/1504815384-8490-1-git-send-email-stef...@linux.vnet.ibm.com
 t [tag update]
patchew/20170908163859.29820-1-richard.hender...@linaro.org -> 
patchew/20170908163859.29820-1-richard.hender...@linaro.org
 * [new tag]   patchew/20170909054613.19148-1-f...@redhat.com -> 
patchew/20170909054613.19148-1-f...@redhat.com
Switched to a new branch 'test'
274b0b33cb buildsys: Move rdma libs to per object
dee13ca0ca buildsys: Move brlapi libs to per object
51486e4caf buildsys: Move usb redir cflags/libs to per object
e2d2141400 buildsys: Move libusb cflags/libs to per object
e653260901 buildsys: Move libcacard cflags/libs to per object
de80133478 buildsys: Move curese cflags/libs to per object
4da943d9c6 buildsys: Move audio libs to per object
c05be913ea buildsys: Move vnc cflags/libs to per object
066db85c1a buildsys: Move sdl cflags/libs to per object
50200dfb4c buildsys: Move gtk/vte cflags/libs to per object
0832346d7d buildsys: Move vde libs to per object
5cad9f269f vl: Don't include vde header
d642c77e93 docker: Drop 'set -e' from run script
05cb7137d9 docker: Add test-block
f829f5d141 docker: Add nettle-devel to fedora image
ca0c6ffecc docker: Use unconfined security profile
c5a6c192c7 docker: Add test_fail and prep_fail
ee94d2b7c9 docker: Fix return code of build_qemu()
0717923a55 docker: Use archive-source.py
b6a02aee1a tests: Add README for vm tests
f0bfdb4c3b MAINTAINERS: Add tests/vm entry
133ce1feb5 Makefile: Add rules to run vm tests
f88480f6c8 tests: Add OpenBSD image
d05508ab23 tests: Add NetBSD image
8370cb8f2f tests: Add FreeBSD image
a8b70bfa3b tests: Add ubuntu.i386 image
413861ba2d tests: Add vm test lib
e10c67002d scripts: Add archive-source.sh
c353b7acd2 qemu.py: Add "wait()" method
fac58fe6c0 gitignore: Ignore vm test images
1022469bf7 tests/docker: Clean up paths
e6dde9693e docker: Enable features explicitly in test-full
f08e8a2903 docker: Update ubuntu image
6c2c01bbe2 docker: reduce noise when building travis.docker
f63179c5a0 docker: don't install device-tree-compiler build-deps in 
travis.docker
f9d3a9d011 docker: docker.py make --no-cache skip checksum test
3695d96c65 docker: ensure NOUSER for travis images

=== OUTPUT BEGIN ===
Checking PATCH 1/37: docker: ensure NOUSER for travis images...
Checking PATCH 2/37: docker: docker.py make --no-cache skip checksum test...
Checking PATCH 3/37: docker: don't install device-tree-compiler build-deps in 
travis.docker...
Checking PATCH 4/37: docker: reduce noise when building travis.docker...
Checking PATCH 5/37: docker: Update ubuntu image...
Checking PATCH 6/37: docker: Enable features explicitly in test-full...
Checking PATCH 7/37: tests/docker: Clean up paths...
Checking PATCH 8/37: gitignore: Ignore vm test images...
Checking PATCH 9/37: qemu.py: Add "wait()" method...
Checking PATCH 10/37: scripts: Add archive-source.sh...
Checking PATCH 11/37: tests: Add vm test lib...
ERROR: line over 90 characters
#86: FILE: tests/vm/basevm.py:60:
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key

WARNING: line over 80 characters
#100: FILE: tests/vm/basevm.py:74:
+self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", 
dir=".")

WARNING: line over 80 characters
#191: FILE: tests/vm/basevm.py:165:
+logging.debug("Creating archive %s for src_dir dir: %s", tarfile, 
src_dir)

WARNING: line over 80 characters
#196: FILE: tests/vm/basevm.py:170:
+"file=%s,if=none,id=%s,cache=writeback,format=raw" 
% \

WARNING: line over 80 characters
#199: FILE: tests/vm/basevm.py:173:
+"virtio-blk,drive=%s,serial=%s,bootindex=1" % 
(name, name)]

ERROR: line over 90 characters
#246: FILE: tests/vm/basevm.py:220:
+VM test 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/3] spapr: XIVE and CAS fixes

[Cédric Le Goater]

On 09/08/2017 04:33 PM, Cédric Le Goater wrote:
> Hello,
> 
> Here are a couple of fixes, one for the XIVE exploitation mode option
> bit which is clearly specified now and for one for the CAS generated
> resets.
> 
> Last is a proposal to handle the rebuild of the device tree using a
> reset when the XIVE exploitation mode is negotiated with CAS. 

I should have copied Sam on this topic for some insights. 

Thanks,

C. 


> The full tree is here:
> 
>   https://github.com/legoater/qemu/commits/xive
> 
> XIVE support for Linux is now merged in 4.14.
> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (3):
>   ppc/xive: fix OV5_XIVE_EXPLOIT bits
>   spapr: fix CAS-generated reset
>   spapr: generate a CAS reset for XIVE exploitation mode
> 
>  hw/ppc/spapr.c  |  8 ++--
>  hw/ppc/spapr_hcall.c| 13 +
>  include/hw/ppc/spapr_ovec.h |  3 ++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 

[Qemu-devel] [PULL v2 35/37] buildsys: Move usb redir cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-10-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 4 ++--
 hw/usb/Makefile.objs | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index b50d662cde..85ea2c25bb 100755
--- a/configure
+++ b/configure
@@ -4264,8 +4264,6 @@ if test "$usb_redir" != "no" ; then
 usb_redir="yes"
 usb_redir_cflags=$($pkg_config --cflags libusbredirparser-0.5)
 usb_redir_libs=$($pkg_config --libs libusbredirparser-0.5)
-QEMU_CFLAGS="$QEMU_CFLAGS $usb_redir_cflags"
-libs_softmmu="$libs_softmmu $usb_redir_libs"
 else
 if test "$usb_redir" = "yes"; then
 feature_not_found "usb-redir" "Install usbredir devel"
@@ -5828,6 +5826,8 @@ fi
 
 if test "$usb_redir" = "yes" ; then
   echo "CONFIG_USB_REDIR=y" >> $config_host_mak
+  echo "USB_REDIR_CFLAGS=$usb_redir_cflags" >> $config_host_mak
+  echo "USB_REDIR_LIBS=$usb_redir_libs" >> $config_host_mak
 fi
 
 if test "$opengl" = "yes" ; then
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 9ce9eadb47..92eb199697 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -38,6 +38,8 @@ endif
 
 # usb redirection
 common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
+redirect.o-cflags = $(USB_REDIR_CFLAGS)
+redirect.o-libs = $(USB_REDIR_LIBS)
 
 # usb pass-through
 common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
-- 
2.13.5

[Qemu-devel] [PULL v2 36/37] buildsys: Move brlapi libs to per object

[Fam Zheng]

baum.o already receives the sdl cflags in its per object variable, do
the same for brlapi libs to avoid cluttering libs_softmmu.

Signed-off-by: Fam Zheng 
Message-Id: <20170907084700.952-1-f...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Fam Zheng 
---
 chardev/Makefile.objs | 1 +
 configure | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs
index 52a8127606..d68e1347f9 100644
--- a/chardev/Makefile.objs
+++ b/chardev/Makefile.objs
@@ -20,5 +20,6 @@ chardev-obj-$(CONFIG_WIN32) += char-win-stdio.o
 common-obj-y += msmouse.o wctablet.o testdev.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 baum.o-cflags := $(SDL_CFLAGS)
+baum.o-libs := $(BRLAPI_LIBS)
 
 common-obj-$(CONFIG_SPICE) += spice.o
diff --git a/configure b/configure
index 85ea2c25bb..18f6d0bd5f 100755
--- a/configure
+++ b/configure
@@ -3091,7 +3091,6 @@ int main( void ) { return brlapi__openConnection (NULL, 
NULL, NULL); }
 EOF
   if compile_prog "" "$brlapi_libs" ; then
 brlapi=yes
-libs_softmmu="$brlapi_libs $libs_softmmu"
   else
 if test "$brlapi" = "yes" ; then
   feature_not_found "brlapi" "Install brlapi devel"
@@ -5679,6 +5678,7 @@ if test "$curl" = "yes" ; then
 fi
 if test "$brlapi" = "yes" ; then
   echo "CONFIG_BRLAPI=y" >> $config_host_mak
+  echo "BRLAPI_LIBS=$brlapi_libs" >> $config_host_mak
 fi
 if test "$bluez" = "yes" ; then
   echo "CONFIG_BLUEZ=y" >> $config_host_mak
-- 
2.13.5

[Qemu-devel] [PULL v2 33/37] buildsys: Move libcacard cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-8-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 4 ++--
 hw/usb/Makefile.objs | 6 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 88eafe8a10..be50159161 100755
--- a/configure
+++ b/configure
@@ -4235,8 +4235,6 @@ if test "$smartcard" != "no"; then
 if $pkg_config libcacard; then
 libcacard_cflags=$($pkg_config --cflags libcacard)
 libcacard_libs=$($pkg_config --libs libcacard)
-QEMU_CFLAGS="$QEMU_CFLAGS $libcacard_cflags"
-libs_softmmu="$libs_softmmu $libcacard_libs"
 smartcard="yes"
 else
 if test "$smartcard" = "yes"; then
@@ -5820,6 +5818,8 @@ fi
 
 if test "$smartcard" = "yes" ; then
   echo "CONFIG_SMARTCARD=y" >> $config_host_mak
+  echo "SMARTCARD_CFLAGS=$libcacard_cflags" >> $config_host_mak
+  echo "SMARTCARD_LIBS=$libcacard_libs" >> $config_host_mak
 fi
 
 if test "$libusb" = "yes" ; then
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 97f1c4561a..795ff25a5e 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -26,8 +26,10 @@ common-obj-$(CONFIG_USB_BLUETOOTH)+= dev-bluetooth.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y  += dev-smartcard-reader.o
-common-obj-$(CONFIG_SMARTCARD)+= ccid-card-passthru.o
-common-obj-$(CONFIG_SMARTCARD)+= ccid-card-emulated.o
+common-obj-$(CONFIG_SMARTCARD)+= smartcard.mo
+smartcard.mo-objs := ccid-card-passthru.o ccid-card-emulated.o
+smartcard.mo-cflags := $(SMARTCARD_CFLAGS)
+smartcard.mo-libs := $(SMARTCARD_LIBS)
 endif
 
 ifeq ($(CONFIG_POSIX),y)
-- 
2.13.5

[Qemu-devel] [PULL v2 32/37] buildsys: Move curese cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-6-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 6 --
 ui/Makefile.objs | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 5c15e74310..88eafe8a10 100755
--- a/configure
+++ b/configure
@@ -3134,8 +3134,8 @@ EOF
   unset IFS
   if compile_prog "$curses_inc" "$curses_lib" ; then
 curses_found=yes
-QEMU_CFLAGS="$curses_inc $QEMU_CFLAGS"
-libs_softmmu="$curses_lib $libs_softmmu"
+curses_cflags="$curses_inc $curses_cflags"
+curses_libs="$curses_lib $curses_libs"
 break
   fi
 done
@@ -5600,6 +5600,8 @@ if test "$cocoa" = "yes" ; then
 fi
 if test "$curses" = "yes" ; then
   echo "CONFIG_CURSES=y" >> $config_host_mak
+  echo "CURSES_CFLAGS=$curses_cflags" >> $config_host_mak
+  echo "CURSES_LIBS=$curses_libs" >> $config_host_mak
 fi
 if test "$pipe2" = "yes" ; then
   echo "CONFIG_PIPE2=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index a0b3c15a28..e3403b698b 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -57,3 +57,6 @@ gtk-egl.o-libs += $(OPENGL_LIBS)
 shader.o-libs += $(OPENGL_LIBS)
 console-gl.o-libs += $(OPENGL_LIBS)
 egl-helpers.o-libs += $(OPENGL_LIBS)
+
+curses.o-cflags := $(CURSES_CFLAGS)
+curses.o-libs := $(CURSES_LIBS)
-- 
2.13.5

[Qemu-devel] [PULL v2 37/37] buildsys: Move rdma libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907084230.26493-1-f...@redhat.com>
Reviewed-by: Juan Quintela 
Reviewed-by: Peter Xu 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Fam Zheng 
---
 configure   | 2 +-
 migration/Makefile.objs | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 18f6d0bd5f..ea54d8b882 100755
--- a/configure
+++ b/configure
@@ -2805,7 +2805,6 @@ EOF
   rdma_libs="-lrdmacm -libverbs"
   if compile_prog "" "$rdma_libs" ; then
 rdma="yes"
-libs_softmmu="$libs_softmmu $rdma_libs"
   else
 if test "$rdma" = "yes" ; then
 error_exit \
@@ -6028,6 +6027,7 @@ echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
 
 if test "$rdma" = "yes" ; then
   echo "CONFIG_RDMA=y" >> $config_host_mak
+  echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
 fi
 
 if test "$have_rtnetlink" = "yes" ; then
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 1c7770da46..99e038024d 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-$(CONFIG_RDMA) += rdma.o
 
 common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
 
+rdma.o-libs := $(RDMA_LIBS)
-- 
2.13.5

[Qemu-devel] [PULL v2 31/37] buildsys: Move audio libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-5-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 audio/Makefile.objs |  5 +
 configure   | 15 ++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/audio/Makefile.objs b/audio/Makefile.objs
index c20695668f..8a5ede6e2b 100644
--- a/audio/Makefile.objs
+++ b/audio/Makefile.objs
@@ -12,3 +12,8 @@ common-obj-y += wavcapture.o
 
 sdlaudio.o-cflags := $(SDL_CFLAGS)
 sdlaudio.o-libs := $(SDL_LIBS)
+alsaaudio.o-libs := $(ALSA_LIBS)
+paaudio.o-libs := $(PULSE_LIBS)
+coreaudio.o-libs := $(COREAUDIO_LIBS)
+dsoundaudio.o-libs := $(DSOUND_LIBS)
+ossaudio.o-libs := $(OSS_LIBS)
diff --git a/configure b/configure
index ffc93c7cdb..5c15e74310 100755
--- a/configure
+++ b/configure
@@ -3037,13 +3037,13 @@ for drv in $audio_drv_list; do
 alsa)
 audio_drv_probe $drv alsa/asoundlib.h -lasound \
 "return snd_pcm_close((snd_pcm_t *)0);"
-libs_softmmu="-lasound $libs_softmmu"
+alsa_libs="-lasound"
 ;;
 
 pa)
 audio_drv_probe $drv pulse/pulseaudio.h "-lpulse" \
 "pa_context_set_source_output_volume(NULL, 0, NULL, NULL, NULL); 
return 0;"
-libs_softmmu="-lpulse $libs_softmmu"
+pulse_libs="-lpulse"
 audio_pt_int="yes"
 ;;
 
@@ -3054,16 +3054,16 @@ for drv in $audio_drv_list; do
 ;;
 
 coreaudio)
-  libs_softmmu="-framework CoreAudio $libs_softmmu"
+  coreaudio_libs="-framework CoreAudio"
 ;;
 
 dsound)
-  libs_softmmu="-lole32 -ldxguid $libs_softmmu"
+  dsound_libs="-lole32 -ldxguid"
   audio_win_int="yes"
 ;;
 
 oss)
-  libs_softmmu="$oss_lib $libs_softmmu"
+  oss_libs="$oss_lib"
 ;;
 
 wav)
@@ -5542,6 +5542,11 @@ for drv in $audio_drv_list; do
 def=CONFIG_$(echo $drv | LC_ALL=C tr '[a-z]' '[A-Z]')
 echo "$def=y" >> $config_host_mak
 done
+echo "ALSA_LIBS=$alsa_libs" >> $config_host_mak
+echo "PULSE_LIBS=$pulse_libs" >> $config_host_mak
+echo "COREAUDIO_LIBS=$coreaudio_libs" >> $config_host_mak
+echo "DSOUND_LIBS=$dsound_libs" >> $config_host_mak
+echo "OSS_LIBS=$oss_libs" >> $config_host_mak
 if test "$audio_pt_int" = "yes" ; then
   echo "CONFIG_AUDIO_PT_INT=y" >> $config_host_mak
 fi
-- 
2.13.5

[Qemu-devel] [PULL v2 27/37] buildsys: Move vde libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907083552.17725-3-f...@redhat.com>
Reviewed-by: Jason Wang 
Signed-off-by: Fam Zheng 
---
 configure | 3 +--
 net/Makefile.objs | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 3918c47cd8..25c6ad6b20 100755
--- a/configure
+++ b/configure
@@ -2953,8 +2953,6 @@ int main(void)
 EOF
   if compile_prog "" "$vde_libs" ; then
 vde=yes
-libs_softmmu="$vde_libs $libs_softmmu"
-libs_tools="$vde_libs $libs_tools"
   else
 if test "$vde" = "yes" ; then
   feature_not_found "vde" "Install vde (Virtual Distributed Ethernet) 
devel"
@@ -5531,6 +5529,7 @@ if test "$slirp" = "yes" ; then
 fi
 if test "$vde" = "yes" ; then
   echo "CONFIG_VDE=y" >> $config_host_mak
+  echo "VDE_LIBS=$vde_libs" >> $config_host_mak
 fi
 if test "$netmap" = "yes" ; then
   echo "CONFIG_NETMAP=y" >> $config_host_mak
diff --git a/net/Makefile.objs b/net/Makefile.objs
index 67ba5e26fb..64adf32f40 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -21,3 +21,5 @@ tap-obj-$(CONFIG_SOLARIS) = tap-solaris.o
 tap-obj-y ?= tap-stub.o
 common-obj-$(CONFIG_POSIX) += tap.o $(tap-obj-y)
 common-obj-$(CONFIG_WIN32) += tap-win32.o
+
+vde.o-libs = $(VDE_LIBS)
-- 
2.13.5

[Qemu-devel] [PULL v2 34/37] buildsys: Move libusb cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-9-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 4 ++--
 hw/usb/Makefile.objs | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index be50159161..b50d662cde 100755
--- a/configure
+++ b/configure
@@ -4250,8 +4250,6 @@ if test "$libusb" != "no" ; then
 libusb="yes"
 libusb_cflags=$($pkg_config --cflags libusb-1.0)
 libusb_libs=$($pkg_config --libs libusb-1.0)
-QEMU_CFLAGS="$QEMU_CFLAGS $libusb_cflags"
-libs_softmmu="$libs_softmmu $libusb_libs"
 else
 if test "$libusb" = "yes"; then
 feature_not_found "libusb" "Install libusb devel >= 1.0.13"
@@ -5824,6 +5822,8 @@ fi
 
 if test "$libusb" = "yes" ; then
   echo "CONFIG_USB_LIBUSB=y" >> $config_host_mak
+  echo "LIBUSB_CFLAGS=$libusb_cflags" >> $config_host_mak
+  echo "LIBUSB_LIBS=$libusb_libs" >> $config_host_mak
 fi
 
 if test "$usb_redir" = "yes" ; then
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 795ff25a5e..9ce9eadb47 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -42,6 +42,11 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
 # usb pass-through
 common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
 
+host-libusb.o-cflags := $(LIBUSB_CFLAGS)
+host-libusb.o-libs := $(LIBUSB_LIBS)
+
 ifeq ($(CONFIG_USB_LIBUSB),y)
 common-obj-$(CONFIG_XEN) += xen-usb.o
+xen-usb.o-cflags := $(LIBUSB_CFLAGS)
+xen-usb.o-libs := $(LIBUSB_LIBS)
 endif
-- 
2.13.5

[Qemu-devel] [PULL v2 29/37] buildsys: Move sdl cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-3-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 audio/Makefile.objs | 1 +
 configure   | 2 +-
 ui/Makefile.objs| 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/audio/Makefile.objs b/audio/Makefile.objs
index 481d1aa30e..c20695668f 100644
--- a/audio/Makefile.objs
+++ b/audio/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-$(CONFIG_AUDIO_WIN_INT) += audio_win_int.o
 common-obj-y += wavcapture.o
 
 sdlaudio.o-cflags := $(SDL_CFLAGS)
+sdlaudio.o-libs := $(SDL_LIBS)
diff --git a/configure b/configure
index 8dca32900a..a9577bb952 100755
--- a/configure
+++ b/configure
@@ -2793,7 +2793,6 @@ EOF
 sdl_cflags="$sdl_cflags $x11_cflags"
 sdl_libs="$sdl_libs $x11_libs"
   fi
-  libs_softmmu="$sdl_libs $libs_softmmu"
 fi
 
 ##
@@ -5587,6 +5586,7 @@ if test "$sdl" = "yes" ; then
   echo "CONFIG_SDL=y" >> $config_host_mak
   echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
   echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
+  echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
 fi
 if test "$cocoa" = "yes" ; then
   echo "CONFIG_COCOA=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 146a8ce062..b3e29e21f0 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -27,6 +27,7 @@ sdl.mo-objs += sdl2-gl.o
 endif
 endif
 sdl.mo-cflags := $(SDL_CFLAGS)
+sdl.mo-libs := $(SDL_LIBS)
 
 ifeq ($(CONFIG_OPENGL),y)
 common-obj-y += shader.o
-- 
2.13.5

[Qemu-devel] [PULL v2 28/37] buildsys: Move gtk/vte cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-2-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 3 +--
 ui/Makefile.objs | 4 
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 25c6ad6b20..8dca32900a 100755
--- a/configure
+++ b/configure
@@ -2427,7 +2427,6 @@ if test "$gtk" != "no"; then
 gtk_cflags="$gtk_cflags $x11_cflags"
 gtk_libs="$gtk_libs $x11_libs"
 fi
-libs_softmmu="$gtk_libs $libs_softmmu"
 gtk="yes"
 elif test "$gtk" = "yes"; then
 feature_not_found "gtk" "Install gtk3-devel"
@@ -2677,7 +2676,6 @@ if test "$vte" != "no"; then
 vte_cflags=$($pkg_config --cflags $vtepackage)
 vte_libs=$($pkg_config --libs $vtepackage)
 vteversion=$($pkg_config --modversion $vtepackage)
-libs_softmmu="$vte_libs $libs_softmmu"
 vte="yes"
 elif test "$vte" = "yes"; then
 if test "$gtkabi" = "3.0"; then
@@ -5738,6 +5736,7 @@ fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
+  echo "VTE_LIBS=$vte_libs" >> $config_host_mak
 fi
 if test "$virglrenderer" = "yes" ; then
   echo "CONFIG_VIRGL=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 3369451285..146a8ce062 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -45,6 +45,10 @@ gtk.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
 gtk-egl.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
 gtk-gl-area.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
 
+gtk.o-libs := $(GTK_LIBS) $(VTE_LIBS)
+gtk-egl.o-libs := $(GTK_LIBS) $(VTE_LIBS)
+gtk-gl-area.o-libs := $(GTK_LIBS) $(VTE_LIBS)
+
 gtk-egl.o-libs += $(OPENGL_LIBS)
 shader.o-libs += $(OPENGL_LIBS)
 console-gl.o-libs += $(OPENGL_LIBS)
-- 
2.13.5

[Qemu-devel] [PULL v2 26/37] vl: Don't include vde header

[Fam Zheng]

Nothing in vl.c uses anything from the vde package, do remove the
unnecessary include.

Signed-off-by: Fam Zheng 
Message-Id: <20170907083552.17725-2-f...@redhat.com>
Reviewed-by: Jason Wang 
Signed-off-by: Fam Zheng 
---
 vl.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/vl.c b/vl.c
index fb1f05b937..7187e6c350 100644
--- a/vl.c
+++ b/vl.c
@@ -31,10 +31,6 @@
 #include "sysemu/seccomp.h"
 #endif
 
-#if defined(CONFIG_VDE)
-#include 
-#endif
-
 #ifdef CONFIG_SDL
 #if defined(__APPLE__) || defined(main)
 #include 
-- 
2.13.5

[Qemu-devel] [PULL v2 21/37] docker: Add test_fail and prep_fail

[Fam Zheng]

They both print a message and exit, but with different status code so
distinguish real test errors from env preparation failures.

Signed-off-by: Fam Zheng 
Message-Id: <20170905025614.579-3-f...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Based-on: 20170905021201.25684-1-f...@redhat.com
---
 tests/docker/common.rc | 12 
 1 file changed, 12 insertions(+)

diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 3b45eb91c6..87f5263757 100755
--- a/tests/docker/common.rc
+++ b/tests/docker/common.rc
@@ -32,3 +32,15 @@ build_qemu()
 echo $config_opts
 $QEMU_SRC/configure $config_opts && make $MAKEFLAGS
 }
+
+test_fail()
+{
+echo "$@"
+exit 1
+}
+
+prep_fail()
+{
+echo "$@"
+exit 2
+}
-- 
2.13.5

[Qemu-devel] [PULL v2 22/37] docker: Use unconfined security profile

[Fam Zheng]

Some by default blocked syscalls are required to run tests for example
userfaultfd.

Reviewed-by: Kashyap Chamarthy 
Signed-off-by: Fam Zheng 
Message-Id: <20170905025614.579-4-f...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Based-on: 20170905021201.25684-1-f...@redhat.com
---
 tests/docker/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 4bb02b1bb5..0e4f159619 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -135,6 +135,7 @@ docker-run: docker-qemu-src
$(call quiet-command,   \
$(SRC_PATH)/tests/docker/docker.py run  \
$(if $(NOUSER),,-u $(shell id -u)) -t   \
+   --security-opt seccomp=unconfined   \
$(if $V,,--rm)  \
$(if $(DEBUG),-i,)  \
$(if $(NETWORK),$(if $(subst 
$(NETWORK),,1),--net=$(NETWORK)),--net=none) \
-- 
2.13.5

[Qemu-devel] [PULL v2 30/37] buildsys: Move vnc cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170907082918.7299-4-f...@redhat.com>
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 configure| 14 --
 ui/Makefile.objs | 21 -
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/configure b/configure
index a9577bb952..ffc93c7cdb 100755
--- a/configure
+++ b/configure
@@ -2833,8 +2833,8 @@ EOF
   vnc_sasl_libs="-lsasl2"
   if compile_prog "$vnc_sasl_cflags" "$vnc_sasl_libs" ; then
 vnc_sasl=yes
-libs_softmmu="$vnc_sasl_libs $libs_softmmu"
-QEMU_CFLAGS="$QEMU_CFLAGS $vnc_sasl_cflags"
+vnc_libs="$vnc_libs $vnc_sasl_libs"
+vnc_cflags="$vnc_cflags $vnc_sasl_cflags"
   else
 if test "$vnc_sasl" = "yes" ; then
   feature_not_found "vnc-sasl" "Install Cyrus SASL devel"
@@ -2855,8 +2855,8 @@ EOF
 vnc_jpeg_libs="-ljpeg"
   if compile_prog "$vnc_jpeg_cflags" "$vnc_jpeg_libs" ; then
 vnc_jpeg=yes
-libs_softmmu="$vnc_jpeg_libs $libs_softmmu"
-QEMU_CFLAGS="$QEMU_CFLAGS $vnc_jpeg_cflags"
+vnc_libs="$vnc_libs $vnc_jpeg_libs"
+vnc_cflags="$vnc_cflags $vnc_jpeg_cflags"
   else
 if test "$vnc_jpeg" = "yes" ; then
   feature_not_found "vnc-jpeg" "Install libjpeg-turbo devel"
@@ -2887,8 +2887,8 @@ EOF
   fi
   if compile_prog "$vnc_png_cflags" "$vnc_png_libs" ; then
 vnc_png=yes
-libs_softmmu="$vnc_png_libs $libs_softmmu"
-QEMU_CFLAGS="$QEMU_CFLAGS $vnc_png_cflags"
+vnc_libs="$vnc_libs $vnc_png_libs"
+vnc_cflags="$vnc_cflags $vnc_png_cflags"
   else
 if test "$vnc_png" = "yes" ; then
   feature_not_found "vnc-png" "Install libpng devel"
@@ -5552,6 +5552,8 @@ echo "CONFIG_BDRV_RW_WHITELIST=$block_drv_rw_whitelist" 
>> $config_host_mak
 echo "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" >> $config_host_mak
 if test "$vnc" = "yes" ; then
   echo "CONFIG_VNC=y" >> $config_host_mak
+  echo "VNC_CFLAGS=$vnc_cflags" >> $config_host_mak
+  echo "VNC_LIBS=$vnc_libs" >> $config_host_mak
 fi
 if test "$vnc_sasl" = "yes" ; then
   echo "CONFIG_VNC_SASL=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index b3e29e21f0..a0b3c15a28 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -1,11 +1,14 @@
-vnc-obj-y += vnc.o
-vnc-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o
-vnc-obj-y += vnc-enc-tight.o vnc-palette.o
-vnc-obj-y += vnc-enc-zrle.o
-vnc-obj-y += vnc-auth-vencrypt.o
-vnc-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
-vnc-obj-y += vnc-ws.o
-vnc-obj-y += vnc-jobs.o
+vnc.mo-objs += vnc.o
+vnc.mo-objs += vnc-enc-zlib.o vnc-enc-hextile.o
+vnc.mo-objs += vnc-enc-tight.o vnc-palette.o
+vnc.mo-objs += vnc-enc-zrle.o
+vnc.mo-objs += vnc-auth-vencrypt.o
+vnc.mo-objs += $(if $(CONFIG_VNC_SASL), vnc-auth-sasl.o)
+vnc.mo-objs += vnc-ws.o
+vnc.mo-objs += vnc-jobs.o
+
+vnc.mo-cflags := $(VNC_CFLAGS)
+vnc.mo-libs := $(VNC_LIBS)
 
 common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
 common-obj-y += input.o input-keymap.o input-legacy.o
@@ -14,7 +17,7 @@ common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o 
spice-display.o
 common-obj-$(CONFIG_SDL) += sdl.mo x_keymap.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
-common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
+common-obj-$(CONFIG_VNC) += vnc.mo
 common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
 
 ifeq ($(CONFIG_SDLABI),1.2)
-- 
2.13.5

[Qemu-devel] [PULL v2 24/37] docker: Add test-block

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170905025614.579-6-f...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Based-on: 20170905021201.25684-1-f...@redhat.com
---
 tests/docker/test-block | 21 +
 1 file changed, 21 insertions(+)
 create mode 100755 tests/docker/test-block

diff --git a/tests/docker/test-block b/tests/docker/test-block
new file mode 100755
index 00..2ca1ce54f6
--- /dev/null
+++ b/tests/docker/test-block
@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# Run block test cases
+#
+# Copyright 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+
+. ./common.rc
+
+cd "$BUILD_DIR"
+
+build_qemu --target-list=x86_64-softmmu
+cd tests/qemu-iotests
+for t in raw qcow2 nbd luks; do
+./check -g quick -$t || test_fail "Test failed: iotests $t"
+done
-- 
2.13.5

[Qemu-devel] [PULL v2 25/37] docker: Drop 'set -e' from run script

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170905025614.579-7-f...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Based-on: 20170905021201.25684-1-f...@redhat.com
---
 tests/docker/run | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/docker/run b/tests/docker/run
index 52b76e443d..c8f940de15 100755
--- a/tests/docker/run
+++ b/tests/docker/run
@@ -1,4 +1,4 @@
-#!/bin/bash -e
+#!/bin/bash
 #
 # Docker test runner
 #
@@ -11,8 +11,6 @@
 # or (at your option) any later version. See the COPYING file in
 # the top-level directory.
 
-set -e
-
 if test -n "$V"; then
 set -x
 fi
@@ -20,7 +18,7 @@ fi
 BASE="$(dirname $(readlink -e $0))"
 
 # Prepare the environment
-. /etc/profile || true
+. /etc/profile
 export PATH=/usr/lib/ccache:$PATH
 
 if test -n "$J"; then
@@ -32,7 +30,7 @@ export TEST_DIR=/tmp/qemu-test
 mkdir -p $TEST_DIR/{src,build,install}
 
 # Extract the source tarballs
-tar -C $TEST_DIR/src -xf $BASE/qemu.tar
+tar -C $TEST_DIR/src -xf $BASE/qemu.tar || prep_fail "Failed to untar source"
 
 if test -n "$SHOW_ENV"; then
 if test -f /packages.txt; then
-- 
2.13.5

[Qemu-devel] [PULL v2 20/37] docker: Fix return code of build_qemu()

[Fam Zheng]

Without "set -e", the "&&" makes sure that the return code reflects the
result status, and that make only runs if configure succeeds.

Signed-off-by: Fam Zheng 
Message-Id: <20170905025614.579-2-f...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Based-on: 20170905021201.25684-1-f...@redhat.com
---
 tests/docker/common.rc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 64b36ba2d0..3b45eb91c6 100755
--- a/tests/docker/common.rc
+++ b/tests/docker/common.rc
@@ -30,6 +30,5 @@ build_qemu()
  $@"
 echo "Configure options:"
 echo $config_opts
-$QEMU_SRC/configure $config_opts
-make $MAKEFLAGS
+$QEMU_SRC/configure $config_opts && make $MAKEFLAGS
 }
-- 
2.13.5

[Qemu-devel] [PULL v2 19/37] docker: Use archive-source.py

[Fam Zheng]

Signed-off-by: Fam Zheng 
Message-Id: <20170905021201.25684-13-f...@redhat.com>
---
 tests/docker/Makefile.include | 15 ++-
 tests/docker/run  |  8 +---
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index d7dafdbd27..4bb02b1bb5 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -17,24 +17,13 @@ DOCKER_TOOLS := travis
 TESTS ?= %
 IMAGES ?= %
 
-# Make archive from git repo $1 to tar.gz $2
-make-archive-maybe = $(if $(wildcard $1/*), \
-   $(call quiet-command, \
-   (cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \
-   git archive -1 HEAD --format=tar.gz; \
-   else \
-   git archive -1 $$(git stash create) --format=tar.gz; \
-   fi) > $2, \
-   "ARCHIVE","$(notdir $2)"))
-
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.)
 DOCKER_SRC_COPY := docker-src.$(CUR_TIME)
 
 $(DOCKER_SRC_COPY):
@mkdir $@
-   $(call make-archive-maybe, $(SRC_PATH), $@/qemu.tgz)
-   $(call make-archive-maybe, $(SRC_PATH)/dtc, $@/dtc.tgz)
-   $(call make-archive-maybe, $(SRC_PATH)/pixman, $@/pixman.tgz)
+   $(call quiet-command, $(SRC_PATH)/scripts/archive-source.sh 
$@/qemu.tar, \
+   "GEN", "$@/qemu.tar")
$(call quiet-command, cp $(SRC_PATH)/tests/docker/run $@/run, \
"COPY","RUNNER")
 
diff --git a/tests/docker/run b/tests/docker/run
index ec2541cbd9..52b76e443d 100755
--- a/tests/docker/run
+++ b/tests/docker/run
@@ -32,13 +32,7 @@ export TEST_DIR=/tmp/qemu-test
 mkdir -p $TEST_DIR/{src,build,install}
 
 # Extract the source tarballs
-tar -C $TEST_DIR/src -xzf $BASE/qemu.tgz
-for p in dtc pixman; do
-if test -f $BASE/$p.tgz; then
-tar -C $TEST_DIR/src/$p -xzf $BASE/$p.tgz
-export FEATURES="$FEATURES $p"
-fi
-done
+tar -C $TEST_DIR/src -xf $BASE/qemu.tar
 
 if test -n "$SHOW_ENV"; then
 if test -f /packages.txt; then
-- 
2.13.5