RE: [PATCH 1/5] xen/memory: Introduce CONFIG_ARCH_ACQUIRE_RESOURCE

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Sent: 28 July 2020 12:37
> To: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Jan Beulich 
> <JBeulich@xxxxxxxx>; Wei Liu <wl@xxxxxxx>;
> Roger Pau Monné <roger.pau@xxxxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Paul 
> Durrant <paul@xxxxxxx>; Michał
> Leszczyński <michal.leszczynski@xxxxxxx>; Hubert Jasudowicz 
> <hubert.jasudowicz@xxxxxxx>
> Subject: [PATCH 1/5] xen/memory: Introduce CONFIG_ARCH_ACQUIRE_RESOURCE
> 
> New architectures shouldn't be forced to implement no-op stubs for unused
> functionality.
> 
> Introduce CONFIG_ARCH_ACQUIRE_RESOURCE which can be opted in to, and provide
> compatibility logic in xen/mm.h
> 
> No functional change.

Code-wise, it looks fine, so...

Reviewed-by: Paul Durrant <paul@xxxxxxx>

...but ...

> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Paul Durrant <paul@xxxxxxx>
> CC: Michał Leszczyński <michal.leszczynski@xxxxxxx>
> CC: Hubert Jasudowicz <hubert.jasudowicz@xxxxxxx>
> ---
>  xen/arch/x86/Kconfig     | 1 +
>  xen/common/Kconfig       | 3 +++
>  xen/include/asm-arm/mm.h | 8 --------
>  xen/include/xen/mm.h     | 9 +++++++++
>  4 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index a636a4bb1e..e7644a0a9d 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -6,6 +6,7 @@ config X86
>       select ACPI
>       select ACPI_LEGACY_TABLES_LOOKUP
>       select ARCH_SUPPORTS_INT128
> +     select ARCH_ACQUIRE_RESOURCE

... I do wonder whether 'HAS_ACQUIRE_RESOURCE' is a better and more descriptive 
name.

>       select COMPAT
>       select CORE_PARKING
>       select HAS_ALTERNATIVE
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 15e3b79ff5..593459ea6e 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -22,6 +22,9 @@ config GRANT_TABLE
> 
>         If unsure, say Y.
> 
> +config ARCH_ACQUIRE_RESOURCE
> +     bool
> +
>  config HAS_ALTERNATIVE
>       bool
> 
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index f8ba49b118..0b7de3102e 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -358,14 +358,6 @@ static inline void put_page_and_type(struct page_info 
> *page)
> 
>  void clear_and_clean_page(struct page_info *page);
> 
> -static inline
> -int arch_acquire_resource(struct domain *d, unsigned int type, unsigned int 
> id,
> -                          unsigned long frame, unsigned int nr_frames,
> -                          xen_pfn_t mfn_list[])
> -{
> -    return -EOPNOTSUPP;
> -}
> -
>  unsigned int arch_get_dma_bitsize(void);
> 
>  #endif /*  __ARCH_ARM_MM__ */
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index 1061765bcd..1b2c1f6b32 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -685,4 +685,13 @@ static inline void put_page_alloc_ref(struct page_info 
> *page)
>      }
>  }
> 
> +#ifndef CONFIG_ARCH_ACQUIRE_RESOURCE
> +static inline int arch_acquire_resource(
> +    struct domain *d, unsigned int type, unsigned int id, unsigned long 
> frame,
> +    unsigned int nr_frames, xen_pfn_t mfn_list[])
> +{
> +    return -EOPNOTSUPP;
> +}
> +#endif /* !CONFIG_ARCH_ACQUIRE_RESOURCE */
> +
>  #endif /* __XEN_MM_H__ */
> --
> 2.11.0