KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Jayachandran Chandrasekharan Nair <jnair@marvell.com>
To: Will Deacon <will@kernel.org>
Cc: Tomasz Nowicki <tnowicki@marvell.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Robert Richter <rrichter@marvell.com>,
	Marc Zyngier <maz@kernel.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
Subject: Re: [PATCH 0/2] Workaround for Cavium ThunderX2 erratum 219
Date: Fri, 11 Oct 2019 23:20:41 +0000
Message-ID: <20191011232031.GA29752@dc5-eodlnx05.marvell.com> (raw)
In-Reply-To: <20191011104454.d7dplgyjcnpfi5p3@willie-the-truck>

On Fri, Oct 11, 2019 at 11:44:55AM +0100, Will Deacon wrote:
> Hi JC,
> Thanks for posting this.
> On Fri, Oct 11, 2019 at 10:35:21AM +0000, Jayachandran Chandrasekharan Nair wrote:
> > These two patches are based on the work by Marc Zyngier and addresses
> > Cavium ThunderX2 erratum 219.
> > 
> > This erratum (originally reported by ARM folks) is from an interesting
> > use of the prefetch instruction in the KPTI patchset. The prefetch
> > was done between a TTBR change and the corresponding ISB, and this
> > occasionally caused a crash on ThunderX2.
> > 
> > The first patch removes the troublesome prefetch for ThunderX2.
> > The second patch addresses the case where the issue can be triggered
> > from a guest kernel. The workaround in this case is to trap TTBR
> > accesses by setting HCR_EL2.TVM for guests and doing the system
> > register update from EL2 in a fast path.
> FWIW, I was already planning to send the following to Linus:
> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=errata/tx2-219
> so please base any changes on top of that branch.

Please consider taking my patchset as is, if you don't have
issues with patches.

> > Due to the nature of the erratum, the trap-and-emulate is only
> > needed when SMT is enabled.
> > 
> > The overhead of trap-and-emulate is expected to be negligible on most
> > workloads. A command line option kvm-arm.vm_msr_trap has been
> > provided to override trapping on guest TTBR updates.  This is to
> > address a very limited case where a user wants to run SMT enabled,
> > with a trustworthy guest kernel, and wants to avoid the performance
> > overhead associated with emulating the address translation register
> > changes.
> Do you have any performance data to show the impact of the workaround on
> non-kpti guests? I don't think we can justify the inclusion of a cmdline
> option for this without figures showing that it's really necessary.
> Otherwise, the "very limited case" really is a niche scenario where the
> CONFIG option can simply be disabled.

In my view, you are switching the responsibility here. Even one use
case should be enough reason not to force a performance regression
that cannot be opted out of. You are expected to leave as much policy
as reasonable to the user with safe (and least astonishing) defaults.

A class of guest usecases involve running stock images (linux or
non-linux). The arm64 server ecosystem is still in development, so
we should allow someone evaluating a server system to turn off or
on options as much as possible without forcing a re-compile.

Also, the run-time option is generic enough that it can be switched
on/off for any platform, not just the one affected by this erratum.

So, I disagree with the queued patchset - but I will leave you to make
your call on which way to go.

kvmarm mailing list

      reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 10:35 Jayachandran Chandrasekharan Nair
2019-10-11 10:35 ` [PATCH 1/2] arm64: " Jayachandran Chandrasekharan Nair
2019-10-11 10:35 ` [PATCH 2/2] arm64: KVM: Add option to trap and emulate guest VM sysreg updates Jayachandran Chandrasekharan Nair
2019-10-11 10:44 ` [PATCH 0/2] Workaround for Cavium ThunderX2 erratum 219 Will Deacon
2019-10-11 23:20   ` Jayachandran Chandrasekharan Nair [this message]

Reply instructions:

You may reply publically 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:

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

  git send-email \
    --in-reply-to=20191011232031.GA29752@dc5-eodlnx05.marvell.com \
    --to=jnair@marvell.com \
    --cc=catalin.marinas@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=rrichter@marvell.com \
    --cc=tnowicki@marvell.com \
    --cc=will@kernel.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu kvmarm@archiver.kernel.org
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/ public-inbox