On 2017-09-04 11:27, Lokesh Vutla wrote:
> 
> 
> On Monday 04 September 2017 02:45 PM, Jan Kiszka wrote:
>> On 2017-08-30 21:00, Lokesh Vutla wrote:
>>> MPIDR can be used to compare the GICR_TYPER register
>>> for redistributor base calculation. Logic is imported from
>>> kernel.
>>>
>>> Signed-off-by: Lokesh Vutla <[email protected]>
>>> ---
>>>  hypervisor/arch/arm-common/gic-v3.c         | 13 ++++++++++---
>>>  hypervisor/arch/arm/include/asm/sysregs.h   |  7 +++++++
>>>  hypervisor/arch/arm64/include/asm/sysregs.h | 10 ++++++++++
>>>  3 files changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hypervisor/arch/arm-common/gic-v3.c 
>>> b/hypervisor/arch/arm-common/gic-v3.c
>>> index 12ad017..7e03b64 100644
>>> --- a/hypervisor/arch/arm-common/gic-v3.c
>>> +++ b/hypervisor/arch/arm-common/gic-v3.c
>>> @@ -19,6 +19,7 @@
>>>  #include <asm/gic.h>
>>>  #include <asm/irqchip.h>
>>>  #include <asm/setup.h>
>>> +#include <asm/sysregs.h>
>>>  #include <asm/traps.h>
>>>  
>>>  /*
>>> @@ -90,13 +91,19 @@ static void gic_cpu_reset(struct per_cpu *cpu_data)
>>>  static int gic_cpu_init(struct per_cpu *cpu_data)
>>>  {
>>>     unsigned int mnt_irq = system_config->platform_info.arm.maintenance_irq;
>>> -   u64 typer;
>>> -   u32 pidr;
>>> +   u64 typer, mpidr;
>>> +   u32 pidr, aff;
>>>     u32 cell_icc_ctlr, cell_icc_pmr, cell_icc_igrpen1;
>>>     u32 ich_vtr;
>>>     u32 ich_vmcr;
>>>     void *redist_base = gicr_base;
>>>  
>>> +   mpidr = cpu_data->mpidr;
>>> +   aff = (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
>>> +          MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
>>> +          MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 |
>>> +          MPIDR_AFFINITY_LEVEL(mpidr, 0));
>>> +
>>>     /* Find redistributor */
>>>     do {
>>>             pidr = mmio_read32(redist_base + GICR_PIDR2);
>>> @@ -105,7 +112,7 @@ static int gic_cpu_init(struct per_cpu *cpu_data)
>>>                     break;
>>>  
>>>             typer = mmio_read64(redist_base + GICR_TYPER);
>>> -           if (((typer >> 8) & 0xf) == cpu_data->cpu_id) {
>>> +           if ((typer >> 32) == aff) {
>>>                     cpu_data->gicr_base = redist_base;
>>>                     break;
>>>             }
>>> diff --git a/hypervisor/arch/arm/include/asm/sysregs.h 
>>> b/hypervisor/arch/arm/include/asm/sysregs.h
>>> index e65adc4..4474794 100644
>>> --- a/hypervisor/arch/arm/include/asm/sysregs.h
>>> +++ b/hypervisor/arch/arm/include/asm/sysregs.h
>>> @@ -83,6 +83,13 @@
>>>  #define  SCTLR_C_AND_M_SET(sctlr)  \
>>>     (((sctlr) & (SCTLR_C_BIT | SCTLR_M_BIT)) == (SCTLR_C_BIT | SCTLR_M_BIT))
>>>  
>>> +#define MPIDR_LEVEL_BITS           8
>>> +#define MPIDR_LEVEL_MASK           ((1 << MPIDR_LEVEL_BITS) - 1)
>>> +#define MPIDR_LEVEL_SHIFT(level)   (MPIDR_LEVEL_BITS * (level))
>>> +
>>> +#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>>> +   (((mpidr) >> (MPIDR_LEVEL_BITS * (level))) & MPIDR_LEVEL_MASK)
>>> +
>>>  /* Bits to wipe on cell reset */
>>>  #define  SCTLR_MASK        (SCTLR_M_BIT | SCTLR_A_BIT | SCTLR_C_BIT        
>>> \
>>>                     | SCTLR_I_BIT | SCTLR_V_BIT | SCTLR_WXN_BIT     \
>>> diff --git a/hypervisor/arch/arm64/include/asm/sysregs.h 
>>> b/hypervisor/arch/arm64/include/asm/sysregs.h
>>> index 09135fa..ed4164f 100644
>>> --- a/hypervisor/arch/arm64/include/asm/sysregs.h
>>> +++ b/hypervisor/arch/arm64/include/asm/sysregs.h
>>> @@ -33,6 +33,16 @@
>>>  #define MPIDR_MP_BIT               (1 << 31)
>>>  #define MPIDR_U_BIT                (1 << 30)
>>>  
>>> +#define MPIDR_LEVEL_BITS_SHIFT     3
>>> +#define MPIDR_LEVEL_BITS   (1 << MPIDR_LEVEL_BITS_SHIFT)
>>> +#define MPIDR_LEVEL_MASK   ((1 << MPIDR_LEVEL_BITS) - 1)
>>> +
>>> +#define MPIDR_LEVEL_SHIFT(level) \
>>> +   (((1 << (level)) >> 1) << MPIDR_LEVEL_BITS_SHIFT)
>>
>> Will this really do the right thing for level=3? Doesn't seem to. Also
> 
> hmm..it will. For level = 3, shift is 32 which is right.
> 

Indeed.

>> (1 << x) >> 1 looks like 1 << (x-1) to me, no? The latter is simpler IMHO.
> 
> Yes even I observed that. In fact, it can be decoded as (1 << (level - 1
> + BITS_SHIFT)). But I wanted to keep it consistent with linux kernel[1].
> So I have that definition intact with Linux.
> 

Actually, that won't work for level = 0. So, while being complex,
your/Linux' logic is correct. Sorry for the noise.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to