>-----Original Message-----
>From: Markus Armbruster <arm...@redhat.com>
>Sent: Friday, June 24, 2022 13:35
>To: Yao, Yuan <yuan....@intel.com>
>Cc: Paolo Bonzini <pbonz...@redhat.com>; Philippe Mathieu-Daudé
><f4...@amsat.org>; Dr. David Alan Gilbert
><dgilb...@redhat.com>; Zhong, Yang <yang.zh...@intel.com>; Connor Kuehl
><cku...@redhat.com>; qemu-devel@nongnu.org;
>Yamahata, Isaku <isaku.yamah...@intel.com>
>Subject: Re: [PATCH 1/1] i386/monitor: Fix page table walking issue for LA57
>enabled guest
>
>Yuan Yao <yuan....@intel.com> writes:
>
>> Don't skip next leve page table for pdpe/pde when the
>
>level
Sorry for my typo.
>
>> PG_PRESENT_MASK is set.
>>
>> This fixs the issue that no mapping information was
>
>fixes
>
>> collected from "info mem" for guest with LA57 enabled.
>>
>> Signed-off-by: Yuan Yao <yuan....@intel.com>
>
>Should we add
>
> Fixes: 6c7c3c21f95dd9af8a0691c0dd29b07247984122
Yes, I will add this next time, thanks.
>
>?
>
>> ---
>> target/i386/monitor.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
>> index 8e4b4d600c..3339550bbe 100644
>> --- a/target/i386/monitor.c
>> +++ b/target/i386/monitor.c
>> @@ -489,7 +489,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState
>> *env)
>> cpu_physical_memory_read(pdp_addr + l2 * 8, &pdpe, 8);
>> pdpe = le64_to_cpu(pdpe);
>> end = (l0 << 48) + (l1 << 39) + (l2 << 30);
>> - if (pdpe & PG_PRESENT_MASK) {
>> + if (!(pdpe & PG_PRESENT_MASK)) {
>> prot = 0;
>> mem_print(mon, env, &start, &last_prot, end, prot);
>> continue;
>> @@ -508,7 +508,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState
>> *env)
>> cpu_physical_memory_read(pd_addr + l3 * 8, &pde, 8);
>> pde = le64_to_cpu(pde);
>> end = (l0 << 48) + (l1 << 39) + (l2 << 30) + (l3 << 21);
>> - if (pde & PG_PRESENT_MASK) {
>> + if (!(pde & PG_PRESENT_MASK)) {
>> prot = 0;
>> mem_print(mon, env, &start, &last_prot, end, prot);
>> continue;
>>
>> base-commit: 6d940eff4734bcb40b1a25f62d7cec5a396f994a
>
>The commit message talks about not skipping something when the flag is
>set. However, the patch *flips* the sense of conditions, which means
>were *also* changing behavior when the flag is unset. How?
Yes, this also changes the behavior when the flag is unset, because the
original code does wrong for both set and unset , flips the checking
condition bring all of them to right behavior. I think I can add these
to the commit message in v2.
