| Message ID | 1464962572-3925-10-git-send-email-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/06/16 15:02, Andre Przywara wrote: > LPIs are dynamically created (mapped) at guest runtime and their > actual numbers can be quite high, but is mostly assigned using a very > sparse allocation scheme. So arrays are not an ideal data structure > to hold the information. We use our equivalent of the "Interrupt > Translation Table Entry" (ITTE) to hold the vgic_irq struct for a > virtual LPI embedded in in the ITTE. > Connect the VGIC core code via an accessor function to help it get the > struct vgic_irq for a certain LPI. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > virt/kvm/arm/vgic/vgic-its.c | 34 ++++++++++++++++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.c | 2 +- > virt/kvm/arm/vgic/vgic.h | 6 ++++++ > 3 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index 3ec12ef..4f248ef 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -68,11 +68,29 @@ struct its_collection { > struct its_itte { > struct list_head itte_list; > > + struct vgic_irq irq; > struct its_collection *collection; > u32 lpi; > u32 event_id; > }; > > +/* To be used as an iterator this macro misses the enclosing parentheses */ > +#define for_each_lpi(dev, itte, its) \ > + list_for_each_entry(dev, &(its)->device_list, dev_list) \ > + list_for_each_entry(itte, &(dev)->itt, itte_list) Well, this is not really "for each LPI". This is "for each LPI that can be generated by this ITS". Are you sure that you can always do this on a per-ITS basis? In other words, while this work for a direct translation, it doesn't work for a reverse one. Do we have any such case? > + > +static struct its_itte *find_itte_by_lpi(struct vgic_its *its, int lpi) > +{ > + struct its_device *device; > + struct its_itte *itte; > + > + for_each_lpi(device, itte, its) { > + if (itte->lpi == lpi) > + return itte; > + } > + return NULL; > +} > + > #define BASER_BASE_ADDRESS(x) ((x) & 0xfffffffff000ULL) > > #define ITS_FRAME(addr) ((addr) & ~(SZ_64K - 1)) > @@ -158,6 +176,22 @@ static unsigned long vgic_mmio_read_its_idregs(struct kvm_vcpu *vcpu, > return 0; > } > > +struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid) > +{ > + struct vgic_its *its; > + struct its_itte *itte; > + > + list_for_each_entry(its, &kvm->arch.vits_list, its_list) { > + itte = find_itte_by_lpi(its, intid); > + if (!itte) > + continue; > + > + return &itte->irq; Or rather if (itte) return &itte->irq; This function implements the case I was worried about above. It would be worth mentioning that this *only* works because of 6.1.1 in the architecture spec (an LPI can only be generated by a single EID/DID pair). What doesn't really work here is that you are allowed to program this EID/DID->LPI translation on several ITSs (think of a device moving its doorbell from one ITS to another), which means that you cannot store the vgic_irq in the ITE. Instead, this must be a pointer to IRQ, and the interrupt as part of a separate list. > + } > + > + return NULL; > +} > + > static void its_free_itte(struct its_itte *itte) > { > list_del(&itte->itte_list); > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c > index 69b61ab..6812ff1 100644 > --- a/virt/kvm/arm/vgic/vgic.c > +++ b/virt/kvm/arm/vgic/vgic.c > @@ -58,7 +58,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu, > > /* LPIs are not yet covered */ > if (intid >= VGIC_MIN_LPI) > - return NULL; > + return vgic_its_get_lpi(kvm, intid); > > WARN(1, "Looking up struct vgic_irq for reserved INTID"); > return NULL; > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > index 66578d2..6fecd70 100644 > --- a/virt/kvm/arm/vgic/vgic.h > +++ b/virt/kvm/arm/vgic/vgic.h > @@ -78,6 +78,7 @@ bool vgic_has_its(struct kvm *kvm); > int vits_init(struct kvm *kvm, struct vgic_its *its); > void vits_destroy(struct kvm *kvm, struct vgic_its *its); > int kvm_vgic_register_its_device(void); > +struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid); > #else > static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu) > { > @@ -148,6 +149,11 @@ static int kvm_vgic_register_its_device(void) > { > return -ENODEV; > } > + > +static inline struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid) > +{ > + return NULL; > +} > #endif > > int kvm_register_vgic_device(unsigned long type); > Thanks, M.
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index 3ec12ef..4f248ef 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -68,11 +68,29 @@ struct its_collection { struct its_itte { struct list_head itte_list; + struct vgic_irq irq; struct its_collection *collection; u32 lpi; u32 event_id; }; +/* To be used as an iterator this macro misses the enclosing parentheses */ +#define for_each_lpi(dev, itte, its) \ + list_for_each_entry(dev, &(its)->device_list, dev_list) \ + list_for_each_entry(itte, &(dev)->itt, itte_list) + +static struct its_itte *find_itte_by_lpi(struct vgic_its *its, int lpi) +{ + struct its_device *device; + struct its_itte *itte; + + for_each_lpi(device, itte, its) { + if (itte->lpi == lpi) + return itte; + } + return NULL; +} + #define BASER_BASE_ADDRESS(x) ((x) & 0xfffffffff000ULL) #define ITS_FRAME(addr) ((addr) & ~(SZ_64K - 1)) @@ -158,6 +176,22 @@ static unsigned long vgic_mmio_read_its_idregs(struct kvm_vcpu *vcpu, return 0; } +struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid) +{ + struct vgic_its *its; + struct its_itte *itte; + + list_for_each_entry(its, &kvm->arch.vits_list, its_list) { + itte = find_itte_by_lpi(its, intid); + if (!itte) + continue; + + return &itte->irq; + } + + return NULL; +} + static void its_free_itte(struct its_itte *itte) { list_del(&itte->itte_list); diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c index 69b61ab..6812ff1 100644 --- a/virt/kvm/arm/vgic/vgic.c +++ b/virt/kvm/arm/vgic/vgic.c @@ -58,7 +58,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu, /* LPIs are not yet covered */ if (intid >= VGIC_MIN_LPI) - return NULL; + return vgic_its_get_lpi(kvm, intid); WARN(1, "Looking up struct vgic_irq for reserved INTID"); return NULL; diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index 66578d2..6fecd70 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -78,6 +78,7 @@ bool vgic_has_its(struct kvm *kvm); int vits_init(struct kvm *kvm, struct vgic_its *its); void vits_destroy(struct kvm *kvm, struct vgic_its *its); int kvm_vgic_register_its_device(void); +struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid); #else static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu) { @@ -148,6 +149,11 @@ static int kvm_vgic_register_its_device(void) { return -ENODEV; } + +static inline struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid) +{ + return NULL; +} #endif int kvm_register_vgic_device(unsigned long type);
LPIs are dynamically created (mapped) at guest runtime and their actual numbers can be quite high, but is mostly assigned using a very sparse allocation scheme. So arrays are not an ideal data structure to hold the information. We use our equivalent of the "Interrupt Translation Table Entry" (ITTE) to hold the vgic_irq struct for a virtual LPI embedded in in the ITTE. Connect the VGIC core code via an accessor function to help it get the struct vgic_irq for a certain LPI. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- virt/kvm/arm/vgic/vgic-its.c | 34 ++++++++++++++++++++++++++++++++++ virt/kvm/arm/vgic/vgic.c | 2 +- virt/kvm/arm/vgic/vgic.h | 6 ++++++ 3 files changed, 41 insertions(+), 1 deletion(-)