qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Discussion: Patch series that adds disable-tcg option for ppc targets
@ 2021-04-05 13:32 Bruno Piazera Larsen
  2021-04-06  6:11 ` David Gibson
  0 siblings, 1 reply; 6+ messages in thread
From: Bruno Piazera Larsen @ 2021-04-05 13:32 UTC (permalink / raw)
  To: qemu-ppc
  Cc: qemu-devel, Lucas Mateus Martins Araujo e Castro,
	Fernando Eckhardt Valle, Andre Fernando da Silva,
	Matheus Kowalczuk Ferst, Luis Fernando Fujita Pires,
	David Gibson

[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]

Hi all,

The team I'm working on started to work on adding support for building the ppc target with the disable-tcg option. However, we're not quite sure on where to start with such a big patch.

    * Should we focus first on changing the .c files, so that it will build when we finally patch the meson.build everything build correctly? Or should we start by changing the meson file, so that everyone agrees that the excluded files can indeed be excluded?
    * Should we first finish the whole series to then mail it, or should we send one file at a time, as soon as we have them ready?

So far we're thinking we'll need to:
    * change 6 files (arch_dump.c, machine.c, mmu-hash32.c, mmu-hash64.c, mmu-radix64.c and meson.build);
    * exclude 8 files from the build (dfp_helper.c, excp_helper.c, fpu_helper.c, int_helper.c, mem_helper.c, misc_helper.c, translate.c, timebase_helper.c);
    * create a new one that holds stubs.
Does this sound about right? This is mostly guesswork based on how x86, s390x and ARM are doing it.

Thank in advance


Bruno Piazera Larsen

Instituto de Pesquisas ELDORADO<http://clickemailmkt.eldorado.org.br/ls/click?upn=UPoxpeIcHnAcbUZyo7TTaswyiVb1TXP3jEbQqiiJKKGsxOn8hBEs5ZsMLQfXkKuKXZ7MVDg0ij9eG8HV4TXI75dBzDiNGLxQ8Xx5PzCVNt6TpGrzBbU-2Biu0o69X5ce-2FW-2FOk1uUipuK0fZnWXJEgbRw-3D-3DJY4T_wWk-2BG6VvNBoa1YzxYjhCdFS9IfANIaBzDSklR1NyyrKOI1wj0P-2BdBFcuO4FnHcsA1MyHu0ly1Yt3oDMp7KKdJPM68iKuI2jiRH5v4B0d8wf3chU3qy5n5iXWnW1QjSaNFHOgELzxaP-2FnesTeBgJ5dFkjH4f279sVQpOtyjw5xAqj34M6pgNRAxVvuXif4IWDcVzXg1FzfYlEfkKzr9vvpA3Hg8kitwMtlU3zwbQUBCgL30fQoJPcRPMGKyOY8RmoAlXNqTJYDYIvqmfnI7KLUvw6vKB5R-2B5q1FJRAzX7H-2BmF0NnDET6jMLuIqtCcVIch>

Departamento Computação Embarcada

Analista de Software Trainee

Aviso Legal - Disclaimer<https://www.eldorado.org.br/disclaimer.html>

[-- Attachment #2: Type: text/html, Size: 4850 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: Discussion: Patch series that adds disable-tcg option for ppc targets
@ 2021-04-06 18:38 Bruno Piazera Larsen
  2021-04-06 20:48 ` Fabiano Rosas
  2021-04-07  3:01 ` David Gibson
  0 siblings, 2 replies; 6+ messages in thread
From: Bruno Piazera Larsen @ 2021-04-06 18:38 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-devel, Lucas Mateus Martins Araujo e Castro,
	Fernando Eckhardt Valle, qemu-ppc, Andre Fernando da Silva,
	Matheus Kowalczuk Ferst, Luis Fernando Fujita Pires

[-- Attachment #1: Type: text/plain, Size: 5964 bytes --]

> t's usually best to focus on logical changes, rather than
> file-by-file.  That said, I'd probably suggest changing the .c files
> first, then changing the meson file.

OK, will do!

> I'd lean towards building a whole series, since I think the individual
> file changes won't make a lot of sense on their own.  That said, it's
> ok and encouraged to post a relatively early draft of the series as an
> RFC, so that the overall idea can be reviewed, even if it has obvious
> omissions (just mention them in the cover letter).

Good to know. I'm guessing that the RFC should at least compile, though, right? That's mostly what is holding me from sharing what we have so far.

> I'd expect  mmu-*.c to be excluded.  Those are softmmu implementations
> which shouldn't be used with KVM.  It's possible there are a few
> helpers that will need to be extracted, though.

weirdly enough, removing the mmu-* don't give compilation errors, but linker errors. There are a few helpers that we'll have to see how to deal with, but something that I found odd is that cpu_list and all it's related functions are defined in translate_init.c.inc, instead of cpu.c like most other targets. I'm thinking about moving those to the cpu.c and follow the general standard of the rest

> You'll probably also need changes in hw/ppc/spapr_hcall.c and maybe
> some other parts of the spapr code: there are a number of hypercalls
> that we implement in qemu for TCG, but which are (and must be)
> implemented in KVM when KVM is in use.  So, I expect you'll need to
> suppress compilation of h_enter, h_remove, h_protect, h_read and
> h_bulk_remove at least in the !TCG case.

Yeah. there are 6 files dealing with spapr that are having linking problems, though some might be because of the cpu_list problem... But I'm a bit confused on how to interface with KVM, do I use an ioctl explicitly, or can it handle it implicitly? wenever I google or ask on IRC for KVM stuff the answer is usually "KVM handles it" so I never know if/when to call it

> Sounds fine so far.

Awesome! As the first contribution, I always feel like I'm missing something, thanks for the help (:


Bruno Piazera Larsen

Instituto de Pesquisas ELDORADO<http://clickemailmkt.eldorado.org.br/ls/click?upn=UPoxpeIcHnAcbUZyo7TTaswyiVb1TXP3jEbQqiiJKKGsxOn8hBEs5ZsMLQfXkKuKXZ7MVDg0ij9eG8HV4TXI75dBzDiNGLxQ8Xx5PzCVNt6TpGrzBbU-2Biu0o69X5ce-2FW-2FOk1uUipuK0fZnWXJEgbRw-3D-3DJY4T_wWk-2BG6VvNBoa1YzxYjhCdFS9IfANIaBzDSklR1NyyrKOI1wj0P-2BdBFcuO4FnHcsA1MyHu0ly1Yt3oDMp7KKdJPM68iKuI2jiRH5v4B0d8wf3chU3qy5n5iXWnW1QjSaNFHOgELzxaP-2FnesTeBgJ5dFkjH4f279sVQpOtyjw5xAqj34M6pgNRAxVvuXif4IWDcVzXg1FzfYlEfkKzr9vvpA3Hg8kitwMtlU3zwbQUBCgL30fQoJPcRPMGKyOY8RmoAlXNqTJYDYIvqmfnI7KLUvw6vKB5R-2B5q1FJRAzX7H-2BmF0NnDET6jMLuIqtCcVIch>

Departamento Computação Embarcada

Analista de Software Trainee

Aviso Legal - Disclaimer<https://www.eldorado.org.br/disclaimer.html>

________________________________
De: David Gibson
Enviadas: Terça-feira, 06 de Abril de 2021 03:11
Para: Bruno Piazera Larsen
Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; Lucas Mateus Martins Araujo e Castro; Luis Fernando Fujita Pires; Fernando Eckhardt Valle; Matheus Kowalczuk Ferst; Andre Fernando da Silva
Assunto: Re: Discussion: Patch series that adds disable-tcg option for ppc targets

On Mon, Apr 05, 2021 at 01:32:18PM +0000, Bruno Piazera Larsen wrote:
> Hi all,
>
> The team I'm working on started to work on adding support for
> building the ppc target with the disable-tcg option. However, we're
> not quite sure on where to start with such a big patch.
>
>     * Should we focus first on changing the .c files, so that it
>       will build when we finally patch the meson.build everything
>       build correctly? Or should we start by changing the meson
>       file, so that everyone agrees that the excluded files can
>       indeed be excluded?

It's usually best to focus on logical changes, rather than
file-by-file.  That said, I'd probably suggest changing the .c files
first, then changing the meson file.

>     * Should we first finish the whole series to then mail it, or
>       should we send one file at a time, as soon as we have them
>       ready?

I'd lean towards building a whole series, since I think the individual
file changes won't make a lot of sense on their own.  That said, it's
ok and encouraged to post a relatively early draft of the series as an
RFC, so that the overall idea can be reviewed, even if it has obvious
omissions (just mention them in the cover letter).

> So far we're thinking we'll need to:
>     * change 6 files (arch_dump.c, machine.c, mmu-hash32.c,
>       mmu-hash64.c, mmu-radix64.c and meson.build);

I'd expect  mmu-*.c to be excluded.  Those are softmmu implementations
which shouldn't be used with KVM.  It's possible there are a few
helpers that will need to be extracted, though.

You'll probably also need changes in hw/ppc/spapr_hcall.c and maybe
some other parts of the spapr code: there are a number of hypercalls
that we implement in qemu for TCG, but which are (and must be)
implemented in KVM when KVM is in use.  So, I expect you'll need to
suppress compilation of h_enter, h_remove, h_protect, h_read and
h_bulk_remove at least in the !TCG case.

>     * exclude 8 files from the build (dfp_helper.c, excp_helper.c,
>       fpu_helper.c, int_helper.c, mem_helper.c, misc_helper.c, *
>       translate.c, timebase_helper.c);

That looks about right.

>     * create a new one that holds stubs.

Sounds reasonable.

> Does this sound about right? This is mostly guesswork based on how
> x86, s390x and ARM are doing it.

Sounds fine so far.

--
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: text/html, Size: 11503 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-10  5:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-05 13:32 Discussion: Patch series that adds disable-tcg option for ppc targets Bruno Piazera Larsen
2021-04-06  6:11 ` David Gibson
2021-04-10  5:43   ` Thomas Huth
2021-04-06 18:38 Bruno Piazera Larsen
2021-04-06 20:48 ` Fabiano Rosas
2021-04-07  3:01 ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).