Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

2018-11-09 Thread Eduardo Habkost
On Fri, Nov 09, 2018 at 10:07:07AM -0500, Cleber Rosa wrote:
> Some functionality is dependent on the Python version
> detected/configured on configure.  While it's possible to run the
> Python version later and check for the version, doing it once is
> preferable.  Also, it's a relevant information to keep in build logs,
> as the overall behavior of the build can be affected by it.
> 
> Signed-off-by: Cleber Rosa 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



Re: [Qemu-devel] [RFC QEMU 1/2] arm/virt: Initialize generic timer scale factor dynamically

2018-11-09 Thread Bijan Mottahedeh

On 11/8/2018 6:21 AM, Richard Henderson wrote:

On 11/7/18 7:48 PM, Bijan Mottahedeh wrote:
  
+static void set_system_clock_scale(void)

+{
+unsigned long cntfrq_el0;
+
+asm volatile("mrs %0, cntfrq_el0" : "=r"(cntfrq_el0));
+
+if (cntfrq_el0 == 0) {
+cntfrq_el0 = GTIMER_SCALE_DEF;
+}
+
+system_clock_scale = NANOSECONDS_PER_SECOND / (int)cntfrq_el0;
+}

This only works for kvm.

For TCG you need to use the default always.  In particular, it won't even
compile for an x86 host.


r~
Is it ok to ifdef the asm statement with CONFIG_KVM, or what would be 
the correct way to account for TCG vs. KVM?


Thanks.

--bijan



[Qemu-devel] [PATCH 0/2] refactor fw_cfg_bootsplash() and fw_cfg_reboot()

2018-11-09 Thread Li Qiang
This patchset comes out as the result of the following review as per 
Markus's and Gerd's advice:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg06975.html
-->http://lists.gnu.org/archive/html/qemu-devel/2018-11/msg00701.html

The second patch also fix that the user can set a negative
reboot_timeout.

Li Qiang (2):
  hw: fw_cfg: refactor fw_cfg_bootsplash()
  hw: fw_cfg: refactor fw_cfg_reboot()

 hw/nvram/fw_cfg.c | 63 ---
 vl.c  |  4 +--
 2 files changed, 29 insertions(+), 38 deletions(-)

-- 
2.17.1





[Qemu-devel] [PATCH 2/2] hw: fw_cfg: refactor fw_cfg_reboot()

