> +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
1. Can you rename it to "IntelMmSaveStateLib"?
> + INF_VERSION = 1.29
> + BASE_NAME = IntelSmmSmramSaveStateLib
> + FILE_GUID = 37E8137B-9F74-4250-8951-7A970A3C39C0
> + MODULE_TYPE = DXE_SMM_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = SmmSmramSaveStateLib
2. Can you check back the comments I left for AMD instance and apply similar
changes here?
> **/
> EFI_STATUS
> -EFIAPI
3. Why remove "EFIAPI"?
> +/**
> + Read an SMM Save State register on the target processor. If this function
> + returns EFI_UNSUPPORTED, then the caller is responsible for reading the
> + SMM Save Sate register.
> +
> + @param[in] CpuIndex The index of the CPU to read the SMM Save State.
> The
> + value must be between 0 and the NumberOfCpus field in
> + the System Management System Table (SMST).
> + @param[in] Register The SMM Save State register to read.
> + @param[in] Width The number of bytes to read from the CPU save
> state.
> + @param[out] Buffer Upon return, this holds the CPU register value read
> + from the save state.
> +
> + @retval EFI_SUCCESS The register was read from Save State.
> + @retval EFI_INVALID_PARAMTER Buffer is NULL.
> + @retval EFI_UNSUPPORTED This function does not support reading
> Register.
> + @retval EFI_NOT_FOUND If desired Register not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmramSaveStateReadRegister (
> + IN UINTN CpuIndex,
> + IN EFI_SMM_SAVE_STATE_REGISTER Register,
> + IN UINTN Width,
> + OUT VOID *Buffer
> + )
> +{
> + UINT32 SmmRevId;
> + SMRAM_SAVE_STATE_IOMISC IoMisc;
> + EFI_SMM_SAVE_STATE_IO_INFO *IoInfo;
> +
> + //
> + // Check for special EFI_SMM_SAVE_STATE_REGISTER_LMA
> + //
> + if (Register == EFI_SMM_SAVE_STATE_REGISTER_LMA) {
> + //
> + // Only byte access is supported for this register
> + //
> + if (Width != 1) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *(UINT8 *)Buffer = SmramSaveStateGetRegisterLma ();
4. It's Intel specific flow. I am curious how AMD flow handles the LMA read.
> +
> + return EFI_SUCCESS;
> + }
> +
> + //
> + // Check for special EFI_SMM_SAVE_STATE_REGISTER_IO
> + //
> + if (Register == EFI_SMM_SAVE_STATE_REGISTER_IO) {
5. Similar question here: how AMD flow handles the IO read?
> +
> + //
> + // Convert Register to a register lookup table index
> + //
> + return SmramSaveStateReadRegisterByIndex (CpuIndex,
> SmramSaveStateGetRegisterIndex (Register), Width, Buffer);
6. Can you double check here? The mSmmSmramCpuWidthOffset[] of Intel/AMD
version don't put the GdtBase in the same location.
SMM_SAVE_STATE_REGISTER_MAX_INDEX is 2 for AMD but should be 4 for Intel.
How about define a "CONST UINTN gSmmSaveStateRegisterMaxIndex" with different
values in Intel/AmdSmramSaveState.c?
> EFI_STATUS
> -EFIAPI
7. Why remove "EFIAPI"?
> +UINT8
> +IntelSmramSaveStateGetRegisterLma (
8. Can you implement the SmramSaveStateGetRegisterLma() in Intel/Amd C files?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102815): https://edk2.groups.io/g/devel/message/102815
Mute This Topic: https://groups.io/mt/98172953/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
