Re: [PATCH v5 16/23] x86/mm: add pv prefix to {alloc, free}_page_type

From: "Jan Beulich" <JBeulich@suse.com>
To: WeiLiu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v5 16/23] x86/mm: add pv prefix to {alloc, free}_page_type
Date: Fri, 22 Sep 2017 07:40:04 -0600	[thread overview]
Message-ID: <59C52ED4020000780017EADB@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20170914125852.22129-17-wei.liu2@citrix.com>

>>> On 14.09.17 at 14:58, <wei.liu2@citrix.com> wrote:
> And move the declarations to pv/mm.h. The code will be moved later.
> 
> The stubs contain BUG() because they aren't supposed to be called when
> PV is disabled.

I'd prefer ASSERT_UNREACHABLE() - they return proper errors
after all, and there's no need to bring down a production system.
Additionally could you add (half) a sentence regarding the
PGT_l*_page_table uses outside of PV specific code, which I'm
sure you have verified can't make it into the stubs?

> --- a/xen/include/asm-x86/pv/mm.h
> +++ b/xen/include/asm-x86/pv/mm.h
> @@ -32,6 +32,11 @@ bool pv_map_ldt_shadow_page(unsigned int off);
>  
>  void pv_arch_init_memory(void);
>  
> +int pv_alloc_page_type(struct page_info *page, unsigned long type,
> +                       int preemptible);
> +int pv_free_page_type(struct page_info *page, unsigned long type,
> +                      int preemptible);
> +
>  #else
>  
>  #include <xen/errno.h>
> @@ -51,6 +56,13 @@ static inline bool pv_map_ldt_shadow_page(unsigned int off) { return false; }
>  
>  static inline void pv_arch_init_memory(void) {}
>  
> +static inline int pv_alloc_page_type(struct page_info *page, unsigned long type,
> +                                     int preemptible)
> +{ BUG(); return -EINVAL; }
> +static inline int pv_free_page_type(struct page_info *page, unsigned long type,
> +                                    int preemptible)
> +{ BUG(); return -EINVAL; }

Take the opportunity and make all the "preemptible" parameters bool?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel