Re: [PATCH v2] libelf: improve PVH elfnote parsing

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.