All of lore.kernel.org
 help / color / mirror / Atom feed
From: anup@brainfault.org (Anup Patel)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Date: Fri, 30 Aug 2013 16:06:30 +0530	[thread overview]
Message-ID: <CAAhSdy2gsJHjEsB01v+jJwhY6pt6Y2m9L+kdw45P3NX0aYVLDg@mail.gmail.com> (raw)
In-Reply-To: <20130830094443.GB62188@MacBook-Pro.local>

On Fri, Aug 30, 2013 at 3:14 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Thu, Aug 29, 2013 at 05:02:50PM +0100, Anup Patel wrote:
>> On Thu, Aug 29, 2013 at 6:23 PM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > On Thu, Aug 29, 2013 at 01:31:43PM +0100, Anup Patel wrote:
>> >> On Thu, Aug 29, 2013 at 4:22 PM, Catalin Marinas
>> >> <catalin.marinas@arm.com> wrote:
>> >> > On Fri, Aug 16, 2013 at 07:57:55AM +0100, Anup Patel wrote:
>> >> >> The approach of flushing d-cache by set/way upon first run of VCPU will
>> >> >> not work because for set/way operations ARM ARM says: "For set/way
>> >> >> operations, and for All (entire cache) operations, the point is defined to be
>> >> >> to the next level of caching". In other words, set/way operations work upto
>> >> >> point of unification.
>> >> >
>> >> > I don't understand where you got the idea that set/way operations work
>> >> > up to the point of unification. This is incorrect, the set/way
>> >> > operations work on the level of cache specified by bits 3:1 in the
>> >> > register passed to the DC CISW instruction. For your L3 cache, those
>> >> > bits would be 2 (and __flush_dcache_all() implementation does this
>> >> > dynamically).
>> >>
>> >> The L3-cache is not visible to CPU. It is totally independent and transparent
>> >> to CPU.
>> >
>> > OK. But you say that operations like DC CIVAC actually flush the L3? So
>> > I don't see it as completely transparent to the CPU.
>>
>> It is transparent from CPU perspective. In other words, there is nothing in
>> CPU for controlling/monitoring L3-cache.
>
> We probably have a different understanding of "transparent". It doesn't
> look to me like any more transparent than the L1 or L2 cache. Basically,
> from a software perspective, it needs maintenance. Whether the CPU
> explicitly asks the L3 cache for this or the L3 cache figures it on its
> own based on the L1/L2 operations is irrelevant.

Yes, we have a different understanding regarding "transparent". The goal
behind our L3 is to have external cache such that CPU is not aware of it
and CPU would work fine even if L3 is turned-off on-the-fly. Further such
L3 has to be smarter than normal outer-caches because it will be
maintained automatically by observing CPU operations.

>
> It would have been transparent if the software didn't need to know about
> it at all, but it's not the case.

Currently, KVM is the only place where we need L3 maintenance because
Guest starts with MMU turned-off otherwise we don't need L3 maintenance
in kernel for any kind of IO or Subsystem.

>
>> > Do you have any configuration bits which would make the L3 completely
>> > transparent like always caching even when accesses are non-cacheable and
>> > DC ops to PoC ignoring it?
>>
>> Actually, L3-cache monitors the types of read/write generated by CPU (i.e.
>> whether the request is cacheable/non-cacheable or whether the request is
>> due to DC ops to PoC, or ...).
>>
>> To answer your query, there is no configuration to have L3 caching when
>> accesses are non-cacheable and DC ops to PoC.
>
> So it's an outer cache with some "improvements" to handle DC ops to PoC.
> I think it was a pretty bad decision on the hardware side as we really
> try to get rid of outer caches for many reasons:

Getting rid off outer-caches (such as in this context) may make sense for
Embedded/Low-end systems but for Servers L3-cache makes lot of sense.

Claiming this to be a bad decision all depends on what end application
you are looking at.

>
> 1. Non-standard cache flushing operations (MMIO-based)
> 2. It may require cache maintenance by physical address - something
>    hard to get in a guest OS (unless you virtualise L3 cache
>    maintenance)

We don't have cache maintenance by physical address in our L3-cache.

> 3. Are barriers like DSB propagated correctly? Does a DC op to PoC
>    followed by DSB ensure that the L3 drained the cachelines to RAM?

Yes, DSB works perfectly fine for us with L3.
Yes, DC op to PoC followed by DSB works fine with L3 too.

>
> I think point 2 isn't required because your L3 detects DC ops to PoC. I
> hope point 3 is handled correctly (otherwise look how "nice" the mb()
> macro on arm is to cope with L2x0).
>
> If only 1 is left, we don't need the full outer_cache framework but it
> still needs to be addressed since the assumption is that flush_cache_all
> (or __flush_dcache_all) flushes all cache levels. These are not used in
> generic code but are used during kernel booting, KVM and cpuidle
> drivers.
>
>> > Now, back to the idea of outer_cache framework for arm64. Does your CPU
>> > have separate instructions for flushing this L3 cache?
>>
>> No, CPU does not have separate instruction for flushing L3-cache. On the
>> other hand, L3-cache has MMIO registers which can be use to explicitly
>> flush L3-cache.
>
> I guess you use those in your firmware or boot loader since Linux
> requires clean/invalidated caches at boot (and I plan to push a patch
> which removes kernel D-cache cleaning during boot to spot such problems
> early). A cpuidle driver would probably need this as well.

Yes, cpuidle would be another place where we may need L3-cache
maintenance. In other words, if we need to power-off L3-cache from
kernel then we need to first flush it.

>
> --
> Catalin

--Anup

  reply	other threads:[~2013-08-30 10:36 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-14 11:47 [PATCH] ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache Pranavkumar Sawargaonkar
2013-08-14 12:04 ` Marc Zyngier
2013-08-14 14:22   ` Anup Patel
2013-08-14 15:06     ` Alexander Graf
2013-08-14 15:34       ` Marc Zyngier
2013-08-14 15:41         ` Peter Maydell
2013-08-14 15:57           ` Marc Zyngier
2013-08-14 16:36             ` Anup Patel
2013-08-14 15:23     ` Marc Zyngier
2013-08-14 15:35       ` Peter Maydell
2013-08-14 15:49         ` Marc Zyngier
2013-08-14 17:34           ` Christoffer Dall
2013-08-15  4:44             ` Marc Zyngier
2013-08-15 16:58               ` Christoffer Dall
2013-08-14 15:36       ` Anup Patel
2013-08-15  4:52     ` Marc Zyngier
2013-08-15  6:26       ` Anup Patel
2013-08-15  8:31         ` Marc Zyngier
2013-08-15 13:31           ` Anup Patel
2013-08-15 14:47             ` Marc Zyngier
2013-08-15 15:13               ` Anup Patel
2013-08-15 15:37                 ` Marc Zyngier
2013-08-15 15:45                   ` Anup Patel
2013-08-15 16:53                   ` Christoffer Dall
2013-08-16  5:02                     ` Anup Patel
2013-08-16  6:57                       ` Anup Patel
2013-08-16 17:19                         ` Christoffer Dall
2013-08-16 17:42                           ` Anup Patel
2013-08-16 17:50                             ` Christoffer Dall
2013-08-16 18:06                               ` Christoffer Dall
2013-08-16 18:20                                 ` Anup Patel
2013-08-16 18:11                               ` Anup Patel
2013-08-16 18:20                                 ` Christoffer Dall
2013-08-30  9:52                                 ` Catalin Marinas
2013-08-30 10:44                                   ` Anup Patel
2013-08-30 13:02                                     ` Catalin Marinas
2013-08-30 13:21                                     ` Marc Zyngier
2013-08-30 14:04                                       ` Catalin Marinas
2013-08-30 14:22                                         ` Marc Zyngier
2013-08-30 14:30                                           ` Will Deacon
2013-08-30 14:52                                             ` Anup Patel
2013-08-30 15:12                                             ` Marc Zyngier
2013-08-29 10:52                         ` Catalin Marinas
2013-08-29 12:31                           ` Anup Patel
2013-08-29 12:53                             ` Catalin Marinas
2013-08-29 16:02                               ` Anup Patel
2013-08-30  9:44                                 ` Catalin Marinas
2013-08-30 10:36                                   ` Anup Patel [this message]
2013-08-30 12:52                                     ` Catalin Marinas
2013-08-16 17:14                       ` Christoffer Dall
     [not found]                         ` <CALrVBkvEP1Q0mKpv8ViOTLRvW2ks18MQXgmurSBHn+aJcz+=gw@mail.gmail.com>
2013-08-16 17:28                           ` Christoffer Dall
2013-08-16 17:42                             ` Christoffer Dall
2013-08-15  8:39       ` Pranavkumar Sawargaonkar
2013-08-15 15:42         ` Marc Zyngier
2013-08-14 14:23 ` Sudeep KarkadaNagesha
2013-08-14 14:35   ` Anup Patel
2013-08-14 17:37 ` Christoffer Dall
  -- strict thread matches above, loose matches on Subject: below --
2013-08-14 11:45 Pranavkumar Sawargaonkar

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=CAAhSdy2gsJHjEsB01v+jJwhY6pt6Y2m9L+kdw45P3NX0aYVLDg@mail.gmail.com \
    --to=anup@brainfault.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.