03 Dec, 2011

1 commit


24 Nov, 2011

3 commits

  • virtio pci device reset actually just does an I/O
    write, which in PCI is really posted, that is it
    can complete on CPU before the device has received it.

    Further, interrupts might have been pending on
    another CPU, so device callback might get invoked after reset.

    This conflicts with how drivers use reset, which is typically:
    reset
    unregister
    a callback running after reset completed can race with
    unregister, potentially leading to use after free bugs.

    Fix by flushing out the write, and flushing pending interrupts.

    This assumes that device is never reset from
    its vq/config callbacks, or in parallel with being
    added/removed, document this assumption.

    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Michael S. Tsirkin
     
  • Guest features selector spelling mistake.

    Cc: Pawel Moll
    Cc: Rusty Russell
    Cc: virtualization@lists.linux-foundation.org
    Signed-off-by: Sasha Levin
    Signed-off-by: Rusty Russell

    Sasha Levin
     
  • Fix this compile error on s390:

    CC [M] drivers/virtio/virtio_mmio.o
    drivers/virtio/virtio_mmio.c: In function 'vm_get_features':
    drivers/virtio/virtio_mmio.c:107:2: error: implicit declaration of function 'writel'

    Cc: Christian Borntraeger
    Signed-off-by: Heiko Carstens
    Acked-by: Pawel Moll
    Signed-off-by: Rusty Russell

    Heiko Carstens
     

22 Nov, 2011

1 commit


17 Nov, 2011

1 commit


14 Nov, 2011

1 commit

  • Commit 31a3ddda166cda86d2b5111e09ba4bda5239fae6 introduced
    a use after free in virtio-pci. The main issue is
    that the release method signals removal of the virtio device,
    while remove signals removal of the pci device.

    For example, on driver removal or hot-unplug,
    virtio_pci_release_dev is called before virtio_pci_remove.
    We then might get a crash as virtio_pci_remove tries to use the
    device freed by virtio_pci_release_dev.

    We allocate/free all resources together with the
    pci device, so we can leave the release method empty.

    Signed-off-by: Michael S. Tsirkin
    Acked-by: Amit Shah
    Signed-off-by: Rusty Russell
    Cc: stable@kernel.org

    Michael S. Tsirkin
     

07 Nov, 2011

1 commit

  • * 'modsplit-Oct31_2011' of git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux: (230 commits)
    Revert "tracing: Include module.h in define_trace.h"
    irq: don't put module.h into irq.h for tracking irqgen modules.
    bluetooth: macroize two small inlines to avoid module.h
    ip_vs.h: fix implicit use of module_get/module_put from module.h
    nf_conntrack.h: fix up fallout from implicit moduleparam.h presence
    include: replace linux/module.h with "struct module" wherever possible
    include: convert various register fcns to macros to avoid include chaining
    crypto.h: remove unused crypto_tfm_alg_modname() inline
    uwb.h: fix implicit use of asm/page.h for PAGE_SIZE
    pm_runtime.h: explicitly requires notifier.h
    linux/dmaengine.h: fix implicit use of bitmap.h and asm/page.h
    miscdevice.h: fix up implicit use of lists and types
    stop_machine.h: fix implicit use of smp.h for smp_processor_id
    of: fix implicit use of errno.h in include/linux/of.h
    of_platform.h: delete needless include
    acpi: remove module.h include from platform/aclinux.h
    miscdevice.h: delete unnecessary inclusion of module.h
    device_cgroup.h: delete needless include
    net: sch_generic remove redundant use of
    net: inet_timewait_sock doesnt need
    ...

    Fix up trivial conflicts (other header files, and removal of the ab3550 mfd driver) in
    - drivers/media/dvb/frontends/dibx000_common.c
    - drivers/media/video/{mt9m111.c,ov6650.c}
    - drivers/mfd/ab3550-core.c
    - include/linux/dmaengine.h

    Linus Torvalds
     

02 Nov, 2011

2 commits

  • This patch, based on virtio PCI driver, adds support for memory
    mapped (platform) virtio device. This should allow environments
    like qemu to use virtio-based block & network devices even on
    platforms without PCI support.

    One can define and register a platform device which resources
    will describe memory mapped control registers and "mailbox"
    interrupt. Such device can be also instantiated using the Device
    Tree node with compatible property equal "virtio,mmio".

    Cc: Anthony Liguori
    Cc: Michael S.Tsirkin
    Signed-off-by: Pawel Moll
    Signed-off-by: Rusty Russell

    Pawel Moll
     
  • For the MSI but non-per_vq_vector case, the config/change vq
    also gets added to the list of vqs that need to process the
    MSI interrupt. This is not needed as config has it's own
    handler (vp_config_changed). In any case, vring_interrupt()
    finds nothing needs to be done on this vq.

    I tested this patch by testing the "Fallback:" and "Finally
    fall back" cases in vp_find_vqs(). Please review.

    Signed-off-by: Krishna Kumar
    Acked-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Krishna Kumar
     

01 Nov, 2011

1 commit


24 Oct, 2011

1 commit


23 Jul, 2011

1 commit

  • virtio has been so far used only in the context of virtualization,
    and the virtio Kconfig was sourced directly by the relevant arch
    Kconfigs when VIRTUALIZATION was selected.

    Now that we start using virtio for inter-processor communications,
    we need to source the virtio Kconfig outside of the virtualization
    scope too.

    Moreover, some architectures might use virtio for both virtualization
    and inter-processor communications, so directly sourcing virtio
    might yield unexpected results due to conflicting selections.

    The simple solution offered by this patch is to always source virtio's
    Kconfig in drivers/Kconfig, and remove it from the appropriate arch
    Kconfigs. Additionally, a virtio menu entry has been added so virtio
    drivers don't show up in the general drivers menu.

    This way anyone can use virtio, though it's arguably less accessible
    (and neat!) for virtualization users now.

    Note: some architectures (mips and sh) seem to have a VIRTUALIZATION
    menu merely for sourcing virtio's Kconfig, so that menu is removed too.

    Signed-off-by: Ohad Ben-Cohen
    Signed-off-by: Rusty Russell

    Ohad Ben-Cohen
     

30 May, 2011

3 commits

  • Add an API that tells the other side that callbacks
    should be delayed until a lot of work has been done.
    Implement using the new event_idx feature.

    Note: it might seem advantageous to let the drivers
    ask for a callback after a specific capacity has
    been reached. However, as a single head can
    free many entries in the descriptor table,
    we don't really have a clue about capacity
    until get_buf is called. The API is the simplest
    to implement at the moment, we'll see what kind of
    hints drivers can pass when there's more than one
    user of the feature.

    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Michael S. Tsirkin
     
  • Support for the new event idx feature:
    1. When enabling interrupts, publish the current avail index
    value to the host to get interrupts on the next update.
    2. Use the new avail_event feature to reduce the number
    of exits from the guest.

    Simple test with the simulator:

    [virtio]# time ./virtio_test
    spurious wakeus: 0x7

    real 0m0.169s
    user 0m0.140s
    sys 0m0.019s
    [virtio]# time ./virtio_test --no-event-idx
    spurious wakeus: 0x11

    real 0m0.649s
    user 0m0.295s
    sys 0m0.335s

    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Michael S. Tsirkin
     
  • The virtio balloon driver has a VIRTIO_BALLOON_F_MUST_TELL_HOST
    feature bit. Whenever the bit is set, the guest kernel must
    always tell the host before we free pages back to the allocator.
    Without this feature, we might free a page (and have another
    user touch it) while the hypervisor is unprepared for it.

    But, if the bit is _not_ set, we are under no obligation to
    reverse the order; we're under no obligation to do _anything_.
    As of now, qemu-kvm defines the bit, but doesn't set it.

    This patch makes the "tell host first" logic the only case. This
    should make everybody happy, and reduce the amount of untested or
    untestable code in the kernel.

    This _also_ means that we don't have to preserve a pfn list
    after the pages are freed, which should let us get rid of some
    temporary storage (vb->pfns) eventually.

    Signed-off-by: Dave Hansen
    Signed-off-by: Rusty Russell

    Dave Hansen
     

21 Apr, 2011

2 commits

  • In the case where a virtio-console port is in use (opened by a program)
    and a virtio-console device is removed, the port is kept around but all
    the virtio-related state is assumed to be gone.

    When the port is finally released (close() called), we call
    device_destroy() on the port's device. This results in the parent
    device's structures to be freed as well. This includes the PCI regions
    for the virtio-console PCI device.

    Once this is done, however, virtio_pci_release_dev() kicks in, as the
    last ref to the virtio device is now gone, and attempts to do

    pci_iounmap(pci_dev, vp_dev->ioaddr);
    pci_release_regions(pci_dev);
    pci_disable_device(pci_dev);

    which results in a double-free warning.

    Move the code that releases regions, etc., to the virtio_pci_remove()
    function, and all that's now left in release_dev is the final freeing of
    the vp_dev.

    Signed-off-by: Amit Shah
    Signed-off-by: Rusty Russell

    Amit Shah
     
  • When detaching a buffer from a vq, the avail.idx value should be
    decremented as well.

    This was noticed by hot-unplugging a virtio console port and then
    plugging in a new one on the same number (re-using the vqs which were
    just 'disowned'). qemu reported

    'Guest moved used index from 0 to 256'

    when any IO was attempted on the new port.

    CC: stable@kernel.org
    Reported-by: juzhang
    Signed-off-by: Amit Shah
    Signed-off-by: Rusty Russell

    Amit Shah
     

20 Jan, 2011

1 commit

  • We sometimes need to map between the virtio device and
    the given pci device. One such use is OS installer that
    gets the boot pci device from BIOS and needs to
    find the relevant block device. Since it can't,
    installation fails.

    Instead of creating a top-level devices/virtio-pci
    directory, create each device under the corresponding
    pci device node. Symlinks to all virtio-pci
    devices can be found under the pci driver link in
    bus/pci/drivers/virtio-pci/devices, and all virtio
    devices under drivers/bus/virtio/devices.

    Signed-off-by: Milton Miller
    Signed-off-by: Rusty Russell
    Acked-by: Michael S. Tsirkin
    Tested-by: Michael S. Tsirkin
    Acked-by: Gleb Natapov
    Tested-by: "Daniel P. Berrange"
    Cc: stable@kernel.org

    Milton Miller
     

24 Nov, 2010

2 commits

  • The sysfs files for virtio produce the wrong format and are missing
    the required newline. The output for virtio bus vendor/device should
    have the same format as the corresponding entries for PCI devices.

    Although this technically changes the ABI for sysfs, these files were
    broken to start with!

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Rusty Russell

    Stephen Hemminger
     
  • We can't rely on indirect buffers for capacity
    calculations because they need a memory allocation
    which might fail. In particular, virtio_net can get
    into this situation under stress, and it drops packets
    and performs badly.

    So return the number of buffers we can guarantee users.

    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell
    Reported-By: Krishna Kumar2

    Michael S. Tsirkin
     

26 Jul, 2010

1 commit

  • virtio ring was changed to return an error code on OOM,
    but one caller was missed and still checks for vq->vring.num.
    The fix is just to check for
    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell
    Tested-by: Chris Mason
    Cc: stable@kernel.org # .34.x
    Signed-off-by: Linus Torvalds

    Michael S. Tsirkin
     

23 Jun, 2010

2 commits

  • virtio-pci resets the device at startup by writing to the status
    register, but this does not clear the pci config space,
    specifically msi enable status which affects register
    layout.

    This breaks things like kdump when they try to use e.g. virtio-blk.

    Fix by forcing msi off at startup. Since pci.c already has
    a routine to do this, we export and use it instead of duplicating code.

    Signed-off-by: Michael S. Tsirkin
    Tested-by: Vivek Goyal
    Acked-by: Jesse Barnes
    Cc: linux-pci@vger.kernel.org
    Signed-off-by: Rusty Russell
    Cc: stable@kernel.org

    Michael S. Tsirkin
     
  • add_buf returns ring size on out of memory,
    this is not what devices expect.

    Signed-off-by: Michael S. Tsirkin
    Acked-by: Amit Shah
    Signed-off-by: Rusty Russell
    Cc: stable@kernel.org # .34.x

    Michael S. Tsirkin
     

22 May, 2010

1 commit

  • * 'virtio' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-for-linus: (27 commits)
    drivers/char: Eliminate use after free
    virtio: console: Accept console size along with resize control message
    virtio: console: Store each console's size in the console structure
    virtio: console: Resize console port 0 on config intr only if multiport is off
    virtio: console: Add support for nonblocking write()s
    virtio: console: Rename wait_is_over() to will_read_block()
    virtio: console: Don't always create a port 0 if using multiport
    virtio: console: Use a control message to add ports
    virtio: console: Move code around for future patches
    virtio: console: Remove config work handler
    virtio: console: Don't call hvc_remove() on unplugging console ports
    virtio: console: Return -EPIPE to hvc_console if we lost the connection
    virtio: console: Let host know of port or device add failures
    virtio: console: Add a __send_control_msg() that can send messages without a valid port
    virtio: Revert "virtio: disable multiport console support."
    virtio: add_buf_gfp
    trans_virtio: use virtqueue_xxx wrappers
    virtio-rng: use virtqueue_xxx wrappers
    virtio_ring: remove a level of indirection
    virtio_net: use virtqueue_xxx wrappers
    ...

    Fix up conflicts in drivers/net/virtio_net.c due to new virtqueue_xxx
    wrappers changes conflicting with some other cleanups.

    Linus Torvalds
     

19 May, 2010

3 commits


23 Apr, 2010

1 commit


22 Apr, 2010

1 commit

  • The virtio balloon driver can dig into the reservation pools of the OS
    to satisfy a balloon request. This is not advisable and other balloon
    drivers (drivers/xen/balloon.c) avoid this as well.

    The patch also adds changes to avoid printing a warning if allocation
    fails, since we retry after sometime anyway.

    Signed-off-by: Balbir Singh
    Signed-off-by: Rusty Russell
    Cc: kvm
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Balbir Singh
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

16 Mar, 2010

1 commit


02 Mar, 2010

1 commit


01 Mar, 2010

1 commit

  • I have observed the following error on virtio-net module unload:

    ------------[ cut here ]------------
    WARNING: at kernel/irq/manage.c:858 __free_irq+0xa0/0x14c()
    Hardware name: Bochs
    Trying to free already-free IRQ 0
    Modules linked in: virtio_net(-) virtio_blk virtio_pci virtio_ring
    virtio af_packet e1000 shpchp aacraid uhci_hcd ohci_hcd ehci_hcd [last
    unloaded: scsi_wait_scan]
    Pid: 1957, comm: rmmod Not tainted 2.6.33-rc8-vhost #24
    Call Trace:
    [] warn_slowpath_common+0x7c/0x94
    [] warn_slowpath_fmt+0x41/0x43
    [] ? __free_pages+0x5a/0x70
    [] __free_irq+0xa0/0x14c
    [] free_irq+0x3f/0x65
    [] vp_del_vqs+0x81/0xb1 [virtio_pci]
    [] virtnet_remove+0xda/0x10b [virtio_net]
    [] virtio_dev_remove+0x22/0x4a [virtio]
    [] __device_release_driver+0x66/0xac
    [] driver_detach+0x83/0xa9
    [] bus_remove_driver+0x91/0xb4
    [] driver_unregister+0x6c/0x74
    [] unregister_virtio_driver+0xe/0x10 [virtio]
    [] fini+0x15/0x17 [virtio_net]
    [] sys_delete_module+0x1c3/0x230
    [] ? old_ich_force_enable_hpet+0x117/0x164
    [] ? do_page_fault+0x29c/0x2cc
    [] sysenter_dispatch+0x7/0x27
    ---[ end trace 15e88e4c576cc62b ]---

    The bug is in virtio-pci: we use msix_vector as array index to get irq
    entry, but some vqs do not have a dedicated vector so this causes an out
    of bounds access. By chance, we seem to often get 0 value, which
    results in this error.

    Fix by verifying that vector is legal before using it as index.

    Signed-off-by: Michael S. Tsirkin
    Acked-by: Anthony Liguori
    Acked-by: Shirley Ma
    Acked-by: Amit Shah

    Michael S. Tsirkin
     

24 Feb, 2010

6 commits

  • vq operations depend on vq->data[i] being NULL to figure out if the vq
    entry is in use (since the previous patch).

    We have to initialize them to NULL to ensure we don't work with junk
    data and trigger false BUG_ONs.

    Signed-off-by: Amit Shah
    Signed-off-by: Rusty Russell
    Cc: Shirley Ma

    Amit Shah
     
  • There's currently no way for a virtio driver to ask for unused
    buffers, so it has to keep a list itself to reclaim them at shutdown.
    This is redundant, since virtio_ring stores that information. So
    add a new hook to do this.

    Signed-off-by: Shirley Ma
    Signed-off-by: Amit Shah
    Signed-off-by: Rusty Russell

    Shirley Ma
     
  • virtio is communicating with a virtual "device" that actually runs on
    another host processor. Thus SMP barriers can be used to control
    memory access ordering.

    Where possible, we should use SMP barriers which are more lightweight than
    mandatory barriers, because mandatory barriers also control MMIO effects on
    accesses through relaxed memory I/O windows (which virtio does not use)
    (compare specifically smp_rmb and rmb on x86_64).

    We can't just use smp_mb and friends though, because
    we must force memory ordering even if guest is UP since host could be
    running on another CPU, but SMP barriers are defined to barrier() in
    that configuration. So, for UP fall back to mandatory barriers instead.

    Signed-off-by: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Michael S. Tsirkin
     
  • With DEBUG defined, we add an ->in_use flag to detect if the caller
    invokes two virtio methods in parallel. The barriers attempt to ensure
    timely update of the ->in_use flag.

    But they're voodoo: if we need these barriers it implies that the
    calling code doesn't have sufficient synchronization to ensure the
    code paths aren't invoked at the same time anyway, and we want to
    detect it.

    Also, adding barriers changes timing, so turning on debug has more
    chance of hiding real problems.

    Thanks to MST for drawing my attention to this code...

    CC: Michael S. Tsirkin
    Signed-off-by: Rusty Russell

    Rusty Russell
     
  • When running under qemu-kvm-0.11.0:

    BUG: unable to handle kernel paging request at 56e58955
    ...
    Process vballoon (pid: 1297, ti=c7976000 task=c70a6ca0 task.ti=c7
    ...
    Call Trace:
    [] ? balloon+0x1b3/0x440 [virtio_balloon]
    [] ? schedule+0x327/0x9d0
    [] ? balloon+0x0/0x440 [virtio_balloon]
    [] ? kthread+0x74/0x80
    [] ? kthread+0x0/0x80
    [] ? kernel_thread_helper+0x6/0x30

    need_stats_update should be zero-initialized.

    Signed-off-by: Rusty Russell
    Acked-by: Adam Litke

    Rusty Russell
     
  • This is a fix for my earlier patch: "virtio: Add memory statistics reporting to
    the balloon driver (V4)".

    I discovered that all_vm_events() can sleep and therefore stats collection
    cannot be done in interrupt context. One solution is to handle the interrupt
    by noting that stats need to be collected and waking the existing vballoon
    kthread which will complete the work via stats_handle_request(). Rusty, is
    this a saner way of doing business?

    There is one issue that I would like a broader opinion on. In stats_request, I
    update vb->need_stats_update and then wake up the kthread. The kthread uses
    vb->need_stats_update as a condition variable. Do I need a memory barrier
    between the update and wake_up to ensure that my kthread sees the correct
    value? My testing suggests that it is not needed but I would like some
    confirmation from the experts.

    Signed-off-by: Adam Litke
    To: Rusty Russell
    Cc: Anthony Liguori
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Rusty Russell

    Adam Litke