| Message ID | 1488471305-23752-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | ArmVirtPkg: implement basic capsule support | expand |
On 03/02/17 17:15, Ard Biesheuvel wrote: > This wires up the existing basic support for capsules left in memory by > the OS across a warm reset. This involves wiring up the PEI phase modules > to preserve the capsule images before releasing the memory for normal > consumption, and some tweaks to the boot mode and BDS platform routines. > > As proposed, this allows capsules to be used as a pstore backend, which > keeps the pstore payload in memory rather than in EFI variables. For > example, something like this is supported when the prerequisite Linux > patches have been merged (which are currently under review) > > - modprobe capsule-pstore > - echo 1 > /sys/module/kernel/parameters/panic > - echo c > /proc/sysrq-trigger > - system reboot... > - ls -l /sys/fs/pstore/ > -r--r--r-- 1 root root 4386 Feb 12 00:31 console-efi-capsule-0 > -r--r--r-- 1 root root 9065 Feb 12 00:29 dmesg-efi-capsule-6250071391647825921 > -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825922 > -r--r--r-- 1 root root 9096 Feb 12 00:29 dmesg-efi-capsule-6250071391647825923 > -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825924 > -r--r--r-- 1 root root 9048 Feb 12 00:29 dmesg-efi-capsule-6250071391647825925 > > Updating the firmware image in NOR flash in this way should be feasible as > well, but this is something that builds on top of this basic capsule support, > and involves ESRT and FMP, which have far too few vowels for me to explain > what they entail. > > Ard Biesheuvel (4): > ArmVirtPkg/ArmVirtPlatformLib: base boot mode on capsule presence > ArmVirtPkg/ArmVirtMemoryInitPeiLib: check for capsules before memory > init > ArmVirtPkg/PlatformBootManagerLib: process pending capsules > ArmVirtPkg/ArmVirtQemu: enable basic capsule support > > ArmVirtPkg/ArmVirtQemu.dsc | 7 ++- > ArmVirtPkg/ArmVirtQemu.fdf | 2 + > ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 60 +++++++++++++++++++- > ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf | 9 ++- > ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf | 5 +- > ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 4 ++ > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 17 ++++++ > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 + > 8 files changed, 101 insertions(+), 5 deletions(-) > (1) I think I disagree with this feature. What is the use of capsule-pstore over efivar-pstore? I dislike the addition of a new boot mode (BOOT_ON_FLASH_UPDATE) -- and all the complications it means for PlatformBootManagerLib -- unless there is a *very* good argument for capsule-pstore. Even if there is such a very good argument, a detailed design document would be necessary about - memory regions (address, size, life cycle etc) used for capsule passing from OS to firmware across warm reboot, - interplay between existent and newly pulled in modules (for example, what produces the EFI_HOB_TYPE_UEFI_CAPSULE HOB before patch #1 and "ArmPlatformPkg/PlatformPei/PlatformPeim.inf" consume it, what ensures the HOB would be prodced in time, how does the new boot mode affect other modules, why do we need setting / returning the new boot mode in both patches #1 and #2, ...) Even if said "very good reason" exists, I think I'd insist on a big red switch over all of this, defaulting to "off" -- a build time flag that by default excludes modules from the DSC / FDF, and also a parallel Feature PCD, which short-circuits all the new code in the modules that we build in unconditionally. (Same as we do with SMM_REQUIRE in OVMF.) (2) I most definitely disagree with the idea of firmware executable updates within the guest. That turns firmware image updates into a multi-master problem (on a virtualization host, you update firmware for guests by running a package update, and then shutting down and restarting eligible guests -- the host file that backs the pflash chip that contains the firmware executable is not even writeable to QEMU). Such a feature might perhaps be useful for testing the capsule thing itself, but in production, it's a recipe for disaster. This kind of capsule-based firmware update was invented to solve a chicken-egg problem on physical hardware, where there's nothing "underneath", but on virtual platforms, the problem doesn't exist in the first place. The implication that this feature -- which I already disagree with, without knowing a compelling reason for it -- lays the foundation for FMP, makes me *very* nervous. Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 2 March 2017 at 17:09, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/02/17 17:15, Ard Biesheuvel wrote: >> This wires up the existing basic support for capsules left in memory by >> the OS across a warm reset. This involves wiring up the PEI phase modules >> to preserve the capsule images before releasing the memory for normal >> consumption, and some tweaks to the boot mode and BDS platform routines. >> >> As proposed, this allows capsules to be used as a pstore backend, which >> keeps the pstore payload in memory rather than in EFI variables. For >> example, something like this is supported when the prerequisite Linux >> patches have been merged (which are currently under review) >> >> - modprobe capsule-pstore >> - echo 1 > /sys/module/kernel/parameters/panic >> - echo c > /proc/sysrq-trigger >> - system reboot... >> - ls -l /sys/fs/pstore/ >> -r--r--r-- 1 root root 4386 Feb 12 00:31 console-efi-capsule-0 >> -r--r--r-- 1 root root 9065 Feb 12 00:29 dmesg-efi-capsule-6250071391647825921 >> -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825922 >> -r--r--r-- 1 root root 9096 Feb 12 00:29 dmesg-efi-capsule-6250071391647825923 >> -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825924 >> -r--r--r-- 1 root root 9048 Feb 12 00:29 dmesg-efi-capsule-6250071391647825925 >> >> Updating the firmware image in NOR flash in this way should be feasible as >> well, but this is something that builds on top of this basic capsule support, >> and involves ESRT and FMP, which have far too few vowels for me to explain >> what they entail. >> >> Ard Biesheuvel (4): >> ArmVirtPkg/ArmVirtPlatformLib: base boot mode on capsule presence >> ArmVirtPkg/ArmVirtMemoryInitPeiLib: check for capsules before memory >> init >> ArmVirtPkg/PlatformBootManagerLib: process pending capsules >> ArmVirtPkg/ArmVirtQemu: enable basic capsule support >> >> ArmVirtPkg/ArmVirtQemu.dsc | 7 ++- >> ArmVirtPkg/ArmVirtQemu.fdf | 2 + >> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 60 +++++++++++++++++++- >> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf | 9 ++- >> ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf | 5 +- >> ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 4 ++ >> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 17 ++++++ >> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 + >> 8 files changed, 101 insertions(+), 5 deletions(-) >> > > (1) I think I disagree with this feature. What is the use of > capsule-pstore over efivar-pstore? efivar-pstore keeps the logs itself in EFI variables, and is known to cause problems when it runs out of space. > I dislike the addition of a new boot > mode (BOOT_ON_FLASH_UPDATE) -- and all the complications it means for > PlatformBootManagerLib -- unless there is a *very* good argument for > capsule-pstore. > > Even if there is such a very good argument, a detailed design document > would be necessary about > > - memory regions (address, size, life cycle etc) used for capsule > passing from OS to firmware across warm reboot, > Yes, I wondered about this: how do we keep the OS from putting the capsule in our temporary PEI ram. Some AArch64 systems use on-chip SRAM for temporary PEI RAM, which nicely sidesteps the problem. > - interplay between existent and newly pulled in modules (for example, > what produces the EFI_HOB_TYPE_UEFI_CAPSULE HOB before patch #1 and > "ArmPlatformPkg/PlatformPei/PlatformPeim.inf" consume it, what ensures > the HOB would be prodced in time, how does the new boot mode affect > other modules, why do we need setting / returning the new boot mode > in both patches #1 and #2, ...) > The one concern I have here is that we currently have no way of signalling the boot mode at all, and we have to infer it. Ideally, the UpdateCapsule() and/or ResetSystem() call arguments propagate into this variable, but how this is implemented is platform specific afaict > Even if said "very good reason" exists, I think I'd insist on a big red > switch over all of this, defaulting to "off" -- a build time flag that > by default excludes modules from the DSC / FDF, and also a parallel > Feature PCD, which short-circuits all the new code in the modules that > we build in unconditionally. (Same as we do with SMM_REQUIRE in OVMF.) > Fair enough. I think this mainly comes down to our difference in use cases: for me, ArmVirtQemu is primarily a reference implementation, whereas your interest is strictly virtualization. > (2) I most definitely disagree with the idea of firmware executable > updates within the guest. That turns firmware image updates into a > multi-master problem (on a virtualization host, you update firmware for > guests by running a package update, and then shutting down and > restarting eligible guests -- the host file that backs the pflash chip > that contains the firmware executable is not even writeable to QEMU). > Same point as above. If Linaro members need a reference impementation of FMP, ArmVirtQemu is the vehicle atm. > Such a feature might perhaps be useful for testing the capsule thing > itself, but in production, it's a recipe for disaster. This kind of > capsule-based firmware update was invented to solve a chicken-egg > problem on physical hardware, where there's nothing "underneath", but on > virtual platforms, the problem doesn't exist in the first place. > > The implication that this feature -- which I already disagree with, > without knowing a compelling reason for it -- lays the foundation for > FMP, makes me *very* nervous. > Perhaps it is time to separate the showcase ArmVirtQemu from the QEMU/KVM ArmVirtQemu. In fact, we are already working on a QEMU machine model different from mach-virt which matches the enterprise system that we target more closely, and so we'd need a different ArmVirtPkg platform for that anyway. Another thing that has been on my wishlist for a long time is a QEMU machine that has no RAM below 4 GB (I have local patches for that, but it would be good to have that upstream as well) In fact, I am quite pleased that we have reached this point, since it means the basic functionality is all there. In any case, we will have plenty of time to discuss these things next week. Cheers, Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/02/17 18:22, Ard Biesheuvel wrote: > On 2 March 2017 at 17:09, Laszlo Ersek <lersek@redhat.com> wrote: >> On 03/02/17 17:15, Ard Biesheuvel wrote: >>> This wires up the existing basic support for capsules left in memory by >>> the OS across a warm reset. This involves wiring up the PEI phase modules >>> to preserve the capsule images before releasing the memory for normal >>> consumption, and some tweaks to the boot mode and BDS platform routines. >>> >>> As proposed, this allows capsules to be used as a pstore backend, which >>> keeps the pstore payload in memory rather than in EFI variables. For >>> example, something like this is supported when the prerequisite Linux >>> patches have been merged (which are currently under review) >>> >>> - modprobe capsule-pstore >>> - echo 1 > /sys/module/kernel/parameters/panic >>> - echo c > /proc/sysrq-trigger >>> - system reboot... >>> - ls -l /sys/fs/pstore/ >>> -r--r--r-- 1 root root 4386 Feb 12 00:31 console-efi-capsule-0 >>> -r--r--r-- 1 root root 9065 Feb 12 00:29 dmesg-efi-capsule-6250071391647825921 >>> -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825922 >>> -r--r--r-- 1 root root 9096 Feb 12 00:29 dmesg-efi-capsule-6250071391647825923 >>> -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825924 >>> -r--r--r-- 1 root root 9048 Feb 12 00:29 dmesg-efi-capsule-6250071391647825925 >>> >>> Updating the firmware image in NOR flash in this way should be feasible as >>> well, but this is something that builds on top of this basic capsule support, >>> and involves ESRT and FMP, which have far too few vowels for me to explain >>> what they entail. >>> >>> Ard Biesheuvel (4): >>> ArmVirtPkg/ArmVirtPlatformLib: base boot mode on capsule presence >>> ArmVirtPkg/ArmVirtMemoryInitPeiLib: check for capsules before memory >>> init >>> ArmVirtPkg/PlatformBootManagerLib: process pending capsules >>> ArmVirtPkg/ArmVirtQemu: enable basic capsule support >>> >>> ArmVirtPkg/ArmVirtQemu.dsc | 7 ++- >>> ArmVirtPkg/ArmVirtQemu.fdf | 2 + >>> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 60 +++++++++++++++++++- >>> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf | 9 ++- >>> ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf | 5 +- >>> ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 4 ++ >>> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 17 ++++++ >>> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 + >>> 8 files changed, 101 insertions(+), 5 deletions(-) >>> >> >> (1) I think I disagree with this feature. What is the use of >> capsule-pstore over efivar-pstore? > > efivar-pstore keeps the logs itself in EFI variables, and is known to > cause problems when it runs out of space. I don't think I can debate that, but for VMs, virtio-pstore (an upcoming QEMU feature IIRC) would be a better alternative then. > >> I dislike the addition of a new boot >> mode (BOOT_ON_FLASH_UPDATE) -- and all the complications it means for >> PlatformBootManagerLib -- unless there is a *very* good argument for >> capsule-pstore. >> >> Even if there is such a very good argument, a detailed design document >> would be necessary about >> >> - memory regions (address, size, life cycle etc) used for capsule >> passing from OS to firmware across warm reboot, >> > > Yes, I wondered about this: how do we keep the OS from putting the > capsule in our temporary PEI ram. Some AArch64 systems use on-chip > SRAM for temporary PEI RAM, which nicely sidesteps the problem. Well, one solution would be to reserve both the temporary and the permanent PEI RAM regions used during "flash update boot" as Reserved or AcpiNVS memory from the OS, assuming the firmware feature is enabled. Then the OS would stay away. In OVMF, we do exactly this if the S3 feature is enabled on the QEMU command line -- during first boot, we reserve - the small, boot mode-independent RAM area that is used as temporary SEC/PEI heap & stack, and - the larger, S3 resume-specific RAM area that is used as permanent PEI RAM during S3 resume. (The permanent PEI RAM used during normal boot is larger and separate.) Another idea is to reserve yet another small, separate area, and when gRT->UpdateCapsule() is called, coalesce the input (the capsules) immediately to that reserved area, rather than just keep pointers. (This is somewhat similar to the effects of CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE, although the coalescing would occur immediately.) > >> - interplay between existent and newly pulled in modules (for example, >> what produces the EFI_HOB_TYPE_UEFI_CAPSULE HOB before patch #1 and >> "ArmPlatformPkg/PlatformPei/PlatformPeim.inf" consume it, what ensures >> the HOB would be prodced in time, how does the new boot mode affect >> other modules, why do we need setting / returning the new boot mode >> in both patches #1 and #2, ...) >> > > The one concern I have here is that we currently have no way of > signalling the boot mode at all, and we have to infer it. Ideally, the > UpdateCapsule() and/or ResetSystem() call arguments propagate into > this variable, but how this is implemented is platform specific afaict Yeah, in OVMF we only distinguish "boot with full config" from "s3 resume", and the basis for that is a CMOS register value that is set by QEMU itself -- see rtc_notify_suspend() in "hw/timer/mc146818rtc.c" --, not by the guest OS. On ArmVirtQemu, gRT->ResetSystem() lands in "ArmVirtPkg/Library/ArmVirtPsciResetSystemLib", to my knowledge. The handling of the ResetType parameter does not consider a "capsule update" reset type just yet. So I think ArmVirtQemu's gRT->QueryCapsuleCapabilities() implementation should dedicate a value to this reset type, and then ArmVirtPsciResetSystemLib should both recognize that value, *and* store the fact in some register of the "virt" QEMU board, such that it survives the actual reset. (Or, I guess, this fact could again be stashed in a pre-reserved memory area -- a small one --, because guest memory itself would survive the reset.) This would only work of course if the guest OS rebooted the VM via the appropriate gRT->ResetSystem() call. > >> Even if said "very good reason" exists, I think I'd insist on a big red >> switch over all of this, defaulting to "off" -- a build time flag that >> by default excludes modules from the DSC / FDF, and also a parallel >> Feature PCD, which short-circuits all the new code in the modules that >> we build in unconditionally. (Same as we do with SMM_REQUIRE in OVMF.) >> > > Fair enough. I think this mainly comes down to our difference in use > cases: for me, ArmVirtQemu is primarily a reference implementation, > whereas your interest is strictly virtualization. True. >> (2) I most definitely disagree with the idea of firmware executable >> updates within the guest. That turns firmware image updates into a >> multi-master problem (on a virtualization host, you update firmware for >> guests by running a package update, and then shutting down and >> restarting eligible guests -- the host file that backs the pflash chip >> that contains the firmware executable is not even writeable to QEMU). >> > > Same point as above. If Linaro members need a reference impementation > of FMP, ArmVirtQemu is the vehicle atm. Good point. I think these different use cases do justify the build flag / feature PCD. I guess we could agree that patches & code that affect capsule handling would fall under your maintenance (like Xen and a bunch of other parts already do, in effect!), and I would check such patches against regressing the capsule-less use case, and for general (higher level) sanity. > >> Such a feature might perhaps be useful for testing the capsule thing >> itself, but in production, it's a recipe for disaster. This kind of >> capsule-based firmware update was invented to solve a chicken-egg >> problem on physical hardware, where there's nothing "underneath", but on >> virtual platforms, the problem doesn't exist in the first place. >> >> The implication that this feature -- which I already disagree with, >> without knowing a compelling reason for it -- lays the foundation for >> FMP, makes me *very* nervous. >> > > Perhaps it is time to separate the showcase ArmVirtQemu from the > QEMU/KVM ArmVirtQemu. In fact, we are already working on a QEMU > machine model different from mach-virt which matches the enterprise > system that we target more closely, and so we'd need a different > ArmVirtPkg platform for that anyway. Wow, that's a larger change than I expected :) I agree that if the new QEMU board type is sufficiently different, that could be good justification for a separate DSC. In OVMF we try to handle i440fx, Q35, and Xen dynamically, and that causes a lot of pain. A few months back the Xen community started developing a new domain type (PVH2 if I remember correctly? not entirely sure), and for me that was the breaking point in OVMF's dynamic platform support -- I asked for a separate DSC. (And, in the longer term, to separate out even the current Xen code to that new platform.) We can do some dynamism in the firmware, but PI doesn't really optimize for the same platform drivers (PEIMs and DXE (runtime) drivers) to execute on seriously different platforms. UEFI drivers/apps are different, of course (and they are not the problem now). > Another thing that has been on my > wishlist for a long time is a QEMU machine that has no RAM below 4 GB > (I have local patches for that, but it would be good to have that > upstream as well) > > In fact, I am quite pleased that we have reached this point, since it > means the basic functionality is all there. In any case, we will have > plenty of time to discuss these things next week. Right. Let's hope I can shed this flu sufficiently until then. CC'ing Drew for his information. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
This wires up the existing basic support for capsules left in memory by the OS across a warm reset. This involves wiring up the PEI phase modules to preserve the capsule images before releasing the memory for normal consumption, and some tweaks to the boot mode and BDS platform routines. As proposed, this allows capsules to be used as a pstore backend, which keeps the pstore payload in memory rather than in EFI variables. For example, something like this is supported when the prerequisite Linux patches have been merged (which are currently under review) - modprobe capsule-pstore - echo 1 > /sys/module/kernel/parameters/panic - echo c > /proc/sysrq-trigger - system reboot... - ls -l /sys/fs/pstore/ -r--r--r-- 1 root root 4386 Feb 12 00:31 console-efi-capsule-0 -r--r--r-- 1 root root 9065 Feb 12 00:29 dmesg-efi-capsule-6250071391647825921 -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825922 -r--r--r-- 1 root root 9096 Feb 12 00:29 dmesg-efi-capsule-6250071391647825923 -r--r--r-- 1 root root 9073 Feb 12 00:29 dmesg-efi-capsule-6250071391647825924 -r--r--r-- 1 root root 9048 Feb 12 00:29 dmesg-efi-capsule-6250071391647825925 Updating the firmware image in NOR flash in this way should be feasible as well, but this is something that builds on top of this basic capsule support, and involves ESRT and FMP, which have far too few vowels for me to explain what they entail. Ard Biesheuvel (4): ArmVirtPkg/ArmVirtPlatformLib: base boot mode on capsule presence ArmVirtPkg/ArmVirtMemoryInitPeiLib: check for capsules before memory init ArmVirtPkg/PlatformBootManagerLib: process pending capsules ArmVirtPkg/ArmVirtQemu: enable basic capsule support ArmVirtPkg/ArmVirtQemu.dsc | 7 ++- ArmVirtPkg/ArmVirtQemu.fdf | 2 + ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 60 +++++++++++++++++++- ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf | 9 ++- ArmVirtPkg/Library/ArmVirtPlatformLib/ArmVirtPlatformLib.inf | 5 +- ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c | 4 ++ ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 17 ++++++ ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 + 8 files changed, 101 insertions(+), 5 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel