Re: [Qemu-devel] [PATCH] vl: introduce vm_shutdown()

[Fam Zheng]

On Tue, 02/20 13:10, Stefan Hajnoczi wrote:
> 1. virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the
>virtio_scsi_ctx_check() assertion failure because the BDS AioContext
>has been modified by iothread_stop_all().

Does this patch fix the issue completely? IIUC virtio_scsi_handle_cmd can
already be entered at the time of main thread calling virtio_scsi_clear_aio(),
so this race condition still exists:

  main thread   iothread
-
  vm_shutdown
...
  virtio_bus_stop_ioeventfd
virtio_scsi_dataplane_stop
aio_poll()
  ...
virtio_scsi_data_plane_handle_cmd()
  aio_context_acquire(s->ctx)
  virtio_scsi_acquire(s).enter
  virtio_scsi_clear_aio()
  aio_context_release(s->ctx)
  virtio_scsi_acquire(s).return
  virtio_scsi_handle_cmd_vq()
...
  virtqueue_pop()

Is it possible that the above virtqueue_pop() still returns one element that was
queued before vm_shutdown() was called?

If so I think we additionally need to an "s->ioeventfd_stopped" flag set in
virtio_scsi_stop_ioeventfd() and check it in
virtio_scsi_data_plane_handle_cmd().

Fam

Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer

[Fam Zheng]

On Fri, 02/16 14:44, Philippe Mathieu-Daudé wrote:
> On 02/16/2018 02:41 PM, Kamil Rytarowski wrote:
> > On 16.02.2018 18:30, Philippe Mathieu-Daudé wrote:
> >> But before announcing the host OS being supported again, I'd rather see
> >> reproducible build/tests logs, in a (public - if possible) continuous
> >> integration system. Else it is hard to notice when it get broken.
> >>
> > 
> > This is already done for FreeBSD, NetBSD and OpenBSD.
> 
> We have the ability to run those, but afaik no CI are using them.

That's right. I can try enabling them on patchew, but my gut feeling is that
these still need to be optimized (especially, enabling persisted ccache like our
docker tests, so it is reasonably efficient for our limited resources).

Fam

Re: [Qemu-devel] [PATCH v3 1/3] util/uri.c: replace TAB with whitespace

[Thomas Huth]

 Hi,

thanks for splitting your patch in more reviewable parts :-)

Meta-comment: If you send a patch series, please include a cover letter
next time ("PATCH 0/3") with a brief summary. Then the patch series
shows up nicely as a thread in the e-mail programs of the reviewers (you
also got to send out all files at once with "git send-email 000*" for this).

On 23.02.2018 08:51, Su Hang wrote:
> Formating with clang-format. Change back few code by hand,
> to make sure only include whitespace change.

To make the review really really easy, it would be great if the patch
would not show any differences when displayed with "git diff -w" ... but
that's currently not yet the case since you still changed the position
of newlines in a couple of places, e.g.:

> -while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) ||
> -   ISA_SUB_DELIM(cur) || (*cur == ':'))
> - NEXT(cur);
> +while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) || ISA_SUB_DELIM(cur) 
> ||
> +   (*cur == ':'))
> +NEXT(cur);

Anyway, I've went through the diff with "-w" and the changes look
reasonable to me, so I'm also OK if you want to keep the patch as it is:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v3 2/3] util/uri.c: only remove `return`'s brackets

[Thomas Huth]

On 23.02.2018 08:51, Su Hang wrote:
> only remove brackets that wrap `return` statements' content.
> 
> use `perl -pi -e "s/return \((.*?)\);/return \1;/g" util/uri.c`
> to remove pattern like this: "return (1);"
> 
> Signed-off-by: Su Hang 
> ---
>  util/uri.c | 160 
> ++---
>  1 file changed, 80 insertions(+), 80 deletions(-)

Looks good to me:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Christian Borntraeger]

Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I 
do not 
specify loadparm or boot menu:


qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device 
virtio-blk-ccw,drive=d1,bootindex=1 
LOADPARM=[]
Using virtio-blk.
Using SCSI scheme.
s390x Enumerated Boot Menu.

3 entries detected. Select from boot index 0 to 2.

Please choose:




This will break setups that want to boot up unattended.



On 02/21/2018 08:35 PM, Collin L. Walling wrote:
> Due to the introduction of the QemuIplParameter block and taking some time to
> revist some areas, a few chunks of code have been reworked to better align 
> with
> those changes. I also think the reworkings improve readability. The same 
> functionality is still there, the code just looks a little different (and 
> hopefully looks better).
> 
> --- [v8] ---
> 
> The tl;dr:
> 
> cleaned up some early patches based on review, threw zipl boot option code
> into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
> changes.
> 
> The "pls explain":
> 
> * cleaned up "s390-ccw: refactor boot map table code"
> 
> - removed void ptr casting in a couple of spots
> 
> * cleaned up "s390-ccw: set cp_receive mask..." patch
> 
> - setting the mask consumes a service interrupt, so there is no need 
> for 
>   a followup sclp_print
> 
> * fixed uitoa based on feedback from v7
> 
> * fixed alignment concerns in QemuIplParameters and renamed flags field
> 
> - reasoning: necessary; better readability
> 
> - s/boot_menu_flags/qipl_flags so this field can be (potentially) 
> used for 
>   other purposes in the future
> 
> - when setting the boot menu parms in the bios, we take care to mask 
> out
>   the appropriate qipl_flags for boot menu specific flags
> 
> - boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"
> 
> - also updated comment above this struct
> 
> * cleaned up "s390-ccw: read user input..." patch
> 
> - defines for low core external interrupt code addr and 
>   clock comparator interrupt code
> 
> - take care to make sure buf remains null terminated when passed to 
> read_prompt
> 
> * [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are 
> present"
> 
> - reasoning: better readability; further explanation of feature
> 
> - *nothing new added to this patch series here*
> 
> - zipl options flag setting and parsing *moved to* this patch
> 
> - this attempts to better explain how these fields are used and how 
> they get
>   parsed
> 
> - the commit message gives details on how to set these fields in the 
> zipl
>   configuration file
> 
> - the zipl options are only set for CCW type IPL devices (since no 
>   other devices actually support it)
> 
> * refactored s390_ipl_set_boot_menu
> 
> - reasoning: better readability
> 
> - the idea is that we should take care to appropriately set the boot 
> menu
>   flags for each IPL device type from the beginning. We should not set
>   certain flags for devices that cannot support certain features (eg 
> SCSI 
>   does not support zipl menus, so we should never set the 
> use_zipl_opts flag
>   for SCSI) 
> 
> - since there are no longer boot menu fields specific to each IPL 
> type,
>   the switch statement is simply used to detect if the IPL device type
>   is capable of a boot menu
> 
> - since zipl flags are only set for CCW type IPL devices, I reworked 
>   the logic and removed some indentation
> 
> * s/menu_check_flags/menu_is_enabled
> 
> - reasoning: better readability
> 
> - no parameters
> 
> - "if menu is enabled" reads better than "if these specific flag bits 
> are set"
> 
> * removed menu.h
> 
> - reasoning: file not needed; less to maintain
> 
> - introduction of qipl and better understanding of zipl options led 
> to 
>   this decision. by the end of these changes, this file ended up 
>   housing 4 function declarations and no longer seemed necessary
> 
> - all menu related function declarations are in s390-ccw.h
> 
> - boot menu flags are defined in iplb.h (which aligns with 
> hw/s390x/ipl.h)
> 
> --- [Summary] ---
> 
> These patches implement a boot menu for ECKD DASD and SCSI guests on s390x. 
> The menu will only appear if the disk has been configured for IPL with the 
> zIPL tool and with the following QEMU command line options:
> 
> -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'
> 
> The following must be specified for the device to be IPL'd from:
> 
> -device ...,bootindex=1
> 
> or via the following libvirt domain xml:
> 
> 
>   
> 
> 
> or
> 
> 
>   ...
>   
> 

Re: [Qemu-devel] [PATCH v4 4/7] qdev: add hotpluggable to DeviceState

[Gerd Hoffmann]

  Hi,

> > What type of device is only sometimes hotpluggable ?
> > The commit message says "display devices" and "consoles",
> > but I would expect those to both be types of device which
> > have a class which is never hotpluggable, so you can mark
> > them non-hotpluggable with the existing class flag rather
> > than needing a per-instance flag.
> 
> With this series, a vfio-pci device optionally supports a display.  The
> vfio-pci device is hotpluggable, but QEMU display support is not.  So
> the solution here is to make the vfio-pci device non-hotpluggable only
> when it supports and enables a display.
> 
> Gerd, is there another solution that the display object is instantiated
> separately from the vfio-pci object and the display support in the
> vfio-pci device references the display object via an id.

Well, not really.  At least not without putting much of qemu display
support upside down.  The qemu display (aka QemuConsole) is created and
managed by the display devices, they can't be created independant from
a device ...

The connection between QemuConsole and User Interface (i.e. gtk, spice,
...) is a bit more flexible.  But also not really designed for hotplug
as QemuConsole is not hotpluggable in the first place ...

We could drop the display property and use two devices instead.

  new vfio-pci would behave like display=off with this series.
  added vfio-pci-display has display=on behavior.
  display=auto is not possible.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v4 0/7] vfio: add display support

[Gerd Hoffmann]

> Hi Gerd,
> 
> It's a little bit concerning that the only way we can test the
> region-based display support is with proprietary drivers that nobody
> but NVIDIA has at this point.  Have you considered adding region-based
> display support to the mdev sample tty driver?  I know it sounds
> ridiculous for a serial device to have a display, but the vfio display
> region support isn't really tied to the functionality of the base mdev
> device.  We could have it simply display a static test pattern, just so
> we can test the end to end code path without a dependency on a closed
> vendor driver.

Hmm, have to think about that.  Some way to change the display content
would be nice as you can see whenever display updates are working then.

Maybe the fbcon text rendering code can be used to send the serial
console output from the guest to the display ...

cheers,
  Gerd

[Qemu-devel] [Bug 1636217] Re: qemu-kvm 2.7 does not boot kvm VMs with virtio on top of VMware ESX

[zzfancy]

we still meet similar issue on centos.7 (qemu 2.9.0-16.el7_4.5.1 +
libvirt 3.2.0-14.el7_4.3)

my workaround including:
a) without kvm accel
or 
b) as comment #7 said "-machine type=pc-i440fx-x" where x <= 2.6
or
c) with pci device "disable-modern=on"

i found the function _farcall16 in seabios was invoked 
(https://github.com/coreboot/seabios/blob/af0daeb2687ad2595482b8a71b02a082a5672ceb/src/stacks.c#L418)
and failed when guest hang with 'Booting from hard disk'.

the invoking sequence (in seabios rel-1.11.0-5-g14d91c3) like :
src/boot.c line 614, call_boot_entry->
src/stacks.c line 427, farcall16->
src/stack.c line 411, _farcall16


but the issue perform diff in our two clusters. 

I just name them cluster A(6.0.0.0 3029758 E5 2640 v2,ststem x3650 M4)
and cluster B (6.0.0.0 3029758 E5 2620 v4,system x3650 M5)for easy.

This issue in cluster B not be reproduced in cluster A(same
qemu/libvirt/esxi)

my command:
/usr/libexec/qemu-kvm  -machine pc-i440fx-rhel7.3.0 \
-m 256 -drive file=centos.qcow2,if=none,id=drive-virtio-disk0 \
-device 
virtio-blk-pci,disable-modern=on,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0
 \
-vnc :1 -chardev stdio,id=seabios -device 
isa-debugcon,iobase=0x402,chardev=seabios

hope the above info can help fix the bug.

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

Title:
  qemu-kvm 2.7 does not boot kvm VMs with virtio on top of VMware ESX

Status in QEMU:
  New

Bug description:
  After todays Proxmox update all my Linux VMs stopped booting.

  # How to reproduce
  - Have KVM on top of VMware ESX (I use VMware ESX 6)
  - Boot Linux VM with virtio Disk drive.

  
  # Result
  virtio based VMs do not boot anymore:

  root@demotuxdc:/etc/pve/nodes/demotuxdc/qemu-server# grep virtio0 100.conf 
  bootdisk: virtio0
  virtio0: pvestorage:100/vm-100-disk-1.raw,discard=on,size=20G

  (initially with cache=writethrough, but that doesn´t matter)

  What happens instead is:

  - BIOS displays "Booting from harddisk..."
  - kvm process of VM loops at about 140% of Intel(R) Core(TM) i5-6260U CPU @ 
1.80GHz Skylake dual core CPU

  Disk of course has valid bootsector:

  root@demotuxdc:/srv/pvestorage/images/100# file -sk vm-100-disk-1.raw 
  vm-100-disk-1.raw: DOS/MBR boot sector DOS/MBR boot sector DOS executable 
(COM), boot code
  root@demotuxdc:/srv/pvestorage/images/100# head -c 2048 vm-100-disk-1.raw | 
hd | grep GRUB
  0170  be 94 7d e8 2e 00 cd 18  eb fe 47 52 55 42 20 00  |..}...GRUB .|

  
  # Workaround 1
  - Change disk from virtio0 to scsi0
  - Debian boots out of the box after this change
  - SLES 12 needs a rebuilt initrd
  - CentOS 7 too, but it seems that is not even enough and it still fails (even 
in hostonly="no" mode for dracut)

  
  # Workaround 2
  Downgrade pve-qemu-kvm 2.7.0-3 to 2.6.2-2.

  
  # Expected results
  Disk boots just fine via virtio like it did before.

  
  # Downstream bug report
  Downstream suggests an issue with upstream qemu-kvm:

  https://bugzilla.proxmox.com/show_bug.cgi?id=1181

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

Re: [Qemu-devel] [PATCH v3 3/3] util/uri.c: add brackets to `if` statements

[Thomas Huth]

On 23.02.2018 08:51, Su Hang wrote:
> Add brackets that wrap `if`, `else`, `while` that hold single
> statements.
> 
> In order to do this, I write a simple python regex script.
> 
> Since then, all complaints rised by checkpatch.pl has been suppressed.
> 
> Signed-off-by: Su Hang 
> ---
>  util/uri.c | 462 
> ++---
>  1 file changed, 291 insertions(+), 171 deletions(-)
> 
> diff --git a/util/uri.c b/util/uri.c
> index 278e876ab8b1..48f7298787b1 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -105,18 +105,18 @@ static void uri_clean(URI *uri);
>   */
>  
>  #define IS_UNWISE(p) 
>   \
> -  (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||  
>   \
> -   ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) || 
>   \
> -   ((*(p) == ']')) || ((*(p) == '`')))
> +(((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||
>   \
> + ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) ||   
>   \
> + ((*(p) == ']')) || ((*(p) == '`')))
>  /*
>   * reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | "," |
>   *"[" | "]"
>   */
>  
>  #define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') ||  
>   \
> -((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') ||  
>   \
> -((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') ||  
>   \
> -((x) == ']'))
> +  ((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') ||
>   \
> +  ((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') ||
>   \
> +  ((x) == ']'))

The above whitespace changes should ideally be done in the first patch instead.

>  /*
>   * unreserved = alphanum | mark
> @@ -211,15 +211,17 @@ static int rfc3986_parse_scheme(URI *uri, const char 
> **str)
>  {
>  const char *cur;
>  
> -if (str == NULL)
> +if (str == NULL) {
>  return -1;
> +}
>  
>  cur = *str;
> -if (!ISA_ALPHA(cur))
> +if (!ISA_ALPHA(cur)) {
>  return 2;
> +}
>  cur++;
> -while (ISA_ALPHA(cur) || ISA_DIGIT(cur) ||
> -   (*cur == '+') || (*cur == '-') || (*cur == '.'))
> +while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == 
> '-') ||
> +   (*cur == '.'))
>  cur++;

You've changed the while statement, but checkpatch.pl apparently does not
complain about missing curly braces here ... that's strange, I thought we'd
also wanted to enforce curly braces for while loops. Anyway, could you please
add curly braces around the "*cur++;" here, too?

> @@ -1437,15 +1503,18 @@ done_cd:
>  /* string will overlap, do not use strcpy */
>  tmp = cur;
>  segp += 3;
> -while ((*tmp++ = *segp++) != 0)
> +while ((*tmp++ = *segp++) != 0) {
>  ;
> +}

A bikeshed-painting-friday question for everybody on qemu-devel:
Should there be a single semicolon inside curly braces in this case, or not?

>  /* If there are no previous segments, then keep going from here.  */
>  segp = cur;
> -while ((segp > path) && ((--segp)[0] == '/'))
> +while ((segp > path) && ((--segp)[0] == '/')) {
>  ;

(dito)

> -if (segp == path)
> +}
> +if (segp == path) {
>  continue;
> +}
>  
>  /* "segp" is pointing to the end of a previous segment; find it's
>   * start.  We need to back up to the previous segment and start
[...]
> @@ -1491,8 +1562,9 @@ done_cd:
>  static int is_hex(char c)
>  {
>  if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
> -((c >= 'A') && (c <= 'F')))
> +((c >= 'A') && (c <= 'F'))) {
>  return 1;
> +}
>  return 0;
>  }

Not related to your patch, but an idea for a future clean-up patch:
We've already got qemu_isxdigit(), so there is no real need for this
separate is_hex() function.

[...]
> @@ -2020,17 +2127,19 @@ char *uri_resolve_relative(const char *uri, const 
> char *base)
>   */
>  if (bptr[pos] != ref->path[pos]) { /* check for trivial URI == base 
> */
>  for (; bptr[ix] != 0; ix++) {
> -if (bptr[ix] == '/')
> +if (bptr[ix] == '/') {
>  nbslash++;
> +}
>  }
>  }
>  len = strlen(uptr) + 1;
>  }
>  
>  if (nbslash == 0) {
> -if (uptr != NULL)
> +if (uptr != NULL) {
>  /* exception characters from uri_to_string */
> -val = uri_string_escape(uptr, "/;&=+$,");
> +}
> +val = uri_string_escape(uptr, "/;&=+$,");

That's a bug: uri_string_escape() should be within the curly braces instead.

By the way, found that one with the following trick: Compare the disassemblies
before and after you're changes:

 git

Re: [Qemu-devel] [PATCH RFCv2] s390x/sclp: remove memory hotplug support

[Cornelia Huck]

On Thu, 22 Feb 2018 14:29:09 -0500
Matthew Rosato  wrote:

> On 02/22/2018 06:13 AM, Christian Borntraeger wrote:
> > 
> > 
> > On 02/21/2018 06:39 PM, Cornelia Huck wrote:  
> >> On Tue, 20 Feb 2018 16:05:54 +0100
> >> David Hildenbrand  wrote:
> >>  
> >>> On 20.02.2018 15:57, Cornelia Huck wrote:  
>  On Tue, 20 Feb 2018 13:16:37 +0100
>  David Hildenbrand  wrote:
>  
> > On 20.02.2018 13:05, Christian Borntraeger wrote:
> >>
> >>
> >> On 02/19/2018 06:42 PM, David Hildenbrand wrote:  
> >>> From an architecture point of view, nothing can be mapped into the 
> >>> address
> >>> space on s390x. All there is is memory. Therefore there is also not 
> >>> really
> >>> an interface to communicate such information to the guest. All we can 
> >>> do is
> >>> specify the maximum ram address and guests can probe in that range if
> >>> memory is available and usable (TPROT).  
> >>
> >> In fact there is an interface in SCLP that describes the memory sizes 
> >> (maximum in 
> >> read scp info) and the details (read_storage_element0_info).  I am 
> >> asking myself
> >> if we should re-introduce read_storage_element_info and use that to 
> >> avoid tprot  
> >
> > Yes, we could do that (basically V1 of this patch) but have to glue it
> > to the a compatibility machine then.
> 
>  Actually, this makes quite a bit of sense (introduce the interface for
>  everyone in 2.12 and turn it off in compat machines).
> >>>
> >>> Jup, either 2.12 or 2.13, no need to hurry.
> >>>  
> 
>  Does real hardware have configurations where you can get the memory
>  sizes, but not the attach/deattach support? (Hardware with the feature,
>  but no standby memory defined?)
> > 
> > We have different sclp facilities for attach/detach and information, so
> > we can implement that. 
> > 
> >   
> >>>
> >>> I would guess that "0" for standby memory is valid but only people with
> >>> access to documentation can answer that :)  
> >>
> >> So, should we go with this patch now and re-introduce the read
> >> functions if the above is indeed true?  
> > 
> > Yes, go with this patch. Right now Linux guests will not make use of that, 
> > so
> > we can re-add that if it turns out to be useful for future guests.
> > 
> > 
> > 
> > Matt, last chance to complain with reasons why we want to keep the current 
> > standby memory
> > solution in its current form. (Or please ack the patch if you agree)  
> 
> Nope, this makes sense given its incompatibility w/ the common layer.  I
> also agree with the prior comment that, should we revisit this feature
> in the future, it should probably be via an s390-specific interface.
> 
> Acked-by: Matthew Rosato 

Thanks, applied (with the discussed description tweak) to s390-next.

We should also mention this in the changelog once this hits master.
I'll try to remember to do that.

Re: [Qemu-devel] [PATCH v2 32/32] arm/translate-a64: add all single op FP16 to handle_fp_1src_half

[Alex Bennée]

Richard Henderson  writes:

> On 02/08/2018 09:31 AM, Alex Bennée wrote:
>> This includes FMOV, FABS, FNEG, FSQRT and  FRINT[NPMZAXI]. We re-use
>> existing helpers to achieve this.
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  target/arm/translate-a64.c | 72 
>> ++
>>  1 file changed, 72 insertions(+)
>>
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index 92adf43a89..265bfb14d0 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -4508,6 +4508,66 @@ static void disas_fp_csel(DisasContext *s, uint32_t 
>> insn)
>>  tcg_temp_free_i64(t_true);
>>  }
>>
>> +/* Floating-point data-processing (1 source) - half precision */
>> +static void handle_fp_1src_half(DisasContext *s, int opcode, int rd, int rn)
>> +{
>> +TCGv_ptr fpst = NULL;
>> +TCGv_i32 tcg_op;
>> +TCGv_i32 tcg_res;
>> +
>> +tcg_op = read_fp_sreg(s, rn);
>> +tcg_res = tcg_temp_new_i32();
>> +
>> +switch (opcode) {
>> +case 0x0: /* FMOV */
>> +tcg_gen_mov_i32(tcg_res, tcg_op);
>> +break;
>> +case 0x1: /* FABS */
>> +gen_helper_advsimd_absh(tcg_res, tcg_op);
>> +break;
>> +case 0x2: /* FNEG */
>> +tcg_gen_xori_i32(tcg_res, tcg_op, 0x8000);
>> +break;
>> +case 0x3: /* FSQRT */
>> +gen_helper_sqrt_f16(tcg_res, tcg_op, cpu_env);
>> +break;
>> +case 0x8: /* FRINTN */
>> +case 0x9: /* FRINTP */
>> +case 0xa: /* FRINTM */
>> +case 0xb: /* FRINTZ */
>> +case 0xc: /* FRINTA */
>> +{
>> +TCGv_i32 tcg_rmode = tcg_const_i32(arm_rmode_to_sf(opcode & 7));
>> +fpst = get_fpstatus_ptr(true);
>> +
>> +gen_helper_set_rmode(tcg_rmode, tcg_rmode, fpst);
>> +gen_helper_advsimd_rinth(tcg_res, tcg_op, fpst);
>> +
>> +gen_helper_set_rmode(tcg_rmode, tcg_rmode, fpst);
>> +tcg_temp_free_i32(tcg_rmode);
>> +break;
>> +}
>> +case 0xe: /* FRINTX */
>> +fpst = get_fpstatus_ptr(true);
>> +gen_helper_advsimd_rinth_exact(tcg_res, tcg_op, fpst);
>> +break;
>> +case 0xf: /* FRINTI */
>> +fpst = get_fpstatus_ptr(true);
>> +gen_helper_advsimd_rinth(tcg_res, tcg_op, fpst);
>> +break;
>> +default:
>> +abort();
>> +}
>> +
>> +write_fp_sreg(s, rd, tcg_res);
>
> Some of these helpers will zero-extend from 16 bits, but at least a few won't
> -- notably fmov and fneg.  I wonder if it wouldn't be best to have a
> write_fp_hreg.

I fixed this up by using read_vec_element to load the value.

--
Alex Bennée

[Qemu-devel] [PATCH] s390x/sclp: remove duplicate MAX_AVAIL_SLOTS define

[Cornelia Huck]

It's already defined in cpu.h (to the same value) and not used in
the sclp code.

Signed-off-by: Cornelia Huck 
---
 include/hw/s390x/sclp.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 476cbf78f2..f9db243484 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -35,7 +35,6 @@
 #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
 #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
 #define SCLP_INCREMENT_UNIT 0x1
-#define MAX_AVAIL_SLOTS 32
 #define MAX_STORAGE_INCREMENTS  1020
 
 /* CPU hotplug SCLP codes */
-- 
2.13.6

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 07/67] target/arm: Implement SVE Predicate Logical Operations Group

[Peter Maydell]

On 22 February 2018 at 19:37, Richard Henderson
 wrote:
> On 02/22/2018 10:55 AM, Peter Maydell wrote:
>>> +# Three prediate operand, with governing predicate, flag setting
>>
>> Three what?
>
> Feh, typo for predicate.

Oh, right -- I'd thought it might be some mashup/typo of something-immediate.

-- PMM

Re: [Qemu-devel] [PATCH v3 5/5] aarch64-linux-user: Add support for SVE signal frame records

[Peter Maydell]

On 22 February 2018 at 20:14, Richard Henderson
 wrote:
> On 02/22/2018 08:41 AM, Peter Maydell wrote:
>> On 16 February 2018 at 21:56, Richard Henderson
>>  wrote:

>>> +if (sve_size <= std_size) {
>>> +sve_ofs = size;
>>> +size += sve_size;
>>> +end1_ofs = size;
>>> +} else {
>>> +/* Otherwise we need to allocate extra space.  */
>>> +extra_ofs = size;
>>> +size += sizeof(struct target_extra_context);
>>> +end1_ofs = size;
>>> +size += QEMU_ALIGN_UP(sizeof(struct target_aarch64_ctx), 16);
>>
>> Why do we add the size of target_aarch64_ctx to size here?
>> We already account for the size of the end record later, so
>> what is this one?
>
> This is for the end record within the extra space, as opposed to the end 
> record
> within the standard space which is what we accounted for before.  A comment
> would help, I supposed.

Oh, so 'size' is accounting for both the standard space used
and the extra space? I had thought that 'size' was just counting
up the standard space used, and the extra space count was in
extra_size.

thanks
-- PMM

Re: [Qemu-devel] [qemu-s390x] [PATCH] s390x/sclp: remove duplicate MAX_AVAIL_SLOTS define

[Christian Borntraeger]

On 02/23/2018 10:54 AM, Cornelia Huck wrote:
> It's already defined in cpu.h (to the same value) and not used in
> the sclp code.
> 
> Signed-off-by: Cornelia Huck 
> ---
>  include/hw/s390x/sclp.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index 476cbf78f2..f9db243484 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -35,7 +35,6 @@
>  #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
>  #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
>  #define SCLP_INCREMENT_UNIT 0x1
> -#define MAX_AVAIL_SLOTS 32
>  #define MAX_STORAGE_INCREMENTS  1020
> 
>  /* CPU hotplug SCLP codes */
> 
Do we need this define and s390_get_memslot_count at all after removing
the hotplug code?

Re: [Qemu-devel] [PULL 00/32] target-arm queue

[Peter Maydell]

On 22 February 2018 at 15:22, Peter Maydell  wrote:
> Latest run of arm patches -- most of these are Philippe's SD card
> cleanups. I have more in my queue to review, but 32 is enough
> patches to warrant sending out.
>
> thanks
> -- PMM
>
> The following changes since commit ff8689611a1d954897d857b28f7ef404e11cfa2c:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-signed' 
> into staging (2018-02-22 11:37:05 +)
>
> are available in the Git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20180222
>
> for you to fetch changes up to 4e5cc6756586e967993187657dfcdde4e00288d9:
>
>   sdcard: simplify SD_SEND_OP_COND (ACMD41) (2018-02-22 15:12:54 +)
>
> 
>  * New "raspi3" machine emulating RaspberryPi 3
>  * Fix bad register definitions for VMIDR and VMPIDR (which caused
>assertions for 64-bit guest CPUs with EL2 on big-endian hosts)
>  * hw/char/stm32f2xx_usart: fix TXE/TC bit handling
>  * Fix ast2500 protection register emulation
>  * Lots of SD card emulation cleanups and bugfixes
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Thomas Huth]

On 22.02.2018 20:40, Collin L. Walling wrote:
> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>
>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
 On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
> On 22.02.2018 12:51, Christian Borntraeger wrote:
>> Series
>> Acked-by: Christian Borntraeger 
 Thanks!!!

>>
>> menu on scsi and dasd bootmaps tested successfully.
>>
>> There is one thing that we might want to fix (can be an addon
>> patch since this is a non-customer
>> scenario (no libvirt)).
>>
>> If you start QEMU manually without a bootindex, the -boot menu=on
>> is ignored
>> if no drive has a bootindex.
>>
>> For example:
>>
>> -drive file=/dev/dasda,if=none,id=d1 -device
>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>> does work
>>
>> -drive file=/dev/dasda -boot menu=on
>> does not
>>
>> instead it prints:
>> qemu-system-s390x: boot menu is not supported for this device type.
>>
>> and the boots up the default entry.
>>
> That should indeed be a separate patch, as it would move logic
> currently
> in the BIOS up to QEMU (find the first defined virtio disk and
> select it
> as boot disk).
> In fact it's more complicated than that, because it would have to
> properly account for -boot order=[acdn] and produce the respective
> IPLB.
> While it makes sense, I wouldn't rush that in but rather change the
> error message to indicate that -device bootindex is needed to activate
> the menu, at least for the time being.
> [...]
>
 I can look into it.  Theoretically, the easier fix should just
 involve parsing all
 of the -device commands and looking for a "bootindex=1" field. The
 Qemu options
 code already handles a bulk of this work, so it's just a matter of
 putting it all
 together.

 Shall I whip something up and post what I have as a reply to this
 email chain?
>>> In fact, it should already be there.
>>>
>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>> {
>>>  DeviceState *dev_st;
>>>
>>>  dev_st = get_boot_device(0);
>>>
>>> --> if this returns 0 we have no bootindex statement anywhere and the
>>> BIOS will IPL the default
>>> disk.
>>>
>>>
>> Makes sense.  I'm working on making this patch look as clean as
>> possible. The fact that no boot menu
>> options present means we fallback to using zipl values for CCW being
>> tied into the switch statement
>> is making things a bit tricky. Just have to think the logic through a
>> bit.  Will get back to you once
>> I have something good.
>>
> This should do the trick (this can also be squished painlessly into 6/13
> if desired)

Patch looks fine to me. I can either take it directly like this, or in
case you have to respin (depends on the problem that Christian reported
with the Ubuntu guest), I'm also fine if you squash it into an earlier
patch instead.

 Thomas

Re: [Qemu-devel] [PATCH v6 00/23] RISC-V QEMU Port Submission

[Daniel P . Berrangé]

On Fri, Feb 23, 2018 at 01:11:46PM +1300, Michael Clark wrote:
> QEMU RISC-V Emulation Support (RV64GC, RV32GC)
> 
> This is hopefully the "fix remaining issues in-tree" release.

This code seems to be a mixture of LGPLv2+ and MIT licensed code. The
preferred license for QEMU contributions is GPLv2+. Is there a reason
you need to diverge from this or can it be changed to be all GPLv2+ ?

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

Re: [Qemu-devel] [qemu-s390x] [PATCH] s390x/sclp: remove duplicate MAX_AVAIL_SLOTS define

[Cornelia Huck]

On Fri, 23 Feb 2018 11:02:15 +0100
Christian Borntraeger  wrote:

> On 02/23/2018 10:54 AM, Cornelia Huck wrote:
> > It's already defined in cpu.h (to the same value) and not used in
> > the sclp code.
> > 
> > Signed-off-by: Cornelia Huck 
> > ---
> >  include/hw/s390x/sclp.h | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> > index 476cbf78f2..f9db243484 100644
> > --- a/include/hw/s390x/sclp.h
> > +++ b/include/hw/s390x/sclp.h
> > @@ -35,7 +35,6 @@
> >  #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
> >  #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
> >  #define SCLP_INCREMENT_UNIT 0x1
> > -#define MAX_AVAIL_SLOTS 32
> >  #define MAX_STORAGE_INCREMENTS  1020
> > 
> >  /* CPU hotplug SCLP codes */
> >   
> Do we need this define and s390_get_memslot_count at all after removing
> the hotplug code?
> 

Indeed. Let's kill it with fire :)

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Christian Borntraeger]

On 02/23/2018 11:07 AM, Thomas Huth wrote:
> On 22.02.2018 20:40, Collin L. Walling wrote:
>> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:

 On 02/22/2018 04:40 PM, Collin L. Walling wrote:
> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>> Series
>>> Acked-by: Christian Borntraeger 
> Thanks!!!
>
>>>
>>> menu on scsi and dasd bootmaps tested successfully.
>>>
>>> There is one thing that we might want to fix (can be an addon
>>> patch since this is a non-customer
>>> scenario (no libvirt)).
>>>
>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>> is ignored
>>> if no drive has a bootindex.
>>>
>>> For example:
>>>
>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>> does work
>>>
>>> -drive file=/dev/dasda -boot menu=on
>>> does not
>>>
>>> instead it prints:
>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>
>>> and the boots up the default entry.
>>>
>> That should indeed be a separate patch, as it would move logic
>> currently
>> in the BIOS up to QEMU (find the first defined virtio disk and
>> select it
>> as boot disk).
>> In fact it's more complicated than that, because it would have to
>> properly account for -boot order=[acdn] and produce the respective
>> IPLB.
>> While it makes sense, I wouldn't rush that in but rather change the
>> error message to indicate that -device bootindex is needed to activate
>> the menu, at least for the time being.
>> [...]
>>
> I can look into it.  Theoretically, the easier fix should just
> involve parsing all
> of the -device commands and looking for a "bootindex=1" field. The
> Qemu options
> code already handles a bulk of this work, so it's just a matter of
> putting it all
> together.
>
> Shall I whip something up and post what I have as a reply to this
> email chain?
 In fact, it should already be there.

 static bool s390_gen_initial_iplb(S390IPLState *ipl)
 {
  DeviceState *dev_st;

  dev_st = get_boot_device(0);

 --> if this returns 0 we have no bootindex statement anywhere and the
 BIOS will IPL the default
 disk.


>>> Makes sense.  I'm working on making this patch look as clean as
>>> possible. The fact that no boot menu
>>> options present means we fallback to using zipl values for CCW being
>>> tied into the switch statement
>>> is making things a bit tricky. Just have to think the logic through a
>>> bit.  Will get back to you once
>>> I have something good.
>>>
>> This should do the trick (this can also be squished painlessly into 6/13
>> if desired)
> 
> Patch looks fine to me. I can either take it directly like this, or in
> case you have to respin (depends on the problem that Christian reported
> with the Ubuntu guest), I'm also fine if you squash it into an earlier
> patch instead.

FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also
happens with other guests (e.g. fedora). 

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Thomas Huth]

On 23.02.2018 09:53, Christian Borntraeger wrote:
> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I 
> do not 
> specify loadparm or boot menu:
> 
> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 
> -device virtio-blk-ccw,drive=d1,bootindex=1 

I can reproduce this now, too. FWIW: It only happens with
virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
first). Collin, can you reproduce this, too?

 Thomas

[Qemu-devel] [PATCH] s390x: remove s390_get_memslot_count

[Cornelia Huck]

Not needed anymore after removal of the memory hotplug code.

Signed-off-by: Cornelia Huck 
---
 include/hw/s390x/sclp.h  | 1 -
 target/s390x/cpu.c   | 9 -
 target/s390x/cpu.h   | 4 
 target/s390x/kvm-stub.c  | 5 -
 target/s390x/kvm.c   | 5 -
 target/s390x/kvm_s390x.h | 1 -
 6 files changed, 25 deletions(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 476cbf78f2..f9db243484 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -35,7 +35,6 @@
 #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
 #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
 #define SCLP_INCREMENT_UNIT 0x1
-#define MAX_AVAIL_SLOTS 32
 #define MAX_STORAGE_INCREMENTS  1020
 
 /* CPU hotplug SCLP codes */
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index f02ed19c70..627002b225 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -437,15 +437,6 @@ void s390_cmma_reset(void)
 }
 }
 
-int s390_get_memslot_count(void)
-{
-if (kvm_enabled()) {
-return kvm_s390_get_memslot_count();
-} else {
-return MAX_AVAIL_SLOTS;
-}
-}
-
 int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
 int vq, bool assign)
 {
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 5bd6de7e8e..c5ef930876 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -621,9 +621,6 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
 /* SIGP order code mask corresponding to bit positions 56-63 */
 #define SIGP_ORDER_MASK 0x00ff
 
-/* from s390-virtio-ccw */
-#define MAX_AVAIL_SLOTS  32
-
 /* machine check interruption code */
 
 /* subclasses */
@@ -695,7 +692,6 @@ int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
 int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
 void s390_crypto_reset(void);
 bool s390_get_squash_mcss(void);
-int s390_get_memslot_count(void);
 int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
 void s390_cmma_reset(void);
 void s390_enable_css_support(S390CPU *cpu);
diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
index 8cdcf83845..29b10542cc 100644
--- a/target/s390x/kvm-stub.c
+++ b/target/s390x/kvm-stub.c
@@ -84,11 +84,6 @@ void kvm_s390_cmma_reset(void)
 {
 }
 
-int kvm_s390_get_memslot_count(void)
-{
-  return MAX_AVAIL_SLOTS;
-}
-
 void kvm_s390_reset_vcpu(S390CPU *cpu)
 {
 }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 8ef509ece4..656aaea2cd 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1854,11 +1854,6 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier 
*notifier, uint32_t sch,
 return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
 }
 
-int kvm_s390_get_memslot_count(void)
-{
-return kvm_check_extension(kvm_state, KVM_CAP_NR_MEMSLOTS);
-}
-
 int kvm_s390_get_ri(void)
 {
 return cap_ri;
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index 7a3b862eea..c383bf4ee9 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -30,7 +30,6 @@ int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t 
*tod_clock);
 void kvm_s390_enable_css_support(S390CPU *cpu);
 int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
 int vq, bool assign);
-int kvm_s390_get_memslot_count(void);
 int kvm_s390_cmma_active(void);
 void kvm_s390_cmma_reset(void);
 void kvm_s390_reset_vcpu(S390CPU *cpu);
-- 
2.13.6

Re: [Qemu-devel] [qemu-s390x] [PATCH] s390x: remove s390_get_memslot_count

[Christian Borntraeger]

On 02/23/2018 11:19 AM, Cornelia Huck wrote:
> Not needed anymore after removal of the memory hotplug code.
> 
> Signed-off-by: Cornelia Huck 

Acked-by: Christian Borntraeger 

> ---
>  include/hw/s390x/sclp.h  | 1 -
>  target/s390x/cpu.c   | 9 -
>  target/s390x/cpu.h   | 4 
>  target/s390x/kvm-stub.c  | 5 -
>  target/s390x/kvm.c   | 5 -
>  target/s390x/kvm_s390x.h | 1 -
>  6 files changed, 25 deletions(-)
> 
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index 476cbf78f2..f9db243484 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -35,7 +35,6 @@
>  #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
>  #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
>  #define SCLP_INCREMENT_UNIT 0x1
> -#define MAX_AVAIL_SLOTS 32
>  #define MAX_STORAGE_INCREMENTS  1020
> 
>  /* CPU hotplug SCLP codes */
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index f02ed19c70..627002b225 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -437,15 +437,6 @@ void s390_cmma_reset(void)
>  }
>  }
> 
> -int s390_get_memslot_count(void)
> -{
> -if (kvm_enabled()) {
> -return kvm_s390_get_memslot_count();
> -} else {
> -return MAX_AVAIL_SLOTS;
> -}
> -}
> -
>  int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
>  int vq, bool assign)
>  {
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 5bd6de7e8e..c5ef930876 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -621,9 +621,6 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
>  /* SIGP order code mask corresponding to bit positions 56-63 */
>  #define SIGP_ORDER_MASK 0x00ff
> 
> -/* from s390-virtio-ccw */
> -#define MAX_AVAIL_SLOTS  32
> -
>  /* machine check interruption code */
> 
>  /* subclasses */
> @@ -695,7 +692,6 @@ int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
>  int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
>  void s390_crypto_reset(void);
>  bool s390_get_squash_mcss(void);
> -int s390_get_memslot_count(void);
>  int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
>  void s390_cmma_reset(void);
>  void s390_enable_css_support(S390CPU *cpu);
> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
> index 8cdcf83845..29b10542cc 100644
> --- a/target/s390x/kvm-stub.c
> +++ b/target/s390x/kvm-stub.c
> @@ -84,11 +84,6 @@ void kvm_s390_cmma_reset(void)
>  {
>  }
> 
> -int kvm_s390_get_memslot_count(void)
> -{
> -  return MAX_AVAIL_SLOTS;
> -}
> -
>  void kvm_s390_reset_vcpu(S390CPU *cpu)
>  {
>  }
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 8ef509ece4..656aaea2cd 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1854,11 +1854,6 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier 
> *notifier, uint32_t sch,
>  return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
>  }
> 
> -int kvm_s390_get_memslot_count(void)
> -{
> -return kvm_check_extension(kvm_state, KVM_CAP_NR_MEMSLOTS);
> -}
> -
>  int kvm_s390_get_ri(void)
>  {
>  return cap_ri;
> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> index 7a3b862eea..c383bf4ee9 100644
> --- a/target/s390x/kvm_s390x.h
> +++ b/target/s390x/kvm_s390x.h
> @@ -30,7 +30,6 @@ int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t 
> *tod_clock);
>  void kvm_s390_enable_css_support(S390CPU *cpu);
>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>  int vq, bool assign);
> -int kvm_s390_get_memslot_count(void);
>  int kvm_s390_cmma_active(void);
>  void kvm_s390_cmma_reset(void);
>  void kvm_s390_reset_vcpu(S390CPU *cpu);
> 

Re: [Qemu-devel] [PATCH v2 20/32] arm/translate-a64: add FP16 FCMxx (zero) to simd_two_reg_misc_fp16

[Alex Bennée]

Richard Henderson  writes:

> On 02/22/2018 09:23 AM, Alex Bennée wrote:
>>
>> Richard Henderson  writes:
>>
>>> On 02/08/2018 09:31 AM, Alex Bennée wrote:
 +maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);
>>>
>>>   (8 << is_q) >> size
>>>
>>> ?
>>
>> Hmm I'm not so sure about this. While mine is longer form at least the
>> intent is clear. What about:
>>
>> maxpasses = (is_q ? 4 : 2) << hp
>>
>> It's still a little magical IMHO though...
>
> Two variables then?
>
>   vector_size = 8 << is_q;
>   maxpasses = vector_size >> size;
>
> Why do you want the "hp" variable when you already have "size"?

Well I had introduced hp for:

  genfn = hp ? gen_helper_advsimd_cgt_f16 : gen_helper_neon_cgt_f32;

Instead of having it long form. I guess it could be neater though.

--
Alex Bennée

Re: [Qemu-devel] [PATCH] iotests: Test abnormally large size in compressed cluster descriptor

[Alberto Garcia]

On Thu 22 Feb 2018 08:00:08 PM CET, Eric Blake wrote:
>> One consequence of this is that even if the size field is larger than
>> it needs to be QEMU can handle it just fine: it will read more data
>> from disk but it will ignore the extra bytes.
>
> (is that true even for the corner case when the size field points
> beyond the end of the image?  But not important to the meat of the
> patch)

As a matter of fact this is exactly what happens in this test
case... I'm thinking to expand it so both cases are tested.

Berto

Re: [Qemu-devel] [PATCH v2 1/3] s390x/sclp: proper support of larger send and receive masks

[Cornelia Huck]

On Thu, 22 Feb 2018 17:22:57 +0100
Claudio Imbrenda  wrote:

> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported sclp event masks of size exactly 4 bytes, even though

s/of size/of a size of/

> the archiecture allows the guests to set up sclp event masks from 1 to

s/archiecture/architecture/

> 1021 bytes in length.
> After that patch, the behaviour was almost compliant, but some issues
> were still remaining, in particular regarding the handling of selective
> reads and migration.
> 
> When setting the sclp event mask, a mask size is also specified. Until
> now we only considered the size in order to decide which bits to save
> in the internal state. On the other hand, when a guest performs a
> selective read, it sends a mask, but it does not specify a size; the
> implied size is the size of the last mask that has been set.
> 
> Specifying bits in the mask of selective read that are not available in
> the internal mask should return an error, and bits past the end of the
> mask should obviously be ignored. This can only be achieved by keeping
> track of the lenght of the mask.
> 
> The mask length is thus now part of the internal state that needs to be
> migrated.
> 
> This patch fixes the handling of selective reads, whose size will now
> match the length of the event mask, as per architecture.
> 
> While the default behaviour is to be compliant with the architecture,
> when using older machine models the old broken behaviour is selected
> (allowing only masks of size exactly 4), in order to be able to migrate
> toward older versions.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")
> Signed-off-by: Claudio Imbrenda 
> ---
>  hw/s390x/event-facility.c  | 91 
> +++---
>  hw/s390x/s390-virtio-ccw.c |  8 +++-
>  2 files changed, 85 insertions(+), 14 deletions(-)

Looks reasonable.

Re: [Qemu-devel] [PATCH v2 0/3] s390x/sclp: 64 bit event masks

[Cornelia Huck]

On Thu, 22 Feb 2018 17:22:56 +0100
Claudio Imbrenda  wrote:

> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported 32bit sclp event masks, even though the archiecture
> allows the guests to set up sclp event masks up to 1021 bytes in length.
> With that patch the behaviour was almost compliant, but some issues were
> still remaining, in particular regarding the handling of selective reads
> and migration.
> 
> This patchset fixes migration and the handling of selective reads, and
> puts in place the support for 64-bit sclp event masks internally.
> 
> A new property of the sclp-event device switches between the 32bit masks
> and the compliant behaviour. The property is bound to the machine
> version, so older machines keep the old broken behaviour, allowing for
> migration, but the default is the compliant implementation.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")
> 
> v1 -> v2 
> * improved comments and patch descriptions to better explain why we need
>   this, including better description of the old broken behaviour
> * rename SCLPEVMSK to SCLP_EVMASK to improve readability
> * removed some unneded variable initializations
> * fixed a pre-existing typo
> 
> Claudio Imbrenda (3):
>   s390x/sclp: proper support of larger send and receive masks
>   s390x/sclp: clean up sclp masks
>   s390x/sclp: extend SCLP event masks to 64 bits
> 
>  hw/char/sclpconsole-lm.c  |   4 +-
>  hw/char/sclpconsole.c |   4 +-
>  hw/s390x/event-facility.c | 150 
> +++---
>  hw/s390x/s390-virtio-ccw.c|   8 +-
>  hw/s390x/sclpcpu.c|   4 +-
>  hw/s390x/sclpquiesce.c|   4 +-
>  include/hw/s390x/event-facility.h |  22 +++---
>  7 files changed, 151 insertions(+), 45 deletions(-)
> 

This looks sane as far as I can see without having access to the
documentation.

BTW, I saw you also extended the masks in the Linux (guest) code (at
least, I see patches queued on the s390 features branch...) You also
might want to adjust the "Linux uses 4 byte masks" comment in the code
in the future.

Before I queue this, I'd like some r-bs/a-bs by folks with access to
the documentation.

Re: [Qemu-devel] [PATCH] s390x: remove s390_get_memslot_count

[David Hildenbrand]

On 23.02.2018 11:19, Cornelia Huck wrote:
> Not needed anymore after removal of the memory hotplug code.
> 
> Signed-off-by: Cornelia Huck 
> ---
>  include/hw/s390x/sclp.h  | 1 -
>  target/s390x/cpu.c   | 9 -
>  target/s390x/cpu.h   | 4 
>  target/s390x/kvm-stub.c  | 5 -
>  target/s390x/kvm.c   | 5 -
>  target/s390x/kvm_s390x.h | 1 -
>  6 files changed, 25 deletions(-)
> 
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index 476cbf78f2..f9db243484 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -35,7 +35,6 @@
>  #define SCLP_FC_ASSIGN_ATTACH_READ_STOR 0xE000ULL
>  #define SCLP_STARTING_SUBINCREMENT_ID   0x10001
>  #define SCLP_INCREMENT_UNIT 0x1
> -#define MAX_AVAIL_SLOTS 32
>  #define MAX_STORAGE_INCREMENTS  1020
>  
>  /* CPU hotplug SCLP codes */
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index f02ed19c70..627002b225 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -437,15 +437,6 @@ void s390_cmma_reset(void)
>  }
>  }
>  
> -int s390_get_memslot_count(void)
> -{
> -if (kvm_enabled()) {
> -return kvm_s390_get_memslot_count();
> -} else {
> -return MAX_AVAIL_SLOTS;
> -}
> -}
> -
>  int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
>  int vq, bool assign)
>  {
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 5bd6de7e8e..c5ef930876 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -621,9 +621,6 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
>  /* SIGP order code mask corresponding to bit positions 56-63 */
>  #define SIGP_ORDER_MASK 0x00ff
>  
> -/* from s390-virtio-ccw */
> -#define MAX_AVAIL_SLOTS  32
> -
>  /* machine check interruption code */
>  
>  /* subclasses */
> @@ -695,7 +692,6 @@ int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
>  int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
>  void s390_crypto_reset(void);
>  bool s390_get_squash_mcss(void);
> -int s390_get_memslot_count(void);
>  int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
>  void s390_cmma_reset(void);
>  void s390_enable_css_support(S390CPU *cpu);
> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
> index 8cdcf83845..29b10542cc 100644
> --- a/target/s390x/kvm-stub.c
> +++ b/target/s390x/kvm-stub.c
> @@ -84,11 +84,6 @@ void kvm_s390_cmma_reset(void)
>  {
>  }
>  
> -int kvm_s390_get_memslot_count(void)
> -{
> -  return MAX_AVAIL_SLOTS;
> -}
> -
>  void kvm_s390_reset_vcpu(S390CPU *cpu)
>  {
>  }
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 8ef509ece4..656aaea2cd 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1854,11 +1854,6 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier 
> *notifier, uint32_t sch,
>  return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
>  }
>  
> -int kvm_s390_get_memslot_count(void)
> -{
> -return kvm_check_extension(kvm_state, KVM_CAP_NR_MEMSLOTS);
> -}
> -
>  int kvm_s390_get_ri(void)
>  {
>  return cap_ri;
> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> index 7a3b862eea..c383bf4ee9 100644
> --- a/target/s390x/kvm_s390x.h
> +++ b/target/s390x/kvm_s390x.h
> @@ -30,7 +30,6 @@ int kvm_s390_set_clock_ext(uint8_t *tod_high, uint64_t 
> *tod_clock);
>  void kvm_s390_enable_css_support(S390CPU *cpu);
>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>  int vq, bool assign);
> -int kvm_s390_get_memslot_count(void);
>  int kvm_s390_cmma_active(void);
>  void kvm_s390_cmma_reset(void);
>  void kvm_s390_reset_vcpu(S390CPU *cpu);
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 1/3] s390x/sclp: proper support of larger send and receive masks

[Claudio Imbrenda]

On Fri, 23 Feb 2018 11:31:46 +0100
Cornelia Huck  wrote:

> On Thu, 22 Feb 2018 17:22:57 +0100
> Claudio Imbrenda  wrote:
> 
> > Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > event masks") we only supported sclp event masks of size exactly 4
> > bytes, even though  
> 
> s/of size/of a size of/

will fix

> > the archiecture allows the guests to set up sclp event masks from 1
> > to  
> 
> s/archiecture/architecture/

will fix

> > 1021 bytes in length.
> > After that patch, the behaviour was almost compliant, but some
> > issues were still remaining, in particular regarding the handling
> > of selective reads and migration.
> > 
> > When setting the sclp event mask, a mask size is also specified.
> > Until now we only considered the size in order to decide which bits
> > to save in the internal state. On the other hand, when a guest
> > performs a selective read, it sends a mask, but it does not specify
> > a size; the implied size is the size of the last mask that has been
> > set.
> > 
> > Specifying bits in the mask of selective read that are not
> > available in the internal mask should return an error, and bits
> > past the end of the mask should obviously be ignored. This can only
> > be achieved by keeping track of the lenght of the mask.
> > 
> > The mask length is thus now part of the internal state that needs
> > to be migrated.
> > 
> > This patch fixes the handling of selective reads, whose size will
> > now match the length of the event mask, as per architecture.
> > 
> > While the default behaviour is to be compliant with the
> > architecture, when using older machine models the old broken
> > behaviour is selected (allowing only masks of size exactly 4), in
> > order to be able to migrate toward older versions.
> > 
> > Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > event masks") Signed-off-by: Claudio Imbrenda
> >  ---
> >  hw/s390x/event-facility.c  | 91
> > +++---
> > hw/s390x/s390-virtio-ccw.c |  8 +++- 2 files changed, 85
> > insertions(+), 14 deletions(-)  
> 
> Looks reasonable.
> 

Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage

[Paolo Bonzini]

On 22/02/2018 18:06, John Snow wrote:
> 
> 
> On 02/22/2018 05:57 AM, Kevin Wolf wrote:
>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>> On 20/02/2018 18:04, Peter Lieven wrote:
 Hi,

 I remember we discussed a long time ago to limit the stack usage of all
 functions that are executed in a coroutine
 context to a very low value to be able to safely limit the coroutine
 stack size as well.
>>>
>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>> ide_atapi_cmd_reply_end.
>>>
>>> But perhaps it's not an issue, somebody needs to audit the code.
>>
>> I think John intended to get rid of the recursion sometime, but I doubt
>> he has had the time so far.
>>
> 
> It hasn't been a priority for me.
> 
> Paolo tried to fix ATAPI by adding a BH callback, but that added the
> possibility of a migration halfway through a data transfer IIRC.
> 
> If anyone wants to tackle it, I'll dig up Paolo's patches.

A better possibility is to make it into tail recursion first and then a
while loop.  Maybe introducing some kind of ide_transfer_start_norecurse
that returns "true" if you have a start_transfer callback (so you need
to do another iteration immediately) and "false" if you don't.  I'll
take a look...

Paolo

Re: [Qemu-devel] [PATCH v2 0/3] s390x/sclp: 64 bit event masks

[Claudio Imbrenda]

On Fri, 23 Feb 2018 11:37:55 +0100
Cornelia Huck  wrote:

> On Thu, 22 Feb 2018 17:22:56 +0100
> Claudio Imbrenda  wrote:
> 
> > Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > event masks") we only supported 32bit sclp event masks, even though
> > the archiecture allows the guests to set up sclp event masks up to
> > 1021 bytes in length. With that patch the behaviour was almost
> > compliant, but some issues were still remaining, in particular
> > regarding the handling of selective reads and migration.
> > 
> > This patchset fixes migration and the handling of selective reads,
> > and puts in place the support for 64-bit sclp event masks
> > internally.
> > 
> > A new property of the sclp-event device switches between the 32bit
> > masks and the compliant behaviour. The property is bound to the
> > machine version, so older machines keep the old broken behaviour,
> > allowing for migration, but the default is the compliant
> > implementation.
> > 
> > Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > event masks")
> > 
> > v1 -> v2 
> > * improved comments and patch descriptions to better explain why we
> > need this, including better description of the old broken behaviour
> > * rename SCLPEVMSK to SCLP_EVMASK to improve readability
> > * removed some unneded variable initializations
> > * fixed a pre-existing typo
> > 
> > Claudio Imbrenda (3):
> >   s390x/sclp: proper support of larger send and receive masks
> >   s390x/sclp: clean up sclp masks
> >   s390x/sclp: extend SCLP event masks to 64 bits
> > 
> >  hw/char/sclpconsole-lm.c  |   4 +-
> >  hw/char/sclpconsole.c |   4 +-
> >  hw/s390x/event-facility.c | 150
> > +++---
> > hw/s390x/s390-virtio-ccw.c|   8 +-
> > hw/s390x/sclpcpu.c|   4 +-
> > hw/s390x/sclpquiesce.c|   4 +-
> > include/hw/s390x/event-facility.h |  22 +++--- 7 files changed, 151
> > insertions(+), 45 deletions(-) 
> 
> This looks sane as far as I can see without having access to the
> documentation.
> 
> BTW, I saw you also extended the masks in the Linux (guest) code (at
> least, I see patches queued on the s390 features branch...) You also
> might want to adjust the "Linux uses 4 byte masks" comment in the code
> in the future.

will fix, so that I don't have to remember to do stuff in the future :)
 
> Before I queue this, I'd like some r-bs/a-bs by folks with access to
> the documentation.
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 08/67] target/arm: Implement SVE Predicate Misc Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/cpu.h   |   3 +
>  target/arm/helper-sve.h|   3 +
>  target/arm/sve_helper.c|  86 +++-
>  target/arm/translate-sve.c | 163 
> -
>  target/arm/sve.decode  |  41 
>  5 files changed, 293 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 8befe43a01..27f395183b 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2915,4 +2915,7 @@ static inline uint64_t *aa64_vfp_qreg(CPUARMState *env, 
> unsigned regno)
>  return &env->vfp.zregs[regno].d[0];
>  }
>
> +/* Shared between translate-sve.c and sve_helper.c.  */
> +extern const uint64_t pred_esz_masks[4];
> +
>  #endif
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index 57adc4d912..0c04afff8c 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -20,6 +20,9 @@
>  DEF_HELPER_FLAGS_2(sve_predtest1, TCG_CALL_NO_WG, i32, i64, i64)
>  DEF_HELPER_FLAGS_3(sve_predtest, TCG_CALL_NO_WG, i32, ptr, ptr, i32)
>
> +DEF_HELPER_FLAGS_3(sve_pfirst, TCG_CALL_NO_WG, i32, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_3(sve_pnext, TCG_CALL_NO_WG, i32, ptr, ptr, i32)
> +
>  DEF_HELPER_FLAGS_5(sve_and_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_bic_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_eor_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index b63e7cc90e..cee7d9bcf6 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -39,7 +39,7 @@
>
>  static uint32_t iter_predtest_fwd(uint64_t d, uint64_t g, uint32_t flags)
>  {
> -if (g) {
> +if (likely(g)) {
>  /* Compute N from first D & G.
> Use bit 2 to signal first G bit seen.  */
>  if (!(flags & 4)) {

Belongs in different patch ?

> @@ -114,3 +114,87 @@ LOGICAL_(sve_nand_, DO_NAND)
>  #undef DO_NAND
>  #undef DO_SEL
>  #undef LOGICAL_
> +
> +/* Similar to the ARM LastActiveElement pseudocode function, except the
> +   result is multiplied by the element size.  This includes the not found
> +   indication; e.g. not found for esz=3 is -8.  */

Can we stick to the usual format for multiline comments, please?
(various examples here and elsewhere in the patchset). I know that
over the whole codebase we're a bit variable, but I think this is
the most common arrangement and it's definitely the one we use
in target/arm with perhaps the odd ancient comment as an exception.

/* line 1
 * line 2
 */


> +static void trans_PTRUE(DisasContext *s, arg_PTRUE *a, uint32_t insn)
> +{
> +unsigned fullsz = vec_full_reg_size(s);
> +unsigned ofs = pred_full_reg_offset(s, a->rd);
> +unsigned numelem, setsz, i;
> +uint64_t word, lastword;
> +TCGv_i64 t;

A comment somewhere here about the way this code handles
the instructions that aren't PTRUE would be helpful I think
(specifically that a->pat is 32 for PFALSE and a->rd is
16 for SETFFR).

> +
> +numelem = decode_pred_count(fullsz, a->pat, a->esz);
> +
> +/* Determine what we must store into each bit, and how many.  */
> +if (numelem == 0) {
> +lastword = word = 0;
> +setsz = fullsz;
> +} else {
> +setsz = numelem << a->esz;
> +lastword = word = pred_esz_masks[a->esz];
> +if (setsz % 64) {
> +lastword &= ~(-1ull << (setsz % 64));
> +}
> +}
> +

>  ###
>  # Named instruction formats.  These are generally used to
>  # reduce the amount of duplication between instruction patterns.
>
> +# Two operand with unused vector element size
> +@pd_pn_e0    ... rn:4 . rd:4   &rr_esz esz=0
> +
> +# Two operand
> +@pd_pn  esz:2 ..  ... rn:4 . rd:4  &rr_esz
> +
>  # Three operand with unused vector element size
>  @rd_rn_rm_e0    ... rm:5 ... ... rn:5 rd:5 &rrr_esz esz=0
>
> @@ -77,6 +87,37 @@ NAND_00100101 1. 00  01  1  1  
>   @pd_pg_pn_pm_s
>  # SVE predicate test
>  PTEST  00100101 0101 11 pg:4 0 rn:4 0
>
> +# SVE predicate initialize
> +PTRUE  00100101 esz:2 01100 s:1 111000 pat:5 0 rd:4&ptrue
> +
> +# SVE initialize FFR (SETFFR)
> +PTRUE  00100101 0010 1100 1001    \
> +   &ptrue rd=16 esz=0 pat=31 s=0

I found this very confusing at first, because the leftmost column
looks like it's the instruction name, and thus a copy-and-paste
error. I think it would be easier to read if we gave it a name
that indicates that it's dealing with a group of instructions
rather than only PTRUE.

> +
> +# SVE zero predicate register (PFALSE)
> +# Note that pat=32 is outside of the natural 0..31

[Qemu-devel] [PATCH] loader: don't perform overlapping address check for memory region ROM images

[Mark Cave-Ayland]

All memory region ROM images have a base address of 0 which causes the 
overlapping
address check to fail if more than one memory region ROM image is present, or an
existing ROM image is loaded at address 0.

Make sure that we ignore the overlapping address check in
rom_check_and_register_reset() if this is a memory region ROM image. In 
particular
this fixes the "rom: requested regions overlap" error on startup when trying to
run qemu-system-sparc with a -kernel image since commit 7497638642: "tcx: 
switch to
load_image_mr() and remove prom_addr hack".

Suggested-by: Peter Maydell 
Signed-off-by: Mark Cave-Ayland 
---
 hw/core/loader.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 91669d65aa..c08f130461 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1104,20 +1104,22 @@ int rom_check_and_register_reset(void)
 if (rom->fw_file) {
 continue;
 }
-if ((addr > rom->addr) && (as == rom->as)) {
-fprintf(stderr, "rom: requested regions overlap "
-"(rom %s. free=0x" TARGET_FMT_plx
-", addr=0x" TARGET_FMT_plx ")\n",
-rom->name, addr, rom->addr);
-return -1;
+if (!rom->mr) {
+if ((addr > rom->addr) && (as == rom->as)) {
+fprintf(stderr, "rom: requested regions overlap "
+"(rom %s. free=0x" TARGET_FMT_plx
+", addr=0x" TARGET_FMT_plx ")\n",
+rom->name, addr, rom->addr);
+return -1;
+}
+addr  = rom->addr;
+addr += rom->romsize;
+as = rom->as;
 }
-addr  = rom->addr;
-addr += rom->romsize;
 section = memory_region_find(rom->mr ? rom->mr : get_system_memory(),
  rom->addr, 1);
 rom->isrom = int128_nz(section.size) && 
memory_region_is_rom(section.mr);
 memory_region_unref(section.mr);
-as = rom->as;
 }
 qemu_register_reset(rom_reset, NULL);
 roms_loaded = 1;
-- 
2.11.0

Re: [Qemu-devel] [PATCH qemu repost] qmp: Add qom-list-properties to list QOM object properties

[Paolo Bonzini]

On 22/02/2018 06:00, Alexey Kardashevskiy wrote:
> There is already 'device-list-properties' which does most of the job,
> however it does not handle everything returned by qom-list-types such
> as machines as they inherit directly from TYPE_OBJECT and not TYPE_DEVICE.
> 
> This adds a new qom-list-properties command which prints properties
> of a specific class and its instance. It is pretty much a simplified copy
> of the device-list-properties handler.
> 
> Since it creates an object instance, device properties should appear
> in the output as they are copied to QOM properties at the instance_init
> hook.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> 
> This is a simple rebase on top of the current upstream.
> 
> 
> ---
>  qapi-schema.json | 29 +
>  qmp.c| 52 
>  2 files changed, 81 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0262b9f..fa5f189 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1455,6 +1455,35 @@
>'returns': [ 'DevicePropertyInfo' ] }
>  
>  ##
> +# @QOMPropertyInfo:
> +#
> +# Information about object properties.
> +#
> +# @name: the name of the property
> +# @type: the typename of the property
> +# @description: if specified, the description of the property.
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'QOMPropertyInfo',
> +  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
> +
> +##
> +# @qom-list-properties:
> +#
> +# List properties associated with a QOM object.
> +#
> +# @typename: the type name of an object
> +#
> +# Returns: a list of QOMPropertyInfo describing object properties
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'qom-list-properties',
> +  'data': { 'typename': 'str'},
> +  'returns': [ 'QOMPropertyInfo' ] }
> +
> +##
>  # @xen-set-global-dirty-log:
>  #
>  # Enable or disable the global dirty log mode.
> diff --git a/qmp.c b/qmp.c
> index 793f6f3..f2d4781 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -576,6 +576,58 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
> char *typename,
>  return prop_list;
>  }
>  
> +QOMPropertyInfoList *qmp_qom_list_properties(const char *typename,
> + Error **errp)
> +{
> +ObjectClass *klass;
> +Object *obj;
> +ObjectProperty *prop;
> +ObjectPropertyIterator iter;
> +QOMPropertyInfoList *prop_list = NULL;
> +
> +klass = object_class_by_name(typename);
> +if (klass == NULL) {
> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +  "Class '%s' not found", typename);
> +return NULL;
> +}
> +
> +klass = object_class_dynamic_cast(klass, TYPE_OBJECT);
> +if (klass == NULL) {
> +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", 
> TYPE_OBJECT);
> +return NULL;
> +}
> +
> +if (object_class_is_abstract(klass)) {
> +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
> +   "non-abstract class");

This sucks, it would be nice to enumerate properties on base classes. We
could list class properties only.  However, it can be improved later.

Queued, thanks.

Paolo

> +return NULL;
> +}
> +
> +obj = object_new(typename);
> +
> +object_property_iter_init(&iter, obj);
> +while ((prop = object_property_iter_next(&iter))) {
> +QOMPropertyInfo *info;
> +QOMPropertyInfoList *entry;
> +
> +info = g_malloc0(sizeof(*info));
> +info->name = g_strdup(prop->name);
> +info->type = g_strdup(prop->type);
> +info->has_description = !!prop->description;
> +info->description = g_strdup(prop->description);
> +
> +entry = g_malloc0(sizeof(*entry));
> +entry->value = info;
> +entry->next = prop_list;
> +prop_list = entry;
> +}
> +
> +object_unref(obj);
> +
> +return prop_list;
> +}
> +
>  CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
>  {
>  return arch_query_cpu_definitions(errp);
> 

Re: [Qemu-devel] [PATCH v2 09/67] target/arm: Implement SVE Integer Binary Arithmetic - Predicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 145 +
>  target/arm/sve_helper.c| 196 
> -
>  target/arm/translate-sve.c |  65 +++
>  target/arm/sve.decode  |  42 ++
>  4 files changed, 447 insertions(+), 1 deletion(-)
>

> @@ -105,7 +121,7 @@ LOGICAL_(sve_orn_, DO_ORN)
>  LOGICAL_(sve_nor_, DO_NOR)
>  LOGICAL_(sve_nand_, DO_NAND)
>
> -#undef DO_ADD
> +#undef DO_AND

Should this be in a previous patch?

>  #undef DO_BIC
>  #undef DO_EOR
>  #undef DO_ORR

> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index a9b6ae046d..116002792a 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -211,6 +211,71 @@ static void trans_BIC_zzz(DisasContext *s, arg_BIC_zzz 
> *a, uint32_t insn)
>  do_vector3_z(s, tcg_gen_gvec_andc, 0, a->rd, a->rn, a->rm);
>  }
>
> +/*
> + *** SVE Integer Arithmetic - Binary Predicated Group
> + */
> +
> +static void do_zpzz_ool(DisasContext *s, arg_rprr_esz *a, gen_helper_gvec_4 
> *fn)
> +{
> +unsigned vsz = vec_full_reg_size(s);
> +if (fn == NULL) {
> +unallocated_encoding(s);
> +return;
> +}

I think you do not want to be catching unallocated encodings
this late in the decode process. We have to identify all
the unallocated encodings before we do the "are SVE and
FP instructions supposed to trap" tests, because those don't
apply to unallocated encodings.

> +tcg_gen_gvec_4_ool(vec_full_reg_offset(s, a->rd),
> +   vec_full_reg_offset(s, a->rn),
> +   vec_full_reg_offset(s, a->rm),
> +   pred_full_reg_offset(s, a->pg),
> +   vsz, vsz, 0, fn);
> +}

Rest of patch looks OK.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Paolo Bonzini]

On 15/02/2018 22:25, Marc-André Lureau wrote:
> Hi,
> 
> The following patches fix a regression introduced in commit
> 218bb57dd79d that prevent ASAN from being detected & used.  There is
> also a works around for a GCC ASAN optimization bug. A few test leaks
> are fixed, and a few patches reenable vhost-user memfd test fixing the
> recent race bug that was identified in the previous version.
> 
> Marc-André Lureau (6):
>   build-sys: fix -fsanitize=address check
>   lockable: workaround GCC link issue with ASAN
>   vhost-user-test: add back memfd check
>   vhost-user-test: do not hang if chardev creation failed
>   ahci-test: fix opts leak of skip tests
>   sdhci-test: fix leaks
> 
>  include/qemu/lockable.h |  2 +-
>  tests/ahci-test.c   |  1 +
>  tests/sdhci-test.c  |  2 ++
>  tests/vhost-user-test.c | 94 
> +++--
>  configure   | 22 ++--
>  5 files changed, 83 insertions(+), 38 deletions(-)
> 

Queued 1-2-5-6, not really confident enough in what's going on with
vhost-user-test. :)

Paolo

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 03/67] target/arm: Add SVE decode skeleton

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Including only 4, as-yet unimplemented, instruction patterns
> so that the whole thing compiles.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-a64.c | 11 +++-
>  target/arm/translate-sve.c | 63 
> ++
>  .gitignore |  1 +
>  target/arm/Makefile.objs   | 10 
>  target/arm/sve.decode  | 45 +
>  5 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644 target/arm/translate-sve.c
>  create mode 100644 target/arm/sve.decode
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index e0e7ebf68c..a50fef98af 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -12772,9 +12772,18 @@ static void disas_a64_insn(CPUARMState *env, 
> DisasContext *s)
>  s->fp_access_checked = false;
>
>  switch (extract32(insn, 25, 4)) {
> -case 0x0: case 0x1: case 0x2: case 0x3: /* UNALLOCATED */
> +case 0x0: case 0x1: case 0x3: /* UNALLOCATED */
>  unallocated_encoding(s);
>  break;
> +case 0x2:
> +if (!arm_dc_feature(s, ARM_FEATURE_SVE)) {
> +unallocated_encoding(s);
> +} else if (!sve_access_check(s) || !fp_access_check(s)) {
> +/* exception raised */
> +} else if (!disas_sve(s, insn)) {
> +unallocated_encoding(s);
> +}
> +break;

I realized while working through the rest of the series that this is
too early to do the sve_access_check() and fp_access_check(). Those
only apply to instructions which actually exist, so we mustn't
do the checks until after we've dealt with all the unallocated_encoding()
cases. I think you need to push them down inside disas_sve() somehow.
See also my comments on patch 8.

(We get this wrong for current AArch32 VFP and Neon, but correct
for all of AArch64.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] Add decodetree.py

[Peter Maydell]

On 22 February 2018 at 23:49, Richard Henderson
 wrote:
> The following changes since commit ff8689611a1d954897d857b28f7ef404e11cfa2c:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-signed' 
> into staging (2018-02-22 11:37:05 +)
>
> are available in the Git repository at:
>
>   git://github.com/rth7680/qemu.git tags/pull-decode-20180222
>
> for you to fetch changes up to 568ae7efae7e4b90e213049efb8b6e4e12b47ca3:
>
>   scripts: Add decodetree.py (2018-02-22 15:44:07 -0800)
>
> 
> Add decodetree.py
>
> 
> Richard Henderson (1):
>   scripts: Add decodetree.py

Applied, thanks.

-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 03/67] target/arm: Add SVE decode skeleton

[Peter Maydell]

On 23 February 2018 at 11:40, Peter Maydell  wrote:
> I realized while working through the rest of the series that this is
> too early to do the sve_access_check() and fp_access_check(). Those
> only apply to instructions which actually exist, so we mustn't
> do the checks until after we've dealt with all the unallocated_encoding()
> cases. I think you need to push them down inside disas_sve() somehow.
> See also my comments on patch 8.

...I meant "patch 9".

-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 10/67] target/arm: Implement SVE Integer Reduction Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Excepting MOVPRFX, which isn't a reduction.  Presumably it is
> placed within the group because of its encoding.
>
> Signed-off-by: Richard Henderson 


> @@ -306,8 +399,6 @@ DO_ZPZZ_D(sve_udiv_zpzz_d, uint64_t, DO_DIV)
>  #undef DO_ABD
>  #undef DO_MUL
>  #undef DO_DIV
> -#undef DO_ZPZZ
> -#undef DO_ZPZZ_D
>
>  /* Similar to the ARM LastActiveElement pseudocode function, except the
> result is multiplied by the element size.  This includes the not found

Hunk in wrong patch or incorrect?

> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index 116002792a..49251a53c1 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -276,6 +276,71 @@ void trans_UDIV_zpzz(DisasContext *s, arg_rprr_esz *a, 
> uint32_t insn)
>
>  #undef DO_ZPZZ
>
> +/*
> + *** SVE Integer Reduction Group
> + */
> +
> +typedef void gen_helper_gvec_reduc(TCGv_i64, TCGv_ptr, TCGv_ptr, TCGv_i32);
> +static void do_vpz_ool(DisasContext *s, arg_rpr_esz *a,
> +   gen_helper_gvec_reduc *fn)
> +{
> +unsigned vsz = vec_full_reg_size(s);
> +TCGv_ptr t_zn, t_pg;
> +TCGv_i32 desc;
> +TCGv_i64 temp;
> +
> +if (fn == 0) {
> +unallocated_encoding(s);
> +return;
> +}

Same remarks as for patch 9 about this being too late to
catch unallocated_encodings (or alternatively needing to
do the sve/fp check after this). I won't bother to mention
this issue for later patches, but you can assume it as a
general caveat to my reviewed-by tags.

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Viktor Mihajlovski]

On 23.02.2018 11:17, Thomas Huth wrote:
> On 23.02.2018 09:53, Christian Borntraeger wrote:
>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if 
>> I do not 
>> specify loadparm or boot menu:
>>
>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 
>> -device virtio-blk-ccw,drive=d1,bootindex=1 
> 
> I can reproduce this now, too. FWIW: It only happens with
> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
> first). Collin, can you reproduce this, too?
> 
>  Thomas
> The idea is to support the zipl boot menu on ECKD disks (which have a timeout 
> by themselves)
even without switching the boot menu on. This is to have the same behavior as 
one would see
on LPAR or z/VM.
The issue is that the boot code tries to interpret the program table of SCSI 
bootmaps as if
a boot menu has been specified.

The following patch fixes this. Collin might want to have a say in this matter 
as well.


The scsi program table was erroneously evaluated as implicit
boot menu. This patch adds a per-bootmap-type menu_is_enabled
function.

Signed-off-by: Viktor Mihajlovski 
---
 pc-bios/s390-ccw/bootmap.c  | 4 ++--
 pc-bios/s390-ccw/menu.c | 7 ++-
 pc-bios/s390-ccw/s390-ccw.h | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 81dbd4a..b6fd77e 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -265,7 +265,7 @@ static void run_eckd_boot_script(block_number_t 
bmt_block_nr,
 BootMapTable *bmt = (void *)sec;
 BootMapScript *bms = (void *)sec;
 
-if (menu_is_enabled()) {
+if (menu_is_enabled_for_zipl()) {
 loadparm = eckd_get_boot_menu_index(s1b_block_nr);
 }
 
@@ -568,7 +568,7 @@ static void ipl_scsi(void)
 debug_print_int("program table entries", program_table_entries);
 IPL_assert(program_table_entries != 0, "Empty Program Table");
 
-if (menu_is_enabled()) {
+if (menu_is_enabled_for_enum()) {
 loadparm = menu_get_enum_boot_index(program_table_entries);
 }
 
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 16b5310..ebe5e9f 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -237,7 +237,12 @@ void menu_set_parms(uint8_t boot_menu_flag, uint32_t 
boot_menu_timeout)
 timeout = boot_menu_timeout;
 }
 
-bool menu_is_enabled(void)
+bool menu_is_enabled_for_zipl(void)
 {
 return flag;
 }
+
+bool menu_is_enabled_for_enum(void)
+{
+return flag & ~QIPL_FLAG_BM_OPTS_ZIPL;
+}
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index cfcaf3c..a18381f 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -89,7 +89,8 @@ void zipl_load(void);
 
 /* menu.c */
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
-bool menu_is_enabled(void);
+bool menu_is_enabled_for_zipl(void);
+bool menu_is_enabled_for_enum(void);
 int menu_get_zipl_boot_index(const char *menu_data);
 int menu_get_enum_boot_index(int entries);
 
-- 
1.9.1


-- 
Regards,
 Viktor Mihajlovski

Re: [Qemu-devel] [PATCH v2 11/32] arm/translate-a64: add FP16 F[A]C[EQ/GE/GT] to simd_three_reg_same_fp16

[Alex Bennée]

Richard Henderson  writes:

> On 02/08/2018 09:31 AM, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée 
>> ---
>>  target/arm/helper-a64.c| 49 
>> ++
>>  target/arm/helper-a64.h|  5 +
>>  target/arm/translate-a64.c | 15 ++
>>  3 files changed, 69 insertions(+)
>>
>> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
>> index 25e45121af..78eeda31d1 100644
>> --- a/target/arm/helper-a64.c
>> +++ b/target/arm/helper-a64.c
>> @@ -599,3 +599,52 @@ ADVSIMD_HALFOP(min)
>>  ADVSIMD_HALFOP(max)
>>  ADVSIMD_HALFOP(minnum)
>>  ADVSIMD_HALFOP(maxnum)
>> +
>> +/*
>> + * Floating point comparisons produce an integer result. Softfloat
>> + * routines return float_relation types which we convert to the 0/-1
>> + * Neon requires.
>> + */
>> +
>> +#define ADVSIMD_CMPRES(test) (test) ? 0x : 0
>> +
>> +uint32_t HELPER(advsimd_ceq_f16)(float16 a, float16 b, void *fpstp)
>> +{
>> +float_status *fpst = fpstp;
>> +int compare = float16_compare_quiet(a, b, fpst);
>> +return ADVSIMD_CMPRES(compare == float_relation_equal);
>
> Not using float16_eq etc?

These don't actually exist. But I guess we could make stubs for them
based on the generic float_compare support. But would it buy us much?

>
>> +}
>> +
>> +uint32_t HELPER(advsimd_cge_f16)(float16 a, float16 b, void *fpstp)
>> +{
>> +float_status *fpst = fpstp;
>> +int compare = float16_compare(a, b, fpst);
>> +return ADVSIMD_CMPRES(compare == float_relation_greater ||
>> +  compare == float_relation_equal);
>
> Especially float16_le(b, a, fpst).
>
> Otherwise,
>
> Reviewed-by: Richard Henderson 
>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 11/67] target/arm: Implement SVE bitwise shift by immediate (predicated)

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  25 +
>  target/arm/sve_helper.c| 265 
> +
>  target/arm/translate-sve.c | 128 ++
>  target/arm/sve.decode  |  29 -
>  4 files changed, 445 insertions(+), 2 deletions(-)

>
> +/*
> + * Helpers for extracting complex instruction fields.
> + */
> +
> +/* See e.g. ASL (immediate, predicated).

Typo for "ASR", I guess ?

> + * Returns -1 for unallocated encoding; diagnose later.
> + */
> +static int tszimm_esz(int x)
> +{
> +x >>= 3;  /* discard imm3 */
> +return 31 - clz32(x);
> +}
> +
> +static int tszimm_shr(int x)
> +{
> +return (16 << tszimm_esz(x)) - x;
> +}
> +
> +/* See e.g. LSL (immediate, predicated).  */
> +static int tszimm_shl(int x)
> +{
> +return x - (8 << tszimm_esz(x));
> +}

> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -22,12 +22,20 @@
>  ###
>  # Named fields.  These are primarily for disjoint fields.
>
> +%imm6_22_5 22:1 5:5
>  %imm9_16_1016:s6 10:3
>  %preg4_5   5:4
>
> +# A combination of tsz:imm3 -- extract esize.
> +%tszimm_esz22:2 5:5 !function=tszimm_esz
> +# A combination of tsz:imm3 -- extract (2 * esize) - (tsz:imm3)
> +%tszimm_shr22:2 5:5 !function=tszimm_shr
> +# A combination of tsz:imm3 -- extract (tsz:imm3) - esize
> +%tszimm_shl22:2 5:5 !function=tszimm_shl
> +
>  # Either a copy of rd (at bit 0), or a different source
>  # as propagated via the MOVPRFX instruction.
> -%reg_movprfx   0:5
> +%reg_movprfx   0:5

Squash into relevant previous patch.

>  ###
>  # Named attribute sets.  These are used to make nice(er) names
> @@ -40,7 +48,7 @@
>  &rpr_esz   rd pg rn esz
>  &rprr_srd pg rn rm s
>  &rprr_esz  rd pg rn rm esz
> -
> +&rpri_esz  rd pg rn imm esz

Should either not delete the blank line, or don't add it in the first place.

>  &ptrue rd esz pat s
>
>  ###
> @@ -68,6 +76,11 @@
>  # One register operand, with governing predicate, vector element size
>  @rd_pg_rn   esz:2 ... ... ... pg:3 rn:5 rd:5   &rpr_esz
>
> +# Two register operand, one immediate operand, with predicate,
> +# element size encoded as TSZHL.  User must fill in imm.
> +@rdn_pg_tszimm  .. ... ... ... pg:3 . rd:5 \
> +   &rpri_esz rn=%reg_movprfx esz=%tszimm_esz
> +
>  # Basic Load/Store with 9-bit immediate offset
>  @pd_rn_i9    .. rn:5 . rd:4\
> &rri imm=%imm9_16_10
> @@ -126,6 +139,18 @@ UMAXV  0100 .. 001 001 001 ... . 
> . @rd_pg_rn
>  SMINV  0100 .. 001 010 001 ... . . @rd_pg_rn
>  UMINV  0100 .. 001 011 001 ... . . @rd_pg_rn
>
> +### SVE Shift by Immediate - Predicated Group
> +
> +# SVE bitwise shift by immediate (predicated)
> +ASR_zpzi   0100 .. 000 000 100 ... .. ... . \
> +   @rdn_pg_tszimm imm=%tszimm_shr
> +LSR_zpzi   0100 .. 000 001 100 ... .. ... . \
> +   @rdn_pg_tszimm imm=%tszimm_shr
> +LSL_zpzi   0100 .. 000 011 100 ... .. ... . \
> +   @rdn_pg_tszimm imm=%tszimm_shl
> +ASRD   0100 .. 000 100 100 ... .. ... . \
> +   @rdn_pg_tszimm imm=%tszimm_shr
> +
>  ### SVE Logical - Unpredicated Group
>
>  # SVE bitwise logical operations (unpredicated)
> --
> 2.14.3

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Marc-André Lureau]

Hi

On Fri, Feb 23, 2018 at 12:37 PM, Paolo Bonzini  wrote:
> On 15/02/2018 22:25, Marc-André Lureau wrote:
>> Hi,
>>
>> The following patches fix a regression introduced in commit
>> 218bb57dd79d that prevent ASAN from being detected & used.  There is
>> also a works around for a GCC ASAN optimization bug. A few test leaks
>> are fixed, and a few patches reenable vhost-user memfd test fixing the
>> recent race bug that was identified in the previous version.
>>
>> Marc-André Lureau (6):
>>   build-sys: fix -fsanitize=address check
>>   lockable: workaround GCC link issue with ASAN
>>   vhost-user-test: add back memfd check
>>   vhost-user-test: do not hang if chardev creation failed
>>   ahci-test: fix opts leak of skip tests
>>   sdhci-test: fix leaks
>>
>>  include/qemu/lockable.h |  2 +-
>>  tests/ahci-test.c   |  1 +
>>  tests/sdhci-test.c  |  2 ++
>>  tests/vhost-user-test.c | 94 
>> +++--
>>  configure   | 22 ++--
>>  5 files changed, 83 insertions(+), 38 deletions(-)
>>
>
> Queued 1-2-5-6, not really confident enough in what's going on with
> vhost-user-test. :)

It hangs when chardev creation fails. Which is apparent when running
with --debug-log. That's also why I added "vhost-user-test: do not
hang if chardev creation failed". The problem comes from reusing
socket location without waiting for the idle cleanup. Using different
socket path solves it (vhost-user-test: add back memfd check)

Maxime, could you help review those patches?

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] s390x: remove s390_get_memslot_count

[Cornelia Huck]

On Fri, 23 Feb 2018 11:19:19 +0100
Cornelia Huck  wrote:

> Not needed anymore after removal of the memory hotplug code.
> 
> Signed-off-by: Cornelia Huck 
> ---
>  include/hw/s390x/sclp.h  | 1 -
>  target/s390x/cpu.c   | 9 -
>  target/s390x/cpu.h   | 4 
>  target/s390x/kvm-stub.c  | 5 -
>  target/s390x/kvm.c   | 5 -
>  target/s390x/kvm_s390x.h | 1 -
>  6 files changed, 25 deletions(-)

Queued to s390-next.

[Qemu-devel] [PATCH v3 0/3] s390x/sclp: 64 bit event masks

[Claudio Imbrenda]

Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
we only supported 32bit sclp event masks, even though the archiecture
allows the guests to set up sclp event masks up to 1021 bytes in length.
With that patch the behaviour was almost compliant, but some issues were
still remaining, in particular regarding the handling of selective reads
and migration.

This patchset fixes migration and the handling of selective reads, and
puts in place the support for 64-bit sclp event masks internally.

A new property of the sclp-event device switches between the 32bit masks
and the compliant behaviour. The property is bound to the machine
version, so older machines keep the old broken behaviour, allowing for
migration, but the default is the compliant implementation.

Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")

v2 -> v3
* fixed some typos in the first patch description
* updated an existing comment in the third patch: newer Linux versions
  will support masks larger than 4 bytes.

v1 -> v2 
* improved comments and patch descriptions to better explain why we need
  this, including better description of the old broken behaviour
* rename SCLPEVMSK to SCLP_EVMASK to improve readability
* removed some unneded variable initializations
* fixed a pre-existing typo

Claudio Imbrenda (3):
  s390x/sclp: proper support of larger send and receive masks
  s390x/sclp: clean up sclp masks
  s390x/sclp: extend SCLP event masks to 64 bits

 hw/char/sclpconsole-lm.c  |   4 +-
 hw/char/sclpconsole.c |   4 +-
 hw/s390x/event-facility.c | 157 ++
 hw/s390x/s390-virtio-ccw.c|   8 +-
 hw/s390x/sclpcpu.c|   4 +-
 hw/s390x/sclpquiesce.c|   4 +-
 include/hw/s390x/event-facility.h |  22 +++---
 7 files changed, 155 insertions(+), 48 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 12/67] target/arm: Implement SVE bitwise shift by vector (predicated)

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v2] iotests: Test abnormally large size in compressed cluster descriptor

[Alberto Garcia]

L2 entries for compressed clusters have a field that indicates the
number of sectors used to store the data in the image.

That's however not the size of the compressed data itself, just the
number of sectors where that data is located. The actual data size is
usually not a multiple of the sector size, and therefore cannot be
represented with this field.

The way it works is that QEMU reads all the specified sectors and
starts decompressing the data until there's enough to recover the
original uncompressed cluster. If there are any bytes left that
haven't been decompressed they are simply ignored.

One consequence of this is that even if the size field is larger than
it needs to be QEMU can handle it just fine: it will read more data
from disk but it will ignore the extra bytes.

This test creates an image with a compressed cluster that uses 5
sectors (2.5 KB), increases the size field to the maximum (8192
sectors, or 4 MB) and verifies that the data can be read without
problems.

This test is important because while the decompressed data takes
exactly one cluster, the maximum value allowed in the compressed size
field is twice the cluster size. So although QEMU won't produce images
with such large values we need to make sure that it can handle them.

Another effect of increasing the size field is that it can make it
include data from the following host cluster. In this case 'qemu-img
check' will detect that the refcounts are not correct, and we'll need
to rebuild them.

Additionally, this patch also tests that decreasing the size corrupts
the image since the original data can no longer be recovered. In this
case QEMU returns an error when trying to read the compressed data,
but 'qemu-img check' doesn't see anything wrong if the refcounts are
consistent.

One possible task for the future is to make 'qemu-img check' verify
the sizes of the compressed clusters, by trying to decompress the data
and checking that the size stored in the L2 entry is correct.

Signed-off-by: Alberto Garcia 
---

v2: We now have two scenarios where we make QEMU read data from the
next host cluster and from beyond the end of the image. This
version also runs qemu-img check on the corrupted image.

If the size field is too small, reading fails but qemu-img check
succeeds.

If the size field is too large, reading succeeds but qemu-img
check fails (this can be repaired, though).

---
 tests/qemu-iotests/122 | 38 ++
 tests/qemu-iotests/122.out | 28 
 2 files changed, 66 insertions(+)

diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122
index 45b359c2ba..fd5f43acc3 100755
--- a/tests/qemu-iotests/122
+++ b/tests/qemu-iotests/122
@@ -130,6 +130,44 @@ $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | 
_filter_qemu_io | _fil
 
 
 echo
+echo "=== Corrupted size field in compressed cluster descriptor ==="
+echo
+# Create an empty image, fill half of it with data and compress it.
+# The L2 entry of the first compressed cluster is located at 0x80.
+# The original value is 0x400800a0 (5 sectors for compressed data).
+TEST_IMG="$TEST_IMG".1 _make_test_img 8M
+$QEMU_IO -c "write -P 0x11 0 4M" "$TEST_IMG".1 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IMG convert -c -O qcow2 -o cluster_size=2M "$TEST_IMG".1 "$TEST_IMG"
+
+# Reduce size of compressed data to 4 sectors: this corrupts the image.
+poke_file "$TEST_IMG" $((0x80)) "\x40\x06"
+$QEMU_IO -c "read  -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# 'qemu-img check' however doesn't see anything wrong because it
+# doesn't try to decompress the data and the refcounts are consistent.
+_check_test_img
+
+# Increase size of compressed data to the maximum (8192 sectors).
+# This makes QEMU read more data (8192 sectors instead of 5), but the
+# decompression algorithm stops once we have enough to restore the
+# uncompressed cluster, so the rest of the data is ignored.
+poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe"
+
+# Here the image is too small so we're asking QEMU to read beyond the
+# end of the image.
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+# But if we grow the image we won't be reading beyond its end anymore.
+$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# The refcount data is however wrong because due to the increased size
+# of the compressed data it now reaches the following host cluster.
+# This can be repaired by qemu-img check.
+_check_test_img -r all
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IO -c "read  -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+echo
 echo "=== Full allocation with -S 0 ==="
 echo
 
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out

[Qemu-devel] [PATCH v3 2/3] s390x/sclp: clean up sclp masks

[Claudio Imbrenda]

Introduce an sccb_mask_t to be used for SCLP event masks instead of just
unsigned int or uint32_t. This will allow later to extend the mask with
more ease.

Signed-off-by: Claudio Imbrenda 
---
 hw/char/sclpconsole-lm.c  |  4 ++--
 hw/char/sclpconsole.c |  4 ++--
 hw/s390x/event-facility.c | 20 ++--
 hw/s390x/sclpcpu.c|  4 ++--
 hw/s390x/sclpquiesce.c|  4 ++--
 include/hw/s390x/event-facility.h | 22 +-
 6 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index c500bda..cc4d70a 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -102,12 +102,12 @@ static bool can_handle_event(uint8_t type)
 return type == SCLP_EVENT_MESSAGE || type == SCLP_EVENT_PMSGCMD;
 }
 
-static unsigned int send_mask(void)
+static sccb_mask_t send_mask(void)
 {
 return SCLP_EVENT_MASK_OP_CMD | SCLP_EVENT_MASK_PMSGCMD;
 }
 
-static unsigned int receive_mask(void)
+static sccb_mask_t receive_mask(void)
 {
 return SCLP_EVENT_MASK_MSG | SCLP_EVENT_MASK_PMSGCMD;
 }
diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index d0265df..ec9db13 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -83,12 +83,12 @@ static bool can_handle_event(uint8_t type)
 return type == SCLP_EVENT_ASCII_CONSOLE_DATA;
 }
 
-static unsigned int send_mask(void)
+static sccb_mask_t send_mask(void)
 {
 return SCLP_EVENT_MASK_MSG_ASCII;
 }
 
-static unsigned int receive_mask(void)
+static sccb_mask_t receive_mask(void)
 {
 return SCLP_EVENT_MASK_MSG_ASCII;
 }
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index b7cd075..94fe948 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -29,8 +29,8 @@ typedef struct SCLPEventsBus {
 struct SCLPEventFacility {
 SysBusDevice parent_obj;
 SCLPEventsBus sbus;
-/* guest' receive mask */
-unsigned int receive_mask;
+/* guest's receive mask */
+sccb_mask_t receive_mask;
 /*
  * when false, we keep the same broken, backwards compatible behaviour as
  * before, allowing only masks of size exactly 4; when true, we implement
@@ -61,9 +61,9 @@ static bool event_pending(SCLPEventFacility *ef)
 return false;
 }
 
-static unsigned int get_host_send_mask(SCLPEventFacility *ef)
+static sccb_mask_t get_host_send_mask(SCLPEventFacility *ef)
 {
-unsigned int mask;
+sccb_mask_t mask;
 BusChild *kid;
 SCLPEventClass *child;
 
@@ -77,9 +77,9 @@ static unsigned int get_host_send_mask(SCLPEventFacility *ef)
 return mask;
 }
 
-static unsigned int get_host_receive_mask(SCLPEventFacility *ef)
+static sccb_mask_t get_host_receive_mask(SCLPEventFacility *ef)
 {
-unsigned int mask;
+sccb_mask_t mask;
 BusChild *kid;
 SCLPEventClass *child;
 
@@ -189,7 +189,7 @@ out:
 }
 
 static uint16_t handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
-unsigned int mask)
+sccb_mask_t mask)
 {
 uint16_t rc;
 int slen;
@@ -242,8 +242,8 @@ static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t 
dst_len,
 
 static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
 {
-unsigned int sclp_active_selection_mask;
-unsigned int sclp_cp_receive_mask;
+sccb_mask_t sclp_active_selection_mask;
+sccb_mask_t sclp_cp_receive_mask;
 
 ReadEventData *red = (ReadEventData *) sccb;
 
@@ -285,7 +285,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB 
*sccb)
 {
 WriteEventMask *we_mask = (WriteEventMask *) sccb;
 uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
-uint32_t tmp_mask;
+sccb_mask_t tmp_mask;
 
 if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
 ((mask_length != 4) && !ef->allow_all_mask_sizes)) {
diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c
index 3ee890b..50c021b 100644
--- a/hw/s390x/sclpcpu.c
+++ b/hw/s390x/sclpcpu.c
@@ -37,12 +37,12 @@ void raise_irq_cpu_hotplug(void)
 sclp_service_interrupt(0);
 }
 
-static unsigned int send_mask(void)
+static sccb_mask_t send_mask(void)
 {
 return SCLP_EVENT_MASK_CONFIG_MGT_DATA;
 }
 
-static unsigned int receive_mask(void)
+static sccb_mask_t receive_mask(void)
 {
 return 0;
 }
diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
index 0241643..1c8f5c9 100644
--- a/hw/s390x/sclpquiesce.c
+++ b/hw/s390x/sclpquiesce.c
@@ -28,12 +28,12 @@ static bool can_handle_event(uint8_t type)
 return type == SCLP_EVENT_SIGNAL_QUIESCE;
 }
 
-static unsigned int send_mask(void)
+static sccb_mask_t send_mask(void)
 {
 return SCLP_EVENT_MASK_SIGNAL_QUIESCE;
 }
 
-static unsigned int receive_mask(void)
+static sccb_mask_t receive_mask(void)
 {
 return 0;
 }
diff --git a/include/hw/s390x/event-facility.h 
b/include/hw/s390x/event-facility.h
index 5119b9b..0e2b761 100644
--- a/include/hw/s390x/event-facilit

[Qemu-devel] [PATCH v3 1/3] s390x/sclp: proper support of larger send and receive masks

[Claudio Imbrenda]

Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
we only supported sclp event masks with a size of exactly 4 bytes, even
though the architecture allows the guests to set up sclp event masks
from 1 to 1021 bytes in length.
After that patch, the behaviour was almost compliant, but some issues
were still remaining, in particular regarding the handling of selective
reads and migration.

When setting the sclp event mask, a mask size is also specified. Until
now we only considered the size in order to decide which bits to save
in the internal state. On the other hand, when a guest performs a
selective read, it sends a mask, but it does not specify a size; the
implied size is the size of the last mask that has been set.

Specifying bits in the mask of selective read that are not available in
the internal mask should return an error, and bits past the end of the
mask should obviously be ignored. This can only be achieved by keeping
track of the lenght of the mask.

The mask length is thus now part of the internal state that needs to be
migrated.

This patch fixes the handling of selective reads, whose size will now
match the length of the event mask, as per architecture.

While the default behaviour is to be compliant with the architecture,
when using older machine models the old broken behaviour is selected
(allowing only masks of size exactly 4), in order to be able to migrate
toward older versions.

Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
Signed-off-by: Claudio Imbrenda 
---
 hw/s390x/event-facility.c  | 91 +++---
 hw/s390x/s390-virtio-ccw.c |  8 +++-
 2 files changed, 85 insertions(+), 14 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 155a694..b7cd075 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -31,6 +31,15 @@ struct SCLPEventFacility {
 SCLPEventsBus sbus;
 /* guest' receive mask */
 unsigned int receive_mask;
+/*
+ * when false, we keep the same broken, backwards compatible behaviour as
+ * before, allowing only masks of size exactly 4; when true, we implement
+ * the architecture correctly, allowing all valid mask sizes. Needed for
+ * migration toward older versions.
+ */
+bool allow_all_mask_sizes;
+/* length of the receive mask */
+uint16_t mask_length;
 };
 
 /* return true if any child has event pending set */
@@ -220,6 +229,17 @@ static uint16_t handle_sccb_read_events(SCLPEventFacility 
*ef, SCCB *sccb,
 return rc;
 }
 
+/* copy up to dst_len bytes and fill the rest of dst with zeroes */
+static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
+  uint16_t src_len)
+{
+int i;
+
+for (i = 0; i < dst_len; i++) {
+dst[i] = i < src_len ? src[i] : 0;
+}
+}
+
 static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
 {
 unsigned int sclp_active_selection_mask;
@@ -240,7 +260,9 @@ static void read_event_data(SCLPEventFacility *ef, SCCB 
*sccb)
 sclp_active_selection_mask = sclp_cp_receive_mask;
 break;
 case SCLP_SELECTIVE_READ:
-sclp_active_selection_mask = be32_to_cpu(red->mask);
+copy_mask((uint8_t *)&sclp_active_selection_mask, (uint8_t 
*)&red->mask,
+  sizeof(sclp_active_selection_mask), ef->mask_length);
+sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
 if (!sclp_cp_receive_mask ||
 (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
 sccb->h.response_code =
@@ -259,24 +281,14 @@ out:
 return;
 }
 
-/* copy up to dst_len bytes and fill the rest of dst with zeroes */
-static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
-  uint16_t src_len)
-{
-int i;
-
-for (i = 0; i < dst_len; i++) {
-dst[i] = i < src_len ? src[i] : 0;
-}
-}
-
 static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
 {
 WriteEventMask *we_mask = (WriteEventMask *) sccb;
 uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
 uint32_t tmp_mask;
 
-if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX)) {
+if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
+((mask_length != 4) && !ef->allow_all_mask_sizes)) {
 sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
 goto out;
 }
@@ -301,6 +313,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB 
*sccb)
   mask_length, sizeof(tmp_mask));
 
 sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+ef->mask_length = mask_length;
 
 out:
 return;
@@ -356,6 +369,34 @@ static void command_handler(SCLPEventFacility *ef, SCCB 
*sccb, uint64_t code)
 }
 }
 
+static bool vmstate_event_facility_mask_length_needed(void *opaque)
+{
+SCLPEventFacility *ef = opaque;
+
+return ef->allow_all_mask_sizes

[Qemu-devel] [PATCH v3 3/3] s390x/sclp: extend SCLP event masks to 64 bits

[Claudio Imbrenda]

Extend the SCLP event masks to 64 bits.

Notice that using any of the new bits results in a state that cannot be
migrated to an older version.

Signed-off-by: Claudio Imbrenda 
---
 hw/s390x/event-facility.c | 50 ---
 include/hw/s390x/event-facility.h |  2 +-
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 94fe948..6072fd3 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -30,7 +30,10 @@ struct SCLPEventFacility {
 SysBusDevice parent_obj;
 SCLPEventsBus sbus;
 /* guest's receive mask */
-sccb_mask_t receive_mask;
+union {
+uint32_t receive_mask_compat32;
+sccb_mask_t receive_mask;
+};
 /*
  * when false, we keep the same broken, backwards compatible behaviour as
  * before, allowing only masks of size exactly 4; when true, we implement
@@ -262,7 +265,7 @@ static void read_event_data(SCLPEventFacility *ef, SCCB 
*sccb)
 case SCLP_SELECTIVE_READ:
 copy_mask((uint8_t *)&sclp_active_selection_mask, (uint8_t 
*)&red->mask,
   sizeof(sclp_active_selection_mask), ef->mask_length);
-sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
+sclp_active_selection_mask = be64_to_cpu(sclp_active_selection_mask);
 if (!sclp_cp_receive_mask ||
 (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
 sccb->h.response_code =
@@ -294,21 +297,22 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB 
*sccb)
 }
 
 /*
- * Note: We currently only support masks up to 4 byte length;
- *   the remainder is filled up with zeroes. Linux uses
- *   a 4 byte mask length.
+ * Note: We currently only support masks up to 8 byte length;
+ *   the remainder is filled up with zeroes. Older Linux
+ *   kernels use a 4 byte mask length, newer ones can use both
+ *   8 or 4 depending on what is available on the host.
  */
 
 /* keep track of the guest's capability masks */
 copy_mask((uint8_t *)&tmp_mask, WEM_CP_RECEIVE_MASK(we_mask, mask_length),
   sizeof(tmp_mask), mask_length);
-ef->receive_mask = be32_to_cpu(tmp_mask);
+ef->receive_mask = be64_to_cpu(tmp_mask);
 
 /* return the SCLP's capability masks to the guest */
-tmp_mask = cpu_to_be32(get_host_receive_mask(ef));
+tmp_mask = cpu_to_be64(get_host_receive_mask(ef));
 copy_mask(WEM_RECEIVE_MASK(we_mask, mask_length), (uint8_t *)&tmp_mask,
   mask_length, sizeof(tmp_mask));
-tmp_mask = cpu_to_be32(get_host_send_mask(ef));
+tmp_mask = cpu_to_be64(get_host_send_mask(ef));
 copy_mask(WEM_SEND_MASK(we_mask, mask_length), (uint8_t *)&tmp_mask,
   mask_length, sizeof(tmp_mask));
 
@@ -369,6 +373,21 @@ static void command_handler(SCLPEventFacility *ef, SCCB 
*sccb, uint64_t code)
 }
 }
 
+static bool vmstate_event_facility_mask64_needed(void *opaque)
+{
+SCLPEventFacility *ef = opaque;
+
+return (ef->receive_mask & 0x) != 0;
+}
+
+static int vmstate_event_facility_mask64_pre_load(void *opaque)
+{
+SCLPEventFacility *ef = opaque;
+
+ef->receive_mask &= ~0xULL;
+return 0;
+}
+
 static bool vmstate_event_facility_mask_length_needed(void *opaque)
 {
 SCLPEventFacility *ef = opaque;
@@ -384,6 +403,18 @@ static int 
vmstate_event_facility_mask_length_pre_load(void *opaque)
 return 0;
 }
 
+static const VMStateDescription vmstate_event_facility_mask64 = {
+.name = "vmstate-event-facility/mask64",
+.version_id = 0,
+.minimum_version_id = 0,
+.needed = vmstate_event_facility_mask64_needed,
+.pre_load = vmstate_event_facility_mask64_pre_load,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(receive_mask, SCLPEventFacility),
+VMSTATE_END_OF_LIST()
+ }
+};
+
 static const VMStateDescription vmstate_event_facility_mask_length = {
 .name = "vmstate-event-facility/mask_length",
 .version_id = 0,
@@ -402,10 +433,11 @@ static const VMStateDescription vmstate_event_facility = {
 .version_id = 0,
 .minimum_version_id = 0,
 .fields = (VMStateField[]) {
-VMSTATE_UINT32(receive_mask, SCLPEventFacility),
+VMSTATE_UINT32(receive_mask_compat32, SCLPEventFacility),
 VMSTATE_END_OF_LIST()
  },
 .subsections = (const VMStateDescription * []) {
+&vmstate_event_facility_mask64,
 &vmstate_event_facility_mask_length,
 NULL
  }
diff --git a/include/hw/s390x/event-facility.h 
b/include/hw/s390x/event-facility.h
index 0e2b761..06ba4ea 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -73,7 +73,7 @@ typedef struct WriteEventMask {
 #define WEM_RECEIVE_MASK(wem, mask_len) ((wem)->masks + 2 * (mask_len))
 #define WEM_SEND_MASK(wem, mask_len) ((wem)->masks + 3 * (mask_len))
 
-typedef uint32_t sccb_ma

Re: [Qemu-devel] [PATCH v2 13/67] target/arm: Implement SVE bitwise shift by wide elements (predicated)

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 21 +
>  target/arm/sve_helper.c| 35 +++
>  target/arm/translate-sve.c | 25 +
>  target/arm/sve.decode  |  6 ++
>  4 files changed, 87 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [Bug 1751264] [NEW] qemu-img convert issue in a tmpfs partition

[Teddy VALETTE]

Public bug reported:

qemu-img convert command is slow when the file to convert is located in
a tmpfs formatted partition.

v2.1.0 on debian/jessie x64, ext4: 10m14s
v2.1.0 on debian/jessie x64, tmpfs: 10m15s

v2.1.0 on debian/stretch x64, ext4: 11m9s
v2.1.0 on debian/stretch x64, tmpfs: 10m21.362s

v2.8.0 on debian/jessie x64, ext4: 10m21s
v2.8.0 on debian/jessie x64, tmpfs: Too long

v2.8.0 on debian/stretch x64, ext4: 10m42s
v2.8.0 on debian/stretch x64, tmpfs: Too long

It seems that the issue is caused by this commit :
https://github.com/qemu/qemu/commit/690c7301600162421b928c7f26fd488fd8fa464e

In order to reproduce this bug :

1/ mount a tmpfs partition : mount -t tmpfs tmpfs /tmp
2/ get a vmdk file (we used a 15GB image) and put it on /tmp
3/ run the 'qemu-img convert -O qcow2 /tmp/file.vmdk /path/to/destination' 
command

When we trace the process, we can see that there's a lseek loop which is
very slow (compare to outside a tmpfs partition).

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: qemu-img

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

Title:
  qemu-img convert issue in a tmpfs partition

Status in QEMU:
  New

Bug description:
  qemu-img convert command is slow when the file to convert is located
  in a tmpfs formatted partition.

  v2.1.0 on debian/jessie x64, ext4: 10m14s
  v2.1.0 on debian/jessie x64, tmpfs: 10m15s

  v2.1.0 on debian/stretch x64, ext4: 11m9s
  v2.1.0 on debian/stretch x64, tmpfs: 10m21.362s

  v2.8.0 on debian/jessie x64, ext4: 10m21s
  v2.8.0 on debian/jessie x64, tmpfs: Too long

  v2.8.0 on debian/stretch x64, ext4: 10m42s
  v2.8.0 on debian/stretch x64, tmpfs: Too long

  It seems that the issue is caused by this commit :
  https://github.com/qemu/qemu/commit/690c7301600162421b928c7f26fd488fd8fa464e

  In order to reproduce this bug :

  1/ mount a tmpfs partition : mount -t tmpfs tmpfs /tmp
  2/ get a vmdk file (we used a 15GB image) and put it on /tmp
  3/ run the 'qemu-img convert -O qcow2 /tmp/file.vmdk /path/to/destination' 
command

  When we trace the process, we can see that there's a lseek loop which
  is very slow (compare to outside a tmpfs partition).

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

Re: [Qemu-devel] [PATCH v3 0/3] s390x/sclp: 64 bit event masks

[Cornelia Huck]

On Fri, 23 Feb 2018 13:51:02 +0100
Claudio Imbrenda  wrote:

> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported 32bit sclp event masks, even though the archiecture
> allows the guests to set up sclp event masks up to 1021 bytes in length.
> With that patch the behaviour was almost compliant, but some issues were
> still remaining, in particular regarding the handling of selective reads
> and migration.
> 
> This patchset fixes migration and the handling of selective reads, and
> puts in place the support for 64-bit sclp event masks internally.
> 
> A new property of the sclp-event device switches between the 32bit masks
> and the compliant behaviour. The property is bound to the machine
> version, so older machines keep the old broken behaviour, allowing for
> migration, but the default is the compliant implementation.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")
> 
> v2 -> v3
> * fixed some typos in the first patch description
> * updated an existing comment in the third patch: newer Linux versions
>   will support masks larger than 4 bytes.

Update looks good to me.

> 
> v1 -> v2 
> * improved comments and patch descriptions to better explain why we need
>   this, including better description of the old broken behaviour
> * rename SCLPEVMSK to SCLP_EVMASK to improve readability
> * removed some unneded variable initializations
> * fixed a pre-existing typo
> 
> Claudio Imbrenda (3):
>   s390x/sclp: proper support of larger send and receive masks
>   s390x/sclp: clean up sclp masks
>   s390x/sclp: extend SCLP event masks to 64 bits
> 
>  hw/char/sclpconsole-lm.c  |   4 +-
>  hw/char/sclpconsole.c |   4 +-
>  hw/s390x/event-facility.c | 157 
> ++
>  hw/s390x/s390-virtio-ccw.c|   8 +-
>  hw/s390x/sclpcpu.c|   4 +-
>  hw/s390x/sclpquiesce.c|   4 +-
>  include/hw/s390x/event-facility.h |  22 +++---
>  7 files changed, 155 insertions(+), 48 deletions(-)
> 

[Qemu-devel] Intermittent failure of iotest 203

[Max Reitz]

Hi,

iotest 203 relatively often fails for me, at least when run in parallel.
 When I run the following concurrently on four shells:

$ while TEST_DIR=/tmp/t1 ./check -T -qcow2 203; do; done
$ while TEST_DIR=/tmp/t2 ./check -T -qcow2 203; do; done
$ while TEST_DIR=/tmp/t3 ./check -T -qcow2 203; do; done
$ while TEST_DIR=/tmp/t4 ./check -T -qcow2 203; do; done

Very quickly (like under ten iterations), at least one of those starts
to hang and then fails because of a timeout in vm.get_qmp_event().

Before digging deeper into the ppoll() dungeon* myself, I decided to
report this so I wouldn't have to. :-)

*Backtrace:

(gdb) bt
#0  0x7f354137b4d6 in ppoll () at /lib64/libc.so.6
#1  0x55b659144299 in ppoll (__ss=0x0, __timeout=0x7ffe4eaca230,
__nfds=, __fds=) at
/usr/include/bits/poll2.h:77
#2  0x55b659144299 in qemu_poll_ns (fds=,
nfds=, timeout=timeout@entry=39512999619000) at
util/qemu-timer.c:334
#3  0x55b6591450a3 in os_host_main_loop_wait (timeout=) at util/main-loop.c:255
#4  0x55b6591450a3 in main_loop_wait (nonblocking=)
at util/main-loop.c:515
#5  0x55b658d4a253 in main_loop () at vl.c:1933
#6  0x55b658d4a253 in main (argc=, argv=, envp=) at vl.c:4757

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 14/67] target/arm: Implement SVE Integer Arithmetic - Unary Predicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  60 +
>  target/arm/sve_helper.c| 127 
> +
>  target/arm/translate-sve.c | 111 +++
>  target/arm/sve.decode  |  23 
>  4 files changed, 321 insertions(+)

> diff --git a/target/arm/sve.decode b/target/arm/sve.decode
> index 177f338fed..b875501475 100644
> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -165,6 +165,29 @@ ASR_zpzw   0100 .. 011 000 100 ... . .   
>   @rdn_pg_rm
>  LSR_zpzw   0100 .. 011 001 100 ... . . @rdn_pg_rm
>  LSL_zpzw   0100 .. 011 011 100 ... . . @rdn_pg_rm
>
> +### SVE Integer Arithmetic - Unary Predicated Group
> +
> +# SVE unary bit operations (predicated)
> +# Note esz != 0 for FABS and FNEG.
> +CLS0100 .. 011 000 101 ... . . @rd_pg_rn
> +CLZ0100 .. 011 001 101 ... . . @rd_pg_rn
> +CNT_zpz0100 .. 011 010 101 ... . . 
> @rd_pg_rn
> +CNOT   0100 .. 011 011 101 ... . . @rd_pg_rn
> +NOT_zpz0100 .. 011 110 101 ... . . 
> @rd_pg_rn
> +FABS   0100 .. 011 100 101 ... . . @rd_pg_rn
> +FNEG   0100 .. 011 101 101 ... . . @rd_pg_rn

Indentation seems to be a bit skew for the _zpz lines.

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 15/67] target/arm: Implement SVE Integer Multiply-Add Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 18 ++
>  target/arm/sve_helper.c| 58 
> +-
>  target/arm/translate-sve.c | 31 +
>  target/arm/sve.decode  | 17 ++
>  4 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index 11644125d1..b31d497f31 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -345,6 +345,24 @@ DEF_HELPER_FLAGS_4(sve_neg_h, TCG_CALL_NO_RWG, void, 
> ptr, ptr, ptr, i32)
>  DEF_HELPER_FLAGS_4(sve_neg_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
>  DEF_HELPER_FLAGS_4(sve_neg_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
>
> +DEF_HELPER_FLAGS_6(sve_mla_b, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mla_h, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mla_s, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mla_d, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_6(sve_mls_b, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mls_h, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mls_s, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_6(sve_mls_d, TCG_CALL_NO_RWG,
> +   void, ptr, ptr, ptr, ptr, ptr, i32)
> +
>  DEF_HELPER_FLAGS_5(sve_and_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_bic_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_eor_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index e11823a727..4b08a38ce8 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -932,6 +932,62 @@ DO_ZPZI_D(sve_asrd_d, int64_t, DO_ASRD)
>  #undef DO_SHR
>  #undef DO_SHL
>  #undef DO_ASRD
> -
>  #undef DO_ZPZI
>  #undef DO_ZPZI_D

Deletion of blank line should be dropped or squashed into earlier patch.

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 16/67] target/arm: Implement SVE Integer Arithmetic - Unpredicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 41 ++---
>  target/arm/sve.decode  | 13 +
>  2 files changed, 51 insertions(+), 3 deletions(-)

> @@ -254,7 +288,8 @@ static void do_zpzz_ool(DisasContext *s, arg_rprr_esz *a, 
> gen_helper_gvec_4 *fn)
>  }
>
>  #define DO_ZPZZ(NAME, name) \
> -void trans_##NAME##_zpzz(DisasContext *s, arg_rprr_esz *a, uint32_t insn) \
> +static void trans_##NAME##_zpzz(DisasContext *s, arg_rprr_esz *a, \
> +uint32_t insn)\
>  { \
>  static gen_helper_gvec_4 * const fns[4] = {   \
>  gen_helper_sve_##name##_zpzz_b, gen_helper_sve_##name##_zpzz_h,   \
> @@ -286,7 +321,7 @@ DO_ZPZZ(ASR, asr)
>  DO_ZPZZ(LSR, lsr)
>  DO_ZPZZ(LSL, lsl)
>
> -void trans_SDIV_zpzz(DisasContext *s, arg_rprr_esz *a, uint32_t insn)
> +static void trans_SDIV_zpzz(DisasContext *s, arg_rprr_esz *a, uint32_t insn)
>  {
>  static gen_helper_gvec_4 * const fns[4] = {
>  NULL, NULL, gen_helper_sve_sdiv_zpzz_s, gen_helper_sve_sdiv_zpzz_d
> @@ -294,7 +329,7 @@ void trans_SDIV_zpzz(DisasContext *s, arg_rprr_esz *a, 
> uint32_t insn)
>  do_zpzz_ool(s, a, fns[a->esz]);
>  }
>
> -void trans_UDIV_zpzz(DisasContext *s, arg_rprr_esz *a, uint32_t insn)
> +static void trans_UDIV_zpzz(DisasContext *s, arg_rprr_esz *a, uint32_t insn)
>  {
>  static gen_helper_gvec_4 * const fns[4] = {
>  NULL, NULL, gen_helper_sve_udiv_zpzz_s, gen_helper_sve_udiv_zpzz_d

Should these changes to 'static' have been in a different patch, or was
that to avoid compiler warnings when the functions were introduced but not
used til this patch?

> diff --git a/target/arm/sve.decode b/target/arm/sve.decode
> index 68a1823b72..b40d7dc9a2 100644
> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -68,6 +68,9 @@
>  # Three prediate operand, with governing predicate, flag setting
>  @pd_pg_pn_pm_s  . s:1 .. rm:4 .. pg:4 . rn:4 . rd:4&rprr_s
>
> +# Three operand, vector element size
> +@rd_rn_rm   esz:2 . rm:5  ... ...  rn:5 rd:5   &rrr_esz
> +
>  # Two register operand, with governing predicate, vector element size
>  @rdn_pg_rm  esz:2 ... ... ... pg:3 rm:5 rd:5 \
> &rprr_esz rn=%reg_movprfx
> @@ -205,6 +208,16 @@ MLS0100 .. 0 . 011 ... . 
> .   @rda_pg_rn_rm
>  MLA0100 .. 0 . 110 ... . .   @rdn_pg_ra_rm # MAD
>  MLS0100 .. 0 . 111 ... . .   @rdn_pg_ra_rm # MSB
>
> +### SVE Integer Arithmetic - Unpredicated Group
> +
> +# SVE integer add/subtract vectors (unpredicated)
> +ADD_zzz0100 .. 1 . 000 000 . . 
> @rd_rn_rm
> +SUB_zzz0100 .. 1 . 000 001 . . 
> @rd_rn_rm
> +SQADD_zzz  0100 .. 1 . 000 100 . . @rd_rn_rm
> +UQADD_zzz  0100 .. 1 . 000 101 . . @rd_rn_rm
> +SQSUB_zzz  0100 .. 1 . 000 110 . . @rd_rn_rm
> +UQSUB_zzz  0100 .. 1 . 000 111 . . @rd_rn_rm

Misaligned lines for ADD and SUB.

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 17/67] target/arm: Implement SVE Index Generation Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  5 
>  target/arm/sve_helper.c| 40 +++
>  target/arm/translate-sve.c | 67 
> ++
>  target/arm/sve.decode  | 14 ++
>  4 files changed, 126 insertions(+)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH 2/7] ovmf: link with Tcg2ConfigPei module

[marcandre . lureau]

From: Marc-André Lureau 

This module initializes TPM device type based on variable and
detection.

The module requires VariablePei, which is built with
MEM_VARSTORE_EMU_ENABLE=FALSE.

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 20 
 OvmfPkg/OvmfPkgX64.fdf |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 32c57b04e1..b5cbe8430f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -40,6 +40,7 @@
   DEFINE SMM_REQUIRE = FALSE
   DEFINE TLS_ENABLE  = FALSE
   DEFINE MEM_VARSTORE_EMU_ENABLE = TRUE
+  DEFINE TPM2_ENABLE = FALSE
 
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -209,6 +210,11 @@
   
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
   XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
+  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
+!endif
+
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
@@ -272,6 +278,10 @@
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
   PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
+!if $(TPM2_ENABLE)
+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
+  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
+!endif
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -558,6 +568,12 @@
 
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
 
+!if $(TPM2_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0xb6, 0xe5, 0x01, 0x8b, 
0x19, 0x4f, 0xe8, 0x46, 0xab, 0x93, 0x1c, 0x53, 0x67, 0x1b, 0x90, 0xcc}
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy|1
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInitializationPolicy|1
+!endif
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
@@ -629,6 +645,10 @@
 
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
+
 !if $(SECURE_BOOT_ENABLE) == TRUE
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
 
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index bb46a409d9..dc35d0a1f7 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -168,6 +168,9 @@ INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 INF  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
 INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 !endif
+!if $(TPM2_ENABLE) == TRUE
+INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
 
 

 
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 4/7] ovmf: link with Tcg2Pei module

[marcandre . lureau]

From: Marc-André Lureau 

This module will initialize TPM device, measure reported FVs and BIOS
version.

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 7 +++
 OvmfPkg/OvmfPkgX64.fdf | 1 +
 2 files changed, 8 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b5cbe8430f..34a7c2778e 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -279,6 +279,8 @@
   PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
 !if $(TPM2_ENABLE)
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+  
HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
   Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
 !endif
@@ -647,6 +649,11 @@
 
 !if $(TPM2_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
+
+  NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+  NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+  }
 !endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index dc35d0a1f7..9558142a42 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -170,6 +170,7 @@ INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 !endif
 !if $(TPM2_ENABLE) == TRUE
 INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
 !endif
 
 

-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 3/7] HACK: HobLib: workaround infinite loop

[marcandre . lureau]

From: Marc-André Lureau 

Without this hack, GetNextHob() loops infinitely with the next patch.
I don't understand the reason.

The loop is triggered by the GetFirstGuidHob (&gTpmErrorHobGuid) call.

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 MdePkg/Library/PeiHobLib/HobLib.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdePkg/Library/PeiHobLib/HobLib.c 
b/MdePkg/Library/PeiHobLib/HobLib.c
index 5c0eeb992f..ed3c5fbd6d 100644
--- a/MdePkg/Library/PeiHobLib/HobLib.c
+++ b/MdePkg/Library/PeiHobLib/HobLib.c
@@ -89,6 +89,10 @@ GetNextHob (
 if (Hob.Header->HobType == Type) {
   return Hob.Raw;
 }
+if (GET_HOB_LENGTH (HobStart) == 0) {
+DEBUG ((DEBUG_INFO, "FIXME: GetNextHob length == 0"));
+return NULL;
+}
 Hob.Raw = GET_NEXT_HOB (Hob);
   }
   return NULL;
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 0/7] RFC: ovmf: preliminary TPM2 support

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

The following series adds basic TPM2 support for OVMF-on-QEMU (I
haven't tested TPM1, for lack of interest). It links with the modules
to initializes the device in PEI phase, and do measurements (both PEI
and DXE). The Tcg2Dxe module provides the Tcg2 protocol which allows
the guest to access the measurement log and other facilities.

DxeTpm2MeasureBootLib seems to do its job at measuring images that are
not measured in PEI phase (such as PCI PXE rom)

Tcg2ConfigDxe is mostly interesting for debugging for now.

A major lack is the support for Physical Present Interface (PPI, more
below).

Linux guests seem to work fine. But windows guest generally complains
about the lack of PPI interface (most HLK tests require it, tpm.msc
admin interactions too). I haven't done "real" use-cases tests, as I
lack experience with TPM usage. Any help appreciated to test the TPM.

Tcg2ConfigPei requires variable access, therefore
 must be solved
first. I used "[edk2] [PATCH v2 0/8] OvmfPkg: add the Variable PEIM,
defragment the UEFI memmap" as a base for this series.

I build edk2 with:

$ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE  -DMEM_VARSTORE_EMU_ENABLE=FALSE

I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 
--tpm-state tpmstatedir)

$ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock  --tpm2 &
$ qemu .. -chardev socket,id=chrtpm,path=tpmsock -tpmdev 
emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0

PPI is problematic, because we generally don't want or need SMM, and
qemu is preferred to provide the ACPI tables. We therefore exclude
using Tcg2Smm for now (which also brings other problems). Stefan
Berger has been prototyping qemu code that provides PPI ACPI
interface, but there is some complication regarding memory location,
using a fixed address. My understanding is that the firmware
(seabios/edk2) should allocate the required memory itself (using qemu
linker script for ex) and patch the ACPI table. Then it's hopefully
only a matter of hooking Tcg2PhysicalPresenceLibProcessRequest() as
was done by Stefan in
https://github.com/stefanberger/edk2/commits/tpm2. The main problem I
see with this approach is that the location should remain stable
across reboots (not necessarily poweroff, edk2 uses nvram variables
for PPI flags). More investigation and help needed to support PPI!

Thanks

Related bug:
https://bugzilla.tianocore.org/show_bug.cgi?id=594

Marc-André Lureau (7):
  SecurityPkg/Tcg2Pei: drop Tcg2PhysicalPresenceLib dependency
  ovmf: link with Tcg2ConfigPei module
  HACK: HobLib: workaround infinite loop
  ovmf: link with Tcg2Pei module
  ovmf: link with Tcg2Dxe module
  ovmf: link with Tcg2ConfigDxe module
  ovmf: add DxeTpm2MeasureBootLib

 MdePkg/Library/PeiHobLib/HobLib.c   |  4 +++
 OvmfPkg/OvmfPkgX64.dsc  | 49 -
 OvmfPkg/OvmfPkgX64.fdf  |  9 +++
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   |  2 --
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |  1 -
 5 files changed, 61 insertions(+), 4 deletions(-)

-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH v2 18/67] target/arm: Implement SVE Stack Allocation Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 24 
>  target/arm/sve.decode  | 12 
>  2 files changed, 36 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH 1/7] SecurityPkg/Tcg2Pei: drop Tcg2PhysicalPresenceLib dependency

[marcandre . lureau]

From: Marc-André Lureau 

Apparently, unnecessary. Avoids extra build dependency and churn.

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   | 2 --
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
 2 files changed, 3 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index a7ae3354b5..3758fc6a41 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -18,7 +18,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -44,7 +43,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 
 #define PERF_ID_TCG2_PEI  0x3080
 
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index f7b85444d9..bc910c3baf 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -58,7 +58,6 @@
   PerformanceLib
   MemoryAllocationLib
   ReportStatusCodeLib
-  Tcg2PhysicalPresenceLib
   ResetSystemLib
 
 [Guids]
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 5/7] ovmf: link with Tcg2Dxe module

[marcandre . lureau]

From: Marc-André Lureau 

This module measures and log the boot environment. It also produces
the Tcg2 protocol, which allows for example to read the log from OS:

[0.00] efi: EFI v2.70 by EDK II
[0.00] efi:  SMBIOS=0x3fa1f000  ACPI=0x3fbb6000  ACPI 2.0=0x3fbb6014  
MEMATTR=0x3e7d4318  TPMEventLog=0x3db21018

$ python chipsec_util.py tpm parse_log binary_bios_measurements

[CHIPSEC] Version 1.3.5.dev2
[CHIPSEC] API mode: using OS native API (not using CHIPSEC kernel module)
[CHIPSEC] Executing command 'tpm' with args ['parse_log', 
'/tmp/binary_bios_measurements']

PCR: 0  type: EV_S_CRTM_VERSION size: 0x2   digest: 
1489f923c4dca729178b3e3233458550d8dddf29
+ version:
PCR: 0  type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
fd39ced7c0d2a61f6830c78c7625f94826b05bcc
+ base: 0x82length: 0xe
PCR: 0  type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c
+ base: 0x90length: 0xa0
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35  digest: 
57cd4dc19442475aa82743484f3b1caa88e142b8
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
9b1387306ebb7ff8e795e7be77563666bbf4516e
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
9afa86c507419b8570c62167cb9486d9fc809758
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
5bf8faa078d40ffbd03317c93398b01229a0e1e0
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
734424c9fe8fc71716c42096f4b74c88733b175e
PCR: 7  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x3e  digest: 
252f8ebb85340290b64f4b06a001742be8e5cab6
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x6e  digest: 
22a4f6ee9af6dba01d3528deb64b74b582fc182b
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x80  digest: 
b7811d5bf30a7efd4e385c6179fe10d9290bb9e8
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x84  digest: 
425e502c24fc924e231e0a62327b6b7d1f704573
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x9a  digest: 
0b5d2c98ac5de6148a4a1490ff9d5df69039f04e
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0xbd  digest: 
20bd5f402271d57a88ea314fe35c1705956b1f74
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x88  digest: 
df5d6605cb8f4366d745a8464cfb26c1efdc305c
PCR: 4  type: EV_EFI_ACTION size: 0x28  digest: 
cd0fdb4531a6ec41be2753ba042637d6e5f7f256
PCR: 0  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 2  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 3  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 4  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 5  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473

$ tpm2_pcrlist
sha1 :
  0  : 35bd1786b6909daad610d7598b1d620352d33b8a
  1  : ec0511e860206e0af13c31da2f9e943fb6ca353d
  2  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  3  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  4  : 45a323382bd933f08e7f0e256bc8249e4095b1ec
  5  : d16d7e629fd8d08ca256f9ad3a3a1587c9e6cc1b
  6  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  7  : 518bd167271fbb64589c61e43d8c0165861431d8
  8  : 
  9  : 
  10 : 
  11 : 
  12 : 
  13 : 
  14 : 
  15 : 
  16 : 
  17 : 
  18 : 
  19 : 
  20 : 
  21 : 
  22 : 
  23 : 
sha256 :
  0  : 9ae903dbae3357ac00d223660bac19ea5c021499a56201104332ab966631ce2c
  1  : acc611d90245cf04e77b0ca94901f90e7fa54770f0426f53c3049b532243d1b8
  2  : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
  3  : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
  4  : 7a94ffe8a7729a566d3d3c577fcb4b6b1e671f31540375f80eae6382ab785e35
  5  : a5ceb755d043f32431d63e39f5161464620a3437280494b5850dc1b47cc074e0
  6  : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a72

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 19/67] target/arm: Implement SVE Bitwise Shift - Unpredicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 12 +++
>  target/arm/sve_helper.c| 30 +
>  target/arm/translate-sve.c | 81 
> ++
>  target/arm/sve.decode  | 26 +++
>  4 files changed, 149 insertions(+)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH 6/7] ovmf: link with Tcg2ConfigDxe module

[marcandre . lureau]

From: Marc-André Lureau 

The module allows to tweak and interact with the TPM. Note that many
actions are broken due to implementation of qemu TPM (providing it's
own ACPI table), and the lack of PPI implementation.

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 2 ++
 OvmfPkg/OvmfPkgX64.fdf | 1 +
 2 files changed, 3 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 9bd0709f98..2281bd5ff8 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -669,6 +669,8 @@
   NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
   NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
   }
+
+  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
 !endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index b8dd7ecae4..985404850f 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -399,6 +399,7 @@ INF  
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 
 !if $(TPM2_ENABLE) == TRUE
 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
+INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
 !endif
 
 

-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH v2] iotests: Test abnormally large size in compressed cluster descriptor

[Eric Blake]

On 02/23/2018 06:50 AM, Alberto Garcia wrote:

L2 entries for compressed clusters have a field that indicates the
number of sectors used to store the data in the image.

That's however not the size of the compressed data itself, just the
number of sectors where that data is located. The actual data size is
usually not a multiple of the sector size, and therefore cannot be
represented with this field.





Another effect of increasing the size field is that it can make it
include data from the following host cluster. In this case 'qemu-img
check' will detect that the refcounts are not correct, and we'll need
to rebuild them.


Indeed, tweaking sizes (can) affect refcount computations.



Additionally, this patch also tests that decreasing the size corrupts
the image since the original data can no longer be recovered. In this
case QEMU returns an error when trying to read the compressed data,
but 'qemu-img check' doesn't see anything wrong if the refcounts are
consistent.

One possible task for the future is to make 'qemu-img check' verify
the sizes of the compressed clusters, by trying to decompress the data
and checking that the size stored in the L2 entry is correct.


Indeed, but that means...



+
+# Reduce size of compressed data to 4 sectors: this corrupts the image.
+poke_file "$TEST_IMG" $((0x80)) "\x40\x06"
+$QEMU_IO -c "read  -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# 'qemu-img check' however doesn't see anything wrong because it
+# doesn't try to decompress the data and the refcounts are consistent.
+_check_test_img


...this spot should have a TODO comment that mentions the test needs 
updating if qemu-img check is taught to be pickier.



+
+# Increase size of compressed data to the maximum (8192 sectors).
+# This makes QEMU read more data (8192 sectors instead of 5), but the
+# decompression algorithm stops once we have enough to restore the
+# uncompressed cluster, so the rest of the data is ignored.
+poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe"
+
+# Here the image is too small so we're asking QEMU to read beyond the
+# end of the image.
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+# But if we grow the image we won't be reading beyond its end anymore.
+$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# The refcount data is however wrong because due to the increased size
+# of the compressed data it now reaches the following host cluster.
+# This can be repaired by qemu-img check.
+_check_test_img -r all
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IO -c "read  -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir


Thanks - this indeed tests more scenarios than v1.

With the TODO comment added,
Reviewed-by: Eric Blake 

Hmm - I also wonder - does our refcount code properly account for a 
compressed cluster that would affect the refcount of THREE clusters? 
Remember, qemu will never emit a compressed cluster that touches more 
than two clusters, but when you enlarge the size, if offset part of the 
link was already in the tail of one cluster, then you can bleed over 
into not just one, but two additional host clusters.  Your test didn't 
cover that, because it uses a compressed cluster that maps to the start 
of the host cluster.


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

[Qemu-devel] [PATCH 7/7] ovmf: add DxeTpm2MeasureBootLib

[marcandre . lureau]

From: Marc-André Lureau 

The library registers a security management handler, to measure images
that are not measure in PEI phase.

This seems to work for example with the qemu PXE rom:

Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi

And the following binary_bios_measurements log entry seems to be
added:

PCR: 2  type: EV_EFI_BOOT_SERVICES_DRIVER   size: 0x4e  digest: 
70a22475e9f18806d2ed9193b48d80d26779d9a4

CC: Laszlo Ersek 
CC: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 2281bd5ff8..92ed9f3b0c 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -677,7 +677,10 @@
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
 
   
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
-   }
+!if $(TPM2_ENABLE) == TRUE
+  NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
+!endif
+  }
 !else
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 !endif
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Thomas Huth]

On 23.02.2018 12:50, Viktor Mihajlovski wrote:
> On 23.02.2018 11:17, Thomas Huth wrote:
>> On 23.02.2018 09:53, Christian Borntraeger wrote:
>>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if 
>>> I do not 
>>> specify loadparm or boot menu:
>>>
>>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 
>>> -device virtio-blk-ccw,drive=d1,bootindex=1 
>>
>> I can reproduce this now, too. FWIW: It only happens with
>> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
>> first). Collin, can you reproduce this, too?
>>
>>  Thomas
>> The idea is to support the zipl boot menu on ECKD disks (which have a 
>> timeout by themselves)
> even without switching the boot menu on. This is to have the same behavior as 
> one would see
> on LPAR or z/VM.
> The issue is that the boot code tries to interpret the program table of SCSI 
> bootmaps as if
> a boot menu has been specified.

Ok, thanks for the explanation, makes sense now.

Collin, could you please spin a v9 with the required changes included?

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH v2 1/2] iotest 033: add misaligned write-zeroes test via truncate

[Max Reitz]

On 2018-02-12 14:14, Anton Nefedov wrote:
> This new test case only makes sense for qcow2 while iotest 033 is generic;
> however it matches the test purpose perfectly and also 033 contains those
> do_test() tricks to pass the alignment, which won't look nice being
> duplicated in other tests or moved to the common code.
> 
> Signed-off-by: Anton Nefedov 
> ---
>  tests/qemu-iotests/033 | 28 
>  tests/qemu-iotests/033.out | 13 +
>  2 files changed, 41 insertions(+)
> 
> diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
> index 2cdfd13..5fa3983 100755
> --- a/tests/qemu-iotests/033
> +++ b/tests/qemu-iotests/033
> @@ -64,6 +64,9 @@ do_test()
>   } | $QEMU_IO $IO_EXTRA_ARGS
>  }
>  
> +echo
> +echo "=== Test aligned and misaligned write zeroes operations ==="
> +
>  for write_zero_cmd in "write -z" "aio_write -z"; do
>  for align in 512 4k; do
>   echo
> @@ -102,7 +105,32 @@ for align in 512 4k; do
>  done
>  done
>  
> +
> +# Trigger truncate that would shrink qcow2 L1 table, which is done by
> +#   clearing one entry (8 bytes) with bdrv_co_pwrite_zeroes()
> +
> +echo
> +echo "=== Test misaligned write zeroes via truncate ==="
> +echo
> +
> +CLUSTER_SIZE=$((64 * 1024))
> +L2_COVERAGE=$(($CLUSTER_SIZE * $CLUSTER_SIZE / 8))
> +_make_test_img $(($L2_COVERAGE * 2))

There should be a _cleanup_test_img before this or this test will fail
with nbd.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 20/67] target/arm: Implement SVE Compute Vector Address Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  5 +
>  target/arm/sve_helper.c| 40 
>  target/arm/translate-sve.c | 33 +
>  target/arm/sve.decode  | 12 
>  4 files changed, 90 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 21/67] target/arm: Implement SVE floating-point exponential accelerator

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  4 +++
>  target/arm/sve_helper.c| 81 
> ++
>  target/arm/translate-sve.c | 22 +
>  target/arm/sve.decode  |  7 
>  4 files changed, 114 insertions(+)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index 5280d375f9..e2925ff8ec 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -385,6 +385,10 @@ DEF_HELPER_FLAGS_4(sve_adr_p64, TCG_CALL_NO_RWG, void, 
> ptr, ptr, ptr, i32)
>  DEF_HELPER_FLAGS_4(sve_adr_s32, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
>  DEF_HELPER_FLAGS_4(sve_adr_u32, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
>
> +DEF_HELPER_FLAGS_3(sve_fexpa_h, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_3(sve_fexpa_s, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_3(sve_fexpa_d, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
> +
>  DEF_HELPER_FLAGS_5(sve_and_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_bic_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_eor_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index a290a58c02..4d42653eef 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1101,3 +1101,84 @@ void HELPER(sve_adr_u32)(void *vd, void *vn, void *vm, 
> uint32_t desc)
>  d[i] = n[i] + ((uint64_t)(uint32_t)m[i] << sh);
>  }
>  }
> +
> +void HELPER(sve_fexpa_h)(void *vd, void *vn, uint32_t desc)
> +{
> +static const uint16_t coeff[] = {
> +0x, 0x0016, 0x002d, 0x0045, 0x005d, 0x0075, 0x008e, 0x00a8,
> +0x00c2, 0x00dc, 0x00f8, 0x0114, 0x0130, 0x014d, 0x016b, 0x0189,
> +0x01a8, 0x01c8, 0x01e8, 0x0209, 0x022b, 0x024e, 0x0271, 0x0295,
> +0x02ba, 0x02e0, 0x0306, 0x032e, 0x0356, 0x037f, 0x03a9, 0x03d4,
> +};

Worth a comment that these data tables are from the specification
pseudocode, I think.

> +void HELPER(sve_fexpa_d)(void *vd, void *vn, uint32_t desc)
> +{
> +static const uint64_t coeff[] = {
> +0x0, 0x02C9A3E778061, 0x059B0D3158574, 0x0874518759BC8,
> +0x0B5586CF9890F, 0x0E3EC32D3D1A2, 0x11301D0125B51, 0x1429AAEA92DE0,
> +0x172B83C7D517B, 0x1A35BEB6FCB75, 0x1D4873168B9AA, 0x2063B88628CD6,
> +0x2387A6E756238, 0x26B4565E27CDD, 0x29E9DF51FDEE1, 0x2D285A6E4030B,
> +0x306FE0A31B715, 0x33C08B26416FF, 0x371A7373AA9CB, 0x3A7DB34E59FF7,
> +0x3DEA64C123422, 0x4160A21F72E2A, 0x44E086061892D, 0x486A2B5C13CD0,
> +0x4BFDAD5362A27, 0x4F9B2769D2CA7, 0x5342B569D4F82, 0x56F4736B527DA,
> +0x5AB07DD485429, 0x5E76F15AD2148, 0x6247EB03A5585, 0x6623882552225,
> +0x6A09E667F3BCD, 0x6DFB23C651A2F, 0x71F75E8EC5F74, 0x75FEB564267C9,
> +0x7A11473EB0187, 0x7E2F336CF4E62, 0x82589994CCE13, 0x868D99B4492ED,
> +0x8ACE5422AA0DB, 0x8F1AE99157736, 0x93737B0CDC5E5, 0x97D829FDE4E50,
> +0x9C49182A3F090, 0xA0C667B5DE565, 0xA5503B23E255D, 0xA9E6B5579FDBF,
> +0xAE89F995AD3AD, 0xB33A2B84F15FB, 0xB7F76F2FB5E47, 0xBCC1E904BC1D2,
> +0xC199BDD85529C, 0xC67F12E57D14B, 0xCB720DCEF9069, 0xD072D4A07897C,
> +0xD5818DCFBA487, 0xDA9E603DB3285, 0xDFC97337B9B5F, 0xE502EE78B3FF6,
> +0xEA4AFA2A490DA, 0xEFA1BEE615A27, 0xF50765B6E4540, 0xFA7C1819E90D8,

This confused me at first because it looks like these are 64-bit numbers
but they are only 52 bits. Maybe comment? (or add the leading '000'?)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-img: Make resize error message more general

[Max Reitz]

On 2018-02-05 17:27, Max Reitz wrote:
> The issue:
> 
>   $ qemu-img resize -f qcow2 foo.qcow2
>   qemu-img: Expecting one image file name
>   Try 'qemu-img --help' for more information
> 
> So we gave an image file name, but we omitted the length.  qemu-img
> thinks the last argument is always the size and removes it immediately
> from argv (by decrementing argc), and tries to verify that it is a valid
> size only at a later point.
> 
> So we do not actually know whether that last argument we called "size"
> is indeed a size or whether the user instead forgot to specify that size
> but did give a file name.
> 
> Therefore, the error message should be more general.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1523458
> Signed-off-by: Max Reitz 
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to my block branch.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate()

[Max Reitz]

On 2018-02-14 21:49, Max Reitz wrote:
> For (x-)blockdev-create, all protocol drivers that support image
> creation also need to offer a .bdrv_truncate() implementation that
> matches in features.  A previous series of mine brought gluster's and
> sheepdog's implementation up to par regarding preallocated truncation;
> but I forgot about drivers that have a .bdrv_create() but no
> .bdrv_truncate() at all.
> 
> There is only one of these, and that is ssh.  Since libssh2 does not
> seem to know any way of truncating files, we can only support growing
> files -- but that is what we need for (x-)blockdev-create.
> 
> Note that there are also drivers which do not support growing files,
> namely iscsi and file-posix for host devices (maybe more?  I hope not).
> But these also pretty much ignore the specified size on .bdrv_create()
> and just use the size of the existing device.  They just check that the
> specified size does not exceed the actual size, so that pretty much
> matches what their .bdrv_truncate() supports, and we should be fine
> there.
> 
> 
> Max Reitz (3):
>   block/ssh: Pull ssh_grow_file() from ssh_create()
>   block/ssh: Make ssh_grow_file() blocking
>   block/ssh: Add basic .bdrv_truncate()
> 
>  block/ssh.c | 61 
> +
>  1 file changed, 53 insertions(+), 8 deletions(-)

Applied to my block branch.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 22/67] target/arm: Implement SVE floating-point trig select coefficient

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  4 
>  target/arm/sve_helper.c| 43 +++
>  target/arm/translate-sve.c | 19 +++
>  target/arm/sve.decode  |  4 
>  4 files changed, 70 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3] qcow2: Replace align_offset() with ROUND_UP()

[Max Reitz]

On 2018-02-15 14:10, Alberto Garcia wrote:
> The align_offset() function is equivalent to the ROUND_UP() macro so
> there's no need to use the former. The ROUND_UP() name is also a bit
> more explicit.
> 
> This patch uses ROUND_UP() instead of the slower QEMU_ALIGN_UP()
> because align_offset() already requires that the second parameter is a
> power of two.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Eric Blake 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
> v3 is the same as v2, but rebased on top of the current master fixing
> a merge conflict.
> ---
>  block/qcow2-bitmap.c   |  4 ++--
>  block/qcow2-cluster.c  |  4 ++--
>  block/qcow2-refcount.c |  4 ++--
>  block/qcow2-snapshot.c | 10 +-
>  block/qcow2.c  | 14 +++---
>  block/qcow2.h  |  6 --
>  6 files changed, 18 insertions(+), 24 deletions(-)

Thanks, applied to my block branch:

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

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 23/67] target/arm: Implement SVE Element Count Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  11 ++
>  target/arm/sve_helper.c| 136 ++
>  target/arm/translate-sve.c | 274 
> -
>  target/arm/sve.decode  |  30 -
>  4 files changed, 448 insertions(+), 3 deletions(-)
>

> @@ -127,7 +132,9 @@ static void do_vector3_z(DisasContext *s, GVecGen3Fn 
> *gvec_fn,
>  /* Invoke a vector move on two Zregs.  */
>  static void do_mov_z(DisasContext *s, int rd, int rn)
>  {
> -do_vector2_z(s, tcg_gen_gvec_mov, 0, rd, rn);
> +if (rd != rn) {
> +do_vector2_z(s, tcg_gen_gvec_mov, 0, rd, rn);
> +}
>  }
>
>  /* Initialize a Zreg with replications of a 64-bit immediate.  */
> @@ -168,7 +175,9 @@ static void do_vecop4_p(DisasContext *s, const GVecGen4 
> *gvec_op,
>  /* Invoke a vector move on two Pregs.  */
>  static void do_mov_p(DisasContext *s, int rd, int rn)
>  {
> -do_vector2_p(s, tcg_gen_gvec_mov, 0, rd, rn);
> +if (rd != rn) {
> +do_vector2_p(s, tcg_gen_gvec_mov, 0, rd, rn);
> +}
>  }

These should probably be squashed into an earlier patch.


Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 24/67] target/arm: Implement SVE Bitwise Immediate Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 50 
> ++
>  target/arm/sve.decode  | 17 
>  2 files changed, 67 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/6] vhost-user-test: do not hang if chardev creation failed

[Maxime Coquelin]

On 02/15/2018 10:25 PM, Marc-André Lureau wrote:

Before the chardev name fix, the following error may happen: "attempt
to add duplicate property 'chr-test' to object (type 'container')",
due to races.

Sadly, error_vprintf() uses g_test_message(), so you have to use
read the cryptic --debug-log to see it. Later, it would make sense to
use g_critical() instead, and catch errors with
g_test_expect_message() (in glib 2.34).

Signed-off-by: Marc-André Lureau 
---
  tests/vhost-user-test.c | 1 +
  1 file changed, 1 insertion(+)

Acked-by: Maxime Coquelin 

Thanks,
Maxime

Re: [Qemu-devel] [PATCH v2 25/67] target/arm: Implement SVE Integer Wide Immediate - Predicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 

> +/* Two operand predicated copy immediate with merge.  All valid immediates
> + * can fit within 17 signed bits in the simd_data field.
> + */
> +void HELPER(sve_cpy_m_b)(void *vd, void *vn, void *vg,
> + uint64_t mm, uint32_t desc)
> +{
> +intptr_t i, opr_sz = simd_oprsz(desc) / 8;
> +uint64_t *d = vd, *n = vn;
> +uint8_t *pg = vg;
> +
> +mm = (mm & 0xff) * (-1ull / 0xff);

What is this expression doing? I guess from context that it's
replicating the low 8 bits of mm across the 64-bit value,
but this is too obscure to do without a comment or wrapping
it in a helper function with a useful name, I think.


Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/9] nbd block status base:allocation

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH 0/9] nbd block status base:allocation
Type: series
Message-id: 1518702707-7077-1-git-send-email-vsement...@virtuozzo.com

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20180223125047.343-1-be...@igalia.com -> 
patchew/20180223125047.343-1-be...@igalia.com
Submodule 'capstone' (git://git.qemu.org/capstone.git) registered for path 
'capstone'
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (git://git.qemu.org/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (git://git.qemu-project.org/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/ipxe' (git://git.qemu-project.org/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (git://git.qemu-project.org/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (git://git.qemu-project.org/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (git://github.com/rth7680/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (git://git.qemu-project.org/seabios.git/) registered 
for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (git://github.com/hdeller/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (git://git.qemu-project.org/sgabios.git) registered 
for path 'roms/sgabios'
Submodule 'roms/skiboot' (git://git.qemu.org/skiboot.git) registered for path 
'roms/skiboot'
Submodule 'roms/u-boot' (git://git.qemu-project.org/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/vgabios' (git://git.qemu-project.org/vgabios.git/) registered 
for path 'roms/vgabios'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'd4e7d7ac663fcb55f1b93575445fcbca372f17a7'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'fa981320a1e0968d6fc1b8de319723ff8212b337'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 
'0600d3ae94f93efd10fc6b3c7420a9557a3a1670'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out 
'54d959d97fb331708767b2fd4a878efd2bbc41bb'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 
'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out 
'f3c7e44c70254975df2a00af39701eafbac4d471'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 
'63451fca13c75870e1703eb3e20584d91179aebc'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out 
'649e6202b8d65d46c69f542b1380f840fbe8ab13'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 
'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 
'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 
'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/vgabios'...
Submodule path 'roms/vgabios': checked out 
'19ea12c230ded95928ecaef0db47a82231c2e485'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
7d95dcd iotests: new test 206 for NBD BLOCK_STATUS
906b016 iotests: add file_path helper
015ee72 iotests.py: tiny refactor: move system imports up
1377201 nbd: BLOCK_STATUS for standard get_block_status function: client part
a750bdb nbd/client: fix error messages in nbd_handle_reply_err
6ec6604 block/nbd-client: save first fatal error in nbd_iter_error
1b609ef nbd: BLOCK_STATUS for standard get_block_status function: server part
ac6e460 nbd: change indenting in nbd.h
5e399e1 nbd/server: add nbd_opt_invalid helper

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=204371
SHELL=/bin/sh
USER=patchew
PATCHEW=/home/patchew/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp

Re: [Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Maxime Coquelin]

On 02/15/2018 10:25 PM, Marc-André Lureau wrote:

This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
modify test_read_guest_mem() to use different chardev names, when
using memfd (_test_server_free(), where the chardev is removed, runs
in idle).

Signed-off-by: Marc-André Lureau
---
  tests/vhost-user-test.c | 93 +++--
  1 file changed, 66 insertions(+), 27 deletions(-)


Looks good to me:
Acked-by: Maxime Coquelin 

Thanks,
Maxime

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 26/67] target/arm: Implement SVE Permute - Extract Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  2 ++
>  target/arm/sve_helper.c| 81 
> ++
>  target/arm/translate-sve.c | 29 +
>  target/arm/sve.decode  |  9 +-
>  4 files changed, 120 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index 79493ab647..94f4356ce9 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -414,6 +414,8 @@ DEF_HELPER_FLAGS_4(sve_cpy_z_h, TCG_CALL_NO_RWG, void, 
> ptr, ptr, i64, i32)
>  DEF_HELPER_FLAGS_4(sve_cpy_z_s, TCG_CALL_NO_RWG, void, ptr, ptr, i64, i32)
>  DEF_HELPER_FLAGS_4(sve_cpy_z_d, TCG_CALL_NO_RWG, void, ptr, ptr, i64, i32)
>
> +DEF_HELPER_FLAGS_4(sve_ext, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
> +
>  DEF_HELPER_FLAGS_5(sve_and_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_bic_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(sve_eor_, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 6a95d1ec48..fb3f54300b 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1469,3 +1469,84 @@ void HELPER(sve_cpy_z_d)(void *vd, void *vg, uint64_t 
> val, uint32_t desc)
>  d[i] = (pg[H1(i)] & 1 ? val : 0);
>  }
>  }
> +
> +/* Big-endian hosts need to frob the byte indicies.  If the copy
> + * happens to be 8-byte aligned, then no frobbing necessary.
> + */

Have you run risu tests with a big endian host?

>  ###
>  # Named fields.  These are primarily for disjoint fields.
>
> -%imm4_16_p1 16:4 !function=plus1
> +%imm4_16_p116:4 !function=plus1

Another bit that should be squashed into an earlier patch.

>  %imm6_22_5 22:1 5:5
> +%imm8_16_1016:5 10:3
>  %imm9_16_1016:s6 10:3
>  %preg4_5   5:4
>
> @@ -363,6 +364,12 @@ FCPY   0101 .. 01  110 imm:8 .   
>   @rdn_pg4
>  CPY_m_i0101 .. 01  01 .  .   @rdn_pg4 
> imm=%sh8_i8s
>  CPY_z_i0101 .. 01  00 .  .   @rdn_pg4 
> imm=%sh8_i8s
>
> +### SVE Permute - Extract Group
> +
> +# SVE extract vector (immediate offset)
> +EXT0101 001 . 000 ... rm:5 rd:5 \
> +   &rrri rn=%reg_movprfx imm=%imm8_16_10
> +
>  ### SVE Predicate Logical Operations Group
>
>  # SVE predicate logical operations
> --

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 1/3] s390x/sclp: proper support of larger send and receive masks

[Christian Borntraeger]

On 02/23/2018 01:51 PM, Claudio Imbrenda wrote:
> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported sclp event masks with a size of exactly 4 bytes, even
> though the architecture allows the guests to set up sclp event masks
> from 1 to 1021 bytes in length.
> After that patch, the behaviour was almost compliant, but some issues
> were still remaining, in particular regarding the handling of selective
> reads and migration.
> 
> When setting the sclp event mask, a mask size is also specified. Until
> now we only considered the size in order to decide which bits to save
> in the internal state. On the other hand, when a guest performs a
> selective read, it sends a mask, but it does not specify a size; the
> implied size is the size of the last mask that has been set.
> 
> Specifying bits in the mask of selective read that are not available in
> the internal mask should return an error, and bits past the end of the
> mask should obviously be ignored. This can only be achieved by keeping
> track of the lenght of the mask.
> 
> The mask length is thus now part of the internal state that needs to be
> migrated.
> 
> This patch fixes the handling of selective reads, whose size will now
> match the length of the event mask, as per architecture.
> 
> While the default behaviour is to be compliant with the architecture,
> when using older machine models the old broken behaviour is selected
> (allowing only masks of size exactly 4), in order to be able to migrate
> toward older versions.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")
> Signed-off-by: Claudio Imbrenda 
> ---
>  hw/s390x/event-facility.c  | 91 
> +++---
>  hw/s390x/s390-virtio-ccw.c |  8 +++-
>  2 files changed, 85 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 155a694..b7cd075 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -31,6 +31,15 @@ struct SCLPEventFacility {
>  SCLPEventsBus sbus;
>  /* guest' receive mask */
>  unsigned int receive_mask;
> +/*
> + * when false, we keep the same broken, backwards compatible behaviour as
> + * before, allowing only masks of size exactly 4; when true, we implement
> + * the architecture correctly, allowing all valid mask sizes. Needed for
> + * migration toward older versions.
> + */
> +bool allow_all_mask_sizes;
> +/* length of the receive mask */
> +uint16_t mask_length;
>  };
> 
>  /* return true if any child has event pending set */
> @@ -220,6 +229,17 @@ static uint16_t 
> handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
>  return rc;
>  }
> 
> +/* copy up to dst_len bytes and fill the rest of dst with zeroes */

you just moved this function but shouldnt it be

src_len bytes and fill will zeroes until dst_len?

> +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> +  uint16_t src_len)
> +{
> +int i;
> +
> +for (i = 0; i < dst_len; i++) {
> +dst[i] = i < src_len ? src[i] : 0;
> +}
> +}
> +
>  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
>  {
>  unsigned int sclp_active_selection_mask;
> @@ -240,7 +260,9 @@ static void read_event_data(SCLPEventFacility *ef, SCCB 
> *sccb)
>  sclp_active_selection_mask = sclp_cp_receive_mask;
>  break;
>  case SCLP_SELECTIVE_READ:
> -sclp_active_selection_mask = be32_to_cpu(red->mask);
> +copy_mask((uint8_t *)&sclp_active_selection_mask, (uint8_t 
> *)&red->mask,
> +  sizeof(sclp_active_selection_mask), ef->mask_length);
> +sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
>  if (!sclp_cp_receive_mask ||
>  (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
>  sccb->h.response_code =
> @@ -259,24 +281,14 @@ out:
>  return;
>  }
> 
> -/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> -static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> -  uint16_t src_len)
> -{
> -int i;
> -
> -for (i = 0; i < dst_len; i++) {
> -dst[i] = i < src_len ? src[i] : 0;
> -}
> -}
> -
>  static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
>  {
>  WriteEventMask *we_mask = (WriteEventMask *) sccb;
>  uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
>  uint32_t tmp_mask;
> 
> -if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX)) {
> +if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
> +((mask_length != 4) && !ef->allow_all_mask_sizes)) {
>  sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
>  goto out;
>  }
> @@ -301,6 +313,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB 
> *sccb)
>mask_length,

Re: [Qemu-devel] [PATCH v3 2/3] s390x/sclp: clean up sclp masks

[Christian Borntraeger]

On 02/23/2018 01:51 PM, Claudio Imbrenda wrote:
> Introduce an sccb_mask_t to be used for SCLP event masks instead of just
> unsigned int or uint32_t. This will allow later to extend the mask with
> more ease.
> 
> Signed-off-by: Claudio Imbrenda 

Reviewed-by: Christian Borntraeger 

Re: [Qemu-devel] [PATCH v2 27/67] target/arm: Implement SVE Permute - Unpredicated Group

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  23 +
>  target/arm/translate-a64.h |  14 +++---
>  target/arm/sve_helper.c| 114 
> +
>  target/arm/translate-sve.c | 113 
>  target/arm/sve.decode  |  29 +++-
>  5 files changed, 285 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
> index e519aee314..328aa7fce1 100644
> --- a/target/arm/translate-a64.h
> +++ b/target/arm/translate-a64.h
> @@ -66,18 +66,18 @@ static inline void assert_fp_access_checked(DisasContext 
> *s)
>  static inline int vec_reg_offset(DisasContext *s, int regno,
>   int element, TCGMemOp size)
>  {
> -int offs = 0;
> +int element_size = 1 << size;
> +int offs = element * element_size;
>  #ifdef HOST_WORDS_BIGENDIAN
>  /* This is complicated slightly because vfp.zregs[n].d[0] is
>   * still the low half and vfp.zregs[n].d[1] the high half
>   * of the 128 bit vector, even on big endian systems.
> - * Calculate the offset assuming a fully bigendian 128 bits,
> - * then XOR to account for the order of the two 64 bit halves.
> + * Calculate the offset assuming a fully little-endian 128 bits,
> + * then XOR to account for the order of the 64 bit units.
>   */
> -offs += (16 - ((element + 1) * (1 << size)));
> -offs ^= 8;
> -#else
> -offs += element * (1 << size);
> +if (element_size < 8) {
> +offs ^= 8 - element_size;
> +}
>  #endif
>  offs += offsetof(CPUARMState, vfp.zregs[regno]);
>  assert_fp_access_checked(s);

This looks like it should have been in an earlier patch?

> @@ -85,7 +86,9 @@
>  @pd_pg_pn_pm_s  . s:1 .. rm:4 .. pg:4 . rn:4 . rd:4&rprr_s
>
>  # Three operand, vector element size
> -@rd_rn_rm   esz:2 . rm:5  ... ...  rn:5 rd:5   &rrr_esz
> +@rd_rn_rm   esz:2 . rm:5 ... ... rn:5 rd:5 &rrr_esz

Another fragment that should be squashed.

> +@rdn_rm esz:2 .. .. rm:5 rd:5 \
> +   &rrr_esz rn=%reg_movprfx
>
>  # Three operand with "memory" size, aka immediate left shift
>  @rd_rn_msz_rm   ... rm:5  imm:2 rn:5 rd:5  &rrri

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] intel-iommu and vhost: Do we need 'device-iotlb' and 'ats'?

[Jintack Lim]

On Fri, Feb 23, 2018 at 2:09 AM, Peter Xu  wrote:
> On Fri, Feb 23, 2018 at 06:34:04AM +, Jintack Lim wrote:
>> On Fri, Feb 23, 2018 at 1:10 AM Peter Xu  wrote:
>>
>> > On Fri, Feb 23, 2018 at 12:32:13AM -0500, Jintack Lim wrote:
>> > > Hi Peter,
>> > >
>> > > Hope you had great holidays!
>> > >
>> > > On Thu, Feb 22, 2018 at 10:55 PM, Peter Xu  wrote:
>> > > > On Tue, Feb 20, 2018 at 11:03:46PM -0500, Jintack Lim wrote:
>> > > >> Hi,
>> > > >>
>> > > >> I'm using vhost with the virtual intel-iommu, and this page[1] shows
>> > > >> the QEMU command line example.
>> > > >>
>> > > >> qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split -m 2G \
>> > > >>-device intel-iommu,intremap=on,device-iotlb=on \
>> > > >>-device ioh3420,id=pcie.1,chassis=1 \
>> > > >>-device
>> > > >>
>> > virtio-net-pci,bus=pcie.1,netdev=net0,disable-legacy=on,disable-modern=off,iommu_platform=on,ats=on
>> > > >> \
>> > > >>-netdev tap,id=net0,vhostforce \
>> > > >>$IMAGE_PATH
>> > > >>
>> > > >> I wonder what's the impact of using device-iotlb and ats options as
>> > > >> they are described necessary.
>> > > >>
>> > > >> In my understanding, vhost in the kernel only looks at
>> > > >> VIRTIO_F_IOMMU_PLATFORM, and when it is set, vhost uses a
>> > > >> device-iotlb. In addition, vhost and QEMU communicate using vhost_msg
>> > > >> basically to cache mappings correctly in the vhost, so I wonder what's
>> > > >> the role of ats in this case.
>> > > >
>> > > > The "ats" as virtio device parameter will add ATS capability to the
>> > > > PCI device.
>> > > >
>> > > > The "device-iotlb" as intel-iommu parameter will enable ATS in the
>> > > > IOMMU device (and also report that in ACPI field).
>> > > >
>> > > > If both parameters are provided IIUC it means guest will know virtio
>> > > > device has device-iotlb and it'll treat the device specially (e.g.,
>> > > > guest will need to send device-iotlb invalidations).
>> > >
>> > > Oh, I see. I was focusing on how QEMU and vhost work in the host, but
>> > > I think I missed the guest part! Thanks. I see that the Intel IOMMU
>> > > driver has has_iotlb_device flag for that purpose.
>> > >
>> > > >
>> > > > We'd better keep these parameters when running virtio devices with
>> > > > vIOMMU.  For the rest of vhost/arm specific questions, I'll leave to
>> > > > others.
>> > >
>> > > It seems like SMMU is not checking ATS capability - at least
>> > > ats_enabled flag - but I may miss something here as well :)
>> > >
>> > > >
>> > > > PS: Though IIUC the whole ATS thing may not really be necessary for
>> > > > current VT-d emulation, since even with ATS vhost is registering UNMAP
>> > > > IOMMU notifiers (see vhost_iommu_region_add()), and IIUC that means
>> > > > vhost will receive IOTLB invalidations even without ATS support, and
>> > > > it _might_ still work.
>> > >
>> > > Right. That's what I thought.
>> > >
>> > > Come to think of it, I'm not sure why we need to flush mappings in
>> > > IOMMU and devices separately in the first place... Any thoughts?
>> >
>> > I don't know ATS much, neither.
>> >
>> > You can have a look at chap 4 of vt-d spec:
>> >
>> > One approach to scaling IOTLBs is to enable I/O devices to
>> > participate in the DMA remapping with IOTLBs implemented at
>> > the devices. The Device-IOTLBs alleviate pressure for IOTLB
>> > resources in the core logic, and provide opportunities for
>> > devices to improve performance by pre-fetching address
>> > translations before issuing DMA requests. This may be useful
>> > for devices with strict DMA latency requirements (such as
>> > isochronous devices), and for devices that have large DMA
>> > working set or multiple active DMA streams.
>> >
>> > So I think it's for performance's sake. For example, the DMA operation
>> > won't need to be translated at all if it's pre-translated, so it can
>> > have less latency.  And also, that'll offload some of the translation
>> > process so that workload can be more distributed.
>> >
>> > When with that (caches located both on IOMMU's and device's side), we
>> > need to invalidate all the cache when needed.
>> >
>>
>> Right. I think my question was not clear. My question was that why don’t
>> IOMMU invalidate device-iotlb along with its mappings in one go. Then IOMMU
>> device driver doesn’t need to flush device-iotlb explicitly. Maybe the
>> reason is that ATS and IOMMU are not always coupled.. but I guess it’s time
>> for me to get some more background :)
>
> Ah, I see your point.
>
> I don't know the answer.  My wild guess is that IOMMU is just trying
> to be simple and only provide most basic functionalities, leaving
> complex stuff to CPU.  For example, if IOMMU takes over the ownership
> to deliever device-iotlb invalidations when receiving iotlb
> invalidations, it possibly needs to traverse the device tree someti

Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer

[Peter Maydell]

On 16 February 2018 at 16:46, Brad Smith  wrote:
> Add myself as an OpenBSD maintainer and add OpenBSD as maintained.
>
> Signed-off-by: Brad Smith 
>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 57358a08e2..86329e274f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -386,6 +386,12 @@ M: Kamil Rytarowski 
>  S: Maintained
>  K: ^Subject:.*(?i)NetBSD
>
> +OPENBSD
> +L: qemu-devel@nongnu.org
> +M: Brad Smith 
> +S: Maintained
> +K: ^Subject:.*(?i)OpenBSD
> +
>  W32, W64
>  L: qemu-devel@nongnu.org
>  M: Stefan Weil 
> diff --git a/configure b/configure
> index 913e14839d..850c46ebac 100755
> --- a/configure
> +++ b/configure
> @@ -760,6 +760,7 @@ OpenBSD)
>audio_drv_list="sdl"
>audio_possible_drivers="sdl"
>HOST_VARIANT_DIR="openbsd"
> +  supported_os="yes"
>  ;;
>  Darwin)
>bsd="yes"

Thanks for agreeing to help us with the BSD host maintenance.
I've applied this patch to master. If you and Kamil want to
maintain a subtree for doing pull requests for BSD host
patches that's fine with me; otherwise we can continue to
ad-hoc apply patches through whatever route is simplest.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 3/3] s390x/sclp: extend SCLP event masks to 64 bits

[Christian Borntraeger]

On 02/23/2018 01:51 PM, Claudio Imbrenda wrote:

> 
> +static bool vmstate_event_facility_mask64_needed(void *opaque)
> +{
> +SCLPEventFacility *ef = opaque;
> +
> +return (ef->receive_mask & 0x) != 0;
> +}
> +
> +static int vmstate_event_facility_mask64_pre_load(void *opaque)
> +{
> +SCLPEventFacility *ef = opaque;
> +
> +ef->receive_mask &= ~0xULL;
> +return 0;
> +}

again, no need for a pre-load here. The ef->receive_mask should start out as 0.

Re: [Qemu-devel] [PATCH v2 16/36] file-win32: Support .bdrv_co_create

[Eric Blake]

On 02/21/2018 07:53 AM, Kevin Wolf wrote:

This adds the .bdrv_co_create driver callback to file-win32, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
  block/file-win32.c | 45 +
  1 file changed, 37 insertions(+), 8 deletions(-)




+
+options = (BlockdevCreateOptions) {
+.driver = BLOCKDEV_DRIVER_FILE,
+.u.file = {
+.filename   = (char *) filename,
+.size   = total_size,
+.has_preallocation  = false,
+.has_nocow  = false,


Technically, these last two lines aren't needed (you get false by 
default), but it doesn't hurt to list them either.


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v3 3/3] util/uri.c: add brackets to `if` statements

[苏航]

Thanks for your reply. ^_^
I will apply all your suggestion in my next patch.

> -Original Messages-
> From: "Thomas Huth" 
> Sent Time: 2018-02-23 17:34:12 (Friday)
> To: "Su Hang" , stefa...@redhat.com
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH v3 3/3] util/uri.c: add brackets to `if` 
> statements
> 
> On 23.02.2018 08:51, Su Hang wrote:
> > Add brackets that wrap `if`, `else`, `while` that hold single
> > statements.
> > 
> > In order to do this, I write a simple python regex script.
> > 
> > Since then, all complaints rised by checkpatch.pl has been suppressed.
> > 
> > Signed-off-by: Su Hang 
> > ---
> >  util/uri.c | 462 
> > ++---
> >  1 file changed, 291 insertions(+), 171 deletions(-)
> > 
> > diff --git a/util/uri.c b/util/uri.c
> > index 278e876ab8b1..48f7298787b1 100644
> > --- a/util/uri.c
> > +++ b/util/uri.c
> > @@ -105,18 +105,18 @@ static void uri_clean(URI *uri);
> >   */
> >  
> >  #define IS_UNWISE(p)   
> > \
> > -  (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||
> > \
> > -   ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) ||   
> > \
> > -   ((*(p) == ']')) || ((*(p) == '`')))
> > +(((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||  
> > \
> > + ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) || 
> > \
> > + ((*(p) == ']')) || ((*(p) == '`')))
> >  /*
> >   * reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | "," |
> >   *"[" | "]"
> >   */
> >  
> >  #define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') ||
> > \
> > -((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') ||
> > \
> > -((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') ||
> > \
> > -((x) == ']'))
> > +  ((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') ||  
> > \
> > +  ((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') ||  
> > \
> > +  ((x) == ']'))
> 
> The above whitespace changes should ideally be done in the first patch 
> instead.
> 
> >  /*
> >   * unreserved = alphanum | mark
> > @@ -211,15 +211,17 @@ static int rfc3986_parse_scheme(URI *uri, const char 
> > **str)
> >  {
> >  const char *cur;
> >  
> > -if (str == NULL)
> > +if (str == NULL) {
> >  return -1;
> > +}
> >  
> >  cur = *str;
> > -if (!ISA_ALPHA(cur))
> > +if (!ISA_ALPHA(cur)) {
> >  return 2;
> > +}
> >  cur++;
> > -while (ISA_ALPHA(cur) || ISA_DIGIT(cur) ||
> > -   (*cur == '+') || (*cur == '-') || (*cur == '.'))
> > +while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == 
> > '-') ||
> > +   (*cur == '.'))
> >  cur++;
> 
> You've changed the while statement, but checkpatch.pl apparently does not
> complain about missing curly braces here ... that's strange, I thought we'd
> also wanted to enforce curly braces for while loops. Anyway, could you please
> add curly braces around the "*cur++;" here, too?
> 
> > @@ -1437,15 +1503,18 @@ done_cd:
> >  /* string will overlap, do not use strcpy */
> >  tmp = cur;
> >  segp += 3;
> > -while ((*tmp++ = *segp++) != 0)
> > +while ((*tmp++ = *segp++) != 0) {
> >  ;
> > +}
> 
> A bikeshed-painting-friday question for everybody on qemu-devel:
> Should there be a single semicolon inside curly braces in this case, or not?
> 
> >  /* If there are no previous segments, then keep going from here.  
> > */
> >  segp = cur;
> > -while ((segp > path) && ((--segp)[0] == '/'))
> > +while ((segp > path) && ((--segp)[0] == '/')) {
> >  ;
> 
> (dito)
> 
> > -if (segp == path)
> > +}
> > +if (segp == path) {
> >  continue;
> > +}
> >  
> >  /* "segp" is pointing to the end of a previous segment; find it's
> >   * start.  We need to back up to the previous segment and start
> [...]
> > @@ -1491,8 +1562,9 @@ done_cd:
> >  static int is_hex(char c)
> >  {
> >  if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
> > -((c >= 'A') && (c <= 'F')))
> > +((c >= 'A') && (c <= 'F'))) {
> >  return 1;
> > +}
> >  return 0;
> >  }
> 
> Not related to your patch, but an idea for a future clean-up patch:
> We've already got qemu_isxdigit(), so there is no real need for this
> separate is_hex() function.
> 
> [...]
> > @@ -2020,17 +2127,19 @@ char *uri_resolve_relative(const char *uri, const 
> > char *base)
> >   */
> >  if (bptr[pos] != ref->path[pos]) { /* check for trivial URI == 
> > base */
> >  for (; bptr[ix] != 0; ix++) {
> > -if (bptr[ix] == '/')
> > +if (bptr[i

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Collin L. Walling]

On 02/23/2018 05:11 AM, Christian Borntraeger wrote:


On 02/23/2018 11:07 AM, Thomas Huth wrote:

On 22.02.2018 20:40, Collin L. Walling wrote:

On 02/22/2018 11:45 AM, Collin L. Walling wrote:

On 02/22/2018 10:44 AM, Christian Borntraeger wrote:

On 02/22/2018 04:40 PM, Collin L. Walling wrote:

On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:

On 22.02.2018 12:51, Christian Borntraeger wrote:

Series
Acked-by: Christian Borntraeger 

Thanks!!!


menu on scsi and dasd bootmaps tested successfully.

There is one thing that we might want to fix (can be an addon
patch since this is a non-customer
scenario (no libvirt)).

If you start QEMU manually without a bootindex, the -boot menu=on
is ignored
if no drive has a bootindex.

For example:

-drive file=/dev/dasda,if=none,id=d1 -device
virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
does work

-drive file=/dev/dasda -boot menu=on
does not

instead it prints:
qemu-system-s390x: boot menu is not supported for this device type.

and the boots up the default entry.


That should indeed be a separate patch, as it would move logic
currently
in the BIOS up to QEMU (find the first defined virtio disk and
select it
as boot disk).
In fact it's more complicated than that, because it would have to
properly account for -boot order=[acdn] and produce the respective
IPLB.
While it makes sense, I wouldn't rush that in but rather change the
error message to indicate that -device bootindex is needed to activate
the menu, at least for the time being.
[...]


I can look into it.  Theoretically, the easier fix should just
involve parsing all
of the -device commands and looking for a "bootindex=1" field. The
Qemu options
code already handles a bulk of this work, so it's just a matter of
putting it all
together.

Shall I whip something up and post what I have as a reply to this
email chain?

In fact, it should already be there.

static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
  DeviceState *dev_st;

  dev_st = get_boot_device(0);

--> if this returns 0 we have no bootindex statement anywhere and the
BIOS will IPL the default
disk.



Makes sense.  I'm working on making this patch look as clean as
possible. The fact that no boot menu
options present means we fallback to using zipl values for CCW being
tied into the switch statement
is making things a bit tricky. Just have to think the logic through a
bit.  Will get back to you once
I have something good.


This should do the trick (this can also be squished painlessly into 6/13
if desired)

Patch looks fine to me. I can either take it directly like this, or in
case you have to respin (depends on the problem that Christian reported
with the Ubuntu guest), I'm also fine if you squash it into an earlier
patch instead.

FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also
happens with other guests (e.g. fedora).


Is this on guests using DASD?  If so, a boot menu will appear if the 
appropriate zipl configuration
values are set (even if no -boot menu or loadparm).  Patch 12/13 goes 
into more detail.


Is this not wanted behavior?

--
- Collin L Walling

Re: [Qemu-devel] [PATCH v3 3/3] util/uri.c: add brackets to `if` statements

[Eric Blake]

On 02/23/2018 03:34 AM, Thomas Huth wrote:

On 23.02.2018 08:51, Su Hang wrote:

Add brackets that wrap `if`, `else`, `while` that hold single
statements.

In order to do this, I write a simple python regex script.


Without documenting the script, no one else can reproduce this; but it's 
no different than if they had manually made changes instead of trying to 
script it, so I'm not sure this sentence adds much in its current form.




Since then, all complaints rised by checkpatch.pl has been suppressed.


s/rised/raised/
s/Since then,/With that/
s/has/have/



Signed-off-by: Su Hang 
---
  util/uri.c | 462 ++---
  1 file changed, 291 insertions(+), 171 deletions(-)




  cur = *str;
-if (!ISA_ALPHA(cur))
+if (!ISA_ALPHA(cur)) {
  return 2;
+}
  cur++;
-while (ISA_ALPHA(cur) || ISA_DIGIT(cur) ||
-   (*cur == '+') || (*cur == '-') || (*cur == '.'))
+while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == '-') 
||
+   (*cur == '.'))
  cur++;


You've changed the while statement, but checkpatch.pl apparently does not
complain about missing curly braces here ... that's strange, I thought we'd
also wanted to enforce curly braces for while loops.


Maybe because it gets lost since the condition expanded over more than 
one line?  But yes, now that we've noticed it manually, it should be 
fixed.  While at it, you can avoid the redundant ():


while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || *cur == '+' || *cur == '-' ||
   *cur == '.') {



-while ((*tmp++ = *segp++) != 0)
+while ((*tmp++ = *segp++) != 0) {
  ;
+}


A bikeshed-painting-friday question for everybody on qemu-devel:
Should there be a single semicolon inside curly braces in this case, or not?



Checkpatch doesn't complain, but lone ';' statements are rare.  I'd omit 
it, and use just:


while (cond) {
}


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

Re: [Qemu-devel] intel-iommu and vhost: Do we need 'device-iotlb' and 'ats'?

[Jintack Lim]

Hi Kevin,

On Fri, Feb 23, 2018 at 2:34 AM, Tian, Kevin  wrote:
>> From: Peter Xu
>> Sent: Friday, February 23, 2018 3:09 PM
>>
>> >
>> > Right. I think my question was not clear. My question was that why don’t
>> > IOMMU invalidate device-iotlb along with its mappings in one go. Then
>> IOMMU
>> > device driver doesn’t need to flush device-iotlb explicitly. Maybe the
>> > reason is that ATS and IOMMU are not always coupled.. but I guess it’s
>> time
>> > for me to get some more background :)
>>
>> Ah, I see your point.
>>
>> I don't know the answer.  My wild guess is that IOMMU is just trying
>> to be simple and only provide most basic functionalities, leaving
>> complex stuff to CPU.  For example, if IOMMU takes over the ownership
>> to deliever device-iotlb invalidations when receiving iotlb
>> invalidations, it possibly needs to traverse the device tree sometimes
>> (e.g., for domain invalidations) to know what device is under what
>> domain, which is really compliated.  While it'll be simpler for CPU to
>> do this since it's very possible that the OS keeps a list of devices
>> for a domain already.
>>
>> IMHO that follows the *nix philosophy too - Do One Thing And Do It
>> Well.  Though again, it's wild guess and I may be wrong. :)
>>
>> CCing Alex, in case he has quick answers.
>>
>
> IOMMU and devices are de-coupled. You need a protocol so IOMMU
> knows which device enables translation caches and thus requires
> explicit invalidation, which is how ATS comes to play. ATS is not
> mandatory for vhost, but doing so provides more flexibility e.g.
> to enable I/O page fault if further emulating PCI PRS cap.

Thanks for the explanation!

Thanks,
Jintack

>
> Thanks
> Kevin

[Qemu-devel] [PATCH v3 2/4] target/m68k: add fmod/frem

[Laurent Vivier]

Using a local m68k floatx80_mod()
[copied from previous:
Written by Andreas Grabher for Previous, NeXT Computer Emulator.]

The quotient byte of the FPSR is updated with
the result of the operation.

Signed-off-by: Laurent Vivier 
---
 target/m68k/Makefile.objs |   3 +-
 target/m68k/cpu.h |   1 +
 target/m68k/fpu_helper.c  |  35 +++-
 target/m68k/helper.h  |   2 +
 target/m68k/softfloat.c   | 105 ++
 target/m68k/softfloat.h   |  26 
 target/m68k/translate.c   |   6 +++
 7 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 target/m68k/softfloat.c
 create mode 100644 target/m68k/softfloat.h

diff --git a/target/m68k/Makefile.objs b/target/m68k/Makefile.objs
index d143f20270..ac61948676 100644
--- a/target/m68k/Makefile.objs
+++ b/target/m68k/Makefile.objs
@@ -1,4 +1,5 @@
 obj-y += m68k-semi.o
-obj-y += translate.o op_helper.o helper.o cpu.o fpu_helper.o
+obj-y += translate.o op_helper.o helper.o cpu.o
+obj-y += fpu_helper.o softfloat.o
 obj-y += gdbstub.o
 obj-$(CONFIG_SOFTMMU) += monitor.o
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 65f4fb95cb..2259bf22dc 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -427,6 +427,7 @@ typedef enum {
 /* Quotient */
 
 #define FPSR_QT_MASK  0x00ff
+#define FPSR_QT_SHIFT 16
 
 /* Floating-Point Control Register */
 /* Rounding mode */
diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 3c5a82aaa0..8286228b81 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -23,7 +23,7 @@
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
-#include "fpu/softfloat.h"
+#include "softfloat.h"
 
 /* Undefined offsets may be different on various FPU.
  * On 68040 they return 0.0 (floatx80_zero)
@@ -509,3 +509,36 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, 
uint32_t addr,
 {
 return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
 }
+
+static void make_quotient(CPUM68KState *env, floatx80 val)
+{
+int32_t quotient;
+int sign;
+
+if (floatx80_is_any_nan(val)) {
+return;
+}
+
+quotient = floatx80_to_int32(val, &env->fp_status);
+sign = quotient < 0;
+if (sign) {
+quotient = -quotient;
+}
+
+quotient = (sign << 7) | (quotient & 0x7f);
+env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
+}
+
+void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+res->d = floatx80_mod(val1->d, val0->d, &env->fp_status);
+
+make_quotient(env, res->d);
+}
+
+void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+res->d = floatx80_rem(val1->d, val0->d, &env->fp_status);
+
+make_quotient(env, res->d);
+}
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index 7f400f0def..76a0590c9c 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -63,6 +63,8 @@ DEF_HELPER_3(fmovemx_ld_postinc, i32, env, i32, i32)
 DEF_HELPER_3(fmovemd_st_predec, i32, env, i32, i32)
 DEF_HELPER_3(fmovemd_st_postinc, i32, env, i32, i32)
 DEF_HELPER_3(fmovemd_ld_postinc, i32, env, i32, i32)
+DEF_HELPER_4(fmod, void, env, fp, fp, fp)
+DEF_HELPER_4(frem, void, env, fp, fp, fp)
 
 DEF_HELPER_3(mac_move, void, env, i32, i32)
 DEF_HELPER_3(macmulf, i64, env, i32, i32)
diff --git a/target/m68k/softfloat.c b/target/m68k/softfloat.c
new file mode 100644
index 00..8c77757b4e
--- /dev/null
+++ b/target/m68k/softfloat.c
@@ -0,0 +1,105 @@
+/*
+ * Ported from a work by Andreas Grabher for Previous, NeXT Computer Emulator,
+ * derived from NetBSD M68040 FPSP functions,
+ * derived from release 2a of the SoftFloat IEC/IEEE Floating-point Arithmetic
+ * Package. Those parts of the code (and some later contributions) are
+ * provided under that license, as detailed below.
+ * It has subsequently been modified by contributors to the QEMU Project,
+ * so some portions are provided under:
+ *  the SoftFloat-2a license
+ *  the BSD license
+ *  GPL-v2-or-later
+ *
+ * Any future contributions to this file will be taken to be licensed under
+ * the Softfloat-2a license unless specifically indicated otherwise.
+ */
+
+/* Portions of this work are 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 "softfloat.h"
+#include "fpu/softfloat-macros.h"
+
+/*
+ | Returns the modulo remainder of the extended double-precision floating-point
+ | value `a' with respect to the corresponding value `b'.
+ 
**/
+
+floatx80 floatx80_mod(floatx80 a, floatx80 b, float_status *status)
+{
+flag aSign, zSign;
+int32_t aExp, bExp, expDiff;
+uint64_t aSig0, aSig1, bSig;
+uint64_t qTemp, term0, term1;
+
+aSig0 = extractFloatx80Frac(a);
+aExp =

[Qemu-devel] [PATCH v3 0/4] target/m68k: implement 680x0 FPU (part 3)

[Laurent Vivier]

Implement fmod, frem, fscale, fgetman and fgetexp.

Instead of using functions of libm (v1 of this series)
and converting between host long double and floatx80 type
the new version (v2) adds new floatx80 functions in softfloat.

All the floatx80 functions are copied from "Previous",
the NeXT Computer Emulator, and written by Andreas Grabher.

v3: Move all new functions to target/m68k/softfloat.c
Exports needed functions from fpu/softfloat.c

Laurent Vivier (4):
  softfloat: export some functions
  target/m68k: add fmod/frem
  softfloat: use floatx80_infinity in softfloat
  target/m68k: add fscale, fgetman and fgetexp

 fpu/softfloat-specialize.h  |  17 ++-
 fpu/softfloat.c | 129 +
 {fpu => include/fpu}/softfloat-macros.h |  10 +-
 include/fpu/softfloat.h | 129 -
 target/m68k/Makefile.objs   |   3 +-
 target/m68k/cpu.h   |   1 +
 target/m68k/fpu_helper.c|  50 ++-
 target/m68k/helper.h|   5 +
 target/m68k/softfloat.c | 249 
 target/m68k/softfloat.h |  29 
 target/m68k/translate.c |  15 ++
 11 files changed, 533 insertions(+), 104 deletions(-)
 rename {fpu => include/fpu}/softfloat-macros.h (98%)
 create mode 100644 target/m68k/softfloat.c
 create mode 100644 target/m68k/softfloat.h

-- 
2.14.3

[Qemu-devel] [PATCH v3 1/4] softfloat: export some functions

[Laurent Vivier]

Move fpu/softfloat-macros.h to include/fpu/

Export floatx80 functions to be used by target floatx80
specific implementations.

Exports:
  propagateFloatx80NaN(), extractFloatx80Frac(),
  extractFloatx80Exp(), extractFloatx80Sign(),
  normalizeFloatx80Subnormal(), packFloatx80(),
  roundAndPackFloatx80(), normalizeRoundAndPackFloatx80()

Also exports packFloat32() that will be used to implement
m68k fsinh, fcos, fsin, ftan operations.

Signed-off-by: Laurent Vivier 
---
CC: Aurelien Jarno 
CC: Alex Bennée 
CC: Peter Maydell 

 fpu/softfloat-specialize.h  |   3 +-
 fpu/softfloat.c |  91 +++-
 {fpu => include/fpu}/softfloat-macros.h |  10 +--
 include/fpu/softfloat.h | 120 
 4 files changed, 136 insertions(+), 88 deletions(-)
 rename {fpu => include/fpu}/softfloat-macros.h (98%)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index e81ca001e1..46126e9e0a 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -1011,8 +1011,7 @@ static floatx80 commonNaNToFloatx80(commonNaNT a, 
float_status *status)
 | `b' is a signaling NaN, the invalid exception is raised.
 **/
 
-static floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b,
- float_status *status)
+floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
 {
 flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN;
 flag aIsLargerSignificand;
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index e7fb0d357a..fb4853682e 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -93,7 +93,7 @@ this code that are retained.
 | division and square root approximations.  (Can be specialized to target if
 | desired.)
 **/
-#include "softfloat-macros.h"
+#include "fpu/softfloat-macros.h"
 
 /*
 | Functions and definitions to determine:  (1) whether tininess for underflow
@@ -2192,25 +2192,6 @@ static void
 
 }
 
-/*
-| Packs the sign `zSign', exponent `zExp', and significand `zSig' into a
-| single-precision floating-point value, returning the result.  After being
-| shifted into the proper positions, the three fields are simply added
-| together to form the result.  This means that any integer portion of `zSig'
-| will be added into the exponent.  Since a properly normalized significand
-| will have an integer portion equal to 1, the `zExp' input should be 1 less
-| than the desired result exponent whenever `zSig' is a complete, normalized
-| significand.
-**/
-
-static inline float32 packFloat32(flag zSign, int zExp, uint32_t zSig)
-{
-
-return make_float32(
-  ( ( (uint32_t) zSign )<<31 ) + ( ( (uint32_t) zExp )<<23 ) + zSig);
-
-}
-
 /*
 | Takes an abstract floating-point value having sign `zSign', exponent `zExp',
 | and significand `zSig', and returns the proper single-precision floating-
@@ -2490,42 +2471,6 @@ static float64
 
 }
 
-/*
-| Returns the fraction bits of the extended double-precision floating-point
-| value `a'.
-**/
-
-static inline uint64_t extractFloatx80Frac( floatx80 a )
-{
-
-return a.low;
-
-}
-
-/*
-| Returns the exponent bits of the extended double-precision floating-point
-| value `a'.
-**/
-
-static inline int32_t extractFloatx80Exp( floatx80 a )
-{
-
-return a.high & 0x7FFF;
-
-}
-
-/*
-| Returns the sign bit of the extended double-precision floating-point value
-| `a'.
-**/
-
-static inline flag extractFloatx80Sign( floatx80 a )
-{
-
-return a.high>>15;
-
-}
-
 /*
 | Normalizes the subnormal extended double-precision floating-point value
 | represented by the denormalized significand `aSig'.  The normalized exponent
@@ -2533,30 +2478,14 @@ static inline flag extractFloatx80Sign( floatx80 a )
 | `zSigPtr', respectively.
 **/
 
-static void
- normalizeFloatx80Subnormal( uint64_t aSig, int32_t *zExpPtr, uint64_t 
*zSigPtr )
+void normalizeFloatx80Subnormal(uint64_

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Collin L. Walling]

On 02/23/2018 09:57 AM, Collin L. Walling wrote:

On 02/23/2018 05:11 AM, Christian Borntraeger wrote:


On 02/23/2018 11:07 AM, Thomas Huth wrote:

On 22.02.2018 20:40, Collin L. Walling wrote:

On 02/22/2018 11:45 AM, Collin L. Walling wrote:

On 02/22/2018 10:44 AM, Christian Borntraeger wrote:

On 02/22/2018 04:40 PM, Collin L. Walling wrote:

On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:

On 22.02.2018 12:51, Christian Borntraeger wrote:

Series
Acked-by: Christian Borntraeger 

Thanks!!!


menu on scsi and dasd bootmaps tested successfully.

There is one thing that we might want to fix (can be an addon
patch since this is a non-customer
scenario (no libvirt)).

If you start QEMU manually without a bootindex, the -boot menu=on
is ignored
if no drive has a bootindex.

For example:

-drive file=/dev/dasda,if=none,id=d1 -device
virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
does work

-drive file=/dev/dasda -boot menu=on
does not

instead it prints:
qemu-system-s390x: boot menu is not supported for this device 
type.


and the boots up the default entry.


That should indeed be a separate patch, as it would move logic
currently
in the BIOS up to QEMU (find the first defined virtio disk and
select it
as boot disk).
In fact it's more complicated than that, because it would have to
properly account for -boot order=[acdn] and produce the respective
IPLB.
While it makes sense, I wouldn't rush that in but rather change 
the
error message to indicate that -device bootindex is needed to 
activate

the menu, at least for the time being.
[...]


I can look into it.  Theoretically, the easier fix should just
involve parsing all
of the -device commands and looking for a "bootindex=1" field. The
Qemu options
code already handles a bulk of this work, so it's just a matter of
putting it all
together.

Shall I whip something up and post what I have as a reply to this
email chain?

In fact, it should already be there.

static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
  DeviceState *dev_st;

  dev_st = get_boot_device(0);

--> if this returns 0 we have no bootindex statement anywhere and 
the

BIOS will IPL the default
disk.



Makes sense.  I'm working on making this patch look as clean as
possible. The fact that no boot menu
options present means we fallback to using zipl values for CCW being
tied into the switch statement
is making things a bit tricky. Just have to think the logic through a
bit.  Will get back to you once
I have something good.

This should do the trick (this can also be squished painlessly into 
6/13

if desired)

Patch looks fine to me. I can either take it directly like this, or in
case you have to respin (depends on the problem that Christian reported
with the Ubuntu guest), I'm also fine if you squash it into an earlier
patch instead.
FWIW, my problem (a menu happens even without -boot menu=on or 
loadparm) also

happens with other guests (e.g. fedora).


Is this on guests using DASD?  If so, a boot menu will appear if the 
appropriate zipl configuration
values are set (even if no -boot menu or loadparm).  Patch 12/13 goes 
into more detail.


Is this not wanted behavior?


Just noticed other email chain -- ignore me.

--
- Collin L Walling