From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [RFC PATCH] xen, apic: Setup our own APIC criver and validator for APIC IDs. Date: Fri, 09 Jan 2015 18:01:39 -0500 Message-ID: <54B05DD3.4080209@oracle.com> References: <1420841507-5569-1-git-send-email-konrad.wilk@oracle.com> <1420841507-5569-2-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Y9iZ3-0008Of-1h for xen-devel@lists.xenproject.org; Fri, 09 Jan 2015 23:01:57 +0000 In-Reply-To: <1420841507-5569-2-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk , david.vrabel@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org 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 > Signed-off-by: Konrad Rzeszutek Wilk > --- > 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) > {