[PATCH 0/2 v8] add reserved e820 ranges to the kdump kernel e820 table

[PATCH 0/2 v8] add reserved e820 ranges to the kdump kernel e820 table

[Lianbo Jiang]

This patchset did two things:
a). add a new I/O resource descriptor 'IORES_DESC_RESERVED'

When doing kexec_file_load, the first kernel needs to pass the e820
reserved ranges to the second kernel. But kernel can not exactly
match the e820 reserved ranges when walking through the iomem resources
with the descriptor 'IORES_DESC_NONE', because several e820 types(
e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
_TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
may pass these four types to the kdump kernel, that is not desired result.

So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
for the iomem resources search interfaces. It is helpful to exactly
match the reserved resource ranges when walking through iomem resources.

In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
these code originally related to the descriptor 'IORES_DESC_NONE' need to
be updated. Otherwise, it will be easily confused and also cause some
errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
changed.

b). add the e820 reserved ranges to kdump kernel e820 table

At present, when use the kexec_file_load syscall to load the kernel image
and initramfs(for example: kexec -s -p xxx), 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 basic problem is that this device
is in PCI segment 1 and the kernel PCI probing can not find it without all
the e820 I/O reservations being present in the e820 table. And the kdump
kernel does not have those reservations because the kexec command does not
pass the I/O reservation via the "memmap=xxx" command line option. (This
problem does not show up for other vendors, as SGI is apparently the
actually fails for everyone, but devices in segment 0 are then found by
some legacy lookup method.) The workaround for this is to pass the I/O
reserved regions to the kdump kernel.

MMCONFIG(aka ECAM) space is described in the ACPI MCFG table. If you don't
have ECAM: (a) PCI devices won't work at all on non-x86 systems that use
only ECAM for config access, (b) you won't be albe to access devices on
non-0 segments, (c) you won't be able to access extended config space(
address 0x100-0xffff), which means none of the Extended Capabilities will
be available(AER, ACS, ATS, etc). [Bjorn's comment]

The second issue is that the SME kdump kernel doesn't work without the
e820 reserved ranges. When SME is active in kdump kernel, actually, those
reserved regions are still decrypted, but because those reserved ranges are
not present at all in kdump kernel e820 table, those reserved regions are
considered as encrypted, it goes wrong.

The e820 reserved range is useful in kdump kernel, so it is necessary to
pass the e820 reserved ranges to kdump kernel.

Changes since v1:
1. Modified the value of flags to "0", when walking through the whole
tree for e820 reserved ranges.

Changes since v2:
1. Modified the value of flags to "0", when walking through the whole
tree for e820 reserved ranges.
2. Modified the invalid SOB chain issue.

Changes since v3:
1. Dropped [PATCH 1/3 v3] resource: fix an error which walks through iomem
   resources. Please refer to this commit <010a93bf97c7> "resource: Fix
   find_next_iomem_res() iteration issue"

Changes since v4:
1. Improve the patch log, and add kernel log.

Changes since v5:
1. Rewrite these patches log.

Changes since v6:
1. Modify the [PATCH 1/2], and add the new I/O resource descriptor
   'IORES_DESC_RESERVED' for the iomem resources search interfaces,
   and also updates these codes relates to 'IORES_DESC_NONE'.
2. Modify the [PATCH 2/2], and walk through io resource based on the
   new descriptor 'IORES_DESC_RESERVED'.
3. Update patch log.

Changes since v7:
1. Improve patch log.
2. Improve this function __ioremap_check_desc_other().
3. Modify code comment in the __ioremap_check_desc_other()


Lianbo Jiang (2):
  resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
  x86/kexec_file: add reserved e820 ranges to kdump kernel e820 table

 arch/ia64/kernel/efi.c  |  4 ++++
 arch/x86/kernel/crash.c |  6 ++++++
 arch/x86/kernel/e820.c  |  2 +-
 arch/x86/mm/ioremap.c   | 13 ++++++++++++-
 include/linux/ioport.h  |  1 +
 kernel/resource.c       |  6 +++---
 6 files changed, 27 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Lianbo Jiang]

When doing kexec_file_load, the first kernel needs to pass the e820
reserved ranges to the second kernel. But kernel can not exactly
match the e820 reserved ranges when walking through the iomem resources
with the descriptor 'IORES_DESC_NONE', because several e820 types(
e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
_TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
may pass these four types to the kdump kernel, that is not desired result.

So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
for the iomem resources search interfaces. It is helpful to exactly
match the reserved resource ranges when walking through iomem resources.

In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
these code originally related to the descriptor 'IORES_DESC_NONE' need to
be updated. Otherwise, it will be easily confused and also cause some
errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
changed.

Suggested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 arch/ia64/kernel/efi.c |  4 ++++
 arch/x86/kernel/e820.c |  2 +-
 arch/x86/mm/ioremap.c  | 13 ++++++++++++-
 include/linux/ioport.h |  1 +
 kernel/resource.c      |  6 +++---
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 8f106638913c..1841e9b4db30 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
 				break;
 
 			case EFI_RESERVED_TYPE:
+				name = "reserved";
+				desc = IORES_DESC_RESERVED;
+				break;
+
 			case EFI_RUNTIME_SERVICES_CODE:
 			case EFI_RUNTIME_SERVICES_DATA:
 			case EFI_ACPI_RECLAIM_MEMORY:
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 50895c2f937d..57fafdafb860 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
 	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
 	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
 	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
+	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
 	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
 	case E820_TYPE_RAM:		/* Fall-through: */
 	case E820_TYPE_UNUSABLE:	/* Fall-through: */
-	case E820_TYPE_RESERVED:	/* Fall-through: */
 	default:			return IORES_DESC_NONE;
 	}
 }
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5378d10f1d31..fea2ef99415d 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
 
 static int __ioremap_check_desc_other(struct resource *res)
 {
-	return (res->desc != IORES_DESC_NONE);
+	/*
+	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
+	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
+	 * it has been changed. And the value of 'mem_flags.desc_other'
+	 * is equal to 'true' if we don't strengthen the condition in this
+	 * function, that is wrong. Because originally it is equal to
+	 * 'false' for the same reserved type.
+	 *
+	 * So, that would be nice to keep it the same as before.
+	 */
+	return ((res->desc != IORES_DESC_NONE) &&
+		(res->desc != IORES_DESC_RESERVED));
 }
 
 static int __ioremap_res_check(struct resource *res, void *arg)
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..6ed59de48bd5 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -133,6 +133,7 @@ enum {
 	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
 	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
 	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
+	IORES_DESC_RESERVED			= 8,
 };
 
 /* helpers to define resources */
diff --git a/kernel/resource.c b/kernel/resource.c
index b0fbf685c77a..f34a632c4169 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
 	res->start = start;
 	res->end = end;
 	res->flags = type | IORESOURCE_BUSY;
-	res->desc = IORES_DESC_NONE;
+	res->desc = IORES_DESC_RESERVED;
 
 	while (1) {
 
@@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
 				next_res->start = conflict->end + 1;
 				next_res->end = end;
 				next_res->flags = type | IORESOURCE_BUSY;
-				next_res->desc = IORES_DESC_NONE;
+				next_res->desc = IORES_DESC_RESERVED;
 			}
 		} else {
 			res->start = conflict->end + 1;
@@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
 			res->start = io_start;
 			res->end = io_start + io_num - 1;
 			res->flags |= IORESOURCE_BUSY;
-			res->desc = IORES_DESC_NONE;
+			res->desc = IORES_DESC_RESERVED;
 			res->child = NULL;
 			if (request_resource(parent, res) = 0)
 				reserved = x+1;
-- 
2.17.1

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Dave Young]

+ more people

On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
> When doing kexec_file_load, the first kernel needs to pass the e820
> reserved ranges to the second kernel. But kernel can not exactly
> match the e820 reserved ranges when walking through the iomem resources
> with the descriptor 'IORES_DESC_NONE', because several e820 types(
> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
> may pass these four types to the kdump kernel, that is not desired result.
> 
> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
> for the iomem resources search interfaces. It is helpful to exactly
> match the reserved resource ranges when walking through iomem resources.
> 
> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
> these code originally related to the descriptor 'IORES_DESC_NONE' need to
> be updated. Otherwise, it will be easily confused and also cause some
> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
> changed.
> 
> Suggested-by: Dave Young <dyoung@redhat.com>
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
>  arch/ia64/kernel/efi.c |  4 ++++
>  arch/x86/kernel/e820.c |  2 +-
>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>  include/linux/ioport.h |  1 +
>  kernel/resource.c      |  6 +++---
>  5 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> index 8f106638913c..1841e9b4db30 100644
> --- a/arch/ia64/kernel/efi.c
> +++ b/arch/ia64/kernel/efi.c
> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>  				break;
>  
>  			case EFI_RESERVED_TYPE:
> +				name = "reserved";

Ingo updated X86 code to use "Reserved",  I think it would be good to do
same for this case as well

> +				desc = IORES_DESC_RESERVED;
> +				break;
> +
>  			case EFI_RUNTIME_SERVICES_CODE:
>  			case EFI_RUNTIME_SERVICES_DATA:
>  			case EFI_ACPI_RECLAIM_MEMORY:

Originally, above 3 are all "reserved", so probably they all should be
IORES_DESC_RESERVED.

Can any IA64 people to review this?

> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 50895c2f937d..57fafdafb860 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>  	case E820_TYPE_RAM:		/* Fall-through: */
>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>  	default:			return IORES_DESC_NONE;
>  	}
>  }
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 5378d10f1d31..fea2ef99415d 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>  
>  static int __ioremap_check_desc_other(struct resource *res)
>  {
> -	return (res->desc != IORES_DESC_NONE);
> +	/*
> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
> +	 * it has been changed. And the value of 'mem_flags.desc_other'
> +	 * is equal to 'true' if we don't strengthen the condition in this
> +	 * function, that is wrong. Because originally it is equal to
> +	 * 'false' for the same reserved type.
> +	 *
> +	 * So, that would be nice to keep it the same as before.
> +	 */
> +	return ((res->desc != IORES_DESC_NONE) &&
> +		(res->desc != IORES_DESC_RESERVED));
>  }

Added Tom since he added the check function.  Is it possible to only
check explict valid desc types instead of exclude IORES_DESC_NONE?

>  
>  static int __ioremap_res_check(struct resource *res, void *arg)
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index da0ebaec25f0..6ed59de48bd5 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -133,6 +133,7 @@ enum {
>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
> +	IORES_DESC_RESERVED			= 8,
>  };
>  
>  /* helpers to define resources */
> diff --git a/kernel/resource.c b/kernel/resource.c
> index b0fbf685c77a..f34a632c4169 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>  	res->start = start;
>  	res->end = end;
>  	res->flags = type | IORESOURCE_BUSY;
> -	res->desc = IORES_DESC_NONE;
> +	res->desc = IORES_DESC_RESERVED;
>  
>  	while (1) {
>  
> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>  				next_res->start = conflict->end + 1;
>  				next_res->end = end;
>  				next_res->flags = type | IORESOURCE_BUSY;
> -				next_res->desc = IORES_DESC_NONE;
> +				next_res->desc = IORES_DESC_RESERVED;
>  			}
>  		} else {
>  			res->start = conflict->end + 1;
> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>  			res->start = io_start;
>  			res->end = io_start + io_num - 1;
>  			res->flags |= IORESOURCE_BUSY;
> -			res->desc = IORES_DESC_NONE;
> +			res->desc = IORES_DESC_RESERVED;
>  			res->child = NULL;
>  			if (request_resource(parent, res) = 0)
>  				reserved = x+1;
> -- 
> 2.17.1
> 


There are a lot of places call region_intersects which use DESC_NONE,
I'm not sure if needed changes accordingly.  Cced Dan and Toshi.


Thanks
Dave

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Dave Young]

