Xen devel - Jan 2013 - [PATCH v4 0/7] xen: ARM HDLCD video driver

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.



Changes in v4:
- rename flush_xen_data_tlb_range to flush_xen_data_tlb_range_va;
- replace all the calls to flush_xen_data_tlb_va, with calls to
flush_xen_data_tlb_range_va;
- flush the entire 2MB mapping at BOOT_MISC_VIRT_START rather than just
the first 4k;
- remove flush_xen_data_tlb_va;
- fix indentation;
- rename EARLY_VMAP_START/END to EARLY_VMAP_VIRT_START/END;
- mark early_ioremap as __init;
- reduce the amount of casts in early_ioremap;
- squash the vesa.c changes into this patch;
- rename fb* to lfb*;
- pass a pointer to fb_init;
- use %u for screen dimensions;
- specify loglevel in printk;
- call fb_free on error in fb_alloc;
- no __init on declarations;
- do not break messages to fit 80 columns;
- remove "preserve DTB mappings";
- introduce a new "move setup_mm right after setup_pagetables" patch;
- stop iterating over the DT nodes in device_tree_for_each_node if
func returns a value != 0;
- return 1 from _find_compatible_node when a node is found;
- move the wait loop and the syscfg cfgctrl write into a separate
function;
- fix comments;
- define all registers in write;
- move platform_vexpress.c to platforms/vexpress.c;
- move platform_vexpress.h to arm-arm/platforms/vexpress.h;
- use a lookup table to set the color masks;
- fix indentation;
- make sure mode_string is not NULL and is not bigger than 16 chars
before continuing;
- introduce 2 separate error messages for !hdlcd_start and
!framebuffer_start at the beginning of video_init;
- mark get_color_masks and set_pixclock as __init;
- check that we are running on a vexpress machine before calling
vexpress_syscfg.


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 (7):
      xen/arm: introduce early_ioremap
      xen: infrastructure to have cross-platform video drivers
      xen: introduce a generic framebuffer driver
      xen/arm: move setup_mm right after setup_pagetables
      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/mm.c                        |   40 ++++-
 xen/arch/arm/mode_switch.S               |    2 +-
 xen/arch/arm/platforms/vexpress.c        |  100 +++++++++++
 xen/arch/arm/setup.c                     |   10 +-
 xen/arch/x86/Rules.mk                    |    1 +
 xen/common/device_tree.c                 |   56 ++++++-
 xen/drivers/Makefile                     |    2 +-
 xen/drivers/char/console.c               |   12 +-
 xen/drivers/video/Makefile               |   12 +-
 xen/drivers/video/arm_hdlcd.c            |  280 ++++++++++++++++++++++++++++++
 xen/drivers/video/lfb.c                  |  206 ++++++++++++++++++++++
 xen/drivers/video/lfb.h                  |   49 +++++
 xen/drivers/video/modelines.h            |   77 ++++++++
 xen/drivers/video/vesa.c                 |  179 +++----------------
 xen/drivers/video/vga.c                  |   12 +-
 xen/include/asm-arm/config.h             |    6 +
 xen/include/asm-arm/mm.h                 |    3 +-
 xen/include/asm-arm/page.h               |   24 ++-
 xen/include/asm-arm/platform_vexpress.h  |   17 --
 xen/include/asm-arm/platforms/vexpress.h |   40 +++++
 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 +++
 26 files changed, 951 insertions(+), 217 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_va, that flushes a range of virtual addresses.
Replace all the calls to flush_xen_data_tlb_va with calls to
flush_xen_data_tlb_range_va and remove flush_xen_data_tlb_va.

In the case of the dest_va tlb flush at the beginning of
setup_pagetables, flush the entire 2MB mapping rather than just the
first 4K.


Changes in v4:
- rename flush_xen_data_tlb_range to flush_xen_data_tlb_range_va;
- replace all the calls to flush_xen_data_tlb_va, with calls to
flush_xen_data_tlb_range_va;
- flush the entire 2MB mapping at BOOT_MISC_VIRT_START rather than just
the first 4k;
- remove flush_xen_data_tlb_va;
- fix indentation;
- rename EARLY_VMAP_START/END to EARLY_VMAP_VIRT_START/END;
- mark early_ioremap as __init;
- reduce the amount of casts in early_ioremap.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/mm.c            |   40 ++++++++++++++++++++++++++++++++++++----
 xen/include/asm-arm/config.h |    4 ++++
 xen/include/asm-arm/mm.h     |    3 ++-
 xen/include/asm-arm/page.h   |   24 +++++++++++++++++-------
 4 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 855f83d..ca66395 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -112,7 +112,7 @@ void set_fixmap(unsigned map, unsigned long mfn, unsigned
attributes)
     pte.pt.ai = attributes;
     pte.pt.xn = 1;
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_va(FIXMAP_ADDR(map));
+    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 /* Remove a mapping from a fixmap entry */
@@ -120,7 +120,7 @@ void clear_fixmap(unsigned map)
 {
     lpae_t pte = {0};
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_va(FIXMAP_ADDR(map));
+    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 /* Map a page of domheap memory */
@@ -184,7 +184,7 @@ void *map_domain_page(unsigned long mfn)
      * We may not have flushed this specific subpage at map time,
      * since we only flush the 4k page not the superpage
      */
-    flush_xen_data_tlb_va(va);
+    flush_xen_data_tlb_range_va(va, PAGE_SIZE);
 
     return (void *)va;
 }
@@ -219,7 +219,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset,
paddr_t xen_paddr)
     dest_va = BOOT_MISC_VIRT_START;
     pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT);
     write_pte(xen_second + second_table_offset(dest_va), pte);
-    flush_xen_data_tlb_va(dest_va);
+    flush_xen_data_tlb_range_va(dest_va, SECOND_SIZE);
 
     /* Calculate virt-to-phys offset for the new location */
     phys_offset = xen_paddr - (unsigned long) _start;
@@ -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_VIRT_END - EARLY_VMAP_VIRT_START.
+ */
+void* __init early_ioremap(paddr_t start, size_t len, unsigned attributes)
+{
+    static unsigned long virt_start = EARLY_VMAP_VIRT_START;
+    unsigned long ret_addr = 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_VIRT_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_va(ret_addr, len);
+
+    return (void*)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..e5dce5e 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -60,6 +60,8 @@
  *  6M  -  8M   Early boot misc (see below)
  *
  * 32M - 128M   Frametable: 24 bytes per page for 16GB of RAM
+ * 256M -  1G   VMAP: ioremap and early_ioremap use this virtual address
+ *                    space
  *
  *  1G -   2G   Xenheap: always-mapped memory
  *  2G -   4G   Domheap: on-demand-mapped
@@ -73,9 +75,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_VIRT_START  mk_unsigned_long(0x10000000)
 #define XENHEAP_VIRT_START     mk_unsigned_long(0x40000000)
 #define DOMHEAP_VIRT_START     mk_unsigned_long(0x80000000)
 
+#define EARLY_VMAP_VIRT_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..fb4febc 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -316,16 +316,20 @@ static inline void flush_xen_data_tlb(void)
 }
 
 /*
- * Flush one VA''s hypervisor mappings from the data TLB. This is not
+ * 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_va(unsigned long va)
+static inline void flush_xen_data_tlb_range_va(unsigned long va, unsigned long
size)
 {
-    asm volatile("dsb;" /* Ensure preceding are visible */
-                 STORE_CP32(0, TLBIMVAH)
-                 "dsb;" /* Ensure completion of the TLB flush */
-                 "isb;"
-                 : : "r" (va) : "memory");
+    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 */
@@ -418,8 +422,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

- 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

Abstract away from vesa.c the funcions to handle a linear framebuffer
and print characters to it.
Make use of the new functions in vesa.c.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Changes in v4:
- squash the vesa.c changes into this patch;
- rename fb* to lfb*;
- pass a pointer to fb_init;
- use %u for screen dimensions;
- specify loglevel in printk;
- call fb_free on error in fb_alloc;
- no __init on declarations;
- do not break messages to fit 80 columns.
---
 xen/drivers/video/Makefile |    1 +
 xen/drivers/video/lfb.c    |  206 ++++++++++++++++++++++++++++++++++++++++++++
 xen/drivers/video/lfb.h    |   49 +++++++++++
 xen/drivers/video/vesa.c   |  179 ++++++--------------------------------
 4 files changed, 282 insertions(+), 153 deletions(-)
 create mode 100644 xen/drivers/video/lfb.c
 create mode 100644 xen/drivers/video/lfb.h

diff --git a/xen/drivers/video/Makefile b/xen/drivers/video/Makefile
index 2993c39..77f9d5d 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) += lfb.o
 obj-$(HAS_VGA) += vesa.o
