2012-08-08 17:03:19

by Ido Yariv

[permalink] [raw]
Subject: [PATCH 1/2] vsmp: Use hypervisor layer for initialization

vSMP Foundation detection and initialization could be done by the
hypervisor layer, so set it up that way.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/include/asm/setup.h | 7 -------
arch/x86/kernel/cpu/hypervisor.c | 1 +
arch/x86/kernel/setup.c | 2 --
arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index b518c75..eb617ae 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -50,6 +50,7 @@ extern const struct hypervisor_x86 x86_hyper_vmware;
extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
extern const struct hypervisor_x86 x86_hyper_xen_hvm;
extern const struct hypervisor_x86 x86_hyper_kvm;
+extern const struct hypervisor_x86 x86_hyper_vsmp;

static inline bool hypervisor_x2apic_available(void)
{
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index d0f19f9..a4c5b14 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -26,13 +26,6 @@
#include <asm/bootparam.h>
#include <asm/x86_init.h>

-/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_64
-void vsmp_init(void);
-#else
-static inline void vsmp_init(void) { }
-#endif
-
void setup_bios_corruption_check(void);

#ifdef CONFIG_X86_VISWS
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index a8f8fa9..bcac2ff6 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -40,6 +40,7 @@ static const __initconst struct hypervisor_x86 * const hypervisors[] =
#ifdef CONFIG_KVM_GUEST
&x86_hyper_kvm,
#endif
+ &x86_hyper_vsmp,
};

const struct hypervisor_x86 *x86_hyper;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f4b9b80..f9706d1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -943,8 +943,6 @@ void __init setup_arch(char **cmdline_p)

reserve_crashkernel();

- vsmp_init();
-
io_delay_init();

/*
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 992f890..f655f2c 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -17,12 +17,14 @@
#include <linux/pci_regs.h>
#include <linux/smp.h>
#include <linux/irq.h>
+#include <linux/module.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
#include <asm/io.h>
#include <asm/paravirt.h>
#include <asm/setup.h>
+#include <asm/hypervisor.h>

#define TOPOLOGY_REGISTER_OFFSET 0x10

@@ -132,17 +134,20 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_PCI
static int is_vsmp = -1;

-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
is_vsmp = 0;

if (!early_pci_allowed())
- return;
+ goto out;

/* Check if we are running on a ScaleMP vSMPowered box */
if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
(PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
is_vsmp = 1;
+
+out:
+ return is_vsmp == 1;
}

int is_vsmp_box(void)
@@ -156,8 +161,9 @@ int is_vsmp_box(void)
}

#else
-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
+ return false;
}
int is_vsmp_box(void)
{
@@ -221,16 +227,17 @@ static void vsmp_apic_post_init(void)
apic->vector_allocation_domain = fill_vector_allocation_domain;
}

-void __init vsmp_init(void)
+static void __init vsmp_platform_setup(void)
{
- detect_vsmp_box();
- if (!is_vsmp_box())
- return;
-
x86_platform.apic_post_init = vsmp_apic_post_init;

vsmp_cap_cpus();
-
set_vsmp_pv_ops();
- return;
}
+
+const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
+ .name = "ScaleMP vSMP Foundation",
+ .detect = detect_vsmp_box,
+ .init_platform = vsmp_platform_setup,
+};
+EXPORT_SYMBOL(x86_hyper_vsmp);
--
1.7.7.6


2012-08-08 17:03:22

by Ido Yariv

[permalink] [raw]
Subject: [PATCH v2 2/2] x86/pat: Avoid contention on cpa_lock if possible

vSMP Foundation does not require to serialize CPA by guaranteeing that
the most recent TLB entry will always be used.

To avoid needless contention on cpa_lock, do not lock/unlock it if it
isn't necessary.

Based on work by Shai Fultheim <[email protected]>.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
Changes from v1:
- Use a synthetic CPUID bit and a use static_cpu_has() as suggested by
H. Peter Avnin

arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/vsmp_64.c | 10 ++++++++++
arch/x86/mm/pageattr.c | 30 +++++++++++++++++++++---------
3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 6b7ee5f..92303a0 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -97,6 +97,7 @@
#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */
#define X86_FEATURE_AMD_DCM (3*32+27) /* multi-node processor */
#define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
+#define X86_FEATURE_NO_CPA_LOCK (3*32+29) /* Serializing cpa is not required */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index f655f2c..994359b 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -235,9 +235,19 @@ static void __init vsmp_platform_setup(void)
set_vsmp_pv_ops();
}

