[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/3] x86/spec-ctrl: Split the "Hardware features" diagnostic line


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 19 Aug 2021 16:38:58 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=piQFJEwYN8aTQq5BcODBpOIW9jmbEtxFltx0I56fXZg=; b=ecRZ/4LSXcXoV0Q31zN9CUfQQK/IepYhK4l4glYtB12pPyLtaH19XeJSINc6AdUDl4Xy69X1LSI7nwWQy4coFjY4epZ1kyDG2ka8xq8l+RAIlDMRPs2+M1m3tTnSomUb3KbrgrCtgGgycQOEWefNLpcV2pdUnYgoAuBLt4lYJbDZvLYK2QITljUfCJOCoX35Le4pH0ZX8kFqAbw1/8ieqZ4gcdOImEsl/84LbkTUqG2jxC8V0sLeT1jVUhF9VaFjjAiPlOljMVXO8fIjMFPdpSx4wx097mihskJ/zvuA/GpqzVunDnPPbZPY8pbrDe9yobOZMD4l+Vis9ShDirfFIA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ivm21X4/NLYZWMfGm9gJ+NywhIfDd14uJVLLaGHTFa0fT6Krq7wTa4HGO+TJdN2XgwoqR0kVLSPBex4XqGUjPneNWhY1JjaZy0CgM0SEEijMHPInXXvZDlkZl3fkBBk0K2SonHR3H5QShriWcaQJ9x0Bv3lOdLQNCUFIimyjeaGbNB4DxT0ywlf+MHPt3CM9ZaFC6L77rjFf4kVrd5xhvV2/dIJynnYrjo9nkVowbh6YL7LTs6imLE5+otnvX5sIuP/X8vxxh2UGMR2nDXdmceHhs+DQ6kKKyWadz6VXrIh07Uh2+VqRpa3oEsDqlswuTRD9B4Ae86Z1asWiTTtXkQ==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 19 Aug 2021 14:39:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 17.08.2021 16:30, Andrew Cooper wrote:
> Separate the read-only hints from the features requiring active actions on
> Xen's behalf.
> 
> Also take the opportunity split the IBRS/IBPB and IBPB mess.  More features
> with overlapping enumeration are on the way, and and it is not useful to split
> them like this.
> 
> No practical change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with a remark and a question:

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -317,23 +317,30 @@ static void __init print_details(enum ind_thunk thunk, 
> uint64_t caps)
>  
>      printk("Speculative mitigation facilities:\n");
>  
> -    /* Hardware features which pertain to speculative mitigations. */
> -    printk("  Hardware features:%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_L1D_FLUSH)) ? " L1D_FLUSH" : "",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_SSBD))  ? " SSBD"      : "",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_MD_CLEAR)) ? " MD_CLEAR" : "",
> -           (_7d0 & cpufeat_mask(X86_FEATURE_SRBDS_CTRL)) ? " SRBDS_CTRL" : 
> "",
> -           (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
> -           (caps & ARCH_CAPS_IBRS_ALL)              ? " IBRS_ALL"  : "",
> -           (caps & ARCH_CAPS_RDCL_NO)               ? " RDCL_NO"   : "",
> -           (caps & ARCH_CAPS_RSBA)                  ? " RSBA"      : "",
> -           (caps & ARCH_CAPS_SKIP_L1DFL)            ? " SKIP_L1DFL": "",
> -           (caps & ARCH_CAPS_SSB_NO)                ? " SSB_NO"    : "",
> -           (caps & ARCH_CAPS_MDS_NO)                ? " MDS_NO"    : "",
> -           (caps & ARCH_CAPS_TSX_CTRL)              ? " TSX_CTRL"  : "",
> -           (caps & ARCH_CAPS_TAA_NO)                ? " TAA_NO"    : "");
> +    /*
> +     * Hardware read-only information, stating immunity to certain issues, or
> +     * suggestions of which mitigation to use.
> +     */
> +    printk("  Hardware hints:%s%s%s%s%s%s%s\n",
> +           (caps & ARCH_CAPS_RDCL_NO)                        ? " RDCL_NO"    
>     : "",
> +           (caps & ARCH_CAPS_IBRS_ALL)                       ? " IBRS_ALL"   
>     : "",

I take it you flipped the order of these two to match the ordering
of their bit numbers? I'm slightly inclined to ask whether we
wouldn't better stay with what we had, as I could imagine users
having not sufficiently flexible text matching in place somewhere.
But I'm not going to insist. It only occurred to me and is, unlike
for the IBRS/IBPB re-arrangement of the other part, easily possible
here.

> +           (caps & ARCH_CAPS_RSBA)                           ? " RSBA"       
>     : "",
> +           (caps & ARCH_CAPS_SKIP_L1DFL)                     ? " SKIP_L1DFL" 
>     : "",
> +           (caps & ARCH_CAPS_SSB_NO)                         ? " SSB_NO"     
>     : "",
> +           (caps & ARCH_CAPS_MDS_NO)                         ? " MDS_NO"     
>     : "",
> +           (caps & ARCH_CAPS_TAA_NO)                         ? " TAA_NO"     
>     : "");

I'm curious why we do not report IF_PSCHANGE_MC_NO here.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.