Re: [Qemu-devel] [RFC] ppc: define spapr core types statically

[David Gibson]

On Wed, Sep 27, 2017 at 06:18:34PM +0200, Greg Kurz wrote:
> On Wed, 27 Sep 2017 13:49:17 +0200
> Igor Mammedov  wrote:
> 
> 
> This looks nice indeed (just one remark, see below).
> 
> Will you re-post as a patch series or do you prefer I do it ?

Looks good to me as well.

> 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  include/hw/ppc/spapr_cpu_core.h |  5 ++-
> >  hw/ppc/spapr.c  |  6 +--
> >  hw/ppc/spapr_cpu_core.c | 93 
> > -
> >  target/ppc/kvm.c| 11 -
> >  4 files changed, 41 insertions(+), 74 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr_cpu_core.h 
> > b/include/hw/ppc/spapr_cpu_core.h
> > index 93051e9..42765de 100644
> > --- a/include/hw/ppc/spapr_cpu_core.h
> > +++ b/include/hw/ppc/spapr_cpu_core.h
> > @@ -21,6 +21,8 @@
> >  #define SPAPR_CPU_CORE_GET_CLASS(obj) \
> >   OBJECT_GET_CLASS(sPAPRCPUCoreClass, (obj), TYPE_SPAPR_CPU_CORE)
> >  
> > +#define SPAPR_CPU_CORE_TYPE_NAME(model) model "-" TYPE_SPAPR_CPU_CORE
> > +
> >  typedef struct sPAPRCPUCore {
> >  /*< private >*/
> >  CPUCore parent_obj;
> > @@ -32,9 +34,8 @@ typedef struct sPAPRCPUCore {
> >  
> >  typedef struct sPAPRCPUCoreClass {
> >  DeviceClass parent_class;
> > -ObjectClass *cpu_class;
> > +const char *cpu_type;
> >  } sPAPRCPUCoreClass;
> >  
> >  char *spapr_get_cpu_core_type(const char *model);
> > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
> >  #endif
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8c1a437..00cfe9a 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3125,8 +3125,7 @@ void spapr_core_release(DeviceState *dev)
> >  if (smc->pre_2_10_has_unused_icps) {
> >  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> > -const char *typename = object_class_get_name(scc->cpu_class);
> > -size_t size = object_type_get_instance_size(typename);
> > +size_t size = object_type_get_instance_size(scc->cpu_type);
> >  int i;
> >  
> >  for (i = 0; i < cc->nr_threads; i++) {
> > @@ -3222,8 +3221,7 @@ static void spapr_core_plug(HotplugHandler 
> > *hotplug_dev, DeviceState *dev,
> >  
> >  if (smc->pre_2_10_has_unused_icps) {
> >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> > -const char *typename = object_class_get_name(scc->cpu_class);
> > -size_t size = object_type_get_instance_size(typename);
> > +size_t size = object_type_get_instance_size(scc->cpu_type);
> >  int i;
> >  
> >  for (i = 0; i < cc->nr_threads; i++) {
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 5bea4c9..8a18eaf 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -104,8 +104,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState 
> > *dev, Error **errp)
> >  {
> >  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> > -const char *typename = object_class_get_name(scc->cpu_class);
> > -size_t size = object_type_get_instance_size(typename);
> > +size_t size = object_type_get_instance_size(scc->cpu_type);
> >  CPUCore *cc = CPU_CORE(dev);
> >  int i;
> >  
> > @@ -166,8 +165,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> > Error **errp)
> >  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> >  CPUCore *cc = CPU_CORE(OBJECT(dev));
> > -const char *typename = object_class_get_name(scc->cpu_class);
> > -size_t size = object_type_get_instance_size(typename);
> > +size_t size = object_type_get_instance_size(scc->cpu_type);
> >  Error *local_err = NULL;
> >  void *obj;
> >  int i, j;
> > @@ -186,7 +184,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> > Error **errp)
> >  
> >  obj = sc->threads + i * size;
> >  
> > -object_initialize(obj, size, typename);
> > +object_initialize(obj, size, scc->cpu_type);
> >  cs = CPU(obj);
> >  cpu = POWERPC_CPU(cs);
> >  cs->cpu_index = cc->core_id + i;
> > @@ -231,42 +229,12 @@ err:
> >  error_propagate(errp, local_err);
> >  }
> >  
> > -static const char *spapr_core_models[] = {
> > -/* 970 */
> > -"970_v2.2",
> > -
> > -/* 970MP variants */
> > -"970mp_v1.0",
> > -"970mp_v1.1",
> > -
> > -/* POWER5+ */
> > -"power5+_v2.1",
> > -
> > -/* POWER7 */
> > -"power7_v2.3",
> > -
> > -/* POWER7+ */
> > -"power7+_v2.1",
> > -
> > -/* POWER8 */
> > -"power8_v2.0",
> > -
> > -/* POWER8E */
> > -"power8e_v2.1",
> > -
> > -/* POWER8NVL */
> > -"power8nvl_v1.0",
> > -
> > -/* POWER9 */
> > -"power9_v1.0",
> > -};
> > -
> >  static 

Re: [Qemu-devel] [PATCH RFC 2/3] s390x/tcg: low-address protection support

[Thomas Huth]

On 27.09.2017 19:00, David Hildenbrand wrote:
> This is a neat way to implement low address protection, whereby
> only the first 512 bytes of the first two pages (each 4096 bytes) of
> every address space are protected.
> 
> Store a tec of 0 for the access exception, this is what is defined by
> Enhanced Suppression on Protection in case of a low address protection
> (Bit 61 set to 0, rest undefined).
> 
> We have to make sure to to pass the access address, not the masked page
> address into mmu_translate*().
> 
> Drop the check from testblock. So we can properly test this via
> kvm-unit-tests.
> 
> This will check every access going through one of the MMUs.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/excp_helper.c |  3 +-
>  target/s390x/mem_helper.c  |  8 
>  target/s390x/mmu_helper.c  | 96 
> +-
>  3 files changed, 62 insertions(+), 45 deletions(-)
[...]
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 9daa0fd8e2..44a15449d2 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -106,6 +106,37 @@ static void trigger_page_fault(CPUS390XState *env, 
> target_ulong vaddr,
>  trigger_access_exception(env, type, ilen, tec);
>  }
>  
> +/* check whether the address would be proteted by Low-Address Protection */
> +static bool is_low_address(uint64_t addr)
> +{
> +return addr < 512 || (addr >= 4096 && addr < 4607);
> +}

I like the check from the kernel sources better:

static inline int is_low_address(unsigned long ga)
{
/* Check for address ranges 0..511 and 4096..4607 */
return (ga & ~0x11fful) == 0;
}

... that might result in slightly faster code (depending on the
compiler, of course).

 Thomas

Re: [Qemu-devel] [PATCH RFC 3/3] s390x/tcg: make STFL store into the lowcore

[Thomas Huth]

On 27.09.2017 20:46, David Hildenbrand wrote:
> On 27.09.2017 19:52, Richard Henderson wrote:
>> On 09/27/2017 10:00 AM, David Hildenbrand wrote:
>>> Using virtual memory access is wrong and will soon include low-address
>>> protection checks, which is to be bypassed for STFL.
>>>
>>> This was originally part of a bigger STFL(E) refactoring.
>>>
>>> Signed-off-by: David Hildenbrand 
>>> ---
>>>  target/s390x/helper.h  | 2 +-
>>>  target/s390x/misc_helper.c | 7 ++-
>>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> Need to sort this patch first, so that the series is bisectable.
> 
> Right, this should become #2.
> 
>>
>>>  DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
>>>  DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
>>>  DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
>>> -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>>>  DEF_HELPER_2(stfle, i32, env, i64)
>>>  DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
>>>  DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
>>> @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, 
>>> i64, i64)
>>>  DEF_HELPER_1(per_check_exception, void, env)
>>>  DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
>>>  DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
>>> +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>>>  
>>
>> Why?  Otherwise,
> 
> struct LowCore is only available for !CONFIG_USER_ONLY. Therefore I also
> have to move the helper declaration into !CONFIG_USER_ONLY.

You should mention that in the patch description, that it is a
privileged instruction and thus you also mark it here accordingly.

With that update:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v3 0/9] Support the Capstone disassembler

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Subject: [Qemu-devel] [PATCH v3 0/9] Support the Capstone disassembler
Message-id: 20170926201427.2833-1-richard.hender...@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20170913160333.23622-1-ebl...@redhat.com -> 
patchew/20170913160333.23622-1-ebl...@redhat.com
 * [new tag] patchew/20170926201427.2833-1-richard.hender...@linaro.org 
-> patchew/20170926201427.2833-1-richard.hender...@linaro.org
Switched to a new branch 'test'
1f3a418 disas: Add capstone as submodule
66a7956 disas: Remove monitor_disas_is_physical
eac72b9 ppc: Support Capstone in disas_set_info
4b02a0c arm: Support Capstone in disas_set_info
b06b367 i386: Support Capstone in disas_set_info
6398efc disas: Support the Capstone disassembler library
0c00565 disas: Remove unused flags arguments
6eff19f target/ppc: Convert to disas_set_info hook
e579e51 target/i386: Convert to disas_set_info hook

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=19724
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-60zk3pqy/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
giflib-4.1.6-15.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
libidn-1.33-1.fc25.s390x
slang-2.3.0-7.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
parted-3.2-21.fc25.s390x
flex-2.6.0-3.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
perl-Pod-Simple-3.35-1.fc25.noarch
python2-simplejson-3.10.0-1.fc25.s390x
brltty-5.4-2.fc25.s390x
librados2-10.2.4-2.fc25.s390x
tcp_wrappers-7.6-83.fc25.s390x
libcephfs_jni1-10.2.4-2.fc25.s390x
nettle-devel-3.3-1.fc25.s390x
bzip2-devel-1.0.6-21.fc25.s390x
libuuid-2.28.2-2.fc25.s390x
python3-dnf-1.1.10-6.fc25.noarch
texlive-kpathsea-doc-svn41139-33.fc25.1.noarch
openssh-7.4p1-4.fc25.s390x
texlive-kpathsea-bin-svn40473-33.20160520.fc25.1.s390x
texlive-graphics-svn41015-33.fc25.1.noarch
texlive-dvipdfmx-def-svn40328-33.fc25.1.noarch
texlive-mfware-svn40768-33.fc25.1.noarch
texlive-texlive-scripts-svn41433-33.fc25.1.noarch
texlive-euro-svn22191.1.1-33.fc25.1.noarch
texlive-etex-svn37057.0-33.fc25.1.noarch
texlive-iftex-svn29654.0.2-33.fc25.1.noarch
texlive-palatino-svn31835.0-33.fc25.1.noarch
texlive-texlive-docindex-svn41430-33.fc25.1.noarch
texlive-xunicode-svn30466.0.981-33.fc25.1.noarch
texlive-koma-script-svn41508-33.fc25.1.noarch
texlive-pst-grad-svn15878.1.06-33.fc25.1.noarch
texlive-pst-blur-svn15878.2.0-33.fc25.1.noarch
texlive-jknapltx-svn19440.0-33.fc25.1.noarch
texinfo-6.1-4.fc25.s390x
openssl-devel-1.0.2k-1.fc25.s390x
jansson-2.10-2.fc25.s390x
fedora-repos-25-4.noarch
perl-Errno-1.25-387.fc25.s390x
acl-2.2.52-13.fc25.s390x
systemd-pam-231-17.fc25.s390x
NetworkManager-libnm-1.4.4-5.fc25.s390x
poppler-0.45.0-5.fc25.s390x
ccache-3.3.4-1.fc25.s390x
valgrind-3.12.0-9.fc25.s390x
perl-open-1.10-387.fc25.noarch
libgcc-6.4.1-1.fc25.s390x
libsoup-2.56.1-1.fc25.s390x
libstdc++-devel-6.4.1-1.fc25.s390x
libobjc-6.4.1-1.fc25.s390x
python2-rpm-4.13.0.1-2.fc25.s390x
python2-gluster-3.10.5-1.fc25.s390x
rpm-build-4.13.0.1-2.fc25.s390x
glibc-static-2.24-10.fc25.s390x
lz4-1.8.0-1.fc25.s390x
xapian-core-libs-1.2.24-1.fc25.s390x
elfutils-libelf-devel-0.169-1.fc25.s390x
nss-softokn-3.32.0-1.2.fc25.s390x
pango-1.40.9-1.fc25.s390x
glibc-debuginfo-common-2.24-10.fc25.s390x
libaio-0.3.110-6.fc24.s390x
libfontenc-1.1.3-3.fc24.s390x
lzo-2.08-8.fc24.s390x
isl-0.14-5.fc24.s390x
libXau-1.0.8-6.fc24.s390x
linux-atm-libs-2.5.1-14.fc24.s390x
libXext-1.3.3-4.fc24.s390x
libXxf86vm-1.1.4-3.fc24.s390x
bison-3.0.4-4.fc24.s390x
perl-srpm-macros-1-20.fc25.noarch
gawk-4.1.3-8.fc25.s390x
libwayland-client-1.12.0-1.fc25.s390x
perl-Exporter-5.72-366.fc25.noarch
perl-version-0.99.17-1.fc25.s390x
fftw-libs-double-3.3.5-3.fc25.s390x
libssh2-1.8.0-1.fc25.s390x

Re: [Qemu-devel] [PATCH v2 3/5] pci: Add INTERFACE_PCIE_DEVICE to all PCIe devices

[David Gibson]

On Wed, Sep 27, 2017 at 04:56:33PM -0300, Eduardo Habkost wrote:
> Change all devices that set is_express=1 to implement
> INTERFACE_PCIE_DEVICE.
> 
> Cc: Keith Busch 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Dmitry Fleytman 
> Cc: Jason Wang 
> Cc: "Michael S. Tsirkin" 
> Cc: Marcel Apfelbaum 
> Cc: Paul Burton 
> Cc: Paolo Bonzini 
> Cc: Hannes Reinecke 
> Cc: qemu-bl...@nongnu.org
> Reviewed-by: Alistair Francis 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: David Gibson 

> ---
> Changes v1 -> v2:
> * base-xhci is marked as hybrid, now (in another patch)
> * Included pcie-pci-bridge
> ---
>  hw/block/nvme.c| 4 
>  hw/net/e1000e.c| 4 
>  hw/pci-bridge/pcie_pci_bridge.c| 1 +
>  hw/pci-bridge/pcie_root_port.c | 4 
>  hw/pci-bridge/xio3130_downstream.c | 4 
>  hw/pci-bridge/xio3130_upstream.c   | 4 
>  hw/pci-host/xilinx-pcie.c  | 4 
>  hw/scsi/megasas.c  | 6 ++
>  8 files changed, 31 insertions(+)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 9aa32692a3..441e21ed1f 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1110,6 +1110,10 @@ static const TypeInfo nvme_info = {
>  .instance_size = sizeof(NvmeCtrl),
>  .class_init= nvme_class_init,
>  .instance_init = nvme_instance_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void nvme_register_types(void)
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 6c42b4478c..81f7934a59 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -708,6 +708,10 @@ static const TypeInfo e1000e_info = {
>  .instance_size = sizeof(E1000EState),
>  .class_init = e1000e_class_init,
>  .instance_init = e1000e_instance_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void e1000e_register_types(void)
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index 9aa5cc3e45..88db143633 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -180,6 +180,7 @@ static const TypeInfo pcie_pci_bridge_info = {
>  .class_init = pcie_pci_bridge_class_init,
>  .interfaces = (InterfaceInfo[]) {
>  { TYPE_HOTPLUG_HANDLER },
> +{ INTERFACE_PCIE_DEVICE },
>  { },
>  }
>  };
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 4d588cb22e..9b6e4ce512 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -161,6 +161,10 @@ static const TypeInfo rp_info = {
>  .class_init= rp_class_init,
>  .abstract  = true,
>  .class_size = sizeof(PCIERootPortClass),
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void rp_register_types(void)
> diff --git a/hw/pci-bridge/xio3130_downstream.c 
> b/hw/pci-bridge/xio3130_downstream.c
> index e706f36cb7..7d2f7629c1 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -195,6 +195,10 @@ static const TypeInfo xio3130_downstream_info = {
>  .name  = "xio3130-downstream",
>  .parent= TYPE_PCIE_SLOT,
>  .class_init= xio3130_downstream_class_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void xio3130_downstream_register_types(void)
> diff --git a/hw/pci-bridge/xio3130_upstream.c 
> b/hw/pci-bridge/xio3130_upstream.c
> index a052224bbf..227997ce46 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -166,6 +166,10 @@ static const TypeInfo xio3130_upstream_info = {
>  .name  = "x3130-upstream",
>  .parent= TYPE_PCIE_PORT,
>  .class_init= xio3130_upstream_class_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void xio3130_upstream_register_types(void)
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 4613dda1d2..7659253090 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -317,6 +317,10 @@ static const TypeInfo xilinx_pcie_root_info = {
>  .parent = TYPE_PCI_BRIDGE,
>  .instance_size = sizeof(XilinxPCIERoot),
>  .class_init = xilinx_pcie_root_class_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ }
> +},
>  };
>  
>  static void xilinx_pcie_register(void)
> diff --git 

Re: [Qemu-devel] [PATCH v2 1/5] pci: conventional-pci-device and pci-express-device interfaces

[David Gibson]

On Wed, Sep 27, 2017 at 04:56:31PM -0300, Eduardo Habkost wrote:
> Those two interfaces will be used to indicate which device types
> support Conventional PCI or PCI Express buses.  Management
> software will be able to use the qom-list-types QMP command to
> query that information.
> 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: David Gibson 

> ---
> Changes v1 -> v2:
> * s/legacy/conventional/
>   * Suggested-by: Alex Williamson 
> ---
>  include/hw/pci/pci.h |  6 ++
>  hw/pci/pci.c | 12 
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index aa7ef9cf69..8d02a0a383 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -198,6 +198,12 @@ enum {
>  #define PCI_DEVICE_GET_CLASS(obj) \
>   OBJECT_GET_CLASS(PCIDeviceClass, (obj), TYPE_PCI_DEVICE)
>  
> +/* Implemented by devices that can be plugged on PCI Express buses */
> +#define INTERFACE_PCIE_DEVICE "pci-express-device"
> +
> +/* Implemented by devices that can be plugged on Conventional PCI buses */
> +#define INTERFACE_CONVENTIONAL_PCI_DEVICE "conventional-pci-device"
> +
>  typedef struct PCIINTxRoute {
>  enum {
>  PCI_INTX_ENABLED,
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 1e6fb88eba..1b08e18205 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -170,6 +170,16 @@ static const TypeInfo pci_bus_info = {
>  .class_init = pci_bus_class_init,
>  };
>  
> +static const TypeInfo pcie_interface_info = {
> +.name  = INTERFACE_PCIE_DEVICE,
> +.parent= TYPE_INTERFACE,
> +};
> +
> +static const TypeInfo conventional_pci_interface_info = {
> +.name  = INTERFACE_CONVENTIONAL_PCI_DEVICE,
> +.parent= TYPE_INTERFACE,
> +};
> +
>  static const TypeInfo pcie_bus_info = {
>  .name = TYPE_PCIE_BUS,
>  .parent = TYPE_PCI_BUS,
> @@ -2667,6 +2677,8 @@ static void pci_register_types(void)
>  {
>  type_register_static(_bus_info);
>  type_register_static(_bus_info);
> +type_register_static(_pci_interface_info);
> +type_register_static(_interface_info);
>  type_register_static(_device_type_info);
>  }
>  

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 4/5] pci: Add INTERFACE_CONVENTIONAL_PCI_DEVICE to Conventional PCI devices

[David Gibson]

On Wed, Sep 27, 2017 at 04:56:34PM -0300, Eduardo Habkost wrote:
> Add INTERFACE_CONVENTIONAL_PCI_DEVICE to all direct subtypes of
> TYPE_PCI_DEVICE, except:
> 
> 1) The ones that already have INTERFACE_PCIE_DEVICE set:
> 
> * base-xhci
> * e1000e
> * nvme
> * pvscsi
> * vfio-pci
> * virtio-pci
> * vmxnet3
> 
> 2) base-pci-bridge
> 
> Not all PCI bridges are Conventional PCI devices, so
> INTERFACE_CONVENTIONAL_PCI_DEVICE is added only to the subtypes
> that are actually Conventional PCI:
> 
> * dec-21154-p2p-bridge
> * i82801b11-bridge
> * pbm-bridge
> * pci-bridge
> 
> The direct subtypes of base-pci-bridge not touched by this patch
> are:
> 
> * xilinx-pcie-root: Already marked as PCIe-only.
> * pcie-pci-bridge: Already marked as PCIe-only.
> * pcie-port: all non-abstract subtypes of pcie-port are already
>   marked as PCIe-only devices.
> 
> 3) megasas-base
> 
> Not all megasas devices are Conventional PCI devices, so the
> interface names are added to the subclasses registered by
> megasas_register_types(), according to information in the
> megasas_devices[] array.
> 
> "megasas-gen2" already implements INTERFACE_PCIE_DEVICE, so add
> INTERFACE_CONVENTIONAL_PCI_DEVICE only to "megasas".
> 
> Acked-by: Alberto Garcia 
> Acked-by: John Snow 
> Acked-by: Anthony PERARD 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: David Gibson 

and for the ppc devices

Acked-by: David Gibson 

> ---
> Changes v1 -> v2:
> * s/legacy/conventional/
>   * Suggested-by: Alex Williamson 
> * Note about pcie-pci-bridge on commit message.
> * New devices: sungem, sunhme
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Igor Mammedov 
> Cc: Gerd Hoffmann 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: John Snow 
> Cc: Alberto Garcia 
> Cc: Aurelien Jarno 
> Cc: Yongbok Kim 
> Cc: Jiri Slaby 
> Cc: Alexander Graf 
> Cc: Marcel Apfelbaum 
> Cc: Jason Wang 
> Cc: Jiri Pirko 
> Cc: "Hervé Poussineau" 
> Cc: Peter Maydell 
> Cc: David Gibson 
> Cc: Hannes Reinecke 
> Cc: Mark Cave-Ayland 
> Cc: Artyom Tarasenko 
> Cc: Alex Williamson 
> Cc: qemu-devel@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: qemu-...@nongnu.org
> ---
>  hw/acpi/piix4.c |  1 +
>  hw/audio/ac97.c |  4 
>  hw/audio/es1370.c   |  4 
>  hw/audio/intel-hda.c|  4 
>  hw/char/serial-pci.c| 12 
>  hw/display/cirrus_vga.c |  4 
>  hw/display/qxl.c|  4 
>  hw/display/sm501.c  |  4 
>  hw/display/vga-pci.c|  4 
>  hw/display/vmware_vga.c |  4 
>  hw/i2c/smbus_ich9.c |  4 
>  hw/i386/amd_iommu.c |  4 
>  hw/i386/kvm/pci-assign.c|  4 
>  hw/i386/pc_piix.c   |  4 
>  hw/i386/xen/xen_platform.c  |  4 
>  hw/i386/xen/xen_pvdevice.c  |  4 
>  hw/ide/ich.c|  4 
>  hw/ide/pci.c|  4 
>  hw/ipack/tpci200.c  |  4 
>  hw/isa/i82378.c |  4 
>  hw/isa/lpc_ich9.c   |  1 +
>  hw/isa/piix4.c  |  4 
>  hw/isa/vt82c686.c   | 16 
>  hw/mips/gt64xxx_pci.c   |  4 
>  hw/misc/edu.c   |  5 +
>  hw/misc/ivshmem.c   |  4 
>  hw/misc/macio/macio.c   |  4 
>  hw/misc/pci-testdev.c   |  4 
>  hw/net/e1000.c  |  4 
>  hw/net/eepro100.c   |  4 
>  hw/net/ne2000.c |  4 
>  hw/net/pcnet-pci.c  |  4 
>  hw/net/rocker/rocker.c  |  4 
>  hw/net/rtl8139.c|  4 
>  hw/net/sungem.c |  4 
>  hw/net/sunhme.c |  4 
>  hw/pci-bridge/dec.c |  8 
>  hw/pci-bridge/i82801b11.c   |  4 
>  hw/pci-bridge/pci_bridge_dev.c  |  1 +
>  hw/pci-bridge/pci_expander_bridge.c |  8 
>  hw/pci-host/apb.c   |  8 
>  hw/pci-host/bonito.c  

Re: [Qemu-devel] [PATCH] spapr: move registration of "host" CPU core type to machine code

[David Gibson]

On Wed, Sep 27, 2017 at 02:11:44PM +0200, Igor Mammedov wrote:
> On Wed, 27 Sep 2017 16:39:03 +1000
> David Gibson  wrote:
> 
> > On Tue, Sep 26, 2017 at 10:29:36AM +0200, Igor Mammedov wrote:
> > > On Tue, 26 Sep 2017 09:19:28 +0200
> > > Greg Kurz  wrote:
> > >   
> > > > On Tue, 26 Sep 2017 12:57:39 +1000
> > > > David Gibson  wrote:
> > > >   
> > > > > On Mon, Sep 25, 2017 at 11:47:33AM +0200, Greg Kurz wrote:
> > > > > > The CPU core abstraction belongs to the machine code. This also gets
> > > > > > rid of some code duplication.
> > > > > > 
> > > > > > Signed-off-by: Greg Kurz 
> > > > > > ---
> > > > > > 
> > > > > > hw/ppc/spapr_cpu_core.h is also included elsewhere in 
> > > > > > target/ppc/kvm.c
> > > > > > but this is already handled by the following cleanup patch:  
> > > > > 
> > > > > I don't really see what the advantage of this is.  As others have
> > > > > pointed out it leads to the host type being registered very late,
> > > > > which could cause problems.
> > > > > 
> > > > 
> > > > Well, the goal was to consolidate the code to register sPAPRCPUCore 
> > > > types in
> > > > the spapr code, instead of open-coding it in spapr_cpu_core.c and 
> > > > kvm.c... 
> > > > 
> > > > But now I realize that delaying the registration even more is a bad 
> > > > idea. And,
> > > > the other way round, registering a static type earlier as asked by Igor 
> > > > would
> > > > require all parent types to be already registered, which seems to be 
> > > > impossible
> > > > to guarantee with the current code.  
> > > so I'll leave this code as is for now as as it's a bit of topic
> > > wrt my work on cpu_model removal (or I'll try not to touch it until I 
> > > have to)
> > >   
> > > > Maybe we could at least have kvm_ppc_register_host_cpu_type() to call a
> > > > function in spapr_cpu_core.c instead of duplicating the registration 
> > > > code ?  
> > > Is it possible for other boards (non core based) to use host CPU?
> > > 
> > > but for simplicity sake I'd lave it where it's now.
> > > 
> > > BTW I have an additional question:
> > > what PPC boards are supposed to support KVM/host cpu?  
> > 
> > In theory, any board, but your host has to have a cpu that's suitable
> > for the board in question.  So, if your host is a POWER8, you can use
> > -cpu host with a machine type which supports a POWER8 cpu; which is
> > basically just "pseries".  You couldn't use -cpu host with one of the
> > embedded board types, since they don't expect a POWER8 vCPU.
> > 
> > If you were on an e500 based host, you could use -cpu host with one of
> > the various e500 based machine types.  You couldn't use it with
> > "pseries", since that will only work with a POWER7/8/9 vCPU.
> > 
> > The issue is muddied by the fact that KVM HV (the fast KVM
> > implementation), a) will only work with a PAPR guest (i.e. pseries
> > machine type) and b) can't present a vCPU to the guest that differs
> > from the host (this is a hardware limitation).  If these conditions
> > aren't met, the host kernel should automatically fall back to KVM PR
> > (or fail outright if it's not available).  Of course then it's further
> > muddied by the fact that KVM PR has _some_ ability to present a
> > different vCPU to the guest than is on the host, but it has some curly
> > limitations that we might or might not be able to detect in advance.
> > 
> > And, it's muddied even further by the fact that some of the CPU
> > "variants" are in fact identical cores - the differences are just in
> > peripherals, pin-level connections, cache etc. which the guest can't
> > see and doesn't care about.  Unfortunately, there aren't separate ways
> > for the CPU to advertise core version and other things, so even
> > trivial variants like this will have a different PVR value as a rule.
> > 
> > > (especially taking in account that aliases get patched to it as well,
> > > which I very much dislike since user asked for one CPU but got
> > > another  
> > 
> > Uh.. what?  You should never get something different than what you
> > asked for.  The worst that can happen is if you request something
> > fuzzy, like "POWER8" instead of "POWER8E_v2.0" then exactly which
> > thing in that family you get depends on the host.
> that's what bothers me since when alias is used /like POWER8/
> user gets 'some' cpu and he/she has to be aware that it might be not
> the exactly the same cpu on another host.

Well, yeah.  We did this for the benefit of libvirt, which really
wants to specify "POWER8" or "POWER9" or whatever instead of "host".
KVM HV only works with the host CPU, so it doesn't work for it to
specify some specific cpu model.

> > > and when the same happens on another host migration probably would be
> > > screwed up)  
> > 
> > So, we can migrate between different CPUs if they're similar enough
> > (say, everything within the POWER8 family).  The guest is able to see
> 

Re: [Qemu-devel] [PATCH v2 5/5] pci: Validate interfaces on base_class_init

[David Gibson]

On Wed, Sep 27, 2017 at 04:56:35PM -0300, Eduardo Habkost wrote:
> Make sure we don't forget to add the Conventional PCI or PCI
> Express interface names on PCI device classes in the future.
> 
> Signed-off-by: Eduardo Habkost 

Revieed-by: David Gibson 

> ---
> Changes v1 -> v2:
> * s/legacy/conventional/
>   * Suggested-by: Alex Williamson 
> ---
>  hw/pci/pci.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 1b08e18205..5ed3c8dca4 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2547,6 +2547,17 @@ static void pci_device_class_init(ObjectClass *klass, 
> void *data)
>  pc->realize = pci_default_realize;
>  }
>  
> +static void pci_device_class_base_init(ObjectClass *klass, void *data)
> +{
> +if (!object_class_is_abstract(klass)) {
> +ObjectClass *conventional =
> +object_class_dynamic_cast(klass, 
> INTERFACE_CONVENTIONAL_PCI_DEVICE);
> +ObjectClass *pcie =
> +object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE);
> +assert(conventional || pcie);
> +}
> +}
> +
>  AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
>  {
>  PCIBus *bus = PCI_BUS(dev->bus);
> @@ -2671,6 +2682,7 @@ static const TypeInfo pci_device_type_info = {
>  .abstract = true,
>  .class_size = sizeof(PCIDeviceClass),
>  .class_init = pci_device_class_init,
> +.class_base_init = pci_device_class_base_init,
>  };
>  
>  static void pci_register_types(void)

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 2/5] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

[David Gibson]

On Wed, Sep 27, 2017 at 05:23:51PM +0530, Aravinda Prasad wrote:
> 
> 
> On Wednesday 27 September 2017 12:45 PM, David Gibson wrote:
> > On Thu, Sep 21, 2017 at 02:39:06PM +0530, Aravinda Prasad wrote:
> >>
> >>
> >> On Tuesday 22 August 2017 07:38 AM, David Gibson wrote:
> >>
> >> [ . . . ]
> >>
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index 46012b3..eee8d33 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -123,6 +123,12 @@ struct sPAPRMachineState {
> >>   * occurs during the unplug process. */
> >>  QTAILQ_HEAD(, sPAPRDIMMState) pending_dimm_unplugs;
> >>  
> >> +/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" 
> >> calls */
> >> +target_ulong guest_machine_check_addr;
> >> +bool mc_in_progress;
> >> +int mc_cpu;
> >
> > mc_cpu isn't actually used yet in this patch.  In any case it and
> > mc_in_progress could probably be folded together, no?
> 
>  It is possible to fold mc_cpu and mc_in_progress together with the
>  convention that if it is set to -1 mc is not in progress otherwise it is
>  set to the CPU handling the mc.
> 
> >
> > These values will also need to be migrated, AFAICT.
> 
>  I am thinking of how to handle the migration when machine check handling
>  is in progress. Probably wait for machine check handling to complete
>  before migrating as the error could be irrelevant once migrated to a new
>  hardware. If that is the case we don't need to migrate these values.
> >>>
> >>> Ok.
> >>
> >> This is what I think about handling machine check during migration based
> >> on my understanding of the VM migration code.
> >>
> >> There are two possibilities here. First, migration can be initiated
> >> while the machine check handling is in progress. Second, A machine check
> >> error can happen when the migration is in progress.
> >>
> >> To handle the first case we can add migrate_add_blocker() call when we
> >> start handling the machine check error and issue migrate_del_blocker()
> >> when done. I think this should solve the issue.
> >>
> >> The second case is bit tricky. The migration has already started and
> >> hence migrate_add_blocker() call will fail. We also cannot wait till the
> >> completion of the migration to handle machine check error as the VM's
> >> data could be corrupt.
> >>
> >> Machine check errors should not be an issue when the migration is in the
> >> RAM copy phase as VM is still active with vCPUs running. The problem is
> >> when we hit a machine check when the migration is about to complete. For
> >> example,
> >>
> >> 1. vCPU2 hits a machine check error during migration.
> >>
> >> 2. KVM causes VM exit on vCPU2 and the NIP of vCPU2 is changed to the
> >> guest registered machine check handler.
> >>
> >> 3. The migration_completion() issues vm_stop() and hence either vCPU2 is
> >> never scheduled again on the source hardware or vCPU2 is preempted while
> >> executing the machine check handler.
> >>
> >> 4. vCPU2 is resumed on the target hardware and either starts or
> >> continues processing the machine check error. This could be a problem as
> >> these errors are specific to the source hardware. For instance, when the
> >> the guest issues memory poisoning upon such error, a clean page on the
> >> target hardware is poisoned while the corrupt page on source hardware is
> >> not poisoned.
> >>
> >> The second case of hitting machine check during the final phase of
> >> migration is rare but wanted to check what others think about it.
> > 
> > So, I've had a bit of a think about this.  I don't recall if these
> > fwnmi machine checks are expected on guest RAM, or guest IO addresses.
> 
> It is expected on guest RAM. I am not sure about guest IO address.
> 
> > 
> > 1) If RAM
> > 
> >   What exactly is the guest's notification for?  Even without
> >   migration, the host's free to move guest memory around in host
> >   memory, so it seems any hardware level poking should be done on the
> >   host side.
> 
> If the error is a correctable error, then host takes care of it by
> moving the page to a different location, the guest need not be and will
> not be notified. Guest will be notified if host is not able to fully
> recover. Hence we hit FWNMI in guest when RAM errors are not recovered
> by the host.

Ok.

> >   Is it just to notify the guest that we weren't able to fully recover
> >   on the host side and that page may contain corrupted data?  If
> >   that's so then it seems resuming the handling on the destination is
> >   still right.  It may be new good RAM, but the contents we migrated
> >   could still be corrupt from the machine check event on the source.
> 
> Yes. This is what I am doing in my v5 patch set which I am about to
> post. Additionally I block migration when processing machine check errors.
> 
> > 
> > 2) If IO
> > 
> >   AFAICT this could 

Re: [Qemu-devel] [PATCH v2 2/5] pci: Add interface names to hybrid PCI devices

[David Gibson]

On Wed, Sep 27, 2017 at 04:56:32PM -0300, Eduardo Habkost wrote:
> The following devices support both PCI Express and Conventional
> PCI, by including special code to handle the QEMU_PCI_CAP_EXPRESS
> flag and/or conditional pcie_endpoint_cap_init() calls:
> 
> * vfio-pci (is_express=1, but legacy PCI handled by
>   vfio_populate_device())

In the case of VFIO, won't this depend on the capabilities of the
device on the host?

> * vmxnet3 (is_express=0, but PCIe handled by vmxnet3_realize())
> * pvscsi (is_express=0, but PCIe handled by pvscsi_realize())
> * virtio-pci (is_express=0, but PCIe handled by
>   virtio_pci_dc_realize(), and additional legacy PCI code at
>   virtio_pci_realize())
> * base-xhci (is_express=1, but pcie_endpoint_cap_init() call
>   is conditional on pci_bus_is_express(dev->bus)
>   * Note that xhci does not clear QEMU_PCI_CAP_EXPRESS like the
> other hybrid devices
> 
> Cc: Dmitry Fleytman 
> Cc: Jason Wang 
> Cc: Paolo Bonzini 
> Cc: Gerd Hoffmann 
> Cc: Alex Williamson 
> Cc: "Michael S. Tsirkin" 
> Signed-off-by: Eduardo Habkost 

Except for the query above,

Reviewed-by: David Gibson 

> ---
> Changes v1 -> v2:
> * s/legacy/conventional/
>   * Suggested-by: Alex Williamson 
> * Mark base-xhci as hybrid too
> ---
>  hw/net/vmxnet3.c   | 5 +
>  hw/scsi/vmw_pvscsi.c   | 2 ++
>  hw/usb/hcd-xhci.c  | 5 +
>  hw/vfio/pci.c  | 5 +
>  hw/virtio/virtio-pci.c | 5 +
>  5 files changed, 22 insertions(+)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index a19a7a31dd..f99d9a69ec 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2651,6 +2651,11 @@ static const TypeInfo vmxnet3_info = {
>  .instance_size = sizeof(VMXNET3State),
>  .class_init= vmxnet3_class_init,
>  .instance_init = vmxnet3_instance_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +{ }
> +},
>  };
>  
>  static void vmxnet3_register_types(void)
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 6d3f0bf11d..d6b315f8b2 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -1300,6 +1300,8 @@ static const TypeInfo pvscsi_info = {
>  .class_init= pvscsi_class_init,
>  .interfaces = (InterfaceInfo[]) {
>  { TYPE_HOTPLUG_HANDLER },
> +{ INTERFACE_PCIE_DEVICE },
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
>  { }
>  }
>  };
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index d75c085d94..af3a9d88de 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3670,6 +3670,11 @@ static const TypeInfo xhci_info = {
>  .instance_size = sizeof(XHCIState),
>  .class_init= xhci_class_init,
>  .abstract  = true,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +{ }
> +},
>  };
>  
>  static void qemu_xhci_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 31e1edf447..913433d6ba 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3023,6 +3023,11 @@ static const TypeInfo vfio_pci_dev_info = {
>  .class_init = vfio_pci_dev_class_init,
>  .instance_init = vfio_instance_init,
>  .instance_finalize = vfio_instance_finalize,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +{ }
> +},
>  };
>  
>  static void register_vfio_pci_dev_type(void)
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 8b0d6b69cd..67c8ab63ad 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1958,6 +1958,11 @@ static const TypeInfo virtio_pci_info = {
>  .class_init= virtio_pci_class_init,
>  .class_size= sizeof(VirtioPCIClass),
>  .abstract  = true,
> +.interfaces = (InterfaceInfo[]) {
> +{ INTERFACE_PCIE_DEVICE },
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +{ }
> +},
>  };
>  
>  /* virtio-blk-pci */

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC v2 28/33] migration: return incoming task tag for sockets

