Re: [PATCH v6] x86/dom0: disable SMAP for PV domain building only

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

On Wed, Aug 28, 2024 at 12:51:23PM +0100, Andrew Cooper wrote:
> On 28/08/2024 12:50 pm, Jan Beulich wrote:
> > On 28.08.2024 13:30, Roger Pau Monne wrote:
> >> Move the logic that disables SMAP so it's only performed when building a PV
> >> dom0, PVH dom0 builder doesn't require disabling SMAP.
> >>
> >> The fixes tag is to account for the wrong usage of cpu_has_smap in
> >> create_dom0(), it should instead have used
> >> boot_cpu_has(X86_FEATURE_XEN_SMAP).  Fix while moving the logic to apply 
> >> to PV
> >> only.
> >>
> >> While there also make cr4_pv32_mask __ro_after_init.
> >>
> >> Fixes: 493ab190e5b1 ('xen/sm{e, a}p: allow disabling sm{e, a}p for Xen 
> >> itself')
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > preferably with ...
> >
> >> @@ -1051,6 +1051,34 @@ out:
> >>      return rc;
> >>  }
> >>  
> >> +int __init dom0_construct_pv(struct domain *d,
> >> +                             const module_t *image,
> >> +                             unsigned long image_headroom,
> >> +                             module_t *initrd,
> >> +                             const char *cmdline)
> >> +{
> >> +    int rc;
> >> +
> >> +    /*
> >> +     * Temporarily clear SMAP in CR4 to allow user-accesses in
> >> +     * construct_dom0().  This saves a large number of corner cases
> > ... the final 's' dropped here and ...
> >
> >> +     * interactions with copy_from_user().
> >> +     */
> >> +    if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> >> +    {
> >> +        cr4_pv32_mask &= ~X86_CR4_SMAP;
> >> +        write_cr4(read_cr4() & ~X86_CR4_SMAP);
> >> +    }
> >> +    rc = dom0_construct(d, image, image_headroom, initrd, cmdline);
> >> +    if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> > ... blank lines added around the function call. Happy to adjust while
> > committing, so long as you agree.
> 
> +1 to both suggestions.

Sure, please adjust at commit.

> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks (to both).