Re: [PATCH 1/2 v6] x86/kexec_file: add e820 entry in case e820 type string matches to io resource name

[Dave Young]

On 11/15/18 at 01:44pm, lijiang wrote:
> å 2018å11æ14æ 19:26, Borislav Petkov åé:
> > On Wed, Nov 14, 2018 at 03:29:25PM +0800, Lianbo Jiang wrote:
> >> When load the kernel image and initramfs by kexec_file_load syscall, it can
> >> not add exact e820 reserved type to kdump kernel e820 table.
> >>
> >> Kdump uses walk_iomem_res_desc() to iterate io resources, then adds matched
> >> desc to e820 table for kdump kernel. But, when convert the e820 type into
> >> the iores descriptors, several e820 types are converted to 'IORES_DES_NONE'
> >> in this function e820_type_to_iores_desc(). So the walk_iomem_res_desc()
> >> will get these unnecessary types(E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820_TYPE
> >> _KERN) when iterate io resources by the 'IORES_DES_NONE'.
> >>
> >> It needs filter out these redundant type(such as E820_TYPE_RAM/E820_TYPE_
> >> UNUSABLE/E820_TYPE_KERN) in order to add exact e820 reserved type to kdump
> >> kernel e820 table. Thus it also needs an extra checking in memmap_entry_
> >> callback() to match the e820 type and resource name.
> > 
> > Ok, it took me a while to parse what this is trying to say so let's
> > start from the top:
> > 
> > * What resource type do you do need in the second kernel?
> > 
> 
> Thanks for your comment.
> 
> The e820 reserved ranges need to be passed to the second kernel.
> 
> > * The most important question: why?
> > 
> 
> At present, the upstream kernel does not pass the e820 reserved ranges to the
> second kernel, which might cause two problems:
> 
> The first one is the MMCONFIG issue, the PCI MMCONFIG(extended mode) requires
> the reserved region otherwise it falls back to legacy mode, which might lead to
> the hot-plug device could not be recognized in kdump kernel.
> 
> Another one is that the e820 reserved ranges do not setup in kdump kernel, which
> could cause kdump can't work in some machines. To know more information, please
> refer to the [PATCH 2/2 v6] patch log.
> 
> 
> > * If it is the reserved resource, why aren't you adding
> > IORES_DESC_RESERVED or so which to look for instead of this hacky string
> > comparison?
> > 
> 
> Adding the new descriptor 'IORES_DESC_RESERVED' is also a good solution. I will

I was not sure if something else depends on IORES_DESC_NONE and if it is
easy to split it and add IORES_DESC_RESERVED

But if you can prove it is safe then it would be a better way.

Thanks
Dave