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.
Jan
