Re: [Xen-devel] [PATCH 4/3] x86/svm: Drop svm_segment_register_t

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/07/17 14:37, Jan Beulich wrote:
>>>> On 03.07.17 at 15:10, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -310,6 +310,15 @@ void __init setup_vmcb_dump(void)
>>      register_keyhandler('v', vmcb_dump, "dump AMD-V VMCBs", 1);
>>  }
>>  
>> +static void __init __maybe_unused build_assertions(void)
>> +{
>> +    /* Check struct segment_register against the VMCB segment layout. */
>> +    BUILD_BUG_ON(sizeof(struct segment_register) != 16);
>> +    BUILD_BUG_ON(offsetof(struct segment_register, attr) != 2);
>> +    BUILD_BUG_ON(offsetof(struct segment_register, limit) != 4);
>> +    BUILD_BUG_ON(offsetof(struct segment_register, base) != 8);
>> +}
> As said in reply to patch 1, I think we want to check both position
> and size here. With respective sizeof() checks added (and both
> for the so far missing sel field)
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Lets merge that part of the thread in here:

I don't see what the individual sizeof() checks gains us.  The only
field where a sizeof check would be useful is the attr union, but
offsetof(following field) is the only way to infer the size, as the
union is anonymous.

For the other fields, we'd be checking sizeof(uint{16,32,64}_t) being
correct, which is overkill.

~Andrew

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