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

Re: [PATCH v3 6/7] xen/arm: Taint Xen on incompatible DCZID values


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Mon, 6 Sep 2021 07:59:00 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=wqT3OTunmQEuYn/hPnbl+2ZS4xB6pnzJeMCS5J/gFo0=; b=HsNcM+Eb2ckqiboIeDKnPVu/CB1TgSWGd2KpPkrAFHm3ljb9XD6287WiXFKLnxLaca8bUQUnz4gQYeO5CSYO18GVIt1dl6sVJI9jTvfXSaDQIE+6U9pgywloq6/ctZ/B4UpZ2iDM/kmmfaEwk18C97bvL9UboAGyDC9CYqx+H3t1T04nJkorMzupdmfSvWwUM8nC5uk3Gu0c19THvcLHGeesUjFOsStMZwKBxXxWN+hUqIk0G62uKNvbB7Qph0nWj9/FTXNiS70fzK+KHFmQcDL76FoTCZgcqw1nVuQdwLjHYb5MAUgeH+yKsitlPXjoIR2zju2P7aQ5b6Hr/Q5RZg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kOaNrl38Xj3PI4AOCqtzL5SsAYpkh8esndWQhkQ2W5ffZNG84QPmVRJjwCLfMaRU9HFkooDrKoqan7XVLQrQZhV55dbM6/wWPbPA+Nc0wCgrzZG9nrQdybJfxGBGZ+3uZ2fUj+fvzBD3tyV8lsud7BDVdHX6QYmt5h3t1ytwDjA1uFh4BXIi+/RZRUkgrlBzMSDrs7/b+ht3Pu0qUVRM2xw/Jv83EfJD1GGNikJtkSjutN5EkGrqSBKdmOHGjJEVCVU5CIdu9NCiTG5TRTdnKMOpy3YRmjkmj7v5bot59Vb5JN0FGiyjjwqwZJng44GtTNDk4qAVEYi8eWjTXpn86A==
  • Authentication-results-original: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 06 Sep 2021 07:59:15 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXmbPyvc9RSYz5OUKBEYoCXmX0t6uS+GGAgAO+T4A=
  • Thread-topic: [PATCH v3 6/7] xen/arm: Taint Xen on incompatible DCZID values


> On 3 Sep 2021, at 23:49, Stefano Stabellini <sstabellini@xxxxxxxxxx> 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.marquis@xxxxxxx>
>> ---
>> 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 <sstabellini@xxxxxxxxxx>
> 
> 
> 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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.