[Qemu-devel] [PULL 13/20] configure: Support pkg-config for zlib

[Laurent Vivier]

From: Stefan Weil 

This is needed for builds with the mingw64-* packages from Cygwin,
but also works for Linux.

Move the zlib test also more to the end because users should
get information on the really important missing packages
(which also require zlib) first.

Signed-off-by: Stefan Weil 
Reviewed-by: Stefan Hajnoczi 
Message-Id: <20180712192603.11599-1...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 configure | 40 +++-
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/configure b/configure
index 1ee09bd112..a02df30dde 100755
--- a/configure
+++ b/configure
@@ -2151,23 +2151,6 @@ EOF
   fi
 fi
 
-#
-# zlib check
-
-if test "$zlib" != "no" ; then
-cat > $TMPC << EOF
-#include 
-int main(void) { zlibVersion(); return 0; }
-EOF
-if compile_prog "" "-lz" ; then
-:
-else
-error_exit "zlib check failed" \
-"Make sure to have the zlib libs and headers installed."
-fi
-fi
-LIBS="$LIBS -lz"
-
 ##
 # lzo check
 
@@ -3479,6 +3462,29 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; 
then
 fi
 fi
 
+#
+# zlib check
+
+if test "$zlib" != "no" ; then
+if $pkg_config --exists zlib; then
+zlib_cflags=$($pkg_config --cflags zlib)
+zlib_libs=$($pkg_config --libs zlib)
+QEMU_CFLAGS="$zlib_cflags $QEMU_CFLAGS"
+LIBS="$zlib_libs $LIBS"
+else
+cat > $TMPC << EOF
+#include 
+int main(void) { zlibVersion(); return 0; }
+EOF
+if compile_prog "" "-lz" ; then
+LIBS="$LIBS -lz"
+else
+error_exit "zlib check failed" \
+"Make sure to have the zlib libs and headers installed."
+fi
+fi
+fi
+
 ##
 # SHA command probe for modules
 if test "$modules" = yes; then
-- 
2.17.2

Re: [Qemu-devel] [RFC v4 59/71] cpu: introduce cpu_has_work_with_iothread_lock

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> It will gain some users soon.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  include/qom/cpu.h | 36 +---
>  1 file changed, 33 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 

r~

[Qemu-devel] [PULL 00/20] Trivial patches patches

[Laurent Vivier]

The following changes since commit 808ebd66e467f77c0d1f8c6346235f81e9c99cf2:

  Merge remote-tracking branch 'remotes/riscv/tags/riscv-for-master-3.1-sf0' 
