kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
From: Oliver Upton <oliver.upton@linux.dev>
To: kvmarm@lists.linux.dev
Cc: kvm@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	linux-kernel@vger.kernel.org,
	Oliver Upton <oliver.upton@linux.dev>
Subject: [PATCH v2 00/23] KVM: arm64: Improvements to LPI injection
Date: Tue, 13 Feb 2024 09:32:37 +0000	[thread overview]
Message-ID: <20240213093250.3960069-1-oliver.upton@linux.dev> (raw)

For full details on the what/why, please see the cover letter in v1.

Apologies for the delay on v2, I wanted to spend some time to get a
microbenchmark in place to slam the ITS code pretty hard, and based on
the results I'm glad I did.

The test is built around having vCPU threads and device threads, where
each device can signal a particular number of event IDs (alien, right?)

Anyway, here are the results of that test in some carefully-selected
examples:

+----------------------------+---------------------+------------------------------+
|           Config           | v6.8-rc1 (LPIs/sec) | v6.8-rc1 + series (LPIs/sec) |
+----------------------------+---------------------+------------------------------+
| -v 1 -d 1 -e 1 -i 1000000  |           780151.37 |                   1255291.00 |
| -v 16 -d 16 -e 16 -i 10000 |           437081.55 |                   5078225.70 |
| -v 16 -d 16 -e 17 -i 10000 |           506446.50 |                   1126118.10 |
| -v 64 -d 64 -e 1 -i 100000 |           295097.03 |                   5565964.87 |
| -v 1 -d 1 -e 17 -i 1000    |           732934.43 |                       149.24 |
+----------------------------+---------------------+------------------------------+

While there is an 18x improvement in the scaled-out config (64 vCPUs, 64
devices, 1 event per device), there is an extremely disappointing 4911x
regression in the example that effectively forces a cache eviction for
every lookup.

Clearly the RCU synchronization is a bounding issue in this case. I
think other scenarios where the cache is overcommitted (16 vCPUs, 16
devices, 17 events / device) are able to hide effects somewhat, as other
threads can make forward progress while others are stuck waiting on RCU.

A few ideas on next steps:

 1) Rework the lpi_list_lock as an rwlock. This would obviate the need
    for RCU protection in the LPI cache as well as memory allocations on
    the injection path. This is actually what I had in the internal
    version of the series, although it was very incomplete.

    I'd expect this to nullify the improvement on the
    slightly-overcommitted case and 'fix' the pathological case.

 2) call_rcu() and move on. This feels somewhat abusive of the API, as
    the guest can flood the host with RCU callbacks, but I wasn't able
    to make my machine fall over in any mean configuration of the test.

    I haven't studied the degree to which such a malicious VM could
    adversely affect neighboring workloads.

 3) Redo the whole ITS representation with xarrays and allow RCU readers
    outside of the ITS lock. I haven't fully thought this out, and if we
    pursue this option then we will need a secondary data structure to
    track where ITSes have been placed in guest memory to avoid taking
    the SRCU lock. We can then stick RCU synchronization in ITS command
    processing, which feels right to me, and dump the translation cache
    altogether.

    I'd expect slightly worse average case performance in favor of more
    consistent performance.

Even though it is more work, I'm slightly in favor of (3) as it is a
net reduction in overall complexity of the ITS implementation. But, I
wanted to send out what I had to guage opinions on these options, and
get feedback on the first 10 patches which are an overall win.

v1: https://lore.kernel.org/kvmarm/20240124204909.105952-1-oliver.upton@linux.dev/

v1 -> v2:
 - Add the microbenchmark
 - Add tracepoints / VM stats for the important bits of LPI injection.
   This was extremely useful for making sense of test results.
 - Fix a silly lock imbalance on error path in vgic_add_lpi() (Dan)
 - Constrain xas_for_each() based on the properties of the INTID space
   (Marc)
 - Remove some missed vestiges of the LPI linked-list (Marc)
 - Explicitly free unused cache entry on failed insertion race (Marc)
 - Don't explode people's machines with a boatload of xchg() (I said it
   was WIP!) (Marc)

Oliver Upton (23):
  KVM: arm64: Add tracepoints + stats for LPI cache effectiveness
  KVM: arm64: vgic: Store LPIs in an xarray
  KVM: arm64: vgic: Use xarray to find LPI in vgic_get_lpi()
  KVM: arm64: vgic-v3: Iterate the xarray to find pending LPIs
  KVM: arm64: vgic-its: Walk the LPI xarray in vgic_copy_lpi_list()
  KVM: arm64: vgic: Get rid of the LPI linked-list
  KVM: arm64: vgic: Use atomics to count LPIs
  KVM: arm64: vgic: Free LPI vgic_irq structs in an RCU-safe manner
  KVM: arm64: vgic: Rely on RCU protection in vgic_get_lpi()
  KVM: arm64: vgic: Ensure the irq refcount is nonzero when taking a ref
  KVM: arm64: vgic: Don't acquire the lpi_list_lock in vgic_put_irq()
  KVM: arm64: vgic-its: Lazily allocate LPI translation cache
  KVM: arm64: vgic-its: Pick cache victim based on usage count
  KVM: arm64: vgic-its: Protect cached vgic_irq pointers with RCU
  KVM: arm64: vgic-its: Treat the LPI translation cache as an rculist
  KVM: arm64: vgic-its: Rely on RCU to protect translation cache reads
  KVM: selftests: Align with kernel's GIC definitions
  KVM: selftests: Standardise layout of GIC frames
  KVM: selftests: Add a minimal library for interacting with an ITS
  KVM: selftests: Add helper for enabling LPIs on a redistributor
  KVM: selftests: Use MPIDR_HWID_BITMASK from cputype.h
  KVM: selftests: Hack in support for aligned page allocations
  KVM: selftests: Add stress test for LPI injection

 arch/arm64/include/asm/kvm_host.h             |   3 +
 arch/arm64/kvm/guest.c                        |   5 +-
 arch/arm64/kvm/vgic/trace.h                   |  66 ++
 arch/arm64/kvm/vgic/vgic-debug.c              |   2 +-
 arch/arm64/kvm/vgic/vgic-init.c               |   7 +-
 arch/arm64/kvm/vgic/vgic-its.c                | 220 ++++---
 arch/arm64/kvm/vgic/vgic-v3.c                 |   3 +-
 arch/arm64/kvm/vgic/vgic.c                    |  56 +-
 arch/arm64/kvm/vgic/vgic.h                    |  15 +-
 include/kvm/arm_vgic.h                        |  10 +-
 include/linux/kvm_host.h                      |   4 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/arch_timer.c        |   8 +-
 .../testing/selftests/kvm/aarch64/psci_test.c |   2 +
 .../testing/selftests/kvm/aarch64/vgic_irq.c  |  15 +-
 .../selftests/kvm/aarch64/vgic_lpi_stress.c   | 388 ++++++++++++
 .../kvm/aarch64/vpmu_counter_access.c         |   6 +-
 .../selftests/kvm/dirty_log_perf_test.c       |   5 +-
 .../selftests/kvm/include/aarch64/gic.h       |  15 +-
 .../selftests/kvm/include/aarch64/gic_v3.h    | 586 +++++++++++++++++-
 .../selftests/kvm/include/aarch64/processor.h |   2 -
 .../selftests/kvm/include/aarch64/vgic.h      |  27 +-
 .../selftests/kvm/include/kvm_util_base.h     |   2 +
 tools/testing/selftests/kvm/lib/aarch64/gic.c |  18 +-
 .../selftests/kvm/lib/aarch64/gic_private.h   |   4 +-
 .../selftests/kvm/lib/aarch64/gic_v3.c        |  69 ++-
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 337 +++++++++-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  27 +-
 28 files changed, 1641 insertions(+), 262 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/vgic_lpi_stress.c


