Re: [Xen-devel] [PATCH v5 04/16] x86: a few optimizations to psr codes

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Sat, Sep 30, 2017 at 01:39:14AM +0000, Yi Sun wrote:
> This patch refines psr codes:
> 1. Change type of 'cat_init_feature' to 'bool' to remove the pointless
>    returning of error code.
> 2. Move printk in 'cat_init_feature' to reduce a return path.
> 3. Define a local variable 'ebx' in 'psr_cpu_init' to reduce calling of
>    'cpuid_count_leaf()'.
> 4. Change type of 'write_msr()' to 'uint32_t'. This is needed by later patch:
>    "x86: implement set value flow for MBA".
> 
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

You can keep my RB if the changes in the new version are the ones I
mention below.

> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> 
> v1:
>     - create this patch to make codes clearer.
>       (suggested by Jan Beulich and Roger Pau Monné)
> ---
>  xen/arch/x86/psr.c | 55 
> +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index ac2ae32..c8db0c1 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -117,7 +117,7 @@ static const struct feat_props {
>                            uint32_t data[], unsigned int array_len);
>  
>      /* write_msr is used to write out feature MSR register. */
> -    void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
> +    uint32_t (*write_msr)(unsigned int cos, uint32_t val, enum psr_type 
> type);

With what Jan has requested (adjusting the value before writing it),
do you still need write_msr to return a value?

>  static const struct feat_props l2_cat_props = {
> @@ -1410,6 +1414,7 @@ static void psr_cpu_init(void)
>      unsigned int socket, cpu = smp_processor_id();
>      struct feat_node *feat;
>      struct cpuid_leaf regs;
> +    uint32_t ebx;

IIRC Jan also had some comments on the name of this variable.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel