[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/hvm: add more callback/upcall info to 'I' debug key
On 06.01.2022 16:46, David Vrabel wrote: > Include the type of the callback via and the per-VCPU upcall vector. > > Signed-off-by: David Vrabel <dvrabel@xxxxxxxxxxxx> Welcome back! A couple of stylistic / cosmetic remarks: > --- a/xen/arch/x86/hvm/irq.c > +++ b/xen/arch/x86/hvm/irq.c > @@ -598,7 +598,9 @@ int hvm_local_events_need_delivery(struct vcpu *v) > static void irq_dump(struct domain *d) > { > struct hvm_irq *hvm_irq = hvm_domain_irq(d); > - int i; > + int i; Since you touch this line anyway, would you mind switching to "unsigned int"? > + struct vcpu *v; const? > @@ -630,9 +632,30 @@ static void irq_dump(struct domain *d) > hvm_irq->pci_link_assert_count[1], > hvm_irq->pci_link_assert_count[2], > hvm_irq->pci_link_assert_count[3]); > - printk("Callback via %i:%#"PRIx32",%s asserted\n", > - hvm_irq->callback_via_type, hvm_irq->callback_via.gsi, > - hvm_irq->callback_via_asserted ? "" : " not"); > + for_each_vcpu( d, v ) Depending on whether you consider for_each_vcpu a pseudo-keyword, there's a blank missing here (like for "switch" below), or there are two excess ones. > + { > + if ( v->arch.hvm.evtchn_upcall_vector ) > + printk("%pv: upcall vector: %u\n", > + v, v->arch.hvm.evtchn_upcall_vector); I'm not convinced of (but also not outright opposed to) the resulting redundancy here from using %pv: The domain already got printed once at the top of the function. > + } > + switch( hvm_irq->callback_via_type ) Missing blank ahead of opening parenthesis. > + { > + case HVMIRQ_callback_none: > + printk("Callback via none\n"); > + break; > + case HVMIRQ_callback_gsi: > + printk("Callback via GSI %u\n", hvm_irq->callback_via.gsi); > + break; > + case HVMIRQ_callback_pci_intx: > + printk("Callback via PCI dev %u INTx %u\n", > + hvm_irq->callback_via.pci.dev, > + hvm_irq->callback_via.pci.intx); > + break; > + case HVMIRQ_callback_vector: > + printk("Callback via vector %u\n", hvm_irq->callback_via.vector); > + break; > + } We try to put blank lines between non-fall-through case blocks within a switch(). > + printk(" %s asserted\n", hvm_irq->callback_via_asserted ? "" : " not"); I'm a little puzzled by the blank preceding "not"; how about printk(" %sasserted\n", hvm_irq->callback_via_asserted ? "" : "not "); ? Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |