[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers |
Date: |
Sun, 13 Dec 2020 23:17:10 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 |
On 12/11/20 12:32 PM, Philippe Mathieu-Daudé wrote:
> On 12/11/20 3:46 AM, Huacai Chen wrote:
>> 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?
>
> My understanding is the compiler is complaining for the argument
> passed to the caller, with no knowledge of the callee implementation.
>
> Which makes me wonder if these functions are really inlined...
>
> Do we need to use QEMU_ALWAYS_INLINE for these LDST helpers?
No, this doesn't work neither.
>
> I see Richard used it in commit 80d9d1c6785 ("cputlb: Split out
> load/store_memop").
>
>>>
>>> I'll defer this question to Richard/Peter who have deeper understanding.
>> Any sugguestions? Other patches are updated, except this one.
>
> Searching on the list, I see Marc-André resolved that by
> using a copy on the stack:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg614482.html
>
>>
>> Huacai
>>>
>>>>
>>>> Huacai
>>
>
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Huacai Chen, 2020/12/01
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Philippe Mathieu-Daudé, 2020/12/02
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Huacai Chen, 2020/12/10
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Philippe Mathieu-Daudé, 2020/12/11
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers,
Philippe Mathieu-Daudé <=
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Philippe Mathieu-Daudé, 2020/12/13
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Huacai Chen, 2020/12/13
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Philippe Mathieu-Daudé, 2020/12/14
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Huacai Chen, 2020/12/15
- Re: [PATCH V17 4/6] hw/mips: Add Loongson-3 boot parameter helpers, Philippe Mathieu-Daudé, 2020/12/15