On 03/01/2020 15:30, Jan Beulich wrote:
> On 03.01.2020 15:55, Andrew Cooper wrote:
>> On 03/01/2020 14:49, Jan Beulich wrote:
>>> On 24.12.2019 16:19, Andrew Cooper wrote:
>>>> @@ -439,6 +449,34 @@ def verify_record_static_data_end(self, content):
>>>>              raise RecordError("Static data end record found in v2 stream")
>>>>  
>>>>  
>>>> +    def verify_record_x86_cpuid_policy(self, content):
>>>> +        """ x86 CPUID policy record """
>>>> +
>>>> +        if self.version < 3:
>>>> +            raise RecordError("x86 CPUID policy record found in v2 
>>>> stream")
>>>> +
>>>> +        sz = calcsize(X86_CPUID_POLICY_FORMAT)
>>>> +        contentsz = len(content)
>>>> +
>>>> +        if contentsz < sz or (contentsz % sz) != 0:
>>>> +            raise RecordError("Record length %u, expected multiple of %u" 
>>>> %
>>>> +                              (contentsz, sz))
>>>> +
>>>> +
>>>> +    def verify_record_x86_msr_policy(self, content):
>>>> +        """ x86 MSR policy record """
>>>> +
>>>> +        if self.version < 3:
>>>> +            raise RecordError("x86 MSR policy record found in v2 stream")
>>>> +
>>>> +        sz = calcsize(X86_MSR_POLICY_FORMAT)
>>>> +        contentsz = len(content)
>>>> +
>>>> +        if contentsz < sz or (contentsz % sz) != 0:
>>>> +            raise RecordError("Record length %u, expected multiple of %u" 
>>>> %
>>>> +                              (contentsz, sz))
>>> While I can't even see a theoretical case of the CPUID array
>>> having zero elements, is it really entirely implausible to have
>>> an empty MSRs array? I.e. wouldn't the left side of the "or"
>>> better go away?
>> MSRs will never have 0 entries, because unlike CPUID, we can't omit
>> records with 0s as their content.  This becomes ambiguous when the
>> policy default is nonzero.
> Isn't the same true for CPUID, in particular some of the non-boolean
> fields?

I perhaps misspoke.  We can omit CPUID leave(s) based on information
already sent (max_leaf and/or the absence of an enumerating feature). 
In these cases, the destination side can still fill the remainder in
with 0's.

Top level leaves without an encompassing feature do need sending in full.

The best practical example is for not sending all 63 subleaves of the
xsave state leaf, when most of them are outside of the policy XCR0|XSS bits.

>
>> When we do gain more MSRs, I will see about organising elision based on
>> CPUID features, so we don't have to send a 0 for every single MSR in the
>> policy, but MSRs without CPUID enumeration must always be sent.
>>
>> This means that the one MSR we have currently (MSR_INTEL_PLATFORM_INFO
>> for CPUID Faulting, which we also virtualise on AMD hardware) shall
>> unconditionally be present forever more.
> Hmm, yes. Still the special casing of there needing to be at least
> one entry looks a little odd here (and also for CPUID). I would
> find it more logical if there was just the remainder-must-be-zero
> check. But this is libxc code, so I'm not the one to really judge
> anyway.

The migration stream is split into records with no playload (markers
with external control flow meaning), and data records, which have a payload.

It is an error for a data record to have no payload, because it means
there is a source side generation bug.  In the case of Xen returning 0
MSRs, the record would be omitted entirely, rather than be sent with 0
MSRs worth of data.

~Andrew

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

Reply via email to