Hi Ard, On Thu, Jan 19, 2017 at 06:16:00PM +0000, Ard Biesheuvel wrote: > 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?
Thanks for the pointer. Somehow NorFlashFvbInitialize() gets called and a valid firmware volume header is not found. This irrespective of the state of cache modelling. The function proceeds to install a header for which it first tries to erase the Flash blocks reserved for variable storage. I am not sure why a FV header is not found. However the flash erase in response happens with the pre-DXE memory attributes for the flash region. The GCD services to change the attributes are called later. So this seems like a logical error in the code. Does this make sense to you? Here is the trace for reference. The last print was added by me. >>>> add-symbol-file /home/achgup01/work/genfw/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC49/AARCH64/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe/DEBUG/ArmVeNorFlashDxe.dll 0xEAEA0000 Loading driver at 0x000EAE90000 EntryPoint=0x000EAEA0044 ArmVeNorFlashDxe.efi NorFlashWriteBlocks: informational - Had to enable HSYS_FLASH flag. FvbRead(Parameters: Lba=0, Offset=0x0, *NumBytes=0x40, Buffer @ 0xF3FFF8A8) NorFlashFvbInitialize ValidateFvHeader: No Firmware Volume header present NorFlashFvbInitialize: The FVB Header is not valid. NorFlashFvbInitialize: Installing a correct one for this volume. FvbEraseBlocks() FvbEraseBlocks: Check if: ( StartingLba=0 + NumOfLba=3 - 1 ) > LastBlock=2. FvbEraseBlocks: Erasing Lba=0 @ 0x0FFC0000. NorFlashEraseSingleBlock: BlockAddress=0x0FFC0000 >>>> Any pointers on the absent FV header and how NorFlashFvbInitialize() gets called would be helpful. cheers, Achin _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
