[v2] KVM: arm/arm64: vgic: Add debugfs vgic-state file

Message ID 20170124132555.19094-1-christoffer.dall@linaro.org
State New
Headers show

Commit Message

Christoffer Dall Jan. 24, 2017, 1:25 p.m.
Add a file to debugfs to read the in-kernel state of the vgic.  We don't
do any locking of the entire VGIC state while traversing all the IRQs,
so if the VM is running the user/developer may not see a quiesced state,
but should take care to pause the VM using facilities in user space for
that purpose.

We also don't support LPIs yet, but they can be added easily if needed.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

---
Changes since v1:
 - Check error code in vgic_debug_stop so that users which did not
   succeed in opening the seq file also doesn't do the cleanup, which
   could lead to a kernel crash with multiple readers of this file
   before.
 - Renamed some functions to be a bit less cute
 - Rebased on the patch that gets rid of the pendin field in struct
   vgic_irq.

 arch/arm/kvm/Makefile          |   1 +
 arch/arm64/kvm/Makefile        |   1 +
 include/kvm/arm_vgic.h         |   5 +
 virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic-init.c  |   4 +
 virt/kvm/arm/vgic/vgic.h       |   3 +
 6 files changed, 298 insertions(+)
 create mode 100644 virt/kvm/arm/vgic/vgic-debug.c

-- 
2.9.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

kbuild test robot Jan. 24, 2017, 6:38 p.m. | #1
Hi Christoffer,

[auto build test ERROR on kvmarm/next]
[also build test ERROR on v4.10-rc5 next-20170124]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Christoffer-Dall/KVM-arm-arm64-vgic-Add-debugfs-vgic-state-file/20170124-234016
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-debug.c: In function 'print_irq_state':
>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-debug.c:195:7: error: 'struct vgic_irq' has no member named 'pending_latch'; did you mean 'pending'?

       irq->pending_latch,
          ^~

vim +195 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-debug.c

   189			      " %2x "
   190			      " %2x "
   191			      "     %2d "
   192			      "\n",
   193				type, irq->intid,
   194				(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,
 > 195				irq->pending_latch,

   196				irq->line_level,
   197				irq->active,
   198				irq->enabled,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Auger Eric Jan. 25, 2017, 9:07 a.m. | #2
Hi Christoffer,

On 24/01/2017 14:25, Christoffer Dall wrote:
> Add a file to debugfs to read the in-kernel state of the vgic.  We don't

> do any locking of the entire VGIC state while traversing all the IRQs,

> so if the VM is running the user/developer may not see a quiesced state,

> but should take care to pause the VM using facilities in user space for

> that purpose.

> 

> We also don't support LPIs yet, but they can be added easily if needed.

> 

> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

> ---

> Changes since v1:

>  - Check error code in vgic_debug_stop so that users which did not

>    succeed in opening the seq file also doesn't do the cleanup, which

>    could lead to a kernel crash with multiple readers of this file

>    before.

>  - Renamed some functions to be a bit less cute

>  - Rebased on the patch that gets rid of the pendin field in struct

>    vgic_irq.

> 

>  arch/arm/kvm/Makefile          |   1 +

>  arch/arm64/kvm/Makefile        |   1 +

>  include/kvm/arm_vgic.h         |   5 +

>  virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++

>  virt/kvm/arm/vgic/vgic-init.c  |   4 +

>  virt/kvm/arm/vgic/vgic.h       |   3 +

>  6 files changed, 298 insertions(+)

>  create mode 100644 virt/kvm/arm/vgic/vgic-debug.c

> 

> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile

> index d571243..12b6281 100644

> --- a/arch/arm/kvm/Makefile

> +++ b/arch/arm/kvm/Makefile

> @@ -33,5 +33,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o

>  obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o

>  obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o

>  obj-y += $(KVM)/arm/vgic/vgic-its.o

> +obj-y += $(KVM)/arm/vgic/vgic-debug.o

>  obj-y += $(KVM)/irqchip.o

>  obj-y += $(KVM)/arm/arch_timer.o

> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile

> index d50a82a..e025bec 100644

> --- a/arch/arm64/kvm/Makefile

> +++ b/arch/arm64/kvm/Makefile

> @@ -31,6 +31,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o

> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o

>  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o

> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h

> index da2ce08..0af1477 100644

> --- a/include/kvm/arm_vgic.h

> +++ b/include/kvm/arm_vgic.h

> @@ -166,6 +166,8 @@ struct vgic_its {

>  	struct list_head	collection_list;

>  };

>  

> +struct vgic_state_iter;

> +

>  struct vgic_dist {

>  	bool			in_kernel;

>  	bool			ready;

> @@ -213,6 +215,9 @@ struct vgic_dist {

>  	spinlock_t		lpi_list_lock;

>  	struct list_head	lpi_list_head;

>  	int			lpi_list_count;

> +

> +	/* used by vgic-debug */

> +	struct vgic_state_iter *iter;

>  };

>  

>  struct vgic_v2_cpu_if {

> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c

> new file mode 100644

> index 0000000..ec466a6

> --- /dev/null

> +++ b/virt/kvm/arm/vgic/vgic-debug.c

> @@ -0,0 +1,284 @@

> +/*

> + * Copyright (C) 2016 Linaro

> + * Author: Christoffer Dall <christoffer.dall@linaro.org>

> + *

> + * This program is free software; you can redistribute it and/or modify

> + * it under the terms of the GNU General Public License version 2 as

> + * published by the Free Software Foundation.

> + *

> + * This program is distributed in the hope that it will be useful,

> + * but WITHOUT ANY WARRANTY; without even the implied warranty of

> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> + * GNU General Public License for more details.

> + *

> + * You should have received a copy of the GNU General Public License

> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.

> + */

> +

> +#include <linux/cpu.h>

> +#include <linux/debugfs.h>

> +#include <linux/interrupt.h>

> +#include <linux/kvm_host.h>

> +#include <linux/seq_file.h>

> +#include <linux/uaccess.h>

not requested
> +#include <kvm/arm_vgic.h>

> +#include <asm/kvm_mmu.h>

> +#include "vgic.h"

> +

> +/*

> + * Structure to control looping through the entire vgic state.  We start at

> + * zero for each field and move upwards.  So, if dist_id is 0 we print the

> + * distributor info.  When dist_id is 1, we have already printed it and move

> + * on.

> + *

> + * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and

> + * so on.

> + */

> +struct vgic_state_iter {

> +	int nr_cpus;

> +	int nr_spis;

> +	int dist_id;

> +	int vcpu_id;

> +	int intid;

> +};

> +

> +static void iter_next(struct vgic_state_iter *iter)

> +{

> +	if (iter->dist_id == 0) {

> +		iter->dist_id++;

> +		return;

> +	}

> +

> +	iter->intid++;

> +	if (iter->intid == VGIC_NR_PRIVATE_IRQS &&

> +	    ++iter->vcpu_id < iter->nr_cpus)

> +		iter->intid = 0;

> +}

> +

> +static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,

> +		      loff_t pos)

> +{

> +	int nr_cpus = atomic_read(&kvm->online_vcpus);

> +

> +	memset(iter, 0, sizeof(*iter));

> +

> +	iter->nr_cpus = nr_cpus;

> +	iter->nr_spis = kvm->arch.vgic.nr_spis;

> +

> +	/* Fast forward to the right position if needed */

> +	while (pos--)

> +		iter_next(iter);

> +}

> +

> +static bool end_of_vgic(struct vgic_state_iter *iter)

> +{

> +	return iter->dist_id > 0 &&

> +		iter->vcpu_id == iter->nr_cpus &&

> +		(iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis;

> +}

> +

> +static void *vgic_debug_start(struct seq_file *s, loff_t *pos)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter;

> +

> +	mutex_lock(&kvm->lock);

> +	iter = kvm->arch.vgic.iter;

> +	if (iter) {

> +		iter = ERR_PTR(-EBUSY);

> +		goto out;

> +	}

> +

> +	iter = kmalloc(sizeof(*iter), GFP_KERNEL);

> +	if (!iter) {

> +		iter = ERR_PTR(-ENOMEM);

> +		goto out;

> +	}

> +

> +	iter_init(kvm, iter, *pos);

> +	kvm->arch.vgic.iter = iter;

> +

> +	if (end_of_vgic(iter))

> +		iter = NULL;

> +out:

> +	mutex_unlock(&kvm->lock);

> +	return iter;

> +}

> +

> +static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter = kvm->arch.vgic.iter;

> +

> +	++*pos;

> +	iter_next(iter);

> +	if (end_of_vgic(iter))

> +		iter = NULL;

> +	return iter;

> +}

> +

> +static void vgic_debug_stop(struct seq_file *s, void *v)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter;

> +

> +	/*

> +	 * If the seq file wasn't properly opened, there's nothing to clearn

> +	 * up.

> +	 */

> +	if (IS_ERR(v))

> +		return;

> +

> +	mutex_lock(&kvm->lock);

> +	iter = kvm->arch.vgic.iter;

> +	kfree(iter);

> +	kvm->arch.vgic.iter = NULL;

> +	mutex_unlock(&kvm->lock);

> +}

> +

> +static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)

> +{

> +	seq_printf(s, "Distributor\n");

> +	seq_printf(s, "===========\n");

> +	seq_printf(s, "vgic_model:\t%s\n",

> +		   (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ?

> +		   "GICv3" : "GICv2");

> +	seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);

> +	seq_printf(s, "enabled:\t%d\n", dist->enabled);

> +	seq_printf(s, "\n");

> +

> +	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");

> +	seq_printf(s, "E=enabled, H=hw, L=config_level\n");

s/Pending/Pending Latched
s/S=soft_pending, //
> +}

> +

> +static void print_header(struct seq_file *s, struct vgic_irq *irq,

> +			 struct kvm_vcpu *vcpu)

> +{

> +	int id = 0;

> +	char *hdr = "SPI ";

> +

> +	if (vcpu) {

> +		hdr = "VCPU";

> +		id = vcpu->vcpu_id;

> +	}

> +

> +	seq_printf(s, "\n");

> +	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);

> +	seq_printf(s, "---------------------------------------------------------------\n");

> +}

> +

> +static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,

> +			    struct kvm_vcpu *vcpu)

> +{

> +	char *type;

> +	if (irq->intid < VGIC_NR_SGIS)

> +		type = "SGI";

> +	else if (irq->intid < VGIC_NR_PRIVATE_IRQS)

> +		type = "PPI";

> +	else

> +		type = "SPI";

> +

> +	if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)

> +		print_header(s, irq, vcpu);

> +

> +	seq_printf(s, "       %s %4d "

> +		      "    %2d "

> +		      "%d%d%d%d%d%d "

> +		      "%8x "

> +		      "%8x "

> +		      " %2x "

> +		      " %2x "

> +		      "     %2d "

> +		      "\n",

> +			type, irq->intid,

> +			(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,

> +			irq->pending_latch,

> +			irq->line_level,

> +			irq->active,

> +			irq->enabled,

> +			irq->hw,

> +			irq->config == VGIC_CONFIG_LEVEL,

> +			irq->hwintid,

nit: why not choosing a %4d format like intid?
> +			irq->mpidr,

is it set for GICv2?
> +			irq->source,

> +			irq->priority,

nit: decimal format for prio?
> +			(irq->vcpu) ? irq->vcpu->vcpu_id : -1);

> +

> +}

> +

> +static int vgic_debug_show(struct seq_file *s, void *v)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;

> +	struct vgic_irq *irq;

> +	struct kvm_vcpu *vcpu = NULL;

> +

> +	if (iter->dist_id == 0) {

> +		print_dist_state(s, &kvm->arch.vgic);

> +		return 0;

> +	}

> +

> +	if (!kvm->arch.vgic.initialized)

> +		return 0;

> +

> +	if (iter->vcpu_id < iter->nr_cpus) {

> +		vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);

> +		irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid];

> +	} else {

> +		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];

> +	}

> +

> +	spin_lock(&irq->irq_lock);

> +	print_irq_state(s, irq, vcpu);

> +	spin_unlock(&irq->irq_lock);

> +

> +	return 0;

> +}

> +

> +static struct seq_operations vgic_debug_seq_ops = {

> +	.start = vgic_debug_start,

> +	.next  = vgic_debug_next,

> +	.stop  = vgic_debug_stop,

> +	.show  = vgic_debug_show

> +};

> +

> +static int debug_open(struct inode *inode, struct file *file)

> +{

> +	int ret;

> +	ret = seq_open(file, &vgic_debug_seq_ops);

> +	if (!ret) {

> +		struct seq_file *seq;

> +		/* seq_open will have modified file->private_data */

> +		seq = file->private_data;

> +		seq->private = inode->i_private;

> +	}

> +

> +	return ret;

> +};

> +

> +static struct file_operations vgic_debug_fops = {

> +	.owner   = THIS_MODULE,

> +	.open    = debug_open,

> +	.read    = seq_read,

> +	.llseek  = seq_lseek,

> +	.release = seq_release

> +};

> +

> +int vgic_debug_init(struct kvm *kvm)

> +{

> +	if (!kvm->debugfs_dentry)

> +		return -ENOENT;

> +

> +	if (!debugfs_create_file("vgic-state", 0444,

> +				 kvm->debugfs_dentry,

> +				 kvm,

> +				 &vgic_debug_fops))

> +		return -ENOMEM;

> +

> +	return 0;

> +}

> +

> +int vgic_debug_destroy(struct kvm *kvm)

> +{

> +	return 0;

> +}

> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c

> index c737ea0..276139a 100644

> --- a/virt/kvm/arm/vgic/vgic-init.c

> +++ b/virt/kvm/arm/vgic/vgic-init.c

> @@ -259,6 +259,8 @@ int vgic_init(struct kvm *kvm)

>  	if (ret)

>  		goto out;

>  

> +	vgic_debug_init(kvm);

> +

>  	dist->initialized = true;

>  out:

>  	return ret;

> @@ -288,6 +290,8 @@ static void __kvm_vgic_destroy(struct kvm *kvm)

>  	struct kvm_vcpu *vcpu;

>  	int i;

>  

> +	vgic_debug_destroy(kvm);

> +

>  	kvm_vgic_dist_destroy(kvm);

>  

>  	kvm_for_each_vcpu(i, vcpu, kvm)

> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h

> index b2cf34b..48da1f6 100644

> --- a/virt/kvm/arm/vgic/vgic.h

> +++ b/virt/kvm/arm/vgic/vgic.h

> @@ -102,4 +102,7 @@ int kvm_register_vgic_device(unsigned long type);

>  int vgic_lazy_init(struct kvm *kvm);

>  int vgic_init(struct kvm *kvm);

>  

> +int vgic_debug_init(struct kvm *kvm);

> +int vgic_debug_destroy(struct kvm *kvm);

> +

>  #endif

> 


Besides

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Tested-by: Eric Auger <eric.auger@redhat.com>


Tested on Cavium ThunderX

Eric


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Andre Przywara Jan. 25, 2017, 10:09 a.m. | #3
Hi Christoffer,

On 24/01/17 13:25, Christoffer Dall wrote:
> Add a file to debugfs to read the in-kernel state of the vgic.  We don't

> do any locking of the entire VGIC state while traversing all the IRQs,

> so if the VM is running the user/developer may not see a quiesced state,

> but should take care to pause the VM using facilities in user space for

> that purpose.

> 

> We also don't support LPIs yet, but they can be added easily if needed.

> 

> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>


Thanks for renaming the MPIDR header field! Also I can confirm that the
multiple readers issue has been fixed.

Tested-by: Andre Przywara <andre.przywara@arm.com>


Cheers,
Andre.

> ---

> Changes since v1:

>  - Check error code in vgic_debug_stop so that users which did not

>    succeed in opening the seq file also doesn't do the cleanup, which

>    could lead to a kernel crash with multiple readers of this file

>    before.

>  - Renamed some functions to be a bit less cute

>  - Rebased on the patch that gets rid of the pendin field in struct

>    vgic_irq.

> 

>  arch/arm/kvm/Makefile          |   1 +

>  arch/arm64/kvm/Makefile        |   1 +

>  include/kvm/arm_vgic.h         |   5 +

>  virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++

>  virt/kvm/arm/vgic/vgic-init.c  |   4 +

>  virt/kvm/arm/vgic/vgic.h       |   3 +

>  6 files changed, 298 insertions(+)

>  create mode 100644 virt/kvm/arm/vgic/vgic-debug.c

> 

> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile

> index d571243..12b6281 100644

> --- a/arch/arm/kvm/Makefile

> +++ b/arch/arm/kvm/Makefile

> @@ -33,5 +33,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o

>  obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o

>  obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o

>  obj-y += $(KVM)/arm/vgic/vgic-its.o

> +obj-y += $(KVM)/arm/vgic/vgic-debug.o

>  obj-y += $(KVM)/irqchip.o

>  obj-y += $(KVM)/arm/arch_timer.o

> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile

> index d50a82a..e025bec 100644

> --- a/arch/arm64/kvm/Makefile

> +++ b/arch/arm64/kvm/Makefile

> @@ -31,6 +31,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o

> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o

>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o

>  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o

> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h

> index da2ce08..0af1477 100644

> --- a/include/kvm/arm_vgic.h

> +++ b/include/kvm/arm_vgic.h

> @@ -166,6 +166,8 @@ struct vgic_its {

>  	struct list_head	collection_list;

>  };

>  

> +struct vgic_state_iter;

> +

>  struct vgic_dist {

>  	bool			in_kernel;

>  	bool			ready;

> @@ -213,6 +215,9 @@ struct vgic_dist {

>  	spinlock_t		lpi_list_lock;

>  	struct list_head	lpi_list_head;

>  	int			lpi_list_count;

> +

> +	/* used by vgic-debug */

> +	struct vgic_state_iter *iter;

>  };

>  

>  struct vgic_v2_cpu_if {

> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c

> new file mode 100644

> index 0000000..ec466a6

> --- /dev/null

> +++ b/virt/kvm/arm/vgic/vgic-debug.c

> @@ -0,0 +1,284 @@

> +/*

> + * Copyright (C) 2016 Linaro

> + * Author: Christoffer Dall <christoffer.dall@linaro.org>

> + *

> + * This program is free software; you can redistribute it and/or modify

> + * it under the terms of the GNU General Public License version 2 as

> + * published by the Free Software Foundation.

> + *

> + * This program is distributed in the hope that it will be useful,

> + * but WITHOUT ANY WARRANTY; without even the implied warranty of

> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> + * GNU General Public License for more details.

> + *

> + * You should have received a copy of the GNU General Public License

> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.

> + */

> +

> +#include <linux/cpu.h>

> +#include <linux/debugfs.h>

> +#include <linux/interrupt.h>

> +#include <linux/kvm_host.h>

> +#include <linux/seq_file.h>

> +#include <linux/uaccess.h>

> +#include <kvm/arm_vgic.h>

> +#include <asm/kvm_mmu.h>

> +#include "vgic.h"

> +

> +/*

> + * Structure to control looping through the entire vgic state.  We start at

> + * zero for each field and move upwards.  So, if dist_id is 0 we print the

> + * distributor info.  When dist_id is 1, we have already printed it and move

> + * on.

> + *

> + * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and

> + * so on.

> + */

> +struct vgic_state_iter {

> +	int nr_cpus;

> +	int nr_spis;

> +	int dist_id;

> +	int vcpu_id;

> +	int intid;

> +};

> +

> +static void iter_next(struct vgic_state_iter *iter)

> +{

> +	if (iter->dist_id == 0) {

> +		iter->dist_id++;

> +		return;

> +	}

> +

> +	iter->intid++;

> +	if (iter->intid == VGIC_NR_PRIVATE_IRQS &&

> +	    ++iter->vcpu_id < iter->nr_cpus)

> +		iter->intid = 0;

> +}

> +

> +static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,

> +		      loff_t pos)

> +{

> +	int nr_cpus = atomic_read(&kvm->online_vcpus);

> +

> +	memset(iter, 0, sizeof(*iter));

> +

> +	iter->nr_cpus = nr_cpus;

> +	iter->nr_spis = kvm->arch.vgic.nr_spis;

> +

> +	/* Fast forward to the right position if needed */

> +	while (pos--)

> +		iter_next(iter);

> +}

> +

> +static bool end_of_vgic(struct vgic_state_iter *iter)

> +{

> +	return iter->dist_id > 0 &&

> +		iter->vcpu_id == iter->nr_cpus &&

> +		(iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis;

> +}

> +

> +static void *vgic_debug_start(struct seq_file *s, loff_t *pos)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter;

> +

> +	mutex_lock(&kvm->lock);

> +	iter = kvm->arch.vgic.iter;

> +	if (iter) {

> +		iter = ERR_PTR(-EBUSY);

> +		goto out;

> +	}

> +

> +	iter = kmalloc(sizeof(*iter), GFP_KERNEL);

> +	if (!iter) {

> +		iter = ERR_PTR(-ENOMEM);

> +		goto out;

> +	}

> +

> +	iter_init(kvm, iter, *pos);

> +	kvm->arch.vgic.iter = iter;

> +

> +	if (end_of_vgic(iter))

> +		iter = NULL;

> +out:

> +	mutex_unlock(&kvm->lock);

> +	return iter;

> +}

> +

> +static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter = kvm->arch.vgic.iter;

> +

> +	++*pos;

> +	iter_next(iter);

> +	if (end_of_vgic(iter))

> +		iter = NULL;

> +	return iter;

> +}

> +

> +static void vgic_debug_stop(struct seq_file *s, void *v)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter;

> +

> +	/*

> +	 * If the seq file wasn't properly opened, there's nothing to clearn

> +	 * up.

> +	 */

> +	if (IS_ERR(v))

> +		return;

> +

> +	mutex_lock(&kvm->lock);

> +	iter = kvm->arch.vgic.iter;

> +	kfree(iter);

> +	kvm->arch.vgic.iter = NULL;

> +	mutex_unlock(&kvm->lock);

> +}

> +

> +static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)

> +{

> +	seq_printf(s, "Distributor\n");

> +	seq_printf(s, "===========\n");

> +	seq_printf(s, "vgic_model:\t%s\n",

> +		   (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ?

> +		   "GICv3" : "GICv2");

> +	seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);

> +	seq_printf(s, "enabled:\t%d\n", dist->enabled);

> +	seq_printf(s, "\n");

> +

> +	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");

> +	seq_printf(s, "E=enabled, H=hw, L=config_level\n");

> +}

> +

> +static void print_header(struct seq_file *s, struct vgic_irq *irq,

> +			 struct kvm_vcpu *vcpu)

> +{

> +	int id = 0;

> +	char *hdr = "SPI ";

> +

> +	if (vcpu) {

> +		hdr = "VCPU";

> +		id = vcpu->vcpu_id;

> +	}

> +

> +	seq_printf(s, "\n");

> +	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);

> +	seq_printf(s, "---------------------------------------------------------------\n");

> +}

> +

> +static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,

> +			    struct kvm_vcpu *vcpu)

> +{

> +	char *type;

> +	if (irq->intid < VGIC_NR_SGIS)

> +		type = "SGI";

> +	else if (irq->intid < VGIC_NR_PRIVATE_IRQS)

> +		type = "PPI";

> +	else

> +		type = "SPI";

> +

> +	if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)

> +		print_header(s, irq, vcpu);

> +

> +	seq_printf(s, "       %s %4d "

> +		      "    %2d "

> +		      "%d%d%d%d%d%d "

> +		      "%8x "

> +		      "%8x "

> +		      " %2x "

> +		      " %2x "

> +		      "     %2d "

> +		      "\n",

> +			type, irq->intid,

> +			(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,

> +			irq->pending_latch,

> +			irq->line_level,

> +			irq->active,

> +			irq->enabled,

> +			irq->hw,

> +			irq->config == VGIC_CONFIG_LEVEL,

> +			irq->hwintid,

> +			irq->mpidr,

> +			irq->source,

> +			irq->priority,

> +			(irq->vcpu) ? irq->vcpu->vcpu_id : -1);

> +

> +}

> +

> +static int vgic_debug_show(struct seq_file *s, void *v)

> +{

> +	struct kvm *kvm = (struct kvm *)s->private;

> +	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;

> +	struct vgic_irq *irq;

> +	struct kvm_vcpu *vcpu = NULL;

> +

> +	if (iter->dist_id == 0) {

> +		print_dist_state(s, &kvm->arch.vgic);

> +		return 0;

> +	}

> +

> +	if (!kvm->arch.vgic.initialized)

> +		return 0;

> +

> +	if (iter->vcpu_id < iter->nr_cpus) {

> +		vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);

> +		irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid];

> +	} else {

> +		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];

> +	}

> +

> +	spin_lock(&irq->irq_lock);

> +	print_irq_state(s, irq, vcpu);

> +	spin_unlock(&irq->irq_lock);

> +

> +	return 0;

> +}

> +

> +static struct seq_operations vgic_debug_seq_ops = {

> +	.start = vgic_debug_start,

> +	.next  = vgic_debug_next,

> +	.stop  = vgic_debug_stop,

> +	.show  = vgic_debug_show

> +};

> +

> +static int debug_open(struct inode *inode, struct file *file)

> +{

> +	int ret;

> +	ret = seq_open(file, &vgic_debug_seq_ops);

> +	if (!ret) {

> +		struct seq_file *seq;

> +		/* seq_open will have modified file->private_data */

> +		seq = file->private_data;

> +		seq->private = inode->i_private;

> +	}

> +

> +	return ret;

> +};

> +

> +static struct file_operations vgic_debug_fops = {

> +	.owner   = THIS_MODULE,

> +	.open    = debug_open,

> +	.read    = seq_read,

> +	.llseek  = seq_lseek,

> +	.release = seq_release

> +};

> +

> +int vgic_debug_init(struct kvm *kvm)

> +{

> +	if (!kvm->debugfs_dentry)

> +		return -ENOENT;

> +

> +	if (!debugfs_create_file("vgic-state", 0444,

> +				 kvm->debugfs_dentry,

> +				 kvm,

> +				 &vgic_debug_fops))

> +		return -ENOMEM;

> +

> +	return 0;

> +}

> +

> +int vgic_debug_destroy(struct kvm *kvm)

> +{

> +	return 0;

> +}

> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c

> index c737ea0..276139a 100644

> --- a/virt/kvm/arm/vgic/vgic-init.c

> +++ b/virt/kvm/arm/vgic/vgic-init.c

> @@ -259,6 +259,8 @@ int vgic_init(struct kvm *kvm)

>  	if (ret)

>  		goto out;

>  

> +	vgic_debug_init(kvm);

> +

>  	dist->initialized = true;

>  out:

>  	return ret;

> @@ -288,6 +290,8 @@ static void __kvm_vgic_destroy(struct kvm *kvm)

>  	struct kvm_vcpu *vcpu;

>  	int i;

>  

> +	vgic_debug_destroy(kvm);

> +

>  	kvm_vgic_dist_destroy(kvm);

>  

>  	kvm_for_each_vcpu(i, vcpu, kvm)

> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h

> index b2cf34b..48da1f6 100644

> --- a/virt/kvm/arm/vgic/vgic.h

> +++ b/virt/kvm/arm/vgic/vgic.h

> @@ -102,4 +102,7 @@ int kvm_register_vgic_device(unsigned long type);

>  int vgic_lazy_init(struct kvm *kvm);

>  int vgic_init(struct kvm *kvm);

>  

> +int vgic_debug_init(struct kvm *kvm);

> +int vgic_debug_destroy(struct kvm *kvm);

> +

>  #endif

> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Christoffer Dall Jan. 25, 2017, 12:38 p.m. | #4
On Wed, Jan 25, 2017 at 10:07:15AM +0100, Auger Eric wrote:
> Hi Christoffer,

> 

> On 24/01/2017 14:25, Christoffer Dall wrote:

> > Add a file to debugfs to read the in-kernel state of the vgic.  We don't

> > do any locking of the entire VGIC state while traversing all the IRQs,

> > so if the VM is running the user/developer may not see a quiesced state,

> > but should take care to pause the VM using facilities in user space for

> > that purpose.

> > 

> > We also don't support LPIs yet, but they can be added easily if needed.

> > 

> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

> > ---

> > Changes since v1:

> >  - Check error code in vgic_debug_stop so that users which did not

> >    succeed in opening the seq file also doesn't do the cleanup, which

> >    could lead to a kernel crash with multiple readers of this file

> >    before.

> >  - Renamed some functions to be a bit less cute

> >  - Rebased on the patch that gets rid of the pendin field in struct

> >    vgic_irq.

> > 

> >  arch/arm/kvm/Makefile          |   1 +

> >  arch/arm64/kvm/Makefile        |   1 +

> >  include/kvm/arm_vgic.h         |   5 +

> >  virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++

> >  virt/kvm/arm/vgic/vgic-init.c  |   4 +

> >  virt/kvm/arm/vgic/vgic.h       |   3 +

> >  6 files changed, 298 insertions(+)

> >  create mode 100644 virt/kvm/arm/vgic/vgic-debug.c

> > 

> > diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile

> > index d571243..12b6281 100644

> > --- a/arch/arm/kvm/Makefile

> > +++ b/arch/arm/kvm/Makefile

> > @@ -33,5 +33,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o

> >  obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o

> >  obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o

> >  obj-y += $(KVM)/arm/vgic/vgic-its.o

> > +obj-y += $(KVM)/arm/vgic/vgic-debug.o

> >  obj-y += $(KVM)/irqchip.o

> >  obj-y += $(KVM)/arm/arch_timer.o

> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile

> > index d50a82a..e025bec 100644

> > --- a/arch/arm64/kvm/Makefile

> > +++ b/arch/arm64/kvm/Makefile

> > @@ -31,6 +31,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o

> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o

> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o

> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o

> > +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o

> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o

> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o

> >  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o

> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h

> > index da2ce08..0af1477 100644

> > --- a/include/kvm/arm_vgic.h

> > +++ b/include/kvm/arm_vgic.h

> > @@ -166,6 +166,8 @@ struct vgic_its {

> >  	struct list_head	collection_list;

> >  };

> >  

> > +struct vgic_state_iter;

> > +

> >  struct vgic_dist {

> >  	bool			in_kernel;

> >  	bool			ready;

> > @@ -213,6 +215,9 @@ struct vgic_dist {

> >  	spinlock_t		lpi_list_lock;

> >  	struct list_head	lpi_list_head;

> >  	int			lpi_list_count;

> > +

> > +	/* used by vgic-debug */

> > +	struct vgic_state_iter *iter;

> >  };

> >  

> >  struct vgic_v2_cpu_if {

> > diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c

> > new file mode 100644

> > index 0000000..ec466a6

> > --- /dev/null

> > +++ b/virt/kvm/arm/vgic/vgic-debug.c

> > @@ -0,0 +1,284 @@

> > +/*

> > + * Copyright (C) 2016 Linaro

> > + * Author: Christoffer Dall <christoffer.dall@linaro.org>

> > + *

> > + * This program is free software; you can redistribute it and/or modify

> > + * it under the terms of the GNU General Public License version 2 as

> > + * published by the Free Software Foundation.

> > + *

> > + * This program is distributed in the hope that it will be useful,

> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of

> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> > + * GNU General Public License for more details.

> > + *

> > + * You should have received a copy of the GNU General Public License

> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.

> > + */

> > +

> > +#include <linux/cpu.h>

> > +#include <linux/debugfs.h>

> > +#include <linux/interrupt.h>

> > +#include <linux/kvm_host.h>

> > +#include <linux/seq_file.h>

> > +#include <linux/uaccess.h>

> not requested

> > +#include <kvm/arm_vgic.h>

> > +#include <asm/kvm_mmu.h>

> > +#include "vgic.h"

> > +

> > +/*

> > + * Structure to control looping through the entire vgic state.  We start at

> > + * zero for each field and move upwards.  So, if dist_id is 0 we print the

> > + * distributor info.  When dist_id is 1, we have already printed it and move

> > + * on.

> > + *

> > + * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and

> > + * so on.

> > + */

> > +struct vgic_state_iter {

> > +	int nr_cpus;

> > +	int nr_spis;

> > +	int dist_id;

> > +	int vcpu_id;

> > +	int intid;

> > +};

> > +

> > +static void iter_next(struct vgic_state_iter *iter)

> > +{

> > +	if (iter->dist_id == 0) {

> > +		iter->dist_id++;

> > +		return;

> > +	}

> > +

> > +	iter->intid++;

> > +	if (iter->intid == VGIC_NR_PRIVATE_IRQS &&

> > +	    ++iter->vcpu_id < iter->nr_cpus)

> > +		iter->intid = 0;

> > +}

> > +

> > +static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,

> > +		      loff_t pos)

> > +{

> > +	int nr_cpus = atomic_read(&kvm->online_vcpus);

> > +

> > +	memset(iter, 0, sizeof(*iter));

> > +

> > +	iter->nr_cpus = nr_cpus;

> > +	iter->nr_spis = kvm->arch.vgic.nr_spis;

> > +

> > +	/* Fast forward to the right position if needed */

> > +	while (pos--)

> > +		iter_next(iter);

> > +}

> > +

> > +static bool end_of_vgic(struct vgic_state_iter *iter)

> > +{

> > +	return iter->dist_id > 0 &&

> > +		iter->vcpu_id == iter->nr_cpus &&

> > +		(iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis;

> > +}

> > +

> > +static void *vgic_debug_start(struct seq_file *s, loff_t *pos)

> > +{

> > +	struct kvm *kvm = (struct kvm *)s->private;

> > +	struct vgic_state_iter *iter;

> > +

> > +	mutex_lock(&kvm->lock);

> > +	iter = kvm->arch.vgic.iter;

> > +	if (iter) {

> > +		iter = ERR_PTR(-EBUSY);

> > +		goto out;

> > +	}

> > +

> > +	iter = kmalloc(sizeof(*iter), GFP_KERNEL);

> > +	if (!iter) {

> > +		iter = ERR_PTR(-ENOMEM);

> > +		goto out;

> > +	}

> > +

> > +	iter_init(kvm, iter, *pos);

> > +	kvm->arch.vgic.iter = iter;

> > +

> > +	if (end_of_vgic(iter))

> > +		iter = NULL;

> > +out:

> > +	mutex_unlock(&kvm->lock);

> > +	return iter;

> > +}

> > +

> > +static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos)

> > +{

> > +	struct kvm *kvm = (struct kvm *)s->private;

> > +	struct vgic_state_iter *iter = kvm->arch.vgic.iter;

> > +

> > +	++*pos;

> > +	iter_next(iter);

> > +	if (end_of_vgic(iter))

> > +		iter = NULL;

> > +	return iter;

> > +}

> > +

> > +static void vgic_debug_stop(struct seq_file *s, void *v)

> > +{

> > +	struct kvm *kvm = (struct kvm *)s->private;

> > +	struct vgic_state_iter *iter;

> > +

> > +	/*

> > +	 * If the seq file wasn't properly opened, there's nothing to clearn

> > +	 * up.

> > +	 */

> > +	if (IS_ERR(v))

> > +		return;

> > +

> > +	mutex_lock(&kvm->lock);

> > +	iter = kvm->arch.vgic.iter;

> > +	kfree(iter);

> > +	kvm->arch.vgic.iter = NULL;

> > +	mutex_unlock(&kvm->lock);

> > +}

> > +

> > +static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)

> > +{

> > +	seq_printf(s, "Distributor\n");

> > +	seq_printf(s, "===========\n");

> > +	seq_printf(s, "vgic_model:\t%s\n",

> > +		   (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ?

> > +		   "GICv3" : "GICv2");

> > +	seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);

> > +	seq_printf(s, "enabled:\t%d\n", dist->enabled);

> > +	seq_printf(s, "\n");

> > +

> > +	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");

> > +	seq_printf(s, "E=enabled, H=hw, L=config_level\n");

> s/Pending/Pending Latched

> s/S=soft_pending, //

> > +}

> > +

> > +static void print_header(struct seq_file *s, struct vgic_irq *irq,

> > +			 struct kvm_vcpu *vcpu)

> > +{

> > +	int id = 0;

> > +	char *hdr = "SPI ";

> > +

> > +	if (vcpu) {

> > +		hdr = "VCPU";

> > +		id = vcpu->vcpu_id;

> > +	}

> > +

> > +	seq_printf(s, "\n");

> > +	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);

> > +	seq_printf(s, "---------------------------------------------------------------\n");

> > +}

> > +

> > +static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,

> > +			    struct kvm_vcpu *vcpu)

> > +{

> > +	char *type;

> > +	if (irq->intid < VGIC_NR_SGIS)

> > +		type = "SGI";

> > +	else if (irq->intid < VGIC_NR_PRIVATE_IRQS)

> > +		type = "PPI";

> > +	else

> > +		type = "SPI";

> > +

> > +	if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)

> > +		print_header(s, irq, vcpu);

> > +

> > +	seq_printf(s, "       %s %4d "

> > +		      "    %2d "

> > +		      "%d%d%d%d%d%d "

> > +		      "%8x "

> > +		      "%8x "

> > +		      " %2x "

> > +		      " %2x "

> > +		      "     %2d "

> > +		      "\n",

> > +			type, irq->intid,

> > +			(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,

> > +			irq->pending_latch,

> > +			irq->line_level,

> > +			irq->active,

> > +			irq->enabled,

> > +			irq->hw,

> > +			irq->config == VGIC_CONFIG_LEVEL,

> > +			irq->hwintid,

> nit: why not choosing a %4d format like intid?

> > +			irq->mpidr,

> is it set for GICv2?

> > +			irq->source,

> > +			irq->priority,

> nit: decimal format for prio?

> > +			(irq->vcpu) ? irq->vcpu->vcpu_id : -1);

> > +

> > +}

> > +

> > +static int vgic_debug_show(struct seq_file *s, void *v)

> > +{

> > +	struct kvm *kvm = (struct kvm *)s->private;

> > +	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;

> > +	struct vgic_irq *irq;

> > +	struct kvm_vcpu *vcpu = NULL;

> > +

> > +	if (iter->dist_id == 0) {

> > +		print_dist_state(s, &kvm->arch.vgic);

> > +		return 0;

> > +	}

> > +

> > +	if (!kvm->arch.vgic.initialized)

> > +		return 0;

> > +

> > +	if (iter->vcpu_id < iter->nr_cpus) {

> > +		vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);

> > +		irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid];

> > +	} else {

> > +		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];

> > +	}

> > +

> > +	spin_lock(&irq->irq_lock);

> > +	print_irq_state(s, irq, vcpu);

> > +	spin_unlock(&irq->irq_lock);

> > +

> > +	return 0;

> > +}

> > +

> > +static struct seq_operations vgic_debug_seq_ops = {

> > +	.start = vgic_debug_start,

> > +	.next  = vgic_debug_next,

> > +	.stop  = vgic_debug_stop,

> > +	.show  = vgic_debug_show

> > +};

> > +

> > +static int debug_open(struct inode *inode, struct file *file)

> > +{

> > +	int ret;

> > +	ret = seq_open(file, &vgic_debug_seq_ops);

> > +	if (!ret) {

> > +		struct seq_file *seq;

> > +		/* seq_open will have modified file->private_data */

> > +		seq = file->private_data;

> > +		seq->private = inode->i_private;

> > +	}

> > +

> > +	return ret;

> > +};

> > +

> > +static struct file_operations vgic_debug_fops = {

> > +	.owner   = THIS_MODULE,

> > +	.open    = debug_open,

> > +	.read    = seq_read,

> > +	.llseek  = seq_lseek,

> > +	.release = seq_release

> > +};

> > +

> > +int vgic_debug_init(struct kvm *kvm)

> > +{

> > +	if (!kvm->debugfs_dentry)

> > +		return -ENOENT;

> > +

> > +	if (!debugfs_create_file("vgic-state", 0444,

> > +				 kvm->debugfs_dentry,

> > +				 kvm,

> > +				 &vgic_debug_fops))

> > +		return -ENOMEM;

> > +

> > +	return 0;

> > +}

> > +

> > +int vgic_debug_destroy(struct kvm *kvm)

> > +{

> > +	return 0;

> > +}

> > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c

> > index c737ea0..276139a 100644

> > --- a/virt/kvm/arm/vgic/vgic-init.c

> > +++ b/virt/kvm/arm/vgic/vgic-init.c

> > @@ -259,6 +259,8 @@ int vgic_init(struct kvm *kvm)

> >  	if (ret)

> >  		goto out;

> >  

> > +	vgic_debug_init(kvm);

> > +

> >  	dist->initialized = true;

> >  out:

> >  	return ret;

> > @@ -288,6 +290,8 @@ static void __kvm_vgic_destroy(struct kvm *kvm)

> >  	struct kvm_vcpu *vcpu;

> >  	int i;

> >  

> > +	vgic_debug_destroy(kvm);

> > +

> >  	kvm_vgic_dist_destroy(kvm);

> >  

> >  	kvm_for_each_vcpu(i, vcpu, kvm)

> > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h

> > index b2cf34b..48da1f6 100644

> > --- a/virt/kvm/arm/vgic/vgic.h

> > +++ b/virt/kvm/arm/vgic/vgic.h

> > @@ -102,4 +102,7 @@ int kvm_register_vgic_device(unsigned long type);

> >  int vgic_lazy_init(struct kvm *kvm);

> >  int vgic_init(struct kvm *kvm);

> >  

> > +int vgic_debug_init(struct kvm *kvm);

> > +int vgic_debug_destroy(struct kvm *kvm);

> > +

> >  #endif

> > 

> 

> Besides

> 

> Reviewed-by: Eric Auger <eric.auger@redhat.com>

> Tested-by: Eric Auger <eric.auger@redhat.com>

> 

> Tested on Cavium ThunderX


I've addressed your comments with the following changes:




Thanks,
-Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kerneldiff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
index ec466a6..19c4566 100644
--- a/virt/kvm/arm/vgic/vgic-debug.c
+++ b/virt/kvm/arm/vgic/vgic-debug.c
@@ -20,7 +20,6 @@
 #include <linux/interrupt.h>
 #include <linux/kvm_host.h>
 #include <linux/seq_file.h>
-#include <linux/uaccess.h>
 #include <kvm/arm_vgic.h>
 #include <asm/kvm_mmu.h>
 #include "vgic.h"
@@ -147,8 +146,8 @@ static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)
 	seq_printf(s, "enabled:\t%d\n", dist->enabled);
 	seq_printf(s, "\n");
 
-	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");
-	seq_printf(s, "E=enabled, H=hw, L=config_level\n");
+	seq_printf(s, "P=pending_latch, L=line_level, A=active\n");
+	seq_printf(s, "E=enabled, H=hw, C=config (level=1, edge=0)\n");
 }
 
 static void print_header(struct seq_file *s, struct vgic_irq *irq,
@@ -184,10 +183,10 @@ static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,
 	seq_printf(s, "       %s %4d "
 		      "    %2d "
 		      "%d%d%d%d%d%d "
+		      "%8d "
 		      "%8x "
-		      "%8x "
-		      " %2x "
 		      " %2x "
+		      "%3d "
 		      "     %2d "
 		      "\n",
 			type, irq->intid,

Christoffer Dall Jan. 25, 2017, 12:49 p.m. | #5
On Wed, Jan 25, 2017 at 01:38:40PM +0100, Christoffer Dall wrote:
> On Wed, Jan 25, 2017 at 10:07:15AM +0100, Auger Eric wrote:

> > Hi Christoffer,

> > 

> > On 24/01/2017 14:25, Christoffer Dall wrote:

> > > Add a file to debugfs to read the in-kernel state of the vgic.  We don't

> > > do any locking of the entire VGIC state while traversing all the IRQs,

> > > so if the VM is running the user/developer may not see a quiesced state,

> > > but should take care to pause the VM using facilities in user space for

> > > that purpose.

> > > 

> > > We also don't support LPIs yet, but they can be added easily if needed.

> > > 

> > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

> > > ---

> > > Changes since v1:

> > >  - Check error code in vgic_debug_stop so that users which did not

> > >    succeed in opening the seq file also doesn't do the cleanup, which

> > >    could lead to a kernel crash with multiple readers of this file

> > >    before.

> > >  - Renamed some functions to be a bit less cute

> > >  - Rebased on the patch that gets rid of the pendin field in struct

> > >    vgic_irq.

> > > 

> > >  arch/arm/kvm/Makefile          |   1 +

> > >  arch/arm64/kvm/Makefile        |   1 +

> > >  include/kvm/arm_vgic.h         |   5 +

> > >  virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++

> > >  virt/kvm/arm/vgic/vgic-init.c  |   4 +

> > >  virt/kvm/arm/vgic/vgic.h       |   3 +

> > >  6 files changed, 298 insertions(+)

> > >  create mode 100644 virt/kvm/arm/vgic/vgic-debug.c

> > > 

> > > diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile

> > > index d571243..12b6281 100644

> > > --- a/arch/arm/kvm/Makefile

> > > +++ b/arch/arm/kvm/Makefile

> > > @@ -33,5 +33,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o

> > >  obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o

> > >  obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o

> > >  obj-y += $(KVM)/arm/vgic/vgic-its.o

> > > +obj-y += $(KVM)/arm/vgic/vgic-debug.o

> > >  obj-y += $(KVM)/irqchip.o

> > >  obj-y += $(KVM)/arm/arch_timer.o

> > > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile

> > > index d50a82a..e025bec 100644

> > > --- a/arch/arm64/kvm/Makefile

> > > +++ b/arch/arm64/kvm/Makefile

> > > @@ -31,6 +31,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o

> > >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o

> > >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o

> > >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o

> > > +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o

> > >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o

> > >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o

> > >  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o

> > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h

> > > index da2ce08..0af1477 100644

> > > --- a/include/kvm/arm_vgic.h

> > > +++ b/include/kvm/arm_vgic.h

> > > @@ -166,6 +166,8 @@ struct vgic_its {

> > >  	struct list_head	collection_list;

> > >  };

> > >  

> > > +struct vgic_state_iter;

> > > +

> > >  struct vgic_dist {

> > >  	bool			in_kernel;

> > >  	bool			ready;

> > > @@ -213,6 +215,9 @@ struct vgic_dist {

> > >  	spinlock_t		lpi_list_lock;

> > >  	struct list_head	lpi_list_head;

> > >  	int			lpi_list_count;

> > > +

> > > +	/* used by vgic-debug */

> > > +	struct vgic_state_iter *iter;

> > >  };

> > >  

> > >  struct vgic_v2_cpu_if {

> > > diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c

> > > new file mode 100644

> > > index 0000000..ec466a6

> > > --- /dev/null

> > > +++ b/virt/kvm/arm/vgic/vgic-debug.c

> > > @@ -0,0 +1,284 @@

> > > +/*

> > > + * Copyright (C) 2016 Linaro

> > > + * Author: Christoffer Dall <christoffer.dall@linaro.org>

> > > + *

> > > + * This program is free software; you can redistribute it and/or modify

> > > + * it under the terms of the GNU General Public License version 2 as

> > > + * published by the Free Software Foundation.

> > > + *

> > > + * This program is distributed in the hope that it will be useful,

> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of

> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> > > + * GNU General Public License for more details.

> > > + *

> > > + * You should have received a copy of the GNU General Public License

> > > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.

> > > + */

> > > +

> > > +#include <linux/cpu.h>

> > > +#include <linux/debugfs.h>

> > > +#include <linux/interrupt.h>

> > > +#include <linux/kvm_host.h>

> > > +#include <linux/seq_file.h>

> > > +#include <linux/uaccess.h>

> > not requested

> > > +#include <kvm/arm_vgic.h>

> > > +#include <asm/kvm_mmu.h>

> > > +#include "vgic.h"

> > > +

> > > +/*

> > > + * Structure to control looping through the entire vgic state.  We start at

> > > + * zero for each field and move upwards.  So, if dist_id is 0 we print the

> > > + * distributor info.  When dist_id is 1, we have already printed it and move

> > > + * on.

> > > + *

> > > + * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and

> > > + * so on.

> > > + */

> > > +struct vgic_state_iter {

> > > +	int nr_cpus;

> > > +	int nr_spis;

> > > +	int dist_id;

> > > +	int vcpu_id;

> > > +	int intid;

> > > +};

> > > +

> > > +static void iter_next(struct vgic_state_iter *iter)

> > > +{

> > > +	if (iter->dist_id == 0) {

> > > +		iter->dist_id++;

> > > +		return;

> > > +	}

> > > +

> > > +	iter->intid++;

> > > +	if (iter->intid == VGIC_NR_PRIVATE_IRQS &&

> > > +	    ++iter->vcpu_id < iter->nr_cpus)

> > > +		iter->intid = 0;

> > > +}

> > > +

> > > +static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,

> > > +		      loff_t pos)

> > > +{

> > > +	int nr_cpus = atomic_read(&kvm->online_vcpus);

> > > +

> > > +	memset(iter, 0, sizeof(*iter));

> > > +

> > > +	iter->nr_cpus = nr_cpus;

> > > +	iter->nr_spis = kvm->arch.vgic.nr_spis;

> > > +

> > > +	/* Fast forward to the right position if needed */

> > > +	while (pos--)

> > > +		iter_next(iter);

> > > +}

> > > +

> > > +static bool end_of_vgic(struct vgic_state_iter *iter)

> > > +{

> > > +	return iter->dist_id > 0 &&

> > > +		iter->vcpu_id == iter->nr_cpus &&

> > > +		(iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis;

> > > +}

> > > +

> > > +static void *vgic_debug_start(struct seq_file *s, loff_t *pos)

> > > +{

> > > +	struct kvm *kvm = (struct kvm *)s->private;

> > > +	struct vgic_state_iter *iter;

> > > +

> > > +	mutex_lock(&kvm->lock);

> > > +	iter = kvm->arch.vgic.iter;

> > > +	if (iter) {

> > > +		iter = ERR_PTR(-EBUSY);

> > > +		goto out;

> > > +	}

> > > +

> > > +	iter = kmalloc(sizeof(*iter), GFP_KERNEL);

> > > +	if (!iter) {

> > > +		iter = ERR_PTR(-ENOMEM);

> > > +		goto out;

> > > +	}

> > > +

> > > +	iter_init(kvm, iter, *pos);

> > > +	kvm->arch.vgic.iter = iter;

> > > +

> > > +	if (end_of_vgic(iter))

> > > +		iter = NULL;

> > > +out:

> > > +	mutex_unlock(&kvm->lock);

> > > +	return iter;

> > > +}

> > > +

> > > +static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos)

> > > +{

> > > +	struct kvm *kvm = (struct kvm *)s->private;

> > > +	struct vgic_state_iter *iter = kvm->arch.vgic.iter;

> > > +

> > > +	++*pos;

> > > +	iter_next(iter);

> > > +	if (end_of_vgic(iter))

> > > +		iter = NULL;

> > > +	return iter;

> > > +}

> > > +

> > > +static void vgic_debug_stop(struct seq_file *s, void *v)

> > > +{

> > > +	struct kvm *kvm = (struct kvm *)s->private;

> > > +	struct vgic_state_iter *iter;

> > > +

> > > +	/*

> > > +	 * If the seq file wasn't properly opened, there's nothing to clearn

> > > +	 * up.

> > > +	 */

> > > +	if (IS_ERR(v))

> > > +		return;

> > > +

> > > +	mutex_lock(&kvm->lock);

> > > +	iter = kvm->arch.vgic.iter;

> > > +	kfree(iter);

> > > +	kvm->arch.vgic.iter = NULL;

> > > +	mutex_unlock(&kvm->lock);

> > > +}

> > > +

> > > +static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)

> > > +{

> > > +	seq_printf(s, "Distributor\n");

> > > +	seq_printf(s, "===========\n");

> > > +	seq_printf(s, "vgic_model:\t%s\n",

> > > +		   (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ?

> > > +		   "GICv3" : "GICv2");

> > > +	seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);

> > > +	seq_printf(s, "enabled:\t%d\n", dist->enabled);

> > > +	seq_printf(s, "\n");

> > > +

> > > +	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");

> > > +	seq_printf(s, "E=enabled, H=hw, L=config_level\n");

> > s/Pending/Pending Latched

> > s/S=soft_pending, //

> > > +}

> > > +

> > > +static void print_header(struct seq_file *s, struct vgic_irq *irq,

> > > +			 struct kvm_vcpu *vcpu)

> > > +{

> > > +	int id = 0;

> > > +	char *hdr = "SPI ";

> > > +

> > > +	if (vcpu) {

> > > +		hdr = "VCPU";

> > > +		id = vcpu->vcpu_id;

> > > +	}

> > > +

> > > +	seq_printf(s, "\n");

> > > +	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);

> > > +	seq_printf(s, "---------------------------------------------------------------\n");

> > > +}

> > > +

> > > +static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,

> > > +			    struct kvm_vcpu *vcpu)

> > > +{

> > > +	char *type;

> > > +	if (irq->intid < VGIC_NR_SGIS)

> > > +		type = "SGI";

> > > +	else if (irq->intid < VGIC_NR_PRIVATE_IRQS)

> > > +		type = "PPI";

> > > +	else

> > > +		type = "SPI";

> > > +

> > > +	if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)

> > > +		print_header(s, irq, vcpu);

> > > +

> > > +	seq_printf(s, "       %s %4d "

> > > +		      "    %2d "

> > > +		      "%d%d%d%d%d%d "

> > > +		      "%8x "

> > > +		      "%8x "

> > > +		      " %2x "

> > > +		      " %2x "

> > > +		      "     %2d "

> > > +		      "\n",

> > > +			type, irq->intid,

> > > +			(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,

> > > +			irq->pending_latch,

> > > +			irq->line_level,

> > > +			irq->active,

> > > +			irq->enabled,

> > > +			irq->hw,

> > > +			irq->config == VGIC_CONFIG_LEVEL,

> > > +			irq->hwintid,

> > nit: why not choosing a %4d format like intid?

> > > +			irq->mpidr,

> > is it set for GICv2?

> > > +			irq->source,

> > > +			irq->priority,

> > nit: decimal format for prio?

> > > +			(irq->vcpu) ? irq->vcpu->vcpu_id : -1);

> > > +

> > > +}

> > > +

> > > +static int vgic_debug_show(struct seq_file *s, void *v)

> > > +{

> > > +	struct kvm *kvm = (struct kvm *)s->private;

> > > +	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;

> > > +	struct vgic_irq *irq;

> > > +	struct kvm_vcpu *vcpu = NULL;

> > > +

> > > +	if (iter->dist_id == 0) {

> > > +		print_dist_state(s, &kvm->arch.vgic);

> > > +		return 0;

> > > +	}

> > > +

> > > +	if (!kvm->arch.vgic.initialized)

> > > +		return 0;

> > > +

> > > +	if (iter->vcpu_id < iter->nr_cpus) {

> > > +		vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);

> > > +		irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid];

> > > +	} else {

> > > +		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];

> > > +	}

> > > +

> > > +	spin_lock(&irq->irq_lock);

> > > +	print_irq_state(s, irq, vcpu);

> > > +	spin_unlock(&irq->irq_lock);

> > > +

> > > +	return 0;

> > > +}

> > > +

> > > +static struct seq_operations vgic_debug_seq_ops = {

> > > +	.start = vgic_debug_start,

> > > +	.next  = vgic_debug_next,

> > > +	.stop  = vgic_debug_stop,

> > > +	.show  = vgic_debug_show

> > > +};

> > > +

> > > +static int debug_open(struct inode *inode, struct file *file)

> > > +{

> > > +	int ret;

> > > +	ret = seq_open(file, &vgic_debug_seq_ops);

> > > +	if (!ret) {

> > > +		struct seq_file *seq;

> > > +		/* seq_open will have modified file->private_data */

> > > +		seq = file->private_data;

> > > +		seq->private = inode->i_private;

> > > +	}

> > > +

> > > +	return ret;

> > > +};

> > > +

> > > +static struct file_operations vgic_debug_fops = {

> > > +	.owner   = THIS_MODULE,

> > > +	.open    = debug_open,

> > > +	.read    = seq_read,

> > > +	.llseek  = seq_lseek,

> > > +	.release = seq_release

> > > +};

> > > +

> > > +int vgic_debug_init(struct kvm *kvm)

> > > +{

> > > +	if (!kvm->debugfs_dentry)

> > > +		return -ENOENT;

> > > +

> > > +	if (!debugfs_create_file("vgic-state", 0444,

> > > +				 kvm->debugfs_dentry,

> > > +				 kvm,

> > > +				 &vgic_debug_fops))

> > > +		return -ENOMEM;

> > > +

> > > +	return 0;

> > > +}

> > > +

> > > +int vgic_debug_destroy(struct kvm *kvm)

> > > +{

> > > +	return 0;

> > > +}

> > > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c

> > > index c737ea0..276139a 100644

> > > --- a/virt/kvm/arm/vgic/vgic-init.c

> > > +++ b/virt/kvm/arm/vgic/vgic-init.c

> > > @@ -259,6 +259,8 @@ int vgic_init(struct kvm *kvm)

> > >  	if (ret)

> > >  		goto out;

> > >  

> > > +	vgic_debug_init(kvm);

> > > +

> > >  	dist->initialized = true;

> > >  out:

> > >  	return ret;

> > > @@ -288,6 +290,8 @@ static void __kvm_vgic_destroy(struct kvm *kvm)

> > >  	struct kvm_vcpu *vcpu;

> > >  	int i;

> > >  

> > > +	vgic_debug_destroy(kvm);

> > > +

> > >  	kvm_vgic_dist_destroy(kvm);

> > >  

> > >  	kvm_for_each_vcpu(i, vcpu, kvm)

> > > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h

> > > index b2cf34b..48da1f6 100644

> > > --- a/virt/kvm/arm/vgic/vgic.h

> > > +++ b/virt/kvm/arm/vgic/vgic.h

> > > @@ -102,4 +102,7 @@ int kvm_register_vgic_device(unsigned long type);

> > >  int vgic_lazy_init(struct kvm *kvm);

> > >  int vgic_init(struct kvm *kvm);

> > >  

> > > +int vgic_debug_init(struct kvm *kvm);

> > > +int vgic_debug_destroy(struct kvm *kvm);

> > > +

> > >  #endif

> > > 

> > 

> > Besides

> > 

> > Reviewed-by: Eric Auger <eric.auger@redhat.com>

> > Tested-by: Eric Auger <eric.auger@redhat.com>

> > 

> > Tested on Cavium ThunderX

> 

> I've addressed your comments with the following changes:

> 

> 

> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c

> index ec466a6..19c4566 100644

> --- a/virt/kvm/arm/vgic/vgic-debug.c

> +++ b/virt/kvm/arm/vgic/vgic-debug.c

> @@ -20,7 +20,6 @@

>  #include <linux/interrupt.h>

>  #include <linux/kvm_host.h>

>  #include <linux/seq_file.h>

> -#include <linux/uaccess.h>

>  #include <kvm/arm_vgic.h>

>  #include <asm/kvm_mmu.h>

>  #include "vgic.h"

> @@ -147,8 +146,8 @@ static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)

>  	seq_printf(s, "enabled:\t%d\n", dist->enabled);

>  	seq_printf(s, "\n");

>  

> -	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");

> -	seq_printf(s, "E=enabled, H=hw, L=config_level\n");

> +	seq_printf(s, "P=pending_latch, L=line_level, A=active\n");

> +	seq_printf(s, "E=enabled, H=hw, C=config (level=1, edge=0)\n");

>  }

>  

>  static void print_header(struct seq_file *s, struct vgic_irq *irq,

> @@ -184,10 +183,10 @@ static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,

>  	seq_printf(s, "       %s %4d "

>  		      "    %2d "

>  		      "%d%d%d%d%d%d "

> +		      "%8d "

>  		      "%8x "

> -		      "%8x "

> -		      " %2x "

>  		      " %2x "

> +		      "%3d "

>  		      "     %2d "

>  		      "\n",

>  			type, irq->intid,

> 

> 


Whoops, and this one:



Thanks,
-Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kerneldiff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
index 19c4566..7072ab7 100644
--- a/virt/kvm/arm/vgic/vgic-debug.c
+++ b/virt/kvm/arm/vgic/vgic-debug.c
@@ -162,7 +162,7 @@ static void print_header(struct seq_file *s, struct vgic_irq *irq,
 	}
 
 	seq_printf(s, "\n");
-	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);
+	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHC     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);
 	seq_printf(s, "---------------------------------------------------------------\n");
 }
 

Patch hide | download patch | download mbox

diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index d571243..12b6281 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -33,5 +33,6 @@  obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
 obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o
 obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o
 obj-y += $(KVM)/arm/vgic/vgic-its.o
+obj-y += $(KVM)/arm/vgic/vgic-debug.o
 obj-y += $(KVM)/irqchip.o
 obj-y += $(KVM)/arm/arch_timer.o
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index d50a82a..e025bec 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -31,6 +31,7 @@  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
 kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index da2ce08..0af1477 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -166,6 +166,8 @@  struct vgic_its {
 	struct list_head	collection_list;
 };
 
+struct vgic_state_iter;
+
 struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
