31 Oct, 2019

1 commit

  • In kvm_create_vm(), if we've successfully called kvm_arch_init_vm(), but
    then fail later in the function, we need to call kvm_arch_destroy_vm()
    so that it can do any necessary cleanup (like freeing memory).

    Fixes: 44a95dae1d229a ("KVM: x86: Detect and Initialize AVIC support")

    Signed-off-by: John Sperbeck
    Signed-off-by: Jim Mattson
    Reviewed-by: Junaid Shahid
    [Remove dependency on "kvm: Don't clear reference count on
    kvm_create_vm() error path" which was not committed. - Paolo]
    Signed-off-by: Paolo Bonzini

    Jim Mattson
     

25 Oct, 2019

1 commit


22 Oct, 2019

2 commits


20 Oct, 2019

3 commits

  • The PMU emulation code uses the perf event sample period to trigger
    the overflow detection. This works fine for the *first* overflow
    handling, but results in a huge number of interrupts on the host,
    unrelated to the number of interrupts handled in the guest (a x20
    factor is pretty common for the cycle counter). On a slow system
    (such as a SW model), this can result in the guest only making
    forward progress at a glacial pace.

    It turns out that the clue is in the name. The sample period is
    exactly that: a period. And once the an overflow has occured,
    the following period should be the full width of the associated
    counter, instead of whatever the guest had initially programed.

    Reset the sample period to the architected value in the overflow
    handler, which now results in a number of host interrupts that is
    much closer to the number of interrupts in the guest.

    Fixes: b02386eb7dac ("arm64: KVM: Add PMU overflow interrupt routing")
    Reviewed-by: Andrew Murray
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • The current convention for KVM to request a chained event from the
    host PMU is to set bit[0] in attr.config1 (PERF_ATTR_CFG1_KVM_PMU_CHAINED).

    But as it turns out, this bit gets set *after* we create the kernel
    event that backs our virtual counter, meaning that we never get
    a 64bit counter.

    Moving the setting to an earlier point solves the problem.

    Fixes: 80f393a23be6 ("KVM: arm/arm64: Support chained PMU counters")
    Reviewed-by: Andrew Murray
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • When a counter is disabled, its value is sampled before the event
    is being disabled, and the value written back in the shadow register.

    In that process, the value gets truncated to 32bit, which is adequate
    for any counter but the cycle counter (defined as a 64bit counter).

    This obviously results in a corrupted counter, and things like
    "perf record -e cycles" not working at all when run in a guest...
    A similar, but less critical bug exists in kvm_pmu_get_counter_value.

    Make the truncation conditional on the counter not being the cycle
    counter, which results in a minor code reorganisation.

    Fixes: 80f393a23be6 ("KVM: arm/arm64: Support chained PMU counters")
    Reviewed-by: Andrew Murray
    Reported-by: Julien Thierry
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     

03 Oct, 2019

1 commit


01 Oct, 2019

1 commit


19 Sep, 2019

1 commit

  • Pull KVM updates from Paolo Bonzini:
    "s390:
    - ioctl hardening
    - selftests

    ARM:
    - ITS translation cache
    - support for 512 vCPUs
    - various cleanups and bugfixes

    PPC:
    - various minor fixes and preparation

    x86:
    - bugfixes all over the place (posted interrupts, SVM, emulation
    corner cases, blocked INIT)
    - some IPI optimizations"

    * tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm: (75 commits)
    KVM: X86: Use IPI shorthands in kvm guest when support
    KVM: x86: Fix INIT signal handling in various CPU states
    KVM: VMX: Introduce exit reason for receiving INIT signal on guest-mode
    KVM: VMX: Stop the preemption timer during vCPU reset
    KVM: LAPIC: Micro optimize IPI latency
    kvm: Nested KVM MMUs need PAE root too
    KVM: x86: set ctxt->have_exception in x86_decode_insn()
    KVM: x86: always stop emulation on page fault
    KVM: nVMX: trace nested VM-Enter failures detected by H/W
    KVM: nVMX: add tracepoint for failed nested VM-Enter
    x86: KVM: svm: Fix a check in nested_svm_vmrun()
    KVM: x86: Return to userspace with internal error on unexpected exit reason
    KVM: x86: Add kvm_emulate_{rd,wr}msr() to consolidate VXM/SVM code
    KVM: x86: Refactor up kvm_{g,s}et_msr() to simplify callers
    doc: kvm: Fix return description of KVM_SET_MSRS
    KVM: X86: Tune PLE Window tracepoint
    KVM: VMX: Change ple_window type to unsigned int
    KVM: X86: Remove tailing newline for tracepoints
    KVM: X86: Trace vcpu_id for vmexit
    KVM: x86: Manually calculate reserved bits when loading PDPTRS
    ...

    Linus Torvalds
     

18 Sep, 2019

1 commit

  • The first/last indexes are typically shared with a user app.
    The app can change the 'last' index that the kernel uses
    to store the next result. This change sanity checks the index
    before using it for writing to a potentially arbitrary address.

    This fixes CVE-2019-14821.

    Cc: stable@vger.kernel.org
    Fixes: 5f94c1741bdc ("KVM: Add coalesced MMIO support (common part)")
    Signed-off-by: Matt Delco
    Signed-off-by: Jim Mattson
    Reported-by: syzbot+983c866c3dd6efa3662a@syzkaller.appspotmail.com
    [Use READ_ONCE. - Paolo]
    Signed-off-by: Paolo Bonzini

    Matt Delco
     

11 Sep, 2019

1 commit

  • Commit 49dfe94fe5ad ("KVM: arm/arm64: Fix TRACE_INCLUDE_PATH") fixes
    TRACE_INCLUDE_PATH to the correct relative path to the define_trace.h
    and explains why did the old one work.

    The same fix should be applied to virt/kvm/arm/vgic/trace.h.

    Reviewed-by: Masahiro Yamada
    Signed-off-by: Zenghui Yu
    Signed-off-by: Marc Zyngier

    Zenghui Yu
     

09 Sep, 2019

1 commit

  • While parts of the VGIC support a large number of vcpus (we
    bravely allow up to 512), other parts are more limited.

    One of these limits is visible in the KVM_IRQ_LINE ioctl, which
    only allows 256 vcpus to be signalled when using the CPU or PPI
    types. Unfortunately, we've cornered ourselves badly by allocating
    all the bits in the irq field.

    Since the irq_type subfield (8 bit wide) is currently only taking
    the values 0, 1 and 2 (and we have been careful not to allow anything
    else), let's reduce this field to only 4 bits, and allocate the
    remaining 4 bits to a vcpu2_index, which acts as a multiplier:

    vcpu_id = 256 * vcpu2_index + vcpu_index

    With that, and a new capability (KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)
    allowing this to be discovered, it becomes possible to inject
    PPIs to up to 4096 vcpus. But please just don't.

    Whilst we're there, add a clarification about the use of KVM_IRQ_LINE
    on arm, which is not completely conditionned by KVM_CAP_IRQCHIP.

    Reported-by: Zenghui Yu
    Reviewed-by: Eric Auger
    Reviewed-by: Zenghui Yu
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     

29 Aug, 2019

1 commit

  • Pull arm64 fixes from Will Deacon:
    "Hot on the heels of our last set of fixes are a few more for -rc7.

    Two of them are fixing issues with our virtual interrupt controller
    implementation in KVM/arm, while the other is a longstanding but
    straightforward kallsyms fix which was been acked by Masami and
    resolves an initialisation failure in kprobes observed on arm64.

    - Fix GICv2 emulation bug (KVM)

    - Fix deadlock in virtual GIC interrupt injection code (KVM)

    - Fix kprobes blacklist init failure due to broken kallsyms lookup"

    * tag 'arm64-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux:
    KVM: arm/arm64: vgic-v2: Handle SGI bits in GICD_I{S,C}PENDR0 as WI
    KVM: arm/arm64: vgic: Fix potential deadlock when ap_list is long
    kallsyms: Don't let kallsyms_lookup_size_offset() fail on retrieving the first symbol

    Linus Torvalds
     

28 Aug, 2019

1 commit

  • A guest is not allowed to inject a SGI (or clear its pending state)
    by writing to GICD_ISPENDR0 (resp. GICD_ICPENDR0), as these bits are
    defined as WI (as per ARM IHI 0048B 4.3.7 and 4.3.8).

    Make sure we correctly emulate the architecture.

    Fixes: 96b298000db4 ("KVM: arm/arm64: vgic-new: Add PENDING registers handlers")
    Cc: stable@vger.kernel.org # 4.7+
    Reported-by: Andre Przywara
    Signed-off-by: Marc Zyngier
    Signed-off-by: Will Deacon

    Marc Zyngier
     

27 Aug, 2019

1 commit

  • If the ap_list is longer than 256 entries, merge_final() in list_sort()
    will call the comparison callback with the same element twice, causing
    a deadlock in vgic_irq_cmp().

    Fix it by returning early when irqa == irqb.

    Cc: stable@vger.kernel.org # 4.7+
    Fixes: 8e4447457965 ("KVM: arm/arm64: vgic-new: Add IRQ sorting")
    Signed-off-by: Zenghui Yu
    Signed-off-by: Heyi Guo
    [maz: massaged commit log and patch, added Fixes and Cc-stable]
    Signed-off-by: Marc Zyngier
    Signed-off-by: Will Deacon

    Heyi Guo
     

25 Aug, 2019

2 commits

  • At the moment we use 2 IO devices per GICv3 redistributor: one
    one for the RD_base frame and one for the SGI_base frame.

    Instead we can use a single IO device per redistributor (the 2
    frames are contiguous). This saves slots on the KVM_MMIO_BUS
    which is currently limited to NR_IOBUS_DEVS (1000).

    This change allows to instantiate up to 512 redistributors and may
    speed the guest boot with a large number of VCPUs.

    Signed-off-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Eric Auger
     
  • Detected by Coccinelle (and Will Deacon) using
    scripts/coccinelle/misc/semicolon.cocci.

    Reported-by: Will Deacon
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     

24 Aug, 2019

2 commits

  • …git/kvmarm/kvmarm into kvm/fixes

    Pull KVM/arm fixes from Marc Zyngier as per Paulo's request at:

    https://lkml.kernel.org/r/21ae69a2-2546-29d0-bff6-2ea825e3d968@redhat.com

    "One (hopefully last) set of fixes for KVM/arm for 5.3: an embarassing
    MMIO emulation regression, and a UBSAN splat. Oh well...

    - Don't overskip instructions on MMIO emulation

    - Fix UBSAN splat when initializing PPI priorities"

    * tag 'kvmarm-fixes-for-5.3-3' of git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm:
    KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity
    KVM: arm/arm64: Only skip MMIO insn once

    Will Deacon
     
  • At the moment we initialise the target *mask* of a virtual IRQ to the
    VCPU it belongs to, even though this mask is only defined for GICv2 and
    quickly runs out of bits for many GICv3 guests.
    This behaviour triggers an UBSAN complaint for more than 32 VCPUs:
    ------
    [ 5659.462377] UBSAN: Undefined behaviour in virt/kvm/arm/vgic/vgic-init.c:223:21
    [ 5659.471689] shift exponent 32 is too large for 32-bit type 'unsigned int'
    ------
    Also for GICv3 guests the reporting of TARGET in the "vgic-state" debugfs
    dump is wrong, due to this very same problem.

    Because there is no requirement to create the VGIC device before the
    VCPUs (and QEMU actually does it the other way round), we can't safely
    initialise mpidr or targets in kvm_vgic_vcpu_init(). But since we touch
    every private IRQ for each VCPU anyway later (in vgic_init()), we can
    just move the initialisation of those fields into there, where we
    definitely know the VGIC type.

    On the way make sure we really have either a VGICv2 or a VGICv3 device,
    since the existing code is just checking for "VGICv3 or not", silently
    ignoring the uninitialised case.

    Signed-off-by: Andre Przywara
    Reported-by: Dave Martin
    Tested-by: Julien Grall
    Signed-off-by: Marc Zyngier

    Andre Przywara
     

22 Aug, 2019

1 commit

  • If after an MMIO exit to userspace a VCPU is immediately run with an
    immediate_exit request, such as when a signal is delivered or an MMIO
    emulation completion is needed, then the VCPU completes the MMIO
    emulation and immediately returns to userspace. As the exit_reason
    does not get changed from KVM_EXIT_MMIO in these cases we have to
    be careful not to complete the MMIO emulation again, when the VCPU is
    eventually run again, because the emulation does an instruction skip
    (and doing too many skips would be a waste of guest code :-) We need
    to use additional VCPU state to track if the emulation is complete.
    As luck would have it, we already have 'mmio_needed', which even
    appears to be used in this way by other architectures already.

    Fixes: 0d640732dbeb ("arm64: KVM: Skip MMIO insn after emulation")
    Acked-by: Mark Rutland
    Signed-off-by: Andrew Jones
    Signed-off-by: Marc Zyngier

    Andrew Jones
     

19 Aug, 2019

12 commits

  • When a vpcu is about to block by calling kvm_vcpu_block, we call
    back into the arch code to allow any form of synchronization that
    may be required at this point (SVN stops the AVIC, ARM synchronises
    the VMCR and enables GICv4 doorbells). But this synchronization
    comes in quite late, as we've potentially waited for halt_poll_ns
    to expire.

    Instead, let's move kvm_arch_vcpu_blocking() to the beginning of
    kvm_vcpu_block(), which on ARM has several benefits:

    - VMCR gets synchronised early, meaning that any interrupt delivered
    during the polling window will be evaluated with the correct guest
    PMR
    - GICv4 doorbells are enabled, which means that any guest interrupt
    directly injected during that window will be immediately recognised

    Tang Nianyao ran some tests on a GICv4 machine to evaluate such
    change, and reported up to a 10% improvement for netperf:

    netperf result:
    D06 as server, intel 8180 server as client
    with change:
    package 512 bytes - 5500 Mbits/s
    package 64 bytes - 760 Mbits/s
    without change:
    package 512 bytes - 5000 Mbits/s
    package 64 bytes - 710 Mbits/s

    Acked-by: Paolo Bonzini
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • Since commit 503a62862e8f ("KVM: arm/arm64: vgic: Rely on the GIC driver to
    parse the firmware tables"), the vgic_v{2,3}_probe functions stopped using
    a DT node. Commit 909777324588 ("KVM: arm/arm64: vgic-new: vgic_init:
    implement kvm_vgic_hyp_init") changed the functions again, and now they
    require exactly one argument, a struct gic_kvm_info populated by the GIC
    driver. Unfortunately the comments regressed and state that a DT node is
    used instead. Change the function comments to reflect the current
    prototypes.

    Signed-off-by: Alexandru Elisei
    Signed-off-by: Marc Zyngier

    Alexandru Elisei
     
  • Now that we have a cache of MSI->LPI translations, it is pretty
    easy to implement kvm_arch_set_irq_inatomic (this cache can be
    parsed without sleeping).

    Hopefully, this will improve some LPI-heavy workloads.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • When performing an MSI injection, let's first check if the translation
    is already in the cache. If so, let's inject it quickly without
    going through the whole translation process.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • On a successful translation, preserve the parameters in the LPI
    translation cache. Each translation is reusing the last slot
    in the list, naturally evicting the least recently used entry.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • In order to avoid leaking vgic_irq structures on teardown, we need to
    drop all references to LPIs before deallocating the cache itself.

    This is done by invalidating the cache on vgic teardown.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • If an ITS gets disabled, we need to make sure that further interrupts
    won't hit in the cache. For that, we invalidate the translation cache
    when the ITS is disabled.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • If a vcpu disables LPIs at its redistributor level, we need to make sure
    we won't pend more interrupts. For this, we need to invalidate the LPI
    translation cache.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • The LPI translation cache needs to be discarded when an ITS command
    may affect the translation of an LPI (DISCARD, MAPC and MAPD with V=0)
    or the routing of an LPI to a redistributor with disabled LPIs (MOVI,
    MOVALL).

    We decide to perform a full invalidation of the cache, irrespective
    of the LPI that is affected. Commands are supposed to be rare enough
    that it doesn't matter.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • There's a number of cases where we need to invalidate the caching
    of translations, so let's add basic support for that.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • Our LPI translation cache needs to be able to drop the refcount
    on an LPI whilst already holding the lpi_list_lock.

    Let's add a new primitive for this.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • Add the basic data structure that expresses an MSI to LPI
    translation as well as the allocation/release hooks.

    The size of the cache is arbitrarily defined as 16*nr_vcpus.

    Tested-by: Andre Przywara
    Reviewed-by: Eric Auger
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     

09 Aug, 2019

4 commits


05 Aug, 2019

3 commits

  • Since commit commit 328e56647944 ("KVM: arm/arm64: vgic: Defer
    touching GICH_VMCR to vcpu_load/put"), we leave ICH_VMCR_EL2 (or
    its GICv2 equivalent) loaded as long as we can, only syncing it
    back when we're scheduled out.

    There is a small snag with that though: kvm_vgic_vcpu_pending_irq(),
    which is indirectly called from kvm_vcpu_check_block(), needs to
    evaluate the guest's view of ICC_PMR_EL1. At the point were we
    call kvm_vcpu_check_block(), the vcpu is still loaded, and whatever
    changes to PMR is not visible in memory until we do a vcpu_put().

    Things go really south if the guest does the following:

    mov x0, #0 // or any small value masking interrupts
    msr ICC_PMR_EL1, x0

    [vcpu preempted, then rescheduled, VMCR sampled]

    mov x0, #ff // allow all interrupts
    msr ICC_PMR_EL1, x0
    wfi // traps to EL2, so samping of VMCR

    [interrupt arrives just after WFI]

    Here, the hypervisor's view of PMR is zero, while the guest has enabled
    its interrupts. kvm_vgic_vcpu_pending_irq() will then say that no
    interrupts are pending (despite an interrupt being received) and we'll
    block for no reason. If the guest doesn't have a periodic interrupt
    firing once it has blocked, it will stay there forever.

    To avoid this unfortuante situation, let's resync VMCR from
    kvm_arch_vcpu_blocking(), ensuring that a following kvm_vcpu_check_block()
    will observe the latest value of PMR.

    This has been found by booting an arm64 Linux guest with the pseudo NMI
    feature, and thus using interrupt priorities to mask interrupts instead
    of the usual PSTATE masking.

    Cc: stable@vger.kernel.org # 4.12
    Fixes: 328e56647944 ("KVM: arm/arm64: vgic: Defer touching GICH_VMCR to vcpu_load/put")
    Signed-off-by: Marc Zyngier

    Marc Zyngier
     
  • When calling debugfs functions, there is no need to ever check the
    return value. The function can work or not, but the code logic should
    never do something different based on this.

    Also, when doing this, change kvm_arch_create_vcpu_debugfs() to return
    void instead of an integer, as we should not care at all about if this
    function actually does anything or not.

    Cc: Paolo Bonzini
    Cc: "Radim Krčmář"
    Cc: Thomas Gleixner
    Cc: Ingo Molnar
    Cc: Borislav Petkov
    Cc: "H. Peter Anvin"
    Cc:
    Cc:
    Signed-off-by: Greg Kroah-Hartman
    Signed-off-by: Paolo Bonzini

    Greg KH
     
  • There is no need for this function as all arches have to implement
    kvm_arch_create_vcpu_debugfs() no matter what. A #define symbol
    let us actually simplify the code.

    Signed-off-by: Paolo Bonzini

    Paolo Bonzini