| Message ID | 20180209143937.28866-30-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi, On 09/02/18 14:39, Andre Przywara wrote: > Those three registers are v2 emulation specific, so their implementation > lives entirely in vgic-mmio-v2.c. Also they are handled in one function, > as their implementation is pretty simple. > When the guest enables the distributor, we kick all VCPUs to get > potentially pending interrupts serviced. > > This is based on Linux commit 2b0cda878965, written by Marc Zyngier. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > xen/arch/arm/vgic/vgic-mmio-v2.c | 48 +++++++++++++++++++++++++++++++++++++++- > xen/arch/arm/vgic/vgic.c | 15 +++++++++++++ > xen/arch/arm/vgic/vgic.h | 4 ++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c > index ee685a5a07..0926b3243e 100644 > --- a/xen/arch/arm/vgic/vgic-mmio-v2.c > +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c > @@ -20,9 +20,55 @@ > #include "vgic.h" > #include "vgic-mmio.h" > > +static unsigned long vgic_mmio_read_v2_misc(struct vcpu *vcpu, > + paddr_t addr, unsigned int len) Indentation. > +{ > + u32 value; Please use uint32_t. > + > + switch (addr & 0x0c) Coding style switch ( ... ). But I do admit the mask is actually quite confusing. You rely on the caller only call it with the right reg here. It is probably worth to have a comment explaining it. > + { > + case GICD_CTLR: > + value = vcpu->domain->arch.vgic.enabled ? GICD_CTL_ENABLE : 0; > + break; > + case GICD_TYPER: > + value = vcpu->domain->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS; > + value = (value >> 5) - 1; > + value |= (vcpu->domain->max_vcpus - 1) << 5; > + break; > + case GICD_IIDR: > + value = (PRODUCT_ID_KVM << 24) | (IMPLEMENTER_ARM << 0); We probably want to use _ID_XEN here. > + break; > + default: > + return 0; > + } > + > + return value; > +} > + > +static void vgic_mmio_write_v2_misc(struct vcpu *vcpu, > + paddr_t addr, unsigned int len, > + unsigned long val) Indentation. > +{ > + struct vgic_dist *dist = &vcpu->domain->arch.vgic; > + bool was_enabled = dist->enabled; > + > + switch (addr & 0x0c) Ditto for coding style switch and the mask. > + { > + case GICD_CTLR: > + dist->enabled = val & GICD_CTL_ENABLE; > + if ( !was_enabled && dist->enabled ) > + vgic_kick_vcpus(vcpu->domain); On Xen, this code is definitely not atomic if you have multiple callers. > + break; > + case GICD_TYPER: > + case GICD_IIDR: > + /* Nothing to do */ > + return; > + } > +} > + > static const struct vgic_register_region vgic_v2_dist_registers[] = { > REGISTER_DESC_WITH_LENGTH(GICD_CTLR, > - vgic_mmio_read_raz, vgic_mmio_write_wi, 12, > + vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12, > VGIC_ACCESS_32bit), > REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_IGROUPR, > vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1, > diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c > index 9e7fb1edcb..dc5e011fa3 100644 > --- a/xen/arch/arm/vgic/vgic.c > +++ b/xen/arch/arm/vgic/vgic.c > @@ -678,6 +678,21 @@ int gic_events_need_delivery(void) > return vgic_vcpu_pending_irq(current); > } > > +void vgic_kick_vcpus(struct domain *d) > +{ > + struct vcpu *vcpu; > + > + /* > + * We've injected an interrupt, time to find out who deserves > + * a good kick... > + */ > + for_each_vcpu( d, vcpu ) > + { > + if ( vgic_vcpu_pending_irq(vcpu) ) > + vcpu_unblock(vcpu); Unblock will not notify a vCPU running. You want also want to send SGI if the CPU is running. > + } > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h > index 7747d3f3e0..82fe902e26 100644 > --- a/xen/arch/arm/vgic/vgic.h > +++ b/xen/arch/arm/vgic/vgic.h > @@ -17,6 +17,9 @@ > #ifndef __XEN_ARM_VGIC_NEW_H__ > #define __XEN_ARM_VGIC_NEW_H__ > > +#define PRODUCT_ID_KVM 0x4b /* ASCII code K */ > +#define IMPLEMENTER_ARM 0x43b > + > #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS) > > static inline bool irq_is_pending(struct vgic_irq *irq) > @@ -36,6 +39,7 @@ struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu, > void vgic_put_irq(struct domain *d, struct vgic_irq *irq); > bool vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq, > unsigned long flags); > +void vgic_kick_vcpus(struct domain *d); > > static inline void vgic_get_irq_kref(struct vgic_irq *irq) > { > Cheers,
diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c index ee685a5a07..0926b3243e 100644 --- a/xen/arch/arm/vgic/vgic-mmio-v2.c +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c @@ -20,9 +20,55 @@ #include "vgic.h" #include "vgic-mmio.h" +static unsigned long vgic_mmio_read_v2_misc(struct vcpu *vcpu, + paddr_t addr, unsigned int len) +{ + u32 value; + + switch (addr & 0x0c) + { + case GICD_CTLR: + value = vcpu->domain->arch.vgic.enabled ? GICD_CTL_ENABLE : 0; + break; + case GICD_TYPER: + value = vcpu->domain->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS; + value = (value >> 5) - 1; + value |= (vcpu->domain->max_vcpus - 1) << 5; + break; + case GICD_IIDR: + value = (PRODUCT_ID_KVM << 24) | (IMPLEMENTER_ARM << 0); + break; + default: + return 0; + } + + return value; +} + +static void vgic_mmio_write_v2_misc(struct vcpu *vcpu, + paddr_t addr, unsigned int len, + unsigned long val) +{ + struct vgic_dist *dist = &vcpu->domain->arch.vgic; + bool was_enabled = dist->enabled; + + switch (addr & 0x0c) + { + case GICD_CTLR: + dist->enabled = val & GICD_CTL_ENABLE; + if ( !was_enabled && dist->enabled ) + vgic_kick_vcpus(vcpu->domain); + break; + case GICD_TYPER: + case GICD_IIDR: + /* Nothing to do */ + return; + } +} + static const struct vgic_register_region vgic_v2_dist_registers[] = { REGISTER_DESC_WITH_LENGTH(GICD_CTLR, - vgic_mmio_read_raz, vgic_mmio_write_wi, 12, + vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12, VGIC_ACCESS_32bit), REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_IGROUPR, vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1, diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c index 9e7fb1edcb..dc5e011fa3 100644 --- a/xen/arch/arm/vgic/vgic.c +++ b/xen/arch/arm/vgic/vgic.c @@ -678,6 +678,21 @@ int gic_events_need_delivery(void) return vgic_vcpu_pending_irq(current); } +void vgic_kick_vcpus(struct domain *d) +{ + struct vcpu *vcpu; + + /* + * We've injected an interrupt, time to find out who deserves + * a good kick... + */ + for_each_vcpu( d, vcpu ) + { + if ( vgic_vcpu_pending_irq(vcpu) ) + vcpu_unblock(vcpu); + } +} + /* * Local variables: * mode: C diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h index 7747d3f3e0..82fe902e26 100644 --- a/xen/arch/arm/vgic/vgic.h +++ b/xen/arch/arm/vgic/vgic.h @@ -17,6 +17,9 @@ #ifndef __XEN_ARM_VGIC_NEW_H__ #define __XEN_ARM_VGIC_NEW_H__ +#define PRODUCT_ID_KVM 0x4b /* ASCII code K */ +#define IMPLEMENTER_ARM 0x43b + #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS) static inline bool irq_is_pending(struct vgic_irq *irq) @@ -36,6 +39,7 @@ struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu, void vgic_put_irq(struct domain *d, struct vgic_irq *irq); bool vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq, unsigned long flags); +void vgic_kick_vcpus(struct domain *d); static inline void vgic_get_irq_kref(struct vgic_irq *irq) {
Those three registers are v2 emulation specific, so their implementation lives entirely in vgic-mmio-v2.c. Also they are handled in one function, as their implementation is pretty simple. When the guest enables the distributor, we kick all VCPUs to get potentially pending interrupts serviced. This is based on Linux commit 2b0cda878965, written by Marc Zyngier. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- xen/arch/arm/vgic/vgic-mmio-v2.c | 48 +++++++++++++++++++++++++++++++++++++++- xen/arch/arm/vgic/vgic.c | 15 +++++++++++++ xen/arch/arm/vgic/vgic.h | 4 ++++ 3 files changed, 66 insertions(+), 1 deletion(-)