+static void __cpuinit vsmp_set_cpu_features(struct cpuinfo_x86 *c)
+{
+ /*
+ * vSMP guarantees that the most recent TLB entry will always be used,
+ * so we can avoid serializing cpa
+ */
+ set_cpu_cap(c, X86_FEATURE_NO_CPA_LOCK);
+}
+
const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
.name = "ScaleMP vSMP Foundation",
.detect = detect_vsmp_box,
.init_platform = vsmp_platform_setup,
+ .set_cpu_features = vsmp_set_cpu_features,
};
EXPORT_SYMBOL(x86_hyper_vsmp);
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 931930a..95fbb41 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -46,7 +46,7 @@ struct cpa_data {
* entries change the page attribute in parallel to some other cpu
* splitting a large page entry along with changing the attribute.
*/
-static DEFINE_SPINLOCK(cpa_lock);
+static DEFINE_SPINLOCK(_cpa_lock);

#define CPA_FLUSHTLB 1
#define CPA_ARRAY 2
@@ -110,6 +110,22 @@ static inline unsigned long highmap_end_pfn(void)
# define debug_pagealloc 0
#endif

+static inline void cpa_lock(void)
+{
+ if (debug_pagealloc || static_cpu_has(X86_FEATURE_NO_CPA_LOCK))
+ return;
+
+ spin_lock(&_cpa_lock);
+}
+
+static inline void cpa_unlock(void)
+{
+ if (debug_pagealloc || static_cpu_has(X86_FEATURE_NO_CPA_LOCK))
+ return;
+
+ spin_unlock(&_cpa_lock);
+}
+
static inline int
within(unsigned long addr, unsigned long start, unsigned long end)
{
@@ -509,11 +525,9 @@ static int split_large_page(pte_t *kpte, unsigned long address)
pgprot_t ref_prot;
struct page *base;

- if (!debug_pagealloc)
- spin_unlock(&cpa_lock);
+ cpa_unlock();
base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
- if (!debug_pagealloc)
- spin_lock(&cpa_lock);
+ cpa_lock();
if (!base)
return -ENOMEM;

@@ -801,11 +815,9 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int checkalias)
if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
cpa->numpages = 1;

- if (!debug_pagealloc)
- spin_lock(&cpa_lock);
+ cpa_lock();
ret = __change_page_attr(cpa, checkalias);
- if (!debug_pagealloc)
- spin_unlock(&cpa_lock);
+ cpa_unlock();
if (ret)
return ret;

--
1.7.7.6

2012-08-19 09:12:33

by Ido Yariv

[permalink] [raw]
Subject: Re: [PATCH 1/2] vsmp: Use hypervisor layer for initialization

On Wed, Aug 08, 2012 at 08:01:38PM +0300, Ido Yariv wrote:
> vSMP Foundation detection and initialization could be done by the
> hypervisor layer, so set it up that way.
>
> Signed-off-by: Ido Yariv <[email protected]>
> Acked-by: Shai Fultheim <[email protected]>
> ---

Ping?

Thanks,
Ido.

2012-08-22 08:47:20

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] x86/pat: Avoid contention on cpa_lock if possible


* Ido Yariv <[email protected]> wrote:

> vSMP Foundation does not require to serialize CPA by guaranteeing that
> the most recent TLB entry will always be used.
>
> To avoid needless contention on cpa_lock, do not lock/unlock it if it
> isn't necessary.
>
> Based on work by Shai Fultheim <[email protected]>.
>
> Signed-off-by: Ido Yariv <[email protected]>
> Acked-by: Shai Fultheim <[email protected]>
> ---
> Changes from v1:
> - Use a synthetic CPUID bit and a use static_cpu_has() as suggested by
> H. Peter Avnin
>
> arch/x86/include/asm/cpufeature.h | 1 +
> arch/x86/kernel/vsmp_64.c | 10 ++++++++++
> arch/x86/mm/pageattr.c | 30 +++++++++++++++++++++---------
> 3 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 6b7ee5f..92303a0 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -97,6 +97,7 @@
> #define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */
> #define X86_FEATURE_AMD_DCM (3*32+27) /* multi-node processor */
> #define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
> +#define X86_FEATURE_NO_CPA_LOCK (3*32+29) /* Serializing cpa is not required */

Patch looks mostly good, but could we please use some more
hardware-ish name, instead of referring to a kernel lock?

I.e. how would you name it if this was a real hardware feature?
Certainly not 'No CPA Lock'.

Thanks,

Ingo

2012-08-22 09:05:05

by Ido Yariv

[permalink] [raw]
Subject: [tip:x86/platform] x86/vsmp: Use hypervisor layer for initialization

Commit-ID: c00679af441c5be76e195deed2c76fef65a5d102
Gitweb: http://git.kernel.org/tip/c00679af441c5be76e195deed2c76fef65a5d102
Author: Ido Yariv <[email protected]>
AuthorDate: Wed, 8 Aug 2012 20:01:38 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 22 Aug 2012 10:48:05 +0200

x86/vsmp: Use hypervisor layer for initialization

vSMP detection and initialization could be done by
the hypervisor layer, so set it up that way.

This cleans up vSMP initialization.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/include/asm/setup.h | 7 -------
arch/x86/kernel/cpu/hypervisor.c | 1 +
arch/x86/kernel/setup.c | 2 --
arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index b518c75..eb617ae 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -50,6 +50,7 @@ extern const struct hypervisor_x86 x86_hyper_vmware;
extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
extern const struct hypervisor_x86 x86_hyper_xen_hvm;
extern const struct hypervisor_x86 x86_hyper_kvm;
+extern const struct hypervisor_x86 x86_hyper_vsmp;

static inline bool hypervisor_x2apic_available(void)
{
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index d0f19f9..a4c5b14 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -26,13 +26,6 @@
#include <asm/bootparam.h>
#include <asm/x86_init.h>

-/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_64
-void vsmp_init(void);
-#else
-static inline void vsmp_init(void) { }
-#endif
-
void setup_bios_corruption_check(void);

#ifdef CONFIG_X86_VISWS
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index a8f8fa9..bcac2ff6 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -40,6 +40,7 @@ static const __initconst struct hypervisor_x86 * const hypervisors[] =
#ifdef CONFIG_KVM_GUEST
&x86_hyper_kvm,
#endif
+ &x86_hyper_vsmp,
};

const struct hypervisor_x86 *x86_hyper;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f4b9b80..f9706d1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -943,8 +943,6 @@ void __init setup_arch(char **cmdline_p)

reserve_crashkernel();

- vsmp_init();
-
io_delay_init();

/*
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 992f890..f655f2c 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -17,12 +17,14 @@
#include <linux/pci_regs.h>
#include <linux/smp.h>
#include <linux/irq.h>
+#include <linux/module.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
#include <asm/io.h>
#include <asm/paravirt.h>
#include <asm/setup.h>
+#include <asm/hypervisor.h>

#define TOPOLOGY_REGISTER_OFFSET 0x10

@@ -132,17 +134,20 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_PCI
static int is_vsmp = -1;

-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
is_vsmp = 0;

if (!early_pci_allowed())
- return;
+ goto out;

/* Check if we are running on a ScaleMP vSMPowered box */
if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
(PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
is_vsmp = 1;
+
+out:
+ return is_vsmp == 1;
}

int is_vsmp_box(void)
@@ -156,8 +161,9 @@ int is_vsmp_box(void)
}

#else
-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
+ return false;
}
int is_vsmp_box(void)
{
@@ -221,16 +227,17 @@ static void vsmp_apic_post_init(void)
apic->vector_allocation_domain = fill_vector_allocation_domain;
}

-void __init vsmp_init(void)
+static void __init vsmp_platform_setup(void)
{
- detect_vsmp_box();
- if (!is_vsmp_box())
- return;
-
x86_platform.apic_post_init = vsmp_apic_post_init;

vsmp_cap_cpus();
-
set_vsmp_pv_ops();
- return;
}
+
+const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
+ .name = "ScaleMP vSMP Foundation",
+ .detect = detect_vsmp_box,
+ .init_platform = vsmp_platform_setup,
+};
+EXPORT_SYMBOL(x86_hyper_vsmp);

2012-08-22 09:11:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Use hypervisor layer for initialization


* tip-bot for Ido Yariv <[email protected]> wrote:

> Commit-ID: c00679af441c5be76e195deed2c76fef65a5d102
> Gitweb: http://git.kernel.org/tip/c00679af441c5be76e195deed2c76fef65a5d102
> Author: Ido Yariv <[email protected]>
> AuthorDate: Wed, 8 Aug 2012 20:01:38 +0300
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Wed, 22 Aug 2012 10:48:05 +0200
>
> x86/vsmp: Use hypervisor layer for initialization
>
> vSMP detection and initialization could be done by
> the hypervisor layer, so set it up that way.
>
> This cleans up vSMP initialization.
>
> Signed-off-by: Ido Yariv <[email protected]>
> Acked-by: Shai Fultheim <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/x86/include/asm/hypervisor.h | 1 +
> arch/x86/include/asm/setup.h | 7 -------
> arch/x86/kernel/cpu/hypervisor.c | 1 +
> arch/x86/kernel/setup.c | 2 --
> arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
> 5 files changed, 19 insertions(+), 19 deletions(-)

I had to zap this commit because it breaks the build:

(.init.rodata+0x4c8): undefined reference to `x86_hyper_vsmp'

Thanks,

Ingo

2012-08-22 09:46:57

by Ido Yariv

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Use hypervisor layer for initialization

Hi Ingo,

On Wed, Aug 22, 2012 at 11:11:41AM +0200, Ingo Molnar wrote:
>
> * tip-bot for Ido Yariv <[email protected]> wrote:
>
> > Commit-ID: c00679af441c5be76e195deed2c76fef65a5d102
> > Gitweb: http://git.kernel.org/tip/c00679af441c5be76e195deed2c76fef65a5d102
> > Author: Ido Yariv <[email protected]>
> > AuthorDate: Wed, 8 Aug 2012 20:01:38 +0300
> > Committer: Ingo Molnar <[email protected]>
> > CommitDate: Wed, 22 Aug 2012 10:48:05 +0200
> >
> > x86/vsmp: Use hypervisor layer for initialization
> >
> > vSMP detection and initialization could be done by
> > the hypervisor layer, so set it up that way.
> >
> > This cleans up vSMP initialization.
> >
> > Signed-off-by: Ido Yariv <[email protected]>
> > Acked-by: Shai Fultheim <[email protected]>
> > Link: http://lkml.kernel.org/r/[email protected]
> > Signed-off-by: Ingo Molnar <[email protected]>
> > ---
> > arch/x86/include/asm/hypervisor.h | 1 +
> > arch/x86/include/asm/setup.h | 7 -------
> > arch/x86/kernel/cpu/hypervisor.c | 1 +
> > arch/x86/kernel/setup.c | 2 --
> > arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
> > 5 files changed, 19 insertions(+), 19 deletions(-)
>
> I had to zap this commit because it breaks the build:
>
> (.init.rodata+0x4c8): undefined reference to `x86_hyper_vsmp'

I failed to notice that vsmp_64.c is only built on 64bit, so this will
break on 32bit builds.

How about the patch below?

Thanks,
Ido.

>From 806d8966d9284d5351e9ebed43f6f801e62e946d Mon Sep 17 00:00:00 2001
From: Ido Yariv <[email protected]>
Date: Mon, 11 Jun 2012 14:41:08 +0300
Subject: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization

vSMP Foundation detection and initialization could be done by the
hypervisor layer, so set it up that way.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/include/asm/setup.h | 7 -------
arch/x86/kernel/cpu/hypervisor.c | 3 +++
arch/x86/kernel/setup.c | 2 --
arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index b518c75..eb617ae 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -50,6 +50,7 @@ extern const struct hypervisor_x86 x86_hyper_vmware;
extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
extern const struct hypervisor_x86 x86_hyper_xen_hvm;
extern const struct hypervisor_x86 x86_hyper_kvm;
+extern const struct hypervisor_x86 x86_hyper_vsmp;

static inline bool hypervisor_x2apic_available(void)
{
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index d0f19f9..a4c5b14 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -26,13 +26,6 @@
#include <asm/bootparam.h>
#include <asm/x86_init.h>

-/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_64
-void vsmp_init(void);
-#else
-static inline void vsmp_init(void) { }
-#endif
-
void setup_bios_corruption_check(void);

#ifdef CONFIG_X86_VISWS
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index a8f8fa9..f250d6a 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -40,6 +40,9 @@ static const __initconst struct hypervisor_x86 * const hypervisors[] =
#ifdef CONFIG_KVM_GUEST
&x86_hyper_kvm,
#endif
+#ifdef CONFIG_X86_64
+ &x86_hyper_vsmp,
+#endif
};

const struct hypervisor_x86 *x86_hyper;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f4b9b80..f9706d1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -943,8 +943,6 @@ void __init setup_arch(char **cmdline_p)

reserve_crashkernel();

- vsmp_init();
-
io_delay_init();

/*
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 992f890..f655f2c 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -17,12 +17,14 @@
#include <linux/pci_regs.h>
#include <linux/smp.h>
#include <linux/irq.h>
+#include <linux/module.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
#include <asm/io.h>
#include <asm/paravirt.h>
#include <asm/setup.h>
+#include <asm/hypervisor.h>

#define TOPOLOGY_REGISTER_OFFSET 0x10

@@ -132,17 +134,20 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_PCI
static int is_vsmp = -1;

-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
is_vsmp = 0;

if (!early_pci_allowed())
- return;
+ goto out;

/* Check if we are running on a ScaleMP vSMPowered box */
if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
(PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
is_vsmp = 1;
+
+out:
+ return is_vsmp == 1;
}

int is_vsmp_box(void)
@@ -156,8 +161,9 @@ int is_vsmp_box(void)
}

#else
-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
+ return false;
}
int is_vsmp_box(void)
{
@@ -221,16 +227,17 @@ static void vsmp_apic_post_init(void)
apic->vector_allocation_domain = fill_vector_allocation_domain;
}

-void __init vsmp_init(void)
+static void __init vsmp_platform_setup(void)
{
- detect_vsmp_box();
- if (!is_vsmp_box())
- return;
-
x86_platform.apic_post_init = vsmp_apic_post_init;

vsmp_cap_cpus();
-
set_vsmp_pv_ops();
- return;
}
+
+const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
+ .name = "ScaleMP vSMP Foundation",
+ .detect = detect_vsmp_box,
+ .init_platform = vsmp_platform_setup,
+};
+EXPORT_SYMBOL(x86_hyper_vsmp);
--
1.7.7.6

2012-08-22 09:53:26

by Ido Yariv

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] x86/pat: Avoid contention on cpa_lock if possible

Hi Ingo,

On Wed, Aug 22, 2012 at 10:47:02AM +0200, Ingo Molnar wrote:
>
> * Ido Yariv <[email protected]> wrote:
>
> > vSMP Foundation does not require to serialize CPA by guaranteeing that
> > the most recent TLB entry will always be used.
> >
> > To avoid needless contention on cpa_lock, do not lock/unlock it if it
> > isn't necessary.
> >
> > Based on work by Shai Fultheim <[email protected]>.
> >
> > Signed-off-by: Ido Yariv <[email protected]>
> > Acked-by: Shai Fultheim <[email protected]>
> > ---
> > Changes from v1:
> > - Use a synthetic CPUID bit and a use static_cpu_has() as suggested by
> > H. Peter Avnin
> >
> > arch/x86/include/asm/cpufeature.h | 1 +
> > arch/x86/kernel/vsmp_64.c | 10 ++++++++++
> > arch/x86/mm/pageattr.c | 30 +++++++++++++++++++++---------
> > 3 files changed, 32 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> > index 6b7ee5f..92303a0 100644
> > --- a/arch/x86/include/asm/cpufeature.h
> > +++ b/arch/x86/include/asm/cpufeature.h
> > @@ -97,6 +97,7 @@
> > #define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */
> > #define X86_FEATURE_AMD_DCM (3*32+27) /* multi-node processor */
> > #define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
> > +#define X86_FEATURE_NO_CPA_LOCK (3*32+29) /* Serializing cpa is not required */
>
> Patch looks mostly good, but could we please use some more
> hardware-ish name, instead of referring to a kernel lock?

Sure thing. How about X86_FEATURE_TLB_RELIABLE?

Thanks,
Ido.

>From fc3adcd1a9a86d65836d461bc22f54955ba3fc82 Mon Sep 17 00:00:00 2001
From: Ido Yariv <[email protected]>
Date: Mon, 11 Jun 2012 15:17:56 +0300
Subject: [PATCH v3] x86/pat: Avoid contention on cpa_lock if possible

vSMP Foundation does not require to serialize CPA by guaranteeing that
the most recent TLB entry will always be used.

To avoid needless contention on cpa_lock, do not lock/unlock it if it
isn't necessary.

Based on work by Shai Fultheim <[email protected]>.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/vsmp_64.c | 10 ++++++++++
arch/x86/mm/pageattr.c | 30 +++++++++++++++++++++---------
3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 6b7ee5f..9b3d075 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -97,6 +97,7 @@
#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */
#define X86_FEATURE_AMD_DCM (3*32+27) /* multi-node processor */
#define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
+#define X86_FEATURE_TLB_RELIABLE (3*32+29) /* Serializing cpa is not required */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index f655f2c..6e245d8 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -235,9 +235,19 @@ static void __init vsmp_platform_setup(void)
set_vsmp_pv_ops();
}

+static void __cpuinit vsmp_set_cpu_features(struct cpuinfo_x86 *c)
+{
+ /*
+ * vSMP guarantees that the most recent TLB entry will always be used,
+ * so we can avoid serializing cpa
+ */
+ set_cpu_cap(c, X86_FEATURE_TLB_RELIABLE);
+}
+
const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
.name = "ScaleMP vSMP Foundation",
.detect = detect_vsmp_box,
.init_platform = vsmp_platform_setup,
+ .set_cpu_features = vsmp_set_cpu_features,
};
EXPORT_SYMBOL(x86_hyper_vsmp);
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index a718e0d..bc4720e 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -46,7 +46,7 @@ struct cpa_data {
* entries change the page attribute in parallel to some other cpu
* splitting a large page entry along with changing the attribute.
*/
-static DEFINE_SPINLOCK(cpa_lock);
+static DEFINE_SPINLOCK(_cpa_lock);

#define CPA_FLUSHTLB 1
#define CPA_ARRAY 2
@@ -110,6 +110,22 @@ static inline unsigned long highmap_end_pfn(void)
# define debug_pagealloc 0
#endif

+static inline void cpa_lock(void)
+{
+ if (debug_pagealloc || static_cpu_has(X86_FEATURE_TLB_RELIABLE))
+ return;
+
+ spin_lock(&_cpa_lock);
+}
+
+static inline void cpa_unlock(void)
+{
+ if (debug_pagealloc || static_cpu_has(X86_FEATURE_TLB_RELIABLE))
+ return;
+
+ spin_unlock(&_cpa_lock);
+}
+
static inline int
within(unsigned long addr, unsigned long start, unsigned long end)
{
@@ -509,11 +525,9 @@ static int split_large_page(pte_t *kpte, unsigned long address)
pgprot_t ref_prot;
struct page *base;

- if (!debug_pagealloc)
- spin_unlock(&cpa_lock);
+ cpa_unlock();
base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
- if (!debug_pagealloc)
- spin_lock(&cpa_lock);
+ cpa_lock();
if (!base)
return -ENOMEM;

@@ -801,11 +815,9 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int checkalias)
if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
cpa->numpages = 1;

- if (!debug_pagealloc)
- spin_lock(&cpa_lock);
+ cpa_lock();
ret = __change_page_attr(cpa, checkalias);
- if (!debug_pagealloc)
- spin_unlock(&cpa_lock);
+ cpa_unlock();
if (ret)
return ret;

--
1.7.7.6

2012-08-22 10:08:02

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] x86/pat: Avoid contention on cpa_lock if possible


* Ido Yariv <[email protected]> wrote:

> Hi Ingo,
>
> On Wed, Aug 22, 2012 at 10:47:02AM +0200, Ingo Molnar wrote:
> >
> > * Ido Yariv <[email protected]> wrote:
> >
> > > vSMP Foundation does not require to serialize CPA by guaranteeing that
> > > the most recent TLB entry will always be used.
> > >
> > > To avoid needless contention on cpa_lock, do not lock/unlock it if it
> > > isn't necessary.
> > >
> > > Based on work by Shai Fultheim <[email protected]>.
> > >
> > > Signed-off-by: Ido Yariv <[email protected]>
> > > Acked-by: Shai Fultheim <[email protected]>
> > > ---
> > > Changes from v1:
> > > - Use a synthetic CPUID bit and a use static_cpu_has() as suggested by
> > > H. Peter Avnin
> > >
> > > arch/x86/include/asm/cpufeature.h | 1 +
> > > arch/x86/kernel/vsmp_64.c | 10 ++++++++++
> > > arch/x86/mm/pageattr.c | 30 +++++++++++++++++++++---------
> > > 3 files changed, 32 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> > > index 6b7ee5f..92303a0 100644
> > > --- a/arch/x86/include/asm/cpufeature.h
> > > +++ b/arch/x86/include/asm/cpufeature.h
> > > @@ -97,6 +97,7 @@
> > > #define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */
> > > #define X86_FEATURE_AMD_DCM (3*32+27) /* multi-node processor */
> > > #define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
> > > +#define X86_FEATURE_NO_CPA_LOCK (3*32+29) /* Serializing cpa is not required */
> >
> > Patch looks mostly good, but could we please use some more
> > hardware-ish name, instead of referring to a kernel lock?
>
> Sure thing. How about X86_FEATURE_TLB_RELIABLE?

