Re: [PATCH v2 01/15] arm/arm64: KVM: rework MPIDR assignment and add accessors

2014-10-31 Thread Andre Przywara
Hi Christoffer,

On 15/10/14 17:25, Christoffer Dall wrote:
> On Thu, Aug 21, 2014 at 02:06:42PM +0100, Andre Przywara wrote:
>> The virtual MPIDR registers (containing topology information) for the
>> guest are currently mapped linearily to the vcpu_id. Improve this
>> mapping for arm64 by using three levels to not artificially limit the
>> number of vCPUs. Also add an accessor to later allow easier access to
>> a vCPU with a given MPIDR.
>> Use this new accessor in the PSCI emulation.
>>
>> Signed-off-by: Andre Przywara 
>> ---
>>  arch/arm/include/asm/kvm_emulate.h   |2 +-
>>  arch/arm/include/asm/kvm_host.h  |2 ++
>>  arch/arm/kvm/arm.c   |   15 +++
>>  arch/arm/kvm/psci.c  |   15 ---
>>  arch/arm64/include/asm/kvm_emulate.h |3 ++-
>>  arch/arm64/include/asm/kvm_host.h|2 ++
>>  arch/arm64/kvm/sys_regs.c|   11 +--
>>  7 files changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_emulate.h 
>> b/arch/arm/include/asm/kvm_emulate.h
>> index 69b7469..18e45f7 100644
>> --- a/arch/arm/include/asm/kvm_emulate.h
>> +++ b/arch/arm/include/asm/kvm_emulate.h
>> @@ -159,7 +159,7 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu 
>> *vcpu)
>>  
>>  static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
>>  {
>> -return vcpu->arch.cp15[c0_MPIDR];
>> +return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
> 
> why?

Because of the two current users one did that masking afterwards and the
second does not care (applies an ever stricter mask).
All the subsequent users in the GICv3 emulation also do this, so I
deemed it useful to have the masking in here. For KVM purposes I guess
we only are interested in the affinity bits. I can rename the function
to make this more obvious if you like.

Cheers,
Andre.

>>  }
>>  
>>  static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
>> diff --git a/arch/arm/include/asm/kvm_host.h 
>> b/arch/arm/include/asm/kvm_host.h
>> index 6dfb404..cf99ad0 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -233,4 +233,6 @@ static inline void vgic_arch_setup(const struct 
>> vgic_params *vgic)
>>  int kvm_perf_init(void);
>>  int kvm_perf_teardown(void);
>>  
>> +struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>> +
>>  #endif /* __ARM_KVM_HOST_H__ */
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 7d5065e..addb990 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -990,6 +990,21 @@ static void check_kvm_target_cpu(void *ret)
>>  *(int *)ret = kvm_target_cpu();
>>  }
>>  
>> +struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
>> +{
>> +unsigned long c_mpidr;
>> +struct kvm_vcpu *vcpu;
>> +int i;
>> +
>> +mpidr &= MPIDR_HWID_BITMASK;
>> +kvm_for_each_vcpu(i, vcpu, kvm) {
>> +c_mpidr = kvm_vcpu_get_mpidr(vcpu);
>> +if (c_mpidr == mpidr)
>> +return vcpu;
>> +}
>> +return NULL;
>> +}
>> +
>>  /**
>>   * Initialize Hyp-mode and memory mappings on all CPUs.
>>   */
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 09cf377..49f0992 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -21,6 +21,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  /*
>>   * This is an implementation of the Power State Coordination Interface
>> @@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
>>  static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>  {
>>  struct kvm *kvm = source_vcpu->kvm;
>> -struct kvm_vcpu *vcpu = NULL, *tmp;
>> +struct kvm_vcpu *vcpu = NULL;
>>  wait_queue_head_t *wq;
>>  unsigned long cpu_id;
>>  unsigned long context_id;
>> -unsigned long mpidr;
>>  phys_addr_t target_pc;
>> -int i;
>>  
>> -cpu_id = *vcpu_reg(source_vcpu, 1);
>> +cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
>>  if (vcpu_mode_is_32bit(source_vcpu))
>>  cpu_id &= ~((u32) 0);
>>  
>> -kvm_for_each_vcpu(i, tmp, kvm) {
>> -mpidr = kvm_vcpu_get_mpidr(tmp);
>> -if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & 
>> MPIDR_HWID_BITMASK)) {
>> -vcpu = tmp;
>> -break;
>> -}
>> -}
>> +vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
>>  
>>  /*
>>   * Make sure the caller requested a valid CPU and that the CPU is
>> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
>> b/arch/arm64/include/asm/kvm_emulate.h
>> index fdc3e21..4b8d636 100644
>> --- a/arch/arm64/include/asm/kvm_emulate.h
>> +++ b/arch/arm64/include/asm/kvm_emulate.h
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
>>  unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu)

Re: [PATCH v2 01/15] arm/arm64: KVM: rework MPIDR assignment and add accessors

2014-10-15 Thread Christoffer Dall
On Thu, Aug 21, 2014 at 02:06:42PM +0100, Andre Przywara wrote:
> The virtual MPIDR registers (containing topology information) for the
> guest are currently mapped linearily to the vcpu_id. Improve this
> mapping for arm64 by using three levels to not artificially limit the
> number of vCPUs. Also add an accessor to later allow easier access to
> a vCPU with a given MPIDR.
> Use this new accessor in the PSCI emulation.
> 
> Signed-off-by: Andre Przywara 
> ---
>  arch/arm/include/asm/kvm_emulate.h   |2 +-
>  arch/arm/include/asm/kvm_host.h  |2 ++
>  arch/arm/kvm/arm.c   |   15 +++
>  arch/arm/kvm/psci.c  |   15 ---
>  arch/arm64/include/asm/kvm_emulate.h |3 ++-
>  arch/arm64/include/asm/kvm_host.h|2 ++
>  arch/arm64/kvm/sys_regs.c|   11 +--
>  7 files changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_emulate.h 
> b/arch/arm/include/asm/kvm_emulate.h
> index 69b7469..18e45f7 100644
> --- a/arch/arm/include/asm/kvm_emulate.h
> +++ b/arch/arm/include/asm/kvm_emulate.h
> @@ -159,7 +159,7 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu 
> *vcpu)
>  
>  static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
>  {
> - return vcpu->arch.cp15[c0_MPIDR];
> + return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;

why?

>  }
>  
>  static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 6dfb404..cf99ad0 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -233,4 +233,6 @@ static inline void vgic_arch_setup(const struct 
> vgic_params *vgic)
>  int kvm_perf_init(void);
>  int kvm_perf_teardown(void);
>  
> +struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> +
>  #endif /* __ARM_KVM_HOST_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 7d5065e..addb990 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -990,6 +990,21 @@ static void check_kvm_target_cpu(void *ret)
>   *(int *)ret = kvm_target_cpu();
>  }
>  
> +struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
> +{
> + unsigned long c_mpidr;
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + mpidr &= MPIDR_HWID_BITMASK;
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + c_mpidr = kvm_vcpu_get_mpidr(vcpu);
> + if (c_mpidr == mpidr)
> + return vcpu;
> + }
> + return NULL;
> +}
> +
>  /**
>   * Initialize Hyp-mode and memory mappings on all CPUs.
>   */
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 09cf377..49f0992 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * This is an implementation of the Power State Coordination Interface
> @@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
>  static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  {
>   struct kvm *kvm = source_vcpu->kvm;
> - struct kvm_vcpu *vcpu = NULL, *tmp;
> + struct kvm_vcpu *vcpu = NULL;
>   wait_queue_head_t *wq;
>   unsigned long cpu_id;
>   unsigned long context_id;
> - unsigned long mpidr;
>   phys_addr_t target_pc;
> - int i;
>  
> - cpu_id = *vcpu_reg(source_vcpu, 1);
> + cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
>   if (vcpu_mode_is_32bit(source_vcpu))
>   cpu_id &= ~((u32) 0);
>  
> - kvm_for_each_vcpu(i, tmp, kvm) {
> - mpidr = kvm_vcpu_get_mpidr(tmp);
> - if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & 
> MPIDR_HWID_BITMASK)) {
> - vcpu = tmp;
> - break;
> - }
> - }
> + vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
>  
>   /*
>* Make sure the caller requested a valid CPU and that the CPU is
> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> b/arch/arm64/include/asm/kvm_emulate.h
> index fdc3e21..4b8d636 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
>  unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
> @@ -179,7 +180,7 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct 
> kvm_vcpu *vcpu)
>  
>  static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
>  {
> - return vcpu_sys_reg(vcpu, MPIDR_EL1);
> + return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;

why?

>  }
>  
>  static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index e10c45a..017fbae 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/

[PATCH v2 01/15] arm/arm64: KVM: rework MPIDR assignment and add accessors

2014-08-21 Thread Andre Przywara
The virtual MPIDR registers (containing topology information) for the
guest are currently mapped linearily to the vcpu_id. Improve this
mapping for arm64 by using three levels to not artificially limit the
number of vCPUs. Also add an accessor to later allow easier access to
a vCPU with a given MPIDR.
Use this new accessor in the PSCI emulation.

Signed-off-by: Andre Przywara 
---
 arch/arm/include/asm/kvm_emulate.h   |2 +-
 arch/arm/include/asm/kvm_host.h  |2 ++
 arch/arm/kvm/arm.c   |   15 +++
 arch/arm/kvm/psci.c  |   15 ---
 arch/arm64/include/asm/kvm_emulate.h |3 ++-
 arch/arm64/include/asm/kvm_host.h|2 ++
 arch/arm64/kvm/sys_regs.c|   11 +--
 7 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/kvm_emulate.h 
b/arch/arm/include/asm/kvm_emulate.h
index 69b7469..18e45f7 100644
--- a/arch/arm/include/asm/kvm_emulate.h
+++ b/arch/arm/include/asm/kvm_emulate.h
@@ -159,7 +159,7 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu 
*vcpu)
 
 static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
 {
-   return vcpu->arch.cp15[c0_MPIDR];
+   return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 6dfb404..cf99ad0 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -233,4 +233,6 @@ static inline void vgic_arch_setup(const struct vgic_params 
*vgic)
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 #endif /* __ARM_KVM_HOST_H__ */
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 7d5065e..addb990 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -990,6 +990,21 @@ static void check_kvm_target_cpu(void *ret)
*(int *)ret = kvm_target_cpu();
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
+{
+   unsigned long c_mpidr;
+   struct kvm_vcpu *vcpu;
+   int i;
+
+   mpidr &= MPIDR_HWID_BITMASK;
+   kvm_for_each_vcpu(i, vcpu, kvm) {
+   c_mpidr = kvm_vcpu_get_mpidr(vcpu);
+   if (c_mpidr == mpidr)
+   return vcpu;
+   }
+   return NULL;
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 09cf377..49f0992 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * This is an implementation of the Power State Coordination Interface
@@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
struct kvm *kvm = source_vcpu->kvm;
-   struct kvm_vcpu *vcpu = NULL, *tmp;
+   struct kvm_vcpu *vcpu = NULL;
wait_queue_head_t *wq;
unsigned long cpu_id;
unsigned long context_id;
-   unsigned long mpidr;
phys_addr_t target_pc;
-   int i;
 
-   cpu_id = *vcpu_reg(source_vcpu, 1);
+   cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
if (vcpu_mode_is_32bit(source_vcpu))
cpu_id &= ~((u32) 0);
 
-   kvm_for_each_vcpu(i, tmp, kvm) {
-   mpidr = kvm_vcpu_get_mpidr(tmp);
-   if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & 
MPIDR_HWID_BITMASK)) {
-   vcpu = tmp;
-   break;
-   }
-   }
+   vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
 
/*
 * Make sure the caller requested a valid CPU and that the CPU is
diff --git a/arch/arm64/include/asm/kvm_emulate.h 
b/arch/arm64/include/asm/kvm_emulate.h
index fdc3e21..4b8d636 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
@@ -179,7 +180,7 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct 
kvm_vcpu *vcpu)
 
 static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
 {
-   return vcpu_sys_reg(vcpu, MPIDR_EL1);
+   return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index e10c45a..017fbae 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -244,4 +244,6 @@ static inline void vgic_arch_setup(const struct vgic_params 
*vgic)
}
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/