[Peter Xu]

On Fri, Sep 22, 2017 at 09:11:50PM +0100, Dr. David Alan Gilbert wrote:

[...]

> > -void tcp_start_incoming_migration(const char *host_port, Error **errp)
> > +guint tcp_start_incoming_migration(const char *host_port, Error **errp)
> >  {
> >  Error *err = NULL;
> >  SocketAddress *saddr = tcp_build_address(host_port, );
> > +guint tag;
> > +
> >  if (!err) {
> > -socket_start_incoming_migration(saddr, );
> > +tag = socket_start_incoming_migration(saddr, );
> >  }
> 
> I'd be tempted to initialise that tag = 0   for the case where
> there's an error; but OK.

Yeh, I think it worths a touch-up.

> 
> Reviewed-by: Dr. David Alan Gilbert 

Will take the r-b after fixing above.  Thanks!

-- 
Peter Xu

[Qemu-devel] [PATCH v4 5/5] aio: fix assert when remove poll during destroy

[Peter Xu]

From: Stefan Hajnoczi 

After iothread is enabled internally inside QEMU with GMainContext, we
may encounter this warning when destroying the iothread:

(qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll:
 assertion '!SOURCE_DESTROYED (source)' failed

The problem is that g_source_remove_poll() does not allow to remove one
source from array if the source is detached from its owner
context. (peterx: which IMHO does not make much sense)

Fix it on QEMU side by avoid calling g_source_remove_poll() if we know
the object is during destruction, and we won't leak anything after all
since the array will be gone soon cleanly even with that fd.

Signed-off-by: Stefan Hajnoczi 
[peterx: write the commit message]
Signed-off-by: Peter Xu 
---
 util/aio-posix.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 2d51239ec6..5946ac09f0 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -223,7 +223,14 @@ void aio_set_fd_handler(AioContext *ctx,
 return;
 }
 
-g_source_remove_poll(>source, >pfd);
+/* If the GSource is in the process of being destroyed then
+ * g_source_remove_poll() causes an assertion failure.  Skip
+ * removal in that case, because glib cleans up its state during
+ * destruction anyway.
+ */
+if (!g_source_is_destroyed(>source)) {
+g_source_remove_poll(>source, >pfd);
+}
 
 /* If the lock is held, just mark the node as deleted */
 if (qemu_lockcnt_count(>list_lock)) {
-- 
2.13.5

[Qemu-devel] [PATCH v4 3/5] iothread: export iothread_stop()

[Peter Xu]

So that internal iothread users can explicitly stop one iothread without
destroying it.

Since at it, fix iothread_stop() to allow it to be called multiple
times.  Before this patch we may call iothread_stop() more than once on
single iothread, while that may not be correct since qemu_thread_join()
is not allowed to run twice.  From manual of pthread_join():

  Joining with a thread that has previously been joined results in
  undefined behavior.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 include/sysemu/iothread.h |  1 +
 iothread.c| 24 
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index b07663f0a1..110329b2b4 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -52,6 +52,7 @@ GMainContext *iothread_get_g_main_context(IOThread *iothread);
  * "query-iothreads".
  */
 IOThread *iothread_create(const char *id, Error **errp);
+void iothread_stop(IOThread *iothread);
 void iothread_destroy(IOThread *iothread);
 
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index 33f996e561..b3c092b2d7 100644
--- a/iothread.c
+++ b/iothread.c
@@ -80,13 +80,10 @@ static void *iothread_run(void *opaque)
 return NULL;
 }
 
-static int iothread_stop(Object *object, void *opaque)
+void iothread_stop(IOThread *iothread)
 {
-IOThread *iothread;
-
-iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
-if (!iothread || !iothread->ctx) {
-return 0;
+if (!iothread->ctx || iothread->stopping) {
+return;
 }
 iothread->stopping = true;
 aio_notify(iothread->ctx);
@@ -94,6 +91,17 @@ static int iothread_stop(Object *object, void *opaque)
 g_main_loop_quit(iothread->main_loop);
 }
 qemu_thread_join(>thread);
+}
+
+static int iothread_stop_iter(Object *object, void *opaque)
+{
+IOThread *iothread;
+
+iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
+if (!iothread) {
+return 0;
+}
+iothread_stop(iothread);
 return 0;
 }
 
@@ -108,7 +116,7 @@ static void iothread_instance_finalize(Object *obj)
 {
 IOThread *iothread = IOTHREAD(obj);
 
-iothread_stop(obj, NULL);
+iothread_stop(iothread);
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 if (!iothread->ctx) {
@@ -328,7 +336,7 @@ void iothread_stop_all(void)
 aio_context_release(ctx);
 }
 
-object_child_foreach(container, iothread_stop, NULL);
+object_child_foreach(container, iothread_stop_iter, NULL);
 }
 
 static gpointer iothread_g_main_context_init(gpointer opaque)
-- 
2.13.5

[Qemu-devel] [PATCH v4 4/5] iothread: delay the context release to finalize

[Peter Xu]

When gcontext is used with iothread, the context will be destroyed
during iothread_stop().  That's not good since sometimes we would like
to keep the resources until iothread is destroyed, but we may want to
stop the thread before that point.

Delay the destruction of gcontext to iothread finalize.  Then we can do:

  iothread_stop(thread);
  some_cleanup_on_resources();
  iothread_destroy(thread);

We may need this patch if we want to run chardev IOs in iothreads and
hopefully clean them up correctly.  For more specific information,
please see 2b316774f6 ("qemu-char: do not operate on sources from
finalize callbacks").

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 iothread.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/iothread.c b/iothread.c
index b3c092b2d7..27a4288578 100644
--- a/iothread.c
+++ b/iothread.c
@@ -71,8 +71,6 @@ static void *iothread_run(void *opaque)
 g_main_loop_unref(loop);
 
 g_main_context_pop_thread_default(iothread->worker_context);
-g_main_context_unref(iothread->worker_context);
-iothread->worker_context = NULL;
 }
 }
 
@@ -117,6 +115,10 @@ static void iothread_instance_finalize(Object *obj)
 IOThread *iothread = IOTHREAD(obj);
 
 iothread_stop(iothread);
+if (iothread->worker_context) {
+g_main_context_unref(iothread->worker_context);
+iothread->worker_context = NULL;
+}
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 if (!iothread->ctx) {
-- 
2.13.5

[Qemu-devel] [PATCH v4 2/5] iothread: provide helpers for internal use

[Peter Xu]

IOThread is a general framework that contains IO loop environment and a
real thread behind.  It's also good to be used internally inside qemu.
Provide some helpers for it to create iothreads to be used internally.

Put all the internal used iothreads into the internal object container.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 include/sysemu/iothread.h |  8 
 iothread.c| 16 
 2 files changed, 24 insertions(+)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index d2985b30ba..b07663f0a1 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread *iothread);
 void iothread_stop_all(void);
 GMainContext *iothread_get_g_main_context(IOThread *iothread);
 
+/*
+ * Helpers used to allocate iothreads for internal use.  These
+ * iothreads will not be seen by monitor clients when query using
+ * "query-iothreads".
+ */
+IOThread *iothread_create(const char *id, Error **errp);
+void iothread_destroy(IOThread *iothread);
+
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index 44c8944dc4..33f996e561 100644
--- a/iothread.c
+++ b/iothread.c
@@ -354,3 +354,19 @@ GMainContext *iothread_get_g_main_context(IOThread 
*iothread)
 
 return iothread->worker_context;
 }
+
+IOThread *iothread_create(const char *id, Error **errp)
+{
+Object *obj;
+
+obj = object_new_with_props(TYPE_IOTHREAD,
+object_get_internal_root(),
+id, errp, NULL);
+
+return IOTHREAD(obj);
+}
+
+void iothread_destroy(IOThread *iothread)
+{
+object_unparent(OBJECT(iothread));
+}
-- 
2.13.5

[Qemu-devel] [PATCH v4 1/5] qom: provide root container for internal objs

[Peter Xu]

We have object_get_objects_root() to keep user created objects, however
no place for objects that will be used internally.  Create such a
container for internal objects.

CC: Andreas Färber 
CC: Markus Armbruster 
CC: Paolo Bonzini 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Peter Xu 
---
 include/qom/object.h | 11 +++
 qom/object.c | 11 +++
 2 files changed, 22 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index f3e5cff37a..e0d9824415 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1214,6 +1214,17 @@ Object *object_get_root(void);
 Object *object_get_objects_root(void);
 
 /**
+ * object_get_internal_root:
+ *
+ * Get the container object that holds internally used object
+ * instances.  Any object which is put into this container must not be
+ * user visible, and it will not be exposed in the QOM tree.
+ *
+ * Returns: the internal object container
+ */
+Object *object_get_internal_root(void);
+
+/**
  * object_get_canonical_path_component:
  *
  * Returns: The final component in the object's canonical path.  The canonical
diff --git a/qom/object.c b/qom/object.c
index 3e18537e9b..6a7bd9257b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1370,6 +1370,17 @@ Object *object_get_objects_root(void)
 return container_get(object_get_root(), "/objects");
 }
 
+Object *object_get_internal_root(void)
+{
+static Object *internal_root;
+
+if (!internal_root) {
+internal_root = object_new("container");
+}
+
+return internal_root;
+}
+
 static void object_get_child_property(Object *obj, Visitor *v,
   const char *name, void *opaque,
   Error **errp)
-- 
2.13.5

[Qemu-devel] [PATCH v4 0/5] iothread: allow to create internal iothreads

[Peter Xu]

v4:
- fix comment in patch 1 [Fam, Stefan]
- add one more patch from Stefan to fix the aio glib warning

v3:
- pick up r-bs (one missing for Fam's on last patch)
- fix patch 1 to create isolated internal container

v2:
- add one patch to provide object_get_internal_root() [Daniel]
- patch 2: use the new object_get_internal_root()
- patch 3: fix commit message, "reentrant" is wrongly used by me. it
  should be "called multiple times"; move iothread->ctx check into
  iothread_stop() [Fam]
- patch 4: add one paragraph in commit message, mention about the glib
  issue. [Fam]

When trying to support monitor OOB (out-of-band) commands, I found
that the monitor IO thread I did looks just like iothread.  It would
be best if I can use iothread directly.  However it seems that it was
mostly used by "-object iothread" before but not friendly to internal
usages.  This series tries to export essential functions to do it.

Also, I think patch 2 also fixes a bug in iothread_stop().

Please review. Thanks.

Peter Xu (4):
  qom: provide root container for internal objs
  iothread: provide helpers for internal use
  iothread: export iothread_stop()
  iothread: delay the context release to finalize

Stefan Hajnoczi (1):
  aio: fix assert when remove poll during destroy

 include/qom/object.h  | 11 +++
 include/sysemu/iothread.h |  9 +
 iothread.c| 46 --
 qom/object.c  | 11 +++
 util/aio-posix.c  |  9 -
 5 files changed, 75 insertions(+), 11 deletions(-)

-- 
2.13.5

Re: [Qemu-devel] A glib warning encountered with internal iothread

[Peter Xu]

On Wed, Sep 27, 2017 at 03:14:40PM +0200, Paolo Bonzini wrote:
> On 27/09/2017 14:36, Fam Zheng wrote:
> > On Wed, 09/27 13:17, Stefan Hajnoczi wrote:
> >> On Tue, Sep 26, 2017 at 07:13:43PM +0800, Fam Zheng wrote:
> >>> On Tue, 09/26 17:11, Peter Xu wrote:
> >>>  void aio_context_unref(AioContext *ctx)
> >>>  {
> >>> +assert(ctx->refcnt > 0);
> >>> +if (--ctx->refcnt == 0) {
> >>> +aio_set_event_notifier(ctx, >notifier, false, NULL, NULL);
> >>> +}
> >>
> >> This isn't a general solution because Linux AIO also has a file
> >> descriptor that is removed in aio_ctx_finalize().
> > 
> > Right. Another option is to move everything in aio_context_finalize() into 
> > the
> > "if (--ctx->refcnt == 0) { ... }" block, before calling g_source_unref().
> 
> I think I prefer Stefan's solution.  If the GSource has been ref'ed
> independent of the AioContext object, it may be a problem to free it early.

Stefan's patch works for me.  I'll put Stefan's patch into iothread
series and repost soon.

Thanks Stefan (and all)!

-- 
Peter Xu

Re: [Qemu-devel] [RFC v2 24/33] migration: synchronize dirty bitmap for resume

[Peter Xu]

On Fri, Sep 22, 2017 at 12:33:19PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > This patch implements the first part of core RAM resume logic for
> > postcopy. ram_resume_prepare() is provided for the work.
> > 
> > When the migration is interrupted by network failure, the dirty bitmap
> > on the source side will be meaningless, because even the dirty bit is
> > cleared, it is still possible that the sent page was lost along the way
> > to destination. Here instead of continue the migration with the old
> > dirty bitmap on source, we ask the destination side to send back its
> > received bitmap, then invert it to be our initial dirty bitmap.
> > 
> > The source side send thread will issue the MIG_CMD_RECV_BITMAP requests,
> > once per ramblock, to ask for the received bitmap. On destination side,
> > MIG_RP_MSG_RECV_BITMAP will be issued, along with the requested bitmap.
> > Data will be received on the return-path thread of source, and the main
> > migration thread will be notified when all the ramblock bitmaps are
> > synchronized.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  migration/migration.c  |  4 +++
> >  migration/migration.h  |  1 +
> >  migration/ram.c| 67 
> > ++
> >  migration/trace-events |  4 +++
> >  4 files changed, 76 insertions(+)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 19b7f3a5..19aed72 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2605,6 +2605,8 @@ static void migration_instance_finalize(Object *obj)
> >  
> >  g_free(params->tls_hostname);
> >  g_free(params->tls_creds);
> > +
> > +qemu_sem_destroy(>rp_state.rp_sem);
> >  }
> >  
> >  static void migration_instance_init(Object *obj)
> > @@ -2629,6 +2631,8 @@ static void migration_instance_init(Object *obj)
> >  params->has_downtime_limit = true;
> >  params->has_x_checkpoint_delay = true;
> >  params->has_block_incremental = true;
> > +
> > +qemu_sem_init(>rp_state.rp_sem, 1);
> >  }
> >  
> >  /*
> > diff --git a/migration/migration.h b/migration/migration.h
> > index a3a0582..d041369 100644
> > --- a/migration/migration.h
> > +++ b/migration/migration.h
> > @@ -107,6 +107,7 @@ struct MigrationState
> >  QEMUFile *from_dst_file;
> >  QemuThreadrp_thread;
> >  bool  error;
> > +QemuSemaphore rp_sem;
> >  } rp_state;
> >  
> >  double mbps;
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 5d938e3..afabcf5 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -47,6 +47,7 @@
> >  #include "exec/target_page.h"
> >  #include "qemu/rcu_queue.h"
> >  #include "migration/colo.h"
> > +#include "savevm.h"
> >  
> >  /***/
> >  /* ram save/restore */
> > @@ -295,6 +296,8 @@ struct RAMState {
> >  RAMBlock *last_req_rb;
> >  /* Queue of outstanding page requests from the destination */
> >  QemuMutex src_page_req_mutex;
> > +/* Ramblock counts to sync dirty bitmap. Only used for recovery */
> > +int ramblock_to_sync;
> >  QSIMPLEQ_HEAD(src_page_requests, RAMSrcPageRequest) src_page_requests;
> >  };
> >  typedef struct RAMState RAMState;
> > @@ -2770,6 +2773,56 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> > version_id)
> >  return ret;
> >  }
> >  
> > +/* Sync all the dirty bitmap with destination VM.  */
> > +static int ram_dirty_bitmap_sync_all(MigrationState *s, RAMState *rs)
> > +{
> > +RAMBlock *block;
> > +QEMUFile *file = s->to_dst_file;
> > +int ramblock_count = 0;
> > +
> > +trace_ram_dirty_bitmap_sync_start();
> > +
> > +/*
> > + * We do this in such order:
> > + *
> > + * 1. calculate block count
> > + * 2. fill in the count to N
> > + * 3. send MIG_CMD_RECV_BITMAP requests
> > + * 4. wait on the semaphore until N -> 0
> > + */
> > +
> > +RAMBLOCK_FOREACH(block) {
> > +ramblock_count++;
> > +}
> > +
> > +atomic_set(>ramblock_to_sync, ramblock_count);
> > +RAMBLOCK_FOREACH(block) {
> > +qemu_savevm_send_recv_bitmap(file, block->idstr);
> > +}
> > +
> > +trace_ram_dirty_bitmap_sync_wait();
> 
> Please include the RAMBlock name in the trace, so if it hangs we can
> see where.

This is to note when we start to wait, while there is a trace below
when we reload one single ramblock at [1].  Would that suffice?

> 
> > +
> > +/* Wait until all the ramblocks' dirty bitmap synced */
> > +while (atomic_read(>ramblock_to_sync)) {
> > +qemu_sem_wait(>rp_state.rp_sem);
> > +}
> 
> Do you need to make ramblock_to_sync global and use atomics - I think
> you can simplify it;  if you qemu_sem_init to 0, then I think you
> can do:
>while (ramblock_count--) {
>qemu_sem_wait(>rp_state.rp_sem);
>}
> 
> qemu_sem_wait will block until the semaphore is 

Re: [Qemu-devel] [PATCH] docker: Don't mount ccache db if NOUSER=1

[Philippe Mathieu-Daudé]

On Mon, Sep 25, 2017 at 4:54 AM, Fam Zheng  wrote:
> With NOUSER=1 the container runs code as root, which may create
> privileged files that will not be be accssible next time. Skip ccache
> dir mount in this case.
>
> Signed-off-by: Fam Zheng 

Acked-by: Philippe Mathieu-Daudé 

> ---
>  tests/docker/Makefile.include | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 0e4f159619..6f9ea196a7 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -143,9 +143,11 @@ docker-run: docker-qemu-src
> -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
> -e V=$V -e J=$J -e DEBUG=$(DEBUG)   \
> -e SHOW_ENV=$(SHOW_ENV) \
> -   -e CCACHE_DIR=/var/tmp/ccache   \
> +   $(if $(NOUSER),,\
> +   -e CCACHE_DIR=/var/tmp/ccache   \
> +   -v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
> +   )   \
> -v $$(readlink -e 
> $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
> -   -v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z   \
> $(IMAGE)\
> /var/tmp/qemu/run   \
> $(TEST), "  RUN $(TEST) in ${IMAGE}")
> --
> 2.13.5
>

Re: [Qemu-devel] [Qemu-arm] [PATCH v1 1/8] Replace all occurances of __FUNCTION__ with __func__

[Peter Maydell]

On 26 September 2017 at 06:32, Eric Blake  wrote:
> On 09/25/2017 07:08 PM, Alistair Francis wrote:
>> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
>> index 58005b6619..32687afced 100644
>> --- a/hw/arm/nseries.c
>> +++ b/hw/arm/nseries.c
>> @@ -463,7 +463,7 @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, 
>> int len)
>>  uint8_t ret;
>>
>>  if (len > 9) {
>> -hw_error("%s: FIXME: bad SPI word width %i\n", __FUNCTION__, len);
>> +hw_error("%s: FIXME: bad SPI word width %i\n", __func__, len);
>
> Not this patch's problem, but it would probably be simpler if hw_error()
> were a macro that automatically prefixed __func__, rather than making
> every caller have to supply it themselves.

I'm not sure there's a great deal of benefit to that change, because
use of hw_error() in new code is rarely correct (it does an abort()
so it should never be used for guest-triggered conditions, which is
about the only time that you might be interested in a guest register
dump rather than just asserting). Most of its existing uses are in
crufty old device models.

thanks
-- PMM

Re: [Qemu-devel] Ping: [PATCH 0/2] ui/cocoa.m: enable guest to see control-alt key combinations

[Peter Maydell]

On 27 September 2017 at 15:55, Programmingkid  wrote:
> Currently if the user needs to send a control-alt key combination, he or she 
> was either out of luck or had to rely on the monitor's sendkey command to do 
> so. With this patch the user can now directly send control-alt key 
> combinations. This is great for Windows guest that may need the 
> control-alt-delete key combination.
>

Thanks for the ping. This is on my to-review list, but I had
a quick scan of it, noticed that you didn't make the code
in patch 2 work the way I'd asked in my review of v1, thought
about just fixing it myself and then ran out of time to
look at it. I'm currently several thousand miles away from
my OSX machine...

-- PMM

Re: [Qemu-devel] [PULL 0/7] migration queue

[Peter Maydell]

On 27 September 2017 at 06:57, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> The following changes since commit 31bc1d8481af414cfa2857f905e40f7d8e6d5b2e:
>
>   Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into 
> staging (2017-09-26 19:49:08 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20170927a
>
> for you to fetch changes up to 2f168d0708581c33baf6c78d75a89e8cd705f9f6:
>
>   migration: Route more error paths (2017-09-27 11:44:18 +0100)
>
> 
> Migration pull 2017-09-27
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Peter Maydell]

On 27 September 2017 at 16:14, Michael S. Tsirkin  wrote:
> On Tue, Sep 26, 2017 at 08:35:59PM +0100, Peter Maydell wrote:
>> On 26 September 2017 at 20:31, Michael S. Tsirkin  wrote:
>> > Please do not apply this, trivial is not appropriate for functional
>> > changes like this.
>>
>> It's not a functional change, it's just bumping a test timeout.
>> If you think we should be doing something else that's fine (as
>> with any other patch), but in principle I think this is totally
>> fine as a -trivial patch.

