| Message ID | 20180305160415.16760-28-andre.przywara@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 05/03/18 16:03, Andre Przywara wrote: > 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 in the ranks. > > We include this new header conditionally from within the old header > file for the time being to avoid touching all the users. > > This is based on Linux commit b18b57787f5e, written by Christoffer Dall. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > Changelog RFC ... v1: > - rename header file to new_vgic.h > - drop unneeded data structures (vgic_its, vgic_v<x>_cpu_if) > - reorder members in vgic_irq to avoid padding > - move flags members into bool bitfields > - drop prototypes > - use unsigned and uint<x>_t data types > - keep arch_vcpu member name as "vgic" > > xen/include/asm-arm/new_vgic.h | 198 +++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/vgic.h | 6 ++ > 2 files changed, 204 insertions(+) > create mode 100644 xen/include/asm-arm/new_vgic.h > > diff --git a/xen/include/asm-arm/new_vgic.h b/xen/include/asm-arm/new_vgic.h > new file mode 100644 > index 0000000000..54be5aa3eb > --- /dev/null > +++ b/xen/include/asm-arm/new_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_NEW_VGIC_H > +#define __ASM_ARM_NEW_VGIC_H > + > +#include <asm/atomic.h> > +#include <asm/mmio.h> > +#include <xen/list.h> > +#include <xen/mm.h> > +#include <xen/spinlock.h> > + > +#define VGIC_V3_MAX_CPUS 255 > +#define VGIC_V2_MAX_CPUS 8 > +#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 > + > +#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) < VGIC_NR_PRIVATE_IRQS) > +#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \ > + (irq) <= VGIC_MAX_SPI) > + > +enum vgic_type { > + VGIC_V2, /* Good ol' GICv2 */ > + VGIC_V3, /* New fancy GICv3 */ > +}; > + > +#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, Again, I don't think it is necessary to set to 0 here as IIRC an enum always start at 0 if not specified. Also, you might want to add a comment that this enum can only contain two values because of the way you store it (see bool config:1). > + VGIC_CONFIG_LEVEL > +}; > + > +struct vgic_irq { > + struct list_head ap_list; > + > + struct vcpu *vcpu; /* > + * SGIs and PPIs: The VCPU > + * SPIs and LPIs: The VCPU whose ap_list > + * this is queued on. > + */ > + > + struct vcpu *target_vcpu; /* > + * The VCPU that this interrupt should > + * be sent to, as a result of the > + * targets reg (v2) or the affinity reg (v3). > + */ > + > + spinlock_t irq_lock; /* Protects the content of the struct */ > + uint32_t intid; /* Guest visible INTID */ > + atomic_t refcount; /* Used for LPIs */ > + uint32_t hwintid; /* HW INTID number */ > + union > + { > + struct { > + uint8_t targets; /* GICv2 target VCPUs mask */ > + uint8_t source; /* GICv2 SGIs only */ > + }; > + uint32_t mpidr; /* GICv3 target VCPU */ > + }; > + uint8_t priority; > + bool line_level:1; /* Level only */ > + bool pending_latch:1; /* > + * The pending latch state used to > + * calculate the pending state for both > + * level and edge triggered IRQs. > + */ > + bool active:1; /* not used for LPIs */ > + bool enabled:1; > + bool hw:1; /* Tied to HW IRQ */ > + bool config:1; /* Level or edge */ > + struct list_head lpi_list; /* Used to link all LPIs together */ > +}; > + > +struct vgic_register_region; Again, do we really need the forward declaration here? > + > +enum iodev_type { > + IODEV_DIST, > + IODEV_REDIST, > +}; > + > +struct vgic_io_device { > + gfn_t base_fn; > + struct vcpu *redist_vcpu; > + const struct vgic_register_region *regions; > + enum iodev_type iodev_type; > + unsigned int nr_regions; > +}; > + > +struct vgic_dist { > + bool ready; > + bool initialized; > + > + /* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */ > + uint32_t version; > + > + /* Do injected MSIs require an additional device ID? */ > + bool msis_require_devid; > + > + unsigned int nr_spis; > + > + /* base addresses in guest physical address space: */ > + paddr_t vgic_dist_base; /* distributor */ > + union > + { > + /* either a GICv2 CPU interface */ > + paddr_t vgic_cpu_base; > + /* or a number of GICv3 redistributor regions */ > + struct > + { > + paddr_t vgic_redist_base; > + paddr_t vgic_redist_free_offset; > + }; > + }; > + > + /* distributor enabled */ > + bool enabled; > + > + struct vgic_irq *spis; > + unsigned long *allocated_irqs; /* bitmap of IRQs allocated */ > + > + struct vgic_io_device dist_iodev; > + > + bool has_its; > + > + /* > + * Contains the attributes and gpa of the LPI configuration table. > + * Since we report GICR_TYPER.CommonLPIAff as 0b00, we can share > + * one address across all redistributors. > + * GICv3 spec: 6.1.2 "LPI Configuration tables" > + */ > + uint64_t propbaser; > + > + /* Protects the lpi_list and the count value below. */ > + spinlock_t lpi_list_lock; > + struct list_head lpi_list_head; > + unsigned int lpi_list_count; > +}; > + > +struct vgic_cpu { > + struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS]; > + > + struct list_head ap_list_head; > + spinlock_t ap_list_lock; /* Protects the ap_list */ > + > + unsigned int used_lrs; > + > + /* > + * List of IRQs that this VCPU should consider because they are either > + * Active or Pending (hence the name; AP list), or because they recently > + * were one of the two and need to be migrated off this list to another > + * VCPU. > + */ > + > + /* > + * Members below are used with GICv3 emulation only and represent > + * parts of the redistributor. > + */ > + struct vgic_io_device rd_iodev; > + struct vgic_io_device sgi_iodev; > + > + /* Contains the attributes and gpa of the LPI pending tables. */ > + uint64_t pendbaser; > + > + bool lpis_enabled; > + > + /* Cache guest priority bits */ > + uint32_t num_pri_bits; > + > + /* Cache guest interrupt ID bits */ > + uint32_t num_id_bits; > +}; > + > +#define vgic_initialized(k) ((k)->arch.vgic.initialized) > +#define vgic_ready(k) ((k)->arch.vgic.ready) > +#define vgic_valid_spi(k, i) (((i) >= VGIC_NR_PRIVATE_IRQS) && \ > + ((i) < (k)->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS)) What does k stands for? Shouldn't it be 'd' for domain? > + > +#endif /* __ASM_ARM_NEW_VGIC_H */ Missing emacs magic. > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > index 84d82e6eb3..b28b8f8df7 100644 > --- a/xen/include/asm-arm/vgic.h > +++ b/xen/include/asm-arm/vgic.h > @@ -18,6 +18,10 @@ > #ifndef __ASM_ARM_VGIC_H__ > #define __ASM_ARM_VGIC_H__ > > +#ifdef CONFIG_NEW_VGIC > +#include <asm/new_vgic.h> > +#else > + > #include <xen/bitops.h> > #include <xen/radix-tree.h> > #include <xen/rbtree.h> > @@ -299,6 +303,8 @@ extern bool vgic_to_sgi(struct vcpu *v, register_t sgir, > const struct sgi_target *target); > extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq); > > +#endif /* !CONFIG_NEW_VGIC */ > + > /*** Common VGIC functions used by Xen arch code ****/ > > /* > Cheers,
Hi, On 06/03/18 17:46, Julien Grall wrote: > Hi Andre, > > On 05/03/18 16:03, Andre Przywara wrote: >> 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 in the ranks. >> >> We include this new header conditionally from within the old header >> file for the time being to avoid touching all the users. >> >> This is based on Linux commit b18b57787f5e, written by Christoffer Dall. >> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> >> --- >> Changelog RFC ... v1: >> - rename header file to new_vgic.h >> - drop unneeded data structures (vgic_its, vgic_v<x>_cpu_if) >> - reorder members in vgic_irq to avoid padding >> - move flags members into bool bitfields >> - drop prototypes >> - use unsigned and uint<x>_t data types >> - keep arch_vcpu member name as "vgic" >> >> xen/include/asm-arm/new_vgic.h | 198 >> +++++++++++++++++++++++++++++++++++++++++ >> xen/include/asm-arm/vgic.h | 6 ++ >> 2 files changed, 204 insertions(+) >> create mode 100644 xen/include/asm-arm/new_vgic.h >> >> diff --git a/xen/include/asm-arm/new_vgic.h >> b/xen/include/asm-arm/new_vgic.h >> new file mode 100644 >> index 0000000000..54be5aa3eb >> --- /dev/null >> +++ b/xen/include/asm-arm/new_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_NEW_VGIC_H >> +#define __ASM_ARM_NEW_VGIC_H >> + >> +#include <asm/atomic.h> >> +#include <asm/mmio.h> >> +#include <xen/list.h> >> +#include <xen/mm.h> >> +#include <xen/spinlock.h> >> + >> +#define VGIC_V3_MAX_CPUS 255 >> +#define VGIC_V2_MAX_CPUS 8 >> +#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 >> + >> +#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) < >> VGIC_NR_PRIVATE_IRQS) >> +#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \ >> + (irq) <= VGIC_MAX_SPI) >> + >> +enum vgic_type { >> + VGIC_V2, /* Good ol' GICv2 */ >> + VGIC_V3, /* New fancy GICv3 */ >> +}; >> + >> +#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, > > Again, I don't think it is necessary to set to 0 here as IIRC an enum > always start at 0 if not specified. But I consider this more readable, especially since this is a bool now. So it's immediately obvious that EDGE is 0. Shall I replace this with: #define VGIC_CONFIG_EDGE false #define VGIC_CONFIG_LEVEL true now? Or using 0 and 1? The enum vgic_irq_config type is now not used anywhere. > Also, you might want to add a comment that this enum can only contain > two values because of the way you store it (see bool config:1). > >> + VGIC_CONFIG_LEVEL >> +}; >> + >> +struct vgic_irq { >> + struct list_head ap_list; >> + >> + struct vcpu *vcpu; /* >> + * SGIs and PPIs: The VCPU >> + * SPIs and LPIs: The VCPU whose ap_list >> + * this is queued on. >> + */ >> + >> + struct vcpu *target_vcpu; /* >> + * The VCPU that this interrupt should >> + * be sent to, as a result of the >> + * targets reg (v2) or the affinity >> reg (v3). >> + */ >> + >> + spinlock_t irq_lock; /* Protects the content of the struct */ >> + uint32_t intid; /* Guest visible INTID */ >> + atomic_t refcount; /* Used for LPIs */ >> + uint32_t hwintid; /* HW INTID number */ >> + union >> + { >> + struct { >> + uint8_t targets; /* GICv2 target VCPUs mask */ >> + uint8_t source; /* GICv2 SGIs only */ >> + }; >> + uint32_t mpidr; /* GICv3 target VCPU */ >> + }; >> + uint8_t priority; >> + bool line_level:1; /* Level only */ >> + bool pending_latch:1; /* >> + * The pending latch state used to >> + * calculate the pending state for both >> + * level and edge triggered IRQs. >> + */ >> + bool active:1; /* not used for LPIs */ >> + bool enabled:1; >> + bool hw:1; /* Tied to HW IRQ */ >> + bool config:1; /* Level or edge */ >> + struct list_head lpi_list; /* Used to link all LPIs together */ >> +}; >> + >> +struct vgic_register_region; > > Again, do we really need the forward declaration here? No, we don't. I think I removed this, but it got reintroduced over a rebase. >> + >> +enum iodev_type { >> + IODEV_DIST, >> + IODEV_REDIST, >> +}; >> + >> +struct vgic_io_device { >> + gfn_t base_fn; >> + struct vcpu *redist_vcpu; >> + const struct vgic_register_region *regions; >> + enum iodev_type iodev_type; >> + unsigned int nr_regions; >> +}; >> + >> +struct vgic_dist { >> + bool ready; >> + bool initialized; >> + >> + /* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */ >> + uint32_t version; >> + >> + /* Do injected MSIs require an additional device ID? */ >> + bool msis_require_devid; >> + >> + unsigned int nr_spis; >> + >> + /* base addresses in guest physical address space: */ >> + paddr_t vgic_dist_base; /* distributor */ >> + union >> + { >> + /* either a GICv2 CPU interface */ >> + paddr_t vgic_cpu_base; >> + /* or a number of GICv3 redistributor regions */ >> + struct >> + { >> + paddr_t vgic_redist_base; >> + paddr_t vgic_redist_free_offset; >> + }; >> + }; >> + >> + /* distributor enabled */ >> + bool enabled; >> + >> + struct vgic_irq *spis; >> + unsigned long *allocated_irqs; /* bitmap of IRQs allocated */ >> + >> + struct vgic_io_device dist_iodev; >> + >> + bool has_its; >> + >> + /* >> + * Contains the attributes and gpa of the LPI configuration table. >> + * Since we report GICR_TYPER.CommonLPIAff as 0b00, we can share >> + * one address across all redistributors. >> + * GICv3 spec: 6.1.2 "LPI Configuration tables" >> + */ >> + uint64_t propbaser; >> + >> + /* Protects the lpi_list and the count value below. */ >> + spinlock_t lpi_list_lock; >> + struct list_head lpi_list_head; >> + unsigned int lpi_list_count; >> +}; >> + >> +struct vgic_cpu { >> + struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS]; >> + >> + struct list_head ap_list_head; >> + spinlock_t ap_list_lock; /* Protects the ap_list */ >> + >> + unsigned int used_lrs; >> + >> + /* >> + * List of IRQs that this VCPU should consider because they are >> either >> + * Active or Pending (hence the name; AP list), or because they >> recently >> + * were one of the two and need to be migrated off this list to >> another >> + * VCPU. >> + */ >> + >> + /* >> + * Members below are used with GICv3 emulation only and represent >> + * parts of the redistributor. >> + */ >> + struct vgic_io_device rd_iodev; >> + struct vgic_io_device sgi_iodev; >> + >> + /* Contains the attributes and gpa of the LPI pending tables. */ >> + uint64_t pendbaser; >> + >> + bool lpis_enabled; >> + >> + /* Cache guest priority bits */ >> + uint32_t num_pri_bits; >> + >> + /* Cache guest interrupt ID bits */ >> + uint32_t num_id_bits; >> +}; >> + >> +#define vgic_initialized(k) ((k)->arch.vgic.initialized) >> +#define vgic_ready(k) ((k)->arch.vgic.ready) >> +#define vgic_valid_spi(k, i) (((i) >= VGIC_NR_PRIVATE_IRQS) && \ >> + ((i) < (k)->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS)) > > What does k stands for? Shouldn't it be 'd' for domain? Indeed, good catch. K is the new D ;-) But as we don't use those macros anyway, I will just remove them. Cheers, Andre. >> + >> +#endif /* __ASM_ARM_NEW_VGIC_H */ > > Missing emacs magic. > >> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h >> index 84d82e6eb3..b28b8f8df7 100644 >> --- a/xen/include/asm-arm/vgic.h >> +++ b/xen/include/asm-arm/vgic.h >> @@ -18,6 +18,10 @@ >> #ifndef __ASM_ARM_VGIC_H__ >> #define __ASM_ARM_VGIC_H__ >> +#ifdef CONFIG_NEW_VGIC >> +#include <asm/new_vgic.h> >> +#else >> + >> #include <xen/bitops.h> >> #include <xen/radix-tree.h> >> #include <xen/rbtree.h> >> @@ -299,6 +303,8 @@ extern bool vgic_to_sgi(struct vcpu *v, register_t >> sgir, >> const struct sgi_target *target); >> extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, >> unsigned int irq); >> +#endif /* !CONFIG_NEW_VGIC */ >> + >> /*** Common VGIC functions used by Xen arch code ****/ >> /* >> > > Cheers, >
Hi Andre, On 03/06/2018 06:01 PM, Andre Przywara wrote: > On 06/03/18 17:46, Julien Grall wrote: >> On 05/03/18 16:03, Andre Przywara wrote: >>> 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 in the ranks. >>> >>> We include this new header conditionally from within the old header >>> file for the time being to avoid touching all the users. >>> >>> This is based on Linux commit b18b57787f5e, written by Christoffer Dall. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> >>> --- >>> Changelog RFC ... v1: >>> - rename header file to new_vgic.h >>> - drop unneeded data structures (vgic_its, vgic_v<x>_cpu_if) >>> - reorder members in vgic_irq to avoid padding >>> - move flags members into bool bitfields >>> - drop prototypes >>> - use unsigned and uint<x>_t data types >>> - keep arch_vcpu member name as "vgic" >>> >>> xen/include/asm-arm/new_vgic.h | 198 >>> +++++++++++++++++++++++++++++++++++++++++ >>> xen/include/asm-arm/vgic.h | 6 ++ >>> 2 files changed, 204 insertions(+) >>> create mode 100644 xen/include/asm-arm/new_vgic.h >>> >>> diff --git a/xen/include/asm-arm/new_vgic.h >>> b/xen/include/asm-arm/new_vgic.h >>> new file mode 100644 >>> index 0000000000..54be5aa3eb >>> --- /dev/null >>> +++ b/xen/include/asm-arm/new_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_NEW_VGIC_H >>> +#define __ASM_ARM_NEW_VGIC_H >>> + >>> +#include <asm/atomic.h> >>> +#include <asm/mmio.h> >>> +#include <xen/list.h> >>> +#include <xen/mm.h> >>> +#include <xen/spinlock.h> >>> + >>> +#define VGIC_V3_MAX_CPUS 255 >>> +#define VGIC_V2_MAX_CPUS 8 >>> +#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 >>> + >>> +#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) < >>> VGIC_NR_PRIVATE_IRQS) >>> +#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \ >>> + (irq) <= VGIC_MAX_SPI) >>> + >>> +enum vgic_type { >>> + VGIC_V2, /* Good ol' GICv2 */ >>> + VGIC_V3, /* New fancy GICv3 */ >>> +}; >>> + >>> +#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, >> >> Again, I don't think it is necessary to set to 0 here as IIRC an enum >> always start at 0 if not specified. > > But I consider this more readable, especially since this is a bool now. > So it's immediately obvious that EDGE is 0. Fair enough, I probably know to much the C spec :). > Shall I replace this with: > #define VGIC_CONFIG_EDGE false > #define VGIC_CONFIG_LEVEL true > now? Or using 0 and 1? > > The enum vgic_irq_config type is now not used anywhere. I don't mind to keep the enum with a comment above. If you switch to define, then I would prefer to use false/true. Cheers,
diff --git a/xen/include/asm-arm/new_vgic.h b/xen/include/asm-arm/new_vgic.h new file mode 100644 index 0000000000..54be5aa3eb --- /dev/null +++ b/xen/include/asm-arm/new_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_NEW_VGIC_H +#define __ASM_ARM_NEW_VGIC_H + +#include <asm/atomic.h> +#include <asm/mmio.h> +#include <xen/list.h> +#include <xen/mm.h> +#include <xen/spinlock.h> + +#define VGIC_V3_MAX_CPUS 255 +#define VGIC_V2_MAX_CPUS 8 +#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 + +#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) < VGIC_NR_PRIVATE_IRQS) +#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \ + (irq) <= VGIC_MAX_SPI) + +enum vgic_type { + VGIC_V2, /* Good ol' GICv2 */ + VGIC_V3, /* New fancy GICv3 */ +}; + +#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 { + struct list_head ap_list; + + struct vcpu *vcpu; /* + * SGIs and PPIs: The VCPU + * SPIs and LPIs: The VCPU whose ap_list + * this is queued on. + */ + + struct vcpu *target_vcpu; /* + * The VCPU that this interrupt should + * be sent to, as a result of the + * targets reg (v2) or the affinity reg (v3). + */ + + spinlock_t irq_lock; /* Protects the content of the struct */ + uint32_t intid; /* Guest visible INTID */ + atomic_t refcount; /* Used for LPIs */ + uint32_t hwintid; /* HW INTID number */ + union + { + struct { + uint8_t targets; /* GICv2 target VCPUs mask */ + uint8_t source; /* GICv2 SGIs only */ + }; + uint32_t mpidr; /* GICv3 target VCPU */ + }; + uint8_t priority; + bool line_level:1; /* Level only */ + bool pending_latch:1; /* + * The pending latch state used to + * calculate the pending state for both + * level and edge triggered IRQs. + */ + bool active:1; /* not used for LPIs */ + bool enabled:1; + bool hw:1; /* Tied to HW IRQ */ + bool config:1; /* Level or edge */ + struct list_head lpi_list; /* Used to link all LPIs together */ +}; + +struct vgic_register_region; + +enum iodev_type { + IODEV_DIST, + IODEV_REDIST, +}; + +struct vgic_io_device { + gfn_t base_fn; + struct vcpu *redist_vcpu; + const struct vgic_register_region *regions; + enum iodev_type iodev_type; + unsigned int nr_regions; +}; + +struct vgic_dist { + bool ready; + bool initialized; + + /* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */ + uint32_t version; + + /* Do injected MSIs require an additional device ID? */ + bool msis_require_devid; + + unsigned int nr_spis; + + /* base addresses in guest physical address space: */ + paddr_t vgic_dist_base; /* distributor */ + union + { + /* either a GICv2 CPU interface */ + paddr_t vgic_cpu_base; + /* or a number of GICv3 redistributor regions */ + struct + { + paddr_t vgic_redist_base; + paddr_t vgic_redist_free_offset; + }; + }; + + /* distributor enabled */ + bool enabled; + + struct vgic_irq *spis; + unsigned long *allocated_irqs; /* bitmap of IRQs allocated */ + + struct vgic_io_device dist_iodev; + + bool has_its; + + /* + * Contains the attributes and gpa of the LPI configuration table. + * Since we report GICR_TYPER.CommonLPIAff as 0b00, we can share + * one address across all redistributors. + * GICv3 spec: 6.1.2 "LPI Configuration tables" + */ + uint64_t propbaser; + + /* Protects the lpi_list and the count value below. */ + spinlock_t lpi_list_lock; + struct list_head lpi_list_head; + unsigned int lpi_list_count; +}; + +struct vgic_cpu { + struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS]; + + struct list_head ap_list_head; + spinlock_t ap_list_lock; /* Protects the ap_list */ + + unsigned int used_lrs; + + /* + * List of IRQs that this VCPU should consider because they are either + * Active or Pending (hence the name; AP list), or because they recently + * were one of the two and need to be migrated off this list to another + * VCPU. + */ + + /* + * Members below are used with GICv3 emulation only and represent + * parts of the redistributor. + */ + struct vgic_io_device rd_iodev; + struct vgic_io_device sgi_iodev; + + /* Contains the attributes and gpa of the LPI pending tables. */ + uint64_t pendbaser; + + bool lpis_enabled; + + /* Cache guest priority bits */ + uint32_t num_pri_bits; + + /* Cache guest interrupt ID bits */ + uint32_t num_id_bits; +}; + +#define vgic_initialized(k) ((k)->arch.vgic.initialized) +#define vgic_ready(k) ((k)->arch.vgic.ready) +#define vgic_valid_spi(k, i) (((i) >= VGIC_NR_PRIVATE_IRQS) && \ + ((i) < (k)->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS)) + +#endif /* __ASM_ARM_NEW_VGIC_H */ diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index 84d82e6eb3..b28b8f8df7 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -18,6 +18,10 @@ #ifndef __ASM_ARM_VGIC_H__ #define __ASM_ARM_VGIC_H__ +#ifdef CONFIG_NEW_VGIC +#include <asm/new_vgic.h> +#else + #include <xen/bitops.h> #include <xen/radix-tree.h> #include <xen/rbtree.h> @@ -299,6 +303,8 @@ extern bool vgic_to_sgi(struct vcpu *v, register_t sgir, const struct sgi_target *target); extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq); +#endif /* !CONFIG_NEW_VGIC */ + /*** Common VGIC functions used by Xen arch code ****/ /*
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 in the ranks. We include this new header conditionally from within the old header file for the time being to avoid touching all the users. This is based on Linux commit b18b57787f5e, written by Christoffer Dall. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- Changelog RFC ... v1: - rename header file to new_vgic.h - drop unneeded data structures (vgic_its, vgic_v<x>_cpu_if) - reorder members in vgic_irq to avoid padding - move flags members into bool bitfields - drop prototypes - use unsigned and uint<x>_t data types - keep arch_vcpu member name as "vgic" xen/include/asm-arm/new_vgic.h | 198 +++++++++++++++++++++++++++++++++++++++++ xen/include/asm-arm/vgic.h | 6 ++ 2 files changed, 204 insertions(+) create mode 100644 xen/include/asm-arm/new_vgic.h