[Qemu-devel] Re: [RESEND #3][PATCH] i386: Catch all non-present ptes in cpu_get_phys_page_debug

[Jan Kiszka <jan.kiszka@web.de>]

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> It helps debugging guests when yet unmapped pages are correctly reported
>> as, well, unmapped.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> ---
>>  target-i386/helper.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> Index: b/target-i386/helper.c
>> ===================================================================
>> --- a/target-i386/helper.c
>> +++ b/target-i386/helper.c
>> @@ -1192,6 +1192,8 @@ target_phys_addr_t cpu_get_phys_page_deb
>>              page_size = 4096;
>>              pte = ldq_phys(pte_addr);
>>          }
>> +        if (!(pte & PG_PRESENT_MASK))
>> +            return -1;
>>      } else {
>>          uint32_t pde;
>>  
>> @@ -1211,10 +1213,10 @@ target_phys_addr_t cpu_get_phys_page_deb
>>                  /* page directory entry */
>>                  pte_addr = ((pde & ~0xfff) + ((addr >> 10) & 0xffc))
>> & env->a20_mask;
>>                  pte = ldl_phys(pte_addr);
>> -                if (!(pte & PG_PRESENT_MASK))
>> -                    return -1;
>>                  page_size = 4096;
>>              }
>> +            if (!(pte & PG_PRESENT_MASK))
>> +                return -1;
>>          }
>>   
> 
> This looks redundant.  In the PSE=1 case, the check for pde &
> PG_PRESENT_MASK takes care of it.  In the PSE=0 clause, there is already
> a check for pte & PG_PRESENT_MASK.

True.

> 
> The first chunk looks like a genuine fix though.

Here is the stripped-down version:

-----------

It helps debugging guests when yet unmapped pages are correctly reported
as, well, unmapped.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 target-i386/helper.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: b/target-i386/helper.c
===================================================================
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1192,6 +1192,8 @@ target_phys_addr_t cpu_get_phys_page_deb
             page_size = 4096;
             pte = ldq_phys(pte_addr);
         }
+        if (!(pte & PG_PRESENT_MASK))
+            return -1;
     } else {
         uint32_t pde;