linux-kernel - Re: [PATCH] x86/jailhouse: fix building without X86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a23RAFyRUwM-L8d3oTDhFdqZb9qobKd+ecu8L0uJgTsWQ@mail.gmail.com>
Date:   Tue, 16 Jan 2018 09:50:16 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Dou Liyang <douly.fnst@...fujitsu.com>
Cc:     Jan Kiszka <jan.kiszka@...mens.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        jailhouse-dev@...glegroups.com, Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Juergen Gross <jgross@...e.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/jailhouse: fix building without X86_X2APIC

On Tue, Jan 16, 2018 at 9:17 AM, Dou Liyang <douly.fnst@...fujitsu.com> 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