into staging (2018-10-25 17:41:03 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/trivial-patches-pull-request

for you to fetch changes up to 4b03da6e87c34793137a231b558231fd406c05e8:

  ppc: move at24c to its own CONFIG_ symbol (2018-10-26 17:17:32 +0200)


QEMU trivial patches collected between June and October 2018
(Thank you to Thomas Huth)



Cleber Rosa (6):
  tests/tcg/README: fix location for lm32 tests
  qemu-iotests: fix filename containing checks
  docs/devel/testing.rst: add missing newlines after code block
  scripts/decodetree.py: remove unused imports
  scripts/qemu.py: remove trailing quotes on docstring
  qemu-iotests: make 218 executable

Emilio G. Cota (1):
  linux-user: fix comment s/atomic_write/atomic_set/

Li Qiang (2):
  memory.h: fix typos in comments
  cpu.h: fix a typo in comment

Paolo Bonzini (1):
  ppc: move at24c to its own CONFIG_ symbol

Philippe Mathieu-Daudé (6):
  qobject: Catch another straggler for use of qdict_put_str()
  xen: Use the PCI_DEVICE macro
  tests/bios-tables-test: Remove an useless cast
  hw/pci-host: Remove useless parenthesis around DIV_ROUND_UP macro
  hw/intc/gicv3: Remove useless parenthesis around DIV_ROUND_UP macro
  milkymist-minimac2: Use qemu_log_mask(GUEST_ERROR) instead of
error_report

Stefan Weil (2):
  tests: Fix typos in comments and help message (found by codespell)
  configure: Support pkg-config for zlib

Thomas Huth (1):
  configs/alpha: Remove unused CONFIG_PARALLEL_ISA switch

yuchenlin (1):
  vga_int: remove unused function protype

 configure | 40 +++
 default-configs/alpha-softmmu.mak |  1 -
 default-configs/ppc-softmmu.mak   |  1 +
 docs/devel/testing.rst|  2 ++
 hw/display/vga_int.h  |  1 -
 hw/net/milkymist-minimac2.c   | 14 ++
 hw/nvram/Makefile.objs|  2 +-
 hw/pci-host/piix.c|  2 +-
 hw/pci-host/q35.c |  2 +-
 hw/xen/xen_pt_config_init.c   |  8 +++---
 include/exec/memory.h |  6 ++--
 include/hw/intc/arm_gicv3_common.h|  2 +-
 include/qom/cpu.h |  2 +-
 linux-user/qemu.h |  2 +-
 qobject/block-qdict.c |  2 +-
 scripts/decodetree.py |  2 --
 scripts/qemu.py   |  2 +-
 tests/bios-tables-test.c  |  4 +--
 tests/docker/Makefile.include |  2 +-
 tests/docker/docker.py|  4 +--
 tests/guest-debug/test-gdbstub.py |  2 +-
 tests/qemu-iotests/218|  0
 tests/qemu-iotests/common.qemu|  2 +-
 tests/qemu-iotests/common.rc  |  4 +--
 tests/tcg/Makefile.include|  2 +-
 tests/tcg/Makefile.probe  |  2 +-
 tests/tcg/README  |  2 +-
 tests/tcg/mips/mips64-dsp/subq_s_pw.c |  2 +-
 28 files changed, 63 insertions(+), 54 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

-- 
2.17.2

Re: [Qemu-devel] [RFC v4 58/71] cpu: call .cpu_has_work with the CPU lock held

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  include/qom/cpu.h | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [Qemu-devel] [PATCH 2/6] Extend image_info struct with MIPS specific fp_abi and interp_fp_abi fields

[Aleksandar Markovic]

> Subject: [PATCH 2/6] Extend image_info struct with MIPS specific fp_abi and 
> interp_fp_abi fields
> 
> From: Stefan Markovic 
> 
> Signed-off-by: Stefan Markovic 
> ---

A brief commit message is needed. Perhaps with the description of the 
role/purpose of two fields introduced in this patch. Other than that:

Reviewed-by: Aleksandar Markovic 

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[Mark Kanda]

On 10/26/2018 1:37 PM, P J P wrote:

+-- On Fri, 26 Oct 2018, Mark Kanda wrote --+
| Deja vu requested that we include the following text in the commit message:
|
|   Discovered by Deja vu Security. Reported by Oracle.
|
| Would that be acceptable?

Generally an email-id is used/preferred in the commit log message. We could
use above for acknowledgement and avoid Reported-by in the commit log message
if that suits Deja vu team.

Please let me know your/their preference.



Yes, please use that acknowledgement text in lieu of a 'Reported-by' line.

Thanks,

-Mark

Re: [Qemu-devel] [PULL v2 00/43] Machine queue, 2018-10-25

[Peter Maydell]

On 25 October 2018 at 14:32, Eduardo Habkost  wrote:
> Changes v1 -> v2:
> * Fix 'make check' warnings (Igor)
>
>
> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' 
> into staging (2018-10-23 17:20:23 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
>
> for you to fetch changes up to 8fa922c241e63f018f5b55c03ac494ae3d5fe594:
>
>   net: xgmac: convert SysBus init method to a realize method (2018-10-24 
> 06:44:59 -0300)
>
> 
> Machine queue, 2018-10-25
>
> * sysbus init/realize cleanups
>   (Cédric Le Goater, Philippe Mathieu-Daudé)
> * memory-device refactoring (David Hildenbrand)
> * -smp: deprecate incorrect CPUs topology (Igor Mammedov)
> * -numa parsing cleanups (Markus Armbruster)
> * Fix hostmem-file memory leak (Zhang Yi)
> * Typo fix (Li Qiang)
>
> 
>
> Queue for Machine Core patches
>

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 15/20] xen: Use the PCI_DEVICE macro

[Laurent Vivier]

From: Philippe Mathieu-Daudé 

TYPE_XEN_PT_DEVICE is a subclass of TYPE_PCI_DEVICE, the clean way
to access the PCIDevice pointer is using the PCI_DEVICE() macro.

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Markus Armbruster 
Acked-by: Anthony PERARD 
Acked-by: Michael S. Tsirkin 
Message-Id: <20180705155811.20366-4-f4...@amsat.org>
Signed-off-by: Laurent Vivier 
---
 hw/xen/xen_pt_config_init.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index aee31c62bb..47f9010c75 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -358,7 +358,7 @@ static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
 static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
  int index)
 {
-PCIDevice *d = >dev;
+PCIDevice *d = PCI_DEVICE(s);
 XenPTRegion *region = NULL;
 PCIIORegion *r;
 
@@ -469,7 +469,7 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, 
XenPTReg *cfg_entry,
 {
 XenPTRegInfo *reg = cfg_entry->reg;
 XenPTRegion *base = NULL;
-PCIDevice *d = >dev;
+PCIDevice *d = PCI_DEVICE(s);
 const PCIIORegion *r;
 uint32_t writable_mask = 0;
 uint32_t bar_emu_mask = 0;
@@ -543,7 +543,7 @@ static int 
xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
 {
 XenPTRegInfo *reg = cfg_entry->reg;
 XenPTRegion *base = NULL;
-PCIDevice *d = (PCIDevice *)>dev;
+PCIDevice *d = PCI_DEVICE(s);
 uint32_t writable_mask = 0;
 uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 pcibus_t r_size = 0;
@@ -1587,7 +1587,7 @@ static int xen_pt_pcie_size_init(XenPCIPassthroughState 
*s,
  const XenPTRegGroupInfo *grp_reg,
  uint32_t base_offset, uint8_t *size)
 {
-PCIDevice *d = >dev;
+PCIDevice *d = PCI_DEVICE(s);
 uint8_t version = get_capability_version(s, base_offset);
 uint8_t type = get_device_type(s, base_offset);
 uint8_t pcie_size = 0;
-- 
2.17.2

[Qemu-devel] [PULL 10/20] linux-user: fix comment s/atomic_write/atomic_set/

[Laurent Vivier]

From: "Emilio G. Cota" 

Signed-off-by: Emilio G. Cota 
Message-Id: <20180811211011.6277-1-c...@braap.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/qemu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 1beb6a2cfc..dde3f26f5a 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -143,7 +143,7 @@ typedef struct TaskState {
 /* Nonzero if process_pending_signals() needs to do something (either
  * handle a pending signal or unblock signals).
  * This flag is written from a signal handler so should be accessed via
- * the atomic_read() and atomic_write() functions. (It is not accessed
+ * the atomic_read() and atomic_set() functions. (It is not accessed
  * from multiple threads.)
  */
 int signal_pending;
-- 
2.17.2

[Qemu-devel] [PULL 07/20] scripts/decodetree.py: remove unused imports

[Laurent Vivier]

From: Cleber Rosa 

Signed-off-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20181004161852.11673-8-cr...@redhat.com>
Signed-off-by: Laurent Vivier 
---
 scripts/decodetree.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 277f9a9bba..457cffea90 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -149,12 +149,10 @@
 #   trans_addl_i(ctx, _opi, insn)
 #
 
-import io
 import os
 import re
 import sys
 import getopt
-import pdb
 
 insnwidth = 32
 insnmask = 0x
-- 
2.17.2

[Qemu-devel] [PULL 05/20] qemu-iotests: fix filename containing checks

[Laurent Vivier]

From: Cleber Rosa 

Commit cce293a2945 moved some functions from common.config to
common.rc, but the error messages still reference the old file
location.

Signed-off-by: Cleber Rosa 
Reviewed-by: Max Reitz 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20181004161852.11673-5-cr...@redhat.com>
Signed-off-by: Laurent Vivier 
---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 44bee16a5e..70ca65b49b 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -170,7 +170,7 @@ if [ ! -e "$TEST_DIR" ]; then
 fi
 
 if [ ! -d "$TEST_DIR" ]; then
-echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
+echo "common.rc: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
 exit 1
 fi
 
@@ -179,7 +179,7 @@ if [ -z "$REMOTE_TEST_DIR" ]; then
 fi
 
 if [ ! -d "$SAMPLE_IMG_DIR" ]; then
-echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a 
directory"
+echo "common.rc: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a 
directory"
 exit 1
 fi
 
-- 
2.17.2

Re: [Qemu-devel] [RFC v4 60/71] ppc: convert to cpu_has_work_with_iothread_lock

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> Soon we will call cpu_has_work without the BQL.
> 
> Cc: David Gibson 
> Cc: Alexander Graf 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Emilio G. Cota 
> ---
>  target/ppc/translate_init.inc.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode

[Aleksandar Markovic]

> Subject: [PATCH 5/6] Determine the desired FPU mode
> 
> From: Stefan Markovic 
> 
> Floating-point mode is calculated from MIPS.abiflags FP ABI value
> (based on kernel implementation). Illegal combinations are rejected.
> 
> Signed-off-by: Stefan Markovic 
> ---

Reviewed-by: Aleksandar Markovic 

Re: [Qemu-devel] [RFC v4 67/71] cpus-common: release BQL earlier in run_on_cpu

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> After completing the conversion to per-CPU locks, there is no need
> to release the BQL after having called cpu_kick.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  cpus-common.c | 20 +---
>  1 file changed, 5 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [Qemu-devel] [RFC v4 70/71] cpus-common: move exclusive_idle higher in the file

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> This will simplify the following commit's diff.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  cpus-common.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [Qemu-devel] [PATCH 3/6] Extract MIPS abiflags from ELF file

[Aleksandar Markovic]

> Subject: [PATCH 3/6] Extract MIPS abiflags from ELF file
> 
> From: Stefan Markovic 
> 
> Signed-off-by: Stefan Markovic 
> ---

A brief commit message is needed. Other than that:

Reviewed-by: Aleksandar Markovic 

[Qemu-devel] [PATCH] target/riscv/pmp.c: pmpcfg_csr_read returns bogus value on RV64

[Dayeol Lee]

pmp_read_cfg() returns 8-bit value, which is combined together to form a single 
pmpcfg CSR. 
The default promotion rules will result in an integer here ("i*8" is integer, 
which
flows through) resulting in a 32-bit signed value on most hosts.
That's bogus on RV64I, with the high bits of the CSR being wrong.

Signed-off-by: Dayeol Lee 
Reviewed-by: Palmer Dabbelt 
---
 target/riscv/pmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index c828950..3d3906a 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -330,7 +330,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t 
reg_index)
 {
 int i;
 target_ulong cfg_val = 0;
-uint8_t val = 0;
+target_ulong val = 0;
 
 if(sizeof(target_ulong) == 8)
 reg_index /= 2;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode

[Aleksandar Markovic]

> From: Peter Maydell 
> Subject: Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode
> 
> On 26 October 2018 at 15:21, Stefan Markovic  
> wrote:
> > From: Stefan Markovic 
> >
> > Floating-point mode is calculated from MIPS.abiflags FP ABI value
> > (based on kernel implementation). Illegal combinations are rejected.
> >
> > Signed-off-by: Stefan Markovic 
> > ---
> >  linux-user/mips/cpu_loop.c | 75 
> > ++
> >  1 file changed, 75 insertions(+)
> 
> > + if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
> > +|| (info->interp_fp_abi > MAX_FP_ABI &&
> > +info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
> > +fprintf(stderr, "qemu: Program and interpreter have "
> > +"unexpected FPU modes\n");
> > +exit(137);
> 
> Why are we exit()ing with a funny exit status code here?
> 
> If this is a "can't happen" case, then we should assert(). If
> it is a "can happen if fed an odd binary" case, then we should just
> exit(1) as we do already in this function for an unsupported NaN mode.
> 

Thanks for the review.

This is a "can happen if fed an odd binary" case. Or, in other words, and more 
precisely, an executable compiled with one FP option attempts to load a library 
compiled with another, incompatible, FP option.

Kernel counterpart lines are:

https://elixir.bootlin.com/linux/v4.19/source/arch/mips/kernel/elf.c#L211
https://elixir.bootlin.com/linux/v4.19/source/arch/mips/kernel/elf.c#L263

I think the error code is important for MIPS loader to work as designed in such 
cases. Stefan should be best positioned to explain and analyze the cases, since 
he worked on verifying and fixing involved scenarios, not only from QEMU 
perspective. However, he will be back most likely only on Monday.

Thanks again,
Aleksandar

> > +}
> > +
> > +prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> > +: fpu_reqs[info->fp_abi];
> > +interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> > +: 
> > fpu_reqs[info->interp_fp_abi];
> > +
> > +prog_req.single &= interp_req.single;
> > +prog_req.soft &= interp_req.soft;
> > +prog_req.fr1 &= interp_req.fr1;
> > +prog_req.frdefault &= interp_req.frdefault;
> > +prog_req.fre &= interp_req.fre;
> > +
> > +bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
> > +  env->insn_flags & ISA_MIPS64R2 ||
> > +  env->insn_flags & ISA_MIPS32R6 ||
> > +  env->insn_flags & ISA_MIPS64R6;
> > +
> > +if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
> > +env->CP0_Config5 |= (1 << CP0C5_FRE);
> > +if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
> > +env->hflags |= MIPS_HFLAG_FRE;
> > +}
> > +} else if ((prog_req.fr1 && prog_req.frdefault) ||
> > + (prog_req.single && !prog_req.frdefault)) {
> > +if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
> > +&& cpu_has_mips_r2_r6) || prog_req.fr1) {
> > +env->CP0_Status |= (1 << CP0St_FR);
> > +env->hflags |= MIPS_HFLAG_F64;
> > +}
> > +} else  if (!prog_req.fre && !prog_req.frdefault &&
> > +  !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
> > +exit(137);
> > +}
> 
> Ditto here (and we haven't printed any error message here...)
> 
> thanks
> -- PMM
> 

[Qemu-devel] [PULL 17/20] hw/pci-host: Remove useless parenthesis around DIV_ROUND_UP macro

[Laurent Vivier]

From: Philippe Mathieu-Daudé 

Patch created mechanically by rerunning:

  $  spatch --sp-file scripts/coccinelle/round.cocci \
--macro-file scripts/cocci-macro-file.h \
--dir . --in-place

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Markus Armbruster 
Acked-by: Michael S. Tsirkin 
Message-Id: <20180705155811.20366-7-f4...@amsat.org>
Signed-off-by: Laurent Vivier 
---
 hw/pci-host/piix.c | 2 +-
 hw/pci-host/q35.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index da73743fa2..47293a3915 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -144,7 +144,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
 memory_region_transaction_begin();
 for (i = 0; i < 13; i++) {
 pam_update(>pam_regions[i], i,
-   pd->config[I440FX_PAM + (DIV_ROUND_UP(i, 2))]);
+   pd->config[I440FX_PAM + DIV_ROUND_UP(i, 2)]);
 }
 memory_region_set_enabled(>smram_region,
   !(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 8ce1e09932..966a7cf92d 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -356,7 +356,7 @@ static void mch_update_pam(MCHPCIState *mch)
 memory_region_transaction_begin();
 for (i = 0; i < 13; i++) {
 pam_update(>pam_regions[i], i,
-   pd->config[MCH_HOST_BRIDGE_PAM0 + (DIV_ROUND_UP(i, 2))]);
+   pd->config[MCH_HOST_BRIDGE_PAM0 + DIV_ROUND_UP(i, 2)]);
 }
 memory_region_transaction_commit();
 }
-- 
2.17.2

Re: [Qemu-devel] [RFC v4 48/71] mips: convert to cpu_interrupt_request

[Richard Henderson]

On 10/25/18 3:46 PM, Emilio G. Cota wrote:
> Cc: Aurelien Jarno 
> Cc: Aleksandar Markovic 
> Cc: James Hogan 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/mips/cpu.c | 7 ---
>  target/mips/kvm.c | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 

r~

[Qemu-devel] [PULL 01/20] configs/alpha: Remove unused CONFIG_PARALLEL_ISA switch

[Laurent Vivier]

From: Thomas Huth 

We don't use CONFIG_PARALLEL_ISA in any of our Makefiles, so this
is just a dead config option which can be removed.

Fixes: a4cb773928e047b137c6998209cf2eec857fac6b
Signed-off-by: Thomas Huth 
Acked-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <1540376314-5727-1-git-send-email-th...@redhat.com>
Signed-off-by: Laurent Vivier 
---
 default-configs/alpha-softmmu.mak | 1 -
 1 file changed, 1 deletion(-)

diff --git a/default-configs/alpha-softmmu.mak 
b/default-configs/alpha-softmmu.mak
index eb58b40254..4d654eaa0b 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -8,7 +8,6 @@ CONFIG_I82374=y
 CONFIG_I8254=y
 CONFIG_I8257=y
 CONFIG_PARALLEL=y
-CONFIG_PARALLEL_ISA=y
 CONFIG_FDC=y
 CONFIG_PCKBD=y
 CONFIG_VGA_CIRRUS=y
-- 
2.17.2

Re: [Qemu-devel] [PATCH 6/6] Add prctl() PR_SET_FP_MODE and PR_GET_FP_MODE implementations

[Aleksandar Markovic]

> From: Stefan Markovic 
> Subject: [PATCH 6/6] Add prctl() PR_SET_FP_MODE and PR_GET_FP_MODE 
> implementations
> 
> From: Stefan Markovic 
> 
> Signed-off-by: Stefan Markovic 

>  #define TARGET_PR_SET_FP_MODE  45
>  #define TARGET_PR_GET_FP_MODE  46
> +#define TARGET_PR_FP_MODE_FR  (1 << 0)
> +#define TARGET_PR_FP_MODE_FRE (1 << 1)

There should be one space more to the left of (1 << 0) and (1 << 1) to achieve 
nicer alignment.

A short commit message is needed too. Other than that:

Reviewed-by: Aleksandar Markovic 

Re: [Qemu-devel] [RFC v4 36/71] arm: convert to cpu_interrupt_request

[Emilio G. Cota]

On Fri, Oct 26, 2018 at 14:39:21 +0100, Alex Bennée wrote:
> 
> Emilio G. Cota  writes:
> 
> > Cc: Peter Maydell 
> 
> This will need to catch-up in the next re-base as there is a merge conflict.

Yep, this series is so long that I decided to keep the same
baseline as in v3, so that I could just git diff the two
branches to track the delta.

I'll rebase on master in the next iteration.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v2] migration: avoid segmentfault when take a snapshot of a VM which being migrated

[Dr. David Alan Gilbert]

* Jia Lina (jialin...@baidu.com) wrote:
> During an active background migration, snapshot will trigger a
> segmentfault. As snapshot clears the "current_migration" struct
> and updates "to_dst_file" before it finds out that there is a
> migration task, Migration accesses the null pointer in
> "current_migration" struct and qemu crashes eventually.
> 
> Signed-off-by: Jia Lina 
> Signed-off-by: Chai Wen 
> Signed-off-by: Zhang Yu 

Thanks, that looks better.



Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/migration.c |  2 +-
>  migration/migration.h |  2 ++
>  migration/savevm.c| 19 +++
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index d6ae879dc8..b5e71c7bfc 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -711,7 +711,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
> **errp)
>   * Return true if we're already in the middle of a migration
>   * (i.e. any of the active or setup states)
>   */
> -static bool migration_is_setup_or_active(int state)
> +bool migration_is_setup_or_active(int state)
>  {
>  switch (state) {
>  case MIGRATION_STATUS_ACTIVE:
> diff --git a/migration/migration.h b/migration/migration.h
> index f7813f8261..e413d4d8b6 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -241,6 +241,8 @@ void migrate_fd_error(MigrationState *s, const Error 
> *error);
>  
>  void migrate_fd_connect(MigrationState *s, Error *error_in);
>  
> +bool migration_is_setup_or_active(int state);
> +
>  void migrate_init(MigrationState *s);
>  bool migration_is_blocked(Error **errp);
>  /* True if outgoing migration has entered postcopy phase */
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 2d10e45582..eeade8cb92 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1319,21 +1319,25 @@ static int qemu_savevm_state(QEMUFile *f, Error 
> **errp)
>  MigrationState *ms = migrate_get_current();
>  MigrationStatus status;
>  
> -migrate_init(ms);
> -
> -ms->to_dst_file = f;
> +if (migration_is_setup_or_active(ms->state) ||
> +ms->state == MIGRATION_STATUS_CANCELLING ||
> +ms->state == MIGRATION_STATUS_COLO) {
> +error_setg(errp, QERR_MIGRATION_ACTIVE);
> +return -EINVAL;
> +}
>  
>  if (migration_is_blocked(errp)) {
> -ret = -EINVAL;
> -goto done;
> +return -EINVAL;
>  }
>  
>  if (migrate_use_block()) {
>  error_setg(errp, "Block migration and snapshots are incompatible");
> -ret = -EINVAL;
> -goto done;
> +return -EINVAL;
>  }
>  
> +migrate_init(ms);
> +ms->to_dst_file = f;
> +
>  qemu_mutex_unlock_iothread();
>  qemu_savevm_state_header(f);
>  qemu_savevm_state_setup(f);
> @@ -1355,7 +1359,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>  error_setg_errno(errp, -ret, "Error while writing VM state");
>  }
>  
> -done:
>  if (ret != 0) {
>  status = MIGRATION_STATUS_FAILED;
>  } else {
> -- 
> 2.13.2.windows.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 1/3] target/mips: Rename MMI-related masks

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename MMI-related masks.

Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 51a5488..e38d50d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2159,7 +2159,7 @@ enum {
  *7 111 |   *   |   *   |   *   |   *   | PSLLW |   *   | PSRLW | PSRAW
  */
 
-#define MASK_TX79_MMI(op) (MASK_OP_MAJOR(op) | ((op) & 0x3F))
+#define MASK_MMI(op) (MASK_OP_MAJOR(op) | ((op) & 0x3F))
 enum {
 TX79_MMI_MADD   = 0x00 | TX79_CLASS_MMI, /* Same as OPC_MADD */
 TX79_MMI_MADDU  = 0x01 | TX79_CLASS_MMI, /* Same as OPC_MADDU */
@@ -2210,7 +2210,7 @@ enum {
  *7 111 |   *   |   *   | PEXT5 | PPAC5
  */
 
-#define MASK_TX79_MMI0(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
+#define MASK_MMI0(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
 enum {
 TX79_MMI0_PADDW  = (0x00 << 6) | TX79_MMI_CLASS_MMI0,
 TX79_MMI0_PSUBW  = (0x01 << 6) | TX79_MMI_CLASS_MMI0,
@@ -2261,7 +2261,7 @@ enum {
  *7 111 |   *   |   *   |   *   |   *
  */
 
-#define MASK_TX79_MMI1(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
+#define MASK_MMI1(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
 enum {
 TX79_MMI1_PABSW  = (0x01 << 6) | TX79_MMI_CLASS_MMI1,
 TX79_MMI1_PCEQW  = (0x02 << 6) | TX79_MMI_CLASS_MMI1,
@@ -2305,7 +2305,7 @@ enum {
  *7 111 | PMULTH| PDIVBW| PEXEW | PROT3W
  */
 
-#define MASK_TX79_MMI2(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
+#define MASK_MMI2(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
 enum {
 TX79_MMI2_PMADDW = (0x00 << 6) | TX79_MMI_CLASS_MMI2,
 TX79_MMI2_PSLLVW = (0x02 << 6) | TX79_MMI_CLASS_MMI2,
@@ -2353,7 +2353,7 @@ enum {
  *7 111 |   *   |   *   | PEXCW |   *
  */
 
-#define MASK_TX79_MMI3(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
+#define MASK_MMI3(op) (MASK_OP_MAJOR(op) | ((op) & 0x7FF))
 enum {
 TX79_MMI3_PMADDUW = (0x00 << 6) | TX79_MMI_CLASS_MMI3,
 TX79_MMI3_PSRAVW  = (0x03 << 6) | TX79_MMI_CLASS_MMI3,
@@ -24683,7 +24683,7 @@ static void decode_opc_special3_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 
 static void decode_tx79_mmi0(CPUMIPSState *env, DisasContext *ctx)
 {
-uint32_t opc = MASK_TX79_MMI0(ctx->opcode);
+uint32_t opc = MASK_MMI0(ctx->opcode);
 
 switch (opc) {
 case TX79_MMI0_PADDW: /* TODO: TX79_MMI0_PADDW */
@@ -24722,7 +24722,7 @@ static void decode_tx79_mmi0(CPUMIPSState *env, 
DisasContext *ctx)
 
 static void decode_tx79_mmi1(CPUMIPSState *env, DisasContext *ctx)
 {
-uint32_t opc = MASK_TX79_MMI1(ctx->opcode);
+uint32_t opc = MASK_MMI1(ctx->opcode);
 
 switch (opc) {
 case TX79_MMI1_PABSW: /* TODO: TX79_MMI1_PABSW */
@@ -24754,7 +24754,7 @@ static void decode_tx79_mmi1(CPUMIPSState *env, 
DisasContext *ctx)
 
 static void decode_tx79_mmi2(CPUMIPSState *env, DisasContext *ctx)
 {
-uint32_t opc = MASK_TX79_MMI2(ctx->opcode);
+uint32_t opc = MASK_MMI2(ctx->opcode);
 
 switch (opc) {
 case TX79_MMI2_PMADDW:/* TODO: TX79_MMI2_PMADDW */
@@ -24790,7 +24790,7 @@ static void decode_tx79_mmi2(CPUMIPSState *env, 
DisasContext *ctx)
 
 static void decode_tx79_mmi3(CPUMIPSState *env, DisasContext *ctx)
 {
-uint32_t opc = MASK_TX79_MMI3(ctx->opcode);
+uint32_t opc = MASK_MMI3(ctx->opcode);
 
 switch (opc) {
 case TX79_MMI3_PMADDUW:/* TODO: TX79_MMI3_PMADDUW */
@@ -24817,7 +24817,7 @@ static void decode_tx79_mmi3(CPUMIPSState *env, 
DisasContext *ctx)
 
 static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
 {
-uint32_t opc = MASK_TX79_MMI(ctx->opcode);
+uint32_t opc = MASK_MMI(ctx->opcode);
 int rs = extract32(ctx->opcode, 21, 5);
 int rt = extract32(ctx->opcode, 16, 5);
 int rd = extract32(ctx->opcode, 11, 5);
-- 
2.7.4

[Qemu-devel] [PATCH 2/3] target/mips: Rename MMI-related opcodes

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename MMI-related opcodes.

Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 470 
 1 file changed, 235 insertions(+), 235 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e38d50d..4b008d8 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2099,7 +2099,7 @@ enum {
  * The TX79-specific Multimedia Instruction encodings
  * ==
  *
- * TX79 Multimedia Instruction encoding table keys:
+ * MMI Instruction encoding table keys:
  *
  * *   This code is reserved for future use. An attempt to execute it
  * causes a Reserved Instruction exception.
@@ -2110,7 +2110,7 @@ enum {
  * DMULTU, DDIV, DDIVU, LL, LLD, SC, SCD, LWC2 and SWC2. An attempt
  * to execute it causes a Reserved Instruction exception.
  *
- * TX79 Multimedia Instructions encoded by opcode field (MMI, LQ, SQ):
+ * MMI Instructions encoded by opcode field (MMI, LQ, SQ):
  *
  *  31260
  * +++
@@ -2132,13 +2132,13 @@ enum {
  */
 
 enum {
-TX79_CLASS_MMI = 0x1C << 26,/* Same as OPC_SPECIAL2 */
-TX79_LQ= 0x1E << 26,/* Same as OPC_MSA */
-TX79_SQ= 0x1F << 26,/* Same as OPC_SPECIAL3 */
+MMI_CLASS_MMI = 0x1C << 26,/* Same as OPC_SPECIAL2 */
+MMI_LQ= 0x1E << 26,/* Same as OPC_MSA */
+MMI_SQ= 0x1F << 26,/* Same as OPC_SPECIAL3 */
 };
 
 /*
- * TX79 Multimedia Instructions with opcode field = MMI:
+ * MMI Instructions with opcode field = MMI:
  *
  *  3126 5  0
  * ++---++
@@ -2161,35 +2161,35 @@ enum {
 
 #define MASK_MMI(op) (MASK_OP_MAJOR(op) | ((op) & 0x3F))
 enum {
-TX79_MMI_MADD   = 0x00 | TX79_CLASS_MMI, /* Same as OPC_MADD */
-TX79_MMI_MADDU  = 0x01 | TX79_CLASS_MMI, /* Same as OPC_MADDU */
-TX79_MMI_PLZCW  = 0x04 | TX79_CLASS_MMI,
-TX79_MMI_CLASS_MMI0 = 0x08 | TX79_CLASS_MMI,
-TX79_MMI_CLASS_MMI2 = 0x09 | TX79_CLASS_MMI,
-TX79_MMI_MFHI1  = 0x10 | TX79_CLASS_MMI, /* Same minor as OPC_MFHI */
-TX79_MMI_MTHI1  = 0x11 | TX79_CLASS_MMI, /* Same minor as OPC_MTHI */
-TX79_MMI_MFLO1  = 0x12 | TX79_CLASS_MMI, /* Same minor as OPC_MFLO */
-TX79_MMI_MTLO1  = 0x13 | TX79_CLASS_MMI, /* Same minor as OPC_MTLO */
-TX79_MMI_MULT1  = 0x18 | TX79_CLASS_MMI, /* Same minor as OPC_MULT */
-TX79_MMI_MULTU1 = 0x19 | TX79_CLASS_MMI, /* Same minor as OPC_MULTU */
-TX79_MMI_DIV1   = 0x1A | TX79_CLASS_MMI, /* Same minor as OPC_DIV */
-TX79_MMI_DIVU1  = 0x1B | TX79_CLASS_MMI, /* Same minor as OPC_DIVU */
-TX79_MMI_MADD1  = 0x20 | TX79_CLASS_MMI,
-TX79_MMI_MADDU1 = 0x21 | TX79_CLASS_MMI,
-TX79_MMI_CLASS_MMI1 = 0x28 | TX79_CLASS_MMI,
-TX79_MMI_CLASS_MMI3 = 0x29 | TX79_CLASS_MMI,
-TX79_MMI_PMFHL  = 0x30 | TX79_CLASS_MMI,
-TX79_MMI_PMTHL  = 0x31 | TX79_CLASS_MMI,
-TX79_MMI_PSLLH  = 0x34 | TX79_CLASS_MMI,
-TX79_MMI_PSRLH  = 0x36 | TX79_CLASS_MMI,
-TX79_MMI_PSRAH  = 0x37 | TX79_CLASS_MMI,
-TX79_MMI_PSLLW  = 0x3C | TX79_CLASS_MMI,
-TX79_MMI_PSRLW  = 0x3E | TX79_CLASS_MMI,
-TX79_MMI_PSRAW  = 0x3F | TX79_CLASS_MMI,
+MMI_OPC_MADD   = 0x00 | MMI_CLASS_MMI, /* Same as OPC_MADD */
+MMI_OPC_MADDU  = 0x01 | MMI_CLASS_MMI, /* Same as OPC_MADDU */
+MMI_OPC_PLZCW  = 0x04 | MMI_CLASS_MMI,
+MMI_OPC_CLASS_MMI0 = 0x08 | MMI_CLASS_MMI,
+MMI_OPC_CLASS_MMI2 = 0x09 | MMI_CLASS_MMI,
+MMI_OPC_MFHI1  = 0x10 | MMI_CLASS_MMI, /* Same minor as OPC_MFHI */
+MMI_OPC_MTHI1  = 0x11 | MMI_CLASS_MMI, /* Same minor as OPC_MTHI */
+MMI_OPC_MFLO1  = 0x12 | MMI_CLASS_MMI, /* Same minor as OPC_MFLO */
+MMI_OPC_MTLO1  = 0x13 | MMI_CLASS_MMI, /* Same minor as OPC_MTLO */
+MMI_OPC_MULT1  = 0x18 | MMI_CLASS_MMI, /* Same minor as OPC_MULT */
+MMI_OPC_MULTU1 = 0x19 | MMI_CLASS_MMI, /* Same minor as OPC_MULTU */
+MMI_OPC_DIV1   = 0x1A | MMI_CLASS_MMI, /* Same minor as OPC_DIV */
+MMI_OPC_DIVU1  = 0x1B | MMI_CLASS_MMI, /* Same minor as OPC_DIVU */
+MMI_OPC_MADD1  = 0x20 | MMI_CLASS_MMI,
+MMI_OPC_MADDU1 = 0x21 | MMI_CLASS_MMI,
+MMI_OPC_CLASS_MMI1 = 0x28 | MMI_CLASS_MMI,
+MMI_OPC_CLASS_MMI3 = 0x29 | MMI_CLASS_MMI,
+MMI_OPC_PMFHL  = 0x30 | MMI_CLASS_MMI,
+MMI_OPC_PMTHL  = 0x31 | MMI_CLASS_MMI,
+MMI_OPC_PSLLH  = 0x34 | MMI_CLASS_MMI,
+MMI_OPC_PSRLH  = 0x36 | MMI_CLASS_MMI,
+MMI_OPC_PSRAH  = 0x37 | MMI_CLASS_MMI,
+MMI_OPC_PSLLW  = 0x3C | MMI_CLASS_MMI,
+MMI_OPC_PSRLW  = 0x3E | MMI_CLASS_MMI,
+MMI_OPC_PSRAW  = 0x3F | MMI_CLASS_MMI,
 };
 
 /*
- * TX79 Multimedia Instructions with opcode field 

[Qemu-devel] [PATCH 3/3] target/mips: Rename MMI-related functions

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename MMI-related functions.

Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 4b008d8..155331f 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24681,7 +24681,7 @@ static void decode_opc_special3_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 }
 }
 
-static void decode_tx79_mmi0(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t opc = MASK_MMI0(ctx->opcode);
 
@@ -24720,7 +24720,7 @@ static void decode_tx79_mmi0(CPUMIPSState *env, 
DisasContext *ctx)
 }
 }
 
-static void decode_tx79_mmi1(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi1(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t opc = MASK_MMI1(ctx->opcode);
 
@@ -24752,7 +24752,7 @@ static void decode_tx79_mmi1(CPUMIPSState *env, 
DisasContext *ctx)
 }
 }
 
-static void decode_tx79_mmi2(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t opc = MASK_MMI2(ctx->opcode);
 
@@ -24788,7 +24788,7 @@ static void decode_tx79_mmi2(CPUMIPSState *env, 
DisasContext *ctx)
 }
 }
 
-static void decode_tx79_mmi3(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t opc = MASK_MMI3(ctx->opcode);
 
@@ -24815,7 +24815,7 @@ static void decode_tx79_mmi3(CPUMIPSState *env, 
DisasContext *ctx)
 }
 }
 
-static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t opc = MASK_MMI(ctx->opcode);
 int rs = extract32(ctx->opcode, 21, 5);
@@ -24824,16 +24824,16 @@ static void decode_tx79_mmi(CPUMIPSState *env, 
DisasContext *ctx)
 
 switch (opc) {
 case MMI_OPC_CLASS_MMI0:
-decode_tx79_mmi0(env, ctx);
+decode_mmi0(env, ctx);
 break;
 case MMI_OPC_CLASS_MMI1:
-decode_tx79_mmi1(env, ctx);
+decode_mmi1(env, ctx);
 break;
 case MMI_OPC_CLASS_MMI2:
-decode_tx79_mmi2(env, ctx);
+decode_mmi2(env, ctx);
 break;
 case MMI_OPC_CLASS_MMI3:
-decode_tx79_mmi3(env, ctx);
+decode_mmi3(env, ctx);
 break;
 case MMI_OPC_MULT1:
 case MMI_OPC_MULTU1:
@@ -24873,12 +24873,12 @@ static void decode_tx79_mmi(CPUMIPSState *env, 
DisasContext *ctx)
 }
 }
 
-static void decode_tx79_lq(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi_lq(CPUMIPSState *env, DisasContext *ctx)
 {
 generate_exception_end(ctx, EXCP_RI);/* TODO: MMI_LQ */
 }
 
-static void gen_tx79_sq(DisasContext *ctx, int base, int rt, int offset)
+static void gen_mmi_sq(DisasContext *ctx, int base, int rt, int offset)
 {
 generate_exception_end(ctx, EXCP_RI);/* TODO: MMI_SQ */
 }
@@ -24904,7 +24904,7 @@ static void gen_tx79_sq(DisasContext *ctx, int base, 
int rt, int offset)
  * In user mode, QEMU must verify the upper and lower 11 bits to distinguish
  * between SQ and RDHWR, as the Linux kernel does.
  */
-static void decode_tx79_sq(CPUMIPSState *env, DisasContext *ctx)
+static void decode_mmi_sq(CPUMIPSState *env, DisasContext *ctx)
 {
 int base = extract32(ctx->opcode, 21, 5);
 int rt = extract32(ctx->opcode, 16, 5);
@@ -24922,7 +24922,7 @@ static void decode_tx79_sq(CPUMIPSState *env, 
DisasContext *ctx)
 }
 #endif
 
-gen_tx79_sq(ctx, base, rt, offset);
+gen_mmi_sq(ctx, base, rt, offset);
 }
 
 static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
@@ -26231,14 +26231,14 @@ static void decode_opc(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 case OPC_SPECIAL2:
 if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) {
-decode_tx79_mmi(env, ctx);
+decode_mmi(env, ctx);
 } else {
 decode_opc_special2_legacy(env, ctx);
 }
 break;
 case OPC_SPECIAL3:
 if (ctx->insn_flags & INSN_R5900) {
-decode_tx79_sq(env, ctx);/* MMI_SQ */
+decode_mmi_sq(env, ctx);/* MMI_SQ */
 } else {
 decode_opc_special3(env, ctx);
 }
@@ -26902,7 +26902,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 break;
 case OPC_MSA: /* OPC_MDMX */
 if (ctx->insn_flags & INSN_R5900) {
-decode_tx79_lq(env, ctx);/* MMI_LQ */
+decode_mmi_lq(env, ctx);/* MMI_LQ */
 } else {
 /* MDMX: Not implemented. */
 gen_msa(env, ctx);
-- 
2.7.4

[Qemu-devel] [PATCH 0/3] target/mips: Rename MMI-related code elements

[Aleksandar Markovic]

From: Aleksandar Markovic 

This series renames MMI-related code elements so that they do not
contain TX79 substring. Tx79 is one of CPUs that support MMI ASE.
Opcodes and other code elements should be as generic as possible,
and should not contain CPU name if they are supported by multiple
CPUs. In cases when there is a single-CPU-specific opcode, an
anoter special convention should apply, like, for example,
MMI_OPC__TX79_XXX or MMI_OPC__R5900_YYY. So far these cases were
not identified, but there will be some in the future. But overall,
the great MMI opcodes are shared (stay the same) between different
CPUs that support MMI.

Aleksandar Markovic (3):
  target/mips: Rename MMI-related masks
  target/mips: Rename MMI-related opcodes
  target/mips: Rename MMI-related functions

 target/mips/translate.c | 518 
 1 file changed, 259 insertions(+), 259 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PATCH 04/10] cputlb: Split large page tracking per mmu_idx

[Emilio G. Cota]

On Tue, Oct 23, 2018 at 08:02:47 +0100, Richard Henderson wrote:
> +static void tlb_flush_page_locked(CPUArchState *env, int midx,
> +  target_ulong addr)
> +{
> +target_ulong lp_addr = env->tlb_d[midx].large_page_addr;
> +target_ulong lp_mask = env->tlb_d[midx].large_page_mask;
> +
> +/* Check if we need to flush due to large pages.  */
> +if ((addr & lp_mask) == lp_addr) {
> +tlb_debug("forcing full flush midx %d ("
> +  TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
> +  midx, lp_addr, lp_mask);
> +tlb_flush_one_mmuidx_locked(env, midx);
> +} else {
> +int pidx = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> +tlb_flush_entry_locked(>tlb_table[midx][pidx], addr);
> +tlb_flush_vtlb_page_locked(env, midx, addr);

Just noticed that we should use tlb_entry here, e.g.:

 } else {
-int pidx = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-tlb_flush_entry_locked(>tlb_table[midx][pidx], addr);
+CPUTLBEntry *entry = tlb_entry(env, midx, addr);
+
+tlb_flush_entry_locked(entry, addr);
 tlb_flush_vtlb_page_locked(env, midx, addr);
 }

Thanks,

Emilio

Re: [Qemu-devel] [PATCH 2/4] migration: introduce lockless multithreads model

[Emilio G. Cota]

On Wed, Oct 17, 2018 at 12:10:15 +0200, Paolo Bonzini wrote:
> On 16/10/2018 13:10, guangrong.x...@gmail.com wrote:

> An idea: the total number of requests is going to be very small, and a
> PtrRing is not the nicest data structure for multiple producer/single
> consumer.  So you could instead:
(snip)
> - now that you have request indices, you can replace the completion
> ptr_ring with a bitmap, and set a bit in the bitmap with set_bit_atomic
> to report completion.  On the writer side you use find_next_bit to find
(snip)
> Emilio, can you review the above ideas?

Sorry it took me a while to go through this.

I like your suggestions. Just one nit; I'm not sure I understood
the use case very well, but I think using a bitmap to signal
completion might be suboptimal, since we'd have several
thread spinning on the same cacheline yet caring about
different bits.

Xiao: a couple of suggestions

- Since you'll be adding a generic module, make its commit and
  description self-contained. That is, mentioning in the
  log that this will be used for migration is fine, but please
  describe the module (and the assumptions it makes about its
  users) in general, so that someone that doesn't know anything
  about migration can still understand this module (and hopefully
  adopt it for other use cases).

- I'd like to see a simple test program (or rather, benchmark)
  that shows how this works. This benchmark would be completely
  unrelated to migration; it should just be a simple test of
  the performance/scalability of this module.
  Having this benchmark would help (1) discuss and quantitately
  evaluate modifications to the module, and (2) help others to
  quickly understand what the module does.
  See tests/qht-bench.c for an example.

Thanks,

Emilio

[Qemu-devel] [Bug 1485180] Re: Ctrl Alt G -- Multiple Virtual Machines

[Launchpad Bug Tracker]

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

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

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

Title:
  Ctrl Alt G -- Multiple Virtual Machines

Status in QEMU:
  Expired

Bug description:
  I'm using Fedora 22.

  Firstly, what works:
  A single VM instance, running Windows. Although, I am keeping this (GTK) 
window focused.

  What really fails:
  If I have two running VM's, WIndows XP and Windows Vista:
  1. I press Ctrl-Alt-G to get the focus.
  2. That works first time.
  3. Then I press Ctrl-Alt-G again.
  4. Then Alt-Tab to the other machine (switching from XP to Vista, or back.)
  5. Then press Ctrl-Alt-G to gain focus:
  - Problem is that now the Ctrl-Alt-G, although showing in the title bar, only 
grabs the mouse, but NOT the keyboard. That is to say, whilst in Ctrl-Alt-G 
mode the second time, pressing Alt-Tab jumps back to the other VM!

  Pressing Alt-F4 quits! Regardless of whether Ctrl-Alt-G mode or 
not!
  But only when running two VM's.

  Thanks
  Misha

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

[Qemu-devel] [Bug 1493033] Re: memory leak/high memory usage with spice webdav feature

[Launchpad Bug Tracker]

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

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

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

Title:
  memory leak/high memory usage with spice webdav feature

Status in QEMU:
  Expired

Bug description:
  This bug is being open due the comment:
  https://bugs.freedesktop.org/show_bug.cgi?id=91350#c9

  Description of problem:
  When copying big files from client to guest, the memory usage in the host 
grows by about the size of the file. This is partially spice problem due the 
memory pool being able to increase as much as necessary without a limit which 
should be handled by the patches sent in the mailing list [0]

  [0] http://lists.freedesktop.org/archives/spice-
  devel/2015-August/021644.html

  At the same time, massif shows high memory usage by qemu as well [1]
  (output attached)

  [1] (peak)
  ->49.64% (267,580,319B) 0x308B89: malloc_and_trace (vl.c:2724)
  | ->49.38% (266,167,561B) 0x67CE678: g_malloc (gmem.c:97)
  | | ->49.03% (264,241,152B) 0x511D8E: qemu_coroutine_new 
(coroutine-ucontext.c:106)
  | | | ->49.03% (264,241,152B) 0x510E24: qemu_coroutine_create 
(qemu-coroutine.c:74)
  (...)

  The file being shared was a 320M ogv video.

  Version-Release number of selected component (if applicable):
  QEMU emulator version 2.3.93
  SPICE and SPICE-GTK: from git master

  How reproducible:
  100%

  Steps to Reproduce:
  1-) build spice-gtk with --enable-webdav=yes
  2-) enable webdav in your VM by following:
  https://elmarco.fedorapeople.org/manual.html#_folder_sharing
  3-) using remote-viewer with webdav patches, connects to a fedora guest
  4-) Open nautilus, go to 'Browse Network'
  5-) On remote-viewer, enable shared folder by File > Preferences > [X] Share 
folder
  6-) The spice client folder should appear: Double-click to mount it.
  7-) Check the memory of your qemu process
  8-) Copy a big file (let's say, 300 MB) from the shared folder to local VM
  9-) See the memory consumption of qemu grows by a lot;

  Actual results:
  Memory usage grows during copy and is not freed

  Expected results:
  Memory should have an upper limit to grow and should be freed after copy

  Additional info:
  Also reported in Fedora/rawhide: 
https://bugzilla.redhat.com/show_bug.cgi?id=1256376
  SPICE upstream bug: https://bugs.freedesktop.org/show_bug.cgi?id=91350

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

Re: [Qemu-devel] [PATCH v1] lsi53c895a: check message length value is valid

[Peter Maydell]

On 26 October 2018 at 20:43, P J P  wrote:
> From: Prasad J Pandit 
>
> While writing a message in 'lsi_do_msgin', message length value
> in 'msg_len' could be invalid. Add check to avoid OOB access issue.
>
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/scsi/lsi53c895a.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> Update v1: add .post_load routine and an assert() call
>   -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05730.html
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index d1e6534311..3a40e62853 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -861,12 +861,13 @@ static void lsi_do_status(LSIState *s)
>
>  static void lsi_do_msgin(LSIState *s)
>  {
> -int len;
> +uint8_t len;
>  trace_lsi_do_msgin(s->dbc, s->msg_len);
>  s->sfbr = s->msg[0];
>  len = s->msg_len;
>  if (len > s->dbc)
>  len = s->dbc;
> +assert(len <= LSI_MAX_MSGIN_LEN);
>  pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
>  /* Linux drivers rely on the last byte being in the SIDL.  */
>  s->sidl = s->msg[len - 1];

Is it possible to get here with len == 0 ?

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/3] MIPS queue for October 2018 - part 3

[Peter Maydell]

On 25 October 2018 at 21:19, Aleksandar Markovic
 wrote:
> From: Aleksandar Markovic 
>
> The following changes since commit 808ebd66e467f77c0d1f8c6346235f81e9c99cf2:
>
>   Merge remote-tracking branch 'remotes/riscv/tags/riscv-for-master-3.1-sf0' 
> into staging (2018-10-25 17:41:03 +0100)
>
> are available in the git repository at:
>
>   https://github.com/AMarkovic/qemu tags/mips-queue-oct-2018-part-3
>
> for you to fetch changes up to 89a955e8df3dba6f96859cd0339356243b6c996f:
>
>   target/mips: Add disassembler support for nanoMIPS (2018-10-25 22:13:33 
> +0200)
>
> 
> MIPS queue for October 2018 - part 3
>
>   - this pull request contains three assorted nanoMIPS issues
>   - three checkpatch.pl warnings are known and should be ignored
> 
>


Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Peter Maydell]

On 26 October 2018 at 14:26, Singh, Brijesh  wrote:
>
>
> On 10/25/2018 07:59 PM, Michael S. Tsirkin wrote:
>> On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
>>> These should presumably all be "ULL". (The "UL" suffix is
>>> usually a bug, as it's either unnecessary or should be ULL.)
>>
>> Yea.  Fixed. Brijesh could you start cleaning up that header generally?
>> It has all kind of weird code like using bitfields for hardware
>> accesses. That isn't portable - switch to full dword fields with shift
>> and | to operate them and proper cpu_to_le APIs or similar please.
>>
>
>
> Noted, I will look into cleaning up this and send patches for reviews.

Thanks. You might like to look at the extract32()/extract64()/
deposit32()/deposit64() functions in bitops.h, which are often (but not
always) cleaner than hand-rolled shifts-and-logical-ops for assembling
and disassembling fields.

thanks
-- PMM

[Qemu-devel] [PATCH v1] lsi53c895a: check message length value is valid

[P J P]

From: Prasad J Pandit 

While writing a message in 'lsi_do_msgin', message length value
in 'msg_len' could be invalid. Add check to avoid OOB access issue.

Signed-off-by: Prasad J Pandit 
---
 hw/scsi/lsi53c895a.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Update v1: add .post_load routine and an assert() call
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05730.html

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d1e6534311..3a40e62853 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -861,12 +861,13 @@ static void lsi_do_status(LSIState *s)
 
 static void lsi_do_msgin(LSIState *s)
 {
-int len;
+uint8_t len;
 trace_lsi_do_msgin(s->dbc, s->msg_len);
 s->sfbr = s->msg[0];
 len = s->msg_len;
 if (len > s->dbc)
 len = s->dbc;
+assert(len <= LSI_MAX_MSGIN_LEN);
 pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
 /* Linux drivers rely on the last byte being in the SIDL.  */
 s->sidl = s->msg[len - 1];
@@ -2103,11 +2104,23 @@ static int lsi_pre_save(void *opaque)
 return 0;
 }
 
+static int lsi_post_load(void *opaque, int version_id)
+{
+LSIState *s = opaque;
+
+if (s->msg_len < 0 || s->msg_len > LSI_MAX_MSGIN_LEN) {
+return -EINVAL;
+}
+
+return 0;
+}
+
 static const VMStateDescription vmstate_lsi_scsi = {
 .name = "lsiscsi",
 .version_id = 0,
 .minimum_version_id = 0,
 .pre_save = lsi_pre_save,
+.post_load = lsi_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_PCI_DEVICE(parent_obj, LSIState),
 
-- 
2.17.2

[Qemu-devel] template for a script for running an ad-hoc QEMU image

[Peter Maydell]

In a conversation this week I mentioned the scripts I use for running
ad-hoc QEMU guest images that I have. The idea here is just that
by making sure that whenever I get a test image I set it up to be
run in the same way, I don't have the problem of trying to remember
how to run the guest if I come back to it after six months.
Here's an example -- I generally just copy-paste-and-edit an older
script when I get a new test image.


#!/bin/sh -e
TESTDIR="$(cd "$(dirname "$0")"; pwd)"
QEMU="$@"

${QEMU} -M lm3s6965evb -kernel "${TESTDIR}"/qs_ek-lm3s6965.bin


I put each guest image in its own directory, and the script is
always named "runme" and is executable. Then any test can be run with
path/to/test/runme path/to/qemu-system-whatever

There's no magic here but there are a couple of nice nuances here:
 * the rune at the top sets TESTDIR to the directory containing the
   script, regardless of what the current working directory is when
   you run the script; references to kernels, disk files, etc should
   then all use $TESTDIR rather than being absolute or relative paths
 * the use of $@ means you can also do

path/to/test/runme gdb --args path/to/qemu-system-whatever
path/to/test/runme valgrind path/to/qemu-system-whatever

and other similar things.

This doesn't fix any of the problems of using ad-hoc human-run
images for testing, but at least it makes them all be runnable in
the same way, reducing the barrier to randomly running one of them.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1] bt: use size_t type for length parameters instead of int

[P J P]

+-- On Sun, 21 Oct 2018, P J P wrote --+
| The length parameter values are not negative, thus use an unsigned
| type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
| calls. If it was negative, it could lead to memory corruption issues.
| Add check to avoid it.
| 
| Reported-by: Arash TC 
| Signed-off-by: Prasad J Pandit 
| ---
|  bt-host.c  |  8 +++---
|  bt-vhci.c  |  7 +++---
|  hw/bt/core.c   |  2 +-
|  hw/bt/hci-csr.c| 20 +++
|  hw/bt/hci.c| 38 ++--
|  hw/bt/hid.c| 10 
|  hw/bt/l2cap.c  | 56 ++
|  hw/bt/sdp.c|  6 ++---
|  hw/usb/dev-bluetooth.c | 12 -
|  include/hw/bt.h|  8 +++---
|  include/sysemu/bt.h| 10 
|  11 files changed, 90 insertions(+), 87 deletions(-)
| 
| Update v1: add assert check in vhci_host_send. Also check other places wherein
| length is used with fixed size buffers.
|   -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03831.html

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 0/3] target/mips: Rename MMI-related code elements

[Fredrik Noring]

Hi Aleksandar,

> This series renames MMI-related code elements so that they do not
> contain TX79 substring. Tx79 is one of CPUs that support MMI ASE.
> Opcodes and other code elements should be as generic as possible,
> and should not contain CPU name if they are supported by multiple
> CPUs. In cases when there is a single-CPU-specific opcode, an
> anoter special convention should apply, like, for example,
> MMI_OPC__TX79_XXX or MMI_OPC__R5900_YYY. So far these cases were
> not identified, but there will be some in the future. But overall,
> the great MMI opcodes are shared (stay the same) between different
> CPUs that support MMI.
> 
> Aleksandar Markovic (3):
>   target/mips: Rename MMI-related masks
>   target/mips: Rename MMI-related opcodes
>   target/mips: Rename MMI-related functions
> 
>  target/mips/translate.c | 518 
> 
>  1 file changed, 259 insertions(+), 259 deletions(-)

This is interesting. Could you name a few other ISAs, beside the R5900
and the TX79, that have 128-bit GPRs and equivalent MMIs?

Fredrik

[Qemu-devel] [PATCH v3 15/16] gdbstub: add multiprocess extension support

[Luc Michel]

Add multiprocess extension support by enabling multiprocess mode when
the peer requests it, and by replying that we actually support it in the
qSupported reply packet.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 09480df2bf..a1b63de34b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1718,10 +1718,16 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
 cc = CPU_GET_CLASS(first_cpu);
 if (cc->gdb_core_xml_file != NULL) {
 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
 }
+
+if (strstr(p, "multiprocess+")) {
+s->multiprocess = true;
+}
+pstrcat(buf, sizeof(buf), ";multiprocess+");
+
 put_packet(s, buf);
 break;
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
-- 
2.19.1

[Qemu-devel] [PATCH v3 11/16] gdbstub: add support for extended mode packet

[Luc Michel]

Add support for the '!' extended mode packet. This is required for the
multiprocess extension.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 4d8474204f..9c239c1760 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1291,10 +1291,13 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
+case '!':
+put_packet(s, "OK");
+break;
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
 snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
  gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/1] i386: Add PKU/OSPKE on Skylake-Server CPU model

[Eduardo Habkost]

On Fri, Oct 26, 2018 at 01:53:10PM +0800, Tao Xu wrote:
> On 10/25/18 9:28 PM, Eduardo Habkost wrote:
> > On Wed, Oct 17, 2018 at 12:48:58PM +0200, Paolo Bonzini wrote:
> > > On 17/10/2018 11:30, Tao Xu wrote:
> > > > As the release document ref below link (page 13):
> > > > https://software.intel.com/sites/default/files/managed/c5/15/\
> > > > architecture-instruction-set-extensions-programming-reference.pdf
> > > > 
> > > > PKU is supported in Skylake Server (Only Server) and later, and
> > > > on Intel(R) Xeon(R) Processor Scalable Family. OSPKE is to reads
> > > > the value of PKRU (Instruction of PKU) into EAX and clears EDX.
> > > > So PKU/OSPKE are supposed to be in Skylake-Server CPU model.
> > > > And PKU/OSPKE 's CPUID has been exposed to QEMU. But PKU/OSPKE
> > > > can't be find in Skylake-Server CPU model in the code.
> > > > So this patch will fix PKU/OSPKE this issue in Skylake-Server
> > > > CPU model.
> > > OSPKE is not needed, since it is added automatically based on CR4 (and
> > > is not set on boot).
> > Correct.
> > 
> > > Also, the guru of CPU model compatibility is Eduardo, so I'll wait for
> > > him to chime in anyway.
> > Sorry for taking so long to reply.  This can be safely done only
> > if every host that is able to run Skylake-Server today is
> > guaranteed to support PKU.  Is that the case?
> > 
> > You'll also need Skylake-Server-*-cpu.pku=off entries on
> > PC_COMPAT_3_0 to keep PKU disabled on pc-*-3.0 and older.
> > 
> Thank you Eduardo,
> 
> 
> But I can't find PC_COMPAT_3_0 in  include/hw/i386/pc.h. Will it exist on
> 
> QEMU 3.1 and will I add "pku=off" after QEMU 3.1  release?

PC_COMPAT_3_0 was added to qemu.git master a few weeks ago, by
commit 9b4cf107b09d18ac30f46fd1c4de8585ccba030c.

-- 
Eduardo

[Qemu-devel] [PATCH v3 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached

[Luc Michel]

When gdb_set_stop_cpu() is called with a CPU associated to a process
currently not attached by the GDB client, return without modifying the
stop CPU. Otherwise, GDB get confused if it receives packets with a
thread-id it does not know about.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index b3461eff9e..09480df2bf 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1784,10 +1784,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 return RS_IDLE;
 }
 
 void gdb_set_stop_cpu(CPUState *cpu)
 {
+GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
+
+if (!p->attached) {
+/* Having a stop CPU corresponding to a process that is not attached
+ * confuses GDB. So we ignore the request.
+ */
+return;
+}
+
 gdbserver_state->c_cpu = cpu;
 gdbserver_state->g_cpu = cpu;
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.19.1

[Qemu-devel] [PATCH v3 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters

[Luc Michel]

Create two separate CPU clusters for APUs and RPUs.

Signed-off-by: Luc Michel 
---
 include/hw/arm/xlnx-zynqmp.h |  3 +++
 hw/arm/xlnx-zynqmp.c | 23 +++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 98f925ab84..591515c760 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -29,10 +29,11 @@
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/dma/xlnx-zdma.h"
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/timer/xlnx-zynqmp-rtc.h"
+#include "hw/cpu/cluster.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
TYPE_XLNX_ZYNQMP)
 
@@ -75,10 +76,12 @@
 typedef struct XlnxZynqMPState {
 /*< private >*/
 DeviceState parent_obj;
 
 /*< public >*/
+CPUClusterState apu_cluster;
+CPUClusterState rpu_cluster;
 ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
 ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
 GICState gic;
 MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c195040350..7cc01f0b77 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -176,16 +176,23 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, 
const char *boot_cpu,
 {
 Error *err = NULL;
 int i;
 int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
XLNX_ZYNQMP_NUM_RPU_CPUS);
 
+object_initialize_child(OBJECT(s), "rpu-cluster", >rpu_cluster,
+sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
+_abort, NULL);
+
+qdev_prop_set_uint32(DEVICE(>rpu_cluster), "cluster-id", 1);
+qdev_init_nofail(DEVICE(>rpu_cluster));
+
 for (i = 0; i < num_rpus; i++) {
 char *name;
 
 object_initialize(>rpu_cpu[i], sizeof(s->rpu_cpu[i]),
   "cortex-r5f-" TYPE_ARM_CPU);
-object_property_add_child(OBJECT(s), "rpu-cpu[*]",
+object_property_add_child(OBJECT(>rpu_cluster), "rpu-cpu[*]",
   OBJECT(>rpu_cpu[i]), _abort);
 
 name = object_get_canonical_path_component(OBJECT(>rpu_cpu[i]));
 if (strcmp(name, boot_cpu)) {
 /* Secondary CPUs start in PSCI powered-down state */
@@ -211,14 +218,19 @@ static void xlnx_zynqmp_init(Object *obj)
 {
 XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
 int i;
 int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
+object_initialize_child(obj, "apu-cluster", >apu_cluster,
+sizeof(s->apu_cluster), TYPE_CPU_CLUSTER,
+_abort, NULL);
+
 for (i = 0; i < num_apus; i++) {
-object_initialize_child(obj, "apu-cpu[*]", >apu_cpu[i],
-sizeof(s->apu_cpu[i]),
-"cortex-a53-" TYPE_ARM_CPU, _abort, 
NULL);
+object_initialize_child(OBJECT(>apu_cluster), "apu-cpu[*]",
+>apu_cpu[i], sizeof(s->apu_cpu[i]),
+"cortex-a53-" TYPE_ARM_CPU, _abort,
+NULL);
 }
 
 sysbus_init_child_obj(obj, "gic", >gic, sizeof(s->gic),
   gic_class_name());
 
@@ -331,10 +343,13 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 qdev_prop_set_uint32(DEVICE(>gic), "num-cpu", num_apus);
 qdev_prop_set_bit(DEVICE(>gic), "has-security-extensions", s->secure);
 qdev_prop_set_bit(DEVICE(>gic),
   "has-virtualization-extensions", s->virt);
 
+qdev_prop_set_uint32(DEVICE(>apu_cluster), "cluster-id", 0);
+qdev_init_nofail(DEVICE(>apu_cluster));
+
 /* Realize APUs before realizing the GIC. KVM requires this.  */
 for (i = 0; i < num_apus; i++) {
 char *name;
 
 object_property_set_int(OBJECT(>apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
-- 
2.19.1

[Qemu-devel] [PATCH v3 12/16] gdbstub: add support for vAttach packets

[Luc Michel]

Add support for the vAttach packets. In multiprocess mode, GDB sends
them to attach to additional processes.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 9c239c1760..e5eddd8e2b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1337,10 +1337,45 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 break;
 }
 goto unknown_command;
 }
 break;
+} else if (strncmp(p, "Attach;", 7) == 0) {
+unsigned long pid;
+
+p += 7;
+
+if (qemu_strtoul(p, , 16, )) {
+put_packet(s, "E22");
+break;
+}
+
+process = gdb_get_process(s, pid);
+
+if (process == NULL) {
+put_packet(s, "E22");
+break;
+}
+
+cpu = get_first_cpu_in_process(s, process);
+
+if (cpu == NULL) {
+/* Refuse to attach an empty process */
+put_packet(s, "E22");
+break;
+}
+
+process->attached = true;
+
+s->g_cpu = cpu;
+s->c_cpu = cpu;
+
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+
+put_packet(s, buf);
+break;
 } else {
 goto unknown_command;
 }
 case 'k':
 /* Kill the target */
-- 
2.19.1

[Qemu-devel] [PATCH v3 02/16] gdbstub: introduce GDB processes

[Luc Michel]

Add a structure GDBProcess that represent processes from the GDB
semantic point of view.

CPUs can be split into different processes, by grouping them under
different cpu-cluster objects.  Each occurrence of a cpu-cluster object
implies the existence of the corresponding process in the GDB stub. The
GDB process ID is derived from the corresponding cluster ID as follows:

  GDB PID = cluster ID + 1

This is because PIDs -1 and 0 are reserved in GDB and cannot be used by
processes.

When no such container are found, all the CPUs are put in a unique GDB
process (create_unique_process()). This is also the case when compiled
in user mode, where multi-processes do not make much sense for now.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index c4e4f9f082..096ac8e99c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -27,10 +27,11 @@
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
 #include "sysemu/sysemu.h"
 #include "exec/gdbstub.h"
+#include "hw/cpu/cluster.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
 
 #include "qemu/sockets.h"
@@ -294,10 +295,15 @@ typedef struct GDBRegisterState {
 gdb_reg_cb set_reg;
 const char *xml;
 struct GDBRegisterState *next;
 } GDBRegisterState;
 
+typedef struct GDBProcess {
+uint32_t pid;
+bool attached;
+} GDBProcess;
+
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
 RS_GETLINE,
 RS_GETLINE_ESC,
@@ -322,10 +328,13 @@ typedef struct GDBState {
 int running_state;
 #else
 CharBackend chr;
 Chardev *mon_chr;
 #endif
+bool multiprocess;
+GDBProcess *processes;
+int process_num;
 char syscall_buf[256];
 gdb_syscall_complete_cb current_syscall_cb;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -1749,10 +1758,24 @@ void gdb_exit(CPUArchState *env, int code)
 #ifndef CONFIG_USER_ONLY
   qemu_chr_fe_deinit(>chr, true);
 #endif
 }
 
+/*
+ * Create a unique process containing all the CPUs.
+ */
+static void create_unique_process(GDBState *s)
+{
+GDBProcess *process;
+
+s->processes = g_malloc0(sizeof(GDBProcess));
+s->process_num = 1;
+process = >processes[0];
+
+process->pid = 1;
+}
+
 #ifdef CONFIG_USER_ONLY
 int
 gdb_handlesig(CPUState *cpu, int sig)
 {
 GDBState *s;
@@ -1846,10 +1869,11 @@ static bool gdb_accept(void)
 }
 
 s = g_malloc0(sizeof(GDBState));
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+create_unique_process(s);
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2002,10 +2026,57 @@ static const TypeInfo char_gdb_type_info = {
 .name = TYPE_CHARDEV_GDB,
 .parent = TYPE_CHARDEV,
 .class_init = char_gdb_class_init,
 };
 
+static int find_cpu_clusters(Object *child, void *opaque)
+{
+if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+GDBState *s = (GDBState *) opaque;
+CPUClusterState *cluster = CPU_CLUSTER(child);
+
+s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
+
+GDBProcess *process = >processes[s->process_num - 1];
+
+/* GDB process IDs -1 and 0 are reserved */
+process->pid = cluster->cluster_id + 1;
+process->attached = false;
+return 0;
+}
+
+return object_child_foreach(child, find_cpu_clusters, opaque);
+}
+
+static int pid_order(const void *a, const void *b)
+{
+GDBProcess *pa = (GDBProcess *) a;
+GDBProcess *pb = (GDBProcess *) b;
+
+return pa->pid - pb->pid;
+}
+
+static void create_processes(GDBState *s)
+{
+object_child_foreach(object_get_root(), find_cpu_clusters, s);
+
+if (!s->processes) {
+/* No CPU cluster specified by the machine */
+create_unique_process(s);
+} else {
+/* Sort by PID */
+qsort(s->processes, s->process_num, sizeof(s->processes[0]), 
pid_order);
+}
+}
+
+static void cleanup_processes(GDBState *s)
+{
+g_free(s->processes);
+s->process_num = 0;
+s->processes = NULL;
+}
+
 int gdbserver_start(const char *device)
 {
 trace_gdbstub_op_start(device);
 
 GDBState *s;
@@ -2058,15 +2129,19 @@ int gdbserver_start(const char *device)
NULL, _abort);
 monitor_init(mon_chr, 0);
 } else {
 qemu_chr_fe_deinit(>chr, true);
 mon_chr = s->mon_chr;
+cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+
+create_processes(s);
+
 if (chr) {
 qemu_chr_fe_init(>chr, chr, _abort);
 qemu_chr_fe_set_handlers(>chr, gdb_chr_can_receive, gdb_chr_receive,
  gdb_chr_event, NULL, NULL, NULL, true);
 }
-- 
2.19.1

[Qemu-devel] [PATCH v3 08/16] gdbstub: add multiprocess support to Xfer:features:read:

[Luc Michel]

Change the Xfer:features:read: packet handling to support the
multiprocess extension. This packet is used to request the XML
description of the CPU. In multiprocess mode, different descriptions can
be sent for different processes.

This function now takes the process to send the description for as a
parameter, and use a buffer in the process structure to store the
generated description.

It takes the first CPU of the process to generate the description.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 47 +++
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b8aa4b34af..b7079eff4a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
 } GDBRegisterState;
 
 typedef struct GDBProcess {
 uint32_t pid;
 bool attached;
+
+char target_xml[1024];
 } GDBProcess;
 
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
@@ -801,55 +803,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
 }
 
 return cpu;
 }
 
-static const char *get_feature_xml(const char *p, const char **newp,
-   CPUClass *cc)
+static const char *get_feature_xml(const GDBState *s, const char *p,
+   const char **newp, GDBProcess *process)
 {
 size_t len;
 int i;
 const char *name;
-static char target_xml[1024];
+CPUState *cpu = get_first_cpu_in_process(s, process);
+CPUClass *cc = CPU_GET_CLASS(cpu);
 
 len = 0;
 while (p[len] && p[len] != ':')
 len++;
 *newp = p + len;
 
 name = NULL;
 if (strncmp(p, "target.xml", len) == 0) {
+char *buf = process->target_xml;
+const size_t buf_sz = sizeof(process->target_xml);
+
 /* Generate the XML description for this CPU.  */
-if (!target_xml[0]) {
+if (!buf[0]) {
 GDBRegisterState *r;
-CPUState *cpu = first_cpu;
 
-pstrcat(target_xml, sizeof(target_xml),
+pstrcat(buf, buf_sz,
 ""
 ""
 "");
 if (cc->gdb_arch_name) {
 gchar *arch = cc->gdb_arch_name(cpu);
-pstrcat(target_xml, sizeof(target_xml), "");
-pstrcat(target_xml, sizeof(target_xml), arch);
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
+pstrcat(buf, buf_sz, arch);
+pstrcat(buf, buf_sz, "");
 g_free(arch);
 }
-pstrcat(target_xml, sizeof(target_xml), "gdb_core_xml_file);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "gdb_core_xml_file);
+pstrcat(buf, buf_sz, "\"/>");
 for (r = cpu->gdb_regs; r; r = r->next) {
-pstrcat(target_xml, sizeof(target_xml), "xml);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "xml);
+pstrcat(buf, buf_sz, "\"/>");
 }
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
 }
-return target_xml;
+return buf;
 }
 if (cc->gdb_get_dynamic_xml) {
-CPUState *cpu = first_cpu;
 char *xmlname = g_strndup(p, len);
 const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
 
 g_free(xmlname);
 if (xml) {
@@ -1255,10 +1259,11 @@ out:
 }
 
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
+GDBProcess *process;
 CPUClass *cc;
 const char *p;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
@@ -1636,18 +1641,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
 target_ulong total_len;
 
-cc = CPU_GET_CLASS(first_cpu);
+process = gdb_get_cpu_process(s, s->g_cpu);
+cc = CPU_GET_CLASS(s->g_cpu);
 if (cc->gdb_core_xml_file == NULL) {
 goto unknown_command;
 }
 
 gdb_has_xml = true;
 p += 19;
-xml = get_feature_xml(p, , cc);
+xml = get_feature_xml(s, p, , process);
 if (!xml) {
 snprintf(buf, sizeof(buf), "E00");
 put_packet(s, buf);
 break;
 }
@@ -2315,10 +2321,11 @@ static int find_cpu_clusters(Object *child, void 
*opaque)
 GDBProcess *process = >processes[s->process_num - 1];
 
 /* GDB process IDs -1 and 0 are reserved */
 process->pid = cluster->cluster_id + 1;
 process->attached = false;
+process->target_xml[0] = '\0';
 return 0;
 }
 
 return 

[Qemu-devel] [PATCH v3 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

[Luc Michel]

Change the thread info related packets handling to support multiprocess
extension.

Add the CPUs class name in the extra info to help differentiate
them in multiprocess mode.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 17fec8a41c..b8aa4b34af 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1257,11 +1257,10 @@ out:
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
-uint32_t thread;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
@@ -1553,30 +1552,46 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "QC%s",
  gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
-s->query_cpu = first_cpu;
+s->query_cpu = gdb_first_cpu(s);
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
 report_cpuinfo:
 if (s->query_cpu) {
-snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
+snprintf(buf, sizeof(buf), "m%s",
+ gdb_fmt_thread_id(s, s->query_cpu,
+   thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-s->query_cpu = CPU_NEXT(s->query_cpu);
+s->query_cpu = gdb_next_cpu(s, s->query_cpu);
 } else
 put_packet(s, "l");
 break;
 } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-thread = strtoull(p+16, (char **), 16);
-cpu = find_cpu(thread);
+if (read_thread_id(p + 16, , , ) == GDB_READ_THREAD_ERR) 
{
+put_packet(s, "E22");
+break;
+}
+cpu = gdb_get_cpu(s, pid, tid);
 if (cpu != NULL) {
 cpu_synchronize_state(cpu);
-/* memtohex() doubles the required space */
-len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-   "CPU#%d [%s]", cpu->cpu_index,
-   cpu->halted ? "halted " : "running");
+
+if (s->multiprocess && (s->process_num > 1)) {
+/* Print the CPU model in multiprocess mode */
+ObjectClass *oc = object_get_class(OBJECT(cpu));
+const char *cpu_model = object_class_get_name(oc);
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d %s [%s]", cpu->cpu_index,
+   cpu_model,
+   cpu->halted ? "halted " : "running");
+} else {
+/* memtohex() doubles the required space */
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d [%s]", cpu->cpu_index,
+   cpu->halted ? "halted " : "running");
+}
 trace_gdbstub_op_extra_info((char *)mem_buf);
 memtohex(buf, mem_buf, len);
 put_packet(s, buf);
 }
 break;
-- 
2.19.1

[Qemu-devel] [PATCH v3 00/16] gdbstub: support for the multiprocess extension

[Luc Michel]

changes since v2:
  - patch 1introducing the cpu-cluster type. I didn't opt for an
   Interface, but I can add one if you think it's necessary.
   For now this class inherits from Device and has a
   cluster-id property, used by the GDB stub to compute a
   PID.

  - patch 2removed GDB group related code as it has been replaced
   with CPU clusters

  - patch 2/8  moved GDBProcess target_xml field introduction into patch
   8 [Philippe]

  - patch 3gdb_get_cpu_pid() now search for CPU being a child of a
   cpu-cluster object. Use the cluster-id to compute the PID.

  - patch 4gdb_get_process() does not rely on s->processes array
   indices anymore as PIDs can now be sparse. Instead, iterate
   over the array to find the process.

  - patch 3/4  removed Reviewed-by tags because of substantial changes.

  - patch 4/7  read_thread_id() hardening [Philippe]

  - patch 12   safer vAttach packet parsing [Phillipe]

  - patch 16   put APUs and RPUs in different clusters instead of GDB
   groups

changes since v1:
  - rename qemu_get_thread_id() to gdb_fmt_thread_id() [Philippe]
  - check qemu_strtoul() return value for 'D' packets [Philippe]


This series adds support for the multiprocess extension of the GDB
remote protocol in the QEMU GDB stub.

This extension is useful to split QEMU emulated CPUs in different
processes from the point of view of the GDB client. It adds the
possibility to debug different kind of processors (e.g. an AArch64 and
an ARMv7 CPU) at the same time (it is not possible otherwise since GDB
expects an SMP view at the thread granularity.

CPUs are grouped using specially named QOM containers. CPUs that are
children of such a container are grouped under the same GDB process.

The last patch groups the CPUs of different model in the zynqmp machines
into separate processes.

To test this patchset, you can use the following commands:

(Note that this requires a recent enough GDB, I think GDB 7.2 is OK.
Also, it must be compiled to support both ARM and AArch64 architectures)

Run QEMU: (-smp 6 in xlnx-zcu102 enables both cortex-a53 and cortex-r5
CPUs)

qemu-system-aarch64 -M xlnx-zcu102 -gdb tcp::1234 -S -smp 6

Run the following commands in GDB:

target extended :1234
add-inferior
inferior 2
attach 2
info threads

I want to thanks the Xilinx's QEMU team who sponsored this work for
their collaboration and their prototype implementation.


Luc Michel (16):
  hw/cpu: introduce CPU clusters
  gdbstub: introduce GDB processes
  gdbstub: add multiprocess support to '?' packets
  gdbstub: add multiprocess support to 'H' and 'T' packets
  gdbstub: add multiprocess support to vCont packets
  gdbstub: add multiprocess support to 'sC' packets
  gdbstub: add multiprocess support to (f|s)ThreadInfo and
ThreadExtraInfo
  gdbstub: add multiprocess support to Xfer:features:read:
  gdbstub: add multiprocess support to gdb_vm_state_change()
  gdbstub: add multiprocess support to 'D' packets
  gdbstub: add support for extended mode packet
  gdbstub: add support for vAttach packets
  gdbstub: processes initialization on new peer connection
  gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
  gdbstub: add multiprocess extension support
  arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

 include/hw/arm/xlnx-zynqmp.h |   3 +
 include/hw/cpu/cluster.h |  38 +++
 gdbstub.c| 628 ++-
 hw/arm/xlnx-zynqmp.c |  23 +-
 hw/cpu/cluster.c |  49 +++
 hw/cpu/Makefile.objs |   2 +-
 6 files changed, 662 insertions(+), 81 deletions(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

-- 
2.19.1

[Qemu-devel] [PATCH v3 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()

[Luc Michel]

Add support for multiprocess extension in gdb_vm_state_change()
function.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b7079eff4a..39b1766f28 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1710,10 +1710,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
 GDBState *s = gdbserver_state;
 CPUState *cpu = s->c_cpu;
 char buf[256];
+char thread_id[16];
 const char *type;
 int ret;
 
 if (running || s->state == RS_INACTIVE) {
 return;
@@ -1721,10 +1722,18 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 /* Is there a GDB syscall waiting to be sent?  */
 if (s->current_syscall_cb) {
 put_packet(s, s->syscall_buf);
 return;
 }
+
+if (cpu == NULL) {
+/* No process attached */
+return;
+}
+
+gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+
 switch (state) {
 case RUN_STATE_DEBUG:
 if (cpu->watchpoint_hit) {
 switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
 case BP_MEM_READ:
@@ -1738,12 +1747,12 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 break;
 }
 trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
 (target_ulong)cpu->watchpoint_hit->vaddr);
 snprintf(buf, sizeof(buf),
- "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
- GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
+ "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+ GDB_SIGNAL_TRAP, thread_id, type,
  (target_ulong)cpu->watchpoint_hit->vaddr);
 cpu->watchpoint_hit = NULL;
 goto send_packet;
 } else {
 trace_gdbstub_hit_break();
@@ -1781,11 +1790,11 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 trace_gdbstub_hit_unknown(state);
 ret = GDB_SIGNAL_UNKNOWN;
 break;
 }
 gdb_set_stop_cpu(cpu);
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
 
 send_packet:
 put_packet(s, buf);
 
 /* disable single step if it was enabled */
-- 
2.19.1

Re: [Qemu-devel] [PULL v2 05/43] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Eduardo Habkost]

On Thu, Oct 25, 2018 at 06:17:59PM +0100, David Gibson wrote:
> On Thu, Oct 25, 2018 at 10:32:23AM -0300, Eduardo Habkost wrote:
> > From: Philippe Mathieu-Daudé 
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > Reviewed-by: Artyom Tarasenko 
> > Reviewed-by: Cédric Le Goater 
> > Message-Id: <20181002212522.23303-3-f4...@amsat.org>
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  hw/timer/sun4v-rtc.c  | 13 +++--
> >  hw/timer/trace-events |  4 
> >  2 files changed, 7 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> > index 310523225f..13be94f8da 100644
> > --- a/hw/timer/sun4v-rtc.c
> > +++ b/hw/timer/sun4v-rtc.c
> > @@ -14,15 +14,8 @@
> >  #include "hw/sysbus.h"
> >  #include "qemu/timer.h"
> >  #include "hw/timer/sun4v-rtc.h"
> > +#include "trace.h"
> >  
> > -//#define DEBUG_SUN4V_RTC
> > -
> > -#ifdef DEBUG_SUN4V_RTC
> > -#define DPRINTF(fmt, ...)   \
> > -do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
> > -#else
> > -#define DPRINTF(fmt, ...) do {} while (0)
> > -#endif
> >  
> >  #define TYPE_SUN4V_RTC "sun4v_rtc"
> >  #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
> > @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr 
> > addr,
> >  /* accessing the high 32 bits */
> >  val >>= 32;
> >  }
> > -DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
> > +trace_sun4v_rtc_read(addr, val);
> >  return val;
> >  }
> >  
> >  static void sun4v_rtc_write(void *opaque, hwaddr addr,
> >   uint64_t val, unsigned size)
> >  {
> > -DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
> > +trace_sun4v_rtc_read(addr, val);
> 
> Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().

Oops, my bad.  I can fix this manually in case there's a v3 pull
request, but I would really prefer that to be included in a
follow up patch instead of making this block the entire pull
request.

-- 
Eduardo

[Qemu-devel] [PATCH v3 03/16] gdbstub: add multiprocess support to '?' packets

[Luc Michel]

The gdb_get_cpu_pid() function does the PID lookup for the given CPU. It
checks if the CPU is a direct child of a CPU cluster. If it is, the
returned PID is the cluster ID plus one (cluster IDs start at 0, GDB
PIDs at 1). When the CPU is not a child of such a container, the PID of
the first process is returned.

The gdb_fmt_thread_id() function generates the string to be used to identify
a given thread, in a response packet for the peer. This function
supports generating thread IDs when multiprocess mode is enabled (in the
form `p.').

Use them in the reply to a '?' request.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 57 +--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 096ac8e99c..9ca743a4c6 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -638,10 +638,49 @@ static int memtox(char *buf, const char *mem, int len)
 }
 }
 return p - buf;
 }
 
+static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
+{
+#ifndef CONFIG_USER_ONLY
+gchar *path, *name;
+Object *obj;
+CPUClusterState *cluster;
+uint32_t ret;
+
+path = object_get_canonical_path(OBJECT(cpu));
+name = object_get_canonical_path_component(OBJECT(cpu));
+
+if (path == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+path[strlen(path) - strlen(name) - 1] = '\0';
+
+obj = object_resolve_path_type(path, TYPE_CPU_CLUSTER, NULL);
+
+if (obj == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+cluster = CPU_CLUSTER(obj);
+ret = cluster->cluster_id + 1;
+
+out:
+g_free(name);
+g_free(path);
+
+return ret;
+
+#else
+return s->processes[0].pid;
+#endif
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -907,10 +946,23 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
+   char *buf, size_t buf_size)
+{
+if (s->multiprocess) {
+snprintf(buf, buf_size, "p%02x.%02x",
+ gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+} else {
+snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+}
+
+return buf;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1018,22 +1070,23 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 const char *p;
 uint32_t thread;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
+char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP,
- cpu_gdb_index(s->c_cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 /* Remove all the breakpoints when this query is issued,
  * because gdb is doing and initial connect and the state
  * should be cleaned up.
  */
-- 
2.19.1

Re: [Qemu-devel] [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[Cole Robinson]

On 10/25/2018 09:37 PM, Daniel P. Berrangé wrote:

On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:

While being at it deprecate cirrus too.

Reason (short version): use stdvga instead.
Verbose version:
 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful


Every single one of my guests is using cirrus. This wasn't an explicit
choice on my part, so I believe it is being used as the default in
virt-install.



virt-manager/virt-install 2.0.0 will never explicitly set cirrus, but 
that release is quite new. The previous release's default for x86 was 
roughly:


if using spice graphics:
use qxl
elif guest os is windows:
use vga
else:
use cirrus

It was definitely sub optimal. Maybe your virt-install commands were 
explicitly setting --graphics vnc which would trigger cirrus in that case.



I don't debate the points in the blog post above that stdvga is a
better choice, but I don't think that's enough to justify deprecating
cirrus at this point in time, because when it then gets deleted it
will break way too many existing deployments.

We need to socialize info in that blog post above more widely and
especially ensure that apps are not using that by default. I don't
see it being viable to formally deprecate it in QEMU any time soon
though given existing usage.

Re: [Qemu-devel] [PATCH] cpu.h: fix a typo in comment

[Laurent Vivier]

On 05/09/2018 13:29, Li Qiang wrote:
> Found by reading the code.
> 
> Signed-off-by: Li Qiang 
> ---
>  include/qom/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index dc130cd307..5bb94a9f86 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -852,7 +852,7 @@ extern CPUInterruptHandler cpu_interrupt_handler;
>  /**
>   * cpu_interrupt:
>   * @cpu: The CPU to set an interrupt on.
> - * @mask: The interupts to set.
> + * @mask: The interrupts to set.
>   *
>   * Invokes the interrupt handler.
>   */
> 

Applied

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 0/9] iotests: Make them work for both Python 2 and 3

[Eduardo Habkost]

On Mon, Oct 22, 2018 at 02:52:58PM +0100, Max Reitz wrote:
> This series prepares the iotests to work with both Python 2 and 3.  In
> some places, it adds version-specific code and decides what to do based
> on the version (for instance, whether to import the StringIO or the
> BytesIO class from 'io' for use with the test runner), but most of the
> time, it just makes code work for both versions in general.
> 
> And when we make the switch to make Python 3 mandatory, we can simply
> drop the Python 2 branches.

Queued on python-next, thanks!

-- 
Eduardo

[Qemu-devel] [PATCH v2] migration: avoid segmentfault when take a snapshot of a VM which being migrated

[Jia Lina]

During an active background migration, snapshot will trigger a
segmentfault. As snapshot clears the "current_migration" struct
and updates "to_dst_file" before it finds out that there is a
migration task, Migration accesses the null pointer in
"current_migration" struct and qemu crashes eventually.

Signed-off-by: Jia Lina 
Signed-off-by: Chai Wen 
Signed-off-by: Zhang Yu 
---
 migration/migration.c |  2 +-
 migration/migration.h |  2 ++
 migration/savevm.c| 19 +++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index d6ae879dc8..b5e71c7bfc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -711,7 +711,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
  * Return true if we're already in the middle of a migration
  * (i.e. any of the active or setup states)
  */
-static bool migration_is_setup_or_active(int state)
+bool migration_is_setup_or_active(int state)
 {
 switch (state) {
 case MIGRATION_STATUS_ACTIVE:
diff --git a/migration/migration.h b/migration/migration.h
index f7813f8261..e413d4d8b6 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -241,6 +241,8 @@ void migrate_fd_error(MigrationState *s, const Error 
*error);
 
 void migrate_fd_connect(MigrationState *s, Error *error_in);
 
+bool migration_is_setup_or_active(int state);
+
 void migrate_init(MigrationState *s);
 bool migration_is_blocked(Error **errp);
 /* True if outgoing migration has entered postcopy phase */
diff --git a/migration/savevm.c b/migration/savevm.c
index 2d10e45582..eeade8cb92 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1319,21 +1319,25 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 MigrationState *ms = migrate_get_current();
 MigrationStatus status;
 
-migrate_init(ms);
-
-ms->to_dst_file = f;
+if (migration_is_setup_or_active(ms->state) ||
+ms->state == MIGRATION_STATUS_CANCELLING ||
+ms->state == MIGRATION_STATUS_COLO) {
+error_setg(errp, QERR_MIGRATION_ACTIVE);
+return -EINVAL;
+}
 
 if (migration_is_blocked(errp)) {
-ret = -EINVAL;
-goto done;
+return -EINVAL;
 }
 
 if (migrate_use_block()) {
 error_setg(errp, "Block migration and snapshots are incompatible");
-ret = -EINVAL;
-goto done;
+return -EINVAL;
 }
 
+migrate_init(ms);
+ms->to_dst_file = f;
+
 qemu_mutex_unlock_iothread();
 qemu_savevm_state_header(f);
 qemu_savevm_state_setup(f);
@@ -1355,7 +1359,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 error_setg_errno(errp, -ret, "Error while writing VM state");
 }
 
-done:
 if (ret != 0) {
 status = MIGRATION_STATUS_FAILED;
 } else {
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: check size before data buffer access

[Cédric Le Goater]

Hello Prasad,

On 10/25/18 8:45 AM, P J P wrote:
>   Hello Cedric,
> 
> +-- On Wed, 24 Oct 2018, Cédric Le Goater wrote --+
> | I think using a data[8] would be more appropriate. It would make the 
> | pnv_lpc_do_eccb() routine a little more complex. I tried to rewrite it to 
> | have a common one with the P9 LPC model but could not find a common 
> pattern. 
> | P9 is purely MMIO based. Something on the TODO list.
> | 
> | 8 bytes accesses will then fail anyhow because all MemoryRegionOps have a 
> | max_access_size = 4.
> 
> Thank you for these inputs, appreciate it. To confirm,
> 
>  - You plan to send a v2 patch to fix the OOB access issue along with 
>refactoring pnv_lpc_do_eccb? 

we might improve pnv_lpc_do_eccb() when the P9 LPC model is sent. I am not
sure of that as this is for P8 only.

> If yes, kindly include me in CC please.

yes sure.  

> OR
> 
>  - While we refactor the routine for better, a patch below seem okay to fix 
>the OOB access issue?

I think it is fine. Please add something like :  

qemu_log_mask(LOG_GUEST_ERROR, "ECCB: invalid operation at @0x%08x" 
" size %d\n", opb_addr, sz);


Thanks,

C.

> ===
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index d7721320a2..655d5f3d07 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -155,9 +155,13 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, 
> uint64_t cmd)
>  /* XXX Check for magic bits at the top, addr size etc... */
>  unsigned int sz = (cmd & ECCB_CTL_SZ_MASK) >> ECCB_CTL_SZ_LSH;
>  uint32_t opb_addr = cmd & ECCB_CTL_ADDR_MASK;
> -uint8_t data[4];
> +uint8_t data[8];
>  bool success;
>  
> +if (sz > sizeof(data)) {
> +return;
> +}
> +
>  if (cmd & ECCB_CTL_READ) {
>  success = opb_read(lpc, opb_addr, data, sz);
>  if (success) {
> ===
> 
> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
> 

[Qemu-devel] [PATCH v3 10/16] gdbstub: add multiprocess support to 'D' packets

[Luc Michel]

'D' packets are used by GDB to detach from a process. In multiprocess
mode, the PID to detach from is sent in the request.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 60 ---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 39b1766f28..4d8474204f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1036,24 +1036,39 @@ static int gdb_breakpoint_remove(target_ulong addr, 
target_ulong len, int type)
 default:
 return -ENOSYS;
 }
 }
 
+static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
+{
+cpu_breakpoint_remove_all(cpu, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+cpu_watchpoint_remove_all(cpu, BP_GDB);
+#endif
+}
+
+static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
+{
+CPUState *cpu = get_first_cpu_in_process(s, p);
+
+while (cpu) {
+gdb_cpu_breakpoint_remove_all(cpu);
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+}
+
 static void gdb_breakpoint_remove_all(void)
 {
 CPUState *cpu;
 
 if (kvm_enabled()) {
 kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
 return;
 }
 
 CPU_FOREACH(cpu) {
-cpu_breakpoint_remove_all(cpu, BP_GDB);
-#ifndef CONFIG_USER_ONLY
-cpu_watchpoint_remove_all(cpu, BP_GDB);
-#endif
+gdb_cpu_breakpoint_remove_all(cpu);
 }
 }
 
 static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 {
@@ -1328,13 +1343,44 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 /* Kill the target */
 error_report("QEMU: Terminated via GDBstub");
 exit(0);
 case 'D':
 /* Detach packet */
-gdb_breakpoint_remove_all();
-gdb_syscall_mode = GDB_SYS_DISABLED;
-gdb_continue(s);
+pid = 1;
+
+if (s->multiprocess) {
+unsigned long lpid;
+if (*p != ';') {
+put_packet(s, "E22");
+break;
+}
+
+if (qemu_strtoul(p + 1, , 16, )) {
+put_packet(s, "E22");
+break;
+}
+
+pid = lpid;
+}
+
+process = gdb_get_process(s, pid);
+gdb_process_breakpoint_remove_all(s, process);
+process->attached = false;
+
+if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
+s->c_cpu = gdb_first_cpu(s);
+}
+
+if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+s->g_cpu = gdb_first_cpu(s);
+}
+
+if (s->c_cpu == NULL) {
+/* No more process attached */
+gdb_syscall_mode = GDB_SYS_DISABLED;
+gdb_continue(s);
+}
 put_packet(s, "OK");
 break;
 case 's':
 if (*p != '\0') {
 addr = strtoull(p, (char **), 16);
-- 
2.19.1

[Qemu-devel] [PATCH v3 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets

[Luc Michel]

Add a couple of helper functions to cope with GDB threads and processes.

The gdb_get_process() function looks for a process given a pid.

The gdb_get_cpu() function returns the CPU corresponding to the (pid,
tid) pair given as parameters.

The read_thread_id() function parses the thread-id sent by the peer.
This function supports the multiprocess extension thread-id syntax.  The
return value specifies if the parsing failed, or if a special case was
encountered (all processes or all threads).

Use them in 'H' and 'T' packets handling to support the multiprocess
extension.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 152 +++---
 1 file changed, 134 insertions(+), 18 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 9ca743a4c6..c437af7aca 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -677,10 +677,73 @@ out:
 #else
 return s->processes[0].pid;
 #endif
 }
 
+static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
+{
+int i;
+
+if (!pid) {
+/* 0 means any process, we take the first one */
+return >processes[0];
+}
+
+for (i = 0; i < s->process_num; i++) {
+if (s->processes[i].pid == pid) {
+return >processes[i];
+}
+}
+
+return NULL;
+}
+
+static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
+{
+return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
+}
+
+static CPUState *find_cpu(uint32_t thread_id)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (cpu_gdb_index(cpu) == thread_id) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
+{
+GDBProcess *process;
+CPUState *cpu = find_cpu(tid);
+
+if (!tid) {
+/* 0 means any thread, we take the first one */
+tid = 1;
+}
+
+if (cpu == NULL) {
+return NULL;
+}
+
+process = gdb_get_cpu_process(s, cpu);
+
+if (process->pid != pid) {
+return NULL;
+}
+
+if (!process->attached) {
+return NULL;
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -933,23 +996,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 
 cpu_synchronize_state(cpu);
 cpu_set_pc(cpu, pc);
 }
 
-static CPUState *find_cpu(uint32_t thread_id)
-{
-CPUState *cpu;
-
-CPU_FOREACH(cpu) {
-if (cpu_gdb_index(cpu) == thread_id) {
-return cpu;
-}
-}
-
-return NULL;
-}
-
 static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
char *buf, size_t buf_size)
 {
 if (s->multiprocess) {
 snprintf(buf, buf_size, "p%02x.%02x",
@@ -959,10 +1009,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, 
CPUState *cpu,
 }
 
 return buf;
 }
 
+typedef enum GDBThreadIdKind {
+GDB_ONE_THREAD = 0,
+GDB_ALL_THREADS, /* One process, all threads */
+GDB_ALL_PROCESSES,
+GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
+  uint32_t *pid, uint32_t *tid)
+{
+unsigned long p, t;
+int ret;
+
+if (*buf == 'p') {
+buf++;
+ret = qemu_strtoul(buf, , 16, );
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+/* Skip '.' */
+buf++;
+} else {
+p = 1;
+}
+
+ret = qemu_strtoul(buf, , 16, );
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+*end_buf = buf;
+
+if (p == -1) {
+return GDB_ALL_PROCESSES;
+}
+
+if (pid) {
+*pid = p;
+}
+
+if (t == -1) {
+return GDB_ALL_THREADS;
+}
+
+if (tid) {
+*tid = t;
+}
+
+return GDB_ONE_THREAD;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1067,16 +1171,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
 uint32_t thread;
+uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
+GDBThreadIdKind thread_kind;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
@@ -1280,16 +1386,22 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 else
 put_packet(s, "E22");
 break;
 case 'H':
 type = *p++;
-thread = strtoull(p, (char **), 16);
-if (thread == -1 || thread == 0) {
+
+thread_kind = 

[Qemu-devel] [PATCH v3 13/16] gdbstub: processes initialization on new peer connection

[Luc Michel]

When a new connection is established, we set the first process to be
attached, and the others detached. The first CPU of the first process
is selected as the current CPU.

Signed-off-by: Luc Michel 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e5eddd8e2b..b3461eff9e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2242,13 +2242,14 @@ static bool gdb_accept(void)
 close(fd);
 return false;
 }
 
 s = g_malloc0(sizeof(GDBState));
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 create_unique_process(s);
+s->processes[0].attached = true;
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2330,12 +2331,23 @@ static void gdb_chr_receive(void *opaque, const uint8_t 
*buf, int size)
 }
 }
 
 static void gdb_chr_event(void *opaque, int event)
 {
+int i;
+GDBState *s = (GDBState *) opaque;
+
 switch (event) {
 case CHR_EVENT_OPENED:
+/* Start with first process attached, others detached */
+for (i = 0; i < s->process_num; i++) {
+s->processes[i].attached = !i;
+}
+
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
+
 vm_stop(RUN_STATE_PAUSED);
 gdb_has_xml = false;
 break;
 default:
 break;
@@ -2509,19 +2521,17 @@ int gdbserver_start(const char *device)
 mon_chr = s->mon_chr;
 cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 
 create_processes(s);
 
 if (chr) {
 qemu_chr_fe_init(>chr, chr, _abort);
 qemu_chr_fe_set_handlers(>chr, gdb_chr_can_receive, gdb_chr_receive,
- gdb_chr_event, NULL, NULL, NULL, true);
+ gdb_chr_event, NULL, s, NULL, true);
 }
 s->state = chr ? RS_IDLE : RS_INACTIVE;
 s->mon_chr = mon_chr;
 s->current_syscall_cb = NULL;
 
-- 
2.19.1

[Qemu-devel] [PATCH v3 05/16] gdbstub: add multiprocess support to vCont packets

[Luc Michel]

Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
the CPUs in currently attached processes.

Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
iterate over CPUs of a given process.

Use them to add multiprocess extension support to vCont packets.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 117 +++---
 1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c437af7aca..d1ec245d1d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -713,10 +713,40 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static CPUState *get_first_cpu_in_process(const GDBState *s,
+  GDBProcess *process)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+{
+uint32_t pid = gdb_get_cpu_pid(s, cpu);
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_pid(s, cpu) == pid) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
 static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
 {
 GDBProcess *process;
 CPUState *cpu = find_cpu(tid);
 
@@ -740,10 +770,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t 
pid, uint32_t tid)
 }
 
 return cpu;
 }
 
+/* Return the cpu following @cpu, while ignoring
+ * unattached processes.
+ */
+static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
+{
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_process(s, cpu)->attached) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
+/* Return the first attached cpu */
+static CPUState *gdb_first_cpu(const GDBState *s)
+{
+CPUState *cpu = first_cpu;
+GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+if (!process->attached) {
+return gdb_next_cpu(s, cpu);
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -1078,14 +1139,16 @@ static int is_query_packet(const char *p, const char 
*query, char separator)
  * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
  * a format error, 0 on success.
  */
 static int gdb_handle_vcont(GDBState *s, const char *p)
 {
-int res, idx, signal = 0;
+int res, signal = 0;
 char cur_action;
 char *newstates;
 unsigned long tmp;
+uint32_t pid, tid;
+GDBProcess *process;
 CPUState *cpu;
 #ifdef CONFIG_USER_ONLY
 int max_cpus = 1; /* global variable max_cpus exists only in system mode */
 
 CPU_FOREACH(cpu) {
@@ -1124,29 +1187,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
 } else if (cur_action != 'c' && cur_action != 's') {
 /* unknown/invalid/unsupported command */
 res = -ENOTSUP;
 goto out;
 }
-/* thread specification. special values: (none), -1 = all; 0 = any */
-if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
-if (*p == ':') {
-p += 3;
-}
-for (idx = 0; idx < max_cpus; idx++) {
-if (newstates[idx] == 1) {
-newstates[idx] = cur_action;
+
+if (*p++ != ':') {
+res = -ENOTSUP;
+goto out;
+}
+
+switch (read_thread_id(p, , , )) {
+case GDB_READ_THREAD_ERR:
+res = -EINVAL;
+goto out;
+
+case GDB_ALL_PROCESSES:
+cpu = gdb_first_cpu(s);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
 }
+
+cpu = gdb_next_cpu(s, cpu);
 }
-} else if (*p == ':') {
-p++;
-res = qemu_strtoul(p, , 16, );
-if (res) {
+break;
+
+case GDB_ALL_THREADS:
+process = gdb_get_process(s, pid);
+
+if (!process->attached) {
+res = -EINVAL;
 goto out;
 }
 
-/* 0 means any thread, so we pick the first valid CPU */
-cpu = tmp ? find_cpu(tmp) : first_cpu;
+cpu = get_first_cpu_in_process(s, process);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
+}
+
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+break;
+
+case GDB_ONE_THREAD:
+cpu = gdb_get_cpu(s, pid, tid);
 
 /* invalid CPU/thread 

[Qemu-devel] [PATCH v3 01/16] hw/cpu: introduce CPU clusters

[Luc Michel]

This commit adds the cpu-cluster type. It aims at gathering CPUs from
the same cluster in a machine.

For now it only has a `cluster-id` property.

Signed-off-by: Luc Michel 
---
 include/hw/cpu/cluster.h | 38 +++
 hw/cpu/cluster.c | 49 
 hw/cpu/Makefile.objs |  2 +-
 3 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
new file mode 100644
index 00..f233a47a4a
--- /dev/null
+++ b/include/hw/cpu/cluster.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+#ifndef QEMU_HW_CPU_CLUSTER_H
+#define QEMU_HW_CPU_CLUSTER_H
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+#define TYPE_CPU_CLUSTER "cpu-cluster"
+#define CPU_CLUSTER(obj) \
+OBJECT_CHECK(CPUClusterState, (obj), TYPE_CPU_CLUSTER)
+
+typedef struct CPUClusterState {
+/*< private >*/
+DeviceState parent_obj;
+
+/*< public >*/
+uint32_t cluster_id;
+} CPUClusterState;
+
+#endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
new file mode 100644
index 00..11121e6f26
--- /dev/null
+++ b/hw/cpu/cluster.c
@@ -0,0 +1,49 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+
+#include "hw/cpu/cluster.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+
+static Property cpu_cluster_properties[] = {
+DEFINE_PROP_UINT32("cluster-id", CPUClusterState, cluster_id, 0),
+DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpu_cluster_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = cpu_cluster_properties;
+}
+
+static const TypeInfo cpu_cluster_type_info = {
+.name = TYPE_CPU_CLUSTER,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(CPUClusterState),
+.class_init = cpu_cluster_class_init,
+};
+
+static void cpu_cluster_register_types(void)
+{
+type_register_static(_cluster_type_info);
+}
+
+type_init(cpu_cluster_register_types)
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index cd52d20b65..8db9e8a7b3 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,5 +1,5 @@
 obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-common-obj-y += core.o
+common-obj-y += core.o cluster.o
-- 
2.19.1

[Qemu-devel] [PATCH v3 06/16] gdbstub: add multiprocess support to 'sC' packets

[Luc Michel]

Change the sC packet handling to support the multiprocess extension.
Instead of returning the first thread, we return the first thread of the
current process.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index d1ec245d1d..17fec8a41c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1543,13 +1543,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 type = strtoul(p, (char **), 16);
 sstep_flags = type;
 put_packet(s, "OK");
 break;
 } else if (strcmp(p,"C") == 0) {
-/* "Current thread" remains vague in the spec, so always return
- *  the first CPU (gdb returns the first thread). */
-put_packet(s, "QC1");
+/* "Current thread" remains vague in the spec, so always return the
+ * first thread of the current process (gdb returns the first
+ * thread).
+ */
+cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, 
s->g_cpu));
+snprintf(buf, sizeof(buf), "QC%s",
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
 s->query_cpu = first_cpu;
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
-- 
2.19.1

Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES

[Eduardo Habkost]

On Fri, Oct 26, 2018 at 11:01:25AM +0800, Robert Hoo wrote:
> On Wed, 2018-10-24 at 07:06 -0300, Eduardo Habkost wrote:
> > On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> > > Note RSBA is specially treated -- no matter host support it or not,
> > > qemu
> > > pretends it is supported.
> > > 
> > > Signed-off-by: Robert Hoo 
> > 
> > I am now wondering what else we need to be able to remove
> > CPUID_7_0_EDX_ARCH_CAPABILITIES from
> > feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
> > 
> > This series is necessary for that, be I think we still can't let
> > the VM be migrated if arch-capabilities is enabled and we're
> > running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
> > kvm_feature_msrs.
> > 
> > Reviewed-by: Eduardo Habkost 
> > 
> > > ---
> > >  target/i386/cpu.c | 31 ++-
> > >  target/i386/cpu.h |  8 
> > >  target/i386/kvm.c | 11 +++
> > >  3 files changed, 49 insertions(+), 1 deletion(-)
> > > 
> [...]
> > >  
> > >  typedef struct X86RegisterInfo32 {
> > > @@ -3696,7 +3717,15 @@ static uint32_t
> > > x86_cpu_get_supported_feature_word(FeatureWord w,
> > >  wi-
> > > >cpuid.reg);
> > >  break;
> > >  case MSR_FEATURE_WORD:
> > > -r = kvm_arch_get_supported_msr_feature(kvm_state, wi-
> > > >msr.index);
> > > +/* Special case:
> > > + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA
> > > [bit 2]
> > > + * is always supported in guest.
> > > + */
> > > +if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> > > +r = MSR_ARCH_CAP_RSBA;
> > > +}
> > > +r |= kvm_arch_get_supported_msr_feature(kvm_state,
> > > +wi->msr.index);
> > >  break;
> After I add the filtering out MSR feature, whose CPUID dependency fails
> , in x86_cpu_filter_features(), 1 issue comes out here: 
> 
> If running on an old platform that doesn't have ARCH_CAPABILITIES MSR,
> but we still pretends it here, then qemu will always print out
> "warning: host doesn't support requested feature: MSR(10AH).rsba [bit
> 2]", with -cpu 'host', which does not look comfortable.
> How about remove this hunk for now? leave it to when we fully decide
> how to handle ARCH_CAPABILITIES live-migration safely. 

I will remove that hunk in x86-next, thanks for noting!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Paolo Bonzini]

On 25/10/2018 10:52, Gerd Hoffmann wrote:
> We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
> 
> Reproducer:
> outw(0xff60, 0x220);
> outw(0x1020, 0x220);
> outw(0xffb0, 0x220);
> Result:
> Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])

I am dumb and I don't understand.  In set_ar_dr you get

v = 0xff
ar = 15
dr = 15

and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
is accessed.

The next accesses use SLOT->ksr which is 0 so it's fine too.

Paolo

Re: [Qemu-devel] [PATCH v7 04/20] target/mips: Add and integrate MXU decoding engine placeholder

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Provide the placeholder and add the invocation logic for MXU
> decoding engine.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 8 
>   1 file changed, 8 insertions(+)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index fefe9ac..128cabe 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,6 +23844,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
> +{
> +MIPS_INVAL("decode_opc_mxu");
> +generate_exception_end(ctx, EXCP_RI);
> +}
> +
>   static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>   {
>   int rs, rt, rd;
> @@ -26087,6 +26093,8 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>   case OPC_SPECIAL2:
>   if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) {
>   decode_tx79_mmi(env, ctx);
> +} else if (ctx->insn_flags & ASE_MXU) {
> +decode_opc_mxu(env, ctx);


Is the best way to implement this to include processing of MUL, CLZ, 
CLO, SDDP instructions into decode_opc_mxu as their encodings aren't 
overlaid by MXU instructions

considering MIPS SPECIAL2 instruction pool and MXU Instruction Set?


>   } else {
>   decode_opc_special2_legacy(env, ctx);
>   }

Re: [Qemu-devel] [PATCH 1/1] i386: Add PKU/OSPKE on Skylake-Server CPU model

[Tao Xu]

On 10/25/18 9:28 PM, Eduardo Habkost wrote:

On Wed, Oct 17, 2018 at 12:48:58PM +0200, Paolo Bonzini wrote:

On 17/10/2018 11:30, Tao Xu wrote:

As the release document ref below link (page 13):
https://software.intel.com/sites/default/files/managed/c5/15/\
architecture-instruction-set-extensions-programming-reference.pdf

PKU is supported in Skylake Server (Only Server) and later, and
on Intel(R) Xeon(R) Processor Scalable Family. OSPKE is to reads
the value of PKRU (Instruction of PKU) into EAX and clears EDX.
So PKU/OSPKE are supposed to be in Skylake-Server CPU model.
And PKU/OSPKE 's CPUID has been exposed to QEMU. But PKU/OSPKE
can't be find in Skylake-Server CPU model in the code.
So this patch will fix PKU/OSPKE this issue in Skylake-Server
CPU model.

OSPKE is not needed, since it is added automatically based on CR4 (and
is not set on boot).

Correct.


Also, the guru of CPU model compatibility is Eduardo, so I'll wait for
him to chime in anyway.

Sorry for taking so long to reply.  This can be safely done only
if every host that is able to run Skylake-Server today is
guaranteed to support PKU.  Is that the case?

You'll also need Skylake-Server-*-cpu.pku=off entries on
PC_COMPAT_3_0 to keep PKU disabled on pc-*-3.0 and older.


Thank you Eduardo,


But I can't find PC_COMPAT_3_0 in  include/hw/i386/pc.h. Will it exist on

QEMU 3.1 and will I add "pku=off" after QEMU 3.1  release?


Best Regard

Tao Xu

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[P J P]

  Hello Dan, all

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
| > While being at it deprecate cirrus too.
| > 
| > Reason (short version): use stdvga instead.
| > Verbose version:
| > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
| 
| 
| I don't debate the points in the blog post above that stdvga is a
| better choice, but I don't think that's enough to justify deprecating
| cirrus at this point in time, because when it then gets deleted it
| will break way too many existing deployments.
| 
| We need to socialize info in that blog post above more widely and
| especially ensure that apps are not using that by default. I don't
| see it being viable to formally deprecate it in QEMU any time soon
| though given existing usage.

To note, IMO there are other devices/sources in QEMU which are potential 
candidates for deprecation, similar to adlib etc. It'll help if we could 
device a process to deprecate/remove such code base. Other than maintenance it 
invariably also becomes source of security issues.

Ex.(similar to Fedora) we could announce such candidate on qemu-devel list and 
after review over a period of say a month, candidate will be 
deprecated/expunged. (thinking aloud)

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v1] arm: check bit index before usage

[Peter Maydell]

On 25 October 2018 at 21:31, P J P  wrote:
> +-- On Thu, 25 Oct 2018, Peter Maydell wrote --+
> | Hi; thanks for this patch. Looking at the SA1110 manual,
> | it says that writes to the reserved bits [31:28] are
> | ignored. So I think that rather than doing this check
> | here, we should do what the strongarm_ppc_* code in the
> | same file does -- mask off the high bits for writes to
> | the direction and state registers. Then it will not
> | be possible for high bits to be set here that cause an
> | out-of-range array access.
>
> ===
> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
> index ec2627374d..dd8c4b1f2e 100644
> --- a/hw/arm/strongarm.c
> +++ b/hw/arm/strongarm.c
> @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr
> offset,
>
>  switch (offset) {
>  case GPDR:/* GPIO Pin-Direction registers */
> -s->dir = value;
> +s->dir = value & 0x3f;
>  strongarm_gpio_handler_update(s);
>  break;
>
>  case GPSR:/* GPIO Pin-Output Set registers */
> -s->olevel |= value;
> +s->olevel |= value & 0x3f;
>  strongarm_gpio_handler_update(s);
>  break;
> ===
>
> does this seem okay?

Yes, that's what I had in mind.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 0/3] i.MX: Add the i.MX6UL SOC and a reference board.

[Thomas Huth]

On 2018-07-30 22:03, Jean-Christophe Dubois wrote:
> This series adds the i.MX6UL SOC from NXP/Freescale and the reference
> evaluation board.
> 
> This series was tested by booting linux 4.18 (built using imx_v6_v7_defconfig)
> on the emulated board (with the appropriate device tree).
> 
> Jean-Christophe Dubois (3):
>   i.MX6UL: Add i.MX6UL specific CCM device
>   i.MX6UL: Add i.MX6UL SOC
>   i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board
> 
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs|   1 +
>  hw/arm/fsl-imx6ul.c | 617 ++
>  hw/arm/mcimx6ul-evk.c   |  85 +++
>  hw/misc/Makefile.objs   |   1 +
>  hw/misc/imx6ul_ccm.c| 890 
>  hw/misc/trace-events|   7 +
>  include/hw/arm/fsl-imx6ul.h | 339 
>  include/hw/misc/imx6ul_ccm.h| 226 
>  9 files changed, 2167 insertions(+)
>  create mode 100644 hw/arm/fsl-imx6ul.c
>  create mode 100644 hw/arm/mcimx6ul-evk.c
>  create mode 100644 hw/misc/imx6ul_ccm.c
>  create mode 100644 include/hw/arm/fsl-imx6ul.h
>  create mode 100644 include/hw/misc/imx6ul_ccm.h

Can we please also get an entry to the MAINTAINERS file for this new
board / files?

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Richard Henderson]

On 10/25/18 7:03 PM, Maciej W. Rozycki wrote:
> Overall this source file is clearly a modified copy of an ancient version 
> of the opcode table included with the opcodes library from binutils and I 
> think it would benefit from a refresh.

You can't do that because of GPL v3, sadly.


r~

Re: [Qemu-devel] [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
| > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
| > | 
| > | Reproducer:
| > | outw(0xff60, 0x220);
| > | outw(0x1020, 0x220);
| > | outw(0xffb0, 0x220);
| > | Result:
| > | Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
| > 
| > + Reported-by: Wangjunqing 
| 
| So you have a CVE number for this ?

No, since the adlib device is not used as much and is being deprecated, I'm 
not inclined to get one.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH RFC 4/4] net/slirp: add ipv6-hostfwd option for user netdev type

[Thomas Huth]

On 2018-10-26 01:03, Maxim Samoylov wrote:
> This allows forwarding TCP6 and UDP6 connections down to
> netdev=user connected guests.
> 
> Signed-off-by: Maxim Samoylov 
> ---
>  hmp-commands.hx |  31 
>  include/net/slirp.h |   2 +
>  net/slirp.c | 214 
> 
>  qapi/net.json   |   3 +-
>  4 files changed, 249 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index db0c681..b0e1a08 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1448,6 +1448,37 @@ STEXI
>  Remove host-to-guest TCP or UDP redirection.
>  ETEXI
>  
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_add",
> +.args_type  = "arg1:s,arg2:s?,arg3:s?",
> +.params = "[hub_id name]|[netdev_id] 
> [tcp|udp]:[hostaddr6]:hostport-guestaddr6:guestport",
> +.help   = "redirect TCP6 or UDP6 connections from host to guest 
> (requires -net user)",
> +.cmd= hmp_ipv6_hostfwd_add,
> +},
> +#endif
> +STEXI
> +@item hostfwd_add
> +@findex hostfwd_add
> +Redirect TCP6 or UDP6 connections from host to guest (requires -net user).
> +ETEXI
> +
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_remove",
> +.args_type  = "arg1:s,arg2:s?,arg3:s?",
> +.params = "[hub_id name]|[netdev_id] 
> [tcp|udp]:[hostaddr6]:hostport",
> +.help   = "remove host-to-guest TCP6 or UDP6 redirection",
> +.cmd= hmp_ipv6_hostfwd_remove,
> +},

 Hi,

could you please remove the "[hub_id name]" touple here? I recently sent
a patch to deprecate it for the IPv4 version, too:

https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg03198.html

Also I think it would make sense if you mention in the help text that
IPv6 addresses have to be given with square brackets?

 Thanks,
  Thomas

[Qemu-devel] [Bug 1800088] [NEW] Assertion fail while usb camera redirect

[yueyihua]

Public bug reported:

This may happen during usb camera redirect. But if i move the camera
lens from left to right or up to down, this always happen. My qemu-
version is 2.10.0 and following is the error information:

2018-10-26T03:37:54.925231Z qemu-kvm: usbredirparser: error unexpected extra 
data ep 00
qemu-kvm: hw/usb/redirect.c:1313: usbredir_chardev_read: Assertion 
`dev->read_buf == ((void *)0)' failed.
2018-10-26 03:37:57.120+: shutting down, reason=crashed

** 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/1800088

Title:
  Assertion fail while usb camera redirect

Status in QEMU:
  New

Bug description:
  This may happen during usb camera redirect. But if i move the camera
  lens from left to right or up to down, this always happen. My qemu-
  version is 2.10.0 and following is the error information:

  2018-10-26T03:37:54.925231Z qemu-kvm: usbredirparser: error unexpected extra 
data ep 00
  qemu-kvm: hw/usb/redirect.c:1313: usbredir_chardev_read: Assertion 
`dev->read_buf == ((void *)0)' failed.
  2018-10-26 03:37:57.120+: shutting down, reason=crashed

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

[Qemu-devel] [PATCH v2] strongarm: mask off high[32:28] bits from dir and state registers

[P J P]

From: Prasad J Pandit 

The high[32:28] bits of 'direction' and 'state' registers of
SA-1100/SA-1110 device are reserved. Setting them may lead to
OOB 's->handler[]' array access issue. Mask off [32:28] bits to
avoid it.

Reported-by: Moguofang 
Signed-off-by: Prasad J Pandit 
---
 hw/arm/strongarm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Update v2: mask off high[32:28] bits
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05746.html

diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index ec2627374d..dd8c4b1f2e 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr 
offset,
 
 switch (offset) {
 case GPDR:/* GPIO Pin-Direction registers */
-s->dir = value;
+s->dir = value & 0x3f;
 strongarm_gpio_handler_update(s);
 break;
 
 case GPSR:/* GPIO Pin-Output Set registers */
-s->olevel |= value;
+s->olevel |= value & 0x3f;
 strongarm_gpio_handler_update(s);
 break;
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH v7 05/20] target/mips: Add MXU decoding engine

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add MXU decoding engine: add handlers for all instruction pools,
> and main decode handler. The handlers, for now, for the purpose
> of this patch, contain only sceleton in the form of a single
> switch statement.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 1143 
> ++-
>   1 file changed, 1141 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 128cabe..ed72b32 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,12 +23844,1151 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +/*
> + *
> + * Decode MXU pool00
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +---+-+-+---+---+---+---+
> + *
> + */
> +static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
> +{
> +uint32_t opcode = extract32(ctx->opcode, 18, 3);
> +
> +switch (opcode) {
> +case OPC_MXU_S32MAX:
> +/* TODO: Implement emulation of S32MAX instruction. */
> +MIPS_INVAL("OPC_MXU_S32MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_S32MIN:
> +/* TODO: Implement emulation of S32MIN instruction. */
> +MIPS_INVAL("OPC_MXU_S32MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16MAX:
> +/* TODO: Implement emulation of D16MAX instruction. */
> +MIPS_INVAL("OPC_MXU_D16MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16MIN:
> +/* TODO: Implement emulation of D16MIN instruction. */
> +MIPS_INVAL("OPC_MXU_D16MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8MAX:
> +/* TODO: Implement emulation of Q8MAX instruction. */
> +MIPS_INVAL("OPC_MXU_Q8MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8MIN:
> +/* TODO: Implement emulation of Q8MIN instruction. */
> +MIPS_INVAL("OPC_MXU_Q8MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8SLT:
> +/* TODO: Implement emulation of Q8SLT instruction. */
> +MIPS_INVAL("OPC_MXU_Q8SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8SLTU:
> +/* TODO: Implement emulation of Q8SLTU instruction. */
> +MIPS_INVAL("OPC_MXU_Q8SLTU");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +default:
> +MIPS_INVAL("decode_opc_mxu");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +}
> +}
> +
> +/*
> + *
> + * Decode MXU pool01
> + *
> + *  S32SLT, D16SLT, D16AVG, D16AVGR, Q8AVG, Q8AVGR:
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL01|
> + *  +---+-+-+---+---+---+---+
> + *
> + *  Q8MADD:


Q8ADD, instead of Q8MADD.

Otherwise:

Reviewed-by: Stefan Markovic 


> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |en2|0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL01|
> + *  +---+---+-+-+---+---+---+---+
> + *
> + */
> +static void decode_opc_mxu__pool01(CPUMIPSState *env, DisasContext *ctx)
> +{
> +uint32_t opcode = extract32(ctx->opcode, 18, 3);
> +
> +switch (opcode) {
> +case OPC_MXU_S32SLT:
> +/* TODO: Implement emulation of S32SLT instruction. */
> +MIPS_INVAL("OPC_MXU_S32SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16SLT:
> +/* TODO: Implement emulation of D16SLT instruction. */
> +MIPS_INVAL("OPC_MXU_D16SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16AVG:
> +/* TODO: Implement emulation of D16AVG instruction. */
> +MIPS_INVAL("OPC_MXU_D16AVG");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16AVGR:
> +/* TODO: Implement emulation of D16AVGR instruction. */
> +MIPS_INVAL("OPC_MXU_D16AVGR");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8AVG:
> +/* TODO: Implement emulation of Q8AVG instruction. */
> +MIPS_INVAL("OPC_MXU_Q8AVG");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +

Re: [Qemu-devel] [PATCH v7 08/20] target/mips: Add bit encoding for MXU execute add/sub pattern 'eptn2'

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add bit encoding for MXU execute 2-bit add/subtract pattern 'eptn2'.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 6 ++
>   1 file changed, 6 insertions(+)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 97fb2e0..665a584 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23855,6 +23855,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   #define MXU_APTN2_SA2
>   #define MXU_APTN2_SS3
>   
> +/* MXU execute add/subtract 2-bit pattern 'eptn2' */
> +#define MXU_EPTN2_AA0
> +#define MXU_EPTN2_AS1
> +#define MXU_EPTN2_SA2
> +#define MXU_EPTN2_SS3
> +
>   
>   /*
>*

Re: [Qemu-devel] [PATCH v7 06/20] target/mips: Add bit encoding for MXU accumulate add/sub 1-bit pattern 'aptn1'

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add bit encoding for MXU accumulate add/subtract 1-bit pattern
> 'aptn1'.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 6 ++
>   1 file changed, 6 insertions(+)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index ed72b32..f274ac1 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,6 +23844,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +
> +/* MXU accumulate add/subtract 1-bit pattern 'aptn1' */
> +#define MXU_APTN1_A0
> +#define MXU_APTN1_S1
> +
> +
>   /*
>*
>* Decode MXU pool00

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[P J P]

+-- On Thu, 25 Oct 2018, Ameya More wrote --+
| While Mark and I reported this issue to you, it was actually discovered by 
| Dejvau Security and they should receive credit for reporting this issue. 
| http://www.dejavusecurity.com

I see; Would it be possible to share email-id of the original reporter to 
include in the commit log message?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 1/3] arm: check bit index before use

[Paolo Bonzini]

On 22/10/2018 14:09, P J P wrote:
> From: Prasad J Pandit 
> 
> While performing gpio write via strongarm_gpio_handler_update
> routine, the 'bit' index could access beyond s->handler[28] array.
> Add check to avoid OOB access.
> 
> Reported-by: Moguofang 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/arm/strongarm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
> index ec2627374d..3dda75feaf 100644
> --- a/hw/arm/strongarm.c
> +++ b/hw/arm/strongarm.c
> @@ -532,7 +532,9 @@ static void 
> strongarm_gpio_handler_update(StrongARMGPIOInfo *s)
>  
>  for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
>  bit = ctz32(diff);
> -qemu_set_irq(s->handler[bit], (level >> bit) & 1);
> +if (bit < sizeof(s->handler) / sizeof(s->handler[0])) {
> +qemu_set_irq(s->handler[bit], (level >> bit) & 1);
> +}
>  }
>  
>  s->prev_level = level;
> 

This is correct, but please use ARRAY_SIZE(s->handler).

Paolo

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| > -int msg_len;
| > +uint8_t msg_len;
| 
| Not wrong per se, but it's also not clear why it's needed.  I understand
| that you want to switch from signed to unsigned, but it is not mentioned
| in the commit message.

Changed to uint8_t because IIUC 'msg_len' is likely be < LSI_MAX_MSGIN_LEN=8. 
And uint32_t seems rather large for it.
 
| The switch to 8-bit, and below from VMSTATE_INT32 to VMSTATE_UINT8, is
| wrong because it changes the format of the live migration stream.

I see.

| I'm not sure it's appropriate to check for out of bounds reads here,
| because if s->msg_len is greater than LSI_MAX_MSGIN_LEN, then this test
| doesn't exclude you've already had an out of bounds write before.
| Indeed the msg_len is checked in lsi_add_msg_byte in order to avoid out
| of bounds accesses in either lsi_add_msg_byte or lsi_do_msgin. You
| could assert here that the variable is in range, I guess.

Okay.

| However, the out of bounds s->msg_len can actually happen in one other
| case: namely, if a malicious live migration stream includes a bogus
| s->msg_len.  Such live migration stream should be rejected; the fix for
| that is to add a lsi_post_load function, point to it in
| vmstate_lsi_scsi, and check there for s->msg_len <= LSI_MAX_MSGIN_LEN.

Okay, sending a revised patch v1 in a bit.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| I am dumb and I don't understand.  In set_ar_dr you get
| 
|   v = 0xff
|   ar = 15
|   dr = 15
| 
| and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
| seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
| is accessed.
| 
| The next accesses use SLOT->ksr which is 0 so it's fine too.

In set_ar_dr

  SLOT->AR = ar ? >AR_TABLE[ar<<2] : RATE_0;

SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr is set to 
15, in CALC_FCSLOT()

  SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
>   Hello Dan, all
> 
> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> | > While being at it deprecate cirrus too.
> | > 
> | > Reason (short version): use stdvga instead.
> | > Verbose version:
> | > 
> https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> | 
> | 
> | I don't debate the points in the blog post above that stdvga is a
> | better choice, but I don't think that's enough to justify deprecating
> | cirrus at this point in time, because when it then gets deleted it
> | will break way too many existing deployments.
> | 
> | We need to socialize info in that blog post above more widely and
> | especially ensure that apps are not using that by default. I don't
> | see it being viable to formally deprecate it in QEMU any time soon
> | though given existing usage.
> 
> To note, IMO there are other devices/sources in QEMU which are potential 
> candidates for deprecation, similar to adlib etc. It'll help if we could 
> device a process to deprecate/remove such code base. Other than maintenance 
> it 
> invariably also becomes source of security issues.
> 
> Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> and 
> after review over a period of say a month, candidate will be
> deprecated/expunged. (thinking aloud)

QEMU has a deprecation process:

  https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features

Most of the stuff deprecated is CLI args / monitor commands, etc where
mgmt apps just adjust the way they are calling QEMU, so end user's VMs
are largely not impacted.

Deprecating a device type that is widely used is not desirable because
that will cause breakage of existing guests.  Distros are free to disable
devices in their builds if they want to reduce the scope for CVEs in
packages they maintain, but again they should think carefully about how
many users they are going to break by doing so.

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

Re: [Qemu-devel] [PATCH v7 13/20] target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Move MUL, S32M2I, S32I2M handling out of switch. These are all
> instructions that do not depend on MXU_EN flag of MXU_CR.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 41 +++--
>   1 file changed, 23 insertions(+), 18 deletions(-)


See my comment for patch 04/20.

CLZ, CLO, SDDP are missing?


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c8c71c4..111affb 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24859,6 +24859,29 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   {
>   uint32_t opcode = extract32(ctx->opcode, 0, 6);
>   
> +if (opcode == OPC__MXU_MUL) {
> +uint32_t  rs, rt, rd, op1;
> +
> +rs = extract32(ctx->opcode, 21, 5);
> +rt = extract32(ctx->opcode, 16, 5);
> +rd = extract32(ctx->opcode, 11, 5);
> +op1 = MASK_SPECIAL2(ctx->opcode);
> +
> +gen_arith(ctx, op1, rd, rs, rt);
> +
> +return;
> +}
> +
> +if (opcode == OPC_MXU_S32M2I) {
> +gen_mxu_s32m2i(ctx);
> +return;
> +}
> +
> +if (opcode == OPC_MXU_S32I2M) {
> +gen_mxu_s32i2m(ctx);
> +return;
> +}
> +
>   switch (opcode) {
>   case OPC_MXU_S32MADD:
>   /* TODO: Implement emulation of S32MADD instruction. */
> @@ -24870,18 +24893,6 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   MIPS_INVAL("OPC_MXU_S32MADDU");
>   generate_exception_end(ctx, EXCP_RI);
>   break;
> -case OPC__MXU_MUL: /* 0x2 - unused in MXU specs */
> -{
> -uint32_t  rs, rt, rd, op1;
> -
> -rs = extract32(ctx->opcode, 21, 5);
> -rt = extract32(ctx->opcode, 16, 5);
> -rd = extract32(ctx->opcode, 11, 5);
> -op1 = MASK_SPECIAL2(ctx->opcode);
> -
> -gen_arith(ctx, op1, rd, rs, rt);
> -}
> -break;
>   case OPC_MXU__POOL00:
>   decode_opc_mxu__pool00(env, ctx);
>   break;
> @@ -25033,12 +25044,6 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   MIPS_INVAL("OPC_MXU_S16SDI");
>   generate_exception_end(ctx, EXCP_RI);
>   break;
> -case OPC_MXU_S32M2I:
> -gen_mxu_s32m2i(ctx);
> -break;
> -case OPC_MXU_S32I2M:
> -gen_mxu_s32i2m(ctx);
> -break;
>   case OPC_MXU_D32SLL:
>   /* TODO: Implement emulation of D32SLL instruction. */
>   MIPS_INVAL("OPC_MXU_D32SLL");

Re: [Qemu-devel] [PATCH v7 19/20] target/mips: Move MXU_EN check one level higher

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Move MXU_EN check to the main MXU decoding function, to avoid code
> repetition.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 509 
> ++--
>   1 file changed, 238 insertions(+), 271 deletions(-)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 61c1662..3620ae5 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23960,23 +23960,16 @@ static void gen_mxu_s32m2i(DisasContext *ctx)
>   static void gen_mxu_s8ldd(DisasContext *ctx)
>   {
>   TCGv t0, t1;
> -TCGLabel *l0;
>   uint32_t XRa, Rb, s8, optn3;
>   
>   t0 = tcg_temp_new();
>   t1 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   s8 = extract32(ctx->opcode, 10, 8);
>   optn3 = extract32(ctx->opcode, 18, 3);
>   Rb = extract32(ctx->opcode, 21, 5);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_gpr(t0, Rb);
>   tcg_gen_addi_tl(t0, t0, (int8_t)s8);
>   
> @@ -24034,8 +24027,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx)
>   
>   gen_store_mxu_gpr(t0, XRa);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   }
> @@ -24046,7 +24037,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx)
>   static void gen_mxu_d16mul(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, optn2;
>   
>   t0 = tcg_temp_new();
> @@ -24054,18 +24044,12 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   t2 = tcg_temp_new();
>   t3 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
>   XRd = extract32(ctx->opcode, 18, 4);
>   optn2 = extract32(ctx->opcode, 22, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t1, XRb);
>   tcg_gen_sextract_tl(t0, t1, 0, 16);
>   tcg_gen_sextract_tl(t1, t1, 16, 16);
> @@ -24094,8 +24078,6 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   gen_store_mxu_gpr(t3, XRa);
>   gen_store_mxu_gpr(t2, XRd);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   tcg_temp_free(t2);
> @@ -24109,7 +24091,6 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   static void gen_mxu_d16mac(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, optn2, aptn2;
>   
>   t0 = tcg_temp_new();
> @@ -24117,8 +24098,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   t2 = tcg_temp_new();
>   t3 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
> @@ -24126,10 +24105,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   optn2 = extract32(ctx->opcode, 22, 2);
>   aptn2 = extract32(ctx->opcode, 24, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t1, XRb);
>   tcg_gen_sextract_tl(t0, t1, 0, 16);
>   tcg_gen_sextract_tl(t1, t1, 16, 16);
> @@ -24180,8 +24155,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   gen_store_mxu_gpr(t3, XRa);
>   gen_store_mxu_gpr(t2, XRd);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   tcg_temp_free(t2);
> @@ -24195,7 +24168,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3, t4, t5, t6, t7;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, sel;
>   
>   t0 = tcg_temp_new();
> @@ -24207,18 +24179,12 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   t6 = tcg_temp_new();
>   t7 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
>   XRd = extract32(ctx->opcode, 18, 4);
>   sel = extract32(ctx->opcode, 22, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t3, XRb);
>   gen_load_mxu_gpr(t7, XRc);
>   
> @@ -24269,8 +24235,6 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   gen_store_mxu_gpr(t0, XRd);
>   gen_store_mxu_gpr(t1, XRa);
>   
> -gen_set_label(l0);
> -

Re: [Qemu-devel] [PATCH v7 20/20] target/mips: Amend MXU ASE overview note

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add prefix, suffix, operation descriptions, and other corrections
> and amendments to the comment that describes MXU ASE.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 84 
> +++--
>   1 file changed, 74 insertions(+), 10 deletions(-)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3620ae5..9bd5f27 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1410,25 +1410,89 @@ enum {
>* MXU unit contains 17 registers called X0-X16. X0 is always zero, and X16 
> is
>* the control register.
>*
> - * The notation used in MXU assembler mnemonics:
> + * The notation used in MXU assembler mnemonics
> + * 
> + *
> + *  Registers:
>*
>*   XRa, XRb, XRc, XRd - MXU registers
>*   Rb, Rc, Rd, Rs, Rt - general purpose MIPS registers
> - *   s12- a subfield of an instruction code
> - *   strd2  - a subfield of an instruction code
> - *   eptn2  - a subfield of an instruction code
> - *   eptn3  - a subfield of an instruction code
> - *   optn2  - a subfield of an instruction code
> - *   optn3  - a subfield of an instruction code
> - *   sft4   - a subfield of an instruction code
> + *
> + *  Subfields:
> + *
> + *   aptn1  - 1-bit accumulate add/subtract pattern
> + *   aptn2  - 2-bit accumulate add/subtract pattern
> + *   eptn2  - 2-bit execute add/subtract pattern
> + *   optn2  - 2-bit operand pattern
> + *   optn3  - 3-bit operand pattern
> + *   sft4   - 4-bit shift amount
> + *   strd2  - 2-bit stride amount
> + *
> + *  Prefixes:
> + *
> + *   
> + * S 32
> + * D 16
> + * Q  8
> + *
> + *  Suffixes:
> + *
> + *   E - Expand results
> + *   F - Fixed point multiplication
> + *   L - Low part result
> + *   R - Doing rounding
> + *   V - Variable instead of immediate
> + *   W - Combine above L and V
> + *
> + *  Operations:
> + *
> + *   ADD   - Add or subtract
> + *   ADDC  - Add with carry-in
> + *   ACC   - Accumulate
> + *   ASUM  - Sum together then accumulate (add or subtract)
> + *   ASUMC - Sum together then accumulate (add or subtract) with carry-in
> + *   AVG   - Average between 2 operands
> + *   ABD   - Absolute difference
> + *   ALN   - Align data
> + *   AND   - Logical bitwise 'and' operation
> + *   CPS   - Copy sign
> + *   EXTR  - Extract bits
> + *   I2M   - Move from GPR register to MXU register
> + *   LDD   - Load data from memory to XRF
> + *   LDI   - Load data from memory to XRF (and increase the address base)
> + *   LUI   - Load unsigned immediate
> + *   MUL   - Multiply
> + *   MULU  - Unsigned multiply
> + *   MADD  - 64-bit operand add 32x32 product
> + *   MSUB  - 64-bit operand subtract 32x32 product
> + *   MAC   - Multiply and accumulate (add or subtract)
> + *   MAD   - Multiply and add or subtract
> + *   MAX   - Maximum between 2 operands
> + *   MIN   - Minimum between 2 operands
> + *   M2I   - Move from MXU register to GPR register
> + *   MOVZ  - Move if zero
> + *   MOVN  - Move if non-zero
> + *   NOR   - Logical bitwise 'nor' operation
> + *   OR- Logical bitwise 'or' operation
> + *   STD   - Store data from XRF to memory
> + *   SDI   - Store data from XRF to memory (and increase the address base)
> + *   SLT   - Set of less than comparison
> + *   SAD   - Sum of absolute differences
> + *   SLL   - Logical shift left
> + *   SLR   - Logical shift right
> + *   SAR   - Arithmetic shift right
> + *   SAT   - Saturation
> + *   SFL   - Shuffle
> + *   SCOP  - Calculate x’s scope (-1, means x<0; 0, means x==0; 1, means x>0)
> + *   XOR   - Logical bitwise 'exclusive or' operation
>*
>* Load/Store instructions   Multiplication instructions
>* ---   ---
>*
>*  S32LDD XRa, Rb, s12   S32MADD XRa, XRd, Rs, Rt
>*  S32STD XRa, Rb, s12   S32MADDU XRa, XRd, Rs, Rt
> - *  S32LDDV XRa, Rb, rc, strd2S32SUB XRa, XRd, Rs, Rt
> - *  S32STDV XRa, Rb, rc, strd2S32SUBU XRa, XRd, Rs, Rt
> + *  S32LDDV XRa, Rb, rc, strd2S32MSUB XRa, XRd, Rs, Rt
> + *  S32STDV XRa, Rb, rc, strd2S32MSUBU XRa, XRd, Rs, Rt
>*  S32LDI XRa, Rb, s12   S32MUL XRa, XRd, Rs, Rt
>*  S32SDI XRa, Rb, s12   S32MULU XRa, XRd, Rs, Rt
>*  S32LDIV XRa, Rb, rc, strd2D16MUL XRa, XRb, XRc, XRd, optn2

[Qemu-devel] [PATCH 0/2] Deprecate the "collie" machine and Strongarm devices

[Thomas Huth]

These files lack an entry in the MAINTAINERS file, and according to
the initial commits, the board and devices are incomplete. Since there
have hardly been any commits in the past to really improve them, we
should consider to mark them as deprecated now.

Thomas Huth (2):
  hw/arm: Deprecate the "collie" board
  arm: Deprecate the Strongarm sa1100 and sa1110 processors

 hw/arm/collie.c  |  1 +
 qemu-deprecated.texi | 10 ++
 2 files changed, 11 insertions(+)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] hw/arm: Deprecate the "collie" board

[Thomas Huth]

"collie" has no entry in the MAINTAINERS file, and the initial commit
with ID c64b21d519a6ecae12f65625fa60f3035ed88644 said:

"Add very basic implementation of collie PDA emulation. The system lacks
 LoCoMo and graphics/sound emulation. Linux kernel boots up to mounting
 rootfs (theoretically it can be provided in pflash images)."

Thus the board is incomplete, since after that initial commit, there were
only the usual QEMU API-related rework patches applied to this file.
Unless someone speaks up and says that this board is still useful, we
should consider to remove it, so mark it as deprecated now.

Signed-off-by: Thomas Huth 
---
 hw/arm/collie.c  | 1 +
 qemu-deprecated.texi | 5 +
 2 files changed, 6 insertions(+)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 48b732c..e41bbc5 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -60,6 +60,7 @@ static void collie_machine_init(MachineClass *mc)
 mc->init = collie_init;
 mc->ignore_memory_transaction_failures = true;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("sa1110");
+mc->deprecation_reason = "board is incomplete and unmaintained";
 }
 
 DEFINE_MACHINE("collie", collie_machine_init)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 11b870c..acf9809 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -129,6 +129,11 @@ This machine type uses an unmaintained firmware, broken in 
lots of ways,
 and unable to start post-2004 operating systems. 40p machine type should be
 used instead.
 
+@subsection collie (ARM) (since 3.1)
+
+The board is incomplete and unmaintained. Use a different ARM board if
+possible instead.
+
 @section Device options
 
 @subsection Block device options
-- 
1.8.3.1

[Qemu-devel] [PATCH v1] xen: preserve COMPAT in CFLAGS

[Olaf Hering]

A given Qemu version can not predict what version of Xen it will run on.
There are some checks in configure to decide what Xen libraries and
functions are available. How exactly these functions must be accessed
has to be decided by configure and the user who is compiling Qemu.
In no way some random header file must override this decision.

Remove the breakage introduced by commit 5eeb39c24b, which would always
hide the libxc interfaces the given version of Qemu knows about.

The current symptom of such breakage is a build failure with qemu-2.9
and older, in combination with Xen 4.12.

Fixes: 5eeb39c24b7d4da5d129bfdd9c4fd21cfb3d28d6
Signed-off-by: Olaf Hering 
---
 include/hw/xen/xen_common.h | 9 -
 1 file changed, 9 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 5f1402b494..33fa2d3497 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -1,15 +1,6 @@
 #ifndef QEMU_HW_XEN_COMMON_H
 #define QEMU_HW_XEN_COMMON_H
 
-/*
- * If we have new enough libxenctrl then we do not want/need these compat
- * interfaces, despite what the user supplied cflags might say. They
- * must be undefined before including xenctrl.h
- */
-#undef XC_WANT_COMPAT_EVTCHN_API
-#undef XC_WANT_COMPAT_GNTTAB_API
-#undef XC_WANT_COMPAT_MAP_FOREIGN_API
-
 #include 
 #include 
 #include 

Re: [Qemu-devel] [PATCH 0/2] Deprecate the "collie" machine and Strongarm devices

[Peter Maydell]

On 26 October 2018 at 11:06, Thomas Huth  wrote:
> These files lack an entry in the MAINTAINERS file, and according to
> the initial commits, the board and devices are incomplete. Since there
> have hardly been any commits in the past to really improve them, we
> should consider to mark them as deprecated now.
>
> Thomas Huth (2):
>   hw/arm: Deprecate the "collie" board
>   arm: Deprecate the Strongarm sa1100 and sa1110 processors

Hi Guenter; there's a proposal here to deprecate (and eventually
remove) the 'collie' board (strongarm) from QEMU. Is that one of
the ones you're currently using in your automated testing of Linux
kernels on QEMU?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] nvme: check size before memcpy

[Paolo Bonzini]

On 22/10/2018 14:14, P J P wrote:
> From: Prasad J Pandit 
> 
> While in nvme_mmio_read, memcpy could read past the 'n->bar'
> buffer, if addr offset was pointing towards its tail end.
> Add check to avoid OOB access.
> 
> Reported-by: Caihongzhu 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index fc7dacb816..87afc19b61 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1059,7 +1059,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr 
> addr, unsigned size)
>  /* should RAZ, fall through for now */
>  }
>  
> -if (addr < sizeof(n->bar)) {
> +if (addr + size <= sizeof(n->bar)) {
>  memcpy(, ptr + addr, size);
>  } else {
>  NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
> 

Do you have a reproducer?  In particular, I think this cannot happen
because memory_region_dispatch_read will block accesses beyond 4 bytes,
and earlier code in this function already check that accesses are
aligned to 32 bits.

We could clarify it with

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb816..427e69a78d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1167,7 +1167,7 @@ static const MemoryRegionOps nvme_mmio_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 .impl = {
 .min_access_size = 2,
-.max_access_size = 8,
+.max_access_size = 4,
 },
 };


but if my understanding is right then there is no bug.

Paolo

Re: [Qemu-devel] [PATCH v1] arm: check bit index before usage

[P J P]

+-- On Fri, 26 Oct 2018, Peter Maydell wrote --+
| > ===
| > diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
| > index ec2627374d..dd8c4b1f2e 100644
| > --- a/hw/arm/strongarm.c
| > +++ b/hw/arm/strongarm.c
| > @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr
| > offset,
| >
| >  switch (offset) {
| >  case GPDR:/* GPIO Pin-Direction registers */
| > -s->dir = value;
| > +s->dir = value & 0x3f;
| >  strongarm_gpio_handler_update(s);
| >  break;
| >
| >  case GPSR:/* GPIO Pin-Output Set registers */
| > -s->olevel |= value;
| > +s->olevel |= value & 0x3f;
| >  strongarm_gpio_handler_update(s);
| >  break;
| > ===
| >
| > does this seem okay?
| 
| Yes, that's what I had in mind.

Cool, patch sent.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: check size before data buffer access

[P J P]

+-- On Fri, 26 Oct 2018, Cédric Le Goater wrote --+
| On 10/25/18 8:45 AM, P J P wrote:
| >  - While we refactor the routine for better, a patch below seem okay to fix 
| >the OOB access issue?
| 
| I think it is fine. Please add something like :  
| 
|   qemu_log_mask(LOG_GUEST_ERROR, "ECCB: invalid operation at @0x%08x" 
|   " size %d\n", opb_addr, sz);

Okay, will do.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 09:48:35AM +0100, Cole Robinson wrote:
> On 10/25/2018 09:37 PM, Daniel P. Berrangé wrote:
> > On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > > While being at it deprecate cirrus too.
> > > 
> > > Reason (short version): use stdvga instead.
> > > Verbose version:
> > >  
> > > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > 
> > Every single one of my guests is using cirrus. This wasn't an explicit
> > choice on my part, so I believe it is being used as the default in
> > virt-install.
> > 
> 
> virt-manager/virt-install 2.0.0 will never explicitly set cirrus, but that
> release is quite new. The previous release's default for x86 was roughly:
> 
> if using spice graphics:
> use qxl
> elif guest os is windows:
> use vga
> else:
> use cirrus
> 
> It was definitely sub optimal. Maybe your virt-install commands were
> explicitly setting --graphics vnc which would trigger cirrus in that case.

Yep, I've always tended to use vnc, since QXL has historically been a
pretty buggy & unreliable device model with guests often breaking.

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

Re: [Qemu-devel] [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 12:38:53PM +0530, P J P wrote:
> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> | On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
> | > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
> | > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
> | > | 
> | > | Reproducer:
> | > | outw(0xff60, 0x220);
> | > | outw(0x1020, 0x220);
> | > | outw(0xffb0, 0x220);
> | > | Result:
> | > | Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
> | > 
> | > + Reported-by: Wangjunqing 
> | 
> | So you have a CVE number for this ?
> 
> No, since the adlib device is not used as much and is being deprecated, I'm 
> not inclined to get one.

Any security issue that affects code in QEMU that is currently being
shipped by distros should have a CVE.

Whether we intend to deprecate & delete it later should not be a factor
because we are free to cancel the deprecation process at any time if we
find a reason to keep the feature around.

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

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 10:42:08AM +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
> >   Hello Dan, all
> > 
> > +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> > | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > | > While being at it deprecate cirrus too.
> > | > 
> > | > Reason (short version): use stdvga instead.
> > | > Verbose version:
> > | > 
> > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > | 
> > | 
> > | I don't debate the points in the blog post above that stdvga is a
> > | better choice, but I don't think that's enough to justify deprecating
> > | cirrus at this point in time, because when it then gets deleted it
> > | will break way too many existing deployments.
> > | 
> > | We need to socialize info in that blog post above more widely and
> > | especially ensure that apps are not using that by default. I don't
> > | see it being viable to formally deprecate it in QEMU any time soon
> > | though given existing usage.
> > 
> > To note, IMO there are other devices/sources in QEMU which are potential 
> > candidates for deprecation, similar to adlib etc. It'll help if we could 
> > device a process to deprecate/remove such code base. Other than maintenance 
> > it 
> > invariably also becomes source of security issues.
> > 
> > Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> > and 
> > after review over a period of say a month, candidate will be
> > deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I should also say that QEMU as an upstream project has multiple goals.
Running KVM guests with modern PV hardware is only one of them, albeit
a widely used one. Being able to run old legacy OS with old hardware,
and running arbitrary embedded boards/devices with emulation are both
use cases that QEMU project aims to address. To eliminate all the old
"crufty" device emulation in name of improving security for KVM, would
be to eliminate core use cases of the project. THis is why we're trying
to persue the direction of making it easier for vendors to disable
features and devices they don't wish to support & thus limit their
downstream CVE exposure.

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

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Paolo Bonzini]

On 26/10/2018 11:34, P J P wrote:
> +-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
> | I am dumb and I don't understand.  In set_ar_dr you get
> | 
> | v = 0xff
> | ar = 15
> | dr = 15
> | 
> | and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
> | seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
> | is accessed.
> | 
> | The next accesses use SLOT->ksr which is 0 so it's fine too.
> 
> In set_ar_dr
> 
>   SLOT->AR = ar ? >AR_TABLE[ar<<2] : RATE_0;
> 
> SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr is set 
> to 
> 15, in CALC_FCSLOT()
> 
>   SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Oh, thanks!  I said I was dumb. :)  So the fix is just this:

diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h
index e7e578a48e..7199afaa3c 100644
--- a/hw/audio/fmopl.h
+++ b/hw/audio/fmopl.h
@@ -72,8 +72,8 @@ typedef struct fm_opl_f {
/* Rhythm sention */
uint8_t rhythm; /* Rhythm mode , key flag */
/* time tables */
-   int32_t AR_TABLE[75];   /* atttack rate tables */
-   int32_t DR_TABLE[75];   /* decay rate tables   */
+   int32_t AR_TABLE[76];   /* atttack rate tables */
+   int32_t DR_TABLE[76];   /* decay rate tables   */
uint32_t FN_TABLE[1024];  /* fnumber -> increment counter */
/* LFO */
int32_t *ams_table;

and init_timetables will just fill it with the right value?  (I checked
against another implementation at http://opl3.cozendey.com/).

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Paolo Bonzini]

On 26/10/2018 11:59, Daniel P. Berrangé wrote:
> I should also say that QEMU as an upstream project has multiple goals.
> Running KVM guests with modern PV hardware is only one of them, albeit
> a widely used one. Being able to run old legacy OS with old hardware,
> and running arbitrary embedded boards/devices with emulation are both
> use cases that QEMU project aims to address. To eliminate all the old
> "crufty" device emulation in name of improving security for KVM, would
> be to eliminate core use cases of the project. THis is why we're trying
> to persue the direction of making it easier for vendors to disable
> features and devices they don't wish to support & thus limit their
> downstream CVE exposure.

Indeed.  If we had to deprecate a feature just because it had an
off-by-one bug, no C program would grow beyond 1000 lines of code...

Paolo

[Qemu-devel] [PATCH 2/2] arm: Deprecate the Strongarm sa1100 and sa1110 processors

[Thomas Huth]

The deprecated "collie" board is the only user of the Strongarm
devices, so if "collie" goes away, we should remove the Strongarm
devices, too.

Signed-off-by: Thomas Huth 
---
 qemu-deprecated.texi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index acf9809..0de5c7f 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -116,6 +116,11 @@ The @option{[hub_id name]} parameter tuple of the 
'hostfwd_add' and
 The ``ivshmem'' device type is replaced by either the ``ivshmem-plain''
 or ``ivshmem-doorbell`` device types.
 
+@subsection Strongarm sa1100 and sa1110 and related devices (since 3.1)
+
+Without the deprecated "collie" board there is no other machine which is
+able to use these devices.
+
 @section System emulator machines
 
 @subsection pc-0.10 and pc-0.11 (since 3.0)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
> >   Hello Dan, all
> > 
> > +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> > | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > | > While being at it deprecate cirrus too.
> > | > 
> > | > Reason (short version): use stdvga instead.
> > | > Verbose version:
> > | > 
> > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > | 
> > | 
> > | I don't debate the points in the blog post above that stdvga is a
> > | better choice, but I don't think that's enough to justify deprecating
> > | cirrus at this point in time, because when it then gets deleted it
> > | will break way too many existing deployments.
> > | 
> > | We need to socialize info in that blog post above more widely and
> > | especially ensure that apps are not using that by default. I don't
> > | see it being viable to formally deprecate it in QEMU any time soon
> > | though given existing usage.
> > 
> > To note, IMO there are other devices/sources in QEMU which are potential 
> > candidates for deprecation, similar to adlib etc. It'll help if we could 
> > device a process to deprecate/remove such code base. Other than maintenance 
> > it 
> > invariably also becomes source of security issues.
> > 
> > Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> > and 
> > after review over a period of say a month, candidate will be
> > deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I'm a bit mixed here;  until recent guest kernels I've always treated
Cirrus as the 'safe' one that's likely to work on anything - so losing
it worries me.  Having said that, I understand why we want to deprecate
it;  but we should give a much longer deprecation warning - a few years?
I don't see any harm warning that it's deprecated and you really should
try not to use it.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK