[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5ee03973-1f14-44eb-b9ac-a41bc9ae474e@sifive.com>
Date: Sat, 2 Nov 2024 14:57:13 -0500
From: Samuel Holland <samuel.holland@...ive.com>
To: Jesse T <mr.bossman075@...il.com>
Cc: Palmer Dabbelt <palmer@...belt.com>, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org, Andrew Morton <akpm@...ux-foundation.org>,
Arnd Bergmann <arnd@...db.de>, Alexandre Ghiti <alexghiti@...osinc.com>
Subject: Re: [PATCH 6/6] riscv: Remove CONFIG_PAGE_OFFSET
Hi Jesse,
On 2024-10-27 3:25 PM, Jesse T wrote:
> On Sat, Oct 26, 2024 at 1:16 PM Samuel Holland
> <samuel.holland@...ive.com> wrote:
>>
>> The current definition of CONFIG_PAGE_OFFSET is problematic for a couple
>> of reasons:
>> 1) The value is misleading for normal 64-bit kernels, where it is
>> overridden at runtime if Sv48 or Sv39 is chosen. This is especially
>> the case for XIP kernels, which always use Sv39.
>> 2) The option is not user-visible, but for NOMMU kernels it must be a
>> valid RAM address, and for !RELOCATABLE it must additionally be the
>> exact address where the kernel is loaded.
>>
>> Fix both of these by removing the option.
>> 1) For MMU kernels, drop the indirection through Kconfig. Additionally,
>> for XIP, drop the indirection through kernel_map.
>> 2) For NOMMU kernels, use the user-visible physical RAM base if
>> provided. Otherwise, force the kernel to be relocatable.
>>
>> Signed-off-by: Samuel Holland <samuel.holland@...ive.com>
> Reviewed-by: Jesse Taube <mr.bossman075@...il.com>
Thanks!
>> ---
>>
>> arch/riscv/Kconfig | 8 +-------
>> arch/riscv/include/asm/page.h | 15 ++++++++-------
>> arch/riscv/include/asm/pgtable.h | 2 +-
>> arch/riscv/mm/init.c | 8 ++------
>> 4 files changed, 12 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 33aa79d84021..297ccbf4b5ce 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -195,6 +195,7 @@ config RISCV
>> select PCI_DOMAINS_GENERIC if PCI
>> select PCI_ECAM if (ACPI && PCI)
>> select PCI_MSI if PCI
>> + select RELOCATABLE if !MMU && !PHYS_RAM_BASE_FIXED
>> select RISCV_ALTERNATIVE if !XIP_KERNEL
>> select RISCV_APLIC
>> select RISCV_IMSIC
>> @@ -282,13 +283,6 @@ config MMU
>> Select if you want MMU-based virtualised addressing space
>> support by paged memory management. If unsure, say 'Y'.
>>
>> -config PAGE_OFFSET
>> - hex
>> - default 0x80000000 if !MMU && RISCV_M_MODE
>> - default 0x80200000 if !MMU
>
> Should the default values above now be moved to CONFIG_PHYS_RAM_BASE?
No, there doesn't need to be a default address, because the default behavior is
to be relocatable. If the kernel is running in S-mode, then it does not have
access to all of RAM, because the firmware is using some of it. So in that case
I don't think it makes sense to hardcode any addresses in the kernel.
For M-mode, the default for CONFIG_PHYS_RAM_BASE is already 0x80000000.
Regards,
Samuel
>> - default 0xc0000000 if 32BIT
>> - default 0xff60000000000000 if 64BIT
>> -
>> config KASAN_SHADOW_OFFSET
>> hex
>> depends on KASAN_GENERIC
>> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
>> index 24d1ac052609..d1acd1efe0dc 100644
>> --- a/arch/riscv/include/asm/page.h
>> +++ b/arch/riscv/include/asm/page.h
>> @@ -28,15 +28,16 @@
>> */
>> #ifdef CONFIG_MMU
>> #ifdef CONFIG_64BIT
>> -#define PAGE_OFFSET kernel_map.page_offset
>> -/*
>> - * By default, CONFIG_PAGE_OFFSET value corresponds to SV57 address space so
>> - * define the PAGE_OFFSET value for SV48 and SV39.
>> - */
>> +#define PAGE_OFFSET_L5 _AC(0xff60000000000000, UL)
>> #define PAGE_OFFSET_L4 _AC(0xffffaf8000000000, UL)
>> #define PAGE_OFFSET_L3 _AC(0xffffffd600000000, UL)
>> +#ifdef CONFIG_XIP_KERNEL
>> +#define PAGE_OFFSET PAGE_OFFSET_L3
>> #else
>> -#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
>> +#define PAGE_OFFSET kernel_map.page_offset
>> +#endif /* CONFIG_XIP_KERNEL */
>> +#else
>> +#define PAGE_OFFSET _AC(0xc0000000, UL)
>> #endif /* CONFIG_64BIT */
>> #else
>> #define PAGE_OFFSET ((unsigned long)phys_ram_base)
>> @@ -100,7 +101,6 @@ typedef struct page *pgtable_t;
>> #define ARCH_PFN_OFFSET (PFN_DOWN((unsigned long)phys_ram_base))
>>
>> struct kernel_mapping {
>> - unsigned long page_offset;
>> unsigned long virt_addr;
>> unsigned long virt_offset;
>> uintptr_t phys_addr;
>> @@ -114,6 +114,7 @@ struct kernel_mapping {
>> uintptr_t xiprom;
>> uintptr_t xiprom_sz;
>> #else
>> + unsigned long page_offset;
>> unsigned long va_kernel_pa_offset;
>> #endif
>> };
>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>> index d0190ee9b2e4..b8125c3aa85d 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -15,7 +15,7 @@
>> #ifdef CONFIG_RELOCATABLE
>> #define KERNEL_LINK_ADDR UL(0)
>> #else
>> -#define KERNEL_LINK_ADDR _AC(CONFIG_PAGE_OFFSET, UL)
>> +#define KERNEL_LINK_ADDR _AC(CONFIG_PHYS_RAM_BASE, UL)
>> #endif
>> #define KERN_VIRT_SIZE (UL(-1))
>> #else
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 0aad925848a4..a1a185e6faf0 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -843,6 +843,8 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
>> uintptr_t set_satp_mode_pmd = ((unsigned long)set_satp_mode) & PMD_MASK;
>> u64 satp_mode_cmdline = __pi_set_satp_mode_from_cmdline(dtb_pa);
>>
>> + kernel_map.page_offset = PAGE_OFFSET_L5;
>> +
>> if (satp_mode_cmdline == SATP_MODE_57) {
>> disable_pgtable_l5();
>> } else if (satp_mode_cmdline == SATP_MODE_48) {
>> @@ -1090,11 +1092,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>> kernel_map.virt_addr = KERNEL_LINK_ADDR + kernel_map.virt_offset;
>>
>> #ifdef CONFIG_XIP_KERNEL
>> -#ifdef CONFIG_64BIT
>> - kernel_map.page_offset = PAGE_OFFSET_L3;
>> -#else
>> - kernel_map.page_offset = _AC(CONFIG_PAGE_OFFSET, UL);
>> -#endif
>> kernel_map.xiprom = (uintptr_t)CONFIG_XIP_PHYS_ADDR;
>> kernel_map.xiprom_sz = (uintptr_t)(&_exiprom) - (uintptr_t)(&_xiprom);
>>
>> @@ -1106,7 +1103,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>> kernel_map.va_kernel_xip_data_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr
>> + (uintptr_t)&_sdata - (uintptr_t)&_start;
>> #else
>> - kernel_map.page_offset = _AC(CONFIG_PAGE_OFFSET, UL);
>> kernel_map.phys_addr = (uintptr_t)(&_start);
>> kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
>> kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
>> --
>> 2.45.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
Powered by blists - more mailing lists