| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <47bfcbe2-1631-9733-74c2-fb3e56540052@redhat.com>
Date: Mon, 25 Mar 2019 14:53:02 +0800
From: lijiang <lijiang@...hat.com>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
tglx@...utronix.de, mingo@...hat.com, akpm@...ux-foundation.org,
dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
x86@...nel.org, hpa@...or.com, dyoung@...hat.com, bhe@...hat.com,
Thomas.Lendacky@....com
Subject: Re: [PATCH 2/3 v9] resource: add the new I/O resource descriptor
'IORES_DESC_RESERVED'
在 2019年03月23日 03:28, Borislav Petkov 写道:
> On Thu, Mar 21, 2019 at 06:33:08PM +0800, Lianbo Jiang wrote:
>> When doing kexec_file_load, the first kernel needs to pass the e820
>
> Please end function names with parentheses.
>
>> reserved ranges to the second kernel.
>
> ... because... ?
>
>> But kernel can not exactly match the e820 reserved ranges
> ^
> the
>
>> 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.
>
> Rewrite that sentence.
>
>> So, this patch adds a new I/O resource descriptor 'IORES_DESC_RESERVED'
>
> Avoid having "This patch" or "This commit" in the commit message. It is
> tautologically useless.
>
> Also, do
>
> $ git grep 'This patch' Documentation/process
>
> for more details.
>
>> 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
>
> "the code"
>
>> be updated.
>
>> Otherwise, it will be easily confused and also cause some errors.
>
> What 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.
>
> That sentence I cannot parse.
Thanks for your comment. I will improve the patch log next post.
>
>> Suggested-by: Borislav Petkov <bp@...e.de>
>> Signed-off-by: Lianbo Jiang <lijiang@...hat.com>
>> ---
>> arch/x86/kernel/e820.c | 2 +-
>> include/linux/ioport.h | 1 +
>> kernel/resource.c | 6 +++---
>> 3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>> index 2879e234e193..16fcde196243 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -1050,10 +1050,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/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 */
>
> IORES_DESC_RESERVED is supposed to represent E820_TYPE_RESERVED. And if> that is the case, then all three hunks below look wrong to me. If you
> want to pass E820_TYPE_RESERVED ranges, then do that explicitly.
In this function, i printed its values, and only got the value of reserved
type, so i changed the IORES_DESC_NONE to the IORES_DESC_RESERVED.
In addition, after the new descriptor 'IORES_DESC_RESERVED' is introduced,
the IORES_DESC_NONE does not include the IORES_DESC_RESERVED any more, it
could miss to handle the value of the reserved type.
Do you mean i should never touch the three chunks? If i made a mistake, i
will remove this changes next post.
Thanks.
Lianbo
>
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index e81b17b53fa5..ee7348761858 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -990,7 +990,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) {
>>
>> @@ -1025,7 +1025,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;
>> @@ -1488,7 +1488,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
>>
>
Powered by blists - more mailing lists