linux-kernel - Re: [PATCH Part2 RFC v4 05/40] x86/sev: Add RMP entry lookup helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <39be0f79-e8e4-fd4a-5c4a-47731c61740d@amd.com>
Date:   Fri, 16 Jul 2021 12:22:33 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        "H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Dov Murik <dovmurik@...ux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@....com>,
        Borislav Petkov <bp@...en8.de>,
        Michael Roth <michael.roth@....com>,
        Vlastimil Babka <vbabka@...e.cz>, tony.luck@...el.com,
        npmccallum@...hat.com, brijesh.ksingh@...il.com
Subject: Re: [PATCH Part2 RFC v4 05/40] x86/sev: Add RMP entry lookup helpers


On 7/15/21 2:28 PM, Brijesh Singh wrote:
>
>
> On 7/15/21 1:37 PM, Sean Christopherson wrote:
>> On Wed, Jul 07, 2021, Brijesh Singh wrote:
>>> The snp_lookup_page_in_rmptable() can be used by the host to read
>>> the RMP
>>> entry for a given page. The RMP entry format is documented in AMD
>>> PPR, see
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fattachment.cgi%3Fid%3D296015&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C2140214b3fbd4a71617008d947bf9ae7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637619710568694335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=AkCyolw0P%2BrRFF%2FAnRozld4GkegQ0hR%2F523DI48jB4g%3D&amp;reserved=0.
>>>
>>
>> Ewwwwww, the RMP format isn't architectural!?
>>
>>    Architecturally the format of RMP entries are not specified in
>> APM. In order
>>    to assist software, the following table specifies select portions
>> of the RMP
>>    entry format for this specific product.
>>
>
> Unfortunately yes.
>
> But the documented fields in the RMP entry is architectural. The entry
> fields are documented in the APM section 15.36. So, in future we are
> guaranteed to have those fields available. If we are reading the RMP
> table directly, then architecture should provide some other means to
> get to fields from the RMP entry.
>
>
>> I know we generally don't want to add infrastructure without good
>> reason, but on
>> the other hand exposing a microarchitectural data structure to the
>> kernel at large
>> is going to be a disaster if the format does change on a future
>> processor.
>>
>> Looking at the future patches, dump_rmpentry() is the only power
>> user, e.g.
>> everything else mostly looks at "assigned" and "level" (and one
>> ratelimited warn
>> on "validated" in snp_make_page_shared(), but I suspect that
>> particular check
>> can and should be dropped).
>>
>
> Yes, we need "assigned" and "level" and other entries are mainly for
> the debug purposes.
>
For the debug purposes, we would like to dump additional RMP entries. If
we go with your proposed function then how do we get those information
in the dump_rmpentry()? How about if we provide two functions; the first
function provides architectural format and second provides the raw
values which can be used by the dump_rmpentry() helper.

struct rmpentry *snp_lookup_rmpentry(unsigned long paddr, int *level);

The 'struct rmpentry' uses the format defined in APM Table 15-36.

struct _rmpentry *_snp_lookup_rmpentry(unsigned long paddr, int *level);

The 'struct _rmpentry' will use include the PPR definition (basically
what we have today in this patch).

Thoughts ?


>> So, what about hiding "struct rmpentry" and possibly renaming it to
>> something
>> scary/microarchitectural, e.g. something like
>>
>
> Yes, it will work fine.
>
>> /*
>>   * Returns 1 if the RMP entry is assigned, 0 if it exists but is not
>> assigned,
>>   * and -errno if there is no corresponding RMP entry.
>>   */
>> int snp_lookup_rmpentry(struct page *page, int *level)
>> {
>>     unsigned long phys = page_to_pfn(page) << PAGE_SHIFT;
>>     struct rmpentry *entry, *large_entry;
>>     unsigned long vaddr;
>>
>>     if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
>>         return -ENXIO;
>>
>>     vaddr = rmptable_start + rmptable_page_offset(phys);
>>     if (unlikely(vaddr > rmptable_end))
>>         return -EXNIO;
>>
>>     entry = (struct rmpentry *)vaddr;
>>
>>     /* Read a large RMP entry to get the correct page level used in
>> RMP entry. */
>>     vaddr = rmptable_start + rmptable_page_offset(phys & PMD_MASK);
>>     large_entry = (struct rmpentry *)vaddr;
>>     *level = RMP_TO_X86_PG_LEVEL(rmpentry_pagesize(large_entry));
>>
>>     return !!entry->assigned;
>> }
>>
>>
>> And then move dump_rmpentry() (or add a helper) in sev.c so that
>> "struct rmpentry"
>> can be declared in sev.c.
>>
>