Hi Andre, On 4/15/19 2:34 PM, Andre Przywara wrote: > On Thu, 21 Mar 2019 13:33:18 +0100 > Auger Eric <eric.auger@xxxxxxxxxx> wrote: > > Hi Eric, > > many thanks for the feedback, turns out that this description here is > somewhat tricky. We only really care about the guest's view, but the > various host states (support configured out, disabled on the command line, > not supported by firmware, not needed on this CPU) play into this as well. > So in my understanding the information presented here does not necessarily > copy the information returned by the firmware interface, it just presents > what the guest can expect from the firmware interface. > > I tried to reword the description in v5 to address any ambiguities, > meanwhile trying to answer your specific questions below. > Please have a look at v5 and tell me if that is clearer now. > >> 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@xxxxxxx> >>> --- >>> 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? > > In contrast to WORKAROUND_2 this one here is stateless. KVM potentially > offers the HVC call and says whether it's needed or not: What the guest > makes out of it, is up to the guest's discretion. The guest has to call > this HVC every time it wants to mitigate the issue. KVM needs to maintain > continuity on migration regarding offering this HVC call and its effect. OK that's clear now. Thank you for the explanations. > >>> + 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. > > Not really sure what you mean by this, exactly? Well I meant the SMCCC spec uses the NOT_SUPPORTED, NOT_REQUIRED. The difficulty somewhat is each layer has a different terminology, SMCC spec, kernel, user and the translation was 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? > > Most differences originate in the statelessness of WORKAROUND_1. Also we > have this information here in the host kernel: NOT_AVAIL is returned when > the workaround has been explicitly disabled on the command line, UNKNOWN > is used when the workaround has been configured out. > We use this to avoid migration to a "worse place": UNKNOWN is never worse > than "not available". > >>> + 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? > > I think we don't care. KVM always offers this HVC call. I think the trick > here is that is not the firmware interface, but a description of it. It > just tells what the guest is able to call, we don't copy the results of > the firmware call in here. Yes the confusion came that only the FW spec gets quoted. To understand the uapi definition knowledge about the overall mitigation scheme is requested. Thanks Eric > > Cheers, > Andre. > > >>> + or not needed. >>> + >>> +[1] https://developer.arm.com/-/media/developer/pdf/ARM_DEN_0070A_Firmware_interfaces_for_mitigating_CVE-2017-5715.pdf >>> >> Thanks >> >> Eric >