Hi all, this patch series introduces a very simple driver for the ARM HDLCD Controller, that means that we can finally have something on the screen while Xen is booting on ARM :) The driver is capable of reading the mode property on device tree and setting the HDLCD accordingly. It is also capable of setting the required OSC5 timer to the right frequency for the pixel clock. In order to reduce code duplication with x86, I tried to generalize the existing vesa character rendering functions into a architecture agnostic framebuffer driver that can be used by vesa and the hdlcd drivers. I would very much appreciate if you could give a close look at the vesa changes because I don''t have any x86 test machines that boot in vesa mode, therefore I couldn''t test it. Changes in v3: - rename fb_cr to fb_carriage_return. Changes in v2: - rebase on latest xen-unstable; - add support for multiple resolutions; - add support to dynamically change the OSC5 motherboard timer; - add the patch "preserve DTB mappings". Stefano Stabellini (8): xen/arm: introduce early_ioremap xen: infrastructure to have cross-platform video drivers xen: introduce a generic framebuffer driver xen/vesa: use the new fb_* functions xen/arm: preserve DTB mappings xen/device_tree: introduce find_compatible_node xen/arm: introduce vexpress_syscfg xen/arm: introduce a driver for the ARM HDLCD controller xen/arch/arm/Makefile | 1 + xen/arch/arm/Rules.mk | 2 + xen/arch/arm/kernel.h | 2 + xen/arch/arm/mm.c | 44 +++++ xen/arch/arm/platform_vexpress.c | 97 +++++++++++ xen/arch/arm/setup.c | 8 +- xen/arch/x86/Rules.mk | 1 + xen/common/device_tree.c | 51 ++++++ xen/drivers/Makefile | 2 +- xen/drivers/char/console.c | 12 +- xen/drivers/video/Makefile | 12 +- xen/drivers/video/arm_hdlcd.c | 282 +++++++++++++++++++++++++++++++ xen/drivers/video/fb.c | 209 +++++++++++++++++++++++ xen/drivers/video/fb.h | 49 ++++++ xen/drivers/video/modelines.h | 69 ++++++++ xen/drivers/video/vesa.c | 179 +++----------------- xen/drivers/video/vga.c | 12 +- xen/include/asm-arm/config.h | 4 + xen/include/asm-arm/mm.h | 5 +- xen/include/asm-arm/page.h | 23 +++ xen/include/asm-arm/platform_vexpress.h | 23 +++ xen/include/asm-x86/config.h | 1 + xen/include/xen/device_tree.h | 3 + xen/include/xen/vga.h | 9 +- xen/include/xen/video.h | 24 +++ 25 files changed, 940 insertions(+), 184 deletions(-) Cheers, Stefano
Introduce a function to map a range of physical memory into Xen virtual memory. It doesn''t need domheap to be setup. It is going to be used to map the videoram. Add flush_xen_data_tlb_range, that flushes a range of virtual addresses. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/mm.c | 32 ++++++++++++++++++++++++++++++++ xen/include/asm-arm/config.h | 2 ++ xen/include/asm-arm/mm.h | 3 ++- xen/include/asm-arm/page.h | 23 +++++++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 855f83d..0d7a163 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -367,6 +367,38 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe) frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pages * sizeof(struct page_info)); } +/* Map the physical memory range start - start + len into virtual + * memory and return the virtual address of the mapping. + * start has to be 2MB aligned. + * len has to be < EARLY_VMAP_END - EARLY_VMAP_START. + */ +void* early_ioremap(paddr_t start, size_t len, unsigned attributes) +{ + static unsigned long virt_start = EARLY_VMAP_START; + void* ret_addr = (void *)virt_start; + paddr_t end = start + len; + + ASSERT(!(start & (~SECOND_MASK))); + ASSERT(!(virt_start & (~SECOND_MASK))); + + /* The range we need to map is too big */ + if ( virt_start + len >= EARLY_VMAP_END ) + return NULL; + + while ( start < end ) + { + lpae_t e = mfn_to_xen_entry(start >> PAGE_SHIFT); + e.pt.ai = attributes; + write_pte(xen_second + second_table_offset(virt_start), e); + + start += SECOND_SIZE; + virt_start += SECOND_SIZE; + } + flush_xen_data_tlb_range((unsigned long) ret_addr, len); + + return ret_addr; +} + enum mg { mg_clear, mg_ro, mg_rw, mg_rx }; static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) { diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 2a05539..87db0d1 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -73,9 +73,11 @@ #define FIXMAP_ADDR(n) (mk_unsigned_long(0x00400000) + (n) * PAGE_SIZE) #define BOOT_MISC_VIRT_START mk_unsigned_long(0x00600000) #define FRAMETABLE_VIRT_START mk_unsigned_long(0x02000000) +#define EARLY_VMAP_START mk_unsigned_long(0x10000000) #define XENHEAP_VIRT_START mk_unsigned_long(0x40000000) #define DOMHEAP_VIRT_START mk_unsigned_long(0x80000000) +#define EARLY_VMAP_END XENHEAP_VIRT_START #define HYPERVISOR_VIRT_START XEN_VIRT_START #define DOMHEAP_ENTRIES 1024 /* 1024 2MB mapping slots */ diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index e95ece1..4ed5df6 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -150,7 +150,8 @@ extern void setup_frametable_mappings(paddr_t ps, paddr_t pe); extern void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes); /* Remove a mapping from a fixmap entry */ extern void clear_fixmap(unsigned map); - +/* map a 2MB aligned physical range in virtual memory. */ +void* early_ioremap(paddr_t start, size_t len, unsigned attributes); #define mfn_valid(mfn) ({ \ unsigned long __m_f_n = (mfn); \ diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h index d89261e..0790dda 100644 --- a/xen/include/asm-arm/page.h +++ b/xen/include/asm-arm/page.h @@ -328,6 +328,23 @@ static inline void flush_xen_data_tlb_va(unsigned long va) : : "r" (va) : "memory"); } +/* + * Flush a range of VA''s hypervisor mappings from the data TLB. This is not + * sufficient when changing code mappings or for self modifying code. + */ +static inline void flush_xen_data_tlb_range(unsigned long va, unsigned long size) +{ + unsigned long end = va + size; + while ( va < end ) { + asm volatile("dsb;" /* Ensure preceding are visible */ + STORE_CP32(0, TLBIMVAH) + "dsb;" /* Ensure completion of the TLB flush */ + "isb;" + : : "r" (va) : "memory"); + va += PAGE_SIZE; + } +} + /* Flush all non-hypervisor mappings from the TLB */ static inline void flush_guest_tlb(void) { @@ -418,8 +435,14 @@ static inline uint64_t gva_to_ipa(uint32_t va) #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1) #define THIRD_SHIFT PAGE_SHIFT +#define THIRD_SIZE (1u << THIRD_SHIFT) +#define THIRD_MASK (~(THIRD_SIZE - 1)) #define SECOND_SHIFT (THIRD_SHIFT + LPAE_SHIFT) +#define SECOND_SIZE (1u << SECOND_SHIFT) +#define SECOND_MASK (~(SECOND_SIZE - 1)) #define FIRST_SHIFT (SECOND_SHIFT + LPAE_SHIFT) +#define FIRST_SIZE (1u << FIRST_SHIFT) +#define FIRST_MASK (~(FIRST_SIZE - 1)) /* Calculate the offsets into the pagetables for a given VA */ #define first_linear_offset(va) (va >> FIRST_SHIFT) -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 2/8] xen: infrastructure to have cross-platform video drivers
- introduce a new HAS_VIDEO config variable; - build xen/drivers/video/font* if HAS_VIDEO; - rename vga_puts to video_puts; - rename vga_init to video_init; - rename vga_endboot to video_endboot. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> --- xen/arch/arm/Rules.mk | 1 + xen/arch/x86/Rules.mk | 1 + xen/drivers/Makefile | 2 +- xen/drivers/char/console.c | 12 ++++++------ xen/drivers/video/Makefile | 10 +++++----- xen/drivers/video/vesa.c | 4 ++-- xen/drivers/video/vga.c | 12 ++++++------ xen/include/asm-x86/config.h | 1 + xen/include/xen/vga.h | 9 +-------- xen/include/xen/video.h | 24 ++++++++++++++++++++++++ 10 files changed, 48 insertions(+), 28 deletions(-) create mode 100644 xen/include/xen/video.h diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk index a45c654..fa9f9c1 100644 --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -7,6 +7,7 @@ # HAS_DEVICE_TREE := y +HAS_VIDEO := y CFLAGS += -fno-builtin -fno-common -Wredundant-decls CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index 963850f..0a9d68d 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -3,6 +3,7 @@ HAS_ACPI := y HAS_VGA := y +HAS_VIDEO := y HAS_CPUFREQ := y HAS_PCI := y HAS_PASSTHROUGH := y diff --git a/xen/drivers/Makefile b/xen/drivers/Makefile index 7239375..9c70f20 100644 --- a/xen/drivers/Makefile +++ b/xen/drivers/Makefile @@ -3,4 +3,4 @@ subdir-$(HAS_CPUFREQ) += cpufreq subdir-$(HAS_PCI) += pci subdir-$(HAS_PASSTHROUGH) += passthrough subdir-$(HAS_ACPI) += acpi -subdir-$(HAS_VGA) += video +subdir-$(HAS_VIDEO) += video diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index ff360fe..1b7a593 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -21,7 +21,7 @@ #include <xen/delay.h> #include <xen/guest_access.h> #include <xen/shutdown.h> -#include <xen/vga.h> +#include <xen/video.h> #include <xen/kexec.h> #include <asm/debugger.h> #include <asm/div64.h> @@ -297,7 +297,7 @@ static void dump_console_ring_key(unsigned char key) buf[sofar] = ''\0''; sercon_puts(buf); - vga_puts(buf); + video_puts(buf); free_xenheap_pages(buf, order); } @@ -383,7 +383,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) spin_lock_irq(&console_lock); sercon_puts(kbuf); - vga_puts(kbuf); + video_puts(kbuf); if ( opt_console_to_ring ) { @@ -464,7 +464,7 @@ static void __putstr(const char *str) ASSERT(spin_is_locked(&console_lock)); sercon_puts(str); - vga_puts(str); + video_puts(str); if ( !console_locks_busted ) { @@ -592,7 +592,7 @@ void __init console_init_preirq(void) if ( *p == '','' ) p++; if ( !strncmp(p, "vga", 3) ) - vga_init(); + video_init(); else if ( !strncmp(p, "none", 4) ) continue; else if ( (sh = serial_parse_handle(p)) >= 0 ) @@ -694,7 +694,7 @@ void __init console_endboot(void) printk("\n"); } - vga_endboot(); + video_endboot(); /* * If user specifies so, we fool the switch routine to redirect input diff --git a/xen/drivers/video/Makefile b/xen/drivers/video/Makefile index 6c3e5b4..2993c39 100644 --- a/xen/drivers/video/Makefile +++ b/xen/drivers/video/Makefile @@ -1,5 +1,5 @@ -obj-y := vga.o -obj-$(CONFIG_X86) += font_8x14.o -obj-$(CONFIG_X86) += font_8x16.o -obj-$(CONFIG_X86) += font_8x8.o -obj-$(CONFIG_X86) += vesa.o +obj-$(HAS_VGA) := vga.o +obj-$(HAS_VIDEO) += font_8x14.o +obj-$(HAS_VIDEO) += font_8x16.o +obj-$(HAS_VIDEO) += font_8x8.o +obj-$(HAS_VGA) += vesa.o diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index d0a83ff..aaf8b23 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -108,7 +108,7 @@ void __init vesa_init(void) memset(lfb, 0, vram_remap); - vga_puts = vesa_redraw_puts; + video_puts = vesa_redraw_puts; printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " "using %uk, total %uk\n", @@ -193,7 +193,7 @@ void __init vesa_endboot(bool_t keep) if ( keep ) { xpos = 0; - vga_puts = vesa_scroll_puts; + video_puts = vesa_scroll_puts; } else { diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c index a98bd00..40e5963 100644 --- a/xen/drivers/video/vga.c +++ b/xen/drivers/video/vga.c @@ -21,7 +21,7 @@ static unsigned char *video; static void vga_text_puts(const char *s); static void vga_noop_puts(const char *s) {} -void (*vga_puts)(const char *) = vga_noop_puts; +void (*video_puts)(const char *) = vga_noop_puts; /* * ''vga=<mode-specifier>[,keep]'' where <mode-specifier> is one of: @@ -62,7 +62,7 @@ void vesa_endboot(bool_t keep); #define vesa_endboot(x) ((void)0) #endif -void __init vga_init(void) +void __init video_init(void) { char *p; @@ -85,7 +85,7 @@ void __init vga_init(void) columns = vga_console_info.u.text_mode_3.columns; lines = vga_console_info.u.text_mode_3.rows; memset(video, 0, columns * lines * 2); - vga_puts = vga_text_puts; + video_puts = vga_text_puts; break; case XEN_VGATYPE_VESA_LFB: case XEN_VGATYPE_EFI_LFB: @@ -97,16 +97,16 @@ void __init vga_init(void) } } -void __init vga_endboot(void) +void __init video_endboot(void) { - if ( vga_puts == vga_noop_puts ) + if ( video_puts == vga_noop_puts ) return; printk("Xen is %s VGA console.\n", vgacon_keep ? "keeping" : "relinquishing"); if ( !vgacon_keep ) - vga_puts = vga_noop_puts; + video_puts = vga_noop_puts; else { int bus, devfn; diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h index 0c4868c..e8da4f7 100644 --- a/xen/include/asm-x86/config.h +++ b/xen/include/asm-x86/config.h @@ -38,6 +38,7 @@ #define CONFIG_ACPI_CSTATE 1 #define CONFIG_VGA 1 +#define CONFIG_VIDEO 1 #define CONFIG_HOTPLUG 1 #define CONFIG_HOTPLUG_CPU 1 diff --git a/xen/include/xen/vga.h b/xen/include/xen/vga.h index cc690b9..f72b63d 100644 --- a/xen/include/xen/vga.h +++ b/xen/include/xen/vga.h @@ -9,17 +9,10 @@ #ifndef _XEN_VGA_H #define _XEN_VGA_H -#include <public/xen.h> +#include <xen/video.h> #ifdef CONFIG_VGA extern struct xen_vga_console_info vga_console_info; -void vga_init(void); -void vga_endboot(void); -extern void (*vga_puts)(const char *); -#else -#define vga_init() ((void)0) -#define vga_endboot() ((void)0) -#define vga_puts(s) ((void)0) #endif #endif /* _XEN_VGA_H */ diff --git a/xen/include/xen/video.h b/xen/include/xen/video.h new file mode 100644 index 0000000..2e897f9 --- /dev/null +++ b/xen/include/xen/video.h @@ -0,0 +1,24 @@ +/* + * video.h + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of this archive + * for more details. + */ + +#ifndef _XEN_VIDEO_H +#define _XEN_VIDEO_H + +#include <public/xen.h> + +#ifdef CONFIG_VIDEO +void video_init(void); +extern void (*video_puts)(const char *); +void video_endboot(void); +#else +#define video_init() ((void)0) +#define video_puts(s) ((void)0) +#define video_endboot() ((void)0) +#endif + +#endif /* _XEN_VIDEO_H */ -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 3/8] xen: introduce a generic framebuffer driver
Abstract away from vesa.c the funcions to handle a linear framebuffer and print characters to it. The corresponding functions are going to be removed from vesa.c in the next patch. Changes in v3: - rename fb_cr to fb_carriage_return. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/drivers/video/Makefile | 1 + xen/drivers/video/fb.c | 209 ++++++++++++++++++++++++++++++++++++++++++++ xen/drivers/video/fb.h | 49 ++++++++++ 3 files changed, 259 insertions(+), 0 deletions(-) create mode 100644 xen/drivers/video/fb.c create mode 100644 xen/drivers/video/fb.h diff --git a/xen/drivers/video/Makefile b/xen/drivers/video/Makefile index 2993c39..3b3eb43 100644 --- a/xen/drivers/video/Makefile +++ b/xen/drivers/video/Makefile @@ -2,4 +2,5 @@ obj-$(HAS_VGA) := vga.o obj-$(HAS_VIDEO) += font_8x14.o obj-$(HAS_VIDEO) += font_8x16.o obj-$(HAS_VIDEO) += font_8x8.o +obj-$(HAS_VIDEO) += fb.o obj-$(HAS_VGA) += vesa.o diff --git a/xen/drivers/video/fb.c b/xen/drivers/video/fb.c new file mode 100644 index 0000000..a4f0500 --- /dev/null +++ b/xen/drivers/video/fb.c @@ -0,0 +1,209 @@ +/****************************************************************************** + * fb.c + * + * linear frame buffer handling. + */ + +#include <xen/config.h> +#include <xen/kernel.h> +#include <xen/lib.h> +#include <xen/errno.h> +#include "fb.h" +#include "font.h" + +#define MAX_XRES 1900 +#define MAX_YRES 1200 +#define MAX_BPP 4 +#define MAX_FONT_W 8 +#define MAX_FONT_H 16 +static __initdata unsigned int line_len[MAX_XRES / MAX_FONT_W]; +static __initdata unsigned char lbuf[MAX_XRES * MAX_BPP]; +static __initdata unsigned char text_buf[(MAX_XRES / MAX_FONT_W) * \ + (MAX_YRES / MAX_FONT_H)]; + +struct fb_status { + struct fb_prop fbp; + + unsigned char *lbuf, *text_buf; + unsigned int *line_len; + unsigned int xpos, ypos; +}; +static struct fb_status fb; + +static void fb_show_line( + const unsigned char *text_line, + unsigned char *video_line, + unsigned int nr_chars, + unsigned int nr_cells) +{ + unsigned int i, j, b, bpp, pixel; + + bpp = (fb.fbp.bits_per_pixel + 7) >> 3; + + for ( i = 0; i < fb.fbp.font->height; i++ ) + { + unsigned char *ptr = fb.lbuf; + + for ( j = 0; j < nr_chars; j++ ) + { + const unsigned char *bits = fb.fbp.font->data; + bits += ((text_line[j] * fb.fbp.font->height + i) * + ((fb.fbp.font->width + 7) >> 3)); + for ( b = fb.fbp.font->width; b--; ) + { + pixel = (*bits & (1u<<b)) ? fb.fbp.pixel_on : 0; + memcpy(ptr, &pixel, bpp); + ptr += bpp; + } + } + + memset(ptr, 0, (fb.fbp.width - nr_chars * fb.fbp.font->width) * bpp); + memcpy(video_line, fb.lbuf, nr_cells * fb.fbp.font->width * bpp); + video_line += fb.fbp.bytes_per_line; + } +} + +/* Fast mode which redraws all modified parts of a 2D text buffer. */ +void fb_redraw_puts(const char *s) +{ + unsigned int i, min_redraw_y = fb.ypos; + char c; + + /* Paste characters into text buffer. */ + while ( (c = *s++) != ''\0'' ) + { + if ( (c == ''\n'') || (fb.xpos >= fb.fbp.text_columns) ) + { + if ( ++fb.ypos >= fb.fbp.text_rows ) + { + min_redraw_y = 0; + fb.ypos = fb.fbp.text_rows - 1; + memmove(fb.text_buf, fb.text_buf + fb.fbp.text_columns, + fb.ypos * fb.fbp.text_columns); + memset(fb.text_buf + fb.ypos * fb.fbp.text_columns, 0, fb.xpos); + } + fb.xpos = 0; + } + + if ( c != ''\n'' ) + fb.text_buf[fb.xpos++ + fb.ypos * fb.fbp.text_columns] = c; + } + + /* Render modified section of text buffer to VESA linear framebuffer. */ + for ( i = min_redraw_y; i <= fb.ypos; i++ ) + { + const unsigned char *line = fb.text_buf + i * fb.fbp.text_columns; + unsigned int width; + + for ( width = fb.fbp.text_columns; width; --width ) + if ( line[width - 1] ) + break; + fb_show_line(line, + fb.fbp.lfb + i * fb.fbp.font->height * fb.fbp.bytes_per_line, + width, max(fb.line_len[i], width)); + fb.line_len[i] = width; + } + + fb.fbp.flush(); +} + +/* Slower line-based scroll mode which interacts better with dom0. */ +void fb_scroll_puts(const char *s) +{ + unsigned int i; + char c; + + while ( (c = *s++) != ''\0'' ) + { + if ( (c == ''\n'') || (fb.xpos >= fb.fbp.text_columns) ) + { + unsigned int bytes = (fb.fbp.width * + ((fb.fbp.bits_per_pixel + 7) >> 3)); + unsigned char *src = fb.fbp.lfb + fb.fbp.font->height * fb.fbp.bytes_per_line; + unsigned char *dst = fb.fbp.lfb; + + /* New line: scroll all previous rows up one line. */ + for ( i = fb.fbp.font->height; i < fb.fbp.height; i++ ) + { + memcpy(dst, src, bytes); + src += fb.fbp.bytes_per_line; + dst += fb.fbp.bytes_per_line; + } + + /* Render new line. */ + fb_show_line( + fb.text_buf, + fb.fbp.lfb + (fb.fbp.text_rows-1) * fb.fbp.font->height * fb.fbp.bytes_per_line, + fb.xpos, fb.fbp.text_columns); + + fb.xpos = 0; + } + + if ( c != ''\n'' ) + fb.text_buf[fb.xpos++] = c; + } + + fb.fbp.flush(); +} + +void fb_carriage_return(void) +{ + fb.xpos = 0; +} + +int __init fb_init(struct fb_prop fbp) +{ + if ( fbp.width > MAX_XRES || fbp.height > MAX_YRES ) + { + printk("Couldn''t initialize a %xx%x framebuffer early.\n", + fbp.width, fbp.height); + return -EINVAL; + } + + fb.fbp = fbp; + fb.lbuf = lbuf; + fb.text_buf = text_buf; + fb.line_len = line_len; + return 0; +} + +int __init fb_alloc(void) +{ + fb.lbuf = NULL; + fb.text_buf = NULL; + fb.line_len = NULL; + + fb.lbuf = xmalloc_bytes(fb.fbp.bytes_per_line); + if ( !fb.lbuf ) + goto fail; + + fb.text_buf = xzalloc_bytes(fb.fbp.text_columns * fb.fbp.text_rows); + if ( !fb.text_buf ) + goto fail; + + fb.line_len = xzalloc_array(unsigned int, fb.fbp.text_columns); + if ( !fb.line_len ) + goto fail; + + memcpy(fb.lbuf, lbuf, fb.fbp.bytes_per_line); + memcpy(fb.text_buf, text_buf, fb.fbp.text_columns * fb.fbp.text_rows); + memcpy(fb.line_len, line_len, fb.fbp.text_columns); + + return 0; + +fail: + printk(XENLOG_ERR "Couldn''t allocate enough memory to drive " + "the framebuffer\n"); + xfree(fb.lbuf); + xfree(fb.text_buf); + xfree(fb.line_len); + + return -ENOMEM; +} + +void fb_free(void) +{ + xfree(fb.lbuf); + xfree(fb.text_buf); + xfree(fb.line_len); +} diff --git a/xen/drivers/video/fb.h b/xen/drivers/video/fb.h new file mode 100644 index 0000000..0084ffa --- /dev/null +++ b/xen/drivers/video/fb.h @@ -0,0 +1,49 @@ +/* + * xen/drivers/video/fb.h + * + * Cross-platform framebuffer library + * + * Stefano Stabellini <stefano.stabellini@eu.citrix.com> + * Copyright (c) 2012 Citrix Systems. + * + * 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. + */ + +#ifndef _XEN_FB_H +#define _XEN_FB_H + +#include <xen/init.h> + +struct fb_prop { + const struct font_desc *font; + unsigned char *lfb; + unsigned int pixel_on; + uint16_t width, height; + uint16_t bytes_per_line; + uint16_t bits_per_pixel; + void (*flush)(void); + + unsigned int text_columns; + unsigned int text_rows; +}; + +void fb_redraw_puts(const char *s); +void fb_scroll_puts(const char *s); +void fb_carriage_return(void); +void fb_free(void); + +/* initialize the framebuffer, can be called early (before xmalloc is + * available) */ +int __init fb_init(struct fb_prop fbp); +/* fb_alloc allocates internal structures using xmalloc */ +int __init fb_alloc(void); + +#endif -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 4/8] xen/vesa: use the new fb_* functions
Make use of the framebuffer functions previously introduced. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/drivers/video/vesa.c | 179 +++++++--------------------------------------- 1 files changed, 26 insertions(+), 153 deletions(-) diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index aaf8b23..9f24d03 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -13,20 +13,15 @@ #include <asm/io.h> #include <asm/page.h> #include "font.h" +#include "fb.h" #define vlfb_info vga_console_info.u.vesa_lfb -#define text_columns (vlfb_info.width / font->width) -#define text_rows (vlfb_info.height / font->height) -static void vesa_redraw_puts(const char *s); -static void vesa_scroll_puts(const char *s); +static void lfb_flush(void); -static unsigned char *lfb, *lbuf, *text_buf; -static unsigned int *__initdata line_len; +static unsigned char *lfb; static const struct font_desc *font; static bool_t vga_compat; -static unsigned int pixel_on; -static unsigned int xpos, ypos; static unsigned int vram_total; integer_param("vesa-ram", vram_total); @@ -87,29 +82,26 @@ void __init vesa_early_init(void) void __init vesa_init(void) { - if ( !font ) - goto fail; - - lbuf = xmalloc_bytes(vlfb_info.bytes_per_line); - if ( !lbuf ) - goto fail; + struct fb_prop fbp; - text_buf = xzalloc_bytes(text_columns * text_rows); - if ( !text_buf ) - goto fail; + if ( !font ) + return; - line_len = xzalloc_array(unsigned int, text_columns); - if ( !line_len ) - goto fail; + fbp.font = font; + fbp.bits_per_pixel = vlfb_info.bits_per_pixel; + fbp.bytes_per_line = vlfb_info.bytes_per_line; + fbp.width = vlfb_info.width; + fbp.height = vlfb_info.height; + fbp.flush = lfb_flush; + fbp.text_columns = vlfb_info.width / font->width; + fbp.text_rows = vlfb_info.height / font->height; - lfb = ioremap(vlfb_info.lfb_base, vram_remap); + fbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap); if ( !lfb ) - goto fail; + return; memset(lfb, 0, vram_remap); - video_puts = vesa_redraw_puts; - printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " "using %uk, total %uk\n", vlfb_info.lfb_base, lfb, @@ -131,7 +123,7 @@ void __init vesa_init(void) { /* Light grey in truecolor. */ unsigned int grey = 0xaaaaaaaa; - pixel_on = + fbp.pixel_on = ((grey >> (32 - vlfb_info. red_size)) << vlfb_info. red_pos) | ((grey >> (32 - vlfb_info.green_size)) << vlfb_info.green_pos) | ((grey >> (32 - vlfb_info. blue_size)) << vlfb_info. blue_pos); @@ -139,15 +131,14 @@ void __init vesa_init(void) else { /* White(ish) in default pseudocolor palette. */ - pixel_on = 7; + fbp.pixel_on = 7; } - return; - - fail: - xfree(lbuf); - xfree(text_buf); - xfree(line_len); + if ( fb_init(fbp) < 0 ) + return; + if ( fb_alloc() < 0 ) + return; + video_puts = fb_redraw_puts; } #include <asm/mtrr.h> @@ -192,8 +183,8 @@ void __init vesa_endboot(bool_t keep) { if ( keep ) { - xpos = 0; - video_puts = vesa_scroll_puts; + video_puts = fb_scroll_puts; + fb_carriage_return(); } else { @@ -202,124 +193,6 @@ void __init vesa_endboot(bool_t keep) memset(lfb + i * vlfb_info.bytes_per_line, 0, vlfb_info.width * bpp); lfb_flush(); + fb_free(); } - - xfree(line_len); -} - -/* Render one line of text to given linear framebuffer line. */ -static void vesa_show_line( - const unsigned char *text_line, - unsigned char *video_line, - unsigned int nr_chars, - unsigned int nr_cells) -{ - unsigned int i, j, b, bpp, pixel; - - bpp = (vlfb_info.bits_per_pixel + 7) >> 3; - - for ( i = 0; i < font->height; i++ ) - { - unsigned char *ptr = lbuf; - - for ( j = 0; j < nr_chars; j++ ) - { - const unsigned char *bits = font->data; - bits += ((text_line[j] * font->height + i) * - ((font->width + 7) >> 3)); - for ( b = font->width; b--; ) - { - pixel = (*bits & (1u<<b)) ? pixel_on : 0; - memcpy(ptr, &pixel, bpp); - ptr += bpp; - } - } - - memset(ptr, 0, (vlfb_info.width - nr_chars * font->width) * bpp); - memcpy(video_line, lbuf, nr_cells * font->width * bpp); - video_line += vlfb_info.bytes_per_line; - } -} - -/* Fast mode which redraws all modified parts of a 2D text buffer. */ -static void __init vesa_redraw_puts(const char *s) -{ - unsigned int i, min_redraw_y = ypos; - char c; - - /* Paste characters into text buffer. */ - while ( (c = *s++) != ''\0'' ) - { - if ( (c == ''\n'') || (xpos >= text_columns) ) - { - if ( ++ypos >= text_rows ) - { - min_redraw_y = 0; - ypos = text_rows - 1; - memmove(text_buf, text_buf + text_columns, - ypos * text_columns); - memset(text_buf + ypos * text_columns, 0, xpos); - } - xpos = 0; - } - - if ( c != ''\n'' ) - text_buf[xpos++ + ypos * text_columns] = c; - } - - /* Render modified section of text buffer to VESA linear framebuffer. */ - for ( i = min_redraw_y; i <= ypos; i++ ) - { - const unsigned char *line = text_buf + i * text_columns; - unsigned int width; - - for ( width = text_columns; width; --width ) - if ( line[width - 1] ) - break; - vesa_show_line(line, - lfb + i * font->height * vlfb_info.bytes_per_line, - width, max(line_len[i], width)); - line_len[i] = width; - } - - lfb_flush(); -} - -/* Slower line-based scroll mode which interacts better with dom0. */ -static void vesa_scroll_puts(const char *s) -{ - unsigned int i; - char c; - - while ( (c = *s++) != ''\0'' ) - { - if ( (c == ''\n'') || (xpos >= text_columns) ) - { - unsigned int bytes = (vlfb_info.width * - ((vlfb_info.bits_per_pixel + 7) >> 3)); - unsigned char *src = lfb + font->height * vlfb_info.bytes_per_line; - unsigned char *dst = lfb; - - /* New line: scroll all previous rows up one line. */ - for ( i = font->height; i < vlfb_info.height; i++ ) - { - memcpy(dst, src, bytes); - src += vlfb_info.bytes_per_line; - dst += vlfb_info.bytes_per_line; - } - - /* Render new line. */ - vesa_show_line( - text_buf, - lfb + (text_rows-1) * font->height * vlfb_info.bytes_per_line, - xpos, text_columns); - - xpos = 0; - } - - if ( c != ''\n'' ) - text_buf[xpos++] = c; - } - - lfb_flush(); } -- 1.7.2.5
At the moment we destroy the DTB mappings we have in setup_pagetables and we don''t restore them until setup_mm. Keep the temporary DTB mapping until we create the new ones. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/kernel.h | 2 ++ xen/arch/arm/mm.c | 12 ++++++++++++ xen/arch/arm/setup.c | 1 + xen/include/asm-arm/mm.h | 2 ++ 4 files changed, 17 insertions(+), 0 deletions(-) diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h index 4533568..a179ffb 100644 --- a/xen/arch/arm/kernel.h +++ b/xen/arch/arm/kernel.h @@ -38,4 +38,6 @@ struct kernel_info { int kernel_prepare(struct kernel_info *info); void kernel_load(struct kernel_info *info); +extern char _sdtb[]; + #endif /* #ifdef __ARCH_ARM_KERNEL_H__ */ diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 0d7a163..2410794 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -34,6 +34,7 @@ #include <asm/current.h> #include <public/memory.h> #include <xen/sched.h> +#include "kernel.h" struct domain *dom_xen, *dom_io; @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ + /* preserve the DTB mapping a little while longer */ + pte = mfn_to_xen_entry(((unsigned long) _sdtb + boot_phys_offset) >> PAGE_SHIFT); + write_pte(xen_second + second_linear_offset(BOOT_MISC_VIRT_START), pte); + /* From now on, no mapping may be both writable and executable. */ WRITE_CP32(READ_CP32(HSCTLR) | SCTLR_WXN, HSCTLR); /* Flush everything after setting WXN bit. */ flush_xen_text_tlb(); } +void __init destroy_dtb_mapping(void) +{ + /* destroy old DTB mapping */ + xen_second[second_linear_offset(BOOT_MISC_VIRT_START)].bits = 0; + dsb(); +} + /* MMU setup for secondary CPUS (which already have paging enabled) */ void __cpuinit mmu_init_secondary_cpu(void) { diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 2076724..06a878f 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -229,6 +229,7 @@ void __init start_xen(unsigned long boot_phys_offset, init_xen_time(); + destroy_dtb_mapping(); setup_mm(atag_paddr, fdt_size); /* Setup Hyp vector base */ diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index 4ed5df6..6fa4308 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -139,6 +139,8 @@ extern unsigned long total_pages; /* Boot-time pagetable setup */ extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr); +/* Destroy temporary DTB mapping */ +extern void destroy_dtb_mapping(void); /* MMU setup for seccondary CPUS (which already have paging enabled) */ extern void __cpuinit mmu_init_secondary_cpu(void); /* Set up the xenheap: up to 1GB of contiguous, always-mapped memory. -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 6/8] xen/device_tree: introduce find_compatible_node
Introduce a find_compatible_node function that can be used by device drivers to find the node corresponding to their device in the device tree. Initialize device_tree_flattened early in start_xen, so that it is available before setup_mm. Get rid of fdt in the process. Also add device_tree_node_compatible to device_tree.h, that is currently missing. Changes in v2: - remove fdt; - return early from _find_compatible_node, if a node has already been found. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/setup.c | 7 ++--- xen/common/device_tree.c | 51 +++++++++++++++++++++++++++++++++++++++++ xen/include/xen/device_tree.h | 3 ++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 06a878f..2c7ee5a 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -196,7 +196,6 @@ void __init start_xen(unsigned long boot_phys_offset, unsigned long atag_paddr, unsigned long cpuid) { - void *fdt; size_t fdt_size; int cpus, i; @@ -204,12 +203,12 @@ void __init start_xen(unsigned long boot_phys_offset, smp_clear_cpu_maps(); - fdt = (void *)BOOT_MISC_VIRT_START + device_tree_flattened = (void *)BOOT_MISC_VIRT_START + (atag_paddr & ((1 << SECOND_SHIFT) - 1)); - fdt_size = device_tree_early_init(fdt); + fdt_size = device_tree_early_init(device_tree_flattened); cpus = smp_get_max_cpus(); - cmdline_parse(device_tree_bootargs(fdt)); + cmdline_parse(device_tree_bootargs(device_tree_flattened)); setup_pagetables(boot_phys_offset, get_xen_paddr()); diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 8b4ef2f..d4391f8 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -172,6 +172,57 @@ int device_tree_for_each_node(const void *fdt, return 0; } +struct find_compat { + const char *compatible; + int found; + int node; + int depth; + u32 address_cells; + u32 size_cells; +}; + +static int _find_compatible_node(const void *fdt, + int node, const char *name, int depth, + u32 address_cells, u32 size_cells, + void *data) +{ + struct find_compat *c = (struct find_compat *) data; + + if ( c->found ) + return 0; + + if ( device_tree_node_compatible(fdt, node, c->compatible) ) + { + c->found = 1; + c->node = node; + c->depth = depth; + c->address_cells = address_cells; + c->size_cells = size_cells; + } + return 0; +} + +int find_compatible_node(const char *compatible, int *node, int *depth, + u32 *address_cells, u32 *size_cells) +{ + int ret; + struct find_compat c; + c.compatible = compatible; + c.found = 0; + + ret = device_tree_for_each_node(device_tree_flattened, _find_compatible_node, &c); + if ( !c.found ) + return ret; + else + { + *node = c.node; + *depth = c.depth; + *address_cells = c.address_cells; + *size_cells = c.size_cells; + return 1; + } +} + /** * device_tree_bootargs - return the bootargs (the Xen command line) * @fdt flat device tree. diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index a0e3a97..5a75f0e 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -54,6 +54,9 @@ void device_tree_set_reg(u32 **cell, u32 address_cells, u32 size_cells, u64 start, u64 size); u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name); bool_t device_tree_node_matches(const void *fdt, int node, const char *match); +bool_t device_tree_node_compatible(const void *fdt, int node, const char *match); +int find_compatible_node(const char *compatible, int *node, int *depth, + u32 *address_cells, u32 *size_cells); int device_tree_for_each_node(const void *fdt, device_tree_node_func func, void *data); const char *device_tree_bootargs(const void *fdt); -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 7/8] xen/arm: introduce vexpress_syscfg
Introduce a Versatile Express specific function to read/write motherboard settings. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/Makefile | 1 + xen/arch/arm/platform_vexpress.c | 97 +++++++++++++++++++++++++++++++ xen/include/asm-arm/platform_vexpress.h | 23 +++++++ 3 files changed, 121 insertions(+), 0 deletions(-) create mode 100644 xen/arch/arm/platform_vexpress.c diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 4c61b04..24689c5 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -18,6 +18,7 @@ obj-y += p2m.o obj-y += percpu.o obj-y += guestcopy.o obj-y += physdev.o +obj-y += platform_vexpress.o obj-y += setup.o obj-y += time.o obj-y += smpboot.o diff --git a/xen/arch/arm/platform_vexpress.c b/xen/arch/arm/platform_vexpress.c new file mode 100644 index 0000000..41e3806 --- /dev/null +++ b/xen/arch/arm/platform_vexpress.c @@ -0,0 +1,97 @@ +/* + * xen/arch/arm/platform_vexpress.c + * + * Versatile Express specific settings + * + * Stefano Stabellini <stefano.stabellini@eu.citrix.com> + * Copyright (c) 2012 Citrix Systems. + * + * 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/platform_vexpress.h> +#include <xen/mm.h> + +#define DCC_SHIFT 26 +#define FUNCTION_SHIFT 20 +#define SITE_SHIFT 16 +#define POSITION_SHIFT 12 +#define DEVICE_SHIFT 0 + +int vexpress_syscfg(int write, int function, int device, uint32_t *data) +{ + uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC); + uint32_t stat; + int dcc = 0; /* DCC to access */ + int site = 0; /* motherboard */ + int position = 0; /* motherboard */ + + set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED); + + if ( syscfg[V2M_SYS_CFGCTRL] & V2M_SYS_CFG_START ) + return -1; + + /* clear the complete bit in the V2M_SYS_CFGSTAT status register */ + syscfg[V2M_SYS_CFGSTAT] = 0; + + if ( write ) + { + /* write data */ + syscfg[V2M_SYS_CFGDATA] = *data; + + /* set control register */ + syscfg[V2M_SYS_CFGCTRL] = V2M_SYS_CFG_START | V2M_SYS_CFG_WRITE | + (dcc << DCC_SHIFT) | (function << FUNCTION_SHIFT) | + (site << SITE_SHIFT) | (position << POSITION_SHIFT) | + (device << DEVICE_SHIFT); + + /* wait for complete flag to be set */ + do { + stat = syscfg[V2M_SYS_CFGSTAT]; + dsb(); + } while ( !(stat & V2M_SYS_CFG_COMPLETE) ); + + /* check error status and return error flag if set */ + if ( stat & V2M_SYS_CFG_ERROR ) + return -1; + } else { + /* set control register */ + syscfg[V2M_SYS_CFGCTRL] = V2M_SYS_CFG_START | (dcc << DCC_SHIFT) | + (function << FUNCTION_SHIFT) | (site << SITE_SHIFT) | + (position << POSITION_SHIFT) | (device << DEVICE_SHIFT); + + /* wait for complete flag to be set */ + do { + stat = syscfg[V2M_SYS_CFGSTAT]; + dsb(); + } while ( !(stat & V2M_SYS_CFG_COMPLETE) ); + + /* check error status flag and return error flag if set */ + if ( stat & V2M_SYS_CFG_ERROR ) + return -1; + else + /* read data */ + *data = syscfg[V2M_SYS_CFGDATA]; + } + + clear_fixmap(FIXMAP_MISC); + + return 0; +} + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/asm-arm/platform_vexpress.h b/xen/include/asm-arm/platform_vexpress.h index 3556af3..407602d 100644 --- a/xen/include/asm-arm/platform_vexpress.h +++ b/xen/include/asm-arm/platform_vexpress.h @@ -6,6 +6,29 @@ #define V2M_SYS_FLAGSSET (0x30) #define V2M_SYS_FLAGSCLR (0x34) +#define V2M_SYS_CFGDATA (0x00A0/4) +#define V2M_SYS_CFGCTRL (0x00A4/4) +#define V2M_SYS_CFGSTAT (0x00A8/4) + +#define V2M_SYS_CFG_START (1<<31) +#define V2M_SYS_CFG_WRITE (1<<30) +#define V2M_SYS_CFG_ERROR (1<<1) +#define V2M_SYS_CFG_COMPLETE (1<<0) + +#define V2M_SYS_CFG_OSC_FUNC 1 +#define V2M_SYS_CFG_OSC0 0 +#define V2M_SYS_CFG_OSC1 1 +#define V2M_SYS_CFG_OSC2 2 +#define V2M_SYS_CFG_OSC3 3 +#define V2M_SYS_CFG_OSC4 4 +#define V2M_SYS_CFG_OSC5 5 + +#ifndef __ASSEMBLY__ +#include <xen/inttypes.h> + +int vexpress_syscfg(int write, int function, int device, uint32_t *data); +#endif + #endif /* __ASM_ARM_PLATFORM_H */ /* * Local variables: -- 1.7.2.5
Stefano Stabellini
2012-Dec-18 18:46 UTC
[PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller
Read the screen resolution setting from device tree, find the corresponding modeline in a small table of standard video modes, set the hardware accordingly. Use vexpress_syscfg to configure the pixel clock. Use the generic framebuffer functions to print on the screen. Changes in v2: - read mode from DT; - support multiple resolutions; - use vexpress_syscfg to set the pixclock. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/Rules.mk | 1 + xen/drivers/video/Makefile | 1 + xen/drivers/video/arm_hdlcd.c | 282 +++++++++++++++++++++++++++++++++++++++++ xen/drivers/video/modelines.h | 69 ++++++++++ xen/include/asm-arm/config.h | 2 + 5 files changed, 355 insertions(+), 0 deletions(-) create mode 100644 xen/drivers/video/arm_hdlcd.c create mode 100644 xen/drivers/video/modelines.h diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk index fa9f9c1..9580e6b 100644 --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -8,6 +8,7 @@ HAS_DEVICE_TREE := y HAS_VIDEO := y +HAS_ARM_HDLCD := y CFLAGS += -fno-builtin -fno-common -Wredundant-decls CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe diff --git a/xen/drivers/video/Makefile b/xen/drivers/video/Makefile index 3b3eb43..8a6f5da 100644 --- a/xen/drivers/video/Makefile +++ b/xen/drivers/video/Makefile @@ -4,3 +4,4 @@ obj-$(HAS_VIDEO) += font_8x16.o obj-$(HAS_VIDEO) += font_8x8.o obj-$(HAS_VIDEO) += fb.o obj-$(HAS_VGA) += vesa.o +obj-$(HAS_ARM_HDLCD) += arm_hdlcd.o diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c new file mode 100644 index 0000000..9e69856 --- /dev/null +++ b/xen/drivers/video/arm_hdlcd.c @@ -0,0 +1,282 @@ +/* + * xen/drivers/video/arm_hdlcd.c + * + * Driver for ARM HDLCD Controller + * + * Stefano Stabellini <stefano.stabellini@eu.citrix.com> + * Copyright (c) 2012 Citrix Systems. + * + * 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/delay.h> +#include <asm/types.h> +#include <asm/platform_vexpress.h> +#include <xen/config.h> +#include <xen/device_tree.h> +#include <xen/libfdt/libfdt.h> +#include <xen/init.h> +#include <xen/mm.h> +#include "font.h" +#include "fb.h" +#include "modelines.h" + +#define HDLCD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_MISC)) + +#define HDLCD_INTMASK (0x18/4) +#define HDLCD_FBBASE (0x100/4) +#define HDLCD_LINELENGTH (0x104/4) +#define HDLCD_LINECOUNT (0x108/4) +#define HDLCD_LINEPITCH (0x10C/4) +#define HDLCD_BUS (0x110/4) +#define HDLCD_VSYNC (0x200/4) +#define HDLCD_VBACK (0x204/4) +#define HDLCD_VDATA (0x208/4) +#define HDLCD_VFRONT (0x20C/4) +#define HDLCD_HSYNC (0x210/4) +#define HDLCD_HBACK (0x214/4) +#define HDLCD_HDATA (0x218/4) +#define HDLCD_HFRONT (0x21C/4) +#define HDLCD_POLARITIES (0x220/4) +#define HDLCD_COMMAND (0x230/4) +#define HDLCD_PF (0x240/4) +#define HDLCD_RED (0x244/4) +#define HDLCD_GREEN (0x248/4) +#define HDLCD_BLUE (0x24C/4) + +static void vga_noop_puts(const char *s) {} +void (*video_puts)(const char *) = vga_noop_puts; + +static void hdlcd_flush(void) +{ + dsb(); +} + +static void set_color_masks(int bpp, + int *red_shift, int *green_shift, int *blue_shift, + int *red_size, int *green_size, int *blue_size) +{ + switch (bpp) { + case 2: + *red_shift = 0; + *green_shift = 5; + *blue_shift = 11; + *red_size = 5; + *green_size = 6; + *blue_size = 5; + break; + case 3: + case 4: + *red_shift = 0; + *green_shift = 8; + *blue_shift = 16; + *red_size = 8; + *green_size = 8; + *blue_size = 8; + break; + default: + BUG(); + break; + } +} + +static void set_pixclock(uint32_t pixclock) +{ + vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock); +} + +void __init video_init(void) +{ + int node, depth; + u32 address_cells, size_cells; + struct fb_prop fbp; + unsigned char *lfb; + paddr_t hdlcd_start, hdlcd_size; + paddr_t framebuffer_start, framebuffer_size; + const struct fdt_property *prop; + const u32 *cell; + const char *mode_string; + char _mode_string[16]; + int bpp; + int red_shift, green_shift, blue_shift; + int red_size, green_size, blue_size; + struct modeline *videomode = NULL; + int i; + + if ( find_compatible_node("arm,hdlcd", &node, &depth, + &address_cells, &size_cells) <= 0 ) + return; + + prop = fdt_get_property(device_tree_flattened, node, "reg", NULL); + if ( !prop ) + return; + + cell = (const u32 *)prop->data; + device_tree_get_reg(&cell, address_cells, size_cells, + &hdlcd_start, &hdlcd_size); + + prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL); + if ( !prop ) + return; + + cell = (const u32 *)prop->data; + device_tree_get_reg(&cell, address_cells, size_cells, + &framebuffer_start, &framebuffer_size); + + mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL); + if ( !mode_string ) + { + bpp = 4; + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, + &red_size, &green_size, &blue_size); + memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60")); + } + if ( strlen(mode_string) < strlen("800x600@60") ) + { + printk("HDLCD: invalid modeline=%s\n", mode_string); + return; + } else { + char *s = strchr(mode_string, ''-''); + if ( !s ) + { + printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n", + mode_string); + bpp = 4; + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, + &red_size, &green_size, &blue_size); + memcpy(_mode_string, mode_string, strlen(mode_string) + 1); + } else { + if ( strlen(s) < 6 ) + { + printk("HDLCD: invalid mode %s\n", mode_string); + return; + } + s++; + if ( !strncmp(s, "16", 2) ) + { + bpp = 2; + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, + &red_size, &green_size, &blue_size); + } + else if ( !strncmp(s, "24", 2) ) + { + bpp = 3; + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, + &red_size, &green_size, &blue_size); + } + else if ( !strncmp(s, "32", 2) ) + { + bpp = 4; + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, + &red_size, &green_size, &blue_size); + } else { + printk("HDLCD: unsupported bpp %s\n", s); + return; + } + i = s - mode_string - 1; + memcpy(_mode_string, mode_string, i); + memcpy(_mode_string + i, mode_string + i + 3, 4); + } + } + + for ( i = 0; i < ARRAY_SIZE(videomodes); i++ ) + { + if ( !strcmp(_mode_string, videomodes[i].mode) ) + { + videomode = &videomodes[i]; + break; + } + } + if ( !videomode ) + { + printk("HDLCD: unsupported videomode %s\n", _mode_string); + return; + } + + + if ( !hdlcd_start || !framebuffer_start ) + return; + + if ( framebuffer_size < bpp * videomode->xres * videomode->yres ) + { + printk("HDLCD: the framebuffer is too small, disable the HDLCD driver\n"); + return; + } + + printk("Initializing HDLCD driver\n"); + + lfb = early_ioremap(framebuffer_start, framebuffer_size, DEV_WC); + if ( !lfb ) + { + printk("Couldn''t map the framebuffer\n"); + return; + } + memset(lfb, 0x00, bpp * videomode->xres * videomode->yres); + + /* uses FIXMAP_MISC */ + set_pixclock(videomode->pixclock); + + set_fixmap(FIXMAP_MISC, hdlcd_start >> PAGE_SHIFT, DEV_SHARED); + HDLCD[HDLCD_COMMAND] = 0; + + HDLCD[HDLCD_LINELENGTH] = videomode->xres * bpp; + HDLCD[HDLCD_LINECOUNT] = videomode->yres - 1; + HDLCD[HDLCD_LINEPITCH] = videomode->xres * bpp; + HDLCD[HDLCD_PF] = ((bpp - 1) << 3); + HDLCD[HDLCD_INTMASK] = 0; + HDLCD[HDLCD_FBBASE] = framebuffer_start; + HDLCD[HDLCD_BUS] = 0xf00 | (1 << 4); + HDLCD[HDLCD_VBACK] = videomode->vback - 1; + HDLCD[HDLCD_VSYNC] = videomode->vsync - 1; + HDLCD[HDLCD_VDATA] = videomode->yres - 1; + HDLCD[HDLCD_VFRONT] = videomode->vfront - 1; + HDLCD[HDLCD_HBACK] = videomode->hback - 1; + HDLCD[HDLCD_HSYNC] = videomode->hsync - 1; + HDLCD[HDLCD_HDATA] = videomode->xres - 1; + HDLCD[HDLCD_HFRONT] = videomode->hfront - 1; + HDLCD[HDLCD_POLARITIES] = (1 << 2) | (1 << 3); + HDLCD[HDLCD_RED] = (red_size << 8) | red_shift; + HDLCD[HDLCD_GREEN] = (green_size << 8) | green_shift; + HDLCD[HDLCD_BLUE] = (blue_size << 8) | blue_shift; + HDLCD[HDLCD_COMMAND] = 1; + clear_fixmap(FIXMAP_MISC); + + fbp.pixel_on = (((1 << red_size) - 1) << red_shift) | + (((1 << green_size) - 1) << green_shift) | + (((1 << blue_size) - 1) << blue_shift); + fbp.lfb = lfb; + fbp.font = &font_vga_8x16; + fbp.bits_per_pixel = bpp*8; + fbp.bytes_per_line = bpp*videomode->xres; + fbp.width = videomode->xres; + fbp.height = videomode->yres; + fbp.flush = hdlcd_flush; + fbp.text_columns = videomode->xres / 8; + fbp.text_rows = videomode->yres / 16; + if ( fb_init(fbp) < 0 ) + return; + video_puts = fb_scroll_puts; +} + +void video_endboot(void) +{ + if ( video_puts != vga_noop_puts ) + fb_alloc(); +} + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h new file mode 100644 index 0000000..b91368d --- /dev/null +++ b/xen/drivers/video/modelines.h @@ -0,0 +1,69 @@ +/* + * xen/drivers/video/modelines.h + * + * Timings for many popular monitor resolutions + * + * Stefano Stabellini <stefano.stabellini@eu.citrix.com> + * Copyright (c) 2012 Citrix Systems. + * + * 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. + */ + +#ifndef _XEN_MODLINES_H +#define _XEN_MODLINES_H + +struct modeline { + const char* mode; /* in the form 1280x1024@60 */ + uint32_t pixclock; /* Khz */ + uint32_t xres; + uint32_t hfront; /* horizontal front porch in pixels */ + uint32_t hsync; /* horizontal sync pulse in pixels */ + uint32_t hback; /* horizontal back porch in pixels */ + uint32_t yres; + uint32_t vfront; /* vertical front porch in lines */ + uint32_t vsync; /* vertical sync pulse in lines */ + uint32_t vback; /* vertical back porch in lines */ +}; + +struct modeline __initdata videomodes[] = { + { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, + { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, + { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, + { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, + { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, + { "800x600@60", 40000, 800, 40, 128, 88 , 600, 1, 4, 23 }, + { "800x600@72", 50000, 800, 56, 120, 64 , 600, 37, 6, 23 }, + { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, + { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, + { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, + { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, + { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, + { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, + { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, + { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, + { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, + { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, + { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, + { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, + { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, + { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, + { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, + { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, + { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, + { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, + { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, + { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 }, + { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, + { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, + { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, +}; + +#endif diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 87db0d1..d8aa66b 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -19,6 +19,8 @@ #define CONFIG_DOMAIN_PAGE 1 +#define CONFIG_VIDEO 1 + #define OPT_CONSOLE_STR "com1" #ifdef MAX_PHYS_CPUS -- 1.7.2.5
Jan Beulich
2012-Dec-19 08:08 UTC
Re: [PATCH v3 3/8] xen: introduce a generic framebuffer driver
>>> On 18.12.12 at 19:46, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > +int __init fb_init(struct fb_prop fbp)Any reason to pass a structure by value here?> +{ > + if ( fbp.width > MAX_XRES || fbp.height > MAX_YRES ) > + { > + printk("Couldn''t initialize a %xx%x framebuffer early.\n",%x to me seems to produce rather meaningless messages here; customarily screen dimensions are expected to be decimal. Also, in new code you should specify a log level.> + fbp.width, fbp.height); > + return -EINVAL; > + } > + > + fb.fbp = fbp; > + fb.lbuf = lbuf; > + fb.text_buf = text_buf; > + fb.line_len = line_len; > + return 0; > +} > + > +int __init fb_alloc(void) > +{ > + fb.lbuf = NULL; > + fb.text_buf = NULL; > + fb.line_len = NULL; > + > + fb.lbuf = xmalloc_bytes(fb.fbp.bytes_per_line); > + if ( !fb.lbuf ) > + goto fail; > + > + fb.text_buf = xzalloc_bytes(fb.fbp.text_columns * fb.fbp.text_rows); > + if ( !fb.text_buf ) > + goto fail; > + > + fb.line_len = xzalloc_array(unsigned int, fb.fbp.text_columns); > + if ( !fb.line_len ) > + goto fail; > + > + memcpy(fb.lbuf, lbuf, fb.fbp.bytes_per_line); > + memcpy(fb.text_buf, text_buf, fb.fbp.text_columns * fb.fbp.text_rows); > + memcpy(fb.line_len, line_len, fb.fbp.text_columns); > + > + return 0; > + > +fail: > + printk(XENLOG_ERR "Couldn''t allocate enough memory to drive " > + "the framebuffer\n");I think it was generally agreed that breaking up messages to fit 80 columns is undesirable.> + xfree(fb.lbuf); > + xfree(fb.text_buf); > + xfree(fb.line_len);fb_free()?> + > + return -ENOMEM; > +} > + > +void fb_free(void) > +{ > + xfree(fb.lbuf); > + xfree(fb.text_buf); > + xfree(fb.line_len); > +} > --- /dev/null > +++ b/xen/drivers/video/fb.h > @@ -0,0 +1,49 @@ > +/* > + * xen/drivers/video/fb.h > + * > + * Cross-platform framebuffer library > + * > + * Stefano Stabellini <stefano.stabellini@eu.citrix.com> > + * Copyright (c) 2012 Citrix Systems. > + * > + * 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. > + */ > + > +#ifndef _XEN_FB_H > +#define _XEN_FB_H > + > +#include <xen/init.h> > + > +struct fb_prop { > + const struct font_desc *font; > + unsigned char *lfb; > + unsigned int pixel_on; > + uint16_t width, height; > + uint16_t bytes_per_line; > + uint16_t bits_per_pixel; > + void (*flush)(void); > + > + unsigned int text_columns; > + unsigned int text_rows; > +}; > + > +void fb_redraw_puts(const char *s); > +void fb_scroll_puts(const char *s); > +void fb_carriage_return(void); > +void fb_free(void); > + > +/* initialize the framebuffer, can be called early (before xmalloc is > + * available) */ > +int __init fb_init(struct fb_prop fbp); > +/* fb_alloc allocates internal structures using xmalloc */ > +int __init fb_alloc(void);No __init annotations on declarations, please. Jan> + > +#endif
Jan Beulich
2012-Dec-19 08:13 UTC
Re: [PATCH v3 3/8] xen: introduce a generic framebuffer driver
>>> On 18.12.12 at 19:46, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > Abstract away from vesa.c the funcions to handle a linear framebuffer > and print characters to it. > The corresponding functions are going to be removed from vesa.c in the > next patch. > > Changes in v3: > - rename fb_cr to fb_carriage_return. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Oh, and (sorry for not noticing this earlier) as this is about _linear_ frame buffers only, naming the types, functions, and variables lfb_* rather than fb_* seems very desirable to me. Jan
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:> Introduce a function to map a range of physical memory into Xen virtual > memory. > It doesn''t need domheap to be setup. > It is going to be used to map the videoram. > > Add flush_xen_data_tlb_range, that flushes a range of virtual addresses. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/mm.c | 32 ++++++++++++++++++++++++++++++++ > xen/include/asm-arm/config.h | 2 ++ > xen/include/asm-arm/mm.h | 3 ++- > xen/include/asm-arm/page.h | 23 +++++++++++++++++++++++ > 4 files changed, 59 insertions(+), 1 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 855f83d..0d7a163 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -367,6 +367,38 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe) > frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pages * sizeof(struct page_info)); > } > > +/* Map the physical memory range start - start + len into virtual > + * memory and return the virtual address of the mapping. > + * start has to be 2MB aligned. > + * len has to be < EARLY_VMAP_END - EARLY_VMAP_START. > + */ > +void* early_ioremap(paddr_t start, size_t len, unsigned attributes)Should be __init I think, to discourage its use later on. If when we need that they proper vmap+ioremap should be implemented.> +{ > + static unsigned long virt_start = EARLY_VMAP_START;Is the idea that if/when we implement proper vmap + ioremap support we should initialise the vmap area with EARLY_VMAP_START..virt_start already assigned and virt_start..EARLY_VMAP_END free (removing all the EARLY_ I guess)? At some point I suppose we will need to integrate this with xen/common/vmap.c.> + void* ret_addr = (void *)virt_start; > + paddr_t end = start + len; > + > + ASSERT(!(start & (~SECOND_MASK))); > + ASSERT(!(virt_start & (~SECOND_MASK))); > + > + /* The range we need to map is too big */ > + if ( virt_start + len >= EARLY_VMAP_END ) > + return NULL; > + > + while ( start < end ) > + { > + lpae_t e = mfn_to_xen_entry(start >> PAGE_SHIFT); > + e.pt.ai = attributes; > + write_pte(xen_second + second_table_offset(virt_start), e); > + > + start += SECOND_SIZE; > + virt_start += SECOND_SIZE; > + } > + flush_xen_data_tlb_range((unsigned long) ret_addr, len);Just cast this in the return? At the moment you cast to void* only to cast it back to unsigned long.> + > + return ret_addr; > +} > + > enum mg { mg_clear, mg_ro, mg_rw, mg_rx }; > static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) > { > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index 2a05539..87db0d1 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -73,9 +73,11 @@ > #define FIXMAP_ADDR(n) (mk_unsigned_long(0x00400000) + (n) * PAGE_SIZE) > #define BOOT_MISC_VIRT_START mk_unsigned_long(0x00600000) > #define FRAMETABLE_VIRT_START mk_unsigned_long(0x02000000) > +#define EARLY_VMAP_START mk_unsigned_long(0x10000000)There is a comment right above here which describes Xen virtual memory layout, this needs updating as Tim requested before. Also the convention is FOO_VIRT_START.> #define XENHEAP_VIRT_START mk_unsigned_long(0x40000000) > #define DOMHEAP_VIRT_START mk_unsigned_long(0x80000000) > > +#define EARLY_VMAP_END XENHEAP_VIRT_START > #define HYPERVISOR_VIRT_START XEN_VIRT_START > > #define DOMHEAP_ENTRIES 1024 /* 1024 2MB mapping slots */ > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index e95ece1..4ed5df6 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -150,7 +150,8 @@ extern void setup_frametable_mappings(paddr_t ps, paddr_t pe); > extern void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes); > /* Remove a mapping from a fixmap entry */ > extern void clear_fixmap(unsigned map); > - > +/* map a 2MB aligned physical range in virtual memory. */ > +void* early_ioremap(paddr_t start, size_t len, unsigned attributes); > > #define mfn_valid(mfn) ({ \ > unsigned long __m_f_n = (mfn); \ > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > index d89261e..0790dda 100644 > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -328,6 +328,23 @@ static inline void flush_xen_data_tlb_va(unsigned long va) > : : "r" (va) : "memory"); > } > > +/* > + * Flush a range of VA''s hypervisor mappings from the data TLB. This is not > + * sufficient when changing code mappings or for self modifying code. > + */ > +static inline void flush_xen_data_tlb_range(unsigned long va, unsigned long size) > +{ > + unsigned long end = va + size; > + while ( va < end ) { > + asm volatile("dsb;" /* Ensure preceding are visible */Either this dsb should be on the next line (aligned with the following lines) or the following lines should be indented further to match (we have both styles in this file, but lets not have a 3rd). Although it would probably be better to just call flush_xen_data_tlb_va here? Function should be flush_xen_data_tlb_range_va I think. I''m not sure it is worth a new function though, perhaps just add size to the existing flush_xen_data_tlb_va, there''s not too many callers? While I was grepping I noticed flush_xen_data_tlb_va(dest_va) in setup_pagetables(). dest_va has just been setup with a second level (2M mapping). Is it not a bit dodgy to only flush the first 4K of that?> + STORE_CP32(0, TLBIMVAH) > + "dsb;" /* Ensure completion of the TLB flush */ > + "isb;" > + : : "r" (va) : "memory"); > + va += PAGE_SIZE; > + } > +} > + > /* Flush all non-hypervisor mappings from the TLB */ > static inline void flush_guest_tlb(void) > { > @@ -418,8 +435,14 @@ static inline uint64_t gva_to_ipa(uint32_t va) > #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1) > > #define THIRD_SHIFT PAGE_SHIFT > +#define THIRD_SIZE (1u << THIRD_SHIFT) > +#define THIRD_MASK (~(THIRD_SIZE - 1)) > #define SECOND_SHIFT (THIRD_SHIFT + LPAE_SHIFT) > +#define SECOND_SIZE (1u << SECOND_SHIFT) > +#define SECOND_MASK (~(SECOND_SIZE - 1)) > #define FIRST_SHIFT (SECOND_SHIFT + LPAE_SHIFT) > +#define FIRST_SIZE (1u << FIRST_SHIFT) > +#define FIRST_MASK (~(FIRST_SIZE - 1))Whitespace is off here.> > /* Calculate the offsets into the pagetables for a given VA */ > #define first_linear_offset(va) (va >> FIRST_SHIFT)
Ian Campbell
2012-Dec-19 12:29 UTC
Re: [PATCH v3 4/8] xen/vesa: use the new fb_* functions
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:> Make use of the framebuffer functions previously introduced.Weren''t you going to squash this into the previous patch, since it is effectively code motion?
> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) > write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); > /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ > > + /* preserve the DTB mapping a little while longer */Not so much "preserve" as "put back" after this function clobbered it. I''m not convinced by this effectively open coding of a "stack" of mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be used for ephemeral mappings within individual functions or over short spans of code -- otherwise there is plenty of potential for clashes which lead to hard to decipher bugs. Can we not map the boot fdt as and when we need it instead of trying to preserve a mapping? Or maybe FIXMAP_BOOT_FDT?> + pte = mfn_to_xen_entry(((unsigned long) _sdtb + boot_phys_offset) >> PAGE_SHIFT); > + write_pte(xen_second + second_linear_offset(BOOT_MISC_VIRT_START), pte);This use of _sdtb is only valid if CONFIG_DTB_FILE. You need to propagate atag_paddr as passed to start_xen. Ian.
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:> Introduce a Versatile Express specific function to read/write > motherboard settings. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/platform_vexpress.c | 97 +++++++++++++++++++++++++++++++ > xen/include/asm-arm/platform_vexpress.h | 23 +++++++I wonder if we ought to start a "platforms" subdirs include and arch? This is presumably the first of many. Going the plat-<foo> route seems unnecessary at least at this stage.> +#include <asm/platform_vexpress.h> > +#include <xen/mm.h> > + > +#define DCC_SHIFT 26 > +#define FUNCTION_SHIFT 20 > +#define SITE_SHIFT 16 > +#define POSITION_SHIFT 12 > +#define DEVICE_SHIFT 0 > + > +int vexpress_syscfg(int write, int function, int device, uint32_t *data) > +{ > + uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC); > + uint32_t stat; > + int dcc = 0; /* DCC to access */ > + int site = 0; /* motherboard */ > + int position = 0; /* motherboard */motherboard twice?> + set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED); > + > + if ( syscfg[V2M_SYS_CFGCTRL] & V2M_SYS_CFG_START ) > + return -1; > + > + /* clear the complete bit in the V2M_SYS_CFGSTAT status register */Do you mean clear all the bits or specifically the "completion" bit?> + syscfg[V2M_SYS_CFGSTAT] = 0; > + > + if ( write ) > + { > + /* write data */ > + syscfg[V2M_SYS_CFGDATA] = *data; > + > + /* set control register */ > + syscfg[V2M_SYS_CFGCTRL] = V2M_SYS_CFG_START | V2M_SYS_CFG_WRITE | > + (dcc << DCC_SHIFT) | (function << FUNCTION_SHIFT) | > + (site << SITE_SHIFT) | (position << POSITION_SHIFT) | > + (device << DEVICE_SHIFT);Most of this shifting is repeated below, perhaps do it once into a local var to avoid them getting out of sync?> + > + /* wait for complete flag to be set */ > + do { > + stat = syscfg[V2M_SYS_CFGSTAT]; > + dsb(); > + } while ( !(stat & V2M_SYS_CFG_COMPLETE) );I assume there''s no wait-for-event or way to relax this spin loop? Since this is repeated below a helper "wait_for_ocmplet" might be good. Actually, this while bit "set control register" until the error check is common to both the read and write cases, isn''t it?> + > + /* check error status and return error flag if set */ > + if ( stat & V2M_SYS_CFG_ERROR ) > + return -1; > + } else { > + /* set control register */ > + syscfg[V2M_SYS_CFGCTRL] = V2M_SYS_CFG_START | (dcc << DCC_SHIFT) | > + (function << FUNCTION_SHIFT) | (site << SITE_SHIFT) | > + (position << POSITION_SHIFT) | (device << DEVICE_SHIFT); > + > + /* wait for complete flag to be set */ > + do { > + stat = syscfg[V2M_SYS_CFGSTAT]; > + dsb(); > + } while ( !(stat & V2M_SYS_CFG_COMPLETE) ); > + > + /* check error status flag and return error flag if set */ > + if ( stat & V2M_SYS_CFG_ERROR ) > + return -1; > + else > + /* read data */ > + *data = syscfg[V2M_SYS_CFGDATA]; > + } > + > + clear_fixmap(FIXMAP_MISC); > + > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-set-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/asm-arm/platform_vexpress.h b/xen/include/asm-arm/platform_vexpress.h > index 3556af3..407602d 100644 > --- a/xen/include/asm-arm/platform_vexpress.h > +++ b/xen/include/asm-arm/platform_vexpress.h > @@ -6,6 +6,29 @@ > #define V2M_SYS_FLAGSSET (0x30) > #define V2M_SYS_FLAGSCLR (0x34) > > +#define V2M_SYS_CFGDATA (0x00A0/4) > +#define V2M_SYS_CFGCTRL (0x00A4/4) > +#define V2M_SYS_CFGSTAT (0x00A8/4)It''d be better to either define all registers in bytes (as the existing FLAGS*) or words (the new ones) and not to mix the two...> + > +#define V2M_SYS_CFG_START (1<<31) > +#define V2M_SYS_CFG_WRITE (1<<30) > +#define V2M_SYS_CFG_ERROR (1<<1) > +#define V2M_SYS_CFG_COMPLETE (1<<0) > + > +#define V2M_SYS_CFG_OSC_FUNC 1 > +#define V2M_SYS_CFG_OSC0 0 > +#define V2M_SYS_CFG_OSC1 1 > +#define V2M_SYS_CFG_OSC2 2 > +#define V2M_SYS_CFG_OSC3 3 > +#define V2M_SYS_CFG_OSC4 4 > +#define V2M_SYS_CFG_OSC5 5 > + > +#ifndef __ASSEMBLY__ > +#include <xen/inttypes.h> > + > +int vexpress_syscfg(int write, int function, int device, uint32_t *data); > +#endif > + > #endif /* __ASM_ARM_PLATFORM_H */ > /* > * Local variables:
Ian Campbell
2012-Dec-19 12:58 UTC
Re: [PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:> +static void set_color_masks(int bpp, > + int *red_shift, int *green_shift, int *blue_shift, > + int *red_size, int *green_size, int *blue_size)This is crying out for a pointer to a struct.> +{ > + switch (bpp) { > + case 2: > + *red_shift = 0; > + *green_shift = 5; > + *blue_shift = 11; > + *red_size = 5; > + *green_size = 6; > + *blue_size = 5; > + break; > + case 3: > + case 4: > + *red_shift = 0; > + *green_shift = 8; > + *blue_shift = 16; > + *red_size = 8; > + *green_size = 8; > + *blue_size = 8; > + break; > + default: > + BUG(); > + break; > + } > +} > + > +static void set_pixclock(uint32_t pixclock) > +{Doesn''t there need to be some sort of "are we running on a vexpress" check here? e.g. a DTB compatibility node check or something?> + vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock); > +} > + > +void __init video_init(void) > +{ > + int node, depth; > + u32 address_cells, size_cells; > + struct fb_prop fbp; > + unsigned char *lfb; > + paddr_t hdlcd_start, hdlcd_size; > + paddr_t framebuffer_start, framebuffer_size; > + const struct fdt_property *prop; > + const u32 *cell; > + const char *mode_string; > + char _mode_string[16]; > + int bpp; > + int red_shift, green_shift, blue_shift; > + int red_size, green_size, blue_size; > + struct modeline *videomode = NULL; > + int i; > + > + if ( find_compatible_node("arm,hdlcd", &node, &depth, > + &address_cells, &size_cells) <= 0 ) > + return; > + > + prop = fdt_get_property(device_tree_flattened, node, "reg", NULL); > + if ( !prop ) > + return; > + > + cell = (const u32 *)prop->data; > + device_tree_get_reg(&cell, address_cells, size_cells, > + &hdlcd_start, &hdlcd_size); > + > + prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL); > + if ( !prop ) > + return; > + > + cell = (const u32 *)prop->data; > + device_tree_get_reg(&cell, address_cells, size_cells, > + &framebuffer_start, &framebuffer_size); > + > + mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL); > + if ( !mode_string ) > + { > + bpp = 4; > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > + &red_size, &green_size, &blue_size); > + memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60"));What associates mode_string with this _mode_string?> + }Or should there be an else here? Otherwise can''t mode_string be NULL?> + if ( strlen(mode_string) < strlen("800x600@60") ) > + { > + printk("HDLCD: invalid modeline=%s\n", mode_string); > + return; > + } else { > + char *s = strchr(mode_string, ''-''); > + if ( !s ) > + { > + printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n", > + mode_string); > + bpp = 4; > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > + &red_size, &green_size, &blue_size); > + memcpy(_mode_string, mode_string, strlen(mode_string) + 1);What if strlen(mode_string)+1 > 16 ?> + } else { > + if ( strlen(s) < 6 ) > + { > + printk("HDLCD: invalid mode %s\n", mode_string); > + return; > + }Indentation> + s++; > + if ( !strncmp(s, "16", 2) ) > + { > + bpp = 2; > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > + &red_size, &green_size, &blue_size); > + } > + else if ( !strncmp(s, "24", 2) ) > + { > + bpp = 3; > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > + &red_size, &green_size, &blue_size); > + } > + else if ( !strncmp(s, "32", 2) ) > + { > + bpp = 4; > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > + &red_size, &green_size, &blue_size);This all smells like a lookup table to me.> + } else {extra space here.> + printk("HDLCD: unsupported bpp %s\n", s); > + return; > + } > + i = s - mode_string - 1; > + memcpy(_mode_string, mode_string, i); > + memcpy(_mode_string + i, mode_string + i + 3, 4); > + } > + } > + > + for ( i = 0; i < ARRAY_SIZE(videomodes); i++ ) > + { > + if ( !strcmp(_mode_string, videomodes[i].mode) ) > + { > + videomode = &videomodes[i]; > + break; > + } > + } > + if ( !videomode ) > + { > + printk("HDLCD: unsupported videomode %s\n", _mode_string); > + return; > + } > + > + > + if ( !hdlcd_start || !framebuffer_start )Worth a message? Also couldn''t you have checked this much earlier (before the mode parsing stuff).> + return; > + > + if ( framebuffer_size < bpp * videomode->xres * videomode->yres ) > + { > + printk("HDLCD: the framebuffer is too small, disable the HDLCD driver\n");"disable" or "disabling" ? Ian.
Ian Campbell
2012-Dec-19 14:06 UTC
Re: [PATCH v3 6/8] xen/device_tree: introduce find_compatible_node
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:> Introduce a find_compatible_node function that can be used by device> +static int _find_compatible_node(const void *fdt, > + int node, const char *name, int depth, > + u32 address_cells, u32 size_cells, > + void *data) > +{ > + struct find_compat *c = (struct find_compat *) data; > + > + if ( c->found ) > + return 0;It''d be nice if returning e.g. 1 would cause device_tree_for_each_node to stop walking the DTB and return immediately. Would make this function cleaner and avoid pointlessly parsing the rest of the DTB.> + > + if ( device_tree_node_compatible(fdt, node, c->compatible) ) > + { > + c->found = 1; > + c->node = node; > + c->depth = depth; > + c->address_cells = address_cells; > + c->size_cells = size_cells; > + } > + return 0; > +} > + > +int find_compatible_node(const char *compatible, int *node, int *depth, > + u32 *address_cells, u32 *size_cells) > +{ > + int ret; > + struct find_compat c; > + c.compatible = compatible; > + c.found = 0; > + > + ret = device_tree_for_each_node(device_tree_flattened, _find_compatible_node, &c); > + if ( !c.found ) > + return ret;
Stefano Stabellini
2013-Jan-08 13:51 UTC
Re: [PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller
On Wed, 19 Dec 2012, Ian Campbell wrote:> On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote: > > +static void set_color_masks(int bpp, > > + int *red_shift, int *green_shift, int *blue_shift, > > + int *red_size, int *green_size, int *blue_size) > > This is crying out for a pointer to a struct.OK> > +{ > > + switch (bpp) { > > + case 2: > > + *red_shift = 0; > > + *green_shift = 5; > > + *blue_shift = 11; > > + *red_size = 5; > > + *green_size = 6; > > + *blue_size = 5; > > + break; > > + case 3: > > + case 4: > > + *red_shift = 0; > > + *green_shift = 8; > > + *blue_shift = 16; > > + *red_size = 8; > > + *green_size = 8; > > + *blue_size = 8; > > + break; > > + default: > > + BUG(); > > + break; > > + } > > +} > > + > > +static void set_pixclock(uint32_t pixclock) > > +{ > > Doesn''t there need to be some sort of "are we running on a vexpress" > check here? e.g. a DTB compatibility node check or something?Yes. TBH the ideal thing to have would be a set of generic platform functions that get initialized early in start_xen and then used lated on; but considering that at the moment we only have one platform, it is even difficult to figure out what functions we would need to export.> > + vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock); > > +} > > + > > +void __init video_init(void) > > +{ > > + int node, depth; > > + u32 address_cells, size_cells; > > + struct fb_prop fbp; > > + unsigned char *lfb; > > + paddr_t hdlcd_start, hdlcd_size; > > + paddr_t framebuffer_start, framebuffer_size; > > + const struct fdt_property *prop; > > + const u32 *cell; > > + const char *mode_string; > > + char _mode_string[16]; > > + int bpp; > > + int red_shift, green_shift, blue_shift; > > + int red_size, green_size, blue_size; > > + struct modeline *videomode = NULL; > > + int i; > > + > > + if ( find_compatible_node("arm,hdlcd", &node, &depth, > > + &address_cells, &size_cells) <= 0 ) > > + return; > > + > > + prop = fdt_get_property(device_tree_flattened, node, "reg", NULL); > > + if ( !prop ) > > + return; > > + > > + cell = (const u32 *)prop->data; > > + device_tree_get_reg(&cell, address_cells, size_cells, > > + &hdlcd_start, &hdlcd_size); > > + > > + prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL); > > + if ( !prop ) > > + return; > > + > > + cell = (const u32 *)prop->data; > > + device_tree_get_reg(&cell, address_cells, size_cells, > > + &framebuffer_start, &framebuffer_size); > > + > > + mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL); > > + if ( !mode_string ) > > + { > > + bpp = 4; > > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > > + &red_size, &green_size, &blue_size); > > + memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60")); > > What associates mode_string with this _mode_string?_mode_string is RW while mode_string is RO. This particular mode string (1280x1024@60) is chosen arbitrarily.> > + if ( strlen(mode_string) < strlen("800x600@60") ) > > + { > > + printk("HDLCD: invalid modeline=%s\n", mode_string); > > + return; > > + } else { > > + char *s = strchr(mode_string, ''-''); > > + if ( !s ) > > + { > > + printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n", > > + mode_string); > > + bpp = 4; > > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift, > > + &red_size, &green_size, &blue_size); > > + memcpy(_mode_string, mode_string, strlen(mode_string) + 1); > > What if strlen(mode_string)+1 > 16 ?in theory it can''t be if it follows the DT spec, but I''ll add a check for it
Stefano Stabellini
2013-Jan-08 17:16 UTC
Re: [PATCH v3 7/8] xen/arm: introduce vexpress_syscfg
On Wed, 19 Dec 2012, Ian Campbell wrote:> On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote: > > Introduce a Versatile Express specific function to read/write > > motherboard settings. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/arch/arm/Makefile | 1 + > > xen/arch/arm/platform_vexpress.c | 97 +++++++++++++++++++++++++++++++ > > xen/include/asm-arm/platform_vexpress.h | 23 +++++++ > > I wonder if we ought to start a "platforms" subdirs include and arch? > This is presumably the first of many. Going the plat-<foo> route seems > unnecessary at least at this stage.I can move these two files in a subdirectory, but until we have at least another platform is going to be difficult to come up with a decent abstraction.> > +#include <asm/platform_vexpress.h> > > +#include <xen/mm.h> > > + > > +#define DCC_SHIFT 26 > > +#define FUNCTION_SHIFT 20 > > +#define SITE_SHIFT 16 > > +#define POSITION_SHIFT 12 > > +#define DEVICE_SHIFT 0 > > + > > +int vexpress_syscfg(int write, int function, int device, uint32_t *data) > > +{ > > + uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC); > > + uint32_t stat; > > + int dcc = 0; /* DCC to access */ > > + int site = 0; /* motherboard */ > > + int position = 0; /* motherboard */ > > motherboard twice? > > > + set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED); > > + > > + if ( syscfg[V2M_SYS_CFGCTRL] & V2M_SYS_CFG_START ) > > + return -1; > > + > > + /* clear the complete bit in the V2M_SYS_CFGSTAT status register */ > > Do you mean clear all the bits or specifically the "completion" bit?the completion bit but there isn''t much else on that register, so I think it is OK to zero it.> > + syscfg[V2M_SYS_CFGSTAT] = 0; > > + > > + if ( write ) > > + { > > + /* write data */ > > + syscfg[V2M_SYS_CFGDATA] = *data; > > + > > + /* set control register */ > > + syscfg[V2M_SYS_CFGCTRL] = V2M_SYS_CFG_START | V2M_SYS_CFG_WRITE | > > + (dcc << DCC_SHIFT) | (function << FUNCTION_SHIFT) | > > + (site << SITE_SHIFT) | (position << POSITION_SHIFT) | > > + (device << DEVICE_SHIFT); > > Most of this shifting is repeated below, perhaps do it once into a local > var to avoid them getting out of sync? > > > + > > + /* wait for complete flag to be set */ > > + do { > > + stat = syscfg[V2M_SYS_CFGSTAT]; > > + dsb(); > > + } while ( !(stat & V2M_SYS_CFG_COMPLETE) ); > > I assume there''s no wait-for-event or way to relax this spin loop?Nope> Since this is repeated below a helper "wait_for_ocmplet" might be good. > > Actually, this while bit "set control register" until the error check is > common to both the read and write cases, isn''t it?I''ll refactor them both into a separate function
Stefano Stabellini
2013-Jan-08 18:33 UTC
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings
On Wed, 19 Dec 2012, Ian Campbell wrote:> > @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) > > write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); > > /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ > > > > + /* preserve the DTB mapping a little while longer */ > > Not so much "preserve" as "put back" after this function clobbered it. > > I''m not convinced by this effectively open coding of a "stack" of > mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be > used for ephemeral mappings within individual functions or over short > spans of code -- otherwise there is plenty of potential for clashes > which lead to hard to decipher bugs. > > Can we not map the boot fdt as and when we need it instead of trying to > preserve a mapping?I don''t like this idea because it means that the device initialization functions in start_xen that are called after setup_pagetables and before setup_mm, like gic_init, suddenly become position dependent: they cannot be moved after setup_mm.> Or maybe FIXMAP_BOOT_FDT?Assuming that it fits in a single 4k page, that could work. However we would have a "temporary" fixmap mapping that is only used at boot time and never again. At that point maybe it is better to map the DTB somewhere else in another 2MB slot from head.S, preserve the mapping in setup_pagetables and remove it at the beginning of setup_mm. Maybe it could be called BOOT_EARLY_FDT.
Stefano Stabellini
2013-Jan-08 19:02 UTC
Re: [PATCH v3 1/8] xen/arm: introduce early_ioremap
On Wed, 19 Dec 2012, Ian Campbell wrote:> On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote: > > Introduce a function to map a range of physical memory into Xen virtual > > memory. > > It doesn''t need domheap to be setup. > > It is going to be used to map the videoram. > > > > Add flush_xen_data_tlb_range, that flushes a range of virtual addresses. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/arch/arm/mm.c | 32 ++++++++++++++++++++++++++++++++ > > xen/include/asm-arm/config.h | 2 ++ > > xen/include/asm-arm/mm.h | 3 ++- > > xen/include/asm-arm/page.h | 23 +++++++++++++++++++++++ > > 4 files changed, 59 insertions(+), 1 deletions(-) > > > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > > index 855f83d..0d7a163 100644 > > --- a/xen/arch/arm/mm.c > > +++ b/xen/arch/arm/mm.c > > @@ -367,6 +367,38 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe) > > frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pages * sizeof(struct page_info)); > > } > > > > +/* Map the physical memory range start - start + len into virtual > > + * memory and return the virtual address of the mapping. > > + * start has to be 2MB aligned. > > + * len has to be < EARLY_VMAP_END - EARLY_VMAP_START. > > + */ > > +void* early_ioremap(paddr_t start, size_t len, unsigned attributes) > > Should be __init I think, to discourage its use later on. If when we > need that they proper vmap+ioremap should be implemented.OK> > +{ > > + static unsigned long virt_start = EARLY_VMAP_START; > > Is the idea that if/when we implement proper vmap + ioremap support we > should initialise the vmap area with EARLY_VMAP_START..virt_start > already assigned and virt_start..EARLY_VMAP_END free (removing all the > EARLY_ I guess)?Yes, that was the idea.> At some point I suppose we will need to integrate this with > xen/common/vmap.c. > > > + void* ret_addr = (void *)virt_start; > > + paddr_t end = start + len; > > + > > + ASSERT(!(start & (~SECOND_MASK))); > > + ASSERT(!(virt_start & (~SECOND_MASK))); > > + > > + /* The range we need to map is too big */ > > + if ( virt_start + len >= EARLY_VMAP_END ) > > + return NULL; > > + > > + while ( start < end ) > > + { > > + lpae_t e = mfn_to_xen_entry(start >> PAGE_SHIFT); > > + e.pt.ai = attributes; > > + write_pte(xen_second + second_table_offset(virt_start), e); > > + > > + start += SECOND_SIZE; > > + virt_start += SECOND_SIZE; > > + } > > + flush_xen_data_tlb_range((unsigned long) ret_addr, len); > > Just cast this in the return? At the moment you cast to void* only to > cast it back to unsigned long.OK> > + > > + return ret_addr; > > +} > > + > > enum mg { mg_clear, mg_ro, mg_rw, mg_rx }; > > static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) > > { > > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > > index 2a05539..87db0d1 100644 > > --- a/xen/include/asm-arm/config.h > > +++ b/xen/include/asm-arm/config.h > > @@ -73,9 +73,11 @@ > > #define FIXMAP_ADDR(n) (mk_unsigned_long(0x00400000) + (n) * PAGE_SIZE) > > #define BOOT_MISC_VIRT_START mk_unsigned_long(0x00600000) > > #define FRAMETABLE_VIRT_START mk_unsigned_long(0x02000000) > > +#define EARLY_VMAP_START mk_unsigned_long(0x10000000) > > There is a comment right above here which describes Xen virtual memory > layout, this needs updating as Tim requested before. > > Also the convention is FOO_VIRT_START.OK> > #define XENHEAP_VIRT_START mk_unsigned_long(0x40000000) > > #define DOMHEAP_VIRT_START mk_unsigned_long(0x80000000) > > > > +#define EARLY_VMAP_END XENHEAP_VIRT_START > > #define HYPERVISOR_VIRT_START XEN_VIRT_START > > > > #define DOMHEAP_ENTRIES 1024 /* 1024 2MB mapping slots */ > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > > index e95ece1..4ed5df6 100644 > > --- a/xen/include/asm-arm/mm.h > > +++ b/xen/include/asm-arm/mm.h > > @@ -150,7 +150,8 @@ extern void setup_frametable_mappings(paddr_t ps, paddr_t pe); > > extern void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes); > > /* Remove a mapping from a fixmap entry */ > > extern void clear_fixmap(unsigned map); > > - > > +/* map a 2MB aligned physical range in virtual memory. */ > > +void* early_ioremap(paddr_t start, size_t len, unsigned attributes); > > > > #define mfn_valid(mfn) ({ \ > > unsigned long __m_f_n = (mfn); \ > > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > > index d89261e..0790dda 100644 > > --- a/xen/include/asm-arm/page.h > > +++ b/xen/include/asm-arm/page.h > > @@ -328,6 +328,23 @@ static inline void flush_xen_data_tlb_va(unsigned long va) > > : : "r" (va) : "memory"); > > } > > > > +/* > > + * Flush a range of VA''s hypervisor mappings from the data TLB. This is not > > + * sufficient when changing code mappings or for self modifying code. > > + */ > > +static inline void flush_xen_data_tlb_range(unsigned long va, unsigned long size) > > +{ > > + unsigned long end = va + size; > > + while ( va < end ) { > > + asm volatile("dsb;" /* Ensure preceding are visible */ > > Either this dsb should be on the next line (aligned with the following > lines) or the following lines should be indented further to match (we > have both styles in this file, but lets not have a 3rd).OK> Although it would probably be better to just call flush_xen_data_tlb_va > here? > > Function should be flush_xen_data_tlb_range_va I think. I''m not sure it > is worth a new function though, perhaps just add size to the existing > flush_xen_data_tlb_va, there''s not too many callers?OK, I''ll keep and rename the new function> While I was grepping I noticed flush_xen_data_tlb_va(dest_va) in > setup_pagetables(). dest_va has just been setup with a second level (2M > mapping). Is it not a bit dodgy to only flush the first 4K of that?You are right! WOW! Nice catch!
On 08/01/13 18:33, Stefano Stabellini wrote:> On Wed, 19 Dec 2012, Ian Campbell wrote: >>> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) >>> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); >>> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ >>> >>> + /* preserve the DTB mapping a little while longer */ >> >> Not so much "preserve" as "put back" after this function clobbered it. >> >> I''m not convinced by this effectively open coding of a "stack" of >> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be >> used for ephemeral mappings within individual functions or over short >> spans of code -- otherwise there is plenty of potential for clashes >> which lead to hard to decipher bugs. >> >> Can we not map the boot fdt as and when we need it instead of trying to >> preserve a mapping? > > I don''t like this idea because it means that the device initialization > functions in start_xen that are called after setup_pagetables and before > setup_mm, like gic_init, suddenly become position dependent: they cannot > be moved after setup_mm.The mapping of the DTB into BOOT_MISC_VIRT_START was only for use by device_tree_early_init(). Everything else should be using the final mapping setup in setup_mm(). I think this means setup_mm() needs to immediately after setup_pagestables() and before gic_init(), pl011_init() etc. David
Stefano Stabellini
2013-Jan-08 19:32 UTC
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings
On Tue, 8 Jan 2013, David Vrabel wrote:> On 08/01/13 18:33, Stefano Stabellini wrote: > > On Wed, 19 Dec 2012, Ian Campbell wrote: > >>> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) > >>> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); > >>> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ > >>> > >>> + /* preserve the DTB mapping a little while longer */ > >> > >> Not so much "preserve" as "put back" after this function clobbered it. > >> > >> I''m not convinced by this effectively open coding of a "stack" of > >> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be > >> used for ephemeral mappings within individual functions or over short > >> spans of code -- otherwise there is plenty of potential for clashes > >> which lead to hard to decipher bugs. > >> > >> Can we not map the boot fdt as and when we need it instead of trying to > >> preserve a mapping? > > > > I don''t like this idea because it means that the device initialization > > functions in start_xen that are called after setup_pagetables and before > > setup_mm, like gic_init, suddenly become position dependent: they cannot > > be moved after setup_mm. > > The mapping of the DTB into BOOT_MISC_VIRT_START was only for use by > device_tree_early_init(). Everything else should be using the final > mapping setup in setup_mm(). > > I think this means setup_mm() needs to immediately after > setup_pagestables() and before gic_init(), pl011_init() etc.That would be a very easy way out of this problem. I think I am going to go for it.