On 12/15/14 12:40, Olivier Martin wrote: > > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: 11 December 2014 02:46 >> To: [email protected]; [email protected]; >> [email protected]; [email protected] >> Subject: [edk2] [PATCH v3 01/13] ArmVirtualizationPkg: VirtFdtDxe: >> forward FwCfg addresses from DTB to PCDs >> >> Qemu's firmware configuration interface for ARM consists of two MMIO >> registers, a 16-bit selector, and a 64-bit data register that allows >> the >> guest to transfer data with 8, 16, 32, and 64-bit wide accesses. Parse >> the >> base address from the DTB, and expose the registers to the rest of DXE >> via >> dynamic PCDs. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <[email protected]> >> --- >> >> Notes: >> v3: >> - updated to the layout with 64-bit wide data register >> >> v2: >> - use "qemu,fw-cfg-mmio" as compatible property >> - the DTB node specifies a single contiguous region >> >> ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf | 2 ++ >> ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c | 24 >> ++++++++++++++++++++++++ >> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec | 3 +++ >> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc | 3 +++ >> 4 files changed, 32 insertions(+) >> >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf >> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf >> index 1c9dd20..daafe6c 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf >> @@ -47,6 +47,8 @@ >> [Pcd] >> gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeBaseAddress >> gArmVirtualizationTokenSpaceGuid.PcdArmPsciMethod >> + gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress >> + gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress >> gArmTokenSpaceGuid.PcdGicDistributorBase >> gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase >> gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c >> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c >> index d002e66..7050196 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c >> @@ -44,6 +44,7 @@ typedef enum { >> PropertyTypeUart, >> PropertyTypeTimer, >> PropertyTypePsci, >> + PropertyTypeFwCfg, >> } PROPERTY_TYPE; >> >> typedef struct { >> @@ -59,6 +60,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = { >> { PropertyTypeTimer, "arm,armv7-timer" }, >> { PropertyTypeTimer, "arm,armv8-timer" }, >> { PropertyTypePsci, "arm,psci-0.2" }, >> + { PropertyTypeFwCfg, "qemu,fw-cfg-mmio" }, >> { PropertyTypeUnknown, "" } >> }; >> >> @@ -115,6 +117,10 @@ InitializeVirtFdtDxe ( >> CONST INTERRUPT_PROPERTY *InterruptProp; >> INT32 SecIntrNum, IntrNum, VirtIntrNum, >> HypIntrNum; >> CONST CHAR8 *PsciMethod; >> + UINT64 FwCfgSelectorAddress; >> + UINT64 FwCfgSelectorSize; >> + UINT64 FwCfgDataAddress; >> + UINT64 FwCfgDataSize; >> >> DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress); >> ASSERT (DeviceTreeBase != NULL); >> @@ -160,6 +166,24 @@ InitializeVirtFdtDxe ( >> (PropType == PropertyTypePsci)); >> >> switch (PropType) { >> + case PropertyTypeFwCfg: >> + ASSERT (Len == 2 * sizeof (UINT64)); >> + >> + FwCfgDataAddress = fdt64_to_cpu (((UINT64 *)RegProp)[0]); >> + FwCfgDataSize = 8; >> + FwCfgSelectorAddress = FwCfgDataAddress + FwCfgDataSize; >> + FwCfgSelectorSize = 2; >> + >> + ASSERT (FwCfgSelectorAddress <= MAX_UINTN - FwCfgSelectorSize + >> 1); >> + ASSERT (FwCfgDataAddress <= MAX_UINTN - FwCfgDataSize + >> 1); > > I am not sure to understand these ASSERT(). > > Should it not be: > ASSERT (FwCfgSelectorAddress < (1 << (FwCfgSelectorSize - 1))); > ASSERT (FwCfgDataAddress < (1 << (FwCfgDataSize - 1)); > > Or > > ASSERT (FwCfgSelectorAddress <= MAX_UINT64); // This one is likely to always > be true. > ASSERT (FwCfgDataAddress <= MAX_UINT16); > > If 'Address' means something else than base address of the MMIO registers > maybe a comment should be added to clarify these two assertions.
The ASSERT()s express that the last byte in each MMIO range is expressible as a MAX_UINTN. Mathematically, Address + Size - 1 <= MAX_UINTN However, in C, if the left hand side (which is performed in UINT64) contained an unsigned overflow -- because Address were too high -- then a check of the form ASSERT (Address + Size - 1 <= MAX_UINTN) would precisely miss it -- the overflow would happen before the comparison. For this reason, the equivalent mathematical form Address <= ((MAX_UINTN - Size) + 1) (which emerges by subtracting (Size - 1) from both sides) is coded, which is safe. Namely, the Size variables are set to very small, but still positive, constant values. Hence, while the above expression is mathematically identical, it has no chance to overflow or underflow in C either (unlike the more obvious coding). Thanks Laszlo > >> + >> + PcdSet64 (PcdFwCfgSelectorAddress, FwCfgSelectorAddress); >> + PcdSet64 (PcdFwCfgDataAddress, FwCfgDataAddress); >> + >> + DEBUG ((EFI_D_INFO, "Found FwCfg @ 0x%Lx/0x%Lx\n", >> FwCfgSelectorAddress, >> + FwCfgDataAddress)); >> + break; >> + >> case PropertyTypeVirtio: >> ASSERT (Len == 16); >> // >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec >> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec >> index b581add..9941154 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec >> @@ -53,3 +53,6 @@ >> # PcdArmPsciMethod == 2 : use SMC >> # >> >> gArmVirtualizationTokenSpaceGuid.PcdArmPsciMethod|0|UINT32|0x00000003 >> + >> + >> gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress|0x0|UINT64|0x0 >> 0000004 >> + >> gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress|0x0|UINT64|0x00000 >> 005 >> diff --git >> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> index 61689b7..1f3ddea 100644 >> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc >> @@ -173,6 +173,9 @@ >> >> gArmVirtualizationTokenSpaceGuid.PcdArmPsciMethod|0 >> >> + gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress|0x0 >> + gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress|0x0 >> + >> >> ####################################################################### >> ######### >> # >> # Components Section - list of all EDK II Modules needed by this >> Platform >> -- >> 1.8.3.1 >> >> >> >> ----------------------------------------------------------------------- >> ------- >> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >> from Actuate! Instantly Supercharge Your Business Reports and >> Dashboards >> with Interactivity, Sharing, Native Excel Exports, App Integration & >> more >> Get technology previously reserved for billion-dollar corporations, >> FREE >> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.c >> lktrk >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
