Re: [RFC 27/29] xen/arm: Add platform specific code for the exynos5

[Julien Grall <julien.grall@linaro.org>]

On 04/30/2013 11:00 AM, Ian Campbell wrote:

> On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/arch/arm/platforms/Makefile         |    1 +
>>  xen/arch/arm/platforms/exynos5.c        |  105 +++++++++++++++++++++++++++++++
>>  xen/include/asm-arm/platforms/exynos5.h |   40 ++++++++++++
>>  3 files changed, 146 insertions(+)
>>  create mode 100644 xen/arch/arm/platforms/exynos5.c
>>  create mode 100644 xen/include/asm-arm/platforms/exynos5.h
>>
>> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
>> index 4313e95..ff2b65b 100644
>> --- a/xen/arch/arm/platforms/Makefile
>> +++ b/xen/arch/arm/platforms/Makefile
>> @@ -1 +1,2 @@
>>  obj-y += vexpress.o
>> +obj-y += exynos5.o
>> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
>> new file mode 100644
>> index 0000000..01e12b7
>> --- /dev/null
>> +++ b/xen/arch/arm/platforms/exynos5.c
>> @@ -0,0 +1,105 @@
>> +/*
>> + * xen/arch/arm/platforms/exynos5.c
>> + *
>> + * Exynos5 specific settings
>> + *
>> + * Julien Grall <julien.grall@linaro.org>
>> + * Copyright (c) 2013 Linaro Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <asm/p2m.h>
>> +#include <xen/config.h>
>> +#include <xen/device_tree.h>
>> +#include <xen/domain_page.h>
>> +#include <xen/mm.h>
>> +#include <asm/platforms/exynos5.h>
>> +#include <asm/platform.h>
>> +
>> +static int exynos5_init_time(void)
>> +{
>> +    uint32_t reg;
>> +
>> +    // enable timer on exynos5 arndale board
>> +    // should probably be done by u-boot
> 
> /* */ please.
> 
>> +    reg = platform_read_register(EXYNOS5_MCT_G_TCON);
>> +    platform_write_register(EXYNOS5_MCT_G_TCON, reg | EXYNOS5_MCT_G_TCON_START);
>> +
>> +    return 0;
>> +}
>> +
>> +/* Additionnal mapping for dom0 (Not in the DTS) */
> 
> "Additional mappings"
> 
>> +static int exynos5_specific_mapping(struct domain *d)
>> +{
>> +    paddr_t ma = 0;
>> +    uint32_t *dst;
>> +    int res;
>> +
>> +    /*
>> +     * Set temporary guest traps with 0xe14fff7c which is hvc(0xfffc)
>> +     * a hyp panic!
>> +     * TODO: Find why:
>> +     *  1) Xen abort directly after local_abort_enable when
>> +     *  the p2m_populate_ram is not here.
> 
> It will actually be aborting here somewhere but that abort can't be
> delivered until the point aborts are enabled. My patch "xen: arm: enable
> aborts on all physical processors." will enable aborts much earlier so
> you might get a better indication of exactly where it goes wrong.


My comment is wrong. In fact, I try to map domain memory before the
guest page table is loaded on the CPU.

>> +     *  2) Linux doesn't start without this trick
> 
> Ouch. I presume it doesn't actually end up calling hvc 0xfffc? Does it
> work if you just map/unmap without writing anything?
> 
> What does Linux do without the trick? Touches address 0x0 perhaps? Would
> anything be mapped there on a real Exynos?

Not even.

(XEN) Guest data abort: Translation fault at level 2
(XEN)     gva=40004000
(XEN)     gpa=0000000040004000
(XEN)     instruction syndrome invalid
(XEN)     eat=0 cm=0 s1ptw=0 dfsc=6
(XEN) dom0 IPA 0x0000000040004000
(XEN) P2M @ 02ffbfc0 mfn:0xbfdfe
(XEN) 1ST[0x1] = 0x00000000bfdfb6ff
(XEN) 2ND[0x0] = 0x0000000000000000
(XEN) ----[ Xen-4.3-unstable  arm32  debug=y  Tainted:    C ]----
(XEN) CPU:    0
(XEN) PC:     50008338
(XEN) CPSR:   800001d3 MODE:32-bit Guest SVC
(XEN)      R0: 40004000 R1: 00000c12 R2: 40008000 R3: 40004000
(XEN)      R4: 40008000 R5: 00000000 R6: 0000000e R7: ffffffff
(XEN)      R8: 501bf8e0 R9: 40000000 R10:50000000 R11:10201105 R12:500080a8
(XEN) USR: SP: 00000000 LR: 00000000
(XEN) SVC: SP: 00000000 LR: 500083cc SPSR:000001d3
(XEN) ABT: SP: 00000000 LR: 00000000 SPSR:00000000
(XEN) UND: SP: 00000000 LR: 00000000 SPSR:00000000
(XEN) IRQ: SP: 00000000 LR: 00000000 SPSR:00000000
(XEN) FIQ: SP: 00000000 LR: 00000000 SPSR:00000000
(XEN) FIQ: R8: 00000000 R9: 00000000 R10:00000000 R11:00000000 R12:00000000
(XEN)
(XEN) TTBR0 0000000000 TTBR1 0000000000 TCR 00000000
(XEN) SCTLR 00c50078
(XEN) IFAR 00000000 DFAR 00000000
(XEN)
(XEN) HTTBR bfed2000
(XEN) HDFAR 40004000
(XEN) HIFAR 0
(XEN) HPFAR 400040
(XEN) HCR 00002835
(XEN) HSR   90000046
(XEN) VTTBR 10000bfdfe000
(XEN)
(XEN) DFSR 0 DFAR 0
(XEN) IFSR 0 IFAR 0
(XEN)
(XEN) GUEST STACK GOES HERE
(XEN) domain_crash_sync called from traps.c:968

FYI the kernel is loaded at 0x50008000.

> I had a weird one running on the v8 foundation model where I had to add
> a NOP hypercall to Linux's head.S before a certain point -- I wonder if
> this is related?

I did some test and noticed this code is at the wrong place. I didn't
find the issue because with/without this trick linux 3.9 will boot. It's
only happened on the linux 3.7 tree.

The code should be done after the kernel is loading in the memory. But I
think I should remove this code and patch the kernel. What do you think?

By the way, which kernel version do you use? What is your modification?

> It'll be a missing flush or barrier of course, the question is where ;-)


Before the unmap_domain_page, right?

 
>> +     */
>> +    p2m_populate_ram(d, 0x0, 0x1000 - 1);
>> +
>> +    res = gvirt_to_maddr(0, &ma);
>> +    if ( res )
>> +    {
>> +        printk(XENLOG_ERR "Unable to translate guest address\n");
>> +        return -EFAULT;
>> +    }
>> +
>> +    dst = map_domain_page(ma >> PAGE_SHIFT);
>> +    dst[2] = 0xe14fff7c;
>> +    unmap_domain_page(dst);
>> +
>> +    /* Map the chip ID */
>> +    map_mmio_regions(d, EXYNOS5_PA_CHIPID, EXYNOS5_PA_CHIPID + PAGE_SIZE - 1,
>> +                     EXYNOS5_PA_CHIPID);
>> +
>> +    /* Map the PWM region */
>> +    map_mmio_regions(d, EXYNOS5_PA_TIMER,
>> +                     EXYNOS5_PA_TIMER + (PAGE_SIZE * 2) - 1,
>> +                     EXYNOS5_PA_TIMER);
>> +
>> +    return 0;
>> +}
>> +
>> +static void exynos5_reset(void)
>> +{
>> +    platform_write_register(EXYNOS5_SWRESET, 1);
>> +}
>> +
>> +static const char const *exynos5_dt_compat[] __initdata =
>> +{
>> +    "samsung,exynos5250",
>> +    NULL
>> +};
>> +
>> +PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
>> +    .compatible = exynos5_dt_compat,
>> +    .init_time = exynos5_init_time,
>> +    .specific_mapping = exynos5_specific_mapping,
>> +    .reset = exynos5_reset,
>> +PLATFORM_END
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/include/asm-arm/platforms/exynos5.h b/xen/include/asm-arm/platforms/exynos5.h
>> new file mode 100644
>> index 0000000..d77623c
>> --- /dev/null
>> +++ b/xen/include/asm-arm/platforms/exynos5.h
>> @@ -0,0 +1,40 @@
>> +#ifndef __ASM_ARM_PLATFORMS_EXYNOS5_H
>> +#define __ASM_ASM_PLATFORMS_EXYSNO5_H
>> +
>> +#define EXYNOS5_MCT_BASE            0x101c0000
>> +#define EXYNOS5_MCTREG(x)           (EXYNOS5_MCT_BASE + (x))
>> +#define EXYNOS5_MCT_G_TCON		    EXYNOS5_MCTREG(0x240)
>> +#define EXYNOS5_MCT_G_TCON_START	(1 << 8)
>> +
>> +#define EXYNOS5_PA_CHIPID           0x10000000
>> +#define EXYNOS5_PA_TIMER            0x12dd0000
>> +/* Base address of system controller */
>> +#define EXYNOS5_PA_PMU              0x10040000
>> +
>> +#define EXYNOS5_SWRESET             (EXYNOS5_PA_PMU + 0x0400)
>> +
>> +#define S5P_PA_SYSRAM   0x02020000
>> +
>> +/* Constants below is only used in assembly because the DTS is not yet parsed */
>> +#ifdef __ASSEMBLY__
>> +
>> +/* GIC Base Address */
>> +#define EXYNOS5_GIC_BASE_ADDRESS    0x10480000
>> +
>> +/* Timer's frequency */
>> +#define EXYNOS5_TIMER_FREQUENCY     (24 * 1000 * 1000) /* 24 MHz */
>> +
>> +/* Arndale machine ID */
>> +#define MACH_TYPE_SMDK5250          3774
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* __ASM_ARM_PLATFORMS_EXYNOS5_H */
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
> 
>