Re: [Xen-devel] [PATCH 1/3] x86/emul: Introduce build time assertions for struct segment_register

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.07.17 at 15:05, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 03/07/17 13:29, Jan Beulich wrote:
>>>>> On 30.06.17 at 17:04, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> This structure is shared with hardware in the AMD VMCB.
>> Indeed, but do we really depend on that in the emulator code?
>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -7899,6 +7899,12 @@ static void __init __maybe_unused 
>>> build_assertions(void)
>>>      BUILD_BUG_ON(X86_EVENTTYPE_SW_INTERRUPT != 4);
>>>      BUILD_BUG_ON(X86_EVENTTYPE_PRI_SW_EXCEPTION != 5);
>>>      BUILD_BUG_ON(X86_EVENTTYPE_SW_EXCEPTION != 6);
>>> +
>>> +    /* 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);
>> I.e. for these two I don't think I can see any dependency, and ...
>>
>>> +    BUILD_BUG_ON(offsetof(struct segment_register, limit) != 4);
>>> +    BUILD_BUG_ON(offsetof(struct segment_register, base) != 8);
>> ... for these two I think all we require is >=. Otoh, if these were put
>> in SVM code, then I could see the point of them being the way they
>> are.
> 
> These are indeed only for the SVM code.  See the impending 4/3 patch
> which I'm about to post.

Ah, I see. But then I'm still unconvinced we need them here as
well. I.e. I'd rather see that new patch to replace the one here.

>>  I'd then even raise the question whether we wouldn't also want
>> offsetof() checks.
> 
> I don't understand; these are offsetof checks.

Oops, I must have inferred sizeof() from the first one, and with
the literal numbers being applicable to both this didn't cause any
inconsistency. In any event - I think we should have both sizeof()
and offsetof() ones.

Jan


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