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

[PATCH] xen,apic: Setup our own APIC driver 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.

The probing of APIC drivers is dependent on the build. The
arch/x86/kernel/apic/Makefile lists them as (assuming 64-bit):
 apic_numachip.o
 x2apic_uv_x.o
 x2apic_phys.o
 x2apic_cluster.o
 apic_flat_64.o

Looking at .apicdrivers section I see:
xen_apic, apic_x2apic_phys, apic_x2apic_cluster, apic_physflatapic_flat
addresses.  Since we build from arch/x86/xen which we can before or
after x86/kernel/apic is built. As such we add in an late probe
function to change to the Xen PV if it hand't been done during bootup.

Reported-by: Cathy Avery <cathy.avery@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/apic.c      | 169 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/xen/enlighten.c |  90 +------------------------
 2 files changed, 170 insertions(+), 89 deletions(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index 7005ced..9b9a5fc 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -7,6 +7,7 @@
 #include <xen/xen.h>
 #include <xen/interface/physdev.h>
 #include "xen-ops.h"
+#include "smp.h"
 
 static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
 {
@@ -28,7 +29,175 @@ static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
 	return 0xfd;
 }
 
+static unsigned long xen_set_apic_id(unsigned int x)
+{
+	WARN_ON(1);
+	return x;
+}
+
+static unsigned int xen_get_apic_id(unsigned long x)
+{
+	return ((x)>>24) & 0xFFu;
+}
+
+static u32 xen_apic_read(u32 reg)
+{
+	struct xen_platform_op op = {
+		.cmd = XENPF_get_cpuinfo,
+		.interface_version = XENPF_INTERFACE_VERSION,
+		.u.pcpu_info.xen_cpuid = 0,
+	};
+	int ret = 0;
+
+	/* Shouldn't need this as APIC is turned off for PV, and we only
+	 * get called on the bootup processor. But just in case. */
+	if (!xen_initial_domain() || smp_processor_id())
+		return 0;
+
+	if (reg == APIC_LVR)
+		return 0x10;
+
+	if (reg != APIC_ID)
+		return 0;
+
+	ret = HYPERVISOR_dom0_op(&op);
+	if (ret)
+		return 0;
+
+	return op.u.pcpu_info.apic_id << 24;
+}
+
+static void xen_apic_write(u32 reg, u32 val)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static u64 xen_apic_icr_read(void)
+{
+	return 0;
+}
+
+static void xen_apic_icr_write(u32 low, u32 id)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static u32 xen_safe_apic_wait_icr_idle(void)
+{
+        return 0;
+}
+
+
+static int probe_xen(void)
+{
+	if (xen_pv_domain())
+		return 1;
+
+	return 0;
+}
+
+static int xen_madt_oem_check(char *oem_id, char *oem_table_id)
+{
+	return 1;
+}
+
+static int xen_id_always_valid(int apicid)
+{
+	return 1;
+}
+
+static int xen_id_always_registered(void)
+{
+	return 1;
+}
+
+static int xen_phys_pkg_id(int initial_apic_id, int index_msb)
+{
+	return initial_apic_id >> index_msb;
+}
+
+static void xen_noop(void)
+{
+}
+
+static void xen_silent_inquire(int apicid)
+{
+}
+
+static struct apic xen_apic = {
+	.name 				= "Xen PV",
+	.probe 				= probe_xen,
+	.acpi_madt_oem_check		= xen_madt_oem_check,
+	.apic_id_valid 			= xen_id_always_valid,
+	.apic_id_registered 		= xen_id_always_registered,
+
+	/* .irq_delivery_mode - used in native_compose_msi_msg only */
+	/* .irq_dest_mode     - used in native_compose_msi_msg only */
+
+	.target_cpus			= default_target_cpus,
+	.disable_esr			= 0,
+	/* .dest_logical      -  default_send_IPI_ use it but we use our own. */
+	.check_apicid_used		= default_check_apicid_used, /* Used on 32-bit */
+
+	.vector_allocation_domain	= flat_vector_allocation_domain,
+	.init_apic_ldr			= xen_noop, /* setup_local_APIC calls it */
+
+	.ioapic_phys_id_map		= default_ioapic_phys_id_map, /* Used on 32-bit */
+	.setup_apic_routing		= NULL,
+	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid, /* Used on 32-bit */
+	.check_phys_apicid_present	= default_check_phys_apicid_present, /* smp_sanity_check needs it */
+	.phys_pkg_id			= xen_phys_pkg_id, /* detect_ht */
+
+	.get_apic_id 			= xen_get_apic_id,
+	.set_apic_id 			= xen_set_apic_id, /* Can be NULL on 32-bit. */
+	.apic_id_mask			= 0xFF << 24, /* Used by verify_local_APIC. Match with what xen_get_apic_id does. */
+
+	.cpu_mask_to_apicid_and		= flat_cpu_mask_to_apicid_and,
+
+	.send_IPI_mask 			= xen_send_IPI_mask,
+	.send_IPI_mask_allbutself 	= xen_send_IPI_mask_allbutself,
+	.send_IPI_allbutself 		= xen_send_IPI_allbutself,
+	.send_IPI_all 			= xen_send_IPI_all,
+	.send_IPI_self 			= xen_send_IPI_self,
+
+	/* .wait_for_init_deassert- used  by AP bootup - smp_callin which we don't use */
+	.inquire_remote_apic		= xen_silent_inquire,
+
+	.read				= xen_apic_read,
+	.write				= xen_apic_write,
+	.eoi_write			= xen_apic_write,
+
+	.icr_read 			= xen_apic_icr_read,
+	.icr_write 			= xen_apic_icr_write,
+	.wait_icr_idle 			= xen_noop,
+	.safe_wait_icr_idle 		= xen_safe_apic_wait_icr_idle,
+
+#ifdef CONFIG_X86_32
+	/* generic_processor_info */
+	.x86_32_early_logical_apicid	= default_x86_32_early_logical_apicid,
+#endif
+};
+
+void __init xen_apic_check(void)
+{
+	if (apic == &xen_apic)
+		return;
+
+	pr_info("Switched APIC routing from %s to %s.\n", apic->name,
+		xen_apic.name);
+	apic = &xen_apic;
+}
 void __init xen_init_apic(void)
 {
 	x86_io_apic_ops.read = xen_io_apic_read;
+	/* On PV guests the APIC CPUID bit is disabled so none of the
+	 * routines end up executing. */
+	if (!xen_initial_domain())
+		apic = &xen_apic;
+
+	x86_platform.apic_post_init = xen_apic_check;
 }
+apic_driver(xen_apic);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 78a881b..6c13a45 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -927,92 +927,6 @@ static void xen_io_delay(void)
 {
 }
 
-#ifdef CONFIG_X86_LOCAL_APIC
-static unsigned long xen_set_apic_id(unsigned int x)
-{
-	WARN_ON(1);
-	return x;
-}
-static unsigned int xen_get_apic_id(unsigned long x)
-{
-	return ((x)>>24) & 0xFFu;
-}
-static u32 xen_apic_read(u32 reg)
-{
-	struct xen_platform_op op = {
-		.cmd = XENPF_get_cpuinfo,
-		.interface_version = XENPF_INTERFACE_VERSION,
-		.u.pcpu_info.xen_cpuid = 0,
-	};
-	int ret = 0;
-
-	/* Shouldn't need this as APIC is turned off for PV, and we only
-	 * get called on the bootup processor. But just in case. */
-	if (!xen_initial_domain() || smp_processor_id())
-		return 0;
-
-	if (reg == APIC_LVR)
-		return 0x10;
-
-	if (reg != APIC_ID)
-		return 0;
-
-	ret = HYPERVISOR_dom0_op(&op);
-	if (ret)
-		return 0;
-
-	return op.u.pcpu_info.apic_id << 24;
-}
-
-static void xen_apic_write(u32 reg, u32 val)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static u64 xen_apic_icr_read(void)
-{
-	return 0;
-}
-
-static void xen_apic_icr_write(u32 low, u32 id)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static void xen_apic_wait_icr_idle(void)
-{
-        return;
-}
-
-static u32 xen_safe_apic_wait_icr_idle(void)
-{
-        return 0;
-}
-
-static void set_xen_basic_apic_ops(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;
-
-#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;
-#endif
-}
-
-#endif
-
 static void xen_clts(void)
 {
 	struct multicall_space mcs;
@@ -1601,7 +1515,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	/*
 	 * set up the basic apic ops.
 	 */
-	set_xen_basic_apic_ops();
+	xen_init_apic();
 #endif
 
 	if (xen_feature(XENFEAT_mmu_pt_update_preserve_ad)) {
@@ -1714,8 +1628,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 		if (HYPERVISOR_dom0_op(&op) == 0)
 			boot_params.kbd_status = op.u.firmware_info.u.kbd_shift_flags;
 
-		xen_init_apic();
-
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
 
-- 
2.1.0

Re: [Xen-devel] [PATCH] xen, apic: Setup our own APIC driver and validator for APIC IDs.

[David Vrabel]

On 27/02/15 21:14, 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.

I was applying this but it breaks the build.

arch/x86/built-in.o:(.data+0x2a28): undefined reference to
`xen_send_IPI_mask'
arch/x86/built-in.o:(.data+0x2a30): undefined reference to
`xen_send_IPI_mask_allbutself'
arch/x86/built-in.o:(.data+0x2a38): undefined reference to
`xen_send_IPI_allbutself'
arch/x86/built-in.o:(.data+0x2a40): undefined reference to
`xen_send_IPI_all'
arch/x86/built-in.o:(.data+0x2a48): undefined reference to
`xen_send_IPI_self'

There are some minor things that I was going to fix up (see below).

I also found the commit message a bit garbled so rewrote it to:

    x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs

    Instead of mangling the default APIC driver, provide a Xen PV guest
    specific one that explicitly provides appropriate methods.

    This allows use to report that all APIC IDs are valid, allowing dom0
    to boot with more than 255 VCPUs.

    Since the probe order of APIC drivers is link dependent, we add in an
    late probe function to change to the Xen PV if it hadn't been done
    during bootup.


> +static u32 xen_safe_apic_wait_icr_idle(void)
> +{
> +        return 0;
> +}
> +
> +

Extra blank line.

> +static int probe_xen(void)

xen_apic_probe_pv

> +static struct apic xen_apic = {

static struct apic xen_pv_apic

> +void __init xen_apic_check(void)

static

David

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

[David Vrabel]

On 27/02/15 21:14, 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.

I was applying this but it breaks the build.

arch/x86/built-in.o:(.data+0x2a28): undefined reference to
`xen_send_IPI_mask'
arch/x86/built-in.o:(.data+0x2a30): undefined reference to
`xen_send_IPI_mask_allbutself'
arch/x86/built-in.o:(.data+0x2a38): undefined reference to
`xen_send_IPI_allbutself'
arch/x86/built-in.o:(.data+0x2a40): undefined reference to
`xen_send_IPI_all'
arch/x86/built-in.o:(.data+0x2a48): undefined reference to
`xen_send_IPI_self'

There are some minor things that I was going to fix up (see below).

I also found the commit message a bit garbled so rewrote it to:

    x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs

    Instead of mangling the default APIC driver, provide a Xen PV guest
    specific one that explicitly provides appropriate methods.

    This allows use to report that all APIC IDs are valid, allowing dom0
    to boot with more than 255 VCPUs.

    Since the probe order of APIC drivers is link dependent, we add in an
    late probe function to change to the Xen PV if it hadn't been done
    during bootup.


> +static u32 xen_safe_apic_wait_icr_idle(void)
> +{
> +        return 0;
> +}
> +
> +

Extra blank line.

> +static int probe_xen(void)

xen_apic_probe_pv

> +static struct apic xen_apic = {

static struct apic xen_pv_apic

> +void __init xen_apic_check(void)

static

David

Re: [Xen-devel] [PATCH] xen, apic: Setup our own APIC driver and validator for APIC IDs.

[Konrad Rzeszutek Wilk]

On Mon, Mar 02, 2015 at 11:24:04AM +0000, David Vrabel wrote:
> On 27/02/15 21:14, 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.
> 
> I was applying this but it breaks the build.

Could you send me your .config please.
> 
> arch/x86/built-in.o:(.data+0x2a28): undefined reference to
> `xen_send_IPI_mask'
> arch/x86/built-in.o:(.data+0x2a30): undefined reference to
> `xen_send_IPI_mask_allbutself'
> arch/x86/built-in.o:(.data+0x2a38): undefined reference to
> `xen_send_IPI_allbutself'
> arch/x86/built-in.o:(.data+0x2a40): undefined reference to
> `xen_send_IPI_all'
> arch/x86/built-in.o:(.data+0x2a48): undefined reference to
> `xen_send_IPI_self'
> 
> There are some minor things that I was going to fix up (see below).
> 
> I also found the commit message a bit garbled so rewrote it to:
> 
>     x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs
> 
>     Instead of mangling the default APIC driver, provide a Xen PV guest
>     specific one that explicitly provides appropriate methods.
> 
>     This allows use to report that all APIC IDs are valid, allowing dom0
>     to boot with more than 255 VCPUs.
> 
>     Since the probe order of APIC drivers is link dependent, we add in an
>     late probe function to change to the Xen PV if it hadn't been done
>     during bootup.
> 
> 
> > +static u32 xen_safe_apic_wait_icr_idle(void)
> > +{
> > +        return 0;
> > +}
> > +
> > +
> 
> Extra blank line.
> 
> > +static int probe_xen(void)
> 
> xen_apic_probe_pv
> 
> > +static struct apic xen_apic = {
> 
> static struct apic xen_pv_apic
> 
> > +void __init xen_apic_check(void)
> 
> static
> 
> David

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

[Konrad Rzeszutek Wilk]

On Mon, Mar 02, 2015 at 11:24:04AM +0000, David Vrabel wrote:
> On 27/02/15 21:14, 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.
> 
> I was applying this but it breaks the build.

Could you send me your .config please.
> 
> arch/x86/built-in.o:(.data+0x2a28): undefined reference to
> `xen_send_IPI_mask'
> arch/x86/built-in.o:(.data+0x2a30): undefined reference to
> `xen_send_IPI_mask_allbutself'
> arch/x86/built-in.o:(.data+0x2a38): undefined reference to
> `xen_send_IPI_allbutself'
> arch/x86/built-in.o:(.data+0x2a40): undefined reference to
> `xen_send_IPI_all'
> arch/x86/built-in.o:(.data+0x2a48): undefined reference to
> `xen_send_IPI_self'
> 
> There are some minor things that I was going to fix up (see below).
> 
> I also found the commit message a bit garbled so rewrote it to:
> 
>     x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs
> 
>     Instead of mangling the default APIC driver, provide a Xen PV guest
>     specific one that explicitly provides appropriate methods.
> 
>     This allows use to report that all APIC IDs are valid, allowing dom0
>     to boot with more than 255 VCPUs.
> 
>     Since the probe order of APIC drivers is link dependent, we add in an
>     late probe function to change to the Xen PV if it hadn't been done
>     during bootup.
> 
> 
> > +static u32 xen_safe_apic_wait_icr_idle(void)
> > +{
> > +        return 0;
> > +}
> > +
> > +
> 
> Extra blank line.
> 
> > +static int probe_xen(void)
> 
> xen_apic_probe_pv
> 
> > +static struct apic xen_apic = {
> 
> static struct apic xen_pv_apic
> 
> > +void __init xen_apic_check(void)
> 
> static
> 
> David

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

[Konrad Rzeszutek Wilk]

On Mon, Mar 02, 2015 at 02:40:06PM +0000, David Vrabel wrote:
> On 02/03/15 14:22, Konrad Rzeszutek Wilk wrote:
> > On Mon, Mar 02, 2015 at 11:24:04AM +0000, David Vrabel wrote:
> >> On 27/02/15 21:14, 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.
> >>
> >> I was applying this but it breaks the build.
> > 
> > Could you send me your .config please.
> 
> Attached.

Pls see:

>From 0dd50e87b434462db13c6515c5cf34be68475ef3 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 9 Jan 2015 17:55:52 -0500
Subject: [PATCH] x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs

Instead of mangling the default APIC driver, provide a Xen PV guest
specific one that explicitly provides appropriate methods.

This allows use to report that all APIC IDs are valid, allowing dom0
to boot with more than 255 VCPUs.

Since the probe order of APIC drivers is link dependent, we add in an
late probe function to change to the Xen PV if it hadn't been done
during bootup.

Suggested-by: David Vrabel <david.vrabel@citrix.com>
Reported-by: Cathy Avery <cathy.avery@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/apic.c      | 169 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/xen/enlighten.c |  90 +------------------------
 2 files changed, 170 insertions(+), 89 deletions(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index 7005ced..86bea3e 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -7,6 +7,7 @@
 #include <xen/xen.h>
 #include <xen/interface/physdev.h>
 #include "xen-ops.h"
+#include "smp.h"
 
 static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
 {
@@ -28,7 +29,175 @@ static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
 	return 0xfd;
 }
 
+static unsigned long xen_set_apic_id(unsigned int x)
+{
+	WARN_ON(1);
+	return x;
+}
+
+static unsigned int xen_get_apic_id(unsigned long x)
+{
+	return ((x)>>24) & 0xFFu;
+}
+
+static u32 xen_apic_read(u32 reg)
+{
+	struct xen_platform_op op = {
+		.cmd = XENPF_get_cpuinfo,
+		.interface_version = XENPF_INTERFACE_VERSION,
+		.u.pcpu_info.xen_cpuid = 0,
+	};
+	int ret = 0;
+
+	/* Shouldn't need this as APIC is turned off for PV, and we only
+	 * get called on the bootup processor. But just in case. */
+	if (!xen_initial_domain() || smp_processor_id())
+		return 0;
+
+	if (reg == APIC_LVR)
+		return 0x10;
+
+	if (reg != APIC_ID)
+		return 0;
+
+	ret = HYPERVISOR_dom0_op(&op);
+	if (ret)
+		return 0;
+
+	return op.u.pcpu_info.apic_id << 24;
+}
+
+static void xen_apic_write(u32 reg, u32 val)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static u64 xen_apic_icr_read(void)
+{
+	return 0;
+}
+
+static void xen_apic_icr_write(u32 low, u32 id)
+{
+	/* Warn to see if there's any stray references */
+	WARN_ON(1);
+}
+
+static u32 xen_safe_apic_wait_icr_idle(void)
+{
+        return 0;
+}
+
+static int xen_apic_probe_pv(void)
+{
+	if (xen_pv_domain())
+		return 1;
+
+	return 0;
+}
+
+static int xen_madt_oem_check(char *oem_id, char *oem_table_id)
+{
+	return 1;
+}
+
+static int xen_id_always_valid(int apicid)
+{
+	return 1;
+}
+
+static int xen_id_always_registered(void)
+{
+	return 1;
+}
+
+static int xen_phys_pkg_id(int initial_apic_id, int index_msb)
+{
+	return initial_apic_id >> index_msb;
+}
+
+static void xen_noop(void)
+{
+}
+
+static void xen_silent_inquire(int apicid)
+{
+}
+
+static struct apic xen_pv_apic = {
+	.name 				= "Xen PV",
+	.probe 				= xen_apic_probe_pv,
+	.acpi_madt_oem_check		= xen_madt_oem_check,
+	.apic_id_valid 			= xen_id_always_valid,
+	.apic_id_registered 		= xen_id_always_registered,
+
+	/* .irq_delivery_mode - used in native_compose_msi_msg only */
+	/* .irq_dest_mode     - used in native_compose_msi_msg only */
+
+	.target_cpus			= default_target_cpus,
+	.disable_esr			= 0,
+	/* .dest_logical      -  default_send_IPI_ use it but we use our own. */
+	.check_apicid_used		= default_check_apicid_used, /* Used on 32-bit */
+
+	.vector_allocation_domain	= flat_vector_allocation_domain,
+	.init_apic_ldr			= xen_noop, /* setup_local_APIC calls it */
+
+	.ioapic_phys_id_map		= default_ioapic_phys_id_map, /* Used on 32-bit */
+	.setup_apic_routing		= NULL,
+	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid, /* Used on 32-bit */
+	.check_phys_apicid_present	= default_check_phys_apicid_present, /* smp_sanity_check needs it */
+	.phys_pkg_id			= xen_phys_pkg_id, /* detect_ht */
+
+	.get_apic_id 			= xen_get_apic_id,
+	.set_apic_id 			= xen_set_apic_id, /* Can be NULL on 32-bit. */
+	.apic_id_mask			= 0xFF << 24, /* Used by verify_local_APIC. Match with what xen_get_apic_id does. */
+
+	.cpu_mask_to_apicid_and		= flat_cpu_mask_to_apicid_and,
+
+#ifdef CONFIG_SMP
+	.send_IPI_mask 			= xen_send_IPI_mask,
+	.send_IPI_mask_allbutself 	= xen_send_IPI_mask_allbutself,
+	.send_IPI_allbutself 		= xen_send_IPI_allbutself,
+	.send_IPI_all 			= xen_send_IPI_all,
+	.send_IPI_self 			= xen_send_IPI_self,
+#endif
+	/* .wait_for_init_deassert- used  by AP bootup - smp_callin which we don't use */
+	.inquire_remote_apic		= xen_silent_inquire,
+
+	.read				= xen_apic_read,
+	.write				= xen_apic_write,
+	.eoi_write			= xen_apic_write,
+
+	.icr_read 			= xen_apic_icr_read,
+	.icr_write 			= xen_apic_icr_write,
+	.wait_icr_idle 			= xen_noop,
+	.safe_wait_icr_idle 		= xen_safe_apic_wait_icr_idle,
+
+#ifdef CONFIG_X86_32
+	/* generic_processor_info */
+	.x86_32_early_logical_apicid	= default_x86_32_early_logical_apicid,
+#endif
+};
+
+static void __init xen_apic_check(void)
+{
+	if (apic == &xen_pv_apic)
+		return;
+
+	pr_info("Switched APIC routing from %s to %s.\n", apic->name,
+		xen_pv_apic.name);
+	apic = &xen_pv_apic;
+}
 void __init xen_init_apic(void)
 {
 	x86_io_apic_ops.read = xen_io_apic_read;
+	/* On PV guests the APIC CPUID bit is disabled so none of the
+	 * routines end up executing. */
+	if (!xen_initial_domain())
+		apic = &xen_pv_apic;
+
+	x86_platform.apic_post_init = xen_apic_check;
 }
+apic_driver(xen_pv_apic);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 78a881b..6c13a45 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -927,92 +927,6 @@ static void xen_io_delay(void)
 {
 }
 
-#ifdef CONFIG_X86_LOCAL_APIC
-static unsigned long xen_set_apic_id(unsigned int x)
-{
-	WARN_ON(1);
-	return x;
-}
-static unsigned int xen_get_apic_id(unsigned long x)
-{
-	return ((x)>>24) & 0xFFu;
-}
-static u32 xen_apic_read(u32 reg)
-{
-	struct xen_platform_op op = {
-		.cmd = XENPF_get_cpuinfo,
-		.interface_version = XENPF_INTERFACE_VERSION,
-		.u.pcpu_info.xen_cpuid = 0,
-	};
-	int ret = 0;
-
-	/* Shouldn't need this as APIC is turned off for PV, and we only
-	 * get called on the bootup processor. But just in case. */
-	if (!xen_initial_domain() || smp_processor_id())
-		return 0;
-
-	if (reg == APIC_LVR)
-		return 0x10;
-
-	if (reg != APIC_ID)
-		return 0;
-
-	ret = HYPERVISOR_dom0_op(&op);
-	if (ret)
-		return 0;
-
-	return op.u.pcpu_info.apic_id << 24;
-}
-
-static void xen_apic_write(u32 reg, u32 val)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static u64 xen_apic_icr_read(void)
-{
-	return 0;
-}
-
-static void xen_apic_icr_write(u32 low, u32 id)
-{
-	/* Warn to see if there's any stray references */
-	WARN_ON(1);
-}
-
-static void xen_apic_wait_icr_idle(void)
-{
-        return;
-}
-
-static u32 xen_safe_apic_wait_icr_idle(void)
-{
-        return 0;
-}
-
-static void set_xen_basic_apic_ops(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;
-
-#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;
-#endif
-}
-
-#endif
-
 static void xen_clts(void)
 {
 	struct multicall_space mcs;
@@ -1601,7 +1515,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	/*
 	 * set up the basic apic ops.
 	 */
-	set_xen_basic_apic_ops();
+	xen_init_apic();
 #endif
 
 	if (xen_feature(XENFEAT_mmu_pt_update_preserve_ad)) {
@@ -1714,8 +1628,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 		if (HYPERVISOR_dom0_op(&op) == 0)
 			boot_params.kbd_status = op.u.firmware_info.u.kbd_shift_flags;
 
-		xen_init_apic();
-
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
 
-- 
2.1.0

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

[David Vrabel]

On 02/03/15 17:06, Konrad Rzeszutek Wilk wrote:
> Subject: [PATCH] x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs
> 
> Instead of mangling the default APIC driver, provide a Xen PV guest
> specific one that explicitly provides appropriate methods.
> 
> This allows use to report that all APIC IDs are valid, allowing dom0
> to boot with more than 255 VCPUs.
> 
> Since the probe order of APIC drivers is link dependent, we add in an
> late probe function to change to the Xen PV if it hadn't been done
> during bootup.

Applied to devel/for-linus-4.1, thanks.

I've only given the briefest of smoke-tests.  It would probably be a
good idea to queue this branch for some testing early.

David

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

[Konrad Rzeszutek Wilk]

On Mon, Mar 02, 2015 at 05:25:55PM +0000, David Vrabel wrote:
> On 02/03/15 17:06, Konrad Rzeszutek Wilk wrote:
> > Subject: [PATCH] x86/xen: Provide a "Xen PV" APIC driver to support >255 VCPUs
> > 
> > Instead of mangling the default APIC driver, provide a Xen PV guest
> > specific one that explicitly provides appropriate methods.
> > 
> > This allows use to report that all APIC IDs are valid, allowing dom0
> > to boot with more than 255 VCPUs.
> > 
> > Since the probe order of APIC drivers is link dependent, we add in an
> > late probe function to change to the Xen PV if it hadn't been done
> > during bootup.
> 
> Applied to devel/for-linus-4.1, thanks.

Thank you.
> 
> I've only given the briefest of smoke-tests.  It would probably be a
> good idea to queue this branch for some testing early.

<nods>

I did the 'perf' test which does use the IPI in an PV guest and
in dom0 and it worked. And naturally bootup on an machine with an unholy
amount of CPUs (x2APIC) - and also smaller ones.

And then the smoke-test that Boris does as well (on a sister lab setup).

But it would be good to make sure this patch does not blow anything up.
> 
> David

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

[Boris Ostrovsky]

On 03/02/2015 12:32 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 02, 2015 at 05:25:55PM +0000, David Vrabel wrote:
>
>> I've only given the briefest of smoke-tests.  It would probably be a
>> good idea to queue this branch for some testing early.
> <nods>
>
> I did the 'perf' test which does use the IPI in an PV guest and
> in dom0 and it worked. And naturally bootup on an machine with an unholy
> amount of CPUs (x2APIC) - and also smaller ones.
>
> And then the smoke-test that Boris does as well (on a sister lab setup).
>
> But it would be good to make sure this patch does not blow anything up.

I will queue it for overnight runs once it shows up in the tree.


-boris