[PATCH V0] linux PVH: Set CR4 flags

[PATCH V0] linux PVH: Set CR4 flags

[Mukesh Rathor]

Konrad,

The CR4 settings were dropped from my earlier patch because you didn't
wanna enable them. But since you do now, we need to set them in the APs
also. If you decide not too again, please apply my prev patch
"pvh: disable pse feature for now".

thanks
Mukesh

[PATCH] pvh: set cr4 flags for APs

[Mukesh Rathor]

We need to set cr4 flags for APs that are already set for BSP.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 arch/x86/xen/enlighten.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a4d7b64..201d09a 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
 	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM guests
 	 * (which PVH shared codepaths), while X86_CR0_PG is for PVH. */
 	write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM);
+
+	if (!cpu)
+		return;
+	/*
+	 * For BSP, PSE PGE are set in probe_page_size_mask(), for APs
+	 * set them here. For all, OSFXSR OSXMMEXCPT are set in fpu_init.
+	*/
+	if (cpu_has_pse)
+		set_in_cr4(X86_CR4_PSE);
+
+	if (cpu_has_pge)
+		set_in_cr4(X86_CR4_PGE);
 }
 
 /*
-- 
1.7.2.3

Re: [PATCH] pvh: set cr4 flags for APs

[Konrad Rzeszutek Wilk]

On Wed, Jan 29, 2014 at 04:15:18PM -0800, Mukesh Rathor wrote:
> We need to set cr4 flags for APs that are already set for BSP.

The title is missing the 'xen' part.

I rewrote it a bit and I think this should go in 3.14.

David, Boris: It is not the full fix as there are other parts to
make an PVH guest use 2MB or 1GB pages- but this fixes an obvious
bug.



>From 797ea6812ff0a90cce966a4ff6bad57cbadc43b5 Mon Sep 17 00:00:00 2001
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Date: Wed, 29 Jan 2014 16:15:18 -0800
Subject: [PATCH] xen/pvh: set CR4 flags for APs

The Xen ABI sets said flags for the BSP, but it does
not do that for the CR4. As such fix it up to make
sure we have that flag set.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/enlighten.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a4d7b64..201d09a 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
 	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM guests
 	 * (which PVH shared codepaths), while X86_CR0_PG is for PVH. */
 	write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM);
