Date:   Fri, 24 Feb 2017 09:04:21 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Borislav Petkov <bp@...en8.de>
CC:     <linux-arch@...r.kernel.org>, <linux-efi@...r.kernel.org>,
        <kvm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        <kasan-dev@...glegroups.com>, <linux-mm@...ck.org>,
        <iommu@...ts.linux-foundation.org>, Rik van Riel <riel@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Toshimitsu Kani <toshi.kani@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Corbet <corbet@....net>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Larry Woodman <lwoodman@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [RFC PATCH v4 14/28] Add support to access boot related data in
 the clear

On 2/24/2017 4:21 AM, Borislav Petkov wrote:
> On Thu, Feb 23, 2017 at 03:34:30PM -0600, Tom Lendacky wrote:
>> Hmm... maybe I'm missing something here.  This doesn't have anything to
>> do with kexec or efi_reuse_config.  This has to do with the fact that
>
> I said kexec because kexec uses the setup_data mechanism to pass config
> tables to the second kernel, for example.
>
>> when a system boots the setup data and the EFI data are not encrypted.
>> Since it's not encrypted we need to be sure that any early_memremap()
>> and memremap() calls remove the encryption mask from the resulting
>> pagetable entry that is created so the data can be accessed properly.
>
> Anyway, I'd prefer not to do this ad-hoc caching if it can be
> helped. You're imposing an arbitrary limit of 32 there which the
> setup_data linked list doesn't have. So if you really want to go
> inspect those elements, you could iterate over them starting from
> boot_params.hdr.setup_data, just like parse_setup_data() does. Most of
> the time that list should be non-existent and if it is, it will be short
> anyway.
>

I looked at doing that but you get into this cyclical situation unless
you specifically map each setup data elemement as decrypted. This is ok
for early_memremap since we have early_memremap_decrypted() but a new
memremap_decrypted() would have to be added. But I was trying to avoid
having to do multiple mapping calls inside the current mapping call.

I can always look at converting the setup_data_list from an array
into a list to eliminate the 32 entry limit, too.

Let me look at adding the early_memremap_decrypted() type support to
memremap() and see how that looks.

> And if we really decide that we need to cache it for later inspection
> due to speed considerations, as you do in memremap_is_setup_data(), you
> could do that in the default: branch of parse_setup_data() and do it
> just once: I don't see why you need to do add_to_setup_data_list() *and*
> update_setup_data_list() when you could add both pointer and updated
> size once.

I do the add followed by the update because we can't determine the true
size of the setup data until it is first mapped so that the data->len
field can be accessed. In order to map it properly the physical
address range needs to be added to the list before it is mapped. After
it's mapped, the true physical address range can be calculated and
updated.

Thanks,
Tom

>

Powered by blists - more mailing lists