Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <51298b17-9e12-7a08-7322-594deac52f53@amd.com>
Date:   Thu, 25 Aug 2022 13:54:26 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v12 43/46] virt: Add SEV-SNP guest driver
Content-Language: en-US
To:     Peter Gonda <pgonda@google.com>,
        Dionna Amalie Glaze <dionnaglaze@google.com>
Cc:     Brijesh Singh <brijesh.singh@amd.com>,
        the arch/x86 maintainers <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "open list:X86 KVM CPUs" <kvm@vger.kernel.org>,
        linux-efi <linux-efi@vger.kernel.org>,
        platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev,
        Linux Memory Management List <linux-mm@kvack.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        brijesh.ksingh@gmail.com, Tony Luck <tony.luck@intel.com>,
        Marc Orr <marcorr@google.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
References: <20220307213356.2797205-1-brijesh.singh@amd.com>
 <20220307213356.2797205-44-brijesh.singh@amd.com>
 <CAAH4kHYm1BhjJXUMH12kzR0Xun=fUTj-3Hy6At0XR_09Bf0Ccw@mail.gmail.com>
 <CAMkAt6oKQ3CnmNdrJLMWreExkN56t9vs=B883_JD+HtiNYw9HA@mail.gmail.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <CAMkAt6oKQ3CnmNdrJLMWreExkN56t9vs=B883_JD+HtiNYw9HA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VGpJdW5qL2RKTnlQbytnblA3TDVVREhQYmRJT1ZFalhuZ0V4SVhQYkp1ZmU2?=
 =?utf-8?B?aThFclZpYU1pZXJqQ2pWbS9BajdHZUVoOUdYQjRkMDJBTHk3U3ZLZXJJSkRn?=
 =?utf-8?B?WGg4M09aZngySkJNWnpnMEtpVTF0UUZoY3VUZWUyL3hoU1pZZGhwTkFod3ZV?=
 =?utf-8?B?Rk5FWFU3MnRhM1B3QzVqNldhYTFuaDVaU3VVaWt5UHBBTDRBbkg2aGI2Y0dM?=
 =?utf-8?B?ZHhDVWxrNzBZQllwNWlpYnd2cVVpdi9HQVBjQkQydDl1QnNkT0p6SVhVdmlw?=
 =?utf-8?B?YXlJR3V5OVFTTkN4S016RTh1NGppM094VXM4WWx5aVYxY3NPZGZJWGF5Zmcv?=
 =?utf-8?B?ZUJvNDdqQUkySTQ3L3VDYWIzN005YUE5RVQ2bVJZZHpVTkt3c3doL0s2SWdV?=
 =?utf-8?B?T3A0MWM0L0RULzhPakFvZ0FWQW1Hd2lwSWF1TGMxdzBMM1hvT0xFOUd3eHlq?=
 =?utf-8?B?Zmh5VHk2dUZTaC9RSUs4Z242UTQrQ0NhZGZLTExWMWtZSWFJYTBGbUlBNlhx?=
 =?utf-8?B?MWdLdExYZStDVEVhYU5KRld1dmQ2ckpIUTJQa01EV3NiL0pnVVhVaWVKUHQ2?=
 =?utf-8?B?bjZYZXBlblVwdVhkQ0M5RXRDTEJ4L3FUanBEV294RFhDZXR4a2JyMms1SDlZ?=
 =?utf-8?B?YTFXdnQvMGNzZjNmR2lXK2daSWRiRmtFLzl4M0MrWmpDOFNzaXNFa3hJTWQv?=
 =?utf-8?B?RFo0clFUVU9tM2RFMzFuMU1jakV2dWdJNmtYM0c5bStDVkRWVXdKQTdGRDBr?=
 =?utf-8?B?aHJ0cHVNTGpLeXJSeEZvcWRaak5EUmRwSDczcVhqaFJBcEJ4eE4vbnBGUTFN?=
 =?utf-8?B?aHdESEVKU0Y4a0hJdjFxQk1WNm1wT2N2ejhHWDczWjRieksvSTJyUFlIT0lK?=
 =?utf-8?B?YkVHSm56eko5cmdmRC9QM2dwazFEaU9xN1dvT1VaYlpBM1NrQlZZZXJEK0o0?=
 =?utf-8?B?NWdIS0xFNWwxWVpDcFRjSzc3QTVZbjVYM0RldkxWUCt3RDRSRlo2REExdHB0?=
 =?utf-8?B?UEtPaWp1aU5pYk5oS3EwVkdXNTdQRTh3L01FWkJpTnZmSW5xNGdwcTJRdUVt?=
 =?utf-8?B?QzkxTTdzajF4VkYzSnN3RmdMZHNIZkNmb1Z0d0tkdWx1cWk2TGE2QXFvSTlB?=
 =?utf-8?B?RzFsc3RYME1mWE1GVXZNZExBUStheXEwSjRCa1Z4dXhLdWlkdi9ONlhVQzBv?=
 =?utf-8?B?RXpJWUt6aHQ3a0tRSVlzeUx4ZVZRZmZiaDBQUUxsTWlFcDAwMnRmcVdQK25H?=
 =?utf-8?B?Mm9SR0JqcVlKemQrTHlOd0RlK1NqQ3FNaGg5eDB1MUZhN09LbHExOWZ1cGh4?=
 =?utf-8?B?NDFEckNtZVdFWGJIeFFSdkxpbW5WZ1J3bVlvVjFDTTFLV1psbnlWV05TZUh4?=
 =?utf-8?B?TjhqMis5THNnWTdwL21MMUIxejgxeFpqRnlWZ1dJRkVsV0tHOHYwUVMwNElP?=
 =?utf-8?B?b1pocUs1MjdjcEFrZlQ1L04zazNVVmZIcTA4bDYwdWdTVlM4amhNT0dQYjUz?=
 =?utf-8?B?RzdZSGdJM1NrQTdUakVaYmlhWXNINlBXMCthTkVPeVFsVFZETEhnZWdtRElq?=
 =?utf-8?B?YVE4UHJYam1zcVdQRnAyZS8xTUtaNGtRUENCdXkrdjFHeFd2bElvVW1EZnVT?=
 =?utf-8?B?dWlxM1JVVnpqeXV0bzFmWFRvV0t6NnRLbWJXQVNZOHhZNUJyZW1jb3h3Tlla?=
 =?utf-8?B?VkkxOTNib0lQT1NwSUd5Y2hicTBsOEVwY0FzSEh1YzRmSmR0TkpvOWt5dXdQ?=
 =?utf-8?B?bWFZMzB6b0pmVzFNK0ZHV3c3c1FjOEpIVEdLQmpyb0xhZW8vV1NPZzBFakx1?=
 =?utf-8?B?OE80RUd1Z0NEdm4zWXdqSHBiZGE4RW9ZYkNzRHNjR2ptaENNWEw0U1Z1Y0JT?=
 =?utf-8?B?S3FzZC9UL0NDKzV4U0tUc1d1azkvRE9VU1BmQ3F2T21SeksyMC9TM0FkTEdx?=
 =?utf-8?B?STZZbmhXN2UvKzhaMG1FM093MDE0ZU1yUGp0NStCSStFb0VvTHB6dlA5cHdM?=
 =?utf-8?B?bGYwSmRkUlRoYW1WYUtGMFExZ3pvaDlvcTYvWmxDNnZaNzArUmU5Tmw3aHc3?=
 =?utf-8?B?cXdFN3dRNlZBOHIvZk9hbU5rVEtscCt1aTB4ODJ3NDBVczkrWUhkMnU1WVRN?=
 =?utf-8?Q?8yWnSbNT+YbtzkeVaiAD70a6y?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a8e91f02-b9d5-49b5-091e-08da86cb3d70
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Aug 2022 18:54:29.6394
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: h+NV2oIV8ktVImJn3Np3WMHm8EdDNkrbjlS9OBfn2svKwaFrz/VaGUW4LaTtI2foqYwGa/GJAd2humZG2Jmfkw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB4742
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/24/22 14:28, Peter Gonda wrote:
> On Wed, Aug 24, 2022 at 12:01 PM Dionna Amalie Glaze
> <dionnaglaze@google.com> wrote:
>>
>> Apologies for the necropost, but I noticed strange behavior testing my
>> own Golang-based wrapper around the /dev/sev-guest driver.
>>
>>> +
>>> +static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
>>> +                               u8 type, void *req_buf, size_t req_sz, void *resp_buf,
>>> +                               u32 resp_sz, __u64 *fw_err)
>>> +{
>>> +       unsigned long err;
>>> +       u64 seqno;
>>> +       int rc;
>>> +
>>> +       /* Get message sequence and verify that its a non-zero */
>>> +       seqno = snp_get_msg_seqno(snp_dev);
>>> +       if (!seqno)
>>> +               return -EIO;
>>> +
>>> +       memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
>>> +
>>> +       /* Encrypt the userspace provided payload */
>>> +       rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
>>> +       if (rc)
>>> +               return rc;
>>> +
>>> +       /* Call firmware to process the request */
>>> +       rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
>>> +       if (fw_err)
>>> +               *fw_err = err;
>>> +
>>> +       if (rc)
>>> +               return rc;
>>> +
>>
>> The fw_err is written back regardless of rc, so since err is
>> uninitialized, you can end up with garbage written back. I've worked
>> around this by only caring about fw_err when the result is -EIO, but
>> thought that I should bring this up.
> 
> I also noticed that we use a u64 in snp_guest_request_ioctl.fw_err and
> u32 in sev_issue_cmd.error when these should be errors from the
> sev_ret_code enum IIUC.

The reason for the u64 is that the Extended Guest Request can return a 
firmware error or a hypervisor error. To distinguish between the two, a 
firmware error is contained in the lower 32-bits, while a hypervisor error 
is contained in the upper 32-bits (e.g. when not enough contiguous pages 
of memory have been supplied).

Thanks,
Tom

> 
> We can fix snp_issue_guest_request() to set |fw_err| to zero when it
> returns 0 but what should we return to userspace if we encounter an
> error that prevents the FW from even being called? In ` crypto: ccp -
> Ensure psp_ret is always init'd in __sev_platform_init_locked()` we
> set the return to -1 so we could continue that convection here and
> better codify it in the sev_ret_code enum.
> 
>>
>> --
>> -Dionna Glaze, PhD (she/her)