From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web08.1659.1620242230059642093 for ; Wed, 05 May 2021 12:17:10 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=TX21owHc; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620242229; 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=NSgX5K4Gsuw95MHGvAVGq1VKQRAtOF+7cJDghVU9ZxQ=; b=TX21owHcwKeHdFopPk66CsfO2LCvPL2nNpT04BBMR04NZ8lMV7QO3HW8CnLAXGnwcYBmHN 8jzjVq8R2mxlrf2NEokClMh/LprxomhGzsu5g0aYo6cDYQT7QDk5ZLvHgo+AYEiVNwJIwK 7dS5/kMdSlIBw26H/+UzkU9xFJlwo3I= 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-346-FSJF1XjzPgqNwSKnXEfasw-1; Wed, 05 May 2021 15:17:06 -0400 X-MC-Unique: FSJF1XjzPgqNwSKnXEfasw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 285A679EC0; Wed, 5 May 2021 19:17:05 +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 3B09B60D08; Wed, 5 May 2021 19:17:03 +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> From: "Laszlo Ersek" Message-ID: <44eb7ad0-7374-e39a-6dde-39571e7a5219@redhat.com> Date: Wed, 5 May 2021 21:17:02 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 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: 7bit On 05/04/21 22:28, 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. I'm sorry that I'm responding out-of-order to this email of yours -- for some reason, your email is not correctly threaded in my list folder. It is not connected to the 05/28 sub-thread, but to the blurb. > 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. That's right. First, you are only providing an X64 function definition. Second, I requested that even the *declaration* of the function be restricted to X64. Third, I requested that we document in the leading comment that, unless CPUID reports support for PVALIDATE, the function will trigger an #UD; it's the caller's responsibility *not* to call the function, then. With those in mind, EFI_UNSUPPORTED *need not* stand for any particular "platform characteristic", it can stand for whatever we want it to -- such as mismatched page size. Still, in case you disliked this ambiguity of EFI_UNSUPPORTED, I offered EFI_NO_MAPPING, for the page size mismatch case. > I am good with it. I will go ahead with it. Thanks -- if you still prefer the UINTN approach, I can live with that too (although EFI_STATUS would certainly make me happier). Thank you! Laszlo