Le lun. 18 mars 2019 11:34, Marcel Apfelbaum <marcel.apfelb...@gmail.com> a
écrit :
> Hi Christian,
>
> On 3/18/19 11:27 AM, Christian Borntraeger wrote:
> >
> > On 16.03.19 12:09, Philippe Mathieu-Daudé wrote:
> >> Hi Marcel,
> >>
> >> On 3/16/19 10:50 AM, Marcel Apfelbaum wrote:
> >>> Configuring QEMU with:
> >>> configure --cc=clang --target-list=s390x-softmmu
> >>> And compiling it using a 32 bit machine leads to:
> >> Because there sizeof(ram_addr_t) = sizeof(uintptr_t) = 32.
> >>
> >>> hw/s390x/s390-virtio-ccw.c:175:27: error: implicit conversion from
> >>> 'unsigned long long' to 'ram_addr_t' (aka 'unsigned int')
> changes value
> >>> from 8796091973632 to 4293918720 [-Werror,-Wconstant-conversion]
> >>> chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> >>> ~ ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> >> The comment 1 line earlier is:
> >>
> >> /* KVM does not allow memslots >= 8 TB */
> >>
> >> Clang is correct, this KVM_SLOT_MAX_BYTES is incorrect on a 32bit s390,
> >> you need a 64bit system.
> > KVM is only supported on 64bit s390.
> >
>
> So maybe the fix I proposed is enough.
>
Enough to silent a warning due to a bug, as confirmed Christian KVM code
should be reachable on 32 bit hosts.
Safer would it be to fix the bug.
>
> >
> >> Per Hacking:
> >>
> >> Use hwaddr for guest physical addresses except pcibus_t
> >> for PCI addresses. In addition, ram_addr_t is a QEMU internal
> address
> >> space that maps guest RAM physical addresses into an intermediate
> >> address space that can map to host virtual address spaces. Generally
> >> speaking, the size of guest memory can always fit into ram_addr_t but
> >> it would not be correct to store an actual guest physical address in
> a
> >> ram_addr_t.
> >>
> >> My understanding is we should not use ram_addr_t with KVM but rather
> >> hwaddr, but I'm not sure.
> >>
> >> I don't know about s390, if 32bit host is supported or supports KVM.
> >> If it is, maybe this could work:
> >>
> >> I don't think the following is clean:
> >>
> >> #if TARGET_LONG_BITS == 32
> >> # define KVM_SLOT_MAX_BYTES RAM_ADDR_MAX
> >> #else
> >> # define KVM_SLOT_MAX_BYTES \
> >> ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & SEG_MSK)
> >> #endif
> >>
> >> But checking ifdef CONFIG_KVM might be clever:
> >>
> >> -- >8 --
> >> @@ -161,7 +161,7 @@ static void virtio_ccw_register_hcalls(void)
> >> static void s390_memory_init(ram_addr_t mem_size)
> >> {
> >> MemoryRegion *sysmem = get_system_memory();
> >> - ram_addr_t chunk, offset = 0;
> >> + hwaddr offset = 0;
> >> unsigned int number = 0;
> >> gchar *name;
> >>
> >> @@ -169,14 +169,16 @@ static void s390_memory_init(ram_addr_t mem_size)
> >> name = g_strdup_printf("s390.ram");
> >> while (mem_size) {
> >> MemoryRegion *ram = g_new(MemoryRegion, 1);
> >> - uint64_t size = mem_size;
> >> + uint64_t chunk_size = mem_size;
> >>
> >> +#ifdef CONFIG_KVM
> >> /* KVM does not allow memslots >= 8 TB */
> >> - chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> >> - memory_region_allocate_system_memory(ram, NULL, name, chunk);
> >> + chunk_size = MIN(mem_size, KVM_SLOT_MAX_BYTES);
> >> +#endif
> >> + memory_region_allocate_system_memory(ram, NULL, name,
> chunk_size);
> >> memory_region_add_subregion(sysmem, offset, ram);
> >> - mem_size -= chunk;
> >> - offset += chunk;
> >> + mem_size -= chunk_size;
> >> + offset += chunk_size;
> >> g_free(name);
> >> name = g_strdup_printf("s390.ram.%u", ++number);
> >> }
> >> ---
> >>
> >> Anyway s390x experts will figure that out ;)
> >
> > I will have a look.
> >
>
> Appreciated, I just need to be able to compile QEMU with clang on a
> 32bit machine.
> Any fix would do.
>
> Thanks,
> Marcel
>
>
>
>
>
