Re: [Xen-devel] [PATCH v3 05/15] x86: refactor psr: Use 'feat_mask' to record featues enabled.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.10.16 at 05:40, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> @@ -33,6 +36,8 @@ struct psr_cat_hw_info {
>  };
>  
>  struct psr_cat_socket_info {
> +    /* bit 1~0: [01]->L3 CAT-only, [10]->L3 CDP */
> +    unsigned int feat_mask;

For the moment this looks like it wants to be an enumeration. Are
you going to later add bits such that more than one can be set at
a time?

> @@ -238,7 +241,7 @@ static inline void psr_assoc_init(void)
>      {
>          unsigned int socket = cpu_to_socket(smp_processor_id());
>  
> -        if ( test_bit(socket, cat_socket_enable) )
> +        if ( cat_socket_info[socket].feat_mask )
>              psra->cos_mask = ((1ull << get_count_order(
>                            cat_socket_info[socket].l3_info.cos_max)) - 1) << 
> 32;

With it being unclear what this is going to be used for later, it
seems questionable that you test for any flag to be set, rather
than just the low two ones. The more that this guard a
construct which has been made (more obvious that it is?) L3
CAT specific by a prior patch.

This would then of course also affect other parts of the patch.

Jan


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