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(...)]'. > with max_lr_idx = 4 (a common case), we end up filling vgic_lr[0..3], > while the rest of the code expects it in vgic_lr[12..15]. This defeats > the point of being able to replace the world switch without changing the > rest of the code. It also means that the position of a given LR in > memory now depends on a runtime constant instead of a compile-time constant. > > If you had something different in mind, please give me some sample code, > because I'm a bit lost as to what you really have in mind. > >>> >>>> Also is there a way to get rid of the constants, that implicitly hard codes max >>>> number of LRs, doesn't make the code portable. >>> >>> Well, it is a sad fact of life that the maximum number of LRs *is* >>> hardcoded to an architectural limit of 16. These are CPU registers, and >>> there is only so many of them (and probably a lot less in practice - >>> filling 4 of them has proved to be an extremely rare case). >> >> Yes I'm aware of that it was 64 (or maybe still is) on armv7 but specs have >> changed from time to time. > > This doesn't have much to do with ARMv7 vs ARMv8, but with the GIC > architecture (well, ARMv7 doesn't support GICv3, so that's a moot point): > > - GICv2: max 64 LRs (in practice, GIC400: 4 LRs) > - GICv3: max 16 LRs (in practice, Cortex-A57: 4 LRs) > > Thanks, > > M. > -- 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