[PATCH v2 7/7] ARM: KVM: Unlock vgic-v3 support
Vladimir Murzin
vladimir.murzin at arm.com
Tue Sep 6 06:18:10 PDT 2016
On 05/09/16 12:29, Christoffer Dall wrote:
> On Tue, Aug 16, 2016 at 11:46:58AM +0100, Vladimir Murzin wrote:
>> It is time to get access to common version of vgic-v3.
>
> common version?
>
Since patch#2 it not private to arm64 or I should rephrase this?
>>
>> We basically would need to tell build system how to pick it up and
>> undo KVM_ARM_VGIC_V3 guarding introduced in 4f64cb6 ("arm/arm64: KVM:
>> Only allow 64bit hosts to build VGICv3") and remove stubs. However,
>> since vgic got ITS support KVM_ARM_VGIC_V3 tend to protect a little
>> bit more than just plain vgic-v3 - this guard is used for ITS too
>> which is not supported in 32-bit world yet. So, along with removal of
>> KVM_ARM_VGIC_V3 guard introduce the new one - KVM_ARM_VGIC_V3_ITS to
>> protect ITS related code.
>
> I don't find this paragraph particularly helpful, I'm afraid.
>
Sorry for that. It seems to much for one patch, I'll split it per your
suggestion bellow.
>>
>> The only unpleasant part is how we decide which save/restore sequence to
>> use under __vgic_save_state() - we don't have patching framework in hand
>> like arm64, so have to check runtime on every invocation.
>
> Do static keys work on 32-bit arm?
>
I'll try to use them.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin at arm.com>
>> ---
>> arch/arm/include/asm/kvm_host.h | 4 +++
>> arch/arm/include/asm/kvm_hyp.h | 5 ++++
>> arch/arm/kvm/Makefile | 2 ++
>> arch/arm/kvm/hyp/Makefile | 1 +
>> arch/arm/kvm/hyp/switch.c | 20 +++++++++++--
>> arch/arm64/kvm/Kconfig | 4 +--
>> include/kvm/arm_vgic.h | 8 ------
>> virt/kvm/arm/vgic/vgic-kvm-device.c | 10 ++-----
>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 ++
>> virt/kvm/arm/vgic/vgic-mmio.c | 2 --
>> virt/kvm/arm/vgic/vgic-mmio.h | 3 --
>> virt/kvm/arm/vgic/vgic.h | 54 ++---------------------------------
>> 12 files changed, 38 insertions(+), 77 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index de338d9..1312597 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -39,7 +39,11 @@
>>
>> #include <kvm/arm_vgic.h>
>>
>> +#ifdef CONFIG_ARM_GIC_V3
>> +#define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
>> +#else
>> #define KVM_MAX_VCPUS VGIC_V2_MAX_CPUS
>> +#endif
>>
>> #define KVM_REQ_VCPU_EXIT 8
>>
>> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
>> index e604ad68..95669b3 100644
>> --- a/arch/arm/include/asm/kvm_hyp.h
>> +++ b/arch/arm/include/asm/kvm_hyp.h
>> @@ -88,6 +88,8 @@
>>
>> #define VFP_FPEXC __ACCESS_VFP(FPEXC)
>>
>> +#define ID_PFR1 __ACCESS_CP15(c0, 0, c1, 1)
>> +
>> /* AArch64 compatibility macros, only for the timer so far */
>> #define read_sysreg_el0(r) read_sysreg(r##_el0)
>> #define write_sysreg_el0(v, r) write_sysreg(v, r##_el0)
>> @@ -103,6 +105,9 @@ void __timer_restore_state(struct kvm_vcpu *vcpu);
>> 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);
>> +
>> void __sysreg_save_state(struct kvm_cpu_context *ctxt);
>> void __sysreg_restore_state(struct kvm_cpu_context *ctxt);
>>
>> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
>> index 10d77a6..043d817f 100644
>> --- a/arch/arm/kvm/Makefile
>> +++ b/arch/arm/kvm/Makefile
>> @@ -26,8 +26,10 @@ obj-y += $(KVM)/arm/vgic/vgic.o
>> obj-y += $(KVM)/arm/vgic/vgic-init.o
>> obj-y += $(KVM)/arm/vgic/vgic-irqfd.o
>> obj-y += $(KVM)/arm/vgic/vgic-v2.o
>> +obj-y += $(KVM)/arm/vgic/vgic-v3.o
>> obj-y += $(KVM)/arm/vgic/vgic-mmio.o
>> obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
>> +obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o
>> obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o
>> obj-y += $(KVM)/irqchip.o
>> obj-y += $(KVM)/arm/arch_timer.o
>> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
>> index 8dfa5f7..3023bb5 100644
>> --- a/arch/arm/kvm/hyp/Makefile
>> +++ b/arch/arm/kvm/hyp/Makefile
>> @@ -5,6 +5,7 @@
>> KVM=../../../../virt/kvm
>>
>> obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
>> +obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
>> obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
>>
>> obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>> diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
>> index b13caa9..9666bae 100644
>> --- a/arch/arm/kvm/hyp/switch.c
>> +++ b/arch/arm/kvm/hyp/switch.c
>> @@ -15,6 +15,8 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include <linux/irqchip/arm-gic-v3.h>
>> +
>> #include <asm/kvm_asm.h>
>> #include <asm/kvm_hyp.h>
>>
>> @@ -74,14 +76,28 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
>> write_sysreg(read_sysreg(MIDR), VPIDR);
>> }
>>
>> +static bool __hyp_text __has_useable_gicv3_cpuif(void)
>> +{
>> + if (IS_ENABLED(CONFIG_ARM_GIC_V3) && (read_sysreg(ID_PFR1) >> 28))
>
> Do we have a define for bit 28 we could use?
>
> Does this actually work on all v7 boards? The v7 ARM ARM seems to state
> that this bitfield is Reserved, UNK. Does that somehow mean 'is going
> to be zero'?
>
>> + return !!(read_sysreg(ICC_HSRE) & ICC_SRE_EL2_SRE);
>> + else
>> + return false;
>> +}
>> +
>> static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
>> {
>> - __vgic_v2_save_state(vcpu);
>> + if (__has_useable_gicv3_cpuif())
>> + __vgic_v3_save_state(vcpu);
>> + else
>> + __vgic_v2_save_state(vcpu);
>> }
>>
>> static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
>> {
>> - __vgic_v2_restore_state(vcpu);
>> + if (__has_useable_gicv3_cpuif())
>> + __vgic_v3_restore_state(vcpu);
>> + else
>> + __vgic_v2_restore_state(vcpu);
>> }
>>
>> static bool __hyp_text __populate_fault_info(struct kvm_vcpu *vcpu)
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index 9c9edc9..6eaf12c 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -16,7 +16,7 @@ menuconfig VIRTUALIZATION
>>
>> if VIRTUALIZATION
>>
>> -config KVM_ARM_VGIC_V3
>> +config KVM_ARM_VGIC_V3_ITS
>
> I feel like this could have been simplified with adding the new ITS
> guard in a separate patch.
>
> In fact, you could then have a separate patch that enables compilation
> of the gicv3 code and removes the static inlines etc., and then finally
> a patch that adds in the logic for the world switch.
Agree. Will rework this patch.
Thanks
Vladimir
More information about the linux-arm-kernel
mailing list