[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


  • To: David Vrabel <dvrabel@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 6 Jan 2022 17:26:37 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9Swk76muHQKM4C14x8IWxeU9P9ERecA7pBQN5GbjnAI=; b=JISniLaOoAskb4UZ8jSpfUdVgtmYKKlxUFQxH8brSGpr8HlDBtKa93HA3vlT+4PMm5fh6NpIHTqrIyq/STqWR9hMPIRspL4A7Dy799Y2IvivkGChqCwBDalZ8/0fPML4CZslGkGe1yzkQgAaWZ3qoIwELqKgyKox2I6j1EXD6x6psb6YCiDnzm8CPDwkwzNdvLDEU7bsIOzGjFri50j/wgoavUwP0Y38zFfZ9OerOpeiMeb8YKpmZ74KirhVz1JHF6NJFqNZv2nYWQ9MyTjKMxpUfcZAW/9z8nGoKGa/8RoF/mMMcRnnKirkPkTrbqCS2uHWJ3ZLl6d5cWF7ifcu9Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EeKQD8K4jVU7/Q4KM9qT7CKrtzZCz7DHQbhfVMXh2XnX1GFa7MmE4bbj9N8J+szJrcviIWe383uHYr17QcPgIeKugMnJ+eFDTZ2xMpfUrnUK4GqTfbUXa4Z3ylLANRtTYX45XKDfp3P0ej627rRtolotgUvGQLODWOHbJCvn02bpH3jFYx/P24KL/zUxjb4+AWzjTamgWiptPszV94m1Ng8NTByofUQgeruUAHLM2qo8bVcUtWLhtplHYP4Cru/jtm8y/fTKOMxOjKY0M67+z5hN2hIz0ZTnE8f3GuUudabbEwjGpPxaNe+leJ4A9UEYc8FeGDT4N+EYwbcnvYMXOQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: David Vrabel <dvrabel@xxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 06 Jan 2022 16:27:03 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.