+
+	if (!cpu)
+		return;
+	/*
+	 * For BSP, PSE PGE are set in probe_page_size_mask(), for APs
+	 * set them here. For all, OSFXSR OSXMMEXCPT are set in fpu_init.
+	*/
+	if (cpu_has_pse)
+		set_in_cr4(X86_CR4_PSE);
+
+	if (cpu_has_pge)
+		set_in_cr4(X86_CR4_PGE);
 }
 
 /*
-- 
1.8.3.1


> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  arch/x86/xen/enlighten.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index a4d7b64..201d09a 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
>  	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM guests
>  	 * (which PVH shared codepaths), while X86_CR0_PG is for PVH. */
>  	write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM);
> +
> +	if (!cpu)
> +		return;
> +	/*
> +	 * For BSP, PSE PGE are set in probe_page_size_mask(), for APs
> +	 * set them here. For all, OSFXSR OSXMMEXCPT are set in fpu_init.
> +	*/
> +	if (cpu_has_pse)
> +		set_in_cr4(X86_CR4_PSE);
> +
> +	if (cpu_has_pge)
> +		set_in_cr4(X86_CR4_PGE);
>  }
>  
>  /*
> -- 
> 1.7.2.3
> 

Re: [PATCH] pvh: set cr4 flags for APs

[Mukesh Rathor]

On Mon, 3 Feb 2014 06:49:14 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Wed, Jan 29, 2014 at 04:15:18PM -0800, Mukesh Rathor wrote:
> > We need to set cr4 flags for APs that are already set for BSP.
> 
> The title is missing the 'xen' part.

The patch is for linux, not xen.

> I rewrote it a bit and I think this should go in 3.14.
> 
> David, Boris: It is not the full fix as there are other parts to
> make an PVH guest use 2MB or 1GB pages- but this fixes an obvious
> bug.
> 
> 
> 
> From 797ea6812ff0a90cce966a4ff6bad57cbadc43b5 Mon Sep 17 00:00:00 2001
> From: Mukesh Rathor <mukesh.rathor@oracle.com>
> Date: Wed, 29 Jan 2014 16:15:18 -0800
> Subject: [PATCH] xen/pvh: set CR4 flags for APs
> 
> The Xen ABI sets said flags for the BSP, but it does

NO it does not. I said it few times, it's set by probe_page_size_mask
(which is in linux) for the BSP. The comment below also says it.

thanks
mukesh

> not do that for the CR4. As such fix it up to make
> sure we have that flag set.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/enlighten.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index a4d7b64..201d09a 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
>  	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM
> guests
>  	 * (which PVH shared codepaths), while X86_CR0_PG is for
> PVH. */ write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP |
> X86_CR0_AM); +
> +	if (!cpu)
> +		return;
> +	/*
> +	 * For BSP, PSE PGE are set in probe_page_size_mask(), for
> APs
> +	 * set them here. For all, OSFXSR OSXMMEXCPT are set in
> fpu_init.
> +	*/
> +	if (cpu_has_pse)
> +		set_in_cr4(X86_CR4_PSE);
> +
> +	if (cpu_has_pge)
> +		set_in_cr4(X86_CR4_PGE);
>  }
>  
>  /*

Re: [PATCH] pvh: set cr4 flags for APs

[Konrad Rzeszutek Wilk]

On Mon, Feb 03, 2014 at 11:30:01AM -0800, Mukesh Rathor wrote:
> On Mon, 3 Feb 2014 06:49:14 -0500
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > On Wed, Jan 29, 2014 at 04:15:18PM -0800, Mukesh Rathor wrote:
> > > We need to set cr4 flags for APs that are already set for BSP.
> > 
> > The title is missing the 'xen' part.
> 
> The patch is for linux, not xen.

Right. And hence you need to prefix the title with 'xen' in it
otherwise it won't be obvious from the Linux log line for what
component of the Linux tree it is.

> 
> > I rewrote it a bit and I think this should go in 3.14.
> > 
> > David, Boris: It is not the full fix as there are other parts to
> > make an PVH guest use 2MB or 1GB pages- but this fixes an obvious
> > bug.
> > 
> > 
> > 
> > From 797ea6812ff0a90cce966a4ff6bad57cbadc43b5 Mon Sep 17 00:00:00 2001
> > From: Mukesh Rathor <mukesh.rathor@oracle.com>
> > Date: Wed, 29 Jan 2014 16:15:18 -0800
> > Subject: [PATCH] xen/pvh: set CR4 flags for APs
> > 
> > The Xen ABI sets said flags for the BSP, but it does
> 
> NO it does not. I said it few times, it's set by probe_page_size_mask
> (which is in linux) for the BSP. The comment below also says it.

Where does it set it for APs? Can we piggyback on that?


> 
> thanks
> mukesh
> 
> > not do that for the CR4. As such fix it up to make
> > sure we have that flag set.
> > 
> > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  arch/x86/xen/enlighten.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index a4d7b64..201d09a 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
> >  	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM
> > guests
> >  	 * (which PVH shared codepaths), while X86_CR0_PG is for
> > PVH. */ write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP |
> > X86_CR0_AM); +
> > +	if (!cpu)
> > +		return;
> > +	/*
> > +	 * For BSP, PSE PGE are set in probe_page_size_mask(), for
> > APs
> > +	 * set them here. For all, OSFXSR OSXMMEXCPT are set in
> > fpu_init.
> > +	*/
> > +	if (cpu_has_pse)
> > +		set_in_cr4(X86_CR4_PSE);
> > +
> > +	if (cpu_has_pge)
> > +		set_in_cr4(X86_CR4_PGE);
> >  }
> >  
> >  /*
> 

Re: [PATCH] pvh: set cr4 flags for APs

[Konrad Rzeszutek Wilk]

On Mon, Feb 03, 2014 at 02:52:40PM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 03, 2014 at 11:30:01AM -0800, Mukesh Rathor wrote:
> > On Mon, 3 Feb 2014 06:49:14 -0500
> > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > 
> > > On Wed, Jan 29, 2014 at 04:15:18PM -0800, Mukesh Rathor wrote:
> > > > We need to set cr4 flags for APs that are already set for BSP.
> > > 
> > > The title is missing the 'xen' part.
> > 
> > The patch is for linux, not xen.
> 
> Right. And hence you need to prefix the title with 'xen' in it
> otherwise it won't be obvious from the Linux log line for what
> component of the Linux tree it is.
> 
> > 
> > > I rewrote it a bit and I think this should go in 3.14.
> > > 
> > > David, Boris: It is not the full fix as there are other parts to
> > > make an PVH guest use 2MB or 1GB pages- but this fixes an obvious
> > > bug.
> > > 
> > > 
> > > 
> > > From 797ea6812ff0a90cce966a4ff6bad57cbadc43b5 Mon Sep 17 00:00:00 2001
> > > From: Mukesh Rathor <mukesh.rathor@oracle.com>
> > > Date: Wed, 29 Jan 2014 16:15:18 -0800
> > > Subject: [PATCH] xen/pvh: set CR4 flags for APs
> > > 
> > > The Xen ABI sets said flags for the BSP, but it does
> > 
> > NO it does not. I said it few times, it's set by probe_page_size_mask
> > (which is in linux) for the BSP. The comment below also says it.
> 
> Where does it set it for APs? Can we piggyback on that?

And since I am in a hurry to fix an build regression I did the research
myself - but this kind of information needs to be in the commit message.

Here is what I have, please comment as I want to send a git pull to Linux
within the hour.

>From 125ef07fd58e963cc286554f6536e46c9712033c Mon Sep 17 00:00:00 2001
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Date: Wed, 29 Jan 2014 16:15:18 -0800
Subject: [PATCH] xen/pvh: set CR4 flags for APs

During bootup in the 'probe_page_size_mask' these CR4
flags are set in there. But for AP processors they
are not set as we do not use 'secondary_startup_64' which
the baremetal kernels uses. Instead do it in
this function which we use in Xen PVH during our
startup for AP and BSP processors.

As such fix it up to make sure we have that flag set.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/enlighten.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a4d7b64..201d09a 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
 	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM guests
 	 * (which PVH shared codepaths), while X86_CR0_PG is for PVH. */
 	write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM);
