Re: [PATCH] x86/pv: Improve dom0_update_physmap() with CONFIG_SPECULATIVE_HARDEN_BRANCH

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.04.2021 13:46, Andrew Cooper wrote:
> dom0_update_physmap() is mostly called in two tight loops, where the lfences
> hidden in is_pv_32bit_domain() have a substantial impact.
> 
> None of the boot time construction needs protection against malicious
> speculation, so use a local variable and calculate is_pv_32bit_domain() just
> once.
> 
> Reformat the some of the code for legibility, now that the volume has reduced,
> and removal of some gratuitous negations.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one mior remark:

> @@ -463,9 +465,9 @@ int __init dom0_construct_pv(struct domain *d,
>          vinitrd_end    = vinitrd_start + initrd_len;
>          vphysmap_start = round_pgup(vinitrd_end);
>      }
> -    vphysmap_end     = vphysmap_start + (nr_pages * (!is_pv_32bit_domain(d) ?
> -                                                     sizeof(unsigned long) :
> -                                                     sizeof(unsigned int)));
> +    vphysmap_end = vphysmap_start +
> +        (nr_pages * (compat ? sizeof(unsigned int) : sizeof(unsigned long)));
> +
>      if ( parms.p2m_base != UNSET_ADDR )
>          vphysmap_end = vphysmap_start;
>      vstartinfo_start = round_pgup(vphysmap_end);

I think a blank line as separator would make more sense ahead of
and/or after the full setting of vphysmap_end, but not so much
between the two parts of it.

Jan