diff --git a/xen/drivers/video/lfb.c b/xen/drivers/video/lfb.c
new file mode 100644
index 0000000..84d92d9
--- /dev/null
+++ b/xen/drivers/video/lfb.c
@@ -0,0 +1,206 @@
+/******************************************************************************
+ * lfb.c
+ *
+ * linear frame buffer handling.
+ */
+
+#include <xen/config.h>
+#include <xen/kernel.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include "lfb.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 lfb_status {
+    struct lfb_prop lfbp;
+
+    unsigned char *lbuf, *text_buf;
+    unsigned int *line_len;
+    unsigned int xpos, ypos;
+};
+static struct lfb_status lfb;
+
+static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
+
+    for ( i = 0; i < lfb.lfbp.font->height; i++ )
+    {
+        unsigned char *ptr = lfb.lbuf;
+
+        for ( j = 0; j < nr_chars; j++ )
+        {
+            const unsigned char *bits = lfb.lfbp.font->data;
+            bits += ((text_line[j] * lfb.lfbp.font->height + i) *
+                     ((lfb.lfbp.font->width + 7) >> 3));
+            for ( b = lfb.lfbp.font->width; b--; )
+            {
+                pixel = (*bits & (1u<<b)) ? lfb.lfbp.pixel_on : 0;
+                memcpy(ptr, &pixel, bpp);
+                ptr += bpp;
+            }
+        }
+
+        memset(ptr, 0, (lfb.lfbp.width - nr_chars * lfb.lfbp.font->width) *
bpp);
+        memcpy(video_line, lfb.lbuf, nr_cells * lfb.lfbp.font->width * bpp);
+        video_line += lfb.lfbp.bytes_per_line;
+    }
+}
+
+/* Fast mode which redraws all modified parts of a 2D text buffer. */
+void lfb_redraw_puts(const char *s)
+{
+    unsigned int i, min_redraw_y = lfb.ypos;
+    char c;
+
+    /* Paste characters into text buffer. */
+    while ( (c = *s++) != ''\0'' )
+    {
+        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
+        {
+            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
+            {
+                min_redraw_y = 0;
+                lfb.ypos = lfb.lfbp.text_rows - 1;
+                memmove(lfb.text_buf, lfb.text_buf + lfb.lfbp.text_columns,
+                        lfb.ypos * lfb.lfbp.text_columns);
+                memset(lfb.text_buf + lfb.ypos * lfb.lfbp.text_columns, 0,
lfb.xpos);
+            }
+            lfb.xpos = 0;
+        }
+
+        if ( c != ''\n'' )
+            lfb.text_buf[lfb.xpos++ + lfb.ypos * lfb.lfbp.text_columns] = c;
+    }
+
+    /* Render modified section of text buffer to VESA linear framebuffer. */
+    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
+    {
+        const unsigned char *line = lfb.text_buf + i * lfb.lfbp.text_columns;
+        unsigned int width;
+
+        for ( width = lfb.lfbp.text_columns; width; --width )
+            if ( line[width - 1] )
+                 break;
+        lfb_show_line(line,
+                       lfb.lfbp.lfb + i * lfb.lfbp.font->height *
lfb.lfbp.bytes_per_line,
+                       width, max(lfb.line_len[i], width));
+        lfb.line_len[i] = width;
+    }
+
+    lfb.lfbp.flush();
+}
+
+/* Slower line-based scroll mode which interacts better with dom0. */
+void lfb_scroll_puts(const char *s)
+{
+    unsigned int i;
+    char c;
+
+    while ( (c = *s++) != ''\0'' )
+    {
+        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
+        {
+            unsigned int bytes = (lfb.lfbp.width *
+                                  ((lfb.lfbp.bits_per_pixel + 7) >> 3));
+            unsigned char *src = lfb.lfbp.lfb + lfb.lfbp.font->height *
lfb.lfbp.bytes_per_line;
+            unsigned char *dst = lfb.lfbp.lfb;
+
+            /* New line: scroll all previous rows up one line. */
+            for ( i = lfb.lfbp.font->height; i < lfb.lfbp.height; i++ )
+            {
+                memcpy(dst, src, bytes);
+                src += lfb.lfbp.bytes_per_line;
+                dst += lfb.lfbp.bytes_per_line;
+            }
+
+            /* Render new line. */
+            lfb_show_line(
+                lfb.text_buf,
+                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
+                lfb.xpos, lfb.lfbp.text_columns);
+
+            lfb.xpos = 0;
+        }
+
+        if ( c != ''\n'' )
+            lfb.text_buf[lfb.xpos++] = c;
+    }
+
+    lfb.lfbp.flush();
+}
+
+void lfb_carriage_return(void)
+{
+    lfb.xpos = 0;
+}
+
+int __init lfb_init(struct lfb_prop *lfbp)
+{
+    if ( lfbp->width > MAX_XRES || lfbp->height > MAX_YRES )
+    {
+        printk(XENLOG_WARNING "Couldn''t initialize a %ux%u
framebuffer early.\n",
+                        lfbp->width, lfbp->height);
+        return -EINVAL;
+    }
+
+    lfb.lfbp = *lfbp;
+    lfb.lbuf = lbuf;
+    lfb.text_buf = text_buf;
+    lfb.line_len = line_len;
+    return 0;
+}
+
+int __init lfb_alloc(void)
+{
+    lfb.lbuf = NULL;
+    lfb.text_buf = NULL;
+    lfb.line_len = NULL;
+
+    lfb.lbuf = xmalloc_bytes(lfb.lfbp.bytes_per_line);
+    if ( !lfb.lbuf )
+        goto fail;
+
+    lfb.text_buf = xzalloc_bytes(lfb.lfbp.text_columns * lfb.lfbp.text_rows);
+    if ( !lfb.text_buf )
+        goto fail;
+
+    lfb.line_len = xzalloc_array(unsigned int, lfb.lfbp.text_columns);
+    if ( !lfb.line_len )
+        goto fail;
+
+    memcpy(lfb.lbuf, lbuf, lfb.lfbp.bytes_per_line);
+    memcpy(lfb.text_buf, text_buf, lfb.lfbp.text_columns * lfb.lfbp.text_rows);
+    memcpy(lfb.line_len, line_len, lfb.lfbp.text_columns);
+
+    return 0;
+
+fail:
+    printk(XENLOG_ERR "Couldn''t allocate enough memory to drive
the framebuffer\n");
+    lfb_free();
+
+    return -ENOMEM;
+}
+
+void lfb_free(void)
+{
+    xfree(lfb.lbuf);
+    xfree(lfb.text_buf);
+    xfree(lfb.line_len);
+}
diff --git a/xen/drivers/video/lfb.h b/xen/drivers/video/lfb.h
new file mode 100644
index 0000000..479ed04
--- /dev/null
+++ b/xen/drivers/video/lfb.h
@@ -0,0 +1,49 @@
+/*
+ * xen/drivers/video/lfb.h
+ *
+ * Cross-platform framebuffer library
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ * Copyright (c) 2013 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_LFB_H
+#define _XEN_LFB_H
+
+#include <xen/init.h>
+
+struct lfb_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 lfb_redraw_puts(const char *s);
+void lfb_scroll_puts(const char *s);
+void lfb_carriage_return(void);
+void lfb_free(void);
+
+/* initialize the framebuffer, can be called early (before xmalloc is
+ * available) */
+int lfb_init(struct lfb_prop *lfbp);
+/* lfb_alloc allocates internal structures using xmalloc */
+int lfb_alloc(void);
+
+#endif
diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index aaf8b23..c216371 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 "lfb.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 lfb_prop lfbp;
 
-    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;
+    lfbp.font = font;
+    lfbp.bits_per_pixel = vlfb_info.bits_per_pixel;
+    lfbp.bytes_per_line = vlfb_info.bytes_per_line;
+    lfbp.width = vlfb_info.width;
+    lfbp.height = vlfb_info.height;
+    lfbp.flush = lfb_flush;
+    lfbp.text_columns = vlfb_info.width / font->width;
+    lfbp.text_rows = vlfb_info.height / font->height;
 
-    lfb = ioremap(vlfb_info.lfb_base, vram_remap);
+    lfbp.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 ( lfb_init(&lfbp) < 0 )
+        return;
+    if ( lfb_alloc() < 0 )
+        return;
+    video_puts = lfb_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 = lfb_scroll_puts;
+        lfb_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();
+        lfb_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 restore them only in setup_mm.

Move setup_mm right after setup_pagetables so that we don''t leave over
portions of the initialization code without a working DTB mapping.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/setup.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2076724..8882514 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -212,6 +212,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     cmdline_parse(device_tree_bootargs(fdt));
 
     setup_pagetables(boot_phys_offset, get_xen_paddr());
+    setup_mm(atag_paddr, fdt_size);
 
 #ifdef EARLY_UART_ADDRESS
     /* TODO Need to get device tree or command line for UART address */
@@ -229,8 +230,6 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     init_xen_time();
 
-    setup_mm(atag_paddr, fdt_size);
-
     /* Setup Hyp vector base */
     WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
     printk("Set hyp vector base to %"PRIx32" (expected
%p)\n",
-- 
1.7.2.5

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 v4:
- stop iterating over the DT nodes in device_tree_for_each_node if func
returns a value != 0;
- return 1 from _find_compatible_node when a node is found.

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      |   56 +++++++++++++++++++++++++++++++++++++++-
 xen/include/xen/device_tree.h |    3 ++
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 8882514..89ef3df 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());
     setup_mm(atag_paddr, fdt_size);
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 8b4ef2f..68be270 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -137,7 +137,7 @@ u32 device_tree_get_u32(const void *fdt, int node, const
char *prop_name)
  * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
  *
  * Returns 0 if all nodes were iterated over successfully.  If @func
- * returns a negative value, that value is returned immediately.
+ * returns a value different from 0, that value is returned immediately.
  */
 int device_tree_for_each_node(const void *fdt,
                               device_tree_node_func func, void *data)
@@ -166,12 +166,64 @@ int device_tree_for_each_node(const void *fdt,
 
         ret = func(fdt, node, name, depth,
                    address_cells[depth-1], size_cells[depth-1], data);
-        if ( ret < 0 )
+        if ( ret != 0 )
             return ret;
     }
     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 1;
+
+    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 1;
+    }
+    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

Introduce a Versatile Express specific function to read/write
motherboard settings.

Changes in v4:
- move the wait loop and the syscfg cfgctrl write into a separate
function;
- fix comments;
- define all registers in write;
- move platform_vexpress.c to platforms/vexpress.c;
- move platform_vexpress.h to arm-arm/platforms/vexpress.h.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/Makefile                    |    1 +
 xen/arch/arm/mode_switch.S               |    2 +-
 xen/arch/arm/platforms/vexpress.c        |  100 ++++++++++++++++++++++++++++++
 xen/include/asm-arm/platform_vexpress.h  |   17 -----
 xen/include/asm-arm/platforms/vexpress.h |   40 ++++++++++++
 5 files changed, 142 insertions(+), 18 deletions(-)
 create mode 100644 xen/arch/arm/platforms/vexpress.c
 delete mode 100644 xen/include/asm-arm/platform_vexpress.h
 create mode 100644 xen/include/asm-arm/platforms/vexpress.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 4c61b04..781e022 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -1,4 +1,5 @@
 subdir-y += lib
+subdir-y += platforms
 
 obj-y += dummy.o
 obj-y += early_printk.o
diff --git a/xen/arch/arm/mode_switch.S b/xen/arch/arm/mode_switch.S
index 7c3b357..ba42898 100644
--- a/xen/arch/arm/mode_switch.S
+++ b/xen/arch/arm/mode_switch.S
@@ -19,7 +19,7 @@
 
 #include <asm/config.h>
 #include <asm/page.h>
-#include <asm/platform_vexpress.h>
+#include <asm/platforms/vexpress.h>
 #include <asm/asm_defns.h>
 #include "gic.h"
 
diff --git a/xen/arch/arm/platforms/vexpress.c
b/xen/arch/arm/platforms/vexpress.c
new file mode 100644
index 0000000..b57b62e
--- /dev/null
+++ b/xen/arch/arm/platforms/vexpress.c
@@ -0,0 +1,100 @@
+/*
+ * xen/arch/arm/platform_vexpress.c
+ *
+ * Versatile Express specific settings
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ * Copyright (c) 2013 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/platforms/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
+
+static inline int vexpress_ctrl_start(uint32_t *syscfg, int write,
+                                      int function, int device)
+{
+    int dcc = 0; /* DCC to access */
+    int site = 0; /* motherboard */
+    int position = 0; /* daughterboard */
+    uint32_t stat;
+
+    /* set control register */
+    syscfg[V2M_SYS_CFGCTRL/4] = V2M_SYS_CFG_START |
+        (write ? V2M_SYS_CFG_WRITE : 0) |
+        (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/4];
+        dsb();
+    } while ( !(stat & V2M_SYS_CFG_COMPLETE) );
+
+    /* check error status and return error flag if set */
+    if ( stat & V2M_SYS_CFG_ERROR )
+    {
+        printk(KERN_ERR "V2M SYS_CFGSTAT reported a configuration
error\n");
+        return -1;
+    }
+    return 0;
+}
+
+int vexpress_syscfg(int write, int function, int device, uint32_t *data)
+{
+    uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC);
+    int ret = -1;
+
+    set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED);
+
+    if ( syscfg[V2M_SYS_CFGCTRL/4] & V2M_SYS_CFG_START )
+        goto out;
+
+    /* clear the complete bit in the V2M_SYS_CFGSTAT status register */
+    syscfg[V2M_SYS_CFGSTAT/4] = 0;
+
+    if ( write )
+    {
+        /* write data */
+        syscfg[V2M_SYS_CFGDATA/4] = *data;
+
+        if ( vexpress_ctrl_start(syscfg, write, function, device) < 0 )
+            goto out;
+    } else {
+        if ( vexpress_ctrl_start(syscfg, write, function, device) < 0 )
+            goto out;
+        else
+            /* read data */
+            *data = syscfg[V2M_SYS_CFGDATA/4];
+    }
+
+    ret = 0;
+out:
+    clear_fixmap(FIXMAP_MISC);
+    return ret;
+}
+
+/*
+ * 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
deleted file mode 100644
index 3556af3..0000000
--- a/xen/include/asm-arm/platform_vexpress.h
+++ /dev/null
@@ -1,17 +0,0 @@
-#ifndef __ASM_ARM_PLATFORM_H
-#define __ASM_ARM_PLATFORM_H
-
-/* V2M */
-#define V2M_SYS_MMIO_BASE     (0x1c010000)
-#define V2M_SYS_FLAGSSET      (0x30)
-#define V2M_SYS_FLAGSCLR      (0x34)
-
-#endif /* __ASM_ARM_PLATFORM_H */
-/*
- * Local variables:
- * mode: C
- * c-set-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/asm-arm/platforms/vexpress.h
b/xen/include/asm-arm/platforms/vexpress.h
new file mode 100644
index 0000000..e464913
--- /dev/null
+++ b/xen/include/asm-arm/platforms/vexpress.h
@@ -0,0 +1,40 @@
+#ifndef __ASM_ARM_PLATFORMS_VEXPRESS_H
+#define __ASM_ARM_PLATFORMS_VEXPRESS_H
+
+/* V2M */
+#define V2M_SYS_MMIO_BASE     (0x1c010000)
+#define V2M_SYS_FLAGSSET      (0x30)
+#define V2M_SYS_FLAGSCLR      (0x34)
+
+#define V2M_SYS_CFGDATA       (0x00A0)
+#define V2M_SYS_CFGCTRL       (0x00A4)
+#define V2M_SYS_CFGSTAT       (0x00A8)
+
+#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_PLATFORMS_VEXPRESS_H */
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.2.5

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 v4:
- use a lookup table to set the color masks;
- fix indentation;
- make sure mode_string is not NULL and is not bigger than 16 chars
before continuing;
- introduce 2 separate error messages for !hdlcd_start and
!framebuffer_start at the beginning of video_init;
- mark get_color_masks and set_pixclock as __init;
- check that we are running on a vexpress machine before calling
vexpress_syscfg.

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 |  280 +++++++++++++++++++++++++++++++++++++++++
 xen/drivers/video/modelines.h |   77 +++++++++++
 xen/include/asm-arm/config.h  |    2 +
 5 files changed, 361 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 77f9d5d..a756292 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) += lfb.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..1057f5d
--- /dev/null
+++ b/xen/drivers/video/arm_hdlcd.c
@@ -0,0 +1,280 @@
+/*
+ * xen/drivers/video/arm_hdlcd.c
+ *
+ * Driver for ARM HDLCD Controller
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ * Copyright (c) 2013 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/platforms/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 "lfb.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)
+
+struct color_masks {
+    int red_shift;
+    int red_size;
+    int green_shift;
+    int green_size;
+    int blue_shift;
+    int blue_size;
+};
+
+struct pixel_colors {
+    const char* bpp;
+    struct color_masks colors;
+};
+
+struct pixel_colors __initdata colors[] = {
+    { "16", { 0, 5, 11, 5, 6, 5 } },
+    { "24", { 0, 8, 16, 8, 8, 8 } },
+    { "32", { 0, 8, 16, 8, 8, 8 } },
+};
+
+static void vga_noop_puts(const char *s) {}
+void (*video_puts)(const char *) = vga_noop_puts;
+
+static void hdlcd_flush(void)
+{
+    dsb();
+}
+
+static int __init get_color_masks(const char* bpp, struct color_masks **masks)
+{
+    int i;
+    for ( i = 0; i < ARRAY_SIZE(colors); i++ )
+    {
+        if ( !strncmp(colors[i].bpp, bpp, 2) )
+        {
+            *masks = &colors[i].colors;
+            return 0;
+        }
+    }
+    return -1;
+}
+
+static void __init set_pixclock(uint32_t pixclock)
+{
+    if ( device_tree_node_compatible(device_tree_flattened, 0,
"arm,vexpress") )
+            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 lfb_prop lfbp;
+    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 bytes_per_pixel = 4;
+    struct color_masks *c = NULL;
+    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); 
+
+    if ( !hdlcd_start )
+    {
+        printk(KERN_ERR "HDLCD address missing from device tree, disabling
driver\n");
+        return;
+    }
+
+    if ( !hdlcd_start || !framebuffer_start )
+    {
+        printk(KERN_ERR "HDLCD: framebuffer address missing from device
tree, disabling driver\n");
+        return;
+    }
+
+    mode_string = fdt_getprop(device_tree_flattened, node, "mode",
NULL);
+    if ( !mode_string )
+    {
+        get_color_masks("32", &c);
+        memcpy(_mode_string, "1280x1024@60",
strlen("1280x1024@60"));
+        bytes_per_pixel = 4;
+    }
+    else if ( strlen(mode_string) < strlen("800x600@60") ||
+            strlen(mode_string) > sizeof(_mode_string) - 1 )
+    {
+        printk(KERN_ERR "HDLCD: invalid modeline=%s\n", mode_string);
+        return;
+    } else {
+        char *s = strchr(mode_string, ''-'');
+        if ( !s )
+        {
+            printk(KERN_INFO "HDLCD: bpp not found in modeline %s, assume
32 bpp\n",
+                    mode_string);
+            get_color_masks("32", &c);
+            memcpy(_mode_string, mode_string, strlen(mode_string) + 1);
+            bytes_per_pixel = 4;
+        } else {
+            if ( strlen(s) < 6 )
+            {
+                printk(KERN_ERR "HDLCD: invalid mode %s\n",
mode_string);
+                return;
+            }
+            s++;
+            if ( get_color_masks(s, &c) < 0 )
+            {
+                printk(KERN_WARNING "HDLCD: unsupported bpp %s\n",
s);
+                return;
+            }
+            bytes_per_pixel = simple_strtoll(s, NULL, 10) / 8;
+        }
+        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(KERN_WARNING "HDLCD: unsupported videomode %s\n",
_mode_string);
+        return;
+    }
+
+    if ( framebuffer_size < bytes_per_pixel * videomode->xres *
videomode->yres )
+    {
+        printk(KERN_ERR "HDLCD: the framebuffer is too small, disabling
the HDLCD driver\n");
+        return;
+    }
+
+    printk(KERN_INFO "Initializing HDLCD driver\n");
+
+    lfb = early_ioremap(framebuffer_start, framebuffer_size, DEV_WC);
+    if ( !lfb )
+    {
+        printk(KERN_ERR "Couldn''t map the framebuffer\n");
+        return;
+    }
+    memset(lfb, 0x00, bytes_per_pixel * 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 * bytes_per_pixel;
+    HDLCD[HDLCD_LINECOUNT] = videomode->yres - 1;
+    HDLCD[HDLCD_LINEPITCH] = videomode->xres * bytes_per_pixel;
+    HDLCD[HDLCD_PF] = ((bytes_per_pixel - 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] = (c->red_size << 8) | c->red_shift;
+    HDLCD[HDLCD_GREEN] = (c->green_size << 8) | c->green_shift;
+    HDLCD[HDLCD_BLUE] = (c->blue_size << 8) | c->blue_shift;
+    HDLCD[HDLCD_COMMAND] = 1;
+    clear_fixmap(FIXMAP_MISC);
+
+    lfbp.pixel_on = (((1 << c->red_size) - 1) <<
c->red_shift) |
+        (((1 << c->green_size) - 1) << c->green_shift) |
+        (((1 << c->blue_size) - 1) << c->blue_shift);
+    lfbp.lfb = lfb;
+    lfbp.font = &font_vga_8x16;
+    lfbp.bits_per_pixel = bytes_per_pixel*8;
+    lfbp.bytes_per_line = bytes_per_pixel*videomode->xres;
+    lfbp.width = videomode->xres;
+    lfbp.height = videomode->yres;
+    lfbp.flush = hdlcd_flush;
+    lfbp.text_columns = videomode->xres / 8;
+    lfbp.text_rows = videomode->yres / 16;
+    if ( lfb_init(&lfbp) < 0 )
+            return;
+    video_puts = lfb_scroll_puts;
+}
+
+void video_endboot(void)
+{
+    if ( video_puts != vga_noop_puts )
+        lfb_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..9cb7cdd
--- /dev/null
+++ b/xen/drivers/video/modelines.h
@@ -0,0 +1,77 @@
+/*
+ * xen/drivers/video/modelines.h
+ *
+ * Timings for many popular monitor resolutions
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the
"Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 1999 by The XFree86 Project, Inc.
+ * Copyright (c) 2013 Citrix Systems
+ */
+
+#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 e5dce5e..8e934a4 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

>>> On 08.01.13 at 21:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> --- /dev/null
> +++ b/xen/drivers/video/lfb.c
> @@ -0,0 +1,206 @@
>
+/******************************************************************************
> + * lfb.c
> + *
> + * linear frame buffer handling.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/kernel.h>
> +#include <xen/lib.h>
> +#include <xen/errno.h>
> +#include "lfb.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)];
Imo it would be better to move all these __initdata definitions
do to where the using __init functions actually live, to make
sure unintended use of them in non-__init ones is noticed.
> +
> +struct lfb_status {
> +    struct lfb_prop lfbp;
> +
> +    unsigned char *lbuf, *text_buf;
> +    unsigned int *line_len;
> +    unsigned int xpos, ypos;
> +};
> +static struct lfb_status lfb;
> +
> +static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
> +
> +    for ( i = 0; i < lfb.lfbp.font->height; i++ )
> +    {
> +        unsigned char *ptr = lfb.lbuf;
> +
> +        for ( j = 0; j < nr_chars; j++ )
> +        {
> +            const unsigned char *bits = lfb.lfbp.font->data;
> +            bits += ((text_line[j] * lfb.lfbp.font->height + i) *
> +                     ((lfb.lfbp.font->width + 7) >> 3));
> +            for ( b = lfb.lfbp.font->width; b--; )
> +            {
> +                pixel = (*bits & (1u<<b)) ? lfb.lfbp.pixel_on :
0;
> +                memcpy(ptr, &pixel, bpp);
> +                ptr += bpp;
> +            }
> +        }
> +
> +        memset(ptr, 0, (lfb.lfbp.width - nr_chars *
lfb.lfbp.font->width) * bpp);
> +        memcpy(video_line, lfb.lbuf, nr_cells * lfb.lfbp.font->width *
bpp);
> +        video_line += lfb.lfbp.bytes_per_line;
> +    }
> +}
> +
> +/* Fast mode which redraws all modified parts of a 2D text buffer. */
> +void lfb_redraw_puts(const char *s)
> +{
> +    unsigned int i, min_redraw_y = lfb.ypos;
> +    char c;
> +
> +    /* Paste characters into text buffer. */
> +    while ( (c = *s++) != ''\0'' )
> +    {
> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
> +        {
> +            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
> +            {
> +                min_redraw_y = 0;
> +                lfb.ypos = lfb.lfbp.text_rows - 1;
> +                memmove(lfb.text_buf, lfb.text_buf +
lfb.lfbp.text_columns,
> +                        lfb.ypos * lfb.lfbp.text_columns);
> +                memset(lfb.text_buf + lfb.ypos * lfb.lfbp.text_columns, 0,
lfb.xpos);
> +            }
> +            lfb.xpos = 0;
> +        }
> +
> +        if ( c != ''\n'' )
> +            lfb.text_buf[lfb.xpos++ + lfb.ypos * lfb.lfbp.text_columns] =
c;
> +    }
> +
> +    /* Render modified section of text buffer to VESA linear framebuffer.
*/
> +    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
> +    {
> +        const unsigned char *line = lfb.text_buf + i *
lfb.lfbp.text_columns;
> +        unsigned int width;
> +
> +        for ( width = lfb.lfbp.text_columns; width; --width )
> +            if ( line[width - 1] )
> +                 break;
> +        lfb_show_line(line,
> +                       lfb.lfbp.lfb + i * lfb.lfbp.font->height *
lfb.lfbp.bytes_per_line,
> +                       width, max(lfb.line_len[i], width));
> +        lfb.line_len[i] = width;
> +    }
> +
> +    lfb.lfbp.flush();
> +}
> +
> +/* Slower line-based scroll mode which interacts better with dom0. */
> +void lfb_scroll_puts(const char *s)
> +{
> +    unsigned int i;
> +    char c;
> +
> +    while ( (c = *s++) != ''\0'' )
> +    {
> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
> +        {
> +            unsigned int bytes = (lfb.lfbp.width *
> +                                  ((lfb.lfbp.bits_per_pixel + 7) >>
3));
> +            unsigned char *src = lfb.lfbp.lfb + lfb.lfbp.font->height *
lfb.lfbp.bytes_per_line;
> +            unsigned char *dst = lfb.lfbp.lfb;
> +
> +            /* New line: scroll all previous rows up one line. */
> +            for ( i = lfb.lfbp.font->height; i < lfb.lfbp.height;
i++ )
> +            {
> +                memcpy(dst, src, bytes);
> +                src += lfb.lfbp.bytes_per_line;
> +                dst += lfb.lfbp.bytes_per_line;
> +            }
> +
> +            /* Render new line. */
> +            lfb_show_line(
> +                lfb.text_buf,
> +                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
Long line?
> +                lfb.xpos, lfb.lfbp.text_columns);
> +
> +            lfb.xpos = 0;
> +        }
> +
> +        if ( c != ''\n'' )
> +            lfb.text_buf[lfb.xpos++] = c;
> +    }
> +
> +    lfb.lfbp.flush();
> +}
> +
> +void lfb_carriage_return(void)
> +{
> +    lfb.xpos = 0;
> +}
> +
> +int __init lfb_init(struct lfb_prop *lfbp)
> +{
> +    if ( lfbp->width > MAX_XRES || lfbp->height > MAX_YRES )
> +    {
> +        printk(XENLOG_WARNING "Couldn''t initialize a %ux%u
framebuffer early.\n",
> +                        lfbp->width, lfbp->height);
Indentation?
> +        return -EINVAL;
> +    }
> +
> +    lfb.lfbp = *lfbp;
> +    lfb.lbuf = lbuf;
> +    lfb.text_buf = text_buf;
> +    lfb.line_len = line_len;
> +    return 0;
> +}
> +
> +int __init lfb_alloc(void)
> +{
> +    lfb.lbuf = NULL;
> +    lfb.text_buf = NULL;
> +    lfb.line_len = NULL;
> +
> +    lfb.lbuf = xmalloc_bytes(lfb.lfbp.bytes_per_line);
> +    if ( !lfb.lbuf )
> +        goto fail;
> +
> +    lfb.text_buf = xzalloc_bytes(lfb.lfbp.text_columns *
lfb.lfbp.text_rows);
> +    if ( !lfb.text_buf )
> +        goto fail;
> +
> +    lfb.line_len = xzalloc_array(unsigned int, lfb.lfbp.text_columns);
> +    if ( !lfb.line_len )
> +        goto fail;
> +
> +    memcpy(lfb.lbuf, lbuf, lfb.lfbp.bytes_per_line);
> +    memcpy(lfb.text_buf, text_buf, lfb.lfbp.text_columns *
lfb.lfbp.text_rows);
> +    memcpy(lfb.line_len, line_len, lfb.lfbp.text_columns);
> +
> +    return 0;
> +
> +fail:
> +    printk(XENLOG_ERR "Couldn''t allocate enough memory to
drive the framebuffer\n");
> +    lfb_free();
> +
> +    return -ENOMEM;
> +}
> +
> +void lfb_free(void)
> +{
> +    xfree(lfb.lbuf);
> +    xfree(lfb.text_buf);
> +    xfree(lfb.line_len);
> +}

This function would then perhaps better go before the __init stuff.
> @@ -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 ( lfb_init(&lfbp) < 0 )
> +        return;
> +    if ( lfb_alloc() < 0 )
> +        return;
What''s the point of calling lfb_alloc() right after lfb_init()? The
static
buffers set up by lfb_init() will get replaced by dynamically allocated
ones right away. Or in other words - what do you need the static
buffers for in the first place?

Jan
> +    video_puts = lfb_redraw_puts;
>  }
>  
>  #include <asm/mtrr.h>

On Wed, 9 Jan 2013, Jan Beulich wrote:
> >>> On 08.01.13 at 21:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> > --- /dev/null
> > +++ b/xen/drivers/video/lfb.c
> > @@ -0,0 +1,206 @@
> >
+/******************************************************************************
> > + * lfb.c
> > + *
> > + * linear frame buffer handling.
> > + */
> > +
> > +#include <xen/config.h>
> > +#include <xen/kernel.h>
> > +#include <xen/lib.h>
> > +#include <xen/errno.h>
> > +#include "lfb.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)];
> 
> Imo it would be better to move all these __initdata definitions
> do to where the using __init functions actually live, to make
> sure unintended use of them in non-__init ones is noticed.
OK

> > +
> > +struct lfb_status {
> > +    struct lfb_prop lfbp;
> > +
> > +    unsigned char *lbuf, *text_buf;
> > +    unsigned int *line_len;
> > +    unsigned int xpos, ypos;
> > +};
> > +static struct lfb_status lfb;
> > +
> > +static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
> > +
> > +    for ( i = 0; i < lfb.lfbp.font->height; i++ )
> > +    {
> > +        unsigned char *ptr = lfb.lbuf;
> > +
> > +        for ( j = 0; j < nr_chars; j++ )
> > +        {
> > +            const unsigned char *bits = lfb.lfbp.font->data;
> > +            bits += ((text_line[j] * lfb.lfbp.font->height + i) *
> > +                     ((lfb.lfbp.font->width + 7) >> 3));
> > +            for ( b = lfb.lfbp.font->width; b--; )
> > +            {
> > +                pixel = (*bits & (1u<<b)) ?
lfb.lfbp.pixel_on : 0;
> > +                memcpy(ptr, &pixel, bpp);
> > +                ptr += bpp;
> > +            }
> > +        }
> > +
> > +        memset(ptr, 0, (lfb.lfbp.width - nr_chars *
lfb.lfbp.font->width) * bpp);
> > +        memcpy(video_line, lfb.lbuf, nr_cells *
lfb.lfbp.font->width * bpp);
> > +        video_line += lfb.lfbp.bytes_per_line;
> > +    }
> > +}
> > +
> > +/* Fast mode which redraws all modified parts of a 2D text buffer. */
> > +void lfb_redraw_puts(const char *s)
> > +{
> > +    unsigned int i, min_redraw_y = lfb.ypos;
> > +    char c;
> > +
> > +    /* Paste characters into text buffer. */
> > +    while ( (c = *s++) != ''\0'' )
> > +    {
> > +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
> > +        {
> > +            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
> > +            {
> > +                min_redraw_y = 0;
> > +                lfb.ypos = lfb.lfbp.text_rows - 1;
> > +                memmove(lfb.text_buf, lfb.text_buf +
lfb.lfbp.text_columns,
> > +                        lfb.ypos * lfb.lfbp.text_columns);
> > +                memset(lfb.text_buf + lfb.ypos *
lfb.lfbp.text_columns, 0, lfb.xpos);
> > +            }
> > +            lfb.xpos = 0;
> > +        }
> > +
> > +        if ( c != ''\n'' )
> > +            lfb.text_buf[lfb.xpos++ + lfb.ypos *
lfb.lfbp.text_columns] = c;
> > +    }
> > +
> > +    /* Render modified section of text buffer to VESA linear
framebuffer. */
> > +    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
> > +    {
> > +        const unsigned char *line = lfb.text_buf + i *
lfb.lfbp.text_columns;
> > +        unsigned int width;
> > +
> > +        for ( width = lfb.lfbp.text_columns; width; --width )
> > +            if ( line[width - 1] )
> > +                 break;
> > +        lfb_show_line(line,
> > +                       lfb.lfbp.lfb + i * lfb.lfbp.font->height *
lfb.lfbp.bytes_per_line,
> > +                       width, max(lfb.line_len[i], width));
> > +        lfb.line_len[i] = width;
> > +    }
> > +
> > +    lfb.lfbp.flush();
> > +}
> > +
> > +/* Slower line-based scroll mode which interacts better with dom0. */
> > +void lfb_scroll_puts(const char *s)
> > +{
> > +    unsigned int i;
> > +    char c;
> > +
> > +    while ( (c = *s++) != ''\0'' )
> > +    {
> > +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
> > +        {
> > +            unsigned int bytes = (lfb.lfbp.width *
> > +                                  ((lfb.lfbp.bits_per_pixel + 7)
>> 3));
> > +            unsigned char *src = lfb.lfbp.lfb +
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
> > +            unsigned char *dst = lfb.lfbp.lfb;
> > +
> > +            /* New line: scroll all previous rows up one line. */
> > +            for ( i = lfb.lfbp.font->height; i <
lfb.lfbp.height; i++ )
> > +            {
> > +                memcpy(dst, src, bytes);
> > +                src += lfb.lfbp.bytes_per_line;
> > +                dst += lfb.lfbp.bytes_per_line;
> > +            }
> > +
> > +            /* Render new line. */
> > +            lfb_show_line(
> > +                lfb.text_buf,
> > +                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
> 
> Long line?
Right

> > +                lfb.xpos, lfb.lfbp.text_columns);
> > +
> > +            lfb.xpos = 0;
> > +        }
> > +
> > +        if ( c != ''\n'' )
> > +            lfb.text_buf[lfb.xpos++] = c;
> > +    }
> > +
> > +    lfb.lfbp.flush();
> > +}
> > +
> > +void lfb_carriage_return(void)
> > +{
> > +    lfb.xpos = 0;
> > +}
> > +
> > +int __init lfb_init(struct lfb_prop *lfbp)
> > +{
> > +    if ( lfbp->width > MAX_XRES || lfbp->height >
MAX_YRES )
> > +    {
> > +        printk(XENLOG_WARNING "Couldn''t initialize a
%ux%u framebuffer early.\n",
> > +                        lfbp->width, lfbp->height);
> 
> Indentation?
OK

> > +        return -EINVAL;
> > +    }
> > +
> > +    lfb.lfbp = *lfbp;
> > +    lfb.lbuf = lbuf;
> > +    lfb.text_buf = text_buf;
> > +    lfb.line_len = line_len;
> > +    return 0;
> > +}
> > +
> > +int __init lfb_alloc(void)
> > +{
> > +    lfb.lbuf = NULL;
> > +    lfb.text_buf = NULL;
> > +    lfb.line_len = NULL;
> > +
> > +    lfb.lbuf = xmalloc_bytes(lfb.lfbp.bytes_per_line);
> > +    if ( !lfb.lbuf )
> > +        goto fail;
> > +
> > +    lfb.text_buf = xzalloc_bytes(lfb.lfbp.text_columns *
lfb.lfbp.text_rows);
> > +    if ( !lfb.text_buf )
> > +        goto fail;
> > +
> > +    lfb.line_len = xzalloc_array(unsigned int,
lfb.lfbp.text_columns);
> > +    if ( !lfb.line_len )
> > +        goto fail;
> > +
> > +    memcpy(lfb.lbuf, lbuf, lfb.lfbp.bytes_per_line);
> > +    memcpy(lfb.text_buf, text_buf, lfb.lfbp.text_columns *
lfb.lfbp.text_rows);
> > +    memcpy(lfb.line_len, line_len, lfb.lfbp.text_columns);
> > +
> > +    return 0;
> > +
> > +fail:
> > +    printk(XENLOG_ERR "Couldn''t allocate enough memory
to drive the framebuffer\n");
> > +    lfb_free();
> > +
> > +    return -ENOMEM;
> > +}
> > +
> > +void lfb_free(void)
> > +{
> > +    xfree(lfb.lbuf);
> > +    xfree(lfb.text_buf);
> > +    xfree(lfb.line_len);
> > +}
> 
> 
> This function would then perhaps better go before the __init stuff.
> 
> > @@ -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 ( lfb_init(&lfbp) < 0 )
> > +        return;
> > +    if ( lfb_alloc() < 0 )
> > +        return;
> 
> What''s the point of calling lfb_alloc() right after lfb_init()?
The static
> buffers set up by lfb_init() will get replaced by dynamically allocated
> ones right away. Or in other words - what do you need the static
> buffers for in the first place?
The idea was that you could have output on the screen before xmalloc
works.

Given that vesa_init is always called after xmalloc is available we call
lfb_alloc() right away. Thanks to the fact that I moved setup_mm()
earlier in the ARM version of start_xen, that should be now possible for the
arm_hdlcd driver too. I''ll to remove lfb_alloc.

On 09/01/13 14:05, Stefano Stabellini wrote:
> On Wed, 9 Jan 2013, Jan Beulich wrote:
>>>>> On 08.01.13 at 21:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
>>> --- /dev/null
>>> +++ b/xen/drivers/video/lfb.c
>>> @@ -0,0 +1,206 @@
>>>
+/******************************************************************************
>>> + * lfb.c
>>> + *
>>> + * linear frame buffer handling.
>>> + */
>>> +
>>> +#include <xen/config.h>
>>> +#include <xen/kernel.h>
>>> +#include <xen/lib.h>
>>> +#include <xen/errno.h>
>>> +#include "lfb.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)];
>> Imo it would be better to move all these __initdata definitions
>> do to where the using __init functions actually live, to make
>> sure unintended use of them in non-__init ones is noticed.
> OK
>
>
>>> +
>>> +struct lfb_status {
>>> +    struct lfb_prop lfbp;
>>> +
>>> +    unsigned char *lbuf, *text_buf;
>>> +    unsigned int *line_len;
>>> +    unsigned int xpos, ypos;
>>> +};
>>> +static struct lfb_status lfb;
>>> +
>>> +static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
>>> +
>>> +    for ( i = 0; i < lfb.lfbp.font->height; i++ )
>>> +    {
>>> +        unsigned char *ptr = lfb.lbuf;
>>> +
>>> +        for ( j = 0; j < nr_chars; j++ )
>>> +        {
>>> +            const unsigned char *bits = lfb.lfbp.font->data;
>>> +            bits += ((text_line[j] * lfb.lfbp.font->height + i)
*
>>> +                     ((lfb.lfbp.font->width + 7) >> 3));
>>> +            for ( b = lfb.lfbp.font->width; b--; )
>>> +            {
>>> +                pixel = (*bits & (1u<<b)) ?
lfb.lfbp.pixel_on : 0;
>>> +                memcpy(ptr, &pixel, bpp);
>>> +                ptr += bpp;
>>> +            }
>>> +        }
>>> +
>>> +        memset(ptr, 0, (lfb.lfbp.width - nr_chars *
lfb.lfbp.font->width) * bpp);
>>> +        memcpy(video_line, lfb.lbuf, nr_cells *
lfb.lfbp.font->width * bpp);
>>> +        video_line += lfb.lfbp.bytes_per_line;
>>> +    }
>>> +}
>>> +
>>> +/* Fast mode which redraws all modified parts of a 2D text buffer.
*/
>>> +void lfb_redraw_puts(const char *s)
>>> +{
>>> +    unsigned int i, min_redraw_y = lfb.ypos;
>>> +    char c;
>>> +
>>> +    /* Paste characters into text buffer. */
>>> +    while ( (c = *s++) != ''\0'' )
>>> +    {
>>> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
>>> +        {
>>> +            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
>>> +            {
>>> +                min_redraw_y = 0;
>>> +                lfb.ypos = lfb.lfbp.text_rows - 1;
>>> +                memmove(lfb.text_buf, lfb.text_buf +
lfb.lfbp.text_columns,
>>> +                        lfb.ypos * lfb.lfbp.text_columns);
>>> +                memset(lfb.text_buf + lfb.ypos *
lfb.lfbp.text_columns, 0, lfb.xpos);
>>> +            }
>>> +            lfb.xpos = 0;
>>> +        }
>>> +
>>> +        if ( c != ''\n'' )
>>> +            lfb.text_buf[lfb.xpos++ + lfb.ypos *
lfb.lfbp.text_columns] = c;
>>> +    }
>>> +
>>> +    /* Render modified section of text buffer to VESA linear
framebuffer. */
>>> +    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
>>> +    {
>>> +        const unsigned char *line = lfb.text_buf + i *
lfb.lfbp.text_columns;
>>> +        unsigned int width;
>>> +
>>> +        for ( width = lfb.lfbp.text_columns; width; --width )
>>> +            if ( line[width - 1] )
>>> +                 break;
>>> +        lfb_show_line(line,
>>> +                       lfb.lfbp.lfb + i * lfb.lfbp.font->height
* lfb.lfbp.bytes_per_line,
>>> +                       width, max(lfb.line_len[i], width));
>>> +        lfb.line_len[i] = width;
>>> +    }
>>> +
>>> +    lfb.lfbp.flush();
>>> +}
>>> +
>>> +/* Slower line-based scroll mode which interacts better with dom0.
*/
>>> +void lfb_scroll_puts(const char *s)
>>> +{
>>> +    unsigned int i;
>>> +    char c;
>>> +
>>> +    while ( (c = *s++) != ''\0'' )
>>> +    {
>>> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
>>> +        {
>>> +            unsigned int bytes = (lfb.lfbp.width *
>>> +                                  ((lfb.lfbp.bits_per_pixel + 7)
>> 3));
>>> +            unsigned char *src = lfb.lfbp.lfb +
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
>>> +            unsigned char *dst = lfb.lfbp.lfb;
>>> +
>>> +            /* New line: scroll all previous rows up one line. */
>>> +            for ( i = lfb.lfbp.font->height; i <
lfb.lfbp.height; i++ )
>>> +            {
>>> +                memcpy(dst, src, bytes);
>>> +                src += lfb.lfbp.bytes_per_line;
>>> +                dst += lfb.lfbp.bytes_per_line;
>>> +            }
>>> +
>>> +            /* Render new line. */
>>> +            lfb_show_line(
>>> +                lfb.text_buf,
>>> +                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
>> Long line?
> Right
>
The calculation of

lfb.lfbp.lfb + lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;

is done twice in the above section - surely that''s constant over time,
so could be stored in another local variable - thus making the line shorter and
at the same time more obvious that "it''s the same thing".

--
Mats

On 09/01/13 14:45, Mats Petersson wrote:
> On 09/01/13 14:05, Stefano Stabellini wrote:
>> On Wed, 9 Jan 2013, Jan Beulich wrote:
>>>>>> On 08.01.13 at 21:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
>>>> --- /dev/null
>>>> +++ b/xen/drivers/video/lfb.c
>>>> @@ -0,0 +1,206 @@
>>>>
+/******************************************************************************
>>>> + * lfb.c
>>>> + *
>>>> + * linear frame buffer handling.
>>>> + */
>>>> +
>>>> +#include <xen/config.h>
>>>> +#include <xen/kernel.h>
>>>> +#include <xen/lib.h>
>>>> +#include <xen/errno.h>
>>>> +#include "lfb.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)];
>>> Imo it would be better to move all these __initdata definitions
>>> do to where the using __init functions actually live, to make
>>> sure unintended use of them in non-__init ones is noticed.
>> OK
>>
>>
>>>> +
>>>> +struct lfb_status {
>>>> +    struct lfb_prop lfbp;
>>>> +
>>>> +    unsigned char *lbuf, *text_buf;
>>>> +    unsigned int *line_len;
>>>> +    unsigned int xpos, ypos;
>>>> +};
>>>> +static struct lfb_status lfb;
>>>> +
>>>> +static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
>>>> +
>>>> +    for ( i = 0; i < lfb.lfbp.font->height; i++ )
>>>> +    {
>>>> +        unsigned char *ptr = lfb.lbuf;
>>>> +
>>>> +        for ( j = 0; j < nr_chars; j++ )
>>>> +        {
>>>> +            const unsigned char *bits =
lfb.lfbp.font->data;
>>>> +            bits += ((text_line[j] * lfb.lfbp.font->height
+ i) *
>>>> +                     ((lfb.lfbp.font->width + 7) >>
3));
>>>> +            for ( b = lfb.lfbp.font->width; b--; )
>>>> +            {
>>>> +                pixel = (*bits & (1u<<b)) ?
lfb.lfbp.pixel_on : 0;
>>>> +                memcpy(ptr, &pixel, bpp);
>>>> +                ptr += bpp;
>>>> +            }
>>>> +        }
>>>> +
>>>> +        memset(ptr, 0, (lfb.lfbp.width - nr_chars *
lfb.lfbp.font->width) * bpp);
>>>> +        memcpy(video_line, lfb.lbuf, nr_cells *
lfb.lfbp.font->width * bpp);
>>>> +        video_line += lfb.lfbp.bytes_per_line;
>>>> +    }
>>>> +}
>>>> +
>>>> +/* Fast mode which redraws all modified parts of a 2D text
buffer. */
>>>> +void lfb_redraw_puts(const char *s)
>>>> +{
>>>> +    unsigned int i, min_redraw_y = lfb.ypos;
>>>> +    char c;
>>>> +
>>>> +    /* Paste characters into text buffer. */
>>>> +    while ( (c = *s++) != ''\0'' )
>>>> +    {
>>>> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
>>>> +        {
>>>> +            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
>>>> +            {
>>>> +                min_redraw_y = 0;
>>>> +                lfb.ypos = lfb.lfbp.text_rows - 1;
>>>> +                memmove(lfb.text_buf, lfb.text_buf +
lfb.lfbp.text_columns,
>>>> +                        lfb.ypos * lfb.lfbp.text_columns);
>>>> +                memset(lfb.text_buf + lfb.ypos *
lfb.lfbp.text_columns, 0, lfb.xpos);
>>>> +            }
>>>> +            lfb.xpos = 0;
>>>> +        }
>>>> +
>>>> +        if ( c != ''\n'' )
>>>> +            lfb.text_buf[lfb.xpos++ + lfb.ypos *
lfb.lfbp.text_columns] = c;
>>>> +    }
>>>> +
>>>> +    /* Render modified section of text buffer to VESA linear
framebuffer. */
>>>> +    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
>>>> +    {
>>>> +        const unsigned char *line = lfb.text_buf + i *
lfb.lfbp.text_columns;
>>>> +        unsigned int width;
>>>> +
>>>> +        for ( width = lfb.lfbp.text_columns; width; --width )
>>>> +            if ( line[width - 1] )
>>>> +                 break;
>>>> +        lfb_show_line(line,
>>>> +                       lfb.lfbp.lfb + i *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
>>>> +                       width, max(lfb.line_len[i], width));
>>>> +        lfb.line_len[i] = width;
>>>> +    }
>>>> +
>>>> +    lfb.lfbp.flush();
>>>> +}
>>>> +
>>>> +/* Slower line-based scroll mode which interacts better with
dom0. */
>>>> +void lfb_scroll_puts(const char *s)
>>>> +{
>>>> +    unsigned int i;
>>>> +    char c;
>>>> +
>>>> +    while ( (c = *s++) != ''\0'' )
>>>> +    {
>>>> +        if ( (c == ''\n'') || (lfb.xpos >=
lfb.lfbp.text_columns) )
>>>> +        {
>>>> +            unsigned int bytes = (lfb.lfbp.width *
>>>> +                                  ((lfb.lfbp.bits_per_pixel +
7) >> 3));
>>>> +            unsigned char *src = lfb.lfbp.lfb +
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
>>>> +            unsigned char *dst = lfb.lfbp.lfb;
>>>> +
>>>> +            /* New line: scroll all previous rows up one line.
*/
>>>> +            for ( i = lfb.lfbp.font->height; i <
lfb.lfbp.height; i++ )
>>>> +            {
>>>> +                memcpy(dst, src, bytes);
>>>> +                src += lfb.lfbp.bytes_per_line;
>>>> +                dst += lfb.lfbp.bytes_per_line;
>>>> +            }
>>>> +
>>>> +            /* Render new line. */
>>>> +            lfb_show_line(
>>>> +                lfb.text_buf,
>>>> +                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
>>> Long line?
>> Right
>>
> The calculation of
>
> lfb.lfbp.lfb + lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
>
> is done twice in the above section - surely that''s constant over
time, so could be stored in another local variable - thus making the line
shorter and at the same time more obvious that "it''s the same
thing".
No, it isn''t! I started out with

lfb.lfbp.font->height * lfb.lfbp.bytes_per_line

being calculated twice. Then mistakenly thought that it was MORE of the same,
missing out that the second calculation is different. Sorry for that. But the
font->hight * bytes_per_line calculation is definitely the same in both
places.

--
Mats
>
> --
> Mats
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>

On Wed, 9 Jan 2013, Mats Petersson wrote:
> On 09/01/13 14:45, Mats Petersson wrote:
> > On 09/01/13 14:05, Stefano Stabellini wrote:
> >> On Wed, 9 Jan 2013, Jan Beulich wrote:
> >>>>>> On 08.01.13 at 21:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> >>>> --- /dev/null
> >>>> +++ b/xen/drivers/video/lfb.c
> >>>> @@ -0,0 +1,206 @@
> >>>>
+/******************************************************************************
> >>>> + * lfb.c
> >>>> + *
> >>>> + * linear frame buffer handling.
> >>>> + */
> >>>> +
> >>>> +#include <xen/config.h>
> >>>> +#include <xen/kernel.h>
> >>>> +#include <xen/lib.h>
> >>>> +#include <xen/errno.h>
> >>>> +#include "lfb.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)];
> >>> Imo it would be better to move all these __initdata
definitions
> >>> do to where the using __init functions actually live, to make
> >>> sure unintended use of them in non-__init ones is noticed.
> >> OK
> >>
> >>
> >>>> +
> >>>> +struct lfb_status {
> >>>> +    struct lfb_prop lfbp;
> >>>> +
> >>>> +    unsigned char *lbuf, *text_buf;
> >>>> +    unsigned int *line_len;
> >>>> +    unsigned int xpos, ypos;
> >>>> +};
> >>>> +static struct lfb_status lfb;
> >>>> +
> >>>> +static void lfb_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 = (lfb.lfbp.bits_per_pixel + 7) >> 3;
> >>>> +
> >>>> +    for ( i = 0; i < lfb.lfbp.font->height; i++ )
> >>>> +    {
> >>>> +        unsigned char *ptr = lfb.lbuf;
> >>>> +
> >>>> +        for ( j = 0; j < nr_chars; j++ )
> >>>> +        {
> >>>> +            const unsigned char *bits =
lfb.lfbp.font->data;
> >>>> +            bits += ((text_line[j] *
lfb.lfbp.font->height + i) *
> >>>> +                     ((lfb.lfbp.font->width + 7)
>> 3));
> >>>> +            for ( b = lfb.lfbp.font->width; b--; )
> >>>> +            {
> >>>> +                pixel = (*bits & (1u<<b)) ?
lfb.lfbp.pixel_on : 0;
> >>>> +                memcpy(ptr, &pixel, bpp);
> >>>> +                ptr += bpp;
> >>>> +            }
> >>>> +        }
> >>>> +
> >>>> +        memset(ptr, 0, (lfb.lfbp.width - nr_chars *
lfb.lfbp.font->width) * bpp);
> >>>> +        memcpy(video_line, lfb.lbuf, nr_cells *
lfb.lfbp.font->width * bpp);
> >>>> +        video_line += lfb.lfbp.bytes_per_line;
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> +/* Fast mode which redraws all modified parts of a 2D
text buffer. */
> >>>> +void lfb_redraw_puts(const char *s)
> >>>> +{
> >>>> +    unsigned int i, min_redraw_y = lfb.ypos;
> >>>> +    char c;
> >>>> +
> >>>> +    /* Paste characters into text buffer. */
> >>>> +    while ( (c = *s++) != ''\0'' )
> >>>> +    {
> >>>> +        if ( (c == ''\n'') || (lfb.xpos
>= lfb.lfbp.text_columns) )
> >>>> +        {
> >>>> +            if ( ++lfb.ypos >= lfb.lfbp.text_rows )
> >>>> +            {
> >>>> +                min_redraw_y = 0;
> >>>> +                lfb.ypos = lfb.lfbp.text_rows - 1;
> >>>> +                memmove(lfb.text_buf, lfb.text_buf +
lfb.lfbp.text_columns,
> >>>> +                        lfb.ypos *
lfb.lfbp.text_columns);
> >>>> +                memset(lfb.text_buf + lfb.ypos *
lfb.lfbp.text_columns, 0, lfb.xpos);
> >>>> +            }
> >>>> +            lfb.xpos = 0;
> >>>> +        }
> >>>> +
> >>>> +        if ( c != ''\n'' )
> >>>> +            lfb.text_buf[lfb.xpos++ + lfb.ypos *
lfb.lfbp.text_columns] = c;
> >>>> +    }
> >>>> +
> >>>> +    /* Render modified section of text buffer to VESA
linear framebuffer. */
> >>>> +    for ( i = min_redraw_y; i <= lfb.ypos; i++ )
> >>>> +    {
> >>>> +        const unsigned char *line = lfb.text_buf + i *
lfb.lfbp.text_columns;
> >>>> +        unsigned int width;
> >>>> +
> >>>> +        for ( width = lfb.lfbp.text_columns; width;
--width )
> >>>> +            if ( line[width - 1] )
> >>>> +                 break;
> >>>> +        lfb_show_line(line,
> >>>> +                       lfb.lfbp.lfb + i *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
> >>>> +                       width, max(lfb.line_len[i],
width));
> >>>> +        lfb.line_len[i] = width;
> >>>> +    }
> >>>> +
> >>>> +    lfb.lfbp.flush();
> >>>> +}
> >>>> +
> >>>> +/* Slower line-based scroll mode which interacts better
with dom0. */
> >>>> +void lfb_scroll_puts(const char *s)
> >>>> +{
> >>>> +    unsigned int i;
> >>>> +    char c;
> >>>> +
> >>>> +    while ( (c = *s++) != ''\0'' )
> >>>> +    {
> >>>> +        if ( (c == ''\n'') || (lfb.xpos
>= lfb.lfbp.text_columns) )
> >>>> +        {
> >>>> +            unsigned int bytes = (lfb.lfbp.width *
> >>>> +                                 
((lfb.lfbp.bits_per_pixel + 7) >> 3));
> >>>> +            unsigned char *src = lfb.lfbp.lfb +
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
> >>>> +            unsigned char *dst = lfb.lfbp.lfb;
> >>>> +
> >>>> +            /* New line: scroll all previous rows up one
line. */
> >>>> +            for ( i = lfb.lfbp.font->height; i <
lfb.lfbp.height; i++ )
> >>>> +            {
> >>>> +                memcpy(dst, src, bytes);
> >>>> +                src += lfb.lfbp.bytes_per_line;
> >>>> +                dst += lfb.lfbp.bytes_per_line;
> >>>> +            }
> >>>> +
> >>>> +            /* Render new line. */
> >>>> +            lfb_show_line(
> >>>> +                lfb.text_buf,
> >>>> +                lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
> >>> Long line?
> >> Right
> >>
> > The calculation of
> >
> > lfb.lfbp.lfb + lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
> >
> > is done twice in the above section - surely that''s constant
over time, so could be stored in another local variable - thus making the line
shorter and at the same time more obvious that "it''s the same
thing".
> No, it isn''t! I started out with
> 
> lfb.lfbp.font->height * lfb.lfbp.bytes_per_line
> 
> being calculated twice. Then mistakenly thought that it was MORE of the
same, missing out that the second calculation is different. Sorry for that. But
the font->hight * bytes_per_line calculation is definitely the same in both
places.
Thanks for the review!

lfb_scroll_puts and lfb_show_line are just vesa_scroll_puts and
vesa_show_line renamed and with some basic variable name changes.
I would be reticent to change them in this patch unless necessary.

On Tue, 2013-01-08 at 20:03 +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_va, that flushes a range of virtual addresses.
> Replace all the calls to flush_xen_data_tlb_va with calls to
> flush_xen_data_tlb_range_va and remove flush_xen_data_tlb_va.
> 
> In the case of the dest_va tlb flush at the beginning of
> setup_pagetables, flush the entire 2MB mapping rather than just the
> first 4K.
> 
> 
> Changes in v4:
> - rename flush_xen_data_tlb_range to flush_xen_data_tlb_range_va;
> - replace all the calls to flush_xen_data_tlb_va, with calls to
> flush_xen_data_tlb_range_va;
> - flush the entire 2MB mapping at BOOT_MISC_VIRT_START rather than just
> the first 4k;
> - remove flush_xen_data_tlb_va;
These all seem like a separate patch (or patches) to me.
> - fix indentation;
> - rename EARLY_VMAP_START/END to EARLY_VMAP_VIRT_START/END;
> - mark early_ioremap as __init;
> - reduce the amount of casts in early_ioremap.
[...]
>  /*
> - * Flush one VA''s hypervisor mappings from the data TLB. This is
not
> + * 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_va(unsigned long va)
> +static inline void flush_xen_data_tlb_range_va(unsigned long va, unsigned
long size)
>  {
> -    asm volatile("dsb;" /* Ensure preceding are visible */
> -                 STORE_CP32(0, TLBIMVAH)
> -                 "dsb;" /* Ensure completion of the TLB flush */
> -                 "isb;"
> -                 : : "r" (va) : "memory");
> +    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;"
You can hoist all the barriers here to above/below the loop I think.

Ian.

On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini
wrote:
> - 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>
Jan, do you want to apply this or shall I take it along with the ARM
bits?

AIUI it has no dependency on patch 1/7.

Ian.

On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini
wrote:
> At the moment we destroy the DTB mappings we have in setup_pagetables
> and we restore them only in setup_mm.
> 
> Move setup_mm right after setup_pagetables so that we don''t leave
over
> portions of the initialization code without a working DTB mapping.
"leave over"? I think you just mean to say "This ensures we have
a valid
DTB mapping while running the subsequent initialization code" or
something like that.

Do you still get the :
        printk("Xen heap: %lu pages  Dom heap: %lu pages\n
message with this change? Or do all the printks in this function need to
become early_printk/early_panic?

Actually, this function has 1x early_printk, 1x printk and 1x panic,
it''s a bit confused I think ;-)
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/setup.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 2076724..8882514 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -212,6 +212,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>      cmdline_parse(device_tree_bootargs(fdt));
>  
>      setup_pagetables(boot_phys_offset, get_xen_paddr());
> +    setup_mm(atag_paddr, fdt_size);
>  
>  #ifdef EARLY_UART_ADDRESS
>      /* TODO Need to get device tree or command line for UART address */
> @@ -229,8 +230,6 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      init_xen_time();
>  
> -    setup_mm(atag_paddr, fdt_size);
> -
>      /* Setup Hyp vector base */
>      WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
>      printk("Set hyp vector base to %"PRIx32" (expected
%p)\n",

On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini
wrote:
> Introduce a Versatile Express specific function to read/write
> motherboard settings.
> 
> Changes in v4:
> - move the wait loop and the syscfg cfgctrl write into a separate
> function;
> - fix comments;
> - define all registers in write;
> - move platform_vexpress.c to platforms/vexpress.c;
> - move platform_vexpress.h to arm-arm/platforms/vexpress.h.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/Makefile                    |    1 +
>  xen/arch/arm/mode_switch.S               |    2 +-
>  xen/arch/arm/platforms/vexpress.c        |  100
++++++++++++++++++++++++++++++
Did you forget to git add xen/arch/arm/platforms/Makefile ?
>  xen/include/asm-arm/platform_vexpress.h  |   17 -----
>  xen/include/asm-arm/platforms/vexpress.h |   40 ++++++++++++
git send-email/format-patch -M is your friend here.

>>> On 10.01.13 at 16:49, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
>> - 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>
> 
> Jan, do you want to apply this or shall I take it along with the ARM
> bits?
> 
> AIUI it has no dependency on patch 1/7.
Neither you nor I can validly apply this without Keir''s ack,
I''m
afraid.

Jan

On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini
wrote:
> - 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>
Adding Keir for his ack/nack too.
> ---
>  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 */

On Thu, 2013-01-10 at 16:08 +0000, Jan Beulich wrote:
> >>> On 10.01.13 at 16:49, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
> >> - 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>
> > 
> > Jan, do you want to apply this or shall I take it along with the ARM
> > bits?
> > 
> > AIUI it has no dependency on patch 1/7.
> 
> Neither you nor I can validly apply this without Keir''s ack,
I''m
> afraid.
True. I''ve CCd him into the original mail.

Ian,

On 10/01/2013 16:08, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 10.01.13 at 16:49, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>> On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
>>> - 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>
>> 
>> Jan, do you want to apply this or shall I take it along with the ARM
>> bits?
>> 
>> AIUI it has no dependency on patch 1/7.
> 
> Neither you nor I can validly apply this without Keir''s ack,
I''m
> afraid.
Looks like just trivial renaming of stuff.

Acked-by: Keir Fraser <keir@xen.org>
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On Thu, 10 Jan 2013, Ian Campbell wrote:
> On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
> > Introduce a Versatile Express specific function to read/write
> > motherboard settings.
> > 
> > Changes in v4:
> > - move the wait loop and the syscfg cfgctrl write into a separate
> > function;
> > - fix comments;
> > - define all registers in write;
> > - move platform_vexpress.c to platforms/vexpress.c;
> > - move platform_vexpress.h to arm-arm/platforms/vexpress.h.
> > 
> > Signed-off-by: Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/arch/arm/Makefile                    |    1 +
> >  xen/arch/arm/mode_switch.S               |    2 +-
> >  xen/arch/arm/platforms/vexpress.c        |  100
++++++++++++++++++++++++++++++
> 
> Did you forget to git add xen/arch/arm/platforms/Makefile ?
oops

> >  xen/include/asm-arm/platform_vexpress.h  |   17 -----
> >  xen/include/asm-arm/platforms/vexpress.h |   40 ++++++++++++
> 
> git send-email/format-patch -M is your friend here.
> 
Ah! I''ll use it next time.

On Thu, 10 Jan 2013, Ian Campbell wrote:
> On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
> > At the moment we destroy the DTB mappings we have in setup_pagetables
> > and we restore them only in setup_mm.
> > 
> > Move setup_mm right after setup_pagetables so that we don''t
leave over
> > portions of the initialization code without a working DTB mapping.
> 
> "leave over"? I think you just mean to say "This ensures we
have a valid
> DTB mapping while running the subsequent initialization code" or
> something like that.
right

> Do you still get the :
>         printk("Xen heap: %lu pages  Dom heap: %lu pages\n
> message with this change? Or do all the printks in this function need to
> become early_printk/early_panic?
Well spotted! I''ll turn it into an early_printk.

> Actually, this function has 1x early_printk, 1x printk and 1x panic,
> it''s a bit confused I think ;-)
After the update there are going to be 2 early_printk, but I don''t see
the panic you are referring to.


> > Signed-off-by: Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/arch/arm/setup.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 2076724..8882514 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -212,6 +212,7 @@ void __init start_xen(unsigned long
boot_phys_offset,
> >      cmdline_parse(device_tree_bootargs(fdt));
> >  
> >      setup_pagetables(boot_phys_offset, get_xen_paddr());
> > +    setup_mm(atag_paddr, fdt_size);
> >  
> >  #ifdef EARLY_UART_ADDRESS
> >      /* TODO Need to get device tree or command line for UART address
*/
> > @@ -229,8 +230,6 @@ void __init start_xen(unsigned long
boot_phys_offset,
> >  
> >      init_xen_time();
> >  
> > -    setup_mm(atag_paddr, fdt_size);
> > -
> >      /* Setup Hyp vector base */
> >      WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
> >      printk("Set hyp vector base to %"PRIx32" (expected
%p)\n",
> 
> 
>

On Thu, 10 Jan 2013, Ian Campbell wrote:
> On Tue, 2013-01-08 at 20:03 +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_va, that flushes a range of virtual
addresses.
> > Replace all the calls to flush_xen_data_tlb_va with calls to
> > flush_xen_data_tlb_range_va and remove flush_xen_data_tlb_va.
> > 
> > In the case of the dest_va tlb flush at the beginning of
> > setup_pagetables, flush the entire 2MB mapping rather than just the
> > first 4K.
> > 
> > 
> > Changes in v4:
> > - rename flush_xen_data_tlb_range to flush_xen_data_tlb_range_va;
> > - replace all the calls to flush_xen_data_tlb_va, with calls to
> > flush_xen_data_tlb_range_va;
> > - flush the entire 2MB mapping at BOOT_MISC_VIRT_START rather than
just
> > the first 4k;
> > - remove flush_xen_data_tlb_va;
> 
> These all seem like a separate patch (or patches) to me.
I created three other patches: one to introduce
flush_xen_data_tlb_range_va and replace all the calls to
flush_xen_data_tlb_va with calls to flush_xen_data_tlb_range_va; a
second one to fix the 2MB flush at the beginning of setup_pagetables;
and a third one to introduce the missing FIRST, SECOND and THIRD MASK
and SIZE definitions.

> > - fix indentation;
> > - rename EARLY_VMAP_START/END to EARLY_VMAP_VIRT_START/END;
> > - mark early_ioremap as __init;
> > - reduce the amount of casts in early_ioremap.
> 
> [...]
> >  /*
> > - * Flush one VA''s hypervisor mappings from the data TLB.
This is not
> > + * 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_va(unsigned long va)
> > +static inline void flush_xen_data_tlb_range_va(unsigned long va,
unsigned long size)
> >  {
> > -    asm volatile("dsb;" /* Ensure preceding are visible */
> > -                 STORE_CP32(0, TLBIMVAH)
> > -                 "dsb;" /* Ensure completion of the TLB
flush */
> > -                 "isb;"
> > -                 : : "r" (va) : "memory");
> > +    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;"
> 
> You can hoist all the barriers here to above/below the loop I think.
OK

On Mon, 2013-01-14 at 17:18 +0000, Stefano Stabellini
wrote:
> 
> > Actually, this function has 1x early_printk, 1x printk and 1x panic,
> > it''s a bit confused I think ;-)
> 
> After the update there are going to be 2 early_printk, but I don''t
see
> the panic you are referring to.
AH, it was in my boot-wrapper branch only:
    if ( ! e )
        panic("Not not enough space for xenheap\n");

Sorry about that.

I''ll take care of that in my eventual rebasing.

Ian.

On Thu, 2013-01-10 at 16:28 +0000, Keir Fraser wrote:
> On 10/01/2013 16:08, "Jan Beulich" <JBeulich@suse.com>
wrote:
> 
> >>>> On 10.01.13 at 16:49, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
> >> On Tue, 2013-01-08 at 20:03 +0000, Stefano Stabellini wrote:
> >>> - 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>
> >> 
> >> Jan, do you want to apply this or shall I take it along with the
ARM
> >> bits?
> >> 
> >> AIUI it has no dependency on patch 1/7.
> > 
> > Neither you nor I can validly apply this without Keir''s ack,
I''m
> > afraid.
> 
> Looks like just trivial renaming of stuff.
> 
> Acked-by: Keir Fraser <keir@xen.org>
Are you also OK with the following patch "xen: introduce a generic
framebuffer driver" (latest is v6 in
<1358276756-6189-6-git-send-email-stefano.stabellini@eu.citrix.com>)

AFAIK it is also largely code motion (from vesa.c to lfb.c)

Ian.