Re: [PATCH v4 2/2] KVM: doc: add API documentation on the KVM_REG_ARM_WORKAROUNDS register

From: Auger Eric <eric.auger@redhat.com>
To: Andre Przywara <andre.przywara@arm.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Christoffer Dall <christoffer.dall@arm.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	Dave Martin <dave.martin@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Steven Price <steven.price@arm.com>
Subject: Re: [PATCH v4 2/2] KVM: doc: add API documentation on the KVM_REG_ARM_WORKAROUNDS register
Date: Thu, 21 Mar 2019 13:33:18 +0100	[thread overview]
Message-ID: <60c2e180-03aa-1b9e-f19c-90d890b99d15@redhat.com> (raw)
In-Reply-To: <20190228234334.20456-3-andre.przywara@arm.com>

Hi Andre,

On 3/1/19 12:43 AM, Andre Przywara wrote:
> Add documentation for the newly defined firmware registers to save and
> restore any vulnerability migitation status.
s/migitation/mitigation
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Documentation/virtual/kvm/arm/psci.txt | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/arm/psci.txt b/Documentation/virtual/kvm/arm/psci.txt
> index aafdab887b04..1ed0f0515cd8 100644
> --- a/Documentation/virtual/kvm/arm/psci.txt
> +++ b/Documentation/virtual/kvm/arm/psci.txt
> @@ -28,3 +28,28 @@ The following register is defined:
>    - Allows any PSCI version implemented by KVM and compatible with
>      v0.2 to be set with SET_ONE_REG
>    - Affects the whole VM (even if the register view is per-vcpu)
> +
> +* KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1:
> +  Holds the state of the firmware controlled workaround to mitigate
> +  CVE-2017-5715, as described under SMCCC_ARCH_WORKAROUND_1 in [1].> +  Accepted values are:
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_NOT_AVAIL: Workaround not available.
> +      The mitigation status for the guest is unknown.
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_AVAIL: The workaround HVC call is
> +      available to the guest and required for the mitigation.
It is unclear to me why we don't report the actual enable status too as
for WA2. Assuming the mitigation is not applied on the source (ie. AVAIL
but SMCCC_ARCH_WORKAROUND_1 not called), do we need it on the destination?
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_UNAFFECTED: The workaround HVC call
> +      is available to the guest, but it is not needed on this VCPU.
> +
> +* KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2:
> +  Holds the state of the firmware controlled workaround to mitigate
> +  CVE-2018-3639, as described under SMCCC_ARCH_WORKAROUND_2 in [1].
This does not really match the terminology used in [1]. At least it is
not straightforward to me.
> +  Accepted values are:
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_NOT_AVAIL: Workaround not available.
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_UNKNOWN: Workaround state unknown.
both included in NOT_SUPPORTED. By the way why did we need to be more
precise here? Seems WA1 also has the UNKNOWN state as part of
UNSUPPORTED. Why isn't it homogeneous?
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_AVAIL: Workaround available, and can
> +      be disabled by a vCPU. If KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_ENABLED is
> +      set, it is active for this vCPU.
> +    KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_UNAFFECTED: Workaround always active
NOT_REQUIRED or 1?
> +      or not needed.
> +
> +[1] https://developer.arm.com/-/media/developer/pdf/ARM_DEN_0070A_Firmware_interfaces_for_mitigating_CVE-2017-5715.pdf
> 
Thanks

Eric