linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot
@ 2023-11-30  9:07 Shaoqin Huang
  2023-11-30  9:07 ` [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files Shaoqin Huang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Shaoqin Huang @ 2023-11-30  9:07 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, linuxppc-dev,
	Shaoqin Huang, Nikos Nikoleris, Eric Auger, Nadav Amit,
	Alexandru Elisei, David Woodhouse

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  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 ` Shaoqin Huang
  2024-01-15 12:44   ` 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
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Shaoqin Huang @ 2023-11-30  9:07 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, linuxppc-dev,
	Shaoqin Huang, Nikos Nikoleris, Eric Auger, Nadav Amit,
	Alexandru Elisei, David Woodhouse

From: Alexandru Elisei <alexandru.elisei@arm.com>

There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
with functionality relies on the __ASSEMBLY__ prepocessor constant being
correctly defined to work correctly. So far, kvm-unit-tests has relied on
the assembly files to define the constant before including any header
files which depend on it.

Let's make sure that nobody gets this wrong and define it as a compiler
constant when compiling assembly files. __ASSEMBLY__ is now defined for all
.S files, even those that didn't set it explicitely before.

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 Makefile           | 5 ++++-
 arm/cstart.S       | 1 -
 arm/cstart64.S     | 1 -
 powerpc/cstart64.S | 1 -
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 602910dd..27ed14e6 100644
--- a/Makefile
+++ b/Makefile
@@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
 
 autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
 
+AFLAGS  = $(CFLAGS)
+AFLAGS += -D__ASSEMBLY__
+
 LDFLAGS += -nostdlib $(no_pie) -z noexecstack
 
 $(libcflat): $(cflatobjs)
@@ -113,7 +116,7 @@ directories:
 	@mkdir -p $(OBJDIRS)
 
 %.o: %.S
-	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<
 
 -include */.*.d */*/.*.d
 
diff --git a/arm/cstart.S b/arm/cstart.S
index 3dd71ed9..b24ecabc 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -5,7 +5,6 @@
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
-#define __ASSEMBLY__
 #include <auxinfo.h>
 #include <asm/assembler.h>
 #include <asm/thread_info.h>
diff --git a/arm/cstart64.S b/arm/cstart64.S
index bc2be45a..a8ad6dc8 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -5,7 +5,6 @@
  *
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
-#define __ASSEMBLY__
 #include <auxinfo.h>
 #include <asm/asm-offsets.h>
 #include <asm/assembler.h>
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 34e39341..fa32ef24 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -5,7 +5,6 @@
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
-#define __ASSEMBLY__
 #include <asm/hcall.h>
 #include <asm/ppc_asm.h>
 #include <asm/rtas.h>
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [kvm-unit-tests PATCH v1 02/18] powerpc: Replace the physical allocator with the page allocator
  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
@ 2023-11-30  9:07 ` Shaoqin Huang
  2023-11-30 10:35 ` [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot Alexandru Elisei
  2024-02-16 15:47 ` Alexandru Elisei
  3 siblings, 0 replies; 11+ messages in thread
From: Shaoqin Huang @ 2023-11-30  9:07 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Laurent Vivier, Thomas Huth, kvm, Alexandru Elisei, kvm-ppc,
	linuxppc-dev

From: Alexandru Elisei <alexandru.elisei@arm.com>

The spapr_hcall test makes two page sized allocations using the physical
allocator. Replace the physical allocator with the page allocator, which
has has more features (like support for freeing allocations), and would
allow for further simplification of the physical allocator.

CC: Laurent Vivier <lvivier@redhat.com>
CC: Thomas Huth <thuth@redhat.com>
CC: kvm-ppc@vger.kernel.org
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/powerpc/setup.c     | 9 ++++++---
 powerpc/Makefile.common | 1 +
 powerpc/spapr_hcall.c   | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 1be4c030..80fd38ae 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -15,6 +15,7 @@
 #include <devicetree.h>
 #include <alloc.h>
 #include <alloc_phys.h>
+#include <alloc_page.h>
 #include <argv.h>
 #include <asm/setup.h>
 #include <asm/page.h>
@@ -111,6 +112,7 @@ static void mem_init(phys_addr_t freemem_start)
 	struct mem_region primary, mem = {
 		.start = (phys_addr_t)-1,
 	};
+	phys_addr_t base, top;
 	int nr_regs, i;
 
 	nr_regs = dt_get_memory_params(regs, NR_MEM_REGIONS);
@@ -146,9 +148,10 @@ static void mem_init(phys_addr_t freemem_start)
 	__physical_start = mem.start;	/* PHYSICAL_START */
 	__physical_end = mem.end;	/* PHYSICAL_END */
 
-	phys_alloc_init(freemem_start, primary.end - freemem_start);
-	phys_alloc_set_minimum_alignment(__icache_bytes > __dcache_bytes
-					 ? __icache_bytes : __dcache_bytes);
+	base = PAGE_ALIGN(freemem_start) >> PAGE_SHIFT;
+	top = primary.end >> PAGE_SHIFT;
+	page_alloc_init_area(0, base, top);
+	page_alloc_ops_enable();
 }
 
 void setup(const void *fdt)
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index f8f47490..ae70443a 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -34,6 +34,7 @@ include $(SRCDIR)/scripts/asm-offsets.mak
 cflatobjs += lib/util.o
 cflatobjs += lib/getchar.o
 cflatobjs += lib/alloc_phys.o
+cflatobjs += lib/alloc_page.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
 cflatobjs += lib/migrate.o
diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
index e9b5300a..77ab4187 100644
--- a/powerpc/spapr_hcall.c
+++ b/powerpc/spapr_hcall.c
@@ -8,6 +8,7 @@
 #include <libcflat.h>
 #include <util.h>
 #include <alloc.h>
+#include <alloc_page.h>
 #include <asm/hcall.h>
 #include <asm/processor.h>
 
@@ -58,8 +59,8 @@ static void test_h_page_init(int argc, char **argv)
 	if (argc > 1)
 		report_abort("Unsupported argument: '%s'", argv[1]);
 
-	dst = memalign(PAGE_SIZE, PAGE_SIZE);
-	src = memalign(PAGE_SIZE, PAGE_SIZE);
+	dst = alloc_page();
+	src = alloc_page();
 	if (!dst || !src)
 		report_abort("Failed to alloc memory");
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot
  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
  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 10:35 ` Alexandru Elisei
  2023-12-01  8:40   ` Shaoqin Huang
  2024-02-16 15:47 ` Alexandru Elisei
  3 siblings, 1 reply; 11+ messages in thread
From: Alexandru Elisei @ 2023-11-30 10:35 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Andrew Jones,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

Hi,

Thank you so much for reviving this, much appreciated.

I wanted to let you know that I definitely plan to review the series as
soon as possible, unfortunately I don't believe I won't be able to do that
for at least 2 weeks.

Thanks,
Alex

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot
  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
  0 siblings, 0 replies; 11+ messages in thread
From: Shaoqin Huang @ 2023-12-01  8:40 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Andrew Jones,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

Hi Alexandru,

Just take your time. I also appreciate your work. :)

Thanks,
Shaoqin

On 11/30/23 18:35, Alexandru Elisei wrote:
> Hi,
> 
> Thank you so much for reviving this, much appreciated.
> 
> I wanted to let you know that I definitely plan to review the series as
> soon as possible, unfortunately I don't believe I won't be able to do that
> for at least 2 weeks.
> 
> Thanks,
> Alex
> 
> 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 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
>>
> 

-- 
Shaoqin


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  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
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Jones @ 2024-01-15 12:44 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, linuxppc-dev,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, Alexandru Elisei,
	David Woodhouse

On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote:
> From: Alexandru Elisei <alexandru.elisei@arm.com>
> 
> There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
> with functionality relies on the __ASSEMBLY__ prepocessor constant being
> correctly defined to work correctly. So far, kvm-unit-tests has relied on
> the assembly files to define the constant before including any header
> files which depend on it.
> 
> Let's make sure that nobody gets this wrong and define it as a compiler
> constant when compiling assembly files. __ASSEMBLY__ is now defined for all
> .S files, even those that didn't set it explicitely before.
> 
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  Makefile           | 5 ++++-
>  arm/cstart.S       | 1 -
>  arm/cstart64.S     | 1 -
>  powerpc/cstart64.S | 1 -
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 602910dd..27ed14e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
>  
>  autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
>  
> +AFLAGS  = $(CFLAGS)
> +AFLAGS += -D__ASSEMBLY__
> +
>  LDFLAGS += -nostdlib $(no_pie) -z noexecstack
>  
>  $(libcflat): $(cflatobjs)
> @@ -113,7 +116,7 @@ directories:
>  	@mkdir -p $(OBJDIRS)
>  
>  %.o: %.S
> -	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
> +	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<

I think we can drop the two hunks above from this patch and just rely on
the compiler to add __ASSEMBLY__ for us when compiling assembly files.

Thanks,
drew

>  
>  -include */.*.d */*/.*.d
>  
> diff --git a/arm/cstart.S b/arm/cstart.S
> index 3dd71ed9..b24ecabc 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -5,7 +5,6 @@
>   *
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
> -#define __ASSEMBLY__
>  #include <auxinfo.h>
>  #include <asm/assembler.h>
>  #include <asm/thread_info.h>
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index bc2be45a..a8ad6dc8 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -5,7 +5,6 @@
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2.
>   */
> -#define __ASSEMBLY__
>  #include <auxinfo.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/assembler.h>
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index 34e39341..fa32ef24 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -5,7 +5,6 @@
>   *
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
> -#define __ASSEMBLY__
>  #include <asm/hcall.h>
>  #include <asm/ppc_asm.h>
>  #include <asm/rtas.h>
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  2024-01-15 12:44   ` Andrew Jones
@ 2024-02-15 16:05     ` Alexandru Elisei
  2024-02-15 16:32       ` Andrew Jones
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandru Elisei @ 2024-02-15 16:05 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Shaoqin Huang,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

Hi Drew,

On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote:
> On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote:
> > From: Alexandru Elisei <alexandru.elisei@arm.com>
> > 
> > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
> > with functionality relies on the __ASSEMBLY__ prepocessor constant being
> > correctly defined to work correctly. So far, kvm-unit-tests has relied on
> > the assembly files to define the constant before including any header
> > files which depend on it.
> > 
> > Let's make sure that nobody gets this wrong and define it as a compiler
> > constant when compiling assembly files. __ASSEMBLY__ is now defined for all
> > .S files, even those that didn't set it explicitely before.
> > 
> > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> > ---
> >  Makefile           | 5 ++++-
> >  arm/cstart.S       | 1 -
> >  arm/cstart64.S     | 1 -
> >  powerpc/cstart64.S | 1 -
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 602910dd..27ed14e6 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
> >  
> >  autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
> >  
> > +AFLAGS  = $(CFLAGS)
> > +AFLAGS += -D__ASSEMBLY__
> > +
> >  LDFLAGS += -nostdlib $(no_pie) -z noexecstack
> >  
> >  $(libcflat): $(cflatobjs)
> > @@ -113,7 +116,7 @@ directories:
> >  	@mkdir -p $(OBJDIRS)
> >  
> >  %.o: %.S
> > -	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
> > +	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<
> 
> I think we can drop the two hunks above from this patch and just rely on
> the compiler to add __ASSEMBLY__ for us when compiling assembly files.

I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I
missing something?

[1] https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__

Thanks,
Alex

