| Message ID | 20180209143937.28866-42-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 09/02/18 14:39, Andre Przywara wrote: > When we dump guest state on the Xen console, we also print the state of > IRQs that are on a VCPU. > Add the code to dump the state of an IRQ handled by the new VGIC. > > 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 3b475ed1a4..97ffdba5ad 100644 > --- a/xen/arch/arm/vgic/vgic.c > +++ b/xen/arch/arm/vgic/vgic.c > @@ -757,6 +757,19 @@ void vgic_free_virq(struct domain *d, unsigned int virq) > clear_bit(virq, d->arch.vgic.allocated_irqs); > } > > +void gic_dump_vgic_info(struct vcpu *v) > +{ > + struct vgic_cpu *vgic_cpu = &v->arch.vgic_cpu; > + struct vgic_irq *irq; > + > + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) I don't think you can assume that the vCPU is not running somewhere else. So likely you want to take the lock while dumping the info. > + printk(" on CPU: %s %s irq %u: %spending, %sactive, %senabled\n", I am not sure the value of "on CPU". > + irq->hw ? "hardware" : "virtual", > + irq->config == VGIC_CONFIG_LEVEL ? "level" : "edge", > + irq->intid, irq_is_pending(irq) ? "" : "not ", > + irq->active ? "" : "not ", irq->enabled ? "" : "not "); > +} > + > struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, > unsigned int virq) > { > Cheers,
Hi, On 19/02/18 12:26, Julien Grall wrote: > Hi Andre, > > On 09/02/18 14:39, Andre Przywara wrote: >> When we dump guest state on the Xen console, we also print the state of >> IRQs that are on a VCPU. >> Add the code to dump the state of an IRQ handled by the new VGIC. >> >> 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 3b475ed1a4..97ffdba5ad 100644 >> --- a/xen/arch/arm/vgic/vgic.c >> +++ b/xen/arch/arm/vgic/vgic.c >> @@ -757,6 +757,19 @@ void vgic_free_virq(struct domain *d, unsigned >> int virq) >> clear_bit(virq, d->arch.vgic.allocated_irqs); >> } >> +void gic_dump_vgic_info(struct vcpu *v) >> +{ >> + struct vgic_cpu *vgic_cpu = &v->arch.vgic_cpu; >> + struct vgic_irq *irq; >> + >> + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) > > I don't think you can assume that the vCPU is not running somewhere > else. So likely you want to take the lock while dumping the info. Oh, good point. Totally forgot the locking here :-( Same for the IRQs within. Thanks for pointing this out. > >> + printk(" on CPU: %s %s irq %u: %spending, %sactive, >> %senabled\n", > > I am not sure the value of "on CPU". That is meant to be a short phrase for "being on the ap_list", which is an implementation specific term. "Active" or "pending" alone are confusing or misleading. If you have a better term (not too long!), I am happy to take that. Cheers, Andre. > >> + irq->hw ? "hardware" : "virtual", >> + irq->config == VGIC_CONFIG_LEVEL ? "level" : "edge", >> + irq->intid, irq_is_pending(irq) ? "" : "not ", >> + irq->active ? "" : "not ", irq->enabled ? "" : "not "); >> +} >> + >> struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, >> unsigned int virq) >> { >> > > Cheers, >
On 02/26/2018 04:58 PM, Andre Przywara wrote: > Hi, > > On 19/02/18 12:26, Julien Grall wrote: >> Hi Andre, >> >> On 09/02/18 14:39, Andre Przywara wrote: >>> When we dump guest state on the Xen console, we also print the state of >>> IRQs that are on a VCPU. >>> Add the code to dump the state of an IRQ handled by the new VGIC. >>> >>> 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 3b475ed1a4..97ffdba5ad 100644 >>> --- a/xen/arch/arm/vgic/vgic.c >>> +++ b/xen/arch/arm/vgic/vgic.c >>> @@ -757,6 +757,19 @@ void vgic_free_virq(struct domain *d, unsigned >>> int virq) >>> clear_bit(virq, d->arch.vgic.allocated_irqs); >>> } >>> +void gic_dump_vgic_info(struct vcpu *v) >>> +{ >>> + struct vgic_cpu *vgic_cpu = &v->arch.vgic_cpu; >>> + struct vgic_irq *irq; >>> + >>> + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) >> >> I don't think you can assume that the vCPU is not running somewhere >> else. So likely you want to take the lock while dumping the info. > > Oh, good point. Totally forgot the locking here :-( > Same for the IRQs within. > Thanks for pointing this out. > >> >>> + printk(" on CPU: %s %s irq %u: %spending, %sactive, >>> %senabled\n", >> >> I am not sure the value of "on CPU". > > That is meant to be a short phrase for "being on the ap_list", which is > an implementation specific term. "Active" or "pending" alone are > confusing or misleading. If you have a better term (not too long!), I am > happy to take that. How about a print before dumping the list? This would avoid the on CPU on each line and it can be longer :). Cheers,
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c index 3b475ed1a4..97ffdba5ad 100644 --- a/xen/arch/arm/vgic/vgic.c +++ b/xen/arch/arm/vgic/vgic.c @@ -757,6 +757,19 @@ void vgic_free_virq(struct domain *d, unsigned int virq) clear_bit(virq, d->arch.vgic.allocated_irqs); } +void gic_dump_vgic_info(struct vcpu *v) +{ + struct vgic_cpu *vgic_cpu = &v->arch.vgic_cpu; + struct vgic_irq *irq; + + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) + printk(" on CPU: %s %s irq %u: %spending, %sactive, %senabled\n", + irq->hw ? "hardware" : "virtual", + irq->config == VGIC_CONFIG_LEVEL ? "level" : "edge", + irq->intid, irq_is_pending(irq) ? "" : "not ", + irq->active ? "" : "not ", irq->enabled ? "" : "not "); +} + struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, unsigned int virq) {
When we dump guest state on the Xen console, we also print the state of IRQs that are on a VCPU. Add the code to dump the state of an IRQ handled by the new VGIC. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- xen/arch/arm/vgic/vgic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)