[PATCH] x86/Xen: tidy xen-head.S

Jan Beulich posted 1 patch 1 year, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
[PATCH] x86/Xen: tidy xen-head.S
Posted by Jan Beulich 1 year, 2 months ago
First of all drop 32-bit leftovers, including the inclusion of the file
from head_32.S. Then further move PV-only ELF notes inside the XEN_PV
conditional. Finally have the "supported features" note actually report
reality: All three of the features are supported and/or applicable only
in certain cases.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -524,8 +524,6 @@ __INITRODATA
 int_msg:
 	.asciz "Unknown interrupt or fault at: %p %p %p\n"
 
-#include "../../x86/xen/xen-head.S"
-
 /*
  * The IDT and GDT 'descriptors' are a strange 48-bit object
  * only used by the lidt and lgdt instructions. They are not
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -83,27 +83,33 @@ SYM_CODE_END(asm_cpu_bringup_and_idle)
 	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
 	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
 	ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
-#ifdef CONFIG_X86_32
-	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __PAGE_OFFSET)
-#else
 	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)
 	/* Map the p2m table to a 512GB-aligned user address. */
 	ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))
-#endif
 #ifdef CONFIG_XEN_PV
 	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
+	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii "!writable_page_tables")
+	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
+	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
+		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
+# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
+#else
+# define FEATURES_PV 0
+#endif
+#ifdef CONFIG_XEN_PVH
+# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
+#else
+# define FEATURES_PVH 0
+#endif
+#ifdef CONFIG_XEN_DOM0
+# define FEATURES_DOM0 (1 << XENFEAT_dom0)
+#else
+# define FEATURES_DOM0 0
 #endif
 	ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR hypercall_page)
-	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
-		.ascii "!writable_page_tables|pae_pgdir_above_4gb")
 	ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
-		.long (1 << XENFEAT_writable_page_tables) |       \
-		      (1 << XENFEAT_dom0) |                       \
-		      (1 << XENFEAT_linux_rsdp_unrestricted))
-	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
+		.long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
 	ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
-	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
-		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
 	ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
 	ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
 	ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR __HYPERVISOR_VIRT_START)
Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Juergen Gross 1 year, 2 months ago
On 15.02.23 12:33, Jan Beulich wrote:
> First of all drop 32-bit leftovers, including the inclusion of the file
> from head_32.S.

I don't see why we would want to drop 32-bit HVM and PVH support.

> Then further move PV-only ELF notes inside the XEN_PV
> conditional. Finally have the "supported features" note actually report
> reality: All three of the features are supported and/or applicable only
> in certain cases.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/arch/x86/kernel/head_32.S
> +++ b/arch/x86/kernel/head_32.S
> @@ -524,8 +524,6 @@ __INITRODATA
>   int_msg:
>   	.asciz "Unknown interrupt or fault at: %p %p %p\n"
>   
> -#include "../../x86/xen/xen-head.S"
> -

This is wrong for non-PV cases.

>   /*
>    * The IDT and GDT 'descriptors' are a strange 48-bit object
>    * only used by the lidt and lgdt instructions. They are not
> --- a/arch/x86/xen/xen-head.S
> +++ b/arch/x86/xen/xen-head.S
> @@ -83,27 +83,33 @@ SYM_CODE_END(asm_cpu_bringup_and_idle)
>   	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
>   	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
>   	ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
> -#ifdef CONFIG_X86_32
> -	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __PAGE_OFFSET)
> -#else
>   	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)

This will be wrong for 32-bit guests now. I'm not sure the value is really
used in Xen for non-PV guests, though.

>   	/* Map the p2m table to a 512GB-aligned user address. */
>   	ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))

Move this under the CONFIG_PV umbrella?

> -#endif
>   #ifdef CONFIG_XEN_PV
>   	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
> +	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii "!writable_page_tables")
> +	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
> +	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
> +		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
> +# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
> +#else
> +# define FEATURES_PV 0
> +#endif
> +#ifdef CONFIG_XEN_PVH
> +# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
> +#else
> +# define FEATURES_PVH 0
> +#endif
> +#ifdef CONFIG_XEN_DOM0
> +# define FEATURES_DOM0 (1 << XENFEAT_dom0)
> +#else
> +# define FEATURES_DOM0 0
>   #endif
>   	ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR hypercall_page)
> -	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
> -		.ascii "!writable_page_tables|pae_pgdir_above_4gb")
>   	ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
> -		.long (1 << XENFEAT_writable_page_tables) |       \
> -		      (1 << XENFEAT_dom0) |                       \
> -		      (1 << XENFEAT_linux_rsdp_unrestricted))
> -	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
> +		.long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
>   	ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
> -	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
> -		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>   	ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
>   	ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
>   	ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR __HYPERVISOR_VIRT_START)

Are XEN_ELFNOTE_MOD_START_PFN and XEN_ELFNOTE_HV_START_LOW really relevant
for the non-PV case? I don't think so (in theory XEN_ELFNOTE_MOD_START_PFN
could be used, but the main reason for its introduction was PV guests IIRC).


Juergen
Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Jan Beulich 1 year, 2 months ago
On 15.02.2023 13:05, Juergen Gross wrote:
> On 15.02.23 12:33, Jan Beulich wrote:
>> First of all drop 32-bit leftovers, including the inclusion of the file
>> from head_32.S.
> 
> I don't see why we would want to drop 32-bit HVM and PVH support.

HVM doesn't use this, does it? But yes, the PVH aspect as well as ...

>> --- a/arch/x86/kernel/head_32.S
>> +++ b/arch/x86/kernel/head_32.S
>> @@ -524,8 +524,6 @@ __INITRODATA
>>   int_msg:
>>   	.asciz "Unknown interrupt or fault at: %p %p %p\n"
>>   
>> -#include "../../x86/xen/xen-head.S"
>> -
> 
> This is wrong for non-PV cases.

... this and ...

>> --- a/arch/x86/xen/xen-head.S
>> +++ b/arch/x86/xen/xen-head.S
>> @@ -83,27 +83,33 @@ SYM_CODE_END(asm_cpu_bringup_and_idle)
>>   	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
>>   	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
>>   	ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
>> -#ifdef CONFIG_X86_32
>> -	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __PAGE_OFFSET)
>> -#else
>>   	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)
> 
> This will be wrong for 32-bit guests now. I'm not sure the value is really
> used in Xen for non-PV guests, though.
> 
>>   	/* Map the p2m table to a 512GB-aligned user address. */
>>   	ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))
> 
> Move this under the CONFIG_PV umbrella?

... these occurred to me over lunch (and I was hoping to be able to point
out my mistake before getting feedback). I'll check whether VIRT_BASE can
also be moved into the PV-only section.

>> -#endif
>>   #ifdef CONFIG_XEN_PV
>>   	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
>> +	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii "!writable_page_tables")
>> +	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>> +	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>> +		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>> +# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
>> +#else
>> +# define FEATURES_PV 0
>> +#endif
>> +#ifdef CONFIG_XEN_PVH
>> +# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
>> +#else
>> +# define FEATURES_PVH 0
>> +#endif
>> +#ifdef CONFIG_XEN_DOM0
>> +# define FEATURES_DOM0 (1 << XENFEAT_dom0)
>> +#else
>> +# define FEATURES_DOM0 0
>>   #endif
>>   	ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR hypercall_page)
>> -	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
>> -		.ascii "!writable_page_tables|pae_pgdir_above_4gb")
>>   	ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
>> -		.long (1 << XENFEAT_writable_page_tables) |       \
>> -		      (1 << XENFEAT_dom0) |                       \
>> -		      (1 << XENFEAT_linux_rsdp_unrestricted))
>> -	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>> +		.long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
>>   	ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
>> -	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>> -		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>   	ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
>>   	ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
>>   	ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR __HYPERVISOR_VIRT_START)
> 
> Are XEN_ELFNOTE_MOD_START_PFN and XEN_ELFNOTE_HV_START_LOW really relevant
> for the non-PV case? I don't think so (in theory XEN_ELFNOTE_MOD_START_PFN
> could be used, but the main reason for its introduction was PV guests IIRC).

I wasn't sufficiently certain for MOD_START_PFN, so I'd prefer to leave it
untouched for now. HV_START_LOW might be 32-bit PV only really; I'll check
and then maybe drop (or move).

Jan
Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Juergen Gross 1 year, 2 months ago
On 15.02.23 13:42, Jan Beulich wrote:
> On 15.02.2023 13:05, Juergen Gross wrote:
>> On 15.02.23 12:33, Jan Beulich wrote:
>>> First of all drop 32-bit leftovers, including the inclusion of the file
>>> from head_32.S.
>>
>> I don't see why we would want to drop 32-bit HVM and PVH support.
> 
> HVM doesn't use this, does it? But yes, the PVH aspect as well as ...

hypercall_page is located in xen-head.S.

> 
>>> --- a/arch/x86/kernel/head_32.S
>>> +++ b/arch/x86/kernel/head_32.S
>>> @@ -524,8 +524,6 @@ __INITRODATA
>>>    int_msg:
>>>    	.asciz "Unknown interrupt or fault at: %p %p %p\n"
>>>    
>>> -#include "../../x86/xen/xen-head.S"
>>> -
>>
>> This is wrong for non-PV cases.
> 
> ... this and ...
> 
>>> --- a/arch/x86/xen/xen-head.S
>>> +++ b/arch/x86/xen/xen-head.S
>>> @@ -83,27 +83,33 @@ SYM_CODE_END(asm_cpu_bringup_and_idle)
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
>>> -#ifdef CONFIG_X86_32
>>> -	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __PAGE_OFFSET)
>>> -#else
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)
>>
>> This will be wrong for 32-bit guests now. I'm not sure the value is really
>> used in Xen for non-PV guests, though.
>>
>>>    	/* Map the p2m table to a 512GB-aligned user address. */
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))
>>
>> Move this under the CONFIG_PV umbrella?
> 
> ... these occurred to me over lunch (and I was hoping to be able to point
> out my mistake before getting feedback). I'll check whether VIRT_BASE can
> also be moved into the PV-only section.

Thanks.

> 
>>> -#endif
>>>    #ifdef CONFIG_XEN_PV
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
>>> +	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii "!writable_page_tables")
>>> +	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>> +	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>> +		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>> +# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
>>> +#else
>>> +# define FEATURES_PV 0
>>> +#endif
>>> +#ifdef CONFIG_XEN_PVH
>>> +# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
>>> +#else
>>> +# define FEATURES_PVH 0
>>> +#endif
>>> +#ifdef CONFIG_XEN_DOM0
>>> +# define FEATURES_DOM0 (1 << XENFEAT_dom0)
>>> +#else
>>> +# define FEATURES_DOM0 0
>>>    #endif
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR hypercall_page)
>>> -	ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
>>> -		.ascii "!writable_page_tables|pae_pgdir_above_4gb")
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
>>> -		.long (1 << XENFEAT_writable_page_tables) |       \
>>> -		      (1 << XENFEAT_dom0) |                       \
>>> -		      (1 << XENFEAT_linux_rsdp_unrestricted))
>>> -	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>> +		.long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
>>> -	ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>> -		.quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
>>>    	ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR __HYPERVISOR_VIRT_START)
>>
>> Are XEN_ELFNOTE_MOD_START_PFN and XEN_ELFNOTE_HV_START_LOW really relevant
>> for the non-PV case? I don't think so (in theory XEN_ELFNOTE_MOD_START_PFN
>> could be used, but the main reason for its introduction was PV guests IIRC).
> 
> I wasn't sufficiently certain for MOD_START_PFN, so I'd prefer to leave it
> untouched for now. HV_START_LOW might be 32-bit PV only really; I'll check
> and then maybe drop (or move).

Fine with me.


Juergen
Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Andrew Cooper 1 year, 2 months ago
On 15/02/2023 1:12 pm, Juergen Gross wrote:
> On 15.02.23 13:42, Jan Beulich wrote:
>> On 15.02.2023 13:05, Juergen Gross wrote:
>>> On 15.02.23 12:33, Jan Beulich wrote:
>>>> First of all drop 32-bit leftovers, including the inclusion of the
>>>> file
>>>> from head_32.S.
>>>
>>> I don't see why we would want to drop 32-bit HVM and PVH support.
>>
>> HVM doesn't use this, does it? But yes, the PVH aspect as well as ...
>
> hypercall_page is located in xen-head.S.
>
>>
>>>> --- a/arch/x86/kernel/head_32.S
>>>> +++ b/arch/x86/kernel/head_32.S
>>>> @@ -524,8 +524,6 @@ __INITRODATA
>>>>    int_msg:
>>>>        .asciz "Unknown interrupt or fault at: %p %p %p\n"
>>>>    -#include "../../x86/xen/xen-head.S"
>>>> -
>>>
>>> This is wrong for non-PV cases.
>>
>> ... this and ...
>>
>>>> --- a/arch/x86/xen/xen-head.S
>>>> +++ b/arch/x86/xen/xen-head.S
>>>> @@ -83,27 +83,33 @@ SYM_CODE_END(asm_cpu_bringup_and_idle)
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
>>>> -#ifdef CONFIG_X86_32
>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __PAGE_OFFSET)
>>>> -#else
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR
>>>> __START_KERNEL_map)
>>>
>>> This will be wrong for 32-bit guests now. I'm not sure the value is
>>> really
>>> used in Xen for non-PV guests, though.
>>>
>>>>        /* Map the p2m table to a 512GB-aligned user address. */
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE *
>>>> PTRS_PER_PUD))
>>>
>>> Move this under the CONFIG_PV umbrella?
>>
>> ... these occurred to me over lunch (and I was hoping to be able to
>> point
>> out my mistake before getting feedback). I'll check whether VIRT_BASE
>> can
>> also be moved into the PV-only section.
>
> Thanks.
>
>>
>>>> -#endif
>>>>    #ifdef CONFIG_XEN_PV
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii
>>>> "!writable_page_tables")
>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>>> +        .quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>>> +# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
>>>> +#else
>>>> +# define FEATURES_PV 0
>>>> +#endif
>>>> +#ifdef CONFIG_XEN_PVH
>>>> +# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
>>>> +#else
>>>> +# define FEATURES_PVH 0
>>>> +#endif
>>>> +#ifdef CONFIG_XEN_DOM0
>>>> +# define FEATURES_DOM0 (1 << XENFEAT_dom0)
>>>> +#else
>>>> +# define FEATURES_DOM0 0
>>>>    #endif
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR
>>>> hypercall_page)
>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
>>>> -        .ascii "!writable_page_tables|pae_pgdir_above_4gb")
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
>>>> -        .long (1 << XENFEAT_writable_page_tables) |       \
>>>> -              (1 << XENFEAT_dom0) |                       \
>>>> -              (1 << XENFEAT_linux_rsdp_unrestricted))
>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>>> +        .long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>>> -        .quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
>>>>        ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR
>>>> __HYPERVISOR_VIRT_START)
>>>
>>> Are XEN_ELFNOTE_MOD_START_PFN and XEN_ELFNOTE_HV_START_LOW really
>>> relevant
>>> for the non-PV case? I don't think so (in theory
>>> XEN_ELFNOTE_MOD_START_PFN
>>> could be used, but the main reason for its introduction was PV
>>> guests IIRC).
>>
>> I wasn't sufficiently certain for MOD_START_PFN, so I'd prefer to
>> leave it
>> untouched for now. HV_START_LOW might be 32-bit PV only really; I'll
>> check
>> and then maybe drop (or move).
>
> Fine with me.

HV_START_LOW is PV32 only.  It's the negotiation for the virtual address
split with Xen, and was never implemented properly for PV64.

MOD_START_PFN is PV only.  It's not applicable for HVM/PVH.

For PVH guests, XEN_ELFNOTE_PHYS32_ENTRY really is the only necessary
note, and that's what XTF uses.

~Andrew

Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Jan Beulich 1 year, 2 months ago
On 15.02.2023 14:25, Andrew Cooper wrote:
> On 15/02/2023 1:12 pm, Juergen Gross wrote:
>> On 15.02.23 13:42, Jan Beulich wrote:
>>> On 15.02.2023 13:05, Juergen Gross wrote:
>>>> On 15.02.23 12:33, Jan Beulich wrote:
>>>>> -#endif
>>>>>    #ifdef CONFIG_XEN_PV
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
>>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,       .ascii
>>>>> "!writable_page_tables")
>>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>>>> +    ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>>>> +        .quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>>>> +# define FEATURES_PV (1 << XENFEAT_writable_page_tables)
>>>>> +#else
>>>>> +# define FEATURES_PV 0
>>>>> +#endif
>>>>> +#ifdef CONFIG_XEN_PVH
>>>>> +# define FEATURES_PVH (1 << XENFEAT_linux_rsdp_unrestricted)
>>>>> +#else
>>>>> +# define FEATURES_PVH 0
>>>>> +#endif
>>>>> +#ifdef CONFIG_XEN_DOM0
>>>>> +# define FEATURES_DOM0 (1 << XENFEAT_dom0)
>>>>> +#else
>>>>> +# define FEATURES_DOM0 0
>>>>>    #endif
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR
>>>>> hypercall_page)
>>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,
>>>>> -        .ascii "!writable_page_tables|pae_pgdir_above_4gb")
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,
>>>>> -        .long (1 << XENFEAT_writable_page_tables) |       \
>>>>> -              (1 << XENFEAT_dom0) |                       \
>>>>> -              (1 << XENFEAT_linux_rsdp_unrestricted))
>>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz "yes")
>>>>> +        .long FEATURES_PV | FEATURES_PVH | FEATURES_DOM0)
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz "generic")
>>>>> -    ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID,
>>>>> -        .quad _PAGE_PRESENT; .quad _PAGE_PRESENT)
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_SUSPEND_CANCEL, .long 1)
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_MOD_START_PFN,  .long 1)
>>>>>        ELFNOTE(Xen, XEN_ELFNOTE_HV_START_LOW,   _ASM_PTR
>>>>> __HYPERVISOR_VIRT_START)
>>>>
>>>> Are XEN_ELFNOTE_MOD_START_PFN and XEN_ELFNOTE_HV_START_LOW really
>>>> relevant
>>>> for the non-PV case? I don't think so (in theory
>>>> XEN_ELFNOTE_MOD_START_PFN
>>>> could be used, but the main reason for its introduction was PV
>>>> guests IIRC).
>>>
>>> I wasn't sufficiently certain for MOD_START_PFN, so I'd prefer to
>>> leave it
>>> untouched for now. HV_START_LOW might be 32-bit PV only really; I'll
>>> check
>>> and then maybe drop (or move).
>>
>> Fine with me.
> 
> HV_START_LOW is PV32 only.  It's the negotiation for the virtual address
> split with Xen, and was never implemented properly for PV64.

Not the least because there it would be the upper bound that could
be moved down, not the lower one that's movable upwards for PV32.
I suppose we'll get there once we have 5-level paging support and
can shrink the hole needed for 4-level PV guests.

> MOD_START_PFN is PV only.  It's not applicable for HVM/PVH.

It isn't right now, yes. I continue to be uncertain and would
prefer to leave it as is.

Jan

Re: [PATCH] x86/Xen: tidy xen-head.S
Posted by Andrew Cooper 1 year, 2 months ago
On 15/02/2023 1:36 pm, Jan Beulich wrote:
> On 15.02.2023 14:25, Andrew Cooper wrote:
>> MOD_START_PFN is PV only.  It's not applicable for HVM/PVH.
> It isn't right now, yes. I continue to be uncertain and would
> prefer to leave it as is.

Its the wrong address space to be applicable to HVM/PVH.

Not to mention that HVM/PVH already have normal ways of passing the
module list, so even if you fancied respecifying it to be usable in
HVM/PVH, there would be no interest in anyone using it.

~Andrew