linux-kernel - Re: Reserved bits and commit x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach

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: <87fs27xzxz.fsf@jcompost-mobl.amr.corp.intel.com>
Date:   Wed, 18 Oct 2023 16:39:52 -0700
From:   "Compostella, Jeremy" <jeremy.compostella@...el.com>
To:     Adam Dunlap <acdunlap@...gle.com>
Cc:     Ingo Molnar <mingo@...nel.org>, kirill.shutemov@...ux.intel.com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Felix Held <felix-coreboot@...ixheld.de>
Subject: Re: Reserved bits and commit x86/sev-es: Set x86_virt_bits to the
 correct value straight away, instead of a two-phase approach

Adam Dunlap <acdunlap@...gle.com> writes:

> On Wed, Oct 18, 2023 at 3:27 PM Compostella, Jeremy
> <jeremy.compostella@...el.com> wrote:
>> In the light of commit fbf6449f84bf I am wondering what is the right
>> approach to fix the regression for AMD and then fix the MTRR check for
>> Intel. Should we introduce a new cpu_dev callback to read the number
>> of reserved bits and take it into account in get_cpu_address_sizes() ?
>
> I think this approach makes sense. It seems better to have one
> function that simply sets it to the right thing rather than setting
> it to one value and then adjusting it (fbf6449f84bf did that for
> x86_virt_bits, although it caused some other problems). However, I'm
> not sure it would solve the problem your original patch tried to
> fix, since x86_phys_bits would still be set after intel_init, which
> apparently uses the value.

Using cscope, I don't see any evidence of any vendor init code using
`x86_phys_bits'. To my knowledge, they seem to be only setting
x86_phys_bits or adjusting it.


> Would it work to move the call to get_cpu_address_sizes() to nearer
> the start of early_identify_cpu()?  We could also add a cpu_dev
> callback so it doesn't need the 2-phase approach, but this would at
> least bring it back into parity with v6.6-rc6.

Such a change should resolve the issue I reported on this thread. I
can run a quick smoke test later tonight or tomorrow.

> Ex (untested):
>
> ---
>  arch/x86/kernel/cpu/common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index bcd3b2df83bb..cdbe8241e250 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1592,6 +1592,8 @@ static void __init early_identify_cpu(struct
> cpuinfo_x86 *c)
>   if (!have_cpuid_p())
>   identify_cpu_without_cpuid(c);
>
> + get_cpu_address_sizes(c);
> +
>   /* cyrix could have cpuid enabled via c_identify()*/
>   if (have_cpuid_p()) {
>   cpu_detect(c);
> @@ -1612,8 +1614,6 @@ static void __init early_identify_cpu(struct
> cpuinfo_x86 *c)
>   setup_clear_cpu_cap(X86_FEATURE_CPUID);
>   }
>
> - get_cpu_address_sizes(c);
> -
>   setup_force_cpu_cap(X86_FEATURE_ALWAYS);
>
>   cpu_set_bug_bits(c);