Re: [Xen-devel] [PATCH v13 12/23] x86: refactor psr: L3 CAT: set value: implement write msr flow.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 07/06/17 4:07 AM >>>
>v13:
    >- use 'skip_prior_features'.
>- add 'const' for some variables.

You didn't go quite far enough with this:

>+struct cos_write_info
>+{
>+    unsigned int cos;
>+    struct feat_node *feature;
>+    const uint32_t *val;

With this, ...

>static int write_psr_msrs(unsigned int socket, unsigned int cos,
                           >uint32_t val[], unsigned int array_len,
                           
... I can't see why this can't be const too. Of course that would then affect an
earlier patch.

>enum psr_feat_type feat_type)
>{
>-    return -ENOENT;
>+    int ret;
>+    struct psr_socket_info *info = get_socket_info(socket);
>+    struct cos_write_info data =
>+    {
>+        .cos = cos,
>+        .feature = info->features[feat_type],
>+        .props = feat_props[feat_type],
>+    };
>+
>+    if ( cos > info->features[feat_type]->cos_max )
>+        return -EINVAL;
>+
>+    /* Skip to the feature's value head. */
>+    ret = skip_prior_features(&val, &array_len, feat_type);
>+    if ( ret )
>+        return ret;
>+
>+    if ( array_len < feat_props[feat_type]->cos_num )
>+        return -ENOSPC;
>+
>+    data.val = val;
>+
>+    if ( socket == cpu_to_socket(smp_processor_id()) )
>+        do_write_psr_msrs(&data);
>+    else
>+    {
>+        unsigned int cpu = get_socket_cpu(socket);
>+
>+        if ( cpu >= nr_cpu_ids )
>+            return -ENOTSOCK;
>+        on_selected_cpus(cpumask_of(cpu), do_write_psr_msrs, &data, 1);

How frequent an operation can this be? Considering that the actual MSR write(s)
in the handler is (are) conditional I wonder whether it wouldn't be worthwhile
trying to avoid the IPI altogether, by pre-checking whether any write actually
needs doing.

Jan


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