question about head_64.S

question about head_64.S

[Cao jin]

Hi,
  I have been digging into this file for a while, and I still have 2
questions unclear, hope to get your help.

1.
At the entry of startup_64, we set all the data segment registers to 0,
according to commit 08da5a2ca("x86_64: Early segment setup for VT"), it
is said to accelerate the decompression under VT. I don't know Intel VT,
but I did test under physical machine and virtual machine(with KVM, and
intel VT enabled in BIOS) with following patch:

diff --git a/arch/x86/boot/compressed/head_64.S
b/arch/x86/boot/compressed/head_64.S
index 58f6a467f1fa..595f3c300173 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -260,12 +260,12 @@ ENTRY(startup_64)
         */

        /* Setup data segments. */
-       xorl    %eax, %eax
-       movl    %eax, %ds
-       movl    %eax, %es
-       movl    %eax, %ss
-       movl    %eax, %fs
-       movl    %eax, %gs
+//     xorl    %eax, %eax
+//     movl    %eax, %ds
+//     movl    %eax, %es
+//     movl    %eax, %ss
+//     movl    %eax, %fs
+//     movl    %eax, %gs

I don't see any obvious booting time difference, is there anything I missed?
Also, I don't find explicit document saying we should zero these
registers under VT.

2.
Why gdt64 has following definition?:

gdt64:
	.word	gdt_end - gdt
	.long	0
	.word	0
	.quad   0

obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes
long, so why not just:

gdt64:
	.word	gdt_end - gdt
	.quad   0

With above modification, it can boot.
-- 
Sincerely,
Cao jin

Re: question about head_64.S

[Thomas Gleixner]

On Tue, 15 Jan 2019, Cao jin wrote:

> Hi,
>   I have been digging into this file for a while, and I still have 2
> questions unclear, hope to get your help.
> 
> 1.
> At the entry of startup_64, we set all the data segment registers to 0,
> according to commit 08da5a2ca("x86_64: Early segment setup for VT"), it
> is said to accelerate the decompression under VT. I don't know Intel VT,
> but I did test under physical machine and virtual machine(with KVM, and
> intel VT enabled in BIOS) with following patch:
> 
> diff --git a/arch/x86/boot/compressed/head_64.S
> b/arch/x86/boot/compressed/head_64.S
> index 58f6a467f1fa..595f3c300173 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -260,12 +260,12 @@ ENTRY(startup_64)
>          */
> 
>         /* Setup data segments. */
> -       xorl    %eax, %eax
> -       movl    %eax, %ds
> -       movl    %eax, %es
> -       movl    %eax, %ss
> -       movl    %eax, %fs
> -       movl    %eax, %gs
> +//     xorl    %eax, %eax
> +//     movl    %eax, %ds
> +//     movl    %eax, %es
> +//     movl    %eax, %ss
> +//     movl    %eax, %fs
> +//     movl    %eax, %gs
> 
> I don't see any obvious booting time difference, is there anything I missed?
> Also, I don't find explicit document saying we should zero these
> registers under VT.

The decompressor is position independent code, so all segments have to be
set to 0.

The patch you mentioned was just adding fs/gs to the list of segments
which are cleared and the commit message is not very clear. Though if you
dig further down then you find the original version of that patch:

  commit ffb6017563aa("[PATCH] x86-64: x86_64 - Fix FS/GS registers for VT execution")

That one has a proper explantaion.

Thanks,

	tglx

Re: question about head_64.S

[Cao jin]

On 1/15/19 11:55 PM, Thomas Gleixner wrote:
> On Tue, 15 Jan 2019, Cao jin wrote:
> 
>> Hi,
>>   I have been digging into this file for a while, and I still have 2
>> questions unclear, hope to get your help.
>>
>> 1.
>> At the entry of startup_64, we set all the data segment registers to 0,
>> according to commit 08da5a2ca("x86_64: Early segment setup for VT"), it
>> is said to accelerate the decompression under VT. I don't know Intel VT,
>> but I did test under physical machine and virtual machine(with KVM, and
>> intel VT enabled in BIOS) with following patch:
>>
>> diff --git a/arch/x86/boot/compressed/head_64.S
>> b/arch/x86/boot/compressed/head_64.S
>> index 58f6a467f1fa..595f3c300173 100644
>> --- a/arch/x86/boot/compressed/head_64.S
>> +++ b/arch/x86/boot/compressed/head_64.S
>> @@ -260,12 +260,12 @@ ENTRY(startup_64)
>>          */
>>
>>         /* Setup data segments. */
>> -       xorl    %eax, %eax
>> -       movl    %eax, %ds
>> -       movl    %eax, %es
>> -       movl    %eax, %ss
>> -       movl    %eax, %fs
>> -       movl    %eax, %gs
>> +//     xorl    %eax, %eax
>> +//     movl    %eax, %ds
>> +//     movl    %eax, %es
>> +//     movl    %eax, %ss
>> +//     movl    %eax, %fs
>> +//     movl    %eax, %gs
>>
>> I don't see any obvious booting time difference, is there anything I missed?
>> Also, I don't find explicit document saying we should zero these
>> registers under VT.
> 
> The decompressor is position independent code, so all segments have to be
> set to 0.
> 

Thank you Thomas! But I've never heard that PIC is correlated with
segment register value, could you elaborate a little bit? Because as I
know, startup_64 is in long mode, and CPU will treat all segment(except
fs, gs) base as 0, no matter whatever in them. And until now, I only see
fs is touched when parsing command line, not see any explicit gs usage.

On the other hand, I test the patch above, it can boot up, so seems
segment register value here is not necessary to be 0?

> The patch you mentioned was just adding fs/gs to the list of segments
> which are cleared and the commit message is not very clear. Though if you
> dig further down then you find the original version of that patch:
> 
>   commit ffb6017563aa("[PATCH] x86-64: x86_64 - Fix FS/GS registers for VT execution")
> 
> That one has a proper explantaion.
> 

Oh, I still not reach to the real kernel itself yet. At first glance,
"but it is important to reload them in protected mode" make sense to me.

But more confusion rise up: under 64-bit boot protocol, we can have more
than one entry? startup_64 in both:

  arch/x86/kernel/head_64.S
and
  arch/x86/boot/compressed/head_64.S

can be jumped to via bootloader? Seems Documentation/x86/boot.txt
doesn't say that.

-- 
Sincerely,
Cao jin

Re: question about head_64.S

[Cao jin]

Hi, Kirll,

On 1/15/19 7:45 PM, Cao jin wrote:
> Hi,
>   I have been digging into this file for a while, and I still have 2
> questions unclear, hope to get your help.
> 

> 
> 2.
> Why gdt64 has following definition?:
> 
> gdt64:
> 	.word	gdt_end - gdt
> 	.long	0
> 	.word	0
> 	.quad   0
> 
> obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes
> long, so why not just:
> 
> gdt64:
> 	.word	gdt_end - gdt
> 	.quad   0
> 
> With above modification, it can boot.
> 

Seems you introduced gdt64 code in commit beebaccd50, could you help
with this question?

And it also remind me of another question about adjust_got which is also
introduced by you. Because I failed to construct a test environment with
ld version less than 2.24 until now, so I wanna do a quick ask here:
does it make sense to adjust GOT from the 4th entry of it? Because as I
know, the first 3 entries are special one, which (I guess) will be not used.
-- 
Sincerely,
Cao jin

Re: question about head_64.S

[Kirill A. Shutemov]

On Tue, Jan 22, 2019 at 03:31:25PM +0800, Cao jin wrote:
> Hi, Kirll,
> 
> On 1/15/19 7:45 PM, Cao jin wrote:
> > Hi,
> >   I have been digging into this file for a while, and I still have 2
> > questions unclear, hope to get your help.
> > 
> 
> > 
> > 2.
> > Why gdt64 has following definition?:
> > 
> > gdt64:
> > 	.word	gdt_end - gdt
> > 	.long	0
> > 	.word	0
> > 	.quad   0
> > 
> > obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes
> > long, so why not just:
> > 
> > gdt64:
> > 	.word	gdt_end - gdt
> > 	.quad   0
> > 
> > With above modification, it can boot.
> > 
> 
> Seems you introduced gdt64 code in commit beebaccd50, could you help
> with this question?

Looks like you are right. I've got confused at some point.

Could you prepare a patch?

> And it also remind me of another question about adjust_got which is also
> introduced by you. Because I failed to construct a test environment with
> ld version less than 2.24 until now, so I wanna do a quick ask here:
> does it make sense to adjust GOT from the 4th entry of it? Because as I
> know, the first 3 entries are special one, which (I guess) will be not used.

No.

These 3 entries are reserved for a special symbols (like entry 0 for
_DYNAMIC). It means linker should not use these entries for normal
symbols, but it doesn't mean that they don't need to be adjusted during
the load.

-- 
 Kirill A. Shutemov

Re: question about head_64.S

[Cao jin]

On 1/22/19 9:08 PM, Kirill A. Shutemov wrote:
> On Tue, Jan 22, 2019 at 03:31:25PM +0800, Cao jin wrote:
>> Hi, Kirll,
>>

>>> 2.
>>> Why gdt64 has following definition?:
>>>
>>> gdt64:
>>> 	.word	gdt_end - gdt
>>> 	.long	0
>>> 	.word	0
>>> 	.quad   0
>>>
>>> obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes
>>> long, so why not just:
>>>
>>> gdt64:
>>> 	.word	gdt_end - gdt
>>> 	.quad   0
>>>
>>> With above modification, it can boot.
>>>
>>
>> Seems you introduced gdt64 code in commit beebaccd50, could you help
>> with this question?
> 
> Looks like you are right. I've got confused at some point.
> 
> Could you prepare a patch?

Sure.

> 
>> And it also remind me of another question about adjust_got which is also
>> introduced by you. Because I failed to construct a test environment with
>> ld version less than 2.24 until now, so I wanna do a quick ask here:
>> does it make sense to adjust GOT from the 4th entry of it? Because as I
>> know, the first 3 entries are special one, which (I guess) will be not used.
> 
> No.
> 
> These 3 entries are reserved for a special symbols (like entry 0 for
> _DYNAMIC). It means linker should not use these entries for normal
> symbols, but it doesn't mean that they don't need to be adjusted during
> the load.
> 

Thanks for your info! BTW, could I know how you construct the test
environment?

I tried centos6, the GCC version is too old to compile; then tried
fedora28 with binutils-2.20.51.0.2-5.48.el6.x86_64.rpm from centos6, ld
reported errors; and then tried compiling binutils source with tag 2.23,
stopped at configure phase:(

-- 
Sincerely,
Cao jin

Re: question about head_64.S

[Kirill A. Shutemov]

On Wed, Jan 23, 2019 at 12:01:23PM +0800, Cao jin wrote:
> Thanks for your info! BTW, could I know how you construct the test
> environment?

Nothing fancy. I just built different versions of binutils from git.

-- 
 Kirill A. Shutemov