> OK. I'd rather not see it applied as-is though.

Oops, looks like it's already hit master (07897000194a).
I can revert it if you feel strongly about it, but my personal
feeling is that the timeout needs to be long enough that you
don't hit it in any configuration. Usually if you're running
tests yourself then you'll notice it's stuck before it
hits a timeout -- the timeout is just so that fully automated
test runs don't hang forever.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Michael S. Tsirkin]

On Tue, Sep 26, 2017 at 08:35:59PM +0100, Peter Maydell wrote:
> On 26 September 2017 at 20:31, Michael S. Tsirkin  wrote:
> > On Mon, Sep 25, 2017 at 12:06:40AM +0300, Michael Tokarev wrote:
> >> 22.09.2017 06:06, Thomas Huth wrote:
> >> > If QEMU has been compiled with the flags --enable-tcg-interpreter and
> >> > --enable-debug, the guest is running incredibly slow. The pxe boot test
> >> > can take up to 400 seconds when testing the pseries ppc64 machine. While
> >> > we should still look for ways to speed up the test on the pseries 
> >> > machine,
> >> > it's better to increase the timeout in this test to 600 seconds anyway to
> >> > allow the test to pass successfully now with this unusal configuration
> >> > already.
> >>
> >> Applied to -trivial, thanks!
> >>
> >> /mjt
> >
> > Please do not apply this, trivial is not appropriate for functional
> > changes like this.
> 
> It's not a functional change, it's just bumping a test timeout.
> If you think we should be doing something else that's fine (as
> with any other patch), but in principle I think this is totally
> fine as a -trivial patch.
> 
> thanks
> -- PMM

OK. I'd rather not see it applied as-is though.

-- 
MST

Re: [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

On Mon, Sep 25, 2017 at 5:55 PM, Emilio G. Cota  wrote:
> On Mon, Sep 25, 2017 at 17:08:48 -0700, Alistair Francis wrote:
>> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
>> index caa1e8e689..41ba1600c0 100644
>> --- a/tests/atomic_add-bench.c
>> +++ b/tests/atomic_add-bench.c
>> @@ -29,7 +29,7 @@ static const char commands_string[] =
>>  static void usage_complete(char *argv[])
>>  {
>>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
>> -fprintf(stderr, "options:\n%s\n", commands_string);
>> +fprintf(stderr, "options:\n%s", commands_string);
>>  }
>
> We do want that trailing \n, unless we move it to commands_string.

Yeah, this was a mistake. It should have swapped to error_report().

>
> Also, I think using error_report here would be confusing -- this is a 
> standalone
> test program with as little QEMU-specific knowledge as possible (QemuThreads
> are used for portability); using error_report here is confusing (this is not
> an error).

Do you mean in all tests/ sub directory or just a few certain cases?

>
>> diff --git a/tests/check-qlit b/tests/check-qlit
>> new file mode 100755
>> index 
>> ..950429524e3eb07e6daed1fe01caad0f5d554809
>> GIT binary patch
>> literal 272776
>> zcmeEvdwf*Ywf~vPB$){zGeCghJ;($So{10*LNEgfoIs*MKvBRDLV(l`3b5QKOS6
>
> ? I don't know what this is, I don't seem to have this binary in my
> checked out tree.

Yeah, this shouldn't be here.

Thanks,
Alistair

>
> (snips thousands of lines)
>
>> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
>> index 11c1cec766..2637d601a9 100644
>> --- a/tests/qht-bench.c
>> +++ b/tests/qht-bench.c
>> @@ -5,6 +5,7 @@
>>   *   See the COPYING file in the top-level directory.
>>   */
>>  #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>>  #include "qemu/processor.h"
>>  #include "qemu/atomic.h"
>>  #include "qemu/qht.h"
>> @@ -89,7 +90,7 @@ static const char commands_string[] =
>>  static void usage_complete(int argc, char *argv[])
>>  {
>>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
>> -fprintf(stderr, "options:\n%s\n", commands_string);
>> +fprintf(stderr, "options:\n%s", commands_string);
>
> Same as above: this removes the necessary trailing \n.
>
>>  exit(-1);
>>  }
>>
>> @@ -328,7 +329,7 @@ static void htable_init(void)
>>  retries++;
>>  }
>>  }
>> -fprintf(stderr, " populated after %zu retries\n", retries);
>> +error_report(" populated after %zu retries", retries);
>>  }
>
> ditto -- I'd rather keep fprintf(stderr) here, it's less confusing.
>
> Thanks,
>
> Emilio

Re: [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command

[Jan Dakinevich]

> OK but if it's useful as an hmp command, why not as a qmp command?
>

The command is designed for debugging and produces quite sightly output. 
For respective qmp command most of `info virtio' output would excessive 
and unneccesary.


On 09/27/2017 10:43 PM, Michael S. Tsirkin wrote:

On Wed, Sep 27, 2017 at 06:09:42PM +0300, Jan Dakinevich wrote:

The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.

Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.

Cc: Denis V. Lunev 
Signed-off-by: Jan Dakinevich 


OK but if it's useful as an hmp command, why not as a qmp command?


---
v2:
* Moved hmp_info_virtio and stuff to hmp.c to avoid build error
* Added printing of device status
* Listed common virtio features

v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
---
  hmp-commands-info.hx|  14 +
  hmp.c   | 148 
  hmp.h   |   1 +
  hw/block/virtio-blk.c   |  20 ++
  hw/char/virtio-serial-bus.c |  15 +
  hw/display/virtio-gpu.c |  12 
  hw/net/virtio-net.c |  34 ++
  hw/scsi/virtio-scsi.c   |  15 +
  hw/virtio/virtio-balloon.c  |  15 +
  hw/virtio/virtio-bus.c  |   1 +
  include/hw/virtio/virtio.h  |   2 +
  monitor.c   |   1 +
  12 files changed, 278 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
  },
  
  STEXI

+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name   = "virtio",
+.args_type  = "",
+.params = "",
+.help   = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
  @end table
  ETEXI
  
diff --git a/hmp.c b/hmp.c

index ace729d..009d808 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
  #include "hw/intc/intc.h"
  #include "migration/snapshot.h"
  #include "migration/misc.h"
+#include "hw/virtio/virtio.h"
  
  #ifdef CONFIG_SPICE

  #include 
@@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon, const 
QDict *qdict)
  }
  hmp_handle_error(mon, );
  }
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FIELD 32
+
+static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
+{
+uint8_t status[] = {
+VIRTIO_CONFIG_S_ACKNOWLEDGE,
+VIRTIO_CONFIG_S_DRIVER,
+VIRTIO_CONFIG_S_DRIVER_OK,
+VIRTIO_CONFIG_S_FEATURES_OK,
+VIRTIO_CONFIG_S_NEEDS_RESET,
+VIRTIO_CONFIG_S_FAILED,
+};
+const char *names[] = {
+"acknowledge",
+"driver",
+"driver_ok",
+"features_ok",
+"needs_reset"
+"failed",
+};
+const char *comma = "";
+int idx;
+
+monitor_printf(mon, "%*sstatus:   0x%02"PRIx8" ",
+   HMP_INFO_VIRTIO_INDENT, "", vdev->status);
+
+for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
+if (!(vdev->status & status[idx])) {
+continue;
+}
+monitor_printf(mon, "%s%s", comma, names[idx]);
+if (names[idx]) {
+comma = ",";
+}
+}
+monitor_printf(mon, "\n");
+}
+
+static void hmp_info_virtio_print_common_features(Monitor *mon,
+  VirtIODevice *vdev)
+{
+uint64_t features[] = {
+VIRTIO_RING_F_INDIRECT_DESC,
+VIRTIO_RING_F_EVENT_IDX,
+VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_ANY_LAYOUT,
+VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_VERSION_1,
+};
+const char *names[] = {
+"indirect_desc",
+"event_idx",
+"notify_on_empty",
+"any_layout",
+"iommu_platform",
+"version_1",
+};
+int idx;
+
+monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
+const char *ack = vdev->guest_features & features[idx] ? "acked" : "";
+
+if (!(vdev->host_features & features[idx])) {
+continue;
+}
+monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
+   HMP_INFO_VIRTIO_FIELD, names[idx],
+   HMP_INFO_VIRTIO_FIELD, ack);
+}
+}
+
+static void hmp_info_virtio_print_specific_features(Monitor *mon,
+VirtIODevice *vdev)
+{
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+int idx;
+
+if (!vdc->get_feature_name) {
+return;
+}
+
+monitor_printf(mon, 

Re: [Qemu-devel] [PATCH v1 1/8] Replace all occurances of __FUNCTION__ with __func__

[Alistair Francis]

On Tue, Sep 26, 2017 at 6:32 AM, Eric Blake  wrote:
> On 09/25/2017 07:08 PM, Alistair Francis wrote:
>> Replace all occurs of __FUNCTION__ except for the check in checkpatch
>> with the non GCC specific __func__.
>>
>> One line in hcd-musb.c was manually tweaked to pass checkpatch.
>>
>> Signed-off-by: Alistair Francis 
>> Cc: Gerd Hoffmann 
>> Cc: Andrzej Zaborowski 
>> Cc: Stefano Stabellini 
>> Cc: Anthony Perard 
>> Cc: John Snow 
>> Cc: Aurelien Jarno 
>> Cc: Yongbok Kim 
>> Cc: Peter Crosthwaite 
>> Cc: Stefan Hajnoczi  (supporter:Block
>> Cc: Fam Zheng  (supporter:Block
>
> That looks funny, with no closing ).  Something's breaking down between
> get_maintainers.pl and your eventual email, although it's not fatal.

Yeah, that's a copy and paste error, will fix.

>
>> Cc: Juan Quintela 
>> Cc: "Dr. David Alan Gilbert" 
>> Cc: qemu-...@nongnu.org
>> Cc: qemu-bl...@nongnu.org
>> Cc: xen-de...@lists.xenproject.org
>> ---
>>
>
>>  65 files changed, 273 insertions(+), 273 deletions(-)
>
> Big but mechanical, so I'm okay without splitting it further.

Phew! I did not want to split it.

>
>>
>> diff --git a/audio/audio_int.h b/audio/audio_int.h
>> index 5bcb1c60e1..543b1bd8d5 100644
>> --- a/audio/audio_int.h
>> +++ b/audio/audio_int.h
>> @@ -253,7 +253,7 @@ static inline int audio_ring_dist (int dst, int src, int 
>> len)
>>  #define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n)
>>
>>  #if defined _MSC_VER || defined __GNUC__
>> -#define AUDIO_FUNC __FUNCTION__
>> +#define AUDIO_FUNC __func__
>>  #else
>>  #define AUDIO_FUNC __FILE__ ":" AUDIO_STRINGIFY (__LINE__)
>>  #endif
>
> This can be further simplified.  We really aren't using _MSC_VER as our
> compiler (can anyone prove me wrong?), and we DO require a C99 compiler
> (per C99 6.4.2.2, __func__ support is mandatory), so we don't really
> need the #else branch (or, for that matter, we probably don't even need
> AUDIO_FUNC).  But to keep this patch mechanical, that can be a separate
> followup.

I have a second patch that removes AUDIO_FUNC

>
>> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
>> index 58005b6619..32687afced 100644
>> --- a/hw/arm/nseries.c
>> +++ b/hw/arm/nseries.c
>> @@ -463,7 +463,7 @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, 
>> int len)
>>  uint8_t ret;
>>
>>  if (len > 9) {
>> -hw_error("%s: FIXME: bad SPI word width %i\n", __FUNCTION__, len);
>> +hw_error("%s: FIXME: bad SPI word width %i\n", __func__, len);
>
> Not this patch's problem, but it would probably be simpler if hw_error()
> were a macro that automatically prefixed __func__, rather than making
> every caller have to supply it themselves.

I'm going to leave this for another day, but I think you are right.

>
>> +++ b/hw/arm/omap1.c
>
>> @@ -1716,7 +1716,7 @@ static void omap_clkm_write(void *opaque, hwaddr addr,
>>  case 0x18:   /* ARM_SYSST */
>>  if ((s->clkm.clocking_scheme ^ (value >> 11)) & 7) {
>>  s->clkm.clocking_scheme = (value >> 11) & 7;
>> -printf("%s: clocking scheme set to %s\n", __FUNCTION__,
>> +printf("%s: clocking scheme set to %s\n", __func__,
>>  clkschemename[s->clkm.clocking_scheme]);
>
> Worth fixing the indentation while you are here?

Fixed

>
>> @@ -2473,7 +2473,7 @@ static void omap_pwt_write(void *opaque, hwaddr addr,
>>  case 0x04:   /* VRC */
>>  if ((value ^ s->vrc) & 1) {
>>  if (value & 1)
>> -printf("%s: %iHz buzz on\n", __FUNCTION__, (int)
>> +printf("%s: %iHz buzz on\n", __func__, (int)
>>  /* 1.5 MHz from a 12-MHz or 13-MHz PWT_CLK 
>> */
>>  ((omap_clk_getrate(s->clk) >> 3) /
>>   /* Pre-multiplexer divider */
>
> Likewise?

This doesn't actually change the indention. It's all one argument to a function.

>
>> @@ -3330,13 +3330,13 @@ static void omap_mcbsp_writeh(void *opaque, hwaddr 
>> addr,
>>  s->mcr[1] = value & 0x03e3;
>>  if (value & 3)   /* XMCM */
>>  printf("%s: Tx channel selection mode enable attempt\n",
>> -__FUNCTION__);
>> +__func__);
>>  return;
>>  case 0x1a:   /* MCR1 */
>>  s->mcr[0] = value & 0x03e1;
>>  if (value & 1)   /* RMCM */
>>  printf("%s: Rx channel selection mode enable attempt\n",
>> -__FUNCTION__);
>> +__func__);
>
> and again

Fixed.

>
>
>> +++ b/hw/arm/omap2.c
>> @@ -1312,7 

[Qemu-devel] Ping: [PATCH 0/2] ui/cocoa.m: enable guest to see control-alt key combinations

[Programmingkid]

Currently if the user needs to send a control-alt key combination, he or she 
was either out of luck or had to rely on the monitor's sendkey command to do 
so. With this patch the user can now directly send control-alt key 
combinations. This is great for Windows guest that may need the 
control-alt-delete key combination. 

John Arbuckle (2):
  move ungrab to ctrl-alt-g
  send ctrl-alt key combinations to guest if not used by QEMU

 ui/cocoa.m | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

-- 
2.11.0 (Apple Git-81)

Re: [Qemu-devel] [PATCH v1 3/8] hw: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

On Mon, Sep 25, 2017 at 8:51 PM, Thomas Huth  wrote:
> On 26.09.2017 02:08, Alistair Francis wrote:
>> Replace a large number of the fprintf(stderr, "*\n" calls with
>> error_report(). The functions were renamed with these commands and then
>> compiler issues where manually fixed.
>>
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N;N; {s|fprintf(stderr, 
>> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>> find ./* -type f -exec sed -i \
>> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>> {} +
>>
>> Some lines where then manually tweaked to pass checkpatch.
>>
>> Signed-off-by: Alistair Francis 
>> Cc: Andrzej Zaborowski 
>> Cc: Jan Kiszka 
>> Cc: Stefan Hajnoczi 
>> Cc: Paolo Bonzini 
>> Cc: Thomas Huth 
>> Cc: Gerd Hoffmann 
>> Cc: "Michael S. Tsirkin" 
>> Cc: Richard Henderson 
>> Cc: Eduardo Habkost 
>> Cc: Stefano Stabellini 
>> Cc: Anthony Perard 
>> Cc: John Snow 
>> Cc: Christian Borntraeger 
>> Cc: Cornelia Huck 
>> Cc: Alexander Graf 
>> Cc: Michael Walle 
>> Cc: Paul Burton 
>> Cc: Aurelien Jarno 
>> Cc: Yongbok Kim 
>> Cc: "Hervé Poussineau" 
>> Cc: Anthony Green 
>> Cc: Jason Wang 
>> Cc: Chris Wulff 
>> Cc: Marek Vasut 
>> Cc: Jia Liu 
>> Cc: Stafford Horne 
>> Cc: Marcel Apfelbaum 
>> Cc: Magnus Damm 
>> Cc: Fabien Chouteau 
>> Cc: Mark Cave-Ayland 
>> Cc: Artyom Tarasenko 
>> Cc: qemu-...@nongnu.org
>> Cc: qemu-bl...@nongnu.org
>> Cc: xen-de...@lists.xenproject.org
>> Cc: qemu-...@nongnu.org
>> ---
>>
>>  hw/arm/armv7m.c |  2 +-
>>  hw/arm/boot.c   | 34 +--
>>  hw/arm/gumstix.c| 13 
>>  hw/arm/mainstone.c  |  7 ++--
>>  hw/arm/musicpal.c   |  2 +-
>>  hw/arm/omap1.c  |  5 +--
>>  hw/arm/omap2.c  | 21 ++--
>>  hw/arm/omap_sx1.c   |  6 ++--
>>  hw/arm/palm.c   | 10 +++---
>>  hw/arm/pxa2xx.c |  7 ++--
>>  hw/arm/stellaris.c  |  3 +-
>>  hw/arm/tosa.c   | 17 +-
>>  hw/arm/versatilepb.c|  2 +-
>>  hw/arm/vexpress.c   |  8 ++---
>>  hw/arm/z2.c |  6 ++--
>>  hw/block/dataplane/virtio-blk.c |  6 ++--
>>  hw/block/onenand.c  |  8 ++---
>>  hw/block/tc58128.c  | 44 -
>>  hw/bt/core.c| 15 +
>>  hw/bt/hci-csr.c | 17 +-
>>  hw/bt/hci.c | 30 -
>>  hw/bt/hid.c |  2 +-
>>  hw/bt/l2cap.c   | 47 ++-
>>  hw/bt/sdp.c |  7 ++--
>>  hw/char/exynos4210_uart.c   |  6 ++--
>>  hw/char/mcf_uart.c  |  5 +--
>>  hw/char/sh_serial.c |  9 +++---
>>  hw/core/loader.c| 31 +-
>>  hw/core/ptimer.c|  7 ++--
>>  hw/cris/axis_dev88.c|  3 +-
>>  hw/cris/boot.c  |  5 +--
>>  

Re: [Qemu-devel] [PATCH 1/2] vl: Eliminate defconfig variable

[Alistair Francis]

On Wed, Sep 27, 2017 at 1:32 PM, Eduardo Habkost  wrote:
> Both -nodefconfig and -no-user-config options do the same thing
> today, we only need one variable to keep track of them.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Eduardo Habkost 

Acked-by: Alistair Francis 

Thanks,
Alistair

> ---
>  vl.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 4fd01fda91..b347a97a5b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3093,7 +3093,6 @@ int main(int argc, char **argv, char **envp)
>  const char *qtest_log = NULL;
>  const char *pid_file = NULL;
>  const char *incoming = NULL;
> -bool defconfig = true;
>  bool userconfig = true;
>  bool nographic = false;
>  DisplayType display_type = DT_DEFAULT;
> @@ -3194,8 +3193,6 @@ int main(int argc, char **argv, char **envp)
>  popt = lookup_opt(argc, argv, , );
>  switch (popt->index) {
>  case QEMU_OPTION_nodefconfig:
> -defconfig = false;
> -break;
>  case QEMU_OPTION_nouserconfig:
>  userconfig = false;
>  break;
> @@ -3203,7 +3200,7 @@ int main(int argc, char **argv, char **envp)
>  }
>  }
>
> -if (defconfig && userconfig) {
> +if (userconfig) {
>  if (qemu_read_default_config_file() < 0) {
>  exit(1);
>  }
> --
> 2.13.5
>
>

Re: [Qemu-devel] [PATCH 2/2] qemu-options: Deprecate -nodefconfig

[Alistair Francis]

On Wed, Sep 27, 2017 at 1:32 PM, Eduardo Habkost  wrote:
> Since 2012 (commit ba6212d8 "Eliminate cpus-x86_64.conf file") we
> have no default config files that would be disabled using
> -nodefconfig.  Update documentation and document -nodefconfig as
> deprecated.
>
> Cc: Markus Armbruster 
> Signed-off-by: Eduardo Habkost 

Acked-by: Alistair Francis 

Thanks,
Alistair

> ---
>  qemu-options.hx | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 39225ae6c3..bc52e79184 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4069,24 +4069,25 @@ output to stdout. This can be later used as input 
> file for @code{-readconfig} op
>  ETEXI
>  DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig,
>  "-nodefconfig\n"
> -"do not load default config files at startup\n",
> +"do not load default config files at startup 
> (deprecated)\n",
>  QEMU_ARCH_ALL)
>  STEXI
>  @item -nodefconfig
>  @findex -nodefconfig
> -Normally QEMU loads configuration files from @var{sysconfdir} and 
> @var{datadir} at startup.
> -The @code{-nodefconfig} option will prevent QEMU from loading any of those 
> config files.
> +This option was used to disable loading of config files from @var{sysconfdir}
> +and @var{datadir}, but it is deprecated as QEMU doesn't load any config files
> +from @var{datadir} anymore.  To disable loading of config files from
> +@var{sysconfdir}, use @code{-no-user-config} instead.
>  ETEXI
>  DEF("no-user-config", 0, QEMU_OPTION_nouserconfig,
>  "-no-user-config\n"
> -"do not load user-provided config files at startup\n",
> +"do not load default user-provided config files at 
> startup\n",
>  QEMU_ARCH_ALL)
>  STEXI
>  @item -no-user-config
>  @findex -no-user-config
>  The @code{-no-user-config} option makes QEMU not load any of the 
> user-provided
> -config files on @var{sysconfdir}, but won't make it skip the QEMU-provided 
> config
> -files from @var{datadir}.
> +config files on @var{sysconfdir}.
>  ETEXI
>  DEF("trace", HAS_ARG, QEMU_OPTION_trace,
>  "-trace [[enable=]][,events=][,file=]\n"
> --
> 2.13.5
>
>

Re: [Qemu-devel] [PATCH v4 18/23] qemu-img: Change compare_sectors() to be byte-based

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> In the continuing quest to make more things byte-based, change
> compare_sectors(), renaming it to compare_buffers() in the
> process.  Note that one caller (qemu-img compare) only cares
> about the first difference, while the other (qemu-img rebase)
> cares about how many consecutive sectors have the same
> equal/different status; however, this patch does not bother to
> micro-optimize the compare case to avoid the comparisons of
> sectors beyond the first mismatch.  Both callers are always
> passing valid buffers in, so the initial check for buffer size
> can be turned into an assertion.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v3: new patch
> ---
>  qemu-img.c | 55 +++
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 2e05f92e85..034122eba5 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1155,31 +1155,28 @@ static int is_allocated_sectors_min(const uint8_t 
> *buf, int n, int *pnum,
>  }
> 
>  /*
> - * Compares two buffers sector by sector. Returns 0 if the first sector of 
> both
> - * buffers matches, non-zero otherwise.
> + * Compares two buffers sector by sector. Returns 0 if the first
> + * sector of each buffer matches, non-zero otherwise.
>   *
> - * pnum is set to the number of sectors (including and immediately following
> - * the first one) that are known to have the same comparison result
> + * pnum is set to the sector-aligned size of the buffer prefix that
> + * has the same matching status as the first sector.
>   */
> -static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
> -int *pnum)
> +static int compare_buffers(const uint8_t *buf1, const uint8_t *buf2,
> +   int64_t bytes, int64_t *pnum)
>  {
>  bool res;
> -int i;
> +int64_t i = MIN(bytes, BDRV_SECTOR_SIZE);
> 
> -if (n <= 0) {
> -*pnum = 0;
> -return 0;
> -}
> +assert(bytes > 0);
> 
> -res = !!memcmp(buf1, buf2, 512);
> -for(i = 1; i < n; i++) {
> -buf1 += 512;
> -buf2 += 512;
> +res = !!memcmp(buf1, buf2, i);

It is temporarily confusing that 'i' is never again used for this
particular parameter, because

> +while (i < bytes) {

This gives the brief impression that we might be looping in a way that
changes the comparison size passed to memcmp, which isn't true.

Just me being cranky, though. It's probably still the best way, because
of how you have to prime the loop. Doing it the literal-minded way
requires an extra i += len, so:

Reviewed-by: John Snow 

Re: [Qemu-devel] HDMI Question

[Alistair Francis]

On Wed, Sep 27, 2017 at 5:31 AM, Ni, Xingrong  wrote:
> Hello,
>
> I am using qemu and hit a question and would like to ask here, can we 
> simulate a HDMI device in Qemu, if so, how to do it?

Hello,

I don't see any reason why you can't. We (Xilinx) have a display port
device in QEMU that we use.

Basically how it works is you model the devices registers and use
QEMUs display backend (usually over SDL or VNC) to display the image.

Here is a pretty recent example of a display port device in QEMU:
https://github.com/qemu/qemu/blob/master/hw/display/xlnx_dp.c

In saying that display devices are pretty intense and this will not be
a trivial task.

Thanks,
Alistair

>
>
> Thanks & Best Wishes!
> Rocky
>
>

Re: [Qemu-devel] [PATCH v4 01/23] block: Allow NULL file for bdrv_get_block_status()

[Eric Blake]

On 09/25/2017 05:43 PM, John Snow wrote:
> 
> 
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> Not all callers care about which BDS owns the mapping for a given
>> range of the file.  This patch merely simplifies the callers by
>> consolidating the logic in the common call point, while guaranteeing
>> a non-NULL file to all the driver callbacks, for no semantic change.
>> The only caller that does not care about pnum is bdrv_is_allocated,
>> as invoked by vvfat; we can likewise add assertions that the rest
>> of the stack does not have to worry about a NULL pnum.
>>
>> Furthermore, this will also set the stage for a future cleanup: when
>> a caller does not care about which BDS owns an offset, it would be
>> nice to allow the driver to optimize things to not have to return
>> BDRV_BLOCK_OFFSET_VALID in the first place.  In the case of fragmented
>> allocation (for example, it's fairly easy to create a qcow2 image
>> where consecutive guest addresses are not at consecutive host
>> addresses), the current contract requires bdrv_get_block_status()
>> to clamp *pnum to the limit where host addresses are no longer
>> consecutive, but allowing a NULL file means that *pnum could be
>> set to the full length of known-allocated data.
>>
> 
> Function reads slightly worse for the wear now with all of the return
> logic handled at various places within, but unifying it might be even
> stranger, perhaps..
> 
> Let's see if I hate this more:
> 
> out:
> bdrv_dec_in_flight(bs);
> bdrv_dec_in_flight(bs);
> if (ret >= 0 && sector_num + *pnum == total_sectors) {
> ret |= BDRV_BLOCK_EOF;
> }
> early_out:
> if (file) {
> *file = local_file;
> }
> return ret;
> 
> 
> and then earlier in the function, we can just:
> 
> if (total_sectors < 0) {
>   ret = total_sectors;
>   goto early_out;
> }

Seems reasonable enough, I'll work that in to v5, since there are other
reasons to respin the series anyway.

> 
> It's only shed paint, though:
> 
> Reviewed-by: John Snow 
> 
> I'm looking at the rest of the series now, so please stand by.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 00/26] ppc-for-2.11 queue 20170927

[Peter Maydell]

On 27 September 2017 at 00:42, David Gibson <da...@gibson.dropbear.id.au> wrote:
> The following changes since commit 31bc1d8481af414cfa2857f905e40f7d8e6d5b2e:
>
>   Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into 
> staging (2017-09-26 19:49:08 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.11-20170927
>
> for you to fetch changes up to e451b85f1bf3c8140be51e2b03eb71ab96c246a5:
>
>   macio: use object link between MACIO_IDE and MAC_DBDMA object (2017-09-27 
> 13:05:41 +1000)
>
> 
> ppc patch queue 2017-09-27
>
> Contains
>  * a number of Mac machine type fixes
>  * a number of embedded machine type fixes (preliminary to adding the
>Sam460ex board)
>  * a important fix for handling of migration with KVM PR
>  * assorted other minor fixes and cleanups
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v4 17/23] qemu-img: Change check_empty_sectors() to byte-based

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> Continue on the quest to make more things byte-based instead of
> sector-based.
> 
> Signed-off-by: Eric Blake 


Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> If a read error is encountered during 'qemu-img compare', we
> were printing the "Error while reading offset ..." message twice.
> Update the testsuite for the improved output.
> 
> Further simplify the code by hoisting the error code conversion
> into the helper function, rather than repeating it at the callers.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v3: new patch
> ---
>  qemu-img.c | 19 +--
>  tests/qemu-iotests/074.out |  2 --
>  2 files changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index dfccebe6bc..3e1e373e8f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1196,8 +1196,10 @@ static int64_t sectors_to_bytes(int64_t sectors)
>  /*
>   * Check if passed sectors are empty (not allocated or contain only 0 bytes)
>   *
> - * Returns 0 in case sectors are filled with 0, 1 if sectors contain non-zero
> - * data and negative value on error.
> + * Intended for use by 'qemu-img compare': Returns 0 in case sectors are
> + * filled with 0, 1 if sectors contain non-zero data (this is a comparison
> + * failure), and 4 on error (the exit status for read errors), after emitting
> + * an error message.
>   *
>   * @param blk:  BlockBackend for the image
>   * @param sect_num: Number of first sector to check
> @@ -1218,7 +1220,7 @@ static int check_empty_sectors(BlockBackend *blk, 
> int64_t sect_num,
>  if (ret < 0) {
>  error_report("Error while reading offset %" PRId64 " of %s: %s",
>   sectors_to_bytes(sect_num), filename, strerror(-ret));
> -return ret;
> +return 4;
>  }
>  idx = find_nonzero(buffer, sect_count * BDRV_SECTOR_SIZE);
>  if (idx >= 0) {
> @@ -1473,11 +1475,6 @@ static int img_compare(int argc, char **argv)
>filename2, buf1, quiet);
>  }
>  if (ret) {
> -if (ret < 0) {
> -error_report("Error while reading offset %" PRId64 ": 
> %s",
> - sectors_to_bytes(sector_num), 
> strerror(-ret));
> -ret = 4;
> -}
>  goto out;
>  }
>  }
> @@ -1522,12 +1519,6 @@ static int img_compare(int argc, char **argv)
>  ret = check_empty_sectors(blk_over, sector_num, nb_sectors,
>filename_over, buf1, quiet);
>  if (ret) {
> -if (ret < 0) {
> -error_report("Error while reading offset %" PRId64
> - " of %s: %s", 
> sectors_to_bytes(sector_num),
> - filename_over, strerror(-ret));
> -ret = 4;
> -}
>  goto out;
>  }
>  }
> diff --git a/tests/qemu-iotests/074.out b/tests/qemu-iotests/074.out
> index 8fba5aea9c..ede66c3f81 100644
> --- a/tests/qemu-iotests/074.out
> +++ b/tests/qemu-iotests/074.out
> @@ -4,7 +4,6 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
>  wrote 512/512 bytes at offset 512
>  512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  qemu-img: Error while reading offset 0 of 
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> -qemu-img: Error while reading offset 0: Input/output error
>  4
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
>  Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
> @@ -12,7 +11,6 @@ Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
>  wrote 512/512 bytes at offset 512
>  512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  qemu-img: Error while reading offset 0 of 
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> -qemu-img: Error while reading offset 0 of 
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
>  Warning: Image size mismatch!
>  4
>  Cleanup
> 

Hm, naively I might assume it's best for the caller to report the error
and to leave the function a nicely self-contained helper, but I won't
insist on it.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] iotests: Fix 195 if IMGFMT is part of TEST_DIR

[Eric Blake]

On 09/27/2017 04:13 PM, Max Reitz wrote:
> do_run_qemu() in iotest 195 first applies _filter_imgfmt when printing
> qemu's command line and _filter_testdir only afterwards.  Therefore, if
> the image format is part of the test directory path, _filter_testdir
> will no longer apply and the actual output will differ from the
> reference output even in case of success.
> 
> For example, TEST_DIR might be "/tmp/test-qcow2", in which case
> _filter_imgfmt first transforms this to "/tmp/test-IMGFMT" which is no
> longer recognized as the TEST_DIR by _filter_testdir.
> 
> Fix this by not applying _filter_imgfmt in do_run_qemu() but in
> run_qemu() instead, and only after _filter_testdir.
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/195 | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 15/23] qemu-img: Add find_nonzero()

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> During 'qemu-img compare', when we are checking that an allocated
> portion of one file is all zeros, we don't need to waste time
> computing how many additional sectors after the first non-zero
> byte are also non-zero.  Create a new helper find_nonzero() to do
> the check for a first non-zero sector, and rebase
> check_empty_sectors() to use it.
> 
> The new interface intentionally uses bytes in its interface, even
> though it still crawls the buffer a sector at a time; it is robust
> to a partial sector at the end of the buffer.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: John Snow 

[Qemu-devel] [PATCH] iotests: Fix 195 if IMGFMT is part of TEST_DIR

[Max Reitz]

do_run_qemu() in iotest 195 first applies _filter_imgfmt when printing
qemu's command line and _filter_testdir only afterwards.  Therefore, if
the image format is part of the test directory path, _filter_testdir
will no longer apply and the actual output will differ from the
reference output even in case of success.

For example, TEST_DIR might be "/tmp/test-qcow2", in which case
_filter_imgfmt first transforms this to "/tmp/test-IMGFMT" which is no
longer recognized as the TEST_DIR by _filter_testdir.

Fix this by not applying _filter_imgfmt in do_run_qemu() but in
run_qemu() instead, and only after _filter_testdir.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/195 | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/195 b/tests/qemu-iotests/195
index 05a239cbf5..e7a403ded2 100755
--- a/tests/qemu-iotests/195
+++ b/tests/qemu-iotests/195
@@ -44,15 +44,16 @@ _supported_os Linux
 
 function do_run_qemu()
 {
-echo Testing: "$@" | _filter_imgfmt
+echo Testing: "$@"
 $QEMU -nographic -qmp-pretty stdio -serial none "$@"
 echo
 }
 
 function run_qemu()
 {
-do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp \
-  | _filter_qemu_io | _filter_generated_node_ids
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt | _filter_qemu \
+  | _filter_qmp | _filter_qemu_io \
+  | _filter_generated_node_ids
 }
 
 size=64M
-- 
2.13.5

Re: [Qemu-devel] [PATCH v4 0/6] Misc improvements to crypto block driver

[Max Reitz]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> This is a followup to
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00781.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06464.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02923.html
> 
> This collection of patches first improves the performance of the
> crypto block driver and then does various cleanups to improve ongoing
> maint work.
> 
> Changed in v4:
> 
>   - Drop intermediate patch that replaced '512' with a constant (Max)
>   - Use MIN() macro where needed (Max)
>   - Fix bounce buffer size at 1MB instead of varying per sector size (Max)
>   - Convert missing qcrypto_block_encrypt call to sectors in qcow.c (Max)
> 
> Changed in v3:
> 
>   - Support passthrough of BDRV_REQ_FUA (Eric)
>   - Fix potential truncation of payload offset values (Eric)
>   - Use encryption scheme sector size instead of BDRV_SECTOR_SIZE (Kevin)
>   - Use QEMU_IS_ALIGNED where appropriate (Eric)
>   - Remove unused 'sector_num' variable (Eric)
>   - Fix whitespace alignment (Eric)
>   - Fix math error in qcow conversion (Eric)
> 
> Daniel P. Berrange (6):
>   block: use 1 MB bounce buffers for crypto instead of 16KB
>   crypto: expose encryption sector size in APIs
>   block: fix data type casting for crypto payload offset
>   block: convert crypto driver to bdrv_co_preadv|pwritev
>   block: convert qcrypto_block_encrypt|decrypt to take bytes offset
>   block: support passthrough of BDRV_REQ_FUA in crypto driver
> 
>  block/crypto.c | 130 
> ++---
>  block/qcow.c   |  11 +++--
>  block/qcow2-cluster.c  |   8 ++-
>  block/qcow2.c  |   4 +-
>  crypto/block-luks.c|  18 ---
>  crypto/block-qcow.c|  13 +++--
>  crypto/block.c |  26 +++---
>  crypto/blockpriv.h |   5 +-
>  include/crypto/block.h |  29 ---
>  9 files changed, 148 insertions(+), 96 deletions(-)

Thanks; hoping that is OK with you, I've applied this series to my block
branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 14/23] qemu-img: Speed up compare on pre-allocated larger file

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> Compare the following images with all-zero contents:
> $ truncate --size 1M A
> $ qemu-img create -f qcow2 -o preallocation=off B 1G
> $ qemu-img create -f qcow2 -o preallocation=metadata C 1G
> 
> On my machine, the difference is noticeable for pre-patch speeds,
> with more than an order of magnitude in difference caused by the
> choice of preallocation in the qcow2 file:
> 
> $ time ./qemu-img compare -f raw -F qcow2 A B
> Warning: Image size mismatch!
> Images are identical.
> 
> real  0m0.014s
> user  0m0.007s
> sys   0m0.007s
> 
> $ time ./qemu-img compare -f raw -F qcow2 A C
> Warning: Image size mismatch!
> Images are identical.
> 
> real  0m0.341s
> user  0m0.144s
> sys   0m0.188s
> 
> Why? Because bdrv_is_allocated() returns false for image B but
> true for image C, throwing away the fact that both images know
> via lseek(SEEK_HOLE) that the entire image still reads as zero.
> From there, qemu-img ends up calling bdrv_pread() for every byte
> of the tail, instead of quickly looking for the next allocation.
> The solution: use block_status instead of is_allocated, giving:
> 
> $ time ./qemu-img compare -f raw -F qcow2 A C
> Warning: Image size mismatch!
> Images are identical.
> 
> real  0m0.014s
> user  0m0.011s
> sys   0m0.003s
> 
> which is on par with the speeds for no pre-allocation.
> 
> Signed-off-by: Eric Blake 

Makes good sense to me.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v4 5/6] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Max Reitz]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Instead of sector offset, take the bytes offset when encrypting
> or decrypting data.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 12 
>  block/qcow.c   | 11 +++
>  block/qcow2-cluster.c  |  8 +++-
>  block/qcow2.c  |  4 ++--
>  crypto/block-luks.c| 12 
>  crypto/block-qcow.c| 12 
>  crypto/block.c | 20 ++--
>  crypto/blockpriv.h |  4 ++--
>  include/crypto/block.h | 14 --
>  9 files changed, 56 insertions(+), 41 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 1/2] config: qemu_config_parse() return number of config groups

[Eric Blake]

On 09/27/2017 03:41 PM, Eduardo Habkost wrote:
> Change qemu_config_parse() to return the number of config groups
> in success and -EINVAL on error. This will allow callers of
> qemu_config_parse() to check if something was really loaded from
> the config file.
> 
> All existing callers of qemu_config_parse() and
> qemu_read_config_file() only check if the return value was
> negative, so the change shouldn't affect them.
> 
> Signed-off-by: Eduardo Habkost 
> ---

Reviewed-by: Eric Blake 

> Changes v2 -> v3:
> * None (rebase only)
> 
> Changes v1 -> v2:
> * Remove unnecessary translation of qemu_config_parse()
>   erros to -EINVAL at block/blkdebug.c:read_config()
>   * Suggsted-by: Markus Armbruster 

Do you want the Suggested-by (spelled correctly) in the commit body proper?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 4/6] block: convert crypto driver to bdrv_co_preadv|pwritev

[Max Reitz]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Make the crypto driver implement the bdrv_co_preadv|pwritev
> callbacks, and also use bdrv_co_preadv|pwritev for I/O
> with the protocol driver beneath. This replaces sector based
> I/O with byte based I/O, and allows us to stop assuming the
> physical sector size matches the encryption sector size.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 106 
> +
>  1 file changed, 54 insertions(+), 52 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] vl: Print warning when a default config file is loaded

[Eric Blake]

On 09/27/2017 03:41 PM, Eduardo Habkost wrote:
> In case there were options set in the default config file, print
> a warning so users can update their scripts.
> 
> If somebody wants to keep the config file as-is, avoid the
> warning and use a command-line that will work in future QEMU
> versions, they can use:
> 
>  $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf
> 
> I was going to add an additional message suggesting it as a
> solution, but I thought it could make it more confusing. The
> solution can be documented in the QEMU 2.11 ChangeLog.
> 
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v2 -> v3:
> * Rebase (no code changes)
> * Commit message update: suggest -no-user-config

> +if (ret > 0) {
> +loc_set_none();
> +error_report("Warning: Future QEMU versions won't load %s 
> automatically",
> + CONFIG_QEMU_CONFDIR "/qemu.conf");

Shouldn't we now use warn_report() with no 'Warning:' prefix?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 1/2] config: qemu_config_parse() return number of config groups

[Eduardo Habkost]

Change qemu_config_parse() to return the number of config groups
in success and -EINVAL on error. This will allow callers of
qemu_config_parse() to check if something was really loaded from
the config file.

All existing callers of qemu_config_parse() and
qemu_read_config_file() only check if the return value was
negative, so the change shouldn't affect them.

Signed-off-by: Eduardo Habkost 
---
Changes v2 -> v3:
* None (rebase only)

Changes v1 -> v2:
* Remove unnecessary translation of qemu_config_parse()
  erros to -EINVAL at block/blkdebug.c:read_config()
  * Suggsted-by: Markus Armbruster 
---
 block/blkdebug.c   |  1 -
 util/qemu-config.c | 15 +++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 46e53f2f09..dfdf9b91aa 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -244,7 +244,6 @@ static int read_config(BDRVBlkdebugState *s, const char 
*filename,
 ret = qemu_config_parse(f, config_groups, filename);
 if (ret < 0) {
 error_setg(errp, "Could not parse blkdebug config file");
-ret = -EINVAL;
 goto fail;
 }
 }
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 405dd1a1d7..99b0e46fa3 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -385,6 +385,7 @@ void qemu_config_write(FILE *fp)
 }
 }
 
+/* Returns number of config groups on success, -errno on error */
 int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
 {
 char line[1024], group[64], id[64], arg[64], value[1024];
@@ -392,7 +393,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 QemuOptsList *list = NULL;
 Error *local_err = NULL;
 QemuOpts *opts = NULL;
-int res = -1, lno = 0;
+int res = -EINVAL, lno = 0;
+int count = 0;
 
 loc_push_none();
 while (fgets(line, sizeof(line), fp) != NULL) {
@@ -413,6 +415,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 goto out;
 }
 opts = qemu_opts_create(list, id, 1, NULL);
+count++;
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
@@ -423,6 +426,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 goto out;
 }
 opts = qemu_opts_create(list, NULL, 0, _abort);
+count++;
 continue;
 }
 value[0] = '\0';
@@ -447,7 +451,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 error_report("error reading file");
 goto out;
 }
-res = 0;
+res = count;
 out:
 loc_pop();
 return res;
@@ -464,12 +468,7 @@ int qemu_read_config_file(const char *filename)
 
 ret = qemu_config_parse(f, vm_config_groups, filename);
 fclose(f);
-
-if (ret == 0) {
-return 0;
-} else {
-return -EINVAL;
-}
+return ret;
 }
 
 static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
-- 
2.13.5

[Qemu-devel] [PATCH v3 2/2] vl: Print warning when a default config file is loaded

[Eduardo Habkost]

In case there were options set in the default config file, print
a warning so users can update their scripts.

If somebody wants to keep the config file as-is, avoid the
warning and use a command-line that will work in future QEMU
versions, they can use:

 $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf

I was going to add an additional message suggesting it as a
solution, but I thought it could make it more confusing. The
solution can be documented in the QEMU 2.11 ChangeLog.

Reviewed-by: Markus Armbruster 
Signed-off-by: Eduardo Habkost 
---
Changes v2 -> v3:
* Rebase (no code changes)
* Commit message update: suggest -no-user-config
---
 vl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/vl.c b/vl.c
index 4fd01fda91..888649bbd1 100644
--- a/vl.c
+++ b/vl.c
@@ -3048,6 +3048,12 @@ static int qemu_read_default_config_file(void)
 return ret;
 }
 
+if (ret > 0) {
+loc_set_none();
+error_report("Warning: Future QEMU versions won't load %s 
automatically",
+ CONFIG_QEMU_CONFDIR "/qemu.conf");
+}
+
 return 0;
 }
 
-- 
2.13.5

[Qemu-devel] [PATCH v3 0/2] vl: Print warning if a non-empty default config file is found

[Eduardo Habkost]

This missed v2.9 and v2.10.  Let's try again: we can include this
on v2.11, and remove the default config file in QEMU 2.12 or
2.13.

Changes v2 -> v3:
* Rebase to latest qemu.git master

Changes v1 -> v2:
* Remove unnecessary translation of qemu_config_parse()
  erros to -EINVAL at block/blkdebug.c:read_config()
  * Suggsted-by: Markus Armbruster 

We plan to remove support for /etc/qemu/qemu.conf in the near
future. Make QEMU print a warning in case there a non-empty
/etc/qemu/qemu.conf is loaded, so users have time to adapt.

Eduardo Habkost (2):
  config: qemu_config_parse() return number of config groups
  vl: Print warning when a default config file is loaded

 block/blkdebug.c   |  1 -
 util/qemu-config.c | 15 +++
 vl.c   |  6 ++
 3 files changed, 13 insertions(+), 9 deletions(-)

-- 
2.13.5

Re: [Qemu-devel] [PATCH v4 1/6] block: use 1 MB bounce buffers for crypto instead of 16KB

[Max Reitz]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Using 16KB bounce buffers creates a significant performance
> penalty for I/O to encrypted volumes on storage which high
> I/O latency (rotating rust & network drives), because it
> triggers lots of fairly small I/O operations.
> 
> On tests with rotating rust, and cache=none|directsync,
> write speed increased from 2MiB/s to 32MiB/s, on a par
> with that achieved by the in-kernel luks driver. With
> other cache modes the in-kernel driver is still notably
> faster because it is able to report completion of the
> I/O request before any encryption is done, while the
> in-QEMU driver must encrypt the data before completion.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 28 +++-
>  1 file changed, 15 insertions(+), 13 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3] spapr: disable decrementer during reset

[Cédric Le Goater]

On 07/17/2017 06:16 AM, Nikunj A Dadhania wrote:
> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG.
> 
> When reset happens, all the CPUs are in halted state. First CPU is brought out
> of reset and secondary CPUs would be initialized by the guest kernel using a
> rtas call start-cpu.
> 
> However, in case of TCG, decrementer interrupts keep on coming and waking the
> secondary CPUs up.
> 
> These secondary CPUs would see the decrementer interrupt pending, which makes
> cpu::has_work() to bring them out of wait loop and start executing
> tcg_exec_cpu().
> 
> The problem with this is all the CPUs wake up and start booting SLOF image,
> causing the following exception(4 CPUs TCG VM):
> 
> [   81.440850] reboot: Restarting system
> 
> SLOF
> S
> SLOF
> SLOFLOF[0[0m 
> **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> [0m **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> [0m *m**[?25l 
> **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> ***
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> ERROR: Flatten device tree not available!
> 
>  exception 300
> SRR0 = 60e4  SRR1 = 80008000
> SPRG2 = 0040  SPRG3 = 4bd8
> ERROR: Flatten device tree not available!
> 
>  exception 300
> SRR0 = 60e4  SRR1 = 80008000
> SPRG2 = 0040  SPRG3 = 4bd8
> 
> During reset, disable decrementer interrupt for secondary CPUs and enable them
> when the secondary CPUs are brought online by rtas start-cpu call.
> 
> Reported-by: Bharata B Rao 
> Signed-off-by: Nikunj A Dadhania 
> ---
>  hw/ppc/spapr_cpu_core.c | 9 +
>  hw/ppc/spapr_rtas.c | 8 
>  2 files changed, 17 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ea278ce..bbfe8c2 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -87,6 +87,15 @@ static void spapr_cpu_reset(void *opaque)
>  
>  env->spr[SPR_HIOR] = 0;
>  
> +/* Disable DECR for secondary cpus */
> +if (cs != first_cpu) {
> +if (env->mmu_model == POWERPC_MMU_3_00) {
> +env->spr[SPR_LPCR] &= ~LPCR_DEE;
> +} else {
> +/* P7 and P8 both have same bit for DECR */
> +env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
> +}
> +}
>  /*
>   * This is a hack for the benefit of KVM PR - it abuses the SDR1
>   * slot in kvm_sregs to communicate the userspace address of the
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 94a2799..4623d1d 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -174,6 +174,14 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, 
> sPAPRMachineState *spapr,
>  kvm_cpu_synchronize_state(cs);
>  
>  env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +
> +/* Enable DECR interrupt */
> +if (env->mmu_model == POWERPC_MMU_3_00) {
> +env->spr[SPR_LPCR] |= LPCR_DEE;
> +} else {
> +/* P7 and P8 both have same bit for DECR */
> +env->spr[SPR_LPCR] |= LPCR_P8_PECE3;
> +}
>  env->nip = start;
>  env->gpr[3] = r3;
>  cs->halted = 0;


you should also disable the decrementer in the stop-cpu call
when a cpu is hot unplugged.

One thing I don't explain is why the decrementer interrupt 
does not wake up the cpu after being  hot unplugged. I am not 
sure the code doing :

env->msr = 0;

is responsible of this behavior. 

With the xive patchset, I am having issues in that area. 
the decrementer wakes up the cpu just after hot unplugged
and I need to set the LPCR to disable it. 

I wonder why XICS is immune to that. 

C.

[Qemu-devel] [PATCH 2/2] qemu-options: Deprecate -nodefconfig

[Eduardo Habkost]

Since 2012 (commit ba6212d8 "Eliminate cpus-x86_64.conf file") we
have no default config files that would be disabled using
-nodefconfig.  Update documentation and document -nodefconfig as
deprecated.

Cc: Markus Armbruster 
Signed-off-by: Eduardo Habkost 
---
 qemu-options.hx | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 39225ae6c3..bc52e79184 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4069,24 +4069,25 @@ output to stdout. This can be later used as input file 
for @code{-readconfig} op
 ETEXI
 DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig,
 "-nodefconfig\n"
-"do not load default config files at startup\n",
+"do not load default config files at startup 
(deprecated)\n",
 QEMU_ARCH_ALL)
 STEXI
 @item -nodefconfig
 @findex -nodefconfig
-Normally QEMU loads configuration files from @var{sysconfdir} and 
@var{datadir} at startup.
-The @code{-nodefconfig} option will prevent QEMU from loading any of those 
config files.
+This option was used to disable loading of config files from @var{sysconfdir}
+and @var{datadir}, but it is deprecated as QEMU doesn't load any config files
+from @var{datadir} anymore.  To disable loading of config files from
+@var{sysconfdir}, use @code{-no-user-config} instead.
 ETEXI
 DEF("no-user-config", 0, QEMU_OPTION_nouserconfig,
 "-no-user-config\n"
-"do not load user-provided config files at startup\n",
+"do not load default user-provided config files at 
startup\n",
 QEMU_ARCH_ALL)
 STEXI
 @item -no-user-config
 @findex -no-user-config
 The @code{-no-user-config} option makes QEMU not load any of the user-provided
-config files on @var{sysconfdir}, but won't make it skip the QEMU-provided 
config
-files from @var{datadir}.
+config files on @var{sysconfdir}.
 ETEXI
 DEF("trace", HAS_ARG, QEMU_OPTION_trace,
 "-trace [[enable=]][,events=][,file=]\n"
-- 
2.13.5

[Qemu-devel] [PATCH 1/2] vl: Eliminate defconfig variable

[Eduardo Habkost]

Both -nodefconfig and -no-user-config options do the same thing
today, we only need one variable to keep track of them.

Suggested-by: Markus Armbruster 
Signed-off-by: Eduardo Habkost 
---
 vl.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 4fd01fda91..b347a97a5b 100644
--- a/vl.c
+++ b/vl.c
@@ -3093,7 +3093,6 @@ int main(int argc, char **argv, char **envp)
 const char *qtest_log = NULL;
 const char *pid_file = NULL;
 const char *incoming = NULL;
-bool defconfig = true;
 bool userconfig = true;
 bool nographic = false;
 DisplayType display_type = DT_DEFAULT;
@@ -3194,8 +3193,6 @@ int main(int argc, char **argv, char **envp)
 popt = lookup_opt(argc, argv, , );
 switch (popt->index) {
 case QEMU_OPTION_nodefconfig:
-defconfig = false;
-break;
 case QEMU_OPTION_nouserconfig:
 userconfig = false;
 break;
@@ -3203,7 +3200,7 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
-if (defconfig && userconfig) {
+if (userconfig) {
 if (qemu_read_default_config_file() < 0) {
 exit(1);
 }
-- 
2.13.5

[Qemu-devel] [PATCH 0/2] Deprecate -nodefconfig

[Eduardo Habkost]

Since 2012 (commit ba6212d8 "Eliminate cpus-x86_64.conf file") we
have no default config files that would be disabled using
-nodefconfig.  This series cleans up the code, updates
documentation, and document -nodefconfig as deprecated.

Eduardo Habkost (2):
  vl: Eliminate defconfig variable
  qemu-options: Deprecate -nodefconfig

 vl.c|  5 +
 qemu-options.hx | 13 +++--
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 4/5] pci: Add INTERFACE_CONVENTIONAL_PCI_DEVICE to Conventional PCI devices

[Eduardo Habkost]

Add INTERFACE_CONVENTIONAL_PCI_DEVICE to all direct subtypes of
TYPE_PCI_DEVICE, except:

1) The ones that already have INTERFACE_PCIE_DEVICE set:

* base-xhci
* e1000e
* nvme
* pvscsi
* vfio-pci
* virtio-pci
* vmxnet3

2) base-pci-bridge

Not all PCI bridges are Conventional PCI devices, so
INTERFACE_CONVENTIONAL_PCI_DEVICE is added only to the subtypes
that are actually Conventional PCI:

* dec-21154-p2p-bridge
* i82801b11-bridge
* pbm-bridge
* pci-bridge

The direct subtypes of base-pci-bridge not touched by this patch
are:

* xilinx-pcie-root: Already marked as PCIe-only.
* pcie-pci-bridge: Already marked as PCIe-only.
* pcie-port: all non-abstract subtypes of pcie-port are already
  marked as PCIe-only devices.

3) megasas-base

Not all megasas devices are Conventional PCI devices, so the
interface names are added to the subclasses registered by
megasas_register_types(), according to information in the
megasas_devices[] array.

"megasas-gen2" already implements INTERFACE_PCIE_DEVICE, so add
INTERFACE_CONVENTIONAL_PCI_DEVICE only to "megasas".

Acked-by: Alberto Garcia 
Acked-by: John Snow 
Acked-by: Anthony PERARD 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* s/legacy/conventional/
  * Suggested-by: Alex Williamson 
* Note about pcie-pci-bridge on commit message.
* New devices: sungem, sunhme

Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Gerd Hoffmann 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: John Snow 
Cc: Alberto Garcia 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Jiri Slaby 
Cc: Alexander Graf 
Cc: Marcel Apfelbaum 
Cc: Jason Wang 
Cc: Jiri Pirko 
Cc: "Hervé Poussineau" 
Cc: Peter Maydell 
Cc: David Gibson 
Cc: Hannes Reinecke 
Cc: Mark Cave-Ayland 
Cc: Artyom Tarasenko 
Cc: Alex Williamson 
Cc: qemu-devel@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/acpi/piix4.c |  1 +
 hw/audio/ac97.c |  4 
 hw/audio/es1370.c   |  4 
 hw/audio/intel-hda.c|  4 
 hw/char/serial-pci.c| 12 
 hw/display/cirrus_vga.c |  4 
 hw/display/qxl.c|  4 
 hw/display/sm501.c  |  4 
 hw/display/vga-pci.c|  4 
 hw/display/vmware_vga.c |  4 
 hw/i2c/smbus_ich9.c |  4 
 hw/i386/amd_iommu.c |  4 
 hw/i386/kvm/pci-assign.c|  4 
 hw/i386/pc_piix.c   |  4 
 hw/i386/xen/xen_platform.c  |  4 
 hw/i386/xen/xen_pvdevice.c  |  4 
 hw/ide/ich.c|  4 
 hw/ide/pci.c|  4 
 hw/ipack/tpci200.c  |  4 
 hw/isa/i82378.c |  4 
 hw/isa/lpc_ich9.c   |  1 +
 hw/isa/piix4.c  |  4 
 hw/isa/vt82c686.c   | 16 
 hw/mips/gt64xxx_pci.c   |  4 
 hw/misc/edu.c   |  5 +
 hw/misc/ivshmem.c   |  4 
 hw/misc/macio/macio.c   |  4 
 hw/misc/pci-testdev.c   |  4 
 hw/net/e1000.c  |  4 
 hw/net/eepro100.c   |  4 
 hw/net/ne2000.c |  4 
 hw/net/pcnet-pci.c  |  4 
 hw/net/rocker/rocker.c  |  4 
 hw/net/rtl8139.c|  4 
 hw/net/sungem.c |  4 
 hw/net/sunhme.c |  4 
 hw/pci-bridge/dec.c |  8 
 hw/pci-bridge/i82801b11.c   |  4 
 hw/pci-bridge/pci_bridge_dev.c  |  1 +
 hw/pci-bridge/pci_expander_bridge.c |  8 
 hw/pci-host/apb.c   |  8 
 hw/pci-host/bonito.c|  4 
 hw/pci-host/gpex.c  |  4 
 hw/pci-host/grackle.c   |  4 
 hw/pci-host/piix.c  |  8 
 hw/pci-host/ppce500.c   |  4 
 hw/pci-host/prep.c  |  4 
 hw/pci-host/q35.c   |  4 
 hw/pci-host/uninorth.c  | 16 
 hw/pci-host/versatile.c |  4 
 hw/ppc/ppc4xx_pci.c

[Qemu-devel] [PATCH v2 5/5] pci: Validate interfaces on base_class_init

[Eduardo Habkost]

Make sure we don't forget to add the Conventional PCI or PCI
Express interface names on PCI device classes in the future.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* s/legacy/conventional/
  * Suggested-by: Alex Williamson 
---
 hw/pci/pci.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 1b08e18205..5ed3c8dca4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2547,6 +2547,17 @@ static void pci_device_class_init(ObjectClass *klass, 
void *data)
 pc->realize = pci_default_realize;
 }
 
+static void pci_device_class_base_init(ObjectClass *klass, void *data)
+{
+if (!object_class_is_abstract(klass)) {
+ObjectClass *conventional =
+object_class_dynamic_cast(klass, 
INTERFACE_CONVENTIONAL_PCI_DEVICE);
+ObjectClass *pcie =
+object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE);
+assert(conventional || pcie);
+}
+}
+
 AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
 {
 PCIBus *bus = PCI_BUS(dev->bus);
@@ -2671,6 +2682,7 @@ static const TypeInfo pci_device_type_info = {
 .abstract = true,
 .class_size = sizeof(PCIDeviceClass),
 .class_init = pci_device_class_init,
+.class_base_init = pci_device_class_base_init,
 };
 
 static void pci_register_types(void)
-- 
2.13.5

[Qemu-devel] [PATCH v2 3/5] pci: Add INTERFACE_PCIE_DEVICE to all PCIe devices

[Eduardo Habkost]

Change all devices that set is_express=1 to implement
INTERFACE_PCIE_DEVICE.

Cc: Keith Busch 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Dmitry Fleytman 
Cc: Jason Wang 
Cc: "Michael S. Tsirkin" 
Cc: Marcel Apfelbaum 
Cc: Paul Burton 
Cc: Paolo Bonzini 
Cc: Hannes Reinecke 
Cc: qemu-bl...@nongnu.org
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* base-xhci is marked as hybrid, now (in another patch)
* Included pcie-pci-bridge
---
 hw/block/nvme.c| 4 
 hw/net/e1000e.c| 4 
 hw/pci-bridge/pcie_pci_bridge.c| 1 +
 hw/pci-bridge/pcie_root_port.c | 4 
 hw/pci-bridge/xio3130_downstream.c | 4 
 hw/pci-bridge/xio3130_upstream.c   | 4 
 hw/pci-host/xilinx-pcie.c  | 4 
 hw/scsi/megasas.c  | 6 ++
 8 files changed, 31 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9aa32692a3..441e21ed1f 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1110,6 +1110,10 @@ static const TypeInfo nvme_info = {
 .instance_size = sizeof(NvmeCtrl),
 .class_init= nvme_class_init,
 .instance_init = nvme_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void nvme_register_types(void)
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 6c42b4478c..81f7934a59 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -708,6 +708,10 @@ static const TypeInfo e1000e_info = {
 .instance_size = sizeof(E1000EState),
 .class_init = e1000e_class_init,
 .instance_init = e1000e_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void e1000e_register_types(void)
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index 9aa5cc3e45..88db143633 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -180,6 +180,7 @@ static const TypeInfo pcie_pci_bridge_info = {
 .class_init = pcie_pci_bridge_class_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_HOTPLUG_HANDLER },
+{ INTERFACE_PCIE_DEVICE },
 { },
 }
 };
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 4d588cb22e..9b6e4ce512 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -161,6 +161,10 @@ static const TypeInfo rp_info = {
 .class_init= rp_class_init,
 .abstract  = true,
 .class_size = sizeof(PCIERootPortClass),
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void rp_register_types(void)
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index e706f36cb7..7d2f7629c1 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -195,6 +195,10 @@ static const TypeInfo xio3130_downstream_info = {
 .name  = "xio3130-downstream",
 .parent= TYPE_PCIE_SLOT,
 .class_init= xio3130_downstream_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xio3130_downstream_register_types(void)
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index a052224bbf..227997ce46 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -166,6 +166,10 @@ static const TypeInfo xio3130_upstream_info = {
 .name  = "x3130-upstream",
 .parent= TYPE_PCIE_PORT,
 .class_init= xio3130_upstream_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xio3130_upstream_register_types(void)
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 4613dda1d2..7659253090 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -317,6 +317,10 @@ static const TypeInfo xilinx_pcie_root_info = {
 .parent = TYPE_PCI_BRIDGE,
 .instance_size = sizeof(XilinxPCIERoot),
 .class_init = xilinx_pcie_root_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xilinx_pcie_register(void)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 0db68aacee..535ee267c3 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2451,6 +2451,7 @@ typedef struct MegasasInfo {
 int osts;
 const VMStateDescription *vmsd;
 Property *props;
+InterfaceInfo *interfaces;
 } MegasasInfo;
 
 static struct MegasasInfo megasas_devices[] = {
@@ -2480,6 +2481,10 @@ static struct MegasasInfo 

[Qemu-devel] [PATCH v2 0/5] Mark conventional/PCIe/hybrid PCI devices using interface names

[Eduardo Habkost]

Changes v1 -> v2:
* Use "Conventional PCI" instead of "legacy PCI"
  * Suggested-by: Alex Williamson 
* Mark base-xhci as hybrid too
  * Reported-by: Marcel Apfelbaum 
* Mark pcie-pci-bridge as PCI Express only
* New Conventional PCI devices: sungem, sunhme

v1 was a reimplementation of one portion of an old RFC:
  Subject: [RFC v2 00/20] qmp: Report bus information on
   'query-machines'

This series marks each PCI device type as Conventional PCI, PCI
Express, or "hybrid".  It uses two new QOM interface names to do
that: INTERFACE_CONVENTIONAL_PCI_DEVICE
("conventional-pci-device") and INTERFACE_PCIE_DEVICE
("pci-express-device").  Conventional PCI devices will implement
only the former; PCIe-only devices will implement only the
latter; hybrid devices will implement both interfaces.

With this, management software will then be able to use
qom-list-types to find out which PCI devices are
conventional/express/hybrid.

In the future, the new interface names can be used in the
bus/slot querying commands, to indicate which types of devices
are accepted on each slot.

The last patch in the series adds an assertion to the PCI device
class code, to ensure we won't forget to add the corresponding
interface names to new PCI device classes.

Eduardo Habkost (5):
  pci: conventional-pci-device and pci-express-device interfaces
  pci: Add interface names to hybrid PCI devices
  pci: Add INTERFACE_PCIE_DEVICE to all PCIe devices
  pci: Add INTERFACE_CONVENTIONAL_PCI_DEVICE to Conventional PCI devices
  pci: Validate interfaces on base_class_init

 include/hw/pci/pci.h|  6 ++
 hw/acpi/piix4.c |  1 +
 hw/audio/ac97.c |  4 
 hw/audio/es1370.c   |  4 
 hw/audio/intel-hda.c|  4 
 hw/block/nvme.c |  4 
 hw/char/serial-pci.c| 12 
 hw/display/cirrus_vga.c |  4 
 hw/display/qxl.c|  4 
 hw/display/sm501.c  |  4 
 hw/display/vga-pci.c|  4 
 hw/display/vmware_vga.c |  4 
 hw/i2c/smbus_ich9.c |  4 
 hw/i386/amd_iommu.c |  4 
 hw/i386/kvm/pci-assign.c|  4 
 hw/i386/pc_piix.c   |  4 
 hw/i386/xen/xen_platform.c  |  4 
 hw/i386/xen/xen_pvdevice.c  |  4 
 hw/ide/ich.c|  4 
 hw/ide/pci.c|  4 
 hw/ipack/tpci200.c  |  4 
 hw/isa/i82378.c |  4 
 hw/isa/lpc_ich9.c   |  1 +
 hw/isa/piix4.c  |  4 
 hw/isa/vt82c686.c   | 16 
 hw/mips/gt64xxx_pci.c   |  4 
 hw/misc/edu.c   |  5 +
 hw/misc/ivshmem.c   |  4 
 hw/misc/macio/macio.c   |  4 
 hw/misc/pci-testdev.c   |  4 
 hw/net/e1000.c  |  4 
 hw/net/e1000e.c |  4 
 hw/net/eepro100.c   |  4 
 hw/net/ne2000.c |  4 
 hw/net/pcnet-pci.c  |  4 
 hw/net/rocker/rocker.c  |  4 
 hw/net/rtl8139.c|  4 
 hw/net/sungem.c |  4 
 hw/net/sunhme.c |  4 
 hw/net/vmxnet3.c|  5 +
 hw/pci-bridge/dec.c |  8 
 hw/pci-bridge/i82801b11.c   |  4 
 hw/pci-bridge/pci_bridge_dev.c  |  1 +
 hw/pci-bridge/pci_expander_bridge.c |  8 
 hw/pci-bridge/pcie_pci_bridge.c |  1 +
 hw/pci-bridge/pcie_root_port.c  |  4 
 hw/pci-bridge/xio3130_downstream.c  |  4 
 hw/pci-bridge/xio3130_upstream.c|  4 
 hw/pci-host/apb.c   |  8 
 hw/pci-host/bonito.c|  4 
 hw/pci-host/gpex.c  |  4 
 hw/pci-host/grackle.c   |  4 
 hw/pci-host/piix.c  |  8 
 hw/pci-host/ppce500.c   |  4 
 hw/pci-host/prep.c  |  4 
 hw/pci-host/q35.c   |  4 
 hw/pci-host/uninorth.c  | 16 
 hw/pci-host/versatile.c |  4 
 hw/pci-host/xilinx-pcie.c   |  4 
 hw/pci/pci.c| 24 
 hw/ppc/ppc4xx_pci.c |  4 
 hw/scsi/esp-pci.c   |  4 
 hw/scsi/lsi53c895a.c|  4 
 hw/scsi/megasas.c   | 10 ++
 hw/scsi/mptsas.c|  4 
 hw/scsi/vmw_pvscsi.c|  2 ++
 hw/sd/sdhci.c   |  4 
 hw/sh4/sh_pci.c |  4 
 hw/sparc64/sun4u.c  |  4 
 hw/usb/hcd-ehci-pci.c   |  4 
 hw/usb/hcd-ohci.c

[Qemu-devel] [PATCH v2 2/5] pci: Add interface names to hybrid PCI devices

[Eduardo Habkost]

The following devices support both PCI Express and Conventional
PCI, by including special code to handle the QEMU_PCI_CAP_EXPRESS
flag and/or conditional pcie_endpoint_cap_init() calls:

* vfio-pci (is_express=1, but legacy PCI handled by
  vfio_populate_device())
* vmxnet3 (is_express=0, but PCIe handled by vmxnet3_realize())
* pvscsi (is_express=0, but PCIe handled by pvscsi_realize())
* virtio-pci (is_express=0, but PCIe handled by
  virtio_pci_dc_realize(), and additional legacy PCI code at
  virtio_pci_realize())
* base-xhci (is_express=1, but pcie_endpoint_cap_init() call
  is conditional on pci_bus_is_express(dev->bus)
  * Note that xhci does not clear QEMU_PCI_CAP_EXPRESS like the
other hybrid devices

Cc: Dmitry Fleytman 
Cc: Jason Wang 
Cc: Paolo Bonzini 
Cc: Gerd Hoffmann 
Cc: Alex Williamson 
Cc: "Michael S. Tsirkin" 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* s/legacy/conventional/
  * Suggested-by: Alex Williamson 
* Mark base-xhci as hybrid too
---
 hw/net/vmxnet3.c   | 5 +
 hw/scsi/vmw_pvscsi.c   | 2 ++
 hw/usb/hcd-xhci.c  | 5 +
 hw/vfio/pci.c  | 5 +
 hw/virtio/virtio-pci.c | 5 +
 5 files changed, 22 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a19a7a31dd..f99d9a69ec 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2651,6 +2651,11 @@ static const TypeInfo vmxnet3_info = {
 .instance_size = sizeof(VMXNET3State),
 .class_init= vmxnet3_class_init,
 .instance_init = vmxnet3_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ }
+},
 };
 
 static void vmxnet3_register_types(void)
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 6d3f0bf11d..d6b315f8b2 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1300,6 +1300,8 @@ static const TypeInfo pvscsi_info = {
 .class_init= pvscsi_class_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_HOTPLUG_HANDLER },
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
 { }
 }
 };
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d75c085d94..af3a9d88de 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3670,6 +3670,11 @@ static const TypeInfo xhci_info = {
 .instance_size = sizeof(XHCIState),
 .class_init= xhci_class_init,
 .abstract  = true,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ }
+},
 };
 
 static void qemu_xhci_class_init(ObjectClass *klass, void *data)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 31e1edf447..913433d6ba 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3023,6 +3023,11 @@ static const TypeInfo vfio_pci_dev_info = {
 .class_init = vfio_pci_dev_class_init,
 .instance_init = vfio_instance_init,
 .instance_finalize = vfio_instance_finalize,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ }
+},
 };
 
 static void register_vfio_pci_dev_type(void)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 8b0d6b69cd..67c8ab63ad 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1958,6 +1958,11 @@ static const TypeInfo virtio_pci_info = {
 .class_init= virtio_pci_class_init,
 .class_size= sizeof(VirtioPCIClass),
 .abstract  = true,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ }
+},
 };
 
 /* virtio-blk-pci */
-- 
2.13.5

[Qemu-devel] [PATCH v2 1/5] pci: conventional-pci-device and pci-express-device interfaces

[Eduardo Habkost]

Those two interfaces will be used to indicate which device types
support Conventional PCI or PCI Express buses.  Management
software will be able to use the qom-list-types QMP command to
query that information.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* s/legacy/conventional/
  * Suggested-by: Alex Williamson 
---
 include/hw/pci/pci.h |  6 ++
 hw/pci/pci.c | 12 
 2 files changed, 18 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index aa7ef9cf69..8d02a0a383 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -198,6 +198,12 @@ enum {
 #define PCI_DEVICE_GET_CLASS(obj) \
  OBJECT_GET_CLASS(PCIDeviceClass, (obj), TYPE_PCI_DEVICE)
 
+/* Implemented by devices that can be plugged on PCI Express buses */
+#define INTERFACE_PCIE_DEVICE "pci-express-device"
+
+/* Implemented by devices that can be plugged on Conventional PCI buses */
+#define INTERFACE_CONVENTIONAL_PCI_DEVICE "conventional-pci-device"
+
 typedef struct PCIINTxRoute {
 enum {
 PCI_INTX_ENABLED,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 1e6fb88eba..1b08e18205 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -170,6 +170,16 @@ static const TypeInfo pci_bus_info = {
 .class_init = pci_bus_class_init,
 };
 
+static const TypeInfo pcie_interface_info = {
+.name  = INTERFACE_PCIE_DEVICE,
+.parent= TYPE_INTERFACE,
+};
+
+static const TypeInfo conventional_pci_interface_info = {
+.name  = INTERFACE_CONVENTIONAL_PCI_DEVICE,
+.parent= TYPE_INTERFACE,
+};
+
 static const TypeInfo pcie_bus_info = {
 .name = TYPE_PCIE_BUS,
 .parent = TYPE_PCI_BUS,
@@ -2667,6 +2677,8 @@ static void pci_register_types(void)
 {
 type_register_static(_bus_info);
 type_register_static(_bus_info);
+type_register_static(_pci_interface_info);
+type_register_static(_interface_info);
 type_register_static(_device_type_info);
 }
 
-- 
2.13.5

[Qemu-devel] [Bug 1719984] Re: wrgsbase misemulated in x86_64-softmmu

[Todd Eisenberger]

For further data, the faulting instruction is
f3 48 0f ae df  wrgsbase %rdi

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

Title:
  wrgsbase misemulated in x86_64-softmmu

Status in QEMU:
  New

Bug description:
  qemu revision: cfe4cade054c0e0d00d0185cdc433a9e3ce3e2e4
  command: ./qemu-system-x86_64 -m 2048 -nographic -net none -smp 4,threads=2 
-machine q35 -kernel zircon.bin -cpu Haswell,+smap,-check -initrd bootdata.bin 
-append 'TERM=screen kernel.halt-on-panic=true '

  On this revision, the VM reports CPUID.07H.0H.EBX[0] = 1.  In this VM,
  with CR4[16] set to 1, wrgsbase triggers #UD, which mismatches the
  behavior described in Intel's instruction reference.

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

Re: [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command

[Michael S. Tsirkin]

On Wed, Sep 27, 2017 at 06:09:42PM +0300, Jan Dakinevich wrote:
> The command is intended for exposing device specific virtio feature bits
> and their negotiation status. It is convenient and useful for debug
> purpose.
> 
> Names of features are taken from a devices via get_feature_name() within
> VirtioDeviceClass. If certain device doesn't implement it, the command
> will print only hexadecimal value of feature mask.
> 
> Cc: Denis V. Lunev 
> Signed-off-by: Jan Dakinevich 

OK but if it's useful as an hmp command, why not as a qmp command?

> ---
> v2:
> * Moved hmp_info_virtio and stuff to hmp.c to avoid build error
> * Added printing of device status
> * Listed common virtio features
> 
> v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
> ---
>  hmp-commands-info.hx|  14 +
>  hmp.c   | 148 
> 
>  hmp.h   |   1 +
>  hw/block/virtio-blk.c   |  20 ++
>  hw/char/virtio-serial-bus.c |  15 +
>  hw/display/virtio-gpu.c |  12 
>  hw/net/virtio-net.c |  34 ++
>  hw/scsi/virtio-scsi.c   |  15 +
>  hw/virtio/virtio-balloon.c  |  15 +
>  hw/virtio/virtio-bus.c  |   1 +
>  include/hw/virtio/virtio.h  |   2 +
>  monitor.c   |   1 +
>  12 files changed, 278 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 4f1ece9..2550027 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -868,6 +868,20 @@ ETEXI
>  },
>  
>  STEXI
> +@item info virtio
> +@findex virtio
> +Display guest and host fetures for all virtio devices.
> +ETEXI
> +
> +{
> +.name   = "virtio",
> +.args_type  = "",
> +.params = "",
> +.help   = "show virtio info",
> +.cmd= hmp_info_virtio,
> +},
> +
> +STEXI
>  @end table
>  ETEXI
>  
> diff --git a/hmp.c b/hmp.c
> index ace729d..009d808 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -43,6 +43,7 @@
>  #include "hw/intc/intc.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
> +#include "hw/virtio/virtio.h"
>  
>  #ifdef CONFIG_SPICE
>  #include 
> @@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon, const 
> QDict *qdict)
>  }
>  hmp_handle_error(mon, );
>  }
> +
> +#define HMP_INFO_VIRTIO_INDENT 2
> +#define HMP_INFO_VIRTIO_FIELD 32
> +
> +static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
> +{
> +uint8_t status[] = {
> +VIRTIO_CONFIG_S_ACKNOWLEDGE,
> +VIRTIO_CONFIG_S_DRIVER,
> +VIRTIO_CONFIG_S_DRIVER_OK,
> +VIRTIO_CONFIG_S_FEATURES_OK,
> +VIRTIO_CONFIG_S_NEEDS_RESET,
> +VIRTIO_CONFIG_S_FAILED,
> +};
> +const char *names[] = {
> +"acknowledge",
> +"driver",
> +"driver_ok",
> +"features_ok",
> +"needs_reset"
> +"failed",
> +};
> +const char *comma = "";
> +int idx;
> +
> +monitor_printf(mon, "%*sstatus:   0x%02"PRIx8" ",
> +   HMP_INFO_VIRTIO_INDENT, "", vdev->status);
> +
> +for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
> +if (!(vdev->status & status[idx])) {
> +continue;
> +}
> +monitor_printf(mon, "%s%s", comma, names[idx]);
> +if (names[idx]) {
> +comma = ",";
> +}
> +}
> +monitor_printf(mon, "\n");
> +}
> +
> +static void hmp_info_virtio_print_common_features(Monitor *mon,
> +  VirtIODevice *vdev)
> +{
> +uint64_t features[] = {
> +VIRTIO_RING_F_INDIRECT_DESC,
> +VIRTIO_RING_F_EVENT_IDX,
> +VIRTIO_F_NOTIFY_ON_EMPTY,
> +VIRTIO_F_ANY_LAYOUT,
> +VIRTIO_F_IOMMU_PLATFORM,
> +VIRTIO_F_VERSION_1,
> +};
> +const char *names[] = {
> +"indirect_desc",
> +"event_idx",
> +"notify_on_empty",
> +"any_layout",
> +"iommu_platform",
> +"version_1",
> +};
> +int idx;
> +
> +monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
> +
> +for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
> +const char *ack = vdev->guest_features & features[idx] ? "acked" : 
> "";
> +
> +if (!(vdev->host_features & features[idx])) {
> +continue;
> +}
> +monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
> +   HMP_INFO_VIRTIO_FIELD, names[idx],
> +   HMP_INFO_VIRTIO_FIELD, ack);
> +}
> +}
> +
> +static void hmp_info_virtio_print_specific_features(Monitor *mon,
> +VirtIODevice *vdev)
> +{
> +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> +int idx;
> +
> +if (!vdc->get_feature_name) {
> +return;
> +}
> +
> +monitor_printf(mon, 

Re: [Qemu-devel] [PATCH v4 12/23] block: Convert bdrv_get_block_status_above() to bytes

[John Snow]

On 09/27/2017 02:57 PM, Eric Blake wrote:
> On 09/27/2017 01:41 PM, John Snow wrote:
>>
>>
>> On 09/13/2017 12:03 PM, Eric Blake wrote:
>>> We are gradually moving away from sector-based interfaces, towards
>>> byte-based.  In the common case, allocation is unlikely to ever use
>>> values that are not naturally sector-aligned, but it is possible
>>> that byte-based values will let us be more precise about allocation
>>> at the end of an unaligned file that can do byte-based access.
>>>
>>> Changing the name of the function from bdrv_get_block_status_above()
>>> to bdrv_block_status_above() ensures that the compiler enforces that
>>> all callers are updated.  For now, the io.c layer still assert()s
>>> that all callers are sector-aligned, but that can be relaxed when a
>>> later patch implements byte-based block status in the drivers.
>>>
>>> For the most part this patch is just the addition of scaling at the
>>> callers followed by inverse scaling at bdrv_block_status().  But some
>>> code, particularly bdrv_block_status(), gets a lot simpler because
>>> it no longer has to mess with sectors.  Likewise, mirror code no
>>> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore
>>> drop an assertion (fix a neighboring assertion to use is_power_of_2
>>> while there).
>>>
>>
>> Huh, I suppose so, yeah. Do you have a test that covers what happens in
>> this newly available use case?
> 
> Not directly - the mirror code no longer requires sector alignment, but
> is still unlikely to use sub-sector requests unless a particular driver
> returns really small status information.  I suppose we could tweak the
> blkdebug driver to force status requests to be fragmented at
> ridiculously small alignments, and then prove that mirroring still
> occurs correctly, once all the series are in, but it's probably more
> effort than it is worth to force sub-sector mirroring if we don't have a
> real use case that will rely on it.
> 

Hmm, yeah, the code probably can't be exercised currently but I do
wonder if we're removing too many breadcrumbs for potential problem
spots if someone decides to return sub-sector information in the future.

Well, I suppose I haven't been too diligent about complaining about
their removal elsewhere, so for consistency:

Either with or without the assertion removed as you see fit:

Reviewed-by: John Snow 

>>
>>> For ease of review, bdrv_get_block_status() was tackled separately.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>
>> Looks mechanically correct, anyway.
>>
>> Reviewed-by: John Snow 
>>
> 

Re: [Qemu-devel] kvm_intel fails to load on Conroe CPUs running Linux 4.12

[Gerhard Wiesinger]

On 15.09.2017 19:07, Paolo Bonzini wrote:

On 15/09/2017 16:43, Gerhard Wiesinger wrote:

On 27.08.2017 20:55, Paolo Bonzini wrote:


Il 27 ago 2017 4:48 PM, "Gerhard Wiesinger" > ha scritto:

 On 27.08.2017 14 :03, Paolo Bonzini wrote:


 We will revert the patch, but 4.13.0 will not have the fix.
 Expect it in later stable kernels (because vacations).


 Thnx. Why will 4.13.0 NOT have the fix?


Because maintainers are on vacation! :-)



Hello Paolo,

Any update on this for 4.12 and 4.13 kernels?

A late fix is better than a wrong fix.  Hope to get to it next week!


Hello Paolo,

Any update? Thnx.

Ciao,
Gerhard

[Qemu-devel] [Bug 1719984] [NEW] wrgsbase misemulated in x86_64-softmmu

[Todd Eisenberger]

Public bug reported:

qemu revision: cfe4cade054c0e0d00d0185cdc433a9e3ce3e2e4
command: ./qemu-system-x86_64 -m 2048 -nographic -net none -smp 4,threads=2 
-machine q35 -kernel zircon.bin -cpu Haswell,+smap,-check -initrd bootdata.bin 
-append 'TERM=screen kernel.halt-on-panic=true '

On this revision, the VM reports CPUID.07H.0H.EBX[0] = 1.  In this VM,
with CR4[16] set to 1, wrgsbase triggers #UD, which mismatches the
behavior described in Intel's instruction reference.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  wrgsbase misemulated in x86_64-softmmu

Status in QEMU:
  New

Bug description:
  qemu revision: cfe4cade054c0e0d00d0185cdc433a9e3ce3e2e4
  command: ./qemu-system-x86_64 -m 2048 -nographic -net none -smp 4,threads=2 
-machine q35 -kernel zircon.bin -cpu Haswell,+smap,-check -initrd bootdata.bin 
-append 'TERM=screen kernel.halt-on-panic=true '

  On this revision, the VM reports CPUID.07H.0H.EBX[0] = 1.  In this VM,
  with CR4[16] set to 1, wrgsbase triggers #UD, which mismatches the
  behavior described in Intel's instruction reference.

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

Re: [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare()

[Eric Blake]

On 09/27/2017 02:05 PM, John Snow wrote:
> 
> 
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> As long as we are querying the status for a chunk smaller than
>> the known image size, we are guaranteed that a successful return
>> will have set pnum to a non-zero size (pnum is zero only for
>> queries beyond the end of the file).  Use that to slightly
>> simplify the calculation of the current chunk size being compared.
>> Likewise, we don't have to shrink the amount of data operated on
>> until we know we have to read the file, and therefore have to fit
>> in the bounds of our buffer.  Also, note that 'total_sectors_over'
>> is equivalent to 'progress_base'.
>>
>> With these changes in place, sectors_to_process() is now dead code,
>> and can be removed.
>>
>> Signed-off-by: Eric Blake 
>>

>> @@ -1402,14 +1393,9 @@ static int img_compare(int argc, char **argv)
>>  goto out;
>>  }
>>  allocated2 = status2 & BDRV_BLOCK_ALLOCATED;
>> -if (pnum1) {
>> -nb_sectors = MIN(nb_sectors,
>> - DIV_ROUND_UP(pnum1, BDRV_SECTOR_SIZE));
>> -}
>> -if (pnum2) {
>> -nb_sectors = MIN(nb_sectors,
>> - DIV_ROUND_UP(pnum2, BDRV_SECTOR_SIZE));
>> -}
>> +
>> +assert(pnum1 && pnum2);
>> +nb_sectors = DIV_ROUND_UP(MIN(pnum1, pnum2), BDRV_SECTOR_SIZE);
> 
> In the apocalyptic future where non-sector sized returns are possible,
> does this math make sense?
> 
> e.g. say the return is zeroes, but it's not aligned anymore, so we
> assume we have an extra half a sector's worth of zeroes here.

Not introduced in this patch, but a good question for 12/23.  We want to
round up rather than down to ensure that we don't inf-loop on a partial
sector response; but at the same time, you're right that if we got a
report of a half-sector zero and we widen it, we can't guarantee that
the second half is zero.

On the bright side, this rounding goes away when later patches switch
img_compare to be byte-based, later in this series.  But you're right
that it is probably smarter to have 12/23 assert that things are already
aligned (and thus we don't need to round in the first place).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare()

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> As long as we are querying the status for a chunk smaller than
> the known image size, we are guaranteed that a successful return
> will have set pnum to a non-zero size (pnum is zero only for
> queries beyond the end of the file).  Use that to slightly
> simplify the calculation of the current chunk size being compared.
> Likewise, we don't have to shrink the amount of data operated on
> until we know we have to read the file, and therefore have to fit
> in the bounds of our buffer.  Also, note that 'total_sectors_over'
> is equivalent to 'progress_base'.
> 
> With these changes in place, sectors_to_process() is now dead code,
> and can be removed.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v3: new patch
> ---
>  qemu-img.c | 40 +++-
>  1 file changed, 11 insertions(+), 29 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index b91133b922..f8423e9b3f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1171,11 +1171,6 @@ static int64_t sectors_to_bytes(int64_t sectors)
>  return sectors << BDRV_SECTOR_BITS;
>  }
> 
> -static int64_t sectors_to_process(int64_t total, int64_t from)
> -{
> -return MIN(total - from, IO_BUF_SIZE >> BDRV_SECTOR_BITS);
> -}
> -
>  /*
>   * Check if passed sectors are empty (not allocated or contain only 0 bytes)
>   *
> @@ -1372,13 +1367,9 @@ static int img_compare(int argc, char **argv)
>  goto out;
>  }
> 
> -for (;;) {
> +while (sector_num < total_sectors) {
>  int64_t status1, status2;
> 
> -nb_sectors = sectors_to_process(total_sectors, sector_num);
> -if (nb_sectors <= 0) {
> -break;
> -}
>  status1 = bdrv_block_status_above(bs1, NULL,
>sector_num * BDRV_SECTOR_SIZE,
>(total_sectors1 - sector_num) *
> @@ -1402,14 +1393,9 @@ static int img_compare(int argc, char **argv)
>  goto out;
>  }
>  allocated2 = status2 & BDRV_BLOCK_ALLOCATED;
> -if (pnum1) {
> -nb_sectors = MIN(nb_sectors,
> - DIV_ROUND_UP(pnum1, BDRV_SECTOR_SIZE));
> -}
> -if (pnum2) {
> -nb_sectors = MIN(nb_sectors,
> - DIV_ROUND_UP(pnum2, BDRV_SECTOR_SIZE));
> -}
> +
> +assert(pnum1 && pnum2);
> +nb_sectors = DIV_ROUND_UP(MIN(pnum1, pnum2), BDRV_SECTOR_SIZE);

In the apocalyptic future where non-sector sized returns are possible,
does this math make sense?

e.g. say the return is zeroes, but it's not aligned anymore, so we
assume we have an extra half a sector's worth of zeroes here.

> 
>  if (strict) {
>  if ((status1 & ~BDRV_BLOCK_OFFSET_MASK) !=
> @@ -1422,9 +1408,10 @@ static int img_compare(int argc, char **argv)
>  }
>  }
>  if ((status1 & BDRV_BLOCK_ZERO) && (status2 & BDRV_BLOCK_ZERO)) {
> -nb_sectors = DIV_ROUND_UP(MIN(pnum1, pnum2), BDRV_SECTOR_SIZE);
> +/* nothing to do */
>  } else if (allocated1 == allocated2) {
>  if (allocated1) {
> +nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> 
> BDRV_SECTOR_BITS);
>  ret = blk_pread(blk1, sector_num << BDRV_SECTOR_BITS, buf1,
>  nb_sectors << BDRV_SECTOR_BITS);
>  if (ret < 0) {
> @@ -1453,7 +1440,7 @@ static int img_compare(int argc, char **argv)
>  }
>  }
>  } else {
> -
> +nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> BDRV_SECTOR_BITS);
>  if (allocated1) {
>  ret = check_empty_sectors(blk1, sector_num, nb_sectors,
>filename1, buf1, quiet);
> @@ -1476,30 +1463,24 @@ static int img_compare(int argc, char **argv)
> 
>  if (total_sectors1 != total_sectors2) {
>  BlockBackend *blk_over;
> -int64_t total_sectors_over;
>  const char *filename_over;
> 
>  qprintf(quiet, "Warning: Image size mismatch!\n");
>  if (total_sectors1 > total_sectors2) {
> -total_sectors_over = total_sectors1;
>  blk_over = blk1;
>  filename_over = filename1;
>  } else {
> -total_sectors_over = total_sectors2;
>  blk_over = blk2;
>  filename_over = filename2;
>  }
> 
> -for (;;) {
> +while (sector_num < progress_base) {
>  int64_t count;
> 
> -nb_sectors = sectors_to_process(total_sectors_over, sector_num);
> -if (nb_sectors <= 0) {
> -break;
> -}
>  ret = bdrv_is_allocated_above(blk_bs(blk_over), NULL,
>sector_num * BDRV_SECTOR_SIZE,
> -  nb_sectors * BDRV_SECTOR_SIZE,
> + 

Re: [Qemu-devel] libvirt/QEMU/SEV interaction

[Richard Relph]

Forgive the top post... some of the conversation has been trimmed, but I 
need to go back to first principles of SEV in order to make sure we all 
have a clear understanding of what the goal is.


The goal - for BOTH guest owner and cloud provider - is to get to a VM 
where ONLY the guest owner (GO) has access to the GO "secrets". 
Legitimate cloud providers (ie, those that wish to not retain a back 
door in to their customer's VMs) want this every bit as much as GOs want 
it. It is this privacy concern that some believe holds back broader 
adoption of public cloud for sensitive applications.


To provide this additional privacy will require some changes to the 
"untrustable" model that seems to be in place now. There is value for 
everyone in creating a "trustable" model.


Given that, the root of the problem for the GO is trust. How can the GO 
know that every instruction in their VM is "theirs"? AMD Epyc CPUs 
decrypt every instruction fetch (and guest page table walk) in an SEV 
guest VM with that VM's random memory encryption key. (Data can either 
be encrypted or not, at the guest's choosing.) Only the SEV FW and the 
guest itself can encrypt memory with that key. The SEV FW measures every 
byte it encrypts for the guest and provides that measurement to the GO. 
The GO is free to ignore the value and run the guest "as-is". But 
recommended practice will be to inspect the measurement, verify it, and 
only then provide the guest VM with "secrets" necessary to decrypt 
disks, connect to privileged network resources, etc.


As has been observed, the BIOS has a great deal of power. It is 
impossible to maintain the GO's privacy in a VM where the BIOS (or any 
other code, for that matter) is "unknown". It simply is a violation of 
the trust model both the GO and the CP want to have to allow unknown 
code from the CP to enter the GO's VM. (Yes, there are LOTS of other 
ways for untrusted code to get in and secrets to get out... we're only 
trying to close this door between the CP and the guest VM at this time. ;-)


I anticipate that legitimate cloud providers will be happy (or at least 
willing) to share with customers the source for their BIOS. The GO can 
inspect the source, build the binary from that source, and generate the 
required hash. Or they may just trust that someone else has done that 
work and accept the hash the CP posts on their BIOS image. (Note that 
when the hash is returned by the SEV FW, it is in HMAC form, with a 
nonce that the GO can compute, and a key the GO provided at launch time.)


Whether the "BIOS" is a "static shim" as Michael suggests, or a full 
BIOS, or even a BIOS+kernel+initrd is really not too significant. What 
is significant is that the GO has a basis for trusting all code that is 
imported in to their VM by the CP. And that NONE of the code provided by 
the CP is "unknown" and unauditable by the GO. If the CP has a way to 
inject code unknown to the GO in to the guest VM, the trust model is 
broken and both GO and CP suffer the consequences.


When the CP needs to update the BIOS image, they will have to inform the 
GO and allow the GO to establish trust in the CP's new BIOS image somehow.


I hope that helps outline what we're doing, and why.

Richard


On 9/27/17 11:12 AM, Michael S. Tsirkin wrote:

On Wed, Sep 27, 2017 at 08:39:24AM -0500, Brijesh Singh wrote:

Hi Michael,


On 09/26/2017 09:36 AM, Michael S. Tsirkin wrote:

...


8. libvirt launches the guest with "-S"
9. While creating the SEV guest qemu does the following
   i) create encryption context using GO's DH, session-info and guest policy
  (LAUNCH_START)
   ii) encrypts the guest bios (LAUNCH_UPDATE_DATA)
   iii) calls LAUNCH_MEASUREMENT to get the encrypted bios measurement


This part troubles me. This seems to mean that the guest being launched
must know what the measurement of the bios is going to be.  This means
that the cloud provider can not update the bios without breaking guests.
Also, while in practice you typically can run an old bios image on a new
qemu instance, this is not really tested so would be very hard to
support properly in QEMU.





The guest itself does not need to know the measurement of the bios --
the SEV launch flow empowers the GO to validate the bootstrap code (bios)
before GO can provide a confidential information to the guest. Please note
that the validating of the measurement flow is optional. GO can ask cloud
provider to ignore the measurement all together and boot the SEV guest.


As the OS runs on top of the bios, it does not seem prudent to boot the
SEV guest without a way to verify that the bios is safe to use.
Linux generally trusts the firmware it runs on. Are you looking for
examples of how a malicious firmware can leak info out of the guest?


The measurement flow can be useful when GO decides to provide a custom
bios and want to know that his bios is used for booting the guest. In this
case, since the guest owner knows the initial contents of the guest 

Re: [Qemu-devel] [PATCH v3 0/9] Support the Capstone disassembler

[Richard Henderson]

On 09/26/2017 01:14 PM, Richard Henderson wrote:
> Richard Henderson (9):
>   target/i386: Convert to disas_set_info hook
>   target/ppc: Convert to disas_set_info hook
>   disas: Remove unused flags arguments
>   disas: Support the Capstone disassembler library
>   i386: Support Capstone in disas_set_info
>   arm: Support Capstone in disas_set_info
>   ppc: Support Capstone in disas_set_info
>   disas: Remove monitor_disas_is_physical
>   disas: Add capstone as submodule

Ho hum, ignore this patch.  Need to handle this in a different way.

And, apparently, figure out what's wrong with the patchew docker builds,
considering that it didn't even check out the capstone submodule...


r~

Re: [Qemu-devel] [PATCH] linux-headers: sync against v4.14-rc1

[Dr. David Alan Gilbert]

cc'ing in Paolo who I think knows more about checking this sync.

Dave

* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Signed-off-by: Alexey Perevalov 
> ---
>  include/standard-headers/asm-x86/hyperv.h| 19 ++---
>  include/standard-headers/linux/pci_regs.h| 42 
> 
>  include/standard-headers/linux/virtio_ring.h |  4 +--
>  linux-headers/asm-s390/kvm.h |  6 
>  linux-headers/linux/kvm.h|  3 +-
>  linux-headers/linux/userfaultfd.h| 16 ++-
>  6 files changed, 64 insertions(+), 26 deletions(-)
> 
> diff --git a/include/standard-headers/asm-x86/hyperv.h 
> b/include/standard-headers/asm-x86/hyperv.h
> index fac7651..5f95d5e 100644
> --- a/include/standard-headers/asm-x86/hyperv.h
> +++ b/include/standard-headers/asm-x86/hyperv.h
> @@ -149,12 +149,9 @@
>   */
>  #define HV_X64_DEPRECATING_AEOI_RECOMMENDED  (1 << 9)
>  
> -/*
> - * HV_VP_SET available
> - */
> +/* Recommend using the newer ExProcessorMasks interface */
>  #define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED(1 << 11)
>  
> -
>  /*
>   * Crash notification flag.
>   */
> @@ -242,7 +239,11 @@
>   (~((1ull << HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT) - 1))
>  
>  /* Declare the various hypercall operations. */
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE   0x0002
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST0x0003
>  #define HVCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX  0x0013
> +#define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX   0x0014
>  #define HVCALL_POST_MESSAGE  0x005c
>  #define HVCALL_SIGNAL_EVENT  0x005d
>  
> @@ -259,6 +260,16 @@
>  #define HV_PROCESSOR_POWER_STATE_C2  2
>  #define HV_PROCESSOR_POWER_STATE_C3  3
>  
> +#define HV_FLUSH_ALL_PROCESSORS  BIT(0)
> +#define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES  BIT(1)
> +#define HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLYBIT(2)
> +#define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT   BIT(3)
> +
> +enum HV_GENERIC_SET_FORMAT {
> + HV_GENERIC_SET_SPARCE_4K,
> + HV_GENERIC_SET_ALL,
> +};
> +
>  /* hypercall status code */
>  #define HV_STATUS_SUCCESS0
>  #define HV_STATUS_INVALID_HYPERCALL_CODE 2
> diff --git a/include/standard-headers/linux/pci_regs.h 
> b/include/standard-headers/linux/pci_regs.h
> index c22d3eb..f8d5804 100644
> --- a/include/standard-headers/linux/pci_regs.h
> +++ b/include/standard-headers/linux/pci_regs.h
> @@ -513,6 +513,7 @@
>  #define  PCI_EXP_DEVSTA_URD  0x0008  /* Unsupported Request Detected */
>  #define  PCI_EXP_DEVSTA_AUXPD0x0010  /* AUX Power Detected */
>  #define  PCI_EXP_DEVSTA_TRPND0x0020  /* Transactions Pending */
> +#define PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V112  /* v1 endpoints without 
> link end here */
>  #define PCI_EXP_LNKCAP   12  /* Link Capabilities */
>  #define  PCI_EXP_LNKCAP_SLS  0x000f /* Supported Link Speeds */
>  #define  PCI_EXP_LNKCAP_SLS_2_5GB 0x0001 /* LNKCAP2 SLS Vector bit 0 */
> @@ -556,7 +557,7 @@
>  #define  PCI_EXP_LNKSTA_DLLLA0x2000  /* Data Link Layer Link Active 
> */
>  #define  PCI_EXP_LNKSTA_LBMS 0x4000  /* Link Bandwidth Management Status */
>  #define  PCI_EXP_LNKSTA_LABS 0x8000  /* Link Autonomous Bandwidth Status */
> -#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V1   20  /* v1 endpoints end 
> here */
> +#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V1   20  /* v1 endpoints with 
> link end here */
>  #define PCI_EXP_SLTCAP   20  /* Slot Capabilities */
>  #define  PCI_EXP_SLTCAP_ABP  0x0001 /* Attention Button Present */
>  #define  PCI_EXP_SLTCAP_PCP  0x0002 /* Power Controller Present */
> @@ -639,7 +640,7 @@
>  #define  PCI_EXP_DEVCTL2_OBFF_MSGB_EN0x4000  /* Enable OBFF Message 
> type B */
>  #define  PCI_EXP_DEVCTL2_OBFF_WAKE_EN0x6000  /* OBFF using WAKE# 
> signaling */
>  #define PCI_EXP_DEVSTA2  42  /* Device Status 2 */
> -#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2   44  /* v2 endpoints end 
> here */
> +#define PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V244  /* v2 endpoints without 
> link end here */
>  #define PCI_EXP_LNKCAP2  44  /* Link Capabilities 2 */
>  #define  PCI_EXP_LNKCAP2_SLS_2_5GB   0x0002 /* Supported Speed 2.5GT/s */
>  #define  PCI_EXP_LNKCAP2_SLS_5_0GB   0x0004 /* Supported Speed 5.0GT/s */
> @@ -647,6 +648,7 @@
>  #define  PCI_EXP_LNKCAP2_CROSSLINK   0x0100 /* Crosslink supported */
>  #define PCI_EXP_LNKCTL2  48  /* Link Control 2 */
>  #define PCI_EXP_LNKSTA2  50  /* Link Status 2 */
> +#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2   52  /* v2 endpoints with 
> link end here */
>  #define PCI_EXP_SLTCAP2  52  /* Slot Capabilities 2 */
>  #define PCI_EXP_SLTCTL2  56  /* Slot Control 2 */
>  #define PCI_EXP_SLTSTA2 

Re: [Qemu-devel] [PATCH v4 12/23] block: Convert bdrv_get_block_status_above() to bytes

[Eric Blake]

On 09/27/2017 01:41 PM, John Snow wrote:
> 
> 
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based.  In the common case, allocation is unlikely to ever use
>> values that are not naturally sector-aligned, but it is possible
>> that byte-based values will let us be more precise about allocation
>> at the end of an unaligned file that can do byte-based access.
>>
>> Changing the name of the function from bdrv_get_block_status_above()
>> to bdrv_block_status_above() ensures that the compiler enforces that
>> all callers are updated.  For now, the io.c layer still assert()s
>> that all callers are sector-aligned, but that can be relaxed when a
>> later patch implements byte-based block status in the drivers.
>>
>> For the most part this patch is just the addition of scaling at the
>> callers followed by inverse scaling at bdrv_block_status().  But some
>> code, particularly bdrv_block_status(), gets a lot simpler because
>> it no longer has to mess with sectors.  Likewise, mirror code no
>> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore
>> drop an assertion (fix a neighboring assertion to use is_power_of_2
>> while there).
>>
> 
> Huh, I suppose so, yeah. Do you have a test that covers what happens in
> this newly available use case?

Not directly - the mirror code no longer requires sector alignment, but
is still unlikely to use sub-sector requests unless a particular driver
returns really small status information.  I suppose we could tweak the
blkdebug driver to force status requests to be fragmented at
ridiculously small alignments, and then prove that mirroring still
occurs correctly, once all the series are in, but it's probably more
effort than it is worth to force sub-sector mirroring if we don't have a
real use case that will rely on it.

> 
>> For ease of review, bdrv_get_block_status() was tackled separately.
>>
>> Signed-off-by: Eric Blake 
>>
> 
> Looks mechanically correct, anyway.
> 
> Reviewed-by: John Snow 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC 1/3] accel/tcg: allow to invalidate a write TLB entry immediately

[David Hildenbrand]

On 27.09.2017 19:48, Richard Henderson wrote:
> On 09/27/2017 10:00 AM, David Hildenbrand wrote:
>> Background: s390x implements Low-Address Protection (LAP). If LAP is
>> enabled, writing to effective addresses (before any transaltion)
>> 0-511 and 4096-4607 triggers a protection exception.
>>
>> So we have subpage protection on the first two pages of every address
>> space (where the lowcore - the CPU private data resides).
>>
>> By immediately invalidating the write entry but allowing the caller to
>> continue, we force every write access onto these first two pages into
>> the slow path. we will get a tlb fault with the specific accessed
>> addresses and can then evaluate if protection applies or not.
>>
>> We have to make sure to ignore the invalid bit if tlb_fill() succeeds.
> 
> This is similar to a scheme I proposed to PMM wrt handling ARM v8M 
> translation.
>  Reusing TLB_INVALID_MASK would appear to work, but I wonder if it wouldn't be
> clearer to use another bit.  I believe I had proposed a TLB_FORCE_SLOW_MASK.

The only downside of another bit is that we have to duplicate all
checks. Using TLB_INVALID_MASK avoids this change (because it simply works).

Of course, we could come up with something as simple as

#define TLB_INVALID_MASK (TLB_INVALID | TLB_FORCE_SLOW)

and fixup the few places where TLB_INVALID_MASK really just has to be
TLB_INVALID.

Have no strong opinion on this. This way requires minimal changes.

> 
> Thoughts, Peter?
> 
> 
> r~
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH RFC 3/3] s390x/tcg: make STFL store into the lowcore

[David Hildenbrand]

On 27.09.2017 19:52, Richard Henderson wrote:
> On 09/27/2017 10:00 AM, David Hildenbrand wrote:
>> Using virtual memory access is wrong and will soon include low-address
>> protection checks, which is to be bypassed for STFL.
>>
>> This was originally part of a bigger STFL(E) refactoring.
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  target/s390x/helper.h  | 2 +-
>>  target/s390x/misc_helper.c | 7 ++-
>>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> Need to sort this patch first, so that the series is bisectable.

Right, this should become #2.

> 
>>  DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
>>  DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
>>  DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
>> -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>>  DEF_HELPER_2(stfle, i32, env, i64)
>>  DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
>>  DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
>> @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, 
>> i64, i64)
>>  DEF_HELPER_1(per_check_exception, void, env)
>>  DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
>>  DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
>> +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>>  
> 
> Why?  Otherwise,

struct LowCore is only available for !CONFIG_USER_ONLY. Therefore I also
have to move the helper declaration into !CONFIG_USER_ONLY.

Thanks!

> 
> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 


-- 

Thanks,

David

[Qemu-devel] [Bug 1713825] Re: Booting Windows 2016 with qxl video crashes qemu

[Maciej Piechotka]

I reproduce it on 2.10.0

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

Title:
  Booting Windows 2016 with qxl video crashes qemu

Status in QEMU:
  New

Bug description:
  launched from libvirt.

  qemu version: 2.9.0
  host: Linux  4.9.34-gentoo #1 SMP Sat Jul 29 13:28:43 PDT 2017 
x86_64 Intel(R) Core(TM) i7-3930K CPU @ 3.20GHz GenuineIntel GNU/Linux
  guest: Windows 2016 64 bit

  Thread 28 (Thread 0x7f0e2edff700 (LWP 29860)):
  #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
  set = {__val = {18446744067266837079, 139698892694944, 
139699853745096, 139700858749789, 4222451712, 139694281220640, 139694281220741, 
139694281220640, 139694281220640, 139694281220810, 
  139694281220940, 139694281220640, 139694281220940, 0, 0, 0}}
  pid = 
  tid = 
  #1  0x7f0ea40b644a in __GI_abort () at abort.c:89
  save_stage = 2
  act = {__sigaction_handler = {sa_handler = 0x7f0e2edfe5c0, 
sa_sigaction = 0x7f0e2edfe5c0}, sa_mask = {__val = {139694281219872, 
139698106269697, 139698892695344, 4, 2676511744, 0, 139698892695144, 0, 
139698892694912, 1, 4737316546111099904, 139700859888720, 
4737316546111099904, 139700862161824, 139700911349760, 94211934977482}}, 
sa_flags = 416, 
sa_restorer = 0x55af6ceb0500 <__PRETTY_FUNCTION__.36381>}
  sigs = {__val = {32, 0 }}
  #2  0x7f0ea40abab6 in __assert_fail_base (fmt=, 
assertion=assertion@entry=0x55af6ceafdca "offset < qxl->vga.vram_size", 
  file=file@entry=0x55af6ceaeaa0 
"/var/tmp/portage/app-emulation/qemu-2.9.0-r2/work/qemu-2.9.0/hw/display/qxl.c",
 line=line@entry=416, 
  function=function@entry=0x55af6ceb0500 <__PRETTY_FUNCTION__.36381> 
"qxl_ram_set_dirty") at assert.c:92
  str = 0x7f0d1c026220 "\340r\002\034\r\177"
  total = 4096
  #3  0x7f0ea40abb81 in __GI___assert_fail 
(assertion=assertion@entry=0x55af6ceafdca "offset < qxl->vga.vram_size", 
  file=file@entry=0x55af6ceaeaa0 
"/var/tmp/portage/app-emulation/qemu-2.9.0-r2/work/qemu-2.9.0/hw/display/qxl.c",
 line=line@entry=416, 
  function=function@entry=0x55af6ceb0500 <__PRETTY_FUNCTION__.36381> 
"qxl_ram_set_dirty") at assert.c:101
  No locals.
  #4  0x55af6cc58805 in qxl_ram_set_dirty (qxl=, 
ptr=) at 
/var/tmp/portage/app-emulation/qemu-2.9.0-r2/work/qemu-2.9.0/hw/display/qxl.c:416
  base = 
  offset = 
  qxl = 
  ptr = 
  base = 
  offset = 
  #5  0x55af6cc5b9e2 in interface_release_resource (sin=0x55af71a91ed0, 
ext=...) at 
/var/tmp/portage/app-emulation/qemu-2.9.0-r2/work/qemu-2.9.0/hw/display/qxl.c:767
  qxl = 0x55af71a91450
  ring = 
  item = 
  id = 18446690739814400920
  __func__ = "interface_release_resource"
  #6  0x7f0ea510afa8 in red_drawable_unref (red_drawable=0x7f0d1c026120) at 
red-worker.c:101
  No locals.
  #7  0x7f0ea510b609 in red_drawable_unref (red_drawable=) 
at red-worker.c:104
  No locals.
  #8  0x7f0ea510eae9 in drawable_unref 
(drawable=drawable@entry=0x7f0e68285ac0) at display-channel.c:1438
  display = 0x55af71dbd3c0
  __FUNCTION__ = "drawable_unref"
  #9  0x7f0ea51109f7 in draw_until (display=display@entry=0x55af71dbd3c0, 
surface=surface@entry=0x7f0e6828aae8, last=0x7f0e68285ac0) at 
display-channel.c:1637
  container = 0x0
  now = 0x7f0e68285ac0
  #10 0x7f0ea510f93f in display_channel_draw (display=0x55af71dbd3c0, 
area=0x7f0e2edfe8e0, surface_id=) at display-channel.c:1729
  surface = 0x7f0e6828aae8
  last = 
  __FUNCTION__ = "display_channel_draw"
  __func__ = "display_channel_draw"

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

Re: [Qemu-devel] [PATCH v4 12/23] block: Convert bdrv_get_block_status_above() to bytes

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  In the common case, allocation is unlikely to ever use
> values that are not naturally sector-aligned, but it is possible
> that byte-based values will let us be more precise about allocation
> at the end of an unaligned file that can do byte-based access.
> 
> Changing the name of the function from bdrv_get_block_status_above()
> to bdrv_block_status_above() ensures that the compiler enforces that
> all callers are updated.  For now, the io.c layer still assert()s
> that all callers are sector-aligned, but that can be relaxed when a
> later patch implements byte-based block status in the drivers.
> 
> For the most part this patch is just the addition of scaling at the
> callers followed by inverse scaling at bdrv_block_status().  But some
> code, particularly bdrv_block_status(), gets a lot simpler because
> it no longer has to mess with sectors.  Likewise, mirror code no
> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore
> drop an assertion (fix a neighboring assertion to use is_power_of_2
> while there).
> 

Huh, I suppose so, yeah. Do you have a test that covers what happens in
this newly available use case?

> For ease of review, bdrv_get_block_status() was tackled separately.
> 
> Signed-off-by: Eric Blake 
> 

Looks mechanically correct, anyway.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v4 11/23] block: Switch bdrv_co_get_block_status_above() to byte-based

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> type (no semantic change), and rename it to match the corresponding
> public function rename.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Fam Zheng 


Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v4 10/23] block: Switch bdrv_common_block_status_above() to byte-based

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> function (no semantic change).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Fam Zheng 

