On 12/17/14 22:34, Laszlo Ersek wrote:
> Changes in v4:
> - addressing review comments from Olivier & Peter (see per-patch
> notes)
> - the fw_cfg MMIO selector & data registers are big endian now
> - public branch:
>
> https://github.com/lersek/edk2/commits/armvirt_fwcfg_efi_kernel_support_bz1128341_v4
>
> Thanks
> Laszlo
>
> Laszlo Ersek (13):
> ArmVirtualizationPkg: VirtFdtDxe: forward FwCfg addresses from DTB
> to PCDs
> ArmVirtualizationPkg: introduce QemuFwCfgLib instance for DXE
> drivers
> ArmVirtualizationPkg: clone PlatformIntelBdsLib from ArmPlatformPkg
> ArmVirtualizationPkg: PlatformIntelBdsLib: add basic policy
> OvmfPkg: extract QemuBootOrderLib
> OvmfPkg: QemuBootOrderLib: featurize PCI-like device path
> translation
> OvmfPkg: introduce VIRTIO_MMIO_TRANSPORT_GUID
> ArmVirtualizationPkg: VirtFdtDxe: use dedicated
> VIRTIO_MMIO_TRANSPORT_GUID
> OvmfPkg: QemuBootOrderLib: widen ParseUnitAddressHexList() to UINT64
> OvmfPkg: QemuBootOrderLib: OFW-to-UEFI translation for virtio-mmio
> ArmVirtualizationPkg: PlatformIntelBdsLib: adhere to QEMU's boot
> order
> ArmVirtualizationPkg: identify "new shell" as builtin shell for
> Intel BDS
> ArmVirtualizationPkg: Intel BDS: load EFI-stubbed Linux kernel from
> fw_cfg
I committed this series as r16566-r16578 to SVN.
Patch #2 was updated as follows, relative to v4:
> diff --git
> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> index 375cc94..e9e96d7 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> @@ -138,22 +138,22 @@ InternalQemuFwCfgReadBytes (
>
> #ifdef MDE_CPU_AARCH64
> while (Ptr < End) {
> - *(UINT64 *)Ptr = SwapBytes64 (MmioRead64 (mFwCfgDataAddress));
> + *(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
> Ptr += 8;
> }
> if (Left & 4) {
> - *(UINT32 *)Ptr = SwapBytes32 (MmioRead32 (mFwCfgDataAddress));
> + *(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
> Ptr += 4;
> }
> #else
> while (Ptr < End) {
> - *(UINT32 *)Ptr = SwapBytes32 (MmioRead32 (mFwCfgDataAddress));
> + *(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
> Ptr += 4;
> }
> #endif
>
> if (Left & 2) {
> - *(UINT16 *)Ptr = SwapBytes16 (MmioRead16 (mFwCfgDataAddress));
> + *(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress);
> Ptr += 2;
> }
> if (Left & 1) {
For the following reason:
The fw_cfg data register, unlike the selector register, transfers blobs,
not integer values. (Any integer-like interpretation for (parts of) the
transferred blob is subject to independent and selector-specific
agreement and is not regulated on the transfer level.) V4 did not
distinguish the data register from the selector register in this sense,
and incorrectly swizzled bytes for the data register automatically. (The
qemu code was also wrong.)
See the qemu patch and discussion here:
<http://thread.gmane.org/gmane.comp.emulators.qemu/312419>.
Regarding the review / testing status of the patchset that I pushed:
- OvmfPkg patches (05-07, 09-10): Acked-by Jordan
- Patches 04, 11-12: Reviewed-by Olivier
- I answered all v3 questions and addressed all explicit v3 requests
regarding the rest of the patches:
patch# changes in v4:
------ --------------
01 comment on the MMIO address range ASSERT()s in
InitializeVirtFdtDxe() [Olivier]
Olivier's v3 comments that the v4 changes address:
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11651/focus=11703
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11704/focus=11716
02 The fw_cfg selector and data registers are big endian now.
Swap bytes explicitly in QemuFwCfgSelectItem() and
InternalQemuFwCfgReadBytes(). (NOTE: the swapping in
InternalQemuFwCfgReadBytes() was unjustified and wrong, and
has been reverted. See above.)
03 clean up "missing space before open parenthesis" and "missing
space after comma or semicolon" coding style problems
[Olivier]
Olivier's v3 comments that the v4 changes address:
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11651/focus=11718
08 no changes (we discussed this patch but Olivier didn't request
any changes ultimately)
Discussion:
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11651/focus=11720
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11721
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11737
13 handle the no-initrd case -- don't pass "initrd=initrd" to the
kernel then [Olivier, Peter]
Olivier's v3 comments that the v4 changes address:
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11651/focus=11734
- When Peter Maydell helped test the host endianness qemu patch
<http://thread.gmane.org/gmane.comp.emulators.qemu/312419> on a
big-endian host, he performed those tests with a firmware image that I
built for him and that contained this patchset
<http://people.redhat.com/~lersek/for_Peter/>. IOW Peter's testing was
end-to-end and exercised this functionality. (Many thanks for that.)
I committed the set to SVN because the last posting, v4, has spent two
weeks on the list without any additional feedback; because I addressed
all explicit requests I had received for v3; because I got independent
testing; and because Ard has just posted patches to edk2-devel that
share code context with this series (and should be probably rebased
therefore).
Thanks
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
