From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ak@linux.intel.com>
Received: from mga14.intel.com ([192.55.52.115])
	by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256)
	(Exim 4.80)
	(envelope-from <ak@linux.intel.com>)
	id 1fLqPE-0007ta-5m
	for speck@linutronix.de; Thu, 24 May 2018 15:35:48 +0200
Date: Thu, 24 May 2018 06:35:45 -0700
From: Andi Kleen <ak@linux.intel.com>
Subject: [MODERATED] Re: [PATCH v5 6/8] L1TFv4 4
Message-ID: <20180524133545.GN4486@tassilo.jf.intel.com>
References: <cover.1527111786.git.ak@linux.intel.com>
 <20180523215651.BFF82610ED@crypto-ml.lab.linutronix.de>
 <20180524040438.3u66mkm2skcpg24w@treble>
MIME-Version: 1.0
In-Reply-To: <20180524040438.3u66mkm2skcpg24w@treble>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
To: speck@linutronix.de
List-ID: <speck.linutronix.de>

On Wed, May 23, 2018 at 11:04:38PM -0500, speck for Josh Poimboeuf wrote:
> > index 2c0bd38a44ab..ffb178fe6356 100644
> > --- a/arch/x86/kernel/cpu/cpuid-deps.c
> > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > @@ -118,4 +118,5 @@ void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
> >  void setup_clear_cpu_cap(unsigned int feature)
> >  {
> >  	do_clear_cpu_cap(NULL, feature);
> > +	clear_bit(feature, (unsigned long *)cpu_caps_set);
> 
> The caps code is a jungle, so who knows... but isn't this what
> cpu_caps_cleared is for?  Aren't the cpu_caps_set bits supposed to stay
> set (for some mysterious reason)?

Set beats cleared without this change. I tested it.

> 
> AFAICT, the cpu_caps_cleared bit already gets set in do_clear_cpu_cap()
> -> clear_feature().  So I don't see that clearing the cpu_caps_set bit
> actually accomplishes anything on top of that.

The patch is needed.

> 
> >  }
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 443e31c33fa6..99155e9b28c4 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -785,7 +785,26 @@ static void __init trim_low_memory_range(void)
> >  {
> >  	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
> >  }
> > -	
> > +
> > +static __init void check_maxpa_memory(void)
> > +{
> > +	u64 half_pa;
> > +
> > +	if (!boot_cpu_has(X86_BUG_L1TF))
> > +		return;
> > +
> > +	half_pa = (u64)l1tf_pfn_limit() << PAGE_SHIFT;
> > +
> > +	/*
> > +	 * This is extremely unlikely to happen because almost all systems have far
> > +	 * more MAX_PA/2 than DIMM slots.
> 
> s/DIMM slots/actual RAM/?

It's the same in this case.

> 
> Also is MAX_PA a macro or something else?  I don't see it in the code

It's a logical construct but doesn't correspond to a macro in the code.

-Andi