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
