Re: RuntimeDXE unable to save variables to flash

[Laszlo Ersek <lersek@redhat.com>]

On 10/09/18 12:25, Hristo Mihaylov wrote:
> Thanks Laszlo,
> 
>> Accessing invalid MSRs may raise injections. How do you know the MSR 0x1FE is valid (and the Data32 value is valid)?
> 
> I'll investigate this further.
> 
>> That's an Fvb->Write() call. Do you have access to the source of the flash driver (which produces the FVB protocol instance)?
> 
> Yes, I found that the writing is done in a function called SendSpiCmd, here's the signature (I can't disclose the full source).
> 
> ```
> /**
>   This function sends the programmed SPI command to the slave device.
> 
>   @param[in] This                 Pointer to the PCH_SPI_PROTOCOL instance.
>   @param[in] SpiRegionType        The SPI Region type for flash cycle which is listed in the Descriptor
>   @param[in] FlashCycleType       The Flash SPI cycle type list in HSFC (Hardware Sequencing Flash Control Register) register
>   @param[in] Address              The Flash Linear Address must fall within a region for which BIOS has access permissions.
>   @param[in] ByteCount            Number of bytes in the data portion of the SPI cycle.
>   @param[in,out] Buffer           Pointer to caller-allocated buffer containing the dada received or sent during the SPI cycle.
> 
>   @retval EFI_SUCCESS             SPI command completes successfully.
>   @retval EFI_DEVICE_ERROR        Device error, the command aborts abnormally.
>   @retval EFI_ACCESS_DENIED       Some unrecognized command encountered in hardware sequencing mode
>   @retval EFI_INVALID_PARAMETER   The parameters specified are not valid.
> **/
> EFI_STATUS
> SendSpiCmd (
>   IN     EFI_SPI_PROTOCOL   *This,
>   IN     FLASH_REGION_TYPE  FlashRegionType,
>   IN     FLASH_CYCLE_TYPE   FlashCycleType,
>   IN     UINT32             Address,
>   IN     UINT32             ByteCount,
>   IN OUT UINT8              *Buffer
>   )
> ```
> 
> It fails when it does a sanity check. It checks `if (Address + ByteCount) > FlashRegionSize`. The FlashRegionSize 
> seems to be estimated correctly, because it's used to write other values to that region, which succeed.
> 
> When I log the sanity check like for the incorrect value I see: `SendSpiCmd: (46334052 + 60 = 46334112) > 20971520` 
> the address seems to be way off.
> 
> The Address is calculated in FvbGetLbaAddress. Almost a 1 to 1 copy of 
> https://github.com/tianocore/edk2/blob/75b7aa9528bdd05a7ecf4e64a6beb478d31b402c/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c#L237
> 
> I thought that the issue is either because of invalid FV block or the size of the flash region is calculated
>  incorrectly, but this only happens in SMM. Now I'm leaning more towards permissions.
> 
> Any advice?

Hmm, not much. Either the blockmap is wrong, as you suggest, or else the
Lba input param to FvbGetLbaAddress() is bogus, and FvbGetLbaAddress()
returns EFI_INVALID_PARAMETER, when it reaches the terminator blockmap
entry (without finding Lba).

Does the caller of FvbGetLbaAddress() check the return value?

Laszlo