Re: [PATCH] openrisc: Implement fixmap to fix eralycon

[Stafford Horne <shorne@gmail.com>]

Review from myself, I sent this one out pretty quickly just to get some
conversation started.

In the subject if should be 'earlycon' no 'eralycon'.

On Fri, Sep 27, 2024 at 03:56:56PM +0100, Stafford Horne wrote:
> With commit 53c98e35dcbc ("openrisc: mm: remove unneeded early ioremap
> code") it was commented that early ioremap was not used in OpenRISC.  I
> acked this but was wrong.  Earlycon now fails with the below trace:
..
>
> To fix this we could either implement early_ioremap or implement fixmap.
> In this patch we choose the later option of implementing basic fixmap
> support.
> 
> While fixing this we also remove the old FIX_IOREMAP slots that were
> used by early ioremap code.  That code was also removed by commit
> 53c98e35dcbc ("openrisc: mm: remove unneeded early ioremap code") but
> these definitions were not cleaned up.
> 
> Fixes: 53c98e35dcbc ("openrisc: mm: remove unneeded early ioremap code")
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  arch/openrisc/Kconfig              |  3 +++
>  arch/openrisc/include/asm/fixmap.h | 21 +++++-------------
>  arch/openrisc/mm/init.c            | 34 ++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index 69c0258700b2..3279ef457c57 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -65,6 +65,9 @@ config STACKTRACE_SUPPORT
>  config LOCKDEP_SUPPORT
>  	def_bool  y
>  
> +config FIX_EARLYCON_MEM
> +	def_bool y
> +
>  menu "Processor type and features"
>  
>  choice
> diff --git a/arch/openrisc/include/asm/fixmap.h b/arch/openrisc/include/asm/fixmap.h
> index ecdb98a5839f..aaa6a26a3e92 100644
> --- a/arch/openrisc/include/asm/fixmap.h
> +++ b/arch/openrisc/include/asm/fixmap.h
> @@ -26,29 +26,18 @@
>  #include <linux/bug.h>
>  #include <asm/page.h>
>  
> -/*
> - * On OpenRISC we use these special fixed_addresses for doing ioremap
> - * early in the boot process before memory initialization is complete.
> - * This is used, in particular, by the early serial console code.
> - *
> - * It's not really 'fixmap', per se, but fits loosely into the same
> - * paradigm.
> - */
>  enum fixed_addresses {
> -	/*
> -	 * FIX_IOREMAP entries are useful for mapping physical address
> -	 * space before ioremap() is useable, e.g. really early in boot
> -	 * before kmalloc() is working.
> -	 */
> -#define FIX_N_IOREMAPS  32
> -	FIX_IOREMAP_BEGIN,
> -	FIX_IOREMAP_END = FIX_IOREMAP_BEGIN + FIX_N_IOREMAPS - 1,
> +	FIX_EARLYCON_MEM_BASE,
>  	__end_of_fixed_addresses
>  };
>  
>  #define FIXADDR_SIZE		(__end_of_fixed_addresses << PAGE_SHIFT)
>  /* FIXADDR_BOTTOM might be a better name here... */
>  #define FIXADDR_START		(FIXADDR_TOP - FIXADDR_SIZE)
> +#define FIXMAP_PAGE_IO		PAGE_KERNEL_NOCACHE
> +
> +extern void __set_fixmap(enum fixed_addresses idx,
> +			 phys_addr_t phys, pgprot_t flags);
>  
>  #include <asm-generic/fixmap.h>
>  
> diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> index 1dcd78c8f0e9..7397d18c95d7 100644
> --- a/arch/openrisc/mm/init.c
> +++ b/arch/openrisc/mm/init.c
> @@ -207,6 +207,40 @@ void __init mem_init(void)
>  	return;
>  }
>  
> +static int __init map_page(unsigned long va, phys_addr_t pa, int flags)
> +{
> +	p4d_t *p4d;
> +	pud_t *pud;
> +	pmd_t *pd;
> +	pte_t *pg;
> +	int err = -ENOMEM;
> +
> +	p4d = p4d_offset(pgd_offset_k(va), va);
> +	pud = pud_offset(p4d, va);
> +	pd = pmd_offset(pud, va);
> +	pg = pte_alloc_kernel(pd, va);
> +
> +	if (pg != NULL) {
> +		err = 0;
> +		set_pte_at(&init_mm, va, pg, pfn_pte(pa >> PAGE_SHIFT,
> +				__pgprot(flags)));

We maybe need to do something like flush tlb here, but since this is only used
by earlycon and before any io access is made it may be ok.

> +	}
> +	return err;
> +}
> +
> +void __init __set_fixmap(enum fixed_addresses idx,
> +			 phys_addr_t phys, pgprot_t prot)
> +{
> +	unsigned long address = __fix_to_virt(idx);
> +
> +	if (idx >= __end_of_fixed_addresses) {
> +		BUG();
> +		return;
> +	}
> +
> +	map_page(address, phys, pgprot_val(prot));
> +}
> +
>  static const pgprot_t protection_map[16] = {
>  	[VM_NONE]					= PAGE_NONE,
>  	[VM_READ]					= PAGE_READONLY_X,
> -- 
> 2.44.0
>