All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien@xen.org>
Cc: xen-devel@lists.xenproject.org, Julien Grall <jgrall@amazon.com>,
	 Stefano Stabellini <sstabellini@kernel.org>,
	 Bertrand Marquis <bertrand.marquis@arm.com>,
	 Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	 Andrew Cooper <andrew.cooper3@citrix.com>,
	 George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,  Wei Liu <wl@xen.org>
Subject: Re: [PATCH 09/16] xen/arm: Move fixmap definitions in a separate header
Date: Tue, 7 Jun 2022 18:08:57 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2206071808350.21215@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <20220520120937.28925-10-julien@xen.org>

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> To use properly the fixmap definitions, their user would need
> also new to include <xen/acpi.h>. This is not very great when
> the user itself is not meant to directly use ACPI definitions.
> 
> Including <xen/acpi.h> in <asm/config.h> is not option because
> the latter header is included by everyone. So move out the fixmap
> entries definition in a new header.
> 
> Take the opportunity to also move {set, clear}_fixmap() prototypes
> in the new header.
> 
> Note that most of the definitions in <xen/acpi.h> now need to be
> surrounded with #ifndef __ASSEMBLY__ because <asm/fixmap.h> will
> be used in assembly (see EARLY_UART_VIRTUAL_ADDRESS).
> 
> The split will become more helpful in a follow-up patch where new
> fixmap entries will be defined.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>     There was some disagreement with Stefano on whether fixmap.h
>     should include acpi.h or this should be the other way around.
> 
>     I chose the former because each components should decide how
>     much entries in the fixmap they need and also because this is
>     the current behavior on x86. We should stay consitent
>     between arch to avoid any headers mess.
> 
>     Jan acked this patch, so I am assuming he is happy with this
>     approach. I would be OK to rework it if others agree with
>     Stefano's view.

noone is speaking so:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>



>     Changes in v4:
>         - Add Jan's acked-by
>         - Record Stefano's disagreement on the approach
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/acpi/lib.c                 |  2 ++
>  xen/arch/arm/include/asm/config.h       |  6 ------
>  xen/arch/arm/include/asm/early_printk.h |  1 +
>  xen/arch/arm/include/asm/fixmap.h       | 24 ++++++++++++++++++++++++
>  xen/arch/arm/include/asm/mm.h           |  4 ----
>  xen/arch/arm/kernel.c                   |  1 +
>  xen/arch/arm/mm.c                       |  1 +
>  xen/include/xen/acpi.h                  | 18 +++++++++++-------
>  8 files changed, 40 insertions(+), 17 deletions(-)
>  create mode 100644 xen/arch/arm/include/asm/fixmap.h
> 
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index a59cc4074cfb..41d521f720ac 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -25,6 +25,8 @@
>  #include <xen/init.h>
>  #include <xen/mm.h>
>  
> +#include <asm/fixmap.h>
> +
>  static bool fixmap_inuse;
>  
>  char *__acpi_map_table(paddr_t phys, unsigned long size)
> diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
> index b25c9d39bb32..3e2a55a91058 100644
> --- a/xen/arch/arm/include/asm/config.h
> +++ b/xen/arch/arm/include/asm/config.h
> @@ -169,12 +169,6 @@
>  
>  #endif
>  
> -/* Fixmap slots */
> -#define FIXMAP_CONSOLE  0  /* The primary UART */
> -#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
> -#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
> -#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> -
>  #define NR_hypercalls 64
>  
>  #define STACK_ORDER 3
> diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
> index 8dc911cf48a3..c5149b2976da 100644
> --- a/xen/arch/arm/include/asm/early_printk.h
> +++ b/xen/arch/arm/include/asm/early_printk.h
> @@ -11,6 +11,7 @@
>  #define __ARM_EARLY_PRINTK_H__
>  
>  #include <xen/page-size.h>
> +#include <asm/fixmap.h>
>  
>  #ifdef CONFIG_EARLY_PRINTK
>  
> diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
> new file mode 100644
> index 000000000000..1cee51e52ab9
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/fixmap.h
> @@ -0,0 +1,24 @@
> +/*
> + * fixmap.h: compile-time virtual memory allocation
> + */
> +#ifndef __ASM_FIXMAP_H
> +#define __ASM_FIXMAP_H
> +
> +#include <xen/acpi.h>
> +
> +/* Fixmap slots */
> +#define FIXMAP_CONSOLE  0  /* The primary UART */
> +#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
> +#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
> +#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> +
> +#ifndef __ASSEMBLY__
> +
> +/* Map a page in a fixmap entry */
> +extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
> +/* Remove a mapping from a fixmap entry */
> +extern void clear_fixmap(unsigned map);
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __ASM_FIXMAP_H */
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 424aaf28230b..045a8ba4bb63 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -191,10 +191,6 @@ extern void mmu_init_secondary_cpu(void);
>  extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long nr_mfns);
>  /* Map a frame table to cover physical addresses ps through pe */
>  extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
> -/* Map a 4k page in a fixmap entry */
> -extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
> -/* Remove a mapping from a fixmap entry */
> -extern void clear_fixmap(unsigned map);
>  /* map a physical range in virtual memory */
>  void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned attributes);
>  
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 8f43caa1866d..25ded1c056d9 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -15,6 +15,7 @@
>  #include <xen/vmap.h>
>  
>  #include <asm/byteorder.h>
> +#include <asm/fixmap.h>
>  #include <asm/kernel.h>
>  #include <asm/setup.h>
>  
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 6b7b72de27fe..52b2a0394047 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -41,6 +41,7 @@
>  #include <xen/sizes.h>
>  #include <xen/libfdt/libfdt.h>
>  
> +#include <asm/fixmap.h>
>  #include <asm/setup.h>
>  
>  /* Override macros from asm/page.h to make them work with mfn_t */
> diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
> index 39d51fcd01dd..1b9c75e68fc4 100644
> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -28,6 +28,15 @@
>  #define _LINUX
>  #endif
>  
> +/*
> + * Fixmap pages to reserve for ACPI boot-time tables (see
> + * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/fixmap.h),
> + * 64 pages(256KB) is large enough for most cases.)
> + */
> +#define NUM_FIXMAP_ACPI_PAGES  64
> +
> +#ifndef __ASSEMBLY__
> +
>  #include <xen/list.h>
>  
>  #include <acpi/acpi.h>
> @@ -39,13 +48,6 @@
>  #define ACPI_MADT_GET_POLARITY(inti)	ACPI_MADT_GET_(POLARITY, inti)
>  #define ACPI_MADT_GET_TRIGGER(inti)	ACPI_MADT_GET_(TRIGGER, inti)
>  
> -/*
> - * Fixmap pages to reserve for ACPI boot-time tables (see
> - * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/config.h,
> - * 64 pages(256KB) is large enough for most cases.)
> - */
> -#define NUM_FIXMAP_ACPI_PAGES  64
> -
>  #define BAD_MADT_ENTRY(entry, end) (                                        \
>                  (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) ||  \
>                  (entry)->header.length < sizeof(*(entry)))
> @@ -207,4 +209,6 @@ void acpi_reboot(void);
>  void acpi_dmar_zap(void);
>  void acpi_dmar_reinstate(void);
>  
> +#endif /* __ASSEMBLY__ */
> +
>  #endif /*_LINUX_ACPI_H*/
> -- 
> 2.32.0
> 


  reply	other threads:[~2022-06-08  1:09 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20 12:09 [PATCH 00/16] xen/arm: mm: Remove open-coding mappings Julien Grall
2022-05-20 12:09 ` [PATCH 01/16] xen/arm: mm: Allow other mapping size in xen_pt_update_entry() Julien Grall
2022-06-03 22:27   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 02/16] xen/arm: mm: Add support for the contiguous bit Julien Grall
2022-05-20 12:09 ` [PATCH 03/16] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted Julien Grall
2022-05-26 15:28   ` Luca Fancellu
2022-06-03 22:30   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 04/16] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings() Julien Grall
2022-06-03 22:31   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 05/16] xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen() Julien Grall
2022-06-03 22:33   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 06/16] xen/arm32: mm: Re-implement setup_xenheap_mappings() " Julien Grall
2022-05-20 12:09 ` [PATCH 07/16] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap Julien Grall
2022-05-20 12:09 ` [PATCH 08/16] xen/arm: mm: Allow page-table allocation from the boot allocator Julien Grall
2022-05-20 12:09 ` [PATCH 09/16] xen/arm: Move fixmap definitions in a separate header Julien Grall
2022-06-08  1:08   ` Stefano Stabellini [this message]
2022-05-20 12:09 ` [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure Julien Grall
2022-05-21 11:38   ` Julien Grall
2022-05-24  2:11   ` Wei Chen
2022-05-24  8:58     ` Julien Grall
2022-05-26 15:55   ` Luca Fancellu
2022-06-08  1:08   ` Stefano Stabellini
2022-06-08 10:01     ` Julien Grall
2022-06-09  0:25       ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 11/16] xen/arm: mm: Clean-up the includes and order them Julien Grall
2022-05-20 12:09 ` [PATCH 12/16] xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table() Julien Grall
2022-05-20 12:09 ` [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator Julien Grall
2022-05-23  7:28   ` Michal Orzel
2022-05-23 19:51     ` Julien Grall
2022-05-24  6:12       ` Michal Orzel
2022-05-24  7:57   ` Bertrand Marquis
2022-06-03 23:09   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 14/16] xen/arm64: mm: Add memory to the boot allocator first Julien Grall
2022-05-26 17:10   ` Luca Fancellu
2022-06-03 23:09   ` Stefano Stabellini
2022-05-20 12:09 ` [PATCH 15/16] xen/arm: mm: Rework setup_xenheap_mappings() Julien Grall
2022-05-20 12:09 ` [PATCH 16/16] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen() Julien Grall
2022-05-26 17:24   ` Luca Fancellu
2022-06-03 23:09   ` Stefano Stabellini
2022-06-08  1:14   ` Stefano Stabellini
2022-06-11 11:30 ` [PATCH 00/16] xen/arm: mm: Remove open-coding mappings Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2206071808350.21215@ubuntu-linux-20-04-desktop \
    --to=sstabellini@kernel.org \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=george.dunlap@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jgrall@amazon.com \
    --cc=julien@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH 09/16] xen/arm: Move fixmap definitions in a separate header' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.