Re: [RFC v1 04/38] target/arm: move psci.c into tcg/softmmu/

[Claudio Fontana <cfontana@suse.de>]

On 2/22/21 6:22 PM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> From: Claudio Fontana <cfontana@centriq4.arch.suse.de>
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  target/arm/internals.h              | 23 ++++++++++-------------
>>  target/arm/tcg/helper.c             |  2 ++
>>  target/arm/{ => tcg/softmmu}/psci.c |  0
>>  target/arm/tcg/user/psci.c          | 26 ++++++++++++++++++++++++++
>>  target/arm/meson.build              |  1 -
>>  target/arm/tcg/meson.build          |  3 +++
>>  target/arm/tcg/softmmu/meson.build  |  4 ++++
>>  target/arm/tcg/user/meson.build     |  4 ++++
>>  8 files changed, 49 insertions(+), 14 deletions(-)
>>  rename target/arm/{ => tcg/softmmu}/psci.c (100%)
>>  create mode 100644 target/arm/tcg/user/psci.c
>>  create mode 100644 target/arm/tcg/softmmu/meson.build
>>  create mode 100644 target/arm/tcg/user/meson.build
>>
>> diff --git a/target/arm/internals.h b/target/arm/internals.h
>> index 05cebc8597..6384461177 100644
>> --- a/target/arm/internals.h
>> +++ b/target/arm/internals.h
>> @@ -292,21 +292,18 @@ vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len);
>>  /* Callback function for when a watchpoint or breakpoint triggers. */
>>  void arm_debug_excp_handler(CPUState *cs);
>>  
>> -#if defined(CONFIG_USER_ONLY) || !defined(CONFIG_TCG)
>> -static inline bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>> -{
>> -    return false;
>> -}
>> -static inline void arm_handle_psci_call(ARMCPU *cpu)
>> -{
>> -    g_assert_not_reached();
>> -}
>> -#else
> 
> I'm not sure I'm a fan of pushing these #ifdef tweaks down into the main
> code when the compiler is good an eliding them away. I guess we need
> this because the helper.o wants to be a shared object between both user
> and softmmu/sysemu mode?


Hi Alex,

yes, but will try to clean this up, I agree that this is too much preprocessor stuff.

Ciao,

CLaudio

> 
>> -/* Return true if the r0/x0 value indicates that this SMC/HVC is a PSCI call. */
>> +#ifdef CONFIG_TCG
>> +/*
>> + * Return true only for softmmu, if the r0/x0 value indicates that this
>> + * SMC/HVC is a PSCI call.
>> + */
>>  bool arm_is_psci_call(ARMCPU *cpu, int excp_type);
>> -/* Actually handle a PSCI call */
>> +
>> +#ifndef CONFIG_USER_ONLY
>>  void arm_handle_psci_call(ARMCPU *cpu);
>> -#endif
>> +#endif /* !CONFIG_USER_ONLY */
>> +
>> +#endif /* CONFIG_TCG */
>>  
>>  /**
>>   * arm_clear_exclusive: clear the exclusive monitor
>> diff --git a/target/arm/tcg/helper.c b/target/arm/tcg/helper.c
>> index 0e1a3b9421..beb8a5deed 100644
>> --- a/target/arm/tcg/helper.c
>> +++ b/target/arm/tcg/helper.c
>> @@ -10040,11 +10040,13 @@ void arm_cpu_do_interrupt(CPUState *cs)
>>                        env->exception.syndrome);
>>      }
>>  
>> +#ifndef CONFIG_USER_ONLY
>>      if (arm_is_psci_call(cpu, cs->exception_index)) {
>>          arm_handle_psci_call(cpu);
>>          qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
>>          return;
>>      }
>> +#endif /* CONFIG_USER_ONLY */
>>  
>>      /*
>>       * Semihosting semantics depend on the register width of the code
>> diff --git a/target/arm/psci.c b/target/arm/tcg/softmmu/psci.c
>> similarity index 100%
>> rename from target/arm/psci.c
>> rename to target/arm/tcg/softmmu/psci.c
>> diff --git a/target/arm/tcg/user/psci.c b/target/arm/tcg/user/psci.c
>> new file mode 100644
>> index 0000000000..f3e293c516
>> --- /dev/null
>> +++ b/target/arm/tcg/user/psci.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Copyright (C) 2014 - Linaro
>> + * Author: Rob Herring <rob.herring@linaro.org>
>> + *
>> + *  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 <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "internals.h"
>> +
>> +bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>> +{
>> +    return false;
>> +}
>> diff --git a/target/arm/meson.build b/target/arm/meson.build
>> index 0172937b40..3d23a6c687 100644
>> --- a/target/arm/meson.build
>> +++ b/target/arm/meson.build
>> @@ -19,7 +19,6 @@ arm_softmmu_ss.add(files(
>>    'arm-powerctl.c',
>>    'machine.c',
>>    'monitor.c',
>> -  'psci.c',
>>  ))
>>  arm_user_ss = ss.source_set()
>>  
>> diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
>> index 3b4146d079..4d9ed4b9cf 100644
>> --- a/target/arm/tcg/meson.build
>> +++ b/target/arm/tcg/meson.build
>> @@ -36,3 +36,6 @@ arm_ss.add(when: ['TARGET_AARCH64','CONFIG_TCG'], if_true: files(
>>    'pauth_helper.c',
>>    'sve_helper.c',
>>  ))
>> +
>> +subdir('user')
>> +subdir('softmmu')
>> diff --git a/target/arm/tcg/softmmu/meson.build b/target/arm/tcg/softmmu/meson.build
>> new file mode 100644
>> index 0000000000..f136c8bb8b
>> --- /dev/null
>> +++ b/target/arm/tcg/softmmu/meson.build
>> @@ -0,0 +1,4 @@
>> +
>> +arm_softmmu_ss.add(when: ['CONFIG_TCG','CONFIG_SOFTMMU'], if_true: files(
>> +  'psci.c',
>> +))
>> diff --git a/target/arm/tcg/user/meson.build b/target/arm/tcg/user/meson.build
>> new file mode 100644
>> index 0000000000..f18d08c52c
>> --- /dev/null
>> +++ b/target/arm/tcg/user/meson.build
>> @@ -0,0 +1,4 @@
>> +
>> +arm_user_ss.add(when: ['CONFIG_TCG','CONFIG_USER_ONLY'], if_true: files(
>> +  'psci.c',
>> +))
> 
>