Re: [PATCH v3 05/22] arm64: KVM: Implement vgic-v3 save/restore

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux