KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Guo Ren <guoren@kernel.org>
To: Will Deacon <will.deacon@arm.com>
Cc: aou@eecs.berkeley.edu, Arnd Bergmann <arnd@arndb.de>,
	Marc Zyngier <marc.zyngier@arm.com>,
	catalin.marinas@arm.com, Anup Patel <anup.Patel@wdc.com>,
	linux-kernel@vger.kernel.org, rppt@linux.ibm.com,
	hch@infradead.org, Atish.Patra@wdc.com,
	Julien Grall <julien.grall@arm.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	gary@garyguo.net, paul.walmsley@sifive.com,
	linux-riscv@lists.infradead.org, kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file
Date: Thu, 20 Jun 2019 17:33:03 +0800
Message-ID: <CAJF2gTSiiiewTLwVAXvPLO7rTSUw1rg8VtFLzANdP2S2EEbTjg@mail.gmail.com> (raw)
In-Reply-To: <20190619123939.GF7767@fuggles.cambridge.arm.com>

On Wed, Jun 19, 2019 at 8:39 PM Will Deacon <will.deacon@arm.com> wrote:
>
> On Wed, Jun 19, 2019 at 08:18:04PM +0800, Guo Ren wrote:
> > On Wed, Jun 19, 2019 at 5:12 PM Will Deacon <will.deacon@arm.com> wrote:
> > > This is one place where I'd actually prefer not to go down the route of
> > > making the code generic. Context-switching and low-level TLB management
> > > is deeply architecture-specific and I worry that by trying to make this
> > > code common, we run the real risk of introducing subtle bugs on some
> > > architecture every time it is changed.
> > "Add generic asid code" and "move arm's into generic" are two things.
> > We could do
> > first and let architecture's maintainer to choose.
>
> If I understand the proposal being discussed, it involves basing that
> generic ASID allocation code around the arm64 implementation which I don't
> necessarily think is a good starting point.
...
>
> > > Furthermore, the algorithm we use
> > > on arm64 is designed to scale to large systems using DVM and may well be
> > > too complex and/or sub-optimal for architectures with different system
> > > topologies or TLB invalidation mechanisms.
> > It's just a asid algorithm not very complex and there is a callback
> > for architecture to define their
> > own local hart tlb flush. Seems it has nothing with DVM or tlb
> > broadcast mechanism.
>
> I'm pleased that you think the algorithm is not very complex, but I'm also
> worried that you might not have fully understood some of its finer details.
I understand your concern about my less understanding of asid
technology. Here is
my short-description of arm64 asid allocator: (If you find anything
wrong, please
correct me directly, thx :)
The asid allocator use five level check to reduce the cost of switch_mm.
 1. Check if the asid version is the same (it's general)
 2. Check reserved_asid which is set in rollover flush_context() and
the key point is
     keep the same bit position with the current asid version instead
of input version.
 3. Check if the position of bitmap is free then it could be set &
used directly.
 4. find_next_zero_bit() (a little performance cost)
 5. flush_context  (this is the worst cost with increase current asid version)

Check is level by level and cost is also higher with the next level.
The design that
impressed me the most was reserved_asid and bitmap and the 2th level and 3th
level will prevent unnecessary find_next_zero_bit().

The atomic 64 bit asid is also ok for 32-bit system and it won't cost
a lot in 1th 2th 3th
level check.

The operation of set/clear mm_cpumask was removed in arm64 compared to arm32.
It seems no side effect on current arm64 system, but from software
meaning it's wrong.
So I think it should be reserved in generic version.

>
> The reason I mention DVM and TLB broadcasting is because, depending on
> the mechanisms in your architecture relating to those, it may be strictly
> required that all concurrently running threads of a process have the same
> ASID at any given point in time, or it may be that you really don't care.
>
> If you don't care, then the arm64 allocator is over-engineered and likely
> inefficient for your system. If you do care, then it's worth considering
> whether a lock is sufficient around the allocator if you don't expect high
> core counts. Another possibility is that you end up using only one ASID and
> invalidating the local TLB on every context switch. Yet another design
> would be to manage per-cpu ASID pools.
I'll keep my system use the same ASID for SMP + IOMMU :P
Yes, there are two styles of asid allocator: per-cpu ASID (MIPS) or
same ASID (ARM).
If the CPU couldn't support cache/tlb coherency maintian in hardware,
it should use
per-cpu ASID style because IPI is expensive and per-cpu ASID style
need more software
mechanism to improve performance (eg: delay cache flush). From software view the
same ASID is clearer and easier to build bigger system with more TLB caches.

I think the same ASID style is a more sensible choice for modern
processor and let it be
one of generic is reasonable.

>
> So rather than blindly copying the arm64 code, I suggest sitting down and
> designing something that fits to your architecture instead. You may end up
> with something that is both simpler and more efficient.
In fact, riscv folks have discussed a lot about arm's asid allocator
and I learned
a lot from the discussion:
https://lore.kernel.org/linux-riscv/20190327100201.32220-1-anup.patel@wdc.com/

We are developing C-SKY and RISC-V ISA cpu cores and make it generic
is good for us.

Best Regards
 Guo Ren
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply index

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 16:36 [PATCH RFC 00/14] kvm/arm: Align the VMID allocation with the arm64 ASID one Julien Grall
2019-03-21 16:36 ` [PATCH RFC 01/14] arm64/mm: Introduce asid_info structure and move asid_generation/asid_map to it Julien Grall
2019-03-21 17:03   ` Suzuki K Poulose
2019-03-21 17:27     ` Julien Grall
2019-03-21 16:36 ` [PATCH RFC 02/14] arm64/mm: Move active_asids and reserved_asids to asid_info Julien Grall
2019-03-21 16:36 ` [PATCH RFC 03/14] arm64/mm: Move bits " Julien Grall
2019-03-21 16:36 ` [PATCH RFC 04/14] arm64/mm: Move the variable lock and tlb_flush_pending " Julien Grall
2019-03-21 16:36 ` [PATCH RFC 05/14] arm64/mm: Remove dependency on MM in new_context Julien Grall
2019-03-21 16:36 ` [PATCH RFC 06/14] arm64/mm: Store the number of asid allocated per context Julien Grall
2019-03-21 16:36 ` [PATCH RFC 07/14] arm64/mm: Introduce NUM_ASIDS Julien Grall
2019-03-21 16:36 ` [PATCH RFC 08/14] arm64/mm: Split asid_inits in 2 parts Julien Grall
2019-03-21 16:36 ` [PATCH RFC 09/14] arm64/mm: Split the function check_and_switch_context in 3 parts Julien Grall
2019-03-21 16:36 ` [PATCH RFC 10/14] arm64/mm: Introduce a callback to flush the local context Julien Grall
2019-03-21 16:36 ` [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file Julien Grall
2019-06-05 16:56   ` Julien Grall
2019-06-05 20:41     ` Palmer Dabbelt
2019-06-11  1:56       ` Gary Guo
2019-06-19  8:07     ` Guo Ren
2019-06-19  8:54       ` Julien Grall
2019-06-19  9:12         ` Will Deacon
2019-06-19 12:18           ` Guo Ren
2019-06-19 12:39             ` Will Deacon
2019-06-20  9:33               ` Guo Ren [this message]
2019-06-24 10:40                 ` Will Deacon
2019-06-25  7:25                   ` Palmer Dabbelt
2019-09-07 23:52                   ` Guo Ren
2019-09-12 14:02                     ` Will Deacon
2019-09-12 14:59                       ` Guo Ren
2019-09-13  7:13                         ` Guo Ren
2019-09-14  8:49                           ` Guo Ren
2019-09-16 12:57                           ` Jean-Philippe Brucker
2019-09-19 13:07                             ` Guo Ren
2019-09-19 15:18                               ` Jean-Philippe Brucker
2019-09-20  0:07                                 ` Guo Ren
2019-09-20  7:18                                   ` Jean-Philippe Brucker
2019-09-14 14:01                       ` Palmer Dabbelt
2019-09-15  5:03                         ` Anup Patel
2019-09-16 18:18                           ` Will Deacon
2019-09-16 18:28                             ` Palmer Dabbelt
2019-09-17  3:42                             ` Anup Patel
2019-09-19 13:36                               ` Guo Ren
2019-06-19 11:51         ` Guo Ren
2019-06-19 12:52           ` Julien Grall
2019-06-21 14:16           ` Catalin Marinas
2019-06-23 16:35             ` Guo Ren
2019-06-24 10:22               ` Will Deacon
2019-06-27  9:41                 ` qi.fuli
2019-06-27 10:26                   ` Will Deacon
2019-06-24 15:38               ` Catalin Marinas
2019-06-30  4:29                 ` Guo Ren
2019-07-01  9:17                   ` Catalin Marinas
2019-07-16  3:31                     ` Guo Ren
2019-07-22 16:38                       ` Catalin Marinas
2019-03-21 16:36 ` [PATCH RFC 12/14] arm64/lib: asid: Allow user to update the context under the lock Julien Grall
2019-03-21 16:36 ` [PATCH RFC 13/14] arm/kvm: Introduce a new VMID allocator Julien Grall
2019-03-21 16:36 ` [PATCH RFC 14/14] kvm/arm: Align the VMID allocation with the arm64 ASID one Julien Grall

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:
  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=CAJF2gTSiiiewTLwVAXvPLO7rTSUw1rg8VtFLzANdP2S2EEbTjg@mail.gmail.com \
    --to=guoren@kernel.org \
    --cc=Atish.Patra@wdc.com \
    --cc=anup.Patel@wdc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=gary@garyguo.net \
    --cc=hch@infradead.org \
    --cc=julien.grall@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=palmer@sifive.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rppt@linux.ibm.com \
    --cc=will.deacon@arm.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

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:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


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