From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend
Date: Wed, 14 Sep 2016 16:47:51 +0100 [thread overview]
Message-ID: <57D97127.3070209@arm.com> (raw)
In-Reply-To: <57D96AA0.2010607@arm.com>
On 14/09/16 16:20, Vladimir Murzin wrote:
> On 13/09/16 10:22, Christoffer Dall wrote:
>> On Tue, Sep 13, 2016 at 10:11:10AM +0100, Marc Zyngier wrote:
>>> On 13/09/16 09:20, Christoffer Dall wrote:
>>>> On Mon, Sep 12, 2016 at 03:49:15PM +0100, Vladimir Murzin wrote:
>>>>> Currently GIC backend is selected via alternative framework and this
>>>>> is fine. We are going to introduce vgic-v3 to 32-bit world and there
>>>>> we don't have patching framework in hand, so we can either check
>>>>> support for GICv3 every time we need to choose which backend to use or
>>>>> try to optimise it by using static keys. The later looks quite
>>>>> promising because we can share logic involved in selecting GIC backend
>>>>> between architectures if both uses static keys.
>>>>>
>>>>> This patch moves arm64 from alternative to static keys framework for
>>>>> selecting GIC backend. For that we embed static key into vgic_global
>>>>> and enable the key during vgic initialisation based on what has
>>>>> already been exposed by the host GIC driver.
>>>>>
>>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm•com>
>>>>> ---
>>>>> arch/arm64/kvm/hyp/switch.c | 21 +++++++++++----------
>>>>> include/kvm/arm_vgic.h | 4 ++++
>>>>> virt/kvm/arm/vgic/vgic-init.c | 4 ++++
>>>>> virt/kvm/arm/vgic/vgic.c | 2 +-
>>>>> 4 files changed, 20 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>>>>> index 5a84b45..d5c4cc5 100644
>>>>> --- a/arch/arm64/kvm/hyp/switch.c
>>>>> +++ b/arch/arm64/kvm/hyp/switch.c
>>>>> @@ -16,6 +16,8 @@
>>>>> */
>>>>>
>>>>> #include <linux/types.h>
>>>>> +#include <linux/jump_label.h>
>>>>> +
>>>>> #include <asm/kvm_asm.h>
>>>>> #include <asm/kvm_hyp.h>
>>>>>
>>>>> @@ -126,17 +128,13 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
>>>>> write_sysreg(0, vttbr_el2);
>>>>> }
>>>>>
>>>>> -static hyp_alternate_select(__vgic_call_save_state,
>>>>> - __vgic_v2_save_state, __vgic_v3_save_state,
>>>>> - ARM64_HAS_SYSREG_GIC_CPUIF);
>>>>> -
>>>>> -static hyp_alternate_select(__vgic_call_restore_state,
>>>>> - __vgic_v2_restore_state, __vgic_v3_restore_state,
>>>>> - ARM64_HAS_SYSREG_GIC_CPUIF);
>>>>> -
>>>>> static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
>>>>> {
>>>>> - __vgic_call_save_state()(vcpu);
>>>>> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
>>>>
>>>> It's a bit weird that we use _unlikely for GICv3 (at least if/when GICv3
>>>> hardware becomes mainstream), but as we don't have another primitive for
>>>> the 'default disabled' case, I suppose that's the best we can do.
>>>
>>> We could always revert the "likelihood" of that test once GICv3 has
>>> conquered the world. Or start patching the 32bit kernel like we do for
>>> 64bit...
>>>
>>>>
>>>>> + __vgic_v3_save_state(vcpu);
>>>>> + else
>>>>> + __vgic_v2_save_state(vcpu);
>>>>> +
>>>>> write_sysreg(read_sysreg(hcr_el2) & ~HCR_INT_OVERRIDE, hcr_el2);
>>>>> }
>>>>>
>>>>> @@ -149,7 +147,10 @@ static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
>>>>> val |= vcpu->arch.irq_lines;
>>>>> write_sysreg(val, hcr_el2);
>>>>>
>>>>> - __vgic_call_restore_state()(vcpu);
>>>>> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
>>>>> + __vgic_v3_restore_state(vcpu);
>>>>> + else
>>>>> + __vgic_v2_restore_state(vcpu);
>>>>> }
>>>>>
>>>>> static bool __hyp_text __true_value(void)
>>>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>>>>> index 19b698e..994665a 100644
>>>>> --- a/include/kvm/arm_vgic.h
>>>>> +++ b/include/kvm/arm_vgic.h
>>>>> @@ -23,6 +23,7 @@
>>>>> #include <linux/types.h>
>>>>> #include <kvm/iodev.h>
>>>>> #include <linux/list.h>
>>>>> +#include <linux/jump_label.h>
>>>>>
>>>>> #define VGIC_V3_MAX_CPUS 255
>>>>> #define VGIC_V2_MAX_CPUS 8
>>>>> @@ -63,6 +64,9 @@ struct vgic_global {
>>>>>
>>>>> /* Only needed for the legacy KVM_CREATE_IRQCHIP */
>>>>> bool can_emulate_gicv2;
>>>>> +
>>>>> + /* GIC system register CPU interface */
>>>>> + struct static_key_false gicv3_cpuif;
>>>>
>>>> Documentation/static-keys.txt says that we are not supposed to use
>>>> struct static_key_false directly. This will obviously work quite
>>>> nicely, but we could consider adding a pair of
>>>> DECLARE_STATIC_KEY_TRUE/FALSE macros that don't have the assignments,
>>>> but obviously this will need an ack from other maintainers.
>>>>
>>>> Thoughts?
>>>
>>> Grepping through the tree shows that we're not the only abusers of this
>>> (dynamic debug is far worse!). Happy to write the additional macros and
>>> submit them if nobody beats me to it.
>>>
>>>>
>>>>
>>>>> };
>>>>>
>>>>> extern struct vgic_global kvm_vgic_global_state;
>>>>> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
>>>>> index 83777c1..14d6718 100644
>>>>> --- a/virt/kvm/arm/vgic/vgic-init.c
>>>>> +++ b/virt/kvm/arm/vgic/vgic-init.c
>>>>> @@ -405,6 +405,10 @@ int kvm_vgic_hyp_init(void)
>>>>> break;
>>>>> case GIC_V3:
>>>>> ret = vgic_v3_probe(gic_kvm_info);
>>>>> + if (!ret) {
>>>>> + static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
>>>>> + kvm_info("GIC system register CPU interface\n");
>>>>
>>>> nit: add enabled to the info message?
>>>>
>>>>> + }
>>>>> break;
>>>>> default:
>>>>> ret = -ENODEV;
>>>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>>>>> index e83b7fe..8a529a7 100644
>>>>> --- a/virt/kvm/arm/vgic/vgic.c
>>>>> +++ b/virt/kvm/arm/vgic/vgic.c
>>>>> @@ -29,7 +29,7 @@
>>>>> #define DEBUG_SPINLOCK_BUG_ON(p)
>>>>> #endif
>>>>>
>>>>> -struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
>>>>> +struct vgic_global __section(.hyp.text) kvm_vgic_global_state = {.gicv3_cpuif = STATIC_KEY_FALSE_INIT,};
>>>>>
>>>>> /*
>>>>> * Locking order is always:
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>
>>>> Overall this looks really nice, as long as we're clear on the static
>>>> keys stuff.
>>>
>>> Indeed, we should get this sorted, though I'm not sure this should be a
>>> blocker for this code.
>>>
>> Agreed, let's ship it!
>
> To make it clear, should I respin with "enabled" into the info message
> and macros for static keys?
I think we can fix the message up when applying the patches. As for the
macros, we should have a separate series that does it treewide.
Christoffer?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2016-09-14 15:47 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-12 14:49 [PATCH v4 00/10] ARM: KVM: Support for vgic-v3 Vladimir Murzin
2016-09-12 14:49 ` [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend Vladimir Murzin
2016-09-13 8:20 ` Christoffer Dall
2016-09-13 9:11 ` Marc Zyngier
2016-09-13 9:22 ` Christoffer Dall
2016-09-14 15:20 ` Vladimir Murzin
2016-09-14 15:47 ` Marc Zyngier [this message]
2016-09-15 9:03 ` Christoffer Dall
2016-09-22 10:01 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 02/10] arm64: KVM: Move GIC accessors to arch_gicv3.h Vladimir Murzin
2016-09-12 14:49 ` [PATCH v4 03/10] arm64: KVM: Move vgic-v3 save/restore to virt/kvm/arm/hyp Vladimir Murzin
2016-09-13 8:51 ` Christoffer Dall
2016-09-12 14:49 ` [PATCH v4 04/10] KVM: arm64: vgic-its: Introduce config option to guard ITS specific code Vladimir Murzin
2016-09-13 8:51 ` Christoffer Dall
2016-09-12 14:49 ` [PATCH v4 05/10] KVM: arm: vgic: Fix compiler warnings when built for 32-bit Vladimir Murzin
2016-09-13 8:51 ` Christoffer Dall
2016-09-22 10:01 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 06/10] KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host systems Vladimir Murzin
2016-09-13 8:51 ` Christoffer Dall
2016-09-22 10:00 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 07/10] ARM: Introduce MPIDR_LEVEL_SHIFT macro Vladimir Murzin
2016-09-13 8:38 ` Christoffer Dall
2016-09-13 9:04 ` Vladimir Murzin
2016-09-13 10:12 ` Marc Zyngier
2016-09-13 10:32 ` Vladimir Murzin
2016-09-13 10:44 ` Marc Zyngier
2016-09-14 15:21 ` Vladimir Murzin
2016-09-14 15:50 ` Marc Zyngier
2016-09-22 9:59 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 08/10] ARM: Move system register accessors to asm/cp15.h Vladimir Murzin
2016-09-13 8:52 ` Christoffer Dall
2016-09-22 9:59 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 09/10] ARM: gic-v3: Introduce 32-to-64-bit mappings for GICv3 cpu registers Vladimir Murzin
2016-09-13 8:52 ` Christoffer Dall
2016-09-22 9:57 ` Marc Zyngier
2016-09-12 14:49 ` [PATCH v4 10/10] ARM: KVM: Support vgic-v3 Vladimir Murzin
2016-09-13 8:52 ` Christoffer Dall
2016-09-22 9:58 ` Marc Zyngier
2016-09-15 9:13 ` [PATCH v4 00/10] ARM: KVM: Support for vgic-v3 Christoffer Dall
2016-09-15 10:33 ` Vladimir Murzin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57D97127.3070209@arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox