Re: [Xen-devel] [PATCH L1TF v10 4/8] is_hvm/pv_domain: block speculation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.19 at 20:29, <nmanthey@xxxxxxxxx> wrote:
> On 4/5/19 17:34, Andrew Cooper wrote:
>> On 14/03/2019 12:50, Norbert Manthey wrote:
>>> When checking for being an hvm domain, or PV domain, we have to make
>>> sure that speculation cannot bypass that check, and eventually access
>>> data that should not end up in cache for the current domain type.
>>>
>>> This is part of the speculative hardening effort.
>>>
>>> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> ---
>>>  xen/include/xen/sched.h | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -922,7 +922,8 @@ void watchdog_domain_destroy(struct domain *d);
>>>  
>>>  static inline bool is_pv_domain(const struct domain *d)
>>>  {
>>> -    return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
>>> +    return IS_ENABLED(CONFIG_PV)
>>> +           ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
>>>  }
>>>  
>>>  static inline bool is_pv_vcpu(const struct vcpu *v)
>>> @@ -953,7 +954,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu 
> *v)
>>>  #endif
>>>  static inline bool is_hvm_domain(const struct domain *d)
>>>  {
>>> -    return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : 
>>> false;
>>> +    return IS_ENABLED(CONFIG_HVM)
>>> +           ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
>>>  }
>> Unfortunately, this breaks the livepatch build, in a rather insidious
>> way.  The asm inside evaluate_nospec() forces these out-of-line.
>>
>> Duplicate symbol 'domctl.c#is_pv_domain.isra.0' (ffff82d080270390 !=
>> ffff82d08036a005)
>>
>> On forcing these to be always_inline, the next bit of breakage occurs at:
>>
>> Duplicate symbol 'domctl.c#is_pv_32bit_domain' (ffff82d080270270 !=
>> ffff82d08036a000)
>>
>> which means we now have a goose chase of throwing always_inline through
>> the code.
>>
>> I'm going to have to revert this in XenServer, especially seeing as
>> OSSTest is currently out of action, which AFAICT, makes XenServer's
>> testing the only automatic testing which staging is getting.
>>
>> I also need to see about upstreaming out local patch which causes the
>> build to properly fail in cases which we know will break the ability to
>> build livepatches.
> 
> I see, thanks for the pointer. I guess the build only fails in case you
> patch some function that needs one of the two? In case you have
> something that reproduces the problem, and can be shared, we might have
> a look. Thanks!

I too did notice the issue while committing your patches, so there's
no special setup needed (might be compiler version dependent,
though). I merely failed to make the connection to live-patching, or
else I would have refrained from pushing the changes.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel