On Sun, May 13, 2018 at 06:55:46PM +0000, Thomas Gleixner wrote: > On Thu, 10 May 2018, Kirill A. Shutemov wrote: > > > + /* > > + * paging_prepare() and cleanup_trampoline() below can have GOT > > + * references. Adjust the table with address we are running at. > > + */ > > + > > + /* The GOP was not adjusted before */ > > GOP == EFI speak for Graphics Output Protocol. What the heck?
I was not aware about Graphics Output Protocol. > > > + xorq %rax, %rax > > And this clearing of RAX is related to this because? Sure you need it for > adjust_got() but adding a comment to that is too much asked for, right? Huh? The comment just above the line describes why it's needed. > > + /* Calculate the address the binary is loaded at. */ > > + call 1f > > +1: popq %rdi > > + subq $1b, %rdi > > + > > + call adjust_gop > > + > > /* > > * At this point we are in long mode with 4-level paging enabled, > > * but we might want to enable 5-level paging or vice versa. > > @@ -381,6 +396,24 @@ trampoline_return: > > pushq $0 > > popfq > > > > + /* > > + * Previously we've adjusted the GOT with address the binary was > > + * loaded at. Now we need to re-adjust for relocation address. > > + */ > > Breaking up those comments makes it more readable, right? Yes, I think so. The first comment is for the whole block of code below. The second is the comment for the first step. > > + /* > > + * Calculate the address the binary is loaded at. > > + * This address was used to adjust the table before and we need to > > + * undo the change. > > + */ > > + call 1f > > +1: popq %rax > > + subq $1b, %rax > > + > > + /* The new adjustment is relocation address */ > > is the relocation address > > > + movq %rbx, %rdi > > + call adjust_gop > > + > > /* > > * Copy the compressed kernel to the end of our buffer > > * where decompression in place becomes safe. > > @@ -481,19 +514,6 @@ relocated: > > shrq $3, %rcx > > rep stosq > > > > -/* > > - * Adjust our own GOT > > - */ > > - leaq _got(%rip), %rdx > > - leaq _egot(%rip), %rcx > > -1: > > - cmpq %rcx, %rdx > > - jae 2f > > - addq %rbx, (%rdx) > > - addq $8, %rdx > > - jmp 1b > > -2: > > - > > /* > > * Do the extraction, and jump to the new kernel.. > > */ > > @@ -512,6 +532,26 @@ relocated: > > */ > > jmp *%rax > > > > +/* > > + * Adjust global offest table > > offest? > > > + * > > + * RAX is previous adjustment of the table to undo (0 if it's the first > > time we touch GOP). > > is the previous > > And there is no reason to make that line overly long. > > > + * RDI is the new adjustment to apply. > > + */ > > +adjust_gop: > > + /* Walk through the GOT adding the address to the entries */ > > + leaq _got(%rip), %rdx > > + leaq _egot(%rip), %rcx > > +1: > > + cmpq %rcx, %rdx > > + jae 2f > > + subq %rax, (%rdx) /* Undo previous adjustment */ > > + addq %rdi, (%rdx) /* Apply the new adjustment */ > > + addq $8, %rdx > > + jmp 1b > > +2: > > + ret > > I'm really tired of your carelessness. The amount of half baken stuff you > submit is way above the tolerance level by now. I asked you several times > to be more careful, but you simply do not care at all. Get your act > together finally. I don't think it's fair. Yes, I have hard time write correctly, even in my native languages. I'm blind to mistakes I do. I'm sorry about them. But I do care about bugs in my code and I do my best addressing them. It took a lot of effort to find root cause of the bug and your statement about my carelessness doesn't match my assessment. Have a nice day. -- Kirill A. Shutemov
