From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web09.1697.1620242382123842765 for ; Wed, 05 May 2021 12:19:42 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dpXd7XSv; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620242381; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kYcyMp667j74aUqchSdOY7fGODAF+uLjRMuBsEgGHWk=; b=dpXd7XSv7F6efQnr0eormpIVp/hfKaNbJzx7IjKeF2Avw0/qzvv4KZhLbTCVkb7jjzSiGr 13hrlkjxgq2HfmkxtY8J/UQ1GQO+saIVk4rijXQw+nxNM1vdLOPriFKCw3AzuwTXrG+PEo mb9I2RG+Vyt0Kw7Ck47Gevy+l/JP43g= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-199-Mf2g5T44NMO6rWGqs_qZZg-1; Wed, 05 May 2021 15:19:39 -0400 X-MC-Unique: Mf2g5T44NMO6rWGqs_qZZg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C95F01922960; Wed, 5 May 2021 19:19:37 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-136.ams2.redhat.com [10.36.113.136]) by smtp.corp.redhat.com (Postfix) with ESMTP id A55F76062C; Wed, 5 May 2021 19:19:35 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH RFC v2 05/28] MdePkg: Add AsmPvalidate() support To: devel@edk2.groups.io, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas References: <20210430115148.22267-1-brijesh.singh@amd.com> <20210430115148.22267-6-brijesh.singh@amd.com> <167BF2A01FA60569.6407@groups.io> <167BF53DA09B327E.22277@groups.io> <68377db4-c1b6-afc9-3885-ad1e702aff0c@amd.com> From: "Laszlo Ersek" Message-ID: <7121b739-a5bd-d892-b8e3-07d85493f5ab@redhat.com> Date: Wed, 5 May 2021 21:19:34 +0200 MIME-Version: 1.0 In-Reply-To: <68377db4-c1b6-afc9-3885-ad1e702aff0c@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 05/05/21 01:03, Brijesh Singh wrote: > > On 5/4/21 3:28 PM, Brijesh Singh wrote: >> On 5/4/21 2:55 PM, Brijesh Singh via groups.io wrote: >>> On 5/4/21 2:07 PM, Brijesh Singh via groups.io wrote: >>>>> Return EFI_UNSUPPORTED (0x8000_0003), or even EFI_NO_MAPPING >>>>> (0x8000_0017), for value 6 (FAIL_SIZEMISMATCH). >>>> I am not sure if we really want to do this. You will see later in the >>>> patches that in some cases the PVALIDATE will return a failure and we >>>> will need to know the failure code to determine the next steps. >>>> Especially this particular error code is used later. This error happens >>>> when the page size of the backing pages does not match with the >>>> pvalidated size. In those cases we need to retry the PVALIDATE with >>>> lower page size so that a validation succeed. One such a example is: >>>> >>>> - Guest ask hypervisor to add the page as 2M in RMP table. >>>> >>>> - Hypervisor added the page as 512 4K pages - because it was not able to >>>> find a large backing pages. >>>> >>>> - Guest attempts to pvalidate the page as a 2M. The pvalidate will >>>> return a failure saying its a size mismatch between the requested >>>> pvalidated and RMP table. The recommendation is that guest should try >>>> with a smaller page size. >>>> >>>> I would prefer to pass the pvalidate error as-is to caller so that it >>>> can make the correct decision. >>>> >>> I am perfectly fine if the function return UINTN and then use #define >>> instead of the enum to define the PVALIDATE return code. So that caller >>> can check the error code. Let me know your thought on #define instead of >>> the enum. >> Apologies, I missed the fact that you said document the mapping between >> the PVALIDATE return value and EFI_STATUS. So a caller is responsible to >> look at the EFI document to know what the error code means. The >> unsupported here does not mean that PVALIDATE is not support on >> platform. I am good with it. I will go ahead with it. > > While coding it I am coming to realizing that mapping from PVALIDATE > return value to EFI will add more code in assemblym and wanted to make > sure that we all are okay with it. The proposed mapping looks like this: > > // No mapping required, both the values are zero > > PVALIDATE_RET_SUCCESS -> EFI_SUCCESS    > > // Pvalidate.nasm need to map from PVALIDATE fail input (1) to > EFI_INVALID_PRAMS (2) > > PVALIATE_RET_FAIL_INPUT -> EFI_INVALID_PARAMS > > // Pvalidate.nasm need to map from PVALIDATE SIZE_MISMATCH (6) to > EFI_UNSUPPORT(3) > > PVALIATE_RET_SIZEMISMATCH -> EFI_UNSUPPORTED > > May I recommend to use UINTN and simply propagate the PVALIDATE return > value  or use the below mapping to simplify the pvalidate.nasm > implementation: > > PVALIDATE_RET_SUCCESS(0) -> EFI_SUCCESS(0) > > PVALIDATE_RET_FAIL_INPUT(1) -> EFI_LOAD_ERROR(1) > > PVALIDATE_RET_SIZE_MISMATCH(6) -> EFI_NOT_READY(6) The additional complexity in the assembly source code is not lost on me. I'm receptive to that. I think EFI_LOAD_ERROR and EFI_NOT_READY would do more damage than good; those error codes are meaningless in this context. Therefore, please go ahead with the UINTN return type. And, whether you pick #defines, or enum constants, for the symbolic names (for the comparisons at the call sites), that's up to you. Thanks! Laszlo