Re: [EXTERNAL] [PATCH v7 8/9] x86/time: add a domain context record for tsc_info...

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.08.2020 18:36, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 28 August 2020 16:53
>> To: paul@xxxxxxx
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Ian Jackson' 
>> <ian.jackson@xxxxxxxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>;
>> 'Andrew Cooper' <andrew.cooper3@xxxxxxxxxx>; 'George Dunlap' 
>> <george.dunlap@xxxxxxxxxx>; 'Julien
>> Grall' <julien@xxxxxxx>; 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>; 
>> 'Roger Pau Monné'
>> <roger.pau@xxxxxxxxxx>
>> Subject: Re: [EXTERNAL] [PATCH v7 8/9] x86/time: add a domain context record 
>> for tsc_info...
>>
>> On 28.08.2020 13:08, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>> Sent: 26 August 2020 15:03
>>>> To: Paul Durrant <paul@xxxxxxx>
>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Durrant, Paul <pdurrant@xxxxxxxxxxxx>; 
>>>> Ian Jackson
>>>> <ian.jackson@xxxxxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Andrew Cooper 
>>>> <andrew.cooper3@xxxxxxxxxx>;
>> George
>>>> Dunlap <george.dunlap@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano 
>>>> Stabellini
>>>> <sstabellini@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> Subject: RE: [EXTERNAL] [PATCH v7 8/9] x86/time: add a domain context 
>>>> record for tsc_info...
>>>>
>>>> CAUTION: This email originated from outside of the organization. Do not 
>>>> click links or open
>>>> attachments unless you can confirm the sender and know the content is safe.
>>>>
>>>>
>>>>
>>>> On 18.08.2020 12:30, Paul Durrant wrote:
>>>>> --- a/xen/include/public/save.h
>>>>> +++ b/xen/include/public/save.h
>>>>> @@ -93,7 +93,18 @@ struct domain_shared_info_context {
>>>>>
>>>>>  DECLARE_DOMAIN_SAVE_TYPE(SHARED_INFO, 2, struct 
>>>>> domain_shared_info_context);
>>>>>
>>>>> -#define DOMAIN_SAVE_CODE_MAX 2
>>>>> +#if defined(__i386__) || defined(__x86_64__)
>>>>> +struct domain_tsc_info_context {
>>>>> +    uint32_t mode;
>>>>> +    uint32_t incarnation;
>>>>> +    uint64_t elapsed_nsec;
>>>>> +    uint32_t khz;
>>>>> +};
>>>>
>>>> sizeof() for this struct varies between 32-bit and 64-bit - is
>>>> this not a problem? (alignof() varies too, but there I think
>>>> it's indeed not a problem, albeit it could still be taken care
>>>> of by using uint64_aligned_t, alongside the addition of an
>>>> explicit padding field).
>>>
>>> I don't think it should matter because domain context records have
>>> implicit padding to align up to the next 64-bit boundary,
>>
>> Could you remind me where this is written down and enforced?
>>
> 
> With the series fully applied, see xen/include/public/save.h
> line 62-68 for the comment and then see domain_save_end() in
> xen/common/save.c for where the padding is applied.

Ah, yes, this helped find the places in the patches. Therefore with
the stray blank line addition removed from tools/misc/xen-domctx.c
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan