On Tue, Jun 04, 2024 at 08:47:22AM -0700, Dave Hansen wrote: > On 6/4/24 08:32, Kirill A. Shutemov wrote: > > What about the comment below? > > > > /* > > * One possible reason for the failure is if kexec raced > > * with memory conversion. In this case shared bit in > > * page table got set (or not cleared) during > > * shared<->private conversion, but the page is actually > > * private. So this failure is not going to affect the > > * kexec'ed kernel. > > * > > * The only thing one can do at this point on failure > > * at this point is panic. In absence of better options, > > * it is reasonable to proceed, hoping the failure is a > > * benign shared bit mismatch due to the race. > > * > > * Also, even if the failure is real and the page cannot > > * be touched as private, the kdump kernel will boot > > * fine as it uses pre-reserved memory. What happens > > * next depends on what the dumping process does and > > * there's a reasonable chance to produce useful dump > > * on crash. > > * > > * Regardless, the print leaves a trace in the log to > > * give a clue for debug. > > */ > > It's rambling too much for my taste. > > Let's boil this down to what matters: > > 1. Failures to change encryption status here can lead a future kernel > to touch shared memory with a private mapping > 2. That causes an immediate unrecoverable guest shutdown (right?) Right. > 3. kdump kernels should not be affected since they have their own > memory ranges and its encryption status is not being tweawked here > 4. The pr_err() may help make some sense out of #2 when it happens > > I'm not sure the reason behind the failed conversion is important here. The important part is that failure can be benign. It explains "can" in #1. But okay. > I wouldn't mention panic(). > > We don't need to opine about what the next kernel might or might not do. Is this any better? /* * If tdx_enc_status_changed() fails, it leaves memory * in an unknown state. If the memory remains shared, * it can result in an unrecoverable guest shutdown on * the first accessed through a private mapping. * * The kdump kernel boot is not impacted as it uses * a pre-reserved memory range that is always private. * However, gathering crash information could lead to * a crash if it accesses unconverted memory through * a private mapping. * * pr_err() may assist in understanding such crashes. */ -- Kiryl Shutsemau / Kirill A. Shutemov
