Re: [XenARM] [PATCH] xen/arm: Add CPU ID for Broadcom Brahma-B15

[Marc C <marc.ceeeee@xxxxxxxxx>]

Hello Ian,

> Thanks for resending, I saw the original but somehow failed to add it to
> my queue of things to look at, sorry!

No problem. Thanks for taking the time to look at the patch!

>> +        .long 0x420F00F2             /* Broadcom Brahma-B15 */
>> +        .long 0xFF0FFFFF             /* Mask */
>
> This mask includes the revision field (last nibble). Is it intentional
> too only support a single revision of this processor?

Yes. At this time, supporting only this specific version of this CPU
is required.

>> +        .long v7_init
>
> v7_init fiddles with the implementation defined ACTLR register in a
> Cortex-A15/A7 specific manner. Is the B15 compatible with ARM's Cortex
> A15 in this regard?

Correct. The B15 will also require the SMP bit to be OR'ed in ACTLR,
so they can be considered compatible in this regard.

I could go ahead with refactoring the code in proc-v7.S, especially if
you expect there to be any future ARMv7 chips with unique
initialization requirements. I think going with your suggestion would
align the code with the Linux kernel's arch/arm/mm/proc-v7.S, which I
assume is a good thing!

Thanks,
Marc

On Thu, Oct 17, 2013 at 2:47 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Wed, 2013-10-16 at 15:57 -0700, marc.ceeeee@xxxxxxxxx wrote:
>> From: Marc Carino <marc.ceeeee@xxxxxxxxx>
>>
>> Let Xen recognize the Broadcom Brahma-B15 CPU by adding the appropriate
>> MIDR mask to the initialization phase. Further, ensure that the console
>> output properly reports the CPU manufacturer as "Broadcom Corporation".
>
> Thanks for resending, I saw the original but somehow failed to add it to
> my queue of things to look at, sorry!
>
>>
>> Signed-off-by: Marc Carino <marc.ceeeee@xxxxxxxxx>
>> ---
>>  xen/arch/arm/arm32/proc-v7.S |    8 ++++++++
>>  xen/arch/arm/setup.c         |    1 +
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
>> index 6577a89..2c8cb9c 100644
>> --- a/xen/arch/arm/arm32/proc-v7.S
>> +++ b/xen/arch/arm/arm32/proc-v7.S
>> @@ -43,6 +43,14 @@ __v7_ca7mp_proc_info:
>>          .long v7_init
>>          .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
>>
>> +        .section ".init.proc.info", #alloc, #execinstr
>> +        .type __v7_brahma15mp_proc_info, #object
>> +__v7_brahma15mp_proc_info:
>> +        .long 0x420F00F2             /* Broadcom Brahma-B15 */
>> +        .long 0xFF0FFFFF             /* Mask */
>
> This mask includes the revision field (last nibble). Is it intentional
> too only support a single revision of this processor?
>
>> +        .long v7_init
>
> v7_init fiddles with the implementation defined ACTLR register in a
> Cortex-A15/A7 specific manner. Is the B15 compatible with ARM's Cortex
> A15 in this regard?
>
> If not then I think this should be changed to:
>
> ca7mp_init:
> ca15mp_init:
>         /* Set up the SMP bit in ACTLR */
>         mrc   CP32(r0, ACTLR)
>         orr   r0, r0, #(ACTLR_V7_SMP) /* enable SMP bit */
>         mcr   CP32(r0, ACTLR)
> v7_init: /* Generic v7 init */
>         mov   pc, lr
>
> and the A15/A7 entries updated to point to the new names.
>
> Perhaps there isn't much point in listing a processor here which doesn't
> need any init, but doing so will mean it'll get picked up if we ever add
> some init for generic v7, or if we add new hooks etc.
>
> Ian.
>
>> +        .size __v7_brahma15mp_proc_info, . - __v7_brahma15mp_proc_info
>> +
>>  /*
>>   * Local variables:
>>   * mode: ASM
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 49f344c..7d6e596 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -61,6 +61,7 @@ static void __init init_idle_domain(void)
>>
>>  static const char * __initdata processor_implementers[] = {
>>      ['A'] = "ARM Limited",
>> +    ['B'] = "Broadcom Corporation",
>>      ['D'] = "Digital Equipment Corp",
>>      ['M'] = "Motorola, Freescale Semiconductor Inc.",
>>      ['Q'] = "Qualcomm Inc.",
>
>

_______________________________________________
Xen-arm mailing list
Xen-arm@xxxxxxxxxxxxx
http://lists.xen.org/cgi-bin/mailman/listinfo/xen-arm