Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 18 Mar 2014 11:30:42 +0100
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2014-03-18 08:36, Paolo Bonzini wrote:
> > Il 18/03/2014 08:19, Jan Kiszka ha scritto:
> >> On 2014-03-18 02:54, Luiz Capitulino wrote:
> >>> If you start a Linux guest with more than 4GB of memory and try to
> >>> look at a
> >>> memory address, you will get an error from gdb:
> >>>
> >>> (gdb) p node_data[0]->node_id
> >>> Cannot access memory at address 0xffff88013fffd3a0
> >>> (gdb)
> >>
> >> I suppose this is x86-64, not 32-bit with PTE, right?
> >>
> >>>
> >>> I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't
> >>> handle the
> >>> case where the PDPTE has the PS bit set (although I didn't check
> >>> where Linux
> >>> sets that bit). This commit adds the PS bit handling, which fixes the
> >>> problem
> >>> for me.
> >>>
> >>> Signed-off-by: Luiz capitulino <lcapitulino@redhat.com>
> >>> ---
> >>>
> >>> Two observations:
> >>>
> >>>  1. This bug has always existed, so it's not a regression, so I'm not
> >>> sure
> >>>     it's worth it to fix for 2.0
> > 
> > Sure, why not?
> > 
> >>>  2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(),
> >>>     so I'm not completely sure this is the right thing to do
> >>>
> >>>  target-i386/helper.c | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/target-i386/helper.c b/target-i386/helper.c
> >>> index 4f447b8..9b7803f 100644
> >>> --- a/target-i386/helper.c
> >>> +++ b/target-i386/helper.c
> >>> @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs,
> >>> vaddr addr)
> >>>                  return -1;
> >>>          }
> >>>
> >>> +        if (pdpe & PG_PSE_MASK) {
> >>> +            page_size = 1024 * 1024 * 1024;
> >>> +            pte = pdpe & ~( (page_size - 1) & ~0xfff);
> >>> +            pte &= ~(PG_NX_MASK | PG_HI_USER_MASK);
> >>> +            goto out;
> >>> +        }
> >>
> >> Does this also apply if we are not in long mode?
> > 
> > No, it doesn't.  The only valid bits in a PAE PDPTE are P, PWT and PCD.
> >  Bit 7 (PS) is reserved.
> 
> Right, this belongs in the "if (env->hflags & HF_LMA_MASK)" block.
> 
> And the subject or description should mention that
> x86_cpu_get_phys_page_debug was lacking support for 1G hugepages.

To be honest, although the PS bit is set and that indicates a 1GB page,
I didn't know Linux does that. I thought Linux would use 4KB pages for
everything unless it's explicitly asked to use bigger pages. Also, note that
I was using gdb to debug really early kernel boot code (start_kernel()).

I'd feel more confident to have such a changelog after I find out where
exactly Linux sets that bit, but I won't have time in the next days. On the
other hand, the patch does fix the problem to me.