[PATCH v2 0/3] semihosting: Restrict to TCG

[PATCH v2 0/3] semihosting: Restrict to TCG

[Philippe Mathieu-Daudé]

v2: Address Paolo's comment

Semihosting currently uses the TCG probe_access API,
so it is pointless to have it in the binary when TCG
isn't.

It could be implemented for other accelerators, but
work need to be done. Meanwhile, do not enable it
unless TCG is available.

Philippe Mathieu-Daudé (3):
  target/mips: Restrict semihosting to TCG
  target/riscv: Restrict semihosting to TCG
  semihosting: Restrict to TCG

 semihosting/Kconfig  | 1 +
 target/mips/Kconfig  | 2 +-
 target/riscv/Kconfig | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.41.0

[PATCH v2 1/3] target/mips: Restrict semihosting to TCG

[Philippe Mathieu-Daudé]

Semihosting currently uses the TCG probe_access API. To prepare for
encoding the TCG dependency in Kconfig, do not enable it unless TCG
is available.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/Kconfig b/target/mips/Kconfig
index eb19c94c7d..876048b150 100644
--- a/target/mips/Kconfig
+++ b/target/mips/Kconfig
@@ -1,6 +1,6 @@
 config MIPS
     bool
-    select SEMIHOSTING
+    imply SEMIHOSTING if TCG
 
 config MIPS64
     bool
-- 
2.41.0

[PATCH v2 2/3] target/riscv: Restrict semihosting to TCG

[Philippe Mathieu-Daudé]

Semihosting currently uses the TCG probe_access API. To prepare for
encoding the TCG dependency in Kconfig, do not enable it unless TCG
is available.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index 5f30df22f2..cbafbedaeb 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
 config RISCV32
     bool
-    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
+    imply ARM_COMPATIBLE_SEMIHOSTING if TCG # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
 
 config RISCV64
     bool
-    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
+    imply ARM_COMPATIBLE_SEMIHOSTING if TCG # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
-- 
2.41.0

[PATCH v2 3/3] semihosting: Restrict to TCG

[Philippe Mathieu-Daudé]

Semihosting currently uses the TCG probe_access API.
It is pointless to have it in the binary when TCG isn't.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 semihosting/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/semihosting/Kconfig b/semihosting/Kconfig
index eaf3a20ef5..fbe6ac87f9 100644
--- a/semihosting/Kconfig
+++ b/semihosting/Kconfig
@@ -1,6 +1,7 @@
 
 config SEMIHOSTING
        bool
+       depends on TCG
 
 config ARM_COMPATIBLE_SEMIHOSTING
        bool
-- 
2.41.0

Re: [PATCH v2 0/3] semihosting: Restrict to TCG

[Philippe Mathieu-Daudé]

On 30/5/24 16:53, Philippe Mathieu-Daudé wrote:
> v2: Address Paolo's comment

Missing review: 1 & 2

> Semihosting currently uses the TCG probe_access API,
> so it is pointless to have it in the binary when TCG
> isn't.
> 
> It could be implemented for other accelerators, but
> work need to be done. Meanwhile, do not enable it
> unless TCG is available.
> 
> Philippe Mathieu-Daudé (3):
>    target/mips: Restrict semihosting to TCG
>    target/riscv: Restrict semihosting to TCG
>    semihosting: Restrict to TCG
> 
>   semihosting/Kconfig  | 1 +
>   target/mips/Kconfig  | 2 +-
>   target/riscv/Kconfig | 4 ++--
>   3 files changed, 4 insertions(+), 3 deletions(-)
> 

Re: [PATCH v2 0/3] semihosting: Restrict to TCG

[Philippe Mathieu-Daudé]

Kind ping :)

On 3/6/24 10:27, Philippe Mathieu-Daudé wrote:
> On 30/5/24 16:53, Philippe Mathieu-Daudé wrote:
>> v2: Address Paolo's comment
> 
> Missing review: 1 & 2
> 
>> Semihosting currently uses the TCG probe_access API,
>> so it is pointless to have it in the binary when TCG
>> isn't.
>>
>> It could be implemented for other accelerators, but
>> work need to be done. Meanwhile, do not enable it
>> unless TCG is available.
>>
>> Philippe Mathieu-Daudé (3):
>>    target/mips: Restrict semihosting to TCG
>>    target/riscv: Restrict semihosting to TCG
>>    semihosting: Restrict to TCG
>>
>>   semihosting/Kconfig  | 1 +
>>   target/mips/Kconfig  | 2 +-
>>   target/riscv/Kconfig | 4 ++--
>>   3 files changed, 4 insertions(+), 3 deletions(-)
>>
> 

Re: [PATCH v2 0/3] semihosting: Restrict to TCG

[Anton Johansson via]

On 06/06/24, Philippe Mathieu-Daudé wrote:
> Kind ping :)

I'm off today, I'll take a look tomorrow morning!:)
//Anton

Re: [PATCH v2 1/3] target/mips: Restrict semihosting to TCG

[Anton Johansson via]

On 30/05/24, Philippe Mathieu-Daudé wrote:
> Semihosting currently uses the TCG probe_access API. To prepare for
> encoding the TCG dependency in Kconfig, do not enable it unless TCG
> is available.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/mips/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

xtensa and m68k also `select SEMIHOSTING`, were these missed?

Otherwise for patch 1 & 2:
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH v2 1/3] target/mips: Restrict semihosting to TCG

[Philippe Mathieu-Daudé]

On 7/6/24 13:08, Anton Johansson wrote:
> On 30/05/24, Philippe Mathieu-Daudé wrote:
>> Semihosting currently uses the TCG probe_access API. To prepare for
>> encoding the TCG dependency in Kconfig, do not enable it unless TCG
>> is available.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/mips/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> xtensa and m68k also `select SEMIHOSTING`, were these missed?

TCG is the only accelerator they use, so it is kinda implicit,
but you are right, I'll update for completeness.

> Otherwise for patch 1 & 2:
> Reviewed-by: Anton Johansson <anjo@rev.ng>

Thanks!

Re: [PATCH v2 1/3] target/mips: Restrict semihosting to TCG

[Alex Bennée]

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

> On 7/6/24 13:08, Anton Johansson wrote:
>> On 30/05/24, Philippe Mathieu-Daudé wrote:
>>> Semihosting currently uses the TCG probe_access API. To prepare for
>>> encoding the TCG dependency in Kconfig, do not enable it unless TCG
>>> is available.
>>>
>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   target/mips/Kconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> xtensa and m68k also `select SEMIHOSTING`, were these missed?
>
> TCG is the only accelerator they use, so it is kinda implicit,
> but you are right, I'll update for completeness.

So I'll wait for a v3?

>
>> Otherwise for patch 1 & 2:
>> Reviewed-by: Anton Johansson <anjo@rev.ng>
>
> Thanks!

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 1/3] target/mips: Restrict semihosting to TCG

[Philippe Mathieu-Daudé]

On 10/6/24 11:29, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 7/6/24 13:08, Anton Johansson wrote:
>>> On 30/05/24, Philippe Mathieu-Daudé wrote:
>>>> Semihosting currently uses the TCG probe_access API. To prepare for
>>>> encoding the TCG dependency in Kconfig, do not enable it unless TCG
>>>> is available.
>>>>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    target/mips/Kconfig | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>> xtensa and m68k also `select SEMIHOSTING`, were these missed?
>>
>> TCG is the only accelerator they use, so it is kinda implicit,
>> but you are right, I'll update for completeness.
> 
> So I'll wait for a v3?

Yes, on the way...