On 07/12/15 18:05, Mario Smarduch wrote: > > > On 12/7/2015 9:37 AM, Marc Zyngier wrote: >> On 07/12/15 17:18, Mario Smarduch wrote: >>> >>> >>> On 12/7/2015 8:52 AM, Marc Zyngier wrote: >>>> Hi Mario, >>>> >>>> On 07/12/15 16:40, Mario Smarduch wrote: >>>>> Hi Marc, >>>>> >>>>> On 12/7/2015 2:53 AM, Marc Zyngier wrote: >>>>>> Implement the vgic-v3 save restore as a direct translation of >>>>>> the assembly code version. >>>>>> >>>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >>>>>> --- >>>>>> arch/arm64/kvm/hyp/Makefile | 1 + >>>>>> arch/arm64/kvm/hyp/hyp.h | 3 + >>>>>> arch/arm64/kvm/hyp/vgic-v3-sr.c | 226 ++++++++++++++++++++++++++++++++++++++++ >>>>>> 3 files changed, 230 insertions(+) >>>>>> create mode 100644 arch/arm64/kvm/hyp/vgic-v3-sr.c >>>>>> >>>>>> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile >>>>>> index d8d5968..d1e38ce 100644 >>>>>> --- a/arch/arm64/kvm/hyp/Makefile >>>>>> +++ b/arch/arm64/kvm/hyp/Makefile >>>>>> @@ -3,3 +3,4 @@ >>>>>> # >>>>>> >>>>>> obj-$(CONFIG_KVM_ARM_HOST) += vgic-v2-sr.o >>>>>> +obj-$(CONFIG_KVM_ARM_HOST) += vgic-v3-sr.o >>>>>> diff --git a/arch/arm64/kvm/hyp/hyp.h b/arch/arm64/kvm/hyp/hyp.h >>>>>> index ac63553..5759f9f 100644 >>>>>> --- a/arch/arm64/kvm/hyp/hyp.h >>>>>> +++ b/arch/arm64/kvm/hyp/hyp.h >>>>>> @@ -32,5 +32,8 @@ >>>>>> void __vgic_v2_save_state(struct kvm_vcpu *vcpu); >>>>>> void __vgic_v2_restore_state(struct kvm_vcpu *vcpu); >>>>>> >>>>>> +void __vgic_v3_save_state(struct kvm_vcpu *vcpu); >>>>>> +void __vgic_v3_restore_state(struct kvm_vcpu *vcpu); >>>>>> + >>>>>> #endif /* __ARM64_KVM_HYP_H__ */ >>>>>> >>>>>> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c >>>>>> new file mode 100644 >>>>>> index 0000000..78d05f3 >>>>>> --- /dev/null >>>>>> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c >>>>>> @@ -0,0 +1,226 @@ >>>>>> +/* >>>>>> + * Copyright (C) 2012-2015 - ARM Ltd >>>>>> + * Author: Marc Zyngier <marc.zyngier@xxxxxxx> >>>>>> + * >>>>>> + * This program is free software; you can redistribute it and/or modify >>>>>> + * it under the terms of the GNU General Public License version 2 as >>>>>> + * published by the Free Software Foundation. >>>>>> + * >>>>>> + * This program is distributed in the hope that it will be useful, >>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>>> + * GNU General Public License for more details. >>>>>> + * >>>>>> + * You should have received a copy of the GNU General Public License >>>>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >>>>>> + */ >>>>>> + >>>>>> +#include <linux/compiler.h> >>>>>> +#include <linux/irqchip/arm-gic-v3.h> >>>>>> +#include <linux/kvm_host.h> >>>>>> + >>>>>> +#include <asm/kvm_mmu.h> >>>>>> + >>>>>> +#include "hyp.h" >>>>>> + >>>>>> +#define vtr_to_max_lr_idx(v) ((v) & 0xf) >>>>>> +#define vtr_to_nr_pri_bits(v) (((u32)(v) >> 29) + 1) >>>>>> + >>>>>> +#define read_gicreg(r) \ >>>>>> + ({ \ >>>>>> + u64 reg; \ >>>>>> + asm volatile("mrs_s %0, " __stringify(r) : "=r" (reg)); \ >>>>>> + reg; \ >>>>>> + }) >>>>>> + >>>>>> +#define write_gicreg(v,r) \ >>>>>> + do { \ >>>>>> + u64 __val = (v); \ >>>>>> + asm volatile("msr_s " __stringify(r) ", %0" : : "r" (__val));\ >>>>>> + } while (0) >>>>>> + >>>>>> +/* vcpu is already in the HYP VA space */ >>>>>> +void __hyp_text __vgic_v3_save_state(struct kvm_vcpu *vcpu) >>>>>> +{ >>>>>> + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >>>>>> + u64 val; >>>>>> + u32 max_lr_idx, nr_pri_bits; >>>>>> + >>>>>> + /* >>>>>> + * Make sure stores to the GIC via the memory mapped interface >>>>>> + * are now visible to the system register interface. >>>>>> + */ >>>>>> + dsb(st); >>>>>> + >>>>>> + cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2); >>>>>> + cpu_if->vgic_misr = read_gicreg(ICH_MISR_EL2); >>>>>> + cpu_if->vgic_eisr = read_gicreg(ICH_EISR_EL2); >>>>>> + cpu_if->vgic_elrsr = read_gicreg(ICH_ELSR_EL2); >>>>>> + >>>>>> + write_gicreg(0, ICH_HCR_EL2); >>>>>> + val = read_gicreg(ICH_VTR_EL2); >>>>>> + max_lr_idx = vtr_to_max_lr_idx(val); >>>>>> + nr_pri_bits = vtr_to_nr_pri_bits(val); >>>>>> + >>>>> Can you setup a base pointer to cpu_if->vgic_lr and use an offset? >>>> >>>> I could, but I fail to see what we'd gain by using this (aside from >>>> slightly shorter lines). Or am I completely missing the point? >>> >>> Skip adding the offset of vgic_lr to cpu_if pointer. >> >> But if we do that, we also change the layout that EL1 expect. Assume we >> do something like this: >> >> u64 *current_lr = cpu_if->vgic_lr; >> >> switch (max_lr_idx) { >> case 15: >> current_lr++ = read_gicreg(ICH_LR15_EL2); >> case 14: >> current_lr++ = read_gicreg(ICH_LR14_EL2); >> [...] >> } >> > > I was thinking something like 'current_lr[VGIC_V3_LR_INDEX(...)]'. That doesn't change anything, the compiler is perfectly able to optimize something like this: [...] ffffffc0007f31ac: 38624862 ldrb w2, [x3,w2,uxtw] ffffffc0007f31b0: 10000063 adr x3, ffffffc0007f31bc <__vgic_v3_save_state+0x64> ffffffc0007f31b4: 8b228862 add x2, x3, w2, sxtb #2 ffffffc0007f31b8: d61f0040 br x2 ffffffc0007f31bc: d53ccde2 mrs x2, s3_4_c12_c13_7 ffffffc0007f31c0: f9001c02 str x2, [x0,#56] ffffffc0007f31c4: d53ccdc2 mrs x2, s3_4_c12_c13_6 ffffffc0007f31c8: f9002002 str x2, [x0,#64] ffffffc0007f31cc: d53ccda2 mrs x2, s3_4_c12_c13_5 ffffffc0007f31d0: f9002402 str x2, [x0,#72] ffffffc0007f31d4: d53ccd82 mrs x2, s3_4_c12_c13_4 ffffffc0007f31d8: f9002802 str x2, [x0,#80] ffffffc0007f31dc: d53ccd62 mrs x2, s3_4_c12_c13_3 ffffffc0007f31e0: f9002c02 str x2, [x0,#88] ffffffc0007f31e4: d53ccd42 mrs x2, s3_4_c12_c13_2 ffffffc0007f31e8: f9003002 str x2, [x0,#96] ffffffc0007f31ec: d53ccd22 mrs x2, s3_4_c12_c13_1 ffffffc0007f31f0: f9003402 str x2, [x0,#104] ffffffc0007f31f4: d53ccd02 mrs x2, s3_4_c12_c13_0 ffffffc0007f31f8: f9003802 str x2, [x0,#112] ffffffc0007f31fc: d53ccce2 mrs x2, s3_4_c12_c12_7 ffffffc0007f3200: f9003c02 str x2, [x0,#120] ffffffc0007f3204: d53cccc2 mrs x2, s3_4_c12_c12_6 ffffffc0007f3208: f9004002 str x2, [x0,#128] ffffffc0007f320c: d53ccca2 mrs x2, s3_4_c12_c12_5 ffffffc0007f3210: f9004402 str x2, [x0,#136] ffffffc0007f3214: d53ccc82 mrs x2, s3_4_c12_c12_4 ffffffc0007f3218: f9004802 str x2, [x0,#144] ffffffc0007f321c: d53ccc62 mrs x2, s3_4_c12_c12_3 ffffffc0007f3220: f9004c02 str x2, [x0,#152] ffffffc0007f3224: d53ccc42 mrs x2, s3_4_c12_c12_2 ffffffc0007f3228: f9005002 str x2, [x0,#160] ffffffc0007f322c: d53ccc22 mrs x2, s3_4_c12_c12_1 ffffffc0007f3230: f9005402 str x2, [x0,#168] ffffffc0007f3234: d53ccc02 mrs x2, s3_4_c12_c12_0 ffffffc0007f3238: 7100183f cmp w1, #0x6 ffffffc0007f323c: f9005802 str x2, [x0,#176] As you can see, this is as optimal as it gets, short of being able to find a nice way to use more than one register... Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
