[PATCH 00/11] riscv: Memory type control for platforms with physical memory aliases

[PATCH 00/11] riscv: Memory type control for platforms with physical memory aliases

[Samuel Holland]

On some RISC-V platforms, including StarFive JH7100 and ESWIN EIC7700,
RAM is mapped to multiple physical address ranges, with each alias
having a different set of statically-determined Physical Memory
Attributes (PMAs). Software selects the PMAs for a page by choosing a
PFN from the corresponding physical address range. On these platforms,
this is the only way to allocate noncached memory for use with
noncoherent DMA.

 - Patch 1 adds a new binding to describe physical memory regions in
   the devicetree.
 - Patches 2-6 refactor existing memory type support to be modeled as
   variants on top of Svpbmt.
 - Patches 7-10 add logic to transform the PFN to use the desired alias
   when reading/writing page tables.
 - Patch 11 enables this new method of memory type control on JH7100.

I have boot-tested this series on platforms with each of the 4 ways to
select a memory type: SiFive FU740 (none), QEMU (Svpbmt), Allwinner D1
(XTheadMae), and ESWIN EIC7700 (aliases).


Samuel Holland (11):
  dt-bindings: riscv: Describe physical memory regions
  riscv: mm: Increment PFN in place when splitting mappings
  riscv: mm: Deduplicate pgtable address conversion functions
  riscv: mm: Deduplicate _PAGE_CHG_MASK definition
  riscv: ptdump: Only show N and MT bits when enabled in the kernel
  riscv: mm: Fix up memory types when writing page tables
  riscv: mm: Expose all page table bits to assembly code
  riscv: alternative: Add an ALTERNATIVE_3 macro
  riscv: alternative: Allow calls with alternate link registers
  riscv: mm: Use physical memory aliases to apply PMAs
  riscv: dts: starfive: jh7100: Use physical memory ranges for DMA

 .../bindings/riscv/physical-memory.yaml       | 101 ++++++++++
 arch/riscv/Kconfig                            |   3 +
 arch/riscv/Kconfig.errata                     |  20 +-
 .../boot/dts/starfive/jh7100-common.dtsi      |  30 +--
 arch/riscv/include/asm/alternative-macros.h   |  45 ++++-
 arch/riscv/include/asm/errata_list.h          |  45 -----
 arch/riscv/include/asm/hwcap.h                |   1 +
 arch/riscv/include/asm/pgtable-32.h           |  19 +-
 arch/riscv/include/asm/pgtable-64.h           | 178 ++++++++++--------
 arch/riscv/include/asm/pgtable-bits.h         |  42 ++++-
 arch/riscv/include/asm/pgtable.h              |  55 +++---
 arch/riscv/kernel/alternative.c               |   4 +-
 arch/riscv/kernel/cpufeature.c                |   6 +
 arch/riscv/kernel/setup.c                     |   1 +
 arch/riscv/mm/Makefile                        |   1 +
 arch/riscv/mm/init.c                          |   8 +-
 arch/riscv/mm/kasan_init.c                    |   8 +-
 arch/riscv/mm/memory-alias.S                  | 101 ++++++++++
 arch/riscv/mm/pageattr.c                      |  17 +-
 arch/riscv/mm/pgtable.c                       |  91 +++++++++
 arch/riscv/mm/ptdump.c                        |  19 +-
 include/dt-bindings/riscv/physical-memory.h   |  44 +++++
 22 files changed, 596 insertions(+), 243 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/riscv/physical-memory.yaml
 create mode 100644 arch/riscv/mm/memory-alias.S
 create mode 100644 include/dt-bindings/riscv/physical-memory.h

-- 
2.45.1

[PATCH 01/11] dt-bindings: riscv: Describe physical memory regions

[Samuel Holland]

Information about physical memory regions is needed by both the kernel
and M-mode firmware. For example, the kernel needs to know about
noncacheable aliases of cacheable memory in order to allocate coherent
memory pages for DMA. M-mode firmware needs to know about aliases so it
can protect itself from lower-privileged software. Firmware also needs
to know the platform's Physical Address Width in order to efficiently
implement Smmpt.

The RISC-V Privileged Architecture delegates the description of Physical
Memory Attributes to the platform. On DT-based platforms, it makes sense
to put this information in the devicetree.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 .../bindings/riscv/physical-memory.yaml       | 101 ++++++++++++++++++
 include/dt-bindings/riscv/physical-memory.h   |  44 ++++++++
 2 files changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/physical-memory.yaml
 create mode 100644 include/dt-bindings/riscv/physical-memory.h

diff --git a/Documentation/devicetree/bindings/riscv/physical-memory.yaml b/Documentation/devicetree/bindings/riscv/physical-memory.yaml
new file mode 100644
index 000000000000..deb49b34672f
--- /dev/null
+++ b/Documentation/devicetree/bindings/riscv/physical-memory.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/physical-memory.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Physical Memory Regions
+
+maintainers:
+  - Samuel Holland <samuel.holland@sifive.com>
+
+description:
+  The RISC-V Privileged Architecture defines a number of Physical Memory
+  Attributes (PMAs) which apply to a given region of memory. These include the
+  types of accesses (read, write, execute, LR/SC, and/or AMO) allowed within
+  a region, the supported access widths and alignments, the cacheability and
+  coherence of the region, and whether or not accesses to the region may have
+  side effects.
+
+  Some RISC-V platforms provide multiple physical address mappings for main
+  memory or certain peripherals. Each alias of a region generally has different
+  PMAs (e.g. cacheable vs non-cacheable), which allows software to dynamically
+  select the PMAs for an access by referencing the corresponding alias.
+
+  The RISC-V Supervisor Domains specification defines a platform-specific
+  Physical Address Width (PAW), which describes the largest physical address
+  supported by a platform. Any access to an address >= 2^PAW is guaranteed to
+  raise an access fault, and therefore metadata (e.g. Memory Protection Tables)
+  need not be maintained for those addresses.
+
+  On DT-based RISC-V platforms, all of this information is provided by the
+  riscv,physical-memory-regions property of the root node.
+
+properties:
+  $nodename:
+    const: '/'
+
+  riscv,physical-memory-regions:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      A table of physical memory regions. The first entry in the table must
+      cover the entire range of physical addresses supported by the platform
+      (i.e. 0 to 2^PAW-1) and provides the default PMAs for all addresses not
+      covered by another table entry. Remaining table entries provide PMAs for
+      more specific physical memory regions, which must be contained within the
+      range of entry 0, but which must not overlap with each other.
+    minItems: 1
+    maxItems: 256
+    items:
+      minItems: 4
+      maxItems: 6
+      additionalItems: true
+      items:
+        - description: CPU physical address (#address-cells)
+        - description: >
+            Size (#size-cells). For entry 0, if the size is zero, the size is
+            assumed to be 2^(32 * #size-cells).
+        - description: >
+            Flags describing the most restrictive PMAs for any address within
+            the region.
+
+            The least significant byte indicates the types of accesses allowed
+            for this region. Note that a memory region may support a type of
+            access (e.g. AMOs) even if the CPU does not.
+
+            The next byte describes the cacheability, coherence, idempotency,
+            and ordering PMAs for this region. It also includes a flag to
+            indicate that accesses to a region are unsafe and must be
+            prohibited by software (for example using PMPs or Smmpt).
+
+            The third byte is reserved for future PMAs.
+
+            The most significant byte is the index of the lowest-numbered entry
+            which this entry is an alias of, if any. Aliases need not be the
+            same size, for example if a smaller memory region repeats within a
+            larger alias.
+        - description: Reserved for describing future PMAs
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/riscv/physical-memory.h>
+
+    / {
+      compatible = "starfive,jh7100";
+      #address-cells = <2>;
+      #size-cells = <2>;
+      riscv,physical-memory-regions =
+          <0x00 0x00000000 0x40 0x00000000 (PMA_RW | PMA_IO) 0x0>,
+          <0x00 0x18000000 0x00 0x00020000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY) 0x0>,
+          <0x00 0x18080000 0x00 0x00020000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY) 0x0>,
+          <0x00 0x41000000 0x00 0x1f000000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY) 0x0>,
+          <0x00 0x61000000 0x00 0x1f000000 (PMA_RWXA | PMA_NONCOHERENT_MEMORY | PMR_ALIAS(3)) 0x0>,
+          <0x00 0x80000000 0x08 0x00000000 (PMA_RWXA | PMA_NONCOHERENT_MEMORY) 0x0>,
+          <0x10 0x00000000 0x08 0x00000000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY | PMR_ALIAS(5)) 0x0>,
+          <0x20 0x00000000 0x10 0x00000000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY) 0x0>,
+          <0x30 0x00000000 0x10 0x00000000 (PMA_RWXA | PMA_NONCOHERENT_MEMORY | PMR_ALIAS(7)) 0x0>;
+    };
+
+...
diff --git a/include/dt-bindings/riscv/physical-memory.h b/include/dt-bindings/riscv/physical-memory.h
new file mode 100644
index 000000000000..7cb2e58fa8c1
--- /dev/null
+++ b/include/dt-bindings/riscv/physical-memory.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_RISCV_PHYSICAL_MEMORY_H
+#define _DT_BINDINGS_RISCV_PHYSICAL_MEMORY_H
+
+#define PMA_READ			(1 << 0)
+#define PMA_WRITE			(1 << 1)
+#define PMA_EXECUTE			(1 << 2)
+#define PMA_AMO_MASK			(3 << 4)
+#define PMA_AMO_NONE			(0 << 4)
+#define PMA_AMO_SWAP			(1 << 4)
+#define PMA_AMO_LOGICAL			(2 << 4)
+#define PMA_AMO_ARITHMETIC		(3 << 4)
+#define PMA_RSRV_MASK			(3 << 6)
+#define PMA_RSRV_NONE			(0 << 6)
+#define PMA_RSRV_NON_EVENTUAL		(1 << 6)
+#define PMA_RSRV_EVENTUAL		(2 << 6)
+
+#define PMA_RW				(PMA_READ | PMA_WRITE)
+#define PMA_RWA				(PMA_RW | PMA_AMO_ARITHMETIC | PMA_RSRV_EVENTUAL)
+#define PMA_RWX				(PMA_RW | PMA_EXECUTE)
+#define PMA_RWXA			(PMA_RWA | PMA_EXECUTE)
+
+#define PMA_ORDER_MASK			(3 << 8)
+#define PMA_ORDER_IO_RELAXED		(0 << 8)
+#define PMA_ORDER_IO_STRONG		(1 << 8)
+#define PMA_ORDER_MEMORY		(2 << 8)
+#define PMA_READ_IDEMPOTENT		(1 << 10)
+#define PMA_WRITE_IDEMPOTENT		(1 << 11)
+#define PMA_CACHEABLE			(1 << 12)
+#define PMA_COHERENT			(1 << 13)
+
+#define PMA_UNSAFE			(1 << 15)
+
+#define PMA_IO				(PMA_ORDER_IO_RELAXED)
+#define PMA_NONCACHEABLE_MEMORY		(PMA_ORDER_MEMORY | PMA_READ_IDEMPOTENT | \
+						PMA_WRITE_IDEMPOTENT)
+#define PMA_NONCOHERENT_MEMORY		(PMA_NONCACHEABLE_MEMORY | PMA_CACHEABLE)
+#define PMA_NORMAL_MEMORY		(PMA_NONCOHERENT_MEMORY | PMA_COHERENT)
+
+#define PMR_ALIAS_MASK			(0xff << 24)
+#define PMR_ALIAS(n)			((n) << 24)
+
+#endif /* _DT_BINDINGS_RISCV_PHYSICAL_MEMORY_H */
-- 
2.45.1

Re: [PATCH 01/11] dt-bindings: riscv: Describe physical memory regions

[Rob Herring (Arm)]

On Fri, 01 Nov 2024 17:07:55 -0700, Samuel Holland wrote:
> Information about physical memory regions is needed by both the kernel
> and M-mode firmware. For example, the kernel needs to know about
> noncacheable aliases of cacheable memory in order to allocate coherent
> memory pages for DMA. M-mode firmware needs to know about aliases so it
> can protect itself from lower-privileged software. Firmware also needs
> to know the platform's Physical Address Width in order to efficiently
> implement Smmpt.
> 
> The RISC-V Privileged Architecture delegates the description of Physical
> Memory Attributes to the platform. On DT-based platforms, it makes sense
> to put this information in the devicetree.
> 
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  .../bindings/riscv/physical-memory.yaml       | 101 ++++++++++++++++++
>  include/dt-bindings/riscv/physical-memory.h   |  44 ++++++++
>  2 files changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/riscv/physical-memory.yaml
>  create mode 100644 include/dt-bindings/riscv/physical-memory.h
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/physical-memory.example.dtb: /: compatible: 'oneOf' conditional failed, one must be fixed:
	['starfive,jh7100'] is too short
	'starfive,jh7100' is not one of ['beagle,beaglev-starlight-jh7100-r0', 'starfive,visionfive-v1']
	'starfive,jh7100' is not one of ['milkv,mars', 'pine64,star64', 'starfive,visionfive-2-v1.2a', 'starfive,visionfive-2-v1.3b']
	from schema $id: http://devicetree.org/schemas/riscv/starfive.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/physical-memory.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241102000843.1301099-2-samuel.holland@sifive.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH 02/11] riscv: mm: Increment PFN in place when splitting mappings

[Samuel Holland]

The current code separates page table entry values into a PFN and a
pgprot_t before incrementing the PFN and combining the two parts using
pfn_pXX(). On some hardware with custom page table formats or memory
aliases, the pfn_pXX() functions need to transform the PTE value, so
these functions would need to apply the opposite transformation when
breaking apart the PTE value.

However, both transformations can be avoided by incrementing the PFN in
place, as done by pte_advance_pfn() and set_ptes().

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/mm/pageattr.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
index 271d01a5ba4d..335060adc1a6 100644
--- a/arch/riscv/mm/pageattr.c
+++ b/arch/riscv/mm/pageattr.c
@@ -109,9 +109,8 @@ static int __split_linear_mapping_pmd(pud_t *pudp,
 			continue;
 
 		if (pmd_leaf(pmdp_get(pmdp))) {
+			pte_t pte = pmd_pte(pmdp_get(pmdp));
 			struct page *pte_page;
-			unsigned long pfn = _pmd_pfn(pmdp_get(pmdp));
-			pgprot_t prot = __pgprot(pmd_val(pmdp_get(pmdp)) & ~_PAGE_PFN_MASK);
 			pte_t *ptep_new;
 			int i;
 
@@ -121,7 +120,7 @@ static int __split_linear_mapping_pmd(pud_t *pudp,
 
 			ptep_new = (pte_t *)page_address(pte_page);
 			for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
-				set_pte(ptep_new, pfn_pte(pfn + i, prot));
+				set_pte(ptep_new, pte_advance_pfn(pte, i));
 
 			smp_wmb();
 
@@ -149,9 +148,8 @@ static int __split_linear_mapping_pud(p4d_t *p4dp,
 			continue;
 
 		if (pud_leaf(pudp_get(pudp))) {
+			pmd_t pmd = __pmd(pud_val(pudp_get(pudp)));
 			struct page *pmd_page;
-			unsigned long pfn = _pud_pfn(pudp_get(pudp));
-			pgprot_t prot = __pgprot(pud_val(pudp_get(pudp)) & ~_PAGE_PFN_MASK);
 			pmd_t *pmdp_new;
 			int i;
 
@@ -162,7 +160,8 @@ static int __split_linear_mapping_pud(p4d_t *p4dp,
 			pmdp_new = (pmd_t *)page_address(pmd_page);
 			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new)
 				set_pmd(pmdp_new,
-					pfn_pmd(pfn + ((i * PMD_SIZE) >> PAGE_SHIFT), prot));
+					__pmd(pmd_val(pmd) +
+					      (i << (PMD_SHIFT - PAGE_SHIFT + PFN_PTE_SHIFT))));
 
 			smp_wmb();
 
@@ -198,9 +197,8 @@ static int __split_linear_mapping_p4d(pgd_t *pgdp,
 			continue;
 
 		if (p4d_leaf(p4dp_get(p4dp))) {
+			pud_t pud = __pud(p4d_val(p4dp_get(p4dp)));
 			struct page *pud_page;
-			unsigned long pfn = _p4d_pfn(p4dp_get(p4dp));
-			pgprot_t prot = __pgprot(p4d_val(p4dp_get(p4dp)) & ~_PAGE_PFN_MASK);
 			pud_t *pudp_new;
 			int i;
 
@@ -215,7 +213,8 @@ static int __split_linear_mapping_p4d(pgd_t *pgdp,
 			pudp_new = (pud_t *)page_address(pud_page);
 			for (i = 0; i < PTRS_PER_PUD; ++i, ++pudp_new)
 				set_pud(pudp_new,
-					pfn_pud(pfn + ((i * PUD_SIZE) >> PAGE_SHIFT), prot));
+					__pud(pud_val(pud) +
+					      (i << (PUD_SHIFT - PAGE_SHIFT + PFN_PTE_SHIFT))));
 
 			/*
 			 * Make sure the pud filling is not reordered with the
-- 
2.45.1

Re: [PATCH 02/11] riscv: mm: Increment PFN in place when splitting mappings

[Alexandre Ghiti]

Hi Samuel,

On 02/11/2024 01:07, Samuel Holland wrote:
> The current code separates page table entry values into a PFN and a
> pgprot_t before incrementing the PFN and combining the two parts using
> pfn_pXX(). On some hardware with custom page table formats or memory
> aliases, the pfn_pXX() functions need to transform the PTE value, so
> these functions would need to apply the opposite transformation when
> breaking apart the PTE value.
>
> However, both transformations can be avoided by incrementing the PFN in
> place, as done by pte_advance_pfn() and set_ptes().
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>   arch/riscv/mm/pageattr.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
> index 271d01a5ba4d..335060adc1a6 100644
> --- a/arch/riscv/mm/pageattr.c
> +++ b/arch/riscv/mm/pageattr.c
> @@ -109,9 +109,8 @@ static int __split_linear_mapping_pmd(pud_t *pudp,
>   			continue;
>   
>   		if (pmd_leaf(pmdp_get(pmdp))) {
> +			pte_t pte = pmd_pte(pmdp_get(pmdp));
>   			struct page *pte_page;
> -			unsigned long pfn = _pmd_pfn(pmdp_get(pmdp));
> -			pgprot_t prot = __pgprot(pmd_val(pmdp_get(pmdp)) & ~_PAGE_PFN_MASK);
>   			pte_t *ptep_new;
>   			int i;
>   
> @@ -121,7 +120,7 @@ static int __split_linear_mapping_pmd(pud_t *pudp,
>   
>   			ptep_new = (pte_t *)page_address(pte_page);
>   			for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
> -				set_pte(ptep_new, pfn_pte(pfn + i, prot));
> +				set_pte(ptep_new, pte_advance_pfn(pte, i));
>   
>   			smp_wmb();
>   
> @@ -149,9 +148,8 @@ static int __split_linear_mapping_pud(p4d_t *p4dp,
>   			continue;
>   
>   		if (pud_leaf(pudp_get(pudp))) {
> +			pmd_t pmd = __pmd(pud_val(pudp_get(pudp)));


Nit: You could use pud_pte() here.


>   			struct page *pmd_page;
> -			unsigned long pfn = _pud_pfn(pudp_get(pudp));
> -			pgprot_t prot = __pgprot(pud_val(pudp_get(pudp)) & ~_PAGE_PFN_MASK);
>   			pmd_t *pmdp_new;
>   			int i;
>   
> @@ -162,7 +160,8 @@ static int __split_linear_mapping_pud(p4d_t *p4dp,
>   			pmdp_new = (pmd_t *)page_address(pmd_page);
>   			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new)
>   				set_pmd(pmdp_new,
> -					pfn_pmd(pfn + ((i * PMD_SIZE) >> PAGE_SHIFT), prot));
> +					__pmd(pmd_val(pmd) +
> +					      (i << (PMD_SHIFT - PAGE_SHIFT + PFN_PTE_SHIFT))));


Nit: Here you could use pte_advance_pfn(pmd, i << (PMD_SHIFT - PAGE_SHIFT))


>   
>   			smp_wmb();
>   
> @@ -198,9 +197,8 @@ static int __split_linear_mapping_p4d(pgd_t *pgdp,
>   			continue;
>   
>   		if (p4d_leaf(p4dp_get(p4dp))) {
> +			pud_t pud = __pud(p4d_val(p4dp_get(p4dp)));
>   			struct page *pud_page;
> -			unsigned long pfn = _p4d_pfn(p4dp_get(p4dp));
> -			pgprot_t prot = __pgprot(p4d_val(p4dp_get(p4dp)) & ~_PAGE_PFN_MASK);
>   			pud_t *pudp_new;
>   			int i;
>   
> @@ -215,7 +213,8 @@ static int __split_linear_mapping_p4d(pgd_t *pgdp,
>   			pudp_new = (pud_t *)page_address(pud_page);
>   			for (i = 0; i < PTRS_PER_PUD; ++i, ++pudp_new)
>   				set_pud(pudp_new,
> -					pfn_pud(pfn + ((i * PUD_SIZE) >> PAGE_SHIFT), prot));
> +					__pud(pud_val(pud) +
> +					      (i << (PUD_SHIFT - PAGE_SHIFT + PFN_PTE_SHIFT))));


Nit: Ditto


>   
>   			/*
>   			 * Make sure the pud filling is not reordered with the


Other than the nits (which are up to you), you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

[PATCH 03/11] riscv: mm: Deduplicate pgtable address conversion functions

[Samuel Holland]

Some functions were defined equivalently in both pgtable.h and
pgtable-64.h. Keep only one definition, and move it to pgtable-64.h
unless it is also used for Sv32. Note that while Sv32 uses only two
levels of page tables, the kernel is not consistent with how they are
folded. THP requires pfn_pmd()/pmd_pfn() and mm/init.c requires
pfn_pgd()/pgd_pfn(), so for now both are provided.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/pgtable-32.h |  4 ++++
 arch/riscv/include/asm/pgtable-64.h | 28 ++++++++++++++--------------
 arch/riscv/include/asm/pgtable.h    | 23 ++++-------------------
 arch/riscv/mm/init.c                |  8 ++++----
 arch/riscv/mm/kasan_init.c          |  8 ++++----
 5 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index 00f3369570a8..23137347dc15 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -33,6 +33,10 @@
 					  _PAGE_WRITE | _PAGE_EXEC |	\
 					  _PAGE_USER | _PAGE_GLOBAL))
 
+#define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
+#define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))
+#define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
+
 static const __maybe_unused int pgtable_l4_enabled;
 static const __maybe_unused int pgtable_l5_enabled;
 
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 0897dd99ab8d..33e7ff049c4a 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -213,19 +213,20 @@ static inline pud_t pfn_pud(unsigned long pfn, pgprot_t prot)
 	return __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }
 
-static inline unsigned long _pud_pfn(pud_t pud)
+#define pud_pfn pud_pfn
+static inline unsigned long pud_pfn(pud_t pud)
 {
 	return __page_val_to_pfn(pud_val(pud));
 }
 
 static inline pmd_t *pud_pgtable(pud_t pud)
 {
-	return (pmd_t *)pfn_to_virt(__page_val_to_pfn(pud_val(pud)));
+	return (pmd_t *)pfn_to_virt(pud_pfn(pud));
 }
 
 static inline struct page *pud_page(pud_t pud)
 {
-	return pfn_to_page(__page_val_to_pfn(pud_val(pud)));
+	return pfn_to_page(pud_pfn(pud));
 }
 
 #define mm_p4d_folded  mm_p4d_folded
@@ -257,11 +258,6 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
 	return __pmd((pfn << _PAGE_PFN_SHIFT) | prot_val);
 }
 
-static inline unsigned long _pmd_pfn(pmd_t pmd)
-{
-	return __page_val_to_pfn(pmd_val(pmd));
-}
-
 #define mk_pmd(page, prot)    pfn_pmd(page_to_pfn(page), prot)
 
 #define pmd_ERROR(e) \
@@ -316,7 +312,7 @@ static inline p4d_t pfn_p4d(unsigned long pfn, pgprot_t prot)
 	return __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }
 
-static inline unsigned long _p4d_pfn(p4d_t p4d)
+static inline unsigned long p4d_pfn(p4d_t p4d)
 {
 	return __page_val_to_pfn(p4d_val(p4d));
 }
@@ -324,7 +320,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)
 static inline pud_t *p4d_pgtable(p4d_t p4d)
 {
 	if (pgtable_l4_enabled)
-		return (pud_t *)pfn_to_virt(__page_val_to_pfn(p4d_val(p4d)));
+		return (pud_t *)pfn_to_virt(p4d_pfn(p4d));
 
 	return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
 }
@@ -332,7 +328,7 @@ static inline pud_t *p4d_pgtable(p4d_t p4d)
 
 static inline struct page *p4d_page(p4d_t p4d)
 {
-	return pfn_to_page(__page_val_to_pfn(p4d_val(p4d)));
+	return pfn_to_page(p4d_pfn(p4d));
 }
 
 #define pud_index(addr) (((addr) >> PUD_SHIFT) & (PTRS_PER_PUD - 1))
@@ -378,10 +374,15 @@ static inline void pgd_clear(pgd_t *pgd)
 		set_pgd(pgd, __pgd(0));
 }
 
+static inline unsigned long pgd_pfn(pgd_t pgd)
+{
+	return __page_val_to_pfn(pgd_val(pgd));
+}
+
 static inline p4d_t *pgd_pgtable(pgd_t pgd)
 {
 	if (pgtable_l5_enabled)
-		return (p4d_t *)pfn_to_virt(__page_val_to_pfn(pgd_val(pgd)));
+		return (p4d_t *)pfn_to_virt(pgd_pfn(pgd));
 
 	return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
 }
@@ -389,9 +390,8 @@ static inline p4d_t *pgd_pgtable(pgd_t pgd)
 
 static inline struct page *pgd_page(pgd_t pgd)
 {
-	return pfn_to_page(__page_val_to_pfn(pgd_val(pgd)));
+	return pfn_to_page(pgd_pfn(pgd));
 }
-#define pgd_page(pgd)	pgd_page(pgd)
 
 #define p4d_index(addr) (((addr) >> P4D_SHIFT) & (PTRS_PER_P4D - 1))
 
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index e79f15293492..3e0e1177107d 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -258,19 +258,19 @@ static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
 	return __pgd((pfn << _PAGE_PFN_SHIFT) | prot_val);
 }
 
-static inline unsigned long _pgd_pfn(pgd_t pgd)
+static inline unsigned long pmd_pfn(pmd_t pmd)
 {
-	return __page_val_to_pfn(pgd_val(pgd));
+	return __page_val_to_pfn(pmd_val(pmd));
 }
 
 static inline struct page *pmd_page(pmd_t pmd)
 {
-	return pfn_to_page(__page_val_to_pfn(pmd_val(pmd)));
+	return pfn_to_page(pmd_pfn(pmd));
 }
 
 static inline unsigned long pmd_page_vaddr(pmd_t pmd)
 {
-	return (unsigned long)pfn_to_virt(__page_val_to_pfn(pmd_val(pmd)));
+	return (unsigned long)pfn_to_virt(pmd_pfn(pmd));
 }
 
 static inline pte_t pmd_pte(pmd_t pmd)
@@ -673,21 +673,6 @@ static inline pmd_t pmd_mkinvalid(pmd_t pmd)
 	return __pmd(pmd_val(pmd) & ~(_PAGE_PRESENT|_PAGE_PROT_NONE));
 }
 
-#define __pmd_to_phys(pmd)  (__page_val_to_pfn(pmd_val(pmd)) << PAGE_SHIFT)
-
-static inline unsigned long pmd_pfn(pmd_t pmd)
-{
-	return ((__pmd_to_phys(pmd) & PMD_MASK) >> PAGE_SHIFT);
-}
-
-#define __pud_to_phys(pud)  (__page_val_to_pfn(pud_val(pud)) << PAGE_SHIFT)
-
-#define pud_pfn pud_pfn
-static inline unsigned long pud_pfn(pud_t pud)
-{
-	return ((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT);
-}
-
 static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
 {
 	return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 0e8c20adcd98..7282b62b7e8d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -497,7 +497,7 @@ static void __meminit create_pmd_mapping(pmd_t *pmdp,
 		ptep = pt_ops.get_pte_virt(pte_phys);
 		memset(ptep, 0, PAGE_SIZE);
 	} else {
-		pte_phys = PFN_PHYS(_pmd_pfn(pmdp[pmd_idx]));
+		pte_phys = PFN_PHYS(pmd_pfn(pmdp[pmd_idx]));
 		ptep = pt_ops.get_pte_virt(pte_phys);
 	}
 
@@ -599,7 +599,7 @@ static void __meminit create_pud_mapping(pud_t *pudp, uintptr_t va, phys_addr_t
 		nextp = pt_ops.get_pmd_virt(next_phys);
 		memset(nextp, 0, PAGE_SIZE);
 	} else {
-		next_phys = PFN_PHYS(_pud_pfn(pudp[pud_index]));
+		next_phys = PFN_PHYS(pud_pfn(pudp[pud_index]));
 		nextp = pt_ops.get_pmd_virt(next_phys);
 	}
 
@@ -625,7 +625,7 @@ static void __meminit create_p4d_mapping(p4d_t *p4dp, uintptr_t va, phys_addr_t
 		nextp = pt_ops.get_pud_virt(next_phys);
 		memset(nextp, 0, PAGE_SIZE);
 	} else {
-		next_phys = PFN_PHYS(_p4d_pfn(p4dp[p4d_index]));
+		next_phys = PFN_PHYS(p4d_pfn(p4dp[p4d_index]));
 		nextp = pt_ops.get_pud_virt(next_phys);
 	}
 
@@ -682,7 +682,7 @@ void __meminit create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa, phy
 		nextp = get_pgd_next_virt(next_phys);
 		memset(nextp, 0, PAGE_SIZE);
 	} else {
-		next_phys = PFN_PHYS(_pgd_pfn(pgdp[pgd_idx]));
+		next_phys = PFN_PHYS(pgd_pfn(pgdp[pgd_idx]));
 		nextp = get_pgd_next_virt(next_phys);
 	}
 
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
index c301c8d291d2..bac65e3268a4 100644
--- a/arch/riscv/mm/kasan_init.c
+++ b/arch/riscv/mm/kasan_init.c
@@ -171,7 +171,7 @@ static void __init kasan_early_clear_pud(p4d_t *p4dp,
 	if (!pgtable_l4_enabled) {
 		pudp = (pud_t *)p4dp;
 	} else {
-		base_pud = pt_ops.get_pud_virt(pfn_to_phys(_p4d_pfn(p4dp_get(p4dp))));
+		base_pud = pt_ops.get_pud_virt(pfn_to_phys(p4d_pfn(p4dp_get(p4dp))));
 		pudp = base_pud + pud_index(vaddr);
 	}
 
@@ -196,7 +196,7 @@ static void __init kasan_early_clear_p4d(pgd_t *pgdp,
 	if (!pgtable_l5_enabled) {
 		p4dp = (p4d_t *)pgdp;
 	} else {
-		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(pgdp_get(pgdp))));
+		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(pgd_pfn(pgdp_get(pgdp))));
 		p4dp = base_p4d + p4d_index(vaddr);
 	}
 
@@ -242,7 +242,7 @@ static void __init kasan_early_populate_pud(p4d_t *p4dp,
 	if (!pgtable_l4_enabled) {
 		pudp = (pud_t *)p4dp;
 	} else {
-		base_pud = pt_ops.get_pud_virt(pfn_to_phys(_p4d_pfn(p4dp_get(p4dp))));
+		base_pud = pt_ops.get_pud_virt(pfn_to_phys(p4d_pfn(p4dp_get(p4dp))));
 		pudp = base_pud + pud_index(vaddr);
 	}
 
@@ -280,7 +280,7 @@ static void __init kasan_early_populate_p4d(pgd_t *pgdp,
 	if (!pgtable_l5_enabled) {
 		p4dp = (p4d_t *)pgdp;
 	} else {
-		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(pgdp_get(pgdp))));
+		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(pgd_pfn(pgdp_get(pgdp))));
 		p4dp = base_p4d + p4d_index(vaddr);
 	}
 
-- 
2.45.1

Re: [PATCH 03/11] riscv: mm: Deduplicate pgtable address conversion functions

[Alexandre Ghiti]

Hi Samuel,

On 02/11/2024 01:07, Samuel Holland wrote:
> Some functions were defined equivalently in both pgtable.h and
> pgtable-64.h. Keep only one definition, and move it to pgtable-64.h
> unless it is also used for Sv32. Note that while Sv32 uses only two
> levels of page tables, the kernel is not consistent with how they are
> folded. THP requires pfn_pmd()/pmd_pfn() and mm/init.c requires
> pfn_pgd()/pgd_pfn(), so for now both are provided.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>   arch/riscv/include/asm/pgtable-32.h |  4 ++++
>   arch/riscv/include/asm/pgtable-64.h | 28 ++++++++++++++--------------
>   arch/riscv/include/asm/pgtable.h    | 23 ++++-------------------
>   arch/riscv/mm/init.c                |  8 ++++----
>   arch/riscv/mm/kasan_init.c          |  8 ++++----
>   5 files changed, 30 insertions(+), 41 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
> index 00f3369570a8..23137347dc15 100644
> --- a/arch/riscv/include/asm/pgtable-32.h
> +++ b/arch/riscv/include/asm/pgtable-32.h
> @@ -33,6 +33,10 @@
>   					  _PAGE_WRITE | _PAGE_EXEC |	\
>   					  _PAGE_USER | _PAGE_GLOBAL))
>   
> +#define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
> +#define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))


pud_pfn() and p4d_pfn() should not be used in 32-bit right? I think you 
can remove their definitions and simplify pgd_pfn().


> +#define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
> +
>   static const __maybe_unused int pgtable_l4_enabled;
>   static const __maybe_unused int pgtable_l5_enabled;
>   
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 0897dd99ab8d..33e7ff049c4a 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -213,19 +213,20 @@ static inline pud_t pfn_pud(unsigned long pfn, pgprot_t prot)
>   	return __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>   }
>   
> -static inline unsigned long _pud_pfn(pud_t pud)
> +#define pud_pfn pud_pfn
> +static inline unsigned long pud_pfn(pud_t pud)
>   {
>   	return __page_val_to_pfn(pud_val(pud));
>   }
>   
>   static inline pmd_t *pud_pgtable(pud_t pud)
>   {
> -	return (pmd_t *)pfn_to_virt(__page_val_to_pfn(pud_val(pud)));
> +	return (pmd_t *)pfn_to_virt(pud_pfn(pud));
>   }
>   
>   static inline struct page *pud_page(pud_t pud)
>   {
> -	return pfn_to_page(__page_val_to_pfn(pud_val(pud)));
> +	return pfn_to_page(pud_pfn(pud));
>   }
>   
>   #define mm_p4d_folded  mm_p4d_folded
> @@ -257,11 +258,6 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
>   	return __pmd((pfn << _PAGE_PFN_SHIFT) | prot_val);
>   }
>   
> -static inline unsigned long _pmd_pfn(pmd_t pmd)
> -{
> -	return __page_val_to_pfn(pmd_val(pmd));
> -}
> -
>   #define mk_pmd(page, prot)    pfn_pmd(page_to_pfn(page), prot)
>   
>   #define pmd_ERROR(e) \
> @@ -316,7 +312,7 @@ static inline p4d_t pfn_p4d(unsigned long pfn, pgprot_t prot)
>   	return __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>   }
>   
> -static inline unsigned long _p4d_pfn(p4d_t p4d)
> +static inline unsigned long p4d_pfn(p4d_t p4d)
>   {
>   	return __page_val_to_pfn(p4d_val(p4d));
>   }
> @@ -324,7 +320,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)
>   static inline pud_t *p4d_pgtable(p4d_t p4d)
>   {
>   	if (pgtable_l4_enabled)
> -		return (pud_t *)pfn_to_virt(__page_val_to_pfn(p4d_val(p4d)));
> +		return (pud_t *)pfn_to_virt(p4d_pfn(p4d));
>   
>   	return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
>   }
> @@ -332,7 +328,7 @@ static inline pud_t *p4d_pgtable(p4d_t p4d)
>   
>   static inline struct page *p4d_page(p4d_t p4d)
>   {
> -	return pfn_to_page(__page_val_to_pfn(p4d_val(p4d)));
> +	return pfn_to_page(p4d_pfn(p4d));
>   }
>   
>   #define pud_index(addr) (((addr) >> PUD_SHIFT) & (PTRS_PER_PUD - 1))
> @@ -378,10 +374,15 @@ static inline void pgd_clear(pgd_t *pgd)
>   		set_pgd(pgd, __pgd(0));
>   }
>   
> +static inline unsigned long pgd_pfn(pgd_t pgd)
> +{
> +	return __page_val_to_pfn(pgd_val(pgd));
> +}
> +
>   static inline p4d_t *pgd_pgtable(pgd_t pgd)
>   {
>   	if (pgtable_l5_enabled)
> -		return (p4d_t *)pfn_to_virt(__page_val_to_pfn(pgd_val(pgd)));
> +		return (p4d_t *)pfn_to_virt(pgd_pfn(pgd));
>   
>   	return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
>   }
> @@ -389,9 +390,8 @@ static inline p4d_t *pgd_pgtable(pgd_t pgd)
>   
>   static inline struct page *pgd_page(pgd_t pgd)
>   {
> -	return pfn_to_page(__page_val_to_pfn(pgd_val(pgd)));
> +	return pfn_to_page(pgd_pfn(pgd));
>   }
> -#define pgd_page(pgd)	pgd_page(pgd)
>   
>   #define p4d_index(addr) (((addr) >> P4D_SHIFT) & (PTRS_PER_P4D - 1))
>   
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index e79f15293492..3e0e1177107d 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -258,19 +258,19 @@ static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
>   	return __pgd((pfn << _PAGE_PFN_SHIFT) | prot_val);
>   }
>   
> -static inline unsigned long _pgd_pfn(pgd_t pgd)
> +static inline unsigned long pmd_pfn(pmd_t pmd)
>   {
> -	return __page_val_to_pfn(pgd_val(pgd));
> +	return __page_val_to_pfn(pmd_val(pmd));
>   }
>   
>   static inline struct page *pmd_page(pmd_t pmd)
>   {
> -	return pfn_to_page(__page_val_to_pfn(pmd_val(pmd)));
> +	return pfn_to_page(pmd_pfn(pmd));
>   }
>   
>   static inline unsigned long pmd_page_vaddr(pmd_t pmd)
>   {
> -	return (unsigned long)pfn_to_virt(__page_val_to_pfn(pmd_val(pmd)));
> +	return (unsigned long)pfn_to_virt(pmd_pfn(pmd));
>   }
>   
>   static inline pte_t pmd_pte(pmd_t pmd)
> @@ -673,21 +673,6 @@ static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>   	return __pmd(pmd_val(pmd) & ~(_PAGE_PRESENT|_PAGE_PROT_NONE));
>   }
>   
> -#define __pmd_to_phys(pmd)  (__page_val_to_pfn(pmd_val(pmd)) << PAGE_SHIFT)
> -
> -static inline unsigned long pmd_pfn(pmd_t pmd)
> -{
> -	return ((__pmd_to_phys(pmd) & PMD_MASK) >> PAGE_SHIFT);
> -}
> -
> -#define __pud_to_phys(pud)  (__page_val_to_pfn(pud_val(pud)) << PAGE_SHIFT)
> -
> -#define pud_pfn pud_pfn
> -static inline unsigned long pud_pfn(pud_t pud)
> -{
> -	return ((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT);
> -}
> -
>   static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>   {
>   	return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 0e8c20adcd98..7282b62b7e8d 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -497,7 +497,7 @@ static void __meminit create_pmd_mapping(pmd_t *pmdp,
>   		ptep = pt_ops.get_pte_virt(pte_phys);
>   		memset(ptep, 0, PAGE_SIZE);
>   	} else {
> -		pte_phys = PFN_PHYS(_pmd_pfn(pmdp[pmd_idx]));
> +		pte_phys = PFN_PHYS(pmd_pfn(pmdp[pmd_idx]));
>   		ptep = pt_ops.get_pte_virt(pte_phys);
>   	}
>   
> @@ -599,7 +599,7 @@ static void __meminit create_pud_mapping(pud_t *pudp, uintptr_t va, phys_addr_t
>   		nextp = pt_ops.get_pmd_virt(next_phys);
>   		memset(nextp, 0, PAGE_SIZE);
>   	} else {
> -		next_phys = PFN_PHYS(_pud_pfn(pudp[pud_index]));
> +		next_phys = PFN_PHYS(pud_pfn(pudp[pud_index]));
>   		nextp = pt_ops.get_pmd_virt(next_phys);
>   	}
>   
> @@ -625,7 +625,7 @@ static void __meminit create_p4d_mapping(p4d_t *p4dp, uintptr_t va, phys_addr_t
>   		nextp = pt_ops.get_pud_virt(next_phys);
>   		memset(nextp, 0, PAGE_SIZE);
>   	} else {
> -		next_phys = PFN_PHYS(_p4d_pfn(p4dp[p4d_index]));
> +		next_phys = PFN_PHYS(p4d_pfn(p4dp[p4d_index]));
>   		nextp = pt_ops.get_pud_virt(next_phys);
>   	}
>   
> @@ -682,7 +682,7 @@ void __meminit create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa, phy
>   		nextp = get_pgd_next_virt(next_phys);
>   		memset(nextp, 0, PAGE_SIZE);
>   	} else {
> -		next_phys = PFN_PHYS(_pgd_pfn(pgdp[pgd_idx]));
> +		next_phys = PFN_PHYS(pgd_pfn(pgdp[pgd_idx]));
>   		nextp = get_pgd_next_virt(next_phys);
>   	}
>   
> diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
> index c301c8d291d2..bac65e3268a4 100644
> --- a/arch/riscv/mm/kasan_init.c
> +++ b/arch/riscv/mm/kasan_init.c
> @@ -171,7 +171,7 @@ static void __init kasan_early_clear_pud(p4d_t *p4dp,
>   	if (!pgtable_l4_enabled) {
>   		pudp = (pud_t *)p4dp;
>   	} else {
> -		base_pud = pt_ops.get_pud_virt(pfn_to_phys(_p4d_pfn(p4dp_get(p4dp))));
> +		base_pud = pt_ops.get_pud_virt(pfn_to_phys(p4d_pfn(p4dp_get(p4dp))));
>   		pudp = base_pud + pud_index(vaddr);
>   	}
>   
> @@ -196,7 +196,7 @@ static void __init kasan_early_clear_p4d(pgd_t *pgdp,
>   	if (!pgtable_l5_enabled) {
>   		p4dp = (p4d_t *)pgdp;
>   	} else {
> -		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(pgdp_get(pgdp))));
> +		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(pgd_pfn(pgdp_get(pgdp))));
>   		p4dp = base_p4d + p4d_index(vaddr);
>   	}
>   
> @@ -242,7 +242,7 @@ static void __init kasan_early_populate_pud(p4d_t *p4dp,
>   	if (!pgtable_l4_enabled) {
>   		pudp = (pud_t *)p4dp;
>   	} else {
> -		base_pud = pt_ops.get_pud_virt(pfn_to_phys(_p4d_pfn(p4dp_get(p4dp))));
> +		base_pud = pt_ops.get_pud_virt(pfn_to_phys(p4d_pfn(p4dp_get(p4dp))));
>   		pudp = base_pud + pud_index(vaddr);
>   	}
>   
> @@ -280,7 +280,7 @@ static void __init kasan_early_populate_p4d(pgd_t *pgdp,
>   	if (!pgtable_l5_enabled) {
>   		p4dp = (p4d_t *)pgdp;
>   	} else {
> -		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(pgdp_get(pgdp))));
> +		base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(pgd_pfn(pgdp_get(pgdp))));
>   		p4dp = base_p4d + p4d_index(vaddr);
>   	}
>   


Otherwise, this is a nice cleanup, you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

[PATCH 04/11] riscv: mm: Deduplicate _PAGE_CHG_MASK definition

[Samuel Holland]

The two existing definitions are equivalent because _PAGE_MTMASK is
defined as 0 on riscv32.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/pgtable-32.h | 5 -----
 arch/riscv/include/asm/pgtable-64.h | 7 -------
 arch/riscv/include/asm/pgtable.h    | 6 ++++++
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index 23137347dc15..7dc0751d67dc 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -28,11 +28,6 @@
 #define _PAGE_IO		0
 #define _PAGE_MTMASK		0
 
-/* Set of bits to preserve across pte_modify() */
-#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
-					  _PAGE_WRITE | _PAGE_EXEC |	\
-					  _PAGE_USER | _PAGE_GLOBAL))
-
 #define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
 #define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))
 #define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 33e7ff049c4a..4ba88592b8d1 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -66,7 +66,6 @@ typedef struct {
 
 #define pmd_val(x)      ((x).pmd)
 #define __pmd(x)        ((pmd_t) { (x) })
-
 #define PTRS_PER_PMD    (PAGE_SIZE / sizeof(pmd_t))
 
 /*
@@ -166,12 +165,6 @@ static inline u64 riscv_page_io(void)
 #define _PAGE_IO		riscv_page_io()
 #define _PAGE_MTMASK		riscv_page_mtmask()
 
-/* Set of bits to preserve across pte_modify() */
-#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
-					  _PAGE_WRITE | _PAGE_EXEC |	\
-					  _PAGE_USER | _PAGE_GLOBAL |	\
-					  _PAGE_MTMASK))
-
 static inline int pud_present(pud_t pud)
 {
 	return (pud_val(pud) & _PAGE_PRESENT);
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 3e0e1177107d..afa0b455eaa4 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -201,6 +201,12 @@ extern struct pt_alloc_ops pt_ops __meminitdata;
 #define _PAGE_IOREMAP	((_PAGE_KERNEL & ~_PAGE_MTMASK) | _PAGE_IO)
 #define PAGE_KERNEL_IO		__pgprot(_PAGE_IOREMAP)
 
+/* Set of bits to preserve across pte_modify() */
+#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
+					  _PAGE_WRITE | _PAGE_EXEC |	\
+					  _PAGE_USER | _PAGE_GLOBAL |	\
+					  _PAGE_MTMASK))
+
 extern pgd_t swapper_pg_dir[];
 extern pgd_t trampoline_pg_dir[];
 extern pgd_t early_pg_dir[];
-- 
2.45.1

Re: [PATCH 04/11] riscv: mm: Deduplicate _PAGE_CHG_MASK definition

[Alexandre Ghiti]

On 02/11/2024 01:07, Samuel Holland wrote:
> The two existing definitions are equivalent because _PAGE_MTMASK is
> defined as 0 on riscv32.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>   arch/riscv/include/asm/pgtable-32.h | 5 -----
>   arch/riscv/include/asm/pgtable-64.h | 7 -------
>   arch/riscv/include/asm/pgtable.h    | 6 ++++++
>   3 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
> index 23137347dc15..7dc0751d67dc 100644
> --- a/arch/riscv/include/asm/pgtable-32.h
> +++ b/arch/riscv/include/asm/pgtable-32.h
> @@ -28,11 +28,6 @@
>   #define _PAGE_IO		0
>   #define _PAGE_MTMASK		0
>   
> -/* Set of bits to preserve across pte_modify() */
> -#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
> -					  _PAGE_WRITE | _PAGE_EXEC |	\
> -					  _PAGE_USER | _PAGE_GLOBAL))
> -
>   #define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
>   #define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))
>   #define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 33e7ff049c4a..4ba88592b8d1 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -66,7 +66,6 @@ typedef struct {
>   
>   #define pmd_val(x)      ((x).pmd)
>   #define __pmd(x)        ((pmd_t) { (x) })
> -
>   #define PTRS_PER_PMD    (PAGE_SIZE / sizeof(pmd_t))
>   
>   /*
> @@ -166,12 +165,6 @@ static inline u64 riscv_page_io(void)
>   #define _PAGE_IO		riscv_page_io()
>   #define _PAGE_MTMASK		riscv_page_mtmask()
>   
> -/* Set of bits to preserve across pte_modify() */
> -#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
> -					  _PAGE_WRITE | _PAGE_EXEC |	\
> -					  _PAGE_USER | _PAGE_GLOBAL |	\
> -					  _PAGE_MTMASK))
> -
>   static inline int pud_present(pud_t pud)
>   {
>   	return (pud_val(pud) & _PAGE_PRESENT);
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 3e0e1177107d..afa0b455eaa4 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -201,6 +201,12 @@ extern struct pt_alloc_ops pt_ops __meminitdata;
>   #define _PAGE_IOREMAP	((_PAGE_KERNEL & ~_PAGE_MTMASK) | _PAGE_IO)
>   #define PAGE_KERNEL_IO		__pgprot(_PAGE_IOREMAP)
>   
> +/* Set of bits to preserve across pte_modify() */
> +#define _PAGE_CHG_MASK  (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ |	\
> +					  _PAGE_WRITE | _PAGE_EXEC |	\
> +					  _PAGE_USER | _PAGE_GLOBAL |	\
> +					  _PAGE_MTMASK))
> +
>   extern pgd_t swapper_pg_dir[];
>   extern pgd_t trampoline_pg_dir[];
>   extern pgd_t early_pg_dir[];

Nice clean up too, you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

[PATCH 05/11] riscv: ptdump: Only show N and MT bits when enabled in the kernel

[Samuel Holland]

When the Svnapot or Svpbmt extension is not implemented, the
corresponding page table bits are reserved, and must be zero. There is
no need to show them in the ptdump output.

When the Kconfig option for an extension is disabled, we assume it is
not implemented. In that case, the kernel may provide a fallback
definition for the fields, like how _PAGE_MTMASK is defined on riscv32.
Using those fallback definitions in ptdump would produce incorrect
results. To avoid this, hide the fields from the ptdump output.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/mm/ptdump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
index 9d5f657a251b..58a7322e9a82 100644
--- a/arch/riscv/mm/ptdump.c
+++ b/arch/riscv/mm/ptdump.c
@@ -135,11 +135,13 @@ struct prot_bits {
 
 static const struct prot_bits pte_bits[] = {
 	{
-#ifdef CONFIG_64BIT
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
 		.mask = _PAGE_NAPOT,
 		.set = "N",
 		.clear = ".",
 	}, {
+#endif
+#ifdef CONFIG_RISCV_ISA_SVPBMT
 		.mask = _PAGE_MTMASK_SVPBMT,
 		.set = "MT(%s)",
 		.clear = "  ..  ",
@@ -215,7 +217,7 @@ static void dump_prot(struct pg_state *st)
 		if (val) {
 			if (pte_bits[i].mask == _PAGE_SOFT)
 				sprintf(s, pte_bits[i].set, val >> 8);
-#ifdef CONFIG_64BIT
+#ifdef CONFIG_RISCV_ISA_SVPBMT
 			else if (pte_bits[i].mask == _PAGE_MTMASK_SVPBMT) {
 				if (val == _PAGE_NOCACHE_SVPBMT)
 					sprintf(s, pte_bits[i].set, "NC");
-- 
2.45.1

Re: [PATCH 05/11] riscv: ptdump: Only show N and MT bits when enabled in the kernel

[Alexandre Ghiti]

On 02/11/2024 01:07, Samuel Holland wrote:
> When the Svnapot or Svpbmt extension is not implemented, the
> corresponding page table bits are reserved, and must be zero. There is
> no need to show them in the ptdump output.
>
> When the Kconfig option for an extension is disabled, we assume it is
> not implemented. In that case, the kernel may provide a fallback
> definition for the fields, like how _PAGE_MTMASK is defined on riscv32.
> Using those fallback definitions in ptdump would produce incorrect
> results. To avoid this, hide the fields from the ptdump output.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>   arch/riscv/mm/ptdump.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
> index 9d5f657a251b..58a7322e9a82 100644
> --- a/arch/riscv/mm/ptdump.c
> +++ b/arch/riscv/mm/ptdump.c
> @@ -135,11 +135,13 @@ struct prot_bits {
>   
>   static const struct prot_bits pte_bits[] = {
>   	{
> -#ifdef CONFIG_64BIT
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
>   		.mask = _PAGE_NAPOT,
>   		.set = "N",
>   		.clear = ".",
>   	}, {
> +#endif
> +#ifdef CONFIG_RISCV_ISA_SVPBMT
>   		.mask = _PAGE_MTMASK_SVPBMT,
>   		.set = "MT(%s)",
>   		.clear = "  ..  ",
> @@ -215,7 +217,7 @@ static void dump_prot(struct pg_state *st)
>   		if (val) {
>   			if (pte_bits[i].mask == _PAGE_SOFT)
>   				sprintf(s, pte_bits[i].set, val >> 8);
> -#ifdef CONFIG_64BIT
> +#ifdef CONFIG_RISCV_ISA_SVPBMT
>   			else if (pte_bits[i].mask == _PAGE_MTMASK_SVPBMT) {
>   				if (val == _PAGE_NOCACHE_SVPBMT)
>   					sprintf(s, pte_bits[i].set, "NC");


It makes sense so:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

[PATCH 06/11] riscv: mm: Fix up memory types when writing page tables

[Samuel Holland]

Currently, Linux on RISC-V has three ways to specify the cacheability
and ordering PMAs of a page:
 1) Do nothing; assume the system is entirely cache-coherent and rely on
    the hardware for any ordering requirements
 2) Use the page table bits specified by Svpbmt
 3) Use the page table bits specified by XTheadMae

To support all three methods, the kernel dynamically determines the
definitions of the _PAGE_NOCACHE and _PAGE_IO fields. However, this
alone is not sufficient, as XTheadMae uses a nonzero memory type value
for normal memory pages. So the kernel has an additional alternative
sequence (ALT_THEAD_PMA) to insert the correct memory type when writing
page table entries.

Some RISC-V platforms use a fourth method to specify the cacheability of
a page of RAM: RAM is mapped to multiple physical address ranges, with
each alias having a different set of statically-determined PMAs.
Software selects the PMAs for a page by choosing a PFN from the
corresponding physical address range. This strategy also requires
applying a transformation when writing page table entries. Since these
physical memory aliases should be invisible to the rest of the kernel,
the opposite transformation must be applied when reading page table
entries.

However, with this last method of specifying PMAs, there is no inherent
way to indicate the cacheability of a page in the pgprot_t value, since
the PFN itself determines cacheability. One possible way is to reuse the
PTE bits from Svpbmt, as Svpbmt is the standard extension. This requires
the Svpbmt version of _PAGE_NOCACHE and _PAGE_IO to be available even
when the CPU does not support the extension.

It turns out that with some clever bit manipulation, it is just as
efficient to transform all three Svpbmt memory type values to the
corresponding XTheadMae values, as it is to check for and insert the
one XTheadMae memory type value for normal memory. This allows the
_PAGE_NOCACHE and _PAGE_IO definitions to be compile-time constants,
and it centralizes all memory type handling to one set of ALTERNATIVE
macros.

For a kernel with both Svpbmt and XTheadMae enabled, this change reduces
both kernel text size and the number of alternatives applied at boot.
However, there are a couple of small costs. For platforms using the
first method ("do nothing"), we must mask off the memory type bits when
writing page table entries, whereas previously no action was needed.
Second, when reading page table entries, the XTheadMae values must be
transformed back to the Svpbmt values. This "unfix" operation is also
needed for alias-based PMA selection, so both methods can use the same
ALTERNATIVE.

As a side effect, this change fixes the reporting of the NAPOT and
memory type bits from ptdump on platforms with XTheadMae.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/Kconfig.errata            |   1 +
 arch/riscv/include/asm/errata_list.h |  45 -----------
 arch/riscv/include/asm/pgtable-32.h  |   3 +
 arch/riscv/include/asm/pgtable-64.h  | 108 ++++++++++++++++++---------
 arch/riscv/include/asm/pgtable.h     |  18 +++--
 arch/riscv/mm/ptdump.c               |  13 ++--
 6 files changed, 93 insertions(+), 95 deletions(-)

diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 2acc7d876e1f..2806ed7916c7 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -86,6 +86,7 @@ config ERRATA_THEAD_MAE
 	bool "Apply T-Head's memory attribute extension (XTheadMae) errata"
 	depends on ERRATA_THEAD && 64BIT && MMU
 	select RISCV_ALTERNATIVE_EARLY
+	select RISCV_ISA_SVPBMT
 	default y
 	help
 	  This will apply the memory attribute extension errata to handle the
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 7c8a71a526a3..b127f4891083 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -58,51 +58,6 @@ asm(ALTERNATIVE("sfence.vma %0, %1", "sfence.vma", SIFIVE_VENDOR_ID,	\
 		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
 		: : "r" (addr), "r" (asid) : "memory")
 
-/*
- * _val is marked as "will be overwritten", so need to set it to 0
- * in the default case.
- */
-#define ALT_SVPBMT_SHIFT 61
-#define ALT_THEAD_MAE_SHIFT 59
-#define ALT_SVPBMT(_val, prot)						\
-asm(ALTERNATIVE_2("li %0, 0\t\nnop",					\
-		  "li %0, %1\t\nslli %0,%0,%3", 0,			\
-			RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,	\
-		  "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID,	\
-			ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
-		: "=r"(_val)						\
-		: "I"(prot##_SVPBMT >> ALT_SVPBMT_SHIFT),		\
-		  "I"(prot##_THEAD >> ALT_THEAD_MAE_SHIFT),		\
-		  "I"(ALT_SVPBMT_SHIFT),				\
-		  "I"(ALT_THEAD_MAE_SHIFT))
-
-#ifdef CONFIG_ERRATA_THEAD_MAE
-/*
- * IO/NOCACHE memory types are handled together with svpbmt,
- * so on T-Head chips, check if no other memory type is set,
- * and set the non-0 PMA type if applicable.
- */
-#define ALT_THEAD_PMA(_val)						\
-asm volatile(ALTERNATIVE(						\
-	__nops(7),							\
-	"li      t3, %1\n\t"						\
-	"slli    t3, t3, %3\n\t"					\
-	"and     t3, %0, t3\n\t"					\
-	"bne     t3, zero, 2f\n\t"					\
-	"li      t3, %2\n\t"						\
-	"slli    t3, t3, %3\n\t"					\
-	"or      %0, %0, t3\n\t"					\
-	"2:",  THEAD_VENDOR_ID,						\
-		ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)		\
-	: "+r"(_val)							\
-	: "I"(_PAGE_MTMASK_THEAD >> ALT_THEAD_MAE_SHIFT),		\
-	  "I"(_PAGE_PMA_THEAD >> ALT_THEAD_MAE_SHIFT),			\
-	  "I"(ALT_THEAD_MAE_SHIFT)					\
-	: "t3")
-#else
-#define ALT_THEAD_PMA(_val)
-#endif
-
 #define ALT_CMO_OP(_op, _start, _size, _cachesize)			\
 asm volatile(ALTERNATIVE(						\
 	__nops(5),							\
diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index 7dc0751d67dc..b422a15fb464 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -28,6 +28,9 @@
 #define _PAGE_IO		0
 #define _PAGE_MTMASK		0
 
+#define ALT_FIXUP_MT(_val)
+#define ALT_UNFIX_MT(_val)
+
 #define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
 #define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))
 #define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 4ba88592b8d1..4e8a32f035d7 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -8,7 +8,7 @@
 
 #include <linux/bits.h>
 #include <linux/const.h>
-#include <asm/errata_list.h>
+#include <asm/alternative-macros.h>
 
 extern bool pgtable_l4_enabled;
 extern bool pgtable_l5_enabled;
@@ -109,6 +109,8 @@ enum napot_cont_order {
 #define HUGE_MAX_HSTATE		2
 #endif
 
+#ifdef CONFIG_RISCV_ISA_SVPBMT
+
 /*
  * [62:61] Svpbmt Memory Type definitions:
  *
@@ -117,9 +119,9 @@ enum napot_cont_order {
  *  10 - IO     Non-cacheable, non-idempotent, strongly-ordered I/O memory
  *  11 - Rsvd   Reserved for future standard use
  */
-#define _PAGE_NOCACHE_SVPBMT	(1UL << 61)
-#define _PAGE_IO_SVPBMT		(1UL << 62)
-#define _PAGE_MTMASK_SVPBMT	(_PAGE_NOCACHE_SVPBMT | _PAGE_IO_SVPBMT)
+#define _PAGE_NOCACHE		(1UL << 61)
+#define _PAGE_IO		(2UL << 61)
+#define _PAGE_MTMASK		(3UL << 61)
 
 /*
  * [63:59] T-Head Memory Type definitions:
@@ -128,42 +130,66 @@ enum napot_cont_order {
  * bit[61] B - Bufferable
  * bit[60] SH - Shareable
  * bit[59] Sec - Trustable
- * 00110 - NC   Weakly-ordered, Non-cacheable, Bufferable, Shareable, Non-trustable
  * 01110 - PMA  Weakly-ordered, Cacheable, Bufferable, Shareable, Non-trustable
+ * 00110 - NC   Weakly-ordered, Non-cacheable, Bufferable, Shareable, Non-trustable
  * 10010 - IO   Strongly-ordered, Non-cacheable, Non-bufferable, Shareable, Non-trustable
+ *
+ * ALT_FIXUP_MT translates Svpbmt memory types to XTheadMae memory types.
+ * Pseudocode operating on bits [63:60]:
+ *   t0 = mt << 1
+ *   if (t0 == 0)
+ *     t0 |= 2
+ *   t0 ^= 0x5
+ *   mt ^= t0
+ *
+ * ALT_UNFIX_MT translates XTheadMae memory types to Svpbmt memory types.
+ * Pseudocode operating on bits [63:60]:
+ *   t0 = mt & 0xd
+ *   t0 ^= t0 >> 1
+ *   mt ^= t0
  */
-#define _PAGE_PMA_THEAD		((1UL << 62) | (1UL << 61) | (1UL << 60))
-#define _PAGE_NOCACHE_THEAD	((1UL << 61) | (1UL << 60))
-#define _PAGE_IO_THEAD		((1UL << 63) | (1UL << 60))
-#define _PAGE_MTMASK_THEAD	(_PAGE_PMA_THEAD | _PAGE_IO_THEAD | (1UL << 59))
-
-static inline u64 riscv_page_mtmask(void)
-{
-	u64 val;
-
-	ALT_SVPBMT(val, _PAGE_MTMASK);
-	return val;
-}
 
-static inline u64 riscv_page_nocache(void)
-{
-	u64 val;
+#define ALT_FIXUP_MT(_val)								\
+	asm(ALTERNATIVE_2("addi	t0, zero, 0x3\n\t"					\
+			  "slli	t0, t0, 61\n\t"						\
+			  "not	t0, t0\n\t"						\
+			  "and	%0, %0, t0\n\t"						\
+			  "nop\n\t"							\
+			  "nop\n\t"							\
+			  "nop",							\
+			  __nops(7),							\
+			  0, RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,		\
+			  "srli	t0, %0, 59\n\t"						\
+			  "seqz t1, t0\n\t"						\
+			  "slli	t1, t1, 1\n\t"						\
+			  "or	t0, t0, t1\n\t"						\
+			  "xori	t0, t0, 0x5\n\t"					\
+			  "slli	t0, t0, 60\n\t"						\
+			  "xor	%0, %0, t0",						\
+			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
+			  : "+r" (_val) :: "t0", "t1")
+
+#define ALT_UNFIX_MT(_val)								\
+	asm(ALTERNATIVE(__nops(6),							\
+			  "srli	t0, %0, 60\n\t"						\
+			  "andi	t0, t0, 0xd\n\t"					\
+			  "srli	t1, t0, 1\n\t"						\
+			  "xor	t0, t0, t1\n\t"						\
+			  "slli	t0, t0, 60\n\t"						\
+			  "xor	%0, %0, t0",						\
+			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
+			  : "+r" (_val) :: "t0", "t1")
 
-	ALT_SVPBMT(val, _PAGE_NOCACHE);
-	return val;
-}
+#else
 
-static inline u64 riscv_page_io(void)
-{
-	u64 val;
+#define _PAGE_NOCACHE		0
+#define _PAGE_IO		0
+#define _PAGE_MTMASK		0
 
-	ALT_SVPBMT(val, _PAGE_IO);
-	return val;
-}
+#define ALT_FIXUP_MT(_val)
+#define ALT_UNFIX_MT(_val)
 
-#define _PAGE_NOCACHE		riscv_page_nocache()
-#define _PAGE_IO		riscv_page_io()
-#define _PAGE_MTMASK		riscv_page_mtmask()
+#endif /* CONFIG_RISCV_ISA_SVPBMT */
 
 static inline int pud_present(pud_t pud)
 {
@@ -203,7 +229,11 @@ static inline void pud_clear(pud_t *pudp)
 
 static inline pud_t pfn_pud(unsigned long pfn, pgprot_t prot)
 {
-	return __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
+	pud_t pud = __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
+
+	ALT_FIXUP_MT(pud);
+
+	return pud;
 }
 
 #define pud_pfn pud_pfn
@@ -244,11 +274,11 @@ static inline bool mm_pud_folded(struct mm_struct *mm)
 
 static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
 {
-	unsigned long prot_val = pgprot_val(prot);
+	pmd_t pmd = __pmd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 
-	ALT_THEAD_PMA(prot_val);
+	ALT_FIXUP_MT(pmd);
 
-	return __pmd((pfn << _PAGE_PFN_SHIFT) | prot_val);
+	return pmd;
 }
 
 #define mk_pmd(page, prot)    pfn_pmd(page_to_pfn(page), prot)
@@ -302,7 +332,11 @@ static inline void p4d_clear(p4d_t *p4d)
 
 static inline p4d_t pfn_p4d(unsigned long pfn, pgprot_t prot)
 {
-	return __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
+	p4d_t p4d = __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
+
+	ALT_FIXUP_MT(p4d);
+
+	return p4d;
 }
 
 static inline unsigned long p4d_pfn(p4d_t p4d)
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index afa0b455eaa4..3ffcff76ac0d 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -257,11 +257,11 @@ static inline void pmd_clear(pmd_t *pmdp)
 
 static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
 {
-	unsigned long prot_val = pgprot_val(prot);
+	pgd_t pgd = __pgd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 
-	ALT_THEAD_PMA(prot_val);
+	ALT_FIXUP_MT(pgd);
 
-	return __pgd((pfn << _PAGE_PFN_SHIFT) | prot_val);
+	return pgd;
 }
 
 static inline unsigned long pmd_pfn(pmd_t pmd)
@@ -338,11 +338,11 @@ static inline unsigned long pte_pfn(pte_t pte)
 /* Constructs a page table entry */
 static inline pte_t pfn_pte(unsigned long pfn, pgprot_t prot)
 {
-	unsigned long prot_val = pgprot_val(prot);
+	pte_t pte = __pte((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 
-	ALT_THEAD_PMA(prot_val);
+	ALT_FIXUP_MT(pte);
 
-	return __pte((pfn << _PAGE_PFN_SHIFT) | prot_val);
+	return pte;
 }
 
 #define mk_pte(page, prot)       pfn_pte(page_to_pfn(page), prot)
@@ -489,9 +489,11 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 {
 	unsigned long newprot_val = pgprot_val(newprot);
 
-	ALT_THEAD_PMA(newprot_val);
+	ALT_UNFIX_MT(pte);
+	pte = __pte((pte_val(pte) & _PAGE_CHG_MASK) | newprot_val);
+	ALT_FIXUP_MT(pte);
 
-	return __pte((pte_val(pte) & _PAGE_CHG_MASK) | newprot_val);
+	return pte;
 }
 
 #define pgd_ERROR(e) \
diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
index 58a7322e9a82..6528c2561437 100644
--- a/arch/riscv/mm/ptdump.c
+++ b/arch/riscv/mm/ptdump.c
@@ -142,7 +142,7 @@ static const struct prot_bits pte_bits[] = {
 	}, {
 #endif
 #ifdef CONFIG_RISCV_ISA_SVPBMT
-		.mask = _PAGE_MTMASK_SVPBMT,
+		.mask = _PAGE_MTMASK,
 		.set = "MT(%s)",
 		.clear = "  ..  ",
 	}, {
@@ -218,10 +218,10 @@ static void dump_prot(struct pg_state *st)
 			if (pte_bits[i].mask == _PAGE_SOFT)
 				sprintf(s, pte_bits[i].set, val >> 8);
 #ifdef CONFIG_RISCV_ISA_SVPBMT
-			else if (pte_bits[i].mask == _PAGE_MTMASK_SVPBMT) {
-				if (val == _PAGE_NOCACHE_SVPBMT)
+			else if (pte_bits[i].mask == _PAGE_MTMASK) {
+				if (val == _PAGE_NOCACHE)
 					sprintf(s, pte_bits[i].set, "NC");
-				else if (val == _PAGE_IO_SVPBMT)
+				else if (val == _PAGE_IO)
 					sprintf(s, pte_bits[i].set, "IO");
 				else
 					sprintf(s, pte_bits[i].set, "??");
@@ -282,9 +282,12 @@ static void note_page(struct ptdump_state *pt_st, unsigned long addr,
 		      int level, u64 val)
 {
 	struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
-	u64 pa = PFN_PHYS(pte_pfn(__pte(val)));
 	u64 prot = 0;
+	u64 pa;
 
+	ALT_UNFIX_MT(val);
+
+	pa = PFN_PHYS(pte_pfn(__pte(val)));
 	if (level >= 0)
 		prot = val & pg_level[level].mask;
 
-- 
2.45.1

Re: [PATCH 06/11] riscv: mm: Fix up memory types when writing page tables

[Alexandre Ghiti]

On 02/11/2024 01:08, Samuel Holland wrote:
> Currently, Linux on RISC-V has three ways to specify the cacheability
> and ordering PMAs of a page:
>   1) Do nothing; assume the system is entirely cache-coherent and rely on
>      the hardware for any ordering requirements
>   2) Use the page table bits specified by Svpbmt
>   3) Use the page table bits specified by XTheadMae
>
> To support all three methods, the kernel dynamically determines the
> definitions of the _PAGE_NOCACHE and _PAGE_IO fields. However, this
> alone is not sufficient, as XTheadMae uses a nonzero memory type value
> for normal memory pages. So the kernel has an additional alternative
> sequence (ALT_THEAD_PMA) to insert the correct memory type when writing
> page table entries.


I have just taken a look, and it's not exactly when the page table is 
written but rather when the page table entry is being created.

And I have to admit that I find it weird, moving that to the set_pXd() 
functions seems way more robust. Indeed those functions must be used to 
write a page table entry but a page table entry can be created by other 
means than with the pfn_pXd() functions.

This is what I did for NAPOT to hide the size of the mapping contained 
in the pfn from the generic kernel here 
https://lore.kernel.org/linux-riscv/20240802151430.99114-1-alexghiti@rivosinc.com/


>
> Some RISC-V platforms use a fourth method to specify the cacheability of
> a page of RAM: RAM is mapped to multiple physical address ranges, with
> each alias having a different set of statically-determined PMAs.
> Software selects the PMAs for a page by choosing a PFN from the
> corresponding physical address range. This strategy also requires
> applying a transformation when writing page table entries. Since these
> physical memory aliases should be invisible to the rest of the kernel,
> the opposite transformation must be applied when reading page table
> entries.
>
> However, with this last method of specifying PMAs, there is no inherent
> way to indicate the cacheability of a page in the pgprot_t value, since
> the PFN itself determines cacheability. One possible way is to reuse the
> PTE bits from Svpbmt, as Svpbmt is the standard extension. This requires
> the Svpbmt version of _PAGE_NOCACHE and _PAGE_IO to be available even
> when the CPU does not support the extension.
>
> It turns out that with some clever bit manipulation, it is just as
> efficient to transform all three Svpbmt memory type values to the
> corresponding XTheadMae values, as it is to check for and insert the
> one XTheadMae memory type value for normal memory. This allows the
> _PAGE_NOCACHE and _PAGE_IO definitions to be compile-time constants,
> and it centralizes all memory type handling to one set of ALTERNATIVE
> macros.
>
> For a kernel with both Svpbmt and XTheadMae enabled, this change reduces
> both kernel text size and the number of alternatives applied at boot.
> However, there are a couple of small costs. For platforms using the
> first method ("do nothing"), we must mask off the memory type bits when
> writing page table entries, whereas previously no action was needed.
> Second, when reading page table entries, the XTheadMae values must be
> transformed back to the Svpbmt values. This "unfix" operation is also
> needed for alias-based PMA selection, so both methods can use the same
> ALTERNATIVE.
>
> As a side effect, this change fixes the reporting of the NAPOT and
> memory type bits from ptdump on platforms with XTheadMae.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>   arch/riscv/Kconfig.errata            |   1 +
>   arch/riscv/include/asm/errata_list.h |  45 -----------
>   arch/riscv/include/asm/pgtable-32.h  |   3 +
>   arch/riscv/include/asm/pgtable-64.h  | 108 ++++++++++++++++++---------
>   arch/riscv/include/asm/pgtable.h     |  18 +++--
>   arch/riscv/mm/ptdump.c               |  13 ++--
>   6 files changed, 93 insertions(+), 95 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 2acc7d876e1f..2806ed7916c7 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -86,6 +86,7 @@ config ERRATA_THEAD_MAE
>   	bool "Apply T-Head's memory attribute extension (XTheadMae) errata"
>   	depends on ERRATA_THEAD && 64BIT && MMU
>   	select RISCV_ALTERNATIVE_EARLY
> +	select RISCV_ISA_SVPBMT
>   	default y
>   	help
>   	  This will apply the memory attribute extension errata to handle the
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 7c8a71a526a3..b127f4891083 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -58,51 +58,6 @@ asm(ALTERNATIVE("sfence.vma %0, %1", "sfence.vma", SIFIVE_VENDOR_ID,	\
>   		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
>   		: : "r" (addr), "r" (asid) : "memory")
>   
> -/*
> - * _val is marked as "will be overwritten", so need to set it to 0
> - * in the default case.
> - */
> -#define ALT_SVPBMT_SHIFT 61
> -#define ALT_THEAD_MAE_SHIFT 59
> -#define ALT_SVPBMT(_val, prot)						\
> -asm(ALTERNATIVE_2("li %0, 0\t\nnop",					\
> -		  "li %0, %1\t\nslli %0,%0,%3", 0,			\
> -			RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,	\
> -		  "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID,	\
> -			ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
> -		: "=r"(_val)						\
> -		: "I"(prot##_SVPBMT >> ALT_SVPBMT_SHIFT),		\
> -		  "I"(prot##_THEAD >> ALT_THEAD_MAE_SHIFT),		\
> -		  "I"(ALT_SVPBMT_SHIFT),				\
> -		  "I"(ALT_THEAD_MAE_SHIFT))
> -
> -#ifdef CONFIG_ERRATA_THEAD_MAE
> -/*
> - * IO/NOCACHE memory types are handled together with svpbmt,
> - * so on T-Head chips, check if no other memory type is set,
> - * and set the non-0 PMA type if applicable.
> - */
> -#define ALT_THEAD_PMA(_val)						\
> -asm volatile(ALTERNATIVE(						\
> -	__nops(7),							\
> -	"li      t3, %1\n\t"						\
> -	"slli    t3, t3, %3\n\t"					\
> -	"and     t3, %0, t3\n\t"					\
> -	"bne     t3, zero, 2f\n\t"					\
> -	"li      t3, %2\n\t"						\
> -	"slli    t3, t3, %3\n\t"					\
> -	"or      %0, %0, t3\n\t"					\
> -	"2:",  THEAD_VENDOR_ID,						\
> -		ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)		\
> -	: "+r"(_val)							\
> -	: "I"(_PAGE_MTMASK_THEAD >> ALT_THEAD_MAE_SHIFT),		\
> -	  "I"(_PAGE_PMA_THEAD >> ALT_THEAD_MAE_SHIFT),			\
> -	  "I"(ALT_THEAD_MAE_SHIFT)					\
> -	: "t3")
> -#else
> -#define ALT_THEAD_PMA(_val)
> -#endif
> -
>   #define ALT_CMO_OP(_op, _start, _size, _cachesize)			\
>   asm volatile(ALTERNATIVE(						\
>   	__nops(5),							\
> diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
> index 7dc0751d67dc..b422a15fb464 100644
> --- a/arch/riscv/include/asm/pgtable-32.h
> +++ b/arch/riscv/include/asm/pgtable-32.h
> @@ -28,6 +28,9 @@
>   #define _PAGE_IO		0
>   #define _PAGE_MTMASK		0
>   
> +#define ALT_FIXUP_MT(_val)
> +#define ALT_UNFIX_MT(_val)
> +
>   #define pud_pfn(pud)				(pmd_pfn((pmd_t){ pud }))
>   #define p4d_pfn(p4d)				(pud_pfn((pud_t){ p4d }))
>   #define pgd_pfn(pgd)				(p4d_pfn((p4d_t){ pgd }))
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 4ba88592b8d1..4e8a32f035d7 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -8,7 +8,7 @@
>   
>   #include <linux/bits.h>
>   #include <linux/const.h>
> -#include <asm/errata_list.h>
> +#include <asm/alternative-macros.h>
>   
>   extern bool pgtable_l4_enabled;
>   extern bool pgtable_l5_enabled;
> @@ -109,6 +109,8 @@ enum napot_cont_order {
>   #define HUGE_MAX_HSTATE		2
>   #endif
>   
> +#ifdef CONFIG_RISCV_ISA_SVPBMT
> +
>   /*
>    * [62:61] Svpbmt Memory Type definitions:
>    *
> @@ -117,9 +119,9 @@ enum napot_cont_order {
>    *  10 - IO     Non-cacheable, non-idempotent, strongly-ordered I/O memory
>    *  11 - Rsvd   Reserved for future standard use
>    */
> -#define _PAGE_NOCACHE_SVPBMT	(1UL << 61)
> -#define _PAGE_IO_SVPBMT		(1UL << 62)
> -#define _PAGE_MTMASK_SVPBMT	(_PAGE_NOCACHE_SVPBMT | _PAGE_IO_SVPBMT)
> +#define _PAGE_NOCACHE		(1UL << 61)
> +#define _PAGE_IO		(2UL << 61)
> +#define _PAGE_MTMASK		(3UL << 61)
>   
>   /*
>    * [63:59] T-Head Memory Type definitions:
> @@ -128,42 +130,66 @@ enum napot_cont_order {
>    * bit[61] B - Bufferable
>    * bit[60] SH - Shareable
>    * bit[59] Sec - Trustable
> - * 00110 - NC   Weakly-ordered, Non-cacheable, Bufferable, Shareable, Non-trustable
>    * 01110 - PMA  Weakly-ordered, Cacheable, Bufferable, Shareable, Non-trustable
> + * 00110 - NC   Weakly-ordered, Non-cacheable, Bufferable, Shareable, Non-trustable
>    * 10010 - IO   Strongly-ordered, Non-cacheable, Non-bufferable, Shareable, Non-trustable
> + *
> + * ALT_FIXUP_MT translates Svpbmt memory types to XTheadMae memory types.
> + * Pseudocode operating on bits [63:60]:
> + *   t0 = mt << 1
> + *   if (t0 == 0)
> + *     t0 |= 2
> + *   t0 ^= 0x5
> + *   mt ^= t0
> + *
> + * ALT_UNFIX_MT translates XTheadMae memory types to Svpbmt memory types.
> + * Pseudocode operating on bits [63:60]:
> + *   t0 = mt & 0xd
> + *   t0 ^= t0 >> 1
> + *   mt ^= t0
>    */
> -#define _PAGE_PMA_THEAD		((1UL << 62) | (1UL << 61) | (1UL << 60))
> -#define _PAGE_NOCACHE_THEAD	((1UL << 61) | (1UL << 60))
> -#define _PAGE_IO_THEAD		((1UL << 63) | (1UL << 60))
> -#define _PAGE_MTMASK_THEAD	(_PAGE_PMA_THEAD | _PAGE_IO_THEAD | (1UL << 59))
> -
> -static inline u64 riscv_page_mtmask(void)
> -{
> -	u64 val;
> -
> -	ALT_SVPBMT(val, _PAGE_MTMASK);
> -	return val;
> -}
>   
> -static inline u64 riscv_page_nocache(void)
> -{
> -	u64 val;
> +#define ALT_FIXUP_MT(_val)								\
> +	asm(ALTERNATIVE_2("addi	t0, zero, 0x3\n\t"					\
> +			  "slli	t0, t0, 61\n\t"						\
> +			  "not	t0, t0\n\t"						\
> +			  "and	%0, %0, t0\n\t"						\
> +			  "nop\n\t"							\
> +			  "nop\n\t"							\
> +			  "nop",							\
> +			  __nops(7),							\
> +			  0, RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,		\
> +			  "srli	t0, %0, 59\n\t"						\
> +			  "seqz t1, t0\n\t"						\
> +			  "slli	t1, t1, 1\n\t"						\
> +			  "or	t0, t0, t1\n\t"						\
> +			  "xori	t0, t0, 0x5\n\t"					\
> +			  "slli	t0, t0, 60\n\t"						\
> +			  "xor	%0, %0, t0",						\
> +			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
> +			  : "+r" (_val) :: "t0", "t1")
> +
> +#define ALT_UNFIX_MT(_val)								\
> +	asm(ALTERNATIVE(__nops(6),							\
> +			  "srli	t0, %0, 60\n\t"						\
> +			  "andi	t0, t0, 0xd\n\t"					\
> +			  "srli	t1, t0, 1\n\t"						\
> +			  "xor	t0, t0, t1\n\t"						\
> +			  "slli	t0, t0, 60\n\t"						\
> +			  "xor	%0, %0, t0",						\
> +			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
> +			  : "+r" (_val) :: "t0", "t1")
>   
> -	ALT_SVPBMT(val, _PAGE_NOCACHE);
> -	return val;
> -}
> +#else
>   
> -static inline u64 riscv_page_io(void)
> -{
> -	u64 val;
> +#define _PAGE_NOCACHE		0
> +#define _PAGE_IO		0
> +#define _PAGE_MTMASK		0
>   
> -	ALT_SVPBMT(val, _PAGE_IO);
> -	return val;
> -}
> +#define ALT_FIXUP_MT(_val)
> +#define ALT_UNFIX_MT(_val)
>   
> -#define _PAGE_NOCACHE		riscv_page_nocache()
> -#define _PAGE_IO		riscv_page_io()
> -#define _PAGE_MTMASK		riscv_page_mtmask()
> +#endif /* CONFIG_RISCV_ISA_SVPBMT */
>   
>   static inline int pud_present(pud_t pud)
>   {
> @@ -203,7 +229,11 @@ static inline void pud_clear(pud_t *pudp)
>   
>   static inline pud_t pfn_pud(unsigned long pfn, pgprot_t prot)
>   {
> -	return __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
> +	pud_t pud = __pud((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
> +
> +	ALT_FIXUP_MT(pud);
> +
> +	return pud;
>   }
>   
>   #define pud_pfn pud_pfn
> @@ -244,11 +274,11 @@ static inline bool mm_pud_folded(struct mm_struct *mm)
>   
>   static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
>   {
> -	unsigned long prot_val = pgprot_val(prot);
> +	pmd_t pmd = __pmd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>   
> -	ALT_THEAD_PMA(prot_val);
> +	ALT_FIXUP_MT(pmd);
>   
> -	return __pmd((pfn << _PAGE_PFN_SHIFT) | prot_val);
> +	return pmd;
>   }
>   
>   #define mk_pmd(page, prot)    pfn_pmd(page_to_pfn(page), prot)
> @@ -302,7 +332,11 @@ static inline void p4d_clear(p4d_t *p4d)
>   
>   static inline p4d_t pfn_p4d(unsigned long pfn, pgprot_t prot)
>   {
> -	return __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
> +	p4d_t p4d = __p4d((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
> +
> +	ALT_FIXUP_MT(p4d);
> +
> +	return p4d;
>   }
>   
>   static inline unsigned long p4d_pfn(p4d_t p4d)
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index afa0b455eaa4..3ffcff76ac0d 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -257,11 +257,11 @@ static inline void pmd_clear(pmd_t *pmdp)
>   
>   static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
>   {
> -	unsigned long prot_val = pgprot_val(prot);
> +	pgd_t pgd = __pgd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>   
> -	ALT_THEAD_PMA(prot_val);
> +	ALT_FIXUP_MT(pgd);
>   
> -	return __pgd((pfn << _PAGE_PFN_SHIFT) | prot_val);
> +	return pgd;
>   }
>   
>   static inline unsigned long pmd_pfn(pmd_t pmd)
> @@ -338,11 +338,11 @@ static inline unsigned long pte_pfn(pte_t pte)
>   /* Constructs a page table entry */
>   static inline pte_t pfn_pte(unsigned long pfn, pgprot_t prot)
>   {
> -	unsigned long prot_val = pgprot_val(prot);
> +	pte_t pte = __pte((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>   
> -	ALT_THEAD_PMA(prot_val);
> +	ALT_FIXUP_MT(pte);
>   
> -	return __pte((pfn << _PAGE_PFN_SHIFT) | prot_val);
> +	return pte;
>   }
>   
>   #define mk_pte(page, prot)       pfn_pte(page_to_pfn(page), prot)
> @@ -489,9 +489,11 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>   {
>   	unsigned long newprot_val = pgprot_val(newprot);
>   
> -	ALT_THEAD_PMA(newprot_val);
> +	ALT_UNFIX_MT(pte);
> +	pte = __pte((pte_val(pte) & _PAGE_CHG_MASK) | newprot_val);
> +	ALT_FIXUP_MT(pte);
>   
> -	return __pte((pte_val(pte) & _PAGE_CHG_MASK) | newprot_val);
> +	return pte;
>   }
>   
>   #define pgd_ERROR(e) \
> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
> index 58a7322e9a82..6528c2561437 100644
> --- a/arch/riscv/mm/ptdump.c
> +++ b/arch/riscv/mm/ptdump.c
> @@ -142,7 +142,7 @@ static const struct prot_bits pte_bits[] = {
>   	}, {
>   #endif
>   #ifdef CONFIG_RISCV_ISA_SVPBMT
> -		.mask = _PAGE_MTMASK_SVPBMT,
> +		.mask = _PAGE_MTMASK,
>   		.set = "MT(%s)",
>   		.clear = "  ..  ",
>   	}, {
> @@ -218,10 +218,10 @@ static void dump_prot(struct pg_state *st)
>   			if (pte_bits[i].mask == _PAGE_SOFT)
>   				sprintf(s, pte_bits[i].set, val >> 8);
>   #ifdef CONFIG_RISCV_ISA_SVPBMT
> -			else if (pte_bits[i].mask == _PAGE_MTMASK_SVPBMT) {
> -				if (val == _PAGE_NOCACHE_SVPBMT)
> +			else if (pte_bits[i].mask == _PAGE_MTMASK) {
> +				if (val == _PAGE_NOCACHE)
>   					sprintf(s, pte_bits[i].set, "NC");
> -				else if (val == _PAGE_IO_SVPBMT)
> +				else if (val == _PAGE_IO)
>   					sprintf(s, pte_bits[i].set, "IO");
>   				else
>   					sprintf(s, pte_bits[i].set, "??");
> @@ -282,9 +282,12 @@ static void note_page(struct ptdump_state *pt_st, unsigned long addr,
>   		      int level, u64 val)
>   {
>   	struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
> -	u64 pa = PFN_PHYS(pte_pfn(__pte(val)));
>   	u64 prot = 0;
> +	u64 pa;
>   
> +	ALT_UNFIX_MT(val);
> +
> +	pa = PFN_PHYS(pte_pfn(__pte(val)));
>   	if (level >= 0)
>   		prot = val & pg_level[level].mask;
>   

Re: [PATCH 06/11] riscv: mm: Fix up memory types when writing page tables

[Samuel Holland]

Hi Alex,

On 2024-11-05 5:03 AM, Alexandre Ghiti wrote:
> On 02/11/2024 01:08, Samuel Holland wrote:
>> Currently, Linux on RISC-V has three ways to specify the cacheability
>> and ordering PMAs of a page:
>>   1) Do nothing; assume the system is entirely cache-coherent and rely on
>>      the hardware for any ordering requirements
>>   2) Use the page table bits specified by Svpbmt
>>   3) Use the page table bits specified by XTheadMae
>>
>> To support all three methods, the kernel dynamically determines the
>> definitions of the _PAGE_NOCACHE and _PAGE_IO fields. However, this
>> alone is not sufficient, as XTheadMae uses a nonzero memory type value
>> for normal memory pages. So the kernel has an additional alternative
>> sequence (ALT_THEAD_PMA) to insert the correct memory type when writing
>> page table entries.
> 
> 
> I have just taken a look, and it's not exactly when the page table is written
> but rather when the page table entry is being created.
> 
> And I have to admit that I find it weird, moving that to the set_pXd() functions
> seems way more robust. Indeed those functions must be used to write a page table
> entry but a page table entry can be created by other means than with the
> pfn_pXd() functions.
> 
> This is what I did for NAPOT to hide the size of the mapping contained in the
> pfn from the generic kernel here https://lore.kernel.org/linux-
> riscv/20240802151430.99114-1-alexghiti@rivosinc.com/

I've just sent out a v2 which follows your suggestion[1]. It turns out that the
pXXp_get()/set_pXX() functions aren't 100% robust either, with some examples
shown in patch 5 of the new series[2]. And there's some ongoing discussion about
if it's appropriate to put nontrivial code in those helper functions[3]. So I
don't know what's really the best strategy here.

Regards,
Samuel

[1]:
https://lore.kernel.org/linux-riscv/20251009015839.3460231-1-samuel.holland@sifive.com/
[2]:
https://lore.kernel.org/linux-riscv/20251009015839.3460231-6-samuel.holland@sifive.com/
[3]:
https://lore.kernel.org/all/20251006082238.GQ3245006@noisy.programming.kicks-ass.net/

[PATCH 07/11] riscv: mm: Expose all page table bits to assembly code

[Samuel Holland]

pgtable-32.h and pgtable-64.h are not usable by assembly code, so move
all page table field definitions to pgtable-bits.h.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/pgtable-32.h   | 11 -------
 arch/riscv/include/asm/pgtable-64.h   | 30 -------------------
 arch/riscv/include/asm/pgtable-bits.h | 42 +++++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index b422a15fb464..ba50b65b434b 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -17,17 +17,6 @@
 
 #define MAX_POSSIBLE_PHYSMEM_BITS 34
 
-/*
- * rv32 PTE format:
- * | XLEN-1  10 | 9             8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
- *       PFN      reserved for SW   D   A   G   U   X   W   R   V
- */
-#define _PAGE_PFN_MASK  GENMASK(31, 10)
-
-#define _PAGE_NOCACHE		0
-#define _PAGE_IO		0
-#define _PAGE_MTMASK		0
-
 #define ALT_FIXUP_MT(_val)
 #define ALT_UNFIX_MT(_val)
 
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 4e8a32f035d7..174b6a5837c2 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -68,20 +68,6 @@ typedef struct {
 #define __pmd(x)        ((pmd_t) { (x) })
 #define PTRS_PER_PMD    (PAGE_SIZE / sizeof(pmd_t))
 
-/*
- * rv64 PTE format:
- * | 63 | 62 61 | 60 54 | 53  10 | 9             8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
- *   N      MT     RSV    PFN      reserved for SW   D   A   G   U   X   W   R   V
- */
-#define _PAGE_PFN_MASK  GENMASK(53, 10)
-
-/*
- * [63] Svnapot definitions:
- * 0 Svnapot disabled
- * 1 Svnapot enabled
- */
-#define _PAGE_NAPOT_SHIFT	63
-#define _PAGE_NAPOT		BIT(_PAGE_NAPOT_SHIFT)
 /*
  * Only 64KB (order 4) napot ptes supported.
  */
@@ -111,18 +97,6 @@ enum napot_cont_order {
 
 #ifdef CONFIG_RISCV_ISA_SVPBMT
 
-/*
- * [62:61] Svpbmt Memory Type definitions:
- *
- *  00 - PMA    Normal Cacheable, No change to implied PMA memory type
- *  01 - NC     Non-cacheable, idempotent, weakly-ordered Main Memory
- *  10 - IO     Non-cacheable, non-idempotent, strongly-ordered I/O memory
- *  11 - Rsvd   Reserved for future standard use
- */
-#define _PAGE_NOCACHE		(1UL << 61)
-#define _PAGE_IO		(2UL << 61)
-#define _PAGE_MTMASK		(3UL << 61)
-
 /*
  * [63:59] T-Head Memory Type definitions:
  * bit[63] SO - Strong Order
@@ -182,10 +156,6 @@ enum napot_cont_order {
 
 #else
 
-#define _PAGE_NOCACHE		0
-#define _PAGE_IO		0
-#define _PAGE_MTMASK		0
-
 #define ALT_FIXUP_MT(_val)
 #define ALT_UNFIX_MT(_val)
 
diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
index a8f5205cea54..96710d4c1817 100644
--- a/arch/riscv/include/asm/pgtable-bits.h
+++ b/arch/riscv/include/asm/pgtable-bits.h
@@ -6,6 +6,16 @@
 #ifndef _ASM_RISCV_PGTABLE_BITS_H
 #define _ASM_RISCV_PGTABLE_BITS_H
 
+/*
+ * rv32 PTE format:
+ * | XLEN-1  10 | 9             8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
+ *       PFN      reserved for SW   D   A   G   U   X   W   R   V
+ *
+ * rv64 PTE format:
+ * | 63 | 62 61 | 60 54 | 53 10 | 9             8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
+ *   N      MT     RSV     PFN    reserved for SW   D   A   G   U   X   W   R   V
+ */
+
 #define _PAGE_ACCESSED_OFFSET 6
 
 #define _PAGE_PRESENT   (1 << 0)
@@ -22,6 +32,36 @@
 #define _PAGE_DEVMAP    (1 << 9)    /* RSW, devmap */
 #define _PAGE_TABLE     _PAGE_PRESENT
 
+#define _PAGE_PFN_SHIFT		10
+#ifdef CONFIG_64BIT
+#define _PAGE_PFN_MASK		GENMASK(53, 10)
+#else
+#define _PAGE_PFN_MASK		GENMASK(31, 10)
+#endif /* CONFIG_64BIT */
+
+#ifdef CONFIG_RISCV_ISA_SVPBMT
+/*
+ * [62:61] Svpbmt Memory Type definitions:
+ *
+ *  00 - PMA    Normal Cacheable, No change to implied PMA memory type
+ *  01 - NC     Non-cacheable, idempotent, weakly-ordered Main Memory
+ *  10 - IO     Non-cacheable, non-idempotent, strongly-ordered I/O memory
+ *  11 - Rsvd   Reserved for future standard use
+ */
+#define _PAGE_NOCACHE		(UL(1) << 61)
+#define _PAGE_IO		(UL(2) << 61)
+#define _PAGE_MTMASK		(UL(3) << 61)
+#else
+#define _PAGE_NOCACHE		0
+#define _PAGE_IO		0
+#define _PAGE_MTMASK		0
+#endif /* CONFIG_RISCV_ISA_SVPBMT */
+
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
+#define _PAGE_NAPOT_SHIFT	63
+#define _PAGE_NAPOT		BIT(_PAGE_NAPOT_SHIFT)
+#endif /* CONFIG_RISCV_ISA_SVNAPOT */
+
 /*
  * _PAGE_PROT_NONE is set on not-present pages (and ignored by the hardware) to
  * distinguish them from swapped out pages
@@ -31,8 +71,6 @@
 /* Used for swap PTEs only. */
 #define _PAGE_SWP_EXCLUSIVE _PAGE_ACCESSED
 
-#define _PAGE_PFN_SHIFT 10
-
 /*
  * when all of R/W/X are zero, the PTE is a pointer to the next level
  * of the page table; otherwise, it is a leaf PTE.
-- 
2.45.1

[PATCH 08/11] riscv: alternative: Add an ALTERNATIVE_3 macro

[Samuel Holland]

ALT_FIXUP_PMA() is already using ALTERNATIVE_2(), but needs to be
extended to handle a fourth case. Add ALTERNATIVE_3(), which extends
ALTERNATIVE_2() with another block of new content.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/alternative-macros.h | 45 ++++++++++++++++++---
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
index 721ec275ce57..b6027a8b6b50 100644
--- a/arch/riscv/include/asm/alternative-macros.h
+++ b/arch/riscv/include/asm/alternative-macros.h
@@ -50,8 +50,17 @@
 	ALT_NEW_CONTENT \vendor_id_2, \patch_id_2, \enable_2, "\new_c_2"
 .endm
 
+.macro ALTERNATIVE_CFG_3 old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
+				new_c_2, vendor_id_2, patch_id_2, enable_2,	\
+				new_c_3, vendor_id_3, patch_id_3, enable_3
+	ALTERNATIVE_CFG_2 "\old_c", "\new_c_1", \vendor_id_1, \patch_id_1, \enable_1 \
+				    "\new_c_2", \vendor_id_2, \patch_id_2, \enable_2 \
+	ALT_NEW_CONTENT \vendor_id_3, \patch_id_3, \enable_3, "\new_c_3"
+.endm
+
 #define __ALTERNATIVE_CFG(...)		ALTERNATIVE_CFG __VA_ARGS__
 #define __ALTERNATIVE_CFG_2(...)	ALTERNATIVE_CFG_2 __VA_ARGS__
+#define __ALTERNATIVE_CFG_3(...)	ALTERNATIVE_CFG_3 __VA_ARGS__
 
 #else /* !__ASSEMBLY__ */
 
@@ -98,6 +107,13 @@
 	__ALTERNATIVE_CFG(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1)	\
 	ALT_NEW_CONTENT(vendor_id_2, patch_id_2, enable_2, new_c_2)
 
+#define __ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
+				   new_c_2, vendor_id_2, patch_id_2, enable_2,	\
+				   new_c_3, vendor_id_3, patch_id_3, enable_3)	\
+	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
+				   new_c_2, vendor_id_2, patch_id_2, enable_2)	\
+	ALT_NEW_CONTENT(vendor_id_3, patch_id_3, enable_3, new_c_3)
+
 #endif /* __ASSEMBLY__ */
 
 #define _ALTERNATIVE_CFG(old_c, new_c, vendor_id, patch_id, CONFIG_k)	\
@@ -108,6 +124,13 @@
 	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, patch_id_1, IS_ENABLED(CONFIG_k_1),	\
 				   new_c_2, vendor_id_2, patch_id_2, IS_ENABLED(CONFIG_k_2))
 
+#define _ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
+				  new_c_2, vendor_id_2, patch_id_2, CONFIG_k_2,		\
+				  new_c_3, vendor_id_3, patch_id_3, CONFIG_k_3)		\
+	__ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, IS_ENABLED(CONFIG_k_1),	\
+				   new_c_2, vendor_id_2, patch_id_2, IS_ENABLED(CONFIG_k_2),	\
+				   new_c_3, vendor_id_3, patch_id_3, IS_ENABLED(CONFIG_k_3))
+
 #else /* CONFIG_RISCV_ALTERNATIVE */
 #ifdef __ASSEMBLY__
 
@@ -121,6 +144,9 @@
 #define _ALTERNATIVE_CFG_2(old_c, ...)	\
 	ALTERNATIVE_CFG old_c
 
+#define _ALTERNATIVE_CFG_3(old_c, ...)	\
+	ALTERNATIVE_CFG old_c
+
 #else /* !__ASSEMBLY__ */
 
 #define __ALTERNATIVE_CFG(old_c)	\
@@ -132,6 +158,9 @@
 #define _ALTERNATIVE_CFG_2(old_c, ...)	\
 	__ALTERNATIVE_CFG(old_c)
 
+#define _ALTERNATIVE_CFG_3(old_c, ...)	\
+	__ALTERNATIVE_CFG(old_c)
+
 #endif /* __ASSEMBLY__ */
 #endif /* CONFIG_RISCV_ALTERNATIVE */
 
@@ -152,15 +181,21 @@
 	_ALTERNATIVE_CFG(old_content, new_content, vendor_id, patch_id, CONFIG_k)
 
 /*
- * A vendor wants to replace an old_content, but another vendor has used
- * ALTERNATIVE() to patch its customized content at the same location. In
- * this case, this vendor can create a new macro ALTERNATIVE_2() based
- * on the following sample code and then replace ALTERNATIVE() with
- * ALTERNATIVE_2() to append its customized content.
+ * Variant of ALTERNATIVE() that supports two sets of replacement content.
  */
 #define ALTERNATIVE_2(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
 				   new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2)		\
 	_ALTERNATIVE_CFG_2(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,	\
 					new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2)
 
+/*
+ * Variant of ALTERNATIVE() that supports three sets of replacement content.
+ */
+#define ALTERNATIVE_3(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
+				   new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2,		\
+				   new_content_3, vendor_id_3, patch_id_3, CONFIG_k_3)		\
+	_ALTERNATIVE_CFG_3(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,	\
+					new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2,	\
+					new_content_3, vendor_id_3, patch_id_3, CONFIG_k_3)
+
 #endif
-- 
2.45.1

Re: [PATCH 08/11] riscv: alternative: Add an ALTERNATIVE_3 macro

[Andrew Jones]

On Fri, Nov 01, 2024 at 05:08:02PM -0700, Samuel Holland wrote:
> ALT_FIXUP_PMA() is already using ALTERNATIVE_2(), but needs to be
> extended to handle a fourth case. Add ALTERNATIVE_3(), which extends
> ALTERNATIVE_2() with another block of new content.
> 
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  arch/riscv/include/asm/alternative-macros.h | 45 ++++++++++++++++++---
>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index 721ec275ce57..b6027a8b6b50 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -50,8 +50,17 @@
>  	ALT_NEW_CONTENT \vendor_id_2, \patch_id_2, \enable_2, "\new_c_2"
>  .endm
>  
> +.macro ALTERNATIVE_CFG_3 old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
> +				new_c_2, vendor_id_2, patch_id_2, enable_2,	\
> +				new_c_3, vendor_id_3, patch_id_3, enable_3
> +	ALTERNATIVE_CFG_2 "\old_c", "\new_c_1", \vendor_id_1, \patch_id_1, \enable_1 \
> +				    "\new_c_2", \vendor_id_2, \patch_id_2, \enable_2 \

We don't want the '\' on the end of the above line.

> +	ALT_NEW_CONTENT \vendor_id_3, \patch_id_3, \enable_3, "\new_c_3"
> +.endm
> +
>  #define __ALTERNATIVE_CFG(...)		ALTERNATIVE_CFG __VA_ARGS__
>  #define __ALTERNATIVE_CFG_2(...)	ALTERNATIVE_CFG_2 __VA_ARGS__
> +#define __ALTERNATIVE_CFG_3(...)	ALTERNATIVE_CFG_3 __VA_ARGS__
>  
>  #else /* !__ASSEMBLY__ */
>  
> @@ -98,6 +107,13 @@
>  	__ALTERNATIVE_CFG(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1)	\
>  	ALT_NEW_CONTENT(vendor_id_2, patch_id_2, enable_2, new_c_2)
>  
> +#define __ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
> +				   new_c_2, vendor_id_2, patch_id_2, enable_2,	\
> +				   new_c_3, vendor_id_3, patch_id_3, enable_3)	\
> +	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, patch_id_1, enable_1,	\
> +				   new_c_2, vendor_id_2, patch_id_2, enable_2)	\
> +	ALT_NEW_CONTENT(vendor_id_3, patch_id_3, enable_3, new_c_3)
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #define _ALTERNATIVE_CFG(old_c, new_c, vendor_id, patch_id, CONFIG_k)	\
> @@ -108,6 +124,13 @@
>  	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, patch_id_1, IS_ENABLED(CONFIG_k_1),	\
>  				   new_c_2, vendor_id_2, patch_id_2, IS_ENABLED(CONFIG_k_2))
>  
> +#define _ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
> +				  new_c_2, vendor_id_2, patch_id_2, CONFIG_k_2,		\
> +				  new_c_3, vendor_id_3, patch_id_3, CONFIG_k_3)		\
> +	__ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, patch_id_1, IS_ENABLED(CONFIG_k_1),	\
> +				   new_c_2, vendor_id_2, patch_id_2, IS_ENABLED(CONFIG_k_2),	\
> +				   new_c_3, vendor_id_3, patch_id_3, IS_ENABLED(CONFIG_k_3))
> +
>  #else /* CONFIG_RISCV_ALTERNATIVE */
>  #ifdef __ASSEMBLY__
>  
> @@ -121,6 +144,9 @@
>  #define _ALTERNATIVE_CFG_2(old_c, ...)	\
>  	ALTERNATIVE_CFG old_c
>  
> +#define _ALTERNATIVE_CFG_3(old_c, ...)	\
> +	ALTERNATIVE_CFG old_c
> +
>  #else /* !__ASSEMBLY__ */
>  
>  #define __ALTERNATIVE_CFG(old_c)	\
> @@ -132,6 +158,9 @@
>  #define _ALTERNATIVE_CFG_2(old_c, ...)	\
>  	__ALTERNATIVE_CFG(old_c)
>  
> +#define _ALTERNATIVE_CFG_3(old_c, ...)	\
> +	__ALTERNATIVE_CFG(old_c)
> +
>  #endif /* __ASSEMBLY__ */
>  #endif /* CONFIG_RISCV_ALTERNATIVE */
>  
> @@ -152,15 +181,21 @@
>  	_ALTERNATIVE_CFG(old_content, new_content, vendor_id, patch_id, CONFIG_k)
>  
>  /*
> - * A vendor wants to replace an old_content, but another vendor has used
> - * ALTERNATIVE() to patch its customized content at the same location. In
> - * this case, this vendor can create a new macro ALTERNATIVE_2() based
> - * on the following sample code and then replace ALTERNATIVE() with
> - * ALTERNATIVE_2() to append its customized content.
> + * Variant of ALTERNATIVE() that supports two sets of replacement content.
>   */
>  #define ALTERNATIVE_2(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
>  				   new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2)		\
>  	_ALTERNATIVE_CFG_2(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,	\
>  					new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2)
>  
> +/*
> + * Variant of ALTERNATIVE() that supports three sets of replacement content.
> + */
> +#define ALTERNATIVE_3(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,		\
> +				   new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2,		\
> +				   new_content_3, vendor_id_3, patch_id_3, CONFIG_k_3)		\
> +	_ALTERNATIVE_CFG_3(old_content, new_content_1, vendor_id_1, patch_id_1, CONFIG_k_1,	\
> +					new_content_2, vendor_id_2, patch_id_2, CONFIG_k_2,	\
> +					new_content_3, vendor_id_3, patch_id_3, CONFIG_k_3)
> +
>  #endif
> -- 
> 2.45.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 09/11] riscv: alternative: Allow calls with alternate link registers

[Samuel Holland]

Alternative assembly code may wish to use an alternate link register to
minimize the number of clobbered registers. Apply the offset fix to all
jalr (not jr) instructions, i.e. where rd is not x0.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/kernel/alternative.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index 0128b161bfda..54d79e6f4afa 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -121,8 +121,8 @@ void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
 			if (!riscv_insn_is_jalr(insn2))
 				continue;
 
-			/* if instruction pair is a call, it will use the ra register */
-			if (RV_EXTRACT_RD_REG(insn) != 1)
+			/* if instruction pair is a call, it will save a link register */
+			if (RV_EXTRACT_RD_REG(insn) == 0)
 				continue;
 
 			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
-- 
2.45.1

[PATCH 10/11] riscv: mm: Use physical memory aliases to apply PMAs

[Samuel Holland]

On some RISC-V platforms, RAM is mapped to multiple physical address
ranges, with each alias having a different set of statically-determined
Physical Memory Attributes (PMAs). Software selects the PMAs for a page
by choosing a PFN from the corresponding physical address range.

Implement this by transforming the PFN when writing page tables.  If the
memory type field is nonzero, replace the PFN with the corresponding PFN
from the noncached alias. Similarly, when reading from the page tables,
if the PFN is found in a noncached alias, replace it with the
corresponding PFN from the cached alias, and insert _PAGE_NOCACHE.

The rest of the kernel sees only the cached PFNs and _PAGE_MTMASK values
as if Svpbmt was implemented.

Memory alias pairs are determined from the devicetree. A new cpufeature
bit is required because that is the only way to trigger alternative
patching.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/Kconfig                  |   3 +
 arch/riscv/include/asm/hwcap.h      |   1 +
 arch/riscv/include/asm/pgtable-64.h |  27 ++++++--
 arch/riscv/include/asm/pgtable.h    |   8 +++
 arch/riscv/kernel/cpufeature.c      |   6 ++
 arch/riscv/kernel/setup.c           |   1 +
 arch/riscv/mm/Makefile              |   1 +
 arch/riscv/mm/memory-alias.S        | 101 ++++++++++++++++++++++++++++
 arch/riscv/mm/pgtable.c             |  91 +++++++++++++++++++++++++
 9 files changed, 235 insertions(+), 4 deletions(-)
 create mode 100644 arch/riscv/mm/memory-alias.S

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 62545946ecf4..d28d1dab5f26 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -566,6 +566,9 @@ config RISCV_ISA_SVPBMT
 
 	   The Svpbmt extension is only available on 64-bit cpus.
 
+	   This option also controls selection of memory type based on
+	   physical memory aliases.
+
 	   If you don't know what to do here, say Y.
 
 config TOOLCHAIN_HAS_V
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 46d9de54179e..8a37e22f4223 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -94,6 +94,7 @@
 #define RISCV_ISA_EXT_ZAWRS		85
 #define RISCV_ISA_EXT_SVVPTC		86
 
+#define RISCV_ISA_EXT_XLINUXMEMALIAS	126
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
 #define RISCV_ISA_EXT_MAX		128
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 174b6a5837c2..6b4af408a37a 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -124,27 +124,46 @@ enum napot_cont_order {
  */
 
 #define ALT_FIXUP_MT(_val)								\
-	asm(ALTERNATIVE_2("addi	t0, zero, 0x3\n\t"					\
+	asm(ALTERNATIVE_3("addi	t0, zero, 0x3\n\t"					\
 			  "slli	t0, t0, 61\n\t"						\
 			  "not	t0, t0\n\t"						\
 			  "and	%0, %0, t0\n\t"						\
 			  "nop\n\t"							\
 			  "nop\n\t"							\
+			  "nop\n\t"							\
 			  "nop",							\
-			  __nops(7),							\
+			  __nops(8),							\
 			  0, RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,		\
+			  "addi t0, zero, 0x3\n\t"					\
+			  "slli	t0, t0, 61\n\t"						\
+			  "and	t0, %0, t0\n\t"						\
+			  "beqz t0, 2f\n\t"						\
+			  "xor	t1, %0, t0\n\t"						\
+			  "1: auipc t0, %%pcrel_hi(riscv_fixup_memory_alias)\n\t"	\
+			  "jalr	t0, t0, %%pcrel_lo(1b)\n\t"				\
+			  "mv	%0, t1\n"						\
+			  "2:",								\
+			  0, RISCV_ISA_EXT_XLINUXMEMALIAS, CONFIG_RISCV_ISA_SVPBMT,	\
 			  "srli	t0, %0, 59\n\t"						\
 			  "seqz t1, t0\n\t"						\
 			  "slli	t1, t1, 1\n\t"						\
 			  "or	t0, t0, t1\n\t"						\
 			  "xori	t0, t0, 0x5\n\t"					\
 			  "slli	t0, t0, 60\n\t"						\
-			  "xor	%0, %0, t0",						\
+			  "xor	%0, %0, t0\n\t"						\
+			  "nop",							\
 			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
 			  : "+r" (_val) :: "t0", "t1")
 
 #define ALT_UNFIX_MT(_val)								\
-	asm(ALTERNATIVE(__nops(6),							\
+	asm(ALTERNATIVE_2(__nops(6),							\
+			  "mv	t1, %0\n\t"						\
+			  "1: auipc t0, %%pcrel_hi(riscv_unfix_memory_alias)\n\t"	\
+			  "jalr	t0, t0, %%pcrel_lo(1b)\n\t"				\
+			  "mv	%0, t1\n\t"						\
+			  "nop\n\t"							\
+			  "nop",							\
+			  0, RISCV_ISA_EXT_XLINUXMEMALIAS, CONFIG_RISCV_ISA_SVPBMT,	\
 			  "srli	t0, %0, 60\n\t"						\
 			  "andi	t0, t0, 0xd\n\t"					\
 			  "srli	t1, t0, 1\n\t"						\
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 3ffcff76ac0d..0e52dfaaff63 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -949,6 +949,14 @@ extern u64 satp_mode;
 void paging_init(void);
 void misc_mem_init(void);
 
+#ifdef CONFIG_RISCV_ISA_SVPBMT
+bool __init riscv_have_memory_alias(void);
+void __init riscv_init_memory_alias(void);
+#else
+static inline bool riscv_have_memory_alias(void) { return false; }
+static inline void riscv_init_memory_alias(void) {}
+#endif /* CONFIG_RISCV_ISA_SVPBMT */
+
 /*
  * ZERO_PAGE is a global shared page that is always zero,
  * used for zero-mapped memory areas, etc.
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3a8eeaa9310c..ca36f8240a86 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -892,6 +892,12 @@ void __init riscv_fill_hwcap(void)
 			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
 	}
 
+	/* Vendor-independent alternatives require a bit in the ISA bitmap. */
+	if (riscv_have_memory_alias()) {
+		set_bit(RISCV_ISA_EXT_XLINUXMEMALIAS, riscv_isa);
+		pr_info("Using physical memory alias for noncached mappings\n");
+	}
+
 	memset(print_str, 0, sizeof(print_str));
 	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
 		if (riscv_isa[0] & BIT_MASK(i))
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index a2cde65b69e9..ab718fc4538f 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -287,6 +287,7 @@ void __init setup_arch(char **cmdline_p)
 	}
 
 	riscv_init_cbo_blocksizes();
+	riscv_init_memory_alias();
 	riscv_fill_hwcap();
 	init_rt_signal_env();
 	apply_boot_alternatives();
diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
index cbe4d775ef56..50d843b298cd 100644
--- a/arch/riscv/mm/Makefile
+++ b/arch/riscv/mm/Makefile
@@ -33,3 +33,4 @@ endif
 obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
 obj-$(CONFIG_RISCV_DMA_NONCOHERENT) += dma-noncoherent.o
 obj-$(CONFIG_RISCV_NONSTANDARD_CACHE_OPS) += cache-ops.o
+obj-$(CONFIG_RISCV_ISA_SVPBMT) += memory-alias.o
diff --git a/arch/riscv/mm/memory-alias.S b/arch/riscv/mm/memory-alias.S
new file mode 100644
index 000000000000..df2e8cc3f69c
--- /dev/null
+++ b/arch/riscv/mm/memory-alias.S
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 SiFive
+ */
+
+#include <linux/bits.h>
+#include <linux/linkage.h>
+#include <asm/asm.h>
+#include <asm/pgtable.h>
+
+#define CACHED_BASE_OFFSET	(0 * RISCV_SZPTR)
+#define NONCACHED_BASE_OFFSET	(1 * RISCV_SZPTR)
+#define SIZE_OFFSET		(2 * RISCV_SZPTR)
+
+#define SIZEOF_PAIR		(4 * RISCV_SZPTR)
+
+SYM_CODE_START(riscv_fixup_memory_alias)
+	addi	sp, sp, -4 * SZREG
+	REG_S	t2, (0 * SZREG)(sp)
+	REG_S	t3, (1 * SZREG)(sp)
+	REG_S	t4, (2 * SZREG)(sp)
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
+	REG_S	t5, (3 * SZREG)(sp)
+
+	/* Save and mask off _PAGE_NAPOT if present. */
+	li	t5, _PAGE_NAPOT
+	and	t5, t1, t5
+	xor	t1, t1, t5
+#endif
+
+	lla	t2, memory_alias_pairs
+.Lfixup_loop:
+	REG_L	t3, SIZE_OFFSET(t2)
+	beqz	t3, .Lfixup_end
+	REG_L	t4, CACHED_BASE_OFFSET(t2)
+	sub	t4, t1, t4
+	bltu	t4, t3, .Lfixup_found
+	addi	t2, t2, SIZEOF_PAIR
+	j	.Lfixup_loop
+
+.Lfixup_found:
+	REG_L	t3, NONCACHED_BASE_OFFSET(t2)
+	add	t1, t3, t4
+
+.Lfixup_end:
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
+	xor	t1, t1, t5
+
+	REG_L	t5, (3 * SZREG)(sp)
+#endif
+	REG_L	t4, (2 * SZREG)(sp)
+	REG_L	t3, (1 * SZREG)(sp)
+	REG_L	t2, (0 * SZREG)(sp)
+	addi	sp, sp, 4 * SZREG
+	jr	t0
+SYM_CODE_END(riscv_fixup_memory_alias)
+
+SYM_CODE_START(riscv_unfix_memory_alias)
+	addi	sp, sp, -4 * SZREG
+	REG_S	t2, (0 * SZREG)(sp)
+	REG_S	t3, (1 * SZREG)(sp)
+	REG_S	t4, (2 * SZREG)(sp)
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
+	REG_S	t5, (3 * SZREG)(sp)
+
+	/* Save and mask off _PAGE_NAPOT if present. */
+	li	t5, _PAGE_NAPOT
+	and	t5, t1, t5
+	xor	t1, t1, t5
+#endif
+
+	lla	t2, memory_alias_pairs
+.Lunfix_loop:
+	REG_L	t3, SIZE_OFFSET(t2)
+	beqz	t3, .Lunfix_end
+	REG_L	t4, NONCACHED_BASE_OFFSET(t2)
+	sub	t4, t1, t4
+	bltu	t4, t3, .Lunfix_found
+	addi	t2, t2, SIZEOF_PAIR
+	j	.Lunfix_loop
+
+.Lunfix_found:
+	REG_L	t3, CACHED_BASE_OFFSET(t2)
+	add	t1, t3, t4
+
+	/* PFN was in the noncached alias, so mark it as such. */
+	li	t2, _PAGE_NOCACHE
+	or	t1, t1, t2
+
+.Lunfix_end:
+#ifdef CONFIG_RISCV_ISA_SVNAPOT
+	xor	t1, t1, t5
+
+	REG_L	t5, (3 * SZREG)(sp)
+#endif
+	REG_L	t4, (2 * SZREG)(sp)
+	REG_L	t3, (1 * SZREG)(sp)
+	REG_L	t2, (0 * SZREG)(sp)
+	addi	sp, sp, 4 * SZREG
+	jr	t0
+SYM_CODE_END(riscv_unfix_memory_alias)
diff --git a/arch/riscv/mm/pgtable.c b/arch/riscv/mm/pgtable.c
index 4ae67324f992..8dd43001cd10 100644
--- a/arch/riscv/mm/pgtable.c
+++ b/arch/riscv/mm/pgtable.c
@@ -1,8 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <asm/pgalloc.h>
+#include <dt-bindings/riscv/physical-memory.h>
+#include <linux/bitfield.h>
 #include <linux/gfp.h>
 #include <linux/kernel.h>
+#include <linux/memblock.h>
+#include <linux/of.h>
 #include <linux/pgtable.h>
 
 int ptep_set_access_flags(struct vm_area_struct *vma,
@@ -155,3 +159,90 @@ pmd_t pmdp_collapse_flush(struct vm_area_struct *vma,
 	return pmd;
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
+#ifdef CONFIG_RISCV_ISA_SVPBMT
+struct memory_alias_pair {
+	unsigned long cached_base;
+	unsigned long noncached_base;
+	unsigned long size;
+	int index;
+} memory_alias_pairs[5];
+
+bool __init riscv_have_memory_alias(void)
+{
+	return memory_alias_pairs[0].size;
+}
+
+void __init riscv_init_memory_alias(void)
+{
+	int na = of_n_addr_cells(of_root);
+	int ns = of_n_size_cells(of_root);
+	int nc = na + ns + 2;
+	const __be32 *prop;
+	int pairs = 0;
+	int len;
+
+	prop = of_get_property(of_root, "riscv,physical-memory-regions", &len);
+	if (!prop)
+		return;
+
+	len /= sizeof(__be32);
+	for (int i = 0; len >= nc; i++, prop += nc, len -= nc) {
+		unsigned long base = of_read_ulong(prop, na);
+		unsigned long size = of_read_ulong(prop + na, ns);
+		unsigned long flags = be32_to_cpup(prop + na + ns);
+		struct memory_alias_pair *pair;
+		int alias;
+
+		/* We only care about non-coherent memory. */
+		if ((flags & PMA_ORDER_MASK) != PMA_ORDER_MEMORY || (flags & PMA_COHERENT))
+			continue;
+
+		/* The cacheable alias must be usable memory. */
+		if ((flags & PMA_CACHEABLE) &&
+		    !memblock_overlaps_region(&memblock.memory, base, size))
+			continue;
+
+		alias = FIELD_GET(PMR_ALIAS_MASK, flags);
+		if (alias) {
+			pair = NULL;
+			for (int j = 0; j < pairs; j++) {
+				if (alias == memory_alias_pairs[j].index) {
+					pair = &memory_alias_pairs[j];
+					break;
+				}
+			}
+			if (!pair)
+				continue;
+		} else {
+			/* Leave room for the null sentinel. */
+			if (pairs == ARRAY_SIZE(memory_alias_pairs) - 1)
+				continue;
+			pair = &memory_alias_pairs[pairs++];
+			pair->index = i;
+		}
+
+		/* Align the address and size with the page table PFN field. */
+		base >>= PAGE_SHIFT - _PAGE_PFN_SHIFT;
+		size >>= PAGE_SHIFT - _PAGE_PFN_SHIFT;
+
+		if (flags & PMA_CACHEABLE)
+			pair->cached_base = base;
+		else
+			pair->noncached_base = base;
+		pair->size = min_not_zero(pair->size, size);
+	}
+
+	/* Remove any unmatched pairs. */
+	for (int i = 0; i < pairs; i++) {
+		struct memory_alias_pair *pair = &memory_alias_pairs[i];
+
+		if (pair->cached_base && pair->noncached_base && pair->size)
+			continue;
+
+		for (int j = i + 1; j < pairs; j++)
+			memory_alias_pairs[j - 1] = memory_alias_pairs[j];
+		memory_alias_pairs[--pairs].size = 0;
+	}
+}
+#endif /* CONFIG_RISCV_ISA_SVPBMT */
-- 
2.45.1

Re: [PATCH 10/11] riscv: mm: Use physical memory aliases to apply PMAs

[kernel test robot]

Hi Samuel,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.12-rc5 next-20241101]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Samuel-Holland/dt-bindings-riscv-Describe-physical-memory-regions/20241102-081311
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241102000843.1301099-11-samuel.holland%40sifive.com
patch subject: [PATCH 10/11] riscv: mm: Use physical memory aliases to apply PMAs
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20241102/202411022337.JHozQieV-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411022337.JHozQieV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411022337.JHozQieV-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "riscv_fixup_memory_alias" [arch/riscv/kvm/kvm.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 10/11] riscv: mm: Use physical memory aliases to apply PMAs

[Emil Renner Berthing]

Samuel Holland wrote:
> On some RISC-V platforms, RAM is mapped to multiple physical address
> ranges, with each alias having a different set of statically-determined
> Physical Memory Attributes (PMAs). Software selects the PMAs for a page
> by choosing a PFN from the corresponding physical address range.
>
> Implement this by transforming the PFN when writing page tables.  If the
> memory type field is nonzero, replace the PFN with the corresponding PFN
> from the noncached alias. Similarly, when reading from the page tables,
> if the PFN is found in a noncached alias, replace it with the
> corresponding PFN from the cached alias, and insert _PAGE_NOCACHE.
>
> The rest of the kernel sees only the cached PFNs and _PAGE_MTMASK values
> as if Svpbmt was implemented.
>
> Memory alias pairs are determined from the devicetree. A new cpufeature
> bit is required because that is the only way to trigger alternative
> patching.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>  arch/riscv/Kconfig                  |   3 +
>  arch/riscv/include/asm/hwcap.h      |   1 +
>  arch/riscv/include/asm/pgtable-64.h |  27 ++++++--
>  arch/riscv/include/asm/pgtable.h    |   8 +++
>  arch/riscv/kernel/cpufeature.c      |   6 ++
>  arch/riscv/kernel/setup.c           |   1 +
>  arch/riscv/mm/Makefile              |   1 +
>  arch/riscv/mm/memory-alias.S        | 101 ++++++++++++++++++++++++++++
>  arch/riscv/mm/pgtable.c             |  91 +++++++++++++++++++++++++
>  9 files changed, 235 insertions(+), 4 deletions(-)
>  create mode 100644 arch/riscv/mm/memory-alias.S
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 62545946ecf4..d28d1dab5f26 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -566,6 +566,9 @@ config RISCV_ISA_SVPBMT
>
>  	   The Svpbmt extension is only available on 64-bit cpus.
>
> +	   This option also controls selection of memory type based on
> +	   physical memory aliases.
> +

I wonder how much work it would be to make this its own Kconfig option only
enabled by JH7100 and EIC7700 support. Hopefully future chips will just
implement the standard Svpmt and this code will be unneded overhead.

/Emil

>  	   If you don't know what to do here, say Y.
>
>  config TOOLCHAIN_HAS_V
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 46d9de54179e..8a37e22f4223 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -94,6 +94,7 @@
>  #define RISCV_ISA_EXT_ZAWRS		85
>  #define RISCV_ISA_EXT_SVVPTC		86
>
> +#define RISCV_ISA_EXT_XLINUXMEMALIAS	126
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>
>  #define RISCV_ISA_EXT_MAX		128
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 174b6a5837c2..6b4af408a37a 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -124,27 +124,46 @@ enum napot_cont_order {
>   */
>
>  #define ALT_FIXUP_MT(_val)								\
> -	asm(ALTERNATIVE_2("addi	t0, zero, 0x3\n\t"					\
> +	asm(ALTERNATIVE_3("addi	t0, zero, 0x3\n\t"					\
>  			  "slli	t0, t0, 61\n\t"						\
>  			  "not	t0, t0\n\t"						\
>  			  "and	%0, %0, t0\n\t"						\
>  			  "nop\n\t"							\
>  			  "nop\n\t"							\
> +			  "nop\n\t"							\
>  			  "nop",							\
> -			  __nops(7),							\
> +			  __nops(8),							\
>  			  0, RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,		\
> +			  "addi t0, zero, 0x3\n\t"					\
> +			  "slli	t0, t0, 61\n\t"						\
> +			  "and	t0, %0, t0\n\t"						\
> +			  "beqz t0, 2f\n\t"						\
> +			  "xor	t1, %0, t0\n\t"						\
> +			  "1: auipc t0, %%pcrel_hi(riscv_fixup_memory_alias)\n\t"	\
> +			  "jalr	t0, t0, %%pcrel_lo(1b)\n\t"				\
> +			  "mv	%0, t1\n"						\
> +			  "2:",								\
> +			  0, RISCV_ISA_EXT_XLINUXMEMALIAS, CONFIG_RISCV_ISA_SVPBMT,	\
>  			  "srli	t0, %0, 59\n\t"						\
>  			  "seqz t1, t0\n\t"						\
>  			  "slli	t1, t1, 1\n\t"						\
>  			  "or	t0, t0, t1\n\t"						\
>  			  "xori	t0, t0, 0x5\n\t"					\
>  			  "slli	t0, t0, 60\n\t"						\
> -			  "xor	%0, %0, t0",						\
> +			  "xor	%0, %0, t0\n\t"						\
> +			  "nop",							\
>  			  THEAD_VENDOR_ID, ERRATA_THEAD_MAE, CONFIG_ERRATA_THEAD_MAE)	\
>  			  : "+r" (_val) :: "t0", "t1")
>
>  #define ALT_UNFIX_MT(_val)								\
> -	asm(ALTERNATIVE(__nops(6),							\
> +	asm(ALTERNATIVE_2(__nops(6),							\
> +			  "mv	t1, %0\n\t"						\
> +			  "1: auipc t0, %%pcrel_hi(riscv_unfix_memory_alias)\n\t"	\
> +			  "jalr	t0, t0, %%pcrel_lo(1b)\n\t"				\
> +			  "mv	%0, t1\n\t"						\
> +			  "nop\n\t"							\
> +			  "nop",							\
> +			  0, RISCV_ISA_EXT_XLINUXMEMALIAS, CONFIG_RISCV_ISA_SVPBMT,	\
>  			  "srli	t0, %0, 60\n\t"						\
>  			  "andi	t0, t0, 0xd\n\t"					\
>  			  "srli	t1, t0, 1\n\t"						\
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 3ffcff76ac0d..0e52dfaaff63 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -949,6 +949,14 @@ extern u64 satp_mode;
>  void paging_init(void);
>  void misc_mem_init(void);
>
> +#ifdef CONFIG_RISCV_ISA_SVPBMT
> +bool __init riscv_have_memory_alias(void);
> +void __init riscv_init_memory_alias(void);
> +#else
> +static inline bool riscv_have_memory_alias(void) { return false; }
> +static inline void riscv_init_memory_alias(void) {}
> +#endif /* CONFIG_RISCV_ISA_SVPBMT */
> +
>  /*
>   * ZERO_PAGE is a global shared page that is always zero,
>   * used for zero-mapped memory areas, etc.
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3a8eeaa9310c..ca36f8240a86 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -892,6 +892,12 @@ void __init riscv_fill_hwcap(void)
>  			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
>  	}
>
> +	/* Vendor-independent alternatives require a bit in the ISA bitmap. */
> +	if (riscv_have_memory_alias()) {
> +		set_bit(RISCV_ISA_EXT_XLINUXMEMALIAS, riscv_isa);
> +		pr_info("Using physical memory alias for noncached mappings\n");
> +	}
> +
>  	memset(print_str, 0, sizeof(print_str));
>  	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
>  		if (riscv_isa[0] & BIT_MASK(i))
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index a2cde65b69e9..ab718fc4538f 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -287,6 +287,7 @@ void __init setup_arch(char **cmdline_p)
>  	}
>
>  	riscv_init_cbo_blocksizes();
> +	riscv_init_memory_alias();
>  	riscv_fill_hwcap();
>  	init_rt_signal_env();
>  	apply_boot_alternatives();
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index cbe4d775ef56..50d843b298cd 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -33,3 +33,4 @@ endif
>  obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
>  obj-$(CONFIG_RISCV_DMA_NONCOHERENT) += dma-noncoherent.o
>  obj-$(CONFIG_RISCV_NONSTANDARD_CACHE_OPS) += cache-ops.o
> +obj-$(CONFIG_RISCV_ISA_SVPBMT) += memory-alias.o
> diff --git a/arch/riscv/mm/memory-alias.S b/arch/riscv/mm/memory-alias.S
> new file mode 100644
> index 000000000000..df2e8cc3f69c
> --- /dev/null
> +++ b/arch/riscv/mm/memory-alias.S
> @@ -0,0 +1,101 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2024 SiFive
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/linkage.h>
> +#include <asm/asm.h>
> +#include <asm/pgtable.h>
> +
> +#define CACHED_BASE_OFFSET	(0 * RISCV_SZPTR)
> +#define NONCACHED_BASE_OFFSET	(1 * RISCV_SZPTR)
> +#define SIZE_OFFSET		(2 * RISCV_SZPTR)
> +
> +#define SIZEOF_PAIR		(4 * RISCV_SZPTR)
> +
> +SYM_CODE_START(riscv_fixup_memory_alias)
> +	addi	sp, sp, -4 * SZREG
> +	REG_S	t2, (0 * SZREG)(sp)
> +	REG_S	t3, (1 * SZREG)(sp)
> +	REG_S	t4, (2 * SZREG)(sp)
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
> +	REG_S	t5, (3 * SZREG)(sp)
> +
> +	/* Save and mask off _PAGE_NAPOT if present. */
> +	li	t5, _PAGE_NAPOT
> +	and	t5, t1, t5
> +	xor	t1, t1, t5
> +#endif
> +
> +	lla	t2, memory_alias_pairs
> +.Lfixup_loop:
> +	REG_L	t3, SIZE_OFFSET(t2)
> +	beqz	t3, .Lfixup_end
> +	REG_L	t4, CACHED_BASE_OFFSET(t2)
> +	sub	t4, t1, t4
> +	bltu	t4, t3, .Lfixup_found
> +	addi	t2, t2, SIZEOF_PAIR
> +	j	.Lfixup_loop
> +
> +.Lfixup_found:
> +	REG_L	t3, NONCACHED_BASE_OFFSET(t2)
> +	add	t1, t3, t4
> +
> +.Lfixup_end:
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
> +	xor	t1, t1, t5
> +
> +	REG_L	t5, (3 * SZREG)(sp)
> +#endif
> +	REG_L	t4, (2 * SZREG)(sp)
> +	REG_L	t3, (1 * SZREG)(sp)
> +	REG_L	t2, (0 * SZREG)(sp)
> +	addi	sp, sp, 4 * SZREG
> +	jr	t0
> +SYM_CODE_END(riscv_fixup_memory_alias)
> +
> +SYM_CODE_START(riscv_unfix_memory_alias)
> +	addi	sp, sp, -4 * SZREG
> +	REG_S	t2, (0 * SZREG)(sp)
> +	REG_S	t3, (1 * SZREG)(sp)
> +	REG_S	t4, (2 * SZREG)(sp)
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
> +	REG_S	t5, (3 * SZREG)(sp)
> +
> +	/* Save and mask off _PAGE_NAPOT if present. */
> +	li	t5, _PAGE_NAPOT
> +	and	t5, t1, t5
> +	xor	t1, t1, t5
> +#endif
> +
> +	lla	t2, memory_alias_pairs
> +.Lunfix_loop:
> +	REG_L	t3, SIZE_OFFSET(t2)
> +	beqz	t3, .Lunfix_end
> +	REG_L	t4, NONCACHED_BASE_OFFSET(t2)
> +	sub	t4, t1, t4
> +	bltu	t4, t3, .Lunfix_found
> +	addi	t2, t2, SIZEOF_PAIR
> +	j	.Lunfix_loop
> +
> +.Lunfix_found:
> +	REG_L	t3, CACHED_BASE_OFFSET(t2)
> +	add	t1, t3, t4
> +
> +	/* PFN was in the noncached alias, so mark it as such. */
> +	li	t2, _PAGE_NOCACHE
> +	or	t1, t1, t2
> +
> +.Lunfix_end:
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
> +	xor	t1, t1, t5
> +
> +	REG_L	t5, (3 * SZREG)(sp)
> +#endif
> +	REG_L	t4, (2 * SZREG)(sp)
> +	REG_L	t3, (1 * SZREG)(sp)
> +	REG_L	t2, (0 * SZREG)(sp)
> +	addi	sp, sp, 4 * SZREG
> +	jr	t0
> +SYM_CODE_END(riscv_unfix_memory_alias)
> diff --git a/arch/riscv/mm/pgtable.c b/arch/riscv/mm/pgtable.c
> index 4ae67324f992..8dd43001cd10 100644
> --- a/arch/riscv/mm/pgtable.c
> +++ b/arch/riscv/mm/pgtable.c
> @@ -1,8 +1,12 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <asm/pgalloc.h>
> +#include <dt-bindings/riscv/physical-memory.h>
> +#include <linux/bitfield.h>
>  #include <linux/gfp.h>
>  #include <linux/kernel.h>
> +#include <linux/memblock.h>
> +#include <linux/of.h>
>  #include <linux/pgtable.h>
>
>  int ptep_set_access_flags(struct vm_area_struct *vma,
> @@ -155,3 +159,90 @@ pmd_t pmdp_collapse_flush(struct vm_area_struct *vma,
>  	return pmd;
>  }
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +
> +#ifdef CONFIG_RISCV_ISA_SVPBMT
> +struct memory_alias_pair {
> +	unsigned long cached_base;
> +	unsigned long noncached_base;
> +	unsigned long size;
> +	int index;
> +} memory_alias_pairs[5];
> +
> +bool __init riscv_have_memory_alias(void)
> +{
> +	return memory_alias_pairs[0].size;
> +}
> +
> +void __init riscv_init_memory_alias(void)
> +{
> +	int na = of_n_addr_cells(of_root);
> +	int ns = of_n_size_cells(of_root);
> +	int nc = na + ns + 2;
> +	const __be32 *prop;
> +	int pairs = 0;
> +	int len;
> +
> +	prop = of_get_property(of_root, "riscv,physical-memory-regions", &len);
> +	if (!prop)
> +		return;
> +
> +	len /= sizeof(__be32);
> +	for (int i = 0; len >= nc; i++, prop += nc, len -= nc) {
> +		unsigned long base = of_read_ulong(prop, na);
> +		unsigned long size = of_read_ulong(prop + na, ns);
> +		unsigned long flags = be32_to_cpup(prop + na + ns);
> +		struct memory_alias_pair *pair;
> +		int alias;
> +
> +		/* We only care about non-coherent memory. */
> +		if ((flags & PMA_ORDER_MASK) != PMA_ORDER_MEMORY || (flags & PMA_COHERENT))
> +			continue;
> +
> +		/* The cacheable alias must be usable memory. */
> +		if ((flags & PMA_CACHEABLE) &&
> +		    !memblock_overlaps_region(&memblock.memory, base, size))
> +			continue;
> +
> +		alias = FIELD_GET(PMR_ALIAS_MASK, flags);
> +		if (alias) {
> +			pair = NULL;
> +			for (int j = 0; j < pairs; j++) {
> +				if (alias == memory_alias_pairs[j].index) {
> +					pair = &memory_alias_pairs[j];
> +					break;
> +				}
> +			}
> +			if (!pair)
> +				continue;
> +		} else {
> +			/* Leave room for the null sentinel. */
> +			if (pairs == ARRAY_SIZE(memory_alias_pairs) - 1)
> +				continue;
> +			pair = &memory_alias_pairs[pairs++];
> +			pair->index = i;
> +		}
> +
> +		/* Align the address and size with the page table PFN field. */
> +		base >>= PAGE_SHIFT - _PAGE_PFN_SHIFT;
> +		size >>= PAGE_SHIFT - _PAGE_PFN_SHIFT;
> +
> +		if (flags & PMA_CACHEABLE)
> +			pair->cached_base = base;
> +		else
> +			pair->noncached_base = base;
> +		pair->size = min_not_zero(pair->size, size);
> +	}
> +
> +	/* Remove any unmatched pairs. */
> +	for (int i = 0; i < pairs; i++) {
> +		struct memory_alias_pair *pair = &memory_alias_pairs[i];
> +
> +		if (pair->cached_base && pair->noncached_base && pair->size)
> +			continue;
> +
> +		for (int j = i + 1; j < pairs; j++)
> +			memory_alias_pairs[j - 1] = memory_alias_pairs[j];
> +		memory_alias_pairs[--pairs].size = 0;
> +	}
> +}
> +#endif /* CONFIG_RISCV_ISA_SVPBMT */
> --
> 2.45.1

[PATCH 11/11] riscv: dts: starfive: jh7100: Use physical memory ranges for DMA

[Samuel Holland]

JH7100 provides a physical memory region which is a noncached alias of
normal cacheable DRAM. Now that Linux can apply PMAs by selecting
between aliases of a physical memory region, any page of DRAM can be
marked as noncached for use with DMA, and the preallocated DMA pool is
no longer needed. This allows portable kernels to boot on JH7100 boards.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/Kconfig.errata                     | 19 ------------
 .../boot/dts/starfive/jh7100-common.dtsi      | 30 ++++---------------
 2 files changed, 6 insertions(+), 43 deletions(-)

diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 2806ed7916c7..fc2c7fb2caff 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -53,25 +53,6 @@ config ERRATA_SIFIVE_CIP_1200
 
 	  If you don't know what to do here, say "Y".
 
-config ERRATA_STARFIVE_JH7100
-	bool "StarFive JH7100 support"
-	depends on ARCH_STARFIVE
-	depends on !DMA_DIRECT_REMAP
-	depends on NONPORTABLE
-	select DMA_GLOBAL_POOL
-	select RISCV_DMA_NONCOHERENT
-	select RISCV_NONSTANDARD_CACHE_OPS
-	select SIFIVE_CCACHE
-	default n
-	help
-	  The StarFive JH7100 was a test chip for the JH7110 and has
-	  caches that are non-coherent with respect to peripheral DMAs.
-	  It was designed before the Zicbom extension so needs non-standard
-	  cache operations through the SiFive cache controller.
-
-	  Say "Y" if you want to support the BeagleV Starlight and/or
-	  StarFive VisionFive V1 boards.
-
 config ERRATA_THEAD
 	bool "T-HEAD errata"
 	depends on RISCV_ALTERNATIVE
diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index ae1a6aeb0aea..34885fe40e2d 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -9,8 +9,14 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/pinctrl-starfive-jh7100.h>
+#include <dt-bindings/riscv/physical-memory.h>
 
 / {
+	riscv,physical-memory-regions =
+		<0x00 0x00000000 0x40 0x00000000 (PMA_RW | PMA_IO) 0x0>,
+		<0x00 0x80000000 0x08 0x00000000 (PMA_RWXA | PMA_NONCOHERENT_MEMORY) 0x0>,
+		<0x10 0x00000000 0x08 0x00000000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY | PMR_ALIAS(1)) 0x0>;
+
 	aliases {
 		mmc0 = &sdio0;
 		mmc1 = &sdio1;
@@ -42,30 +48,6 @@ led-ack {
 		};
 	};
 
-	reserved-memory {
-		#address-cells = <2>;
-		#size-cells = <2>;
-		ranges;
-
-		dma-reserved@fa000000 {
-			reg = <0x0 0xfa000000 0x0 0x1000000>;
-			no-map;
-		};
-
-		linux,dma@107a000000 {
-			compatible = "shared-dma-pool";
-			reg = <0x10 0x7a000000 0x0 0x1000000>;
-			no-map;
-			linux,dma-default;
-		};
-	};
-
-	soc {
-		dma-ranges = <0x00 0x80000000 0x00 0x80000000 0x00 0x7a000000>,
-			     <0x00 0xfa000000 0x10 0x7a000000 0x00 0x01000000>,
-			     <0x00 0xfb000000 0x00 0xfb000000 0x07 0x85000000>;
-	};
-
 	wifi_pwrseq: wifi-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&gpio 37 GPIO_ACTIVE_LOW>;
-- 
2.45.1

Re: [PATCH 11/11] riscv: dts: starfive: jh7100: Use physical memory ranges for DMA

[Emil Renner Berthing]

Samuel Holland wrote:
> JH7100 provides a physical memory region which is a noncached alias of
> normal cacheable DRAM. Now that Linux can apply PMAs by selecting
> between aliases of a physical memory region, any page of DRAM can be
> marked as noncached for use with DMA, and the preallocated DMA pool is
> no longer needed. This allows portable kernels to boot on JH7100 boards.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>  arch/riscv/Kconfig.errata                     | 19 ------------
>  .../boot/dts/starfive/jh7100-common.dtsi      | 30 ++++---------------
>  2 files changed, 6 insertions(+), 43 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 2806ed7916c7..fc2c7fb2caff 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -53,25 +53,6 @@ config ERRATA_SIFIVE_CIP_1200
>
>  	  If you don't know what to do here, say "Y".
>
> -config ERRATA_STARFIVE_JH7100
> -	bool "StarFive JH7100 support"
> -	depends on ARCH_STARFIVE
> -	depends on !DMA_DIRECT_REMAP
> -	depends on NONPORTABLE
> -	select DMA_GLOBAL_POOL

Hi Samuel,

Thank you for working on this!

The JH7100 still need the RISCV_NONSTANDARD_CACHE_OPS from the sifive-ccache
driver for streaming DMA, so if I just remove the 3 lines above instead of the
whole ERRATE_STARFIVE_JH7110 (and enable RISCV_ISA_SVPBMT) this series works on
my Starlight board.

> -	select RISCV_DMA_NONCOHERENT
> -	select RISCV_NONSTANDARD_CACHE_OPS
> -	select SIFIVE_CCACHE
> -	default n
> -	help
> -	  The StarFive JH7100 was a test chip for the JH7110 and has
> -	  caches that are non-coherent with respect to peripheral DMAs.
> -	  It was designed before the Zicbom extension so needs non-standard
> -	  cache operations through the SiFive cache controller.
> -
> -	  Say "Y" if you want to support the BeagleV Starlight and/or
> -	  StarFive VisionFive V1 boards.
> -
>  config ERRATA_THEAD
>  	bool "T-HEAD errata"
>  	depends on RISCV_ALTERNATIVE
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index ae1a6aeb0aea..34885fe40e2d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -9,8 +9,14 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/leds/common.h>
>  #include <dt-bindings/pinctrl/pinctrl-starfive-jh7100.h>
> +#include <dt-bindings/riscv/physical-memory.h>
>
>  / {
> +	riscv,physical-memory-regions =
> +		<0x00 0x00000000 0x40 0x00000000 (PMA_RW | PMA_IO) 0x0>,
> +		<0x00 0x80000000 0x08 0x00000000 (PMA_RWXA | PMA_NONCOHERENT_MEMORY) 0x0>,
> +		<0x10 0x00000000 0x08 0x00000000 (PMA_RWX | PMA_NONCACHEABLE_MEMORY | PMR_ALIAS(1)) 0x0>;
> +

The size and placement of the DMA pool was kind of arbitrary but the above is
a feature of the SoC, so should be moved to jh7100.dtsi instead.

>  	aliases {
>  		mmc0 = &sdio0;
>  		mmc1 = &sdio1;
> @@ -42,30 +48,6 @@ led-ack {
>  		};
>  	};
>
> -	reserved-memory {
> -		#address-cells = <2>;
> -		#size-cells = <2>;
> -		ranges;
> -
> -		dma-reserved@fa000000 {
> -			reg = <0x0 0xfa000000 0x0 0x1000000>;
> -			no-map;
> -		};
> -
> -		linux,dma@107a000000 {
> -			compatible = "shared-dma-pool";
> -			reg = <0x10 0x7a000000 0x0 0x1000000>;
> -			no-map;
> -			linux,dma-default;
> -		};
> -	};
> -
> -	soc {
> -		dma-ranges = <0x00 0x80000000 0x00 0x80000000 0x00 0x7a000000>,
> -			     <0x00 0xfa000000 0x10 0x7a000000 0x00 0x01000000>,
> -			     <0x00 0xfb000000 0x00 0xfb000000 0x07 0x85000000>;
> -	};
> -
>  	wifi_pwrseq: wifi-pwrseq {
>  		compatible = "mmc-pwrseq-simple";
>  		reset-gpios = <&gpio 37 GPIO_ACTIVE_LOW>;
> --
> 2.45.1
>

Re: [PATCH 00/11] riscv: Memory type control for platforms with physical memory aliases

[Bo Gan]

On 11/1/24 17:07, Samuel Holland wrote:
> 
> On some RISC-V platforms, including StarFive JH7100 and ESWIN EIC7700,
> RAM is mapped to multiple physical address ranges, with each alias
> having a different set of statically-determined Physical Memory
> Attributes (PMAs). Software selects the PMAs for a page by choosing a
> PFN from the corresponding physical address range. On these platforms,
> this is the only way to allocate noncached memory for use with
> noncoherent DMA.
> 
>   - Patch 1 adds a new binding to describe physical memory regions in
>     the devicetree.
>   - Patches 2-6 refactor existing memory type support to be modeled as
>     variants on top of Svpbmt.
>   - Patches 7-10 add logic to transform the PFN to use the desired alias
>     when reading/writing page tables.
>   - Patch 11 enables this new method of memory type control on JH7100.
> 
> I have boot-tested this series on platforms with each of the 4 ways to
> select a memory type: SiFive FU740 (none), QEMU (Svpbmt), Allwinner D1
> (XTheadMae), and ESWIN EIC7700 (aliases).
> 

Hi Samuel,

Any update on this? I see ESWIN has started their EIC7700 upstreaming
effort, and it'll likely rely on this. Is there any follow up series?
BTW, Icenowy's working on upstreaming the Verisilicon DC8200 driver.
His work also depend on this patchset in order to test on JH7110/EIC7700

Thanks!