@@ -213,6 +215,9 @@  struct vgic_dist {
 	spinlock_t		lpi_list_lock;
 	struct list_head	lpi_list_head;
 	int			lpi_list_count;
+
+	/* used by vgic-debug */
+	struct vgic_state_iter *iter;
 };
 
 struct vgic_v2_cpu_if {
diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
new file mode 100644
index 0000000..ec466a6
--- /dev/null
+++ b/virt/kvm/arm/vgic/vgic-debug.c
@@ -0,0 +1,284 @@ 
+/*
+ * Copyright (C) 2016 Linaro
+ * Author: Christoffer Dall <christoffer.dall@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/cpu.h>
+#include <linux/debugfs.h>
+#include <linux/interrupt.h>
+#include <linux/kvm_host.h>
+#include <linux/seq_file.h>
+#include <linux/uaccess.h>
+#include <kvm/arm_vgic.h>
+#include <asm/kvm_mmu.h>
+#include "vgic.h"
+
+/*
+ * Structure to control looping through the entire vgic state.  We start at
+ * zero for each field and move upwards.  So, if dist_id is 0 we print the
+ * distributor info.  When dist_id is 1, we have already printed it and move
+ * on.
+ *
+ * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and
+ * so on.
+ */
+struct vgic_state_iter {
+	int nr_cpus;
+	int nr_spis;
+	int dist_id;
+	int vcpu_id;
+	int intid;
+};
+
+static void iter_next(struct vgic_state_iter *iter)
+{
+	if (iter->dist_id == 0) {
+		iter->dist_id++;
+		return;
+	}
+
+	iter->intid++;
+	if (iter->intid == VGIC_NR_PRIVATE_IRQS &&
+	    ++iter->vcpu_id < iter->nr_cpus)
+		iter->intid = 0;
+}
+
+static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,
+		      loff_t pos)
+{
+	int nr_cpus = atomic_read(&kvm->online_vcpus);
+
+	memset(iter, 0, sizeof(*iter));
+
+	iter->nr_cpus = nr_cpus;
+	iter->nr_spis = kvm->arch.vgic.nr_spis;
+
+	/* Fast forward to the right position if needed */
+	while (pos--)
+		iter_next(iter);
+}
+
+static bool end_of_vgic(struct vgic_state_iter *iter)
+{
+	return iter->dist_id > 0 &&
+		iter->vcpu_id == iter->nr_cpus &&
+		(iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis;
+}
+
+static void *vgic_debug_start(struct seq_file *s, loff_t *pos)
+{
+	struct kvm *kvm = (struct kvm *)s->private;
+	struct vgic_state_iter *iter;
+
+	mutex_lock(&kvm->lock);
+	iter = kvm->arch.vgic.iter;
+	if (iter) {
+		iter = ERR_PTR(-EBUSY);
+		goto out;
+	}
+
+	iter = kmalloc(sizeof(*iter), GFP_KERNEL);
+	if (!iter) {
+		iter = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	iter_init(kvm, iter, *pos);
+	kvm->arch.vgic.iter = iter;
+
+	if (end_of_vgic(iter))
+		iter = NULL;
+out:
+	mutex_unlock(&kvm->lock);
+	return iter;
+}
+
+static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	struct kvm *kvm = (struct kvm *)s->private;
+	struct vgic_state_iter *iter = kvm->arch.vgic.iter;
+
+	++*pos;
+	iter_next(iter);
+	if (end_of_vgic(iter))
+		iter = NULL;
+	return iter;
+}
+
+static void vgic_debug_stop(struct seq_file *s, void *v)
+{
+	struct kvm *kvm = (struct kvm *)s->private;
+	struct vgic_state_iter *iter;
+
+	/*
+	 * If the seq file wasn't properly opened, there's nothing to clearn
+	 * up.
+	 */
+	if (IS_ERR(v))
+		return;
+
+	mutex_lock(&kvm->lock);
+	iter = kvm->arch.vgic.iter;
+	kfree(iter);
+	kvm->arch.vgic.iter = NULL;
+	mutex_unlock(&kvm->lock);
+}
+
+static void print_dist_state(struct seq_file *s, struct vgic_dist *dist)
+{
+	seq_printf(s, "Distributor\n");
+	seq_printf(s, "===========\n");
+	seq_printf(s, "vgic_model:\t%s\n",
+		   (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ?
+		   "GICv3" : "GICv2");
+	seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);
+	seq_printf(s, "enabled:\t%d\n", dist->enabled);
+	seq_printf(s, "\n");
+
+	seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n");
+	seq_printf(s, "E=enabled, H=hw, L=config_level\n");
+}
+
+static void print_header(struct seq_file *s, struct vgic_irq *irq,
+			 struct kvm_vcpu *vcpu)
+{
+	int id = 0;
+	char *hdr = "SPI ";
+
+	if (vcpu) {
+		hdr = "VCPU";
+		id = vcpu->vcpu_id;
+	}
+
+	seq_printf(s, "\n");
+	seq_printf(s, "%s%2d TYP   ID TGT_ID PLAEHL     HWID   TARGET SRC PRI VCPU_ID\n", hdr, id);
+	seq_printf(s, "---------------------------------------------------------------\n");
+}
+
+static void print_irq_state(struct seq_file *s, struct vgic_irq *irq,
+			    struct kvm_vcpu *vcpu)
+{
+	char *type;
+	if (irq->intid < VGIC_NR_SGIS)
+		type = "SGI";
+	else if (irq->intid < VGIC_NR_PRIVATE_IRQS)
+		type = "PPI";
+	else
+		type = "SPI";
+
+	if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)
+		print_header(s, irq, vcpu);
+
+	seq_printf(s, "       %s %4d "
+		      "    %2d "
+		      "%d%d%d%d%d%d "
+		      "%8x "
+		      "%8x "
+		      " %2x "
+		      " %2x "
+		      "     %2d "
+		      "\n",
+			type, irq->intid,
+			(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,
+			irq->pending_latch,
+			irq->line_level,
+			irq->active,
+			irq->enabled,
+			irq->hw,
+			irq->config == VGIC_CONFIG_LEVEL,
+			irq->hwintid,
+			irq->mpidr,
+			irq->source,
+			irq->priority,
+			(irq->vcpu) ? irq->vcpu->vcpu_id : -1);
+
+}
+
+static int vgic_debug_show(struct seq_file *s, void *v)
+{
+	struct kvm *kvm = (struct kvm *)s->private;
+	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;
+	struct vgic_irq *irq;
+	struct kvm_vcpu *vcpu = NULL;
+
+	if (iter->dist_id == 0) {
+		print_dist_state(s, &kvm->arch.vgic);
+		return 0;
+	}
+
+	if (!kvm->arch.vgic.initialized)
+		return 0;
+
+	if (iter->vcpu_id < iter->nr_cpus) {
+		vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);
+		irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid];
+	} else {
+		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];
+	}
+
+	spin_lock(&irq->irq_lock);
+	print_irq_state(s, irq, vcpu);
+	spin_unlock(&irq->irq_lock);
+
+	return 0;
+}
+
+static struct seq_operations vgic_debug_seq_ops = {
+	.start = vgic_debug_start,
+	.next  = vgic_debug_next,
+	.stop  = vgic_debug_stop,
+	.show  = vgic_debug_show
+};
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	int ret;
+	ret = seq_open(file, &vgic_debug_seq_ops);
+	if (!ret) {
+		struct seq_file *seq;
+		/* seq_open will have modified file->private_data */
+		seq = file->private_data;
+		seq->private = inode->i_private;
+	}
+
+	return ret;
+};
+
+static struct file_operations vgic_debug_fops = {
+	.owner   = THIS_MODULE,
+	.open    = debug_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = seq_release
+};
+
+int vgic_debug_init(struct kvm *kvm)
+{
+	if (!kvm->debugfs_dentry)
+		return -ENOENT;
+
+	if (!debugfs_create_file("vgic-state", 0444,
+				 kvm->debugfs_dentry,
+				 kvm,
+				 &vgic_debug_fops))
+		return -ENOMEM;
+
+	return 0;
+}
+
+int vgic_debug_destroy(struct kvm *kvm)
+{
+	return 0;
+}
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index c737ea0..276139a 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -259,6 +259,8 @@  int vgic_init(struct kvm *kvm)
 	if (ret)
 		goto out;
 
+	vgic_debug_init(kvm);
+
 	dist->initialized = true;
 out:
 	return ret;
@@ -288,6 +290,8 @@  static void __kvm_vgic_destroy(struct kvm *kvm)
 	struct kvm_vcpu *vcpu;
 	int i;
 
+	vgic_debug_destroy(kvm);
+
 	kvm_vgic_dist_destroy(kvm);
 
 	kvm_for_each_vcpu(i, vcpu, kvm)
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index b2cf34b..48da1f6 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -102,4 +102,7 @@  int kvm_register_vgic_device(unsigned long type);
 int vgic_lazy_init(struct kvm *kvm);
 int vgic_init(struct kvm *kvm);
 
+int vgic_debug_init(struct kvm *kvm);
+int vgic_debug_destroy(struct kvm *kvm);
+
 #endif