[Qemu-devel] [Bug 1740219] Re: static linux-user ARM emulation has several-second startup time

[ChristianEhrhardt]

The sha above is the merge, thanks Luke.

The actual change by you is
commit 2a53535af471f4bee9d6cb5b363746b8d5ed21dd
Author: Luke Shumaker 
Date:   Thu Dec 28 13:08:13 2017 -0500

linux-user: init_guest_space: Try to make ARM space+commpage
continuous

I'll be away a week but then look at taking this fix in.

@Luke - to check in advance, are there depending changes post 2.11.1
that are needed for this that you know of?

** Changed in: qemu (Ubuntu)
   Status: Incomplete => Triaged

** Changed in: qemu (Ubuntu)
   Importance: Undecided => High

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

Title:
  static linux-user ARM emulation has several-second startup time

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Triaged

Bug description:
  static linux-user emulation has several-second startup time

  My problem: I'm a Parabola packager, and I'm updating our
  qemu-user-static package from 2.8 to 2.11.  With my new
  statically-linked 2.11, running `qemu-arm /my/arm-chroot/bin/true`
  went from taking 0.006s to 3s!  This does not happen with the normal
  dynamically linked 2.11, or the old static 2.8.

  What happens is it gets stuck in
  `linux-user/elfload.c:init_guest_space()`.  What `init_guest_space`
  does is map 2 parts of the address space: `[base, base+guest_size]`
  and `[base+0x, base+0x+page_size]`; where it must find
  an acceptable `base`.  Its strategy is to `mmap(NULL, guest_size,
  ...)` decide where the first range is, and then check if that
  +0x is also available.  If it isn't, then it starts trying
  `mmap(base, ...)` for the entire address space from low-address to
  high-address.

  "Normally," it finds an accaptable `base` within the first 2 tries.
  With a static 2.11, it's taking thousands of tries.

  

  Now, from my understanding, there are 2 factors working together to
  cause that in static 2.11 but not the other builds:

   - 2.11 increased the default `guest_size` from 0xf700 to 0x
   - PIE (and thus ASLR) is disabled for static builds

  For some reason that I don't understand, with the smaller
  `guest_size` the initial `mmap(NULL, guest_size, ...)` usually
  returns an acceptable address range; but larger `guest_size` makes it
  consistently return a block of memory that butts right up against
  another already mapped chunk of memory.  This isn't just true on the
  older builds, it's true with the 2.11 builds if I use the `-R` flag to
  shrink the `guest_size` back down to 0xf700.  That is with
  linux-hardened 4.13.13 on x86-64.

  So then, it it falls back to crawling the entire address space; so it
  tries base=0x1000.  With ASLR, that probably succeeds.  But with
  ASLR being disabled on static builds, the text segment is at
  0x6000; which is does not leave room for the needed
  0x1000-size block before it.  So then it tries base=0x2000.
  And so on, more than 6000 times until it finally gets to and passes
  the text segment; calling mmap more than 12000 times.

  

  I'm not sure what the fix is.  Perhaps try to mmap a continuous chunk
  of size 0x1000, then munmap it and then mmap the 2 chunks that we
  actually need.  The disadvantage to that is that it does not support
  the sparse address space that the current algorithm supports for
  `guest_size < 0x`.  If `guest_size < 0x` *and* the big
  mmap fails, then it could fall back to a sparse search; though I'm not
  sure the current algorithm is a good choice for it, as we see in this
  bug.  Perhaps it should inspect /proc/self/maps to try to find a
  suitable range before ever calling mmap?

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

Re: [Qemu-devel] [PATCH 1/1] s390x/cpumodel: fix feature groups and breakage of MSA8

[David Hildenbrand]

On 20.03.2018 14:07, Christian Borntraeger wrote:
> Since commit 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions
> for Multiple-epoch facility") -cpu help no longer shows the MSA8
> feature group. Turns out that we forgot to add the new MEPOCH_PTFF
> group enum.
> 
> Fixes: 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions for 
> Multiple-epoch facility")
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/cpu_features.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index e306aa7ab2..968b12fdfe 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -78,6 +78,7 @@ typedef enum {
>  S390_FEAT_GROUP_MSA_EXT_6,
>  S390_FEAT_GROUP_MSA_EXT_7,
>  S390_FEAT_GROUP_MSA_EXT_8,
> +S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
>  S390_FEAT_GROUP_MAX,
>  } S390FeatGroup;
>  
> 

Indeed, thanks!

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/1] s390x/cpumodel: fix feature groups and breakage of MSA8

[David Hildenbrand]

On 20.03.2018 14:17, Christian Borntraeger wrote:
> David, Jason, Michael,
> 
> the cpumodel code is somewhat fragile as we have to add maintain things
> in multiple places. I would like to have more robust code, e.g. by either
> generating more or by having build bug_ons or something like that. 
> Any idea is highly welcome.
> 
> Christian

Yes, error prone. I was also wondering if we should explicitly address
array items when initializing - e.g. in target/s390x/cpu_features.c the
"indexed by feature number for easy lookup" part. More LOC but less
error prone.

As Mimu also suggested, if we can generate something automatically, we
should do that.


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH V2 1/1] mach-virt: Set VM's SMBIOS system version to mc->name

[Andrew Jones]

On Thu, Mar 22, 2018 at 04:23:18PM -0500, Wei Huang wrote:
> Instead of using "1.0" as the system version of SMBIOS, we should use
> mc->name for mach-virt machine type to be consistent other architectures.
> With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will
> show:
> 
> Handle 0x0100, DMI type 1, 27 bytes
> System Information
> Manufacturer: QEMU
> Product Name: KVM Virtual Machine
> Version: virt-2.12
> Serial Number: Not Specified
> ...
> 
> instead of:
> 
> Handle 0x0100, DMI type 1, 27 bytes
> System Information
> Manufacturer: QEMU
> Product Name: KVM Virtual Machine
> Version: 1.0
> Serial Number: Not Specified
> ...
> 
> For backward compatibility, we allow older machine types to keep "1.0"
> as the default system version.
> 
> Signed-off-by: Wei Huang 
> ---
>  hw/arm/virt.c | 8 +++-
>  include/hw/arm/virt.h | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2c07245047..94dcb125d3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1132,6 +1132,8 @@ static void *machvirt_dtb(const struct arm_boot_info 
> *binfo, int *fdt_size)
>  
>  static void virt_build_smbios(VirtMachineState *vms)
>  {
> +MachineClass *mc = MACHINE_GET_CLASS(vms);
> +VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>  uint8_t *smbios_tables, *smbios_anchor;
>  size_t smbios_tables_len, smbios_anchor_len;
>  const char *product = "QEMU Virtual Machine";
> @@ -1145,7 +1147,8 @@ static void virt_build_smbios(VirtMachineState *vms)
>  }
>  
>  smbios_set_defaults("QEMU", product,
> -"1.0", false, true, SMBIOS_ENTRY_POINT_30);
> +vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
> +true, SMBIOS_ENTRY_POINT_30);
>  
>  smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
>&smbios_anchor, &smbios_anchor_len);
> @@ -1646,8 +1649,11 @@ static void virt_2_11_instance_init(Object *obj)
>  
>  static void virt_machine_2_11_options(MachineClass *mc)
>  {
> +VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>  virt_machine_2_12_options(mc);
>  SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_11);
> +vmc->smbios_old_sys_ver = true;
>  }
>  DEFINE_VIRT_MACHINE(2, 11)
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 33b0ff3892..ba0c1a4faa 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -85,6 +85,7 @@ typedef struct {
>  bool no_its;
>  bool no_pmu;
>  bool claim_edge_triggered_timers;
> +bool smbios_old_sys_ver;
>  } VirtMachineClass;
>  
>  typedef struct {
> -- 
> 2.14.3
> 
>

I don't think we need the compat code in this case. I used to pedantically
recommend compat code for all guest visible changes, but after speaking
with Igor about the policy for introducing compat code for ACPI table
updates, I learned that we can relax a bit on a case by case basis. In
this case I would recommend relaxing, but I won't insist, as the compat
code is indeed the safest way to go.

Reviewed-by: Andrew Jones 

Re: [Qemu-devel] [PATCH 1/1] s390x/cpumodel: fix feature groups and breakage of MSA8

[Christian Borntraeger]

On 03/20/2018 02:07 PM, Christian Borntraeger wrote:
> Since commit 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions
> for Multiple-epoch facility") -cpu help no longer shows the MSA8
> feature group. Turns out that we forgot to add the new MEPOCH_PTFF
> group enum.
> 
> Fixes: 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions for 
> Multiple-epoch facility")
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/cpu_features.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index e306aa7ab2..968b12fdfe 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -78,6 +78,7 @@ typedef enum {
>  S390_FEAT_GROUP_MSA_EXT_6,
>  S390_FEAT_GROUP_MSA_EXT_7,
>  S390_FEAT_GROUP_MSA_EXT_8,
> +S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
>  S390_FEAT_GROUP_MAX,
>  } S390FeatGroup;
> 

applied to my s390-next branch.

Re: [Qemu-devel] [PATCH] s390x/pci: forbid multifunction pci device

[Christian Borntraeger]

applied to my s390-next branch.


On 03/14/2018 06:14 AM, Yi Min Zhao wrote:
> Currently we don't support pci multifunction. If a pci with
> multifucntion is plugged, the guest will spin forever. This patch fixes
> this.
> 
> Signed-off-by: Yi Min Zhao 
> Reviewed-by: Pierre Morel 
> ---
>  hw/s390x/s390-pci-bus.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 77a50cab36..10da87458e 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -816,6 +816,11 @@ static void s390_pcihost_hot_plug(HotplugHandler 
> *hotplug_dev,
>  PCIBridge *pb = PCI_BRIDGE(dev);
>  PCIDevice *pdev = PCI_DEVICE(dev);
> 
> +if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +error_setg(errp, "multifunction not supported in s390");
> +return;
> +}
> +
>  pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
>  pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
> 
> @@ -835,6 +840,11 @@ static void s390_pcihost_hot_plug(HotplugHandler 
> *hotplug_dev,
>  } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>  pdev = PCI_DEVICE(dev);
> 
> +if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +error_setg(errp, "multifunction not supported in s390");
> +return;
> +}
> +
>  if (!dev->id) {
>  /* In the case the PCI device does not define an id */
>  /* we generate one based on the PCI address */
> 

Re: [Qemu-devel] [PATCH v3 3/4] target/ppc: add hash MMU support on POWER9 for PowerNV only

[David Gibson]

I'm not quite sure what the "for PowerNV only" in the subject is
supposed to be indicating.

On Thu, Mar 15, 2018 at 01:34:01PM +, Cédric Le Goater wrote:
> The HPTE bits definitions are slightly modified in ISA v3.0. Let's add
> some helpers to hide the differences in the hash MMU code.
> 
> On a POWER9 processor, the Partition Table is composed of a pair of
> doublewords per partition. The first doubleword indicates whether the
> partition uses HPT or Radix Trees translation and contains the address
> of the host's translation table structure and size.
> 
> The first doubleword of the PTCR holds the Hash Page Table base
> address for the host when the hash MMU is in use. Also add an helper
> to retrieve the HPT base address depending on the MMU revision.
> 
> Signed-off-by: Cédric Le Goater 
> ---
> 
>  Changes since v2:
> 
>  - reworked ppc_hash64_hpt_reg() to cover only powernv machines. pseries
>machines being handled with cpu->vhyp at the ppc_hash64_hpt_base()
>level.
> 
>  Changes since v1:
> 
>  - introduced ppc64_v3_get_patbe0()
>  
>  hw/ppc/spapr_hcall.c   |  5 +++--
>  target/ppc/mmu-book3s-v3.h |  5 +
>  target/ppc/mmu-hash64.c| 52 
> ++
>  target/ppc/mmu-hash64.h| 34 --
>  4 files changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 3215c3b4aec3..b437f8825819 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -94,7 +94,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  return H_PARAMETER;
>  }
>  
> -raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
> +raddr = (ptel & ppc_hash64_hpte_r_rpn(cpu)) & ~((1ULL << apshift) - 1);

h_enter() will never be used on PowerNV, for example, so why is it
being changed?

>  if (is_ram_address(spapr, raddr)) {
>  /* Regular RAM - should have WIMG=0010 */
> @@ -586,7 +586,8 @@ static int rehash_hpte(PowerPCCPU *cpu,
>  
>  base_pg_shift = ppc_hash64_hpte_page_shift_noslb(cpu, pte0, pte1);
>  assert(base_pg_shift); /* H_ENTER shouldn't allow a bad encoding */
> -avpn = HPTE64_V_AVPN_VAL(pte0) & ~(((1ULL << base_pg_shift) - 1) >> 23);
> +avpn = ppc_hash64_hpte_v_avpn_val(cpu, pte0) &
> +~(((1ULL << base_pg_shift) - 1) >> 23);
>  
>  if (pte0 & HPTE64_V_SECONDARY) {
>  pteg = ~pteg;
> diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h
> index fdf80987d7b2..a7ab580c3140 100644
> --- a/target/ppc/mmu-book3s-v3.h
> +++ b/target/ppc/mmu-book3s-v3.h
> @@ -54,6 +54,11 @@ static inline bool ppc64_radix_guest(PowerPCCPU *cpu)
>  int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>int mmu_idx);
>  
> +static inline hwaddr ppc64_v3_get_patbe0(PowerPCCPU *cpu)
> +{
> +return ldq_phys(CPU(cpu)->as, cpu->env.spr[SPR_PTCR] & PTCR_PATB);
> +}
> +
>  #endif /* TARGET_PPC64 */
>  
>  #endif /* CONFIG_USER_ONLY */
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index c9b72b742956..c425edd93ebe 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -289,6 +289,26 @@ target_ulong helper_load_slb_vsid(CPUPPCState *env, 
> target_ulong rb)
>  return rt;
>  }
>  
> +hwaddr ppc_hash64_hpt_reg(PowerPCCPU *cpu)
> +{
> +CPUPPCState *env = &cpu->env;
> +
> +/* We should not reach this routine on sPAPR machines */
> +assert(!cpu->vhyp);
> +
> +/* PowerNV machine */
> +if (msr_hv) {
> +if (env->mmu_model & POWERPC_MMU_V3) {
> +return ppc64_v3_get_patbe0(cpu);
> +} else {
> +return cpu->env.spr[SPR_SDR1];
> +}
> +} else {
> +error_report("PowerNV guest support Unimplemented");
> +exit(1);
> +}
> +}
> +
>  /* Check No-Execute or Guarded Storage */
>  static inline int ppc_hash64_pte_noexec_guard(PowerPCCPU *cpu,
>ppc_hash_pte64_t pte)
> @@ -451,8 +471,9 @@ void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const 
> ppc_hash_pte64_t *hptes,
>  false, n * HASH_PTE_SIZE_64);
>  }
>  
> -static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
> -uint64_t pte0, uint64_t pte1)
> +static unsigned hpte_page_shift(PowerPCCPU *cpu,
> +const struct ppc_one_seg_page_size *sps,
> +uint64_t pte0, uint64_t pte1)
>  {
>  int i;
>  
> @@ -478,7 +499,7 @@ static unsigned hpte_page_shift(const struct 
> ppc_one_seg_page_size *sps,
>  continue;
>  }
>  
> -mask = ((1ULL << ps->page_shift) - 1) & HPTE64_R_RPN;
> +mask = ((1ULL << ps->page_shift) - 1) & ppc_hash64_hpte_r_rpn(cpu);
>  
>  if ((pte1 & mask) == ((uint64_t)ps->pte_enc << HPTE64_R_RPN_SHIFT)) {
>  return ps->page_shift;
> @@ -488,6 +509,18 @@ static unsigned hpte

Re: [Qemu-devel] [Bug 1754542] Re: colo: vm crash with segmentation fault

[Li Zhijian]

Just noticed that's a little old, you may need to rebase it


Thanks


On 03/23/2018 11:51 AM, Li Zhijian wrote:



On 03/21/2018 02:04 PM, Zhang Chen wrote:

Hi Suiheng,

I made a new guest image and retest it, and got the same bug from latest branch.
I found that after the COLO checkpoint begin, the secondary guest always send
reset request to Qemu like someone still push the reset button in the guest.
And this bug occurred in COLO frame related codes. This part of codes wrote
by Li zhijian and Zhang hailiang and currently maintained by Zhang hailiang.
So, I add them to this thread.

CC Zhijian and Hailiang:
Any idea or comments about this bug?


One clue is the memory of SVM not is same with PVM.
we can try to compare the memory after checkpoint, i had a draft patch to do 
this before.


Thanks






If you want to test COLO currently, you can try the old version of COLO:
https://github.com/zhangckid/qemu/tree/qemu-colo-18mar10-legacy


Thanks
Zhang Chen

On Mon, Mar 19, 2018 at 10:08 AM, 李穗恒 <1754...@bugs.launchpad.net 
> wrote:

    Hi Zhang Chen，
    I follow the https://wiki.qemu.org/Features/COLO 
, And Vm no crash.
    But SVM rebooting constantly after print RESET, PVM normal startup.

    Secondary:
    {"timestamp": {"seconds": 1521421788, "microseconds": 541058}, "event": 
"RESUME"}
    {"timestamp": {"seconds": 1521421808, "microseconds": 493484}, "event": 
"STOP"}
    {"timestamp": {"seconds": 1521421808, "microseconds": 686466}, "event": 
"RESUME"}
    {"timestamp": {"seconds": 1521421808, "microseconds": 696152}, "event": "RESET", 
"data": {"guest": true}}
    {"timestamp": {"seconds": 1521421808, "microseconds": 740653}, "event": "RESET", 
"data": {"guest": true}}
    {"timestamp": {"seconds": 1521421818, "microseconds": 74}, "event": 
"STOP"}
    {"timestamp": {"seconds": 1521421818, "microseconds": 969883}, "event": 
"RESUME"}
    {"timestamp": {"seconds": 1521421818, "microseconds": 979986}, "event": "RESET", 
"data": {"guest": true}}
    {"timestamp": {"seconds": 1521421819, "microseconds": 22652}, "event": "RESET", 
"data": {"guest": true}}


    The command(I run two VM in sample machine):

    Primary:
    sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64 -enable-kvm 
-boot c -m 2048 -smp 2 -qmp stdio  -name primary -cpu qemu64,+kvmclock -device 
piix3-usb-uhci -device usb-tablet \
        -netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device 
rtl8139,id=e0,netdev=hn0 \
        -chardev socket,id=mirror0,host=192.168.0.33,port=9003,server,nowait \
        -chardev socket,id=compare1,host=192.168.0.33,port=9004,server,wait \
        -chardev socket,id=compare0,host=192.168.0.33,port=9001,server,nowait \
        -chardev socket,id=compare0-0,host=192.168.0.33,port=9001 \
        -chardev 
socket,id=compare_out,host=192.168.0.33,port=9005,server,nowait \
        -chardev socket,id=compare_out0,host=192.168.0.33,port=9005 \
        -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
        -object 
filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
        -object 
filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
        -object iothread,id=iothread1 \
        -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 \
        -drive 
if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,children.0.file.filename=/var/lib/libvirt/images/1.raw,children.0.driver=raw
 -S

    Secondary:
    sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64 -boot c -m 
2048 -smp 2 -qmp stdio  -name secondary -enable-kvm -cpu qemu64,+kvmclock \
        -device piix3-usb-uhci -device usb-tablet \
        -netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown \
        -device rtl8139,netdev=hn0 \
        -chardev socket,id=red0,host=192.168.0.33,port=9003,reconnect=1 \
        -chardev socket,id=red1,host=192.168.0.33,port=9004,reconnect=1 \
        -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
        -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
        -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
        -drive 
if=none,id=colo-disk0,file.filename=/var/lib/libvirt/images/2.raw,driver=raw,node-name=node0
 \
        -drive 
if=ide,id=active-disk0,driver=replication,mode=secondary,file.driver=qcow2,top-id=active-disk0,file.file.filename=/mnt/ramfs/active_disk.img,file.backing.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.backing=colo-disk0
 \
        -incoming tcp:0:

    Secondary:
      {'execute':'qmp_capabilities'}
      { 'execute': 'nbd-server-start',
        'arguments': {'addr': {'type': 'inet', 'data': {'host': '192.168.0.33', 
'port': '8889'} } }
      }
      {'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk0', 
'wri

[Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Thomas Huth]

Most of the RISC-V boards currently crash when they are started with
"-nodefaults", e.g.:

$ gdb --args riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e
[...]
(gdb) r
Program received signal SIGSEGV, Segmentation fault.
qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
at chardev/char-fe.c:210
210 } else if (s->be) {
(gdb) bt
 0  0x558695f8 in qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
at chardev/char-fe.c:210
 1  0x556fb425 in sifive_uart_create ([...], chr=0x0, [...])
at hw/riscv/sifive_uart.c:169
 2  0x556f8cc4 in riscv_sifive_e_init (machine=[...])
at hw/riscv/sifive_e.c:164
[...]

With "-nodefaults" there are no entries in serial_hds[], so qemu_chr_fe_init()
finally tries to dereference a NULL pointer. Let's fix this problem by
creating a "null" chardev in this case instead.

Signed-off-by: Thomas Huth 
---
 For other boards / targets, see also:
 https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05073.html

 hw/riscv/riscv_htif.c  | 5 +
 hw/riscv/sifive_uart.c | 5 +
 2 files changed, 10 insertions(+)

diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
index 3e17f30..d3d31ff 100644
--- a/hw/riscv/riscv_htif.c
+++ b/hw/riscv/riscv_htif.c
@@ -245,9 +245,14 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, 
MemoryRegion *main_mem,
 s->pending_read = 0;
 s->allow_tohost = 0;
 s->fromhost_inprogress = 0;
+
+if (!chr) {
+chr = qemu_chardev_new(NULL, TYPE_CHARDEV_NULL, NULL, &error_abort);
+}
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
 htif_be_change, s, NULL, true);
+
 if (base) {
 memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
 TYPE_HTIF_UART, size);
diff --git a/hw/riscv/sifive_uart.c b/hw/riscv/sifive_uart.c
index b0c3798..2bde8bb 100644
--- a/hw/riscv/sifive_uart.c
+++ b/hw/riscv/sifive_uart.c
@@ -166,9 +166,14 @@ SiFiveUARTState *sifive_uart_create(MemoryRegion 
*address_space, hwaddr base,
 {
 SiFiveUARTState *s = g_malloc0(sizeof(SiFiveUARTState));
 s->irq = irq;
+
+if (!chr) {
+chr = qemu_chardev_new(NULL, TYPE_CHARDEV_NULL, NULL, &error_abort);
+}
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, uart_can_rx, uart_rx, uart_event,
 uart_be_change, s, NULL, true);
+
 memory_region_init_io(&s->mmio, NULL, &uart_ops, s,
   TYPE_SIFIVE_UART, SIFIVE_UART_MAX);
 memory_region_add_subregion(address_space, base, &s->mmio);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 3/4] target/ppc: add hash MMU support on POWER9 for PowerNV only

[Cédric Le Goater]

On 03/23/2018 09:24 AM, David Gibson wrote:
> I'm not quite sure what the "for PowerNV only" in the subject is
> supposed to be indicating.

That's the initial subject which didn't evolve with the code, which is 
now changing some sPAPR hcalls. And anyhow, hash MMU is supported on
POWER9 TCG, so there is no reason for the 'only'. KVM is another matter.

I will change the subject and split the patch in two parts, a first 
one for the changes in the HPTE bits definitions introduced by ISA v3.0 
which has some impact on the sPAPR platform. And a second patch to
introduce ppc_hash64_hpt_reg() which adds support for hash MMU to
the PowerNV platform.

Thanks,

C.

 
> On Thu, Mar 15, 2018 at 01:34:01PM +, Cédric Le Goater wrote:
>> The HPTE bits definitions are slightly modified in ISA v3.0. Let's add
>> some helpers to hide the differences in the hash MMU code.
>>
>> On a POWER9 processor, the Partition Table is composed of a pair of
>> doublewords per partition. The first doubleword indicates whether the
>> partition uses HPT or Radix Trees translation and contains the address
>> of the host's translation table structure and size.
>>
>> The first doubleword of the PTCR holds the Hash Page Table base
>> address for the host when the hash MMU is in use. Also add an helper
>> to retrieve the HPT base address depending on the MMU revision.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>
>>  Changes since v2:
>>
>>  - reworked ppc_hash64_hpt_reg() to cover only powernv machines. pseries
>>machines being handled with cpu->vhyp at the ppc_hash64_hpt_base()
>>level.
>>
>>  Changes since v1:
>>
>>  - introduced ppc64_v3_get_patbe0()
>>  
>>  hw/ppc/spapr_hcall.c   |  5 +++--
>>  target/ppc/mmu-book3s-v3.h |  5 +
>>  target/ppc/mmu-hash64.c| 52 
>> ++
>>  target/ppc/mmu-hash64.h| 34 --
>>  4 files changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 3215c3b4aec3..b437f8825819 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -94,7 +94,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, 
>> sPAPRMachineState *spapr,
>>  return H_PARAMETER;
>>  }
>>  
>> -raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
>> +raddr = (ptel & ppc_hash64_hpte_r_rpn(cpu)) & ~((1ULL << apshift) - 1);
> 
> h_enter() will never be used on PowerNV, for example, so why is it
> being changed?
> 
>>  if (is_ram_address(spapr, raddr)) {
>>  /* Regular RAM - should have WIMG=0010 */
>> @@ -586,7 +586,8 @@ static int rehash_hpte(PowerPCCPU *cpu,
>>  
>>  base_pg_shift = ppc_hash64_hpte_page_shift_noslb(cpu, pte0, pte1);
>>  assert(base_pg_shift); /* H_ENTER shouldn't allow a bad encoding */
>> -avpn = HPTE64_V_AVPN_VAL(pte0) & ~(((1ULL << base_pg_shift) - 1) >> 23);
>> +avpn = ppc_hash64_hpte_v_avpn_val(cpu, pte0) &
>> +~(((1ULL << base_pg_shift) - 1) >> 23);
>>  
>>  if (pte0 & HPTE64_V_SECONDARY) {
>>  pteg = ~pteg;
>> diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h
>> index fdf80987d7b2..a7ab580c3140 100644
>> --- a/target/ppc/mmu-book3s-v3.h
>> +++ b/target/ppc/mmu-book3s-v3.h
>> @@ -54,6 +54,11 @@ static inline bool ppc64_radix_guest(PowerPCCPU *cpu)
>>  int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>>int mmu_idx);
>>  
>> +static inline hwaddr ppc64_v3_get_patbe0(PowerPCCPU *cpu)
>> +{
>> +return ldq_phys(CPU(cpu)->as, cpu->env.spr[SPR_PTCR] & PTCR_PATB);
>> +}
>> +
>>  #endif /* TARGET_PPC64 */
>>  
>>  #endif /* CONFIG_USER_ONLY */
>> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
>> index c9b72b742956..c425edd93ebe 100644
>> --- a/target/ppc/mmu-hash64.c
>> +++ b/target/ppc/mmu-hash64.c
>> @@ -289,6 +289,26 @@ target_ulong helper_load_slb_vsid(CPUPPCState *env, 
>> target_ulong rb)
>>  return rt;
>>  }
>>  
>> +hwaddr ppc_hash64_hpt_reg(PowerPCCPU *cpu)
>> +{
>> +CPUPPCState *env = &cpu->env;
>> +
>> +/* We should not reach this routine on sPAPR machines */
>> +assert(!cpu->vhyp);
>> +
>> +/* PowerNV machine */
>> +if (msr_hv) {
>> +if (env->mmu_model & POWERPC_MMU_V3) {
>> +return ppc64_v3_get_patbe0(cpu);
>> +} else {
>> +return cpu->env.spr[SPR_SDR1];
>> +}
>> +} else {
>> +error_report("PowerNV guest support Unimplemented");
>> +exit(1);
>> +}
>> +}
>> +
>>  /* Check No-Execute or Guarded Storage */
>>  static inline int ppc_hash64_pte_noexec_guard(PowerPCCPU *cpu,
>>ppc_hash_pte64_t pte)
>> @@ -451,8 +471,9 @@ void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const 
>> ppc_hash_pte64_t *hptes,
>>  false, n * HASH_PTE_SIZE_64);
>>  }
>>  
>> -static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
>> -uint64_t

Re: [Qemu-devel] [PATCH v2 0/6] Extend vhost-user to support VFIO based accelerators

[Tiwei Bie]

On Thu, Mar 22, 2018 at 04:55:39PM +0200, Michael S. Tsirkin wrote:
> On Mon, Mar 19, 2018 at 03:15:31PM +0800, Tiwei Bie wrote:
> > This patch set does some small extensions to vhost-user protocol
> > to support VFIO based accelerators, and makes it possible to get
> > the similar performance of VFIO based PCI passthru while keeping
> > the virtio device emulation in QEMU.
> 
> I love your patches!
> Yet there are some things to improve.
> Posting comments separately as individual messages.
> 

Thank you so much! :-)

It may take me some time to address all your comments.
They're really helpful! I'll try to address and reply
to these comments in the next few days. Thanks again!
I do appreciate it!

Best regards,
Tiwei Bie

> 
> > How does accelerator accelerate vhost (data path)
> > =
> > 
> > Any virtio ring compatible devices potentially can be used as the
> > vhost data path accelerators. We can setup the accelerator based
> > on the informations (e.g. memory table, features, ring info, etc)
> > available on the vhost backend. And accelerator will be able to use
> > the virtio ring provided by the virtio driver in the VM directly.
> > So the virtio driver in the VM can exchange e.g. network packets
> > with the accelerator directly via the virtio ring. That is to say,
> > we will be able to use the accelerator to accelerate the vhost
> > data path. We call it vDPA: vhost Data Path Acceleration.
> > 
> > Notice: Although the accelerator can talk with the virtio driver
> > in the VM via the virtio ring directly. The control path events
> > (e.g. device start/stop) in the VM will still be trapped and handled
> > by QEMU, and QEMU will deliver such events to the vhost backend
> > via standard vhost protocol.
> > 
> > Below link is an example showing how to setup a such environment
> > via nested VM. In this case, the virtio device in the outer VM is
> > the accelerator. It will be used to accelerate the virtio device
> > in the inner VM. In reality, we could use virtio ring compatible
> > hardware device as the accelerators.
> > 
> > http://dpdk.org/ml/archives/dev/2017-December/085044.html
> > 
> > In above example, it doesn't require any changes to QEMU, but
> > it has lower performance compared with the traditional VFIO
> > based PCI passthru. And that's the problem this patch set wants
> > to solve.
> > 
> > The performance issue of vDPA/vhost-user and solutions
> > ==
> > 
> > For vhost-user backend, the critical issue in vDPA is that the
> > data path performance is relatively low and some host threads are
> > needed for the data path, because some necessary mechanisms are
> > missing to support:
> > 
> > 1) guest driver notifies the device directly;
> > 2) device interrupts the guest directly;
> > 
> > So this patch set does some small extensions to the vhost-user
> > protocol to make both of them possible. It leverages the same
> > mechanisms (e.g. EPT and Posted-Interrupt on Intel platform) as
> > the PCI passthru.
> > 
> > A new protocol feature bit is added to negotiate the accelerator
> > feature support. Two new slave message types are added to control
> > the notify region and queue interrupt passthru for each queue.
> > >From the view of vhost-user protocol design, it's very flexible.
> > The passthru can be enabled/disabled for each queue individually,
> > and it's possible to accelerate each queue by different devices.
> > More design and implementation details can be found from the last
> > patch.
> > 
> > Difference between vDPA and PCI passthru
> > 
> > 
> > The key difference between PCI passthru and vDPA is that, in vDPA
> > only the data path of the device (e.g. DMA ring, notify region and
> > queue interrupt) is pass-throughed to the VM, the device control
> > path (e.g. PCI configuration space and MMIO regions) is still
> > defined and emulated by QEMU.
> > 
> > The benefits of keeping virtio device emulation in QEMU compared
> > with virtio device PCI passthru include (but not limit to):
> > 
> > - consistent device interface for guest OS in the VM;
> > - max flexibility on the hardware (i.e. the accelerators) design;
> > - leveraging the existing virtio live-migration framework;
> > 
> > Why extend vhost-user for vDPA
> > ==
> > 
> > We have already implemented various virtual switches (e.g. OVS-DPDK)
> > based on vhost-user for VMs in the Cloud. They are purely software
> > running on CPU cores. When we have accelerators for such NFVi applications,
> > it's ideal if the applications could keep using the original interface
> > (i.e. vhost-user netdev) with QEMU, and infrastructure is able to decide
> > when and how to switch between CPU and accelerators within the interface.
> > And the switching (i.e. switch between CPU and accelerators) can be done
> > flexibly and quickly inside the applications.
> > 
> > More d

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Bastian Koppelmann]

On 03/23/2018 09:36 AM, Thomas Huth wrote:
> Most of the RISC-V boards currently crash when they are started with
> "-nodefaults", e.g.:
> 
> $ gdb --args riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e
> [...]
> (gdb) r
> Program received signal SIGSEGV, Segmentation fault.
> qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
> at chardev/char-fe.c:210
> 210   } else if (s->be) {
> (gdb) bt
>  0  0x558695f8 in qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
> at chardev/char-fe.c:210
>  1  0x556fb425 in sifive_uart_create ([...], chr=0x0, [...])
> at hw/riscv/sifive_uart.c:169
>  2  0x556f8cc4 in riscv_sifive_e_init (machine=[...])
> at hw/riscv/sifive_e.c:164
> [...]
> 
> With "-nodefaults" there are no entries in serial_hds[], so qemu_chr_fe_init()
> finally tries to dereference a NULL pointer. Let's fix this problem by
> creating a "null" chardev in this case instead.
> 
> Signed-off-by: Thomas Huth 
> ---
>  For other boards / targets, see also:
>  https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05073.html
> 
>  hw/riscv/riscv_htif.c  | 5 +
>  hw/riscv/sifive_uart.c | 5 +
>  2 files changed, 10 insertions(+)
> 

Reviewed-by: Bastian Koppelmann 

Cheers,
Bastian

Re: [Qemu-devel] [Bug 1754542] Re: colo: vm crash with segmentation fault

[Zhang Chen]

Thanks zhijian.

On Fri, Mar 23, 2018 at 4:34 PM, Li Zhijian 
wrote:

> Just noticed that's a little old, you may need to rebase it
>
>
> Thanks
>
>
> On 03/23/2018 11:51 AM, Li Zhijian wrote:
>
>>
>>
>> On 03/21/2018 02:04 PM, Zhang Chen wrote:
>>
>>> Hi Suiheng,
>>>
>>> I made a new guest image and retest it, and got the same bug from latest
>>> branch.
>>> I found that after the COLO checkpoint begin, the secondary guest always
>>> send
>>> reset request to Qemu like someone still push the reset button in the
>>> guest.
>>> And this bug occurred in COLO frame related codes. This part of codes
>>> wrote
>>> by Li zhijian and Zhang hailiang and currently maintained by Zhang
>>> hailiang.
>>> So, I add them to this thread.
>>>
>>> CC Zhijian and Hailiang:
>>> Any idea or comments about this bug?
>>>
>>
>> One clue is the memory of SVM not is same with PVM.
>> we can try to compare the memory after checkpoint, i had a draft patch to
>> do this before.
>>
>>
>> Thanks
>>
>>
>>
>>
>>
>>> If you want to test COLO currently, you can try the old version of COLO:
>>> https://github.com/zhangckid/qemu/tree/qemu-colo-18mar10-legacy
>>>
>>>
>>> Thanks
>>> Zhang Chen
>>>
>>> On Mon, Mar 19, 2018 at 10:08 AM, 李穗恒 <1754...@bugs.launchpad.net
>>> > wrote:
>>>
>>> Hi Zhang Chen，
>>> I follow the https://wiki.qemu.org/Features/COLO <
>>> https://wiki.qemu.org/Features/COLO>, And Vm no crash.
>>>
>>> But SVM rebooting constantly after print RESET, PVM normal startup.
>>>
>>> Secondary:
>>> {"timestamp": {"seconds": 1521421788, "microseconds": 541058},
>>> "event": "RESUME"}
>>> {"timestamp": {"seconds": 1521421808, "microseconds": 493484},
>>> "event": "STOP"}
>>> {"timestamp": {"seconds": 1521421808, "microseconds": 686466},
>>> "event": "RESUME"}
>>> {"timestamp": {"seconds": 1521421808, "microseconds": 696152},
>>> "event": "RESET", "data": {"guest": true}}
>>> {"timestamp": {"seconds": 1521421808, "microseconds": 740653},
>>> "event": "RESET", "data": {"guest": true}}
>>> {"timestamp": {"seconds": 1521421818, "microseconds": 74},
>>> "event": "STOP"}
>>> {"timestamp": {"seconds": 1521421818, "microseconds": 969883},
>>> "event": "RESUME"}
>>> {"timestamp": {"seconds": 1521421818, "microseconds": 979986},
>>> "event": "RESET", "data": {"guest": true}}
>>> {"timestamp": {"seconds": 1521421819, "microseconds": 22652},
>>> "event": "RESET", "data": {"guest": true}}
>>>
>>>
>>> The command(I run two VM in sample machine):
>>>
>>> Primary:
>>> sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64
>>> -enable-kvm -boot c -m 2048 -smp 2 -qmp stdio  -name primary -cpu
>>> qemu64,+kvmclock -device piix3-usb-uhci -device usb-tablet \
>>> -netdev tap,id=hn0,vhost=off,script=/e
>>> tc/qemu-ifup,downscript=/etc/qemu-ifdown -device
>>> rtl8139,id=e0,netdev=hn0 \
>>> -chardev socket,id=mirror0,host=192.168.0.33,port=9003,server,nowait
>>> \
>>> -chardev socket,id=compare1,host=192.168.0.33,port=9004,server,wait
>>> \
>>> -chardev 
>>> socket,id=compare0,host=192.168.0.33,port=9001,server,nowait
>>> \
>>> -chardev socket,id=compare0-0,host=192.168.0.33,port=9001 \
>>> -chardev socket,id=compare_out,host=192
>>> .168.0.33,port=9005,server,nowait \
>>> -chardev socket,id=compare_out0,host=192.168.0.33,port=9005 \
>>> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
>>> -object filter-redirector,netdev=hn0,i
>>> d=redire0,queue=rx,indev=compare_out \
>>> -object filter-redirector,netdev=hn0,i
>>> d=redire1,queue=rx,outdev=compare0 \
>>> -object iothread,id=iothread1 \
>>> -object colo-compare,id=comp0,primary_
>>> in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
>>> \
>>> -drive if=ide,id=colo-disk0,driver=qu
>>> orum,read-pattern=fifo,vote-threshold=1,children.0.file.file
>>> name=/var/lib/libvirt/images/1.raw,children.0.driver=raw -S
>>>
>>> Secondary:
>>> sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64
>>> -boot c -m 2048 -smp 2 -qmp stdio  -name secondary -enable-kvm -cpu
>>> qemu64,+kvmclock \
>>> -device piix3-usb-uhci -device usb-tablet \
>>> -netdev tap,id=hn0,vhost=off,script=/e
>>> tc/qemu-ifup,downscript=/etc/qemu-ifdown \
>>> -device rtl8139,netdev=hn0 \
>>> -chardev socket,id=red0,host=192.168.0.33,port=9003,reconnect=1
>>> \
>>> -chardev socket,id=red1,host=192.168.0.33,port=9004,reconnect=1
>>> \
>>> -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
>>> -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
>>> \
>>> -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
>>> -drive if=none,id=colo-disk0,file.fil
>>> ename=/var/lib/libvirt/images/2.raw,driver=raw,node-name=node0 \
>>> -drive if=ide,id=active-disk0,driver=
>>> replication,mode=se

[Qemu-devel] [PULL 1/2] s390x/pci: forbid multifunction pci device

[Christian Borntraeger]

From: Yi Min Zhao 

Currently we don't support pci multifunction. If a pci with
multifucntion is plugged, the guest will spin forever. This patch fixes
this.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Pierre Morel 
Reviewed-by: Thomas Huth 
Signed-off-by: Christian Borntraeger 
---
 hw/s390x/s390-pci-bus.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 77a50cab36..10da87458e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -816,6 +816,11 @@ static void s390_pcihost_hot_plug(HotplugHandler 
*hotplug_dev,
 PCIBridge *pb = PCI_BRIDGE(dev);
 PCIDevice *pdev = PCI_DEVICE(dev);
 
+if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+error_setg(errp, "multifunction not supported in s390");
+return;
+}
+
 pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
 pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
 
@@ -835,6 +840,11 @@ static void s390_pcihost_hot_plug(HotplugHandler 
*hotplug_dev,
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
 pdev = PCI_DEVICE(dev);
 
+if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+error_setg(errp, "multifunction not supported in s390");
+return;
+}
+
 if (!dev->id) {
 /* In the case the PCI device does not define an id */
 /* we generate one based on the PCI address */
-- 
2.14.3

[Qemu-devel] [PULL 2/2] s390x/cpumodel: fix feature groups and breakage of MSA8

[Christian Borntraeger]

Since commit 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions
for Multiple-epoch facility") -cpu help no longer shows the MSA8
feature group. Turns out that we forgot to add the new MEPOCH_PTFF
group enum.

Fixes: 46a99c9f73c7 ("s390x/cpumodel: model PTFF subfunctions for 
Multiple-epoch facility")
Reviewed-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
---
 target/s390x/cpu_features.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index e306aa7ab2..968b12fdfe 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -78,6 +78,7 @@ typedef enum {
 S390_FEAT_GROUP_MSA_EXT_6,
 S390_FEAT_GROUP_MSA_EXT_7,
 S390_FEAT_GROUP_MSA_EXT_8,
+S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
 S390_FEAT_GROUP_MAX,
 } S390FeatGroup;
 
-- 
2.14.3

[Qemu-devel] [PULL 0/2] s390x: Fixes for 2.12

[Christian Borntraeger]

Peter,

2 fixes for 2.12. Please pull

The following changes since commit d522e0bd18364f6d784d43edab35b0563d21f6f3:

  gitmodules: Use the QEMU mirror of qemu-palcode (2018-03-22 19:24:16 +)

are available in the Git repository at:

  git://github.com/borntraeger/qemu.git tags/s390x-20180323

for you to fetch changes up to 06a97edac172f52971d7bdca6bb56a1edcb1b8f6:

  s390x/cpumodel: fix feature groups and breakage of MSA8 (2018-03-23 09:05:42 
+)


s390x: Fixes for 2.12

- Fix for the s390 cpumodel
- Forbid multifunction PCI devices


Christian Borntraeger (1):
  s390x/cpumodel: fix feature groups and breakage of MSA8

Yi Min Zhao (1):
  s390x/pci: forbid multifunction pci device

 hw/s390x/s390-pci-bus.c | 10 ++
 target/s390x/cpu_features.h |  1 +
 2 files changed, 11 insertions(+)

Re: [Qemu-devel] [PATCH for 2.13 0/5] linux-user: move arch specific parts to arch directories

[Peter Maydell]

On 22 March 2018 at 21:58, Laurent Vivier  wrote:
> Some files like signal.c are really hard to read
> because all architectures are mixed in the same
> file.
>
> This series moves from signal.c these parts to
> the architecture dedicated directories in linux-user.
> Moerover, this allows to compare easier functions
> between architectures (it helps to debug problems).
> Adding new functions for a new architecture will
> be facilitated too.
>
> As we are doing that for signal.c, we can also
> do that for main.c, for the cpu loop part
> and the cpu loop prologue too.

Yeah, this is something I've kind of had on my backlog
of "it would be nice to do this" for years. Thanks
for putting in the work!

> checkpatch.pl is not happy... but I only want to
> move code from a file to another. I don't want
> to change the content of the parts I move.

Yeah, that's reasonable. (Since the move will reduce
the effectiveness of git blame anyway we might want
to do a fix-coding-style pass afterwards, but we can
do that as a separate patchset.)

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.12] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

[Peter Maydell]

On 22 March 2018 at 20:42, Philippe Mathieu-Daudé  wrote:
> Hi Peter,
>
> On 03/22/2018 03:29 PM, Peter Maydell wrote:
>> On 22 March 2018 at 14:23, Andrew Jones  wrote:
>>> On Thu, Mar 15, 2018 at 01:34:41PM +, Peter Maydell wrote:
 If the GIC has the security extension support enabled, then a
 non-secure access to ICC_PMR must take account of the non-secure
 view of interrupt priorities, where real priorities 0..0x7f
 are secure-only and not visible to the non-secure guest, and
 priorities 0x80..0xff are shown to the guest as if they were
 0x00..0xff. We had the logic here wrong:
>>>
>>> 0x00..0x7f
>>
>> I think 0x00..0xff is correct.
>
> I guess Andrew only suggested to correct the hex prefix in your comment:
> - ... where real priorities 0..0x7f
> + ... where real priorities 0x00..0x7f

Oh, right. Yes, can do that. I was confused by the suggested correction
being directly under "0x00..0xff" in my commit message.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure

[Roman Kagan]

On Thu, Mar 22, 2018 at 03:38:13PM -0300, Eduardo Habkost wrote:
> On Thu, Mar 22, 2018 at 04:58:03PM +0300, Roman Kagan wrote:
> > On Thu, Mar 22, 2018 at 10:22:18AM -0300, Eduardo Habkost wrote:
> > > On Thu, Mar 22, 2018 at 04:00:14PM +0300, Roman Kagan wrote:
> > > > On Wed, Mar 21, 2018 at 05:19:24PM -0300, Eduardo Habkost wrote:
> > > > > On Wed, Mar 21, 2018 at 07:57:29PM +0300, Roman Kagan wrote:
> > > > > > On Wed, Mar 21, 2018 at 02:18:54PM +0100, Vitaly Kuznetsov wrote:
> > > > > > > Roman Kagan  writes:
> > > > > > > 
> > > > > > > > On Tue, Mar 20, 2018 at 06:35:00PM +0100, Vitaly Kuznetsov 
> > > > > > > > wrote:
> > > > > > > >> Requiring tsc_is_stable_and_known() is too restrictive: even 
> > > > > > > >> without INVTCS
> > > > > > > >> nested Hyper-V-on-KVM enables TSC pages for its guests e.g. 
> > > > > > > >> when
> > > > > > > >> Reenlightenment MSRs are present. Presence of frequency MSRs 
> > > > > > > >> doesn't mean
> > > > > > > >> these frequencies are stable, it just means they're available 
> > > > > > > >> for reading.
> > > > > > > >> 
> > > > > > > >> Signed-off-by: Vitaly Kuznetsov 
> > > > > > > >> ---
> > > > > > > >>  target/i386/kvm.c | 2 +-
> > > > > > > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >> 
> > > > > > > >> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > > > > > > >> index 7d9f9ca0b1..74fc3d3b2c 100644
> > > > > > > >> --- a/target/i386/kvm.c
> > > > > > > >> +++ b/target/i386/kvm.c
> > > > > > > >> @@ -651,7 +651,7 @@ static int 
> > > > > > > >> hyperv_handle_properties(CPUState *cs)
> > > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > > >> HV_TIME_REF_COUNT_AVAILABLE;
> > > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > > >> HV_REFERENCE_TSC_AVAILABLE;
> > > > > > > >>  
> > > > > > > >> -if (has_msr_hv_frequencies && 
> > > > > > > >> tsc_is_stable_and_known(env)) {
> > > > > > > >> +if (has_msr_hv_frequencies && env->tsc_khz) {
> > > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > > >> HV_ACCESS_FREQUENCY_MSRS;
> > > > > > > >>  env->features[FEAT_HYPERV_EDX] |= 
> > > > > > > >> HV_FREQUENCY_MSRS_AVAILABLE;
> > > > > > > >>  }
> > > > > > > >
> > > > > > > > I suggest that we add a corresponding cpu property here, too.  
> > > > > > > > The guest
> > > > > > > > may legitimately rely on these msrs when it sees the support in 
> > > > > > > > CPUID,
> > > > > > > > and migrating from a kernel with the feature supported (4.14+) 
> > > > > > > > to an
> > > > > > > > older one will make it crash.
> > > > > > > >
> > > > > > > 
> > > > > > > This can be arranged, but what happens to people who use these 
> > > > > > > features
> > > > > > > today? Assuming they also passed 'invtsc' they have stable TSC 
> > > > > > > page
> > > > > > > clocksource already (when Hyper-V role is enabled) but when we 
> > > > > > > start
> > > > > > > requesting a new 'hv_frequency' cpu property they'll suddenly 
> > > > > > > lose what
> > > > > > > they have...
> > > > > > 
> > > > > > I see two cases here:
> > > > > > 
> > > > > > 1) people start a new VM, and discover that their old configuration 
> > > > > > is
> > > > > >not enough for this feature to work.
> > > > > > 
> > > > > >They need to reconfigure and restart the VM.  This costs them 
> > > > > > some
> > > > > >time investigating and restarting, but not data.
> > > > > 
> > > > > If we keep machine-type compatibility, people will need to do
> > > > > that only if they change the machine-type (or use the "pc" or
> > > > > "q35" aliases).  If they copy the old configuration, it will keep
> > > > > working.
> > > > 
> > > > The problem is that the feature is not fixed by the machine-type, due to
> > > > the forgotten property: it only depends on the KVM version.  So, once
> > > > (if) we add the property and make the feature deterministic, we'll lose
> > > > compatibility one way or another.
> > > > 
> > > > Or are you suggesting that for pre-2.12 machine types we leave the
> > > > property at "decided by your KVM" state?
> > > 
> > > Yes, that's what I mean.  This looks like the only way to avoid
> > > losing features by just cold-rebooting an existing VM.
> > > 
> > > The scenario I'm thinking is this:
> > > 
> > > 1) pc-2.11 VM started on host running QEMU 2.11
> > > 2) VM migrated to a host containing this patch
> > > 3) 1 year later, the VM is shut down and booted again.
> > > 4) Things stop working inside the VM because hv-frequency is
> > >unexpectedly gone.
> > > 
> > > Machine-type compatibility code would avoid (4).
> > 
> > Right, but (4) typically means that you fail to start your workload from
> > a clean state, so you just go and fix it; no data is lost.
> > 
> > Compare this to a migration to an older KVM which results in your guest
> > crashing, where you risk data loss and still have to meddle with
> > configs.
> 
> True. To make it worse, we are already una

[Qemu-devel] [Consult] nfs-vsocks support

[Liu, Jing2]

Hello,

I am currently trying to use nfs-vsocks on x86 for vitural machine 
filesystem by some manuals on 
https://www.spinics.net/lists/linux-nfs/msg64563.html

and https://lwn.net/Articles/647516/

It tells the quickstart steps with the following codes but I got some 
problems as listed.

 * Linux kernel: https://github.com/stefanha/linux.git vsock-nfs
 * QEMU virtio-vsock device: https://github.com/stefanha/qemu.git vsock
 * nfs-utils vsock: https://github.com/stefanha/nfs-utils.git vsock

1. I would like to ask what is the current status of these codes? If 
there're some updates? :)
2. nfs-utils codes could not compile for me... I refered to the README 
but failed for both ways. It seems no configure script and 
autoheader/automake... So I don't know how to install this.
3. For the 5th step (Start nfsd), I'd like to know the rpc.nfsd --vsock 
is enabled by which codes? Is it the CONFIG_VHOST_VSOCK kernel module in 
host? Because I installed the kernel with vsock-nfs repo with the 
specified config options but failed to do rpc.nfsd --vsock.



Could you give me some help and thanks in advance!

BRs,
Jing Liu

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Peter Maydell]

On 23 March 2018 at 08:36, Thomas Huth  wrote:
> Most of the RISC-V boards currently crash when they are started with
> "-nodefaults", e.g.:
>
> $ gdb --args riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e
> [...]
> (gdb) r
> Program received signal SIGSEGV, Segmentation fault.
> qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
> at chardev/char-fe.c:210
> 210 } else if (s->be) {
> (gdb) bt
>  0  0x558695f8 in qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
> at chardev/char-fe.c:210
>  1  0x556fb425 in sifive_uart_create ([...], chr=0x0, [...])
> at hw/riscv/sifive_uart.c:169
>  2  0x556f8cc4 in riscv_sifive_e_init (machine=[...])
> at hw/riscv/sifive_e.c:164
> [...]
>
> With "-nodefaults" there are no entries in serial_hds[], so qemu_chr_fe_init()
> finally tries to dereference a NULL pointer. Let's fix this problem by
> creating a "null" chardev in this case instead.

Isn't it possible to fix this another level further down
by having qemu_chr_fe_init() &c handle having a NULL chardev?
That would be nicer than requiring every UART model to handle
it, because inevitably people will forget about that corner case.

> Signed-off-by: Thomas Huth 
> ---
>  For other boards / targets, see also:
>  https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05073.html
>
>  hw/riscv/riscv_htif.c  | 5 +
>  hw/riscv/sifive_uart.c | 5 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
> index 3e17f30..d3d31ff 100644
> --- a/hw/riscv/riscv_htif.c
> +++ b/hw/riscv/riscv_htif.c
> @@ -245,9 +245,14 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, 
> MemoryRegion *main_mem,
>  s->pending_read = 0;
>  s->allow_tohost = 0;
>  s->fromhost_inprogress = 0;
> +
> +if (!chr) {
> +chr = qemu_chardev_new(NULL, TYPE_CHARDEV_NULL, NULL, &error_abort);
> +}
>  qemu_chr_fe_init(&s->chr, chr, &error_abort);
>  qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
>  htif_be_change, s, NULL, true);
> +
>  if (base) {
>  memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
>  TYPE_HTIF_UART, size);

As an aside: this source file looks like it's a device, but
it isn't a QOM device. That seems like it needs fixing, and it
should probably be in hw/char or hw/misc.

> diff --git a/hw/riscv/sifive_uart.c b/hw/riscv/sifive_uart.c
> index b0c3798..2bde8bb 100644
> --- a/hw/riscv/sifive_uart.c
> +++ b/hw/riscv/sifive_uart.c

If this is a UART why isn't it in hw/char ? It should also
be a proper QOM device, and moved to the right place in
the source tree.

> @@ -166,9 +166,14 @@ SiFiveUARTState *sifive_uart_create(MemoryRegion 
> *address_space, hwaddr base,
>  {
>  SiFiveUARTState *s = g_malloc0(sizeof(SiFiveUARTState));
>  s->irq = irq;
> +
> +if (!chr) {
> +chr = qemu_chardev_new(NULL, TYPE_CHARDEV_NULL, NULL, &error_abort);
> +}
>  qemu_chr_fe_init(&s->chr, chr, &error_abort);
>  qemu_chr_fe_set_handlers(&s->chr, uart_can_rx, uart_rx, uart_event,
>  uart_be_change, s, NULL, true);
> +
>  memory_region_init_io(&s->mmio, NULL, &uart_ops, s,
>TYPE_SIFIVE_UART, SIFIVE_UART_MAX);
>  memory_region_add_subregion(address_space, base, &s->mmio);
> --
> 1.8.3.1

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 19/23] qmp: isolate responses into io thread

[Marc-André Lureau]

Hi

On Fri, Mar 23, 2018 at 6:50 AM, Peter Xu  wrote:
> On Thu, Mar 22, 2018 at 01:00:19PM +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu  wrote:
>> > For those monitors who have enabled IO thread, we'll offload the
>> > responding procedure into IO thread.  The main reason is that chardev is
>> > not thread safe, and we need to do all the read/write IOs in the same
>> > thread.  For use_io_thr=true monitors, that thread is the IO thread.
>>
>> Actually, the chr write path is suppose to be somewhat thread safe
>> (chr_write_lock).
>>
>> Secondly, the function responsible to write monitor data has some
>> thread-safety, it's called monitor_flush_locked(), because you need
>> the mon->out_lock.
>>
>> I think that patch is making things more complicated than they need to
>> be. You should be able to call monitor_json_emitter/monitor_puts()
>> directly from any thread, it will queue the data, start writing, and
>> add a watch if necessary in the appropriate context.
>>
>> Am I missing something?
>
> Yes there are the locks either in monitor code and chardev code to
> protect write operations.  However I don't know enough to be sure of
> its safety.  Considering that, having the whole chardev operations
> separated into the other IOThread seems to be the only safe approach
> to me for now.

I have studied the code in more details, and I'll propose to revert
this patch early in 2.13. I'll send a 2.13 RFC qmp series shortly.



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v8 18/23] qmp: support out-of-band (oob) execution

[Marc-André Lureau]

Hi

On Fri, Mar 23, 2018 at 6:18 AM, Peter Xu  wrote:
> On Thu, Mar 22, 2018 at 11:22:14AM +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu  wrote:
>> > Having "allow-oob" to true for a command does not mean that this command
>> > will always be run in out-of-band mode.  The out-of-band quick path will
>> > only be executed if we specify the extra "run-oob" flag when sending the
>> > QMP request:
>> >
>> > { "execute":   "command-that-allows-oob",
>> >   "arguments": { ... },
>> >   "control":   { "run-oob": true } }
>> >
>> > The "control" key is introduced to store this extra flag.  "control"
>> > field is used to store arguments that are shared by all the commands,
>> > rather than command specific arguments.  Let "run-oob" be the first.
>> >
>> > Note that in the patch I exported qmp_dispatch_check_obj() to be used to
>> > check the request earlier, and at the same time allowed "id" field to be
>> > there since actually we always allow that.
>> >
>> > Reviewed-by: Stefan Hajnoczi 
>> > Signed-off-by: Peter Xu 
>> > ---
>> >  include/qapi/qmp/dispatch.h |  2 ++
>> >  monitor.c   | 84 
>> > -
>> >  qapi/qmp-dispatch.c | 33 +-
>> >  trace-events|  2 ++
>> >  4 files changed, 111 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
>> > index 26eb13ff41..ffb4652f71 100644
>> > --- a/include/qapi/qmp/dispatch.h
>> > +++ b/include/qapi/qmp/dispatch.h
>> > @@ -48,6 +48,8 @@ bool qmp_command_is_enabled(const QmpCommand *cmd);
>> >  const char *qmp_command_name(const QmpCommand *cmd);
>> >  bool qmp_has_success_response(const QmpCommand *cmd);
>> >  QObject *qmp_build_error_object(Error *err);
>> > +QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp);
>> > +bool qmp_is_oob(QDict *dict);
>> >
>> >  typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
>> >
>> > diff --git a/monitor.c b/monitor.c
>> > index 4d57a8d341..5c8afe9f50 100644
>> > --- a/monitor.c
>> > +++ b/monitor.c
>> > @@ -1110,6 +1110,44 @@ static void qmp_caps_apply(Monitor *mon, 
>> > QMPCapabilityList *list)
>> >  }
>> >  }
>> >
>> > +/*
>> > + * Return true if check successful, or false otherwise.  When false is
>> > + * returned, detailed error will be in errp if provided.
>> > + */
>> > +static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
>> > +{
>> > +const char *command;
>> > +QmpCommand *cmd;
>> > +
>> > +command = qdict_get_try_str(req, "execute");
>> > +if (!command) {
>> > +error_setg(errp, "Command field 'execute' missing");
>> > +return false;
>> > +}
>> > +
>> > +cmd = qmp_find_command(mon->qmp.commands, command);
>> > +if (!cmd) {
>> > +error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
>> > +  "The command %s has not been found", command);
>> > +return false;
>> > +}
>> > +
>> > +if (qmp_is_oob(req)) {
>> > +if (!qmp_oob_enabled(mon)) {
>> > +error_setg(errp, "Please enable Out-Of-Band first "
>> > +   "for the session during capabilities negociation");
>> > +return false;
>> > +}
>> > +if (!(cmd->options & QCO_ALLOW_OOB)) {
>> > +error_setg(errp, "The command %s does not support OOB",
>> > +   command);
>> > +return false;
>> > +}
>> > +}
>> > +
>> > +return true;
>> > +}
>> > +
>> >  void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
>> >Error **errp)
>> >  {
>> > @@ -4001,6 +4039,7 @@ static void monitor_qmp_bh_dispatcher(void *data)
>> >  QMPRequest *req_obj = monitor_qmp_requests_pop_one();
>> >
>> >  if (req_obj) {
>> > +trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: 
>> > "");
>> >  monitor_qmp_dispatch_one(req_obj);
>> >  /* Reschedule instead of looping so the main loop stays 
>> > responsive */
>> >  qemu_bh_schedule(mon_global.qmp_dispatcher_bh);
>> > @@ -4024,17 +4063,31 @@ static void handle_qmp_command(JSONMessageParser 
>> > *parser, GQueue *tokens)
>> >  error_setg(&err, QERR_JSON_PARSING);
>> >  }
>> >  if (err) {
>> > -monitor_qmp_respond(mon, NULL, err, NULL);
>> > -qobject_decref(req);
>> > -return;
>> > +goto err;
>> > +}
>> > +
>> > +/* Check against the request in general layout */
>> > +qdict = qmp_dispatch_check_obj(req, &err);
>> > +if (!qdict) {
>> > +goto err;
>> >  }
>> >
>> > -qdict = qobject_to_qdict(req);
>> > -if (qdict) {
>> > -id = qdict_get(qdict, "id");
>> > -qobject_incref(id);
>> > -qdict_del(qdict, "id");
>> > -} /* else will fail qmp_dispatch() */
>> > +/* Check against OOB specific */
>> > +if (!qmp_cmd_oob_ch

Re: [Qemu-devel] [PATCH v2 1/1] iotests: fix test case 185

[Stefan Hajnoczi]

On Fri, Mar 23, 2018 at 3:43 AM, QingFeng Hao  wrote:
> Test case 185 failed since commit 4486e89c219 --- "vl: introduce 
> vm_shutdown()".
> It's because of the newly introduced function vm_shutdown calls 
> bdrv_drain_all,
> which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
> that doubles the speed and offset is doubled.
> Some jobs' status are changed as well.
>
> The fix is to not resume the jobs that are already yielded and also change
> 185.out accordingly.
>
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: QingFeng Hao 
> ---
>  blockjob.c | 10 +-
>  include/block/blockjob.h   |  5 +
>  tests/qemu-iotests/185.out | 11 +--

If drain no longer forces the block job to iterate, shouldn't the test
output remain the same?  (The means the test is fixed by the QEMU
patch.)

>  3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index ef3ed69ff1..fa9838ac97 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -206,11 +206,16 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob 
> *job)
>
>  static void block_job_pause(BlockJob *job)
>  {
> -job->pause_count++;
> +if (!job->yielded) {
> +job->pause_count++;
> +}

The pause cannot be ignored.  This change introduces a bug.

Pause is not a synchronous operation that stops the job immediately.
Pause just remembers that the job needs to be paused.   When the job
runs again (e.g. timer callback, fd handler) it eventually reaches
block_job_pause_point() where it really pauses.

The bug in this patch is:

1. The job has a timer pending.
2. block_job_pause() is called during drain.
3. The timer fires during drain but now the job doesn't know it needs
to pause, so it continues running!

Instead what needs to happen is that block_job_pause() remains
unmodified but block_job_resume if extended:

static void block_job_resume(BlockJob *job)
{
assert(job->pause_count > 0);
job->pause_count--;
if (job->pause_count) {
return;
}
+if (job_yielded_before_pause_and_is_still_yielded) {
block_job_enter(job);
+}
}

This handles the case I mentioned above, where the yield ends before
pause ends (therefore resume must enter the job!).

To make this a little clearer, there are two cases to consider:

Case 1:
1. Job yields
2. Pause
3. Job is entered from timer/fd callback
4. Resume (enter job? yes)

Case 2:
1. Job yields
2. Pause
3. Resume (enter job? no)
4. Job is entered from timer/fd callback

Stefan

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.12] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

[Andrew Jones]

On Thu, Mar 22, 2018 at 05:42:02PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Peter,
> 
> On 03/22/2018 03:29 PM, Peter Maydell wrote:
> > On 22 March 2018 at 14:23, Andrew Jones  wrote:
> >> On Thu, Mar 15, 2018 at 01:34:41PM +, Peter Maydell wrote:
> >>> If the GIC has the security extension support enabled, then a
> >>> non-secure access to ICC_PMR must take account of the non-secure
> >>> view of interrupt priorities, where real priorities 0..0x7f
> >>> are secure-only and not visible to the non-secure guest, and
> >>> priorities 0x80..0xff are shown to the guest as if they were
> >>> 0x00..0xff. We had the logic here wrong:
> >>
> >> 0x00..0x7f
> > 
> > I think 0x00..0xff is correct.
> 
> I guess Andrew only suggested to correct the hex prefix in your comment:
> - ... where real priorities 0..0x7f
> + ... where real priorities 0x00..0x7f

Actually I just hadn't thought that through properly. But thanks for
giving me the benefit of the doubt :-)

> > 
> > the NS view covers the whole 0x00..0xff range, but more sparsely.
> > (OK, technically you can't ever read 0xff, only 0xfe.)
> 

You can read 0xff, because 0xff will be written when either 0xfe or 0xff
are written. Then, when reading, priority == 0xff will fail the != 0xff
test (which is QEMU's implementation of the pseudo code where PRIMask()
is hardcoded to 0xff). In that case it'll just return the priority value
unchanged, which is 0xff.

Thanks,
drew

Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment

[Roman Kagan]

On Thu, Mar 22, 2018 at 04:01:46PM -0300, Eduardo Habkost wrote:
> On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote:
> > We can also expose Hyper-V frequency MSRs when reenlightenment feature is
> > enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC
> > page clocksources to its guests.
> > 
> > Signed-off-by: Vitaly Kuznetsov 
> > ---
> > - Expose frequency MSRs only when either INVTSC or Reenlightenment is
> >   provided [Paolo Bonzini]
> > ---
> >  target/i386/kvm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 75f4e1d69e..2c3c19d690 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -651,7 +651,8 @@ static int hyperv_handle_properties(CPUState *cs)
> >  env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
> >  env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
> >  
> > -if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
> > +if (has_msr_hv_frequencies && env->tsc_khz &&
> > +(tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) 
> > {
> 
> Continuing the discussion we had in v3 about being possible to
> crash when migrating to a host running an older kernel:
> 
> This patch doesn't fix the issue, because it is still implicitly
> enabling hv-frequencies.
> 
> But I don't think this patch will make the situation any worse,
> because we don't have any KVM versions that support
> HV_X64_MSR_REENLIGHTENMENT_CONTROL but not
> HV_X64_MSR_TSC_FREQUENCY.
> 
> This means that we can safely make "hv-reenlightenment=on"
> automatically set "hv-frequencies=on", when we finally introduce
> a hv-frequencies property.

Do I get you right that there's no controversy about the need for
hv-frequencies?

> Roman, do you agree?

Well, this strictly depends on the answer to your next question, because
hv-reenlightenment is most certainly 2.13 material.

> The next question is: do we need to fix this and introduce a
> hv-frequencies property in 2.12, or can this wait for 2.13?

I'm tempted to stick hv-frequencies into 2.12 and retrospectively into
2.11-stable, to minimize the exposure to the problem but to keep the
effort and added complexity reasonable.  But I'm not sure this will be
ok from the compatibility policy standpoint.  So I'd appreciate an
advice on this.
[Should I better just post a patch doing that and continue this
discussion there?]

Thanks,
Roman.

Re: [Qemu-devel] [PULL 00/25] RISC-V Post-merge spec conformance and cleanup

[Peter Maydell]

On 20 March 2018 at 22:25, Michael Clark  wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> The following changes since commit f1a63fcfcd92c88be8942b5ae71aef9749a4f135:
>
>   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
>
> are available in the git repository at:
>
>   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-2.12-fixes
>
> for you to fetch changes up to e1a247183ac0816284dfda61423f7030b6686679:
>
>   RISC-V: Remove erroneous comment from translate.c (2018-03-20 14:34:13 
> -0700)

I know this pull request needs a respin, but I gave it a test build
run anyway to see if there were any issues thrown up by that (there
weren't, which is great).

I did notice that the tag name you quote here, "tags/riscv-qemu-2.12-fixes",
doesn't seem to be a tag pushed to your git repo. I tested with
"tags/riscv-qemu-2.12-fixes-v5" which I'm guessing is what you meant.
You probably want to have a look into the workflow which generates
these pullreq emails though to ensure it's creating them with the
tag names that you intend, though.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Daniel P . Berrangé]

On Thu, Mar 22, 2018 at 09:27:55PM +0200, Michael S. Tsirkin wrote:
> Make sure all generated files go into qemu-build subdirectory.
> We can then include them like this:
>  #include "qemu-build/trace.h"
> 
> This serves two purposes:
> - make it easy to detect which files are in the source
>   directory (a bit more work for writers, easier for readers)
> - reduce chances of conflicts with possible stale files in source
>   directory (which could be left over from e.g. old patches, etc)

If people care about this, then they can just be doing a build
with  srcdir != builddir config.  If people are using srcdir == builddir
then they likely *want* all the generated files in their srcdir.

IMHO it would be valid for us to consider if we could just mandate
srcdir != builddir, but if people object to such a proposal, then I
don't think we should arbitrarily move all generated source files
in this way, as that's effectively the same thing forced onto devs.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Auger Eric]

Hi,

I observe a regression on KVM accelerated qemu-system-aarch64:

Unexpected error in kvm_device_access() at
/home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
failed: Group 6 attr 0xc664: Device or resource busy
2018-03-23 10:00:00.085+: shutting down, reason=crashed

I did a bisect and it looks the commit that introduces the regression is

"
commit 3fd2457d18edf5736f713dfe1ada9c87a9badab1
Author: Peter Xu 
Date:   Fri Mar 9 17:00:03 2018 +0800

monitor: enable IO thread for (qmp & !mux) typed

Start to use dedicate IO thread for QMP monitors that are not using
MUXed chardev.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
Message-Id: <20180309090006.10018-21-pet...@redhat.com>
Signed-off-by: Eric Blake 
"

If reverted on top of v2.12.0-rc0, the guest boots fine again. The link
is not obvious though.

Thanks

Eric

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Peter Maydell]

On 23 March 2018 at 10:24, Auger Eric  wrote:
> Hi,
>
> I observe a regression on KVM accelerated qemu-system-aarch64:
>
> Unexpected error in kvm_device_access() at
> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
> 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
> failed: Group 6 attr 0xc664: Device or resource busy
> 2018-03-23 10:00:00.085+: shutting down, reason=crashed

Can you get a backtrace for this? (I guess you'd need to fiddle
with the kvm_device_access() code to make it assert rather
than passing back the error).

thanks
-- PMM

[Qemu-devel] [PATCH] monitor: fix expected qmp_capabilities error description regression

[Marc-André Lureau]

Fix regression introduced in commit cf869d53172920536a14180a83292b240e9d0545:

Before:
{"execute":"foo"}
{"error": {"class": "CommandNotFound", "desc": "Expecting capabilities 
negotiation with 'qmp_capabilities'"}}

After:
{"execute":"foo"}
{"error": {"class": "CommandNotFound", "desc": "The command foo has not been 
found"}}

Because qmp_cmd_oob_check() happens before
monitor_qmp_dispatch_one(). Move the error manipulation code to
monitor_qmp_respond() instead.

Signed-off-by: Marc-André Lureau 
---
 monitor.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/monitor.c b/monitor.c
index 6ccd2fc089..d57134 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3997,6 +3997,18 @@ static void monitor_qmp_respond(Monitor *mon, QObject 
*rsp,
 qdict_put_obj(qobject_to(QDict, rsp), "id", id);
 }
 
+if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
+qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
+if (qdict
+&& !g_strcmp0(qdict_get_try_str(qdict, "class"),
+  QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
+/* Provide a more useful error message */
+qdict_del(qdict, "desc");
+qdict_put_str(qdict, "desc", "Expecting capabilities"
+  " negotiation with 'qmp_capabilities'");
+}
+}
+
 monitor_json_emitter(mon, rsp);
 }
 
@@ -4028,7 +4040,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
 {
 Monitor *mon, *old_mon;
 QObject *req, *rsp = NULL, *id;
-QDict *qdict = NULL;
 bool need_resume;
 
 req = req_obj->req;
@@ -4051,18 +4062,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
 
 cur_mon = old_mon;
 
-if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
-qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
-if (qdict
-&& !g_strcmp0(qdict_get_try_str(qdict, "class"),
-QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
-/* Provide a more useful error message */
-qdict_del(qdict, "desc");
-qdict_put_str(qdict, "desc", "Expecting capabilities negotiation"
-  " with 'qmp_capabilities'");
-}
-}
-
 /* Respond if necessary */
 monitor_qmp_respond(mon, rsp, NULL, id);
 
-- 
2.17.0.rc1.1.g4c4f2b46a3

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Alberto Garcia]

On Thu 22 Mar 2018 05:12:26 PM CET, Laurent Vivier wrote:

> diff --git a/block/quorum.c b/block/quorum.c
> index 14333c18aa..304442ef65 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -608,7 +608,7 @@ static void read_quorum_children_entry(void *opaque)
>  static int read_quorum_children(QuorumAIOCB *acb)
>  {
>  BDRVQuorumState *s = acb->bs->opaque;
> -int i, ret;
> +int i;
>  
>  acb->children_read = s->num_children;
>  for (i = 0; i < s->num_children; i++) {
> @@ -643,9 +643,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
>  qemu_coroutine_yield();
>  }
>  
> -ret = acb->vote_ret;
> -
> -return ret;
> +return acb->vote_ret;
>  }

The Quorum part,

Reviewed-by: Alberto Garcia 

Berto

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

[Eduardo Otubo]

On 16/03/2018 - 11:46:57, Thomas Huth wrote:
> On 27.11.2017 09:40, Eduardo Otubo wrote:
> > On Fri, Nov 24, 2017 at 06:44:59PM +0100, Thomas Huth wrote:
> >>  Hi Eduardo,
> >>
> >> On 24.11.2017 14:46, Eduardo Otubo wrote:
> >>> v3:
> >>>  * Removed all unecessary local_err
> >>>  * Change return of isa_bus_dma() and DMA_init() from void to int8_t,
> >>>returning -EBUSY on error and 0 on success
> >>>  * Added qdev_cleanup_nofail() in case isa_bus_dma() returns error. The
> >>>cleanup looks safe, but please review if I didn't miss any detail
> >>>
> >>> v2:
> >>>  * Removed user_creatable=false and replaced by error handling using
> >>>Error **errp and error_propagate();
> >>
> >> Version changelog should go below the "---" separator, otherwise it will
> >> be included in the git changelog as well, which is kind of ugly.
> >>
> >>> QEMU fails when used with the following command line:
> >>>
> >>> ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p,accel=tcg -device 
> >>> i82374
> >>> qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion 
> >>> `!bus->dma[0] && !bus->dma[1]' failed.
> >>> Aborted (core dumped)
> >>>
> >>> The 40p machine type already created the device i82374. If specified in 
> >>> the
> >>> command line, it will try to create it again, hence generating the error. 
> >>> The
> >>> function isa_bus_dma() isn't supposed to be called twice for the same 
> >>> bus. One
> >>> way to avoid this problem is to set user_creatable=false.
> >>
> >> You don't do that user_creatable=false here anymore, so please remove it
> >> from the description.
> >>
> >>> A possible fix in a near future would be making
> >>> isa_bus_dma()/DMA_init()/i82374_realize() return an error instead of 
> >>> asserting
> >>> as well.
> >>
> >> You should rephrase that sentence as well.
> >>
> > 
> > Well, I think one mistake lead to another here. I've always put the 
> > changelog
> > before the --- and that's why the old commit message. For example:
> > 2f668be77501c0232a84aafb6a066c9915987f0e. But I guess on that context it 
> > made
> > sense to use the changelog since the commit message was too simplistic. I'm
> > gonna fix this on the v3 then, among other thigs that I need to fix. Thanks 
> > for
> > the heads up :)
> 
>  Hi,
> 
> did you ever send a v4? The problem seems still to persist...
> 

I didn't, I guess I put this on the bottom of my priority list and never got to
it.

I'll send a new version right away.
Thanks for the reminder.

-- 
Eduardo Otubo

Re: [Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly

[Peter Maydell]

On 20 March 2018 at 01:36, Trent Piepho  wrote:
> Linux does not detect a break from this IMX serial driver as a magic
> sysrq.  Nor does it note a break in the port error counts.
>
> The former is because the Linux driver uses the BRCD bit in the USR2
> register to trigger the RS-232 break handler in the kernel, which is
> where sysrq hooks in.  The emulated UART was not setting this status
> bit.
>
> The latter is because the Linux driver expects, in addition to the BRK
> bit, that the ERR bit is set when a break is read in the FIFO.  A break
> should also count as a frame error, so add that bit too.
>
> Cc: Andrey Smirnov 
> Signed-off-by: Trent Piepho 

Yep, the data sheet is nice and clear here about the requirements.

Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Kevin Wolf]

Am 23.03.2018 um 11:22 hat Daniel P. Berrangé geschrieben:
> On Thu, Mar 22, 2018 at 09:27:55PM +0200, Michael S. Tsirkin wrote:
> > Make sure all generated files go into qemu-build subdirectory.
> > We can then include them like this:
> >  #include "qemu-build/trace.h"
> > 
> > This serves two purposes:
> > - make it easy to detect which files are in the source
> >   directory (a bit more work for writers, easier for readers)
> > - reduce chances of conflicts with possible stale files in source
> >   directory (which could be left over from e.g. old patches, etc)
> 
> If people care about this, then they can just be doing a build
> with  srcdir != builddir config.  If people are using srcdir == builddir
> then they likely *want* all the generated files in their srcdir.
> 
> IMHO it would be valid for us to consider if we could just mandate
> srcdir != builddir, but if people object to such a proposal, then I
> don't think we should arbitrarily move all generated source files
> in this way, as that's effectively the same thing forced onto devs.

Not necessarily. I build from the srcdir simply because it's more
convenient to be able to easily start the editor and make from the same
directory (or sometimes start make from inside the editor). I suppose
that (or for users, being too lazy to create a second directory for that
one-time build) is the motivation for most other users of srcdir ==
builddir, too.

I don't really mind where generated source files are stored, they just
happen to end up in my root source directory if I want to be able to
build from there without dealing with paths. I won't be angry if they
are suddenly in a different directory as long as it doesn't mean more
typing for me. Moving them to a separate directory might in fact make
things a bit nicer occasionally. Not enough for me to actively propose
such a change, but if Michael wants to do this, I'm certainly not
opposed.

Kevin

Re: [Qemu-devel] [PULL 0/2] s390x: Fixes for 2.12

[Peter Maydell]

On 23 March 2018 at 09:14, Christian Borntraeger  wrote:
> Peter,
>
> 2 fixes for 2.12. Please pull
>
> The following changes since commit d522e0bd18364f6d784d43edab35b0563d21f6f3:
>
>   gitmodules: Use the QEMU mirror of qemu-palcode (2018-03-22 19:24:16 +)
>
> are available in the Git repository at:
>
>   git://github.com/borntraeger/qemu.git tags/s390x-20180323
>
> for you to fetch changes up to 06a97edac172f52971d7bdca6bb56a1edcb1b8f6:
>
>   s390x/cpumodel: fix feature groups and breakage of MSA8 (2018-03-23 
> 09:05:42 +)
>
> 
> s390x: Fixes for 2.12
>
> - Fix for the s390 cpumodel
> - Forbid multifunction PCI devices
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Check undefined opcodes for SWP in A32 decoder

[Peter Maydell]

On 23 March 2018 at 03:58, Onur Sahin  wrote:
> Hi all,
>
> I have noticed that the decoding part in ARM/A32 does not verify the
> opcodes for SWP instructions. The opcode field ([23:20]) for SWP
> instructions should be 0 or 4, and QEMU does not check against these
> values.
>
> Other opcode values less than 8 are Undefined within the encoding
> space of sychronization primitives (e.g., SWP, LDREX*). See section
> A5.2.10 of ARMv7-A manual for reference. Because of the missing opcode
> check, QEMU happily executes these Undefined cases as a SWP instruction.
>
> The following fix adds proper opcode checks before assuming a valid SWP.
>
> Best,
> Onur
>
> Signed-off-by: Onur Sahin 

Yes, we've been historically a bit lax with our arm decoding.
It's good to tighten it up, I think. Thanks for sending this patch;
I have some review comments below.

> ---
>  target-arm/translate.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index bd5d5cb..fb31c12 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8831,7 +8831,7 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  }
>  }
>  tcg_temp_free_i32(addr);
> -} else {
> +} else if (!(insn & 0x00B0)) {

I think we have already checked bit 23, so we don't need to check it again
(this is the else case of the "if (insn & (1 << 23))" condition).
I think we should also be checking bits [11:8] are zeroes. These are
"(0)" in the instruction description, which means that the instruction
is UNPREDICTABLE if those are not zero (see A5.1.2 "UNDEFINED and
UNPREDICTABLE instruction set space"). It's architecturally permitted
to not check those bits, but elsewhere in the decoder we generally
prefer to UNDEF on the UNPREDICTABLE cases.

So I would make this condition
} else if ((insn & 0x00300f00) == 0) {

I have been adding comments to the decoder as I modify it which
indicate what bits have been decoded at various points. In this case
I think we should add
   /* 0b_0001_0x00____1001_
*  - SWP, SWPB
*/

just inside this else if () { ...} block.

(and then you can delete the "SWP instruction" comment.)

>  /* SWP instruction */

What version of QEMU did you make this patch against? The current
git master has some other lines between the '} else {' and this
comment. Generally patches should be against git master, as that's
where all our development happens.

>  rm = (insn) & 0xf;
>
> @@ -8852,6 +8852,9 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  tcg_temp_free_i32(addr);
>  store_reg(s, rd, tmp2);
>  }
> +else {

Minor style nit: the "else {" should go on the same line as the
preceding "}".

> +goto illegal_op;
> +}
>  }
>  } else {
>  int address_offset;
> --
> 1.8.3.1

thanks
-- PMM

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Auger Eric]

Hi,

On 23/03/18 11:26, Peter Maydell wrote:
> On 23 March 2018 at 10:24, Auger Eric  wrote:
>> Hi,
>>
>> I observe a regression on KVM accelerated qemu-system-aarch64:
>>
>> Unexpected error in kvm_device_access() at
>> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
>> 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
>> failed: Group 6 attr 0xc664: Device or resource busy
>> 2018-03-23 10:00:00.085+: shutting down, reason=crashed
> 
> Can you get a backtrace for this? (I guess you'd need to fiddle
> with the kvm_device_access() code to make it assert rather
> than passing back the error).

OK. I will try to do so. As I could have expected, I cannot reproduce on
a standalone qemu command line. The problem observed above is seen with
libvirt launch which may be doing some other QMP stuff concurrently?

Thanks

Eric
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR

[Peter Maydell]

On 21 March 2018 at 08:00, Shannon Zhao  wrote:
> On 2018/3/20 19:54, Peter Maydell wrote:
>> Can you still successfully migrate a VM from a QEMU version
>> without this bugfix to one with the bugfix ?
>>
> I've tested this case. I can migrate a VM between these two versions.

Hmm. Looking at the code I can't see how that would work,
except by accident. Let me see if I understand what's happening
here:

In the code in master, we have QEMU data structures
(bitmaps, etc) which have one entry for each of GICV3_MAXIRQ
irqs. That includes the RAZ/WI unused space for the SPIs/PPIs, so
for a 1-bit-per-irq bitmap:
 [0x, irq 32, irq 33,  ]

When we fill in the values from KVM into these data structures,
we start after the unused space, because the for_each_dist_irq_reg()
macro starts with _irq = GIC_INTERNAL. But we forgot to adjust
the offset value we use for the KVM access, so we start by
reading the RAZ/WI values from KVM, and the data structure
contents end up with:
 [0x, 0x, irq 32, irq 33, ... ]
(and the last irqs wouldn't get transferred).

With this change to the code we will get the offset right and
the data structure will be filled as
 [0x, irq 32, irq 33,  ]

But for migration from the old version, the data structure
we receive from the migration source will contain the old
broken layout of
 [0x, 0x, irq 32, irq 33, ... ]
so if the new code doesn't do anything special to handle
migration from that old version then it will write zeroes to
irq 32..63, and then write incorrect values for all the irqs
after that, won't it?

That suggests to me that we need to have some code in the
migration post-load routine that identifies that the data
is coming from an old version with this bug, and shifts
all the data down in the arrays so that the code to write
it to the kernel can handle it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Christian Borntraeger]

As Max Reitz said, this breaks several qemu iotests. I can reproduce that on 
s390
e.g. with ./check -qcow2 030 in the qemu-iotest.

Why was this merged?


On 03/09/2018 10:00 AM, Peter Xu wrote:
> Start to use dedicate IO thread for QMP monitors that are not using
> MUXed chardev.
> 
> Reviewed-by: Fam Zheng 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor.c b/monitor.c
> index c6de5f123e..9e8ee2ed14 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -36,6 +36,7 @@
>  #include "net/slirp.h"
>  #include "chardev/char-fe.h"
>  #include "chardev/char-io.h"
> +#include "chardev/char-mux.h"
>  #include "ui/qemu-spice.h"
>  #include "sysemu/numa.h"
>  #include "monitor/monitor.h"
> @@ -4533,8 +4534,10 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
>  void monitor_init(Chardev *chr, int flags)
>  {
>  Monitor *mon = g_malloc(sizeof(*mon));
> +/* Enable IOThread for QMPs that are not using MUX chardev backends. */
> +bool use_io_thr = (!CHARDEV_IS_MUX(chr)) && (flags & 
> MONITOR_USE_CONTROL);
> 
> -monitor_data_init(mon, false, false);
> +monitor_data_init(mon, false, use_io_thr);
> 
>  qemu_chr_fe_init(&mon->chr, chr, &error_abort);
>  mon->flags = flags;
> 

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Peter Maydell]

On 23 March 2018 at 12:01, Auger Eric  wrote:
> Hi,
>
> On 23/03/18 11:26, Peter Maydell wrote:
>> On 23 March 2018 at 10:24, Auger Eric  wrote:
>>> Hi,
>>>
>>> I observe a regression on KVM accelerated qemu-system-aarch64:
>>>
>>> Unexpected error in kvm_device_access() at
>>> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
>>> 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
>>> failed: Group 6 attr 0xc664: Device or resource busy
>>> 2018-03-23 10:00:00.085+: shutting down, reason=crashed
>>
>> Can you get a backtrace for this? (I guess you'd need to fiddle
>> with the kvm_device_access() code to make it assert rather
>> than passing back the error).
>
> OK. I will try to do so. As I could have expected, I cannot reproduce on
> a standalone qemu command line. The problem observed above is seen with
> libvirt launch which may be doing some other QMP stuff concurrently?

Hmm, that could be a bit painful to debug. I dunno if libvirt
has a "launch QEMU under gdb" option. If not, you could try
something like:
   if (condition we want to get a backtrace on) {
   printf("hit condition, attach gdb to process %d\n", (int)getpid());
   for (;;) { }
   }

and then QEMU will sit in a loop waiting for you to do a
  gdb path/to/qemu 

thanks
-- PMM

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Christian Borntraeger]

On 03/23/2018 01:11 PM, Peter Maydell wrote:
> On 23 March 2018 at 12:01, Auger Eric  wrote:
>> Hi,
>>
>> On 23/03/18 11:26, Peter Maydell wrote:
>>> On 23 March 2018 at 10:24, Auger Eric  wrote:
 Hi,

 I observe a regression on KVM accelerated qemu-system-aarch64:

 Unexpected error in kvm_device_access() at
 /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
 failed: Group 6 attr 0xc664: Device or resource busy
 2018-03-23 10:00:00.085+: shutting down, reason=crashed
>>>
>>> Can you get a backtrace for this? (I guess you'd need to fiddle
>>> with the kvm_device_access() code to make it assert rather
>>> than passing back the error).
>>
>> OK. I will try to do so. As I could have expected, I cannot reproduce on
>> a standalone qemu command line. The problem observed above is seen with
>> libvirt launch which may be doing some other QMP stuff concurrently?
> 
> Hmm, that could be a bit painful to debug. I dunno if libvirt
> has a "launch QEMU under gdb" option. If not, you could try
> something like:
>if (condition we want to get a backtrace on) {
>printf("hit condition, attach gdb to process %d\n", (int)getpid());
>for (;;) { }
>}
> 
> and then QEMU will sit in a loop waiting for you to do a
>   gdb path/to/qemu 
> 
> thanks
> -- PMM
> 

This patch also breaks the qemu iotest. (qemu hangs).

backtrace for that is


Thread 4 (Thread 0x3ffa9c3c910 (LWP 171339)):
#0  0x03ffb9d11a70 in __lll_lock_wait () at /lib64/libpthread.so.0
#1  0x03ffb9d0a630 in pthread_mutex_lock () at /lib64/libpthread.so.0
#2  0x01557e90 in qemu_mutex_lock_impl (mutex=0x1a63f90 
, file=0x160bc36 "/home/cborntra/REPOS/qemu/cpus.c", 
line=1757)
at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:67
#3  0x0108ad5e in qemu_mutex_lock_iothread () at 
/home/cborntra/REPOS/qemu/cpus.c:1757
#4  0x01089c70 in qemu_dummy_cpu_thread_fn (arg=0x3d4f9eb0) at 
/home/cborntra/REPOS/qemu/cpus.c:1258
#5  0x03ffb9d07a88 in start_thread () at /lib64/libpthread.so.0
#6  0x03ffb731940e in thread_start () at /lib64/libc.so.6

Thread 3 (Thread 0x3ffaa43d910 (LWP 171338)):
#0  0x03ffb730c050 in poll () at /lib64/libc.so.6
#1  0x03ffb8bd13b4 in g_main_context_iterate.isra () at 
/lib64/libglib-2.0.so.0
#2  0x03ffb8bd1840 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#3  0x011f499e in iothread_run (opaque=0x3d2a4110) at 
/home/cborntra/REPOS/qemu/iothread.c:70
#4  0x03ffb9d07a88 in start_thread () at /lib64/libpthread.so.0
#5  0x03ffb731940e in thread_start () at /lib64/libc.so.6

Thread 2 (Thread 0x3ffab743910 (LWP 171336)):
#0  0x03ffb7313ada in syscall () at /lib64/libc.so.6
#1  0x01558b5e in qemu_futex_wait (f=0x1e9b41c , 
val=4294967295) at /home/cborntra/REPOS/qemu/include/qemu/futex.h:29
#2  0x01558e16 in qemu_event_wait (ev=0x1e9b41c ) 
at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:445
#3  0x0157af82 in call_rcu_thread (opaque=0x0) at 
/home/cborntra/REPOS/qemu/util/rcu.c:261
#4  0x03ffb9d07a88 in start_thread () at /lib64/libpthread.so.0
#5  0x03ffb731940e in thread_start () at /lib64/libc.so.6

Thread 1 (Thread 0x3ffba146290 (LWP 171335)):
#0  0x03ffb730c1a2 in ppoll () at /lib64/libc.so.6
#1  0x015502da in qemu_poll_ns (fds=0x3d3b2720, nfds=1, timeout=-1) at 
/home/cborntra/REPOS/qemu/util/qemu-timer.c:322
#2  0x01554882 in aio_poll (ctx=0x3d3924e0, blocking=true) at 
/home/cborntra/REPOS/qemu/util/aio-posix.c:629
#3  0x0145533e in bdrv_drain_recurse (bs=0x3d3a6a70) at 
/home/cborntra/REPOS/qemu/block/io.c:197
#4  0x01455f1a in bdrv_drain_all_begin () at 
/home/cborntra/REPOS/qemu/block/io.c:447
#5  0x014560d6 in bdrv_drain_all () at 
/home/cborntra/REPOS/qemu/block/io.c:476
#6  0x01089216 in do_vm_stop (state=RUN_STATE_SHUTDOWN, 
send_stop=false) at /home/cborntra/REPOS/qemu/cpus.c:1010
#7  0x01089266 in vm_shutdown () at 
/home/cborntra/REPOS/qemu/cpus.c:1022
#8  0x01208c08 in main (argc=18, argv=0x3fffbcfdbd8, 
envp=0x3fffbcfdc70) at /home/cborntra/REPOS/qemu/vl.c:4732

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Daniel P . Berrangé]

On Fri, Mar 23, 2018 at 12:11:17PM +, Peter Maydell wrote:
> On 23 March 2018 at 12:01, Auger Eric  wrote:
> > Hi,
> >
> > On 23/03/18 11:26, Peter Maydell wrote:
> >> On 23 March 2018 at 10:24, Auger Eric  wrote:
> >>> Hi,
> >>>
> >>> I observe a regression on KVM accelerated qemu-system-aarch64:
> >>>
> >>> Unexpected error in kvm_device_access() at
> >>> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
> >>> 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
> >>> failed: Group 6 attr 0xc664: Device or resource busy
> >>> 2018-03-23 10:00:00.085+: shutting down, reason=crashed
> >>
> >> Can you get a backtrace for this? (I guess you'd need to fiddle
> >> with the kvm_device_access() code to make it assert rather
> >> than passing back the error).
> >
> > OK. I will try to do so. As I could have expected, I cannot reproduce on
> > a standalone qemu command line. The problem observed above is seen with
> > libvirt launch which may be doing some other QMP stuff concurrently?
> 
> Hmm, that could be a bit painful to debug. I dunno if libvirt
> has a "launch QEMU under gdb" option. If not, you could try

Nothing official, but the following trick should still work:

https://www.berrange.com/posts/2011/10/12/debugging-early-startup-of-kvm-with-gdb-when-launched-by-libvirtd/


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Peter Xu]

On Fri, Mar 23, 2018 at 01:01:43PM +0100, Auger Eric wrote:
> Hi,
> 
> On 23/03/18 11:26, Peter Maydell wrote:
> > On 23 March 2018 at 10:24, Auger Eric  wrote:
> >> Hi,
> >>
> >> I observe a regression on KVM accelerated qemu-system-aarch64:
> >>
> >> Unexpected error in kvm_device_access() at
> >> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
> >> 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
> >> failed: Group 6 attr 0xc664: Device or resource busy
> >> 2018-03-23 10:00:00.085+: shutting down, reason=crashed
> > 
> > Can you get a backtrace for this? (I guess you'd need to fiddle
> > with the kvm_device_access() code to make it assert rather
> > than passing back the error).
> 
> OK. I will try to do so. As I could have expected, I cannot reproduce on
> a standalone qemu command line. The problem observed above is seen with
> libvirt launch which may be doing some other QMP stuff concurrently?

Maybe not concurrently inside libvirt, but after the patches AFAIK the
QMP part can be concurrent with something else or break some timing.

Anyway, sorry for that!  I'll see what I can do to help.

-- 
Peter Xu

[Qemu-devel] [PATCH v2 6/7] hw/vfio/display: add ramfb support

[Gerd Hoffmann]

So we have a boot display when using a vgpu as primary display.

Use vfio-pci-ramfb instead of vfio-pci to enable it.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/vfio/vfio-common.h |  2 ++
 hw/vfio/display.c | 10 ++
 hw/vfio/pci.c | 15 +++
 3 files changed, 27 insertions(+)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index d9360148e6..c8841a75c0 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -27,6 +27,7 @@
 #include "qemu/queue.h"
 #include "qemu/notify.h"
 #include "ui/console.h"
+#include "hw/display/ramfb.h"
 #ifdef CONFIG_LINUX
 #include 
 #endif
@@ -153,6 +154,7 @@ typedef struct VFIODMABuf {
 
 typedef struct VFIODisplay {
 QemuConsole *con;
+RAMFBState *ramfb;
 struct {
 VFIORegion buffer;
 DisplaySurface *surface;
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 7d727ce910..b172fd408a 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -124,6 +124,9 @@ static void vfio_display_dmabuf_update(void *opaque)
 
 primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
 if (primary == NULL) {
+if (dpy->ramfb) {
+ramfb_display_update(dpy->con, dpy->ramfb);
+}
 return;
 }
 
@@ -181,6 +184,8 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, 
Error **errp)
 vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
   &vfio_display_dmabuf_ops,
   vdev);
+if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
+vdev->dpy->ramfb = ramfb_setup(errp);
 return 0;
 }
 
@@ -217,6 +222,9 @@ static void vfio_display_region_update(void *opaque)
 return;
 }
 if (!plane.drm_format || !plane.size) {
+if (dpy->ramfb) {
+ramfb_display_update(dpy->con, dpy->ramfb);
+}
 return;
 }
 format = qemu_drm_format_to_pixman(plane.drm_format);
@@ -289,6 +297,8 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev, 
Error **errp)
 vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
   &vfio_display_region_ops,
   vdev);
+if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
+vdev->dpy->ramfb = ramfb_setup(errp);
 return 0;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b9bc6cd310..7408992388 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3224,9 +3224,24 @@ static const TypeInfo vfio_pci_dev_info = {
 },
 };
 
+static void vfio_pci_ramfb_dev_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->hotpluggable = false;
+}
+
+static const TypeInfo vfio_pci_ramfb_dev_info = {
+.name = "vfio-pci-ramfb",
+.parent = "vfio-pci",
+.instance_size = sizeof(VFIOPCIDevice),
+.class_init = vfio_pci_ramfb_dev_class_init,
+};
+
 static void register_vfio_pci_dev_type(void)
 {
 type_register_static(&vfio_pci_dev_info);
+type_register_static(&vfio_pci_ramfb_dev_info);
 }
 
 type_init(register_vfio_pci_dev_type)
-- 
2.9.3

[Qemu-devel] [PATCH v2 5/7] hw/display: add virtio-ramfb

[Gerd Hoffmann]

Like virtio-vga, but using ramfb instead of legacy vga.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-ramfb.c | 149 ++
 hw/display/Makefile.objs  |   2 +-
 2 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 hw/display/virtio-ramfb.c

diff --git a/hw/display/virtio-ramfb.c b/hw/display/virtio-ramfb.c
new file mode 100644
index 00..7611c16d39
--- /dev/null
+++ b/hw/display/virtio-ramfb.c
@@ -0,0 +1,149 @@
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "ui/console.h"
+#include "hw/virtio/virtio-pci.h"
+#include "hw/display/ramfb.h"
+#include "qapi/error.h"
+
+/*
+ * virtio-vga: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_RAMFB "virtio-ramfb"
+#define VIRTIO_RAMFB(obj) \
+OBJECT_CHECK(VirtIORAMFB, (obj), TYPE_VIRTIO_RAMFB)
+
+typedef struct VirtIORAMFB {
+VirtIOPCIProxy parent_obj;
+VirtIOGPU  vdev;
+RAMFBState *ramfb;
+} VirtIORAMFB;
+
+static void virtio_ramfb_invalidate_display(void *opaque)
+{
+VirtIORAMFB *vramfb = opaque;
+
+if (vramfb->vdev.enable) {
+virtio_gpu_ops.invalidate(&vramfb->vdev);
+}
+}
+
+static void virtio_ramfb_update_display(void *opaque)
+{
+VirtIORAMFB *vramfb = opaque;
+VirtIOGPU *g = &vramfb->vdev;
+
+if (vramfb->vdev.enable) {
+virtio_gpu_ops.gfx_update(&vramfb->vdev);
+} else {
+ramfb_display_update(g->scanout[0].con, vramfb->ramfb);
+}
+}
+
+static int virtio_ramfb_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
+{
+VirtIORAMFB *vramfb = opaque;
+
+if (virtio_gpu_ops.ui_info) {
+return virtio_gpu_ops.ui_info(&vramfb->vdev, idx, info);
+}
+return -1;
+}
+
+static void virtio_ramfb_gl_block(void *opaque, bool block)
+{
+VirtIORAMFB *vramfb = opaque;
+
+if (virtio_gpu_ops.gl_block) {
+virtio_gpu_ops.gl_block(&vramfb->vdev, block);
+}
+}
+
+static const GraphicHwOps virtio_ramfb_ops = {
+.invalidate = virtio_ramfb_invalidate_display,
+.gfx_update = virtio_ramfb_update_display,
+.ui_info = virtio_ramfb_ui_info,
+.gl_block = virtio_ramfb_gl_block,
+};
+
+static const VMStateDescription vmstate_virtio_ramfb = {
+.name = "virtio-ramfb",
+.version_id = 2,
+.minimum_version_id = 2,
+.fields = (VMStateField[]) {
+/* no pci stuff here, saving the virtio device will handle that */
+/* FIXME */
+VMSTATE_END_OF_LIST()
+}
+};
+
+/* RAMFB device wrapper around PCI device around virtio GPU */
+static void virtio_ramfb_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+VirtIORAMFB *vramfb = VIRTIO_RAMFB(vpci_dev);
+VirtIOGPU *g = &vramfb->vdev;
+Error *err = NULL;
+int i;
+
+/* init virtio bits */
+qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus));
+virtio_pci_force_virtio_1(vpci_dev);
+object_property_set_bool(OBJECT(g), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+/* add stdramfb mmio regions */
+vramfb->ramfb = ramfb_setup(errp);
+graphic_console_set_hwops(g->scanout[0].con, &virtio_ramfb_ops, vramfb);
+
+for (i = 0; i < g->conf.max_outputs; i++) {
+object_property_set_link(OBJECT(g->scanout[i].con),
+ OBJECT(vpci_dev),
+ "device", errp);
+}
+}
+
+static Property virtio_ramfb_properties[] = {
+DEFINE_VIRTIO_GPU_PCI_PROPERTIES(VirtIOPCIProxy),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_ramfb_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+dc->props = virtio_ramfb_properties;
+dc->vmsd = &vmstate_virtio_ramfb;
+dc->hotpluggable = false;
+
+k->realize = virtio_ramfb_realize;
+pcidev_k->class_id = PCI_CLASS_DISPLAY_OTHER;
+}
+
+static void virtio_ramfb_inst_initfn(Object *obj)
+{
+VirtIORAMFB *dev = VIRTIO_RAMFB(obj);
+
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_GPU);
+}
+
+static TypeInfo virtio_ramfb_info = {
+.name  = TYPE_VIRTIO_RAMFB,
+.parent= TYPE_VIRTIO_PCI,
+.instance_size = sizeof(struct VirtIORAMFB),
+.instance_init = virtio_ramfb_inst_initfn,
+.class_init= virtio_ramfb_class_init,
+};
+
+static void virtio_ramfb_register_types(void)
+{
+type_register_static(&virtio_ramfb_info);
+}
+
+type_init(virtio_ramfb_register_types)
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 07d2f48782..e71d9d2ca6 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -37,7 +37,7 @@ obj-$(CONFIG_VGA) += vga.o
 common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o
 
 obj-$(CONFIG_VIRTIO) += virtio-gpu.o virtio-gpu-

[Qemu-devel] [PATCH v2 0/7] ramfb: simple boot framebuffer, no legacy vga

[Gerd Hoffmann]

  Hi,

Ok folks, here is a experimental patch series for a legacy free boot
framebuffer.  If you want play with it I recommend getting the bits from

https://www.kraxel.org/cgit/qemu/log/?h=sirius/ramfb

because they come with an updated seabios and a new vgabios rom and an
experimental OVMF build.

Functional overview
---

The boot framebuffer is expected to be configured by the firmware, so it
uses fw_cfg as interface.  Initialization goes as follows:

  (1) Check whenever etc/ramfb is present.
  (2) Allocate framebuffer from RAM.
  (3) Fill struct RAMFBCfg, write it to etc/ramfb.

Done.  You can write stuff to the framebuffer now, and it should appear
automagically on the screen.

Note that this isn't very efficient because it does a full display
update on each refresh.  No dirty tracking.  Dirty tracking would have
to be active for the whole ram slot, so that wouldn't be very efficient
either.  So it is *really* intended to be only active for a short time
at boot, before the guest loaded the drivers for the real display
hardware.

Firmware support -- seabios
---

seavgabios is able to emulate vga text mode on top of a framebuffer, for
coreboot native graphics initialialization.  Which works fine for
everything which writes text using the vgabios interface (basically
everyhing which works with sgabios).

So I hacked that up to work with ramfb.  Right now it's proof-of-concept
code with too much cut+paste, so it will clearly need a bunch of
cleanups if this approach turns out to be workable.  Look here:

https://www.kraxe.org/cgit/seabios/log/?h=ramfb

Firmware support -- edk2


There is a EFI driver too.  Likewise a hackish proof-of-concept thing,
clearly not in a mergeable state, but good enough for playing.  Note
that the build disables QemuVideoDxe and VirtoGpu drivers, so ramfb is
the only supported display.  Code is here:

https://github.com/kraxel/edk2/commits/ramfb

Firmware blob is in pc-bios/OVMF-ramfb.fd, to be used with -bios.

So, how to play?


There is ramfb-testdev.  Standalone device, for testing purposes.  Also
can listen on vga ports and logs any access, so we can see the bad boys.
Use "qemu -vga none -device ramfb-testdev".  Add "vgalog=on" to watch
guests accessing vga registers.

There is virtio-ramfb.  Simliar to virtio-vga, but using ramfb instead of
adding vga compatibility.  Shows how you can wire up ramfb support to
some display device.  Unlike virtio-vga it should work fine on arm.  Use
"qemu -vga none -device virtio-ramfb" for this one.

Tried to add qxl-ramfb, for windows guest tests, but that doesn't work
yet.  Don't use, unless you want help debugging ;)

There is virtio-pci-ramfb, which provides boot display support to vgpu
devices.

In general using UEFI works better than BIOS, because guests don't
expect legacy vga being present then.

What works?
---

Both windows and linux UEFI guests handle the ramfb GOP just fine.

BIOS boot loaders for linux all use vgabios calls for text mode, so they
show up just fine.  Also ipxe, seabios itself of course.  So you can
boot up your linux guest.  vesafb works too.

What doesn't work?
--

vgacon (direct vga hardware access).  Linux boots just fine
nevertheless, the only effect is that you don't see any boot messages
until the drm driver loads.

Windows in BIOS mode.  Boot logo shows up just fine.  But at some point
windows does lots of vga register accesses (even though it sets the
video mode via vesa bios interface) and appears to be unhappy that
things don't work as expected because there is no vga hardware
emulation.

Known issues


Handover from ramfb-backed efifb to the native linux driver is tricky.
Usually efifb gets kicked out when the native driver loads because of
overlapping ressources.  With efifb being in RAM instead of using a GPU
PCI bar this doesn't happen though, so you'll end up with two
framebuffer devices.

In case vgaarb classifies the GPU as primary display device fbcon will
switch all VTs over to the framebuffer device of the real GPU, so there
isn't a noticable difference.  Otherwise you'll end up with a
non-visible fbcon, because it continues to run on ramfb whereas qemu
switched over to the GPU because the native linux driver initialized the
display.

xorg/wayland will show up on the GPU in any case because they prefer drm
over fbdev, so they wouldn't run on efifb.

Not tested yet
--

ARM.

ramfb -> gpu handover with windows guests (only ramfb-testdev so far).

enjoy,
  Gerd

Gerd Hoffmann (7):
  [testing] update bios, add vgabios-ramfb
  [testing] add ovmf build with ramfb support
  hw/display: add ramfb, a simple boot framebuffer living in guest ram
  hw/display: add ramfb-testdev
  hw/display: add virtio-ramfb
  hw/vfio/display: add ramfb support
  [wip] hw/display: add qxl-ramfb

 hw/display/qxl.h  |   2 +
 include/hw/display/ramfb.h|   8

[Qemu-devel] [PATCH v2 3/7] hw/display: add ramfb, a simple boot framebuffer living in guest ram

[Gerd Hoffmann]

The boot framebuffer is expected to be configured by the firmware, so it
uses fw_cfg as interface.  Initialization goes as follows:

  (1) Check whenever etc/ramfb is present.
  (2) Allocate framebuffer from RAM.
  (3) Fill struct RAMFBCfg, write it to etc/ramfb.

Done.  You can write stuff to the framebuffer now, and it should appear
automagically on the screen.

Note that this isn't very efficient because it does a full display
update on each refresh.  No dirty tracking.  Dirty tracking would have
to be active for the whole ram slot, so that wouldn't be very efficient
either.  So it is *really* intended to be only active for a short time
at boot, before the guest loaded the drivers for the real display
hardware.

This is the ramfb core code.  Some windup is needed for display devices
which want have a ramfb boot display.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/display/ramfb.h |  8 
 hw/display/ramfb.c | 95 ++
 hw/display/Makefile.objs   |  2 +
 3 files changed, 105 insertions(+)
 create mode 100644 include/hw/display/ramfb.h
 create mode 100644 hw/display/ramfb.c

diff --git a/include/hw/display/ramfb.h b/include/hw/display/ramfb.h
new file mode 100644
index 00..f3a772e99e
--- /dev/null
+++ b/include/hw/display/ramfb.h
@@ -0,0 +1,8 @@
+#ifndef RAMFB_H
+#define RAMFB_H
+
+typedef struct RAMFBState RAMFBState;
+void ramfb_display_update(QemuConsole *con, RAMFBState *s);
+RAMFBState *ramfb_setup(Error **errp);
+
+#endif /* RAMFB_H */
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
new file mode 100644
index 00..5425e1feb1
--- /dev/null
+++ b/hw/display/ramfb.c
@@ -0,0 +1,95 @@
+/*
+ * early boot framebuffer in guest ram
+ * configured using fw_cfg
+ *
+ * Copyright Red Hat, Inc. 2017
+ *
+ * Author:
+ * Gerd Hoffmann 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/loader.h"
+#include "hw/display/ramfb.h"
+#include "ui/console.h"
+#include "sysemu/sysemu.h"
+
+struct RAMFBCfg {
+uint64_t addr;
+uint32_t fourcc;
+uint32_t flags;
+uint32_t width;
+uint32_t height;
+uint32_t stride;
+};
+
+struct RAMFBState {
+DisplaySurface *ds;
+uint32_t width, height;
+struct RAMFBCfg cfg;
+};
+
+static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
+{
+RAMFBState *s = dev;
+void *framebuffer;
+uint32_t stride, fourcc, format;
+hwaddr addr, length;
+
+s->width  = be32_to_cpu(s->cfg.width);
+s->height = be32_to_cpu(s->cfg.height);
+stride= be32_to_cpu(s->cfg.stride);
+fourcc= be32_to_cpu(s->cfg.fourcc);
+addr  = be64_to_cpu(s->cfg.addr);
+length= stride * s->height;
+format= qemu_drm_format_to_pixman(fourcc);
+
+fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
+s->width, s->height, addr);
+framebuffer = address_space_map(&address_space_memory,
+addr, &length, false);
+if (!framebuffer || length < stride * s->height) {
+s->width = 0;
+s->height = 0;
+return;
+}
+s->ds = qemu_create_displaysurface_from(s->width, s->height,
+format, stride, framebuffer);
+}
+
+void ramfb_display_update(QemuConsole *con, RAMFBState *s)
+{
+if (!s->width || !s->height) {
+return;
+}
+
+if (s->ds) {
+dpy_gfx_replace_surface(con, s->ds);
+s->ds = NULL;
+}
+
+/* simple full screen update */
+dpy_gfx_update(con, 0, 0, s->width, s->height);
+}
+
+RAMFBState *ramfb_setup(Error **errp)
+{
+FWCfgState *fw_cfg = fw_cfg_find();
+RAMFBState *s;
+
+if (!fw_cfg || !fw_cfg->dma_enabled) {
+error_setg(errp, "ramfb device requires fw_cfg with DMA");
+return NULL;
+}
+
+s = g_new0(RAMFBState, 1);
+
+rom_add_vga("vgabios-ramfb.bin");
+fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
+ NULL, ramfb_fw_cfg_write, s,
+ &s->cfg, sizeof(s->cfg), false);
+return s;
+}
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 3c7c75b94d..fae17fd5ea 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -1,3 +1,5 @@
+common-obj-y += ramfb.o
+
 common-obj-$(CONFIG_ADS7846) += ads7846.o
 common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
 common-obj-$(CONFIG_G364FB) += g364fb.o
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/7] hw/display: add ramfb-testdev

[Gerd Hoffmann]

Standalone device using ramfb.  Intended for testing only.  Offers
optional logging of vga port access, for debugging purposes.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/ramfb-testdev.c | 96 ++
 hw/display/Makefile.objs   |  1 +
 2 files changed, 97 insertions(+)
 create mode 100644 hw/display/ramfb-testdev.c

diff --git a/hw/display/ramfb-testdev.c b/hw/display/ramfb-testdev.c
new file mode 100644
index 00..1e65cf4957
--- /dev/null
+++ b/hw/display/ramfb-testdev.c
@@ -0,0 +1,96 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/loader.h"
+#include "hw/isa/isa.h"
+#include "hw/display/ramfb.h"
+#include "ui/console.h"
+#include "sysemu/sysemu.h"
+
+#define TYPE_RAMFB "ramfb-testdev"
+#define RAMFB(obj) OBJECT_CHECK(ISARAMFBState, (obj), TYPE_RAMFB)
+
+typedef struct ISARAMFBState {
+ISADevice parent_obj;
+QemuConsole *con;
+RAMFBState *state;
+PortioList vga_port_list;
+bool vgalog;
+} ISARAMFBState;
+
+/* log vga port activity, for trouble-shooting purposes */
+static uint32_t vga_log_read(void *opaque, uint32_t addr)
+{
+fprintf(stderr, "%s: port 0x%x\n", __func__, addr);
+return -1;
+}
+
+static void vga_log_write(void *opaque, uint32_t addr, uint32_t val)
+{
+fprintf(stderr, "%s: port 0x%x, value 0x%x\n", __func__, addr, val);
+}
+
+static const MemoryRegionPortio vga_portio_list[] = {
+{ 0x04,  2, 1, .read = vga_log_read, .write = vga_log_write }, /* 3b4 */
+{ 0x0a,  1, 1, .read = vga_log_read, .write = vga_log_write }, /* 3ba */
+{ 0x10, 16, 1, .read = vga_log_read, .write = vga_log_write }, /* 3c0 */
+{ 0x24,  2, 1, .read = vga_log_read, .write = vga_log_write }, /* 3d4 */
+{ 0x2a,  1, 1, .read = vga_log_read, .write = vga_log_write }, /* 3da */
+PORTIO_END_OF_LIST(),
+};
+
+static void display_update_wrapper(void *dev)
+{
+ISARAMFBState *ramfb = RAMFB(dev);
+
+if (0 /* native driver active */) {
+/* non-test device would run native display update here */;
+} else {
+ramfb_display_update(ramfb->con, ramfb->state);
+}
+}
+
+static const GraphicHwOps wrapper_ops = {
+.gfx_update = display_update_wrapper,
+};
+
+static void ramfb_realizefn(DeviceState *dev, Error **errp)
+{
+ISARAMFBState *ramfb = RAMFB(dev);
+
+ramfb->con = graphic_console_init(dev, 0, &wrapper_ops, dev);
+ramfb->state = ramfb_setup(errp);
+
+if (ramfb->vgalog) {
+isa_register_portio_list(ISA_DEVICE(dev), &ramfb->vga_port_list,
+ 0x3b0, vga_portio_list, NULL, "log-vga");
+}
+}
+
+static Property ramfb_properties[] = {
+DEFINE_PROP_BOOL("vgalog", struct ISARAMFBState, vgalog, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ramfb_class_initfn(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+dc->realize = ramfb_realizefn;
+dc->props = ramfb_properties;
+dc->desc = "ram framebuffer test device";
+}
+
+static const TypeInfo ramfb_info = {
+.name  = TYPE_RAMFB,
+.parent= TYPE_ISA_DEVICE,
+.instance_size = sizeof(ISARAMFBState),
+.class_init= ramfb_class_initfn,
+};
+
+static void ramfb_register_types(void)
+{
+type_register_static(&ramfb_info);
+}
+
+type_init(ramfb_register_types)
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index fae17fd5ea..07d2f48782 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -1,4 +1,5 @@
 common-obj-y += ramfb.o
+common-obj-$(CONFIG_VGA_ISA) += ramfb-testdev.o
 
 common-obj-$(CONFIG_ADS7846) += ads7846.o
 common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
-- 
2.9.3

[Qemu-devel] [PATCH v2 1/7] [testing] update bios, add vgabios-ramfb

[Gerd Hoffmann]

source code: https://www.kraxe.org/cgit/seabios/log/?h=ramfb
---
 pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes
 pc-bios/bios.bin  | Bin 131072 -> 131072 bytes
 pc-bios/vgabios-ramfb.bin | Bin 0 -> 28160 bytes
 3 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 pc-bios/vgabios-ramfb.bin

diff --git a/pc-bios/bios-256k.bin b/pc-bios/bios-256k.bin
index 
0061dc99288bee61e60e31d1e24a52b97b0c5fb6..3d279529d6106c8577f4d205cc8d3c532e5f8431
 100644
GIT binary patch
delta 45325
zcmZ_02Ut|c_y2!qD2uwXC`D8dkX1pkfLKsalww20uCahoiHThoD+*>oT~~~KOsrU9
ziHXFhEUV%oDj2(o3AV(VTwDtpW3cZ3eeOcc=llHrKE9TFr<`eL&YYQhcQZ2FGVuQ(
zvwS3`j%4BK+gv26yqYBWSC^y~;8QRY7{F?<3FLqw?vkXgD@k79HMr|5Nv;hfDHyB*
zg+S#eNy*^P07-fV-T)h@-B6NzK_G|$qrgN^El^6A>IF(tGtdrn2aCZ*@Es@xURp^q
zffC@@NRmDWJHZ)HtFa_a2J65kAOqJXl9UJp*a&ujKf$&jN%{$l2$rNt;4HWqoGwY>
zO(iJ{tZs$`ATmUfego>}l5_*~Y9UEwAhjhjfv8Y~fg>O#Op?9=d%-0zrZp-G+``eL
z;Ai01Mv^+Tm85?_^>&g}AAAY61F!VgabgHW)Sz;vV
z6_^qyN$ulNnQoFawmW*Q2MPnF;Ip2nTyJ!1AEX8C`=StV7IaONq@V$kbP@ako`4<$
z(da>vl)eieZ$XbCl5}*aB-I#>F$I0WLEtx1k{W}y-~o6I#*UJtACjO0U}&-=O$Xb6
zYP2M!ft$cIMv^`nD@pHC5H?)(zzOhv
zh9tS`Fs7;K+LA5JRVzqyR7jn80=L6hvoY
z*uW)FKMV0$>5_CB7kw8=(m?PvxCPoRmZV~!T_Q>Opd17nBq<&&1_Cr%DoI_ycrX{_
zfFHnFa0~nmR7Oeq80f$(Vke-e11oJ`5<(MX`5V;ytWjn?R)Z77W23vvS
zPDvUBmVpf*2jqccVEEUV8M`FuJQ%whh3t`}?7fmy@D0*`i(&mvl1751U>*1a+}e-v
zSLI4l9Qf*cOuqw?)Cc?s?t{NUhaXUxgHS&33)pi=lJWlBB6WOVXjAG5$wz
zG2yBtO#^Yipr&9Fm;ub-BnsG3ht(9yoF)Z2-r?JK*BLBscKA6O;15F;E0F&P?(FjX-(?KKcVg
z4HQt5Nw2_mS0-Hr_1&111w3mpX=iOFeFtuXC@&@*_GZ%Wz@aX}>M<#>J}T9KNzMJ3
z)CX(>??EenCdGk2z>+{Fodc7!Owxm2wQQ8Msu7c%8Z)Uns0F;hD-hBInL#*c4Zh(ol5Eq-kS>W!62?q)SFwx)&d=`k=t;Lc87B@oo
zfn(q%sNWb%2uNyz-T}{mV-QRcxCI`AcERWs@HMyxzHBN}T
z0-gX>xFppBO+hTk4#)Vb+n@sA1b6`a+CnwK5pWww?IdXp(7+XG3*y12;0vWW)CLjXDfyqD*)`A>x7?cC=NGKr~
z2R;X@!A4*L^P`|(z^fA$XV4WSf>~fK$OpfIKfym>cxM=Ga3~!gCEzvijK-1xc7XHX
z26zctc0t#{;pz#J!5`o`2#A530hWX7z#&$WJiug-1$Kg?;7`yj4l@kQ0S3@D9>W9D
zKgGvvuo(Q+O_KU`haQ0OJusJn4C?em*8?3`3{HWYz_Axv3kHHzkOe{$AOp+*tHB{~
z75ojHd!xc&I>-cvdSm==;NmH0*hiB3gZW@1I0UNqm81q>7|??~;25|KJQ5`-3`Bqg
zFc?e$m%$(XB`koLq96=J00C};t^?6S;NwAesBW3
zNymr(SV?LH27-cdP@)ug`(P9}2+n~U!0Qu9VxOWLKSO1}MX+lklnWf51ls|w0@uly
zQNRo?g5N>?Y0v?X16*`yEJz0F1^Dnzg{=nD!EW$3@R*4)2Ooh+U<>#aIL|^Wzyn~K
z4Z(BZihxVNVIG`v5DK!vK9C3Q1CKNeAJ{b?$_=i8&;=O(j0Ldy-~zY@DnLUF%VZF~
z5JLj41EtQj$InXl`nhj=w6<{Z5lm*)k`hlK{;14fG8ZZa6Sb}K``hby3
zs32T?4mN=UAjbfc38pSXcY?*)FdX0@r~nbmq4lfKVju%)H3|ZM0?)OW9owK6+ff)e
z3@(C?c0e950@t1B$z3RSH*_H#AI^JFBXAYm1MBvp>%gC&?KhYK`!L5rIav2C>;v%p
z4!$igfpYK$G}(`BB*+3AKrwg+qH|FYSPssC#P4APz*>-g86T|=U^;+Ppym%4QxF68
zfFD8ILr_K#av0l8FcEA5-vKEPGXxZZKY_mq`yQ|#IOe0<0W-sffPSD1#2!H7A!2qSl>%9J3v0T4ua1>FwmXFv^xh~0^fn7
z;2v;4k6k1<3tC>l$_IQdqLN_OMU1~07gxbOP!4Rs{W25@><7PrhoJTq%ze-m^aoqO
zz@N}-;1+lY-hfp0U#ivEfo13kgl;0|yth3yABLGU&7L^?i(f$?ApSP0gDgJ8vV
zEW5z#CKLzcfILtLegb#FQ^0Ov3WJ8AEr1ANrJ
zjjjO$z*Mjg^t^)}0d?;p4s-@Xz%;N6)VYTifVQ9qsB<6bz~^8OaD4zv2>u2&{(usK
zzrbgIVtoOtf#pw(|4+E60M0VzJ4gawf|+HoVpgp8kI=24`(t$J6Id~@0F;1EPq8wV
z!*GF1;Ndgq!E@*U2>uKHB5-&Cg$5fyA(;F(W(ZgfF8q!0Z}<{30L%yV{y_!6ZLp*Q
z)({*5F0WuPfDW{JjcqCLeuG{CXMyWm*nZID9nyf6UJPlJQHlX0!5Q!?xa`QJ7a+u$Nxi^w@IAN-JXK5@2WA2*(70d&
z2K=irsWpfMN!78(0RlV(wXwHKPofV{3MxRXnn|C6bKoj?4F+KIl>+92W#A(4z(%Y+
z=mrwNXixwy0S68COQ1984?YEHU?(tv>e#%+0i7F5mp0S)h5|iU4Yq(kKsl%uh&>-@j_qhM=vRo^Pv%&bxs+Ej
zopr3n-U!Du){*TN(P_+|9TKC{SObrBJ+OCv-TsnMt<`C=2Qfpbtj}AW#y;a*YZ0~u
zETu+!i>suoX1E>|X%SP?nU{}00WwaNfAY}d~y=SwsaQgnmIxv
zWe>AWIODOfaLEYqQ#xx?F9Tu2M->ekGI)UDwBc9zk64s^%INE35e+j~?`}6q>M&;T
zrHiaHsU3_p*ALK0`d99=S}NgmXK*~fk^Xp`--YHdB{DrpXf2N@%^n*EWR5L}{X0liR{qAZIp!thJh6P=I}zg8(uU(ljof;Wne{#hCKARp)-^00bP}V;bpTBqRErC
zzfIIgWv{wInM%;aD8y#WQWra|Kd#gz!&-}iEVDfR4YCwFZL~+CqG`t+U8Oyri$__k
zZ`103+iZsCD9w#7e#`E5+j?R6sHCDLCoB)0G>5M&Db(NbGdiw4*-wmH#DbVwEMLS%
z;?JW+Y$BU1hAn2D-1p-6A>R>Yn&|_=9J-3ViD)
zW_`4Za`dzrYSKk;A(SLLkxjVbvGBwa1Wd4aI6-AbEn$P}|I`x)7M9dT8S^x3rk8Ps
zN5uPC-bx$t#lbvB<|)FyK==#M=L^=9wH9-}V6iMy6nw#&
zv%%ug7p#R>laA;Dm>$XOu5^L^q|5NKG|_4W>x%3%RxT&&i+I9BTe5sromIZ2FI$pZPEXt8i5^J0y~`ju=Tdo3QX
zWb7`a^DQCwWb!uX0l!eKQV
z&7Vhzl-2C1nlxg1;4GJpHP;^(qt~!#zOB31zJ|3-_iF{KZ771O<(H#PW;fJFu8u%W
zoO(B0&hUh2_wPJYLPf8FJS<1{%1eFycM49gLd|YMEW;EL|
zxo_w^vIjUCUKt$9W&gZ^YgsVswJ%vC5jvIp9K&vXT!eaIdyV_MbpN8*fjWJ7QK&^9
zwF3zyoxkDk!q+yNMw-Qns=`$A!JeXI9c$@v7AIteBB*HncpL*-QdRP_o}z|e$M}IB
z;zz-f8h=7^8*1`SnBAOkaQ_tJz!79zor!{A4^)P7xnWPycRj09e<$?YaQeuqOB!^|
zjr_()5qc#%R?FoSv2Z=}YIYtuU&d-1N2dn=H@1BURv!#>V~HECE9rOi6erfRCh5^s
zdX_S(K`vr)#j2NNc$yfZLO=n{nl_g+bh}@^&ZZ|
zOm}F~p#1=CR#gL?gYKDTVuN6+b`)#v(P$s5f-sgrVHF
zZV3XjETZXG%qRR&KUn`aelu&CleZy#Ac-GU_7-JNIIA{%Uyf+Q!WZOoYZJ%N|a4Q
zYe0r#=AlDQ+v&2{Df76PyoogkqxD+OyG=@x+RWgi&8JCi4#9^hlA<=+quk6#(K)&O
z<&8Z=!6r7?Hyq)H)6gt0=&7UJ4_#q+4jI*;31da*W)_f6qRZ|fLXl}WVd_o+SN;%h9#mk)Y(vq>43>EFG3v3uWc{v
zx+yXp0*9B4MeuNFJG_XWP>RyrM0-*9a
zZqIx5ePtE;WoJ;R!NG95U^grb)>bz-%8jIQBsiTt*ij^IVUB*%P29@R*yrQDHr12{
z85$Y`ZI=zD#^H{}M!S>6j4dqMGv@{p-ImRmifDoddelQ+AWF8dh-ONK!>LeocB+?o
z{dVZ+0N7DE3T{-WrEC?3%J2%kaYTe{WdXeWsOY@yX5Csehe{gp|*&px-KTN
zyz&?nqR6UczP}5yuE8Y8$*_7>-+W2{Nw))$tASyGi{ZJc$;S}$5P7;;kLc%llZ71=
zEt=%8*6KAhUAmCW7jYsbhxLo~K}

[Qemu-devel] [PATCH v2 7/7] [wip] hw/display: add qxl-ramfb

[Gerd Hoffmann]

Like qxl-vga, but using ramfb instead of legacy vga.

NOT WORKING YET.

Also: Hackish proof-of-concept, with debug msgs.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/qxl.h   |  2 ++
 hw/display/qxl.c   | 47 +++
 ui/spice-display.c |  6 ++
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 089696ef62..7bfdf429bf 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -5,6 +5,7 @@
 
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
+#include "hw/display/ramfb.h"
 #include "vga_int.h"
 #include "qemu/thread.h"
 
@@ -31,6 +32,7 @@ enum qxl_mode {
 
 typedef struct PCIQXLDevice {
 PCIDevice  pci;
+RAMFBState *ramfb;
 PortioList vga_port_list;
 SimpleSpiceDisplay ssd;
 intid;
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index a71714ccb4..ebe71fbcf5 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1893,7 +1893,12 @@ static void qxl_hw_update(void *opaque)
 {
 PCIQXLDevice *qxl = opaque;
 
-qxl_render_update(qxl);
+if (qxl->mode == QXL_MODE_UNDEFINED &&
+qxl->ramfb != NULL) {
+ramfb_display_update(qxl->vga.con, qxl->ramfb);
+} else {
+qxl_render_update(qxl);
+}
 }
 
 static void qxl_dirty_one_surface(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
@@ -1968,7 +1973,8 @@ static void display_update(DisplayChangeListener *dcl,
 {
 PCIQXLDevice *qxl = container_of(dcl, PCIQXLDevice, ssd.dcl);
 
-if (qxl->mode == QXL_MODE_VGA) {
+if (qxl->mode == QXL_MODE_VGA ||
+(qxl->mode == QXL_MODE_UNDEFINED && qxl->ramfb)) {
 qemu_spice_display_update(&qxl->ssd, x, y, w, h);
 }
 }
@@ -1979,7 +1985,9 @@ static void display_switch(DisplayChangeListener *dcl,
 PCIQXLDevice *qxl = container_of(dcl, PCIQXLDevice, ssd.dcl);
 
 qxl->ssd.ds = surface;
-if (qxl->mode == QXL_MODE_VGA) {
+if (qxl->mode == QXL_MODE_VGA ||
+(qxl->mode == QXL_MODE_UNDEFINED && qxl->ramfb)) {
+fprintf(stderr, "%s: !native\n", __func__);
 qemu_spice_display_switch(&qxl->ssd, surface);
 }
 }
@@ -1988,7 +1996,8 @@ static void display_refresh(DisplayChangeListener *dcl)
 {
 PCIQXLDevice *qxl = container_of(dcl, PCIQXLDevice, ssd.dcl);
 
-if (qxl->mode == QXL_MODE_VGA) {
+if (qxl->mode == QXL_MODE_VGA ||
+(qxl->mode == QXL_MODE_UNDEFINED && qxl->ramfb)) {
 qemu_spice_display_refresh(&qxl->ssd);
 }
 }
@@ -2204,6 +2213,19 @@ static void qxl_realize_secondary(PCIDevice *dev, Error 
**errp)
 qxl_realize_common(qxl, errp);
 }
 
+static void qxl_realize_ramfb(PCIDevice *dev, Error **errp)
+{
+PCIQXLDevice *qxl = PCI_QXL(dev);
+
+qxl_realize_secondary(dev, errp);
+qxl_hard_reset(qxl, 0);
+
+qxl->ramfb = ramfb_setup(errp);
+qxl->ssd.dcl.ops = &display_listener_ops;
+qxl->ssd.dcl.con = qxl->vga.con;
+register_displaychangelistener(&qxl->ssd.dcl);
+}
+
 static int qxl_pre_save(void *opaque)
 {
 PCIQXLDevice* d = opaque;
@@ -2472,11 +2494,28 @@ static const TypeInfo qxl_secondary_info = {
 .class_init= qxl_secondary_class_init,
 };
 
+static void qxl_ramfb_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+k->realize = qxl_realize_ramfb;
+k->class_id = PCI_CLASS_DISPLAY_OTHER;
+dc->desc = "Spice QXL GPU (ramfb)";
+}
+
+static const TypeInfo qxl_ramfb_info = {
+.name  = "qxl-ramfb",
+.parent= TYPE_PCI_QXL,
+.class_init= qxl_ramfb_class_init,
+};
+
 static void qxl_register_types(void)
 {
 type_register_static(&qxl_pci_type_info);
 type_register_static(&qxl_primary_info);
 type_register_static(&qxl_secondary_info);
+type_register_static(&qxl_ramfb_info);
 }
 
 type_init(qxl_register_types)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index fe734821dd..c8082b0649 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -86,6 +86,9 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay 
*ssd, uint32_t id,
QXLDevSurfaceCreate *surface,
qxl_async_io async)
 {
+fprintf(stderr, "%s: %dx%d, %lx, %s\n", __func__,
+surface->width, surface->height, (unsigned long)surface->mem,
+(surface->group_id == MEMSLOT_GROUP_HOST) ? "host" : "guest");
 trace_qemu_spice_create_primary_surface(ssd->qxl.id, id, surface, async);
 if (async != QXL_SYNC) {
 spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface,
@@ -310,6 +313,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
 {
 QXLDevMemSlot memslot;
 
+fprintf(stderr, "%s:\n", __func__);
 memset(&memslot, 0, sizeof(memslot));
 memslot.slot_group_id = MEMSLOT_GROUP_HOST;
 memslot.virt_end = ~0;
@@ -343,6 +347,8 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
 

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Peter Xu]

On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
> As Max Reitz said, this breaks several qemu iotests. I can reproduce that on 
> s390
> e.g. with ./check -qcow2 030 in the qemu-iotest.
> 
> Why was this merged?

My fault.  Sorry.  I should have done iotests before submission.

Now I'm trying to fix those iotest issues.

I'm not extremely familiar with block, so the progress is a bit slow,
but I'll do it asap (though I'll possibly need some help from the
block team, since there are some not-easy ones for me).

-- 
Peter Xu

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Thomas Huth]

On 23.03.2018 10:56, Peter Maydell wrote:
> On 23 March 2018 at 08:36, Thomas Huth  wrote:
>> Most of the RISC-V boards currently crash when they are started with
>> "-nodefaults", e.g.:
>>
>> $ gdb --args riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e
>> [...]
>> (gdb) r
>> Program received signal SIGSEGV, Segmentation fault.
>> qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
>> at chardev/char-fe.c:210
>> 210 } else if (s->be) {
>> (gdb) bt
>>  0  0x558695f8 in qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
>> at chardev/char-fe.c:210
>>  1  0x556fb425 in sifive_uart_create ([...], chr=0x0, [...])
>> at hw/riscv/sifive_uart.c:169
>>  2  0x556f8cc4 in riscv_sifive_e_init (machine=[...])
>> at hw/riscv/sifive_e.c:164
>> [...]
>>
>> With "-nodefaults" there are no entries in serial_hds[], so 
>> qemu_chr_fe_init()
>> finally tries to dereference a NULL pointer. Let's fix this problem by
>> creating a "null" chardev in this case instead.
> 
> Isn't it possible to fix this another level further down
> by having qemu_chr_fe_init() &c handle having a NULL chardev?

Not sure, ... I don't think that we should create a "null" device in
chardev/char-fe.c - that would sound like a layer violation to me.

So all we could do there is to set an error in "errp" or to simply
"ignore" the NULL pointer (so that b->chr simply gets set to NULL here).

In the first case, we'd run into abort()s instead, since lots of caller
call this function with errp = &error_abort. So that does also not sound
very user-friendly (unless we change the error_aborts into error_fatals
maybe).

In the second case, QEMU seems either bails out later in serial_mm_init
(when calling serial_realize_core()) with:

$ riscv32-softmmu/qemu-system-riscv32 -nodefaults -M virt
qemu-system-riscv32: Can't create serial device, empty char device

... or it seems to start in case of the other riscv machines. Not sure
whether we can trust all the remaining functions to deal correctly with
a NULL pointer in b->chr here though... I guess we could try and find
out later where there are other crashes in that case.

Opinions?

> That would be nicer than requiring every UART model to handle
> it, because inevitably people will forget about that corner case.

At least we need some more automatic qtest coverage here... (e.g.
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05033.html)

 Thomas

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Auger Eric]

Hi,

On 23/03/18 13:11, Peter Maydell wrote:
> On 23 March 2018 at 12:01, Auger Eric  wrote:
>> Hi,
>>
>> On 23/03/18 11:26, Peter Maydell wrote:
>>> On 23 March 2018 at 10:24, Auger Eric  wrote:
 Hi,

 I observe a regression on KVM accelerated qemu-system-aarch64:

 Unexpected error in kvm_device_access() at
 /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
 failed: Group 6 attr 0xc664: Device or resource busy
 2018-03-23 10:00:00.085+: shutting down, reason=crashed
>>>
>>> Can you get a backtrace for this? (I guess you'd need to fiddle
>>> with the kvm_device_access() code to make it assert rather
>>> than passing back the error).
>>
>> OK. I will try to do so. As I could have expected, I cannot reproduce on
>> a standalone qemu command line. The problem observed above is seen with
>> libvirt launch which may be doing some other QMP stuff concurrently?
> 
> Hmm, that could be a bit painful to debug. I dunno if libvirt
> has a "launch QEMU under gdb" option. If not, you could try
> something like:
>if (condition we want to get a backtrace on) {
>printf("hit condition, attach gdb to process %d\n", (int)getpid());
>for (;;) { }
>}

Thanks for the hint. Here is the stack I get.

#0  kvm_device_access (fd=31, group=6, attr=50788, val=0x5937c88, write=false, 
errp=0x16984a8 ) at 
/home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164
#1  0x004f8ce4 in arm_gicv3_icc_reset (env=0xa1fc8330, 
ri=0x597f910) at /home/augere/UPSTREAM/qemu/hw/intc/arm_gicv3_kvm.c:632
#2  0x006351ac in cp_reg_reset (key=0x597f730, value=0x597f910, 
opaque=0xa1fc0010) at /home/augere/UPSTREAM/qemu/target/arm/cpu.c:78
#3  0xa47edce4 in g_hash_table_foreach () from /lib64/libglib-2.0.so.0
#4  0x00635394 in arm_cpu_reset (s=0xa1fc0010) at 
/home/augere/UPSTREAM/qemu/target/arm/cpu.c:130
#5  0x0090c888 in cpu_reset (cpu=0xa1fc0010) at qom/cpu.c:249
#6  0x005793d8 in do_cpu_reset (opaque=0xa1fc0010) at 
/home/augere/UPSTREAM/qemu/hw/arm/boot.c:665
#7  0x0073095c in qemu_devices_reset () at hw/core/reset.c:69
#8  0x006976e0 in qemu_system_reset (reason=SHUTDOWN_CAUSE_NONE) at 
vl.c:1731
#9  0x0069fd60 in main (argc=69, argv=0xe877d1a8, 
envp=0xe877d3d8) at vl.c:4697

Thanks

Eric
> 
> and then QEMU will sit in a loop waiting for you to do a
>   gdb path/to/qemu 
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Michael Tokarev]

22.03.2018 19:12, Laurent Vivier wrote:
> I've re-run some scripts from the coccinelle directory,
> and they have found some problems.
> 
> This series fixes them.
> 
> Laurent Vivier (4):
>   error: Strip trailing '\n' from error string arguments (again again)
>   error: Remove NULL checks on error_propagate() calls
>   qdict: remove useless cast
>   Remove unnecessary variables for function return value

I've applied patches 1-3, but the 4th patch is a bit.. interesting.
As has already been said, it touches auto-generated files, which is
sort of fine, but it _also_ removes comments, which - I think - is
quite a bit wrong.

I can apply just selected hunks, but.. hmm..

/mjt

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Laurent Vivier]

On 23/03/2018 13:37, Michael Tokarev wrote:
> 22.03.2018 19:12, Laurent Vivier wrote:
>> I've re-run some scripts from the coccinelle directory,
>> and they have found some problems.
>>
>> This series fixes them.
>>
>> Laurent Vivier (4):
>>   error: Strip trailing '\n' from error string arguments (again again)
>>   error: Remove NULL checks on error_propagate() calls
>>   qdict: remove useless cast
>>   Remove unnecessary variables for function return value
> 
> I've applied patches 1-3, but the 4th patch is a bit.. interesting.
> As has already been said, it touches auto-generated files, which is
> sort of fine, but it _also_ removes comments, which - I think - is
> quite a bit wrong.
> 
> I can apply just selected hunks, but.. hmm..

Thank you, I'm going to rework this one.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Christian Borntraeger]

On 03/23/2018 01:25 PM, Peter Xu wrote:
> On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
>> As Max Reitz said, this breaks several qemu iotests. I can reproduce that on 
>> s390
>> e.g. with ./check -qcow2 030 in the qemu-iotest.
>>
>> Why was this merged?
> 
> My fault.  Sorry.  I should have done iotests before submission.
> 
> Now I'm trying to fix those iotest issues.
> 
> I'm not extremely familiar with block, so the progress is a bit slow,
> but I'll do it asap (though I'll possibly need some help from the
> block team, since there are some not-easy ones for me).

As this patch also seems to trigger something else for Eric, can we revert this
patch for the time being and re-apply as soon as we have solutions for the 
issues?

After all a hard hang in the iotest can break CI environments (as you run into 
timeouts).

[Qemu-devel] [PATCH] vhost: Allow abutting regions

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

My rework of section adding combines overlapping or abutting regions,
but checks they're actually the same underlying RAM block.
Fix the case where two blocks abut but don't overlap; that new region
should get added (but not combined), but my previous patch was disallowing it.

Fixes: c1ece84e7c9

Reported-by: Alex Williamson 
Signed-off-by: Dr. David Alan Gilbert 
---
 hw/virtio/vhost.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 250f886acb..fc9062a89f 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -595,10 +595,15 @@ static void vhost_region_add_section(struct vhost_dev 
*dev,
 prev_sec->offset_within_address_space,
 prev_sec->offset_within_region);
 } else {
-error_report("%s: Overlapping but not coherent sections "
- "at %"PRIx64,
- __func__, mrs_gpa);
-return;
+/* abutting regions are fine, but overlapping ones with
+ * different blocks/offsets shouldn't happen
+ */
+if (mrs_gpa != prev_gpa_end + 1) {
+error_report("%s: Overlapping but not coherent sections "
+ "at %"PRIx64,
+ __func__, mrs_gpa);
+return;
+}
 }
 }
 }
-- 
2.14.3

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Auger Eric]

Hi Peter,

On 23/03/18 13:19, Peter Xu wrote:
> On Fri, Mar 23, 2018 at 01:01:43PM +0100, Auger Eric wrote:
>> Hi,
>>
>> On 23/03/18 11:26, Peter Maydell wrote:
>>> On 23 March 2018 at 10:24, Auger Eric  wrote:
 Hi,

 I observe a regression on KVM accelerated qemu-system-aarch64:

 Unexpected error in kvm_device_access() at
 /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
 2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
 failed: Group 6 attr 0xc664: Device or resource busy
 2018-03-23 10:00:00.085+: shutting down, reason=crashed
>>>
>>> Can you get a backtrace for this? (I guess you'd need to fiddle
>>> with the kvm_device_access() code to make it assert rather
>>> than passing back the error).
>>
>> OK. I will try to do so. As I could have expected, I cannot reproduce on
>> a standalone qemu command line. The problem observed above is seen with
>> libvirt launch which may be doing some other QMP stuff concurrently?
> 
> Maybe not concurrently inside libvirt, but after the patches AFAIK the
> QMP part can be concurrent with something else or break some timing.
> 
> Anyway, sorry for that!  I'll see what I can do to help.

No worries

Thanks

Eric
> 

Re: [Qemu-devel] Regression on KVM qemu-system-aarch64 since "monitor: enable IO thread for (qmp & !mux) typed"

[Peter Xu]

On Fri, Mar 23, 2018 at 01:36:36PM +0100, Auger Eric wrote:
> Hi,
> 
> On 23/03/18 13:11, Peter Maydell wrote:
> > On 23 March 2018 at 12:01, Auger Eric  wrote:
> >> Hi,
> >>
> >> On 23/03/18 11:26, Peter Maydell wrote:
> >>> On 23 March 2018 at 10:24, Auger Eric  wrote:
>  Hi,
> 
>  I observe a regression on KVM accelerated qemu-system-aarch64:
> 
>  Unexpected error in kvm_device_access() at
>  /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164:
>  2018-03-23T09:59:59.629439Z qemu-system-aarch64: KVM_GET_DEVICE_ATTR
>  failed: Group 6 attr 0xc664: Device or resource busy
>  2018-03-23 10:00:00.085+: shutting down, reason=crashed
> >>>
> >>> Can you get a backtrace for this? (I guess you'd need to fiddle
> >>> with the kvm_device_access() code to make it assert rather
> >>> than passing back the error).
> >>
> >> OK. I will try to do so. As I could have expected, I cannot reproduce on
> >> a standalone qemu command line. The problem observed above is seen with
> >> libvirt launch which may be doing some other QMP stuff concurrently?
> > 
> > Hmm, that could be a bit painful to debug. I dunno if libvirt
> > has a "launch QEMU under gdb" option. If not, you could try
> > something like:
> >if (condition we want to get a backtrace on) {
> >printf("hit condition, attach gdb to process %d\n", (int)getpid());
> >for (;;) { }
> >}
> 
> Thanks for the hint. Here is the stack I get.
> 
> #0  kvm_device_access (fd=31, group=6, attr=50788, val=0x5937c88, 
> write=false, errp=0x16984a8 ) at 
> /home/augere/UPSTREAM/qemu/accel/kvm/kvm-all.c:2164
> #1  0x004f8ce4 in arm_gicv3_icc_reset (env=0xa1fc8330, 
> ri=0x597f910) at /home/augere/UPSTREAM/qemu/hw/intc/arm_gicv3_kvm.c:632
> #2  0x006351ac in cp_reg_reset (key=0x597f730, value=0x597f910, 
> opaque=0xa1fc0010) at /home/augere/UPSTREAM/qemu/target/arm/cpu.c:78
> #3  0xa47edce4 in g_hash_table_foreach () from /lib64/libglib-2.0.so.0
> #4  0x00635394 in arm_cpu_reset (s=0xa1fc0010) at 
> /home/augere/UPSTREAM/qemu/target/arm/cpu.c:130
> #5  0x0090c888 in cpu_reset (cpu=0xa1fc0010) at qom/cpu.c:249
> #6  0x005793d8 in do_cpu_reset (opaque=0xa1fc0010) at 
> /home/augere/UPSTREAM/qemu/hw/arm/boot.c:665
> #7  0x0073095c in qemu_devices_reset () at hw/core/reset.c:69
> #8  0x006976e0 in qemu_system_reset (reason=SHUTDOWN_CAUSE_NONE) at 
> vl.c:1731
> #9  0x0069fd60 in main (argc=69, argv=0xe877d1a8, 
> envp=0xe877d3d8) at vl.c:4697

I'm not sure, but I suspect libvirt has already sent something to QEMU
before this point, which breaks the timing of the old QMP code (the
old QMP won't do anything until main loop).

I'm thinking maybe I should postpone the monitors at least until main
loop so that we can still keep that assumption.  But I'll see what do
others think of it first.

Thanks,

-- 
Peter Xu

[Qemu-devel] [PATCH for-2.12 2/2] i386/hyperv: error out if features requested but unsupported

[Roman Kagan]

In order to guarantee compatibility on migration, QEMU should have
complete control over the features it announces to the guest via CPUID.

However, for a number of Hyper-V-related cpu properties, if the
corresponding feature is not supported by the underlying KVM, the
propery is silently ignored and the feature is not announced to the
guest.

Refuse to start with an error instead.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Roman Kagan 
---
 target/i386/kvm.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index fb20ff18c2..c9c359241c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -658,17 +658,34 @@ static int hyperv_handle_properties(CPUState *cs)
 env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
 env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
 }
-if (cpu->hyperv_crash && has_msr_hv_crash) {
+if (cpu->hyperv_crash) {
+if (!has_msr_hv_crash) {
+fprintf(stderr,
+"Hyper-V crash MSRs are not supported by kernel\n");
+return -ENOSYS;
+}
 env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
 }
 env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
-if (cpu->hyperv_reset && has_msr_hv_reset) {
+if (cpu->hyperv_reset) {
+   if (!has_msr_hv_reset) {
+fprintf(stderr, "Hyper-V reset MSR is not supported by kernel\n");
+return -ENOSYS;
+}
 env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
 }
-if (cpu->hyperv_vpindex && has_msr_hv_vpindex) {
+if (cpu->hyperv_vpindex) {
+if (!has_msr_hv_vpindex) {
+fprintf(stderr, "Hyper-V VP_INDEX is not supported by kernel\n");
+return -ENOSYS;
+}
 env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
 }
-if (cpu->hyperv_runtime && has_msr_hv_runtime) {
+if (cpu->hyperv_runtime) {
+if (!has_msr_hv_runtime) {
+fprintf(stderr, "Hyper-V VP_INDEX is not supported by kernel\n");
+return -ENOSYS;
+}
 env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
 }
 if (cpu->hyperv_synic) {
-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 0/2] i386/hyperv: fully control Hyper-V features in CPUID

[Roman Kagan]

In order to guarantee compatibility on migration, QEMU should have
complete control over the features it announces to the guest via CPUID.

However, a number of Hyper-V-related features happen to depend on the
support in the underlying KVM, with no regard to QEMU configuration.

Make QEMU regain control over what Hyper-V features it announces to the
guest.

Note: the patches are also being proposed for stable-2.11, even though
one of them introduces a new cpu property.  This is done to minimize the
number of published QEMU releases where the behavior of the features is
unpredictable, with potentially fatal consequences for the guest.

Note #2: there are other problems in the surrounding code, like ugly
error reporting or inconsistent population of MSRs.  I think this can be
put off to post-2.12.

Roman Kagan (2):
  i386/hyperv: add hv-frequencies cpu property
  i386/hyperv: error out if features requested but unsupported

Cc: qemu-sta...@nongnu.org

 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm.c | 37 +
 3 files changed, 31 insertions(+), 8 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 1/2] i386/hyperv: add hv-frequencies cpu property

[Roman Kagan]

In order to guarantee compatibility on migration, QEMU should have
complete control over the features it announces to the guest via CPUID.

However, the declared availability of Hyper-V frequency MSRs
(HV_X64_MSR_TSC_FREQUENCY and HV_X64_MSR_APIC_FREQUENCY) depends solely
on the support for them in the underlying KVM.

Introduce "hv-frequencies" cpu property (off by default) which gives
QEMU full control over whether these MSRs are announced.

While at this, drop the redundant check of the cpu tsc frequency, and
decouple this feature from hv-time.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Roman Kagan 
---
Note: this patch introduces a new cpu property, which is not what we
normally do in stable branches.  However, this appears to be the minimal
effort/churn approach to reduce the number of published QEMU releases
where the behavior of the feature is unpredictable, with potentially
fatal consequences for the guest.

 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm.c | 12 
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 78db1b833a..1b219fafc4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1296,6 +1296,7 @@ struct X86CPU {
 bool hyperv_runtime;
 bool hyperv_synic;
 bool hyperv_stimer;
+bool hyperv_frequencies;
 bool check_cpuid;
 bool enforce_cpuid;
 bool expose_kvm;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 555ae79d29..1a6b082b6f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4761,6 +4761,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
 DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
 DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
+DEFINE_PROP_BOOL("hv-frequencies", X86CPU, hyperv_frequencies, false),
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index d23fff12f5..fb20ff18c2 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -648,11 +648,15 @@ static int hyperv_handle_properties(CPUState *cs)
 env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
 env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
 env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
-
-if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
-env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
-env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
+}
+if (cpu->hyperv_frequencies) {
+if (!has_msr_hv_frequencies) {
+fprintf(stderr,
+"Hyper-V frequency MSRs are not supported by kernel\n");
+return -ENOSYS;
 }
+env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
+env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
 }
 if (cpu->hyperv_crash && has_msr_hv_crash) {
 env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
-- 
2.14.3

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Peter Xu]

On Fri, Mar 23, 2018 at 01:44:54PM +0100, Christian Borntraeger wrote:
> 
> 
> On 03/23/2018 01:25 PM, Peter Xu wrote:
> > On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
> >> As Max Reitz said, this breaks several qemu iotests. I can reproduce that 
> >> on s390
> >> e.g. with ./check -qcow2 030 in the qemu-iotest.
> >>
> >> Why was this merged?
> > 
> > My fault.  Sorry.  I should have done iotests before submission.
> > 
> > Now I'm trying to fix those iotest issues.
> > 
> > I'm not extremely familiar with block, so the progress is a bit slow,
> > but I'll do it asap (though I'll possibly need some help from the
> > block team, since there are some not-easy ones for me).
> 
> As this patch also seems to trigger something else for Eric, can we revert 
> this
> patch for the time being and re-apply as soon as we have solutions for the 
> issues?
> 
> After all a hard hang in the iotest can break CI environments (as you run 
> into timeouts).

I'm fine with it if it helps.

Then I can work in background to solved existing problems, then we
turn it on again in 2.13 when ready (but still it might break things -
at least it can be during dev cycle rather than softfreeze).

-- 
Peter Xu

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Peter Maydell]

On 23 March 2018 at 12:31, Thomas Huth  wrote:
> On 23.03.2018 10:56, Peter Maydell wrote:
>> On 23 March 2018 at 08:36, Thomas Huth  wrote:
>>> Most of the RISC-V boards currently crash when they are started with
>>> "-nodefaults", e.g.:
>>>
>>> $ gdb --args riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e
>>> [...]
>>> (gdb) r
>>> Program received signal SIGSEGV, Segmentation fault.
>>> qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
>>> at chardev/char-fe.c:210
>>> 210 } else if (s->be) {
>>> (gdb) bt
>>>  0  0x558695f8 in qemu_chr_fe_init ([...], s=s@entry=0x0, [...])
>>> at chardev/char-fe.c:210
>>>  1  0x556fb425 in sifive_uart_create ([...], chr=0x0, [...])
>>> at hw/riscv/sifive_uart.c:169
>>>  2  0x556f8cc4 in riscv_sifive_e_init (machine=[...])
>>> at hw/riscv/sifive_e.c:164
>>> [...]
>>>
>>> With "-nodefaults" there are no entries in serial_hds[], so 
>>> qemu_chr_fe_init()
>>> finally tries to dereference a NULL pointer. Let's fix this problem by
>>> creating a "null" chardev in this case instead.
>>
>> Isn't it possible to fix this another level further down
>> by having qemu_chr_fe_init() &c handle having a NULL chardev?
>
> Not sure, ... I don't think that we should create a "null" device in
> chardev/char-fe.c - that would sound like a layer violation to me.

It already does treat a NULL Chardev* as being "do nothing"
most of the time: eg qemu_chr_fe_write(), qemu_chr_fe_read_all(),
qemu_chr_fe_ioctl(), qemu_chr_fe_set_handlers() &c all have
"if be->chr is NULL, do nothing" checks.

> So all we could do there is to set an error in "errp" or to simply
> "ignore" the NULL pointer (so that b->chr simply gets set to NULL here).

I think that the latter seems to be what the design intends:
a backend with a NULL chardev pointer is "backend with no
chardev attached at the moment". At least some of our UART devices
handle that as "just dump output into nothingness", which
seems logical to me.

Cc'ing Marc-André and Paolo as the chardev maintainers --
do you have a plan for what the intended design here is?

thanks
-- PMM

[Qemu-devel] [PATCH] target/xtensa: fix timers test

[Max Filippov]

Change frequency of the core used in tests so that clock cycle takes
exactly 64ns. Change icount power used in tests to 6, so that each
instruction takes exactly 1 clock cycle. With these changes the
assumptions of the xtensa timers test are correct and the test must
always pass.

Longer story:
  http://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04326.html

Cc: Pavel Dovgaluk 
Signed-off-by: Max Filippov 
---
 target/xtensa/core-dc232b.c | 2 +-
 tests/tcg/xtensa/Makefile   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/xtensa/core-dc232b.c b/target/xtensa/core-dc232b.c
index aa07018af4e7..8846336f5cfa 100644
--- a/target/xtensa/core-dc232b.c
+++ b/target/xtensa/core-dc232b.c
@@ -47,7 +47,7 @@ static XtensaConfig dc232b __attribute__((unused)) = {
 }
 },
 .isa_internal = &xtensa_modules,
-.clock_freq_khz = 1,
+.clock_freq_khz = 15625,
 DEFAULT_SECTIONS
 };
 
diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile
index 2882c431e4a9..091518c05583 100644
--- a/tests/tcg/xtensa/Makefile
+++ b/tests/tcg/xtensa/Makefile
@@ -5,7 +5,7 @@ CROSS=xtensa-$(CORE)-elf-
 
 ifndef XT
 SIM = ../../../xtensa-softmmu/qemu-system-xtensa
-SIMFLAGS = -M sim -cpu $(CORE) -nographic -semihosting -icount 7 $(EXTFLAGS) 
-kernel
+SIMFLAGS = -M sim -cpu $(CORE) -nographic -semihosting -icount 6 $(EXTFLAGS) 
-kernel
 SIMDEBUG = -s -S
 else
 SIM = xt-run
-- 
2.11.0

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Peter Maydell]

On 23 March 2018 at 13:01, Peter Xu  wrote:
> On Fri, Mar 23, 2018 at 01:44:54PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 03/23/2018 01:25 PM, Peter Xu wrote:
>> > On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
>> >> As Max Reitz said, this breaks several qemu iotests. I can reproduce that 
>> >> on s390
>> >> e.g. with ./check -qcow2 030 in the qemu-iotest.
>> >>
>> >> Why was this merged?
>> >
>> > My fault.  Sorry.  I should have done iotests before submission.
>> >
>> > Now I'm trying to fix those iotest issues.
>> >
>> > I'm not extremely familiar with block, so the progress is a bit slow,
>> > but I'll do it asap (though I'll possibly need some help from the
>> > block team, since there are some not-easy ones for me).
>>
>> As this patch also seems to trigger something else for Eric, can we revert 
>> this
>> patch for the time being and re-apply as soon as we have solutions for the 
>> issues?
>>
>> After all a hard hang in the iotest can break CI environments (as you run 
>> into timeouts).
>
> I'm fine with it if it helps.

OK. I'm running a revert of commit 3fd2457d18edf5736f713dfe1ada9c87a9badab1
through my buildtests and will push it to master assuming it passes.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Christian Borntraeger]

On 03/23/2018 02:01 PM, Peter Xu wrote:
> On Fri, Mar 23, 2018 at 01:44:54PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 03/23/2018 01:25 PM, Peter Xu wrote:
>>> On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
 As Max Reitz said, this breaks several qemu iotests. I can reproduce that 
 on s390
 e.g. with ./check -qcow2 030 in the qemu-iotest.

 Why was this merged?
>>>
>>> My fault.  Sorry.  I should have done iotests before submission.
>>>
>>> Now I'm trying to fix those iotest issues.
>>>
>>> I'm not extremely familiar with block, so the progress is a bit slow,
>>> but I'll do it asap (though I'll possibly need some help from the
>>> block team, since there are some not-easy ones for me).
>>
>> As this patch also seems to trigger something else for Eric, can we revert 
>> this
>> patch for the time being and re-apply as soon as we have solutions for the 
>> issues?
>>
>> After all a hard hang in the iotest can break CI environments (as you run 
>> into timeouts).
> 
> I'm fine with it if it helps.
> 
> Then I can work in background to solved existing problems, then we
> turn it on again in 2.13 when ready (but still it might break things -
> at least it can be during dev cycle rather than softfreeze).

Strangely enough, reverting 3fd2457d18edf5736f713dfe1
now causes make check errors... :-/


  GTESTER check-qtest-s390x
  GTESTER check-qtest-i386
**
ERROR:/home/cborntra/REPOS/qemu/tests/qmp-test.c:97:test_qmp_protocol: 
assertion failed: (entry)
GTester: last random seed: R02S1cbb7e297cc349b33ae39f10dd92764a
**
ERROR:/home/cborntra/REPOS/qemu/tests/qmp-test.c:190:test_qmp_oob: assertion 
failed: (qdict_haskey(resp, "return"))
GTester: last random seed: R02S7afc5ab63f6f29401d702dceb1f8030b
make: *** [/home/cborntra/REPOS/qemu/tests/Makefile.include:880: 
check-qtest-s390x] Error 1
make: *** Waiting for unfinished jobs
^Cmake: *** [/home/cborntra/REPOS/qemu/tests/Makefile.include:880: 
check-qtest-i386] Interrupt


Do we have patches depending on this change?

Re: [Qemu-devel] [PATCH v2 0/7] ramfb: simple boot framebuffer, no legacy vga

[Laszlo Ersek]

Adding Ard and Marc, and keeping the context undisturbed for his sake.
Comments at the bottom.

On 03/23/18 13:25, Gerd Hoffmann wrote:
>   Hi,
> 
> Ok folks, here is a experimental patch series for a legacy free boot
> framebuffer.  If you want play with it I recommend getting the bits from
> 
>   https://www.kraxel.org/cgit/qemu/log/?h=sirius/ramfb
> 
> because they come with an updated seabios and a new vgabios rom and an
> experimental OVMF build.
> 
> Functional overview
> ---
> 
> The boot framebuffer is expected to be configured by the firmware, so it
> uses fw_cfg as interface.  Initialization goes as follows:
> 
>   (1) Check whenever etc/ramfb is present.
>   (2) Allocate framebuffer from RAM.
>   (3) Fill struct RAMFBCfg, write it to etc/ramfb.
> 
> Done.  You can write stuff to the framebuffer now, and it should appear
> automagically on the screen.
> 
> Note that this isn't very efficient because it does a full display
> update on each refresh.  No dirty tracking.  Dirty tracking would have
> to be active for the whole ram slot, so that wouldn't be very efficient
> either.  So it is *really* intended to be only active for a short time
> at boot, before the guest loaded the drivers for the real display
> hardware.
> 
> Firmware support -- seabios
> ---
> 
> seavgabios is able to emulate vga text mode on top of a framebuffer, for
> coreboot native graphics initialialization.  Which works fine for
> everything which writes text using the vgabios interface (basically
> everyhing which works with sgabios).
> 
> So I hacked that up to work with ramfb.  Right now it's proof-of-concept
> code with too much cut+paste, so it will clearly need a bunch of
> cleanups if this approach turns out to be workable.  Look here:
> 
>   https://www.kraxe.org/cgit/seabios/log/?h=ramfb
> 
> Firmware support -- edk2
> 
> 
> There is a EFI driver too.  Likewise a hackish proof-of-concept thing,
> clearly not in a mergeable state, but good enough for playing.  Note
> that the build disables QemuVideoDxe and VirtoGpu drivers, so ramfb is
> the only supported display.  Code is here:
> 
>   https://github.com/kraxel/edk2/commits/ramfb
> 
> Firmware blob is in pc-bios/OVMF-ramfb.fd, to be used with -bios.
> 
> So, how to play?
> 
> 
> There is ramfb-testdev.  Standalone device, for testing purposes.  Also
> can listen on vga ports and logs any access, so we can see the bad boys.
> Use "qemu -vga none -device ramfb-testdev".  Add "vgalog=on" to watch
> guests accessing vga registers.
> 
> There is virtio-ramfb.  Simliar to virtio-vga, but using ramfb instead of
> adding vga compatibility.  Shows how you can wire up ramfb support to
> some display device.  Unlike virtio-vga it should work fine on arm.  Use
> "qemu -vga none -device virtio-ramfb" for this one.
> 
> Tried to add qxl-ramfb, for windows guest tests, but that doesn't work
> yet.  Don't use, unless you want help debugging ;)
> 
> There is virtio-pci-ramfb, which provides boot display support to vgpu
> devices.
> 
> In general using UEFI works better than BIOS, because guests don't
> expect legacy vga being present then.
> 
> What works?
> ---
> 
> Both windows and linux UEFI guests handle the ramfb GOP just fine.
> 
> BIOS boot loaders for linux all use vgabios calls for text mode, so they
> show up just fine.  Also ipxe, seabios itself of course.  So you can
> boot up your linux guest.  vesafb works too.
> 
> What doesn't work?
> --
> 
> vgacon (direct vga hardware access).  Linux boots just fine
> nevertheless, the only effect is that you don't see any boot messages
> until the drm driver loads.
> 
> Windows in BIOS mode.  Boot logo shows up just fine.  But at some point
> windows does lots of vga register accesses (even though it sets the
> video mode via vesa bios interface) and appears to be unhappy that
> things don't work as expected because there is no vga hardware
> emulation.
> 
> Known issues
> 
> 
> Handover from ramfb-backed efifb to the native linux driver is tricky.
> Usually efifb gets kicked out when the native driver loads because of
> overlapping ressources.  With efifb being in RAM instead of using a GPU
> PCI bar this doesn't happen though, so you'll end up with two
> framebuffer devices.
> 
> In case vgaarb classifies the GPU as primary display device fbcon will
> switch all VTs over to the framebuffer device of the real GPU, so there
> isn't a noticable difference.  Otherwise you'll end up with a
> non-visible fbcon, because it continues to run on ramfb whereas qemu
> switched over to the GPU because the native linux driver initialized the
> display.
> 
> xorg/wayland will show up on the GPU in any case because they prefer drm
> over fbdev, so they wouldn't run on efifb.
> 
> Not tested yet
> --
> 
> ARM.
> 
> ramfb -> gpu handover with windows guests (only ramfb-testdev so far).
> 
> enjoy,
>   Gerd
> 
>

Re: [Qemu-devel] [PATCH V2 1/1] mach-virt: Set VM's SMBIOS system version to mc->name

[Peter Maydell]

On 23 March 2018 at 07:48, Andrew Jones  wrote:
> On Thu, Mar 22, 2018 at 04:23:18PM -0500, Wei Huang wrote:
>> Instead of using "1.0" as the system version of SMBIOS, we should use
>> mc->name for mach-virt machine type to be consistent other architectures.
>> With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will
>> show:
>>
>> Handle 0x0100, DMI type 1, 27 bytes
>> System Information
>> Manufacturer: QEMU
>> Product Name: KVM Virtual Machine
>> Version: virt-2.12
>> Serial Number: Not Specified
>> ...
>>
>> instead of:
>>
>> Handle 0x0100, DMI type 1, 27 bytes
>> System Information
>> Manufacturer: QEMU
>> Product Name: KVM Virtual Machine
>> Version: 1.0
>> Serial Number: Not Specified
>> ...
>>
>> For backward compatibility, we allow older machine types to keep "1.0"
>> as the default system version.
>>
>> Signed-off-by: Wei Huang 

> I don't think we need the compat code in this case. I used to pedantically
> recommend compat code for all guest visible changes, but after speaking
> with Igor about the policy for introducing compat code for ACPI table
> updates, I learned that we can relax a bit on a case by case basis. In
> this case I would recommend relaxing, but I won't insist, as the compat
> code is indeed the safest way to go.
>
> Reviewed-by: Andrew Jones 

Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v8 20/23] monitor: enable IO thread for (qmp & !mux) typed

[Peter Xu]

On Fri, Mar 23, 2018 at 02:23:27PM +0100, Christian Borntraeger wrote:
> 
> 
> On 03/23/2018 02:01 PM, Peter Xu wrote:
> > On Fri, Mar 23, 2018 at 01:44:54PM +0100, Christian Borntraeger wrote:
> >>
> >>
> >> On 03/23/2018 01:25 PM, Peter Xu wrote:
> >>> On Fri, Mar 23, 2018 at 01:10:51PM +0100, Christian Borntraeger wrote:
>  As Max Reitz said, this breaks several qemu iotests. I can reproduce 
>  that on s390
>  e.g. with ./check -qcow2 030 in the qemu-iotest.
> 
>  Why was this merged?
> >>>
> >>> My fault.  Sorry.  I should have done iotests before submission.
> >>>
> >>> Now I'm trying to fix those iotest issues.
> >>>
> >>> I'm not extremely familiar with block, so the progress is a bit slow,
> >>> but I'll do it asap (though I'll possibly need some help from the
> >>> block team, since there are some not-easy ones for me).
> >>
> >> As this patch also seems to trigger something else for Eric, can we revert 
> >> this
> >> patch for the time being and re-apply as soon as we have solutions for the 
> >> issues?
> >>
> >> After all a hard hang in the iotest can break CI environments (as you run 
> >> into timeouts).
> > 
> > I'm fine with it if it helps.
> > 
> > Then I can work in background to solved existing problems, then we
> > turn it on again in 2.13 when ready (but still it might break things -
> > at least it can be during dev cycle rather than softfreeze).
> 
> Strangely enough, reverting 3fd2457d18edf5736f713dfe1
> now causes make check errors... :-/
> 
> 
>   GTESTER check-qtest-s390x
>   GTESTER check-qtest-i386
> **
> ERROR:/home/cborntra/REPOS/qemu/tests/qmp-test.c:97:test_qmp_protocol: 
> assertion failed: (entry)
> GTester: last random seed: R02S1cbb7e297cc349b33ae39f10dd92764a
> **
> ERROR:/home/cborntra/REPOS/qemu/tests/qmp-test.c:190:test_qmp_oob: assertion 
> failed: (qdict_haskey(resp, "return"))
> GTester: last random seed: R02S7afc5ab63f6f29401d702dceb1f8030b
> make: *** [/home/cborntra/REPOS/qemu/tests/Makefile.include:880: 
> check-qtest-s390x] Error 1
> make: *** Waiting for unfinished jobs
> ^Cmake: *** [/home/cborntra/REPOS/qemu/tests/Makefile.include:880: 
> check-qtest-i386] Interrupt
> 
> 
> Do we have patches depending on this change?

It's the new OOB tests that's failing.  I'm preparing a patchset that
reverts the enablement patch, along with those tests.

Now I'm testing that branch with more platforms and also all
raw+qcow2 iotests.  After that's solid, I'll post that out soon if it
goes smoothly.

Hopefully that would fix all problems so far.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Michael S. Tsirkin]

On Fri, Mar 23, 2018 at 10:22:27AM +, Daniel P. Berrangé wrote:
> On Thu, Mar 22, 2018 at 09:27:55PM +0200, Michael S. Tsirkin wrote:
> > Make sure all generated files go into qemu-build subdirectory.
> > We can then include them like this:
> >  #include "qemu-build/trace.h"
> > 
> > This serves two purposes:
> > - make it easy to detect which files are in the source
> >   directory (a bit more work for writers, easier for readers)
> > - reduce chances of conflicts with possible stale files in source
> >   directory (which could be left over from e.g. old patches, etc)
> 
> If people care about this, then they can just be doing a build
> with  srcdir != builddir config.

Helps with the second point but I do not see how this will help address
the first point.

I see include "x.h" in source, it's natural to ask where it is.

I look for it in the c file directory. Not there.

I look for it in the include, it's not there. Where then?

qemu source root on the include path. Not there.

Oh tcg directory is in the include path for some reason. Not there.

