[PATCH] xen/x86: Zero out .bss for PV guests

[PATCH] xen/x86: Zero out .bss for PV guests

[Boris Ostrovsky]

Baremetal kernels clear .bss early in the boot. Since Xen PV guests don't
excecute that early code they should do it too.

(Since we introduce macros for specifying 32- and 64-bit registers we
can get rid of ifdefs in startup_xen())

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/xen-head.S | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index b65f59a..2af87d1 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -35,16 +35,31 @@
 #define PVH_FEATURES (0)
 #endif
 
-	__INIT
-ENTRY(startup_xen)
-	cld
 #ifdef CONFIG_X86_32
-	mov %esi,xen_start_info
-	mov $init_thread_union+THREAD_SIZE,%esp
+#define REG(register)	%e##register
+#define WSIZE_SHIFT	2
+#define STOS		stosl
 #else
-	mov %rsi,xen_start_info
-	mov $init_thread_union+THREAD_SIZE,%rsp
+#define REG(register)	%r##register
+#define WSIZE_SHIFT	3
+#define STOS		stosq
 #endif
+
+	__INIT
+ENTRY(startup_xen)
+	cld
+
+	/* Clear .bss */
+	xor REG(ax),REG(ax)
+	mov $__bss_start,REG(di)
+	mov $__bss_stop,REG(cx)
+	sub REG(di),REG(cx)
+	shr $WSIZE_SHIFT,REG(cx)
+	rep STOS
+
+	mov REG(si),xen_start_info
+	mov $init_thread_union+THREAD_SIZE,REG(sp)
+
 	jmp xen_start_kernel
 
 	__FINIT
-- 
2.1.0

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[David Vrabel]

On 22/02/16 22:06, Boris Ostrovsky wrote:
> Baremetal kernels clear .bss early in the boot. Since Xen PV guests don't
> excecute that early code they should do it too.
> 
> (Since we introduce macros for specifying 32- and 64-bit registers we
> can get rid of ifdefs in startup_xen())

.bss must have been cleared for PV guests otherwise they would be
horribly broken.  What was the method and why is it no longer sufficient?

David

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[Andrew Cooper]

On 24/02/16 14:12, David Vrabel wrote:
> On 22/02/16 22:06, Boris Ostrovsky wrote:
>> Baremetal kernels clear .bss early in the boot. Since Xen PV guests don't
>> excecute that early code they should do it too.
>>
>> (Since we introduce macros for specifying 32- and 64-bit registers we
>> can get rid of ifdefs in startup_xen())
> .bss must have been cleared for PV guests otherwise they would be
> horribly broken.  What was the method and why is it no longer sufficient?

The domain builder hands out zeroed pages.  I don't believe we guarantee
that the guests RAM is clean, but it is in practice.

~Andrew

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[Boris Ostrovsky]

On 02/24/2016 09:15 AM, Andrew Cooper wrote:
> On 24/02/16 14:12, David Vrabel wrote:
>> On 22/02/16 22:06, Boris Ostrovsky wrote:
>>> Baremetal kernels clear .bss early in the boot. Since Xen PV guests don't
>>> excecute that early code they should do it too.
>>>
>>> (Since we introduce macros for specifying 32- and 64-bit registers we
>>> can get rid of ifdefs in startup_xen())
>> .bss must have been cleared for PV guests otherwise they would be
>> horribly broken.  What was the method and why is it no longer sufficient?

I couldn't find this being done anywhere, hence this patch.

> The domain builder hands out zeroed pages.  I don't believe we guarantee
> that the guests RAM is clean, but it is in practice.

OK, that's what I suspected but didn't actually look.

I, in fact, wonder whether this should go to stable trees as well.

-boris

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[David Vrabel]

On 24/02/16 14:52, Boris Ostrovsky wrote:
> On 02/24/2016 09:15 AM, Andrew Cooper wrote:
>> On 24/02/16 14:12, David Vrabel wrote:
>>> On 22/02/16 22:06, Boris Ostrovsky wrote:
>>>> Baremetal kernels clear .bss early in the boot. Since Xen PV guests
>>>> don't
>>>> excecute that early code they should do it too.
>>>>
>>>> (Since we introduce macros for specifying 32- and 64-bit registers we
>>>> can get rid of ifdefs in startup_xen())
>>> .bss must have been cleared for PV guests otherwise they would be
>>> horribly broken.  What was the method and why is it no longer
>>> sufficient?
> 
> I couldn't find this being done anywhere, hence this patch.
> 
>> The domain builder hands out zeroed pages.  I don't believe we guarantee
>> that the guests RAM is clean, but it is in practice.
> 
> OK, that's what I suspected but didn't actually look.
> 
> I, in fact, wonder whether this should go to stable trees as well.

Yes.  Can you respin with a commit message explaining?  (Or just provide
the message here and I'll fix it up).

David

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[Luis R. Rodriguez]

On Wed, Feb 24, 2016 at 6:58 AM, David Vrabel <david.vrabel@citrix.com> wrote:
> Yes.  Can you respin with a commit message explaining?  (Or just provide
> the message here and I'll fix it up).

Is there no way to re-use somehow the clear_bss() from bare metal?
This uses a section range:

/* Don't add a printk in there. printk relies on the PDA which is not
initialized
   yet. */
static void __init clear_bss(void)
{
        memset(__bss_start, 0,
               (unsigned long) __bss_stop - (unsigned long) __bss_start);
}

Perhaps the section range might be different for PV guests? Or can
this simply not work even if one added a guest bss section size, or
would it be too late for PV guests, ie we need to do it in asm on PV
guests as you did? If so why.

 Luis

Re: [Xen-devel] [PATCH] xen/x86: Zero out .bss for PV guests

[Boris Ostrovsky]

On 02/24/2016 11:22 AM, Luis R. Rodriguez wrote:
> On Wed, Feb 24, 2016 at 6:58 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>> Yes.  Can you respin with a commit message explaining?  (Or just provide
>> the message here and I'll fix it up).
> Is there no way to re-use somehow the clear_bss() from bare metal?

xen_start_info lives in .bss and it is initialized in startup assembly code.

We could move it to .data but I'd still want to do this first thing in 
startup_xen(). We may add more code there later that touches something 
in .bss and with delayed section clearing we may blow away that data.

-boris

> This uses a section range:
>
> /* Don't add a printk in there. printk relies on the PDA which is not
> initialized
>     yet. */
> static void __init clear_bss(void)
> {
>          memset(__bss_start, 0,
>                 (unsigned long) __bss_stop - (unsigned long) __bss_start);
> }
>
> Perhaps the section range might be different for PV guests? Or can
> this simply not work even if one added a guest bss section size, or
> would it be too late for PV guests, ie we need to do it in asm on PV
> guests as you did? If so why.
>
>   Luis