| Message ID | 50D55DB2.6080601@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
Wen Congyang <wency@cn.fujitsu.com> writes: > ((pde & 0x1fe000) << 19) is the bits 39:32 of the final physical address, and > we shouldn't use unit32_t to calculate it. Convert the type to hwaddr to fix > this problem. Spotted by Coverity. > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > > --- > target-i386/arch_memory_mapping.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c > index c6c7874..844893f 100644 > --- a/target-i386/arch_memory_mapping.c > +++ b/target-i386/arch_memory_mapping.c > @@ -115,7 +115,7 @@ static void walk_pde2(MemoryMappingList *list, > hwaddr pde_start_addr, int32_t a20_mask, > bool pse) > { > - hwaddr pde_addr, pte_start_addr, start_paddr; > + hwaddr pde_addr, pte_start_addr, start_paddr, high_paddr; > uint32_t pde; > target_ulong line_addr, start_vaddr; > int i; > @@ -130,8 +130,13 @@ static void walk_pde2(MemoryMappingList *list, > > line_addr = (((unsigned int)i & 0x3ff) << 22); > if ((pde & PG_PSE_MASK) && pse) { > - /* 4 MB page */ > - start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19); > + /* > + * 4 MB page: > + * bits 39:32 are bits 20:13 of the PDE > + * bit3 31:22 are bits 31:22 of the PDE > + */ > + high_paddr = ((hwaddr)(pde & 0x1fe000) << 19); > + start_paddr = (pde & ~0x3fffff) | high_paddr; > if (cpu_physical_memory_is_io(start_paddr)) { > /* I/O region */ > continue; Extra points for extending the comment. Reviewed-by: Markus Armbruster <armbru@redhat.com>
Am 07.01.2013 13:06, schrieb Markus Armbruster: > Wen Congyang <wency@cn.fujitsu.com> writes: > >> ((pde & 0x1fe000) << 19) is the bits 39:32 of the final physical address, and >> we shouldn't use unit32_t to calculate it. Convert the type to hwaddr to fix >> this problem. > > Spotted by Coverity. > >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> >> --- >> target-i386/arch_memory_mapping.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c >> index c6c7874..844893f 100644 >> --- a/target-i386/arch_memory_mapping.c >> +++ b/target-i386/arch_memory_mapping.c >> @@ -115,7 +115,7 @@ static void walk_pde2(MemoryMappingList *list, >> hwaddr pde_start_addr, int32_t a20_mask, >> bool pse) >> { >> - hwaddr pde_addr, pte_start_addr, start_paddr; >> + hwaddr pde_addr, pte_start_addr, start_paddr, high_paddr; >> uint32_t pde; >> target_ulong line_addr, start_vaddr; >> int i; >> @@ -130,8 +130,13 @@ static void walk_pde2(MemoryMappingList *list, >> >> line_addr = (((unsigned int)i & 0x3ff) << 22); >> if ((pde & PG_PSE_MASK) && pse) { >> - /* 4 MB page */ >> - start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19); >> + /* >> + * 4 MB page: >> + * bits 39:32 are bits 20:13 of the PDE >> + * bit3 31:22 are bits 31:22 of the PDE >> + */ >> + high_paddr = ((hwaddr)(pde & 0x1fe000) << 19); >> + start_paddr = (pde & ~0x3fffff) | high_paddr; >> if (cpu_physical_memory_is_io(start_paddr)) { >> /* I/O region */ >> continue; > > Extra points for extending the comment. ...and a "target-i386: " prefix in the subject would be appreciated, since it does not seem to fix a general issue. Andreas > > Reviewed-by: Markus Armbruster <armbru@redhat.com> >
On Mon, 07 Jan 2013 14:10:46 +0100 Andreas Färber <afaerber@suse.de> wrote: > Am 07.01.2013 13:06, schrieb Markus Armbruster: > > Wen Congyang <wency@cn.fujitsu.com> writes: > > > >> ((pde & 0x1fe000) << 19) is the bits 39:32 of the final physical address, and > >> we shouldn't use unit32_t to calculate it. Convert the type to hwaddr to fix > >> this problem. > > > > Spotted by Coverity. > > > >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > >> > >> --- > >> target-i386/arch_memory_mapping.c | 11 ++++++++--- > >> 1 file changed, 8 insertions(+), 3 deletions(-) > >> > >> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c > >> index c6c7874..844893f 100644 > >> --- a/target-i386/arch_memory_mapping.c > >> +++ b/target-i386/arch_memory_mapping.c > >> @@ -115,7 +115,7 @@ static void walk_pde2(MemoryMappingList *list, > >> hwaddr pde_start_addr, int32_t a20_mask, > >> bool pse) > >> { > >> - hwaddr pde_addr, pte_start_addr, start_paddr; > >> + hwaddr pde_addr, pte_start_addr, start_paddr, high_paddr; > >> uint32_t pde; > >> target_ulong line_addr, start_vaddr; > >> int i; > >> @@ -130,8 +130,13 @@ static void walk_pde2(MemoryMappingList *list, > >> > >> line_addr = (((unsigned int)i & 0x3ff) << 22); > >> if ((pde & PG_PSE_MASK) && pse) { > >> - /* 4 MB page */ > >> - start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19); > >> + /* > >> + * 4 MB page: > >> + * bits 39:32 are bits 20:13 of the PDE > >> + * bit3 31:22 are bits 31:22 of the PDE > >> + */ > >> + high_paddr = ((hwaddr)(pde & 0x1fe000) << 19); > >> + start_paddr = (pde & ~0x3fffff) | high_paddr; > >> if (cpu_physical_memory_is_io(start_paddr)) { > >> /* I/O region */ > >> continue; > > > > Extra points for extending the comment. > > ...and a "target-i386: " prefix in the subject would be appreciated, > since it does not seem to fix a general issue. I can do that myself when I apply the patch (which will take a little bit, as I'm just back from vacation). > > Andreas > > > > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > > >
On Sat, 22 Dec 2012 15:13:54 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote: > ((pde & 0x1fe000) << 19) is the bits 39:32 of the final physical address, and > we shouldn't use unit32_t to calculate it. Convert the type to hwaddr to fix > this problem. > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Applied to the qmp branch, thanks. > > --- > target-i386/arch_memory_mapping.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c > index c6c7874..844893f 100644 > --- a/target-i386/arch_memory_mapping.c > +++ b/target-i386/arch_memory_mapping.c > @@ -115,7 +115,7 @@ static void walk_pde2(MemoryMappingList *list, > hwaddr pde_start_addr, int32_t a20_mask, > bool pse) > { > - hwaddr pde_addr, pte_start_addr, start_paddr; > + hwaddr pde_addr, pte_start_addr, start_paddr, high_paddr; > uint32_t pde; > target_ulong line_addr, start_vaddr; > int i; > @@ -130,8 +130,13 @@ static void walk_pde2(MemoryMappingList *list, > > line_addr = (((unsigned int)i & 0x3ff) << 22); > if ((pde & PG_PSE_MASK) && pse) { > - /* 4 MB page */ > - start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19); > + /* > + * 4 MB page: > + * bits 39:32 are bits 20:13 of the PDE > + * bit3 31:22 are bits 31:22 of the PDE > + */ > + high_paddr = ((hwaddr)(pde & 0x1fe000) << 19); > + start_paddr = (pde & ~0x3fffff) | high_paddr; > if (cpu_physical_memory_is_io(start_paddr)) { > /* I/O region */ > continue;
diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c index c6c7874..844893f 100644 --- a/target-i386/arch_memory_mapping.c +++ b/target-i386/arch_memory_mapping.c @@ -115,7 +115,7 @@ static void walk_pde2(MemoryMappingList *list, hwaddr pde_start_addr, int32_t a20_mask, bool pse) { - hwaddr pde_addr, pte_start_addr, start_paddr; + hwaddr pde_addr, pte_start_addr, start_paddr, high_paddr; uint32_t pde; target_ulong line_addr, start_vaddr; int i; @@ -130,8 +130,13 @@ static void walk_pde2(MemoryMappingList *list, line_addr = (((unsigned int)i & 0x3ff) << 22); if ((pde & PG_PSE_MASK) && pse) { - /* 4 MB page */ - start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19); + /* + * 4 MB page: + * bits 39:32 are bits 20:13 of the PDE + * bit3 31:22 are bits 31:22 of the PDE + */ + high_paddr = ((hwaddr)(pde & 0x1fe000) << 19); + start_paddr = (pde & ~0x3fffff) | high_paddr; if (cpu_physical_memory_is_io(start_paddr)) { /* I/O region */ continue;
((pde & 0x1fe000) << 19) is the bits 39:32 of the final physical address, and we shouldn't use unit32_t to calculate it. Convert the type to hwaddr to fix this problem. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- target-i386/arch_memory_mapping.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)