Re: [Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

2015-06-08 Thread Julien Grall

Hi Ian,

On 08/06/2015 11:05, Ian Campbell wrote:

On Fri, 2015-06-05 at 19:18 +0100, Julien Grall wrote:

On 05/06/2015 16:56, Ian Campbell wrote:

On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:

From: Chen Baozi 

There are 3 places to change:

* Initialise vMPIDR value in vcpu_initialise()
* Find the vCPU from vMPIDR affinity information when accessing GICD
registers in vGIC
* Find the vCPU from vMPIDR affinity information when booting with vPSCI
in vGIC
- Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.


Does this "- Also ..." not need to be done at the same time as the
change to how we describe things in the FDT? Since that is where the
guest gets the parameter from, isn't it?


Well, we only support 8 CPUs.


I was assuming this would change later in the patch. I don't think PSCI
0.1 is limited to gic v2, is it? (If it is then this is worth mentioning
in the commit message too)


It's not limited to GICv2. But I wasn't able to find the definition of 
CPUID parameter.



  So this changes will return the same value
as before. It may be worth to mention it.

In another side, both PSCI 0.1 and PSCI 0.2 are modified to respect the
MPIDR like within this patch. The working in the commit message may be
misleading.


Yes, possibly.


Somehow the code path slightly differ when the PSCI 0.2 for guest has
been added. The spec says (PSCI 0.1 Section 6.3 (ARM DEN 0022A)):

"Ideally platform discovery mechanism such as firmware tables would be
used by secure firmware to describe the set of valid CPUIDs to the
hypervisor or Rich OS, if the former is not present. The hypervisor in
turn can create and supply virtual discovery mechanisms to its guests.""

I interpreted this as CPUID is equal to the "reg" register in DT (which
is an MPIDR-like value).

FWIW, this is the interpretation made by Linux too.


Documentation/devicetree/bindings/arm/cpus.txt requires it to be the
MPIDR value on armv7 and v8.


Well this document only describes the device tree binding. I don't find 
anything which say that this value should be directly passed to the 
PSCI. Did I miss something?


Not really related to this patch in particular, but the size of "reg" 
may be 2 cells which we currently don't handle.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

2015-06-08 Thread Ian Campbell
On Fri, 2015-06-05 at 19:18 +0100, Julien Grall wrote:
> On 05/06/2015 16:56, Ian Campbell wrote:
> > On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:
> >> From: Chen Baozi 
> >>
> >> There are 3 places to change:
> >>
> >> * Initialise vMPIDR value in vcpu_initialise()
> >> * Find the vCPU from vMPIDR affinity information when accessing GICD
> >>registers in vGIC
> >> * Find the vCPU from vMPIDR affinity information when booting with vPSCI
> >>in vGIC
> >>- Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.
> >
> > Does this "- Also ..." not need to be done at the same time as the
> > change to how we describe things in the FDT? Since that is where the
> > guest gets the parameter from, isn't it?
> 
> Well, we only support 8 CPUs.

I was assuming this would change later in the patch. I don't think PSCI
0.1 is limited to gic v2, is it? (If it is then this is worth mentioning
in the commit message too)

>  So this changes will return the same value 
> as before. It may be worth to mention it.
> 
> In another side, both PSCI 0.1 and PSCI 0.2 are modified to respect the 
> MPIDR like within this patch. The working in the commit message may be 
> misleading.

Yes, possibly.

> Somehow the code path slightly differ when the PSCI 0.2 for guest has 
> been added. The spec says (PSCI 0.1 Section 6.3 (ARM DEN 0022A)):
> 
> "Ideally platform discovery mechanism such as firmware tables would be
> used by secure firmware to describe the set of valid CPUIDs to the
> hypervisor or Rich OS, if the former is not present. The hypervisor in
> turn can create and supply virtual discovery mechanisms to its guests.""
> 
> I interpreted this as CPUID is equal to the "reg" register in DT (which 
> is an MPIDR-like value).
> 
> FWIW, this is the interpretation made by Linux too.

Documentation/devicetree/bindings/arm/cpus.txt requires it to be the
MPIDR value on armv7 and v8.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

2015-06-05 Thread Julien Grall

On 05/06/2015 16:56, Ian Campbell wrote:

On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:

From: Chen Baozi 

There are 3 places to change:

* Initialise vMPIDR value in vcpu_initialise()
* Find the vCPU from vMPIDR affinity information when accessing GICD
   registers in vGIC
* Find the vCPU from vMPIDR affinity information when booting with vPSCI
   in vGIC
   - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.


Does this "- Also ..." not need to be done at the same time as the
change to how we describe things in the FDT? Since that is where the
guest gets the parameter from, isn't it?


Well, we only support 8 CPUs. So this changes will return the same value 
as before. It may be worth to mention it.


In another side, both PSCI 0.1 and PSCI 0.2 are modified to respect the 
MPIDR like within this patch. The working in the commit message may be 
misleading.


Somehow the code path slightly differ when the PSCI 0.2 for guest has 
been added. The spec says (PSCI 0.1 Section 6.3 (ARM DEN 0022A)):


"Ideally platform discovery mechanism such as firmware tables would be
used by secure firmware to describe the set of valid CPUIDs to the
hypervisor or Rich OS, if the former is not present. The hypervisor in
turn can create and supply virtual discovery mechanisms to its guests.""

I interpreted this as CPUID is equal to the "reg" register in DT (which 
is an MPIDR-like value).


FWIW, this is the interpretation made by Linux too.

Regards,

--
--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

2015-06-05 Thread Ian Campbell
On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:
> From: Chen Baozi 
> 
> There are 3 places to change:
> 
> * Initialise vMPIDR value in vcpu_initialise()
> * Find the vCPU from vMPIDR affinity information when accessing GICD
>   registers in vGIC
> * Find the vCPU from vMPIDR affinity information when booting with vPSCI
>   in vGIC
>   - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.

Does this "- Also ..." not need to be done at the same time as the
change to how we describe things in the FDT? Since that is where the
guest gets the parameter from, isn't it?

> Signed-off-by: Chen Baozi 
> Reviewed-by: Julien Grall 
> ---
>  xen/arch/arm/domain.c  | 6 +-
>  xen/arch/arm/vgic-v3.c | 2 +-
>  xen/arch/arm/vpsci.c   | 5 +
>  3 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 2bde26e..0cf147c 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -501,11 +501,7 @@ int vcpu_initialise(struct vcpu *v)
>  
>  v->arch.sctlr = SCTLR_GUEST_INIT;
>  
> -/*
> - * By default exposes an SMP system with AFF0 set to the VCPU ID
> - * TODO: Handle multi-threading processor and cluster
> - */
> -v->arch.vmpidr = MPIDR_SMP | (v->vcpu_id << MPIDR_AFF0_SHIFT);
> +v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>  
>  v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
>  
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 540f85f..ef9a71a 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -61,7 +61,7 @@ static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain 
> *d, uint64_t irouter)
>  if ( irouter & GICD_IROUTER_SPI_MODE_ANY )
>  return d->vcpu[0];
>  
> -vcpu_id = irouter & MPIDR_AFF0_MASK;
> +vcpu_id = vaffinity_to_vcpuid(irouter);
>  if ( vcpu_id >= d->max_vcpus )
>  return NULL;
>  
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 5d899be..aebe1e2 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -32,10 +32,7 @@ static int do_common_cpu_on(register_t target_cpu, 
> register_t entry_point,
>  int is_thumb = entry_point & 1;
>  register_t vcpuid;
>  
> -if( ver == XEN_PSCI_V_0_2 )
> -vcpuid = (target_cpu & MPIDR_HWID_MASK);
> -else
> -vcpuid = target_cpu;
> +vcpuid = vaffinity_to_vcpuid(target_cpu);
>  
>  if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
>  return PSCI_INVALID_PARAMETERS;



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

2015-06-01 Thread Chen Baozi
From: Chen Baozi 

There are 3 places to change:

* Initialise vMPIDR value in vcpu_initialise()
* Find the vCPU from vMPIDR affinity information when accessing GICD
  registers in vGIC
* Find the vCPU from vMPIDR affinity information when booting with vPSCI
  in vGIC
  - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.

Signed-off-by: Chen Baozi 
Reviewed-by: Julien Grall 
---
 xen/arch/arm/domain.c  | 6 +-
 xen/arch/arm/vgic-v3.c | 2 +-
 xen/arch/arm/vpsci.c   | 5 +
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2bde26e..0cf147c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -501,11 +501,7 @@ int vcpu_initialise(struct vcpu *v)
 
 v->arch.sctlr = SCTLR_GUEST_INIT;
 
-/*
- * By default exposes an SMP system with AFF0 set to the VCPU ID
- * TODO: Handle multi-threading processor and cluster
- */
-v->arch.vmpidr = MPIDR_SMP | (v->vcpu_id << MPIDR_AFF0_SHIFT);
+v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
 
 v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
 
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 540f85f..ef9a71a 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -61,7 +61,7 @@ static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, 
uint64_t irouter)
 if ( irouter & GICD_IROUTER_SPI_MODE_ANY )
 return d->vcpu[0];
 
-vcpu_id = irouter & MPIDR_AFF0_MASK;
+vcpu_id = vaffinity_to_vcpuid(irouter);
 if ( vcpu_id >= d->max_vcpus )
 return NULL;
 
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 5d899be..aebe1e2 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -32,10 +32,7 @@ static int do_common_cpu_on(register_t target_cpu, 
register_t entry_point,
 int is_thumb = entry_point & 1;
 register_t vcpuid;
 
-if( ver == XEN_PSCI_V_0_2 )
-vcpuid = (target_cpu & MPIDR_HWID_MASK);
-else
-vcpuid = target_cpu;
+vcpuid = vaffinity_to_vcpuid(target_cpu);
 
 if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
 return PSCI_INVALID_PARAMETERS;
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel