All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Fenghua Yu' <fenghua.yu@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ravi V Shankar <ravi.v.shankar@intel.com>,
	"Xiaoyao Li " <xiaoyao.li@intel.com>,
	Christopherson Sean J <sean.j.christopherson@intel.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	Michael Chan <michael.chan@broadcom.com>
Cc: linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: RE: [PATCH v7 04/21] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access
Date: Thu, 18 Apr 2019 09:20:53 +0000	[thread overview]
Message-ID: <c8ca959a4bc549b59baca563660d42d6@AcuMS.aculab.com> (raw)
In-Reply-To: <1555536851-17462-5-git-send-email-fenghua.yu@intel.com>

From: Fenghua Yu
> Sent: 17 April 2019 22:34
> 
> set_cpu_cap() calls locked BTS and clear_cpu_cap() calls locked BTR to
> operate on bitmap defined in x86_capability.
> 
> Locked BTS/BTR accesses a single unsigned long location. In 64-bit mode,
> the location is at:
> base address of x86_capability + (bit offset in x86_capability / 64) * 8
> 
> Since base address of x86_capability may not be aligned to unsigned long,
> the single unsigned long location may cross two cache lines and
> accessing the location by locked BTS/BTR introductions will cause
> split lock.

Isn't the problem that the type (and definition) of x86_capability[] are wrong.
If the 'bitmap' functions are used for it, it should be defined as a bitmap.
This would make it 'unsigned long' not __u32.

This type munging of bitmaps only works on LE systems.

OTOH the locked BTS/BTR instructions could be changed to use 32 bit accesses.
ISTR some of the associated functions use byte accesses.

Perhaps there ought to be asm wrappers for BTS/BTR that do 8bit and
32bit accesses.

> 
> To fix the split lock issue, align x86_capability to size of unsigned long
> so that the location will be always within one cache line.
> 
> Changing x86_capability's type to unsigned long may also fix the issue
> because x86_capability will be naturally aligned to size of unsigned long.
> But this needs additional code changes. So choose the simpler solution
> by setting the array's alignment to size of unsigned long.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/include/asm/processor.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 2bb3a648fc12..7c62b9ad6e5a 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -93,7 +93,9 @@ struct cpuinfo_x86 {
>  	__u32			extended_cpuid_level;
>  	/* Maximum supported CPUID level, -1=no CPUID: */
>  	int			cpuid_level;
> -	__u32			x86_capability[NCAPINTS + NBUGINTS];
> +	/* Aligned to size of unsigned long to avoid split lock in atomic ops */
> +	__u32			x86_capability[NCAPINTS + NBUGINTS]
> +				__aligned(sizeof(unsigned long));
>  	char			x86_vendor_id[16];
>  	char			x86_model_id[64];
>  	/* in KB - valid for CPUS which support this call: */
> --
> 2.19.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  reply	other threads:[~2019-04-18  9:19 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-17 21:33 [PATCH v7 00/21] x86/split_lock: Enable split lock detection Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 01/21] x86/common: Align cpu_caps_cleared and cpu_caps_set to unsigned long Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 02/21] drivers/net/b44: Align pwol_mask to unsigned long for better performance Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 03/21] wlcore: simplify/fix/optimize reg_ch_conf_pending operations Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 04/21] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access Fenghua Yu
2019-04-18  9:20   ` David Laight [this message]
2019-04-18 11:08     ` David Laight
2019-04-18 11:49       ` Thomas Gleixner
2019-04-18 13:14         ` David Laight
2019-04-18 13:26           ` David Laight
2019-04-17 21:33 ` [PATCH v7 05/21] x86/msr-index: Define MSR_IA32_CORE_CAPABILITY and split lock detection bit Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 06/21] x86/cpufeatures: Enumerate MSR_IA32_CORE_CAPABILITY Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 07/21] x86/split_lock: Enumerate split lock detection by MSR_IA32_CORE_CAPABILITY Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 08/21] x86/split_lock: Enumerate split lock detection on Icelake mobile processor Fenghua Yu
2019-04-17 21:33 ` [PATCH v7 09/21] x86/split_lock: Define MSR TEST_CTL register Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 10/21] x86/split_lock: Define per CPU variable to cache MSR TEST_CTL Fenghua Yu
2019-04-17 22:14   ` Thomas Gleixner
2019-04-18  1:28     ` Fenghua Yu
2019-04-18  6:31       ` Thomas Gleixner
2019-04-17 21:34 ` [PATCH v7 11/21] x86/split_lock: Handle #AC exception for split lock Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 12/21] kvm/x86: Emulate MSR IA32_CORE_CAPABILITY Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 13/21] kvm/vmx: Emulate MSR TEST_CTL Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 14/21] x86/split_lock: Enable split lock detection by default Fenghua Yu
2019-04-17 22:41   ` Thomas Gleixner
2019-04-17 21:34 ` [PATCH v7 15/21] x86/split_lock: Add a sysfs interface to enable/disable split lock detection during run time Fenghua Yu
2019-04-17 22:47   ` Thomas Gleixner
2019-04-18  0:57     ` Fenghua Yu
2019-04-18  6:41       ` Thomas Gleixner
2019-04-23 20:48         ` Fenghua Yu
2019-04-24 13:45           ` David Laight
2019-04-17 21:34 ` [PATCH v7 16/21] x86/split_lock: Document the new sysfs file for split lock detection Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 17/21] x86/clearcpuid: Support multiple clearcpuid options Fenghua Yu
2019-04-17 23:05   ` Thomas Gleixner
2019-04-17 21:34 ` [PATCH v7 18/21] x86/clearcpuid: Support feature flag string in kernel option clearcpuid Fenghua Yu
2019-04-17 23:19   ` Thomas Gleixner
2019-04-17 23:47     ` Fenghua Yu
2019-04-18  6:16       ` Thomas Gleixner
2019-04-17 21:34 ` [PATCH v7 19/21] x86/clearcpuid: Apply cleared feature bits that are forced set before Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 20/21] x86/clearcpuid: Clear CPUID bit in CPUID faulting Fenghua Yu
2019-04-17 21:34 ` [PATCH v7 21/21] x86/clearcpuid: Change document for kernel option clearcpuid Fenghua Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c8ca959a4bc549b59baca563660d42d6@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=ashok.raj@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=kvalo@codeaurora.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.