[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFEAcA9TsQg4EXr4MAM2VbrzB9v5UA1F4o9tOCka=3e4wjbBGQ@mail.gmail.com>
Date: Tue, 5 Sep 2017 18:50:36 +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 6/6] target-arm: kvm64: Handle SError interrupt for
the guest OS
On 18 August 2017 at 15:23, Dongjiu Geng <gengdongjiu@...wei.com> wrote:
> When guest OS happens SError interrupt(SEI), it will trap to host.
> Host firstly calls memory failure to deal with this error and decide
> whether it needs to deliver SIGBUS signal to userspace. The advantage
> that using signal to notify is that it can make the notification method
> is general, non-KVM user can also use it. when userspace gets this
> signal and knows this is SError interrupt, it will translate the
> delivered host VA to PA and record this PA to GHES.
>
> Because ARMv8.2 adds an extension to RAS to allow system software insert
> implicit Error Synchronization Barrier operations to isolate the error and
> allow passes specified syndrome to guest OS, so after record the CPER, user
> space calls IOCTL to pass a specified syndrome to KVM, then switch to guest
> OS, guest OS can use the recorded CPER record and syndrome information to
> do the recovery.
>
> The steps are shown below:
> 1. translate the host VA to guest OS PA and record this error PA to HEST table.
> 2. set specified virtual SError syndrome and pass the value to KVM.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@...wei.com>
> Signed-off-by: Quanming Wu <wuquanming@...wei.com>
> ---
> linux-headers/linux/kvm.h | 1 +
> target/arm/internals.h | 1 +
> target/arm/kvm64.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 30 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 2aa176e..10dfcab 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1356,6 +1356,7 @@ struct kvm_s390_ucas_mapping {
> /* Available with KVM_CAP_S390_CMMA_MIGRATION */
> #define KVM_S390_GET_CMMA_BITS _IOWR(KVMIO, 0xb8, struct kvm_s390_cmma_log)
> #define KVM_S390_SET_CMMA_BITS _IOW(KVMIO, 0xb9, struct kvm_s390_cmma_log)
> +#define KVM_ARM_SEI _IO(KVMIO, 0xb10)
>
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index fc0ad6d..18b1cbc 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -237,6 +237,7 @@ enum arm_exception_class {
> #define ARM_EL_ISV (1 << ARM_EL_ISV_SHIFT)
> #define ARM_EL_EC_MASK ((0x3F) << ARM_EL_EC_SHIFT)
> #define ARM_EL_FSC_TYPE (0x3C)
> +#define ARM_EL_ISS_MASK ((1 << ARM_EL_IL_SHIFT) - 1)
>
> #define FSC_SEA (0x10)
> #define FSC_SEA_TTW0 (0x14)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index d3bdab2..b84cb49 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -616,6 +616,22 @@ static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset)
> return -EINVAL;
> }
>
> +static int kvm_inject_arm_sei(CPUState *cs)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> +
> + unsigned long syndrome = env->exception.vaddress;
> + /* set virtual SError syndrome */
> + if (arm_feature(env, ARM_FEATURE_RAS_EXTENSION)) {
> + syndrome = syndrome & ARM_EL_ISS_MASK;
> + } else {
> + syndrome = 0;
> + }
> +
> + return kvm_vcpu_ioctl(CPU(cpu), KVM_ARM_SEI, &syndrome);
This looks odd. If we don't have the RAS extension why do
we need to do anything at all here ?
> +}
> +
> /* Inject synchronous external abort */
> static int kvm_inject_arm_sea(CPUState *c)
> {
> @@ -1007,6 +1023,15 @@ static bool is_abort_sea(unsigned long syndrome)
> }
> }
>
> +static bool is_abort_sei(unsigned long syndrome)
> +{
> + uint8_t ec = ((syndrome & ARM_EL_EC_MASK) >> ARM_EL_EC_SHIFT);
You don't need to bother masking here -- in other places in
QEMU we assume that the EC field is at the top of the word,
so just "syndrome >> ARM_EL_EC_SHIFT" is sufficient.
> + if ((ec != EC_SERROR))
> + return false;
> + else
> + return true;
scripts/checkpatch.pl should tell you that this if needs braces
(it's good to get in the habit of running it on all patches; it
is not always correct, so judgement is required, but it will flag
up some common mistakes).
In this particular case, you should just
return ec == EC_SERROR;
though.
> +}
> +
> void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> {
> ram_addr_t ram_addr;
> @@ -1024,6 +1049,9 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> if (is_abort_sea(env->exception.syndrome)) {
> ghes_update_guest(ACPI_HEST_NOTIFY_SEA, paddr);
> kvm_inject_arm_sea(c);
> + } else if (is_abort_sei(env->exception.syndrome)) {
> + ghes_update_guest(ACPI_HEST_NOTIFY_SEI, paddr);
> + kvm_inject_arm_sei(c);
> }
> return;
> }
> --
> 1.8.3.1
thanks
-- PMM
Powered by blists - more mailing lists