qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 3be5cc: target-ppc: Document TOCTTOU in hugep


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 3be5cc: target-ppc: Document TOCTTOU in hugepage support
Date: Wed, 23 Mar 2016 07:30:06 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 3be5cc23245637577210e43e68dd3babd01fbb5e
      
https://github.com/qemu/qemu/commit/3be5cc23245637577210e43e68dd3babd01fbb5e
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-18 (Fri, 18 Mar 2016)

  Changed paths:
    M target-ppc/kvm.c

  Log Message:
  -----------
  target-ppc: Document TOCTTOU in hugepage support

The code to find the minimum page size is is vulnerable to TOCTTOU.
Added in commit 2d103aa "target-ppc: fix hugepage support when using
memory-backend-file" (v2.4.0).  Since I can't fix it myself right now,
add a FIXME comment.

Cc: Paolo Bonzini <address@hidden>
Cc: Michael Roth <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>


  Commit: e3ad72965a86b19df5d011117e186c62893085ef
      
https://github.com/qemu/qemu/commit/e3ad72965a86b19df5d011117e186c62893085ef
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-18 (Fri, 18 Mar 2016)

  Changed paths:
    M contrib/ivshmem-server/main.c

  Log Message:
  -----------
  ivshmem-server: Fix and clean up command line help

Burying error messages in ~20 lines of usage help is bad form.  Print
a single line pointing to -h instead.

Print -h help to stdout rather than stderr.  Fix default of -p.  Clean
up the help text a bit.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 3625c739eaf304fdf8ab3552ea6140cce07962a2
      
https://github.com/qemu/qemu/commit/3625c739eaf304fdf8ab3552ea6140cce07962a2
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-18 (Fri, 18 Mar 2016)

  Changed paths:
    M contrib/ivshmem-server/ivshmem-server.c
    M contrib/ivshmem-server/ivshmem-server.h
    M contrib/ivshmem-server/main.c
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem-server: Don't overload POSIX shmem and file name

Option -m NAME is interpreted as directory name if we can statfs() it
and its on hugetlbfs.  Else it's interpreted as POSIX shared memory
object name.  This is nuts.

Always interpret -m as directory.  Create new -M for POSIX shared
memory.  Last of -m or -M wins.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>


  Commit: ad4929384b64ffc9908915fd40cb80f455835d4a
      
https://github.com/qemu/qemu/commit/ad4929384b64ffc9908915fd40cb80f455835d4a
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-18 (Fri, 18 Mar 2016)

  Changed paths:
    M qemu-doc.texi

  Log Message:
  -----------
  qemu-doc: Fix ivshmem huge page example

Option parameter "share" is missing.  Without it, you get a *private*
mmap(), which defeats ivshmem's purpose pretty thoroughly ;)

While there, switch to the conventional mountpoint of hugetlbfs
/dev/hugepages.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: 330b58368ca16c31efdadcf8263f7f903546af50
      
https://github.com/qemu/qemu/commit/330b58368ca16c31efdadcf8263f7f903546af50
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M default-configs/pci.mak
    M tests/Makefile
    M util/event_notifier-posix.c

  Log Message:
  -----------
  event_notifier: Make event_notifier_init_fd() #ifdef CONFIG_EVENTFD

Event notifiers are designed for eventfd(2).  They can fall back to
pipes, but according to Paolo, event_notifier_init_fd() really
requires the real thing, and should therefore be under #ifdef
CONFIG_EVENTFD.  Do that.

Its only user is ivshmem, which is currently CONFIG_POSIX.  Narrow it
to CONFIG_EVENTFD.

Cc: Paolo Bonzini <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: 998261726a6aff3c8a88ae6965e51a6717b467ff
      
https://github.com/qemu/qemu/commit/998261726a6aff3c8a88ae6965e51a6717b467ff
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M tests/ivshmem-test.c
    M tests/libqos/pci-pc.c

  Log Message:
  -----------
  tests/libqos/pci-pc: Fix qpci_pc_iomap() to map BARs aligned

qpci_pc_iomap() maps BARs one after the other, without padding.  This
is wrong.  PCI Local Bus Specification Revision 3.0, 6.2.5.1. Address
Maps: "all address spaces used are a power of two in size and are
naturally aligned".  That's because the size of a BAR is given by the
number of address bits the device decodes, and the BAR needs to be
mapped at a multiple of that size to ensure the address decoding
works.

Fix qpci_pc_iomap() accordingly.  This takes care of a FIXME in
ivshmem-test.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 4958fe5d3c77a40a5250bc820b582a29d73605ab
      
https://github.com/qemu/qemu/commit/4958fe5d3c77a40a5250bc820b582a29d73605ab
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem-test: Improve test case /ivshmem/single

Test state of registers after reset.

Test reading Interrupt Status clears it.

Test (invalid) read of Doorbell.

Add more comments.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 14c5d49ab39fe767c39b8adce9bee5494063d56a
      
https://github.com/qemu/qemu/commit/14c5d49ab39fe767c39b8adce9bee5494063d56a
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem-test: Clean up wait for devices to become operational

test_ivshmem_server() waits until the first byte in BAR 2 contains the
0x42 we put into shared memory.  Works because the byte reads zero
until the device maps the shared memory gotten from the server.

Check the IVPosition register instead: it's initially -1, and becomes
non-negative right when the device maps the share memory, so no
change, just cleaner, because it's what guest software is supposed to
do.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 41b65e5eda4364fa966cb7bbf693a1d0cb4e8e1e
      
https://github.com/qemu/qemu/commit/41b65e5eda4364fa966cb7bbf693a1d0cb4e8e1e
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem-test: Improve test cases /ivshmem/server-*

Document missing test: behavior with MSI-X present but not enabled.

For MSI-X, we test and clear the interrupt pending bit before testing
the interrupt.  For INTx, we only clear.  Change to test and clear for
consistency.

Test MSI-X vector 1 in addition to vector 0.

Improve comments.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: fdee2025dd690b35099dd4d04eeef27c2bc1bc9c
      
https://github.com/qemu/qemu/commit/fdee2025dd690b35099dd4d04eeef27c2bc1bc9c
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    A docs/specs/ivshmem-spec.txt
    R docs/specs/ivshmem_device_spec.txt

  Log Message:
  -----------
  ivshmem: Rewrite specification document

This started as an attempt to update ivshmem_device_spec.txt for
clarity, accuracy and completeness while working on its code, and
quickly became a full rewrite.  Since the diff would be useless
anyway, I'm using the opportunity to rename the file to
ivshmem-spec.txt.

I tried hard to ensure the new text contradicts neither the old text
nor the code.  If the new text contradicts the old text but not the
code, it's probably a bug in the old text.  If the new text
contradicts both, its probably a bug in the new text.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 97553976dde2181be3ff95863905c4e34776ae70
      
https://github.com/qemu/qemu/commit/97553976dde2181be3ff95863905c4e34776ae70
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Add missing newlines to debug printfs

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: a4fa93bf20a4be3377df6ac9c9d63cccc31ab68c
      
https://github.com/qemu/qemu/commit/a4fa93bf20a4be3377df6ac9c9d63cccc31ab68c
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Compile debug prints unconditionally to prevent bit-rot

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: e64befe929cc4c4a53a79ff469b1b52c210a3468
      
https://github.com/qemu/qemu/commit/e64befe929cc4c4a53a79ff469b1b52c210a3468
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Clean up after commit 9940c32

IVShmemState member eventfd_chr is useless since commit 9940c32.  Drop
it.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: c20fc0c3ee1ca83e6f3416acad31439bffed7977
      
https://github.com/qemu/qemu/commit/c20fc0c3ee1ca83e6f3416acad31439bffed7977
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Drop ivshmem_event() stub

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 71c265816dd2772f89ebb377381c836dfca09d70
      
https://github.com/qemu/qemu/commit/71c265816dd2772f89ebb377381c836dfca09d70
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M docs/specs/ivshmem-spec.txt
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Don't destroy the chardev on version mismatch

Yes, the chardev is commonly useless after we read a bad version from
it, but destroying it is inappropriate anyway: the user created it, so
the user should be able to hold on to it as long as he likes.  We
don't destroy it on other errors.  Screwed up in commit 5105b1d.

Stop reading instead.

Also note QEMU's behavior in ivshmem-spec.txt.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 9cf70c52253ccadc137d40eb0de2c0f25a127334
      
https://github.com/qemu/qemu/commit/9cf70c52253ccadc137d40eb0de2c0f25a127334
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Fix harmless misuse of Error

We reuse errp after passing it host_memory_backend_get_memory().  If
both host_memory_backend_get_memory() and the reuse set an error, the
reuse will fail the assertion in error_setv().  Fortunately,
host_memory_backend_get_memory() can't fail.

Pass it &error_abort to make our assumption explicit, and to get the
assertion failure in the right place should it become invalid.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: d855e2756554583b2b041c9e0a8381bdd202cbc3
      
https://github.com/qemu/qemu/commit/d855e2756554583b2b041c9e0a8381bdd202cbc3
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Failed realize() can leave migration blocker behind

If pci_ivshmem_realize() fails after it created its migration blocker,
the blocker is left in place.  Fix that by creating it last.

Likewise, if it fails after it called fifo8_create(), it leaks fifo
memory.  Fix that the same way.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 434ad76db531b4bcad241aa4b2abd4bccacca89a
      
https://github.com/qemu/qemu/commit/434ad76db531b4bcad241aa4b2abd4bccacca89a
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Clean up register callbacks

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 082751e82bed9482fc75863e0b305949bb230c6a
      
https://github.com/qemu/qemu/commit/082751e82bed9482fc75863e0b305949bb230c6a
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Clean up MSI-X conditions

There are three predicates related to MSI-X:

* ivshmem_has_feature(s, IVSHMEM_MSI) is true unless the non-MSI-X
  variant of the device is selected with msi=off.

* msix_present() is true when the device has the PCI capability MSI-X.
  It's initially false, and becomes true during successful realize of
  the MSI-X variant of the device.  Thus, it's the same as
  ivshmem_has_feature(s, IVSHMEM_MSI) for realized devices.

* msix_enabled() is true when msix_present() is true and guest software
  has enabled MSI-X.

Code that differs between the non-MSI-X and the MSI-X variant of the
device needs to be guarded by ivshmem_has_feature(s, IVSHMEM_MSI) or
by msix_present(), except the latter works only for realized devices.

Code that depends on whether MSI-X is in use needs to be guarded with
msix_enabled().

Code review led me to two minor messes:

* ivshmem_vector_notify() calls msix_notify() even when
  !msix_enabled(), unlike most other MSI-X-capable devices.  As far as
  I can tell, msix_notify() does nothing when !msix_enabled().  Add
  the guard anyway.

* Most callers of ivshmem_use_msix() guard it with
  ivshmem_has_feature(s, IVSHMEM_MSI).  Not necessary, because
  ivshmem_use_msix() does nothing when !msix_present().  That's
  ivshmem's only use of msix_present(), though.  Guard it
  consistently, and drop the now redundant msix_present() check.
  While there, rename ivshmem_use_msix() to ivshmem_msix_vector_use().

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>


  Commit: 2d1d422d1145085fdde6a90d70cbbff7103d469b
      
https://github.com/qemu/qemu/commit/2d1d422d1145085fdde6a90d70cbbff7103d469b
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Leave INTx alone when using MSI-X

The ivshmem device can either use MSI-X or legacy INTx for interrupts.

With MSI-X enabled, peer interrupt events trigger an MSI as they
should.  But software can still raise INTx via interrupt status and
mask register in BAR 0.  This is explicitly prohibited by PCI Local
Bus Specification Revision 3.0, section 6.8.3.3:

    While enabled for MSI or MSI-X operation, a function is prohibited
    from using its INTx# pin (if implemented) to request service (MSI,
    MSI-X, and INTx# are mutually exclusive).

Fix the device model to leave INTx alone when using MSI-X.

Document that we claim to use INTx in config space even when we don't.
Unlike other devices, ivshmem does *not* use INTx when configured for
MSI-X and MSI-X isn't enabled by software.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: 3c27969b3e7397bb63f363fc8b8bc0e601542d76
      
https://github.com/qemu/qemu/commit/3c27969b3e7397bb63f363fc8b8bc0e601542d76
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Assert interrupts are set up once

An interrupt is set up when the interrupt's file descriptor is
received.  Each message applies to the next interrupt vector.
Therefore, each vector cannot be set up more than once.

ivshmem_add_kvm_msi_virq() half-heartedly tries not to rely on this by
doing nothing then, but that's not going to recover from this error
should it become possible in the future.  watch_vector_notifier()
doesn't even try.

Simply assert what is the case, so we get alerted if we ever screw it
up.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: cd9953f720e1d57cfa86bc0882abced45ba96d3c
      
https://github.com/qemu/qemu/commit/cd9953f720e1d57cfa86bc0882abced45ba96d3c
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Simplify rejection of invalid peer ID from server

ivshmem_read() processes server messages.  These are 64 bit signed
integers.  -1 is shared memory setup, 16 bit unsigned is a peer ID,
anything else is invalid.

ivshmem_read() rejects invalid negative messages right away, silently.

Invalid positive messages get rejected only in resize_peers(), and
ivshmem_read() then prints the rather cryptic message "failed to
resize peers array".

Extend the first check to cover all invalid messages, make it report
"server sent invalid message", and drop the second check.

Now resize_peers() can't fail anymore; simplify.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: ca0b7566cc2cbd245e804fe03d556b0dbee1fd2e
      
https://github.com/qemu/qemu/commit/ca0b7566cc2cbd245e804fe03d556b0dbee1fd2e
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Disentangle ivshmem_read()

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 9db51b4d64ded01536b3851a5a50e484ac2f7899
      
https://github.com/qemu/qemu/commit/9db51b4d64ded01536b3851a5a50e484ac2f7899
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Plug leaks on unplug, fix peer disconnect

close_peer_eventfds() cleans up three things: ioeventfd triggers if
they exist, eventfds, and the array to store them.

Commit 98609cd (v1.2.0) fixed it not to clean up ioeventfd triggers
when they don't exist (property ioeventfd=off, which is the default).
Unfortunately, the fix also made it skip cleanup of the eventfds and
the array then.  This is a memory and file descriptor leak on unplug.

Additionally, the reset of nb_eventfds is skipped.  Doesn't matter on
unplug.  On peer disconnect, however, this permanently wedges the
interrupt vectors used for that peer's ID.  The eventfds stay behind,
but aren't connected to a peer anymore.  When the ID gets recycled for
a new peer, the new peer's eventfds get assigned to vectors after the
old ones.  Commonly, the device's number of vectors matches the
server's, so the new ones get dropped with a "Too many eventfd
received" message.  Interrupts either don't work (common case) or go
to the wrong vector.

Fix by narrowing the conditional to just the ioeventfd trigger
cleanup.

While there, move the "invalid" peer check to the only caller where it
can actually happen, and tighten it to reject own ID.

Cc: Paolo Bonzini <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 3a55fc0f243104998bee5106b121cff257df5d33
      
https://github.com/qemu/qemu/commit/3a55fc0f243104998bee5106b121cff257df5d33
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem: Receive shared memory synchronously in realize()

When configured for interrupts (property "chardev" given), we receive
the shared memory from an ivshmem server.  We do so asynchronously
after realize() completes, by setting up callbacks with
qemu_chr_add_handlers().

Keeping server I/O out of realize() that way avoids delays due to a
slow server.  This is probably relevant only for hot plug.

However, this funny "no shared memory, yet" state of the device also
causes a raft of issues that are hard or impossible to work around:

* The guest is exposed to this state: when we enter and leave it its
  shared memory contents is apruptly replaced, and device register
  IVPosition changes.

  This is a known issue.  We document that guests should not access
  the shared memory after device initialization until the IVPosition
  register becomes non-negative.

  For cold plug, the funny state is unlikely to be visible in
  practice, because we normally receive the shared memory long before
  the guest gets around to mess with the device.

  For hot plug, the timing is tighter, but the relative slowness of
  PCI device configuration has a good chance to hide the funny state.

  In either case, guests complying with the documented procedure are
  safe.

* Migration becomes racy.

  If migration completes before the shared memory setup completes on
  the source, shared memory contents is silently lost.  Fortunately,
  migration is rather unlikely to win this race.

  If the shared memory's ramblock arrives at the destination before
  shared memory setup completes, migration fails.

  There is no known way for a management application to wait for
  shared memory setup to complete.

  All you can do is retry failed migration.  You can improve your
  chances by leaving more time between running the destination QEMU
  and the migrate command.

  To mitigate silent memory loss, you need to ensure the server
  initializes shared memory exactly the same on source and
  destination.

  These issues are entirely undocumented so far.

I'd expect the server to be almost always fast enough to hide these
issues.  But then rare catastrophic races are in a way the worst kind.

This is way more trouble than I'm willing to take from any device.
Kill the funny state by receiving shared memory synchronously in
realize().  If your hot plug hangs, go kill your ivshmem server.

For easier review, this commit only makes the receive synchronous, it
doesn't add the necessary error propagation.  Without that, the funny
state persists.  The next commit will do that, and kill it off for
real.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 1309cf448a6d88d8a693c15d5b11ad07af2321ab
      
https://github.com/qemu/qemu/commit/1309cf448a6d88d8a693c15d5b11ad07af2321ab
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M docs/specs/ivshmem-spec.txt
    M hw/misc/ivshmem.c
    M qemu-doc.texi

  Log Message:
  -----------
  ivshmem: Propagate errors through ivshmem_recv_setup()

This kills off the funny state described in the previous commit.

Simplify ivshmem_io_read() accordingly, and update documentation.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>


  Commit: a3feb08639e7982f47c3981fea79d527d3dfc0ac
      
https://github.com/qemu/qemu/commit/a3feb08639e7982f47c3981fea79d527d3dfc0ac
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Rely on server sending the ID right after the version

The protocol specification (ivshmem-spec.txt, formerly
ivshmem_device_spec.txt) has always required the ID message to be sent
right at the beginning, and ivshmem-server has always complied.  The
device, however, accepts it out of order.  If an interrupt setup
arrived before it, though, it would be misinterpreted as connect
notification.  Fix the latent bug by relying on the spec and
ivshmem-server's actual behavior.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: ba5970a178ef927c34cb8a6dfff54de0a58497c6
      
https://github.com/qemu/qemu/commit/ba5970a178ef927c34cb8a6dfff54de0a58497c6
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Drop the hackish test for UNIX domain chardev

The chardev must be capable of transmitting SCM_RIGHTS ancillary
messages.  We check it by comparing CharDriverState member filename to
"unix:".  That's almost as brittle as it is disgusting.

When the actual transmission all happened asynchronously, this check
was all we could do in realize(), and thus better than nothing.  But
now we receive at least one SCM_RIGHTS synchronously in realize(),
it's not worth its keep anymore.  Drop it.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: ee276391a38c784fd1a3ce33eab0481348d518d1
      
https://github.com/qemu/qemu/commit/ee276391a38c784fd1a3ce33eab0481348d518d1
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Simplify how we cope with short reads from server

Short reads from a UNIX domain sockets are exceedingly unlikely when
the other side always sends eight bytes and we always read eight
bytes.  We cope with them anyway.  However, the code doing that is
rather convoluted.  Dumb it down radically.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 08183c20b8b0782e4c30156eb7202d1117ca22f5
      
https://github.com/qemu/qemu/commit/08183c20b8b0782e4c30156eb7202d1117ca22f5
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Tighten check of property "size"

If size_t is narrower than 64 bits, passing uint64_t ivshmem_size to
mmap() truncates.  Reject such sizes.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 5503e285041979dd29698ecb41729b3b22622e8d
      
https://github.com/qemu/qemu/commit/5503e285041979dd29698ecb41729b3b22622e8d
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Implement shm=... with a memory backend

ivshmem has its very own code to create and map shared memory.
Replace that with an implicitly created memory backend.  Reduces the
number of ways we create BAR 2 from three to two.

The memory-backend-file is currently available only with CONFIG_LINUX,
so this adds a second Linuxism to ivshmem (the other one is eventfd).
Should we ever need to make it portable to systems where
memory-backend-file can't be made to serve, we could create a
memory-backend-shmem that allocates memory with shm_open().

Bonus fix: shared memory files are now created with permissions 0655
instead of 0777.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: c2d8019cd7e9c38c5ef9dc44ae1e3adccd57a6a4
      
https://github.com/qemu/qemu/commit/c2d8019cd7e9c38c5ef9dc44ae1e3adccd57a6a4
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Simplify memory regions for BAR 2 (shared memory)

ivshmem_realize() puts the shared memory region in a container region.
Used to be necessary to permit delayed mapping of the shared memory.
However, we recently moved to synchronous mapping, in "ivshmem:
Receive shared memory synchronously in realize()" and the commit
following it.  The container is redundant since then.  Drop it.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: 8baeb22bfc3b57a8568c17b34c66ea2ff54df09a
      
https://github.com/qemu/qemu/commit/8baeb22bfc3b57a8568c17b34c66ea2ff54df09a
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Inline check_shm_size() into its only caller

Improve the error messages while there.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>


  Commit: 55e8a154359be12ca4c9730c562d1e3d4b1bd2a1
      
https://github.com/qemu/qemu/commit/55e8a154359be12ca4c9730c562d1e3d4b1bd2a1
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/core/qdev-properties.c
    M include/hw/qdev-properties.h

  Log Message:
  -----------
  qdev: New DEFINE_PROP_ON_OFF_AUTO

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 2a845da73653bf98a3187bceb40364d9565f70c7
      
https://github.com/qemu/qemu/commit/2a845da73653bf98a3187bceb40364d9565f70c7
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Replace int role_val by OnOffAuto master

In preparation of making it a qdev property.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 5400c02b90bb647a961f3210255178b68602bd5b
      
https://github.com/qemu/qemu/commit/5400c02b90bb647a961f3210255178b68602bd5b
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M docs/specs/ivshmem-spec.txt
    M hw/misc/ivshmem.c
    M qemu-doc.texi
    M tests/ivshmem-test.c

  Log Message:
  -----------
  ivshmem: Split ivshmem-plain, ivshmem-doorbell off ivshmem

ivshmem can be configured with and without interrupt capability
(a.k.a. "doorbell").  The two configurations have largely disjoint
options, which makes for a confusing (and badly checked) user
interface.  Moreover, the device can't tell the guest whether its
doorbell is enabled.

Create two new device models ivshmem-plain and ivshmem-doorbell, and
deprecate the old one.

Changes from ivshmem:

* PCI revision is 1 instead of 0.  The new revision is fully backwards
  compatible for guests.  Guests may elect to require at least
  revision 1 to make sure they're not exposed to the funny "no shared
  memory, yet" state.

* Property "role" replaced by "master".  role=master becomes
  master=on, role=peer becomes master=off.  Default is off instead of
  auto.

* Property "use64" is gone.  The new devices always have 64 bit BARs.

Changes from ivshmem to ivshmem-plain:

* The Interrupt Pin register in PCI config space is zero (does not use
  an interrupt pin) instead of one (uses INTA).

* Property "x-memdev" is renamed to "memdev".

* Properties "shm" and "size" are gone.  Use property "memdev"
  instead.

* Property "msi" is gone.  The new device can't have MSI-X capability.
  It can't interrupt anyway.

* Properties "ioeventfd" and "vectors" are gone.  They're meaningless
  without interrupts anyway.

Changes from ivshmem to ivshmem-doorbell:

* Property "msi" is gone.  The new device always has MSI-X capability.

* Property "ioeventfd" defaults to on instead of off.

* Property "size" is gone.  The new device can only map all the shared
  memory received from the server.

Guests can easily find out whether the device is configured for
interrupts by checking for MSI-X capability.

Note: some code added in sub-optimal places to make the diff easier to
review.  The next commit will move it to more sensible places.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: ddc852844388bb43a39319f0be8a4c4af3a4c526
      
https://github.com/qemu/qemu/commit/ddc852844388bb43a39319f0be8a4c4af3a4c526
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Clean up after the previous commit

Move code to more sensible places.  Use the opportunity to reorder and
document IVShmemState members.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 13fd2cb68953ac066116d444415f9d5ec7ce1200
      
https://github.com/qemu/qemu/commit/13fd2cb68953ac066116d444415f9d5ec7ce1200
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Drop ivshmem property x-memdev

Use ivshmem-plain instead.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 62a830b688a93419baa89061b6b5faf8b5e10808
      
https://github.com/qemu/qemu/commit/62a830b688a93419baa89061b6b5faf8b5e10808
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M docs/specs/ivshmem-spec.txt
    M hw/misc/ivshmem.c
    M qemu-doc.texi

  Log Message:
  -----------
  ivshmem: Require master to have ID zero

Migration with ivshmem needs to be carefully orchestrated to work.
Exactly one peer (the "master") migrates to the destination, all other
peers need to unplug (and disconnect), migrate, plug back (and
reconnect).  This is sort of documented in qemu-doc.

If peers connect on the destination before migration completes, the
shared memory can get messed up.  This isn't documented anywhere.  Fix
that in qemu-doc.

To avoid messing up register IVPosition on migration, the server must
assign the same ID on source and destination.  ivshmem-spec.txt leaves
ID assignment unspecified, however.

Amend ivshmem-spec.txt to require the first client to receive ID zero.
The example ivshmem-server complies: it always assigns the first
unused ID.

For a bit of additional safety, enforce ID zero for the master.  This
does nothing when we're not using a server, because the ID is zero for
all peers then.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: a335c6f204eefba8ff935bcee8f31f51d2174119
      
https://github.com/qemu/qemu/commit/a335c6f204eefba8ff935bcee8f31f51d2174119
  Author: Markus Armbruster <address@hidden>
  Date:   2016-03-21 (Mon, 21 Mar 2016)

  Changed paths:
    M contrib/ivshmem-server/main.c

  Log Message:
  -----------
  contrib/ivshmem-server: Print "not for production" warning

The code is okay for illustrating how things work and for testing, but
its error handling make it unfit for production use.  Print a warning
to protect the innocent.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Message-Id: <address@hidden>


  Commit: 2538039f2c26d66053426fb547e4f25e669baf62
      
https://github.com/qemu/qemu/commit/2538039f2c26d66053426fb547e4f25e669baf62
  Author: Peter Maydell <address@hidden>
  Date:   2016-03-23 (Wed, 23 Mar 2016)

  Changed paths:
    M contrib/ivshmem-server/ivshmem-server.c
    M contrib/ivshmem-server/ivshmem-server.h
    M contrib/ivshmem-server/main.c
    M default-configs/pci.mak
    A docs/specs/ivshmem-spec.txt
    R docs/specs/ivshmem_device_spec.txt
    M hw/core/qdev-properties.c
    M hw/misc/ivshmem.c
    M include/hw/qdev-properties.h
    M qemu-doc.texi
    M target-ppc/kvm.c
    M tests/Makefile
    M tests/ivshmem-test.c
    M tests/libqos/pci-pc.c
    M util/event_notifier-posix.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-ivshmem-2016-03-18' 
into staging

ivshmem: Fixes, cleanups, device model split

# gpg: Signature made Mon 21 Mar 2016 20:33:54 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-ivshmem-2016-03-18: (40 commits)
  contrib/ivshmem-server: Print "not for production" warning
  ivshmem: Require master to have ID zero
  ivshmem: Drop ivshmem property x-memdev
  ivshmem: Clean up after the previous commit
  ivshmem: Split ivshmem-plain, ivshmem-doorbell off ivshmem
  ivshmem: Replace int role_val by OnOffAuto master
  qdev: New DEFINE_PROP_ON_OFF_AUTO
  ivshmem: Inline check_shm_size() into its only caller
  ivshmem: Simplify memory regions for BAR 2 (shared memory)
  ivshmem: Implement shm=... with a memory backend
  ivshmem: Tighten check of property "size"
  ivshmem: Simplify how we cope with short reads from server
  ivshmem: Drop the hackish test for UNIX domain chardev
  ivshmem: Rely on server sending the ID right after the version
  ivshmem: Propagate errors through ivshmem_recv_setup()
  ivshmem: Receive shared memory synchronously in realize()
  ivshmem: Plug leaks on unplug, fix peer disconnect
  ivshmem: Disentangle ivshmem_read()
  ivshmem: Simplify rejection of invalid peer ID from server
  ivshmem: Assert interrupts are set up once
  ...

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/ffa6564c9b13...2538039f2c26

reply via email to

[Prev in Thread] Current Thread [Next in Thread]