On 01.08.2024 17:36, Roger Pau Monné wrote:
> On Thu, Aug 01, 2024 at 12:28:06PM +0200, Jan Beulich wrote:
>> On 01.08.2024 11:52, Roger Pau Monne wrote:
>>> @@ -1907,16 +1890,25 @@ void asmlinkage __init noreturn
>>> __start_xen(unsigned long mbi_p)
>>> if ( cpu_has_smep && opt_smep != SMEP_HVM_ONLY )
>>> setup_force_cpu_cap(X86_FEATURE_XEN_SMEP);
>>> if ( boot_cpu_has(X86_FEATURE_XEN_SMEP) )
>>> + {
>>> set_in_cr4(X86_CR4_SMEP);
>>> + cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
>>
>> Could be just "cr4_pv32_mask = X86_CR4_SMEP" now?
>
> Yes, indeed, same below then.
>
>>> + }
>>>
>>> if ( !opt_smap )
>>> setup_clear_cpu_cap(X86_FEATURE_SMAP);
>>> if ( cpu_has_smap && opt_smap != SMAP_HVM_ONLY )
>>> setup_force_cpu_cap(X86_FEATURE_XEN_SMAP);
>>> if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
>>> - set_in_cr4(X86_CR4_SMAP);
>>> -
>>> - cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
>>> + /*
>>> + * Set SMAP on the %cr4 mask so that it's set for APs on bringup,
>>> but
>>> + * don't set for the BSP until domain building is done.
>>> + *
>>> + * Don't set it in cr4_pv32_mask either, until it's also set on the
>>> + * BSP. Otherwise the BUG in cr4_pv32_restore would trigger for
>>> events
>>> + * received on the BSP.
>>> + */
>>> + mmu_cr4_features |= X86_CR4_SMAP;
>>
>> Don't you put APs at risk this way of triggering the BUG in
>> cr4_pv32_restore()?
>> They'll have the bit set in %cr4, but the bit remains clear in cr4_pv32_mask
>> until much later.
>
> As long as the bit is set in %cr4, but not in cr4_pv32_mask the BUG in
> cr4_pv32_restore won't hit.
Hmm, you're right. Despite staring at that for quite some time, I got
it wrong. Feel free to add
Reviewed-by: Jan Beulich <[email protected]>
then, preferably with said minor adjustment (left in context above).
Jan