Yeah, something like that. Maybe hpa can think of something
better?

Thanks,

Ingo

2012-08-22 10:08:53

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Use hypervisor layer for initialization


* Ido Yariv <[email protected]> wrote:

> Hi Ingo,
>
> On Wed, Aug 22, 2012 at 11:11:41AM +0200, Ingo Molnar wrote:
> >
> > * tip-bot for Ido Yariv <[email protected]> wrote:
> >
> > > Commit-ID: c00679af441c5be76e195deed2c76fef65a5d102
> > > Gitweb: http://git.kernel.org/tip/c00679af441c5be76e195deed2c76fef65a5d102
> > > Author: Ido Yariv <[email protected]>
> > > AuthorDate: Wed, 8 Aug 2012 20:01:38 +0300
> > > Committer: Ingo Molnar <[email protected]>
> > > CommitDate: Wed, 22 Aug 2012 10:48:05 +0200
> > >
> > > x86/vsmp: Use hypervisor layer for initialization
> > >
> > > vSMP detection and initialization could be done by
> > > the hypervisor layer, so set it up that way.
> > >
> > > This cleans up vSMP initialization.
> > >
> > > Signed-off-by: Ido Yariv <[email protected]>
> > > Acked-by: Shai Fultheim <[email protected]>
> > > Link: http://lkml.kernel.org/r/[email protected]
> > > Signed-off-by: Ingo Molnar <[email protected]>
> > > ---
> > > arch/x86/include/asm/hypervisor.h | 1 +
> > > arch/x86/include/asm/setup.h | 7 -------
> > > arch/x86/kernel/cpu/hypervisor.c | 1 +
> > > arch/x86/kernel/setup.c | 2 --
> > > arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
> > > 5 files changed, 19 insertions(+), 19 deletions(-)
> >
> > I had to zap this commit because it breaks the build:
> >
> > (.init.rodata+0x4c8): undefined reference to `x86_hyper_vsmp'
>
> I failed to notice that vsmp_64.c is only built on 64bit, so this will
> break on 32bit builds.
>
> How about the patch below?

Please test it in the configs that matter and resubmit.

Thanks,

Ingo

2012-08-22 11:20:34

by Ido Yariv

[permalink] [raw]
Subject: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization

vSMP Foundation detection and initialization could be done by the
hypervisor layer, so set it up that way.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
Changes from v1:
- Fixed build errors on 32bit kernels

arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/include/asm/setup.h | 7 -------
arch/x86/kernel/cpu/hypervisor.c | 3 +++
arch/x86/kernel/setup.c | 2 --
arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++----------
5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index b518c75..eb617ae 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -50,6 +50,7 @@ extern const struct hypervisor_x86 x86_hyper_vmware;
extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
extern const struct hypervisor_x86 x86_hyper_xen_hvm;
extern const struct hypervisor_x86 x86_hyper_kvm;
+extern const struct hypervisor_x86 x86_hyper_vsmp;

static inline bool hypervisor_x2apic_available(void)
{
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index d0f19f9..a4c5b14 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -26,13 +26,6 @@
#include <asm/bootparam.h>
#include <asm/x86_init.h>

-/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_64
-void vsmp_init(void);
-#else
-static inline void vsmp_init(void) { }
-#endif
-
void setup_bios_corruption_check(void);

#ifdef CONFIG_X86_VISWS
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index a8f8fa9..f250d6a 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -40,6 +40,9 @@ static const __initconst struct hypervisor_x86 * const hypervisors[] =
#ifdef CONFIG_KVM_GUEST
&x86_hyper_kvm,
#endif
+#ifdef CONFIG_X86_64
+ &x86_hyper_vsmp,
+#endif
};

const struct hypervisor_x86 *x86_hyper;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f4b9b80..f9706d1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -943,8 +943,6 @@ void __init setup_arch(char **cmdline_p)

reserve_crashkernel();

- vsmp_init();
-
io_delay_init();

/*
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 992f890..f655f2c 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -17,12 +17,14 @@
#include <linux/pci_regs.h>
#include <linux/smp.h>
#include <linux/irq.h>
+#include <linux/module.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
#include <asm/io.h>
#include <asm/paravirt.h>
#include <asm/setup.h>
+#include <asm/hypervisor.h>

#define TOPOLOGY_REGISTER_OFFSET 0x10

@@ -132,17 +134,20 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_PCI
static int is_vsmp = -1;

-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
is_vsmp = 0;

if (!early_pci_allowed())
- return;
+ goto out;

/* Check if we are running on a ScaleMP vSMPowered box */
if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
(PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
is_vsmp = 1;
+
+out:
+ return is_vsmp == 1;
}

int is_vsmp_box(void)
@@ -156,8 +161,9 @@ int is_vsmp_box(void)
}

#else
-static void __init detect_vsmp_box(void)
+static bool __init detect_vsmp_box(void)
{
+ return false;
}
int is_vsmp_box(void)
{
@@ -221,16 +227,17 @@ static void vsmp_apic_post_init(void)
apic->vector_allocation_domain = fill_vector_allocation_domain;
}

-void __init vsmp_init(void)
+static void __init vsmp_platform_setup(void)
{
- detect_vsmp_box();
- if (!is_vsmp_box())
- return;
-
x86_platform.apic_post_init = vsmp_apic_post_init;

vsmp_cap_cpus();
-
set_vsmp_pv_ops();
- return;
}
+
+const __refconst struct hypervisor_x86 x86_hyper_vsmp = {
+ .name = "ScaleMP vSMP Foundation",
+ .detect = detect_vsmp_box,
+ .init_platform = vsmp_platform_setup,
+};
+EXPORT_SYMBOL(x86_hyper_vsmp);
--
1.7.7.6

2012-08-22 17:38:00

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization


* Ido Yariv <[email protected]> wrote:

> +#ifdef CONFIG_X86_64
> + &x86_hyper_vsmp,
> +#endif

Why doesn't it all depend on CONFIG_X86_VSMP?

Thanks,

Ingo

Subject: RE: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization

Ingo,

CONFIG_X86_VSMP is only for setting cache-line size to 4KB. Rest of the bits were never depended on this.

Regards,

--Shai


> -----Original Message-----
> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, August 22, 2012 20:38
> To: Ido Yariv
> Cc: [email protected]; Thomas Gleixner; H. Peter Anvin; Shai
> Fultheim ([email protected])
> Subject: Re: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization
>
>
> * Ido Yariv <[email protected]> wrote:
>
> > +#ifdef CONFIG_X86_64
> > + &x86_hyper_vsmp,
> > +#endif
>
> Why doesn't it all depend on CONFIG_X86_VSMP?
>
> Thanks,
>
> Ingo

2012-08-22 17:48:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization


* Shai Fultheim ([email protected]) <[email protected]> wrote:

> Ingo,
>
> CONFIG_X86_VSMP is only for setting cache-line size to 4KB.
> Rest of the bits were never depended on this.

That's a problem - please move vSMP functionality under a
simple, properly named Kconfig toggle. Many of the build
problems are related to that.

Thanks,

Ingo

Subject: RE: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization

Ingo,

Use of vSMP Foundation does not require compiling with CONFIG_X86_VSMP, which is off by default in any distribution to date.

vsmp_64.c and is_vsmp_box() are depended on CONFIG_X86_64, and we will make sure to fix any deviation from the current situation. We want very much to move to the new hypervisor API, but do not want to loose existing functionality that can hurt all our install base.

Let us go back and see if there is a cleaner way to fix this, while keeping the existing functionality.

Thanks for your understanding,
Shai.


--Shai


> -----Original Message-----
> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, August 22, 2012 20:49
> To: Shai Fultheim ([email protected])
> Cc: Ido Yariv; [email protected]; Thomas Gleixner; H. Peter Anvin
> Subject: Re: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization
>
>
> * Shai Fultheim ([email protected]) <[email protected]> wrote:
>
> > Ingo,
> >
> > CONFIG_X86_VSMP is only for setting cache-line size to 4KB.
> > Rest of the bits were never depended on this.
>
> That's a problem - please move vSMP functionality under a
> simple, properly named Kconfig toggle. Many of the build
> problems are related to that.
>
> Thanks,
>
> Ingo

2012-08-22 18:20:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86/vsmp: Use hypervisor layer for initialization


* Shai Fultheim ([email protected]) <[email protected]> wrote:

> Ingo,
>
> Use of vSMP Foundation does not require compiling with
> CONFIG_X86_VSMP, which is off by default in any distribution
> to date.

That is simply a bug: CONFIG_X86_VSMP should be called
CONFIG_X86_VSMP_ALIGN, and you should introduce a *real*
CONFIG_X86_VSMP toggle to allow people to not build the
vSMP platform specific bits.

Thanks,

Ingo