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

[Jan Beulich <jbeulich@suse.com>]

On 15.01.2020 13:53, Andrew Cooper wrote:
> On 14/01/2020 16:25, Jan Beulich wrote:
>>> --- 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.
> 
> I can do this, but it doesn't make the error any less cryptic.
> 
> With _PAGE_NX hidden:
> 
> head.S: Assembler messages:
> head.S:677: Error: invalid operands (*ABS* and *UND* sections) for `|'

This is something that could be improved in the future in gas
(by simply naming the symbol found to be *UND*).

> With it visible:
> 
> head.S: Assembler messages:
> head.S:677: Error: invalid character '?' in operand 1

This, otoh, can't sensibly be expected to see an improvement.
(Well, perhaps the full line could be quoted, but that's true
for about every parsing error.)

Jan

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