On Thu, Feb 18, 2021 at 10:22 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Thu, 18 Feb 2021 at 14:07, Bin Meng <bmeng...@gmail.com> wrote:
> > On Thu, Feb 18, 2021 at 9:26 PM Peter Maydell <peter.mayd...@linaro.org>
> > wrote:
> > > Fails to compile, 32 bit hosts:
> > >
> > > ../../hw/riscv/virt.c: In function 'virt_machine_init':
> > > ../../hw/riscv/virt.c:621:43: error: comparison is always false due to
> > > limited range of data type [-Werror=type-limits]
> > > if ((uint64_t)(machine->ram_size) > 10 * GiB) {
> > > ^
> > > ../../hw/riscv/virt.c:623:33: error: large integer implicitly
> > > truncated to unsigned type [-Werror=overflow]
> > > machine->ram_size = 10 * GiB;
> > > ^~
> >
> > This kind of error is tricky. I wonder whether we should deprecate
> > 32-bit host support though.
>
> 32-bit host is still not uncommon outside the x86 world...
>
> The thing that makes this particular check awkward is that
> machine->ram_size is a ram_addr_t, whose size is 64 bits if
> either (a) the host is 64 bits or (b) CONFIG_XEN_BACKEND is
> enabled, so it's effectively only 32-bits on 32-bit-not-x86.
>
> It might be a good idea if we decided that we would just make
> ram_addr_t 64-bits everywhere, to avoid this kind of "we
> have an unusual config only on some more-obscure hosts" issue.
> (We did that for hwaddr back in commit 4be403c8158e1 in 2012,
> when it was still called target_phys_addr_t.) This change
> would probably be a performance hit for 32-bit-non-x86 hosts;
> it would be interesting to see whether it was measurably
> significant.
Okay, will send a patch to change ram_addr_t to 64-bit.
Regards,
Bin
