linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"maz@kernel.org" <maz@kernel.org>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"julien.thierry.kdev@gmail.com" <julien.thierry.kdev@gmail.com>,
	"suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
	"jean-philippe@linaro.org" <jean-philippe@linaro.org>,
	"Alexandru.Elisei@arm.com" <Alexandru.Elisei@arm.com>,
	Linuxarm <linuxarm@huawei.com>,
	"qperret@google.com" <qperret@google.com>
Subject: Re: [PATCH v2 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one
Date: Thu, 22 Jul 2021 10:50:10 +0100	[thread overview]
Message-ID: <20210722095010.GA12012@willie-the-truck> (raw)
In-Reply-To: <f7d708704fb84380af85298a98f7a48c@huawei.com>

On Thu, Jul 22, 2021 at 06:45:14AM +0000, Shameerali Kolothum Thodi wrote:
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/tlb.c
> > b/arch/arm64/kvm/hyp/nvhe/tlb.c
> > > index 83dc3b271bc5..42df9931ed9a 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/tlb.c
> > > +++ b/arch/arm64/kvm/hyp/nvhe/tlb.c
> > > @@ -140,10 +140,10 @@ void __kvm_flush_cpu_context(struct
> > kvm_s2_mmu *mmu)
> > >  	__tlb_switch_to_host(&cxt);
> > >  }
> > >
> > > -void __kvm_flush_vm_context(void)
> > > +void __kvm_tlb_flush_local_all(void)
> > >  {
> > > -	dsb(ishst);
> > > -	__tlbi(alle1is);
> > > +	dsb(nshst);
> > > +	__tlbi(alle1);
> > >
> > >  	/*
> > >  	 * VIPT and PIPT caches are not affected by VMID, so no maintenance
> > > @@ -155,7 +155,7 @@ void __kvm_flush_vm_context(void)
> > >  	 *
> > >  	 */
> > >  	if (icache_is_vpipt())
> > > -		asm volatile("ic ialluis");
> > > +		asm volatile("ic iallu" : : );
> > >
> > > -	dsb(ish);
> > > +	dsb(nsh);
> > 
> > Hmm, I'm wondering whether having this local stuff really makes sense for
> > VMIDs. For ASIDs, where rollover can be frequent and TLBI could result in
> > IPI on 32-bit, the local option was important, but here rollover is less
> > frequent, DVM is relied upon to work and the cost of a hypercall is
> > significant with nVHE.
> > 
> > So I do think you could simplify patch 2 slightly to drop the
> > flush_pending and just issue inner-shareable invalidation on rollover.
> > With that, it might also make it straightforward to clear active_asids
> > when scheduling out a vCPU, which would solve the other problem I
> > mentioned
> > about unnecessarily reserving a bunch of the VMID space.
> 
> Ok. I will try out the above suggestion. Hope it will be acceptable for 8 bit 
> VMID systems as well as there is a higher chance for rollover especially
> when we introduce pinned VMIDs(I am not sure such platforms care about
> Pinned VMID or not. If not, we could limit Pinned VMIDs to 16 bit systems).

So I woke up at 3am in a cold sweat after dreaming about this code.

I think my suggestion above still stands for the VMID allocator, but
interestingly, it would _not_ be valid for the ASID allocator because there
the ASID is part of the active context and so, during the window where the
active_asid is out of sync with the TTBR, receiving a broadcast TLBI from a
concurrent rollover wouldn't be enough to knock out the old ASID from the
TLB despite it subsequently being made available for reallocation. So the
local TLB invalidation is not just a performance hint as I said; it's crucial
to the way the thing works (and this is also why the CnP code has to switch
to the reserved TTBR0).

As an aside: I'm more and more inclined to rip out the CnP stuff given
that it doesn't appear to being any benefits, but does have some clear
downsides. Perhaps something for next week.

Will

  parent reply	other threads:[~2021-07-22  9:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 15:56 [PATCH v2 0/3] kvm/arm: New VMID allocator based on asid Shameer Kolothum
2021-06-16 15:56 ` [PATCH v2 1/3] arch/arm64: Introduce a capability to tell whether 16-bit VMID is available Shameer Kolothum
2021-07-21 15:23   ` Will Deacon
2021-07-22  6:24     ` Shameerali Kolothum Thodi
2021-06-16 15:56 ` [PATCH v2 2/3] kvm/arm: Introduce a new vmid allocator for KVM Shameer Kolothum
2021-07-21 16:06   ` Will Deacon
2021-07-22  6:34     ` Shameerali Kolothum Thodi
2021-06-16 15:56 ` [PATCH v2 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one Shameer Kolothum
2021-07-21 16:31   ` Will Deacon
2021-07-22  6:45     ` Shameerali Kolothum Thodi
2021-07-22  9:11       ` Quentin Perret
2021-07-22 19:33         ` Marco Elver
2021-07-22  9:50       ` Will Deacon [this message]
2021-07-22 15:22         ` Vladimir Murzin
2021-07-22 15:38           ` Will Deacon
2021-07-23 15:49             ` Vladimir Murzin
2021-07-13  7:07 ` [PATCH v2 0/3] kvm/arm: New VMID allocator based on asid Shameerali Kolothum Thodi

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=20210722095010.GA12012@willie-the-truck \
    --to=will@kernel.org \
    --cc=Alexandru.Elisei@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=jean-philippe@linaro.org \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=maz@kernel.org \
    --cc=qperret@google.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=suzuki.poulose@arm.com \
    --subject='Re: [PATCH v2 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one' \
    /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

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).