| Message ID | 20130528141922.135a6dd0@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 28 May 2013 14:19:22 -0400 Luiz Capitulino <lcapitulino@redhat.com> wrote: > The code used to walk IA-32e page-tables, and possibly PAE page-tables, > uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. > > However, as we use a uint64_t to store the resulting address, that mask > gets expanded to 0xfffffffffffff000 which not only ends up selecting > reserved bits but also selects the XD bit (execute-disable) which > happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. > > This commit fixes that problem by replacing ~0xfff by a correct mask > that only selects the address bit range (ie. bits 51:12). > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> Ping? Wen? Would be nice get a Reviewed-by before merging... > --- > > PS: I (obviously) don't any more core dumps with this patch applied, but > I couldn't check if the Windows dump is correct (does anyone know > how to do this?). I did quickly check on Linux though. > > target-i386/arch_memory_mapping.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c > index 844893f..24884bd 100644 > --- a/target-i386/arch_memory_mapping.c > +++ b/target-i386/arch_memory_mapping.c > @@ -75,6 +75,8 @@ static void walk_pte2(MemoryMappingList *list, > } > > /* PAE Paging or IA-32e Paging */ > +#define PLM4_ADDR_MASK 0xffffffffff000 /* selects bits 51:12 */ > + > static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, > int32_t a20_mask, target_ulong start_line_addr) > { > @@ -105,7 +107,7 @@ static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, > continue; > } > > - pte_start_addr = (pde & ~0xfff) & a20_mask; > + pte_start_addr = (pde & PLM4_ADDR_MASK) & a20_mask; > walk_pte(list, pte_start_addr, a20_mask, line_addr); > } > } > @@ -208,7 +210,7 @@ static void walk_pdpe(MemoryMappingList *list, > continue; > } > > - pde_start_addr = (pdpe & ~0xfff) & a20_mask; > + pde_start_addr = (pdpe & PLM4_ADDR_MASK) & a20_mask; > walk_pde(list, pde_start_addr, a20_mask, line_addr); > } > } > @@ -231,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list, > } > > line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48); > - pdpe_start_addr = (pml4e & ~0xfff) & a20_mask; > + pdpe_start_addr = (pml4e & PLM4_ADDR_MASK) & a20_mask; > walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr); > } > } > @@ -249,7 +251,7 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) > if (env->hflags & HF_LMA_MASK) { > hwaddr pml4e_addr; > > - pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask; > + pml4e_addr = (env->cr[3] & PLM4_ADDR_MASK) & env->a20_mask; > walk_pml4e(list, pml4e_addr, env->a20_mask); > } else > #endif
On 05/30/13 14:59, Luiz Capitulino wrote: > On Tue, 28 May 2013 14:19:22 -0400 > Luiz Capitulino <lcapitulino@redhat.com> wrote: > >> The code used to walk IA-32e page-tables, and possibly PAE page-tables, >> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. >> >> However, as we use a uint64_t to store the resulting address, that mask >> gets expanded to 0xfffffffffffff000 which not only ends up selecting >> reserved bits but also selects the XD bit (execute-disable) which >> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. >> >> This commit fixes that problem by replacing ~0xfff by a correct mask >> that only selects the address bit range (ie. bits 51:12). >> >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > Ping? Wen? > > Would be nice get a Reviewed-by before merging... I didn't miss your submission and did find it OK, I just felt unsure about stating so, because "simple" patches like this are prime territory to burn someone's R-b's worth (ie. to expose a reviewer's lack of information / experience). But hey, what can I lose? The patch does look good to me, so Reviewed-by: Laszlo Ersek <lersek@redhat.com> > >> --- >> >> PS: I (obviously) don't any more core dumps with this patch applied, but >> I couldn't check if the Windows dump is correct (does anyone know >> how to do this?). I did quickly check on Linux though. >> >> target-i386/arch_memory_mapping.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c >> index 844893f..24884bd 100644 >> --- a/target-i386/arch_memory_mapping.c >> +++ b/target-i386/arch_memory_mapping.c >> @@ -75,6 +75,8 @@ static void walk_pte2(MemoryMappingList *list, >> } >> >> /* PAE Paging or IA-32e Paging */ >> +#define PLM4_ADDR_MASK 0xffffffffff000 /* selects bits 51:12 */ >> + >> static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, >> int32_t a20_mask, target_ulong start_line_addr) >> { >> @@ -105,7 +107,7 @@ static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, >> continue; >> } >> >> - pte_start_addr = (pde & ~0xfff) & a20_mask; >> + pte_start_addr = (pde & PLM4_ADDR_MASK) & a20_mask; >> walk_pte(list, pte_start_addr, a20_mask, line_addr); >> } >> } >> @@ -208,7 +210,7 @@ static void walk_pdpe(MemoryMappingList *list, >> continue; >> } >> >> - pde_start_addr = (pdpe & ~0xfff) & a20_mask; >> + pde_start_addr = (pdpe & PLM4_ADDR_MASK) & a20_mask; >> walk_pde(list, pde_start_addr, a20_mask, line_addr); >> } >> } >> @@ -231,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list, >> } >> >> line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48); >> - pdpe_start_addr = (pml4e & ~0xfff) & a20_mask; >> + pdpe_start_addr = (pml4e & PLM4_ADDR_MASK) & a20_mask; >> walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr); >> } >> } >> @@ -249,7 +251,7 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) >> if (env->hflags & HF_LMA_MASK) { >> hwaddr pml4e_addr; >> >> - pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask; >> + pml4e_addr = (env->cr[3] & PLM4_ADDR_MASK) & env->a20_mask; >> walk_pml4e(list, pml4e_addr, env->a20_mask); >> } else >> #endif > >
On Thu, 30 May 2013 15:16:18 +0200 Laszlo Ersek <lersek@redhat.com> wrote: > On 05/30/13 14:59, Luiz Capitulino wrote: > > On Tue, 28 May 2013 14:19:22 -0400 > > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > > >> The code used to walk IA-32e page-tables, and possibly PAE page-tables, > >> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. > >> > >> However, as we use a uint64_t to store the resulting address, that mask > >> gets expanded to 0xfffffffffffff000 which not only ends up selecting > >> reserved bits but also selects the XD bit (execute-disable) which > >> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. > >> > >> This commit fixes that problem by replacing ~0xfff by a correct mask > >> that only selects the address bit range (ie. bits 51:12). > >> > >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > > > Ping? Wen? > > > > Would be nice get a Reviewed-by before merging... > > I didn't miss your submission and did find it OK, I just felt unsure > about stating so, because "simple" patches like this are prime territory > to burn someone's R-b's worth (ie. to expose a reviewer's lack of > information / experience). But hey, what can I lose? The patch does look > good to me, so Thank you Laszlo! It's also new territory for me, that's why I'm asking for reviews (otherwise I'd just sneak it in some pull request :-) > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > > > >> --- > >> > >> PS: I (obviously) don't any more core dumps with this patch applied, but > >> I couldn't check if the Windows dump is correct (does anyone know > >> how to do this?). I did quickly check on Linux though. > >> > >> target-i386/arch_memory_mapping.c | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c > >> index 844893f..24884bd 100644 > >> --- a/target-i386/arch_memory_mapping.c > >> +++ b/target-i386/arch_memory_mapping.c > >> @@ -75,6 +75,8 @@ static void walk_pte2(MemoryMappingList *list, > >> } > >> > >> /* PAE Paging or IA-32e Paging */ > >> +#define PLM4_ADDR_MASK 0xffffffffff000 /* selects bits 51:12 */ > >> + > >> static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, > >> int32_t a20_mask, target_ulong start_line_addr) > >> { > >> @@ -105,7 +107,7 @@ static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, > >> continue; > >> } > >> > >> - pte_start_addr = (pde & ~0xfff) & a20_mask; > >> + pte_start_addr = (pde & PLM4_ADDR_MASK) & a20_mask; > >> walk_pte(list, pte_start_addr, a20_mask, line_addr); > >> } > >> } > >> @@ -208,7 +210,7 @@ static void walk_pdpe(MemoryMappingList *list, > >> continue; > >> } > >> > >> - pde_start_addr = (pdpe & ~0xfff) & a20_mask; > >> + pde_start_addr = (pdpe & PLM4_ADDR_MASK) & a20_mask; > >> walk_pde(list, pde_start_addr, a20_mask, line_addr); > >> } > >> } > >> @@ -231,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list, > >> } > >> > >> line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48); > >> - pdpe_start_addr = (pml4e & ~0xfff) & a20_mask; > >> + pdpe_start_addr = (pml4e & PLM4_ADDR_MASK) & a20_mask; > >> walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr); > >> } > >> } > >> @@ -249,7 +251,7 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) > >> if (env->hflags & HF_LMA_MASK) { > >> hwaddr pml4e_addr; > >> > >> - pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask; > >> + pml4e_addr = (env->cr[3] & PLM4_ADDR_MASK) & env->a20_mask; > >> walk_pml4e(list, pml4e_addr, env->a20_mask); > >> } else > >> #endif > > > > >
Am 30.05.2013 15:16, schrieb Luiz Capitulino: > On Thu, 30 May 2013 15:16:18 +0200 > Laszlo Ersek <lersek@redhat.com> wrote: > >> On 05/30/13 14:59, Luiz Capitulino wrote: >>> On Tue, 28 May 2013 14:19:22 -0400 >>> Luiz Capitulino <lcapitulino@redhat.com> wrote: >>> >>>> The code used to walk IA-32e page-tables, and possibly PAE page-tables, >>>> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. >>>> >>>> However, as we use a uint64_t to store the resulting address, that mask >>>> gets expanded to 0xfffffffffffff000 which not only ends up selecting >>>> reserved bits but also selects the XD bit (execute-disable) which >>>> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. >>>> >>>> This commit fixes that problem by replacing ~0xfff by a correct mask >>>> that only selects the address bit range (ie. bits 51:12). >>>> >>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> >>> >>> Ping? Wen? >>> >>> Would be nice get a Reviewed-by before merging... >> >> I didn't miss your submission and did find it OK, I just felt unsure >> about stating so, because "simple" patches like this are prime territory >> to burn someone's R-b's worth (ie. to expose a reviewer's lack of >> information / experience). But hey, what can I lose? The patch does look >> good to me, so > > Thank you Laszlo! It's also new territory for me, that's why I'm asking > for reviews (otherwise I'd just sneak it in some pull request :-) Luiz, you aware aware that I have another fix by Nuohan queued that seemed orthogonal? If someone reviews my refactoring series (which resent that patch) I would like to send out a PULL for that rather soon, since it blocks further CPU work. I would then include your fix as well to avoid merge conflicts. Andreas > >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> >>> >>>> --- >>>> >>>> PS: I (obviously) don't any more core dumps with this patch applied, but >>>> I couldn't check if the Windows dump is correct (does anyone know >>>> how to do this?). I did quickly check on Linux though. >>>> >>>> target-i386/arch_memory_mapping.c | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c >>>> index 844893f..24884bd 100644 >>>> --- a/target-i386/arch_memory_mapping.c >>>> +++ b/target-i386/arch_memory_mapping.c >>>> @@ -75,6 +75,8 @@ static void walk_pte2(MemoryMappingList *list, >>>> } >>>> >>>> /* PAE Paging or IA-32e Paging */ >>>> +#define PLM4_ADDR_MASK 0xffffffffff000 /* selects bits 51:12 */ >>>> + >>>> static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, >>>> int32_t a20_mask, target_ulong start_line_addr) >>>> { >>>> @@ -105,7 +107,7 @@ static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, >>>> continue; >>>> } >>>> >>>> - pte_start_addr = (pde & ~0xfff) & a20_mask; >>>> + pte_start_addr = (pde & PLM4_ADDR_MASK) & a20_mask; >>>> walk_pte(list, pte_start_addr, a20_mask, line_addr); >>>> } >>>> } >>>> @@ -208,7 +210,7 @@ static void walk_pdpe(MemoryMappingList *list, >>>> continue; >>>> } >>>> >>>> - pde_start_addr = (pdpe & ~0xfff) & a20_mask; >>>> + pde_start_addr = (pdpe & PLM4_ADDR_MASK) & a20_mask; >>>> walk_pde(list, pde_start_addr, a20_mask, line_addr); >>>> } >>>> } >>>> @@ -231,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list, >>>> } >>>> >>>> line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48); >>>> - pdpe_start_addr = (pml4e & ~0xfff) & a20_mask; >>>> + pdpe_start_addr = (pml4e & PLM4_ADDR_MASK) & a20_mask; >>>> walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr); >>>> } >>>> } >>>> @@ -249,7 +251,7 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) >>>> if (env->hflags & HF_LMA_MASK) { >>>> hwaddr pml4e_addr; >>>> >>>> - pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask; >>>> + pml4e_addr = (env->cr[3] & PLM4_ADDR_MASK) & env->a20_mask; >>>> walk_pml4e(list, pml4e_addr, env->a20_mask); >>>> } else >>>> #endif >>> >>> >> >
On Thu, 30 May 2013 16:10:28 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 30.05.2013 15:16, schrieb Luiz Capitulino: > > On Thu, 30 May 2013 15:16:18 +0200 > > Laszlo Ersek <lersek@redhat.com> wrote: > > > >> On 05/30/13 14:59, Luiz Capitulino wrote: > >>> On Tue, 28 May 2013 14:19:22 -0400 > >>> Luiz Capitulino <lcapitulino@redhat.com> wrote: > >>> > >>>> The code used to walk IA-32e page-tables, and possibly PAE page-tables, > >>>> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. > >>>> > >>>> However, as we use a uint64_t to store the resulting address, that mask > >>>> gets expanded to 0xfffffffffffff000 which not only ends up selecting > >>>> reserved bits but also selects the XD bit (execute-disable) which > >>>> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. > >>>> > >>>> This commit fixes that problem by replacing ~0xfff by a correct mask > >>>> that only selects the address bit range (ie. bits 51:12). > >>>> > >>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > >>> > >>> Ping? Wen? > >>> > >>> Would be nice get a Reviewed-by before merging... > >> > >> I didn't miss your submission and did find it OK, I just felt unsure > >> about stating so, because "simple" patches like this are prime territory > >> to burn someone's R-b's worth (ie. to expose a reviewer's lack of > >> information / experience). But hey, what can I lose? The patch does look > >> good to me, so > > > > Thank you Laszlo! It's also new territory for me, that's why I'm asking > > for reviews (otherwise I'd just sneak it in some pull request :-) > > Luiz, you aware aware that I have another fix by Nuohan queued that > seemed orthogonal? Yes, they should be orthogonal. > If someone reviews my refactoring series (which > resent that patch) I would like to send out a PULL for that rather soon, > since it blocks further CPU work. I would then include your fix as well > to avoid merge conflicts. Thanks, although I was going to include it in my tomorrow's QMP pull request. Will this disturb your work?
Am 30.05.2013 16:14, schrieb Luiz Capitulino: > On Thu, 30 May 2013 16:10:28 +0200 > Andreas Färber <afaerber@suse.de> wrote: > >> Am 30.05.2013 15:16, schrieb Luiz Capitulino: >>> On Thu, 30 May 2013 15:16:18 +0200 >>> Laszlo Ersek <lersek@redhat.com> wrote: >>> >>>> On 05/30/13 14:59, Luiz Capitulino wrote: >>>>> On Tue, 28 May 2013 14:19:22 -0400 >>>>> Luiz Capitulino <lcapitulino@redhat.com> wrote: >>>>> >>>>>> The code used to walk IA-32e page-tables, and possibly PAE page-tables, >>>>>> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. >>>>>> >>>>>> However, as we use a uint64_t to store the resulting address, that mask >>>>>> gets expanded to 0xfffffffffffff000 which not only ends up selecting >>>>>> reserved bits but also selects the XD bit (execute-disable) which >>>>>> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. >>>>>> >>>>>> This commit fixes that problem by replacing ~0xfff by a correct mask >>>>>> that only selects the address bit range (ie. bits 51:12). >>>>>> >>>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> >>>>> >>>>> Ping? Wen? >>>>> >>>>> Would be nice get a Reviewed-by before merging... >>>> >>>> I didn't miss your submission and did find it OK, I just felt unsure >>>> about stating so, because "simple" patches like this are prime territory >>>> to burn someone's R-b's worth (ie. to expose a reviewer's lack of >>>> information / experience). But hey, what can I lose? The patch does look >>>> good to me, so >>> >>> Thank you Laszlo! It's also new territory for me, that's why I'm asking >>> for reviews (otherwise I'd just sneak it in some pull request :-) >> >> Luiz, you aware aware that I have another fix by Nuohan queued that >> seemed orthogonal? > > Yes, they should be orthogonal. > >> If someone reviews my refactoring series (which >> resent that patch) I would like to send out a PULL for that rather soon, >> since it blocks further CPU work. I would then include your fix as well >> to avoid merge conflicts. > > Thanks, although I was going to include it in my tomorrow's QMP pull > request. Will this disturb your work? I hope not - could you then please pick up Nuohan's bugfix as well? Still I'd be interested in your review - and I have one more patch to add that I created offline yesterday wrt first paging-enabled CPU. Andreas
On Thu, 30 May 2013 16:22:32 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 30.05.2013 16:14, schrieb Luiz Capitulino: > > On Thu, 30 May 2013 16:10:28 +0200 > > Andreas Färber <afaerber@suse.de> wrote: > > > >> Am 30.05.2013 15:16, schrieb Luiz Capitulino: > >>> On Thu, 30 May 2013 15:16:18 +0200 > >>> Laszlo Ersek <lersek@redhat.com> wrote: > >>> > >>>> On 05/30/13 14:59, Luiz Capitulino wrote: > >>>>> On Tue, 28 May 2013 14:19:22 -0400 > >>>>> Luiz Capitulino <lcapitulino@redhat.com> wrote: > >>>>> > >>>>>> The code used to walk IA-32e page-tables, and possibly PAE page-tables, > >>>>>> uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. > >>>>>> > >>>>>> However, as we use a uint64_t to store the resulting address, that mask > >>>>>> gets expanded to 0xfffffffffffff000 which not only ends up selecting > >>>>>> reserved bits but also selects the XD bit (execute-disable) which > >>>>>> happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. > >>>>>> > >>>>>> This commit fixes that problem by replacing ~0xfff by a correct mask > >>>>>> that only selects the address bit range (ie. bits 51:12). > >>>>>> > >>>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > >>>>> > >>>>> Ping? Wen? > >>>>> > >>>>> Would be nice get a Reviewed-by before merging... > >>>> > >>>> I didn't miss your submission and did find it OK, I just felt unsure > >>>> about stating so, because "simple" patches like this are prime territory > >>>> to burn someone's R-b's worth (ie. to expose a reviewer's lack of > >>>> information / experience). But hey, what can I lose? The patch does look > >>>> good to me, so > >>> > >>> Thank you Laszlo! It's also new territory for me, that's why I'm asking > >>> for reviews (otherwise I'd just sneak it in some pull request :-) > >> > >> Luiz, you aware aware that I have another fix by Nuohan queued that > >> seemed orthogonal? > > > > Yes, they should be orthogonal. > > > >> If someone reviews my refactoring series (which > >> resent that patch) I would like to send out a PULL for that rather soon, > >> since it blocks further CPU work. I would then include your fix as well > >> to avoid merge conflicts. > > > > Thanks, although I was going to include it in my tomorrow's QMP pull > > request. Will this disturb your work? > > I hope not - could you then please pick up Nuohan's bugfix as well? Sure. > Still I'd be interested in your review - and I have one more patch to > add that I created offline yesterday wrt first paging-enabled CPU. You can CC me.
Am 28.05.2013 20:19, schrieb Luiz Capitulino: > The code used to walk IA-32e page-tables, and possibly PAE page-tables, > uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. > > However, as we use a uint64_t to store the resulting address, that mask > gets expanded to 0xfffffffffffff000 which not only ends up selecting > reserved bits but also selects the XD bit (execute-disable) which > happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. > > This commit fixes that problem by replacing ~0xfff by a correct mask > that only selects the address bit range (ie. bits 51:12). > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> Reviewed-by: Andreas Färber <afaerber@suse.de> But please add a target-i386: prefix when queuing it. Andreas
diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c index 844893f..24884bd 100644 --- a/target-i386/arch_memory_mapping.c +++ b/target-i386/arch_memory_mapping.c @@ -75,6 +75,8 @@ static void walk_pte2(MemoryMappingList *list, } /* PAE Paging or IA-32e Paging */ +#define PLM4_ADDR_MASK 0xffffffffff000 /* selects bits 51:12 */ + static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, int32_t a20_mask, target_ulong start_line_addr) { @@ -105,7 +107,7 @@ static void walk_pde(MemoryMappingList *list, hwaddr pde_start_addr, continue; } - pte_start_addr = (pde & ~0xfff) & a20_mask; + pte_start_addr = (pde & PLM4_ADDR_MASK) & a20_mask; walk_pte(list, pte_start_addr, a20_mask, line_addr); } } @@ -208,7 +210,7 @@ static void walk_pdpe(MemoryMappingList *list, continue; } - pde_start_addr = (pdpe & ~0xfff) & a20_mask; + pde_start_addr = (pdpe & PLM4_ADDR_MASK) & a20_mask; walk_pde(list, pde_start_addr, a20_mask, line_addr); } } @@ -231,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list, } line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48); - pdpe_start_addr = (pml4e & ~0xfff) & a20_mask; + pdpe_start_addr = (pml4e & PLM4_ADDR_MASK) & a20_mask; walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr); } } @@ -249,7 +251,7 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) if (env->hflags & HF_LMA_MASK) { hwaddr pml4e_addr; - pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask; + pml4e_addr = (env->cr[3] & PLM4_ADDR_MASK) & env->a20_mask; walk_pml4e(list, pml4e_addr, env->a20_mask); } else #endif
The code used to walk IA-32e page-tables, and possibly PAE page-tables, uses the bit mask ~0xfff to get the next PML4E/PDPTE/PDE/PTE address. However, as we use a uint64_t to store the resulting address, that mask gets expanded to 0xfffffffffffff000 which not only ends up selecting reserved bits but also selects the XD bit (execute-disable) which happens to be enabled by Windows 8, causing qemu_get_ram_ptr() to abort. This commit fixes that problem by replacing ~0xfff by a correct mask that only selects the address bit range (ie. bits 51:12). Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- PS: I (obviously) don't any more core dumps with this patch applied, but I couldn't check if the Windows dump is correct (does anyone know how to do this?). I did quickly check on Linux though. target-i386/arch_memory_mapping.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)