On 22.03.2024 15:24, Jason Andryuk wrote: > On 2024-03-21 09:45, Jason Andryuk wrote: >> On 2024-03-20 10:39, Jan Beulich wrote: >>> On 19.03.2024 21:58, Jason Andryuk wrote: > >>>> @@ -536,6 +565,10 @@ elf_errorstatus elf_xen_parse(struct elf_binary >>>> *elf, >>>> parms->p2m_base = UNSET_ADDR; >>>> parms->elf_paddr_offset = UNSET_ADDR; >>>> parms->phys_entry = UNSET_ADDR32; >>>> + parms->phys_min = 0; >>>> + parms->phys_max = 0xffffffff; >>>> + parms->phys_align = 0x200000; >>> >>> I think this wants to be MB(2) (requiring a pre-patch to centralize MB() >>> in the tool stack to tools/include/xen-tools/common-macros.h). And I >>> further think this needs to be an arch-specific constant, even if right >>> now the note is expected to be present only for x86. Which then also >>> needs saying ... > > Are you thinking something like the following in libelf-dominfo.c: > > #define X86_PHYS_ALIGN_DEFAULT MB(2) > #define X86_PHYS_MAX_DEFAULT (GB(4) - 1) > > and setting as: > parms->phys_max = X86_PHYS_MAX_DEFAULT; > parms->phys_align = X86_PHYS_ALIGN_DEFAULT; > > libelf is arch neutral, so there isn't a natural place to introduce > arch-specific defines. Or were you looking for each arch to set it? We > only care about x86 right now, so we can do something like: > > #if x86 > #define ARCH_PHYS_MAX_DEFAULT (GB(4) - 1) > #define ARCH_PHYS_ALIGN_DEFAULT MB(2) > #else > #define ARCH_PHYS_MAX_DEFAULT 0 > #define ARCH_PHYS_ALIGN_DEFAULT 0 > #endif
More like the latter. The former only if the phys_* fields themselves were to also become x86-only. As you say, libelf in its present shape doesn't easily lend itself to such arch-specifics. Jan
