[v2] mips: do not list individual devices from configs/

[PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Paolo Bonzini 2 years ago

itc_reconfigure() is referenced from TCG, provide a stub if needed.
This makes it possible to build a QEMU binary that only includes
boards without a CPS device (only Malta and Boston create one).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
 hw/mips/meson.build     |  1 +
 2 files changed, 27 insertions(+)
 create mode 100644 hw/mips/mips_itu-stub.c

diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
new file mode 100644
index 00000000000..4cc82b8461f
--- /dev/null
+++ b/hw/mips/mips_itu-stub.c
@@ -0,0 +1,26 @@
+/*
+ * Inter-Thread Communication Unit emulation.
+ *
+ * Copyright (c) 2016 Imagination Technologies
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/misc/mips_itu.h"
+
+void itc_reconfigure(MIPSITUState *tag)
+{
+    abort();
+}
diff --git a/hw/mips/meson.build b/hw/mips/meson.build
index f06d88f3430..2b1b96147a6 100644
--- a/hw/mips/meson.build
+++ b/hw/mips/meson.build
@@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
 mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c'))
 mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
 mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
+mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
 
 if 'CONFIG_TCG' in config_all_accel
 mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
-- 
2.43.0

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Philippe Mathieu-Daudé 2 years ago

Hi Paolo,

On 7/2/24 12:14, Paolo Bonzini wrote:
> itc_reconfigure() is referenced from TCG, provide a stub if needed.
> This makes it possible to build a QEMU binary that only includes
> boards without a CPS device (only Malta and Boston create one).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>   hw/mips/meson.build     |  1 +
>   2 files changed, 27 insertions(+)
>   create mode 100644 hw/mips/mips_itu-stub.c
> 
> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> new file mode 100644
> index 00000000000..4cc82b8461f
> --- /dev/null
> +++ b/hw/mips/mips_itu-stub.c
> @@ -0,0 +1,26 @@
> +/*
> + * Inter-Thread Communication Unit emulation.
> + *
> + * Copyright (c) 2016 Imagination Technologies
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.

This is your code addition, so "Inter-Thread Communication Unit stubs" /
RH / SPDX GPLv2-or-later.

> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/mips_itu.h"
> +
> +void itc_reconfigure(MIPSITUState *tag)
> +{
> +    abort();

As Zoltan suggested, g_assert_not_reached(). Indeed this path
can't be reached without ITU, TCG won't emit anything and will
call instead:

     qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
                   register_name, reg, sel);

I'm reluctant to add stubs, but since it helps you (hoping we
can figure a clean way to split architectural access to hw/ from
tcg/ one day):

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Paolo Bonzini 2 years ago

On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Paolo,
>
> On 7/2/24 12:14, Paolo Bonzini wrote:
> > itc_reconfigure() is referenced from TCG, provide a stub if needed.
> > This makes it possible to build a QEMU binary that only includes
> > boards without a CPS device (only Malta and Boston create one).
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> >   hw/mips/meson.build     |  1 +
> >   2 files changed, 27 insertions(+)
> >   create mode 100644 hw/mips/mips_itu-stub.c
> >
> > diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> > new file mode 100644
> > index 00000000000..4cc82b8461f
> > --- /dev/null
> > +++ b/hw/mips/mips_itu-stub.c
> > @@ -0,0 +1,26 @@
> > +/*
> > + * Inter-Thread Communication Unit emulation.
> > + *
> > + * Copyright (c) 2016 Imagination Technologies
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library 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
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>
> This is your code addition, so "Inter-Thread Communication Unit stubs" /
> RH / SPDX GPLv2-or-later.
>
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/misc/mips_itu.h"
> > +
> > +void itc_reconfigure(MIPSITUState *tag)
> > +{
> > +    abort();
>
> As Zoltan suggested, g_assert_not_reached(). Indeed this path
> can't be reached without ITU, TCG won't emit anything and will
> call instead:
>
>      qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>                    register_name, reg, sel);
>
> I'm reluctant to add stubs, but since it helps you (hoping we
> can figure a clean way to split architectural access to hw/ from
> tcg/ one day):

Not sure how it's reached anyway, because the saar field of
DisasContext is never written...

Paolo

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Philippe Mathieu-Daudé 2 years ago

On 8/2/24 09:26, Paolo Bonzini wrote:
> On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Paolo,
>>
>> On 7/2/24 12:14, Paolo Bonzini wrote:
>>> itc_reconfigure() is referenced from TCG, provide a stub if needed.
>>> This makes it possible to build a QEMU binary that only includes
>>> boards without a CPS device (only Malta and Boston create one).
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>    hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>>>    hw/mips/meson.build     |  1 +
>>>    2 files changed, 27 insertions(+)
>>>    create mode 100644 hw/mips/mips_itu-stub.c
>>>
>>> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
>>> new file mode 100644
>>> index 00000000000..4cc82b8461f
>>> --- /dev/null
>>> +++ b/hw/mips/mips_itu-stub.c
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + * Inter-Thread Communication Unit emulation.
>>> + *
>>> + * Copyright (c) 2016 Imagination Technologies
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * This library 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
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>
>> This is your code addition, so "Inter-Thread Communication Unit stubs" /
>> RH / SPDX GPLv2-or-later.
>>
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/misc/mips_itu.h"
>>> +
>>> +void itc_reconfigure(MIPSITUState *tag)
>>> +{
>>> +    abort();
>>
>> As Zoltan suggested, g_assert_not_reached(). Indeed this path
>> can't be reached without ITU, TCG won't emit anything and will
>> call instead:
>>
>>       qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>>                     register_name, reg, sel);
>>
>> I'm reluctant to add stubs, but since it helps you (hoping we
>> can figure a clean way to split architectural access to hw/ from
>> tcg/ one day):
> 
> Not sure how it's reached anyway, because the saar field of
> DisasContext is never written...

Commit 5fb2dcd179 ("target/mips: Provide R/W access to SAARI
and SAAR CP0 registers") is an incomplete frontport of
https://github.com/MIPS/qemu/commit/c9340491cc.

Commit 043715d1e0 ("target/mips: Update ITU to utilize SAARI
and SAAR CP0 registers") is an incomplete frontport of
https://github.com/MIPS/qemu/commit/e03079c699.

So this feature is working in MIPS fork since 2017, and during
2019 mainstream only got part of it merged but never completed.

5 years passed.

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Philippe Mathieu-Daudé 2 years ago

On 9/2/24 08:22, Philippe Mathieu-Daudé wrote:
> On 8/2/24 09:26, Paolo Bonzini wrote:
>> On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé 
>> <philmd@linaro.org> wrote:
>>>
>>> Hi Paolo,
>>>
>>> On 7/2/24 12:14, Paolo Bonzini wrote:
>>>> itc_reconfigure() is referenced from TCG, provide a stub if needed.
>>>> This makes it possible to build a QEMU binary that only includes
>>>> boards without a CPS device (only Malta and Boston create one).
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>    hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>>>>    hw/mips/meson.build     |  1 +
>>>>    2 files changed, 27 insertions(+)
>>>>    create mode 100644 hw/mips/mips_itu-stub.c
>>>>
>>>> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
>>>> new file mode 100644
>>>> index 00000000000..4cc82b8461f
>>>> --- /dev/null
>>>> +++ b/hw/mips/mips_itu-stub.c
>>>> @@ -0,0 +1,26 @@
>>>> +/*
>>>> + * Inter-Thread Communication Unit emulation.
>>>> + *
>>>> + * Copyright (c) 2016 Imagination Technologies
>>>> + *
>>>> + * This library is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * This library 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
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with this library; if not, see 
>>>> <http://www.gnu.org/licenses/>.
>>>
>>> This is your code addition, so "Inter-Thread Communication Unit stubs" /
>>> RH / SPDX GPLv2-or-later.
>>>
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/misc/mips_itu.h"
>>>> +
>>>> +void itc_reconfigure(MIPSITUState *tag)
>>>> +{
>>>> +    abort();
>>>
>>> As Zoltan suggested, g_assert_not_reached(). Indeed this path
>>> can't be reached without ITU, TCG won't emit anything and will
>>> call instead:
>>>
>>>       qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>>>                     register_name, reg, sel);
>>>
>>> I'm reluctant to add stubs, but since it helps you (hoping we
>>> can figure a clean way to split architectural access to hw/ from
>>> tcg/ one day):
>>
>> Not sure how it's reached anyway, because the saar field of
>> DisasContext is never written...

Alternatively:
https://lore.kernel.org/qemu-devel/20240209090513.9401-3-philmd@linaro.org/

> Commit 5fb2dcd179 ("target/mips: Provide R/W access to SAARI
> and SAAR CP0 registers") is an incomplete frontport of
> https://github.com/MIPS/qemu/commit/c9340491cc.
> 
> Commit 043715d1e0 ("target/mips: Update ITU to utilize SAARI
> and SAAR CP0 registers") is an incomplete frontport of
> https://github.com/MIPS/qemu/commit/e03079c699.
> 
> So this feature is working in MIPS fork since 2017, and during
> 2019 mainstream only got part of it merged but never completed.
> 
> 5 years passed.

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by BALATON Zoltan 2 years ago

On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> itc_reconfigure() is referenced from TCG, provide a stub if needed.
> This makes it possible to build a QEMU binary that only includes
> boards without a CPS device (only Malta and Boston create one).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> hw/mips/meson.build     |  1 +
> 2 files changed, 27 insertions(+)
> create mode 100644 hw/mips/mips_itu-stub.c
>
> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> new file mode 100644
> index 00000000000..4cc82b8461f
> --- /dev/null
> +++ b/hw/mips/mips_itu-stub.c
> @@ -0,0 +1,26 @@
> +/*
> + * Inter-Thread Communication Unit emulation.
> + *
> + * Copyright (c) 2016 Imagination Technologies
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/mips_itu.h"
> +
> +void itc_reconfigure(MIPSITUState *tag)
> +{
> +    abort();

Isn't g_assert_not_reached() preferred now for it giving better 
dignostics? But I don't know, just asking.

Regards,
BALATON Zoltan

> +}
> diff --git a/hw/mips/meson.build b/hw/mips/meson.build
> index f06d88f3430..2b1b96147a6 100644
> --- a/hw/mips/meson.build
> +++ b/hw/mips/meson.build
> @@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
> mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c'))
> mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
> mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
> +mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
>
> if 'CONFIG_TCG' in config_all_accel
> mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
>

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

Posted by Paolo Bonzini 2 years ago

Il mer 7 feb 2024, 14:16 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:

> On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> > itc_reconfigure() is referenced from TCG, provide a stub if needed.
> > This makes it possible to build a QEMU binary that only includes
> > boards without a CPS device (only Malta and Boston create one).
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> > hw/mips/meson.build     |  1 +
> > 2 files changed, 27 insertions(+)
> > create mode 100644 hw/mips/mips_itu-stub.c
> >
> > diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> > new file mode 100644
> > index 00000000000..4cc82b8461f
> > --- /dev/null
> > +++ b/hw/mips/mips_itu-stub.c
> > @@ -0,0 +1,26 @@
> > +/*
> > + * Inter-Thread Communication Unit emulation.
> > + *
> > + * Copyright (c) 2016 Imagination Technologies
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library 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
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/misc/mips_itu.h"
> > +
> > +void itc_reconfigure(MIPSITUState *tag)
> > +{
> > +    abort();
>
> Isn't g_assert_not_reached() preferred now for it giving better
> dignostics? But I don't know, just asking.
>

Sure, changed.

Paolo


> Regards,
> BALATON Zoltan
>
> > +}
> > diff --git a/hw/mips/meson.build b/hw/mips/meson.build
> > index f06d88f3430..2b1b96147a6 100644
> > --- a/hw/mips/meson.build
> > +++ b/hw/mips/meson.build
> > @@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true:
> files('fw_cfg.c'))
> > mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true:
> files('loongson3_bootp.c', 'loongson3_virt.c'))
> > mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
> > mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
> > +mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
> >
> > if 'CONFIG_TCG' in config_all_accel
> > mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
> >
>
>