| Message ID | 20170217063936.13208-9-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > @@ -804,7 +800,7 @@ static void mcinfo_clear(struct mc_info *mi) > x86_mcinfo_nentries(mi) = 0; > } > > -void *x86_mcinfo_reserve(struct mc_info *mi, int size) > +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type) There's no need for fixed width types here afaics. With them replaced by "unsigned int" Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 02/17/17 03:07 -0700, Jan Beulich wrote: > >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > > @@ -804,7 +800,7 @@ static void mcinfo_clear(struct mc_info *mi) > > x86_mcinfo_nentries(mi) = 0; > > } > > > > -void *x86_mcinfo_reserve(struct mc_info *mi, int size) > > +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type) > > There's no need for fixed width types here afaics. With them > replaced by "unsigned int" > Reviewed-by: Jan Beulich <jbeulich@suse.com> > The reason I use uint16_t is because the type of mcinfo_common.type and .size is uint16_t. Haozhong
>>> On 20.02.17 at 03:48, <haozhong.zhang@intel.com> wrote: > On 02/17/17 03:07 -0700, Jan Beulich wrote: >> >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: >> > @@ -804,7 +800,7 @@ static void mcinfo_clear(struct mc_info *mi) >> > x86_mcinfo_nentries(mi) = 0; >> > } >> > >> > -void *x86_mcinfo_reserve(struct mc_info *mi, int size) >> > +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type) >> >> There's no need for fixed width types here afaics. With them >> replaced by "unsigned int" >> Reviewed-by: Jan Beulich <jbeulich@suse.com> > > The reason I use uint16_t is because the type of mcinfo_common.type > and .size is uint16_t. I guessed that, but the parameters have no need to be fixed width just because they get stored into some fixed width field. Please remember that dealing with plain (unsigned) int is slightly more efficient than dealing with uint16_t (or uint64_t, btw). Jan
On 02/20/17 02:02 -0700, Jan Beulich wrote: > >>> On 20.02.17 at 03:48, <haozhong.zhang@intel.com> wrote: > > On 02/17/17 03:07 -0700, Jan Beulich wrote: > >> >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > >> > @@ -804,7 +800,7 @@ static void mcinfo_clear(struct mc_info *mi) > >> > x86_mcinfo_nentries(mi) = 0; > >> > } > >> > > >> > -void *x86_mcinfo_reserve(struct mc_info *mi, int size) > >> > +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type) > >> > >> There's no need for fixed width types here afaics. With them > >> replaced by "unsigned int" > >> Reviewed-by: Jan Beulich <jbeulich@suse.com> > > > > The reason I use uint16_t is because the type of mcinfo_common.type > > and .size is uint16_t. > > I guessed that, but the parameters have no need to be fixed > width just because they get stored into some fixed width field. > Please remember that dealing with plain (unsigned) int is slightly > more efficient than dealing with uint16_t (or uint64_t, btw). > OK, I'll change to the unsigned type. Thanks, Haozhong
diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c b/xen/arch/x86/cpu/mcheck/mcaction.c index 9cf2499..cc90e7c 100644 --- a/xen/arch/x86/cpu/mcheck/mcaction.c +++ b/xen/arch/x86/cpu/mcheck/mcaction.c @@ -13,14 +13,12 @@ mci_action_add_pageoffline(int bank, struct mc_info *mi, if (!mi) return NULL; - rec = x86_mcinfo_reserve(mi, sizeof(*rec)); + rec = x86_mcinfo_reserve(mi, sizeof(*rec), MC_TYPE_RECOVERY); if (!rec) { mi->flags |= MCINFO_FLAGS_UNCOMPLETE; return NULL; } - rec->common.type = MC_TYPE_RECOVERY; - rec->common.size = sizeof(*rec); rec->mc_bank = bank; rec->action_types = MC_ACTION_PAGE_OFFLINE; rec->action_info.page_retire.mfn = mfn; diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c index f682520..28bf579 100644 --- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -204,7 +204,7 @@ static void mca_init_bank(enum mca_source who, if (!mi) return; - mib = x86_mcinfo_reserve(mi, sizeof(*mib)); + mib = x86_mcinfo_reserve(mi, sizeof(*mib), MC_TYPE_BANK); if (!mib) { mi->flags |= MCINFO_FLAGS_UNCOMPLETE; @@ -213,8 +213,6 @@ static void mca_init_bank(enum mca_source who, mib->mc_status = mca_rdmsr(MSR_IA32_MCx_STATUS(bank)); - mib->common.type = MC_TYPE_BANK; - mib->common.size = sizeof (struct mcinfo_bank); mib->mc_bank = bank; if (mib->mc_status & MCi_STATUS_MISCV) @@ -250,8 +248,6 @@ static int mca_init_global(uint32_t flags, struct mcinfo_global *mig) struct domain *d; /* Set global information */ - mig->common.type = MC_TYPE_GLOBAL; - mig->common.size = sizeof (struct mcinfo_global); status = mca_rdmsr(MSR_IA32_MCG_STATUS); mig->mc_gstatus = status; mig->mc_domid = mig->mc_vcpuid = -1; @@ -351,7 +347,7 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks *bankmask, if ( (mctc = mctelem_reserve(which)) != NULL ) { mci = mctelem_dataptr(mctc); mcinfo_clear(mci); - mig = x86_mcinfo_reserve(mci, sizeof(*mig)); + mig = x86_mcinfo_reserve(mci, sizeof(*mig), MC_TYPE_GLOBAL); /* mc_info should at least hold up the global information */ ASSERT(mig); mca_init_global(mc_flags, mig); @@ -804,7 +800,7 @@ static void mcinfo_clear(struct mc_info *mi) x86_mcinfo_nentries(mi) = 0; } -void *x86_mcinfo_reserve(struct mc_info *mi, int size) +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type) { int i; unsigned long end1, end2; @@ -831,7 +827,11 @@ void *x86_mcinfo_reserve(struct mc_info *mi, int size) /* there's enough space. add entry. */ x86_mcinfo_nentries(mi)++; - return memset(mic_index, 0, size); + memset(mic_index, 0, size); + mic_index->size = size; + mic_index->type = type; + + return mic_index; } static void x86_mcinfo_apei_save( diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h index 56877c1..2f4e7a4 100644 --- a/xen/arch/x86/cpu/mcheck/mce.h +++ b/xen/arch/x86/cpu/mcheck/mce.h @@ -146,7 +146,7 @@ typedef struct mcinfo_extended *(*x86_mce_callback_t) (struct mc_info *, uint16_t, uint64_t); extern void x86_mce_callback_register(x86_mce_callback_t); -void *x86_mcinfo_reserve(struct mc_info *mi, int size); +void *x86_mcinfo_reserve(struct mc_info *mi, uint16_t size, uint16_t type); void x86_mcinfo_dump(struct mc_info *mi); static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr) diff --git a/xen/arch/x86/cpu/mcheck/mce_amd.c b/xen/arch/x86/cpu/mcheck/mce_amd.c index 599e465..fe51be9 100644 --- a/xen/arch/x86/cpu/mcheck/mce_amd.c +++ b/xen/arch/x86/cpu/mcheck/mce_amd.c @@ -218,15 +218,13 @@ amd_f10_handler(struct mc_info *mi, uint16_t bank, uint64_t status) if ( !(status & MCi_STATUS_MISCV) ) return NULL; - mc_ext = x86_mcinfo_reserve(mi, sizeof(*mc_ext)); + mc_ext = x86_mcinfo_reserve(mi, sizeof(*mc_ext), MC_TYPE_EXTENDED); if ( !mc_ext ) { mi->flags |= MCINFO_FLAGS_UNCOMPLETE; return NULL; } - mc_ext->common.type = MC_TYPE_EXTENDED; - mc_ext->common.size = sizeof(*mc_ext); mc_ext->mc_msrs = 3; mc_ext->mc_msr[0].reg = MSR_F10_MC4_MISC1; diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c index b5ee8b8..9e5ee3d 100644 --- a/xen/arch/x86/cpu/mcheck/mce_intel.c +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c @@ -200,17 +200,13 @@ intel_get_extended_msrs(struct mcinfo_global *mig, struct mc_info *mi) !(mig->mc_gstatus & MCG_STATUS_EIPV)) return NULL; - mc_ext = x86_mcinfo_reserve(mi, sizeof(*mc_ext)); + mc_ext = x86_mcinfo_reserve(mi, sizeof(*mc_ext), MC_TYPE_EXTENDED); if (!mc_ext) { mi->flags |= MCINFO_FLAGS_UNCOMPLETE; return NULL; } - /* this function will called when CAP(9).MCG_EXT_P = 1 */ - mc_ext->common.type = MC_TYPE_EXTENDED; - mc_ext->common.size = sizeof(struct mcinfo_extended); - for (i = MSR_IA32_MCG_EAX; i <= MSR_IA32_MCG_R15; i++) intel_get_extended_msr(mc_ext, i);
All existing calls to x86_mcinfo_reserve() are followed by statements that set the size and the type of the reserved space, so move them into x86_mcinfo_reserve() to simplify the code. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: Christoph Egger <chegger@amazon.de> Cc: Liu Jinsong <jinsong.liu@alibaba-inc.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/cpu/mcheck/mcaction.c | 4 +--- xen/arch/x86/cpu/mcheck/mce.c | 16 ++++++++-------- xen/arch/x86/cpu/mcheck/mce.h | 2 +- xen/arch/x86/cpu/mcheck/mce_amd.c | 4 +--- xen/arch/x86/cpu/mcheck/mce_intel.c | 6 +----- 5 files changed, 12 insertions(+), 20 deletions(-)