Re: [PATCH] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
On 3 July 2018 at 23:46, Borislav Petkov wrote: > On Tue, Jul 03, 2018 at 04:16:57PM -0500, Brijesh Singh wrote: >> I agree with Ard, it may be good idea to extend the UEFI spec to >> include encryption information. Having this information may be helpful >> in some cases, e.g if we ever need to map a specific non IO memory as >> unencrypted. So far we have not seen the need for it. But I will ask AMD >> folks working closely with UEFI committee to float this and submit it as >> enhancement in Tianocore BZ. > > Except that if the IO memory handling unencrypted changes in future > incarnations, the changes to the spec become moot. I'm just saying... > Quite the opposite. If we allocate a EFI_MEMORY_xx bit to signify that a region should be mapped as encrypted, we no longer have to reason about these things in the kernel but we can simply apply the attributes that the UEFI memory map provides us. A platform could then describe both encrypted and unencrypted MMIO regions at will. -- 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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
On Tue, Jul 03, 2018 at 04:16:57PM -0500, Brijesh Singh wrote: > I agree with Ard, it may be good idea to extend the UEFI spec to > include encryption information. Having this information may be helpful > in some cases, e.g if we ever need to map a specific non IO memory as > unencrypted. So far we have not seen the need for it. But I will ask AMD > folks working closely with UEFI committee to float this and submit it as > enhancement in Tianocore BZ. Except that if the IO memory handling unencrypted changes in future incarnations, the changes to the spec become moot. I'm just saying... -- 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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
On 7/3/18 10:44 AM, Borislav Petkov wrote:
> (dropping stable@ as this is not how you send patches to stable).
>
> On Tue, Jul 03, 2018 at 05:37:18PM +0200, Ard Biesheuvel wrote:
>> On 3 July 2018 at 15:32, Brijesh Singh wrote:
>>> SEV guest fails to update the UEFI runtime variables stored in the
>>> flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
>>> when SEV is active") unconditionally maps all the UEFI runtime data
>>> as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
>>> as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
>>> guest and hypervisor can access the data.
>>>
>> I'm uncomfortable having to carry these heuristics in the kernel. The
>> UEFI memory map should be the definitive source of information
>> regarding how the OS should map the regions it describes, and if we
>> need to guess the encryption status, we are likely to get it wrong at
>> least some of the times.
I agree with Ard, it may be good idea to extend the UEFI spec to
include encryption information. Having this information may be helpful
in some cases, e.g if we ever need to map a specific non IO memory as
unencrypted. So far we have not seen the need for it. But I will ask AMD
folks working closely with UEFI committee to float this and submit it as
enhancement in Tianocore BZ.
> I think the problem here is that IO memory can't be encrypted, at least
> at the moment. Thus this patch. I believe future versions will be able
> to handle encrypted IO but that's something Brijesh can correct me on.
Yes you are right, IO memory can't be encrypted. We map all IO memory
ranges as unencrypted everywhere else in the kernel. The
EFI_MEMORY_MAPPED_IO type should also be mapped as unencrypted.
> So it is not really about the UEFI spec but about what the hardware
> does/supports currently.
>
> And I don't think that change matters on anything else besides AMD with
> SEV enabled...
>
> Thx.
>
--
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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
On 7/3/2018 8:32 AM, Brijesh Singh wrote:
> SEV guest fails to update the UEFI runtime variables stored in the
> flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
> when SEV is active") unconditionally maps all the UEFI runtime data
> as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
> as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
> guest and hypervisor can access the data.
>
> Fixes: 1379edd59673 (x86/efi: Access EFI data as encrypted ...)
> Cc: Tom Lendacky
> Cc: Thomas Gleixner
> Cc: Borislav Petkov
> Cc: linux-efi@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: Ard Biesheuvel
> Cc: Matt Fleming
> Cc: Andy Lutomirski
> Cc: # 4.15.x
> Signed-off-by: Brijesh Singh
Reviewed-by: Tom Lendacky
> ---
> 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 77873ce..5f2eb32 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -417,7 +417,7 @@ static void __init __map_region(efi_memory_desc_t *md,
> u64 va)
> if (!(md->attribute & EFI_MEMORY_WB))
> flags |= _PAGE_PCD;
>
> - if (sev_active())
> + if (sev_active() && md->type != EFI_MEMORY_MAPPED_IO)
> flags |= _PAGE_ENC;
>
> pfn = md->phys_addr >> PAGE_SHIFT;
>
--
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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
(dropping stable@ as this is not how you send patches to stable).
On Tue, Jul 03, 2018 at 05:37:18PM +0200, Ard Biesheuvel wrote:
> On 3 July 2018 at 15:32, Brijesh Singh wrote:
> > SEV guest fails to update the UEFI runtime variables stored in the
> > flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
> > when SEV is active") unconditionally maps all the UEFI runtime data
> > as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
> > as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
> > guest and hypervisor can access the data.
> >
>
> I'm uncomfortable having to carry these heuristics in the kernel. The
> UEFI memory map should be the definitive source of information
> regarding how the OS should map the regions it describes, and if we
> need to guess the encryption status, we are likely to get it wrong at
> least some of the times.
I think the problem here is that IO memory can't be encrypted, at least
at the moment. Thus this patch. I believe future versions will be able
to handle encrypted IO but that's something Brijesh can correct me on.
So it is not really about the UEFI spec but about what the hardware
does/supports currently.
And I don't think that change matters on anything else besides AMD with
SEV enabled...
Thx.
--
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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
On 3 July 2018 at 15:32, Brijesh Singh wrote:
> SEV guest fails to update the UEFI runtime variables stored in the
> flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
> when SEV is active") unconditionally maps all the UEFI runtime data
> as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
> as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
> guest and hypervisor can access the data.
>
I'm uncomfortable having to carry these heuristics in the kernel. The
UEFI memory map should be the definitive source of information
regarding how the OS should map the regions it describes, and if we
need to guess the encryption status, we are likely to get it wrong at
least some of the times.
Is any work underway to get the UEFI spec extended to take encrypted
memory into account? I am aware that we cannot disclose specifics, but
you should be able to disclose whether it is under discussion or not.
In the mean time, we will have to do something, I know that, but I
would like to discuss the proper solution before proceeding with the
stop gap one.
--
Ard.
> Fixes: 1379edd59673 (x86/efi: Access EFI data as encrypted ...)
> Cc: Tom Lendacky
> Cc: Thomas Gleixner
> Cc: Borislav Petkov
> Cc: linux-efi@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: Ard Biesheuvel
> Cc: Matt Fleming
> Cc: Andy Lutomirski
> Cc: # 4.15.x
> Signed-off-by: Brijesh Singh
> ---
> 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 77873ce..5f2eb32 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -417,7 +417,7 @@ static void __init __map_region(efi_memory_desc_t *md,
> u64 va)
> if (!(md->attribute & EFI_MEMORY_WB))
> flags |= _PAGE_PCD;
>
> - if (sev_active())
> + if (sev_active() && md->type != EFI_MEMORY_MAPPED_IO)
> flags |= _PAGE_ENC;
>
> pfn = md->phys_addr >> PAGE_SHIFT;
> --
> 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 v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size
On 3 July 2018 at 17:24, Bartlomiej Zolnierkiewicz wrote: > On Monday, July 02, 2018 02:02:47 PM Ard Biesheuvel wrote: >> On 2 July 2018 at 13:57, Bartlomiej Zolnierkiewicz >> wrote: >> > On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote: >> >> On 2 July 2018 at 13:26, Hans de Goede wrote: >> >> > Bartlomiej, >> >> > >> >> > Now that the fbcon deferred console takeover patches have been >> >> > merged I believe this series can be merged too ? >> >> > >> >> > Note the first patch has an ack from Ard for merging the >> >> > 1 line efi change through the fbdev tree. >> >> > >> >> >> >> ... or I could take everything through the efi tree instead, as >> >> already discussed between Bartlomiej and me in the context of another >> >> patch series that touches both the fbdev and efi trees. >> >> >> >> Bartlomiej, that would require your ack on patch >> >> >> >> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer >> >> >> >> https://marc.info/?l=linux-fbdev&m=152933484616993&w=2 >> >> >> >> so if you're ok with that, I will queue both of these for v4.19 >> > >> > I would really prefer to merge this patchset through fbdev tree >> > as efi tree doesn't have fbcon deferred console takeover patches >> > (which are required by efifb changes under discussion). >> > >> >> Ah ok, I didn't realise that. I don't think there will be any >> conflicts, since the efifb changes in the efi tree and these changes >> operate on different parts of the file. But let's double check before >> taking stuff into -next. >> (efi/next is not pulled into -next directly, but via the tip:efi tree, >> and I haven't sent a pull request yet for v4.19) > > I've verified this with -next from today and it auto-merged fine (for > testing purposes I've applied both patches to -next and then pulled in > efi/next from efi.git tree). > > I've applied both patches to fbdev-for-next (I hope it is fine with you). > Absolutely fine, and thanks for double checking. -- 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 v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size
On Monday, July 02, 2018 02:02:47 PM Ard Biesheuvel wrote: > On 2 July 2018 at 13:57, Bartlomiej Zolnierkiewicz > wrote: > > On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote: > >> On 2 July 2018 at 13:26, Hans de Goede wrote: > >> > Bartlomiej, > >> > > >> > Now that the fbcon deferred console takeover patches have been > >> > merged I believe this series can be merged too ? > >> > > >> > Note the first patch has an ack from Ard for merging the > >> > 1 line efi change through the fbdev tree. > >> > > >> > >> ... or I could take everything through the efi tree instead, as > >> already discussed between Bartlomiej and me in the context of another > >> patch series that touches both the fbdev and efi trees. > >> > >> Bartlomiej, that would require your ack on patch > >> > >> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer > >> > >> https://marc.info/?l=linux-fbdev&m=152933484616993&w=2 > >> > >> so if you're ok with that, I will queue both of these for v4.19 > > > > I would really prefer to merge this patchset through fbdev tree > > as efi tree doesn't have fbcon deferred console takeover patches > > (which are required by efifb changes under discussion). > > > > Ah ok, I didn't realise that. I don't think there will be any > conflicts, since the efifb changes in the efi tree and these changes > operate on different parts of the file. But let's double check before > taking stuff into -next. > (efi/next is not pulled into -next directly, but via the tip:efi tree, > and I haven't sent a pull request yet for v4.19) I've verified this with -next from today and it auto-merged fine (for testing purposes I've applied both patches to -next and then pulled in efi/next from efi.git tree). I've applied both patches to fbdev-for-next (I hope it is fine with you). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- 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] x86/efi: Access EFI MMIO data as unencrypted when SEV is active
SEV guest fails to update the UEFI runtime variables stored in the
flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
when SEV is active") unconditionally maps all the UEFI runtime data
as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
guest and hypervisor can access the data.
Fixes: 1379edd59673 (x86/efi: Access EFI data as encrypted ...)
Cc: Tom Lendacky
Cc: Thomas Gleixner
Cc: Borislav Petkov
Cc: linux-efi@vger.kernel.org
Cc: k...@vger.kernel.org
Cc: Ard Biesheuvel
Cc: Matt Fleming
Cc: Andy Lutomirski
Cc: # 4.15.x
Signed-off-by: Brijesh Singh
---
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 77873ce..5f2eb32 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -417,7 +417,7 @@ static void __init __map_region(efi_memory_desc_t *md, u64
va)
if (!(md->attribute & EFI_MEMORY_WB))
flags |= _PAGE_PCD;
- if (sev_active())
+ if (sev_active() && md->type != EFI_MEMORY_MAPPED_IO)
flags |= _PAGE_ENC;
pfn = md->phys_addr >> PAGE_SHIFT;
--
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 v6 3/3] x86: paravirt: make native_save_fl extern inline
On 26/06/18 18:22, Nick Desaulniers wrote: > On Tue, Jun 26, 2018 at 3:13 AM Ingo Molnar wrote: >> Ok! >> >> Acked-by: Ingo Molnar >> >> What's the planned upstreaming route for these patches/fixes? > > While the fix is mainly for paravirt, 2/3 of the patches exclusively > touch arch/x86, so I think they should go up in the x86 tree (as > opposed to paravirt's), if you would be so kind. hpa mentioned not > handling merges in https://lkml.org/lkml/2018/6/14/903, so I reached > out to you and tglx. As there is no paravirt tree the x86 one seems to be appropriate. :-) Juergen -- 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
