Hej Christoffer, On 06/04/16 15:46, Christoffer Dall wrote: > On Wed, Apr 06, 2016 at 03:09:50PM +0100, Andre Przywara wrote: >> >> >> On 06/04/16 14:57, Christoffer Dall wrote: >>> On Tue, Apr 05, 2016 at 10:10:09PM +0200, Christoffer Dall wrote: >>>> On Tue, Apr 05, 2016 at 02:34:42PM +0100, Andre Przywara wrote: >>>>> Hi, >>>>> >>>>> On 29/03/16 14:09, Christoffer Dall wrote: >>>>>> On Fri, Mar 25, 2016 at 02:04:27AM +0000, Andre Przywara wrote: >>>>>>> From: Christoffer Dall <christoffer.dall@xxxxxxxxxx> >>>>>>> >>>>>>> Add a new header file for the new and improved GIC implementation. >>>>>>> The big change is that we now have a struct vgic_irq per IRQ instead >>>>>>> of spreading all the information over various bitmaps. >>>>>>> >>>>>>> We include this new header conditionally from within the old header >>>>>>> file for the time being to avoid touching all the users. >>>>>>> >>>>>>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> >>>>>>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> >>>>>>> --- >>>>>>> include/kvm/arm_vgic.h | 5 ++ >>>>>>> include/kvm/vgic/vgic.h | 198 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> 2 files changed, 203 insertions(+) >>>>>>> create mode 100644 include/kvm/vgic/vgic.h >>>>>>> >>>>>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >>>>>>> index 7656a46..db289a2 100644 >>>>>>> --- a/include/kvm/arm_vgic.h >>>>>>> +++ b/include/kvm/arm_vgic.h >>>>>>> @@ -19,6 +19,10 @@ >>>>>>> #ifndef __ASM_ARM_KVM_VGIC_H >>>>>>> #define __ASM_ARM_KVM_VGIC_H >>>>>>> >>>>>>> +#ifdef CONFIG_KVM_NEW_VGIC >>>>>>> +#include <kvm/vgic/vgic.h> >>>>>>> +#else >>>>>>> + >>>>>>> #include <linux/kernel.h> >>>>>>> #include <linux/kvm.h> >>>>>>> #include <linux/irqreturn.h> >>>>>>> @@ -376,4 +380,5 @@ static inline int vgic_v3_probe(struct device_node *vgic_node, >>>>>>> } >>>>>>> #endif >>>>>>> >>>>>>> +#endif /* old VGIC include */ >>>>>>> #endif >>>>>>> diff --git a/include/kvm/vgic/vgic.h b/include/kvm/vgic/vgic.h >>>>>>> new file mode 100644 >>>>>>> index 0000000..659f8b1 >>>>>>> --- /dev/null >>>>>>> +++ b/include/kvm/vgic/vgic.h >>>>>>> @@ -0,0 +1,198 @@ >>>>>>> +/* >>>>>>> + * Copyright (C) 2015, 2016 ARM Ltd. >>>>>>> + * >>>>>>> + * 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/>. >>>>>>> + */ >>>>>>> +#ifndef __ASM_ARM_KVM_VGIC_VGIC_H >>>>>>> +#define __ASM_ARM_KVM_VGIC_VGIC_H >>>>>>> + >>>>>>> +#include <linux/kernel.h> >>>>>>> +#include <linux/kvm.h> >>>>>>> +#include <linux/irqreturn.h> >>>>>>> +#include <linux/spinlock.h> >>>>>>> +#include <linux/types.h> >>>>>>> +#include <kvm/iodev.h> >>>>>>> + >>>>>>> +#define VGIC_V3_MAX_CPUS 255 >>>>>>> +#define VGIC_V2_MAX_CPUS 8 >>>>>>> +#define VGIC_NR_IRQS_LEGACY 256 >>>>>>> +#define VGIC_NR_SGIS 16 >>>>>>> +#define VGIC_NR_PPIS 16 >>>>>>> +#define VGIC_NR_PRIVATE_IRQS (VGIC_NR_SGIS + VGIC_NR_PPIS) >>>>>>> +#define VGIC_MAX_PRIVATE (VGIC_NR_PRIVATE_IRQS - 1) >>>>>>> +#define VGIC_MAX_SPI 1019 >>>>>>> +#define VGIC_MAX_RESERVED 1023 >>>>>>> +#define VGIC_MIN_LPI 8192 >>>>>>> + >>>>>>> +enum vgic_type { >>>>>>> + VGIC_V2, /* Good ol' GICv2 */ >>>>>>> + VGIC_V3, /* New fancy GICv3 */ >>>>>>> +}; >>>>>>> + >>>>>>> +/* same for all guests, as depending only on the _host's_ GIC model */ >>>>>>> +struct vgic_global { >>>>>>> + /* type of the host GIC */ >>>>>>> + enum vgic_type type; >>>>>>> + >>>>>>> + /* Physical address of vgic virtual cpu interface */ >>>>>>> + phys_addr_t vcpu_base; >>>>>>> + >>>>>>> + /* virtual control interface mapping */ >>>>>>> + void __iomem *vctrl_base; >>>>>>> + >>>>>>> + /* Number of implemented list registers */ >>>>>>> + int nr_lr; >>>>>>> + >>>>>>> + /* Maintenance IRQ number */ >>>>>>> + unsigned int maint_irq; >>>>>>> + >>>>>>> + /* maximum number of VCPUs allowed (GICv2 limits us to 8) */ >>>>>>> + int max_gic_vcpus; >>>>>>> + >>>>>>> + /* Only needed for the legacy KVM_CREATE_IRQCHIP */ >>>>>>> + bool can_emulate_gicv2; >>>>>>> +}; >>>>>>> + >>>>>>> +extern struct vgic_global kvm_vgic_global_state; >>>>>>> + >>>>>>> +#define VGIC_V2_MAX_LRS (1 << 6) >>>>>>> +#define VGIC_V3_MAX_LRS 16 >>>>>>> +#define VGIC_V3_LR_INDEX(lr) (VGIC_V3_MAX_LRS - 1 - lr) >>>>>>> + >>>>>>> +enum vgic_irq_config { >>>>>>> + VGIC_CONFIG_EDGE = 0, >>>>>>> + VGIC_CONFIG_LEVEL >>>>>>> +}; >>>>>>> + >>>>>>> +struct vgic_irq { >>>>>>> + spinlock_t irq_lock; /* Protects the content of the struct */ >>>>>>> + struct list_head ap_list; >>>>>>> + >>>>>>> + struct kvm_vcpu *vcpu; /* SGIs and PPIs: The VCPU >>>>>>> + * SPIs and LPIs: The VCPU whose ap_list >>>>>>> + * on which this is queued. >>>>>>> + */ >>>>>>> + >>>>>>> + struct kvm_vcpu *target_vcpu; /* The VCPU that this interrupt should >>>>>>> + * be send to, as a result of the >>>>>>> + * targets reg (v2) or the >>>>>>> + * affinity reg (v3). >>>>>>> + */ >>>>>>> + >>>>>>> + u32 intid; /* Guest visible INTID */ >>>>>>> + bool pending; >>>>>>> + bool line_level; /* Level only */ >>>>>>> + bool soft_pending; /* Level only */ >>>>>>> + bool active; /* not used for LPIs */ >>>>>>> + bool enabled; >>>>>>> + bool hw; /* Tied to HW IRQ */ >>>>>>> + u32 hwintid; /* HW INTID number */ >>>>>>> + union { >>>>>>> + u8 targets; /* GICv2 target VCPUs mask */ >>>>>>> + u32 mpidr; /* GICv3 target VCPU */ >>>>>>> + }; >>>>>>> + u8 source; /* GICv2 SGIs only */ >>>>>>> + u8 priority; >>>>>>> + enum vgic_irq_config config; /* Level or edge */ >>>>>>> +}; >>>>>>> + >>>>>>> +struct vgic_dist { >>>>>>> + bool in_kernel; >>>>>>> + bool ready; >>>>>>> + >>>>>>> + /* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */ >>>>>>> + u32 vgic_model; >>>>>>> + >>>>>>> + int nr_spis; >>>>>>> + >>>>>>> + /* TODO: Consider moving to global state */ >>>>>>> + /* Virtual control interface mapping */ >>>>>>> + void __iomem *vctrl_base; >>>>>>> + >>>>>>> + /* base addresses in guest physical address space: */ >>>>>>> + gpa_t vgic_dist_base; /* distributor */ >>>>>>> + union { >>>>>>> + /* either a GICv2 CPU interface */ >>>>>>> + gpa_t vgic_cpu_base; >>>>>>> + /* or a number of GICv3 redistributor regions */ >>>>>>> + gpa_t vgic_redist_base; >>>>>>> + }; >>>>>>> + >>>>>>> + /* distributor enabled */ >>>>>>> + u32 enabled; >>>>>>> + >>>>>>> + struct vgic_irq *spis; >>>>>>> +}; >>>>>>> + >>>>>>> +struct vgic_v2_cpu_if { >>>>>>> + u32 vgic_hcr; >>>>>>> + u32 vgic_vmcr; >>>>>>> + u32 vgic_misr; /* Saved only */ >>>>>>> + u64 vgic_eisr; /* Saved only */ >>>>>>> + u64 vgic_elrsr; /* Saved only */ >>>>>>> + u32 vgic_apr; >>>>>>> + u32 vgic_lr[VGIC_V2_MAX_LRS]; >>>>>>> +}; >>>>>>> + >>>>>>> +struct vgic_v3_cpu_if { >>>>>>> +#ifdef CONFIG_KVM_ARM_VGIC_V3 >>>>>>> + u32 vgic_hcr; >>>>>>> + u32 vgic_vmcr; >>>>>>> + u32 vgic_sre; /* Restored only, change ignored */ >>>>>>> + u32 vgic_misr; /* Saved only */ >>>>>>> + u32 vgic_eisr; /* Saved only */ >>>>>>> + u32 vgic_elrsr; /* Saved only */ >>>>>>> + u32 vgic_ap0r[4]; >>>>>>> + u32 vgic_ap1r[4]; >>>>>>> + u64 vgic_lr[VGIC_V3_MAX_LRS]; >>>>>>> +#endif >>>>>>> +}; >>>>>>> + >>>>>>> +struct vgic_cpu { >>>>>>> + /* CPU vif control registers for world switch */ >>>>>>> + union { >>>>>>> + struct vgic_v2_cpu_if vgic_v2; >>>>>>> + struct vgic_v3_cpu_if vgic_v3; >>>>>>> + }; >>>>>>> + >>>>>>> + /* TODO: Move nr_lr to a global state */ >>>>>> >>>>>> what is our current plan and status about this TODO? >>>>> >>>>> The point is that the current HYP code is accessing this field. We do >>>>> have this field in the current "global" struct vgic_params there as >>>>> well, but this struct is (more or less) private to vgic-v2.c, so not >>>>> easily accessible from virt/kvm/arm/hyp/*.c. >>>>> So I suggest we keep this in here for the time being and eventually >>>>> remove it (and rework the save/restore code) once we get rid of the old >>>>> VGIC code. >>>>> >>>> Hmmm, I haven't tried it, but I don't understand why moving it now is >>>> difficult. I'd really like to avoid having a bunch of todo's lingering >>>> around and having duplicated state in the new code. >>>> >>>> I think I noticed a number of places in this patch series which refers >>>> tot he vgic_cpu->nr_lr instead of the global place already... >>>> >>> So I wrote a patch for this and it doesn't really look that bad. I'll >>> send it as part of a little prerequisite series later. >> >> I just fixed it as well - the private new VGIC part is trivial indeed. >> But how did you solve the reference to the vgic_cpu.nr_lr in >> virt/kvm/arm/hyp/vgic-v2-sr.c? With #ifdef .. #else? Or passing in the >> number as in vgic-v3-sr.c? >> > > I sent you a couple of patches as I said I would. > > I just used an extern with the kern_hyp_va() macro trickery applied. > Not the prettiest thing in the world, but hey. It should serve just > fine for moving on with this, and then we can worry about the finesses > as the vgic series progresses. Agreed, thanks for those patches. I think I will just cherry-pick 2/3 and 3/3. I am tempted to post my version of what you did in 1/3, which goes a bit further and avoids some copying between different kind of structs. Cheers, Andre. -- 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
