Re: [Qemu-devel] [PATCH v6 02/11] Add vhost-user-input-pci

[Gerd Hoffmann]

> +static void vhost_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
> +{
> +VirtIOInput *vinput = VIRTIO_INPUT(vdev);
> +VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
> +int ret;
> +
> +memset(config_data, 0, vinput->cfg_size);
> +
> +ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, 
> vinput->cfg_size);
> +if (ret) {
> +error_report("vhost-user-input: get device config space failed");
> +return;
> +}
> +}
> +
> +static void vhost_input_set_config(VirtIODevice *vdev,
> +   const uint8_t *config_data)
> +{
> +VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
> +int ret;
> +
> +ret = vhost_dev_set_config(&vhi->vhost->dev, config_data,
> +   0, sizeof(virtio_input_config),
> +   VHOST_SET_CONFIG_TYPE_MASTER);
> +if (ret) {
> +error_report("vhost-user-input: set device config space failed");
> +return;
> +}
> +
> +virtio_notify_config(vdev);
> +}

These two look rather generic, the only virtio-input specific thing is
the config space size.  Can we store the size somewhere, then move the
functions to common vhost-user code?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v6 04/11] contrib: add vhost-user-input

[Gerd Hoffmann]

On Tue, Apr 23, 2019 at 03:19:57PM +0200, Marc-André Lureau wrote:
> Add a vhost-user input backend example, based on virtio-input-host
> device. It takes an evdev path as argument, and can be associated with
> a vhost-user-input device via a UNIX socket:
> 
> $ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock
> 
> $ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock
>   -device vhost-user-input-pci,chardev=vuic
> 
> This example is intentionally not included in $TOOLS, to not be built
> and installed by default.

I think we should build it by default, so it doesn't bitrot,
even if we don't install it by default.

> +static int unix_sock_new(char *path)
> +{

Move to common code?

cheers,
  Gerd

[Qemu-devel] [PATCH 3/3] sev: Change SEV to use EncryptedRAMBlock Notifier

[Natarajan, Janakarajan]

The EncryptedRAMBlock Notifier lets SEV know which guest RAM pages
will contain encrypted guest data.

Using this notifier lets SEV skip pinning pages that do not contain
encrypted data.

Signed-off-by: Janakarajan Natarajan 
---
 target/i386/sev.c | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index cd77f6b5d4..610e992e64 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -127,21 +127,11 @@ sev_set_guest_state(SevState new_state)
 }
 
 static void
-sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t size)
+sev_ram_block_encrypted_added(RAMBlockEncryptedNotifier *n,
+  void *host, size_t size)
 {
 int r;
 struct kvm_enc_region range;
-ram_addr_t offset;
-MemoryRegion *mr;
-
-/*
- * The RAM device presents a memory region that should be treated
- * as IO region and should not be pinned.
- */
-mr = memory_region_from_host(host, &offset);
-if (mr && memory_region_is_ram_device(mr)) {
-return;
-}
 
 range.addr = (__u64)(unsigned long)host;
 range.size = size;
@@ -156,7 +146,8 @@ sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t 
size)
 }
 
 static void
-sev_ram_block_removed(RAMBlockNotifier *n, void *host, size_t size)
+sev_ram_block_encrypted_removed(RAMBlockEncryptedNotifier *n,
+void *host, size_t size)
 {
 int r;
 struct kvm_enc_region range;
@@ -172,9 +163,9 @@ sev_ram_block_removed(RAMBlockNotifier *n, void *host, 
size_t size)
 }
 }
 
-static struct RAMBlockNotifier sev_ram_notifier = {
-.ram_block_added = sev_ram_block_added,
-.ram_block_removed = sev_ram_block_removed,
+static struct RAMBlockEncryptedNotifier sev_ram_encrypted_notifier = {
+.ram_block_encrypted_added = sev_ram_block_encrypted_added,
+.ram_block_encrypted_removed = sev_ram_block_encrypted_removed,
 };
 
 static void
@@ -794,7 +785,7 @@ sev_guest_init(const char *id)
 goto err;
 }
 
-ram_block_notifier_add(&sev_ram_notifier);
+ram_block_encrypted_notifier_add(&sev_ram_encrypted_notifier);
 qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
 qemu_add_vm_change_state_handler(sev_vm_state_change, s);
 
-- 
2.20.1

[Qemu-devel] [PATCH 0/3] Add RAM block encrypted notifier

[Natarajan, Janakarajan]

Currently, the SEV guest launch registers to a RAM block notifier. When
called, we issue KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION ioctl to register
the memory with the KVM driver. These ioctls should be called only for
the region which contains the encrypted data but the RAM block notifier
gets called for any memory region allocated during the guest creation.
Some of those memory regions do not contain encrypted data so we end up
calling the ioctl for a memory region which contains unencrypted data
(e.g. vga RAM etc.).

In case of SEV, only the guest RAM and pflash unit=0 contain the
encrypted data. To solve this problem, we introduce a new notifier (RAM
block encrypted). If a memory region will contain encrypted data then
the caller can use memory_region_mark_encrypted() to set the memory
region as encrypted. Clients can register to the RAM block encrypted
notifier and they will be called when a memory region is set encrypted.

Janakarajan Natarajan (3):
  ram-encrypted-notifier: Introduce a RAM block encrypted notifier
  hw: Notify listeners about guest pages which contain encrypted data
  sev: Change SEV to use EncryptedRAMBlock Notifier

 exec.c |  6 ++
 hw/i386/pc.c   |  1 +
 hw/i386/pc_sysfw.c |  2 ++
 hw/mem/memory-device.c |  1 +
 include/exec/memory.h  | 18 ++
 include/exec/ramlist.h | 19 +++
 memory.c   | 16 
 numa.c | 33 +
 stubs/ram-block.c  |  8 
 target/i386/sev.c  | 25 -
 10 files changed, 112 insertions(+), 17 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/3] ram-encrypted-notifier: Introduce a RAM block encrypted notifier

[Natarajan, Janakarajan]

A client can register to this notifier to know whether the newly added or
removed memory region is marked as encrypted. This information is needed
for the SEV guest launch. In SEV guest, some memory regions may contain
encrypted data (e.g guest RAM). The memory region which contains the
encrypted data should be registered/unregistered using the
KVM_MEMORY_{REG,UNREG}_ENCRYPTED ioctl.

Signed-off-by: Janakarajan Natarajan 
---
 exec.c |  1 +
 include/exec/memory.h  | 18 ++
 include/exec/ramlist.h | 19 +++
 memory.c   | 16 
 numa.c | 33 +
 stubs/ram-block.c  |  8 
 6 files changed, 95 insertions(+)

diff --git a/exec.c b/exec.c
index 2646207661..a02c394e48 100644
--- a/exec.c
+++ b/exec.c
@@ -79,6 +79,7 @@
  * are protected by the ramlist lock.
  */
 RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
+RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
 
 static MemoryRegion *system_memory;
 static MemoryRegion *system_io;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9144a47f57..ae720ff511 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -374,6 +374,7 @@ struct MemoryRegion {
 bool terminates;
 bool ram_device;
 bool enabled;
+bool encrypted;
 bool warning_printed; /* For reservations */
 uint8_t vga_logging_count;
 MemoryRegion *alias;
@@ -1131,6 +1132,23 @@ int memory_region_iommu_attrs_to_index(IOMMUMemoryRegion 
*iommu_mr,
  */
 int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
 
+/**
+ * memory_region_mark_encrypted: marks the memory region as encrypted and
+ * lets the listeners of encrypted ram know that a memory region containing
+ * encrypted ram block has been added
+ *
+ * @mr: the memory region
+ */
+void memory_region_mark_encrypted(MemoryRegion *mr);
+
+/**
+ * memory_region_is_encrypted: returns if the memory region was marked as
+ * encrypted when it was created
+ *
+ * @mr: the memory region
+ */
+bool memory_region_is_encrypted(MemoryRegion *mr);
+
 /**
  * memory_region_name: get a memory region's name
  *
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index bc4faa1b00..5349f27fa5 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -7,6 +7,7 @@
 #include "qemu/rcu_queue.h"
 
 typedef struct RAMBlockNotifier RAMBlockNotifier;
+typedef struct RAMBlockEncryptedNotifier RAMBlockEncryptedNotifier;
 
 #define DIRTY_MEMORY_VGA   0
 #define DIRTY_MEMORY_CODE  1
@@ -55,6 +56,11 @@ typedef struct RAMList {
 } RAMList;
 extern RAMList ram_list;
 
+typedef struct RAMBlockEncryptedNotifierList {
+QLIST_HEAD(, RAMBlockEncryptedNotifier) ram_block_notifiers;
+} RAMBlockEncryptedNotifierList;
+extern RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
+
 /* Should be holding either ram_list.mutex, or the RCU lock. */
 #define  INTERNAL_RAMBLOCK_FOREACH(block)  \
 QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
@@ -70,6 +76,14 @@ struct RAMBlockNotifier {
 QLIST_ENTRY(RAMBlockNotifier) next;
 };
 
+struct RAMBlockEncryptedNotifier {
+void (*ram_block_encrypted_added)(RAMBlockEncryptedNotifier *n,
+  void *host, size_t size);
+void (*ram_block_encrypted_removed)(RAMBlockEncryptedNotifier *n,
+void *host, size_t size);
+QLIST_ENTRY(RAMBlockEncryptedNotifier) next;
+};
+
 void ram_block_notifier_add(RAMBlockNotifier *n);
 void ram_block_notifier_remove(RAMBlockNotifier *n);
 void ram_block_notify_add(void *host, size_t size);
@@ -77,4 +91,9 @@ void ram_block_notify_remove(void *host, size_t size);
 
 void ram_block_dump(Monitor *mon);
 
+void ram_block_encrypted_notifier_add(RAMBlockEncryptedNotifier *n);
+void ram_block_encrypted_notifier_remove(RAMBlockEncryptedNotifier *n);
+void ram_block_encrypted_notify_add(void *host, size_t size);
+void ram_block_encrypted_notify_remove(void *host, size_t size);
+
 #endif /* RAMLIST_H */
diff --git a/memory.c b/memory.c
index bb2b71ee38..eca02d369b 100644
--- a/memory.c
+++ b/memory.c
@@ -2009,6 +2009,22 @@ int memory_region_iommu_num_indexes(IOMMUMemoryRegion 
*iommu_mr)
 return imrc->num_indexes(iommu_mr);
 }
 
+void memory_region_mark_encrypted(MemoryRegion *mr)
+{
+RAMBlock *block = mr->ram_block;
+
+mr->encrypted = kvm_memcrypt_enabled();
+
+if (mr->encrypted) {
+ram_block_encrypted_notify_add(block->host, block->max_length);
+}
+}
+
+bool memory_region_is_encrypted(MemoryRegion *mr)
+{
+return mr->encrypted;
+}
+
 void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 {
 uint8_t mask = 1 << client;
diff --git a/numa.c b/numa.c
index 3875e1efda..08601366c5 100644
--- a/numa.c
+++ b/numa.c
@@ -638,6 +638,39 @@ MemdevList *qmp_query_memdev(Error **errp)
 return list;
 }
 
+void ram_block_encrypted_notifier_add(

[Qemu-devel] [PATCH 2/3] hw: Notify listeners about guest pages which contain encrypted data

[Natarajan, Janakarajan]

PC ram, pflash unit 0 rom and pc-dimm memory hotplug ram blocks need to be
encrypted.

Also, notify listeners when freeing a MemoryRegion if it has encrypted
data.

Signed-off-by: Janakarajan Natarajan 
---
 exec.c | 5 +
 hw/i386/pc.c   | 1 +
 hw/i386/pc_sysfw.c | 2 ++
 hw/mem/memory-device.c | 1 +
 4 files changed, 9 insertions(+)

diff --git a/exec.c b/exec.c
index a02c394e48..25be8f84f3 100644
--- a/exec.c
+++ b/exec.c
@@ -2442,6 +2442,11 @@ void qemu_ram_free(RAMBlock *block)
 }
 
 if (block->host) {
+/* Notify only if encrypted */
+if (memory_region_is_encrypted(block->mr)) {
+ram_block_encrypted_notify_remove(block->host, block->max_length);
+}
+
 ram_block_notify_remove(block->host, block->max_length);
 }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f2c15bf1f2..3af3094543 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1740,6 +1740,7 @@ void pc_memory_init(PCMachineState *pcms,
 ram = g_malloc(sizeof(*ram));
 memory_region_allocate_system_memory(ram, NULL, "pc.ram",
  machine->ram_size);
+memory_region_mark_encrypted(ram);
 *ram_memory = ram;
 ram_below_4g = g_malloc(sizeof(*ram_below_4g));
 memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index c628540774..40d7da5ff6 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -199,6 +199,8 @@ static void pc_system_flash_map(PCMachineState *pcms,
 
 /* Encrypt the pflash boot ROM */
 if (kvm_memcrypt_enabled()) {
+/* Mark pflash unit 0 as encrypted. This will pin the pages */
+memory_region_mark_encrypted(flash_mem);
 flash_ptr = memory_region_get_ram_ptr(flash_mem);
 flash_size = memory_region_size(flash_mem);
 ret = kvm_memcrypt_encrypt_data(flash_ptr, flash_size);
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index 5f2c408036..b2e4d4 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -295,6 +295,7 @@ void memory_device_plug(MemoryDeviceState *md, MachineState 
*ms)
 
 memory_region_add_subregion(&ms->device_memory->mr,
 addr - ms->device_memory->base, mr);
+memory_region_mark_encrypted(mr);
 trace_memory_device_plug(DEVICE(md)->id ? DEVICE(md)->id : "", addr);
 }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v6 05/11] vhost-user: add vhost_user_gpu_set_socket()

[Gerd Hoffmann]

On Tue, Apr 23, 2019 at 03:19:58PM +0200, Marc-André Lureau wrote:
> Add a new vhost-user message to give a unix socket to a vhost-user
> backend for GPU display updates.

Can you split input/gpu into two patch series?

> +Wire format
> +===
> +
> +Unless specified differently, numbers are in the machine native byte
> +order.
> +
> +A vhost-user-gpu request consists of 2 header fields and a payload.
> +
> ++-+--+-+
> +| request | size | payload |
> ++-+--+-+
> +
> +Header
> +--
> +
> +:request: ``u32``, type of the request
> +
> +:size: ``u32``, size of the payload
> +
> +A reply consists only of a payload, whose content depends on the request.

I'd suggest to use the same format for replies, only with "request"
meaning "status" in replies.  Allows for OK/ERROR status in replies,
and having a size field in replies too should make things more robust.
Also allows for an empty reply (status=ok,size=0).

cheers,
  Gerd

[Qemu-devel] [Bug 1826422] Re: Regression: QEMU 4.0 hangs the host (*bisect included*)

[Saverio Miroddi]

ok, so, if I understand correctly, the workaround is:

- set `x-no-kvm-intx=on` and enable MSI in the guest (via regedit or
module params)

which may lead to a performance regression (at least under certain
circumstances).

Is it therefore preferrable, performance and configuration-wise, to use
QEMU 3.1.0, if there are no 4.0.0 feature requirements, until this issue
is sorted out?

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

Title:
  Regression: QEMU 4.0 hangs the host (*bisect included*)

Status in QEMU:
  New

Bug description:
  The commit b2fc91db84470a78f8e93f5b5f913c17188792c8 seemingly
  introduced a regression on my system.

  When I start QEMU, the guest and the host hang (I need a hard reset to
  get back to a working system), before anything shows on the guest.

  I use QEMU with GPU passthrough (which worked perfectly until the
  commit above). This is the command I use:

  ```
  /path/to/qemu-system-x86_64
-drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
-drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
-enable-kvm
-machine q35,accel=kvm,mem-merge=off
-cpu 
host,kvm=off,hv_vendor_id=vgaptrocks,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time
-smp 4,cores=4,sockets=1,threads=1
-m 10240
-vga none
-rtc base=localtime
-serial none
-parallel none
-usb
-device usb-tablet
-device vfio-pci,host=01:00.0,multifunction=on
-device vfio-pci,host=01:00.1
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device virtio-scsi-pci,id=scsi
-drive file=/path/to/guest.img,id=hdd1,format=qcow2,if=none,cache=writeback
-device scsi-hd,drive=hdd1
-net nic,model=virtio
-net user,smb=/path/to/shared
  ```

  If I run QEMU without GPU passthrough, it runs fine.

  Some details about my system:

  - O/S: Mint 19.1 x86-64 (it's based on Ubuntu 18.04)
  - Kernel: 4.15
  - `configure` options: `--target-list=x86_64-softmmu --enable-gtk 
--enable-spice --audio-drv-list=pa`
  - EDK2 version: 1a734ed85fda71630c795832e6d24ea560caf739 (20/Apr/2019)
  - CPU: i7-6700k
  - Motherboard: ASRock Z170 Gaming-ITX/ac
  - VGA: Gigabyte GTX 960 Mini-ITX

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

Re: [Qemu-devel] [PATCH] hvf: Add missing break statement

[Paolo Bonzini]

On 22/04/19 18:09, Philippe Mathieu-Daudé wrote:
> Oops... I'm surprised no compiler warned about this yet...
> 
> This probably mean:
> - This code is not covered by Continuous Integration
> - Upstream maintainers are not building this code
> - Upstream is not running this code
> 
> Please tell me I'm wrong!

Indeed I am not building or running this code since it requires a Mac.
I am thinking of adding a Patchew builder on macincloud, but I haven't
done it yet.

However, it is unlikely to be signaled by a compiler---maybe by
Coverity.  Probably the return value doesn't mean much to the rest of
the code, the "cpu->halted = 1" is what counts.  I have queued the patch.

Paolo

Re: [Qemu-devel] [PATCH v3 0/3] char-socket: Fix race condition

[Paolo Bonzini]

On 23/04/19 18:55, Daniel P. Berrangé wrote:
> ping - paolo/marc-andré - unless I'm missing something, it looks like
> this chardev series slipped through the cracks and missed 4.0

Yeah, it had a bug unfortunately.  I'm looking at it RSN.

Paolo

> 
> On Fri, Feb 22, 2019 at 03:46:23PM +0200, Alberto Garcia wrote:
>> This fixes a race condition in which the tcp_chr_read() ioc handler
>> can close a connection that is being written to from another thread.
>>
>> Note: vhost-user-test still fails if QTEST_VHOST_USER_FIXME is set.
>>
>> Berto
>>
>> RFC: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01510.html
>>
>> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01834.html
>> - Fixes memory leaks and adds a qemu_idle_add() function
>>
>> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06137.html
>> - Rebased on top of the current master (fc3dbb90f2eb069801bfb4cfe9cbc)
>> - Patches 1 and 2: Remove the changes in char-pty.c, they're not
>>needed after the rebase.
>> - Patch 3: Fix conflicts after the rebase.
>>
>> v3:
>> - Patch 3: Add tcp_chr_disconnect_locked() [Daniel]
>>
>> Alberto Garcia (3):
>>   main-loop: Fix GSource leak in qio_task_thread_worker()
>>   main-loop: Add qemu_idle_add()
>>   char-socket: Lock tcp_chr_disconnect() and socket_reconnect_timeout()
>>
>>  chardev/char-socket.c| 25 +
>>  include/qemu/main-loop.h | 12 
>>  io/task.c|  9 +++--
>>  util/main-loop.c |  9 +
>>  4 files changed, 45 insertions(+), 10 deletions(-)
>>
>> -- 
>> 2.11.0
>>
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] [PATCH v6 08/11] contrib: add vhost-user-gpu

[Gerd Hoffmann]

  Hi,

> +#ifdef CONFIG_LIBDRM_INTEL
> +static bool
> +intel_alloc_bo(struct drm_buffer *buf)
> +{
> +uint32_t tiling = I915_TILING_NONE;
> +
> +buf->intel_bo = drm_intel_bo_alloc_tiled(buf->dev->bufmgr, 
> "vhost-user-gpu",
> + buf->width, buf->height,
> + (buf->bpp / 8), &tiling,
> + &buf->stride, 0);

Why do you go for intel specific code here?

Can't you do the same with gbm_bo_create() or
gbm_bo_create_with_modifiers() ?

> +static bool
> +intel_map_bo(struct drm_buffer *buf)
> +{
> +if (drm_intel_gem_bo_map_gtt(buf->intel_bo) != 0) {
> +return false;
> +}

gbm_bo_map()

> +static bool
> +intel_export_bo_to_prime(struct drm_buffer *buffer, int *fd)
> +{
> +if (drm_intel_bo_gem_export_to_prime(buffer->intel_bo, fd) < 0) {
> +return false;
> +}

gbm_bo_get_fd()

> +get_pixman_format(uint32_t virtio_gpu_format)
> +{
> +switch (virtio_gpu_format) {
> +#ifdef HOST_WORDS_BIGENDIAN
> +case VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM:
> +return PIXMAN_b8g8r8x8;

Move that to a header for sharing?  Simliar to the bswap helpers?

> +static void
> +virgl_cmd_create_resource_3d(VuGpu *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> +struct virtio_gpu_resource_create_3d c3d;
> +struct virgl_renderer_resource_create_args args;
> +
> +VUGPU_FILL_CMD(c3d);
> +
> +args.handle = c3d.resource_id;
> +args.target = c3d.target;
> +args.format = c3d.format;
> +args.bind = c3d.bind;
> +args.width = c3d.width;
> +args.height = c3d.height;
> +args.depth = c3d.depth;
> +args.array_size = c3d.array_size;
> +args.last_level = c3d.last_level;
> +args.nr_samples = c3d.nr_samples;
> +args.flags = c3d.flags;
> +virgl_renderer_resource_create(&args, NULL, 0);
> +}

The virtio_gpu_resource_create_3d -> virgl_renderer_resource_create_args
mapping looks like an opportunity for code sharing too.

Hmm, but virgl_renderer_resource_create seems to be the only call with
an args struct, so maybe not worth the effort for this single case.

cheers,
  Gerd

[Qemu-devel] About pt-br keyboard layout

[Lin Ma]

Hi all,

While I launch qemu with vnc + pt-br keyboard layout on my pc, If I type 
shift + 6 in iPXE shell or grub shell via my usual 105-key keyboard,
shift + 6 would be mapped to "(apostrophe), But IIUC the correct 
character should be ¨(diaeresis) in pt-br layout.


I'm wondering that is it the expected behavior? In other words, Does it 
make sense if I want to type multi-character characters(say Ç or ¨) 
without guest os's help?




About pt-br keyboard (ABNT2), please refer to:
http://www.cjdinfo.com.br/utilitario-tabela-caracteres
https://docs.microsoft.com/en-us/globalization/keyboards/kbdbr_2.html


Any feedback would be appreciated,
Lin

Re: [Qemu-devel] [RHEL-8.1 virt 0/2] Enable SEV VM to boot with assigned PCI device

[Paolo Bonzini]

On 10/04/19 02:08, Gary R Hook wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1667249
> 
> On an AMD SEV enabled host with an SEV enabled guest, attaching an
> assigned device to the VM results in a failure to start the VM:
> 
> qemu-kvm: -device vfio-pci,host=01:00.0,id=hostdev0,bus=pci.2,addr=0x0: 
> sev_ram_block_added: failed to register region (0x7fd96e6bb000+0x2) error 
> 'Cannot allocate memory'
> 
> In this example the assigned device is a simple Intel 82574L NIC:
> 
> 01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network 
> Connection
>   Subsystem: Intel Corporation Gigabit CT Desktop Adapter
>   Flags: bus master, fast devsel, latency 0, IRQ 89, NUMA node 0
>   Memory at fb9c (32-bit, non-prefetchable) [size=128K]
>   Memory at fb90 (32-bit, non-prefetchable) [size=512K]
> 
> Note that the error indicates the region as (base+size) where a size
> of 0x2 is 128K, which matches that of BAR0 for the device.
> dmesg on the host also reports:
> 
> SVM: SEV: Failure locking 32 pages.
> 
> SEV guests make use of the RAMBlock notifier in QEMU to add page
> pinnings for SEV; the kernel side of the call only knows how to pin
> pages with get_user_pages(), and this currently faults on non-page
> backed mappings (e.g. the mmap of an MMIO BAR).
> 
> To resolve this failure, change the order of the memory region type
> assignment and avoid pinning device memory regions.
> 
> Cc: "Danilo C. L. de Paula" 
> Cc: Eduardo Habkost 
> Cc: Paolo Bonzini 
> Cc: qemu-devel@nongnu.org
> Cc: Richard Henderson 
> 
> Danilo C. L. de Paula (2):
>   redhat: branching qemu-kvm to rhel-8.1.0
>   redhat: renaming branch to rhel-8.1.0
> 
> Gary R Hook (2):
>   Subject: memory: Fix the memory region type assignment order
>   Subject: target/i386: sev: Do not pin the ram device memory region
> 
>  .gitpublish   |  6 +++---
>  memory.c  |  9 -
>  target/i386/sev.c | 11 +++
>  3 files changed, 22 insertions(+), 4 deletions(-)
> 

Acked-by: Paolo Bonzini 

Re: [Qemu-devel] Following up questions related to QEMU and I/O Thread

[Paolo Bonzini]

On 23/04/19 14:04, Stefan Hajnoczi wrote:
>> In addition, does Virtio-scsi support Batch I/O Submission feature
>> which may be able to increase the IOPS via reducing the number of
>> system calls?
>
> I don't see obvious batching support in drivers/scsi/virtio_scsi.c.
> The Linux block layer supports batching but I'm not sure if the SCSI
> layer does.

I think he's referring to QEMU, in which case yes, virtio-scsi does
batch I/O submission.  See virtio_scsi_handle_cmd_req_prepare and
virtio_scsi_handle_cmd_req_submit in hw/scsi/virtio-scsi.c, they do
blk_io_plug and blk_io_unplug in order to batch I/O requests from QEMU
to the host kernel.

Paolo

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Paolo Bonzini]

On 25/04/19 22:00, Eduardo Habkost wrote:
> diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> new file mode 100644
> index 00..c710f23388
> --- /dev/null
> +++ b/hw/core/qdev-hotplug-stubs.c
> @@ -0,0 +1,44 @@
> +/*
> + * qdev hotplug handler stubs (for user-mode emulation and unit tests)

Can you explain the issue with unit tests in the commit message?

> + *  Copyright (c) 2019 Red Hat Inc
> + *
> + * Authors:
> + *  Eduardo Habkost 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev-core.h"
> +#include "hw/hotplug.h"
> +
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +{
> +return NULL;
> +}
> +
> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> +  DeviceState *plugged_dev,
> +  Error **errp)
> +{
> +assert(plug_handler == NULL);
> +}
> +
> +void hotplug_handler_plug(HotplugHandler *plug_handler,
> +  DeviceState *plugged_dev,
> +  Error **errp)
> +{
> +assert(plug_handler == NULL);
> +}

Would it work if you instead make these functions (and the others in
hw/core/hotplug.c) inlines in include/hw/hotplug.h?

Then all that remains is qdev_get_hotplug_handler.

Paolo

Re: [Qemu-devel] About pt-br keyboard layout

[Gerd Hoffmann]

On Fri, Apr 26, 2019 at 04:09:12PM +0800, Lin Ma wrote:
> Hi all,
> 
> While I launch qemu with vnc + pt-br keyboard layout on my pc, If I type
> shift + 6 in iPXE shell or grub shell via my usual 105-key keyboard,
> shift + 6 would be mapped to "(apostrophe), But IIUC the correct character
> should be ¨(diaeresis) in pt-br layout.
> 
> I'm wondering that is it the expected behavior? In other words, Does it make
> sense if I want to type multi-character characters(say Ç or ¨) without guest
> os's help?

No.  -k is for translating keysyms back into the correct scancodes,
because this is what the (virtual) keyboard passes to the guest.
So -k must match the *hosts* keyboard layout.

Translating the scancodes into keysyms again is the job of the guest,
so that requires the keymapping you want to use being active in the
guest os.  Typically that would be the same you have on the host, but
that isn't required.  The guest can work with -- for example -- us
layout (like it is often the case in boot loaders).  Behavior should be
identical to physical hardware here.

HTH,
  Gerd

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Like Xu]

On 2019/4/26 4:00, Eduardo Habkost wrote:

This series moves some qdev code outside qdev.o, so it can be
compiled only in CONFIG_SOFTMMU.

The code being moved includes two qdev_get_machine() calls, so
this will make it easier to move qdev_get_machine() to
CONFIG_SOFTMMU later.

After this series, there's one remaining qdev_get_machine() call
that seems more difficult to remove:

 static void device_set_realized(Object *obj, bool value, Error **errp)
 {
 /* [...] */
 if (!obj->parent) {
 gchar *name = g_strdup_printf("device[%d]", unattached_count++);

 object_property_add_child(container_get(qdev_get_machine(),
 "/unattached"),
   name, obj, &error_abort);
 unattached_parent = true;
 g_free(name);
 }
 /* [...] */
 }



I may have an experimental patch to fix device_set_realized issue:

1. in qdev_get_machine():
replace
dev = container_get(object_get_root(), "/machine");
with
dev = object_resolve_path("/machine", NULL);

2. in device_set_realized():

Using
Object *container = qdev_get_machine() ?
qdev_get_machine() : object_get_root();
and pass it to
object_property_add_child(
container_get(container, "/unattached"),
name, obj, &error_abort);

With this fix, we could say the qdev_get_machine() does
return the "/machine" object (or null) not a confused "/container".

We could continue to use qdev_get_machine() in system emulation mode,
getting rid of its surprising side effect as Markus said.

The return value of qdev_get_machine() in user-only mode
is the same object returned by object_get_root(),
so no semantic changes.



This one is tricky because on system emulation mode it needs
"/machine" to already exist, but in user-only mode it needs to
implicitly create a "/machine" container.

Eduardo Habkost (4):
   machine: Move gpio code to hw/core/gpio.c
   move qdev hotplug code to qdev-hotplug.c
   qdev: Don't compile hotplug code in user-mode emulation
   qdev-hotplug: Don't check type of qdev_get_machine()

  hw/core/bus.c|  11 --
  hw/core/gpio.c   | 206 
  hw/core/qdev-hotplug-stubs.c |  44 +++
  hw/core/qdev-hotplug.c   |  64 ++
  hw/core/qdev.c   | 219 ---
  hw/core/Makefile.objs|   5 +-
  tests/Makefile.include   |   3 +-
  7 files changed, 320 insertions(+), 232 deletions(-)
  create mode 100644 hw/core/gpio.c
  create mode 100644 hw/core/qdev-hotplug-stubs.c
  create mode 100644 hw/core/qdev-hotplug.c

[Qemu-devel] [PATCH 1/3] migration/colo.c: Remove redundant input parameter

[Zhang Chen]

From: Zhang Chen 

The colo_do_failover no need the input parameter.

Signed-off-by: Zhang Chen 
---
 include/migration/colo.h  | 2 +-
 migration/colo-failover.c | 2 +-
 migration/colo.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index 99ce17aca7..ddebe0ad27 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -37,7 +37,7 @@ bool migration_incoming_in_colo_state(void);
 COLOMode get_colo_mode(void);
 
 /* failover */
-void colo_do_failover(MigrationState *s);
+void colo_do_failover(void);
 
 void colo_checkpoint_notify(void *opaque);
 #endif
diff --git a/migration/colo-failover.c b/migration/colo-failover.c
index 4854a96c92..e9ca0b4774 100644
--- a/migration/colo-failover.c
+++ b/migration/colo-failover.c
@@ -39,7 +39,7 @@ static void colo_failover_bh(void *opaque)
 return;
 }
 
-colo_do_failover(NULL);
+colo_do_failover();
 }
 
 void failover_request_active(Error **errp)
diff --git a/migration/colo.c b/migration/colo.c
index 238a6d62c7..8c1644091f 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -193,7 +193,7 @@ COLOMode get_colo_mode(void)
 }
 }
 
-void colo_do_failover(MigrationState *s)
+void colo_do_failover(void)
 {
 /* Make sure VM stopped while failover happened. */
 if (!colo_runstate_is_stopped()) {
-- 
2.17.GIT

[Qemu-devel] [PATCH 0/3] Optimize COLO related codes and description

[Zhang Chen]

From: Zhang Chen 

In this series we optimize codes and fix some tiny issues.

Zhang Chen (3):
  migration/colo.c: Remove redundant input parameter
  migration/colo.h: Remove obsolete codes
  qemu-option.hx: Update missed parameter for colo-compare

 include/migration/colo.h  | 4 +---
 migration/colo-failover.c | 2 +-
 migration/colo.c  | 2 +-
 qemu-options.hx   | 9 ++---
 4 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.17.GIT

[Qemu-devel] [PATCH 3/3] qemu-option.hx: Update missed parameter for colo-compare

[Zhang Chen]

From: Zhang Chen 

We missed the iothread related args in this file.
This patch is used to fix this issue.

Signed-off-by: Zhang Chen 
---
 qemu-options.hx | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 08749a3391..a4500c99ef 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4420,13 +4420,15 @@ Dump the network traffic on netdev @var{dev} to the 
file specified by
 The file format is libpcap, so it can be analyzed with tools such as tcpdump
 or Wireshark.
 
-@item -object 
colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid}[,vnet_hdr_support]
+@item -object 
colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid},iothread=@var{id}[,vnet_hdr_support]
 
 Colo-compare gets packet from primary_in@var{chardevid} and 
secondary_in@var{chardevid}, than compare primary packet with
 secondary packet. If the packets are same, we will output primary
 packet to outdev@var{chardevid}, else we will notify colo-frame
 do checkpoint and send primary packet to outdev@var{chardevid}.
-if it has the vnet_hdr_support flag, colo compare will send/recv packet with 
vnet_hdr_len.
+In order to improve efficiency, we need to put the task of comparison
+in another thread. If it has the vnet_hdr_support flag, colo compare
+will send/recv packet with vnet_hdr_len.
 
 we must use it with the help of filter-mirror and filter-redirector.
 
@@ -4441,10 +4443,11 @@ primary:
 -chardev socket,id=compare0-0,host=3.3.3.3,port=9001
 -chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
 -chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-object iothread,id=iothread1
 -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
 -object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out
 -object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0
--object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0
+-object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 secondary:
 -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
-- 
2.17.GIT

[Qemu-devel] [PATCH 2/3] migration/colo.h: Remove obsolete codes

[Zhang Chen]

From: Zhang Chen 

Signed-off-by: Zhang Chen 
---
 include/migration/colo.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index ddebe0ad27..f6fbe23ec9 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -22,8 +22,6 @@ enum colo_event {
 COLO_EVENT_FAILOVER,
 };
 
-void colo_info_init(void);
-
 void migrate_start_colo_process(MigrationState *s);
 bool migration_in_colo_state(void);
 
-- 
2.17.GIT

Re: [Qemu-devel] [PATCH] Revert "armv7m: Guard against no -kernel argument"

[Stefan Hajnoczi]

On Thu, Apr 25, 2019 at 08:07:06PM +0200, Philippe Mathieu-Daudé wrote:
> On 1/4/19 4:16 PM, Peter Maydell wrote:
> > On Thu, 3 Jan 2019 at 14:41, Stefan Hajnoczi  wrote:
> >>
> >> This reverts commit 01fd41ab3fb69971c24a69ed49cde96086d81278.
> >>
> >> The generic loader device (-device loader,file=kernel.bin) can be used
> >> to load a kernel instead of the -kernel option.  Some boards have flash
> >> memory (pflash) that is set via the -pflash or -drive options.
> >>
> >> Allow starting QEMU without the -kernel option to accommodate these
> >> scenarios.
> >>
> >> Suggested-by: Peter Maydell 
> >> Signed-off-by: Stefan Hajnoczi 
> 
> Previous to this commit (v3.1), we have:
> 
> $ qemu-system-aarch64 -M netduino2
> qemu-system-aarch64: Guest image must be specified (using -kernel)
> 
> Now (v4.0) we get:
> 
> $ qemu-system-aarch64 -M netduino2
> qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)

A user-friendly error message is needed here.  The check for -kernel was
too specific and is not desirable for microbit where we use -device
loader.

Old boards probably want to continue using -kernel.  New boards like
microbit may use just -device loader.  Perhaps there is even a group
that wants both options.

A solution is to introduce explicit checks so that we can tell the user
the appropriate option for the machine type.  I can work on this if you
like, but probably won't be able to send a patch until Tuesday.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] migration/colo.c: Remove redundant input parameter

[Zhang, Chen]

This patch have been integrated in this series:
http://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg04520.html
Please review it at this link.

Thanks
Zhang Chen


> -Original Message-
> From: Zhang, Chen
> Sent: Friday, April 26, 2019 11:40 AM
> To: Laurent Vivier ; Dr. David Alan Gilbert
> ; Juan Quintela ; zhanghailiang
> ; qemu-dev 
> Cc: Zhang Chen ; Zhang, Chen 
> Subject: [PATCH] migration/colo.c: Remove redundant input parameter
> 
> From: Zhang Chen 
> 
> The colo_do_failover no need the input parameter.
> 
> Signed-off-by: Zhang Chen 
> ---
>  include/migration/colo.h  | 2 +-
>  migration/colo-failover.c | 2 +-
>  migration/colo.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/migration/colo.h b/include/migration/colo.h index
> 99ce17aca7..ddebe0ad27 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -37,7 +37,7 @@ bool migration_incoming_in_colo_state(void);
>  COLOMode get_colo_mode(void);
> 
>  /* failover */
> -void colo_do_failover(MigrationState *s);
> +void colo_do_failover(void);
> 
>  void colo_checkpoint_notify(void *opaque);  #endif diff --git 
> a/migration/colo-
> failover.c b/migration/colo-failover.c index 4854a96c92..e9ca0b4774 100644
> --- a/migration/colo-failover.c
> +++ b/migration/colo-failover.c
> @@ -39,7 +39,7 @@ static void colo_failover_bh(void *opaque)
>  return;
>  }
> 
> -colo_do_failover(NULL);
> +colo_do_failover();
>  }
> 
>  void failover_request_active(Error **errp) diff --git a/migration/colo.c
> b/migration/colo.c index 238a6d62c7..8c1644091f 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -193,7 +193,7 @@ COLOMode get_colo_mode(void)
>  }
>  }
> 
> -void colo_do_failover(MigrationState *s)
> +void colo_do_failover(void)
>  {
>  /* Make sure VM stopped while failover happened. */
>  if (!colo_runstate_is_stopped()) {
> --
> 2.17.GIT

Re: [Qemu-devel] [PATCH] migration/colo.h: Remove obsolete codes

[Zhang, Chen]

This patch have been integrated in this series:
http://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg04520.html
Please review it at this link.

Thanks
Zhang Chen

> -Original Message-
> From: Zhang, Chen
> Sent: Friday, April 26, 2019 11:40 AM
> To: Laurent Vivier ; Dr. David Alan Gilbert
> ; Juan Quintela ; zhanghailiang
> ; qemu-dev 
> Cc: Zhang Chen ; Zhang, Chen 
> Subject: [PATCH] migration/colo.h: Remove obsolete codes
> 
> From: Zhang Chen 
> 
> Signed-off-by: Zhang Chen 
> ---
>  include/migration/colo.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/migration/colo.h b/include/migration/colo.h index
> ddebe0ad27..f6fbe23ec9 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -22,8 +22,6 @@ enum colo_event {
>  COLO_EVENT_FAILOVER,
>  };
> 
> -void colo_info_init(void);
> -
>  void migrate_start_colo_process(MigrationState *s);  bool
> migration_in_colo_state(void);
> 
> --
> 2.17.GIT

Re: [Qemu-devel] [PATCH v6 02/11] Add vhost-user-input-pci

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 9:12 AM Gerd Hoffmann  wrote:
>
> > +static void vhost_input_get_config(VirtIODevice *vdev, uint8_t 
> > *config_data)
> > +{
> > +VirtIOInput *vinput = VIRTIO_INPUT(vdev);
> > +VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
> > +int ret;
> > +
> > +memset(config_data, 0, vinput->cfg_size);
> > +
> > +ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, 
> > vinput->cfg_size);
> > +if (ret) {
> > +error_report("vhost-user-input: get device config space failed");
> > +return;
> > +}
> > +}
> > +
> > +static void vhost_input_set_config(VirtIODevice *vdev,
> > +   const uint8_t *config_data)
> > +{
> > +VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
> > +int ret;
> > +
> > +ret = vhost_dev_set_config(&vhi->vhost->dev, config_data,
> > +   0, sizeof(virtio_input_config),
> > +   VHOST_SET_CONFIG_TYPE_MASTER);
> > +if (ret) {
> > +error_report("vhost-user-input: set device config space failed");
> > +return;
> > +}
> > +
> > +virtio_notify_config(vdev);
> > +}
>
> These two look rather generic, the only virtio-input specific thing is
> the config space size.  Can we store the size somewhere, then move the
> functions to common vhost-user code?

vhost-user-input is simple, vhost-user-gpu is fiddling with the config
on qemu side.

virtio-input set_config() calls virtio_notify_config(), while
virtio-gpu does not (vhost-user versions copy that).

There isn't much code to share at this point.


-- 
Marc-André Lureau

[Qemu-devel] [PATCH] MAINTAINERS: Add qemu-options* to related field

[Zhang Chen]

From: Zhang Chen 

No one to maintain qemu-options related file, add it to Markus's
Command line option argument parsing field.

Signed-off-by: Zhang Chen 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 23db6f8408..acc3b32f88 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1800,6 +1800,7 @@ F: tests/test-keyval.c
 F: tests/test-qemu-opts.c
 F: util/keyval.c
 F: util/qemu-option.c
+F: qemu-options*
 
 Coverity model
 M: Markus Armbruster 
-- 
2.17.GIT

[Qemu-devel] Failing qemu-iotest 005 with raw

[Thomas Huth]

When running iotest 005 with raw, the test currently fails for me:

005 - output mismatch (see 005.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/005.out   2019-04-23
16:43:11.0 +0200
+++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/005.out.bad   2019-04-26
11:34:11.0 +0200
@@ -1,13 +1,12 @@
 QA output created by 005

 creating large image
+qemu-img: TEST_DIR/t.IMGFMT: The image size is too large for file
format 'IMGFMT'
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912

 small read
-read 4096/4096 bytes at offset 1024
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Input/output error

 small write
-wrote 4096/4096 bytes at offset 8192
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+write failed: Input/output error
 *** done


Could this be fixed somehow, or should the test rather be skipped for
IMGFMT=raw?

 Thomas

Re: [Qemu-devel] [PATCH v6 04/11] contrib: add vhost-user-input

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 9:16 AM Gerd Hoffmann  wrote:
>
> On Tue, Apr 23, 2019 at 03:19:57PM +0200, Marc-André Lureau wrote:
> > Add a vhost-user input backend example, based on virtio-input-host
> > device. It takes an evdev path as argument, and can be associated with
> > a vhost-user-input device via a UNIX socket:
> >
> > $ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock
> >
> > $ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock
> >   -device vhost-user-input-pci,chardev=vuic
> >
> > This example is intentionally not included in $TOOLS, to not be built
> > and installed by default.
>
> I think we should build it by default, so it doesn't bitrot,
> even if we don't install it by default.
>
> > +static int unix_sock_new(char *path)
> > +{
>
> Move to common code?
>

Better, use existing unix_listen()!

thanks



-- 
Marc-André Lureau

[Qemu-devel] [PATCH] trace: fix runstate tracing

[Yury Kotov]

Signed-off-by: Yury Kotov 
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index ff5dfb6fbc..ad9b181e57 100644
--- a/vl.c
+++ b/vl.c
@@ -725,7 +725,7 @@ void runstate_set(RunState new_state)
 assert(new_state < RUN_STATE__MAX);
 
 trace_runstate_set(current_run_state, RunState_str(current_run_state),
-   new_state, RunState_str(current_run_state));
+   new_state, RunState_str(new_state));
 
 if (current_run_state == new_state) {
 return;
-- 
2.21.0

[Qemu-devel] [PATCH 0/6] Add vhost-user-input

[Marc-André Lureau]

Hi,

This is the vhost-user-input part of "[PATCH v6 00/11] vhost-user for input & 
GPU".

v1: (changes since original v6 series)
- add "libvhost-user: fix -Waddress-of-packed-member" & "util: simplify 
unix_listen()"
- use unix_listen()
- build vhost-user-input by default (when applicable)

Marc-André Lureau (6):
  libvhost-user: fix -Waddress-of-packed-member
  libvhost-user: add PROTOCOL_F_CONFIG if {set,get}_config
  Add vhost-user-backend
  Add vhost-user-input-pci
  util: simplify unix_listen()
  contrib: add vhost-user-input

 include/hw/virtio/virtio-input.h   |  14 +
 include/sysemu/vhost-user-backend.h|  57 
 backends/vhost-user.c  | 209 +
 contrib/libvhost-user/libvhost-user.c  |  10 +-
 contrib/vhost-user-input/main.c| 393 +
 hw/input/vhost-user-input.c| 129 
 hw/virtio/vhost-user-input-pci.c   |  53 
 util/qemu-sockets.c|  18 +-
 MAINTAINERS|   4 +
 Makefile   |   9 +
 Makefile.objs  |   1 +
 backends/Makefile.objs |   2 +
 contrib/vhost-user-input/Makefile.objs |   1 +
 hw/input/Kconfig   |   5 +
 hw/input/Makefile.objs |   1 +
 hw/virtio/Makefile.objs|   1 +
 16 files changed, 888 insertions(+), 19 deletions(-)
 create mode 100644 include/sysemu/vhost-user-backend.h
 create mode 100644 backends/vhost-user.c
 create mode 100644 contrib/vhost-user-input/main.c
 create mode 100644 hw/input/vhost-user-input.c
 create mode 100644 hw/virtio/vhost-user-input-pci.c
 create mode 100644 contrib/vhost-user-input/Makefile.objs

-- 
2.21.0.313.ge35b8cb8e2

[Qemu-devel] [PATCH 2/6] libvhost-user: add PROTOCOL_F_CONFIG if {set, get}_config

[Marc-André Lureau]

Add the config protocol feature bit if the set_config & get_config
callbacks are implemented.

Signed-off-by: Marc-André Lureau 
---
 contrib/libvhost-user/libvhost-user.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index dcf4a969f2..74d42177c5 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -1157,6 +1157,10 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg 
*vmsg)
 features |= 1ULL << VHOST_USER_PROTOCOL_F_PAGEFAULT;
 }
 
+if (dev->iface->get_config && dev->iface->set_config) {
+features |= 1ULL << VHOST_USER_PROTOCOL_F_CONFIG;
+}
+
 if (dev->iface->get_protocol_features) {
 features |= dev->iface->get_protocol_features(dev);
 }
-- 
2.21.0.313.ge35b8cb8e2

[Qemu-devel] [PATCH 1/6] libvhost-user: fix -Waddress-of-packed-member

[Marc-André Lureau]

/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c: In function 
‘vu_set_mem_table_exec_postcopy’:
/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c:546:31: warning: 
taking address of packed member of ‘struct VhostUserMsg’ may result in an 
unaligned pointer value [-Waddress-of-packed-member]
  546 | VhostUserMemory *memory = &vmsg->payload.memory;
  |   ^
/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c: In function 
‘vu_set_mem_table_exec’:
/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c:688:31: warning: 
taking address of packed member of ‘struct VhostUserMsg’ may result in an 
unaligned pointer value [-Waddress-of-packed-member]
  688 | VhostUserMemory *memory = &vmsg->payload.memory;
  |   ^
/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c: In function 
‘vu_set_vring_addr_exec’:
/home/elmarco/src/qemu/contrib/libvhost-user/libvhost-user.c:817:36: warning: 
taking address of packed member of ‘struct VhostUserMsg’ may result in an 
unaligned pointer value [-Waddress-of-packed-member]
  817 | struct vhost_vring_addr *vra = &vmsg->payload.addr;
  |^~~

Signed-off-by: Marc-André Lureau 
---
 contrib/libvhost-user/libvhost-user.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index e08d6c7b97..dcf4a969f2 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -542,7 +542,7 @@ static bool
 vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
 {
 int i;
-VhostUserMemory *memory = &vmsg->payload.memory;
+VhostUserMemory m = vmsg->payload.memory, *memory = &m;
 dev->nregions = memory->nregions;
 
 DPRINT("Nregions: %d\n", memory->nregions);
@@ -684,7 +684,7 @@ static bool
 vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
 int i;
-VhostUserMemory *memory = &vmsg->payload.memory;
+VhostUserMemory m = vmsg->payload.memory, *memory = &m;
 
 for (i = 0; i < dev->nregions; i++) {
 VuDevRegion *r = &dev->regions[i];
@@ -813,7 +813,7 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
-struct vhost_vring_addr *vra = &vmsg->payload.addr;
+struct vhost_vring_addr addr = vmsg->payload.addr, *vra = &addr;
 unsigned int index = vra->index;
 VuVirtq *vq = &dev->vq[index];
 
-- 
2.21.0.313.ge35b8cb8e2

[Qemu-devel] [PATCH 5/6] util: simplify unix_listen()

[Marc-André Lureau]

The only caller of unix_listen() left is qga/channel-posix.c.

There is no need to deal with legacy coma-trailing options ",...".

Signed-off-by: Marc-André Lureau 
---
 util/qemu-sockets.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9705051690..d1664e83d6 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -966,26 +966,12 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, 
Error **errp)
 /* compatibility wrapper */
 int unix_listen(const char *str, Error **errp)
 {
-char *path, *optstr;
-int sock, len;
 UnixSocketAddress *saddr;
+int sock;
 
 saddr = g_new0(UnixSocketAddress, 1);
-
-optstr = strchr(str, ',');
-if (optstr) {
-len = optstr - str;
-if (len) {
-path = g_malloc(len+1);
-snprintf(path, len+1, "%.*s", len, str);
-saddr->path = path;
-}
-} else {
-saddr->path = g_strdup(str);
-}
-
+saddr->path = g_strdup(str);
 sock = unix_listen_saddr(saddr, errp);
-
 qapi_free_UnixSocketAddress(saddr);
 return sock;
 }
-- 
2.21.0.313.ge35b8cb8e2

[Qemu-devel] [PATCH 6/6] contrib: add vhost-user-input

[Marc-André Lureau]

Add a vhost-user input backend example, based on virtio-input-host
device. It takes an evdev path as argument, and can be associated with
a vhost-user-input device via a UNIX socket:

$ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock

$ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock
  -device vhost-user-input-pci,chardev=vuic

This example is intentionally not included in $TOOLS, and not
installed by default.

Signed-off-by: Marc-André Lureau 
---
 contrib/vhost-user-input/main.c| 393 +
 MAINTAINERS|   1 +
 Makefile   |   9 +
 Makefile.objs  |   1 +
 contrib/vhost-user-input/Makefile.objs |   1 +
 5 files changed, 405 insertions(+)
 create mode 100644 contrib/vhost-user-input/main.c
 create mode 100644 contrib/vhost-user-input/Makefile.objs

diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
new file mode 100644
index 00..8d493f598e
--- /dev/null
+++ b/contrib/vhost-user-input/main.c
@@ -0,0 +1,393 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include 
+#include 
+
+#include "qemu/iov.h"
+#include "qemu/bswap.h"
+#include "qemu/sockets.h"
+#include "contrib/libvhost-user/libvhost-user.h"
+#include "contrib/libvhost-user/libvhost-user-glib.h"
+#include "standard-headers/linux/virtio_input.h"
+#include "qapi/error.h"
+
+typedef struct virtio_input_event virtio_input_event;
+typedef struct virtio_input_config virtio_input_config;
+
+typedef struct VuInput {
+VugDev dev;
+GSource *evsrc;
+int evdevfd;
+GArray *config;
+virtio_input_config *sel_config;
+struct {
+virtio_input_event event;
+VuVirtqElement *elem;
+} *queue;
+uint32_t qindex, qsize;
+} VuInput;
+
+static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
+{
+VuDev *dev = &vi->dev.parent;
+VuVirtq *vq = vu_get_queue(dev, 0);
+VuVirtqElement *elem;
+int i, len;
+
+/* queue up events ... */
+if (vi->qindex == vi->qsize) {
+vi->qsize++;
+vi->queue = g_realloc_n(vi->queue, vi->qsize, sizeof(vi->queue[0]));
+}
+vi->queue[vi->qindex++].event = *event;
+
+/* ... until we see a report sync ... */
+if (event->type != htole16(EV_SYN) ||
+event->code != htole16(SYN_REPORT)) {
+return;
+}
+
+/* ... then check available space ... */
+for (i = 0; i < vi->qindex; i++) {
+elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
+if (!elem) {
+while (--i >= 0) {
+vu_queue_unpop(dev, vq, vi->queue[i].elem, 0);
+}
+vi->qindex = 0;
+g_warning("virtio-input queue full");
+return;
+}
+vi->queue[i].elem = elem;
+}
+
+/* ... and finally pass them to the guest */
+for (i = 0; i < vi->qindex; i++) {
+elem = vi->queue[i].elem;
+len = iov_from_buf(elem->in_sg, elem->in_num,
+   0, &vi->queue[i].event, sizeof(virtio_input_event));
+vu_queue_push(dev, vq, elem, len);
+g_free(elem);
+}
+
+vu_queue_notify(&vi->dev.parent, vq);
+vi->qindex = 0;
+}
+
+static void
+vi_evdev_watch(VuDev *dev, int condition, void *data)
+{
+VuInput *vi = data;
+int fd = vi->evdevfd;
+
+g_debug("Got evdev condition %x", condition);
+
+struct virtio_input_event virtio;
+struct input_event evdev;
+int rc;
+
+for (;;) {
+rc = read(fd, &evdev, sizeof(evdev));
+if (rc != sizeof(evdev)) {
+break;
+}
+
+g_debug("input %d %d %d", evdev.type, evdev.code, evdev.value);
+
+virtio.type  = htole16(evdev.type);
+virtio.code  = htole16(evdev.code);
+virtio.value = htole32(evdev.value);
+vi_input_send(vi, &virtio);
+}
+}
+
+
+static void vi_handle_status(VuInput *vi, virtio_input_event *event)
+{
+struct input_event evdev;
+int rc;
+
+if (gettimeofday(&evdev.time, NULL)) {
+perror("vi_handle_status: gettimeofday");
+return;
+}
+
+evdev.type = le16toh(event->type);
+evdev.code = le16toh(event->code);
+evdev.value = le32toh(event->value);
+
+rc = write(vi->evdevfd, &evdev, sizeof(evdev));
+if (rc == -1) {
+perror("vi_host_handle_status: write");
+}
+}
+
+static void vi_handle_sts(VuDev *dev, int qidx)
+{
+VuInput *vi = container_of(dev, VuInput, dev.parent);
+VuVirtq *vq = vu_get_queue(dev, qidx);
+virtio_input_event event;
+VuVirtqElement *elem;
+int len;
+
+g_debug("%s", G_STRFUNC);
+
+for (;;) {
+elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
+if (!elem) {
+break;
+}
+
+memset(&event, 0, sizeof(event));
+len = iov_to_buf(

[Qemu-devel] [PATCH 3/6] Add vhost-user-backend

[Marc-André Lureau]

Create a vhost-user-backend object that holds a connection to a
vhost-user backend (or "slave" process) and can be referenced from
virtio devices that support it. See later patches for input & gpu
usage.

Note: a previous iteration of this object made it user-creatable, and
allowed managed sub-process spawning, but that has been dropped for
now.

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/vhost-user-backend.h |  57 
 backends/vhost-user.c   | 209 
 MAINTAINERS |   2 +
 backends/Makefile.objs  |   2 +
 4 files changed, 270 insertions(+)
 create mode 100644 include/sysemu/vhost-user-backend.h
 create mode 100644 backends/vhost-user.c

diff --git a/include/sysemu/vhost-user-backend.h 
b/include/sysemu/vhost-user-backend.h
new file mode 100644
index 00..9abf8f06a1
--- /dev/null
+++ b/include/sysemu/vhost-user-backend.h
@@ -0,0 +1,57 @@
+/*
+ * QEMU vhost-user backend
+ *
+ * Copyright (C) 2018 Red Hat Inc
+ *
+ * Authors:
+ *  Marc-André Lureau 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_VHOST_USER_BACKEND_H
+#define QEMU_VHOST_USER_BACKEND_H
+
+#include "qom/object.h"
+#include "exec/memory.h"
+#include "qemu/option.h"
+#include "qemu/bitmap.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+#include "chardev/char-fe.h"
+#include "io/channel.h"
+
+#define TYPE_VHOST_USER_BACKEND "vhost-user-backend"
+#define VHOST_USER_BACKEND(obj) \
+OBJECT_CHECK(VhostUserBackend, (obj), TYPE_VHOST_USER_BACKEND)
+#define VHOST_USER_BACKEND_GET_CLASS(obj) \
+OBJECT_GET_CLASS(VhostUserBackendClass, (obj), TYPE_VHOST_USER_BACKEND)
+#define VHOST_USER_BACKEND_CLASS(klass) \
+OBJECT_CLASS_CHECK(VhostUserBackendClass, (klass), TYPE_VHOST_USER_BACKEND)
+
+typedef struct VhostUserBackend VhostUserBackend;
+typedef struct VhostUserBackendClass VhostUserBackendClass;
+
+struct VhostUserBackendClass {
+ObjectClass parent_class;
+};
+
+struct VhostUserBackend {
+/* private */
+Object parent;
+
+char *chr_name;
+CharBackend chr;
+VhostUserState vhost_user;
+struct vhost_dev dev;
+VirtIODevice *vdev;
+bool started;
+bool completed;
+};
+
+int vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
+unsigned nvqs, Error **errp);
+void vhost_user_backend_start(VhostUserBackend *b);
+void vhost_user_backend_stop(VhostUserBackend *b);
+
+#endif
diff --git a/backends/vhost-user.c b/backends/vhost-user.c
new file mode 100644
index 00..2b055544a7
--- /dev/null
+++ b/backends/vhost-user.c
@@ -0,0 +1,209 @@
+/*
+ * QEMU vhost-user backend
+ *
+ * Copyright (C) 2018 Red Hat Inc
+ *
+ * Authors:
+ *  Marc-André Lureau 
+ *
+ * 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 "hw/qdev.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "qom/object_interfaces.h"
+#include "sysemu/vhost-user-backend.h"
+#include "sysemu/kvm.h"
+#include "io/channel-command.h"
+#include "hw/virtio/virtio-bus.h"
+
+static bool
+ioeventfd_enabled(void)
+{
+return kvm_enabled() && kvm_eventfds_enabled();
+}
+
+int
+vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
+unsigned nvqs, Error **errp)
+{
+int ret;
+
+assert(!b->vdev && vdev);
+
+if (!ioeventfd_enabled()) {
+error_setg(errp, "vhost initialization failed: requires kvm");
+return -1;
+}
+
+if (!vhost_user_init(&b->vhost_user, &b->chr, errp)) {
+return -1;
+}
+
+b->vdev = vdev;
+b->dev.nvqs = nvqs;
+b->dev.vqs = g_new(struct vhost_virtqueue, nvqs);
+
+ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "vhost initialization failed");
+return -1;
+}
+
+return 0;
+}
+
+void
+vhost_user_backend_start(VhostUserBackend *b)
+{
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(b->vdev)));
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+int ret, i ;
+
+if (b->started) {
+return;
+}
+
+if (!k->set_guest_notifiers) {
+error_report("binding does not support guest notifiers");
+return;
+}
+
+ret = vhost_dev_enable_notifiers(&b->dev, b->vdev);
+if (ret < 0) {
+return;
+}
+
+ret = k->set_guest_notifiers(qbus->parent, b->dev.nvqs, true);
+if (ret < 0) {
+error_report("Error binding guest notifier");
+goto err_host_notifiers;
+}
+
+b->dev.acked_features = b->vdev->guest_features;
+ret = vhost_dev_start(&b->dev, b->vdev);
+if (ret < 0) {
+error_report("Error start vhost dev");
+goto err_guest_notifiers;

[Qemu-devel] [PATCH 4/6] Add vhost-user-input-pci

[Marc-André Lureau]

Add a new virtio-input device, which connects to a vhost-user
backend.

Instead of reading configuration directly from an input device /
evdev (like virtio-input-host), it reads it over vhost-user protocol
with {SET,GET}_CONFIG messages. The vhost-user-backend handles the
queues & events setup.

Signed-off-by: Marc-André Lureau 
---
 include/hw/virtio/virtio-input.h |  14 
 hw/input/vhost-user-input.c  | 129 +++
 hw/virtio/vhost-user-input-pci.c |  53 +
 MAINTAINERS  |   1 +
 hw/input/Kconfig |   5 ++
 hw/input/Makefile.objs   |   1 +
 hw/virtio/Makefile.objs  |   1 +
 7 files changed, 204 insertions(+)
 create mode 100644 hw/input/vhost-user-input.c
 create mode 100644 hw/virtio/vhost-user-input-pci.c

diff --git a/include/hw/virtio/virtio-input.h b/include/hw/virtio/virtio-input.h
index 054c38836f..4fca03e796 100644
--- a/include/hw/virtio/virtio-input.h
+++ b/include/hw/virtio/virtio-input.h
@@ -2,6 +2,7 @@
 #define QEMU_VIRTIO_INPUT_H
 
 #include "ui/input.h"
+#include "sysemu/vhost-user-backend.h"
 
 /* - */
 /* virtio input protocol */
@@ -42,11 +43,18 @@ typedef struct virtio_input_event virtio_input_event;
 #define VIRTIO_INPUT_HOST_GET_PARENT_CLASS(obj) \
 OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_INPUT_HOST)
 
+#define TYPE_VHOST_USER_INPUT   "vhost-user-input"
+#define VHOST_USER_INPUT(obj)  \
+OBJECT_CHECK(VHostUserInput, (obj), TYPE_VHOST_USER_INPUT)
+#define VHOST_USER_INPUT_GET_PARENT_CLASS(obj) \
+OBJECT_GET_PARENT_CLASS(obj, TYPE_VHOST_USER_INPUT)
+
 typedef struct VirtIOInput VirtIOInput;
 typedef struct VirtIOInputClass VirtIOInputClass;
 typedef struct VirtIOInputConfig VirtIOInputConfig;
 typedef struct VirtIOInputHID VirtIOInputHID;
 typedef struct VirtIOInputHost VirtIOInputHost;
+typedef struct VHostUserInput VHostUserInput;
 
 struct VirtIOInputConfig {
 virtio_input_config   config;
@@ -98,6 +106,12 @@ struct VirtIOInputHost {
 int   fd;
 };
 
+struct VHostUserInput {
+VirtIOInput   parent_obj;
+
+VhostUserBackend  *vhost;
+};
+
 void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event);
 void virtio_input_init_config(VirtIOInput *vinput,
   virtio_input_config *config);
diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c
new file mode 100644
index 00..6da497b1a8
--- /dev/null
+++ b/hw/input/vhost-user-input.c
@@ -0,0 +1,129 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio-input.h"
+
+static int vhost_input_config_change(struct vhost_dev *dev)
+{
+error_report("vhost-user-input: unhandled backend config change");
+return -1;
+}
+
+static const VhostDevConfigOps config_ops = {
+.vhost_dev_config_notifier = vhost_input_config_change,
+};
+
+static void vhost_input_realize(DeviceState *dev, Error **errp)
+{
+VHostUserInput *vhi = VHOST_USER_INPUT(dev);
+VirtIOInput *vinput = VIRTIO_INPUT(dev);
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+vhost_dev_set_config_notifier(&vhi->vhost->dev, &config_ops);
+vinput->cfg_size = sizeof_field(virtio_input_config, u);
+if (vhost_user_backend_dev_init(vhi->vhost, vdev, 2, errp) == -1) {
+return;
+}
+}
+
+static void vhost_input_change_active(VirtIOInput *vinput)
+{
+VHostUserInput *vhi = VHOST_USER_INPUT(vinput);
+
+if (vinput->active) {
+vhost_user_backend_start(vhi->vhost);
+} else {
+vhost_user_backend_stop(vhi->vhost);
+}
+}
+
+static void vhost_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+VirtIOInput *vinput = VIRTIO_INPUT(vdev);
+VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+int ret;
+
+memset(config_data, 0, vinput->cfg_size);
+
+ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, 
vinput->cfg_size);
+if (ret) {
+error_report("vhost-user-input: get device config space failed");
+return;
+}
+}
+
+static void vhost_input_set_config(VirtIODevice *vdev,
+   const uint8_t *config_data)
+{
+VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+int ret;
+
+ret = vhost_dev_set_config(&vhi->vhost->dev, config_data,
+   0, sizeof(virtio_input_config),
+   VHOST_SET_CONFIG_TYPE_MASTER);
+if (ret) {
+error_report("vhost-user-input: set device config space failed");
+return;
+}
+
+virtio_n

Re: [Qemu-devel] [PATCH 6/6] contrib: add vhost-user-input

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 12:25 PM Marc-André Lureau
 wrote:
>
> Add a vhost-user input backend example, based on virtio-input-host
> device. It takes an evdev path as argument, and can be associated with
> a vhost-user-input device via a UNIX socket:
>
> $ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock
>
> $ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock
>   -device vhost-user-input-pci,chardev=vuic
>
> This example is intentionally not included in $TOOLS, and not
> installed by default.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  contrib/vhost-user-input/main.c| 393 +
>  MAINTAINERS|   1 +
>  Makefile   |   9 +
>  Makefile.objs  |   1 +
>  contrib/vhost-user-input/Makefile.objs |   1 +
>  5 files changed, 405 insertions(+)
>  create mode 100644 contrib/vhost-user-input/main.c
>  create mode 100644 contrib/vhost-user-input/Makefile.objs
>
> diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
> new file mode 100644
> index 00..8d493f598e
> --- /dev/null
> +++ b/contrib/vhost-user-input/main.c
> @@ -0,0 +1,393 @@
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include 
> +#include 
> +
> +#include "qemu/iov.h"
> +#include "qemu/bswap.h"
> +#include "qemu/sockets.h"
> +#include "contrib/libvhost-user/libvhost-user.h"
> +#include "contrib/libvhost-user/libvhost-user-glib.h"
> +#include "standard-headers/linux/virtio_input.h"
> +#include "qapi/error.h"
> +
> +typedef struct virtio_input_event virtio_input_event;
> +typedef struct virtio_input_config virtio_input_config;
> +
> +typedef struct VuInput {
> +VugDev dev;
> +GSource *evsrc;
> +int evdevfd;
> +GArray *config;
> +virtio_input_config *sel_config;
> +struct {
> +virtio_input_event event;
> +VuVirtqElement *elem;
> +} *queue;
> +uint32_t qindex, qsize;
> +} VuInput;
> +
> +static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
> +{
> +VuDev *dev = &vi->dev.parent;
> +VuVirtq *vq = vu_get_queue(dev, 0);
> +VuVirtqElement *elem;
> +int i, len;
> +
> +/* queue up events ... */
> +if (vi->qindex == vi->qsize) {
> +vi->qsize++;
> +vi->queue = g_realloc_n(vi->queue, vi->qsize, sizeof(vi->queue[0]));
> +}
> +vi->queue[vi->qindex++].event = *event;
> +
> +/* ... until we see a report sync ... */
> +if (event->type != htole16(EV_SYN) ||
> +event->code != htole16(SYN_REPORT)) {
> +return;
> +}
> +
> +/* ... then check available space ... */
> +for (i = 0; i < vi->qindex; i++) {
> +elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
> +if (!elem) {
> +while (--i >= 0) {
> +vu_queue_unpop(dev, vq, vi->queue[i].elem, 0);
> +}
> +vi->qindex = 0;
> +g_warning("virtio-input queue full");
> +return;
> +}
> +vi->queue[i].elem = elem;
> +}
> +
> +/* ... and finally pass them to the guest */
> +for (i = 0; i < vi->qindex; i++) {
> +elem = vi->queue[i].elem;
> +len = iov_from_buf(elem->in_sg, elem->in_num,
> +   0, &vi->queue[i].event, 
> sizeof(virtio_input_event));
> +vu_queue_push(dev, vq, elem, len);
> +g_free(elem);
> +}
> +
> +vu_queue_notify(&vi->dev.parent, vq);
> +vi->qindex = 0;
> +}
> +
> +static void
> +vi_evdev_watch(VuDev *dev, int condition, void *data)
> +{
> +VuInput *vi = data;
> +int fd = vi->evdevfd;
> +
> +g_debug("Got evdev condition %x", condition);
> +
> +struct virtio_input_event virtio;
> +struct input_event evdev;
> +int rc;
> +
> +for (;;) {
> +rc = read(fd, &evdev, sizeof(evdev));
> +if (rc != sizeof(evdev)) {
> +break;
> +}
> +
> +g_debug("input %d %d %d", evdev.type, evdev.code, evdev.value);
> +
> +virtio.type  = htole16(evdev.type);
> +virtio.code  = htole16(evdev.code);
> +virtio.value = htole32(evdev.value);
> +vi_input_send(vi, &virtio);
> +}
> +}
> +
> +
> +static void vi_handle_status(VuInput *vi, virtio_input_event *event)
> +{
> +struct input_event evdev;
> +int rc;
> +
> +if (gettimeofday(&evdev.time, NULL)) {
> +perror("vi_handle_status: gettimeofday");
> +return;
> +}
> +
> +evdev.type = le16toh(event->type);
> +evdev.code = le16toh(event->code);
> +evdev.value = le32toh(event->value);
> +
> +rc = write(vi->evdevfd, &evdev, sizeof(evdev));
> +if (rc == -1) {
> +perror("vi_host_handle_status: write");
> +}
> +}
> +
> +static void vi_handle_sts(VuDev *dev, int qidx)
> +{
> +VuInput *vi = contain

Re: [Qemu-devel] Failing qemu-iotest 005 with raw

[Kevin Wolf]

Am 26.04.2019 um 11:41 hat Thomas Huth geschrieben:
> 
> When running iotest 005 with raw, the test currently fails for me:
> 
> 005 - output mismatch (see 005.out.bad)
> --- /home/thuth/devel/qemu/tests/qemu-iotests/005.out 2019-04-23
> 16:43:11.0 +0200
> +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/005.out.bad 2019-04-26
> 11:34:11.0 +0200
> @@ -1,13 +1,12 @@
>  QA output created by 005
> 
>  creating large image
> +qemu-img: TEST_DIR/t.IMGFMT: The image size is too large for file
> format 'IMGFMT'
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912
> [...]
> 
> Could this be fixed somehow, or should the test rather be skipped for
> IMGFMT=raw?

The test passes for me on XFS. Looks like the raw driver can handle
large image files, but your host filesystem can't.

We would have to check whether the host filesystem can support large
files and skip the test if it can't. I'm not sure how to do that. But
actually, this isn't testing anything interesting for raw, so just
unconditionally disabling the test for raw could be reasonable enough.

Kevin

[Qemu-devel] [PATCH v2 00/10] s390x: new guest features

[Christian Borntraeger]

Adding gen15.

v1->v2: - rework csske deprecation
- white space fixes
- also require msa4 for msa9

Christian Borntraeger (7):
*** BLURB HERE ***

Christian Borntraeger (10):
  linux header sync
  s390x/cpumodel: ignore csske for expansion
  s390x/cpumodel: Miscellaneous-Instruction-Extensions Facility 3
  s390x/cpumodel: msa9 facility
  s390x/cpumodel: vector enhancements
  s390x/cpumodel: enhanced sort facility
  s390x/cpumodel: add Deflate-conversion facility
  s390x/cpumodel: add gen15 defintions
  s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards
  s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

 linux-headers/asm-s390/kvm.h|   5 +-
 target/s390x/cpu_features.c |  54 
 target/s390x/cpu_features.h |   3 +
 target/s390x/cpu_features_def.h |  49 +++
 target/s390x/cpu_models.c   |   6 ++
 target/s390x/gen-features.c | 105 
 target/s390x/kvm.c  |  18 ++
 7 files changed, 239 insertions(+), 1 deletion(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Christian Borntraeger]

8561 and 8562 will be gen15 machines. There is no name yet, lets us use
the cpu id as base name. Later on we can provide aliases with the proper
name.

Signed-off-by: Christian Borntraeger 
---
 target/s390x/cpu_models.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index d683635eb5..dd6415103f 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
 CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
 CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
 CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model ZR1 
GA1"),
+CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
+CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
 };
 
 #define QEMU_MAX_CPU_TYPE 0x2827
-- 
2.17.1

[Qemu-devel] [PATCH v2 09/10] s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards

[Christian Borntraeger]

These facilities are deprecated and no longer needed.

Signed-off-by: Christian Borntraeger 
---
 target/s390x/gen-features.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 6260f56dc1..c346b76bdf 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 #include "cpu_features_def.h"
 
 #define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))
@@ -833,6 +834,11 @@ static void set_bits(uint64_t list[], BitSpec bits)
 }
 }
 
+static inline void clear_bit(uint64_t list[], unsigned long nr)
+{
+list[nr / 64] &= ~(1ULL << (nr % 64));
+}
+
 static void print_feature_defs(void)
 {
 uint64_t base_feat[S390_FEAT_MAX / 64 + 1] = {};
@@ -843,6 +849,12 @@ static void print_feature_defs(void)
 printf("\n/* CPU model feature list data */\n");
 
 for (i = 0; i < ARRAY_SIZE(CpuFeatDef); i++) {
+/* With gen15 CSSKE and BPB are deprecated */
+if (strcmp(CpuFeatDef[i].name, "S390_FEAT_LIST_GEN15_GA1") == 0) {
+clear_bit(base_feat, S390_FEAT_CONDITIONAL_SSKE);
+clear_bit(default_feat, S390_FEAT_CONDITIONAL_SSKE);
+clear_bit(default_feat, S390_FEAT_BPB);
+}
 set_bits(base_feat, CpuFeatDef[i].base_bits);
 /* add the base to the default features */
 set_bits(default_feat, CpuFeatDef[i].base_bits);
-- 
2.17.1

[Qemu-devel] [PATCH v2 08/10] s390x/cpumodel: add gen15 defintions

[Christian Borntraeger]

add several new features (msa9, sort, deflate, additional vector
instructions, new general purpose instructions) to generation 15.

Also disable csske and bpb from the default model. This will allow
to migrate gen15 machines to future machines that do not have these
features.

Signed-off-by: Christian Borntraeger 
---
 target/s390x/gen-features.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 8fc2e8e72f..6260f56dc1 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -419,6 +419,10 @@ static uint16_t base_GEN14_GA1[] = {
 
 #define base_GEN14_GA2 EmptyFeat
 
+static uint16_t base_GEN15_GA1[] = {
+S390_FEAT_MISC_INSTRUCTION_EXT3,
+};
+
 /* Full features (in order of release)
  * Automatically includes corresponding base features.
  * Full features are all features this hardware supports even if kvm/QEMU do 
not
@@ -548,6 +552,16 @@ static uint16_t full_GEN14_GA1[] = {
 
 #define full_GEN14_GA2 EmptyFeat
 
+static uint16_t full_GEN15_GA1[] = {
+S390_FEAT_VECTOR_ENH2,
+S390_FEAT_GROUP_ENH_SORT,
+S390_FEAT_GROUP_DEFLATE_CONVERSION,
+S390_FEAT_VECTOR_BCD_ENH,
+S390_FEAT_GROUP_MSA_EXT_9,
+S390_FEAT_GROUP_MSA_EXT_9_PCKMO,
+S390_FEAT_ETOKEN,
+};
+
 /* Default features (in order of release)
  * Automatically includes corresponding base features.
  * Default features are all features this version of QEMU supports for this
@@ -624,6 +638,16 @@ static uint16_t default_GEN14_GA1[] = {
 
 #define default_GEN14_GA2 EmptyFeat
 
+static uint16_t default_GEN15_GA1[] = {
+S390_FEAT_VECTOR_ENH2,
+S390_FEAT_GROUP_ENH_SORT,
+S390_FEAT_GROUP_DEFLATE_CONVERSION,
+S390_FEAT_VECTOR_BCD_ENH,
+S390_FEAT_GROUP_MSA_EXT_9,
+S390_FEAT_GROUP_MSA_EXT_9_PCKMO,
+S390_FEAT_ETOKEN,
+};
+
 /* QEMU (CPU model) features */
 
 static uint16_t qemu_V2_11[] = {
@@ -740,6 +764,7 @@ static CpuFeatDefSpec CpuFeatDef[] = {
 CPU_FEAT_INITIALIZER(GEN13_GA2),
 CPU_FEAT_INITIALIZER(GEN14_GA1),
 CPU_FEAT_INITIALIZER(GEN14_GA2),
+CPU_FEAT_INITIALIZER(GEN15_GA1),
 };
 
 #define FEAT_GROUP_INITIALIZER(_name)  \
-- 
2.17.1

[Qemu-devel] [PATCH v2 03/10] s390x/cpumodel: Miscellaneous-Instruction-Extensions Facility 3

[Christian Borntraeger]

Provide the "Miscellaneous-Instruction-Extensions Facility 3" via
stfle.61.

Signed-off-by: Christian Borntraeger 
Reviewed-by: David Hildenbrand 
---
 target/s390x/cpu_features.c | 1 +
 target/s390x/cpu_features_def.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 1843c84aaa..bbd8902087 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -83,6 +83,7 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("minste2", S390_FEAT_TYPE_STFL, 58, 
"Miscellaneous-instruction-extensions facility 2"),
 FEAT_INIT("sema", S390_FEAT_TYPE_STFL, 59, "Semaphore-assist facility"),
 FEAT_INIT("tsi", S390_FEAT_TYPE_STFL, 60, "Time-slice Instrumentation 
facility"),
+FEAT_INIT("minste3", S390_FEAT_TYPE_STFL, 61, 
"Miscellaneous-Instruction-Extensions Facility 3"),
 FEAT_INIT("ri", S390_FEAT_TYPE_STFL, 64, "CPU runtime-instrumentation 
facility"),
 FEAT_INIT("zpci", S390_FEAT_TYPE_STFL, 69, "z/PCI facility"),
 FEAT_INIT("aen", S390_FEAT_TYPE_STFL, 71, 
"General-purpose-adapter-event-notification facility"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 5fc7e7bf01..31dd678301 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -71,6 +71,7 @@ typedef enum {
 S390_FEAT_MISC_INSTRUCTION_EXT,
 S390_FEAT_SEMAPHORE_ASSIST,
 S390_FEAT_TIME_SLICE_INSTRUMENTATION,
+S390_FEAT_MISC_INSTRUCTION_EXT3,
 S390_FEAT_RUNTIME_INSTRUMENTATION,
 S390_FEAT_ZPCI,
 S390_FEAT_ADAPTER_EVENT_NOTIFICATION,
-- 
2.17.1

[Qemu-devel] [PATCH v2 05/10] s390x/cpumodel: vector enhancements

[Christian Borntraeger]

Add vector enhancements to the cpu model.

Signed-off-by: Christian Borntraeger 
Reviewed-by: David Hildenbrand 
---
 target/s390x/cpu_features.c | 2 ++
 target/s390x/cpu_features_def.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 154e2bb354..35873253be 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -108,6 +108,8 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("irbm", S390_FEAT_TYPE_STFL, 145, 
"Insert-reference-bits-multiple facility"),
 FEAT_INIT("msa8-base", S390_FEAT_TYPE_STFL, 146, 
"Message-security-assist-extension-8 facility (excluding subfunctions)"),
 FEAT_INIT("cmmnt", S390_FEAT_TYPE_STFL, 147, "CMM: ESSA-enhancement (no 
translate) facility"),
+FEAT_INIT("vxeh2", S390_FEAT_TYPE_STFL, 148, "Vector Enhancements facility 
2"),
+FEAT_INIT("vxbeh", S390_FEAT_TYPE_STFL, 152, "Vector BCD enhancements 
facility 1"),
 FEAT_INIT("msa9-base", S390_FEAT_TYPE_STFL, 155, 
"Message-security-assist-extension-9 facility (excluding subfunctions)"),
 FEAT_INIT("etoken", S390_FEAT_TYPE_STFL, 156, "Etoken facility"),
 
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 030784811b..ce2223c9d7 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -96,6 +96,8 @@ typedef enum {
 S390_FEAT_INSERT_REFERENCE_BITS_MULT,
 S390_FEAT_MSA_EXT_8,
 S390_FEAT_CMM_NT,
+S390_FEAT_VECTOR_ENH2,
+S390_FEAT_VECTOR_BCD_ENH,
 S390_FEAT_MSA_EXT_9,
 S390_FEAT_ETOKEN,
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 01/10] linux header sync

[Christian Borntraeger]

to be replaced by a proper one

Signed-off-by: Christian Borntraeger 
---
 linux-headers/asm-s390/kvm.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0265482f8f..03ab5968c7 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -152,7 +152,10 @@ struct kvm_s390_vm_cpu_subfunc {
__u8 pcc[16];   /* with MSA4 */
__u8 ppno[16];  /* with MSA5 */
__u8 kma[16];   /* with MSA8 */
-   __u8 reserved[1808];
+   __u8 kdsa[16];  /* with MSA9 */
+   __u8 sortl[32]; /* with STFLE.150 */
+   __u8 dfltcc[32];/* with STFLE.151 */
+   __u8 reserved[1728];
 };
 
 /* kvm attributes for crypto */
-- 
2.17.1

[Qemu-devel] [PATCH v2 04/10] s390x/cpumodel: msa9 facility

[Christian Borntraeger]

Provide the MSA9 facility (stfle.155).
This also contains pckmo functions for key wrapping. Keep them in a
separate group to disable those as a block if necessary.

Signed-off-by: Christian Borntraeger 
---
 target/s390x/cpu_features.c | 32 +
 target/s390x/cpu_features.h |  1 +
 target/s390x/cpu_features_def.h | 31 
 target/s390x/cpu_models.c   |  2 ++
 target/s390x/gen-features.c | 42 +
 target/s390x/kvm.c  |  6 +
 6 files changed, 114 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index bbd8902087..154e2bb354 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -108,6 +108,7 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("irbm", S390_FEAT_TYPE_STFL, 145, 
"Insert-reference-bits-multiple facility"),
 FEAT_INIT("msa8-base", S390_FEAT_TYPE_STFL, 146, 
"Message-security-assist-extension-8 facility (excluding subfunctions)"),
 FEAT_INIT("cmmnt", S390_FEAT_TYPE_STFL, 147, "CMM: ESSA-enhancement (no 
translate) facility"),
+FEAT_INIT("msa9-base", S390_FEAT_TYPE_STFL, 155, 
"Message-security-assist-extension-9 facility (excluding subfunctions)"),
 FEAT_INIT("etoken", S390_FEAT_TYPE_STFL, 156, "Etoken facility"),
 
 /* SCLP SCCB Byte 80 - 98  (bit numbers relative to byte-80) */
@@ -242,6 +243,11 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("pckmo-aes-128", S390_FEAT_TYPE_PCKMO, 18, "PCKMO 
Encrypted-AES-128-Key"),
 FEAT_INIT("pckmo-aes-192", S390_FEAT_TYPE_PCKMO, 19, "PCKMO 
Encrypted-AES-192-Key"),
 FEAT_INIT("pckmo-aes-256", S390_FEAT_TYPE_PCKMO, 20, "PCKMO 
Encrypted-AES-256-Key"),
+FEAT_INIT("pckmo-ecc-p256", S390_FEAT_TYPE_PCKMO, 32, "PCKMO 
Encrypt-ECC-P256-Key"),
+FEAT_INIT("pckmo-ecc-p384", S390_FEAT_TYPE_PCKMO, 33, "PCKMO 
Encrypt-ECC-P384-Key"),
+FEAT_INIT("pckmo-ecc-p521", S390_FEAT_TYPE_PCKMO, 34, "PCKMO 
Encrypt-ECC-P521-Key"),
+FEAT_INIT("pckmo-ecc-ed25519", S390_FEAT_TYPE_PCKMO, 40 , "PCKMO 
Encrypt-ECC-Ed25519-Key"),
+FEAT_INIT("pckmo-ecc-ed448", S390_FEAT_TYPE_PCKMO, 41 , "PCKMO 
Encrypt-ECC-Ed448-Key"),
 
 FEAT_INIT("kmctr-dea", S390_FEAT_TYPE_KMCTR, 1, "KMCTR DEA"),
 FEAT_INIT("kmctr-tdea-128", S390_FEAT_TYPE_KMCTR, 2, "KMCTR TDEA-128"),
@@ -298,6 +304,13 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("pcc-xts-aes-256", S390_FEAT_TYPE_PCC, 52, "PCC 
Compute-XTS-Parameter-Using-AES-256"),
 FEAT_INIT("pcc-xts-eaes-128", S390_FEAT_TYPE_PCC, 58, "PCC 
Compute-XTS-Parameter-Using-Encrypted-AES-128"),
 FEAT_INIT("pcc-xts-eaes-256", S390_FEAT_TYPE_PCC, 60, "PCC 
Compute-XTS-Parameter-Using-Encrypted-AES-256"),
+FEAT_INIT("pcc-scalar-mult-p256", S390_FEAT_TYPE_PCC, 64, "PCC 
Scalar-Multiply-P256"),
+FEAT_INIT("pcc-scalar-mult-p384", S390_FEAT_TYPE_PCC, 65, "PCC 
Scalar-Multiply-P384"),
+FEAT_INIT("pcc-scalar-mult-p521", S390_FEAT_TYPE_PCC, 66, "PCC 
Scalar-Multiply-P521"),
+FEAT_INIT("pcc-scalar-mult-ed25519", S390_FEAT_TYPE_PCC, 72, "PCC 
Scalar-Multiply-Ed25519"),
+FEAT_INIT("pcc-scalar-mult-ed448", S390_FEAT_TYPE_PCC, 73, "PCC 
Scalar-Multiply-Ed448"),
+FEAT_INIT("pcc-scalar-mult-x25519", S390_FEAT_TYPE_PCC, 80, "PCC 
Scalar-Multiply-X25519"),
+FEAT_INIT("pcc-scalar-mult-x448", S390_FEAT_TYPE_PCC, 81, "PCC 
Scalar-Multiply-X448"),
 
 FEAT_INIT("ppno-sha-512-drng", S390_FEAT_TYPE_PPNO, 3, "PPNO 
SHA-512-DRNG"),
 FEAT_INIT("prno-trng-qrtcr", S390_FEAT_TYPE_PPNO, 112, "PRNO 
TRNG-Query-Raw-to-Conditioned-Ratio"),
@@ -309,6 +322,22 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("kma-gcm-eaes-128", S390_FEAT_TYPE_KMA, 26, "KMA 
GCM-Encrypted-AES-128"),
 FEAT_INIT("kma-gcm-eaes-192", S390_FEAT_TYPE_KMA, 27, "KMA 
GCM-Encrypted-AES-192"),
 FEAT_INIT("kma-gcm-eaes-256", S390_FEAT_TYPE_KMA, 28, "KMA 
GCM-Encrypted-AES-256"),
+
+FEAT_INIT("kdsa-ecdsa-verify-p256", S390_FEAT_TYPE_KDSA, 1, "KDSA 
ECDSA-Verify-P256"),
+FEAT_INIT("kdsa-ecdsa-verify-p384", S390_FEAT_TYPE_KDSA, 2, "KDSA 
ECDSA-Verify-P384"),
+FEAT_INIT("kdsa-ecdsa-verify-p521", S390_FEAT_TYPE_KDSA, 3, "KDSA 
ECDSA-Verify-P521"),
+FEAT_INIT("kdsa-ecdsa-sign-p256", S390_FEAT_TYPE_KDSA, 9, "KDSA 
ECDSA-Sign-P256"),
+FEAT_INIT("kdsa-ecdsa-sign-p384", S390_FEAT_TYPE_KDSA, 10, "KDSA 
ECDSA-Sign-P384"),
+FEAT_INIT("kdsa-ecdsa-sign-p521", S390_FEAT_TYPE_KDSA, 11, "KDSA 
ECDSA-Sign-P521"),
+FEAT_INIT("kdsa-eecdsa-sign-p256", S390_FEAT_TYPE_KDSA, 17, "KDSA 
Encrypted-ECDSA-Sign-P256"),
+FEAT_INIT("kdsa-eecdsa-sign-p384", S390_FEAT_TYPE_KDSA, 18, "KDSA 
Encrypted-ECDSA-Sign-P384"),
+FEAT_INIT("kdsa-eecdsa-sign-p521", S390_FEAT_TYPE_KDSA, 19, "KDSA 
Encrypted-ECDSA-Sign-P521"),
+FEAT_INIT("kdsa-eddsa-verify-ed25519", S390_FEAT_TYPE_KDSA, 32, "KDSA 
EdDSA-Verify-Ed25519"),
+FEAT_INIT("kdsa-eddsa-verify-ed448", S390_FEAT_TYPE_KDSA, 36, "KDSA 
EdDSA-Verify-Ed448"),
+FEAT_

Re: [Qemu-devel] [PATCH v2 02/10] s390x/cpumodel: ignore csske for expansion

[David Hildenbrand]

On 26.04.19 13:09, Christian Borntraeger wrote:
> csske will be removed in a future machine. Ignore it for expanding the
> cpu model. Otherwise qemu falls back to z9.
> 
> Signed-off-by: Christian Borntraeger 
> Cc: qemu-sta...@nongnu.org
> ---
>  target/s390x/cpu_models.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index e5afa15512..b4bb5de635 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -1322,6 +1322,8 @@ static void init_ignored_base_feat(void)
>   S390_FEAT_KM_TDEA_192,
>   S390_FEAT_KIMD_SHA_1,
>   S390_FEAT_KLMD_SHA_1,
> + /* CSSKE is deprecated on newer generations */
> + S390_FEAT_CONDITIONAL_SSKE,
>  };
>  int i;
>  
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH v2 02/10] s390x/cpumodel: ignore csske for expansion

[Christian Borntraeger]

csske will be removed in a future machine. Ignore it for expanding the
cpu model. Otherwise qemu falls back to z9.

Signed-off-by: Christian Borntraeger 
Cc: qemu-sta...@nongnu.org
---
 target/s390x/cpu_models.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index e5afa15512..b4bb5de635 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -1322,6 +1322,8 @@ static void init_ignored_base_feat(void)
  S390_FEAT_KM_TDEA_192,
  S390_FEAT_KIMD_SHA_1,
  S390_FEAT_KLMD_SHA_1,
+ /* CSSKE is deprecated on newer generations */
+ S390_FEAT_CONDITIONAL_SSKE,
 };
 int i;
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 06/10] s390x/cpumodel: enhanced sort facility

[Christian Borntraeger]

add the enhanced sort facility.

Signed-off-by: Christian Borntraeger 
Reviewed-by: David Hildenbrand 
---
 target/s390x/cpu_features.c | 10 ++
 target/s390x/cpu_features.h |  1 +
 target/s390x/cpu_features_def.h |  8 
 target/s390x/gen-features.c | 14 ++
 target/s390x/kvm.c  |  6 ++
 5 files changed, 39 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 35873253be..1d19b3072e 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -109,6 +109,7 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("msa8-base", S390_FEAT_TYPE_STFL, 146, 
"Message-security-assist-extension-8 facility (excluding subfunctions)"),
 FEAT_INIT("cmmnt", S390_FEAT_TYPE_STFL, 147, "CMM: ESSA-enhancement (no 
translate) facility"),
 FEAT_INIT("vxeh2", S390_FEAT_TYPE_STFL, 148, "Vector Enhancements facility 
2"),
+FEAT_INIT("esort-base", S390_FEAT_TYPE_STFL, 150, "Enhanced-sort facility 
(excluding subfunctions)"),
 FEAT_INIT("vxbeh", S390_FEAT_TYPE_STFL, 152, "Vector BCD enhancements 
facility 1"),
 FEAT_INIT("msa9-base", S390_FEAT_TYPE_STFL, 155, 
"Message-security-assist-extension-9 facility (excluding subfunctions)"),
 FEAT_INIT("etoken", S390_FEAT_TYPE_STFL, 156, "Etoken facility"),
@@ -340,6 +341,12 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("kdsa-eddsa-sign-ed448", S390_FEAT_TYPE_KDSA, 44, "KDSA 
EdDSA-Sign-Ed448"),
 FEAT_INIT("kdsa-eeddsa-sign-ed25519", S390_FEAT_TYPE_KDSA, 48, "KDSA 
Encrypted-EdDSA-Sign-Ed25519"),
 FEAT_INIT("kdsa-eeddsa-sign-ed448", S390_FEAT_TYPE_KDSA, 52, "KDSA 
Encrypted-EdDSA-Sign-Ed448"),
+
+FEAT_INIT("sortl-sflr", S390_FEAT_TYPE_SORTL, 1, "SORTL SFLR"),
+FEAT_INIT("sortl-svlr", S390_FEAT_TYPE_SORTL, 2, "SORTL SVLR"),
+FEAT_INIT("sortl-32", S390_FEAT_TYPE_SORTL, 130, "SORTL 32 input lists"),
+FEAT_INIT("sortl-128", S390_FEAT_TYPE_SORTL, 132, "SORTL 128 input lists"),
+FEAT_INIT("sortl-f0", S390_FEAT_TYPE_SORTL, 192, "SORTL format 0 
parameter-block"),
 };
 
 const S390FeatDef *s390_feat_def(S390Feat feat)
@@ -403,6 +410,7 @@ void s390_fill_feat_block(const S390FeatBitmap features, 
S390FeatType type,
 case S390_FEAT_TYPE_PPNO:
 case S390_FEAT_TYPE_KMA:
 case S390_FEAT_TYPE_KDSA:
+case S390_FEAT_TYPE_SORTL:
 set_be_bit(0, data); /* query is always available */
 break;
 default:
@@ -430,6 +438,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
S390FeatType type,
nr_bits = 16384;
break;
 case S390_FEAT_TYPE_PLO:
+case S390_FEAT_TYPE_SORTL:
nr_bits = 256;
break;
 default:
@@ -501,6 +510,7 @@ static S390FeatGroupDef s390_feature_groups[] = {
 FEAT_GROUP_INIT("msa9", MSA_EXT_9, "Message-security-assist-extension 9 
facility"),
 FEAT_GROUP_INIT("msa9_pckmo", MSA_EXT_9_PCKMO, 
"Message-security-assist-extension 9 PCKMO subfunctions"),
 FEAT_GROUP_INIT("mepochptff", MULTIPLE_EPOCH_PTFF, "PTFF enhancements 
introduced with Multiple-epoch facility"),
+FEAT_GROUP_INIT("esort", ENH_SORT, "Enhanced-sort facility"),
 };
 
 const S390FeatGroupDef *s390_feat_group_def(S390FeatGroup group)
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index 5ffd3db083..3b8c5b25dd 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -40,6 +40,7 @@ typedef enum {
 S390_FEAT_TYPE_PPNO,
 S390_FEAT_TYPE_KMA,
 S390_FEAT_TYPE_KDSA,
+S390_FEAT_TYPE_SORTL,
 } S390FeatType;
 
 /* Definition of a CPU feature */
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index ce2223c9d7..bb8585847f 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -97,6 +97,7 @@ typedef enum {
 S390_FEAT_MSA_EXT_8,
 S390_FEAT_CMM_NT,
 S390_FEAT_VECTOR_ENH2,
+S390_FEAT_ESORT_BASE,
 S390_FEAT_VECTOR_BCD_ENH,
 S390_FEAT_MSA_EXT_9,
 S390_FEAT_ETOKEN,
@@ -346,6 +347,13 @@ typedef enum {
 S390_FEAT_EEDDSA_SIGN_ED25519,
 S390_FEAT_EEDDSA_SIGN_ED448,
 
+/* SORTL */
+S390_FEAT_SORTL_SFLR,
+S390_FEAT_SORTL_SVLR,
+S390_FEAT_SORTL_32,
+S390_FEAT_SORTL_128,
+S390_FEAT_SORTL_F0,
+
 S390_FEAT_MAX,
 } S390Feat;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index a2f9e2b43f..0a62cc5631 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -245,6 +245,15 @@
 S390_FEAT_PCKMO_ECC_ED25519, \
 S390_FEAT_PCKMO_ECC_ED448
 
+#define S390_FEAT_GROUP_ENH_SORT \
+S390_FEAT_ESORT_BASE, \
+S390_FEAT_SORTL_SFLR, \
+S390_FEAT_SORTL_SVLR, \
+S390_FEAT_SORTL_32, \
+S390_FEAT_SORTL_128, \
+S390_FEAT_SORTL_F0
+
+
 /* cpu feature groups */
 static uint16_t group_PLO[] = {
 S390_FEAT_GROUP_PLO,
@@ -294,6 +303,10 @@ static uint16_t group_MSA_EXT_9_PCKMO[] = {
 S390_FEAT_GROUP_MSA_EXT_9_PCKMO,
 };
 
+static uint16_t group_ENH_SORT[

[Qemu-devel] [PATCH v2 07/10] s390x/cpumodel: add Deflate-conversion facility

[Christian Borntraeger]

add the deflate conversion facility.

Signed-off-by: Christian Borntraeger 
---
 target/s390x/cpu_features.c |  9 +
 target/s390x/cpu_features.h |  1 +
 target/s390x/cpu_features_def.h |  7 +++
 target/s390x/gen-features.c | 12 
 target/s390x/kvm.c  |  6 ++
 5 files changed, 35 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 1d19b3072e..f64f581c86 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -110,6 +110,7 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("cmmnt", S390_FEAT_TYPE_STFL, 147, "CMM: ESSA-enhancement (no 
translate) facility"),
 FEAT_INIT("vxeh2", S390_FEAT_TYPE_STFL, 148, "Vector Enhancements facility 
2"),
 FEAT_INIT("esort-base", S390_FEAT_TYPE_STFL, 150, "Enhanced-sort facility 
(excluding subfunctions)"),
+FEAT_INIT("deflate-base", S390_FEAT_TYPE_STFL, 151, "Deflate-conversion 
facility (excluding subfunctions)"),
 FEAT_INIT("vxbeh", S390_FEAT_TYPE_STFL, 152, "Vector BCD enhancements 
facility 1"),
 FEAT_INIT("msa9-base", S390_FEAT_TYPE_STFL, 155, 
"Message-security-assist-extension-9 facility (excluding subfunctions)"),
 FEAT_INIT("etoken", S390_FEAT_TYPE_STFL, 156, "Etoken facility"),
@@ -347,6 +348,11 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("sortl-32", S390_FEAT_TYPE_SORTL, 130, "SORTL 32 input lists"),
 FEAT_INIT("sortl-128", S390_FEAT_TYPE_SORTL, 132, "SORTL 128 input lists"),
 FEAT_INIT("sortl-f0", S390_FEAT_TYPE_SORTL, 192, "SORTL format 0 
parameter-block"),
+
+FEAT_INIT("dfltcc-gdht", S390_FEAT_TYPE_DFLTCC, 1, "DFLTCC GDHT"),
+FEAT_INIT("dfltcc-cmpr", S390_FEAT_TYPE_DFLTCC, 2, "DFLTCC CMPR"),
+FEAT_INIT("dfltcc-xpnd", S390_FEAT_TYPE_DFLTCC, 4, "DFLTCC XPND"),
+FEAT_INIT("dfltcc-f0", S390_FEAT_TYPE_DFLTCC, 192, "DFLTCC format 0 
parameter-block"),
 };
 
 const S390FeatDef *s390_feat_def(S390Feat feat)
@@ -411,6 +417,7 @@ void s390_fill_feat_block(const S390FeatBitmap features, 
S390FeatType type,
 case S390_FEAT_TYPE_KMA:
 case S390_FEAT_TYPE_KDSA:
 case S390_FEAT_TYPE_SORTL:
+case S390_FEAT_TYPE_DFLTCC:
 set_be_bit(0, data); /* query is always available */
 break;
 default:
@@ -439,6 +446,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
S390FeatType type,
break;
 case S390_FEAT_TYPE_PLO:
 case S390_FEAT_TYPE_SORTL:
+case S390_FEAT_TYPE_DFLTCC:
nr_bits = 256;
break;
 default:
@@ -511,6 +519,7 @@ static S390FeatGroupDef s390_feature_groups[] = {
 FEAT_GROUP_INIT("msa9_pckmo", MSA_EXT_9_PCKMO, 
"Message-security-assist-extension 9 PCKMO subfunctions"),
 FEAT_GROUP_INIT("mepochptff", MULTIPLE_EPOCH_PTFF, "PTFF enhancements 
introduced with Multiple-epoch facility"),
 FEAT_GROUP_INIT("esort", ENH_SORT, "Enhanced-sort facility"),
+FEAT_GROUP_INIT("deflate", DEFLATE_CONVERSION, "Deflate-conversion 
facility"),
 };
 
 const S390FeatGroupDef *s390_feat_group_def(S390FeatGroup group)
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index 3b8c5b25dd..da695a8346 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -41,6 +41,7 @@ typedef enum {
 S390_FEAT_TYPE_KMA,
 S390_FEAT_TYPE_KDSA,
 S390_FEAT_TYPE_SORTL,
+S390_FEAT_TYPE_DFLTCC,
 } S390FeatType;
 
 /* Definition of a CPU feature */
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index bb8585847f..292b17b35d 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -98,6 +98,7 @@ typedef enum {
 S390_FEAT_CMM_NT,
 S390_FEAT_VECTOR_ENH2,
 S390_FEAT_ESORT_BASE,
+S390_FEAT_DEFLATE_BASE,
 S390_FEAT_VECTOR_BCD_ENH,
 S390_FEAT_MSA_EXT_9,
 S390_FEAT_ETOKEN,
@@ -354,6 +355,12 @@ typedef enum {
 S390_FEAT_SORTL_128,
 S390_FEAT_SORTL_F0,
 
+/* DEFLATE */
+S390_FEAT_DEFLATE_GHDT,
+S390_FEAT_DEFLATE_CMPR,
+S390_FEAT_DEFLATE_XPND,
+S390_FEAT_DEFLATE_F0,
+
 S390_FEAT_MAX,
 } S390Feat;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 0a62cc5631..8fc2e8e72f 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -254,6 +254,13 @@
 S390_FEAT_SORTL_F0
 
 
+#define S390_FEAT_GROUP_DEFLATE_CONVERSION \
+S390_FEAT_DEFLATE_BASE, \
+S390_FEAT_DEFLATE_GHDT, \
+S390_FEAT_DEFLATE_CMPR, \
+S390_FEAT_DEFLATE_XPND, \
+S390_FEAT_DEFLATE_F0
+
 /* cpu feature groups */
 static uint16_t group_PLO[] = {
 S390_FEAT_GROUP_PLO,
@@ -307,6 +314,10 @@ static uint16_t group_ENH_SORT[] = {
 S390_FEAT_GROUP_ENH_SORT,
 };
 
+static uint16_t group_DEFLATE_CONVERSION[] = {
+S390_FEAT_GROUP_DEFLATE_CONVERSION,
+};
+
 /* Base features (in order of release)
  * Only non-hypervisor managed features belong here.
  * Base feature sets are static meaning they do not change in future QEMU
@@ -766,6 +777,7 @@ static Fea

Re: [Qemu-devel] [PATCH v2 09/10] s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards

[David Hildenbrand]

On 26.04.19 13:10, Christian Borntraeger wrote:
> These facilities are deprecated and no longer needed.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/gen-features.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 6260f56dc1..c346b76bdf 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -13,6 +13,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include "cpu_features_def.h"
>  
>  #define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))
> @@ -833,6 +834,11 @@ static void set_bits(uint64_t list[], BitSpec bits)
>  }
>  }
>  
> +static inline void clear_bit(uint64_t list[], unsigned long nr)
> +{
> +list[nr / 64] &= ~(1ULL << (nr % 64));
> +}
> +
>  static void print_feature_defs(void)
>  {
>  uint64_t base_feat[S390_FEAT_MAX / 64 + 1] = {};
> @@ -843,6 +849,12 @@ static void print_feature_defs(void)
>  printf("\n/* CPU model feature list data */\n");
>  
>  for (i = 0; i < ARRAY_SIZE(CpuFeatDef); i++) {
> +/* With gen15 CSSKE and BPB are deprecated */
> +if (strcmp(CpuFeatDef[i].name, "S390_FEAT_LIST_GEN15_GA1") == 0) {
> +clear_bit(base_feat, S390_FEAT_CONDITIONAL_SSKE);
> +clear_bit(default_feat, S390_FEAT_CONDITIONAL_SSKE);
> +clear_bit(default_feat, S390_FEAT_BPB);
> +}
>  set_bits(base_feat, CpuFeatDef[i].base_bits);
>  /* add the base to the default features */
>  set_bits(default_feat, CpuFeatDef[i].base_bits);
> 

Can you squash that into the previous patch, so it doesn't look like the
case model is suddenly changed?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 09/10] s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards

[David Hildenbrand]

On 26.04.19 13:18, Christian Borntraeger wrote:
> 
> 
> On 26.04.19 13:18, David Hildenbrand wrote:
>> On 26.04.19 13:10, Christian Borntraeger wrote:
>>> These facilities are deprecated and no longer needed.
>>>
>>> Signed-off-by: Christian Borntraeger 
>>> ---
>>>  target/s390x/gen-features.c | 12 
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>> index 6260f56dc1..c346b76bdf 100644
>>> --- a/target/s390x/gen-features.c
>>> +++ b/target/s390x/gen-features.c
>>> @@ -13,6 +13,7 @@
>>>  
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include "cpu_features_def.h"
>>>  
>>>  #define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))
>>> @@ -833,6 +834,11 @@ static void set_bits(uint64_t list[], BitSpec bits)
>>>  }
>>>  }
>>>  
>>> +static inline void clear_bit(uint64_t list[], unsigned long nr)
>>> +{
>>> +list[nr / 64] &= ~(1ULL << (nr % 64));
>>> +}
>>> +
>>>  static void print_feature_defs(void)
>>>  {
>>>  uint64_t base_feat[S390_FEAT_MAX / 64 + 1] = {};
>>> @@ -843,6 +849,12 @@ static void print_feature_defs(void)
>>>  printf("\n/* CPU model feature list data */\n");
>>>  
>>>  for (i = 0; i < ARRAY_SIZE(CpuFeatDef); i++) {
>>> +/* With gen15 CSSKE and BPB are deprecated */
>>> +if (strcmp(CpuFeatDef[i].name, "S390_FEAT_LIST_GEN15_GA1") == 0) {
>>> +clear_bit(base_feat, S390_FEAT_CONDITIONAL_SSKE);
>>> +clear_bit(default_feat, S390_FEAT_CONDITIONAL_SSKE);
>>> +clear_bit(default_feat, S390_FEAT_BPB);
>>> +}
>>>  set_bits(base_feat, CpuFeatDef[i].base_bits);
>>>  /* add the base to the default features */
>>>  set_bits(default_feat, CpuFeatDef[i].base_bits);
>>>
>>
>> Can you squash that into the previous patch, so it doesn't look like the
>> case model is suddenly changed?
> 
> can do. I wanted it to be separate for better review.
> 

Makes perfect sense.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Christian Borntraeger]

On 26.04.19 13:21, David Hildenbrand wrote:
> On 26.04.19 13:10, Christian Borntraeger wrote:
>> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
>> the cpu id as base name. Later on we can provide aliases with the proper
>> name.
>>
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  target/s390x/cpu_models.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index d683635eb5..dd6415103f 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
>> ZR1 GA1"),
>> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
>> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>>  };
>>  
>>  #define QEMU_MAX_CPU_TYPE 0x2827
>>
> 
> Can you fixup the comment at the beginning of the definitions, stating
> that base features of new HW are no longer a superset?

Will do.

Re: [Qemu-devel] [PATCH v2 09/10] s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards

[Christian Borntraeger]

On 26.04.19 13:18, David Hildenbrand wrote:
> On 26.04.19 13:10, Christian Borntraeger wrote:
>> These facilities are deprecated and no longer needed.
>>
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  target/s390x/gen-features.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 6260f56dc1..c346b76bdf 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -13,6 +13,7 @@
>>  
>>  #include 
>>  #include 
>> +#include 
>>  #include "cpu_features_def.h"
>>  
>>  #define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))
>> @@ -833,6 +834,11 @@ static void set_bits(uint64_t list[], BitSpec bits)
>>  }
>>  }
>>  
>> +static inline void clear_bit(uint64_t list[], unsigned long nr)
>> +{
>> +list[nr / 64] &= ~(1ULL << (nr % 64));
>> +}
>> +
>>  static void print_feature_defs(void)
>>  {
>>  uint64_t base_feat[S390_FEAT_MAX / 64 + 1] = {};
>> @@ -843,6 +849,12 @@ static void print_feature_defs(void)
>>  printf("\n/* CPU model feature list data */\n");
>>  
>>  for (i = 0; i < ARRAY_SIZE(CpuFeatDef); i++) {
>> +/* With gen15 CSSKE and BPB are deprecated */
>> +if (strcmp(CpuFeatDef[i].name, "S390_FEAT_LIST_GEN15_GA1") == 0) {
>> +clear_bit(base_feat, S390_FEAT_CONDITIONAL_SSKE);
>> +clear_bit(default_feat, S390_FEAT_CONDITIONAL_SSKE);
>> +clear_bit(default_feat, S390_FEAT_BPB);
>> +}
>>  set_bits(base_feat, CpuFeatDef[i].base_bits);
>>  /* add the base to the default features */
>>  set_bits(default_feat, CpuFeatDef[i].base_bits);
>>
> 
> Can you squash that into the previous patch, so it doesn't look like the
> case model is suddenly changed?

can do. I wanted it to be separate for better review.

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[David Hildenbrand]

On 26.04.19 13:10, Christian Borntraeger wrote:
> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
> the cpu id as base name. Later on we can provide aliases with the proper
> name.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/cpu_models.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index d683635eb5..dd6415103f 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model ZR1 
> GA1"),
> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>  };
>  
>  #define QEMU_MAX_CPU_TYPE 0x2827
> 

Can you fixup the comment at the beginning of the definitions, stating
that base features of new HW are no longer a superset?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Christian Borntraeger]

On 26.04.19 13:21, David Hildenbrand wrote:
> On 26.04.19 13:10, Christian Borntraeger wrote:
>> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
>> the cpu id as base name. Later on we can provide aliases with the proper
>> name.
>>
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  target/s390x/cpu_models.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index d683635eb5..dd6415103f 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
>> ZR1 GA1"),
>> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
>> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>>  };
>>  
>>  #define QEMU_MAX_CPU_TYPE 0x2827
>>
> 
> Can you fixup the comment at the beginning of the definitions, stating
> that base features of new HW are no longer a superset?
> 

Something like

/*
 * CPU definition list in order of release. Up to generation 14 base features
 * of a following release have been a superset of the previous release. With
 * generation 15 two features have been deprecated.
 */

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 10:27:58AM +0200, Paolo Bonzini wrote:
> On 25/04/19 22:00, Eduardo Habkost wrote:
> > diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> > new file mode 100644
> > index 00..c710f23388
> > --- /dev/null
> > +++ b/hw/core/qdev-hotplug-stubs.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * qdev hotplug handler stubs (for user-mode emulation and unit tests)
> 
> Can you explain the issue with unit tests in the commit message?

I don't think there are issues with unit tests.  I just thought
it was pointless to link qdev-hotplug.o into unit tests binaries
if no hotplug handler object is created by them.

What do you think?  Should I keep qdev-hotplug.o and hotplug.o in
the unit tests binaries?

> 
> > + *  Copyright (c) 2019 Red Hat Inc
> > + *
> > + * Authors:
> > + *  Eduardo Habkost 
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see 
> > .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/hotplug.h"
> > +
> > +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> > +{
> > +return NULL;
> > +}
> > +
> > +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> > +  DeviceState *plugged_dev,
> > +  Error **errp)
> > +{
> > +assert(plug_handler == NULL);
> > +}
> > +
> > +void hotplug_handler_plug(HotplugHandler *plug_handler,
> > +  DeviceState *plugged_dev,
> > +  Error **errp)
> > +{
> > +assert(plug_handler == NULL);
> > +}
> 
> Would it work if you instead make these functions (and the others in
> hw/core/hotplug.c) inlines in include/hw/hotplug.h?
> 
> Then all that remains is qdev_get_hotplug_handler.

I think that would work.  I'll do that in v2, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 00/10] s390x: new guest features

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190426111003.21246-1-borntrae...@de.ibm.com/



Hi,

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

Type: series
Message-id: 20190426111003.21246-1-borntrae...@de.ibm.com
Subject: [Qemu-devel] [PATCH v2 00/10] s390x: new guest features

=== 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
 * [new tag]   
patchew/20190426111003.21246-1-borntrae...@de.ibm.com -> 
patchew/20190426111003.21246-1-borntrae...@de.ibm.com
Switched to a new branch 'test'
18f54345e8 s390x/cpumodel: wire up 8561 and 8562 as gen15 machines
163a238ad5 s390x/cpumodel: disable csske and bpb from gen15 cpu models onwards
dbe4f8f72f s390x/cpumodel: add gen15 defintions
c9369bbdf2 s390x/cpumodel: add Deflate-conversion facility
659a7911c6 s390x/cpumodel: enhanced sort facility
57ff8285dc s390x/cpumodel: vector enhancements
7d6b294b58 s390x/cpumodel: msa9 facility
686cd4656b s390x/cpumodel: Miscellaneous-Instruction-Extensions Facility 3
477fabcbc3 s390x/cpumodel: ignore csske for expansion
964a0a1164 linux header sync

=== OUTPUT BEGIN ===
1/10 Checking commit 964a0a1164ff (linux header sync)
2/10 Checking commit 477fabcbc36d (s390x/cpumodel: ignore csske for expansion)
3/10 Checking commit 686cd4656b1a (s390x/cpumodel: 
Miscellaneous-Instruction-Extensions Facility 3)
ERROR: line over 90 characters
#22: FILE: target/s390x/cpu_features.c:86:
+FEAT_INIT("minste3", S390_FEAT_TYPE_STFL, 61, 
"Miscellaneous-Instruction-Extensions Facility 3"),

total: 1 errors, 0 warnings, 14 lines checked

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

4/10 Checking commit 7d6b294b5845 (s390x/cpumodel: msa9 facility)
ERROR: line over 90 characters
#22: FILE: target/s390x/cpu_features.c:111:
+FEAT_INIT("msa9-base", S390_FEAT_TYPE_STFL, 155, 
"Message-security-assist-extension-9 facility (excluding subfunctions)"),

WARNING: line over 80 characters
#30: FILE: target/s390x/cpu_features.c:246:
+FEAT_INIT("pckmo-ecc-p256", S390_FEAT_TYPE_PCKMO, 32, "PCKMO 
Encrypt-ECC-P256-Key"),

WARNING: line over 80 characters
#31: FILE: target/s390x/cpu_features.c:247:
+FEAT_INIT("pckmo-ecc-p384", S390_FEAT_TYPE_PCKMO, 33, "PCKMO 
Encrypt-ECC-P384-Key"),

WARNING: line over 80 characters
#32: FILE: target/s390x/cpu_features.c:248:
+FEAT_INIT("pckmo-ecc-p521", S390_FEAT_TYPE_PCKMO, 34, "PCKMO 
Encrypt-ECC-P521-Key"),

ERROR: line over 90 characters
#33: FILE: target/s390x/cpu_features.c:249:
+FEAT_INIT("pckmo-ecc-ed25519", S390_FEAT_TYPE_PCKMO, 40 , "PCKMO 
Encrypt-ECC-Ed25519-Key"),

ERROR: line over 90 characters
#34: FILE: target/s390x/cpu_features.c:250:
+FEAT_INIT("pckmo-ecc-ed448", S390_FEAT_TYPE_PCKMO, 41 , "PCKMO 
Encrypt-ECC-Ed448-Key"),

WARNING: line over 80 characters
#42: FILE: target/s390x/cpu_features.c:307:
+FEAT_INIT("pcc-scalar-mult-p256", S390_FEAT_TYPE_PCC, 64, "PCC 
Scalar-Multiply-P256"),

WARNING: line over 80 characters
#43: FILE: target/s390x/cpu_features.c:308:
+FEAT_INIT("pcc-scalar-mult-p384", S390_FEAT_TYPE_PCC, 65, "PCC 
Scalar-Multiply-P384"),

WARNING: line over 80 characters
#44: FILE: target/s390x/cpu_features.c:309:
+FEAT_INIT("pcc-scalar-mult-p521", S390_FEAT_TYPE_PCC, 66, "PCC 
Scalar-Multiply-P521"),

ERROR: line over 90 characters
#45: FILE: target/s390x/cpu_features.c:310:
+FEAT_INIT("pcc-scalar-mult-ed25519", S390_FEAT_TYPE_PCC, 72, "PCC 
Scalar-Multiply-Ed25519"),

ERROR: line over 90 characters
#46: FILE: target/s390x/cpu_features.c:311:
+FEAT_INIT("pcc-scalar-mult-ed448", S390_FEAT_TYPE_PCC, 73, "PCC 
Scalar-Multiply-Ed448"),

ERROR: line over 90 characters
#47: FILE: target/s390x/cpu_features.c:312:
+FEAT_INIT("pcc-scalar-mult-x25519", S390_FEAT_TYPE_PCC, 80, "PCC 
Scalar-Multiply-X25519"),

WARNING: line over 80 characters
#48: FILE: target/s390x/cpu_features.c:313:
+FEAT_INIT("pcc-scalar-mult-x448", S390_FEAT_TYPE_PCC, 81, "PCC 
Scalar-Multiply-X448"),

WARNING: line over 80 characters
#57: FILE: target/s390x/cpu_features.c:326:
+FEAT_INIT("kdsa-ecdsa-verify-p256", S390_FEAT_TYPE_KDSA, 1, "KDSA 
ECDSA-Verify-P256"),

WARNING: line over 80 characters
#58: FILE: target/s390x/cpu_features.c:327:
+FEAT_INIT("kdsa-ecdsa-verify-p384", S390_FEAT_TYPE_KDSA, 2, "KDSA 
ECDSA-Verify-P384"),

WARNING: line over 80 characters
#59: FILE: target/s390x/cpu_features.c:328:
+FEAT_INIT("kdsa-ecdsa-verify-p521", S390_FEAT_TYPE_KDSA, 3, "KDSA 
ECDSA-Verify-P521"),

WARNING: line over 80 characters
#60: FILE: target/s390x/cpu_features.c:329:
+FEAT_INIT("kdsa-ecdsa-sign-p256", S390

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[David Hildenbrand]

On 26.04.19 13:10, Christian Borntraeger wrote:
> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
> the cpu id as base name. Later on we can provide aliases with the proper
> name.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/cpu_models.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index d683635eb5..dd6415103f 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model ZR1 
> GA1"),
> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>  };

Thinking out loud, I know that official names are not published yet.
Looking at the recent history (z13, z14), my educated guess would be
z15. I guess pretty much everybody would guess that.

So I wonder if using "z15" and e.g. "z15ZR1" or "z15s" would be easier
to have, although in the end maybe not correct (whatever names smart
people are able to come up with, history told us that consistent naming
does not seem to be one of the core strengths of CPU marketing people in
general).

So if we need aliases either way, and z15 is easier to understand than
"8561" for any user, what about going forward with that and introducing
actual alias later on? We can tag the names to be inofficial.

CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "z15", "Inofficial name for
gen15 model 1 GA1 (8561)"),
CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "z15ZR1", "Inofficial name
for gen15 model 2 GA1 (8562)"),

(please don't eat me alive)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[David Hildenbrand]

On 26.04.19 13:24, Christian Borntraeger wrote:
> 
> 
> On 26.04.19 13:21, David Hildenbrand wrote:
>> On 26.04.19 13:10, Christian Borntraeger wrote:
>>> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
>>> the cpu id as base name. Later on we can provide aliases with the proper
>>> name.
>>>
>>> Signed-off-by: Christian Borntraeger 
>>> ---
>>>  target/s390x/cpu_models.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index d683635eb5..dd6415103f 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>>>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>>>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>>>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
>>> ZR1 GA1"),
>>> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
>>> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>>>  };
>>>  
>>>  #define QEMU_MAX_CPU_TYPE 0x2827
>>>
>>
>> Can you fixup the comment at the beginning of the definitions, stating
>> that base features of new HW are no longer a superset?
>>
> 
> Something like
> 
> /*
>  * CPU definition list in order of release. Up to generation 14 base 
> features>  * of a following release have been a superset of the previous
release. With
>  * generation 15 two features have been deprecated.
>  */
> 

"one base feature and one optional feature" ?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Christian Borntraeger]

On 26.04.19 13:32, David Hildenbrand wrote:
> On 26.04.19 13:10, Christian Borntraeger wrote:
>> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
>> the cpu id as base name. Later on we can provide aliases with the proper
>> name.
>>
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  target/s390x/cpu_models.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index d683635eb5..dd6415103f 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
>> ZR1 GA1"),
>> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
>> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>>  };
> 
> Thinking out loud, I know that official names are not published yet.
> Looking at the recent history (z13, z14), my educated guess would be
> z15. I guess pretty much everybody would guess that.

Not sure about trademark aspects - especially if this really becomes z15. The 
smaller
machine has no real history (ZR1 vs. s vs BC). So I think I would rather have a 
correct
number than a partially correct name.

> 
> So I wonder if using "z15" and e.g. "z15ZR1" or "z15s" would be easier
> to have, although in the end maybe not correct (whatever names smart
> people are able to come up with, history told us that consistent naming
> does not seem to be one of the core strengths of CPU marketing people in
> general).
> 
> So if we need aliases either way, and z15 is easier to understand than
> "8561" for any user, what about going forward with that and introducing
> actual alias later on? We can tag the names to be inofficial.
> 
> CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "z15", "Inofficial name for
> gen15 model 1 GA1 (8561)"),
> CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "z15ZR1", "Inofficial name
> for gen15 model 2 GA1 (8562)"),
> 
> (please don't eat me alive)
> 

Re: [Qemu-devel] [PATCH] Revert "armv7m: Guard against no -kernel argument"

[Peter Maydell]

On Fri, 26 Apr 2019 at 10:17, Stefan Hajnoczi  wrote:
>
> On Thu, Apr 25, 2019 at 08:07:06PM +0200, Philippe Mathieu-Daudé wrote:
> > Previous to this commit (v3.1), we have:
> >
> > $ qemu-system-aarch64 -M netduino2
> > qemu-system-aarch64: Guest image must be specified (using -kernel)
> >
> > Now (v4.0) we get:
> >
> > $ qemu-system-aarch64 -M netduino2
> > qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)
>
> A user-friendly error message is needed here.  The check for -kernel was
> too specific and is not desirable for microbit where we use -device
> loader.

It's consistent with how many of our other board models work.
If you try to run them without providing any guest code
in any way, then they just do something not very useful.
(For A-profile this used to often be "die with the 'execution
from something not ROM or RAM' message" and is now "just steam
through executing garbage and/or taking exceptions in a tight loop.")
This is how real hardware also works :-)

(Strictly speaking this message appearing is a defect in
QEMU's emulation -- the 'lockup' state behaviour is architecturally
well-defined and is supposed to cause the CPU to sit there doing
continual instruction fetches from a specific (non-valid) address
until either the system is reset or an NMI handler kicks in which
might be able to rescue it. We don't bother to emulate this detail
because lockup is pretty much always a guest bug and the only thing
I've seen using the lockup behaviour and expecting to recover from it
is test code.)

> Old boards probably want to continue using -kernel.  New boards like
> microbit may use just -device loader.  Perhaps there is even a group
> that wants both options.
>
> A solution is to introduce explicit checks so that we can tell the user
> the appropriate option for the machine type.  I can work on this if you
> like, but probably won't be able to send a patch until Tuesday.

But it's difficult to tell how to identify whether there's really
any guest code there. For instance the user might want to start
QEMU, connect via the gdbstub and load guest code from gdb.
Or they might be using the generic-loader device. Or they might
really be using -kernel but with a broken guest image which doesn't
have a vector table in it, which will result in the same message.
I guess you could have a heuristic for "if an M-profile CPU is in reset
and the value it loads for the starting PC is zero and the gdb
stub is not connected, then print a warning that the guest image
is missing or there's no vector table" but I'm not a big fan of
heuristics...

thanks
-- PMM

Re: [Qemu-devel] About pt-br keyboard layout

[Lin Ma]

On 4/26/19 4:38 PM, Gerd Hoffmann wrote:

On Fri, Apr 26, 2019 at 04:09:12PM +0800, Lin Ma wrote:

Hi all,

While I launch qemu with vnc + pt-br keyboard layout on my pc, If I type
shift + 6 in iPXE shell or grub shell via my usual 105-key keyboard,
shift + 6 would be mapped to "(apostrophe), But IIUC the correct character
should be ¨(diaeresis) in pt-br layout.

I'm wondering that is it the expected behavior? In other words, Does it make
sense if I want to type multi-character characters(say Ç or ¨) without guest
os's help?

No.  -k is for translating keysyms back into the correct scancodes,
because this is what the (virtual) keyboard passes to the guest.
So -k must match the *hosts* keyboard layout.
So, that means, If I launch qemu with -k pt-br on a host, I need to 
ensure the keyboard layout
must be pt-br as well onthathypervisorhost,Then when I connected to this 
vnc server(qemu)
throughvncviewer on mylaptop(us keyboard layout), I type shift + 6, I 
can get¨(diaeresis)

character, am I right?

Translating the scancodes into keysyms again is the job of the guest,
so that requires the keymapping you want to use being active in the
guest os.  Typically that would be the same you have on the host, but
that isn't required.  The guest can work with -- for example -- us
layout (like it is often the case in boot loaders).  Behavior should be
identical to physical hardware here.

HTH,
   Gerd

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Christian Borntraeger]

On 26.04.19 13:52, David Hildenbrand wrote:
> On 26.04.19 13:36, Christian Borntraeger wrote:
>>
>>
>> On 26.04.19 13:32, David Hildenbrand wrote:
>>> On 26.04.19 13:10, Christian Borntraeger wrote:
 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
 the cpu id as base name. Later on we can provide aliases with the proper
 name.

 Signed-off-by: Christian Borntraeger 
 ---
  target/s390x/cpu_models.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
 index d683635eb5..dd6415103f 100644
 --- a/target/s390x/cpu_models.c
 +++ b/target/s390x/cpu_models.c
 @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
 ZR1 GA1"),
 +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
 +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
  };
>>>
>>> Thinking out loud, I know that official names are not published yet.
>>> Looking at the recent history (z13, z14), my educated guess would be
>>> z15. I guess pretty much everybody would guess that.
>>
>> Not sure about trademark aspects - especially if this really becomes z15. 
>> The smaller
>> machine has no real history (ZR1 vs. s vs BC). So I think I would rather 
>> have a correct
>> number than a partially correct name.
> 
> We could also use "gen15a" and "gen15b", still better to get than magic
> numbers. (yeah well, they are not magic)
> 
> If you want to stick with numbers, be aware that cpu numbers are not
> injective, so at some point we will need e.g. "8561.2", just so you're
> aware of the implications.

I think whatever we have here is only used internally for expansion (host-model)
and the user will use later the real name when available. (custom). So probably 
this
does not matter for a long time. But I might be wrong.
I tend to prefer gen15 over z15 but 856x has also its charm.

Re: [Qemu-devel] [PATCH v6 05/11] vhost-user: add vhost_user_gpu_set_socket()

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 9:33 AM Gerd Hoffmann  wrote:
>
> On Tue, Apr 23, 2019 at 03:19:58PM +0200, Marc-André Lureau wrote:
> > Add a new vhost-user message to give a unix socket to a vhost-user
> > backend for GPU display updates.
>
> Can you split input/gpu into two patch series?
>
> > +Wire format
> > +===
> > +
> > +Unless specified differently, numbers are in the machine native byte
> > +order.
> > +
> > +A vhost-user-gpu request consists of 2 header fields and a payload.
> > +
> > ++-+--+-+
> > +| request | size | payload |
> > ++-+--+-+
> > +
> > +Header
> > +--
> > +
> > +:request: ``u32``, type of the request
> > +
> > +:size: ``u32``, size of the payload
> > +
> > +A reply consists only of a payload, whose content depends on the request.
>
> I'd suggest to use the same format for replies, only with "request"
> meaning "status" in replies.  Allows for OK/ERROR status in replies,
> and having a size field in replies too should make things more robust.
> Also allows for an empty reply (status=ok,size=0).


That brings more questions and ambiguity imho.

Can we leave that for future protocol extensions negotiated with
GET/SET_PROTOCOL_FEATURES ?



--
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 11/13] tests: acpi: add simple arm/virt testcase

[Igor Mammedov]

On Fri, 26 Apr 2019 00:51:56 +0800
x00249684  wrote:

> Hi Igor,
> 
> +static void test_acpi_virt_tcg(void)
> +{
> +test_data data = {
> +.machine = "virt",
> +.uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> +.uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> +.cd = 
> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> +.ram_start = 0x4000ULL,
> +.scan_len = 128ULL * 1024 * 1024,
> +};
> +
> +test_acpi_one("-cpu cortex-a57 ", &data);
> 
> Replaced the cortex-a57 with host and succesfully tested on the hisilicon 
> arm64 
> D05 board. Otherwise it failed with "kvm_init_vcpu failed: Invalid argument".
> Is it possilbe to set the cpu type like numa-test.c?

I think it works with numa-test because it uses TCG only but in case of 
bios-tables-test
it uses accel="kvm:tcg" to leverage KVM capabilities whenever possible to speed 
up test.

Now back to our ARM test case, uefi requirement is to use 64bit CPU (hence it 
was cortex-a57)
however unlike x86 it obviously breaks since KVM accelerator on ARM host is used
and it doesn't work with anything other than 'host' cpu model.

I think we still want to use KVM whenever possible, but problem lies in that
user (testcase) doesn't have an idea if KVM accelerator is available and host 
is 64 CPU.

to sum up we need to support 2 modes:
  1. host is 64 ARM, use kvm with -cpu host
  2. all other cases use tcg with -cpu cortex-a57

I can hack to probe if /dev/kvm is accessible and host is 64 bit and use #1
otherwise fallback to #2
or as quick fix do only #2 initially and think about a better solution to #1 

Is there any other suggestions/opinions how to approach issue/proceed.

PS:
we probably would like to reuse this not only for acpi tests but also for other
arm/virt test cases that involve running guest code. 

> Thanks!
> 
> Best Regards,
> Wei

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[David Hildenbrand]

On 26.04.19 13:36, Christian Borntraeger wrote:
> 
> 
> On 26.04.19 13:32, David Hildenbrand wrote:
>> On 26.04.19 13:10, Christian Borntraeger wrote:
>>> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
>>> the cpu id as base name. Later on we can provide aliases with the proper
>>> name.
>>>
>>> Signed-off-by: Christian Borntraeger 
>>> ---
>>>  target/s390x/cpu_models.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index d683635eb5..dd6415103f 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>>>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>>>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>>>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
>>> ZR1 GA1"),
>>> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
>>> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>>>  };
>>
>> Thinking out loud, I know that official names are not published yet.
>> Looking at the recent history (z13, z14), my educated guess would be
>> z15. I guess pretty much everybody would guess that.
> 
> Not sure about trademark aspects - especially if this really becomes z15. The 
> smaller
> machine has no real history (ZR1 vs. s vs BC). So I think I would rather have 
> a correct
> number than a partially correct name.

We could also use "gen15a" and "gen15b", still better to get than magic
numbers. (yeah well, they are not magic)

If you want to stick with numbers, be aware that cpu numbers are not
injective, so at some point we will need e.g. "8561.2", just so you're
aware of the implications.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] About pt-br keyboard layout

[Gerd Hoffmann]

> > So -k must match the *hosts* keyboard layout.
> So, that means, If I launch qemu with -k pt-br on a host, I need to ensure
> the keyboard layout
> must be pt-br as well onthathypervisorhost,Then when I connected to this vnc
> server(qemu)
> throughvncviewer on mylaptop(us keyboard layout), I type shift + 6, I can
> get¨(diaeresis)
> character, am I right?

No.  I've simplified a bit, assuming both vncviewer and qemu run on the
host machine.  -k $layout must match the keyboard layout of the machine
running vncviewer.  So, in your case it should be us (the laptops
layout).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[David Hildenbrand]

On 26.04.19 14:01, Christian Borntraeger wrote:
> 
> 
> On 26.04.19 13:52, David Hildenbrand wrote:
>> On 26.04.19 13:36, Christian Borntraeger wrote:
>>>
>>>
>>> On 26.04.19 13:32, David Hildenbrand wrote:
 On 26.04.19 13:10, Christian Borntraeger wrote:
> 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
> the cpu id as base name. Later on we can provide aliases with the proper
> name.
>
> Signed-off-by: Christian Borntraeger 
> ---
>  target/s390x/cpu_models.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index d683635eb5..dd6415103f 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
>  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
>  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
>  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
> ZR1 GA1"),
> +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 GA1"),
> +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 GA1"),
>  };

 Thinking out loud, I know that official names are not published yet.
 Looking at the recent history (z13, z14), my educated guess would be
 z15. I guess pretty much everybody would guess that.
>>>
>>> Not sure about trademark aspects - especially if this really becomes z15. 
>>> The smaller
>>> machine has no real history (ZR1 vs. s vs BC). So I think I would rather 
>>> have a correct
>>> number than a partially correct name.
>>
>> We could also use "gen15a" and "gen15b", still better to get than magic
>> numbers. (yeah well, they are not magic)
>>
>> If you want to stick with numbers, be aware that cpu numbers are not
>> injective, so at some point we will need e.g. "8561.2", just so you're
>> aware of the implications.
> 
> I think whatever we have here is only used internally for expansion 
> (host-model)
> and the user will use later the real name when available. (custom). So 
> probably this
> does not matter for a long time. But I might be wrong.
> I tend to prefer gen15 over z15 but 856x has also its charm.
> 

Another question would be, can we rename before the next QEMU release,
so it will never be officially part of QEMU? Then we don't need aliases
after all. Of course, distros have to take care of that as well, but
that shouldn't be upstream QEMUs business.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v6 05/11] vhost-user: add vhost_user_gpu_set_socket()

[Gerd Hoffmann]

  Hi,

> > > +Wire format
> > > +===
> > > +
> > > +Unless specified differently, numbers are in the machine native byte
> > > +order.
> > > +
> > > +A vhost-user-gpu request consists of 2 header fields and a payload.
> > > +
> > > ++-+--+-+
> > > +| request | size | payload |
> > > ++-+--+-+
> > > +
> > > +Header
> > > +--
> > > +
> > > +:request: ``u32``, type of the request
> > > +
> > > +:size: ``u32``, size of the payload
> > > +
> > > +A reply consists only of a payload, whose content depends on the request.
> >
> > I'd suggest to use the same format for replies, only with "request"
> > meaning "status" in replies.  Allows for OK/ERROR status in replies,
> > and having a size field in replies too should make things more robust.
> > Also allows for an empty reply (status=ok,size=0).
> 
> 
> That brings more questions and ambiguity imho.

What questions for example?

> Can we leave that for future protocol extensions negotiated with
> GET/SET_PROTOCOL_FEATURES ?

I don't think negotiating such a basic protocol change is a good idea.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file

[Philippe Mathieu-Daudé]

On 4/26/19 7:42 AM, Thomas Huth wrote:
> On 26/04/2019 00.55, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 4/19/19 9:56 AM, Thomas Huth wrote:
>>> First patch fixes a problem with ohci_die(), second patch moves PCI code 
>>> into
>>> a separate file, so that the sysbus OHCI device can also be used without
>>> the dependency on the PCI code.
>>>
>>> v2: Split the patch into two patches, one for the ohci_die() fix and one
>>> for the PCI code movement.
>>
>> Way cleaner. I wonder why you don't use a typedef for the void
>> (*ohci_die_fn)(struct OHCIState *) prototype.
> 
> It does not work in that case:
> 
> typedef struct OHCIState {// <-- struct OHCIState definition
> [...]
> uint32_t async_td;
> bool async_complete;
> 
> void (*ohci_die)(struct OHCIState *ohci); // <-- ohci_die definition
> } OHCIState; // <-- typedef OHCIState definition
> 
> The typedef is defined after the ohci_die entry.

I was thinking of forward declaration:

typedef struct OHCIState OHCIState;

typedef void (ohci_die_fn)(OHCIState *ohci);

struct OHCIState {
[...]
uint32_t async_td;
bool async_complete;

ohci_die_fn *ohci_die;
} OHCIState;

static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
  uint32_t num_ports,
  dma_addr_t localmem_base,
  char *masterbus, uint32_t firstport,
  AddressSpace *as,
  ohci_die_fn *ohci_die, Error **errp)
{ ...

> 
>> Anyway to this series:
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Tested-by: Philippe Mathieu-Daudé 
> 
>  Thanks!
>   Thomas
> 

Re: [Qemu-devel] [PATCH v6 05/11] vhost-user: add vhost_user_gpu_set_socket()

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 2:06 PM Gerd Hoffmann  wrote:
>
>   Hi,
>
> > > > +Wire format
> > > > +===
> > > > +
> > > > +Unless specified differently, numbers are in the machine native byte
> > > > +order.
> > > > +
> > > > +A vhost-user-gpu request consists of 2 header fields and a payload.
> > > > +
> > > > ++-+--+-+
> > > > +| request | size | payload |
> > > > ++-+--+-+
> > > > +
> > > > +Header
> > > > +--
> > > > +
> > > > +:request: ``u32``, type of the request
> > > > +
> > > > +:size: ``u32``, size of the payload
> > > > +
> > > > +A reply consists only of a payload, whose content depends on the 
> > > > request.
> > >
> > > I'd suggest to use the same format for replies, only with "request"
> > > meaning "status" in replies.  Allows for OK/ERROR status in replies,
> > > and having a size field in replies too should make things more robust.
> > > Also allows for an empty reply (status=ok,size=0).
> >
> >
> > That brings more questions and ambiguity imho.
>
> What questions for example?

This opens up different kind of possible replies, and error handling.

With current proposal and needs, the reply (or absence of reply) is
entirely driven by the request.

With your proposal, should all request have a reply? which makes a lot
more code synchronous, and complicates both sides unnecessarily.

>
> > Can we leave that for future protocol extensions negotiated with
> > GET/SET_PROTOCOL_FEATURES ?
>
> I don't think negotiating such a basic protocol change is a good idea.

Well, then I would rather focus on improving protocol negociation,
rather than adding unnecessary protocol changes.

Given that GET/SET_PROTOCOL_FEATURES is the first messages being sent,
why couldn't it have flags indicating new protocol revision?

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file

[Thomas Huth]

On 26/04/2019 14.14, Philippe Mathieu-Daudé wrote:
> On 4/26/19 7:42 AM, Thomas Huth wrote:
>> On 26/04/2019 00.55, Philippe Mathieu-Daudé wrote:
>>> Hi Thomas,
>>>
>>> On 4/19/19 9:56 AM, Thomas Huth wrote:
 First patch fixes a problem with ohci_die(), second patch moves PCI code 
 into
 a separate file, so that the sysbus OHCI device can also be used without
 the dependency on the PCI code.

 v2: Split the patch into two patches, one for the ohci_die() fix and one
 for the PCI code movement.
>>>
>>> Way cleaner. I wonder why you don't use a typedef for the void
>>> (*ohci_die_fn)(struct OHCIState *) prototype.
>>
>> It does not work in that case:
>>
>> typedef struct OHCIState {// <-- struct OHCIState definition
>> [...]
>> uint32_t async_td;
>> bool async_complete;
>>
>> void (*ohci_die)(struct OHCIState *ohci); // <-- ohci_die definition
>> } OHCIState; // <-- typedef OHCIState definition
>>
>> The typedef is defined after the ohci_die entry.
> 
> I was thinking of forward declaration:
> 
> typedef struct OHCIState OHCIState;
> 
> typedef void (ohci_die_fn)(OHCIState *ohci);

Could work, too, but I don't like typedeferities... so unless Gerd
forces me to use that here, I'd prefer to keep the patch in its current
shape.

 Thomas

Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file

[Philippe Mathieu-Daudé]

On 4/26/19 2:20 PM, Thomas Huth wrote:
> On 26/04/2019 14.14, Philippe Mathieu-Daudé wrote:
>> On 4/26/19 7:42 AM, Thomas Huth wrote:
>>> On 26/04/2019 00.55, Philippe Mathieu-Daudé wrote:
 Hi Thomas,

 On 4/19/19 9:56 AM, Thomas Huth wrote:
> First patch fixes a problem with ohci_die(), second patch moves PCI code 
> into
> a separate file, so that the sysbus OHCI device can also be used without
> the dependency on the PCI code.
>
> v2: Split the patch into two patches, one for the ohci_die() fix and one
> for the PCI code movement.

 Way cleaner. I wonder why you don't use a typedef for the void
 (*ohci_die_fn)(struct OHCIState *) prototype.
>>>
>>> It does not work in that case:
>>>
>>> typedef struct OHCIState {// <-- struct OHCIState definition
>>> [...]
>>> uint32_t async_td;
>>> bool async_complete;
>>>
>>> void (*ohci_die)(struct OHCIState *ohci); // <-- ohci_die definition
>>> } OHCIState; // <-- typedef OHCIState definition
>>>
>>> The typedef is defined after the ohci_die entry.
>>
>> I was thinking of forward declaration:
>>
>> typedef struct OHCIState OHCIState;
>>
>> typedef void (ohci_die_fn)(OHCIState *ohci);
> 
> Could work, too, but I don't like typedeferities... so unless Gerd
> forces me to use that here, I'd prefer to keep the patch in its current
> shape.

Fine with me ;)

[Qemu-devel] aio context ownership during bdrv_close()

[Anton Kuchin]

I can't figure out ownership of aio context during bdrv_close().

As far as I understand bdrv_unref() shold be called with acquired aio 
context to prevent concurrent operations (at least most usages in 
blockdev.c explicitly acquire and release context, but not all).


But if refcount reaches zero and bs is going to be deleted in 
bdrv_close() we need to ensure that drain is finished data is flushed 
and there are no more pending coroutines and bottomhalves, so drain and 
flush functions can enter coroutine and perform yield in several places. 
As a result control returns to coroutine caller that will release aio 
context and when completion bh will continue cleanup process it will be 
executed without ownership of context. Is this a valid situation?


Moreover if yield happens bs that is being deleted has zero refcount but 
is still present in lists graph_bdrv_states and all_bdrv_states and can 
be accidentally accessed. Shouldn't we remove it from these lists ASAP 
when deletion process starts as we do from monitor_bdrv_states?

Re: [Qemu-devel] [PATCH 1/3] ram-encrypted-notifier: Introduce a RAM block encrypted notifier

[Igor Mammedov]

On Thu, 25 Apr 2019 22:58:18 +
"Natarajan, Janakarajan"  wrote:

> A client can register to this notifier to know whether the newly added or
> removed memory region is marked as encrypted. This information is needed
> for the SEV guest launch. In SEV guest, some memory regions may contain
> encrypted data (e.g guest RAM). The memory region which contains the
> encrypted data should be registered/unregistered using the
> KVM_MEMORY_{REG,UNREG}_ENCRYPTED ioctl.
> 
> Signed-off-by: Janakarajan Natarajan 
> ---
>  exec.c |  1 +
>  include/exec/memory.h  | 18 ++
>  include/exec/ramlist.h | 19 +++
>  memory.c   | 16 
>  numa.c | 33 +
>  stubs/ram-block.c  |  8 
>  6 files changed, 95 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 2646207661..a02c394e48 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -79,6 +79,7 @@
>   * are protected by the ramlist lock.
>   */
>  RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
> +RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
>  
>  static MemoryRegion *system_memory;
>  static MemoryRegion *system_io;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 9144a47f57..ae720ff511 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -374,6 +374,7 @@ struct MemoryRegion {
>  bool terminates;
>  bool ram_device;
>  bool enabled;
> +bool encrypted;
>  bool warning_printed; /* For reservations */
>  uint8_t vga_logging_count;
>  MemoryRegion *alias;
> @@ -1131,6 +1132,23 @@ int 
> memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
>   */
>  int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
>  
> +/**
> + * memory_region_mark_encrypted: marks the memory region as encrypted and
> + * lets the listeners of encrypted ram know that a memory region containing
> + * encrypted ram block has been added
> + *
> + * @mr: the memory region
> + */
> +void memory_region_mark_encrypted(MemoryRegion *mr);
> +
> +/**
> + * memory_region_is_encrypted: returns if the memory region was marked as
> + * encrypted when it was created
> + *
> + * @mr: the memory region
> + */
> +bool memory_region_is_encrypted(MemoryRegion *mr);
> +
>  /**
>   * memory_region_name: get a memory region's name
>   *
> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> index bc4faa1b00..5349f27fa5 100644
> --- a/include/exec/ramlist.h
> +++ b/include/exec/ramlist.h
> @@ -7,6 +7,7 @@
>  #include "qemu/rcu_queue.h"
>  
>  typedef struct RAMBlockNotifier RAMBlockNotifier;
> +typedef struct RAMBlockEncryptedNotifier RAMBlockEncryptedNotifier;
>  
>  #define DIRTY_MEMORY_VGA   0
>  #define DIRTY_MEMORY_CODE  1
> @@ -55,6 +56,11 @@ typedef struct RAMList {
>  } RAMList;
>  extern RAMList ram_list;
>  
> +typedef struct RAMBlockEncryptedNotifierList {
> +QLIST_HEAD(, RAMBlockEncryptedNotifier) ram_block_notifiers;
> +} RAMBlockEncryptedNotifierList;
> +extern RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
> +
>  /* Should be holding either ram_list.mutex, or the RCU lock. */
>  #define  INTERNAL_RAMBLOCK_FOREACH(block)  \
>  QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
> @@ -70,6 +76,14 @@ struct RAMBlockNotifier {
>  QLIST_ENTRY(RAMBlockNotifier) next;
>  };
>  
> +struct RAMBlockEncryptedNotifier {
> +void (*ram_block_encrypted_added)(RAMBlockEncryptedNotifier *n,
> +  void *host, size_t size);
> +void (*ram_block_encrypted_removed)(RAMBlockEncryptedNotifier *n,
> +void *host, size_t size);
> +QLIST_ENTRY(RAMBlockEncryptedNotifier) next;
> +};
> +
>  void ram_block_notifier_add(RAMBlockNotifier *n);
>  void ram_block_notifier_remove(RAMBlockNotifier *n);
>  void ram_block_notify_add(void *host, size_t size);
> @@ -77,4 +91,9 @@ void ram_block_notify_remove(void *host, size_t size);
>  
>  void ram_block_dump(Monitor *mon);
>  
> +void ram_block_encrypted_notifier_add(RAMBlockEncryptedNotifier *n);
> +void ram_block_encrypted_notifier_remove(RAMBlockEncryptedNotifier *n);
> +void ram_block_encrypted_notify_add(void *host, size_t size);
> +void ram_block_encrypted_notify_remove(void *host, size_t size);
> +
>  #endif /* RAMLIST_H */
> diff --git a/memory.c b/memory.c
> index bb2b71ee38..eca02d369b 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2009,6 +2009,22 @@ int memory_region_iommu_num_indexes(IOMMUMemoryRegion 
> *iommu_mr)
>  return imrc->num_indexes(iommu_mr);
>  }
>  
> +void memory_region_mark_encrypted(MemoryRegion *mr)
> +{
> +RAMBlock *block = mr->ram_block;
> +
> +mr->encrypted = kvm_memcrypt_enabled();
> +
> +if (mr->encrypted) {
> +ram_block_encrypted_notify_add(block->host, block->max_length);
> +}
> +}
> +
> +bool memory_region_is_encrypted(MemoryRegion *mr)
> +{
> +return m

Re: [Qemu-devel] [PATCH 3/6] Add vhost-user-backend

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 12:27 PM Marc-André Lureau
 wrote:
>
> Create a vhost-user-backend object that holds a connection to a
> vhost-user backend (or "slave" process) and can be referenced from
> virtio devices that support it. See later patches for input & gpu
> usage.
>
> Note: a previous iteration of this object made it user-creatable, and
> allowed managed sub-process spawning, but that has been dropped for
> now.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  include/sysemu/vhost-user-backend.h |  57 
>  backends/vhost-user.c   | 209 
>  MAINTAINERS |   2 +
>  backends/Makefile.objs  |   2 +
>  4 files changed, 270 insertions(+)
>  create mode 100644 include/sysemu/vhost-user-backend.h
>  create mode 100644 backends/vhost-user.c
>
> diff --git a/include/sysemu/vhost-user-backend.h 
> b/include/sysemu/vhost-user-backend.h
> new file mode 100644
> index 00..9abf8f06a1
> --- /dev/null
> +++ b/include/sysemu/vhost-user-backend.h
> @@ -0,0 +1,57 @@
> +/*
> + * QEMU vhost-user backend
> + *
> + * Copyright (C) 2018 Red Hat Inc
> + *
> + * Authors:
> + *  Marc-André Lureau 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef QEMU_VHOST_USER_BACKEND_H
> +#define QEMU_VHOST_USER_BACKEND_H
> +
> +#include "qom/object.h"
> +#include "exec/memory.h"
> +#include "qemu/option.h"
> +#include "qemu/bitmap.h"
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-user.h"
> +#include "chardev/char-fe.h"
> +#include "io/channel.h"
> +
> +#define TYPE_VHOST_USER_BACKEND "vhost-user-backend"
> +#define VHOST_USER_BACKEND(obj) \
> +OBJECT_CHECK(VhostUserBackend, (obj), TYPE_VHOST_USER_BACKEND)
> +#define VHOST_USER_BACKEND_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(VhostUserBackendClass, (obj), TYPE_VHOST_USER_BACKEND)
> +#define VHOST_USER_BACKEND_CLASS(klass) \
> +OBJECT_CLASS_CHECK(VhostUserBackendClass, (klass), 
> TYPE_VHOST_USER_BACKEND)
> +
> +typedef struct VhostUserBackend VhostUserBackend;
> +typedef struct VhostUserBackendClass VhostUserBackendClass;
> +
> +struct VhostUserBackendClass {
> +ObjectClass parent_class;
> +};
> +
> +struct VhostUserBackend {
> +/* private */
> +Object parent;
> +
> +char *chr_name;
> +CharBackend chr;
> +VhostUserState vhost_user;
> +struct vhost_dev dev;
> +VirtIODevice *vdev;
> +bool started;
> +bool completed;
> +};
> +
> +int vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
> +unsigned nvqs, Error **errp);
> +void vhost_user_backend_start(VhostUserBackend *b);
> +void vhost_user_backend_stop(VhostUserBackend *b);
> +
> +#endif
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> new file mode 100644
> index 00..2b055544a7
> --- /dev/null
> +++ b/backends/vhost-user.c
> @@ -0,0 +1,209 @@
> +/*
> + * QEMU vhost-user backend
> + *
> + * Copyright (C) 2018 Red Hat Inc
> + *
> + * Authors:
> + *  Marc-André Lureau 
> + *
> + * 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 "hw/qdev.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/error-report.h"
> +#include "qom/object_interfaces.h"
> +#include "sysemu/vhost-user-backend.h"
> +#include "sysemu/kvm.h"
> +#include "io/channel-command.h"
> +#include "hw/virtio/virtio-bus.h"
> +
> +static bool
> +ioeventfd_enabled(void)
> +{
> +return kvm_enabled() && kvm_eventfds_enabled();
> +}
> +
> +int
> +vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
> +unsigned nvqs, Error **errp)
> +{
> +int ret;
> +
> +assert(!b->vdev && vdev);
> +
> +if (!ioeventfd_enabled()) {
> +error_setg(errp, "vhost initialization failed: requires kvm");
> +return -1;
> +}
> +
> +if (!vhost_user_init(&b->vhost_user, &b->chr, errp)) {
> +return -1;
> +}
> +
> +b->vdev = vdev;
> +b->dev.nvqs = nvqs;
> +b->dev.vqs = g_new(struct vhost_virtqueue, nvqs);
> +
> +ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 
> 0);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "vhost initialization failed");
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +void
> +vhost_user_backend_start(VhostUserBackend *b)
> +{
> +BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(b->vdev)));
> +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +int ret, i ;
> +
> +if (b->started) {
> +return;
> +}
> +
> +if (!k->set_guest_notifiers) {
> +error_report("binding does not support guest notifiers");
> +return;
> +}
> +
> +ret = vhost_dev_enable_notifiers(&b->dev, b->vdev);
> +if (ret < 0) {
> +r

Re: [Qemu-devel] About pt-br keyboard layout

[Lin Ma]

On 4/26/19 7:48 PM, Gerd Hoffmann wrote:

So -k must match the *hosts* keyboard layout.

So, that means, If I launch qemu with -k pt-br on a host, I need to ensure
the keyboard layout
must be pt-br as well onthathypervisorhost,Then when I connected to this vnc
server(qemu)
throughvncviewer on mylaptop(us keyboard layout), I type shift + 6, I can
get¨(diaeresis)
character, am I right?

No.  I've simplified a bit, assuming both vncviewer and qemu run on the
host machine.  -k $layout must match the keyboard layout of the machine
running vncviewer.  So, in your case it should be us (the laptops
layout).

:-)  Thank you very much!
Lin

Re: [Qemu-devel] [PATCH v2 10/10] s390x/cpumodel: wire up 8561 and 8562 as gen15 machines

[Cornelia Huck]

On Fri, 26 Apr 2019 14:05:30 +0200
David Hildenbrand  wrote:

> On 26.04.19 14:01, Christian Borntraeger wrote:
> > 
> > 
> > On 26.04.19 13:52, David Hildenbrand wrote:  
> >> On 26.04.19 13:36, Christian Borntraeger wrote:  
> >>>
> >>>
> >>> On 26.04.19 13:32, David Hildenbrand wrote:  
>  On 26.04.19 13:10, Christian Borntraeger wrote:  
> > 8561 and 8562 will be gen15 machines. There is no name yet, lets us use
> > the cpu id as base name. Later on we can provide aliases with the proper
> > name.
> >
> > Signed-off-by: Christian Borntraeger 
> > ---
> >  target/s390x/cpu_models.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> > index d683635eb5..dd6415103f 100644
> > --- a/target/s390x/cpu_models.c
> > +++ b/target/s390x/cpu_models.c
> > @@ -83,6 +83,8 @@ static S390CPUDef s390_cpu_defs[] = {
> >  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
> >  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 
> > GA2"),
> >  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 
> > Model ZR1 GA1"),
> > +CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "8561", "IBM 8561 
> > GA1"),
> > +CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "8562", "IBM 8562 
> > GA1"),
> >  };  
> 
>  Thinking out loud, I know that official names are not published yet.
>  Looking at the recent history (z13, z14), my educated guess would be
>  z15. I guess pretty much everybody would guess that.  
> >>>
> >>> Not sure about trademark aspects - especially if this really becomes z15. 
> >>> The smaller
> >>> machine has no real history (ZR1 vs. s vs BC). So I think I would rather 
> >>> have a correct
> >>> number than a partially correct name.  
> >>
> >> We could also use "gen15a" and "gen15b", still better to get than magic
> >> numbers. (yeah well, they are not magic)
> >>
> >> If you want to stick with numbers, be aware that cpu numbers are not
> >> injective, so at some point we will need e.g. "8561.2", just so you're
> >> aware of the implications.  
> > 
> > I think whatever we have here is only used internally for expansion 
> > (host-model)
> > and the user will use later the real name when available. (custom). So 
> > probably this
> > does not matter for a long time. But I might be wrong.
> > I tend to prefer gen15 over z15 but 856x has also its charm.

FWIW, I'd prefer gen15 over 856x, but...

> >   
> 
> Another question would be, can we rename before the next QEMU release,
> so it will never be officially part of QEMU? Then we don't need aliases
> after all. Of course, distros have to take care of that as well, but
> that shouldn't be upstream QEMUs business.

...if we could do that, I'd like that even better.

[Qemu-devel] [Bug 1826422] Re: Regression: QEMU 4.0 hangs the host (*bisect included*)

[Alex Williamson]

The change in QEMU 4.0 is only a change in defaults of the machine type,
it can be entirely reverted in the VM config with kernel_irqchip=on or
 with libvirt.  Using a machine type prior to the
q35 4.0 machine type would also avoid it.  There are no performance
issues with these configurations that would favor using 3.1 over 4.0.

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

Title:
  Regression: QEMU 4.0 hangs the host (*bisect included*)

Status in QEMU:
  New

Bug description:
  The commit b2fc91db84470a78f8e93f5b5f913c17188792c8 seemingly
  introduced a regression on my system.

  When I start QEMU, the guest and the host hang (I need a hard reset to
  get back to a working system), before anything shows on the guest.

  I use QEMU with GPU passthrough (which worked perfectly until the
  commit above). This is the command I use:

  ```
  /path/to/qemu-system-x86_64
-drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
-drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
-enable-kvm
-machine q35,accel=kvm,mem-merge=off
-cpu 
host,kvm=off,hv_vendor_id=vgaptrocks,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time
-smp 4,cores=4,sockets=1,threads=1
-m 10240
-vga none
-rtc base=localtime
-serial none
-parallel none
-usb
-device usb-tablet
-device vfio-pci,host=01:00.0,multifunction=on
-device vfio-pci,host=01:00.1
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device usb-host,vendorid=,productid=
-device virtio-scsi-pci,id=scsi
-drive file=/path/to/guest.img,id=hdd1,format=qcow2,if=none,cache=writeback
-device scsi-hd,drive=hdd1
-net nic,model=virtio
-net user,smb=/path/to/shared
  ```

  If I run QEMU without GPU passthrough, it runs fine.

  Some details about my system:

  - O/S: Mint 19.1 x86-64 (it's based on Ubuntu 18.04)
  - Kernel: 4.15
  - `configure` options: `--target-list=x86_64-softmmu --enable-gtk 
--enable-spice --audio-drv-list=pa`
  - EDK2 version: 1a734ed85fda71630c795832e6d24ea560caf739 (20/Apr/2019)
  - CPU: i7-6700k
  - Motherboard: ASRock Z170 Gaming-ITX/ac
  - VGA: Gigabyte GTX 960 Mini-ITX

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

Re: [Qemu-devel] [PULL 00/19] first batch of s390x patches for 4.1

[Peter Maydell]

On Thu, 25 Apr 2019 at 14:21, Cornelia Huck  wrote:
>
> The following changes since commit 3284aa128153750f14a61e8a96fd085e6f2999b6:
>
>   Merge remote-tracking branch 'remotes/lersek/tags/edk2-pull-2019-04-22' 
> into staging (2019-04-24 13:19:41 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cohuck/qemu tags/s390x-20190425
>
> for you to fetch changes up to 41c3d4269b1c18d12d33c5bf089dace25d08e82e:
>
>   Merge tag 's390-ccw-bios-2019-04-12' into s390-next-staging (2019-04-25 
> 14:09:20 +0200)
>
> 
> - properly detect page size of initial memory
> - support for IPL (boot) from ECKD DASD passed through via vfio-ccw
>
> 
>


Applied, thanks.

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

-- PMM

Re: [Qemu-devel] [PATCH v6 08/11] contrib: add vhost-user-gpu

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 10:03 AM Gerd Hoffmann  wrote:
>
>   Hi,
>
> > +#ifdef CONFIG_LIBDRM_INTEL
> > +static bool
> > +intel_alloc_bo(struct drm_buffer *buf)
> > +{
> > +uint32_t tiling = I915_TILING_NONE;
> > +
> > +buf->intel_bo = drm_intel_bo_alloc_tiled(buf->dev->bufmgr, 
> > "vhost-user-gpu",
> > + buf->width, buf->height,
> > + (buf->bpp / 8), &tiling,
> > + &buf->stride, 0);
>
> Why do you go for intel specific code here?
>

No idea, most likely some functions were lacking when I looked at it.
GBM is fine now.

> Can't you do the same with gbm_bo_create() or
> gbm_bo_create_with_modifiers() ?

It's not clear to me how to properly use
gbm_bo_create_with_modifiers(). I went with gbm_bo_create() for now,
it "works for me".

>
> > +static bool
> > +intel_map_bo(struct drm_buffer *buf)
> > +{
> > +if (drm_intel_gem_bo_map_gtt(buf->intel_bo) != 0) {
> > +return false;
> > +}
>
> gbm_bo_map()
>
> > +static bool
> > +intel_export_bo_to_prime(struct drm_buffer *buffer, int *fd)
> > +{
> > +if (drm_intel_bo_gem_export_to_prime(buffer->intel_bo, fd) < 0) {
> > +return false;
> > +}
>
> gbm_bo_get_fd()
>
> > +get_pixman_format(uint32_t virtio_gpu_format)
> > +{
> > +switch (virtio_gpu_format) {
> > +#ifdef HOST_WORDS_BIGENDIAN
> > +case VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM:
> > +return PIXMAN_b8g8r8x8;
>
> Move that to a header for sharing?  Simliar to the bswap helpers?

ack

>
> > +static void
> > +virgl_cmd_create_resource_3d(VuGpu *g,
> > + struct virtio_gpu_ctrl_command *cmd)
> > +{
> > +struct virtio_gpu_resource_create_3d c3d;
> > +struct virgl_renderer_resource_create_args args;
> > +
> > +VUGPU_FILL_CMD(c3d);
> > +
> > +args.handle = c3d.resource_id;
> > +args.target = c3d.target;
> > +args.format = c3d.format;
> > +args.bind = c3d.bind;
> > +args.width = c3d.width;
> > +args.height = c3d.height;
> > +args.depth = c3d.depth;
> > +args.array_size = c3d.array_size;
> > +args.last_level = c3d.last_level;
> > +args.nr_samples = c3d.nr_samples;
> > +args.flags = c3d.flags;
> > +virgl_renderer_resource_create(&args, NULL, 0);
> > +}
>
> The virtio_gpu_resource_create_3d -> virgl_renderer_resource_create_args
> mapping looks like an opportunity for code sharing too.
>
> Hmm, but virgl_renderer_resource_create seems to be the only call with
> an args struct, so maybe not worth the effort for this single case.

yeah, I will not bother for now

thanks!

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-4.1] block: Assert that drv->bdrv_child_perm is set in bdrv_child_perm()

[Alberto Garcia]

ping

On Thu 04 Apr 2019 01:29:53 PM CEST, Alberto Garcia wrote:
> There is no need to check for this because all block drivers that have
> children implement bdrv_child_perm and all callers already ensure that
> bs->drv is set.
>
> Furthermore, if this check would fail then the callers would end up
> with uninitialized values for nperm and nshared.
>
> This patch replaces the check with an assertion.
>
> Signed-off-by: Alberto Garcia 
> ---
>  block.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 3050854528..5f92565692 100644
> --- a/block.c
> +++ b/block.c
> @@ -1742,11 +1742,10 @@ static void bdrv_child_perm(BlockDriverState *bs, 
> BlockDriverState *child_bs,
>  uint64_t parent_perm, uint64_t parent_shared,
>  uint64_t *nperm, uint64_t *nshared)
>  {
> -if (bs->drv && bs->drv->bdrv_child_perm) {
> -bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
> - parent_perm, parent_shared,
> - nperm, nshared);
> -}
> +assert(bs->drv && bs->drv->bdrv_child_perm);
> +bs->drv->bdrv_child_perm(bs, c, role, reopen_queue,
> + parent_perm, parent_shared,
> + nperm, nshared);
>  /* TODO Take force_share from reopen_queue */
>  if (child_bs && child_bs->force_share) {
>  *nshared = BLK_PERM_ALL;
> -- 
> 2.11.0

Re: [Qemu-devel] [PATCH for-4.1] commit: Use bdrv_append() in commit_start()

[Alberto Garcia]

ping

On Wed 03 Apr 2019 04:37:48 PM CEST, Alberto Garcia wrote:
> This function combines bdrv_set_backing_hd() and bdrv_replace_node()
> so we can use it to simplify the code a bit in commit_start().
>
> Signed-off-by: Alberto Garcia 
> ---
>  block/commit.c | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/block/commit.c b/block/commit.c
> index ba60fef58a..a0beb7d265 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -304,23 +304,14 @@ void commit_start(const char *job_id, BlockDriverState 
> *bs,
>  commit_top_bs->total_sectors = top->total_sectors;
>  bdrv_set_aio_context(commit_top_bs, bdrv_get_aio_context(top));
>  
> -bdrv_set_backing_hd(commit_top_bs, top, &local_err);
> +bdrv_append(commit_top_bs, top, &local_err);
>  if (local_err) {
> -bdrv_unref(commit_top_bs);
> -commit_top_bs = NULL;
> -error_propagate(errp, local_err);
> -goto fail;
> -}
> -bdrv_replace_node(top, commit_top_bs, &local_err);
> -if (local_err) {
> -bdrv_unref(commit_top_bs);
>  commit_top_bs = NULL;
>  error_propagate(errp, local_err);
>  goto fail;
>  }
>  
>  s->commit_top_bs = commit_top_bs;
> -bdrv_unref(commit_top_bs);
>  
>  /* Block all nodes between top and base, because they will
>   * disappear from the chain after this operation. */
> -- 
> 2.11.0

Re: [Qemu-devel] [PATCH for-4.1 v2 0/2] commit: Make base read-only if there is an early failure

[Alberto Garcia]

ping

On Thu 11 Apr 2019 02:32:26 PM CEST, Alberto Garcia wrote:
> Hi,
>
> this is the same patch I posted yesterday, but with a test case.
>
> Berto
>
> Alberto Garcia (2):
>   commit: Make base read-only if there is an early failure
>   iotests: Check that images are in read-only mode after block-commit
>
>  block/commit.c |   3 ++
>  tests/qemu-iotests/249 | 115 
> +
>  tests/qemu-iotests/249.out |  35 ++
>  tests/qemu-iotests/group   |   1 +
>  4 files changed, 154 insertions(+)
>  create mode 100755 tests/qemu-iotests/249
>  create mode 100644 tests/qemu-iotests/249.out
>
> -- 
> 2.11.0

Re: [Qemu-devel] [PATCH for-4.1] block: Use bdrv_unref_child() for all children in bdrv_close()

[Alberto Garcia]

ping

On Sun 31 Mar 2019 01:17:47 PM CEST, Alberto Garcia wrote:
> bdrv_unref_child() does the following things:
>
>   - Updates the child->bs->inherits_from pointer.
>   - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
>   - Calls bdrv_unref() to unref the child BlockDriverState.
>
> When bdrv_unref_child() was introduced in commit 33a604075c it was not
> used in bdrv_close() because the drivers that had additional children
> (like quorum or blkverify) had already called bdrv_unref() on their
> children during their own close functions.
>
> This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
> blkverify) so there's no reason not to use bdrv_unref_child() in
> bdrv_close() anymore.
>
> After this there's also no need to remove bs->backing and bs->file
> separately from the rest of the children, so bdrv_close() can be
> simplified.
>
> Signed-off-by: Alberto Garcia 
> ---
>  block.c | 16 +++-
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/block.c b/block.c
> index 0a93ee9ac8..3b7c12d68e 100644
> --- a/block.c
> +++ b/block.c
> @@ -3842,22 +3842,12 @@ static void bdrv_close(BlockDriverState *bs)
>  bs->drv = NULL;
>  }
>  
> -bdrv_set_backing_hd(bs, NULL, &error_abort);
> -
> -if (bs->file != NULL) {
> -bdrv_unref_child(bs, bs->file);
> -bs->file = NULL;
> -}
> -
>  QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
> -/* TODO Remove bdrv_unref() from drivers' close function and use
> - * bdrv_unref_child() here */
> -if (child->bs->inherits_from == bs) {
> -child->bs->inherits_from = NULL;
> -}
> -bdrv_detach_child(child);
> +bdrv_unref_child(bs, child);
>  }
>  
> +bs->backing = NULL;
> +bs->file = NULL;
>  g_free(bs->opaque);
>  bs->opaque = NULL;
>  atomic_set(&bs->copy_on_read, 0);
> -- 
> 2.11.0

Re: [Qemu-devel] [RFC PATCH v1 01/10] KVM: SVM: Add KVM_SEV SEND_START command

[Borislav Petkov]

On Wed, Apr 24, 2019 at 04:09:59PM +, Singh, Brijesh wrote:
> The command is used to create an outgoing SEV guest encryption context.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Joerg Roedel 
> Cc: Borislav Petkov 
> Cc: Tom Lendacky 
> Cc: x...@kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Brijesh Singh 
> ---
>  .../virtual/kvm/amd-memory-encryption.rst |  24 +
>  arch/x86/kvm/svm.c| 101 ++
>  include/uapi/linux/kvm.h  |  12 +++
>  3 files changed, 137 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/amd-memory-encryption.rst 
> b/Documentation/virtual/kvm/amd-memory-encryption.rst
> index 659bbc093b52..340ac4f87321 100644
> --- a/Documentation/virtual/kvm/amd-memory-encryption.rst
> +++ b/Documentation/virtual/kvm/amd-memory-encryption.rst
> @@ -238,6 +238,30 @@ Returns: 0 on success, -negative on error
>  __u32 trans_len;
>  };
>  
> +10. KVM_SEV_SEND_START
> +--
> +
> +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
> +outgoing guest encryption context.
> +
> +Parameters (in): struct kvm_sev_send_start
> +
> +Returns: 0 on success, -negative on error
> +
> +::
> +struct kvm_sev_send_start {
> +__u32 policy; /* guest policy */
> +
> +__u64 pdh_cert_uaddr; /* platform Diffie-Hellman 
> certificate */
> +__u32 pdh_cert_len;
> +
> +__u64 plat_cert_uaddr;/* platform certificate chain 
> */
> +__u32 plat_cert_len;
> +
> +__u64 amd_cert_uaddr; /* AMD certificate */
> +__u32 amd_cert_len;

__u64 session_uaddr;
__u32 session_len;

too, right?

> +};
> +
>  References
>  ==
>  
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 406b558abfef..4c2a225ba546 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -6955,6 +6955,104 @@ static int sev_launch_secret(struct kvm *kvm, struct 
> kvm_sev_cmd *argp)
>   return ret;
>  }
>  
> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
> + void *amd_cert = NULL, *session_data = NULL;
> + void *pdh_cert = NULL, *plat_cert = NULL;
> + struct sev_data_send_start *data = NULL;
> + struct kvm_sev_send_start params;
> + int ret;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
> + sizeof(struct kvm_sev_send_start)))
> + return -EFAULT;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + /* userspace wants to query the session length */
> + if (!params.session_len)
> + goto cmd;
> +
> + if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
> + !params.session_uaddr)
> + return -EINVAL;
> +
> + /* copy the certificate blobs from userspace */
> + pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, 
> params.pdh_cert_len);
> + if (IS_ERR(pdh_cert)) {
> + ret = PTR_ERR(pdh_cert);
> + goto e_free;
> + }
> +
> + data->pdh_cert_address = __psp_pa(pdh_cert);
> + data->pdh_cert_len = params.pdh_cert_len;
> +
> + plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, 
> params.plat_cert_len);
> + if (IS_ERR(plat_cert)) {
> + ret = PTR_ERR(plat_cert);
> + goto e_free_pdh;
> + }
> +
> + data->plat_cert_address = __psp_pa(plat_cert);
> + data->plat_cert_len = params.plat_cert_len;
> +
> + amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, 
> params.amd_cert_len);
> + if (IS_ERR(amd_cert)) {
> + ret = PTR_ERR(amd_cert);
> + goto e_free_plat_cert;
> + }
> +
> + data->amd_cert_address = __psp_pa(amd_cert);
> + data->amd_cert_len = params.amd_cert_len;
> +
> + ret = -ENOMEM;
> + session_data = kmalloc(params.session_len, GFP_KERNEL);

If the user is supposed to query the session length first, you could
save it in a global variable perhaps and use that value instead of
trusting the user to give you the correct one in params.session_len for
the allocation...

> + if (!session_data)
> + goto e_free_amd_cert;
> +
> + data->session_address = __psp_pa(session_data);
> + data->session_len = params.session_len;
> +cmd:
> + data->handle = sev->handle;
> + ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, data, &argp->error);
> +
> + /* if we queried the session length, FW responded with expected data */

<--- ... here you have the session length from the fw.

-- 
Regards/Gruss,
Boris.

Good mailing prac

Re: [Qemu-devel] [RFC PATCH v1 01/10] KVM: SVM: Add KVM_SEV SEND_START command

[Singh, Brijesh]

On 4/26/19 9:10 AM, Borislav Petkov wrote:
> On Wed, Apr 24, 2019 at 04:09:59PM +, Singh, Brijesh wrote:
>> The command is used to create an outgoing SEV guest encryption context.
>>
>> Cc: Thomas Gleixner 
>> Cc: Ingo Molnar 
>> Cc: "H. Peter Anvin" 
>> Cc: Paolo Bonzini 
>> Cc: "Radim Krčmář" 
>> Cc: Joerg Roedel 
>> Cc: Borislav Petkov 
>> Cc: Tom Lendacky 
>> Cc: x...@kernel.org
>> Cc: k...@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> Signed-off-by: Brijesh Singh 
>> ---
>>   .../virtual/kvm/amd-memory-encryption.rst |  24 +
>>   arch/x86/kvm/svm.c| 101 ++
>>   include/uapi/linux/kvm.h  |  12 +++
>>   3 files changed, 137 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/amd-memory-encryption.rst 
>> b/Documentation/virtual/kvm/amd-memory-encryption.rst
>> index 659bbc093b52..340ac4f87321 100644
>> --- a/Documentation/virtual/kvm/amd-memory-encryption.rst
>> +++ b/Documentation/virtual/kvm/amd-memory-encryption.rst
>> @@ -238,6 +238,30 @@ Returns: 0 on success, -negative on error
>>   __u32 trans_len;
>>   };
>>   
>> +10. KVM_SEV_SEND_START
>> +--
>> +
>> +The KVM_SEV_SEND_START command can be used by the hypervisor to create an
>> +outgoing guest encryption context.
>> +
>> +Parameters (in): struct kvm_sev_send_start
>> +
>> +Returns: 0 on success, -negative on error
>> +
>> +::
>> +struct kvm_sev_send_start {
>> +__u32 policy; /* guest policy */
>> +
>> +__u64 pdh_cert_uaddr; /* platform Diffie-Hellman 
>> certificate */
>> +__u32 pdh_cert_len;
>> +
>> +__u64 plat_cert_uaddr;/* platform certificate chain 
>> */
>> +__u32 plat_cert_len;
>> +
>> +__u64 amd_cert_uaddr; /* AMD certificate */
>> +__u32 amd_cert_len;
> 
>  __u64 session_uaddr;
>  __u32 session_len;
> 
> too, right?


Ah good catch, I will fix in next rev. thanks


> 
>> +};
>> +
>>   References
>>   ==
>>   
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 406b558abfef..4c2a225ba546 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -6955,6 +6955,104 @@ static int sev_launch_secret(struct kvm *kvm, struct 
>> kvm_sev_cmd *argp)
>>  return ret;
>>   }
>>   
>> +static int sev_send_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>> +{
>> +struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
>> +void *amd_cert = NULL, *session_data = NULL;
>> +void *pdh_cert = NULL, *plat_cert = NULL;
>> +struct sev_data_send_start *data = NULL;
>> +struct kvm_sev_send_start params;
>> +int ret;
>> +
>> +if (!sev_guest(kvm))
>> +return -ENOTTY;
>> +
>> +if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
>> +sizeof(struct kvm_sev_send_start)))
>> +return -EFAULT;
>> +
>> +data = kzalloc(sizeof(*data), GFP_KERNEL);
>> +if (!data)
>> +return -ENOMEM;
>> +
>> +/* userspace wants to query the session length */
>> +if (!params.session_len)
>> +goto cmd;
>> +
>> +if (!params.pdh_cert_uaddr || !params.pdh_cert_len ||
>> +!params.session_uaddr)
>> +return -EINVAL;
>> +
>> +/* copy the certificate blobs from userspace */
>> +pdh_cert = psp_copy_user_blob(params.pdh_cert_uaddr, 
>> params.pdh_cert_len);
>> +if (IS_ERR(pdh_cert)) {
>> +ret = PTR_ERR(pdh_cert);
>> +goto e_free;
>> +}
>> +
>> +data->pdh_cert_address = __psp_pa(pdh_cert);
>> +data->pdh_cert_len = params.pdh_cert_len;
>> +
>> +plat_cert = psp_copy_user_blob(params.plat_cert_uaddr, 
>> params.plat_cert_len);
>> +if (IS_ERR(plat_cert)) {
>> +ret = PTR_ERR(plat_cert);
>> +goto e_free_pdh;
>> +}
>> +
>> +data->plat_cert_address = __psp_pa(plat_cert);
>> +data->plat_cert_len = params.plat_cert_len;
>> +
>> +amd_cert = psp_copy_user_blob(params.amd_cert_uaddr, 
>> params.amd_cert_len);
>> +if (IS_ERR(amd_cert)) {
>> +ret = PTR_ERR(amd_cert);
>> +goto e_free_plat_cert;
>> +}
>> +
>> +data->amd_cert_address = __psp_pa(amd_cert);
>> +data->amd_cert_len = params.amd_cert_len;
>> +
>> +ret = -ENOMEM;
>> +session_data = kmalloc(params.session_len, GFP_KERNEL);
> 
> If the user is supposed to query the session length first, you could
> save it in a global variable perhaps and use that value instead of
> trusting the user to give you the correct one in params.session_len for
> the allocation...
> 

Yes that's doable but I am afraid that caching the value may lead us to
wrong path and also divergence from the SEV API spec. The spec says the
returned length is a minimum length but its possible that caller can
give a bigger buffer and FW will still work with i

Re: [Qemu-devel] Using --enable-kvm fails with WindowsXP guest on an AMD host (works on Intel host)

[balducci]

(apologies if this doesn't get properly threaded into:
http://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg03407.html)

> After upgrading host (Gentoo Linux) a Windows XP guest can't boot anymore.
>
> Some findings while testing:
>
> * XP image boots & works OK on an Intel machine (Gentoo Linux)
> ** qemu: 3.1.0-r4
> ** kernel: 5.0.7
> ** CPU: Intel(R) Core(TM) i7-6500U CPU @ 2.50GHz
>
> * XP image FAILS to boot on an AMD machine (Gentoo Linux)
> ** qemu: 3.1.0-r4
> ** kernel: 4.19.34
> ** CPU: AMD FX-6300 Six-Core Processor
> ** image boots OK if NOT using --enable-kvm
>
> * XP image FAILS to boot on an AMD machine (Gentoo Linux)
> ** qemu: 3.1.0-r4 & 3.1.0-r1
> ** kernel: 4.19.34 & 5.0.7
> ** CPU: AMD Opteron(tm) Processor 4170 HE
> ** image boots OK if NOT using --enable-kvm
> ** The image worked OK on this machine with Qemu 2.12.1 & Kernel 4.18.20

Same here, on an AMD Athlon(tm) X4 860K Quad Core Processor

AFAICS, something has gone wrong starting with kernel-5.0.6

For me:
GOOD  BAD
4.20.14   5.0.6
5.0.2 5.0.7
5.0.3
5.0.4
5.0.5

Full details available in my earlier report here
https://bugzilla.kernel.org/show_bug.cgi?id=203327

I've updated my original report with a reference to this one.

ciao
-gabriele

[Qemu-devel] [Bug 1826568] [NEW] RISC-V Disassembler/translator instruction decoding disagreement

[Floyd42]

Public bug reported:


When running QEMU V3.1.0 for platform  RISC-V, 64bit, Spike V1.10 with "-d 
in_asm -singlestep -D qemu_log.txt", my (faulty) test code brought up this 
message in the logs:

  0x8002cade:  05139517e2bf  illegal 
  Disassembler disagrees with translator over instruction decoding
  Please report this to qemu-devel@nongnu.org


You may want to resolve the disagreement.

Axel

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

Title:
  RISC-V Disassembler/translator instruction decoding disagreement

Status in QEMU:
  New

Bug description:
  
  When running QEMU V3.1.0 for platform  RISC-V, 64bit, Spike V1.10 with "-d 
in_asm -singlestep -D qemu_log.txt", my (faulty) test code brought up this 
message in the logs:

0x8002cade:  05139517e2bf  illegal 
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org

  
  You may want to resolve the disagreement.

  Axel

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

Re: [Qemu-devel] [PULL 00/11] Machine queue, 2019-04-25

[Peter Maydell]

On Thu, 25 Apr 2019 at 18:57, Eduardo Habkost  wrote:
>
> The following changes since commit 3284aa128153750f14a61e8a96fd085e6f2999b6:
>
>   Merge remote-tracking branch 'remotes/lersek/tags/edk2-pull-2019-04-22' 
> into staging (2019-04-24 13:19:41 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
>
> for you to fetch changes up to 119906afa5ca610adb87c55ab0d8e53c9104bfc3:
>
>   util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() (2019-04-25 14:17:36 
> -0300)
>
> 
> Machine queue, 2019-04-25
>
> * 4.1 machine-types (Cornelia Huck)
> * Support MAP_SYNC on pmem memory backends (Zhang Yi)
> * -cpu parsing fixes and cleanups (Eduardo Habkost)
> * machine initialization cleanups (Wei Yang, Markus Armbruster)
>
> 



Applied, thanks.

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

-- PMM

Re: [Qemu-devel] [PATCH 1/3] ram-encrypted-notifier: Introduce a RAM block encrypted notifier

[Janakarajan Natarajan]

On 4/26/19 7:29 AM, Igor Mammedov wrote:
> On Thu, 25 Apr 2019 22:58:18 +
> "Natarajan, Janakarajan"  wrote:
>
>> A client can register to this notifier to know whether the newly added or
>> removed memory region is marked as encrypted. This information is needed
>> for the SEV guest launch. In SEV guest, some memory regions may contain
>> encrypted data (e.g guest RAM). The memory region which contains the
>> encrypted data should be registered/unregistered using the
>> KVM_MEMORY_{REG,UNREG}_ENCRYPTED ioctl.
>>
>> Signed-off-by: Janakarajan Natarajan 
>> ---
>>   exec.c |  1 +
>>   include/exec/memory.h  | 18 ++
>>   include/exec/ramlist.h | 19 +++
>>   memory.c   | 16 
>>   numa.c | 33 +
>>   stubs/ram-block.c  |  8 
>>   6 files changed, 95 insertions(+)
>>
>> diff --git a/exec.c b/exec.c
>> index 2646207661..a02c394e48 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -79,6 +79,7 @@
>>* are protected by the ramlist lock.
>>*/
>>   RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
>> +RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
>>   
>>   static MemoryRegion *system_memory;
>>   static MemoryRegion *system_io;
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 9144a47f57..ae720ff511 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -374,6 +374,7 @@ struct MemoryRegion {
>>   bool terminates;
>>   bool ram_device;
>>   bool enabled;
>> +bool encrypted;
>>   bool warning_printed; /* For reservations */
>>   uint8_t vga_logging_count;
>>   MemoryRegion *alias;
>> @@ -1131,6 +1132,23 @@ int 
>> memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
>>*/
>>   int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
>>   
>> +/**
>> + * memory_region_mark_encrypted: marks the memory region as encrypted and
>> + * lets the listeners of encrypted ram know that a memory region containing
>> + * encrypted ram block has been added
>> + *
>> + * @mr: the memory region
>> + */
>> +void memory_region_mark_encrypted(MemoryRegion *mr);
>> +
>> +/**
>> + * memory_region_is_encrypted: returns if the memory region was marked as
>> + * encrypted when it was created
>> + *
>> + * @mr: the memory region
>> + */
>> +bool memory_region_is_encrypted(MemoryRegion *mr);
>> +
>>   /**
>>* memory_region_name: get a memory region's name
>>*
>> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
>> index bc4faa1b00..5349f27fa5 100644
>> --- a/include/exec/ramlist.h
>> +++ b/include/exec/ramlist.h
>> @@ -7,6 +7,7 @@
>>   #include "qemu/rcu_queue.h"
>>   
>>   typedef struct RAMBlockNotifier RAMBlockNotifier;
>> +typedef struct RAMBlockEncryptedNotifier RAMBlockEncryptedNotifier;
>>   
>>   #define DIRTY_MEMORY_VGA   0
>>   #define DIRTY_MEMORY_CODE  1
>> @@ -55,6 +56,11 @@ typedef struct RAMList {
>>   } RAMList;
>>   extern RAMList ram_list;
>>   
>> +typedef struct RAMBlockEncryptedNotifierList {
>> +QLIST_HEAD(, RAMBlockEncryptedNotifier) ram_block_notifiers;
>> +} RAMBlockEncryptedNotifierList;
>> +extern RAMBlockEncryptedNotifierList ram_block_encrypted_notifier_list;
>> +
>>   /* Should be holding either ram_list.mutex, or the RCU lock. */
>>   #define  INTERNAL_RAMBLOCK_FOREACH(block)  \
>>   QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
>> @@ -70,6 +76,14 @@ struct RAMBlockNotifier {
>>   QLIST_ENTRY(RAMBlockNotifier) next;
>>   };
>>   
>> +struct RAMBlockEncryptedNotifier {
>> +void (*ram_block_encrypted_added)(RAMBlockEncryptedNotifier *n,
>> +  void *host, size_t size);
>> +void (*ram_block_encrypted_removed)(RAMBlockEncryptedNotifier *n,
>> +void *host, size_t size);
>> +QLIST_ENTRY(RAMBlockEncryptedNotifier) next;
>> +};
>> +
>>   void ram_block_notifier_add(RAMBlockNotifier *n);
>>   void ram_block_notifier_remove(RAMBlockNotifier *n);
>>   void ram_block_notify_add(void *host, size_t size);
>> @@ -77,4 +91,9 @@ void ram_block_notify_remove(void *host, size_t size);
>>   
>>   void ram_block_dump(Monitor *mon);
>>   
>> +void ram_block_encrypted_notifier_add(RAMBlockEncryptedNotifier *n);
>> +void ram_block_encrypted_notifier_remove(RAMBlockEncryptedNotifier *n);
>> +void ram_block_encrypted_notify_add(void *host, size_t size);
>> +void ram_block_encrypted_notify_remove(void *host, size_t size);
>> +
>>   #endif /* RAMLIST_H */
>> diff --git a/memory.c b/memory.c
>> index bb2b71ee38..eca02d369b 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -2009,6 +2009,22 @@ int memory_region_iommu_num_indexes(IOMMUMemoryRegion 
>> *iommu_mr)
>>   return imrc->num_indexes(iommu_mr);
>>   }
>>   
>> +void memory_region_mark_encrypted(MemoryRegion *mr)
>> +{
>> +RAMBlock *block = mr->ram_block;
>> +
>> +mr->encrypted =

[Qemu-devel] [PATCH v4 00/10] block/pflash_cfi02: Implement missing AMD pflash functionality

[Stephen Checkoway]

The goal of this patch series implement the following AMD command-set parallel
flash functionality:
- flash interleaving;
- nonuniform sector sizes;
- erase suspend/resume commands; and
- multi-sector erase.

During refactoring and implementation, I discovered several bugs that are
fixed here as well:
- flash commands use only 11-bits of the address in most cases, but the
  current code uses all of them [1];
- entering CFI mode from autoselect mode and then exiting CFI mode should
  return the chip to autoselect mode, but the current code returns to read
  array mode; and
- reset command should be ignored during sector/chip erase, but the current
  code performs the reset.

The first patch in the series adds a test for the existing behavior. Tests for
additional behavior/bug fixes are added in the relevant patch.

1. I found firmware in the wild that relies on the 11-bit address behavior,
   probably due to a bug in the firmware itself.

Changes from v1:
- Fix missing spaces around *, -, and ?;
- Fix missing Signed-off-by line on patch 7; and
- Replace use of errc with g_printerr and exit.

Changes from v2:
- Remove global_qtest from tests; and
- Test the CFI table changes.

Changes from v3:
- Replace err.h/err() with glib functions; and
- Reformat qtest_initf lines.

Stephen Checkoway (10):
  block/pflash_cfi02: Add test for supported commands
  block/pflash_cfi02: Refactor, NFC intended
  block/pflash_cfi02: Fix command address comparison
  block/pflash_cfi02: Implement intereleaved flash devices
  block/pflash_cfi02: Implement nonuniform sector sizes
  block/pflash_cfi02: Fix CFI in autoselect mode
  block/pflash_cfi02: Fix reset command not ignored during erase
  block/pflash_cfi02: Implement multi-sector erase
  block/pflash_cfi02: Implement erase suspend/resume
  block/pflash_cfi02: Use the chip erase time specified in the CFI table

 hw/block/pflash_cfi02.c   | 843 +++---
 tests/Makefile.include|   2 +
 tests/pflash-cfi02-test.c | 812 
 3 files changed, 1420 insertions(+), 237 deletions(-)
 create mode 100644 tests/pflash-cfi02-test.c

-- 
2.20.1 (Apple Git-117)

[Qemu-devel] [PATCH v4 01/10] block/pflash_cfi02: Add test for supported commands

[Stephen Checkoway]

Test the AMD command set for parallel flash chips. This test uses an
ARM musicpal board with a pflash drive to test the following list of
currently-supported commands.
- Autoselect
- CFI
- Sector erase
- Chip erase
- Program
- Unlock bypass
- Reset

Signed-off-by: Stephen Checkoway 
---
 tests/Makefile.include|   2 +
 tests/pflash-cfi02-test.c | 225 ++
 2 files changed, 227 insertions(+)
 create mode 100644 tests/pflash-cfi02-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 36fc73fef5..dbdb2c0082 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -263,6 +263,7 @@ check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
 check-qtest-arm-y += tests/hexloader-test$(EXESUF)
+check-qtest-arm-$(CONFIG_PFLASH_CFI02) += tests/pflash-cfi02-test$(EXESUF)
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
@@ -773,6 +774,7 @@ tests/device-introspect-test$(EXESUF): 
tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
 tests/hexloader-test$(EXESUF): tests/hexloader-test.o
+tests/pflash-cfi02$(EXESUF): tests/pflash-cfi02-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
 tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
new file mode 100644
index 00..40af1bb523
--- /dev/null
+++ b/tests/pflash-cfi02-test.c
@@ -0,0 +1,225 @@
+/*
+ * QTest testcase for parallel flash with AMD command set
+ *
+ * Copyright (c) 2019 Stephen Checkoway
+ *
+ * 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 "libqtest.h"
+
+/*
+ * To test the pflash_cfi02 device, we run QEMU with the musicpal machine with
+ * a pflash drive. This enables us to test some flash configurations, but not
+ * all. In particular, we're limited to a 16-bit wide flash device.
+ */
+
+#define MP_FLASH_SIZE_MAX (32 * 1024 * 1024)
+#define BASE_ADDR (0x1ULL - MP_FLASH_SIZE_MAX)
+
+#define FLASH_WIDTH 2
+#define CFI_ADDR (FLASH_WIDTH * 0x55)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+
+#define CFI_CMD 0x98
+#define UNLOCK0_CMD 0xAA
+#define UNLOCK1_CMD 0x55
+#define AUTOSELECT_CMD 0x90
+#define RESET_CMD 0xF0
+#define PROGRAM_CMD 0xA0
+#define SECTOR_ERASE_CMD 0x30
+#define CHIP_ERASE_CMD 0x10
+#define UNLOCK_BYPASS_CMD 0x20
+#define UNLOCK_BYPASS_RESET_CMD 0x00
+
+static char image_path[] = "/tmp/qtest.XX";
+
+static inline void flash_write(uint64_t byte_addr, uint16_t data)
+{
+qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
+}
+
+static inline uint16_t flash_read(uint64_t byte_addr)
+{
+return qtest_readw(global_qtest, BASE_ADDR + byte_addr);
+}
+
+static void unlock(void)
+{
+flash_write(UNLOCK0_ADDR, UNLOCK0_CMD);
+flash_write(UNLOCK1_ADDR, UNLOCK1_CMD);
+}
+
+static void reset(void)
+{
+flash_write(0, RESET_CMD);
+}
+
+static void sector_erase(uint64_t byte_addr)
+{
+unlock();
+flash_write(UNLOCK0_ADDR, 0x80);
+unlock();
+flash_write(byte_addr, SECTOR_ERASE_CMD);
+}
+
+static void wait_for_completion(uint64_t byte_addr)
+{
+/* If DQ6 is toggling, step the clock and ensure the toggle stops. */
+if ((flash_read(byte_addr) & 0x40) ^ (flash_read(byte_addr) & 0x40)) {
+/* Wait for erase or program to finish. */
+clock_step_next();
+/* Ensure that DQ6 has stopped toggling. */
+g_assert_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+}
+}
+
+static void bypass_program(uint64_t byte_addr, uint16_t data)
+{
+flash_write(UNLOCK0_ADDR, PROGRAM_CMD);
+flash_write(byte_addr, data);
+/*
+ * Data isn't valid until DQ6 stops toggling. We don't model this as
+ * writes are immediate, but if this changes in the future, we can wait
+ * until the program is complete.
+ */
+wait_for_completion(byte_addr);
+}
+
+static void program(uint64_t byte_addr, uint16_t data)
+{
+unlock();
+bypass_program(byte_addr, data);
+}
+
+static void chip_erase(void)
+{
+unlock();
+flash_write(UNLOCK0_ADDR, 0x80);
+unlock();
+flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
+}
+
+static void test_flash(void)
+{
+global_qtest = qtest_initf("-M musicpal,accel=qtest "
+   "-drive 
if=pflash,file=%s,format=raw,copy-on-read",
+   image_path);
+/* Check the IDs. */
+unlock();
+flash_write(UNLOCK0_ADDR, AUTOSELECT_CMD);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x), ==, 0x00BF);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), =

[Qemu-devel] [PATCH v4 07/10] block/pflash_cfi02: Fix reset command not ignored during erase

[Stephen Checkoway]

When the flash device is performing a chip erase, all commands are
ignored. When it is performing a sector erase, only the erase suspend
command is valid, which is currently not supported.

In particular, the reset command should not cause the device to reset to
read array mode while programming is on going.

Signed-off-by: Stephen Checkoway 
---
 hw/block/pflash_cfi02.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index be10036886..cb1160eb35 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -325,7 +325,8 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 pfl->bank_width * 2, value);
 }
 
-if (cmd == 0xF0) {
+/* Reset does nothing during chip erase and sector erase. */
+if (cmd == 0xF0 && pfl->cmd != 0x10 && pfl->cmd != 0x30) {
 if (pfl->wcycle == WCYCLE_AUTOSELECT_CFI) {
 /* Return to autoselect mode. */
 pfl->wcycle = 3;
-- 
2.20.1 (Apple Git-117)

[Qemu-devel] [PATCH v4 03/10] block/pflash_cfi02: Fix command address comparison

[Stephen Checkoway]

Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.

Signed-off-by: Stephen Checkoway 
Acked-by: Thomas Huth 
---
 hw/block/pflash_cfi02.c   |  8 +++-
 tests/pflash-cfi02-test.c | 12 ++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 4b7af71806..e4bff0c8f8 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -296,11 +296,13 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 
 DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
 offset, value, width);
-boff = offset & (pfl->sector_len - 1);
+boff = offset;
 if (pfl->width == 2)
 boff = boff >> 1;
 else if (pfl->width == 4)
 boff = boff >> 2;
+/* Only the least-significant 11 bits are used in most cases. */
+boff &= 0x7FF;
 switch (pfl->wcycle) {
 case 0:
 /* Set the device in I/O access mode if required */
@@ -519,6 +521,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+/* Only 11 bits are used in the comparison. */
+pfl->unlock_addr0 &= 0x7FF;
+pfl->unlock_addr1 &= 0x7FF;
+
 chip_len = pfl->sector_len * pfl->nb_blocs;
 
 memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 40af1bb523..ea5f8b2648 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -21,8 +21,8 @@
 
 #define FLASH_WIDTH 2
 #define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
 
 #define CFI_CMD 0x98
 #define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(void)
 g_assert_cmpint(flash_read(6), ==, 0xCDEF);
 g_assert_cmpint(flash_read(8), ==, 0x);
 
+/* Test ignored high order bits of address. */
+flash_write(FLASH_WIDTH * 0x, UNLOCK0_CMD);
+flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD);
+flash_write(FLASH_WIDTH * 0x, AUTOSELECT_CMD);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x), ==, 0x00BF);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+reset();
+
 qtest_quit(global_qtest);
 }
 
-- 
2.20.1 (Apple Git-117)

[Qemu-devel] [PATCH v4 02/10] block/pflash_cfi02: Refactor, NFC intended

[Stephen Checkoway]

Simplify and refactor for upcoming commits. In particular, pull out all
of the code to modify the status into simple helper functions. Status
handling becomes more complex once multiple chips are interleaved to
produce a single device.

No change in functionality is intended with this commit.

Signed-off-by: Stephen Checkoway 
---
 hw/block/pflash_cfi02.c | 221 +---
 1 file changed, 95 insertions(+), 126 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f2c6201f81..4b7af71806 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -46,18 +46,19 @@
 #include "hw/sysbus.h"
 #include "trace.h"
 
-//#define PFLASH_DEBUG
-#ifdef PFLASH_DEBUG
+#define PFLASH_DEBUG false
 #define DPRINTF(fmt, ...)  \
 do {   \
-fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__);   \
+if (PFLASH_DEBUG) {\
+fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__);   \
+}  \
 } while (0)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
+/* Special write cycle for CFI queries. */
+#define WCYCLE_CFI 7
+
 struct PFlashCFI02 {
 /*< private >*/
 SysBusDevice parent_obj;
@@ -97,6 +98,31 @@ struct PFlashCFI02 {
 void *storage;
 };
 
+/*
+ * Toggle status bit DQ7.
+ */
+static inline void toggle_dq7(PFlashCFI02 *pfl)
+{
+pfl->status ^= 0x80;
+}
+
+/*
+ * Set status bit DQ7 to bit 7 of value.
+ */
+static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
+{
+pfl->status &= 0x7F;
+pfl->status |= value & 0x80;
+}
+
+/*
+ * Toggle status bit DQ6.
+ */
+static inline void toggle_dq6(PFlashCFI02 *pfl)
+{
+pfl->status ^= 0x40;
+}
+
 /*
  * Set up replicated mappings of the same region.
  */
@@ -126,7 +152,7 @@ static void pflash_timer (void *opaque)
 
 trace_pflash_timer_expired(pfl->cmd);
 /* Reset flash */
-pfl->status ^= 0x80;
+toggle_dq7(pfl);
 if (pfl->bypass) {
 pfl->wcycle = 2;
 } else {
@@ -136,12 +162,34 @@ static void pflash_timer (void *opaque)
 pfl->cmd = 0;
 }
 
-static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
-int width, int be)
+/*
+ * Read data from flash.
+ */
+static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
+ unsigned int width)
 {
+uint8_t *p = (uint8_t *)pfl->storage + offset;
+uint64_t ret = pfl->be ? ldn_be_p(p, width) : ldn_le_p(p, width);
+/* XXX: Need a trace_pflash_data_read(offset, ret, width) */
+switch (width) {
+case 1:
+trace_pflash_data_read8(offset, ret);
+break;
+case 2:
+trace_pflash_data_read16(offset, ret);
+break;
+case 4:
+trace_pflash_data_read32(offset, ret);
+break;
+}
+return ret;
+}
+
+static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
+{
+PFlashCFI02 *pfl = opaque;
 hwaddr boff;
-uint32_t ret;
-uint8_t *p;
+uint64_t ret;
 
 ret = -1;
 trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -166,39 +214,8 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 case 0x80:
 /* We accept reads during second unlock sequence... */
 case 0x00:
-flash_read:
 /* Flash area read */
-p = pfl->storage;
-switch (width) {
-case 1:
-ret = p[offset];
-trace_pflash_data_read8(offset, ret);
-break;
-case 2:
-if (be) {
-ret = p[offset] << 8;
-ret |= p[offset + 1];
-} else {
-ret = p[offset];
-ret |= p[offset + 1] << 8;
-}
-trace_pflash_data_read16(offset, ret);
-break;
-case 4:
-if (be) {
-ret = p[offset] << 24;
-ret |= p[offset + 1] << 16;
-ret |= p[offset + 2] << 8;
-ret |= p[offset + 3];
-} else {
-ret = p[offset];
-ret |= p[offset + 1] << 8;
-ret |= p[offset + 2] << 16;
-ret |= p[offset + 3] << 24;
-}
-trace_pflash_data_read32(offset, ret);
-break;
-}
+ret = pflash_data_read(pfl, offset, width);
 break;
 case 0x90:
 /* flash ID read */
@@ -213,23 +230,23 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 case 0x0E:
 case 0x0F:
 ret = boff & 0x01 ? pfl->ident3 : pfl->ident2;
-if (ret == (uint8_t)-1) {
-goto flash_read;
+if (ret != (uint8_t)-1) {
+break;
 }
-break;
+/* Fall through to data read. */
 default:
-   

[Qemu-devel] [PATCH v4 06/10] block/pflash_cfi02: Fix CFI in autoselect mode

[Stephen Checkoway]

After a flash device enters CFI mode from autoselect mode, the reset
command returns the device to autoselect mode. An additional reset
command is necessary to return to read array mode.

Signed-off-by: Stephen Checkoway 
Acked-by: Thomas Huth 
---
 hw/block/pflash_cfi02.c   | 21 +
 tests/pflash-cfi02-test.c | 39 +++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index c4efbe8cdf..be10036886 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -61,8 +61,9 @@ do {   \
  */
 #define PFLASH_MAX_ERASE_REGIONS 4
 
-/* Special write cycle for CFI queries. */
+/* Special write cycles for CFI queries. */
 #define WCYCLE_CFI 7
+#define WCYCLE_AUTOSELECT_CFI 8
 
 struct PFlashCFI02 {
 /*< private >*/
@@ -325,6 +326,12 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 }
 
 if (cmd == 0xF0) {
+if (pfl->wcycle == WCYCLE_AUTOSELECT_CFI) {
+/* Return to autoselect mode. */
+pfl->wcycle = 3;
+pfl->cmd = 0x90;
+return;
+}
 goto reset_flash;
 }
 }
@@ -350,7 +357,6 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 /* We're in read mode */
 check_unlock0:
 if (masked_addr == 0x55 && cmd == 0x98) {
-enter_CFI_mode:
 /* Enter CFI query mode */
 pfl->wcycle = WCYCLE_CFI;
 pfl->cmd = 0x98;
@@ -427,9 +433,15 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 /* Unlock bypass reset */
 goto reset_flash;
 }
-/* We can enter CFI query mode from autoselect mode */
+/*
+ * We can enter CFI query mode from autoselect mode, but we must
+ * return to autoselect mode after a reset.
+ */
 if (masked_addr == 0x55 && cmd == 0x98) {
-goto enter_CFI_mode;
+/* Enter autoselect CFI query mode */
+pfl->wcycle = WCYCLE_AUTOSELECT_CFI;
+pfl->cmd = 0x98;
+return;
 }
 /* No break here */
 default:
@@ -510,6 +522,7 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 }
 break;
 case WCYCLE_CFI: /* Special value for CFI queries */
+case WCYCLE_AUTOSELECT_CFI:
 DPRINTF("%s: invalid write in CFI query mode\n", __func__);
 goto reset_flash;
 default:
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 703f084c5d..c2798bbb36 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -477,6 +477,42 @@ static void test_geometry(const void *opaque)
 qtest_quit(qtest);
 }
 
+/*
+ * Test that
+ * 1. enter autoselect mode;
+ * 2. enter CFI mode; and then
+ * 3. exit CFI mode
+ * leaves the flash device in autoselect mode.
+ */
+static void test_cfi_in_autoselect(const void *opaque)
+{
+const FlashConfig *config = opaque;
+QTestState *qtest;
+qtest = qtest_initf("-M musicpal,accel=qtest"
+" -drive if=pflash,file=%s,format=raw,copy-on-read",
+image_path);
+FlashConfig explicit_config = expand_config_defaults(config);
+explicit_config.qtest = qtest;
+const FlashConfig *c = &explicit_config;
+
+/* 1. Enter autoselect. */
+unlock(c);
+flash_cmd(c, UNLOCK0_ADDR, AUTOSELECT_CMD);
+g_assert_cmpint(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF));
+
+/* 2. Enter CFI. */
+flash_cmd(c, CFI_ADDR, CFI_CMD);
+g_assert_cmpint(flash_query(c, FLASH_ADDR(0x10)), ==, replicate(c, 'Q'));
+g_assert_cmpint(flash_query(c, FLASH_ADDR(0x11)), ==, replicate(c, 'R'));
+g_assert_cmpint(flash_query(c, FLASH_ADDR(0x12)), ==, replicate(c, 'Y'));
+
+/* 3. Exit CFI. */
+reset(c);
+g_assert_cmpint(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF));
+
+qtest_quit(qtest);
+}
+
 static void cleanup(void *opaque)
 {
 unlink(image_path);
@@ -604,6 +640,9 @@ int main(int argc, char **argv)
 qtest_add_data_func(path, config, test_geometry);
 g_free(path);
 }
+
+qtest_add_data_func("pflash-cfi02/cfi-in-autoselect", &configuration[0],
+test_cfi_in_autoselect);
 int result = g_test_run();
 cleanup(NULL);
 return result;
-- 
2.20.1 (Apple Git-117)

[Qemu-devel] [PATCH v4 05/10] block/pflash_cfi02: Implement nonuniform sector sizes

[Stephen Checkoway]

Some flash chips support sectors of different sizes. For example, the
AMD AM29LV160DT has 31 64 kB sectors, one 32 kB sector, two 8 kB
sectors, and a 16 kB sector, in that order. The AM29LV160DB has those in
the reverse order.

The `num-blocks` and `sector-length` properties work exactly as they did
before: a flash device with uniform sector lengths. To get non-uniform
sector lengths for up to four regions, the following properties may be
set
- region 0. `num-blocks0` and `sector-length0`;
- region 1. `num-blocks1` and `sector-length1`;
- region 2. `num-blocks2` and `sector-length2`; and
- region 3. `num-blocks3` and `sector-length3`.

If the uniform and nonuniform properties are set, then both must specify
a flash device with the same total size. It would be better to disallow
both being set, or make `num-blocks0` and `sector-length0` alias
`num-blocks` and `sector-length`, but that would make testing currently
impossible.

Signed-off-by: Stephen Checkoway 
Acked-by: Thomas Huth 
---
 hw/block/pflash_cfi02.c   | 177 +---
 tests/pflash-cfi02-test.c | 185 --
 2 files changed, 265 insertions(+), 97 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 101628b4ec..c4efbe8cdf 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -28,7 +28,6 @@
  * - unlock bypass command
  * - CFI queries
  *
- * It does not implement boot blocs with reduced size
  * It does not implement software data protection as found in many real chips
  * It does not implement erase suspend/resume commands
  * It does not implement multiple sectors erase
@@ -55,6 +54,13 @@ do {   \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
+/*
+ * The size of the cfi_table indirectly depends on this and the start of the
+ * PRI table directly depends on it. 4 is the maximum size (and also what
+ * seems common) without changing the PRT table address.
+ */
+#define PFLASH_MAX_ERASE_REGIONS 4
+
 /* Special write cycle for CFI queries. */
 #define WCYCLE_CFI 7
 
@@ -64,8 +70,10 @@ struct PFlashCFI02 {
 /*< public >*/
 
 BlockBackend *blk;
-uint32_t sector_len;
-uint32_t nb_blocs;
+uint32_t uniform_nb_blocs;
+uint32_t uniform_sector_len;
+uint32_t nb_blocs[PFLASH_MAX_ERASE_REGIONS];
+uint32_t sector_len[PFLASH_MAX_ERASE_REGIONS];
 uint64_t total_len;
 uint64_t interleave_multiplier;
 uint8_t mappings;
@@ -86,7 +94,7 @@ struct PFlashCFI02 {
 uint16_t ident3;
 uint16_t unlock_addr0;
 uint16_t unlock_addr1;
-uint8_t cfi_table[0x52];
+uint8_t cfi_table[0x4D];
 QEMUTimer timer;
 /* The device replicates the flash memory across its memory space.  Emulate
  * that by having a container (.mem) filled with an array of aliases
@@ -189,6 +197,25 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr 
offset,
 return ret;
 }
 
+/*
+ * offset should be a byte offset of the QEMU device and _not_ a device
+ * offset.
+ */
+static uint32_t pflash_sector_len(PFlashCFI02 *pfl, hwaddr offset)
+{
+assert(offset < pfl->total_len);
+int nb_regions = pfl->cfi_table[0x2C];
+hwaddr addr = 0;
+for (int i = 0; i < nb_regions; ++i) {
+uint64_t region_size = (uint64_t)pfl->nb_blocs[i] * pfl->sector_len[i];
+if (addr <= offset && offset < addr + region_size) {
+return pfl->sector_len[i];
+}
+addr += region_size;
+}
+abort();
+}
+
 static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
 {
 PFlashCFI02 *pfl = opaque;
@@ -285,6 +312,7 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 PFlashCFI02 *pfl = opaque;
 uint8_t *p;
 uint8_t cmd;
+uint32_t sector_len;
 
 cmd = value;
 if (pfl->cmd != 0xA0) {
@@ -446,12 +474,14 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 case 0x30:
 /* Sector erase */
 p = pfl->storage;
-offset &= ~(pfl->sector_len - 1);
-DPRINTF("%s: start sector erase at " TARGET_FMT_plx "\n", __func__,
-offset);
+sector_len = pflash_sector_len(pfl, offset);
+offset &= ~(sector_len - 1);
+DPRINTF("%s: start sector erase at %0*" PRIx64 "-%0*" PRIx64 "\n",
+__func__, pfl->bank_width * 2, offset,
+pfl->bank_width * 2, offset + sector_len - 1);
 if (!pfl->ro) {
-memset(p + offset, 0xFF, pfl->sector_len);
-pflash_update(pfl, offset, pfl->sector_len);
+memset(p + offset, 0xFF, sector_len);
+pflash_update(pfl, offset, sector_len);
 }
 set_dq7(pfl, 0x00);
 /* Let's wait 1/2 second before sector erase is done */
@@ -515,15 +545,14 @@ static const MemoryRegionOps pflash_cfi02_ops = {
 static void pflash_cfi02_realiz

[Qemu-devel] [PATCH v4 04/10] block/pflash_cfi02: Implement intereleaved flash devices

[Stephen Checkoway]

It's common for multiple narrow flash chips to be hooked up in parallel
to support wider buses. For example, four 8-bit wide flash chips (x8)
may be combined in parallel to produce a 32-bit wide device. Similarly,
two 16-bit wide chips (x16) may be combined.

This commit introduces `device-width` and `max-device-width` properties,
similar to pflash_cfi01, with the following meanings:
- `width`: The width of the logical, qemu device (same as before);
- `device-width`: The width of an individual flash chip, defaulting to
  `width`; and
- `max-device-width`: The maximum width of an individual flash chip,
  defaulting to `device-width`.

Nothing needs to change to support reading such interleaved devices but
commands (e.g., erase and programming) must be sent to all devices at
the same time or else the various chips will be in different states.

For example, a 4-byte wide logical device can be composed of four x8/x16
devices in x8 mode. That is, each device supports both x8 or x16 and
they're being used in the byte, rather than word, mode. This
configuration would have `width=4`, `device-width=1`, and
`max-device-width=2`.

In addition to commands being sent to all devices, guest firmware
expects the status and CFI queries to be replicated for each device.
(The one exception to the response replication is that each device gets
to report its own status bit DQ7 while programming because its value
depends on the value being programmed which will usually differ for each
device.)

Testing is limited to 16-bit wide devices due to the current inability
to override the properties set by `pflash_cfi02_register`, but multiple
configurations are tested.

Stop using global_qtest. Instead, package the qtest variable inside the
FlashConfig structure.

Signed-off-by: Stephen Checkoway 
Acked-by: Thomas Huth 
---
 hw/block/pflash_cfi02.c   | 270 +++--
 tests/pflash-cfi02-test.c | 476 ++
 2 files changed, 576 insertions(+), 170 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index e4bff0c8f8..101628b4ec 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -28,7 +28,6 @@
  * - unlock bypass command
  * - CFI queries
  *
- * It does not support flash interleaving.
  * It does not implement boot blocs with reduced size
  * It does not implement software data protection as found in many real chips
  * It does not implement erase suspend/resume commands
@@ -67,15 +66,19 @@ struct PFlashCFI02 {
 BlockBackend *blk;
 uint32_t sector_len;
 uint32_t nb_blocs;
-uint32_t chip_len;
+uint64_t total_len;
+uint64_t interleave_multiplier;
 uint8_t mappings;
-uint8_t width;
+uint8_t bank_width; /* Width of the QEMU device in bytes. */
+uint8_t device_width; /* Width of individual pflash chip. */
+uint8_t max_device_width; /* Maximum width of individual pflash chip. */
 uint8_t be;
+int device_shift; /* Amount to shift an offset to get a device address. */
 int wcycle; /* if 0, the flash is read normally */
 int bypass;
 int ro;
 uint8_t cmd;
-uint8_t status;
+uint64_t status;
 /* FIXME: implement array device properties */
 uint16_t ident0;
 uint16_t ident1;
@@ -103,16 +106,17 @@ struct PFlashCFI02 {
  */
 static inline void toggle_dq7(PFlashCFI02 *pfl)
 {
-pfl->status ^= 0x80;
+pfl->status ^= pfl->interleave_multiplier * 0x80;
 }
 
 /*
  * Set status bit DQ7 to bit 7 of value.
  */
-static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
+static inline void set_dq7(PFlashCFI02 *pfl, uint64_t value)
 {
-pfl->status &= 0x7F;
-pfl->status |= value & 0x80;
+uint64_t mask = pfl->interleave_multiplier * 0x80;
+pfl->status &= ~mask;
+pfl->status |= value & mask;
 }
 
 /*
@@ -120,7 +124,7 @@ static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
  */
 static inline void toggle_dq6(PFlashCFI02 *pfl)
 {
-pfl->status ^= 0x40;
+pfl->status ^= pfl->interleave_multiplier * 0x40;
 }
 
 /*
@@ -188,7 +192,6 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr 
offset,
 static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
 {
 PFlashCFI02 *pfl = opaque;
-hwaddr boff;
 uint64_t ret;
 
 ret = -1;
@@ -198,12 +201,10 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, 
unsigned int width)
 ++pfl->read_counter > PFLASH_LAZY_ROMD_THRESHOLD) {
 pflash_register_memory(pfl, 1);
 }
-offset &= pfl->chip_len - 1;
-boff = offset & 0xFF;
-if (pfl->width == 2)
-boff = boff >> 1;
-else if (pfl->width == 4)
-boff = boff >> 2;
+/* Mask by the total length of the chip to account for alias mappings. */
+offset &= pfl->total_len - 1;
+hwaddr device_addr = offset >> pfl->device_shift;
+
 switch (pfl->cmd) {
 default:
 /* This should never happen : reset state & treat it as a read*/
@@ -215,29 +216,32 @@ static uint64_t pfla