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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74769): https://edk2.groups.io/g/devel/message/74769 Mute This Topic: https://groups.io/mt/82479052/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
