linux-kernel - Re: [PATCH v11 4/6] target-arm: kvm64: detect guest RAS EXTENSION feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 5 Sep 2017 18:26:41 +0100
From:   Peter Maydell <peter.maydell@...aro.org>
To:     Dongjiu Geng <gengdongjiu@...wei.com>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        Igor Mammedov <imammedo@...hat.com>,
        Shannon Zhao <zhaoshenglong@...wei.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        QEMU Developers <qemu-devel@...gnu.org>,
        qemu-arm <qemu-arm@...gnu.org>, kvm-devel <kvm@...r.kernel.org>,
        "edk2-devel@...ts.01.org" <edk2-devel@...ts.01.org>,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Will Deacon <will.deacon@....com>,
        James Morse <james.morse@....com>,
        Tyler Baicar <tbaicar@...eaurora.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Ingo Molnar <mingo@...nel.org>, bp@...e.de,
        shiju.jose@...wei.com, zjzhang@...eaurora.org,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-acpi@...r.kernel.org, devel@...ica.org,
        john.garry@...wei.com, jonathan.cameron@...wei.com,
        shameerali.kolothum.thodi@...wei.com, huangdaode@...ilicon.com,
        wangzhou1@...ilicon.com, Huangshaoyu <huangshaoyu@...wei.com>,
        wuquanming <wuquanming@...wei.com>,
        Linuxarm <linuxarm@...wei.com>, zhengqiang10@...wei.com
Subject: Re: [PATCH v11 4/6] target-arm: kvm64: detect guest RAS EXTENSION feature

On 18 August 2017 at 15:23, Dongjiu Geng <gengdongjiu@...wei.com> wrote:
> check if kvm supports guest RAS EXTENSION. if so, set
> corresponding feature bit for vcpu.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@...wei.com>
> ---
>  linux-headers/linux/kvm.h | 1 +
>  target/arm/cpu.h          | 3 +++
>  target/arm/kvm64.c        | 8 ++++++++
>  3 files changed, 12 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 7971a4f..2aa176e 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -929,6 +929,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_PPC_SMT_POSSIBLE 147
>  #define KVM_CAP_HYPERV_SYNIC2 148
>  #define KVM_CAP_HYPERV_VP_INDEX 149
> +#define KVM_CAP_ARM_RAS_EXTENSION 150
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>

Hi. Changes to linux-headers need to be done as a patch of their
own created using scripts/update-linux-headers.sh run against a
mainline kernel tree (and with a commit message that quotes the
kernel commit hash used). This ensures that we have a consistent
set of headers that don't diverge from the kernel copy.

> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b39d64a..6b0961b 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -611,6 +611,8 @@ struct ARMCPU {
>
>      /* CPU has memory protection unit */
>      bool has_mpu;
> +    /* CPU has ras extension unit */
> +    bool has_ras_extension;
>      /* PMSAv7 MPU number of supported regions */
>      uint32_t pmsav7_dregion;
>
> @@ -1229,6 +1231,7 @@ enum arm_features {
>      ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
>      ARM_FEATURE_PMU, /* has PMU support */
>      ARM_FEATURE_VBAR, /* has cp15 VBAR */
> +    ARM_FEATURE_RAS_EXTENSION, /*has RAS extension support */

Missing space after '/*' ?

>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index a16abc8..0781367 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -518,6 +518,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          unset_feature(&env->features, ARM_FEATURE_PMU);
>      }
>
> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_RAS_EXTENSION)) {
> +        cpu->has_ras_extension = true;
> +        set_feature(&env->features, ARM_FEATURE_RAS_EXTENSION);
> +    } else {
> +        cpu->has_ras_extension = false;
> +        unset_feature(&env->features, ARM_FEATURE_RAS_EXTENSION);
> +    }
> +

Shouldn't we need to also tell the kernel that we actually want
it to expose RAS to the guest? Compare the PMU code in this
function, where we set a kvm_init_features bit to do this.
(This suggests that your ABI for the kernel part of this feature
may not be correct?)

You should also not be calling set_feature() here -- if the
CPU features bit doesn't say "this CPU should have the RAS
extensions" we shouldn't create a CPU with them. Instead
you should set it in kvm_arm_get_host_cpu_features() (again,
compare the PMU code).

thanks
-- PMM