Re: [PATCH] hvf: arm: Fix encodings for ID_AA64PFR1_EL1 and debug System registers

[Zenghui Yu <zenghui.yu@linux.dev>]

On 2024/5/5 21:18, Alexander Graf wrote:
> 
> On 03.05.24 19:34, Zenghui Yu wrote:
>> We wrongly encoded ID_AA64PFR1_EL1 using {3,0,0,4,2} in 
>> hvf_sreg_match[] so
>> we fail to get the expected ARMCPRegInfo from cp_regs hash table with the
>> wrong key.
>>
>> Fix it with the correct encoding {3,0,0,4,1}. With that fixed, the Linux
>> guest can properly detect FEAT_SSBS2 on my M1 HW.
>>
>> All DBG{B,W}{V,C}R_EL1 registers are also wrongly encoded with op0 == 14.
>> It happens to work because HVF_SYSREG(CRn, CRm, 14, op1, op2) equals to
>> HVF_SYSREG(CRn, CRm, 2, op1, op2), by definition. But we shouldn't 
>> rely on
>> it.
>>
>> Fixes: a1477da3ddeb ("hvf: Add Apple Silicon support")
>> Signed-off-by: Zenghui Yu <zenghui.yu@linux.dev>
> 
> 
> Nice catch! Did you find them only because of functional issues or have 
> you taken an automated pass somehow to validate the sysreg definitions 
> are correct?

Neither, this was found by code inspection. I was just curious about how
VMM would interact with hypervisor to expose features to the guest. Docs
at https://developer.apple.com/documentation/hypervisor are not that
detailed (I may need to read further ;-) ).

P.S., I verified on M1 that ID_AA64PFR1_EL1 is 0x20 at
hvf_arm_get_host_cpu_features() stage and is overwritten as 0x0 later
w/o this patch, which indicates the (guest visible) feature ID registers
are "writable" to some extent.

> Reviewed-by: Alexander Graf <agraf@csgraf.de>

Thanks!

Zenghui