Correct Toshi's email addr
On 11/30/18 at 11:37am, Dave Young wrote:
> + more people
> 
> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
> > When doing kexec_file_load, the first kernel needs to pass the e820
> > reserved ranges to the second kernel. But kernel can not exactly
> > match the e820 reserved ranges when walking through the iomem resources
> > with the descriptor 'IORES_DESC_NONE', because several e820 types(
> > e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
> > _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
> > may pass these four types to the kdump kernel, that is not desired result.
> > 
> > So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
> > for the iomem resources search interfaces. It is helpful to exactly
> > match the reserved resource ranges when walking through iomem resources.
> > 
> > In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
> > these code originally related to the descriptor 'IORES_DESC_NONE' need to
> > be updated. Otherwise, it will be easily confused and also cause some
> > errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
> > descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
> > changed.
> > 
> > Suggested-by: Dave Young <dyoung@redhat.com>
> > Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> > ---
> >  arch/ia64/kernel/efi.c |  4 ++++
> >  arch/x86/kernel/e820.c |  2 +-
> >  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
> >  include/linux/ioport.h |  1 +
> >  kernel/resource.c      |  6 +++---
> >  5 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> > index 8f106638913c..1841e9b4db30 100644
> > --- a/arch/ia64/kernel/efi.c
> > +++ b/arch/ia64/kernel/efi.c
> > @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
> >  				break;
> >  
> >  			case EFI_RESERVED_TYPE:
> > +				name = "reserved";
> 
> Ingo updated X86 code to use "Reserved",  I think it would be good to do
> same for this case as well
> 
> > +				desc = IORES_DESC_RESERVED;
> > +				break;
> > +
> >  			case EFI_RUNTIME_SERVICES_CODE:
> >  			case EFI_RUNTIME_SERVICES_DATA:
> >  			case EFI_ACPI_RECLAIM_MEMORY:
> 
> Originally, above 3 are all "reserved", so probably they all should be
> IORES_DESC_RESERVED.
> 
> Can any IA64 people to review this?
> 
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index 50895c2f937d..57fafdafb860 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
> >  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
> >  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
> >  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
> > +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
> >  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
> >  	case E820_TYPE_RAM:		/* Fall-through: */
> >  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
> > -	case E820_TYPE_RESERVED:	/* Fall-through: */
> >  	default:			return IORES_DESC_NONE;
> >  	}
> >  }
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index 5378d10f1d31..fea2ef99415d 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
> >  
> >  static int __ioremap_check_desc_other(struct resource *res)
> >  {
> > -	return (res->desc != IORES_DESC_NONE);
> > +	/*
> > +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
> > +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
> > +	 * it has been changed. And the value of 'mem_flags.desc_other'
> > +	 * is equal to 'true' if we don't strengthen the condition in this
> > +	 * function, that is wrong. Because originally it is equal to
> > +	 * 'false' for the same reserved type.
> > +	 *
> > +	 * So, that would be nice to keep it the same as before.
> > +	 */
> > +	return ((res->desc != IORES_DESC_NONE) &&
> > +		(res->desc != IORES_DESC_RESERVED));
> >  }
> 
> Added Tom since he added the check function.  Is it possible to only
> check explict valid desc types instead of exclude IORES_DESC_NONE?
> 
> >  
> >  static int __ioremap_res_check(struct resource *res, void *arg)
> > diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> > index da0ebaec25f0..6ed59de48bd5 100644
> > --- a/include/linux/ioport.h
> > +++ b/include/linux/ioport.h
> > @@ -133,6 +133,7 @@ enum {
> >  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
> >  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
> >  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
> > +	IORES_DESC_RESERVED			= 8,
> >  };
> >  
> >  /* helpers to define resources */
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index b0fbf685c77a..f34a632c4169 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
> >  	res->start = start;
> >  	res->end = end;
> >  	res->flags = type | IORESOURCE_BUSY;
> > -	res->desc = IORES_DESC_NONE;
> > +	res->desc = IORES_DESC_RESERVED;
> >  
> >  	while (1) {
> >  
> > @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
> >  				next_res->start = conflict->end + 1;
> >  				next_res->end = end;
> >  				next_res->flags = type | IORESOURCE_BUSY;
> > -				next_res->desc = IORES_DESC_NONE;
> > +				next_res->desc = IORES_DESC_RESERVED;
> >  			}
> >  		} else {
> >  			res->start = conflict->end + 1;
> > @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
> >  			res->start = io_start;
> >  			res->end = io_start + io_num - 1;
> >  			res->flags |= IORESOURCE_BUSY;
> > -			res->desc = IORES_DESC_NONE;
> > +			res->desc = IORES_DESC_RESERVED;
> >  			res->child = NULL;
> >  			if (request_resource(parent, res) = 0)
> >  				reserved = x+1;
> > -- 
> > 2.17.1
> > 
> 
> 
> There are a lot of places call region_intersects which use DESC_NONE,
> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
> 
> 
> Thanks
> Dave

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年11月30日 11:37, Dave Young 写道:
> + more people
> 

Thank you, Dave.

That would be fine to let more people review this patch.

> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>> When doing kexec_file_load, the first kernel needs to pass the e820
>> reserved ranges to the second kernel. But kernel can not exactly
>> match the e820 reserved ranges when walking through the iomem resources
>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>> may pass these four types to the kdump kernel, that is not desired result.
>>
>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>> for the iomem resources search interfaces. It is helpful to exactly
>> match the reserved resource ranges when walking through iomem resources.
>>
>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>> be updated. Otherwise, it will be easily confused and also cause some
>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>> changed.
>>
>> Suggested-by: Dave Young <dyoung@redhat.com>
>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>> ---
>>  arch/ia64/kernel/efi.c |  4 ++++
>>  arch/x86/kernel/e820.c |  2 +-
>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>  include/linux/ioport.h |  1 +
>>  kernel/resource.c      |  6 +++---
>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
>> index 8f106638913c..1841e9b4db30 100644
>> --- a/arch/ia64/kernel/efi.c
>> +++ b/arch/ia64/kernel/efi.c
>> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>>  				break;
>>  
>>  			case EFI_RESERVED_TYPE:
>> +				name = "reserved";
> 
> Ingo updated X86 code to use "Reserved",  I think it would be good to do
> same for this case as well
> 

I have noticed the changes on x86, but for IA64, i'm not sure whether it should do the same
thing, so keep it as before.

If IA64 people would like to give any comment, that will be appreciated.


>> +				desc = IORES_DESC_RESERVED;
>> +				break;
>> +
>>  			case EFI_RUNTIME_SERVICES_CODE:
>>  			case EFI_RUNTIME_SERVICES_DATA:
>>  			case EFI_ACPI_RECLAIM_MEMORY:
> 
> Originally, above 3 are all "reserved", so probably they all should be
> IORES_DESC_RESERVED.
> 

This is a good question.

If above 3 types are converted to the new descriptor 'IORES_DESC_RESERVED', how to handle
the all 'default' case? Because the all 'default' case is also converted to the descriptor
'IORES_DESC_NONE' and the name 'reserved'.

> Can any IA64 people to review this?
> 
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>> index 50895c2f937d..57fafdafb860 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
>> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>>  	case E820_TYPE_RAM:		/* Fall-through: */
>>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
>> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>>  	default:			return IORES_DESC_NONE;
>>  	}
>>  }
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index 5378d10f1d31..fea2ef99415d 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>>  
>>  static int __ioremap_check_desc_other(struct resource *res)
>>  {
>> -	return (res->desc != IORES_DESC_NONE);
>> +	/*
>> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
>> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
>> +	 * it has been changed. And the value of 'mem_flags.desc_other'
>> +	 * is equal to 'true' if we don't strengthen the condition in this
>> +	 * function, that is wrong. Because originally it is equal to
>> +	 * 'false' for the same reserved type.
>> +	 *
>> +	 * So, that would be nice to keep it the same as before.
>> +	 */
>> +	return ((res->desc != IORES_DESC_NONE) &&
>> +		(res->desc != IORES_DESC_RESERVED));
>>  }
> 
> Added Tom since he added the check function.  Is it possible to only
> check explict valid desc types instead of exclude IORES_DESC_NONE?
> 

So sorry that i forgot to add Tom.

I'm looking forward to Tom's reply. If i made a mistake, please help me
to point out. Thanks a lot.

Regards,
Lianbo

>>  
>>  static int __ioremap_res_check(struct resource *res, void *arg)
>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>> index da0ebaec25f0..6ed59de48bd5 100644
>> --- a/include/linux/ioport.h
>> +++ b/include/linux/ioport.h
>> @@ -133,6 +133,7 @@ enum {
>>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
>> +	IORES_DESC_RESERVED			= 8,
>>  };
>>  
>>  /* helpers to define resources */
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index b0fbf685c77a..f34a632c4169 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>  	res->start = start;
>>  	res->end = end;
>>  	res->flags = type | IORESOURCE_BUSY;
>> -	res->desc = IORES_DESC_NONE;
>> +	res->desc = IORES_DESC_RESERVED;
>>  
>>  	while (1) {
>>  
>> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>  				next_res->start = conflict->end + 1;
>>  				next_res->end = end;
>>  				next_res->flags = type | IORESOURCE_BUSY;
>> -				next_res->desc = IORES_DESC_NONE;
>> +				next_res->desc = IORES_DESC_RESERVED;
>>  			}
>>  		} else {
>>  			res->start = conflict->end + 1;
>> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>>  			res->start = io_start;
>>  			res->end = io_start + io_num - 1;
>>  			res->flags |= IORESOURCE_BUSY;
>> -			res->desc = IORES_DESC_NONE;
>> +			res->desc = IORES_DESC_RESERVED;
>>  			res->child = NULL;
>>  			if (request_resource(parent, res) = 0)
>>  				reserved = x+1;
>> -- 
>> 2.17.1
>>
> 
> 
> There are a lot of places call region_intersects which use DESC_NONE,
> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
> 
> 
> Thanks
> Dave
> 

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Borislav Petkov]

On Fri, Nov 30, 2018 at 03:04:44PM +0800, lijiang wrote:
> I have noticed the changes on x86, but for IA64, i'm not sure whether it should do the same
> thing, so keep it as before.
> 
> If IA64 people would like to give any comment, that will be appreciated.

I think you should not touch ia64 and not make Tony unnecessarily power
up the old rust.

:-)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年12月07日 04:11, Borislav Petkov 写道:
> On Fri, Nov 30, 2018 at 03:04:44PM +0800, lijiang wrote:
>> I have noticed the changes on x86, but for IA64, i'm not sure whether it should do the same
>> thing, so keep it as before.
>>
>> If IA64 people would like to give any comment, that will be appreciated.
> 
> I think you should not touch ia64 and not make Tony unnecessarily power
> up the old rust.
> 
> :-)
> 
Ok, it's good to me. And i will get rid of these changes for ia64 in patch v9.

Thanks.
Lianbo

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年12月07日 04:11, Borislav Petkov 写道:
> On Fri, Nov 30, 2018 at 03:04:44PM +0800, lijiang wrote:
>> I have noticed the changes on x86, but for IA64, i'm not sure whether it should do the same
>> thing, so keep it as before.
>>
>> If IA64 people would like to give any comment, that will be appreciated.
> 
> I think you should not touch ia64 and not make Tony unnecessarily power
> up the old rust.
> 

Ok, i will get rid of previous changes to IA64 in next post.

Thanks.

> :-)
> 

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Lendacky, Thomas]

