From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Claudio Fontana <cfontana@suse.de>,
Richard Henderson <richard.henderson@linaro.org>,
qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Huacai Chen" <chenhuacai@kernel.org>,
"Eduardo Habkost" <ehabkost@redhat.com>,
"Riku Voipio" <riku.voipio@iki.fi>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH 3/6] accel/tcg: Restrict tb_gen_code() from other accelerators
Date: Mon, 15 Mar 2021 15:48:41 +0100 [thread overview]
Message-ID: <07edfd36-0a54-0ffe-cd0f-09a146da083f@amsat.org> (raw)
In-Reply-To: <0baee669-a6c1-2e25-5272-c654689fe6b7@suse.de>
On 3/15/21 2:52 PM, Claudio Fontana wrote:
> On 1/21/21 7:06 AM, Richard Henderson wrote:
>> On 1/17/21 11:12 PM, Claudio Fontana wrote:
>>> On 1/17/21 5:48 PM, Philippe Mathieu-Daudé wrote:
>>>> tb_gen_code() is only called within TCG accelerator,
>>>> declare it locally.
>>>
>>> Is this used only in accel/tcg/cpu-exec.c ? Should it be a static function there?
>>
>> Possibly, but there's a *lot* of code that would have to be moved. For now,
>> queuing a slightly modified version of the patch.
>>
>>>> --- a/accel/tcg/user-exec.c
>>>> +++ b/accel/tcg/user-exec.c
>>>> @@ -28,6 +28,7 @@
>>>> #include "qemu/atomic128.h"
>>>> #include "trace/trace-root.h"
>>>> #include "trace/mem.h"
>>>> +#include "internal.h"
>>
>> Not needed by this patch.
>>
>>
>> r~
>>
>
> Hello,
>
> resurrecting this, and in reference to its commit: "c03f041f128301c6a6c32242846be08719cd4fc3",
>
> the name "internal.h" ends up polluting the include paths,
> so that when working for example on s390x, including "internal.h" ends up including this instead of the file in target/s390x/.
>
> I am not sure what exactly the right solution is, for this specific problem,
> and if we should look at the include paths settings in detail,
>
> but in my view calling files just "internal.h" or "internals.h" in general is not a good idea.
>
> I can see two issues with this naming:
>
> 1) it describes nothing about the actual intended contents, other that they should be "internal".
> Rather it would be better to know what the file is intended to contain, or we end up (as we end up) with very large files containing completely unrelated content.
>
> 2) we end up with clashes in our include paths if we are not super careful.
>
> Probably in this case, the target/s390x/internal.h could be given another name (s390x-internal.h) and then split up in the future (there is a whole bunch of unrelated suff).
>
> For accel/tcg/internal.h, maybe renaming it, or removing it altogether could both be good options?
I think something like commit ed5bad39e57 is required, restricting
the scope of:
add_project_arguments('-iquote',
meson.current_source_dir() / 'tcg' / tcg_arch,
... to tcg, and ...
'-iquote',
meson.current_source_dir() / 'accel/tcg',
to accel.
language: ['c', 'cpp', 'objc'])
next prev parent reply other threads:[~2021-03-15 14:49 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-17 16:48 [PATCH 0/6] accel: Restrict TCG-specific code Philippe Mathieu-Daudé
2021-01-17 16:48 ` [PATCH 1/6] accel/tcg: Make cpu_gen_init() static Philippe Mathieu-Daudé
2021-01-18 9:14 ` Claudio Fontana
2021-01-21 5:54 ` Richard Henderson
2021-01-17 16:48 ` [PATCH 2/6] accel/tcg: Restrict tb_flush_jmp_cache() from other accelerators Philippe Mathieu-Daudé
2021-01-18 9:14 ` Claudio Fontana
2021-01-21 5:55 ` Richard Henderson
2021-01-17 16:48 ` [PATCH 3/6] accel/tcg: Restrict tb_gen_code() " Philippe Mathieu-Daudé
2021-01-18 9:12 ` Claudio Fontana
2021-01-21 6:06 ` Richard Henderson
2021-03-15 13:52 ` Claudio Fontana
2021-03-15 14:48 ` Philippe Mathieu-Daudé [this message]
2021-01-17 16:48 ` [PATCH 4/6] accel/tcg: Declare missing cpu_loop_exit*() stubs Philippe Mathieu-Daudé
2021-01-18 9:02 ` Claudio Fontana
2021-01-18 9:29 ` Claudio Fontana
2021-01-18 9:39 ` Philippe Mathieu-Daudé
2021-01-18 10:03 ` Claudio Fontana
2021-02-15 12:01 ` Alex Bennée
2021-01-21 6:21 ` Richard Henderson
2021-01-17 16:48 ` [RFC PATCH 5/6] accel/tcg: Restrict cpu_io_recompile() from other accelerators Philippe Mathieu-Daudé
2021-01-18 9:04 ` Claudio Fontana
2021-01-21 6:53 ` Richard Henderson
2021-01-17 16:48 ` [RFC PATCH 6/6] softmmu: Restrict watchpoint handlers to TCG accelerator Philippe Mathieu-Daudé
2021-01-18 9:10 ` Claudio Fontana
2021-01-18 9:36 ` Philippe Mathieu-Daudé
2021-02-15 10:42 ` Claudio Fontana
2021-02-15 12:05 ` Alex Bennée
2021-01-21 6:56 ` Richard Henderson
2021-01-18 9:20 ` [PATCH 0/6] accel: Restrict TCG-specific code Claudio Fontana
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=07edfd36-0a54-0ffe-cd0f-09a146da083f@amsat.org \
--to=f4bug@amsat.org \
--cc=alex.bennee@linaro.org \
--cc=cfontana@suse.de \
--cc=chenhuacai@kernel.org \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=riku.voipio@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.