[RFC PATCH] Boot with more than 255 CPUs in initial domain.

[RFC PATCH] Boot with more than 255 CPUs in initial domain.

[Konrad Rzeszutek Wilk]

Hey,

This is RFC since:
 1) I haven't yet tested it on an x2APIC enabled box.
 2) Not sure if we just want override the 'apic->' as done elsewhere
    instead of adding our own APIC driver.

This fixes the issue of trying to boot Linux under an 8 socket machine
and only seeing an subset of CPUs.

 arch/x86/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 15 deletions(-)

[RFC PATCH] xen, apic: Setup our own APIC criver and validator for APIC IDs.

[Konrad Rzeszutek Wilk]

Via CPUID masking and the different apic-> overrides we
effectively make PV guests only but with the default APIC
driver. That is OK as an PV guest should never access any
APIC registers. However, the APIC is also used to limit the
amount of CPUs if the APIC IDs are incorrect - and since we
mask the x2APIC from the CPUID - any APIC IDs above 0xFF
are deemed incorrect by the default APIC routines.

As such add a new routine to check for APIC ID which will
be only used if the CPUID (native one) tells us the system
is using x2APIC.

This allows us to boot with more than 255 CPUs if running
as initial domain.

Reported-by: Cathy Avery <cathy.avery@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 15 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 6bf3a13..84d979e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -989,27 +989,76 @@ static u32 xen_safe_apic_wait_icr_idle(void)
         return 0;
 }
 
-static void set_xen_basic_apic_ops(void)
+
+static int probe_xen(void)
 {
-	apic->read = xen_apic_read;
-	apic->write = xen_apic_write;
-	apic->icr_read = xen_apic_icr_read;
-	apic->icr_write = xen_apic_icr_write;
-	apic->wait_icr_idle = xen_apic_wait_icr_idle;
-	apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
-	apic->set_apic_id = xen_set_apic_id;
-	apic->get_apic_id = xen_get_apic_id;
+	if (!xen_initial_domain())
+		return 0;
 
-#ifdef CONFIG_SMP
-	apic->send_IPI_allbutself = xen_send_IPI_allbutself;
-	apic->send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
-	apic->send_IPI_mask = xen_send_IPI_mask;
-	apic->send_IPI_all = xen_send_IPI_all;
-	apic->send_IPI_self = xen_send_IPI_self;
+	return 1;
+}
+
+static int id_valid_xen(int apicid)
+{
+	return 1;
+}
+
+static struct apic xen_null_apic = {
+	.name = "Xen",
+	.probe = probe_xen,
+	/* The rest is copied from the default. */
+};
+
+/*
+ * This is needed as in enlighten.c we mask the x2APIC bit because we
+ * do not want PV guests to use anything but the default apic routines
+ * (which have an abstraction layer that we use).
+ *
+ * However the default ->apic_id_valid enforces that the APIC ID MUST
+ * be below 0xFF which is not the case for x2APIC - so we need a way
+ * to allow that to function.
+ */
+static bool __init xen_check_x2apic(void)
+{
+#ifdef CONFIG_X2APIC
+	unsigned int ax, bx, cx, dx;
+
+	ax = 1;
+	cx = 0; /* Don't care about dx, and bx */
+	native_cpuid(&ax, &bx, &cx, &dx);
+	if (cx & (1 << (X86_FEATURE_X2APIC % 32)))
+		return true;
 #endif
+	return false;
 }
 
+void __init set_xen_basic_apic_ops(void)
+{
+	memcpy(&xen_null_apic, apic, sizeof(struct apic));
+	xen_null_apic.probe = probe_xen;
+	xen_null_apic.name = "Xen";
+
+	xen_null_apic.read = xen_apic_read;
+	xen_null_apic.write = xen_apic_write;
+	xen_null_apic.icr_read = xen_apic_icr_read;
+	xen_null_apic.icr_write = xen_apic_icr_write;
+	xen_null_apic.wait_icr_idle = xen_apic_wait_icr_idle;
+	xen_null_apic.safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
+	xen_null_apic.set_apic_id = xen_set_apic_id;
+	xen_null_apic.get_apic_id = xen_get_apic_id;
+
+	xen_null_apic.send_IPI_allbutself = xen_send_IPI_allbutself;
+	xen_null_apic.send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
+	xen_null_apic.send_IPI_mask = xen_send_IPI_mask;
+	xen_null_apic.send_IPI_all = xen_send_IPI_all;
+	xen_null_apic.send_IPI_self = xen_send_IPI_self;
 #endif
+	if (xen_check_x2apic()) {
+		printk(KERN_INFO "Xen: Using x2APIC!\n");
+		xen_null_apic.apic_id_valid = id_valid_xen;
+	}
+}
+apic_driver(xen_null_apic);
 
 static void xen_clts(void)
 {
-- 
2.1.0

Re: [RFC PATCH] xen, apic: Setup our own APIC criver and validator for APIC IDs.

[Boris Ostrovsky]

On 01/09/2015 05:11 PM, Konrad Rzeszutek Wilk wrote:
> Via CPUID masking and the different apic-> overrides we
> effectively make PV guests only but with the default APIC
> driver. That is OK as an PV guest should never access any
> APIC registers. However, the APIC is also used to limit the
> amount of CPUs if the APIC IDs are incorrect - and since we
> mask the x2APIC from the CPUID - any APIC IDs above 0xFF
> are deemed incorrect by the default APIC routines.
>
> As such add a new routine to check for APIC ID which will
> be only used if the CPUID (native one) tells us the system
> is using x2APIC.
>
> This allows us to boot with more than 255 CPUs if running
> as initial domain.
>
> Reported-by: Cathy Avery <cathy.avery@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>   arch/x86/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 64 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 6bf3a13..84d979e 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -989,27 +989,76 @@ static u32 xen_safe_apic_wait_icr_idle(void)
>           return 0;
>   }
>   
> -static void set_xen_basic_apic_ops(void)
> +
> +static int probe_xen(void)
>   {
> -	apic->read = xen_apic_read;
> -	apic->write = xen_apic_write;
> -	apic->icr_read = xen_apic_icr_read;
> -	apic->icr_write = xen_apic_icr_write;
> -	apic->wait_icr_idle = xen_apic_wait_icr_idle;
> -	apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
> -	apic->set_apic_id = xen_set_apic_id;
> -	apic->get_apic_id = xen_get_apic_id;
> +	if (!xen_initial_domain())
> +		return 0;

Why not allow loading this driver for all guests?

>   
> -#ifdef CONFIG_SMP
> -	apic->send_IPI_allbutself = xen_send_IPI_allbutself;
> -	apic->send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
> -	apic->send_IPI_mask = xen_send_IPI_mask;
> -	apic->send_IPI_all = xen_send_IPI_all;
> -	apic->send_IPI_self = xen_send_IPI_self;
> +	return 1;
> +}
> +
> +static int id_valid_xen(int apicid)
> +{
> +	return 1;
> +}
> +
> +static struct apic xen_null_apic = {
> +	.name = "Xen",
> +	.probe = probe_xen,
> +	/* The rest is copied from the default. */
> +};
> +
> +/*
> + * This is needed as in enlighten.c we mask the x2APIC bit because we
> + * do not want PV guests to use anything but the default apic routines
> + * (which have an abstraction layer that we use).
> + *
> + * However the default ->apic_id_valid enforces that the APIC ID MUST
> + * be below 0xFF which is not the case for x2APIC - so we need a way
> + * to allow that to function.
> + */
> +static bool __init xen_check_x2apic(void)
> +{
> +#ifdef CONFIG_X2APIC
> +	unsigned int ax, bx, cx, dx;
> +
> +	ax = 1;
> +	cx = 0; /* Don't care about dx, and bx */
> +	native_cpuid(&ax, &bx, &cx, &dx);
> +	if (cx & (1 << (X86_FEATURE_X2APIC % 32)))
> +		return true;
>   #endif
> +	return false;
>   }
>   
> +void __init set_xen_basic_apic_ops(void)
> +{
> +	memcpy(&xen_null_apic, apic, sizeof(struct apic));
> +	xen_null_apic.probe = probe_xen;
> +	xen_null_apic.name = "Xen";
> +
> +	xen_null_apic.read = xen_apic_read;
> +	xen_null_apic.write = xen_apic_write;
> +	xen_null_apic.icr_read = xen_apic_icr_read;
> +	xen_null_apic.icr_write = xen_apic_icr_write;
> +	xen_null_apic.wait_icr_idle = xen_apic_wait_icr_idle;
> +	xen_null_apic.safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
> +	xen_null_apic.set_apic_id = xen_set_apic_id;
> +	xen_null_apic.get_apic_id = xen_get_apic_id;
> +
> +	xen_null_apic.send_IPI_allbutself = xen_send_IPI_allbutself;
> +	xen_null_apic.send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
> +	xen_null_apic.send_IPI_mask = xen_send_IPI_mask;
> +	xen_null_apic.send_IPI_all = xen_send_IPI_all;
> +	xen_null_apic.send_IPI_self = xen_send_IPI_self;
>   #endif
> +	if (xen_check_x2apic()) {
> +		printk(KERN_INFO "Xen: Using x2APIC!\n");

We are not really using x2APIC, we are just allowing large APIC IDs.

> +		xen_null_apic.apic_id_valid = id_valid_xen;

Can we use id_valid_xen() for both APIC flavors and check for x2APIC 
presence there?

> +	}
> +}
> +apic_driver(xen_null_apic);

This (and all xen_apic_* routine) should then probably go into a 
separate file.

-boris

>   
>   static void xen_clts(void)
>   {