Re: [Xen-devel] [PATCH v2] x86: generic MSRs save/restore

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.12.13 at 14:23, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Jan Beulich wrote:
>>>>> On 16.12.13 at 11:05, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>>> Jan Beulich wrote:
>>>>>>> On 16.12.13 at 10:53, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>>> wrote: 
>>>>> @@ -467,6 +487,7 @@ int main(int argc, char **argv)
>>>>>          case HVM_SAVE_CODE(VIRIDIAN_VCPU): dump_viridian_vcpu();
>>>>>          break; case HVM_SAVE_CODE(VMCE_VCPU): dump_vmce_vcpu();
>>>>>          break; case HVM_SAVE_CODE(TSC_ADJUST): dump_tsc_adjust();
>>>>> break; +        case HVM_SAVE_CODE(CPU_MSR): dump_cpu_msr(); break;
>>>>>          case HVM_SAVE_CODE(END): break;
>>>>>          default:
>>>>>              printf(" ** Don't understand type %u: skipping\n",
>>>>> ======================= 
>>>>> 
>>>>> w/o the declaration, case HVM_SAVE_CODE(CPU_MSR) compiling fail.
>>>> 
>>>> Right, because you ought to use CPU_MSR_CODE here, which the
>>>> public header does define.
>>> 
>>> Isn't it ugly? not unified style w/ other typecodes.
>>> Why not simply add declaration at Xen side?
>> 
>> We absolutely should not declare things that aren't correct.
> 
> You have had the declaration in the public header with variable size data 
> structure:
> struct hvm_msr {
>     uint32_t count;
>     struct hvm_one_msr {
>         uint32_t index;
>         uint32_t _rsvd;
>         uint64_t val;
>     } msr[1 /* variable size */];
> };
> Why can't declare DECLARE_HVM_SAVE_TYPE(CPU_MSR, CPU_MSR_CODE, struct 
> hvm_msr)? They are both variable size.

That's not the point. We should make it impossible for someone to
erroneously use in particular HVM_SAVE_LENGTH() for variable
size records.

Anyway - why don't you simply bring this up with your colleagues
who introduced the XSAVE record? I'm really just following its
model...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel