| Message ID | 20180209143937.28866-44-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi, On 09/02/18 14:39, Andre Przywara wrote: > The Xen core code requires an interrupt controller emulation to implement > arch_move_irqs(), to move the affinity of an hardware mapped virtual IRQ > to another core. In the moment we don't implement this > physical-follow-virtual regime in our new VGIC, so just provide an empty > stub implementation to make the linker happy. physical-follow-virtual is a must feature for the new vGIC. This has shown better interrupt latency. > Similarily vgic_clear_pending_irqs() is required by the ARM code, > although it is suspected that it is actually not necessary. Go with a > stub for now. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > xen/arch/arm/vgic/vgic.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c > index d91028bd43..77fa756329 100644 > --- a/xen/arch/arm/vgic/vgic.c > +++ b/xen/arch/arm/vgic/vgic.c > @@ -770,6 +770,19 @@ void gic_dump_vgic_info(struct vcpu *v) > irq->active ? "" : "not ", irq->enabled ? "" : "not "); > } > > +void vgic_clear_pending_irqs(struct vcpu *v) > +{ > + /* > + * TODO: It is unclear whether we really need this, so we might instead > + * remove it on the caller site. > + */ I remember some issue with the current vGIC when not removing pending interrupts on PSCI CPU ON. But that was a while ago. I will have another try and see if we can drop it. > +} > + > +void arch_move_irqs(struct vcpu *v) > +{ > + /* TODO: implement this (?) */ Here you would need to go through the interrupt and modify the affinity of the physical IRQs routed to that vCPU. > +} > + > struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, > unsigned int virq) > { > Cheers,
Hi, On 19/02/18 12:34, Julien Grall wrote: > Hi, > > On 09/02/18 14:39, Andre Przywara wrote: >> The Xen core code requires an interrupt controller emulation to implement >> arch_move_irqs(), to move the affinity of an hardware mapped virtual IRQ >> to another core. In the moment we don't implement this >> physical-follow-virtual regime in our new VGIC, so just provide an empty >> stub implementation to make the linker happy. > > physical-follow-virtual is a must feature for the new vGIC. This has > shown better interrupt latency. > >> Similarily vgic_clear_pending_irqs() is required by the ARM code, >> although it is suspected that it is actually not necessary. Go with a >> stub for now. >> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> >> --- >> xen/arch/arm/vgic/vgic.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c >> index d91028bd43..77fa756329 100644 >> --- a/xen/arch/arm/vgic/vgic.c >> +++ b/xen/arch/arm/vgic/vgic.c >> @@ -770,6 +770,19 @@ void gic_dump_vgic_info(struct vcpu *v) >> irq->active ? "" : "not ", irq->enabled ? "" : "not "); >> } >> +void vgic_clear_pending_irqs(struct vcpu *v) >> +{ >> + /* >> + * TODO: It is unclear whether we really need this, so we might >> instead >> + * remove it on the caller site. >> + */ > > I remember some issue with the current vGIC when not removing pending > interrupts on PSCI CPU ON. But that was a while ago. I will have another > try and see if we can drop it. So that's what I came up with: CPU_ON can be called from two states: - The initial state of a core before it's being brought up for the first time. - After the OS has called CPU_OFF. The PSCI spec says that before calling CPU_OFF an OS all IRQs must have been migrated away from that core. I take this as no IRQs are allowed, hence we don't have to clear anything on CPU_ON. In both cases the CPU is expected to enter in a defined state, which includes all interrupts masked on the CPU level (SPSR.ELx.[DAIF] = 1). The GIC defaults to ISPENDR being 0. So I take we should not be held responsible for clearing the pending state upon CPU_ON. What is your opinion? >> +} >> + >> +void arch_move_irqs(struct vcpu *v) >> +{ >> + /* TODO: implement this (?) */ > > Here you would need to go through the interrupt and modify the affinity > of the physical IRQs routed to that vCPU. Since this is apparently the next best thing after sliced bread, I implemented this now. Coming to a theatre near you any time soon. > >> +} >> + >> struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, >> unsigned int virq) >> { >> > > Cheers, >
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c index d91028bd43..77fa756329 100644 --- a/xen/arch/arm/vgic/vgic.c +++ b/xen/arch/arm/vgic/vgic.c @@ -770,6 +770,19 @@ void gic_dump_vgic_info(struct vcpu *v) irq->active ? "" : "not ", irq->enabled ? "" : "not "); } +void vgic_clear_pending_irqs(struct vcpu *v) +{ + /* + * TODO: It is unclear whether we really need this, so we might instead + * remove it on the caller site. + */ +} + +void arch_move_irqs(struct vcpu *v) +{ + /* TODO: implement this (?) */ +} + struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, unsigned int virq) {
The Xen core code requires an interrupt controller emulation to implement arch_move_irqs(), to move the affinity of an hardware mapped virtual IRQ to another core. In the moment we don't implement this physical-follow-virtual regime in our new VGIC, so just provide an empty stub implementation to make the linker happy. Similarily vgic_clear_pending_irqs() is required by the ARM code, although it is suspected that it is actually not necessary. Go with a stub for now. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- xen/arch/arm/vgic/vgic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)