T24gMTEvMjkvMjAxOCAwOTozNyBQTSwgRGF2ZSBZb3VuZyB3cm90ZToNCj4gKyBtb3JlIHBlb3Bs
ZQ0KPiANCj4gT24gMTEvMjkvMTggYXQgMDQ6MDlwbSwgTGlhbmJvIEppYW5nIHdyb3RlOg0KPj4g
V2hlbiBkb2luZyBrZXhlY19maWxlX2xvYWQsIHRoZSBmaXJzdCBrZXJuZWwgbmVlZHMgdG8gcGFz
cyB0aGUgZTgyMA0KPj4gcmVzZXJ2ZWQgcmFuZ2VzIHRvIHRoZSBzZWNvbmQga2VybmVsLiBCdXQg
a2VybmVsIGNhbiBub3QgZXhhY3RseQ0KPj4gbWF0Y2ggdGhlIGU4MjAgcmVzZXJ2ZWQgcmFuZ2Vz
IHdoZW4gd2Fsa2luZyB0aHJvdWdoIHRoZSBpb21lbSByZXNvdXJjZXMNCj4+IHdpdGggdGhlIGRl
c2NyaXB0b3IgJ0lPUkVTX0RFU0NfTk9ORScsIGJlY2F1c2Ugc2V2ZXJhbCBlODIwIHR5cGVzKA0K
Pj4gZS5nLiBFODIwX1RZUEVfUkVTRVJWRURfS0VSTi9FODIwX1RZUEVfUkFNL0U4MjBfVFlQRV9V
TlVTQUJMRS9FODIwDQo+PiBfVFlQRV9SRVNFUlZFRCkgYXJlIGNvbnZlcnRlZCB0byB0aGUgZGVz
Y3JpcHRvciAnSU9SRVNfREVTQ19OT05FJy4gSXQNCj4+IG1heSBwYXNzIHRoZXNlIGZvdXIgdHlw
ZXMgdG8gdGhlIGtkdW1wIGtlcm5lbCwgdGhhdCBpcyBub3QgZGVzaXJlZCByZXN1bHQuDQo+Pg0K
Pj4gU28sIHRoaXMgcGF0Y2ggYWRkcyBhIG5ldyBJL08gcmVzb3VyY2UgZGVzY3JpcHRvciAnSU9S
RVNfREVTQ19SRVNFUlZFRCcNCj4+IGZvciB0aGUgaW9tZW0gcmVzb3VyY2VzIHNlYXJjaCBpbnRl
cmZhY2VzLiBJdCBpcyBoZWxwZnVsIHRvIGV4YWN0bHkNCj4+IG1hdGNoIHRoZSByZXNlcnZlZCBy
ZXNvdXJjZSByYW5nZXMgd2hlbiB3YWxraW5nIHRocm91Z2ggaW9tZW0gcmVzb3VyY2VzLg0KPj4N
Cj4+IEluIGFkZGl0aW9uLCBzaW5jZSB0aGUgbmV3IGRlc2NyaXB0b3IgJ0lPUkVTX0RFU0NfUkVT
RVJWRUQnIGlzIGludHJvZHVjZWQsDQo+PiB0aGVzZSBjb2RlIG9yaWdpbmFsbHkgcmVsYXRlZCB0
byB0aGUgZGVzY3JpcHRvciAnSU9SRVNfREVTQ19OT05FJyBuZWVkIHRvDQo+PiBiZSB1cGRhdGVk
LiBPdGhlcndpc2UsIGl0IHdpbGwgYmUgZWFzaWx5IGNvbmZ1c2VkIGFuZCBhbHNvIGNhdXNlIHNv
bWUNCj4+IGVycm9ycy4gQmVjYXVzZSB0aGUgJ0U4MjBfVFlQRV9SRVNFUlZFRCcgdHlwZSBpcyBj
b252ZXJ0ZWQgdG8gdGhlIG5ldw0KPj4gZGVzY3JpcHRvciAnSU9SRVNfREVTQ19SRVNFUlZFRCcg
aW5zdGVhZCBvZiAnSU9SRVNfREVTQ19OT05FJywgaXQgaGFzIGJlZW4NCj4+IGNoYW5nZWQuDQo+
Pg0KPj4gU3VnZ2VzdGVkLWJ5OiBEYXZlIFlvdW5nIDxkeW91bmdAcmVkaGF0LmNvbT4NCj4+IFNp
Z25lZC1vZmYtYnk6IExpYW5ibyBKaWFuZyA8bGlqaWFuZ0ByZWRoYXQuY29tPg0KPj4gLS0tDQo+
PiAgYXJjaC9pYTY0L2tlcm5lbC9lZmkuYyB8ICA0ICsrKysNCj4+ICBhcmNoL3g4Ni9rZXJuZWwv
ZTgyMC5jIHwgIDIgKy0NCj4+ICBhcmNoL3g4Ni9tbS9pb3JlbWFwLmMgIHwgMTMgKysrKysrKysr
KysrLQ0KPj4gIGluY2x1ZGUvbGludXgvaW9wb3J0LmggfCAgMSArDQo+PiAga2VybmVsL3Jlc291
cmNlLmMgICAgICB8ICA2ICsrKy0tLQ0KPj4gIDUgZmlsZXMgY2hhbmdlZCwgMjEgaW5zZXJ0aW9u
cygrKSwgNSBkZWxldGlvbnMoLSkNCj4+DQo+PiBkaWZmIC0tZ2l0IGEvYXJjaC9pYTY0L2tlcm5l
bC9lZmkuYyBiL2FyY2gvaWE2NC9rZXJuZWwvZWZpLmMNCj4+IGluZGV4IDhmMTA2NjM4OTEzYy4u
MTg0MWU5YjRkYjMwIDEwMDY0NA0KPj4gLS0tIGEvYXJjaC9pYTY0L2tlcm5lbC9lZmkuYw0KPj4g
KysrIGIvYXJjaC9pYTY0L2tlcm5lbC9lZmkuYw0KPj4gQEAgLTEyMzEsNiArMTIzMSwxMCBAQCBl
ZmlfaW5pdGlhbGl6ZV9pb21lbV9yZXNvdXJjZXMoc3RydWN0IHJlc291cmNlICpjb2RlX3Jlc291
cmNlLA0KPj4gIAkJCQlicmVhazsNCj4+ICANCj4+ICAJCQljYXNlIEVGSV9SRVNFUlZFRF9UWVBF
Og0KPj4gKwkJCQluYW1lID0gInJlc2VydmVkIjsNCj4gDQo+IEluZ28gdXBkYXRlZCBYODYgY29k
ZSB0byB1c2UgIlJlc2VydmVkIiwgIEkgdGhpbmsgaXQgd291bGQgYmUgZ29vZCB0byBkbw0KPiBz
YW1lIGZvciB0aGlzIGNhc2UgYXMgd2VsbA0KPiANCj4+ICsJCQkJZGVzYyA9IElPUkVTX0RFU0Nf
UkVTRVJWRUQ7DQo+PiArCQkJCWJyZWFrOw0KPj4gKw0KPj4gIAkJCWNhc2UgRUZJX1JVTlRJTUVf
U0VSVklDRVNfQ09ERToNCj4+ICAJCQljYXNlIEVGSV9SVU5USU1FX1NFUlZJQ0VTX0RBVEE6DQo+
PiAgCQkJY2FzZSBFRklfQUNQSV9SRUNMQUlNX01FTU9SWToNCj4gDQo+IE9yaWdpbmFsbHksIGFi
b3ZlIDMgYXJlIGFsbCAicmVzZXJ2ZWQiLCBzbyBwcm9iYWJseSB0aGV5IGFsbCBzaG91bGQgYmUN
Cj4gSU9SRVNfREVTQ19SRVNFUlZFRC4NCj4gDQo+IENhbiBhbnkgSUE2NCBwZW9wbGUgdG8gcmV2
aWV3IHRoaXM/DQo+IA0KPj4gZGlmZiAtLWdpdCBhL2FyY2gveDg2L2tlcm5lbC9lODIwLmMgYi9h
cmNoL3g4Ni9rZXJuZWwvZTgyMC5jDQo+PiBpbmRleCA1MDg5NWMyZjkzN2QuLjU3ZmFmZGFmYjg2
MCAxMDA2NDQNCj4+IC0tLSBhL2FyY2gveDg2L2tlcm5lbC9lODIwLmMNCj4+ICsrKyBiL2FyY2gv
eDg2L2tlcm5lbC9lODIwLmMNCj4+IEBAIC0xMDQ4LDEwICsxMDQ4LDEwIEBAIHN0YXRpYyB1bnNp
Z25lZCBsb25nIF9faW5pdCBlODIwX3R5cGVfdG9faW9yZXNfZGVzYyhzdHJ1Y3QgZTgyMF9lbnRy
eSAqZW50cnkpDQo+PiAgCWNhc2UgRTgyMF9UWVBFX05WUzoJCXJldHVybiBJT1JFU19ERVNDX0FD
UElfTlZfU1RPUkFHRTsNCj4+ICAJY2FzZSBFODIwX1RZUEVfUE1FTToJCXJldHVybiBJT1JFU19E
RVNDX1BFUlNJU1RFTlRfTUVNT1JZOw0KPj4gIAljYXNlIEU4MjBfVFlQRV9QUkFNOgkJcmV0dXJu
IElPUkVTX0RFU0NfUEVSU0lTVEVOVF9NRU1PUllfTEVHQUNZOw0KPj4gKwljYXNlIEU4MjBfVFlQ
RV9SRVNFUlZFRDoJcmV0dXJuIElPUkVTX0RFU0NfUkVTRVJWRUQ7DQo+PiAgCWNhc2UgRTgyMF9U
WVBFX1JFU0VSVkVEX0tFUk46CS8qIEZhbGwtdGhyb3VnaDogKi8NCj4+ICAJY2FzZSBFODIwX1RZ
UEVfUkFNOgkJLyogRmFsbC10aHJvdWdoOiAqLw0KPj4gIAljYXNlIEU4MjBfVFlQRV9VTlVTQUJM
RToJLyogRmFsbC10aHJvdWdoOiAqLw0KPj4gLQljYXNlIEU4MjBfVFlQRV9SRVNFUlZFRDoJLyog
RmFsbC10aHJvdWdoOiAqLw0KPj4gIAlkZWZhdWx0OgkJCXJldHVybiBJT1JFU19ERVNDX05PTkU7
DQo+PiAgCX0NCj4+ICB9DQo+PiBkaWZmIC0tZ2l0IGEvYXJjaC94ODYvbW0vaW9yZW1hcC5jIGIv
YXJjaC94ODYvbW0vaW9yZW1hcC5jDQo+PiBpbmRleCA1Mzc4ZDEwZjFkMzEuLmZlYTJlZjk5NDE1
ZCAxMDA2NDQNCj4+IC0tLSBhL2FyY2gveDg2L21tL2lvcmVtYXAuYw0KPj4gKysrIGIvYXJjaC94
ODYvbW0vaW9yZW1hcC5jDQo+PiBAQCAtODMsNyArODMsMTggQEAgc3RhdGljIGJvb2wgX19pb3Jl
bWFwX2NoZWNrX3JhbShzdHJ1Y3QgcmVzb3VyY2UgKnJlcykNCj4+ICANCj4+ICBzdGF0aWMgaW50
IF9faW9yZW1hcF9jaGVja19kZXNjX290aGVyKHN0cnVjdCByZXNvdXJjZSAqcmVzKQ0KPj4gIHsN
Cj4+IC0JcmV0dXJuIChyZXMtPmRlc2MgIT0gSU9SRVNfREVTQ19OT05FKTsNCj4+ICsJLyoNCj4+
ICsJICogQnV0IG5vdywgdGhlICdFODIwX1RZUEVfUkVTRVJWRUQnIHR5cGUgaXMgY29udmVydGVk
IHRvIHRoZSBuZXcNCj4+ICsJICogZGVzY3JpcHRvciAnSU9SRVNfREVTQ19SRVNFUlZFRCcgaW5z
dGVhZCBvZiAnSU9SRVNfREVTQ19OT05FJywNCj4+ICsJICogaXQgaGFzIGJlZW4gY2hhbmdlZC4g
QW5kIHRoZSB2YWx1ZSBvZiAnbWVtX2ZsYWdzLmRlc2Nfb3RoZXInDQo+PiArCSAqIGlzIGVxdWFs
IHRvICd0cnVlJyBpZiB3ZSBkb24ndCBzdHJlbmd0aGVuIHRoZSBjb25kaXRpb24gaW4gdGhpcw0K
Pj4gKwkgKiBmdW5jdGlvbiwgdGhhdCBpcyB3cm9uZy4gQmVjYXVzZSBvcmlnaW5hbGx5IGl0IGlz
IGVxdWFsIHRvDQo+PiArCSAqICdmYWxzZScgZm9yIHRoZSBzYW1lIHJlc2VydmVkIHR5cGUuDQo+
PiArCSAqDQo+PiArCSAqIFNvLCB0aGF0IHdvdWxkIGJlIG5pY2UgdG8ga2VlcCBpdCB0aGUgc2Ft
ZSBhcyBiZWZvcmUuDQo+PiArCSAqLw0KPj4gKwlyZXR1cm4gKChyZXMtPmRlc2MgIT0gSU9SRVNf
REVTQ19OT05FKSAmJg0KPj4gKwkJKHJlcy0+ZGVzYyAhPSBJT1JFU19ERVNDX1JFU0VSVkVEKSk7
DQo+PiAgfQ0KPiANCj4gQWRkZWQgVG9tIHNpbmNlIGhlIGFkZGVkIHRoZSBjaGVjayBmdW5jdGlv
bi4gIElzIGl0IHBvc3NpYmxlIHRvIG9ubHkNCj4gY2hlY2sgZXhwbGljdCB2YWxpZCBkZXNjIHR5
cGVzIGluc3RlYWQgb2YgZXhjbHVkZSBJT1JFU19ERVNDX05PTkU/DQoNClNvcnJ5IGZvciB0aGUg
ZGVsYXkuLi4NCg0KVGhlIG9yaWdpbmFsIGludGVudCBvZiB0aGUgY2hlY2sgd2FzIHRvIG1hcCBt
b3N0IG1lbW9yeSBhcyBlbmNyeXB0ZWQgdW5kZXINClNFViBpZiBpdCB3YXMgbWFya2VkIHdpdGgg
YSBzcGVjaWZpYyBkZXNjcmlwdG9yLCBzaW5jZSBpdCB3YXMgbGlrZWx5IHRvDQpub3QgYmUgTU1J
Ty4gSSB0cmllZCBjb252ZXJ0aW5nIG1vc3QgdGhpbmdzIHRoYXQgbWFwcGVkIG1lbW9yeSB0byBt
ZW1yZW1hcA0KdnMgaW9yZW1hcCwgYnV0IEFDUEkgd2FzIG9uZSBhcmVhIHRoYXQgSSBsZWZ0IGFs
b25lIGFuZCB0aGlzIGNoZWNrIGNhdGNoZXMNCnRoZSBtYXBwaW5nIG9mIHRoZSBBQ1BJIHRhYmxl
cy4gSSBzdXBwb3NlIGl0J3MgcG9zc2libGUgdG8gY2hhbmdlIHRoaXMgdG8NCmNoZWNrIGp1c3Qg
Zm9yIElPUkVTX0RFU0NfQUNQSV8qIHZhbHVlcywgYnV0IEkgd291bGQgaGF2ZSB0byBkbyBzb21l
DQp0ZXN0aW5nLg0KDQpUaGFua3MsDQpUb20NCg0KPiANCj4+ICANCj4+ICBzdGF0aWMgaW50IF9f
aW9yZW1hcF9yZXNfY2hlY2soc3RydWN0IHJlc291cmNlICpyZXMsIHZvaWQgKmFyZykNCj4+IGRp
ZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L2lvcG9ydC5oIGIvaW5jbHVkZS9saW51eC9pb3BvcnQu
aA0KPj4gaW5kZXggZGEwZWJhZWMyNWYwLi42ZWQ1OWRlNDhiZDUgMTAwNjQ0DQo+PiAtLS0gYS9p
bmNsdWRlL2xpbnV4L2lvcG9ydC5oDQo+PiArKysgYi9pbmNsdWRlL2xpbnV4L2lvcG9ydC5oDQo+
PiBAQCAtMTMzLDYgKzEzMyw3IEBAIGVudW0gew0KPj4gIAlJT1JFU19ERVNDX1BFUlNJU1RFTlRf
TUVNT1JZX0xFR0FDWQk9IDUsDQo+PiAgCUlPUkVTX0RFU0NfREVWSUNFX1BSSVZBVEVfTUVNT1JZ
CT0gNiwNCj4+ICAJSU9SRVNfREVTQ19ERVZJQ0VfUFVCTElDX01FTU9SWQkJPSA3LA0KPj4gKwlJ
T1JFU19ERVNDX1JFU0VSVkVECQkJPSA4LA0KPj4gIH07DQo+PiAgDQo+PiAgLyogaGVscGVycyB0
byBkZWZpbmUgcmVzb3VyY2VzICovDQo+PiBkaWZmIC0tZ2l0IGEva2VybmVsL3Jlc291cmNlLmMg
Yi9rZXJuZWwvcmVzb3VyY2UuYw0KPj4gaW5kZXggYjBmYmY2ODVjNzdhLi5mMzRhNjMyYzQxNjkg
MTAwNjQ0DQo+PiAtLS0gYS9rZXJuZWwvcmVzb3VyY2UuYw0KPj4gKysrIGIva2VybmVsL3Jlc291
cmNlLmMNCj4+IEBAIC05OTQsNyArOTk0LDcgQEAgX19yZXNlcnZlX3JlZ2lvbl93aXRoX3NwbGl0
KHN0cnVjdCByZXNvdXJjZSAqcm9vdCwgcmVzb3VyY2Vfc2l6ZV90IHN0YXJ0LA0KPj4gIAlyZXMt
PnN0YXJ0ID0gc3RhcnQ7DQo+PiAgCXJlcy0+ZW5kID0gZW5kOw0KPj4gIAlyZXMtPmZsYWdzID0g
dHlwZSB8IElPUkVTT1VSQ0VfQlVTWTsNCj4+IC0JcmVzLT5kZXNjID0gSU9SRVNfREVTQ19OT05F
Ow0KPj4gKwlyZXMtPmRlc2MgPSBJT1JFU19ERVNDX1JFU0VSVkVEOw0KPj4gIA0KPj4gIAl3aGls
ZSAoMSkgew0KPj4gIA0KPj4gQEAgLTEwMjksNyArMTAyOSw3IEBAIF9fcmVzZXJ2ZV9yZWdpb25f
d2l0aF9zcGxpdChzdHJ1Y3QgcmVzb3VyY2UgKnJvb3QsIHJlc291cmNlX3NpemVfdCBzdGFydCwN
Cj4+ICAJCQkJbmV4dF9yZXMtPnN0YXJ0ID0gY29uZmxpY3QtPmVuZCArIDE7DQo+PiAgCQkJCW5l
eHRfcmVzLT5lbmQgPSBlbmQ7DQo+PiAgCQkJCW5leHRfcmVzLT5mbGFncyA9IHR5cGUgfCBJT1JF
U09VUkNFX0JVU1k7DQo+PiAtCQkJCW5leHRfcmVzLT5kZXNjID0gSU9SRVNfREVTQ19OT05FOw0K
Pj4gKwkJCQluZXh0X3Jlcy0+ZGVzYyA9IElPUkVTX0RFU0NfUkVTRVJWRUQ7DQo+PiAgCQkJfQ0K
Pj4gIAkJfSBlbHNlIHsNCj4+ICAJCQlyZXMtPnN0YXJ0ID0gY29uZmxpY3QtPmVuZCArIDE7DQo+
PiBAQCAtMTQ3Nyw3ICsxNDc3LDcgQEAgc3RhdGljIGludCBfX2luaXQgcmVzZXJ2ZV9zZXR1cChj
aGFyICpzdHIpDQo+PiAgCQkJcmVzLT5zdGFydCA9IGlvX3N0YXJ0Ow0KPj4gIAkJCXJlcy0+ZW5k
ID0gaW9fc3RhcnQgKyBpb19udW0gLSAxOw0KPj4gIAkJCXJlcy0+ZmxhZ3MgfD0gSU9SRVNPVVJD
RV9CVVNZOw0KPj4gLQkJCXJlcy0+ZGVzYyA9IElPUkVTX0RFU0NfTk9ORTsNCj4+ICsJCQlyZXMt
PmRlc2MgPSBJT1JFU19ERVNDX1JFU0VSVkVEOw0KPj4gIAkJCXJlcy0+Y2hpbGQgPSBOVUxMOw0K
Pj4gIAkJCWlmIChyZXF1ZXN0X3Jlc291cmNlKHBhcmVudCwgcmVzKSA9PSAwKQ0KPj4gIAkJCQly
ZXNlcnZlZCA9IHgrMTsNCj4+IC0tIA0KPj4gMi4xNy4xDQo+Pg0KPiANCj4gDQo+IFRoZXJlIGFy
ZSBhIGxvdCBvZiBwbGFjZXMgY2FsbCByZWdpb25faW50ZXJzZWN0cyB3aGljaCB1c2UgREVTQ19O
T05FLA0KPiBJJ20gbm90IHN1cmUgaWYgbmVlZGVkIGNoYW5nZXMgYWNjb3JkaW5nbHkuICBDY2Vk
IERhbiBhbmQgVG9zaGkuDQo+IA0KPiANCj4gVGhhbmtzDQo+IERhdmUNCj4gDQo

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年12月05日 05:33, Lendacky, Thomas 写道:
> On 11/29/2018 09:37 PM, Dave Young wrote:
>> + more people
>>
>> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>>> When doing kexec_file_load, the first kernel needs to pass the e820
>>> reserved ranges to the second kernel. But kernel can not exactly
>>> match the e820 reserved ranges when walking through the iomem resources
>>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>>> may pass these four types to the kdump kernel, that is not desired result.
>>>
>>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>>> for the iomem resources search interfaces. It is helpful to exactly
>>> match the reserved resource ranges when walking through iomem resources.
>>>
>>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>>> be updated. Otherwise, it will be easily confused and also cause some
>>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>>> changed.
>>>
>>> Suggested-by: Dave Young <dyoung@redhat.com>
>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>> ---
>>>  arch/ia64/kernel/efi.c |  4 ++++
>>>  arch/x86/kernel/e820.c |  2 +-
>>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>>  include/linux/ioport.h |  1 +
>>>  kernel/resource.c      |  6 +++---
>>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
>>> index 8f106638913c..1841e9b4db30 100644
>>> --- a/arch/ia64/kernel/efi.c
>>> +++ b/arch/ia64/kernel/efi.c
>>> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>>>  				break;
>>>  
>>>  			case EFI_RESERVED_TYPE:
>>> +				name = "reserved";
>>
>> Ingo updated X86 code to use "Reserved",  I think it would be good to do
>> same for this case as well
>>
>>> +				desc = IORES_DESC_RESERVED;
>>> +				break;
>>> +
>>>  			case EFI_RUNTIME_SERVICES_CODE:
>>>  			case EFI_RUNTIME_SERVICES_DATA:
>>>  			case EFI_ACPI_RECLAIM_MEMORY:
>>
>> Originally, above 3 are all "reserved", so probably they all should be
>> IORES_DESC_RESERVED.
>>
>> Can any IA64 people to review this?
>>
>>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>>> index 50895c2f937d..57fafdafb860 100644
>>> --- a/arch/x86/kernel/e820.c
>>> +++ b/arch/x86/kernel/e820.c
>>> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>>>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>>>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>>>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
>>> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>>>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>>>  	case E820_TYPE_RAM:		/* Fall-through: */
>>>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
>>> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>>>  	default:			return IORES_DESC_NONE;
>>>  	}
>>>  }
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index 5378d10f1d31..fea2ef99415d 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>>>  
>>>  static int __ioremap_check_desc_other(struct resource *res)
>>>  {
>>> -	return (res->desc != IORES_DESC_NONE);
>>> +	/*
>>> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
>>> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
>>> +	 * it has been changed. And the value of 'mem_flags.desc_other'
>>> +	 * is equal to 'true' if we don't strengthen the condition in this
>>> +	 * function, that is wrong. Because originally it is equal to
>>> +	 * 'false' for the same reserved type.
>>> +	 *
>>> +	 * So, that would be nice to keep it the same as before.
>>> +	 */
>>> +	return ((res->desc != IORES_DESC_NONE) &&
>>> +		(res->desc != IORES_DESC_RESERVED));
>>>  }
>>
>> Added Tom since he added the check function.  Is it possible to only
>> check explict valid desc types instead of exclude IORES_DESC_NONE?
> 
> Sorry for the delay...
> 
> The original intent of the check was to map most memory as encrypted under
> SEV if it was marked with a specific descriptor, since it was likely to
> not be MMIO. I tried converting most things that mapped memory to memremap
> vs ioremap, but ACPI was one area that I left alone and this check catches
> the mapping of the ACPI tables. I suppose it's possible to change this to
> check just for IORES_DESC_ACPI_* values, but I would have to do some
> testing.
> 
 