2018-11-09 Thread Li Qiang
Currently the user can set a negative reboot_timeout.
Also it is wrong to parse 'reboot-timeout' with qemu_opt_get() and then
convert it to number. This patch refactor this function by following:
1. ensure reboot_timeout is in 0~0x
2. use qemu_opt_get_number() to parse reboot_timeout
3. simlify code

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 23 +++
 vl.c  |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 78f43dad93..6aca80846a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -178,24 +178,23 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 
 static void fw_cfg_reboot(FWCfgState *s)
 {
-int reboot_timeout = -1;
-char *p;
-const char *temp;
+const char *reboot_timeout = NULL;
 
 /* get user configuration */
 QemuOptsList *plist = qemu_find_opts("boot-opts");
 QemuOpts *opts = QTAILQ_FIRST(>head);
-if (opts != NULL) {
-temp = qemu_opt_get(opts, "reboot-timeout");
-if (temp != NULL) {
-p = (char *)temp;
-reboot_timeout = strtol(p, , 10);
-}
+reboot_timeout = qemu_opt_get(opts, "reboot-timeout");
+
+if (reboot_timeout == NULL) {
+return;
 }
+int64_t rt_val = qemu_opt_get_number(opts, "reboot-timeout", -1);
+
 /* validate the input */
-if (reboot_timeout > 0x) {
-error_report("reboot timeout is larger than 65535, force it to 
65535.");
-reboot_timeout = 0x;
+if (rt_val < 0 || rt_val > 0x) {
+error_report("reboot timeout is invalid,"
+ "it should be a value between 0 and 65535");
+exit(1);
 }
 fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(_timeout, 4), 4);
 }
diff --git a/vl.c b/vl.c
index be37da46f0..086127ff0b 100644
--- a/vl.c
+++ b/vl.c
@@ -339,7 +339,7 @@ static QemuOptsList qemu_boot_opts = {
 .type = QEMU_OPT_NUMBER,
 }, {
 .name = "reboot-timeout",
-.type = QEMU_OPT_STRING,
+.type = QEMU_OPT_NUMBER,
 }, {
 .name = "strict",
 .type = QEMU_OPT_BOOL,
-- 
2.17.1





[Qemu-devel] [PATCH 1/2] hw: fw_cfg: refactor fw_cfg_bootsplash()

2018-11-09 Thread Li Qiang
Currently when the splash-time value is bigger than 0x
we report and correct it, when it is less than 0 we just ingore it.
Also we use qemu_opt_get() to get 'splash-time', then convert it to a number
ourselves. This is wrong. This patch does following:
1. use qemu_opt_get_number() to parse 'splash-time'
2. exit when the splash-time is invalid or loading the splash file failed
3. simplify code

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 40 
 vl.c  |  2 +-
 2 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 946f765f7f..78f43dad93 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -118,55 +118,47 @@ error:
 
 static void fw_cfg_bootsplash(FWCfgState *s)
 {
-int boot_splash_time = -1;
 const char *boot_splash_filename = NULL;
-char *p;
+const char *boot_splash_time = NULL;
 char *filename, *file_data;
 gsize file_size;
 int file_type;
-const char *temp;
 
 /* get user configuration */
 QemuOptsList *plist = qemu_find_opts("boot-opts");
 QemuOpts *opts = QTAILQ_FIRST(>head);
-if (opts != NULL) {
-temp = qemu_opt_get(opts, "splash");
-if (temp != NULL) {
-boot_splash_filename = temp;
-}
-temp = qemu_opt_get(opts, "splash-time");
-if (temp != NULL) {
-p = (char *)temp;
-boot_splash_time = strtol(p, , 10);
-}
-}
+boot_splash_filename = qemu_opt_get(opts, "splash");
+boot_splash_time = qemu_opt_get(opts, "splash-time");
 
 /* insert splash time if user configurated */
-if (boot_splash_time >= 0) {
+if (boot_splash_time) {
+int64_t bst_val = qemu_opt_get_number(opts, "splash-time", -1);
 /* validate the input */
-if (boot_splash_time > 0x) {
-error_report("splash time is big than 65535, force it to 65535.");
-boot_splash_time = 0x;
+if (bst_val < 0 || bst_val > 0x) {
+error_report("splash time is invalid,"
+ "it should be a value between 0 and 65535");
+exit(1);
 }
 /* use little endian format */
-qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
-qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
+qemu_extra_params_fw[0] = (uint8_t)(bst_val & 0xff);
+qemu_extra_params_fw[1] = (uint8_t)((bst_val >> 8) & 0xff);
 fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
 }
 
 /* insert splash file if user configurated */
-if (boot_splash_filename != NULL) {
+if (boot_splash_filename) {
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, boot_splash_filename);
 if (filename == NULL) {
-error_report("failed to find file '%s'.", boot_splash_filename);
-return;
+error_report("failed to find file '%s'", boot_splash_filename);
+exit(1);
 }
 
 /* loading file data */
 file_data = read_splashfile(filename, _size, _type);
 if (file_data == NULL) {
 g_free(filename);
-return;
+error_report("failed to read file '%s'", boot_splash_filename);
+exit(1);
 }
 g_free(boot_splash_filedata);
 boot_splash_filedata = (uint8_t *)file_data;
diff --git a/vl.c b/vl.c
index 55bab005b6..be37da46f0 100644
--- a/vl.c
+++ b/vl.c
@@ -336,7 +336,7 @@ static QemuOptsList qemu_boot_opts = {
 .type = QEMU_OPT_STRING,
 }, {
 .name = "splash-time",
-.type = QEMU_OPT_STRING,
+.type = QEMU_OPT_NUMBER,
 }, {
 .name = "reboot-timeout",
 .type = QEMU_OPT_STRING,
-- 
2.17.1





Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd

2018-11-09 Thread H. Peter Anvin
On 11/9/18 5:40 AM, Li Zhijian wrote:
> Just noticed that there is a field xloadflags at recent protocol
>   60 Protocol 2.12:  (Kernel 3.8) Added the xloadflags field and
> extension fields
>   61 to struct boot_params for loading bzImage and ramdisk
>   62 above 4G in 64bit.
> [snip]
>  617 Field name: xloadflags
>  618 Type:   read
>  619 Offset/size:    0x236/2
>  620 Protocol:   2.12+
>  621
>  622   This field is a bitmask.
>  623
>  624   Bit 0 (read): XLF_KERNEL_64
>  625 - If 1, this kernel has the legacy 64-bit entry point at
> 0x200.
>  626
>  627   Bit 1 (read): XLF_CAN_BE_LOADED_ABOVE_4G
>  628 - If 1, kernel/boot_params/cmdline/ramdisk can be above 4G.
>  629
> 
> maybe we can reuse this field and append a new Bit 5
> XLF_INITRD_MAX_SIZE_4G and increase header version.
> For the old protocol version 2.12+, if  XLF_CAN_BE_LOADED_ABOVE_4G is
> set, we can also realize ~4GB initrd is allowed.
> 
> bootloader side:
> if protocol >= 2.15
>    if XLF_INITRD_LOAD_BELOW_4G
>   support ~4G initrd
>    fi
> else if protocol >=2.12
>    if XLF_CAN_BE_LOADED_ABOVE_4G
>     support ~4G initrd
>    fi
> fi
> 

The two are equivalent.  Obviously you have to load above 4 GB if you
have more than 4 GB of initrd.  If XLF_CAN_BE_LOADED_ABOVE_4G is not
set, then you most likely are on a 32-bit kernel and there are more
fundamental limits (even if you were to load it above the 2 GB mark, you
would be limited by the size of kernel memory.)

So, in case you are wondering: the bootloader that broke when setting
the initrd_max field above 2 GB was, of course, Grub.

So just use XLF_CAN_BE_LOADED_ABOVE_4G. There is no need for a new flag
or new field.

Also note that the ext_ramdisk_image and ext_ramdisk_size are part of
struct boot_params as opposed to struct setup_header, which means that
they are not supported when entering via the 16-bit BIOS entry point,
and I am willing to bet that there will be, ahem, "strangeness" if
entered via the 32-bit entry point if at least the command line is
loaded above the 4 GB mark; the initrd should be fine, though.

This is obviosly not an issue in EFI environments, where we enter
through the EFI handover entry point.

The main reason these were not added to struct setup_header is that
there are only 24 bytes left in that header and so space is highly
precious. One way to deal with that if we really, really need to would
be to add an initrd/initramfs type of setup_data.

-hpa



Re: [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements

2018-11-09 Thread Eduardo Habkost
On Fri, Nov 09, 2018 at 10:07:10AM -0500, Cleber Rosa wrote:
> The "check" target is not a target that will run all other tests
> listed, so in order to be accurate it's necessary to list those that
> will run.  The same is true for "check-clean".
> 
> Then, to give a better visual impression of the differences in the
> various targets, let's add empty lines.
> 
> Finally, a small (and hopeful) grammar fix from a non-native speaker.
> 
> Signed-off-by: Cleber Rosa 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.

2018-11-09 Thread Paolo Bonzini
On 09/11/2018 15:14, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
> 
> Signed-off-by: Gerd Hoffmann 

No objection, but can we make some effort of, at least, putting the
stack backtrace in the commit log?

Paolo


> ---
>  include/hw/bt.h |   3 -
>  hw/bt/hid.c | 554 
> 
>  vl.c|  34 +---
>  hw/bt/Makefile.objs |   3 +-
>  qemu-options.hx |   9 -
>  5 files changed, 2 insertions(+), 601 deletions(-)
>  delete mode 100644 hw/bt/hid.c
> 
> diff --git a/include/hw/bt.h b/include/hw/bt.h
> index b5e11d4d43..73fb1911f6 100644
> --- a/include/hw/bt.h
> +++ b/include/hw/bt.h
> @@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
>  /* bt-sdp.c */
>  void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
>  
> -/* bt-hid.c */
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
> -
>  /* Link Management Protocol layer defines */
>  
>  #define LLID_ACLU_CONT   0x1
> diff --git a/hw/bt/hid.c b/hw/bt/hid.c
> deleted file mode 100644
> index 056291f9b5..00
> --- a/hw/bt/hid.c
> +++ /dev/null
> @@ -1,554 +0,0 @@
> -/*
> - * QEMU Bluetooth HID Profile wrapper for USB HID.
> - *
> - * Copyright (C) 2007-2008 OpenMoko, Inc.
> - * Written by Andrzej Zaborowski 
> - *
> - * 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 or
> - * (at your option) version 3 of the License.
> - *
> - * 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, if not, see .
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "qemu/timer.h"
> -#include "ui/console.h"
> -#include "hw/input/hid.h"
> -#include "hw/bt.h"
> -
> -enum hid_transaction_req {
> -BT_HANDSHAKE = 0x0,
> -BT_HID_CONTROL   = 0x1,
> -BT_GET_REPORT= 0x4,
> -BT_SET_REPORT= 0x5,
> -BT_GET_PROTOCOL  = 0x6,
> -BT_SET_PROTOCOL  = 0x7,
> -BT_GET_IDLE  = 0x8,
> -BT_SET_IDLE  = 0x9,
> -BT_DATA  = 0xa,
> -BT_DATC  = 0xb,
> -};
> -
> -enum hid_transaction_handshake {
> -BT_HS_SUCCESSFUL = 0x0,
> -BT_HS_NOT_READY  = 0x1,
> -BT_HS_ERR_INVALID_REPORT_ID  = 0x2,
> -BT_HS_ERR_UNSUPPORTED_REQUEST= 0x3,
> -BT_HS_ERR_INVALID_PARAMETER  = 0x4,
> -BT_HS_ERR_UNKNOWN= 0xe,
> -BT_HS_ERR_FATAL  = 0xf,
> -};
> -
> -enum hid_transaction_control {
> -BT_HC_NOP= 0x0,
> -BT_HC_HARD_RESET = 0x1,
> -BT_HC_SOFT_RESET = 0x2,
> -BT_HC_SUSPEND= 0x3,
> -BT_HC_EXIT_SUSPEND   = 0x4,
> -BT_HC_VIRTUAL_CABLE_UNPLUG   = 0x5,
> -};
> -
> -enum hid_protocol {
> -BT_HID_PROTO_BOOT= 0,
> -BT_HID_PROTO_REPORT  = 1,
> -};
> -
> -enum hid_boot_reportid {
> -BT_HID_BOOT_INVALID  = 0,
> -BT_HID_BOOT_KEYBOARD,
> -BT_HID_BOOT_MOUSE,
> -};
> -
> -enum hid_data_pkt {
> -BT_DATA_OTHER= 0,
> -BT_DATA_INPUT,
> -BT_DATA_OUTPUT,
> -BT_DATA_FEATURE,
> -};
> -
> -#define BT_HID_MTU   48
> -
> -/* HID interface requests */
> -#define GET_REPORT   0xa101
> -#define GET_IDLE 0xa102
> -#define GET_PROTOCOL 0xa103
> -#define SET_REPORT   0x2109
> -#define SET_IDLE 0x210a
> -#define SET_PROTOCOL 0x210b
> -
> -struct bt_hid_device_s {
> -struct bt_l2cap_device_s btdev;
> -struct bt_l2cap_conn_params_s *control;
> -struct bt_l2cap_conn_params_s *interrupt;
> -HIDState hid;
> -
> -int proto;
> -int connected;
> -int data_type;
> -int intr_state;
> -struct {
> -int len;
> -uint8_t buffer[1024];
> -} dataother, datain, dataout, feature, intrdataout;
> -enum {
> -bt_state_ready,
> -bt_state_transaction,
> -bt_state_suspend,
> -} state;
> -};
> -
> -static void bt_hid_reset(struct bt_hid_device_s *s)
> -{
> -struct bt_scatternet_s *net = s->btdev.device.net;
> -
> -/* Go as far as... */
> -bt_l2cap_device_done(>btdev);
> -

Re: [Qemu-devel] [PATCH v1 0/3] intel-iommu: add support for 5-level virtual IOMMU.

2018-11-09 Thread no-reply
Hi,

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

Type: series
Message-id: 1541764187-10732-1-git-send-email-yu.c.zh...@linux.intel.com
Subject: [Qemu-devel] [PATCH v1 0/3] intel-iommu: add support for 5-level 
virtual IOMMU.

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20181109221213.7310-1-cr...@redhat.com -> 
patchew/20181109221213.7310-1-cr...@redhat.com
Switched to a new branch 'test'
bcc712a4f8 intel-iommu: search iotlb for levels supported by the address width.
b21a8d281a intel-iommu: extend VTD emulation to allow 57-bit IOVA address width.
5234b6784d intel-iommu: differentiate host address width from IOVA address 
width.

=== OUTPUT BEGIN ===
Checking PATCH 1/3: intel-iommu: differentiate host address width from IOVA 
address width
WARNING: line over 80 characters
#48: FILE: hw/i386/intel_iommu.c:709:
+ uint64_t *slptep, uint32_t *slpte_level, bool 
*reads,

total: 0 errors, 1 warnings, 188 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: intel-iommu: extend VTD emulation to allow 57-bit IOVA 
address width
ERROR: else should follow close brace '}'
#49: FILE: hw/i386/intel_iommu.c:3128:
 }
+else if (s->aw_bits == VTD_AW_57BIT) {

ERROR: Error messages should not contain newlines
#116: FILE: hw/i386/intel_iommu.c:3301:
+ "host and guest are capable of 5-level paging.\n");

total: 2 errors, 0 warnings, 122 lines checked

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

Checking PATCH 3/3: intel-iommu: search iotlb for levels supported by the 
address width
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH for 3.1] RISC-V: Respect fences for user-only emulators

2018-11-09 Thread Alistair Francis
On Fri, Nov 9, 2018 at 11:21 AM Palmer Dabbelt  wrote:
>
> Our current fence implementation ignores fences for the user-only
> configurations.  This is incorrect but unlikely to manifest: it requires
> multi-threaded user-only code that takes advantage of the weakness in
> the host's memory model and can be inlined by TCG.
>
> This patch simply treats fences the same way for all our emulators.
> I've given it to testing as I don't want to construct a test that would
> actually trigger the failure.
>
> Our fence implementation has an additional deficiency where we map all
> RISC-V fences to full fences.  Now that we have a formal memory model
> for RISC-V we can start to take advantage of the strength bits on our
> fence instructions.  This requires a bit more though, so I'm going to

s/though/thought/g

> split it out because the implementation is still correct without taking
> advantage of these weaker fences.
>
> Thanks to Richard Henderson for pointing out both of the issues.
>
> Signed-off-by: Palmer Dabbelt 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/translate.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 18d7b6d1471d..624d1c679a84 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1766,7 +1766,6 @@ static void decode_RV32_64G(CPURISCVState *env, 
> DisasContext *ctx)
>   GET_RM(ctx->opcode));
>  break;
>  case OPC_RISC_FENCE:
> -#ifndef CONFIG_USER_ONLY
>  if (ctx->opcode & 0x1000) {
>  /* FENCE_I is a no-op in QEMU,
>   * however we need to end the translation block */
> @@ -1777,7 +1776,6 @@ static void decode_RV32_64G(CPURISCVState *env, 
> DisasContext *ctx)
>  /* FENCE is a full memory barrier. */
>  tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
>  }
> -#endif
>  break;
>  case OPC_RISC_SYSTEM:
>  gen_system(env, ctx, MASK_OP_SYSTEM(ctx->opcode), rd, rs1,
> --
> 2.18.1
>
>



Re: [Qemu-devel] [PATCH] bitmap: Update count after a merge

2018-11-09 Thread John Snow



On 11/09/2018 12:57 PM, Eric Blake wrote:
> On 9/27/18 12:23 PM, John Snow wrote:
>>
>>
>> On 09/26/2018 11:11 PM, Eric Blake wrote:
>>> We need an accurate count of the number of bits set in a bitmap
>>> after a merge. In particular, since the merge operation short-circuits
>>> a merge from an empty source, if you have bitmaps A, B, and C where
>>> B started empty, then merge C into B, and B into A, an inaccurate
>>> count meant that A did not get the contents of C.
>>>
> 
>>
>> OK, tests coming up. Thanks for the CC to stable.
> 
> Did we ever add tests for this yet?
> 

Running behind. Not yet. On my list.



Re: [Qemu-devel] [PATCH v2 00/11] block: Deal with filters

2018-11-09 Thread Eric Blake

On 8/9/18 5:31 PM, Max Reitz wrote:

Note 1: This series depends on v10 of my “block: Fix some filename
generation issues” series.

Based-on: <20180809213528.14738-1-mre...@redhat.com>

Note 2: This is technically the first part of my active mirror followup.
But just very technically.  I noticed that that followup started to
consist of two parts, namely (A) fix filtery things in the block layer,
and (B) fix active mirror.  So I decided to split it.  This is part A.
Part B comes later.


When we introduced filters, we did it a bit casually.  Sure, we talked a
lot about them before, but that was mostly discussion about where
implicit filters should be added to the graph (note that we currently
only have two implicit filters, those being mirror and commit).  But in
the end, we really just designated some drivers filters (Quorum,
blkdebug, etc.) and added some specifically (throttle, COR), without
really looking through the block layer to see where issues might occur.

It turns out vast areas of the block layer just don't know about filters
and cannot really handle them.  Many cases will work in practice, in
others, well, too bad, you cannot use some feature because some part
deep inside the block layer looks at your filters and thinks they are
format nodes.

This series sets out to correct a bit of that.  I lost my head many
times and I'm sure this series is incomplete in many ways, but it really
doesn't do any good if it sits on my disk any longer, it needs to go out
now.


Is there any of this series that still needs to go in 3.1?

I'm trying to fix an NBD bug with incorrect alignment in response to 
NBD_CMD_BLOCK_STATUS.  I wanted to use blkdebug to force a 
larger-than-512 minimum alignment exposure to the guest.  But right now, 
nbd/server.c:nbd_export_bitmap() does a braindead:


while (true) {
bm = bdrv_find_dirty_bitmap(bs, bitmap);
if (bm != NULL || bs->backing == NULL) {
break;
}

bs = bs->backing->bs;
}

which fails to see through a blkdebug filter to find a dirty bitmap in 
the underlying qcow2 BDS (because blkdebug uses bs->file, not 
bs->backing).  It looks like your series will make my task a lot easier, 
but if it's not going in for 3.1, I still need to push my bug fix now, 
and defer the testsuite additions to later when we can more sanely see 
through filters.


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



Re: [Qemu-devel] [PATCH v2 02/11] blockdev: Check @replaces in blockdev_mirror_common

2018-11-09 Thread Eric Blake

On 8/9/18 5:31 PM, Max Reitz wrote:

There is no reason why the constraints we put on @replaces should be
limited to drive-mirror.  Therefore, move the sanity checks from
qmp_drive_mirror() to blockdev_mirror_common() so they apply to
blockdev-mirror as well.

Signed-off-by: Max Reitz 
---
  blockdev.c | 55 --
  1 file changed, 33 insertions(+), 22 deletions(-)



Reviewed-by: Eric Blake 

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



Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

2018-11-09 Thread Cleber Rosa



On 11/9/18 1:25 PM, Eduardo Habkost wrote:
>>
>> "Python -V" is quite different from "sys.version".  Indeed it includes
>> the "Python " prefix, but that's all, everything else is the version number.
> 
> Is this a guarantee documented somewhere?
> 

Oops, looks like I missed that comment, and failed to address it.  Now,
I must say I don't expect a documented guarantee about this will exist,
but it looks like this is an acknowledged use case:

https://bugs.python.org/issue18338

- Cleber.



Re: [Qemu-devel] [PATCH 2/4] check-venv: use recorded Python version

2018-11-09 Thread Eduardo Habkost
On Fri, Nov 09, 2018 at 10:07:08AM -0500, Cleber Rosa wrote:
> The current approach works fine, but it runs Python on every make
> command (even if it's not related to the venv usage).
> 
> This is just an optimization, and not a change of behavior.
> 
> Signed-off-by: Cleber Rosa 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



[Qemu-devel] [RFC PATCH 1/2] Acceptance Tests: add QemuImgTest base class

2018-11-09 Thread Cleber Rosa
Testing other utilities such as qemu-img do not require the same
infrastructure that testing QEMU itself does.  Let's add a base class
that just sets the suitable qemu-img binary to be used during test.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 1e54fd5932..dfd147b7e2 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -40,6 +40,20 @@ def pick_default_qemu_bin():
 return qemu_bin_from_src_dir_path
 
 
+def pick_default_qemu_img_bin():
+"""
+Picks the path of a QEMU binary, starting either in the current working
+directory or in the source tree root directory.
+"""
+qemu_img_bin_local = os.path.abspath("qemu-img")
+if is_readable_executable_file(qemu_img_bin_local):
+return qemu_img_bin_local
+
+qemu_img_bin_from_src_dir_path = os.path.join(SRC_ROOT_DIR, "qemu-img")
+if is_readable_executable_file(qemu_img_bin_from_src_dir_path):
+return qemu_img_bin_from_src_dir_path
+
+
 class Test(avocado.Test):
 def setUp(self):
 self.vm = None
@@ -52,3 +66,9 @@ class Test(avocado.Test):
 def tearDown(self):
 if self.vm is not None:
 self.vm.shutdown()
+
+
+class QemuImgTest(avocado.Test):
+def setUp(self):
+self.qemu_img_bin = self.params.get('qemu_img_bin',
+
default=pick_default_qemu_img_bin())
-- 
2.19.1




[Qemu-devel] [RFC PATCH 0/2] Acceptance tests for qemu-img

2018-11-09 Thread Cleber Rosa
The initial goal of this RFC is to get feedback on tests not specific
to the QEMU main binary, but specific to other components such as
qemu-img.

For this experiment, a small issue with the zero and negative number
of I/O operations given to the bench command was chosen.

Cleber Rosa (2):
  Acceptance Tests: add QemuImgTest base class
  qemu-img: consider a zero number of I/O requests an invalid count

 qemu-img.c  |   8 ++---
 tests/acceptance/avocado_qemu/__init__.py   |  20 
 tests/acceptance/qemu_img_bench.py  |  34 
 tests/acceptance/qemu_img_bench.py.data/img |   1 +
 tests/data/images/empty/raw | Bin 0 -> 1024 bytes
 5 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 tests/acceptance/qemu_img_bench.py
 create mode 12 tests/acceptance/qemu_img_bench.py.data/img
 create mode 100644 tests/data/images/empty/raw

-- 
2.19.1




[Qemu-devel] [RFC PATCH 2/2] qemu-img: consider a zero number of I/O requests an invalid count

2018-11-09 Thread Cleber Rosa
It's debatable whether it makes sense to consider the bench command
successful when no I/O requests will be performed.  This changes the
behavior to consider a zero count of I/O requests an invalid value.
While at it, avoid using signed types for number of I/O requests.

The image file used, is a simple raw image with 1K size.  There's a
obvious trade off between creating and reusing those images.  This is
an experiment in sharing those among tests.  It was created using:

 mkdir -p tests/data/images/empty
 cd tests/data/images/empty
 qemu-img create raw 1K

This relies on the Test's "get_data()", which by default looks for
data files on sources that map to various levels of specificity of a
test: file, test and additionally with variant and a symlink.

One other possibility with regards to in-tree images, is to extend the
Test's "get_data()" API (which is extensible by design) and make the
common images directory a "source".  The resulting API usage would be
similar to:

   self.get_data("empty/raw", source="common_images")

or simply:

   self.get_data("empty/raw")

To look for "empty/raw" in any of the available sources.  That would
make the symlink unnecessary.

Reference: 
https://avocado-framework.readthedocs.io/en/65.0/api/core/avocado.core.html#avocado.core.test.TestData
Signed-off-by: Cleber Rosa 
---
 qemu-img.c  |   8 ++---
 tests/acceptance/qemu_img_bench.py  |  34 
 tests/acceptance/qemu_img_bench.py.data/img |   1 +
 tests/data/images/empty/raw | Bin 0 -> 1024 bytes
 4 files changed, 39 insertions(+), 4 deletions(-)
 create mode 100644 tests/acceptance/qemu_img_bench.py
 create mode 12 tests/acceptance/qemu_img_bench.py.data/img
 create mode 100644 tests/data/images/empty/raw

diff --git a/qemu-img.c b/qemu-img.c
index 4c96db7ba4..7ffcdd1589 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3960,7 +3960,7 @@ typedef struct BenchData {
 int bufsize;
 int step;
 int nrreq;
-int n;
+unsigned int n;
 int flush_interval;
 bool drain_on_flush;
 uint8_t *buf;
@@ -4051,7 +4051,7 @@ static int img_bench(int argc, char **argv)
 bool quiet = false;
 bool image_opts = false;
 bool is_write = false;
-int count = 75000;
+unsigned int count = 75000;
 int depth = 64;
 int64_t offset = 0;
 size_t bufsize = 4096;
@@ -4098,7 +4098,7 @@ static int img_bench(int argc, char **argv)
 {
 unsigned long res;
 
-if (qemu_strtoul(optarg, NULL, 0, ) < 0 || res > INT_MAX) {
+if (qemu_strtoul(optarg, NULL, 0, ) < 0 || res > UINT_MAX || 
res <= 0) {
 error_report("Invalid request count specified");
 return 1;
 }
@@ -4248,7 +4248,7 @@ static int img_bench(int argc, char **argv)
 .flush_interval = flush_interval,
 .drain_on_flush = drain_on_flush,
 };
-printf("Sending %d %s requests, %d bytes each, %d in parallel "
+printf("Sending %u %s requests, %d bytes each, %d in parallel "
"(starting at offset %" PRId64 ", step size %d)\n",
data.n, data.write ? "write" : "read", data.bufsize, data.nrreq,
data.offset, data.step);
diff --git a/tests/acceptance/qemu_img_bench.py 
b/tests/acceptance/qemu_img_bench.py
new file mode 100644
index 00..327524ad8f
--- /dev/null
+++ b/tests/acceptance/qemu_img_bench.py
@@ -0,0 +1,34 @@
+# Test for the basic qemu-img bench command
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import os
+
+from avocado_qemu import QemuImgTest
+from avocado.utils import process
+
+
+class Bench(QemuImgTest):
+"""
+Runs the qemu-img tool with the bench command and different
+options and verifies the expected outcome.
+
+:avocado: enable
+"""
+def check_invalid_count(self, count):
+cmd = "%s bench -c %d %s" % (self.qemu_img_bin, count, 
self.get_data("img"))
+result = process.run(cmd, ignore_status=True)
+self.assertEqual(1, result.exit_status)
+self.assertIn(b"Invalid request count", result.stderr)
+
+def test_zero_count(self):
+self.check_invalid_count(0)
+
+def test_negative_count(self):
+self.check_invalid_count(-1)
diff --git a/tests/acceptance/qemu_img_bench.py.data/img 
b/tests/acceptance/qemu_img_bench.py.data/img
new file mode 12
index 00..6d01ef2e85
--- /dev/null
+++ b/tests/acceptance/qemu_img_bench.py.data/img
@@ -0,0 +1 @@
+../../data/images/empty/raw
\ No newline at end of file
diff --git a/tests/data/images/empty/raw b/tests/data/images/empty/raw
new file mode 100644
index 
..06d7405020018ddf3cacee90fd4af10487da3d20
GIT binary patch
literal 1024
ScmZQz7zLvtFd70QH3R?z00031

literal 0
HcmV?d1

-- 
2.19.1




[Qemu-devel] [PATCH] Acceptance test: add coverage tests for -smp option

2018-11-09 Thread Wainer dos Santos Moschetta
This adds tests for SMP option, by passing -smp with
various combinations of cpus, cores, threads, and sockets
values it checks that invalid topologies are not accepted
as well as missing values are correctly calculated.

Signed-off-by: Wainer dos Santos Moschetta 
---
 tests/acceptance/smp_option_coverage.py | 218 
 1 file changed, 218 insertions(+)
 create mode 100644 tests/acceptance/smp_option_coverage.py

diff --git a/tests/acceptance/smp_option_coverage.py 
b/tests/acceptance/smp_option_coverage.py
new file mode 100644
index 00..ab68d1a67d
--- /dev/null
+++ b/tests/acceptance/smp_option_coverage.py
@@ -0,0 +1,218 @@
+# QEMU -smp option coverage test.
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+#  Wainer dos Santos Moschetta 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from functools import reduce
+from avocado.utils.process import run
+
+from avocado_qemu import Test
+
+
+class SmpOption(Test):
+"""
+Launches QEMU with various cpus, cores, threads, sockets, and maxcpus
+combination through -smp option, to check it does not accept invalid SMP
+topologies as well as it is able to calculate correctly any missing values.
+
+:avocado: enable
+:avocado: tags=slow,coverage
+"""
+def setUp(self):
+super().setUp()
+self.cores = self.params.get('cores', default=2)
+self.threads = self.params.get('threads', default=2)
+self.sockets = self.params.get('sockets', default=2)
+self.cpus = self.params.get('cpus', default=8)
+
+def get_smp_topology(self):
+"""
+Returns a dict with the id of cores, threads and sockets.
+"""
+res = self.vm.qmp('query-hotpluggable-cpus')['return']
+cpus = [x['props'] for x in res]
+
+return reduce(lambda x, y: {'core-id': 
x['core-id'].union([y['core-id']]),
+'thread-id': 
x['thread-id'].union([y['thread-id']]),
+'socket-id': 
x['socket-id'].union([y['socket-id']])},
+  cpus, {'core-id': set(), 'thread-id': set(), 
'socket-id': set()})
+
+@staticmethod
+def build_option(**kwargs):
+"""
+Builds string for the -smp option.
+Use cpus, cores, threads, sockets, maxcpus keys.
+"""
+option_list = []
+if kwargs.get('cpus', None) is not None:
+option_list.append(str(kwargs.get('cpus')))
+for key, val in kwargs.items():
+if key == 'cpus':
+continue
+option_list.append('%s=%s' % (key, val))
+
+return ",".join(option_list)
+
+def launch_and_check(self, expect_cores=1, expect_threads=1,
+ expect_sockets=1, **kwargs):
+"""
+Launches VM and check its SMP topology was correctly set.
+Use cpus, cores, threads, sockets, and maxcpus keys to specify the
+topology.
+"""
+option = self.build_option(**{key: val for key, val in kwargs.items()
+  if key in ['cpus', 'cores', 'threads',
+ 'sockets', 'maxcpus']})
+self.vm.add_args('-smp', option)
+self.vm.launch()
+smp = self.get_smp_topology()
+self.assertEqual(smp['core-id'], set(range(0, expect_cores)))
+self.assertEqual(smp['thread-id'], set(range(0, expect_threads)))
+self.assertEqual(smp['socket-id'], set(range(0, expect_sockets)))
+
+def launch_and_check_fail(self, **kwargs):
+"""
+Launches VM and check the process exits with expected error code, for
+cases where the topology is expected not valid.
+"""
+option = self.build_option(**kwargs)
+res = run("%s -smp %s" % (self.qemu_bin, option), timeout=10,
+  ignore_status=True)
+self.assertNotEqual(res.exit_status, 0)
+
+# Passing cpus and maxcpus only.
+#
+
+def test_cpus_eq_maxcpus(self):
+self.launch_and_check(cpus=self.cpus,
+  maxcpus=self.cpus,
+  expect_sockets=self.cpus)
+
+def test_cpus_lt_maxcpus(self):
+maxcpus = self.cpus * 2
+self.launch_and_check(cpus=self.cpus,
+  maxcpus=maxcpus,
+  expect_sockets=maxcpus)
+
+def test_cpus_gt_maxcpus(self):
+self.launch_and_check_fail(cpus=self.cpus,
+   maxcpus=self.cpus // 2)
+
+# Passing a combination of cores, threads and sockets only.
+#
+
+def test_no_cores_no_threads_no_sockets(self):
+self.launch_and_check(cpus=self.cpus,
+  expect_sockets=self.cpus)
+
+def test_no_cores_no_threads_sockets(self):
+self.launch_and_check(sockets=self.sockets,
+  

[Qemu-devel] [PATCH] RFC: net/socket: learn to talk with a unix dgram socket

2018-11-09 Thread Marc-André Lureau
-net socket has a fd argument, and may be passed pre-opened sockets.

TCP sockets use framing.
UDP sockets have datagram boundaries.

When given a unix dgram socket, it will be able to read from it, but
will attempt to send on the dgram_dst, which is unset. The other end
will not receive the data.

Let's teach -net socket to recognize a UNIX DGRAM socket, and use the
regular send() command (without dgram_dst).

This makes running slirp out-of-process possible that
way (python pseudo-code):

a, b = socket.socketpair(socket.AF_UNIX, socket.SOCK_DGRAM)

subprocess.Popen('qemu -net socket,fd=%d -net user' % a.fileno(), shell=True)
subprocess.Popen('qemu ... -net nic -net socket,fd=%d' % b.fileno(), shell=True)

(to make slirp a seperate project altogether, we would have to have
some compatibility code and/or deprecate various options & HMP
commands for dynamic port forwarding etc - but this looks like a
reachable goal)

Signed-off-by: Marc-André Lureau 
---
 net/socket.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 7095eb749f..8a9c30892d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -119,9 +119,13 @@ static ssize_t net_socket_receive_dgram(NetClientState 
*nc, const uint8_t *buf,
 ssize_t ret;
 
 do {
-ret = qemu_sendto(s->fd, buf, size, 0,
-  (struct sockaddr *)>dgram_dst,
-  sizeof(s->dgram_dst));
+if (s->dgram_dst.sin_family != AF_UNIX) {
+ret = qemu_sendto(s->fd, buf, size, 0,
+  (struct sockaddr *)>dgram_dst,
+  sizeof(s->dgram_dst));
+} else {
+ret = send(s->fd, buf, size, 0);
+}
 } while (ret == -1 && errno == EINTR);
 
 if (ret == -1 && errno == EAGAIN) {
@@ -322,6 +326,15 @@ static NetSocketState 
*net_socket_fd_init_dgram(NetClientState *peer,
 int newfd;
 NetClientState *nc;
 NetSocketState *s;
+SocketAddress *sa;
+SocketAddressType sa_type;
+
+sa = socket_local_address(fd, errp);
+if (!sa) {
+return NULL;
+}
+sa_type = sa->type;
+qapi_free_SocketAddress(sa);
 
 /* fd passed: multicast: "learn" dgram_dst address from bound address and 
save it
  * Because this may be "shared" socket from a "master" process, datagrams 
would be recv()
@@ -365,8 +378,12 @@ static NetSocketState 
*net_socket_fd_init_dgram(NetClientState *peer,
  "socket: fd=%d (cloned mcast=%s:%d)",
  fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
 } else {
+if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
+s->dgram_dst.sin_family = AF_UNIX;
+}
+
 snprintf(nc->info_str, sizeof(nc->info_str),
- "socket: fd=%d", fd);
+ "socket: fd=%d %s", fd, SocketAddressType_str(sa_type));
 }
 
 return s;
-- 
2.19.1.708.g4ede3d42df




[Qemu-devel] [PATCH] net: refactor net_socket_send()

2018-11-09 Thread Marc-André Lureau
Simplify net_socket_send(), avoid extra buf/buf1 variable, and
backwards goto.

Signed-off-by: Marc-André Lureau 
---
 net/socket.c | 24 +---
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index c92354049b..8a9c30892d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -159,17 +159,12 @@ static void net_socket_send(void *opaque)
 {
 NetSocketState *s = opaque;
 int size;
-int ret;
-uint8_t buf1[NET_BUFSIZE];
-const uint8_t *buf;
-
-size = qemu_recv(s->fd, buf1, sizeof(buf1), 0);
-if (size < 0) {
-if (errno != EWOULDBLOCK)
-goto eoc;
-} else if (size == 0) {
+uint8_t buf[NET_BUFSIZE];
+
+size = qemu_recv(s->fd, buf, sizeof(buf), 0);
+if (size == 0 || (size < 0 && errno != EWOULDBLOCK) ||
+net_fill_rstate(>rs, buf, size) == -1) {
 /* end of connection */
-eoc:
 net_socket_read_poll(s, false);
 net_socket_write_poll(s, false);
 if (s->listen_fd != -1) {
@@ -181,15 +176,6 @@ static void net_socket_send(void *opaque)
 net_socket_rs_init(>rs, net_socket_rs_finalize, false);
 s->nc.link_down = true;
 memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
-
-return;
-}
-buf = buf1;
-
-ret = net_fill_rstate(>rs, buf, size);
-
-if (ret == -1) {
-goto eoc;
 }
 }
 
-- 
2.19.1.708.g4ede3d42df




Re: [Qemu-devel] [RFC PATCH spice v3 1/3] QXL interface: add a function to identify monitors in the guest

2018-11-09 Thread Gerd Hoffmann
On Thu, Nov 08, 2018 at 11:05:10AM +0100, Lukáš Hrázký wrote:
> Hello,
> 
> On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > + * The device_display_id_{start,count} denotes the sequence of device 
> > > display
> > > + * IDs that map to the zero-based sequence of monitor IDs provided by 
> > > monitors
> > > + * config on this interface. For example with device_display_id_start = 
> > > 2 and
> > > + * device_display_id_count = 3 you get the following mapping:
> > > + * monitor_id  ->  device_display_id
> > > + *  0  ->  2
> > > + *  1  ->  3
> > > + *  2  ->  4
> > > + *
> > > + * Note this example is unsupported in practice. The only supported 
> > > cases are
> > > + * either a single device display ID (count = 1) or multiple device 
> > > display IDs
> > > + * in a sequence starting from 0.
> > 
> > This is confusing.  The usage of this api in the qemu counterpart looks
> > sane though.
> 
> Not sure what you find confusing in particular... The example? Using an
> example and then saying it's not supported?

Yes, that for example.  Also wondering why device_display_id doesn't
start at zero.

cheers,
  Gerd




Re: [Qemu-devel] [RFC PATCH spice v3 1/3] QXL interface: add a function to identify monitors in the guest

2018-11-09 Thread Frediano Ziglio
> On Thu, Nov 08, 2018 at 11:05:10AM +0100, Lukáš Hrázký wrote:
> > Hello,
> > 
> > On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > + * The device_display_id_{start,count} denotes the sequence of device
> > > > display
> > > > + * IDs that map to the zero-based sequence of monitor IDs provided by
> > > > monitors
> > > > + * config on this interface. For example with device_display_id_start
> > > > = 2 and
> > > > + * device_display_id_count = 3 you get the following mapping:
> > > > + * monitor_id  ->  device_display_id
> > > > + *  0  ->  2
> > > > + *  1  ->  3
> > > > + *  2  ->  4
> > > > + *
> > > > + * Note this example is unsupported in practice. The only supported
> > > > cases are
> > > > + * either a single device display ID (count = 1) or multiple device
> > > > display IDs
> > > > + * in a sequence starting from 0.
> > > 
> > > This is confusing.  The usage of this api in the qemu counterpart looks
> > > sane though.
> > 
> > Not sure what you find confusing in particular... The example? Using an
> > example and then saying it's not supported?
> 
> Yes, that for example.  Also wondering why device_display_id doesn't
> start at zero.
> 

You introduced this ID so you should remember.
It starts from 0, just this is not the first registration, the other displays
were registered to other QXL instances.
This was introduced for virtio-gpu to be able to register the different
outputs to different QXL instances so to have

first output/display:
  spice_qxl_set_device_info(instance1, path1, 0, 1);
second:
  spice_qxl_set_device_info(instance2, path1, 1, 1);
third:
  spice_qxl_set_device_info(instance3, path1, 2, 1);

So as you can see device_display_id start from 0 but does not mean
is always 0 for every call.

> cheers,
>   Gerd
> 
> 
> 

Frediano



Re: [Qemu-devel] xen_disk qdevification

2018-11-09 Thread Kevin Wolf
Am 09.11.2018 um 11:27 hat Paul Durrant geschrieben:
> > -Original Message-
> > From: Paul Durrant
> > Sent: 08 November 2018 16:44
> > To: Paul Durrant ; 'Kevin Wolf'
> > 
> > Cc: Stefano Stabellini ; qemu-bl...@nongnu.org;
> > Tim Smith ; qemu-devel@nongnu.org; 'Markus
> > Armbruster' ; Anthony Perard
> > ; xen-de...@lists.xenproject.org; Max Reitz
> > 
> > Subject: RE: [Qemu-devel] xen_disk qdevification
> > 
> > > -Original Message-
> > > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On
> > Behalf
> > > Of Paul Durrant
> > > Sent: 08 November 2018 15:44
> > > To: 'Kevin Wolf' 
> > > Cc: Stefano Stabellini ; qemu-bl...@nongnu.org;
> > > Tim Smith ; qemu-devel@nongnu.org; 'Markus
> > > Armbruster' ; Anthony Perard
> > > ; xen-de...@lists.xenproject.org; Max Reitz
> > > 
> > > Subject: Re: [Xen-devel] [Qemu-devel] xen_disk qdevification
> > >
> > > > -Original Message-
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > Sent: 08 November 2018 15:21
> > > > To: Paul Durrant 
> > > > Cc: 'Markus Armbruster' ; Anthony Perard
> > > > ; Tim Smith ; Stefano
> > > > Stabellini ; qemu-bl...@nongnu.org; qemu-
> > > > de...@nongnu.org; Max Reitz ; xen-
> > > > de...@lists.xenproject.org
> > > > Subject: Re: [Qemu-devel] xen_disk qdevification
> > > >
> > > > Am 08.11.2018 um 15:00 hat Paul Durrant geschrieben:
> > > > > > -Original Message-
> > > > > > From: Markus Armbruster [mailto:arm...@redhat.com]
> > > > > > Sent: 05 November 2018 15:58
> > > > > > To: Paul Durrant 
> > > > > > Cc: 'Kevin Wolf' ; Tim Smith
> > > ;
> > > > > > Stefano Stabellini ; qemu-
> > bl...@nongnu.org;
> > > > qemu-
> > > > > > de...@nongnu.org; Max Reitz ; Anthony Perard
> > > > > > ; xen-de...@lists.xenproject.org
> > > > > > Subject: Re: [Qemu-devel] xen_disk qdevification
> > > > > >
> > > > > > Paul Durrant  writes:
> > > > > >
> > > > > > >> -Original Message-
> > > > > > >> From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > > >> Sent: 02 November 2018 11:04
> > > > > > >> To: Tim Smith 
> > > > > > >> Cc: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org;
> > qemu-
> > > > > > >> bl...@nongnu.org; Anthony Perard ;
> > > Paul
> > > > > > Durrant
> > > > > > >> ; Stefano Stabellini
> > > > ;
> > > > > > >> Max Reitz ; arm...@redhat.com
> > > > > > >> Subject: xen_disk qdevification (was: [PATCH 0/3] Performance
> > > > > > improvements
> > > > > > >> for xen_disk v2)
> > > > > > >>
> > > > > > >> Am 02.11.2018 um 11:00 hat Tim Smith geschrieben:
> > > > > > >> > A series of performance improvements for disks using the Xen
> > PV
> > > > ring.
> > > > > > >> >
> > > > > > >> > These have had fairly extensive testing.
> > > > > > >> >
> > > > > > >> > The batching and latency improvements together boost the
> > > > throughput
> > > > > > >> > of small reads and writes by two to six percent (measured
> > using
> > > > fio
> > > > > > >> > in the guest)
> > > > > > >> >
> > > > > > >> > Avoiding repeated calls to posix_memalign() reduced the dirty
> > > > heap
> > > > > > >> > from 25MB to 5MB in the case of a single datapath process
> > while
> > > > also
> > > > > > >> > improving performance.
> > > > > > >> >
> > > > > > >> > v2 removes some checkpatch complaints and fixes the CCs
> > > > > > >>
> > > > > > >> Completely unrelated, but since you're the first person
> > touching
> > > > > > >> xen_disk in a while, you're my victim:
> > > > > > >>
> > > > > > >> At KVM Forum we discussed sending a patch to deprecate xen_disk
> > > > because
> > > > > > >> after all those years, it still hasn't been converted to qdev.
> > > > Markus
> > > > > > is
> > > > > > >> currently fixing some other not yet qdevified block device, but
> > > > after
> > > > > > >> that xen_disk will be the only one left.
> > > > > > >>
> > > > > > >> A while ago, a downstream patch review found out that there are
> > > > some
> > > > > > QMP
> > > > > > >> commands that would immediately crash if a xen_disk device were
> > > > present
> > > > > > >> because of the lacking qdevification. This is not the code
> > > quality
> > > > > > >> standard I envision for QEMU. It's time for non-qdev devices to
> > > go.
> > > > > > >>
> > > > > > >> So if you guys are still interested in the device, could
> > someone
> > > > please
> > > > > > >> finally look into converting it?
> > > > > > >>
> > > > > > >
> > > > > > > I have a patch series to do exactly this. It's somewhat involved
> > > as
> > > > I
> > > > > > > need to convert the whole PV backend infrastructure. I will try
> > to
> > > > > > > rebase and clean up my series a.s.a.p.
> > > > > >
> > > > > > Awesome!  Please coordinate with Anthony Prerard to avoid
> > > duplicating
> > > > > > work if you haven't done so already.
> > > > >
> > > > > I've come across a bit of a problem that I'm not sure how best to
> > deal
> > > > > with and so am looking for some advice.
> > > > >
> > > > > I now have a qdevified PV disk backend but I can't bring it 

Re: [Qemu-devel] [RFC PATCH spice v3 1/3] QXL interface: add a function to identify monitors in the guest

2018-11-09 Thread Gerd Hoffmann
  Hi,

> first output/display:
>   spice_qxl_set_device_info(instance1, path1, 0, 1);
> second:
>   spice_qxl_set_device_info(instance2, path1, 1, 1);
> third:
>   spice_qxl_set_device_info(instance3, path1, 2, 1);

That is a much better example IMHO.

cheers,
  Gerd




Re: [Qemu-devel] [PULL] A Single RISC-V Patch for 3.1-rc1

2018-11-09 Thread Peter Maydell
On 8 November 2018 at 18:52, Palmer Dabbelt  wrote:
> On Thu, 08 Nov 2018 10:38:51 PST (-0800), alistai...@gmail.com wrote:
>>
>> On Thu, Nov 8, 2018 at 10:35 AM Palmer Dabbelt  wrote:
>>>
>>>
>>> The following changes since commit
>>> a7ce790a029bd94eb320d8c69f38900f5233997e:
>>>
>>>   tcg/tcg-op.h: Add multiple include guard (2018-11-08 15:15:32 +)
>>>
>>> are available in the Git repository at:
>>>
>>>   git://github.com/riscv/riscv-qemu.git tags/riscv-for-master-3.1-rc1
>>>
>>> for you to fetch changes up to 00a014ac01feac875468d38c376f7e06f050f992:
>>>
>>>   riscv: spike: Fix memory leak in the board init (2018-11-08 08:41:06
>>> -0800)
>>>
>>> 
>>> A Single RISC-V Patch for 3.1-rc1
>>>
>>> This tag contains a single patch that I'd like to target for rc1: a fix
>>> for a memory leak that was detected by static code analysis.
>>>
>>> There are still three patch sets that I'd like to try to get up for 3.1:
>>>
>>> * The patch set Basian just published that contains fixes for a pair of
>>>   issues he found when converting our port to decodetree.
>>> * An as-of-yet-unwritten fix to the third issue that Basian pointed out.
>>> * A fix to our fflags bug, which is currently coupled to some CSR
>>>   refactoring that I don't think is OK for 3.1.
>>
>>
>> There is one more patch fixing a memory leak in the virt board as well.
>
>
> Oh, sorry about that -- I thought they were the same patch!
>
> Peter: let me know if you want me to submit a new copy of this PR with both
> patches or a follow-on PR.  Sorry for the noise!

As it happens, I'd just run this pullreq through testing overnight,
so I've pushed it to master; you can send another one with the other
patch.

thanks
-- PMM



Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Laurent Vivier
On 09/11/2018 11:00, Paolo Bonzini wrote:
> On 09/11/2018 09:29, Laurent Vivier wrote:
>> On 09/11/2018 09:26, Thomas Huth wrote:
>>> On 2018-11-09 09:21, Laurent Vivier wrote:
 On 09/11/2018 09:13, Li Qiang wrote:
> In set_netdev(), the peers[i] is initialized
> qemu_find_net_clients_except() when i is in
> 0 between 'queues' it can't be NULL.
>
> Signed-off-by: Li Qiang 
> ---
>  hw/core/qdev-properties-system.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 8b22fb5..b45a7ef 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, 
> const char *name,
>  }
>  
>  for (i = 0; i < queues; i++) {
> -if (peers[i] == NULL) {
> -err = -ENOENT;
> -goto out;
> -}
>  
>  if (peers[i]->peer) {
>  err = -EEXIST;
>

 Reviewed-by: Laurent Vivier 
>>>
>>> So who can pick up such patches? We don't have a maintainer for the QDEV
>>> subsystem anymore... should it go via qemu-trivial?
>>
>> It seems trivial enough to go via qemu-trivial, but it would be good to
>> have a R-b from someone else than me to confirm.
> 
> I would have expected Jason to pick it up, after all it's networking,
> but it's okay if you want to take it too.

If Jason pushes it before I do, I will remove it from my queue.
[I have the bad habit to rebase by branch before a pull request]

Thanks,
Laurent




[Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests

2018-11-09 Thread David Hildenbrand
Basically copy all int64 list tests but adapt them to work on uint64
instead.

Signed-off-by: David Hildenbrand 
---
 tests/test-string-input-visitor.c | 71 +--
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/tests/test-string-input-visitor.c 
b/tests/test-string-input-visitor.c
index 2f6360e9ca..731094f789 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -111,7 +111,6 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
   6, 7, 8 };
 int64_t expect2[] = { 32767, -32768, -32767 };
 int64_t expect3[] = { INT64_MIN, INT64_MAX };
-uint64_t expect4[] = { UINT64_MAX };
 Error *err = NULL;
 int64List *res = NULL;
 Visitor *v;
@@ -129,9 +128,6 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 "-9223372036854775808,9223372036854775807");
 check_ilist(v, expect3, ARRAY_SIZE(expect3));
 
-v = visitor_input_test_init(data, "18446744073709551615");
-check_ulist(v, expect4, ARRAY_SIZE(expect4));
-
 /* Empty list */
 
 v = visitor_input_test_init(data, "");
@@ -174,6 +170,71 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 visit_end_list(v, NULL);
 }
 
+static void test_visitor_in_uintList(TestInputVisitorData *data,
+ const void *unused)
+{
+uint64_t expect1[] = { 1, 2, 0, 2, 3, 4, 20, 5, 6, 7, 8, 9, 1, 2, 3, 4, 5,
+   6, 7, 8 };
+uint64_t expect2[] = { 32767, -32768, -32767 };
+uint64_t expect3[] = { UINT64_MAX };
+Error *err = NULL;
+uint64List *res = NULL;
+Visitor *v;
+uint64_t val;
+
+/* Valid lists */
+
+v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
+check_ulist(v, expect1, ARRAY_SIZE(expect1));
+
+v = visitor_input_test_init(data, "32767,-32768--32767");
+check_ulist(v, expect2, ARRAY_SIZE(expect2));
+
+v = visitor_input_test_init(data, "18446744073709551615");
+check_ulist(v, expect3, ARRAY_SIZE(expect3));
+
+/* Empty list */
+
+v = visitor_input_test_init(data, "");
+visit_type_uint64List(v, NULL, , _abort);
+g_assert(!res);
+
+/* Not a list */
+
+v = visitor_input_test_init(data, "not an uint list");
+
+visit_type_uint64List(v, NULL, , );
+error_free_or_abort();
+g_assert(!res);
+
+/* Unvisited list tail */
+
+v = visitor_input_test_init(data, "0,2-3");
+
+visit_start_list(v, NULL, NULL, 0, _abort);
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(val, ==, 0);
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(val, ==, 2);
+
+visit_check_list(v, );
+error_free_or_abort();
+visit_end_list(v, NULL);
+
+/* Visit beyond end of list */
+
+v = visitor_input_test_init(data, "0");
+
+visit_start_list(v, NULL, NULL, 0, _abort);
+visit_type_uint64(v, NULL, , );
+g_assert_cmpuint(val, ==, 0);
+visit_type_uint64(v, NULL, , );
+error_free_or_abort();
+
+visit_check_list(v, _abort);
+visit_end_list(v, NULL);
+}
+
 static void test_visitor_in_bool(TestInputVisitorData *data,
  const void *unused)
 {
@@ -334,6 +395,8 @@ int main(int argc, char **argv)
_visitor_data, test_visitor_in_int);
 input_visitor_test_add("/string-visitor/input/intList",
_visitor_data, test_visitor_in_intList);
+input_visitor_test_add("/string-visitor/input/uintList",
+   _visitor_data, test_visitor_in_uintList);
 input_visitor_test_add("/string-visitor/input/bool",
_visitor_data, test_visitor_in_bool);
 input_visitor_test_add("/string-visitor/input/number",
-- 
2.17.2




[Qemu-devel] [PATCH RFC 6/6] test-string-input-visitor: add range overflow tests

2018-11-09 Thread David Hildenbrand
Let's make sure that the range handling code can properly deal with
ranges that end at the biggest possible number.

Signed-off-by: David Hildenbrand 
---
 tests/test-string-input-visitor.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/test-string-input-visitor.c 
b/tests/test-string-input-visitor.c
index 731094f789..809bd59fca 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -111,6 +111,7 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
   6, 7, 8 };
 int64_t expect2[] = { 32767, -32768, -32767 };
 int64_t expect3[] = { INT64_MIN, INT64_MAX };
+int64_t expect4[] = { INT64_MAX - 2,  INT64_MAX - 1, INT64_MAX };
 Error *err = NULL;
 int64List *res = NULL;
 Visitor *v;
@@ -128,6 +129,10 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 "-9223372036854775808,9223372036854775807");
 check_ilist(v, expect3, ARRAY_SIZE(expect3));
 
+v = visitor_input_test_init(data,
+"9223372036854775805-9223372036854775807");
+check_ilist(v, expect4, ARRAY_SIZE(expect4));
+
 /* Empty list */
 
 v = visitor_input_test_init(data, "");
@@ -177,6 +182,7 @@ static void test_visitor_in_uintList(TestInputVisitorData 
*data,
6, 7, 8 };
 uint64_t expect2[] = { 32767, -32768, -32767 };
 uint64_t expect3[] = { UINT64_MAX };
+uint64_t expect4[] = { UINT64_MAX - 2,  UINT64_MAX - 1, UINT64_MAX };
 Error *err = NULL;
 uint64List *res = NULL;
 Visitor *v;
@@ -193,6 +199,10 @@ static void test_visitor_in_uintList(TestInputVisitorData 
*data,
 v = visitor_input_test_init(data, "18446744073709551615");
 check_ulist(v, expect3, ARRAY_SIZE(expect3));
 
+v = visitor_input_test_init(data,
+"18446744073709551613-18446744073709551615");
+check_ulist(v, expect4, ARRAY_SIZE(expect4));
+
 /* Empty list */
 
 v = visitor_input_test_init(data, "");
-- 
2.17.2




[Qemu-devel] [PATCH RFC 0/6] qapi: rewrite string-input-visitor

2018-11-09 Thread David Hildenbrand
Rewrite string-input-visitor to be (hopefully) less ugly. Support
int and uint lists (including ranges, but not implemented via type
"Range").

Virtual walks are now supported and more errors are cought (and some bugs
fixed). Fix and extend the tests. Parsing of uint64_t is now properly
supported.

Importantly, when parsing a list we now return the list and not an
ordered set (we are not an ordered set parser after all). Whoever needs
that can add it on top. As far as I can see, current code can deal with
it but I'll have to look at the details.

Guess once this part here is done, the output visitor is the next thing
to rework.

David Hildenbrand (6):
  cutils: add qemu_strtod()
  qapi: use qemu_strtod() in string-input-visitor
  qapi: rewrite string-input-visitor
  test-string-input-visitor: use virtual walk
  test-string-input-visitor: split off uint64 list tests
  test-string-input-visitor: add range overflow tests

 include/qapi/string-input-visitor.h |   3 +-
 include/qemu/cutils.h   |   1 +
 qapi/string-input-visitor.c | 444 
 tests/test-string-input-visitor.c   | 131 +---
 util/cutils.c   |  22 ++
 5 files changed, 377 insertions(+), 224 deletions(-)

-- 
2.17.2




[Qemu-devel] [PATCH RFC 2/6] qapi: use qemu_strtod() in string-input-visitor

2018-11-09 Thread David Hildenbrand
Let's use the new function.

Signed-off-by: David Hildenbrand 
---
 qapi/string-input-visitor.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index b3fdd0827d..dee708d384 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -20,6 +20,7 @@
 #include "qemu/option.h"
 #include "qemu/queue.h"
 #include "qemu/range.h"
+#include "qemu/cutils.h"
 
 
 struct StringInputVisitor
@@ -313,12 +314,9 @@ static void parse_type_number(Visitor *v, const char 
*name, double *obj,
   Error **errp)
 {
 StringInputVisitor *siv = to_siv(v);
-char *endp = (char *) siv->string;
 double val;
 
-errno = 0;
-val = strtod(siv->string, );
-if (errno || endp == siv->string || *endp) {
+if (qemu_strtod(siv->string, NULL, )) {
 error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"number");
 return;
-- 
2.17.2




[Qemu-devel] [RFC v8 05/18] virtio-iommu: Add the iommu regions

2018-11-09 Thread Eric Auger
This patch initializes the iommu memory regions so that
PCIe end point transactions get translated. The translation
function is not yet implemented though.

Signed-off-by: Eric Auger 

---
v6 -> v7:
- use primary_bus
- rebase on new translate proto featuring iommu_idx

v5 -> v6:
- include qapi/error.h
- fix g_hash_table_lookup key in virtio_iommu_find_add_as

v4 -> v5:
- use PCI bus handle as a key
- use get_primary_pci_bus() callback

v3 -> v4:
- add trace_virtio_iommu_init_iommu_mr

v2 -> v3:
- use IOMMUMemoryRegion
- iommu mr name built with BDF
- rename smmu_get_sid into virtio_iommu_get_sid and use PCI_BUILD_BDF
---
 hw/virtio/trace-events   |  2 +
 hw/virtio/virtio-iommu.c | 94 
 include/hw/virtio/virtio-iommu.h |  2 +
 3 files changed, 98 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index e6177ca0e4..9270b0463e 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -59,3 +59,5 @@ virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) 
"domain=%d endpoint=%d"
 virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, 
uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" 
virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
 virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) 
"domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
+virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int 
flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
+virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 5346440c51..2043623b30 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -22,6 +22,10 @@
 #include "qemu-common.h"
 #include "hw/virtio/virtio.h"
 #include "sysemu/kvm.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/i386/pc.h"
+#include "hw/arm/virt.h"
 #include "trace.h"
 
 #include "standard-headers/linux/virtio_ids.h"
@@ -33,6 +37,50 @@
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
+{
+return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
+}
+
+static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
+  int devfn)
+{
+VirtIOIOMMU *s = opaque;
+IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
+IOMMUDevice *sdev;
+
+if (!sbus) {
+sbus = g_malloc0(sizeof(IOMMUPciBus) +
+ sizeof(IOMMUDevice *) * IOMMU_PCI_DEVFN_MAX);
+sbus->bus = bus;
+g_hash_table_insert(s->as_by_busptr, bus, sbus);
+}
+
+sdev = sbus->pbdev[devfn];
+if (!sdev) {
+char *name = g_strdup_printf("%s-%d-%d",
+ TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+ pci_bus_num(bus), devfn);
+sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
+
+sdev->viommu = s;
+sdev->bus = bus;
+sdev->devfn = devfn;
+
+trace_virtio_iommu_init_iommu_mr(name);
+
+memory_region_init_iommu(>iommu_mr, sizeof(sdev->iommu_mr),
+ TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+ OBJECT(s), name,
+ UINT64_MAX);
+address_space_init(>as,
+   MEMORY_REGION(>iommu_mr), TYPE_VIRTIO_IOMMU);
+}
+
+return >as;
+
+}
+
 static int virtio_iommu_attach(VirtIOIOMMU *s,
struct virtio_iommu_req_attach *req)
 {
@@ -204,6 +252,27 @@ static void virtio_iommu_handle_command(VirtIODevice 
*vdev, VirtQueue *vq)
 }
 }
 
+static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
+IOMMUAccessFlags flag,
+int iommu_idx)
+{
+IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+uint32_t sid;
+
+IOMMUTLBEntry entry = {
+.target_as = _space_memory,
+.iova = addr,
+.translated_addr = addr,
+.addr_mask = ~(hwaddr)0,
+.perm = IOMMU_NONE,
+};
+
+sid = virtio_iommu_get_sid(sdev);
+
+trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+return entry;
+}
+
 static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
 VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
@@ -286,6 +355,15 @@ static void virtio_iommu_device_realize(DeviceState *dev, 
Error **errp)
 virtio_add_feature(>features, VIRTIO_IOMMU_F_DOMAIN_BITS);
 virtio_add_feature(>features, VIRTIO_IOMMU_F_MAP_UNMAP);
 virtio_add_feature(>features, VIRTIO_IOMMU_F_BYPASS);
+
+memset(s->as_by_bus_num, 0, sizeof(s->as_by_bus_num));
+s->as_by_busptr = 

[Qemu-devel] [RFC v8 07/18] virtio-iommu: Implement attach/detach command

2018-11-09 Thread Eric Auger
This patch implements the endpoint attach/detach to/from
a domain.

Signed-off-by: Eric Auger 

---
---
 hw/virtio/virtio-iommu.c | 40 ++--
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index e4014a8800..bd245ea979 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -82,8 +82,8 @@ static void 
virtio_iommu_detach_endpoint_from_domain(viommu_endpoint *ep)
 ep->domain = NULL;
 }
 
-viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id);
-viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id)
+static viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
+  uint32_t ep_id)
 {
 viommu_endpoint *ep;
 
@@ -112,8 +112,8 @@ static void virtio_iommu_put_endpoint(gpointer data)
 g_free(ep);
 }
 
-viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id);
-viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id)
+static viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s,
+  uint32_t domain_id)
 {
 viommu_domain *domain;
 
@@ -191,10 +191,27 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
 {
 uint32_t domain_id = le32_to_cpu(req->domain);
 uint32_t ep_id = le32_to_cpu(req->endpoint);
+viommu_domain *domain;
+viommu_endpoint *ep;
 
 trace_virtio_iommu_attach(domain_id, ep_id);
 
-return VIRTIO_IOMMU_S_UNSUPP;
+ep = virtio_iommu_get_endpoint(s, ep_id);
+if (ep->domain) {
+/*
+ * the device is already attached to a domain,
+ * detach it first
+ */
+virtio_iommu_detach_endpoint_from_domain(ep);
+}
+
+domain = virtio_iommu_get_domain(s, domain_id);
+QLIST_INSERT_HEAD(>endpoint_list, ep, next);
+
+ep->domain = domain;
+g_tree_ref(domain->mappings);
+
+return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_detach(VirtIOIOMMU *s,
@@ -202,10 +219,21 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
 {
 uint32_t domain_id = le32_to_cpu(req->domain);
 uint32_t ep_id = le32_to_cpu(req->endpoint);
+viommu_endpoint *ep;
 
 trace_virtio_iommu_detach(domain_id, ep_id);
 
-return VIRTIO_IOMMU_S_UNSUPP;
+ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+if (!ep) {
+return VIRTIO_IOMMU_S_NOENT;
+}
+
+if (!ep->domain) {
+return VIRTIO_IOMMU_S_INVAL;
+}
+
+virtio_iommu_detach_endpoint_from_domain(ep);
+return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_map(VirtIOIOMMU *s,
-- 
2.17.2




Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

2018-11-09 Thread Aleksandar Markovic
> From: Fredrik Noring 
> Subject: Re: [PATCH v2 4/6] target/mips: Fix decoding mechanism of special 
> R5900 opcodes
> 
> Hi Aleksandar,
> 
> > Fredrik, do you know by any chance if a document exists that would justify
> > inclusion of non-R5900 DMULT, DMULTU, DDIV, DDIVU in R5900 executables by
> > gcc for R5900? Is it included by cross-gcc or by native gcc, or by both?
> >
> > I think gcc folks must have had a good reason for that, some kind of
> > design - it can't be 'I really like/miss this instruction, let's include
> > it...'
> 
> The R5900 reports itself as MIPS III ...

This is very unclear. What do you mean by this? How does R5900 do that? I can't 
find any trace of such intentions in R5900 docs.

> ... and DMULT, DMULTU, DDIV and DDIVU
> are part of the MIPS III ISA. They are emulated in user mode to support
> generic MIPS III programs.

Pure MIPS III executables should not be a concern of the R5900 emulation, but 
R5900 executables.

Could you please provide a document that would justify inclusion of these 
non-R5900 instruction in an R5900 emulation?

Thanks,
Aleksandar



Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd

2018-11-09 Thread Juergen Gross
On 09/11/2018 10:57, Li Zhijian wrote:
> On 11/9/2018 3:20 PM, Ingo Molnar wrote:
>> * Li Zhijian  wrote:
>>
 If the kernel initrd creation process creates an initrd which
 is larger than 2GB and also claims that it can't be placed
 with any part of it above 2GB, then that sounds like a bug
 in the initrd creation process...
>>> Exactly, it's a real problem.
>>>
>>> Add x86 maintainers and LKML:
>>>
>>> The background is that QEMU want to support up to 4G initrd. but
>>> linux header (
>>> initrd_addr_max field) only allow 2G-1.
>>> Is one of the below approaches reasonable:
>>> 1) change initrd_addr_max to 4G-1 directly
>>> simply(arch/x86/boot/header.S)?
>>> 2) lie QEMU bootloader the initrd_addr_max is 4G-1 even though header
>>> said 2G-1
>>> 3) any else
>> A 10 years old comment from hpa says:
>>
>>    initrd_addr_max: .long 0x7fff
>>  # (Header version 0x0203 or
>> later)
>>  # The highest safe address for
>>  # the contents of an initrd
>>  # The current kernel allows
>> up to 4 GB,
>>  # but leave it at 2 GB to avoid
>>  # possible bootloader bugs.
>>
>> To avoid the potential of bugs lurking in dozens of major and hundreds of
>> minor iterations of various Linux bootloaders I'd prefer a real solution
>> and extend it - because if there's a 2GB initrd for some weird reason
>> today there might be a 4GB one in two years.
> 
> thank a lots. that's amazing.
> 
> 
>>
>> The real solution would be to:
>>
>>   - Extend the boot protocol with a 64-bit field, named initrd_addr64_max
>>     or such.
>>   - We don't change the old field - but if the new field is set by new
>>     kernels then new bootloaders can use that as a new initrd_addr64_max
>>     value. (or reject to load the kernel if the address is too high.)
>>
>>   - The kernel build should also emit a warning when building larger than
>>     2GB initrds, with a list of bootloaders that support the new
>> protocol.
> 
> Actually i just knew QEMU(Seabios + optionrom(linuxboot_dma.bin)) can
> support ~4GB initrd so far.
> 
> i just drafted at patch to add this field. could you have a look.
> another patch which is to document initrd_addr64_max is ongoing.
> 
> commit db463ac9c1975f115d1ce2acb82d530c2b63b888
> Author: Li Zhijian 
> Date:   Fri Nov 9 17:24:14 2018 +0800
> 
>     x86: Add header field initrd_addr64_max
>         Years ago, kernel had support load ~4GB initrd. But for some
> weird reasons (
>     avoid possible bootloader bugs), it only allow leave initrd under
> 2GB address
>     space(see initrd_addr_max fild at arch/x86/boot/header.S).
>         So modern bootloaders have not chance to load >=2G initrd
> previously.
>         To avoid the potential of bugs lurking in dozens of major and
> hundreds of
>     minor iterations of various Linux bootloaders. Ingo suggests to add
> a new field
>     initrd_addr64_max. If bootloader believes that it can load initrd to
>>=2G
>     address space, it can use initrd_addr64_max as the maximum loading
> address in
>     stead of the old field initrd_addr_max.
> 
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index 4c881c8..5fc3ebe 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -300,7 +300,7 @@ _start:
>     # Part 2 of the header, from the old setup.S
>  
>     .ascii  "HdrS"  # header signature
> -   .word   0x020e  # header version number (>= 0x0105)
> +   .word   0x020f  # header version number (>= 0x0105)
>     # or else old loadlin-1.5 will
> fail)
>     .globl realmode_swtch
>  realmode_swtch:    .word   0, 0    # default_switch, SETUPSEG
> @@ -562,6 +562,12 @@ acpi_rsdp_addr:    .quad 0
> # 64-bit physical pointer to the
>     # ACPI RSDP table, added
> with
>     # version 2.14
>  
> +#ifdef CONFIG_INITRD_SIZE_4GB
> +initrd_addr64_max: .quad 0x    # allow ~4G initrd since
> 2.15
> +#else
> +initrd_addr64_max: .quad 0

Shouldn't this be 0x7fff?

And please update Documentation/x86/boot.txt


Juergen



Re: [Qemu-devel] QEMU and Kconfig

2018-11-09 Thread Paolo Bonzini
On 08/11/2018 22:00, Eduardo Habkost wrote:
> Understood.  My interpretation of "target" was just "a QEMU
> binary".  In other words, I thought we were talking about
> anything that could be compiled in/out from a specific QEMU
> binary.

The idea is "target" as opposed to "host".

> Do you have a specific reason to restrict the scope to only
> guest-visible effects?  Is this just a way to reduce the effort
> required for the task, or there are other caveats I'm missing?

Because that's what default-configs/ is for---producing
config-devices.mak.  IOW it's mostly to reduce the scope, but also
because there are differences between config-devices.mak (produced from
default-configs/) and config-{host,target}.h (produced by configure).

In particular, config-host.h and config-target.h are header files, but
config-devices.mak is not because the same file is linked into multiple
QEMU binaries that can and will enable different devices.  The only way
to use a hypothetical config-devices.h would be to move its users from
common-obj-y to obj-y.

Paolo



Re: [Qemu-devel] xen_disk qdevification

2018-11-09 Thread Paul Durrant
> -Original Message-
> From: Paul Durrant
> Sent: 08 November 2018 16:44
> To: Paul Durrant ; 'Kevin Wolf'
> 
> Cc: Stefano Stabellini ; qemu-bl...@nongnu.org;
> Tim Smith ; qemu-devel@nongnu.org; 'Markus
> Armbruster' ; Anthony Perard
> ; xen-de...@lists.xenproject.org; Max Reitz
> 
> Subject: RE: [Qemu-devel] xen_disk qdevification
> 
> > -Original Message-
> > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On
> Behalf
> > Of Paul Durrant
> > Sent: 08 November 2018 15:44
> > To: 'Kevin Wolf' 
> > Cc: Stefano Stabellini ; qemu-bl...@nongnu.org;
> > Tim Smith ; qemu-devel@nongnu.org; 'Markus
> > Armbruster' ; Anthony Perard
> > ; xen-de...@lists.xenproject.org; Max Reitz
> > 
> > Subject: Re: [Xen-devel] [Qemu-devel] xen_disk qdevification
> >
> > > -Original Message-
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Sent: 08 November 2018 15:21
> > > To: Paul Durrant 
> > > Cc: 'Markus Armbruster' ; Anthony Perard
> > > ; Tim Smith ; Stefano
> > > Stabellini ; qemu-bl...@nongnu.org; qemu-
> > > de...@nongnu.org; Max Reitz ; xen-
> > > de...@lists.xenproject.org
> > > Subject: Re: [Qemu-devel] xen_disk qdevification
> > >
> > > Am 08.11.2018 um 15:00 hat Paul Durrant geschrieben:
> > > > > -Original Message-
> > > > > From: Markus Armbruster [mailto:arm...@redhat.com]
> > > > > Sent: 05 November 2018 15:58
> > > > > To: Paul Durrant 
> > > > > Cc: 'Kevin Wolf' ; Tim Smith
> > ;
> > > > > Stefano Stabellini ; qemu-
> bl...@nongnu.org;
> > > qemu-
> > > > > de...@nongnu.org; Max Reitz ; Anthony Perard
> > > > > ; xen-de...@lists.xenproject.org
> > > > > Subject: Re: [Qemu-devel] xen_disk qdevification
> > > > >
> > > > > Paul Durrant  writes:
> > > > >
> > > > > >> -Original Message-
> > > > > >> From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > >> Sent: 02 November 2018 11:04
> > > > > >> To: Tim Smith 
> > > > > >> Cc: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org;
> qemu-
> > > > > >> bl...@nongnu.org; Anthony Perard ;
> > Paul
> > > > > Durrant
> > > > > >> ; Stefano Stabellini
> > > ;
> > > > > >> Max Reitz ; arm...@redhat.com
> > > > > >> Subject: xen_disk qdevification (was: [PATCH 0/3] Performance
> > > > > improvements
> > > > > >> for xen_disk v2)
> > > > > >>
> > > > > >> Am 02.11.2018 um 11:00 hat Tim Smith geschrieben:
> > > > > >> > A series of performance improvements for disks using the Xen
> PV
> > > ring.
> > > > > >> >
> > > > > >> > These have had fairly extensive testing.
> > > > > >> >
> > > > > >> > The batching and latency improvements together boost the
> > > throughput
> > > > > >> > of small reads and writes by two to six percent (measured
> using
> > > fio
> > > > > >> > in the guest)
> > > > > >> >
> > > > > >> > Avoiding repeated calls to posix_memalign() reduced the dirty
> > > heap
> > > > > >> > from 25MB to 5MB in the case of a single datapath process
> while
> > > also
> > > > > >> > improving performance.
> > > > > >> >
> > > > > >> > v2 removes some checkpatch complaints and fixes the CCs
> > > > > >>
> > > > > >> Completely unrelated, but since you're the first person
> touching
> > > > > >> xen_disk in a while, you're my victim:
> > > > > >>
> > > > > >> At KVM Forum we discussed sending a patch to deprecate xen_disk
> > > because
> > > > > >> after all those years, it still hasn't been converted to qdev.
> > > Markus
> > > > > is
> > > > > >> currently fixing some other not yet qdevified block device, but
> > > after
> > > > > >> that xen_disk will be the only one left.
> > > > > >>
> > > > > >> A while ago, a downstream patch review found out that there are
> > > some
> > > > > QMP
> > > > > >> commands that would immediately crash if a xen_disk device were
> > > present
> > > > > >> because of the lacking qdevification. This is not the code
> > quality
> > > > > >> standard I envision for QEMU. It's time for non-qdev devices to
> > go.
> > > > > >>
> > > > > >> So if you guys are still interested in the device, could
> someone
> > > please
> > > > > >> finally look into converting it?
> > > > > >>
> > > > > >
> > > > > > I have a patch series to do exactly this. It's somewhat involved
> > as
> > > I
> > > > > > need to convert the whole PV backend infrastructure. I will try
> to
> > > > > > rebase and clean up my series a.s.a.p.
> > > > >
> > > > > Awesome!  Please coordinate with Anthony Prerard to avoid
> > duplicating
> > > > > work if you haven't done so already.
> > > >
> > > > I've come across a bit of a problem that I'm not sure how best to
> deal
> > > > with and so am looking for some advice.
> > > >
> > > > I now have a qdevified PV disk backend but I can't bring it up
> because
> > > > it fails to acquire a write lock on the qcow2 it is pointing at.
> This
> > > > is because there is also an emulated IDE drive using the same qcow2.
> > > > This does not appear to be a problem for the non-qdev xen-disk,
> > > > presumably because it is not opening the qcow2 until the emulated
> > > > 

[Qemu-devel] [PATCH RFC 4/6] test-string-input-visitor: use virtual walk

2018-11-09 Thread David Hildenbrand
We now support virtual walks, so use that instead.

Signed-off-by: David Hildenbrand 
---
 tests/test-string-input-visitor.c | 36 +++
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/tests/test-string-input-visitor.c 
b/tests/test-string-input-visitor.c
index 0a4152f79d..2f6360e9ca 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -114,7 +114,6 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 uint64_t expect4[] = { UINT64_MAX };
 Error *err = NULL;
 int64List *res = NULL;
-int64List *tail;
 Visitor *v;
 int64_t val;
 
@@ -151,39 +150,28 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 
 v = visitor_input_test_init(data, "0,2-3");
 
-/* Would be simpler if the visitor genuinely supported virtual walks */
-visit_start_list(v, NULL, (GenericList **), sizeof(*res),
- _abort);
-tail = res;
-visit_type_int64(v, NULL, >value, _abort);
-g_assert_cmpint(tail->value, ==, 0);
-tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res));
-g_assert(tail);
-visit_type_int64(v, NULL, >value, _abort);
-g_assert_cmpint(tail->value, ==, 2);
-tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res));
-g_assert(tail);
+visit_start_list(v, NULL, NULL, 0, _abort);
+visit_type_int64(v, NULL, , _abort);
+g_assert_cmpint(val, ==, 0);
+visit_type_int64(v, NULL, , _abort);
+g_assert_cmpint(val, ==, 2);
 
 visit_check_list(v, );
 error_free_or_abort();
-visit_end_list(v, (void **));
-
-qapi_free_int64List(res);
+visit_end_list(v, NULL);
 
 /* Visit beyond end of list */
+
 v = visitor_input_test_init(data, "0");
 
-visit_start_list(v, NULL, (GenericList **), sizeof(*res),
- _abort);
-tail = res;
-visit_type_int64(v, NULL, >value, );
-g_assert_cmpint(tail->value, ==, 0);
+visit_start_list(v, NULL, NULL, 0, _abort);
+visit_type_int64(v, NULL, , );
+g_assert_cmpint(val, ==, 0);
 visit_type_int64(v, NULL, , );
 error_free_or_abort();
-visit_check_list(v, _abort);
-visit_end_list(v, (void **));
 
-qapi_free_int64List(res);
+visit_check_list(v, _abort);
+visit_end_list(v, NULL);
 }
 
 static void test_visitor_in_bool(TestInputVisitorData *data,
-- 
2.17.2




[Qemu-devel] [PATCH RFC 1/6] cutils: add qemu_strtod()

2018-11-09 Thread David Hildenbrand
Let's provide a wrapper for strtod().

Signed-off-by: David Hildenbrand 
---
 include/qemu/cutils.h |  1 +
 util/cutils.c | 22 ++
 2 files changed, 23 insertions(+)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 7071bfe2d4..84fb9e53c6 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -146,6 +146,7 @@ int qemu_strtoi64(const char *nptr, const char **endptr, 
int base,
   int64_t *result);
 int qemu_strtou64(const char *nptr, const char **endptr, int base,
   uint64_t *result);
+int qemu_strtod(const char *nptr, const char **endptr, double *result);
 
 int parse_uint(const char *s, unsigned long long *value, char **endptr,
int base);
diff --git a/util/cutils.c b/util/cutils.c
index 698bd315bd..850e3af9ea 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -544,6 +544,28 @@ int qemu_strtou64(const char *nptr, const char **endptr, 
int base,
 return check_strtox_error(nptr, ep, endptr, errno);
 }
 
+/**
+ * Convert string @nptr to a double.
+ *
+ * Works like qemu_strtoul(), except it stores +/-HUGE_VAL on
+ * overflow/underflow.
+ */
+int qemu_strtod(const char *nptr, const char **endptr, double *result)
+{
+char *ep;
+
+if (!nptr) {
+if (endptr) {
+*endptr = nptr;
+}
+return -EINVAL;
+}
+
+errno = 0;
+*result = strtod(nptr, );
+return check_strtox_error(nptr, ep, endptr, errno);
+}
+
 /**
  * Searches for the first occurrence of 'c' in 's', and returns a pointer
  * to the trailing null byte if none was found.
-- 
2.17.2




Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-09 Thread Gerd Hoffmann
  Hi,

> I am also suspicious about hw/bt/ but don't know enough
> about that subsystem to say if it could benefit from
> using QOM objects more.

I'm wondering whenever anyone would even notice if we just rm -rf hw/bt

Looking through the changelog for the last five years (after hw/ split)
the only thing I see is fixing warnings from compiler or coverity,
adapting to changes in other systems (chardev for example) and treewide
changes.  Not a *single* patch specific to bluetooth ...

cheers,
  Gerd




[Qemu-devel] [RFC v8 08/18] virtio-iommu: Implement map/unmap

2018-11-09 Thread Eric Auger
This patch implements virtio_iommu_map/unmap.

Signed-off-by: Eric Auger 

---

v5 -> v6:
- use new v0.6 fields
- replace error_report by qemu_log_mask

v3 -> v4:
- implement unmap semantics as specified in v0.4
---
 hw/virtio/trace-events   |  3 ++
 hw/virtio/virtio-iommu.c | 94 +++-
 2 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 4b15086872..b3c5c2604e 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -65,3 +65,6 @@ virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
 virtio_iommu_put_endpoint(uint32_t ep_id) "Free endpoint=%d"
 virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
 virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
+virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, uint64_t 
next_low, uint64_t next_high) "Unmap left [0x%"PRIx64",0x%"PRIx64"], new 
interval=[0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t 
next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"], new 
interval=[0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc 
[0x%"PRIx64",0x%"PRIx64"]"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index bd245ea979..1a1ed4d3d3 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "qemu/iov.h"
 #include "qemu-common.h"
 #include "hw/virtio/virtio.h"
@@ -57,6 +58,13 @@ typedef struct viommu_interval {
 uint64_t high;
 } viommu_interval;
 
+typedef struct viommu_mapping {
+uint64_t virt_addr;
+uint64_t phys_addr;
+uint64_t size;
+uint32_t flags;
+} viommu_mapping;
+
 static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
 {
 return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
@@ -244,10 +252,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
 uint64_t virt_start = le64_to_cpu(req->virt_start);
 uint64_t virt_end = le64_to_cpu(req->virt_end);
 uint32_t flags = le32_to_cpu(req->flags);
+viommu_domain *domain;
+viommu_interval *interval;
+viommu_mapping *mapping;
+
+interval = g_malloc0(sizeof(*interval));
+
+interval->low = virt_start;
+interval->high = virt_end;
+
+domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+if (!domain) {
+return VIRTIO_IOMMU_S_NOENT;
+}
+
+mapping = g_tree_lookup(domain->mappings, (gpointer)interval);
+if (mapping) {
+g_free(interval);
+return VIRTIO_IOMMU_S_INVAL;
+}
 
 trace_virtio_iommu_map(domain_id, virt_start, virt_end, phys_start, flags);
 
-return VIRTIO_IOMMU_S_UNSUPP;
+mapping = g_malloc0(sizeof(*mapping));
+mapping->virt_addr = virt_start;
+mapping->phys_addr = phys_start;
+mapping->size = virt_end - virt_start + 1;
+mapping->flags = flags;
+
+g_tree_insert(domain->mappings, interval, mapping);
+
+return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_unmap(VirtIOIOMMU *s,
@@ -256,10 +291,65 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
 uint32_t domain_id = le32_to_cpu(req->domain);
 uint64_t virt_start = le64_to_cpu(req->virt_start);
 uint64_t virt_end = le64_to_cpu(req->virt_end);
+uint64_t size = virt_end - virt_start + 1;
+viommu_mapping *mapping;
+viommu_interval interval;
+viommu_domain *domain;
 
 trace_virtio_iommu_unmap(domain_id, virt_start, virt_end);
 
-return VIRTIO_IOMMU_S_UNSUPP;
+domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+if (!domain) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: no domain\n", __func__);
+return VIRTIO_IOMMU_S_NOENT;
+}
+interval.low = virt_start;
+interval.high = virt_end;
+
+mapping = g_tree_lookup(domain->mappings, (gpointer)());
+
+while (mapping) {
+viommu_interval current;
+uint64_t low  = mapping->virt_addr;
+uint64_t high = mapping->virt_addr + mapping->size - 1;
+
+current.low = low;
+current.high = high;
+
+if (low == interval.low && size >= mapping->size) {
+g_tree_remove(domain->mappings, (gpointer)());
+interval.low = high + 1;
+trace_virtio_iommu_unmap_left_interval(current.low, current.high,
+interval.low, interval.high);
+} else if (high == interval.high && size >= mapping->size) {
+trace_virtio_iommu_unmap_right_interval(current.low, current.high,
+interval.low, interval.high);
+g_tree_remove(domain->mappings, (gpointer)());
+interval.high = low - 1;
+} else if (low > interval.low && high < interval.high) {
+trace_virtio_iommu_unmap_inc_interval(current.low, current.high);
+g_tree_remove(domain->mappings, (gpointer)());
+} else {
+break;
+  

[Qemu-devel] [RFC v8 00/18] VIRTIO-IOMMU device

2018-11-09 Thread Eric Auger
This series rebases the virtio-iommu device on qemu 3.1.0-rc0
and implements the v0.8 virtio-iommu spec [1]. Most importantly
the pci proxy for the virtio-iommu device is now available and
gets instantiated by the ARM virt machine when the ",iommu=virtio"
virt machine option is added.

The virtio-iommu-pci device always uses the 00:01.0 BDF and is
transparently instantiated by machvirt. The rationale is the
virtio-iommu-pci is not bound to be hotplugged and its BDF must
be known when building the guest device tree or ACPI tables.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/v3.1.0-rc0-virtio-iommu-v0.8

References:
[1] [PATCH v3 0/7] Add virtio-iommu driver

[2] guest branch featuring the virtio-iommu driver v0.8 + ACPI
integration not yet officially released by Jean.
https://github.com/eauger/linux/tree/v4.19-rc7-virtio-iommu-v0.8-iort

Testing:
- tested with guest using virtio-net-pci
  (,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on)
  and virtio-blk-pci
- VFIO/VHOST integration is not part of this series
- When using the virtio-blk-pci, some EDK2 FW versions feature
  unmapped transactions and in that case the guest fails to boot.

History:

v7 -> v8:
- virtio-iommu-pci added
- virt instantiation modified
- DT and ACPI modified to exclude the iommu RID from the mapping
- VIRTIO_IOMMU_F_BYPASS, VIRTIO_F_VERSION_1 features exposed

v6 -> v7:
- rebase on qemu 3.0.0-rc3
- minor update against v0.7
- fix issue with EP not on pci.0 and ACPI probing
- change the instantiation method

v5 -> v6:
- minor update against v0.6 spec
- fix g_hash_table_lookup in virtio_iommu_find_add_as
- replace some error_reports by qemu_log_mask(LOG_GUEST_ERROR, ...)

v4 -> v5:
- event queue and fault reporting
- we now return the IOAPIC MSI region if the virtio-iommu is instantiated
  in a PC machine.
- we bypass transactions on MSI HW region and fault on reserved ones.
- We support ACPI boot with mach-virt (based on IORT proposal)
- We moved to the new driver naming conventions
- simplified mach-virt instantiation
- worked around the disappearing of pci_find_primary_bus
- in virtio_iommu_translate, check the dev->as is not NULL
- initialize as->device_list in virtio_iommu_get_as
- initialize bufstate.error to false in virtio_iommu_probe

v3 -> v4:
- probe request support although no reserved region is returned at
  the moment
- unmap semantics less strict, as specified in v0.4
- device registration, attach/detach revisited
- split into smaller patches to ease review
- propose a way to inform the IOMMU mr about the page_size_mask
  of underlying HW IOMMU, if any
- remove warning associated with the translation of the MSI doorbell

v2 -> v3:
- rebase on top of 2.10-rc0 and especially
  [PATCH qemu v9 0/2] memory/iommu: QOM'fy IOMMU MemoryRegion
- add mutex init
- fix as->mappings deletion using g_tree_ref/unref
- when a dev is attached whereas it is already attached to
  another address space, first detach it
- fix some error values
- page_sizes = TARGET_PAGE_MASK;
- I haven't changed the unmap() semantics yet, waiting for the
  next virtio-iommu spec revision.

v1 -> v2:
- fix redifinition of viommu_as typedef

Eric Auger (18):
  update-linux-headers: Import virtio_iommu.h
  linux-headers: Partial update for virtio-iommu v0.8
  virtio-iommu: Add skeleton
  virtio-iommu: Decode the command payload
  virtio-iommu: Add the iommu regions
  virtio-iommu: Endpoint and domains structs and helpers
  virtio-iommu: Implement attach/detach command
  virtio-iommu: Implement map/unmap
  virtio-iommu: Implement translate
  virtio-iommu: Implement probe request
  virtio-iommu: Add an msi_bypass property
  virtio-iommu: Implement fault reporting
  virtio_iommu: Handle reserved regions in translation process
  virtio-iommu-pci: Add virtio iommu pci support
  hw/arm/virt: Add virtio-iommu to the virt board
  hw/arm/virt-acpi-build: Introduce fill_iort_idmap helper
  hw/arm/virt-acpi-build: Add virtio-iommu node in IORT table
  hw/arm/virt: Allow virtio-iommu instantiation

 hw/arm/virt-acpi-build.c  |   90 +-
 hw/arm/virt.c |   56 +-
 hw/virtio/Makefile.objs   |1 +
 hw/virtio/trace-events|   26 +
 hw/virtio/virtio-iommu.c  | 1064 +
 hw/virtio/virtio-pci.c|   50 +
 hw/virtio/virtio-pci.h|   14 +
 include/hw/acpi/acpi-defs.h   |   21 +-
 include/hw/pci/pci.h  |1 +
 include/hw/virtio/virtio-iommu.h  |   65 +
 include/standard-headers/linux/virtio_ids.h   |1 +
 include/standard-headers/linux/virtio_iommu.h |  159 +++
 linux-headers/linux/virtio_iommu.h|1 +
 qdev-monitor.c|1 +
 scripts/update-linux-headers.sh   |3 +
 15 files changed, 1522 insertions(+), 31 deletions(-)
 create mode 

[Qemu-devel] [RFC v8 02/18] linux-headers: Partial update for virtio-iommu v0.8

2018-11-09 Thread Eric Auger
Partial sync against Jean-Philippe's branch:
git://linux-arm.org/linux-jpb.git virtio-iommu/v0.8

Signed-off-by: Eric Auger 
---
 include/standard-headers/linux/virtio_ids.h   |   1 +
 include/standard-headers/linux/virtio_iommu.h | 159 ++
 linux-headers/linux/virtio_iommu.h|   1 +
 3 files changed, 161 insertions(+)
 create mode 100644 include/standard-headers/linux/virtio_iommu.h
 create mode 100644 linux-headers/linux/virtio_iommu.h

diff --git a/include/standard-headers/linux/virtio_ids.h 
b/include/standard-headers/linux/virtio_ids.h
index 6d5c3b2d4f..cfe47c5d9a 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -43,5 +43,6 @@
 #define VIRTIO_ID_INPUT18 /* virtio input */
 #define VIRTIO_ID_VSOCK19 /* virtio vsock transport */
 #define VIRTIO_ID_CRYPTO   20 /* virtio crypto */
+#define VIRTIO_ID_IOMMU23 /* virtio IOMMU */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/standard-headers/linux/virtio_iommu.h 
b/include/standard-headers/linux/virtio_iommu.h
new file mode 100644
index 00..0a40b21ea9
--- /dev/null
+++ b/include/standard-headers/linux/virtio_iommu.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Virtio-iommu definition v0.8
+ *
+ * Copyright (C) 2018 Arm Ltd.
+ */
+#ifndef _LINUX_VIRTIO_IOMMU_H
+#define _LINUX_VIRTIO_IOMMU_H
+
+#include "standard-headers/linux/types.h"
+
+/* Feature bits */
+#define VIRTIO_IOMMU_F_INPUT_RANGE 0
+#define VIRTIO_IOMMU_F_DOMAIN_BITS 1
+#define VIRTIO_IOMMU_F_MAP_UNMAP   2
+#define VIRTIO_IOMMU_F_BYPASS  3
+#define VIRTIO_IOMMU_F_PROBE   4
+
+struct virtio_iommu_config {
+   /* Supported page sizes */
+   uint64_tpage_size_mask;
+   /* Supported IOVA range */
+   struct virtio_iommu_range {
+   uint64_tstart;
+   uint64_tend;
+   } input_range;
+   /* Max domain ID size */
+   uint8_t domain_bits;
+   uint8_t padding[3];
+   /* Probe buffer size */
+   uint32_tprobe_size;
+};
+
+/* Request types */
+#define VIRTIO_IOMMU_T_ATTACH  0x01
+#define VIRTIO_IOMMU_T_DETACH  0x02
+#define VIRTIO_IOMMU_T_MAP 0x03
+#define VIRTIO_IOMMU_T_UNMAP   0x04
+#define VIRTIO_IOMMU_T_PROBE   0x05
+
+/* Status types */
+#define VIRTIO_IOMMU_S_OK  0x00
+#define VIRTIO_IOMMU_S_IOERR   0x01
+#define VIRTIO_IOMMU_S_UNSUPP  0x02
+#define VIRTIO_IOMMU_S_DEVERR  0x03
+#define VIRTIO_IOMMU_S_INVAL   0x04
+#define VIRTIO_IOMMU_S_RANGE   0x05
+#define VIRTIO_IOMMU_S_NOENT   0x06
+#define VIRTIO_IOMMU_S_FAULT   0x07
+
+struct virtio_iommu_req_head {
+   uint8_t type;
+   uint8_t reserved[3];
+};
+
+struct virtio_iommu_req_tail {
+   uint8_t status;
+   uint8_t reserved[3];
+};
+
+struct virtio_iommu_req_attach {
+   struct virtio_iommu_req_headhead;
+   uint32_tdomain;
+   uint32_tendpoint;
+   uint8_t reserved[8];
+   struct virtio_iommu_req_tailtail;
+};
+
+struct virtio_iommu_req_detach {
+   struct virtio_iommu_req_headhead;
+   uint32_tdomain;
+   uint32_tendpoint;
+   uint8_t reserved[8];
+   struct virtio_iommu_req_tailtail;
+};
+
+#define VIRTIO_IOMMU_MAP_F_READ(1 << 0)
+#define VIRTIO_IOMMU_MAP_F_WRITE   (1 << 1)
+#define VIRTIO_IOMMU_MAP_F_EXEC(1 << 2)
+#define VIRTIO_IOMMU_MAP_F_MMIO(1 << 3)
+
+#define VIRTIO_IOMMU_MAP_F_MASK
(VIRTIO_IOMMU_MAP_F_READ |  \
+VIRTIO_IOMMU_MAP_F_WRITE | 
\
+VIRTIO_IOMMU_MAP_F_EXEC |  
\
+VIRTIO_IOMMU_MAP_F_MMIO)
+
+struct virtio_iommu_req_map {
+   struct virtio_iommu_req_headhead;
+   uint32_tdomain;
+   uint64_tvirt_start;
+   uint64_tvirt_end;
+   uint64_t

[Qemu-devel] [RFC v8 03/18] virtio-iommu: Add skeleton

2018-11-09 Thread Eric Auger
This patchs adds the skeleton for the virtio-iommu device.

Signed-off-by: Eric Auger 

---
v7 -> v8:
- expose VIRTIO_IOMMU_F_BYPASS and VIRTIO_F_VERSION_1
  features
- set_config dummy implementation + tracing
- add trace in get_features
- set the features on realize() and store the acked ones
- remove inclusion of linux/virtio_iommu.h

v6 -> v7:
- removed qapi-event.h include
- add primary_bus and associated property

v4 -> v5:
- use the new v0.5 terminology (domain, endpoint)
- add the event virtqueue

v3 -> v4:
- use page_size_mask instead of page_sizes
- added set_features()
- added some traces (reset, set_status, set_features)
- empty virtio_iommu_set_config() as the driver MUST NOT
  write to device configuration fields
- add get_config trace

v2 -> v3:
- rebase on 2.10-rc0, ie. use IOMMUMemoryRegion and remove
  iommu_ops.
- advertise VIRTIO_IOMMU_F_MAP_UNMAP feature
- page_sizes set to TARGET_PAGE_SIZE

Conflicts:
hw/virtio/trace-events
---
 hw/virtio/Makefile.objs  |   1 +
 hw/virtio/trace-events   |   9 +
 hw/virtio/virtio-iommu.c | 273 +++
 include/hw/virtio/virtio-iommu.h |  62 +++
 4 files changed, 345 insertions(+)
 create mode 100644 hw/virtio/virtio-iommu.c
 create mode 100644 include/hw/virtio/virtio-iommu.h

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 1b2799cfd8..49eba67a54 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -10,6 +10,7 @@ obj-$(CONFIG_VIRTIO_CRYPTO) += virtio-crypto.o
 obj-$(call land,$(CONFIG_VIRTIO_CRYPTO),$(CONFIG_VIRTIO_PCI)) += 
virtio-crypto-pci.o
 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
+obj-$(CONFIG_LINUX) += virtio-iommu.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
 endif
 
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 07bcbe9e85..84da904d8b 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -46,3 +46,12 @@ virtio_balloon_handle_output(const char *name, uint64_t gpa) 
"section name: %s g
 virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: %d 
actual: %d"
 virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d 
oldactual: %d"
 virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: 
0x%"PRIx64" num_pages: %d"
+
+# hw/virtio/virtio-iommu.c
+#
+virtio_iommu_device_reset(void) "reset!"
+virtio_iommu_get_features(uint64_t features) "device supports 
features=0x%"PRIx64
+virtio_iommu_set_features(uint64_t features) "features accepted by the driver 
=0x%"PRIx64
+virtio_iommu_device_status(uint8_t status) "driver status = %d"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, 
uint8_t domain_bits, uint32_t probe_size) "page_size_mask=0x%"PRIx64" 
start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
+virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, 
uint8_t domain_bits, uint32_t probe_size) "page_size_mask=0x%"PRIx64" 
start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
new file mode 100644
index 00..cf7dee128a
--- /dev/null
+++ b/hw/virtio/virtio-iommu.c
@@ -0,0 +1,273 @@
+/*
+ * virtio-iommu device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu-common.h"
+#include "hw/virtio/virtio.h"
+#include "sysemu/kvm.h"
+#include "trace.h"
+
+#include "standard-headers/linux/virtio_ids.h"
+
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-iommu.h"
+
+/* Max size */
+#define VIOMMU_DEFAULT_QUEUE_SIZE 256
+
+static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
+  struct iovec *iov,
+  unsigned int iov_cnt)
+{
+return -ENOENT;
+}
+static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
+  struct iovec *iov,
+  unsigned int iov_cnt)
+{
+return -ENOENT;
+}
+static int virtio_iommu_handle_map(VirtIOIOMMU *s,
+   struct iovec *iov,
+   unsigned int iov_cnt)
+{
+return -ENOENT;
+}
+static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
+   

