diff mbox

[v7,08/13] KVM: arm/arm64: Remove kvm_vgic_vcpu_early_init

Message ID 1526851372-13009-9-git-send-email-eric.auger@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Auger May 20, 2018, 9:22 p.m. UTC 
kvm_vgic_vcpu_early_init gets called after kvm_vgic_cpu_init which
is confusing. The call path is as follows:
kvm_vm_ioctl_create_vcpu
|_ kvm_arch_cpu_create
   |_ kvm_vcpu_init
      |_ kvm_vgic_vcpu_init
|_ kvm_arch_vcpu_postcreate
   |_ kvm_vgic_vcpu_early_init

Static initialization currently done in kvm_vgic_vcpu_early_init()
can be moved to kvm_vgic_vcpu_init(). So let's move the code and
remove kvm_vgic_vcpu_early_init(). kvm_arch_vcpu_postcreate() does
not nothing.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/kvm/arm_vgic.h        |  1 -
 virt/kvm/arm/arm.c            |  1 -
 virt/kvm/arm/vgic/vgic-init.c | 79 ++++++++++++++++++++-----------------------
 3 files changed, 36 insertions(+), 45 deletions(-)

Comments

kernel test robot May 21, 2018, 8:21 p.m. UTC | #1 
Hi Eric,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kvmarm/next]
[also build test WARNING on v4.17-rc6]
[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/Eric-Auger/KVM-arm-arm64-Allow-multiple-GICv3-redistributor-regions/20180522-004717
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 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c: In function 'kvm_vgic_vcpu_init':
>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:199:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     int i, ret;
            ^~~

vim +/ret +199 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c

   185	
   186	/**
   187	 * kvm_vgic_vcpu_init() - Initialize static VGIC VCPU data
   188	 * structures and register VCPU-specific KVM iodevs
   189	 *
   190	 * @vcpu: pointer to the VCPU being created and initialized
   191	 *
   192	 * Only do initialization, but do not actually enable the
   193	 * VGIC CPU interface
   194	 */
   195	int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
   196	{
   197		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
   198		struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 > 199		int i, ret;
   200	
   201		INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
   202		spin_lock_init(&vgic_cpu->ap_list_lock);
   203	
   204		/*
   205		 * Enable and configure all SGIs to be edge-triggered and
   206		 * configure all PPIs as level-triggered.
   207		 */
   208		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
   209			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
   210	
   211			INIT_LIST_HEAD(&irq->ap_list);
   212			spin_lock_init(&irq->irq_lock);
   213			irq->intid = i;
   214			irq->vcpu = NULL;
   215			irq->target_vcpu = vcpu;
   216			irq->targets = 1U << vcpu->vcpu_id;
   217			kref_init(&irq->refcount);
   218			if (vgic_irq_is_sgi(i)) {
   219				/* SGIs */
   220				irq->enabled = 1;
   221				irq->config = VGIC_CONFIG_EDGE;
   222			} else {
   223				/* PPIs */
   224				irq->config = VGIC_CONFIG_LEVEL;
   225			}
   226		}
   227	
   228		if (!irqchip_in_kernel(vcpu->kvm))
   229			return 0;
   230	
   231		/*
   232		 * If we are creating a VCPU with a GICv3 we must also register the
   233		 * KVM io device for the redistributor that belongs to this VCPU.
   234		 */
   235		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
   236			mutex_lock(&vcpu->kvm->lock);
   237			ret = vgic_register_redist_iodev(vcpu);
   238			mutex_unlock(&vcpu->kvm->lock);
   239		}
   240		return ret;
   241	}
   242	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot May 22, 2018, 12:10 p.m. UTC | #2 
Hi Eric,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kvmarm/next]
[also build test WARNING on v4.17-rc6 next-20180517]
[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/Eric-Auger/KVM-arm-arm64-Allow-multiple-GICv3-redistributor-regions/20180522-004717
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c: In function 'kvm_vgic_vcpu_init':
>> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:199:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     int i, ret;
            ^~~

vim +/ret +199 arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c

   185	
   186	/**
   187	 * kvm_vgic_vcpu_init() - Initialize static VGIC VCPU data
   188	 * structures and register VCPU-specific KVM iodevs
   189	 *
   190	 * @vcpu: pointer to the VCPU being created and initialized
   191	 *
   192	 * Only do initialization, but do not actually enable the
   193	 * VGIC CPU interface
   194	 */
   195	int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
   196	{
   197		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
   198		struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 > 199		int i, ret;
   200	
   201		INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
   202		spin_lock_init(&vgic_cpu->ap_list_lock);
   203	
   204		/*
   205		 * Enable and configure all SGIs to be edge-triggered and
   206		 * configure all PPIs as level-triggered.
   207		 */
   208		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
   209			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
   210	
   211			INIT_LIST_HEAD(&irq->ap_list);
   212			spin_lock_init(&irq->irq_lock);
   213			irq->intid = i;
   214			irq->vcpu = NULL;
   215			irq->target_vcpu = vcpu;
   216			irq->targets = 1U << vcpu->vcpu_id;
   217			kref_init(&irq->refcount);
   218			if (vgic_irq_is_sgi(i)) {
   219				/* SGIs */
   220				irq->enabled = 1;
   221				irq->config = VGIC_CONFIG_EDGE;
   222			} else {
   223				/* PPIs */
   224				irq->config = VGIC_CONFIG_LEVEL;
   225			}
   226		}
   227	
   228		if (!irqchip_in_kernel(vcpu->kvm))
   229			return 0;
   230	
   231		/*
   232		 * If we are creating a VCPU with a GICv3 we must also register the
   233		 * KVM io device for the redistributor that belongs to this VCPU.
   234		 */
   235		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
   236			mutex_lock(&vcpu->kvm->lock);
   237			ret = vgic_register_redist_iodev(vcpu);
   238			mutex_unlock(&vcpu->kvm->lock);
   239		}
   240		return ret;
   241	}
   242	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Eric Auger May 22, 2018, 12:22 p.m. UTC | #3 
Hi,

On 05/22/2018 02:10 PM, kbuild test robot wrote:
> Hi Eric,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on kvmarm/next]
> [also build test WARNING on v4.17-rc6 next-20180517]
> [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/Eric-Auger/KVM-arm-arm64-Allow-multiple-GICv3-redistributor-regions/20180522-004717
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next
> config: arm-axm55xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm 
> 
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
> 
> All warnings (new ones prefixed by >>):
> 
>    arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c: In function 'kvm_vgic_vcpu_init':
>>> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:199:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
>      int i, ret;

Fixed in v8, sent this morning. Sorry for the inconvenience.

Thanks

Eric
>             ^~~
> 
> vim +/ret +199 arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-init.c
> 
>    185	
>    186	/**
>    187	 * kvm_vgic_vcpu_init() - Initialize static VGIC VCPU data
>    188	 * structures and register VCPU-specific KVM iodevs
>    189	 *
>    190	 * @vcpu: pointer to the VCPU being created and initialized
>    191	 *
>    192	 * Only do initialization, but do not actually enable the
>    193	 * VGIC CPU interface
>    194	 */
>    195	int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>    196	{
>    197		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>    198		struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>  > 199		int i, ret;
>    200	
>    201		INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
>    202		spin_lock_init(&vgic_cpu->ap_list_lock);
>    203	
>    204		/*
>    205		 * Enable and configure all SGIs to be edge-triggered and
>    206		 * configure all PPIs as level-triggered.
>    207		 */
>    208		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
>    209			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
>    210	
>    211			INIT_LIST_HEAD(&irq->ap_list);
>    212			spin_lock_init(&irq->irq_lock);
>    213			irq->intid = i;
>    214			irq->vcpu = NULL;
>    215			irq->target_vcpu = vcpu;
>    216			irq->targets = 1U << vcpu->vcpu_id;
>    217			kref_init(&irq->refcount);
>    218			if (vgic_irq_is_sgi(i)) {
>    219				/* SGIs */
>    220				irq->enabled = 1;
>    221				irq->config = VGIC_CONFIG_EDGE;
>    222			} else {
>    223				/* PPIs */
>    224				irq->config = VGIC_CONFIG_LEVEL;
>    225			}
>    226		}
>    227	
>    228		if (!irqchip_in_kernel(vcpu->kvm))
>    229			return 0;
>    230	
>    231		/*
>    232		 * If we are creating a VCPU with a GICv3 we must also register the
>    233		 * KVM io device for the redistributor that belongs to this VCPU.
>    234		 */
>    235		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
>    236			mutex_lock(&vcpu->kvm->lock);
>    237			ret = vgic_register_redist_iodev(vcpu);
>    238			mutex_unlock(&vcpu->kvm->lock);
>    239		}
>    240		return ret;
>    241	}
>    242	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
diff mbox

Patch

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 90e489f..08ccbe3 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -338,7 +338,6 @@  void kvm_vgic_early_init(struct kvm *kvm);
 int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
-void kvm_vgic_vcpu_early_init(struct kvm_vcpu *vcpu);
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 int kvm_vgic_map_resources(struct kvm *kvm);
 int kvm_vgic_hyp_init(void);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a4c1b76..e90fff2 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -290,7 +290,6 @@  struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
-	kvm_vgic_vcpu_early_init(vcpu);
 }
 
 void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 8901b2d..f8750d7 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -44,7 +44,7 @@ 
  *
  * CPU Interface:
  *
- * - kvm_vgic_vcpu_early_init(): initialization of static data that
+ * - kvm_vgic_vcpu_init(): initialization of static data that
  *   doesn't depend on any sizing information or emulation type. No
  *   allocation is allowed there.
  */
@@ -67,46 +67,6 @@  void kvm_vgic_early_init(struct kvm *kvm)
 	spin_lock_init(&dist->lpi_list_lock);
 }
 
-/**
- * kvm_vgic_vcpu_early_init() - Initialize static VGIC VCPU data structures
- * @vcpu: The VCPU whose VGIC data structures whould be initialized
- *
- * Only do initialization, but do not actually enable the VGIC CPU interface
- * yet.
- */
-void kvm_vgic_vcpu_early_init(struct kvm_vcpu *vcpu)
-{
-	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
-	int i;
-
-	INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
-	spin_lock_init(&vgic_cpu->ap_list_lock);
-
-	/*
-	 * Enable and configure all SGIs to be edge-triggered and
-	 * configure all PPIs as level-triggered.
-	 */
-	for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
-		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
-
-		INIT_LIST_HEAD(&irq->ap_list);
-		spin_lock_init(&irq->irq_lock);
-		irq->intid = i;
-		irq->vcpu = NULL;
-		irq->target_vcpu = vcpu;
-		irq->targets = 1U << vcpu->vcpu_id;
-		kref_init(&irq->refcount);
-		if (vgic_irq_is_sgi(i)) {
-			/* SGIs */
-			irq->enabled = 1;
-			irq->config = VGIC_CONFIG_EDGE;
-		} else {
-			/* PPIs */
-			irq->config = VGIC_CONFIG_LEVEL;
-		}
-	}
-}
-
 /* CREATION */
 
 /**
@@ -224,13 +184,46 @@  static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 }
 
 /**
- * kvm_vgic_vcpu_init() - Register VCPU-specific KVM iodevs
+ * kvm_vgic_vcpu_init() - Initialize static VGIC VCPU data
+ * structures and register VCPU-specific KVM iodevs
+ *
  * @vcpu: pointer to the VCPU being created and initialized
+ *
+ * Only do initialization, but do not actually enable the
+ * VGIC CPU interface
  */
 int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 {
-	int ret = 0;
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	int i, ret;
+
+	INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
+	spin_lock_init(&vgic_cpu->ap_list_lock);
+
+	/*
+	 * Enable and configure all SGIs to be edge-triggered and
+	 * configure all PPIs as level-triggered.
+	 */
+	for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+
+		INIT_LIST_HEAD(&irq->ap_list);
+		spin_lock_init(&irq->irq_lock);
+		irq->intid = i;
+		irq->vcpu = NULL;
+		irq->target_vcpu = vcpu;
+		irq->targets = 1U << vcpu->vcpu_id;
+		kref_init(&irq->refcount);
+		if (vgic_irq_is_sgi(i)) {
+			/* SGIs */
+			irq->enabled = 1;
+			irq->config = VGIC_CONFIG_EDGE;
+		} else {
+			/* PPIs */
+			irq->config = VGIC_CONFIG_LEVEL;
+		}
+	}
 
 	if (!irqchip_in_kernel(vcpu->kvm))
 		return 0;