On Wed, Dec 10, 2025 at 10:55:50AM +0100, Jan Beulich wrote: > On 10.12.2025 10:42, Roger Pau Monné wrote: > > On Tue, Apr 26, 2022 at 12:26:10PM +0200, Jan Beulich wrote: > >> Just like for PV guests MMU_MACHPHYS_UPDATE implies marking of the > >> respective page as dirty, additions to a HVM guest's P2M should do so. > >> > >> For HVM the opposite is also true: Pages being removed from the P2M are > >> no longer dirty at their prior GFN; there's no point in telling the tool > >> stack to try and copy that page, when this will fail anyway (until > >> perhaps a new page gets placed there). Introduce paging_mark_pfn_clean() > >> (intentionally without a paging_mark_clean() counterpart) to handle > >> this. Note that while there is an earlier call to set_gpfn_from_mfn() in > >> guest_physmap_add_entry(), but there's little reason to mark the page > >> clean there when later in the function it'll be marked dirty. This is > >> even more so given that at this point it's only the M2P that gets > >> updated, with the P2M still left unchanged. > >> > >> Signed-off-by: Jan Beulich <[email protected]> > > > > Reviewed-by: Roger Pau Monné <[email protected]> > > Thanks. > > >> --- a/xen/arch/x86/mm/paging.c > >> +++ b/xen/arch/x86/mm/paging.c > >> @@ -260,7 +260,7 @@ static int paging_log_dirty_disable(stru > >> } > >> > >> /* Mark a page as dirty, with taking guest pfn as parameter */ > >> -void paging_mark_pfn_dirty(struct domain *d, pfn_t pfn) > >> +static void mark_pfn_dirty(struct domain *d, pfn_t pfn, bool dirty) > > > > Nit: set_pfn_logdirty() or similar? The function name makes it look like > > it's marking the pfn as dirty (when it's also used to clear it). > > > > No strong opinion, it just seems to me the name is slightly confusing. > > Changed. While making the change I noticed that I had one other change in > there for a possible v4. This is the extra hunk: > > @@ -260,7 +260,7 @@ void paging_mark_pfn_dirty(struct domain > return; > > /* Shared MFNs should NEVER be marked dirty */ > - BUG_ON(paging_mode_translate(d) && SHARED_M2P(pfn_x(pfn))); > + BUG_ON(dirty && paging_mode_translate(d) && SHARED_M2P(pfn_x(pfn))); > > /* > * Values with the MSB set denote MFNs that aren't really part of the > > I hope that won't invalidate your R-b.
No, that's fine, please keep the RB. Thanks, Roger.
