Message ID | 20200129121235.1814563-4-anthony.perard@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | OvmfXen: Set PcdFSBClock at runtime | expand |
On 01/29/20 13:12, Anthony PERARD wrote: > We are going to use new fields from the Xen headers. Apply the EDK2 > coding style so that the code that is going to use it doesn't look out > of place. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > OvmfPkg/Include/IndustryStandard/Xen/xen.h | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) This is highly appreciated. Comments below: > > diff --git a/OvmfPkg/Include/IndustryStandard/Xen/xen.h b/OvmfPkg/Include/IndustryStandard/Xen/xen.h > index e55d93263285..ac9155089902 100644 > --- a/OvmfPkg/Include/IndustryStandard/Xen/xen.h > +++ b/OvmfPkg/Include/IndustryStandard/Xen/xen.h > @@ -183,10 +183,10 @@ struct vcpu_time_info { > * The correct way to interact with the version number is similar to > * Linux's seqlock: see the implementations of read_seqbegin/read_seqretry. > */ > - UINT32 version; > + UINT32 Version; > UINT32 pad0; > - UINT64 tsc_timestamp; /* TSC at last update of time vals. */ > - UINT64 system_time; /* Time, in nanosecs, since boot. */ > + UINT64 TSCTimestamp; /* TSC at last update of time vals. */ (1) Should be "TscTimestamp". Acronyms are de-capitalized when composed into longer identifiers, to maintain a consistent CamelCase. > + UINT64 SystemTime; /* Time, in nanosecs, since boot. */ > /* > * Current system time: > * system_time + > @@ -194,11 +194,11 @@ struct vcpu_time_info { > * CPU frequency (Hz): > * ((10^9 << 32) / tsc_to_system_mul) >> tsc_shift > */ > - UINT32 tsc_to_system_mul; > - INT8 tsc_shift; > + UINT32 TSCToSystemMultiplier; > + INT8 TSCShift; (2) Ditto (both fields). > INT8 pad1[3]; > }; /* 32 bytes */ > -typedef struct vcpu_time_info vcpu_time_info_t; > +typedef struct vcpu_time_info XEN_VCPU_TIME_INFO; > > struct vcpu_info { > /* > @@ -234,7 +234,7 @@ struct vcpu_info { > #endif /* XEN_HAVE_PV_UPCALL_MASK */ > xen_ulong_t evtchn_pending_sel; > struct arch_vcpu_info arch; > - struct vcpu_time_info time; > + struct vcpu_time_info Time; > }; /* 64 bytes (x86) */ > #ifndef __XEN__ > typedef struct vcpu_info vcpu_info_t; > @@ -250,7 +250,7 @@ typedef struct vcpu_info vcpu_info_t; > * of this structure remaining constant. > */ > struct shared_info { > - struct vcpu_info vcpu_info[XEN_LEGACY_MAX_VCPUS]; > + struct vcpu_info VcpuInfo[XEN_LEGACY_MAX_VCPUS]; Yes, this is a good example. "Vcpu" and not "VCPU" or "VCpu". > > /* > * A domain can create "event channels" on which it can send and receive > @@ -299,6 +299,7 @@ struct shared_info { > }; > #ifndef __XEN__ > typedef struct shared_info shared_info_t; > +typedef struct shared_info XEN_SHARED_INFO; > #endif > > /* Turn a plain number into a C UINTN constant. */ > Assuming the OVMF platforms continue to build at this stage into the series, and provided that (1) and (2) are fixed: Reviewed-by: Laszlo Ersek <lersek@redhat.com>
On Wed, Jan 29, 2020 at 05:14:35PM +0100, Laszlo Ersek wrote: > On 01/29/20 13:12, Anthony PERARD wrote: > Assuming the OVMF platforms continue to build at this stage into the > series, and provided that (1) and (2) are fixed: I'll fix (1) and (2). I've build tests both OvmfXen and OvmfPkgX64, and I've grep for some of those field, and the struct name, so I think ArmVirtXen will also continue to build. > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks,
diff --git a/OvmfPkg/Include/IndustryStandard/Xen/xen.h b/OvmfPkg/Include/IndustryStandard/Xen/xen.h index e55d93263285..ac9155089902 100644 --- a/OvmfPkg/Include/IndustryStandard/Xen/xen.h +++ b/OvmfPkg/Include/IndustryStandard/Xen/xen.h @@ -183,10 +183,10 @@ struct vcpu_time_info { * The correct way to interact with the version number is similar to * Linux's seqlock: see the implementations of read_seqbegin/read_seqretry. */ - UINT32 version; + UINT32 Version; UINT32 pad0; - UINT64 tsc_timestamp; /* TSC at last update of time vals. */ - UINT64 system_time; /* Time, in nanosecs, since boot. */ + UINT64 TSCTimestamp; /* TSC at last update of time vals. */ + UINT64 SystemTime; /* Time, in nanosecs, since boot. */ /* * Current system time: * system_time + @@ -194,11 +194,11 @@ struct vcpu_time_info { * CPU frequency (Hz): * ((10^9 << 32) / tsc_to_system_mul) >> tsc_shift */ - UINT32 tsc_to_system_mul; - INT8 tsc_shift; + UINT32 TSCToSystemMultiplier; + INT8 TSCShift; INT8 pad1[3]; }; /* 32 bytes */ -typedef struct vcpu_time_info vcpu_time_info_t; +typedef struct vcpu_time_info XEN_VCPU_TIME_INFO; struct vcpu_info { /* @@ -234,7 +234,7 @@ struct vcpu_info { #endif /* XEN_HAVE_PV_UPCALL_MASK */ xen_ulong_t evtchn_pending_sel; struct arch_vcpu_info arch; - struct vcpu_time_info time; + struct vcpu_time_info Time; }; /* 64 bytes (x86) */ #ifndef __XEN__ typedef struct vcpu_info vcpu_info_t; @@ -250,7 +250,7 @@ typedef struct vcpu_info vcpu_info_t; * of this structure remaining constant. */ struct shared_info { - struct vcpu_info vcpu_info[XEN_LEGACY_MAX_VCPUS]; + struct vcpu_info VcpuInfo[XEN_LEGACY_MAX_VCPUS]; /* * A domain can create "event channels" on which it can send and receive @@ -299,6 +299,7 @@ struct shared_info { }; #ifndef __XEN__ typedef struct shared_info shared_info_t; +typedef struct shared_info XEN_SHARED_INFO; #endif /* Turn a plain number into a C UINTN constant. */
We are going to use new fields from the Xen headers. Apply the EDK2 coding style so that the code that is going to use it doesn't look out of place. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- OvmfPkg/Include/IndustryStandard/Xen/xen.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)