| Message ID | 1449485618-9443-6-git-send-email-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@arm.com> > --- > 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@arm.com> > + * > + * 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? 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. > + switch (max_lr_idx) { > + case 15: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)] = read_gicreg(ICH_LR15_EL2); > + case 14: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)] = read_gicreg(ICH_LR14_EL2); > + case 13: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)] = read_gicreg(ICH_LR13_EL2); > + case 12: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)] = read_gicreg(ICH_LR12_EL2); > + case 11: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)] = read_gicreg(ICH_LR11_EL2); > + case 10: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)] = read_gicreg(ICH_LR10_EL2); > + case 9: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)] = read_gicreg(ICH_LR9_EL2); > + case 8: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)] = read_gicreg(ICH_LR8_EL2); > + case 7: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)] = read_gicreg(ICH_LR7_EL2); > + case 6: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)] = read_gicreg(ICH_LR6_EL2); > + case 5: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)] = read_gicreg(ICH_LR5_EL2); > + case 4: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)] = read_gicreg(ICH_LR4_EL2); > + case 3: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)] = read_gicreg(ICH_LR3_EL2); > + case 2: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)] = read_gicreg(ICH_LR2_EL2); > + case 1: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)] = read_gicreg(ICH_LR1_EL2); > + case 0: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)] = read_gicreg(ICH_LR0_EL2); > + } > + > + switch (nr_pri_bits) { > + case 7: > + cpu_if->vgic_ap0r[3] = read_gicreg(ICH_AP0R3_EL2); > + cpu_if->vgic_ap0r[2] = read_gicreg(ICH_AP0R2_EL2); > + case 6: > + cpu_if->vgic_ap0r[1] = read_gicreg(ICH_AP0R1_EL2); > + default: > + cpu_if->vgic_ap0r[0] = read_gicreg(ICH_AP0R0_EL2); > + } > + > + switch (nr_pri_bits) { > + case 7: > + cpu_if->vgic_ap1r[3] = read_gicreg(ICH_AP1R3_EL2); > + cpu_if->vgic_ap1r[2] = read_gicreg(ICH_AP1R2_EL2); > + case 6: > + cpu_if->vgic_ap1r[1] = read_gicreg(ICH_AP1R1_EL2); > + default: > + cpu_if->vgic_ap1r[0] = read_gicreg(ICH_AP1R0_EL2); > + } > + > + val = read_gicreg(ICC_SRE_EL2); > + write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2); > + isb(); /* Make sure ENABLE is set at EL2 before setting SRE at EL1 */ > + write_gicreg(1, ICC_SRE_EL1); > +} > + > +void __hyp_text __vgic_v3_restore_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; > + > + /* > + * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a > + * Group0 interrupt (as generated in GICv2 mode) to be > + * delivered as a FIQ to the guest, with potentially fatal > + * consequences. So we must make sure that ICC_SRE_EL1 has > + * been actually programmed with the value we want before > + * starting to mess with the rest of the GIC. > + */ > + write_gicreg(cpu_if->vgic_sre, ICC_SRE_EL1); > + isb(); > + > + write_gicreg(cpu_if->vgic_hcr, ICH_HCR_EL2); > + write_gicreg(cpu_if->vgic_vmcr, ICH_VMCR_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); > + > + switch (nr_pri_bits) { > + case 7: > + write_gicreg(cpu_if->vgic_ap1r[3], ICH_AP1R3_EL2); > + write_gicreg(cpu_if->vgic_ap1r[2], ICH_AP1R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap1r[1], ICH_AP1R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap1r[0], ICH_AP1R0_EL2); > + } > + > + switch (nr_pri_bits) { > + case 7: > + write_gicreg(cpu_if->vgic_ap0r[3], ICH_AP0R3_EL2); > + write_gicreg(cpu_if->vgic_ap0r[2], ICH_AP0R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap0r[1], ICH_AP0R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap0r[0], ICH_AP0R0_EL2); > + } > + Same comments here. > + switch (max_lr_idx) { > + case 15: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)], ICH_LR15_EL2); > + case 14: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)], ICH_LR14_EL2); > + case 13: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)], ICH_LR13_EL2); > + case 12: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)], ICH_LR12_EL2); > + case 11: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)], ICH_LR11_EL2); > + case 10: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)], ICH_LR10_EL2); > + case 9: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)], ICH_LR9_EL2); > + case 8: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)], ICH_LR8_EL2); > + case 7: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)], ICH_LR7_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)], ICH_LR6_EL2); > + case 5: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)], ICH_LR5_EL2); > + case 4: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)], ICH_LR4_EL2); > + case 3: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)], ICH_LR3_EL2); > + case 2: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)], ICH_LR2_EL2); > + case 1: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)], ICH_LR1_EL2); > + case 0: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)], ICH_LR0_EL2); > + } > + > + /* > + * Ensures that the above will have reached the > + * (re)distributors. This ensure the guest will read the > + * correct values from the memory-mapped interface. > + */ > + isb(); > + dsb(sy); > + > + /* > + * Prevent the guest from touching the GIC system registers if > + * SRE isn't enabled for GICv3 emulation. > + */ > + if (!cpu_if->vgic_sre) { > + write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE, > + ICC_SRE_EL2); > + } > +} > + > +u64 __hyp_text __vgic_v3_read_ich_vtr_el2(void) > +{ > + return read_gicreg(ICH_VTR_EL2); > +} > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@arm.com> >> --- >> 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@arm.com> >> + * >> + * 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? > 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). Thanks, M.
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@arm.com> >>> --- >>> 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@arm.com> >>> + * >>> + * 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. > >> 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. > > Thanks, > > M. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@arm.com> >>>> --- >>>> 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@arm.com> >>>> + * >>>> + * 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); [...] } 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.
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@arm.com> >>>>> --- >>>>> 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@arm.com> >>>>> + * >>>>> + * 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@arm.com> >>>>>> --- >>>>>> 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@arm.com> >>>>>> + * >>>>>> + * 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.
On 12/7/2015 10:20 AM, Marc Zyngier wrote: > On 07/12/15 18:05, Mario Smarduch wrote: >> >> >> On 12/7/2015 9:37 AM, Marc Zyngier wrote: [...] >>> >> >> 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... Interesting, thanks for the dump I'm no expert on pipeline optimizations but I'm wondering with these system register accesses can these be executed out of order provided you didn't have what I thinks are write after read dependencies? It's only 4 registers here, there are some other longer stretches in subsequent patches. I minor note here is some white space in this patch. > > Thanks, > > M. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 7 Dec 2015 18:14:36 -0800 Mario Smarduch <m.smarduch@samsung.com> wrote: > > > On 12/7/2015 10:20 AM, Marc Zyngier wrote: > > On 07/12/15 18:05, Mario Smarduch wrote: > >> > >> > >> On 12/7/2015 9:37 AM, Marc Zyngier wrote: > [...] > >>> > >> > >> 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... > > Interesting, thanks for the dump I'm no expert on pipeline optimizations but I'm > wondering with these system register accesses can these be executed out of order > provided you didn't have what I thinks are write after read dependencies? System-register reads can be executed out of order, that is not a problem. Even the stores can be executed out of order as the CPU renames the GP registers (depending on the microarchitecture, of course). Now, what I'd *really* like to see is GCC to output something similar to what we have in the original code, where we use as many registers as possible to store the data, and output it in one go, possibly using strp. So far, I haven't found a way to convince the compiler to do so. > It's only 4 registers here, there are some other longer stretches in subsequent > patches. > > I minor note here is some white space in this patch. Ah, thanks for letting me know. I'll chase those. Thanks, M.
On Mon, Dec 07, 2015 at 10:53:21AM +0000, 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@arm.com> > --- > 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@arm.com> > + * > + * 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); > + > + switch (max_lr_idx) { > + case 15: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)] = read_gicreg(ICH_LR15_EL2); > + case 14: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)] = read_gicreg(ICH_LR14_EL2); > + case 13: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)] = read_gicreg(ICH_LR13_EL2); > + case 12: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)] = read_gicreg(ICH_LR12_EL2); > + case 11: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)] = read_gicreg(ICH_LR11_EL2); > + case 10: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)] = read_gicreg(ICH_LR10_EL2); > + case 9: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)] = read_gicreg(ICH_LR9_EL2); > + case 8: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)] = read_gicreg(ICH_LR8_EL2); > + case 7: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)] = read_gicreg(ICH_LR7_EL2); > + case 6: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)] = read_gicreg(ICH_LR6_EL2); > + case 5: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)] = read_gicreg(ICH_LR5_EL2); > + case 4: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)] = read_gicreg(ICH_LR4_EL2); > + case 3: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)] = read_gicreg(ICH_LR3_EL2); > + case 2: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)] = read_gicreg(ICH_LR2_EL2); > + case 1: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)] = read_gicreg(ICH_LR1_EL2); > + case 0: > + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)] = read_gicreg(ICH_LR0_EL2); > + } > + > + switch (nr_pri_bits) { > + case 7: > + cpu_if->vgic_ap0r[3] = read_gicreg(ICH_AP0R3_EL2); > + cpu_if->vgic_ap0r[2] = read_gicreg(ICH_AP0R2_EL2); > + case 6: > + cpu_if->vgic_ap0r[1] = read_gicreg(ICH_AP0R1_EL2); > + default: > + cpu_if->vgic_ap0r[0] = read_gicreg(ICH_AP0R0_EL2); > + } > + > + switch (nr_pri_bits) { > + case 7: > + cpu_if->vgic_ap1r[3] = read_gicreg(ICH_AP1R3_EL2); > + cpu_if->vgic_ap1r[2] = read_gicreg(ICH_AP1R2_EL2); > + case 6: > + cpu_if->vgic_ap1r[1] = read_gicreg(ICH_AP1R1_EL2); > + default: > + cpu_if->vgic_ap1r[0] = read_gicreg(ICH_AP1R0_EL2); > + } > + > + val = read_gicreg(ICC_SRE_EL2); > + write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2); > + isb(); /* Make sure ENABLE is set at EL2 before setting SRE at EL1 */ > + write_gicreg(1, ICC_SRE_EL1); > +} > + > +void __hyp_text __vgic_v3_restore_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; > + > + /* > + * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a > + * Group0 interrupt (as generated in GICv2 mode) to be > + * delivered as a FIQ to the guest, with potentially fatal > + * consequences. So we must make sure that ICC_SRE_EL1 has > + * been actually programmed with the value we want before > + * starting to mess with the rest of the GIC. > + */ > + write_gicreg(cpu_if->vgic_sre, ICC_SRE_EL1); > + isb(); > + > + write_gicreg(cpu_if->vgic_hcr, ICH_HCR_EL2); > + write_gicreg(cpu_if->vgic_vmcr, ICH_VMCR_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); > + > + switch (nr_pri_bits) { > + case 7: > + write_gicreg(cpu_if->vgic_ap1r[3], ICH_AP1R3_EL2); > + write_gicreg(cpu_if->vgic_ap1r[2], ICH_AP1R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap1r[1], ICH_AP1R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap1r[0], ICH_AP1R0_EL2); > + } > + white space issues > + switch (nr_pri_bits) { > + case 7: > + write_gicreg(cpu_if->vgic_ap0r[3], ICH_AP0R3_EL2); > + write_gicreg(cpu_if->vgic_ap0r[2], ICH_AP0R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap0r[1], ICH_AP0R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap0r[0], ICH_AP0R0_EL2); > + } > + > + switch (max_lr_idx) { > + case 15: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)], ICH_LR15_EL2); > + case 14: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)], ICH_LR14_EL2); > + case 13: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)], ICH_LR13_EL2); > + case 12: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)], ICH_LR12_EL2); > + case 11: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)], ICH_LR11_EL2); > + case 10: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)], ICH_LR10_EL2); > + case 9: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)], ICH_LR9_EL2); > + case 8: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)], ICH_LR8_EL2); > + case 7: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)], ICH_LR7_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)], ICH_LR6_EL2); > + case 5: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)], ICH_LR5_EL2); > + case 4: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)], ICH_LR4_EL2); > + case 3: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)], ICH_LR3_EL2); > + case 2: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)], ICH_LR2_EL2); > + case 1: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)], ICH_LR1_EL2); > + case 0: > + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)], ICH_LR0_EL2); > + } > + > + /* > + * Ensures that the above will have reached the > + * (re)distributors. This ensure the guest will read the You fixed the wrong instance of 'ensure' ;) > + * correct values from the memory-mapped interface. > + */ > + isb(); > + dsb(sy); > + > + /* > + * Prevent the guest from touching the GIC system registers if > + * SRE isn't enabled for GICv3 emulation. > + */ > + if (!cpu_if->vgic_sre) { > + write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE, > + ICC_SRE_EL2); > + } > +} > + > +u64 __hyp_text __vgic_v3_read_ich_vtr_el2(void) > +{ > + return read_gicreg(ICH_VTR_EL2); > +} Otherwise: Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@arm.com> + * + * 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); + + switch (max_lr_idx) { + case 15: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)] = read_gicreg(ICH_LR15_EL2); + case 14: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)] = read_gicreg(ICH_LR14_EL2); + case 13: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)] = read_gicreg(ICH_LR13_EL2); + case 12: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)] = read_gicreg(ICH_LR12_EL2); + case 11: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)] = read_gicreg(ICH_LR11_EL2); + case 10: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)] = read_gicreg(ICH_LR10_EL2); + case 9: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)] = read_gicreg(ICH_LR9_EL2); + case 8: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)] = read_gicreg(ICH_LR8_EL2); + case 7: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)] = read_gicreg(ICH_LR7_EL2); + case 6: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)] = read_gicreg(ICH_LR6_EL2); + case 5: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)] = read_gicreg(ICH_LR5_EL2); + case 4: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)] = read_gicreg(ICH_LR4_EL2); + case 3: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)] = read_gicreg(ICH_LR3_EL2); + case 2: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)] = read_gicreg(ICH_LR2_EL2); + case 1: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)] = read_gicreg(ICH_LR1_EL2); + case 0: + cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)] = read_gicreg(ICH_LR0_EL2); + } + + switch (nr_pri_bits) { + case 7: + cpu_if->vgic_ap0r[3] = read_gicreg(ICH_AP0R3_EL2); + cpu_if->vgic_ap0r[2] = read_gicreg(ICH_AP0R2_EL2); + case 6: + cpu_if->vgic_ap0r[1] = read_gicreg(ICH_AP0R1_EL2); + default: + cpu_if->vgic_ap0r[0] = read_gicreg(ICH_AP0R0_EL2); + } + + switch (nr_pri_bits) { + case 7: + cpu_if->vgic_ap1r[3] = read_gicreg(ICH_AP1R3_EL2); + cpu_if->vgic_ap1r[2] = read_gicreg(ICH_AP1R2_EL2); + case 6: + cpu_if->vgic_ap1r[1] = read_gicreg(ICH_AP1R1_EL2); + default: + cpu_if->vgic_ap1r[0] = read_gicreg(ICH_AP1R0_EL2); + } + + val = read_gicreg(ICC_SRE_EL2); + write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2); + isb(); /* Make sure ENABLE is set at EL2 before setting SRE at EL1 */ + write_gicreg(1, ICC_SRE_EL1); +} + +void __hyp_text __vgic_v3_restore_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; + + /* + * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a + * Group0 interrupt (as generated in GICv2 mode) to be + * delivered as a FIQ to the guest, with potentially fatal + * consequences. So we must make sure that ICC_SRE_EL1 has + * been actually programmed with the value we want before + * starting to mess with the rest of the GIC. + */ + write_gicreg(cpu_if->vgic_sre, ICC_SRE_EL1); + isb(); + + write_gicreg(cpu_if->vgic_hcr, ICH_HCR_EL2); + write_gicreg(cpu_if->vgic_vmcr, ICH_VMCR_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); + + switch (nr_pri_bits) { + case 7: + write_gicreg(cpu_if->vgic_ap1r[3], ICH_AP1R3_EL2); + write_gicreg(cpu_if->vgic_ap1r[2], ICH_AP1R2_EL2); + case 6: + write_gicreg(cpu_if->vgic_ap1r[1], ICH_AP1R1_EL2); + default: + write_gicreg(cpu_if->vgic_ap1r[0], ICH_AP1R0_EL2); + } + + switch (nr_pri_bits) { + case 7: + write_gicreg(cpu_if->vgic_ap0r[3], ICH_AP0R3_EL2); + write_gicreg(cpu_if->vgic_ap0r[2], ICH_AP0R2_EL2); + case 6: + write_gicreg(cpu_if->vgic_ap0r[1], ICH_AP0R1_EL2); + default: + write_gicreg(cpu_if->vgic_ap0r[0], ICH_AP0R0_EL2); + } + + switch (max_lr_idx) { + case 15: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(15)], ICH_LR15_EL2); + case 14: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(14)], ICH_LR14_EL2); + case 13: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(13)], ICH_LR13_EL2); + case 12: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(12)], ICH_LR12_EL2); + case 11: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(11)], ICH_LR11_EL2); + case 10: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(10)], ICH_LR10_EL2); + case 9: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(9)], ICH_LR9_EL2); + case 8: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(8)], ICH_LR8_EL2); + case 7: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(7)], ICH_LR7_EL2); + case 6: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(6)], ICH_LR6_EL2); + case 5: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(5)], ICH_LR5_EL2); + case 4: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(4)], ICH_LR4_EL2); + case 3: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(3)], ICH_LR3_EL2); + case 2: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(2)], ICH_LR2_EL2); + case 1: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(1)], ICH_LR1_EL2); + case 0: + write_gicreg(cpu_if->vgic_lr[VGIC_V3_LR_INDEX(0)], ICH_LR0_EL2); + } + + /* + * Ensures that the above will have reached the + * (re)distributors. This ensure the guest will read the + * correct values from the memory-mapped interface. + */ + isb(); + dsb(sy); + + /* + * Prevent the guest from touching the GIC system registers if + * SRE isn't enabled for GICv3 emulation. + */ + if (!cpu_if->vgic_sre) { + write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE, + ICC_SRE_EL2); + } +} + +u64 __hyp_text __vgic_v3_read_ich_vtr_el2(void) +{ + return read_gicreg(ICH_VTR_EL2); +}
Implement the vgic-v3 save restore as a direct translation of the assembly code version. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- 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