Re: [PATCH v2 2/2] KVM: arm64: ptdump: Flush the last region

[Dev Jain]

On 25/07/26 10:58 pm, Marc Zyngier wrote:
> On Sat, 25 Jul 2026 18:16:37 +0100,
> Dev Jain <dev.jain@xxxxxxx> wrote:
>>
>>
>>
>> On 25/07/26 12:24 am, Wei-Lin Chang wrote:
>>> Currently the stage-2 ptdump calls note_page() at each leaf entry visit.
>>> This simply misses the output of the last region, because note_page()
>>> only dumps output when it detects a change in level/prot, or when the
>>> walk enters a next marker section. The last region in the guest IPA
>>> space with the same level/prot is not dumped since there is no change
>>> after it.
>>>
>>> Call note_page_flush() to dump the final region. note_page_flush()
>>> expects ptdump_state.range[] to be initialized to the range of the
>>> address space, so also do that.
>>>
>>> Also change the second marker's start address to ULONG_MAX so we never
>>> cross it. This avoids dumping redundant marker names (which are NULL),
>>> and advancing beyond the end of the marker array.
>>>
>>> Fixes: 7c4f73548ed1 ("KVM: arm64: Register ptdump with debugfs on guest creation")
>>> Reported-by: Sashiko AI <sashiko-bot@xxxxxxxxxx>
>>> Closes: https://lore.kernel.org/kvmarm/20260630122758.891011F00A3A@xxxxxxxxxxxxxxx/
>>> Signed-off-by: Wei-Lin Chang <weilin.chang@xxxxxxx>
>>> ---
>>>  arch/arm64/kvm/ptdump.c | 16 ++++++++++++----
>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/ptdump.c b/arch/arm64/kvm/ptdump.c
>>> index c9140e22abcf..222577163d2b 100644
>>> --- a/arch/arm64/kvm/ptdump.c
>>> +++ b/arch/arm64/kvm/ptdump.c
>>> @@ -130,7 +130,7 @@ static struct kvm_ptdump_guest_state *kvm_ptdump_parser_create(struct kvm_s2_mmu
>>>  	}
>>>  
>>>  	st->ipa_marker[0].name		= "Guest IPA";
>>> -	st->ipa_marker[1].start_address = BIT(pgtable->ia_bits);
>>> +	st->ipa_marker[1].start_address = ULONG_MAX;
>>>  
>>>  	st->mmu				= mmu;
>>>  	return st;
>>> @@ -148,18 +148,26 @@ static int kvm_ptdump_guest_show(struct seq_file *m, void *unused)
>>>  		.flags	= KVM_PGTABLE_WALK_LEAF,
>>>  	};
>>>  
>>> +	guard(write_lock)(&kvm->mmu_lock);
>>>  	st->parser_state = (struct ptdump_pg_state) {
>>>  		.marker		= &st->ipa_marker[0],
>>>  		.level		= -1,
>>>  		.pg_level	= &st->level[0],
>>>  		.seq		= m,
>>> +		.ptdump		= {
>>> +			.range = (struct ptdump_range[]){
>>> +				{0, BIT(mmu->pgt->ia_bits)},
>>> +				{0, 0}
>>> +			}
>>
>> I am having a real hard time reading that indentation :) you
>> could make it look like what we have currently in ptdump_walk().
> 
> This patch:
> 
> 		.ptdump		= {
> 			.range = (struct ptdump_range[]){
> 				{0, BIT(mmu->pgt->ia_bits)},
> 				{0, 0}
> 			}
> 		}
> 
> ptdump_walk():
> 
> 		.ptdump = {
> 			[...]
> 			.range = (struct ptdump_range[]){
> 				{info->base_addr, end},
> 				{0, 0}
> 			}
> 		}
> 
> Zero difference in indentation. Consider using a better email client,
> or look at the applied patches.

I did apply the patches with b4 :) I should have better explained myself.

ptdump_walk does not do a tab before the "=" . That looks better:


.ptdump = {
	[---]
	.range = (struct ptdump_range[]){
		{info->base_addr, end},
		{0, 0}
	}
}

whereas after this patch, kvm_ptdump_guest_show looks like:

.ptdump		= {
	.range = (struct ptdump_range[]){
		{0, BIT(mmu->pgt->ia_bits)},
		{0, 0}
	}
}


In the former, the range initialization starts almost below its corresponding
opening bracket. That looks better to me. Since kvm_ptdump_guest_show did not expect
"nested" declaration before, it looked fine, but after this patch you have a
combination of doing tabs before the equal sign, and not doing the tab in
".range = " .

So my nitpick would be to drop those tabs. Feel free to ignore if you deem this
as unnecessary churn.


> 
> 	M.
>