Re: [RFC PATCH v2 05/32] x86: Use encrypted access of BOOT related data with SEV
On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > When Secure Encrypted Virtualization (SEV) is active, BOOT data (such as > EFI related data, setup data) is encrypted and needs to be accessed as > such when mapped. Update the architecture override in early_memremap to > keep the encryption attribute when mapping this data. This could also explain why persistent memory needs to be accessed decrypted with SEV. In general, what the difference in that aspect is in respect to SME. And I'd write that in the comment over the function. And not say "E820 areas are checked in making this determination." because that is visible but say *why* we need to check those ranges and determine access depending on their type. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 02/32] x86: Secure Encrypted Virtualization (SEV) support
On Thu, Mar 02, 2017 at 10:12:20AM -0500, Brijesh Singh wrote: > From: Tom Lendacky > > Provide support for Secure Encyrpted Virtualization (SEV). This initial > support defines a flag that is used by the kernel to determine if it is > running with SEV active. > > Signed-off-by: Tom Lendacky Btw, you need to add your Signed-off-by here after Tom's to denote that you're handing that patch forward. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V12 05/10] acpi: apei: handle SEA notification type for ARMv8
Hi Tyler,
On 06/03/17 20:44, Tyler Baicar wrote:
> ARM APEI extension proposal added SEA (Synchronous External Abort)
> notification type for ARMv8.
> Add a new GHES error source handling function for SEA. If an error
> source's notification type is SEA, then this function can be registered
> into the SEA exception handler. That way GHES will parse and report
> SEA exceptions when they occur.
> An SEA can interrupt code that had interrupts masked and is treated as
> an NMI. To aid this the page of address space for mapping APEI buffers
> while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
> changed to use the helper methods to find the prot_t to map with in
> the same way as ghes_ioremap_pfn_irq().
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index d178dc0..b2d57fc 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -41,6 +41,8 @@
> #include
> #include
>
> +#include
> +
> static const char *fault_name(unsigned int esr);
>
> #ifdef CONFIG_KPROBES
> @@ -498,6 +500,17 @@ static int do_sea(unsigned long addr, unsigned int esr,
> struct pt_regs *regs)
> pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
>fault_name(esr), esr, addr);
>
> + /*
> + * Synchronous aborts may interrupt code which had interrupts masked.
> + * Before calling out into the wider kernel tell the interested
> + * subsystems.
> + */
> + if (IS_ENABLED(ACPI_APEI_SEA)) {
IS_ENABLED() needs the CONFIG_ version of the symbols, otherwise this doesn't
get built.
(I guess the testing from the previous always-enabled version is still valid)
> + nmi_enter();
> + ghes_notify_sea();
> + nmi_exit();
> + }
> +
> info.si_signo = SIGBUS;
> info.si_errno = 0;
> info.si_code = 0;
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index b0140c8..c545dd1 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER
> PCIe AER errors may be reported via APEI firmware first mode.
> Turn on this option to enable the corresponding support.
>
> +config ACPI_APEI_SEA
> + bool "APEI Synchronous External Abort logging/recovering support"
> + depends on ARM64 && ACPI_APEI && ACPI_APEI_GHES
Nit: ACPI_APEI_GHES already depends on ACPI_APEI
> + default y
> + help
> + This option should be enabled if the system supports
> + firmware first handling of SEA (Synchronous External Abort).
> + SEA happens with certain faults of data abort or instruction
> + abort synchronous exceptions on ARMv8 systems. If a system
> + supports firmware first handling of SEA, the platform analyzes
> + and handles hardware error notifications from SEA, and it may then
> + form a HW error record for the OS to parse and handle. This
> + option allows the OS to look for such hardware error record, and
> + take appropriate action.
> +
> config ACPI_APEI_MEMORY_FAILURE
> bool "APEI memory error recovering support"
> depends on ACPI_APEI && MEMORY_FAILURE
Reviewed-by: James Morse
Thanks,
James
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V12 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
Hi Tyler, On 06/03/17 20:44, Tyler Baicar wrote: > When a memory error, CPU error, PCIe error, or other type of hardware error > that's covered by RAS occurs, firmware should populate the shared GHES memory > location with the proper GHES structures to notify the OS of the error. > For example, platforms that implement firmware first handling may implement > separate GHES sources for corrected errors and uncorrected errors. If the > error is an uncorrectable error, then the firmware will notify the OS > immediately since the error needs to be handled ASAP. The OS will then be able > to take the appropriate action needed such as offlining a page. If the error > is a corrected error, then the firmware will not interrupt the OS immediately. > Instead, the OS will see and report the error the next time it's GHES timer > expires. The kernel will first parse the GHES structures and report the errors > through the kernel logs and then notify the user space through RAS trace > events. This allows user space applications such as RAS Daemon to see the > errors and report them however the user desires. This patchset extends the > kernel functionality for RAS errors based on updates in the UEFI 2.6 and > ACPI 6.1 specifications. This series doesn't apply cleanly to v4.11-rc1, what did you base it on? Please base this on a v4.11 release candidate if you want it considered for v4.12. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support
Hi Tyler,
On 06/03/17 20:45, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..a1a3dff 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,16 @@
> #define FSC_FAULT(0x04)
> #define FSC_ACCESS (0x08)
> #define FSC_PERM (0x0c)
> +#define FSC_SEA (0x10)
> +#define FSC_SEA_TTW0 (0x14)
> +#define FSC_SEA_TTW1 (0x15)
> +#define FSC_SEA_TTW2 (0x16)
> +#define FSC_SEA_TTW3 (0x17)
> +#define FSC_SECC (0x18)
> +#define FSC_SECC_TTW0(0x1c)
aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)
> +#define FSC_SECC_TTW1(0x1d)
> +#define FSC_SECC_TTW2(0x1e)
> +#define FSC_SECC_TTW3(0x1f)
>
> /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> #define HPFAR_MASK (~0xf)
> #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..f3608c9 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
>
> /* Check the stage-2 fault is trans. fault or write fault */
> fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> + * is no need to pass the error into the guest.
> + */
> + if (is_abort_synchronous(fault_status)) {
> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {
... Looking further up in this function:
> is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
> if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
> kvm_inject_vabt(vcpu);
> return 1;
> }
... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).
We need to do the handle_guest_sea() before this code.
kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.
I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.
Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.
> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
> kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
> kvm_vcpu_trap_get_class(vcpu),
> (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..31c5171 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
> }
>
> /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + /*
> + * synchronize_rcu() will wait for nmi_exit(), so no need to
> + * rcu_read_lock().
> + */
This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!
> + if(IS_ENABLED(ACPI_APEI_SEA)) {
> + rcu_read_lock();
> + ghes_notify_sea();
> + rcu_read_unlock();
> + }
> +
> + return 0;
> +}
> +
> +/*
> * Dispatch a data abort to the relevant handler.
> */
> asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int
> esr,
>
Thanks,
James
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 07/32] x86/efi: Access EFI data as encrypted when SEV is active
On Thu, Mar 02, 2017 at 10:13:21AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky
>
> EFI data is encrypted when the kernel is run under SEV. Update the
> page table references to be sure the EFI memory areas are accessed
> encrypted.
>
> Signed-off-by: Tom Lendacky
> Signed-off-by: Brijesh Singh
This SOB chain looks good.
> ---
> arch/x86/platform/efi/efi_64.c | 15 ++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 2d8674d..9a76ed8 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -45,6 +45,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * We allocate runtime services regions bottom-up, starting from -4G, i.e.
> @@ -286,7 +287,10 @@ int __init efi_setup_page_tables(unsigned long
> pa_memmap, unsigned num_pages)
>* as trim_bios_range() will reserve the first page and isolate it away
>* from memory allocators anyway.
>*/
> - if (kernel_map_pages_in_pgd(pgd, 0x0, 0x0, 1, _PAGE_RW)) {
> + pf = _PAGE_RW;
> + if (sev_active())
> + pf |= _PAGE_ENC;
> + if (kernel_map_pages_in_pgd(pgd, 0x0, 0x0, 1, pf)) {
> pr_err("Failed to create 1:1 mapping for the first page!\n");
> return 1;
> }
> @@ -329,6 +333,9 @@ static void __init __map_region(efi_memory_desc_t *md,
> u64 va)
> if (!(md->attribute & EFI_MEMORY_WB))
> flags |= _PAGE_PCD;
>
> + if (sev_active())
> + flags |= _PAGE_ENC;
> +
So I'm wondering if we could avoid this sprinkling of _PAGE_ENC in the
EFI code by defining something like __supported_pte_mask but called
__efi_base_page_flags or so which has _PAGE_ENC cleared in the SME case,
i.e., when baremetal and has it set in the SEV case.
Then we could simply OR in __efi_base_page_flags which the SME/SEV code
will set appropriately early enough.
Hmm.
Matt, what do you think?
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 08/32] x86: Use PAGE_KERNEL protection for ioremap of memory page
On Thu, Mar 02, 2017 at 10:13:32AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky
>
> In order for memory pages to be properly mapped when SEV is active, we
> need to use the PAGE_KERNEL protection attribute as the base protection.
> This will insure that memory mapping of, e.g. ACPI tables, receives the
> proper mapping attributes.
>
> Signed-off-by: Tom Lendacky
> ---
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index c400ab5..481c999 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -151,7 +151,15 @@ static void __iomem *__ioremap_caller(resource_size_t
> phys_addr,
> pcm = new_pcm;
> }
>
> + /*
> +* If the page being mapped is in memory and SEV is active then
> +* make sure the memory encryption attribute is enabled in the
> +* resulting mapping.
> +*/
> prot = PAGE_KERNEL_IO;
> + if (sev_active() && page_is_mem(pfn))
Hmm, a resource tree walk per ioremap call. This could get expensive for
ioremap-heavy workloads.
__ioremap_caller() gets called here during boot 55 times so not a whole
lot but I wouldn't be surprised if there were some nasty use cases which
ioremap a lot.
...
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 9b5f044..db56ba3 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -518,6 +518,46 @@ int __weak page_is_ram(unsigned long pfn)
> }
> EXPORT_SYMBOL_GPL(page_is_ram);
>
> +/*
> + * This function returns true if the target memory is marked as
> + * IORESOURCE_MEM and IORESOUCE_BUSY and described as other than
> + * IORES_DESC_NONE (e.g. IORES_DESC_ACPI_TABLES).
> + */
> +static int walk_mem_range(unsigned long start_pfn, unsigned long nr_pages)
> +{
> + struct resource res;
> + unsigned long pfn, end_pfn;
> + u64 orig_end;
> + int ret = -1;
> +
> + res.start = (u64) start_pfn << PAGE_SHIFT;
> + res.end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1;
> + res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> + orig_end = res.end;
> + while ((res.start < res.end) &&
> + (find_next_iomem_res(&res, IORES_DESC_NONE, true) >= 0)) {
> + pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + end_pfn = (res.end + 1) >> PAGE_SHIFT;
> + if (end_pfn > pfn)
> + ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
> + if (ret)
> + break;
> + res.start = res.end + 1;
> + res.end = orig_end;
> + }
> + return ret;
> +}
So the relevant difference between this one and walk_system_ram_range()
is this:
- ret = (*func)(pfn, end_pfn - pfn, arg);
+ ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
so it seems to me you can have your own *func() pointer which does that
IORES_DESC_NONE comparison. And then you can define your own workhorse
__walk_memory_range() which gets called by both walk_mem_range() and
walk_system_ram_range() instead of almost duplicating them.
And looking at walk_system_ram_res(), that one looks similar too except
the pfn computation. But AFAICT the pfn/end_pfn things are computed from
res.start and res.end so it looks to me like all those three functions
are crying for unification...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/18] pstore: Extract common arguments into structure
On Tue, Mar 7, 2017 at 6:55 AM, Kees Cook wrote:
> The read/mkfile pair pass the same arguments and should be cleared
> between calls. Move to a structure and wipe it after every loop.
>
> Signed-off-by: Kees Cook
> ---
> fs/pstore/platform.c | 55
> +++---
> include/linux/pstore.h | 28 -
> 2 files changed, 57 insertions(+), 26 deletions(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 320a673ecb5b..3fa1575a6e36 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -766,16 +766,9 @@ EXPORT_SYMBOL_GPL(pstore_unregister);
> void pstore_get_records(int quiet)
> {
> struct pstore_info *psi = psinfo;
> - char*buf = NULL;
> - ssize_t size;
> - u64 id;
> - int count;
> - enum pstore_type_id type;
> - struct timespec time;
> + struct pstore_recordrecord = { .psi = psi, };
> int failed = 0, rc;
> - boolcompressed;
> int unzipped_len = -1;
> - ssize_t ecc_notice_size = 0;
>
> if (!psi)
> return;
> @@ -784,39 +777,51 @@ void pstore_get_records(int quiet)
> if (psi->open && psi->open(psi))
> goto out;
>
> - while ((size = psi->read(&id, &type, &count, &time, &buf, &compressed,
> -&ecc_notice_size, psi)) > 0) {
> - if (compressed && (type == PSTORE_TYPE_DMESG)) {
> + while ((record.size = psi->read(&record.id, &record.type,
> +&record.count, &record.time,
> +&record.buf, &record.compressed,
> +&record.ecc_notice_size,
> +record.psi)) > 0) {
> + if (record.compressed &&
> + record.type == PSTORE_TYPE_DMESG) {
> if (big_oops_buf)
> - unzipped_len = pstore_decompress(buf,
> - big_oops_buf, size,
> + unzipped_len = pstore_decompress(
> + record.buf,
> + big_oops_buf,
> + record.size,
> big_oops_buf_sz);
>
> if (unzipped_len > 0) {
> - if (ecc_notice_size)
> + if (record.ecc_notice_size)
> memcpy(big_oops_buf + unzipped_len,
> - buf + size, ecc_notice_size);
> - kfree(buf);
> - buf = big_oops_buf;
> - size = unzipped_len;
> - compressed = false;
> + record.buf + recorrecord.size,
A typo on record.size.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/18] pstore: Avoid race in module unloading
Hi Kees,
On Tue, Mar 7, 2017 at 6:55 AM, Kees Cook wrote:
> Technically, it might be possible for struct pstore_info to go out of
> scope after the module_put(), so report the backend name first.
But in that case, using pstore will crash the kernel anyway, right?
If so, why pstore doesn't keep a reference until unregister?
Do I miss something?
Thanks,
Namhyung
>
> Signed-off-by: Kees Cook
> ---
> fs/pstore/platform.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 074fe85a2078..d69ef8a840b9 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -722,10 +722,10 @@ int pstore_register(struct pstore_info *psi)
> */
> backend = psi->name;
>
> - module_put(owner);
> -
> pr_info("Registered %s as persistent store backend\n", psi->name);
>
> + module_put(owner);
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(pstore_register);
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/18] pstore: Avoid race in module unloading
On Tue, Mar 7, 2017 at 8:16 AM, Namhyung Kim wrote:
> Hi Kees,
>
> On Tue, Mar 7, 2017 at 6:55 AM, Kees Cook wrote:
>> Technically, it might be possible for struct pstore_info to go out of
>> scope after the module_put(), so report the backend name first.
>
> But in that case, using pstore will crash the kernel anyway, right?
> If so, why pstore doesn't keep a reference until unregister?
> Do I miss something?
I could be wrong with this, since the backend can't call unregister
until register has finished... I'll drop this patch.
-Kees
>
> Thanks,
> Namhyung
>
>
>>
>> Signed-off-by: Kees Cook
>> ---
>> fs/pstore/platform.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> index 074fe85a2078..d69ef8a840b9 100644
>> --- a/fs/pstore/platform.c
>> +++ b/fs/pstore/platform.c
>> @@ -722,10 +722,10 @@ int pstore_register(struct pstore_info *psi)
>> */
>> backend = psi->name;
>>
>> - module_put(owner);
>> -
>> pr_info("Registered %s as persistent store backend\n", psi->name);
>>
>> + module_put(owner);
>> +
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(pstore_register);
>> --
>> 2.7.4
>>
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V12 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
On 3/7/2017 4:37 AM, James Morse wrote: Hi Tyler, On 06/03/17 20:44, Tyler Baicar wrote: When a memory error, CPU error, PCIe error, or other type of hardware error that's covered by RAS occurs, firmware should populate the shared GHES memory location with the proper GHES structures to notify the OS of the error. For example, platforms that implement firmware first handling may implement separate GHES sources for corrected errors and uncorrected errors. If the error is an uncorrectable error, then the firmware will notify the OS immediately since the error needs to be handled ASAP. The OS will then be able to take the appropriate action needed such as offlining a page. If the error is a corrected error, then the firmware will not interrupt the OS immediately. Instead, the OS will see and report the error the next time it's GHES timer expires. The kernel will first parse the GHES structures and report the errors through the kernel logs and then notify the user space through RAS trace events. This allows user space applications such as RAS Daemon to see the errors and report them however the user desires. This patchset extends the kernel functionality for RAS errors based on updates in the UEFI 2.6 and ACPI 6.1 specifications. This series doesn't apply cleanly to v4.11-rc1, what did you base it on? Please base this on a v4.11 release candidate if you want it considered for v4.12. This was based on 4.10, I will base it on 4.11-rc1 for the next patch set. Thanks, Tyler -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 28/28] x86: Add support to make use of Secure Memory Encryption
On 3/1/2017 12:40 PM, Borislav Petkov wrote:
On Thu, Feb 16, 2017 at 09:48:25AM -0600, Tom Lendacky wrote:
This patch adds the support to check if SME has been enabled and if
memory encryption should be activated (checking of command line option
based on the configuration of the default state). If memory encryption
is to be activated, then the encryption mask is set and the kernel is
encrypted "in place."
Signed-off-by: Tom Lendacky
---
arch/x86/kernel/head_64.S |1 +
arch/x86/kernel/mem_encrypt_init.c | 71 +++-
arch/x86/mm/mem_encrypt.c |2 +
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index edd2f14..e6820e7 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -97,6 +97,7 @@ startup_64:
* Save the returned mask in %r12 for later use.
*/
push%rsi
+ movq%rsi, %rdi
callsme_enable
pop %rsi
movq%rax, %r12
diff --git a/arch/x86/kernel/mem_encrypt_init.c
b/arch/x86/kernel/mem_encrypt_init.c
index 07cbb90..35c5e3d 100644
--- a/arch/x86/kernel/mem_encrypt_init.c
+++ b/arch/x86/kernel/mem_encrypt_init.c
@@ -19,6 +19,12 @@
#include
#include
+#include
+#include
+#include
+
+static char sme_cmdline_arg_on[] __initdata = "mem_encrypt=on";
+static char sme_cmdline_arg_off[] __initdata = "mem_encrypt=off";
extern void sme_encrypt_execute(unsigned long, unsigned long, unsigned long,
void *, pgd_t *);
@@ -217,8 +223,71 @@ unsigned long __init sme_get_me_mask(void)
return sme_me_mask;
}
-unsigned long __init sme_enable(void)
+unsigned long __init sme_enable(void *boot_data)
unsigned long __init sme_enable(struct boot_params *bp)
works too.
Ok, will do.
And then you need to correct the function signature in the
!CONFIG_AMD_MEM_ENCRYPT case, at the end of this file, too:
unsigned long __init sme_enable(struct boot_params *bp) { return 0; }
Yup, missed that. I'll make it match.
{
+ struct boot_params *bp = boot_data;
+ unsigned int eax, ebx, ecx, edx;
+ unsigned long cmdline_ptr;
+ bool enable_if_found;
+ void *cmdline_arg;
+ u64 msr;
+
+ /* Check for an AMD processor */
+ eax = 0;
+ ecx = 0;
+ native_cpuid(&eax, &ebx, &ecx, &edx);
+ if ((ebx != 0x68747541) || (edx != 0x69746e65) || (ecx != 0x444d4163))
+ goto out;
+
+ /* Check for the SME support leaf */
+ eax = 0x8000;
+ ecx = 0;
+ native_cpuid(&eax, &ebx, &ecx, &edx);
+ if (eax < 0x801f)
+ goto out;
+
+ /*
+* Check for the SME feature:
+* CPUID Fn8000_001F[EAX] - Bit 0
+* Secure Memory Encryption support
+* CPUID Fn8000_001F[EBX] - Bits 5:0
+* Pagetable bit position used to indicate encryption
+*/
+ eax = 0x801f;
+ ecx = 0;
+ native_cpuid(&eax, &ebx, &ecx, &edx);
+ if (!(eax & 1))
+ goto out;
+
+ /* Check if SME is enabled */
+ msr = native_read_msr(MSR_K8_SYSCFG);
This native_read_msr() wankery is adding this check:
if (msr_tracepoint_active(__tracepoint_read_msr))
and here it is clearly too early for tracepoints. Please use __rdmsr()
which is purely doing the MSR operation. (... and exception handling for
Ah, good catch. I'll switch to __rdmsr().
when the RDMSR itself raises an exception but we're very early here too
so the MSR better be there, otherwise we'll blow up).
Yes, it will be there if SME support is indicated in the CPUID result.
+ if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
+ goto out;
+
+ /*
+* Fixups have not been to applied phys_base yet, so we must obtain
...not been applied to phys_base yet ...
Yup.
+* the address to the SME command line option in the following way.
+*/
+ if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT)) {
+ asm ("lea sme_cmdline_arg_off(%%rip), %0"
+: "=r" (cmdline_arg)
+: "p" (sme_cmdline_arg_off));
+ enable_if_found = false;
+ } else {
+ asm ("lea sme_cmdline_arg_on(%%rip), %0"
+: "=r" (cmdline_arg)
+: "p" (sme_cmdline_arg_on));
+ enable_if_found = true;
+ }
+
+ cmdline_ptr = bp->hdr.cmd_line_ptr | ((u64)bp->ext_cmd_line_ptr << 32);
+
+ if (cmdline_find_option_bool((char *)cmdline_ptr, cmdline_arg))
+ sme_me_mask = enable_if_found ? 1UL << (ebx & 0x3f) : 0;
+ else
+ sme_me_mask = enable_if_found ? 0 : 1UL << (ebx & 0x3f);
I have a better idea: you can copy __cmdline_find_option() +
cmdline_find_option() to arch/x86/lib/cmdline.c in a pre-patch. Then,
pass in a buffer and check f
Re: [PATCH V12 05/10] acpi: apei: handle SEA notification type for ARMv8
Hello James,
On 3/7/2017 4:37 AM, James Morse wrote:
On 06/03/17 20:44, Tyler Baicar wrote:
ARM APEI extension proposal added SEA (Synchronous External Abort)
notification type for ARMv8.
Add a new GHES error source handling function for SEA. If an error
source's notification type is SEA, then this function can be registered
into the SEA exception handler. That way GHES will parse and report
SEA exceptions when they occur.
An SEA can interrupt code that had interrupts masked and is treated as
an NMI. To aid this the page of address space for mapping APEI buffers
while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
changed to use the helper methods to find the prot_t to map with in
the same way as ghes_ioremap_pfn_irq().
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index d178dc0..b2d57fc 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -41,6 +41,8 @@
#include
#include
+#include
+
static const char *fault_name(unsigned int esr);
#ifdef CONFIG_KPROBES
@@ -498,6 +500,17 @@ static int do_sea(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
fault_name(esr), esr, addr);
+ /*
+* Synchronous aborts may interrupt code which had interrupts masked.
+* Before calling out into the wider kernel tell the interested
+* subsystems.
+*/
+ if (IS_ENABLED(ACPI_APEI_SEA)) {
IS_ENABLED() needs the CONFIG_ version of the symbols, otherwise this doesn't
get built.
(I guess the testing from the previous always-enabled version is still valid)
Okay, I will use CONFIG_ACPI_APEI_SEA in the next patch set.
+ nmi_enter();
+ ghes_notify_sea();
+ nmi_exit();
+ }
+
info.si_signo = SIGBUS;
info.si_errno = 0;
info.si_code = 0;
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index b0140c8..c545dd1 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER
PCIe AER errors may be reported via APEI firmware first mode.
Turn on this option to enable the corresponding support.
+config ACPI_APEI_SEA
+ bool "APEI Synchronous External Abort logging/recovering support"
+ depends on ARM64 && ACPI_APEI && ACPI_APEI_GHES
Nit: ACPI_APEI_GHES already depends on ACPI_APEI
I can remove ACPI_APEI here then.
+ default y
+ help
+ This option should be enabled if the system supports
+ firmware first handling of SEA (Synchronous External Abort).
+ SEA happens with certain faults of data abort or instruction
+ abort synchronous exceptions on ARMv8 systems. If a system
+ supports firmware first handling of SEA, the platform analyzes
+ and handles hardware error notifications from SEA, and it may then
+ form a HW error record for the OS to parse and handle. This
+ option allows the OS to look for such hardware error record, and
+ take appropriate action.
+
config ACPI_APEI_MEMORY_FAILURE
bool "APEI memory error recovering support"
depends on ACPI_APEI && MEMORY_FAILURE
Reviewed-by: James Morse
Thanks!
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 28/28] x86: Add support to make use of Secure Memory Encryption
On Tue, Mar 07, 2017 at 10:05:00AM -0600, Tom Lendacky wrote: > I can do that. Because phys_base hasn't been updated yet, I'll have to > create "on" and "off" constants and get their address in a similar way > to the command line option so that I can do the strncmp properly. Actually, wouldn't it be simpler to inspect the passed in buffer for containing the chars 'o', 'n' - in that order, or 'o', 'f', 'f' - in that order too? Because __cmdline_find_option() does copy the option characters into the buffer. Then you wouldn't need those "on" and "off" constants... -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support
Hello James,
On 3/7/2017 4:48 AM, James Morse wrote:
On 06/03/17 20:45, Tyler Baicar wrote:
Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.
diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..a1a3dff 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,16 @@
#define FSC_FAULT (0x04)
#define FSC_ACCESS(0x08)
#define FSC_PERM (0x0c)
+#define FSC_SEA(0x10)
+#define FSC_SEA_TTW0 (0x14)
+#define FSC_SEA_TTW1 (0x15)
+#define FSC_SEA_TTW2 (0x16)
+#define FSC_SEA_TTW3 (0x17)
+#define FSC_SECC (0x18)
+#define FSC_SECC_TTW0 (0x1c)
aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)
+#define FSC_SECC_TTW1 (0x1d)
+#define FSC_SECC_TTW2 (0x1e)
+#define FSC_SECC_TTW3 (0x1f)
/* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
#define HPFAR_MASK(~0xf)
#endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct
kvm_run *run)
/* Check the stage-2 fault is trans. fault or write fault */
fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
- if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
- fault_status != FSC_ACCESS) {
+
+ /* The host kernel will handle the synchronous external abort. There
+* is no need to pass the error into the guest.
+*/
+ if (is_abort_synchronous(fault_status)) {
+ if(handle_guest_sea((unsigned long)fault_ipa,
+ kvm_vcpu_get_hsr(vcpu))) {
... Looking further up in this function:
is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
kvm_inject_vabt(vcpu);
return 1;
}
... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).
We need to do the handle_guest_sea() before this code.
kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.
I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.
Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.
Yes, I'll move the handle_guest_sea() call above this. My testing didn't
call into that if statement for some reason...it made it to the
handle_guest_sea() call successfully.
If there is no work for the GHES code to do it will return and could
still make the kvm_inject_vabt() call. It will also return and do that
same thing if the error was not fatal in GHES...would that be an issue?
+ kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx
ESR_EL2=%#lx\n",
+ kvm_vcpu_trap_get_class(vcpu),
+ (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+ (unsigned long)kvm_vcpu_get_hsr(vcpu));
+ return -EFAULT;
+ }
+ } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+ fault_status != FSC_ACCESS) {
kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
kvm_vcpu_trap_get_class(vcpu),
(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b2d57fc..31c5171 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
}
/*
+ * Handle Synchronous External Aborts that occur in a guest kernel.
+ */
+int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+ /*
+* synchronize_rcu() will wait for nmi_exit(), so no need to
+* rcu_read_lock().
+*/
This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!
:) Okay I will move the rcu calls into ghes_notify_sea() and remove the
comments.
Thanks,
Tyler
+ if(IS_ENABLED(ACPI_APEI_SEA)) {
+ rcu_read_lock();
+ ghes_notify_sea();
+
Re: [PATCH V12 07/10] efi: print unrecognized CPER section
On 3/6/2017 2:05 PM, Joe Perches wrote:
On Mon, 2017-03-06 at 13:45 -0700, Tyler Baicar wrote:
UEFI spec allows for non-standard section in Common Platform Error
Record. This is defined in section N.2.3 of UEFI version 2.5.
Currently if the CPER section's type (UUID) does not match with
one of the section types that the kernel knows how to parse, the
section is skipped. Therefore, user is not able to see
such CPER data, for instance, error record of non-standard section.
For above mentioned case, this change prints out the raw data in
hex in dmesg buffer. Data length is taken from Error Data length
field of Generic Error Data Entry.
Hi Tyler.
Trivia: (probably not worth resubmitting for this)
There's a slight mismatch between logging output and commit
message. Now there's an ASCII block after the output.
Another suggestion below.
True, I can update this commit for the next patch set.
Following is a sample output from dmesg:
[ 115.771702] {1}[Hardware Error]: Hardware error from APEI Generic Hardware
Error Source: 2
[ 115.779042] {1}[Hardware Error]: It has been corrected by h/w and requires
no further action
[ 115.787456] {1}[Hardware Error]: event severity: corrected
[ 115.792927] {1}[Hardware Error]: Error 0, type: corrected
[ 115.798415] {1}[Hardware Error]: fru_id:
----
[ 115.805596] {1}[Hardware Error]: fru_text:
[ 115.816105] {1}[Hardware Error]: section type:
d2e2621c-f936-468d-0d84-15a4ed015c8b
[ 115.823880] {1}[Hardware Error]: section length: 88
[ 115.828779] {1}[Hardware Error]: : 0101 0002 5f434345
525f4543
[ 115.836153] {1}[Hardware Error]: 0010: 574d
[ 115.843531] {1}[Hardware Error]: 0020:
[ 115.850908] {1}[Hardware Error]: 0030:
[ 115.858288] {1}[Hardware Error]: 0040: fe80 0004
5f434345
[ 115.865665] {1}[Hardware Error]: 0050: 525f4543 574d
[]
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
[]
@@ -591,8 +591,16 @@ static void cper_estatus_print_section(
cper_print_proc_arm(newpfx, arm_err);
else
goto err_section_too_small;
- } else
- printk("%s""section type: unknown, %pUl\n", newpfx, sec_type);
+ } else {
+ const void *unknown_err;
+
+ unknown_err = acpi_hest_generic_data_payload(gdata);
+ printk("%ssection type: unknown, %pUl\n", newpfx, sec_type);
+ printk("%ssection length: %d\n", newpfx,
+ gdata->error_data_length);
It might be nice to output this as
printk("%ssection length: %d (%#x)\n",
newpfx, gdata->error_data_length,
gdata->error_data_length);
so it's easy to know the appropriate hex buffer length too.
I will make this change in the next patch set.
Thanks,
Tyler
+ print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4,
+ unknown_err, gdata->error_data_length, true);
+ }
return;
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] EFI fixes
Linus, Please pull the latest efi-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git efi-urgent-for-linus # HEAD: d1eb98143c56f24fef125f5bbed49ae0b52fb7d6 efi/arm: Fix boot crash with CONFIG_CPUMASK_OFFSTACK=y A boot crash fix, and a secure boot related boot messages fix. Thanks, Ingo --> Ard Biesheuvel (2): efi/libstub: Treat missing SecureBoot variable as Secure Boot disabled efi/arm: Fix boot crash with CONFIG_CPUMASK_OFFSTACK=y drivers/firmware/efi/arm-runtime.c| 1 + drivers/firmware/efi/libstub/secureboot.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 349dc3e1e52e..974c5a31a005 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -65,6 +65,7 @@ static bool __init efi_virtmap_init(void) bool systab_found; efi_mm.pgd = pgd_alloc(&efi_mm); + mm_init_cpumask(&efi_mm); init_new_context(NULL, &efi_mm); systab_found = false; diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c index 6def402bf569..5da36e56b36a 100644 --- a/drivers/firmware/efi/libstub/secureboot.c +++ b/drivers/firmware/efi/libstub/secureboot.c @@ -45,6 +45,8 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) size = sizeof(secboot); status = get_efi_var(efi_SecureBoot_name, &efi_variable_guid, NULL, &size, &secboot); + if (status == EFI_NOT_FOUND) + return efi_secureboot_mode_disabled; if (status != EFI_SUCCESS) goto out_efi_err; @@ -78,7 +80,5 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg) out_efi_err: pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot status.\n"); - if (status == EFI_NOT_FOUND) - return efi_secureboot_mode_disabled; return efi_secureboot_mode_unknown; } -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/18] pstore: Extract common arguments into structure
On Tue, Mar 7, 2017 at 8:22 AM, Namhyung Kim wrote:
> On Tue, Mar 7, 2017 at 6:55 AM, Kees Cook wrote:
>> The read/mkfile pair pass the same arguments and should be cleared
>> between calls. Move to a structure and wipe it after every loop.
>>
>> Signed-off-by: Kees Cook
>> ---
>> fs/pstore/platform.c | 55
>> +++---
>> include/linux/pstore.h | 28 -
>> 2 files changed, 57 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> index 320a673ecb5b..3fa1575a6e36 100644
>> --- a/fs/pstore/platform.c
>> +++ b/fs/pstore/platform.c
>> @@ -766,16 +766,9 @@ EXPORT_SYMBOL_GPL(pstore_unregister);
>> void pstore_get_records(int quiet)
>> {
>> struct pstore_info *psi = psinfo;
>> - char*buf = NULL;
>> - ssize_t size;
>> - u64 id;
>> - int count;
>> - enum pstore_type_id type;
>> - struct timespec time;
>> + struct pstore_recordrecord = { .psi = psi, };
>> int failed = 0, rc;
>> - boolcompressed;
>> int unzipped_len = -1;
>> - ssize_t ecc_notice_size = 0;
>>
>> if (!psi)
>> return;
>> @@ -784,39 +777,51 @@ void pstore_get_records(int quiet)
>> if (psi->open && psi->open(psi))
>> goto out;
>>
>> - while ((size = psi->read(&id, &type, &count, &time, &buf,
>> &compressed,
>> -&ecc_notice_size, psi)) > 0) {
>> - if (compressed && (type == PSTORE_TYPE_DMESG)) {
>> + while ((record.size = psi->read(&record.id, &record.type,
>> +&record.count, &record.time,
>> +&record.buf, &record.compressed,
>> +&record.ecc_notice_size,
>> +record.psi)) > 0) {
>> + if (record.compressed &&
>> + record.type == PSTORE_TYPE_DMESG) {
>> if (big_oops_buf)
>> - unzipped_len = pstore_decompress(buf,
>> - big_oops_buf, size,
>> + unzipped_len = pstore_decompress(
>> + record.buf,
>> + big_oops_buf,
>> + record.size,
>> big_oops_buf_sz);
>>
>> if (unzipped_len > 0) {
>> - if (ecc_notice_size)
>> + if (record.ecc_notice_size)
>> memcpy(big_oops_buf + unzipped_len,
>> - buf + size, ecc_notice_size);
>> - kfree(buf);
>> - buf = big_oops_buf;
>> - size = unzipped_len;
>> - compressed = false;
>> + record.buf + recorrecord.size,
>
> A typo on record.size.
Thanks! Yeah, 0-day noticed this too. I've refreshed the patches in my
tree with the correction now.
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 14/28] Add support to access boot related data in the clear
On 02/16/17 at 09:45am, Tom Lendacky wrote:
[snip]
> + * This function determines if an address should be mapped encrypted.
> + * Boot setup data, EFI data and E820 areas are checked in making this
> + * determination.
> + */
> +static bool memremap_should_map_encrypted(resource_size_t phys_addr,
> + unsigned long size)
> +{
> + /*
> + * SME is not active, return true:
> + * - For early_memremap_pgprot_adjust(), returning true or false
> + * results in the same protection value
> + * - For arch_memremap_do_ram_remap(), returning true will allow
> + * the RAM remap to occur instead of falling back to ioremap()
> + */
> + if (!sme_active())
> + return true;
>From the function name shouldn't above be return false?
> +
> + /* Check if the address is part of the setup data */
> + if (memremap_is_setup_data(phys_addr, size))
> + return false;
> +
> + /* Check if the address is part of EFI boot/runtime data */
> + switch (efi_mem_type(phys_addr)) {
> + case EFI_BOOT_SERVICES_DATA:
> + case EFI_RUNTIME_SERVICES_DATA:
Only these two types needed? I'm not sure about this, just bring up the
question.
> + return false;
> + default:
> + break;
> + }
> +
> + /* Check if the address is outside kernel usable area */
> + switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) {
> + case E820_TYPE_RESERVED:
> + case E820_TYPE_ACPI:
> + case E820_TYPE_NVS:
> + case E820_TYPE_UNUSABLE:
> + return false;
> + default:
> + break;
> + }
> +
> + return true;
> +}
> +
Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 24/28] x86: Access the setup data through debugfs decrypted
On 02/16/17 at 09:47am, Tom Lendacky wrote:
> Use memremap() to map the setup data. This simplifies the code and will
> make the appropriate decision as to whether a RAM remapping can be done
> or if a fallback to ioremap_cache() is needed (which includes checking
> PageHighMem).
>
> Signed-off-by: Tom Lendacky
> ---
> arch/x86/kernel/kdebugfs.c | 30 +++---
> 1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kernel/kdebugfs.c b/arch/x86/kernel/kdebugfs.c
> index bdb83e4..c3d354d 100644
> --- a/arch/x86/kernel/kdebugfs.c
> +++ b/arch/x86/kernel/kdebugfs.c
> @@ -48,17 +48,13 @@ static ssize_t setup_data_read(struct file *file, char
> __user *user_buf,
>
> pa = node->paddr + sizeof(struct setup_data) + pos;
> pg = pfn_to_page((pa + count - 1) >> PAGE_SHIFT);
> - if (PageHighMem(pg)) {
> - p = ioremap_cache(pa, count);
> - if (!p)
> - return -ENXIO;
> - } else
> - p = __va(pa);
> + p = memremap(pa, count, MEMREMAP_WB);
> + if (!p)
> + return -ENXIO;
-ENOMEM looks better for memremap, ditto for other places..
>
> remain = copy_to_user(user_buf, p, count);
>
> - if (PageHighMem(pg))
> - iounmap(p);
> + memunmap(p);
>
> if (remain)
> return -EFAULT;
> @@ -127,15 +123,12 @@ static int __init create_setup_data_nodes(struct dentry
> *parent)
> }
>
> pg = pfn_to_page((pa_data+sizeof(*data)-1) >> PAGE_SHIFT);
> - if (PageHighMem(pg)) {
> - data = ioremap_cache(pa_data, sizeof(*data));
> - if (!data) {
> - kfree(node);
> - error = -ENXIO;
> - goto err_dir;
> - }
> - } else
> - data = __va(pa_data);
> + data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
> + if (!data) {
> + kfree(node);
> + error = -ENXIO;
> + goto err_dir;
> + }
>
> node->paddr = pa_data;
> node->type = data->type;
> @@ -143,8 +136,7 @@ static int __init create_setup_data_nodes(struct dentry
> *parent)
> error = create_setup_data_node(d, no, node);
> pa_data = data->next;
>
> - if (PageHighMem(pg))
> - iounmap(data);
> + memunmap(data);
> if (error)
> goto err_dir;
> no++;
>
Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 25/28] x86: Access the setup data through sysfs decrypted
On 02/16/17 at 09:47am, Tom Lendacky wrote:
> Use memremap() to map the setup data. This will make the appropriate
> decision as to whether a RAM remapping can be done or if a fallback to
> ioremap_cache() is needed (similar to the setup data debugfs support).
>
> Signed-off-by: Tom Lendacky
> ---
> arch/x86/kernel/ksysfs.c | 27 ++-
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/ksysfs.c b/arch/x86/kernel/ksysfs.c
> index 4afc67f..d653b3e 100644
> --- a/arch/x86/kernel/ksysfs.c
> +++ b/arch/x86/kernel/ksysfs.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -79,12 +80,12 @@ static int get_setup_data_paddr(int nr, u64 *paddr)
> *paddr = pa_data;
> return 0;
> }
> - data = ioremap_cache(pa_data, sizeof(*data));
> + data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
> if (!data)
> return -ENOMEM;
>
> pa_data = data->next;
> - iounmap(data);
> + memunmap(data);
> i++;
> }
> return -EINVAL;
> @@ -97,17 +98,17 @@ static int __init get_setup_data_size(int nr, size_t
> *size)
> u64 pa_data = boot_params.hdr.setup_data;
>
> while (pa_data) {
> - data = ioremap_cache(pa_data, sizeof(*data));
> + data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
> if (!data)
> return -ENOMEM;
> if (nr == i) {
> *size = data->len;
> - iounmap(data);
> + memunmap(data);
> return 0;
> }
>
> pa_data = data->next;
> - iounmap(data);
> + memunmap(data);
> i++;
> }
> return -EINVAL;
> @@ -127,12 +128,12 @@ static ssize_t type_show(struct kobject *kobj,
> ret = get_setup_data_paddr(nr, &paddr);
> if (ret)
> return ret;
> - data = ioremap_cache(paddr, sizeof(*data));
> + data = memremap(paddr, sizeof(*data), MEMREMAP_WB);
> if (!data)
> return -ENOMEM;
>
> ret = sprintf(buf, "0x%x\n", data->type);
> - iounmap(data);
> + memunmap(data);
> return ret;
> }
>
> @@ -154,7 +155,7 @@ static ssize_t setup_data_data_read(struct file *fp,
> ret = get_setup_data_paddr(nr, &paddr);
> if (ret)
> return ret;
> - data = ioremap_cache(paddr, sizeof(*data));
> + data = memremap(paddr, sizeof(*data), MEMREMAP_WB);
> if (!data)
> return -ENOMEM;
>
> @@ -170,15 +171,15 @@ static ssize_t setup_data_data_read(struct file *fp,
> goto out;
>
> ret = count;
> - p = ioremap_cache(paddr + sizeof(*data), data->len);
> + p = memremap(paddr + sizeof(*data), data->len, MEMREMAP_WB);
> if (!p) {
> ret = -ENOMEM;
> goto out;
> }
> memcpy(buf, p + off, count);
> - iounmap(p);
> + memunmap(p);
> out:
> - iounmap(data);
> + memunmap(data);
> return ret;
> }
>
> @@ -250,13 +251,13 @@ static int __init get_setup_data_total_num(u64 pa_data,
> int *nr)
> *nr = 0;
> while (pa_data) {
> *nr += 1;
> - data = ioremap_cache(pa_data, sizeof(*data));
> + data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
> if (!data) {
> ret = -ENOMEM;
> goto out;
> }
> pa_data = data->next;
> - iounmap(data);
> + memunmap(data);
> }
>
> out:
>
It would be better that these cleanup patches are sent separately.
Acked-by: Dave Young
Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
EFI allocates runtime services regions top-down, starting from EFI_VA_START to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of EFI region. The upper boundary of memory regions randomized by KASLR should be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. Correct it in this patch. Signed-off-by: Baoquan He --- arch/x86/mm/kaslr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c index 887e571..aed2064 100644 --- a/arch/x86/mm/kaslr.c +++ b/arch/x86/mm/kaslr.c @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE; #if defined(CONFIG_X86_ESPFIX64) static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; #elif defined(CONFIG_EFI) -static const unsigned long vaddr_end = EFI_VA_START; +static const unsigned long vaddr_end = EFI_VA_END; #else static const unsigned long vaddr_end = __START_KERNEL_map; #endif @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) */ BUILD_BUG_ON(vaddr_start >= vaddr_end); BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && -vaddr_end >= EFI_VA_START); +vaddr_end >= EFI_VA_END); BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || IS_ENABLED(CONFIG_EFI)) && vaddr_end >= __START_KERNEL_map); -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] x86/efi: Correct a tiny mistake in code comment
EFI allocate runtime services regions down from EFI_VA_START, -4G. It should be top-down handling. Signed-off-by: Baoquan He --- arch/x86/platform/efi/efi_64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c index a4695da..6cbf9e0 100644 --- a/arch/x86/platform/efi/efi_64.c +++ b/arch/x86/platform/efi/efi_64.c @@ -47,7 +47,7 @@ #include /* - * We allocate runtime services regions bottom-up, starting from -4G, i.e. + * We allocate runtime services regions top-down, starting from -4G, i.e. * 0x___ and limit EFI VA mapping space to 64G. */ static u64 efi_va = EFI_VA_START; -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
