Re: [Qemu-devel] [PATCH 3/9] tcg: Use extract2 in tcg_gen_shifti_i64

[Richard Henderson]

On 3/9/19 12:30 PM, Aleksandar Markovic wrote:
> 
> 
> On Thursday, March 7, 2019, Richard Henderson  > wrote:
> 
> Signed-off-by: Richard Henderson  >
> ---
>  tcg/tcg-op.c | 47 ---
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 
> 
> Extract2 is not a good name for this new function, IMHO.

If you're going to bike shed the name, you should suggest something else.

But I think it's a pretty good name, since it extracts one register output out
of two register inputs.


r~

Re: [Qemu-devel] [PATCH 0/2] target/ppc: Optimize VSX instructions using deposit_i64()

[David Gibson]

On Sat, Mar 09, 2019 at 10:42:53PM +0100, Philippe Mathieu-Daudé wrote:
> Hi David, Richard.
> 
> I found these two patches while cleaning dangling branches on my
> previous laptop... Original commits date is 2017-07-21 05:24:05...
> I simply rebased them.
> 
> It doesn't have to be merged for soft freeze, but since I'm doing
> housekeeping I rather send it to keep archived by the ML.

Applied, thanks.

> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (2):
>   target/ppc: Optimize xviexpdp() using deposit_i64()
>   target/ppc: Optimize x[sv]xsigdp using deposit_i64()
> 
>  target/ppc/translate/vsx-impl.inc.c | 26 +++---
>  1 file changed, 7 insertions(+), 19 deletions(-)
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 00/10] bundle edk2 platform firmware with QEMU

[Michael S. Tsirkin]

On Sat, Mar 09, 2019 at 01:48:16AM +0100, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/qemu.git
> Branch: edk2_build
> 
> This series advances the roms/edk2 submodule to the "edk2-stable201903"
> release, and builds and captures platform firmware binaries from that
> release. At this point they are meant to be used by both end-users and
> by Igor's ACPI unit tests in qtest ("make check").
> 
> Previous discussion:
> 
>   [Qemu-devel] bundling edk2 platform firmware images with QEMU
>   80f0bae3-e79a-bb68-04c4-1c9c684d95b8@redhat.com">http://mid.mail-archive.com/80f0bae3-e79a-bb68-04c4-1c9c684d95b8@redhat.com
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02601.html
> 
> Note that the series was formatted with "--no-binary" (affecting patch
> #8), therefore it cannot be applied with "git-am". See the remote
> repo/branch reference near the top instead.
> 
> Thanks,
> Laszlo

High time IMO.

Reviewed-by: Michael S. Tsirkin 

Laszlo I suggest you add an entry to MAINTAINERS
and start doing pull requests.

Peter, what do you say? OK with you?


> Laszlo Ersek (10):
>   roms: lift "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"
>   roms/edk2-funcs.sh: require gcc-4.8+ for building i386 and x86_64
>   tests/uefi-test-tools/build.sh: work around TianoCore#1607
>   roms/edk2: advance to tag edk2-stable201903
>   roms/edk2-funcs.sh: add the qemu_edk2_get_thread_count() function
>   roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
>   roms: build edk2 firmware binaries and variable store templates
>   pc-bios: add edk2 firmware binaries and variable store templates
>   pc-bios: document the edk2 firmware images; add firmware descriptors
>   Makefile: install the edk2 firmware images and their descriptors
> 
>  Makefile   |  17 +-
>  pc-bios/README |  11 +
>  pc-bios/descriptors/50-edk2-i386-secure.json   |  34 +++
>  pc-bios/descriptors/50-edk2-x86_64-secure.json |  35 +++
>  pc-bios/descriptors/60-edk2-aarch64.json   |  31 +++
>  pc-bios/descriptors/60-edk2-arm.json   |  31 +++
>  pc-bios/descriptors/60-edk2-i386.json  |  33 +++
>  pc-bios/descriptors/60-edk2-x86_64.json|  34 +++
>  pc-bios/edk2-aarch64-code.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-arm-code.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-arm-vars.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-i386-code.fd  | Bin 0 -> 3653632 bytes
>  pc-bios/edk2-i386-secure-code.fd   | Bin 0 -> 3653632 bytes
>  pc-bios/edk2-i386-vars.fd  | Bin 0 -> 540672 bytes
>  pc-bios/edk2-licenses.txt  | 209 
>  pc-bios/edk2-x86_64-code.fd| Bin 0 -> 3653632 bytes
>  pc-bios/edk2-x86_64-secure-code.fd | Bin 0 -> 3653632 bytes
>  roms/Makefile  |   9 +-
>  roms/Makefile.edk2 | 138 +++
>  roms/edk2  |   2 +-
>  roms/edk2-build.sh |  55 +
>  roms/edk2-funcs.sh | 253 
>  tests/uefi-test-tools/build.sh | 100 +---
>  23 files changed, 897 insertions(+), 95 deletions(-)
>  create mode 100644 pc-bios/descriptors/50-edk2-i386-secure.json
>  create mode 100644 pc-bios/descriptors/50-edk2-x86_64-secure.json
>  create mode 100644 pc-bios/descriptors/60-edk2-aarch64.json
>  create mode 100644 pc-bios/descriptors/60-edk2-arm.json
>  create mode 100644 pc-bios/descriptors/60-edk2-i386.json
>  create mode 100644 pc-bios/descriptors/60-edk2-x86_64.json
>  create mode 100644 pc-bios/edk2-aarch64-code.fd
>  create mode 100644 pc-bios/edk2-arm-code.fd
>  create mode 100644 pc-bios/edk2-arm-vars.fd
>  create mode 100644 pc-bios/edk2-i386-code.fd
>  create mode 100644 pc-bios/edk2-i386-secure-code.fd
>  create mode 100644 pc-bios/edk2-i386-vars.fd
>  create mode 100644 pc-bios/edk2-licenses.txt
>  create mode 100644 pc-bios/edk2-x86_64-code.fd
>  create mode 100644 pc-bios/edk2-x86_64-secure-code.fd
>  create mode 100644 roms/Makefile.edk2
>  create mode 100755 roms/edk2-build.sh
>  create mode 100644 roms/edk2-funcs.sh
> 
> -- 
> 2.19.1.3.g30247aa5d201

Re: [Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces

[Deepika Choudhary]

Oh~
Okay!

On Sun, Mar 10, 2019, 02:30 Peter Maydell 
wrote:

> Deepika: the tricky part is the makefiles, not the configure script...
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1817345
>
> Title:
>   configure script breaks when $source_path contains white spaces
>
> Status in QEMU:
>   New
>
> Bug description:
>   Hi,
>
>   I noticed that the configure script breaks when the qemu source
>   directory is in a path containing white spaces, in particular the list
>   of targets is not correctly generated when calling "./configure
>   --help".
>
>   Steps to reproduce the problem:
>
>   $ mkdir "dir with spaces"
>   $ cd dir\ with\ spaces/
>   $ git clone https://git.qemu.org/git/qemu.git
>   $ cd qemu/
>   $ ./configure --help | grep -A3 target-list
>
>
>   Actual result:
>
> --target-list=LIST   set target list (default: build everything)
>  Available targets: dir with *-softmmu dir
> with
>  *-linux-user
>
>
>   Expected result:
>
> --target-list=LIST   set target list (default: build everything)
>  Available targets: aarch64-softmmu
> alpha-softmmu
>  arm-softmmu cris-softmmu hppa-softmmu
> i386-softmmu
>  lm32-softmmu m68k-softmmu microblaze-softmmu
>
>
>   This happens because the $mak_wilds variable uses spaces to separate
> different paths, maybe newlines may be used, which are less likely to be in
> directory names.
>
>   BTW "shellcheck" may help finding some other problems.
>
>   Qemu version:
>
>   $ git describe
>   v3.1.0-1960-ga05838cb2a
>
>   Thanks,
>  Antonio
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1817345/+subscriptions
>

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

Title:
  configure script breaks when $source_path contains white spaces

Status in QEMU:
  New

Bug description:
  Hi,

  I noticed that the configure script breaks when the qemu source
  directory is in a path containing white spaces, in particular the list
  of targets is not correctly generated when calling "./configure
  --help".

  Steps to reproduce the problem:

  $ mkdir "dir with spaces"
  $ cd dir\ with\ spaces/
  $ git clone https://git.qemu.org/git/qemu.git
  $ cd qemu/
  $ ./configure --help | grep -A3 target-list

  
  Actual result:

--target-list=LIST   set target list (default: build everything)
 Available targets: dir with *-softmmu dir with 
 *-linux-user

  
  Expected result:

--target-list=LIST   set target list (default: build everything)
 Available targets: aarch64-softmmu alpha-softmmu 
 arm-softmmu cris-softmmu hppa-softmmu i386-softmmu 
 lm32-softmmu m68k-softmmu microblaze-softmmu 

  
  This happens because the $mak_wilds variable uses spaces to separate 
different paths, maybe newlines may be used, which are less likely to be in 
directory names.

  BTW "shellcheck" may help finding some other problems.

  Qemu version:

  $ git describe 
  v3.1.0-1960-ga05838cb2a

  Thanks,
 Antonio

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

[Qemu-devel] [PATCH] Replace calls to object_child_foreach() with object_child_foreach_recursive()

[Ernest Esene]

Replace calls to object_child_foreach() with object_child_foreach_recursive()
when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device,
pc_dimm_slot2bitmap, build_dimm_list.

Signed-off-by: Ernest Esene 
---
 hw/acpi/nvdimm.c   | 5 +++--
 hw/core/sysbus.c   | 2 +-
 hw/mem/pc-dimm.c   | 5 +++--
 hw/virtio/virtio-balloon.c | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e53b2cb..846f44b 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -41,7 +41,7 @@ static int nvdimm_device_list(Object *obj, void *opaque)
 *list = g_slist_append(*list, DEVICE(obj));
 }
 
-object_child_foreach(obj, nvdimm_device_list, opaque);
+object_child_foreach_recursive(obj, nvdimm_device_list, opaque);
 return 0;
 }
 
@@ -56,7 +56,8 @@ static GSList *nvdimm_get_device_list(void)
 {
 GSList *list = NULL;
 
-object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
+object_child_foreach_recursive(qdev_get_machine(),
+   nvdimm_device_list, &list);
 return list;
 }
 
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9f9edbc..c16d57c 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -43,7 +43,7 @@ static int find_sysbus_device(Object *obj, void *opaque)
 
 if (!sbdev) {
 /* Container, traverse it for children */
-return object_child_foreach(obj, find_sysbus_device, opaque);
+return object_child_foreach_recursive(obj, find_sysbus_device, opaque);
 }
 
 find->func(sbdev, find->opaque);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 152400b..844c8ac 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -84,7 +84,7 @@ static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
 }
 }
 
-object_child_foreach(obj, pc_dimm_slot2bitmap, opaque);
+object_child_foreach_recursive(obj, pc_dimm_slot2bitmap, opaque);
 return 0;
 }
 
@@ -100,7 +100,8 @@ static int pc_dimm_get_free_slot(const int *hint, int 
max_slots, Error **errp)
 }
 
 bitmap = bitmap_new(max_slots);
-object_child_foreach(qdev_get_machine(), pc_dimm_slot2bitmap, bitmap);
+object_child_foreach_recursive(qdev_get_machine(),
+   pc_dimm_slot2bitmap, bitmap);
 
 /* check if requested slot is not occupied */
 if (hint) {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e3a6594..ce4b8a5 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -591,7 +591,7 @@ static int build_dimm_list(Object *obj, void *opaque)
 }
 }
 
-object_child_foreach(obj, build_dimm_list, opaque);
+object_child_foreach_recursive(obj, build_dimm_list, opaque);
 return 0;
 }
 
-- 
2.14.2

Re: [Qemu-devel] [PATCH] hw/riscv/virt: re-add machine-specific compatible string to /soc/ node

[Bin Meng]

Hi Lukas,

On Mon, Feb 11, 2019 at 6:13 AM Lukas Auer
 wrote:
>
> Re-add the previous compatible string "riscv-virtio-soc" to the soc
> device tree node to allow U-Boot and Linux to bind machine-specific
> drivers to it. The current compatible string "simple-bus" is retained.
>
> This is required by U-Boot to bind devices early, as part of the
> pre-relocation driver model.
>

I see no problem with U-Boot working with current compatible string
"simple-bus". In fact I had planned to remove the compatible string
"riscv-virtio-soc" in U-Boot but did not get time to work on it.

> Fixes: 53f54508dae6("hw/riscv/virtio: Set the soc device tree node as a
> simple-bus")
> Signed-off-by: Lukas Auer 
> ---
>
>  hw/riscv/virt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Regards,
Bin

[Qemu-devel] [PATCH v3 3/4] hw/i386: Use edk2_add_host_crypto_policy()

[Philippe Mathieu-Daudé]

Enable the EDK2 Crypto Policy features on the PC machine.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 42128183e9..00dc377df0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -38,6 +38,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/timer/hpet.h"
 #include "hw/firmware/smbios.h"
+#include "hw/firmware/uefi_edk2.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "multiboot.h"
@@ -1046,6 +1047,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, 
PCMachineState *pcms)
 return fw_cfg;
 }
 
+static void pc_uefi_setup(PCMachineState *pcms)
+{
+edk2_add_host_crypto_policy(pcms->fw_cfg);
+}
+
 static long get_file_size(FILE *f)
 {
 long where, size;
@@ -1645,6 +1651,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 if (pcms->fw_cfg) {
 pc_build_smbios(pcms);
 pc_build_feature_control_file(pcms);
+pc_uefi_setup(pcms);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 }
-- 
2.20.1

[Qemu-devel] [PATCH v3 4/4] hw/arm/virt: Use edk2_add_host_crypto_policy()

[Philippe Mathieu-Daudé]

Enable the EDK2 Crypto Policy features on the Virt machine.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/virt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7f66ddad89..940f41056a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -55,6 +55,7 @@
 #include "hw/intc/arm_gicv3_common.h"
 #include "kvm_arm.h"
 #include "hw/firmware/smbios.h"
+#include "hw/firmware/uefi_edk2.h"
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 #include "hw/arm/smmuv3.h"
@@ -1304,6 +1305,11 @@ static void virt_build_smbios(VirtMachineState *vms)
 }
 }
 
+static void virt_uefi_setup(VirtMachineState *vms)
+{
+edk2_add_host_crypto_policy(vms->fw_cfg);
+}
+
 static
 void virt_machine_done(Notifier *notifier, void *data)
 {
@@ -1332,6 +1338,7 @@ void virt_machine_done(Notifier *notifier, void *data)
 
 virt_acpi_setup(vms);
 virt_build_smbios(vms);
+virt_uefi_setup(vms);
 }
 
 static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
-- 
2.20.1

[Qemu-devel] [PATCH v3 1/4] hw/nvram/fw_cfg: Add fw_cfg_add_file_from_host()

[Philippe Mathieu-Daudé]

Add a function to read the full content of file on the host, and add
a new 'file' name item to the fw_cfg device.

Signed-off-by: Philippe Mathieu-Daudé 
---
v2: s/ptr/data, corrected documentation (Laszlo)
v3: inverted the if() logic
---
 hw/nvram/fw_cfg.c | 21 +
 include/hw/nvram/fw_cfg.h | 23 +++
 2 files changed, 44 insertions(+)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7fdf04adc9..2a345bfd5c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -826,6 +826,27 @@ void fw_cfg_add_file(FWCfgState *s,  const char *filename,
 fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);
 }
 
+void *fw_cfg_add_file_from_host(FWCfgState *s, const char *filename,
+const char *host_path, size_t *len)
+{
+GError *gerr = NULL;
+gchar *data = NULL;
+gsize contents_len = 0;
+
+if (!g_file_get_contents(host_path, &data, &contents_len, &gerr)) {
+error_report("%s", gerr->message);
+g_error_free(gerr);
+return NULL;
+}
+fw_cfg_add_file(s, filename, data, contents_len);
+
+if (len) {
+*len = contents_len;
+}
+
+return data;
+}
+
 void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
 void *data, size_t len)
 {
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index f5a6895a74..7c4dbe2a2a 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -166,6 +166,29 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t 
value);
 void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
  size_t len);
 
+/**
+ * fw_cfg_add_file_from_host:
+ * @s: fw_cfg device being modified
+ * @filename: name of new fw_cfg file item
+ * @host_path: path of the host file to read the data from
+ * @len: pointer to hold the length of the host file (optional)
+ *
+ * Read the content of a host file as a raw "blob" then add a new NAMED
+ * fw_cfg item of the file size. If @len is provided, it will contain the
+ * total length read from the host file. The data read from the host
+ * filesystem is owned by the new fw_cfg entry, and is stored into the data
+ * structure of the fw_cfg device.
+ * The next available (unused) selector key starting at FW_CFG_FILE_FIRST
+ * will be used; also, a new entry will be added to the file directory
+ * structure residing at key value FW_CFG_FILE_DIR, containing the item name,
+ * data size, and assigned selector key value.
+ *
+ * Returns: pointer to the newly allocated file content, or NULL if an error
+ * occured. The returned pointer must be freed with g_free().
+ */
+void *fw_cfg_add_file_from_host(FWCfgState *s, const char *filename,
+const char *host_path, size_t *len);
+
 /**
  * fw_cfg_add_file_callback:
  * @s: fw_cfg device being modified
-- 
2.20.1

[Qemu-devel] [PATCH v3 0/4] fw_cfg: Add edk2_add_host_crypto_policy()

[Philippe Mathieu-Daudé]

Hi,

This series consists of:
- add fw_cfg_add_file_from_host()
- add edk2_add_host_crypto_policy() and the Edk2Crypto object

The Edk2Crypto object is used to hold configuration values specific
to EDK2.

The edk2_add_host_crypto_policy() function loads crypto policies
from the host, and register them as fw_cfg named file items.

So far only the 'https' policy is supported.

A usercase example is the 'HTTPS Boof' feature of OVMF [*].

Usage example:

$ qemu-system-x86_64 \
--object edk2_crypto,id=https,\
ciphers=/etc/crypto-policies/back-ends/openssl.config,\
cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin

(On Fedora these files are provided by the ca-certificates and
crypto-policies packages).

[*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README

Since v2:
- Split of
Since v1:
- Addressed Michael and Laszlo comments.

Please review,

Phil.

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02522.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01598.html

Philippe Mathieu-Daudé (4):
  hw/nvram/fw_cfg: Add fw_cfg_add_file_from_host()
  hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()
  hw/i386: Use edk2_add_host_crypto_policy()
  hw/arm/virt: Use edk2_add_host_crypto_policy()

 MAINTAINERS |   8 ++
 hw/Makefile.objs|   1 +
 hw/arm/virt.c   |   7 +
 hw/firmware/Makefile.objs   |   1 +
 hw/firmware/uefi_edk2_crypto_policies.c | 177 
 hw/i386/pc.c|   7 +
 hw/nvram/fw_cfg.c   |  21 +++
 include/hw/firmware/uefi_edk2.h |  28 
 include/hw/nvram/fw_cfg.h   |  23 +++
 9 files changed, 273 insertions(+)
 create mode 100644 hw/firmware/Makefile.objs
 create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
 create mode 100644 include/hw/firmware/uefi_edk2.h

-- 
2.20.1

[Qemu-devel] [PATCH v3 2/4] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()

[Philippe Mathieu-Daudé]

The Edk2Crypto object is used to hold configuration values specific
to EDK2.

The edk2_add_host_crypto_policy() function loads crypto policies
from the host, and register them as fw_cfg named file items.
So far only the 'https' policy is supported.

A usercase example is the 'HTTPS Boof' feature of OVMF [*].

Usage example:

  $ qemu-system-x86_64 \
  --object edk2_crypto,id=https,\
  ciphers=/etc/crypto-policies/back-ends/openssl.config,\
  cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin

(On Fedora these files are provided by the ca-certificates and
crypto-policies packages).

[*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README

Signed-off-by: Philippe Mathieu-Daudé 
---
v3:
- '-object' -> '--object' in commit description (Eric)
- reworded the 'TODO: g_free' comment
---
 MAINTAINERS |   8 ++
 hw/Makefile.objs|   1 +
 hw/firmware/Makefile.objs   |   1 +
 hw/firmware/uefi_edk2_crypto_policies.c | 177 
 include/hw/firmware/uefi_edk2.h |  28 
 5 files changed, 215 insertions(+)
 create mode 100644 hw/firmware/Makefile.objs
 create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
 create mode 100644 include/hw/firmware/uefi_edk2.h

diff --git a/MAINTAINERS b/MAINTAINERS
index cf09a4c127..70122b3d0d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2206,6 +2206,14 @@ F: include/hw/i2c/smbus_master.h
 F: include/hw/i2c/smbus_slave.h
 F: include/hw/i2c/smbus_eeprom.h
 
+EDK2 Firmware
+M: Laszlo Ersek 
+M: Philippe Mathieu-Daudé 
+S: Maintained
+F: docs/interop/firmware.json
+F: hw/firmware/uefi_edk2_crypto_policies.c
+F: include/hw/firmware/uefi_edk2.h
+
 Usermode Emulation
 --
 Overall
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 82aa7fab8e..2b075aa1e0 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -8,6 +8,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += char/
 devices-dirs-$(CONFIG_SOFTMMU) += cpu/
 devices-dirs-$(CONFIG_SOFTMMU) += display/
 devices-dirs-$(CONFIG_SOFTMMU) += dma/
+devices-dirs-$(CONFIG_SOFTMMU) += firmware/
 devices-dirs-$(CONFIG_SOFTMMU) += gpio/
 devices-dirs-$(CONFIG_HYPERV) += hyperv/
 devices-dirs-$(CONFIG_I2C) += i2c/
diff --git a/hw/firmware/Makefile.objs b/hw/firmware/Makefile.objs
new file mode 100644
index 00..ea1f6d44df
--- /dev/null
+++ b/hw/firmware/Makefile.objs
@@ -0,0 +1 @@
+common-obj-y += uefi_edk2_crypto_policies.o
diff --git a/hw/firmware/uefi_edk2_crypto_policies.c 
b/hw/firmware/uefi_edk2_crypto_policies.c
new file mode 100644
index 00..5f88819a50
--- /dev/null
+++ b/hw/firmware/uefi_edk2_crypto_policies.c
@@ -0,0 +1,177 @@
+/*
+ * UEFI EDK2 Support
+ *
+ * Copyright (c) 2019 Red Hat Inc.
+ *
+ * Author:
+ *  Philippe Mathieu-Daudé 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "hw/firmware/uefi_edk2.h"
+
+
+#define TYPE_EDK2_CRYPTO "edk2_crypto"
+
+#define EDK2_CRYPTO_CLASS(klass) \
+ OBJECT_CLASS_CHECK(Edk2CryptoClass, (klass), \
+TYPE_EDK2_CRYPTO)
+#define EDK2_CRYPTO_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(Edk2CryptoClass, (obj), \
+  TYPE_EDK2_CRYPTO)
+#define EDK2_CRYPTO(obj) \
+ INTERFACE_CHECK(Edk2Crypto, (obj), \
+ TYPE_EDK2_CRYPTO)
+
+typedef struct Edk2Crypto {
+Object parent_obj;
+
+/*
+ * Path to the acceptable ciphersuites and the preferred order from
+ * the host-side crypto policy.
+ */
+char *ciphers_path;
+
+/* Path to the trusted CA certificates configured on the host side. */
+char *cacerts_path;
+} Edk2Crypto;
+
+typedef struct Edk2CryptoClass {
+ObjectClass parent_class;
+} Edk2CryptoClass;
+
+
+static void edk2_crypto_prop_set_ciphers(Object *obj, const char *value,
+ Error **errp G_GNUC_UNUSED)
+{
+Edk2Crypto *s = EDK2_CRYPTO(obj);
+
+g_free(s->ciphers_path);
+s->ciphers_path = g_strdup(value);
+}
+
+static char *edk2_crypto_prop_get_ciphers(Object *obj,
+  Error **errp G_GNUC_UNUSED)
+{
+Edk2Crypto *s = EDK2_CRYPTO(obj);
+
+return g_strdup(s->ciphers_path);
+}
+
+static void edk2_crypto_prop_set_cacerts(Object *obj, const char *value,
+ Error **errp G_GNUC_UNUSED)
+{
+Edk2Crypto *s = EDK2_CRYPTO(obj);
+
+g_free(s->cacerts_path);
+s->cacerts_path = g_strdup(value);
+}
+
+static char *edk2_crypto_prop_get_cacerts(Object *obj,
+  Error **errp G_GNUC_UNUSED)
+{
+Edk2Crypto *s = EDK2_CRYPTO(obj);
+
+return g_strdup(s->cacerts_path);
+}
+
+static void edk2_crypto_finalize(Object *obj)
+{
+Edk2Crypto *s = EDK2_CRYPTO(obj);
+
+g_free(s->ciphers_path);
+g_free(s->cacer

[Qemu-devel] [PATCH 6/6] target/m68k: Reduce the scope of the 'zero' tcg_temp

[Philippe Mathieu-Daudé]

Reduce the scope of the 'zero' tcg_temp. Since this tcg_temp is
allocated with tcg_const_i32(), free it using tcg_temp_free_i32().

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/m68k/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index b51b8a2a12..3f27079379 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -3664,7 +3664,7 @@ static void rotate_x_flags(TCGv reg, TCGv X, int size)
 /* Result of rotate_x() is valid if 0 <= shift <= size */
 static TCGv rotate_x(TCGv reg, TCGv shift, int left, int size)
 {
-TCGv X, shl, shr, shx, sz, zero;
+TCGv X, shl, shr, shx, sz;
 
 sz = tcg_const_i32(size);
 
@@ -3672,14 +3672,14 @@ static TCGv rotate_x(TCGv reg, TCGv shift, int left, 
int size)
 shl = tcg_temp_new();
 shx = tcg_temp_new();
 if (left) {
+TCGv zero = tcg_const_i32(0);
 tcg_gen_mov_i32(shl, shift);  /* shl = shift */
 tcg_gen_movi_i32(shr, size + 1);
 tcg_gen_sub_i32(shr, shr, shift); /* shr = size + 1 - shift */
 tcg_gen_subi_i32(shx, shift, 1);  /* shx = shift - 1 */
 /* shx = shx < 0 ? size : shx; */
-zero = tcg_const_i32(0);
 tcg_gen_movcond_i32(TCG_COND_LT, shx, shx, zero, sz, shx);
-tcg_temp_free(zero);
+tcg_temp_free_i32(zero);
 } else {
 tcg_gen_mov_i32(shr, shift);  /* shr = shift */
 tcg_gen_movi_i32(shl, size + 1);
-- 
2.19.1

[Qemu-devel] [PATCH 3/6] target/m68k: Fix a tcg_temp leak

[Philippe Mathieu-Daudé]

The function gen_get_ccr() returns a tcg_temp created with
tcg_temp_new(). Free it with tcg_temp_free().

Signed-off-by: Philippe Mathieu-Daudé 
---
Laurent/Richard, feel free to squash this with the next patch, but
IMHO having it split as a previous step makes the next patch easier
to review.
---
 target/m68k/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 55766fd7ef..ea95d55a11 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2224,6 +2224,7 @@ static TCGv gen_get_sr(DisasContext *s)
 sr = tcg_temp_new();
 tcg_gen_andi_i32(sr, QREG_SR, 0xffe0);
 tcg_gen_or_i32(sr, sr, ccr);
+tcg_temp_free(ccr);
 return sr;
 }
 
-- 
2.19.1

[Qemu-devel] [PATCH 5/6] target/m68k: Optimize rotate_x() using extract_i32()

[Philippe Mathieu-Daudé]

Optimize rotate_x() using tcg_gen_extract_i32(). We can now free the
'sz' tcg_temp earlier. Since it is allocated with tcg_const_i32(),
free it with tcg_temp_free_i32().

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/m68k/translate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index f43ac07b7f..b51b8a2a12 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -3686,6 +3686,7 @@ static TCGv rotate_x(TCGv reg, TCGv shift, int left, int 
size)
 tcg_gen_sub_i32(shl, shl, shift); /* shl = size + 1 - shift */
 tcg_gen_sub_i32(shx, sz, shift); /* shx = size - shift */
 }
+tcg_temp_free_i32(sz);
 
 /* reg = (reg << shl) | (reg >> shr) | (x << shx); */
 
@@ -3701,9 +3702,7 @@ static TCGv rotate_x(TCGv reg, TCGv shift, int left, int 
size)
 /* X = (reg >> size) & 1 */
 
 X = tcg_temp_new();
-tcg_gen_shr_i32(X, reg, sz);
-tcg_gen_andi_i32(X, X, 1);
-tcg_temp_free(sz);
+tcg_gen_extract_i32(X, reg, size, 1);
 
 return X;
 }
-- 
2.19.1

[Qemu-devel] [PATCH 1/6] target/m68k: Reduce the l1 TCGLabel scope

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/m68k/translate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 6217a683f1..ab801b6ceb 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -3019,7 +3019,6 @@ DISAS_INSN(branch)
 int32_t offset;
 uint32_t base;
 int op;
-TCGLabel *l1;
 
 base = s->pc;
 op = (insn >> 8) & 0xf;
@@ -3035,7 +3034,7 @@ DISAS_INSN(branch)
 }
 if (op > 1) {
 /* Bcc */
-l1 = gen_new_label();
+TCGLabel *l1 = gen_new_label();
 gen_jmpcc(s, ((insn >> 8) & 0xf) ^ 1, l1);
 gen_jmp_tb(s, 1, base + offset);
 gen_set_label(l1);
-- 
2.19.1

[Qemu-devel] [PATCH 4/6] target/m68k: Optimize get_sr() using deposit_i32()

[Philippe Mathieu-Daudé]

Doing so we free one tcg_temp.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/m68k/translate.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index ea95d55a11..f43ac07b7f 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2217,15 +2217,11 @@ static TCGv gen_get_ccr(DisasContext *s)
 
 static TCGv gen_get_sr(DisasContext *s)
 {
-TCGv ccr;
-TCGv sr;
+TCGv dest;
 
-ccr = gen_get_ccr(s);
-sr = tcg_temp_new();
-tcg_gen_andi_i32(sr, QREG_SR, 0xffe0);
-tcg_gen_or_i32(sr, sr, ccr);
-tcg_temp_free(ccr);
-return sr;
+dest = gen_get_ccr(s);
+tcg_gen_deposit_i32(dest, dest, QREG_SR, 5, 11);
+return dest;
 }
 
 static void gen_set_sr_im(DisasContext *s, uint16_t val, int ccr_only)
-- 
2.19.1

[Qemu-devel] [PATCH 0/6] target/m68k: Optimize few instructions using deposit/extraxt_i32()

[Philippe Mathieu-Daudé]

Hi Laurent, Richard.

I found these patches while cleaning dangling branches on my
previous laptop... Original commits date is 2017-07-21 09:31:32...
I simply had to rebase them.

It doesn't have to be merged for soft freeze, but since I'm doing
housekeeping I rather send it to keep archived by the ML.

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  target/m68k: Reduce the l1 TCGLabel scope
  target/m68k: Optimize the partset instruction using deposit_i32()
  target/m68k: Fix a tcg_temp leak
  target/m68k: Optimize get_sr() using deposit_i32()
  target/m68k: Optimize rotate_x() using extract_i32()
  target/m68k: Reduce the scope of the 'zero' tcg_temp

 target/m68k/translate.c | 31 ---
 1 file changed, 12 insertions(+), 19 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 2/6] target/m68k: Optimize the partset instruction using deposit_i32()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/m68k/translate.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index ab801b6ceb..55766fd7ef 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -720,17 +720,15 @@ static void gen_partset_reg(int opsize, TCGv reg, TCGv 
val)
 TCGv tmp;
 switch (opsize) {
 case OS_BYTE:
-tcg_gen_andi_i32(reg, reg, 0xff00);
 tmp = tcg_temp_new();
 tcg_gen_ext8u_i32(tmp, val);
-tcg_gen_or_i32(reg, reg, tmp);
+tcg_gen_deposit_i32(reg, tmp, reg, 8, 24);
 tcg_temp_free(tmp);
 break;
 case OS_WORD:
-tcg_gen_andi_i32(reg, reg, 0x);
 tmp = tcg_temp_new();
 tcg_gen_ext16u_i32(tmp, val);
-tcg_gen_or_i32(reg, reg, tmp);
+tcg_gen_deposit_i32(reg, tmp, reg, 16, 16);
 tcg_temp_free(tmp);
 break;
 case OS_LONG:
-- 
2.19.1

[Qemu-devel] [PATCH 2/2] target/ppc: Optimize x[sv]xsigdp using deposit_i64()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/ppc/translate/vsx-impl.inc.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 48c58deb14..3203165f49 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -1618,8 +1618,7 @@ static void gen_xsxsigdp(DisasContext *ctx)
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, zr, zr, t0);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, nan, zr, t0);
 get_cpu_vsrh(t1, xB(ctx->opcode));
-tcg_gen_andi_i64(rt, t1, 0x000F);
-tcg_gen_or_i64(rt, rt, t0);
+tcg_gen_deposit_i64(rt, t0, t1, 0, 52);
 
 tcg_temp_free_i64(t0);
 tcg_temp_free_i64(t1);
@@ -1655,8 +1654,7 @@ static void gen_xsxsigqp(DisasContext *ctx)
 tcg_gen_movi_i64(t0, 0x0001);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, zr, zr, t0);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, nan, zr, t0);
-tcg_gen_andi_i64(xth, xbh, 0x);
-tcg_gen_or_i64(xth, xth, t0);
+tcg_gen_deposit_i64(xth, t0, xbh, 0, 48);
 set_cpu_vsrh(rD(ctx->opcode) + 32, xth);
 tcg_gen_mov_i64(xtl, xbl);
 set_cpu_vsrl(rD(ctx->opcode) + 32, xtl);
@@ -1845,16 +1843,14 @@ static void gen_xvxsigdp(DisasContext *ctx)
 tcg_gen_movi_i64(t0, 0x0010);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, zr, zr, t0);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, nan, zr, t0);
-tcg_gen_andi_i64(xth, xbh, 0x000F);
-tcg_gen_or_i64(xth, xth, t0);
+tcg_gen_deposit_i64(xth, t0, xbh, 0, 52);
 set_cpu_vsrh(xT(ctx->opcode), xth);
 
 tcg_gen_extract_i64(exp, xbl, 52, 11);
 tcg_gen_movi_i64(t0, 0x0010);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, zr, zr, t0);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, nan, zr, t0);
-tcg_gen_andi_i64(xtl, xbl, 0x000F);
-tcg_gen_or_i64(xtl, xtl, t0);
+tcg_gen_deposit_i64(xth, t0, xbl, 0, 52);
 set_cpu_vsrl(xT(ctx->opcode), xtl);
 
 tcg_temp_free_i64(t0);
-- 
2.19.1

[Qemu-devel] [PATCH 0/2] target/ppc: Optimize VSX instructions using deposit_i64()

[Philippe Mathieu-Daudé]

Hi David, Richard.

I found these two patches while cleaning dangling branches on my
previous laptop... Original commits date is 2017-07-21 05:24:05...
I simply rebased them.

It doesn't have to be merged for soft freeze, but since I'm doing
housekeeping I rather send it to keep archived by the ML.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  target/ppc: Optimize xviexpdp() using deposit_i64()
  target/ppc: Optimize x[sv]xsigdp using deposit_i64()

 target/ppc/translate/vsx-impl.inc.c | 26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 1/2] target/ppc: Optimize xviexpdp() using deposit_i64()

[Philippe Mathieu-Daudé]

The t0 tcg_temp register is now unused, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/ppc/translate/vsx-impl.inc.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index e73197e717..48c58deb14 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -1726,7 +1726,6 @@ static void gen_xviexpdp(DisasContext *ctx)
 TCGv_i64 xal;
 TCGv_i64 xbh;
 TCGv_i64 xbl;
-TCGv_i64 t0;
 
 if (unlikely(!ctx->vsx_enabled)) {
 gen_exception(ctx, POWERPC_EXCP_VSXU);
@@ -1742,20 +1741,13 @@ static void gen_xviexpdp(DisasContext *ctx)
 get_cpu_vsrl(xal, xA(ctx->opcode));
 get_cpu_vsrh(xbh, xB(ctx->opcode));
 get_cpu_vsrl(xbl, xB(ctx->opcode));
-t0 = tcg_temp_new_i64();
 
-tcg_gen_andi_i64(xth, xah, 0x800F);
-tcg_gen_andi_i64(t0, xbh, 0x7FF);
-tcg_gen_shli_i64(t0, t0, 52);
-tcg_gen_or_i64(xth, xth, t0);
+tcg_gen_deposit_i64(xth, xah, xbh, 52, 11);
 set_cpu_vsrh(xT(ctx->opcode), xth);
-tcg_gen_andi_i64(xtl, xal, 0x800F);
-tcg_gen_andi_i64(t0, xbl, 0x7FF);
-tcg_gen_shli_i64(t0, t0, 52);
-tcg_gen_or_i64(xtl, xtl, t0);
+
+tcg_gen_deposit_i64(xtl, xal, xbl, 52, 11);
 set_cpu_vsrl(xT(ctx->opcode), xtl);
 
-tcg_temp_free_i64(t0);
 tcg_temp_free_i64(xth);
 tcg_temp_free_i64(xtl);
 tcg_temp_free_i64(xah);
-- 
2.19.1

Re: [Qemu-devel] [PATCH v4 00/29] Kconfig dependencies for ARM machines

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1551968334-18982-1-git-send-email-th...@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  hw/usb/dev-wacom.o
  CC  hw/usb/dev-storage.o
In file included from /tmp/qemu-test/src/hw/tpm/tpm_emulator.c:37:
/tmp/qemu-test/src/hw/tpm/tpm_ioctl.h:11:10: fatal error: sys/uio.h: No such 
file or directory
 #include 
  ^~~
compilation terminated.


The full log is available at
http://patchew.org/logs/1551968334-18982-1-git-send-email-th...@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces

[Peter Maydell]

Deepika: the tricky part is the makefiles, not the configure script...

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

Title:
  configure script breaks when $source_path contains white spaces

Status in QEMU:
  New

Bug description:
  Hi,

  I noticed that the configure script breaks when the qemu source
  directory is in a path containing white spaces, in particular the list
  of targets is not correctly generated when calling "./configure
  --help".

  Steps to reproduce the problem:

  $ mkdir "dir with spaces"
  $ cd dir\ with\ spaces/
  $ git clone https://git.qemu.org/git/qemu.git
  $ cd qemu/
  $ ./configure --help | grep -A3 target-list

  
  Actual result:

--target-list=LIST   set target list (default: build everything)
 Available targets: dir with *-softmmu dir with 
 *-linux-user

  
  Expected result:

--target-list=LIST   set target list (default: build everything)
 Available targets: aarch64-softmmu alpha-softmmu 
 arm-softmmu cris-softmmu hppa-softmmu i386-softmmu 
 lm32-softmmu m68k-softmmu microblaze-softmmu 

  
  This happens because the $mak_wilds variable uses spaces to separate 
different paths, maybe newlines may be used, which are less likely to be in 
directory names.

  BTW "shellcheck" may help finding some other problems.

  Qemu version:

  $ git describe 
  v3.1.0-1960-ga05838cb2a

  Thanks,
 Antonio

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

Re: [Qemu-devel] [PULL 0/7] Block patches

[Peter Maydell]

On Fri, 8 Mar 2019 at 16:53, Stefan Hajnoczi  wrote:
>
> The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6:
>
>   Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' 
> into staging (2019-03-07 16:16:02 +)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 6ca206204fa773c8626d59caf2a5676d6cc35f52:
>
>   iothread: document about why we need explicit aio_poll() (2019-03-08 
> 10:20:57 +)
>
> 
> Pull request
>
> 

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH 3/9] tcg: Use extract2 in tcg_gen_shifti_i64

[Aleksandar Markovic]

On Thursday, March 7, 2019, Richard Henderson 
wrote:

> Signed-off-by: Richard Henderson 
> ---
>  tcg/tcg-op.c | 47 ---
>  1 file changed, 24 insertions(+), 23 deletions(-)
>
>
Extract2 is not a good name for this new function, IMHO.



> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index deacc63e3b..34e0dbc6e0 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -1355,31 +1355,32 @@ static inline void tcg_gen_shifti_i64(TCGv_i64
> ret, TCGv_i64 arg1,
>  tcg_gen_shli_i32(TCGV_HIGH(ret), TCGV_LOW(arg1), c);
>  tcg_gen_movi_i32(TCGV_LOW(ret), 0);
>  }
> -} else {
> -TCGv_i32 t0, t1;
> -
> -t0 = tcg_temp_new_i32();
> -t1 = tcg_temp_new_i32();
> -if (right) {
> -tcg_gen_shli_i32(t0, TCGV_HIGH(arg1), 32 - c);
> -if (arith) {
> -tcg_gen_sari_i32(t1, TCGV_HIGH(arg1), c);
> -} else {
> -tcg_gen_shri_i32(t1, TCGV_HIGH(arg1), c);
> -}
> -tcg_gen_shri_i32(TCGV_LOW(ret), TCGV_LOW(arg1), c);
> -tcg_gen_or_i32(TCGV_LOW(ret), TCGV_LOW(ret), t0);
> -tcg_gen_mov_i32(TCGV_HIGH(ret), t1);
> +} else if (right) {
> +if (TCG_TARGET_HAS_extract2_i32) {
> +tcg_gen_extract2_i32(TCGV_LOW(ret), TCGV_LOW(arg1),
> + TCGV_HIGH(arg1), c);
>  } else {
> -tcg_gen_shri_i32(t0, TCGV_LOW(arg1), 32 - c);
> -/* Note: ret can be the same as arg1, so we use t1 */
> -tcg_gen_shli_i32(t1, TCGV_LOW(arg1), c);
> -tcg_gen_shli_i32(TCGV_HIGH(ret), TCGV_HIGH(arg1), c);
> -tcg_gen_or_i32(TCGV_HIGH(ret), TCGV_HIGH(ret), t0);
> -tcg_gen_mov_i32(TCGV_LOW(ret), t1);
> +tcg_gen_shri_i32(TCGV_LOW(ret), TCGV_LOW(arg1), c);
> +tcg_gen_deposit_i32(TCGV_LOW(ret), TCGV_LOW(ret),
> +TCGV_HIGH(arg1), 32 - c, c);
>  }
> -tcg_temp_free_i32(t0);
> -tcg_temp_free_i32(t1);
> +if (arith) {
> +tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_HIGH(arg1), c);
> +} else {
> +tcg_gen_shri_i32(TCGV_HIGH(ret), TCGV_HIGH(arg1), c);
> +}
> +} else {
> +if (TCG_TARGET_HAS_extract2_i32) {
> +tcg_gen_extract2_i32(TCGV_HIGH(ret), TCGV_LOW(arg1),
> + TCGV_HIGH(arg1), 32 - c);
> +} else {
> +TCGv_i32 t0 = tcg_temp_new_i32();
> +tcg_gen_shri_i32(t0, TCGV_LOW(arg1), 32 - c);
> +tcg_gen_deposit_i32(TCGV_HIGH(ret), t0,
> +TCGV_HIGH(arg1), c, 32 - c);
> +tcg_temp_free_i32(t0);
> +}
> +tcg_gen_shli_i32(TCGV_LOW(ret), TCGV_LOW(arg1), c);
>  }
>  }
>
> --
> 2.17.2
>
>
>

[Qemu-devel] [Bug 1819289] [NEW] Windows 95 and Windows 98 will not install or run

[John M]

Public bug reported:

The last version of QEMU I have been able to run Windows 95 or Windows
98 on was 2.7 or 2.8. Recent versions since then even up to 3.1 will
either not install or will not run 95 or 98 at all. I have tried every
combination of options like isapc or no isapc, cpu pentium  or cpu as
486. Tried different memory configurations, but they just don't work
anymore.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Windows 95 and Windows 98 will not install or run

Status in QEMU:
  New

Bug description:
  The last version of QEMU I have been able to run Windows 95 or Windows
  98 on was 2.7 or 2.8. Recent versions since then even up to 3.1 will
  either not install or will not run 95 or 98 at all. I have tried every
  combination of options like isapc or no isapc, cpu pentium  or cpu as
  486. Tried different memory configurations, but they just don't work
  anymore.

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

[Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces

[Deepika Choudhary]

If calling from any Unix shell, and the parameter has spaces, then we
need to quote it.we can also use single quotes, these are more powerful.
They stop the shell from interpreting anything ($, !, \, *, ", etc,
except ').


can't we use this approach here??

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

Title:
  configure script breaks when $source_path contains white spaces

Status in QEMU:
  New

Bug description:
  Hi,

  I noticed that the configure script breaks when the qemu source
  directory is in a path containing white spaces, in particular the list
  of targets is not correctly generated when calling "./configure
  --help".

  Steps to reproduce the problem:

  $ mkdir "dir with spaces"
  $ cd dir\ with\ spaces/
  $ git clone https://git.qemu.org/git/qemu.git
  $ cd qemu/
  $ ./configure --help | grep -A3 target-list

  
  Actual result:

--target-list=LIST   set target list (default: build everything)
 Available targets: dir with *-softmmu dir with 
 *-linux-user

  
  Expected result:

--target-list=LIST   set target list (default: build everything)
 Available targets: aarch64-softmmu alpha-softmmu 
 arm-softmmu cris-softmmu hppa-softmmu i386-softmmu 
 lm32-softmmu m68k-softmmu microblaze-softmmu 

  
  This happens because the $mak_wilds variable uses spaces to separate 
different paths, maybe newlines may be used, which are less likely to be in 
directory names.

  BTW "shellcheck" may help finding some other problems.

  Qemu version:

  $ git describe 
  v3.1.0-1960-ga05838cb2a

  Thanks,
 Antonio

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

[Qemu-devel] [PATCH v3 2/2] hw/nvram/fw_cfg: Use the ldst API

[Philippe Mathieu-Daudé]

The load/store API eases code review.

Reviewed-by: Laszlo Ersek 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/nvram/fw_cfg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7fdf04adc9..3d8859e333 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -85,7 +85,7 @@ static char *read_splashfile(char *filename, gsize 
*file_sizep,
 }
 
 /* check magic ID */
-filehead = ((content[0] & 0xff) + (content[1] << 8)) & 0x;
+filehead = lduw_le_p(content);
 if (filehead == 0xd8ff) {
 file_type = JPG_FILE;
 } else if (filehead == 0x4d42) {
@@ -96,7 +96,7 @@ static char *read_splashfile(char *filename, gsize 
*file_sizep,
 
 /* check BMP bpp */
 if (file_type == BMP_FILE) {
-bmp_bpp = (content[28] + (content[29] << 8)) & 0x;
+bmp_bpp = lduw_le_p(&content[28]);
 if (bmp_bpp != 24) {
 goto error;
 }
-- 
2.20.1

[Qemu-devel] [PATCH v3 0/2] fw_cfg: Trivial cleanups

[Philippe Mathieu-Daudé]

Hi Laurent,

These 2 patches are trivial cleanups extracted from a bigger fw_cfg series.

Please consider for your next Trivial pull request.

Regards,

Phil.

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02522.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01598.html

Philippe Mathieu-Daudé (2):
  hw/arm/virt: Remove null-check in virt_build_smbios()
  hw/nvram/fw_cfg: Use the ldst API

 hw/arm/virt.c | 4 
 hw/nvram/fw_cfg.c | 4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v3 1/2] hw/arm/virt: Remove null-check in virt_build_smbios()

[Philippe Mathieu-Daudé]

Since commit 578f3c7b0835 ("arm: add fw_cfg to "virt" board",
2014-12-22), the machvirt_init() unconditionally creates the
fw_cfg object.  Later, commit c30e15658b1b ("smbios: implement
smbios support for mach-virt", 2015-09-07) added a superfluous
null-check on it.
Remove this superfluous check.

Reviewed-by: Laszlo Ersek 
Reviewed-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2: Corrected commit reference (Laszlo)
v3: Dropped 'Fixes:' (Markus)
---
 hw/arm/virt.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7f66ddad89..377e95a4cd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1281,10 +1281,6 @@ static void virt_build_smbios(VirtMachineState *vms)
 size_t smbios_tables_len, smbios_anchor_len;
 const char *product = "QEMU Virtual Machine";
 
-if (!vms->fw_cfg) {
-return;
-}
-
 if (kvm_enabled()) {
 product = "KVM Virtual Machine";
 }
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 16/18] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()

[Philippe Mathieu-Daudé]

Hi Eric,

On 3/8/19 3:16 AM, Eric Blake wrote:
> On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote:
>> The Edk2Crypto object is used to hold configuration values specific
>> to EDK2.
>>
>> The edk2_add_host_crypto_policy() function loads crypto policies
>> from the host, and register them as fw_cfg named file items.
>> So far only the 'https' policy is supported.
>>
>> An usercase example is the 'HTTPS Boof' feature of OVMF [*].
> 
> s/An/A/ since "user" is a pronounced or hard 'u' (English is funny, but
> the rule of thumb is you add the consonant only before a soft u, and not
> a pronounced one; as in "give an umbrella to a unicorn")

I appreciate the correction, thanks :)

>>
>> Usage example:
>>
>>   $ qemu-system-x86_64 \
>>   -object edk2_crypto,id=https,\
> 
> Might as well use --object (both spellings work for qemu, but since
> --object is the only spelling for qemu-img/qemu-nbd, being consistent
> between the lot is useful).

$ git grep -- ' -object ' | wc -l
83

^ cover various subsystems:

$ git grep -l -- ' -object '
docs/amd-memory-encryption.txt
docs/can.txt
docs/memory-hotplug.txt
docs/nvdimm.txt
docs/pr-manager.rst
docs/pvrdma.txt
docs/replay.txt
hw/virtio/vhost-user.c
include/authz/listfile.h
include/authz/pamacct.h
include/authz/simple.h
include/crypto/secret.h
include/crypto/tlscredsanon.h
include/crypto/tlscredsx509.h
qapi/misc.json
qemu-doc.texi
qemu-options.hx
target/i386/sev_i386.h
tests/bios-tables-test.c
tests/qemu-iotests/127
tests/qemu-iotests/200
tests/vhost-user-test.c


$ git grep -- ' --object ' | wc -l
252

^ mostly for the block subsystem:

$ git grep -l -- ' --object '
block/vxhs.c
include/crypto/tlscredspsk.h
qemu-doc.texi
qemu-img.texi
qemu-io.c
qemu-nbd.c
qemu-nbd.texi
tests/qemu-iotests/049
tests/qemu-iotests/049.out
tests/qemu-iotests/087
tests/qemu-iotests/134
tests/qemu-iotests/149.out
tests/qemu-iotests/158
tests/qemu-iotests/178
tests/qemu-iotests/188
tests/qemu-iotests/189
tests/qemu-iotests/198
tests/qemu-iotests/233

I'll change, but I'm not sure what is the default we should enforce...

> 
>>   ciphers=/etc/crypto-policies/back-ends/openssl.config,\
>>   cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin
> 
> (I really should follow through on my threat to teach QemuOpts to ignore
> whitespace after ','; but for this commit message, it's obvious the
> indentation has to be stripped for the command line to be valid)
> 
>>
>> (On Fedora these files are provided by the ca-certificates and
>> crypto-policies packages).
>>
>> [*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---

Re: [Qemu-devel] 'make check' error

[Emilio G. Cota]

On Sat, Mar 09, 2019 at 13:53:32 +0800, Li Qiang wrote:
> Hi all, 
> 
> Today I ‘git clone’ && configure && make && make check 
> 
> And get following error, 
> 
> fp-test.c:50:10: fatal error: fail.h: No such file or directory
>  #include "fail.h"
>   ^~~~
> 
> I look at the commit:
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=3ac1f81329f4dfdc10a51e180f9cf28dbcb02a3c;hp=b44b5abeae4a3b54ccbd7137f59c0a8923cecec9
> 
> Seems it’s old commit, I think I got ‘make check’ work after this commit.
> So I don’t know anywhere wrong.
> 
> Any hints?

fail.h is part of berkeley-testfloat-3 -- I suspect the
berkeley-testfloat-3 git submodule wasn't checked out.

Make sure both berkeley-softfloat-3 and berkeley-testfloat-3 are
checked out at $src/tests/fp. If not, you can get them with
"git submodule init && git submodule update".

Hope that helps,

Emilio

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

[Peter Maydell]

On Fri, 8 Mar 2019 at 12:58, Kevin Wolf  wrote:
>
> The following changes since commit c4e0780ed1ffd056f205348d387a61b4136a45df:
>
>   Merge remote-tracking branch 
> 'remotes/vivier2/tags/linux-user-for-4.0-pull-request' into staging 
> (2019-03-07 18:40:43 +)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to e88153ea9a40009a8ae7648282c0eac1b7f5494f:
>
>   qcow2 spec: Describe string header extensions (2019-03-08 12:26:46 +0100)
>
> 
> Block layer patches:
>
> - qcow2: Support for external data files
> - qcow2: Default to 4KB for the qcow2 cache entry size
> - Apply block driver whitelist for -drive format=help
> - Several qemu-iotests improvements
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PULL 05/21] migration: Introduce ignore-shared capability

[Markus Armbruster]

"Dr. David Alan Gilbert (git)"  writes:

> From: Yury Kotov 
>
> We want to use local migration to update QEMU for running guests.
> In this case we don't need to migrate shared (file backed) RAM.
> So, add a capability to ignore such blocks during live migration.
>
> Signed-off-by: Yury Kotov 
> Message-Id: <20190215174548.2630-3-yury-ko...@yandex-team.ru>
> Reviewed-by: Dr. David Alan Gilbert 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 1fd7bbea9b..eab87340b2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -409,13 +409,16 @@
>  #   devices (and thus take locks) immediately at the end of 
> migration.
>  #   (since 3.0)
>  #
> +# @x-ignore-shared: If enabled, QEMU will not migrate shared memory (since 
> 4.0)

What exactly is considered "shared memory"?

Specifically: say you have an ivshmem-plain device.  Is its shared
memory migrated?

No objection to the pull request; if documentation improvements are
necessary, we can do them in a follow-up patch.

> +#
>  # Since: 1.2
>  ##
>  { 'enum': 'MigrationCapability',
>'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
> 'block', 'return-path', 'pause-before-switchover', 'x-multifd',
> -   'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate' ] }
> +   'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> +   'x-ignore-shared' ] }
>  
>  ##
>  # @MigrationCapabilityStatus:

Re: [Qemu-devel] [PATCH v3 7/8] multifd: Drop x-

[Markus Armbruster]

Juan Quintela  writes:

> We make it supported from now on.
>
> Reviewed-by: Dr. David Alan Gilbert 
> Signed-off-by: Juan Quintela 
> ---
>  hmp.c | 10 +-
>  migration/migration.c | 26 +-
>  qapi/migration.json   | 34 +-
>  3 files changed, 35 insertions(+), 35 deletions(-)

Acked-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v3 4/8] multifd: Drop x-multifd-page-count parameter

[Markus Armbruster]

Juan Quintela  writes:

> Libvirt don't want to expose (and explain it).  From now on we measure
> the number of packages in bytes instead of pages, so it is the same
> independently of architecture.  We choose the page size of x86.
> Notice that in the following patch we make this variable.
>
> Signed-off-by: Juan Quintela 
> Reviewed-by: Dr. David Alan Gilbert 

Acked-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH V3 4/7] Migration/colo.c: Add new COLOExitReason to handle all failover state

[Markus Armbruster]

Zhang Chen  writes:

> From: Zhang Chen 
>
> In this patch we add the processing state for COLOExitReason,
> because we have to identify COLO in the failover processing state or
> failover error state. In the way, we can handle all the failover state.
> We have improved the description of the COLOExitReason by the way.
>
> Signed-off-by: Zhang Chen 
> ---
>  migration/colo.c| 24 +---
>  qapi/migration.json | 15 +--
>  2 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 89325952c7..dbe2b88807 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -267,7 +267,11 @@ COLOStatus *qmp_query_colo_status(Error **errp)
   switch (failover_get_state()) {

failover_get_state() returns FailoverStatus, i.e. one of
FAILOVER_STATUS_NONE, _REQUIRE, _ACTIVE, _COMPLETED, _RELAUNCH.

   case FAILOVER_STATUS_NONE:
   s->reason = COLO_EXIT_REASON_NONE;
   break;
   case FAILOVER_STATUS_REQUIRE:
>  s->reason = COLO_EXIT_REASON_REQUEST;
>  break;
>  default:
> -s->reason = COLO_EXIT_REASON_ERROR;
> +if (migration_in_colo_state()) {
> +s->reason = COLO_EXIT_REASON_PROCESSING;
> +} else {
> +s->reason = COLO_EXIT_REASON_ERROR;
> +}
>  }
>  
>  return s;

In which FailoverStatus can migration_in_colo_state() return true?

> @@ -579,16 +583,13 @@ out:
   /*
* There are only two reasons we can get here, some error happened
>   * or the user triggered failover.
>   */
>  switch (failover_get_state()) {
> -case FAILOVER_STATUS_NONE:
> -qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
> -  COLO_EXIT_REASON_ERROR);
> -break;
>  case FAILOVER_STATUS_COMPLETED:
>  qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
>COLO_EXIT_REASON_REQUEST);
>  break;
>  default:
> -abort();
> +qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
> +  COLO_EXIT_REASON_ERROR);
>  }
>  
>  /* Hope this not to be too long to wait here */

Change of behavior for FAILOVER_STATUS_REQUIRE, _ACTIVE, _COMPLETED,
_RELAUNCH: send a COLO_EXIT event instead of calling abort().  Why?

> @@ -850,17 +851,18 @@ out:
>  error_report_err(local_err);
>  }
>  
> +/*
> + * There are only two reasons we can get here, some error happened
> + * or the user triggered failover.
> + */
>  switch (failover_get_state()) {
> -case FAILOVER_STATUS_NONE:
> -qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
> -  COLO_EXIT_REASON_ERROR);
> -break;
>  case FAILOVER_STATUS_COMPLETED:
>  qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
>COLO_EXIT_REASON_REQUEST);
>  break;
>  default:
> -abort();
> +qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
> +  COLO_EXIT_REASON_ERROR);
>  }
>  
>  if (fb) {

Same question.

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7a795ecc16..089ed67807 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -983,19 +983,22 @@
>  ##
>  # @COLOExitReason:
>  #
> -# The reason for a COLO exit
> +# Describe the reason for COLO exit.

The old text was better.

>  #
> -# @none: no failover has ever happened. This can't occur in the
> -# COLO_EXIT event, only in the result of query-colo-status.
> +# @none: failover has never happened. This state does not occur
> +# in the COLO_EXIT event, and is only visible in the result of
> +# query-colo-status.

This might be a small improvment.

>  #
> -# @request: COLO exit is due to an external request
> +# @request: COLO exit caused by an external request.
>  #
> -# @error: COLO exit is due to an internal error
> +# @error: COLO exit caused by an internal error.

I like the old text better.

> +#
> +# @processing: COLO is currently handling a failover (since 4.0).
>  #
>  # Since: 3.1
>  ##
>  { 'enum': 'COLOExitReason',
> -  'data': [ 'none', 'request', 'error' ] }
> +  'data': [ 'none', 'request', 'error' , 'processing' ] }
>  
>  ##
>  # @x-colo-lost-heartbeat:

The patch conflates addition of a new member with doc improvements.
Tolerable since both are small.

Re: [Qemu-devel] [PATCH 07/10] roms: build edk2 firmware binaries and variable store templates

[Philippe Mathieu-Daudé]

Hi Laszlo,

On 3/9/19 1:48 AM, Laszlo Ersek wrote:
> Add the "efi" target to "Makefile".
> 
> Introduce "Makefile.edk2" for building and cleaning the firmware images
> and varstore templates.
> 
> Collect the common bits from the recipes in the helper script
> "edk2-build.sh".
> 
> Signed-off-by: Laszlo Ersek 
> ---
>  roms/Makefile  |   5 +
>  roms/Makefile.edk2 | 138 
>  roms/edk2-build.sh |  55 
>  3 files changed, 198 insertions(+)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index 2e83ececa25a..054b432834ba 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -61,6 +61,7 @@ default:
>   @echo "  skiboot-- update skiboot.lid"
>   @echo "  u-boot.e500-- update u-boot.e500"
>   @echo "  u-boot.sam460  -- update u-boot.sam460"
> + @echo "  efi-- update UEFI (edk2) platform firmware"
>  
>  bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k
>   cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios.bin
> @@ -143,6 +144,9 @@ skiboot:
>   $(MAKE) -C skiboot CROSS=$(powerpc64_cross_prefix)
>   cp skiboot/skiboot.lid ../pc-bios/skiboot.lid
>  
> +efi: edk2-basetools
> + $(MAKE) -f Makefile.edk2
> +
>  clean:
>   rm -rf seabios/.config seabios/out seabios/builds
>   $(MAKE) -C sgabios clean
> @@ -153,3 +157,4 @@ clean:
>   rm -rf u-boot/build.e500
>   $(MAKE) -C u-boot-sam460ex distclean
>   $(MAKE) -C skiboot clean
> + $(MAKE) -f Makefile.edk2 clean
> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
> new file mode 100644
> index ..ad6fff044cd6
> --- /dev/null
> +++ b/roms/Makefile.edk2
> @@ -0,0 +1,138 @@
> +# Makefile for building firmware binaries and variable store templates for a
> +# number of virtual platforms in edk2.
> +#
> +# Copyright (C) 2019, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License that accompanies this
> +# distribution. The full text of the license may be found at
> +# .
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +toolchain = $(shell source ./edk2-funcs.sh && qemu_edk2_get_toolchain $(1))
> +
> +licenses := \
> + edk2/License.txt \
> + edk2/OvmfPkg/License.txt \
> + edk2/CryptoPkg/Library/OpensslLib/openssl/LICENSE
> +
> +# The "edk2-arm-vars.fd" varstore template is suitable for aarch64 as well.
> +# Similarly, the "edk2-i386-vars.fd" varstore template is suitable for x86_64
> +# as well, independently of "secure" too.
> +all: \
> + ../pc-bios/edk2-aarch64-code.fd \
> + ../pc-bios/edk2-arm-code.fd \
> + ../pc-bios/edk2-i386-code.fd \
> + ../pc-bios/edk2-i386-secure-code.fd \
> + ../pc-bios/edk2-x86_64-code.fd \
> + ../pc-bios/edk2-x86_64-secure-code.fd \
> + \
> + ../pc-bios/edk2-arm-vars.fd \
> + ../pc-bios/edk2-i386-vars.fd \
> + \
> + ../pc-bios/edk2-licenses.txt
> +
> +submodules:
> + cd edk2 && git submodule update --init --force
> +
> +# See notes on the ".NOTPARALLEL" target and the "+" indicator in
> +# "tests/uefi-test-tools/Makefile".
> +.NOTPARALLEL:
> +
> +../pc-bios/edk2-aarch64-code.fd: submodules
> + +./edk2-build.sh \
> + aarch64 \
> + --arch=AARCH64 \
> + --platform=ArmVirtPkg/ArmVirtQemu.dsc \
> + -D NETWORK_IP6_ENABLE \
> + -D HTTP_BOOT_ENABLE
> + cp edk2/Build/ArmVirtQemu-AARCH64/DEBUG_$(call 
> toolchain,aarch64)/FV/QEMU_EFI.fd \
> + $@
> + truncate --size=64M $@
[...]

Trying on Ubuntu I get:

$ make -C roms efi
[...]
Fd File Name:QEMU_EFI
(/home/phil/source/qemu/roms/edk2/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/FV/QEMU_EFI.fd)
Fd File Name:QEMU_VARS
(/home/phil/source/qemu/roms/edk2/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/FV/QEMU_VARS.fd)
GUID cross reference file can be found at
/home/phil/source/qemu/roms/edk2/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/FV/Guid.xref
- Done -
Build end time: 16:33:29, Mar.09 2019
Build total time: 00:03:35
cp edk2/Build/ArmVirtQemu-AARCH64/DEBUG_/FV/QEMU_EFI.fd \
../pc-bios/edk2-aarch64-code.fd
cp: cannot stat 'edk2/Build/ArmVirtQemu-AARCH64/DEBUG_/FV/QEMU_EFI.fd':
No such file or directory
Makefile.edk2:45: recipe for target '../pc-bios/edk2-aarch64-code.fd' failed
make[1]: *** [../pc-bios/edk2-aarch64-code.fd] Error 1
make[1]: Leaving directory '/home/phil/source/qemu/roms'
Makefile:148: recipe for target 'efi' failed
make: *** [efi] Error 2
make: Leaving directory '/home/phil/source/qemu/roms'

The edk2-build.sh script source edksetup.sh, then you call the
'toolchain' command out of the edk2-build.sh script, but here
the edksetup.sh setup is no more effective.

Regards,

Phil.

Re: [Qemu-devel] [PATCH 2/9] tcg: Add INDEX_op_extract2_{i32,i64}

[Richard Henderson]

On 3/8/19 3:28 PM, Philippe Mathieu-Daudé wrote:
>> +CASE_OP_32_64(extract2):
>> +if (arg_is_const(op->args[1]) && arg_is_const(op->args[2])) {
>> +TCGArg v1 = arg_info(op->args[1])->val;
>> +TCGArg v2 = arg_info(op->args[2])->val;
>> +tmp = (v1 >> op->args[3]) | (v2 << (64 - op->args[3]));
> Shouldn't this be:
> 
> tmp = (v1 >> op->args[3]) | (v2 << (TCG_TARGET_REG_BITS - op->args[3]));

No, but there should be different constants for the two cases.

Thanks.


r~

Re: [Qemu-devel] [PATCH v6 01/14] qapi: qapi for audio backends

[Zoltán Kővágó]

On 2019-03-09 10:27, Markus Armbruster wrote:
> "Kővágó, Zoltán"  writes:
> 
>> This patch adds structures into qapi to replace the existing
>> configuration structures used by audio backends currently. This qapi
>> will be the base of the -audiodev command line parameter (that replaces
>> the old environment variables based config).
>>
>> This is not a 1:1 translation of the old options, I've tried to make
>> them much more consistent (e.g. almost every backend had an option to
>> specify buffer size, but the name was different for every backend, and
>> some backends required usecs, while some other required frames, samples
>> or bytes). Also tried to reduce the number of abbreviations used by the
>> config keys.
>>
>> Some of the more important changes:
>> * use `in` and `out` instead of `ADC` and `DAC`, as the former is more
>>   user friendly imho
>> * moved buffer settings into the global setting area (so it's the same
>>   for all backends that support it. Backends that can't change buffer
>>   size will simply ignore them). Also using usecs, as it's probably more
>>   user friendly than samples or bytes.
>> * try-poll is now an alsa backend specific option (as all other backends
>>   currently ignore it)
>>
>> Signed-off-by: Kővágó, Zoltán 
>> Reviewed-by: Markus Armbruster 
>> ---
>>
>> Notes:
>> Changes from v5:
>> 
>> * documentation fixes
>> * renamed buffer-len to buffer-length and period-len to period-length
>> 
>> Changes from v4:
>> 
>> * documentation fixes
>> * renamed pa's source/sink to pa-in/pa-out
>> * per-direction options changed per Markus Armbruster's comments
>> 
>> Changes from v2:
>> 
>> * update copyright, version numbers
>> * remove #optional
>> * per-direction options are now optional (needed for 
>> qobject_object_visitor_new_str)
>> * removed unnecessary AudiodevNoOptions
>> * changed integers to unsigned
>>
>>  qapi/audio.json   | 304 ++
>>  qapi/qapi-schema.json |   1 +
>>  qapi/Makefile.objs|   6 +-
>>  3 files changed, 308 insertions(+), 3 deletions(-)
>>  create mode 100644 qapi/audio.json
>>
>> diff --git a/qapi/audio.json b/qapi/audio.json
>> new file mode 100644
>> index 00..97aee37288
>> --- /dev/null
>> +++ b/qapi/audio.json
>> @@ -0,0 +1,304 @@
>> +# -*- mode: python -*-
>> +#
>> +# Copyright (C) 2015-2019 Zoltán Kővágó 
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +# See the COPYING file in the top-level directory.
>> +
>> +##
>> +# @AudiodevPerDirectionOptions:
>> +#
>> +# General audio backend options that are used for both playback and
>> +# recording.
>> +#
>> +# @fixed-settings: use fixed settings for host input/output. When off,
>> +#  frequency, channels and format must not be
>> +#  specified (default true)
>> +#
>> +# @frequency: frequency to use when using fixed settings
>> +# (default 44100)
>> +#
>> +# @channels: number of channels when using fixed settings (default 2)
>> +#
>> +# @voices: number of voices to use (default 1)
>> +#
>> +# @format: sample format to use when using fixed settings
>> +#  (default s16)
>> +#
>> +# @buffer-length: the buffer length in microseconds
> 
> The name buffer-length suggests bytes rather than microseconds.  That's
> why I suggested @buffer-capacity.  Matter of taste, up to you and Gerd.

@period-length has the same issue.  For me @buffer-capacity and
@period-capacity feels a bit weird.  I'm not 100% familiar with qapi
conventions, but for me 'size' is what's usually in bytes, and 'length'
can be something else (number of characters in a string, length of a
sound file, ...).

> 
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'struct': 'AudiodevPerDirectionOptions',
>> +  'data': {
>> +'*fixed-settings': 'bool',
>> +'*frequency':  'uint32',
>> +'*channels':   'uint32',
>> +'*voices': 'uint32',
>> +'*format': 'AudioFormat',
>> +'*buffer-length':  'uint32' } }
> [...]
> 
> Regardless:
> Reviewed-by: Markus Armbruster 
> 

Regards,
Zoltan

Re: [Qemu-devel] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize()

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> Back in abe147e0ce4 when fw_cfg_add_file() was introduced, there
> was no QOM design, object where not created and released at runtime.
> Later 38f3adc34d finished the QOM conversion of the fw_cfg device,
> adding the fw_cfg_common_realize() method.
> The time has come to add the equivalent destructor and release the
> memory allocated for 'files'.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/nvram/fw_cfg.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index b2dc0a80cb..0fb020edce 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -959,6 +959,13 @@ static void fw_cfg_common_realize(DeviceState *dev, 
> Error **errp)
>  qemu_add_machine_init_done_notifier(&s->machine_ready);
>  }
>  
> +static void fw_cfg_common_unrealize(DeviceState *dev, Error **errp)
> +{
> +FWCfgState *s = FW_CFG(dev);
> +
> +g_free(s->files);
> +}
> +

Still leaks at least s->entries[0], s->entries[1], s->entry_order,
doesn't it?

>  FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
>  AddressSpace *dma_as)
>  {
> @@ -1127,6 +1134,7 @@ static void fw_cfg_io_class_init(ObjectClass *klass, 
> void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  
>  dc->realize = fw_cfg_io_realize;
> +dc->unrealize = fw_cfg_common_unrealize;
>  dc->props = fw_cfg_io_properties;
>  }
>  
> @@ -1190,6 +1198,7 @@ static void fw_cfg_mem_class_init(ObjectClass *klass, 
> void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  
>  dc->realize = fw_cfg_mem_realize;
> +dc->unrealize = fw_cfg_common_unrealize;
>  dc->props = fw_cfg_mem_properties;
>  }

Re: [Qemu-devel] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command

[Markus Armbruster]

The subject "Add QMP 'info fw_cfg' command" misspells query-fw_cfg-items
:)

Eric Blake  writes:

> On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote:
>> When debugging a paravirtualized guest firmware, it results very
>> useful to dump the fw_cfg table.
>> Add a QMP command which returns the most useful fields.
>> Since the QMP protocol is not designed for passing stream data,
>> we don't display a fw_cfg item data, only it's size:
>> 
>> { "execute": "query-fw_cfg-items" }
>> {
>> "return": [
>> {
>> "architecture_specific": false,
>> "key": 0,
>> "writeable": false,
>> "size": 4,
>> "keyname": "signature"
>
> You could return a base64-encoded representation of the field (we do
> that in other interfaces where raw binary can't be passed reliably over
> JSON).  For 4 bytes, that makes sense,
>
>
>> {
>> "architecture_specific": true,
>> "key": 3,
>> "writeable": false,
>> "size": 324,
>> "keyname": "e820_tables"
>
> for 324 bytes, it gets a bit long. Still, may be worth it for the added
> debug visibility.
>
>
>> +++ b/qapi/misc.json
>> @@ -3051,3 +3051,47 @@
>>'data': 'NumaOptions',
>>'allow-preconfig': true
>>  }
>> +
>> +##
>> +# @FirmwareConfigurationItem:
>> +#
>> +# Firmware Configuration (fw_cfg) item.
>> +#
>> +# @key: The uint16 selector key.
>> +# @keyname: The stringified name if the selector refers to a well-known
>> +#   numerically defined item.

Ignorant questions, I'm afraid...

What determines the possible values of @key?

What's the difference between a "well-known" and a "not well-known"
value?  Examples?

>> +# @architecture_specific: Indicates whether the configuration setting is
>
> Prefer '-' over '_' in new interfaces.
>
>> +# architecture specific.
>> +#  false: The item is a generic configuration item.
>> +#  true:  The item is specific to a particular architecture.
>> +# @writeable: Indicates whether the configuration setting is writeable by
>> +# the guest.
>
> writable
>
>> +# @size: The length of @data associated with the item.
>> +# @data: A string representating the firmware configuration data.
>
> representing
>
>> +#Note: This field is currently not used.
>
> Either drop the field until it has a use, or let it be the base64
> encoding and use it now.
>
>> +# @path: If the key is a 'file', the named file path.
>> +# @order: If the key is a 'file', the named file order.
>> +#
>> +# Since 4.0
>> +##
>> +{ 'struct': 'FirmwareConfigurationItem',
>> +  'data': { 'key': 'uint16',
>> +'*keyname': 'str',
>> +'architecture_specific': 'bool',
>> +'writeable': 'bool',
>> +'*data': 'str',
>> +'size': 'int',
>> +'*path': 'str',
>> +'*order': 'int' } }
>
> Is it worth making 'keyname' an enum type, and turning this into a flat
> union where 'path' and 'order' appear on the branch associated with
> 'file', and where all other well-known keynames have smaller branches?

Discriminator can't be optional.  Obvious solution: add a suitabable
enum value for "other" key values.

Leads to something like this (untested):

{ 'union': 'FirmwareConfigurationItem',
  'base': { 'key': 'uint16',
'keyname': 'FirmwareConfigurationKey',
... more members that make sense regardless of @key ... },
  'discriminator': 'key',
  'data': {
'file': 'FirmwareConfigurationItemFile',
... more variants, if any ... } }

where 'FirmwareConfigurationItemFile' is a struct type containing the
members that make sense just for 'file'.

>> +
>> +
>> +##
>> +# @query-fw_cfg-items:
>
> That looks weird to mix - and _. Any reason we can't just go with
> query-firmware-config?
>
>> +#
>> +# Returns the list of Firmware Configuration items.
>> +#
>> +# Returns: A list of @FirmwareConfigurationItem for each entry.
>> +#
>> +# Since 4.0
>> +##
>> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']}
>> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 06/18] hw/nvram/fw_cfg: Remove the unnecessary boot_splash_filedata_size

[Laurent Vivier]

On 08/03/2019 07:49, Thomas Huth wrote:
> On 08/03/2019 02.32, Philippe Mathieu-Daudé wrote:
>> The 'boot_splash_filedata_size' was introduced as a global variable
>> in 3d3b8303c6f. This variable is used as a 'size' argument to the
>> fw_cfg_add_file(). This function has an interface contract with his
>> 'data' argument, but there is no such contract for 'size' (this is
>> not a referenced pointer).  We can simply remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  hw/nvram/fw_cfg.c   | 5 ++---
>>  include/sysemu/sysemu.h | 1 -
>>  vl.c| 1 -
>>  3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 8eb76a382c..b2dc0a80cb 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -217,15 +217,14 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>>  }
>>  g_free(boot_splash_filedata);
>>  boot_splash_filedata = (uint8_t *)file_data;
>> -boot_splash_filedata_size = file_size;
>>  
>>  /* insert data */
>>  if (file_type == JPG_FILE) {
>>  fw_cfg_add_file(s, "bootsplash.jpg",
>> -boot_splash_filedata, boot_splash_filedata_size);
>> +boot_splash_filedata, file_size);
>>  } else {
>>  fw_cfg_add_file(s, "bootsplash.bmp",
>> -boot_splash_filedata, boot_splash_filedata_size);
>> +boot_splash_filedata, file_size);
>>  }
>>  g_free(filename);
>>  }
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 89604a8328..6065d9e420 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -110,7 +110,6 @@ extern int old_param;
>>  extern int boot_menu;
>>  extern bool boot_strict;
>>  extern uint8_t *boot_splash_filedata;
>> -extern size_t boot_splash_filedata_size;
>>  extern bool enable_mlock;
>>  extern bool enable_cpu_pm;
>>  extern QEMUClockType rtc_clock;
>> diff --git a/vl.c b/vl.c
>> index 4c5cc0d8ad..fad6fec38c 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -188,7 +188,6 @@ const char *prom_envs[MAX_PROM_ENVS];
>>  int boot_menu;
>>  bool boot_strict;
>>  uint8_t *boot_splash_filedata;
>> -size_t boot_splash_filedata_size;
>>  bool wakeup_suspend_enabled;
>>  
>>  int icount_align_option;
>>
> 
> Nice, one global variable less!
> 
> Reviewed-by: Thomas Huth 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [Qemu-devel] [Qemu-trivial] [Qemu-ppc] [PATCH v2 02/18] hw/i386: Remove unused include

[Laurent Vivier]

On 08/03/2019 12:32, Thomas Huth wrote:
> On 08/03/2019 02.32, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> v2: Drop files that do use fw_cfg (Michael):
>> - hw/i386/acpi-build.c
>> - hw/i386/pc.c
>> ---
>>  hw/acpi/piix4.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>> index 8fd25a5926..7b98121070 100644
>> --- a/hw/acpi/piix4.c
>> +++ b/hw/acpi/piix4.c
>> @@ -28,7 +28,6 @@
>>  #include "sysemu/sysemu.h"
>>  #include "qapi/error.h"
>>  #include "qemu/range.h"
>> -#include "hw/nvram/fw_cfg.h"
>>  #include "exec/address-spaces.h"
>>  #include "hw/acpi/piix4.h"
>>  #include "hw/acpi/pcihp.h"
>>
> 
> Reviewed-by: Thomas Huth 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [Qemu-devel] [Qemu-trivial] [PATCH] thunk: improve readability of allocation loop

[Laurent Vivier]

On 08/03/2019 20:43, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta 
> ---
>  thunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/thunk.c b/thunk.c
> index 17f3d320bb..7f31cffe09 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -86,7 +86,7 @@ void thunk_register_struct(int id, const char *name, const 
> argtype *types)
>  #endif
>  /* now we can alloc the data */
>  
> -for(i = 0;i < 2; i++) {
> +for (i = 0; i < ARRAY_SIZE(se->field_offsets); i++) {
>  offset = 0;
>  max_align = 1;
>  se->field_offsets[i] = g_new(int, nb_fields);
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize()

[Markus Armbruster]

Laszlo Ersek  writes:

> On 03/08/19 07:55, Thomas Huth wrote:
>> On 08/03/2019 02.32, Philippe Mathieu-Daudé wrote:
>>> Back in abe147e0ce4 when fw_cfg_add_file() was introduced, there

Suggest to say "commit abe147e0ce4".

>>> was no QOM design, object where not created and released at runtime.
>
> s/object where/objects were/
>
>>> Later 38f3adc34d finished the QOM conversion of the fw_cfg device,
>>> adding the fw_cfg_common_realize() method.
>
> (I'm not sure if 38f3adc34d "finished the QOM conversion", but that's
> just my lack of QOM knowledge. Hopefully someone can confirm whether
> this statement is accurate.)
>
>>> The time has come to add the equivalent destructor and release the
>>> memory allocated for 'files'.
>> 
>> You should mention that the unrealize function is currently never called
>> since the object never gets destroyed (AFAIK). But I hope we can fix
>> that in the not so distant future, so:
>> 
>> Reviewed-by: Thomas Huth 
>> 
>
> How about we delay this patch until such time the function is actually
> called?
>
> This series is already huge, and quite divergent considering its goals.
> (Please refer to the blurb.)
>
> I don't mind this patch, but I think it should either belong to a
> separate fw_cfg cleanup series (or "wave" if you like).
>
> Or else, we should have a bug reported somewhere public, ensuring that
> we eventually call the function introduced here. And then the commit
> message should spell out -- as you say -- that the function is not
> called yet, but it will be, because of .

I suspect filing a bug "somewhere" and making use of it would be much
more work than simply applying the fix.

I'm fine with including trivial patches for stuff you spot "on the way".
I'd be also fine with some patches spun off into a separate cleanup
series that László doesn't have to review, if you can do that safely,
and with little effort.

Re: [Qemu-devel] [PULL 0/5] Vga 20190308 patches

[Peter Maydell]

On Fri, 8 Mar 2019 at 12:42, Gerd Hoffmann  wrote:
>
> The following changes since commit 32694e98b8d7a246345448a8f707d2e11d6c65e2:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/machine-next-pull-request' into staging (2019-03-06 
> 18:52:19 +)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/vga-20190308-pull-request
>
> for you to fetch changes up to 61dbd9eaa1450f94886fb41366e4687d405c52c8:
>
>   virtio-gpu: make virtio_gpu_reset static (2019-03-08 09:36:23 +0100)
>
> 
> vga: virtio reset fix, add ati emulation.
>
> 

Hi -- I'm afraid this has conflicted with the Kconfig merge:

CONFLICT (modify/delete): default-configs/pci.mak deleted in HEAD and
modified in remotes/kraxel/tags/vga-20190308-pull-request.

Can you figure out what the Kconfig equivalent of the
pci.mak change is and resend, please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Added periodic IRQ support for bcm2836_control local timer

[Peter Maydell]

On Sat, 9 Mar 2019 at 01:03, bzt  wrote:
> Thanks for your answers. If I don't clear the INTENABLE flag, then the
> IRQ would keep firing constantly. This is not how the real hardware
> works: it triggers the IRQ once, and then it inhibits. The timer won't
> trigger the IRQ again until you acknowledge it by writing the INTFLAG
> into the ack register. My solution emulates this behaviour. That's
> what the triggered flag was for in my original patch. Should I bring
> that flag back or is the current solution acceptable knowing this?

Huh. The QA7 spec doc is pretty clear that the IRQ is kept high
until the guest acknowledges it (and that is how in general
IRQ/FIQ works for Arm -- it is level triggered and it stays high
until the guest silences the device):
"An interrupt is generated as long as the interrupt flag is set
and the interrupt-enable bit is set" and "The user must clear the
interrupt flag".


thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 03/18] cutils: Add qemu_strdup_hexlify() and qemu_strdup_unhexlify()

[Markus Armbruster]

Laszlo Ersek  writes:

> Hi Phil,
>
> most important comment at the bottom.
>
> On 03/08/19 02:32, Philippe Mathieu-Daudé wrote:
>> Add two helpers: one to represent a binary data as a string of
>> hexadecimal values, and one to restore a such string into its
>> original binary data.
>> 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  include/qemu/cutils.h | 33 ++
>>  util/cutils.c | 55 +++
>>  2 files changed, 88 insertions(+)
>> 
>> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
>> index d2dad3057c..375a5508b0 100644
>> --- a/include/qemu/cutils.h
>> +++ b/include/qemu/cutils.h
>> @@ -171,6 +171,39 @@ bool test_buffer_is_zero_next_accel(void);
>>  int uleb128_encode_small(uint8_t *out, uint32_t n);
>>  int uleb128_decode_small(const uint8_t *in, uint32_t *n);
>>  
>> +/**
>> + * qemu_strdup_hexlify:
>
> (1) I think the name "hexlify" is unusual.

hexlify-buffer is an interactive autoloaded Lisp function in
‘hexl.el’.

(hexlify-buffer)

Convert a binary buffer to hexl format.
This discards the buffer’s undo information.

;-P

>I think we should use
> encode/decode terminology, or hex/unhex, or, if we want to stick with
> the "stringify" pattern, hexify/unhexify. (No "l".)
>
>> + *
>> + * Encode a sequence of binary data into its hexadecimal stringified
>> + * representation.
>> + *
>> + * @ptr: Buffer to hexlify

Similar parameters elsewhere in this header are called @buf.

>> + * @size: Length of the buffer
>> + *
>> + * Use qemu_strdup_unhexlify() to convert the hex string to original data.
>> + *
>> + * Returns: A newly allocated, zero-terminated hex encoded string 
>> representing
>> + * the data. The returned string must be freed with g_free().
>> + */
>> +gchar *qemu_strdup_hexlify(gconstpointer ptr, gsize size);

Avoid the silly GLib types, please.

>> +
>> +/**
>> + * qemu_strdup_unhexlify:
>> + *
>> + * Decode a sequence of hexadecimal encoded text into binary data.
>> + *
>> + * @hex_string: String to unhexlify
>> + * @out_size: if not NULL: gsize to be written with the data length
>> + *
>> + * This function is the opposite of qemu_strdup_hexlify().
>> + *
>> + * Returns: A newly allocated buffer containing the binary data that text
>> + * represents. The returned buffer must be freed with g_free().
>> + * Note that the returned binary data is not necessarily zero-terminated,
>> + * so it should not be used as a character string.
>> + */
>> +gpointer qemu_strdup_unhexlify(const gchar *hex_string, gsize *out_size);
>> +
>>  /**
>>   * qemu_pstrcmp0:
>>   * @str1: a non-NULL pointer to a C string (*str1 can be NULL)
>> diff --git a/util/cutils.c b/util/cutils.c
>> index e098debdc0..bf324c0d8b 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -779,6 +779,61 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n)
>>  }
>>  }
>>  
>> +static guchar hexval(const gchar v)

Naming the parameter @ch would be more idiomatic, I think.

>> +{
>> +switch (v) {
>> +case '0' ... '9':
>> +return v - '0';
>> +case 'A' ... 'F':
>> +return v - 'A' + 10;
>> +case 'a' ... 'f':
>> +return v - 'a' + 10;
>> +default:
>> +return 0;
>> +}
>> +}
>
> (2) I don't think that we should silently translate invalid characters
> to zero, in any hexadecimal decoder.

Yup.  Let's abort().

>> +
>> +gchar *qemu_strdup_hexlify(gconstpointer ptr, gsize len)
>> +{
>> +guchar *data = (guchar *)ptr;
>> +gchar *hex_string;
>> +
>> +if (!ptr || !len) {
>> +return g_strdup("");
>> +}

A null pointer is not the same as the empty string.  Replace this by

assert(ptr);

and ...

>> +
>> +hex_string = g_malloc(2 * len + 1);
>
> (3) Should check against integer overflow in the g_malloc() argument
> (multiplication and addition).

E.g.
assert(len <= (SIZE_MAX - 1) / 2);

>> +for (gsize i = 0; i < len; i++) {
>> +g_snprintf(&hex_string[2 * i], 3, "%02x", data[i]);
>> +}
>> +

... esnure termination here

hex_string[2 * i] = 0;

What does g_snprintf() buy us over plain snprintf()?  I count 400+ uses
of the latter, and none of the former.

>> +return hex_string;
>> +}
>> +
>> +gpointer qemu_strdup_unhexlify(const gchar *hex_string, gsize *out_size)
>> +{
>> +size_t size = 0;
>> +guchar *data = NULL;
>> +
>> +if (hex_string) {

A null pointer is not the same as the empty string.  assert(hex_string)
and make the conversion unconditional.


>> +size = strlen(hex_string) / 2;
>
> (4) Should likely check that the length of the string is an even integer.
>
>> +if (size) {
>> +size_t i;
>> +
>> +data = g_new(guchar, size + 1);
>> +for (i = 0; i < size; i++) {
>> +data[i]  = hexval(*hex_string++) << 4;
>> +data[i] |= hexval(*hex_string++);
>> +}
>> +data[i] = '\0';

Re: [Qemu-devel] [PATCH] thunk: improve readability of allocation loop

[Stefan Hajnoczi]

On Fri, Mar 8, 2019 at 7:43 PM Aarushi Mehta  wrote:
>
> Signed-off-by: Aarushi Mehta 
> ---
>  thunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH v5 1/2] Add generic Nios II board.

[Peter Maydell]

On Fri, 8 Mar 2019 at 23:51, Sandra Loosemore  wrote:
>
> On 3/7/19 7:57 AM, Peter Maydell wrote:
> >> diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
> >> index 5f0ab2f..c697047 100644
> >> --- a/hw/nios2/boot.c
> >> +++ b/hw/nios2/boot.c
> >> @@ -140,6 +140,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
> >>   uint64_t entry, low, high;
> >>   uint32_t base32;
> >>   int big_endian = 0;
> >> +int kernel_space = 0;
> >>
> >>   #ifdef TARGET_WORDS_BIGENDIAN
> >>   big_endian = 1;
> >> @@ -155,10 +156,12 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr 
> >> ddr_base,
> >>  translate_kernel_address, NULL,
> >>  &entry, NULL, NULL,
> >>  big_endian, EM_ALTERA_NIOS2, 0, 0);
> >> +kernel_space = 1;
> >>   }
> >>
> >>   /* Always boot into physical ram. */
> >> -boot_info.bootstrap_pc = ddr_base + 0xc000 + (entry & 
> >> 0x07ff);
> >> +boot_info.bootstrap_pc = ddr_base + (kernel_space ? 0xc000 : 
> >> 0)
> >> + + (entry & 0x07ff);
> >
> > It's not clear to me what's going on here, or why an
> > entry address of 0xc000_ is special, but I don't
> > know anything about NiosII -- maybe it's clear if you do?
>
> The processor reference guide documents that the kernel is placed at
> virtual memory address 0xc000.
>
> https://www.intel.com/content/www/us/en/programmable/documentation/iga1409256728501.html#iga1409332620358
>
> The problem the patch to boot.c is trying to solve is that we found
> things like unwind info were screwed up when using -kernel to load
> executables with an entry address other than 0xc000.

Ah, so this is one of those architectures like MIPS that
has multiple aliases of the same lump of physical RAM with
different properties. So an ELF file could be linked to
load in one of multiple aliases.

> > Why isn't the bootstrap_pc just always the entry address ?
> > Some comments on what is being done here and the use cases
> > being addressed might assist. I wasn't able to work out what
> > the remarks in the commit message meant, I'm afraid.
> >
> > Looking at the code, I don't think that the second call to
> > load_elf() will return a different entry address to the first
> > one (ie translate_kernel_address() is not applied to &entry).
> > That means that kernel_space is only true if entry == 0xc000,
> > and
> >(kernel_space ? 0xc000 : 0) + (entry & 0x07ff);
> > is almost the same thing as just 'entry'.
>
> It seems like these remarks are directed more at the existing code than
> the patch  :-S  TBH, I don't know why it was done that way originally.

Yeah, some of this is the fault of the original code, which is
doing weird things and not documenting why. But you're changing
this code, so you're currently the best expert we have at
what the architecture should do. So I'm trying to figure out
by asking you questions what the architecture does and what
the code is intended to do.

Right now my impression from reading the code and this patch is
that the existing code is doing something odd and possibly
wrong but which happens to work for whatever set of executables
it was being tested with, and then this patch is layering on a
workaround that fixes some other set of use cases -- but it would
be better if we could figure out what the real hardware does and
do that, which should then work in all the cases we're trying to
support (or we can say "no, these binaries are built wrongly and
they wouldn't load on the real hardware either"). If the real
hardware is doing something odd that's OK, but then we should
have a nice clear comment describing what the hardware does.

> > The description says this is "generic", but it appears to
> > be almost identical to the existing 10M50 board model,
> > including having exactly the same devices at the same
> > apparently arbitrary addresses.
> >
> > Could we instead add a machine parameter to the existing
> > board so you could say "-machine 10m50-ghrd,mmu=no"
> > (and drop the other changes -- it's not clear what they're
> > needed for) ?
> >
> > If it really ought to be a separate board model, perhaps
> > it should be in the same source file and share the common
> > code.
>
> I didn't write this code, but the intent was actually to allow
> executables linked for the 3c120 devboards we'd been using for hardware
> testing to run in this emulation; not to emulate a mmu-less 10m50 board.

That sounds like the right answer is "provide a model of a 3c120
board", not "provide something that is labelled generic but
is actually a 10m50 with no MMU" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] usb-mtp: fix return status of delete

[Peter Maydell]

On Fri, 8 Mar 2019 at 22:14, Bandan Das  wrote:
>
>
> Spotted by Coverity: CID 1399414
>
> mtp delete allows the a return status of delete succeeded,
> partial_delete or readonly - when none of the objects could be
> deleted.
>
> Some initiators recurse over the objects themselves. In that case,
> only READ_ONLY can be returned.
>
> Signed-off-by: Bandan Das 
> ---
>  hw/usb/dev-mtp.c | 25 +
>  1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 06e376bcd2..e3401aad75 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1137,9 +1137,9 @@ static MTPData *usb_mtp_get_object_prop_value(MTPState 
> *s, MTPControl *c,
>
>  /* Return correct return code for a delete event */
>  enum {
> -ALL_DELETE,
> -PARTIAL_DELETE,
> +ALL_DELETE = 1,
>  READ_ONLY,
> +PARTIAL_DELETE,

This is an enum, ie a set of incrementing values, but you're
using them as bit positions you're ORing together. If they're
really bit flags you should define them properly that way,
so it's clear what you're doing.

At the moment PARTIAL_DELETE is "ALL_DELETE | READ_ONLY", which
doesn't seem like it makes much sense.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH] docs/booting.rst: start documenting the boot process

[Peter Maydell]

On Fri, 8 Mar 2019 at 21:16, Alex Bennée  wrote:
>
> While working on some test cases I realised there was quite a lot of
> assumed knowledge about how things boot up. I thought it would be
> worth gathering this together in a user facing document where we could
> pour in the details and background to the boot process. As it's quite
> wordy I thought it should be a separate document to the manual (which
> can obviously reference this). So far I've managed almost a thousand
> words and haven't actually related anything to QEMU's options yet.
>
> So going forward:
>
>   - is this a useful document to have in docs?
>   - are there any other areas to mention?
> - out auto-magic -bios selection seems like something worth covering
>   - should we have some worked examples of command lines?
> - I was thinking for example of read-only and pflash examples
>   - we should also describe why direct kernel boots exits
> - and also the fact they are not that direct (some code executes
>   before a kernel - even if it's less than a full firmware)
>
> Submitted for comment
>
> Signed-off-by: Alex Bennée 
> ---
>  docs/booting.rst | 127 +++
>  1 file changed, 127 insertions(+)
>  create mode 100644 docs/booting.rst

We're trying to move towards having no documents in
the root docs/ directory, but instead having them all
in one of the various subdirectories that will each
be one of our five manuals:
https://wiki.qemu.org/Features/Documentation

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 01/18] hw/arm/virt: Remove null-check in virt_build_smbios()

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> Since commit 578f3c7b0835 ("arm: add fw_cfg to "virt" board",
> 2014-12-22), the machvirt_init() unconditionally creates the
> fw_cfg object.  Later, commit c30e15658b1b ("smbios: implement
> smbios support for mach-virt", 2015-09-07) added a superfluous
> null-check on it.
> Remove this superfluous check.
>
> Fixes: c30e15658b1b

I'd use 'Fixes:' only for bug fixes.  This is a cleanup

> Reviewed-by: Laszlo Ersek 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PULL 3/4] usb-mtp: prevent null dereference while deleting objects

[Peter Maydell]

On Fri, 8 Mar 2019 at 19:46, Bandan Das  wrote:
> This is very broken! I think something like this should work:
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 06e376bcd2..87a4bfb415 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1138,8 +1138,8 @@ static MTPData *usb_mtp_get_object_prop_value(MTPState 
> *s, MTPControl *c,
>  /* Return correct return code for a delete event */
>  enum {
>  ALL_DELETE,
> -PARTIAL_DELETE,
>  READ_ONLY,
> +PARTIAL_DELETE,
>  };

This is defining these values as an incrementing series...


>  if (o->format == FMT_UNDEFINED_OBJECT) {
>  if (remove(o->path)) {
> -partial_delete = true;
> +ret |= READ_ONLY;
>  } else {
>  usb_mtp_object_free_one(s, o);
> -success = true;
> +ret |= ALL_DELETE;

...but here we're using them as bits which we OR together.
In particular ALL_DELETE is 0, so ORing it in will
do nothing.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/2] tricore patches

[Peter Maydell]

On Fri, 8 Mar 2019 at 09:59, Bastian Koppelmann
 wrote:
>
> The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6:
>
>   Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' 
> into staging (2019-03-07 16:16:02 +)
>
> are available in the Git repository at:
>
>   https://github.com/bkoppelmann/qemu-tricore-upstream.git 
> tags/pull-tricore-2019-03-08
>
> for you to fetch changes up to d1c1d88ce6fd768cb98cd7af81b1b23ac9fe168d:
>
>   tricore: fixed RCR_CADDN instruction (2019-03-08 10:00:59 +0100)
>
> 
> Fixes mixed up operands in CADDN and CADD
>
> 
> David Brenken (2):
>   tricore: fixed RCR_CADD instruction
>   tricore: fixed RCR_CADDN instruction
>
>  target/tricore/translate.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> --

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH v3 00/14] pflash: Fixes and cleanups

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190307130323.9353-1-arm...@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  hw/display/cirrus_vga.o
  CC  hw/display/cirrus_vga_isa.o
/tmp/qemu-test/src/hw/block/pflash_cfi02.c: In function 
'pflash_cfi02_unrealize':
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:698:5: error: unknown type name 
'pflash_t'; did you mean 'fpos_t'?
 pflash_t *pfl = CFI_PFLASH02(dev);
 ^~~~
 fpos_t
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:698:21: error: implicit declaration 
of function 'CFI_PFLASH02'; did you mean 'HW_FLASH_H'? 
[-Werror=implicit-function-declaration]
 pflash_t *pfl = CFI_PFLASH02(dev);
 ^~~~
 HW_FLASH_H
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:698:21: error: nested extern 
declaration of 'CFI_PFLASH02' [-Werror=nested-externs]
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:698:21: error: initialization of 
'int *' from 'int' makes pointer from integer without a cast 
[-Werror=int-conversion]
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:699:19: error: request for member 
'timer' in something not a structure or union
 timer_del(&pfl->timer);
   ^~
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190307130323.9353-1-arm...@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH 00/10] bundle edk2 platform firmware with QEMU

[Michal Prívozník]

On 3/9/19 1:48 AM, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/qemu.git
> Branch: edk2_build
> 
> This series advances the roms/edk2 submodule to the "edk2-stable201903"
> release, and builds and captures platform firmware binaries from that
> release. At this point they are meant to be used by both end-users and
> by Igor's ACPI unit tests in qtest ("make check").
> 
> Previous discussion:
> 
>   [Qemu-devel] bundling edk2 platform firmware images with QEMU
>   80f0bae3-e79a-bb68-04c4-1c9c684d95b8@redhat.com">http://mid.mail-archive.com/80f0bae3-e79a-bb68-04c4-1c9c684d95b8@redhat.com
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02601.html
> 
> Note that the series was formatted with "--no-binary" (affecting patch
> #8), therefore it cannot be applied with "git-am". See the remote
> repo/branch reference near the top instead.
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (10):
>   roms: lift "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"
>   roms/edk2-funcs.sh: require gcc-4.8+ for building i386 and x86_64
>   tests/uefi-test-tools/build.sh: work around TianoCore#1607
>   roms/edk2: advance to tag edk2-stable201903
>   roms/edk2-funcs.sh: add the qemu_edk2_get_thread_count() function
>   roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
>   roms: build edk2 firmware binaries and variable store templates
>   pc-bios: add edk2 firmware binaries and variable store templates
>   pc-bios: document the edk2 firmware images; add firmware descriptors
>   Makefile: install the edk2 firmware images and their descriptors
> 
>  Makefile   |  17 +-
>  pc-bios/README |  11 +
>  pc-bios/descriptors/50-edk2-i386-secure.json   |  34 +++
>  pc-bios/descriptors/50-edk2-x86_64-secure.json |  35 +++
>  pc-bios/descriptors/60-edk2-aarch64.json   |  31 +++
>  pc-bios/descriptors/60-edk2-arm.json   |  31 +++
>  pc-bios/descriptors/60-edk2-i386.json  |  33 +++
>  pc-bios/descriptors/60-edk2-x86_64.json|  34 +++
>  pc-bios/edk2-aarch64-code.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-arm-code.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-arm-vars.fd   | Bin 0 -> 67108864 bytes
>  pc-bios/edk2-i386-code.fd  | Bin 0 -> 3653632 bytes
>  pc-bios/edk2-i386-secure-code.fd   | Bin 0 -> 3653632 bytes
>  pc-bios/edk2-i386-vars.fd  | Bin 0 -> 540672 bytes
>  pc-bios/edk2-licenses.txt  | 209 
>  pc-bios/edk2-x86_64-code.fd| Bin 0 -> 3653632 bytes
>  pc-bios/edk2-x86_64-secure-code.fd | Bin 0 -> 3653632 bytes
>  roms/Makefile  |   9 +-
>  roms/Makefile.edk2 | 138 +++
>  roms/edk2  |   2 +-
>  roms/edk2-build.sh |  55 +
>  roms/edk2-funcs.sh | 253 
>  tests/uefi-test-tools/build.sh | 100 +---
>  23 files changed, 897 insertions(+), 95 deletions(-)
>  create mode 100644 pc-bios/descriptors/50-edk2-i386-secure.json
>  create mode 100644 pc-bios/descriptors/50-edk2-x86_64-secure.json
>  create mode 100644 pc-bios/descriptors/60-edk2-aarch64.json
>  create mode 100644 pc-bios/descriptors/60-edk2-arm.json
>  create mode 100644 pc-bios/descriptors/60-edk2-i386.json
>  create mode 100644 pc-bios/descriptors/60-edk2-x86_64.json
>  create mode 100644 pc-bios/edk2-aarch64-code.fd
>  create mode 100644 pc-bios/edk2-arm-code.fd
>  create mode 100644 pc-bios/edk2-arm-vars.fd
>  create mode 100644 pc-bios/edk2-i386-code.fd
>  create mode 100644 pc-bios/edk2-i386-secure-code.fd
>  create mode 100644 pc-bios/edk2-i386-vars.fd
>  create mode 100644 pc-bios/edk2-licenses.txt
>  create mode 100644 pc-bios/edk2-x86_64-code.fd
>  create mode 100644 pc-bios/edk2-x86_64-secure-code.fd
>  create mode 100644 roms/Makefile.edk2
>  create mode 100755 roms/edk2-build.sh
>  create mode 100644 roms/edk2-funcs.sh
> 

Unsure whether my ACK is worth anything on this list, but you have it.

Reviewed-by: Michal Privoznik 

Now that libvirt patches for consuming those JSON descriptions are
almost merged, distros can start picking this up.

Michal

Re: [Qemu-devel] [PULL 19/22] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

[Wei Wang]

On 03/09/2019 02:30 AM, Dr. David Alan Gilbert wrote:

* Peter Maydell (peter.mayd...@linaro.org) wrote:

On Wed, 6 Mar 2019 at 11:55, Dr. David Alan Gilbert (git)
 wrote:

From: Wei Wang 


Wei: Can you look at this


Sure, I'll send a followup patch to fix this.





The new feature enables the virtio-balloon device to receive hints of
guest free pages from the free page vq.

A notifier is registered to the migration precopy notifier chain. The
notifier calls free_page_start after the migration thread syncs the dirty
bitmap, so that the free page optimization starts to clear bits of free
pages from the bitmap. It calls the free_page_stop before the migration
thread syncs the bitmap, which is the end of the current round of ram
save. The free_page_stop is also called to stop the optimization in the
case when there is an error occurred in the process of ram saving.

Note: balloon will report pages which were free at the time of this call.
As the reporting happens asynchronously, dirty bit logging must be
enabled before this free_page_start call is made. Guest reporting must be
disabled before the migration dirty bitmap is synchronized.

Signed-off-by: Wei Wang 
CC: Michael S. Tsirkin 
CC: Dr. David Alan Gilbert 
CC: Juan Quintela 
CC: Peter Xu 
Message-Id: <1544516693-5395-8-git-send-email-wei.w.w...@intel.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Dr. David Alan Gilbert 
   dgilbert: Dropped kernel header update, fixed up CMD_ID_* name change
---

Hi -- Coverity points out a use-after-free here (CID 1399412):


+static bool get_free_page_hints(VirtIOBalloon *dev)
+{
+VirtQueueElement *elem;
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+VirtQueue *vq = dev->free_page_vq;
+
+while (dev->block_iothread) {
+qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
+}
+
+elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+if (!elem) {
+return false;
+}
+
+if (elem->out_num) {
+uint32_t id;
+size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
+ &id, sizeof(id));
+virtqueue_push(vq, elem, size);
+g_free(elem);

Here we free elem...


+
+virtio_tswap32s(vdev, &id);
+if (unlikely(size != sizeof(id))) {
+virtio_error(vdev, "received an incorrect cmd id");
+return false;
+}
+if (id == dev->free_page_report_cmd_id) {
+dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+} else {
+/*
+ * Stop the optimization only when it has started. This
+ * avoids a stale stop sign for the previous command.
+ */
+if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+}
+}
+}
+
+if (elem->in_num) {

...but we can fall through here and try to dereference elem...


+if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
+  elem->in_sg[0].iov_len);
+}
+virtqueue_push(vq, elem, 1);
+g_free(elem);

...and then free it again.

OK, so the question here is:
   Is it allowed to have both in and out in the same queue element; if
it's not then we need to error the device.
   If it is allowed then we need to fix up the out_num case.



I think it is allowed. From virtqueue_pop, an elem could consist of 
several inbufs and outbufs.


Probably we could just delay the free till the end of this 
function..will post the fix patch for review soon.


Best,
Wei

Re: [Qemu-devel] [PATCH v3 5/10] qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX

[Unai Martinez Corral]

2019/3/9 10:46, Laurent Vivier:
> On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> > -QEMU_PATH=/usr/local/bin
> > -
> > +QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
> > +QEMU_SUFFIX="${QEMU_SUFFIX:-}"
> >  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
> >  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> >
> > -QEMU_SUFFIX=""
>
> Same question as for PATCH 3/10: why?

Same context. Moreover, in this case envvars already existed and were
hardcoded. I don't think it does any harm to let a external caller set
the defaults.

> And I think the usage text should be formatted differently to be cleared and 
> looks
> like more the one of qemu-XXX:

Agree. I will update when the remaining patches are reviewed, since 6
out of 10 are affected by this change.

Unai

Re: [Qemu-devel] [PATCH v3 3/10] qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT

[Unai Martinez Corral]

2019/3/9 10:42, Laurent Vivier:
> On 06/03/2019 05:46, Unai Martinez-Corral wrote:
> > Allow to set options '--persistent' and/or '--credential' through
> > environment variables. If not defined, defaults are used ('no').
> > Anyway, command-line arguments have priority over environment variables.
>
> Could you explain why we could need to use the env variable instead of
> the command line parameter?
>
> I don't see any use case for that.

The main use case is to provide defaults when this script is included
in a docker container. There are three actors involved:

- Developers of QEMU providing some defaults in the script.
- Developer of a docker image including the script from upstream but
changing some defaults by setting envvars inside the container.
- User of the container that might want to override the settings
either by setting the envvars or through the command line.

If the entrypoint to the docker image is any script that executes
qemu-binfmt-conf.sh at some point, the user cannot provide options
through the command line. Envvars allow to do so, without requiring
the user to customize the docker image.

Regards,

Unai

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20190306031221.GA53@03612eec87fc -> 
patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
fba284afb5 qemu-binfmt-conf.sh: support QEMU_TARGETS
4457bd13fd qemu-binfmt-conf.sh: update usage()
151616125f qemu-binfmt-conf.sh: refactor usage()
fbdecd5e3d qemu-binfmt-conf.sh: add option --reset 
68a0016a98 qemu-binfmt-conf.sh: generalize  to positional 
443f62c7b4 qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
40a55b7af4 qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
21aa966104 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
7811dcc19c qemu-binfmt-conf.sh: make opts -p and -c boolean
2e880d1b68 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/10 Checking commit 2e880d1b68dc (qemu-binfmt-conf.sh: enforce safe style 
consistency)
WARNING: line over 80 characters
#53: FILE: scripts/qemu-binfmt-conf.sh:299:
+if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; 
then

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 7811dcc19c2e (qemu-binfmt-conf.sh: make opts -p and -c 
boolean)
ERROR: line over 90 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:327:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent 
-- "$@")

total: 1 errors, 0 warnings, 43 lines checked

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

3/10 Checking commit 21aa96610492 (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and 
QEMU_PERSISTENT)
4/10 Checking commit 40a55b7af4ec (qemu-binfmt-conf.sh: remove 'qemu' prefix 
from cli options)
ERROR: line over 90 characters
#46: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 443f62c7b47e (qemu-binfmt-conf.sh: honour QEMU_PATH and/or 
QEMU_SUFFIX)
6/10 Checking commit 68a0016a98c6 (qemu-binfmt-conf.sh: generalize  to 
positional )
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+   Supported formats for CPUS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+   See QEMU target list below. If CPUS is 'ALL' or empty, configure all 
known

ERROR: line over 90 characters
#146: FILE: scripts/qemu-binfmt-conf.sh:367:
+options=$(getopt -o :dsQ:S:e:hcp -l 
debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit fbdecd5e3dc1 (qemu-binfmt-conf.sh: add option --reset 
)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s 
-1 > {}' \;

ERROR: line over 90 characters
#85: FILE: scripts/qemu-binfmt-conf.sh:390:
+options=$(getopt -o r:dsQ:S:e:hcp -l 
reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- 
"$@")

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit 151616125f5b (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit 4457bd13fda7 (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit fba284afb546 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+Supported formats for TARGETS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+See QEMU target list below. If TARGETS is 'ALL' or 

Re: [Qemu-devel] [PATCH v3 5/10] qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX

[Laurent Vivier]

On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> Allow to set 'path' or 'suffix' through environment variables,
> consistently with 'persistent' and 'credential'.
> 
> Signed-off-by: Unai Martinez-Corral 
> ---
>  scripts/qemu-binfmt-conf.sh | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 68aa4c3f78..c113ff131e 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -176,6 +176,7 @@ Usage: qemu-binfmt-conf.sh [--path 
> PATH][--debian][--systemd CPU]
> --help:display this usage
> --path:set path to qemu interpreter ($QEMU_PATH)
> --suffix:  add a suffix to the default interpreter name
> +  ($QEMU_SUFFIX)
> --debian:  don't write into /proc,
>instead generate update-binfmts templates
> --systemd: don't write into /proc,
> @@ -321,13 +322,11 @@ BINFMT_SET=qemu_register_interpreter
>  SYSTEMDDIR="/etc/binfmt.d"
>  DEBIANDIR="/usr/share/binfmts"
> 
> -QEMU_PATH=/usr/local/bin
> -
> +QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
> +QEMU_SUFFIX="${QEMU_SUFFIX:-}"
>  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> 
> -QEMU_SUFFIX=""
> -
>  options=$(getopt -o ds:Q:S:e:hcp -l 
> debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
>  eval set -- "$options"
> 
> --
> 2.20.1
> 

Same question as for PATCH 3/10: why?

And I think the usage text should be formatted differently to be cleared and 
looks 
like more the one of qemu-XXX:

  usage: qemu-m68k [options] program [arguments...]
  Linux CPU emulator (compiled for m68k emulation)

  Options and associated environment variables:

  Argument Env-variable  Description
  -h print this help
  -help  
  -g port  QEMU_GDB  wait gdb connection to 'port'
  -L path  QEMU_LD_PREFIXset the elf interpreter prefix to 
'path'
  -s size  QEMU_STACK_SIZE   set the stack size to 'size' bytes
  ...

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 4/10] qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options

[Laurent Vivier]

On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> This breaks backward compatibility.
> 
> Options 'qemu-path' and 'qemu-suffix' have the 'qemu-' prefix, which is
> not present in other option names ('debian', 'systemd', 'persistent',
> 'credential'...). In order to keep consistency, the prefix is removed.
> 
> Signed-off-by: Unai Martinez-Corral 
> ---
>  scripts/qemu-binfmt-conf.sh | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH v3 3/10] qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT

[Laurent Vivier]

On 06/03/2019 05:46, Unai Martinez-Corral wrote:
> Allow to set options '--persistent' and/or '--credential' through
> environment variables. If not defined, defaults are used ('no').
> Anyway, command-line arguments have priority over environment variables.
> 

Could you explain why we could need to use the env variable instead of
the command line parameter?

I don't see any use case for that.

Thanks,
Laurent

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: Gluster 6 compatibility

[Niels de Vos]

On Fri, Mar 08, 2019 at 02:11:51PM +0100, Kevin Wolf wrote:
> Am 05.03.2019 um 16:46 hat Niels de Vos geschrieben:
> > Gluster 6 is currently available as release candidate. There have been a
> > few changes to libgfapi.so that need to be adapted by consuming projects
> > like QEMU. Fedora Rawhide already contains glusterfs-6.0-RC0, and this
> > prevents rebuilds of QEMU there (https://bugzilla.redhat.com/1684298).
> > 
> > The following two changes should be sufficient to consume Gluster 6 once
> > it is released. These have been tested on CentOS-7 with Gluster 5 and
> > Gluster 6 (minimal manual qemu-img tests only).
> > 
> > This v2 post contains changes suggested by Daniel P. Berrangé and Kevin
> > Wolf. Thanks!
> 
> Thanks, applied to the block branch.

Thanks! Stefano Garzarella gave a suggestion for further cleanup. I was
planning to address that (no #ifdef for function arguments) next week
when I'm back from a trip, Is that something you would also like to see,
or do you prefer the change to stay minimal/small as it is now? I'm
happy to send a followup if you agree that it is cleaner.

Niels

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20190306031221.GA53@03612eec87fc -> 
patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
e8052727ed qemu-binfmt-conf.sh: support QEMU_TARGETS
a5fe269cca qemu-binfmt-conf.sh: update usage()
e65c0db987 qemu-binfmt-conf.sh: refactor usage()
66b0955155 qemu-binfmt-conf.sh: add option --reset 
2200f3015d qemu-binfmt-conf.sh: generalize  to positional 
686c9749cf qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
0c9fb8d9aa qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
df7c3ef8c3 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
e2f88c2bbf qemu-binfmt-conf.sh: make opts -p and -c boolean
08b6a8a8a1 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/10 Checking commit 08b6a8a8a145 (qemu-binfmt-conf.sh: enforce safe style 
consistency)
WARNING: line over 80 characters
#53: FILE: scripts/qemu-binfmt-conf.sh:299:
+if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; 
then

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit e2f88c2bbf66 (qemu-binfmt-conf.sh: make opts -p and -c 
boolean)
ERROR: line over 90 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:327:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent 
-- "$@")

total: 1 errors, 0 warnings, 43 lines checked

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

3/10 Checking commit df7c3ef8c37d (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and 
QEMU_PERSISTENT)
4/10 Checking commit 0c9fb8d9aa9f (qemu-binfmt-conf.sh: remove 'qemu' prefix 
from cli options)
ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 686c9749cff9 (qemu-binfmt-conf.sh: honour QEMU_PATH and/or 
QEMU_SUFFIX)
6/10 Checking commit 2200f3015dd0 (qemu-binfmt-conf.sh: generalize  to 
positional )
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+   Supported formats for CPUS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+   See QEMU target list below. If CPUS is 'ALL' or empty, configure all 
known

ERROR: line over 90 characters
#146: FILE: scripts/qemu-binfmt-conf.sh:367:
+options=$(getopt -o :dsQ:S:e:hcp -l 
debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit 66b095515580 (qemu-binfmt-conf.sh: add option --reset 
)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s 
-1 > {}' \;

ERROR: line over 90 characters
#85: FILE: scripts/qemu-binfmt-conf.sh:390:
+options=$(getopt -o r:dsQ:S:e:hcp -l 
reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- 
"$@")

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit e65c0db9871e (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit a5fe269cca58 (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit e8052727edf4 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+Supported formats for TARGETS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+See QEMU target list below. If TARGETS is 'ALL' or 

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20190306031221.GA53@03612eec87fc -> 
patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
bedda47489 qemu-binfmt-conf.sh: support QEMU_TARGETS
ac6c160734 qemu-binfmt-conf.sh: update usage()
7c6effbb95 qemu-binfmt-conf.sh: refactor usage()
4f23b0b002 qemu-binfmt-conf.sh: add option --reset 
71392f21ea qemu-binfmt-conf.sh: generalize  to positional 
946395451d qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
abd051bb2e qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
700f8fcdf7 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
65a2b3aabf qemu-binfmt-conf.sh: make opts -p and -c boolean
3272769df3 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/10 Checking commit 3272769df356 (qemu-binfmt-conf.sh: enforce safe style 
consistency)
WARNING: line over 80 characters
#53: FILE: scripts/qemu-binfmt-conf.sh:299:
+if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; 
then

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 65a2b3aabf28 (qemu-binfmt-conf.sh: make opts -p and -c 
boolean)
ERROR: line over 90 characters
#50: FILE: scripts/qemu-binfmt-conf.sh:327:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent 
-- "$@")

total: 1 errors, 0 warnings, 43 lines checked

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

3/10 Checking commit 700f8fcdf7cb (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and 
QEMU_PERSISTENT)
4/10 Checking commit abd051bb2edc (qemu-binfmt-conf.sh: remove 'qemu' prefix 
from cli options)
ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l 
debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 946395451d14 (qemu-binfmt-conf.sh: honour QEMU_PATH and/or 
QEMU_SUFFIX)
6/10 Checking commit 71392f21ea89 (qemu-binfmt-conf.sh: generalize  to 
positional )
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+   Supported formats for CPUS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+   See QEMU target list below. If CPUS is 'ALL' or empty, configure all 
known

ERROR: line over 90 characters
#146: FILE: scripts/qemu-binfmt-conf.sh:367:
+options=$(getopt -o :dsQ:S:e:hcp -l 
debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit 4f23b0b00271 (qemu-binfmt-conf.sh: add option --reset 
)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s 
-1 > {}' \;

ERROR: line over 90 characters
#85: FILE: scripts/qemu-binfmt-conf.sh:390:
+options=$(getopt -o r:dsQ:S:e:hcp -l 
reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- 
"$@")

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit 7c6effbb95b5 (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit ac6c160734be (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit bedda4748942 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+Supported formats for TARGETS are: single arch or comma/space separated 
list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+See QEMU target list below. If TARGETS is 'ALL' or 

Re: [Qemu-devel] [PATCH v3 2/10] qemu-binfmt-conf.sh: make opts -p and -c boolean

[Laurent Vivier]

On 06/03/2019 05:42, Unai Martinez-Corral wrote:
> This patch breaks backward compatibility.
> 
> Both '--persistent' and '--credential' default to 'no'. Hence, '-p no'
> or '-c no' are reduntant. Overall, accepting an argument might be
> misleading because options are, indeed, boolean. This patch makes both
> options boolean in getopt, so if any of them is provided the corresponding
> variable is set to true.
> 
> Signed-off-by: Unai Martinez-Corral 
> ---
>  scripts/qemu-binfmt-conf.sh | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH v3 1/10] qemu-binfmt-conf.sh: enforce safe style consistency

[Laurent Vivier]

On 06/03/2019 05:36, Unai Martinez-Corral wrote:
> Spaces are added before '; then', for consistency.
> 
> All the tests are prefixed with 'x', in order to avoid risky comparisons
> (i.e. a user deliberately trying to provoke a syntax error).
> 
> Signed-off-by: Unai Martinez-Corral 
> ---
>  scripts/qemu-binfmt-conf.sh | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH v6 01/14] qapi: qapi for audio backends

[Markus Armbruster]

"Kővágó, Zoltán"  writes:

> This patch adds structures into qapi to replace the existing
> configuration structures used by audio backends currently. This qapi
> will be the base of the -audiodev command line parameter (that replaces
> the old environment variables based config).
>
> This is not a 1:1 translation of the old options, I've tried to make
> them much more consistent (e.g. almost every backend had an option to
> specify buffer size, but the name was different for every backend, and
> some backends required usecs, while some other required frames, samples
> or bytes). Also tried to reduce the number of abbreviations used by the
> config keys.
>
> Some of the more important changes:
> * use `in` and `out` instead of `ADC` and `DAC`, as the former is more
>   user friendly imho
> * moved buffer settings into the global setting area (so it's the same
>   for all backends that support it. Backends that can't change buffer
>   size will simply ignore them). Also using usecs, as it's probably more
>   user friendly than samples or bytes.
> * try-poll is now an alsa backend specific option (as all other backends
>   currently ignore it)
>
> Signed-off-by: Kővágó, Zoltán 
> Reviewed-by: Markus Armbruster 
> ---
>
> Notes:
> Changes from v5:
> 
> * documentation fixes
> * renamed buffer-len to buffer-length and period-len to period-length
> 
> Changes from v4:
> 
> * documentation fixes
> * renamed pa's source/sink to pa-in/pa-out
> * per-direction options changed per Markus Armbruster's comments
> 
> Changes from v2:
> 
> * update copyright, version numbers
> * remove #optional
> * per-direction options are now optional (needed for 
> qobject_object_visitor_new_str)
> * removed unnecessary AudiodevNoOptions
> * changed integers to unsigned
>
>  qapi/audio.json   | 304 ++
>  qapi/qapi-schema.json |   1 +
>  qapi/Makefile.objs|   6 +-
>  3 files changed, 308 insertions(+), 3 deletions(-)
>  create mode 100644 qapi/audio.json
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> new file mode 100644
> index 00..97aee37288
> --- /dev/null
> +++ b/qapi/audio.json
> @@ -0,0 +1,304 @@
> +# -*- mode: python -*-
> +#
> +# Copyright (C) 2015-2019 Zoltán Kővágó 
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# @AudiodevPerDirectionOptions:
> +#
> +# General audio backend options that are used for both playback and
> +# recording.
> +#
> +# @fixed-settings: use fixed settings for host input/output. When off,
> +#  frequency, channels and format must not be
> +#  specified (default true)
> +#
> +# @frequency: frequency to use when using fixed settings
> +# (default 44100)
> +#
> +# @channels: number of channels when using fixed settings (default 2)
> +#
> +# @voices: number of voices to use (default 1)
> +#
> +# @format: sample format to use when using fixed settings
> +#  (default s16)
> +#
> +# @buffer-length: the buffer length in microseconds

The name buffer-length suggests bytes rather than microseconds.  That's
why I suggested @buffer-capacity.  Matter of taste, up to you and Gerd.

> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevPerDirectionOptions',
> +  'data': {
> +'*fixed-settings': 'bool',
> +'*frequency':  'uint32',
> +'*channels':   'uint32',
> +'*voices': 'uint32',
> +'*format': 'AudioFormat',
> +'*buffer-length':  'uint32' } }
[...]

Regardless:
Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v7] pflash: Require backend size to match device, improve errors

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> On 3/8/19 4:40 PM, Kevin Wolf wrote:
>> Am 08.03.2019 um 15:29 hat Markus Armbruster geschrieben:
>>> Kevin Wolf  writes:
>>>
 Am 08.03.2019 um 13:28 hat Markus Armbruster geschrieben:
> Laszlo Ersek  writes:
>> This one has got to be one of the longest bike-shedding sessions! :)
>>
>> I'm fine with this patch, but I could suggest two improvements.
>>
>> (1) When blk_getlength() fails, we could format the negative error code
>> returned by it into the error message.
>
> I can do that.

 By using error_setg_errno(), I assume. Not throwing away error details
 is always good.

>> (2) We could extract the common code to a new function in
>> "hw/block/block.c". (It says "Common code for block device models" on
>> the tin.)
>
> There's so much common code in these two files even before this patch...

 My understanding is that hw/block/block.c contains code that is
 potentially useful to all kinds of block devices, not random code that
 two specific similar devices happen to share.

 If we want to deduplicate some code in the flash devices, without any
 expectation that other devices will use it at some point, I'd rather
 create a new source file hw/block/pflash_common.c or something like
 that.
>>>
>>> Yes.
>>>
>>> The helper I came up with (appended) isn't really specific to flash
>>> devices.  Would it be okay for hw/block/block.c even though only the two
>>> flash devices use it for now?
>> 
>> Hm, it feels more like a helper for devices that can't decide whether
>> they want to be a block device or not. Or that actually don't want to be
>> a block device, but use a BlockBackend anyway. Reading in the whole
>> image isn't something that a normal block device would do.
>> 
>> But yes, it doesn't have flash-specific knowledge, even though I hope
>> that it's functionality that will remain very specific to these two
>> devices.
>> 
>> So it's your call, I don't have a strong opinion either way.
>> 
>>>
>>> bool blk_check_size_and_read_all(BlockBackend *blk, void *buf, hwaddr size,
>>>  Error **errp)
>>> {
>>> int64_t blk_len;
>>> int ret;
>>>
>>> blk_len = blk_getlength(blk);
>>> if (blk_len < 0) {
>>> error_setg_errno(errp, -blk_len,
>>>  "can't get size of block backend '%s'",
>>>  blk_name(blk));
>>> return false;
>>> }
>>> if (blk_len != size) {
>>> error_setg(errp, "device requires %" PRIu64 " bytes, "
>>>"block backend '%s' provides %" PRIu64 " bytes",
>>>size, blk_name(blk), blk_len);
>> 
>> Should size use HWADDR_PRIu?
>> 
>> I'm not sure if printing the BlockBackend name is a good idea because
>> hopefully one day the BlockBackend will be anonymous even for the flash
>> devices.
>> 
>>> return false;
>>> }
>>>
>>> /* TODO for @size > BDRV_REQUEST_MAX_BYTES, we'd need to loop */
>>> assert(size <= BDRV_REQUEST_MAX_BYTES);
>> 
>> I don't think we'd ever want to read in more than 2 GB into a memory
>> buffer. Before we even get close to this point, the devices should be
>> reworked to be more like an actual block device and read only what is
>> actually accessed.
>
> The biggest NOR available in the market is 256 MiB (bigger size is
> barely ChipSelect MMIO-addressable).
>
> Maybe you can use:
>
> #define NOR_FLASH_MAX_BYTES (256 * MiB)
>
> and refuse bigger flashes.

The comment next to the definition of property "width" in pflash_cfi01.c
suggests the device model can emulate a bunch of flash chips wired
together:

/* width here is the overall width of this QEMU device in bytes.
 * The QEMU device may be emulating a number of flash devices
 * wired up in parallel; the width of each individual flash
 * device should be specified via device-width. If the individual
 * devices have a maximum width which is greater than the width
 * they are being used for, this maximum width should be set via
 * max-device-width (which otherwise defaults to device-width).
 * So for instance a 32-bit wide QEMU flash device made from four
 * 16-bit flash devices used in 8-bit wide mode would be configured
 * with width = 4, device-width = 1, max-device-width = 2.
 *
 * If device-width is not specified we default to backwards
 * compatible behaviour which is a bad emulation of two
 * 16 bit devices making up a 32 bit wide QEMU device. This
 * is deprecated for new uses of this device.
 */

> We could also check what is the widest chip-select range addressable
> by all the supported architectures. I don't think it's worth it.
>
>> 
>>> ret = blk_pread(blk, 0, buf, size);
>
> OK, this function is named blk_check_size_and_read_all. Here we
> read_all. Refactoring this device we should be able to read at
> most sizeof(the biggest sector

Re: [Qemu-devel] [PATCH] qmp-shell: fix nested json regression

[Markus Armbruster]

John Snow  writes:

> On 2/5/19 8:49 AM, Marc-André Lureau wrote:
>> Commit fcfab7541 ("qmp-shell: learn to send commands with quoted
>> arguments") introduces the usage of Python 'shlex' to handle quoted
>> arguments, but it accidentally broke generation of nested JSON
>> structs.
>> 
>> shlex drops quotes, which breaks parsing of the nested struct.
>> 
>> cmd='blockdev-create job-id="job0 foo" 
>> options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}'
>> 
>> shlex.split(cmd)
>> ['blockdev-create',
>>  'job-id=job0 foo',
>>  'options={driver:qcow2,size:16384,file:{driver:file,filename:foo.qcow2}}']
>> 
>> Replace with a regexp to split while respecting quoted strings and 
>> preserving quotes:
>> 
>> re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmd)
>> ['blockdev-create',
>>  'job-id="job0 foo"',
>>  
>> 'options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}']
>> 
>> Fixes: fcfab7541 ("qmp-shell: learn to send commands with quoted arguments")
>> Reported-by: Kashyap Chamarthy 
>> Signed-off-by: Marc-André Lureau 
>
> Hi, did this get misplaced? Would be nice to have back for 4.0 release.

Eduardo, Cleber, I think this one's for you.

Re: [Qemu-devel] [PATCH] hw/char/parallel: Make it possible to compile also without CONFIG_PARALLEL

[Thomas Huth]

On 09/03/2019 09.12, Thomas Huth wrote:
> For the downstream distribution of QEMU, we want to compile without
> CONFIG_PARALLEL. Commit 9157eee1b1c076ff3 already moved the function
> parallel_hds_isa_init() (which is still required for linking) into a file
> that is included anyway, but commit bb3d5ea858e7f888563a moved it
> to a separate file which is only compiled again if CONFIG_PARALLEL is
> set. To be able to link QEMU again without CONFIG_PARALLEL, let's
> move this file unconditionally to common-obj-y again. And while we're
> at it, also rename it to parallel-helper.c (since parallel.c is also
> about ISA already), add a proper comment in there with the rationale
> for the separate file, and a check via object_class_by_name() to see
> whether the device class is available in the binary or not.
> 
> Signed-off-by: Thomas Huth 
> ---
>  hw/char/Makefile.objs |  2 +-
>  hw/char/{parallel-isa.c => parallel-helper.c} | 10 +-
>  hw/char/parallel.c|  1 -
>  hw/i386/Kconfig   |  2 --
>  include/hw/char/parallel.h|  2 ++
>  5 files changed, 12 insertions(+), 5 deletions(-)
>  rename hw/char/{parallel-isa.c => parallel-helper.c} (70%)
> 
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index c4947d7..5476803 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
>  common-obj-$(CONFIG_ESCC) += escc.o
>  common-obj-$(CONFIG_NRF51_SOC) += nrf51_uart.o
>  common-obj-$(CONFIG_PARALLEL) += parallel.o
> -common-obj-$(CONFIG_PARALLEL) += parallel-isa.o
> +common-obj-y += parallel-helper.o

Self-NACK.

This has to be "common-obj-$(CONFIG_ISA_BUS) += parallel-helper.o" since
it uses the function isa_create(). ... thus, maybe I should also not
rename the file here... I'll ponder about that in a v2...

 Thomas

[Qemu-devel] [PATCH] hw/char/parallel: Make it possible to compile also without CONFIG_PARALLEL

[Thomas Huth]

For the downstream distribution of QEMU, we want to compile without
CONFIG_PARALLEL. Commit 9157eee1b1c076ff3 already moved the function
parallel_hds_isa_init() (which is still required for linking) into a file
that is included anyway, but commit bb3d5ea858e7f888563a moved it
to a separate file which is only compiled again if CONFIG_PARALLEL is
set. To be able to link QEMU again without CONFIG_PARALLEL, let's
move this file unconditionally to common-obj-y again. And while we're
at it, also rename it to parallel-helper.c (since parallel.c is also
about ISA already), add a proper comment in there with the rationale
for the separate file, and a check via object_class_by_name() to see
whether the device class is available in the binary or not.

Signed-off-by: Thomas Huth 
---
 hw/char/Makefile.objs |  2 +-
 hw/char/{parallel-isa.c => parallel-helper.c} | 10 +-
 hw/char/parallel.c|  1 -
 hw/i386/Kconfig   |  2 --
 include/hw/char/parallel.h|  2 ++
 5 files changed, 12 insertions(+), 5 deletions(-)
 rename hw/char/{parallel-isa.c => parallel-helper.c} (70%)

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index c4947d7..5476803 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -2,7 +2,7 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_NRF51_SOC) += nrf51_uart.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
-common-obj-$(CONFIG_PARALLEL) += parallel-isa.o
+common-obj-y += parallel-helper.o
 common-obj-$(CONFIG_PL011) += pl011.o
 common-obj-$(CONFIG_SERIAL) += serial.o
 common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
diff --git a/hw/char/parallel-isa.c b/hw/char/parallel-helper.c
similarity index 70%
rename from hw/char/parallel-isa.c
rename to hw/char/parallel-helper.c
index 639e179..c3e868b 100644
--- a/hw/char/parallel-isa.c
+++ b/hw/char/parallel-helper.c
@@ -1,11 +1,15 @@
 /*
  * QEMU Parallel PORT (ISA bus helpers)
  *
+ * These functions reside in a separate file since they also might be
+ * required for linking when compiling QEMU without CONFIG_PARALLEL.
+ *
  * Copyright (c) 2003 Fabrice Bellard
  *
  * SPDX-License-Identifier: MIT
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/isa/isa.h"
 #include "hw/char/parallel.h"
@@ -15,7 +19,7 @@ static void parallel_init(ISABus *bus, int index, Chardev 
*chr)
 DeviceState *dev;
 ISADevice *isadev;
 
-isadev = isa_create(bus, "isa-parallel");
+isadev = isa_create(bus, TYPE_ISA_PARALLEL);
 dev = DEVICE(isadev);
 qdev_prop_set_uint32(dev, "index", index);
 qdev_prop_set_chr(dev, "chardev", chr);
@@ -28,6 +32,10 @@ void parallel_hds_isa_init(ISABus *bus, int n)
 
 assert(n <= MAX_PARALLEL_PORTS);
 
+if (!object_class_by_name(TYPE_ISA_PARALLEL)) {
+return;
+}
+
 for (i = 0; i < n; i++) {
 if (parallel_hds[i]) {
 parallel_init(bus, i, parallel_hds[i]);
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index a80da47..a19e7d9 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -85,7 +85,6 @@ typedef struct ParallelState {
 PortioList portio_list;
 } ParallelState;
 
-#define TYPE_ISA_PARALLEL "isa-parallel"
 #define ISA_PARALLEL(obj) \
 OBJECT_CHECK(ISAParallelState, (obj), TYPE_ISA_PARALLEL)
 
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 78fd703..4d8247a 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -25,8 +25,6 @@ config PC
 select I82374
 select I8257
 select MC146818RTC
-# Needed by the board code:
-select PARALLEL
 # For ACPI builder:
 select SERIAL_ISA
 select ACPI_VMGENID
diff --git a/include/hw/char/parallel.h b/include/hw/char/parallel.h
index d6dd62f..e0a7fe1 100644
--- a/include/hw/char/parallel.h
+++ b/include/hw/char/parallel.h
@@ -5,6 +5,8 @@
 #include "hw/isa/isa.h"
 #include "chardev/char.h"
 
+#define TYPE_ISA_PARALLEL "isa-parallel"
+
 void parallel_hds_isa_init(ISABus *bus, int n);
 
 bool parallel_mm_init(MemoryRegion *address_space,
-- 
1.8.3.1

[Qemu-devel] [PULL 19/25] scsi-disk: Fix crash if request is invaild or disk is no medium

[Paolo Bonzini]

From: Zhengui Li 

Qemu will crash with the assertion error that "assert(r->req.aiocb !=
NULL)" in scsi_read_complete if request is invaild or disk is no medium.
The error is below:
qemu-kvm: hw/scsi/scsi_disk.c:299: scsi_read_complete: Assertion
`r->req.aiocb != NULL' failed.

This patch add a funtion scsi_read_complete_noio to fix it.

Signed-off-by: Zhengui Li 
Message-Id: <1551949966-20092-1-git-send-email-lizhen...@huawei.com>
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-disk.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d4e83ae..e7e865a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -296,22 +296,15 @@ static void scsi_dma_complete(void *opaque, int ret)
 aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
 }
 
-static void scsi_read_complete(void * opaque, int ret)
+static void scsi_read_complete_noio(SCSIDiskReq *r, int ret)
 {
-SCSIDiskReq *r = (SCSIDiskReq *)opaque;
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-int n;
+uint32_t n;
 
-assert(r->req.aiocb != NULL);
-r->req.aiocb = NULL;
-aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-if (scsi_disk_req_check_error(r, ret, true)) {
+assert(r->req.aiocb == NULL);
+if (scsi_disk_req_check_error(r, ret, false)) {
 goto done;
 }
 
-block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
-trace_scsi_disk_read_complete(r->req.tag, r->qiov.size);
-
 n = r->qiov.size / 512;
 r->sector += n;
 r->sector_count -= n;
@@ -319,6 +312,24 @@ static void scsi_read_complete(void * opaque, int ret)
 
 done:
 scsi_req_unref(&r->req);
+}
+
+static void scsi_read_complete(void *opaque, int ret)
+{
+SCSIDiskReq *r = (SCSIDiskReq *)opaque;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+assert(r->req.aiocb != NULL);
+r->req.aiocb = NULL;
+
+aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
+if (ret < 0) {
+block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct);
+} else {
+block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
+trace_scsi_disk_read_complete(r->req.tag, r->qiov.size);
+}
+scsi_read_complete_noio(r, ret);
 aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
 }
 
@@ -395,12 +406,12 @@ static void scsi_read_data(SCSIRequest *req)
 scsi_req_ref(&r->req);
 if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
 trace_scsi_disk_read_data_invalid();
-scsi_read_complete(r, -EINVAL);
+scsi_read_complete_noio(r, -EINVAL);
 return;
 }
 
 if (!blk_is_available(req->dev->conf.blk)) {
-scsi_read_complete(r, -ENOMEDIUM);
+scsi_read_complete_noio(r, -ENOMEDIUM);
 return;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] contrib/elf2dmp: add kernel start address checking

[Paolo Bonzini]

On 08/03/19 18:55, Peter Maydell wrote:
> On Wed, 20 Feb 2019 at 11:30, Paolo Bonzini  wrote:
>>
>> On 19/02/19 22:19, Viktor Prutyanov wrote:
>>> Before this patch, if elf2dmp failed to find NT kernel PE magic in
>>> allowed virtual address range, then it assumes NULL as NT kernel
>>> address and cause segfault.
>>>
>>> This patch fix the problem described above by checking NT kernel address
>>> before futher processing.
>>>
>>> Signed-off-by: Viktor Prutyanov 
>>> ---
>>>  contrib/elf2dmp/main.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
>>> index 1a45eaf565..1bfeb89ba7 100644
>>> --- a/contrib/elf2dmp/main.c
>>> +++ b/contrib/elf2dmp/main.c
>>> @@ -524,6 +524,12 @@ int main(int argc, char *argv[])
>>>  }
>>>  }
>>>
>>> +if (!nt_start_addr) {
>>> +eprintf("Failed to find NT kernel image\n");
>>> +err = 1;
>>> +goto out_ps;
>>> +}
>>> +
>>>  printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
>>>  (char *)nt_start_addr);
>>>
>>>
>>
>> Queued, thanks.
> 
> Hi Paolo -- I noticed this fix wasn't in master yet -- is it in
> a tree you're planning on sending a pullreq for soon?

Yep, I just wanted to flush kconfig and qgraph before everybody starts
sending pull requests for soft freeze.

Paolo

[Qemu-devel] [PULL 18/25] configure: Disable W^X on OpenBSD

[Paolo Bonzini]

From: Philippe Mathieu-Daudé 

Since OpenBSD 6.0 [1], W^X is enforced by default [2].
TCG requires WX access. Disable W^X if it is available.
This fixes:

  # lm32-softmmu/qemu-system-lm32
  Could not allocate dynamic translator buffer

  # sysctl kern.wxabort=1
  kern.wxabort: 0 -> 1
  # lm32-softmmu/qemu-system-lm32
  mmap: Not supported
  Abort trap (core dumped)
  # gdb -q lm32-softmmu/qemu-system-lm32 qemu-system-lm32.core
  (gdb) bt
  #0  0x17e3c156c50a in _thread_sys___syscall () at {standard input}:5
  #1  0x17e3c15e5d7a in *_libc_mmap (addr=Variable "addr" is not 
available.) at /usr/src/lib/libc/sys/mmap.c:47
  #2  0x17e17d9abc8b in alloc_code_gen_buffer () at 
/usr/src/qemu/accel/tcg/translate-all.c:1064
  #3  0x17e17d9abd04 in code_gen_alloc (tb_size=0) at 
/usr/src/qemu/accel/tcg/translate-all.c:1112
  #4  0x17e17d9abe81 in tcg_exec_init (tb_size=0) at 
/usr/src/qemu/accel/tcg/translate-all.c:1149
  #5  0x17e17d9897e9 in tcg_init (ms=0x17e45e456800) at 
/usr/src/qemu/accel/tcg/tcg-all.c:66
  #6  0x17e17d9891b8 in accel_init_machine (acc=0x17e3c3f50800, 
ms=0x17e45e456800) at /usr/src/qemu/accel/accel.c:63
  #7  0x17e17d989312 in configure_accelerator (ms=0x17e45e456800, 
progname=0x7f7f07b0 "lm32-softmmu/qemu-system-lm32") at 
/usr/src/qemu/accel/accel.c:111
  #8  0x17e17d9d8616 in main (argc=1, argv=0x7f7f06b8, 
envp=0x7f7f06c8) at vl.c:4325

[1] https://www.openbsd.org/faq/upgrade60.html
[2] https://undeadly.org/cgi?action=article&sid=20160527203200

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190307142822.8531-3-phi...@redhat.com>
Signed-off-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/configure b/configure
index c9a260c..62a2a49 100755
--- a/configure
+++ b/configure
@@ -5903,6 +5903,17 @@ if test "$mingw32" = "yes" ; then
 done
 fi
 
+# Disable OpenBSD W^X if available
+if test "$tcg" = "yes" && test "$targetos" = "OpenBSD"; then
+cat > $TMPC <

[Qemu-devel] [PULL 22/25] lsi: use enum type for s->msg_action

[Paolo Bonzini]

From: Sven Schnelle 

This makes the code easier to read - no functional change.

Signed-off-by: Sven Schnelle 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190305195519.24303-4-sv...@stackframe.org>
---
 hw/scsi/lsi53c895a.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index c17bb4f..d67584e 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -201,6 +201,13 @@ enum {
 LSI_DMA_IN_PROGRESS, /* DMA operation is in progress */
 };
 
+enum {
+LSI_MSG_ACTION_COMMAND = 0,
+LSI_MSG_ACTION_DISCONNECT = 1,
+LSI_MSG_ACTION_DOUT = 2,
+LSI_MSG_ACTION_DIN = 3,
+};
+
 typedef struct {
 /*< private >*/
 PCIDevice parent_obj;
@@ -214,8 +221,6 @@ typedef struct {
 
 int carry; /* ??? Should this be an a visible register somewhere?  */
 int status;
-/* Action to take at the end of a MSG IN phase.
-   0 = COMMAND, 1 = disconnect, 2 = DATA OUT, 3 = DATA IN.  */
 int msg_action;
 int msg_len;
 uint8_t msg[LSI_MAX_MSGIN_LEN];
@@ -323,7 +328,7 @@ static void lsi_soft_reset(LSIState *s)
 trace_lsi_reset();
 s->carry = 0;
 
-s->msg_action = 0;
+s->msg_action = LSI_MSG_ACTION_COMMAND;
 s->msg_len = 0;
 s->waiting = LSI_NOWAIT;
 s->dsa = 0;
@@ -686,7 +691,7 @@ static void lsi_reselect(LSIState *s, lsi_request *p)
 trace_lsi_reselect(id);
 s->scntl1 |= LSI_SCNTL1_CON;
 lsi_set_phase(s, PHASE_MI);
-s->msg_action = p->out ? 2 : 3;
+s->msg_action = p->out ? LSI_MSG_ACTION_DOUT : LSI_MSG_ACTION_DIN;
 s->current->dma_len = p->pending;
 lsi_add_msg_byte(s, 0x80);
 if (s->current->tag & LSI_TAG_VALID) {
@@ -857,7 +862,7 @@ static void lsi_do_command(LSIState *s)
 lsi_add_msg_byte(s, 4); /* DISCONNECT */
 /* wait data */
 lsi_set_phase(s, PHASE_MI);
-s->msg_action = 1;
+s->msg_action = LSI_MSG_ACTION_DISCONNECT;
 lsi_queue_command(s);
 } else {
 /* wait command complete */
@@ -878,7 +883,7 @@ static void lsi_do_status(LSIState *s)
 s->sfbr = status;
 pci_dma_write(PCI_DEVICE(s), s->dnad, &status, 1);
 lsi_set_phase(s, PHASE_MI);
-s->msg_action = 1;
+s->msg_action = LSI_MSG_ACTION_DISCONNECT;
 lsi_add_msg_byte(s, 0); /* COMMAND COMPLETE */
 }
 
@@ -901,16 +906,16 @@ static void lsi_do_msgin(LSIState *s)
 /* ??? Check if ATN (not yet implemented) is asserted and maybe
switch to PHASE_MO.  */
 switch (s->msg_action) {
-case 0:
+case LSI_MSG_ACTION_COMMAND:
 lsi_set_phase(s, PHASE_CMD);
 break;
-case 1:
+case LSI_MSG_ACTION_DISCONNECT:
 lsi_disconnect(s);
 break;
-case 2:
+case LSI_MSG_ACTION_DOUT:
 lsi_set_phase(s, PHASE_DO);
 break;
-case 3:
+case LSI_MSG_ACTION_DIN:
 lsi_set_phase(s, PHASE_DI);
 break;
 default:
@@ -1062,7 +1067,7 @@ bad:
 qemu_log_mask(LOG_UNIMP, "Unimplemented message 0x%02x\n", msg);
 lsi_set_phase(s, PHASE_MI);
 lsi_add_msg_byte(s, 7); /* MESSAGE REJECT */
-s->msg_action = 0;
+s->msg_action = LSI_MSG_ACTION_COMMAND;
 }
 
 #define LSI_BUF_SIZE 4096
-- 
1.8.3.1

[Qemu-devel] [PULL 23/25] lsi: use SCSI phase names instead of numbers in trace

[Paolo Bonzini]

From: Sven Schnelle 

This makes trace logs much easier to read, especially for
people who are not fluent in SCSI.

Signed-off-by: Sven Schnelle 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190305195519.24303-5-sv...@stackframe.org>
---
 hw/scsi/lsi53c895a.c | 31 +++
 hw/scsi/trace-events |  6 +++---
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d67584e..66f217f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -306,6 +306,22 @@ typedef struct {
 #define LSI53C895A(obj) \
 OBJECT_CHECK(LSIState, (obj), TYPE_LSI53C895A)
 
+static const char *scsi_phases[] = {
+"DOUT",
+"DIN",
+"CMD",
+"STATUS",
+"RSVOUT",
+"RSVIN",
+"MSGOUT",
+"MSGIN"
+};
+
+static const char *scsi_phase_name(int phase)
+{
+return scsi_phases[phase & PHASE_MASK];
+}
+
 static inline int lsi_irq_on_rsl(LSIState *s)
 {
 return (s->sien0 & LSI_SIST0_RSL) && (s->scid & LSI_SCID_RRE);
@@ -1201,8 +1217,9 @@ again:
 s->ia = s->dsp - 12;
 }
 if ((s->sstat1 & PHASE_MASK) != ((insn >> 24) & 7)) {
-trace_lsi_execute_script_blockmove_badphase(s->sstat1 & PHASE_MASK,
-(insn >> 24) & 7);
+trace_lsi_execute_script_blockmove_badphase(
+scsi_phase_name(s->sstat1),
+scsi_phase_name(insn >> 24));
 lsi_script_scsi_interrupt(s, LSI_SIST0_MA, 0);
 break;
 }
@@ -1234,8 +1251,8 @@ again:
 lsi_do_msgin(s);
 break;
 default:
-qemu_log_mask(LOG_UNIMP, "lsi_scsi: Unimplemented phase %d\n",
-  s->sstat1 & PHASE_MASK);
+qemu_log_mask(LOG_UNIMP, "lsi_scsi: Unimplemented phase %s\n",
+  scsi_phase_name(s->sstat1));
 }
 s->dfifo = s->dbc & 0xff;
 s->ctest5 = (s->ctest5 & 0xfc) | ((s->dbc >> 8) & 3);
@@ -1463,10 +1480,8 @@ again:
 cond = s->carry != 0;
 }
 if (cond == jmp && (insn & (1 << 17))) {
-trace_lsi_execute_script_tc_compp(
-(s->sstat1 & PHASE_MASK),
-jmp ? '=' : '!',
-((insn >> 24) & 7));
+trace_lsi_execute_script_tc_compp(scsi_phase_name(s->sstat1),
+jmp ? '=' : '!', scsi_phase_name(insn >> 24));
 cond = (s->sstat1 & PHASE_MASK) == ((insn >> 24) & 7);
 }
 if (cond == jmp && (insn & (1 << 18))) {
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 29aaa75..09f3fc3 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -268,7 +268,7 @@ lsi_memcpy(uint32_t dest, uint32_t src, int count) "memcpy 
dest 0x%"PRIx32" src
 lsi_wait_reselect(void) "Wait Reselect"
 lsi_execute_script(uint32_t dsp, uint32_t insn, uint32_t addr) "SCRIPTS 
dsp=0x%"PRIx32" opcode 0x%"PRIx32" arg 0x%"PRIx32
 lsi_execute_script_blockmove_delayed(void) "Delayed select timeout"
-lsi_execute_script_blockmove_badphase(uint8_t phase, uint8_t expected) "Wrong 
phase got %d expected %d"
+lsi_execute_script_blockmove_badphase(const char *phase, const char *expected) 
"Wrong phase got %s expected %s"
 lsi_execute_script_io_alreadyreselected(void) "Already reselected, jumping to 
alternative address"
 lsi_execute_script_io_selected(uint8_t id, const char *atn) "Selected target 
%d%s"
 lsi_execute_script_io_disconnect(void) "Wait Disconnect"
@@ -278,8 +278,8 @@ lsi_execute_script_io_opcode(const char *opcode, int reg, 
const char *opname, ui
 lsi_execute_script_tc_nop(void) "NOP"
 lsi_execute_script_tc_delayedselect_timeout(void) "Delayed select timeout"
 lsi_execute_script_tc_compc(int result) "Compare carry %d"
-lsi_execute_script_tc_compp(uint8_t phase, int op, uint8_t insn_phase) 
"Compare phase %d %c= %d"
-lsi_execute_script_tc_compd(uint32_t sfbr, uint8_t mask, int op, int result) 
"Compare data 0x%"PRIx32" & 0x%x %c= 0x%x"
+lsi_execute_script_tc_compp(const char *phase, char op, const char 
*insn_phase) "Compare phase %s %c= %s"
+lsi_execute_script_tc_compd(uint32_t sfbr, uint8_t mask, char op, int result) 
"Compare data 0x%"PRIx32" & 0x%x %c= 0x%x"
 lsi_execute_script_tc_jump(uint32_t addr) "Jump to 0x%"PRIx32
 lsi_execute_script_tc_call(uint32_t addr) "Call 0x%"PRIx32
 lsi_execute_script_tc_return(uint32_t addr) "Return to 0x%"PRIx32
-- 
1.8.3.1

[Qemu-devel] [PULL 11/25] lsi: check if SIGP bit is already set in Wait reselect

[Paolo Bonzini]

From: Sven Schnelle 

If SIGP is set, the 'Wait for Reselection' command should jump
immediately to the address stored in the second DWORD of the
instruction. This fixes spurious hangs in the HP-UX 11.11
installer when the SIGP bit gets set by the kernel before the
'Wait for Reselection' command is executed by SCRIPTS.

Signed-off-by: Sven Schnelle 
Tested-by: Helge Deller 
Message-Id: <20190217113717.7077-1-sv...@stackframe.org>
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/lsi53c895a.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 8ba07f8..bcff859 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1297,8 +1297,10 @@ again:
 }
 break;
 case 2: /* Wait Reselect */
-if (!lsi_irq_on_rsl(s)) {
-lsi_wait_reselect(s);
+if (s->istat0 & LSI_ISTAT0_SIGP) {
+s->dsp = s->dnad;
+} else if (!lsi_irq_on_rsl(s)) {
+lsi_wait_reselect(s);
 }
 break;
 case 3: /* Set */
-- 
1.8.3.1

[Qemu-devel] [PULL 16/25] accel: Allow to build QEMU without TCG or KVM support

[Paolo Bonzini]

From: Anthony PERARD 

Instead of deny build of QEMU without a default accelerator, simply
report an error when the user haven't passed -accel or -machine accel=
and TCG and KVM isn't builtin.

./configure already check that at least one accelerator is available.

Signed-off-by: Anthony PERARD 
Signed-off-by: Paolo Bonzini 
---
 accel/accel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/accel/accel.c b/accel/accel.c
index 68b6d56..0d5b370 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -91,7 +91,9 @@ void configure_accelerator(MachineState *ms, const char 
*progname)
 #elif defined(CONFIG_KVM)
 accel = "kvm";
 #else
-#error "No default accelerator available"
+error_report("No accelerator selected and"
+ " no default accelerator available");
+exit(1);
 #endif
 }
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 24/25] lsi: return dfifo value

[Paolo Bonzini]

From: Sven Schnelle 

Code was assigning DFIFO, but didn't return the value to users.

Signed-off-by: Sven Schnelle 
Message-Id: <20190305195519.24303-6-sv...@stackframe.org>
---
 hw/scsi/lsi53c895a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 66f217f..bf6b6a5 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1688,7 +1688,7 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
 break;
 CASE_GET_REG32(temp, 0x1c)
 case 0x20: /* DFIFO */
-ret = 0;
+ret = s->dfifo;
 break;
 case 0x21: /* CTEST4 */
 ret = s->ctest4;
-- 
1.8.3.1

[Qemu-devel] [PULL 12/25] update copyright notice

[Paolo Bonzini]

From: David Kiarie 

Signed-off-by: David Kiarie 
Message-Id: <20190304151827.1813-2-davidkiar...@gmail.com>
Signed-off-by: Paolo Bonzini 
---
 hw/i386/amd_iommu.c | 2 +-
 hw/i386/amd_iommu.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 8ad707a..6eabdf9 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -2,7 +2,7 @@
  * QEMU emulation of AMD IOMMU (AMD-Vi)
  *
  * Copyright (C) 2011 Eduard - Gabriel Munteanu
- * Copyright (C) 2015 David Kiarie, 
+ * Copyright (C) 2015, 2016 David Kiarie Kahurani
  *
  * 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
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index c52886f..0ff9095 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -2,7 +2,7 @@
  * QEMU emulation of an AMD IOMMU (AMD-Vi)
  *
  * Copyright (C) 2011 Eduard - Gabriel Munteanu
- * Copyright (C) 2015 David Kiarie, 
+ * Copyright (C) 2015, 2016 David Kiarie Kahurani
  *
  * 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
-- 
1.8.3.1

[Qemu-devel] [PULL 25/25] exec: streamline flatview_add_to_dispatch

[Paolo Bonzini]

The while loop in flatview_add_to_dispatch can only be executed twice
and it will go through different paths on each execution (in fact
one of the "if" branches is dead).  Remove the loop completely,
the code becomes clearer at the cost of a handful of duplicated
lines.

Reported-by: Wei Yang 
Signed-off-by: Paolo Bonzini 
---
 exec.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/exec.c b/exec.c
index 1d4f378..9647593 100644
--- a/exec.c
+++ b/exec.c
@@ -1601,33 +1601,37 @@ static void register_multipage(FlatView *fv,
 
 void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
 {
-MemoryRegionSection now = *section, remain = *section;
+MemoryRegionSection remain = *section;
 Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
 
-if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
+if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
+MemoryRegionSection now = remain;
 uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
- now.offset_within_address_space;
 
 now.size = int128_min(int128_make64(left), now.size);
 register_subpage(fv, &now);
-} else {
-now.size = int128_zero();
-}
-while (int128_ne(remain.size, now.size)) {
+if (int128_eq(remain.size, now.size)) {
+return;
+}
 remain.size = int128_sub(remain.size, now.size);
 remain.offset_within_address_space += int128_get64(now.size);
 remain.offset_within_region += int128_get64(now.size);
-now = remain;
-if (int128_lt(remain.size, page_size)) {
-register_subpage(fv, &now);
-} else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
-now.size = page_size;
-register_subpage(fv, &now);
-} else {
-now.size = int128_and(now.size, int128_neg(page_size));
-register_multipage(fv, &now);
+}
+
+if (int128_ge(remain.size, page_size)) {
+MemoryRegionSection now = remain;
+now.size = int128_and(now.size, int128_neg(page_size));
+register_multipage(fv, &now);
+if (int128_eq(remain.size, now.size)) {
+return;
 }
+remain.size = int128_sub(remain.size, now.size);
+remain.offset_within_address_space += int128_get64(now.size);
+remain.offset_within_region += int128_get64(now.size);
 }
+
+register_subpage(fv, &remain);
 }
 
 void qemu_flush_coalesced_mmio_buffer(void)
-- 
1.8.3.1

[Qemu-devel] [PULL 20/25] lsi: use ldn_le_p()/stn_le_p()

[Paolo Bonzini]

From: Sven Schnelle 

Instead of using the open-coded versions, use the helper already
present as this makes the code easier to read and less error-prone.

Signed-off-by: Sven Schnelle 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190305195519.24303-2-sv...@stackframe.org>
---
 hw/scsi/lsi53c895a.c | 24 
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index bcff859..402b785 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -289,8 +289,7 @@ typedef struct {
 uint8_t sbr;
 uint32_t adder;
 
-/* Script ram is stored as 32-bit words in host byteorder.  */
-uint32_t script_ram[2048];
+uint8_t script_ram[2048 * sizeof(uint32_t)];
 } LSIState;
 
 #define TYPE_LSI53C810  "lsi53c810"
@@ -2079,29 +2078,14 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
   uint64_t val, unsigned size)
 {
 LSIState *s = opaque;
-uint32_t newval;
-uint32_t mask;
-int shift;
-
-newval = s->script_ram[addr >> 2];
-shift = (addr & 3) * 8;
-mask = ((uint64_t)1 << (size * 8)) - 1;
-newval &= ~(mask << shift);
-newval |= val << shift;
-s->script_ram[addr >> 2] = newval;
+stn_le_p(s->script_ram + addr, size, val);
 }
 
 static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
  unsigned size)
 {
 LSIState *s = opaque;
-uint32_t val;
-uint32_t mask;
-
-val = s->script_ram[addr >> 2];
-mask = ((uint64_t)1 << (size * 8)) - 1;
-val >>= (addr & 3) * 8;
-return val & mask;
+return ldn_le_p(s->script_ram + addr, size);
 }
 
 static const MemoryRegionOps lsi_ram_ops = {
@@ -2244,7 +2228,7 @@ static const VMStateDescription vmstate_lsi_scsi = {
 VMSTATE_BUFFER_UNSAFE(scratch, LSIState, 0, 18 * sizeof(uint32_t)),
 VMSTATE_UINT8(sbr, LSIState),
 
-VMSTATE_BUFFER_UNSAFE(script_ram, LSIState, 0, 2048 * 
sizeof(uint32_t)),
+VMSTATE_BUFFER_UNSAFE(script_ram, LSIState, 0, 8192),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.8.3.1