From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Shaoqin Huang <shahuang@redhat.com>
Cc: Andrew Jones <andrew.jones@linux.dev>,
kvmarm@lists.linux.dev, David Woodhouse <dwmw@amazon.co.uk>,
Eric Auger <eric.auger@redhat.com>,
kvm@vger.kernel.org, Laurent Vivier <lvivier@redhat.com>,
linuxppc-dev@lists.ozlabs.org, Nadav Amit <namit@vmware.com>,
Nico Boehr <nrb@linux.ibm.com>,
Nikos Nikoleris <nikos.nikoleris@arm.com>,
Thomas Huth <thuth@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot
Date: Fri, 16 Feb 2024 15:47:41 +0000 [thread overview]
Message-ID: <Zc-DnSvmJbT4Lk73@raptor> (raw)
In-Reply-To: <20231130090722.2897974-1-shahuang@redhat.com>
Hi,
On Thu, Nov 30, 2023 at 04:07:02AM -0500, Shaoqin Huang wrote:
> Hi,
>
> I'm posting Alexandru's patch set[1] rebased on the latest branch with the
> conflicts being resolved. No big changes compare to its original code.
>
> As this version 1 of this series was posted one years ago, I would first let you
> recall it, what's the intention of this series and what this series do. You can
> view it by click the link[2] and view the cover-letter.
>
> Since when writing the series[1], the efi support for arm64[3] hasn't been
> merged into the kvm-unit-tests, but now the efi support for arm64 has been
> merged. Directly rebase the series[1] onto the latest branch will break the efi
> tests. This is mainly because the Patch #15 ("arm/arm64: Enable the MMU early")
> moves the mmu_enable() out of the setup_mmu(), which causes the efi test will
> not enable the mmu. So I do a small change in the efi_mem_init() which makes the
> efi test also enable the MMU early, and make it works.
>
> And another change should be noticed is in the Patch #17 ("arm/arm64: Perform
> dcache maintenance"). In the efi_mem_init(), it will disable the mmu, and build
> a new pagetable and re-enable the mmu, if the asm_mmu_disable clean and
> invalidate the data caches for entire memory, we don't need to care the dcache
> and after mmu disabled, we use the mmu_setup_early() to re-enable the mmu, which
> takes care all the cache maintenance. But the situation changes since the Patch
> #18 ("arm/arm64: Rework the cache maintenance in asm_mmu_disable") only clean
> and invalidate the data caches for the stack memory area. So we need to clean
> and invalidate the data caches manually before disable the mmu, I'm not
> confident about current cache maintenance at the efi setup patch, so I ask for
> your help to review it if it's right or not.
>
> And I also drop one patch ("s390: Do not use the physical allocator") from[1]
> since this cause s390 test to fail.
This is unfortunate. What tests do you see failing?
I wrote the s390x patch so I can justify dropping the locking in the
physical allocator. And I wanted to drop the locking so I wouldn't have to
do maintenance operation on the spinlock.
Because of how kvm-unit-tests implements spinlocks for arm/arm64, they
don't protect against concurrent accesses when the MMU is turned off. And
because arm and arm64 use the physical allocator during the test boot
sequence, not during a test, using a spin lock is also useless since there
will be no concurrent accesses (the boot phase is done on a single CPU).
But since replacing the physical allocator causes test failures for s390x,
looks like the physical will still be needed to tests, and that requires
having the spinlock.
I guess the best approach would be to teach the physical allocator to do
cache maintenance on the spinlock. We might as well, since the UART needs
it too, and I don't think this series addresses that.
Thanks,
Alex
>
> This series may include bug, so I really appreciate your review to improve this
> series together.
>
> You can get the code from:
>
> $ git clone https://gitlab.com/shahuang/kvm-unit-tests.git \
> -b arm-arm64-rework-cache-maintenance-at-boot-v1
>
> [1] https://gitlab.arm.com/linux-arm/kvm-unit-tests-ae/-/tree/arm-arm64-rework-cache-maintenance-at-boot-v2-wip2
> [2] https://lore.kernel.org/all/20220809091558.14379-1-alexandru.elisei@arm.com/
> [3] https://patchwork.kernel.org/project/kvm/cover/20230530160924.82158-1-nikos.nikoleris@arm.com/
>
> Changelog:
> ----------
> RFC->v1:
> - Gathered Reviewed-by tags.
> - Various changes to commit messages and comments to hopefully make the code
> easier to understand.
> - Patches #8 ("lib/alloc_phys: Expand documentation with usage and limitations")
> are new.
> - Folded patch "arm: page.h: Add missing libcflat.h include" into #17
> ("arm/arm64: Perform dcache maintenance at boot").
> - Reordered the series to group patches that touch aproximately the same code
> together - the patches that change the physical allocator are now first,
> followed come the patches that change how the secondaries are brought online.
> - Fixed several nasty bugs where the r4 register was being clobbered in the arm
> assembly.
> - Unmap the early UART address if the DTB address does not match the early
> address.
> - Added dcache maintenance when a page table is modified with the MMU disabled.
> - Moved the cache maintenance when disabling the MMU to be executed before the
> MMU is disabled.
> - Rebase it on lasted branch which efi support has been merged.
> - Make the efi test also enable MMU early.
> - Add cache maintenance on efi setup path especially before mmu_disable.
>
> RFC: https://lore.kernel.org/all/20220809091558.14379-1-alexandru.elisei@arm.com/
>
> Alexandru Elisei (18):
> Makefile: Define __ASSEMBLY__ for assembly files
> powerpc: Replace the physical allocator with the page allocator
> lib/alloc_phys: Initialize align_min
> lib/alloc_phys: Consolidate allocate functions into memalign_early()
> lib/alloc_phys: Remove locking
> lib/alloc_phys: Remove allocation accounting
> lib/alloc_phys: Add callback to perform cache maintenance
> lib/alloc_phys: Expand documentation with usage and limitations
> arm/arm64: Zero secondary CPUs' stack
> arm/arm64: Allocate secondaries' stack using the page allocator
> arm/arm64: assembler.h: Replace size with end address for
> dcache_by_line_op
> arm/arm64: Add C functions for doing cache maintenance
> arm/arm64: Configure secondaries' stack before enabling the MMU
> arm/arm64: Use pgd_alloc() to allocate mmu_idmap
> arm/arm64: Enable the MMU early
> arm/arm64: Map the UART when creating the translation tables
> arm/arm64: Perform dcache maintenance at boot
> arm/arm64: Rework the cache maintenance in asm_mmu_disable
>
> Makefile | 5 +-
> arm/Makefile.arm | 4 +-
> arm/Makefile.arm64 | 4 +-
> arm/Makefile.common | 6 +-
> arm/cstart.S | 71 +++++++++++++++------
> arm/cstart64.S | 76 +++++++++++++++++------
> lib/alloc_phys.c | 122 ++++++++++++-------------------------
> lib/alloc_phys.h | 28 ++++++---
> lib/arm/asm/assembler.h | 15 ++---
> lib/arm/asm/cacheflush.h | 1 +
> lib/arm/asm/mmu-api.h | 1 +
> lib/arm/asm/mmu.h | 6 --
> lib/arm/asm/page.h | 2 +
> lib/arm/asm/pgtable.h | 39 ++++++++++--
> lib/arm/asm/thread_info.h | 3 +-
> lib/arm/cache.S | 89 +++++++++++++++++++++++++++
> lib/arm/io.c | 31 ++++++++++
> lib/arm/io.h | 3 +
> lib/arm/mmu.c | 37 ++++++++---
> lib/arm/processor.c | 1 -
> lib/arm/setup.c | 82 +++++++++++++++++++++----
> lib/arm/smp.c | 5 ++
> lib/arm64/asm/assembler.h | 11 ++--
> lib/arm64/asm/cacheflush.h | 37 +++++++++++
> lib/arm64/asm/mmu.h | 5 --
> lib/arm64/asm/pgtable.h | 50 +++++++++++++--
> lib/arm64/cache.S | 85 ++++++++++++++++++++++++++
> lib/arm64/processor.c | 1 -
> lib/devicetree.c | 2 +-
> lib/powerpc/setup.c | 9 ++-
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 1 -
> powerpc/spapr_hcall.c | 5 +-
> 33 files changed, 642 insertions(+), 196 deletions(-)
> create mode 100644 lib/arm/asm/cacheflush.h
> create mode 100644 lib/arm/cache.S
> create mode 100644 lib/arm64/asm/cacheflush.h
> create mode 100644 lib/arm64/cache.S
>
> --
> 2.40.1
>
prev parent reply other threads:[~2024-02-16 15:47 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-30 9:07 [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files Shaoqin Huang
2024-01-15 12:44 ` Andrew Jones
2024-02-15 16:05 ` Alexandru Elisei
2024-02-15 16:32 ` Andrew Jones
2024-02-15 17:16 ` Alexandru Elisei
2024-02-15 19:13 ` Andrew Jones
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 02/18] powerpc: Replace the physical allocator with the page allocator Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 03/18] lib/alloc_phys: Initialize align_min Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 04/18] lib/alloc_phys: Consolidate allocate functions into memalign_early() Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 05/18] lib/alloc_phys: Remove locking Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 06/18] lib/alloc_phys: Remove allocation accounting Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 07/18] lib/alloc_phys: Add callback to perform cache maintenance Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 08/18] lib/alloc_phys: Expand documentation with usage and limitations Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 09/18] arm/arm64: Zero secondary CPUs' stack Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 10/18] arm/arm64: Allocate secondaries' stack using the page allocator Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 11/18] arm/arm64: assembler.h: Replace size with end address for dcache_by_line_op Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 12/18] arm/arm64: Add C functions for doing cache maintenance Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 13/18] arm/arm64: Configure secondaries' stack before enabling the MMU Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 14/18] arm/arm64: Use pgd_alloc() to allocate mmu_idmap Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 15/18] arm/arm64: Enable the MMU early Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 16/18] arm/arm64: Map the UART when creating the translation tables Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 17/18] arm/arm64: Perform dcache maintenance at boot Shaoqin Huang
2023-11-30 9:07 ` [kvm-unit-tests PATCH v1 18/18] arm/arm64: Rework the cache maintenance in asm_mmu_disable Shaoqin Huang
2023-11-30 10:35 ` [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot Alexandru Elisei
2023-12-01 8:40 ` Shaoqin Huang
2024-02-16 15:47 ` Alexandru Elisei [this message]
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=Zc-DnSvmJbT4Lk73@raptor \
--to=alexandru.elisei@arm.com \
--cc=andrew.jones@linux.dev \
--cc=dwmw@amazon.co.uk \
--cc=eric.auger@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=lvivier@redhat.com \
--cc=namit@vmware.com \
--cc=nikos.nikoleris@arm.com \
--cc=nrb@linux.ibm.com \
--cc=shahuang@redhat.com \
--cc=thuth@redhat.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 read-only IMAP folder(s) and NNTP newsgroup(s).