Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
On March 25, 2019 8:34:25 PM GMT+01:00, "Lendacky, Thomas" 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'
在 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
> Signed-off-by: Lianbo Jiang
> ---
> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
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
Signed-off-by: Lianbo Jiang
---
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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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
>>> Signed-off-by: Lianbo Jiang
>>> ---
>>> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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
>>> Signed-off-by: Lianbo Jiang
>>> ---
>>> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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'
在 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
>>> Signed-off-by: Lianbo Jiang
>>> ---
>>> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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'
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'
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
>> Signed-off-by: Lianbo Jiang
>> ---
>> 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.
Thanks,
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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
>> Signed-off-by: Lianbo Jiang
>> ---
>> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
在 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 > > 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 >> --- >> 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'
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
> > Signed-off-by: Lianbo Jiang
> > ---
> > 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,
> >
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
+ 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
> Signed-off-by: Lianbo Jiang
> ---
> 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
Re: [PATCH 1/2 v8] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
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 This was suggested by Boris instead :) > Signed-off-by: Lianbo Jiang > --- > 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
