Re: [Xen-devel] [PATCH 2/4] x86/page: Remove bifurcated PAGE_HYPERVISOR constant

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.01.2020 18:50, Andrew Cooper wrote:
> Despite being vaguely aware, the difference between PAGE_HYPERVISOR in ASM and
> C code has nevertheless caused several bugs I should have known better about,
> and contributed to review confusion.
> 
> There are exactly 4 uses of these constants in asm code (and one is shortly
> going to disappear).
> 
> Instead of creating the constants which behave differently between ASM and C
> code, expose all the constants and use non-ambiguous non-NX ones in ASM.

I'm okay with this in principle, but ...

> --- a/xen/include/asm-x86/x86_64/page.h
> +++ b/xen/include/asm-x86/x86_64/page.h
> @@ -172,18 +172,11 @@ static inline intpte_t put_pte_flags(unsigned int x)
>  #define PAGE_HYPERVISOR_RX      (__PAGE_HYPERVISOR_RX      | _PAGE_GLOBAL)
>  #define PAGE_HYPERVISOR_RWX     (__PAGE_HYPERVISOR         | _PAGE_GLOBAL)
>  
> -#ifdef __ASSEMBLY__
> -/* Dependency on NX being available can't be expressed. */
> -# define PAGE_HYPERVISOR         PAGE_HYPERVISOR_RWX
> -# define PAGE_HYPERVISOR_UCMINUS (__PAGE_HYPERVISOR_UCMINUS | _PAGE_GLOBAL)
> -# define PAGE_HYPERVISOR_UC      (__PAGE_HYPERVISOR_UC      | _PAGE_GLOBAL)
> -#else
>  # define PAGE_HYPERVISOR         PAGE_HYPERVISOR_RW
>  # define PAGE_HYPERVISOR_UCMINUS (__PAGE_HYPERVISOR_UCMINUS | \
>                                    _PAGE_GLOBAL | _PAGE_NX)
>  # define PAGE_HYPERVISOR_UC      (__PAGE_HYPERVISOR_UC | \
>                                    _PAGE_GLOBAL | _PAGE_NX)

... I'm afraid the assembler error resulting from someone actually
(and mistakenly) using one of the constants making use of _PAGE_NX
is going to be rather cryptic. Which in turn may motivate people
to actually try to make _PAGE_NX "work" in assembly code. Therefore
I'd like to ask that together with the changes here _PAGE_NX's
#define be framed by #ifndef __ASSEMBLY__, such that any
diagnostic, if it mentions a symbol name, would name the actual
problem, rather than a derived one.

Furthermore from a style perspective the blanks between # and
"define" will also want to go away now.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel