Hi Jiri,

On 11/03/17 at 06:28pm, Jiri Bohac wrote:
> On machines where the GART aperture is mapped over physical RAM
> /proc/vmcore contains the remapped range and reading it may
> cause hangs or reboots. This range needs to be excluded from /proc/vmcore.
> 
> This has originally been implemented by commit dd5f726076cc ("kexec:
> support for kexec on panic using new system call").
> 
> The implementation relied on the GART code adding an iomem_resource for
> this range called "GART", as implemented by commit 56dd669a138c ("[PATCH]
> Insert GART region into resource map")
> 
> However, inserting the iomem_resource from the early GART code caused
> resource conflicts with some AGP drivers (bko#72201), which got avoided by
> reverting the patch in commit 707d4eefbdb3 ("Revert [PATCH] Insert GART
> region into resource map"). This revert introduced the /proc/vmcore bug.
> 
> With the "GART" iomem_resource removed, the defunct code in crash.c has
> been removed by commit f296f2634920 ("x86/kexec: Remove walk_iomem_res()
> call with GART type")

Is this reproduced on a machine with GART existing and passing test with
this patch applied? Do you have a /proc/iomem printing about the machine
you are testing on?

If this patch works, then I am wondering how we shold deal with the old
way in which no '-s' is specified. Since no GART information is exported
to /proc/iomem.

Do we have a way to pick GART region away from iomem_resource to not let
the aperture seen from /proc/iomem?

Thanks
Baoquan

> 
> The patch below stores the location of the GART region in two variables
> (named gart_stolen_ram_start and gart_stolen_ram_end) and reverts/adapts
> parts of f296f2634920 to exclude the region from /proc/vmcore.
> 
> Passing the information via an iomem_resource (or by reserving the range
> in the e820, which would propagate into an iomem_resource) would
> reintroduce bko#72201.
> 
> Signed-off-by: Jiri Bohac <jbo...@suse.cz>
> Fixes: 707d4eefbdb3 ("Revert [PATCH] Insert GART region into resource map")
> 
> diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
> index 1d268098ac2e..324a0a19d166 100644
> --- a/arch/x86/include/asm/gart.h
> +++ b/arch/x86/include/asm/gart.h
> @@ -2,6 +2,7 @@
>  #define _ASM_X86_GART_H
>  
>  #include <asm/e820/api.h>
> +#include <linux/pci.h>
>  
>  extern void set_up_gart_resume(u32, u32);
>  
> @@ -33,6 +34,8 @@ extern int fix_aperture;
>  extern int gart_iommu_aperture;
>  extern int gart_iommu_aperture_allowed;
>  extern int gart_iommu_aperture_disabled;
> +extern u32 gart_stolen_ram_start;
> +extern u32 gart_stolen_ram_end;
>  
>  extern void early_gart_iommu_check(void);
>  extern int gart_iommu_init(void);
> @@ -43,6 +46,8 @@ extern int gart_iommu_hole_init(void);
>  #define gart_iommu_aperture            0
>  #define gart_iommu_aperture_allowed    0
>  #define gart_iommu_aperture_disabled   1
> +#define gart_stolen_ram_start               0
> +#define gart_stolen_ram_end         0
>  
>  static inline void early_gart_iommu_check(void)
>  {
> diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
> index ef2859f9fcce..bc9a9b5b88e3 100644
> --- a/arch/x86/kernel/aperture_64.c
> +++ b/arch/x86/kernel/aperture_64.c
> @@ -49,6 +49,8 @@
>  int gart_iommu_aperture;
>  int gart_iommu_aperture_disabled __initdata;
>  int gart_iommu_aperture_allowed __initdata;
> +u32 gart_stolen_ram_start;
> +u32 gart_stolen_ram_end;
>  
>  int fallback_aper_order __initdata = 1; /* 64MB */
>  int fallback_aper_force __initdata;
> @@ -87,6 +89,9 @@ static u32 __init allocate_aperture(void)
>       register_nosave_region(addr >> PAGE_SHIFT,
>                              (addr+aper_size) >> PAGE_SHIFT);
>  
> +     gart_stolen_ram_start = (u32)addr;
> +     gart_stolen_ram_end = (u32)addr + aper_size - 1;
> +
>       return (u32)addr;
>  }
>  
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 44404e2307bb..ce065d72656d 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -37,6 +37,7 @@
>  #include <asm/reboot.h>
>  #include <asm/virtext.h>
>  #include <asm/intel_pt.h>
> +#include <asm/gart.h>
>  
>  /* Alignment required for elf header segment */
>  #define ELF_CORE_HEADER_ALIGN   4096
> @@ -58,7 +59,7 @@ struct crash_elf_data {
>       struct kimage *image;
>       /*
>        * Total number of ram ranges we have after various adjustments for
> -      * crash reserved region, etc.
> +      * GART, crash reserved region etc.
>        */
>       unsigned int max_nr_ranges;
>  
> @@ -217,7 +218,6 @@ static int get_nr_ram_ranges_callback(u64 start, u64 end, 
> void *arg)
>       return 0;
>  }
>  
> -
>  /* Gather all the required information to prepare elf headers for ram 
> regions */
>  static void fill_up_crash_elf_data(struct crash_elf_data *ced,
>                                  struct kimage *image)
> @@ -231,6 +231,15 @@ static void fill_up_crash_elf_data(struct crash_elf_data 
> *ced,
>  
>       ced->max_nr_ranges = nr_ranges;
>  
> +     /*
> +      * We don't create ELF headers for GART aperture as an attempt
> +      * to dump this memory in second kernel leads to hang/crash.
> +      * If gart aperture is mapped over RAM, one needs to exclude that region
> +      * and that requires an extra phdr.
> +      */
> +     if (gart_stolen_ram_start)
> +             ced->max_nr_ranges++;
> +
>       /* Exclusion of crash region could split memory ranges */
>       ced->max_nr_ranges++;
>  
> @@ -339,6 +348,14 @@ static int elf_header_exclude_ranges(struct 
> crash_elf_data *ced,
>                       return ret;
>       }
>  
> +     /* Exclude GART region */
> +     if (gart_stolen_ram_start) {
> +             ret = exclude_mem_range(cmem, gart_stolen_ram_start,
> +                                     gart_stolen_ram_end);
> +             if (ret)
> +                     return ret;
> +     }
> +
>       return ret;
>  }
>  
> -- 
> Jiri Bohac <jbo...@suse.cz>
> SUSE Labs, Prague, Czechia
> 

Reply via email to