On Thu, Apr 07, 2022 at 04:44:01PM +0200, Philipp Rudo wrote:
Hi Tao,
On Wed, 6 Apr 2022 18:55:38 +0800 Tao Liu ltao@redhat.com wrote:
On Tue, Apr 5, 2022 at 5:37 PM Philipp Rudo prudo@redhat.com wrote:
Hi Coiby, Hi Tao,
On Sat, 2 Apr 2022 16:43:37 +0800 Coiby Xu coxu@redhat.com wrote:
Hi Tao,
On Sat, Apr 02, 2022 at 02:14:38PM +0800, Tao Liu wrote:
Hi Philipp,
On Wed, Mar 30, 2022 at 11:34 PM Philipp Rudo prudo@redhat.com wrote:
[...]
> implemented the reservation in the kernel side, as part of the kernel > crashkernel=auto implementation.
For swiotlb, I think Dave and Emma prefer to do it in kernel space. Maybe you can try a kernel space solution first like [1]?
[1] https://lore.kernel.org/lkml/20190910151341.14986-3-kasong@redhat.com/
> > For rhel9 however, crashkernel=auto has been mirrored to userspace > kexec-tools, so this patch will enable the swiotlb reservation for AMD > sme/sev in kexec-tools as well.
Not really about this patch but I find it weird that we adjust the crashkernel=auto value for swiotlb but not for other cases wher we know that we need a larger crashkernel, e.g. when the dump target is on a LUKS encrypted device. I understand that in the past, when crashkernel=auto was handled by the kernel, we simply didn't know about what the dump target will be when we allocated the crashkernel during boot. But today, when crashkernel=auto is handled in userspace we can.
I didn't have much context on LUKS, by looking into the code, I see only "kdumpctl estimate" has the LUKS related crashkernel value increment. It needs the user to adjust crashkernel value manually by estimate-and-set. Maybe we can merge the crashkernel adjustment of swiotlb and luks together, so luks don't need to "kdumpctl estimate" manually anymore?
For LUKS, a kernel-space solution by reusing LUKS master key is preferred because a) LUKS requires too much memory b) we can't expect the user to wait at the screen to input the passphrase to open the disk when kernel crashes. Let's see how far my work [2] can go.
Yeah, after I played around with LUKS for a while, I agree with your point. kernel side implementation is better for luks.
Just for curiosity, as for the case b), can't we use a password file instead of inputting passphrase manually every time? Like:
$ cat /etc/fstab /dev/mapper/mydata /mnt ext4 defaults 0 0 $ cat /etc/crypttab mydata /dev/vdb /root/lukspass $ cryptsetup luksAddKey /dev/vdb /root/lukspass
If /root/lukspass is accessible, luks device can be automatically mounted, thus no user interaction needed.
Sounds like an interesting idea. My only concern is that we introduce a potential security risk, e.g. somebody might extract the key file from the initrd and use it to access a disk without permission. But I don't have much experience in the area. So I think it's definitely something worth to further investigate.
Yes, although I don't know how it may be exploited, I think some people may object of the idea of using a passpharse file. Btw, this only partially solves the LUKS problem because the second issue is using the passpharse to compute the master key consumes the same amount of memory as using the password manually by the user.
you are right that it is better to solve the problem with LUKS in the kernel. I only used it as an example where today we know that we need more memory but don't automatically increase the crashkernel. So why do it for swiotlb but for others? What's the reason for this inconsistency?
By looking into the "kdumpctl estimate" code, the memory estimating for luks is calculated dynamically, depending on how many crypt devices there are, and their "Memory:" field status.
However for amd sme/sev swiotlb, the reserved memory for 2nd kernel is 64M, see rhel-8 kernel commit ab30d3f ("[x86] Reserve at most 64M of SWIOTLB memory for crashkernel").
Note the extra memory is not also dynamic for the sme/sev case for the kernel implementation,
if (mem_encrypt_active()) mem_enc_req = min(ALIGN(swiotlb_size_or_default(), SZ_1M), 64UL << 20); else mem_enc_req = 0;
I implement it in the kexec-tools side, because the rhel-8 patch is part of craskernel=auto implementation. As far as I know crashkernel=auto has been mirrored to kexec-tools side, so it is reasonable for me to make it in kexec-tools side as well.
I fully understand. But remember that the crashkernel needs to be reserved extremely early during boot. Even before the initramfs is mounted. So when crashkernel=auto was implemented in the kernel we simply didn't had information about many things, like what the root device is and if it is encrypted. swiotlb was one of the few exceptions. By moving crashkernel=auto to the kexec-tools we now have the information and could use it. If we decide to do so (see Coibys mail).
Actually, implementing it in kernel has one benefit which is we can calculate the memory requirement before allocating kdump memory so there is no need to reboot the system.
There is also another problem for implementing it in kexec-tools. Currently kexec-tools assume the default crashernel values are static and only try to update the kernel cmdline parameter crashkernel when the package is updated. For the case of the user dynamically toggling sme/sev on/off, we need a mechanism to automatically update the crashkernel parameter and then reboot the system. And this is not a concern if we implement it in kernel side.