Re: [edk2] [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
Hi Ard, On 2018/2/6 20:04, Ard Biesheuvel wrote: > Currently, the GIC driver has a static dependency on the CPU arch protocol > driver, so it can register its IRQ handler at init time. This means there > is a window between dispatch of the CPU driver and dispatch of the GIC > driver where any unexpected GIC state may trigger an interrupt which we > are not set up to handle yet. Note that this is even the case if we enter > UEFI with interrupts disabled at the CPU, given that any TPL manipulation > involving TPL_HIGH_LEVEL will unconditionally enable IRQs at the CPU side > regardless of whether they were enabled to begin with (but only as soon as > the CPU arch protocol is actually installed) > > So let's reorder the GIC driver with the CPU driver, and let it run its > initialization that puts the GIC into a known state before enabling > interrupts. Move its installation of its IRQ handler to a protocol notify > callback on the CPU arch protocol so that it runs as soon as it becomes > available. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > > This fixes an issue observed with GICv3 guests running under KVM. I'm backporting this patch to the branch UDK2017 with three extra patches. 3ba3528 ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver c397d52 ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol 9a73ffd EmbeddedPkg: Introduce HardwareInterrupt2 protocol 29d33ba ArmPkg: Tidy GIC code before changes. But when I start a VM using the new edk2 binary, it throws below error. ASSERT_EFI_ERROR (Status = Device Error) ASSERT [Shell] /root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/RELEASE_GCC49/AARCH64/ShellPkg/Application/Shell/Shell/DEBUG/AutoGen.c(885): !EFI_ERROR (Status) Synchronous Exception at 0x00023866A1F0 X0 0x00FF X1 0x000A X2 0x00AB X3 0x00023F2A2B80 X4 0x0001 X5 0x0001 X6 0x X7 0x0002 X8 0x X9 0x0007 X10 0x000238B6 X11 0x00023AD86FFF X12 0x X13 0x0001 X14 0x X15 0x X16 0x X17 0x00672E50 X18 0x00672E50 X19 0x00023B2ECE98 X20 0x00023BFF0018 X21 0x8007 X22 0x00023F2CB088 X23 0x X24 0x00023B2ED438 X25 0x00023B2ED440 X26 0x00023F2CA3D8 X27 0x0095 X28 0x4007E010 FP 0x LR 0x00023861E844 V0 0x V1 0x63702F66 V2 0x7363732F312C3140 6567646972622D69 V3 0x V4 0x0010 V5 0x4010040140100401 4010040140100401 V6 0x0010 0010 V7 0x V8 0x V9 0x V10 0x V11 0x V12 0x V13 0x V14 0x V15 0x V16 0x V17 0x V18 0x V19 0x V20 0x V21 0x V22 0x V23 0x V24 0x V25 0x V26 0x V27 0x V28 0x V29 0x V30 0x V31 0x SP 0x00023F2A2B70 ELR 0x00023866A1F0 SPSR 0x6305 FPSR 0x ESR 0x5600DBDB FAR 0x1DE7EC7EDBADC0DE ESR : EC 0x15 IL 0x1 ISS 0xDBDB SVC executed in AArch64 Stack dump: 23F2A2A70: 00023861E820 00023BFF0018 00023F2A2B70 00023F2A2B70 23F2A2A90: 00023F2A2B40 FF80FFD8 00023F2A2B70 00023F2A2B70 23F2A2AB0: 00023F2A2B40 FF80FFD8 23F2A2AD0: 63702F66 6567646972622D69 7363732F312C3140 23F2A2AF0: 0010 23F2A2B10: 4010040140100401 4010040140100401 0010 0010 23F2A2B30: 00023F2A2C2A 00023866A74C 00023B2ECE98 00023BFF0018 23F2A2B50: 8007 00023F2CB088 00023861E834 > 23F2A2B70: 00023860B4A4 0010 5B20545245535341 2F205D6C6C656853 23F2A2B90: 6D70722F746F6F72 55422F646C697562 326B64652F444C49 422F302E372E322D 23F2A2BB0: 6D72412F646C6975 756D6551747269
Re: [edk2] [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
On 2017/11/15 22:03, Ard Biesheuvel wrote:
> On 15 November 2017 at 13:51, Laszlo Ersek wrote:
>> > On 11/14/17 11:22, Ard Biesheuvel wrote:
>>> >> The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions,
>>> >> assuming such regions always have full memory semantics. Given that
>>> >> those regions cannot be mapped as ordinary memory on ARM (due to the
>>> >> fact that the NOR flash requires device semantics while in write mode)
>>> >> this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe,
>>> >> since it may use unaligned accesses and/or DC ZVA instructions, both
>>> >> of which are incompatible with mappings using device semantics.
>>> >>
>>> >> Note that there is no way we can work around this by changing the
>>> >> mapping type between 'memory' and 'device' when switching from read to
>>> >> write mode and back, because the runtime mapping is created by the OS,
>>> >> and cannot be changed at will.
>>> >>
>>> >> So let's just switch to the unaccelerated version of BaseMemoryLib which
>>> >> does not have the same problem.
>>> >>
>>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>>> >> Signed-off-by: Ard Biesheuvel
>>> >> ---
>>> >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++
>>> >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
>>> >> 2 files changed, 4 insertions(+)
>>> >>
>>> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>>> >> index 8a60b61f2aa6..7b220d6e3c31 100644
>>> >> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>>> >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>>> >> @@ -261,6 +261,8 @@ [Components.common]
>>> >>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>>> >>
>>> >>NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
>>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash
>>> >> region
>>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>> >>}
>>> >> !if $(SECURE_BOOT_ENABLE) == TRUE
>>> >>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>>> >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> >> b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> >> index 9a31ec93ca06..7c032e1b07e0 100644
>>> >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>>> >> @@ -252,6 +252,8 @@ [Components.common]
>>> >>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>>> >>
>>> >>NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
>>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash
>>> >> region
>>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>> >>}
>>> >> !if $(SECURE_BOOT_ENABLE) == TRUE
>>> >>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>>> >>
>> >
>> > Reviewed-by: Laszlo Ersek
>> >
>> > Given that we've never seen the symptom reported by Shannon, I must
>> > think Shannon is using some kind of new hardware. May I ask what
>> > hardware that is?
>> >
> I think this is equally likely to occur in any KVM hardware
> virtualization context. It is simply a regression after moving to the
> OptDxe BaseMemoryLib flavor.
Exactly. I'm using Huawei D05. It works well when I use a older edk2
while fails when updating to UDK2017 branch.
Tested-by: Shannon Zhao
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] edk2 occur Data abort: Alignment fault on ARM64
On 2017/11/11 22:14, Shannon Zhao wrote: > Hi Ard, > > On 2017/11/11 19:55, Ard Biesheuvel wrote: >> > On 11 November 2017 at 11:48, Shannon Zhao >> > wrote: >>>> >> > Hi, >>>> >> > >>>> >> > I'm using UDK2017(commit is eea98ee UefiCpuPkg/MpLib: fix potential >>>> >> > overflow issue.) to start a VM on ARM64. But I got below exception >>>> >> > sometimes. While I debug this issue, I didn't find some valuable >>>> >> > information. Do you have any suggestion? Thanks. >>>> >> > >> > Could you try this please? >> > >> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc >> > index c92a69281ae4..6b38c9b21f80 100644 >> > --- a/ArmVirtPkg/ArmVirt.dsc.inc >> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc >> > @@ -228,6 +228,7 @@ >> > >> > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf >> > >> > [LibraryClasses.common.DXE_RUNTIME_DRIVER] >> > + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >> > >> > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf >> >CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf >> > > I test this and looks like it works so far. I will test it for longer > timer. Thanks a lot. I've tested this patch for 24 hours without previous issue happening. It does solve this issue I think. Ard, will you submit a patch? Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] edk2 occur Data abort: Alignment fault on ARM64
Hi Ard, On 2017/11/11 19:55, Ard Biesheuvel wrote: > On 11 November 2017 at 11:48, Shannon Zhao wrote: >> > Hi, >> > >> > I'm using UDK2017(commit is eea98ee UefiCpuPkg/MpLib: fix potential >> > overflow issue.) to start a VM on ARM64. But I got below exception >> > sometimes. While I debug this issue, I didn't find some valuable >> > information. Do you have any suggestion? Thanks. >> > > Could you try this please? > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index c92a69281ae4..6b38c9b21f80 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -228,6 +228,7 @@ > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf > > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf >CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf > I test this and looks like it works so far. I will test it for longer timer. Thanks a lot. -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] edk2 occur Data abort: Alignment fault on ARM64
Hi, I'm using UDK2017(commit is eea98ee UefiCpuPkg/MpLib: fix potential overflow issue.) to start a VM on ARM64. But I got below exception sometimes. While I debug this issue, I didn't find some valuable information. Do you have any suggestion? Thanks. Synchronous Exception at 0x000238A7A018 PC 0x000238A7A018 (0x000238A6+0x0001A018) [ 0] VariableRuntimeDxe.dll PC 0x000238A79DE8 (0x000238A6+0x00019DE8) [ 0] VariableRuntimeDxe.dll PC 0x000238A73418 (0x000238A6+0x00013418) [ 0] VariableRuntimeDxe.dll PC 0x000238A76048 (0x000238A6+0x00016048) [ 0] VariableRuntimeDxe.dll PC 0x000238A7748C (0x000238A6+0x0001748C) [ 0] VariableRuntimeDxe.dll PC 0x0002387A179C (0x00023879+0x0001179C) [ 1] MonotonicCounterRuntimeDxe.dll PC 0x0002387A0794 (0x00023879+0x00010794) [ 1] MonotonicCounterRuntimeDxe.dll PC 0x0002387A0948 (0x00023879+0x00010948) [ 1] MonotonicCounterRuntimeDxe.dll PC 0x0002387A0574 (0x00023879+0x00010574) [ 1] MonotonicCounterRuntimeDxe.dll PC 0x0002387A0174 (0x00023879+0x00010174) [ 1] MonotonicCounterRuntimeDxe.dll PC 0x00023EEA9E38 (0x00023EEA3000+0x6E38) [ 2] DxeCore.dll PC 0x00023EEC3CD4 (0x00023EEA3000+0x00020CD4) [ 2] DxeCore.dll PC 0x00023EEA55D0 (0x00023EEA3000+0x25D0) [ 2] DxeCore.dll PC 0x00023EEA4814 (0x00023EEA3000+0x1814) [ 2] DxeCore.dll PC 0x00023EEA4024 (0x00023EEA3000+0x1024) [ 2] DxeCore.dll [ 0] /root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe/DEBUG/VariableRuntimeDxe.dll [ 1] /root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe/DEBUG/MonotonicCounterRuntimeDxe.dll [ 2] /root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll X0 0x0002389C0060 X1 0x0448 X2 0x001C X3 0x0002349C0018 X4 0x0464 X5 0x0002389C007C X6 0x439A947BAAF32C78 X7 0x9277C34E142E80A1 X8 0x0675D19C X9 0x001B X10 0x000238B6 X11 0x0040 X12 0x X13 0x0008 X14 0x X15 0x X16 0x00023EEA2DF0 X17 0x X18 0x X19 0x000C X20 0x2000 X21 0x X22 0x X23 0x X24 0x X25 0x X26 0x X27 0x X28 0x FP 0x00023EEA29F0 LR 0x000238A79DE8 V0 0x V1 0x V2 0x V3 0x V4 0x V5 0x V6 0x V7 0x V8 0x V9 0x V10 0x V11 0x V12 0x V13 0x V14 0x V15 0x V16 0x V17 0x V18 0x V19 0x V20 0x V21 0x V22 0x V23 0x V24 0x V25 0x V26 0x V27 0x V28 0x V29 0x V30 0x V31 0x SP 0x00023EEA29F0 ELR 0x000238A7A018 SPSR 0x8205 FPSR 0x ESR 0x9621 FAR 0x0454 ESR : EC 0x25 IL 0x1 ISS 0x0021 Data abort: Alignment fault Stack dump: 23EEA28F0: 00023EEA2920 000238B2095C 00023EEA2920 0400 23EEA2910: 007000700403FBB0 008000800400 00023EEA2940 000238B21014 23EEA2930: 00FF00FF0403FBAC 0400 00023EEA2980 000238B222B4 23EEA2950: 00023EEA2980 003E55AA38B22284 0403FBAC 00023BFFF598 23EEA2970: 008000803EEA2BAF 00023EEA2A10 000238B23C8C 23EEA2990: 00023EEA2BAF 00023EEA2AB8 0003FBAE 23EEA29B0: 00023BFFF598 003E55AA 0400 23EEA29D0: 003E00FF 0403FBAC 0004 > 23EEA29F0: 00023EEA2A20 000238A73418 001C 23EEA2A10: 0448 0002389C0060 00023EEA2B1
[edk2] edk2 occur Data abort: Alignment fault when starting VM
-- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] ARM virt machine boots fail with 14 ioh3420
On 2017/4/24 18:16, Marcel Apfelbaum wrote: > On 04/24/2017 01:02 PM, Laszlo Ersek wrote: >> On 04/14/17 04:41, Shannon Zhao wrote: >>> Hi Laszlo, >>> >>> Thanks a lot for your reply:) >>> >>> On 2017/4/14 1:09, Laszlo Ersek wrote: >>>> Adding Andrea, Ard, Drew and Marcel; and the main qemu list >>>> >>>> On 04/13/17 09:37, Shannon Zhao wrote: >>>>> Hi, >>>>> >>>>> I'm testing the PCIe devices hotplug for ARM virt machine and using >>>>> ioh3420 as root port. I found that below command line could work. >>>>> >>>>> qemu-system-aarch64 -machine virt,accel=kvm,usb=off -cpu host -bios >>>>> QEMU_EFI.fd -m 12288 -smp 8,sockets=8,cores=1,threads=1 -device >>>>> ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 -device >>>>> ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 -device >>>>> ioh3420,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 -device >>>>> ioh3420,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 -device >>>>> ioh3420,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 -device >>>>> ioh3420,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 -device >>>>> ioh3420,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x7 -device >>>>> ioh3420,port=0xf,chassis=8,id=pci.8,bus=pcie.0,addr=0x8 -device >>>>> ioh3420,port=0x10,chassis=9,id=pci.9,bus=pcie.0,addr=0x9 -device >>>>> ioh3420,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0xa -device >>>>> ioh3420,port=0x12,chassis=11,id=pci.11,bus=pcie.0,addr=0xb -device >>>>> ioh3420,port=0x13,chassis=12,id=pci.12,bus=pcie.0,addr=0xc -device >>>>> ioh3420,port=0x14,chassis=13,id=pci.13,bus=pcie.0,addr=0xd -device >>>>> i82801b11-bridge,id=pci.17,bus=pcie.0,addr=0x11 -device >>>>> pci-bridge,chassis_nr=18,id=pci.18,bus=pci.17,addr=0x0 -device >>>>> usb-ehci,id=usb,bus=pci.18,addr=0x1 -device >>>>> virtio-scsi-pci,id=scsi0,bus=pci.1,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -drive >>>>> file=/mnt/sdb/guest.raw,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none,aio=native >>>>> >>>>> -device >>>>> scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 >>>>> >>>>> -netdev tap,id=hostnet1,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet1,id=net1,mac=00:16:3e:2b:cc:e1,bus=pci.2,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet2,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet2,id=net2,mac=00:16:3e:22:29:80,bus=pci.3,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet3,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet3,id=net3,mac=00:16:3e:28:07:9a,bus=pci.4,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet4,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet4,id=net4,mac=00:16:3e:3d:cd:b6,bus=pci.5,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet5,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet5,id=net5,mac=00:16:3e:64:9f:b0,bus=pci.6,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet6,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet6,id=net6,mac=00:16:3e:33:5b:d3,bus=pci.7,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet7,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet7,id=net7,mac=00:16:3e:39:7c:df,bus=pci.8,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet8,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet8,id=net8,mac=00:16:3e:0a:c1:4e,bus=pci.9,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet9,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet9,id=net9,mac=00:16:3e:0a:58:a6,bus=pci.10,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet10,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet10,id=net10,mac=00:16:3e:35:b5:80,bus=pci.11,addr=0x0,disable-legacy=on,disable-modern=off >>>>> >>>>> -netdev tap,id=hostnet11,vhost=on -device >>>>> virtio-net-pci,netdev=hostnet11,id=net11,mac=0
Re: [edk2] ARM virt machine boots fail with 14 ioh3420
Hi Laszlo, Thanks a lot for your reply:) On 2017/4/14 1:09, Laszlo Ersek wrote: > Adding Andrea, Ard, Drew and Marcel; and the main qemu list > > On 04/13/17 09:37, Shannon Zhao wrote: >> Hi, >> >> I'm testing the PCIe devices hotplug for ARM virt machine and using >> ioh3420 as root port. I found that below command line could work. >> >> qemu-system-aarch64 -machine virt,accel=kvm,usb=off -cpu host -bios >> QEMU_EFI.fd -m 12288 -smp 8,sockets=8,cores=1,threads=1 -device >> ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 -device >> ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 -device >> ioh3420,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 -device >> ioh3420,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 -device >> ioh3420,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 -device >> ioh3420,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 -device >> ioh3420,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x7 -device >> ioh3420,port=0xf,chassis=8,id=pci.8,bus=pcie.0,addr=0x8 -device >> ioh3420,port=0x10,chassis=9,id=pci.9,bus=pcie.0,addr=0x9 -device >> ioh3420,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0xa -device >> ioh3420,port=0x12,chassis=11,id=pci.11,bus=pcie.0,addr=0xb -device >> ioh3420,port=0x13,chassis=12,id=pci.12,bus=pcie.0,addr=0xc -device >> ioh3420,port=0x14,chassis=13,id=pci.13,bus=pcie.0,addr=0xd -device >> i82801b11-bridge,id=pci.17,bus=pcie.0,addr=0x11 -device >> pci-bridge,chassis_nr=18,id=pci.18,bus=pci.17,addr=0x0 -device >> usb-ehci,id=usb,bus=pci.18,addr=0x1 -device >> virtio-scsi-pci,id=scsi0,bus=pci.1,addr=0x0,disable-legacy=on,disable-modern=off >> -drive >> file=/mnt/sdb/guest.raw,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none,aio=native >> -device >> scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 >> -netdev tap,id=hostnet1,vhost=on -device >> virtio-net-pci,netdev=hostnet1,id=net1,mac=00:16:3e:2b:cc:e1,bus=pci.2,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet2,vhost=on -device >> virtio-net-pci,netdev=hostnet2,id=net2,mac=00:16:3e:22:29:80,bus=pci.3,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet3,vhost=on -device >> virtio-net-pci,netdev=hostnet3,id=net3,mac=00:16:3e:28:07:9a,bus=pci.4,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet4,vhost=on -device >> virtio-net-pci,netdev=hostnet4,id=net4,mac=00:16:3e:3d:cd:b6,bus=pci.5,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet5,vhost=on -device >> virtio-net-pci,netdev=hostnet5,id=net5,mac=00:16:3e:64:9f:b0,bus=pci.6,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet6,vhost=on -device >> virtio-net-pci,netdev=hostnet6,id=net6,mac=00:16:3e:33:5b:d3,bus=pci.7,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet7,vhost=on -device >> virtio-net-pci,netdev=hostnet7,id=net7,mac=00:16:3e:39:7c:df,bus=pci.8,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet8,vhost=on -device >> virtio-net-pci,netdev=hostnet8,id=net8,mac=00:16:3e:0a:c1:4e,bus=pci.9,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet9,vhost=on -device >> virtio-net-pci,netdev=hostnet9,id=net9,mac=00:16:3e:0a:58:a6,bus=pci.10,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet10,vhost=on -device >> virtio-net-pci,netdev=hostnet10,id=net10,mac=00:16:3e:35:b5:80,bus=pci.11,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet11,vhost=on -device >> virtio-net-pci,netdev=hostnet11,id=net11,mac=00:16:3e:4d:b5:bb,bus=pci.12,addr=0x0,disable-legacy=on,disable-modern=off >> -netdev tap,id=hostnet12,vhost=on -device >> virtio-net-pci,netdev=hostnet12,id=net12,mac=00:16:3e:3b:69:e9,bus=pci.13,addr=0x0,disable-legacy=on,disable-modern=off >> -nographic >> >> But if I add one more ioh3420 device by appending above command with >> "-device ioh3420,port=0x15,chassis=14,id=pci.14,bus=pcie.0,addr=0xe", >> the guest can't boot. It seems that the firmware doesn't recognize the >> PCIe devices and print "Connect: PciRoot(0x0): Not Found". >> >> I'm using QEMU 2.8.1 and edk2 at commit 36a0d5c. Is there any limitation >> of the supported PCIe devices? > > In one sentence: you are running out of (emulated) IO space. > > Aarch64 does not have "IO space", but with QEMU, using the "virt" > machine type, we emulate 64KB of IO space, mapped to a special MMIO > range. This is available for PCI resource allocation, for such devices > that have IO BARs (and for such PCI
[edk2] ARM virt machine boots fail with 14 ioh3420
Hi, I'm testing the PCIe devices hotplug for ARM virt machine and using ioh3420 as root port. I found that below command line could work. qemu-system-aarch64 -machine virt,accel=kvm,usb=off -cpu host -bios QEMU_EFI.fd -m 12288 -smp 8,sockets=8,cores=1,threads=1 -device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 -device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 -device ioh3420,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 -device ioh3420,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 -device ioh3420,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 -device ioh3420,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 -device ioh3420,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x7 -device ioh3420,port=0xf,chassis=8,id=pci.8,bus=pcie.0,addr=0x8 -device ioh3420,port=0x10,chassis=9,id=pci.9,bus=pcie.0,addr=0x9 -device ioh3420,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0xa -device ioh3420,port=0x12,chassis=11,id=pci.11,bus=pcie.0,addr=0xb -device ioh3420,port=0x13,chassis=12,id=pci.12,bus=pcie.0,addr=0xc -device ioh3420,port=0x14,chassis=13,id=pci.13,bus=pcie.0,addr=0xd -device i82801b11-bridge,id=pci.17,bus=pcie.0,addr=0x11 -device pci-bridge,chassis_nr=18,id=pci.18,bus=pci.17,addr=0x0 -device usb-ehci,id=usb,bus=pci.18,addr=0x1 -device virtio-scsi-pci,id=scsi0,bus=pci.1,addr=0x0,disable-legacy=on,disable-modern=off -drive file=/mnt/sdb/guest.raw,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none,aio=native -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -netdev tap,id=hostnet1,vhost=on -device virtio-net-pci,netdev=hostnet1,id=net1,mac=00:16:3e:2b:cc:e1,bus=pci.2,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet2,vhost=on -device virtio-net-pci,netdev=hostnet2,id=net2,mac=00:16:3e:22:29:80,bus=pci.3,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet3,vhost=on -device virtio-net-pci,netdev=hostnet3,id=net3,mac=00:16:3e:28:07:9a,bus=pci.4,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet4,vhost=on -device virtio-net-pci,netdev=hostnet4,id=net4,mac=00:16:3e:3d:cd:b6,bus=pci.5,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet5,vhost=on -device virtio-net-pci,netdev=hostnet5,id=net5,mac=00:16:3e:64:9f:b0,bus=pci.6,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet6,vhost=on -device virtio-net-pci,netdev=hostnet6,id=net6,mac=00:16:3e:33:5b:d3,bus=pci.7,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet7,vhost=on -device virtio-net-pci,netdev=hostnet7,id=net7,mac=00:16:3e:39:7c:df,bus=pci.8,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet8,vhost=on -device virtio-net-pci,netdev=hostnet8,id=net8,mac=00:16:3e:0a:c1:4e,bus=pci.9,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet9,vhost=on -device virtio-net-pci,netdev=hostnet9,id=net9,mac=00:16:3e:0a:58:a6,bus=pci.10,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet10,vhost=on -device virtio-net-pci,netdev=hostnet10,id=net10,mac=00:16:3e:35:b5:80,bus=pci.11,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet11,vhost=on -device virtio-net-pci,netdev=hostnet11,id=net11,mac=00:16:3e:4d:b5:bb,bus=pci.12,addr=0x0,disable-legacy=on,disable-modern=off -netdev tap,id=hostnet12,vhost=on -device virtio-net-pci,netdev=hostnet12,id=net12,mac=00:16:3e:3b:69:e9,bus=pci.13,addr=0x0,disable-legacy=on,disable-modern=off -nographic But if I add one more ioh3420 device by appending above command with "-device ioh3420,port=0x15,chassis=14,id=pci.14,bus=pcie.0,addr=0xe", the guest can't boot. It seems that the firmware doesn't recognize the PCIe devices and print "Connect: PciRoot(0x0): Not Found". I'm using QEMU 2.8.1 and edk2 at commit 36a0d5c. Is there any limitation of the supported PCIe devices? Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ArmVirtPkg/HighMemDxe: allow patchable PCD for PcdSystemMemoryBase
On 2016/7/12 21:28, Laszlo Ersek wrote:
> On 07/12/16 15:25, Laszlo Ersek wrote:
>> > On 07/12/16 15:18, Ard Biesheuvel wrote:
>>> >> On 12 July 2016 at 15:07, Laszlo Ersek wrote:
>>> On 07/12/16 15:00, Ard Biesheuvel wrote:
> Redefine the reference to PcdSystemMemoryBase in HighMemDxe.inf as
> a plain [Pcd] rather than [FixedPcd] (and fix up the code as
> appropriate). This allows us to align ArmVirtQemuKernel with
> ArmVirtQemu, given that the former uses a patchable PCD not a fixed
> PCD.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel
> ---
>
> Apologies for the sloppiness on my part, but at least I caught it in
> time :-)
>
> This change is required before we can start using HighMemDxe in
> ArmVirtQemuKernel.
>
> ArmVirtPkg/HighMemDxe/HighMemDxe.c | 2 +-
> ArmVirtPkg/HighMemDxe/HighMemDxe.inf | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> index 4963164fbd8a..7fd7e8e9a539 100644
> --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> @@ -74,7 +74,7 @@ InitializeHighMemDxe (
> CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
> CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
>
> -if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
> +if (PcdGet64 (PcdSystemMemoryBase) != CurBase) {
> Status = gDS->AddMemorySpace (
> EfiGcdMemoryTypeSystemMemory,
> CurBase, CurSize,
> diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> index 2b397626a450..ae632a8bee93 100644
> --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> @@ -45,7 +45,7 @@ [LibraryClasses]
> [Guids]
> gFdtHobGuid
>
> -[FixedPcd]
> +[Pcd]
> gArmTokenSpaceGuid.PcdSystemMemoryBase
>
> [Depex]
>
>>>
>>> Reviewed-by: Laszlo Ersek
>>>
>>> >>
>>> >> Thanks.
>>> >>
>>> You might also want to port this driver to FdtClientProtocol down the
>>> road :)
>>>
>>> >>
>>> >> Indeed, I realized the same when looking at the code. Another problem
>>> >> is that this code assumes that each DT node with device_type=memory
>>> >> describes a single range in its 'reg' property, but in reality, a
>>> >> single node can describe several disjoint regions. As
>>> >> Documentation/devicetree/booting-without-of.txt in the linux repo puts
>>> >> it:
>>> >>
>>> >> """
>>> >> [...]
>>> >> You can either create a single node with all memory ranges in its reg
>>> >> property, or you can create several nodes, as you wish.
>>> >> [...]
>>> >>
>>> >> Required properties:
>>> >>
>>> >> - device_type : has to be "memory"
>>> >> - reg : This property contains all the physical memory ranges of your
>>> >> board. It's a list of addresses/sizes concatenated together, with the
>>> >> number of cells of each defined by the #address-cells and #size-cells
>>> >> of the root node.
>>> >> """
>>> >>
>>> >> The problem is that I am not exactly sure under which circumstances
>>> >> QEMU produces disjoint memory regions, so I have no simple way of
>>> >> testing this.
>> >
>> > The usual method we follow here:
>> > - look at the QEMU code
>> > - write edk2 code accordingly
>> > - ASSERT() that the data from QEMU still looks the way it looked when we
>> > last looked. :)
> We can also ask Shannon about the QEMU behavior (Shannon wrote this
> feature for ArmVirtPkg, 68312710615c). CC'ing him.
When specifying guest NUMA properties, QEMU will generate several memory
nodes.
Refer to below command line which will generate two memory nodes. BTW,
you could use upstream QEMU which already includes guest NUMA feature.
qemu-system-aarch64 \
-enable-kvm -smp 4\
-kernel Image \
-m 512 -machine virt,kernel_irqchip=on \
-initrd guestfs.cpio.gz \
-cpu host -nographic \
-numa node,mem=256M,cpus=0-1,nodeid=0 \
-numa node,mem=256M,cpus=2-3,nodeid=1 \
-append "console=ttyAMA0 root=/dev/ram"
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
From: Shannon Zhao
Add ACPI support for Virt Xen ARM and only for aarch64. It gets the
ACPI tables through Xen ARM multiboot protocol.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
Changes since v2:
* add gFdtClientProtocolGuid to the [Depex]
* make functions static
* move XenAcpiRsdpStructurePtr to InstallXenArmTables ()
* initialize AcpiTable and FdtClient to NULL
Changes since v1:
* move the codes into ArmVirtPkg
* use FdtClient
* don't rely on OvmfPkg/AcpiPlatformDxe/EntryPoint.c while implement own
entry point since it's minor
* use compatible string to find the DT node instead of node path
If you want to test, the corresponding Xen patches can be fetched from:
https://git.linaro.org/people/shannon.zhao/xen.git domu_acpi_v2
---
ArmVirtPkg/ArmVirtXen.dsc | 8 +
ArmVirtPkg/ArmVirtXen.fdf | 8 +
ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c | 244 +
.../XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 50 +
4 files changed, 310 insertions(+)
create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 594ca64..a869986 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -216,3 +216,11 @@
OvmfPkg/XenBusDxe/XenBusDxe.inf
OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+
+ #
+ # ACPI support
+ #
+!if $(ARCH) == AARCH64
+ MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
+!endif
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 13412f9..b1e00e5 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -179,6 +179,14 @@ READ_LOCK_STATUS = TRUE
INF OvmfPkg/XenBusDxe/XenBusDxe.inf
INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+ #
+ # ACPI support
+ #
+!if $(ARCH) == AARCH64
+ INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
+!endif
+
[FV.FVMAIN_COMPACT]
FvAlignment= 16
ERASE_POLARITY = 1
diff --git a/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
new file mode 100644
index 000..c6912ba
--- /dev/null
+++ b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
@@ -0,0 +1,244 @@
+/** @file
+ Xen ARM ACPI Platform Driver using Xen ARM multiboot protocol
+
+ Copyright (C) 2016, Linaro Ltd. All rights reserved.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+/**
+ Get the address of Xen ACPI Root System Description Pointer (RSDP)
+ structure.
+
+ @param RsdpStructurePtr Return pointer to RSDP structure
+
+ @return EFI_SUCCESSFind Xen RSDP structure successfully.
+ @return EFI_NOT_FOUND Don't find Xen RSDP structure.
+ @return EFI_ABORTEDFind Xen RSDP structure, but it's not integrated.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GetXenArmAcpiRsdp (
+ OUT EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER **RsdpPtr
+ )
+{
+ EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *RsdpStructurePtr;
+ EFI_STATUS Status;
+ FDT_CLIENT_PROTOCOL*FdtClient;
+ CONST UINT64 *Reg;
+ UINT32 RegElemSize, RegSize;
+ UINT64 RegBase;
+ UINT8 Sum;
+
+ RsdpStructurePtr = NULL;
+ FdtClient = NULL;
+ //
+ // Get the RSDP structure address from DeviceTree
+ //
+ Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+ (VOID **)&FdtClient);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = FdtClient->FindCompatibleNodeReg (FdtClient, "xen,guest-acpi",
+(CONST VOID **)&Reg, &RegElemSize, &RegSize);
+ if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_WARN, "%a: No 'xen,guest-acpi' compatible DT node found\n",
+ __FUNCTION__));
+return EFI_NOT_FOUND;
+ }
+
+ ASSERT (RegSize == 2 * sizeof (UINT64));
+
+ RegBase = SwapBytes64(Reg[0]);
+ RsdpStructurePtr = (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)RegBase;
+
+ if (RsdpStructurePtr && RsdpStructurePtr->Revision >= 2) {
+Sum = CalculateSum8 ((CONST UINT8 *)Rsdp
Re: [edk2] [PATCH v2] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
On 2016/6/25 11:05, Andrew Fish wrote:
>> > On Jun 24, 2016, at 7:50 PM, Shannon Zhao wrote:
>> >
>> >
>> >
>> > On 2016/6/23 21:42, Ard Biesheuvel wrote:
>>>>> >>>> + //
>>>>> >>>> + // Get the RSDP structure address from DeviceTree
>>>>> >>>> + //
>>>>> >>>> + Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
>>> >> Please add gFdtClientProtocolGuid to the [Depex] section of this module
>>> >>
>> > Hi Ard,
>> >
>> > I try to add gFdtClientProtocolGuid to the [Depex] but I got a compiling
>> > error.
>> >
>> > build.py...
>> > : error F001: Invalid dependency expression: missing operator before {
>> > 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B,
>> > 0x4C } }
>> >Near { 0xFFE06BDD, 0x6107, 0x46A6, { 0x7B, 0xB2, 0x5A, 0x9C,
>> > 0x7E, 0xC5, 0x27, 0x5C }}
>> >
> That is not a common error. Can you attach you INF file? It sounds like a
> syntax error in the [Depex]?
Ah, right. I missed the AND between two elements of [Depex].
Thanks so much :)
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
On 2016/6/23 21:42, Ard Biesheuvel wrote:
>> > + //
>> > + // Get the RSDP structure address from DeviceTree
>> > + //
>> > + Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
> Please add gFdtClientProtocolGuid to the [Depex] section of this module
>
Hi Ard,
I try to add gFdtClientProtocolGuid to the [Depex] but I got a compiling
error.
build.py...
: error F001: Invalid dependency expression: missing operator before {
0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B,
0x4C } }
Near { 0xFFE06BDD, 0x6107, 0x46A6, { 0x7B, 0xB2, 0x5A, 0x9C,
0x7E, 0xC5, 0x27, 0x5C }}
How to deal with this problem?
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
From: Shannon Zhao
Add ACPI support for Virt Xen ARM and only for aarch64. It gets the
ACPI tables through Xen ARM multiboot protocol.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
Changes since v1:
* move the codes into ArmVirtPkg
* use FdtClient
* don't rely on OvmfPkg/AcpiPlatformDxe/EntryPoint.c while implement own
entry point since it's minor
* use compatible string to find the DT node instead of node path
If you want to test, the corresponding Xen patches can be fetched from:
https://git.linaro.org/people/shannon.zhao/xen.git domu_acpi_v2
---
ArmVirtPkg/ArmVirtXen.dsc | 8 +
ArmVirtPkg/ArmVirtXen.fdf | 8 +
ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c | 241 +
.../XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 49 +
4 files changed, 306 insertions(+)
create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 594ca64..a869986 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -216,3 +216,11 @@
OvmfPkg/XenBusDxe/XenBusDxe.inf
OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+
+ #
+ # ACPI support
+ #
+!if $(ARCH) == AARCH64
+ MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
+!endif
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 13412f9..b1e00e5 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -179,6 +179,14 @@ READ_LOCK_STATUS = TRUE
INF OvmfPkg/XenBusDxe/XenBusDxe.inf
INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+ #
+ # ACPI support
+ #
+!if $(ARCH) == AARCH64
+ INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
+!endif
+
[FV.FVMAIN_COMPACT]
FvAlignment= 16
ERASE_POLARITY = 1
diff --git a/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
new file mode 100644
index 000..9258be8
--- /dev/null
+++ b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
@@ -0,0 +1,241 @@
+/** @file
+ Xen ARM ACPI Platform Driver using Xen ARM multiboot protocol
+
+ Copyright (C) 2016, Linaro Ltd. All rights reserved.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *XenAcpiRsdpStructurePtr = NULL;
+
+/**
+ Get the address of Xen ACPI Root System Description Pointer (RSDP)
+ structure.
+
+ @param RsdpStructurePtr Return pointer to RSDP structure
+
+ @return EFI_SUCCESSFind Xen RSDP structure successfully.
+ @return EFI_NOT_FOUND Don't find Xen RSDP structure.
+ @return EFI_ABORTEDFind Xen RSDP structure, but it's not integrated.
+
+**/
+EFI_STATUS
+EFIAPI
+GetXenArmAcpiRsdp (
+ OUT EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER **RsdpPtr
+ )
+{
+ EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *RsdpStructurePtr;
+ EFI_STATUS Status;
+ FDT_CLIENT_PROTOCOL*FdtClient;
+ CONST UINT64 *Reg;
+ UINT32 RegElemSize, RegSize;
+ UINT64 RegBase;
+ UINT8 Sum;
+
+ RsdpStructurePtr = NULL;
+ //
+ // Get the RSDP structure address from DeviceTree
+ //
+ Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+ (VOID **)&FdtClient);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = FdtClient->FindCompatibleNodeReg (FdtClient, "xen,guest-acpi",
+(CONST VOID **)&Reg, &RegElemSize, &RegSize);
+ if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_WARN, "%a: No 'xen,guest-acpi' compatible DT node found\n",
+ __FUNCTION__));
+return EFI_NOT_FOUND;
+ }
+
+ ASSERT (RegSize == 2 * sizeof (UINT64));
+
+ RegBase = SwapBytes64(Reg[0]);
+ RsdpStructurePtr = (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)RegBase;
+
+ if (RsdpStructurePtr && RsdpStructurePtr->Revision >= 2) {
+Sum = CalculateSum8 ((CONST UINT8 *)RsdpStructurePtr,
+sizeof (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER));
+if (Sum != 0) {
+ return EFI_ABORTED;
+}
+
Re: [edk2] [Xen-devel] [PATCH] OvmfPkg: Add ACPI support for Virt Xen ARM
On 2016/6/7 21:50, Julien Grall wrote:
>
> On 31/05/16 05:59, Shannon Zhao wrote:
>> +EFI_STATUS
>> +EFIAPI
>> +GetXenArmAcpiRsdp (
>> + OUT EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER **RsdpPtr
>> + )
>> +{
>> + VOID *Hob;
>> + EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *RsdpStructurePtr;
>> + VOID *DeviceTreeBase;
>> + INT32 Node, Depth, Len;
>> + CONST CHAR8*Type;
>> + CONST VOID *RegProp;
>> +
>> + RsdpStructurePtr = NULL;
>> + //
>> + // Get the RSDP structure address from DeviceTree
>> + //
>> + Hob = GetFirstGuidHob(&gFdtHobGuid);
>> + if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
>> +DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
>> +return EFI_NOT_FOUND;
>> + }
>> + DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
>> +
>> + if (fdt_check_header (DeviceTreeBase) != 0) {
>> +DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__,
>> DeviceTreeBase));
>> +return EFI_NOT_FOUND;
>> + }
>> +
>> + Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");
>
> I am not sure if we want to mandate the modules to live in "/chosen".
> Would it be possible to look by compatible instead?
Sure, I will use the compatible string to find the DT node at next version.
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] OvmfPkg: Add ACPI support for Virt Xen ARM
On 2016/5/31 18:35, Laszlo Ersek wrote: > On 05/31/16 06:59, Shannon Zhao wrote: >> > From: Shannon Zhao >> > >> > Add ACPI support for Virt Xen ARM and it gets the ACPI tables through >> > Xen ARM multiboot protocol. >> > >> > Contributed-under: TianoCore Contribution Agreement 1.0 >> > Signed-off-by: Shannon Zhao >> > --- >> > The corresponding Xen patches can be fetched from: >> > http://git.linaro.org/people/shannon.zhao/xen.git/shortlog/refs/heads/domu_acpi >> > --- >> > ArmVirtPkg/ArmVirtXen.dsc | 6 + >> > ArmVirtPkg/ArmVirtXen.fdf | 6 + >> > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h| 6 + >> > OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c | 207 >> > ++ >> > OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c | 38 >> > OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf | 59 ++ >> > 6 files changed, 322 insertions(+) >> > create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c >> > create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c >> > create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf > Jordan and I might disagree about this patch, but here's my comments > about it: > > I don't think this patch belongs under OvmfPkg. Namely, the only file > that XenArmAcpiPlatformDxe reuses from the existing > OvmfPkg/AcpiPlatformDxe/ directory is "EntryPoint.c", which is (a) > trivial, (b) its conditions and branches should be possible to evaluate > statically for aarch64 Xen. (PcdPciDisableBusEnumeration should be set > to TRUE in ArmVirtXen.dsc.) > > Second, the new code uses the FDT library directly (from EmbeddedPkg), > and accesses QEMU's DTB directly (with the fdt_*() APIs). However, in > ArmVirtPkg we have moved away from this, and now we have a custom > (edk2-only, non-standard) FdtClient protocol for accessing the FDT. > Please see: > - ArmVirtPkg/Include/Protocol/FdtClient.h > - ArmVirtPkg/FdtClientDxe/ > > In order to rebase this patch to the FDT Client Protocol, while keeping > it under OvmfPkg, the "XenArmAcpiPlatformDxe.inf" file would have to > list "ArmVirtPkg/ArmVirtPkg.dec" in the [Packages] section. I think that > would be very ugly. Thus far we have managed to avoid mutual references > between OvmfPkg and ArmVirtPkg: OvmfPkg doesn't consume anything from > ArmVirtPkg, and that's how things should stay in my opinion. > > Which is why I suggest to implement this functionality entirely under > ArmVirtPkg, and using the FDT Client Protocol at that. > Thanks for your explanation. I will move this into ArmVirtPkg. > If a non-trivial chunk of functionality can be identified between > OvmfPkg and ArmVirtPkg in this regard (that currently exists under > OvmfPkg/AcpiPlatformDxe/), then it should be extracted into a library > (under OvmfPkg/Include/Library and OvmfPkg/Library), and the ArmVirtPkg > code should become a client of that library. (You can find many similar > OvmfPkg/Library/ resolutions in the ArmVirtPkg/ DSC files.) > > NB: Ard is going to be on vacation for a while, and I think we should > definitely await his feedback on this. First, my participation in > ArmVirtXen matters is quite minimal. Second, Ard designed the FDT Client > Protocol; in case you need extensions to the current APIs for > implementing the feature, then Ard is the one to review them. I think it will only use the existing FindCompatibleNodeReg() function of FDT Client. So I'll move on next version. Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] OvmfPkg: Add ACPI support for Virt Xen ARM
From: Shannon Zhao
Add ACPI support for Virt Xen ARM and it gets the ACPI tables through
Xen ARM multiboot protocol.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
The corresponding Xen patches can be fetched from:
http://git.linaro.org/people/shannon.zhao/xen.git/shortlog/refs/heads/domu_acpi
---
ArmVirtPkg/ArmVirtXen.dsc | 6 +
ArmVirtPkg/ArmVirtXen.fdf | 6 +
OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h| 6 +
OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c | 207 ++
OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c | 38
OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf | 59 ++
6 files changed, 322 insertions(+)
create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 594ca64..a0d197f 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -216,3 +216,9 @@
OvmfPkg/XenBusDxe/XenBusDxe.inf
OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+
+ #
+ # ACPI Support
+ #
+ MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 13412f9..da30e87 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -179,6 +179,12 @@ READ_LOCK_STATUS = TRUE
INF OvmfPkg/XenBusDxe/XenBusDxe.inf
INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+ #
+ # ACPI Support
+ #
+ INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+ INF OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
+
[FV.FVMAIN_COMPACT]
FvAlignment= 16
ERASE_POLARITY = 1
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
index 08dd7f8..325d7e6 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
@@ -69,6 +69,12 @@ InstallXenTables (
EFI_STATUS
EFIAPI
+InstallXenArmTables (
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
+ );
+
+EFI_STATUS
+EFIAPI
InstallQemuFwCfgTables (
IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
);
diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
new file mode 100644
index 000..c3a351c
--- /dev/null
+++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
@@ -0,0 +1,207 @@
+/** @file
+ OVMF ACPI Xen ARM support
+
+ Copyright (C) 2016, Linaro Ltd. All rights reserved.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "AcpiPlatform.h"
+#include
+#include
+#include
+#include
+#include
+#include
+
+EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *XenAcpiRsdpStructurePtr = NULL;
+
+/**
+ Get the address of Xen ACPI Root System Description Pointer (RSDP)
+ structure.
+
+ @param RsdpStructurePtr Return pointer to RSDP structure
+
+ @return EFI_SUCCESSFind Xen RSDP structure successfully.
+ @return EFI_NOT_FOUND Don't find Xen RSDP structure.
+ @return EFI_ABORTEDFind Xen RSDP structure, but it's not integrated.
+
+**/
+EFI_STATUS
+EFIAPI
+GetXenArmAcpiRsdp (
+ OUT EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER **RsdpPtr
+ )
+{
+ VOID *Hob;
+ EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *RsdpStructurePtr;
+ VOID *DeviceTreeBase;
+ INT32 Node, Depth, Len;
+ CONST CHAR8*Type;
+ CONST VOID *RegProp;
+
+ RsdpStructurePtr = NULL;
+ //
+ // Get the RSDP structure address from DeviceTree
+ //
+ Hob = GetFirstGuidHob(&gFdtHobGuid);
+ if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
+DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
+return EFI_NOT_FOUND;
+ }
+ DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
+
+ if (fdt_check_header (DeviceTreeBase) != 0) {
+DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__,
DeviceTreeBase));
+return EFI_NOT_FOUND;
+ }
+
+ Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");
+ if ( Node < 0 ) {
+DEBUG ((EFI_D_ERROR, "%a: NO /chosen/modules found\n", __FUNCTION__));
+return EFI_NOT_FOUND;
+ }
+
+
+ for (Depth = 0;
+ (Node >= 0) &&a
Re: [edk2] [PATCH] ARM: Virt: Don't generate RTC ACPI node when using UEFI
On 2016/1/12 23:30, Peter Maydell wrote: > On 12 January 2016 at 15:24, Shannon Zhao wrote: >> > When booting VM through UEFI, UEFI takes ownership of the RTC hardware. >> > To DTB UEFI could call libfdt api to disable the RTC device node, but to >> > ACPI it couldn't do that. Therefore, we don't generate the RTC ACPI >> > device in QEMU when using UEFI. > I don't really understand this. I thought that if we were > using ACPI then we would always be doing it via UEFI? > Currently this is true and maybe for a long time this is also true. > Also I think if UEFI wants to take command of some of the > hardware it ought to be UEFI's job to adjust the tables > accordingly before it passes them on to the guest OS. Yes, the ideal method is adjusting the DSDT table in UEFI. But there is almost no way to parse the DSDT table in UEFI. If we want to support that it will introduce ACPI interpreter. This makes it more complex. There is a discussion [1] about this on the edk2 list. [1]https://www.mail-archive.com/edk2-devel@lists.01.org/msg06301.html Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] ArmVirt: About RTC PL031 in ACPI
On 2016/1/8 17:39, Laszlo Ersek wrote:
> On 01/08/16 04:10, Shannon Zhao wrote:
>> >
>> >
>> > On 2016/1/7 22:25, Laszlo Ersek wrote:
>>> >> On 01/07/16 10:31, Shannon Zhao wrote:
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On 2016/1/7 16:16, Ard Biesheuvel wrote:
>>>>>>> >>>>>> On 7 January 2016 at 03:47, Shannon Zhao
>>>>>>> >>>>>> wrote:
>>>>>>>>>>> >>>>>>>>>> Hi,
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> I notice that when booting with DTS UEFI will disable
>>>>>>>>>>> >>>>>>>>>> RTC device PL031
>>>>>>>>>>> >>>>>>>>>> in the DTS by following codes. And it turns out that
>>>>>>>>>>> >>>>>>>>>> only rtc-efi shows
>>>>>>>>>>> >>>>>>>>>> up in guest.
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> //
>>>>>>>>>>> >>>>>>>>>> // UEFI takes ownership of the RTC hardware, and exposes
>>>>>>>>>>> >>>>>>>>>> its functionality
>>>>>>>>>>> >>>>>>>>>> // through the UEFI Runtime Services GetTime, SetTime,
>>>>>>>>>>> >>>>>>>>>> etc. This means we
>>>>>>>>>>> >>>>>>>>>> // need to disable it in the device tree to prevent the
>>>>>>>>>>> >>>>>>>>>> OS from
>>>>>>>>>>> >>>>>>>>>> attaching its
>>>>>>>>>>> >>>>>>>>>> // device driver as well.
>>>>>>>>>>> >>>>>>>>>> //
>>>>>>>>>>> >>>>>>>>>> if ((RtcNode != -1) &&
>>>>>>>>>>> >>>>>>>>>> fdt_setprop_string (DeviceTreeBase, RtcNode,
>>>>>>>>>>> >>>>>>>>>> "status",
>>>>>>>>>>> >>>>>>>>>> "disabled") != 0) {
>>>>>>>>>>> >>>>>>>>>> DEBUG ((EFI_D_WARN, "Failed to set PL031 status to
>>>>>>>>>>> >>>>>>>>>> 'disabled'\n"));
>>>>>>>>>>> >>>>>>>>>> }
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> But when booting with ACPI, there are two RTC devices,
>>>>>>>>>>> >>>>>>>>>> rtc-efi and
>>>>>>>>>>> >>>>>>>>>> PL031(PL031 shows up when kenrel PL031 driver adds
>>>>>>>>>>> >>>>>>>>>> support to probe it
>>>>>>>>>>> >>>>>>>>>> via ACPI). And I didn't see any codes in UEFI to handle
>>>>>>>>>>> >>>>>>>>>> the RTC node in
>>>>>>>>>>> >>>>>>>>>> ACPI table.
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> I think it's hard to modify the DSDT table in UEFI since
>>>>>>>>>>> >>>>>>>>>> there is not a
>>>>>>>>>>> >>>>>>>>>> ACPI lib like libfdt. But for consistency, does it need
>>>>>>>>>>> >>>>>>>>>> to handle it too
>>>>>>>>>>> >>>>>>>>>> when booting with ACPI?
>>>>>>>>>>
Re: [edk2] ArmVirt: About RTC PL031 in ACPI
On 2016/1/7 22:25, Laszlo Ersek wrote:
> On 01/07/16 10:31, Shannon Zhao wrote:
>> >
>> >
>> > On 2016/1/7 16:16, Ard Biesheuvel wrote:
>>> >> On 7 January 2016 at 03:47, Shannon Zhao
>>> >> wrote:
>>>>> >>>> Hi,
>>>>> >>>>
>>>>> >>>> I notice that when booting with DTS UEFI will disable RTC device
>>>>> >>>> PL031
>>>>> >>>> in the DTS by following codes. And it turns out that only rtc-efi
>>>>> >>>> shows
>>>>> >>>> up in guest.
>>>>> >>>>
>>>>> >>>> //
>>>>> >>>> // UEFI takes ownership of the RTC hardware, and exposes its
>>>>> >>>> functionality
>>>>> >>>> // through the UEFI Runtime Services GetTime, SetTime, etc. This
>>>>> >>>> means we
>>>>> >>>> // need to disable it in the device tree to prevent the OS from
>>>>> >>>> attaching its
>>>>> >>>> // device driver as well.
>>>>> >>>> //
>>>>> >>>> if ((RtcNode != -1) &&
>>>>> >>>> fdt_setprop_string (DeviceTreeBase, RtcNode, "status",
>>>>> >>>> "disabled") != 0) {
>>>>> >>>> DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
>>>>> >>>> }
>>>>> >>>>
>>>>> >>>> But when booting with ACPI, there are two RTC devices, rtc-efi and
>>>>> >>>> PL031(PL031 shows up when kenrel PL031 driver adds support to probe
>>>>> >>>> it
>>>>> >>>> via ACPI). And I didn't see any codes in UEFI to handle the RTC node
>>>>> >>>> in
>>>>> >>>> ACPI table.
>>>>> >>>>
>>>>> >>>> I think it's hard to modify the DSDT table in UEFI since there is
>>>>> >>>> not a
>>>>> >>>> ACPI lib like libfdt. But for consistency, does it need to handle it
>>>>> >>>> too
>>>>> >>>> when booting with ACPI?
>>>>> >>>>
>>> >> Yes, it should. I didn't spot this before, or I would have said
>>> >> something.
>>> >>
>>> >> As long as the firmware is driving the RTC, the OS should not be able
>>> >> to attach its driver directly, ACPI or DT alike.
>> >
>> > Is there a way to parse DSDT in UEFI or other ways we could use to mask
>> > the RTC device? I think maybe we could use the STAO table or something
>> > like it which is added by ACPI 6.0.
> Let's see what ways there are for booting an ARM / AARCH64 guest
> ("virt") machine type:
>
> (1) QEMU's builtin (minimal) firmware, and nothing else. There is
> nothing to *consume* ACPI.
>
> (2) QEMU's builtin (minimal) firmware, and a directly booted kernel
> (-kernel option). The kernel gets only a DTB -- there is no
> architecturally defined way to expose ACPI to the kernel.
>
> (3) explicit firmware (-bios or -pflash option), and whatever gets
> booted by the firmware. Firmware here means UEFI, period. The guest OS
> gets both DTB and ACPI (unless disabled by -no-acpi).
>
> (4) explicit firmware (-bios or -pflash opton) plus an immediately
> booted fw_cfg kernel (i.e. -kernel option as well). Firmware again means
> UEFI, the guest OS gets again both DTB and ACPI (unless disabled by
> -no-acpi).
>
> So here's what I suggest:
>
> - modify QEMU to drop the RTC device specification from *both* the DTB
> and the ACPI generator *if* an explicit firmware is passed (with -bios
> or -pflash). Because this means UEFI, and UEFI will take control of the
> RTC. Cases (1) and (2) are unaffected, and cases (3) and (4) are handled
> correctly.
>
> - modify ArmVirtPkg to remove the above quoted disabling -- QEMU should
> handle it for the DTB as well.
>
> See QEMU commit 07abe45c4814, and the "arm_boot_info.firmware_loaded"
> field -- that could be used to control the DTB and ACPI generators.
I'm a bit worried that this way is a little UEFI specific. If there is
another firmware in the future which has a different way to handle RTC,
this will not work. Or I'm over worried?
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] ArmVirt: About RTC PL031 in ACPI
On 2016/1/7 16:16, Ard Biesheuvel wrote:
> On 7 January 2016 at 03:47, Shannon Zhao wrote:
>> > Hi,
>> >
>> > I notice that when booting with DTS UEFI will disable RTC device PL031
>> > in the DTS by following codes. And it turns out that only rtc-efi shows
>> > up in guest.
>> >
>> > //
>> > // UEFI takes ownership of the RTC hardware, and exposes its functionality
>> > // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
>> > // need to disable it in the device tree to prevent the OS from
>> > attaching its
>> > // device driver as well.
>> > //
>> > if ((RtcNode != -1) &&
>> > fdt_setprop_string (DeviceTreeBase, RtcNode, "status",
>> > "disabled") != 0) {
>> > DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
>> > }
>> >
>> > But when booting with ACPI, there are two RTC devices, rtc-efi and
>> > PL031(PL031 shows up when kenrel PL031 driver adds support to probe it
>> > via ACPI). And I didn't see any codes in UEFI to handle the RTC node in
>> > ACPI table.
>> >
>> > I think it's hard to modify the DSDT table in UEFI since there is not a
>> > ACPI lib like libfdt. But for consistency, does it need to handle it too
>> > when booting with ACPI?
>> >
> Yes, it should. I didn't spot this before, or I would have said something.
>
> As long as the firmware is driving the RTC, the OS should not be able
> to attach its driver directly, ACPI or DT alike.
Is there a way to parse DSDT in UEFI or other ways we could use to mask
the RTC device? I think maybe we could use the STAO table or something
like it which is added by ACPI 6.0.
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] ArmVirt: About RTC PL031 in ACPI
Hi,
I notice that when booting with DTS UEFI will disable RTC device PL031
in the DTS by following codes. And it turns out that only rtc-efi shows
up in guest.
//
// UEFI takes ownership of the RTC hardware, and exposes its functionality
// through the UEFI Runtime Services GetTime, SetTime, etc. This means we
// need to disable it in the device tree to prevent the OS from
attaching its
// device driver as well.
//
if ((RtcNode != -1) &&
fdt_setprop_string (DeviceTreeBase, RtcNode, "status",
"disabled") != 0) {
DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
}
But when booting with ACPI, there are two RTC devices, rtc-efi and
PL031(PL031 shows up when kenrel PL031 driver adds support to probe it
via ACPI). And I didn't see any codes in UEFI to handle the RTC node in
ACPI table.
I think it's hard to modify the DSDT table in UEFI since there is not a
ACPI lib like libfdt. But for consistency, does it need to handle it too
when booting with ACPI?
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 2/2] ArmVirtPkg: Add memory space for the high memory nodes
From: Shannon Zhao
Here we add the memory space for the high memory nodes except the lowest
one in FDT. So these spaces will show up in the UEFI memory map.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
ArmVirtPkg/ArmVirtQemu.dsc | 1 +
ArmVirtPkg/ArmVirtQemu.fdf | 1 +
ArmVirtPkg/HighMemDxe/HighMemDxe.c | 105 +++
ArmVirtPkg/HighMemDxe/HighMemDxe.inf | 51 +
4 files changed, 158 insertions(+)
create mode 100644 ArmVirtPkg/HighMemDxe/HighMemDxe.c
create mode 100644 ArmVirtPkg/HighMemDxe/HighMemDxe.inf
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index b0d1d08..e6440ec 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -302,6 +302,7 @@
# Platform Driver
#
ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+ ArmVirtPkg/HighMemDxe/HighMemDxe.inf
OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
OvmfPkg/VirtioNetDxe/VirtioNet.inf
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index 738e3db..f5e6cbd 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -108,6 +108,7 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Core/Dxe/DxeMain.inf
INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+ INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
#
# PI DXE Drivers producing Architectural Protocols (EFI Services)
diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c
b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
new file mode 100644
index 000..08b010e
--- /dev/null
+++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
@@ -0,0 +1,105 @@
+/** @file
+* High memory node enumeration DXE driver for ARM Virtual Machines
+*
+* Copyright (c) 2015, Linaro Ltd. All rights reserved.
+*
+* This program and the accompanying materials are
+* licensed and made available under the terms and conditions of the BSD
License
+* which accompanies this distribution. The full text of the license may be
found at
+* http://opensource.org/licenses/bsd-license.php
+*
+* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
IMPLIED.
+*
+**/
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+EFI_STATUS
+EFIAPI
+InitializeHighMemDxe (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ VOID *Hob;
+ VOID *DeviceTreeBase;
+ INT32Node, Prev;
+ EFI_STATUS Status;
+ CONST CHAR8 *Type;
+ INT32Len;
+ CONST VOID *RegProp;
+ UINT64 CurBase;
+ UINT64 CurSize;
+
+ Hob = GetFirstGuidHob(&gFdtHobGuid);
+ if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
+return EFI_NOT_FOUND;
+ }
+ DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
+
+ if (fdt_check_header (DeviceTreeBase) != 0) {
+DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__,
DeviceTreeBase));
+return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
+
+ //
+ // Check for memory node and add the memory spaces expect the lowest one
+ //
+ for (Prev = 0;; Prev = Node) {
+Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+if (Node < 0) {
+ break;
+}
+
+Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
+ //
+ // Get the 'reg' property of this node. For now, we will assume
+ // two 8 byte quantities for base and size, respectively.
+ //
+ RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+ if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
+
+CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
+CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
+
+if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeSystemMemory,
+ CurBase, CurSize,
+ EFI_MEMORY_WB | EFI_MEMORY_WC |
+ EFI_MEMORY_WT | EFI_MEMORY_UC);
+
+ if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "%a: Failed to add System RAM @ 0x%lx -
0x%lx\n",
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+continue;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ CurBase, CurSize,
+ EFI_MEMORY_WB);
+
+ if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "%a: Failed to set System RAM @ 0x%lx - 0x%lx
attribute\n",
+ __FUNCTION__, CurBase, CurBase + CurSize
[edk2] [PATCH v3 1/2] ArmVirtPkg: Find the lowest memory node
From: Shannon Zhao
While QEMU NUMA support on ARM will introduce more than one /memory node
in the device tree, it needs to find the lowest one and set
PcdSystemMemorySize with the actual size of this memory node.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
Reviewed-by: Laszlo Ersek
---
ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
index 17f2686..7a0fc0e 100644
--- a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
+++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
@@ -72,8 +72,8 @@ ArmPlatformInitializeSystemMemory (
{
VOID *DeviceTreeBase;
INT32Node, Prev;
- UINT64 NewBase;
- UINT64 NewSize;
+ UINT64 NewBase, CurBase;
+ UINT64 NewSize, CurSize;
CONST CHAR8 *Type;
INT32Len;
CONST UINT64 *RegProp;
@@ -90,7 +90,7 @@ ArmPlatformInitializeSystemMemory (
ASSERT (fdt_check_header (DeviceTreeBase) == 0);
//
- // Look for a memory node
+ // Look for the lowest memory node
//
for (Prev = 0;; Prev = Node) {
Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
@@ -110,26 +110,30 @@ ArmPlatformInitializeSystemMemory (
RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
-NewBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
-NewSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
-
-//
-// Make sure the start of DRAM matches our expectation
-//
-ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
-PcdSet64 (PcdSystemMemorySize, NewSize);
+CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
DEBUG ((EFI_D_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
- __FUNCTION__, NewBase, NewBase + NewSize - 1));
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+
+if (NewBase > CurBase || NewBase == 0) {
+ NewBase = CurBase;
+ NewSize = CurSize;
+}
} else {
DEBUG ((EFI_D_ERROR, "%a: Failed to parse FDT memory node\n",
__FUNCTION__));
}
- break;
}
}
//
+ // Make sure the start of DRAM matches our expectation
+ //
+ ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
+ PcdSet64 (PcdSystemMemorySize, NewSize);
+
+ //
// We need to make sure that the machine we are running on has at least
// 128 MB of memory configured, and is currently executing this binary from
// NOR flash. This prevents a device tree image in DRAM from getting
--
2.0.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/2] ArmVirtPkg: Add support for multiple memory nodes
From: Shannon Zhao If there are more than one memory nodes in FDT, currently UEFI will assert. These two patches firstly let UEFI find the memory node with lowest base address and check if the address is what we expected and set PcdSystemMemorySize as the size of this node. Then add other memory spaces through gDS->AddMemorySpace() when it parses FDT. Changes since v2: * Move the new added function to new driver * call gDS->SetMemorySpaceAttributes to set the attribute Changes since v1: * Fix commit message and add reviewed-by tag from Laszlo (PATCH 1/2) * Factor the codes out as a new function, fix coding style (PATCH 2/2) Shannon Zhao (2): ArmVirtPkg: Find the lowest memory node ArmVirtPkg: Add memory space for the high memory nodes ArmVirtPkg/ArmVirtQemu.dsc | 1 + ArmVirtPkg/ArmVirtQemu.fdf | 1 + ArmVirtPkg/HighMemDxe/HighMemDxe.c | 105 +++ ArmVirtPkg/HighMemDxe/HighMemDxe.inf | 51 + ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30 5 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 ArmVirtPkg/HighMemDxe/HighMemDxe.c create mode 100644 ArmVirtPkg/HighMemDxe/HighMemDxe.inf -- 2.0.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
On 2015/12/3 21:25, Laszlo Ersek wrote: > Shannon, > > On 12/01/15 03:02, Shannon Zhao wrote: >> > >> > On 2015/12/1 2:12, Laszlo Ersek wrote: >>> >> Thank you -- this looks better. Unfortunately the >>> >> SetMemorySpaceAttributes() question still has to be addressed, and we're >>> >> still discussing with Ard which way to go about that. Once we have an >>> >> agreement, I'll spell out the further requirements for this. >>> >> >>> >> The "do it all in PEI with HOBs" idea is out, now we're trying to decide >>> >> if (a) we should do it in VirtFdtDxe, and update the APRIORI DXE block >>> >> in the FDF file so that CpuDxe gets dispatched before VirtFdtDxe, or (b) >>> >> if we should move this code to a separate DXE driver that depends on the >>> >> CPU architectural protocol installed by CpuDxe. Whatever the agreement, >>> >> this patch will need to be further updated. >>> >> >>> >> I ask for your patience with this -- it's messier than I would like. I >>> >> hope your NUMA development can proceed even until we converge with the >>> >> help of these WIP patches. >>> >> >> > It's fine since it doesn't block the NUMA patches. :) >> > >>> >> I'll admit that my secret (well, not so secret :)) agenda with this >>> >> series is to pull more people into edk2 development from the QEMU >>> >> community. Albeit somewhat messy, this feature isn't very large or >>> >> complex, so I find it appropriate to "train" you with it -- if you want >>> >> to play along, that is. :) >> > >> > Laszlo, I think this is very cool. I'm interested in this kind of >> > "training". :) >> > > Okay, so we seem to have reached an agreement with Ard in the other thread: > > http://thread.gmane.org/gmane.comp.bios.edk2.devel/5054/focus=5309 > > Here's what I suggest for v3: > > * Patch 1 is good, you can include it without any changes (you have my R-b). > > * Patch 2 should implement what you are doing now (with my comments in > this thread addressed), but it should introduce that functionality as a > separate, new driver. > > Namely, as first step please recursively copy the directory > "ArmVirtPkg/VirtFdtDxe" to "ArmVirtPkg/HighMemDxe". Please also replace > all occurrences of the "VirtFdtDxe" string in source code and in file > names within the copy. > > Second, please generate a new FILE_GUID with the "uuidgen" utility for > the new "ArmVirtPkg/HighMemDxe/HighMemDxe.inf" file. > > Third, add the subject functionality to HighMemDxe. (Addressing my > comments too.) > > Fourth, remove everything else from HighMemDxe; so that its sole > responsibility remains processing the higher-than-lowest /memory nodes > in the DTB. This removal will make a number of libraries, PCDs, protcols > etc superfluous, so please remove the relevant #include directives and > [LibraryClasses] entries. > > Fifth, in "ArmVirtPkg/HighMemDxe/HighMemDxe.inf", the TRUE expression > under [DepEx] should be replaced with gEfiCpuArchProtocolGuid. > > Sixth, the new driver's INF file should be referenced in > "ArmVirtQemu.dsc" and "ArmVirtQemu.fdf". In the latter, it should *not* > be added to APRIORI DXE. > > In short, patch #2 should add a small new driver that does nothing but > looks for memory nodes in the DTB, installs the ranges, and sets their > actual caching attributes to WB (which depends on the CPU arch protocol > internally). Ok, will update this patch. :) Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
On 2015/12/1 2:12, Laszlo Ersek wrote: > Thank you -- this looks better. Unfortunately the > SetMemorySpaceAttributes() question still has to be addressed, and we're > still discussing with Ard which way to go about that. Once we have an > agreement, I'll spell out the further requirements for this. > > The "do it all in PEI with HOBs" idea is out, now we're trying to decide > if (a) we should do it in VirtFdtDxe, and update the APRIORI DXE block > in the FDF file so that CpuDxe gets dispatched before VirtFdtDxe, or (b) > if we should move this code to a separate DXE driver that depends on the > CPU architectural protocol installed by CpuDxe. Whatever the agreement, > this patch will need to be further updated. > > I ask for your patience with this -- it's messier than I would like. I > hope your NUMA development can proceed even until we converge with the > help of these WIP patches. > It's fine since it doesn't block the NUMA patches. :) > I'll admit that my secret (well, not so secret :)) agenda with this > series is to pull more people into edk2 development from the QEMU > community. Albeit somewhat messy, this feature isn't very large or > complex, so I find it appropriate to "train" you with it -- if you want > to play along, that is. :) Laszlo, I think this is very cool. I'm interested in this kind of "training". :) Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
From: Shannon Zhao
Here we add the memory space for the memory nodes except the lowest one
in FDT. So these spaces will show up in the UEFI memory map.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 72
ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4 ++
2 files changed, 76 insertions(+)
diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
index 74f80d1..11a9b5a 100644
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
+++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -274,6 +275,73 @@ ProcessPciHost (
return EFI_SUCCESS;
}
+/**
+ Process the device tree node whose "device_type" property is "memory" and add
+the memory range of this node to System RAM space.
+
+ param[in] DeviceTreeBase Pointer to the device tree.
+
+ param[in] NodeOffset of the device tree node.
+
+ @retval TRUE The "device_type" property of this device tree node
+ is "memory" either adding successfully or
+ unsuccessfully.
+
+ @retval FALSE The "device_type" property of this device tree node
+ is not "memory"
+**/
+STATIC
+BOOLEAN
+EFIAPI
+AddMemorySpaceToSystemRam (
+ IN CONST VOID *DeviceTreeBase,
+ IN INT32 Node
+ )
+{
+ CONST CHAR8 *Type;
+ INT32Len;
+ CONST VOID *RegProp;
+ EFI_STATUS Status;
+ UINT64 CurBase;
+ UINT64 CurSize;
+
+ //
+ // Check for memory node and add the memory spaces expect the lowest one
+ //
+ Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+ if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
+//
+// Get the 'reg' property of this node. For now, we will assume
+// two 8 byte quantities for base and size, respectively.
+//
+RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
+
+ CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
+ CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
+
+ if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
+Status = gDS->AddMemorySpace (
+EfiGcdMemoryTypeSystemMemory,
+CurBase, CurSize,
+EFI_MEMORY_WB | EFI_MEMORY_WC |
+EFI_MEMORY_WT | EFI_MEMORY_UC);
+
+if (EFI_ERROR (Status)) {
+ DEBUG ((EFI_D_ERROR, "%a: Failed to add System RAM @ 0x%lx -
0x%lx\n",
+__FUNCTION__, CurBase, CurBase + CurSize - 1));
+} else {
+ DEBUG ((EFI_D_INFO, "%a: Add System RAM @ 0x%lx - 0x%lx\n",
+__FUNCTION__, CurBase, CurBase + CurSize - 1));
+}
+ }
+}
+
+return TRUE;
+ }
+
+ return FALSE;
+}
EFI_STATUS
EFIAPI
@@ -332,6 +400,10 @@ InitializeVirtFdtDxe (
break;
}
+if (AddMemorySpaceToSystemRam (DeviceTreeBase, Node)) {
+ continue;
+}
+
Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
if (Type == NULL) {
continue;
diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
index ee2503a..0e6927b 100644
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
@@ -43,12 +43,16 @@
VirtioMmioDeviceLib
HobLib
XenIoMmioLib
+ DxeServicesTableLib
[Guids]
gFdtTableGuid
gVirtioMmioTransportGuid
gFdtHobGuid
+[FixedPcd]
+ gArmTokenSpaceGuid.PcdSystemMemoryBase
+
[Pcd]
gArmVirtTokenSpaceGuid.PcdArmPsciMethod
gArmVirtTokenSpaceGuid.PcdFwCfgSelectorAddress
--
2.0.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] ArmVirtPkg: Find the lowest memory node
From: Shannon Zhao
While QEMU NUMA support on ARM will introduce more than one /memory node
in the device tree, it needs to find the lowest one and set
PcdSystemMemorySize with the actual size of this memory node.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
Reviewed-by: Laszlo Ersek
---
ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
index 17f2686..7a0fc0e 100644
--- a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
+++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
@@ -72,8 +72,8 @@ ArmPlatformInitializeSystemMemory (
{
VOID *DeviceTreeBase;
INT32Node, Prev;
- UINT64 NewBase;
- UINT64 NewSize;
+ UINT64 NewBase, CurBase;
+ UINT64 NewSize, CurSize;
CONST CHAR8 *Type;
INT32Len;
CONST UINT64 *RegProp;
@@ -90,7 +90,7 @@ ArmPlatformInitializeSystemMemory (
ASSERT (fdt_check_header (DeviceTreeBase) == 0);
//
- // Look for a memory node
+ // Look for the lowest memory node
//
for (Prev = 0;; Prev = Node) {
Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
@@ -110,26 +110,30 @@ ArmPlatformInitializeSystemMemory (
RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
-NewBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
-NewSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
-
-//
-// Make sure the start of DRAM matches our expectation
-//
-ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
-PcdSet64 (PcdSystemMemorySize, NewSize);
+CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
DEBUG ((EFI_D_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
- __FUNCTION__, NewBase, NewBase + NewSize - 1));
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+
+if (NewBase > CurBase || NewBase == 0) {
+ NewBase = CurBase;
+ NewSize = CurSize;
+}
} else {
DEBUG ((EFI_D_ERROR, "%a: Failed to parse FDT memory node\n",
__FUNCTION__));
}
- break;
}
}
//
+ // Make sure the start of DRAM matches our expectation
+ //
+ ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
+ PcdSet64 (PcdSystemMemorySize, NewSize);
+
+ //
// We need to make sure that the machine we are running on has at least
// 128 MB of memory configured, and is currently executing this binary from
// NOR flash. This prevents a device tree image in DRAM from getting
--
2.0.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/2] ArmVirtPkg: Add support for multiple memory nodes
From: Shannon Zhao If there are more than one memory nodes in FDT, currently UEFI will assert. These two patches firstly let UEFI find the memory node with lowest base address and check if the address is what we expected and set PcdSystemMemorySize as the size of this node. Then add other memory spaces through gDS->AddMemorySpace() when it parses FDT. Changes since v1: * Fix commit message and add reviewed-by tag from Laszlo (PATCH 1/2) * Factor the codes out as a new function, fix coding style (PATCH 2/2) Shannon Zhao (2): ArmVirtPkg: Find the lowest memory node ArmVirtPkg: Add memory space for the memory nodes except the lowest one ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30 +++- ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 72 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4 ++ 3 files changed, 93 insertions(+), 13 deletions(-) -- 2.0.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
On 2015/11/30 18:01, Laszlo Ersek wrote:
> On 11/30/15 10:50, Laszlo Ersek wrote:
>> On 11/30/15 10:28, Ard Biesheuvel wrote:
>>> On 30 November 2015 at 10:22, Shannon Zhao wrote:
>>>>
>>>>
>>>> On 2015/11/30 16:53, Laszlo Ersek wrote:
>>>>> On 11/29/15 07:31, Shannon Zhao wrote:
>>>>>> From: Shannon Zhao
>>>>>>
>>>>>> Here we add the memory space for the memory nodes except the lowest one
>>>>>> in FDT. So these spaces will show up in the UEFI memory map.
>>>>>>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>> Signed-off-by: Shannon Zhao
>>>>>> ---
>>>>>> ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 34
>>>>>> ++
>>>>>> ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4
>>>>>> 2 files changed, 38 insertions(+)
>>>>>>
>>>>>> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>>>>>> b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>>>>>> index 74f80d1..2c8d785 100644
>>>>>> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>>>>>> +++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>>>>>> @@ -27,6 +27,7 @@
>>>>>> #include
>>>>>> #include
>>>>>> #include
>>>>>> +#include
>>>>>>
>>>>>> #include
>>>>>> #include
>>>>>> @@ -304,6 +305,8 @@ InitializeVirtFdtDxe (
>>>>>>UINT64 FwCfgDataSize;
>>>>>>UINT64 FwCfgDmaAddress;
>>>>>>UINT64 FwCfgDmaSize;
>>>>>> + UINT64 CurBase;
>>>>>> + UINT64 CurSize;
>>>>>>
>>>>>>Hob = GetFirstGuidHob(&gFdtHobGuid);
>>>>>>if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
>>>>>> @@ -332,6 +335,37 @@ InitializeVirtFdtDxe (
>>>>>>break;
>>>>>> }
>>>>>>
>>>>>> +//
>>>>>> +// Check for memory node and add the memory spaces expect the
>>>>>> lowest one
>>>>>> +//
>>>>>
>>>>> (1) Based on the DTB node, which looks like:
>>>>>
>>>>> memory {
>>>>> reg = <0x0 0x4000 0x0 0x800>;
>>>>> device_type = "memory";
>>>>> };
>>>>>
>>>>> I agree that checking it here, before we look at "compatible", is a good
>>>>> thing.
>>>>>
>>>>> However, can you please factor this out into a separate function? (Just
>>>>> within this file, as a STATIC function.)
>>>>>
>>>>> I propose that the function return a BOOLEAN:
>>>>> - TRUE if the node was a match (either successfully or unsuccessfully);
>>>>> in which case you can "continue;" with the next iteration directly
>>>>> - FALSE, if there is no match (i.e., the current iteration must
>>>>> proceed, in order to look for different device types)
>>>>>
>>>>
>>>> Sure.
>>>>
>>>>>> +Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
>>>>>> +if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
>>>>>> + //
>>>>>> + // Get the 'reg' property of this node. For now, we will assume
>>>>>> + // two 8 byte quantities for base and size, respectively.
>>>>>> + //
>>>>>> + RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
>>>>>> + if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
>>>>>> +
>>>>>> +CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
>>>>>> +CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
>>>>>> +
>>>>>> +if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
>>>>>> + Status = gDS->AddMemorySpace(EfiGcdMemoryTypeSystemMemory,
>>>>>> +
Re: [edk2] [PATCH 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
On 2015/11/30 16:53, Laszlo Ersek wrote:
> On 11/29/15 07:31, Shannon Zhao wrote:
>> From: Shannon Zhao
>>
>> Here we add the memory space for the memory nodes except the lowest one
>> in FDT. So these spaces will show up in the UEFI memory map.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Shannon Zhao
>> ---
>> ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 34 ++
>> ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4
>> 2 files changed, 38 insertions(+)
>>
>> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>> b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>> index 74f80d1..2c8d785 100644
>> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>> +++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -304,6 +305,8 @@ InitializeVirtFdtDxe (
>>UINT64 FwCfgDataSize;
>>UINT64 FwCfgDmaAddress;
>>UINT64 FwCfgDmaSize;
>> + UINT64 CurBase;
>> + UINT64 CurSize;
>>
>>Hob = GetFirstGuidHob(&gFdtHobGuid);
>>if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
>> @@ -332,6 +335,37 @@ InitializeVirtFdtDxe (
>>break;
>> }
>>
>> +//
>> +// Check for memory node and add the memory spaces expect the lowest one
>> +//
>
> (1) Based on the DTB node, which looks like:
>
> memory {
> reg = <0x0 0x4000 0x0 0x800>;
> device_type = "memory";
> };
>
> I agree that checking it here, before we look at "compatible", is a good
> thing.
>
> However, can you please factor this out into a separate function? (Just
> within this file, as a STATIC function.)
>
> I propose that the function return a BOOLEAN:
> - TRUE if the node was a match (either successfully or unsuccessfully);
> in which case you can "continue;" with the next iteration directly
> - FALSE, if there is no match (i.e., the current iteration must
> proceed, in order to look for different device types)
>
Sure.
>> +Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
>> +if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
>> + //
>> + // Get the 'reg' property of this node. For now, we will assume
>> + // two 8 byte quantities for base and size, respectively.
>> + //
>> + RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
>> + if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
>> +
>> +CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
>> +CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
>> +
>> +if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
>> + Status = gDS->AddMemorySpace(EfiGcdMemoryTypeSystemMemory,
>> + CurBase, CurSize,
>> + EFI_MEMORY_WB | EFI_MEMORY_RUNTIME);
>
> (1) The edk2 coding style requires a space between the function name (or
> function pointer) and the opening paren.
>
> (2) Regarding the indentation of function arguments, they are aligned
> with the first or (more usually:) second character of the function or
> *member* name. In this case, they should be aligned with the second "d"
> in "AddMemorySpace".
>
Thanks for explain the edk2 coding style. Sorry, I'm not familiar with this.
> (3) The last argument is a bitmask of capabilities. As far as I
> remember, all system memory regions I've seen dumped by Linux from the
> UEFI memmap at startup had all of: UC | WC | WT | WB.
>
> I agree that RUNTIME and WB should be listed here, but do you have any
> particular reason for not allowing UC|WC|WT?
>
> (4) Related question -- when you boot the guest kernel with this patch
> in the firmware, and pass "efi=debug" to Linux, do the memory attributes
> of the additional NUMA nodes look identically to the "base" memory? (I'm
> asking this to see if we need additional gDS->SetMemorySpaceAttributes()
> calls.)
>
> Can you paste a sample output from "efi=debug"?
>
Processing EFI memory map:
0x0400-0x07ff [Memory Mapped I/O |RUN| |XP| | | |
| | | |UC]
0x0901-0x09010fff [Memory Mapped I/O |RUN| |XP| | | |
| | |
Re: [edk2] [PATCH 1/2] ArmVirtPkg: Find the lowest memory node
On 2015/11/30 14:24, Ard Biesheuvel wrote:
> On 29 November 2015 at 07:31, Shannon Zhao wrote:
>> > From: Shannon Zhao
>> >
>> > If there are more than one /memory nodes in FDT, it needs to find the
>> > lowest one, and set PcdSystemMemorySize with thes size of this node.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Shannon Zhao
>> > ---
>> > ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30
>> >
>> > 1 file changed, 17 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
>> > b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
>> > index 17f2686..7a0fc0e 100644
>> > --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
>> > +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
>> > @@ -72,8 +72,8 @@ ArmPlatformInitializeSystemMemory (
>> > {
>> >VOID *DeviceTreeBase;
>> >INT32Node, Prev;
>> > - UINT64 NewBase;
>> > - UINT64 NewSize;
>> > + UINT64 NewBase, CurBase;
>> > + UINT64 NewSize, CurSize;
>> >CONST CHAR8 *Type;
>> >INT32Len;
>> >CONST UINT64 *RegProp;
>> > @@ -90,7 +90,7 @@ ArmPlatformInitializeSystemMemory (
>> >ASSERT (fdt_check_header (DeviceTreeBase) == 0);
>> >
>> >//
>> > - // Look for a memory node
>> > + // Look for the lowest memory node
>> >//
>> >for (Prev = 0;; Prev = Node) {
>> > Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
>> > @@ -110,26 +110,30 @@ ArmPlatformInitializeSystemMemory (
>> >RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
>> >if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
>> >
>> > -NewBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
>> > -NewSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
>> > -
>> > -//
>> > -// Make sure the start of DRAM matches our expectation
>> > -//
>> > -ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
>> > -PcdSet64 (PcdSystemMemorySize, NewSize);
>> > +CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
>> > +CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
>> >
>> > DEBUG ((EFI_D_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
>> > - __FUNCTION__, NewBase, NewBase + NewSize - 1));
>> > + __FUNCTION__, CurBase, CurBase + CurSize - 1));
>> > +
>> > +if (NewBase > CurBase || NewBase == 0) {
>> > + NewBase = CurBase;
>> > + NewSize = CurSize;
>> > +}
> I wonder if it would not make more sense to search for the memory node
> whose base == FixedPcdGet64 (PcdSystemMemoryBase) directly, rather
> than find the lowest one and ASSERT() that it is the one we are
> looking for.
>
Yeah, but if there is a (wrong) memory node with a base address lower
than FixedPcdGet64 (PcdSystemMemoryBase), does this work for UEFI or OS?
Thanks,
--
Shannon
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] ArmVirtPkg: Find the lowest memory node
On 2015/11/30 13:56, Laszlo Ersek wrote: > Looks good to me. > > Reviewed-by: Laszlo Ersek > > When committing this (a bit later), I think I'll fix up the commit > message. There'a s typo in it, plus I'd like to add, from your original > email, "QEMU NUMA support on ARM will introduce more than one /memory > node in the device tree". Thanks for your help. :) -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] ArmVirtPkg: Find the lowest memory node
From: Shannon Zhao
If there are more than one /memory nodes in FDT, it needs to find the
lowest one, and set PcdSystemMemorySize with thes size of this node.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
index 17f2686..7a0fc0e 100644
--- a/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
+++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c
@@ -72,8 +72,8 @@ ArmPlatformInitializeSystemMemory (
{
VOID *DeviceTreeBase;
INT32Node, Prev;
- UINT64 NewBase;
- UINT64 NewSize;
+ UINT64 NewBase, CurBase;
+ UINT64 NewSize, CurSize;
CONST CHAR8 *Type;
INT32Len;
CONST UINT64 *RegProp;
@@ -90,7 +90,7 @@ ArmPlatformInitializeSystemMemory (
ASSERT (fdt_check_header (DeviceTreeBase) == 0);
//
- // Look for a memory node
+ // Look for the lowest memory node
//
for (Prev = 0;; Prev = Node) {
Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
@@ -110,26 +110,30 @@ ArmPlatformInitializeSystemMemory (
RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
-NewBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
-NewSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
-
-//
-// Make sure the start of DRAM matches our expectation
-//
-ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
-PcdSet64 (PcdSystemMemorySize, NewSize);
+CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
DEBUG ((EFI_D_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
- __FUNCTION__, NewBase, NewBase + NewSize - 1));
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+
+if (NewBase > CurBase || NewBase == 0) {
+ NewBase = CurBase;
+ NewSize = CurSize;
+}
} else {
DEBUG ((EFI_D_ERROR, "%a: Failed to parse FDT memory node\n",
__FUNCTION__));
}
- break;
}
}
//
+ // Make sure the start of DRAM matches our expectation
+ //
+ ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
+ PcdSet64 (PcdSystemMemorySize, NewSize);
+
+ //
// We need to make sure that the machine we are running on has at least
// 128 MB of memory configured, and is currently executing this binary from
// NOR flash. This prevents a device tree image in DRAM from getting
--
2.0.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] ArmVirtPkg: Add support for multiple memory nodes
From: Shannon Zhao If there are more than one memory nodes in FDT, currently UEFI will assert. These two patches firstly let UEFI find the memory node with lowest base address and check if the address is as we expected and set PcdSystemMemorySize as the size of this node. Then add other memory spaces through gDS->AddMemorySpace() when it parses FDT. Shannon Zhao (2): ArmVirtPkg: Find the lowest memory node ArmVirtPkg: Add memory space for the memory nodes except the lowest one ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 30 +--- ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 34 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4 3 files changed, 55 insertions(+), 13 deletions(-) -- 2.0.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] ArmVirtPkg: Add memory space for the memory nodes except the lowest one
From: Shannon Zhao
Here we add the memory space for the memory nodes except the lowest one
in FDT. So these spaces will show up in the UEFI memory map.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao
---
ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c | 34 ++
ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf | 4
2 files changed, 38 insertions(+)
diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
index 74f80d1..2c8d785 100644
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
+++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -304,6 +305,8 @@ InitializeVirtFdtDxe (
UINT64 FwCfgDataSize;
UINT64 FwCfgDmaAddress;
UINT64 FwCfgDmaSize;
+ UINT64 CurBase;
+ UINT64 CurSize;
Hob = GetFirstGuidHob(&gFdtHobGuid);
if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
@@ -332,6 +335,37 @@ InitializeVirtFdtDxe (
break;
}
+//
+// Check for memory node and add the memory spaces expect the lowest one
+//
+Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
+ //
+ // Get the 'reg' property of this node. For now, we will assume
+ // two 8 byte quantities for base and size, respectively.
+ //
+ RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+ if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
+
+CurBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
+CurSize = fdt64_to_cpu (((UINT64 *)RegProp)[1]);
+
+if (FixedPcdGet64 (PcdSystemMemoryBase) != CurBase) {
+ Status = gDS->AddMemorySpace(EfiGcdMemoryTypeSystemMemory,
+ CurBase, CurSize,
+ EFI_MEMORY_WB | EFI_MEMORY_RUNTIME);
+ if (EFI_ERROR(Status)) {
+ASSERT_EFI_ERROR(Status);
+ }
+ DEBUG ((EFI_D_INFO, "%a: Add System RAM @ 0x%lx - 0x%lx\n",
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+}
+ } else {
+DEBUG ((EFI_D_ERROR, "%a: Failed to parse FDT memory node\n",
+ __FUNCTION__));
+ }
+}
+
Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
if (Type == NULL) {
continue;
diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
index ee2503a..0e6927b 100644
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
@@ -43,12 +43,16 @@
VirtioMmioDeviceLib
HobLib
XenIoMmioLib
+ DxeServicesTableLib
[Guids]
gFdtTableGuid
gVirtioMmioTransportGuid
gFdtHobGuid
+[FixedPcd]
+ gArmTokenSpaceGuid.PcdSystemMemoryBase
+
[Pcd]
gArmVirtTokenSpaceGuid.PcdArmPsciMethod
gArmVirtTokenSpaceGuid.PcdFwCfgSelectorAddress
--
2.0.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Bug report: UEFI asserts when QEMU supports NUMA on ARM
On 2015/11/27 18:05, Laszlo Ersek wrote: > adding Ard, Leif, Drew and Wei > > On 11/27/15 10:00, Shannon Zhao wrote: >> Hi Laszlo, >> >> Currently I work on the QEMU NUMA support on ARM. This will introduce >> more than one /memory nodes in DTS. It causes UEFI assert since UEFI >> only considers that there is only one memory node and the memory base >> doesn't change anyway. >> >> ASSERT >> /home/open-source/edk2/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c(119): >> 0x4000 >> == NewBase >> >> Since the NUMA feature is on its way, I think it needs to fix this and >> consider more than one /memory nodes in DTS. > > This is not just a "fix" but an intrusive new feature. (So your subject > should say "RFE", not "bug report". :)) Discontiguous memory support is > not trivial to add, as exemplified by our long discussion on edk2-devel > with Benjamin Herrenschmidt. > > The least intrusive approach for this is probably the following: > > (1) in ArmPlatformInitializeSystemMemory() > [ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c], modify the code (that > currently asserts) so that it locates the lowest /memory node. > > The current base (== 0x4000) and size (>= SIZE_128MB) checks should > be then enforced for this node only, and the "PcdSystemMemorySize" PCD > should be set to the actual size of this node. > > This alone will ensure that the firmware will ignore all *other* memory > nodes. The (currently only) "base" node should be definitely enough for > reaching and launching the DXE core. > > In the DXE phase our programming environment is much more flexible, so > I'd like to minimize the impact on PEI. (Despite the fact that in theory > it would be possible to expose all this additional memory with resource > descriptor HOBs during PEI, so that the DXE core would immediately > enable them, when it primes the GCD memory space map from the HOBs.) > > (2) As second step, we might be able to update the ArmVirtPkg/VirtFdtDxe > driver. In general, this driver (which runs very early in the DXE phase, > by explicit A PRIORI ordering) only parses bits of information from the > DTB, and sets various PCDs for other drivers to consume. > > However, when a certain type of information is "repetitive", such as the > virtio-mmio transports, then ArmVirtPkg/VirtFdtDxe has to do actual work > -- this kind of information is not easy to store in PCDs. In the > virtio-mmio transport case, we call the VirtioMmioInstallDevice() > library function for each transport found -- this works pretty well, > although it muddles the responsibilities of VirtFdtDxe a little bit. > (But we can certainly live with that.) > > So, the idea here is to iterate over the memory nodes again, ignore the > lowest one, and for each additional memory node found, call > gDS->AddMemorySpace() explicitly, with type > "EfiGcdMemoryTypeSystemMemory", and the right cacheability and runtime > capabilities. > > This *should* make the new memory range immediately available to the DXE > phase, for UEFI memory allocations. (The PI spec says that it "may" > happen -- it looks like in edk2 it actually does happen. Actually, the > PI spec seems to guarantee that this occurs if the base address and the > size of the new memory area are 4KB aligned.) > > Either way, much more importantly, it should ensure that the memory > shows up in the UEFI memory map when the runtime OS is booted. > > (An example for adding system memory during DXE can be seen in: > "ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c", function > ArmJunoEntryPoint().) > > Then we will have to verify that the *actual* cacheability attributes > (as far as the MMU is concerned) will be set *automatically* to the > right ones. > > If that doesn't happen, then we'll have to call > gDS->SetMemorySpaceAttributes() as well, right after each > gDS->AddMemorySpace() call. This will ultimately end up calling the ARM > CPU architectural protocol implementation, and massage the MMU. > > For being safe, I think we should call gDS->SetMemorySpaceAttributes() > unconditionally. > > Do you think you can contribute patches for this? If not, we might be > able to look into it, but I'm not sure when. > Thanks for your explanation. :) I can have a look at this. BTW, If I boot VM with ACPI, is it necessary to parse the FDT? From the test result, it does parse the FDT when booting with ACPI since it asserts there too. Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] Bug report: UEFI asserts when QEMU supports NUMA on ARM
Hi Laszlo, Currently I work on the QEMU NUMA support on ARM. This will introduce more than one /memory nodes in DTS. It causes UEFI assert since UEFI only considers that there is only one memory node and the memory base doesn't change anyway. ASSERT /home/open-source/edk2/ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c(119): 0x4000 == NewBase Since the NUMA feature is on its way, I think it needs to fix this and consider more than one /memory nodes in DTS. Thanks, -- Shannon ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