Some other tree coming from a submodule maybe?

Oh *maybe* it will be generated by some script, I need to run
build to find out.

For the record, more remains to be done.
Here's a list from and oot build of virtio, with some comments:

cc -iquote /home/mst/qemu-oot/hw/virtio


 -iquote hw/virtio 

<- same directory repeated twice.

<- also this is here just for trace.h . Better to just change code to point at 
the right trace header -
   will look into it.

-iquote /home/mst/qemu/tcg

<- why does everyone want tcg? better to just include with tcg/

 -iquote /home/mst/qemu/tcg/i386

<- and i386 specifically?

 -I/home/mst/qemu/linux-headers

<- ok this is so we can override with our own version of headers

 -I/home/mst/qemu-oot/linux-headers

<- why do we have these at all?
   oh it's because we have asm- tricks like linux used to use years ago,
   then we copy the correct asm headers:
 ls linux-headers/asm/
 bitsperlong.h  hyperv.h  kvm.h  kvm_para.h  unistd_32.h  unistd_64.h  
unistd.h  unistd_x32.h

   a better strategy would be to have headers in arch//asm like Linux 
does now,
   no code generation tricks.

 -iquote .

<- build directory. No headers there at all.

 -iquote /home/mst/qemu 

<- turns out we still have headers in source root. Why not move them to 
include/ ?

-iquote /home/mst/qemu/accel/tcg 

<- more tcg goodness for everyone?

-iquote /home/mst/qemu/include 

<- ok that's expected. Why isn't this first on the list though?

-I/usr/include/pixman-1

<- should be limited to ui files I guess?

  -I/usr/include/glib-2.0 

<- ok we need that

-I/usr/lib64/glib-2.0/include 

<- but that one does not exist

  -I/usr/include/p11-kit-1 -I/usr/include/libpng16  -I/usr/include/libdrm

<- more GUI things?

  -I/home/mst/qemu/capstone/include

<- ok a bunch of disasm things.
ls capstone/include/
arm64.h  arm.h  capstone.h  mips.h  platform.h  ppc.h  sparc.h  
systemz.h  x86.h  xcore.h
   can we add this just to disas.c?

  -I../linux-headers

<- all together now: same as qemu-oot but with a relative path now.

 -iquote ..

<- build directory - no headers there

 -iquote /home/mst/qemu/target/arm

<- this is a good idea why?

 -iquote /home/mst/qemu/include

<- deja vu

Wow that was fun.

>  If people are using srcdir == builddir
> then they likely *want* all the generated files in their srcdir.
> 
> IMHO it would be valid for us to consider if we could just mandate
> srcdir != builddir, but if people object to such a proposal, then I
> don't think we should arbitrarily move all generated source files
> in this way, as that's effectively the same thing forced onto devs.
> 
> Regards,
> Daniel

People don't really care.





> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Paolo Bonzini]

On 23/03/2018 14:12, Peter Maydell wrote:
> I think that the latter seems to be what the design intends:
> a backend with a NULL chardev pointer is "backend with no
> chardev attached at the moment". At least some of our UART devices
> handle that as "just dump output into nothingness", which
> seems logical to me.
> 
> Cc'ing Marc-André and Paolo as the chardev maintainers --
> do you have a plan for what the intended design here is?

-nodefaults was added for x86 and other machines whose devices are
discoverable through PCI enumeration, ACPI or something like that, and
on those machine it suppresses the default devices.  For machines that
create their device tree automatically, it makes sense to do the same.

However, if the machine is emulating a real-world board with a fixed SoC
and fixed hardware in the SoC, it makes more sense to create a null backend.

Paolo

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Peter Maydell]

On 23 March 2018 at 14:02, Paolo Bonzini  wrote:

> However, if the machine is emulating a real-world board with a fixed SoC
> and fixed hardware in the SoC, it makes more sense to create a null backend.

That's a lot of duplicate code to say "oh, this is NULL, create the
null backend" in lots of different boards :-(

thanks
-- PMM

[Qemu-devel] [PATCH for-2.12 0/4] Turn OOB off for 2.12-rc1, revert OOB tests

[Peter Xu]

Temporarily disable OOB for 2.12 since it break (a lot of) things.
Luckily it's only a switch so not so hard. Meanwhile, revert all new
tests that checks against it.

Tests done: all target builds and "make check" passed, but only on
x86_64 host.  It can pass all "raw" iotests, but some "qcow2" iotests
are still failing.  I can even reproduce many of the qcow2 fails even
before the whole OOB series, so I suppose that's something already
exists so I'll ignore.

More tests are really welcomed.  Thanks,

Peter

Peter Xu (4):
  Revert "monitor: enable IO thread for (qmp & !mux) typed"
  Revert "tests: qmp-test: add oob test"
  Revert "tests: qmp-test: verify command batching"
  tests: partly revert "qmp: introduce QMPCapability"

 monitor.c|  5 +--
 tests/qmp-test.c | 97 +---
 2 files changed, 2 insertions(+), 100 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 2/4] Revert "tests: qmp-test: add oob test"

[Peter Xu]

This reverts commit d003f7a8f9cafe50119975844fa01afc2baf41fb.

Signed-off-by: Peter Xu 
---
 tests/qmp-test.c | 65 
 1 file changed, 65 deletions(-)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 07c0b87e27..2e4b599a4c 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -164,70 +164,6 @@ static void test_qmp_protocol(void)
 qtest_quit(qts);
 }
 
-/* Tests for Out-Of-Band support. */
-static void test_qmp_oob(void)
-{
-QDict *resp;
-int acks = 0;
-const char *cmd_id;
-
-global_qtest = qtest_init_without_qmp_handshake(common_args);
-
-/* Ignore the greeting message. */
-resp = qmp_receive();
-g_assert(qdict_get_qdict(resp, "QMP"));
-QDECREF(resp);
-
-/* Try a fake capability, it should fail. */
-resp = qmp("{ 'execute': 'qmp_capabilities', "
-   "  'arguments': { 'enable': [ 'cap-does-not-exist' ] } }");
-g_assert(qdict_haskey(resp, "error"));
-QDECREF(resp);
-
-/* Now, enable OOB in current QMP session, it should succeed. */
-resp = qmp("{ 'execute': 'qmp_capabilities', "
-   "  'arguments': { 'enable': [ 'oob' ] } }");
-g_assert(qdict_haskey(resp, "return"));
-QDECREF(resp);
-
-/*
- * Try any command that does not support OOB but with OOB flag. We
- * should get failure.
- */
-resp = qmp("{ 'execute': 'query-cpus',"
-   "  'control': { 'run-oob': true } }");
-g_assert(qdict_haskey(resp, "error"));
-QDECREF(resp);
-
-/*
- * First send the "x-oob-test" command with lock=true and
- * oob=false, it should hang the dispatcher and main thread;
- * later, we send another lock=false with oob=true to continue
- * that thread processing.  Finally we should receive replies from
- * both commands.
- */
-qmp_async("{ 'execute': 'x-oob-test',"
-  "  'arguments': { 'lock': true }, "
-  "  'id': 'lock-cmd'}");
-qmp_async("{ 'execute': 'x-oob-test', "
-  "  'arguments': { 'lock': false }, "
-  "  'control': { 'run-oob': true }, "
-  "  'id': 'unlock-cmd' }");
-
-/* Ignore all events.  Wait for 2 acks */
-while (acks < 2) {
-resp = qmp_receive();
-cmd_id = qdict_get_str(resp, "id");
-if (!g_strcmp0(cmd_id, "lock-cmd") ||
-!g_strcmp0(cmd_id, "unlock-cmd")) {
-acks++;
-}
-QDECREF(resp);
-}
-
-qtest_end();
-}
-
 static int query_error_class(const char *cmd)
 {
 static struct {
@@ -412,7 +348,6 @@ int main(int argc, char *argv[])
 g_test_init(&argc, &argv, NULL);
 
 qtest_add_func("qmp/protocol", test_qmp_protocol);
-qtest_add_func("qmp/oob", test_qmp_oob);
 qmp_schema_init(&schema);
 add_query_tests(&schema);
 
-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 1/4] Revert "monitor: enable IO thread for (qmp & !mux) typed"

[Peter Xu]

This reverts commit 3fd2457d18edf5736f713dfe1ada9c87a9badab1.

Signed-off-by: Peter Xu 
---
 monitor.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 6ccd2fc089..77f4c41cfa 100644
--- a/monitor.c
+++ b/monitor.c
@@ -36,7 +36,6 @@
 #include "net/slirp.h"
 #include "chardev/char-fe.h"
 #include "chardev/char-io.h"
-#include "chardev/char-mux.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/numa.h"
 #include "monitor/monitor.h"
@@ -4537,10 +4536,8 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
 void monitor_init(Chardev *chr, int flags)
 {
 Monitor *mon = g_malloc(sizeof(*mon));
-/* Enable IOThread for QMPs that are not using MUX chardev backends. */
-bool use_io_thr = (!CHARDEV_IS_MUX(chr)) && (flags & MONITOR_USE_CONTROL);
 
-monitor_data_init(mon, false, use_io_thr);
+monitor_data_init(mon, false, false);
 
 qemu_chr_fe_init(&mon->chr, chr, &error_abort);
 mon->flags = flags;
-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 3/4] Revert "tests: qmp-test: verify command batching"

[Peter Xu]

This reverts commit 91ad45061af0fe44ac5dadb5bedaf4d7a08077c8.

Signed-off-by: Peter Xu 
---
 tests/qmp-test.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 2e4b599a4c..d1fa1cb217 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -82,7 +82,6 @@ static void test_qmp_protocol(void)
 QTestState *qts;
 const QListEntry *entry;
 QString *qstr;
-int i;
 
 qts = qtest_init_without_qmp_handshake(common_args);
 
@@ -140,27 +139,6 @@ static void test_qmp_protocol(void)
 g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2);
 QDECREF(resp);
 
-/*
- * Test command batching.  In current test OOB is not enabled, we
- * should be able to run as many commands in batch as we like.
- * Using 16 (>8, which is OOB queue length) to make sure OOB won't
- * break existing clients.  Note: this test does not control the
- * scheduling of QEMU's QMP command processing threads so it may
- * not really trigger batching inside QEMU.  This is just a
- * best-effort test.
- */
-for (i = 0; i < 16; i++) {
-qtest_async_qmp(qts, "{ 'execute': 'query-version' }");
-}
-/* Verify the replies to make sure no command is dropped. */
-for (i = 0; i < 16; i++) {
-resp = qtest_qmp_receive(qts);
-/* It should never be dropped.  Each of them should be a reply. */
-g_assert(qdict_haskey(resp, "return"));
-g_assert(!qdict_haskey(resp, "event"));
-QDECREF(resp);
-}
-
 qtest_quit(qts);
 }
 
-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 4/4] tests: partly revert "qmp: introduce QMPCapability"

[Peter Xu]

Revert the tests in the patch 02130314d8 ("qmp: introduce
QMPCapability", 2018-03-19) since we'll disable OOB for now.

Signed-off-by: Peter Xu 
---
 tests/qmp-test.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index d1fa1cb217..558e83540c 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -80,8 +80,6 @@ static void test_qmp_protocol(void)
 QDict *resp, *q, *ret;
 QList *capabilities;
 QTestState *qts;
-const QListEntry *entry;
-QString *qstr;
 
 qts = qtest_init_without_qmp_handshake(common_args);
 
@@ -91,13 +89,7 @@ static void test_qmp_protocol(void)
 g_assert(q);
 test_version(qdict_get(q, "version"));
 capabilities = qdict_get_qlist(q, "capabilities");
-g_assert(capabilities);
-entry = qlist_first(capabilities);
-g_assert(entry);
-qstr = qobject_to(QString, entry->value);
-g_assert(qstr);
-g_assert_cmpstr(qstring_get_str(qstr), ==, "oob");
-QDECREF(resp);
+g_assert(capabilities && qlist_empty(capabilities));
 
 /* Test valid command before handshake */
 resp = qtest_qmp(qts, "{ 'execute': 'query-version' }");
-- 
2.14.3

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Paolo Bonzini]

On 23/03/2018 15:04, Peter Maydell wrote:
>> However, if the machine is emulating a real-world board with a fixed SoC
>> and fixed hardware in the SoC, it makes more sense to create a null backend.
>
> That's a lot of duplicate code to say "oh, this is NULL, create the
> null backend" in lots of different boards :-(

Note it's a null "backend", not necessarily a null "character device".
Your proposal, namely ensuring that be->chr == NULL is handled properly
in qemu_chr_fe_init, would be just fine.

The main point was that the choice of whether to create the device is up
to the board, and there isn't a single answer valid for all boards.

Paolo

Re: [Qemu-devel] [PATCH for 2.13 2/5] linux-user: remove unneeded #ifdef in signal.c

[Peter Maydell]

On 22 March 2018 at 21:58, Laurent Vivier  wrote:
> Signed-off-by: Laurent Vivier 
> ---
>  linux-user/signal.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 2c08ca14cf..514145b299 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -253,17 +253,15 @@ int do_sigprocmask(int how, const sigset_t *set, 
> sigset_t *oldset)
>  return 0;
>  }
>
> -#if !defined(TARGET_OPENRISC) && !defined(TARGET_NIOS2)
>  /* Just set the guest's signal mask to the specified value; the
>   * caller is assumed to have called block_signals() already.
>   */
> -static void set_sigmask(const sigset_t *set)
> +static __attribute__((unused)) void set_sigmask(const sigset_t *set)
>  {
>  TaskState *ts = (TaskState *)thread_cpu->opaque;
>
>  ts->signal_mask = *set;
>  }
> -#endif
>
>  /* siginfo conversion */
>
> @@ -533,8 +531,7 @@ static void force_sig(int sig)
>   * up the signal frame. oldsig is the signal we were trying to handle
>   * at the point of failure.
>   */
> -#if !defined(TARGET_RISCV)
> -static void force_sigsegv(int oldsig)
> +static __attribute__((unused)) void force_sigsegv(int oldsig)
>  {
>  if (oldsig == SIGSEGV) {
>  /* Make sure we don't try to deliver the signal again; this will
> @@ -545,8 +542,6 @@ static void force_sigsegv(int oldsig)
>  force_sig(TARGET_SIGSEGV);
>  }
>
> -#endif
> -
>  /* abort execution with signal */
>  static void QEMU_NORETURN dump_core_and_abort(int target_sig)
>  {

I don't think this is the right approach. In both of these
cases, the fact that the function is unused is a red flag that
there is a bug in the signal emulation code for those guest CPUs.
If the guest CPU code isn't calling set_sigmask() it is failing
to correctly handle the sigmask field in the ucontext on
signal return, and if it's not calling force_sigsegv() then
it's doing something wrong with the handling of "we couldn't
write to the signal frame" on signal entry.

I think it's worth keeping the #ifdef as a flag and
a list of what targets we need to fix.

(The "riscv isn't using force_sigsegv()" fix looks like it should be
trivial -- their setup_rt_frame() has the code path but it's
incorrectly trying to do this by hand rather than calling
force_sigsegv().)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.12 0/4] Turn OOB off for 2.12-rc1, revert OOB tests

[Peter Xu]

On Fri, Mar 23, 2018 at 10:08:17PM +0800, Peter Xu wrote:
> Temporarily disable OOB for 2.12 since it break (a lot of) things.
> Luckily it's only a switch so not so hard. Meanwhile, revert all new
> tests that checks against it.
> 
> Tests done: all target builds and "make check" passed, but only on
> x86_64 host.  It can pass all "raw" iotests, but some "qcow2" iotests
> are still failing.  I can even reproduce many of the qcow2 fails even
> before the whole OOB series, so I suppose that's something already
> exists so I'll ignore.
> 
> More tests are really welcomed.  Thanks,

CC Eric Blake too.

-- 
Peter Xu

Re: [Qemu-devel] [PATCH for 2.13 3/5] linux-user: define TARGET_ARCH_HAS_SETUP_FRAME

[Peter Maydell]

On 22 March 2018 at 21:58, Laurent Vivier  wrote:
> instead of calling setup_frame() conditionnaly to a list of knowm targets,

"Instead". "conditionally". "known".

> define TARGET_ARCH_HAS_SETUP_FRAME if the target provides the function
> and call it only if the macro is defined

Trailing ".".

>
> Signed-off-by: Laurent Vivier 
> ---

> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -886,18 +886,13 @@ static void handle_pending_signal(CPUArchState 
> *cpu_env, int sig,
>  }
>  #endif
>  /* prepare the stack frame of the virtual CPU */
> -#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) \
> -|| defined(TARGET_OPENRISC) || defined(TARGET_TILEGX) \
> -|| defined(TARGET_PPC64) || defined(TARGET_HPPA) \
> -|| defined(TARGET_NIOS2) || defined(TARGET_X86_64) \
> -|| defined(TARGET_RISCV) || defined(TARGET_XTENSA)
> -/* These targets do not have traditional signals.  */
> -setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
> -#else
> +#if defined(TARGET_ARCH_HAS_SETUP_FRAME)
>  if (sa->sa_flags & TARGET_SA_SIGINFO)
>  setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
>  else
>  setup_frame(sig, sa, &target_old_set, cpu_env);
> +#else
> +setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
>  #endif
>  if (sa->sa_flags & TARGET_SA_RESETHAND) {
>  sa->_sa_handler = TARGET_SIG_DFL;

You might like to add the {} to the if (sa->sa_flags...) while we're
here.

Either way,

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH for 2.13 1/5] linux-user: cleanup signal.c

[Peter Maydell]

On 22 March 2018 at 21:58, Laurent Vivier  wrote:
> move all target specific parts to
> signal.inc.c in arch directory
>
> Signed-off-by: Laurent Vivier 
> ---
>  linux-user/aarch64/signal.inc.c|  556 +++
>  linux-user/alpha/signal.inc.c  |  258 ++
>  linux-user/arm/signal.inc.c|  749 +
>  linux-user/cris/signal.inc.c   |  166 +
>  linux-user/hppa/signal.inc.c   |  188 ++
>  linux-user/i386/signal.inc.c   |  579 
>  linux-user/m68k/signal.inc.c   |  406 +++
>  linux-user/microblaze/signal.inc.c |  226 ++
>  linux-user/mips/signal.inc.c   |  378 +++
>  linux-user/mips64/signal.inc.c |1 +
>  linux-user/nios2/signal.inc.c  |  232 ++
>  linux-user/openrisc/signal.inc.c   |  209 ++
>  linux-user/ppc/signal.inc.c|  667 
>  linux-user/riscv/signal.inc.c  |  196 ++
>  linux-user/s390x/signal.inc.c  |  305 ++
>  linux-user/sh4/signal.inc.c|  327 ++
>  linux-user/signal.c| 6487 
> +---
>  linux-user/sparc/signal.inc.c  |  601 
>  linux-user/sparc64/signal.inc.c|1 +
>  linux-user/tilegx/signal.inc.c |  163 +
>  linux-user/x86_64/signal.inc.c |1 +
>  linux-user/xtensa/signal.inc.c |  253 ++
>  22 files changed, 6463 insertions(+), 6486 deletions(-)

I think anything with a diffstat like this is basically
unreviewable, at least for me :-(

> diff --git a/linux-user/aarch64/signal.inc.c b/linux-user/aarch64/signal.inc.c
> new file mode 100644
> index 00..28fa0f2f22
> --- /dev/null
> +++ b/linux-user/aarch64/signal.inc.c

I was hoping we could have these be standalone .c files,
rather than just #included from linux-user/signal.c.
I appreciate this requires a bit more teasing out of what
needs to become non-static in the common code signal.c
to be callable from the per-target code, though.

> @@ -0,0 +1,556 @@
> +struct target_sigcontext {

New source files should all have the usual sort of comment
header with a copyright and license statement.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] vhost: Allow abutting regions

[Michael S. Tsirkin]

On Fri, Mar 23, 2018 at 12:46:37PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> My rework of section adding combines overlapping or abutting regions,
> but checks they're actually the same underlying RAM block.
> Fix the case where two blocks abut but don't overlap; that new region
> should get added (but not combined), but my previous patch was disallowing it.
> 
> Fixes: c1ece84e7c9
> 
> Reported-by: Alex Williamson 
> Signed-off-by: Dr. David Alan Gilbert 

Can we replace abutting with adjacent pls?

> ---
>  hw/virtio/vhost.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 250f886acb..fc9062a89f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -595,10 +595,15 @@ static void vhost_region_add_section(struct vhost_dev 
> *dev,
>  
> prev_sec->offset_within_address_space,
>  prev_sec->offset_within_region);
>  } else {
> -error_report("%s: Overlapping but not coherent sections "
> - "at %"PRIx64,
> - __func__, mrs_gpa);
> -return;
> +/* abutting regions are fine, but overlapping ones with
> + * different blocks/offsets shouldn't happen
> + */
> +if (mrs_gpa != prev_gpa_end + 1) {
> +error_report("%s: Overlapping but not coherent sections "
> + "at %"PRIx64,
> + __func__, mrs_gpa);
> +return;
> +}
>  }
>  }
>  }
> -- 
> 2.14.3

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Peter Maydell]

On 23 March 2018 at 14:13, Paolo Bonzini  wrote:
> On 23/03/2018 15:04, Peter Maydell wrote:
>>> However, if the machine is emulating a real-world board with a fixed SoC
>>> and fixed hardware in the SoC, it makes more sense to create a null backend.
>>
>> That's a lot of duplicate code to say "oh, this is NULL, create the
>> null backend" in lots of different boards :-(
>
> Note it's a null "backend", not necessarily a null "character device".
> Your proposal, namely ensuring that be->chr == NULL is handled properly
> in qemu_chr_fe_init, would be just fine.

Hmm, the chardev layer code seems to have more ends than I expect.
The "frontend" is the UART model, right, and I thought the
"backend" was the TCP/UDP/serial port/stdio/etc end of things,
but those seem to be Chardevs? If those aren't the backend,
then what is?

What I'd like, anyway, is that every UART model can cope with being
setup with a NULL 'chardev' property, and ideally that that doesn't
require a lot of extra code per-UART, and doesn't require each UART
to create a TYPE_CHARDEV_NULL.

> The main point was that the choice of whether to create the device is up
> to the board, and there isn't a single answer valid for all boards.

I agree with that part, yes, assuming I have the terminology right now.

thanks
-- PMM

[Qemu-devel] [PATCH v2 1/5] make: improve check for stale generated files in source dir

[Laurent Vivier]

From: Daniel P. Berrangé 

When doing a build with builddir != srcdir, if any generated files are
accidentally present in srcdir from a previous build, these can cause
unexpected failures.

Currently there is a rule that checks for existance of config-host.mak,
but there have been cases where config-host.mak is absent, while other
generated files still exist.

Update the check to look at every file listed in $(GENERATED_FILES). To
do this we must move the check further down after $(GENERATED_FILES) has
been populated.

Signed-off-by: Daniel P. Berrangé 
---
 Makefile | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 727ef118f3..d588a13687 100644
--- a/Makefile
+++ b/Makefile
@@ -52,16 +52,6 @@ endif
 
 .git-submodule-status: git-submodule-update config-host.mak
 
-# Check that we're not trying to do an out-of-tree build from
-# a tree that's been used for an in-tree build.
-ifneq ($(realpath $(SRC_PATH)),$(realpath .))
-ifneq ($(wildcard $(SRC_PATH)/config-host.mak),)
-$(error This is an out of tree build but your source tree ($(SRC_PATH)) \
-seems to have been used for an in-tree build. You can fix this by running \
-"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
-endif
-endif
-
 CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
 CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
 CONFIG_XEN := $(CONFIG_XEN_BACKEND)
@@ -1028,6 +1018,25 @@ ifdef SIGNCODE
 endif # SIGNCODE
 endif # CONFIG_WIN
 
+define nl
+
+
+endef
+
+CHECK_FILES = config-host.mak $(filter-out .git-submodule-status, 
$(GENERATED_FILES))
+UNEXPECTED_FILES = $(wildcard $(CHECK_FILES:%=$(SRC_PATH)/%))
+
+# Check that we're not trying to do an out-of-tree build from
+# a tree that's been used for an in-tree build.
+ifneq ($(realpath $(SRC_PATH)),$(realpath .))
+ifneq ($(UNEXPECTED_FILES),)
+$(error Stale files in source tree:${nl}${nl} $(UNEXPECTED_FILES:%=  %${nl}) 
${nl}\
+This is an out of tree build but your source tree ($(SRC_PATH)) \
+seems to have been used for an in-tree build. You can fix this by running \
+"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
+endif
+endif
+
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
 ifneq ($(wildcard config-host.mak),)
-- 
2.14.3

[Qemu-devel] [PATCH v2 0/5] coccinelle: re-run scripts from scripts/coccinelle

[Laurent Vivier]

I've re-run some scripts from the coccinelle directory,
and they have found some problems.

This series fixes them.

v2: only change PATCH 4/4
  - keep comments
  - fix indentation
  I didn't remove changes in autogenerated files as it
  seems they are generated only once.

Daniel P. Berrangé (1):
  make: improve check for stale generated files in source dir

Laurent Vivier (4):
  error: Strip trailing '\n' from error string arguments (again again)
  error: Remove NULL checks on error_propagate() calls
  qdict: remove useless cast
  Remove unnecessary variables for function return value

 Makefile   | 29 +++
 accel/tcg/translate-all.c  |  5 +-
 block/nvme.c   | 11 ++---
 block/quorum.c |  6 +--
 hw/arm/exynos4210.c|  6 +--
 hw/block/vhost-user-blk.c  |  5 +-
 hw/hppa/dino.c |  5 +-
 hw/misc/mos6522.c  |  8 +---
 hw/net/ftgmac100.c |  5 +-
 hw/ppc/pnv_lpc.c   | 16 ++-
 io/channel-websock.c   |  4 +-
 io/net-listener.c  |  6 +--
 monitor.c  |  2 +-
 target/i386/hax-darwin.c   | 10 ++--
 target/i386/hvf/hvf.c  | 24 +-
 target/mips/dsp_helper.c   | 15 ++
 target/xtensa/core-dc232b/xtensa-modules.c | 56 ++
 target/xtensa/core-dc233c/xtensa-modules.c | 56 ++
 target/xtensa/core-de212/xtensa-modules.c  | 48 +--
 target/xtensa/core-fsf/xtensa-modules.c| 32 -
 .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
 target/xtensa/translate.c  |  7 +--
 tests/m48t59-test.c|  6 +--
 tests/test-thread-pool.c   |  6 +--
 util/uri.c |  5 +-
 25 files changed, 117 insertions(+), 280 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v2 3/5] error: Remove NULL checks on error_propagate() calls

[Laurent Vivier]

Re-run Coccinelle patch
scripts/coccinelle/error_propagate_null.cocci

Signed-off-by: Laurent Vivier 
---
 io/channel-websock.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index ec48a305f0..e6608b969d 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -586,9 +586,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel 
*ioc,
 return TRUE;
 }
 
-if (err) {
-error_propagate(&wioc->io_err, err);
-}
+error_propagate(&wioc->io_err, err);
 
 trace_qio_channel_websock_handshake_reply(ioc);
 qio_channel_add_watch(
-- 
2.14.3

[Qemu-devel] [PATCH v2 5/5] Remove unnecessary variables for function return value

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/return_directly.cocci

Signed-off-by: Laurent Vivier 
---
 accel/tcg/translate-all.c  |  5 +-
 block/quorum.c |  6 +--
 hw/arm/exynos4210.c|  6 +--
 hw/block/vhost-user-blk.c  |  5 +-
 hw/hppa/dino.c |  5 +-
 hw/misc/mos6522.c  |  8 +---
 hw/net/ftgmac100.c |  5 +-
 hw/ppc/pnv_lpc.c   | 16 ++-
 io/net-listener.c  |  6 +--
 target/i386/hax-darwin.c   | 10 ++--
 target/mips/dsp_helper.c   | 15 ++
 target/xtensa/core-dc232b/xtensa-modules.c | 56 ++
 target/xtensa/core-dc233c/xtensa-modules.c | 56 ++
 target/xtensa/core-de212/xtensa-modules.c  | 48 +--
 target/xtensa/core-fsf/xtensa-modules.c| 32 -
 .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
 target/xtensa/translate.c  |  7 +--
 tests/m48t59-test.c|  6 +--
 tests/test-thread-pool.c   |  6 +--
 util/uri.c |  5 +-
 20 files changed, 79 insertions(+), 248 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5ad1b919bc..55d822d410 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -644,11 +644,8 @@ static inline void *alloc_code_gen_buffer(void)
 static inline void *alloc_code_gen_buffer(void)
 {
 size_t size = tcg_ctx->code_gen_buffer_size;
-void *buf;
-
-buf = VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
+return VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
 PAGE_EXECUTE_READWRITE);
-return buf;
 }
 #else
 static inline void *alloc_code_gen_buffer(void)
diff --git a/block/quorum.c b/block/quorum.c
index 14333c18aa..304442ef65 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -608,7 +608,7 @@ static void read_quorum_children_entry(void *opaque)
 static int read_quorum_children(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
-int i, ret;
+int i;
 
 acb->children_read = s->num_children;
 for (i = 0; i < s->num_children; i++) {
@@ -643,9 +643,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
 qemu_coroutine_yield();
 }
 
-ret = acb->vote_ret;
-
-return ret;
+return acb->vote_ret;
 }
 
 static int read_fifo_child(QuorumAIOCB *acb)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 06f9d1ffa4..b7463a71ec 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -156,12 +156,8 @@ void exynos4210_write_secondary(ARMCPU *cpu,
 
 static uint64_t exynos4210_calc_affinity(int cpu)
 {
-uint64_t mp_affinity;
-
 /* Exynos4210 has 0x9 as cluster ID */
-mp_affinity = (0x9 << ARM_AFF1_SHIFT) | cpu;
-
-return mp_affinity;
+return (0x9 << ARM_AFF1_SHIFT) | cpu;
 }
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f840f07dfe..3f41ca9e26 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -196,7 +196,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
*vdev,
 Error **errp)
 {
 VHostUserBlk *s = VHOST_USER_BLK(vdev);
-uint64_t get_features;
 
 /* Turn on pre-defined features */
 virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
@@ -215,9 +214,7 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
*vdev,
 virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
 }
 
-get_features = vhost_get_features(&s->dev, user_feature_bits, features);
-
-return get_features;
+return vhost_get_features(&s->dev, user_feature_bits, features);
 }
 
 static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 15aefde09c..c5dcf3104d 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -403,13 +403,10 @@ static void dino_set_irq(void *opaque, int irq, int level)
 static int dino_pci_map_irq(PCIDevice *d, int irq_num)
 {
 int slot = d->devfn >> 3;
-int local_irq;
 
 assert(irq_num >= 0 && irq_num <= 3);
 
-local_irq = slot & 0x03;
-
-return local_irq;
+return slot & 0x03;
 }
 
 static void dino_set_timer_irq(void *opaque, int irq, int level)
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 8ad9fc831e..6163cea6ab 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -176,12 +176,8 @@ static void mos6522_set_sr_int(MOS6522State *s)
 
 static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
 {
-uint64_t d;
-
-d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) 

[Qemu-devel] [PATCH v2 2/5] error: Strip trailing '\n' from error string arguments (again again)

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/err-bad-newline.cocci,
and found new error_report() occurences with '\n'.

Signed-off-by: Laurent Vivier 
---
 target/i386/hvf/hvf.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 15870a4f36..c36753954b 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -86,25 +86,25 @@ static void assert_hvf_ok(hv_return_t ret)
 
 switch (ret) {
 case HV_ERROR:
-error_report("Error: HV_ERROR\n");
+error_report("Error: HV_ERROR");
 break;
 case HV_BUSY:
-error_report("Error: HV_BUSY\n");
+error_report("Error: HV_BUSY");
 break;
 case HV_BAD_ARGUMENT:
-error_report("Error: HV_BAD_ARGUMENT\n");
+error_report("Error: HV_BAD_ARGUMENT");
 break;
 case HV_NO_RESOURCES:
-error_report("Error: HV_NO_RESOURCES\n");
+error_report("Error: HV_NO_RESOURCES");
 break;
 case HV_NO_DEVICE:
-error_report("Error: HV_NO_DEVICE\n");
+error_report("Error: HV_NO_DEVICE");
 break;
 case HV_UNSUPPORTED:
-error_report("Error: HV_UNSUPPORTED\n");
+error_report("Error: HV_UNSUPPORTED");
 break;
 default:
-error_report("Unknown Error\n");
+error_report("Unknown Error");
 }
 
 abort();
@@ -191,7 +191,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 if (mem) {
 mem->size = 0;
 if (do_hvf_set_memory(mem)) {
-error_report("Failed to reset overlapping slot\n");
+error_report("Failed to reset overlapping slot");
 abort();
 }
 }
@@ -211,7 +211,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 }
 
 if (x == hvf_state->num_slots) {
-error_report("No free slots\n");
+error_report("No free slots");
 abort();
 }
 
@@ -221,7 +221,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 mem->region = area;
 
 if (do_hvf_set_memory(mem)) {
-error_report("Error registering new memory slot\n");
+error_report("Error registering new memory slot");
 abort();
 }
 }
@@ -884,7 +884,7 @@ int hvf_vcpu_exec(CPUState *cpu)
 break;
 }
 default:
-error_report("Unrecognized CR %d\n", cr);
+error_report("Unrecognized CR %d", cr);
 abort();
 }
 RIP(env) += ins_len;
@@ -930,7 +930,7 @@ int hvf_vcpu_exec(CPUState *cpu)
 env->error_code = 0;
 break;
 default:
-error_report("%llx: unhandled exit %llx\n", rip, exit_reason);
+error_report("%llx: unhandled exit %llx", rip, exit_reason);
 }
 } while (ret == 0);
 
-- 
2.14.3

[Qemu-devel] [PATCH v2 4/5] qdict: remove useless cast

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/qobject.cocci

Signed-off-by: Laurent Vivier 
---
 block/nvme.c | 11 +--
 monitor.c|  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 8bca57aae6..c4f3a7bc94 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -695,12 +695,11 @@ static void nvme_parse_filename(const char *filename, 
QDict *options,
 unsigned long ns;
 const char *slash = strchr(tmp, '/');
 if (!slash) {
-qdict_put(options, NVME_BLOCK_OPT_DEVICE,
-  qstring_from_str(tmp));
+qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, tmp);
 return;
 }
 device = g_strndup(tmp, slash - tmp);
-qdict_put(options, NVME_BLOCK_OPT_DEVICE, qstring_from_str(device));
+qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, device);
 g_free(device);
 namespace = slash + 1;
 if (*namespace && qemu_strtoul(namespace, NULL, 10, &ns)) {
@@ -708,8 +707,8 @@ static void nvme_parse_filename(const char *filename, QDict 
*options,
namespace);
 return;
 }
-qdict_put(options, NVME_BLOCK_OPT_NAMESPACE,
-  qstring_from_str(*namespace ? namespace : "1"));
+qdict_put_str(options, NVME_BLOCK_OPT_NAMESPACE,
+  *namespace ? namespace : "1");
 }
 }
 
@@ -1082,7 +1081,7 @@ static void nvme_refresh_filename(BlockDriverState *bs, 
QDict *opts)
  bs->drv->format_name);
 }
 
-qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
+qdict_put_str(opts, "driver", bs->drv->format_name);
 bs->full_open_options = opts;
 }
 
diff --git a/monitor.c b/monitor.c
index 6ccd2fc089..6e7667d82f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4316,7 +4316,7 @@ static QObject *get_qmp_greeting(Monitor *mon)
 /* Monitors that are not using IOThread won't support OOB */
 continue;
 }
-qlist_append(cap_list, qstring_from_str(QMPCapability_str(cap)));
+qlist_append_str(cap_list, QMPCapability_str(cap));
 }
 
 return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': %p}}",
-- 
2.14.3

Re: [Qemu-devel] [PATCH for-2.12 0/4] Turn OOB off for 2.12-rc1, revert OOB tests

[Christian Borntraeger]

On 03/23/2018 03:15 PM, Peter Xu wrote:
> On Fri, Mar 23, 2018 at 10:08:17PM +0800, Peter Xu wrote:
>> Temporarily disable OOB for 2.12 since it break (a lot of) things.
>> Luckily it's only a switch so not so hard. Meanwhile, revert all new
>> tests that checks against it.
>>
>> Tests done: all target builds and "make check" passed, but only on
>> x86_64 host.  It can pass all "raw" iotests, but some "qcow2" iotests
>> are still failing.  I can even reproduce many of the qcow2 fails even
>> before the whole OOB series, so I suppose that's something already
>> exists so I'll ignore.
>>
>> More tests are really welcomed.  Thanks,

Series
Tested-by: Christian Borntraeger 


> 
> CC Eric Blake too.
> 

Re: [Qemu-devel] [PATCH] vhost: Allow abutting regions

[Michael S. Tsirkin]

On Fri, Mar 23, 2018 at 12:46:37PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> My rework of section adding combines overlapping or abutting regions,
> but checks they're actually the same underlying RAM block.
> Fix the case where two blocks abut but don't overlap; that new region
> should get added (but not combined), but my previous patch was disallowing it.
> 
> Fixes: c1ece84e7c9
> 
> Reported-by: Alex Williamson 
> Signed-off-by: Dr. David Alan Gilbert 

I prefer adjacent and not abutting.

> ---
>  hw/virtio/vhost.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 250f886acb..fc9062a89f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -595,10 +595,15 @@ static void vhost_region_add_section(struct vhost_dev 
> *dev,
>  
> prev_sec->offset_within_address_space,
>  prev_sec->offset_within_region);
>  } else {
> -error_report("%s: Overlapping but not coherent sections "
> - "at %"PRIx64,
> - __func__, mrs_gpa);
> -return;
> +/* abutting regions are fine, but overlapping ones with
> + * different blocks/offsets shouldn't happen
> + */
> +if (mrs_gpa != prev_gpa_end + 1) {
> +error_report("%s: Overlapping but not coherent sections "
> + "at %"PRIx64,
> + __func__, mrs_gpa);
> +return;
> +}
>  }
>  }
>  }
> -- 
> 2.14.3

Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment

[Vitaly Kuznetsov]

Eduardo Habkost  writes:

> On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote:
>> We can also expose Hyper-V frequency MSRs when reenlightenment feature is
>> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC
>> page clocksources to its guests.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>> ---
>> - Expose frequency MSRs only when either INVTSC or Reenlightenment is
>>   provided [Paolo Bonzini]
>> ---
>>  target/i386/kvm.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 75f4e1d69e..2c3c19d690 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -651,7 +651,8 @@ static int hyperv_handle_properties(CPUState *cs)
>>  env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
>>  env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
>>  
>> -if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
>> +if (has_msr_hv_frequencies && env->tsc_khz &&
>
> Why is the check for env->tsc_khz necessary?
>
> Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported
> by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra
> safety?
>

Yes,

I didn't experiment with passing '0' to Windows but in general it
doesn't sound like a good idea. 

>> +(tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) {
>>  env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
>>  env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
>>  }
>> -- 
>> 2.14.3
>> 

-- 
  Vitaly

Re: [Qemu-devel] [PATCH v2 0/7] ramfb: simple boot framebuffer, no legacy vga

[Gerd Hoffmann]

  Hi,

> I believe the only point of this device model (and the associated guest
> fw driver) is Windows-on-KVM/aarch64.

The other one is vgpu boot display.

And maybe killing vga emulation.  Well, at least be able to not use it
any more for UEFI guests.  It's so much crazy stuff in vga emulation
which isn't used by anyone these days (especially on uefi which doesn't
even need vga text mode any more), except by guys who try to find holes
in qemu for a host escape ...

> Would it be possible to make this stuff available for testing to the
> guy(s) who reported
> ? (Registration in
> the TianoCore bugzilla is open, and once you log in, you can read email
> addresses, and/or comment on BZs of course.)

I plan to look at arm, but that likely will happen after easter holidays
(I'm offline next week).

> > We should make sure that any device model that combines ramfb with
> > another PCI display device is not matched by the OVMF driver for that
> > PCI display device. IOW, we should use separate PCI IDs or subsystem
> > IDs (I don't recall the details off-hand). I'd like to avoid messing
> > with the current device probing code in OVMF. It just wouldn't be nice
> > if two independent drivers (e.g. VirtioGpuDxe and a supposed
> > "RamFbDxe") bound the two interfaces at the same time.
> 
> For example, virtio-vga is already problematic like this (it could be
> driven by both Virtio10Dxe+VirtioGpuDxe, and QemuVideoDxe), and it had
> to be hacked around in Virtio10Dxe. So I'm strongly in favor of a device
> model that clearly looks like one device and one device only to the full
> set of edk2 drivers, without cross-driver hacks.

Yep, that is one of the things which still need to be sorted.  The
approach of the experimental firmware builds to just disable the
QemuVideo and VirtioGpu drivers clearly doesn't fly for anything but the
initial testing.

Giving out different IDs to the ramfb-enabled device variants would be
one option.  Not fully sure whenever that works for virtio-gpu.  But
given that there are no legacy/transitional virtio-gpu devices I think
we are free to change the subsystem ID as we like.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] vhost: Allow abutting regions

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Fri, Mar 23, 2018 at 12:46:37PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > My rework of section adding combines overlapping or abutting regions,
> > but checks they're actually the same underlying RAM block.
> > Fix the case where two blocks abut but don't overlap; that new region
> > should get added (but not combined), but my previous patch was disallowing 
> > it.
> > 
> > Fixes: c1ece84e7c9
> > 
> > Reported-by: Alex Williamson 
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Can we replace abutting with adjacent pls?

Sure; do you want to fix that in commit?

Dave

> > ---
> >  hw/virtio/vhost.c | 13 +
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 250f886acb..fc9062a89f 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -595,10 +595,15 @@ static void vhost_region_add_section(struct vhost_dev 
> > *dev,
> >  
> > prev_sec->offset_within_address_space,
> >  prev_sec->offset_within_region);
> >  } else {
> > -error_report("%s: Overlapping but not coherent sections "
> > - "at %"PRIx64,
> > - __func__, mrs_gpa);
> > -return;
> > +/* abutting regions are fine, but overlapping ones with
> > + * different blocks/offsets shouldn't happen
> > + */
> > +if (mrs_gpa != prev_gpa_end + 1) {
> > +error_report("%s: Overlapping but not coherent 
> > sections "
> > + "at %"PRIx64,
> > + __func__, mrs_gpa);
> > +return;
> > +}
> >  }
> >  }
> >  }
> > -- 
> > 2.14.3
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH for-2.12] hw/riscv: Fix crashes with "-nodefaults"

[Paolo Bonzini]

On 23/03/2018 15:28, Peter Maydell wrote:
>> Note it's a null "backend", not necessarily a null "character device".
>> Your proposal, namely ensuring that be->chr == NULL is handled properly
>> in qemu_chr_fe_init, would be just fine.
> 
> Hmm, the chardev layer code seems to have more ends than I expect.
> The "frontend" is the UART model, right, and I thought the
> "backend" was the TCP/UDP/serial port/stdio/etc end of things,
> but those seem to be Chardevs? If those aren't the backend,
> then what is?

The naming of Chardev/CharBackend was modeled after
BlockDriverState/BlockBackend, and it's not great but at least it's
consistent. :/

The CharBackend struct essentially connects a Chardev and its user.  It
ought to be possible for the Chardev to be NULL.

Paolo

> What I'd like, anyway, is that every UART model can cope with being
> setup with a NULL 'chardev' property, and ideally that that doesn't
> require a lot of extra code per-UART, and doesn't require each UART
> to create a TYPE_CHARDEV_NULL.
> 

Re: [Qemu-devel] [PATCH v2 0/7] ramfb: simple boot framebuffer, no legacy vga

[Laszlo Ersek]

On 03/23/18 15:51, Gerd Hoffmann wrote:
>   Hi,
> 
>> I believe the only point of this device model (and the associated guest
>> fw driver) is Windows-on-KVM/aarch64.
> 
> The other one is vgpu boot display.

Interesting. I know nearly nothing about vgpu, but I hoped it'd come
with its own UEFI GOP driver in the ROM BAR; similarly to assigned
physical GPUs. I thought the oprom would originate from the physical GPU
(with the help of the host kernel), out of which the vgpu was carved out.

Is that far-fetched?

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] vhost: Allow abutting regions

[Michael S. Tsirkin]

On Fri, Mar 23, 2018 at 02:53:38PM +, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > On Fri, Mar 23, 2018 at 12:46:37PM +, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > My rework of section adding combines overlapping or abutting regions,
> > > but checks they're actually the same underlying RAM block.
> > > Fix the case where two blocks abut but don't overlap; that new region
> > > should get added (but not combined), but my previous patch was 
> > > disallowing it.
> > > 
> > > Fixes: c1ece84e7c9
> > > 
> > > Reported-by: Alex Williamson 
> > > Signed-off-by: Dr. David Alan Gilbert 
> > 
> > Can we replace abutting with adjacent pls?
> 
> Sure; do you want to fix that in commit?
> 
> Dave

Rather.

> > > ---
> > >  hw/virtio/vhost.c | 13 +
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index 250f886acb..fc9062a89f 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -595,10 +595,15 @@ static void vhost_region_add_section(struct 
> > > vhost_dev *dev,
> > >  
> > > prev_sec->offset_within_address_space,
> > >  prev_sec->offset_within_region);
> > >  } else {
> > > -error_report("%s: Overlapping but not coherent sections "
> > > - "at %"PRIx64,
> > > - __func__, mrs_gpa);
> > > -return;
> > > +/* abutting regions are fine, but overlapping ones with
> > > + * different blocks/offsets shouldn't happen
> > > + */
> > > +if (mrs_gpa != prev_gpa_end + 1) {
> > > +error_report("%s: Overlapping but not coherent 
> > > sections "
> > > + "at %"PRIx64,
> > > + __func__, mrs_gpa);
> > > +return;
> > > +}
> > >  }
> > >  }
> > >  }
> > > -- 
> > > 2.14.3
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK