Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers

[Philippe Mathieu-Daudé]

On 12/2/20 2:14 AM, Huacai Chen wrote:
> Hi, Phillippe,
> 
> On Tue, Nov 24, 2020 at 6:25 AM Philippe Mathieu-Daudé <f4bug@amsat.org> 
> wrote:
>>
>> On 11/6/20 5:21 AM, Huacai Chen wrote:
>>> Preparing to add Loongson-3 machine support, add Loongson-3's LEFI (a
>>> UEFI-like interface for BIOS-Kernel boot parameters) helpers first.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>>> Co-developed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>  hw/mips/loongson3_bootp.c | 165 +++++++++++++++++++++++++++++++
>>>  hw/mips/loongson3_bootp.h | 241 
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/mips/meson.build       |   1 +
>>>  3 files changed, 407 insertions(+)
>>>  create mode 100644 hw/mips/loongson3_bootp.c
>>>  create mode 100644 hw/mips/loongson3_bootp.h
>>>
>>> diff --git a/hw/mips/loongson3_bootp.c b/hw/mips/loongson3_bootp.c
>>> new file mode 100644
>>> index 0000000..3a16081
>>> --- /dev/null
>>> +++ b/hw/mips/loongson3_bootp.c
>>> @@ -0,0 +1,165 @@
>>> +/*
>>> + * LEFI (a UEFI-like interface for BIOS-Kernel boot parameters) helpers
>>> + *
>>> + * Copyright (c) 2018-2020 Huacai Chen (chenhc@lemote.com)
>>> + * Copyright (c) 2018-2020 Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> + *
>>> + * This program is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <https://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/units.h"
>>> +#include "qemu/cutils.h"
>>> +#include "cpu.h"
>>> +#include "hw/boards.h"
>>> +#include "hw/mips/loongson3_bootp.h"
>>> +
>>> +#define LOONGSON3_CORE_PER_NODE 4
>>> +
>>> +static struct efi_cpuinfo_loongson *init_cpu_info(void *g_cpuinfo, 
>>> uint64_t cpu_freq)
>>> +{
>>> +    struct efi_cpuinfo_loongson *c = g_cpuinfo;
>>> +
>>> +    stl_le_p(&c->cputype, Loongson_3A);
>>> +    stl_le_p(&c->processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
>>
>> Build failing with Clang:
>>
>> FAILED: libqemu-mips64el-softmmu.fa.p/hw_mips_loongson3_bootp.c.o
>> hw/mips/loongson3_bootp.c:35:15: error: taking address of packed member
>> 'processor_id' of class or structure 'efi_cpuinfo_loongson' may result
>> in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
>>     stl_le_p(&c->processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
>>               ^~~~~~~~~~~~~~~
>> 1 error generated.
> We cannot reproduce it on X86/MIPS with clang...

You can reproduce running the Clang job on Gitlab-CI:

https://wiki.qemu.org/Testing/CI/GitLabCI

> And I found that
> stl_le_p() will be __builtin_memcpy(), I don't think memcpy() will
> cause unaligned access. So, any suggestions?

I'll defer this question to Richard/Peter who have deeper understanding.

> 
> Huacai