linux-kernel - Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests

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: <20140915192836.GE3683@laptop.dumpdata.com>
Date:	Mon, 15 Sep 2014 15:28:36 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	David Vrabel <david.vrabel@...rix.com>, roger.pau@...rix.com
Cc:	Mukesh Rathor <mukesh.rathor@...cle.com>,
	xen-devel@...ts.xenproject.org, boris.ostrovsky@...cle.com,
	linux-kernel@...r.kernel.org
Subject: Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in
 PVH guests

On Mon, Sep 15, 2014 at 03:45:53PM +0100, David Vrabel wrote:
> On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
> > On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
> >> 
> >> @@ -413,15 +417,18 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
> >>  					(unsigned long)xen_failsafe_callback;
> >>  		ctxt->user_regs.cs = __KERNEL_CS;
> >>  		per_cpu(xen_cr3, cpu) = __pa(swapper_pg_dir);
> >> -#ifdef CONFIG_X86_32
> >>  	}
> >> -#else
> >> -	} else
> >> -		/* N.B. The user_regs.eip (cpu_bringup_and_idle) is called with
> >> -		 * %rdi having the cpu number - which means are passing in
> >> -		 * as the first parameter the cpu. Subtle!
> >> +#ifdef CONFIG_XEN_PVH
> >> +	else {
> >> +		/*
> >> +		 * The vcpu comes on kernel page tables which have the NX pte
> >> +		 * bit set. This means before DS/SS is touched, NX in
> >> +		 * EFER must be set. Hence the following assembly glue code.
> > 
> > And you ripped out the nice 'N.B' comment I added. Sad :-(
> 
> I think I removed that.
> 
> I don't think passing parameters to a function is particularly subtle
> and this comment is largely superseded by the comment for
> xen_pvh_early_cpu_init() itself.

Good point.
> 
> >> +#ifdef CONFIG_XEN_PVH
> >> +/*
> >> + * xen_pvh_early_cpu_init() - early PVH VCPU initialization
> >> + * @cpu: this cpu number (%rdi)
> >> + * @flag: boolean flag true to indicate this is a secondary vcpu coming up
> >> + *        on this entry point or the primary cpu coming back online.
> > 
> > Why do we do this? Why not just piggyback on the first parameter - the 'cpu'?
> > 
> > If it is zero it is boot CPU.
> 
> "Changes in v5 (Mukesh):
>   - Jan reminded us that vcpu 0 could go offline/online. So, add flag back"

Right, totally forgot about that.

With that I think I think you can tack on 'Reviewed-by: Konrad Rzeszutek
Wilk <konrad.wilk@...cle.com>'.

Granted the PVH ABI doc needs to go in Xen base first as David requested.
Even a draft that explains some of this would be good. 

CC-ing Roger - perhaps v1 of the draft could be posted and we can flesh it
out more?
> 
> David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/