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

[Huacai Chen]

Hi, Rechard and Peter,

On Wed, Dec 2, 2020 at 5:32 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 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.
Any sugguestions? Other patches are updated, except this one.

Huacai
>
> >
> > Huacai