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

Groups > linux.kernel > #1481550 > unrolled thread

Re: [RFC PATCH v2 11/20] mm: Access BOOT related data in the clear

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2 11/20] mm: Access BOOT related data in the clear Andy Lutomirski <luto@amacapital.net> - 2016-09-12 19:00 +0200
    Re: [RFC PATCH v2 11/20] mm: Access BOOT related data in the clear Matt Fleming <matt@codeblueprint.co.uk> - 2016-09-15 12:00 +0200

#1481550 — Re: [RFC PATCH v2 11/20] mm: Access BOOT related data in the clear

On Aug 22, 2016 6:53 PM, "Tom Lendacky" <thomas.lendacky@amd.com> wrote:
>
> BOOT data (such as EFI related data) is not encyrpted when the system is
> booted and needs to be accessed as non-encrypted.  Add support to the
> early_memremap API to identify the type of data being accessed so that
> the proper encryption attribute can be applied.  Currently, two types
> of data are defined, KERNEL_DATA and BOOT_DATA.

What happens when you memremap boot services data outside of early
boot?  Matt just added code that does this.

IMO this API is not so great.  It scatters a specialized consideration
all over the place.  Could early_memremap not look up the PA to figure
out what to do?

--Andy

[leaving the rest here for Matt's benefit]

>                      unsigned long size,
> +                                                   enum memremap_owner owner,
> +                                                   pgprot_t prot)
> +{
> +       return prot;
> +}
> +
>  void __init early_ioremap_reset(void)
>  {
>         early_ioremap_shutdown();
> @@ -213,16 +221,23 @@ early_ioremap(resource_size_t phys_addr, unsigned long size)
>
>  /* Remap memory */
>  void __init *
> -early_memremap(resource_size_t phys_addr, unsigned long size)
> +early_memremap(resource_size_t phys_addr, unsigned long size,
> +              enum memremap_owner owner)
>  {
> -       return (__force void *)__early_ioremap(phys_addr, size,
> -                                              FIXMAP_PAGE_NORMAL);
> +       pgprot_t prot = early_memremap_pgprot_adjust(phys_addr, size, owner,
> +                                                    FIXMAP_PAGE_NORMAL);
> +
> +       return (__force void *)__early_ioremap(phys_addr, size, prot);
>  }
>  #ifdef FIXMAP_PAGE_RO
>  void __init *
> -early_memremap_ro(resource_size_t phys_addr, unsigned long size)
> +early_memremap_ro(resource_size_t phys_addr, unsigned long size,
> +                 enum memremap_owner owner)
>  {
> -       return (__force void *)__early_ioremap(phys_addr, size, FIXMAP_PAGE_RO);
> +       pgprot_t prot = early_memremap_pgprot_adjust(phys_addr, size, owner,
> +                                                    FIXMAP_PAGE_RO);
> +
> +       return (__force void *)__early_ioremap(phys_addr, size, prot);
>  }
>  #endif
>
> @@ -236,7 +251,8 @@ early_memremap_prot(resource_size_t phys_addr, unsigned long size,
>
>  #define MAX_MAP_CHUNK  (NR_FIX_BTMAPS << PAGE_SHIFT)
>
> -void __init copy_from_early_mem(void *dest, phys_addr_t src, unsigned long size)
> +void __init copy_from_early_mem(void *dest, phys_addr_t src, unsigned long size,
> +                               enum memremap_owner owner)
>  {
>         unsigned long slop, clen;
>         char *p;
> @@ -246,7 +262,7 @@ void __init copy_from_early_mem(void *dest, phys_addr_t src, unsigned long size)
>                 clen = size;
>                 if (clen > MAX_MAP_CHUNK - slop)
>                         clen = MAX_MAP_CHUNK - slop;
> -               p = early_memremap(src & PAGE_MASK, clen + slop);
> +               p = early_memremap(src & PAGE_MASK, clen + slop, owner);
>                 memcpy(dest, p + slop, clen);
>                 early_memunmap(p, clen + slop);
>                 dest += clen;
> @@ -265,12 +281,14 @@ early_ioremap(resource_size_t phys_addr, unsigned long size)
>
>  /* Remap memory */
>  void __init *
> -early_memremap(resource_size_t phys_addr, unsigned long size)
> +early_memremap(resource_size_t phys_addr, unsigned long size,
> +              enum memremap_owner owner)
>  {
>         return (void *)phys_addr;
>  }
>  void __init *
> -early_memremap_ro(resource_size_t phys_addr, unsigned long size)
> +early_memremap_ro(resource_size_t phys_addr, unsigned long size,
> +                 enum memremap_owner owner)
>  {
>         return (void *)phys_addr;
>  }
>

[toc] | [next] | [standalone]

#1483942

On Wed, 14 Sep, at 09:20:44AM, Tom Lendacky wrote:
> On 09/12/2016 11:55 AM, Andy Lutomirski wrote:
> > On Aug 22, 2016 6:53 PM, "Tom Lendacky" <thomas.lendacky@amd.com> wrote:
> >>
> >> BOOT data (such as EFI related data) is not encyrpted when the system is
> >> booted and needs to be accessed as non-encrypted.  Add support to the
> >> early_memremap API to identify the type of data being accessed so that
> >> the proper encryption attribute can be applied.  Currently, two types
> >> of data are defined, KERNEL_DATA and BOOT_DATA.
> > 
> > What happens when you memremap boot services data outside of early
> > boot?  Matt just added code that does this.
> > 
> > IMO this API is not so great.  It scatters a specialized consideration
> > all over the place.  Could early_memremap not look up the PA to figure
> > out what to do?
> 
> Yes, I could see if the PA falls outside of the kernel usable area and,
> if so, remove the memory encryption attribute from the mapping (for both
> early_memremap and memremap).
> 
> Let me look into that, I would prefer something along that line over
> this change.

So, the last time we talked about using the address to figure out
whether to encrypt/decrypt you said,

 "I looked into this and this would be a large change also to parse
  tables and build lists."

Has something changed that makes this approach easier?

And again, you need to be careful with the EFI kexec code paths, since
you've got a mixture of boot and kernel data being passed. In
particular the EFI memory map is allocated by the firmware on first
boot (BOOT_DATA) but by the kernel on kexec (KERNEL_DATA).

That's one of the reasons I suggested requiring the caller to decide
on BOOT_DATA vs KERNEL_DATA - when you start looking at kexec the
distinction isn't easily made.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web