On Fri, Jan 12, 2024 at 07:24:35PM +0000, Michael Kelley wrote: > 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? Yes, sounds sensible. -- Kiryl Shutsemau / Kirill A. Shutemov
