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

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

2020-12-15 Thread Philippe Mathieu-Daudé

Le mar. 15 déc. 2020 06:34, Huacai Chen  a écrit :

> Hi, Philippe,
>
> On Mon, Dec 14, 2020 at 9:49 PM Philippe Mathieu-Daudé 
> wrote:
> >
> > On 12/14/20 3:37 AM, Huacai Chen wrote:
> > > Hi, Philippe,
> > >
> > > On Mon, Dec 14, 2020 at 7:09 AM Philippe Mathieu-Daudé <
> f4...@amsat.org> wrote:
> > >>
> > >> On 12/13/20 11:17 PM, Philippe Mathieu-Daudé wrote:
> > >>> 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é <
> f4...@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é <
> f4...@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é 
> > > Signed-off-by: Huacai Chen 
> > > Co-developed-by: Jiaxun Yang 
> > > Signed-off-by: Jiaxun Yang 
> > > ---
> > >  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 000..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 (che...@lemote.com)
> > > + * Copyright (c) 2018-2020 Jiaxun Yang <
> jiaxun.y...@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(>cputype, Loongson_3A);
> > > +stl_le_p(>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(>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

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

2020-12-14 Thread Huacai Chen

Hi, Philippe,

On Mon, Dec 14, 2020 at 9:49 PM Philippe Mathieu-Daudé  wrote:
>
> On 12/14/20 3:37 AM, Huacai Chen wrote:
> > Hi, Philippe,
> >
> > On Mon, Dec 14, 2020 at 7:09 AM Philippe Mathieu-Daudé  
> > wrote:
> >>
> >> On 12/13/20 11:17 PM, Philippe Mathieu-Daudé wrote:
> >>> 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é  
> > 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é 
> >>>  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é 
> > Signed-off-by: Huacai Chen 
> > Co-developed-by: Jiaxun Yang 
> > Signed-off-by: Jiaxun Yang 
> > ---
> >  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 000..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 (che...@lemote.com)
> > + * Copyright (c) 2018-2020 Jiaxun Yang 
> > + *
> > + * 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 
> > .
> > + */
> > +
> > +#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(>cputype, Loongson_3A);
> > +stl_le_p(>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(>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.
> >>
> >> Well, this works:
> >>
> >> -- >8 --
> >> @@ -32,7 +32,7 @@ static struct efi_cpuinfo_loongson *init_cpu_info(void
> >> *g_cpuinfo, uint64_t cpu_
> >>  struct

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

2020-12-14 Thread Philippe Mathieu-Daudé

On 12/14/20 3:37 AM, Huacai Chen wrote:
> Hi, Philippe,
> 
> On Mon, Dec 14, 2020 at 7:09 AM Philippe Mathieu-Daudé  
> wrote:
>>
>> On 12/13/20 11:17 PM, Philippe Mathieu-Daudé wrote:
>>> 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é  
> 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é 
>>>  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é 
> Signed-off-by: Huacai Chen 
> Co-developed-by: Jiaxun Yang 
> Signed-off-by: Jiaxun Yang 
> ---
>  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 000..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 (che...@lemote.com)
> + * Copyright (c) 2018-2020 Jiaxun Yang 
> + *
> + * 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 
> .
> + */
> +
> +#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(>cputype, Loongson_3A);
> +stl_le_p(>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(>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.
>>
>> Well, this works:
>>
>> -- >8 --
>> @@ -32,7 +32,7 @@ static struct efi_cpuinfo_loongson *init_cpu_info(void
>> *g_cpuinfo, uint64_t cpu_
>>  struct efi_cpuinfo_loongson *c = g_cpuinfo;
>>
>>  stl_le_p(>cputype, Loongson_3A);
>> -stl_le_p(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
>> +c->processor_id = cpu_to_le32(MIPS_CPU(first_cpu)->env.CP0_PRid);
>>  if (cpu_freq > UINT_MAX) {
>>  stl_le_p(>cpu_clock_freq, UINT_MAX);
>>  } else {
> 
> This seems not allowed. In include/qemu/bswap.h it

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

2020-12-13 Thread Huacai Chen

Hi, Philippe,

On Mon, Dec 14, 2020 at 7:09 AM Philippe Mathieu-Daudé  wrote:
>
> On 12/13/20 11:17 PM, Philippe Mathieu-Daudé wrote:
> > 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é  
> >>> 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é 
> >  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é 
> >>> Signed-off-by: Huacai Chen 
> >>> Co-developed-by: Jiaxun Yang 
> >>> Signed-off-by: Jiaxun Yang 
> >>> ---
> >>>  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 000..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 (che...@lemote.com)
> >>> + * Copyright (c) 2018-2020 Jiaxun Yang 
> >>> + *
> >>> + * 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 
> >>> .
> >>> + */
> >>> +
> >>> +#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(>cputype, Loongson_3A);
> >>> +stl_le_p(>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(>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.
>
> Well, this works:
>
> -- >8 --
> @@ -32,7 +32,7 @@ static struct efi_cpuinfo_loongson *init_cpu_info(void
> *g_cpuinfo, uint64_t cpu_
>  struct efi_cpuinfo_loongson *c = g_cpuinfo;
>
>  stl_le_p(>cputype, Loongson_3A);
> -stl_le_p(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
> +c->processor_id = cpu_to_le32(MIPS_CPU(first_cpu)->env.CP0_PRid);
>  if (cpu_freq > UINT_MAX) {
>  stl_le_p(>cpu_clock_freq, UINT_MAX);
>  } else {

This seems not allowed. In include/qemu/bswap.h it says:
 * Do an in-place conversion of the value pointed to by @v from

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

2020-12-13 Thread Philippe Mathieu-Daudé

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é 
> Signed-off-by: Huacai Chen 
> Co-developed-by: Jiaxun Yang 
> Signed-off-by: Jiaxun Yang 
> ---
>  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 000..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 (che...@lemote.com)
> + * Copyright (c) 2018-2020 Jiaxun Yang 
> + *
> + * 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 .
> + */
> +
> +#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(>cputype, Loongson_3A);
> +stl_le_p(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
> +if (cpu_freq > UINT_MAX) {
> +stl_le_p(>cpu_clock_freq, UINT_MAX);
> +} else {
> +stl_le_p(>cpu_clock_freq, cpu_freq);
> +}
> +
> +stw_le_p(>cpu_startup_core_id, 0);
> +stl_le_p(>nr_cpus, current_machine->smp.cpus);
> +stl_le_p(>total_node, DIV_ROUND_UP(current_machine->smp.cpus,
> +  LOONGSON3_CORE_PER_NODE));
> +
> +return c;
> +}
> +
> +static struct efi_memory_map_loongson *init_memory_map(void *g_map, uint64_t 
> ram_size)
> +{
> +struct efi_memory_map_loongson *emap = g_map;
> +
> +stl_le_p(>nr_map, 2);
> +stl_le_p(>mem_freq, 3);
> +
> +stl_le_p(>map[0].node_id, 0);
> +stl_le_p(>map[0].mem_type, 1);
> +stq_le_p(>map[0].mem_start, 0x0);
> +stl_le_p(>map[0].mem_size, 240);
> +
> +stl_le_p(>map[1].node_id, 0);
> +stl_le_p(>map[1].mem_type, 2);
> +stq_le_p(>map[1].mem_start, 0x9000);
> +stl_le_p(>map[1].mem_size, (ram_size / MiB) - 256);
> +
> +return emap;
> +}
> +
> +static struct system_loongson *init_system_loongson(void *g_system)
> +{
> +struct system_loongson *s = g_system;
> +
> +stl_le_p(>ccnuma_smp, 0);
> +stl_le_p(>sing_double_channel, 1);
> +stl_le_p(>nr_uarts, 1);
> +stl_le_p(>uarts[0].iotype, 2);
> +stl_le_p(>uarts[0].int_offset, 2);
> +stl_le_p(>uarts[0].uartclk, 2500); /* Random value */
> +stq_le_p(>uarts[0].uart_base, virt_memmap[VIRT_UART].base);
> +
> +return s;
> +}
> +
> +static struct irq_source_routing_table *init_irq_source(void *g_irq_source)
> +{
> +struct irq_source_routing_table *irq_info = g_irq_source;
> +
> +stl_le_p(_info->node_id, 0);
> +stl_le_p(_info->PIC_type, 0);
> +stw_le_p(_info->dma_mask_bits, 64);
> +stq_le_p(_info->pci_mem_start_addr, 
> virt_memmap[VIRT_PCIE_MMIO].base);
> +stq_le_p(_info->pci_mem_end_addr, virt_memmap[VIRT_PCIE_MMIO].base +
> +  virt_memmap[VIRT_PCIE_MMIO].size - 
> 1);
> +stq_le_p(_info->pci_io_start_addr, virt_memmap[VIRT_PCIE_PIO].base);
> +
> +return irq_info;
> +}
> +
> +static struct interface_info *init_interface_info(void *g_interface)
> +{
> +struct interface_info *interface = g_interface;
> +
> +stw_le_p(>vers, 0x01);
> +strpadcpy(interface->description, 64, "UEFI_Version_v1.0", '\0');
> +
> +return interface;
> +}
> +
> +static struct board_devices *board_devices_info(void *g_board)
> +{
> +struct board_devices *bd = g_board;
> +
> +strpadcpy(bd->name, 64, "Loongson-3A-VIRT-1w-V1.00-demo", '\0');
> +
> +return bd;
> +}
> +
> +static struct loongson_special_attribute *init_special_info(void *g_special)
> +{
> +struct loongson_special_attribute *special = g_special;
> +
> +

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

2020-12-13 Thread Philippe Mathieu-Daudé

On 12/13/20 11:17 PM, Philippe Mathieu-Daudé wrote:
> 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é  
>>> 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é  
> 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é 
>>> Signed-off-by: Huacai Chen 
>>> Co-developed-by: Jiaxun Yang 
>>> Signed-off-by: Jiaxun Yang 
>>> ---
>>>  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 000..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 (che...@lemote.com)
>>> + * Copyright (c) 2018-2020 Jiaxun Yang 
>>> + *
>>> + * 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 
>>> .
>>> + */
>>> +
>>> +#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(>cputype, Loongson_3A);
>>> +stl_le_p(>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(>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.

Well, this works:

-- >8 --
@@ -32,7 +32,7 @@ static struct efi_cpuinfo_loongson *init_cpu_info(void
*g_cpuinfo, uint64_t cpu_
 struct efi_cpuinfo_loongson *c = g_cpuinfo;

 stl_le_p(>cputype, Loongson_3A);
-stl_le_p(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
+c->processor_id = cpu_to_le32(MIPS_CPU(first_cpu)->env.CP0_PRid);
 if (cpu_freq > UINT_MAX) {
 stl_le_p(>cpu_clock_freq, UINT_MAX);
 } else {
---

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

2020-12-13 Thread Philippe Mathieu-Daudé

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é  
>> 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é  
 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é 
>> Signed-off-by: Huacai Chen 
>> Co-developed-by: Jiaxun Yang 
>> Signed-off-by: Jiaxun Yang 
>> ---
>>  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 000..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 (che...@lemote.com)
>> + * Copyright (c) 2018-2020 Jiaxun Yang 
>> + *
>> + * 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 .
>> + */
>> +
>> +#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(>cputype, Loongson_3A);
>> +stl_le_p(>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(>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

2020-12-11 Thread Philippe Mathieu-Daudé

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é  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é  
>>> 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é 
> Signed-off-by: Huacai Chen 
> Co-developed-by: Jiaxun Yang 
> Signed-off-by: Jiaxun Yang 
> ---
>  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 000..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 (che...@lemote.com)
> + * Copyright (c) 2018-2020 Jiaxun Yang 
> + *
> + * 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 .
> + */
> +
> +#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(>cputype, Loongson_3A);
> +stl_le_p(>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(>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?

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

2020-12-10 Thread Huacai Chen

Hi, Rechard and Peter,

On Wed, Dec 2, 2020 at 5:32 PM Philippe Mathieu-Daudé  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é  
> > 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é 
> >>> Signed-off-by: Huacai Chen 
> >>> Co-developed-by: Jiaxun Yang 
> >>> Signed-off-by: Jiaxun Yang 
> >>> ---
> >>>  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 000..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 (che...@lemote.com)
> >>> + * Copyright (c) 2018-2020 Jiaxun Yang 
> >>> + *
> >>> + * 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 .
> >>> + */
> >>> +
> >>> +#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(>cputype, Loongson_3A);
> >>> +stl_le_p(>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(>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

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

2020-12-02 Thread 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é  
> 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é 
>>> Signed-off-by: Huacai Chen 
>>> Co-developed-by: Jiaxun Yang 
>>> Signed-off-by: Jiaxun Yang 
>>> ---
>>>  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 000..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 (che...@lemote.com)
>>> + * Copyright (c) 2018-2020 Jiaxun Yang 
>>> + *
>>> + * 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 .
>>> + */
>>> +
>>> +#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(>cputype, Loongson_3A);
>>> +stl_le_p(>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(>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

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

2020-12-01 Thread Huacai Chen

Hi, Phillippe,

On Tue, Nov 24, 2020 at 6:25 AM Philippe Mathieu-Daudé  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é 
> > Signed-off-by: Huacai Chen 
> > Co-developed-by: Jiaxun Yang 
> > Signed-off-by: Jiaxun Yang 
> > ---
> >  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 000..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 (che...@lemote.com)
> > + * Copyright (c) 2018-2020 Jiaxun Yang 
> > + *
> > + * 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 .
> > + */
> > +
> > +#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(>cputype, Loongson_3A);
> > +stl_le_p(>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(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
>   ^~~
> 1 error generated.
We cannot reproduce it on X86/MIPS with clang... And I found that
stl_le_p() will be __builtin_memcpy(), I don't think memcpy() will
cause unaligned access. So, any suggestions?

Huacai
>
> > +if (cpu_freq > UINT_MAX) {
> > +stl_le_p(>cpu_clock_freq, UINT_MAX);
> > +} else {
> > +stl_le_p(>cpu_clock_freq, cpu_freq);
> > +}
> > +
> > +stw_le_p(>cpu_startup_core_id, 0);
> > +stl_le_p(>nr_cpus, current_machine->smp.cpus);
> > +stl_le_p(>total_node, DIV_ROUND_UP(current_machine->smp.cpus,
> > +  LOONGSON3_CORE_PER_NODE));
> > +
> > +return c;
> > +}
> > +
> > +static struct efi_memory_map_loongson *init_memory_map(void *g_map, 
> > uint64_t ram_size)
> > +{
> > +struct efi_memory_map_loongson *emap = g_map;
> > +
> > +stl_le_p(>nr_map, 2);
> > +stl_le_p(>mem_freq, 3);
> > +
> > +stl_le_p(>map[0].node_id, 0);
> > +stl_le_p(>map[0].mem_type, 1);
> > +stq_le_p(>map[0].mem_start, 0x0);
> > +stl_le_p(>map[0].mem_size, 240);
> > +
> > +stl_le_p(>map[1].node_id, 0);
> > +stl_le_p(>map[1].mem_type, 2);
> > +stq_le_p(>map[1].mem_start, 0x9000);
> > +stl_le_p(>map[1].mem_size, (ram_size / MiB) - 256);
> > +
> > +return emap;
> > +}
> > +
> > +static struct system_loongson *init_system_loongson(void *g_system)
> > +{
> > +struct system_loongson *s = g_system;
> > +
> > +stl_le_p(>ccnuma_smp, 0);
> > +stl_le_p(>sing_double_channel, 1);
> > +stl_le_p(>nr_uarts, 1);
> > +stl_le_p(>uarts[0].iotype, 2);
> > +stl_le_p(>uarts[0].int_offset, 2);
> > +stl_le_p(>uarts[0].uartclk, 2500); /* Random value */
> > +stq_le_p(>uarts[0].uart_base, virt_memmap[VIRT_UART].base);
> > +
> > +return s;
> > +}
> > +
> > +static struct irq_source_routing_table *init_irq_source(void *g_irq_source)
> > +{
> > +struct irq_source_routing_table *irq_info = g_irq_source;
> > +
> > +stl_le_p(_info->node_id, 0);
> > +stl_le_p(_info->PIC_type, 0);
> > +stw_le_p(_info->dma_mask_bits, 64);
> > +stq_le_p(_info->pci_mem_start_addr, 
> > virt_memmap[VIRT_PCIE_MMIO].base);
> > +

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

2020-11-23 Thread Philippe Mathieu-Daudé

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é 
> Signed-off-by: Huacai Chen 
> Co-developed-by: Jiaxun Yang 
> Signed-off-by: Jiaxun Yang 
> ---
>  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 000..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 (che...@lemote.com)
> + * Copyright (c) 2018-2020 Jiaxun Yang 
> + *
> + * 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 .
> + */
> +
> +#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(>cputype, Loongson_3A);
> +stl_le_p(>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(>processor_id, MIPS_CPU(first_cpu)->env.CP0_PRid);
  ^~~
1 error generated.

> +if (cpu_freq > UINT_MAX) {
> +stl_le_p(>cpu_clock_freq, UINT_MAX);
> +} else {
> +stl_le_p(>cpu_clock_freq, cpu_freq);
> +}
> +
> +stw_le_p(>cpu_startup_core_id, 0);
> +stl_le_p(>nr_cpus, current_machine->smp.cpus);
> +stl_le_p(>total_node, DIV_ROUND_UP(current_machine->smp.cpus,
> +  LOONGSON3_CORE_PER_NODE));
> +
> +return c;
> +}
> +
> +static struct efi_memory_map_loongson *init_memory_map(void *g_map, uint64_t 
> ram_size)
> +{
> +struct efi_memory_map_loongson *emap = g_map;
> +
> +stl_le_p(>nr_map, 2);
> +stl_le_p(>mem_freq, 3);
> +
> +stl_le_p(>map[0].node_id, 0);
> +stl_le_p(>map[0].mem_type, 1);
> +stq_le_p(>map[0].mem_start, 0x0);
> +stl_le_p(>map[0].mem_size, 240);
> +
> +stl_le_p(>map[1].node_id, 0);
> +stl_le_p(>map[1].mem_type, 2);
> +stq_le_p(>map[1].mem_start, 0x9000);
> +stl_le_p(>map[1].mem_size, (ram_size / MiB) - 256);
> +
> +return emap;
> +}
> +
> +static struct system_loongson *init_system_loongson(void *g_system)
> +{
> +struct system_loongson *s = g_system;
> +
> +stl_le_p(>ccnuma_smp, 0);
> +stl_le_p(>sing_double_channel, 1);
> +stl_le_p(>nr_uarts, 1);
> +stl_le_p(>uarts[0].iotype, 2);
> +stl_le_p(>uarts[0].int_offset, 2);
> +stl_le_p(>uarts[0].uartclk, 2500); /* Random value */
> +stq_le_p(>uarts[0].uart_base, virt_memmap[VIRT_UART].base);
> +
> +return s;
> +}
> +
> +static struct irq_source_routing_table *init_irq_source(void *g_irq_source)
> +{
> +struct irq_source_routing_table *irq_info = g_irq_source;
> +
> +stl_le_p(_info->node_id, 0);
> +stl_le_p(_info->PIC_type, 0);
> +stw_le_p(_info->dma_mask_bits, 64);
> +stq_le_p(_info->pci_mem_start_addr, 
> virt_memmap[VIRT_PCIE_MMIO].base);
> +stq_le_p(_info->pci_mem_end_addr, virt_memmap[VIRT_PCIE_MMIO].base +
> +  virt_memmap[VIRT_PCIE_MMIO].size - 
> 1);
> +stq_le_p(_info->pci_io_start_addr, virt_memmap[VIRT_PCIE_PIO].base);
> +
> +return irq_info;
> +}
> +
> +static struct interface_info *init_interface_info(void *g_interface)
> +{
> +struct interface_info *interface = g_interface;
> +
> +stw_le_p(>vers, 0x01);
> +strpadcpy(interface->description, 64, "UEFI_Version_v1.0", '\0');
> +
> +return

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

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

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

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

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

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

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

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

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

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

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

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

12 matches

Site Navigation

Mail list logo

Footer information