target/arm: "define_arm_cp_regs" and similar for KVM

target/arm: "define_arm_cp_regs" and similar for KVM

[Claudio Fontana]

Hi Peter,

I am trying to find out how to split properly KVM and TCG in target/arm, among other things.

I skipped or stubbed all define_arm_cp_regs and similar functions,

and made a cpregs module that is TCG-only.

Thought it is fine, as we have a kvm_arm_init_cpreg_list that throws away everything TCG-related anyway later:

target/arm/kvm.c:

/* Initialize the ARMCPU cpreg list according to the kernel's                                                                               
 * definition of what CPU registers it knows about (and throw away                                                                          
 * the previous TCG-created cpreg list).                                                                                                    
 */
int kvm_arm_init_cpreg_list(ARMCPU *cpu)

Can you confirm that it is ok to do so?

But I found something peculiar in hw/intc/arm_gicv3_kvm.c:

here we have explictly for a "kvm" module a reginfo definition, and a call to

define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);

/*                                                                                                                                          
 * CPU interface registers of GIC needs to be reset on CPU reset.                                                                           
 * For the calling arm_gicv3_icc_reset() on CPU reset, we register                                                                          
 * below ARMCPRegInfo. As we reset the whole cpu interface under single                                                                     
 * register reset, we define only one register of CPU interface instead                                                                     
 * of defining all the registers.                                                                                                           
 */
static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {



Is this actually an exception, and possibly the only use of define_arm_cp_regs that would be required on KVM too?

Am I under some wrong assumption?

Thank you for any help in clarifying this..

commit 07a5628cb89f13b98fe526117fd07e5e273b5a52
Author: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
Date:   Thu Feb 23 17:21:13 2017 +0530

Thanks,

Claudio


-- 
Claudio Fontana
Engineering Manager Virtualization, SUSE Labs Core

SUSE Software Solutions Italy Srl

Re: target/arm: "define_arm_cp_regs" and similar for KVM

[Peter Maydell]

On Fri, 26 Feb 2021 at 17:27, Claudio Fontana <cfontana@suse.de> wrote:
>
> Hi Peter,
>
> I am trying to find out how to split properly KVM and TCG in target/arm, among other things.
>
> I skipped or stubbed all define_arm_cp_regs and similar functions,
>
> and made a cpregs module that is TCG-only.
>
> Thought it is fine, as we have a kvm_arm_init_cpreg_list that throws away everything TCG-related anyway later:
>
> target/arm/kvm.c:
>
> /* Initialize the ARMCPU cpreg list according to the kernel's
>  * definition of what CPU registers it knows about (and throw away
>  * the previous TCG-created cpreg list).
>  */
> int kvm_arm_init_cpreg_list(ARMCPU *cpu)

It throws away the cpreg *list*, but not the cpreg hash table.
That is, we trust KVM for "what sysregs exist and need to be migrated"
but we still
use the hashtable to find out what QEMU thinks about them and where it stores
them in the CPU state structure if it knows about them. So you still
need to keep the
define_arm_cp_regs stuff to register sysregs. Not doing so will break
any bits of the
code which rely on being able to do "synchronize kernel's idea of
sysreg state to
QEMU" and then "read the register value out of the CPU state struct". It'll also
break visibility of sysregs in the gdbstub.

thanks
-- PMM

Re: target/arm: "define_arm_cp_regs" and similar for KVM

[Claudio Fontana]

On 3/2/21 1:02 PM, Peter Maydell wrote:
> On Fri, 26 Feb 2021 at 17:27, Claudio Fontana <cfontana@suse.de> wrote:
>>
>> Hi Peter,
>>
>> I am trying to find out how to split properly KVM and TCG in target/arm, among other things.
>>
>> I skipped or stubbed all define_arm_cp_regs and similar functions,
>>
>> and made a cpregs module that is TCG-only.
>>
>> Thought it is fine, as we have a kvm_arm_init_cpreg_list that throws away everything TCG-related anyway later:
>>
>> target/arm/kvm.c:
>>
>> /* Initialize the ARMCPU cpreg list according to the kernel's
>>  * definition of what CPU registers it knows about (and throw away
>>  * the previous TCG-created cpreg list).
>>  */
>> int kvm_arm_init_cpreg_list(ARMCPU *cpu)
> 
> It throws away the cpreg *list*, but not the cpreg hash table.
> That is, we trust KVM for "what sysregs exist and need to be migrated"
> but we still
> use the hashtable to find out what QEMU thinks about them and where it stores
> them in the CPU state structure if it knows about them. So you still
> need to keep the
> define_arm_cp_regs stuff to register sysregs. Not doing so will break
> any bits of the
> code which rely on being able to do "synchronize kernel's idea of
> sysreg state to
> QEMU" and then "read the register value out of the CPU state struct". It'll also
> break visibility of sysregs in the gdbstub.
> 
> thanks
> -- PMM
> 

Thanks for the confirmation Peter,

I ended up doing that.

Thanks,

Claudio