| Message ID | 1471390109-10407-2-git-send-email-cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.08.16 at 01:28, <cardoe@cardoe.com> wrote: > For the functions that make up the interface to the MTRR code, drop the > static keyword and prefix them all with mtrr for improved clarity when > they're called outside this file. This also required adjusting or > providing function prototypes to make them callable. I can see why you want to do this for non-static functions, but I can't see why static ones would need to get altered (unless you mean to call them directly in subsequent patches, in which case the rationale above should be adjusted). Nor can I see why the two functions previously having been non-static can't simply be made static, without changing their names, as the patch demonstrates that they don't have callers in other CUs. Jan
On 8/17/16 7:36 AM, Jan Beulich wrote: >>>> On 17.08.16 at 01:28, <cardoe@cardoe.com> wrote: >> For the functions that make up the interface to the MTRR code, drop the >> static keyword and prefix them all with mtrr for improved clarity when >> they're called outside this file. This also required adjusting or >> providing function prototypes to make them callable. > > I can see why you want to do this for non-static functions, but I can't > see why static ones would need to get altered (unless you mean to call > them directly in subsequent patches, in which case the rationale above > should be adjusted). Nor can I see why the two functions previously > having been non-static can't simply be made static, without changing > their names, as the patch demonstrates that they don't have callers > in other CUs. > > Jan > I've added: Future changes will directly call these functions instead of using the indirect call through the ops structure. Does that work?
>>> On 18.08.16 at 03:38, <cardoe@cardoe.com> wrote: > On 8/17/16 7:36 AM, Jan Beulich wrote: >>>>> On 17.08.16 at 01:28, <cardoe@cardoe.com> wrote: >>> For the functions that make up the interface to the MTRR code, drop the >>> static keyword and prefix them all with mtrr for improved clarity when >>> they're called outside this file. This also required adjusting or >>> providing function prototypes to make them callable. >> >> I can see why you want to do this for non-static functions, but I can't >> see why static ones would need to get altered (unless you mean to call >> them directly in subsequent patches, in which case the rationale above >> should be adjusted). Nor can I see why the two functions previously >> having been non-static can't simply be made static, without changing >> their names, as the patch demonstrates that they don't have callers >> in other CUs. > > I've added: > > Future changes will directly call these functions instead of using the > indirect call through the ops structure. > > Does that work? Sure, having seen the further parts of the series. Jan
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c index 234d2ba..224d231 100644 --- a/xen/arch/x86/cpu/mtrr/generic.c +++ b/xen/arch/x86/cpu/mtrr/generic.c @@ -250,7 +250,7 @@ static void set_fixed_range(int msr, int * changed, unsigned int * msrwords) } } -int generic_get_free_region(unsigned long base, unsigned long size, int replace_reg) +int mtrr_generic_get_free_region(unsigned long base, unsigned long size, int replace_reg) /* [SUMMARY] Get a free MTRR. <base> The starting (base) address of the region. <size> The size (in bytes) of the region. @@ -272,7 +272,7 @@ int generic_get_free_region(unsigned long base, unsigned long size, int replace_ return -ENOSPC; } -static void generic_get_mtrr(unsigned int reg, unsigned long *base, +void mtrr_generic_get(unsigned int reg, unsigned long *base, unsigned long *size, mtrr_type *type) { uint64_t _mask, _base; @@ -448,7 +448,7 @@ static void post_set(void) spin_unlock(&set_atomicity_lock); } -static void generic_set_all(void) +void mtrr_generic_set_all(void) { unsigned long mask, count; unsigned long flags; @@ -471,7 +471,7 @@ static void generic_set_all(void) } -static void generic_set_mtrr(unsigned int reg, unsigned long base, +void mtrr_generic_set(unsigned int reg, unsigned long base, unsigned long size, mtrr_type type) /* [SUMMARY] Set variable MTRR register on the local CPU. <reg> The register to set. @@ -514,7 +514,7 @@ static void generic_set_mtrr(unsigned int reg, unsigned long base, local_irq_restore(flags); } -int generic_validate_add_page(unsigned long base, unsigned long size, unsigned int type) +int mtrr_generic_validate_add_page(unsigned long base, unsigned long size, unsigned int type) { unsigned long lbase, last; @@ -549,7 +549,7 @@ int generic_validate_add_page(unsigned long base, unsigned long size, unsigned i } -static int generic_have_wrcomb(void) +int mtrr_generic_have_wrcomb(void) { unsigned long config; rdmsrl(MSR_MTRRcap, config); @@ -565,10 +565,10 @@ int positive_have_wrcomb(void) */ const struct mtrr_ops generic_mtrr_ops = { .use_intel_if = 1, - .set_all = generic_set_all, - .get = generic_get_mtrr, - .get_free_region = generic_get_free_region, - .set = generic_set_mtrr, - .validate_add_page = generic_validate_add_page, - .have_wrcomb = generic_have_wrcomb, + .set_all = mtrr_generic_set_all, + .get = mtrr_generic_get, + .get_free_region = mtrr_generic_get_free_region, + .set = mtrr_generic_set, + .validate_add_page = mtrr_generic_validate_add_page, + .have_wrcomb = mtrr_generic_have_wrcomb, }; diff --git a/xen/arch/x86/cpu/mtrr/mtrr.h b/xen/arch/x86/cpu/mtrr/mtrr.h index b41eb58..619575f 100644 --- a/xen/arch/x86/cpu/mtrr/mtrr.h +++ b/xen/arch/x86/cpu/mtrr/mtrr.h @@ -29,10 +29,16 @@ struct mtrr_ops { int (*have_wrcomb)(void); }; -extern int generic_get_free_region(unsigned long base, unsigned long size, - int replace_reg); -extern int generic_validate_add_page(unsigned long base, unsigned long size, - unsigned int type); +void mtrr_generic_get(unsigned int reg, unsigned long *base, + unsigned long *size, mtrr_type *type); +int mtrr_generic_get_free_region(unsigned long base, unsigned long size, + int replace_reg); +int mtrr_generic_validate_add_page(unsigned long base, unsigned long size, + unsigned int type); +void mtrr_generic_set_all(void); +void mtrr_generic_set(unsigned int reg, unsigned long base, + unsigned long size, mtrr_type type); +int mtrr_generic_have_wrcomb(void); extern const struct mtrr_ops generic_mtrr_ops;
For the functions that make up the interface to the MTRR code, drop the static keyword and prefix them all with mtrr for improved clarity when they're called outside this file. This also required adjusting or providing function prototypes to make them callable. Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- xen/arch/x86/cpu/mtrr/generic.c | 24 ++++++++++++------------ xen/arch/x86/cpu/mtrr/mtrr.h | 14 ++++++++++---- 2 files changed, 22 insertions(+), 16 deletions(-)