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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/08/2024 10:49 am, Jan Beulich wrote:
> On 27.08.2024 14:39, Roger Pau Monne wrote:
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -612,7 +612,24 @@ int __init construct_dom0(struct domain *d, const 
>> module_t *image,
>>      if ( is_hvm_domain(d) )
>>          rc = dom0_construct_pvh(d, image, image_headroom, initrd, cmdline);
>>      else if ( is_pv_domain(d) )
>> +    {
>> +        /*
>> +         * Temporarily clear SMAP in CR4 to allow user-accesses in
>> +         * construct_dom0().  This saves a large number of corner cases
>> +         * 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_pv(d, image, image_headroom, initrd, cmdline);
>> +        if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
>> +        {
>> +            write_cr4(read_cr4() | X86_CR4_SMAP);
>> +            cr4_pv32_mask |= X86_CR4_SMAP;
>> +        }
>> +    }
> Andrew, looking at this code I really wonder what benefit you request to
> move this further is going to have. Afaict no matter where we put it, it'll
> be an #ifdef around each of the two accesses to cr4_pv32_mask when we make
> that variable CONFIG_PV32-only.

This TU is common to PV and HVM, meaning that this logic inspecting
XEN_SMAP and playing with cr4 is included even in !PV builds.

Having it all in x86/pv/dom0_build.c means it will be dropped in a !PV
build.

~Andrew