Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1595612

Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment

Show all headers | View raw

On 03/08/17 at 11:50am, Borislav Petkov wrote:
> On Wed, Mar 08, 2017 at 06:17:50PM +0800, Baoquan He wrote:
> > All right, I will just update the code comment. Just back ported kaslr
> > to our OS product, people reviewed and found the upper boundary of kaslr
> > mm region is EFI_VA_START, that's not correct, it has to be corrected
> > firstly in upstream. Then found the confusion in code comment.
> 
>         BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> +                    vaddr_end >= EFI_VA_END);
> 
> so I think that once we've done the mapping, we won't need anymore VA
> space so we could simply check the range [efi_va, EFI_VA_START] instead.
> 
> However, that won't work currently because evi_va is not valid at
> build time. And it won't work at boot time either because, AFAICT,
> kernel_randomize_memory() runs before efi_enter_virtual_mode() so ...
> 
> So yours is probably OK.
> 
> I guess what's confusing there is the naming - EFI_VA_START and
> EFI_VA_END. They're kinda swapped because of the direction we take when
> we start mapping runtime services, i.e., from the higher (unsigned)
> address to lower.
> 
> I guess we could swap the naming so that it doesn't confuse people but
> that would be up to EFI maintainers.
> 
> Then stuff like that:
> 
> # ifdef CONFIG_EFI
>         { EFI_VA_END,           "EFI Runtime Services" },
> # endif
> 
> will make more sense when they are:
> 
> # ifdef CONFIG_EFI
>         { EFI_VA_START,           "EFI Runtime Services" },
> # endif
> 
> But changing it now could confuse more people who have the current
> mental picture of the mapping direction so I'd vote for the simple fix
> above.

People should understand the meaning of the macro then use it correctly,
one should not assume START == lower address unless they are sure. 

> 
> Again, as previously, this is a maintainer decision.
> 

Personally I think current way is just fine, but agreed it is up to efi
maintainer. 

Thanks
Dave

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 08:50 +0100
  Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Dave Young <dyoung@redhat.com> - 2017-03-08 09:20 +0100
    Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 10:10 +0100
    Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Bhupesh Sharma <bhsharma@redhat.com> - 2017-03-08 10:10 +0100
      Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 10:30 +0100
        Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Borislav Petkov <bp@alien8.de> - 2017-03-08 10:40 +0100
          Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 11:30 +0100
            Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Borislav Petkov <bp@alien8.de> - 2017-03-08 15:00 +0100
              Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Dave Young <dyoung@redhat.com> - 2017-03-09 02:00 +0100
          Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Bhupesh Sharma <bhsharma@redhat.com> - 2017-03-08 21:10 +0100
      Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 11:20 +0100
    Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Baoquan He <bhe@redhat.com> - 2017-03-08 10:10 +0100
      Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Borislav Petkov <bp@suse.de> - 2017-03-08 14:10 +0100
      Re: [PATCH 1/2] x86/efi: Correct a tiny mistake in code comment Dave Young <dyoung@redhat.com> - 2017-03-09 02:50 +0100
  [PATCH v2 1/2] x86/efi/64: Clean up code comment about efi region Baoquan He <bhe@redhat.com> - 2017-03-08 14:00 +0100

csiph-web