I'm looking forward to the result of your test about this change.
Once changed, this patch also needs to be changed accordingly.

Thank you, Tom.

Lianbo
> Thanks,
> Tom
> 
>>
>>>  
>>>  static int __ioremap_res_check(struct resource *res, void *arg)
>>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>> index da0ebaec25f0..6ed59de48bd5 100644
>>> --- a/include/linux/ioport.h
>>> +++ b/include/linux/ioport.h
>>> @@ -133,6 +133,7 @@ enum {
>>>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>>>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>>>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
>>> +	IORES_DESC_RESERVED			= 8,
>>>  };
>>>  
>>>  /* helpers to define resources */
>>> diff --git a/kernel/resource.c b/kernel/resource.c
>>> index b0fbf685c77a..f34a632c4169 100644
>>> --- a/kernel/resource.c
>>> +++ b/kernel/resource.c
>>> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  	res->start = start;
>>>  	res->end = end;
>>>  	res->flags = type | IORESOURCE_BUSY;
>>> -	res->desc = IORES_DESC_NONE;
>>> +	res->desc = IORES_DESC_RESERVED;
>>>  
>>>  	while (1) {
>>>  
>>> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  				next_res->start = conflict->end + 1;
>>>  				next_res->end = end;
>>>  				next_res->flags = type | IORESOURCE_BUSY;
>>> -				next_res->desc = IORES_DESC_NONE;
>>> +				next_res->desc = IORES_DESC_RESERVED;
>>>  			}
>>>  		} else {
>>>  			res->start = conflict->end + 1;
>>> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>>>  			res->start = io_start;
>>>  			res->end = io_start + io_num - 1;
>>>  			res->flags |= IORESOURCE_BUSY;
>>> -			res->desc = IORES_DESC_NONE;
>>> +			res->desc = IORES_DESC_RESERVED;
>>>  			res->child = NULL;
>>>  			if (request_resource(parent, res) = 0)
>>>  				reserved = x+1;
>>> -- 
>>> 2.17.1
>>>
>>
>>
>> There are a lot of places call region_intersects which use DESC_NONE,
>> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
>>
>>
>> Thanks
>> Dave
>>

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年12月05日 05:33, Lendacky, Thomas 写道:
> On 11/29/2018 09:37 PM, Dave Young wrote:
>> + more people
>>
>> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>>> When doing kexec_file_load, the first kernel needs to pass the e820
>>> reserved ranges to the second kernel. But kernel can not exactly
>>> match the e820 reserved ranges when walking through the iomem resources
>>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>>> may pass these four types to the kdump kernel, that is not desired result.
>>>
>>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>>> for the iomem resources search interfaces. It is helpful to exactly
>>> match the reserved resource ranges when walking through iomem resources.
>>>
>>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>>> be updated. Otherwise, it will be easily confused and also cause some
>>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>>> changed.
>>>
>>> Suggested-by: Dave Young <dyoung@redhat.com>
>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>> ---
>>>  arch/ia64/kernel/efi.c |  4 ++++
>>>  arch/x86/kernel/e820.c |  2 +-
>>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>>  include/linux/ioport.h |  1 +
>>>  kernel/resource.c      |  6 +++---
>>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
>>> index 8f106638913c..1841e9b4db30 100644
>>> --- a/arch/ia64/kernel/efi.c
>>> +++ b/arch/ia64/kernel/efi.c
>>> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>>>  				break;
>>>  
>>>  			case EFI_RESERVED_TYPE:
>>> +				name = "reserved";
>>
>> Ingo updated X86 code to use "Reserved",  I think it would be good to do
>> same for this case as well
>>
>>> +				desc = IORES_DESC_RESERVED;
>>> +				break;
>>> +
>>>  			case EFI_RUNTIME_SERVICES_CODE:
>>>  			case EFI_RUNTIME_SERVICES_DATA:
>>>  			case EFI_ACPI_RECLAIM_MEMORY:
>>
>> Originally, above 3 are all "reserved", so probably they all should be
>> IORES_DESC_RESERVED.
>>
>> Can any IA64 people to review this?
>>
>>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>>> index 50895c2f937d..57fafdafb860 100644
>>> --- a/arch/x86/kernel/e820.c
>>> +++ b/arch/x86/kernel/e820.c
>>> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>>>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>>>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>>>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
>>> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>>>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>>>  	case E820_TYPE_RAM:		/* Fall-through: */
>>>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
>>> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>>>  	default:			return IORES_DESC_NONE;
>>>  	}
>>>  }
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index 5378d10f1d31..fea2ef99415d 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>>>  
>>>  static int __ioremap_check_desc_other(struct resource *res)
>>>  {
>>> -	return (res->desc != IORES_DESC_NONE);
>>> +	/*
>>> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
>>> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
>>> +	 * it has been changed. And the value of 'mem_flags.desc_other'
>>> +	 * is equal to 'true' if we don't strengthen the condition in this
>>> +	 * function, that is wrong. Because originally it is equal to
>>> +	 * 'false' for the same reserved type.
>>> +	 *
>>> +	 * So, that would be nice to keep it the same as before.
>>> +	 */
>>> +	return ((res->desc != IORES_DESC_NONE) &&
>>> +		(res->desc != IORES_DESC_RESERVED));
>>>  }
>>
>> Added Tom since he added the check function.  Is it possible to only
>> check explict valid desc types instead of exclude IORES_DESC_NONE?
> 
> Sorry for the delay...
> 
> The original intent of the check was to map most memory as encrypted under
> SEV if it was marked with a specific descriptor, since it was likely to
> not be MMIO. I tried converting most things that mapped memory to memremap
> vs ioremap, but ACPI was one area that I left alone and this check catches
> the mapping of the ACPI tables. I suppose it's possible to change this to
> check just for IORES_DESC_ACPI_* values, but I would have to do some
> testing.
> 

