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

[Jan Beulich <jbeulich@xxxxxxxx>]

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())?

>              parms->elf_paddr_offset = 0;

Similar remark as further up for the elf_msg() down below here.

Jan