[Qemu-devel] [RFC v8 11/18] virtio-iommu: Add an msi_bypass property

2018-11-09 Thread Eric Auger
In case the msi_bypass property is set, it means we need
to register the IOAPIC MSI window as a reserved region:
0xFEE0 - 0xFEEF.

Signed-off-by: Eric Auger 

---
---
 hw/virtio/virtio-iommu.c | 52 
 include/hw/virtio/virtio-iommu.h |  1 +
 2 files changed, 53 insertions(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 6164b5a7d1..e6ad8a0c61 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -41,6 +41,9 @@
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 #define VIOMMU_PROBE_SIZE 512
 
+#define IOAPIC_RANGE_START  (0xfee0)
+#define IOAPIC_RANGE_SIZE   (0x10)
+
 #define SUPPORTED_PROBE_PROPERTIES (\
 VIRTIO_IOMMU_PROBE_T_NONE | \
 VIRTIO_IOMMU_PROBE_T_RESV_MEM)
@@ -103,6 +106,25 @@ static void 
virtio_iommu_detach_endpoint_from_domain(viommu_endpoint *ep)
 ep->domain = NULL;
 }
 
+static void virtio_iommu_register_resv_region(viommu_endpoint *ep,
+  uint8_t subtype,
+  uint64_t start, uint64_t end)
+{
+viommu_interval *interval;
+struct virtio_iommu_probe_resv_mem *reg;
+
+interval = g_malloc0(sizeof(*interval));
+interval->low = start;
+interval->high = end;
+
+reg = g_malloc0(sizeof(*reg));
+reg->subtype = subtype;
+reg->start = cpu_to_le64(start);
+reg->end = cpu_to_le64(end);
+
+g_tree_insert(ep->reserved_regions, interval, reg);
+}
+
 static viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
   uint32_t ep_id)
 {
@@ -120,6 +142,12 @@ static viommu_endpoint 
*virtio_iommu_get_endpoint(VirtIOIOMMU *s,
 ep->reserved_regions = g_tree_new_full((GCompareDataFunc)interval_cmp,
 NULL, (GDestroyNotify)g_free,
 (GDestroyNotify)g_free);
+if (s->msi_bypass) {
+virtio_iommu_register_resv_region(ep, VIRTIO_IOMMU_RESV_MEM_T_MSI,
+  IOAPIC_RANGE_START,
+  IOAPIC_RANGE_SIZE);
+}
+
 return ep;
 }
 
@@ -864,8 +892,32 @@ static void virtio_iommu_set_status(VirtIODevice *vdev, 
uint8_t status)
 trace_virtio_iommu_device_status(status);
 }
 