base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
-- 
2.43.0.687.g38aa6559b0-goog


             reply	other threads:[~2024-02-13  9:33 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-13  9:32 Oliver Upton [this message]
2024-02-13  9:32 ` [PATCH v2 01/23] KVM: arm64: Add tracepoints + stats for LPI cache effectiveness Oliver Upton
2024-02-14 15:55   ` Marc Zyngier
2024-02-13  9:32 ` [PATCH v2 02/23] KVM: arm64: vgic: Store LPIs in an xarray Oliver Upton
2024-02-13 21:52   ` Oliver Upton
2024-02-13  9:32 ` [PATCH v2 03/23] KVM: arm64: vgic: Use xarray to find LPI in vgic_get_lpi() Oliver Upton
2024-02-13  9:32 ` [PATCH v2 04/23] KVM: arm64: vgic-v3: Iterate the xarray to find pending LPIs Oliver Upton
2024-02-13  9:32 ` [PATCH v2 05/23] KVM: arm64: vgic-its: Walk the LPI xarray in vgic_copy_lpi_list() Oliver Upton
2024-02-13  9:32 ` [PATCH v2 06/23] KVM: arm64: vgic: Get rid of the LPI linked-list Oliver Upton
2024-02-13  9:32 ` [PATCH v2 07/23] KVM: arm64: vgic: Use atomics to count LPIs Oliver Upton
2024-02-14 16:47   ` Marc Zyngier
2024-02-14 18:32     ` Oliver Upton
2024-02-14 20:01       ` Marc Zyngier
2024-02-14 23:01         ` Oliver Upton
2024-02-15  9:44           ` Marc Zyngier
2024-02-13  9:32 ` [PATCH v2 08/23] KVM: arm64: vgic: Free LPI vgic_irq structs in an RCU-safe manner Oliver Upton
2024-02-13  9:32 ` [PATCH v2 09/23] KVM: arm64: vgic: Rely on RCU protection in vgic_get_lpi() Oliver Upton
2024-02-13  9:32 ` [PATCH v2 10/23] KVM: arm64: vgic: Ensure the irq refcount is nonzero when taking a ref Oliver Upton
2024-02-13  9:32 ` [PATCH v2 11/23] KVM: arm64: vgic: Don't acquire the lpi_list_lock in vgic_put_irq() Oliver Upton
2024-02-13  9:32 ` [PATCH v2 12/23] KVM: arm64: vgic-its: Lazily allocate LPI translation cache Oliver Upton
2024-02-13  9:32 ` [PATCH v2 13/23] KVM: arm64: vgic-its: Pick cache victim based on usage count Oliver Upton
2024-02-13  9:37 ` [PATCH v2 14/23] KVM: arm64: vgic-its: Protect cached vgic_irq pointers with RCU Oliver Upton
2024-02-13  9:39 ` [PATCH v2 15/23] KVM: arm64: vgic-its: Treat the LPI translation cache as an rculist Oliver Upton
2024-02-13  9:40 ` [PATCH v2 16/23] KVM: arm64: vgic-its: Rely on RCU to protect translation cache reads Oliver Upton
2024-02-13  9:40 ` [PATCH v2 17/23] KVM: selftests: Align with kernel's GIC definitions Oliver Upton
2024-02-13  9:40 ` [PATCH v2 18/23] KVM: selftests: Standardise layout of GIC frames Oliver Upton
2024-02-13  9:41 ` [PATCH v2 19/23] KVM: selftests: Add a minimal library for interacting with an ITS Oliver Upton
2024-02-14 17:32   ` Marc Zyngier
2024-02-14 19:00     ` Oliver Upton
2024-02-14 20:09       ` Marc Zyngier
2024-02-14 20:55         ` Oliver Upton
2024-02-14 21:06           ` Oliver Upton
2024-02-13  9:41 ` [PATCH v2 20/23] KVM: selftests: Add helper for enabling LPIs on a redistributor Oliver Upton
2024-02-13  9:42 ` [PATCH v2 21/23] KVM: selftests: Use MPIDR_HWID_BITMASK from cputype.h Oliver Upton
2024-02-13  9:43 ` [PATCH v2 22/23] KVM: selftests: Hack in support for aligned page allocations Oliver Upton
2024-02-13  9:43 ` [PATCH v2 23/23] KVM: selftests: Add stress test for LPI injection Oliver Upton
2024-02-13 20:12 ` [PATCH v2 00/23] KVM: arm64: Improvements to " Oliver Upton
2024-02-14 17:43 ` Marc Zyngier
2024-02-14 18:40   ` Oliver Upton
2024-02-15 15:37     ` Marc Zyngier
2024-02-15 20:15       ` Oliver Upton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240213093250.3960069-1-oliver.upton@linux.dev \
    --to=oliver.upton@linux.dev \
    --cc=james.morse@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).