Messages in this thread | | | From | "Compostella, Jeremy" <> | 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 | Date | Wed, 18 Oct 2023 16:39:52 -0700 |
| |
Adam Dunlap <acdunlap@google.com> writes:
> On Wed, Oct 18, 2023 at 3:27 PM Compostella, Jeremy > <jeremy.compostella@intel.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);
| |