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@xxxxxxx>
> ---
> 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.
--
Jazz is not dead. It just smells funny...
--
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
