>>> On 12.09.16 at 14:29, <andrew.coop...@citrix.com> wrote:
> On 12/09/16 12:41, Jan Beulich wrote:
>>>>> On 12.09.16 at 11:51, <andrew.coop...@citrix.com> wrote:
>>> @@ -205,11 +222,9 @@ void expand_xsave_states(struct vcpu *v, void *dest,
>>> unsigned int size)
>>>
>>> if ( src )
>>> {
>>> - ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
>>> + BUG_ON((xstate_offsets[index] + xstate_sizes[index]) <= size);
>>> memcpy(dest + xstate_offsets[index], src, xstate_sizes[index]);
>>> }
>>> - else
>>> - memset(dest + xstate_offsets[index], 0, xstate_sizes[index]);
>> So I have difficulty seeing why this memset() wasn't sufficient: It
>> precisely covers for the respective component being in default
>> state.
>
> No it doesn't. The loop skips over all bits which are not set in xstate_bv.
Well, yes, I had corrected myself in the following sentence, resulting
in me just asking for the commit message to get clarified.
> I had (erroneously) come to the conclusion that the "if ( src )" check
> only caught the case where we had bad comp_offsets[] information, but
> rereading the logic, that case would actually corrupt the legacy SSE header.
>
> Overall, it turns out that the "if ( src )" is unconditionally taken.
Oh, I see (same applies to my then wrong comment on patch 6):
We iterate over xstate_bv here, and components with their flag
set in xstate_bv won't see NULL coming back from get_xsave_addr().
I'm sorry for the noise then.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
