Re: [PATCH V9 6/6] hw/mips: Add Loongson-3 machine support

[Philippe Mathieu-Daudé]

On 9/16/20 11:49 AM, Huacai Chen wrote:
> Hi, Philippe,
> 
> On Wed, Sep 16, 2020 at 3:56 PM Philippe Mathieu-Daudé <f4bug@amsat.org> 
> wrote:
>>
>> Hi Huacai,
>>
>> On 9/16/20 4:12 AM, Huacai Chen wrote:
...
>> hw/mips/loongson3_virt.c:373:15: note: each undeclared identifier is
>> reported only once for each function it appears in
>> hw/mips/loongson3_virt.c:373:15: error: excess elements in struct
>> initializer [-Werror]
>> hw/mips/loongson3_virt.c:373:15: note: (near initialization for 'freq_reg')
>> hw/mips/loongson3_virt.c:374:9: error: unknown field 'addr' specified in
>> initializer
>>          .addr = (uintptr_t)(&freq)
>>          ^
>> hw/mips/loongson3_virt.c:374:17: error: excess elements in struct
>> initializer [-Werror]
>>          .addr = (uintptr_t)(&freq)
>>                  ^
>> hw/mips/loongson3_virt.c:374:17: note: (near initialization for 'freq_reg')
>> hw/mips/loongson3_virt.c:372:24: error: storage size of 'freq_reg' isn't
>> known
>>      struct kvm_one_reg freq_reg = {
>>                         ^
>> hw/mips/loongson3_virt.c:380:41: error: 'KVM_GET_ONE_REG' undeclared
>> (first use in this function)
>>          ret = kvm_vcpu_ioctl(first_cpu, KVM_GET_ONE_REG, &freq_reg);
>>                                          ^
>> hw/mips/loongson3_virt.c:372:24: error: unused variable 'freq_reg'
>> [-Werror=unused-variable]
>>      struct kvm_one_reg freq_reg = {
>>                         ^
>> hw/mips/loongson3_virt.c: In function 'init_loongson_params':
>> hw/mips/loongson3_virt.c:467:25: error: cast from pointer to integer of
>> different size [-Werror=pointer-to-int-cast]
>>      lp->memory_offset = (unsigned long long)init_memory_map(p)
>>                          ^
> I guess this happens on a 32bit platform where pointer is 32bit, and
> could you please suggest a best solution for this? Maybe use uintptr_t
> instead of unsigned long long?

Since the machine doesn't have to know the EFI structures
layout, I'd change your method to fill EFI structures as i.e.:

/*
 * @ptr: Pointer to fill
 * @size: Buffer size available at @ptr
 * Returns: Structure size filled on success, -1 on error.
 */
size_t fill_efi_memory_map_loongson(char *ptr, size_t size);

And move that to hw/mips/loongson_efi.{c,h}.

Then you don't need to worry about host pointer size, you just
exchange buffer/size, then caller can round up and increment an
offset.

BTW the EFI helpers are not endian safe.

You should use the helpers described in docs/devel/loads-stores.rst:

stw_le_p, stl_le_p(), ... (as I don't expect big-endian guests).

Regards,

Phil.