Reviewed-by: John Snow 

[Qemu-devel] [PATCH v3] virtio: introduce `info virtio' hmp command

[Jan Dakinevich]

The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.

Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.

Cc: Denis V. Lunev 
Signed-off-by: Jan Dakinevich 
---

v3:
* Avoid signed int
* Use virtio_vdev_has_feature()/virtio_host_has_feature()

v2: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07527.html
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
---
 hmp-commands-info.hx|  14 +
 hmp.c   | 149 
 hmp.h   |   1 +
 hw/block/virtio-blk.c   |  21 +++
 hw/char/virtio-serial-bus.c |  15 +
 hw/display/virtio-gpu.c |  13 
 hw/net/virtio-net.c |  35 +++
 hw/scsi/virtio-scsi.c   |  16 +
 hw/virtio/virtio-balloon.c  |  15 +
 hw/virtio/virtio-bus.c  |   1 +
 include/hw/virtio/virtio.h  |   2 +
 monitor.c   |   1 +
 12 files changed, 283 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
 },
 
 STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name   = "virtio",
+.args_type  = "",
+.params = "",
+.help   = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index ace729d..f231395 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
 #include "hw/intc/intc.h"
 #include "migration/snapshot.h"
 #include "migration/misc.h"
+#include "hw/virtio/virtio.h"
 
 #ifdef CONFIG_SPICE
 #include 
@@ -2894,3 +2895,151 @@ void hmp_info_memory_size_summary(Monitor *mon, const 
QDict *qdict)
 }
 hmp_handle_error(mon, );
 }
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FIELD 32
+
+static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
+{
+uint8_t status[] = {
+VIRTIO_CONFIG_S_ACKNOWLEDGE,
+VIRTIO_CONFIG_S_DRIVER,
+VIRTIO_CONFIG_S_DRIVER_OK,
+VIRTIO_CONFIG_S_FEATURES_OK,
+VIRTIO_CONFIG_S_NEEDS_RESET,
+VIRTIO_CONFIG_S_FAILED,
+};
+const char *names[] = {
+"acknowledge",
+"driver",
+"driver_ok",
+"features_ok",
+"needs_reset"
+"failed",
+};
+const char *comma = "";
+unsigned idx;
+
+monitor_printf(mon, "%*sstatus:   0x%02"PRIx8" ",
+   HMP_INFO_VIRTIO_INDENT, "", vdev->status);
+
+for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
+if (!(vdev->status & status[idx])) {
+continue;
+}
+monitor_printf(mon, "%s%s", comma, names[idx]);
+if (names[idx]) {
+comma = ",";
+}
+}
+monitor_printf(mon, "\n");
+}
+
+static void hmp_info_virtio_print_common_features(Monitor *mon,
+  VirtIODevice *vdev)
+{
+uint64_t features[] = {
+VIRTIO_RING_F_INDIRECT_DESC,
+VIRTIO_RING_F_EVENT_IDX,
+VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_ANY_LAYOUT,
+VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_VERSION_1,
+};
+const char *names[] = {
+"indirect_desc",
+"event_idx",
+"notify_on_empty",
+"any_layout",
+"iommu_platform",
+"version_1",
+};
+unsigned idx;
+
+monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
+const char *ack = virtio_vdev_has_feature(vdev, features[idx])
+? "acked" : "";
+
+if (!virtio_host_has_feature(vdev, features[idx])) {
+continue;
+}
+monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
+   HMP_INFO_VIRTIO_FIELD, names[idx],
+   HMP_INFO_VIRTIO_FIELD, ack);
+}
+}
+
+static void hmp_info_virtio_print_specific_features(Monitor *mon,
+VirtIODevice *vdev)
+{
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+unsigned idx;
+
+if (!vdc->get_feature_name) {
+return;
+}
+
+monitor_printf(mon, "%*sspecific features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < 64; idx++) {
+const char *name = vdc->get_feature_name(vdev, idx);
+const char *ack = virtio_vdev_has_feature(vdev, idx) ? "acked" : "";
+
+if (!name) {
+continue;
+}
+if (!virtio_host_has_feature(vdev, idx)) {
+continue;
+}
+

Re: [Qemu-devel] [PATCH RFC 3/3] s390x/tcg: make STFL store into the lowcore

[Richard Henderson]

On 09/27/2017 10:00 AM, David Hildenbrand wrote:
> Using virtual memory access is wrong and will soon include low-address
> protection checks, which is to be bypassed for STFL.
> 
> This was originally part of a bigger STFL(E) refactoring.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.h  | 2 +-
>  target/s390x/misc_helper.c | 7 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Need to sort this patch first, so that the series is bisectable.

>  DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
>  DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
>  DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
> -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>  DEF_HELPER_2(stfle, i32, env, i64)
>  DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
>  DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, 
> i64)
>  DEF_HELPER_1(per_check_exception, void, env)
>  DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
>  DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>  

Why?  Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH RFC 2/3] s390x/tcg: low-address protection support

[Richard Henderson]

On 09/27/2017 10:00 AM, David Hildenbrand wrote:
> +case PSW_ASC_HOME:
> +return !(env->cregs[13] & _ASCE_PRIVATE_SPACE);
> +default:
> +/* We don't support access register mode */
> +error_report("unsupported addressing mode");
> +exit(1);

I think g_assert_not_reached() is sufficient for cases like this.

Oh, it's just code movement.  Nevermind, perhaps.
Modulo the outcome of discussion on patch 1,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH RFC 1/3] accel/tcg: allow to invalidate a write TLB entry immediately

[Richard Henderson]

On 09/27/2017 10:00 AM, David Hildenbrand wrote:
> Background: s390x implements Low-Address Protection (LAP). If LAP is
> enabled, writing to effective addresses (before any transaltion)
> 0-511 and 4096-4607 triggers a protection exception.
> 
> So we have subpage protection on the first two pages of every address
> space (where the lowcore - the CPU private data resides).
> 
> By immediately invalidating the write entry but allowing the caller to
> continue, we force every write access onto these first two pages into
> the slow path. we will get a tlb fault with the specific accessed
> addresses and can then evaluate if protection applies or not.
> 
> We have to make sure to ignore the invalid bit if tlb_fill() succeeds.

This is similar to a scheme I proposed to PMM wrt handling ARM v8M translation.
 Reusing TLB_INVALID_MASK would appear to work, but I wonder if it wouldn't be
clearer to use another bit.  I believe I had proposed a TLB_FORCE_SLOW_MASK.

Thoughts, Peter?


r~

Re: [Qemu-devel] [PATCH v7 01/20] hw/arm/smmu-common: smmu base device and datatypes

[Peter Maydell]

On 1 September 2017 at 10:21, Eric Auger  wrote:
> The patch introduces the smmu base device and class for the ARM
> smmu. Devices for specific versions will be derived from this
> base device.
>
> We also introduce some important datatypes.
>
> Signed-off-by: Eric Auger 
> Signed-off-by: Prem Mallappa 

> +/*
> + * Copyright (C) 2014-2016 Broadcom Corporation
> + * Copyright (c) 2017 Red Hat, Inc.
> + * Written by Prem Mallappa, Eric Auger
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

Really GPL-2-only ?

Unless there's a good reason, you should probably fix this (including
getting an ack from Prem/Broadcom where it applies to code that came
from him).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command

[Jan Dakinevich]

On 09/27/2017 06:53 PM, Dr. David Alan Gilbert wrote:
> * Jan Dakinevich (jan.dakinev...@virtuozzo.com) wrote:
>> The command is intended for exposing device specific virtio feature bits
>> and their negotiation status. It is convenient and useful for debug
>> purpose.
>>
>> Names of features are taken from a devices via get_feature_name() within
>> VirtioDeviceClass. If certain device doesn't implement it, the command
>> will print only hexadecimal value of feature mask.
>>
>> Cc: Denis V. Lunev 
>> Signed-off-by: Jan Dakinevich 
>> ---
>> v2:
>> * Moved hmp_info_virtio and stuff to hmp.c to avoid build error
>> * Added printing of device status
>> * Listed common virtio features
>>
>> v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
>> ---
>>  hmp-commands-info.hx|  14 +
>>  hmp.c   | 148 
>> 
>>  hmp.h   |   1 +
>>  hw/block/virtio-blk.c   |  20 ++
>>  hw/char/virtio-serial-bus.c |  15 +
>>  hw/display/virtio-gpu.c |  12 
>>  hw/net/virtio-net.c |  34 ++
>>  hw/scsi/virtio-scsi.c   |  15 +
>>  hw/virtio/virtio-balloon.c  |  15 +
>>  hw/virtio/virtio-bus.c  |   1 +
>>  include/hw/virtio/virtio.h  |   2 +
>>  monitor.c   |   1 +
>>  12 files changed, 278 insertions(+)
>>
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index 4f1ece9..2550027 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -868,6 +868,20 @@ ETEXI
>>  },
>>  
>>  STEXI
>> +@item info virtio
>> +@findex virtio
>> +Display guest and host fetures for all virtio devices.
>> +ETEXI
>> +
>> +{
>> +.name   = "virtio",
>> +.args_type  = "",
>> +.params = "",
>> +.help   = "show virtio info",
>> +.cmd= hmp_info_virtio,
>> +},
>> +
>> +STEXI
>>  @end table
>>  ETEXI
>>  
>> diff --git a/hmp.c b/hmp.c
>> index ace729d..009d808 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -43,6 +43,7 @@
>>  #include "hw/intc/intc.h"
>>  #include "migration/snapshot.h"
>>  #include "migration/misc.h"
>> +#include "hw/virtio/virtio.h"
>>  
>>  #ifdef CONFIG_SPICE
>>  #include 
>> @@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon, 
>> const QDict *qdict)
>>  }
>>  hmp_handle_error(mon, );
>>  }
>> +
>> +#define HMP_INFO_VIRTIO_INDENT 2
>> +#define HMP_INFO_VIRTIO_FIELD 32
>> +
>> +static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
>> +{
>> +uint8_t status[] = {
>> +VIRTIO_CONFIG_S_ACKNOWLEDGE,
>> +VIRTIO_CONFIG_S_DRIVER,
>> +VIRTIO_CONFIG_S_DRIVER_OK,
>> +VIRTIO_CONFIG_S_FEATURES_OK,
>> +VIRTIO_CONFIG_S_NEEDS_RESET,
>> +VIRTIO_CONFIG_S_FAILED,
>> +};
>> +const char *names[] = {
>> +"acknowledge",
>> +"driver",
>> +"driver_ok",
>> +"features_ok",
>> +"needs_reset"
>> +"failed",
>> +};
>> +const char *comma = "";
>> +int idx;
>> +
>> +monitor_printf(mon, "%*sstatus:   0x%02"PRIx8" ",
>> +   HMP_INFO_VIRTIO_INDENT, "", vdev->status);
>> +
>> +for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
>> +if (!(vdev->status & status[idx])) {
>> +continue;
>> +}
>> +monitor_printf(mon, "%s%s", comma, names[idx]);
>> +if (names[idx]) {
>> +comma = ",";
>> +}
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>> +
>> +static void hmp_info_virtio_print_common_features(Monitor *mon,
>> +  VirtIODevice *vdev)
>> +{
>> +uint64_t features[] = {
>> +VIRTIO_RING_F_INDIRECT_DESC,
>> +VIRTIO_RING_F_EVENT_IDX,
>> +VIRTIO_F_NOTIFY_ON_EMPTY,
>> +VIRTIO_F_ANY_LAYOUT,
>> +VIRTIO_F_IOMMU_PLATFORM,
>> +VIRTIO_F_VERSION_1,
>> +};
>> +const char *names[] = {
>> +"indirect_desc",
>> +"event_idx",
>> +"notify_on_empty",
>> +"any_layout",
>> +"iommu_platform",
>> +"version_1",
>> +};
>> +int idx;
>> +
>> +monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, 
>> "");
>> +
>> +for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
>> +const char *ack = vdev->guest_features & features[idx] ? "acked" : 
>> "";
>> +
>> +if (!(vdev->host_features & features[idx])) {
>> +continue;
>> +}
>> +monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
>> +   HMP_INFO_VIRTIO_FIELD, names[idx],
>> +   HMP_INFO_VIRTIO_FIELD, ack);
>> +}
>> +}
>> +
>> +static void hmp_info_virtio_print_specific_features(Monitor *mon,
>> +VirtIODevice *vdev)
>> +{
>> +VirtioDeviceClass *vdc = 

Re: [Qemu-devel] [PULL 00/24] Block layer patches

[Peter Maydell]

On 26 September 2017 at 07:21, Kevin Wolf  wrote:
> The following changes since commit 1e3ee834083227f552179f6e43902cba5a866e6b:
>
>   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into 
> staging (2017-09-25 20:31:24 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to b156d51b62e6970753e1f9f36f7c4d5fdbf4c619:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-2017-09-26' into 
> queue-block (2017-09-26 15:03:02 +0200)
>
> 
> Block layer patches
>
> 

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH RFC 2/3] s390x/tcg: low-address protection support

[David Hildenbrand]

This is a neat way to implement low address protection, whereby
only the first 512 bytes of the first two pages (each 4096 bytes) of
every address space are protected.

Store a tec of 0 for the access exception, this is what is defined by
Enhanced Suppression on Protection in case of a low address protection
(Bit 61 set to 0, rest undefined).

We have to make sure to to pass the access address, not the masked page
address into mmu_translate*().

Drop the check from testblock. So we can properly test this via
kvm-unit-tests.

This will check every access going through one of the MMUs.

Signed-off-by: David Hildenbrand 
---
 target/s390x/excp_helper.c |  3 +-
 target/s390x/mem_helper.c  |  8 
 target/s390x/mmu_helper.c  | 96 +-
 3 files changed, 62 insertions(+), 45 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 3e4349d00b..aa0cbf67ac 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -95,7 +95,6 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr,
 DPRINTF("%s: address 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",
 __func__, orig_vaddr, rw, mmu_idx);
 
-orig_vaddr &= TARGET_PAGE_MASK;
 vaddr = orig_vaddr;
 
 if (mmu_idx < MMU_REAL_IDX) {
@@ -127,7 +126,7 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr 
orig_vaddr,
 qemu_log_mask(CPU_LOG_MMU, "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
 __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);
 
-tlb_set_page(cs, orig_vaddr, raddr, prot,
+tlb_set_page(cs, orig_vaddr & TARGET_PAGE_MASK, raddr, prot,
  mmu_idx, TARGET_PAGE_SIZE);
 
 return 0;
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bbbe1c62b3..69a16867d4 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1687,18 +1687,10 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, 
uint64_t a2, uint32_t r3)
 uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
 {
 uintptr_t ra = GETPC();
-CPUState *cs = CPU(s390_env_get_cpu(env));
 int i;
 
 real_addr = wrap_address(env, real_addr) & TARGET_PAGE_MASK;
 
-/* Check low-address protection */
-if ((env->cregs[0] & CR0_LOWPROT) && real_addr < 0x2000) {
-cpu_restore_state(cs, ra);
-program_interrupt(env, PGM_PROTECTION, 4);
-return 1;
-}
-
 for (i = 0; i < TARGET_PAGE_SIZE; i += 8) {
 cpu_stq_real_ra(env, real_addr + i, 0, ra);
 }
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 9daa0fd8e2..44a15449d2 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -106,6 +106,37 @@ static void trigger_page_fault(CPUS390XState *env, 
target_ulong vaddr,
 trigger_access_exception(env, type, ilen, tec);
 }
 
+/* check whether the address would be proteted by Low-Address Protection */
+static bool is_low_address(uint64_t addr)
+{
+return addr < 512 || (addr >= 4096 && addr < 4607);
+}
+
+/* check whether Low-Address Protection is enabled for mmu_translate() */
+static bool lowprot_enabled(const CPUS390XState *env, uint64_t asc)
+{
+if (!(env->cregs[0] & CR0_LOWPROT)) {
+return false;
+}
+if (!(env->psw.mask & PSW_MASK_DAT)) {
+return true;
+}
+
+/* Check the private-space control bit */
+switch (asc) {
+case PSW_ASC_PRIMARY:
+return !(env->cregs[1] & _ASCE_PRIVATE_SPACE);
+case PSW_ASC_SECONDARY:
+return !(env->cregs[7] & _ASCE_PRIVATE_SPACE);
+case PSW_ASC_HOME:
+return !(env->cregs[13] & _ASCE_PRIVATE_SPACE);
+default:
+/* We don't support access register mode */
+error_report("unsupported addressing mode");
+exit(1);
+}
+}
+
 /**
  * Translate real address to absolute (= physical)
  * address by taking care of the prefix mapping.
@@ -323,6 +354,24 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, 
int rw, uint64_t asc,
 }
 
 *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+if (is_low_address(vaddr & TARGET_PAGE_MASK) && lowprot_enabled(env, asc)) 
{
+/*
+ * If any part of this page is currently protected, make sure the
+ * TLB entry will not be reused.
+ *
+ * As the protected range is always the first 512 bytes of the
+ * two first pages, we are able to catch all writes to these areas
+ * just by looking at the start address (triggering the tlb miss).
+ */
+*flags |= PAGE_WRITE_INV;
+if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
+if (exc) {
+trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
+}
+return -EACCES;
+}
+}
+
 vaddr &= TARGET_PAGE_MASK;
 
 if (!(env->psw.mask & PSW_MASK_DAT)) {
@@ -392,50 +441,17 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, 
int rw, uint64_t asc,
 }
 

[Qemu-devel] [PATCH RFC 3/3] s390x/tcg: make STFL store into the lowcore

[David Hildenbrand]

Using virtual memory access is wrong and will soon include low-address
protection checks, which is to be bypassed for STFL.

This was originally part of a bigger STFL(E) refactoring.

Signed-off-by: David Hildenbrand 
---
 target/s390x/helper.h  | 2 +-
 target/s390x/misc_helper.c | 7 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 75ba04fc15..52c2963baa 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -104,7 +104,6 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, 
i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
-DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(stfle, i32, env, i64)
 DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
@@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, 
i64)
 DEF_HELPER_1(per_check_exception, void, env)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 
 DEF_HELPER_2(xsch, void, env, i64)
 DEF_HELPER_2(csch, void, env, i64)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 293fc8428a..0b93381188 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -541,13 +541,18 @@ static unsigned do_stfle(CPUS390XState *env, uint64_t 
words[MAX_STFL_WORDS])
 return max_bit / 64;
 }
 
+#ifndef CONFIG_USER_ONLY
 void HELPER(stfl)(CPUS390XState *env)
 {
 uint64_t words[MAX_STFL_WORDS];
+LowCore *lowcore;
 
+lowcore = cpu_map_lowcore(env);
 do_stfle(env, words);
-cpu_stl_data(env, 200, words[0] >> 32);
+lowcore->stfl_fac_list = cpu_to_be32(words[0] >> 32);
+cpu_unmap_lowcore(lowcore);
 }
+#endif
 
 uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
 {
-- 
2.13.5

[Qemu-devel] [PATCH RFC 1/3] accel/tcg: allow to invalidate a write TLB entry immediately

[David Hildenbrand]

Background: s390x implements Low-Address Protection (LAP). If LAP is
enabled, writing to effective addresses (before any transaltion)
0-511 and 4096-4607 triggers a protection exception.

So we have subpage protection on the first two pages of every address
space (where the lowcore - the CPU private data resides).

By immediately invalidating the write entry but allowing the caller to
continue, we force every write access onto these first two pages into
the slow path. we will get a tlb fault with the specific accessed
addresses and can then evaluate if protection applies or not.

We have to make sure to ignore the invalid bit if tlb_fill() succeeds.

Signed-off-by: David Hildenbrand 
---
 accel/tcg/cputlb.c   | 5 -
 accel/tcg/softmmu_template.h | 4 ++--
 include/exec/cpu-all.h   | 3 +++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index bcbcc4db6c..5bc4233961 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -683,6 +683,9 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
 } else {
 tn.addr_write = address;
 }
+if (prot & PAGE_WRITE_INV) {
+tn.addr_write |= TLB_INVALID_MASK;
+}
 }
 
 /* Pairs with flag setting in tlb_reset_dirty_range */
@@ -967,7 +970,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
target_ulong addr,
 if (!VICTIM_TLB_HIT(addr_write, addr)) {
 tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr);
 }
-tlb_addr = tlbe->addr_write;
+tlb_addr = tlbe->addr_write & ~TLB_INVALID_MASK;
 }
 
 /* Check notdirty */
diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
index d7563292a5..3fc5144316 100644
--- a/accel/tcg/softmmu_template.h
+++ b/accel/tcg/softmmu_template.h
@@ -285,7 +285,7 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 if (!VICTIM_TLB_HIT(addr_write, addr)) {
 tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr);
 }
-tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
+tlb_addr = env->tlb_table[mmu_idx][index].addr_write & 
~TLB_INVALID_MASK;
 }
 
 /* Handle an IO access.  */
@@ -361,7 +361,7 @@ void helper_be_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 if (!VICTIM_TLB_HIT(addr_write, addr)) {
 tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr);
 }
-tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
+tlb_addr = env->tlb_table[mmu_idx][index].addr_write & 
~TLB_INVALID_MASK;
 }
 
 /* Handle an IO access.  */
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index ffe43d5654..24b9509604 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -247,6 +247,9 @@ extern intptr_t qemu_host_page_mask;
 /* original state of the write flag (used when tracking self-modifying
code */
 #define PAGE_WRITE_ORG 0x0010
+/* Invalidate the TLB entry immediately, helpful for s390x
+ * Low-Address-Protection. Used with PAGE_WRITE in tlb_set_page_with_attrs() */
+#define PAGE_WRITE_INV 0x0040
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
 /* FIXME: Code that sets/uses this is broken and needs to go away.  */
 #define PAGE_RESERVED  0x0020
-- 
2.13.5

[Qemu-devel] [PATCH RFC 0/3] s390x/tcg: LAP support using immediate TLB invalidation

[David Hildenbrand]

Details about Low-Address Protection can be found in description of
patch 1 and 2. It is basically a subpage protection of the first two
pages of every address space (for which it is enabled).

We can achieve this by simply directly invalidating the TLB entry and
therefore forcing every write accesses onto these two pages into the slow
path.

With this patch, I can boot Linux just fine (which uses LAP). This also
makes all related kvm-unit-tests that we have pass.

The checks are working that good, that I discovered a STFL bug. STFL
stores into the low addresses but low-address protection does explicitly
not apply. The Linux kernel calls STFL while LAP is active. So without
patch nr 3, booting Linux will fail. (this change is also part of a patch
of my SMP series).

Based on: https://github.com/cohuck/qemu.git s390-next
Available on: https://github.com/dhildenb/qemu.git s390x_lap


David Hildenbrand (3):
  accel/tcg: allow to invalidate a write TLB entry immediately
  s390x/tcg: low-address protection support
  s390x/tcg: make STFL store into the lowcore

 accel/tcg/cputlb.c   |  5 ++-
 accel/tcg/softmmu_template.h |  4 +-
 include/exec/cpu-all.h   |  3 ++
 target/s390x/excp_helper.c   |  3 +-
 target/s390x/helper.h|  2 +-
 target/s390x/mem_helper.c|  8 
 target/s390x/misc_helper.c   |  7 +++-
 target/s390x/mmu_helper.c| 96 
 8 files changed, 78 insertions(+), 50 deletions(-)

-- 
2.13.5

Re: [Qemu-devel] Problem in pcie_pci_bridge_realize()

[Thomas Huth]

On 27.09.2017 18:22, Aleksandr Bezzubikov wrote:
> 2017-09-27 18:50 GMT+03:00 Thomas Huth :
>>  Hi,
>>
>> QEMU currently aborts with an assertion when plugging of the
>> pcie-pci-bridge fails, e.g.:
>>
>> $ mips64el-softmmu/qemu-system-mips64el -M malta -nographic -S \
>> -device pcie-pci-bridge -bios pc-bios/bios.bin
>> qemu-system-mips64el: memory.c:1699: memory_region_finalize:
>>  Assertion `!mr->container' failed.
>> Aborted (core dumped)
[...]
> Hi Thomas,
> This bug was already reported by Eduardo with ppc64, try this patch
> that is intended to fix it
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg06696.html

