Re: [PATCH 13/59] KVM: arm64: nv: Handle virtual EL2 registers in vcpu_read/write_sys_reg()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/07/2019 17:32, Alexandru Elisei wrote:

[...]

>>>> +}
>>>> +
>>>> +#define EL2_SYSREG(el2, el1, translate)	\
>>>> +	[el2 - FIRST_EL2_SYSREG] = { el2, el1, translate }
>>>> +#define PURE_EL2_SYSREG(el2) \
>>>> +	[el2 - FIRST_EL2_SYSREG] = { el2,__INVALID_SYSREG__, NULL }
>>>> +/*
>>>> + * Associate vEL2 registers to their EL1 counterparts on the CPU.
>>>> + * The translate function can be NULL, when the register layout is identical.
>>>> + */
>>>> +struct el2_sysreg_map {
>>>> +	int sysreg;	/* EL2 register index into the array above */
>>>> +	int mapping;	/* associated EL1 register */
>>>> +	u64 (*translate)(u64 value);
>>>> +} nested_sysreg_map[NR_SYS_REGS - FIRST_EL2_SYSREG] = {
>>>> +	PURE_EL2_SYSREG( VPIDR_EL2 ),
>>>> +	PURE_EL2_SYSREG( VMPIDR_EL2 ),
>>>> +	PURE_EL2_SYSREG( ACTLR_EL2 ),
>>>> +	PURE_EL2_SYSREG( HCR_EL2 ),
>>>> +	PURE_EL2_SYSREG( MDCR_EL2 ),
>>>> +	PURE_EL2_SYSREG( HSTR_EL2 ),
>>>> +	PURE_EL2_SYSREG( HACR_EL2 ),
>>>> +	PURE_EL2_SYSREG( VTTBR_EL2 ),
>>>> +	PURE_EL2_SYSREG( VTCR_EL2 ),
>>>> +	PURE_EL2_SYSREG( RVBAR_EL2 ),
>>>> +	PURE_EL2_SYSREG( RMR_EL2 ),
>>>> +	PURE_EL2_SYSREG( TPIDR_EL2 ),
>>>> +	PURE_EL2_SYSREG( CNTVOFF_EL2 ),
>>>> +	PURE_EL2_SYSREG( CNTHCTL_EL2 ),
>>>> +	PURE_EL2_SYSREG( HPFAR_EL2 ),
>>>> +	EL2_SYSREG(      SCTLR_EL2,  SCTLR_EL1,      translate_sctlr ),
>>>> +	EL2_SYSREG(      CPTR_EL2,   CPACR_EL1,      translate_cptr  ),
>>>> +	EL2_SYSREG(      TTBR0_EL2,  TTBR0_EL1,      translate_ttbr0 ),
>>>> +	EL2_SYSREG(      TTBR1_EL2,  TTBR1_EL1,      NULL            ),
>>>> +	EL2_SYSREG(      TCR_EL2,    TCR_EL1,        translate_tcr   ),
>>>> +	EL2_SYSREG(      VBAR_EL2,   VBAR_EL1,       NULL            ),
>>>> +	EL2_SYSREG(      AFSR0_EL2,  AFSR0_EL1,      NULL            ),
>>>> +	EL2_SYSREG(      AFSR1_EL2,  AFSR1_EL1,      NULL            ),
>>>> +	EL2_SYSREG(      ESR_EL2,    ESR_EL1,        NULL            ),
>>>> +	EL2_SYSREG(      FAR_EL2,    FAR_EL1,        NULL            ),
>>>> +	EL2_SYSREG(      MAIR_EL2,   MAIR_EL1,       NULL            ),
>>>> +	EL2_SYSREG(      AMAIR_EL2,  AMAIR_EL1,      NULL            ),
>>>> +};
>>> Figuring out which registers are in this map and which aren't and are supposed
>>> to be treated differently is really cumbersome because they are split into two
>>> types of el2 registers and their order is different from the order in enum
>>> vcpu_sysreg (in kvm_host.h). Perhaps adding a comment about what registers will
>>> be treated differently would make the code a bit easier to follow?
>> I'm not sure what this buys us. We have 3 categories of EL2 sysregs:
>> - Purely emulated
>> - Directly mapped onto an EL1 sysreg
>> - Translated from EL2 to EL1
>>
>> I think the wrappers represent that pretty well, although we could split
>> EL2_SYSREG into DIRECT_EL2_SYSREG and TRANSLATE_EL2_SYSREG. As for the
>> order, does it really matter? We also have the trap table order, which
>> is also different from the enum. Do you propose we reorder everything?
> 
> The wrappers and the naming are fine.
> 
> I was trying to figure out which EL2 registers are in the nested_sysreg_map and
> which aren't (that's what I meant by "two types of registers") by looking at the
> vcpu_sysreg enum. Because the order in the map is different than the order in
> the enum, I was having a difficult time figuring out which registers are not in
> the nested_sysreg_map to make sure we haven't somehow forgot to emulate a register.
> 
> So no, I wasn't asking to reorder everything. I was asking if it would be
> appropriate to write a comment stating the intention to treat registers X, Y and
> Z separately from the registers in nested_sysreg_map.

Ah, fair enough. Yes, that's a very reasonable suggestion.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux