Re: [Xen-devel] [PATCH v3 09/39] ARM: new VGIC: Add accessor to new struct vgic_irq instance

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 03/21/2018 04:32 PM, Andre Przywara wrote:
> The new VGIC implementation centers around a struct vgic_irq instance
> per virtual IRQ.
> Provide a function to retrieve the right instance for a given IRQ
> number and (in case of private interrupts) the right VCPU.
> This also includes the corresponding put function, which does nothing
> for private interrupts and SPIs, but handles the ref-counting for LPIs.
>
> This is based on Linux commit 64a959d66e47, written by Christoffer Dall.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog v2 ... v3:
> - extend comments to note preliminary nature of vgic_get_lpi()
Thank you for the update.

> Changelog v1 ... v2:
> - reorder header file inclusion
>
>   xen/arch/arm/vgic/vgic.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/vgic/vgic.h |  41 +++++++++++++++
>   2 files changed, 175 insertions(+)
>   create mode 100644 xen/arch/arm/vgic/vgic.c
>   create mode 100644 xen/arch/arm/vgic/vgic.h
>
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> new file mode 100644
> index 0000000000..a818e382b1
> --- /dev/null
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (C) 2015, 2016 ARM Ltd.
> + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
> + *
> + * 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/>.
> + */
> +
> +#include <xen/sched.h>
> +#include <asm/bug.h>
> +#include <asm/new_vgic.h>
> +
> +#include "vgic.h"
> +
> +/*
> + * Iterate over the VM's list of mapped LPIs to find the one with a
> + * matching interrupt ID and return a reference to the IRQ structure.
> + *
> + * TODO: This is more documentation of how it should be done. A list is
> + * not a good data structure for Dom0's LPIs, it merely serves as an
> + * example here how to properly do the locking, allocation and refcounting.
> + * So lpi_list_head should be replaced with something more appropriate.
> + */
> +static struct vgic_irq *vgic_get_lpi(struct domain *d, u32 intid)
It looks like I forgot to mention it on previous version. Please replace 
u32 with uint32_t.
[...]

> +struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
> +                              u32 intid)
Here too.

> +{
> +    /* SGIs and PPIs */
> +    if ( intid <= VGIC_MAX_PRIVATE )
> +        return &vcpu->arch.vgic.private_irqs[intid];
> +
> +    /* SPIs */
> +    if ( intid <= VGIC_MAX_SPI )
> +        return &d->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
> +
> +    /* LPIs */
> +    if ( intid >= VGIC_MIN_LPI )
> +        return vgic_get_lpi(d, intid);
> +
> +    ASSERT_UNREACHABLE();
> +
> +    return NULL;
> +}
> +
[...]

> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
> new file mode 100644
> index 0000000000..a3befd386b
> --- /dev/null
> +++ b/xen/arch/arm/vgic/vgic.h
[...]

> +struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
> +                              u32 intid);
And here too.

With that:

Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel