Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling

[Bernhard Beschow <shentey@gmail.com>]

Am 29. Oktober 2022 09:29:33 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
>
>
>On 10/28/22 19:42, Philippe Mathieu-Daudé wrote:
>> On 28/10/22 17:09, Daniel Henrique Barboza wrote:
>>> Bernhard,
>>> 
>>> The 32 builds aren't fancying this patch. The issue is down there:
>>> 
>>> On 10/18/22 18:01, Bernhard Beschow wrote:
>>>> Allows e500 boards to have their root file system reside on flash using
>>>> only builtin devices located in the eLBC memory region.
>>>> 
>>>> Note that the flash memory area is only created when a -pflash argument is
>>>> given, and that the size is determined by the given file. The idea is to
>>>> put users into control.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>>   docs/system/ppc/ppce500.rst | 16 ++++++++
>>>>   hw/ppc/Kconfig              |  1 +
>>>>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 96 insertions(+)
>> 
>>>> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>>>>                                   pmc->platform_bus_base,
>>>>                                   &pms->pbus_dev->mmio);
>>>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>>>> +    if (dinfo) {
>>>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>>>> +        BlockDriverState *bs = blk_bs(blk);
>>>> +        uint64_t size = bdrv_getlength(bs);
>>>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
>>> 
>>> ^ here. The issue is that on a 32 bit system it is not possible to cast the
>>> Int128 type to uint64_t:
>>> 
>>> FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
>>> 3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
>>> 3747../hw/ppc/e500.c: In function 'ppce500_init':
>>> 3748../hw/ppc/e500.c:1069:30: error: incompatible types when initializing type 'uint64_t' {aka 'long long unsigned int'} using type 'Int128'
>>> 3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
>>> 3750      |                              ^~~
>>> 3751[3207/5331] Compiling C object libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o
>>> 
>>> 
>>> What I did to solve the problem is this:
>>> 
>>> 
>>> +         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size); >
>>> This will get the lower 64 bits and return an uint64_t.
>>> 
>>> Note that this function will assert if mmio.size is bigger than UINT64_MAX, but
>>> since you're doing an error(1) on the "if size > mmio_size" conditional, this
>>> assert() is not introducing a new side effect. We'll just fail earlier with
>>> a different error message.
>> 
>> Simply use:
>> 
>>    memory_region_size(pms->pbus_dev->mmio);
>
>Nice! I'll change it in-tree before re-sending the PR.

Thanks Daniel!

Bernhard
>
>
>Daniel
>
>>