+
+	if (!cpu)
+		return;
+	/*
+	 * For BSP, PSE PGE are set in probe_page_size_mask(), for APs
+	 * set them here. For all, OSFXSR OSXMMEXCPT are set in fpu_init.
+	*/
+	if (cpu_has_pse)
+		set_in_cr4(X86_CR4_PSE);
+
+	if (cpu_has_pge)
+		set_in_cr4(X86_CR4_PGE);
 }
 
 /*
-- 
1.8.3.1

Re: [PATCH] pvh: set cr4 flags for APs

[Mukesh Rathor]

On Mon, 3 Feb 2014 15:43:46 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Mon, Feb 03, 2014 at 02:52:40PM -0500, Konrad Rzeszutek Wilk wrote:
> > On Mon, Feb 03, 2014 at 11:30:01AM -0800, Mukesh Rathor wrote:
> > > On Mon, 3 Feb 2014 06:49:14 -0500
> > > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > > 
> > > > On Wed, Jan 29, 2014 at 04:15:18PM -0800, Mukesh Rathor wrote:
> > > > > We need to set cr4 flags for APs that are already set for BSP.
> > > > 
> > > > The title is missing the 'xen' part.
> > > 
> > > The patch is for linux, not xen.
> > 
> > Right. And hence you need to prefix the title with 'xen' in it
> > otherwise it won't be obvious from the Linux log line for what
> > component of the Linux tree it is.
> > 
> > > 
> > > > I rewrote it a bit and I think this should go in 3.14.
> > > > 
> > > > David, Boris: It is not the full fix as there are other parts to
> > > > make an PVH guest use 2MB or 1GB pages- but this fixes an
> > > > obvious bug.
> > > > 
> > > > 
> > > > 
> > > > From 797ea6812ff0a90cce966a4ff6bad57cbadc43b5 Mon Sep 17
> > > > 00:00:00 2001 From: Mukesh Rathor <mukesh.rathor@oracle.com>
> > > > Date: Wed, 29 Jan 2014 16:15:18 -0800
> > > > Subject: [PATCH] xen/pvh: set CR4 flags for APs
> > > > 
> > > > The Xen ABI sets said flags for the BSP, but it does
> > > 
> > > NO it does not. I said it few times, it's set by
> > > probe_page_size_mask (which is in linux) for the BSP. The comment
> > > below also says it.
> > 
> > Where does it set it for APs? Can we piggyback on that?
> 
> And since I am in a hurry to fix an build regression I did the
> research myself - but this kind of information needs to be in the
> commit message.
> 
> Here is what I have, please comment as I want to send a git pull to
> Linux within the hour.
> 
> From 125ef07fd58e963cc286554f6536e46c9712033c Mon Sep 17 00:00:00 2001
> From: Mukesh Rathor <mukesh.rathor@oracle.com>
> Date: Wed, 29 Jan 2014 16:15:18 -0800
> Subject: [PATCH] xen/pvh: set CR4 flags for APs
> 
> During bootup in the 'probe_page_size_mask' these CR4
> flags are set in there. But for AP processors they
> are not set as we do not use 'secondary_startup_64' which
> the baremetal kernels uses. Instead do it in
> this function which we use in Xen PVH during our
> startup for AP and BSP processors.
> 
> As such fix it up to make sure we have that flag set.

Thats good enough for me.

Mukesh


> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/enlighten.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index a4d7b64..201d09a 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1473,6 +1473,18 @@ static void xen_pvh_set_cr_flags(int cpu)
>  	 * X86_CR0_TS, X86_CR0_PE, X86_CR0_ET are set by Xen for HVM
> guests
>  	 * (which PVH shared codepaths), while X86_CR0_PG is for
> PVH. */ write_cr0(read_cr0() | X86_CR0_MP | X86_CR0_NE | X86_CR0_WP |
> X86_CR0_AM); +
> +	if (!cpu)
> +		return;
> +	/*
> +	 * For BSP, PSE PGE are set in probe_page_size_mask(), for
> APs
> +	 * set them here. For all, OSFXSR OSXMMEXCPT are set in
> fpu_init.
> +	*/
> +	if (cpu_has_pse)
> +		set_in_cr4(X86_CR4_PSE);
> +
> +	if (cpu_has_pge)
> +		set_in_cr4(X86_CR4_PGE);
>  }
>  
>  /*

Re: [PATCH V0] linux PVH: Set CR4 flags

[Roger Pau Monné]

On 30/01/14 00:15, Mukesh Rathor wrote:
> Konrad,
> 
> The CR4 settings were dropped from my earlier patch because you didn't
> wanna enable them. But since you do now, we need to set them in the APs
> also. If you decide not too again, please apply my prev patch
> "pvh: disable pse feature for now".

Hello Mukesh,

Could you push your CR related patches to a git repo branch? I'm
currently having a bit of a mess in figuring out which ones should be
applied and in which order.

Thanks, Roger.

Re: [PATCH V0] linux PVH: Set CR4 flags

[Mukesh Rathor]

On Thu, 30 Jan 2014 11:40:44 +0000
Roger Pau Monné <roger.pau@citrix.com> wrote:

> On 30/01/14 00:15, Mukesh Rathor wrote:
> > Konrad,
> > 
> > The CR4 settings were dropped from my earlier patch because you
> > didn't wanna enable them. But since you do now, we need to set them
> > in the APs also. If you decide not too again, please apply my prev
> > patch "pvh: disable pse feature for now".
> 
> Hello Mukesh,
> 
> Could you push your CR related patches to a git repo branch? I'm
> currently having a bit of a mess in figuring out which ones should be
> applied and in which order.
> 
> Thanks, Roger.

Hey Roger,

Unfortunately, I don't have them in a tree because my first patch was 
changed during merge, and also the tree was refreshed.  Basically, the end
result, we leave features enabled on linux side, thus setting not only
the cr0 bits, but also the cr4 PSE and PGE for APs (they were already
set for the BSP). 

Konrad only merged the CR0 setting part of my first patch, hence this 
patch to set the CR4 bits. Hope that makes sense. My latest tree is:

http://oss.us.oracle.com/git/mrathor/linux.git  muk2

thanks
mukesh