lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