On Fri, 22 Jan 2016 15:21:05 +0100
Paolo Bonzini <pbonz...@redhat.com> wrote:
> On 22/01/2016 11:02, Igor Mammedov wrote:
> > On Thu, 21 Jan 2016 12:37:51 +1100
> > David Gibson <da...@gibson.dropbear.id.au> wrote:
> >
> >> The 'base' field of MemoryHotplugState is ram_addr_t, which indicates that
> >> it exists in the abstract address space of RAM regions.
> >>
> >> However, the actual usage of this field indicates that it is a concrete
> >> physical address (it's passed as an offset to memory_region_add_subgregion
> >> for example).
> >>
> >> So, correct its type to 'hwaddr'.
> >>
> >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> >> ---
> >> include/hw/mem/pc-dimm.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> >> index d83bf30..218dfb0 100644
> >> --- a/include/hw/mem/pc-dimm.h
> >> +++ b/include/hw/mem/pc-dimm.h
> >> @@ -77,7 +77,7 @@ typedef struct PCDIMMDeviceClass {
> >> * @mr: hotplug memory address space container
> >> */
> >> typedef struct MemoryHotplugState {
> >> - ram_addr_t base;
> >> + hwaddr base;
> >> MemoryRegion mr;
> >> } MemoryHotplugState;
> >>
> >
> > I agree with this fix but that's not the only place where
> > ram_addr_t needs to be replaced with hwaddr.
> > For example type of MachineState.[max]ram_size fields needs
> > to be changed as well. Because QEMU builds without CONFIG_XEN_BACKEND
> > on 32-bit hosts are broken since ram_addr_t is 32-bits there
> > while some targets assume and use it as 64-bit one.
>
> But on a 32-bit system without CONFIG_XEN_BACKEND you cannot allocate
> more than 4G anyway, so the choice of ram_addr_t is understandable in
> that case.
QEMU build will probably fail with above config but if it succeeds
then maxmem will be silently truncated.
>
> On the other hand, on a 32-bit system without CONFIG_XEN_BACKEND you
> definitely can place 128M of hot plugged memory between say 4096MB and
> 4224MB.
True.
Anyway for this patch
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
>
> Paolo
>