> 
> Thanks,
> drew
> 
> >  
> >  -include */.*.d */*/.*.d
> >  
> > diff --git a/arm/cstart.S b/arm/cstart.S
> > index 3dd71ed9..b24ecabc 100644
> > --- a/arm/cstart.S
> > +++ b/arm/cstart.S
> > @@ -5,7 +5,6 @@
> >   *
> >   * This work is licensed under the terms of the GNU LGPL, version 2.
> >   */
> > -#define __ASSEMBLY__
> >  #include <auxinfo.h>
> >  #include <asm/assembler.h>
> >  #include <asm/thread_info.h>
> > diff --git a/arm/cstart64.S b/arm/cstart64.S
> > index bc2be45a..a8ad6dc8 100644
> > --- a/arm/cstart64.S
> > +++ b/arm/cstart64.S
> > @@ -5,7 +5,6 @@
> >   *
> >   * This work is licensed under the terms of the GNU GPL, version 2.
> >   */
> > -#define __ASSEMBLY__
> >  #include <auxinfo.h>
> >  #include <asm/asm-offsets.h>
> >  #include <asm/assembler.h>
> > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> > index 34e39341..fa32ef24 100644
> > --- a/powerpc/cstart64.S
> > +++ b/powerpc/cstart64.S
> > @@ -5,7 +5,6 @@
> >   *
> >   * This work is licensed under the terms of the GNU LGPL, version 2.
> >   */
> > -#define __ASSEMBLY__
> >  #include <asm/hcall.h>
> >  #include <asm/ppc_asm.h>
> >  #include <asm/rtas.h>
> > -- 
> > 2.40.1
> > 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  2024-02-15 16:05     ` Alexandru Elisei
@ 2024-02-15 16:32       ` Andrew Jones
  2024-02-15 17:16         ` Alexandru Elisei
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Jones @ 2024-02-15 16:32 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Shaoqin Huang,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

On Thu, Feb 15, 2024 at 04:05:56PM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote:
> > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote:
> > > From: Alexandru Elisei <alexandru.elisei@arm.com>
> > > 
> > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
> > > with functionality relies on the __ASSEMBLY__ prepocessor constant being
> > > correctly defined to work correctly. So far, kvm-unit-tests has relied on
> > > the assembly files to define the constant before including any header
> > > files which depend on it.
> > > 
> > > Let's make sure that nobody gets this wrong and define it as a compiler
> > > constant when compiling assembly files. __ASSEMBLY__ is now defined for all
> > > .S files, even those that didn't set it explicitely before.
> > > 
> > > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> > > ---
> > >  Makefile           | 5 ++++-
> > >  arm/cstart.S       | 1 -
> > >  arm/cstart64.S     | 1 -
> > >  powerpc/cstart64.S | 1 -
> > >  4 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index 602910dd..27ed14e6 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
> > >  
> > >  autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
> > >  
> > > +AFLAGS  = $(CFLAGS)
> > > +AFLAGS += -D__ASSEMBLY__
> > > +
> > >  LDFLAGS += -nostdlib $(no_pie) -z noexecstack
> > >  
> > >  $(libcflat): $(cflatobjs)
> > > @@ -113,7 +116,7 @@ directories:
> > >  	@mkdir -p $(OBJDIRS)
> > >  
> > >  %.o: %.S
> > > -	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
> > > +	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<
> > 
> > I think we can drop the two hunks above from this patch and just rely on
> > the compiler to add __ASSEMBLY__ for us when compiling assembly files.
> 
> I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I
> missing something?
> 
> [1] https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__

You're right. I'm not opposed to changing all the __ASSEMBLY__ references
to __ASSEMBLER__. I'll try to do that at some point unless you beat me to
it.

Thanks,
drew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  2024-02-15 16:32       ` Andrew Jones
@ 2024-02-15 17:16         ` Alexandru Elisei
  2024-02-15 19:13           ` Andrew Jones
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandru Elisei @ 2024-02-15 17:16 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Shaoqin Huang,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

Hi Drew,

On Thu, Feb 15, 2024 at 05:32:22PM +0100, Andrew Jones wrote:
> On Thu, Feb 15, 2024 at 04:05:56PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote:
> > > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote:
> > > > From: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > 
> > > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
> > > > with functionality relies on the __ASSEMBLY__ prepocessor constant being
> > > > correctly defined to work correctly. So far, kvm-unit-tests has relied on
> > > > the assembly files to define the constant before including any header
> > > > files which depend on it.
> > > > 
> > > > Let's make sure that nobody gets this wrong and define it as a compiler
> > > > constant when compiling assembly files. __ASSEMBLY__ is now defined for all
> > > > .S files, even those that didn't set it explicitely before.
> > > > 
> > > > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > > > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> > > > ---
> > > >  Makefile           | 5 ++++-
> > > >  arm/cstart.S       | 1 -
> > > >  arm/cstart64.S     | 1 -
> > > >  powerpc/cstart64.S | 1 -
> > > >  4 files changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index 602910dd..27ed14e6 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
> > > >  
> > > >  autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
> > > >  
> > > > +AFLAGS  = $(CFLAGS)
> > > > +AFLAGS += -D__ASSEMBLY__
> > > > +
> > > >  LDFLAGS += -nostdlib $(no_pie) -z noexecstack
> > > >  
> > > >  $(libcflat): $(cflatobjs)
> > > > @@ -113,7 +116,7 @@ directories:
> > > >  	@mkdir -p $(OBJDIRS)
> > > >  
> > > >  %.o: %.S
> > > > -	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
> > > > +	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<
> > > 
> > > I think we can drop the two hunks above from this patch and just rely on
> > > the compiler to add __ASSEMBLY__ for us when compiling assembly files.
> > 
> > I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I
> > missing something?
> > 
> > [1] https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__
> 
> You're right. I'm not opposed to changing all the __ASSEMBLY__ references
> to __ASSEMBLER__. I'll try to do that at some point unless you beat me to
> it.

Actually, I quite prefer the Linux style of using __ASSEMBLY__ instead of
__ASSEMBLER__, because it makes reusing Linux files easier. That, and the
habit formed by staring at Linux assembly files.

Thanks,
Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
  2024-02-15 17:16         ` Alexandru Elisei
@ 2024-02-15 19:13           ` Andrew Jones
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2024-02-15 19:13 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Shaoqin Huang,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

On Thu, Feb 15, 2024 at 05:16:01PM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Thu, Feb 15, 2024 at 05:32:22PM +0100, Andrew Jones wrote:
> > On Thu, Feb 15, 2024 at 04:05:56PM +0000, Alexandru Elisei wrote:
> > > Hi Drew,
> > > 
> > > On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote:
> > > > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote:
> > > > > From: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > > 
> > > > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__)
> > > > > with functionality relies on the __ASSEMBLY__ prepocessor constant being
> > > > > correctly defined to work correctly. So far, kvm-unit-tests has relied on
> > > > > the assembly files to define the constant before including any header
> > > > > files which depend on it.
> > > > > 
> > > > > Let's make sure that nobody gets this wrong and define it as a compiler
> > > > > constant when compiling assembly files. __ASSEMBLY__ is now defined for all
> > > > > .S files, even those that didn't set it explicitely before.
> > > > > 
> > > > > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > > > > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > > Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> > > > > ---
> > > > >  Makefile           | 5 ++++-
> > > > >  arm/cstart.S       | 1 -
> > > > >  arm/cstart64.S     | 1 -
> > > > >  powerpc/cstart64.S | 1 -
> > > > >  4 files changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 602910dd..27ed14e6 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
> > > > >  
> > > > >  autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
> > > > >  
> > > > > +AFLAGS  = $(CFLAGS)
> > > > > +AFLAGS += -D__ASSEMBLY__
> > > > > +
> > > > >  LDFLAGS += -nostdlib $(no_pie) -z noexecstack
> > > > >  
> > > > >  $(libcflat): $(cflatobjs)
> > > > > @@ -113,7 +116,7 @@ directories:
> > > > >  	@mkdir -p $(OBJDIRS)
> > > > >  
> > > > >  %.o: %.S
> > > > > -	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
> > > > > +	$(CC) $(AFLAGS) -c -nostdlib -o $@ $<
> > > > 
> > > > I think we can drop the two hunks above from this patch and just rely on
> > > > the compiler to add __ASSEMBLY__ for us when compiling assembly files.
> > > 
> > > I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I
> > > missing something?
> > > 
> > > [1] https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__
> > 
> > You're right. I'm not opposed to changing all the __ASSEMBLY__ references
> > to __ASSEMBLER__. I'll try to do that at some point unless you beat me to
> > it.
> 
> Actually, I quite prefer the Linux style of using __ASSEMBLY__ instead of
> __ASSEMBLER__, because it makes reusing Linux files easier. That, and the
> habit formed by staring at Linux assembly files.

Those are good arguments and also saves the churn. OK, let's keep this
patch and __ASSEMBLY__

Thanks,
drew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot
  2023-11-30  9:07 [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot Shaoqin Huang
                   ` (2 preceding siblings ...)
  2023-11-30 10:35 ` [kvm-unit-tests PATCH v1 00/18] arm/arm64: Rework cache maintenance at boot Alexandru Elisei
@ 2024-02-16 15:47 ` Alexandru Elisei
  3 siblings, 0 replies; 11+ messages in thread
From: Alexandru Elisei @ 2024-02-16 15:47 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: Laurent Vivier, Thomas Huth, Nico Boehr, kvm, Andrew Jones,
	Nikos Nikoleris, Eric Auger, Nadav Amit, kvmarm, linuxppc-dev,
	David Woodhouse

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-02-16 15:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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 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).