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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----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.

  Paul

> > so as long as fields within the struct don't move (which I think
> > is true in this case) then we should be ok.
> 
> Right.
> 
> Jan