On 30.05.2025 15:17, Sergii Dmytruk wrote:
> This allows the functionality to be reused by other units that need to
> update MTRRs.
>
> This also gets rid of a static variable.
>
> Signed-off-by: Sergii Dmytruk <sergii.dmyt...@3mdeb.com>
This may want to be split.
> --- a/xen/arch/x86/cpu/mtrr/generic.c
> +++ b/xen/arch/x86/cpu/mtrr/generic.c
> @@ -396,9 +396,7 @@ static bool set_mtrr_var_ranges(unsigned int index,
> struct mtrr_var_range *vr)
> return changed;
> }
>
> -static uint64_t deftype;
> -
> -static unsigned long set_mtrr_state(void)
> +static unsigned long set_mtrr_state(uint64_t *deftype)
> /* [SUMMARY] Set the MTRR state for this CPU.
> <state> The MTRR state information to read.
> <ctxt> Some relevant CPU context.
> @@ -416,14 +414,12 @@ static unsigned long set_mtrr_state(void)
> if (mtrr_state.have_fixed && set_fixed_ranges(mtrr_state.fixed_ranges))
> change_mask |= MTRR_CHANGE_MASK_FIXED;
>
> - /* Set_mtrr_restore restores the old value of MTRRdefType,
> - so to set it we fiddle with the saved value */
> - if ((deftype & 0xff) != mtrr_state.def_type
> - || MASK_EXTR(deftype, MTRRdefType_E) != mtrr_state.enabled
> - || MASK_EXTR(deftype, MTRRdefType_FE) != mtrr_state.fixed_enabled) {
> - deftype = (deftype & ~0xcff) | mtrr_state.def_type |
> - MASK_INSR(mtrr_state.enabled, MTRRdefType_E) |
> - MASK_INSR(mtrr_state.fixed_enabled, MTRRdefType_FE);
> + if ((*deftype & 0xff) != mtrr_state.def_type
> + || MASK_EXTR(*deftype, MTRRdefType_E) != mtrr_state.enabled
> + || MASK_EXTR(*deftype, MTRRdefType_FE) != mtrr_state.fixed_enabled)
> {
> + *deftype = (*deftype & ~0xcff) | mtrr_state.def_type |
> + MASK_INSR(mtrr_state.enabled, MTRRdefType_E) |
> + MASK_INSR(mtrr_state.fixed_enabled, MTRRdefType_FE);
> change_mask |= MTRR_CHANGE_MASK_DEFTYPE;
> }
This (together with the caller side adjustment) looks like it could be a
separate
change.
> @@ -440,9 +436,10 @@ static DEFINE_SPINLOCK(set_atomicity_lock);
> * has been called.
> */
>
> -static bool prepare_set(void)
> +struct mtrr_pausing_state mtrr_pause_caching(void)
These becoming non-static without being called from anywhere else isn't going to
be liked by Misra. Hence the part of static -> extern may need to be deferred
until the new user(s) appear(s).
Furthermore this returning of a struct by value isn't very nice, and looks to be
easy to avoid here.
Jan
