Re: [PATCH 1/3] kexec: Do not map the kexec area as decrypted when SEV is active
在 2019年03月26日 01:32, Borislav Petkov 写道:
> On Mon, Mar 25, 2019 at 05:17:55PM +, Singh, Brijesh wrote:
>> By default all the memory regions are mapped encrypted. The
>> set_memory_{encrypt,decrypt}() is a generic function which can be
>> called explicitly to clear/set the encryption mask from the existing
>> memory mapping. The mem_encrypt_active() returns true if either SEV or
>> SME is active. So the __set_memory_enc_dec() uses the
>> memory_encrypt_active() check to ensure that the function is no-op when
>> SME/SEV are not active.
>>
>> Currently, the arch_kexec_post_alloc_pages() unconditionally clear the
>> encryption mask from the kexec area. In case of SEV, we should not clear
>> the encryption mask.
>
> Brijesh, I know all that.
>
> Please read what I said here at the end:
>
> https://lkml.kernel.org/r/20190324150034.gh23...@zn.tnic
>
> With this change, the code looks like this:
>
> + if (sme_active())
> + return set_memory_decrypted((unsigned long)vaddr, pages);
>
> now in __set_memory_enc_dec via set_memory_decrypted():
>
> /* Nothing to do if memory encryption is not active */
> if (!mem_encrypt_active())
> return 0;
>
>
> so you have:
>
> if (sme_active())
>
> ...
>
> if (!mem_encrypt_active())
>
>
> now maybe this is all clear to you and Tom but I betcha others will get
> confused. Probably something like "well, what should be active now, SME,
> SEV or memory encryption in general"?
>
> I hope you're catching my drift.
>
> So if you want to *not* decrypt memory in the SEV case, then doing something
> like this should make it a bit more clear:
>
>
> if (sev_active())
> return;
>
> return set_memory_decrypted((unsigned long)vaddr, pages);
>
> along with a comment *why* we're checking here.
It looks good to me. I will improve them next post.
Thank you, everyone.
Lianbo
>
> But actually, I'd prefer if you had separate wrappers which are called
> for SME and for SEV.
>
> I'll let Tom chime in too.
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On 3/25/19 9:32 PM, Borislav Petkov wrote:
> On Mon, Mar 25, 2019 at 08:23:02PM +0800, Dave Young wrote:
>> Kexec saved the original physical addresses, and pass them to kexeced
>> kernel via x86 setup_data, so both the early parsing or efi init code
>> need to get those physical values from setup_data.
>
> So efi_get_rsdp_addr() needs to be refactored in such a way so that at
> least the loop towards the end gets carved out into a separate function
> - __efi_get_rsdp_addr() or so - which gets config_tables, nr_tables and
> size as arguments and finds the RSDP address in the kexec-ed kernel.
Since we still need to read systab for nr_tables and do signature
check to determine if it's 32bit or 64bit for kexec-ed kernel,
everything except the address of config_tables are common between
normal boot and kexec boot.
> So we'd need something like that:
>
> acpi_physical_address get_rsdp_addr(void)
> {
> acpi_physical_address pa;
>
> pa = get_acpi_rsdp();
>
> if (!pa)
> pa = boot_params->acpi_rsdp_addr;
>
> if (!pa)
> pa = efi_get_rsdp_addr();
>
> if (!pa)
> pa = kexec_get_rdsp_addr(); <--- new function
>
> if (!pa)
> pa = bios_get_rsdp_addr();
>
> return pa;
> }
>
> which would get config_tables from setup_data and call
> __efi_get_rsdp_addr() to dig it out in the kexec'ed kernel.
>
> Junichi, ask if it is still unclear what needs to be done.
efi_get_rsdp_addr() and kexec_get_rsdp_addr() could be implemented
like this (sorry about the pseudo code):
/* This is also used to check if the kernel is kexec-ed. */
unsigned long efi_get_setup_data_addr(void) {
return the address of efi_setup_data if kexec-ed or 0 if not;
}
acpi_physical_address __efi_get_rsdp_addr(unsigned long config_tables) {
// Do mostly same as the current efi_get_rsdp_addr().
// If config_tables is non-zero, use it instead of systab->tables.
}
acpi_physical_address efi_get_rsdp_addr(void) {
if (efi_get_setup_data_addr())
return 0;
return __efi_get_rsdp_addr(0);
}
acpi_physical_address kexec_get_rsdp_addr(void) {
esd = (struct efi_setup_data *) efi_get_setup_data_addr();
if (esd && esd->tables)
return __efi_get_rsdp_addr((unsigned long) esd->tables);
return 0;
}
I don't think it looks nice.. Does this match what you envisage?
--
Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 05/27] Copy secure_boot flag in boot params across kexec reboot
From: Dave Young Kexec reboot in case secure boot being enabled does not keep the secure boot mode in new kernel, so later one can load unsigned kernel via legacy kexec_load. In this state, the system is missing the protections provided by secure boot. Adding a patch to fix this by retain the secure_boot flag in original kernel. secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the stub. Fixing this issue by copying secure_boot flag across kexec reboot. Signed-off-by: Dave Young Signed-off-by: David Howells cc: kexec@lists.infradead.org Signed-off-by: Matthew Garrett --- arch/x86/kernel/kexec-bzimage64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 278cd07228dd..d49554b948fd 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -179,6 +179,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, if (efi_enabled(EFI_OLD_MEMMAP)) return 0; + params->secure_boot = boot_params.secure_boot; ei->efi_loader_signature = current_ei->efi_loader_signature; ei->efi_systab = current_ei->efi_systab; ei->efi_systab_hi = current_ei->efi_systab_hi; -- 2.21.0.392.gf8f6787159e-goog ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 07/27] kexec_file: Restrict at runtime if the kernel is locked down
From: Jiri Bohac
When KEXEC_SIG is not enabled, kernel should not load images through
kexec_file systemcall if the kernel is locked down.
[Modified by David Howells to fit with modifications to the previous patch
and to return -EPERM if the kernel is locked down for consistency with
other lockdowns. Modified by Matthew Garrett to remove the IMA
integration, which will be replaced by integrating with the IMA
architecture policy patches.]
Signed-off-by: Jiri Bohac
Signed-off-by: David Howells
Reviewed-by: Jiri Bohac
cc: kexec@lists.infradead.org
Signed-off-by: Matthew Garrett
---
kernel/kexec_file.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 67f3a866eabe..0cfe4f6f7f85 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int
kernel_fd, int initrd_fd,
}
ret = 0;
+
+ if (kernel_is_locked_down(reason)) {
+ ret = -EPERM;
+ goto out;
+ }
+
break;
/* All other errors are fatal, including nomem, unparseable
--
2.21.0.392.gf8f6787159e-goog
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 04/27] kexec_load: Disable at runtime if the kernel is locked down
From: Matthew Garrett
The kexec_load() syscall permits the loading and execution of arbitrary
code in ring 0, which is something that lock-down is meant to prevent. It
makes sense to disable kexec_load() in this situation.
This does not affect kexec_file_load() syscall which can check for a
signature on the image to be booted.
Signed-off-by: Matthew Garrett
Signed-off-by: David Howells
Acked-by: Dave Young
cc: kexec@lists.infradead.org
Signed-off-by: Matthew Garrett
---
kernel/kexec.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 68559808fdfa..8ea0ce31271f 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -207,6 +207,13 @@ static inline int kexec_load_check(unsigned long
nr_segments,
if (result < 0)
return result;
+ /*
+* kexec can be used to circumvent module loading restrictions, so
+* prevent loading in that case
+*/
+ if (kernel_is_locked_down("kexec of unsigned images"))
+ return -EPERM;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
--
2.21.0.392.gf8f6787159e-goog
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 06/27] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Jiri Bohac
This is a preparatory patch for kexec_file_load() lockdown. A locked down
kernel needs to prevent unsigned kernel images from being loaded with
kexec_file_load(). Currently, the only way to force the signature
verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
usigned images even when the kernel is not locked down at runtime.
This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
turns on the signature verification but allows unsigned images to be
loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
[Modified by David Howells such that:
(1) verify_pefile_signature() differentiates between no-signature and
sig-didn't-match in its returned errors.
(2) kexec fails with EKEYREJECTED and logs an appropriate message if
signature checking is enforced and an signature is not found, uses
unsupported crypto or has no matching key.
(3) kexec fails with EKEYREJECTED if there is a signature for which we
have a key, but signature doesn't match - even if in non-forcing mode.
(4) kexec fails with EBADMSG or some other error if there is a signature
which cannot be parsed - even if in non-forcing mode.
(5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
the signature - even if in non-forcing mode.
]
Signed-off-by: Jiri Bohac
Signed-off-by: David Howells
Reviewed-by: Jiri Bohac
cc: kexec@lists.infradead.org
Signed-off-by: Matthew Garrett
---
arch/x86/Kconfig | 20 ---
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c| 48 ++
4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b4a7f32b68e..735d04a4b18f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2016,20 +2016,30 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
- In addition to that option, you need to enable signature
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE, though if
+ there's a signature that we can check, then it must be valid.
+
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.
+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524..4473cea1e877 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
- return -EKEYREJECTED;
+ return -ENODATA;
}
chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int
pelen,
* (*) 0 if at least one signature chain intersects with the keys in the trust
* keyring, or:
*
+ * (*) -ENODATA if there is no signature present.
+ *
* (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
* chain.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b9b1bc5f9669..58b27c7bdc2b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char
*kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -134,7 +134,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t
Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled
On Mon, 2019-03-25 at 16:09 +0800, Dave Young wrote: > Hi Mimi > On 03/22/19 at 03:35pm, Mimi Zohar wrote: > > Verify IMA is enabled before failing tests or emitting irrelevant > > messages. Also, don't skip the test if signatures are not required. > > > > Suggested-by: Dave Young > > Signed-off-by: Mimi Zohar > > --- > > Dave, if this patch resolves the outstanding issues, I can fold these > > changes into the original patches. (Reminder, these patches will need to > > be updated to support the "lockdown" patch set.) > > They looks good to me, thanks for the update I've folded the kexec_file_load changes into the kexec_file_load test. The remaining kexec_load change is left as a separate patch, since it is dependent on the ikconfig change. > Feel free to add my reviewed-by, I did some tests although not cover all > ima cases. Thanks! Is this meant as a general "reviewed-by" for all of the patches or just this specific one? Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3] kexec: Do not map the kexec area as decrypted when SEV is active
On 3/25/19 1:17 PM, Singh, Brijesh wrote:
>
>
> On 3/25/19 12:32 PM, Borislav Petkov wrote:
>> On Mon, Mar 25, 2019 at 05:17:55PM +, Singh, Brijesh wrote:
>>> By default all the memory regions are mapped encrypted. The
>>> set_memory_{encrypt,decrypt}() is a generic function which can be
>>> called explicitly to clear/set the encryption mask from the existing
>>> memory mapping. The mem_encrypt_active() returns true if either SEV or
>>> SME is active. So the __set_memory_enc_dec() uses the
>>> memory_encrypt_active() check to ensure that the function is no-op when
>>> SME/SEV are not active.
>>>
>>> Currently, the arch_kexec_post_alloc_pages() unconditionally clear the
>>> encryption mask from the kexec area. In case of SEV, we should not clear
>>> the encryption mask.
>>
>> Brijesh, I know all that.
>>
>> Please read what I said here at the end:
>>
>> https://lkml.kernel.org/r/20190324150034.gh23...@zn.tnic
>>
>> With this change, the code looks like this:
>>
>> + if (sme_active())
>> + return set_memory_decrypted((unsigned long)vaddr, pages);
>>
>> now in __set_memory_enc_dec via set_memory_decrypted():
>>
>> /* Nothing to do if memory encryption is not active */
>> if (!mem_encrypt_active())
>> return 0;
>>
>>
>> so you have:
>>
>> if (sme_active())
>>
>> ...
>>
>> if (!mem_encrypt_active())
>>
>>
>> now maybe this is all clear to you and Tom but I betcha others will get
>> confused. Probably something like "well, what should be active now, SME,
>> SEV or memory encryption in general"?
>>
>> I hope you're catching my drift.
>>
>> So if you want to *not* decrypt memory in the SEV case, then doing something
>> like this should make it a bit more clear:
>>
>>
>> if (sev_active())
>> return;
>>
>> return set_memory_decrypted((unsigned long)vaddr, pages);
>>
>
>
> I see your point. I agree it can get confusing.
>
>
>> along with a comment *why* we're checking here.
>>
>> But actually, I'd prefer if you had separate wrappers which are called
>> for SME and for SEV.
>
>
> Just a thought, maybe we can move the above if(sev_active()) check up in
> kernel/kexec_core.c because we don't need to set/clear the encryption
> masks when SEV is active so there is no need to call the wrapper.
IIRC, the wrapper was created to avoid checking in the main kexec support
and move the check down to the arch specific support. So I don't think we
should move it.
I'm not sure about the separate wrappers, I like the above code where the
arch callback returns if sev_active() is true.
Maybe what would help is to describe why there is a difference between SME
and SEV in regards to kexec. During a traditional boot under SME, SME will
encrypt the kernel, so the SME kexec kernel also needs to be un-encrypted
in order to replicate a normal SME boot. During a traditional boot under
SEV, the kernel has already been loaded encrypted, so the SEV kexec kernel
needs to be encrypted in order to replicate a normal SEV boot.
Thanks,
Tom
>
>
>>
>> I'll let Tom chime in too.
>>
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
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/3] kexec: Do not map the kexec area as decrypted when SEV is active
On 3/25/19 12:32 PM, Borislav Petkov wrote:
> On Mon, Mar 25, 2019 at 05:17:55PM +, Singh, Brijesh wrote:
>> By default all the memory regions are mapped encrypted. The
>> set_memory_{encrypt,decrypt}() is a generic function which can be
>> called explicitly to clear/set the encryption mask from the existing
>> memory mapping. The mem_encrypt_active() returns true if either SEV or
>> SME is active. So the __set_memory_enc_dec() uses the
>> memory_encrypt_active() check to ensure that the function is no-op when
>> SME/SEV are not active.
>>
>> Currently, the arch_kexec_post_alloc_pages() unconditionally clear the
>> encryption mask from the kexec area. In case of SEV, we should not clear
>> the encryption mask.
>
> Brijesh, I know all that.
>
> Please read what I said here at the end:
>
> https://lkml.kernel.org/r/20190324150034.gh23...@zn.tnic
>
> With this change, the code looks like this:
>
> + if (sme_active())
> + return set_memory_decrypted((unsigned long)vaddr, pages);
>
> now in __set_memory_enc_dec via set_memory_decrypted():
>
> /* Nothing to do if memory encryption is not active */
> if (!mem_encrypt_active())
> return 0;
>
>
> so you have:
>
> if (sme_active())
>
> ...
>
> if (!mem_encrypt_active())
>
>
> now maybe this is all clear to you and Tom but I betcha others will get
> confused. Probably something like "well, what should be active now, SME,
> SEV or memory encryption in general"?
>
> I hope you're catching my drift.
>
> So if you want to *not* decrypt memory in the SEV case, then doing something
> like this should make it a bit more clear:
>
>
> if (sev_active())
> return;
>
> return set_memory_decrypted((unsigned long)vaddr, pages);
>
I see your point. I agree it can get confusing.
> along with a comment *why* we're checking here.
>
> But actually, I'd prefer if you had separate wrappers which are called
> for SME and for SEV.
Just a thought, maybe we can move the above if(sev_active()) check up in
kernel/kexec_core.c because we don't need to set/clear the encryption
masks when SEV is active so there is no need to call the wrapper.
>
> I'll let Tom chime in too.
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3] kexec: Do not map the kexec area as decrypted when SEV is active
On Mon, Mar 25, 2019 at 05:17:55PM +, Singh, Brijesh wrote:
> By default all the memory regions are mapped encrypted. The
> set_memory_{encrypt,decrypt}() is a generic function which can be
> called explicitly to clear/set the encryption mask from the existing
> memory mapping. The mem_encrypt_active() returns true if either SEV or
> SME is active. So the __set_memory_enc_dec() uses the
> memory_encrypt_active() check to ensure that the function is no-op when
> SME/SEV are not active.
>
> Currently, the arch_kexec_post_alloc_pages() unconditionally clear the
> encryption mask from the kexec area. In case of SEV, we should not clear
> the encryption mask.
Brijesh, I know all that.
Please read what I said here at the end:
https://lkml.kernel.org/r/20190324150034.gh23...@zn.tnic
With this change, the code looks like this:
+ if (sme_active())
+ return set_memory_decrypted((unsigned long)vaddr, pages);
now in __set_memory_enc_dec via set_memory_decrypted():
/* Nothing to do if memory encryption is not active */
if (!mem_encrypt_active())
return 0;
so you have:
if (sme_active())
...
if (!mem_encrypt_active())
now maybe this is all clear to you and Tom but I betcha others will get
confused. Probably something like "well, what should be active now, SME,
SEV or memory encryption in general"?
I hope you're catching my drift.
So if you want to *not* decrypt memory in the SEV case, then doing something
like this should make it a bit more clear:
if (sev_active())
return;
return set_memory_decrypted((unsigned long)vaddr, pages);
along with a comment *why* we're checking here.
But actually, I'd prefer if you had separate wrappers which are called
for SME and for SEV.
I'll let Tom chime in too.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3] kexec: Do not map the kexec area as decrypted when SEV is active
Hi Boris,
On 3/25/19 1:37 AM, Borislav Petkov wrote:
> On Mon, Mar 25, 2019 at 09:58:07AM +0800, lijiang wrote:
>> For the SEV virtual machine, it maps the kexec memroy area as
>> encrypted, so, no need to invoke this function to change anything.
>
> Look at the code:
>
> set_memory_decrypted->__set_memory_enc_dec
>
> It already *does* invoke this function.
>
By default all the memory regions are mapped encrypted. The
set_memory_{encrypt,decrypt}() is a generic function which can be
called explicitly to clear/set the encryption mask from the existing
memory mapping. The mem_encrypt_active() returns true if either SEV or
SME is active. So the __set_memory_enc_dec() uses the
memory_encrypt_active() check to ensure that the function is no-op when
SME/SEV are not active.
Currently, the arch_kexec_post_alloc_pages() unconditionally clear the
encryption mask from the kexec area. In case of SEV, we should not clear
the encryption mask.
>>> if (!mem_encrypt_active())
>>>
>>> and heads will spin from all the checking of memory encryption aspects.
>>>
>>> So this would need a rework so that there are no multiple confusing
>>> checks.
>>
>> About the three functions, here i copied their comment from the
>> arch/x86/mm/mem_encrypt.c
>> Please refer to it.
>
> I know that comment - I have asked for it. Now you go and look at the
> code again with your patch applied.
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On Mon, Mar 25, 2019 at 08:23:02PM +0800, Dave Young wrote:
> efi_enter_virtual_mode() can only run once because of efi firmware/spec
> limitation, and after entered virtual mode, efi firmware just updated
I should remember that - I did it at the time.
> Kexec saved the original physical addresses, and pass them to kexeced
> kernel via x86 setup_data, so both the early parsing or efi init code
> need to get those physical values from setup_data.
So efi_get_rsdp_addr() needs to be refactored in such a way so that at
least the loop towards the end gets carved out into a separate function
- __efi_get_rsdp_addr() or so - which gets config_tables, nr_tables and
size as arguments and finds the RSDP address in the kexec-ed kernel.
So we'd need something like that:
acpi_physical_address get_rsdp_addr(void)
{
acpi_physical_address pa;
pa = get_acpi_rsdp();
if (!pa)
pa = boot_params->acpi_rsdp_addr;
if (!pa)
pa = efi_get_rsdp_addr();
if (!pa)
pa = kexec_get_rdsp_addr(); <--- new function
if (!pa)
pa = bios_get_rsdp_addr();
return pa;
}
which would get config_tables from setup_data and call
__efi_get_rsdp_addr() to dig it out in the kexec'ed kernel.
Junichi, ask if it is still unclear what needs to be done.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 2/3 v9] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
On Mon, Mar 25, 2019 at 02:53:02PM +0800, lijiang wrote: > 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. Yes, IORES_DESC_RESERVED is supposed to denote the e820 reserved type. Why should IORES_DESC_NONE include it ?!?! IORES_DESC_NONE is, well, an invalid, i.e., "none" type: /* * I/O Resource Descriptors * * Descriptors are used by walk_iomem_res_desc() and region_intersects() * for searching a specific resource range in the iomem table. Assign * a new descriptor when a resource range supports the search interfaces. * Otherwise, resource.desc must be set to IORES_DESC_NONE (0). */ > Do you mean i should never touch the three chunks? If i made a mistake, i > will remove this changes next post. I'm looking at the hunks below and you're changing ->desc assignments in some random function which doesn't look like you know what you're doing. Maybe it gets you what you want but it sure as hell doesn't look right to me. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On 03/25/19 at 01:01pm, Borislav Petkov wrote: > On Mon, Mar 25, 2019 at 10:36:33AM +, Junichi Nomura wrote: > > AFAIU, early parsing is new code in v5.1-rc1 to support kexec on systems > > with hotpluggable memory with KASLR enabled. For systems that requires the > > new feature, it may be ok to say "you need to use another kexec interface" > > and/or "you need new kexec-tools". > > No, this exactly should *not* happen. kexec is already full of duct tape > - don't need any more of that. > > So I suggested that efi_get_rsdp_addr() should exit early on in the > kexeced kernel but making this all play nice with the kexec-ed kernel, > as Dave suggests, is better. > > Now, my next question is: why does the RDSP address need to come from > kexec(1) (by way of efi_setup_data) and why can't the kexec'ed kernel > figure it out itself by parsing the EFI tables in a similar way to > efi_get_rsdp_addr ? efi_enter_virtual_mode() can only run once because of efi firmware/spec limitation, and after entered virtual mode, efi firmware just updated the original efi sys table, for example the original systab64->fw_vendor, systab64->tables, and even smbios (only found on some HPE machine) are changed from physcial address to virtual address. In the current efi_get_rsdp_addr, it assumes the efi config tables address is not touched (as physical addresses), it will break then. Kexec saved the original physical addresses, and pass them to kexeced kernel via x86 setup_data, so both the early parsing or efi init code need to get those physical values from setup_data. Thanks Dave ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3 v9] x86/mm: Change the examination condition to avoid confusion
On Mon, Mar 25, 2019 at 05:20:43PM +0800, lijiang wrote: > Let's look at the discussion in patch v8, please refer to this link: > https://lkml.org/lkml/2019/3/16/15 > > I did a test according to Tom's reply, and the test indicated his suggestion > was > correct, we should change this to check for IORES_DESC_ACPI_* values. I know that discussion - I was on CC. Your patch still doesn't explain *why* this change is needed and the fact that you "did a test" and it worked doesn't answer my question a single bit. In fact, it tells me that you have tried something at random without even understanding why this is needed and makes me even more suspicious towards what you're doing. So slow down pls *think* why this change is needed and then *explain* that in the commit message. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On Mon, Mar 25, 2019 at 10:36:33AM +, Junichi Nomura wrote: > AFAIU, early parsing is new code in v5.1-rc1 to support kexec on systems > with hotpluggable memory with KASLR enabled. For systems that requires the > new feature, it may be ok to say "you need to use another kexec interface" > and/or "you need new kexec-tools". No, this exactly should *not* happen. kexec is already full of duct tape - don't need any more of that. So I suggested that efi_get_rsdp_addr() should exit early on in the kexeced kernel but making this all play nice with the kexec-ed kernel, as Dave suggests, is better. Now, my next question is: why does the RDSP address need to come from kexec(1) (by way of efi_setup_data) and why can't the kexec'ed kernel figure it out itself by parsing the EFI tables in a similar way to efi_get_rsdp_addr ? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On 03/25/19 at 10:36am, Junichi Nomura wrote: > On 3/25/19 7:15 PM, Dave Young wrote: > > On 03/25/19 at 09:54am, Boris Petkov wrote: > >> On March 25, 2019 9:27:21 AM GMT+01:00, Junichi Nomura > >> wrote: > >>> On 3/25/19 3:59 PM, Dave Young wrote: > On 03/25/19 at 06:47am, Junichi Nomura wrote: > > On 3/25/19 3:19 PM, Dave Young wrote: > >> On 03/25/19 at 02:01pm, Dave Young wrote: > >> I think normally people do not see this bug, because kernel will > >>> set the > >> rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with > > > > I think it's only done for file-based kexec interface. > > Saw Kairui's another reply, yes, kexec-tools need a patch to fill the > value as well then. > > I would vote for a repost of your old patch with some #ifdef > >>> > >>> Thanks for comments, Dave, Kairui and Baoquan. > >>> > >>> The problem for me is it's a regression in v5.1-rc1, that breaks > >>> existing setup. If early parsing of RSDP is required only for newly > >>> supported configuration, I'm fine such configuration requires > >>> new tools or new options. > >>> > >>> This is the 1st version plus #ifdef around the EFI code. > >> > >> I'm going to repeat that again until you get it: > >> > >> If the kexec kernel should continue to use efi_systab_init() then you > >> should make efi_get_rsdp_addr() exit early in the kexec-ed kernel. > > > > In that way, early parsing will fail in kexeced kernel, am I missing > > something? The early code become complicated but since we have already > > the early acpi parsing why not to make it consistent in kexeced kernel? > > AFAIU, early parsing is new code in v5.1-rc1 to support kexec on systems > with hotpluggable memory with KASLR enabled. For systems that requires the > new feature, it may be ok to say "you need to use another kexec interface" > and/or "you need new kexec-tools". Ok, it makes some sense, Kairui mentioned he will work on a kexec-tools patch then this becomes a must. Thanks Dave ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On 3/25/19 7:15 PM, Dave Young wrote: > On 03/25/19 at 09:54am, Boris Petkov wrote: >> On March 25, 2019 9:27:21 AM GMT+01:00, Junichi Nomura >> wrote: >>> On 3/25/19 3:59 PM, Dave Young wrote: On 03/25/19 at 06:47am, Junichi Nomura wrote: > On 3/25/19 3:19 PM, Dave Young wrote: >> On 03/25/19 at 02:01pm, Dave Young wrote: >> I think normally people do not see this bug, because kernel will >>> set the >> rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with > > I think it's only done for file-based kexec interface. Saw Kairui's another reply, yes, kexec-tools need a patch to fill the value as well then. I would vote for a repost of your old patch with some #ifdef >>> >>> Thanks for comments, Dave, Kairui and Baoquan. >>> >>> The problem for me is it's a regression in v5.1-rc1, that breaks >>> existing setup. If early parsing of RSDP is required only for newly >>> supported configuration, I'm fine such configuration requires >>> new tools or new options. >>> >>> This is the 1st version plus #ifdef around the EFI code. >> >> I'm going to repeat that again until you get it: >> >> If the kexec kernel should continue to use efi_systab_init() then you >> should make efi_get_rsdp_addr() exit early in the kexec-ed kernel. > > In that way, early parsing will fail in kexeced kernel, am I missing > something? The early code become complicated but since we have already > the early acpi parsing why not to make it consistent in kexeced kernel? AFAIU, early parsing is new code in v5.1-rc1 to support kexec on systems with hotpluggable memory with KASLR enabled. For systems that requires the new feature, it may be ok to say "you need to use another kexec interface" and/or "you need new kexec-tools". -- Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On 03/25/19 at 09:54am, Boris Petkov wrote: > On March 25, 2019 9:27:21 AM GMT+01:00, Junichi Nomura > wrote: > >On 3/25/19 3:59 PM, Dave Young wrote: > >> On 03/25/19 at 06:47am, Junichi Nomura wrote: > >>> On 3/25/19 3:19 PM, Dave Young wrote: > On 03/25/19 at 02:01pm, Dave Young wrote: > I think normally people do not see this bug, because kernel will > >set the > rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with > >>> > >>> I think it's only done for file-based kexec interface. > >> > >> Saw Kairui's another reply, yes, kexec-tools need a patch to fill the > >> value as well then. > >> > >> I would vote for a repost of your old patch with some #ifdef > > > >Thanks for comments, Dave, Kairui and Baoquan. > > > >The problem for me is it's a regression in v5.1-rc1, that breaks > >existing setup. If early parsing of RSDP is required only for newly > >supported configuration, I'm fine such configuration requires > >new tools or new options. > > > >This is the 1st version plus #ifdef around the EFI code. > > I'm going to repeat that again until you get it: > > If the kexec kernel should continue to use efi_systab_init() then you > should make efi_get_rsdp_addr() exit early in the kexec-ed kernel. In that way, early parsing will fail in kexeced kernel, am I missing something? The early code become complicated but since we have already the early acpi parsing why not to make it consistent in kexeced kernel? Thanks Dave ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2] x86/boot: Don't try to search RSDP from EFI when kexec-booted
On 3/25/19 5:54 PM, Boris Petkov wrote:
> I'm going to repeat that again until you get it:
>
> If the kexec kernel should continue to use efi_systab_init() then you
> should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
Do you think this one is ok? Either works for me.
[PATCH v2] x86/boot: Don't try to search RSDP from EFI when kexec-booted
Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
in the early parsing code tries to search RSDP from EFI table but
whose address is virtual.
Normally kexec(1) provides physical address of config_table via boot_params
and EFI code uses that during initialization.
For the early boot code, we just exit efi_get_rsdp_addr() early if the kernel
is booted by kexec.
Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in boot_params")
Signed-off-by: Jun'ichi Nomura
Cc: Chao Fan
Cc: Borislav Petkov
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -44,6 +44,26 @@ static acpi_physical_address get_acpi_rsdp(void)
return addr;
}
+#ifdef CONFIG_EFI
+static bool is_kexec_booted(void)
+{
+ struct setup_data *data;
+
+ /*
+* kexec-tools provides EFI setup data so that kexec-ed kernel
+* can find proper tables.
+*/
+ data = (struct setup_data *) boot_params->hdr.setup_data;
+ while (data) {
+ if (data->type == SETUP_EFI)
+ return true;
+ data = (struct setup_data *) data->next;
+ }
+
+ return false;
+}
+#endif
+
/* Search EFI system tables for RSDP. */
static acpi_physical_address efi_get_rsdp_addr(void)
{
@@ -57,6 +77,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
int size, i;
char *sig;
+ /* If the system is kexec-booted, poking EFI systab may not work. */
+ if (is_kexec_booted())
+ return 0;
+
ei = _params->efi_info;
sig = (char *)>efi_loader_signature;
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3 v9] x86/mm: Change the examination condition to avoid confusion
在 2019年03月25日 14:40, Borislav Petkov 写道: > On Mon, Mar 25, 2019 at 11:11:45AM +0800, lijiang wrote: >> I mean it needs to find all the value of the 'IORES_DESC_ACPI_*' type. > > A function called __ioremap_check_desc_other() needs to find > IORES_DESC_ACPI_* types... > > No, still don't know what you're trying to do. Let's look at the discussion in patch v8, please refer to this link: https://lkml.org/lkml/2019/3/16/15 I did a test according to Tom's reply, and the test indicated his suggestion was correct, we should change this to check for IORES_DESC_ACPI_* values. > >> As above mentioned, it needs to find all the value of the 'IORES_DESC_ACPI_*' >> type, so we should explicitly use the 'IORES_DESC_ACPI_*' type as the check >> condition instead of the 'IORES_DESC_NONE'. > > And now the same question I'm asking you each time: WHY does it need to find > the ACPI types? > When SEV is enabled and the page being mapped is in memory, need to ensure the memory encryption attribute is also enabled in the resulting mapping. I believe Tom knows better than me. :-) Thanks. Lianbo ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86/boot: Use EFI setup data if provided
On March 25, 2019 9:27:21 AM GMT+01:00, Junichi Nomura wrote: >On 3/25/19 3:59 PM, Dave Young wrote: >> On 03/25/19 at 06:47am, Junichi Nomura wrote: >>> On 3/25/19 3:19 PM, Dave Young wrote: On 03/25/19 at 02:01pm, Dave Young wrote: I think normally people do not see this bug, because kernel will >set the rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with >>> >>> I think it's only done for file-based kexec interface. >> >> Saw Kairui's another reply, yes, kexec-tools need a patch to fill the >> value as well then. >> >> I would vote for a repost of your old patch with some #ifdef > >Thanks for comments, Dave, Kairui and Baoquan. > >The problem for me is it's a regression in v5.1-rc1, that breaks >existing setup. If early parsing of RSDP is required only for newly >supported configuration, I'm fine such configuration requires >new tools or new options. > >This is the 1st version plus #ifdef around the EFI code. I'm going to repeat that again until you get it: If the kexec kernel should continue to use efi_systab_init() then you should make efi_get_rsdp_addr() exit early in the kexec-ed kernel. -- Sent from a small device: formatting sux and brevity is inevitable. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2] x86/boot: Use EFI setup data if provided
On 3/25/19 3:59 PM, Dave Young wrote:
> On 03/25/19 at 06:47am, Junichi Nomura wrote:
>> On 3/25/19 3:19 PM, Dave Young wrote:
>>> On 03/25/19 at 02:01pm, Dave Young wrote:
>>> I think normally people do not see this bug, because kernel will set the
>>> rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with
>>
>> I think it's only done for file-based kexec interface.
>
> Saw Kairui's another reply, yes, kexec-tools need a patch to fill the
> value as well then.
>
> I would vote for a repost of your old patch with some #ifdef
Thanks for comments, Dave, Kairui and Baoquan.
The problem for me is it's a regression in v5.1-rc1, that breaks
existing setup. If early parsing of RSDP is required only for newly
supported configuration, I'm fine such configuration requires
new tools or new options.
This is the 1st version plus #ifdef around the EFI code.
Build tested both with and without CONFIG_EFI to see no warnings.
[PATCH v2] x86/boot: Use EFI setup data if provided
Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
in the early parsing code tries to search RSDP from EFI table but
whose address is virtual.
Since kexec(1) provides physical address of config_table via boot_params,
efi_get_rsdp_addr() should look for setup_data in the same way as
efi_systab_init() in arch/x86/platform/efi/efi.c does.
Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in boot_params")
Signed-off-by: Jun'ichi Nomura
Acked-by: Dave Young
Cc: Chao Fan
Cc: Borislav Petkov
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
return addr;
}
+#ifdef CONFIG_EFI
+static unsigned long efi_get_setup_data_addr(void)
+{
+ struct setup_data *data;
+ u64 pa_data;
+
+ pa_data = boot_params->hdr.setup_data;
+ while (pa_data) {
+ data = (struct setup_data *) pa_data;
+ if (data->type == SETUP_EFI)
+ return pa_data + sizeof(struct setup_data);
+ pa_data = data->next;
+ }
+
+ return 0;
+}
+#endif
+
/* Search EFI system tables for RSDP. */
static acpi_physical_address efi_get_rsdp_addr(void)
{
@@ -53,10 +71,12 @@ static acpi_physical_address efi_get_rsdp_addr(void)
unsigned long systab, systab_tables, config_tables;
unsigned int nr_tables;
struct efi_info *ei;
+ struct efi_setup_data *esd;
bool efi_64;
int size, i;
char *sig;
+ esd = (struct efi_setup_data *) efi_get_setup_data_addr();
ei = _params->efi_info;
sig = (char *)>efi_loader_signature;
@@ -86,13 +106,13 @@ static acpi_physical_address efi_get_rsdp_addr(void)
if (efi_64) {
efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab;
- config_tables = stbl->tables;
+ config_tables = esd ? esd->tables : stbl->tables;
nr_tables = stbl->nr_tables;
size= sizeof(efi_config_table_64_t);
} else {
efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab;
- config_tables = stbl->tables;
+ config_tables = esd ? esd->tables : stbl->tables;
nr_tables = stbl->nr_tables;
size= sizeof(efi_config_table_32_t);
}
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled
Hi Mimi On 03/22/19 at 03:35pm, Mimi Zohar wrote: > Verify IMA is enabled before failing tests or emitting irrelevant > messages. Also, don't skip the test if signatures are not required. > > Suggested-by: Dave Young > Signed-off-by: Mimi Zohar > --- > Dave, if this patch resolves the outstanding issues, I can fold these > changes into the original patches. (Reminder, these patches will need to > be updated to support the "lockdown" patch set.) They looks good to me, thanks for the update Feel free to add my reviewed-by, I did some tests although not cover all ima cases. Thanks Dave > > .../selftests/kexec/test_kexec_file_load.sh| 27 > ++ > tools/testing/selftests/kexec/test_kexec_load.sh | 24 --- > 2 files changed, 33 insertions(+), 18 deletions(-) > > diff --git a/tools/testing/selftests/kexec/test_kexec_file_load.sh > b/tools/testing/selftests/kexec/test_kexec_file_load.sh > index 1d2e5e799523..57b636792086 100755 > --- a/tools/testing/selftests/kexec/test_kexec_file_load.sh > +++ b/tools/testing/selftests/kexec/test_kexec_file_load.sh > @@ -110,11 +110,20 @@ kexec_file_load_test() > log_fail "$succeed_msg (missing IMA sig)" > fi > > - if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \ > - && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then > + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \ > + && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \ > + && [ $ima_read_policy -eq 0 ]; then > log_fail "$succeed_msg (possibly missing IMA sig)" > fi > > + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 0 ]; then > + log_info "No signature verification required" > + elif [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \ > + && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \ > + && [ $ima_read_policy -eq 1 ]; then > + log_info "No signature verification required" > + fi > + > log_pass "$succeed_msg" > fi > > @@ -136,8 +145,9 @@ kexec_file_load_test() > log_pass "$failed_msg (missing IMA sig)" > fi > > - if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \ > - && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then > + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \ > + && [ $ima_sig_required -eq 0 ] && [ $ima_read_policy -eq 0 ] \ > + && [ $ima_signed -eq 0 ]; then > log_pass "$failed_msg (possibly missing IMA sig)" > fi > > @@ -157,6 +167,9 @@ if [ $? -eq 0 ]; then > fi > > # Determine which kernel config options are enabled > +kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled" > +ima_appraise=$? > + > kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \ > "architecture specific policy enabled" > arch_policy=$? > @@ -178,12 +191,6 @@ ima_sig_required=$? > get_secureboot_mode > secureboot=$? > > -if [ $secureboot -eq 0 ] && [ $arch_policy -eq 0 ] && \ > - [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] && \ > - [ $ima_read_policy -eq 1 ]; then > - log_skip "No signature verification required" > -fi > - > # Are there pe and ima signatures > check_for_pesig > pe_signed=$? > diff --git a/tools/testing/selftests/kexec/test_kexec_load.sh > b/tools/testing/selftests/kexec/test_kexec_load.sh > index 2a66c8897f55..49c6aa929137 100755 > --- a/tools/testing/selftests/kexec/test_kexec_load.sh > +++ b/tools/testing/selftests/kexec/test_kexec_load.sh > @@ -1,8 +1,8 @@ > #!/bin/sh > # SPDX-License-Identifier: GPL-2.0 > -# Loading a kernel image via the kexec_load syscall should fail > -# when the kernel is CONFIG_KEXEC_VERIFY_SIG enabled and the system > -# is booted in secureboot mode. > +# > +# Prevent loading a kernel image via the kexec_load syscall when > +# signatures are required. (Dependent on CONFIG_IMA_ARCH_POLICY.) > > TEST="$0" > . ./kexec_common_lib.sh > @@ -18,20 +18,28 @@ if [ $? -eq 0 ]; then > log_skip "kexec_load is not enabled" > fi > > +kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled" > +ima_appraise=$? > + > +kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \ > + "IMA architecture specific policy enabled" > +arch_policy=$? > + > get_secureboot_mode > secureboot=$? > > -# kexec_load should fail in secure boot mode > +# kexec_load should fail in secure boot mode and CONFIG_IMA_ARCH_POLICY > enabled > kexec --load $KERNEL_IMAGE > /dev/null 2>&1 > if [ $? -eq 0 ]; then > kexec --unload > - if [ $secureboot -eq 1 ]; then > + if [ $secureboot -eq 1 ] && [ $arch_policy -eq 1 ]; then > log_fail "kexec_load succeeded" > - else > - log_pass "kexec_load succeeded" > + elif [ $ima_appraise -eq 0 -o $arch_policy -eq 0 ];
Re: [PATCH] x86/boot: Use EFI setup data if provided
On 03/25/19 at 06:47am, Junichi Nomura wrote:
> On 3/25/19 3:19 PM, Dave Young wrote:
> > On 03/25/19 at 02:01pm, Dave Young wrote:
> >> On 03/25/19 at 12:27am, Junichi Nomura wrote:
> >>> On Fri, Mar 22, 2019 at 04:23:28PM +0100, Borislav Petkov wrote:
> On Fri, Mar 22, 2019 at 11:03:43AM +, Junichi Nomura wrote:
> > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > in the early parsing code tries to search RSDP from EFI table but
> > whose address is virtual.
> >
> > Since kexec(1) provides physical address of config_table via
> > boot_params,
> > efi_get_rsdp_addr() should look for setup_data in the same way as
> > efi_systab_init() in arch/x86/platform/efi/efi.c does.
>
> If the kexec kernel should continue to use efi_systab_init() then you
> should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
> >>>
> >>> I'm not sure which way kexec devel is going. Added kexec list.
> >>> Here is the version that exits early in efi_get_rsdp_addr().
> >>>
> >>> [PATCH] x86/boot: Don't try to search RSDP from EFI when kexec-booted
> >>>
> >>> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> >>> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> >>> in the early parsing code tries to search RSDP from EFI table but
> >>> whose address is virtual.
> >>>
> >>> Normally kexec(1) provides physical address of config_table via
> >>> boot_params
> >>> and EFI code uses that during initialization.
> >>> For the early boot code, we just exit efi_get_rsdp_addr() early if the
> >>> kernel
> >>> is booted by kexec.
> >>>
> >>> Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> >>> boot_params")
> >>> Signed-off-by: Jun'ichi Nomura
> >>> Cc: Chao Fan
> >>> Cc: Borislav Petkov
> >>>
> >>> diff --git a/arch/x86/boot/compressed/acpi.c
> >>> b/arch/x86/boot/compressed/acpi.c
> >>> index 0ef4ad5..1cefc43 100644
> >>> --- a/arch/x86/boot/compressed/acpi.c
> >>> +++ b/arch/x86/boot/compressed/acpi.c
> >>> @@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
> >>> return addr;
> >>> }
> >>>
> >>> +static bool is_kexec_booted(void)
> >>> +{
> >>> + struct setup_data *data;
> >>> +
> >>> + /*
> >>> + * kexec-tools provides EFI setup data so that kexec-ed kernel
> >>> + * can find proper tables.
> >>> + */
> >>> + data = (struct setup_data *) boot_params->hdr.setup_data;
> >>> + while (data) {
> >>> + if (data->type == SETUP_EFI)
> >>> + return true;
> >>> + data = (struct setup_data *) data->next;
> >>> + }
> >>> +
> >>> + return false;
> >>> +}
> >>> +
> >>> /* Search EFI system tables for RSDP. */
> >>> static acpi_physical_address efi_get_rsdp_addr(void)
> >>> {
> >>> @@ -57,6 +75,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
> >>> int size, i;
> >>> char *sig;
> >>>
> >>> + /* If the system is kexec-booted, poking EFI systab may not work. */
> >>> + if (is_kexec_booted())
> >>> + return 0;
> >>> +
> >>> ei = _params->efi_info;
> >>> sig = (char *)>efi_loader_signature;
> >>>
> >>>
> >>> ___
> >>> kexec mailing list
> >>> kexec@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/kexec
> >>
> >> Good catch, this way looks good to me. But the function
> >> is_kexec_booted can be compiled when #ifdef CONFIG_EFI
> >>
> >> Otherwise:
> >>
> >> Acked-by: Dave Young
> >>
> >
> > Hold on, I replied too quick. One question is does the above patch
> > passed your test? It can workaround and skip the wrong phys addr
> > issue, but the acpi early parsing still fails because efi_get_rsdp_addr
> > return 0?
>
> The patch works for me.
> Early parsing fails with the 2nd patch but it boots fine.
> EFI initialization is done later without boot_params->acpi_rsdp_addr.
> I think that's how v5.0 and earlier kernels work.
Ok, thanks
>
> > If this is the case you may need go with your old patch.
> >
> > I think normally people do not see this bug, because kernel will set the
> > rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with
>
> I think it's only done for file-based kexec interface.
Saw Kairui's another reply, yes, kexec-tools need a patch to fill the
value as well then.
I would vote for a repost of your old patch with some #ifdef
Thanks
Dave
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
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
>> Signed-off-by: Lianbo Jiang
>> ---
>> 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
Re: [PATCH] x86/boot: Use EFI setup data if provided
On 3/25/19 3:19 PM, Dave Young wrote:
> On 03/25/19 at 02:01pm, Dave Young wrote:
>> On 03/25/19 at 12:27am, Junichi Nomura wrote:
>>> On Fri, Mar 22, 2019 at 04:23:28PM +0100, Borislav Petkov wrote:
On Fri, Mar 22, 2019 at 11:03:43AM +, Junichi Nomura wrote:
> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> in the early parsing code tries to search RSDP from EFI table but
> whose address is virtual.
>
> Since kexec(1) provides physical address of config_table via boot_params,
> efi_get_rsdp_addr() should look for setup_data in the same way as
> efi_systab_init() in arch/x86/platform/efi/efi.c does.
If the kexec kernel should continue to use efi_systab_init() then you
should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
>>>
>>> I'm not sure which way kexec devel is going. Added kexec list.
>>> Here is the version that exits early in efi_get_rsdp_addr().
>>>
>>> [PATCH] x86/boot: Don't try to search RSDP from EFI when kexec-booted
>>>
>>> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
>>> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
>>> in the early parsing code tries to search RSDP from EFI table but
>>> whose address is virtual.
>>>
>>> Normally kexec(1) provides physical address of config_table via boot_params
>>> and EFI code uses that during initialization.
>>> For the early boot code, we just exit efi_get_rsdp_addr() early if the
>>> kernel
>>> is booted by kexec.
>>>
>>> Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
>>> boot_params")
>>> Signed-off-by: Jun'ichi Nomura
>>> Cc: Chao Fan
>>> Cc: Borislav Petkov
>>>
>>> diff --git a/arch/x86/boot/compressed/acpi.c
>>> b/arch/x86/boot/compressed/acpi.c
>>> index 0ef4ad5..1cefc43 100644
>>> --- a/arch/x86/boot/compressed/acpi.c
>>> +++ b/arch/x86/boot/compressed/acpi.c
>>> @@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
>>> return addr;
>>> }
>>>
>>> +static bool is_kexec_booted(void)
>>> +{
>>> + struct setup_data *data;
>>> +
>>> + /*
>>> +* kexec-tools provides EFI setup data so that kexec-ed kernel
>>> +* can find proper tables.
>>> +*/
>>> + data = (struct setup_data *) boot_params->hdr.setup_data;
>>> + while (data) {
>>> + if (data->type == SETUP_EFI)
>>> + return true;
>>> + data = (struct setup_data *) data->next;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> /* Search EFI system tables for RSDP. */
>>> static acpi_physical_address efi_get_rsdp_addr(void)
>>> {
>>> @@ -57,6 +75,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
>>> int size, i;
>>> char *sig;
>>>
>>> + /* If the system is kexec-booted, poking EFI systab may not work. */
>>> + if (is_kexec_booted())
>>> + return 0;
>>> +
>>> ei = _params->efi_info;
>>> sig = (char *)>efi_loader_signature;
>>>
>>>
>>> ___
>>> kexec mailing list
>>> kexec@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/kexec
>>
>> Good catch, this way looks good to me. But the function
>> is_kexec_booted can be compiled when #ifdef CONFIG_EFI
>>
>> Otherwise:
>>
>> Acked-by: Dave Young
>>
>
> Hold on, I replied too quick. One question is does the above patch
> passed your test? It can workaround and skip the wrong phys addr
> issue, but the acpi early parsing still fails because efi_get_rsdp_addr
> return 0?
The patch works for me.
Early parsing fails with the 2nd patch but it boots fine.
EFI initialization is done later without boot_params->acpi_rsdp_addr.
I think that's how v5.0 and earlier kernels work.
> If this is the case you may need go with your old patch.
>
> I think normally people do not see this bug, because kernel will set the
> rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with
I think it's only done for file-based kexec interface.
> different kernel versions, eg.
>
> old kernel kexec to new kernel.
>
> And the old kernel does not set boot_params->acpi_rsdp_addr
>
> Is this correct?
I'm testing kexec from v5.1-rc1 to v5.1-rc1, i.e. same kernel.
--
Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] x86/boot: Use EFI setup data if provided
On Mon, Mar 25, 2019 at 2:20 PM Dave Young wrote:
>
> On 03/25/19 at 02:01pm, Dave Young wrote:
> > On 03/25/19 at 12:27am, Junichi Nomura wrote:
> > > On Fri, Mar 22, 2019 at 04:23:28PM +0100, Borislav Petkov wrote:
> > > > On Fri, Mar 22, 2019 at 11:03:43AM +, Junichi Nomura wrote:
> > > > > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > > > > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > > > > in the early parsing code tries to search RSDP from EFI table but
> > > > > whose address is virtual.
> > > > >
> > > > > Since kexec(1) provides physical address of config_table via
> > > > > boot_params,
> > > > > efi_get_rsdp_addr() should look for setup_data in the same way as
> > > > > efi_systab_init() in arch/x86/platform/efi/efi.c does.
> > > >
> > > > If the kexec kernel should continue to use efi_systab_init() then you
> > > > should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
> > >
> > > I'm not sure which way kexec devel is going. Added kexec list.
> > > Here is the version that exits early in efi_get_rsdp_addr().
> > >
> > > [PATCH] x86/boot: Don't try to search RSDP from EFI when kexec-booted
> > >
> > > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > > in the early parsing code tries to search RSDP from EFI table but
> > > whose address is virtual.
> > >
> > > Normally kexec(1) provides physical address of config_table via
> > > boot_params
> > > and EFI code uses that during initialization.
> > > For the early boot code, we just exit efi_get_rsdp_addr() early if the
> > > kernel
> > > is booted by kexec.
> > >
> > > Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > > boot_params")
> > > Signed-off-by: Jun'ichi Nomura
> > > Cc: Chao Fan
> > > Cc: Borislav Petkov
> > >
> > > diff --git a/arch/x86/boot/compressed/acpi.c
> > > b/arch/x86/boot/compressed/acpi.c
> > > index 0ef4ad5..1cefc43 100644
> > > --- a/arch/x86/boot/compressed/acpi.c
> > > +++ b/arch/x86/boot/compressed/acpi.c
> > > @@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
> > > return addr;
> > > }
> > >
> > > +static bool is_kexec_booted(void)
> > > +{
> > > + struct setup_data *data;
> > > +
> > > + /*
> > > +* kexec-tools provides EFI setup data so that kexec-ed kernel
> > > +* can find proper tables.
> > > +*/
> > > + data = (struct setup_data *) boot_params->hdr.setup_data;
> > > + while (data) {
> > > + if (data->type == SETUP_EFI)
> > > + return true;
> > > + data = (struct setup_data *) data->next;
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > /* Search EFI system tables for RSDP. */
> > > static acpi_physical_address efi_get_rsdp_addr(void)
> > > {
> > > @@ -57,6 +75,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
> > > int size, i;
> > > char *sig;
> > >
> > > + /* If the system is kexec-booted, poking EFI systab may not work. */
> > > + if (is_kexec_booted())
> > > + return 0;
> > > +
> > > ei = _params->efi_info;
> > > sig = (char *)>efi_loader_signature;
> > >
> > >
> > > ___
> > > kexec mailing list
> > > kexec@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/kexec
> >
> > Good catch, this way looks good to me. But the function
> > is_kexec_booted can be compiled when #ifdef CONFIG_EFI
> >
> > Otherwise:
> >
> > Acked-by: Dave Young
> >
>
> Hold on, I replied too quick. One question is does the above patch
> passed your test? It can workaround and skip the wrong phys addr
> issue, but the acpi early parsing still fails because efi_get_rsdp_addr
> return 0?
>
> If this is the case you may need go with your old patch.
>
> I think normally people do not see this bug, because kernel will set the
> rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with
> different kernel versions, eg.
>
> old kernel kexec to new kernel.
>
> And the old kernel does not set boot_params->acpi_rsdp_addr
>
> Is this correct?
>
> Thanks
> Dave
Hi Dave, actually only kexec_file_load will always set the
boot_params->acpi_rsdp_addr. Can't guarantee how user space tools will
prepare the boot_prams if kexec_load is used, so it's should very
likely to happen.
And for the patch, I also think the first patch looks better, if we
just return 0 early in efi_get_rsdp_addr aren't we still failing to
parse the rsdp in early code?
--
Best Regards,
Kairui Song
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3 v9] x86/mm: Change the examination condition to avoid confusion
On Mon, Mar 25, 2019 at 11:11:45AM +0800, lijiang wrote: > I mean it needs to find all the value of the 'IORES_DESC_ACPI_*' type. A function called __ioremap_check_desc_other() needs to find IORES_DESC_ACPI_* types... No, still don't know what you're trying to do. > As above mentioned, it needs to find all the value of the 'IORES_DESC_ACPI_*' > type, so we should explicitly use the 'IORES_DESC_ACPI_*' type as the check > condition instead of the 'IORES_DESC_NONE'. And now the same question I'm asking you each time: WHY does it need to find the ACPI types? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3] kexec: Do not map the kexec area as decrypted when SEV is active
On Mon, Mar 25, 2019 at 09:58:07AM +0800, lijiang wrote: > For the SEV virtual machine, it maps the kexec memroy area as > encrypted, so, no need to invoke this function to change anything. Look at the code: set_memory_decrypted->__set_memory_enc_dec It already *does* invoke this function. > > if (!mem_encrypt_active()) > > > > and heads will spin from all the checking of memory encryption aspects. > > > > So this would need a rework so that there are no multiple confusing > > checks. > > About the three functions, here i copied their comment from the > arch/x86/mm/mem_encrypt.c > Please refer to it. I know that comment - I have asked for it. Now you go and look at the code again with your patch applied. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] x86/boot: Use EFI setup data if provided
On 03/25/19 at 02:01pm, Dave Young wrote:
> On 03/25/19 at 12:27am, Junichi Nomura wrote:
> > On Fri, Mar 22, 2019 at 04:23:28PM +0100, Borislav Petkov wrote:
> > > On Fri, Mar 22, 2019 at 11:03:43AM +, Junichi Nomura wrote:
> > > > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > > > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > > > in the early parsing code tries to search RSDP from EFI table but
> > > > whose address is virtual.
> > > >
> > > > Since kexec(1) provides physical address of config_table via
> > > > boot_params,
> > > > efi_get_rsdp_addr() should look for setup_data in the same way as
> > > > efi_systab_init() in arch/x86/platform/efi/efi.c does.
> > >
> > > If the kexec kernel should continue to use efi_systab_init() then you
> > > should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
> >
> > I'm not sure which way kexec devel is going. Added kexec list.
> > Here is the version that exits early in efi_get_rsdp_addr().
> >
> > [PATCH] x86/boot: Don't try to search RSDP from EFI when kexec-booted
> >
> > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > in the early parsing code tries to search RSDP from EFI table but
> > whose address is virtual.
> >
> > Normally kexec(1) provides physical address of config_table via boot_params
> > and EFI code uses that during initialization.
> > For the early boot code, we just exit efi_get_rsdp_addr() early if the
> > kernel
> > is booted by kexec.
> >
> > Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > boot_params")
> > Signed-off-by: Jun'ichi Nomura
> > Cc: Chao Fan
> > Cc: Borislav Petkov
> >
> > diff --git a/arch/x86/boot/compressed/acpi.c
> > b/arch/x86/boot/compressed/acpi.c
> > index 0ef4ad5..1cefc43 100644
> > --- a/arch/x86/boot/compressed/acpi.c
> > +++ b/arch/x86/boot/compressed/acpi.c
> > @@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
> > return addr;
> > }
> >
> > +static bool is_kexec_booted(void)
> > +{
> > + struct setup_data *data;
> > +
> > + /*
> > +* kexec-tools provides EFI setup data so that kexec-ed kernel
> > +* can find proper tables.
> > +*/
> > + data = (struct setup_data *) boot_params->hdr.setup_data;
> > + while (data) {
> > + if (data->type == SETUP_EFI)
> > + return true;
> > + data = (struct setup_data *) data->next;
> > + }
> > +
> > + return false;
> > +}
> > +
> > /* Search EFI system tables for RSDP. */
> > static acpi_physical_address efi_get_rsdp_addr(void)
> > {
> > @@ -57,6 +75,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
> > int size, i;
> > char *sig;
> >
> > + /* If the system is kexec-booted, poking EFI systab may not work. */
> > + if (is_kexec_booted())
> > + return 0;
> > +
> > ei = _params->efi_info;
> > sig = (char *)>efi_loader_signature;
> >
> >
> > ___
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
>
> Good catch, this way looks good to me. But the function
> is_kexec_booted can be compiled when #ifdef CONFIG_EFI
>
> Otherwise:
>
> Acked-by: Dave Young
>
Hold on, I replied too quick. One question is does the above patch
passed your test? It can workaround and skip the wrong phys addr
issue, but the acpi early parsing still fails because efi_get_rsdp_addr
return 0?
If this is the case you may need go with your old patch.
I think normally people do not see this bug, because kernel will set the
rsdp in boot_params->acpi_rsdp_addr. Maybe you are testing with
different kernel versions, eg.
old kernel kexec to new kernel.
And the old kernel does not set boot_params->acpi_rsdp_addr
Is this correct?
Thanks
Dave
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] x86/boot: Use EFI setup data if provided
On 03/25/19 at 12:27am, Junichi Nomura wrote:
> On Fri, Mar 22, 2019 at 04:23:28PM +0100, Borislav Petkov wrote:
> > On Fri, Mar 22, 2019 at 11:03:43AM +, Junichi Nomura wrote:
> > > Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > > boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > > in the early parsing code tries to search RSDP from EFI table but
> > > whose address is virtual.
> > >
> > > Since kexec(1) provides physical address of config_table via boot_params,
> > > efi_get_rsdp_addr() should look for setup_data in the same way as
> > > efi_systab_init() in arch/x86/platform/efi/efi.c does.
> >
> > If the kexec kernel should continue to use efi_systab_init() then you
> > should make efi_get_rsdp_addr() exit early in the kexec-ed kernel.
>
> I'm not sure which way kexec devel is going. Added kexec list.
> Here is the version that exits early in efi_get_rsdp_addr().
>
> [PATCH] x86/boot: Don't try to search RSDP from EFI when kexec-booted
>
> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> in the early parsing code tries to search RSDP from EFI table but
> whose address is virtual.
>
> Normally kexec(1) provides physical address of config_table via boot_params
> and EFI code uses that during initialization.
> For the early boot code, we just exit efi_get_rsdp_addr() early if the kernel
> is booted by kexec.
>
> Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in boot_params")
> Signed-off-by: Jun'ichi Nomura
> Cc: Chao Fan
> Cc: Borislav Petkov
>
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> index 0ef4ad5..1cefc43 100644
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -44,6 +44,24 @@ static acpi_physical_address get_acpi_rsdp(void)
> return addr;
> }
>
> +static bool is_kexec_booted(void)
> +{
> + struct setup_data *data;
> +
> + /*
> + * kexec-tools provides EFI setup data so that kexec-ed kernel
> + * can find proper tables.
> + */
> + data = (struct setup_data *) boot_params->hdr.setup_data;
> + while (data) {
> + if (data->type == SETUP_EFI)
> + return true;
> + data = (struct setup_data *) data->next;
> + }
> +
> + return false;
> +}
> +
> /* Search EFI system tables for RSDP. */
> static acpi_physical_address efi_get_rsdp_addr(void)
> {
> @@ -57,6 +75,10 @@ static acpi_physical_address efi_get_rsdp_addr(void)
> int size, i;
> char *sig;
>
> + /* If the system is kexec-booted, poking EFI systab may not work. */
> + if (is_kexec_booted())
> + return 0;
> +
> ei = _params->efi_info;
> sig = (char *)>efi_loader_signature;
>
>
> ___
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Good catch, this way looks good to me. But the function
is_kexec_booted can be compiled when #ifdef CONFIG_EFI
Otherwise:
Acked-by: Dave Young
Thanks
Dave
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
