Re: [PATCH] arm64: efi: ignore EFI_MEMORY_XP attribute if RP and/or WP are set

2017-09-15 Thread Ard Biesheuvel 
On 15 September 2017 at 11:53, Stephen Boyd  wrote:
> On 09/14, Ard Biesheuvel wrote:
>> The UEFI memory map is a bit vague about how to interpret the
>> EFI_MEMORY_XP attribute when it is combined with EFI_MEMORY_RP and/or
>> EFI_MEMORY_WP, which have retroactively been redefined as cacheability
>> attributes rather than permission attributes.
>>
>> So let's ignore EFI_MEMORY_XP if _RP and/or _WP are also set. In this
>> case, it is likely that they are being used to describe the capability
>> of the region (i.e., whether it has the controls to reconfigure it as
>> non-executable) rather than the nature of the contents of the region
>> (i.e., whether it contains data that we will never attempt to execute)
>>
>> Cc: Stephen Boyd 
>
> Reported-by: Stephen Boyd 
>
> I will test early next week and provide a tested-by. Thanks.
>

Great, thanks.
--
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] arm64: efi: ignore EFI_MEMORY_XP attribute if RP and/or WP are set

2017-09-15 Thread Stephen Boyd 
On 09/14, Ard Biesheuvel wrote:
> The UEFI memory map is a bit vague about how to interpret the
> EFI_MEMORY_XP attribute when it is combined with EFI_MEMORY_RP and/or
> EFI_MEMORY_WP, which have retroactively been redefined as cacheability
> attributes rather than permission attributes.
> 
> So let's ignore EFI_MEMORY_XP if _RP and/or _WP are also set. In this
> case, it is likely that they are being used to describe the capability
> of the region (i.e., whether it has the controls to reconfigure it as
> non-executable) rather than the nature of the contents of the region
> (i.e., whether it contains data that we will never attempt to execute)
> 
> Cc: Stephen Boyd 

Reported-by: Stephen Boyd 

I will test early next week and provide a tested-by. Thanks.

-- 
Qualcomm Innovation Center, Inc. is a member of 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 Part1 PATCH v3 13/17] x86/io: Unroll string I/O when SEV is active

2017-09-15 Thread Brijesh Singh 



On 09/15/2017 11:22 AM, Borislav Petkov wrote:

mem_encrypt_init() where everything should be set up already.



Yep, its safe to derefs the static key in mem_encrypt_init(). I've
tried the approach and it seems to be work fine. I will include the
required changes in next rev. thanks

--
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 Part1 PATCH v3 13/17] x86/io: Unroll string I/O when SEV is active

2017-09-15 Thread Borislav Petkov 
On Fri, Sep 15, 2017 at 09:48:53AM -0500, Brijesh Singh wrote:
> I see the similar issue with non SEV guest with my simple patch below.
> Guest will reboot as soon as it tries to enable the key.

Can't do it there as the pagetable is not setup yet and you're probably
getting a #PF on any of the derefs down the static_key_enable() path.

I guess one possible place to enable the static key would be in
mem_encrypt_init() where everything should be set up already.

-- 
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 Part1 PATCH v3 13/17] x86/io: Unroll string I/O when SEV is active

2017-09-15 Thread Brijesh Singh 



On 09/15/2017 09:40 AM, Borislav Petkov wrote:

I need to figure out the include hell first.


I am working with slightly newer patch sets -- in that patch Tom has
moved the sev_active() definition in arch/x86/mm/mem_encrypt.c and I
have no issue using your recommended (since I no longer need the include
path changes).

But in my quick run I did found a runtime issue, it seems enabling the static
key in sme_enable is too early. Guest reboots as soon as it tries to enable
the key.

I see the similar issue with non SEV guest with my simple patch below.
Guest will reboot as soon as it tries to enable the key.

--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -40,6 +40,8 @@ pmdval_t early_pmd_flags = __PAGE_KERNEL_LARGE & 
~(_PAGE_GLOBAL | _PAGE_NX);
 
 #define __head __section(.head.text)
 
+DEFINE_STATIC_KEY_FALSE(__testme);

+
 static void __head *fixup_pointer(void *ptr, unsigned long physaddr)
 {
return ptr - (void *)_text + (void *)physaddr;
@@ -71,6 +73,8 @@ unsigned long __head __startup_64(unsigned long physaddr,
if (load_delta & ~PMD_PAGE_MASK)
for (;;);
 
+   static_branch_enable(&__testme);

+
/* Activate Secure Memory Encryption (SME) if supported and enabled */
sme_enable(bp);

--
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 Part1 PATCH v3 13/17] x86/io: Unroll string I/O when SEV is active

2017-09-15 Thread Borislav Petkov 
On Fri, Sep 15, 2017 at 09:13:00AM -0500, Brijesh Singh wrote:
> thanks for the suggestion Boris, it will make patch much simpler.
> I will try this out.

It won't build - this was supposed to show the general idea.

I need to figure out the include hell first.

-- 
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 Part1 PATCH v3 13/17] x86/io: Unroll string I/O when SEV is active

2017-09-15 Thread Brijesh Singh 



On 09/15/2017 07:24 AM, Borislav Petkov wrote:

On Tue, Aug 22, 2017 at 06:52:48PM +0200, Borislav Petkov wrote:

As always, the devil is in the detail.


Ok, actually we can make this much simpler by using a static key. A
conceptual patch below - I only need to fix that crazy include hell I'm
stepping into with this.

In any case, we were talking about having a static branch already so
this fits the whole strategy.



thanks for the suggestion Boris, it will make patch much simpler.
I will try this out.

-Brijesh
--
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