Thanks, that fixed the issue indeed!

 Thomas

Re: [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test

[Dr. David Alan Gilbert]

* Thomas Huth (th...@redhat.com) wrote:
> On 27.09.2017 01:07, Eduardo Habkost wrote:
> > When running device_add tests, test all devices in a single run
> > instead of restarting QEMU every time.
> > 
> > There's a plug_all testcase argument that can be used to make the
> > test code plug all devices at once.  I'm adding this mode because
> > there's a crash that was detected while testing the script
> > without any device_del command, but the crash goes away if we
> > device_del every device immediately:
> > 
> >   $ ../scripts/device-crash-test ./x86_64-softmmu/qemu-system-x86_64 
> > --quick -t method=device_add  --strict -t plug_all=1
> >   INFO: running test case: binary=./x86_64-softmmu/qemu-system-x86_64 
> > plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
> >   WARNING: qemu received signal -6: ./x86_64-softmmu/qemu-system-x86_64 
> > -chardev socket,id=mon,path=/var/tmp/qemu-23578-monitor.sock -mon 
> > chardev=mon,mode=control -display none -vga none -S -machine 
> > pc-0.12,accel=kvm
> >   ERROR: result: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 
> > accel=kvm machine=pc-0.12 device=* method=device_add
> >   ERROR: cmdline: ./x86_64-softmmu/qemu-system-x86_64 -S -machine 
> > pc-0.12,accel=kvm
> >   ERROR: log: audio: Could not init `oss' audio driver
> >   ERROR: log: qemu-system-x86_64: .../qemu/migration/savevm.c:721: 
> > vmstate_register_with_alias_id: Assertion `!se->compat || se->instance_id 
> > == 0' failed.
> >   ERROR: last device_add device: usb-net
> 
> FWIW, I've seen similar crashes while working on my HMP device_add
> tester, when I skipped the device_del and plugged-in the devices a
> couple of times instead. I think this happens because some devices are
> doing a vmstate_register() in their instance_init() or realize()
> function, instead of using dc->vmsd as they should. However, with the
> git master branch as of today (31bc1d8481af414cfa2857f905e), I am
> currently not able anymore to reproduce the problem with the HMP
> device_add tester, so one of the recent "set user_creatable = false"
> patches might have fixed the issue for me already...

I think I've seen this problem when two things end up trying to register
with the same migration name;  I'll be honest I can't remember the
details, but I think the choice is either between having a full bus
name, e.g.

pci-xx:yy.z:aa:bb:.c:.. then similar for the USB chain

or
mydevice  + number

where number is the instance=id.  So to hit this I think
it's typically a case of messing up that long path and it no
longer being unique.
Printing the path in vmstate_register_with_alias_id after the
pstrcat might help.
I did add some sanity checking in there for a case I'd
previously hit where the name just got truncated, but I don't
think you should be able to hit that any more.

Dave

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

Re: [Qemu-devel] [PATCH v2] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case

[Thomas Huth]

On 25.09.2017 01:21, Aleksandr Bezzubikov wrote:
> QEMU with the pcie-pci-bridge device crashes if the guest board doesn't 
> support MSI,
> e.g. 'qemu-system-ppc64 -M prep -device pcie-pci-bridge'.
> This is caused by wrong pcie-pci-bridge instantiation error handling. This 
> patch fixes this issue
> by falling back to legacy INTx if MSI is not available.
> Also set the bridge's 'msi' property default value to 'auto' in order to 
> trigger errors 
> only when user explicitly set msi=on.
> 
> v2:
> rewrite the commit message
> 
> Reported-by: Eduardo Habkost 
> Signed-off-by: Aleksandr Bezzubikov 
> Reviewed-by: Marcel Apfelbaum 
> ---
>  hw/pci-bridge/pcie_pci_bridge.c | 24 ++--
>  1 file changed, 18 insertions(+), 6 deletions(-)

This also fixes the issue that I've seen with qemu-system-mips64el today:

$ mips64el-softmmu/qemu-system-mips64el -M malta -nographic -S -device
pcie-pci-bridge -bios pc-bios/bios.bin
qemu-system-mips64el: memory.c:1699: memory_region_finalize:
 Assertion `!mr->container' failed.
Aborted (core dumped)

So feel free to add:

Tested-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 2/2] qcow2: truncate the tail of the image file after shrinking the image

[Max Reitz]

On 2017-09-27 18:27, Pavel Butsykin wrote:
> On 27.09.2017 19:00, Max Reitz wrote:
>> On 2017-09-22 11:39, Pavel Butsykin wrote:
>>> Now after shrinking the image, at the end of the image file, there
>>> might be a
>>> tail that probably will never be used. So we can find the last used
>>> cluster and
>>> cut the tail.
>>>
>>> Signed-off-by: Pavel Butsykin 
>>> ---
>>>   block/qcow2-refcount.c | 22 ++
>>>   block/qcow2.c  | 23 +++
>>>   block/qcow2.h  |  1 +
>>>   3 files changed, 46 insertions(+)
>>>
>>> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
>>> index 88d5a3f1ad..aa3fd6cf17 100644
>>> --- a/block/qcow2-refcount.c
>>> +++ b/block/qcow2-refcount.c
>>> @@ -3181,3 +3181,25 @@ out:
>>>   g_free(reftable_tmp);
>>>   return ret;
>>>   }
>>> +
>>> +int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)
>>> +{
>>> +    BDRVQcow2State *s = bs->opaque;
>>> +    int64_t i;
>>> +
>>> +    for (i = size_to_clusters(s, size) - 1; i >= 0; i--) {
>>> +    uint64_t refcount;
>>> +    int ret = qcow2_get_refcount(bs, i, );
>>> +    if (ret < 0) {
>>> +    fprintf(stderr, "Can't get refcount for cluster %"
>>> PRId64 ": %s\n",
>>> +    i, strerror(-ret));
>>> +    return ret;
>>> +    }
>>> +    if (refcount > 0) {
>>> +    return i;
>>> +    }
>>> +    }
>>> +    qcow2_signal_corruption(bs, true, -1, -1,
>>> +    "There are no references in the refcount
>>> table.");
>>> +    return -EIO;
>>> +}
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 8a4311d338..8dfb5393a7 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>> @@ -3106,6 +3106,7 @@ static int qcow2_truncate(BlockDriverState *bs,
>>> int64_t offset,
>>>   new_l1_size = size_to_l1(s, offset);
>>>     if (offset < old_length) {
>>> +    int64_t last_cluster, old_file_size;
>>>   if (prealloc != PREALLOC_MODE_OFF) {
>>>   error_setg(errp,
>>>  "Preallocation can't be used for shrinking
>>> an image");
>>> @@ -3134,6 +3135,28 @@ static int qcow2_truncate(BlockDriverState
>>> *bs, int64_t offset,
>>>    "Failed to discard unused refblocks");
>>>   return ret;
>>>   }
>>> +
>>> +    old_file_size = bdrv_getlength(bs->file->bs);
>>> +    if (old_file_size < 0) {
>>> +    error_setg_errno(errp, -old_file_size,
>>> + "Failed to inquire current file length");
>>> +    return old_file_size;
>>> +    }
>>> +    last_cluster = qcow2_get_last_cluster(bs, old_file_size);
>>> +    if (last_cluster < 0) {
>>> +    error_setg_errno(errp, -last_cluster,
>>> + "Failed to find the last cluster");
>>> +    return last_cluster;
>>> +    }
>>
>> My idea was rather that you just wouldn't truncate the image file if
>> something fails here.  So in any of these new cases where you currently
>> just report the whole truncate operation as having failed, you could
>> just emit a warning and not do the truncation of bs->file.
> 
> I'm not sure that's right. If the qcow2_get_last_cluster() returned an
> error, probably with the image was a problem.. can we continue to work
> with the image without risking to damage it even more? if something bad
> happened with the reftable we usually mark the image as corrupted, it's
> the same thing.

Well, the only thing that's left to do is to write the new size into the
image header, so I think that should work just fine...

I won't disagree that bdrv_getlength() or qcow2_get_last_cluster()
failing may be reasons to stop truncation (although I don't think they
necessarily are at this point).

But I could well imagine that the below bdrv_truncate() of bs->file
fails for benign reasons, e.g. because the underlying protocol does not
support shrinking of images or something.  Then we probably should carry on.

Max

> Although if the shrink is almost complete, the truncation of bs->file
> isn't so important thing and we could update qcow2 header.
> 
>> I can live with the current version, though, so:
>>
>> Reviewed-by: Max Reitz 
>>
>> But I'll wait for a response from you before merging this series.
>>
>> Max
>>
>>> +    if ((last_cluster + 1) * s->cluster_size < old_file_size) {
>>> +    ret = bdrv_truncate(bs->file, (last_cluster + 1) *
>>> s->cluster_size,
>>> +    PREALLOC_MODE_OFF, NULL);
>>> +    if (ret < 0) {
>>> +    error_setg_errno(errp, -ret,
>>> + "Failed to truncate the tail of the
>>> image");
>>> +    return ret;
>>> +    }
>>> +    }
>>>   } else {
>>>   ret = qcow2_grow_l1_table(bs, new_l1_size, true);
>>>   if (ret < 0) {
>>> diff --git a/block/qcow2.h 

Re: [Qemu-devel] [PATCH] xen-hvm: use errno in error message

[Anthony PERARD]

On Wed, Sep 27, 2017 at 05:10:09PM +0100, Wei Liu wrote:
> The error code is encoding in errno, not rc.

^ encoded
I think.

Otherwise,
Reviewed-by: Anthony PERARD 

Thanks,

> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Stefano Stabellini 
> Cc: Anthony PERARD 
> ---
>  hw/i386/xen/xen-hvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index d9ccd5d0d6..f79816a649 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -1446,7 +1446,7 @@ void xen_hvm_modified_memory(ram_addr_t start, 
> ram_addr_t length)
>  if (rc) {
>  fprintf(stderr,
>  "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, 
> %s\n",
> -__func__, start, nb_pages, rc, strerror(-rc));
> +__func__, start, nb_pages, rc, strerror(errno));
>  }
>  }
>  }
> -- 
> 2.11.0
> 

-- 
Anthony PERARD

Re: [Qemu-devel] blockdev-commit design

[Max Reitz]

On 2017-09-26 19:59, Kevin Wolf wrote:

[...]

>   * The old block-commit command decides between an "actual" commit job
> and the mirror-based active commit based on whether top is the
> active layer.
> 
> We don't get an option for the active layer any more now, so this
> isn't how things can work for blockdev-commit. We could probably
> check whether top has a BlockBackend parent, but that's not really
> what we're interested in anyway. Maybe the best we could do to
> decide this automatically is to check whether there is any parent of
> top that requires write permissions. If there is, we need active
> commit, otherwise the "normal" one is good enough.
> 
> However, who says that the intentions of the user stay as we deduce
> them at the start of the block job? Who says that the user doesn't
> want to add a writable disk as a user of the node while the block
> job is running?
> 
> The optimal solution to this would be that the commit filter node
> responds to permission requests and switches between active and
> "normal" commit mode. I'm not sure how hard this would be to
> implement.
> 
> As long as we don't have the automatic switch, do we have to allow
> the user to specify explicitly which mode they want instead of
> automatically choosing one?

Probably a stupid question: What advantage does the commit job have over
the mirror job? I.e. would it be possible to just always do a mirror?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/2] qcow2: truncate the tail of the image file after shrinking the image

[Pavel Butsykin]

On 27.09.2017 19:00, Max Reitz wrote:

On 2017-09-22 11:39, Pavel Butsykin wrote:

Now after shrinking the image, at the end of the image file, there might be a
tail that probably will never be used. So we can find the last used cluster and
cut the tail.

Signed-off-by: Pavel Butsykin 
---
  block/qcow2-refcount.c | 22 ++
  block/qcow2.c  | 23 +++
  block/qcow2.h  |  1 +
  3 files changed, 46 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 88d5a3f1ad..aa3fd6cf17 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3181,3 +3181,25 @@ out:
  g_free(reftable_tmp);
  return ret;
  }
+
+int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)
+{
+BDRVQcow2State *s = bs->opaque;
+int64_t i;
+
+for (i = size_to_clusters(s, size) - 1; i >= 0; i--) {
+uint64_t refcount;
+int ret = qcow2_get_refcount(bs, i, );
+if (ret < 0) {
+fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n",
+i, strerror(-ret));
+return ret;
+}
+if (refcount > 0) {
+return i;
+}
+}
+qcow2_signal_corruption(bs, true, -1, -1,
+"There are no references in the refcount table.");
+return -EIO;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 8a4311d338..8dfb5393a7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3106,6 +3106,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t 
offset,
  new_l1_size = size_to_l1(s, offset);
  
  if (offset < old_length) {

+int64_t last_cluster, old_file_size;
  if (prealloc != PREALLOC_MODE_OFF) {
  error_setg(errp,
 "Preallocation can't be used for shrinking an image");
@@ -3134,6 +3135,28 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t 
offset,
   "Failed to discard unused refblocks");
  return ret;
  }
+
+old_file_size = bdrv_getlength(bs->file->bs);
+if (old_file_size < 0) {
+error_setg_errno(errp, -old_file_size,
+ "Failed to inquire current file length");
+return old_file_size;
+}
+last_cluster = qcow2_get_last_cluster(bs, old_file_size);
+if (last_cluster < 0) {
+error_setg_errno(errp, -last_cluster,
+ "Failed to find the last cluster");
+return last_cluster;
+}


My idea was rather that you just wouldn't truncate the image file if
something fails here.  So in any of these new cases where you currently
just report the whole truncate operation as having failed, you could
just emit a warning and not do the truncation of bs->file.


I'm not sure that's right. If the qcow2_get_last_cluster() returned an
error, probably with the image was a problem.. can we continue to work
with the image without risking to damage it even more? if something bad
happened with the reftable we usually mark the image as corrupted, it's
the same thing.

Although if the shrink is almost complete, the truncation of bs->file
isn't so important thing and we could update qcow2 header.


I can live with the current version, though, so:

Reviewed-by: Max Reitz 

But I'll wait for a response from you before merging this series.

Max


+if ((last_cluster + 1) * s->cluster_size < old_file_size) {
+ret = bdrv_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
+PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed to truncate the tail of the image");
+return ret;
+}
+}
  } else {
  ret = qcow2_grow_l1_table(bs, new_l1_size, true);
  if (ret < 0) {
diff --git a/block/qcow2.h b/block/qcow2.h
index 5a289a81e2..782a206ecb 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -597,6 +597,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int 
refcount_order,
  BlockDriverAmendStatusCB *status_cb,
  void *cb_opaque, Error **errp);
  int qcow2_shrink_reftable(BlockDriverState *bs);
+int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);
  
  /* qcow2-cluster.c functions */

  int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,

Re: [Qemu-devel] Problem in pcie_pci_bridge_realize()

[Aleksandr Bezzubikov]

2017-09-27 18:50 GMT+03:00 Thomas Huth :
>  Hi,
>
> QEMU currently aborts with an assertion when plugging of the
> pcie-pci-bridge fails, e.g.:
>
> $ mips64el-softmmu/qemu-system-mips64el -M malta -nographic -S \
> -device pcie-pci-bridge -bios pc-bios/bios.bin
> qemu-system-mips64el: memory.c:1699: memory_region_finalize:
>  Assertion `!mr->container' failed.
> Aborted (core dumped)
>
> The backtrace points to the pcie_pci_bridge_realize() function:
>
> #0  0x7150f1f7 in raise () at /lib64/libc.so.6
> #1  0x715108e8 in abort () at /lib64/libc.so.6
> #2  0x71508266 in __assert_fail_base () at /lib64/libc.so.6
> #3  0x71508312 in  () at /lib64/libc.so.6
> #4  0x557cedaf in memory_region_finalize (obj=)
> at /home/thuth/devel/qemu/memory.c:1699
> #5  0x55a49852 in object_unref (type=, 
> obj=0x56e68580)
> at /home/thuth/devel/qemu/qom/object.c:453
> #6  0x55a49852 in object_unref (data=0x56e68580) at 
> /home/thuth/devel/qemu/qom/object.c:467
> #7  0x55a49852 in object_unref (obj=0x56e68580) at 
> /home/thuth/devel/qemu/qom/object.c:902
> #8  0x55a48887 in object_property_del_child (obj=0x56dc7760, 
> child=child@entry=0x56e68580, errp=0x0) at 
> /home/thuth/devel/qemu/qom/object.c:427
> #9  0x55a490a4 in object_unparent (obj=obj@entry=0x56e68580)
> at /home/thuth/devel/qemu/qom/object.c:446
> #10 0x559906ae in shpc_free (d=d@entry=0x56dc7760)
> at /home/thuth/devel/qemu/hw/pci/shpc.c:676
> #11 0x55987470 in pcie_pci_bridge_realize (d=0x56dc7760, 
> errp=0x7fffd710)
> at /home/thuth/devel/qemu/hw/pci-bridge/pcie_pci_bridge.c:84
> #12 0x5598ca67 in pci_qdev_realize (qdev=0x56dc7760, 
> errp=0x7fffd7b0)
> at /home/thuth/devel/qemu/hw/pci/pci.c:2024
> #13 0x5590ee4a in device_set_realized (obj=, 
> value=, errp=0x7fffd8e8) at 
> /home/thuth/devel/qemu/hw/core/qdev.c:914
>
> Any clue what might be wrong here?
>
>  Thomas

Hi Thomas,
This bug was already reported by Eduardo with ppc64, try this patch
that is intended to fix it

http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg06696.html

-- 
Aleksandr Bezzubikov

Re: [Qemu-devel] [RFC] ppc: define spapr core types statically

[Greg Kurz]

On Wed, 27 Sep 2017 13:49:17 +0200
Igor Mammedov  wrote:

> --
> patch does 3 things at the same time and should be split but
> it has 'host' consolidation is it as well to demonstrate idea
> --
> 
> spapr core type definition doesn't have any fields that
> require it to be defined at runtime. So replace code
> that fills in TypeInfo at runtime with static TypeInfo
> array that does the same at complie time.
> 
> And replace sPAPRCPUCoreClass::cpu_class with cpu
> type name since were used just to get that at points
> it were accessed.
> 
> While at that, consolidate move 'host' core type
> registration into spapr_cpu_core.c, similar like
> it's done in x86 target.
> 

This looks nice indeed (just one remark, see below).

Will you re-post as a patch series or do you prefer I do it ?

> Signed-off-by: Igor Mammedov 
> ---
>  include/hw/ppc/spapr_cpu_core.h |  5 ++-
>  hw/ppc/spapr.c  |  6 +--
>  hw/ppc/spapr_cpu_core.c | 93 
> -
>  target/ppc/kvm.c| 11 -
>  4 files changed, 41 insertions(+), 74 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 93051e9..42765de 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -21,6 +21,8 @@
>  #define SPAPR_CPU_CORE_GET_CLASS(obj) \
>   OBJECT_GET_CLASS(sPAPRCPUCoreClass, (obj), TYPE_SPAPR_CPU_CORE)
>  
> +#define SPAPR_CPU_CORE_TYPE_NAME(model) model "-" TYPE_SPAPR_CPU_CORE
> +
>  typedef struct sPAPRCPUCore {
>  /*< private >*/
>  CPUCore parent_obj;
> @@ -32,9 +34,8 @@ typedef struct sPAPRCPUCore {
>  
>  typedef struct sPAPRCPUCoreClass {
>  DeviceClass parent_class;
> -ObjectClass *cpu_class;
> +const char *cpu_type;
>  } sPAPRCPUCoreClass;
>  
>  char *spapr_get_cpu_core_type(const char *model);
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
>  #endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8c1a437..00cfe9a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3125,8 +3125,7 @@ void spapr_core_release(DeviceState *dev)
>  if (smc->pre_2_10_has_unused_icps) {
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  int i;
>  
>  for (i = 0; i < cc->nr_threads; i++) {
> @@ -3222,8 +3221,7 @@ static void spapr_core_plug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  
>  if (smc->pre_2_10_has_unused_icps) {
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  int i;
>  
>  for (i = 0; i < cc->nr_threads; i++) {
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 5bea4c9..8a18eaf 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -104,8 +104,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, 
> Error **errp)
>  {
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  CPUCore *cc = CPU_CORE(dev);
>  int i;
>  
> @@ -166,8 +165,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
>  CPUCore *cc = CPU_CORE(OBJECT(dev));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  Error *local_err = NULL;
>  void *obj;
>  int i, j;
> @@ -186,7 +184,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  
>  obj = sc->threads + i * size;
>  
> -object_initialize(obj, size, typename);
> +object_initialize(obj, size, scc->cpu_type);
>  cs = CPU(obj);
>  cpu = POWERPC_CPU(cs);
>  cs->cpu_index = cc->core_id + i;
> @@ -231,42 +229,12 @@ err:
>  error_propagate(errp, local_err);
>  }
>  
> -static const char *spapr_core_models[] = {
> -/* 970 */
> -"970_v2.2",
> -
> -/* 970MP variants */
> -"970mp_v1.0",
> -"970mp_v1.1",
> -
> -/* POWER5+ */
> -"power5+_v2.1",
> -
> -/* POWER7 */
> -"power7_v2.3",
> -
> -

Re: [Qemu-devel] [PATCH v3 1/6] tcg: Add types and operations for host vectors

[Richard Henderson]

On 09/26/2017 12:28 PM, Alex Bennée wrote:
>>  * TCGv_ptr : a host pointer type
>> +* TCGv_vec : a host vector type; the exact size is not exposed
>> + to the CPU front-end code.
> 
> Isn't this a guest vector type (which is pointed to by a host pointer)?

No, it's a host vector, which we have created in response to expanding a guest
vector operation.

> A one line comment wouldn't go amiss here. This looks like we are
> allocating a new temp of the same type as an existing temp?
> 
>> +TCGv_vec tcg_temp_new_vec_matching(TCGv_vec match)

Yes.

>> +All of the vector ops have a final constant argument that specifies the
>> +length of the vector operation LEN as 64 << LEN bits.
> 
> That doesn't scan well. So would a 4 lane operation be encoded as 64 <<
> 4? Is this because we are using the bottom bits for something?

64 << 0 = 64
64 << 1 = 128
64 << 2 = 256.

I've fixed up the wording a bit.

>> +  Copy C across the entire vector.
>> +  At present the only supported values for C are 0 and -1.
> 
> I guess this is why the size in unimportant? This is for clearing or
> setting the whole of the vector? What does len mean in this case?

Yes.  Len still means the length of the whole vector.

Elsewhere there's a comment about maybe using dupi{8,16,32,64}_vec instead.
However I wanted to put that off until we do some more conversions and see
exactly what's going to be needed.


>> +* and_vec v0, v1, v2, len
>> +* or_vec  v0, v1, v2, len
>> +* xor_vec v0, v1, v2, len
>> +* andc_vecv0, v1, v2, len
>> +* orc_vec v0, v1, v2, len
>> +* not_vec v0, v1, len
>> +
>> +  Similarly, logical operations.
> 
> Similarly, logical operations with and without compliment?

Sure.


r~