| 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: <18563829-c8ec-87bd-90a6-137e79e2c8f1@redhat.com>
Date: Thu, 21 Jun 2018 17:13:11 +0800
From: lijiang <lijiang@...hat.com>
To: Tom Lendacky <thomas.lendacky@....com>,
linux-kernel@...r.kernel.org
Cc: iommu@...ts.linux-foundation.org, kexec@...ts.infradead.org,
dyoung@...hat.com
Subject: Re: [PATCH 1/4 V3] Add a function(ioremap_encrypted) for kdump when
AMD sme enabled
在 2018年06月21日 00:00, Tom Lendacky 写道:
> On 6/16/2018 3:27 AM, Lianbo Jiang wrote:
>> It is convenient to remap the old memory encrypted to the second
>> kernel by calling ioremap_encrypted().
>>
>> Signed-off-by: Lianbo Jiang <lijiang@...hat.com>
>> ---
>> Some changes:
>> 1. remove the sme_active() check in __ioremap_caller().
>> 2. put some logic into the early_memremap_pgprot_adjust() for
>> early memremap.
>>
>> arch/x86/include/asm/io.h | 3 +++
>> arch/x86/mm/ioremap.c | 28 ++++++++++++++++++++--------
>> 2 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>> index f6e5b93..989d60b 100644
>> --- a/arch/x86/include/asm/io.h
>> +++ b/arch/x86/include/asm/io.h
>> @@ -192,6 +192,9 @@ extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
>> #define ioremap_cache ioremap_cache
>> extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
>> #define ioremap_prot ioremap_prot
>> +extern void __iomem *ioremap_encrypted(resource_size_t phys_addr,
>> + unsigned long size);
>> +#define ioremap_encrypted ioremap_encrypted
>>
>> /**
>> * ioremap - map bus memory into CPU space
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index c63a545..e365fc4 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -24,6 +24,7 @@
>> #include <asm/pgalloc.h>
>> #include <asm/pat.h>
>> #include <asm/setup.h>
>> +#include <linux/crash_dump.h>
>>
>> #include "physaddr.h"
>>
>> @@ -131,7 +132,8 @@ static void __ioremap_check_mem(resource_size_t addr, unsigned long size,
>> * caller shouldn't need to know that small detail.
>> */
>> static void __iomem *__ioremap_caller(resource_size_t phys_addr,
>> - unsigned long size, enum page_cache_mode pcm, void *caller)
>> + unsigned long size, enum page_cache_mode pcm,
>> + void *caller, bool encrypted)
>> {
>> unsigned long offset, vaddr;
>> resource_size_t last_addr;
>> @@ -199,7 +201,7 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
>> * resulting mapping.
>> */
>> prot = PAGE_KERNEL_IO;
>> - if (sev_active() && mem_flags.desc_other)
>> + if ((sev_active() && mem_flags.desc_other) || encrypted)
>> prot = pgprot_encrypted(prot);
>>
>> switch (pcm) {
>> @@ -291,7 +293,7 @@ void __iomem *ioremap_nocache(resource_size_t phys_addr, unsigned long size)
>> enum page_cache_mode pcm = _PAGE_CACHE_MODE_UC_MINUS;
>>
>> return __ioremap_caller(phys_addr, size, pcm,
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL(ioremap_nocache);
>>
>> @@ -324,7 +326,7 @@ void __iomem *ioremap_uc(resource_size_t phys_addr, unsigned long size)
>> enum page_cache_mode pcm = _PAGE_CACHE_MODE_UC;
>>
>> return __ioremap_caller(phys_addr, size, pcm,
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL_GPL(ioremap_uc);
>>
>> @@ -341,7 +343,7 @@ EXPORT_SYMBOL_GPL(ioremap_uc);
>> void __iomem *ioremap_wc(resource_size_t phys_addr, unsigned long size)
>> {
>> return __ioremap_caller(phys_addr, size, _PAGE_CACHE_MODE_WC,
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL(ioremap_wc);
>>
>> @@ -358,14 +360,21 @@ EXPORT_SYMBOL(ioremap_wc);
>> void __iomem *ioremap_wt(resource_size_t phys_addr, unsigned long size)
>> {
>> return __ioremap_caller(phys_addr, size, _PAGE_CACHE_MODE_WT,
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL(ioremap_wt);
>>
>> +void __iomem *ioremap_encrypted(resource_size_t phys_addr, unsigned long size)
>> +{
>> + return __ioremap_caller(phys_addr, size, _PAGE_CACHE_MODE_WB,
>> + __builtin_return_address(0), true);
>> +}
>> +EXPORT_SYMBOL(ioremap_encrypted);
>> +
>> void __iomem *ioremap_cache(resource_size_t phys_addr, unsigned long size)
>> {
>> return __ioremap_caller(phys_addr, size, _PAGE_CACHE_MODE_WB,
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL(ioremap_cache);
>>
>> @@ -374,7 +383,7 @@ void __iomem *ioremap_prot(resource_size_t phys_addr, unsigned long size,
>> {
>> return __ioremap_caller(phys_addr, size,
>> pgprot2cachemode(__pgprot(prot_val)),
>> - __builtin_return_address(0));
>> + __builtin_return_address(0), false);
>> }
>> EXPORT_SYMBOL(ioremap_prot);
>>
>> @@ -688,6 +697,9 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr,
>> if (encrypted_prot && memremap_should_map_decrypted(phys_addr, size))
>> encrypted_prot = false;
>>
>> + if (sme_active() && is_kdump_kernel())
>> + encrypted_prot = false;
>> +
>
> This change doesn't seem to go with the patch description. Maybe it
> should be a separate patch? Or provide more detail in the description
> for this change.
>
> What is this change addressing? What early_memremap() calls were failing
> that required this? If this truly is required, you can probably move it
> up into the "if (sme_active())" block a few lines higher in the function.
>
Thanks. About this issue, you could read the description of cover letter, we put some
logic into the early_memremap_pgprot_adjust() in order to avoid the previous changes,
such as arch/x86/include/asm/dmi.h, arch/x86/kernel/acpi/boot.c and drivers/acpi/tables.c.
They will call early_memremap() to remap the memory, but we need to remap the memory
without encryption.
Lianbo
> Thanks,
> Tom
>
>> return encrypted_prot ? pgprot_encrypted(prot)
>> : pgprot_decrypted(prot);
>> }
>>
Powered by blists - more mailing lists