On 08/24/2015 07:24 PM, Andre Przywara wrote:

Hi,

On 12/08/15 14:02, Eric Auger wrote:

quoted

On 07/10/2015 04:21 PM, Andre Przywara wrote:

quoted

Currently we initialize all the possible GIC frame addresses in one
function, without looking at the specific GIC model we instantiate
for the guest.
As this gets confusing when adding another VGIC model later, lets
move these initializations into the respective model's init 

nit: tobe more precise the init emulation function (not the
vgic_v2/v3_init_model model's init function). pfouh?! ;-)
functions.

OK, will try to find a wording that is not completely confusing.

quoted

quoted

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 virt/kvm/arm/vgic-v2-emul.c | 3 +++
 virt/kvm/arm/vgic-v3-emul.c | 3 +++
 virt/kvm/arm/vgic.c         | 3 ---
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 1390797..8faa28c 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c

@@ -567,6 +567,9 @@ void vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.init_model = vgic_v2_init_model;
 	dist->vm_ops.map_resources = vgic_v2_map_resources;
 
+	dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
+	dist->vgic_dist_base = VGIC_ADDR_UNDEF;

Looks strange to see the common dist_base here. Why don't you leave it
in common part, kvm_vgic_create; all the more so you left
kvm->arch.vgic.vctrl_base = vgic->vctrl_base in kvm_vgic_create.

The idea behind this is that dist_base refers to similar, but not
identical distributors (v2 vs. v3), so I found it a good idea to
initialize it in here. Also vctrl_base is host facing and not set by
userland, so this doesn't really compare here.

ok

Eric

Cheers,
Andre.

quoted

quoted

+
 	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
 }
 

diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index d2eeb20..1f42348 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c

@@ -885,6 +885,9 @@ void vgic_v3_init_emulation(struct kvm *kvm)
 	dist->vm_ops.destroy_model = vgic_v3_destroy_model;
 	dist->vm_ops.map_resources = vgic_v3_map_resources;
 
+	dist->vgic_dist_base = VGIC_ADDR_UNDEF;
+	dist->vgic_redist_base = VGIC_ADDR_UNDEF;
+
 	kvm->arch.max_vcpus = KVM_MAX_VCPUS;
 }
 

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index cc8f5ed..59f1801 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c

@@ -1830,9 +1830,6 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 	kvm->arch.vgic.in_kernel = true;
 	kvm->arch.vgic.vgic_model = type;
 	kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
-	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
-	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
-	kvm->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
 
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {