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.
~Andrew