If you have any updates, would you mind sharing the test results about this
issue?

Thank you, Tom.

Lianbo

> Thanks,
> Tom
> 
>>
>>>  
>>>  static int __ioremap_res_check(struct resource *res, void *arg)
>>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>> index da0ebaec25f0..6ed59de48bd5 100644
>>> --- a/include/linux/ioport.h
>>> +++ b/include/linux/ioport.h
>>> @@ -133,6 +133,7 @@ enum {
>>>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>>>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>>>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
>>> +	IORES_DESC_RESERVED			= 8,
>>>  };
>>>  
>>>  /* helpers to define resources */
>>> diff --git a/kernel/resource.c b/kernel/resource.c
>>> index b0fbf685c77a..f34a632c4169 100644
>>> --- a/kernel/resource.c
>>> +++ b/kernel/resource.c
>>> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  	res->start = start;
>>>  	res->end = end;
>>>  	res->flags = type | IORESOURCE_BUSY;
>>> -	res->desc = IORES_DESC_NONE;
>>> +	res->desc = IORES_DESC_RESERVED;
>>>  
>>>  	while (1) {
>>>  
>>> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  				next_res->start = conflict->end + 1;
>>>  				next_res->end = end;
>>>  				next_res->flags = type | IORESOURCE_BUSY;
>>> -				next_res->desc = IORES_DESC_NONE;
>>> +				next_res->desc = IORES_DESC_RESERVED;
>>>  			}
>>>  		} else {
>>>  			res->start = conflict->end + 1;
>>> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>>>  			res->start = io_start;
>>>  			res->end = io_start + io_num - 1;
>>>  			res->flags |= IORESOURCE_BUSY;
>>> -			res->desc = IORES_DESC_NONE;
>>> +			res->desc = IORES_DESC_RESERVED;
>>>  			res->child = NULL;
>>>  			if (request_resource(parent, res) = 0)
>>>  				reserved = x+1;
>>> -- 
>>> 2.17.1
>>>
>>
>>
>> There are a lot of places call region_intersects which use DESC_NONE,
>> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
>>
>>
>> Thanks
>> Dave
>>

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年12月05日 05:33, Lendacky, Thomas 写道:
> On 11/29/2018 09:37 PM, Dave Young wrote:
>> + more people
>>
>> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>>> When doing kexec_file_load, the first kernel needs to pass the e820
>>> reserved ranges to the second kernel. But kernel can not exactly
>>> match the e820 reserved ranges when walking through the iomem resources
>>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>>> may pass these four types to the kdump kernel, that is not desired result.
>>>
>>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>>> for the iomem resources search interfaces. It is helpful to exactly
>>> match the reserved resource ranges when walking through iomem resources.
>>>
>>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>>> be updated. Otherwise, it will be easily confused and also cause some
>>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>>> changed.
>>>
>>> Suggested-by: Dave Young <dyoung@redhat.com>
>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>> ---
>>>  arch/ia64/kernel/efi.c |  4 ++++
>>>  arch/x86/kernel/e820.c |  2 +-
>>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>>  include/linux/ioport.h |  1 +
>>>  kernel/resource.c      |  6 +++---
>>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
>>> index 8f106638913c..1841e9b4db30 100644
>>> --- a/arch/ia64/kernel/efi.c
>>> +++ b/arch/ia64/kernel/efi.c
>>> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>>>  				break;
>>>  
>>>  			case EFI_RESERVED_TYPE:
>>> +				name = "reserved";
>>
>> Ingo updated X86 code to use "Reserved",  I think it would be good to do
>> same for this case as well
>>
>>> +				desc = IORES_DESC_RESERVED;
>>> +				break;
>>> +
>>>  			case EFI_RUNTIME_SERVICES_CODE:
>>>  			case EFI_RUNTIME_SERVICES_DATA:
>>>  			case EFI_ACPI_RECLAIM_MEMORY:
>>
>> Originally, above 3 are all "reserved", so probably they all should be
>> IORES_DESC_RESERVED.
>>
>> Can any IA64 people to review this?
>>
>>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>>> index 50895c2f937d..57fafdafb860 100644
>>> --- a/arch/x86/kernel/e820.c
>>> +++ b/arch/x86/kernel/e820.c
>>> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>>>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>>>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>>>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
>>> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>>>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>>>  	case E820_TYPE_RAM:		/* Fall-through: */
>>>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
>>> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>>>  	default:			return IORES_DESC_NONE;
>>>  	}
>>>  }
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index 5378d10f1d31..fea2ef99415d 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>>>  
>>>  static int __ioremap_check_desc_other(struct resource *res)
>>>  {
>>> -	return (res->desc != IORES_DESC_NONE);
>>> +	/*
>>> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
>>> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
>>> +	 * it has been changed. And the value of 'mem_flags.desc_other'
>>> +	 * is equal to 'true' if we don't strengthen the condition in this
>>> +	 * function, that is wrong. Because originally it is equal to
>>> +	 * 'false' for the same reserved type.
>>> +	 *
>>> +	 * So, that would be nice to keep it the same as before.
>>> +	 */
>>> +	return ((res->desc != IORES_DESC_NONE) &&
>>> +		(res->desc != IORES_DESC_RESERVED));
>>>  }
>>
>> Added Tom since he added the check function.  Is it possible to only
>> check explict valid desc types instead of exclude IORES_DESC_NONE?
> 
> Sorry for the delay...
> 
> The original intent of the check was to map most memory as encrypted under
> SEV if it was marked with a specific descriptor, since it was likely to
> not be MMIO. I tried converting most things that mapped memory to memremap
> vs ioremap, but ACPI was one area that I left alone and this check catches
> the mapping of the ACPI tables. I suppose it's possible to change this to
> check just for IORES_DESC_ACPI_* values, but I would have to do some
> testing.

Recently, i tested it according to your advice, here it is really checking for the
'IORES_DESC_ACPI_*' values.  If you agree to this change, i would add the following
patch into this patch set and post them again.

[root@localhost linux]# git diff arch/x86/mm/ioremap.c
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 0029604af8a4..0e3ba620612d 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -83,7 +83,8 @@ static bool __ioremap_check_ram(struct resource *res)
 
 static int __ioremap_check_desc_other(struct resource *res)
 {
-       return (res->desc != IORES_DESC_NONE);
+       return ((res->desc = IORES_DESC_ACPI_TABLES) ||
+               (res->desc = IORES_DESC_ACPI_NV_STORAGE));
 }


Thanks.
Lianbo

> 
> Thanks,
> Tom
> 
>>
>>>  
>>>  static int __ioremap_res_check(struct resource *res, void *arg)
>>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>> index da0ebaec25f0..6ed59de48bd5 100644
>>> --- a/include/linux/ioport.h
>>> +++ b/include/linux/ioport.h
>>> @@ -133,6 +133,7 @@ enum {
>>>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>>>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>>>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
>>> +	IORES_DESC_RESERVED			= 8,
>>>  };
>>>  
>>>  /* helpers to define resources */
>>> diff --git a/kernel/resource.c b/kernel/resource.c
>>> index b0fbf685c77a..f34a632c4169 100644
>>> --- a/kernel/resource.c
>>> +++ b/kernel/resource.c
>>> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  	res->start = start;
>>>  	res->end = end;
>>>  	res->flags = type | IORESOURCE_BUSY;
>>> -	res->desc = IORES_DESC_NONE;
>>> +	res->desc = IORES_DESC_RESERVED;
>>>  
>>>  	while (1) {
>>>  
>>> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>  				next_res->start = conflict->end + 1;
>>>  				next_res->end = end;
>>>  				next_res->flags = type | IORESOURCE_BUSY;
>>> -				next_res->desc = IORES_DESC_NONE;
>>> +				next_res->desc = IORES_DESC_RESERVED;
>>>  			}
>>>  		} else {
>>>  			res->start = conflict->end + 1;
>>> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>>>  			res->start = io_start;
>>>  			res->end = io_start + io_num - 1;
>>>  			res->flags |= IORESOURCE_BUSY;
>>> -			res->desc = IORES_DESC_NONE;
>>> +			res->desc = IORES_DESC_RESERVED;
>>>  			res->child = NULL;
>>>  			if (request_resource(parent, res) = 0)
>>>  				reserved = x+1;
>>> -- 
>>> 2.17.1
>>>
>>
>>
>> There are a lot of places call region_intersects which use DESC_NONE,
>> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
>>
>>
>> Thanks
>> Dave
>>

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Lendacky, Thomas]

T24gMy8xNi8xOSAyOjMxIEFNLCBsaWppYW5nIHdyb3RlOg0KPiANCj4gDQo+IOWcqCAyMDE45bm0
MTLmnIgwNeaXpSAwNTozMywgTGVuZGFja3ksIFRob21hcyDlhpnpgZM6DQo+PiBPbiAxMS8yOS8y
MDE4IDA5OjM3IFBNLCBEYXZlIFlvdW5nIHdyb3RlOg0KPj4+ICsgbW9yZSBwZW9wbGUNCj4+Pg0K
Pj4+IE9uIDExLzI5LzE4IGF0IDA0OjA5cG0sIExpYW5ibyBKaWFuZyB3cm90ZToNCj4+Pj4gV2hl
biBkb2luZyBrZXhlY19maWxlX2xvYWQsIHRoZSBmaXJzdCBrZXJuZWwgbmVlZHMgdG8gcGFzcyB0
aGUgZTgyMA0KPj4+PiByZXNlcnZlZCByYW5nZXMgdG8gdGhlIHNlY29uZCBrZXJuZWwuIEJ1dCBr
ZXJuZWwgY2FuIG5vdCBleGFjdGx5DQo+Pj4+IG1hdGNoIHRoZSBlODIwIHJlc2VydmVkIHJhbmdl
cyB3aGVuIHdhbGtpbmcgdGhyb3VnaCB0aGUgaW9tZW0gcmVzb3VyY2VzDQo+Pj4+IHdpdGggdGhl
IGRlc2NyaXB0b3IgJ0lPUkVTX0RFU0NfTk9ORScsIGJlY2F1c2Ugc2V2ZXJhbCBlODIwIHR5cGVz
KA0KPj4+PiBlLmcuIEU4MjBfVFlQRV9SRVNFUlZFRF9LRVJOL0U4MjBfVFlQRV9SQU0vRTgyMF9U
WVBFX1VOVVNBQkxFL0U4MjANCj4+Pj4gX1RZUEVfUkVTRVJWRUQpIGFyZSBjb252ZXJ0ZWQgdG8g
dGhlIGRlc2NyaXB0b3IgJ0lPUkVTX0RFU0NfTk9ORScuIEl0DQo+Pj4+IG1heSBwYXNzIHRoZXNl
IGZvdXIgdHlwZXMgdG8gdGhlIGtkdW1wIGtlcm5lbCwgdGhhdCBpcyBub3QgZGVzaXJlZCByZXN1
bHQuDQo+Pj4+DQo+Pj4+IFNvLCB0aGlzIHBhdGNoIGFkZHMgYSBuZXcgSS9PIHJlc291cmNlIGRl
c2NyaXB0b3IgJ0lPUkVTX0RFU0NfUkVTRVJWRUQnDQo+Pj4+IGZvciB0aGUgaW9tZW0gcmVzb3Vy
Y2VzIHNlYXJjaCBpbnRlcmZhY2VzLiBJdCBpcyBoZWxwZnVsIHRvIGV4YWN0bHkNCj4+Pj4gbWF0
Y2ggdGhlIHJlc2VydmVkIHJlc291cmNlIHJhbmdlcyB3aGVuIHdhbGtpbmcgdGhyb3VnaCBpb21l
bSByZXNvdXJjZXMuDQo+Pj4+DQo+Pj4+IEluIGFkZGl0aW9uLCBzaW5jZSB0aGUgbmV3IGRlc2Ny
aXB0b3IgJ0lPUkVTX0RFU0NfUkVTRVJWRUQnIGlzIGludHJvZHVjZWQsDQo+Pj4+IHRoZXNlIGNv
ZGUgb3JpZ2luYWxseSByZWxhdGVkIHRvIHRoZSBkZXNjcmlwdG9yICdJT1JFU19ERVNDX05PTkUn
IG5lZWQgdG8NCj4+Pj4gYmUgdXBkYXRlZC4gT3RoZXJ3aXNlLCBpdCB3aWxsIGJlIGVhc2lseSBj
b25mdXNlZCBhbmQgYWxzbyBjYXVzZSBzb21lDQo+Pj4+IGVycm9ycy4gQmVjYXVzZSB0aGUgJ0U4
MjBfVFlQRV9SRVNFUlZFRCcgdHlwZSBpcyBjb252ZXJ0ZWQgdG8gdGhlIG5ldw0KPj4+PiBkZXNj
cmlwdG9yICdJT1JFU19ERVNDX1JFU0VSVkVEJyBpbnN0ZWFkIG9mICdJT1JFU19ERVNDX05PTkUn
LCBpdCBoYXMgYmVlbg0KPj4+PiBjaGFuZ2VkLg0KPj4+Pg0KPj4+PiBTdWdnZXN0ZWQtYnk6IERh
dmUgWW91bmcgPGR5b3VuZ0ByZWRoYXQuY29tPg0KPj4+PiBTaWduZWQtb2ZmLWJ5OiBMaWFuYm8g
SmlhbmcgPGxpamlhbmdAcmVkaGF0LmNvbT4NCj4+Pj4gLS0tDQo+Pj4+ICBhcmNoL2lhNjQva2Vy
bmVsL2VmaS5jIHwgIDQgKysrKw0KPj4+PiAgYXJjaC94ODYva2VybmVsL2U4MjAuYyB8ICAyICst
DQo+Pj4+ICBhcmNoL3g4Ni9tbS9pb3JlbWFwLmMgIHwgMTMgKysrKysrKysrKysrLQ0KPj4+PiAg
aW5jbHVkZS9saW51eC9pb3BvcnQuaCB8ICAxICsNCj4+Pj4gIGtlcm5lbC9yZXNvdXJjZS5jICAg
ICAgfCAgNiArKystLS0NCj4+Pj4gIDUgZmlsZXMgY2hhbmdlZCwgMjEgaW5zZXJ0aW9ucygrKSwg
NSBkZWxldGlvbnMoLSkNCj4+Pj4NCj4+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvaWE2NC9rZXJuZWwv
ZWZpLmMgYi9hcmNoL2lhNjQva2VybmVsL2VmaS5jDQo+Pj4+IGluZGV4IDhmMTA2NjM4OTEzYy4u
MTg0MWU5YjRkYjMwIDEwMDY0NA0KPj4+PiAtLS0gYS9hcmNoL2lhNjQva2VybmVsL2VmaS5jDQo+
Pj4+ICsrKyBiL2FyY2gvaWE2NC9rZXJuZWwvZWZpLmMNCj4+Pj4gQEAgLTEyMzEsNiArMTIzMSwx
MCBAQCBlZmlfaW5pdGlhbGl6ZV9pb21lbV9yZXNvdXJjZXMoc3RydWN0IHJlc291cmNlICpjb2Rl
X3Jlc291cmNlLA0KPj4+PiAgCQkJCWJyZWFrOw0KPj4+PiAgDQo+Pj4+ICAJCQljYXNlIEVGSV9S
RVNFUlZFRF9UWVBFOg0KPj4+PiArCQkJCW5hbWUgPSAicmVzZXJ2ZWQiOw0KPj4+DQo+Pj4gSW5n
byB1cGRhdGVkIFg4NiBjb2RlIHRvIHVzZSAiUmVzZXJ2ZWQiLCAgSSB0aGluayBpdCB3b3VsZCBi
ZSBnb29kIHRvIGRvDQo+Pj4gc2FtZSBmb3IgdGhpcyBjYXNlIGFzIHdlbGwNCj4+Pg0KPj4+PiAr
CQkJCWRlc2MgPSBJT1JFU19ERVNDX1JFU0VSVkVEOw0KPj4+PiArCQkJCWJyZWFrOw0KPj4+PiAr
DQo+Pj4+ICAJCQljYXNlIEVGSV9SVU5USU1FX1NFUlZJQ0VTX0NPREU6DQo+Pj4+ICAJCQljYXNl
IEVGSV9SVU5USU1FX1NFUlZJQ0VTX0RBVEE6DQo+Pj4+ICAJCQljYXNlIEVGSV9BQ1BJX1JFQ0xB
SU1fTUVNT1JZOg0KPj4+DQo+Pj4gT3JpZ2luYWxseSwgYWJvdmUgMyBhcmUgYWxsICJyZXNlcnZl
ZCIsIHNvIHByb2JhYmx5IHRoZXkgYWxsIHNob3VsZCBiZQ0KPj4+IElPUkVTX0RFU0NfUkVTRVJW
RUQuDQo+Pj4NCj4+PiBDYW4gYW55IElBNjQgcGVvcGxlIHRvIHJldmlldyB0aGlzPw0KPj4+DQo+
Pj4+IGRpZmYgLS1naXQgYS9hcmNoL3g4Ni9rZXJuZWwvZTgyMC5jIGIvYXJjaC94ODYva2VybmVs
L2U4MjAuYw0KPj4+PiBpbmRleCA1MDg5NWMyZjkzN2QuLjU3ZmFmZGFmYjg2MCAxMDA2NDQNCj4+
Pj4gLS0tIGEvYXJjaC94ODYva2VybmVsL2U4MjAuYw0KPj4+PiArKysgYi9hcmNoL3g4Ni9rZXJu
ZWwvZTgyMC5jDQo+Pj4+IEBAIC0xMDQ4LDEwICsxMDQ4LDEwIEBAIHN0YXRpYyB1bnNpZ25lZCBs
b25nIF9faW5pdCBlODIwX3R5cGVfdG9faW9yZXNfZGVzYyhzdHJ1Y3QgZTgyMF9lbnRyeSAqZW50
cnkpDQo+Pj4+ICAJY2FzZSBFODIwX1RZUEVfTlZTOgkJcmV0dXJuIElPUkVTX0RFU0NfQUNQSV9O
Vl9TVE9SQUdFOw0KPj4+PiAgCWNhc2UgRTgyMF9UWVBFX1BNRU06CQlyZXR1cm4gSU9SRVNfREVT
Q19QRVJTSVNURU5UX01FTU9SWTsNCj4+Pj4gIAljYXNlIEU4MjBfVFlQRV9QUkFNOgkJcmV0dXJu
IElPUkVTX0RFU0NfUEVSU0lTVEVOVF9NRU1PUllfTEVHQUNZOw0KPj4+PiArCWNhc2UgRTgyMF9U
WVBFX1JFU0VSVkVEOglyZXR1cm4gSU9SRVNfREVTQ19SRVNFUlZFRDsNCj4+Pj4gIAljYXNlIEU4
MjBfVFlQRV9SRVNFUlZFRF9LRVJOOgkvKiBGYWxsLXRocm91Z2g6ICovDQo+Pj4+ICAJY2FzZSBF
ODIwX1RZUEVfUkFNOgkJLyogRmFsbC10aHJvdWdoOiAqLw0KPj4+PiAgCWNhc2UgRTgyMF9UWVBF
X1VOVVNBQkxFOgkvKiBGYWxsLXRocm91Z2g6ICovDQo+Pj4+IC0JY2FzZSBFODIwX1RZUEVfUkVT
RVJWRUQ6CS8qIEZhbGwtdGhyb3VnaDogKi8NCj4+Pj4gIAlkZWZhdWx0OgkJCXJldHVybiBJT1JF
U19ERVNDX05PTkU7DQo+Pj4+ICAJfQ0KPj4+PiAgfQ0KPj4+PiBkaWZmIC0tZ2l0IGEvYXJjaC94
ODYvbW0vaW9yZW1hcC5jIGIvYXJjaC94ODYvbW0vaW9yZW1hcC5jDQo+Pj4+IGluZGV4IDUzNzhk
MTBmMWQzMS4uZmVhMmVmOTk0MTVkIDEwMDY0NA0KPj4+PiAtLS0gYS9hcmNoL3g4Ni9tbS9pb3Jl
bWFwLmMNCj4+Pj4gKysrIGIvYXJjaC94ODYvbW0vaW9yZW1hcC5jDQo+Pj4+IEBAIC04Myw3ICs4
MywxOCBAQCBzdGF0aWMgYm9vbCBfX2lvcmVtYXBfY2hlY2tfcmFtKHN0cnVjdCByZXNvdXJjZSAq
cmVzKQ0KPj4+PiAgDQo+Pj4+ICBzdGF0aWMgaW50IF9faW9yZW1hcF9jaGVja19kZXNjX290aGVy
KHN0cnVjdCByZXNvdXJjZSAqcmVzKQ0KPj4+PiAgew0KPj4+PiAtCXJldHVybiAocmVzLT5kZXNj
ICE9IElPUkVTX0RFU0NfTk9ORSk7DQo+Pj4+ICsJLyoNCj4+Pj4gKwkgKiBCdXQgbm93LCB0aGUg
J0U4MjBfVFlQRV9SRVNFUlZFRCcgdHlwZSBpcyBjb252ZXJ0ZWQgdG8gdGhlIG5ldw0KPj4+PiAr
CSAqIGRlc2NyaXB0b3IgJ0lPUkVTX0RFU0NfUkVTRVJWRUQnIGluc3RlYWQgb2YgJ0lPUkVTX0RF
U0NfTk9ORScsDQo+Pj4+ICsJICogaXQgaGFzIGJlZW4gY2hhbmdlZC4gQW5kIHRoZSB2YWx1ZSBv
ZiAnbWVtX2ZsYWdzLmRlc2Nfb3RoZXInDQo+Pj4+ICsJICogaXMgZXF1YWwgdG8gJ3RydWUnIGlm
IHdlIGRvbid0IHN0cmVuZ3RoZW4gdGhlIGNvbmRpdGlvbiBpbiB0aGlzDQo+Pj4+ICsJICogZnVu
Y3Rpb24sIHRoYXQgaXMgd3JvbmcuIEJlY2F1c2Ugb3JpZ2luYWxseSBpdCBpcyBlcXVhbCB0bw0K
Pj4+PiArCSAqICdmYWxzZScgZm9yIHRoZSBzYW1lIHJlc2VydmVkIHR5cGUuDQo+Pj4+ICsJICoN
Cj4+Pj4gKwkgKiBTbywgdGhhdCB3b3VsZCBiZSBuaWNlIHRvIGtlZXAgaXQgdGhlIHNhbWUgYXMg
YmVmb3JlLg0KPj4+PiArCSAqLw0KPj4+PiArCXJldHVybiAoKHJlcy0+ZGVzYyAhPSBJT1JFU19E
RVNDX05PTkUpICYmDQo+Pj4+ICsJCShyZXMtPmRlc2MgIT0gSU9SRVNfREVTQ19SRVNFUlZFRCkp
Ow0KPj4+PiAgfQ0KPj4+DQo+Pj4gQWRkZWQgVG9tIHNpbmNlIGhlIGFkZGVkIHRoZSBjaGVjayBm
dW5jdGlvbi4gIElzIGl0IHBvc3NpYmxlIHRvIG9ubHkNCj4+PiBjaGVjayBleHBsaWN0IHZhbGlk
IGRlc2MgdHlwZXMgaW5zdGVhZCBvZiBleGNsdWRlIElPUkVTX0RFU0NfTk9ORT8NCj4+DQo+PiBT
b3JyeSBmb3IgdGhlIGRlbGF5Li4uDQo+Pg0KPj4gVGhlIG9yaWdpbmFsIGludGVudCBvZiB0aGUg
Y2hlY2sgd2FzIHRvIG1hcCBtb3N0IG1lbW9yeSBhcyBlbmNyeXB0ZWQgdW5kZXINCj4+IFNFViBp
ZiBpdCB3YXMgbWFya2VkIHdpdGggYSBzcGVjaWZpYyBkZXNjcmlwdG9yLCBzaW5jZSBpdCB3YXMg
bGlrZWx5IHRvDQo+PiBub3QgYmUgTU1JTy4gSSB0cmllZCBjb252ZXJ0aW5nIG1vc3QgdGhpbmdz
IHRoYXQgbWFwcGVkIG1lbW9yeSB0byBtZW1yZW1hcA0KPj4gdnMgaW9yZW1hcCwgYnV0IEFDUEkg
d2FzIG9uZSBhcmVhIHRoYXQgSSBsZWZ0IGFsb25lIGFuZCB0aGlzIGNoZWNrIGNhdGNoZXMNCj4+
IHRoZSBtYXBwaW5nIG9mIHRoZSBBQ1BJIHRhYmxlcy4gSSBzdXBwb3NlIGl0J3MgcG9zc2libGUg
dG8gY2hhbmdlIHRoaXMgdG8NCj4+IGNoZWNrIGp1c3QgZm9yIElPUkVTX0RFU0NfQUNQSV8qIHZh
bHVlcywgYnV0IEkgd291bGQgaGF2ZSB0byBkbyBzb21lDQo+PiB0ZXN0aW5nLg0KPiANCj4gUmVj
ZW50bHksIGkgdGVzdGVkIGl0IGFjY29yZGluZyB0byB5b3VyIGFkdmljZSwgaGVyZSBpdCBpcyBy
ZWFsbHkgY2hlY2tpbmcgZm9yIHRoZQ0KPiAnSU9SRVNfREVTQ19BQ1BJXyonIHZhbHVlcy4gIElm
IHlvdSBhZ3JlZSB0byB0aGlzIGNoYW5nZSwgaSB3b3VsZCBhZGQgdGhlIGZvbGxvd2luZw0KPiBw
YXRjaCBpbnRvIHRoaXMgcGF0Y2ggc2V0IGFuZCBwb3N0IHRoZW0gYWdhaW4uDQo+IA0KPiBbcm9v
dEBsb2NhbGhvc3QgbGludXhdIyBnaXQgZGlmZiBhcmNoL3g4Ni9tbS9pb3JlbWFwLmMNCj4gZGlm
ZiAtLWdpdCBhL2FyY2gveDg2L21tL2lvcmVtYXAuYyBiL2FyY2gveDg2L21tL2lvcmVtYXAuYw0K
PiBpbmRleCAwMDI5NjA0YWY4YTQuLjBlM2JhNjIwNjEyZCAxMDA2NDQNCj4gLS0tIGEvYXJjaC94
ODYvbW0vaW9yZW1hcC5jDQo+ICsrKyBiL2FyY2gveDg2L21tL2lvcmVtYXAuYw0KPiBAQCAtODMs
NyArODMsOCBAQCBzdGF0aWMgYm9vbCBfX2lvcmVtYXBfY2hlY2tfcmFtKHN0cnVjdCByZXNvdXJj
ZSAqcmVzKQ0KPiAgDQo+ICBzdGF0aWMgaW50IF9faW9yZW1hcF9jaGVja19kZXNjX290aGVyKHN0
cnVjdCByZXNvdXJjZSAqcmVzKQ0KPiAgew0KPiAtICAgICAgIHJldHVybiAocmVzLT5kZXNjICE9
IElPUkVTX0RFU0NfTk9ORSk7DQo+ICsgICAgICAgcmV0dXJuICgocmVzLT5kZXNjID09IElPUkVT
X0RFU0NfQUNQSV9UQUJMRVMpIHx8DQo+ICsgICAgICAgICAgICAgICAocmVzLT5kZXNjID09IElP
UkVTX0RFU0NfQUNQSV9OVl9TVE9SQUdFKSk7DQoNCkknbSBub3QgYSBiaWcgZmFuIG9mIHRoaXMu
IEkgdGhpbmsgeW91IHNob3VsZCBsZWF2ZSBpdCBhcyB0aGUgcHJldmlvdXMNCmNoZWNrIHlvdSBo
YWQgZm9yIElPUkVTX0RFU0NfTk9ORSBhbmQgSU9SRVNfREVTQ19SRVNFUlZFRC4gVGhlcmUncyBu
bw0KdGVsbGluZyB3aGF0IHR5cGUgb2YgcmVzb3VyY2VzIG1heSBiZSBtYXBwZWQgaW4gdGhlIGZ1
dHVyZSB3aGVyZSB0aGlzDQp3aWxsIGJyZWFrLg0KDQpBZGRpbmcgYSBuaWNlIGNvbW1lbnQgaGVy
ZSBhYm91dCBob3cgSU9SRVNfREVTQ19OT05FIG9yaWdpbmFsbHkgd2FzIHRvDQppZGVudGlmeSBN
TUlPIGFuZCByZXNlcnZlZCBhcmVhcy4gTm93IElPUkVTX0RFU0NfUkVTRVJWRUQgaGFzIGJlZW4g
Y3JlYXRlZA0KZm9yIHRoZSByZXNlcnZlZCBhcmVhcyBzbyB0aGUgY2hlY2sgbmVlZHMgdG8gYmUg
ZXhwYW5kZWQgc28gdGhhdCB0aGVzZQ0KYXJlYXMgYXJlbid0IG1hcHBlZCBlbmNyeXB0ZWQgd2hl
biB1c2luZyBpb3JlbWFwLg0KDQpUaGFua3MsDQpUb20NCg0KPiAgfQ0KPiANCj4gDQo+IFRoYW5r
cy4NCj4gTGlhbmJvDQo+IA0KPj4NCj4+IFRoYW5rcywNCj4+IFRvbQ0KPj4NCj4+Pg0KPj4+PiAg
DQo+Pj4+ICBzdGF0aWMgaW50IF9faW9yZW1hcF9yZXNfY2hlY2soc3RydWN0IHJlc291cmNlICpy
ZXMsIHZvaWQgKmFyZykNCj4+Pj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvaW9wb3J0Lmgg
Yi9pbmNsdWRlL2xpbnV4L2lvcG9ydC5oDQo+Pj4+IGluZGV4IGRhMGViYWVjMjVmMC4uNmVkNTlk
ZTQ4YmQ1IDEwMDY0NA0KPj4+PiAtLS0gYS9pbmNsdWRlL2xpbnV4L2lvcG9ydC5oDQo+Pj4+ICsr
KyBiL2luY2x1ZGUvbGludXgvaW9wb3J0LmgNCj4+Pj4gQEAgLTEzMyw2ICsxMzMsNyBAQCBlbnVt
IHsNCj4+Pj4gIAlJT1JFU19ERVNDX1BFUlNJU1RFTlRfTUVNT1JZX0xFR0FDWQk9IDUsDQo+Pj4+
ICAJSU9SRVNfREVTQ19ERVZJQ0VfUFJJVkFURV9NRU1PUlkJPSA2LA0KPj4+PiAgCUlPUkVTX0RF
U0NfREVWSUNFX1BVQkxJQ19NRU1PUlkJCT0gNywNCj4+Pj4gKwlJT1JFU19ERVNDX1JFU0VSVkVE
CQkJPSA4LA0KPj4+PiAgfTsNCj4+Pj4gIA0KPj4+PiAgLyogaGVscGVycyB0byBkZWZpbmUgcmVz
b3VyY2VzICovDQo+Pj4+IGRpZmYgLS1naXQgYS9rZXJuZWwvcmVzb3VyY2UuYyBiL2tlcm5lbC9y
ZXNvdXJjZS5jDQo+Pj4+IGluZGV4IGIwZmJmNjg1Yzc3YS4uZjM0YTYzMmM0MTY5IDEwMDY0NA0K
Pj4+PiAtLS0gYS9rZXJuZWwvcmVzb3VyY2UuYw0KPj4+PiArKysgYi9rZXJuZWwvcmVzb3VyY2Uu
Yw0KPj4+PiBAQCAtOTk0LDcgKzk5NCw3IEBAIF9fcmVzZXJ2ZV9yZWdpb25fd2l0aF9zcGxpdChz
dHJ1Y3QgcmVzb3VyY2UgKnJvb3QsIHJlc291cmNlX3NpemVfdCBzdGFydCwNCj4+Pj4gIAlyZXMt
PnN0YXJ0ID0gc3RhcnQ7DQo+Pj4+ICAJcmVzLT5lbmQgPSBlbmQ7DQo+Pj4+ICAJcmVzLT5mbGFn
cyA9IHR5cGUgfCBJT1JFU09VUkNFX0JVU1k7DQo+Pj4+IC0JcmVzLT5kZXNjID0gSU9SRVNfREVT
Q19OT05FOw0KPj4+PiArCXJlcy0+ZGVzYyA9IElPUkVTX0RFU0NfUkVTRVJWRUQ7DQo+Pj4+ICAN
Cj4+Pj4gIAl3aGlsZSAoMSkgew0KPj4+PiAgDQo+Pj4+IEBAIC0xMDI5LDcgKzEwMjksNyBAQCBf
X3Jlc2VydmVfcmVnaW9uX3dpdGhfc3BsaXQoc3RydWN0IHJlc291cmNlICpyb290LCByZXNvdXJj
ZV9zaXplX3Qgc3RhcnQsDQo+Pj4+ICAJCQkJbmV4dF9yZXMtPnN0YXJ0ID0gY29uZmxpY3QtPmVu
ZCArIDE7DQo+Pj4+ICAJCQkJbmV4dF9yZXMtPmVuZCA9IGVuZDsNCj4+Pj4gIAkJCQluZXh0X3Jl
cy0+ZmxhZ3MgPSB0eXBlIHwgSU9SRVNPVVJDRV9CVVNZOw0KPj4+PiAtCQkJCW5leHRfcmVzLT5k
ZXNjID0gSU9SRVNfREVTQ19OT05FOw0KPj4+PiArCQkJCW5leHRfcmVzLT5kZXNjID0gSU9SRVNf
REVTQ19SRVNFUlZFRDsNCj4+Pj4gIAkJCX0NCj4+Pj4gIAkJfSBlbHNlIHsNCj4+Pj4gIAkJCXJl
cy0+c3RhcnQgPSBjb25mbGljdC0+ZW5kICsgMTsNCj4+Pj4gQEAgLTE0NzcsNyArMTQ3Nyw3IEBA
IHN0YXRpYyBpbnQgX19pbml0IHJlc2VydmVfc2V0dXAoY2hhciAqc3RyKQ0KPj4+PiAgCQkJcmVz
LT5zdGFydCA9IGlvX3N0YXJ0Ow0KPj4+PiAgCQkJcmVzLT5lbmQgPSBpb19zdGFydCArIGlvX251
bSAtIDE7DQo+Pj4+ICAJCQlyZXMtPmZsYWdzIHw9IElPUkVTT1VSQ0VfQlVTWTsNCj4+Pj4gLQkJ
CXJlcy0+ZGVzYyA9IElPUkVTX0RFU0NfTk9ORTsNCj4+Pj4gKwkJCXJlcy0+ZGVzYyA9IElPUkVT
X0RFU0NfUkVTRVJWRUQ7DQo+Pj4+ICAJCQlyZXMtPmNoaWxkID0gTlVMTDsNCj4+Pj4gIAkJCWlm
IChyZXF1ZXN0X3Jlc291cmNlKHBhcmVudCwgcmVzKSA9PSAwKQ0KPj4+PiAgCQkJCXJlc2VydmVk
ID0geCsxOw0KPj4+PiAtLSANCj4+Pj4gMi4xNy4xDQo+Pj4+DQo+Pj4NCj4+Pg0KPj4+IFRoZXJl
IGFyZSBhIGxvdCBvZiBwbGFjZXMgY2FsbCByZWdpb25faW50ZXJzZWN0cyB3aGljaCB1c2UgREVT
Q19OT05FLA0KPj4+IEknbSBub3Qgc3VyZSBpZiBuZWVkZWQgY2hhbmdlcyBhY2NvcmRpbmdseS4g
IENjZWQgRGFuIGFuZCBUb3NoaS4NCj4+Pg0KPj4+DQo+Pj4gVGhhbmtzDQo+Pj4gRGF2ZQ0KPj4+
DQo

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2019年03月26日 03:34, Lendacky, Thomas 写道:
> On 3/16/19 2:31 AM, lijiang wrote:
>>
>>
>> 在 2018年12月05日 05:33, Lendacky, Thomas 写道:
>>> On 11/29/2018 09:37 PM, Dave Young wrote:
>>>> + more people
>>>>
>>>> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>>>>> When doing kexec_file_load, the first kernel needs to pass the e820
>>>>> reserved ranges to the second kernel. But kernel can not exactly
>>>>> match the e820 reserved ranges when walking through the iomem resources
>>>>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>>>>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>>>>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>>>>> may pass these four types to the kdump kernel, that is not desired result.
>>>>>
>>>>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>>>>> for the iomem resources search interfaces. It is helpful to exactly
>>>>> match the reserved resource ranges when walking through iomem resources.
>>>>>
>>>>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>>>>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>>>>> be updated. Otherwise, it will be easily confused and also cause some
>>>>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>>>>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>>>>> changed.
>>>>>
>>>>> Suggested-by: Dave Young <dyoung@redhat.com>
>>>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>>>> ---
>>>>>  arch/ia64/kernel/efi.c |  4 ++++
>>>>>  arch/x86/kernel/e820.c |  2 +-
>>>>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>>>>  include/linux/ioport.h |  1 +
>>>>>  kernel/resource.c      |  6 +++---
>>>>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
>>>>> index 8f106638913c..1841e9b4db30 100644
>>>>> --- a/arch/ia64/kernel/efi.c
>>>>> +++ b/arch/ia64/kernel/efi.c
>>>>> @@ -1231,6 +1231,10 @@ efi_initialize_iomem_resources(struct resource *code_resource,
>>>>>  				break;
>>>>>  
>>>>>  			case EFI_RESERVED_TYPE:
>>>>> +				name = "reserved";
>>>>
>>>> Ingo updated X86 code to use "Reserved",  I think it would be good to do
>>>> same for this case as well
>>>>
>>>>> +				desc = IORES_DESC_RESERVED;
>>>>> +				break;
>>>>> +
>>>>>  			case EFI_RUNTIME_SERVICES_CODE:
>>>>>  			case EFI_RUNTIME_SERVICES_DATA:
>>>>>  			case EFI_ACPI_RECLAIM_MEMORY:
>>>>
>>>> Originally, above 3 are all "reserved", so probably they all should be
>>>> IORES_DESC_RESERVED.
>>>>
>>>> Can any IA64 people to review this?
>>>>
>>>>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>>>>> index 50895c2f937d..57fafdafb860 100644
>>>>> --- a/arch/x86/kernel/e820.c
>>>>> +++ b/arch/x86/kernel/e820.c
>>>>> @@ -1048,10 +1048,10 @@ static unsigned long __init e820_type_to_iores_desc(struct e820_entry *entry)
>>>>>  	case E820_TYPE_NVS:		return IORES_DESC_ACPI_NV_STORAGE;
>>>>>  	case E820_TYPE_PMEM:		return IORES_DESC_PERSISTENT_MEMORY;
>>>>>  	case E820_TYPE_PRAM:		return IORES_DESC_PERSISTENT_MEMORY_LEGACY;
>>>>> +	case E820_TYPE_RESERVED:	return IORES_DESC_RESERVED;
>>>>>  	case E820_TYPE_RESERVED_KERN:	/* Fall-through: */
>>>>>  	case E820_TYPE_RAM:		/* Fall-through: */
>>>>>  	case E820_TYPE_UNUSABLE:	/* Fall-through: */
>>>>> -	case E820_TYPE_RESERVED:	/* Fall-through: */
>>>>>  	default:			return IORES_DESC_NONE;
>>>>>  	}
>>>>>  }
>>>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>>>> index 5378d10f1d31..fea2ef99415d 100644
>>>>> --- a/arch/x86/mm/ioremap.c
>>>>> +++ b/arch/x86/mm/ioremap.c
>>>>> @@ -83,7 +83,18 @@ static bool __ioremap_check_ram(struct resource *res)
>>>>>  
>>>>>  static int __ioremap_check_desc_other(struct resource *res)
>>>>>  {
>>>>> -	return (res->desc != IORES_DESC_NONE);
>>>>> +	/*
>>>>> +	 * But now, the 'E820_TYPE_RESERVED' type is converted to the new
>>>>> +	 * descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE',
>>>>> +	 * it has been changed. And the value of 'mem_flags.desc_other'
>>>>> +	 * is equal to 'true' if we don't strengthen the condition in this
>>>>> +	 * function, that is wrong. Because originally it is equal to
>>>>> +	 * 'false' for the same reserved type.
>>>>> +	 *
>>>>> +	 * So, that would be nice to keep it the same as before.
>>>>> +	 */
>>>>> +	return ((res->desc != IORES_DESC_NONE) &&
>>>>> +		(res->desc != IORES_DESC_RESERVED));
>>>>>  }
>>>>
>>>> Added Tom since he added the check function.  Is it possible to only
>>>> check explict valid desc types instead of exclude IORES_DESC_NONE?
>>>
>>> Sorry for the delay...
>>>
>>> The original intent of the check was to map most memory as encrypted under
>>> SEV if it was marked with a specific descriptor, since it was likely to
>>> not be MMIO. I tried converting most things that mapped memory to memremap
>>> vs ioremap, but ACPI was one area that I left alone and this check catches
>>> the mapping of the ACPI tables. I suppose it's possible to change this to
>>> check just for IORES_DESC_ACPI_* values, but I would have to do some
>>> testing.
>>
>> Recently, i tested it according to your advice, here it is really checking for the
>> 'IORES_DESC_ACPI_*' values.  If you agree to this change, i would add the following
>> patch into this patch set and post them again.
>>
>> [root@localhost linux]# git diff arch/x86/mm/ioremap.c
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index 0029604af8a4..0e3ba620612d 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -83,7 +83,8 @@ static bool __ioremap_check_ram(struct resource *res)
>>  
>>  static int __ioremap_check_desc_other(struct resource *res)
>>  {
>> -       return (res->desc != IORES_DESC_NONE);
>> +       return ((res->desc = IORES_DESC_ACPI_TABLES) ||
>> +               (res->desc = IORES_DESC_ACPI_NV_STORAGE));
> 
> I'm not a big fan of this. I think you should leave it as the previous
> check you had for IORES_DESC_NONE and IORES_DESC_RESERVED. There's no
> telling what type of resources may be mapped in the future where this
> will break.
> 
> Adding a nice comment here about how IORES_DESC_NONE originally was to
> identify MMIO and reserved areas. Now IORES_DESC_RESERVED has been created
> for the reserved areas so the check needs to be expanded so that these
> areas aren't mapped encrypted when using ioremap.

Thanks for your comment.

It's OK for me. I'm not sure whether Boris has any suggestion.

Thanks.
Lianbo

> 
> Thanks,
> Tom
> 
>>  }
>>
>>
>> Thanks.
>> Lianbo
>>
>>>
>>> Thanks,
>>> Tom
>>>
>>>>
>>>>>  
>>>>>  static int __ioremap_res_check(struct resource *res, void *arg)
>>>>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>>>> index da0ebaec25f0..6ed59de48bd5 100644
>>>>> --- a/include/linux/ioport.h
>>>>> +++ b/include/linux/ioport.h
>>>>> @@ -133,6 +133,7 @@ enum {
>>>>>  	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
>>>>>  	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
>>>>>  	IORES_DESC_DEVICE_PUBLIC_MEMORY		= 7,
>>>>> +	IORES_DESC_RESERVED			= 8,
>>>>>  };
>>>>>  
>>>>>  /* helpers to define resources */
>>>>> diff --git a/kernel/resource.c b/kernel/resource.c
>>>>> index b0fbf685c77a..f34a632c4169 100644
>>>>> --- a/kernel/resource.c
>>>>> +++ b/kernel/resource.c
>>>>> @@ -994,7 +994,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>>>  	res->start = start;
>>>>>  	res->end = end;
>>>>>  	res->flags = type | IORESOURCE_BUSY;
>>>>> -	res->desc = IORES_DESC_NONE;
>>>>> +	res->desc = IORES_DESC_RESERVED;
>>>>>  
>>>>>  	while (1) {
>>>>>  
>>>>> @@ -1029,7 +1029,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
>>>>>  				next_res->start = conflict->end + 1;
>>>>>  				next_res->end = end;
>>>>>  				next_res->flags = type | IORESOURCE_BUSY;
>>>>> -				next_res->desc = IORES_DESC_NONE;
>>>>> +				next_res->desc = IORES_DESC_RESERVED;
>>>>>  			}
>>>>>  		} else {
>>>>>  			res->start = conflict->end + 1;
>>>>> @@ -1477,7 +1477,7 @@ static int __init reserve_setup(char *str)
>>>>>  			res->start = io_start;
>>>>>  			res->end = io_start + io_num - 1;
>>>>>  			res->flags |= IORESOURCE_BUSY;
>>>>> -			res->desc = IORES_DESC_NONE;
>>>>> +			res->desc = IORES_DESC_RESERVED;
>>>>>  			res->child = NULL;
>>>>>  			if (request_resource(parent, res) = 0)
>>>>>  				reserved = x+1;
>>>>> -- 
>>>>> 2.17.1
>>>>>
>>>>
>>>>
>>>> There are a lot of places call region_intersects which use DESC_NONE,
>>>> I'm not sure if needed changes accordingly.  Cced Dan and Toshi.
>>>>
>>>>
>>>> Thanks
>>>> Dave
>>>>

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Boris Petkov]

On March 25, 2019 8:34:25 PM GMT+01:00, "Lendacky, Thomas" <Thomas.Lendacky@amd.com> wrote:
>On 3/16/19 2:31 AM, lijiang 
>Adding a nice comment here about how IORES_DESC_NONE originally was to
>identify MMIO and reserved areas. Now IORES_DESC_RESERVED has been
>created
>for the reserved areas so the check needs to be expanded

Expanded and made explicit *which* areas should not be mapped encrypted and then the function name corrected. 

Thx.

-- 
Sent from a small device: formatting sux and brevity is inevitable. 

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[Dave Young]

On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
> When doing kexec_file_load, the first kernel needs to pass the e820
> reserved ranges to the second kernel. But kernel can not exactly
> match the e820 reserved ranges when walking through the iomem resources
> with the descriptor 'IORES_DESC_NONE', because several e820 types(
> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
> may pass these four types to the kdump kernel, that is not desired result.
> 
> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
> for the iomem resources search interfaces. It is helpful to exactly
> match the reserved resource ranges when walking through iomem resources.
> 
> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
> these code originally related to the descriptor 'IORES_DESC_NONE' need to
> be updated. Otherwise, it will be easily confused and also cause some
> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
> changed.
> 
> Suggested-by: Dave Young <dyoung@redhat.com>

This was suggested by Boris instead :) 

> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
>  arch/ia64/kernel/efi.c |  4 ++++
>  arch/x86/kernel/e820.c |  2 +-
>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>  include/linux/ioport.h |  1 +
>  kernel/resource.c      |  6 +++---
>  5 files changed, 21 insertions(+), 5 deletions(-)
> 
> 
[snip]

Thanks
Dave

Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'

[lijiang]

在 2018年11月30日 11:41, Dave Young 写道:
> On 11/29/18 at 04:09pm, Lianbo Jiang wrote:
>> When doing kexec_file_load, the first kernel needs to pass the e820
>> reserved ranges to the second kernel. But kernel can not exactly
>> match the e820 reserved ranges when walking through the iomem resources
>> with the descriptor 'IORES_DESC_NONE', because several e820 types(
>> e.g. E820_TYPE_RESERVED_KERN/E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820
>> _TYPE_RESERVED) are converted to the descriptor 'IORES_DESC_NONE'. It
>> may pass these four types to the kdump kernel, that is not desired result.
>>
>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>> for the iomem resources search interfaces. It is helpful to exactly
>> match the reserved resource ranges when walking through iomem resources.
>>
>> In addition, since the new descriptor 'IORES_DESC_RESERVED' is introduced,
>> these code originally related to the descriptor 'IORES_DESC_NONE' need to
>> be updated. Otherwise, it will be easily confused and also cause some
>> errors. Because the 'E820_TYPE_RESERVED' type is converted to the new
>> descriptor 'IORES_DESC_RESERVED' instead of 'IORES_DESC_NONE', it has been
>> changed.
>>
>> Suggested-by: Dave Young <dyoung@redhat.com>
> 
> This was suggested by Boris instead :) 
> 

Sorry for this, i forgot to change it.
And i will correct this in next version.

Thanks.

>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>> ---
>>  arch/ia64/kernel/efi.c |  4 ++++
>>  arch/x86/kernel/e820.c |  2 +-
>>  arch/x86/mm/ioremap.c  | 13 ++++++++++++-
>>  include/linux/ioport.h |  1 +
>>  kernel/resource.c      |  6 +++---
>>  5 files changed, 21 insertions(+), 5 deletions(-)
>>
>>
> [snip]
> 
> Thanks
> Dave
> 

[PATCH 2/2 v8] x86/kexec_file: add reserved e820 ranges to kdump kernel e820 table

[Lianbo Jiang]

At present, when use the kexec_file_load syscall to load the kernel image
and initramfs(for example: kexec -s -p xxx), 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 basic problem is that this device
is in PCI segment 1 and the kernel PCI probing can not find it without all
the e820 I/O reservations being present in the e820 table. And the kdump
kernel does not have those reservations because the kexec command does not
pass the I/O reservation via the "memmap=xxx" command line option. (This
problem does not show up for other vendors, as SGI is apparently the
actually fails for everyone, but devices in segment 0 are then found by
some legacy lookup method.) The workaround for this is to pass the I/O
reserved regions to the kdump kernel.

MMCONFIG(aka ECAM) space is described in the ACPI MCFG table. If you don't
have ECAM: (a) PCI devices won't work at all on non-x86 systems that use
only ECAM for config access, (b) you won't be albe to access devices on
non-0 segments, (c) you won't be able to access extended config space(
address 0x100-0xffff), which means none of the Extended Capabilities will
be available(AER, ACS, ATS, etc). [Bjorn's comment]

The second issue is that the SME kdump kernel doesn't work without the
e820 reserved ranges. When SME is active in kdump kernel, actually, those
reserved regions are still decrypted, but because those reserved ranges are
not present at all in kdump kernel e820 table, those reserved regions are
considered as encrypted, it goes wrong.

The e820 reserved range is useful in kdump kernel, so it is necessary to
pass the e820 reserved ranges to kdump kernel.

Suggested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 arch/x86/kernel/crash.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index f631a3f15587..5354a84f1684 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -380,6 +380,12 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
 	walk_iomem_res_desc(IORES_DESC_ACPI_NV_STORAGE, flags, 0, -1, &cmd,
 			memmap_entry_callback);
 
+	/* Add e820 reserved ranges */
+	cmd.type = E820_TYPE_RESERVED;
+	flags = IORESOURCE_MEM;
+	walk_iomem_res_desc(IORES_DESC_RESERVED, flags, 0, -1, &cmd,
+			   memmap_entry_callback);
+
 	/* Add crashk_low_res region */
 	if (crashk_low_res.end) {
 		ei.addr = crashk_low_res.start;
-- 
2.17.1