> On 3 Sep 2021, at 23:49, Stefano Stabellini <sstabell...@kernel.org> wrote:
>
> On Wed, 25 Aug 2021, Bertrand Marquis wrote:
>> Use arm64 cpu feature sanitization to TAIN Xen if different DCZID values
> ^ TAINT
>
>
>> are found (ftr_dczid is using only STRICT method).
>> In this case actual memory being cleaned by DC ZVA operations would be
>> different depending on the cores which could make a guest zeroing too
>> much or too little memory if it is merged between CPUs.
>>
>> We could, on processor supporting it, trap access to DCZID_EL0 register
> ^ processors
Could those typos be fixed during commit ?
>
>> using HFGRTR_EL2 register but this would not solve the case where a
>> process is being migrated during a copy or if it cached the value of the
>> register.
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
>> ---
>> Change in v3: none
>> Change in v2: Patch introduced in v2
>> ---
>> xen/arch/arm/arm64/cpufeature.c | 14 +++++++++++---
>> xen/arch/arm/cpufeature.c | 2 ++
>> xen/include/asm-arm/cpufeature.h | 8 ++++++++
>> 3 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/cpufeature.c
>> b/xen/arch/arm/arm64/cpufeature.c
>> index 61f629ebaa..b1936ef1d6 100644
>> --- a/xen/arch/arm/arm64/cpufeature.c
>> +++ b/xen/arch/arm/arm64/cpufeature.c
>> @@ -329,14 +329,11 @@ static const struct arm64_ftr_bits ftr_mvfr2[] = {
>> ARM64_FTR_END,
>> };
>>
>> -#if 0
>> -/* TODO: handle this when sanitizing cache related registers */
>> static const struct arm64_ftr_bits ftr_dczid[] = {
>> ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, DCZID_DZP_SHIFT, 1,
>> 1),
>> ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, DCZID_BS_SHIFT,
>> 4, 0),
>> ARM64_FTR_END,
>> };
>> -#endif
>>
>> static const struct arm64_ftr_bits ftr_id_isar0[] = {
>> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>> ID_ISAR0_DIVIDE_SHIFT, 4, 0),
>> @@ -592,6 +589,17 @@ void update_system_features(const struct cpuinfo_arm
>> *new)
>>
>> SANITIZE_ID_REG(zfr64, 0, aa64zfr0);
>>
>> + /*
>> + * Comment from Linux:
>
> I don't know if I would keep or remove "Comment from Linux"
I added that because the comment itself does not really apply to Xen.
I could have rephrased the comment/
Anyway I have no objection to remove that statement.
Do I need to send a v2 for that ?
Cheers
Bertrand
>
> In any case:
>
> Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
>
>
> Also I gave a quick test of the series on a ZCU102 and it worked fine.
>
>
>> + * Userspace may perform DC ZVA instructions. Mismatched block sizes
>> + * could result in too much or too little memory being zeroed if a
>> + * process is preempted and migrated between CPUs.
>> + *
>> + * ftr_dczid is using STRICT comparison so we will taint Xen if
>> different
>> + * values are found.
>> + */
>> + SANITIZE_REG(dczid, 0, dczid);
>> +
>> if ( cpu_feature64_has_el0_32(&system_cpuinfo) )
>> {
>> SANITIZE_ID_REG(pfr32, 0, pfr0);
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index f600a611bd..113f20f601 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -125,6 +125,8 @@ void identify_cpu(struct cpuinfo_arm *c)
>>
>> c->zfr64.bits[0] = READ_SYSREG(ID_AA64ZFR0_EL1);
>>
>> + c->dczid.bits[0] = READ_SYSREG(DCZID_EL0);
>> +
>> aarch32_el0 = cpu_feature64_has_el0_32(c);
>> #endif
>>
>> diff --git a/xen/include/asm-arm/cpufeature.h
>> b/xen/include/asm-arm/cpufeature.h
>> index 52cb3133e0..5219fd3bab 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -259,6 +259,14 @@ struct cpuinfo_arm {
>> register_t bits[1];
>> } zfr64;
>>
>> + /*
>> + * DCZID is only used to check for incoherent values between cores
>> + * and taint Xen in this case
>> + */
>> + struct {
>> + register_t bits[1];
>> + } dczid;
>> +
>> #endif
>>
>> /*
>> --
>> 2.17.1
