On 19 January 2017 at 12:31, Achin Gupta <[email protected]> wrote:
> Hi Leif/Ard,
>
> On Wed, Jan 18, 2017 at 10:05:00PM +0000, Leif Lindholm wrote:
>> Hi Achin,
>>
>> On Wed, Jan 18, 2017 at 08:24:06PM +0000, [email protected] wrote:
>> > From: Achin Gupta <[email protected]>
>> >
>> > The NOR flash banks were being mapped in the translation tables with the
>> > same
>> > memory attributes as RAM in the system. These attributes mark the region as
>> > Normal Memory and could additionally be cacheable or non-cacheable.
>> >
>> > Either type of attributes are unsuitable for NOR Flash since write
>> > operations
>> > could be performed on it. Normal Memory does not guarantee ordering of
>> > transactions that Device memory does. So the commands sent to the Flash
>> > device
>> > may not arrive in the right order unless barriers are used. Commands might
>> > not
>> > get past the CPU caches in case the region has been mapped with cacheable
>> > attributes.
>> >
>> > This patch fixes the problem by mapping the NOR Flash memory region with
>> > Device
>> > memory attributes.
>>
>> To add some background to Ard's comment - this was not unintentionally
>> done:
>> https://github.com/tianocore/edk2/commit/19bb46c411279dcd30d540c56e5993c5f771c319
>
> Thanks! I missed this commit. There is some background to the problem I am
> facing below.
>
>>
>> Was the reasoning behind this commit incorrect - do you have a
>> (pre-DXE?) use-case that creates a problem?
>
> AFAIU, The current memory attributes for NOR Flash work only for reads. They
> should additionally be RO to flag any unexpected writes.
>
> Mine is a DXE use case! In NorFlashDxe.c, commands are send to the Flash
> (erase
> block etc.). They might never reach the device if there is a write-back cache
> in
> between. So either device or Normal memory with non-cacheable/write-through
> attributes and barriers should work.
>
> If I turn on cache state modelling in the Base FVP, the code gets stuck in
> NorFlashReadStatusRegister() in the below loop in NorFlashEraseSingleBlock():
>
> // Wait until the status register gives us the all clear
> do {
> StatusRegister = NorFlashReadStatusRegister (Instance, BlockAddress);
> } while ((StatusRegister & P30_SR_BIT_WRITE) != P30_SR_BIT_WRITE);
>
> I think the SEND_NOR_COMMANDs at the beginning of the function get stuck in
> the
> cache. Changing the flash memory attributes as per this patch solves the
> problem.
>
> The original patch from Ard mentions that the NOR Flash DXE driver should
> change
> the attributes of the region it wants to write to. Is this what is missing?
>
NorFlashFvbInitialize() in
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c contains the
following calls:
Status = gDS->AddMemorySpace (
EfiGcdMemoryTypeMemoryMappedIo,
Instance->DeviceBaseAddress, RuntimeMmioRegionSize,
EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
);
ASSERT_EFI_ERROR (Status);
Status = gDS->SetMemorySpaceAttributes (
Instance->DeviceBaseAddress, RuntimeMmioRegionSize,
EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
ASSERT_EFI_ERROR (Status);
which is supposed to set device attributes on the NOR region that is
actually exposed to the upper layers as read-write capable.
Perhaps you can enable GCD debugging to trace these calls, to ensure
that the address you are stalled on is actually covered?
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
