From: Edgecombe, Rick P <rick.p.edgecombe@xxxxxxxxx> Sent: Friday, January 12, 2024 9:17 AM > > On Fri, 2024-01-12 at 15:07 +0000, Michael Kelley wrote: > > The comment is Kirill Shutemov's suggestion based on comments in > > an earlier version of the patch series. See [1]. The intent is to > > prevent > > some later revision to slow_virt_to_phys() from adding a check for > > the > > present bit and breaking the CoCo VM hypervisor callback. Yes, the > > comment could get stale, but I'm not sure how else to call out the > > implicit dependency. The idea of creating a private version of > > slow_virt_to_phys() for use only in the CoCo VM hypervisor callback > > is also discussed in the thread, but that seems worse overall. > > Well, it's not a huge deal, but I would have just put a comment at the > caller like: > > /* > * Use slow_virt_to_phys() instead of vmalloc_to_page(), because it > * returns the PFN even for NP PTEs. > */ Yes, that comment is added in this patch. > > If someone is changing slow_virt_to_phys() they should check the > callers to make sure they won't break anything. They can see the > comment then and have an easy time. > > An optional comment at slow_virt_to_phys() could explain how the > function works in regards to the present bit, but not include details > about CoCoO VM page transition's usage of the present bit. The proposed > comment text looks like something more appropriate for a commit log. Kirill -- you originally asked for a comment in slow_virt_to_phys(). [1] Are you OK with Rick's proposal? Michael [1] https://lore.kernel.org/lkml/20230828221333.5blshosyqafbgwlc@xxxxxxxxxxxxxxxxx/
