Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751256AbeAPIuU (ORCPT + 1 other); Tue, 16 Jan 2018 03:50:20 -0500 Received: from mail-oi0-f68.google.com ([209.85.218.68]:38188 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750790AbeAPIuS (ORCPT ); Tue, 16 Jan 2018 03:50:18 -0500 X-Google-Smtp-Source: ACJfBostVuUixVn3p26VkE1Hach7ChH2QN87O6CVCiZ4IXb+t3h3CAlGAUNAjpHIQ4PM4u7Yu5tn1kXDLWtFJ+eP9rA= MIME-Version: 1.0 In-Reply-To: <86917190-5499-3fb8-7b5c-736dc2daeca5@cn.fujitsu.com> References: <20180115202335.1645883-1-arnd@arndb.de> <4284ee79-a87c-a47d-7e96-682bdd8754e1@cn.fujitsu.com> <86917190-5499-3fb8-7b5c-736dc2daeca5@cn.fujitsu.com> From: Arnd Bergmann Date: Tue, 16 Jan 2018 09:50:16 +0100 X-Google-Sender-Auth: 8RbcLGHhBYGURCjYGHL92ZphpRU Message-ID: Subject: Re: [PATCH] x86/jailhouse: fix building without X86_X2APIC To: Dou Liyang Cc: Jan Kiszka , Thomas Gleixner , "the arch/x86 maintainers" , jailhouse-dev@googlegroups.com, Ingo Molnar , "H. Peter Anvin" , Juergen Gross , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 16, 2018 at 9:17 AM, Dou Liyang wrote: > Hi Arnd, > > [...] >>> >>> The reason I don't want to expose the x2apic_mode and x2apic_phys is >>> that they may be misused in X2APIC=n case. So I create an interface to >>> wrap it. do you think so? ;-) >> >> >> I'm not sure I follow what the intention of that is. If you want to hide > > > My purpose of that is hiding the variables in X2APIC=n case. But why? I'd say either hide them all the time, or don't hide them at all. >> I see nothing wrong it with this, but also don't see anything it does >> that improves the interface. >> > > Another way we can choice is wrap the code with "CONFIG_X86_X2APIC". > But why? That just makes perfectly reasonably code uglier. Generally speaking, compiler conditionals are better than preprocessor conditionals for this, as they are easier to read and provide better compile-time coverage when things go wrong, such as the missing declaration. Arnd > --------------------------------8<------------------------------------ > diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c > index d6d5976a9b51..d4aee43c8912 100644 > --- a/arch/x86/kernel/jailhouse.c > +++ b/arch/x86/kernel/jailhouse.c > @@ -65,6 +65,7 @@ static void __init jailhouse_get_smp_config(unsigned int > early) > }; > unsigned int cpu; > > +#ifdef CONFIG_X86_X2APIC > if (x2apic_enabled()) { > /* > * We do not have access to IR inside Jailhouse non-root > cells. > @@ -79,6 +80,7 @@ static void __init jailhouse_get_smp_config(unsigned int > early) > */ > default_acpi_madt_oem_check("", ""); > } > +#endif