On Fri, May 21, 2021 at 07:26:21AM +0200, Juergen Gross wrote:
> On 20.05.21 11:28, Jan Beulich wrote:
> > On 20.05.2021 11:27, Roger Pau Monné wrote:
> > > On Wed, May 19, 2021 at 12:34:19PM +0200, Jan Beulich wrote:
> > > > On 18.05.2021 16:47, Roger Pau Monne wrote:
> > > > > @@ -425,8 +425,11 @@ static elf_errorstatus
> > > > > elf_xen_addr_calc_check(struct elf_binary *elf,
> > > > > return -1;
> > > > > }
> > > > > - /* Initial guess for virt_base is 0 if it is not explicitly
> > > > > defined. */
> > > > > - if ( parms->virt_base == UNSET_ADDR )
> > > > > + /*
> > > > > + * Initial guess for virt_base is 0 if it is not explicitly
> > > > > defined in the
> > > > > + * PV case. For PVH virt_base is forced to 0 because paging is
> > > > > disabled.
> > > > > + */
> > > > > + if ( parms->virt_base == UNSET_ADDR || hvm )
> > > > > {
> > > > > parms->virt_base = 0;
> > > > > elf_msg(elf, "ELF: VIRT_BASE unset, using %#" PRIx64 "\n",
> > > >
> > > > This message is wrong now if hvm is true but parms->virt_base !=
> > > > UNSET_ADDR.
> > > > Best perhaps is to avoid emitting the message altogether when hvm is
> > > > true.
> > > > (Since you'll be touching it anyway, perhaps a good opportunity
> > > > to do
> away
> > > > with passing parms->virt_base to elf_msg(), and instead simply use a
> > > > literal
> > > > zero.)
> > > >
> > > > > @@ -441,8 +444,10 @@ static elf_errorstatus
> > > > > elf_xen_addr_calc_check(struct elf_binary *elf,
> > > > > *
> > > > > * If we are using the modern ELF notes interface then the
> > > > > default
> > > > > * is 0.
> > > > > + *
> > > > > + * For PVH this is forced to 0, as it's already a
> > > > > legacy option
> for PV.
> > > > > */
> > > > > - if ( parms->elf_paddr_offset == UNSET_ADDR )
> > > > > + if ( parms->elf_paddr_offset == UNSET_ADDR || hvm )
> > > > > {
> > > > > if ( parms->elf_note_start )
> > > >
> > > > Don't you want "|| hvm" here as well, or alternatively suppress the
> > > > fallback to the __xen_guest section in the PVH case (near the end of
> > > > elf_xen_parse())?
> > >
> > > The legacy __xen_guest section doesn't support PHYS32_ENTRY, so yes,
> > > that part could be completely skipped when called from an HVM
> > > container.
> > >
> > > I think I will fix that in another patch though if you are OK, as
> > > it's not strictly related to the calculation fixes done here.
> >
> > That's fine; it wants to be a prereq to the one here then, though,
> > I think.
>
> Would it be possible to add some comment to xen/include/public/elfnote.h
> Indicating which elfnotes are evaluated for which guest types,
For PVH after this patch the only mandatory note should be
PHYS32_ENTRY. BSD_SYMTAB is also parsed if found on that case.
> including
> a hint which elfnotes _have_ been evaluated before this series?
Before this patch mostly all PV notes are parsed, and you have to
manage to get suitable values set so that:
if ( (parms->virt_kstart > parms->virt_kend) ||
(parms->virt_entry < parms->virt_kstart) ||
(parms->virt_entry > parms->virt_kend) ||
(parms->virt_base > parms->virt_kstart) )
{
elf_err(elf, "ERROR: ELF start or entries are out of bounds\n");
return -1;
}
Doesn't trigger. That can be done by setting virt_entry or the native
elf entry point to a value that satisfies the condition. Or by setting
a suitable offset in VIRT_BASE to a value that matches the offset
added to the native entry point in the ELF header.
I can try to write this up, albeit I think it can get messy. Not sure
what's the best approach here regarding recommended settings to
satisfy old Xen versions.
Roger.
