Thank you so much for your time with such a detailed reply. I need more
time to understand all the information :)
On 05/29/2015 10:45 PM, Laszlo Ersek wrote:
> On 05/29/15 13:58, Heyi Guo wrote:
>> Hi Laszlo and Ard,
>>
>> As you two are the new maintainers for ArmVirtPkg, I suppose you are the
>> right persons to ask this question :)
>>
>> I'm running UEFI SCT aganst QEMU aarch64, and I got error for functional
>> test against HII_CONFIG_ACCESS_PROTOCOL installed in PlatformDxe module.
>>
>> The test case is BBTestRouteConfigFunctionTestCheckpoint1 in
>> TestCase/UEFI/EFI/Protocol/HIIConfigAccess/BlackBoxTest/HIIConfigAccessBBTestFunction.c;
>> it will construct the configuration in some way (sorry I'm not familiar
>> with HII and not understand the process clearly), and then call
>> RouteConfig of HII_CONFIG_ACCESS_PROTOCOL.
> Well, "some way" is certainly the key question here.
>
>> The configuration constructed is like this:
> I'm breaking it up:
>
>> GUID=1cc53572800cab4c87ac3b084a6304b1&
>> NAME=004d00610069006e0046006f0072006d00530074006100740065&
>> PATH=01041400dfc5dcd907405e4390988970935504b27fff0400&
>> OFFSET=0000&
>> WIDTH=0020&
>> VALUE=00000000000000000000000000000000000000000000007400650073006e0055
> Okay, it's been a very long time since I've last looked at HII config
> strings. In any case, if you play with PlatformDxe (go to Device Manager
> | OVMF Platform Configuration, play with the preferred resolution),
> *and* you have build OVMF / ArmVirtPkg with sufficiently long debug
> messages enabled, then you will see debug messages like:
>
> ExtractConfig: Request="GUID=1cc53572800cab4c87ac3b084a6304b1&
> NAME=004d00610069006e0046006f0072006d00530074006100740065&
> PATH=01041400dfc5dcd907405e4390988970935504b27fff0400&
> OFFSET=0&
> WIDTH=0024"
>
> ExtractConfig: Results="GUID=1cc53572800cab4c87ac3b084a6304b1&
> NAME=004d00610069006e0046006f0072006d00530074006100740065&
> PATH=01041400dfc5dcd907405e4390988970935504b27fff0400&
> OFFSET=0&
> WIDTH=0024&VALUE=000000000000000000000000000000000000000000000030003800340078003000340036"
>
> The GUID that you can see is gOvmfPlatformConfigGuid, and it is used in
> "OvmfPkg/PlatformDxe/Platform.c" as the formset GUID. So basically the
> GUID just identifies the formset for which the HII message is for.
>
> The NAME entry is hexadecimal encoding for the CHAR16 string
> "MainFormState". "MainFormState" is the name of the object that stores
> the state of the form / widget in binary representation. The type is
> MAIN_FORM_STATE, declared in "OvmfPkg/PlatformDxe/Platform.h". The form
> definition in "OvmfPkg/PlatformDxe/PlatformForms.vfr" defines the object
> with name "MainFormState", with type MAIN_FORM_STATE, as the state of
> the form.
>
> (You can see some -- hopefully -- enlightening messages in the commit log:
>
> git log -- OvmfPkg/PlatformDxe/ | less
> )
>
> I'm unsure about PATH, it probably partakes in identifying the form
> within the formset, or some such.
>
> In any case, OFFSET, WIDTH, and VALUE describe the contents of
> MainFormState, in binary representation (well, a subset thereof). You
> can see in "OvmfPkg/PlatformDxe/Platform.h" that the size of
> MAIN_FORM_STATE is (2 * 16 + 4) == 30 == 0x24. That means that the above
> OFFSET, WIDTH and VALUE fields define the complete contents of
> MAIN_FORM_STATE.
>
> //
> // This structure describes the form state. Its fields relate strictly
> // to the visual widgets on the form.
> //
> typedef struct {
> UINT16 CurrentPreferredResolution[MAXSIZE_RES_CUR];
> UINT32 NextPreferredResolution;
> } MAIN_FORM_STATE;
>
> The "CurrentPreferredResolution" field is a CHAR16 string; it
> corresponds to the (read only) text field that you see on the form to
> the right of the "Preferred Resolution at Next Boot" label. Its maximum
> size is MAXSIZE_RES_CUR==16 UCS2 characters, including the terminating
> L'\0'.
>
> The "NextPreferredResolution" field is an index into the dynamically
> gathered list of resolutions that the firstly registered GOP supports.
> You can see this selection to the right of the "Change Preferred
> Resolution for Next Boot" label.
>
> (These two fields are separate on the form because
> CurrentPreferredResolution is parsed from non-volatile variable data: it
> can be unset to begin with, plus it can be a resolution that is not even
> *found* in the GOP's list. This might happen eg. if you change the model
> of the emulated video card, or change its video RAM size. Whereas the
> drop-down list offers the currently available GOP modes. So you pick
> your preferred resolution, select "Commit Changes and Exit", and
> (ultimately) reboot the VM.)
>
> Above I quoted the ExtractConfig messages; such messages relate to the
> form engine / display browser extracting configuration from drivers, so
> that the form state can be updated for display. (Refer to SVN r15374.)
>
> The other direction is called "RouteConfig" (see SVN r15375), when the
> form engine updates *part* of the form state (thereby updating part of
> the preexistent driver configuration). This is triggered when you hit
> Enter on "Commit Changes and Exit" (see how the QUESTION_SAVE_EXIT entry
> is hooked up to EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT, in the VFR
> file and in Callback()) *and* the state actually changed.
>
> For example, if you change the NextPreferredResolution dropdown list
> index from 0 to 1, the form engine will send down the following config
> string:
>
>
> RouteConfig: Configuration="GUID=1cc53572800cab4c87ac3b084a6304b1&
> NAME=004d00610069006e0046006f0072006d00530074006100740065&
> PATH=01041400dfc5dcd907405e4390988970935504b27fff0400&
>
> (the usual stuff)
>
> OFFSET=0000&
> WIDTH=0020&VALUE=0000000000000000000000000000000000000030003800340078003000340036&OFFSET=0020&
> WIDTH=0004&
> VALUE=00000001"
>
> Aha! So it sends the first 32 bytes (the CurrentPreferredResolution UCS2
> string) first, then the value of the UINT32 NextPreferredResolution
> index second.
>
> Our RouteConfig() handler performs a read-modify-write cycle in response
> -- that's necessary because the config update can be partial in general.
> See again r15375.
>
> Alright, now we understand what's what.
>
> Apparently, the test case is trying to send down the following config
> string:
>
>> OFFSET=0000&
>> WIDTH=0020&
>> VALUE=00000000000000000000000000000000000000000000007400650073006e0055
> wich means
>
> set the 'CurrentPreferredResolution' string of the form state to
> '<bunch of zeroes>tesnU'
>
> Obviously, our RouteConfig() handler ignores any "changes" to this
> string -- the string is marked READ_ONLY on the form to begin with, it
> only serves display purposes. So we have:
>
> RouteConfig()
> // READ
> PlatformConfigToFormState()
> PlatformConfigLoad()
> GetVariable2()
> gRT->GetVariable()
>
> // MODIFY (may be partial)
> gHiiConfigRouting->ConfigToBlock()
>
> // WRITE
> FormStateToPlatformConfig()
> PlatformConfigSave()
> gRT->SetVariable()
>
>> I traced the code and found it failed in FormStateToPlatformConfig in
>> Platform.c, due to mNumGopModes being 0.
>>
>> if (MainFormState->NextPreferredResolution >= mNumGopModes) {
>> return EFI_INVALID_PARAMETER;
>> }
> Yes. Near the end of this RMW cycle, the NextPreferredResolution field
> is 0. That's because no GOP has been installed yet, so the READ part --
> regardless of the existence of the non-volatile variable that stores the
> preferred resolution -- leaves NextPreferredResolution at zero. After
> all there is no list of resolutions to search, and determine the index
> of the current preference.
>
> In addition, in the MODIFY part, the config string that the tester app
> sends down doesn't cover NextPreferredResolution at all (that would be
> OFFSET=0020, WIDTH=0004).
>
> This is not a problem per se. For example, if the non-volatile variable
> states 2048*2048 as your current (last) preference, but your new video
> ram setting (from the QEMU side) only allows a max resolution of
> 1024x768 (just making up the numbers now), there's no way 2048*2048 will
> be found in the list. In this case, the list index will be left at zero,
> as initial choice.
>
> Now, since we're *routing* the config, that means the user has actually
> selected "Commit". In this case the resolution at index zero (which is
> usually 640x480) will be looked up from the list of resolutions that the
> GOP advertised, and written to the non-volatile variable.
>
> However:
>
>> I suppose GOP related data will be initialized when
>> gEfiGraphicsOutputProtocolGuid is installed,
> Yes. Routing *any* config (ie. committing any selection made on the
> form) makes no sense if the resolution list is empty. You cannot select
> an element in that empty list at any subscript. You cannot derive the
> preferred next resolution, and you cannot write it to the non-volatile
> variable.
>
> This is why FormStateToPlatformConfig() returns EFI_INVALID_PARAMETER:
> the form state is untranslateable to platform configuration.
>
> In normal operation, this can never happen, because using the form, you
> can never trigger a RouteConfig unless at least one resolution has been
> collected.
>
> Basically, the tester utility is sending a RouteConfig message with
> invalid, nonsensical configuration. It says "leave
> NextPreferredResolution at its current value, *and* make that
> configuration permanent". Well, the configuration corresponding to the
> current value of NextPreferredResolution *cannot* be made permanent,
> because it makes no sense. It's an index into something that has not
> been initialized yet.
>
>> so the questions are:
>>
>> 1. Shall I enable vga on qemu to pass this test?
> Yes.
>
>> How to enable vga? I
>> tried -display sdl -vga std, but nothing changed.
> Build ArmVirtQemu with -D INTEL_BDS, and then pass the following options
> to QEMU:
>
> -M virt -cpu cortex-a57 -display sdl -device VGA
>
>> 2. Can we modify the code to make it not depend on GOP?
> No.
>
> I don't know how the tester utility composes the config string, but it
> can't just send down (a seemingly random) one, and expect the driver to
> bend over backwards and make it permanent.
>
> We could probably hack it around, by postponing the installation of the
> EFI_HII_CONFIG_ACCESS_PROTOCOL instance from the entry point of the
> driver (PlatformInit()) to the GOP installation callback
> (GopInstalled()). This would simply prevent the tester utility from
> finding the RouteConfig() function, because until after GOP installation
> PlatformDxe would simply not expose itself as HII-accessible.
>
> However, until someone provides evidence that OvmfPkg/PlatformDxe/,
> as-is, breaks an HII rule, I'm not willing to write such a patch. I'll
> have to be shown why the tester utility is in its right to do what it
> does, and why PlatformDxe is wrong to reject it.
>
>> Please kindly help with this issue and correct me if there is anything
>> wrong in my understanding.
> I hope the above helps.
>
> Thanks
> Laszlo
>
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
