Re: [Part1 PATCH v4 10/17] x86/mm, resource: Use PAGE_KERNEL protection for ioremap of memory pages

From: Borislav Petkov
Date: Sun Sep 17 2017 - 10:07:15 EST


On Sat, Sep 16, 2017 at 07:34:11AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lendacky@xxxxxxx>
>
> In order for memory pages to be properly mapped when SEV is active, we
> need to use the PAGE_KERNEL protection attribute as the base protection.
> This will insure that memory mapping of, e.g. ACPI tables, receives the
> proper mapping attributes.
>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Laura Abbott <labbott@xxxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> Cc: "JÃrÃme Glisse" <jglisse@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> ---
> arch/x86/mm/ioremap.c | 77 ++++++++++++++++++++++++++++++++++++++++++--------
> include/linux/ioport.h | 3 ++
> kernel/resource.c | 19 +++++++++++++
> 3 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 52cc0f4ed494..812b8a8066ba 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -27,6 +27,11 @@
>
> #include "physaddr.h"
>
> +struct ioremap_mem_flags {
> + bool system_ram;
> + bool desc_other;
> +};
> +
> /*
> * Fix up the linear direct mapping of the kernel to avoid cache attribute
> * conflicts.
> @@ -56,19 +61,61 @@ int ioremap_change_attr(unsigned long vaddr, unsigned long size,
> return err;
> }
>
> -static int __ioremap_check_ram(unsigned long start_pfn, unsigned long nr_pages,
> - void *arg)
> +static int __ioremap_check_ram(struct resource *res)
> {
> + unsigned long start_pfn, stop_pfn;
> unsigned long i;
>
> - for (i = 0; i < nr_pages; ++i)
> - if (pfn_valid(start_pfn + i) &&
> - !PageReserved(pfn_to_page(start_pfn + i)))
> - return 1;
> + if ((res->flags & IORESOURCE_SYSTEM_RAM) != IORESOURCE_SYSTEM_RAM)
> + return 0;
> +
> + start_pfn = (res->start + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + stop_pfn = (res->end + 1) >> PAGE_SHIFT;
> + if (stop_pfn > start_pfn) {
> + for (i = 0; i < (stop_pfn - start_pfn); ++i)
> + if (pfn_valid(start_pfn + i) &&
> + !PageReserved(pfn_to_page(start_pfn + i)))
> + return 1;
> + }
>
> return 0;

Should return bool I guess.

Btw, this whole resource code is needlessly complex. Trying to follow
through it is one sick brain-twister. One day someone should go and
clean up that silly function pointer passing.

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--