lkml.org 
[lkml]   [2019]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] arm64: Introduce ISAR6 CPU ID register
From
Date
On 12/12/2019 10:01 PM, Mark Rutland wrote:
> On Thu, Dec 12, 2019 at 03:22:13PM +0000, Suzuki Kuruppassery Poulose wrote:
>> On 12/12/2019 14:46, Mark Rutland wrote:
>>> On Thu, Dec 12, 2019 at 03:44:23PM +0530, Anshuman Khandual wrote:
>>>> +#define ID_ISAR6_JSCVT_SHIFT 0
>>>> +#define ID_ISAR6_DP_SHIFT 4
>>>> +#define ID_ISAR6_FHM_SHIFT 8
>>>> +#define ID_ISAR6_SB_SHIFT 12
>>>> +#define ID_ISAR6_SPECRES_SHIFT 16
>>>> +#define ID_ISAR6_BF16_SHIFT 20
>>>> +#define ID_ISAR6_I8MM_SHIFT 24
>>>
>>>> @@ -399,6 +399,7 @@ static const struct __ftr_reg_entry {
>>>> ARM64_FTR_REG(SYS_ID_ISAR4_EL1, ftr_generic_32bits),
>>>> ARM64_FTR_REG(SYS_ID_ISAR5_EL1, ftr_id_isar5),
>>>> ARM64_FTR_REG(SYS_ID_MMFR4_EL1, ftr_id_mmfr4),
>>>
>>>> + ARM64_FTR_REG(SYS_ID_ISAR6_EL1, ftr_generic_32bits),
>>>
>>> Using ftr_generic_32bits exposes the lowest-common-denominator for all
>>> 4-bit fields in the register, and I don't think that's the right thing
>>> to do here, because:
>>>
>>> * We have no idea what ID_ISAR6 bits [31:28] may mean in future.
>>>
>>> * AFAICT, the instructions described by ID_ISAR6.SPECRES (from the
>>> ARMv8.0-PredInv extension) operate on the local PE and are not
>>> broadcast. To make those work as a guest expects, the host will need
>>> to do additional things (e.g. to preserve that illusion when a vCPU is
>>> migrated from one pCPU to another and back).
>>>
>>> Given that, think we should add an explicit ftr_id_isar6 which only
>>> exposes the fields that we're certain are safe to expose to a guest
>>> (i.e. without SPECRES).
>>
>> Agree. Thanks for pointing this out. I recommended the usage of
>> generic_32bits table without actually looking at the feature
>> definitions.
>
> No worries; this is /really/ easy to miss!
>
> Looking again, comparing to ARM DDI 0487E.a, there are a few other
> things we should probably sort out:
>
> * ID_DFR0 fields need more thought; we should limit what we expose here.
> I don't think it's valid for us to expose TraceFilt, and I suspect we

Sure, will go ahead and drop TraceFilt [28..31] from ID_DFR0 register.

> need to add capping for debug features we currently emulate.

Could you please elaborate ?

>
> * ID_ISAR0[31:28] are RES0 in ARMv8, Reserved/UNK in ARMv7.
> We should probably ftr_id_isar0 so we can hide those bits.

Sure, will do.

>
> * ID_ISAR5[23:10] are RES0
> We handle this already! :)

I may be missing something here but some of these fields are already there.

#define ID_ISAR5_RDM_SHIFT 24
#define ID_ISAR5_CRC32_SHIFT 16
#define ID_ISAR5_SHA2_SHIFT 12
#define ID_ISAR5_SHA1_SHIFT 8
#define ID_ISAR5_AES_SHIFT 4
#define ID_ISAR5_SEVL_SHIFT 0

static const struct arm64_ftr_bits ftr_id_isar5[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_RDM_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_CRC32_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA2_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SHA1_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_AES_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR5_SEVL_SHIFT, 4, 0),
ARM64_FTR_END,
};

>
> * ID_MMFR4.SpecSEI should be trated as higher safe.
> We should update ftr_id_mmfr4 to handle this and other fields.

Sure but should we also export other fields as higher safe in there ?

>
> * ID_PFR0 is missing DIT and CSV2
> We should probably add these (but neither RAS not AMU).

Sure, will do.

>
> * ID_PFR2 is missing
> We should probably add this for SSBS and CSV3.

Sure but should we add corresponding ID_AA64PFR2_EL1 register as well ?

>
> Thanks,
> Mark.
>

\
 
 \ /
  Last update: 2019-12-18 06:54    [W:0.035 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site