+static bool virtio_iommu_get_msi_bypass(Object *obj, Error **errp)
+{
+VirtIOIOMMU *s = VIRTIO_IOMMU(obj);
+
+return s->msi_bypass;
+}
+
+static void virtio_iommu_set_msi_bypass(Object *obj, bool value, Error **errp)
+{
+VirtIOIOMMU *s = VIRTIO_IOMMU(obj);
+
+s->msi_bypass = value;
+}
+
 static void virtio_iommu_instance_init(Object *obj)
 {
+VirtIOIOMMU *s = VIRTIO_IOMMU(obj);
+
+object_property_add_bool(obj, "msi_bypass", virtio_iommu_get_msi_bypass,
+ virtio_iommu_set_msi_bypass, NULL);
+object_property_set_description(obj, "msi_bypass",
+"Indicates whether msis are bypassed by "
+"the IOMMU. Default is YES",
+NULL);
+
+s->msi_bypass = true;
 }
 
 static const VMStateDescription vmstate_virtio_iommu = {
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index f55f48d304..56c8b4e57f 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -59,6 +59,7 @@ typedef struct VirtIOIOMMU {
 GTree *domains;
 QemuMutex mutex;
 GTree *endpoints;
+bool msi_bypass;
 } VirtIOIOMMU;
 
 #endif
-- 
2.17.2




[Qemu-devel] [RFC v8 15/18] hw/arm/virt: Add virtio-iommu to the virt board

2018-11-09 Thread Eric Auger
Both the virtio-iommu device and its dedicated mmio
transport get instantiated when requested.

Signed-off-by: Eric Auger 

---

v6 -> v7:
- align to the smmu instantiation code

v4 -> v5:
- VirtMachineClass no_iommu added in this patch
- Use object_resolve_path_type
---
 hw/arm/virt.c | 48 +---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a2b8d8f7c2..f2994c4359 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -29,6 +29,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "monitor/qdev.h"
 #include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "hw/arm/arm.h"
@@ -49,6 +50,7 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/arm/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/arm/fdt.h"
@@ -59,6 +61,7 @@
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 #include "hw/arm/smmuv3.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
 static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1085,6 +1088,33 @@ static void create_smmu(const VirtMachineState *vms, 
qemu_irq *pic,
 g_free(node);
 }
 
+static void create_virtio_iommu(VirtMachineState *vms,
+const char *pciehb_nodename, PCIBus *bus)
+{
+const char compat[] = "virtio,pci-iommu";
+uint16_t bdf = 0x8; /* 00:01.0 */
+DeviceState *dev;
+char *node;
+
+dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
+object_property_set_bool(OBJECT(dev), true, "realized", _fatal);
+
+node = g_strdup_printf("%s/virtio_iommu@%d", pciehb_nodename, bdf);
+qemu_fdt_add_subnode(vms->fdt, node);
+qemu_fdt_setprop(vms->fdt, node, "compatible", compat, sizeof(compat));
+qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
+ 1, bdf << 8 /* phys.hi */,
+ 1, 0/* phys.mid */,
+ 1, 0/* phys.lo  */,
+ 1, 0/* size.hi  */,
+ 1, 0/* size.low */);
+
+qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
+qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms->iommu_phandle);
+g_free(node);
+}
+
+
 static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
 {
 hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
@@ -1205,10 +1235,22 @@ static void create_pcie(VirtMachineState *vms, qemu_irq 
*pic)
 if (vms->iommu) {
 vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
 
-create_smmu(vms, pic, pci->bus);
+switch (vms->iommu) {
+case VIRT_IOMMU_SMMUV3:
+create_smmu(vms, pic, pci->bus);
+qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
+   0x0, vms->iommu_phandle, 0x0, 0x1);
+break;
+case VIRT_IOMMU_VIRTIO:
+create_virtio_iommu(vms, nodename, pci->bus);
+qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
+   0x0, vms->iommu_phandle, 0x0, 0x8,
+   0x9, vms->iommu_phandle, 0x9, 0xfff7);
+break;
+default:
+g_assert_not_reached();
+}
 
-qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
-   0x0, vms->iommu_phandle, 0x0, 0x1);
 }
 
 g_free(nodename);
-- 
2.17.2




Re: [Qemu-devel] [PATCH 0/2] linux-user/mips: Support the n32 ABI for the R5900

2018-11-09 Thread Laurent Vivier
On 08/11/2018 19:42, Fredrik Noring wrote:
> Recognise the R5900, which reports itself as MIPS III, as a 64-bit CPU
> supporting the n32 ABI. Test that DMULT is emulated in user mode.

if you have time, o32 & n32 support needs to be reworked.

We have two binaries qemu-mips and qemu-mipsn32 sharing the same ELF
mask/magic.

As n32 identifies a kernel ABI version, we should have only one binary
for qemu-mips and qemu-mipsn32 and the ABI version should be identified
at runtime as it is done for ARM:

  ce4defa062 Arm Linux EABI syscall support.

[I think we can use e_flags for that]

I never did the change because I don't know where to find mipsn32
binaries to test my changes.

Thanks,
Laurent



[Qemu-devel] [PATCH RFC 3/6] qapi: rewrite string-input-visitor

2018-11-09 Thread David Hildenbrand
The input visitor has some problems right now, especially
- unsigned type "Range" is used to process signed ranges, resulting in
  inconsistent behavior and ugly/magical code
- uint64_t are parsed like int64_t, so big uint64_t values are not
  supported and error messages are misleading
- lists/ranges of int64_t are accepted although no list is parsed and
  we should rather report an error
- lists/ranges are preparsed using int64_t, making it hard to
  implement uint64_t values or uint64_t lists
- types that don't support lists don't bail out

So let's rewrite it by getting rid of usage of the type "Range" and
properly supporting lists of int64_t and uint64_t (including ranges of
both types), fixing the above mentioned issues.

Lists of other types are not supported and will properly report an
error. Virtual walks are now supported.

Tests have to be fixed up:
- Two BUGs were hardcoded that are fixed now
- The string-input-visitor now actually returns a parsed list and not
  an ordered set.

While at it, do some smaller style changes (e.g. use g_assert).

Signed-off-by: David Hildenbrand 
---
 include/qapi/string-input-visitor.h |   3 +-
 qapi/string-input-visitor.c | 436 +---
 tests/test-string-input-visitor.c   |  18 +-
 3 files changed, 264 insertions(+), 193 deletions(-)

diff --git a/include/qapi/string-input-visitor.h 
b/include/qapi/string-input-visitor.h
index 33551340e3..2c40f7f5a6 100644
--- a/include/qapi/string-input-visitor.h
+++ b/include/qapi/string-input-visitor.h
@@ -19,8 +19,7 @@ typedef struct StringInputVisitor StringInputVisitor;
 
 /*
  * The string input visitor does not implement support for visiting
- * QAPI structs, alternates, null, or arbitrary QTypes.  It also
- * requires a non-null list argument to visit_start_list().
+ * QAPI structs, alternates, null, or arbitrary QTypes.
  */
 Visitor *string_input_visitor_new(const char *str);
 
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index dee708d384..16da47409e 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -4,10 +4,10 @@
  * Copyright Red Hat, Inc. 2012-2016
  *
  * Author: Paolo Bonzini 
+ * David Hildenbrand 
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
  * See the COPYING.LIB file in the top-level directory.
- *
  */
 
 #include "qemu/osdep.h"
@@ -18,21 +18,39 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qnull.h"
 #include "qemu/option.h"
-#include "qemu/queue.h"
-#include "qemu/range.h"
 #include "qemu/cutils.h"
 
+typedef enum ListMode {
+/* no list parsing active / no list expected */
+LM_NONE,
+/* we have an unparsed string remaining */
+LM_UNPARSED,
+/* we have an unfinished int64 range */
+LM_INT64_RANGE,
+/* we have an unfinished uint64 range */
+LM_UINT64_RANGE,
+/* we have parsed the string completely and no range is remaining */
+LM_END,
+} ListMode;
+
+typedef union RangeLimit {
+int64_t i64;
+uint64_t u64;
+} RangeLimit;
 
 struct StringInputVisitor
 {
 Visitor visitor;
 
-GList *ranges;
-GList *cur_range;
-int64_t cur;
+/* Porperties related to list processing */
+ListMode lm;
+RangeLimit rangeNext;
+RangeLimit rangeEnd;
+const char *unparsed_string;
+void *list;
 
+/* the original string to parse */
 const char *string;
-void *list; /* Only needed for sanity checking the caller */
 };
 
 static StringInputVisitor *to_siv(Visitor *v)
@@ -40,136 +58,48 @@ static StringInputVisitor *to_siv(Visitor *v)
 return container_of(v, StringInputVisitor, visitor);
 }
 
-static void free_range(void *range, void *dummy)
-{
-g_free(range);
-}
-
-static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
-{
-char *str = (char *) siv->string;
-long long start, end;
-Range *cur;
-char *endptr;
-
-if (siv->ranges) {
-return 0;
-}
-
-if (!*str) {
-return 0;
-}
-
-do {
-errno = 0;
-start = strtoll(str, , 0);
-if (errno == 0 && endptr > str) {
-if (*endptr == '\0') {
-cur = g_malloc0(sizeof(*cur));
-range_set_bounds(cur, start, start);
-siv->ranges = range_list_insert(siv->ranges, cur);
-cur = NULL;
-str = NULL;
-} else if (*endptr == '-') {
-str = endptr + 1;
-errno = 0;
-end = strtoll(str, , 0);
-if (errno == 0 && endptr > str && start <= end &&
-(start > INT64_MAX - 65536 ||
- end < start + 65536)) {
-if (*endptr == '\0') {
-cur = g_malloc0(sizeof(*cur));
-range_set_bounds(cur, start, end);
-siv->ranges = range_list_insert(siv->ranges, cur);
-cur = NULL;
-   

Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-09 Thread Gerd Hoffmann
On Fri, Nov 09, 2018 at 12:17:31PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > I am also suspicious about hw/bt/ but don't know enough
> > about that subsystem to say if it could benefit from
> > using QOM objects more.
> 
> I'm wondering whenever anyone would even notice if we just rm -rf hw/bt
> 
> Looking through the changelog for the last five years (after hw/ split)
> the only thing I see is fixing warnings from compiler or coverity,
> adapting to changes in other systems (chardev for example) and treewide
> changes.  Not a *single* patch specific to bluetooth ...

Tried this after studying docs:

  qemu -usb -device usb-bt-dongle -bt hci,vlan=0 -bt device:keyboard

Segfaults right anway on first keypress.
I guess that qualifies as "broken and obviously unused".

cheers,
  Gerd




[Qemu-devel] [RFC v8 06/18] virtio-iommu: Endpoint and domains structs and helpers

2018-11-09 Thread Eric Auger
This patch introduce domain and endpoint internal
datatypes. Both are stored in RB trees. The domain
owns a list of endpoints attached to it.

Helpers to get/put end points and domains are introduced.
get() helpers will become static in subsequent patches.

Signed-off-by: Eric Auger 

---

v6 -> v7:
- on virtio_iommu_find_add_as the bus number computation may
  not be finalized yet so we cannot register the EPs at that time.
  Hence, let's remove the get_endpoint and also do not use the
  bus number for building the memory region name string (only
  used for debug though).

v4 -> v5:
- initialize as->endpoint_list

v3 -> v4:
- new separate patch
---
 hw/virtio/trace-events   |   4 ++
 hw/virtio/virtio-iommu.c | 125 ++-
 2 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 9270b0463e..4b15086872 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -61,3 +61,7 @@ virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, 
uint64_t virt_end, uin
 virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) 
"domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
 virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int 
flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
 virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
+virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
+virtio_iommu_put_endpoint(uint32_t ep_id) "Free endpoint=%d"
+virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
+virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 2043623b30..e4014a8800 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -33,20 +33,124 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-iommu.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci.h"
 
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+typedef struct viommu_domain {
+uint32_t id;
+GTree *mappings;
+QLIST_HEAD(, viommu_endpoint) endpoint_list;
+} viommu_domain;
+
+typedef struct viommu_endpoint {
+uint32_t id;
+viommu_domain *domain;
+QLIST_ENTRY(viommu_endpoint) next;
+VirtIOIOMMU *viommu;
+} viommu_endpoint;
+
+typedef struct viommu_interval {
+uint64_t low;
+uint64_t high;
+} viommu_interval;
+
 static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
 {
 return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
 }
 
+static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+viommu_interval *inta = (viommu_interval *)a;
+viommu_interval *intb = (viommu_interval *)b;
+
+if (inta->high <= intb->low) {
+return -1;
+} else if (intb->high <= inta->low) {
+return 1;
+} else {
+return 0;
+}
+}
+
+static void virtio_iommu_detach_endpoint_from_domain(viommu_endpoint *ep)
+{
+QLIST_REMOVE(ep, next);
+ep->domain = NULL;
+}
+
+viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id);
+viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t ep_id)
+{
+viommu_endpoint *ep;
+
+ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+if (ep) {
+return ep;
+}
+ep = g_malloc0(sizeof(*ep));
+ep->id = ep_id;
+ep->viommu = s;
+trace_virtio_iommu_get_endpoint(ep_id);
+g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
+return ep;
+}
+
+static void virtio_iommu_put_endpoint(gpointer data)
+{
+viommu_endpoint *ep = (viommu_endpoint *)data;
+
+if (ep->domain) {
+virtio_iommu_detach_endpoint_from_domain(ep);
+g_tree_unref(ep->domain->mappings);
+}
+
+trace_virtio_iommu_put_endpoint(ep->id);
+g_free(ep);
+}
+
+viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id);
+viommu_domain *virtio_iommu_get_domain(VirtIOIOMMU *s, uint32_t domain_id)
+{
+viommu_domain *domain;
+
+domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+if (domain) {
+return domain;
+}
+domain = g_malloc0(sizeof(*domain));
+domain->id = domain_id;
+domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
+   NULL, (GDestroyNotify)g_free,
+   (GDestroyNotify)g_free);
+g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
+QLIST_INIT(>endpoint_list);
+trace_virtio_iommu_get_domain(domain_id);
+return domain;
+}
+
+static void virtio_iommu_put_domain(gpointer data)
+{
+viommu_domain *domain = (viommu_domain *)data;
+viommu_endpoint *iter, *tmp;
+
+QLIST_FOREACH_SAFE(iter, >endpoint_list, next, tmp) {
+virtio_iommu_detach_endpoint_from_domain(iter);
+}
+g_tree_destroy(domain->mappings);
+trace_virtio_iommu_put_domain(domain->id);
+g_free(domain);
+}
+

Re: [Qemu-devel] [PATCH v5 13/24] hw: acpi: Do not create hotplug method when handler is not defined

2018-11-09 Thread Igor Mammedov
On Mon,  5 Nov 2018 02:40:36 +0100
Samuel Ortiz  wrote:

> CPU and memory ACPI hotplug are not necessarily handled through SCI
> events. For example, with Hardware-reduced ACPI, the GED device will
> manage ACPI hotplug entirely.
> As a consequence, we make the CPU and memory specific events AML
> generation optional. The code will only be added when the method name is
> not NULL.
patch doesn't belong to this series, it should be go along with
GED device patch.

Suggest to drop it for now.

> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
> Signed-off-by: Samuel Ortiz 
> ---
>  hw/acpi/cpu.c|  8 +---
>  hw/acpi/memory_hotplug.c | 11 +++
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index f10b190019..cd41377b5a 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -569,9 +569,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
> CPUHotplugFeatures opts,
>  aml_append(sb_scope, cpus_dev);
>  aml_append(table, sb_scope);
>  
> -method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> -aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
> -aml_append(table, method);
> +if (event_handler_method) {
> +method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> +aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
> +aml_append(table, method);
> +}
>  
>  g_free(cphp_res_path);
>  }
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 8c7c1013f3..db2c4df961 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -715,10 +715,13 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> nr_mem,
>  }
>  aml_append(table, dev_container);
>  
> -method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> -aml_append(method,
> -aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD));
> -aml_append(table, method);
> +if (event_handler_method) {
> +method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> +aml_append(method,
> +   aml_call0(MEMORY_DEVICES_CONTAINER "."
> + MEMORY_SLOT_SCAN_METHOD));
> +aml_append(table, method);
> +}
>  
>  g_free(mhp_res_path);
>  }




Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Laurent Vivier
On 09/11/2018 09:13, Li Qiang wrote:
> In set_netdev(), the peers[i] is initialized
> qemu_find_net_clients_except() when i is in
> 0 between 'queues' it can't be NULL.
> 
> Signed-off-by: Li Qiang 
> ---
>  hw/core/qdev-properties-system.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 8b22fb5..b45a7ef 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
> char *name,
>  }
>  
>  for (i = 0; i < queues; i++) {
> -if (peers[i] == NULL) {
> -err = -ENOENT;
> -goto out;
> -}
>  
>  if (peers[i]->peer) {
>  err = -EEXIST;
> 


Applied to my trivial-patches branch.

Thanks,
Laurent



Re: [Qemu-devel] [PATCH 1/1 V2] Add vhost-pci-blk driver

2018-11-09 Thread Kevin Wolf
Am 09.11.2018 um 02:48 hat Dongli Zhang geschrieben:
> 
> 
> On 11/09/2018 12:47 AM, Michael S. Tsirkin wrote:
> > On Thu, Nov 08, 2018 at 10:09:00PM +0800, Dongli Zhang wrote:
> >> It looks the kernel space vhost-blk can only process raw image.
> >>
> >> How about to verify that only raw image is used in the drive command line 
> >> when
> >> vhost-blk-pci is paired with it?
> >>
> >> Otherwise, vhost-blk-pci might be working with qcow2 image without any 
> >> warning
> >> on qemu side.
> >>
> >> Dongli Zhang
> > 
> > raw being raw can you really verify that?
> 
> I meant to verify the property 'format=' of '-drive', e.g., to check if
> BlockBackend->root->bs->drv->format_name is raw?

I don't like string comparisons for this, but at least check for "file"
and "host_device" then, because "raw" is a format driver that can be
used above any protocol driver, not just for local files.

Though rather than comparing strings, I'd check bs->drv == _file.

> We allow the user to erroneously give a qcow2 file with 'format=raw'.
> However, if 'format=qcow2' is set explicitly, vhots-blk-pci would exit
> with error as only raw is supported in kernel space.

Please note that the QEMU block layer offers various options even for
file-posix images, and silently ignoring them is probably not very nice.
So just checking the driver is not enough, you also need to check that
no feature is enabled that vhost can't provide.

At first sight, this includes at least detect-zeroes, copy-on-read and
cache.no-flush. There may be more, but this just needs some work to find
all of them. More interesting question: How will we make sure that the
list is updated when we add new options?

Is it actually a good idea to use the block layer at all for this, or
would just taking a filename string be a better idea for this device?
Of course, if you want to do automatic switchover as Stefan suggested,
havin a block node is required and we need to figure out the exact
conditions when we can switch and a way to keep them updated when we
change things.

Kevin



Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Paolo Bonzini
On 09/11/2018 09:29, Laurent Vivier wrote:
> On 09/11/2018 09:26, Thomas Huth wrote:
>> On 2018-11-09 09:21, Laurent Vivier wrote:
>>> On 09/11/2018 09:13, Li Qiang wrote:
 In set_netdev(), the peers[i] is initialized
 qemu_find_net_clients_except() when i is in
 0 between 'queues' it can't be NULL.

 Signed-off-by: Li Qiang 
 ---
  hw/core/qdev-properties-system.c | 4 
  1 file changed, 4 deletions(-)

 diff --git a/hw/core/qdev-properties-system.c 
 b/hw/core/qdev-properties-system.c
 index 8b22fb5..b45a7ef 100644
 --- a/hw/core/qdev-properties-system.c
 +++ b/hw/core/qdev-properties-system.c
 @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
 char *name,
  }
  
  for (i = 0; i < queues; i++) {
 -if (peers[i] == NULL) {
 -err = -ENOENT;
 -goto out;
 -}
  
  if (peers[i]->peer) {
  err = -EEXIST;

>>>
>>> Reviewed-by: Laurent Vivier 
>>
>> So who can pick up such patches? We don't have a maintainer for the QDEV
>> subsystem anymore... should it go via qemu-trivial?
> 
> It seems trivial enough to go via qemu-trivial, but it would be good to
> have a R-b from someone else than me to confirm.

I would have expected Jason to pick it up, after all it's networking,
but it's okay if you want to take it too.

Reviewed-by: Paolo Bonzini 

Thanks,

Paolo



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-09 Thread Mark Cave-Ayland
On 06/11/2018 18:43, Peter Maydell wrote:

> I had an idea for how to get a rough list of source files
> containing devices that haven't been QOMified. The theory
> is that a pre-QOM device generally has an "init" function
> which allocates memory for the device struct. So looking in
> hw/ for files which call g_new*() or g_malloc*() should get
> us all the non-QOM devices (as well as a pile of false
> positives, of course). The following link is the result of
> doing that and then eyeballing the results for false positives
> and throwing those out. It might have missed one or two
> files or included one or two by mistake. But I think it's
> pretty close, and it seems to have caught all the obvious
> ones I knew about. There are 61 files on this list.
> 
> I am also suspicious about hw/bt/ but don't know enough
> about that subsystem to say if it could benefit from
> using QOM objects more.

> hw/ppc/prep.c

I had a quick look at this and AFAICT it's in the old -M prep code which is
deprecated and hopefully scheduled for removal soon, so I think we can leave 
this one.

> hw/sparc64/sparc64.c

This would appear to be from sparc64_cpu_devinit() which sets up a ResetData
structure like this:

reset_info = g_malloc0(sizeof(ResetData));
reset_info->cpu = cpu;
reset_info->prom_addr = prom_addr;
qemu_register_reset(main_cpu_reset, reset_info);

Presumably this can now be handled by using CPUClass's reset() method? The 
prom_addr
field is a per-machine parameter that is used to derive the starting PC for the 
CPU,
so I believe converting this to a qdev parameter on the SPARC CPU should 
suffice.


ATB,

Mark.



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-09 Thread Peter Maydell
On 9 November 2018 at 10:17, Mark Cave-Ayland
 wrote:
> On 06/11/2018 18:43, Peter Maydell wrote:
>
>> I had an idea for how to get a rough list of source files
>> containing devices that haven't been QOMified. The theory
>> is that a pre-QOM device generally has an "init" function
>> which allocates memory for the device struct. So looking in
>> hw/ for files which call g_new*() or g_malloc*() should get
>> us all the non-QOM devices (as well as a pile of false
>> positives, of course).

>> hw/sparc64/sparc64.c
>
> This would appear to be from sparc64_cpu_devinit() which sets up a ResetData
> structure like this:
>
> reset_info = g_malloc0(sizeof(ResetData));
> reset_info->cpu = cpu;
> reset_info->prom_addr = prom_addr;
> qemu_register_reset(main_cpu_reset, reset_info);

I think the code I saw that looked like a non-QOMified
device was cpu_timer_create().

The ResetData stuff is just working around the fact that
CPUs don't get reset unless the board code does something
to arrange for that to happen, so it's OK. (I would like
us to eventually tackle properly overhauling our approach
to reset, but that's a separate body of work.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH 1/2] linux-user/mips: Support the n32 ABI for the R5900

2018-11-09 Thread Laurent Vivier
On 08/11/2018 19:43, Fredrik Noring wrote:
> Recognise the R5900, which reports itself as MIPS III, as a 64-bit CPU
> supporting the n32 ABI.
> 
> Signed-off-by: Fredrik Noring 
> ---
>  linux-user/mips64/target_elf.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/linux-user/mips64/target_elf.h b/linux-user/mips64/target_elf.h
> index ec55d8542a..5f2f2df29f 100644
> --- a/linux-user/mips64/target_elf.h
> +++ b/linux-user/mips64/target_elf.h
> @@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
>  if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_64R6) {
>  return "I6400";
>  }
> +if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
> +return "R5900";
> +}
>  return "5KEf";
>  }
>  #endif
> 

Reviewed-by: Laurent Vivier 




[Qemu-devel] [RFC v8 01/18] update-linux-headers: Import virtio_iommu.h

2018-11-09 Thread Eric Auger
Update the script to update the virtio_iommu.h header.

Signed-off-by: Eric Auger 
---
 scripts/update-linux-headers.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 0a964fe240..55fd271a32 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -159,6 +159,9 @@ fi
 cat <$output/linux-headers/linux/virtio_config.h
 #include "standard-headers/linux/virtio_config.h"
 EOF
+cat <$output/linux-headers/linux/virtio_iommu.h
+#include "standard-headers/linux/virtio_iommu.h"
+EOF
 cat <$output/linux-headers/linux/virtio_ring.h
 #include "standard-headers/linux/virtio_ring.h"
 EOF
-- 
2.17.2




[Qemu-devel] [RFC v8 10/18] virtio-iommu: Implement probe request

2018-11-09 Thread Eric Auger
This patch implements the PROBE request. At the moment,
no reserved regions are returned as none are registered
per device. Only a NONE property is returned.

Signed-off-by: Eric Auger 

---
v7 -> v8:
- adapt to removal of value filed in virtio_iommu_probe_property

v6 -> v7:
- adapt to the change in virtio_iommu_probe_resv_mem fields
- use get_endpoint() instead of directly checking the EP
  was registered.

v4 -> v5:
- initialize bufstate.error to false
- add cpu_to_le64(size)
---
 hw/virtio/trace-events   |   2 +
 hw/virtio/virtio-iommu.c | 189 ++-
 2 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 1f0e143b55..19824c3e91 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -69,3 +69,5 @@ virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, 
uint64_t next_low,
 virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t 
next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"], new 
interval=[0x%"PRIx64",0x%"PRIx64"]"
 virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc 
[0x%"PRIx64",0x%"PRIx64"]"
 virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t 
sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
+virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t 
start, uint64_t end, uint32_t flags, size_t filled) "dev= %d, subtype=%d 
start=0x%"PRIx64" end=0x%"PRIx64" flags=%d filled=0x%lx"
+virtio_iommu_fill_none_property(uint32_t devid) "devid=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 281d6d69c9..6164b5a7d1 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -39,6 +39,11 @@
 
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
+#define VIOMMU_PROBE_SIZE 512
+
+#define SUPPORTED_PROBE_PROPERTIES (\
+VIRTIO_IOMMU_PROBE_T_NONE | \
+VIRTIO_IOMMU_PROBE_T_RESV_MEM)
 
 typedef struct viommu_domain {
 uint32_t id;
@@ -51,6 +56,7 @@ typedef struct viommu_endpoint {
 viommu_domain *domain;
 QLIST_ENTRY(viommu_endpoint) next;
 VirtIOIOMMU *viommu;
+GTree *reserved_regions;
 } viommu_endpoint;
 
 typedef struct viommu_interval {
@@ -65,6 +71,13 @@ typedef struct viommu_mapping {
 uint32_t flags;
 } viommu_mapping;
 
+typedef struct viommu_property_buffer {
+viommu_endpoint *endpoint;
+size_t filled;
+uint8_t *start;
+bool error;
+} viommu_property_buffer;
+
 static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
 {
 return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
@@ -104,6 +117,9 @@ static viommu_endpoint 
*virtio_iommu_get_endpoint(VirtIOIOMMU *s,
 ep->viommu = s;
 trace_virtio_iommu_get_endpoint(ep_id);
 g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
+ep->reserved_regions = g_tree_new_full((GCompareDataFunc)interval_cmp,
+NULL, (GDestroyNotify)g_free,
+(GDestroyNotify)g_free);
 return ep;
 }
 
@@ -117,6 +133,7 @@ static void virtio_iommu_put_endpoint(gpointer data)
 }
 
 trace_virtio_iommu_put_endpoint(ep->id);
+g_tree_destroy(ep->reserved_regions);
 g_free(ep);
 }
 
@@ -352,6 +369,138 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
 return VIRTIO_IOMMU_S_INVAL;
 }
 
+/**
+ * virtio_iommu_fill_resv_mem_prop - Add a RESV_MEM probe
+ * property into the probe request buffer
+ *
+ * @key: interval handle
+ * @value: handle to the reserved memory region
+ * @data: handle to the probe request buffer state
+ */
+static gboolean virtio_iommu_fill_resv_mem_prop(gpointer key,
+gpointer value,
+gpointer data)
+{
+struct virtio_iommu_probe_resv_mem *resv =
+(struct virtio_iommu_probe_resv_mem *)value;
+struct virtio_iommu_probe_property *prop;
+struct virtio_iommu_probe_resv_mem *current;
+viommu_property_buffer *bufstate = (viommu_property_buffer *)data;
+size_t size = sizeof(*resv), total_size;
+uint8_t *prop_value;
+
+total_size = size + sizeof(*prop);
+
+if (bufstate->filled + total_size >= VIOMMU_PROBE_SIZE) {
+bufstate->error = true;
+/* get the traversal stopped by returning true */
+return true;
+}
+prop = (struct virtio_iommu_probe_property *)
+(bufstate->start + bufstate->filled);
+prop->type = cpu_to_le16(VIRTIO_IOMMU_PROBE_T_RESV_MEM) &
+VIRTIO_IOMMU_PROBE_T_MASK;
+prop->length = cpu_to_le16(size);
+
+prop_value = (uint8_t *)prop + 4;
+current = (struct virtio_iommu_probe_resv_mem *)prop_value;
+*current = *resv;
+bufstate->filled += total_size;
+trace_virtio_iommu_fill_resv_property(bufstate->endpoint->id,
+  resv->subtype, resv->start,
+  

[Qemu-devel] deprecating/removing bluetooth (was: Re: List of files containing devices which have not been QOMified)

2018-11-09 Thread Gerd Hoffmann
On Fri, Nov 09, 2018 at 02:16:53PM +0100, Paolo Bonzini wrote:
> On 09/11/2018 13:39, Thomas Huth wrote:
> > On 2018-11-09 12:29, Gerd Hoffmann wrote:
> >>
> >> Tried this after studying docs:
> >>
> >>   qemu -usb -device usb-bt-dongle -bt hci,vlan=0 -bt device:keyboard
> >>
> >> Segfaults right anway on first keypress.
> >> I guess that qualifies as "broken and obviously unused".
> > 
> > Thanks for checking! I guess that means we could even get rid of it
> > without deprecating it first if it is broken already for more than two
> > releases...?
> 
> I think what others were using bluetooth passthrough.  But it's
> certainly possible that it's broken.

That code is in bt-*.c (toplevel dir).  Likewise no actual code changes
since ages.  Comes in two variants, host + vhci.  vhci commit says:

commit ab2b6f507ded382df734fe6a237ec56e2f421de4
Author: balrog 
Date:   Mon Sep 29 00:31:41 2008 +

Use VHCI to allow the host os to participate in a qemu bluetooth "vlan".

This does the reverse of bt-host.c, proxying from guest to host.
Appears to be more reliable.

Hmm.  Not much of a clue.  Also ten years old.

There is also a null hci, not connecting to anything.  Probably useful as
dummy when your board has bluetooth (see arm/nseries) but you don't want
hook up any bluetooth devices.

Anyone has used bluetooth passthrough and can report what the state is?

I guess the keyboard can be dropped right away no matter what.

cheers,
  Gerd




Re: [Qemu-devel] [RFC v4 50/71] s390x: convert to cpu_interrupt_request

2018-11-09 Thread Cornelia Huck
On Thu, 25 Oct 2018 10:46:23 -0400
"Emilio G. Cota"  wrote:

> Cc: Cornelia Huck 
> Cc: Christian Borntraeger 
> Cc: Alexander Graf 
> Cc: David Hildenbrand 
> Cc: qemu-s3...@nongnu.org
> Reviewed-by: Richard Henderson 
> Signed-off-by: Emilio G. Cota 
> ---
>  hw/intc/s390_flic.c | 2 +-
>  target/s390x/cpu.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 



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

2018-11-09 Thread no-reply
Hi,

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

Type: series
Message-id: 20181106110548.4209-1-luc.mic...@greensocs.com
Subject: [Qemu-devel] [PATCH v4 00/16] gdbstub: support for the multiprocess 
extension

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
fb318fec24 arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters
c5a62c16df gdbstub: add multiprocess extension support
289b14424d gdbstub: gdb_set_stop_cpu: ignore request when process is not 
attached
9b832d7f2b gdbstub: processes initialization on new peer connection
44677eb35d gdbstub: add support for vAttach packets
d6011b9b46 gdbstub: add support for extended mode packet
fcd0123525 gdbstub: add multiprocess support to 'D' packets
0eb2ccca13 gdbstub: add multiprocess support to gdb_vm_state_change()
2168eedd20 gdbstub: add multiprocess support to Xfer:features:read:
22a0ea97f2 gdbstub: add multiprocess support to (f|s)ThreadInfo and 
ThreadExtraInfo
74c0bfb13d gdbstub: add multiprocess support to 'sC' packets
c6c4a08714 gdbstub: add multiprocess support to vCont packets
5198da536f gdbstub: add multiprocess support to 'H' and 'T' packets
48d430aeb1 gdbstub: add multiprocess support to '?' packets
df394b8412 gdbstub: introduce GDB processes
daeb666871 hw/cpu: introduce CPU clusters

=== OUTPUT BEGIN ===
Checking PATCH 1/16: hw/cpu: introduce CPU clusters...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100644

ERROR: do not initialise statics to 0 or NULL
#63: FILE: hw/cpu/cluster.c:28:
+static uint64_t cluster_id_auto_increment = 0;

total: 1 errors, 1 warnings, 102 lines checked

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

Checking PATCH 2/16: gdbstub: introduce GDB processes...
Checking PATCH 3/16: gdbstub: add multiprocess support to '?' packets...
Checking PATCH 4/16: gdbstub: add multiprocess support to 'H' and 'T' packets...
Checking PATCH 5/16: gdbstub: add multiprocess support to vCont packets...
Checking PATCH 6/16: gdbstub: add multiprocess support to 'sC' packets...
Checking PATCH 7/16: gdbstub: add multiprocess support to (f|s)ThreadInfo and 
ThreadExtraInfo...
Checking PATCH 8/16: gdbstub: add multiprocess support to Xfer:features:read:...
Checking PATCH 9/16: gdbstub: add multiprocess support to 
gdb_vm_state_change()...
Checking PATCH 10/16: gdbstub: add multiprocess support to 'D' packets...
Checking PATCH 11/16: gdbstub: add support for extended mode packet...
Checking PATCH 12/16: gdbstub: add support for vAttach packets...
Checking PATCH 13/16: gdbstub: processes initialization on new peer 
connection...
Checking PATCH 14/16: gdbstub: gdb_set_stop_cpu: ignore request when process is 
not attached...
Checking PATCH 15/16: gdbstub: add multiprocess extension support...
Checking PATCH 16/16: arm/xlnx-zynqmp: put APUs and RPUs in separate CPU 
clusters...
=== OUTPUT END ===

Test command exited with code: 1


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

[Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.

2018-11-09 Thread Gerd Hoffmann
Broken (segfaultson first keypress) and appearently unused.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/bt.h |   3 -
 hw/bt/hid.c | 554 
 vl.c|  34 +---
 hw/bt/Makefile.objs |   3 +-
 qemu-options.hx |   9 -
 5 files changed, 2 insertions(+), 601 deletions(-)
 delete mode 100644 hw/bt/hid.c

diff --git a/include/hw/bt.h b/include/hw/bt.h
index b5e11d4d43..73fb1911f6 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
 /* bt-sdp.c */
 void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
 
-/* bt-hid.c */
-struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
-
 /* Link Management Protocol layer defines */
 
 #define LLID_ACLU_CONT 0x1
diff --git a/hw/bt/hid.c b/hw/bt/hid.c
deleted file mode 100644
index 056291f9b5..00
--- a/hw/bt/hid.c
+++ /dev/null
@@ -1,554 +0,0 @@
-/*
- * QEMU Bluetooth HID Profile wrapper for USB HID.
- *
- * Copyright (C) 2007-2008 OpenMoko, Inc.
- * Written by Andrzej Zaborowski 
- *
- * 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 or
- * (at your option) version 3 of the License.
- *
- * 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, if not, see .
- */
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/timer.h"
-#include "ui/console.h"
-#include "hw/input/hid.h"
-#include "hw/bt.h"
-
-enum hid_transaction_req {
-BT_HANDSHAKE   = 0x0,
-BT_HID_CONTROL = 0x1,
-BT_GET_REPORT  = 0x4,
-BT_SET_REPORT  = 0x5,
-BT_GET_PROTOCOL= 0x6,
-BT_SET_PROTOCOL= 0x7,
-BT_GET_IDLE= 0x8,
-BT_SET_IDLE= 0x9,
-BT_DATA= 0xa,
-BT_DATC= 0xb,
-};
-
-enum hid_transaction_handshake {
-BT_HS_SUCCESSFUL   = 0x0,
-BT_HS_NOT_READY= 0x1,
-BT_HS_ERR_INVALID_REPORT_ID= 0x2,
-BT_HS_ERR_UNSUPPORTED_REQUEST  = 0x3,
-BT_HS_ERR_INVALID_PARAMETER= 0x4,
-BT_HS_ERR_UNKNOWN  = 0xe,
-BT_HS_ERR_FATAL= 0xf,
-};
-
-enum hid_transaction_control {
-BT_HC_NOP  = 0x0,
-BT_HC_HARD_RESET   = 0x1,
-BT_HC_SOFT_RESET   = 0x2,
-BT_HC_SUSPEND  = 0x3,
-BT_HC_EXIT_SUSPEND = 0x4,
-BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
-};
-
-enum hid_protocol {
-BT_HID_PROTO_BOOT  = 0,
-BT_HID_PROTO_REPORT= 1,
-};
-
-enum hid_boot_reportid {
-BT_HID_BOOT_INVALID= 0,
-BT_HID_BOOT_KEYBOARD,
-BT_HID_BOOT_MOUSE,
-};
-
-enum hid_data_pkt {
-BT_DATA_OTHER  = 0,
-BT_DATA_INPUT,
-BT_DATA_OUTPUT,
-BT_DATA_FEATURE,
-};
-
-#define BT_HID_MTU 48
-
-/* HID interface requests */
-#define GET_REPORT 0xa101
-#define GET_IDLE   0xa102
-#define GET_PROTOCOL   0xa103
-#define SET_REPORT 0x2109
-#define SET_IDLE   0x210a
-#define SET_PROTOCOL   0x210b
-
-struct bt_hid_device_s {
-struct bt_l2cap_device_s btdev;
-struct bt_l2cap_conn_params_s *control;
-struct bt_l2cap_conn_params_s *interrupt;
-HIDState hid;
-
-int proto;
-int connected;
-int data_type;
-int intr_state;
-struct {
-int len;
-uint8_t buffer[1024];
-} dataother, datain, dataout, feature, intrdataout;
-enum {
-bt_state_ready,
-bt_state_transaction,
-bt_state_suspend,
-} state;
-};
-
-static void bt_hid_reset(struct bt_hid_device_s *s)
-{
-struct bt_scatternet_s *net = s->btdev.device.net;
-
-/* Go as far as... */
-bt_l2cap_device_done(>btdev);
-bt_l2cap_device_init(>btdev, net);
-
-hid_reset(>hid);
-s->proto = BT_HID_PROTO_REPORT;
-s->state = bt_state_ready;
-s->dataother.len = 0;
-s->datain.len = 0;
-s->dataout.len = 0;
-s->feature.len = 0;
-s->intrdataout.len = 0;
-s->intr_state = 0;
-}
-
-static int bt_hid_out(struct bt_hid_device_s *s)
-{
-if (s->data_type == BT_DATA_OUTPUT) {
-/* nothing */
-;
-}
-
-if (s->data_type == 

[Qemu-devel] [PATCH v2] pulseaudio: process audio data in smaller chunks

2018-11-09 Thread Gerd Hoffmann
The rate of pulseaudio absorbing the audio stream is used to control the
the rate of the guests audio stream.  When the emulated hardware uses
small chunks (like intel-hda does) we need small chunks on the audio
backend side too, otherwise that feedback loop doesn't work very well.

Cc: Max Ehrlich 
Cc: Martin Schrodt 
Buglink: https://bugs.launchpad.net/bugs/1795527
Signed-off-by: Gerd Hoffmann 
---
 audio/paaudio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 949769774d..4c100bc318 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -227,7 +227,7 @@ static void *qpa_thread_out (void *arg)
 }
 }
 
-decr = to_mix = audio_MIN (pa->live, pa->g->conf.samples >> 2);
+decr = to_mix = audio_MIN(pa->live, pa->g->conf.samples >> 5);
 rpos = pa->rpos;
 
 if (audio_pt_unlock(>pt, __func__)) {
@@ -319,7 +319,7 @@ static void *qpa_thread_in (void *arg)
 }
 }
 
-incr = to_grab = audio_MIN (pa->dead, pa->g->conf.samples >> 2);
+incr = to_grab = audio_MIN(pa->dead, pa->g->conf.samples >> 5);
 wpos = pa->wpos;
 
 if (audio_pt_unlock(>pt, __func__)) {
-- 
2.9.3




Re: [Qemu-devel] [PATCH v5 02/24] hw: acpi: Export ACPI build alignment API

2018-11-09 Thread Igor Mammedov
On Mon,  5 Nov 2018 02:40:25 +0100
Samuel Ortiz  wrote:

> This is going to be needed by the Hardware-reduced ACPI routines.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
> Signed-off-by: Samuel Ortiz 
the patch is probably misplaced withing series,
if there is an external user within this series then this patch should
be squashed there, otherwise it doesn't belong to this series.

> ---
>  include/hw/acpi/aml-build.h | 2 ++
>  hw/acpi/aml-build.c | 8 
>  hw/i386/acpi-build.c| 8 
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 6c36903c0a..73fc6659f2 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -384,6 +384,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
>   const char *oem_id, const char *oem_table_id);
>  void *acpi_data_push(GArray *table_data, unsigned size);
>  unsigned acpi_data_len(GArray *table);
> +/* Align AML blob size to a multiple of 'align' */
> +void acpi_align_size(GArray *blob, unsigned align);
>  void acpi_add_table(GArray *table_offsets, GArray *table_data);
>  void acpi_build_tables_init(AcpiBuildTables *tables);
>  void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 1e43cd736d..51b608432f 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1565,6 +1565,14 @@ unsigned acpi_data_len(GArray *table)
>  return table->len;
>  }
>  
> +void acpi_align_size(GArray *blob, unsigned align)
> +{
> +/* Align size to multiple of given size. This reduces the chance
> + * we need to change size in the future (breaking cross version 
> migration).
> + */
> +g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> +}
> +
>  void acpi_add_table(GArray *table_offsets, GArray *table_data)
>  {
>  uint32_t offset = table_data->len;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d0362e1382..81d98fa34f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -282,14 +282,6 @@ static void acpi_get_pci_holes(Range *hole, Range 
> *hole64)
> NULL));
>  }
>  
> -static void acpi_align_size(GArray *blob, unsigned align)
> -{
> -/* Align size to multiple of given size. This reduces the chance
> - * we need to change size in the future (breaking cross version 
> migration).
> - */
> -g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> -}
> -
>  /* FACS */
>  static void
>  build_facs(GArray *table_data, BIOSLinker *linker)




Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

2018-11-09 Thread Fredrik Noring
Hi Aleksandar,

> Tx79 mentions the opposite: that DDIV, DDIVU, DMULT, DMULTU are not
> included in R5900 set.
> 
> I think that the best solution that you exclude DDIV, DDIVU, DMULT, DMULTU
> in a separate patch - there is no document to support their inclusion.

As Maciej noted, the 64-bit MIPS Linux psABI is indivisible, so how could
your alternative possibly work?

Fredrik



Re: [Qemu-devel] [PATCH] qga-win: fix leaks of build_guest_disk_info()

2018-11-09 Thread Michael Roth
Quoting Marc-André Lureau (2018-11-03 08:01:43)
> Introduced in commit b1ba8890e63ce9432c41c5c3fc229f54c87c9c99, vol_h
> handle should be closed, and "out" cleanup should be done after
> DeviceIoControl() fails.
> 
> Signed-off-by: Marc-André Lureau 

Thanks, applied to qga tree:
  https://github.com/mdroth/qemu/commits/qga

> ---
>  qga/commands-win32.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index ef1d7d48d2..62e1b51dfe 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -797,7 +797,7 @@ static GuestDiskAddressList *build_guest_disk_info(char 
> *guid, Error **errp)
>  0, extents, size, NULL, NULL)) {
>  error_setg_win32(errp, GetLastError(),
>  "failed to get disk extents");
> -return NULL;
> +goto out;
>  }
>  } else if (last_err == ERROR_INVALID_FUNCTION) {
>  /* Possibly CD-ROM or a shared drive. Try to pass the volume */
> @@ -855,6 +855,9 @@ static GuestDiskAddressList *build_guest_disk_info(char 
> *guid, Error **errp)
> 
> 
>  out:
> +if (vol_h != INVALID_HANDLE_VALUE) {
> +CloseHandle(vol_h);
> +}
>  qapi_free_GuestDiskAddress(disk);
>  g_free(extents);
>  g_free(name);
> -- 
> 2.19.1.708.g4ede3d42df
> 



Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.

2018-11-09 Thread Liam Merwick




On 09/11/2018 14:14, Gerd Hoffmann wrote:

Broken (segfaultson first keypress) and appearently unused.



s/segfaultson/segfaults on/
s/appearently/apparently/


Signed-off-by: Gerd Hoffmann 


one question at the end, otherwise

Reviewed-by: Liam Merwick 


---
  include/hw/bt.h |   3 -
  hw/bt/hid.c | 554 
  vl.c|  34 +---
  hw/bt/Makefile.objs |   3 +-
  qemu-options.hx |   9 -
  5 files changed, 2 insertions(+), 601 deletions(-)
  delete mode 100644 hw/bt/hid.c

diff --git a/include/hw/bt.h b/include/hw/bt.h
index b5e11d4d43..73fb1911f6 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
  /* bt-sdp.c */
  void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
  
-/* bt-hid.c */

-struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
-
  /* Link Management Protocol layer defines */
  
  #define LLID_ACLU_CONT		0x1

diff --git a/hw/bt/hid.c b/hw/bt/hid.c
deleted file mode 100644
index 056291f9b5..00
--- a/hw/bt/hid.c
+++ /dev/null
@@ -1,554 +0,0 @@
-/*
- * QEMU Bluetooth HID Profile wrapper for USB HID.
- *
- * Copyright (C) 2007-2008 OpenMoko, Inc.
- * Written by Andrzej Zaborowski 
- *
- * 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 or
- * (at your option) version 3 of the License.
- *
- * 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, if not, see .
- */
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/timer.h"
-#include "ui/console.h"
-#include "hw/input/hid.h"
-#include "hw/bt.h"
-
-enum hid_transaction_req {
-BT_HANDSHAKE   = 0x0,
-BT_HID_CONTROL = 0x1,
-BT_GET_REPORT  = 0x4,
-BT_SET_REPORT  = 0x5,
-BT_GET_PROTOCOL= 0x6,
-BT_SET_PROTOCOL= 0x7,
-BT_GET_IDLE= 0x8,
-BT_SET_IDLE= 0x9,
-BT_DATA= 0xa,
-BT_DATC= 0xb,
-};
-
-enum hid_transaction_handshake {
-BT_HS_SUCCESSFUL   = 0x0,
-BT_HS_NOT_READY= 0x1,
-BT_HS_ERR_INVALID_REPORT_ID= 0x2,
-BT_HS_ERR_UNSUPPORTED_REQUEST  = 0x3,
-BT_HS_ERR_INVALID_PARAMETER= 0x4,
-BT_HS_ERR_UNKNOWN  = 0xe,
-BT_HS_ERR_FATAL= 0xf,
-};
-
-enum hid_transaction_control {
-BT_HC_NOP  = 0x0,
-BT_HC_HARD_RESET   = 0x1,
-BT_HC_SOFT_RESET   = 0x2,
-BT_HC_SUSPEND  = 0x3,
-BT_HC_EXIT_SUSPEND = 0x4,
-BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
-};
-
-enum hid_protocol {
-BT_HID_PROTO_BOOT  = 0,
-BT_HID_PROTO_REPORT= 1,
-};
-
-enum hid_boot_reportid {
-BT_HID_BOOT_INVALID= 0,
-BT_HID_BOOT_KEYBOARD,
-BT_HID_BOOT_MOUSE,
-};
-
-enum hid_data_pkt {
-BT_DATA_OTHER  = 0,
-BT_DATA_INPUT,
-BT_DATA_OUTPUT,
-BT_DATA_FEATURE,
-};
-
-#define BT_HID_MTU 48
-
-/* HID interface requests */
-#define GET_REPORT 0xa101
-#define GET_IDLE   0xa102
-#define GET_PROTOCOL   0xa103
-#define SET_REPORT 0x2109
-#define SET_IDLE   0x210a
-#define SET_PROTOCOL   0x210b
-
-struct bt_hid_device_s {
-struct bt_l2cap_device_s btdev;
-struct bt_l2cap_conn_params_s *control;
-struct bt_l2cap_conn_params_s *interrupt;
-HIDState hid;
-
-int proto;
-int connected;
-int data_type;
-int intr_state;
-struct {
-int len;
-uint8_t buffer[1024];
-} dataother, datain, dataout, feature, intrdataout;
-enum {
-bt_state_ready,
-bt_state_transaction,
-bt_state_suspend,
-} state;
-};
-
-static void bt_hid_reset(struct bt_hid_device_s *s)
-{
-struct bt_scatternet_s *net = s->btdev.device.net;
-
-/* Go as far as... */
-bt_l2cap_device_done(>btdev);
-bt_l2cap_device_init(>btdev, net);
-
-hid_reset(>hid);
-s->proto = BT_HID_PROTO_REPORT;
-s->state = bt_state_ready;
-s->dataother.len = 0;
-s->datain.len = 0;
-s->dataout.len = 0;
-s->feature.len = 0;
-s->intrdataout.len = 0;
-

[Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs

2018-11-09 Thread Cleber Rosa
For the two Python jobs, which seem to have the goal of making sure
QEMU builds successfully on the 3.0-3.6 spectrum of Python 3 versions,
the specified version is only applicable if a Python virtual
environment is used.  To do that, it's necessary to define the
(primary?) language of the job to be Python.

Also, Travis doesn't have a 3.0 Python installation available for the
chosen distro, 3.2 being the lower version available.

Reference: 
https://docs.travis-ci.com/user/languages/python/#specifying-python-versions
Signed-off-by: Cleber Rosa 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index aa49c7b114..5c18d3e268 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -112,9 +112,11 @@ matrix:
   compiler: clang
 # Python builds
 - env: CONFIG="--target-list=x86_64-softmmu"
+  language: python
   python:
-- "3.0"
+- "3.2"
 - env: CONFIG="--target-list=x86_64-softmmu"
+  language: python
   python:
 - "3.6"
 # Acceptance (Functional) tests
-- 
2.19.1




[Qemu-devel] [PATCH v3 2/7] target/arm64: hold BQL when calling do_interrupt()

2018-11-09 Thread Alex Bennée
Fix the assertion failure when running interrupts.

Signed-off-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/kvm64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 6351a54b28..c39150e5e1 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1000,7 +1000,9 @@ bool kvm_arm_handle_debug(CPUState *cs, struct 
kvm_debug_exit_arch *debug_exit)
 cs->exception_index = EXCP_BKPT;
 env->exception.syndrome = debug_exit->hsr;
 env->exception.vaddress = debug_exit->far;
+qemu_mutex_lock_iothread();
 cc->do_interrupt(cs);
+qemu_mutex_unlock_iothread();
 
 return false;
 }
-- 
2.17.1




[Qemu-devel] [PATCH v3 4/7] tests/guest-debug: fix scoping of failcount

2018-11-09 Thread Alex Bennée
You should declare you are using a global version of a variable before
you attempt to modify it in a function.

Signed-off-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 tests/guest-debug/test-gdbstub.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/guest-debug/test-gdbstub.py 
b/tests/guest-debug/test-gdbstub.py
index 0e4ac01426..c7e3986a24 100644
--- a/tests/guest-debug/test-gdbstub.py
+++ b/tests/guest-debug/test-gdbstub.py
@@ -16,6 +16,7 @@ def report(cond, msg):
 print ("PASS: %s" % (msg))
 else:
 print ("FAIL: %s" % (msg))
+global failcount
 failcount += 1
 
 
-- 
2.17.1




[Qemu-devel] [PATCH v3 1/7] target/arm64: properly handle DBGVR RESS bits

2018-11-09 Thread Alex Bennée
This only fails with some (broken) versions of gdb but we should
treat the top bits of DBGBVR as RESS. Properly sign extend QEMU's
reference copy of dbgbvr and also update the register descriptions in
the comment.

Signed-off-by: Alex Bennée 

---
v2
  - sanitise register on insertion
  - update reference description
v3
  - fix bogus sextract64
---
 target/arm/kvm64.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 5de8ff0ac5..6351a54b28 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -103,7 +103,7 @@ static void kvm_arm_init_debug(CPUState *cs)
  * capable of fancier matching but that will require exposing that
  * fanciness to GDB's interface
  *
- * D7.3.2 DBGBCR_EL1, Debug Breakpoint Control Registers
+ * DBGBCR_EL1, Debug Breakpoint Control Registers
  *
  *  31  24 23  20 19   16 15 14  13  12   9 8   5 43 2   1  0
  * +--+--+---+-++--+-+--+-+---+
@@ -115,12 +115,25 @@ static void kvm_arm_init_debug(CPUState *cs)
  * SSC/HMC/PMC: Security, Higher and Priv access control (Table D-12)
  * BAS: Byte Address Select (RES1 for AArch64)
  * E: Enable bit
+ *
+ * DBGBVR_EL1, Debug Breakpoint Value Registers
+ *
+ *  63  53 52   49 48   2  1 0
+ * +--+---+--+-+
+ * | RESS | VA[52:49] | VA[48:2] | 0 0 |
+ * +--+---+--+-+
+ *
+ * Depending on the addressing mode bits the top bits of the register
+ * are a sign extension of the highest applicable VA bit. Some
+ * versions of GDB don't do it correctly so we ensure they are correct
+ * here so future PC comparisons will work properly.
  */
+
 static int insert_hw_breakpoint(target_ulong addr)
 {
 HWBreakpoint brk = {
 .bcr = 0x1, /* BCR E=1, enable */
-.bvr = addr
+.bvr = sextract64(addr, 0, 53)
 };
 
 if (cur_hw_bps >= max_hw_bps) {
-- 
2.17.1




[Qemu-devel] [PATCH v3 0/7] KVM Guest Debug fixes (plus TCG EL2 debug tweaks)

2018-11-09 Thread Alex Bennée
Hi,

I missed a fix I'd applied locally from v2 so this is a resend with
some additional tags, some changes suggested by rth and one more fix
for the test case.

So these are fixes for guest debug when running under KVM. While
re-spinning these I came across an anomaly which pointed to a kernel bug
that caused the 1st single-step to fail. This is being discussed at on
the kvm-arm list:

  Subject: [RFC PATCH] KVM: arm64: don't single-step for non-emulated faults
  Date: Wed, 7 Nov 2018 17:10:31 +
  Message-Id: <20181107171031.22573-1-alex.ben...@linaro.org>

It looks like there will be another patch series on its way to address
this.

As debugging HYP mode code is next to impossible on real hardware I
tried re-creating the single-step bug under TCG. As a result I ran into
some debug and EL2 cases that failed. The final two patches are some
fixes but I'm still seeing some weird behaviour although it is currently
obscured by timer interrupts constantly firing as I enter the to be
single-stepped guest EL1 instruction so they can probably be skipped for
3.1.

The following patches still need review:
 0001/target arm64 properly handle DBGVR RESS bits.patch
 0005/tests guest debug don t use symbol resolution for.patch
 0007/arm fix aa64_generate_debug_exceptions to work wi.patch

Alex Bennée (7):
  target/arm64: properly handle DBGVR RESS bits
  target/arm64: hold BQL when calling do_interrupt()
  target/arm64: kvm debug set target_el when passing exception to guest
  tests/guest-debug: fix scoping of failcount
  tests/guest-debug: don't use symbol resolution for PC checks
  arm: use symbolic MDCR_TDE in arm_debug_target_el
  arm: fix aa64_generate_debug_exceptions to work with EL2

 target/arm/cpu.h  | 41 +++
 target/arm/kvm64.c| 20 +--
 tests/guest-debug/test-gdbstub.py | 24 +++---
 3 files changed, 58 insertions(+), 27 deletions(-)

-- 
2.17.1




[Qemu-devel] [PATCH v3 3/7] target/arm64: kvm debug set target_el when passing exception to guest

2018-11-09 Thread Alex Bennée
When we are debugging the guest all exceptions come our way but might
be for the guest's own debug exceptions. We use the ->do_interrupt()
infrastructure to inject the exception into the guest. However, we are
missing a full setup of the exception structure, causing an assert
later down the line.

Signed-off-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
v2
  - tweak commit msg for grammar
---
 target/arm/kvm64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index c39150e5e1..46fbe6d8ff 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1000,6 +1000,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct 
kvm_debug_exit_arch *debug_exit)
 cs->exception_index = EXCP_BKPT;
 env->exception.syndrome = debug_exit->hsr;
 env->exception.vaddress = debug_exit->far;
+env->exception.target_el = 1;
 qemu_mutex_lock_iothread();
 cc->do_interrupt(cs);
 qemu_mutex_unlock_iothread();
-- 
2.17.1




[Qemu-devel] [PATCH v3 7/7] arm: fix aa64_generate_debug_exceptions to work with EL2

2018-11-09 Thread Alex Bennée
The test was incomplete and incorrectly caused debug exceptions to be
generated when returning to EL2 after a failed attempt to single-step
an EL1 instruction. Fix this while cleaning up the function a little.

Signed-off-by: Alex Bennée 

---
v3
  - further re-arrangement as suggested by rth
---
 target/arm/cpu.h | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1efff21a18..814ff69bc2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2764,23 +2764,35 @@ static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
 return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0;
 }
 
+/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */
 static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
 {
-if (arm_is_secure(env)) {
-/* MDCR_EL3.SDD disables debug events from Secure state */
-if (extract32(env->cp15.mdcr_el3, 16, 1) != 0
-|| arm_current_el(env) == 3) {
-return false;
-}
+int cur_el = arm_current_el(env);
+int debug_el;
+
+if (cur_el == 3) {
+return false;
 }
 
-if (arm_current_el(env) == arm_debug_target_el(env)) {
-if ((extract32(env->cp15.mdscr_el1, 13, 1) == 0)
-|| (env->daif & PSTATE_D)) {
-return false;
-}
+/* MDCR_EL3.SDD disables debug events from Secure state */
+if (arm_is_secure_below_el3(env)
+&& extract32(env->cp15.mdcr_el3, 16, 1)) {
+return false;
 }
-return true;
+
+/*
+ * Same EL to same EL debug exceptions need MDSCR_KDE enabled
+ * while not masking the (D)ebug bit in DAIF.
+ */
+debug_el = arm_debug_target_el(env);
+
+if (cur_el == debug_el) {
+return extract32(env->cp15.mdscr_el1, 13, 1)
+&& !(env->daif & PSTATE_D);
+}
+
+/* Otherwise the debug target needs to be a higher EL */
+return debug_el > cur_el;
 }
 
 static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
@@ -2833,9 +2845,6 @@ static inline bool 
aa32_generate_debug_exceptions(CPUARMState *env)
  * since the pseudocode has it at all callsites except for the one in
  * CheckSoftwareStep(), where it is elided because both branches would
  * always return the same value.
- *
- * Parts of the pseudocode relating to EL2 and EL3 are omitted because we
- * don't yet implement those exception levels or their associated trap bits.
  */
 static inline bool arm_generate_debug_exceptions(CPUARMState *env)
 {
-- 
2.17.1




[Qemu-devel] [PATCH v3 6/7] arm: use symbolic MDCR_TDE in arm_debug_target_el

2018-11-09 Thread Alex Bennée
We already have this symbol defined so lets use it.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b5eff79f73..1efff21a18 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2743,7 +2743,7 @@ static inline int arm_debug_target_el(CPUARMState *env)
 
 if (arm_feature(env, ARM_FEATURE_EL2) && !secure) {
 route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
-   env->cp15.mdcr_el2 & (1 << 8);
+   env->cp15.mdcr_el2 & MDCR_TDE;
 }
 
 if (route_to_el2) {
-- 
2.17.1




[Qemu-devel] [PATCH v3 5/7] tests/guest-debug: don't use symbol resolution for PC checks

2018-11-09 Thread Alex Bennée
It turns out symbol resolution isn't enough as modern kernels are
often padded with check code at the start of functions. GDB seems to
put the breakpoint at the first non-check instruction which causes
comparisons with the symbol resolution to fail.

For normal breakpoints we can detect the hit just by checking
hit_count instead. For hardware breakpoints we fish the breakpoint
address out of what gdb.execute() reported it was set at.

Signed-off-by: Alex Bennée 
---
 tests/guest-debug/test-gdbstub.py | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tests/guest-debug/test-gdbstub.py 
b/tests/guest-debug/test-gdbstub.py
index c7e3986a24..3de174b74b 100644
--- a/tests/guest-debug/test-gdbstub.py
+++ b/tests/guest-debug/test-gdbstub.py
@@ -6,9 +6,10 @@ from __future__ import print_function
 # gdb ${KERNEL}.vmlinux -x ${QEMU_SRC}/tests/guest-debug/test-gdbstub.py
 
 import gdb
+import re
 
 failcount = 0
-
+addr_match = re.compile("(0x[0-9a-f]{4,16})")
 
 def report(cond, msg):
 "Report success/fail of test"
@@ -37,26 +38,30 @@ def check_break(sym_name):
 gdb.execute("c")
 
 # hopefully we came back
-end_pc = gdb.parse_and_eval('$pc')
-print ("%s == %s %d" % (end_pc, sym.value(), bp.hit_count))
+hit = bp.hit_count
 bp.delete()
 
-# can we test we hit bp?
-return end_pc == sym.value()
+# did we hit bp?
+return hit > 0
 
 
 # We need to do hbreak manually as the python interface doesn't export it
+# As the resolution of sym_name might not exactly match where the
+# breakpoint actually ends up we need to fish it out from result of
+# gdb.execute.
 def check_hbreak(sym_name):
 "Setup hardware breakpoint, continue and check we stopped."
-sym, ok = gdb.lookup_symbol(sym_name)
-gdb.execute("hbreak %s" % (sym_name))
+result = gdb.execute("hbreak %s" % (sym_name), to_string=True)
+addr_txt = addr_match.search(result).group()
+addr = int(addr_txt, 16)
+
 gdb.execute("c")
 
 # hopefully we came back
 end_pc = gdb.parse_and_eval('$pc')
-print ("%s == %s" % (end_pc, sym.value()))
+print ("%s == %s" % (end_pc, addr))
 
-if end_pc == sym.value():
+if end_pc == addr:
 gdb.execute("d 1")
 return True
 else:
-- 
2.17.1




[Qemu-devel] [PATCH v7] lsi: Reselection needed to remove pending commands from queue

2018-11-09 Thread George Kennedy
Under heavy IO (e.g. fio) the queue is not checked frequently enough for
pending commands. As a result some pending commands are timed out by the
linux sym53c8xx driver, which sends SCSI Abort messages for the timed out
commands. The SCSI Abort messages result in linux errors, which show up
on the console and in /var/log/messages.

e.g.
sd 0:0:3:0: [sdd] tag#33 ABORT operation started
scsi target0:0:3: control msgout:
80 20 47 d
sd 0:0:3:0: ABORT operation complete.
scsi target0:0:4: message d sent on bad reselection

Now following a WAIT DISCONNECT Script instruction, and if there is no
current command, check for a pending command on the queue and if one
exists call lsi_reselect().

Signed-off-by: George Kennedy 
---
Thanks again Paolo,

Your latest suggested fix reduces the patch even further. Now there's
no need for a "pending" flag.

(Will try to find the intermediary patches and mail them offlist).

 hw/scsi/lsi53c895a.c | 40 +---
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d1e6534..959b893 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -298,6 +298,18 @@ static inline int lsi_irq_on_rsl(LSIState *s)
 return (s->sien0 & LSI_SIST0_RSL) && (s->scid & LSI_SCID_RRE);
 }
 
+static lsi_request *get_pending_req(LSIState *s)
+{
+lsi_request *p;
+
+QTAILQ_FOREACH(p, >queue, next) {
+if (p->pending) {
+return p;
+}
+}
+return NULL;
+}
+
 static void lsi_soft_reset(LSIState *s)
 {
 trace_lsi_reset();
@@ -446,7 +458,6 @@ static void lsi_update_irq(LSIState *s)
 {
 int level;
 static int last_level;
-lsi_request *p;
 
 /* It's unclear whether the DIP/SIP bits should be cleared when the
Interrupt Status Registers are cleared or when istat0 is read.
@@ -477,12 +488,12 @@ static void lsi_update_irq(LSIState *s)
 lsi_set_irq(s, level);
 
 if (!level && lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON)) {
+lsi_request *p;
+
 trace_lsi_update_irq_disconnected();
-QTAILQ_FOREACH(p, >queue, next) {
-if (p->pending) {
-lsi_reselect(s, p);
-break;
-}
+p = get_pending_req(s);
+if (p) {
+lsi_reselect(s, p);
 }
 }
 }
@@ -1064,11 +1075,12 @@ static void lsi_wait_reselect(LSIState *s)
 
 trace_lsi_wait_reselect();
 
-QTAILQ_FOREACH(p, >queue, next) {
-if (p->pending) {
-lsi_reselect(s, p);
-break;
-}
+if (s->current) {
+return;
+}
+p = get_pending_req(s);
+if (p) {
+lsi_reselect(s, p);
 }
 if (s->current == NULL) {
 s->waiting = 1;
@@ -1258,6 +1270,12 @@ again:
 case 1: /* Disconnect */
 trace_lsi_execute_script_io_disconnect();
 s->scntl1 &= ~LSI_SCNTL1_CON;
+if (!s->current) {
+lsi_request *p = get_pending_req(s);
+if (p) {
+lsi_reselect(s, p);
+}
+}
 break;
 case 2: /* Wait Reselect */
 if (!lsi_irq_on_rsl(s)) {
-- 
1.8.3.1




[Qemu-devel] [RFC PATCH 1/2] tests/test-qht-par: test gets stuck intermittently on OSX

2018-11-09 Thread Cleber Rosa
To be fully honest, this may not be a OSX (alone) condition, but may
be a situation that only happens with OSX on Travis-CI, were resources
are quite limited.

I have personal experience with tests that exercise parallelism or
depend on timing to fail on Travis.  Because I'm not 100% certain that
this is a situation that only happens with OSX on Travis-CI, and
because I'm not certain that we should be skipping tests because
they're running on Travis-CI, let's disable them on OSX as a whole.

A small note: this type of change makes me believe that there should
be a list of testing related caveats or TODO list tracked on the
documentation.

Signed-off-by: Cleber Rosa 
---
 tests/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 074eece558..c821b01467 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -87,7 +87,10 @@ check-unit-y += tests/test-rcu-simpleq$(EXESUF)
 check-unit-y += tests/test-rcu-tailq$(EXESUF)
 check-unit-y += tests/test-qdist$(EXESUF)
 check-unit-y += tests/test-qht$(EXESUF)
+# test-qht-par gets stuck quite often on OSX
+ifneq ($(CONFIG_DARWIN),y)
 check-unit-y += tests/test-qht-par$(EXESUF)
+endif
 check-unit-y += tests/test-bitops$(EXESUF)
 check-unit-y += tests/test-bitcnt$(EXESUF)
 check-unit-y += tests/test-qdev-global-props$(EXESUF)
-- 
2.19.1




[Qemu-devel] [RFC PATCH 0/2] Address OS X Travis failures

2018-11-09 Thread Cleber Rosa
The OS X jobs on Travis fail for a number of reasons.  This is an
attempt to decide its faith, either as something that will be kept on
the configuration (and properly maintained), removed, or maybe active
but with failures waived (Travis allows for that).

The result of this can be seen in these jobs part of a recent build:

 - https://travis-ci.org/clebergnu/qemu/jobs/452517040
 - https://travis-ci.org/clebergnu/qemu/jobs/452517041
 - https://travis-ci.org/clebergnu/qemu/jobs/452517042
 - https://travis-ci.org/clebergnu/qemu/jobs/452517043
 - https://travis-ci.org/clebergnu/qemu/jobs/452517044
 - https://travis-ci.org/clebergnu/qemu/jobs/452517045
 - https://travis-ci.org/clebergnu/qemu/jobs/452517046

Cleber Rosa (2):
  tests/test-qht-par: test gets stuck intermittently on OSX
  Travis CI: break down OSX+clang jobs

 .travis.yml| 29 -
 tests/Makefile.include |  3 +++
 2 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.19.1




Re: [Qemu-devel] [PATCH v2 4/6] target/mips: Fix decoding mechanism of special R5900 opcodes

2018-11-09 Thread Aleksandar Markovic
> From: Fredrik Noring 
> Subject: Re: [PATCH v2 4/6] target/mips: Fix decoding mechanism of special 
> R5900 opcodes
>
> Hi Aleksandar,
>
> > Tx79 mentions the opposite: that DDIV, DDIVU, DMULT, DMULTU are not
> > included in R5900 set.
> >
> > I think that the best solution that you exclude DDIV, DDIVU, DMULT, DMULTU
> > in a separate patch - there is no document to support their inclusion.
>
> As Maciej noted, the 64-bit MIPS Linux psABI is indivisible, so how could
> your alternative possibly work?

Since we are rapidly approaching 3.1 release, we don't have time for prolonged 
discussions - so please provide the patch that removes emulation of these 
instructions that don't belong to R5900 set, and, if you find a justification 
document later on, they can be reintroduced in 3.1+ timeframe.

Thanks,
Aleksnadar


[Qemu-devel] [RFC PATCH 2/2] Travis CI: break down OSX+clang jobs

2018-11-09 Thread Cleber Rosa
The OSX jobs were timing out on Travis, due to the long time they need
to run tests with all targets.  Let's break them down to avoid hitting
the time limit, and at the same time, give faster results.

Additionally the qtest based tests were hanging intermittently.  The
first debugging attempt involved making their execution verbose.  That
alone seemed to produce a positive effect and no hangs were observed
after that.  Given that Travis checks for output to decide if a test
is hung or not, it makes sense that a verbose execution minimizes
false positives.

Signed-off-by: Cleber Rosa 
---
 .travis.yml | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index aa49c7b114..505561aae6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -107,7 +107,34 @@ matrix:
 - env: CONFIG="--disable-tcg"
TEST_CMD=""
   compiler: gcc
-- env: CONFIG=""
+# osx+clang jobs are broken in a number of target specific sets to
+# allow jobs to finish before timing out
+- env: 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,cris-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=i386-softmmu,lm32-softmmu,m68k-softmmu,moxie-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,riscv32-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=s390x-softmmu,sh4-softmmu,sh4eb-softmmu,sparc-softmmu,sparc64-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=unicore32-softmmu,x86_64-softmmu,xtensa-softmmu,xtensaeb-softmmu"
+   TEST_CMD="make V=1 check"
+  os: osx
+  compiler: clang
+- env: 
CONFIG="--target-list=hppa-softmmu,microblaze-softmmu,microblazeel-softmmu,tricore-softmmu,riscv64-softmmu"
+   TEST_CMD="make V=1 check"
   os: osx
   compiler: clang
 # Python builds
-- 
2.19.1




Re: [Qemu-devel] [RFC PATCH 1/2] tests/test-qht-par: test gets stuck intermittently on OSX

2018-11-09 Thread Peter Maydell
On 9 November 2018 at 15:30, Cleber Rosa  wrote:
> To be fully honest, this may not be a OSX (alone) condition, but may
> be a situation that only happens with OSX on Travis-CI, were resources
> are quite limited.
>
> I have personal experience with tests that exercise parallelism or
> depend on timing to fail on Travis.  Because I'm not 100% certain that
> this is a situation that only happens with OSX on Travis-CI, and
> because I'm not certain that we should be skipping tests because
> they're running on Travis-CI, let's disable them on OSX as a whole.

Does this test still get stuck in builds after a458774ad711bceabefb ?
That fixed an OSX-specific problem that caused hangs (notably
in test-bdrv-drain, but it could I guess also affect other tests).

thanks
-- PMM



Re: [Qemu-devel] [PATCH v3 0/7] KVM Guest Debug fixes (plus TCG EL2 debug tweaks)

2018-11-09 Thread Alex Bennée


Alex Bennée  writes:

> Hi,
>
> I missed a fix I'd applied locally from v2 so this is a resend with
> some additional tags, some changes suggested by rth and one more fix
> for the test case.
>
> So these are fixes for guest debug when running under KVM. While
> re-spinning these I came across an anomaly which pointed to a kernel bug
> that caused the 1st single-step to fail. This is being discussed at on
> the kvm-arm list:
>
>   Subject: [RFC PATCH] KVM: arm64: don't single-step for non-emulated faults
>   Date: Wed, 7 Nov 2018 17:10:31 +
>   Message-Id: <20181107171031.22573-1-alex.ben...@linaro.org>
>
> It looks like there will be another patch series on its way to address
> this.

For reference Mark has posted this now:

   Subject: [PATCH 0/2] kvm/arm: make singlestep behaviour consistent
   Date: Fri,  9 Nov 2018 15:07:09 +
   Message-Id: <20181109150711.45864-1-mark.rutl...@arm.com>
--
Alex Bennée



Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Thomas Huth
On 2018-11-09 09:21, Laurent Vivier wrote:
> On 09/11/2018 09:13, Li Qiang wrote:
>> In set_netdev(), the peers[i] is initialized
>> qemu_find_net_clients_except() when i is in
>> 0 between 'queues' it can't be NULL.
>>
>> Signed-off-by: Li Qiang 
>> ---
>>  hw/core/qdev-properties-system.c | 4 
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/hw/core/qdev-properties-system.c 
>> b/hw/core/qdev-properties-system.c
>> index 8b22fb5..b45a7ef 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
>> char *name,
>>  }
>>  
>>  for (i = 0; i < queues; i++) {
>> -if (peers[i] == NULL) {
>> -err = -ENOENT;
>> -goto out;
>> -}
>>  
>>  if (peers[i]->peer) {
>>  err = -EEXIST;
>>
> 
> Reviewed-by: Laurent Vivier 

So who can pick up such patches? We don't have a maintainer for the QDEV
subsystem anymore... should it go via qemu-trivial?

 Thomas



Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Laurent Vivier
On 09/11/2018 09:26, Thomas Huth wrote:
> On 2018-11-09 09:21, Laurent Vivier wrote:
>> On 09/11/2018 09:13, Li Qiang wrote:
>>> In set_netdev(), the peers[i] is initialized
>>> qemu_find_net_clients_except() when i is in
>>> 0 between 'queues' it can't be NULL.
>>>
>>> Signed-off-by: Li Qiang 
>>> ---
>>>  hw/core/qdev-properties-system.c | 4 
>>>  1 file changed, 4 deletions(-)
>>>
>>> diff --git a/hw/core/qdev-properties-system.c 
>>> b/hw/core/qdev-properties-system.c
>>> index 8b22fb5..b45a7ef 100644
>>> --- a/hw/core/qdev-properties-system.c
>>> +++ b/hw/core/qdev-properties-system.c
>>> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
>>> char *name,
>>>  }
>>>  
>>>  for (i = 0; i < queues; i++) {
>>> -if (peers[i] == NULL) {
>>> -err = -ENOENT;
>>> -goto out;
>>> -}
>>>  
>>>  if (peers[i]->peer) {
>>>  err = -EEXIST;
>>>
>>
>> Reviewed-by: Laurent Vivier 
> 
> So who can pick up such patches? We don't have a maintainer for the QDEV
> subsystem anymore... should it go via qemu-trivial?

It seems trivial enough to go via qemu-trivial, but it would be good to
have a R-b from someone else than me to confirm.

Thanks,
Laurent



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

2018-11-09 Thread Luc Michel
On 11/8/18 5:09 PM, Philippe Mathieu-Daudé wrote:
> On 6/11/18 12:05, Luc Michel wrote:
>> changes since v3:
>>    - patch 1    cpu_cluster.h: remove QEMU_ from the multiple includes
>>     guard #ifdef/#define [Alistair]
>>
>>    - patch 1    cpu_cluster.c: include osdep.h first [Alistair]
>>
>>    - patch 1    use uint64_t for cluster ID for prosperity :) [Philippe]
> 
> Actually my comment about 32-bit had the opposite meaning, I suppose
> having 256 kind of cpu cores in the same SoC is probably enough for a
> long time IMHO.
Oops sorry about that :) I thought you were referring to an old quote
from a guy at Redmond about 640KiB of RAM ;)

A case I was thinking about that would require more than 8- or 16-bit ID
is if for whatever reason we need to model a SoC with sparse
(non-continuous) cluster IDs.

Anyway, I can put back the uint32_t for the next re-roll, I think it's a
good compromise.

Thanks.

Luc

> 
>>
>>    - patch 1    auto-assign a cluster ID to newly created clusters
>> [Philippe]
>>
>>    - patch 2    fix mid-code variable declaration [Alistair]
>>
>>    - patch 3    add a comment in gdb_get_cpu_pid() when retrieving CPU
>>     parent canonical path [Alistair]
>>
>>    - patch 14   fix a typo in the commit message [Alistair]
>>
>> changes since v2:
>>    - patch 1    introducing 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 2    removed 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 3    gdb_get_cpu_pid() now search for CPU being a child of a
>>     cpu-cluster object. Use the cluster-id to compute the
>> PID.
>>
>>    - patch 4    gdb_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 CPU clusters
>>
>>   

[Qemu-devel] [PATCH 0/2] virtio-9p: qmp interface for set/query io throttle for fsdev devices

2018-11-09 Thread xiezhide
These patches provide the qmp interface, to set/query the io throttle
status of the all fsdev devices that are present in a vm.
Some of the patches also remove the
duplicate code that was present in block and fsdev files.

Zhide Xie (2):
  fsdev-qmp: qmp interface for set/query io throttle for fsdev devices.
  fsdev-qmp: fix coding style issue

Makefile|  20 +++-
Makefile.objs   |   8 ++
block/throttle.c   |   6 +-
blockdev.c |  96 +
fsdev/qemu-fsdev-dummy.c|  11 ++
fsdev/qemu-fsdev-throttle.c| 144 +-
fsdev/qemu-fsdev-throttle.h   |   6 +-
fsdev/qemu-fsdev.c|  29 ++
hmp-commands-info.hx |  15 +++
hmp-commands.hx |  15 +++
hmp.c |  83 +--
hmp.h |   4 +
include/qemu/throttle-options.h|   3 +-
include/qemu/throttle.h   |   4 +-
include/qemu/typedefs.h  |   1 +
monitor.c   |   4 +
qapi/block-core.json   | 122 +-
qapi/fsdev.json |  96 +
qapi/qapi-schema.json   |   1 +
qapi/tlimits.json   |  89 
qmp.c  |  12 +++
util/throttle.c| 224 
++--
22 files changed, 639 insertions(+), 354 deletions(-)
create mode 100644 qapi/fsdev.json
create mode 100644 qapi/tlimits.json

--
1.8.3.1


[Qemu-devel] [Bug 1802465] [NEW] typing string via VNC is unreliable

2018-11-09 Thread Zhaocong
Public bug reported:

QEMU version is 3.0.0

# Description

The problem is that, when typing string through VNC, it can be
unreliable -- sometimes some key strokes get skipped, sometimes get
swapped, sometimes get repeated.  There's no problem when typing through
VNC on physical hardware.

# Steps to reproduce

1. Launch virtual machine by:

qemu-kvm -display vnc=:1 -m 2048 opensuse-leap-15.qcow2

2. Connect to VNC by:

vncviewer -Shared :5901

3. Simulate a series of key strokes by "vncdotool" [1]:

vncdotool -s 127.0.0.1::5901 typefile strings_to_be_typed.txt

4. Usually after a few hundred keys are typed, something goes wrong.

I attached a screenshot that it mistypes " hello" to "h ello".

[1] https://github.com/sibson/vncdotool

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "qemu-vnc-mistype.png"
   
https://bugs.launchpad.net/bugs/1802465/+attachment/5210655/+files/qemu-vnc-mistype.png

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

Title:
  typing string via VNC is unreliable

Status in QEMU:
  New

Bug description:
  QEMU version is 3.0.0

  # Description

  The problem is that, when typing string through VNC, it can be
  unreliable -- sometimes some key strokes get skipped, sometimes get
  swapped, sometimes get repeated.  There's no problem when typing
  through VNC on physical hardware.

  # Steps to reproduce

  1. Launch virtual machine by:

  qemu-kvm -display vnc=:1 -m 2048 opensuse-leap-15.qcow2

  2. Connect to VNC by:

  vncviewer -Shared :5901

  3. Simulate a series of key strokes by "vncdotool" [1]:

  vncdotool -s 127.0.0.1::5901 typefile strings_to_be_typed.txt

  4. Usually after a few hundred keys are typed, something goes wrong.

  I attached a screenshot that it mistypes " hello" to "h ello".

  [1] https://github.com/sibson/vncdotool

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



[Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Li Qiang
In set_netdev(), the peers[i] is initialized
qemu_find_net_clients_except() when i is in
0 between 'queues' it can't be NULL.

Signed-off-by: Li Qiang 
---
 hw/core/qdev-properties-system.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 8b22fb5..b45a7ef 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const char 
*name,
 }
 
 for (i = 0; i < queues; i++) {
-if (peers[i] == NULL) {
-err = -ENOENT;
-goto out;
-}
 
 if (peers[i]->peer) {
 err = -EEXIST;
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Laurent Vivier
On 09/11/2018 09:13, Li Qiang wrote:
> In set_netdev(), the peers[i] is initialized
> qemu_find_net_clients_except() when i is in
> 0 between 'queues' it can't be NULL.
> 
> Signed-off-by: Li Qiang 
> ---
>  hw/core/qdev-properties-system.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 8b22fb5..b45a7ef 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
> char *name,
>  }
>  
>  for (i = 0; i < queues; i++) {
> -if (peers[i] == NULL) {
> -err = -ENOENT;
> -goto out;
> -}
>  
>  if (peers[i]->peer) {
>  err = -EEXIST;
> 

Reviewed-by: Laurent Vivier 




Re: [Qemu-devel] [PATCH] hw: set_netdev: remove useless code

2018-11-09 Thread Thomas Huth
On 2018-11-09 09:13, Li Qiang wrote:
> In set_netdev(), the peers[i] is initialized
> qemu_find_net_clients_except() when i is in
> 0 between 'queues' it can't be NULL.
> 
> Signed-off-by: Li Qiang 
> ---
>  hw/core/qdev-properties-system.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 8b22fb5..b45a7ef 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -288,10 +288,6 @@ static void set_netdev(Object *obj, Visitor *v, const 
> char *name,
>  }
>  
>  for (i = 0; i < queues; i++) {
> -if (peers[i] == NULL) {
> -err = -ENOENT;
> -goto out;
> -}
>  
>  if (peers[i]->peer) {
>  err = -EEXIST;
> 

Reviewed-by: Thomas Huth 



[Qemu-devel] [RFC v8 09/18] virtio-iommu: Implement translate

2018-11-09 Thread Eric Auger
This patch implements the translate callback

Signed-off-by: Eric Auger 

---
v6 -> v7:
- implemented bypass-mode

v5 -> v6:
- replace error_report by qemu_log_mask

v4 -> v5:
- check the device domain is not NULL
- s/printf/error_report
- set flags to IOMMU_NONE in case of all translation faults
---
 hw/virtio/trace-events   |  1 +
 hw/virtio/virtio-iommu.c | 57 +++-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index b3c5c2604e..1f0e143b55 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -68,3 +68,4 @@ virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
 virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, uint64_t 
next_low, uint64_t next_high) "Unmap left [0x%"PRIx64",0x%"PRIx64"], new 
interval=[0x%"PRIx64",0x%"PRIx64"]"
 virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t 
next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"], new 
interval=[0x%"PRIx64",0x%"PRIx64"]"
 virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc 
[0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t 
sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1a1ed4d3d3..281d6d69c9 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -480,19 +480,74 @@ static IOMMUTLBEntry 
virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
 int iommu_idx)
 {
 IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+VirtIOIOMMU *s = sdev->viommu;
 uint32_t sid;
+viommu_endpoint *ep;
+viommu_mapping *mapping;
+viommu_interval interval;
+bool bypass_allowed;
+
+interval.low = addr;
+interval.high = addr + 1;
 
 IOMMUTLBEntry entry = {
 .target_as = _space_memory,
 .iova = addr,
 .translated_addr = addr,
-.addr_mask = ~(hwaddr)0,
+.addr_mask = (1 << ctz32(s->config.page_size_mask)) - 1,
 .perm = IOMMU_NONE,
 };
 
+bypass_allowed = virtio_has_feature(s->acked_features,
+VIRTIO_IOMMU_F_BYPASS);
+
 sid = virtio_iommu_get_sid(sdev);
 
 trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+qemu_mutex_lock(>mutex);
+
+ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(sid));
+if (!ep) {
+if (!bypass_allowed) {
+error_report("%s sid=%d is not known!!", __func__, sid);
+} else {
+entry.perm = flag;
+}
+goto unlock;
+}
+
+if (!ep->domain) {
+if (!bypass_allowed) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s %02x:%02x.%01x not attached to any domain\n",
+  __func__, PCI_BUS_NUM(sid), PCI_SLOT(sid), 
PCI_FUNC(sid));
+} else {
+entry.perm = flag;
+}
+goto unlock;
+}
+
+mapping = g_tree_lookup(ep->domain->mappings, (gpointer)());
+if (!mapping) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s no mapping for 0x%"PRIx64" for sid=%d\n",
+  __func__, addr, sid);
+goto unlock;
+}
+
+if (((flag & IOMMU_RO) && !(mapping->flags & VIRTIO_IOMMU_MAP_F_READ)) ||
+((flag & IOMMU_WO) && !(mapping->flags & VIRTIO_IOMMU_MAP_F_WRITE))) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "Permission error on 0x%"PRIx64"(%d): allowed=%d\n",
+  addr, flag, mapping->flags);
+goto unlock;
+}
+entry.translated_addr = addr - mapping->virt_addr + mapping->phys_addr;
+entry.perm = flag;
+trace_virtio_iommu_translate_out(addr, entry.translated_addr, sid);
+
+unlock:
+qemu_mutex_unlock(>mutex);
 return entry;
 }
 
-- 
2.17.2




[Qemu-devel] [RFC v8 18/18] hw/arm/virt: Allow virtio-iommu instantiation

2018-11-09 Thread Eric Auger
The virtio-iommu now can be instantiated by adding the virt
machine option "-M virt,iommu=virtio"

Signed-off-by: Eric Auger 
---
 hw/arm/virt.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f2994c4359..d2080a1ba9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1710,6 +1710,8 @@ static char *virt_get_iommu(Object *obj, Error **errp)
 return g_strdup("none");
 case VIRT_IOMMU_SMMUV3:
 return g_strdup("smmuv3");
+case VIRT_IOMMU_VIRTIO:
+return g_strdup("virtio");
 default:
 g_assert_not_reached();
 }
@@ -1721,11 +1723,13 @@ static void virt_set_iommu(Object *obj, const char 
*value, Error **errp)
 
 if (!strcmp(value, "smmuv3")) {
 vms->iommu = VIRT_IOMMU_SMMUV3;
+} else if (!strcmp(value, "virtio")) {
+vms->iommu = VIRT_IOMMU_VIRTIO;
 } else if (!strcmp(value, "none")) {
 vms->iommu = VIRT_IOMMU_NONE;
 } else {
 error_setg(errp, "Invalid iommu value");
-error_append_hint(errp, "Valid values are none, smmuv3.\n");
+error_append_hint(errp, "Valid values are none, smmuv3, virtio.\n");
 }
 }
 
@@ -1901,7 +1905,7 @@ static void virt_3_1_instance_init(Object *obj)
 object_property_add_str(obj, "iommu", virt_get_iommu, virt_set_iommu, 
NULL);
 object_property_set_description(obj, "iommu",
 "Set the IOMMU type. "
-"Valid values are none and smmuv3",
+"Valid values are none, smmuv3, virtio",
 NULL);
 
 vms->memmap = a15memmap;
-- 
2.17.2




[Qemu-devel] [RFC v8 16/18] hw/arm/virt-acpi-build: Introduce fill_iort_idmap helper

2018-11-09 Thread Eric Auger
To avoid code duplication, let's introduce an helper that
fills one IORT ID mappings array index.

Signed-off-by: Eric Auger 

---

v8: new
---
 hw/arm/virt-acpi-build.c | 43 
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 5785fb697c..ec7c4835fe 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -396,6 +396,17 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, 
unsigned xsdt_tbl_offset)
 return rsdp_table;
 }
 
+static inline void
+fill_iort_idmap(AcpiIortIdMapping *idmap, int i,
+uint32_t input_base, uint32_t id_count,
+uint32_t output_base, uint32_t output_reference)
+{
+idmap[i].input_base = cpu_to_le32(input_base);
+idmap[i].id_count = cpu_to_le32(id_count);
+idmap[i].output_base = cpu_to_le32(output_base);
+idmap[i].output_reference = cpu_to_le32(output_reference);
+}
+
 static void
 build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
@@ -453,13 +464,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 smmu->gerr_gsiv = cpu_to_le32(irq + 2);
 smmu->sync_gsiv = cpu_to_le32(irq + 3);
 
-/* Identity RID mapping covering the whole input RID range */
-idmap = >id_mapping_array[0];
-idmap->input_base = 0;
-idmap->id_count = cpu_to_le32(0x);
-idmap->output_base = 0;
-/* output IORT node is the ITS group node (the first node) */
-idmap->output_reference = cpu_to_le32(iort_node_offset);
+/*
+ * Identity RID mapping covering the whole input RID range.
+ * The output IORT node is the ITS group node (the first node).
+ */
+fill_iort_idmap(smmu->id_mapping_array, 0, 0, 0x, 0,
+iort_node_offset);
 }
 
 /* Root Complex Node */
@@ -477,18 +487,17 @@ build_iort(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 rc->memory_properties.memory_flags = 0x3; /* CCA = CPM = DCAS = 1 */
 rc->pci_segment_number = 0; /* MCFG pci_segment */
 
-/* Identity RID mapping covering the whole input RID range */
-idmap = >id_mapping_array[0];
-idmap->input_base = 0;
-idmap->id_count = cpu_to_le32(0x);
-idmap->output_base = 0;
-
 if (vms->iommu == VIRT_IOMMU_SMMUV3) {
-/* output IORT node is the smmuv3 node */
-idmap->output_reference = cpu_to_le32(smmu_offset);
+/* Identity RID mapping and output IORT node is the iommu node */
+fill_iort_idmap(rc->id_mapping_array, 0, 0, 0x, 0,
+smmu_offset);
 } else {
-/* output IORT node is the ITS group node (the first node) */
-idmap->output_reference = cpu_to_le32(iort_node_offset);
+/*
+ * Identity RID mapping and the output IORT node is the ITS group
+ * node (the first node).
+ */
+fill_iort_idmap(rc->id_mapping_array, 0, 0, 0x, 0,
+iort_node_offset);
 }
 
 /*
-- 
2.17.2




Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-09 Thread Peter Maydell
On 9 November 2018 at 12:39, Thomas Huth  wrote:
> On 2018-11-09 12:29, Gerd Hoffmann wrote:
>> On Fri, Nov 09, 2018 at 12:17:31PM +0100, Gerd Hoffmann wrote:
>>>   Hi,
>>>
 I am also suspicious about hw/bt/ but don't know enough
 about that subsystem to say if it could benefit from
 using QOM objects more.
>>>
>>> I'm wondering whenever anyone would even notice if we just rm -rf hw/bt
>>>
>>> Looking through the changelog for the last five years (after hw/ split)
>>> the only thing I see is fixing warnings from compiler or coverity,
>>> adapting to changes in other systems (chardev for example) and treewide
>>> changes.  Not a *single* patch specific to bluetooth ...
>>
>> Tried this after studying docs:
>>
>>   qemu -usb -device usb-bt-dongle -bt hci,vlan=0 -bt device:keyboard
>>
>> Segfaults right anway on first keypress.
>> I guess that qualifies as "broken and obviously unused".
>
> Thanks for checking! I guess that means we could even get rid of it
> without deprecating it first if it is broken already for more than two
> releases...?

Maybe; but it's potentially a big feature and that's just
a test of one use case. I would be happier if we stuck
to our standard deprecation strategy. (We're not dumping
it in 3.1 so we have the easy opportunity to let our
users have at least one release of notice by putting in
a deprecation notice in for 3.1.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH 0/2] ipmi: Allow UUID to be set for a BMC

2018-11-09 Thread Corey Minyard

On 11/8/18 5:22 PM, David Gibson wrote:

On Thu, Nov 08, 2018 at 08:19:42AM -0600, miny...@acm.org wrote:

The code was using the qemu UUID for the BMC.  But that's really
not a good method.  In general, you don't want the GUID to change
when you migrate, and you want the GUID to be the same between
invocations of qemu (if you have a GUID).

Hrm.  Generally the qemu UUID should remain the same across a
migration too, and I think that will be the case if using libvirt.
Maybe not if running qemu by hand and not specifying the uuid on the
command line.

I don't really have an objection to allowing the BMC's id to be
explicitly controlled, but the rationale above seems a bit
disingenuous.



I'm not sure about migration.  I suppose it could be migrated, but I
would consider the BMC part of the hardware that needs to be the
same on both sides.  It's a fuzzy line, I suppose.  The qemu UUID
is migrated, so I suppose that's not an issue.

Controlling it explicitly is important for some testing I do, and might
be for other people at some point in time, if you are trying to
emulate something specific.  And when re-invoking qemu, you
might want to keep it the same to avoid confusing software.

The big question for you is the lack of a GUID if it's not supplied.
With this change, the get GUID command will return a "command
not supported" error if the GUID is not supplied.

Thanks,

-corey


Plus, if you have multiple BMCs, they need to have different
GUIDs or the host code cannot tell them apart.  I'm not sure
anyone really uses multiple BMCs, but I do a lot of testing
with that scenario.

This change lets the user set the GUID on the command line, and
if the GUID is not set return an error for the GUID fetch command.
This maps better to how IPMI should work.

This change relies on the UUID being set to all zeros to know that
it is not set.  This is not optimal, perhaps, but an all zero
UUID isn't valid (it's the Nil UUID), so it should be ok.






Re: [Qemu-devel] [RFC v4 24/71] s390x: convert to cpu_halted

2018-11-09 Thread Cornelia Huck
On Wed, 31 Oct 2018 16:56:54 +
Alex Bennée  wrote:

> Emilio G. Cota  writes:
> 
> > On Wed, Oct 31, 2018 at 16:13:05 +, Alex Bennée wrote:  
> >> > @@ -353,10 +355,12 @@ void s390_cpu_unhalt(S390CPU *cpu)
> >> >  CPUState *cs = CPU(cpu);
> >> >  trace_cpu_unhalt(cs->cpu_index);
> >> >
> >> > -if (cs->halted) {
> >> > -cs->halted = 0;
> >> > +cpu_mutex_lock(cs);
> >> > +if (cpu_halted(cs)) {
> >> > +cpu_halted_set(cs, 0);
> >> >  cs->exception_index = -1;
> >> >  }
> >> > +cpu_mutex_unlock(cs);  
> >>
> >> I think this locking is superfluous as you already added locking when
> >> you introduced the helper.  
> >
> > It gives a small perf gain, since it avoids locking & unlocking twice
> > in a row (cpu_halted and cpu_halted_set both take the lock if needed).  
> 
> Maybe a one line comment then above the first lock:
> 
>   /* lock outside cpu_halted(_set) to avoid bouncing */
> 
> Or some such.

Yes, please.

Or introduce something like cpu_halted_flip(cs, new_state) where you
could convert the above to

if (cpu_halted_flip(cs, 0)) { /* cpu halted before */
cs_exception_index = -1;
}

Or is that actually an uncommon pattern?

> 
> >  
> >> Otherwise:
> >>
> >> Reviewed-by: Alex Bennée   
> >
> > Thanks!
> >
> > Emilio  
> 
> 
> --
> Alex Bennée




Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd

2018-11-09 Thread Ingo Molnar


* Li Zhijian  wrote:

> > If the kernel initrd creation process creates an initrd which
> > is larger than 2GB and also claims that it can't be placed
> > with any part of it above 2GB, then that sounds like a bug
> > in the initrd creation process...
> 
> Exactly, it's a real problem.
> 
> Add x86 maintainers and LKML:
> 
> The background is that QEMU want to support up to 4G initrd. but linux header 
> (
> initrd_addr_max field) only allow 2G-1.
> Is one of the below approaches reasonable:
> 1) change initrd_addr_max to 4G-1 directly simply(arch/x86/boot/header.S)?
> 2) lie QEMU bootloader the initrd_addr_max is 4G-1 even though header said 
> 2G-1
> 3) any else

A 10 years old comment from hpa says:

  initrd_addr_max: .long 0x7fff
# (Header version 0x0203 or later)
# The highest safe address for
# the contents of an initrd
# The current kernel allows up to 4 GB,
# but leave it at 2 GB to avoid
# possible bootloader bugs.

To avoid the potential of bugs lurking in dozens of major and hundreds of 
minor iterations of various Linux bootloaders I'd prefer a real solution 
and extend it - because if there's a 2GB initrd for some weird reason 
today there might be a 4GB one in two years.

The real solution would be to:

 - Extend the boot protocol with a 64-bit field, named initrd_addr64_max 
   or such.

 - We don't change the old field - but if the new field is set by new
   kernels then new bootloaders can use that as a new initrd_addr64_max
   value. (or reject to load the kernel if the address is too high.)

 - The kernel build should also emit a warning when building larger than 
   2GB initrds, with a list of bootloaders that support the new protocol.

Or something along those lines.

Thanks,

Ingo



Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd

2018-11-09 Thread Li Zhijian

On 11/9/2018 3:20 PM, Ingo Molnar wrote:

* Li Zhijian  wrote:


If the kernel initrd creation process creates an initrd which
is larger than 2GB and also claims that it can't be placed
with any part of it above 2GB, then that sounds like a bug
in the initrd creation process...

Exactly, it's a real problem.

Add x86 maintainers and LKML:

The background is that QEMU want to support up to 4G initrd. but linux header (
initrd_addr_max field) only allow 2G-1.
Is one of the below approaches reasonable:
1) change initrd_addr_max to 4G-1 directly simply(arch/x86/boot/header.S)?
2) lie QEMU bootloader the initrd_addr_max is 4G-1 even though header said 2G-1
3) any else

A 10 years old comment from hpa says:

   initrd_addr_max: .long 0x7fff
 # (Header version 0x0203 or later)
 # The highest safe address for
 # the contents of an initrd
 # The current kernel allows up to 4 GB,
 # but leave it at 2 GB to avoid
 # possible bootloader bugs.

To avoid the potential of bugs lurking in dozens of major and hundreds of
minor iterations of various Linux bootloaders I'd prefer a real solution
and extend it - because if there's a 2GB initrd for some weird reason
today there might be a 4GB one in two years.


thank a lots. that's amazing.




The real solution would be to:

  - Extend the boot protocol with a 64-bit field, named initrd_addr64_max
or such.
  - We don't change the old field - but if the new field is set by new
kernels then new bootloaders can use that as a new initrd_addr64_max
value. (or reject to load the kernel if the address is too high.)

  - The kernel build should also emit a warning when building larger than
2GB initrds, with a list of bootloaders that support the new protocol.


Actually i just knew QEMU(Seabios + optionrom(linuxboot_dma.bin)) can support 
~4GB initrd so far.

i just drafted at patch to add this field. could you have a look.
another patch which is to document initrd_addr64_max is ongoing.

commit db463ac9c1975f115d1ce2acb82d530c2b63b888
Author: Li Zhijian 
Date:   Fri Nov 9 17:24:14 2018 +0800

x86: Add header field initrd_addr64_max

Years ago, kernel had support load ~4GB initrd. But for some weird reasons (

avoid possible bootloader bugs), it only allow leave initrd under 2GB 
address
space(see initrd_addr_max fild at arch/x86/boot/header.S).

So modern bootloaders have not chance to load >=2G initrd previously.

To avoid the potential of bugs lurking in dozens of major and hundreds of

minor iterations of various Linux bootloaders. Ingo suggests to add a new 
field
initrd_addr64_max. If bootloader believes that it can load initrd to >=2G
address space, it can use initrd_addr64_max as the maximum loading address 
in
stead of the old field initrd_addr_max.

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 4c881c8..5fc3ebe 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
 
.ascii  "HdrS"  # header signature

-   .word   0x020e  # header version number (>= 0x0105)
+   .word   0x020f  # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
 realmode_swtch:.word   0, 0# default_switch, SETUPSEG
@@ -562,6 +562,12 @@ acpi_rsdp_addr:.quad 0 # 
64-bit physical pointer to the
# ACPI RSDP table, added with
# version 2.14
 
+#ifdef CONFIG_INITRD_SIZE_4GB

+initrd_addr64_max: .quad 0x# allow ~4G initrd since 2.15
+#else
+initrd_addr64_max: .quad 0
+#endif
+
 # End of setup header #
 
.section ".entrytext", "ax"

diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index 22f89d0..b86013d 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -90,6 +90,7 @@ struct setup_header {
__u32   init_size;
__u32   handover_offset;
__u64   acpi_rsdp_addr;
+   __u64   initrd_addr64_max;
 } __attribute__((packed));
 
 struct sys_desc_table {

diff --git a/init/Kconfig b/init/Kconfig
index a4112e9..611d4af 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1080,6 +1080,14 @@ config BLK_DEV_INITRD
 
  If unsure say Y.
 
+config INITRD_SIZE_4GB

+   bool "4G size initrd support"
+   depends on (X86 || X86_64)
+   help
+ This option enables support ~4GB initrd.
+
+ if 

  1   2   >