linux-kernel - Re: [PATCH Part1 RFC v3 22/22] virt: Add SEV-SNP guest driver

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9c5037b6-d9a7-78e3-f2e4-bf8bebc12da7@amd.com>
Date:   Mon, 5 Jul 2021 05:39:04 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Borislav Petkov <bp@...en8.de>
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>,
        Sean Christopherson <seanjc@...gle.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>, tony.luck@...el.com,
        npmccallum@...hat.com, Dov Murik <dovmurik@...ux.ibm.com>
Subject: Re: [PATCH Part1 RFC v3 22/22] virt: Add SEV-SNP guest driver


On 7/3/21 11:19 AM, Borislav Petkov wrote:
> On Thu, Jul 01, 2021 at 04:32:25PM -0500, Brijesh Singh wrote:
>> The spec definition is present in include/linux/psp-sev.h but sometime
>> we don't expose the spec defs as-is to userspace.
> Why?
>
> Having such undocumented and maybe unwarranted differences - I still
> don't see a clear reason why - is calling for additional and unnecessary
> confusion.

Because some of fields don't make any sense for the userspace interfaces. 


>
>> Several SEV/SEV-SNP does not need to be exposed to the userspace,
>> those which need to be expose we provide a bit modified Linux uapi for
>> it, and for SEV drivers we choose "_user" prefix.
> Is that documented somewhere?
>
> Because "user" doesn't tell me it is a modified structure which is
> different from the spec.

We have good documentation for the SEV ioctl and structure provided
through the KVM interface.

Unfortunately the the documentation for the ioctl and structure provided
through /dev/sev does not exist. We could look into adding this
documentation outside this series. The structure provided through
/dev/sev is identical to the structure documented in the spec with minor
changes such as not exposing reserved fields or rename from paddr to
uaddr etc.


>> e.g
>> a spec definition for the PEK import in include/linux/psp-sev.h is:
>> struct sev_data_pek_cert_import {
>> 	u64 pdh_cert_address;  /* system physical address */
>> 	u32 pdh_cert_len;
>> 	u32 reserved;
>> 	...
>> };
>>
>> But its corresponding userspace structure def in include/uapi/linux/psp-sev.h is:
>> struct sev_user_data_pek_cert_import {
>> 	__u64 pek_cert_uaddr; /* userspace address */
>> 	__u32 pek_cert_len;
>> 	...
>> };
> And the difference is a single "u32 reserved"?

Mostly yes.

>
> Dunno, from where I'm standing this looks like unnecessary confusion to
> me.
>> The ioctl handling takes care of mapping from uaddr to pa and other
>> things as required. So, I took similar approach for the SEV-SNP guest
>> ioctl. In this particular case the guest request structure defined in
>> the spec contains multiple field but many of those fields are managed
>> internally by the kernel (e.g seqno, IV, etc etc).
> Ok, multiple fields sounds like you wanna save on the data that is
> shovelled between kernel and user space and then some of the fields
> don't mean a thing for the user API. Ok.
>
> But again, where is this documented and stated clear so that people are
> aware?
>
> Or are you assuming that since the user counterparts are in
>
> include/uapi/linux/psp-sev.h
> 	^^^^
>
> and it being an uapi header, then that should state that?
>
Yes, the assumption is user wanting to communicate to PSP through
/dev/sev will need to include include psp-sev.h from
uapi/linux/psp-sev.h. The header file itself document the fields
definition, and then user need to refer to SEV SPEC for the further
details. I could start documenting the SNP specific ioctl in
Documentation/virt/coco/sevguest.rst and it can be later expanded to
cover SEV and SEV-ES.

-Brijesh