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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.08.2024 15:51, Roger Pau Monné wrote:
> On Tue, Aug 27, 2024 at 02:07:07PM +0100, Andrew Cooper wrote:
>> On 27/08/2024 2:04 pm, Jan Beulich wrote:
>>> On 27.08.2024 14:59, Andrew Cooper wrote:
>>>> On 27/08/2024 1:39 pm, 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;
>>>>> +        }
>>>>> +    }
>>>> I hate to drag this on further still, but can this logic be move it into
>>>> dom0_construct_pv() itself, rather than here?
>>> Just to mention it: I'm fine with this in principle, as long as this won't
>>> mean a pile of new goto-s in dom0_construct_pv(). If a new wrapper was
>>> introduced (with the present function becoming static), I'd be okay.
>>
>> I'd be happy with that too.
>>
>> In fact, static helpers are probably best, seeing as we'll eventually
>> need real #ifdefary around the cr4_pv32_mask accesses.
> 
> Do you mean a static helper in dom0_build.c for enabling/disabling
> SMAP?

While that's how I understood Andrew's response, I'm not sure that'll buy
us very much. They'll both be used just once, and hence the amount of
#ifdef-ary is going to be the same as when the logic is kept in line.

Jan