Date: Fri, 18 May 2018 07:43:45 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: [MODERATED] Re: [PATCH v17.1 2/2] [PATCH v17.1 2/2] SSB Fix #2
Message-ID: <20180518114345.GU18423@char.us.oracle.com>
References: <20180517031815.016936086@localhost.localdomain>
 <alpine.DEB.2.21.1805181103210.2172@nanos.tec.linutronix.de>
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.21.1805181103210.2172@nanos.tec.linutronix.de>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
To: speck@linutronix.de

On Fri, May 18, 2018 at 11:14:51AM +0200, speck for Thomas Gleixner wrote:
> On Wed, 16 May 2018, speck for konrad.wilk_at_oracle.com wrote:
> 
> > xen: Don't call non-Xen routines early in the boot
> > 
> > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") added a call
> > to  get_cpu_cap() to properly set boot_cpu_has(X86_FEATURE_NX) which is
> > needed to correctly initialize __supported_pte_mask. Doing this so early
> > in the boot can be dangerous --- while this currently works, there is no
> > telling what get_cpu_cap() may decide to add in the future. For example,
> > if it decides to do a printk (which is not something impossible to
> > imagine) a PV guest will not be able to cope with it.
> 
> So this is not an issue today and I don't see a reason why you need to add
> open coded hackery.
> 
> > +	/* Work out if we support NX. */
> > +	if ((cpuid_eax(0x80000000) >= 0x80000001) &&
> > +	    (cpuid_edx(0x80000001) & (1U << (X86_FEATURE_NX & 31))))
> > +		__supported_pte_mask |= _PAGE_NX;
> > +	else
> > +		__supported_pte_mask &= ~_PAGE_NX;
> > +
> 
> And of course this open coded hack is not a 1:1 replacement for
> x86_configure_nx(). 
> 
> So can you please explain what kind of problem you are trying to solve?

This SSB patch series makes Xen PV guests crash.

> 
> AFAICT from the changelog it's just the 'someone might add a printk'
> fear. Come on, that's silly. If that happens its going to be caught in next
> early enough to fix it.

That is more of a obfuscation in case you would like this particular patch
go through the not-secure-channel.
> 
> And if it's really an issue, then the proper fix is to have a common
> function in the cpu ident code which handles this and not that open coded
> hackery just because we can.

Which they had in mind except they expected push back from you. I will relay
your OK with that approach.

> 
> Thanks,
> 
> 	tglx
>