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.
> 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.
